Skip to content

fix(fsck): verify reachable pointer payloads - #601

Closed
forhappy wants to merge 1 commit into
codex/retained-browse-fsckfrom
codex/fsck-reachable-pointer-data
Closed

forhappy wants to merge 1 commit into
codex/retained-browse-fsckfrom
codex/fsck-reachable-pointer-data

Conversation

@forhappy

@forhappy forhappy commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Full fsck could report a protocol-v2 repository clean even when reachable Crab or Git LFS pointer payloads were missing or corrupt. Git connectivity discarded the scanned pointer inventory; valid catalog objects alone did not prove ordered whole-file bytes matched each reachable pointer.

Retain the authenticated temporary Git database in the checker and reuse the existing crab-read::capsule_protocol::verify_installed_dependencies verifier during the data phase. Reachable Crab pointers must have catalog coverage, matching size and exact reconstructed Blake3 bytes; LFS objects must match their SHA-256 identity and size at origin. Existing bounded physical checks and final source/root/browse stability checks remain. The database is reused across phases and removed when its checker drops. Update the fsck guide with these verification boundaries.

Stacked on #594. No format, dependency, configuration or lockfile changes. The advisor plan and both excluded large-file audit reports are absent.

Validation:

  • Before-change reproduction: all seven fixtures reported zero errors, including missing catalog coverage, wrong pointer size, wrong whole-file hash, missing LFS and corrupt LFS.
  • Final focused fsck/orchestration/history tests: 74 passed, zero failures, with no source drift. Format and whitespace checks pass. Production CLI release build and --version pass on committed head be0bda09cd7, with no source drift.
  • The repository CI Clippy policy passes with no source drift. Strict all-target Clippy remains red: an actual parent-versus-candidate run on the same Rust 1.97 toolchain reports 9,712 versus 9,710 errors, with zero introduced diagnostic fingerprints and two removed large-future diagnostics. This comparison does not make the strict gate green; no lint policy or inventory is relaxed.
  • New-binary native RustFS smoke passed on the owned 64 MiB / 532-publication fixture: seven commands, 20 checks, three healthy fsck results with zero errors, and both late browse-control corruption cases rejected with exit 1. Wire traces contain three origin xorb GETs per fault case, followed by the final corrupted-control read. Original control absence and healthy results were restored; root bytes and binary/source inputs were unchanged. This uses the directly Cargo-built CLI, not an installer qualification; negative pointer/hash/LFS cases remain covered by the separate full-runner regression.
  • New regressions cover the seven valid/invalid payload cases, temporary database reuse and cleanup, and the data phase called without an earlier Git phase.

Evidence map:

  • Entry/caller: crab/src/main.rs capsule fsck dispatch -> crab/src/cmd/fsck.rs::run_fsck -> StoreChecker Git/data phases.
  • Owner/callee: CLI owns checker lifetime; crab-read owns reachable-pointer scanning and origin verification, calling verify_catalog_file_recipe and LfsObjectStore::verify_origin.
  • Siblings: historical recovery already uses the shared dependency verifier. Mirror pre-push/reconcile retain their explicit-pointer recipe checks and do not initialize the full Git workspace. Legacy layout checking is unchanged.
  • Current main lacks the capsule fsck path; the deterministic false-clean reproduction uses actual parent 70a9a78c5a6.
  • Dependency contract: Tokio OnceCell retains a successful initialization, retries failed initialization, and releases its owned TempDir with the checker. The shared verifier requires authenticated installed packs and pinned refs/catalog; this caller supplies the captured view and keeps the directory alive through completion.

Is this the best fix? Reusing the existing integrity verifier keeps whole-file/LFS policy in one owner and closes reachable-pointer coverage without another reconstruction path. Physical catalog checking still rereads some bodies; reducing that maintenance cost requires shared API and consumer proof. Normal lookup and push paths are unchanged.

Remaining gates: the final combined 10 GiB matrix, lifecycle coverage and representative S3 latency. Active phase cancellation is unchanged. The parent stack also has the separately recorded rusqlite source-policy inventory mismatch awaiting explicit approval. This draft is not ready to merge.

Native RustFS qualification on exact PR head be0bda09cd7:

  • 28 commands / 45 checks pass with native publication of a 4 MiB Crab file and 64 KiB LFS file. The frozen baseline binary has the same fsck module as this PR’s parent.
  • With healthy local LFS bytes retained, the baseline reports zero errors for both missing origin data and same-size corrupt origin data, and performs no LFS payload GET. The candidate reads origin, reports a failed terminal result and exits 1 for both faults.
  • Exact restoration makes fsck pass after each fault. The authoritative root bytes remain unchanged. An independent eager cold clone reproduces both payloads byte for byte; native Git strict/full fsck passes. Binary and guarded source hashes remain unchanged; no proxy errors or credential disclosure.
  • This is scoped correctness evidence on the direct Cargo-built CLI. It is not installer, final combined 10 GiB or hosted-S3 latency qualification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant