Repository navigation
fix(skills): reject skill names that are not one path component - #163
Merged
Merged
Conversation
Each skill name becomes a directory under the install dir, and install and uninstall delete that directory with shutil.rmtree. Names read from a project's .parallel-cli-skills-manifest.json were never validated, and that file ships with the repository. An entry like "../../../x" let a repo make `parallel-cli skills install --project` delete a directory outside the project, and "." wiped the whole install dir, unmanaged skills included. Accept only names that are a single path component under both POSIX and Windows rules. Unsafe manifest entries are ignored, like other malformed entries. An unsafe name in the remote index fails the install, because it would steer file writes outside the install dir the same way. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
zhengchaol
marked this pull request as ready for review
October 23, 2026 23:52
The install manifest was written with write_text, which follows a symlink. A repository could ship .parallel-cli-skills-manifest.json as a link to a file elsewhere, and `skills install --project` would overwrite that file with JSON. Write a temp file in the install dir and os.replace it, which swaps the link out and also makes the update atomic. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
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.
Skill names from a project's
.parallel-cli-skills-manifest.jsonreachedshutil.rmtree(install_dir / name)without validation. That file ships with the repo, so an entry like../../../xletparallel-cli skills install --projectdelete directories outside the project. Names from the manifest and from the index must now be a single path component. Unsafe manifest entries are ignored, and unsafe index names fail the install.The manifest itself was also written through a symlink, so a repo shipping it as a link to another file had install overwrite that file. It is now written to a temp file and
os.replaced into place.Reproduced against the production index before and after the change. The new tests fail on
main.🤖 Generated with Claude Code