diff --git a/CHANGELOG.md b/CHANGELOG.md index dccec54f..430f1e7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ All notable changes to vouch are documented here. Format follows or dry-running pending proposals without bypassing the review gate. ### Fixed +- `store.put_relation`, `store.put_relation_idempotent`, and `store.put_page` now reject artifacts whose foreign-id references don't resolve in the KB (relation `source` / `target` / `evidence`; page `entities` / `sources`). `proposals.propose_relation` and `proposals.propose_page` surface the same checks at proposal time as `ProposalError`. `bundle.import_check` and `sync.sync_check` run an equivalent cross-artifact pass against the post-merge id set so manifest-consistent bundles can't smuggle relations / pages whose references resolve to nothing — closes the write-time counterpart of the after-the-fact `dangling_relation` finding in `health.lint` (`src/vouch/health.py:135-145`). - `bundle.import_check` / `import_apply` and `sync.sync_check` / `sync_apply` now enforce the Source content-addressing invariant: a `sources//content` member must hash to ``, and a `sources//meta.yaml` must carry a matching `id`/`hash`. Previously the import side trusted the bundle's directory layout, so a manifest-consistent bundle could land a Source whose content did not match its claimed id — `verify_source` would report `stored_ok=False` only after the import had already succeeded with a clean `bundle.import` audit event. The per-file sha256 gate (#74) only proves bytes match the manifest; this closes the write-time counterpart of the `verify.verify_source` detection. - Add `put_relation_idempotent()` to `KBStore` and use it in `supersede()` and `contradict()` so retrying after a partial failure converges to a consistent state instead of raising `ValueError`. - Raise `ProposalError("forbidden_self_approval")` in `proposals.approve()` when `approved_by == proposal.proposed_by`, enforcing the review-gate guarantee documented in the README and CONTRIBUTING. diff --git a/src/vouch/bundle.py b/src/vouch/bundle.py index f57b64d1..c579c67d 100644 --- a/src/vouch/bundle.py +++ b/src/vouch/bundle.py @@ -248,6 +248,157 @@ def _validate_content(path: str, data: bytes, issues: list[str]) -> None: issues.append(f"schema validation failed: {path}: {e}") +def _artifact_id_from_path(path: str) -> tuple[str, str] | None: + """Return (kind, id) for a manifest path, or None for unrelated files. + + Mirrors `sync._artifact_kind` semantics but only emits the entries the + referential pass needs. `sources//...` collapses to ("source", + "") regardless of whether the path is meta.yaml or content. + """ + parts = path.split("/") + if len(parts) < 2: + return None + top = parts[0] + if top == "sources" and len(parts) >= 3: + return "source", parts[1] + singular = { + "claims": "claim", + "pages": "page", + "entities": "entity", + "relations": "relation", + "evidence": "evidence", + }.get(top) + if singular is None: + return None + return singular, Path(parts[-1]).stem + + +def _existing_ids(kb_dir: Path) -> dict[str, set[str]]: + """Snapshot the destination KB's artifact ids per kind. + + Used by the post-merge referential pass: an incoming relation / + page reference is satisfied if the target id is either already on + disk OR is being delivered by this same bundle. + """ + out: dict[str, set[str]] = { + "claim": set(), "page": set(), "entity": set(), + "source": set(), "evidence": set(), + } + for sub, kind, suffix in ( + ("claims", "claim", ".yaml"), + ("entities", "entity", ".yaml"), + ("relations", "relation", ".yaml"), + ("evidence", "evidence", ".yaml"), + ): + d = kb_dir / sub + if not d.is_dir(): + continue + for p in d.glob(f"*{suffix}"): + out[kind].add(p.stem) + pages_dir = kb_dir / "pages" + if pages_dir.is_dir(): + for p in pages_dir.glob("*.md"): + out["page"].add(p.stem) + sources_dir = kb_dir / "sources" + if sources_dir.is_dir(): + for sdir in sources_dir.iterdir(): + if (sdir / "meta.yaml").exists(): + out["source"].add(sdir.name) + return out + + +def _check_graph_integrity( + kb_dir: Path, + incoming: dict[str, bytes], + issues: list[str], +) -> None: + """Verify every Relation/Page/Claim in `incoming` resolves its refs. + + Closes the bundle / sync equivalent of `put_relation` + `put_page` + validation. References are satisfied against the post-merge id set + (destination KB ids plus incoming bundle ids), so a self-contained + bundle that ships an entity alongside the relation that points at + it still imports cleanly. Skips files whose bytes already failed + schema validation upstream so a malformed file does not produce a + second cryptic error. + """ + ids = _existing_ids(kb_dir) + incoming_meta: list[tuple[str, str, bytes]] = [] + for path, body in incoming.items(): + kind_id = _artifact_id_from_path(path) + if kind_id is None: + continue + kind, aid = kind_id + # Only artifacts that can satisfy a reference go into `ids`. + # `relation` is intentionally absent — relations aren't valid + # Relation endpoints. The incoming relation itself is still + # appended below so its refs get checked. + if kind in ids: + ids[kind].add(aid) + incoming_meta.append((path, kind, body)) + node_kinds = ("claim", "page", "entity", "source") + evidence_kinds = ("source", "evidence") + failed_schema = { + i.split(": ", 2)[1] for i in issues + if i.startswith("schema validation failed: ") + } + for path, kind, body in incoming_meta: + if path in failed_schema: + continue + try: + if kind == "relation": + rel = Relation.model_validate(yaml.safe_load(body)) + if not any(rel.source in ids[k] for k in node_kinds): + issues.append( + f"dangling reference: {path}: relation source " + f"{rel.source!r} not in bundle or destination" + ) + if not any(rel.target in ids[k] for k in node_kinds): + issues.append( + f"dangling reference: {path}: relation target " + f"{rel.target!r} not in bundle or destination" + ) + for eid in rel.evidence: + if not any(eid in ids[k] for k in evidence_kinds): + issues.append( + f"dangling reference: {path}: relation " + f"evidence {eid!r} not in bundle or destination" + ) + elif kind == "page": + page = _deserialize_page(body.decode()) + for cid in page.claims: + if cid not in ids["claim"]: + issues.append( + f"dangling reference: {path}: page claim " + f"{cid!r} not in bundle or destination" + ) + for eid in page.entities: + if eid not in ids["entity"]: + issues.append( + f"dangling reference: {path}: page entity " + f"{eid!r} not in bundle or destination" + ) + for sid in page.sources: + if sid not in ids["source"]: + issues.append( + f"dangling reference: {path}: page source " + f"{sid!r} not in bundle or destination" + ) + elif kind == "claim": + claim = Claim.model_validate(yaml.safe_load(body)) + for ref in claim.evidence: + if not any(ref in ids[k] for k in evidence_kinds): + issues.append( + f"dangling reference: {path}: claim citation " + f"{ref!r} not in bundle or destination" + ) + except Exception: + # Schema validation already ran on `body` in `_validate_content` + # and recorded any structural issue. Swallow here so a single + # malformed file doesn't mask the more useful upstream error. + continue + + def _check_source_content_address(path: str, body: bytes, issues: list[str]) -> None: """Enforce the Source content-addressing invariant on import. @@ -304,6 +455,7 @@ def import_check(kb_dir: Path, bundle_path: Path) -> ImportCheckResult: identical: list[str] = [] issues: list[str] = [] bundle_id = "" + incoming: dict[str, bytes] = {} with tarfile.open(bundle_path, "r:gz") as tar: try: @@ -343,11 +495,20 @@ def import_check(kb_dir: Path, bundle_path: Path) -> ImportCheckResult: continue _validate_content(member.name, body, issues) _check_source_content_address(member.name, body, issues) + incoming[member.name] = body for path in manifest_paths: try: tar.getmember(path) except KeyError: issues.append(f"manifest lists missing file: {path}") + # Graph-integrity pass: the storage layer's put_relation / put_page + # validators run on direct writes, but import_apply writes member + # bytes straight to disk, so the only place to enforce referential + # integrity for a bundle is here. Refs are resolved against the + # post-merge id set (destination KB plus incoming bundle), so a + # self-contained bundle that ships an entity alongside the relation + # pointing at it still imports cleanly. + _check_graph_integrity(kb_dir, incoming, issues) return ImportCheckResult( ok=not issues, diff --git a/src/vouch/proposals.py b/src/vouch/proposals.py index 051f2abb..df2ce154 100644 --- a/src/vouch/proposals.py +++ b/src/vouch/proposals.py @@ -147,6 +147,25 @@ def propose_page( ) -> Proposal: if not title.strip(): raise ProposalError("page title is empty") + # Mirror the existence check `propose_claim` already runs on evidence + # ids: a page that lists a claim / entity / source id but never had it + # resolved is exactly the dangling-reference shape `store.put_page` + # used to silently accept (issue: graph-integrity write gates). + for cid in claim_ids or []: + try: + store.get_claim(cid) + except ArtifactNotFoundError as e: + raise ProposalError(f"unknown claim id: {cid}") from e + for eid in entity_ids or []: + try: + store.get_entity(eid) + except ArtifactNotFoundError as e: + raise ProposalError(f"unknown entity id: {eid}") from e + for sid in source_ids or []: + try: + store.get_source(sid) + except ArtifactNotFoundError as e: + raise ProposalError(f"unknown source id: {sid}") from e payload = { "id": slug_hint or _slugify(title), "title": title.strip(), @@ -208,6 +227,31 @@ def propose_relation( ) -> Proposal: if not src or not target or not relation: raise ProposalError("relation needs src, relation, target") + # Endpoint + evidence existence checks mirror the `propose_claim` + # citation loop. The corresponding write-time gate now lives in + # `store.put_relation` / `store.put_relation_idempotent`; surfacing + # the same error here means the agent sees a friendly `ProposalError` + # at proposal time instead of a downstream `ValueError` at approve. + if not _node_exists(store, src): + raise ProposalError( + f"unknown relation source endpoint: {src} (must be an existing " + f"claim, page, entity, or source id)" + ) + if not _node_exists(store, target): + raise ProposalError( + f"unknown relation target endpoint: {target} (must be an " + f"existing claim, page, entity, or source id)" + ) + for eid in evidence or []: + try: + store.get_source(eid) + except ArtifactNotFoundError: + try: + store.get_evidence(eid) + except ArtifactNotFoundError as e: + raise ProposalError( + f"unknown source/evidence id: {eid}" + ) from e rid = f"{src}--{relation}--{target}" payload = { "id": _slugify(rid), @@ -482,6 +526,29 @@ def _ensure_no_existing_artifact( ) +def _node_exists(store: KBStore, node_id: str) -> bool: + """True if `node_id` resolves to a Claim, Page, Entity, or Source. + + The set of valid Relation endpoint kinds; mirrors + `KBStore._node_exists` (storage.py) so propose-time and write-time + rejection use the same definition. + """ + if not node_id: + return False + for getter in ( + store.get_claim, + store.get_page, + store.get_entity, + store.get_source, + ): + try: + getter(node_id) + return True + except ArtifactNotFoundError: + continue + return False + + def _slugify(text: str) -> str: out = [] last_dash = False diff --git a/src/vouch/storage.py b/src/vouch/storage.py index e3c3e270..4f625a3d 100644 --- a/src/vouch/storage.py +++ b/src/vouch/storage.py @@ -308,6 +308,40 @@ def list_sources(self) -> list[Source]: out.append(Source.model_validate(_yaml_load(meta.read_text()))) return out + # --- graph-integrity helpers ------------------------------------------ + + # Closes the structural counterpart of the #81 fix: every graph artifact + # (Relation source/target/evidence, Page entities/sources) must resolve + # to a known artifact in the KB before it lands on disk. The invariant + # was already articulated as a `dangling_relation` finding in + # `health.lint` (`src/vouch/health.py:135-145`) but no write path + # enforced it, so every approve / lifecycle / bundle / sync surface + # silently landed graph edges and pages pointing at nothing. + + def _node_exists(self, node_id: str) -> bool: + """True if `node_id` resolves to a Claim, Page, Entity, or Source.""" + if not node_id: + return False + if self._claim_path(node_id).exists(): + return True + if self._page_path(node_id).exists(): + return True + if self._entity_path(node_id).exists(): + return True + return (self._source_dir(node_id) / "meta.yaml").exists() + + def _evidence_ref_exists(self, ref_id: str) -> bool: + """True if `ref_id` resolves to a Source or Evidence record. + + Matches the citation surface that `put_claim` already accepts: + either a content-hash Source id or an Evidence id. + """ + if not ref_id: + return False + if (self._source_dir(ref_id) / "meta.yaml").exists(): + return True + return self._evidence_path(ref_id).exists() + # --- claims ------------------------------------------------------------ def put_claim(self, claim: Claim) -> Claim: @@ -382,6 +416,12 @@ def put_page(self, page: Page) -> Page: for cid in page.claims: if not self._claim_path(cid).exists(): raise ValueError(f"page {page.id} references unknown claim {cid}") + for eid in page.entities: + if not self._entity_path(eid).exists(): + raise ValueError(f"page {page.id} references unknown entity {eid}") + for sid in page.sources: + if not (self._source_dir(sid) / "meta.yaml").exists(): + raise ValueError(f"page {page.id} references unknown source {sid}") try: with self._page_path(page.id).open("x") as f: f.write(_serialize_page(page)) @@ -435,7 +475,27 @@ def list_entities(self) -> list[Entity]: # --- relations --------------------------------------------------------- + def _validate_relation_refs(self, rel: Relation) -> None: + if not self._node_exists(rel.source): + raise ValueError( + f"relation {rel.id} references unknown source endpoint " + f"{rel.source!r} (must be an existing claim, page, entity, " + f"or source id)" + ) + if not self._node_exists(rel.target): + raise ValueError( + f"relation {rel.id} references unknown target endpoint " + f"{rel.target!r} (must be an existing claim, page, entity, " + f"or source id)" + ) + for eid in rel.evidence: + if not self._evidence_ref_exists(eid): + raise ValueError( + f"relation {rel.id} cites unknown source/evidence {eid!r}" + ) + def put_relation(self, rel: Relation) -> Relation: + self._validate_relation_refs(rel) try: with self._relation_path(rel.id).open("x") as f: f.write(_yaml_dump(rel.model_dump(mode="json"))) @@ -463,6 +523,12 @@ def put_relation_idempotent(self, rel: Relation) -> Relation: text=f"{rel.source} {rel.relation.value} {rel.target}", ) return rel + # Validate before the exclusive create. Skipping validation for the + # "already on disk" branch above is deliberate — a relation that's + # already durable was validated when it landed; re-checking would + # turn supersede/contradict retries into spurious failures whenever + # the linked claim was subsequently archived or retracted. + self._validate_relation_refs(rel) try: with path.open("x") as f: f.write(_yaml_dump(rel.model_dump(mode="json"))) diff --git a/src/vouch/sync.py b/src/vouch/sync.py index 8af93aea..350fc877 100644 --- a/src/vouch/sync.py +++ b/src/vouch/sync.py @@ -188,11 +188,14 @@ def _read_source_file(src: _SyncSource, path: str) -> bytes: return tar.extractfile(tar.getmember(path)).read() # type: ignore[union-attr] -def _validation_issues_for_source(src: _SyncSource) -> list[str]: +def _validation_issues_for_source( + src: _SyncSource, kb_dir: Path | None = None +) -> list[str]: issues: list[str] = [] if src.bundle_path is not None: check = bundle.export_check(src.bundle_path) issues.extend(check.issues) + incoming_bodies: dict[str, bytes] = {} for path, incoming in sorted(src.files.items()): reason = bundle._unsafe_name_reason(path) if reason is not None: @@ -208,13 +211,19 @@ def _validation_issues_for_source(src: _SyncSource) -> list[str]: continue bundle._validate_content(path, data, issues) bundle._check_source_content_address(path, data, issues) + incoming_bodies[path] = data + # Graph-integrity pass mirrors the bundle import path so sync + # doesn't accept Relations / Pages whose endpoints / references + # neither exist locally nor arrive in the same sync. + if kb_dir is not None: + bundle._check_graph_integrity(kb_dir, incoming_bodies, issues) return issues def sync_check(kb_dir: Path, source_path: Path) -> SyncCheckResult: """Compare another KB or bundle with ``kb_dir`` without writing.""" src = _load_source(source_path) - issues = _validation_issues_for_source(src) + issues = _validation_issues_for_source(src, kb_dir=kb_dir) new_files: list[str] = [] identical: list[str] = [] conflicts: list[SyncConflict] = [] diff --git a/tests/test_bundle.py b/tests/test_bundle.py index e93e6afe..51ae5d3e 100644 --- a/tests/test_bundle.py +++ b/tests/test_bundle.py @@ -411,6 +411,49 @@ def test_import_check_passes_when_member_matches_manifest( assert not any("hash mismatch" in i for i in diff.issues), diff.issues +# --- graph integrity on the bundle path ---------------------------------- +# +# `import_apply` writes member bytes directly to disk and never goes +# through `put_relation` / `put_page`, so the storage-layer validators +# can't reach this surface. `_check_graph_integrity` is the bundle's +# equivalent gate: every Relation / Page reference must resolve against +# the post-merge id set (destination KB plus incoming bundle). + + +def _write_multi_member_bundle( + bundle_path: Path, members: dict[str, bytes] +) -> None: + """Build a manifest-consistent bundle with the given member bodies.""" + files = [ + { + "path": name, + "size": len(body), + "sha256": hashlib.sha256(body).hexdigest(), + } + for name, body in members.items() + ] + bundle_id = hashlib.sha256( + b"".join(sorted(f["sha256"].encode() for f in files)) + ).hexdigest() + manifest = { + "spec": bundle.SPEC_VERSION, + "bundle_id": bundle_id, + "files": files, + "counts": {}, + "safety": {"has_proposed": False, "has_state_db": False, + "has_audit_log": False}, + } + with tarfile.open(bundle_path, "w:gz") as tar: + for name, body in members.items(): + info = tarfile.TarInfo(name) + info.size = len(body) + tar.addfile(info, io.BytesIO(body)) + mf_bytes = json.dumps(manifest).encode() + mf_info = tarfile.TarInfo(bundle.MANIFEST_NAME) + mf_info.size = len(mf_bytes) + tar.addfile(mf_info, io.BytesIO(mf_bytes)) + + # --- source content-addressing on the bundle path ------------------------ # # A Source's id is the sha256 of its content (README: "content-addressed @@ -472,6 +515,138 @@ def _write_source_bundle( tar.addfile(mf_info, io.BytesIO(mf_bytes)) +def _relation_yaml(rid: str, source: str, target: str, evidence: list[str]) -> bytes: + import yaml as _yaml + return _yaml.safe_dump({ + "id": rid, "source": source, "relation": "uses", + "target": target, "confidence": 0.7, "evidence": evidence, + "created_at": "2026-05-27T00:00:00+00:00", + "updated_at": "2026-05-27T00:00:00+00:00", + }, sort_keys=False).encode() + + +def _page_md(pid: str, entities: list[str], sources: list[str]) -> bytes: + import yaml as _yaml + meta = { + "id": pid, "title": pid, "type": "concept", "status": "draft", + "claims": [], "entities": entities, "sources": sources, "tags": [], + "created_at": "2026-05-27T00:00:00+00:00", + "updated_at": "2026-05-27T00:00:00+00:00", + } + return f"---\n{_yaml.safe_dump(meta, sort_keys=False)}---\nbody".encode() + + +def test_import_check_rejects_relation_with_dangling_endpoints( + store: KBStore, tmp_path: Path +) -> None: + """A bundle that ships a relation whose source / target are absent + from both the bundle and the destination is rejected — the bundle + integrity story extends from per-file sha256 (#74) to referential + integrity of the graph it carries.""" + bundle_path = tmp_path / "evil-rel.tar.gz" + _write_multi_member_bundle(bundle_path, { + "relations/r-dangling.yaml": _relation_yaml( + "r-dangling", "ghost-source", "ghost-target", [], + ), + }) + + diff = bundle.import_check(store.kb_dir, bundle_path) + assert not diff.ok + assert any("dangling reference" in i and "source" in i for i in diff.issues) + assert any("dangling reference" in i and "target" in i for i in diff.issues) + + +def test_import_apply_refuses_dangling_relation_endpoints( + store: KBStore, tmp_path: Path +) -> None: + bundle_path = tmp_path / "evil-rel.tar.gz" + _write_multi_member_bundle(bundle_path, { + "relations/r-dangling.yaml": _relation_yaml( + "r-dangling", "ghost", "ghost2", [], + ), + }) + + with pytest.raises(RuntimeError, match="dangling reference"): + bundle.import_apply(store.kb_dir, bundle_path) + assert not (store.kb_dir / "relations" / "r-dangling.yaml").exists() + + +def test_import_check_rejects_page_with_dangling_refs( + store: KBStore, tmp_path: Path +) -> None: + bundle_path = tmp_path / "evil-page.tar.gz" + _write_multi_member_bundle(bundle_path, { + "pages/evil-page.md": _page_md( + "evil-page", + entities=["ghost-entity"], + sources=["ghost-source"], + ), + }) + + diff = bundle.import_check(store.kb_dir, bundle_path) + assert not diff.ok + assert any("dangling reference" in i and "page entity" in i for i in diff.issues) + assert any("dangling reference" in i and "page source" in i for i in diff.issues) + + with pytest.raises(RuntimeError, match="dangling reference"): + bundle.import_apply(store.kb_dir, bundle_path) + assert not (store.kb_dir / "pages" / "evil-page.md").exists() + + +def test_import_check_accepts_self_contained_bundle( + store: KBStore, tmp_path: Path +) -> None: + """A bundle that ships an entity alongside the relation that points + at it imports cleanly. The post-merge id set is the union of + destination + incoming, so a self-contained bundle does not get + penalised for not having its dependencies already on disk.""" + import yaml as _yaml + ent_yaml = _yaml.safe_dump({ + "id": "ent-x", "name": "X", "type": "project", + "aliases": [], "description": None, "page": None, + "created_at": "2026-05-27T00:00:00+00:00", + "updated_at": "2026-05-27T00:00:00+00:00", + }, sort_keys=False).encode() + ent2_yaml = _yaml.safe_dump({ + "id": "ent-y", "name": "Y", "type": "project", + "aliases": [], "description": None, "page": None, + "created_at": "2026-05-27T00:00:00+00:00", + "updated_at": "2026-05-27T00:00:00+00:00", + }, sort_keys=False).encode() + bundle_path = tmp_path / "self-contained.tar.gz" + _write_multi_member_bundle(bundle_path, { + "entities/ent-x.yaml": ent_yaml, + "entities/ent-y.yaml": ent2_yaml, + "relations/ent-x--uses--ent-y.yaml": _relation_yaml( + "ent-x--uses--ent-y", "ent-x", "ent-y", [], + ), + }) + + diff = bundle.import_check(store.kb_dir, bundle_path) + assert diff.ok, diff.issues + result = bundle.import_apply(store.kb_dir, bundle_path) + assert "relations/ent-x--uses--ent-y.yaml" in result["written"] + + +def test_import_check_resolves_refs_against_destination_kb( + store: KBStore, tmp_path: Path +) -> None: + """If the destination already has the entity, a bundle shipping just + the relation imports cleanly.""" + from vouch.models import Entity, EntityType + store.put_entity(Entity(id="local-a", name="A", type=EntityType.PROJECT)) + store.put_entity(Entity(id="local-b", name="B", type=EntityType.PROJECT)) + bundle_path = tmp_path / "rel-only.tar.gz" + _write_multi_member_bundle(bundle_path, { + "relations/local-a--uses--local-b.yaml": _relation_yaml( + "local-a--uses--local-b", "local-a", "local-b", [], + ), + }) + + diff = bundle.import_check(store.kb_dir, bundle_path) + assert diff.ok, diff.issues + + def test_import_rejects_source_content_address_mismatch( store: KBStore, tmp_path: Path ) -> None: diff --git a/tests/test_health.py b/tests/test_health.py index 2ab9b30b..817499cb 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -7,7 +7,7 @@ import pytest from vouch import health, index_db -from vouch.models import Claim, ClaimStatus, Proposal, ProposalKind, ProposalStatus, Relation +from vouch.models import Claim, ClaimStatus, Proposal, ProposalKind, ProposalStatus from vouch.storage import KBStore @@ -29,8 +29,23 @@ def test_lint_finds_broken_citation_when_source_removed(store: KBStore) -> None: def test_lint_dangling_relation(store: KBStore) -> None: src = store.put_source(b"e") store.put_claim(Claim(id="c1", text="t", evidence=[src.id])) - store.put_relation(Relation(id="rel-x", source="c1", - relation="uses", target="ghost")) + # Hand-write the YAML to simulate a relation that landed before + # `put_relation` enforced endpoint existence (or one introduced + # by a manual edit). `health.lint` is the after-the-fact safety + # net for exactly this case; the on-disk YAML is still a + # `dangling_relation` finding even though no current write path + # would land it. + dangling_yaml = ( + "id: rel-x\n" + "source: c1\n" + "relation: uses\n" + "target: ghost\n" + "confidence: 0.7\n" + "evidence: []\n" + "created_at: '2026-05-27T00:00:00+00:00'\n" + "updated_at: '2026-05-27T00:00:00+00:00'\n" + ) + (store.kb_dir / "relations" / "rel-x.yaml").write_text(dangling_yaml) report = health.lint(store) codes = {f.code for f in report.findings} assert "dangling_relation" in codes diff --git a/tests/test_storage.py b/tests/test_storage.py index 390ac90b..cd8dcff2 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -232,6 +232,117 @@ def test_relation_round_trip(store: KBStore) -> None: assert [r.id for r in store.relations_to("b")] == ["a-uses-b"] +# --- graph integrity: relation / page write paths reject dangling refs --- +# +# The `dangling_relation` shape used to be reported by `health.lint` after +# the fact (`src/vouch/health.py:135-145`) but no writer enforced it, so +# every approve / lifecycle / bundle path silently landed broken edges. +# These regressions cover the structural counterpart of the #81 fix on +# the Relation + Page surface. + + +def test_put_relation_rejects_unknown_source_endpoint(store: KBStore) -> None: + store.put_entity(Entity(id="real-target", name="T", type=EntityType.PROJECT)) + rel = Relation(id="r1", source="ghost", relation=RelationType.USES, target="real-target") + with pytest.raises(ValueError, match="unknown source endpoint"): + store.put_relation(rel) + assert not (store.kb_dir / "relations" / "r1.yaml").exists() + + +def test_put_relation_rejects_unknown_target_endpoint(store: KBStore) -> None: + store.put_entity(Entity(id="real-src", name="S", type=EntityType.PROJECT)) + rel = Relation(id="r2", source="real-src", relation=RelationType.USES, target="ghost") + with pytest.raises(ValueError, match="unknown target endpoint"): + store.put_relation(rel) + assert not (store.kb_dir / "relations" / "r2.yaml").exists() + + +def test_put_relation_rejects_unknown_evidence_ref(store: KBStore) -> None: + store.put_entity(Entity(id="x", name="X", type=EntityType.PROJECT)) + store.put_entity(Entity(id="y", name="Y", type=EntityType.PROJECT)) + rel = Relation( + id="r3", source="x", relation=RelationType.USES, target="y", + evidence=["ghost-source-or-evidence"], + ) + with pytest.raises(ValueError, match="unknown source/evidence"): + store.put_relation(rel) + assert not (store.kb_dir / "relations" / "r3.yaml").exists() + + +def test_put_relation_accepts_source_id_as_endpoint(store: KBStore) -> None: + # The Relation endpoint surface is documented in the README §"Object + # model" as 'entities / claims / pages'; in practice Source ids are + # also valid graph nodes (sources back claims and crystallize into + # pages). Keep that surface explicit here. + src = store.put_source(b"source-as-node", title="t") + rel = Relation( + id=f"r-{src.id[:8]}", + source=src.id, relation=RelationType.REFERENCES, target=src.id, + evidence=[src.id], + ) + store.put_relation(rel) + assert store.get_relation(rel.id).source == src.id + + +def test_put_relation_idempotent_validates_endpoints_on_first_write( + store: KBStore, +) -> None: + # Lifecycle ops (supersede / contradict) reach `put_relation_idempotent` + # with both endpoints loaded via `get_claim`, so they always satisfy + # the new gate. The negative case here proves a hand-built call with + # a dangling endpoint is still rejected. + store.put_entity(Entity(id="ok", name="OK", type=EntityType.PROJECT)) + rel = Relation(id="r-id", source="ok", relation=RelationType.USES, target="ghost") + with pytest.raises(ValueError, match="unknown target endpoint"): + store.put_relation_idempotent(rel) + assert not (store.kb_dir / "relations" / "r-id.yaml").exists() + + +def test_put_relation_idempotent_skips_revalidation_on_existing_file( + store: KBStore, tmp_path: Path, +) -> None: + # If a relation already exists on disk, `put_relation_idempotent` + # converges without re-checking endpoints. This protects supersede / + # contradict retries from spurious failures if the linked claim was + # later archived or retracted. + store.put_entity(Entity(id="p", name="P", type=EntityType.PROJECT)) + store.put_entity(Entity(id="q", name="Q", type=EntityType.PROJECT)) + rel = Relation(id="r-pq", source="p", relation=RelationType.USES, target="q") + store.put_relation(rel) + # Now "remove" target from the KB. + (store.kb_dir / "entities" / "q.yaml").unlink() + # Repeat call must not raise even though target no longer exists. + store.put_relation_idempotent(rel) + + +def test_put_page_rejects_unknown_entity_ref(store: KBStore) -> None: + page = Page(id="p-bad-ent", title="t", body="b", entities=["ghost-entity"]) + with pytest.raises(ValueError, match="unknown entity"): + store.put_page(page) + assert not (store.kb_dir / "pages" / "p-bad-ent.md").exists() + + +def test_put_page_rejects_unknown_source_ref(store: KBStore) -> None: + page = Page(id="p-bad-src", title="t", body="b", sources=["ghost-src"]) + with pytest.raises(ValueError, match="unknown source"): + store.put_page(page) + assert not (store.kb_dir / "pages" / "p-bad-src.md").exists() + + +def test_put_page_accepts_resolvable_entity_and_source_refs( + store: KBStore, +) -> None: + src = store.put_source(b"hello", title="hi") + store.put_entity(Entity(id="ent1", name="E", type=EntityType.CONCEPT)) + page = Page( + id="p-ok", title="t", body="b", + entities=["ent1"], sources=[src.id], + ) + store.put_page(page) + assert store.get_page("p-ok").entities == ["ent1"] + assert store.get_page("p-ok").sources == [src.id] + + # --- evidence ------------------------------------------------------------- @@ -363,6 +474,37 @@ def test_propose_relation_then_approve(store: KBStore) -> None: assert r.relation == RelationType.CONTRADICTS +def test_propose_relation_rejects_unknown_source(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c-real", text="t", evidence=[src.id])) + with pytest.raises(ProposalError, match="unknown relation source endpoint"): + propose_relation( + store, src="ghost", relation="uses", target="c-real", + proposed_by="a", + ) + + +def test_propose_relation_rejects_unknown_target(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c-real", text="t", evidence=[src.id])) + with pytest.raises(ProposalError, match="unknown relation target endpoint"): + propose_relation( + store, src="c-real", relation="uses", target="ghost", + proposed_by="a", + ) + + +def test_propose_relation_rejects_unknown_evidence_ref(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="t", evidence=[src.id])) + store.put_claim(Claim(id="c2", text="t2", evidence=[src.id])) + with pytest.raises(ProposalError, match="unknown source/evidence"): + propose_relation( + store, src="c1", relation="contradicts", target="c2", + evidence=["nonexistent"], proposed_by="a", + ) + + def test_propose_page_round_trip_through_approval(store: KBStore) -> None: src = store.put_source(b"e") claim = store.put_claim(Claim(id="c1", text="t", evidence=[src.id])) @@ -375,6 +517,30 @@ def test_propose_page_round_trip_through_approval(store: KBStore) -> None: assert store.get_page(artifact.id).body == "body" +def test_propose_page_rejects_unknown_claim_ref(store: KBStore) -> None: + with pytest.raises(ProposalError, match="unknown claim id"): + propose_page( + store, title="t", body="b", + claim_ids=["ghost-claim"], proposed_by="a", + ) + + +def test_propose_page_rejects_unknown_entity_ref(store: KBStore) -> None: + with pytest.raises(ProposalError, match="unknown entity id"): + propose_page( + store, title="t", body="b", + entity_ids=["ghost-entity"], proposed_by="a", + ) + + +def test_propose_page_rejects_unknown_source_ref(store: KBStore) -> None: + with pytest.raises(ProposalError, match="unknown source id"): + propose_page( + store, title="t", body="b", + source_ids=["ghost-source"], proposed_by="a", + ) + + # --- lifecycle ------------------------------------------------------------ diff --git a/tests/test_sync.py b/tests/test_sync.py index aaf313aa..8670683b 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -131,6 +131,52 @@ def test_sync_check_cli_outputs_report( assert "claims/c1.yaml" in payload["new_files"] +# --- graph integrity on the sync path ------------------------------------ +# +# Sync walks the same `bundle._validate_content` per-file pass as +# import, then also calls `bundle._check_graph_integrity` against the +# destination KB. So a directory-or-bundle source whose Relation / +# Page references resolve to nothing locally or in the source itself +# must be rejected by both sync_check and sync_apply. + + +def _hand_write_dangling_relation_kb(root: Path) -> Path: + """Build a `.vouch/` directory whose only relation has dangling + endpoints. Bypasses `put_relation` (which would now reject it) by + writing the YAML directly, simulating an attacker-supplied source.""" + kb = _store(root) + rel_yaml = ( + "id: r-dangling\n" + "source: ghost-source\n" + "relation: uses\n" + "target: ghost-target\n" + "confidence: 0.7\n" + "evidence: []\n" + "created_at: '2026-05-27T00:00:00+00:00'\n" + "updated_at: '2026-05-27T00:00:00+00:00'\n" + ) + (kb.kb_dir / "relations" / "r-dangling.yaml").write_text(rel_yaml) + return kb.root + + +def test_sync_check_rejects_dangling_relation_from_directory_source( + tmp_path: Path, +) -> None: + src_root = _hand_write_dangling_relation_kb(tmp_path / "incoming") + dest = _store(tmp_path / "dest") + report = sync.sync_check(dest.kb_dir, src_root) + assert not report.ok + assert any("dangling reference" in i for i in report.issues), report.issues + + +def test_sync_apply_refuses_dangling_relation(tmp_path: Path) -> None: + src_root = _hand_write_dangling_relation_kb(tmp_path / "incoming") + dest = _store(tmp_path / "dest") + with pytest.raises(RuntimeError, match="dangling reference"): + sync.sync_apply(dest.kb_dir, src_root) + assert not (dest.kb_dir / "relations" / "r-dangling.yaml").exists() + + def test_sync_rejects_source_content_address_mismatch(tmp_path: Path) -> None: """Sync walks the same per-file validation as bundle import, so a source directory whose content does not hash to its name must be