Forbid unwrap usage outside of tests (#693)

## Type of change
```
- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
Replaced uses of unwrap by ? or .expect(), and added lint to forbid them
outside of tests.

Cargo.toml

@@ -22,6 +22,9 @@ bitwarden-crypto = { path = "crates/bitwarden-crypto", version = "=0.1.0" }
 bitwarden-exporters = { path = "crates/bitwarden-exporters", version = "=0.1.0" }
 bitwarden-generators = { path = "crates/bitwarden-generators", version = "=0.1.0" }
 
+[workspace.lints.clippy]
+unwrap_used = "deny"
+
 # Compile all dependencies with some optimizations when building this crate on debug
 # This slows down clean builds by about 50%, but the resulting binaries can be orders of magnitude faster
 # As clean builds won't occur very often, this won't slow down the development process

clippy.toml

@@ -0,0 +1,2 @@
+allow-unwrap-in-tests=true
+allow-expect-in-tests=true

crates/bitwarden-c/Cargo.toml

@@ -21,3 +21,6 @@ bitwarden-json = { path = "../bitwarden-json", features = ["secrets"] }
 
 [dependencies]
 env_logger = ">=0.10.0, <0.12"
+
+[lints]
+workspace = true

crates/bitwarden-c/src/c.rs

@@ -11,7 +11,8 @@ pub async extern "C" fn run_command(
     client_ptr: *const Client,
 ) -> *mut c_char {
     let client = unsafe { ffi_ref!(client_ptr) };
-    let input_str = str::from_utf8(unsafe { CStr::from_ptr(c_str_ptr).to_bytes() }).unwrap();
+    let input_str = str::from_utf8(unsafe { CStr::from_ptr(c_str_ptr) }.to_bytes())
+        .expect("Input should be a valid string");
 
     let result = client.run_command(input_str).await;
     match std::ffi::CString::new(result) {
@@ -28,8 +29,8 @@ pub extern "C" fn init(c_str_ptr: *const c_char) -> *mut Client {
     if c_str_ptr.is_null() {
         box_ptr!(Client::new(None))
     } else {
-        let input_string = str::from_utf8(unsafe { CStr::from_ptr(c_str_ptr).to_bytes() })
-            .unwrap()
+        let input_string = str::from_utf8(unsafe { CStr::from_ptr(c_str_ptr) }.to_bytes())
+            .expect("Input should be a valid string")
             .to_owned();
         box_ptr!(Client::new(Some(input_string)))
     }

crates/bitwarden-cli/Cargo.toml

@@ -15,3 +15,6 @@ clap = { version = "4.5.1", features = ["derive"] }
 color-eyre = "0.6"
 inquire = "0.6.2"
 supports-color = "3.0.0"
+
+[lints]
+workspace = true

crates/bitwarden-crypto/Cargo.toml

@@ -21,7 +21,7 @@ mobile = ["dep:uniffi"] # Mobile-specific features
 [dependencies]
 aes = { version = ">=0.8.2, <0.9", features = ["zeroize"] }
 argon2 = { version = ">=0.5.0, <0.6", features = [
-    "alloc",
+    "std",
     "zeroize",
 ], default-features = false }
 base64 = ">=0.21.2, <0.22"
@@ -48,3 +48,6 @@ zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }
 [dev-dependencies]
 rand_chacha = "0.3.1"
 serde_json = ">=1.0.96, <2.0"
+
+[lints]
+workspace = true

crates/bitwarden-crypto/src/aes.rs

@@ -149,7 +149,8 @@ pub fn decrypt_aes128_hmac(
 
 /// Generate a MAC using HMAC-SHA256.
 fn generate_mac(mac_key: &[u8], iv: &[u8], data: &[u8]) -> Result<[u8; 32]> {
-    let mut hmac = PbkdfSha256Hmac::new_from_slice(mac_key).expect("HMAC can take key of any size");
+    let mut hmac =
+        PbkdfSha256Hmac::new_from_slice(mac_key).expect("hmac new_from_slice should not fail");
     hmac.update(iv);
     hmac.update(data);
     let mac: [u8; PBKDF_SHA256_HMAC_OUT_SIZE] = (*hmac.finalize().into_bytes())

crates/bitwarden-crypto/src/enc_string/symmetric.rs

@@ -119,15 +119,15 @@ impl EncString {
         match enc_type {
             0 => {
                 check_length(buf, 18)?;
-                let iv = buf[1..17].try_into().unwrap();
+                let iv = buf[1..17].try_into().expect("Valid length");
                 let data = buf[17..].to_vec();
 
                 Ok(EncString::AesCbc256_B64 { iv, data })
             }
             1 | 2 => {
                 check_length(buf, 50)?;
-                let iv = buf[1..17].try_into().unwrap();
-                let mac = buf[17..49].try_into().unwrap();
+                let iv = buf[1..17].try_into().expect("Valid length");
+                let mac = buf[17..49].try_into().expect("Valid length");
                 let data = buf[49..].to_vec();
 
                 if enc_type == 1 {

crates/bitwarden-crypto/src/error.rs

@@ -28,6 +28,9 @@ pub enum CryptoError {
     #[error("Fingerprint error, {0}")]
     FingerprintError(#[from] FingerprintError),
 
+    #[error("Argon2 error, {0}")]
+    ArgonError(#[from] argon2::Error),
+
     #[error("Number is zero")]
     ZeroNumber,
 }

crates/bitwarden-crypto/src/fingerprint.rs

@@ -51,7 +51,10 @@ fn hash_word(hash: [u8; 32]) -> Result<String> {
         let remainder = hash_number.clone() % EFF_LONG_WORD_LIST.len();
         hash_number /= EFF_LONG_WORD_LIST.len();
 
-        phrase.push(EFF_LONG_WORD_LIST[remainder.to_usize().unwrap()].to_string());
+        let index = remainder
+            .to_usize()
+            .expect("Remainder is less than EFF_LONG_WORD_LIST.len()");
+        phrase.push(EFF_LONG_WORD_LIST[index].to_string());
     }
 
     Ok(phrase.join("-"))

crates/bitwarden-crypto/src/keys/shareable_key.rs

@@ -2,9 +2,12 @@ use std::pin::Pin;
 
 use aes::cipher::typenum::U64;
 use generic_array::GenericArray;
-use hmac::{Hmac, Mac};
+use hmac::Mac;
 
-use crate::{keys::SymmetricCryptoKey, util::hkdf_expand};
+use crate::{
+    keys::SymmetricCryptoKey,
+    util::{hkdf_expand, PbkdfSha256Hmac},
+};
 
 /// Derive a shareable key using hkdf from secret and name.
 ///
@@ -19,15 +22,16 @@ pub fn derive_shareable_key(
 
     // TODO: Are these the final `key` and `info` parameters or should we change them? I followed
     // the pattern used for sends
-    let res = Hmac::<sha2::Sha256>::new_from_slice(format!("bitwarden-{}", name).as_bytes())
-        .unwrap()
+    let res = PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes())
+        .expect("hmac new_from_slice should not fail")
         .chain_update(secret)
         .finalize()
         .into_bytes();
 
-    let mut key: Pin<Box<GenericArray<u8, U64>>> = hkdf_expand(&res, info).unwrap();
+    let mut key: Pin<Box<GenericArray<u8, U64>>> =
+        hkdf_expand(&res, info).expect("Input is a valid size");
 
-    SymmetricCryptoKey::try_from(key.as_mut_slice()).unwrap()
+    SymmetricCryptoKey::try_from(key.as_mut_slice()).expect("Key is a valid size")
 }
 
 #[cfg(test)]

crates/bitwarden-crypto/src/keys/utils.rs

@@ -25,16 +25,13 @@ pub(super) fn derive_kdf_key(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<Sy
                     iterations.get(),
                     parallelism.get(),
                     Some(32),
-                )
-                .unwrap(),
+                )?,
             );
 
             let salt_sha = sha2::Sha256::new().chain_update(salt).finalize();
 
             let mut hash = [0u8; 32];
-            argon
-                .hash_password_into(secret, &salt_sha, &mut hash)
-                .unwrap();
+            argon.hash_password_into(secret, &salt_sha, &mut hash)?;
             hash
         }
     };

crates/bitwarden-exporters/Cargo.toml

@@ -27,3 +27,6 @@ serde = { version = ">=1.0, <2.0", features = ["derive"] }
 serde_json = ">=1.0.96, <2.0"
 thiserror = ">=1.0.40, <2.0"
 uuid = { version = ">=1.3.3, <2.0", features = ["serde", "v4"] }
+
+[lints]
+workspace = true

crates/bitwarden-exporters/src/csv.rs

@@ -48,7 +48,7 @@ pub(crate) fn export_csv(folders: Vec<Folder>, ciphers: Vec<Cipher>) -> Result<S
 
     let mut wtr = Writer::from_writer(vec![]);
     for row in rows {
-        wtr.serialize(row).unwrap();
+        wtr.serialize(row).expect("Serialize should be infallible");
     }
 
     String::from_utf8(wtr.into_inner().map_err(|_| CsvError::Csv)?).map_err(|_| CsvError::Csv)

crates/bitwarden-generators/Cargo.toml

@@ -32,3 +32,6 @@ uniffi = { version = "=0.26.1", optional = true }
 rand_chacha = "0.3.1"
 tokio = { version = "1.36.0", features = ["rt", "macros"] }
 wiremock = "0.6.0"
+
+[lints]
+workspace = true

crates/bitwarden-json/Cargo.toml

@@ -27,3 +27,6 @@ serde = { version = ">=1.0, <2.0", features = ["derive"] }
 serde_json = ">=1.0.96, <2.0"
 
 bitwarden = { workspace = true }
+
+[lints]
+workspace = true

crates/bitwarden-json/src/response.rs

@@ -57,7 +57,7 @@ impl<T: Serialize + JsonSchema> ResponseIntoString for Response<T> {
             Ok(ser) => ser,
             Err(e) => {
                 let error = Response::error(format!("Failed to serialize Response: {}", e));
-                serde_json::to_string(&error).unwrap()
+                serde_json::to_string(&error).expect("Serialize should be infallible")
             }
         }
     }

crates/bitwarden-napi/Cargo.toml

@@ -32,3 +32,6 @@ napi-build = "2.1.0"
 
 [profile.release]
 lto = true
+
+[lints]
+workspace = true

crates/bitwarden-py/Cargo.toml

@@ -30,3 +30,6 @@ pyo3-asyncio = { version = "0.20.0", features = [
     "attributes",
     "tokio-runtime",
 ] }
+
+[lints]
+workspace = true

crates/bitwarden-uniffi/Cargo.toml

@@ -33,3 +33,6 @@ bitwarden-generators = { workspace = true, features = ["mobile"] }
 
 [build-dependencies]
 uniffi = { version = "=0.26.1", features = ["build"] }
+
+[lints]
+workspace = true

crates/bitwarden-wasm/Cargo.toml

@@ -30,3 +30,6 @@ bitwarden-json = { path = "../bitwarden-json", features = [
 
 [dev-dependencies]
 wasm-bindgen-test = "0.3.41"
+
+[lints]
+workspace = true

crates/bitwarden/Cargo.toml

@@ -84,3 +84,6 @@ rand_chacha = "0.3.1"
 tokio = { version = "1.36.0", features = ["rt", "macros"] }
 wiremock = "0.6.0"
 zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }
+
+[lints]
+workspace = true

crates/bitwarden/src/auth/api/request/mod.rs

@@ -54,7 +54,7 @@ async fn send_identity_connect_request(
     }
 
     let response = request
-        .body(serde_qs::to_string(&body).unwrap())
+        .body(serde_qs::to_string(&body).expect("Serialize should be infallible"))
         .send()
         .await?;
 

crates/bitwarden/src/auth/login/auth_request.rs

@@ -87,7 +87,7 @@ pub(crate) async fn complete_auth_request(
 
     if let IdentityTokenResponse::Authenticated(r) = response {
         let kdf = Kdf::PBKDF2 {
-            iterations: NonZeroU32::new(600_000).unwrap(),
+            iterations: NonZeroU32::new(600_000).expect("Non-zero number"),
         };
 
         client.set_tokens(

crates/bitwarden/src/client/client.rs

@@ -108,16 +108,17 @@ impl Client {
             client_builder
         }
 
-        let external_client = new_client_builder().build().unwrap();
+        let external_client = new_client_builder().build().expect("Build should not fail");
 
         let mut headers = header::HeaderMap::new();
         headers.append(
             "Device-Type",
-            HeaderValue::from_str(&(settings.device_type as u8).to_string()).unwrap(),
+            HeaderValue::from_str(&(settings.device_type as u8).to_string())
+                .expect("All numbers are valid ASCII"),
         );
         let client_builder = new_client_builder().default_headers(headers);
 
-        let client = client_builder.build().unwrap();
+        let client = client_builder.build().expect("Build should not fail");
 
         let identity = bitwarden_api_identity::apis::configuration::Configuration {
             base_path: settings.identity_url,
@@ -262,7 +263,10 @@ impl Client {
             user_key,
             private_key,
         )?);
-        Ok(self.encryption_settings.as_ref().unwrap())
+        Ok(self
+            .encryption_settings
+            .as_ref()
+            .expect("Value is initialized previously"))
     }
 
     #[cfg(feature = "internal")]
@@ -278,7 +282,7 @@ impl Client {
         Ok(self
             .encryption_settings
             .as_ref()
-            .expect("It was initialized on the previous line"))
+            .expect("Value is initialized previously"))
     }
 
     #[cfg(feature = "mobile")]
@@ -307,7 +311,9 @@ impl Client {
         key: SymmetricCryptoKey,
     ) -> &EncryptionSettings {
         self.encryption_settings = Some(EncryptionSettings::new_single_key(key));
-        self.encryption_settings.as_ref().unwrap()
+        self.encryption_settings
+            .as_ref()
+            .expect("Value is initialized previously")
     }
 
     #[cfg(feature = "internal")]
@@ -321,7 +327,7 @@ impl Client {
             .ok_or(Error::VaultLocked)?;
 
         enc.set_org_keys(org_keys)?;
-        Ok(self.encryption_settings.as_ref().unwrap())
+        Ok(&*enc)
     }
 }
 

crates/bitwarden/src/util.rs

@@ -6,19 +6,19 @@ use base64::{
 };
 
 pub fn default_pbkdf2_iterations() -> NonZeroU32 {
-    NonZeroU32::new(600_000).unwrap()
+    NonZeroU32::new(600_000).expect("Non-zero number")
 }
 #[cfg(feature = "internal")]
 pub fn default_argon2_iterations() -> NonZeroU32 {
-    NonZeroU32::new(3).unwrap()
+    NonZeroU32::new(3).expect("Non-zero number")
 }
 #[cfg(feature = "internal")]
 pub fn default_argon2_memory() -> NonZeroU32 {
-    NonZeroU32::new(64).unwrap()
+    NonZeroU32::new(64).expect("Non-zero number")
 }
 #[cfg(feature = "internal")]
 pub fn default_argon2_parallelism() -> NonZeroU32 {
-    NonZeroU32::new(4).unwrap()
+    NonZeroU32::new(4).expect("Non-zero number")
 }
 
 const INDIFFERENT: GeneralPurposeConfig =

crates/bitwarden/src/vault/totp.rs

@@ -220,8 +220,8 @@ fn decode_b32(s: &str) -> Vec<u8> {
     let mut bytes = Vec::new();
 
     for chunk in bits.as_bytes().chunks_exact(8) {
-        let byte_str = std::str::from_utf8(chunk).unwrap();
-        let byte = u8::from_str_radix(byte_str, 2).unwrap();
+        let byte_str = std::str::from_utf8(chunk).expect("The value is a valid string");
+        let byte = u8::from_str_radix(byte_str, 2).expect("The value is a valid binary string");
         bytes.push(byte);
     }
 

crates/bw/Cargo.toml

@@ -26,3 +26,6 @@ bitwarden-cli = { path = "../bitwarden-cli", version = "0.1.0" }
 
 [dev-dependencies]
 tempfile = "3.10.0"
+
+[lints]
+workspace = true

crates/bws/Cargo.toml

@@ -46,3 +46,6 @@ bitwarden = { workspace = true, features = ["secrets"] }
 
 [dev-dependencies]
 tempfile = "3.10.0"
+
+[lints]
+workspace = true