Skip to content

fix(translate-skills): reject deeper-path traversal in sibling-skill regex#2

Merged
vladolaru merged 1 commit into
vladolaru:mainfrom
georgeolaru:fix/sibling-skill-ref-parent-traversal
May 13, 2026
Merged

fix(translate-skills): reject deeper-path traversal in sibling-skill regex#2
vladolaru merged 1 commit into
vladolaru:mainfrom
georgeolaru:fix/sibling-skill-ref-parent-traversal

Conversation

@georgeolaru

Copy link
Copy Markdown
Contributor

Summary

  • Tightens the sibling-skill reference regex so legitimate ../../path/ documentation in skill content no longer derails reconcile.
  • Adds a regression test covering the real-world trigger (claude-session-driver documents that scripts live at ../../scripts/ relative to the skill base directory).

Problem

The regex (?<!\.)\.\./(?P<skill>[A-Za-z0-9._-]+)/ allowed the captured name to consist solely of dots. On input like ../../scripts/, re.findall matched ../../ with skill="..". The bridge then resolved two levels up from the skill directory and tried to vendor the entire plugin — including .claude-plugin/marketplace.json — into the generated skill. The resulting relative_path entries started with .., which tripped _normalize_skill_relative_path's traversal guard in registry.py, aborting reconcile with:

Error: Generated skill file path may not contain parent traversal: ../.claude-plugin/marketplace.json

Fix

Both sides of the ../ are tightened:

  • Lookbehind extended from (?<!\.)(?<![./]) so /../ (part of a deeper path) no longer matches.
  • Captured name must start with [A-Za-z0-9_-], rejecting . and .. outright.

Existing tests still pass; one new regression test confirms ../../scripts/ no longer produces files with leading .. in their relative paths.

Test plan

  • pytest tests -q — 1014 passed (was 1013, +1 new test).
  • Confirmed the new test fails on main and passes after the fix.

…regex

Skill content that documents paths like `../../scripts/` was causing
reconcile to fail with a path-traversal error.  The sibling-reference
regex `(?<!\.)\.\./(?P<skill>[A-Za-z0-9._-]+)/` allowed the captured
name to consist solely of dots, so `re.findall` on `../../scripts/`
matched `../../` with `skill=".."`.  The bridge then resolved two levels
up from the skill directory and tried to vendor the entire plugin
(including `.claude-plugin/marketplace.json`) into the generated skill.
The resulting `relative_path` entries started with `..`, which tripped
the registry's `_normalize_skill_relative_path` guard.

The regex is now tightened on both sides of the `../`:

- Lookbehind extended from `(?<!\.)` to `(?<![./])` so a `../` preceded
  by `/` (i.e. part of a deeper path like `/../`) does not match.
- The captured name must start with `[A-Za-z0-9_-]`, which rejects `.`
  and `..` outright.

Real-world trigger: `claude-session-driver/driving-claude-code-sessions`
documents that scripts live at `../../scripts/` relative to the skill
base directory.
@vladolaru
vladolaru merged commit 9934e82 into vladolaru:main May 13, 2026
1 check passed
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