Skip to content

fix(open): reject unsupported authenticated header capabilities - #37

Open
presempathy-awb wants to merge 2 commits into
NodeDB-Lab:mainfrom
presempathy-awb:draft/upstream-header-capability-20260917
Open

presempathy-awb wants to merge 2 commits into
NodeDB-Lab:mainfrom
presempathy-awb:draft/upstream-header-capability-20260917

Conversation

@presempathy-awb

Copy link
Copy Markdown
Contributor

Summary

Refuse an existing database when either authenticated A/B main-header slot advertises a capability this build does not understand. Report a typed compatibility error rather than silently accepting unknown semantics or describing an intact newer-format store as a bad key/corrupt database.

This is a forward-compatibility safeguard. No current released unknown-flag writer or production incident is claimed. The change is based directly on upstream 7d8ea435975fd63fa2a56434e2898dfb2c5c6aee; it is independent of the separate key-debug PR.

Why both authentication and slot selection matter

The structural decoder previously read the header flags but did not reject unsupported bits. Simply adding a flags check is insufficient: the existing open path reduces failed slot decodes to an absent candidate. An authenticated unsupported slot can then disappear behind an understood alternate, or two such slots can be misreported as unverifiable headers.

This PR preserves the distinction between a slot that fails authentication and one that authenticates but requires unsupported semantics. An unauthenticated bit flip must never manufacture a compatibility error. Once authenticated, however, an unknown capability must not be treated as an interrupted write.

The selected policy is deliberately conservative: either authenticated unsupported slot refuses the open, regardless of sequence ordering. A newer understood slot does not prove that an older unknown capability's effects have been safely removed. There is no format contract here authorizing that inference. A future explicit downgrade/feature-retirement protocol could refine this rule independently; this patch does not guess one.

Complete change list

  • src/errors.rs: add HeaderCapabilityUnsupported { unknown_flags } as a distinct error outside corruption classification, with an operator-facing description of unsupported capabilities.
  • src/pager/format/structural_header.rs: define the currently understood header mask, authenticate the header first, then reject bits outside that mask. Upstream currently understands no capability bits, so its mask remains zero. No writer starts emitting new bits.
  • src/txn/db/open/existing.rs: retain authenticated capability refusals separately from ordinary failed authentication; check those refusals before selecting an active slot, constructing the pager or entering recovery. Counterpart-key attempts still use the existing key derivation/authentication path.
  • src/txn/db/util.rs: preserve the same authenticated refusal in the restore-mode preflight used by public opens. Inspect both slots before returning an understood mode, so the probe cannot mask the full opener's verdict.
  • src/txn/db/open/capability_tests.rs: add a deterministic mixed-slot matrix and a VFS guard that rejects database mutations and recovery-file access before refusal.
  • CHANGELOG.md: document the compatibility refusal and the distinction from unauthenticated corruption fallback.

No release workflow, dependency, external comparison engine, benchmark implementation, page layout, KDF, cipher or normal commit algorithm changes. The encoder remains capable of constructing a header with reserved flags for fixtures; interpreting those flags is the decoder's boundary.

Durable regression coverage

Structural-header tests cover unknown authenticated flags, ordinary understood flags and a flag mutation whose MAC no longer verifies. They establish that compatibility classification occurs after authentication, not from attacker-controlled cleartext alone. The pre-existing candidate's open regression also verifies propagation through A/B selection.

The six additional open-path tests cover:

  1. Unsupported versus understood headers with the unsupported slot in A and B, at lower, equal and higher sequence numbers.
  2. An authenticated unsupported slot alongside a corrupt alternate, in both placements.
  3. Both authenticated slots unsupported.
  4. An unauthenticated unknown flag with an understood alternate: ordinary corruption fallback still opens a readable committed value.
  5. Both slots understood: the latest committed value remains selected.
  6. Public Standalone, ReadOnly and Observer opens: each returns the typed capability error before any database-content mutation or recovery-file access, with the unsupported slot in either position and an understood, unsupported or corrupt alternate.

The mutation guard panics on write, truncate, rename, remove, directory changes and recovery-file opens. That catches attempted mutations even when errors would otherwise be swallowed or final bytes would happen to compare equal. The tests also compare persisted main-file contents. Normal sentinel acquisition is delegated, not bypassed. Native lock implementations may create sentinel files before header validation; this PR does not claim zero filesystem activity before refusal.

