Skip to content

Classify recovery errors by type, so storage cannot make a valid head look invalid - #20

Merged
hmirin merged 1 commit into
mainfrom
fix/recovery-classification
Oct 28, 2026
Merged

hmirin merged 1 commit into
mainfrom
fix/recovery-classification

Conversation

@hmirin

@hmirin hmirin commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Fixes A2 and C6 from the implementation review.

Bugs

  • A2 (high). classify_recovery_error counted a head as Unavailable only if an io::Error was in the chain. The carrier backend reports a missing object with a plain bail!, so a valid head that was only temporarily unreadable was classified Invalid, and recover --publish could overwrite it. The spec says an Unavailable head must never be overridden. The candidate search also skipped newer valid history for the same reason, because read_history_material replaced the typed read error with a new string.
  • C6 (medium). Classification matched message text such as contains("unsupported"). The text can include storage-controlled content, for example an unknown field name, so storage could steer the class.

Reproduced on main before the fix:

  • deleting the carrier's head manifest produced Invalid;
  • a newer valid head with missing objects under junk was skipped, and recover --publish published over it from the floor;
  • an error whose message mentions unsupported was classified Unsupported.

Fix

  • New src/error.rs with typed errors: ObjectUnavailable, MissingStorageHead, Unsupported, NotActiveReader, ForeignRepository, BudgetExhausted. They are raised where those conditions occur, in both backends' open_object and in the manifest, policy, key-file, budget and checkpoint checks. Message text is unchanged.
  • classify_recovery_error checks only the error types in the chain, in the same order as before. A non-InvalidData io::Error still counts as Unavailable.
  • read_history_material keeps the original error and adds context.
  • The contains("budget exhausted") checks in recover became type checks.
  • The old "cannot open the generation key" branch is removed: no code in main produces that message.

Tests

  • carrier_recovery_classifies_missing_head_manifest_as_unavailable
  • carrier_recovery_refuses_to_skip_a_historical_head_with_missing_objects
  • recovery_classification_ignores_storage_controlled_message_text: every old trigger string comes out Invalid.
  • recovery_classification_follows_error_types_through_context
  • cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test --all-targets (144 passed).

Carrier chunk-path failures and directory read-time I/O errors are classified exactly as before. Widening what counts as unavailable is a separate decision.

… look invalid

Recovery decided whether a head was invalid, unavailable, unsupported or
unverifiable by matching error message text, and counted a head as
unavailable only when an io::Error was in the chain. The carrier backend
reported a missing object as a plain string, so a valid head that was only
temporarily unreadable was classified Invalid and recover --publish could
overwrite it, which the spec forbids. The same mislabelling made the
candidate search skip newer valid history. Message matching also let
storage-controlled text (for example an unknown field name) change the class.

Errors that recovery distinguishes now have types in src/error.rs
(ObjectUnavailable, MissingStorageHead, Unsupported, NotActiveReader,
ForeignRepository, BudgetExhausted), raised where those conditions occur,
and classify_recovery_error inspects only the types in the error chain.
read_history_material keeps the original error instead of replacing it with
a new string.
@hmirin
hmirin merged commit f63e9e2 into main Oct 28, 2026
1 check passed
@hmirin
hmirin deleted the fix/recovery-classification branch October 29, 2026 00:10
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