feat: typed verification codes + shared hash helpers#288
feat: typed verification codes + shared hash helpers#288Nelson Spence (Fieldnote-Echo) wants to merge 3 commits into
Conversation
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.
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.
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.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR Summary by QodoAdd typed verification codes and shared SHA-256 helpers
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1. Artifact missing misclassified
|
| match self.code.as_str() { | ||
| codes::ARTIFACT_SHA256_MISMATCH => VerificationCode::ArtifactSha256Mismatch, | ||
| codes::ARTIFACT_FILE_SIZE_MISMATCH => VerificationCode::ArtifactFileSizeMismatch, | ||
| codes::ARTIFACT_PATH_UNAVAILABLE => VerificationCode::ArtifactMissing, |
There was a problem hiding this comment.
1. Artifact missing misclassified 🐞 Bug ≡ Correctness
ReportIssue::classification maps codes::ARTIFACT_PATH_UNAVAILABLE to VerificationCode::ArtifactMissing, but resolve_existing_path emits path_unavailable for any fs::canonicalize error (permission denied, I/O error, etc.), not only NotFound. Downstream consumers that branch on VerificationCode::ArtifactMissing will incorrectly treat permission or I/O failures as absent artifacts.
Agent Prompt
## Issue description
`ReportIssue::classification()` maps `codes::ARTIFACT_PATH_UNAVAILABLE` to `VerificationCode::ArtifactMissing`. However, `resolve_existing_path` emits `path_unavailable` for **any** `fs::canonicalize` error (permission denied, I/O error, etc.), not only `NotFound`. Downstream consumers that branch on `VerificationCode::ArtifactMissing` will incorrectly treat permission or I/O failures as absent artifacts.
## Issue Context
`resolve_existing_path` in `lib.rs` emits `issue_codes.path_unavailable` for any `Err(err)` from `fs::canonicalize`, regardless of `err.kind()`. The classification arm at line 3926 then maps this code to `ArtifactMissing`, conflating two distinct failure modes.
## Fix Focus Areas
- `ordvec-manifest/src/lib.rs[3843-3864]` — add a new `ArtifactUnavailable` (or similar) variant to `VerificationCode`
- `ordvec-manifest/src/lib.rs[3922-3950]` — map `codes::ARTIFACT_PATH_UNAVAILABLE` to the new variant instead of `ArtifactMissing`
- `ordvec-manifest/tests/verification_codes.rs[199-287]` — update the classification round-trip test to cover the new variant
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Wave U2 of the OrdinalDB post-mortem hardening (post-mortem F2 upstream half + F14). Stacked on #287 (
feat/deterministic-manifest) — merge that first; GitHub will retarget this to main.pub constin acodesmodule — zero bare literals at emit sites; internal string-compares switched to consts.#[non_exhaustive] pub enum VerificationCode+ReportIssue::classification()so downstream security code (ordinaldbIntegrityErrormapping) branches on typed values, never strings. Serialized report JSON is byte-stable (regression-tested).sha256_bytes+ boundedsha256_readersharingsha256_file_bounded's EINTR/bounded core; private sqlite duplicate deduped onto the pub fn (F14 — brief-06 reimplemented this by hand, the drift risk this closes).Adversarial 3-lens review: clean (low-only findings). Tests: 95-108 passed across feature combos; clippy/fmt clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BQ1JzAocstWiqtCF5J2V7K