diff --git a/apps/desktop/desktop_native/autotype/src/lib.rs b/apps/desktop/desktop_native/autotype/src/lib.rs index c87fea23b60d..b110d57780ce 100644 --- a/apps/desktop/desktop_native/autotype/src/lib.rs +++ b/apps/desktop/desktop_native/autotype/src/lib.rs @@ -1,3 +1,5 @@ +#![deny(clippy::pedantic, warnings)] + use anyhow::Result; #[cfg_attr(target_os = "linux", path = "linux.rs")] diff --git a/apps/desktop/desktop_native/core/src/autofill/macos.rs b/apps/desktop/desktop_native/core/src/autofill/macos.rs index 08f333abe934..000c8c8d5fb3 100644 --- a/apps/desktop/desktop_native/core/src/autofill/macos.rs +++ b/apps/desktop/desktop_native/core/src/autofill/macos.rs @@ -1,5 +1,10 @@ use anyhow::Result; +/// # Errors +/// +/// This function errors if any error occurs while executing +/// the `ObjC` command, or if converting the `value` argument +/// into a `CString`. pub async fn run_command(value: String) -> Result { desktop_objc::run_command(value).await } diff --git a/apps/desktop/desktop_native/core/src/autofill/mod.rs b/apps/desktop/desktop_native/core/src/autofill/mod.rs index aacec852e904..727f7bcc4d3c 100644 --- a/apps/desktop/desktop_native/core/src/autofill/mod.rs +++ b/apps/desktop/desktop_native/core/src/autofill/mod.rs @@ -1,3 +1,5 @@ +#![deny(clippy::pedantic, warnings)] + #[allow(clippy::module_inception)] #[cfg_attr(target_os = "linux", path = "unix.rs")] #[cfg_attr(target_os = "windows", path = "windows.rs")] diff --git a/apps/desktop/desktop_native/core/src/autofill/unix.rs b/apps/desktop/desktop_native/core/src/autofill/unix.rs index 937adb43e617..e2e5b5cc1be6 100644 --- a/apps/desktop/desktop_native/core/src/autofill/unix.rs +++ b/apps/desktop/desktop_native/core/src/autofill/unix.rs @@ -1,5 +1,8 @@ use anyhow::Result; +/// # Errors +/// +/// TODO #[allow(clippy::unused_async)] pub async fn run_command(_value: String) -> Result { todo!("Unix does not support autofill"); diff --git a/apps/desktop/desktop_native/core/src/autofill/windows.rs b/apps/desktop/desktop_native/core/src/autofill/windows.rs index 09dc68679312..edb2417c85d2 100644 --- a/apps/desktop/desktop_native/core/src/autofill/windows.rs +++ b/apps/desktop/desktop_native/core/src/autofill/windows.rs @@ -1,5 +1,8 @@ use anyhow::Result; +/// # Errors +/// +/// TODO #[allow(clippy::unused_async)] pub async fn run_command(_value: String) -> Result { todo!("Windows does not support autofill"); diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs index 61cb8fc187d8..e66ddd004109 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -1,3 +1,5 @@ +#![deny(clippy::pedantic, warnings)] + use std::{ collections::HashMap, sync::{ @@ -104,7 +106,7 @@ impl ssh_agent::Agent for Bitwarden request_parser::SshAgentSignRequest::SshSigRequest(ref req) => { Some(req.namespace.clone()) } - _ => None, + request_parser::SshAgentSignRequest::SignRequest(_) => None, }; info!( @@ -197,6 +199,9 @@ impl BitwardenDesktopAgent { } } + /// # Panics + /// + /// This function panics if the underlying `RwLock` is poisoned. pub fn stop(&self) { if !self.is_running() { error!("Tried to stop agent while it is not running"); @@ -212,9 +217,17 @@ impl BitwardenDesktopAgent { .clear(); } + /// # Errors + /// + /// This function returns an error if the agent is not running. + /// + /// # Panics + /// + /// This function panics if the underlying `RwLock` is poisoned or + /// if the cipher's public key is not serializable to bytes. pub fn set_keys( &mut self, - new_keys: Vec<(String, String, String)>, + new_keys: &[(&String, &String, &String)], ) -> Result<(), anyhow::Error> { if !self.is_running() { return Err(anyhow::anyhow!( @@ -228,7 +241,7 @@ impl BitwardenDesktopAgent { self.needs_unlock .store(true, std::sync::atomic::Ordering::Relaxed); - for (key, name, cipher_id) in new_keys.iter() { + for (key, name, cipher_id) in new_keys { match parse_key_safe(key) { Ok(private_key) => { let public_key_bytes = private_key @@ -239,8 +252,8 @@ impl BitwardenDesktopAgent { public_key_bytes, BitwardenSshKey { private_key: Some(private_key), - name: name.clone(), - cipher_uuid: cipher_id.clone(), + name: (*name).to_string(), + cipher_uuid: (*cipher_id).to_string(), }, ); } @@ -253,6 +266,13 @@ impl BitwardenDesktopAgent { Ok(()) } + /// # Errors + /// + /// This function returns an error if the agent is not running. + /// + /// # Panics + /// + /// This function panics if the underlying `RwLock` is poisoned. pub fn lock(&mut self) -> Result<(), anyhow::Error> { if !self.is_running() { return Err(anyhow::anyhow!( @@ -272,13 +292,14 @@ impl BitwardenDesktopAgent { Ok(()) } - pub fn clear_keys(&mut self) -> Result<(), anyhow::Error> { + /// # Panics + /// + /// This function panics if the underlying `RwLock` is poisoned. + pub fn clear_keys(&mut self) { let keystore = &mut self.keystore; keystore.0.write().expect("RwLock is not poisoned").clear(); self.needs_unlock .store(true, std::sync::atomic::Ordering::Relaxed); - - Ok(()) } fn get_request_id(&self) -> u32 { @@ -291,6 +312,7 @@ impl BitwardenDesktopAgent { .fetch_add(1, std::sync::atomic::Ordering::Relaxed) } + #[must_use] pub fn is_running(&self) -> bool { self.is_running.load(std::sync::atomic::Ordering::Relaxed) } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/named_pipe_listener_stream.rs b/apps/desktop/desktop_native/core/src/ssh_agent/named_pipe_listener_stream.rs index cb10e873a337..3afa79cf67f0 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/named_pipe_listener_stream.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/named_pipe_listener_stream.rs @@ -45,11 +45,11 @@ impl NamedPipeServerStream { loop { info!("Waiting for connection"); select! { - _ = cancellation_token.cancelled() => { + () = cancellation_token.cancelled() => { info!("[SSH Agent Native Module] Cancellation token triggered, stopping named pipe server"); break; - } - _ = listener.connect() => { + }, + Ok(()) = listener.connect() => { info!("[SSH Agent Native Module] Incoming connection"); let handle = HANDLE(listener.as_raw_handle()); let mut pid = 0; diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs index 77eec5e35c75..3d7581ca17eb 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs @@ -29,14 +29,14 @@ impl Stream for PeercredUnixListenerStream { Poll::Ready(Ok((stream, _))) => { let pid = match stream.peer_cred() { Ok(peer) => match peer.pid() { - Some(pid) => pid, + Some(pid) => u32::try_from(pid).expect("pid should not be negative."), None => { return Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))); } }, Err(_) => return Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))), }; - let peer_info = peerinfo::gather::get_peer_info(pid as u32); + let peer_info = peerinfo::gather::get_peer_info(pid); match peer_info { Ok(info) => Poll::Ready(Some(Ok((stream, info)))), Err(_) => Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))), diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/gather.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/gather.rs index 699203d613d7..af0a6c6f4573 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/gather.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/gather.rs @@ -1,16 +1,17 @@ use sysinfo::{Pid, System}; +use tracing::error; use super::models::PeerInfo; -pub fn get_peer_info(peer_pid: u32) -> Result { +/// +/// # Errors +/// +/// This function returns an error string if there is no matching process +/// for the provided `peer_pid`. +pub(crate) fn get_peer_info(peer_pid: u32) -> Result { let s = System::new_all(); if let Some(process) = s.process(Pid::from_u32(peer_pid)) { - let peer_process_name = match process.name().to_str() { - Some(name) => name.to_string(), - None => { - return Err("Failed to get process name".to_string()); - } - }; + let peer_process_name = process.name().to_string_lossy().to_string(); return Ok(PeerInfo::new( peer_pid, @@ -19,5 +20,6 @@ pub fn get_peer_info(peer_pid: u32) -> Result { )); } - Err("Failed to get process".to_string()) + error!(peer_pid, "No process matching peer PID."); + Err("No process matching PID".to_string()) } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs index fad535cb80e3..7c352be897e2 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs @@ -14,6 +14,7 @@ pub struct PeerInfo { } impl PeerInfo { + #[must_use] pub fn new(uid: u32, pid: u32, process_name: String) -> Self { Self { uid, @@ -24,6 +25,7 @@ impl PeerInfo { } } + #[must_use] pub fn unknown() -> Self { Self { uid: 0, @@ -34,18 +36,22 @@ impl PeerInfo { } } + #[must_use] pub fn uid(&self) -> u32 { self.uid } + #[must_use] pub fn pid(&self) -> u32 { self.pid } + #[must_use] pub fn process_name(&self) -> &str { &self.process_name } + #[must_use] pub fn is_forwarding(&self) -> bool { self.is_forwarding .load(std::sync::atomic::Ordering::Relaxed) @@ -56,11 +62,18 @@ impl PeerInfo { .store(value, std::sync::atomic::Ordering::Relaxed); } + /// # Panics + /// + /// This function panics if the underlying `host_key` mutex is poisoned. pub fn set_host_key(&self, host_key: Vec) { let mut host_key_lock = self.host_key.lock().expect("Mutex is not poisoned"); *host_key_lock = host_key; } + /// # Panics + /// + /// This function panics if the underlying `host_key` mutex is poisoned. + #[must_use] pub fn host_key(&self) -> Vec { self.host_key.lock().expect("Mutex is not poisoned").clone() } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs index 662a4658ede8..2a4346b0314b 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs @@ -7,6 +7,7 @@ use tokio::sync::Mutex; use super::{BitwardenDesktopAgent, SshAgentUIRequest}; impl BitwardenDesktopAgent { + #[allow(clippy::missing_errors_doc)] pub fn start_server( auth_request_tx: tokio::sync::mpsc::Sender, auth_response_rx: Arc>>, diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 39e57bd0bb5e..204492361c1e 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -407,14 +407,13 @@ pub mod sshagent { agent_state: &mut SshAgentState, new_keys: Vec, ) -> napi::Result<()> { + let keys: Vec<_> = new_keys + .iter() + .map(|k| (&k.private_key, &k.name, &k.cipher_id)) + .collect(); let bitwarden_agent_state = &mut agent_state.state; bitwarden_agent_state - .set_keys( - new_keys - .iter() - .map(|k| (k.private_key.clone(), k.name.clone(), k.cipher_id.clone())) - .collect(), - ) + .set_keys(&keys) .map_err(|e| napi::Error::from_reason(e.to_string()))?; Ok(()) } @@ -428,11 +427,9 @@ pub mod sshagent { } #[napi] - pub fn clear_keys(agent_state: &mut SshAgentState) -> napi::Result<()> { + pub fn clear_keys(agent_state: &mut SshAgentState) { let bitwarden_agent_state = &mut agent_state.state; - bitwarden_agent_state - .clear_keys() - .map_err(|e| napi::Error::from_reason(e.to_string())) + bitwarden_agent_state.clear_keys(); } }