fix(parse-sdk-options): prevent shell-quote from collapsing unquoted Bash(X:*) rules to bare Bash (#1350)

* fix(parse-sdk-options): prevent shell-quote from collapsing unquoted Bash(X:*) rules to bare Bash

shell-quote's parse() tokenizes unquoted `(`, `)` as control operators
and barewords containing `*` as glob ops, all returned as non-string
objects. parseClaudeArgsToExtraArgs filtered those out, so an unquoted
`--allowedTools View,Bash(gh:*),Bash(cat:*)` collapsed to bare `Bash` —
silently widening scoped permission rules to unrestricted Bash(*).

Escape shell control metachars to Unicode private-use placeholders
before parse() and restore after; extract .pattern from glob ops.
Preserves existing quote/whitespace handling.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* ci: retrigger

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
alexglynn 2026-06-13 22:45:14 -07:00 committed by GitHub
parent d5726de019
commit a5e5d3b82e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 146 additions and 3 deletions

View File

@ -24,6 +24,35 @@ const ACCUMULATING_FLAGS = new Set([
// Delimiter used to join accumulated flag values // Delimiter used to join accumulated flag values
const ACCUMULATE_DELIMITER = "\x00"; const ACCUMULATE_DELIMITER = "\x00";
// shell-quote treats ()|&;<> as control operators and splits adjacent text
// around them into separate tokens (returned as `{op}` objects, which we then
// dropped). For CLI args these must be literal characters — e.g. unquoted
// `--allowedTools Bash(gh:*)` was being mangled into bare `Bash`, silently
// widening a scoped permission rule to Bash(*). We escape each metachar to a
// Unicode private-use codepoint before parsing and restore it afterward,
// keeping shell-quote's quote/whitespace handling intact.
const SHELL_META_PAIRS: [string, string][] = [
["(", ""],
[")", ""],
["|", ""],
["&", ""],
[";", ""],
["<", ""],
[">", ""],
];
const SHELL_META_ESCAPE = new Map(SHELL_META_PAIRS);
const SHELL_META_UNESCAPE = new Map(SHELL_META_PAIRS.map(([k, v]) => [v, k]));
const SHELL_META_ESCAPE_RE = /[()|&;<>]/g;
const SHELL_META_UNESCAPE_RE = /[-]/g;
function escapeShellMeta(s: string): string {
return s.replace(SHELL_META_ESCAPE_RE, (c) => SHELL_META_ESCAPE.get(c)!);
}
function unescapeShellMeta(s: string): string {
return s.replace(SHELL_META_UNESCAPE_RE, (c) => SHELL_META_UNESCAPE.get(c)!);
}
type McpConfig = { type McpConfig = {
mcpServers?: Record<string, unknown>; mcpServers?: Record<string, unknown>;
}; };
@ -106,9 +135,19 @@ function parseClaudeArgsToExtraArgs(
if (!claudeArgs?.trim()) return {}; if (!claudeArgs?.trim()) return {};
const result: Record<string, string | null> = {}; const result: Record<string, string | null> = {};
const args = parseShellArgs(stripShellComments(claudeArgs)).filter( const args = parseShellArgs(escapeShellMeta(stripShellComments(claudeArgs)))
(arg): arg is string => typeof arg === "string", .map((arg) => {
); if (typeof arg === "string") return unescapeShellMeta(arg);
// With control metachars escaped above, the only non-string shell-quote
// can still emit is a glob op (bareword containing *, ?, or [). Its
// `pattern` field is the verbatim token text — use it as-is so values
// like `Bash(cmd:*)` and `Read(path/**)` round-trip intact.
if (typeof arg === "object" && arg !== null && "pattern" in arg) {
return unescapeShellMeta((arg as { pattern: string }).pattern);
}
return undefined;
})
.filter((arg): arg is string => typeof arg === "string");
for (let i = 0; i < args.length; i++) { for (let i = 0; i < args.length; i++) {
const arg = args[i]; const arg = args[i];

View File

@ -137,6 +137,110 @@ describe("parseSdkOptions", () => {
]); ]);
}); });
test("should preserve unquoted Bash(cmd:*) rules instead of collapsing to bare Bash", () => {
// Regression: shell-quote tokenizes unquoted `(`/`)` as control ops and
// `*` as a glob, which were filtered out — collapsing scoped rules like
// `Bash(gh:*)` into bare `Bash` (= Bash(*), unrestricted shell).
const options: ClaudeOptions = {
claudeArgs: "--allowedTools View,Bash(gh:*),Bash(cat:*)",
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual([
"View",
"Bash(gh:*)",
"Bash(cat:*)",
]);
expect(result.sdkOptions.allowedTools).not.toContain("Bash");
});
test("should preserve unquoted space-separated Bash(cmd:*) rules", () => {
const options: ClaudeOptions = {
claudeArgs: "--allowed-tools Bash(gh:*) Bash(cat:*) Read(//tmp/**)",
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual([
"Bash(gh:*)",
"Bash(cat:*)",
"Read(//tmp/**)",
]);
expect(result.sdkOptions.allowedTools).not.toContain("Bash");
});
test("should preserve unquoted Tool(content) rules without glob chars", () => {
const options: ClaudeOptions = {
claudeArgs:
"--allowedTools Read(~/file),WebFetch(domain:example.com),Edit",
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual([
"Read(~/file)",
"WebFetch(domain:example.com)",
"Edit",
]);
});
test("should still preserve quoted Bash(cmd:*) rules (no regression)", () => {
const options: ClaudeOptions = {
claudeArgs: '--allowedTools "Bash(gh:*),Bash(cat:*)"',
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual([
"Bash(gh:*)",
"Bash(cat:*)",
]);
});
test("should merge quoted tag-mode tools with unquoted user tools without widening", () => {
// Real-world shape: the action's tag mode wraps its own --allowedTools in
// double quotes, then appends the user's claude_args (typically unquoted
// in workflow YAML). Both halves must round-trip.
const options: ClaudeOptions = {
claudeArgs:
'--permission-mode acceptEdits --allowedTools "Glob,Grep,Read,Bash(git add:*),Bash(git commit:*)" ' +
"--model claude-opus-4-7\n" +
"--allowedTools View,Bash(gh:*),Bash(printf:*),Bash(cat:*)",
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.allowedTools).toEqual([
"Glob",
"Grep",
"Read",
"Bash(git add:*)",
"Bash(git commit:*)",
"View",
"Bash(gh:*)",
"Bash(printf:*)",
"Bash(cat:*)",
]);
expect(result.sdkOptions.allowedTools).not.toContain("Bash");
});
test("should preserve unquoted disallowedTools rules without widening", () => {
// Same bug class on the deny side: a scoped deny collapsing to bare
// `Bash` would block all shell instead of the intended prefix.
const options: ClaudeOptions = {
claudeArgs: "--disallowedTools Bash(rm:*),Bash(sudo:*)",
};
const result = parseSdkOptions(options);
expect(result.sdkOptions.disallowedTools).toEqual([
"Bash(rm:*)",
"Bash(sudo:*)",
]);
expect(result.sdkOptions.disallowedTools).not.toContain("Bash");
});
test("should handle mixed camelCase and hyphenated allowedTools flags", () => { test("should handle mixed camelCase and hyphenated allowedTools flags", () => {
const options: ClaudeOptions = { const options: ClaudeOptions = {
claudeArgs: '--allowedTools "Edit,Read" --allowed-tools "Write,Glob"', claudeArgs: '--allowedTools "Edit,Read" --allowed-tools "Write,Glob"',