Skip to content

fix(skills): safely tokenize portable allowed-tools patterns - #4984

Draft
georgelichen wants to merge 3 commits into
bytedance:mainfrom
georgelichen:fix/portable-allowed-tools-followup
Draft

fix(skills): safely tokenize portable allowed-tools patterns#4984
georgelichen wants to merge 3 commits into
bytedance:mainfrom
georgelichen:fix/portable-allowed-tools-followup

Conversation

@georgelichen

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #4912: #4912

This keeps portable allowed-tools parsing safe for Agent Skills declarations such as:

allowed-tools: Bash(tvly *)
allowed-tools: Bash(playwright-cli:*) Bash(npx:*) Bash(npm:*)
  • Tokenize the scalar form with parenthesis awareness so spaces inside Bash(...) stay in one entry.
  • Preserve exact names in the existing YAML-list form, including mixed-case MCP/runtime names.
  • Reject unmatched parentheses instead of silently creating fragments.
  • Keep argument-scoped Bash(...) entries literal and inactive; this PR does not broaden them into unrestricted bash access.
  • Add parser, validation, loader, and policy regression coverage.
  • No Xquik integration is included.

Verification

  • uv run pytest tests/test_skills_parser.py tests/test_skills_validation.py tests/test_skills_installer.py tests/test_skill_review_core.py tests/test_skill_reviewer_public_skill.py tests/test_skill_tool_policy_middleware.py -q — 176 passed
  • Ruff check and format check passed for changed Python files.
  • compileall and git diff --check passed.

The full backend suite currently has an unrelated Windows permission-mode failure in test_runtime_config_store_file_is_owner_only.

kriptoburak and others added 3 commits August 24, 2026 10:52
Portable Agent Skills declarations such as Bash(tvly *) contain spaces inside a command pattern. Keep those patterns as single literal entries while preserving exact names from the existing YAML-list form, so skill loading no longer fragments valid metadata or rewrites mixed-case MCP tools.

Constraint: DeerFlow's current skill policy matches exact tool names and does not inspect Bash arguments
Constraint: Agent Skills scalar syntax uses whitespace-separated entries with parenthesized command patterns
Rejected: raw.split() | fragments Bash(tvly *) into unrelated tool names
Rejected: normalize YAML-list entries | breaks case-sensitive MCP/runtime tool names
Rejected: map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep Bash(...) entries literal and inactive until DeerFlow has an explicit command-pattern authorization model
Tested: 175 focused parser, validation, installer, review, loader, and tool-policy tests; Ruff check and format; compileall; git diff --check
Not-tested: Full backend suite stopped at pre-existing Windows mode assertion test_runtime_config_store_file_is_owner_only
Related: bytedance#4912
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants