Compare commits

..

1 Commits

Author SHA1 Message Date
Claude
ae2fd1754a fix: prevent command injection in test-failure-analysis example
Fix command injection vulnerability where github.event.workflow_run.head_branch
was directly interpolated into shell commands. Branch names containing shell
metacharacters could execute arbitrary commands.

Changes:
- Pass head_branch through environment variables instead of direct interpolation
- Affects gh pr list --head and gh workflow run --ref commands
- Prevents execution of malicious code in branch names

Severity: HIGH
Category: command_injection
2025-12-13 20:47:58 +00:00
6 changed files with 13 additions and 143 deletions

View File

@@ -127,9 +127,6 @@ outputs:
structured_output: structured_output:
description: "JSON string containing all structured output fields when --json-schema is provided in claude_args. Use fromJSON() to parse: fromJSON(steps.id.outputs.structured_output).field_name" description: "JSON string containing all structured output fields when --json-schema is provided in claude_args. Use fromJSON() to parse: fromJSON(steps.id.outputs.structured_output).field_name"
value: ${{ steps.claude-code.outputs.structured_output }} value: ${{ steps.claude-code.outputs.structured_output }}
session_id:
description: "The Claude Code session ID that can be used with --resume to continue this conversation"
value: ${{ steps.claude-code.outputs.session_id }}
runs: runs:
using: "composite" using: "composite"

View File

@@ -82,9 +82,6 @@ outputs:
structured_output: structured_output:
description: "JSON string containing all structured output fields when --json-schema is provided in claude_args (use fromJSON() or jq to parse)" description: "JSON string containing all structured output fields when --json-schema is provided in claude_args (use fromJSON() or jq to parse)"
value: ${{ steps.run_claude.outputs.structured_output }} value: ${{ steps.run_claude.outputs.structured_output }}
session_id:
description: "The Claude Code session ID that can be used with --resume to continue this conversation"
value: ${{ steps.run_claude.outputs.session_id }}
runs: runs:
using: "composite" using: "composite"

View File

