fix: respect --allowedTools when building disallowed tools list

[MaxwellCalkin]

Summary

Fixes #690

• In agent mode, WebSearch and WebFetch were not being explicitly disabled via --disallowedTools, inconsistent with tag mode's security behavior
• When users specified --allowedTools WebSearch in claude_args, it had no effect on the disallowed tools list because buildDisallowedToolsString() was never called in agent mode
• This PR calls buildDisallowedToolsString([], allowedTools) in prepareAgentMode() so that WebSearch/WebFetch are disabled by default, but respected when the user explicitly allows them

Changes

• src/modes/agent/index.ts: Import and call buildDisallowedToolsString() with the parsed allowedTools from claude_args, adding --disallowedTools to claudeArgs when tools need to be disabled
• test/modes/agent.test.ts: Updated existing test and added two new tests:
  • --allowedTools WebSearch removes only WebSearch from the disallowed list (WebFetch remains)
  • --allowedTools WebSearch,WebFetch removes both, resulting in no --disallowedTools flag

Test plan

• Existing agent mode tests updated and passing
• New test: --allowedTools WebSearch removes WebSearch from disallowed list
• New test: --allowedTools WebSearch,WebFetch removes both from disallowed list
• All 62 tests across agent, parse-tools, and create-prompt test files pass
• Manual verification: deploy with claude_args: '--allowedTools "WebSearch"' and confirm WebSearch is available

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

[AntonAM]

It would be great to move this PR forward.

[km-anthropic]

@claude review

[claude]

🔴 parseAllowedTools only captures the first value when users use space-separated syntax (--allowedTools "WebSearch" "WebFetch"), but the downstream parseClaudeArgsToExtraArgs correctly captures all values via its accumulating flag logic. This causes the missed tools to appear in both allowedTools and disallowedTools simultaneously, since buildDisallowedToolsString uses the incomplete parse to decide what to disallow. The comma-separated and repeated-flag syntaxes work correctly; only the space-separated multi-value format triggers this conflict.

Extended reasoning...

Bug Analysis

parseAllowedTools in parse-tools.ts uses regex patterns that each require the --allowedTools (or --allowed-tools) prefix immediately before each value. For the input --allowedTools "WebSearch" "WebFetch", the double-quoted regex matches only --allowedTools "WebSearch" because "WebFetch" stands alone without a preceding --allowedTools flag. The same issue affects unquoted space-separated values.

Downstream Conflict

The downstream parser parseClaudeArgsToExtraArgs in base-action/src/parse-sdk-options.ts has explicit support for this syntax. Lines 107-114 show that for ACCUMULATING_FLAGS (which includes allowedTools and disallowedTools), it consumes all consecutive non-flag values after the flag. So for the same input, it correctly captures both WebSearch and WebFetch.

Step-by-Step Proof

1. User sets CLAUDE_ARGS='--allowedTools "WebSearch" "WebFetch"'
2. parseAllowedTools returns ["WebSearch"] (misses WebFetch)
3. buildDisallowedToolsString([], ["WebSearch"]) returns "WebFetch" (WebFetch stays disallowed since it was not detected as allowed)
4. claudeArgs becomes: --disallowedTools "WebFetch" --allowedTools "WebSearch" "WebFetch"
5. parseClaudeArgsToExtraArgs correctly parses the accumulating flags and produces disallowedTools: ["WebFetch"] AND allowedTools: ["WebSearch", "WebFetch"]
6. At lines 241-244 of parse-sdk-options.ts, both mergedAllowedTools and mergedDisallowedTools are returned as-is with no conflict resolution, so WebFetch ends up in both arrays simultaneously

Impact

The behavior when a tool is in both allowedTools and disallowedTools is undefined and depends on the SDK implementation. The user explicitly asked for WebFetch to be allowed, but the system silently adds it to the disallowed list too, potentially overriding user intent. Before this PR, agent mode had no --disallowedTools at all, so this parsing discrepancy was harmless.

Fix

parseAllowedTools should be updated to also consume consecutive non-flag quoted or unquoted values after --allowedTools, matching the accumulating behavior of parseClaudeArgsToExtraArgs. Alternatively, the function could use the same shell-quote + iteration approach that parseClaudeArgsToExtraArgs uses. The PR tests only cover comma-separated and single-value syntaxes, so a test for space-separated multi-value should be added as well.

[claude]

🟡 The regex /--disallowedTools[^"]*WebSearch/ on line 253 is vacuous — [^"]* stops at the opening quote in --disallowedTools "WebFetch" and can never reach the tool name inside, so not.toMatch(...) always passes regardless of whether WebSearch was actually removed. Fix: use /--disallowedTools\s+"[^"]*WebSearch/ to match inside the quoted value.

Extended reasoning...

What the bug is

The test "--allowedTools WebSearch removes WebSearch from disallowed list" uses a regex assertion to verify that WebSearch is not present in the --disallowedTools value:

expect(result.claudeArgs).not.toMatch(/--disallowedTools[^"]*WebSearch/);

The regex /--disallowedTools[^"]*WebSearch/ matches the literal --disallowedTools, then [^"]* matches zero or more non-quote characters, then looks for WebSearch. However, the production code on line 127 of src/modes/agent/index.ts constructs the string as:

claudeArgs = `${claudeArgs} --disallowedTools "${disallowedTools}"`;

So the actual string looks like --disallowedTools "WebFetch". The [^"]* character class matches only the space between --disallowedTools and the opening ", then stops because " is excluded from the character class. It can never reach inside the quoted value.

Step-by-step proof

Consider the scenario where the code is broken and WebSearch is incorrectly included, producing: --disallowedTools "WebSearch,WebFetch".

1. The regex engine matches --disallowedTools at the start.
2. [^"]* starts matching — it matches the space (not a quote), then encounters " — stops.
3. The engine now expects WebSearch but finds " — no match.
4. not.toMatch(...) passes because the regex didn't match.

The test passes even though the implementation is wrong. The assertion provides zero verification of the test's stated purpose.

Impact

The test cannot catch regressions where buildDisallowedToolsString fails to remove WebSearch from the disallowed list. The other assertions in the test (toContain("--disallowedTools") and toContain("WebFetch")) provide partial coverage of the output format but do not verify the core behavior being tested — that WebSearch was excluded from the disallowed tools value.

Fix

Replace the regex with one that looks inside the quoted value:

expect(result.claudeArgs).not.toMatch(/--disallowedTools\s+"[^"]*WebSearch/);

This matches --disallowedTools, then whitespace, then the opening quote, then scans inside the quoted value for WebSearch.