fix(#4): fail closed when background forward has no listener

crates/openshell-cli/src/ssh.rs

@@ -9,8 +9,8 @@ use miette::{IntoDiagnostic, Result, WrapErr};
 use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, Signal, sigaction};
 use openshell_core::ObjectId;
 use openshell_core::forward::{
-    build_proxy_command, find_ssh_forward_pid, format_gateway_url, resolve_ssh_gateway,
-    shell_escape, validate_ssh_session_response, write_forward_pid,
+    build_proxy_command, find_ssh_forward_pid, format_gateway_url, kill_process, probe_listener,
+    resolve_ssh_gateway, shell_escape, validate_ssh_session_response, write_forward_pid,
 };
 use openshell_core::proto::{
     CreateSshSessionRequest, GetSandboxRequest, SshRelayTarget, TcpForwardFrame, TcpForwardInit,
@@ -369,15 +369,29 @@ pub async fn sandbox_forward(
 
     if background {
         // SSH has forked — find its PID and record it.
-        if let Some(pid) = find_ssh_forward_pid(&session.sandbox_id, port) {
-            write_forward_pid(name, port, pid, &session.sandbox_id, &spec.bind_addr)?;
-        } else {
-            eprintln!(
-                "{} Could not discover backgrounded SSH process; \
-                 forward may be running but is not tracked",
-                "!".yellow(),
-            );
+        let pid = find_ssh_forward_pid(&session.sandbox_id, port);
+
+        if pid.is_none() {
+            return Err(miette::miette!(
+                "Could not discover backgrounded SSH process for port {port}; \
+                 the forward is not tracked and may not be running"
+            ));
         }
+
+        let pid = pid.unwrap();
+
+        // Probe the local listener to confirm the tunnel is actually serving.
+        if !probe_listener(&spec.bind_addr, port) {
+            // Listener never came up — kill the orphaned SSH process and fail.
+            kill_process(pid);
+            return Err(miette::miette!(
+                "SSH process (pid {pid}) started but the local listener on \
+                 {}:{port} is not reachable; killed the background process",
+                spec.bind_addr,
+            ));
+        }
+
+        write_forward_pid(name, port, pid, &session.sandbox_id, &spec.bind_addr)?;
     }
 
     Ok(())

crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs

@@ -1307,7 +1307,11 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
     let forward_port = listener.local_addr().unwrap().port();
     drop(listener);
 
-    run::sandbox_create(
+    // The fake SSH binary exits immediately without creating a background
+    // process or local listener, so the forward correctly fails closed.
+    // The sandbox is still kept (not deleted) because --forward implies
+    // persistence regardless of the forward outcome.
+    let _err = run::sandbox_create(
         &server.endpoint,
         Some("persistent-forward"),
         None,
@@ -1331,7 +1335,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
         &tls,
     )
     .await
-    .expect("sandbox create with forward should succeed");
+    .expect_err("sandbox create with forward should fail when listener is unreachable");
 
     assert!(deleted_names(&server).await.is_empty());
 }
@@ -1430,3 +1434,30 @@ async fn sandbox_create_env_rejects_invalid_key_name() {
         "error should mention invalid key, got: {msg}"
     );
 }
+
+#[tokio::test]
+async fn sandbox_forward_background_fails_when_ssh_process_untracked_and_listener_missing() {
+    let server = run_server().await;
+    let fake_ssh_dir = tempfile::tempdir().unwrap();
+    let xdg_dir = tempfile::tempdir().unwrap();
+    let _env = test_env(&fake_ssh_dir, &xdg_dir);
+    let tls = test_tls(&server);
+    install_fake_ssh(&fake_ssh_dir);
+
+    let listener = std::net::TcpListener::bind(("127.0.0.1", 0)).unwrap();
+    let port = listener.local_addr().unwrap().port();
+    drop(listener);
+    let spec = openshell_core::forward::ForwardSpec::new(port);
+
+    let err = run::sandbox_forward(&server.endpoint, "forward-repro", &spec, true, &tls)
+        .await
+        .expect_err(
+            "background forward should fail when ssh exits successfully but no background process or local listener exists",
+        );
+
+    let text = format!("{err:?}");
+    assert!(
+        text.contains("Could not discover") || text.contains("listener"),
+        "unexpected error: {text}",
+    );
+}

crates/openshell-core/src/forward.rs

@@ -8,9 +8,10 @@
 
 use crate::paths::{create_dir_restricted, xdg_config_dir};
 use miette::{IntoDiagnostic, Result, WrapErr};
-use std::net::TcpListener;
+use std::net::{TcpListener, TcpStream};
 use std::path::PathBuf;
 use std::process::Command;
+use std::time::Duration;
 
 // ---------------------------------------------------------------------------
 // Forward PID file management
@@ -425,6 +426,40 @@ fn lsof_listeners(port: u16) -> Option<String> {
     }
 }
 
+// ---------------------------------------------------------------------------
+// Listener probe
+// ---------------------------------------------------------------------------
+
+/// Maximum number of TCP connect attempts when probing a local listener.
+const LISTENER_PROBE_MAX_ATTEMPTS: u32 = 10;
+
+/// Delay between TCP connect probe attempts.
+const LISTENER_PROBE_INTERVAL: Duration = Duration::from_millis(100);
+
+/// Probe the local listener on `bind_addr:port` by attempting a TCP connect.
+///
+/// Retries up to [`LISTENER_PROBE_MAX_ATTEMPTS`] times with a short delay
+/// between attempts to allow the SSH tunnel time to set up the listener.
+/// Returns `true` if the listener is reachable, `false` otherwise.
+pub fn probe_listener(bind_addr: &str, port: u16) -> bool {
+    for _ in 0..LISTENER_PROBE_MAX_ATTEMPTS {
+        if TcpStream::connect((bind_addr, port)).is_ok() {
+            return true;
+        }
+        std::thread::sleep(LISTENER_PROBE_INTERVAL);
+    }
+    false
+}
+
+/// Kill a process by PID. Best-effort; errors are silently ignored.
+pub fn kill_process(pid: u32) {
+    let _ = Command::new("kill")
+        .arg(pid.to_string())
+        .stdout(std::process::Stdio::null())
+        .stderr(std::process::Stdio::null())
+        .status();
+}
+
 // ---------------------------------------------------------------------------
 // SSH utility functions (shared between CLI and TUI)
 // ---------------------------------------------------------------------------