feat(load): stream verified artifacts and enforce load limits

[Fieldnote-Echo]

Summary

This PR changes verified bundle loading and the APIs that expose it. Relative to its current base, it contains no GitHub workflow or merge-gate changes.

It:

• routes canonical and verified-plan reuse through shared dense-loading code;
• decodes primary, sign, and ID artifacts through ordvec's plan-verified descriptor mechanism;
• streams IDs directly into the final vector and reverse map, without an artifact-sized byte buffer or duplicate-only set;
• adds typed artifact-use errors and keeps cross-artifact shape disagreement separate;
• adds finite primary/auxiliary load limits and propagates them through Rust, Python, adapters, examples, and Haystack serialization;
• encapsulates IdMapSearchReport and adds zero-allocation row iteration/accessors;
• adds checked one-descriptor sparse and hybrid open paths;
• adds a subprocess Rust-heap regression test with a fixed 1 MiB transient allowance.

Actual diff

• Base: ci: inventory Cargo surfaces and enforce merge/release policy #30 / codex/ci-merge-gate at 501a4df44c210f4e241b14c7719fb28298cf9a43
• Head: f104ec17fa8a1c6f3fee1ae42f91d8593668de5f
• 5 commits
• 54 files changed
• 3,893 additions / 665 deletions

The diff is concentrated in core/hybrid database loading, Python and adapter propagation, tests, examples, and documentation. The former physical-device-evidence CI job and its 1,165-line validator/test/doc bundle were removed before this head was pushed. Relative to #30, this PR has no .github diff.

Semantics and boundaries

• “Plan-verified” means matching the size and digest in a verified plan; it does not authenticate the manifest producer.
• The checked sparse path closes the pathname-reopen race and establishes freshness at open on controlled local storage.
• An ordinary mutable file-backed mapping is not an immutable snapshot; later or concurrent writes can still change mapped pages.
• Results are query-major and score-sorted within each query block, not globally sorted.
• Physical-device footprint is not claimed or proved by this PR.

Verification at this head

GitHub Actions run 30210345277 is terminal green: 37 checks passed and none failed, including the merge gate, Linux/macOS/Windows Rust jobs, five Cargo surfaces, dependency policy, package smoke, Python/adapters/sdist, iOS target compilation, actionlint, zizmor, and trusted workflow-policy validation.

Local verification after the final rebase covered:

• workspace all-features: 324 tests passed on the database-equivalent pre-rebase head;
• exact-head load-heap target: 2 tests passed, including the required heap regression;
• exact-head workflow/dependency/release policy: 128 tests passed;
• exact five-root ordvec 0.6.0 dependency policy at 51ee981c87e9e4dad414106f8946f0464c721907;
• formatting, actionlint, and git diff --check.

The physical-device replay and footprint comparison remain external evidence, not a CI claim or merge blocker in this PR.

[qodo-code-review]

PR Summary by Qodo

Add verified artifact re-reads, plan-based IdMapIndex open, and report.rows()

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add read_verified() for verified auxiliary artifacts to close verify-then-read TOCTOU.
• Support IdMapIndex::open_from_verified_plan to reuse VerifiedLoadPlan with freshness
 re-checks.
• Add IdMapSearchReport::rows() to return zipped ScoredRows for hybrid consumers.

Diagram

