T09 · Insecure Skill Coding Practices
Error
- Location
scripts/task_manager.py:66- Finding
Unsanitized Agent ID Allows Path Traversal Outside the Task Data Directory
- Content
View full analysis
Path: agent_dir = AGENTS_DIR / agent_id agent_dir.mkdir(parents=True, exist_ok=True) return agent_dir / "task-list.json" ``` Agent identifiers are accepted directly from CLI arguments without validation, for example: ```python create_parser.add_argument("--agent", required=True, help="Agent ID") list_parser.add_argument("--agent", required=True, help="Agent ID") start_parser.add_argument("--agent", required=True, help="Agent ID") ``` The resulting path is subsequently used for file reads and writes: ```python def load_agent_task_list(agent_id: str) -> Dict[str, Any]: task_file = get_agent_task_file(agent_id) if not task_file.exists(): return { "agent_id": agent_id, "agent_name": agent_id, "current_task": None, "pending_tasks": [], "completed_tasks": [], "failed_tasks": [], "created_at": get_timestamp(), "updated_at": get_timestamp() } try: with open(task_file, 'r', encoding='utf-8') as f: return json.load(f) ``` ```python def save_agent_task_list(agent_id: str, task_list: Dict[str, Any]): task_file = get_agent_task_file(agent_id) task_list["updated_at"] = get_timestamp() with open(task_file, 'w', encoding='utf-8') as f: json.dump(task_list, f, ensure_ascii=False, indent=2) ``` ### Technical Analysis `agent_id` is treated as a filesystem path component rather than as an opaque identifier. Python's `pathlib` preserves traversal components such as `..`. If the supplied value is an absolute path, combining it with `AGENTS_DIR` also causes the intended base directory to be discarded. The implementation neither validates the identifier ...[truncated 2104 chars]- Remediation
View remediation
str: if not AGENT_ID_PATTERN.fullmatch(agent_id): raise ValueError("Invalid Agent ID") return agent_id ``` 2. Resolve and verify the resulting path before creating directories or accessing files: ```python def get_agent_task_file(agent_id: str) -> Path: validate_agent_id(agent_id) base = AGENTS_DIR.resolve() agent_dir = (base / agent_id).resolve() try: agent_dir.relative_to(base) except ValueError: raise ValueError("Agent path escapes the task data directory") agent_dir.mkdir(parents=True, exist_ok=True) return agent_dir / "task-list.json" ``` 3. Explicitly reject: - Absolute paths. - `/` and `\` path separators. - `.` and `..` path components. - Empty, excessively long, or control-character-containing identifiers. 4. Perform validation in the shared path-construction function so every CLI command and programmatic caller receives the same protection. 5. Add regression tests covering: - Relative traversal such as `../../tmp/target`. - Absolute paths. - Backslash-based traversal. - Nested path separators. - Symbolic-link escape attempts. - Valid identifiers containing only approved characters. ]]>