diff --git a/crates/shelldeck-ssh/src/tunnel.rs b/crates/shelldeck-ssh/src/tunnel.rs index 5ef85313..f829d950 100644 --- a/crates/shelldeck-ssh/src/tunnel.rs +++ b/crates/shelldeck-ssh/src/tunnel.rs @@ -82,10 +82,11 @@ impl TunnelManager { } /// Start a local port forward (SSH -L equivalent). - /// Binds to `local_port` and forwards connections to `remote_host:remote_port` through SSH. + /// Binds to `local_host:local_port` and forwards connections to `remote_host:remote_port` through SSH. pub async fn start_local_forward( &mut self, handle: SharedHandle, + local_host: String, local_port: u16, remote_host: String, remote_port: u16, @@ -93,7 +94,7 @@ impl TunnelManager { // Bind before reporting success. The previous availability probe then // spawned a second bind, leaving a TOCTOU window where the method // returned Ok even though no listener was ever created. - let listener = TcpListener::bind(("127.0.0.1", local_port)) + let listener = TcpListener::bind((local_host.as_str(), local_port)) .await .map_err(|error| match error.kind() { std::io::ErrorKind::AddrInUse => SshError::PortInUse(local_port), @@ -112,7 +113,8 @@ impl TunnelManager { tokio::spawn(async move { tracing::info!( - "Local forward: 127.0.0.1:{} -> {}:{}", + "Local forward: {}:{} -> {}:{}", + local_host, local_port, remote_host, remote_port @@ -1048,7 +1050,13 @@ mod tests { let local_port = unused_local_port().await; let mut manager = TunnelManager::new(); let id = manager - .start_local_forward(handle, local_port, "echo.internal".to_owned(), 4242) + .start_local_forward( + handle, + "127.0.0.1".to_owned(), + local_port, + "echo.internal".to_owned(), + 4242, + ) .await .expect("start local forward"); @@ -1096,6 +1104,53 @@ mod tests { server_task.abort(); } + // SDTEST-1942 + #[tokio::test] + async fn local_forward_uses_configured_bind_address_and_rejects_occupied_address() { + let occupied = TcpListener::bind(("::1", 0)).await.expect("IPv6 loopback"); + let port = occupied.local_addr().expect("address").port(); + let (handle, mut requests, server_task) = start_echo_server().await; + let mut manager = TunnelManager::new(); + let error = manager + .start_local_forward( + handle.clone(), + "::1".into(), + port, + "echo.internal".into(), + 4242, + ) + .await + .expect_err("configured address is occupied"); + assert!(matches!(error, crate::SshError::PortInUse(p) if p == port)); + assert!(manager.tunnels().is_empty()); + drop(occupied); + let id = manager + .start_local_forward(handle, "::1".into(), port, "echo.internal".into(), 4242) + .await + .expect("bind configured address"); + assert!( + TcpStream::connect(("127.0.0.1", port)).await.is_err(), + "must not silently listen on the default address" + ); + let mut client = TcpStream::connect(("::1", port)) + .await + .expect("configured listener"); + client.write_all(b"configured bind").await.expect("write"); + let mut reply = [0_u8; 15]; + client.read_exact(&mut reply).await.expect("echo"); + assert_eq!(&reply, b"configured bind"); + let request = timeout(Duration::from_secs(2), requests.recv()) + .await + .expect("request deadline") + .expect("SSH request"); + assert_eq!(request.host, "echo.internal"); + assert_eq!(request.port, 4242); + manager.get_tunnel(&id).expect("handle").stop(); + wait_until_stopped(&manager, id).await; + assert!(TcpStream::connect(("::1", port)).await.is_err()); + server_task.abort(); + } + // SDTEST-565 #[tokio::test] async fn remote_forward_requests_the_port_and_routes_connections_to_the_local_target() { @@ -1291,6 +1346,7 @@ mod tests { let error = manager .start_local_forward( handle.clone(), + "127.0.0.1".to_owned(), occupied_port, "echo.internal".to_owned(), 4242, @@ -1319,6 +1375,7 @@ mod tests { let first_id = manager .start_local_forward( handle.clone(), + "127.0.0.1".to_owned(), first_port, "first.internal".to_owned(), 1001, @@ -1326,7 +1383,13 @@ mod tests { .await .expect("start first tunnel"); let second_id = manager - .start_local_forward(handle, second_port, "second.internal".to_owned(), 1002) + .start_local_forward( + handle, + "127.0.0.1".to_owned(), + second_port, + "second.internal".to_owned(), + 1002, + ) .await .expect("start second tunnel"); let mut first = connect_to_tunnel(first_port).await; diff --git a/crates/shelldeck-ui/src/workspace/forwards.rs b/crates/shelldeck-ui/src/workspace/forwards.rs index aaa98ec9..4a7f4656 100644 --- a/crates/shelldeck-ui/src/workspace/forwards.rs +++ b/crates/shelldeck-ui/src/workspace/forwards.rs @@ -58,7 +58,9 @@ impl Workspace { }; // Don't start if already active - if self.active_tunnels.contains_key(&forward_id) { + if self.active_tunnels.contains_key(&forward_id) + || self.tunnel_starts.contains(forward_id) + { tracing::warn!("Port forward {} is already active", forward_id); return; } @@ -134,6 +136,10 @@ impl Workspace { let remote_port = forward.remote_port; let local_host = forward.local_host.clone(); + let (attempt, thread_shutdown_tx, mut thread_shutdown_rx) = + self.tunnel_starts.begin(forward_id); + let setup_shutdown_tx = thread_shutdown_tx.clone(); + // Spawn a dedicated thread with its own tokio runtime for the SSH tunnel. // The thread stays alive as long as the tunnel is running; the tokio runtime // drives the TcpListener accept loop inside start_local_forward/start_remote_forward. @@ -154,94 +160,96 @@ impl Workspace { }; rt.block_on(async move { - // Establish SSH connection - let client = SshClient::new(); - let mut session = match client.connect(&connection).await { - Ok(s) => s, - Err(e) => { - let msg = format!("SSH connection failed: {}", e); - tracing::error!("{}", msg); - let _ = result_tx.send(Err(msg)); - return; - } - }; - tracing::info!( - "SSH connected for tunnel to {}", - connection.display_name() - ); - - let shared_handle = session.shared_handle(); - let mut tunnel_manager = shelldeck_ssh::tunnel::TunnelManager::new(); - - let tunnel_result = match direction { - ForwardDirection::LocalToRemote => { - tunnel_manager - .start_local_forward( - shared_handle, - local_port, - remote_host, - remote_port, - ) - .await - } - ForwardDirection::RemoteToLocal => { - match session.take_forwarded_tcpip_rx() { - Some(forwarded_rx) => { - tunnel_manager - .start_remote_forward( - shared_handle, - remote_host, - remote_port, - local_host, - local_port, - forwarded_rx, + // Cancellation covers SSH authentication as well as listener setup. + let setup = async { + let client = SshClient::new(); + let mut session = client.connect(&connection).await?; + let shared_handle = session.shared_handle(); + let mut manager = shelldeck_ssh::tunnel::TunnelManager::new(); + let id = match direction { + ForwardDirection::LocalToRemote => { + manager + .start_local_forward( + shared_handle, + local_host, + local_port, + remote_host, + remote_port, + ) + .await? + } + ForwardDirection::RemoteToLocal => { + let rx = + session.take_forwarded_tcpip_rx().ok_or_else(|| { + shelldeck_ssh::SshError::Tunnel( + "remote forwarding channel already taken" + .to_string(), ) - .await - } - None => Err(shelldeck_ssh::SshError::Tunnel( - "remote forwarding channel already taken".to_string(), - )), + })?; + manager + .start_remote_forward( + shared_handle, + remote_host, + remote_port, + local_host, + local_port, + rx, + ) + .await? } - } - ForwardDirection::Dynamic => { - tunnel_manager - .start_socks_forward(shared_handle, local_host, local_port) - .await - } + ForwardDirection::Dynamic => { + manager + .start_socks_forward( + shared_handle, + local_host, + local_port, + ) + .await? + } + }; + Ok::<_, shelldeck_ssh::SshError>((session, manager, id)) }; - - match tunnel_result { - Ok(_tunnel_id) => { - // Create a proxy shutdown channel so the UI thread can - // signal this background thread to tear down the tunnel. - let (thread_shutdown_tx, mut thread_shutdown_rx) = - tokio::sync::mpsc::channel::<()>(1); - - // Build a proxy TunnelHandle that shares the real tunnel's - // Arc-wrapped status and byte counters but uses the - // thread-level shutdown channel. - let tunnel_ref = &tunnel_manager.tunnels() - [tunnel_manager.tunnels().len() - 1]; - let proxy_handle = TunnelHandle::new_proxy( - tunnel_ref.id, - tunnel_ref.status.clone(), - tunnel_ref.bytes_sent.clone(), - tunnel_ref.bytes_received.clone(), + let result = tokio::select! { + biased; + _ = thread_shutdown_rx.recv() => return, + result = setup => result, + }; + match result { + Ok((session, manager, id)) => { + let tunnel = + manager.get_tunnel(&id).expect("newly started tunnel"); + let proxy = TunnelHandle::new_proxy( + id, + tunnel.status.clone(), + tunnel.bytes_sent.clone(), + tunnel.bytes_received.clone(), thread_shutdown_tx, ); - - let _ = result_tx.send(Ok(proxy_handle)); - - // Park this thread -- keep the tokio runtime alive so the - // tunnel's spawned tasks continue to run. Wait for shutdown. - thread_shutdown_rx.recv().await; - - // Shutdown received -- stop all tunnels and exit + // If the UI timed out or disappeared, tear down immediately. + if result_tx.send(Ok(proxy)).is_ok() { + thread_shutdown_rx.recv().await; + } tracing::info!("Stopping tunnels for forward {}", forward_id); - tunnel_manager.stop_all(); - - // Give tunnel tasks a moment to clean up - tokio::time::sleep(std::time::Duration::from_millis(100)).await; + manager.stop_all(); + // Keep the runtime alive through listener/channel cleanup and + // remote cancel acknowledgement (bounded internally to 5s). + let _ = tokio::time::timeout( + std::time::Duration::from_secs(6), + async { + while manager.tunnels().iter().any(|t| t.is_active()) { + tokio::time::sleep( + std::time::Duration::from_millis(10), + ) + .await; + } + }, + ) + .await; + let _ = tokio::time::timeout( + std::time::Duration::from_secs(2), + session.disconnect(), + ) + .await; } Err(e) => { let msg = format!("Tunnel start failed: {}", e); @@ -254,6 +262,7 @@ impl Workspace { let thread_handle = match thread_handle { Ok(h) => h, Err(e) => { + self.tunnel_starts.cancel(forward_id); tracing::error!("Failed to spawn tunnel thread: {}", e); self.port_forwards.update(cx, |pf, _| { if let Some(f) = pf.forwards.iter_mut().find(|f| f.id == forward_id) { @@ -277,7 +286,6 @@ impl Workspace { // Now wait for the result from the background thread. // We use cx.spawn to avoid blocking the UI thread. - let pf_handle = self.port_forwards.downgrade(); let weak_self = cx.entity().downgrade(); let label_for_activity = label.clone(); @@ -292,15 +300,19 @@ impl Workspace { }) .await; - match result { - Ok(Ok(tunnel_handle)) => { - tracing::info!( - "Tunnel started successfully for forward {}", - forward_id - ); - - // Store the active tunnel in the workspace - let _ = weak_self.update(cx, |ws, cx| { + if result.is_err() { + let _ = setup_shutdown_tx.try_send(()); + } + let _ = weak_self.update(cx, |ws, cx| { + // Stop, retry, logout and workspace shutdown retire older callbacks. + if !ws.tunnel_starts.finish(forward_id, attempt) { + if let Ok(Ok(handle)) = result { + handle.stop(); + } + return; + } + match result { + Ok(Ok(tunnel_handle)) => { ws.active_tunnels.insert( forward_id, ActiveTunnel { @@ -308,16 +320,14 @@ impl Workspace { _thread: thread_handle, }, ); - - // Update forward status to Active - ws.port_forwards.update(cx, |pf, _| { + ws.port_forwards.update(cx, |pf, cx| { if let Some(f) = pf.forwards.iter_mut().find(|f| f.id == forward_id) { f.status = ForwardStatus::Active; } + cx.notify(); }); - ws.add_activity_entry( ActivityEntry::new( ActivityKind::Forward, @@ -337,73 +347,60 @@ impl Workspace { ToastLevel::Success, cx, ); - ws.update_dashboard_stats(cx); - cx.notify(); - }); - } - Ok(Err(err_msg)) => { - tracing::error!( - "Tunnel failed for forward {}: {}", - forward_id, - err_msg - ); - - let _ = pf_handle.update(cx, |pf, cx| { - if let Some(f) = pf.forwards.iter_mut().find(|f| f.id == forward_id) - { - f.status = ForwardStatus::Error; - } - cx.notify(); - }); - - let _ = weak_self.update(cx, |ws, cx| { - ws.add_activity( - t!("activity.forward_failed", error = err_msg.as_str()) - .to_string(), - ActivityKind::Error, - cx, - ); - ws.show_toast( - t!("toast.forward.failed", error = err_msg.as_str()) - .to_string(), - ToastLevel::Error, - cx, - ); - }); - } - Err(_timeout) => { - tracing::error!("Tunnel setup timed out for forward {}", forward_id); - - let _ = pf_handle.update(cx, |pf, cx| { - if let Some(f) = pf.forwards.iter_mut().find(|f| f.id == forward_id) - { - f.status = ForwardStatus::Error; + } + error => { + // Also stop a worker whose result did not arrive within 30s. + // The receiver has gone away; a later success is never published. + ws.port_forwards.update(cx, |pf, cx| { + if let Some(f) = + pf.forwards.iter_mut().find(|f| f.id == forward_id) + { + f.status = ForwardStatus::Error; + } + cx.notify(); + }); + match error { + Ok(Err(err_msg)) => { + ws.add_activity( + t!("activity.forward_failed", error = err_msg.as_str()) + .to_string(), + ActivityKind::Error, + cx, + ); + ws.show_toast( + t!("toast.forward.failed", error = err_msg.as_str()) + .to_string(), + ToastLevel::Error, + cx, + ); + } + Err(_) => { + ws.add_activity( + t!( + "activity.forward_timeout", + label = label_for_activity.as_str() + ) + .to_string(), + ActivityKind::Error, + cx, + ); + ws.show_toast( + t!( + "toast.forward.timeout", + label = label_for_activity.as_str() + ) + .to_string(), + ToastLevel::Warning, + cx, + ); + } + Ok(Ok(_)) => unreachable!(), } - cx.notify(); - }); - - let _ = weak_self.update(cx, |ws, cx| { - ws.add_activity( - t!( - "activity.forward_timeout", - label = label_for_activity.as_str() - ) - .to_string(), - ActivityKind::Error, - cx, - ); - ws.show_toast( - t!( - "toast.forward.timeout", - label = label_for_activity.as_str() - ) - .to_string(), - ToastLevel::Warning, - cx, - ); - }); + } } - } + ws.update_dashboard_stats(cx); + cx.notify(); + }); }) .detach(); @@ -413,6 +410,8 @@ impl Workspace { let forward_id = *id; tracing::info!("Stop forward requested: {}", forward_id); + // Cancel setup before retiring an already running tunnel. + let was_starting = self.tunnel_starts.cancel(forward_id); // Look up and remove the active tunnel if let Some(active_tunnel) = self.active_tunnels.remove(&forward_id) { // Signal the tunnel to stop. This sends through the shutdown channel @@ -469,7 +468,19 @@ impl Workspace { }); self.add_activity( - t!("activity.forward_stop_no_active").to_string(), + if was_starting { + let label = self + .port_forwards + .read(cx) + .forwards + .iter() + .find(|f| f.id == forward_id) + .map(|f| f.label.clone().unwrap_or_else(|| f.description())) + .unwrap_or_else(|| forward_id.to_string()); + t!("activity.forward_stopped", label = label.as_str()).to_string() + } else { + t!("activity.forward_stop_no_active").to_string() + }, ActivityKind::Forward, cx, ); diff --git a/crates/shelldeck-ui/src/workspace/mod.rs b/crates/shelldeck-ui/src/workspace/mod.rs index e5533f99..76940c99 100644 --- a/crates/shelldeck-ui/src/workspace/mod.rs +++ b/crates/shelldeck-ui/src/workspace/mod.rs @@ -124,6 +124,7 @@ pub(crate) mod platform_attention; pub use platform_attention::PlatformAttentionNotification; mod forwards; mod manage_requests; +mod tunnel_starts; use manage_requests::{ManageRead, ManageRequests}; mod mentions; mod menu; @@ -498,6 +499,7 @@ pub struct Workspace { pub focus_handle: FocusHandle, /// Active tunnels keyed by the PortForward model ID (not the TunnelHandle internal id). active_tunnels: HashMap, + tunnel_starts: tunnel_starts::TunnelStarts, /// Active script executions keyed by script ID. active_scripts: HashMap, /// Explicit local/SSH coding-agent runs, keyed by run ID. @@ -1443,6 +1445,7 @@ impl Workspace { recent_activity, focus_handle: cx.focus_handle(), active_tunnels: HashMap::new(), + tunnel_starts: tunnel_starts::TunnelStarts::default(), active_scripts: HashMap::new(), active_agent_runs: HashMap::new(), agent_session_bindings: HashMap::new(), diff --git a/crates/shelldeck-ui/src/workspace/navigation.rs b/crates/shelldeck-ui/src/workspace/navigation.rs index 8441bf2b..6ebd938b 100644 --- a/crates/shelldeck-ui/src/workspace/navigation.rs +++ b/crates/shelldeck-ui/src/workspace/navigation.rs @@ -157,6 +157,14 @@ impl Workspace { /// Stop runtime resources that must never survive an account boundary. /// Unlike `shutdown`, this keeps the Workspace itself alive for login. pub(super) fn stop_authenticated_runtime(&mut self, cx: &mut Context) { + // Retire setup attempts before stopping completed tunnels. + self.tunnel_starts.cancel_all(); + self.port_forwards.update(cx, |pf, cx| { + for forward in &mut pf.forwards { + forward.status = shelldeck_core::models::port_forward::ForwardStatus::Inactive; + } + cx.notify(); + }); // Stop all active tunnels for (fwd_id, tunnel) in self.active_tunnels.drain() { tracing::info!("Stopping tunnel for forward {}", fwd_id); diff --git a/crates/shelldeck-ui/src/workspace/tunnel_starts.rs b/crates/shelldeck-ui/src/workspace/tunnel_starts.rs new file mode 100644 index 00000000..84efa93e --- /dev/null +++ b/crates/shelldeck-ui/src/workspace/tunnel_starts.rs @@ -0,0 +1,91 @@ +use std::collections::HashMap; +use tokio::sync::mpsc; +use uuid::Uuid; + +/// Own pending setup cancellation and distinguish retries for the same profile. +#[derive(Default)] +pub(super) struct TunnelStarts(HashMap)>); + +impl TunnelStarts { + pub fn contains(&self, forward: Uuid) -> bool { + self.0.contains_key(&forward) + } + + pub fn begin(&mut self, forward: Uuid) -> (Uuid, mpsc::Sender<()>, mpsc::Receiver<()>) { + self.cancel(forward); + let attempt = Uuid::new_v4(); + let (tx, rx) = mpsc::channel(1); + self.0.insert(forward, (attempt, tx.clone())); + (attempt, tx, rx) + } + + /// A stopped or replaced attempt cannot publish success, failure or timeout. + pub fn finish(&mut self, forward: Uuid, attempt: Uuid) -> bool { + if self.0.get(&forward).is_some_and(|(id, _)| *id == attempt) { + self.0.remove(&forward); + true + } else { + false + } + } + + pub fn cancel(&mut self, forward: Uuid) -> bool { + if let Some((_, tx)) = self.0.remove(&forward) { + let _ = tx.try_send(()); + true + } else { + false + } + } + + pub fn cancel_all(&mut self) { + for (_, (_, tx)) in self.0.drain() { + let _ = tx.try_send(()); + } + } +} + +impl Drop for TunnelStarts { + fn drop(&mut self) { + self.cancel_all(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // SDTEST-1943 + #[tokio::test] + async fn stop_retry_and_account_shutdown_retire_pending_tunnel_results() { + let mut starts = TunnelStarts::default(); + let forward = Uuid::new_v4(); + let (old, _old_tx, mut old_rx) = starts.begin(forward); + assert!(starts.cancel(forward)); + assert_eq!(old_rx.recv().await, Some(())); + let (new, _new_tx, mut new_rx) = starts.begin(forward); + // Delayed success/error for the old setup must not retire the retry. + assert!(!starts.finish(forward, old)); + assert!(starts.contains(forward)); + assert!(matches!( + new_rx.try_recv(), + Err(mpsc::error::TryRecvError::Empty) + )); + assert!(starts.finish(forward, new)); + assert!(!starts.finish(forward, new)); + + let (pending, _tx, mut rx) = starts.begin(forward); + let other = Uuid::new_v4(); + let (second, _tx2, mut rx2) = starts.begin(other); + starts.cancel_all(); + assert_eq!(rx.recv().await, Some(())); + assert_eq!(rx2.recv().await, Some(())); + assert!(!starts.finish(forward, pending)); + assert!(!starts.finish(other, second)); + let (retry, _tx3, mut rx3) = starts.begin(forward); + assert!(!starts.finish(forward, pending)); + drop(starts); + assert_eq!(rx3.recv().await, Some(())); + assert_ne!(retry, pending); + } +} diff --git a/docs/testing/USE_CASES.md b/docs/testing/USE_CASES.md index 91e43302..aa8782c1 100644 --- a/docs/testing/USE_CASES.md +++ b/docs/testing/USE_CASES.md @@ -268,10 +268,10 @@ Connection share a transport or remain isolated. ### SDUC-049 — Local port forward tunnel -`TunnelManager::start_local_forward` binds a local port and forwards -each accepted connection over the SSH session. `check_port_available` -short-circuits if the local port is taken. Bytes-transferred counters -increment for both directions. +`TunnelManager::start_local_forward` binds the saved local address and port +(loopback by default) and forwards each accepted connection over SSH. Bind +errors are returned before reporting success, including a port occupied on +the configured address. Bytes-transferred counters increment in both directions. ### SDUC-050 — Remote port forward tunnel @@ -290,6 +290,10 @@ proxies TCP through the SSH session. active tunnel. `cleanup` removes stopped entries so `active_count` matches `tunnels().len()`. Reverse forwarding explicitly cancels the exact server-side bind on stop, even if its owning SSH session remains connected. +Stopping during SSH setup cancels that attempt. Stop, retry, logout and workspace +shutdown retire older completions, so they cannot reactivate a stopped tunnel or +replace a newer retry. The worker keeps its runtime alive for bounded cleanup +before disconnecting SSH. ### SDUC-053 — Jump-host session @@ -3182,6 +3186,10 @@ an unavailable adapter refuses before any effect. ## Change log +- **2026-09-30**: Amended SDUC-049/052 for saved local bind addresses, pending + setup cancellation, retry ownership and bounded worker cleanup. Added + SDTEST-1942..1944 and native forwarding QA evidence. + - **2026-09-30**: Amended SDUC-050/052/152/154 for configured reverse bind addresses, explicit remote listener cancellation, account-bound async results, current-token expiry and request draft cleanup. SDTEST-565 and diff --git a/docs/testing/forward-qa-2026-09-30.md b/docs/testing/forward-qa-2026-09-30.md new file mode 100644 index 00000000..a21f6273 --- /dev/null +++ b/docs/testing/forward-qa-2026-09-30.md @@ -0,0 +1,73 @@ +# Forwarding QA — 2026-09-30 + +Tested the native Linux app on a private Xvfb display with an isolated XDG +profile, a local SSH server and fictional Manage HTTP data. The baseline binary +contains main at `be4d24b` (v0.9.10); the corrected binary was rebuilt from this +PR. This is app QA on an owned test display, not control of the user's desktop. +`HOME` and the user's SSH configuration were unchanged. The fixture's exact +localhost host key was trusted only for this run and removed afterward. + +## Reproductions and corrections + +| Workflow | Baseline | Corrected app | +|---|---|---| +| Saved local bind `127.0.0.2:8899` | UI showed the saved address, but only `127.0.0.1:8899` accepted connections. | Only `127.0.0.2:8899` accepted connections; an HTTP request through SSH returned the fixture's issues with status 200. | +| Stop during five-second SSH authentication delay | Start, then Stop before authentication completed; the listener later reopened and the row became active. | The listener stayed closed beyond six seconds and the row stayed inactive. | +| Stop, then immediate retry | Older pending completions had no attempt ownership. | A canceled delayed attempt could not replace a newer successful retry; the newer listener remained active after the older delay elapsed. | +| Reverse-forward cleanup | The worker previously destroyed its runtime after a fixed 100 ms, even when explicit remote cancellation was still waiting. | A server deliberately delaying cancellation by two seconds closed its listener after 2.03 seconds. The runtime stayed alive for cleanup, then disconnected SSH. | +| SOCKS forward | Previously covered in the preceding QA batch. | An HTTP request through the SOCKS proxy returned the issues payload, then Stop closed its listener. | +| Logout during ten-second authentication delay | Pending setup was outside authenticated-runtime cleanup. | Logout returned to welcome and the listener remained closed after the delay elapsed. | + +Reproduce the address bug with a local-forward profile using a non-default +loopback address: start it, then attempt TCP connections to both the saved and +default addresses. Reproduce the pending-start bug by delaying public-key +authentication on a test SSH server, clicking Start then Stop before the delay +ends, and checking both the row and socket after authentication would finish. +Repeat with a new Start and with account logout. No production hosts or writes +were used. The reverse-forward fixture delayed its cancellation handler before +closing the remote listener, making the runtime-cleanup check observable. + +The correction passes the saved local address into `TunnelManager`, owns setup +cancellation before starting the worker, and tags each attempt so stopped, +replaced or logged-out callbacks cannot publish success, failure or timeout. +Worker shutdown waits up to six seconds for tunnel cleanup (remote cancellation +has an internal five-second bound), then bounds SSH disconnect to two seconds. +The 30-second UI setup timeout also cancels the worker. The full timeout itself +was not exercised through the native UI. + +## Evidence + +All screenshots contain fictional fixture data on the forwarding/welcome +surfaces. Socket observations are in +[observations.json](screenshots/2026-09-30/forwards/observations.json). + +- Before: [saved address](screenshots/2026-09-30/forwards/03-baseline-bind.png), + [wrong actual bind](screenshots/2026-09-30/forwards/04-baseline-wrong-bind.png), + [stopped pending setup](screenshots/2026-09-30/forwards/05-baseline-stopped-pending.png), + [unwanted reactivation](screenshots/2026-09-30/forwards/06-baseline-reactivated.png). +- After: [configured bind and traffic](screenshots/2026-09-30/forwards/08-fixed-bind-traffic.png), + [stop during setup](screenshots/2026-09-30/forwards/09-fixed-stop-pending.png), + [still stopped after delay](screenshots/2026-09-30/forwards/10-fixed-stays-stopped.png), + [new retry retained](screenshots/2026-09-30/forwards/11-fixed-retry-retained.png). +- Cleanup: [reverse traffic](screenshots/2026-09-30/forwards/12-fixed-reverse-traffic.png), + [SOCKS traffic](screenshots/2026-09-30/forwards/13-fixed-socks-traffic.png), + [all stopped](screenshots/2026-09-30/forwards/14-fixed-all-stopped.png), + [logout after pending setup](screenshots/2026-09-30/forwards/17-logout-remains-closed.png). + +## Validation and limits + +`cargo test --workspace`: 859 unique tests passed (app 100, core 404, SSH 47, +terminal 88, UI 207, update 13); seven gated live tests and two documentation +examples remain ignored. The configured-bind regression additionally ran with +IPv6 loopback (`::1`) to verify address handling without requiring a macOS +IPv4 loopback alias. Workspace Clippy with all targets and warnings denied, +formatting, Linux build and UI compilation passed. SDTEST-1942 exercises actual +SSH echo, occupied configured bind and listener shutdown; SDTEST-1943 exercises +pending cancellation and stale-completion ownership. SDTEST-1944 records the +remaining automated native click-coverage gap. + +Native GUI checks were Linux only. CI runs Linux workspace tests, native macOS +compilation and macOS/Windows core checks; it does not run these new SSH/UI tests +on macOS or Windows. Signing was unchanged. Subscription execution was verified +in the preceding [feature follow-up](feature-followup-qa-2026-09-30.md), and was +not rerun for this forwarding-only change. diff --git a/docs/testing/screenshots/2026-09-30/forwards/03-baseline-bind.png b/docs/testing/screenshots/2026-09-30/forwards/03-baseline-bind.png new file mode 100644 index 00000000..7fa91b3b Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/03-baseline-bind.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/04-baseline-wrong-bind.png b/docs/testing/screenshots/2026-09-30/forwards/04-baseline-wrong-bind.png new file mode 100644 index 00000000..730c2b25 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/04-baseline-wrong-bind.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/05-baseline-stopped-pending.png b/docs/testing/screenshots/2026-09-30/forwards/05-baseline-stopped-pending.png new file mode 100644 index 00000000..91e423e2 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/05-baseline-stopped-pending.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/06-baseline-reactivated.png b/docs/testing/screenshots/2026-09-30/forwards/06-baseline-reactivated.png new file mode 100644 index 00000000..09e35688 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/06-baseline-reactivated.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/08-fixed-bind-traffic.png b/docs/testing/screenshots/2026-09-30/forwards/08-fixed-bind-traffic.png new file mode 100644 index 00000000..45a4dc85 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/08-fixed-bind-traffic.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/09-fixed-stop-pending.png b/docs/testing/screenshots/2026-09-30/forwards/09-fixed-stop-pending.png new file mode 100644 index 00000000..91e423e2 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/09-fixed-stop-pending.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/10-fixed-stays-stopped.png b/docs/testing/screenshots/2026-09-30/forwards/10-fixed-stays-stopped.png new file mode 100644 index 00000000..9faa8106 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/10-fixed-stays-stopped.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/11-fixed-retry-retained.png b/docs/testing/screenshots/2026-09-30/forwards/11-fixed-retry-retained.png new file mode 100644 index 00000000..f1e1fa1c Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/11-fixed-retry-retained.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/12-fixed-reverse-traffic.png b/docs/testing/screenshots/2026-09-30/forwards/12-fixed-reverse-traffic.png new file mode 100644 index 00000000..bddf6ef2 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/12-fixed-reverse-traffic.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/13-fixed-socks-traffic.png b/docs/testing/screenshots/2026-09-30/forwards/13-fixed-socks-traffic.png new file mode 100644 index 00000000..02215ccb Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/13-fixed-socks-traffic.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/14-fixed-all-stopped.png b/docs/testing/screenshots/2026-09-30/forwards/14-fixed-all-stopped.png new file mode 100644 index 00000000..c0e7677e Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/14-fixed-all-stopped.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/17-logout-remains-closed.png b/docs/testing/screenshots/2026-09-30/forwards/17-logout-remains-closed.png new file mode 100644 index 00000000..493ac3a7 Binary files /dev/null and b/docs/testing/screenshots/2026-09-30/forwards/17-logout-remains-closed.png differ diff --git a/docs/testing/screenshots/2026-09-30/forwards/observations.json b/docs/testing/screenshots/2026-09-30/forwards/observations.json new file mode 100644 index 00000000..14d6533c --- /dev/null +++ b/docs/testing/screenshots/2026-09-30/forwards/observations.json @@ -0,0 +1,38 @@ +{ + "baseline-bind": { + "127.0.0.1": "open", + "127.0.0.2": "closed" + }, + "baseline-pending": { + "stop_during_authentication": "reactivated", + "port_after_stop": "open", + "worker_log_order": [ + "Start", + "Stop (no active handle)", + "Tunnel started successfully" + ] + }, + "fixed-bind": { + "127.0.0.1": "closed", + "127.0.0.2": "open", + "forwarded_http_status": 200 + }, + "fixed-pending": { + "stop_during_authentication": "stays inactive", + "port_after_6_seconds": "closed", + "worker_log_order": [ + "Start", + "Stop (pending)" + ] + }, + "fixed-reverse": { + "cancel_delay_seconds": 2, + "listener_closed_after_seconds": 2.03, + "http_status": 200 + }, + "fixed-logout": { + "auth_delay_seconds": 10, + "port_after_logout_and_delay": "closed", + "pending_start_verified_in_worker_log": true + } +} diff --git a/docs/testing/tests-ssh.md b/docs/testing/tests-ssh.md index c9485566..122bf14b 100644 --- a/docs/testing/tests-ssh.md +++ b/docs/testing/tests-ssh.md @@ -151,6 +151,8 @@ Existing: **0 tests.** | SDTEST-620 | *to write* — `live_connect_and_exec` against `sshd` in a container | SDUC-045, SDUC-054 | **Red / P2** | Gated by `SHELLDECK_LIVE_SSH=1`. Optional; the mocks + fake transport should catch most regressions before this. | | SDTEST-1809 | `session.rs::fixed_workspace_subsystem_prepares_releases_and_resumes_exact_remote_repository` | SDUC-491 | Yellow / live | Against an operator-installed `shelldeck-workspace-v1` subsystem, the production client prepares and releases once without a shell, then prepares/resumes a second exact receipt and observes the fixed shell running at the retained clean repository CWD. Passing in the documented opt-in live harness, but excluded from the default suite; gated by `SHELLDECK_LIVE_SSH=1` plus explicit host/key/workspace coordinates. | +| SDTEST-1942 | `tunnel.rs::local_forward_uses_configured_bind_address_and_rejects_occupied_address` | SDUC-049, SDUC-052 | Green | IPv6 loopback bind rejects occupation, accepts real SSH echo only on the configured address, and closes on stop. | + --- ## Retired tests diff --git a/docs/testing/tests-ui-and-app.md b/docs/testing/tests-ui-and-app.md index 4a2658cd..7617a406 100644 --- a/docs/testing/tests-ui-and-app.md +++ b/docs/testing/tests-ui-and-app.md @@ -665,3 +665,5 @@ observed defect. Evidence and repeatable journeys live in | SDTEST-1938 | `workspace/manage_requests.rs::logout_and_relogin_retire_all_previous_reads_and_writes` | SDUC-152, SDUC-154 | Green | Logout/re-login rejects prior account reads, writes and logged-out callbacks, including identical bearer reuse; only the new session can publish. | | SDTEST-1939 | `workspace/manage_requests.rs::permission_failures_keep_session_but_current_revocation_expires_once` | SDUC-152, SDUC-154 | Green | Current 403 stays local; current 401 expires the session once. Concurrent stale 401 and success cannot reject or repopulate a later session. | | SDTEST-1940 | *to write* — native async session recovery and account draft cleanup | SDUC-152, SDUC-154, SDUC-228 | **Red / P1** | Manual X11 pass reproduced delayed-detail disappearance and lingering expired session on v0.9.10, then verified the rebuilt app retains the latest detail, returns to welcome on polling 401, and re-login opens an empty creation draft. Automated rendered/click coverage remains missing. See feature-followup-qa-2026-09-30.md. | +| SDTEST-1943 | `workspace/tunnel_starts.rs::stop_retry_and_account_shutdown_retire_pending_tunnel_results` | SDUC-052, SDUC-152 | Green | Cancellation reaches pending workers; delayed completions cannot remove a newer retry; account shutdown and owner drop retire all attempts. | +| SDTEST-1944 | *to write* — native tunnel setup cancellation and delayed reverse cleanup | SDUC-049, SDUC-050, SDUC-052 | **Red / P1** | Private X11 manual coverage in forward-qa-2026-09-30.md; rendered click automation is not part of CI. |