T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/publish_report.py:376
- Finding
- Repository Path Escape Allows Modification of Files Outside the Target Repository<![CDATA[ ## Vulnerability Details **File Location**: `scripts/publish_report.py`, lines 376–377 and 416–497 **Vulnerability Type**: Insufficient path-boundary validation **Risk Level**: High ### Vulnerable Code ```python repo_dir = Path(args.repo).resolve() source_file = Path(args.report_file).expanduser().resolve() config_path = (repo_dir / args.config_path).resolve() public_dir = config_path.parent ``` The resolved path is subsequently read and used to derive the report destination: ```python records = load_reports_config(config_path) existing_ids = {str(item.get("id", "")).strip() for item in records if isinstance(item, dict)} category_dir_name = sanitize_path_segment(args.category_dir or args.category) dest_dir = public_dir / category_dir_name target_path = pick_unique_destination(dest_dir, source_file, overwrite=args.overwrite) rel_asset = target_path.relative_to(public_dir).as_posix() ``` Both the selected JSON file and the derived destination are modified before repository-relative validation occurs: ```python dest_dir.mkdir(parents=True, exist_ok=True) if not target_path.exists() or file_sha256(target_path) != file_sha256(source_file): shutil.copy2(source_file, target_path) records.insert(0, entry) write_reports_config(config_path, records) if not args.skip_build: print("[INFO] Running npm run build ...") run_cmd(["npm", "run", "build"], cwd=repo_dir) # ... rel_config = config_path.relative_to(repo_dir).as_posix() rel_target = target_path.relative_to(repo_dir).as_posix() ``` ### Technical Analysis The `--config-path` argument is documented as repository-relative, but the implementation accepts absolute paths and traversal sequences such as `../../target.json`. Calling `resolve()` normalizes the path but does not ensure that the resulting path remains under `repo_dir`. The parent directory of the escaped configuration path becomes `public_dir`. Consequently, `dest_dir` and `target_path` can also point outside the repository. The ...[truncated 2068 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject escaped configuration paths immediately after resolution and before reading or writing anything: ```python repo_dir = Path(args.repo).resolve() config_path = (repo_dir / args.config_path).resolve() if not config_path.is_relative_to(repo_dir): raise PublishError("--config-path must remain inside the repository.") ``` 2. Prefer removing the configurable path entirely and always use the declared fixed location: ```python config_path = repo_dir / "public" / "reports_config.json" ``` 3. Independently validate all derived paths before filesystem mutation: ```python public_dir = config_path.parent.resolve() dest_dir = (public_dir / category_dir_name).resolve() target_path = (dest_dir / source_file.name).resolve() for path in (public_dir, dest_dir, target_path): if not path.is_relative_to(repo_dir): raise PublishError(f"Path escapes repository: {path}") ``` 4. Perform every path and branch precondition check before copying the report or rewriting the configuration. 5. Write the updated JSON to a temporary file inside the validated repository and atomically replace the original only after validation succeeds. 6. If later operations such as the build or Git branch creation fail, restore the original configuration and remove newly copied files, or use a temporary worktree so publication is transactional. 7. Add tests covering absolute paths, `..` traversal, symlink-based escapes, and escaped paths in both normal and dry-run modes. ]]>