diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 151bd166..944f002c 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))] @@ -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,19 @@ 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}", @@ -1600,12 +1611,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<()> { 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 +1887,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..91bfc17f 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 { @@ -1726,7 +1728,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 +1769,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 +1813,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, @@ -1821,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. // @@ -1918,7 +1929,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 +2206,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 +4435,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 a834d0a4..7f8e7905 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -307,10 +307,12 @@ 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(spec)?; + let session = ssh_session_config(server, name, tls).await?; let mut command = TokioCommand::from(ssh_base_command(&session.proxy_command)); @@ -319,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. @@ -332,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 { @@ -339,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()? } } @@ -352,7 +356,7 @@ 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)?; + write_forward_pid(name, port, pid, &session.sandbox_id, &spec.bind_addr)?; } else { eprintln!( "{} Could not discover backgrounded SSH process; \ @@ -365,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(), ) } @@ -1130,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")); @@ -1139,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 8d84ed17..142252e5 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; @@ -33,13 +34,21 @@ pub fn forward_pid_path(name: &str, port: u16) -> 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(()) @@ -71,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 @@ -78,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. @@ -121,6 +137,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)?; @@ -180,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. @@ -207,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()), }); } } @@ -215,6 +260,181 @@ 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. +/// +/// 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, show the `lsof` output and suggest `kill` to terminate the +/// owning process. +pub fn check_port_available(spec: &ForwardSpec) -> Result<()> { + 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(()); + } + + // 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, + )); + } + + // 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\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) // --------------------------------------------------------------------------- @@ -366,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"); @@ -392,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), ""); } @@ -423,4 +647,121 @@ 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(&ForwardSpec::new(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(&ForwardSpec::new(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}" + ); + } + + #[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(); + 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 98f4ed0d..f0891079 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,8 +1358,12 @@ async fn start_port_forwards( session.sandbox_id, session.token, ); - // Start a forward for each port. - for port in ports { + // Start a forward for each spec. + for spec in specs { + 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 .arg("-o") @@ -1377,13 +1381,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(); @@ -1418,7 +1421,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); + let _ = openshell_core::forward::write_forward_pid( + &name, port_val, pid, &sid, &bind_addr, + ); } } Ok(Ok(false)) => {