T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/init_skill.py:200
- Finding
- Unvalidated Skill Name Allows Path Traversal and Filesystem Writes Outside the Output Directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/init_skill.py:200-246`, with unvalidated command-line input at `scripts/init_skill.py:288-289` **Vulnerability Type**: Path traversal and insufficient input validation **Risk Level**: Medium ### Vulnerable Code ```python def init_skill(skill_name, path): # 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 # Create resource directories with example files try: scripts_dir = skill_dir / 'scripts' scripts_dir.mkdir(exist_ok=True) example_script = scripts_dir / 'example.py' example_script.write_text(EXAMPLE_SCRIPT.format(skill_name=skill_name)) example_script.chmod(0o755) ``` The command-line values are passed directly to this function: ```python skill_name = sys.argv[1] path = sys.argv[3] result = init_skill(skill_name, path) ``` ### Technical Analysis The command-line help states that `skill_name` must be a kebab-case identifier, but the implementation does not enforce that requirement before using the value as a path component. The expression `Path(path).resolve() / skill_name` does not g ...[truncated 2068 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate `skill_name` before performing any filesystem operation. Enforce the documented format and length: ```python import re if not isinstance(skill_name, str): raise ValueError("Skill name must be a string") if len(skill_name) > 64 or not re.fullmatch( r"[a-z0-9]+(?:-[a-z0-9]+)*", skill_name, ): raise ValueError( "Skill name must be a kebab-case identifier of at most 64 characters" ) ``` 2. Explicitly reject absolute paths, path 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 relative path component") ``` 3. Resolve the final destination and verify containment beneath the base directory: ```python base_dir = Path(path).resolve() skill_dir = (base_dir / skill_name).resolve() if not skill_dir.is_relative_to(base_dir): raise ValueError("Skill destination escapes the requested output directory") ``` 4. Apply validation in both the CLI entry point and `init_skill()` so that library callers cannot bypass it. 5. Create files with restrictive permissions and avoid making generated placeholder scripts executable unless execution is required. 6. Add regression tests covering absolute names, `../`, nested path components, leading or trailing hyphens, consecutive hyphens, and names longer than 64 characters. ]]>
