T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/save_persona.py:89
- Finding
- Path Traversal Through Username Allows Writes Outside the Persona Directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/save_persona.py:89-108` **Vulnerability Type**: Path traversal and arbitrary JSON file write **Risk Level**: High ### Vulnerable Code ```python user_name = meta['user'] output = { "_comment": "Auto-generated by persona-creator skill. Do not edit manually.", "user_id": user_name, "display_name": user_name, "created_at": now, "updated_at": now, "version": 1, "statistics": { "analyzed_messages": len(meta.get('messages', [])), "date_range": date_range, "memory_files_scanned": file_names }, "persona": analysis } output_path = persona_dir / f"{user_name}.json" # If it already exists, retain created_at if output_path.exists(): with open(output_path, 'r', encoding='utf-8') as f: existing = json.load(f) output['created_at'] = existing.get('created_at', now) output['version'] = existing.get('version', 1) + 1 with open(output_path, 'w', encoding='utf-8') as f: json.dump(output, f, ensure_ascii=False, indent=2) ``` The username originates from metadata generated from the `--user` command-line argument in `scripts/analyze.py`: ```python meta = { "user": args.user, "messages": messages, "memory_files": [str(f) for f in memory_files], "persona_dir": args.persona_dir, "template_path": template_path } ``` ### Technical Analysis The user-controlled username is interpolated directly into a filesystem path without an allowlist, canonicalization, or containment check: ```python output_path = persona_dir / f"{user_name}.json" ``` `pathlib.Path` does not prevent traversal components such as `../`. A username such as `../config/profile` therefore produces a destination equivalent to `persona_dir/../config/profile.json`. Because the code opens the destination in write mode, an existing JSON file outside the intended persona directory can be replaced with attacker-influenced persona data. Creation outside the ...[truncated 1187 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Restrict usernames to a conservative allowlist, for example: ```python if not re.fullmatch(r"[A-Za-z0-9_-]{1,64}", user_name): raise ValueError("Invalid username") ``` 2. Explicitly reject absolute paths, path separators, `.` and `..`. 3. Resolve both the base directory and destination before writing, then enforce containment: ```python base = Path(meta["persona_dir"]).resolve() destination = (base / f"{user_name}.json").resolve() if destination.parent != base: raise ValueError("Persona path escapes the configured directory") ``` 4. Consider mapping external usernames to generated internal identifiers rather than using display names as filenames. 5. Avoid following symlinks when the threat model includes other local users. 6. Write to a secure temporary file in the destination directory and atomically replace the target only after validation. ]]>