1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

Run clippy and rustfmt on CI (#12388)

* Run clippy and rustfmt on CI

* Error on warnings and fix a couple of missed lints

* Move import inside function

* Fix unix lints

* Fix windows lints

* Missed some async tests

* Remove unneeded reference
This commit is contained in:
Daniel García
2024-12-19 22:49:45 +01:00
committed by GitHub
parent 40943b5929
commit fff412665f
23 changed files with 132 additions and 128 deletions

View File

@@ -58,3 +58,31 @@ jobs:
run: |
npm ci
npm run lint
rust:
name: Run Rust lint on ${{ matrix.os }}
runs-on: ${{ matrix.os || 'ubuntu-latest' }}
strategy:
matrix:
os:
- ubuntu-latest
- macos-latest
- windows-latest
steps:
- name: Checkout repo
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Check Rust version
run: rustup --version
- name: Run cargo fmt
working-directory: ./apps/desktop/desktop_native
run: cargo fmt --check
- name: Run Clippy
working-directory: ./apps/desktop/desktop_native
run: cargo clippy --all-features --tests
env:
RUSTFLAGS: "-D warnings"

View File

@@ -1,3 +1,4 @@
#[allow(clippy::module_inception)]
#[cfg_attr(target_os = "linux", path = "unix.rs")]
#[cfg_attr(target_os = "windows", path = "windows.rs")]
#[cfg_attr(target_os = "macos", path = "macos.rs")]

View File

@@ -1,5 +1,5 @@
use anyhow::Result;
pub async fn run_command(value: String) -> Result<String> {
pub async fn run_command(_value: String) -> Result<String> {
todo!("Unix does not support autofill");
}

View File

@@ -1,5 +1,5 @@
use anyhow::Result;
pub async fn run_command(value: String) -> Result<String> {
pub async fn run_command(_value: String) -> Result<String> {
todo!("Windows does not support autofill");
}

View File

@@ -1,6 +1,7 @@
use aes::cipher::generic_array::GenericArray;
use anyhow::{anyhow, Result};
#[allow(clippy::module_inception)]
#[cfg_attr(target_os = "linux", path = "unix.rs")]
#[cfg_attr(target_os = "windows", path = "windows.rs")]
#[cfg_attr(target_os = "macos", path = "macos.rs")]
@@ -41,6 +42,7 @@ pub trait BiometricTrait {
) -> Result<String>;
}
#[allow(unused)]
fn encrypt(secret: &str, key_material: &KeyMaterial, iv_b64: &str) -> Result<String> {
let iv = base64_engine
.decode(iv_b64)?
@@ -52,9 +54,10 @@ fn encrypt(secret: &str, key_material: &KeyMaterial, iv_b64: &str) -> Result<Str
Ok(encrypted.to_string())
}
#[allow(unused)]
fn decrypt(secret: &CipherString, key_material: &KeyMaterial) -> Result<String> {
if let CipherString::AesCbc256_B64 { iv, data } = secret {
let decrypted = crypto::decrypt_aes256(&iv, &data, key_material.derive_key()?)?;
let decrypted = crypto::decrypt_aes256(iv, data, key_material.derive_key()?)?;
Ok(String::from_utf8(decrypted)?)
} else {

View File

@@ -33,12 +33,10 @@ impl super::BiometricTrait for Biometric {
.await;
match result {
Ok(result) => {
return Ok(result.is_authorized);
}
Ok(result) => Ok(result.is_authorized),
Err(e) => {
println!("polkit biometric error: {:?}", e);
return Ok(false);
Ok(false)
}
}
}
@@ -52,7 +50,7 @@ impl super::BiometricTrait for Biometric {
return Ok(true);
}
}
return Ok(false);
Ok(false)
}
fn derive_key_material(challenge_str: Option<&str>) -> Result<OsDerivedKey> {
@@ -68,8 +66,8 @@ impl super::BiometricTrait for Biometric {
// so we use a a key derived from the iv. this key is not intended to add any security
// but only a place-holder
let key = Sha256::digest(challenge);
let key_b64 = base64_engine.encode(&key);
let iv_b64 = base64_engine.encode(&challenge);
let key_b64 = base64_engine.encode(key);
let iv_b64 = base64_engine.encode(challenge);
Ok(OsDerivedKey { key_b64, iv_b64 })
}
@@ -100,7 +98,7 @@ impl super::BiometricTrait for Biometric {
let encrypted_secret = crate::password::get_password(service, account).await?;
let secret = CipherString::from_str(&encrypted_secret)?;
return Ok(decrypt(&secret, &key_material)?);
decrypt(&secret, &key_material)
}
}

View File

@@ -88,14 +88,14 @@ impl super::BiometricTrait for Biometric {
let bitwarden = h!("Bitwarden");
let result = KeyCredentialManager::RequestCreateAsync(
&bitwarden,
bitwarden,
KeyCredentialCreationOption::FailIfExists,
)?
.get()?;
let result = match result.Status()? {
KeyCredentialStatus::CredentialAlreadyExists => {
KeyCredentialManager::OpenAsync(&bitwarden)?.get()?
KeyCredentialManager::OpenAsync(bitwarden)?.get()?
}
KeyCredentialStatus::Success => result,
_ => return Err(anyhow!("Failed to create key credential")),
@@ -116,8 +116,8 @@ impl super::BiometricTrait for Biometric {
CryptographicBuffer::CopyToByteArray(&signature_buffer, &mut signature_value)?;
let key = Sha256::digest(&*signature_value);
let key_b64 = base64_engine.encode(&key);
let iv_b64 = base64_engine.encode(&challenge);
let key_b64 = base64_engine.encode(key);
let iv_b64 = base64_engine.encode(challenge);
Ok(OsDerivedKey { key_b64, iv_b64 })
}
@@ -151,12 +151,12 @@ impl super::BiometricTrait for Biometric {
Ok(secret) => {
// If the secret is a CipherString, it is encrypted and we need to decrypt it.
let secret = decrypt(&secret, &key_material)?;
return Ok(secret);
Ok(secret)
}
Err(_) => {
// If the secret is not a CipherString, it is not encrypted and we can return it
// directly.
return Ok(encrypted_secret);
Ok(encrypted_secret)
}
}
}
@@ -206,8 +206,8 @@ fn set_focus(window: HWND) {
pressed = true;
keybd_event(VK_MENU.0 as u8, 0, KEYEVENTF_EXTENDEDKEY, 0);
}
SetForegroundWindow(window);
SetFocus(window);
let _ = SetForegroundWindow(window);
let _ = SetFocus(window);
if pressed {
keybd_event(
VK_MENU.0 as u8,
@@ -245,20 +245,21 @@ mod tests {
assert_eq!(iv.len(), 16);
}
#[test]
#[tokio::test]
#[cfg(feature = "manual_test")]
fn test_prompt() {
async fn test_prompt() {
<Biometric as BiometricTrait>::prompt(
vec![0, 0, 0, 0, 0, 0, 0, 0],
String::from("Hello from Rust"),
)
.await
.unwrap();
}
#[test]
#[tokio::test]
#[cfg(feature = "manual_test")]
fn test_available() {
assert!(<Biometric as BiometricTrait>::available().unwrap())
async fn test_available() {
assert!(<Biometric as BiometricTrait>::available().await.unwrap())
}
#[test]
@@ -275,7 +276,7 @@ mod tests {
match secret {
CipherString::AesCbc256_B64 { iv, data: _ } => {
assert_eq!(iv_b64, base64_engine.encode(&iv));
assert_eq!(iv_b64, base64_engine.encode(iv));
}
_ => panic!("Invalid cipher string"),
}

View File

@@ -9,13 +9,9 @@ use crate::error::{CryptoError, KdfParamError, Result};
use super::CipherString;
pub fn decrypt_aes256(
iv: &[u8; 16],
data: &Vec<u8>,
key: GenericArray<u8, U32>,
) -> Result<Vec<u8>> {
pub fn decrypt_aes256(iv: &[u8; 16], data: &[u8], key: GenericArray<u8, U32>) -> Result<Vec<u8>> {
let iv = GenericArray::from_slice(iv);
let mut data = data.clone();
let mut data = data.to_vec();
let decrypted_key_slice = cbc::Decryptor::<aes::Aes256>::new(&key, iv)
.decrypt_padded_mut::<Pkcs7>(&mut data)
.map_err(|_| CryptoError::KeyDecrypt)?;
@@ -54,7 +50,7 @@ pub fn argon2(
let mut hash = [0u8; 32];
argon
.hash_password_into(secret, &salt, &mut hash)
.hash_password_into(secret, salt, &mut hash)
.map_err(|e| KdfParamError::InvalidParams(format!("Argon2 hashing failed: {e}",)))?;
// Argon2 is using some stack memory that is not zeroed. Eventually some function will

View File

@@ -2,4 +2,5 @@ pub use cipher_string::*;
pub use crypto::*;
mod cipher_string;
#[allow(clippy::module_inception)]
mod crypto;

View File

@@ -4,18 +4,18 @@ use security_framework::passwords::{
};
pub async fn get_password(service: &str, account: &str) -> Result<String> {
let result = String::from_utf8(get_generic_password(&service, &account)?)?;
let result = String::from_utf8(get_generic_password(service, account)?)?;
Ok(result)
}
pub async fn set_password(service: &str, account: &str, password: &str) -> Result<()> {
let result = set_generic_password(&service, &account, password.as_bytes())?;
Ok(result)
set_generic_password(service, account, password.as_bytes())?;
Ok(())
}
pub async fn delete_password(service: &str, account: &str) -> Result<()> {
let result = delete_generic_password(&service, &account)?;
Ok(result)
delete_generic_password(service, account)?;
Ok(())
}
pub async fn is_available() -> Result<bool> {

View File

@@ -1,3 +1,4 @@
#[allow(clippy::module_inception)]
#[cfg_attr(target_os = "linux", path = "unix.rs")]
#[cfg_attr(target_os = "windows", path = "windows.rs")]
#[cfg_attr(target_os = "macos", path = "macos.rs")]

View File

@@ -13,7 +13,7 @@ async fn get_password_new(service: &str, account: &str) -> Result<String> {
let keyring = oo7::Keyring::new().await?;
let attributes = HashMap::from([("service", service), ("account", account)]);
let results = keyring.search_items(&attributes).await?;
let res = results.get(0);
let res = results.first();
match res {
Some(res) => {
let secret = res.secret().await?;
@@ -31,7 +31,7 @@ async fn get_password_legacy(service: &str, account: &str) -> Result<String> {
let keyring = oo7::Keyring::DBus(collection);
let attributes = HashMap::from([("service", service), ("account", account)]);
let results = keyring.search_items(&attributes).await?;
let res = results.get(0);
let res = results.first();
match res {
Some(res) => {
let secret = res.secret().await?;

View File

@@ -42,7 +42,7 @@ pub async fn get_password<'a>(service: &str, account: &str) -> Result<String> {
.to_string_lossy()
};
Ok(String::from(password))
Ok(password)
}
pub async fn set_password(service: &str, account: &str, password: &str) -> Result<()> {

View File

@@ -1,3 +1,4 @@
#[allow(clippy::module_inception)]
#[cfg_attr(target_os = "linux", path = "linux.rs")]
#[cfg_attr(target_os = "windows", path = "unimplemented.rs")]
#[cfg_attr(target_os = "macos", path = "unimplemented.rs")]

View File

@@ -3,5 +3,5 @@ pub async fn on_lock(_: tokio::sync::mpsc::Sender<()>) -> Result<(), Box<dyn std
}
pub async fn is_lock_monitor_available() -> bool {
return false;
false
}

View File

@@ -1,3 +1,4 @@
#[allow(clippy::module_inception)]
#[cfg_attr(target_os = "linux", path = "linux.rs")]
#[cfg_attr(target_os = "windows", path = "windows.rs")]
#[cfg_attr(target_os = "macos", path = "macos.rs")]

View File

@@ -19,24 +19,24 @@ pub async fn generate_keypair(key_algorithm: String) -> Result<SshKey, anyhow::E
_ => return Err(anyhow::anyhow!("Unsupported RSA key size")),
};
let rsa_keypair = ssh_key::private::RsaKeypair::random(&mut rng, bits)
.or_else(|e| Err(anyhow::anyhow!(e.to_string())))?;
.map_err(|e| anyhow::anyhow!(e.to_string()))?;
let private_key = ssh_key::PrivateKey::new(
ssh_key::private::KeypairData::from(rsa_keypair),
"".to_string(),
)
.or_else(|e| Err(anyhow::anyhow!(e.to_string())))?;
.map_err(|e| anyhow::anyhow!(e.to_string()))?;
Ok(private_key)
}
_ => {
return Err(anyhow::anyhow!("Unsupported key algorithm"));
}
}
.or_else(|e| Err(anyhow::anyhow!(e.to_string())))?;
.map_err(|e| anyhow::anyhow!(e.to_string()))?;
let private_key_openssh = key
.to_openssh(LineEnding::LF)
.or_else(|e| Err(anyhow::anyhow!(e.to_string())))?;
.map_err(|e| anyhow::anyhow!(e.to_string()))?;
Ok(SshKey {
private_key: private_key_openssh.to_string(),
public_key: key.public_key().to_string(),

View File

@@ -27,61 +27,43 @@ pub fn import_key(
password: String,
) -> Result<SshKeyImportResult, anyhow::Error> {
match encoded_key.lines().next() {
Some(PKCS1_HEADER) => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::UnsupportedKeyType,
Some(PKCS1_HEADER) => Ok(SshKeyImportResult {
status: SshKeyImportStatus::UnsupportedKeyType,
ssh_key: None,
}),
Some(PKCS8_UNENCRYPTED_HEADER) => match import_pkcs8_key(encoded_key, None) {
Ok(result) => Ok(result),
Err(_) => Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
});
}
Some(PKCS8_UNENCRYPTED_HEADER) => {
return match import_pkcs8_key(encoded_key, None) {
Ok(result) => Ok(result),
Err(_) => Ok(SshKeyImportResult {
}),
},
Some(PKCS8_ENCRYPTED_HEADER) => match import_pkcs8_key(encoded_key, Some(password)) {
Ok(result) => Ok(result),
Err(err) => match err {
SshKeyImportError::PasswordRequired => Ok(SshKeyImportResult {
status: SshKeyImportStatus::PasswordRequired,
ssh_key: None,
}),
SshKeyImportError::WrongPassword => Ok(SshKeyImportResult {
status: SshKeyImportStatus::WrongPassword,
ssh_key: None,
}),
SshKeyImportError::ParsingError => Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
}),
};
}
Some(PKCS8_ENCRYPTED_HEADER) => match import_pkcs8_key(encoded_key, Some(password)) {
Ok(result) => {
return Ok(result);
}
Err(err) => match err {
SshKeyImportError::PasswordRequired => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::PasswordRequired,
ssh_key: None,
});
}
SshKeyImportError::WrongPassword => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::WrongPassword,
ssh_key: None,
});
}
SshKeyImportError::ParsingError => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
});
}
},
},
Some(OPENSSH_HEADER) => {
return import_openssh_key(encoded_key, password);
}
Some(_) => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
});
}
None => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
});
}
Some(OPENSSH_HEADER) => import_openssh_key(encoded_key, password),
Some(_) => Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
}),
None => Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
}),
}
}
@@ -152,14 +134,14 @@ fn import_pkcs8_key(
let pk: Ed25519Keypair =
Ed25519Keypair::from(Ed25519PrivateKey::from_bytes(&pk.secret_key));
let private_key = ssh_key::private::PrivateKey::from(pk);
return Ok(SshKeyImportResult {
Ok(SshKeyImportResult {
status: SshKeyImportStatus::Success,
ssh_key: Some(SshKey {
private_key: private_key.to_openssh(LineEnding::LF).unwrap().to_string(),
public_key: private_key.public_key().to_string(),
key_fingerprint: private_key.fingerprint(HashAlg::Sha256).to_string(),
}),
});
})
}
KeyType::Rsa => {
let pk: rsa::RsaPrivateKey = match password {
@@ -179,7 +161,7 @@ fn import_pkcs8_key(
match rsa_keypair {
Ok(rsa_keypair) => {
let private_key = ssh_key::private::PrivateKey::from(rsa_keypair);
return Ok(SshKeyImportResult {
Ok(SshKeyImportResult {
status: SshKeyImportStatus::Success,
ssh_key: Some(SshKey {
private_key: private_key
@@ -189,22 +171,18 @@ fn import_pkcs8_key(
public_key: private_key.public_key().to_string(),
key_fingerprint: private_key.fingerprint(HashAlg::Sha256).to_string(),
}),
});
}
Err(_) => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
});
})
}
Err(_) => Ok(SshKeyImportResult {
status: SshKeyImportStatus::ParsingError,
ssh_key: None,
}),
}
}
_ => {
return Ok(SshKeyImportResult {
status: SshKeyImportStatus::UnsupportedKeyType,
ssh_key: None,
});
}
_ => Ok(SshKeyImportResult {
status: SshKeyImportStatus::UnsupportedKeyType,
ssh_key: None,
}),
}
}

