Security: skill installer should reject repository symlinks escaping the selected skill directory

[PeterShanxin]

Observed independently in a downstream security review against main (skills/.system/skill-installer/scripts/install-skill-from-github.py).

Summary

The Git-based install path clones an untrusted repository (_git_sparse_checkout) and then copies the selected skill directory with shutil.copytree(src, dest_dir) (_copy_skill). shutil.copytree with default symlinks=False follows symlinks and copies the target’s contents, and the current code performs no validation that a symlink (or its resolved target) remains inside the checked-out selected skill root. A repository that contains a symlink inside the selected skill directory whose resolved target lies outside that directory (or is an absolute path) will therefore cause the installer to copy files from outside the selected skill into $CODEX_HOME/skills.

Verification against current upstream

• Inspected install-skill-from-github.py:140 _copy_skill → shutil.copytree(src, dest_dir) with no symlinks handling, no realpath containment check, no per-entry validation.
• _validate_relative_path and _validate_skill_name guard the requested path/name but not entries inside the checked-out directory.
• _safe_extract_zip guards zip entry traversal but not symlink targets; the git path (the primary “untrusted repo” surface) has no symlink check at all.
• No existing issue/PR for symlink or copytree in this repo’s open 290 issues (checked symlink 2 hits unrelated, install-skill 18 hits none covering this).
• No SECURITY.md in the repository enforces private disclosure for this class; per-reporting instructions a public hardening issue is filed neutrally. If the maintainers prefer private handling, please indicate and this can be moved.

Threat-model note

This is a local file-copy hardening issue, not a remote code execution by itself. Impact depends on what the symlink target contains and where $CODEX_HOME/skills later gets loaded from. The installer is explicitly designed to clone untrusted repositories, so the trust boundary is the selected skill directory, not the whole repo.

Suggested hardening (neutral, non-breaking)

• Reject symlinks in the installed skill by default, or preserve them only when os.path.realpath(entry) is verified to be inside os.path.realpath(selected_skill_root).
• Verify destination containment before each copied entry (walk src rather than blind copytree, or use copytree(..., symlinks=False) with explicit per-file realpath checks and O_NOFOLLOW-style handling).
• Add regression tests for: external absolute symlink, external relative symlink (../../...), internal symlink, and symlink to directory outside selected root.

No sensitive file names, payload that copies real secrets, or weaponized exploit code is included. The description is intentionally generic. Happy to provide additional details privately.