Compare commits

..

1 Commits

Author SHA1 Message Date
Claude
fb6c6b51f4 fix: Encode branch names in URLs to prevent truncation in markdown links
Branch names containing special characters (particularly parentheses)
were breaking markdown links in GitHub comments. This caused URLs to be
truncated when clicked.

Changes:
- Add encodeBranchName() helper function that:
  - Uses encodeURIComponent for basic encoding
  - Preserves forward slashes (GitHub expects literal / in branch URLs)
  - Manually encodes parentheses (not encoded by encodeURIComponent per RFC 3986)
- Apply encoding to branch URLs in:
  - update-comment-link.ts (PR compare URLs)
  - branch-cleanup.ts (branch tree URLs)
  - comment-logic.ts (branch tree URLs)
  - comments/common.ts (branch tree URLs)
- Improve PR link regex to use greedy match with end anchor
- Add test for branch names with special characters
2025-12-09 06:34:34 +00:00
18 changed files with 124 additions and 734 deletions

View File

@@ -36,4 +36,4 @@ jobs:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
claude_args: |
--allowedTools "Bash(bun install),Bash(bun test:*),Bash(bun run format),Bash(bun typecheck)"
--model "claude-opus-4-5"
--model "claude-opus-4-1-20250805"

View File

@@ -127,9 +127,6 @@ outputs:
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"
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:
using: "composite"
@@ -198,7 +195,7 @@ runs:
# Install Claude Code if no custom executable is provided
if [ -z "$PATH_TO_CLAUDE_CODE_EXECUTABLE" ]; then
CLAUDE_CODE_VERSION="2.0.70"
CLAUDE_CODE_VERSION="2.0.62"
echo "Installing Claude Code v${CLAUDE_CODE_VERSION}..."
for attempt in 1 2 3; do
echo "Installation attempt $attempt..."

View File

@@ -82,9 +82,6 @@ outputs:
structured_output:
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 }}
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:
using: "composite"
@@ -124,7 +121,7 @@ runs:
PATH_TO_CLAUDE_CODE_EXECUTABLE: ${{ inputs.path_to_claude_code_executable }}
run: |
if [ -z "$PATH_TO_CLAUDE_CODE_EXECUTABLE" ]; then
CLAUDE_CODE_VERSION="2.0.70"
CLAUDE_CODE_VERSION="2.0.62"
echo "Installing Claude Code v${CLAUDE_CODE_VERSION}..."
for attempt in 1 2 3; do
echo "Installation attempt $attempt..."

View File

