From 54a53a1c348808c0dfeb917626887070d8b00c64 Mon Sep 17 00:00:00 2001 From: neuronull <9162534+neuronull@users.noreply.github.com> Date: Tue, 30 Sep 2025 06:33:32 -0600 Subject: [PATCH] Use tracing in ssh_agent (#16455) * [BEEEP][PM-255518] Use tracing for improved observability * feedback dani-garcia: use DefaultVisitor * set default log level * convert printlns in objc crate * convert printlns in autotype crate * convert printlns in autostart crate * convert printlns in core/password crate * convert printlns in core/biometric crate * convert printlns in napi crate * convert log usage in macos provider crate * convert existing log macros to tracing * fix the cargo.toml sort lint errors * Revert "fix the cargo.toml sort lint errors" This reverts commit fd149ab697d37ea8fc9c22db8f96684fc99bf2d8. * fix the sort lint using correct cargo sort version * feedback coltonhurst: more comments/clarity on behavior * revert changes to ssh_agent * Use tracing in ssh_agent --- .../desktop_native/core/src/ssh_agent/mod.rs | 23 ++++++++-------- .../ssh_agent/named_pipe_listener_stream.rs | 27 +++++++++---------- .../desktop_native/core/src/ssh_agent/unix.rs | 17 ++++++------ 3 files changed, 32 insertions(+), 35 deletions(-) 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 d038ba2277f..3440a0114ae 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -11,6 +11,7 @@ use bitwarden_russh::{ session_bind::SessionBindResult, ssh_agent::{self, SshKey}, }; +use tracing::{error, info}; #[cfg_attr(target_os = "windows", path = "windows.rs")] #[cfg_attr(target_os = "macos", path = "unix.rs")] @@ -86,7 +87,7 @@ impl ssh_agent::Agent info: &peerinfo::models::PeerInfo, ) -> bool { if !self.is_running() { - println!("[BitwardenDesktopAgent] Agent is not running, but tried to call confirm"); + error!("Agent is not running, but tried to call confirm"); return false; } @@ -94,7 +95,7 @@ impl ssh_agent::Agent let request_data = match request_parser::parse_request(data) { Ok(data) => data, Err(e) => { - println!("[SSH Agent] Error while parsing request: {e}"); + error!(error = %e, "Error while parsing request"); return false; } }; @@ -105,12 +106,12 @@ impl ssh_agent::Agent _ => None, }; - println!( - "[SSH Agent] Confirming request from application: {}, is_forwarding: {}, namespace: {}, host_key: {}", + info!( + is_forwarding = %info.is_forwarding(), + namespace = ?namespace.as_ref(), + host_key = %STANDARD.encode(info.host_key()), + "Confirming request from application: {}", info.process_name(), - info.is_forwarding(), - namespace.clone().unwrap_or_default(), - STANDARD.encode(info.host_key()) ); let mut rx_channel = self.get_ui_response_rx.lock().await.resubscribe(); @@ -172,7 +173,7 @@ impl ssh_agent::Agent connection_info.set_host_key(session_bind_info.host_key.clone()); } SessionBindResult::SignatureFailure => { - println!("[BitwardenDesktopAgent] Session bind failure: Signature failure"); + error!("Session bind failure: Signature failure"); } } } @@ -181,7 +182,7 @@ impl ssh_agent::Agent impl BitwardenDesktopAgent { pub fn stop(&self) { if !self.is_running() { - println!("[BitwardenDesktopAgent] Tried to stop agent while it is not running"); + error!("Tried to stop agent while it is not running"); return; } @@ -227,7 +228,7 @@ impl BitwardenDesktopAgent { ); } Err(e) => { - eprintln!("[SSH Agent Native Module] Error while parsing key: {e}"); + error!(error=%e, "Error while parsing key"); } } } @@ -265,7 +266,7 @@ impl BitwardenDesktopAgent { fn get_request_id(&self) -> u32 { if !self.is_running() { - println!("[BitwardenDesktopAgent] Agent is not running, but tried to get request id"); + error!("Agent is not running, but tried to get request id"); return 0; } 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 fccd7ca5ed6..cb10e873a33 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 @@ -14,6 +14,7 @@ use tokio::{ select, }; use tokio_util::sync::CancellationToken; +use tracing::{error, info}; use windows::Win32::{Foundation::HANDLE, System::Pipes::GetNamedPipeClientProcessId}; use crate::ssh_agent::peerinfo::{self, models::PeerInfo}; @@ -31,42 +32,38 @@ impl NamedPipeServerStream { pub fn new(cancellation_token: CancellationToken, is_running: Arc) -> Self { let (tx, rx) = tokio::sync::mpsc::channel(16); tokio::spawn(async move { - println!( - "[SSH Agent Native Module] Creating named pipe server on {}", - PIPE_NAME - ); + info!("Creating named pipe server on {}", PIPE_NAME); let mut listener = match ServerOptions::new().create(PIPE_NAME) { Ok(pipe) => pipe, - Err(err) => { - println!("[SSH Agent Native Module] Encountered an error creating the first pipe. The system's openssh service must likely be disabled"); - println!("[SSH Agent Natvie Module] error: {}", err); + Err(e) => { + error!(error = %e, "Encountered an error creating the first pipe. The system's openssh service must likely be disabled"); cancellation_token.cancel(); is_running.store(false, Ordering::Relaxed); return; } }; loop { - println!("[SSH Agent Native Module] Waiting for connection"); + info!("Waiting for connection"); select! { _ = cancellation_token.cancelled() => { - println!("[SSH Agent Native Module] Cancellation token triggered, stopping named pipe server"); + info!("[SSH Agent Native Module] Cancellation token triggered, stopping named pipe server"); break; } _ = listener.connect() => { - println!("[SSH Agent Native Module] Incoming connection"); + info!("[SSH Agent Native Module] Incoming connection"); let handle = HANDLE(listener.as_raw_handle()); let mut pid = 0; unsafe { if let Err(e) = GetNamedPipeClientProcessId(handle, &mut pid) { - println!("Error getting named pipe client process id {}", e); + error!(error = %e, pid, "Faile to get named pipe client process id"); continue } }; 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); + Err(e) => { + error!(error = %e, pid = %pid, "Failed getting process info"); continue }, Ok(info) => info, @@ -76,8 +73,8 @@ impl NamedPipeServerStream { listener = match ServerOptions::new().create(PIPE_NAME) { Ok(pipe) => pipe, - Err(err) => { - println!("[SSH Agent Native Module] Encountered an error creating a new pipe {}", err); + Err(e) => { + error!(error = %e, "Encountered an error creating a new pipe"); cancellation_token.cancel(); is_running.store(false, Ordering::Relaxed); return; diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs index 53142d4c476..813ebd61cc1 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs @@ -12,6 +12,7 @@ use bitwarden_russh::ssh_agent; use homedir::my_home; use tokio::{net::UnixListener, sync::Mutex}; use tokio_util::sync::CancellationToken; +use tracing::{error, info}; use crate::ssh_agent::peercred_unix_listener_stream::PeercredUnixListenerStream; @@ -36,14 +37,12 @@ impl BitwardenDesktopAgent { 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"); + info!("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" - ); + info!("Could not determine home directory"); return; } }; @@ -65,10 +64,10 @@ impl BitwardenDesktopAgent { } }; - println!("[SSH Agent Native Module] Starting SSH Agent server on {ssh_path:?}"); + info!(socket = %ssh_path, "Starting SSH Agent server"); 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}"); + error!(error = %e, socket = %ssh_path, "Could not remove existing socket file"); if e.kind() != std::io::ErrorKind::NotFound { return; } @@ -79,7 +78,7 @@ impl BitwardenDesktopAgent { // 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}"); + error!(error = %e, socket = ?sockname, "Could not set socket permissions"); return; } @@ -100,10 +99,10 @@ impl BitwardenDesktopAgent { cloned_agent_state .is_running .store(false, std::sync::atomic::Ordering::Relaxed); - println!("[SSH Agent Native Module] SSH Agent server exited"); + info!("SSH Agent server exited"); } Err(e) => { - eprintln!("[SSH Agent Native Module] Error while starting agent server: {e}"); + error!(error = %e, socket = %ssh_path, "Unable to start start agent server"); } } });