fix: parse space-separated allowed-tools strings (fixes #327) - #330
Open
Bimpebabs wants to merge 1 commit into
Open
fix: parse space-separated allowed-tools strings (fixes #327)#330Bimpebabs wants to merge 1 commit into
Bimpebabs wants to merge 1 commit into
Conversation
rng1995
requested changes
Aug 5, 2026
rng1995
left a comment
rng1995
left a comment
Collaborator
There was a problem hiding this comment.
[Automated SkillSpector Review]
Requesting changes. The space-separated allowed-tools behavior is correct and its focused tests pass (11 tests), but the exact patch fails the repository format gate: ruff format --check reports that src/skillspector/nodes/build_context.py would be reformatted. Please run Ruff format on that file and update the PR.
rng1995
reviewed
Aug 5, 2026
| [str(p) for p in permissions] if isinstance(permissions, list) else [] | ||
| ) | ||
| # `allowed-tools` (Agent Skills standard) — accept list or comma string. | ||
| # `allowed-tools` (Agent Skills standard) — accept list, comma string, or space-separated string. |
Collaborator
There was a problem hiding this comment.
Blocking CI issue: the exact head fails ruff format --check; Ruff reports this file would be reformatted. Please run ruff format src/skillspector/nodes/build_context.py and commit the result. The focused behavior tests already pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #327
What
Fixes the
allowed-toolsparsing bug where space-separated tool names were treated as a single tool, causing false positives on fully-declared skills.Why
The current parsing splits on commas but not whitespace. A declaration like
allowed-tools: Bash Readis parsed as a single tool namedBash Read, which never matches any actual tool name. This causes the LP1 check to fire on skills that have correctly declared their tools.How
build_context.py: split theallowed-toolsstring on whitespace (spaces, tabs, newlines) when no comma is present. Comma-separated strings continue to work as before.mcp_least_privilege.py: same fix in_normalize_allowed_toolsfor consistency.Testing
Manual verification
Tested on: Windows 11, Python 3.14
Steps:
allowed-tools: Bash Read(space-separated)skillspector scanon the test skillallowed-tools: Bash, Read(comma-separated) — backward compat confirmedallowed-tools: [Bash, Read](list form) — backward compat confirmedResult: PASS — space-separated parsed correctly, LP1 false positive eliminated, backward compatibility maintained.