T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/build_finetune_plan.py:101
- Finding
- Dry-Run Mode Still Performs Filesystem Writes## Vulnerability Details **File Location**: `scripts/build_finetune_plan.py:16, 101-105` **Vulnerability Type**: Ineffective safety control leading to unintended file creation or overwrite **Risk Level**: Medium ### Vulnerable Code ```python parser.add_argument("--dry-run", action="store_true", help="Run without side effects.") ``` ```python "dry_run": args.dry_run, }, } render(result, Path(args.output), args.format) ``` The `render()` function unconditionally creates the destination directory and writes the requested output: ```python def render(result: dict, output_path: Path, fmt: str) -> None: output_path.parent.mkdir(parents=True, exist_ok=True) if fmt == "json": output_path.write_text(json.dumps(result, indent=2), encoding="utf-8") return ``` ### Technical Analysis The command-line description promises that `--dry-run` will run without side effects. However, the flag is only copied into the generated result and is never evaluated before `render()` is called. Consequently, dry-run execution creates parent directories and creates or overwrites the output file. The output path is supplied by the caller and is not restricted to a dedicated artifact directory. The overwrite therefore applies to any path writable by the account running the script. This does not bypass operating-system permissions or provide privilege escalation, but it defeats a documented safety control that operators or automation may rely upon before approving a real write. ### Attack Path 1. An attacker or untrusted automation influences the `--output` argument. 2. An operator invokes the script with `--dry-run`, expecting validation without filesystem changes. 3. The script builds the result without checking `args.dry_run`. 4. `render()` creates the destination's parent directories and writes to the selected file. 5. If the destination already exists and is writable, i ...[truncated 629 chars]
- Remediation
- ## Remediation Suggestions Enforce dry-run behavior before any call that mutates the filesystem: ```python if args.dry_run: print(json.dumps(result, indent=2)) return 0 render(result, Path(args.output), args.format) ``` Additional hardening should include: - Ensure dry-run mode does not call `mkdir()`, `open()`, `write_text()`, or any other state-changing operation. - Refuse to overwrite an existing output file unless the caller supplies an explicit `--force` option. - If the application has a defined workspace, resolve the destination path and verify that it remains inside the authorized output directory. - Add regression tests that snapshot the filesystem before and after dry-run execution and assert that no files or directories were created or modified. - Update the CLI help only if dry-run is intentionally meant to write output; otherwise, preserve the documented no-side-effect contract.