diff --git a/apps/desktop/desktop_native/autotype/src/lib.rs b/apps/desktop/desktop_native/autotype/src/lib.rs index 92996996434..31721a55294 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 08f333abe93..000c8c8d5fb 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 aacec852e90..727f7bcc4d3 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 937adb43e61..e2e5b5cc1be 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 09dc6867931..ec0dc5d5e91 100644 --- a/apps/desktop/desktop_native/core/src/autofill/windows.rs +++ b/apps/desktop/desktop_native/core/src/autofill/windows.rs @@ -1,6 +1,9 @@ use anyhow::Result; -#[allow(clippy::unused_async)] +/// # Errors +/// +/// TODO +#[allow(clippy::unused_async, missing_errors_doc)] 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 3440a0114ae..98afd8d76fe 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::sync::{ atomic::{AtomicBool, AtomicU32}, Arc, @@ -103,7 +105,7 @@ impl ssh_agent::Agent request_parser::SshAgentSignRequest::SshSigRequest(ref req) => { Some(req.namespace.clone()) } - _ => None, + request_parser::SshAgentSignRequest::SignRequest(_) => None, }; info!( @@ -180,6 +182,9 @@ impl ssh_agent::Agent } 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"); @@ -195,10 +200,15 @@ impl BitwardenDesktopAgent { .clear(); } - pub fn set_keys( - &mut self, - new_keys: Vec<(String, String, String)>, - ) -> Result<(), anyhow::Error> { + /// # 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: &[(String, String, String)]) -> Result<(), anyhow::Error> { if !self.is_running() { return Err(anyhow::anyhow!( "[BitwardenDesktopAgent] Tried to set keys while agent is not running" @@ -211,7 +221,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 @@ -236,6 +246,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!( @@ -255,13 +272,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 { @@ -274,6 +292,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/peercred_unix_listener_stream.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs index 77eec5e35c7..3d7581ca17e 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 699203d613d..af0a6c6f457 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 fad535cb80e..7c352be897e 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/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 3e6a5f00ae2..ff38581dab6 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -312,11 +312,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(); } }