Skip to content

Commit e104a65

Browse files
chore(clippy): enforce production denies in CI (#238)
The crate enables a set of strict clippy denies (no panic, unwrap, expect, or unchecked indexing in production code) but gates them on builds without the test or virt features. Every CI job builds with `--all-features`, which always enables virt, so the denies were switched off across the whole crate in CI and never actually ran. Real violations could land unnoticed, and a few had. This regates the denies on the test feature alone, so production code stays linted even when virt is enabled. The virt test-utility code keeps the latitude it needs through small local allows, since its panics live on scattered match arms rather than a clean module we could exempt wholesale. The change also clears the pre-existing violations that surface once the lint runs for real, in the WebAuthn client-data and assertion paths and the HID framing parser. There are no functional changes, and every feature combination builds clean under the enforced denies.
1 parent e062e5b commit e104a65

7 files changed

Lines changed: 31 additions & 20 deletions

File tree

libwebauthn/src/lib.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
// Tests and the virt test-utility feature are allowed to use unwrap/expect/panic for convenience.
2-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::unwrap_used))]
3-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::expect_used))]
4-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::panic))]
5-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::todo))]
6-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::unreachable))]
7-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::indexing_slicing))]
8-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::unwrap_in_result))]
1+
// Production code must not panic. Tests keep unwrap/expect/panic latitude
2+
// through `not(test)`, and the virt test-utility code through local allows.
3+
#![cfg_attr(not(test), deny(clippy::unwrap_used))]
4+
#![cfg_attr(not(test), deny(clippy::expect_used))]
5+
#![cfg_attr(not(test), deny(clippy::panic))]
6+
#![cfg_attr(not(test), deny(clippy::todo))]
7+
#![cfg_attr(not(test), deny(clippy::unreachable))]
8+
#![cfg_attr(not(test), deny(clippy::indexing_slicing))]
9+
#![cfg_attr(not(test), deny(clippy::unwrap_in_result))]
910

1011
#[cfg(all(
1112
feature = "nfc",

libwebauthn/src/ops/webauthn/client_data.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ impl ClientData {
4141
/// Strings are escaped per WebAuthn L3 §5.8.1.2 (CCDToString), via
4242
/// `serde_json`'s RFC 8259 string encoder. Field order matches the spec:
4343
/// `type`, `challenge`, `origin`, `topOrigin?`, `crossOrigin`.
44+
#[allow(clippy::expect_used)] // serialization of this fixed-shape struct cannot fail
4445
pub fn to_json(&self) -> String {
4546
let operation = match self.operation {
4647
Operation::MakeCredential => "webauthn.create",

libwebauthn/src/ops/webauthn/get_assertion.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,11 @@ impl PrfInputValue {
6161
/// WebAuthn L3 PRF: salt = SHA-256("WebAuthn PRF" || 0x00 || ev.{first,second}).
6262
pub fn to_hmac_secret_input(&self) -> HMACGetSecretInput {
6363
const PREFIX: &[u8] = b"WebAuthn PRF\x00";
64-
let hash = |slice: &[u8]| {
64+
let hash = |slice: &[u8]| -> [u8; 32] {
6565
let mut hasher = Sha256::default();
6666
hasher.update(PREFIX);
6767
hasher.update(slice);
68-
let mut out = [0u8; 32];
69-
out.copy_from_slice(&hasher.finalize()[..32]);
70-
out
68+
hasher.finalize().into()
7169
};
7270
HMACGetSecretInput {
7371
salt1: hash(&self.first),

libwebauthn/src/ops/webauthn/idl/origin.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ pub(crate) fn is_registrable_domain_suffix_or_equal(
315315
return false;
316316
}
317317
let boundary = effective_domain.len() - rp_id.len() - 1;
318-
if effective_domain.as_bytes()[boundary] != b'.' {
318+
if effective_domain.as_bytes().get(boundary) != Some(&b'.') {
319319
return false;
320320
}
321321
if &effective_domain[boundary + 1..] != rp_id {

libwebauthn/src/ops/webauthn/psl/mod.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@
2020
//! Most callers should use [`SystemPublicSuffixList::auto`], which probes
2121
//! the standard system paths for whichever format is available.
2222
23-
// Module-scoped until the crate-wide indexing_slicing deny lands.
24-
#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::indexing_slicing))]
25-
2623
pub mod dafsa;
2724
pub mod dat;
2825
mod system;

libwebauthn/src/transport/hid/channel.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@ impl<'d> HidChannel<'d> {
244244
.open_device(&hidapi)
245245
.or(Err(Error::Transport(TransportError::ConnectionFailed)))?),
246246
#[cfg(feature = "virt")]
247+
#[allow(clippy::unreachable)]
248+
// virtual devices never go through hid_open
247249
HidBackendDevice::VirtualDevice(_) => unreachable!(),
248250
}
249251
}
@@ -317,6 +319,8 @@ impl<'d> HidChannel<'d> {
317319
response
318320
}
319321
#[cfg(feature = "virt")]
322+
#[allow(clippy::panic)]
323+
// virt test-utility: a poisoned lock is unrecoverable
320324
OpenHidDevice::VirtualDevice(backend) => {
321325
let Ok(mut guard) = backend.lock() else {
322326
panic!("Poisoned lock on Virtual HID device");
@@ -378,6 +382,8 @@ impl<'d> HidChannel<'d> {
378382
})?
379383
}
380384
#[cfg(feature = "virt")]
385+
#[allow(clippy::panic)]
386+
// virt test-utility: a poisoned lock is unrecoverable
381387
OpenHidDevice::VirtualDevice(backend) => {
382388
let Ok(mut guard) = backend.lock() else {
383389
panic!("Poisoned lock on Virtual HID device");

libwebauthn/src/transport/hid/framing.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,10 @@ impl HidMessageParser {
141141
if self.packets.is_empty() {
142142
// First packet must be an initialization packet: high bit of
143143
// byte 4 set (CTAP 2.2 §11.2.4).
144-
if packet[4] & PACKET_INITIAL_CMD_MASK == 0 {
144+
let is_initialization = packet
145+
.get(4)
146+
.is_some_and(|&byte| byte & PACKET_INITIAL_CMD_MASK != 0);
147+
if !is_initialization {
145148
error!("First packet is not an initialization packet");
146149
return Err(IOError::new(
147150
IOErrorKind::InvalidData,
@@ -151,15 +154,20 @@ impl HidMessageParser {
151154
} else {
152155
// Continuation packets: same CID as the initial packet, SEQ has
153156
// high bit cleared, SEQ starts at 0 and increments monotonically.
154-
let initial = &self.packets[0];
155-
if packet[..4] != initial[..4] {
157+
let initial_cid = self.packets.first().and_then(|initial| initial.get(..4));
158+
if packet.get(..4) != initial_cid {
156159
error!("Continuation packet CID does not match initial packet");
157160
return Err(IOError::new(
158161
IOErrorKind::InvalidData,
159162
"Continuation packet CID mismatch",
160163
));
161164
}
162-
let seq = packet[4];
165+
let Some(&seq) = packet.get(4) else {
166+
return Err(IOError::new(
167+
IOErrorKind::InvalidData,
168+
"Packet length is invalid",
169+
));
170+
};
163171
if seq & PACKET_INITIAL_CMD_MASK != 0 {
164172
error!(seq, "Unexpected init packet during continuation");
165173
return Err(IOError::new(

0 commit comments

Comments
 (0)