T09 · Insecure Skill Coding Practices
Error
- Location
scripts/specctl.py:146- Finding
Path Traversal Through Unvalidated Spec Identifier
- Content
View full analysis
tuple[Path, str]: active = ctx.specs_active_dir / spec_id if active.exists(): return active, "active" archived = ctx.specs_archive_dir / spec_id if archived.exists(): return archived, "archive" raise SystemExit(f"Spec not found: {spec_id}") ``` The initialization command also constructs its destination directly from the attacker-controlled identifier: ```python def cmd_init(args: argparse.Namespace) -> int: ctx = load_context() ensure_workspace_dirs(ctx) steering_created = init_steering(ctx) spec_dir = ctx.specs_active_dir / args.spec_id if spec_dir.exists(): raise SystemExit(f"Spec already exists: {spec_dir}") spec_dir.mkdir(parents=True, exist_ok=False) timestamp = iso_now() replacements = { "": args.spec_id, "": args.title, "": args.title, "": timestamp, } meta = load_json(ctx.templates_dir / "meta.json") meta["spec_id"] = args.spec_id meta["title"] = args.title meta["spec_kind"] = args.kind meta["created_at"] = timestamp meta["updated_at"] = timestamp save_meta(spec_dir, meta) ``` The archive command repeats the unsafe path construction: ```python def cmd_archive(args: argparse.Namespace) -> int: ctx = load_context() active_dir = ctx.specs_active_dir / args.spec_id if not active_dir.exists(): archived_dir = ctx.specs_archive_dir / args.spec_id if archived_dir.exists(): raise SystemExit(f"Spec is already archived: {args.spec_id}") raise SystemExit(f"Spec not found: {args.spec_id}") meta = load_meta(ac ...[truncated 3526 chars]- Remediation
View remediation
str: if not SPEC_ID_RE.fullmatch(spec_id): raise SystemExit( "Invalid spec_id: use only letters, digits, '.', '_' and '-', " "with a maximum length of 64 characters" ) if spec_id in {".", ".."}: raise SystemExit("Invalid spec_id") return spec_id ``` 2. **Enforce canonical path containment independently of syntax validation.** ```python def contained_child(base: Path, name: str) -> Path: validate_spec_id(name) resolved_base = base.resolve() candidate = (resolved_base / name).resolve() if not candidate.is_relative_to(resolved_base): raise SystemExit("Resolved spec path escapes the workspace") return candidate ``` Use this helper for all paths under both `specs_active_dir` and `specs_archive_dir`. 3. **Reject path-like identifiers explicitly.** Reject: - Absolute paths. - `/` and `\` path separators. - `.` and `..` path components. - Platform-specific drive or UNC path forms. - Null bytes and control characters. 4. **Account for symbolic links.** Validate resolved paths immediately before sensitive operations. For stronger protection, reject symbolic links in relevant path components or use descriptor-relative filesystem operations where supported to reduce time-of-check/time-of-use risks. 5. **Validate before any filesystem operation.** Apply validation in one shared function used by `init`, `find_spec_dir`, and `archive`, rather than relying on individual command handlers. 6. **Add regression tests.** Tests should verify rejection of: - `../outside` - `../../outside` - Absolute POS ...[truncated 269 chars]