T09 · Insecure Skill Coding Practices
- Location
scripts/cli.py:453- Finding
Target-directory symlinks permit writes and recursive deletion outside the authorized migration root
- Content
View full analysis
Vulnerability Details
File Location:
scripts/cli.py:453-470, 884-892
Vulnerability Type: Symlink traversal during artifact deployment and orphan cleanup
Risk Level: HighVulnerable Code
python def write_artifact(artifact: PlannedArtifact, target_root: Path) -> None: target_path = target_root / artifact.relative_path target_path.parent.mkdir(parents=True, exist_ok=True) if isinstance(artifact.payload, GeneratedText): if target_path.is_symlink(): target_path.unlink() target_path.write_text(artifact.payload.content) return if isinstance(artifact.payload, SourceSymlink): if target_path.exists() or target_path.is_symlink(): target_path.unlink() target_path.symlink_to(symlink_target(artifact.payload.source_path, target_path)) return if target_path.is_symlink(): target_path.unlink() shutil.copy2(artifact.payload.source_path, target_path)python if not args.dry_run: for artifact in deployment_plan.artifacts: write_artifact(artifact, deployment_target_root) if deploy_mode == DeployMode.REPLACE: for orphan in deployment_plan.orphaned_skill_dirs: shutil.rmtree(orphan) for orphan in deployment_plan.orphaned_agent_files: orphan.unlink()Technical Analysis
The deployment code constructs destinations by appending an artifact-relative path to the user-selected target root, but it does not resolve the resulting path and verify that it remains inside that root.
Although
write_artifact()handles a symlink at the final destination, it does not reject symlinks in parent components. For example, if.agents/skillsis a symlink to a directory elsewhere in the filesystem, writing.agents/skills/example/SKILL.mdfollows that parent symlink. The resulting write occurs outside the selected migration target.The replace-mode cleanup has the same containment weakness. Orphan path ...[truncated 2034 chars]
- Remediation
View remediation
Remediation Suggestions
-
Resolve the target root once and reject it if it is itself a symlink:
python resolved_root = target_root.resolve(strict=True) -
Before every write, copy, symlink creation, unlink, or recursive deletion, resolve the candidate's existing parent and verify containment:
python resolved_parent = target_path.parent.resolve(strict=True) resolved_parent.relative_to(resolved_root)Reject the operation if containment validation raises
ValueError. -
Explicitly reject symlinks in every destination path component rather than checking only
target_path.is_symlink(). -
Use no-follow or directory-descriptor-based filesystem operations where supported to reduce time-of-check/time-of-use races.
-
For
--replace, validate each deletion candidate immediately before deletion and require it to resolve beneath the exact expected root:- skills beneath
<target>/.agents/skills; - agents beneath
<target>/.codex/agents.
- skills beneath
-
Do not define every unplanned directory or file as removable. Maintain a manifest of artifacts generated by this migrator and restrict cleanup to manifest-owned paths.
-
Refuse recursive deletion if the candidate or any relevant parent is a symlink. Apply the same policy during planning and during the real execution, since planning alone cannot prevent a later symlink substitution.
-
Add regression tests covering symlinked
.agents,.agents/skills,.codex, and.codex/agentsparents for both normal deployment and--replace.
-