T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/init_skill.py:207
- Finding
- Unvalidated Skill Name Allows Output-Path Traversal<![CDATA[ ## Vulnerability Details **File Location**: `scripts/init_skill.py`, lines 207 and 285–286 **Vulnerability Type**: Path traversal through an unvalidated directory name **Risk Level**: Medium ### Vulnerable Code ```python # Determine skill directory path skill_dir = Path(path).resolve() / skill_name ``` The command-line input is passed directly to this operation: ```python skill_name = sys.argv[1] path = sys.argv[3] print(f"🚀 Initializing skill: {skill_name}") print(f" Location: {path}") print() result = init_skill(skill_name, path) ``` ### Technical Analysis The command-line help states that a skill name must be a lowercase hyphen-case identifier, but the implementation does not enforce that requirement. The attacker-controlled `skill_name` is appended directly to the resolved base path. A relative name containing parent-directory components, such as `../../new-target`, causes filesystem operations to escape the directory supplied through `--path`. If `skill_name` is an absolute path, `pathlib` discards the preceding base path and uses the absolute path directly. After constructing the unsafe path, the initializer creates the directory and writes generated files under it, including: - `SKILL.md` - `scripts/example.py` - `references/api_reference.md` - `assets/example_asset.txt` The script refuses to proceed when the resulting directory already exists, so this flaw does not directly overwrite an existing target directory. It nevertheless permits creation of a new directory and files outside the intended output boundary wherever the caller has write permission. ### Attack Path 1. An attacker or untrusted automation supplies a crafted skill name: ```bash python scripts/init_skill.py ../../outside-target --path /tmp/allowed ``` 2. The program constructs `/tmp/allowed/../../outside-target` without validating the skill name or checking the final resolved destination. 3. The operating system resolves the parent-directory components, ...[truncated 996 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce the documented naming convention before performing any filesystem operation: ```python import re SKILL_NAME_PATTERN = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") if not SKILL_NAME_PATTERN.fullmatch(skill_name): raise ValueError( "Skill name must contain only lowercase letters, digits, " "and single separating hyphens" ) ``` 2. Explicitly reject absolute paths and path separators: ```python candidate_name = Path(skill_name) if candidate_name.is_absolute() or len(candidate_name.parts) != 1: raise ValueError("Skill name must be a single relative path component") ``` 3. Resolve and verify the final destination remains beneath the intended base directory: ```python base_dir = Path(path).resolve() skill_dir = (base_dir / skill_name).resolve() try: skill_dir.relative_to(base_dir) except ValueError: raise ValueError("Resolved skill directory escapes the output directory") ``` 4. Apply validation inside `init_skill()` rather than only in the CLI wrapper so programmatic callers receive the same protection. 5. Add regression tests for `../target`, `../../target`, absolute paths, path separators, empty names, consecutive hyphens, and valid hyphen-case names. ]]>