T09 · Insecure Skill Coding Practices
Error
- Location
scripts/package_sticker.py:581- Finding
Unrestricted Output Path Permits Arbitrary Directory Replacement and Recursive Deletion
- Content
View full analysis
int: final_output = args.output.resolve() final_output.parent.mkdir(parents=True, exist_ok=True) failed_output = final_output.with_name(f"{final_output.name}.failed") with tempfile.TemporaryDirectory( prefix=f".{final_output.name}.staging-", dir=final_output.parent, ) as temporary: staged_output = Path(temporary) / final_output.name staged_args = copy.copy(args) staged_args.output = staged_output result = build_candidate(staged_args) if result != 0: if failed_output.exists(): shutil.rmtree(failed_output) staged_output.replace(failed_output) print( "Technical validation failed; existing package preserved. Inspect " f"{failed_output / 'validation' / 'report.json'}" ) return result previous_output = Path(temporary) / "previous-package" if final_output.exists(): final_output.replace(previous_output) try: staged_output.replace(final_output) except Exception: if previous_output.exists() and not final_output.exists(): previous_output.replace(final_output) raise if failed_output.exists(): shutil.rmtree(failed_output) ``` ### Technical Analysis The `--output` command-line argument is converted to an absolute path with `resolve()`, but it is not constrained to an authorized package-output directory. Resolving the path only normalizes it; it does not establish a security boundary. The function subsequently performs destructive operations on caller-selected paths: - An existing `final_output` i ...[truncated 2721 chars]- Remediation
View remediation
.failed`. - Create uniquely named failure directories, such as `.failed-`. - If stale failure artifacts must be removed, validate their ownership marker first. 5. **Require explicit overwrite authorization** - Fail safely when the output already exists unless an explicit `--overwrite` option is provided. - Even with `--overwrite`, only replace a destination verified as an application-owned package. 6. **Preserve recoverable backups** - Keep replaced packages in a persistent backup location rather than inside a temporary directory that is automatically deleted. - Delete backups only after an explicit retention policy or user confirmation. 7. **Add security regression tests** - Verify that output paths outside the authorized root are rejected. - Verify that symlink-based escapes are rejected. - Verify that existing unmarked directories are never replaced. - Verify that unrelated `.failed` directories are never recursively de ...[truncated 10 chars]