fix: enforce Source content-addressing on bundle/sync import#126
Merged
plind-junior merged 28 commits intoJun 2, 2026
Merged
Conversation
…ouchdev#76) crystallize wrote a durable Page directly through store.put_page, embedding sess.task, sess.note, and sess.agent verbatim into the rendered markdown body. The body was never gated by propose_page + approve, so an agent calling kb.session_start(task=<payload>) and getting any one claim approved via crystallize could land arbitrary content into pages/. The page surfaces in kb.read_page, kb.list_pages, kb.context, and (once vouchdev#60 is fixed) kb.search. Restrict the summary body to fields the proposing agent cannot influence: session id (server-generated), timestamps (server clock), and the list of approved artifact ids. The agent-controlled fields remain on the Session model itself and are still queryable, but no longer promoted into a durable Page. Also include summary_page_id in the session.crystallize audit event's object_ids when a page is written, so vouch audit truthfully attributes the write. Adds two regression tests: - test_crystallize_summary_page_does_not_leak_agent_controlled_fields - test_crystallize_audit_event_records_summary_page_id
Leftover from the merge of main (which brought PR vouchdev#62's test_sessions.py changes); ruff F401 was failing CI on PR vouchdev#77.
…index FTS5 on update (vouchdev#78) Two compounding bugs let archived / superseded / redacted claims keep flowing back to agents through kb.context: 1. build_context_pack had no claim.status filter, so any retrieval hit was appended to the pack regardless of subsequent lifecycle mutations. 2. store.update_claim refreshed the embedding cache but never re-indexed the FTS5 row, so claims_fts.status stayed frozen at first-index time. Every lifecycle op (archive, supersede, contradict, confirm) routes through update_claim and was therefore invisible to FTS5 / semantic search. This made ClaimStatus.{ARCHIVED, SUPERSEDED, REDACTED} purely decorative on the read side — agents kept quoting retracted knowledge as if it were live. Fix: - storage.update_claim now also calls index_db.index_claim under a short-lived sqlite connection (mirroring proposals.approve's first-index pattern). FTS5 errors are logged-and-skipped so a lifecycle op never fails because the embedding/FTS5 layer is misconfigured. - context.build_context_pack resolves each claim hit, drops it if the on-disk status is in {ARCHIVED, SUPERSEDED, REDACTED}, and drops it if the claim YAML has been deleted between index time and now. CONTESTED claims keep surfacing so contradictions remain visible. Tests: - test_context_pack_excludes_archived_claims - test_context_pack_excludes_superseded_claims - test_update_claim_refreshes_fts5_status (asserts the FTS5 status column via direct SQL against state.db) No on-disk-layout, schema, or bundle-format change.
Resolves conflicts in CHANGELOG.md, tests/test_sessions.py, and auto-merges src/vouch/sessions.py with the recently-merged: - vouchdev#61 (FTS5 indexing of crystallize summary page) — adds index_db.index_page(...) right after store.put_page(...) inside crystallize. Composes cleanly with this PR's strict-derivation _build_summary_body and the summary_page_id audit fix. - vouchdev#62 (test_crystallize_collects_approval_failures) — preserved as a sibling test. - vouchdev#75 (bundle import sha256 verification) — CHANGELOG entry only. - vouchdev#73 (bundle POSIX separators) — CHANGELOG entry only. Both regression tests added by this PR (test_crystallize_summary_page_does_not_leak_agent_controlled_fields and test_crystallize_audit_event_records_summary_page_id) still pass against the merged sessions.crystallize.
vouchdev#81) The 'claims must cite sources' guarantee (README §'Why this exists' point 3; CONTRIBUTING §'Things we won't merge') used to live only in proposals.propose_claim, so every other write path silently accepted Claim(evidence=[]) and landed an uncited claim: - store.put_claim direct: existence-check loop iterates zero times. - store.update_claim: writes the YAML without re-validating. - bundle.import_apply via _validate_content: defers to Claim.model_validate, which accepted evidence=[] because the model had no min-length constraint. Add @field_validator('evidence') on Claim — raises ValueError when the list is empty. Closes all three bypass paths in one place. store.update_claim additionally re-validates via Claim.model_validate(claim.model_dump()) before persisting, so in-place mutation (c.evidence = []; store.update_claim(c)) raises before the YAML hits disk — the field validator only fires at construction time, not on attribute assignment. Four regression tests: - test_claim_model_rejects_empty_evidence (tests/test_storage.py) — Claim(evidence=[]) raises pydantic.ValidationError. - test_put_claim_rejects_empty_evidence — store.put_claim raises; no claims/<id>.yaml is written. - test_update_claim_rejects_empty_evidence — in-place mutation + update_claim raises; the on-disk YAML is unchanged. - test_import_rejects_uncited_claim (tests/test_bundle.py) — a schema-valid bundle whose claim YAML has evidence: [] is rejected by import_check (schema validation issue) and import_apply raises before writing. The existing guard in proposals.propose_claim becomes a redundant user-facing error message and is left in place for the friendlier CLI/JSONL error string. No on-disk-layout, schema, or bundle-format change; data the model never should have accepted now raises.
…ing from tarball (vouchdev#80) import_check previously only verified that tar members listed in the manifest had matching hashes, but never checked the reverse: whether every manifest entry had a corresponding tar member. A bundle whose manifest.json referenced claims/c1.yaml but whose tarball contained only manifest.json would pass import_check with ok=True, and import_apply would silently write nothing — no exception, no audit event indicating data loss. Add the missing-member pass (mirroring the existing check in export_check) so that manifest entries without a matching tar member produce a "manifest lists missing file" issue. import_apply then refuses to import because check.issues is non-empty.
…hdev#82 review) The new Claim.evidence min-citation validator (vouchdev#81) also fires when claims are read back from disk. A KB that has a pre-existing uncited claims/<id>.yaml from before the fix would otherwise crash vouch lint / vouch doctor with a bare pydantic.ValidationError deep in store.list_claims(). Add _load_claims_for_lint(), a per-file iteration that catches pydantic.ValidationError (and any other load error) and surfaces each bad file as a Finding with code='invalid_claim' and an explicit repair hint: 'edit the YAML to add a citation, or delete the file'. lint() also stops calling status() to populate counts — status() calls the strict store.list_claims() which would re-raise on the same files — and builds the counts dict inline from the safely-loaded valid claims. Regression test in tests/test_health.py: - test_lint_surfaces_legacy_uncited_claim_yaml_without_crashing hand-crafts a claims/legacy.yaml with evidence: [] (matches the on-disk shape an older buggy write path would have left), asserts vouch lint runs to completion, surfaces invalid_claim in findings with the repair-hint message, and that the well-formed sibling claim is still discovered. CHANGELOG migration note expanded to describe the repair hint.
The new inline-built counts in lint() (from 5c881de) is a literal dict with mixed value types (str/int/bool), which mypy correctly inferred as dict[str, object] and rejected against the HealthReport.counts: dict[str, int] annotation. The original status() returned the same mixed dict via an untyped 'dict' return, which masked the mismatch — the narrow type was effectively never checked at the call site. Widen counts to dict[str, Any] to match runtime reality. Also tighten status()'s return annotation from 'dict' to 'dict[str, Any]' for consistency. No caller does arithmetic on counts values; they just echo or pass through, so the widening is risk-free.
…ummary-page-bypass fix(sessions): close crystallize review-gate bypass via summary page
…s-citation fix(models): require Claim.evidence to be non-empty at the model layer
…nd-config fix(context): honor retrieval.backend config instead of hardcoding embeddings
feat(cli): vouch approve accepts multiple ids for scriptable bulk approval
…laim
Bare except Exception on store.get_source() silently swallowed real
I/O and parse errors, masking them as a misleading
ProposalError('unknown source/evidence id'). Narrowing both except
clauses to ArtifactNotFoundError lets genuine errors propagate with
their original type and message intact.
Fixes vouchdev#48
Replace try/except pattern with pytest.raises(Exception) and assert
the raised exception is not a misleading ProposalError('unknown
source/evidence id'). Fixes Ruff B011 and ensures the test fails
if propose_claim unexpectedly succeeds.
Suggested by CodeRabbit review of vouchdev#50.
…error-masking fix: narrow evidence validation to ArtifactNotFoundError in propose_claim
…reindex-retracted-claims fix(context,storage): exclude retracted claims from kb.context and reindex FTS5 on update
…ng-manifest-files fix(bundle): reject import of bundles whose manifest lists files missing from tarball
Fixes vouchdev#125 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ontent-addressing # Conflicts: # CHANGELOG.md # tests/test_bundle.py # tests/test_cli.py
This was referenced Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
bundle.import_check/import_applyandsync.sync_check/sync_applynow enforce the Source content-addressing invariant on the import/sync path. A newbundle._check_source_content_address(path, body, issues)helper runs per member: asources/<sha>/contentfile must satisfysha256(content) == <sha>, and asources/<sha>/meta.yamlmust carry anid(and, when set,hash) that matches its<sha>directory. It is wired intoimport_check(soimport_applyinherits the refusal, since it callsimport_checkand raises on the first issue) and intosync's per-file validation loop (sosync_checkreports it andsync_applyrefuses).Why
Fixes #125. vouch markets Sources as "content-addressed by sha256" —
storage.put_sourcederives the id from the bytes, andverify.verify_sourcere-checkssha256(content) == source.id. But the import/sync paths writesources/<sha>/{meta.yaml,content}straight from the tarball and never verified the content hashed to its claimed id. A manifest-consistent bundle could therefore land a Source whose content does not match its content-address: the per-file sha256 gate from #74 only proves each member's bytes match the manifest entry, not that they match the directory's content-address. The result is an attacker-substitutable evidence body behind a legitimate-looking source id — a claim that "cites source X" points at bytes that were never hashed to X, whilemeta.yamlstill advertises the original hash and the audit log records a cleanbundle.import/sync.applyevent.verify_source/vouch doctoronly flagstored_ok=Falseafter the inconsistent source has already landed; this adds the missing write-time gate.What might break
No on-disk-layout, schema,
kb.*method, bundle-format, or audit-log-shape change. Honest bundles produced byvouch exportare unaffected (their content always hashes to its id), confirmed by an export→import round-trip test. The only behavioural change is that a bundle or sync source that violates the documented content-addressing invariant is now rejected atimport_check/sync_checktime and refused byimport_apply/sync_apply, the same way a hash mismatch (#74) or an uncited claim (#81) already is. A KB that already contains a content-address-mismatched source on disk is unaffected on read and still surfaces viaverify_source/vouch doctor.VEP
Not required. No change to the object model,
kb.*method surface, on-disk layout, bundle format, or audit-log shape — this only tightens import/sync-time validation to match an invariant the model (storage.put_source) and detector (verify.verify_source) already document.Tests
make checkpasses locally (lint + mypy + pytest) — ruff clean, mypySuccess: no issues found in 31 source files, full pytest green on Windows / Python 3.13 except the pre-existingtest_read_under_root_rejects_symlink_at_resolved_pathwhich needs symlink-create privilege (unrelated).tests/test_bundle.py(content-address mismatch rejected by check + apply,meta.yamlid mismatch rejected, honest content-addressed source accepted, realvouch export→ import round-trip passes as a false-positive guard) andtests/test_sync.py(sync_check reports + sync_apply refuses a mismatched source).CHANGELOG.mdupdated under## [Unreleased].