fix: parse multiple --allowed-tools values correctly

The parseAllowedTools() function was using regex matching which only
captured the first occurrence of --allowed-tools. This caused issues
when users specified multiple space-separated quoted tools like:

  --allowed-tools "Bash(git log:*)" "Bash(git diff:*)" "Bash(gh pr:*)"

The fix uses shell-quote library (same as parse-sdk-options.ts) to
properly tokenize arguments. This handles:
- Multiple space-separated quoted tools after a single --allowed-tools
- Multiple --allowedTools flags in the same string
- Mix of comma-separated and space-separated tools
- Glob patterns (which shell-quote returns as objects)

Fixes #746

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Ashwin Bhat <ashwin-ant@users.noreply.github.com>
This commit is contained in:
claude[bot]
2025-12-16 20:54:08 +00:00
parent f375cabfab
commit 2a7d38f775
2 changed files with 98 additions and 18 deletions

View File

@@ -1,22 +1,55 @@
import { parse as parseShellArgs, type ParseEntry } from "shell-quote";
/**
* Extract the string value from a shell-quote ParseEntry.
* Handles both plain strings and glob patterns (which are returned as objects).
*/
function entryToString(entry: ParseEntry): string | null {
if (typeof entry === "string") {
return entry;
}
// Handle glob patterns - shell-quote returns { op: "glob", pattern: "..." }
if (typeof entry === "object" && "op" in entry && entry.op === "glob") {
return (entry as { op: "glob"; pattern: string }).pattern;
}
return null;
}
export function parseAllowedTools(claudeArgs: string): string[] {
// Match --allowedTools or --allowed-tools followed by the value
// Handle both quoted and unquoted values
const patterns = [
/--(?:allowedTools|allowed-tools)\s+"([^"]+)"/, // Double quoted
/--(?:allowedTools|allowed-tools)\s+'([^']+)'/, // Single quoted
/--(?:allowedTools|allowed-tools)\s+([^\s]+)/, // Unquoted
];
if (!claudeArgs?.trim()) return [];
for (const pattern of patterns) {
const match = claudeArgs.match(pattern);
if (match && match[1]) {
// Don't return if the value starts with -- (another flag)
if (match[1].startsWith("--")) {
return [];
const result: string[] = [];
// Use shell-quote to properly tokenize the arguments
// This handles quoted strings, escaped characters, etc.
const rawArgs = parseShellArgs(claudeArgs);
for (let i = 0; i < rawArgs.length; i++) {
const entry = rawArgs[i];
if (!entry) continue;
const arg = entryToString(entry);
if (!arg) continue;
// Match both --allowedTools and --allowed-tools
if (arg === "--allowedTools" || arg === "--allowed-tools") {
// Collect all subsequent non-flag values as tools
while (i + 1 < rawArgs.length) {
const nextEntry = rawArgs[i + 1];
if (!nextEntry) break;
const toolArg = entryToString(nextEntry);
// Stop if we hit another flag or a non-parseable entry
if (!toolArg || toolArg.startsWith("--")) {
break;
}
// Split by comma in case tools are comma-separated within a single value
const tools = toolArg.split(",").map((t) => t.trim());
result.push(...tools.filter((t) => t.length > 0));
i++;
}
return match[1].split(",").map((t) => t.trim());
}
}
return [];
return result;
}

View File

@@ -37,8 +37,9 @@ describe("parseAllowedTools", () => {
test("handles duplicate --allowedTools flags", () => {
const args = "--allowedTools --allowedTools mcp__github__*";
// Should not match the first one since the value is another flag
expect(parseAllowedTools(args)).toEqual([]);
// Should skip the first one since the value is another flag
// and parse the second one correctly
expect(parseAllowedTools(args)).toEqual(["mcp__github__*"]);
});
test("handles typo --alloedTools", () => {
@@ -84,4 +85,50 @@ describe("parseAllowedTools", () => {
"mcp__github_comment__*",
]);
});
test("parses multiple space-separated quoted tools (issue #746)", () => {
// This is the exact format from the bug report
const args =
'--allowed-tools "Bash(git log:*)" "Bash(git diff:*)" "Bash(git fetch:*)" "Bash(gh pr:*)"';
expect(parseAllowedTools(args)).toEqual([
"Bash(git log:*)",
"Bash(git diff:*)",
"Bash(git fetch:*)",
"Bash(gh pr:*)",
]);
});
test("parses multiple --allowedTools flags with different tools", () => {
const args =
'--allowedTools "Edit,Read" --model "claude-3" --allowedTools "Bash(npm install)"';
expect(parseAllowedTools(args)).toEqual([
"Edit",
"Read",
"Bash(npm install)",
]);
});
test("parses mix of comma-separated and space-separated tools", () => {
const args =
'--allowed-tools "Bash(git log:*),Bash(git diff:*)" "Bash(git fetch:*)"';
expect(parseAllowedTools(args)).toEqual([
"Bash(git log:*)",
"Bash(git diff:*)",
"Bash(git fetch:*)",
]);
});
test("handles complex workflow example from issue #746", () => {
const args =
'--allowed-tools "Bash(git log:*)" "Bash(git diff:*)" "Bash(git fetch:*)" "Bash(git reflog:*)" "Bash(git merge-tree:*)" "Bash(gh pr:*)" "Bash(gh api:*)"';
expect(parseAllowedTools(args)).toEqual([
"Bash(git log:*)",
"Bash(git diff:*)",
"Bash(git fetch:*)",
"Bash(git reflog:*)",
"Bash(git merge-tree:*)",
"Bash(gh pr:*)",
"Bash(gh api:*)",
]);
});
});