T09 · Insecure Skill Coding Practices
- Location
scripts/snapshot.py:56- Finding
Path Traversal Through Unvalidated Snapshot Slug
- Content
View full analysis
Vulnerability Details
File Location:
scripts/snapshot.py:56-59, with write operations atscripts/snapshot.py:109-122andscripts/snapshot.py:161-170
Vulnerability Type: Path traversal leading to arbitrary Markdown file creation or overwrite
Risk Level: MediumComplete Code Snippet
python def snapshot_path(workspace: Path, slug: str | None) -> Path: if slug: return snapshots_dir(workspace) / f"{slug}.md" return single_snapshot_path(workspace)The resulting path is used without containment validation:
python def cmd_save(args: argparse.Namespace) -> int: workspace = Path(args.workspace).resolve() slug = args.slug or (slugify(args.course) if args.course else None) path = snapshot_path(workspace, slug) path.parent.mkdir(parents=True, exist_ok=True) text = read_stdin() path.write_text(text, encoding="utf-8") if slug and args.active: active_path(workspace).write_text(slug, encoding="utf-8") write_state_json(workspace, text, slug, path) print(path) return 0A second write-capable command uses the same unsafe path construction:
python def cmd_set_active(args: argparse.Namespace) -> int: workspace = Path(args.workspace).resolve() if not args.slug and not args.course: raise SystemExit("set-active requires --slug or --course.") slug = args.slug or slugify(args.course) path = snapshot_path(workspace, slug) if args.require_exists and not path.exists(): raise SystemExit(f"Snapshot not found: {path}") path.parent.mkdir(parents=True, exist_ok=True) active_path(workspace).write_text(slug, encoding="utf-8") print(slug) return 0Technical Analysis
The
--slugcommand-line argument is accepted directly and concatenated into a filesystem path. Unlike values produced byslugify, an explicit slug is not restricted to a safe file ...[truncated 2095 chars]- Remediation
View remediation
Remediation Suggestions
-
Reject explicit slugs unless they match a strict allowlist, such as
^[A-Za-z0-9_-]+$or an equivalent Unicode-aware policy. -
Prefer processing every supplied slug through
slugify()rather than trusting--slugverbatim. -
Resolve both the state directory and candidate path, then verify containment before any read, directory creation, or write:
python def safe_snapshot_path(workspace: Path, slug: str) -> Path: if not re.fullmatch(r"[\w-]+", slug, flags=re.UNICODE): raise SystemExit("Invalid snapshot slug.") base = snapshots_dir(workspace).resolve() candidate = (base / f"{slug}.md").resolve() if candidate.parent != base: raise SystemExit("Snapshot path escapes the state directory.") return candidate -
Apply the same validation to slugs read from the
_activefile, because persisted state may have been modified externally. -
Validate containment before
mkdir(), not only beforewrite_text(). -
Add regression tests covering
../, nested traversal, absolute paths, empty values, path separators, and malicious_activecontents. -
When possible, run the Skill with filesystem permissions restricted to its workspace.
-