Skip to content

Commit

Permalink
Run clippy and rustfmt on CI (#12388)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dani-garcia authored Dec 19, 2024
1 parent 40943b5 commit fff4126
Show file tree
Hide file tree
Showing 23 changed files with 132 additions and 128 deletions.
28 changes: 28 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
1 change: 1 addition & 0 deletions apps/desktop/desktop_native/core/src/autofill/mod.rs
Original file line number Diff line number Diff line change
@@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/desktop_native/core/src/autofill/unix.rs
Original file line number Diff line number Diff line change
@@ -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");
}
2 changes: 1 addition & 1 deletion apps/desktop/desktop_native/core/src/autofill/windows.rs
Original file line number Diff line number Diff line change
@@ -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");
}
5 changes: 4 additions & 1 deletion apps/desktop/desktop_native/core/src/biometric/mod.rs
Original file line number Diff line number Diff line change
@@ -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")]
Expand Down Expand Up @@ -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)?
Expand All @@ -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 {
Expand Down
14 changes: 6 additions & 8 deletions apps/desktop/desktop_native/core/src/biometric/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand All @@ -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> {
Expand All @@ -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 })
}

Expand Down Expand Up @@ -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)
}
}

Expand Down
29 changes: 15 additions & 14 deletions apps/desktop/desktop_native/core/src/biometric/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand All @@ -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 })
}

Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]
Expand All @@ -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"),
}
Expand Down
10 changes: 3 additions & 7 deletions apps/desktop/desktop_native/core/src/crypto/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/desktop_native/core/src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ pub use cipher_string::*;
pub use crypto::*;

mod cipher_string;
#[allow(clippy::module_inception)]
mod crypto;
10 changes: 5 additions & 5 deletions apps/desktop/desktop_native/core/src/password/macos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/desktop_native/core/src/password/mod.rs
Original file line number Diff line number Diff line change
@@ -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")]
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/desktop_native/core/src/password/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand All @@ -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?;
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/desktop_native/core/src/password/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/desktop_native/core/src/powermonitor/mod.rs
Original file line number Diff line number Diff line change
@@ -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")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
@@ -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")]
Expand Down
8 changes: 4 additions & 4 deletions apps/desktop/desktop_native/core/src/ssh_agent/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Loading

0 comments on commit fff4126

Please sign in to comment.