T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/write_raw_reviews.py:20
- Finding
- Unvalidated Channel Identifiers Permit File Writes Outside the Intended Review Directory## 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 Practices ### Vulnerable 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 `--expected` command-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 `--expected` argument used by `run_daily_review.py`, can provide a channel identifier such as `../../../../tmp/audit-output`. `pathlib` preserves the traversal components, so the final write may resolve outside `out_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
- ## Remediation Suggestions 1. 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}") ``` 2. 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}" ``` 3. 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()`, use `os.path.commonpath()` on resolved paths. 4. Apply the same validation in `discover_channels.py` and `normalize_channel_data.py` so unsafe identifiers are rejected at every trust boundary. 5. Consider exclusive creation or explicit overwrite authorization when a destination already exists. 6. Add regression tests covering `../`, absolute paths, backslashes, encoded separators, control characters, malformed dates, and symlink-based containment escapes.