fix(storage): reject path-traversal artifact ids at the path chokepoint

CHANGELOG.md

@@ -96,6 +96,7 @@ All notable changes to vouch are documented here. Format follows
   KB under `eval/fixture-kb/`, and an `eval` workflow gating retrieval changes
   (#226).
 ### Fixed
+- artifact ids that contain a path separator, `..`, a nul byte, or are empty are now rejected at the storage layer, closing a path-traversal hole. `Source.id` is hex-locked, but the other models (`Claim`, `Page`, `Entity`, `Relation`, `Evidence`, `Session`, `Proposal`) carry a bare `id: str`, so a poisoned id — e.g. one smuggled through a bundle body under a safe on-disk filename — could resolve to a path outside `kb_dir` on the next put / update / lifecycle write (or read an arbitrary file on get). The guard sits at the `_yaml` / `_page_path` chokepoint every `_*_path` helper funnels through, mirroring the `bundle._safe_member_path` tar-traversal fix (fixes #149).
 - `parse_since` (the `--since` parser behind `vouch metrics`/`vouch audit`) now raises a clean `MetricsError` for a duration too large to represent (e.g. `--since 1000000000000d`), instead of letting an uncaught `OverflowError` traceback escape — restoring the documented "clean error, not a traceback" contract.
 - `sync_apply` now loads the sync source exactly once and passes the same `_SyncSource` instance into `sync_check`, closing a TOCTOU window where a bundle replaced on disk between the two `_load_source` calls could cause the validation and write phases to operate on different snapshots. Also eliminates redundant directory walks (KB sources) and triple tarball opens (bundle sources). Fixes #217.
 - `vault_to_kb` now passes `slug_hint=page_id` to `propose_page` so vault edit proposals target the existing page id from frontmatter instead of a slugified copy of the title (fixes #219).

src/vouch/storage.py

@@ -144,6 +144,29 @@ def _yaml_load(text: str) -> Any:
     return yaml.safe_load(text)
 
 
+def _reject_unsafe_id(obj_id: str) -> None:
+    """Reject an artifact id that would escape its subdirectory as a path.
+
+    `Source.id` is locked to a hex sha256, but every other artifact model
+    (`Claim`, `Page`, `Entity`, `Relation`, `Evidence`, `Session`,
+    `Proposal`) declares a bare `id: str` with no validator. A poisoned id —
+    e.g. ``"../../tmp/x"`` smuggled through a bundle body that `import_apply`
+    lands under a safe *filename* but whose in-memory id is traversal — would
+    otherwise resolve to a path outside `kb_dir` on the next put / update /
+    lifecycle write (or read arbitrary files on get). This is the single
+    chokepoint every `_*_path` helper funnels through, so guarding it closes
+    all reach paths at once. Mirrors `bundle._unsafe_name_reason` (#149).
+    """
+    if not obj_id:
+        raise ValueError("artifact id must not be empty")
+    if "\x00" in obj_id:
+        raise ValueError(f"artifact id contains a nul byte: {obj_id!r}")
+    if "/" in obj_id or "\\" in obj_id:
+        raise ValueError(f"artifact id must not contain a path separator: {obj_id!r}")
+    if obj_id in (".", "..") or ".." in Path(obj_id).parts:
+        raise ValueError(f"path traversal in artifact id: {obj_id!r}")
+
+
 _FRONTMATTER_RE = re.compile(r"^---\n(.*?)\n---\n?(.*)$", re.DOTALL)
 
 
@@ -232,15 +255,21 @@ def config_path(self) -> Path:
         return self.kb_dir / CONFIG_FILENAME
 
     def _yaml(self, sub: str, obj_id: str) -> Path:
+        _reject_unsafe_id(obj_id)
         return self.kb_dir / sub / f"{obj_id}.yaml"
 
     def _claim_path(self, claim_id: str) -> Path:
         return self._yaml("claims", claim_id)
 
     def _page_path(self, page_id: str) -> Path:
+        _reject_unsafe_id(page_id)
         return self.kb_dir / "pages" / f"{page_id}.md"
 
     def _source_dir(self, source_id: str) -> Path:
+        # Source.id is hex-sha256-locked on the model, but get_source /
+        # read_source_content / the evidence-ref existence checks route raw
+        # id strings through here, so guard this chokepoint too (#149).
+        _reject_unsafe_id(source_id)
         return self.kb_dir / "sources" / source_id
 
     def _entity_path(self, eid: str) -> Path:

tests/test_storage.py

@@ -131,6 +131,15 @@ def test_source_hash_field_mirrors_id(store: KBStore) -> None:
     assert s.hash == s.id
 
 
+def test_get_source_rejects_traversal_id(store: KBStore) -> None:
+    # get_source / read_source_content take raw id strings; a crafted id must
+    # not resolve to <outside>/meta.yaml or <outside>/content (#149).
+    with pytest.raises(ValueError, match="artifact id"):
+        store.get_source("../../../etc/hostname")
+    with pytest.raises(ValueError, match="artifact id"):
+        store.read_source_content("../../outside")
+
+
 # --- claims ---------------------------------------------------------------
 
 
@@ -217,6 +226,68 @@ def test_page_with_frontmatter_round_trip(store: KBStore) -> None:
     assert back.type == PageType.CONCEPT
 
 
+# --- artifact id path-traversal guard (#149) ------------------------------
+
+
+@pytest.mark.parametrize(
+    "bad_id",
+    ["../escape", "../../etc/passwd", "a/b", "sub\\evil", "/abs", "..", ""],
+)
+def test_yaml_path_rejects_unsafe_id(store: KBStore, bad_id: str) -> None:
+    with pytest.raises(ValueError, match="artifact id"):
+        store._yaml("claims", bad_id)
+
+
+@pytest.mark.parametrize("bad_id", ["../../pwned", "a/b", "/abs"])
+def test_page_path_rejects_unsafe_id(store: KBStore, bad_id: str) -> None:
+    with pytest.raises(ValueError, match="artifact id"):
+        store._page_path(bad_id)
+
+
+def test_put_claim_rejects_traversal_id(store: KBStore) -> None:
+    src = store.put_source(b"e")
+    with pytest.raises(ValueError, match="artifact id"):
+        store.put_claim(Claim(id="../../../tmp/pwned", text="x", evidence=[src.id]))
+
+
+def test_get_claim_rejects_traversal_id(store: KBStore) -> None:
+    # A crafted id on a read path must not resolve to an arbitrary file.
+    with pytest.raises(ValueError, match="artifact id"):
+        store.get_claim("../../../etc/hostname")
+
+
+def test_put_page_rejects_traversal_id(store: KBStore) -> None:
+    with pytest.raises(ValueError, match="artifact id"):
+        store.put_page(Page(id="../../../tmp/pwned", title="x"))
+
+
+def test_update_claim_rejects_disk_smuggled_traversal_id(store: KBStore) -> None:
+    """The bundle exploit: a claim lands under a *safe* filename but carries a
+    traversal string in its `id` field (models don't validate non-Source ids).
+    Reading it is fine; the next write must refuse to escape kb_dir."""
+    src = store.put_source(b"e")
+    planted = Claim(id="innocent", text="looks fine", evidence=[src.id])
+    on_disk = {**planted.model_dump(mode="json"), "id": "../../../tmp/pwned"}
+    claim_file = store.kb_dir / "claims" / "innocent.yaml"
+    claim_file.write_text(_yaml_dump(on_disk))
+
+    c = store.get_claim("innocent")
+    assert c.id == "../../../tmp/pwned"  # in-memory id is traversal
+
+    with pytest.raises(ValueError, match="artifact id"):
+        store.update_claim(c)
+    # nothing escaped, and the planted file is untouched by the refused write.
+    assert claim_file.read_text() == _yaml_dump(on_disk)
+
+
+def test_legitimate_ids_still_resolve(store: KBStore) -> None:
+    """Regression guard: normal slug / hex ids must keep working."""
+    for sub, cid in [("claims", "auth-uses-jwt"), ("entities", "proj_foo"),
+                     ("sessions", "a1b2c3d4")]:
+        assert store._yaml(sub, cid) == store.kb_dir / sub / f"{cid}.yaml"
+    assert store._page_path("overview") == store.kb_dir / "pages" / "overview.md"
+
+
 # --- entities + relations -------------------------------------------------