T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/init_skill.py:194
- Finding
- Skill Name Path Traversal Allows Writes Outside the Requested Destination<![CDATA[ ## Vulnerability Details **File Location**: `scripts/init_skill.py`, lines 194-221 **Vulnerability Type**: Path traversal and insufficient path validation **Risk Level**: Medium ### Vulnerable Code ```python # Determine skill directory path skill_dir = Path(path).resolve() / skill_name # Check if directory already exists if skill_dir.exists(): print(f"❌ Error: Skill directory already exists: {skill_dir}") return None # Create skill directory try: skill_dir.mkdir(parents=True, exist_ok=False) print(f"✅ Created skill directory: {skill_dir}") except Exception as e: print(f"❌ Error creating directory: {e}") return None # Create SKILL.md from template skill_title = title_case_skill_name(skill_name) skill_content = SKILL_TEMPLATE.format( skill_name=skill_name, skill_title=skill_title ) skill_md_path = skill_dir / 'SKILL.md' try: skill_md_path.write_text(skill_content) print("✅ Created SKILL.md") except Exception as e: print(f"❌ Error creating SKILL.md: {e}") return None ``` ### Technical Analysis The script accepts `skill_name` directly from the command line and appends it to the resolved output path without validating that it is a single safe path component. Although the CLI help states that skill names must contain only lowercase letters, digits, and hyphens, this requirement is not enforced. Consequently: - A value containing `../` can traverse outside the requested `--path` directory. - An absolute path can cause `pathlib` to discard the intended base directory. - Nested path separators can create files in unintended subdirectories. The script subsequently creates the computed directory and writes `SKILL.md` and example resources into it. The existing-directory check limits direct overwriting, but it does not prevent creation of a new directory outside the authorized destination. ### Attack Path 1. An attacker or untrusted automation supplies a malicious skill name, such as `../outside-skill` ...[truncated 1079 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate `skill_name` before performing any filesystem operation: ```python if not re.fullmatch(r"[a-z0-9]+(?:-[a-z0-9]+)*", skill_name): raise ValueError("Invalid skill name") ``` 2. Explicitly reject absolute paths, separators, and special path components: ```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 path component") ``` 3. Resolve the final destination and enforce containment beneath the base directory: ```python base_dir = Path(path).resolve() skill_dir = (base_dir / skill_name).resolve() if skill_dir.parent != base_dir: raise ValueError("Skill path escapes the output directory") ``` 4. Apply the same validation inside `init_skill()` rather than relying only on CLI parsing, because the function may also be imported and called programmatically. 5. Add regression tests covering `../name`, absolute paths, embedded separators, empty names, leading or trailing hyphens, and consecutive hyphens. ]]>