Skip to content

fix(security): reject path-escaping skill directory names - #125

Draft
cursor[bot] wants to merge 3 commits into
mainfrom
cursor/critical-bug-investigation-bb41
Draft

fix(security): reject path-escaping skill directory names#125
cursor[bot] wants to merge 3 commits into
mainfrom
cursor/critical-bug-investigation-bb41

Conversation

@cursor

@cursor cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

Summary

  • Fixes critical path traversal where skilllite add trusted SKILL.md frontmatter name for install destinations (skills_path.join(name)), allowing writes outside the skills root (e.g. name: ../keep-me).
  • Fixes the same class of escape in MCP get_skill_info / run_skill and CLI find_skill / remove by validating skill names as a single safe path segment before join.
  • Adds shared validate_skill_dir_name / skill_dir_under_root in skilllite-core plus regression tests for add/MCP/show escape attempts.

Task Linkage

  • Task ID: TASK-2026-073
  • Task folder: tasks/TASK-2026-073-skill-name-path-safety/

Injected Specs

  • spec/verification-integrity.md
  • spec/task-artifact-language.md
  • spec/security-nonnegotiables.md
  • spec/architecture-boundaries.md (reviewed; no boundary changes)
  • spec/rust-conventions.md
  • spec/testing-policy.md
  • spec/docs-sync.md (reviewed; no docs needed — no user-facing command/env/policy wording change)

Validation Evidence

  • Commands executed:
    • cargo test -p skilllite-core path_validation
    • cargo test -p skilllite-commands path_traversal
    • cargo test -p skilllite --test cli_mcp --test cli_skill_management
    • cargo fmt --check
    • cargo clippy -p skilllite-core -p skilllite-commands -p skilllite --all-targets -- -D warnings -A clippy::question_mark -A clippy::useless_borrows_in_formatting
    • python3 scripts/validate_tasks.py
  • Key results:
    • core path_validation: 3 passed
    • commands path_traversal: 3 passed (includes add frontmatter escape rejection)
    • cli_mcp + cli_skill_management: 14 + 21 passed
    • fmt clean; clippy clean with pre-existing allow categories noted on main
    • task validation: Task validation passed (71 task directories checked)

Regression Scope

Docs Sync (EN/ZH)

  • Not needed
  • Updated EN + ZH docs
  • Files:
    • N/A

Review Checklist

  • Acceptance criteria in tasks/TASK-2026-073-skill-name-path-safety/TASK.md satisfied
  • tasks/TASK-2026-073-skill-name-path-safety/STATUS.md updated with latest progress
  • tasks/TASK-2026-073-skill-name-path-safety/REVIEW.md includes merge readiness decision
  • tasks/board.md status is up to date

Security review notes

  • What security policy changed, and why is it needed? Fail-closed single-segment skill directory name validation before skills-root joins; prevents filesystem escape via malicious frontmatter or caller-controlled skill names.
  • Is default behavior more permissive? No — stricter validation only.
  • Does this affect SKILLLITE_* config semantics or backward compatibility? No env/config change; only rejects previously unsafe multi-segment/absolute names.
  • Were tests and EN/ZH docs updated? Tests yes; docs not required.
Open in Web View Automation 

cursoragent and others added 3 commits July 27, 2026 11:03
Validate skill names as a single path segment before joining under
skills roots so malicious SKILL.md frontmatter and MCP/CLI skill_name
values cannot write, read, or delete outside the skills directory.

Co-authored-by: EXboy <EXboys@users.noreply.github.com>
Apply rustfmt to the new add regression test and mark TASK-2026-073
validation complete on the task board.

Co-authored-by: EXboy <EXboys@users.noreply.github.com>
Co-authored-by: EXboy <EXboys@users.noreply.github.com>
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.

1 participant