graph TD
  U([Consumer]) --> V["verify_for_load()"] --> P[("VerifiedLoadPlan")]
  P --> O["IdMapIndex::open_from_verified_plan"] --> I["IdMapIndex"]
  O --> R["Verified reads (rehash+size)"] --> D["Artifacts on disk"]
  I --> S["Search report rows()"]

  subgraph Legend
    direction LR
    _usr([Caller]) ~~~ _fn["Function/API"] ~~~ _data[(Data/plan)] ~~~ _file["On-disk artifacts"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Re-run full `verify_for_load` on every open

• ➕ Simplest mental model; no new plan-reuse codepaths
• ➕ Keeps all policy/edge handling centralized in manifest verification
• ➖ Defeats the goal of plan reuse; repeats manifest parsing + checks
• ➖ Higher latency for apps repeatedly opening the same bundle

2. Pin bytes via file descriptors/locks captured at verification time

• ➕ Strongest TOCTOU mitigation: verification and reads refer to the same opened file
• ➕ Potentially avoids re-hashing when descriptors are trusted/pinned
• ➖ Significant API/portability complexity (Windows/Unix differences, lifetime management)
• ➖ Harder to integrate across crates when plan types are foreign and may cross process boundaries

3. Stream hashing into a fixed-size buffer or mmap with validation

• ➕ Could reduce peak allocations for large artifacts while still binding hash-to-bytes
• ➕ May improve performance for large primary artifacts
• ➖ More complex implementation and error handling (partial reads, mapping failures)
• ➖ Still needs careful design to avoid re-open windows if using paths again

Recommendation: The PR’s approach (single bounded read + in-memory SHA-256 over the exact bytes subsequently parsed/loaded) is a pragmatic security/performance tradeoff: it closes the key TOCTOU windows without requiring invasive manifest API changes. The only meaningful alternative worth considering is descriptor/lock pinning, but that would be a larger cross-platform design; keeping the current additive APIs is appropriate for now. If future workloads show memory pressure, consider a streaming/limited-allocation variant, but preserve the “hash exactly what you load” invariant.

Files changed (7) +583 / -4

Enhancement (4) +466 / -4

id_map.rsAdd plan-reuse open API and report.rows() convenience +80/-2

Add plan-reuse open API and report.rows() convenience

• Adds 'IdMapIndex::open_from_verified_plan' to open from 'VerifiedLoadPlan' while re-hashing primary/sign/IDs artifacts to reject stale plans. Adds 'IdMapSearchReport::rows()' (hybrid-only) to return zipped 'ScoredRow's and a unit test validating ordering and alignment.

ordinaldb/src/id_map.rs

io.rsParse ID sidecar from verified in-memory bytes +47/-0

Parse ID sidecar from verified in-memory bytes

• Adds 'parse_ids_bytes' which validates the ID sidecar format (magic/count/size/duplicates) from an already-read byte slice. Used to avoid re-opening files by path after verification, reducing TOCTOU exposure.

ordinaldb/src/io.rs

manifest.rsAdd VerifiedAuxiliaryArtifactExt::read_verified with bounded read + SHA-256 +181/-0

Add VerifiedAuxiliaryArtifactExt::read_verified with bounded read + SHA-256

• Introduces an extension trait on 'VerifiedAuxiliaryArtifactReport' providing 'read_verified()' which re-reads bytes from the verified path and checks size + SHA-256 against recorded values. Includes tests covering mutation detection and oversized file swaps with allocation bounded to recorded size + 1.

ordinaldb/src/manifest.rs

ordinal.rsLoad dense parts from VerifiedLoadPlan with freshness re-checks +158/-2

Load dense parts from VerifiedLoadPlan with freshness re-checks

• Adds 'load_verified_parts_from_plan' to load the primary RankQuant and optional sign sidecar from an existing 'VerifiedLoadPlan' by bounded reading and re-hashing before 'load_from_bytes'. Implements 'read_primary_verified' to validate primary bytes against recorded size/SHA-256 and enforces sign loading policies consistent with canonical bundle loading.

ordinaldb/src/ordinal.rs

Tests (1) +113 / -0

id_map.rsIntegration tests for open_from_verified_plan and stale-plan rejection +113/-0

Integration tests for open_from_verified_plan and stale-plan rejection

• Adds a round-trip test proving 'open_from_verified_plan' produces equivalent search results to the original index. Adds regression tests that corrupt the primary artifact or sign sidecar after verification and assert the plan-based open rejects the stale/mutated artifacts.

ordinaldb/tests/id_map.rs

Other (2) +4 / -0

Cargo.lockAdd sha2/hex dependencies to ordinaldb lockfile entry +2/-0

Add sha2/hex dependencies to ordinaldb lockfile entry

• Updates the lockfile to include 'sha2' and 'hex' as transitive dependencies for ordinaldb. Enables SHA-256 digest computation and hex encoding for re-verification reads.

Cargo.lock

Cargo.tomlAdd 'sha2' and 'hex' as direct dependencies +2/-0

Add 'sha2' and 'hex' as direct dependencies

• Introduces 'sha2' (SHA-256 hashing) and 'hex' (digest encoding) to support artifact re-hash verification during plan reuse and verified auxiliary reads.

ordinaldb/Cargo.toml

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 375d367d3f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Unbounded auxiliary file read ✓ Resolved 🐞 Bug ☼ Reliability

Description

VerifiedAuxiliaryArtifactExt::read_verified uses std::fs::read() and only checks the recorded
size after the full read, so a post-verification mutation to a huge file can allocate/read unbounded
data before it rejects the mismatch.
This can cause excessive memory usage/OOM (and is especially risky because
load_verified_parts_from_plan calls read_verified() on the sign sidecar and then discards the
bytes).

Code

ordinaldb/src/manifest.rs[R151-174]

+impl VerifiedAuxiliaryArtifactExt for VerifiedAuxiliaryArtifactReport {
+    fn read_verified(&self) -> Result<Vec<u8>, VerificationError> {
+        let path = self.path().ok_or_else(|| {
+            VerificationError::from(ManifestError::invalid(format!(
+                "auxiliary artifact {:?} has no verified path to read",
+                self.name()
+            )))
+        })?;
+        let expected_sha = self.sha256().ok_or_else(|| {
+            VerificationError::from(ManifestError::invalid(format!(
+                "auxiliary artifact {:?} has no recorded sha256 to verify against",
+                self.name()
+            )))
+        })?;
+        let bytes =
+            std::fs::read(path).map_err(|err| VerificationError::from(ManifestError::from(err)))?;
+        // Length is the cheap first discriminator; SHA-256 is authoritative.
+        if self.size_bytes().is_some_and(|len| len != bytes.len() as u64) {
+            return Err(sha256_reverification_failed(self.name()));
+        }
+        let digest = hex::encode(Sha256::digest(&bytes));
+        if !digest.eq_ignore_ascii_case(expected_sha) {
+            return Err(sha256_reverification_failed(self.name()));
+        }

Relevance

⭐⭐⭐ High

Team has accepted similar defensive I/O fixes; pre-check/streaming to avoid OOM is likely welcomed.

PR-#18
PR-#14

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

read_verified() fully buffers the file before checking recorded size/digest. In contrast, other
code in this crate (e.g., ID sidecar reading) checks the expected size via metadata before reading
to prevent oversized/truncated inputs from being read into memory first.

ordinaldb/src/manifest.rs[151-174]
ordinaldb/src/io.rs[667-684]
ordinaldb/src/ordinal.rs[1954-1974]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`read_verified()` reads the entire auxiliary artifact into memory via `std::fs::read(path)` and only then checks size/digest. If the artifact file grows large after verification (mutable storage), this can cause unbounded allocation and potential OOM before the function returns an error.

## Issue Context
The manifest already records the expected size/digest; the implementation should use that to reject obviously wrong sizes before allocating, and ideally hash in a streaming manner with an enforced maximum.

## Fix Focus Areas
- ordinaldb/src/manifest.rs[151-174]

### Suggested implementation
- If `self.size_bytes()` is `Some(expected_len)`:
 - Call `std::fs::metadata(path)?.len()` first and immediately error if it != `expected_len` (this avoids allocating huge buffers in common mutation cases).
 - Then read exactly `expected_len` bytes (e.g., `File::open` + `read_exact` into a pre-sized `Vec<u8>`), compute SHA-256, and compare.
- If `self.size_bytes()` is `None`:
 - Enforce a conservative maximum (e.g., the verification limits’ `max_auxiliary_artifact_bytes` if available in the plan/report, or another explicit cap), and stream-read with a bounded reader while hashing.

### Also consider
- In `load_verified_parts_from_plan`, avoid using `read_verified()` for large sidecars when the bytes are discarded; prefer a streaming re-hash function rather than buffering the whole file.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Freshness check TOCTOU ✓ Resolved 🐞 Bug ⛨ Security

Description

load_verified_parts_from_plan re-hashes the primary/sign artifacts and then loads them again by
path (RankQuant::load / SignBitmap::load), so a concurrent file replacement between the check
and the load can cause the index to open bytes that were never re-verified.
This breaks the documented “stale plan over mutable storage is rejected” guarantee for this new
plan-reuse API.

Code

ordinaldb/src/ordinal.rs[R1939-1975]

+    reverify_primary_freshness(
+        plan.artifact_path(),
+        plan.report().artifact.sha256.as_deref(),
+    )?;
+    let rankquant = RankQuant::load(plan.artifact_path())?;
+    if metadata.dim != rankquant.dim()
+        || metadata.vector_count != rankquant.len()
+        || metadata_bits != rankquant.bits()
+        || plan.row_identity().row_count() != rankquant.len()
+    {
+        return Err(DenseError::metadata_mismatch(
+            "verified manifest metadata does not match loaded RankQuant",
+        ));
+    }
+
+    // Resolve the sign sidecar under the same policy as `load_verified_bundle`
+    // (Forbid / Require / RequireIfSupported / Any), but freshness-re-check a
+    // loaded sidecar via `read_verified` before mapping it.
+    let sign_aux = plan.auxiliary_by_name(crate::io::SIGN_AUX_NAME);
+    if matches!(load_options.sign, SignLoadPolicy::Forbid) && sign_aux.is_some() {
+        return Err(DenseError::SignSidecarForbidden);
+    }
+    let sign_required = match load_options.sign {
+        SignLoadPolicy::Require => true,
+        SignLoadPolicy::RequireIfSupported => sign_compatible(metadata.dim, metadata_bits),
+        SignLoadPolicy::Forbid | SignLoadPolicy::Any => false,
+    };
+    let sign = match sign_aux {
+        // A `Forbid` policy with a declared sidecar already returned above; any
+        // other policy loads a declared sidecar.
+        Some(aux) if !matches!(load_options.sign, SignLoadPolicy::Forbid) => {
+            // Re-hash the sign sidecar against its recorded digest before load;
+            // on success the artifact had a verified path.
+            aux.read_verified()?;
+            let path = aux.path().expect("read_verified succeeds only with a verified path");
+            let sign = SignBitmap::load(path)?;
+            if sign.dim() != rankquant.dim() || sign.len() != rankquant.len() {

Relevance

⭐⭐⭐ High

PR’s stated goal is closing verify-then-reopen TOCTOU; team previously accepted similar
freshness/safety tightening.

PR-#18
PR-#25

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code explicitly hashes/checks and then performs a separate load from the path for both the
primary artifact and the sign sidecar, leaving a race window where on-disk bytes can change between
verification and use.

ordinaldb/src/ordinal.rs[1939-1944]
ordinaldb/src/ordinal.rs[1969-1975]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`load_verified_parts_from_plan` performs a verification step (hash re-check) and then separately opens/loads the artifact via its path. This reintroduces a TOCTOU window: if the file changes after the hash but before/during `RankQuant::load`/`SignBitmap::load`, the loaded bytes are not guaranteed to be the bytes that were validated.

## Issue Context
This function is explicitly intended to make verified-plan reuse fail closed on mutable storage by re-checking freshness. As written, it only checks what was on disk at the moment of hashing, not what is ultimately loaded.

## Fix Focus Areas
- ordinaldb/src/ordinal.rs[1939-1975]

### Suggested implementation directions (choose the most feasible)
1) **Verify-after-load (tighten the window):**
  - Load `RankQuant` / `SignBitmap` first, then immediately compute `sha256_file(path)` and compare against the expected digest from the plan.
  - If mismatched, return an error and drop the loaded object.
  - Do the same for the sign sidecar: after `SignBitmap::load(path)`, compute and compare sha256 again.

2) **Descriptor-based loading (close the window):**
  - Extend underlying loading APIs to accept an already-open `File` (or bytes) so you can hash and load from the same opened handle without reopening by path.

3) **For sign sidecar specifically:**
  - Don’t discard the verified bytes from `read_verified`; either add an API to load `SignBitmap` from bytes, or re-hash after load as in (1).

Ensure the doc comments still accurately describe the security/integrity guarantees after the change.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Primary read size unchecked ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

The verified-artifact readers (read_primary_verified /
VerifiedAuxiliaryArtifactExt::read_verified) use a manifest/plan-recorded u64 size to bound
read_to_end into a Vec<u8> without first validating the size fits in usize, so extremely large
sizes (especially on 32-bit targets) can trigger allocation failure/process termination instead of
returning a structured DenseError/VerificationError. This weakens the intent of the
verified-plan reuse path to fail closed with normal, explicit errors on hostile or misconfigured
inputs.

Code

ordinaldb/src/ordinal.rs[R2017-2031]

+    // `artifact.size_bytes` is the manifest-recorded, verification-checked
+    // size; `metadata.file_size_bytes` is the always-present fallback. Either
+    // came from a manifest that already passed size-bounded verification, so
+    // both are trusted bounds.
+    let recorded_size = plan
+        .report()
+        .artifact
+        .size_bytes
+        .unwrap_or_else(|| plan.metadata().file_size_bytes);
+
+    let file = std::fs::File::open(path)?;
+    let mut bytes = Vec::new();
+    file.take(recorded_size.saturating_add(1))
+        .read_to_end(&mut bytes)?;
+    if bytes.len() as u64 != recorded_size {

Relevance

⭐⭐⭐ High

Team often accepts added bounds/validation to fail closed; usize-fit check prevents OOM/abort on
32-bit.

PR-#18
PR-#38

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited code paths obtain an expected/recorded artifact size as a u64 from verified metadata and
immediately apply it to take(expected_size + 1) followed by read_to_end into a Vec<u8> without
a guard converting/checking the value for usize representability or allocatability. Because Vec
capacity and indexing are usize-based, u64 sizes can exceed what the platform can represent or
allocate (notably on 32-bit), making these reads vulnerable to non-graceful allocation failures
rather than producing the intended structured errors.

ordinaldb/src/ordinal.rs[2017-2044]
ordinaldb/src/manifest.rs[166-193]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The verified-artifact read helpers (`read_primary_verified()` and `VerifiedAuxiliaryArtifactExt::read_verified()`) use a manifest/plan-recorded `u64` size to bound `Read::take(expected_size + 1)` and then `read_to_end` into a `Vec<u8>` without validating that the size can be represented and allocated as a `usize` on the current platform. On 32-bit targets (or extremely constrained environments) or with very large artifacts, this can lead to allocation failure/process termination instead of returning a structured `DenseError`/`VerificationError`, undermining the goal that verified-plan reuse fails closed with explicit errors.

## Issue Context
These methods intentionally avoid unbounded reads by using `Read::take(expected_size + 1)`, but `read_to_end` still grows a `Vec`, whose capacity/indexing is `usize`-based and cannot safely represent all `u64` sizes on all targets. `read_primary_verified()` is used by the verified-plan reuse path (`load_verified_parts_from_plan`), so it is part of the API surface intended to be robust against stale/mutable/hostile storage.

## Fix Focus Areas
- ordinaldb/src/ordinal.rs[2017-2044]
- ordinaldb/src/manifest.rs[166-193]

## Suggested fix
- Add an early guard converting the recorded/expected size (`u64`) to `usize` via `usize::try_from(...)`; if conversion fails, return a structured error (e.g., `DenseError::Limit("primary artifact too large to load on this platform".into())`, a targeted `DenseError::metadata_mismatch(...)`, or a `VerificationError` via `ManifestError::invalid(...)`).
- Optionally pre-reserve with `bytes.try_reserve_exact(expected_size_usize.saturating_add(1))` and map `TryReserveError` into `DenseError::Limit(...)` / `VerificationError` to keep failures explicit and non-fatal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit f104ec1

Results up to commit 375d367 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Freshness check TOCTOU ✓ Resolved 🐞 Bug ⛨ Security

Description

load_verified_parts_from_plan re-hashes the primary/sign artifacts and then loads them again by
path (RankQuant::load / SignBitmap::load), so a concurrent file replacement between the check
and the load can cause the index to open bytes that were never re-verified.
This breaks the documented “stale plan over mutable storage is rejected” guarantee for this new
plan-reuse API.

Code

ordinaldb/src/ordinal.rs[R1939-1975]

+    reverify_primary_freshness(
+        plan.artifact_path(),
+        plan.report().artifact.sha256.as_deref(),
+    )?;
+    let rankquant = RankQuant::load(plan.artifact_path())?;
+    if metadata.dim != rankquant.dim()
+        || metadata.vector_count != rankquant.len()
+        || metadata_bits != rankquant.bits()
+        || plan.row_identity().row_count() != rankquant.len()
+    {
+        return Err(DenseError::metadata_mismatch(
+            "verified manifest metadata does not match loaded RankQuant",
+        ));
+    }
+
+    // Resolve the sign sidecar under the same policy as `load_verified_bundle`
+    // (Forbid / Require / RequireIfSupported / Any), but freshness-re-check a
+    // loaded sidecar via `read_verified` before mapping it.
+    let sign_aux = plan.auxiliary_by_name(crate::io::SIGN_AUX_NAME);
+    if matches!(load_options.sign, SignLoadPolicy::Forbid) && sign_aux.is_some() {
+        return Err(DenseError::SignSidecarForbidden);
+    }
+    let sign_required = match load_options.sign {
+        SignLoadPolicy::Require => true,
+        SignLoadPolicy::RequireIfSupported => sign_compatible(metadata.dim, metadata_bits),
+        SignLoadPolicy::Forbid | SignLoadPolicy::Any => false,
+    };
+    let sign = match sign_aux {
+        // A `Forbid` policy with a declared sidecar already returned above; any
+        // other policy loads a declared sidecar.
+        Some(aux) if !matches!(load_options.sign, SignLoadPolicy::Forbid) => {
+            // Re-hash the sign sidecar against its recorded digest before load;
+            // on success the artifact had a verified path.
+            aux.read_verified()?;
+            let path = aux.path().expect("read_verified succeeds only with a verified path");
+            let sign = SignBitmap::load(path)?;
+            if sign.dim() != rankquant.dim() || sign.len() != rankquant.len() {

Relevance

⭐⭐⭐ High

PR’s stated goal is closing verify-then-reopen TOCTOU; team previously accepted similar
freshness/safety tightening.

PR-#18
PR-#25

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code explicitly hashes/checks and then performs a separate load from the path for both the
primary artifact and the sign sidecar, leaving a race window where on-disk bytes can change between
verification and use.

ordinaldb/src/ordinal.rs[1939-1944]
ordinaldb/src/ordinal.rs[1969-1975]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`load_verified_parts_from_plan` performs a verification step (hash re-check) and then separately opens/loads the artifact via its path. This reintroduces a TOCTOU window: if the file changes after the hash but before/during `RankQuant::load`/`SignBitmap::load`, the loaded bytes are not guaranteed to be the bytes that were validated.

## Issue Context
This function is explicitly intended to make verified-plan reuse fail closed on mutable storage by re-checking freshness. As written, it only checks what was on disk at the moment of hashing, not what is ultimately loaded.

## Fix Focus Areas
- ordinaldb/src/ordinal.rs[1939-1975]

### Suggested implementation directions (choose the most feasible)
1) **Verify-after-load (tighten the window):**
  - Load `RankQuant` / `SignBitmap` first, then immediately compute `sha256_file(path)` and compare against the expected digest from the plan.
  - If mismatched, return an error and drop the loaded object.
  - Do the same for the sign sidecar: after `SignBitmap::load(path)`, compute and compare sha256 again.

2) **Descriptor-based loading (close the window):**
  - Extend underlying loading APIs to accept an already-open `File` (or bytes) so you can hash and load from the same opened handle without reopening by path.

3) **For sign sidecar specifically:**
  - Don’t discard the verified bytes from `read_verified`; either add an API to load `SignBitmap` from bytes, or re-hash after load as in (1).

Ensure the doc comments still accurately describe the security/integrity guarantees after the change.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unbounded auxiliary file read ✓ Resolved 🐞 Bug ☼ Reliability

Description

VerifiedAuxiliaryArtifactExt::read_verified uses std::fs::read() and only checks the recorded
size after the full read, so a post-verification mutation to a huge file can allocate/read unbounded
data before it rejects the mismatch.
This can cause excessive memory usage/OOM (and is especially risky because
load_verified_parts_from_plan calls read_verified() on the sign sidecar and then discards the
bytes).

Code

ordinaldb/src/manifest.rs[R151-174]

+impl VerifiedAuxiliaryArtifactExt for VerifiedAuxiliaryArtifactReport {
+    fn read_verified(&self) -> Result<Vec<u8>, VerificationError> {
+        let path = self.path().ok_or_else(|| {
+            VerificationError::from(ManifestError::invalid(format!(
+                "auxiliary artifact {:?} has no verified path to read",
+                self.name()
+            )))
+        })?;
+        let expected_sha = self.sha256().ok_or_else(|| {
+            VerificationError::from(ManifestError::invalid(format!(
+                "auxiliary artifact {:?} has no recorded sha256 to verify against",
+                self.name()
+            )))
+        })?;
+        let bytes =
+            std::fs::read(path).map_err(|err| VerificationError::from(ManifestError::from(err)))?;
+        // Length is the cheap first discriminator; SHA-256 is authoritative.
+        if self.size_bytes().is_some_and(|len| len != bytes.len() as u64) {
+            return Err(sha256_reverification_failed(self.name()));
+        }
+        let digest = hex::encode(Sha256::digest(&bytes));
+        if !digest.eq_ignore_ascii_case(expected_sha) {
+            return Err(sha256_reverification_failed(self.name()));
+        }

Relevance

⭐⭐⭐ High

Team has accepted similar defensive I/O fixes; pre-check/streaming to avoid OOM is likely welcomed.

PR-#18
PR-#14

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

read_verified() fully buffers the file before checking recorded size/digest. In contrast, other
code in this crate (e.g., ID sidecar reading) checks the expected size via metadata before reading
to prevent oversized/truncated inputs from being read into memory first.

ordinaldb/src/manifest.rs[151-174]
ordinaldb/src/io.rs[667-684]
ordinaldb/src/ordinal.rs[1954-1974]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`read_verified()` reads the entire auxiliary artifact into memory via `std::fs::read(path)` and only then checks size/digest. If the artifact file grows large after verification (mutable storage), this can cause unbounded allocation and potential OOM before the function returns an error.

## Issue Context
The manifest already records the expected size/digest; the implementation should use that to reject obviously wrong sizes before allocating, and ideally hash in a streaming manner with an enforced maximum.

## Fix Focus Areas
- ordinaldb/src/manifest.rs[151-174]

### Suggested implementation
- If `self.size_bytes()` is `Some(expected_len)`:
 - Call `std::fs::metadata(path)?.len()` first and immediately error if it != `expected_len` (this avoids allocating huge buffers in common mutation cases).
 - Then read exactly `expected_len` bytes (e.g., `File::open` + `read_exact` into a pre-sized `Vec<u8>`), compute SHA-256, and compare.
- If `self.size_bytes()` is `None`:
 - Enforce a conservative maximum (e.g., the verification limits’ `max_auxiliary_artifact_bytes` if available in the plan/report, or another explicit cap), and stream-read with a bounded reader while hashing.

### Also consider
- In `load_verified_parts_from_plan`, avoid using `read_verified()` for large sidecars when the bytes are discarded; prefer a streaming re-hash function rather than buffering the whole file.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[Fieldnote-Echo]

Both P1 findings (flagged by Codex + Qodo) fixed on this branch.

Finding 1 — reopen-by-path TOCTOU (load_verified_parts_from_plan): the primary and sign are now constructed from the already-verified in-memory bytes (RankQuant::load_from_bytes / SignBitmap::load_from_bytes) instead of re-hashing and then reopening the path. The sign path uses the Vec read_verified previously discarded; the primary goes through a new bounded, re-hashing read_primary_verified helper (which replaces reverify_primary_freshness). Neither ordvec type is mmap-based, so the bytes hashed are the bytes loaded — no check→use window. (8310121)

Finding 2 — unbounded aux read (read_verified): the whole-file std::fs::read before the size check is replaced with a Read::take(recorded_size + 1) bounded read, so a post-verification swap to a huge file is rejected by length without allocating past the recorded size. SHA-256 check and error variants unchanged; a missing recorded size is now refused rather than read unbounded. (772de71)

Tests: cargo test --workspace and cargo clippy --workspace --all-targets green. Added an oversized-swap rejection test and primary/sign post-verification-corruption rejection tests; comments are honest that a true concurrent-swap race needs a filesystem hook (the fix closes the window by construction).

Scope note: the sibling verify-then-load paths (load_verified_bundle, io.rs primary/sign load) still load by path after the initial verify_for_load. That is a pre-existing baseline that never advertised the stronger freshness guarantee, so it is out of scope for this PR — noted as a candidate follow-up hardening PR that could adopt the same load_from_bytes pattern.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8310121

[Fieldnote-Echo]

Codex (@codex) review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c043edbdce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e98011a72

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20788619e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

[Fieldnote-Echo]

Corrected loader layer pushed at cf62979f0353e24aaa5933966dbf72f5745b3bd2, rebased on PR #30 head c05ae00655107610f56415542f90caa446984ea4.

Fresh review fixes now reject ID count mismatches before allocation, stop recovery from rolling back past a newest backup rejected solely by resource limits, and bind physical-device evidence to both the exact declared OrdinalDB commit and the checked-out device surface. The evidence job uses full-history checkout so the offline revision check can resolve an evidence-only parent.

Local proof: 320 Rust tests passed (1 ignored), workspace clippy/fmt are clean, 88 Python tests passed, and all policy/source checks pass. This PR remains open for bot/human review, but is intentionally not merge-ready until evidence/ios-device-replay.json is attached; that evidence check and the derivative merge gate are the only expected reds.

[Fieldnote-Echo]

Heap-gate follow-up pushed at dff1023a6ed7eb99ed2f657c2d1635cd458ab647. The acceptance signal is now direct peak simultaneously-live transient allocation chains, not peak - post_load, so later retained growth cannot mask an early whole-artifact buffer. Release subprocess measurements: canonical 68,195 bytes; verified-plan reuse 65,614 bytes; fixed allowance 1,048,576 bytes. The masking and retained-reallocation regressions both pass.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dff1023a6e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".