@@ -1,12 +1,11 @@
{
"lockfileVersion": 1,
"configVersion": 0,
"workspaces": {
"": {
"name": "@anthropic-ai/claude-code-base-action",
"dependencies": {
"@actions/core": "^1.10.1",
"@anthropic-ai/claude-agent-sdk": "^0.1.70",
"@anthropic-ai/claude-agent-sdk": "^0.1.52",
"shell-quote": "^1.8.3",
},
"devDependencies": {
@@ -27,7 +26,7 @@
"@actions/io": ["@actions/io@1.1.3", "", {}, "sha512-wi9JjgKLYS7U/z8PPbco+PvTb/nRWjeoFlJ1Qer83k/3C5PHQi28hiVdeE2kHXmIL99mQFawx8qt/JPjZilJ8Q=="],
"@anthropic-ai/claude-agent-sdk": ["@anthropic-ai/claude-agent-sdk@0.1.70", "", { "optionalDependencies": { "@img/sharp-darwin-arm64": "^0.33.5", "@img/sharp-darwin-x64": "^0.33.5", "@img/sharp-linux-arm": "^0.33.5", "@img/sharp-linux-arm64": "^0.33.5", "@img/sharp-linux-x64": "^0.33.5", "@img/sharp-linuxmusl-arm64": "^0.33.5", "@img/sharp-linuxmusl-x64": "^0.33.5", "@img/sharp-win32-x64": "^0.33.5" }, "peerDependencies": { "zod": "^3.24.1" } }, "sha512-4jpFPDX8asys6skO1r3Pzh0Fe9nbND2ASYTWuyFB5iN9bWEL6WScTFyGokjql3M2TkEp9ZGuB2YYpTCdaqT9Sw=="],
"@anthropic-ai/claude-agent-sdk": ["@anthropic-ai/claude-agent-sdk@0.1.52", "", { "optionalDependencies": { "@img/sharp-darwin-arm64": "^0.33.5", "@img/sharp-darwin-x64": "^0.33.5", "@img/sharp-linux-arm": "^0.33.5", "@img/sharp-linux-arm64": "^0.33.5", "@img/sharp-linux-x64": "^0.33.5", "@img/sharp-linuxmusl-arm64": "^0.33.5", "@img/sharp-linuxmusl-x64": "^0.33.5", "@img/sharp-win32-x64": "^0.33.5" }, "peerDependencies": { "zod": "^3.24.1" } }, "sha512-yF8N05+9NRbqYA/h39jQ726HTQFrdXXp7pEfDNKIJ2c4FdWvEjxBA/8ciZIebN6/PyvGDcbEp3yq2Co4rNpg6A=="],
"@fastify/busboy": ["@fastify/busboy@2.1.1", "", {}, "sha512-vBZP4NlzfOlerQTnba4aqZoMhE/a9HY7HRqoOPaETQcSQuWEIyZMHGfVu6w9wGtGK5fED5qRs2DteVCjOH60sA=="],

View File

@@ -11,7 +11,7 @@
},
"dependencies": {
"@actions/core": "^1.10.1",
"@anthropic-ai/claude-agent-sdk": "^0.1.70",
"@anthropic-ai/claude-agent-sdk": "^0.1.52",
"shell-quote": "^1.8.3"
},
"devDependencies": {

View File

@@ -12,79 +12,12 @@ export type ParsedSdkOptions = {
};
// Flags that should accumulate multiple values instead of overwriting
// Include both camelCase and hyphenated variants for CLI compatibility
const ACCUMULATING_FLAGS = new Set([
"allowedTools",
"allowed-tools",
"disallowedTools",
"disallowed-tools",
"mcp-config",
]);
// Delimiter used to join accumulated flag values
const ACCUMULATE_DELIMITER = "\x00";
type McpConfig = {
mcpServers?: Record<string, unknown>;
};
/**
* Merge multiple MCP config values into a single config.
* Each config can be a JSON string or a file path.
* For JSON strings, mcpServers objects are merged.
* For file paths, they are kept as-is (user's file takes precedence and is used last).
*/
function mergeMcpConfigs(configValues: string[]): string {
const merged: McpConfig = { mcpServers: {} };
let lastFilePath: string | null = null;
for (const config of configValues) {
const trimmed = config.trim();
if (!trimmed) continue;
// Check if it's a JSON string (starts with {) or a file path
if (trimmed.startsWith("{")) {
try {
const parsed = JSON.parse(trimmed) as McpConfig;
if (parsed.mcpServers) {
Object.assign(merged.mcpServers!, parsed.mcpServers);
}
} catch {
// If JSON parsing fails, treat as file path
lastFilePath = trimmed;
}
} else {
// It's a file path - store it to handle separately
lastFilePath = trimmed;
}
}
// If we have file paths, we need to keep the merged JSON and let the file
// be handled separately. Since we can only return one value, merge what we can.
// If there's a file path, we need a different approach - read the file at runtime.
// For now, if there's a file path, we'll stringify the merged config.
// The action prepends its config as JSON, so we can safely merge inline JSON configs.
// If no inline configs were found (all file paths), return the last file path
if (Object.keys(merged.mcpServers!).length === 0 && lastFilePath) {
return lastFilePath;
}
// Note: If user passes a file path, we cannot merge it at parse time since
// we don't have access to the file system here. The action's built-in MCP
// servers are always passed as inline JSON, so they will be merged.
// If user also passes inline JSON, it will be merged.
// If user passes a file path, they should ensure it includes all needed servers.
return JSON.stringify(merged);
}
const ACCUMULATING_FLAGS = new Set(["allowedTools", "disallowedTools"]);
/**
* Parse claudeArgs string into extraArgs record for SDK pass-through
* The SDK/CLI will handle --mcp-config, --json-schema, etc.
* For allowedTools and disallowedTools, multiple occurrences are accumulated (null-char joined).
* Accumulating flags also consume all consecutive non-flag values
* (e.g., --allowed-tools "Tool1" "Tool2" "Tool3" captures all three).
* For allowedTools and disallowedTools, multiple occurrences are accumulated (comma-joined).
*/
function parseClaudeArgsToExtraArgs(
claudeArgs?: string,
@@ -104,25 +37,13 @@ function parseClaudeArgsToExtraArgs(
// Check if next arg is a value (not another flag)
if (nextArg && !nextArg.startsWith("--")) {
// For accumulating flags, consume all consecutive non-flag values
// This handles: --allowed-tools "Tool1" "Tool2" "Tool3"
if (ACCUMULATING_FLAGS.has(flag)) {
const values: string[] = [];
while (i + 1 < args.length && !args[i + 1]?.startsWith("--")) {
i++;
values.push(args[i]!);
}
const joinedValues = values.join(ACCUMULATE_DELIMITER);
if (result[flag]) {
result[flag] =
`${result[flag]}${ACCUMULATE_DELIMITER}${joinedValues}`;
} else {
result[flag] = joinedValues;
}
// For accumulating flags, join multiple values with commas
if (ACCUMULATING_FLAGS.has(flag) && result[flag]) {
result[flag] = `${result[flag]},${nextArg}`;
} else {
result[flag] = nextArg;
i++; // Skip the value
}
i++; // Skip the value
} else {
result[flag] = null; // Boolean flag
}
@@ -147,23 +68,12 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
// Detect if --json-schema is present (for hasJsonSchema flag)
const hasJsonSchema = "json-schema" in extraArgs;
// Extract and merge allowedTools from all sources:
// Extract and merge allowedTools from both sources:
// 1. From extraArgs (parsed from claudeArgs - contains tag mode's tools)
// - Check both camelCase (--allowedTools) and hyphenated (--allowed-tools) variants
// 2. From options.allowedTools (direct input - may be undefined)
// This prevents duplicate flags being overwritten when claudeArgs contains --allowedTools
const allowedToolsValues = [
extraArgs["allowedTools"],
extraArgs["allowed-tools"],
]
.filter(Boolean)
.join(ACCUMULATE_DELIMITER);
const extraArgsAllowedTools = allowedToolsValues
? allowedToolsValues
.split(ACCUMULATE_DELIMITER)
.flatMap((v) => v.split(","))
.map((t) => t.trim())
.filter(Boolean)
const extraArgsAllowedTools = extraArgs["allowedTools"]
? extraArgs["allowedTools"].split(",").map((t) => t.trim())
: [];
const directAllowedTools = options.allowedTools
? options.allowedTools.split(",").map((t) => t.trim())
@@ -172,21 +82,10 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
...new Set([...extraArgsAllowedTools, ...directAllowedTools]),
];
delete extraArgs["allowedTools"];
delete extraArgs["allowed-tools"];
// Same for disallowedTools - check both camelCase and hyphenated variants
const disallowedToolsValues = [
extraArgs["disallowedTools"],
extraArgs["disallowed-tools"],
]
.filter(Boolean)
.join(ACCUMULATE_DELIMITER);
const extraArgsDisallowedTools = disallowedToolsValues
? disallowedToolsValues
.split(ACCUMULATE_DELIMITER)
.flatMap((v) => v.split(","))
.map((t) => t.trim())
.filter(Boolean)
// Same for disallowedTools
const extraArgsDisallowedTools = extraArgs["disallowedTools"]
? extraArgs["disallowedTools"].split(",").map((t) => t.trim())
: [];
const directDisallowedTools = options.disallowedTools
? options.disallowedTools.split(",").map((t) => t.trim())
@@ -195,17 +94,6 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
...new Set([...extraArgsDisallowedTools, ...directDisallowedTools]),
];
delete extraArgs["disallowedTools"];
delete extraArgs["disallowed-tools"];
// Merge multiple --mcp-config values by combining their mcpServers objects
// The action prepends its config (github_comment, github_ci, etc.) as inline JSON,
// and users may provide their own config as inline JSON or file path
if (extraArgs["mcp-config"]) {
const mcpConfigValues = extraArgs["mcp-config"].split(ACCUMULATE_DELIMITER);
if (mcpConfigValues.length > 1) {
extraArgs["mcp-config"] = mergeMcpConfigs(mcpConfigValues);
}
}
// Build custom environment
const env: Record<string, string | undefined> = { ...process.env };

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
* Only runs if --json-schema was explicitly provided in claude_args
@@ -197,8 +167,8 @@ export async function parseAndSetStructuredOutputs(
}
export async function runClaude(promptPath: string, options: ClaudeOptions) {
// Feature flag: use SDK path by default, set USE_AGENT_SDK=false to use CLI
const useAgentSdk = process.env.USE_AGENT_SDK !== "false";
// Feature flag: use SDK path when USE_AGENT_SDK=true
const useAgentSdk = process.env.USE_AGENT_SDK === "true";
console.log(
`Using ${useAgentSdk ? "Agent SDK" : "CLI"} path (USE_AGENT_SDK=${process.env.USE_AGENT_SDK ?? "unset"})`,
);
@@ -398,9 +368,6 @@ export async function runClaude(promptPath: string, options: ClaudeOptions) {
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
if (hasJsonSchema) {
try {

View File

@@ -108,48 +108,6 @@ describe("parseSdkOptions", () => {
expect(result.sdkOptions.extraArgs?.["allowedTools"]).toBeUndefined();
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
});
test("should handle hyphenated --allowed-tools flag", () => {
const options: ClaudeOptions = {
claudeArgs: '--allowed-tools "Edit,Read,Write"',
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual(["Edit", "Read", "Write"]);
expect(result.sdkOptions.extraArgs?.["allowed-tools"]).toBeUndefined();
});
test("should accumulate multiple --allowed-tools flags (hyphenated)", () => {
// This is the exact scenario from issue #746
const options: ClaudeOptions = {
claudeArgs:
'--allowed-tools "Bash(git log:*)" "Bash(git diff:*)" "Bash(git fetch:*)" "Bash(gh pr:*)"',
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual([
"Bash(git log:*)",
"Bash(git diff:*)",
"Bash(git fetch:*)",
"Bash(gh pr:*)",
]);
});
test("should handle mixed camelCase and hyphenated allowedTools flags", () => {
const options: ClaudeOptions = {
claudeArgs: '--allowedTools "Edit,Read" --allowed-tools "Write,Glob"',
};
const result = parseSdkOptions(options);
// Both should be merged - note: order depends on which key is found first
expect(result.sdkOptions.allowedTools).toContain("Edit");
expect(result.sdkOptions.allowedTools).toContain("Read");
expect(result.sdkOptions.allowedTools).toContain("Write");
expect(result.sdkOptions.allowedTools).toContain("Glob");
});
});
describe("disallowedTools merging", () => {
@@ -176,129 +134,19 @@ describe("parseSdkOptions", () => {
});
});
describe("mcp-config merging", () => {
test("should pass through single mcp-config in extraArgs", () => {
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '{"mcpServers":{"server1":{"command":"cmd1"}}}'`,
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.extraArgs?.["mcp-config"]).toBe(
'{"mcpServers":{"server1":{"command":"cmd1"}}}',
);
});
test("should merge multiple mcp-config flags with inline JSON", () => {
// Simulates action prepending its config, then user providing their own
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '{"mcpServers":{"github_comment":{"command":"node","args":["server.js"]}}}' --mcp-config '{"mcpServers":{"user_server":{"command":"custom","args":["run"]}}}'`,
};
const result = parseSdkOptions(options);
const mcpConfig = JSON.parse(
result.sdkOptions.extraArgs?.["mcp-config"] as string,
);
expect(mcpConfig.mcpServers).toHaveProperty("github_comment");
expect(mcpConfig.mcpServers).toHaveProperty("user_server");
expect(mcpConfig.mcpServers.github_comment.command).toBe("node");
expect(mcpConfig.mcpServers.user_server.command).toBe("custom");
});
test("should merge three mcp-config flags", () => {
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '{"mcpServers":{"server1":{"command":"cmd1"}}}' --mcp-config '{"mcpServers":{"server2":{"command":"cmd2"}}}' --mcp-config '{"mcpServers":{"server3":{"command":"cmd3"}}}'`,
};
const result = parseSdkOptions(options);
const mcpConfig = JSON.parse(
result.sdkOptions.extraArgs?.["mcp-config"] as string,
);
expect(mcpConfig.mcpServers).toHaveProperty("server1");
expect(mcpConfig.mcpServers).toHaveProperty("server2");
expect(mcpConfig.mcpServers).toHaveProperty("server3");
});
test("should handle mcp-config file path when no inline JSON exists", () => {
const options: ClaudeOptions = {
claudeArgs: `--mcp-config /tmp/user-mcp-config.json`,
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.extraArgs?.["mcp-config"]).toBe(
"/tmp/user-mcp-config.json",
);
});
test("should merge inline JSON configs when file path is also present", () => {
// When action provides inline JSON and user provides a file path,
// the inline JSON configs should be merged (file paths cannot be merged at parse time)
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '{"mcpServers":{"github_comment":{"command":"node"}}}' --mcp-config '{"mcpServers":{"github_ci":{"command":"node"}}}' --mcp-config /tmp/user-config.json`,
};
const result = parseSdkOptions(options);
// The inline JSON configs should be merged
const mcpConfig = JSON.parse(
result.sdkOptions.extraArgs?.["mcp-config"] as string,
);
expect(mcpConfig.mcpServers).toHaveProperty("github_comment");
expect(mcpConfig.mcpServers).toHaveProperty("github_ci");
});
test("should handle mcp-config with other flags", () => {
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '{"mcpServers":{"server1":{}}}' --model claude-3-5-sonnet --mcp-config '{"mcpServers":{"server2":{}}}'`,
};
const result = parseSdkOptions(options);
const mcpConfig = JSON.parse(
result.sdkOptions.extraArgs?.["mcp-config"] as string,
);
expect(mcpConfig.mcpServers).toHaveProperty("server1");
expect(mcpConfig.mcpServers).toHaveProperty("server2");
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
});
test("should handle real-world scenario: action config + user config", () => {
// This is the exact scenario from the bug report
const actionConfig = JSON.stringify({
mcpServers: {
github_comment: {
command: "node",
args: ["github-comment-server.js"],
},
github_ci: { command: "node", args: ["github-ci-server.js"] },
},
});
const userConfig = JSON.stringify({
mcpServers: {
my_custom_server: { command: "python", args: ["server.py"] },
},
});
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '${actionConfig}' --mcp-config '${userConfig}'`,
};
const result = parseSdkOptions(options);
const mcpConfig = JSON.parse(
result.sdkOptions.extraArgs?.["mcp-config"] as string,
);
// All servers should be present
expect(mcpConfig.mcpServers).toHaveProperty("github_comment");
expect(mcpConfig.mcpServers).toHaveProperty("github_ci");
expect(mcpConfig.mcpServers).toHaveProperty("my_custom_server");
});
});
describe("other extraArgs passthrough", () => {
test("should pass through mcp-config in extraArgs", () => {
const options: ClaudeOptions = {
claudeArgs: `--mcp-config '{"mcpServers":{}}' --allowedTools "Edit"`,
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.extraArgs?.["mcp-config"]).toBe(
'{"mcpServers":{}}',
);
});
test("should pass through json-schema in extraArgs", () => {
const options: ClaudeOptions = {
claudeArgs: `--json-schema '{"type":"object"}'`,

View File

@@ -4,10 +4,7 @@ import { describe, test, expect, afterEach, beforeEach, spyOn } from "bun:test";
import { writeFile, unlink } from "fs/promises";
import { tmpdir } from "os";
import { join } from "path";
import {
parseAndSetStructuredOutputs,
parseAndSetSessionId,
} from "../src/run-claude";
import { parseAndSetStructuredOutputs } from "../src/run-claude";
import * as core from "@actions/core";
// Mock execution file path
@@ -38,19 +35,16 @@ async function createMockExecutionFile(
// Spy on core functions
let setOutputSpy: any;
let infoSpy: any;
let warningSpy: any;
beforeEach(() => {
setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {});
infoSpy = spyOn(core, "info").mockImplementation(() => {});
warningSpy = spyOn(core, "warning").mockImplementation(() => {});
});
describe("parseAndSetStructuredOutputs", () => {
afterEach(async () => {
setOutputSpy?.mockRestore();
infoSpy?.mockRestore();
warningSpy?.mockRestore();
try {
await unlink(TEST_EXECUTION_FILE);
} 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

@@ -1,13 +1,12 @@
{
"lockfileVersion": 1,
"configVersion": 0,
"workspaces": {
"": {
"name": "@anthropic-ai/claude-code-action",
"dependencies": {
"@actions/core": "^1.10.1",
"@actions/github": "^6.0.1",
"@anthropic-ai/claude-agent-sdk": "^0.1.70",
"@anthropic-ai/claude-agent-sdk": "^0.1.52",
"@modelcontextprotocol/sdk": "^1.11.0",
"@octokit/graphql": "^8.2.2",
"@octokit/rest": "^21.1.1",
@@ -37,7 +36,7 @@
"@actions/io": ["@actions/io@1.1.3", "", {}, "sha512-wi9JjgKLYS7U/z8PPbco+PvTb/nRWjeoFlJ1Qer83k/3C5PHQi28hiVdeE2kHXmIL99mQFawx8qt/JPjZilJ8Q=="],
"@anthropic-ai/claude-agent-sdk": ["@anthropic-ai/claude-agent-sdk@0.1.70", "", { "optionalDependencies": { "@img/sharp-darwin-arm64": "^0.33.5", "@img/sharp-darwin-x64": "^0.33.5", "@img/sharp-linux-arm": "^0.33.5", "@img/sharp-linux-arm64": "^0.33.5", "@img/sharp-linux-x64": "^0.33.5", "@img/sharp-linuxmusl-arm64": "^0.33.5", "@img/sharp-linuxmusl-x64": "^0.33.5", "@img/sharp-win32-x64": "^0.33.5" }, "peerDependencies": { "zod": "^3.24.1" } }, "sha512-4jpFPDX8asys6skO1r3Pzh0Fe9nbND2ASYTWuyFB5iN9bWEL6WScTFyGokjql3M2TkEp9ZGuB2YYpTCdaqT9Sw=="],
"@anthropic-ai/claude-agent-sdk": ["@anthropic-ai/claude-agent-sdk@0.1.52", "", { "optionalDependencies": { "@img/sharp-darwin-arm64": "^0.33.5", "@img/sharp-darwin-x64": "^0.33.5", "@img/sharp-linux-arm": "^0.33.5", "@img/sharp-linux-arm64": "^0.33.5", "@img/sharp-linux-x64": "^0.33.5", "@img/sharp-linuxmusl-arm64": "^0.33.5", "@img/sharp-linuxmusl-x64": "^0.33.5", "@img/sharp-win32-x64": "^0.33.5" }, "peerDependencies": { "zod": "^3.24.1" } }, "sha512-yF8N05+9NRbqYA/h39jQ726HTQFrdXXp7pEfDNKIJ2c4FdWvEjxBA/8ciZIebN6/PyvGDcbEp3yq2Co4rNpg6A=="],
"@fastify/busboy": ["@fastify/busboy@2.1.1", "", {}, "sha512-vBZP4NlzfOlerQTnba4aqZoMhE/a9HY7HRqoOPaETQcSQuWEIyZMHGfVu6w9wGtGK5fED5qRs2DteVCjOH60sA=="],

View File

@@ -12,7 +12,7 @@
"dependencies": {
"@actions/core": "^1.10.1",
"@actions/github": "^6.0.1",
"@anthropic-ai/claude-agent-sdk": "^0.1.70",
"@anthropic-ai/claude-agent-sdk": "^0.1.52",
"@modelcontextprotocol/sdk": "^1.11.0",
"@octokit/graphql": "^8.2.2",
"@octokit/rest": "^21.1.1",

View File

@@ -15,6 +15,20 @@ import { GITHUB_SERVER_URL } from "../github/api/config";
import { checkAndCommitOrDeleteBranch } from "../github/operations/branch-cleanup";
import { updateClaudeComment } from "../github/operations/comments/update-claude-comment";
/**
* Encodes a branch name for use in a URL, preserving forward slashes.
* GitHub expects literal slashes in branch names (e.g., /tree/feature/branch)
* but other special characters like parentheses need to be encoded.
* Note: encodeURIComponent doesn't encode ( ) ! ' * ~ per RFC 3986,
* but parentheses break markdown links so we encode them manually.
*/
function encodeBranchName(branchName: string): string {
return encodeURIComponent(branchName)
.replace(/%2F/gi, "/")
.replace(/\(/g, "%28")
.replace(/\)/g, "%29");
}
async function run() {
try {
const commentId = parseInt(process.env.CLAUDE_COMMENT_ID!);
@@ -140,7 +154,7 @@ async function run() {
const prBody = encodeURIComponent(
`This PR addresses ${entityType.toLowerCase()} #${context.entityNumber}\n\nGenerated with [Claude Code](https://claude.ai/code)`,
);
const prUrl = `${serverUrl}/${owner}/${repo}/compare/${baseBranch}...${claudeBranch}?quick_pull=1&title=${prTitle}&body=${prBody}`;
const prUrl = `${serverUrl}/${owner}/${repo}/compare/${encodeBranchName(baseBranch)}...${encodeBranchName(claudeBranch)}?quick_pull=1&title=${prTitle}&body=${prBody}`;
prLink = `\n[Create a PR](${prUrl})`;
}
} catch (error) {

View File

@@ -2,6 +2,20 @@ import type { Octokits } from "../api/client";
import { GITHUB_SERVER_URL } from "../api/config";
import { $ } from "bun";
/**
* Encodes a branch name for use in a URL, preserving forward slashes.
* GitHub expects literal slashes in branch names (e.g., /tree/feature/branch)
* but other special characters like parentheses need to be encoded.
* Note: encodeURIComponent doesn't encode ( ) ! ' * ~ per RFC 3986,
* but parentheses break markdown links so we encode them manually.
*/
function encodeBranchName(branchName: string): string {
return encodeURIComponent(branchName)
.replace(/%2F/gi, "/")
.replace(/\(/g, "%28")
.replace(/\)/g, "%29");
}
export async function checkAndCommitOrDeleteBranch(
octokit: Octokits,
owner: string,
@@ -80,7 +94,7 @@ export async function checkAndCommitOrDeleteBranch(
);
// Set branch link since we now have commits
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${encodeBranchName(claudeBranch)}`;
branchLink = `\n[View branch](${branchUrl})`;
} else {
console.log(
@@ -91,7 +105,7 @@ export async function checkAndCommitOrDeleteBranch(
} catch (gitError) {
console.error("Error checking/committing changes:", gitError);
// If we can't check git status, assume the branch might have changes
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${encodeBranchName(claudeBranch)}`;
branchLink = `\n[View branch](${branchUrl})`;
}
} else {
@@ -102,13 +116,13 @@ export async function checkAndCommitOrDeleteBranch(
}
} else {
// Only add branch link if there are commits
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${encodeBranchName(claudeBranch)}`;
branchLink = `\n[View branch](${branchUrl})`;
}
} catch (error) {
console.error("Error comparing commits on Claude branch:", error);
// If we can't compare but the branch exists remotely, include the branch link
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${encodeBranchName(claudeBranch)}`;
branchLink = `\n[View branch](${branchUrl})`;
}
}

View File

@@ -6,112 +6,13 @@
* - For Issues: Create a new branch
*/
import { execFileSync } from "child_process";
import { $ } from "bun";
import * as core from "@actions/core";
import type { ParsedGitHubContext } from "../context";
import type { GitHubPullRequest } from "../types";
import type { Octokits } from "../api/client";
import type { FetchDataResult } from "../data/fetcher";
/**
* Validates a git branch name against a strict whitelist pattern.
* This prevents command injection by ensuring only safe characters are used.
*
* Valid branch names:
* - Start with alphanumeric character (not dash, to prevent option injection)
* - Contain only alphanumeric, forward slash, hyphen, underscore, or period
* - Do not start or end with a period
* - Do not end with a slash
* - Do not contain '..' (path traversal)
* - Do not contain '//' (consecutive slashes)
* - Do not end with '.lock'
* - Do not contain '@{'
* - Do not contain control characters or special git characters (~^:?*[\])
*/
export function validateBranchName(branchName: string): void {
// Check for empty or whitespace-only names
if (!branchName || branchName.trim().length === 0) {
throw new Error("Branch name cannot be empty");
}
// Check for leading dash (prevents option injection like --help, -x)
if (branchName.startsWith("-")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot start with a dash.`,
);
}
// Check for control characters and special git characters (~^:?*[\])
// eslint-disable-next-line no-control-regex
if (/[\x00-\x1F\x7F ~^:?*[\]\\]/.test(branchName)) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain control characters, spaces, or special git characters (~^:?*[\\]).`,
);
}
// Strict whitelist pattern: alphanumeric start, then alphanumeric/slash/hyphen/underscore/period
const validPattern = /^[a-zA-Z0-9][a-zA-Z0-9/_.-]*$/;
if (!validPattern.test(branchName)) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names must start with an alphanumeric character and contain only alphanumeric characters, forward slashes, hyphens, underscores, or periods.`,
);
}
// Check for leading/trailing periods
if (branchName.startsWith(".") || branchName.endsWith(".")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot start or end with a period.`,
);
}
// Check for trailing slash
if (branchName.endsWith("/")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot end with a slash.`,
);
}
// Check for consecutive slashes
if (branchName.includes("//")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain consecutive slashes.`,
);
}
// Additional git-specific validations
if (branchName.includes("..")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain '..'`,
);
}
if (branchName.endsWith(".lock")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot end with '.lock'`,
);
}
if (branchName.includes("@{")) {
throw new Error(
`Invalid branch name: "${branchName}". Branch names cannot contain '@{'`,
);
}
}
/**
* Executes a git command safely using execFileSync to avoid shell interpolation.
*
* Security: execFileSync passes arguments directly to the git binary without
* invoking a shell, preventing command injection attacks where malicious input
* could be interpreted as shell commands (e.g., branch names containing `;`, `|`, `&&`).
*
* @param args - Git command arguments (e.g., ["checkout", "branch-name"])
*/
function execGit(args: string[]): void {
execFileSync("git", args, { stdio: "inherit" });
}
export type BranchInfo = {
baseBranch: string;
claudeBranch?: string;
@@ -152,19 +53,14 @@ export async function setupBranch(
`PR #${entityNumber}: ${commitCount} commits, using fetch depth ${fetchDepth}`,
);
// Validate branch names before use to prevent command injection
validateBranchName(branchName);
// Execute git commands to checkout PR branch (dynamic depth based on PR size)
// Using execFileSync instead of shell template literals for security
execGit(["fetch", "origin", `--depth=${fetchDepth}`, branchName]);
execGit(["checkout", branchName, "--"]);
await $`git fetch origin --depth=${fetchDepth} ${branchName}`;
await $`git checkout ${branchName} --`;
console.log(`Successfully checked out PR branch for PR #${entityNumber}`);
// For open PRs, we need to get the base branch of the PR
const baseBranch = prData.baseRefName;
validateBranchName(baseBranch);
return {
baseBranch,
@@ -222,9 +118,8 @@ export async function setupBranch(
// Ensure we're on the source branch
console.log(`Fetching and checking out source branch: ${sourceBranch}`);
validateBranchName(sourceBranch);
execGit(["fetch", "origin", sourceBranch, "--depth=1"]);
execGit(["checkout", sourceBranch, "--"]);
await $`git fetch origin ${sourceBranch} --depth=1`;
await $`git checkout ${sourceBranch}`;
// Set outputs for GitHub Actions
core.setOutput("CLAUDE_BRANCH", newBranch);
@@ -243,13 +138,11 @@ export async function setupBranch(
// Fetch and checkout the source branch first to ensure we branch from the correct base
console.log(`Fetching and checking out source branch: ${sourceBranch}`);
validateBranchName(sourceBranch);
validateBranchName(newBranch);
execGit(["fetch", "origin", sourceBranch, "--depth=1"]);
execGit(["checkout", sourceBranch, "--"]);
await $`git fetch origin ${sourceBranch} --depth=1`;
await $`git checkout ${sourceBranch}`;
// Create and checkout the new branch from the source branch
execGit(["checkout", "-b", newBranch]);
await $`git checkout -b ${newBranch}`;
console.log(
`Successfully created and checked out local branch: ${newBranch}`,

View File

@@ -1,5 +1,19 @@
import { GITHUB_SERVER_URL } from "../api/config";
/**
* Encodes a branch name for use in a URL, preserving forward slashes.
* GitHub expects literal slashes in branch names (e.g., /tree/feature/branch)
* but other special characters like parentheses need to be encoded.
* Note: encodeURIComponent doesn't encode ( ) ! ' * ~ per RFC 3986,
* but parentheses break markdown links so we encode them manually.
*/
function encodeBranchName(branchName: string): string {
return encodeURIComponent(branchName)
.replace(/%2F/gi, "/")
.replace(/\(/g, "%28")
.replace(/\)/g, "%29");
}
export type ExecutionDetails = {
total_cost_usd?: number;
duration_ms?: number;
@@ -160,7 +174,7 @@ export function updateCommentBody(input: CommentUpdateInput): string {
// Extract owner/repo from jobUrl
const repoMatch = jobUrl.match(/github\.com\/([^\/]+)\/([^\/]+)\//);
if (repoMatch) {
branchUrl = `${GITHUB_SERVER_URL}/${repoMatch[1]}/${repoMatch[2]}/tree/${finalBranchName}`;
branchUrl = `${GITHUB_SERVER_URL}/${repoMatch[1]}/${repoMatch[2]}/tree/${encodeBranchName(finalBranchName)}`;
}
}
@@ -172,8 +186,9 @@ export function updateCommentBody(input: CommentUpdateInput): string {
}
// Add PR link (either from content or provided)
// Use greedy match with end anchor to capture full URL even if it contains parentheses
const prUrl =
prLinkFromContent || (prLink ? prLink.match(/\(([^)]+)\)/)?.[1] : "");
prLinkFromContent || (prLink ? prLink.match(/\((.+)\)$/)?.[1] : "");
if (prUrl) {
links += ` • [Create PR ➔](${prUrl})`;
}

View File

@@ -12,12 +12,26 @@ export function createJobRunLink(
return `[View job run](${jobRunUrl})`;
}
/**
* Encodes a branch name for use in a URL, preserving forward slashes.
* GitHub expects literal slashes in branch names (e.g., /tree/feature/branch)
* but other special characters like parentheses need to be encoded.
* Note: encodeURIComponent doesn't encode ( ) ! ' * ~ per RFC 3986,
* but parentheses break markdown links so we encode them manually.
*/
function encodeBranchName(branchName: string): string {
return encodeURIComponent(branchName)
.replace(/%2F/gi, "/")
.replace(/\(/g, "%28")
.replace(/\)/g, "%29");
}
export function createBranchLink(
owner: string,
repo: string,
branchName: string,
): string {
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${branchName}`;
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${encodeBranchName(branchName)}`;
return `\n[View branch](${branchUrl})`;
}

View File

@@ -139,6 +139,21 @@ describe("updateCommentBody", () => {
);
expect(result).not.toContain("View branch");
});
it("encodes special characters in branch names while preserving slashes", () => {
const input = {
...baseInput,
branchName: "feature/fix(issue)-test",
};
const result = updateCommentBody(input);
// Branch name display should show the original name
expect(result).toContain("`feature/fix(issue)-test`");
// URL should have encoded parentheses but preserved slashes
expect(result).toContain(
"https://github.com/owner/repo/tree/feature/fix%28issue%29-test",
);
});
});
describe("PR link", () => {

View File

@@ -1,201 +0,0 @@
import { describe, expect, it } from "bun:test";
import { validateBranchName } from "../src/github/operations/branch";
describe("validateBranchName", () => {
describe("valid branch names", () => {
it("should accept simple alphanumeric names", () => {
expect(() => validateBranchName("main")).not.toThrow();
expect(() => validateBranchName("feature123")).not.toThrow();
expect(() => validateBranchName("Branch1")).not.toThrow();
});
it("should accept names with hyphens", () => {
expect(() => validateBranchName("feature-branch")).not.toThrow();
expect(() => validateBranchName("fix-bug-123")).not.toThrow();
});
it("should accept names with underscores", () => {
expect(() => validateBranchName("feature_branch")).not.toThrow();
expect(() => validateBranchName("fix_bug_123")).not.toThrow();
});
it("should accept names with forward slashes", () => {
expect(() => validateBranchName("feature/new-thing")).not.toThrow();
expect(() => validateBranchName("user/feature/branch")).not.toThrow();
});
it("should accept names with periods", () => {
expect(() => validateBranchName("v1.0.0")).not.toThrow();
expect(() => validateBranchName("release.1.2.3")).not.toThrow();
});
it("should accept typical branch name formats", () => {
expect(() =>
validateBranchName("claude/issue-123-20250101-1234"),
).not.toThrow();
expect(() => validateBranchName("refs/heads/main")).not.toThrow();
expect(() => validateBranchName("bugfix/JIRA-1234")).not.toThrow();
});
});
describe("command injection attempts", () => {
it("should reject shell command substitution with $()", () => {
expect(() => validateBranchName("$(whoami)")).toThrow();
expect(() => validateBranchName("branch-$(rm -rf /)")).toThrow();
expect(() => validateBranchName("test$(cat /etc/passwd)")).toThrow();
});
it("should reject shell command substitution with backticks", () => {
expect(() => validateBranchName("`whoami`")).toThrow();
expect(() => validateBranchName("branch-`rm -rf /`")).toThrow();
});
it("should reject command chaining with semicolons", () => {
expect(() => validateBranchName("branch; rm -rf /")).toThrow();
expect(() => validateBranchName("test;whoami")).toThrow();
});
it("should reject command chaining with &&", () => {
expect(() => validateBranchName("branch && rm -rf /")).toThrow();
expect(() => validateBranchName("test&&whoami")).toThrow();
});
it("should reject command chaining with ||", () => {
expect(() => validateBranchName("branch || rm -rf /")).toThrow();
expect(() => validateBranchName("test||whoami")).toThrow();
});
it("should reject pipe characters", () => {
expect(() => validateBranchName("branch | cat")).toThrow();
expect(() => validateBranchName("test|grep password")).toThrow();
});
it("should reject redirection operators", () => {
expect(() => validateBranchName("branch > /etc/passwd")).toThrow();
expect(() => validateBranchName("branch < input")).toThrow();
expect(() => validateBranchName("branch >> file")).toThrow();
});
});
describe("option injection attempts", () => {
it("should reject branch names starting with dash", () => {
expect(() => validateBranchName("-x")).toThrow(
/cannot start with a dash/,
);
expect(() => validateBranchName("--help")).toThrow(
/cannot start with a dash/,
);
expect(() => validateBranchName("-")).toThrow(/cannot start with a dash/);
expect(() => validateBranchName("--version")).toThrow(
/cannot start with a dash/,
);
expect(() => validateBranchName("-rf")).toThrow(
/cannot start with a dash/,
);
});
});
describe("path traversal attempts", () => {
it("should reject double dot sequences", () => {
expect(() => validateBranchName("../../../etc")).toThrow();
expect(() => validateBranchName("branch/../secret")).toThrow(/'\.\.'$/);
expect(() => validateBranchName("a..b")).toThrow(/'\.\.'$/);
});
});
describe("git-specific invalid patterns", () => {
it("should reject @{ sequence", () => {
expect(() => validateBranchName("branch@{1}")).toThrow(/@{/);
expect(() => validateBranchName("HEAD@{yesterday}")).toThrow(/@{/);
});
it("should reject .lock suffix", () => {
expect(() => validateBranchName("branch.lock")).toThrow(/\.lock/);
expect(() => validateBranchName("feature.lock")).toThrow(/\.lock/);
});
it("should reject consecutive slashes", () => {
expect(() => validateBranchName("feature//branch")).toThrow(
/consecutive slashes/,
);
expect(() => validateBranchName("a//b//c")).toThrow(
/consecutive slashes/,
);
});
it("should reject trailing slashes", () => {
expect(() => validateBranchName("feature/")).toThrow(
/cannot end with a slash/,
);
expect(() => validateBranchName("branch/")).toThrow(
/cannot end with a slash/,
);
});
it("should reject leading periods", () => {
expect(() => validateBranchName(".hidden")).toThrow();
});
it("should reject trailing periods", () => {
expect(() => validateBranchName("branch.")).toThrow(
/cannot start or end with a period/,
);
});
it("should reject special git refspec characters", () => {
expect(() => validateBranchName("branch~1")).toThrow();
expect(() => validateBranchName("branch^2")).toThrow();
expect(() => validateBranchName("branch:ref")).toThrow();
expect(() => validateBranchName("branch?")).toThrow();
expect(() => validateBranchName("branch*")).toThrow();
expect(() => validateBranchName("branch[0]")).toThrow();
expect(() => validateBranchName("branch\\path")).toThrow();
});
});
describe("control characters and special characters", () => {
it("should reject null bytes", () => {
expect(() => validateBranchName("branch\x00name")).toThrow();
});
it("should reject other control characters", () => {
expect(() => validateBranchName("branch\x01name")).toThrow();
expect(() => validateBranchName("branch\x1Fname")).toThrow();
expect(() => validateBranchName("branch\x7Fname")).toThrow();
});
it("should reject spaces", () => {
expect(() => validateBranchName("branch name")).toThrow();
expect(() => validateBranchName("feature branch")).toThrow();
});
it("should reject newlines and tabs", () => {
expect(() => validateBranchName("branch\nname")).toThrow();
expect(() => validateBranchName("branch\tname")).toThrow();
});
});
describe("empty and whitespace", () => {
it("should reject empty strings", () => {
expect(() => validateBranchName("")).toThrow(/cannot be empty/);
});
it("should reject whitespace-only strings", () => {
expect(() => validateBranchName(" ")).toThrow();
expect(() => validateBranchName("\t\n")).toThrow();
});
});
describe("edge cases", () => {
it("should accept single alphanumeric character", () => {
expect(() => validateBranchName("a")).not.toThrow();
expect(() => validateBranchName("1")).not.toThrow();
});
it("should reject single special characters", () => {
expect(() => validateBranchName(".")).toThrow();
expect(() => validateBranchName("/")).toThrow();
expect(() => validateBranchName("-")).toThrow();
});
});
});