From c51a1400cdedfaa3bfc30ea73c72c4185aef4e02 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 15:37:03 -0700 Subject: [PATCH 1/8] fix(cli): check port availability before starting SSH forward Previously, sandbox create --forward and forward start would attempt the SSH port forward without checking if the port was already in use, resulting in a cryptic 'ssh exited with status 255' error. The sandbox would still be created but the user would not be connected. Add a pre-flight check that tries to bind the port before spawning SSH. If the port is occupied by an existing openshell forward, the error message includes the stop command. If occupied by another process, it suggests lsof to identify it. Applied to both CLI and TUI paths. --- crates/openshell-cli/src/ssh.rs | 2 + crates/openshell-core/src/forward.rs | 62 ++++++++++++++++++++++++++++ crates/openshell-tui/src/lib.rs | 5 +++ 3 files changed, 69 insertions(+) diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index a834d0a4..1a1ed0d4 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -311,6 +311,8 @@ pub async fn sandbox_forward( background: bool, tls: &TlsOptions, ) -> Result<()> { + openshell_core::forward::check_port_available(port)?; + let session = ssh_session_config(server, name, tls).await?; let mut command = TokioCommand::from(ssh_base_command(&session.proxy_command)); diff --git a/crates/openshell-core/src/forward.rs b/crates/openshell-core/src/forward.rs index 8d84ed17..6ff4cf8f 100644 --- a/crates/openshell-core/src/forward.rs +++ b/crates/openshell-core/src/forward.rs @@ -7,6 +7,7 @@ //! start, stop, list, and track background SSH port forwards. use miette::{IntoDiagnostic, Result, WrapErr}; +use std::net::TcpListener; use std::path::PathBuf; use std::process::Command; @@ -215,6 +216,41 @@ pub fn list_forwards() -> Result> { Ok(forwards) } +// --------------------------------------------------------------------------- +// Port availability check +// --------------------------------------------------------------------------- + +/// Check whether a local port is available for forwarding. +/// +/// Attempts to bind `127.0.0.1:`. If the port is already in use, the +/// error message includes an actionable hint: +/// +/// - If an existing openshell forward owns the port, suggest the stop command. +/// - Otherwise, suggest `lsof` to identify the owning process. +pub fn check_port_available(port: u16) -> Result<()> { + if TcpListener::bind(("127.0.0.1", port)).is_ok() { + // Port is free — the listener is dropped immediately, releasing it. + return Ok(()); + } + + // Port is occupied. Check if it belongs to a tracked openshell forward. + if let Ok(forwards) = list_forwards() + && let Some(fwd) = forwards.iter().find(|f| f.port == port && f.alive) + { + return Err(miette::miette!( + "Port {port} is already forwarded to sandbox '{}'.\n\ + Stop it with: openshell forward stop {port} {}", + fwd.sandbox, + fwd.sandbox, + )); + } + + Err(miette::miette!( + "Port {port} is already in use by another process.\n\ + Find it with: lsof -i :{port} -sTCP:LISTEN", + )) +} + // --------------------------------------------------------------------------- // SSH utility functions (shared between CLI and TUI) // --------------------------------------------------------------------------- @@ -423,4 +459,30 @@ mod tests { // 0 is valid u16 but we may want to filter it; 99999 overflows u16. assert_eq!(ports, vec![8080, 3000, 0]); } + + #[test] + fn check_port_available_free_port() { + // Bind to port 0 to get an OS-assigned free port, then drop the + // listener so the port is released before we test it. + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + drop(listener); + + assert!(check_port_available(port).is_ok()); + } + + #[test] + fn check_port_available_occupied_port() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + // Keep the listener alive so the port stays occupied. + + let result = check_port_available(port); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("already in use"), + "expected 'already in use' in error message, got: {msg}" + ); + } } diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 98f4ed0d..a782d5ce 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1360,6 +1360,11 @@ async fn start_port_forwards( // Start a forward for each port. for port in ports { + if let Err(e) = openshell_core::forward::check_port_available(*port) { + tracing::warn!("skipping forward for port {port}: {e}"); + continue; + } + let mut command = std::process::Command::new("ssh"); command .arg("-o") From 2d2177f7bc671d0d151a1c9102254c79a4ed0698 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 16:15:20 -0700 Subject: [PATCH 2/8] feat(cli): support custom bind address for port forwards Extend --forward to accept [bind_address:]port syntax following SSH -L conventions. For example, --forward 0.0.0.0:8080 binds on all interfaces instead of just localhost. Add ForwardSpec type in openshell-core that handles parsing, SSH -L arg generation, and display formatting. Thread it through the CLI, TUI, and all forwarding call sites. The port availability check also uses the specified bind address. --- crates/openshell-cli/src/main.rs | 21 ++- crates/openshell-cli/src/run.rs | 28 +-- crates/openshell-cli/src/ssh.rs | 30 +++- .../sandbox_create_lifecycle_integration.rs | 2 +- crates/openshell-core/src/forward.rs | 164 +++++++++++++++++- crates/openshell-tui/src/app.rs | 25 ++- crates/openshell-tui/src/lib.rs | 16 +- 7 files changed, 241 insertions(+), 45 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 151bd166..62d3b75d 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1093,9 +1093,9 @@ enum SandboxCommands { policy: Option, /// Forward a local port to the sandbox before the initial command or shell starts. - /// Keeps the sandbox alive. + /// Accepts [bind_address:]port (e.g. 8080, 0.0.0.0:8080). Keeps the sandbox alive. #[arg(long, conflicts_with = "no_keep")] - forward: Option, + forward: Option, /// Allocate a pseudo-terminal for the remote command. /// Defaults to auto-detection (on when stdin and stdout are terminals). @@ -1359,8 +1359,8 @@ enum ForwardCommands { /// Start forwarding a local port to a sandbox. #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Start { - /// Port to forward (used as both local and remote port). - port: u16, + /// Port to forward: [bind_address:]port (e.g. 8080, 0.0.0.0:8080). + port: String, /// Sandbox name (defaults to last-used sandbox). #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] @@ -1629,18 +1629,20 @@ async fn main() -> Result<()> { name, background, } => { + let spec = openshell_core::forward::ForwardSpec::parse(&port)?; let ctx = resolve_gateway(&cli.gateway, &cli.gateway_endpoint)?; let mut tls = tls.with_gateway_name(&ctx.name); apply_edge_auth(&mut tls, &ctx.name); let name = resolve_sandbox_name(name, &ctx.name)?; - run::sandbox_forward(&ctx.endpoint, &name, port, background, &tls).await?; + run::sandbox_forward(&ctx.endpoint, &name, &spec, background, &tls).await?; if background { eprintln!( - "{} Forwarding port {port} to sandbox {name} in the background", + "{} Forwarding port {} to sandbox {name} in the background", "✓".green().bold(), + spec.port, ); - eprintln!(" Access at: http://127.0.0.1:{port}/"); - eprintln!(" Stop with: openshell forward stop {port} {name}"); + eprintln!(" Access at: {}", spec.access_url()); + eprintln!(" Stop with: openshell forward stop {} {name}", spec.port); } } }, @@ -1864,6 +1866,9 @@ async fn main() -> Result<()> { }); let editor = editor.map(Into::into); + let forward = forward + .map(|s| openshell_core::forward::ForwardSpec::parse(&s)) + .transpose()?; let keep = keep || !no_keep || editor.is_some() || forward.is_some(); // For `sandbox create`, a missing cluster is not fatal — the diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index db033e45..ccf25865 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1726,7 +1726,7 @@ pub async fn sandbox_create_with_bootstrap( ssh_key: Option<&str>, providers: &[String], policy: Option<&str>, - forward: Option, + forward: Option, command: &[String], tty_override: Option, bootstrap_override: Option, @@ -1767,7 +1767,10 @@ pub async fn sandbox_create_with_bootstrap( .await } -fn sandbox_should_persist(keep: bool, forward: Option) -> bool { +fn sandbox_should_persist( + keep: bool, + forward: Option<&openshell_core::forward::ForwardSpec>, +) -> bool { keep || forward.is_some() } @@ -1808,7 +1811,7 @@ pub async fn sandbox_create( ssh_key: Option<&str>, providers: &[String], policy: Option<&str>, - forward: Option, + forward: Option, command: &[String], tty_override: Option, bootstrap_override: Option, @@ -1918,7 +1921,7 @@ pub async fn sandbox_create( .ok_or_else(|| miette::miette!("sandbox missing from response"))?; let interactive = std::io::stdout().is_terminal(); - let persist = sandbox_should_persist(keep, forward); + let persist = sandbox_should_persist(keep, forward.as_ref()); let sandbox_name = sandbox.name.clone(); // Record this sandbox as the last-used for the active gateway only when it @@ -2195,21 +2198,25 @@ pub async fn sandbox_create( // If --forward was requested, start the background port forward // *before* running the command so that long-running processes // (e.g. `openclaw gateway`) are reachable immediately. - if let Some(port) = forward { + if let Some(ref spec) = forward { sandbox_forward( &effective_server, &sandbox_name, - port, + spec, true, // background &effective_tls, ) .await?; eprintln!( - " {} Forwarding port {port} to sandbox {sandbox_name} in the background\n", + " {} Forwarding port {} to sandbox {sandbox_name} in the background\n", "\u{2713}".green().bold(), + spec.port, + ); + eprintln!(" Access at: {}", spec.access_url()); + eprintln!( + " Stop with: openshell forward stop {} {sandbox_name}", + spec.port, ); - eprintln!(" Access at: http://127.0.0.1:{port}/"); - eprintln!(" Stop with: openshell forward stop {port} {sandbox_name}",); } if let Some(editor) = editor { @@ -4420,7 +4427,8 @@ mod tests { #[test] fn sandbox_should_persist_when_forward_is_requested() { - assert!(sandbox_should_persist(false, Some(8080))); + let spec = openshell_core::forward::ForwardSpec::new(8080); + assert!(sandbox_should_persist(false, Some(&spec))); } #[test] diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index 1a1ed0d4..a7493f0d 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -307,11 +307,11 @@ pub async fn sandbox_connect_editor( pub async fn sandbox_forward( server: &str, name: &str, - port: u16, + spec: &openshell_core::forward::ForwardSpec, background: bool, tls: &TlsOptions, ) -> Result<()> { - openshell_core::forward::check_port_available(port)?; + openshell_core::forward::check_port_available(spec)?; let session = ssh_session_config(server, name, tls).await?; @@ -321,7 +321,7 @@ pub async fn sandbox_forward( .arg("-o") .arg("ExitOnForwardFailure=yes") .arg("-L") - .arg(format!("{port}:127.0.0.1:{port}")); + .arg(spec.ssh_forward_arg()); if background { // SSH -f: fork to background after authentication. @@ -334,6 +334,8 @@ pub async fn sandbox_forward( .stdout(Stdio::inherit()) .stderr(Stdio::inherit()); + let port = spec.port; + let status = if background { command.status().await.into_diagnostic()? } else { @@ -341,7 +343,7 @@ pub async fn sandbox_forward( match tokio::time::timeout(FOREGROUND_FORWARD_STARTUP_GRACE_PERIOD, child.wait()).await { Ok(status) => status.into_diagnostic()?, Err(_) => { - eprintln!("{}", foreground_forward_started_message(name, port)); + eprintln!("{}", foreground_forward_started_message(name, spec)); child.wait().await.into_diagnostic()? } } @@ -367,10 +369,15 @@ pub async fn sandbox_forward( Ok(()) } -fn foreground_forward_started_message(name: &str, port: u16) -> String { +fn foreground_forward_started_message( + name: &str, + spec: &openshell_core::forward::ForwardSpec, +) -> String { format!( - "{} Forwarding port {port} to sandbox {name}\n Access at: http://127.0.0.1:{port}/\n Press Ctrl+C to stop\n {}", + "{} Forwarding port {} to sandbox {name}\n Access at: {}\n Press Ctrl+C to stop\n {}", "✓".green().bold(), + spec.port, + spec.access_url(), "Hint: pass --background to start forwarding without blocking your terminal".dimmed(), ) } @@ -1132,7 +1139,8 @@ mod tests { #[test] fn foreground_forward_started_message_includes_port_and_stop_hint() { - let message = foreground_forward_started_message("demo", 8080); + let spec = openshell_core::forward::ForwardSpec::new(8080); + let message = foreground_forward_started_message("demo", &spec); assert!(message.contains("Forwarding port 8080 to sandbox demo")); assert!(message.contains("Access at: http://127.0.0.1:8080/")); assert!(message.contains("sandbox demo")); @@ -1141,4 +1149,12 @@ mod tests { "Hint: pass --background to start forwarding without blocking your terminal" )); } + + #[test] + fn foreground_forward_started_message_custom_bind_addr() { + let spec = openshell_core::forward::ForwardSpec::parse("0.0.0.0:3000").unwrap(); + let message = foreground_forward_started_message("demo", &spec); + assert!(message.contains("Forwarding port 3000 to sandbox demo")); + assert!(message.contains("Access at: http://localhost:3000/")); + } } diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 3b38ddf3..9fcfeced 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -704,7 +704,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() { None, &[], None, - Some(8080), + Some(openshell_core::forward::ForwardSpec::new(8080)), &["echo".to_string(), "OK".to_string()], Some(false), Some(false), diff --git a/crates/openshell-core/src/forward.rs b/crates/openshell-core/src/forward.rs index 6ff4cf8f..27a07a72 100644 --- a/crates/openshell-core/src/forward.rs +++ b/crates/openshell-core/src/forward.rs @@ -216,23 +216,110 @@ pub fn list_forwards() -> Result> { Ok(forwards) } +// --------------------------------------------------------------------------- +// Forward spec parsing +// --------------------------------------------------------------------------- + +/// A parsed port-forward specification: optional bind address + port. +/// +/// Supports the same `[bind_address:]port` syntax as SSH `-L`. When no bind +/// address is given, defaults to `127.0.0.1`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ForwardSpec { + pub bind_addr: String, + pub port: u16, +} + +impl ForwardSpec { + /// Default bind address when none is specified. + pub const DEFAULT_BIND_ADDR: &str = "127.0.0.1"; + + /// Create a new `ForwardSpec` with the default bind address. + pub fn new(port: u16) -> Self { + Self { + bind_addr: Self::DEFAULT_BIND_ADDR.to_string(), + port, + } + } + + /// Parse a `[bind_address:]port` string. + /// + /// Examples: + /// - `"8080"` → `ForwardSpec { bind_addr: "127.0.0.1", port: 8080 }` + /// - `"0.0.0.0:8080"` → `ForwardSpec { bind_addr: "0.0.0.0", port: 8080 }` + /// - `"::1:8080"` → `ForwardSpec { bind_addr: "::1", port: 8080 }` + pub fn parse(s: &str) -> Result { + // Split on the last ':' to handle IPv6 addresses like "::1:8080". + if let Some(pos) = s.rfind(':') { + let addr = &s[..pos]; + let port_str = &s[pos + 1..]; + if let Ok(port) = port_str.parse::() { + if port == 0 { + return Err(miette::miette!("port must be between 1 and 65535")); + } + return Ok(Self { + bind_addr: addr.to_string(), + port, + }); + } + } + + // No colon or the part after the last colon isn't a valid port — + // treat the entire string as a port number. + let port: u16 = s.parse().map_err(|_| { + miette::miette!("invalid forward spec '{s}': expected [bind_address:]port") + })?; + if port == 0 { + return Err(miette::miette!("port must be between 1 and 65535")); + } + Ok(Self::new(port)) + } + + /// The SSH `-L` local-forward argument: `bind_addr:port:127.0.0.1:port`. + pub fn ssh_forward_arg(&self) -> String { + format!("{}:{}:127.0.0.1:{}", self.bind_addr, self.port, self.port) + } + + /// A human-readable URL for the forwarded port. + pub fn access_url(&self) -> String { + let host = if self.bind_addr == "0.0.0.0" || self.bind_addr == "::" { + "localhost" + } else { + &self.bind_addr + }; + format!("http://{host}:{}/", self.port) + } +} + +impl std::fmt::Display for ForwardSpec { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.bind_addr == Self::DEFAULT_BIND_ADDR { + write!(f, "{}", self.port) + } else { + write!(f, "{}:{}", self.bind_addr, self.port) + } + } +} + // --------------------------------------------------------------------------- // Port availability check // --------------------------------------------------------------------------- /// Check whether a local port is available for forwarding. /// -/// Attempts to bind `127.0.0.1:`. If the port is already in use, the +/// Attempts to bind `:`. If the port is already in use, the /// error message includes an actionable hint: /// /// - If an existing openshell forward owns the port, suggest the stop command. /// - Otherwise, suggest `lsof` to identify the owning process. -pub fn check_port_available(port: u16) -> Result<()> { - if TcpListener::bind(("127.0.0.1", port)).is_ok() { +pub fn check_port_available(spec: &ForwardSpec) -> Result<()> { + if TcpListener::bind((spec.bind_addr.as_str(), spec.port)).is_ok() { // Port is free — the listener is dropped immediately, releasing it. return Ok(()); } + let port = spec.port; + // Port is occupied. Check if it belongs to a tracked openshell forward. if let Ok(forwards) = list_forwards() && let Some(fwd) = forwards.iter().find(|f| f.port == port && f.alive) @@ -468,7 +555,7 @@ mod tests { let port = listener.local_addr().unwrap().port(); drop(listener); - assert!(check_port_available(port).is_ok()); + assert!(check_port_available(&ForwardSpec::new(port)).is_ok()); } #[test] @@ -477,7 +564,7 @@ mod tests { let port = listener.local_addr().unwrap().port(); // Keep the listener alive so the port stays occupied. - let result = check_port_available(port); + let result = check_port_available(&ForwardSpec::new(port)); assert!(result.is_err()); let msg = result.unwrap_err().to_string(); assert!( @@ -485,4 +572,71 @@ mod tests { "expected 'already in use' in error message, got: {msg}" ); } + + #[test] + fn forward_spec_parse_port_only() { + let spec = ForwardSpec::parse("8080").unwrap(); + assert_eq!(spec.bind_addr, "127.0.0.1"); + assert_eq!(spec.port, 8080); + } + + #[test] + fn forward_spec_parse_ipv4_and_port() { + let spec = ForwardSpec::parse("0.0.0.0:8080").unwrap(); + assert_eq!(spec.bind_addr, "0.0.0.0"); + assert_eq!(spec.port, 8080); + } + + #[test] + fn forward_spec_parse_ipv6_and_port() { + let spec = ForwardSpec::parse("::1:8080").unwrap(); + assert_eq!(spec.bind_addr, "::1"); + assert_eq!(spec.port, 8080); + } + + #[test] + fn forward_spec_parse_localhost_and_port() { + let spec = ForwardSpec::parse("localhost:3000").unwrap(); + assert_eq!(spec.bind_addr, "localhost"); + assert_eq!(spec.port, 3000); + } + + #[test] + fn forward_spec_parse_rejects_zero_port() { + assert!(ForwardSpec::parse("0").is_err()); + assert!(ForwardSpec::parse("0.0.0.0:0").is_err()); + } + + #[test] + fn forward_spec_parse_rejects_invalid() { + assert!(ForwardSpec::parse("abc").is_err()); + assert!(ForwardSpec::parse("").is_err()); + } + + #[test] + fn forward_spec_ssh_forward_arg() { + let spec = ForwardSpec::parse("0.0.0.0:8080").unwrap(); + assert_eq!(spec.ssh_forward_arg(), "0.0.0.0:8080:127.0.0.1:8080"); + + let spec = ForwardSpec::parse("8080").unwrap(); + assert_eq!(spec.ssh_forward_arg(), "127.0.0.1:8080:127.0.0.1:8080"); + } + + #[test] + fn forward_spec_access_url() { + let spec = ForwardSpec::parse("8080").unwrap(); + assert_eq!(spec.access_url(), "http://127.0.0.1:8080/"); + + let spec = ForwardSpec::parse("0.0.0.0:8080").unwrap(); + assert_eq!(spec.access_url(), "http://localhost:8080/"); + } + + #[test] + fn forward_spec_display() { + let spec = ForwardSpec::parse("8080").unwrap(); + assert_eq!(spec.to_string(), "8080"); + + let spec = ForwardSpec::parse("0.0.0.0:8080").unwrap(); + assert_eq!(spec.to_string(), "0.0.0.0:8080"); + } } diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index d0d47aa8..9d7f86f3 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -99,8 +99,14 @@ pub struct GatewayEntry { // --------------------------------------------------------------------------- /// Data extracted from the create sandbox form: -/// `(name, image, command, selected_provider_names, forward_ports)`. -pub type CreateFormData = (String, String, String, Vec, Vec); +/// `(name, image, command, selected_provider_names, forward_specs)`. +pub type CreateFormData = ( + String, + String, + String, + Vec, + Vec, +); /// Which field is focused in the create sandbox modal. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -336,8 +342,8 @@ pub struct App { // Create sandbox modal pub create_form: Option, pub pending_create_sandbox: bool, - /// Ports to forward after sandbox creation completes. - pub pending_forward_ports: Vec, + /// Forward specs to apply after sandbox creation completes. + pub pending_forward_ports: Vec, /// Command to exec via SSH after sandbox creation completes. pub pending_exec_command: String, /// Animation ticker handle — aborted when animation stops. @@ -1147,11 +1153,16 @@ impl App { .filter(|p| p.selected) .map(|p| p.name.clone()) .collect(); - let ports: Vec = form + let ports: Vec = form .ports .split(',') - .filter_map(|s| s.trim().parse::().ok()) - .filter(|&p| p > 0) + .filter_map(|s| { + let s = s.trim(); + if s.is_empty() { + return None; + } + openshell_core::forward::ForwardSpec::parse(s).ok() + }) .collect(); Some(( form.name.clone(), diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index a782d5ce..ceebe273 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1313,7 +1313,7 @@ async fn start_port_forwards( gateway_name: &str, sandbox_name: &str, sandbox_id: &str, - ports: &[u16], + specs: &[openshell_core::forward::ForwardSpec], ) { // Create SSH session. let session = { @@ -1358,13 +1358,16 @@ async fn start_port_forwards( session.sandbox_id, session.token, ); - // Start a forward for each port. - for port in ports { - if let Err(e) = openshell_core::forward::check_port_available(*port) { - tracing::warn!("skipping forward for port {port}: {e}"); + // Start a forward for each spec. + for spec in specs { + if let Err(e) = openshell_core::forward::check_port_available(spec) { + tracing::warn!("skipping forward for port {}: {e}", spec.port); continue; } + let ssh_forward_arg = spec.ssh_forward_arg(); + let port_val = spec.port; + let mut command = std::process::Command::new("ssh"); command .arg("-o") @@ -1382,13 +1385,12 @@ async fn start_port_forwards( .arg("-N") .arg("-f") .arg("-L") - .arg(format!("{port}:127.0.0.1:{port}")) + .arg(&ssh_forward_arg) .arg("sandbox") .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()); - let port_val = *port; let sid = session.sandbox_id.clone(); let name = sandbox_name.to_string(); From 2aae31a36a0848596550e5812c69fcb69163bcab Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 17:17:04 -0700 Subject: [PATCH 3/8] fix(cli): look up sandbox by port when stopping a forward Instead of falling back to the last-used sandbox when no name is provided, scan the forwards PID directory for the matching port. Ports are unique across sandboxes so the port alone is sufficient to identify which forward to stop. --- crates/openshell-cli/src/main.rs | 20 +++++++++++++++++--- crates/openshell-cli/src/run.rs | 4 +++- crates/openshell-core/src/forward.rs | 24 ++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 62d3b75d..85a41f1e 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1377,7 +1377,7 @@ enum ForwardCommands { /// Port that was forwarded. port: u16, - /// Sandbox name (defaults to last-used sandbox). + /// Sandbox name (auto-detected from active forwards if omitted). #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] name: Option, }, @@ -1575,8 +1575,22 @@ async fn main() -> Result<()> { command: Some(fwd_cmd), }) => match fwd_cmd { ForwardCommands::Stop { port, name } => { - let gateway_name = resolve_gateway_name(&cli.gateway).unwrap_or_default(); - let name = resolve_sandbox_name(name, &gateway_name)?; + let name = match name { + Some(n) => n, + None => match run::find_forward_by_port(port)? { + Some(n) => { + eprintln!("→ Found forward on sandbox '{n}'"); + n + } + None => { + eprintln!( + "{} No active forward found for port {port}", + "!".yellow(), + ); + return Ok(()); + } + }, + }; if run::stop_forward(&name, port)? { eprintln!( "{} Stopped forward of port {port} for sandbox {name}", diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index ccf25865..3eb0e233 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -48,7 +48,9 @@ pub use crate::ssh::{ sandbox_connect, sandbox_connect_editor, sandbox_exec, sandbox_forward, sandbox_ssh_proxy, sandbox_ssh_proxy_by_name, sandbox_sync_down, sandbox_sync_up, sandbox_sync_up_files, }; -pub use openshell_core::forward::{list_forwards, stop_forward, stop_forwards_for_sandbox}; +pub use openshell_core::forward::{ + find_forward_by_port, list_forwards, stop_forward, stop_forwards_for_sandbox, +}; /// Convert a sandbox phase integer to a human-readable string. fn phase_name(phase: i32) -> &'static str { diff --git a/crates/openshell-core/src/forward.rs b/crates/openshell-core/src/forward.rs index 27a07a72..a2f7b67c 100644 --- a/crates/openshell-core/src/forward.rs +++ b/crates/openshell-core/src/forward.rs @@ -122,6 +122,30 @@ pub fn pid_matches_forward(pid: u32, port: u16, sandbox_id: Option<&str>) -> boo sandbox_id.is_none_or(|id| cmd.contains(id)) } +/// Find the sandbox name that owns a forward on the given port. +/// +/// Scans all PID files in the forwards directory for a file matching +/// `*-.pid`. Ports are unique across sandboxes so at most one +/// match is expected. +pub fn find_forward_by_port(port: u16) -> Result> { + let dir = forward_pid_dir()?; + let entries = match std::fs::read_dir(&dir) { + Ok(e) => e, + Err(_) => return Ok(None), + }; + let suffix = format!("-{port}.pid"); + for entry in entries.flatten() { + let file_name = entry.file_name(); + let file_name = file_name.to_string_lossy(); + if let Some(name) = file_name.strip_suffix(&suffix) { + if !name.is_empty() { + return Ok(Some(name.to_string())); + } + } + } + Ok(None) +} + /// Stop a background port forward. pub fn stop_forward(name: &str, port: u16) -> Result { let pid_path = forward_pid_path(name, port)?; From 9280e2caf9b0468d3b8a93c9411b49c7115658b5 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 17:22:37 -0700 Subject: [PATCH 4/8] fix(cli): detect cross-address-family port conflicts before forwarding check_port_available only tested the requested bind address (e.g. 127.0.0.1), so a server listening on [::] (IPv6 wildcard) would not be detected. Now also runs lsof to catch any listener on the port regardless of address family. The error message includes the lsof output and a kill hint so users can free the port. --- crates/openshell-core/src/forward.rs | 90 +++++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 8 deletions(-) diff --git a/crates/openshell-core/src/forward.rs b/crates/openshell-core/src/forward.rs index a2f7b67c..718254dc 100644 --- a/crates/openshell-core/src/forward.rs +++ b/crates/openshell-core/src/forward.rs @@ -331,19 +331,35 @@ impl std::fmt::Display for ForwardSpec { /// Check whether a local port is available for forwarding. /// -/// Attempts to bind `:`. If the port is already in use, the -/// error message includes an actionable hint: +/// Uses a two-pronged check: +/// 1. Attempts to bind `:` — catches same-family conflicts. +/// 2. Runs `lsof -i : -sTCP:LISTEN` — catches cross-family conflicts +/// (e.g. an IPv6 wildcard listener blocking a port the IPv4 bind test +/// would miss). +/// +/// If the port is already in use the error message includes an actionable +/// hint: /// /// - If an existing openshell forward owns the port, suggest the stop command. -/// - Otherwise, suggest `lsof` to identify the owning process. +/// - Otherwise, show the `lsof` output and suggest `kill` to terminate the +/// owning process. pub fn check_port_available(spec: &ForwardSpec) -> Result<()> { - if TcpListener::bind((spec.bind_addr.as_str(), spec.port)).is_ok() { - // Port is free — the listener is dropped immediately, releasing it. + let port = spec.port; + + // Fast path: try binding on the requested address. If this fails, the + // port is definitely taken on this address family. + let bind_ok = TcpListener::bind((spec.bind_addr.as_str(), port)).is_ok(); + + // Also ask the OS whether *any* process is listening on this port, + // regardless of address family. This catches situations where e.g. a + // server binds [::]:8080 but our IPv4 bind test succeeds. + let lsof_output = lsof_listeners(port); + let lsof_occupied = lsof_output.is_some(); + + if bind_ok && !lsof_occupied { return Ok(()); } - let port = spec.port; - // Port is occupied. Check if it belongs to a tracked openshell forward. if let Ok(forwards) = list_forwards() && let Some(fwd) = forwards.iter().find(|f| f.port == port && f.alive) @@ -356,12 +372,49 @@ pub fn check_port_available(spec: &ForwardSpec) -> Result<()> { )); } + // Build a helpful error with lsof details when available. + if let Some(output) = lsof_output { + return Err(miette::miette!( + "Port {port} is already in use by another process.\n\n\ + {output}\n\n\ + To free the port, find the PID above and run:\n \ + kill \n\n\ + Or find it yourself with:\n \ + lsof -i :{port} -sTCP:LISTEN", + )); + } + Err(miette::miette!( "Port {port} is already in use by another process.\n\ - Find it with: lsof -i :{port} -sTCP:LISTEN", + Find it with: lsof -i :{port} -sTCP:LISTEN\n\ + Then terminate it with: kill ", )) } +/// Run `lsof` to check for any process listening on `port`. +/// +/// Returns the trimmed stdout if at least one listener is found, or `None` if +/// the port is free (or `lsof` is unavailable). +fn lsof_listeners(port: u16) -> Option { + let output = Command::new("lsof") + .arg("-i") + .arg(format!(":{port}")) + .arg("-sTCP:LISTEN") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if stdout.is_empty() { + None + } else { + Some(stdout) + } +} + // --------------------------------------------------------------------------- // SSH utility functions (shared between CLI and TUI) // --------------------------------------------------------------------------- @@ -597,6 +650,27 @@ mod tests { ); } + #[test] + fn check_port_available_occupied_ipv6_wildcard() { + // Bind on [::]:0 (IPv6 wildcard) — this simulates a server like + // `python3 -m http.server` which listens on [::] by default. The + // IPv4-only TcpListener::bind("127.0.0.1", port) might succeed, but + // lsof should detect the listener and the check should still fail. + let listener = match TcpListener::bind("[::]:0") { + Ok(l) => l, + Err(_) => return, // IPv6 not available, skip + }; + let port = listener.local_addr().unwrap().port(); + + let result = check_port_available(&ForwardSpec::new(port)); + assert!(result.is_err(), "expected error for IPv6-occupied port {port}"); + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("already in use"), + "expected 'already in use' in error message, got: {msg}" + ); + } + #[test] fn forward_spec_parse_port_only() { let spec = ForwardSpec::parse("8080").unwrap(); From 34c63c2b89e0906120e0eaed51453bd0c348f915 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 17:42:50 -0700 Subject: [PATCH 5/8] fix(cli): check port availability before creating sandbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the port availability check to the top of sandbox_create so it runs before the sandbox is provisioned. Previously the check only happened inside sandbox_forward, after the sandbox was already created — leaving an orphaned sandbox if the port was occupied. --- crates/openshell-cli/src/run.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 3eb0e233..91bfc17f 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1826,6 +1826,12 @@ pub async fn sandbox_create( )); } + // Check port availability *before* creating the sandbox so we don't + // leave an orphaned sandbox behind when the forward would fail. + if let Some(ref spec) = forward { + openshell_core::forward::check_port_available(spec)?; + } + // Try connecting to the gateway. If the connection fails due to a // connectivity error and bootstrap is allowed, start a new gateway. // From 9ae9cd43493046f9079107a10c0b9b5c87afa424 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 18:22:14 -0700 Subject: [PATCH 6/8] feat(cli): show bind address in forward list output Persist the bind address in the forward PID file as a third tab-separated field and display it as a BIND column in 'openshell forward list'. Old PID files without the field default to 127.0.0.1. --- crates/openshell-cli/src/main.rs | 18 +++++++++++---- crates/openshell-cli/src/ssh.rs | 2 +- crates/openshell-core/src/forward.rs | 34 ++++++++++++++++++++++++---- crates/openshell-tui/src/lib.rs | 3 ++- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 85a41f1e..e800bc98 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1614,12 +1614,20 @@ async fn main() -> Result<()> { .max() .unwrap_or(7) .max(7); + let bind_width = forwards + .iter() + .map(|f| f.bind_addr.len()) + .max() + .unwrap_or(4) + .max(4); println!( - "{: Result<()> { "dead".red().to_string() }; println!( - "{: Result { } /// Write a PID file for a background forward. -pub fn write_forward_pid(name: &str, port: u16, pid: u32, sandbox_id: &str) -> Result<()> { +/// +/// File format: `\t\t` +pub fn write_forward_pid( + name: &str, + port: u16, + pid: u32, + sandbox_id: &str, + bind_addr: &str, +) -> Result<()> { let dir = forward_pid_dir()?; std::fs::create_dir_all(&dir) .into_diagnostic() .wrap_err("failed to create forwards directory")?; let path = forward_pid_path(name, port)?; - std::fs::write(&path, format!("{pid}\t{sandbox_id}")) + std::fs::write(&path, format!("{pid}\t{sandbox_id}\t{bind_addr}")) .into_diagnostic() .wrap_err("failed to write forward PID file")?; Ok(()) @@ -72,6 +80,8 @@ pub fn find_ssh_forward_pid(sandbox_id: &str, port: u16) -> Option { pub struct ForwardPidRecord { pub pid: u32, pub sandbox_id: Option, + /// Bind address from the PID file, or `None` for old-format files. + pub bind_addr: Option, } /// Read the PID from a forward PID file. Returns `None` if the file does not @@ -79,10 +89,15 @@ pub struct ForwardPidRecord { pub fn read_forward_pid(name: &str, port: u16) -> Option { let path = forward_pid_path(name, port).ok()?; let contents = std::fs::read_to_string(path).ok()?; - let mut parts = contents.split_whitespace(); - let pid = parts.next()?.parse().ok()?; + let mut parts = contents.split('\t'); + let pid = parts.next()?.trim().parse().ok()?; let sandbox_id = parts.next().map(str::to_string); - Some(ForwardPidRecord { pid, sandbox_id }) + let bind_addr = parts.next().map(|s| s.trim().to_string()); + Some(ForwardPidRecord { + pid, + sandbox_id, + bind_addr, + }) } /// Check whether a process is alive. @@ -205,6 +220,8 @@ pub struct ForwardInfo { pub port: u16, pub pid: u32, pub alive: bool, + /// Bind address (defaults to `127.0.0.1` for old PID files). + pub bind_addr: String, } /// List all tracked forwards. @@ -232,6 +249,9 @@ pub fn list_forwards() -> Result> { port, pid: record.pid, alive: pid_is_alive(record.pid), + bind_addr: record + .bind_addr + .unwrap_or_else(|| ForwardSpec::DEFAULT_BIND_ADDR.to_string()), }); } } @@ -566,18 +586,21 @@ mod tests { port: 8080, pid: 123, alive: true, + bind_addr: "127.0.0.1".to_string(), }, ForwardInfo { sandbox: "mybox".to_string(), port: 3000, pid: 456, alive: true, + bind_addr: "127.0.0.1".to_string(), }, ForwardInfo { sandbox: "other".to_string(), port: 9090, pid: 789, alive: true, + bind_addr: "0.0.0.0".to_string(), }, ]; assert_eq!(build_sandbox_notes("mybox", &forwards), "fwd:8080,3000"); @@ -592,6 +615,7 @@ mod tests { port: 8080, pid: 123, alive: false, + bind_addr: "127.0.0.1".to_string(), }]; assert_eq!(build_sandbox_notes("mybox", &forwards), ""); } diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index ceebe273..25d156c3 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1367,6 +1367,7 @@ async fn start_port_forwards( let ssh_forward_arg = spec.ssh_forward_arg(); let port_val = spec.port; + let bind_addr = spec.bind_addr.clone(); let mut command = std::process::Command::new("ssh"); command @@ -1425,7 +1426,7 @@ async fn start_port_forwards( match result { Ok(Ok(true)) => { if let Some(pid) = openshell_core::forward::find_ssh_forward_pid(&sid, port_val) { - let _ = openshell_core::forward::write_forward_pid(&name, port_val, pid, &sid); + let _ = openshell_core::forward::write_forward_pid(&name, port_val, pid, &sid, &bind_addr); } } Ok(Ok(false)) => { From 36f60d1e958cb4ad8d7f14511c209683007de5ad Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 18:34:13 -0700 Subject: [PATCH 7/8] chore: fix rustfmt formatting --- crates/openshell-cli/src/main.rs | 5 +---- crates/openshell-core/src/forward.rs | 5 ++++- crates/openshell-tui/src/lib.rs | 4 +++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index e800bc98..944f002c 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1583,10 +1583,7 @@ async fn main() -> Result<()> { n } None => { - eprintln!( - "{} No active forward found for port {port}", - "!".yellow(), - ); + eprintln!("{} No active forward found for port {port}", "!".yellow(),); return Ok(()); } }, diff --git a/crates/openshell-core/src/forward.rs b/crates/openshell-core/src/forward.rs index ca77abfb..142252e5 100644 --- a/crates/openshell-core/src/forward.rs +++ b/crates/openshell-core/src/forward.rs @@ -687,7 +687,10 @@ mod tests { let port = listener.local_addr().unwrap().port(); let result = check_port_available(&ForwardSpec::new(port)); - assert!(result.is_err(), "expected error for IPv6-occupied port {port}"); + assert!( + result.is_err(), + "expected error for IPv6-occupied port {port}" + ); let msg = result.unwrap_err().to_string(); assert!( msg.contains("already in use"), diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 25d156c3..f63512b6 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1426,7 +1426,9 @@ async fn start_port_forwards( match result { Ok(Ok(true)) => { if let Some(pid) = openshell_core::forward::find_ssh_forward_pid(&sid, port_val) { - let _ = openshell_core::forward::write_forward_pid(&name, port_val, pid, &sid, &bind_addr); + let _ = openshell_core::forward::write_forward_pid( + &name, port_val, pid, &sid, &bind_addr, + ); } } Ok(Ok(false)) => { From 951cc01d8dc2024123264820925efdbe11a5ad8f Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 14 Mar 2026 20:59:36 -0700 Subject: [PATCH 8/8] fix(tui): remove blocking check_port_available from async port forward path The check_port_available call in start_port_forwards ran synchronous I/O (TcpListener::bind and lsof subprocess) inside a tokio::spawn async task, blocking the tokio worker thread and stalling the TUI event loop. This caused the TUI to hang after dropping to the shell for exec commands. The CLI paths (sandbox_create and sandbox_forward) already perform this check in appropriate synchronous contexts, and SSH itself will fail gracefully if the port is unavailable. --- crates/openshell-tui/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index f63512b6..f0891079 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1360,11 +1360,6 @@ async fn start_port_forwards( // Start a forward for each spec. for spec in specs { - if let Err(e) = openshell_core::forward::check_port_available(spec) { - tracing::warn!("skipping forward for port {}: {e}", spec.port); - continue; - } - let ssh_forward_arg = spec.ssh_forward_arg(); let port_val = spec.port; let bind_addr = spec.bind_addr.clone();