diff --git a/src-tauri/src/core/sftp/mod.rs b/src-tauri/src/core/sftp/mod.rs index 67bb3b84..b31f20e6 100644 --- a/src-tauri/src/core/sftp/mod.rs +++ b/src-tauri/src/core/sftp/mod.rs @@ -46,6 +46,10 @@ pub use util::{ RemoteTextFile, TextFileOpenResult, WriteRemoteTextResult, classify_text_file, }; +pub(crate) async fn probe_sftp_subsystem(ssh_handle: &Arc) -> AppResult<()> { + SftpBackend::probe(ssh_handle).await +} + #[derive(Debug, Clone, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum CopyEndpointKind { diff --git a/src-tauri/src/core/ssh/io.rs b/src-tauri/src/core/ssh/io.rs index 4818e3de..99a9de2e 100644 --- a/src-tauri/src/core/ssh/io.rs +++ b/src-tauri/src/core/ssh/io.rs @@ -286,6 +286,58 @@ async fn install_remote_shell_integration( .map(|_| ()) } +const CHANNEL_REQUEST_REPLY_TIMEOUT_MS: u64 = 10_000; + +async fn wait_channel_request_reply( + channel: &mut russh::Channel, + request_name: &str, +) -> AppResult<()> { + let reply = timeout( + Duration::from_millis(CHANNEL_REQUEST_REPLY_TIMEOUT_MS), + async { + loop { + match channel.wait().await { + Some(ChannelMsg::Success) => return Ok(()), + Some(ChannelMsg::Failure) => { + return Err(AppError::Channel(format!( + "{request_name} request rejected by server" + ))); + } + Some(ChannelMsg::Close | ChannelMsg::Eof) | None => { + return Err(AppError::Channel(format!( + "SSH channel closed before {request_name} request completed" + ))); + } + Some(_) => {} + } + } + }, + ) + .await; + + match reply { + Ok(result) => result, + Err(_) => Err(AppError::Channel(format!( + "{request_name} request timed out" + ))), + } +} + +async fn close_failed_interactive_channel( + channel: &russh::Channel, + session_id: &str, + stage: &str, +) { + if let Err(error) = channel.close().await { + tracing::debug!( + session_id = %session_id, + stage, + %error, + "Failed to close rejected SSH interactive channel" + ); + } +} + /// Opens a PTY shell channel and detects the remote shell type. /// /// Returns `(channel, Option, ready_marker)`. @@ -317,7 +369,7 @@ pub(super) async fn open_shell_channel( session_id = %session_id, "SSH interactive channel opening" ); - let channel = match handle.channel_open_session().await { + let mut channel = match handle.channel_open_session().await { Ok(channel) => { tracing::info!( session_id = %session_id, @@ -349,16 +401,28 @@ pub(super) async fn open_shell_channel( } } if let Some(fake_cookie_hex) = x11_fake_cookie_hex { - if let Err(error) = channel + match channel .request_x11(true, false, "MIT-MAGIC-COOKIE-1", fake_cookie_hex, 0) .await { - tracing::warn!( - session_id = %session_id, - %error, - "Could not enable X11 forwarding" - ); - local_notice = Some(super::x11_forwarding::enable_failed_message()); + Ok(()) => { + if let Err(error) = wait_channel_request_reply(&mut channel, "X11").await { + tracing::warn!( + session_id = %session_id, + %error, + "Could not enable X11 forwarding" + ); + local_notice = Some(super::x11_forwarding::enable_failed_message()); + } + } + Err(error) => { + tracing::warn!( + session_id = %session_id, + %error, + "Could not enable X11 forwarding" + ); + local_notice = Some(super::x11_forwarding::enable_failed_message()); + } } } @@ -373,7 +437,7 @@ pub(super) async fn open_shell_channel( "SSH PTY request sending" ); if let Err(error) = channel - .request_pty(false, terminal_type, 80, 24, 0, 0, &[]) + .request_pty(true, terminal_type, 80, 24, 0, 0, &[]) .await { tracing::warn!( @@ -381,8 +445,18 @@ pub(super) async fn open_shell_channel( %error, "SSH PTY request failed" ); + close_failed_interactive_channel(&channel, session_id, "pty-send").await; return Err(AppError::Channel(format!("PTY request failed: {}", error))); } + if let Err(error) = wait_channel_request_reply(&mut channel, "PTY").await { + tracing::warn!( + session_id = %session_id, + %error, + "SSH PTY request rejected" + ); + close_failed_interactive_channel(&channel, session_id, "pty-reply").await; + return Err(error); + } tracing::info!( session_id = %session_id, "SSH PTY request completed" @@ -395,17 +469,27 @@ pub(super) async fn open_shell_channel( session_id = %session_id, "SSH shell request sending" ); - if let Err(error) = channel.request_shell(false).await { + if let Err(error) = channel.request_shell(true).await { tracing::warn!( session_id = %session_id, %error, "SSH shell request failed" ); + close_failed_interactive_channel(&channel, session_id, "shell-send").await; return Err(AppError::Channel(format!( "Shell request failed: {}", error ))); } + if let Err(error) = wait_channel_request_reply(&mut channel, "Shell").await { + tracing::warn!( + session_id = %session_id, + %error, + "SSH shell request rejected" + ); + close_failed_interactive_channel(&channel, session_id, "shell-reply").await; + return Err(error); + } tracing::info!( session_id = %session_id, "SSH shell request completed" @@ -1568,6 +1652,86 @@ pub(super) async fn ssh_io_loop( let _ = app.emit(&closed_event, ()); } +async fn run_sftp_only_session_commands( + session_id: &str, + manager: &Arc, + mut cmd_rx: SessionCommandReceiver, + mut disconnect_rx: tokio::sync::mpsc::UnboundedReceiver, +) -> &'static str { + let close_reason = loop { + tokio::select! { + disconnect = disconnect_rx.recv() => { + match disconnect { + Some(reason) => { + tracing::info!( + session_id = %session_id, + %reason, + "SFTP-only SSH transport disconnected" + ); + break "remote-transport-disconnect"; + } + None => break "remote-transport-ended", + } + } + command = cmd_rx.recv() => match command { + Some(SessionCommand::AttachConfirmed { ack }) => { + let _ = ack.send(()); + } + Some(SessionCommand::CaptureExec { result_tx, .. }) => { + drop(result_tx); + } + Some(SessionCommand::Close) => break "local-close-request", + Some( + SessionCommand::DetachRenderer + | SessionCommand::Write { .. } + | SessionCommand::PauseOutput + | SessionCommand::ResumeOutput + | SessionCommand::AckOutput { .. } + | SessionCommand::Resize { .. } + | SessionCommand::CancelCapture { .. } + | SessionCommand::ZmodemAcceptDownload { .. } + | SessionCommand::ZmodemAcceptUpload { .. } + | SessionCommand::ZmodemCancel, + ) => {} + None => break "session-command-channel-closed", + } + } + }; + + manager.remove_session(session_id).await; + close_reason +} + +pub(super) async fn sftp_only_ssh_lifecycle_loop( + app: AppHandle, + session_id: String, + manager: Arc, + _handle: SshHandle, + cmd_rx: SessionCommandReceiver, + disconnect_rx: tokio::sync::mpsc::UnboundedReceiver, + connection_id: Option, +) { + tracing::info!(session_id = %session_id, "SFTP-only SSH lifecycle loop started"); + let close_reason = + run_sftp_only_session_commands(&session_id, &manager, cmd_rx, disconnect_rx).await; + + if let Some(ref conn_id) = connection_id { + if let Some(tunnel_mgr) = app.try_state::>() { + tunnel_mgr + .close_auto_tunnels_for_connection(&app, conn_id) + .await; + } + } + + tracing::info!( + session_id = %session_id, + close_reason, + "SFTP-only SSH session closed" + ); + let closed_event = format!("session-closed-{session_id}"); + let _ = app.emit(&closed_event, ()); +} + async fn handle_zmodem_actions( app: &AppHandle, event_name: &str, @@ -1658,15 +1822,20 @@ mod tests { append_suppressed_visible_and_take_passthrough, build_post_login_command_input, build_startup_command_input, discard_suppressed_output, fallback_shell_integration_timeout, handle_injection_result, handle_injection_timeout, injection_has_timed_out_at, - on_initial_injection_sent, open_shell_channel, should_send_initial_injection, + on_initial_injection_sent, open_shell_channel, run_sftp_only_session_commands, + should_send_initial_injection, }; - use crate::config::SftpCwdFollowMode; + use crate::config::{AiExecutionProfile, SftpCwdFollowMode, SshProfile}; use crate::core::ssh::osc::{OscResult, OscStripper, build_ready_marker}; + use crate::core::{ + DynamicTitleCapabilities, SessionCommand, SessionHandle, SessionInfo, SessionManager, + SessionType, session_command_channel, + }; use russh::{Channel, ChannelId, Disconnect, client, server}; use std::pin::Pin; use std::sync::Arc; use std::time::Instant; - use tokio::sync::mpsc; + use tokio::sync::{mpsc, oneshot}; use tokio::time::{Duration, Sleep, timeout}; struct TestClient; @@ -1682,8 +1851,18 @@ mod tests { } } + fn assert_fake_server_session(result: Result<(), russh::Error>) { + match result { + Ok(()) => {} + Err(russh::Error::IO(error)) if error.kind() == std::io::ErrorKind::BrokenPipe => {} + Err(error) => panic!("fake SSH server session: {error:?}"), + } + } + struct TestServer { agent_request_tx: mpsc::UnboundedSender<()>, + reject_shell: bool, + accept_x11: bool, } impl server::Handler for TestServer { @@ -1703,6 +1882,51 @@ mod tests { Ok(()) } + async fn pty_request( + &mut self, + channel: ChannelId, + _term: &str, + _col_width: u32, + _row_height: u32, + _pix_width: u32, + _pix_height: u32, + _modes: &[(russh::Pty, u32)], + session: &mut server::Session, + ) -> Result<(), Self::Error> { + let _ = session.channel_success(channel); + Ok(()) + } + + async fn shell_request( + &mut self, + channel: ChannelId, + session: &mut server::Session, + ) -> Result<(), Self::Error> { + if self.reject_shell { + let _ = session.channel_failure(channel); + } else { + let _ = session.channel_success(channel); + } + Ok(()) + } + + async fn x11_request( + &mut self, + channel: ChannelId, + _single_connection: bool, + _x11_auth_protocol: &str, + _x11_auth_cookie: &str, + _x11_screen_number: u32, + session: &mut server::Session, + ) -> Result<(), Self::Error> { + if self.accept_x11 { + let _ = session.channel_success(channel); + } else { + let _ = session.channel_failure(channel); + } + Ok(()) + } + async fn agent_request( &mut self, _channel: ChannelId, @@ -1730,11 +1954,15 @@ mod tests { let session = server::run_stream( server_config, server_stream, - TestServer { agent_request_tx }, + TestServer { + agent_request_tx, + reject_shell: false, + accept_x11: true, + }, ) .await .expect("fake SSH server handshake"); - session.await.expect("fake SSH server session"); + assert_fake_server_session(session.await); }); let mut handle = client::connect_stream( @@ -1792,12 +2020,369 @@ mod tests { .expect("fake SSH server task"); } + #[tokio::test] + async fn interactive_shell_rejection_is_reported() { + let (client_stream, server_stream) = tokio::io::duplex(1024 * 1024); + let (agent_request_tx, _agent_request_rx) = mpsc::unbounded_channel(); + let mut rng = russh::keys::key::safe_rng(); + let server_config = Arc::new(server::Config { + auth_rejection_time: Duration::ZERO, + auth_rejection_time_initial: Some(Duration::ZERO), + keys: vec![ + russh::keys::PrivateKey::random(&mut rng, russh::keys::Algorithm::Ed25519) + .expect("test server key"), + ], + ..server::Config::default() + }); + let server_task = tokio::spawn(async move { + let session = server::run_stream( + server_config, + server_stream, + TestServer { + agent_request_tx, + reject_shell: true, + accept_x11: true, + }, + ) + .await + .expect("fake SSH server handshake"); + assert_fake_server_session(session.await); + }); + + let mut handle = client::connect_stream( + Arc::new(client::Config::default()), + client_stream, + TestClient, + ) + .await + .expect("fake SSH client handshake"); + assert!( + handle + .authenticate_none("test") + .await + .expect("none authentication") + .success() + ); + + let result = open_shell_channel( + &mut handle, + "shell-rejection-test", + None, + false, + "xterm-256color", + true, + false, + SftpCwdFollowMode::Off, + 100, + None, + ) + .await; + let error = match result { + Ok(_) => panic!("rejected shell request must fail interactive setup"), + Err(error) => error, + }; + assert!( + error + .to_string() + .contains("Shell request rejected by server") + ); + + handle + .disconnect(Disconnect::ByApplication, "test complete", "") + .await + .expect("disconnect fake SSH session"); + timeout(Duration::from_secs(1), server_task) + .await + .expect("fake SSH server should stop") + .expect("fake SSH server task"); + } + #[tokio::test] async fn interactive_shell_requests_agent_forwarding_only_when_enabled() { assert_agent_forwarding_request(false).await; assert_agent_forwarding_request(true).await; } + #[tokio::test] + async fn x11_success_does_not_hide_shell_rejection() { + let (client_stream, server_stream) = tokio::io::duplex(1024 * 1024); + let (agent_request_tx, _agent_request_rx) = mpsc::unbounded_channel(); + let mut rng = russh::keys::key::safe_rng(); + let server_config = Arc::new(server::Config { + auth_rejection_time: Duration::ZERO, + auth_rejection_time_initial: Some(Duration::ZERO), + keys: vec![ + russh::keys::PrivateKey::random(&mut rng, russh::keys::Algorithm::Ed25519) + .expect("test server key"), + ], + ..server::Config::default() + }); + let server_task = tokio::spawn(async move { + let session = server::run_stream( + server_config, + server_stream, + TestServer { + agent_request_tx, + reject_shell: true, + accept_x11: true, + }, + ) + .await + .expect("fake SSH server handshake"); + assert_fake_server_session(session.await); + }); + + let mut handle = client::connect_stream( + Arc::new(client::Config::default()), + client_stream, + TestClient, + ) + .await + .expect("fake SSH client handshake"); + assert!( + handle + .authenticate_none("test") + .await + .expect("none authentication") + .success() + ); + + let error = open_shell_channel( + &mut handle, + "x11-shell-rejection-test", + Some("00112233445566778899aabbccddeeff"), + false, + "xterm-256color", + true, + false, + SftpCwdFollowMode::Off, + 100, + None, + ) + .await + .expect_err("Shell rejection must not be consumed as an earlier X11 reply"); + assert!( + error + .to_string() + .contains("Shell request rejected by server") + ); + + handle + .disconnect(Disconnect::ByApplication, "test complete", "") + .await + .expect("disconnect fake SSH session"); + timeout(Duration::from_secs(1), server_task) + .await + .expect("fake SSH server should stop") + .expect("fake SSH server task"); + } + + #[tokio::test] + async fn x11_rejection_is_non_fatal_when_pty_and_shell_succeed() { + let (client_stream, server_stream) = tokio::io::duplex(1024 * 1024); + let (agent_request_tx, _agent_request_rx) = mpsc::unbounded_channel(); + let mut rng = russh::keys::key::safe_rng(); + let server_config = Arc::new(server::Config { + auth_rejection_time: Duration::ZERO, + auth_rejection_time_initial: Some(Duration::ZERO), + keys: vec![ + russh::keys::PrivateKey::random(&mut rng, russh::keys::Algorithm::Ed25519) + .expect("test server key"), + ], + ..server::Config::default() + }); + let server_task = tokio::spawn(async move { + let session = server::run_stream( + server_config, + server_stream, + TestServer { + agent_request_tx, + reject_shell: false, + accept_x11: false, + }, + ) + .await + .expect("fake SSH server handshake"); + assert_fake_server_session(session.await); + }); + + let mut handle = client::connect_stream( + Arc::new(client::Config::default()), + client_stream, + TestClient, + ) + .await + .expect("fake SSH client handshake"); + assert!( + handle + .authenticate_none("test") + .await + .expect("none authentication") + .success() + ); + + let (channel, _, _, _, notice) = open_shell_channel( + &mut handle, + "x11-rejection-test", + Some("00112233445566778899aabbccddeeff"), + false, + "xterm-256color", + true, + false, + SftpCwdFollowMode::Off, + 100, + None, + ) + .await + .expect("X11 rejection must not fail an otherwise valid interactive shell"); + assert!(notice.is_some()); + + drop(channel); + handle + .disconnect(Disconnect::ByApplication, "test complete", "") + .await + .expect("disconnect fake SSH session"); + timeout(Duration::from_secs(1), server_task) + .await + .expect("fake SSH server should stop") + .expect("fake SSH server task"); + } + + #[tokio::test] + async fn sftp_only_session_close_cleans_up() { + let session_id = "sftp-only-session".to_string(); + let manager = Arc::new(SessionManager::new()); + let (cmd_tx, cmd_rx) = session_command_channel(session_id.clone()); + let info = SessionInfo { + id: session_id.clone(), + name: "sftp-only".to_string(), + session_type: SessionType::SSH, + started_at: String::new(), + connection_id: None, + connected: true, + owner_window_label: None, + ai_execution_profile: AiExecutionProfile::Disabled, + injection_active: false, + dynamic_title_capabilities: DynamicTitleCapabilities::default(), + remote_file_browser_enabled: true, + remote_stats_enabled: false, + ssh_profile: Some(SshProfile::Standard), + }; + manager + .add_session(SessionHandle { + info, + cmd_tx, + startup_input_barrier: None, + ssh_config: None, + ssh_handle: None, + cwd: Arc::new(tokio::sync::Mutex::new(Default::default())), + remote_fs: None, + }) + .await; + + let loop_manager = manager.clone(); + let loop_session_id = session_id.clone(); + let (_disconnect_tx, disconnect_rx) = mpsc::unbounded_channel(); + let loop_task = tokio::spawn(async move { + run_sftp_only_session_commands(&loop_session_id, &loop_manager, cmd_rx, disconnect_rx) + .await + }); + + let (attach_tx, attach_rx) = oneshot::channel(); + manager + .send_command( + &session_id, + SessionCommand::AttachConfirmed { ack: attach_tx }, + ) + .await + .expect("attach command"); + timeout(Duration::from_secs(1), attach_rx) + .await + .expect("attach acknowledgement must not block") + .expect("attach acknowledgement sender"); + + let (capture_tx, capture_rx) = oneshot::channel(); + manager + .send_command( + &session_id, + SessionCommand::CaptureExec { + marker_id: "capture".to_string(), + wrapped_command: b"echo blocked".to_vec(), + result_tx: capture_tx, + }, + ) + .await + .expect("capture command"); + let capture_result = timeout(Duration::from_secs(1), capture_rx) + .await + .expect("capture result sender must be dropped immediately"); + assert!(capture_result.is_err()); + + manager + .send_command(&session_id, SessionCommand::Close) + .await + .expect("close command"); + let close_reason = timeout(Duration::from_secs(1), loop_task) + .await + .expect("SFTP-only loop must close promptly") + .expect("SFTP-only loop task"); + + assert_eq!(close_reason, "local-close-request"); + assert!(manager.list_sessions().await.is_empty()); + } + + #[tokio::test] + async fn sftp_only_session_remote_disconnect_cleans_up() { + let session_id = "sftp-only-remote-disconnect".to_string(); + let manager = Arc::new(SessionManager::new()); + let (cmd_tx, cmd_rx) = session_command_channel(session_id.clone()); + let info = SessionInfo { + id: session_id.clone(), + name: "sftp-only".to_string(), + session_type: SessionType::SSH, + started_at: String::new(), + connection_id: None, + connected: true, + owner_window_label: None, + ai_execution_profile: AiExecutionProfile::Disabled, + injection_active: false, + dynamic_title_capabilities: DynamicTitleCapabilities::default(), + remote_file_browser_enabled: true, + remote_stats_enabled: false, + ssh_profile: Some(SshProfile::Standard), + }; + manager + .add_session(SessionHandle { + info, + cmd_tx, + startup_input_barrier: None, + ssh_config: None, + ssh_handle: None, + cwd: Arc::new(tokio::sync::Mutex::new(Default::default())), + remote_fs: None, + }) + .await; + + let (disconnect_tx, disconnect_rx) = mpsc::unbounded_channel(); + let loop_manager = manager.clone(); + let loop_session_id = session_id.clone(); + let loop_task = tokio::spawn(async move { + run_sftp_only_session_commands(&loop_session_id, &loop_manager, cmd_rx, disconnect_rx) + .await + }); + + disconnect_tx + .send("SSH server disconnected: test".to_string()) + .expect("disconnect notification"); + let close_reason = timeout(Duration::from_secs(1), loop_task) + .await + .expect("SFTP-only loop must react to remote disconnect") + .expect("SFTP-only loop task"); + + assert_eq!(close_reason, "remote-transport-disconnect"); + assert!(manager.list_sessions().await.is_empty()); + } + #[test] fn post_login_input_normalizes_line_endings_and_adds_enter() { let input = build_post_login_command_input("cd /opt/app\nclear").expect("input"); diff --git a/src-tauri/src/core/ssh/session.rs b/src-tauri/src/core/ssh/session.rs index 2777f604..d4901b4b 100644 --- a/src-tauri/src/core/ssh/session.rs +++ b/src-tauri/src/core/ssh/session.rs @@ -5,7 +5,7 @@ use super::client::{ SshHandle, SshHandler, SshRawHandle, SshStartupCommand, build_client_config, connect_via_stream, connect_with_proxy, }; -use super::io::{open_shell_channel, ssh_io_loop}; +use super::io::{open_shell_channel, sftp_only_ssh_lifecycle_loop, ssh_io_loop}; use crate::config::{ AiExecutionProfile, SshAgentForwardingConfig, SshAgentForwardingPolicy, SshProfile, SshRuntimeMode, effective_cwd_follow_mode_for_runtime, @@ -43,6 +43,47 @@ async fn create_authenticated_connection( .await } +async fn create_session_connection_with_disconnect( + app: &AppHandle, + config: &SshConfig, + enable_agent_forwarding: bool, + diagnostics: Option, +) -> AppResult<( + SshHandle, + Option>, + mpsc::UnboundedReceiver, +)> { + loop { + let (disconnect_tx, disconnect_rx) = mpsc::unbounded_channel(); + match create_authenticated_connection_with_notifications( + app, + config, + Some(disconnect_tx), + None, + enable_agent_forwarding, + diagnostics.clone(), + ) + .await + { + Err(error) if is_agent_auth_retry(&error) => continue, + Ok((handle, x11_rx)) => return Ok((handle, x11_rx, disconnect_rx)), + Err(error) => return Err(error), + } + } +} + +async fn create_sftp_only_replacement_connection( + app: &AppHandle, + config: &SshConfig, + diagnostics: &SshDiagnosticContext, +) -> AppResult<(SshHandle, mpsc::UnboundedReceiver)> { + let (connection, _x11_rx, disconnect_rx) = + create_session_connection_with_disconnect(app, config, false, Some(diagnostics.clone())) + .await?; + crate::core::sftp::probe_sftp_subsystem(&connection).await?; + Ok((connection, disconnect_rx)) +} + async fn create_authenticated_connection_with_notifications( app: &AppHandle, config: &SshConfig, @@ -360,6 +401,48 @@ fn resolve_runtime_capabilities(config: &SshConfig) -> SshRuntimeCapabilities { } } +async fn try_sftp_only_fallback( + session_id: &str, + remote_file_browser_enabled: bool, + shell_error: AppError, + probe: F, +) -> AppResult<()> +where + F: FnOnce() -> Fut, + Fut: Future>, +{ + if !remote_file_browser_enabled { + return Err(shell_error); + } + + match probe().await { + Ok(()) => { + tracing::info!( + session_id = %session_id, + shell_error = %shell_error, + "SSH shell unavailable; continuing with SFTP-only session" + ); + Ok(()) + } + Err(probe_error) => { + tracing::warn!( + session_id = %session_id, + shell_error = %shell_error, + sftp_probe_error = %probe_error, + "SFTP probe failed on current SSH transport" + ); + Err(shell_error) + } + } +} + +fn apply_sftp_only_runtime_policy(info: &mut SessionInfo) { + info.ai_execution_profile = AiExecutionProfile::Disabled; + info.injection_active = false; + info.remote_file_browser_enabled = true; + info.remote_stats_enabled = false; +} + /// Creates an authenticated SSH handle for a saved connection without opening a PTY/shell. /// Used by tunnels to establish their own independent SSH connections. #[allow(dead_code)] @@ -469,21 +552,9 @@ async fn create_ssh_session_inner( } else { None }; - let (ssh_connection, x11_rx) = loop { - match create_authenticated_connection_with_notifications( - &app, - &config, - None, - None, - true, - Some(diagnostics.clone()), - ) - .await - { - Err(error) if is_agent_auth_retry(&error) => continue, - result => break result, - } - }?; + let (mut ssh_connection, x11_rx, mut disconnect_rx) = + create_session_connection_with_disconnect(&app, &config, true, Some(diagnostics.clone())) + .await?; diagnostics.set_stage(SshDiagnosticStage::Authenticated); let capabilities = resolve_runtime_capabilities(&config); let effective_cwd_follow_mode = effective_cwd_follow_mode_for_runtime( @@ -511,28 +582,82 @@ async fn create_ssh_session_inner( let forwarding_enabled = should_attach_agent_forwarding(true, effective_forwarding_config(&config).enabled); - let (channel, injection_script, ready_marker, detected_shell, initial_notice) = - open_shell_channel( - &mut handle, - &session_id, - x11_config.as_ref().map(|cfg| cfg.fake_cookie_hex.as_str()), - forwarding_enabled, - config.terminal_type.as_str(), - capabilities.remote_file_browser_enabled, - capabilities.network_device_profile, - effective_cwd_follow_mode, - config.sftp.shell_detection_timeout_ms, - Some(diagnostics.clone()), - ) - .await?; + let shell_result = open_shell_channel( + &mut handle, + &session_id, + x11_config.as_ref().map(|cfg| cfg.fake_cookie_hex.as_str()), + forwarding_enabled, + config.terminal_type.as_str(), + capabilities.remote_file_browser_enabled, + capabilities.network_device_profile, + effective_cwd_follow_mode, + config.sftp.shell_detection_timeout_ms, + Some(diagnostics.clone()), + ) + .await; drop(handle); - let injection_active = injection_script.is_some(); + let mut sftp_only = false; + let shell = match shell_result { + Ok(shell) => Some(shell), + Err(shell_error) => { + let same_transport_probe = try_sftp_only_fallback( + &session_id, + capabilities.remote_file_browser_enabled, + shell_error, + || crate::core::sftp::probe_sftp_subsystem(&ssh_connection), + ) + .await; + match same_transport_probe { + Ok(()) => { + sftp_only = true; + None + } + Err(shell_error) => { + if !capabilities.remote_file_browser_enabled { + return Err(shell_error); + } + match create_sftp_only_replacement_connection(&app, &config, &diagnostics).await + { + Ok((replacement, replacement_disconnect_rx)) => { + ssh_connection = replacement; + disconnect_rx = replacement_disconnect_rx; + sftp_only = true; + tracing::info!( + session_id = %session_id, + "SFTP-only fallback recovered on a fresh authenticated transport" + ); + None + } + Err(reconnect_error) => { + tracing::warn!( + session_id = %session_id, + %reconnect_error, + "Fresh SSH transport could not establish SFTP-only fallback" + ); + return Err(shell_error); + } + } + } + } + } + }; + debug_assert_eq!(sftp_only, shell.is_none()); + let injection_active = shell + .as_ref() + .is_some_and(|(_, injection_script, _, _, _)| injection_script.is_some()); - if let (Some(rx), Some(x11_config)) = (x11_rx, x11_config) { - super::x11_forwarding::spawn_x11_forwarder(app.clone(), session_id.clone(), rx, x11_config); + if !sftp_only { + if let (Some(rx), Some(x11_config)) = (x11_rx, x11_config) { + super::x11_forwarding::spawn_x11_forwarder( + app.clone(), + session_id.clone(), + rx, + x11_config, + ); + } } - let session_info = SessionInfo { + let mut session_info = SessionInfo { id: session_id.clone(), name: config.name.clone(), session_type: SessionType::SSH, @@ -547,15 +672,16 @@ async fn create_ssh_session_inner( remote_stats_enabled: capabilities.remote_stats_enabled, ssh_profile: Some(config.ssh_profile.clone()), }; + if sftp_only { + apply_sftp_only_runtime_policy(&mut session_info); + } let cwd: SharedCwd = Arc::new(tokio::sync::Mutex::new(Default::default())); let ssh_config_arc: Arc = Arc::new(config.clone()); let ssh_handle_arc: Arc = ssh_connection.clone(); - let output_control_tx = cmd_tx.clone(); - let session_handle = SessionHandle { info: session_info.clone(), - cmd_tx, + cmd_tx: cmd_tx.clone(), startup_input_barrier: None, ssh_config: Some(ssh_config_arc), ssh_handle: Some(ssh_handle_arc), @@ -564,8 +690,10 @@ async fn create_ssh_session_inner( }; manager.add_session(session_handle).await; tracing::info!(session_id = %session_id, "SSH session registered"); - if let Some(hook) = session_ready_hook.as_ref() { - hook(&session_info); + if !sftp_only { + if let Some(hook) = session_ready_hook.as_ref() { + hook(&session_info); + } } if let Some(ref conn_id) = connection_id { @@ -582,36 +710,51 @@ async fn create_ssh_session_inner( } let io_session_id = session_id.clone(); - let io_manager = manager.clone(); - let io_handle = ssh_connection.clone(); - let io_connection_id = connection_id.clone(); - let post_login = config.post_login.clone(); - let startup_command = startup_command.clone(); - let backspace_mode = config.backspace_mode.clone(); - let encoding = config.encoding.clone(); - tokio::spawn(async move { - ssh_io_loop( - app, - io_session_id, - io_manager, - channel, - io_handle, - cmd_rx, - output_control_tx, - cwd, - io_connection_id, - injection_script, - ready_marker, - detected_shell, - post_login, - startup_command, - backspace_mode, - initial_notice, - encoding, - Some(diagnostics), - ) - .await; - }); + match shell { + Some((channel, injection_script, ready_marker, detected_shell, initial_notice)) => { + let output_control_tx = cmd_tx; + let post_login = config.post_login.clone(); + let backspace_mode = config.backspace_mode.clone(); + let encoding = config.encoding.clone(); + tokio::spawn(async move { + ssh_io_loop( + app, + io_session_id, + manager, + channel, + ssh_connection, + cmd_rx, + output_control_tx, + cwd, + connection_id, + injection_script, + ready_marker, + detected_shell, + post_login, + startup_command, + backspace_mode, + initial_notice, + encoding, + Some(diagnostics), + ) + .await; + }); + } + None => { + tokio::spawn(async move { + sftp_only_ssh_lifecycle_loop( + app, + io_session_id, + manager, + ssh_connection, + cmd_rx, + disconnect_rx, + connection_id, + ) + .await; + }); + } + } Ok(session_id) } @@ -807,15 +950,27 @@ pub async fn create_multiplexed_ssh_session( #[cfg(test)] mod tests { use super::{ - is_agent_auth_retry, is_raw_agent_forwarding_config, resolve_runtime_capabilities, - should_attach_agent_forwarding, + apply_sftp_only_runtime_policy, is_agent_auth_retry, is_raw_agent_forwarding_config, + resolve_runtime_capabilities, should_attach_agent_forwarding, try_sftp_only_fallback, }; use crate::config::{ - SftpSettings, SshAgentEndpoint, SshAgentForwardingConfig, SshAgentForwardingPolicy, - SshAgentForwardingSources, SshProfile, SshRuntimeMode, SshTerminalType, + SftpCwdFollowMode, SftpSettings, SshAgentEndpoint, SshAgentForwardingConfig, + SshAgentForwardingPolicy, SshAgentForwardingSources, SshProfile, SshRuntimeMode, + SshTerminalType, }; use crate::core::ssh::client::{SshAuth, SshConfig}; + use crate::core::{ + DynamicTitleCapabilities, SessionHandle, SessionInfo, SessionManager, SessionType, + session_command_channel, + }; use crate::error::AppError; + use russh::{Channel, ChannelId, ChannelMsg, ChannelOpenFailure, Disconnect, client, server}; + use russh_sftp::protocol::{File, FileAttributes, Handle, Name, Status, StatusCode, Version}; + use std::collections::HashMap; + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + use tokio::sync::Mutex; + use tokio::time::{Duration, timeout}; #[test] fn agent_forwarding_requires_both_global_and_connection_flags() { @@ -892,6 +1047,333 @@ mod tests { } } + struct SftpOnlyTestClient; + + impl client::Handler for SftpOnlyTestClient { + type Error = russh::Error; + + async fn check_server_key( + &mut self, + _server_public_key: &russh::keys::PublicKey, + ) -> Result { + Ok(true) + } + } + + #[derive(Clone, Copy)] + enum TestShellBehavior { + Reject, + AcceptWithoutExec, + Disconnect, + } + + fn assert_fake_server_session(result: Result<(), russh::Error>) { + match result { + Ok(()) => {} + Err(russh::Error::IO(error)) if error.kind() == std::io::ErrorKind::BrokenPipe => {} + Err(error) => panic!("fake SSH server session: {error:?}"), + } + } + + struct SftpOnlyTestServer { + channels: Arc>>>, + allow_sftp: bool, + subsystem_requests: Arc, + shell_behavior: TestShellBehavior, + single_session_channel: bool, + } + + impl server::Handler for SftpOnlyTestServer { + type Error = russh::Error; + + async fn auth_none(&mut self, _user: &str) -> Result { + Ok(server::Auth::Accept) + } + + async fn channel_open_session( + &mut self, + channel: Channel, + reply: server::ChannelOpenHandle, + _session: &mut server::Session, + ) -> Result<(), Self::Error> { + if self.single_session_channel && !self.channels.lock().await.is_empty() { + reply + .reject(ChannelOpenFailure::AdministrativelyProhibited) + .await; + return Ok(()); + } + self.channels.lock().await.insert(channel.id(), channel); + reply.accept().await; + Ok(()) + } + + async fn channel_close( + &mut self, + channel: ChannelId, + _session: &mut server::Session, + ) -> Result<(), Self::Error> { + self.channels.lock().await.remove(&channel); + Ok(()) + } + + async fn pty_request( + &mut self, + channel: ChannelId, + _term: &str, + _col_width: u32, + _row_height: u32, + _pix_width: u32, + _pix_height: u32, + _modes: &[(russh::Pty, u32)], + session: &mut server::Session, + ) -> Result<(), Self::Error> { + session.channel_success(channel)?; + Ok(()) + } + + async fn shell_request( + &mut self, + channel: ChannelId, + session: &mut server::Session, + ) -> Result<(), Self::Error> { + match self.shell_behavior { + TestShellBehavior::Reject => session.channel_failure(channel)?, + TestShellBehavior::AcceptWithoutExec => session.channel_success(channel)?, + TestShellBehavior::Disconnect => { + session.disconnect(Disconnect::ByApplication, "SFTP-only account", "")?; + } + } + Ok(()) + } + + async fn exec_request( + &mut self, + channel: ChannelId, + _data: &[u8], + session: &mut server::Session, + ) -> Result<(), Self::Error> { + session.channel_failure(channel)?; + Ok(()) + } + + async fn subsystem_request( + &mut self, + channel_id: ChannelId, + name: &str, + session: &mut server::Session, + ) -> Result<(), Self::Error> { + if name != "sftp" { + session.channel_failure(channel_id)?; + return Ok(()); + } + + self.subsystem_requests.fetch_add(1, Ordering::SeqCst); + if !self.allow_sftp { + session.channel_failure(channel_id)?; + return Ok(()); + } + + let Some(channel) = self.channels.lock().await.remove(&channel_id) else { + session.channel_failure(channel_id)?; + return Ok(()); + }; + session.channel_success(channel_id)?; + russh_sftp::server::run(channel.into_stream(), TestSftpSession::default()).await; + Ok(()) + } + } + + #[derive(Default)] + struct TestSftpSession { + root_dir_read_done: bool, + } + + impl russh_sftp::server::Handler for TestSftpSession { + type Error = StatusCode; + + fn unimplemented(&self) -> Self::Error { + StatusCode::OpUnsupported + } + + async fn init( + &mut self, + _version: u32, + _extensions: HashMap, + ) -> Result { + Ok(Version::new()) + } + + async fn close(&mut self, id: u32, _handle: String) -> Result { + Ok(Status { + id, + status_code: StatusCode::Ok, + error_message: "Ok".to_string(), + language_tag: "en-US".to_string(), + }) + } + + async fn opendir(&mut self, id: u32, path: String) -> Result { + self.root_dir_read_done = false; + Ok(Handle { id, handle: path }) + } + + async fn readdir(&mut self, id: u32, handle: String) -> Result { + if handle == "/" && !self.root_dir_read_done { + self.root_dir_read_done = true; + return Ok(Name { + id, + files: vec![File::new("upload.txt", FileAttributes::default())], + }); + } + Err(StatusCode::Eof) + } + + async fn realpath(&mut self, id: u32, _path: String) -> Result { + Ok(Name { + id, + files: vec![File::dummy("/home/sftp")], + }) + } + } + + async fn start_sftp_only_test_connection( + allow_sftp: bool, + shell_behavior: TestShellBehavior, + single_session_channel: bool, + ) -> ( + client::Handle, + tokio::task::JoinHandle<()>, + Arc, + ) { + let (client_stream, server_stream) = tokio::io::duplex(1024 * 1024); + let channels = Arc::new(Mutex::new(HashMap::new())); + let subsystem_requests = Arc::new(AtomicUsize::new(0)); + let mut rng = russh::keys::key::safe_rng(); + let server_config = Arc::new(server::Config { + auth_rejection_time: Duration::ZERO, + auth_rejection_time_initial: Some(Duration::ZERO), + keys: vec![ + russh::keys::PrivateKey::random(&mut rng, russh::keys::Algorithm::Ed25519) + .expect("test server key"), + ], + ..server::Config::default() + }); + let server_channels = channels.clone(); + let server_subsystem_requests = subsystem_requests.clone(); + let server_task = tokio::spawn(async move { + let session = server::run_stream( + server_config, + server_stream, + SftpOnlyTestServer { + channels: server_channels, + allow_sftp, + subsystem_requests: server_subsystem_requests, + shell_behavior, + single_session_channel, + }, + ) + .await + .expect("fake SSH server handshake"); + assert_fake_server_session(session.await); + }); + + let mut handle = client::connect_stream( + Arc::new(client::Config::default()), + client_stream, + SftpOnlyTestClient, + ) + .await + .expect("fake SSH client handshake"); + assert!( + handle + .authenticate_none("test") + .await + .expect("none authentication") + .success() + ); + (handle, server_task, subsystem_requests) + } + + async fn open_rejected_shell(handle: &mut client::Handle) -> AppError { + super::open_shell_channel( + handle, + "sftp-only-integration-test", + None, + false, + "xterm-256color", + true, + false, + SftpCwdFollowMode::Off, + 100, + None, + ) + .await + .expect_err("test server must reject the interactive shell") + } + + async fn probe_real_sftp( + handle: &client::Handle, + verify_file_browser_operations: bool, + ) -> Result<(), AppError> { + let mut channel = handle + .channel_open_session() + .await + .map_err(|error| AppError::Channel(format!("Failed to open SFTP channel: {error}")))?; + channel + .request_subsystem(true, "sftp") + .await + .map_err(|error| { + AppError::Channel(format!("Failed to request SFTP subsystem: {error}")) + })?; + + match timeout(Duration::from_secs(1), channel.wait()).await { + Ok(Some(ChannelMsg::Success)) => {} + Ok(Some(ChannelMsg::Failure)) => { + return Err(AppError::Channel( + "SFTP subsystem request rejected by server".to_string(), + )); + } + Ok(Some(_)) => { + return Err(AppError::Channel( + "Unexpected SSH response to SFTP subsystem request".to_string(), + )); + } + Ok(None) => { + return Err(AppError::Channel( + "SSH channel closed during SFTP subsystem request".to_string(), + )); + } + Err(_) => { + return Err(AppError::Channel( + "SFTP subsystem request timed out".to_string(), + )); + } + } + + let sftp = russh_sftp::client::SftpSession::new(channel.into_stream()).await?; + if verify_file_browser_operations { + assert_eq!(sftp.canonicalize(".").await?, "/home/sftp"); + let entries: Vec<_> = sftp.read_dir("/").await?.collect(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].file_name(), "upload.txt"); + } + sftp.close().await?; + Ok(()) + } + + async fn stop_sftp_only_test_connection( + handle: &client::Handle, + server_task: tokio::task::JoinHandle<()>, + ) { + let _ = handle + .disconnect(Disconnect::ByApplication, "test complete", "") + .await; + timeout(Duration::from_secs(1), server_task) + .await + .expect("fake SSH server should stop") + .expect("fake SSH server task"); + } + #[test] fn network_device_runtime_capabilities_disable_linux_only_features() { let config = test_config(SshProfile::NetworkDevice); @@ -927,6 +1409,187 @@ mod tests { assert!(!capabilities.network_device_profile); } + #[tokio::test] + async fn sftp_only_fallback_accepts_sftp_when_shell_is_rejected() { + let (mut handle, server_task, subsystem_requests) = + start_sftp_only_test_connection(true, TestShellBehavior::Reject, true).await; + let shell_error = open_rejected_shell(&mut handle).await; + + let result = try_sftp_only_fallback("test-session", true, shell_error, || { + probe_real_sftp(&handle, true) + }) + .await; + + assert!(result.is_ok()); + assert_eq!(subsystem_requests.load(Ordering::SeqCst), 1); + + let manager = SessionManager::new(); + let (cmd_tx, _cmd_rx) = session_command_channel("test-session".to_string()); + let mut info = SessionInfo { + id: "test-session".to_string(), + name: "sftp-only".to_string(), + session_type: SessionType::SSH, + started_at: String::new(), + connection_id: None, + connected: true, + owner_window_label: None, + ai_execution_profile: crate::config::AiExecutionProfile::Posix, + injection_active: true, + dynamic_title_capabilities: DynamicTitleCapabilities::default(), + remote_file_browser_enabled: false, + remote_stats_enabled: true, + ssh_profile: Some(SshProfile::Standard), + }; + apply_sftp_only_runtime_policy(&mut info); + manager + .add_session(SessionHandle { + info, + cmd_tx, + startup_input_barrier: None, + ssh_config: None, + ssh_handle: None, + cwd: Arc::new(Mutex::new(Default::default())), + remote_fs: None, + }) + .await; + let registered = manager.list_sessions().await; + assert_eq!(registered.len(), 1); + assert_eq!(registered[0].session_type, SessionType::SSH); + assert!(registered[0].connected); + assert!(registered[0].remote_file_browser_enabled); + assert!(!registered[0].remote_stats_enabled); + assert_eq!( + registered[0].ai_execution_profile, + crate::config::AiExecutionProfile::Disabled + ); + + stop_sftp_only_test_connection(&handle, server_task).await; + } + + #[tokio::test] + async fn interactive_shell_is_preserved_when_shell_detection_exec_is_rejected() { + let (mut shell_handle, shell_server_task, subsystem_requests) = + start_sftp_only_test_connection(true, TestShellBehavior::AcceptWithoutExec, false) + .await; + + let (channel, injection_script, _, detected_shell, _) = super::open_shell_channel( + &mut shell_handle, + "interactive-shell-detection-rejection-test", + None, + false, + "xterm-256color", + true, + false, + SftpCwdFollowMode::ShellIntegration, + 50, + None, + ) + .await + .expect("server accepts the shell request"); + assert!(injection_script.is_none()); + assert!(detected_shell.is_none()); + probe_real_sftp(&shell_handle, true) + .await + .expect("SFTP must remain available alongside the accepted interactive shell"); + assert_eq!(subsystem_requests.load(Ordering::SeqCst), 1); + + let _ = channel.close().await; + stop_sftp_only_test_connection(&shell_handle, shell_server_task).await; + } + + #[tokio::test] + async fn sftp_only_fallback_recovers_after_shell_attempt_disconnects_transport() { + let (mut broken_handle, broken_server_task, _subsystem_requests) = + start_sftp_only_test_connection(true, TestShellBehavior::Disconnect, false).await; + let shell_error = open_rejected_shell(&mut broken_handle).await; + assert!( + shell_error.to_string().contains("Shell") + || shell_error.to_string().contains("channel") + || shell_error.to_string().contains("SSH"), + "disconnecting shell attempt must fail interactive setup" + ); + assert!( + probe_real_sftp(&broken_handle, false).await.is_err(), + "disconnected transport must not be reusable for SFTP" + ); + + let (fresh_handle, fresh_server_task, fresh_subsystem_requests) = + start_sftp_only_test_connection(true, TestShellBehavior::Reject, false).await; + probe_real_sftp(&fresh_handle, true) + .await + .expect("fresh authenticated transport must recover SFTP-only access"); + assert_eq!(fresh_subsystem_requests.load(Ordering::SeqCst), 1); + + stop_sftp_only_test_connection(&fresh_handle, fresh_server_task).await; + stop_sftp_only_test_connection(&broken_handle, broken_server_task).await; + } + + #[tokio::test] + async fn sftp_only_fallback_preserves_shell_error_when_sftp_probe_fails() { + let (mut handle, server_task, subsystem_requests) = + start_sftp_only_test_connection(false, TestShellBehavior::Reject, false).await; + let shell_error = open_rejected_shell(&mut handle).await; + let shell_error_text = shell_error.to_string(); + let error = try_sftp_only_fallback("test-session", true, shell_error, || { + probe_real_sftp(&handle, false) + }) + .await + .expect_err("SFTP probe failure must preserve the shell error"); + + assert_eq!(error.to_string(), shell_error_text); + assert_eq!(subsystem_requests.load(Ordering::SeqCst), 1); + stop_sftp_only_test_connection(&handle, server_task).await; + } + + #[tokio::test] + async fn sftp_only_fallback_skips_probe_when_file_browser_is_disabled() { + let (mut handle, server_task, subsystem_requests) = + start_sftp_only_test_connection(true, TestShellBehavior::Reject, false).await; + let shell_error = open_rejected_shell(&mut handle).await; + let shell_error_text = shell_error.to_string(); + + let error = try_sftp_only_fallback("test-session", false, shell_error, || { + probe_real_sftp(&handle, false) + }) + .await + .expect_err("disabled file browser must not enable SFTP-only fallback"); + + assert_eq!(error.to_string(), shell_error_text); + assert_eq!(subsystem_requests.load(Ordering::SeqCst), 0); + stop_sftp_only_test_connection(&handle, server_task).await; + } + + #[test] + fn sftp_only_runtime_policy_disables_shell_capabilities() { + let mut info = SessionInfo { + id: "test-session".to_string(), + name: "test".to_string(), + session_type: SessionType::SSH, + started_at: String::new(), + connection_id: None, + connected: true, + owner_window_label: None, + ai_execution_profile: crate::config::AiExecutionProfile::Posix, + injection_active: true, + dynamic_title_capabilities: DynamicTitleCapabilities::default(), + remote_file_browser_enabled: false, + remote_stats_enabled: true, + ssh_profile: Some(SshProfile::Standard), + }; + + apply_sftp_only_runtime_policy(&mut info); + + assert_eq!(info.session_type, SessionType::SSH); + assert!(info.connected); + assert!(info.remote_file_browser_enabled); + assert!(!info.remote_stats_enabled); + assert!(!info.injection_active); + assert_eq!( + info.ai_execution_profile, + crate::config::AiExecutionProfile::Disabled + ); + } + #[test] fn ssh_runtime_mode_defaults_to_standard() { let config: SshConfig = serde_json::from_value(serde_json::json!({