fix(lp1): map scoped allowed-tools grants such as Bash(git:*) by tool name

[Zhuoxi2000]

Fixes #766

What

• _map_allowed_tools_to_categories maps a scoped grant in the Tool(specifier) form by its tool name. Bash(git:*) maps to shell, WebFetch(domain:example.com) to network, Read(./docs/**) to file_read and Edit(src/**) to file_write.
• Only complete grants map. A grant is a bare tool name, or Tool(specifier) where the specifier is everything between the first ( and the final ), that ) ends the entry, and the specifier is not blank. As in Claude Code's permission rule syntax, parentheses inside the specifier are literal, so Bash(echo '('), Bash(echo ')') and Edit(./notes (old)/**) are complete grants. Incomplete or blank entries such as Bash(, Bash(notes, Bash(echo '(' or Bash() map to no category. They still count as an allowed-tools declaration for LP3, as before. The existing 64-character bound on the tool name still applies.
• String allowed-tools values split only on separators outside a specifier, so a scoped grant is kept whole. A ( opens a specifier, and the first ) followed by a comma, whitespace or the end of the value closes it. Any ( or ) in between is literal. "Bash(git status:*) Read" and "Bash(echo '(') Read" are two grants each, and "Read(./docs Bash(notes).md)" and "Read(./docs, Bash(notes).md)" are one Read grant each. The comma form is chosen only when a comma appears outside a specifier, so "Bash(git status:*) Read(./a, b)" is two grants. An unclosed ( keeps the rest of the string in its entry, which the validator then rejects.
• build_context and the LP analyzer now share that splitter through a new module, skillspector/allowed_tools.py, which also holds the validator the analyzer applies before mapping. The tool-name alphabet matches the settings permission-rule check in bundled_execution_surface.py. Unlike that check, this one does not require the specifier's parentheses to balance. The splitter is lazy, so a comma-dense declaration still stops at the analyzer's existing declaration limit without building a large list.

Known limits:

• In a string value, a specifier that itself contains ) followed by a space or comma cannot be kept whole. "Bash(echo ') x') Read" splits into Bash(echo '), x') and Read. This ) boundary is what separates grants in the skills docs example Bash(git add *) Bash(git commit *). In the YAML list form this analyzer keeps such an entry whole.
• Bash() (empty specifier) maps to no category. The docs do not define an empty specifier, so this errs toward raising LP1.
• A ) followed by anything other than a separator does not close the specifier, so a malformed entry such as Read(./docs)/** keeps the rest of the value, including a bare Bash after it. On main that value split at the ) and the Bash was recognised.

Why

The lookup used to match the whole entry, so a scoped grant in the Agent Skills spec's own Tool(specifier) form mapped to no category. A skill that declared allowed-tools: Bash(git:*) Read and ran git status through subprocess got LP1 HIGH for shell. The same skill with bare Bash got no finding. The narrower grant was penalised and the broader one was not. Details and the repro are in the linked issue.

Mapping by tool name alone is not enough while the string forms split on every comma or whitespace. That split cut Read(./docs Bash(notes).md) into Read(./docs and Bash(notes).md), and the second fragment would then have counted as a Bash grant even though only Read was declared. Splitting outside a specifier and validating each grant before mapping keeps specifier text from being read as another tool. Treating parentheses inside the specifier as literal keeps grants such as Bash(echo '(') from being rejected.

This is separate from #685, which changed how the LP1/LP3 explanation and remediation text frame allowed-tools as preapproval. That change did not touch the tool-name lookup, and this one leaves its wording as it is.

Tests

In tests/test_mcp_least_privilege.py::TestLP3AllowedTools:

• test_allowed_tools_scoped_bash_covers_shell_no_lp1, parametrized over the list form ["Bash(git:*)"], the space form "Bash(git:*) Bash(jq:*) Read", a space form with a space inside the specifier "Bash(git status:*) Read", the comma form "Bash(git status:*), Read", and the list entry ["Bash(notes).md)"], whose specifier is notes).md
• test_allowed_tools_bash_scope_with_literal_parens_covers_shell_no_lp1, which scans subprocess.run(["echo", "("], check=True) with ["Bash(echo '(')"], ["Bash(echo ')')"], "Bash(echo '(') Read", "Bash(echo ')') Read", "Bash(echo '('), Read" and "Bash(echo ')'), Read", and requires no LP1 for shell
• test_allowed_tools_incomplete_bash_scope_with_literal_parens_still_lp1, which scans the same code with ["Bash(echo '('"], ["Bash(echo ')'"], "Bash(echo '( Read" and "Bash(echo '(', Read", and requires LP1 for shell
• test_allowed_tools_scoped_non_shell_tool_still_lp1, a negative control: Read(./docs/**) does not cover shell, so LP1 is still raised
• test_map_allowed_tools_uses_tool_name_before_specifier, which checks the category mapping directly for Bash, WebFetch, Read and Edit
• test_allowed_tools_malformed_or_nested_bash_still_lp1, which requires LP1 for shell with "Read(./docs Bash(notes).md)", "Read(./docs, Bash(notes).md)", the list ["Read(./docs Bash(notes).md)"], ["Bash("], ["Bash(notes"], "Bash(notes Read" and ["Bash()"]
• test_normalize_allowed_tools_keeps_scoped_grants_whole, which checks the entries produced from the string forms, including literal ( and ) in a specifier and the ) boundary above
• test_map_allowed_tools_ignores_malformed_grants, which checks that incomplete or blank entries map to nothing and that a specifier may contain parentheses
• test_map_allowed_tools_treats_scope_parens_as_literal, which checks that Bash(echo '('), Bash(echo ')'), Bash(notes).md) and Bash(a)(b) each map to shell

In tests/nodes/test_build_context.py:

• test_build_context_keeps_scoped_allowed_tools_whole, which checks the entries build_context produces from the same string forms in SKILL.md frontmatter, including literal parentheses in a specifier

All runs below use Python 3.12 with the dev dependencies from uv.lock.

Before, on main with only the first three tests applied: 5 failed, 20 passed in tests/test_mcp_least_privilege.py. On the first commit of this PR (4666a72), 17 of the 21 cases added by the second commit fail. On the second commit (2ca88a3), 15 of the 25 cases added or moved by the third commit (318b972) fail. The ones that already pass at each step are guards or splitter cases whose result did not change.

After:

$ pytest -q tests/test_mcp_least_privilege.py
61 passed

$ pytest -q -m "not integration and not provider" tests/test_mcp_least_privilege.py tests/nodes/test_build_context.py tests/nodes/analyzers/test_registry.py
173 passed

$ make test-unit
10010 passed, 14 skipped, 135 deselected, 4 xfailed

End to end, skillspector scan --no-llm runs on a skill whose scripts/run.py calls subprocess.run(['git', 'status']), or subprocess.run(["echo", "("], check=True) for the echo rows. The results for this PR are the same from source and from a freshly built wheel.

allowed-tools	main (b6f0304)	This PR
Bash Read	9, LOW, SAFE (AST4 only)	9, LOW, SAFE (AST4 only)
Bash(git:*) Bash(python3:*) Read	33, MEDIUM, CAUTION, with HIGH: LP1 - Code capability 'shell' detected in scripts/run.py ...	9, LOW, SAFE (AST4 only)
[Bash(git:*)]	33, MEDIUM, CAUTION, LP1 HIGH for shell	9, LOW, SAFE (AST4 only)
Bash(git status:*), Read	33, MEDIUM, CAUTION, LP1 HIGH for shell	9, LOW, SAFE (AST4 only)
Bash(git status:*) Read	33, MEDIUM, CAUTION, LP1 HIGH for shell	9, LOW, SAFE (AST4 only)
["Bash(echo '(')"], ["Bash(echo ')')"]	33, MEDIUM, CAUTION, LP1 HIGH for shell	9, LOW, SAFE (AST4 only)
Bash(echo '(') Read, Bash(echo ')') Read, Bash(echo '('), Read, Bash(echo ')'), Read	33, MEDIUM, CAUTION, LP1 HIGH for shell	9, LOW, SAFE (AST4 only)
["Bash(notes).md)"], ["Bash(a)(b)"]	33, MEDIUM, CAUTION, LP1 HIGH for shell	9, LOW, SAFE (AST4 only); complete Bash grants with specifiers notes).md and a)(b
Read(./docs/**)	33, MEDIUM, CAUTION, LP1 HIGH for shell	unchanged
Read(./docs Bash(notes).md)	33, MEDIUM, CAUTION, LP1 HIGH for shell	unchanged
Read(./docs, Bash(notes).md)	33, MEDIUM, CAUTION, LP1 HIGH for shell	unchanged
["Read(./docs Bash(notes).md)"]	33, MEDIUM, CAUTION, LP1 HIGH for shell	unchanged
["Bash("], ["Bash(notes"], Bash(notes Read, ["Bash()"], ["Bash(echo '('"]	33, MEDIUM, CAUTION, LP1 HIGH for shell	unchanged

Lint: make lint and make format-check pass (ruff 0.15.19 from uv.lock). Ruff v0.15.2, as pinned in .pre-commit-config.yaml, also passes check and format --check on the changed files. mypy 2.1.0 reports no issues in allowed_tools.py or mcp_least_privilege.py. On build_context.py it only reports the missing PyYAML stubs, which main reports too.

[yashrajp22]

The scoped-Bash fix passes the selected tests, but parsing declaration fragments introduces an LP1 regression. The inline comment includes reproduced cases from both source and fresh wheel scans.

[yashrajp22]

Could we preserve and validate each complete scoped grant before looking up its tool name? For allowed-tools: "Read(./docs Bash(notes).md)" (also the comma variant), existing normalization splits inside the path. This new lookup then treats Bash(notes).md) as a Bash declaration and suppresses LP1 for subprocess.run(...), although only Read was declared. I reproduced LP1 on BASE and no LP1 on this head in both source and freshly built wheel scans; the equivalent YAML list correctly retains LP1.

allowed-tools: ["Bash("] also suppresses LP1 on this head. Please keep scope contents intact and reject incomplete declarations before category mapping, with regression coverage for these cases while preserving the new valid scoped-Bash behavior.

[Zhuoxi2000]

Thanks @yashrajp22, good catch. The string splitter cut Read(./docs Bash(notes).md) apart before the new lookup saw it, and Bash( was mapped on its prefix alone. Fixed in 2ca88a3:

• String allowed-tools values now split only on separators outside parentheses. build_context and the LP analyzer share one helper (skillspector/allowed_tools.py). The comma form is chosen only when a comma appears outside parentheses, so Read(./docs, Bash(notes).md) stays one grant and Bash(git status:*) Read(./a, b) is two. An unclosed ( keeps the rest of the string in its entry.
• Each entry is validated before category mapping. It must be a bare tool name, or Tool(specifier) with a non-blank, balanced specifier whose closing ) ends the entry. Bash(, Bash(notes, Bash(), Bash(notes).md) and Bash(a)(b) map to no category. They still count as an allowed-tools declaration for LP3, as before, but cover nothing.
• Valid scoped grants behave as in 4666a72: Bash(git:*) maps to shell, WebFetch(domain:...) to network, and so on.
• Only parentheses nest; quotes and backslashes are not treated as escapes, the same as _permission_rule_is_valid in bundled_execution_surface.py. So Read("x) Bash(y") still reads as two grants. If the host does treat quotes or \) as escaping inside a specifier, I can make the splitter follow that too.

New tests (21 cases):

• TestLP3AllowedTools::test_allowed_tools_malformed_or_nested_bash_still_lp1 covers your space and comma strings, the YAML list equivalent, ["Bash("], ["Bash(notes"], "Bash(notes Read", ["Bash()"] and ["Bash(notes).md)"]. Each must raise LP1 for shell.
• TestLP3AllowedTools::test_normalize_allowed_tools_keeps_scoped_grants_whole and tests/nodes/test_build_context.py::test_build_context_keeps_scoped_allowed_tools_whole pin the entries both parsers produce.
• TestLP3AllowedTools::test_map_allowed_tools_ignores_malformed_grants checks the mapping directly, including a scope that itself contains balanced parentheses.

On 4666a72, 17 of the 21 fail. The 4 that already pass there are guards: the YAML list case, the plain comma string in both parsers, and a stray ). On 2ca88a3 all pass, and the existing scoped-Bash cases still pass unchanged.

$ pytest -q tests/test_mcp_least_privilege.py
41 passed

$ make test-unit
9986 passed, 14 skipped, 135 deselected, 4 xfailed

make lint and make format-check (ruff 0.15.19) pass, and so does ruff 0.15.2 as pinned in .pre-commit-config.yaml. mypy reports no issues in allowed_tools.py or mcp_least_privilege.py. On build_context.py it only reports the missing PyYAML stubs, which main reports too.

skillspector scan --no-llm on a skill whose script calls subprocess.run(['git', 'status']), scanned from source. For 2ca88a3 I also scanned with a freshly built wheel, and the results match.

allowed-tools	main (b6f0304)	4666a72	2ca88a3
Read(./docs Bash(notes).md)	LP1 HIGH, 33	no LP1, 9	LP1 HIGH, 33
Read(./docs, Bash(notes).md)	LP1 HIGH, 33	no LP1, 9	LP1 HIGH, 33
["Read(./docs Bash(notes).md)"]	LP1 HIGH, 33	LP1 HIGH, 33	LP1 HIGH, 33
["Bash("]	LP1 HIGH, 33	no LP1, 9	LP1 HIGH, 33
Bash(git:*) Bash(python3:*) Read, [Bash(git:*)], Bash(git status:*), Read, Bash(git status:*) Read	LP1 HIGH, 33	no LP1, 9	no LP1, 9
Read(./docs/**)	LP1 HIGH, 33	LP1 HIGH, 33	LP1 HIGH, 33

I also updated the PR description. It said the space form still splits Bash(git status:*) into fragments and that no build_context change was needed. That was wrong: that split is what let Bash(notes).md) through.

[yashrajp22]

The previous scoped-entry splitting and incomplete-grant finding is fixed. Fresh source and wheel scans still reproduce the literal-parenthesis case described inline. All 59 selected HEAD tests passed.

[yashrajp22]

Could we allow literal parentheses inside an otherwise complete scope here? Claude Code's permission syntax treats parentheses within the specifier as literal characters. For example, allowed-tools: ["Bash(echo '(')"] is complete, but this balance check returns no tool. Scanning a fixture containing subprocess.run(["echo", "("], check=True) produces shell LP1 in both source and wheel modes; the closing-parenthesis variant and string form reproduce it too. This leaves a false positive for valid scoped Bash declarations that this PR intends to support. Please preserve rejection of incomplete outer grants while accepting literal specifier content, with coverage for these cases.

[Zhuoxi2000]

Thanks @yashrajp22. You're right: the permission rule syntax says "Parentheses inside the specifier are literal", and the balance check rejected complete grants such as Bash(echo '('). Fixed in 318b972, on top of 2ca88a3:

• Validation now follows the documented Tool(specifier) form. The specifier is everything between the first ( and the final ), and that ) must end the entry. Parentheses inside it are not counted, so the only other check is that the specifier is not blank. Bash(echo '(') and Bash(echo ')') map to shell. Incomplete outer grants still map to nothing: Bash(, Bash(notes, Bash(echo '(', Bash(echo ')', and unclosed string forms such as Bash(echo '( Read.
• String values: a ( opens a specifier, and the first ) followed by a comma, whitespace or the end of the value closes it. Any ( or ) in between is literal. So Bash(echo '(') Read, Bash(echo ')') Read and both comma forms are two grants each. The cases from your first review are unchanged: Read(./docs Bash(notes).md) and Read(./docs, Bash(notes).md) stay one Read grant. Bash(git status:*) Read(./a, b) is still two grants. The comma-form check uses the same scan, and entries are still produced lazily.

Two cases that 2ca88a3 pinned as malformed are complete grants under the literal reading. I moved both tests explicitly:

• ["Bash(notes).md)"] as a list entry is Bash with the specifier notes).md. It moved from test_allowed_tools_malformed_or_nested_bash_still_lp1 to test_allowed_tools_scoped_bash_covers_shell_no_lp1 (id list_literal_close_paren_inside_scope). The case from your first review was different: there the fragment Bash(notes).md) came from splitting inside Read(...). That split still doesn't happen, and Read(./docs Bash(notes).md) in the string, comma and list forms still raises LP1.
• Bash(a)(b) is Bash with the specifier a)(b. It moved out of the malformed list in test_map_allowed_tools_ignores_malformed_grants and into the new test_map_allowed_tools_treats_scope_parens_as_literal.

Open points:

• Bash(): the docs don't define an empty specifier, so I kept it as covering nothing, as before. That errs toward raising LP1 rather than hiding it. If it should mean the bare tool, I can map it to shell.
• In a string value, a specifier that itself contains ) followed by a space or comma can't be kept whole. For example, "Bash(echo ') x') Read" splits into Bash(echo '), x') and Read. The ) boundary is what separates Bash(git add *) Bash(git commit *) in the skills docs example, so I kept it. In the list form this analyzer keeps such an entry whole.
• A ) followed by anything other than a separator does not close the specifier, so a malformed value such as Read(./docs)/** Bash now keeps Bash inside the first entry, where main split at the ) and recognised it. I noted it in the PR description's known limits.
• In my previous reply I said this validator matched _permission_rule_is_valid in bundled_execution_surface.py. That settings-rule check still requires balanced parentheses, so the two now differ. I left it alone because it belongs to a different analyzer. I can open a follow-up if you'd like it aligned.

Tests (25 new or moved cases):

• TestLP3AllowedTools::test_allowed_tools_bash_scope_with_literal_parens_covers_shell_no_lp1 uses your fixture, subprocess.run(["echo", "("], check=True), with ["Bash(echo '(')"], ["Bash(echo ')')"], "Bash(echo '(') Read", "Bash(echo ')') Read", "Bash(echo '('), Read" and "Bash(echo ')'), Read". Each must give no LP1 for shell.
• TestLP3AllowedTools::test_allowed_tools_incomplete_bash_scope_with_literal_parens_still_lp1 uses the same fixture with ["Bash(echo '('"], ["Bash(echo ')'"], "Bash(echo '( Read" and "Bash(echo '(', Read". Each must raise LP1 for shell.
• TestLP3AllowedTools::test_normalize_allowed_tools_keeps_scoped_grants_whole and tests/nodes/test_build_context.py::test_build_context_keeps_scoped_allowed_tools_whole add the string and comma forms, plus "Bash(echo ')') Read(./a, b)" and the ) boundary case above.
• TestLP3AllowedTools::test_map_allowed_tools_treats_scope_parens_as_literal covers Bash(echo '('), Bash(echo ')'), Bash(notes).md) and Bash(a)(b). The malformed list gains Bash(echo '(' and Bash(echo ')'.

On 2ca88a3, 15 of the 25 fail, including the moved Bash(notes).md) case. The other 10 already pass there. Four of them are the incomplete-grant guards. Six are splitter cases where the old depth count already split in the same place: the ')' string forms in both parsers, the Read(./a, b) case and the ) boundary. On 318b972 all pass.

$ pytest -q tests/test_mcp_least_privilege.py
61 passed

$ pytest -q -m "not integration and not provider" tests/test_mcp_least_privilege.py tests/nodes/test_build_context.py tests/nodes/analyzers/test_registry.py
173 passed

$ make test-unit
10010 passed, 14 skipped, 135 deselected, 4 xfailed

make lint and make format-check (ruff 0.15.19) pass, and ruff 0.15.2 as pinned in .pre-commit-config.yaml passes check and format --check on the changed files. mypy reports no issues in allowed_tools.py or mcp_least_privilege.py. The branch still merges into current main without conflicts, and the same three test files pass on the merge result.

skillspector scan --no-llm results, as score, severity and recommendation. Each scan used both source and a freshly built wheel, and the two agreed at both commits. The first block uses your fixture. The second uses the earlier fixture, subprocess.run(['git', 'status']).

allowed-tools	2ca88a3	318b972
["Bash(echo '(')"]	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE
["Bash(echo ')')"]	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE
Bash(echo '(') Read	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE
Bash(echo ')') Read	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE
Bash(echo '('), Read	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE
Bash(echo ')'), Read	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE
["Bash(echo '('"]	LP1 HIGH shell, 33 MEDIUM CAUTION	unchanged
Bash Read (control)	no LP1, 9 LOW SAFE	unchanged
Read (control)	LP1 HIGH shell, 33 MEDIUM CAUTION	unchanged

allowed-tools	2ca88a3	318b972
Read(./docs Bash(notes).md), Read(./docs, Bash(notes).md), ["Read(./docs Bash(notes).md)"]	LP1 HIGH shell, 33 MEDIUM CAUTION	unchanged
["Bash("], ["Bash(notes"], Bash(notes Read, ["Bash()"]	LP1 HIGH shell, 33 MEDIUM CAUTION	unchanged
Read(./docs/**)	LP1 HIGH shell, 33 MEDIUM CAUTION	unchanged
Bash Read, Bash(git:*) Bash(python3:*) Read, [Bash(git:*)], Bash(git status:*), Read, Bash(git status:*) Read	no LP1, 9 LOW SAFE	unchanged
["Bash(notes).md)"], ["Bash(a)(b)"]	LP1 HIGH shell, 33 MEDIUM CAUTION	no LP1, 9 LOW SAFE (complete grants, see above)

I also updated the PR description, because it still listed Bash(notes).md) and Bash(a)(b) as malformed.