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
- Content
View full analysis
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: HighVulnerable 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-prefixargument 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 beneathoutput_root.In
pathlib, joining a path with an absolute second operand discards the original base path. For example, if ` ...[truncated 2603 chars]- Remediation
View remediation
Remediation Suggestions
- Treat the prefix strictly as a filename prefix, not as a path. Reject absolute values, path separators,
.., and empty or special path components. - Apply an allowlist such as
^[A-Za-z0-9._-]+$, with an appropriate maximum length. - 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 - Perform this validation immediately before every deletion, directory creation, copy, and manifest write so later code changes cannot bypass the boundary.
- 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.
- Add regression tests covering absolute prefixes,
../traversal, nested separators, symbolic-link boundary cases, empty prefixes, and valid ordinary prefixes. - Run the bundling process under a least-privileged account with write access restricted to the intended output root as defense in depth.
- Treat the prefix strictly as a filename prefix, not as a path. Reject absolute values, path separators,