chore(clippy): enable stricter lints#207
Merged
Merged
Conversation
AlfioEmanueleFresta
force-pushed
the
fix/security-input-hardening
branch
from
2488fa7 to
6a8e588
Compare
AlfioEmanueleFresta
force-pushed
the
chore/clippy-stricter-lints
branch
from
774aa18 to
491c25b
Compare
AlfioEmanueleFresta
force-pushed
the
fix/security-input-hardening
branch
from
6a8e588 to
9236103
Compare
AlfioEmanueleFresta
force-pushed
the
chore/clippy-stricter-lints
branch
from
491c25b to
0a7c090
Compare
AlfioEmanueleFresta
force-pushed
the
fix/security-input-hardening
branch
from
605f30e to
14c0e5f
Compare
`clippy::indexing_slicing` flags any `&v[i]` / `&v[a..b]` that could panic at runtime, complementing the existing `deny(clippy::panic)` which does not see slice-index panics from transitive crates. `clippy::unwrap_in_result` flags `.unwrap()` / `.expect()` inside functions that return `Result`, where bubbling the error is almost always preferable. Enable both for non-test code; tests retain the latitude they already have via the existing cfg_attr pattern. This commit is intentionally lint-failing: subsequent commits in this PR fix the call sites that the new lints surface.
Replace direct slice / index access with `.get()`, `split_first`, `split_at`, or iterator-based equivalents in CTAP message parsing and PIN/UV helpers. Functions affected: - `fido::AuthenticatorData::deserialize`: bound the trailing-data check. - `pin::PinUvAuthProtocolOne::authenticate`: handle the (impossible in practice) case of an HMAC output shorter than 16 bytes. - `pin::pin_hash`: use iterator `take(16)` on the SHA-256 output. - `proto::ctap1::apdu::ApduResponse::try_from`: rewrite using `checked_sub` and `split_at` so an empty / 1-byte packet errors out. - `proto::ctap1::model`: bound the U2F register-response signature split via `checked_sub`. - `proto::ctap2::cbor::CborResponse::try_from`: rewrite using `split_first`. - `proto::ctap2::model::get_assertion`: convert SHA-256 outputs to `[u8; 32]` via `GenericArray::into` and use `first()` on the allowList shortcut.
- `crypto::trial_decrypt_advert`: switch to `.get()` for all EID-key and candidate-advert subslices, returning `None` on length mismatch (the up-front length checks make this dead in practice, but the lint requires explicit bounded access). - `crypto::reserved_bits_are_zero`: use `.first().copied()`. - `digit_encode`: rewrite to iterate `chunks(CHUNK_SIZE)` over the input and use `.get()` on the zero-padding lookup; behaviour is preserved (verified by the existing unit test). - `tunnel::CableTunnelMessage::from_slice`: use `split_first` instead of indexing `slice[0]` after a manual length check. - `tunnel::decode_tunnel_server_domain`: use `KNOWN_TUNNEL_DOMAINS.get()`, convert the SHA-256 digest into a fixed `[u8; 32]`, and use `BASE32_CHARS.get()` / `TLDS.get()`. - `tunnel::connect` (initial msg buffer) and the trace log: bound the buffer slice via `.get()`. - `tunnel::send_cbor` padding-length byte: use `last_mut`.
In `transport::ble::framing` and `transport::hid::framing`, rewrite `frame()` / `message()`, `expected_bytes()`, and length helpers to use `split_first`, `.get()`, and `saturating_sub` instead of direct indexing. The behaviour is preserved (verified by the existing unit tests). In `transport::hid::channel::open` the INIT nonce comparison now uses `response.payload.get(..INIT_NONCE_LEN)` rather than the previously panic-prone slice index; the explicit length check on the line above makes this safe in practice but the lint requires bounded access.
The NFC backend (gated behind the `nfc`/`pcsc`/`libnfc` features) was not exercised by the default `cargo build`, so the lint enabled in the previous commits did not surface there. CI's `cargo clippy --all-targets --all-features` flags 5 sites: - `channel::NfcChannel::handle`: replace `&buf[..len]` (returned by `handle_in_ctx` into a fixed 1024-byte buffer) with `buf.get(..len)` and surface `HandleError::NotEnoughBuffer` if a backend overruns. - `channel::NfcChannel::cbor_send`: replace the `&rest[..250]` / `&rest[250..]` chunking with `rest.split_at(250)`; the `rest.len() > 250` loop predicate keeps this panic-safe. - `libnfc::Channel::connect_to_target`: replace `&modulations[modulations.len() - 1]` with `.last()`, returning `TransportUnavailable` if the device reports no supported baud rates.
AlfioEmanueleFresta
force-pushed
the
chore/clippy-stricter-lints
branch
from
0a7c090 to
782afa7
Compare
AlfioEmanueleFresta
added a commit
that referenced
this pull request
May 14, 2026
Add a module-scoped deny(clippy::indexing_slicing) for non-test code, mirroring the crate-wide lint planned in #207, as an interim guard. Replace the remaining panic-capable slice indexing in parse_header, is_public_suffix and public_suffix with split_at_checked, starts_with, last, .get() and split_once. Behaviour is unchanged (verified by the existing unit tests and the gated system-file test).
AlfioEmanueleFresta
added a commit
that referenced
this pull request
May 18, 2026
Add a module-scoped deny(clippy::indexing_slicing) for non-test code, mirroring the crate-wide lint planned in #207, as an interim guard. Replace the remaining panic-capable slice indexing in parse_header, is_public_suffix and public_suffix with split_at_checked, starts_with, last, .get() and split_once. Behaviour is unchanged (verified by the existing unit tests and the gated system-file test).
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.
Follow-up to #203 (fix(security): bound panics on hostile authenticator inputs).
Enables
deny(clippy::indexing_slicing)anddeny(clippy::unwrap_in_result)for non-test code. The audit identified five reachable panics from direct slice indexing; these lints prevent regressions of that class.Commits
After the lint commit, 69 clippy errors fire across the codebase. Each subsequent commit zeroes the count for the area it touches.
The fixes replace direct slice indexing with
.get(),split_first/split_at, iterator combinators, orsaturating_sub, depending on what reads naturally at each call site. Behaviour is preserved (verified by the existing unit tests).The base of this PR is
fix/security-input-hardeningso the lint and its fixes review on top of the audit security fixes.