T09 · Insecure Skill Coding Practices
- Location
scripts/run.py:90- Finding
Workspace Path Traversal Enables Out-of-Scope File Reads and Writes
- Content
View full analysis
= 3 else "GOAL.md") out_path = workspace / outputs[0] ``` The constructed output path is later used without a workspace-containment check: ```python ensure_dir(out_path.parent) _check_no_placeholders(records) write_jsonl(out_path, records) ``` The underlying write operation atomically replaces an existing destination: ```python def atomic_write_text(path: Path, content: str) -> None: ensure_dir(path.parent) fd, tmp_path = tempfile.mkstemp(prefix=path.name, dir=str(path.parent)) try: with os.fdopen(fd, "w", encoding="utf-8") as handle: handle.write(content) os.replace(tmp_path, path) ``` ### Technical Analysis The `--inputs` and `--outputs` arguments are accepted as arbitrary semicolon-separated paths. These values are joined to the workspace using the `/` operator but are not validated, canonicalized, or checked for containment within the workspace. An absolute `Path` operand causes the workspace prefix to be discarded. Relative paths containing `../` can also resolve outside the workspace. Consequently, the program can access any path permitted by the operating-system identity running the Skill. The output path is particularly significant because `write_jsonl()` eventually calls `os.replace()`. This can replace an existing writable file without prompting the user. A symlinked directory within the workspace may provide another rou ...[truncated 2280 chars]- Remediation
View remediation
Path: relative = Path(value) if relative.is_absolute(): raise ValueError(f"Absolute path is not allowed: {value}") root = workspace.resolve(strict=True) candidate = (root / relative).resolve(strict=False) try: candidate.relative_to(root) except ValueError as exc: raise ValueError(f"Path escapes workspace: {value}") from exc return candidate ``` 3. Apply this validation independently to every input and output path before any read, directory creation, or write. 4. Use an explicit allowlist for expected inputs and outputs rather than accepting arbitrary filesystem locations. 5. Before writing, validate existing parent directories with `resolve()` and reject symlink-based escapes. Where supported, use descriptor-relative operations and no-follow semantics to reduce time-of-check/time-of-use risks. 6. Refuse to overwrite an existing output unless it is the documented output artifact or the user has explicitly enabled an overwrite option. 7. Add tests covering: - Absolute Unix and Windows paths. - `../` traversal. - Nested traversal after normalization. - Symlinked workspace directories pointing outside the workspace. - Existing out-of-workspace targets. - Valid documented workspace-relative paths. ]]>
