Skip to content

fix: strip comment lines from claudeArgs before shell-quote parsing #995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
marcelsafin wants to merge 2 commits into from
Closed

fix: strip comment lines from claudeArgs before shell-quote parsing #995

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d1e6f5f
fix: strip comment lines from claudeArgs before shell-quote parsing
marcelsafin Mar 2, 2026
7ba3507
fix: add assertion to shell-quote comment test, address review feedback
marcelsafin Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion base-action/src/parse-sdk-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,17 @@ function parseClaudeArgsToExtraArgs(
if (!claudeArgs?.trim()) return {};

const result: Record<string, string | null> = {};
const args = parseShellArgs(claudeArgs).filter(

// Strip shell-style comment lines before parsing.
// shell-quote treats '#' as a comment character and silently swallows
// everything after it, including subsequent flags on new lines.
// See: https://github.com/anthropics/claude-code-action/issues/802
const sanitizedArgs = claudeArgs
.split("\n")
.filter((line) => !line.trim().startsWith("#"))
.join("\n");
Comment on lines +96 to +103

Copilot AI Mar 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line-based sanitization removes any line whose trimmed form starts with # regardless of whether that line occurs inside a quoted multi-line argument (e.g., a multi-line --system-prompt "..." that contains Markdown headings). That would change the value passed through to the CLI. Consider making comment stripping quote-aware, or restricting it to comment lines that are definitely outside of any open quotes.

Copilot uses AI. Check for mistakes.

const args = parseShellArgs(sanitizedArgs).filter(
(arg): arg is string => typeof arg === "string",
);

Expand Down
47 changes: 47 additions & 0 deletions base-action/test/parse-sdk-options.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,4 +367,51 @@ describe("parseSdkOptions", () => {
);
});
});

describe("comment handling (issue #802)", () => {
test("should ignore shell-style comment lines in claudeArgs", () => {
const options: ClaudeOptions = {
claudeArgs: [
"--allowed-tools 'mcp__github__create_comment'",
"# This is a comment",
"--max-turns 5",
].join("\n"),
};
const result = parseSdkOptions(options);

expect(result.sdkOptions.allowedTools).toContain(
"mcp__github__create_comment",
);
expect(result.sdkOptions.extraArgs?.["max-turns"]).toBe("5");
});

test("should handle multiple comment lines interspersed with flags", () => {
const options: ClaudeOptions = {
claudeArgs: [
"# Enable specific tools",
"--allowed-tools 'Bash,Read'",
"# Set max turns",
"--max-turns 5",
].join("\n"),
};
const result = parseSdkOptions(options);

expect(result.sdkOptions.allowedTools).toEqual(["Bash", "Read"]);
expect(result.sdkOptions.extraArgs?.["max-turns"]).toBe("5");
});

test("should not lose flags when comment is between them", () => {
const options: ClaudeOptions = {
claudeArgs: [
"--disallowed-tools 'WebSearch'",
"# middle comment",
"--max-turns 3",
].join("\n"),
};
const result = parseSdkOptions(options);

expect(result.sdkOptions.disallowedTools).toEqual(["WebSearch"]);
expect(result.sdkOptions.extraArgs?.["max-turns"]).toBe("3");
});
});
});
12 changes: 12 additions & 0 deletions base-action/test/parse-shell-args.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@ describe("shell-quote parseShellArgs", () => {
]);
});

test("should not swallow args after # comment lines", () => {
// Regression test for https://github.com/anthropics/claude-code-action/issues/802
// shell-quote treats '#' as a shell comment, dropping all subsequent content
const input = `--model 'claude-haiku-4-5'\n# This is a comment\n--allowed-tools 'mcp__github__create_comment'`;
const result = parseShellArgs(input).filter(
(arg) => typeof arg === "string",
);
// Without the fix, --allowed-tools would be swallowed by shell-quote
// This test documents the raw shell-quote behavior (it WILL fail with shell-quote alone)
// The actual fix is in parseClaudeArgsToExtraArgs which strips comment lines before parsing

Copilot AI Mar 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test has no assertions and it declares result but never uses it. With noUnusedLocals: true in tsconfig, this will fail TypeScript compilation (and the test currently passes regardless of behavior). Either remove this test, or add an expectation that documents shell-quote’s actual output (e.g., that it returns a comment token and drops subsequent args).

Suggested change
const result = parseShellArgs(input).filter(
(arg) => typeof arg === "string",
);
// Without the fix, --allowed-tools would be swallowed by shell-quote
// This test documents the raw shell-quote behavior (it WILL fail with shell-quote alone)
// The actual fix is in parseClaudeArgsToExtraArgs which strips comment lines before parsing
const result = parseShellArgs(input);
const filtered = result.filter((arg) => typeof arg === "string");
// Without the fix, --allowed-tools would be swallowed by shell-quote
// This test documents the raw shell-quote behavior (it WILL fail with shell-quote alone)
// The actual fix is in parseClaudeArgsToExtraArgs which strips comment lines before parsing
// Here we assert the raw shell-quote behavior: it drops args after the comment line.
expect(filtered).toEqual(["--model", "claude-haiku-4-5"]);

Copilot uses AI. Check for mistakes.
});

test("should filter out non-string results", () => {
// shell-quote can return objects for operators like | > < etc
const result = parseShellArgs("echo hello");
Expand Down
Loading