T09 · Insecure Skill Coding Practices
Error
- Location
scripts/briefing.sh:207- Finding
Configurable output path enables deletion of unrelated directories
- Content
View full analysis
dict: config = deep_copy(DEFAULT_CONFIG) if CONFIG_FILE.exists(): try: with open(CONFIG_FILE, 'r') as f: user_config = json.load(f) config = deep_merge(config, user_config) except json.JSONDecodeError as e: print(f"⚠️ Config corrupted: {e}", file=sys.stderr) print(f" Using defaults. Fix or delete: " f"{CONFIG_FILE}", file=sys.stderr) except IOError: pass return config ``` `scripts/config.py:170-183` validates paths only when they are changed through the `set` command. The validation also uses a string-prefix comparison rather than a path-component-aware containment check: ```python SAFE_OUTPUT_PREFIXES = [ os.path.realpath(os.path.expanduser("~/Documents")), os.path.realpath(os.path.expanduser("~/Desktop")), os.path.realpath(os.path.expanduser("~/Downloads")), os.path.realpath(os.path.expanduser("~/.briefing-room")), os.path.realpath("/tmp"), ] def _validate_output_path(path: str) -> bool: """Reject output.folder paths outside safe directories.""" expanded = os.path.realpath(os.path.expanduser(path)) return any(expanded.startswith(prefix) for prefix in SAFE_OUTPUT_PREFIXES) def set_value(key: str, value) -> bool: if key == "output.folder" and isinstance(value, str): if not _validate_output_path(value): print(f"Error: output.folder must be under ~/Documents, ~/Desktop, ~/Downloads, ~/.briefing-room, or /tmp", file ...[truncated 3712 chars]- Remediation
View remediation
bool: try: path.relative_to(root) return path != root except ValueError: return False ``` On older versions, use `os.path.commonpath()` and explicitly reject equality with the root. 3. **Require a dedicated application directory.** Restrict output to directories such as: - `~/Documents/Briefing Room` - `~/Desktop/Briefing Room` - `~/Downloads/Briefing Room` - `~/.briefing-room/output` - A uniquely named child under `/tmp` Do not accept `/tmp`, `~/Documents`, `~/Desktop`, or `~/Downloads` themselves. 4. **Mark and verify application-owned directories.** Create a marker such as `.briefing-room-owned` in the output root and require that marker before any cleanup operation. 5. **Validate deletion targets individually.** Before deletion, resolve each candidate and verify that: - It is strictly below the normalized output root. - Its basename exactly matches `YYYY-MM-DD`, rather than merely beginning with `20`. - It contains the application ownership marker or expected briefing artifacts. - It is not a symbolic-link traversal target. 6. **Prefer constrained deletion over broad `rm -rf`.** Remove only known briefing filenames and delete the directory only after it is empty. Add a dry-run mode and log every target before deletion. 7. **Fail closed.** If configuration validation, path resolution, ...[truncated 91 chars]