T09 · Insecure Skill Coding Practices
Warning
- Location
- tooling/quality_gate.py:5565
- Finding
- Unvalidated Workspace-Relative Paths Allow Out-of-Scope Local File Reads<![CDATA[ ## Vulnerability Details **File Location**: `tooling/quality_gate.py:5565-5594` **Vulnerability Type**: Path traversal and unrestricted local file access **Risk Level**: Medium ### Vulnerable Code ```python def _check_citation_anchoring(workspace: Path, outputs: list[str]) -> list[QualityIssue]: from tooling.common import read_jsonl draft_rel = outputs[0] if outputs else "output/DRAFT.md" baseline_rel = "output/citation_anchors.prepolish.jsonl" baseline_path = workspace / baseline_rel draft_path = workspace / draft_rel if not baseline_path.exists(): return [] if not draft_path.exists(): return [] baseline_records = [r for r in read_jsonl(baseline_path) if isinstance(r, dict)] baseline_map: dict[str, set[str]] = {} for rec in baseline_records: if str(rec.get("kind") or "").strip() != "h3": continue title = str(rec.get("title") or "").strip() keys = rec.get("cite_keys") or [] if not title or not isinstance(keys, list): continue baseline_map[title] = set(str(k).strip() for k in keys if str(k).strip()) if not baseline_map: return [ QualityIssue( code="citation_anchors_empty", message=f"`{baseline_rel}` exists but has no H3 citation anchors; delete it and rerun `draft-polisher` to regenerate a baseline.", ) ] draft_text = draft_path.read_text(encoding="utf-8", errors="ignore") ``` ### Technical Analysis The first value in `outputs` is treated as a workspace-relative draft path and joined directly with `workspace`. The implementation does not reject: - Absolute paths. - Paths containing `..` components. - Symlinks that resolve outside the workspace. - Other path forms that escape the intended workspace boundary. With `pathlib`, joining a base path to an absolute path discards the base path. For example, `workspace / Path("/tmp/target")` resolves to ...[truncated 1914 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Introduce a centralized workspace-containment helper: ```python def resolve_workspace_path(workspace: Path, relpath: str) -> Path: root = workspace.resolve() supplied = Path(relpath) if supplied.is_absolute(): raise ValueError("Absolute paths are not permitted") candidate = (root / supplied).resolve(strict=False) if not candidate.is_relative_to(root): raise ValueError("Path escapes the workspace") return candidate ``` 2. Replace the vulnerable join with validated resolution: ```python draft_path = resolve_workspace_path(workspace, draft_rel) baseline_path = resolve_workspace_path(workspace, baseline_rel) ``` 3. If the referenced file must already exist, resolve it with `strict=True` and verify containment after symlink resolution. 4. Reject empty paths, null bytes, unexpected file types, and directories. 5. Restrict the anchoring checker to the declared fixed inputs where caller-selected alternatives are not necessary: ```python draft_path = resolve_workspace_path(workspace, "output/DRAFT.md") baseline_path = resolve_workspace_path( workspace, "output/citation_anchors.prepolish.jsonl" ) ``` 6. Apply the same containment validation to every workspace-derived read and write path in `tooling/executor.py`, `tooling/common.py`, and `tooling/quality_gate.py`. 7. Add tests covering absolute paths, `../` traversal, nested traversal, symlink escapes, valid workspace paths, and malformed output entries. ]]>