T09 · Insecure Skill Coding Practices
- Location
scripts/write_raw_reviews.py:20- Finding
Unvalidated Channel Identifiers Permit File Writes Outside the Intended Review Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/write_raw_reviews.py, lines 20-24
Vulnerability Type: Path traversal and arbitrary file write
Risk Level: High
Category: T09: Insecure Skill Coding PracticesVulnerable Code
python for item in data: date = item["date"] yyyy, mm, _ = date.split("-") out_dir = out_root / yyyy / mm out_dir.mkdir(parents=True, exist_ok=True) out_path = out_dir / f"{item['channel']}_{date}.md"The resulting path is subsequently written without containment validation:
python out_path.write_text(text, encoding="utf-8")The channel value originates from externally supplied input and is normalized only by trimming whitespace and converting it to lowercase:
python channel = str(item.get("channel", "")).strip().lower() if not channel: raise ValueError("channel is required")In the standard workflow, the
--expectedcommand-line argument also supplies channel names without enforcing a safe slug format:python expected = [x.strip().lower() for x in args.expected.split(',') if x.strip()]Technical Analysis
item["channel"]is incorporated directly into a filesystem path. No validation rejects path separators,..components, absolute paths, control characters, or platform-specific path syntax.An attacker who can control normalized input, or the
--expectedargument used byrun_daily_review.py, can provide a channel identifier such as../../../../tmp/audit-output.pathlibpreserves the traversal components, so the final write may resolve outsideout_root.The date is also used as directory and filename material after only checking that it contains three hyphen-separated components. Although normal CLI usage supplies a conventional date, direct script invocation can provide manipulated date components. Both values should be treated as untrusted path components.
The write content is template-controlled rather than fully attacker-controlled, but ...[truncated 1622 chars]
- Remediation
View remediation
Remediation Suggestions
- Enforce a strict channel slug allowlist before any path construction:
python import re CHANNEL_SLUG = re.compile(r"^[a-z0-9][a-z0-9_-]{0,63}$") channel = str(item.get("channel", "")).strip().lower() if not CHANNEL_SLUG.fullmatch(channel): raise ValueError(f"unsafe channel identifier: {channel!r}")- Parse and canonicalize dates rather than splitting strings:
python from datetime import date as date_type parsed = date_type.fromisoformat(item["date"]) date = parsed.isoformat() yyyy = f"{parsed.year:04d}" mm = f"{parsed.month:02d}"- Resolve the destination and verify that it remains under the intended root:
python safe_root = out_root.resolve() out_path = (safe_root / yyyy / mm / f"{channel}_{date}.md").resolve() if not out_path.is_relative_to(safe_root): raise ValueError("output path escapes the configured root")For Python versions lacking
Path.is_relative_to(), useos.path.commonpath()on resolved paths.-
Apply the same validation in
discover_channels.pyandnormalize_channel_data.pyso unsafe identifiers are rejected at every trust boundary. -
Consider exclusive creation or explicit overwrite authorization when a destination already exists.
-
Add regression tests covering
../, absolute paths, backslashes, encoded separators, control characters, malformed dates, and symlink-based containment escapes.