View File

@@ -129,7 +129,7 @@ impl BitwardenDesktopAgent {
.store(true, std::sync::atomic::Ordering::Relaxed);
for (key, name, cipher_id) in new_keys.iter() {
match parse_key_safe(&key) {
match parse_key_safe(key) {
Ok(private_key) => {
let public_key_bytes = private_key
.public_key()
@@ -187,10 +187,8 @@ impl BitwardenDesktopAgent {
return 0;
}
let request_id = self
.request_id
.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
request_id
self.request_id
.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
}
pub fn is_running(&self) -> bool {

View File

@@ -61,7 +61,7 @@ impl NamedPipeServerStream {
}
};
let peer_info = peerinfo::gather::get_peer_info(pid as u32);
let peer_info = peerinfo::gather::get_peer_info(pid);
let peer_info = match peer_info {
Err(err) => {
println!("Failed getting process info for pid {} {}", pid, err);

View File

@@ -89,11 +89,9 @@ pub mod biometrics {
account: String,
key_material: Option<KeyMaterial>,
) -> napi::Result<String> {
let result =
Biometric::get_biometric_secret(&service, &account, key_material.map(|m| m.into()))
.await
.map_err(|e| napi::Error::from_reason(e.to_string()));
result
Biometric::get_biometric_secret(&service, &account, key_material.map(|m| m.into()))
.await
.map_err(|e| napi::Error::from_reason(e.to_string()))
}
/// Derives key material from biometric data. Returns a string encoded with a
@@ -409,8 +407,8 @@ pub mod powermonitors {
.await
.map_err(|e| napi::Error::from_reason(e.to_string()))?;
tokio::spawn(async move {
while let Some(message) = rx.recv().await {
callback.call(Ok(message.into()), ThreadsafeFunctionCallMode::NonBlocking);
while let Some(()) = rx.recv().await {
callback.call(Ok(()), ThreadsafeFunctionCallMode::NonBlocking);
}
});
Ok(())
@@ -812,6 +810,6 @@ pub mod crypto {
desktop_core::crypto::argon2(&secret, &salt, iterations, memory, parallelism)
.map_err(|e| napi::Error::from_reason(e.to_string()))
.map(|v| v.to_vec())
.map(|v| Buffer::from(v))
.map(Buffer::from)
}
}

View File

@@ -1,7 +1,6 @@
use glob::glob;
#[cfg(target_os = "macos")]
fn main() {
use glob::glob;
let mut builder = cc::Build::new();
// Auto compile all .m files in the src/native directory

View File

@@ -100,7 +100,7 @@ mod objc {
}
};
return true;
true
}
}
@@ -115,7 +115,7 @@ pub async fn run_command(input: String) -> Result<String> {
unsafe { objc::runCommand(context.as_ptr(), c_input.as_ptr()) };
// Convert output from ObjC code to Rust string
let objc_output = rx.await?.try_into()?;
let objc_output = rx.await?;
// Convert output from ObjC code to Rust string
// let objc_output = output.try_into()?;