Skip to content

Commit

Permalink
Forbid unwrap usage outside of tests
Browse files Browse the repository at this point in the history
  • Loading branch information
dani-garcia committed Apr 3, 2024
1 parent a408745 commit 44b5d15
Show file tree
Hide file tree
Showing 32 changed files with 99 additions and 41 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
allow-unwrap-in-tests=true
allow-expect-in-tests=true
3 changes: 3 additions & 0 deletions crates/bitwarden-c/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ bitwarden-json = { path = "../bitwarden-json", features = ["secrets"] }

[dependencies]
env_logger = ">=0.10.0, <0.12"

[lints]
workspace = true
7 changes: 4 additions & 3 deletions crates/bitwarden-c/src/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Check warning on line 15 in crates/bitwarden-c/src/c.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-c/src/c.rs#L14-L15

Added lines #L14 - L15 were not covered by tests

let result = client.run_command(input_str).await;
match std::ffi::CString::new(result) {
Expand All @@ -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")

Check warning on line 33 in crates/bitwarden-c/src/c.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-c/src/c.rs#L32-L33

Added lines #L32 - L33 were not covered by tests
.to_owned();
box_ptr!(Client::new(Some(input_string)))
}
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion crates/bitwarden-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
3 changes: 2 additions & 1 deletion crates/bitwarden-crypto/src/aes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
6 changes: 3 additions & 3 deletions crates/bitwarden-crypto/src/enc_string/symmetric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Check warning on line 122 in crates/bitwarden-crypto/src/enc_string/symmetric.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L122 was not covered by tests
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 {
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
5 changes: 4 additions & 1 deletion crates/bitwarden-crypto/src/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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("-"))
Expand Down
7 changes: 4 additions & 3 deletions crates/bitwarden-crypto/src/keys/shareable_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ 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()
.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)]
Expand Down
7 changes: 2 additions & 5 deletions crates/bitwarden-crypto/src/keys/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
};
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-exporters/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion crates/bitwarden-exporters/src/csv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-generators/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions crates/bitwarden-json/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion crates/bitwarden-json/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Check warning on line 60 in crates/bitwarden-json/src/response.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-json/src/response.rs#L60

Added line #L60 was not covered by tests
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-napi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ napi-build = "2.1.0"

[profile.release]
lto = true

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/bitwarden-py/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ pyo3-asyncio = { version = "0.20.0", features = [
"attributes",
"tokio-runtime",
] }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/bitwarden-uniffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,6 @@ bitwarden-generators = { workspace = true, features = ["mobile"] }

[build-dependencies]
uniffi = { version = "=0.26.1", features = ["build"] }

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/bitwarden-wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ bitwarden-json = { path = "../bitwarden-json", features = [

[dev-dependencies]
wasm-bindgen-test = "0.3.41"

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/bitwarden/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion crates/bitwarden/src/auth/api/request/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;

Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/auth/login/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),

Check warning on line 90 in crates/bitwarden/src/auth/login/auth_request.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L90 was not covered by tests
};

client.set_tokens(
Expand Down
20 changes: 13 additions & 7 deletions crates/bitwarden/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")]
Expand All @@ -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")]
Expand Down Expand Up @@ -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")]
Expand All @@ -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)

Check warning on line 330 in crates/bitwarden/src/client/client.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/client/client.rs#L330

Added line #L330 was not covered by tests
}
}

Expand Down
8 changes: 4 additions & 4 deletions crates/bitwarden/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Check warning on line 13 in crates/bitwarden/src/util.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/util.rs#L13

Added line #L13 was not covered by tests
}
#[cfg(feature = "internal")]
pub fn default_argon2_memory() -> NonZeroU32 {
NonZeroU32::new(64).unwrap()
NonZeroU32::new(64).expect("Non-zero number")

Check warning on line 17 in crates/bitwarden/src/util.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/util.rs#L17

Added line #L17 was not covered by tests
}
#[cfg(feature = "internal")]
pub fn default_argon2_parallelism() -> NonZeroU32 {
NonZeroU32::new(4).unwrap()
NonZeroU32::new(4).expect("Non-zero number")

Check warning on line 21 in crates/bitwarden/src/util.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/util.rs#L21

Added line #L21 was not covered by tests
}

const INDIFFERENT: GeneralPurposeConfig =
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden/src/vault/totp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
3 changes: 3 additions & 0 deletions crates/bw/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@ bitwarden-cli = { path = "../bitwarden-cli", version = "0.1.0" }

[dev-dependencies]
tempfile = "3.10.0"

[lints]
workspace = true
3 changes: 3 additions & 0 deletions crates/bws/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,6 @@ bitwarden = { workspace = true, features = ["secrets"] }

[dev-dependencies]
tempfile = "3.10.0"

[lints]
workspace = true
4 changes: 2 additions & 2 deletions crates/bws/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ pub(crate) const DIRECTORY: &str = ".bws";

pub(crate) fn get_config_path(config_file: Option<&Path>, ensure_folder_exists: bool) -> PathBuf {
let config_file = config_file.map(ToOwned::to_owned).unwrap_or_else(|| {
let base_dirs = BaseDirs::new().unwrap();
let base_dirs = BaseDirs::new().expect("A valid home directory should exist");
base_dirs.home_dir().join(DIRECTORY).join(FILENAME)
});

if ensure_folder_exists {
if let Some(parent_folder) = config_file.parent() {
std::fs::create_dir_all(parent_folder).unwrap();
std::fs::create_dir_all(parent_folder).expect("Parent directory should be writable");

Check warning on line 57 in crates/bws/src/config.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/config.rs#L57

Added line #L57 was not covered by tests
}
}

Expand Down
Loading

0 comments on commit 44b5d15

Please sign in to comment.