Skip to content

Cleanups in the public Rust API and errors #119

Description

@Xof

This issue groups 3 related findings.

PUBLIC-API-7 — A corrupt encrypted superblock body is reported as the operational InvalidEncryptionKey, indistinguishable from a wrong passphrase

Location: src/transaction/recovery.rs:348 · Severity: SMELL · Category: error-handling

What the code does. After the key slot has already authenticated the DEK (unwrap_first_matching_slot succeeded at src/transaction/recovery.rs:342), the sealed body is opened with sb.decrypt_body(&cipher, raw).map_err(|_| ChiselError::InvalidEncryptionKey)?. The comment two lines above states the opposite of what the mapping says: "A tag failure here means corruption, not a wrong key (the slot already authenticated the DEK), so map to InvalidEncryptionKey rather than poisoning."

Why it is a problem. At this point the supplied credential is proven correct — the only way decrypt_body can fail is a damaged or tampered superblock body. Reporting InvalidEncryptionKey tells the operator "wrong passphrase or raw key" (src/error.rs:320-323) and classifies as non-fatal (is_fatal() == false, src/error.rs:214), so an automated caller retries credentials or falls through to its "user typo" path while the file is actually corrupt. There is no way for a caller to tell the two apart, and the failure is unrecoverable by retrying keys.

Direction of a fix. Return a distinct error for a post-unwrap body failure — DecryptionFailed { page_id } (already fatal and already meaning "AEAD failure on data we located") or CorruptSuperblock — so the caller can distinguish "try another credential" from "restore from backup".

PUBLIC-API-9 — The public Key type can only be constructed through zeroize::Zeroizing, which the crate neither re-exports nor wraps in a constructor

Location: src/crypto/mod.rs:35, src/lib.rs:86, Cargo.toml:83 · Severity: SMELL · Category: api-design · Status: NEW

What the code does. pub enum Key { Raw(Zeroizing<Vec<u8>>), Passphrase(Zeroizing<String>) } (src/crypto/mod.rs:35-38) is re-exported as chisel::Key (src/lib.rs:86), but it has no constructor (Key::raw(&[u8]) / Key::passphrase(&str)) and lib.rs has no pub use zeroize. The crate pins zeroize = "~1.8" (Cargo.toml:83); README.md:239 tells users to use zeroize::Zeroizing;.

Why it is a problem. Reproduced while writing a downstream probe: chisel::Key::Raw(...) fails to compile with E0433 (use of unresolved module or unlinked crate zeroize) until the consumer adds their own zeroize dependency whose resolved version must unify with chisel's tilde pin. The entire encryption API — Options::encryption_key, add_key, rotate_key, remove_key — is unreachable without that extra, undeclared-in-the-API dependency, and a future bump of chisel's zeroize major would break every caller's construction site without any change to chisel's own signatures.

Direction of a fix. Add inherent constructors (Key::raw(impl Into<Vec<u8>>), Key::passphrase(impl Into<String>)) that wrap in Zeroizing internally, so the third-party type never appears in a construction path; optionally pub use zeroize::Zeroizing as well for callers who already hold one.

PUBLIC-API-10 — Chisel::tag's doc still describes the pre-newtype u32 return ("Returns 0 for untagged handles")

Location: src/lib.rs:613 · Severity: NIT · Category: comment-accuracy

What the code does. The doc reads "Return the tag stored in the handle-table entry for handle. Returns 0 for untagged handles." (src/lib.rs:613-614) while the signature is pub fn tag(&self, handle: Handle) -> Result<Option<Tag>> (src/lib.rs:618) and the body maps stored 0 to None via Tag::newTag(0) is unconstructable by design (src/handle.rs:88-105).

Why it is a problem. A reader writing against this method from the rendered rustdoc looks for a zero sentinel that the type system has deliberately made unrepresentable, and the neighbouring README table (README.md:270) repeats the same stale claim — so the wrong model is reinforced in two places.

Direction of a fix. Reword to "Returns None for untagged handles" and update the matching README API-table row.


Filed from the clean-slate deep review of 2026-07-29. Full context, verification notes, and the delta against ISSUES.md are in docs/reviews/review-20260729-183138.md. Baseline at review time: 681 tests passing, clippy and fmt clean — none of these are toolchain-visible.

Metadata

Metadata

Assignees

No one assigned

    Labels

    review-2026-07-29Found by the clean-slate deep review of 2026-07-29severity:smellWorks but unidiomatic, duplicated, or hard to maintaintype:api-designPublic surface, types, ergonomicstype:docsDocs contradict code; stale or wrong commentstype:error-handlingError propagation, context, classification

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions