draft1: missing flatpak/snap support

apps/desktop/desktop_native/Cargo.lock

@@ -552,9 +552,9 @@ dependencies = [
 
 [[package]]
 name = "cfg-if"
-version = "1.0.0"
+version = "1.0.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
+checksum = "2fd1289c04a9ea8cb22300a459a72a385d7c73d3259e2ed7dcb2af674838cfa9"
 
 [[package]]
 name = "cfg_aliases"
@@ -879,6 +879,7 @@ dependencies = [
  "byteorder",
  "bytes",
  "cbc",
+ "cfg-if",
  "core-foundation",
  "desktop_objc",
  "dirs",

apps/desktop/desktop_native/Cargo.toml

@@ -26,6 +26,7 @@ bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", re
 byteorder = "=1.5.0"
 bytes = "=1.10.1"
 cbc = "=0.1.2"
+cfg-if = "=1.0.3"
 core-foundation = "=0.10.0"
 dirs = "=6.0.0"
 ed25519 = "=2.2.3"

apps/desktop/desktop_native/core/Cargo.toml

@@ -26,6 +26,7 @@ bitwarden-russh = { workspace = true }
 byteorder = { workspace = true }
 bytes = { workspace = true }
 cbc = { workspace = true, features = ["alloc"] }
+cfg-if = { workspace = true }
 dirs = { workspace = true }
 ed25519 = { workspace = true, features = ["pkcs8"] }
 futures = { workspace = true }

apps/desktop/desktop_native/core/src/ssh_agent/mod.rs

@@ -1,6 +1,9 @@
-use std::sync::{
-    atomic::{AtomicBool, AtomicU32},
-    Arc,
+use std::{
+    collections::HashMap,
+    sync::{
+        atomic::{AtomicBool, AtomicU32},
+        Arc, RwLock,
+    },
 };
 
 use base64::{engine::general_purpose::STANDARD, Engine as _};
@@ -24,8 +27,8 @@ pub mod peerinfo;
 mod request_parser;
 
 #[derive(Clone)]
-pub struct BitwardenDesktopAgent<Key> {
-    keystore: ssh_agent::KeyStore<Key>,
+pub struct BitwardenDesktopAgent {
+    keystore: ssh_agent::KeyStore<BitwardenSshKey>,
     cancellation_token: CancellationToken,
     show_ui_request_tx: tokio::sync::mpsc::Sender<SshAgentUIRequest>,
     get_ui_response_rx: Arc<Mutex<tokio::sync::broadcast::Receiver<(u32, bool)>>>,
@@ -76,9 +79,7 @@ impl SshKey for BitwardenSshKey {
     }
 }
 
-impl ssh_agent::Agent<peerinfo::models::PeerInfo, BitwardenSshKey>
-    for BitwardenDesktopAgent<BitwardenSshKey>
-{
+impl ssh_agent::Agent<peerinfo::models::PeerInfo, BitwardenSshKey> for BitwardenDesktopAgent {
     async fn confirm(
         &self,
         ssh_key: BitwardenSshKey,
@@ -178,7 +179,23 @@ impl ssh_agent::Agent<peerinfo::models::PeerInfo, BitwardenSshKey>
     }
 }
 
-impl BitwardenDesktopAgent<BitwardenSshKey> {
+impl BitwardenDesktopAgent {
+    /// Create a new `BitwardenDesktopAgent` from the provided auth channel handles.
+    pub fn new(
+        auth_request_tx: tokio::sync::mpsc::Sender<SshAgentUIRequest>,
+        auth_response_rx: Arc<Mutex<tokio::sync::broadcast::Receiver<(u32, bool)>>>,
+    ) -> Self {
+        Self {
+            keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))),
+            cancellation_token: CancellationToken::new(),
+            show_ui_request_tx: auth_request_tx,
+            get_ui_response_rx: auth_response_rx,
+            request_id: Arc::new(AtomicU32::new(0)),
+            needs_unlock: Arc::new(AtomicBool::new(true)),
+            is_running: Arc::new(AtomicBool::new(false)),
+        }
+    }
+
     pub fn stop(&self) {
         if !self.is_running() {
             println!("[BitwardenDesktopAgent] Tried to stop agent while it is not running");

apps/desktop/desktop_native/core/src/ssh_agent/unix.rs

@@ -1,113 +1,241 @@
-use std::{
-    collections::HashMap,
-    fs,
-    os::unix::fs::PermissionsExt,
-    sync::{
-        atomic::{AtomicBool, AtomicU32},
-        Arc, RwLock,
-    },
-};
+use std::{fs, os::unix::fs::PermissionsExt, path::PathBuf, sync::Arc};
 
+use anyhow::anyhow;
 use bitwarden_russh::ssh_agent;
 use homedir::my_home;
 use tokio::{net::UnixListener, sync::Mutex};
-use tokio_util::sync::CancellationToken;
 
 use crate::ssh_agent::peercred_unix_listener_stream::PeercredUnixListenerStream;
 
-use super::{BitwardenDesktopAgent, BitwardenSshKey, SshAgentUIRequest};
+use super::{BitwardenDesktopAgent, SshAgentUIRequest};
 
-impl BitwardenDesktopAgent<BitwardenSshKey> {
+/// User can override the default socket path with this env var
+const ENV_BITWARDEN_SSH_AUTH_SOCK: &str = "BITWARDEN_SSH_AUTH_SOCK";
+
+const FLATPACK_DATA_DIR: &str = ".var/app/com.bitwarden.desktop/data";
+
+const LEGACY_SOCKFILE_NAME: &str = ".bitwarden-ssh-agent.sock";
+
+const SOCKFILE_NAME: &str = "ssh-agent.sock";
+
+impl BitwardenDesktopAgent {
+    /// Starts the Bitwarden Desktop SSH Agent server.
+    /// # Errors
+    /// Will return `Err` if unable to create and set permissions for socket file path or
+    /// if unable to bind to the socket path.
     pub fn start_server(
         auth_request_tx: tokio::sync::mpsc::Sender<SshAgentUIRequest>,
         auth_response_rx: Arc<Mutex<tokio::sync::broadcast::Receiver<(u32, bool)>>>,
     ) -> Result<Self, anyhow::Error> {
-        let agent = BitwardenDesktopAgent {
-            keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))),
-            cancellation_token: CancellationToken::new(),
-            show_ui_request_tx: auth_request_tx,
-            get_ui_response_rx: auth_response_rx,
-            request_id: Arc::new(AtomicU32::new(0)),
-            needs_unlock: Arc::new(AtomicBool::new(true)),
-            is_running: Arc::new(AtomicBool::new(false)),
-        };
-        let cloned_agent_state = agent.clone();
-        tokio::spawn(async move {
-            let ssh_path = match std::env::var("BITWARDEN_SSH_AUTH_SOCK") {
-                Ok(path) => path,
-                Err(_) => {
-                    println!("[SSH Agent Native Module] BITWARDEN_SSH_AUTH_SOCK not set, using default path");
+        // socket_paths are one of the following:
+        //   - only the env var socket path if it is defined
+        //   - or use the legacy $HOME path if flatpak build
+        //   - or use both legacy and forward looking default paths.
+        // TODO: in PM<create ticket>, remove the legacy path logic and the for loop below.
+        let socket_paths = if let Ok(path) = std::env::var(ENV_BITWARDEN_SSH_AUTH_SOCK) {
+            vec![PathBuf::from(path)]
+        } else {
+            println!("[SSH Agent Native Module] {ENV_BITWARDEN_SSH_AUTH_SOCK} not set, using default path");
 
-                    let ssh_agent_directory = match my_home() {
-                        Ok(Some(home)) => home,
-                        _ => {
-                            println!(
-                                "[SSH Agent Native Module] Could not determine home directory"
-                            );
-                            return;
-                        }
-                    };
+            let mut paths = vec![get_legacy_default_socket_path()?];
 
-                    let is_flatpak = std::env::var("container") == Ok("flatpak".to_string());
-                    if !is_flatpak {
-                        ssh_agent_directory
-                            .join(".bitwarden-ssh-agent.sock")
-                            .to_str()
-                            .expect("Path should be valid")
-                            .to_owned()
-                    } else {
-                        ssh_agent_directory
-                            .join(".var/app/com.bitwarden.desktop/data/.bitwarden-ssh-agent.sock")
-                            .to_str()
-                            .expect("Path should be valid")
-                            .to_owned()
-                    }
-                }
-            };
+            // TODO: handle flatpak/snap
+            if !is_flatpak() {
+                let basedir = get_socket_basedir()?;
+                let socket_path = get_default_socket_path(basedir);
 
-            println!("[SSH Agent Native Module] Starting SSH Agent server on {ssh_path:?}");
-            let sockname = std::path::Path::new(&ssh_path);
-            if let Err(e) = std::fs::remove_file(sockname) {
-                println!("[SSH Agent Native Module] Could not remove existing socket file: {e}");
-                if e.kind() != std::io::ErrorKind::NotFound {
-                    return;
-                }
+                // create the bitwarden subdir if needed
+                fs::create_dir_all(
+                    socket_path
+                        .parent()
+                        .ok_or(anyhow!("Malformed default socket path."))?,
+                )
+                .map_err(|e| anyhow!(format!("Error creating {socket_path:?}: {e}")))?;
+
+                paths.push(socket_path);
             }
+            paths
+        };
 
-            match UnixListener::bind(sockname) {
+        let agent_state = BitwardenDesktopAgent::new(auth_request_tx, auth_response_rx);
+
+        for sock_path in &socket_paths {
+            // if the socket is already present and wasn't cleanly removed during a previous
+            // runtime, remove it before beginning anew.
+            remove_path(sock_path)?;
+
+            println!("[SSH Agent Native Module] Starting SSH Agent server {sock_path:?}");
+
+            match UnixListener::bind(sock_path) {
                 Ok(listener) => {
                     // Only the current user should be able to access the socket
-                    if let Err(e) = fs::set_permissions(sockname, fs::Permissions::from_mode(0o600))
-                    {
-                        println!("[SSH Agent Native Module] Could not set socket permissions: {e}");
-                        return;
-                    }
+                    set_user_permissions(sock_path)?;
 
                     let stream = PeercredUnixListenerStream::new(listener);
 
+                    let cloned_agent_state = agent_state.clone();
                     let cloned_keystore = cloned_agent_state.keystore.clone();
                     let cloned_cancellation_token = cloned_agent_state.cancellation_token.clone();
-                    cloned_agent_state
+
+                    tokio::spawn(async move {
+                        let _ = ssh_agent::serve(
+                            stream,
+                            cloned_agent_state.clone(),
+                            cloned_keystore,
+                            cloned_cancellation_token,
+                        )
+                        .await;
+
+                        cloned_agent_state
+                            .is_running
+                            .store(false, std::sync::atomic::Ordering::Relaxed);
+
+                        println!("[SSH Agent Native Module] SSH Agent server exited");
+                    });
+
+                    agent_state
                         .is_running
                         .store(true, std::sync::atomic::Ordering::Relaxed);
-                    let _ = ssh_agent::serve(
-                        stream,
-                        cloned_agent_state.clone(),
-                        cloned_keystore,
-                        cloned_cancellation_token,
-                    )
-                    .await;
-                    cloned_agent_state
-                        .is_running
-                        .store(false, std::sync::atomic::Ordering::Relaxed);
-                    println!("[SSH Agent Native Module] SSH Agent server exited");
+
+                    println!("[SSH Agent Native Module] SSH Agent is running: {sock_path:?}");
                 }
                 Err(e) => {
                     eprintln!("[SSH Agent Native Module] Error while starting agent server: {e}");
+                    return Err(e.into());
                 }
             }
-        });
+        }
 
-        Ok(agent)
+        Ok(agent_state)
+    }
+}
+
+fn is_flatpak() -> bool {
+    std::env::var("container") == Ok("flatpak".to_string())
+}
+
+// use the $HOME directory
+fn get_legacy_default_socket_path() -> Result<PathBuf, anyhow::Error> {
+    let Ok(Some(mut ssh_agent_directory)) = my_home() else {
+        println!("[SSH Agent Native Module] Could not determine home directory");
+        return Err(anyhow!("Could not determine home directory."));
+    };
+
+    if std::env::var("container") == Ok("flatpak".to_string()) {
+        ssh_agent_directory = ssh_agent_directory.join(FLATPACK_DATA_DIR);
+    }
+
+    ssh_agent_directory = ssh_agent_directory.join(LEGACY_SOCKFILE_NAME);
+
+    Ok(ssh_agent_directory)
+}
+
+// use the platform's base dir
+fn get_default_socket_path(mut path: PathBuf) -> PathBuf {
+    path.push("bitwarden");
+    path.push(SOCKFILE_NAME);
+    path
+}
+
+fn get_socket_basedir() -> Result<PathBuf, anyhow::Error> {
+    // currently the only delineation between mac and linux is this logic.
+    // if we need to expand that, we can make unix.rs the platform agnostic shared logic,
+    // and separate out into linux.rs and macos.rs
+    cfg_if::cfg_if! {
+        if #[cfg(target_os = "linux")] {
+            std::env::var("XDG_RUNTIME_DIR")
+                .map(|path| PathBuf::from(path))
+                .map_err(|_| { anyhow!("$XDG_RUNTIME_DIR is not defined.") })
+
+        } else if #[cfg(target_os = "macos")]{
+            Ok(std::env::temp_dir())
+
+        } else {
+            anyhow!("Unsupported platform");
+        }
+    }
+}
+
+fn set_user_permissions(path: &PathBuf) -> Result<(), anyhow::Error> {
+    fs::set_permissions(path, fs::Permissions::from_mode(0o600))
+        .map_err(|e| anyhow!("Could not set socket permissions for {path:?}: {e}"))
+}
+
+// try to remove the given path if it exists
+fn remove_path(path: &PathBuf) -> Result<(), anyhow::Error> {
+    if let Ok(true) = std::fs::exists(path) {
+        std::fs::remove_file(path).map_err(|e| anyhow!("Error removing socket {path:?}: {e}"))?;
+    }
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use rand::{distr::Alphanumeric, Rng};
+
+    use super::*;
+
+    #[test]
+    fn test_legacy_default_socket_path_success() {
+        let path = get_legacy_default_socket_path().unwrap();
+        let expected = PathBuf::from_iter([
+            std::env::var("HOME").unwrap(),
+            ".bitwarden-ssh-agent.sock".to_string(),
+        ]);
+        assert_eq!(path, expected);
+    }
+
+    #[test]
+    fn test_default_socket_path_success() {
+        let mut expected = PathBuf::from("/");
+        expected.push("bitwarden");
+        expected.push("ssh-agent.sock");
+
+        let path = get_default_socket_path(PathBuf::from("/"));
+
+        assert_eq!(path, expected);
+    }
+
+    fn rand_file_in_temp() -> PathBuf {
+        let mut path = std::env::temp_dir();
+        let s: String = rand::rng()
+            .sample_iter(&Alphanumeric)
+            .take(16)
+            .map(char::from)
+            .collect();
+        path.push(s);
+        path
+    }
+
+    #[test]
+    fn test_remove_path_exists_success() {
+        let path = rand_file_in_temp();
+        fs::write(&path, "").unwrap();
+        remove_path(&path).unwrap();
+
+        assert!(!fs::exists(&path).unwrap());
+    }
+
+    #[test]
+    fn test_remove_path_not_found_success() {
+        let path = rand_file_in_temp();
+        remove_path(&path).unwrap();
+
+        assert!(!fs::exists(&path).unwrap());
+    }
+
+    #[test]
+    fn test_sock_path_file_permissions() {
+        let path = rand_file_in_temp();
+        fs::write(&path, "").unwrap();
+
+        set_user_permissions(&path).unwrap();
+
+        let metadata = fs::metadata(&path).unwrap();
+        let permissions = metadata.permissions().mode();
+
+        assert_eq!(permissions, 0o100_600);
+
+        remove_path(&path).unwrap();
     }
 }

apps/desktop/desktop_native/core/src/ssh_agent/windows.rs

@@ -1,32 +1,18 @@
 use bitwarden_russh::ssh_agent;
 pub mod named_pipe_listener_stream;
 
-use std::{
-    collections::HashMap,
-    sync::{
-        atomic::{AtomicBool, AtomicU32},
-        Arc, RwLock,
-    },
-};
+use std::sync::Arc;
 use tokio::sync::Mutex;
-use tokio_util::sync::CancellationToken;
 
-use super::{BitwardenDesktopAgent, BitwardenSshKey, SshAgentUIRequest};
+use super::{BitwardenDesktopAgent, SshAgentUIRequest};
 
-impl BitwardenDesktopAgent<BitwardenSshKey> {
+impl BitwardenDesktopAgent {
     pub fn start_server(
         auth_request_tx: tokio::sync::mpsc::Sender<SshAgentUIRequest>,
         auth_response_rx: Arc<Mutex<tokio::sync::broadcast::Receiver<(u32, bool)>>>,
     ) -> Result<Self, anyhow::Error> {
-        let agent_state = BitwardenDesktopAgent {
-            keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))),
-            show_ui_request_tx: auth_request_tx,
-            get_ui_response_rx: auth_response_rx,
-            cancellation_token: CancellationToken::new(),
-            request_id: Arc::new(AtomicU32::new(0)),
-            needs_unlock: Arc::new(AtomicBool::new(true)),
-            is_running: Arc::new(AtomicBool::new(true)),
-        };
+        let agent_state = BitwardenDesktopAgent::new(auth_request_tx, auth_response_rx);
+
         let stream = named_pipe_listener_stream::NamedPipeServerStream::new(
             agent_state.cancellation_token.clone(),
             agent_state.is_running.clone(),

apps/desktop/desktop_native/napi/src/lib.rs

@@ -169,7 +169,6 @@ pub mod clipboards {
 pub mod sshagent {
     use std::sync::Arc;
 
-    use desktop_core::ssh_agent::BitwardenSshKey;
     use napi::{
         bindgen_prelude::Promise,
         threadsafe_function::{ErrorStrategy::CalleeHandled, ThreadsafeFunction},
@@ -178,7 +177,7 @@ pub mod sshagent {
 
     #[napi]
     pub struct SshAgentState {
-        state: desktop_core::ssh_agent::BitwardenDesktopAgent<BitwardenSshKey>,
+        state: desktop_core::ssh_agent::BitwardenDesktopAgent,
     }
 
     #[napi(object)]