feat: deterministic manifest schema v2 — the manifest hash is the content address#287
Conversation
Remove manifest_id and created_at from IndexManifest so identical content serializes to identical bytes and sha256(manifest.json) becomes the bundle's content address. Creation writes build: None, sorts auxiliary artifact entries by (name, path), and validation rejects non-canonical embedded paths. Schema version bumps to ordvec.index_manifest.v2 with a targeted schema_version probe so old-schema manifests fail parse with a clear older/newer-schema error. VerificationReport and the sqlite registry drop manifest_id keying; a stale manifest_id column now forces the verification_reports migration. uuid dependency retained: still used for row-identity db_id validation.
Byte-identical double-create, checked-in golden bytes, artifact and auxiliary change sensitivity, auxiliary declaration-order invariance, old-schema rejection with a clear error, and canonical-path validation.
Creation now runs the same canonical-form check on every embedded path (artifact, row identity, auxiliary artifacts) that verification applies, so create can no longer mint a manifest that fails its own default verification (e.g. a Unix filename containing a backslash). Non-escaping `..` segments (a/../index.ovrq) are now rejected as non-canonical by default: they pass lexical-escape and containment checks while aliasing the same bundle content under distinct verified manifest byte-identities. `..` remains available under the existing allow_path_escape opt-in, matching the documented policy split. Absolute path strings (POSIX, drive-letter, UNC) are excluded from the canonical-form check and governed solely by the allow_absolute_paths policy at resolution, restoring the retained absolute-path opt-in on Windows; path_to_manifest_string also strips Windows verbatim prefixes (\\?\) so created absolute paths round-trip through verification. Calibration and encoder-distortion profile ref paths gain the same canonical check (calibration_profile_path_not_canonical, encoder_distortion_profile_path_not_canonical) closing the remaining aliasing surface on hand-written manifests.
The manifest crate embeds and resolves bundle paths, so its behavior is OS-sensitive, and it ships cross-platform (crates.io plus Windows/macOS wheels via ordvec-manifest-python) — but only the ubuntu manifest lane ran its tests. Add a default-features test step to the 3-OS matrix job; the dedicated ubuntu lane keeps covering the feature matrix and clippy.
The crate README still documented ordvec.index_manifest.v1 and showed verification-report examples carrying the removed manifest_id field. Document the v2 deterministic-bytes contract (content addressing, sorted auxiliary entries, canonical embedded paths), drop stale schema-version tags from the row-identity guidance, and add the new auxiliary_artifact_path_not_canonical reason code to the failure list. Record the breaking schema change under [Unreleased] in CHANGELOG.md as CONTRIBUTING.md requires for user-facing changes.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR Summary by QodoDeterministic manifest schema v2 (hash = content address) + cache migration
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1.
|
is_manifest_path_absolute() treated any leading backslash as absolute,
which skipped the canonical-form check, while resolution used platform
Path::is_absolute() semantics — so on Unix a manifest embedding \evil
resolved inside the bundle and verified successfully under the default
policy.
Only UNC/verbatim (\\-prefixed) forms now count as backslash-absolute;
a single leading backslash falls through to the canonical check, which
rejects backslashes. Path resolution now classifies with the same
platform-independent rule for the allow_absolute_paths gate and rejects
outright (`{context}_absolute_path_unresolvable`) whenever the policy
classification and the platform's resolution semantics disagree, so an
absolute-for-policy path can never silently resolve relative to the
manifest base on any OS.
Schema v2 dropped active_manifest's manifest_id column, but init() only migrated verification_reports; CREATE TABLE IF NOT EXISTS left a legacy active_manifest table in place, so activate()'s INSERT failed at runtime with a NOT NULL constraint on old registry DBs. init() now compares each registry table's live column set against the current schema via PRAGMA table_info and drops mismatched tables before recreating them empty. The registry is rebuildable cache/pointer state (cached verification reports and the active-manifest pointer, never source of truth), so stale schemas are dropped, not migrated — no back-compat machinery by design. The check runs on every init(), ahead of any activate() insert, and is idempotent.
The deterministic manifest schema v2 is a breaking change on top of the release-shaped 0.6.0 tree, and the release-publish invariants require the current version to have a dated changelog section with an empty [Unreleased]. Bump every lockstep version field (core, manifest, ffi, both python bindings), the ordvec dep requirement in ordvec-manifest, the README quickstart requirement, and the THREAT_MODEL status line; move the schema-v2 changelog entry into a dated 0.7.0 section. Both lockfiles re-synced to the new member versions. Also bumps crossbeam-epoch 0.9.18 -> 0.9.20 in both lockfiles to clear RUSTSEC-2026-0204 (same fix as on main), so the branch's cargo-deny advisories job passes independently of the merge base. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
…cOS non-UTF-8) Two OS-matrix failures from running ordvec-manifest tests beyond Linux: - The checked-in golden manifest is byte-compared via include_bytes!, and Windows runners check it out through autocrlf, turning every LF into CRLF and failing the golden byte-identity test. Pin the golden dir -text via .gitattributes so checkout never rewrites fixture bytes. - verify_for_load_preserves_non_utf8_base_paths creates a directory named with an invalid UTF-8 byte; macOS APFS rejects that with EILSEQ at creation, so the scenario cannot exist there. Gate the test to Linux instead of cfg(unix). Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
|
Pushed three things beyond the earlier canonical-path/sqlite fixes:
|
Windows reports a missing file as 'The system cannot find the file specified. (os error 2)' — neither 'No such file' nor 'not found'. Accept the windows wording (and the platform-neutral 'os error 2' raw-io suffix), and print the actual message on failure. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Audit remediation for schema-v2 (ordvec-manifest), all found by an adversarial re-review of the stack: - sqlite report-registry migration is now atomic. The v1->v2 RENAME/CREATE/INSERT/DROP ran as a bare execute_batch (each statement auto-committed), so an interruption after the RENAME left a stray verification_reports_old that wedged every future open (the RENAME target already existed), and two processes could race it. Run it in one IMMEDIATE transaction, DROP any leftover _old first, and re-check the need under the write lock so a lost race commits a no-op. - load_manifest_file now rejects an unsupported schema_version explicitly. A genuine v1 manifest already failed the parse (its manifest_id / created_at trip deny_unknown_fields), but a v2-shaped document that only mislabels schema_version would load and fail solely at verify; the loader now enforces the version as documented. - golden-bytes test message no longer misattributes every failure to a manifest schema change (an .ovrq index-format change or an editor newline-normalizing the fixture are the other causes). - CHANGELOG: creation omits the build field (not 'build: null'), and the registry migration line now distinguishes the migrated verification cache from the reset active_manifest pointer. Tests: +leftover-_old migration recovery, +wrong-schema-version load rejection; ordvec-manifest --all-features 102 passed. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
|
Follow-up audit round (adversarial re-review of the full stack, 3 independent reviewers + triangulation). Two fixes landed on this branch in
Security review came back clean (no high/critical under the untrusted-manifest threat model; canonicalization symmetric + fails closed, reads declared-size-bounded, SQL parameterized). Lower-severity items deferred to #293/#294/#295. |
* feat: add sha256_bytes and bounded sha256_reader hash helpers
Extract the EINTR-safe bounded hashing core from sha256_file_bounded and
share it with a new public sha256_reader; add public sha256_bytes and
dedupe the private sqlite.rs copy onto it.
* refactor: name every verification issue code as a pub const
Add ordvec_manifest::codes with one pub const per issue code (including
the previously format!-built path, metric-spec, and row-id families,
now selected via per-context const structs) and reference the consts at
every emit site, internal compare, and bounded-read call. A source-scan
test rejects new bare literals at emit sites; a value-lock test pins the
security-relevant code strings so silent renames break tests.
* feat: typed verification classification and structured mismatch detail
Add non_exhaustive VerificationCode with ReportIssue::classification()
over the named code consts so downstream security code branches on
typed values. ReportIssue gains optional artifact_name and
expected/actual sha256+size fields (skip_serializing_if None keeps
existing report JSON byte-stable), populated at the artifact, auxiliary,
and row-identity mismatch sites for downstream IntegrityError
reconstruction.
* fix: adapt absolute-path-unresolvable codes to typed const registry
The merge of feat/deterministic-manifest landed the new
{context}_absolute_path_unresolvable rejection while this branch had
replaced resolve_existing_path's context string with PathIssueCodes and
moved every emitted code into the pub codes registry. Wire the new
rejection through both: an absolute_path_unresolvable field on
PathIssueCodes, per-context consts (artifact, row_identity,
encoder_distortion_profile, calibration_profile, auxiliary_artifact),
and locked code values for the two security-relevant variants in
verification_codes.rs.
* docs: changelog entry for typed verification codes under 0.7.0
The 0.7.0 release section arrived via the deterministic-manifest merge
with only the schema-v2 entry; add the Added entries for this branch's
typed verification classification, structured mismatch detail, and
shared hash helpers so the release notes cover the full stacked change.
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
* fix: distinguish missing files from I/O failures in typed classification
Audit remediation for the typed VerificationCode classification, from an
adversarial re-review (confirms and extends the qodo finding on #288):
resolve_existing_path emitted *_path_unavailable for every canonicalize
error kind, and classification() mapped ARTIFACT_PATH_UNAVAILABLE to
VerificationCode::ArtifactMissing. A present-but-unreadable primary
artifact (permission denied, symlink loop, I/O error) was therefore
reported to downstream integrity handling as a missing file. The
auxiliary path already discriminated NotFound; the primary and
row-identity paths did not.
- Add codes ARTIFACT_MISSING / ROW_IDENTITY_MISSING, emitted only on an
ErrorKind::NotFound canonicalize failure (via a new optional path_missing
on PathIssueCodes; profiles keep a single path_unavailable code).
- Classify ARTIFACT_MISSING -> ArtifactMissing and the new
ROW_IDENTITY_MISSING -> RowIdentityMissing (closing a second gap: a
missing rows.jsonl previously classified Unknown while its sha/row-count
mismatches were typed). ARTIFACT_PATH_UNAVAILABLE and
ROW_IDENTITY_PATH_UNAVAILABLE now classify Unknown like their siblings.
- Correct the classification() doc: it over-claimed that all
security-relevant families are typed, when path-policy rejections and
diagnostic I/O failures deliberately map to Unknown.
Tests: end-to-end artifact_missing and row_identity_missing emit+classify,
both *_path_unavailable codes pinned to Unknown, locked code-string table
extended; ordvec-manifest --all-features 114 passed.
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
---------
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
The GitHub merge of #288 into this branch left the last push attributed to project-navi-bot, which require_last_push_approval rejects — so the CODEOWNER approval no longer counts and the PR cannot merge. Push an empty commit as the branch owner (Fieldnote-Echo) to reset the last pusher; a re-approval then satisfies the rule. No content change: the stack (deterministic schema v2 + typed verification codes + the audit-round fixes) is complete, up to date with main, and green. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
…t address write_manifest_file's canonical bytes are the bundle's content address, and nested serde_json::Value maps (extensions / attestations) are key-sorted only while serde_json is built without preserve_order. Document that a downstream graph enabling serde_json/preserve_order (or arbitrary_precision) via feature unification would change the content address, and must not be used for content-addressed manifests. Documents the constraint half of #295. (Also serves as a reviewable Fieldnote-Echo push to reset last-push approval on this PR — the earlier empty commit was not a reviewable push.) Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Wave U1 of the OrdinalDB post-mortem hardening (builder post-mortem F3; reconciled plan + implementation spec in the OrdinalDB planning docs; trust co-design: the deterministic manifest is the future Ed25519 seal surface — see ordinaldb docs/roadmap/ordinaldb-trust-spec.md).
Breaking schema change (v2, zero back-compat by decision — nothing published):
manifest_idandcreated_atremoved fromIndexManifest; creation writesbuild: None(no more per-write UUIDs/timestamps) → two writes of identical content produce byte-identical manifest.json, sosha256(manifest.json)is a stable content address.ordvec.index_manifest.v2; old-schema manifests fail with a clear "older/newer manifest schema" error (version probed on parse failure).(name, path)at creation; canonical bundle-relative forward-slash paths enforced at both create and verify (red-team fix: creation previously could embed paths its own verification rejects); golden byte fixture + determinism tests are the merge gates.VerificationReport.manifest_idand the sqlite registry's manifest_id column removed (forced fallout; cache identity already keyed on manifest_sha256).Adversarial 3-lens review pass done pre-PR: 5 medium findings found and fixed (create/verify canonicality asymmetry, non-escaping
..acceptance, Windows absolute-path contract split, stale README/examples, missing CHANGELOG). Tests: 108 passed (-p ordvec-manifest --all-features), root suite 256 passed; clippy/fmt clean; CI matrix extended to run manifest tests across OSes.Note:
uuiddep retained (still used by row-identity db_id validation).Stacked:
feat/typed-verification-codes(U2) builds on this branch.🤖 Generated with Claude Code
https://claude.ai/code/session_01BQ1JzAocstWiqtCF5J2V7K