T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/episode_bundle.py:38
- Finding
- User-Controlled Episode Directory Prefix Allows Arbitrary Directory Deletion and Out-of-Root Writes## Vulnerability Details **File Location**: `scripts/episode_bundle.py:38-39, 115-125, 139-143, 254-257, 281-282` **Vulnerability Type**: Path traversal and unrestricted filesystem operation **Risk Level**: High ### Vulnerable Code ```python def episode_dir(number: int, output_root: Path, prefix: str) -> Path: return output_root / f"{prefix}{episode_slug(number)}" ``` ```python def command_prepare_dest(args: argparse.Namespace) -> int: number = normalize_episode_number(args.episode) output_root = ensure_absolute_path(args.output_root, label="output-root", require_exists=False) assert output_root is not None dest_dir = episode_dir(number, output_root, args.episode_dir_prefix) existed = dest_dir.exists() if existed and args.clear_existing: shutil.rmtree(dest_dir) dest_dir.mkdir(parents=True, exist_ok=True) ``` ```python def command_bundle(args: argparse.Namespace) -> int: number = normalize_episode_number(args.episode) slug = episode_slug(number) output_root = ensure_absolute_path(args.output_root, label="output-root", require_exists=False) assert output_root is not None dest_dir = episode_dir(number, output_root, args.episode_dir_prefix) dest_dir.mkdir(parents=True, exist_ok=True) ``` ```python prepare_dest.add_argument("--episode-dir-prefix", default=DEFAULT_EPISODE_DIR_PREFIX) ``` ```python bundle.add_argument("--episode-dir-prefix", default=DEFAULT_EPISODE_DIR_PREFIX) ``` ### Technical Analysis The `--episode-dir-prefix` argument is accepted without validation and is directly combined with the configured output root. The implementation does not reject absolute prefixes, directory separators, or `..` traversal components, and it does not verify that the resulting destination remains beneath `output_root`. In `pathlib`, joining a path with an absolute second operand discards the original base path. For example, if ` ...[truncated 2603 chars]
- Remediation
- ## Remediation Suggestions 1. Treat the prefix strictly as a filename prefix, not as a path. Reject absolute values, path separators, `..`, and empty or special path components. 2. Apply an allowlist such as `^[A-Za-z0-9._-]+$`, with an appropriate maximum length. 3. Resolve both the output root and proposed destination before any filesystem operation, then enforce strict containment: ```python def safe_episode_dir(number: int, output_root: Path, prefix: str) -> Path: if not re.fullmatch(r"[A-Za-z0-9._-]+", prefix): raise SystemExit("episode-dir-prefix contains invalid characters") root = output_root.resolve(strict=False) destination = (root / f"{prefix}{episode_slug(number)}").resolve(strict=False) if destination == root or root not in destination.parents: raise SystemExit("episode destination escapes output-root") return destination ``` 4. Perform this validation immediately before every deletion, directory creation, copy, and manifest write so later code changes cannot bypass the boundary. 5. Refuse recursive deletion of the output root itself and other protected locations. Consider requiring a marker file created by this application before clearing an existing directory. 6. Add regression tests covering absolute prefixes, `../` traversal, nested separators, symbolic-link boundary cases, empty prefixes, and valid ordinary prefixes. 7. Run the bundling process under a least-privileged account with write access restricted to the intended output root as defense in depth.