@@ -124,36 +124,6 @@ export function prepareRunConfig(
}; };
} }
/**
* Parses session_id from execution file and sets GitHub Action output
* Exported for testing
*/
export async function parseAndSetSessionId(
executionFile: string,
): Promise<void> {
try {
const content = await readFile(executionFile, "utf-8");
const messages = JSON.parse(content) as {
type: string;
subtype?: string;
session_id?: string;
}[];
// Find the system.init message which contains session_id
const initMessage = messages.find(
(m) => m.type === "system" && m.subtype === "init",
);
if (initMessage?.session_id) {
core.setOutput("session_id", initMessage.session_id);
core.info(`Set session_id: ${initMessage.session_id}`);
}
} catch (error) {
// Don't fail the action if session_id extraction fails
core.warning(`Failed to extract session_id: ${error}`);
}
}
/** /**
* Parses structured_output from execution file and sets GitHub Action outputs * Parses structured_output from execution file and sets GitHub Action outputs
* Only runs if --json-schema was explicitly provided in claude_args * Only runs if --json-schema was explicitly provided in claude_args
@@ -398,9 +368,6 @@ export async function runClaude(promptPath: string, options: ClaudeOptions) {
core.setOutput("execution_file", EXECUTION_FILE); core.setOutput("execution_file", EXECUTION_FILE);
// Extract and set session_id
await parseAndSetSessionId(EXECUTION_FILE);
// Parse and set structured outputs only if user provided --json-schema in claude_args // Parse and set structured outputs only if user provided --json-schema in claude_args
if (hasJsonSchema) { if (hasJsonSchema) {
try { try {

View File

@@ -4,10 +4,7 @@ import { describe, test, expect, afterEach, beforeEach, spyOn } from "bun:test";
import { writeFile, unlink } from "fs/promises"; import { writeFile, unlink } from "fs/promises";
import { tmpdir } from "os"; import { tmpdir } from "os";
import { join } from "path"; import { join } from "path";
import { import { parseAndSetStructuredOutputs } from "../src/run-claude";
parseAndSetStructuredOutputs,
parseAndSetSessionId,
} from "../src/run-claude";
import * as core from "@actions/core"; import * as core from "@actions/core";
// Mock execution file path // Mock execution file path
@@ -38,19 +35,16 @@ async function createMockExecutionFile(
// Spy on core functions // Spy on core functions
let setOutputSpy: any; let setOutputSpy: any;
let infoSpy: any; let infoSpy: any;
let warningSpy: any;
beforeEach(() => { beforeEach(() => {
setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {}); setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {});
infoSpy = spyOn(core, "info").mockImplementation(() => {}); infoSpy = spyOn(core, "info").mockImplementation(() => {});
warningSpy = spyOn(core, "warning").mockImplementation(() => {});
}); });
describe("parseAndSetStructuredOutputs", () => { describe("parseAndSetStructuredOutputs", () => {
afterEach(async () => { afterEach(async () => {
setOutputSpy?.mockRestore(); setOutputSpy?.mockRestore();
infoSpy?.mockRestore(); infoSpy?.mockRestore();
warningSpy?.mockRestore();
try { try {
await unlink(TEST_EXECUTION_FILE); await unlink(TEST_EXECUTION_FILE);
} catch { } catch {
@@ -162,66 +156,3 @@ describe("parseAndSetStructuredOutputs", () => {
); );
}); });
}); });
describe("parseAndSetSessionId", () => {
afterEach(async () => {
setOutputSpy?.mockRestore();
infoSpy?.mockRestore();
warningSpy?.mockRestore();
try {
await unlink(TEST_EXECUTION_FILE);
} catch {
// Ignore if file doesn't exist
}
});
test("should extract session_id from system.init message", async () => {
const messages = [
{ type: "system", subtype: "init", session_id: "test-session-123" },
{ type: "result", cost_usd: 0.01 },
];
await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
await parseAndSetSessionId(TEST_EXECUTION_FILE);
expect(setOutputSpy).toHaveBeenCalledWith("session_id", "test-session-123");
expect(infoSpy).toHaveBeenCalledWith("Set session_id: test-session-123");
});
test("should handle missing session_id gracefully", async () => {
const messages = [
{ type: "system", subtype: "init" },
{ type: "result", cost_usd: 0.01 },
];
await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
await parseAndSetSessionId(TEST_EXECUTION_FILE);
expect(setOutputSpy).not.toHaveBeenCalled();
});
test("should handle missing system.init message gracefully", async () => {
const messages = [{ type: "result", cost_usd: 0.01 }];
await writeFile(TEST_EXECUTION_FILE, JSON.stringify(messages));
await parseAndSetSessionId(TEST_EXECUTION_FILE);
expect(setOutputSpy).not.toHaveBeenCalled();
});
test("should handle malformed JSON gracefully with warning", async () => {
await writeFile(TEST_EXECUTION_FILE, "{ invalid json");
await parseAndSetSessionId(TEST_EXECUTION_FILE);
expect(setOutputSpy).not.toHaveBeenCalled();
expect(warningSpy).toHaveBeenCalled();
});
test("should handle non-existent file gracefully with warning", async () => {
await parseAndSetSessionId("/nonexistent/file.json");
expect(setOutputSpy).not.toHaveBeenCalled();
expect(warningSpy).toHaveBeenCalled();
});
});

View File

@@ -53,6 +53,7 @@ jobs:
fromJSON(steps.detect.outputs.structured_output).confidence >= 0.7 fromJSON(steps.detect.outputs.structured_output).confidence >= 0.7
env: env:
GH_TOKEN: ${{ github.token }} GH_TOKEN: ${{ github.token }}
HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }}
run: | run: |
OUTPUT='${{ steps.detect.outputs.structured_output }}' OUTPUT='${{ steps.detect.outputs.structured_output }}'
CONFIDENCE=$(echo "$OUTPUT" | jq -r '.confidence') CONFIDENCE=$(echo "$OUTPUT" | jq -r '.confidence')
@@ -64,7 +65,7 @@ jobs:
echo "Triggering automatic retry..." echo "Triggering automatic retry..."
gh workflow run "${{ github.event.workflow_run.name }}" \ gh workflow run "${{ github.event.workflow_run.name }}" \
--ref "${{ github.event.workflow_run.head_branch }}" --ref "$HEAD_BRANCH"
# Low confidence flaky detection - skip retry # Low confidence flaky detection - skip retry
- name: Low confidence detection - name: Low confidence detection
@@ -83,13 +84,14 @@ jobs:
if: github.event.workflow_run.event == 'pull_request' if: github.event.workflow_run.event == 'pull_request'
env: env:
GH_TOKEN: ${{ github.token }} GH_TOKEN: ${{ github.token }}
HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }}
run: | run: |
OUTPUT='${{ steps.detect.outputs.structured_output }}' OUTPUT='${{ steps.detect.outputs.structured_output }}'
IS_FLAKY=$(echo "$OUTPUT" | jq -r '.is_flaky') IS_FLAKY=$(echo "$OUTPUT" | jq -r '.is_flaky')
CONFIDENCE=$(echo "$OUTPUT" | jq -r '.confidence') CONFIDENCE=$(echo "$OUTPUT" | jq -r '.confidence')
SUMMARY=$(echo "$OUTPUT" | jq -r '.summary') SUMMARY=$(echo "$OUTPUT" | jq -r '.summary')
pr_number=$(gh pr list --head "${{ github.event.workflow_run.head_branch }}" --json number --jq '.[0].number') pr_number=$(gh pr list --head "$HEAD_BRANCH" --json number --jq '.[0].number')
if [ -n "$pr_number" ]; then if [ -n "$pr_number" ]; then
if [ "$IS_FLAKY" = "true" ]; then if [ "$IS_FLAKY" = "true" ]; then

View File

@@ -563,22 +563,9 @@ ${getCommitInstructions(eventData, githubData, context, useCommitSigning)}
${ ${
eventData.claudeBranch eventData.claudeBranch
? ` ? `
When done with changes: When done with changes, provide a PR link:
1. Run git log origin/${eventData.baseBranch}..HEAD and git diff origin/${eventData.baseBranch}...HEAD to understand ALL commits
2. Draft a PR summary analyzing ALL changes (not just the latest commit)
3. Provide a PR link:
[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...${eventData.claudeBranch}?quick_pull=1&title=<url-encoded-title>&body=<url-encoded-body>) [Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...${eventData.claudeBranch}?quick_pull=1&title=<url-encoded-title>&body=<url-encoded-body>)
Use THREE dots (...) between branches. URL-encode all parameters. Use THREE dots (...) between branches. URL-encode all parameters.`
PR body format:
## Summary
<1-3 bullet points>
## Test plan
<Checklist of testing TODOs>
Fixes #<issue-number>
Generated with [Claude Code](https://claude.ai/code)`
: "" : ""
} }
@@ -756,13 +743,8 @@ ${eventData.eventName === "issue_comment" || eventData.eventName === "pull_reque
- Mark each subtask as completed as you progress.${getCommitInstructions(eventData, githubData, context, useCommitSigning)} - Mark each subtask as completed as you progress.${getCommitInstructions(eventData, githubData, context, useCommitSigning)}
${ ${
eventData.claudeBranch eventData.claudeBranch
? `- When creating a pull request, follow these steps: ? `- Provide a URL to create a PR manually in this format:
1. Use git log and git diff to understand the full commit history for the current branch (from the time it diverged from the base branch): [Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...<branch-name>?quick_pull=1&title=<url-encoded-title>&body=<url-encoded-body>)
- Run: git log origin/${eventData.baseBranch}..HEAD
- Run: git diff origin/${eventData.baseBranch}...HEAD
2. Analyze ALL changes that will be included in the pull request, making sure to look at all relevant commits (NOT just the latest commit, but ALL commits that will be included in the pull request), and draft a pull request summary
3. Provide a URL to create a PR manually in this format:
[Create a PR](${GITHUB_SERVER_URL}/${context.repository}/compare/${eventData.baseBranch}...<branch-name>?quick_pull=1&title=<url-encoded-title>&body=<url-encoded-body>)
- IMPORTANT: Use THREE dots (...) between branch names, not two (..) - IMPORTANT: Use THREE dots (...) between branch names, not two (..)
Example: ${GITHUB_SERVER_URL}/${context.repository}/compare/main...feature-branch (correct) Example: ${GITHUB_SERVER_URL}/${context.repository}/compare/main...feature-branch (correct)
NOT: ${GITHUB_SERVER_URL}/${context.repository}/compare/main..feature-branch (incorrect) NOT: ${GITHUB_SERVER_URL}/${context.repository}/compare/main..feature-branch (incorrect)
@@ -770,16 +752,10 @@ ${eventData.eventName === "issue_comment" || eventData.eventName === "pull_reque
Example: Instead of "fix: update welcome message", use "fix%3A%20update%20welcome%20message" Example: Instead of "fix: update welcome message", use "fix%3A%20update%20welcome%20message"
- The target-branch should be '${eventData.baseBranch}'. - The target-branch should be '${eventData.baseBranch}'.
- The branch-name is the current branch: ${eventData.claudeBranch} - The branch-name is the current branch: ${eventData.claudeBranch}
- The PR body MUST follow this format: - The body should include:
## Summary - A clear description of the changes
<1-3 bullet points summarizing the changes> - Reference to the original ${eventData.isPR ? "PR" : "issue"}
- The signature: "Generated with [Claude Code](https://claude.ai/code)"
## Test plan
<Bulleted markdown checklist of TODOs for testing the pull request>
Fixes #<issue-number>
Generated with [Claude Code](https://claude.ai/code)
- Just include the markdown link with text "Create a PR" - do not add explanatory text before it like "You can create a PR using this link"` - Just include the markdown link with text "Create a PR" - do not add explanatory text before it like "You can create a PR using this link"`
: "" : ""
} }