The mixed-slot regression was first reproduced against the fork's equivalent selection path: without the early refusal it proceeded into a removal operation. An independent review then identified the preflight classification gap; the expanded upstream public-mode regression failed before that fix and passes afterward. All fixtures use synthetic deterministic keys and memory-backed stores; no real user data or production credentials are involved.

Verification

Fresh local verification on Rust 1.98.1 / Apple Silicon macOS:

cargo fmt --all -- --check
cargo test --locked -p pagedb --lib capability_tests
cargo clippy --locked -p pagedb --all-targets --all-features -- -D warnings
cargo nextest run --locked -p pagedb --all-features
cargo check --locked -p pagedb --target wasm32-unknown-unknown --lib --features opfs
cargo check --locked -p pagedb --target wasm32-wasip1 --lib
RUSTDOCFLAGS='-D warnings' cargo doc --locked -p pagedb --no-deps --all-features --target x86_64-unknown-linux-gnu
cargo test --locked -p pagedb --doc --all-features
git diff --check

Results: six focused open tests passed; strict Clippy passed; 795/795 full-suite tests passed, with the ten existing slow-test skips unchanged. Browser/WASI checks and strict Linux-target documentation passed. Doctests completed successfully with zero runnable examples. Compilation is not a claim of browser or WASI runtime execution. Hosted Linux/macOS/Windows, feature, invariant and benchmark gates provide separate evidence on the published head.

Compatibility and review boundaries

Existing databases with understood flags keep their normal A/B sequencing and crash fallback. Unknown authenticated flags now produce an explicit refusal, which is the intended behavior change. Callers can distinguish “use a build that understands this store” from recovery-worthy corruption. The error does not authorize clearing flags, rewriting headers, discarding data or retrying with a weaker decoder.

Review authentication ordering in the decoder, preservation of capability errors across key attempts, and the early verdict before pager/recovery setup. Then examine the sequence matrix and the guard VFS. This scope does not introduce a downgrade tool or settle future capability semantics. In particular, it does not copy the fork's known KDF-scope bit into upstream's zero mask.

@farhan-syah farhan-syah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified: 9 tests pass and clippy -D warnings is clean on the PR head. The open-path logic (refuse before slot selection, preflight in peek_restore_mode, the mutation-guard VFS) is sound given the contract the PR asserts.

Blocker, no line to anchor: the contract itself. VERSIONING.md line 14 states that a store this build cannot read fails with FormatVersionUnsupported, decided from the cleartext version. The flags field has no documented meaning anywhere. This PR makes it a public capability-bit protocol with a new public error variant and a refusal rule (either slot, regardless of seq), but leaves VERSIONING.md and the README refusal list unchanged. Without that the variant is undocumented public API.

Direction: define the contract in VERSIONING.md (flags are capability bits, a non-zero authenticated bit refuses the open, the store is untouched) and add HeaderCapabilityUnsupported to the refusal list at VERSIONING.md line 16 and README.md line 182.

Comment thread CHANGELOG.md

### Security

- Opening a database now rejects main-header flag bits that this build does not implement when either slot authenticates, even if the alternate slot is understood. Unknown capabilities are not treated as torn writes; unauthenticated corruption still permits normal slot fallback.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this section. It conflicts with main, which renamed [0.1.0] to [Unreleased]. No version has shipped, so the changelog describes 0.1.0 as a whole and a "now rejects" delta has no baseline. Rebase.

}

#[cfg(test)]
mod tests {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate test. unknown_capability_survives_ab_slot_selection is two_authenticated_unsupported_slots_are_refused_without_mutation in capability_tests.rs minus the mutation guard. Remove this module.

}

fn mac_hk(hk: &DerivedKey, bytes: &[u8]) -> Result<[u8; MAC_LEN]> {
pub(crate) fn mac_hk(hk: &DerivedKey, bytes: &[u8]) -> Result<[u8; MAC_LEN]> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mac_hk goes pub(crate) only for the duplicate test in existing.rs. Revert to private once that test is gone.

type SlotDecode = (Option<(MainDbHeaderFields, bool)>, Option<PagedbError>);

#[cfg(test)]
#[path = "capability_tests.rs"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file lives in open/, so #[path] is not needed. Declare it in open/mod.rs as #[cfg(test)] mod capability_tests; and import what it needs.

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.

2 participants