T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/package_skill.py:93
- Finding
- Skill packaging follows symbolic links and may disclose files outside the skill directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/package_skill.py`, lines 93–100 **Vulnerability Type**: Symlink-following information disclosure during archive creation **Risk Level**: Medium ### Vulnerable Code ```python for file_path in skill_path.rglob('*'): if not file_path.is_file(): continue arcname = file_path.relative_to(skill_path.parent) if should_exclude(arcname): print(f" Skipped: {arcname}") continue zipf.write(file_path, arcname) print(f" Added: {arcname}") ``` The validation performed by `scripts/quick_validate.py` does not reject symbolic links or verify that packaged files resolve inside the selected skill directory. ### Technical Analysis `Path.is_file()` follows symbolic links. Likewise, `zipfile.ZipFile.write()` opens the path and archives the contents of the file referenced by the link. The archive name is calculated from the unresolved path, so the resulting archive can make an external file appear to be an ordinary file inside the packaged skill. The exclusion logic only evaluates path names and selected directory names. It does not: - Call `is_symlink()` to reject symbolic links. - Resolve each candidate and verify containment within `skill_path`. - Use `lstat()` to distinguish regular files from links. - Restrict packaging to trusted file types or explicitly approved resources. Consequently, an untrusted imported skill can reference any file readable by the account running the packager. ### Attack Path 1. An attacker prepares a skill directory with valid `SKILL.md` frontmatter so that `validate_skill()` succeeds. 2. The attacker places a symbolic link inside the skill, for example: ```text assets/local-config.txt -> /home/victim/.config/application/credentials.json ``` 3. The victim imports the skill and runs `scripts/package_skill.py` against it. 4. `rglob('*')` discovers the symbolic link. 5. `file_path.is_file()` follows the link and returns true if its ...[truncated 929 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject symbolic links before adding files: ```python if file_path.is_symlink(): print(f" Skipped symlink: {file_path}") continue ``` 2. Resolve every candidate and enforce containment within the selected skill root: ```python skill_root = skill_path.resolve() for file_path in skill_path.rglob("*"): if file_path.is_symlink(): continue resolved = file_path.resolve(strict=True) if not resolved.is_relative_to(skill_root): raise ValueError(f"File escapes skill root: {file_path}") if not resolved.is_file(): continue ``` 3. Use `os.lstat()` or equivalent no-follow checks to ensure that only regular files are archived. 4. Apply containment validation to every parent component, because links in intermediate directories can also redirect traversal. 5. Fail packaging rather than silently continuing when an escaping link is detected. This makes malicious or accidental archive composition visible to the user. 6. Add automated tests covering: - A symlink to a file outside the skill directory. - A symlink to an internal file. - A symlinked directory. - Broken and cyclic links. - Nested paths containing symlinked parent directories. 7. Document that packaging untrusted skill trees is unsafe until the directory has passed link and containment validation. ]]>