lints: adopt canonical clippy block; bound the VHD BAT allocation by the file - #6
Merged
Conversation
VhdReader::parse_dynamic reads max_entries from the dynamic-disk
"cxsparse" header and allocates from it directly:
let max_entries = u32::from_be_bytes(dh[28..32]...) as usize;
let mut bat_raw = vec![0u8; max_entries * 4];
max_entries is an untrusted u32 straight out of the image. block_size is
validated on the line above it; max_entries is not. A header claiming
0xFFFF_FFFF entries asks for 17_179_869_180 bytes - 16 GiB - before a
single BAT byte is known to exist. The fixture here is under 3 KiB.
RED: the new test fails against the current code with
expected a BAT-bounds error naming the offending size,
got: Vhd decode error: failed to fill whole buffer
Observed behaviour, stated precisely rather than assumed: on this host
(macOS, lazily-committed zero pages) the 16 GiB allocation SUCCEEDS, and
the failure only surfaces later as a generic read_exact error. So this is
not a guaranteed crash. It is an unbounded speculative allocation driven
by an unvalidated length field, which is a memory-pressure/DoS vector on
an allocator that commits eagerly or under a cgroup/ulimit, and which
reports an unhelpful error even when it survives. The second test pins
the happy path so the bound cannot be made too tight.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l lints
GREEN for the preceding RED commit.
parse_dynamic now checks the claimed BAT extent against the actual file
length before allocating, and reports the offending size and offset
verbatim rather than letting it surface as "failed to fill whole buffer":
VHD BAT claims 17179869180 bytes at offset 1536, past the end of
the 2560-byte file
The BAT cannot outgrow the file that holds it, so the file length is the
natural bound (ADR-0012: never trust a length field). The usize::try_from
on the byte count also removes the `max_entries * 4` multiplication, which
overflows usize on a 32-bit target.
disk-forensic had `[lints.rust] unsafe_code = "forbid"` but NO
[lints.clippy] table at all. It now carries the canonical recipe
(CLAUDE.core.md "Rust Lint Posture") plus the ADR-0012 superset:
correctness/suspicious denied, unwrap_used/expect_used denied, all +
pedantic warned with the canonical cast_* allows.
Sixteen production findings surfaced; all are closed at the source:
clippy::unwrap_used 4 VHD footer and BAT integer reads
moved to safe-read bounded readers
(be_u64/be_u32), which is also what
retires the `try_into().unwrap()`
idiom in this file
clippy::doc_markdown 5 backticks added
clippy::cast_lossless 4 u32::from
clippy::missing_fields_in_debug 1 finish() -> finish_non_exhaustive()
on LogicalImage, whose Debug omits
the non-Debug `backend`
clippy::items_after_statements 1 ISO_PVD_OFFSET hoisted to module
scope beside the other magic consts
Two allows, each narrow and with its reason recorded:
too_many_lines container::open is a flat dispatch over every supported
container format; splitting it would scatter one
readable match across helpers. Allowed across the fleet.
redundant_else Two sites in disk4n6. Without `serde` the JSON arm ends
in `return`, so clippy reads the `else` as redundant;
with `serde` enabled it is required. Verified the lint
does NOT fire under --all-features, so removing the
`else` would break the feature-on build.
Gate: cargo build --all-targets --all-features, cargo test (21 suites) and
cargo test --all-features (21 suites), cargo clippy --all-targets -- -D
warnings under BOTH default and --all-features, cargo fmt --check - all
clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
h4x0r
force-pushed
the
lints/canonical-lints
branch
from
August 6, 2026 00:11
6fd9f2e to
ed59866
Compare
safe-read 0.2.1 is newly published and this store had no record for it, so
`cargo vet --locked` fails with:
safe-read:0.2.1 missing ["safe-to-deploy"]
safe-read is ours, published to crates.io by h4x0r, so ADR-0018 mechanism (2)
applies — a trust entry keyed on the publisher, not an exemption pinned to
0.2.1 that would go stale again on its next release. This is the third time in
this sweep that publishing one of our own crates reddened consumers holding
version-pinned records; the trust entry is what stops the cycle.
Verified by control: with the entry removed `cargo vet --locked` reproduces the
failure above, and with it restored the run succeeds.
… target
Two failures, two unrelated causes.
Coverage. The gate is `--fail-under-functions 100`, and it was reporting
111 functions with 1 uncovered. The 133 uncovered LINES in the same output are
not the gate -- the job says so itself, because reader-generic code is
monomorphized per reader type and line % can never reach 100 there.
The single uncovered function was the `map_err` CLOSURE in vhd.rs's
parse_dynamic. It is invisible in the missing-lines list, because line 117
counts as covered: the `map_err` call executes, only the closure body never
does. And it never can -- `bat_bytes` is a u32 multiplied by 4, so
`usize::try_from` cannot fail on any 64-bit target.
Rewritten as `let ... else`. The guard survives untouched for a 32-bit build;
what goes is the closure, which llvm-cov counts as a function it can never see
executed. Deliberately NOT a lowered gate, and not a deleted defensive check.
Clippy. `map(|o| o.status.success()).unwrap_or(false)` in tests/live_linux.rs,
which is `#![cfg(target_os = "linux")]` and therefore invisible to clippy on a
macOS host -- the job failed on a target that cannot be linted locally without
help. Verified by lifting the cfg gate temporarily: the lint is
platform-independent even though the test is not.
Verified by control on both:
coverage with the fix 110 functions, 0 uncovered, 100.00% PASS
closure restored 111 functions, 1 uncovered, 99.10% FAIL
clippy with the fix 0 errors
map_or restored 2 errors
Both mutations were asserted applied before re-running, so neither control can
silently no-op. Full suite: 103 passed, 0 failed under --no-fail-fast.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main moved while this branch was open, conflicting on Cargo.lock, supply-chain/audits.toml and supply-chain/imports.lock. Resolved by taking main's copies of all three and re-deriving, rather than hand-merging attestation records. The branch's only audits.toml addition was a `[[trusted.safe-read]]` entry, and main had independently gained the same one, so nothing was dropped -- checked rather than assumed, because silently losing a trust record would quietly downgrade safe-read to an exemption and nobody would notice. Verified on the merged tree: coverage 110 functions, 0 uncovered, 100.00% (the gated metric) clippy 0 errors, -D warnings tests 103 passed, 0 failed (--no-fail-fast) deny advisories ok, bans ok, licenses ok, sources ok vet Vetting Succeeded (46 fully audited, 1 partially, 103 exempted) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The merge commit staged Cargo.lock and only then refreshed it, so the refresh
never made it into the commit and a stale lock shipped. Two jobs failed on the
one cause:
cargo-vet cannot update the lock file ... --locked was passed
Package completeness cargo package regenerates the lock, so the tree reads
dirty and it refuses without --allow-dirty
Both are the same staleness wearing different error messages. Refreshed
minimally -- `cargo metadata`, not `cargo update`, so nothing is upgraded
beyond what the manifests already require.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(vhd): bound the BAT by the file before allocating; adopt canonical lints
GREEN for the preceding RED commit.
parse_dynamic now checks the claimed BAT extent against the actual file
length before allocating, and reports the offending size and offset
verbatim rather than letting it surface as "failed to fill whole buffer":
The BAT cannot outgrow the file that holds it, so the file length is the
natural bound (ADR-0012: never trust a length field). The usize::try_from
on the byte count also removes the
max_entries * 4multiplication, whichoverflows usize on a 32-bit target.
disk-forensic had
[lints.rust] unsafe_code = "forbid"but NO[lints.clippy] table at all. It now carries the canonical recipe
(CLAUDE.core.md "Rust Lint Posture") plus the ADR-0012 superset:
correctness/suspicious denied, unwrap_used/expect_used denied, all +
pedantic warned with the canonical cast_* allows.
Sixteen production findings surfaced; all are closed at the source:
clippy::unwrap_used 4 VHD footer and BAT integer reads
moved to safe-read bounded readers
(be_u64/be_u32), which is also what
retires the
try_into().unwrap()idiom in this file
clippy::doc_markdown 5 backticks added
clippy::cast_lossless 4 u32::from
clippy::missing_fields_in_debug 1 finish() -> finish_non_exhaustive()
on LogicalImage, whose Debug omits
the non-Debug
backendclippy::items_after_statements 1 ISO_PVD_OFFSET hoisted to module
scope beside the other magic consts
Two allows, each narrow and with its reason recorded:
too_many_lines container::open is a flat dispatch over every supported
container format; splitting it would scatter one
readable match across helpers. Allowed across the fleet.
redundant_else Two sites in disk4n6. Without
serdethe JSON arm endsin
return, so clippy reads theelseas redundant;with
serdeenabled it is required. Verified the lintdoes NOT fire under --all-features, so removing the
elsewould break the feature-on build.Gate: cargo build --all-targets --all-features, cargo test (21 suites) and
cargo test --all-features (21 suites), cargo clippy --all-targets -- -D
warnings under BOTH default and --all-features, cargo fmt --check - all
clean.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
🤖 Generated with Claude Code