diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index a576d5a788..fad8a750f5 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -14,8 +14,9 @@ use miette::{IntoDiagnostic, Report, Result, WrapErr}; use nix::sys::signal::Signal; use openshell_core::driver_mounts; use openshell_core::forward::{ - ForwardSpec, build_proxy_command, format_gateway_url, resolve_ssh_gateway, sandbox_ssh_args, - shell_escape, validate_ssh_session_response, write_forward_pid, + ForwardSpec, build_proxy_command, format_gateway_url, proxy_command_escape, + resolve_ssh_gateway, sandbox_ssh_args, shell_escape, validate_ssh_session_response, + write_forward_pid, }; use openshell_core::proto::{ CreateSshSessionRequest, GetSandboxRequest, SandboxPhase, SshRelayTarget, TcpForwardFrame, @@ -1334,8 +1335,11 @@ fn ssh_upload_command(proxy_command: &str) -> Command { /// `"/sandbox/sub/file"` -> `("/sandbox/sub", "file")` /// `"file.txt"` -> `(".", "file.txt")` fn split_sandbox_path(path: &str) -> (&str, &str) { - match path.rfind('/') { - Some(0) => ("/", &path[1..]), + match path.rfind(['/', '\\']) { + Some(0) => (&path[..1], &path[1..]), + Some(pos) if pos == 2 && path.as_bytes().get(1) == Some(&b':') => { + (&path[..=pos], &path[pos + 1..]) + } Some(pos) => (&path[..pos], &path[pos + 1..]), None => (".", path), } @@ -2126,13 +2130,13 @@ fn host_alias(name: &str, workspace: &str) -> String { fn render_ssh_config(gateway: &str, name: &str, workspace: &str) -> String { let exe = std::env::current_exe().expect("failed to resolve OpenShell executable"); - let exe = shell_escape(&exe.to_string_lossy()); + let exe = proxy_command_escape(&exe.to_string_lossy()); let proxy_cmd = format!( "{exe} ssh-proxy --gateway-name {} --name {} --workspace {}", - shell_escape(gateway), - shell_escape(name), - shell_escape(workspace), + proxy_command_escape(gateway), + proxy_command_escape(name), + proxy_command_escape(workspace), ); let host_alias = host_alias(name, workspace); format!( diff --git a/crates/openshell-core/src/forward.rs b/crates/openshell-core/src/forward.rs index c8e6c10131..e7f6caa06a 100644 --- a/crates/openshell-core/src/forward.rs +++ b/crates/openshell-core/src/forward.rs @@ -906,7 +906,7 @@ pub fn format_gateway_url(scheme: &str, host: &str, port: u16) -> String { format!("{scheme}://{}:{port}", bracket_ipv6_host(host)) } -/// Shell-escape a value for use inside a `ProxyCommand` string. +/// POSIX shell-escape a value for use in command strings. pub fn shell_escape(value: &str) -> String { if value.is_empty() { return "''".to_string(); @@ -923,11 +923,55 @@ pub fn shell_escape(value: &str) -> String { format!("'{escaped}'") } +/// Quote one argument for the Windows command-line parser used by +/// `CreateProcessW`-launched programs. +/// +/// OpenSSH executes `ProxyCommand` directly on Windows instead of passing it +/// through `/bin/sh`. Always quoting each argument keeps executable paths with +/// spaces intact. Backslashes immediately before a quote or the closing quote +/// are doubled according to the Windows C runtime argument parsing rules. +#[cfg(any(windows, test))] +fn windows_command_line_escape(value: &str) -> String { + let mut escaped = String::with_capacity(value.len() + 2); + escaped.push('"'); + let mut backslashes = 0; + for character in value.chars() { + if character == '\\' { + backslashes += 1; + continue; + } + let count = if character == '"' { + backslashes * 2 + 1 + } else { + backslashes + }; + escaped.extend(std::iter::repeat_n('\\', count)); + backslashes = 0; + escaped.push(character); + } + escaped.extend(std::iter::repeat_n('\\', backslashes * 2)); + escaped.push('"'); + escaped +} + +/// Escape one argument for the command-launch behavior OpenSSH uses for +/// `ProxyCommand` on the current host platform. +pub fn proxy_command_escape(value: &str) -> String { + #[cfg(not(windows))] + { + shell_escape(value) + } + #[cfg(windows)] + { + windows_command_line_escape(value) + } +} + /// Build the SSH `ProxyCommand` string used to tunnel to a sandbox. /// -/// Every interpolated argument is shell-escaped so that server-supplied values -/// (gateway URL, sandbox name, workspace, token, gateway name) cannot inject shell -/// metacharacters into the command that OpenSSH executes via `/bin/sh -c`. +/// On Unix, every interpolated argument is shell-escaped for the `/bin/sh -c` +/// invocation used by OpenSSH. On Windows, every argument is quoted for the +/// command line that OpenSSH launches through `CreateProcessW`. pub fn build_proxy_command( exe: &str, gateway_url: &str, @@ -938,12 +982,12 @@ pub fn build_proxy_command( ) -> String { format!( "{} ssh-proxy --gateway {} --sandbox {} --workspace {} --token {} --gateway-name {}", - shell_escape(exe), - shell_escape(gateway_url), - shell_escape(sandbox_name), - shell_escape(workspace), - shell_escape(token), - shell_escape(gateway_name), + proxy_command_escape(exe), + proxy_command_escape(gateway_url), + proxy_command_escape(sandbox_name), + proxy_command_escape(workspace), + proxy_command_escape(token), + proxy_command_escape(gateway_name), ) } @@ -1346,6 +1390,7 @@ mod tests { } } + #[cfg(not(windows))] #[test] fn build_proxy_command_escapes_shell_metacharacters() { // Attacker-controlled values in every escapable position. @@ -1369,6 +1414,7 @@ mod tests { assert!(!outside_single_quotes(&cmd).contains('\n')); } + #[cfg(not(windows))] #[test] fn build_proxy_command_empty_values_quote_rather_than_vanish() { // An empty value must become `''` rather than disappearing — otherwise @@ -1378,6 +1424,7 @@ mod tests { assert!(cmd.contains("--workspace ''")); } + #[cfg(not(windows))] #[test] fn build_proxy_command_safe_values_pass_through_unquoted() { let cmd = build_proxy_command( @@ -1411,8 +1458,39 @@ mod tests { } } + #[test] + fn windows_command_line_escape_round_trips_through_crt_parsing() { + assert_eq!(windows_command_line_escape(""), r#""""#); + assert_eq!( + windows_command_line_escape(r"C:\Program Files\openshell.exe"), + r#""C:\Program Files\openshell.exe""# + ); + assert_eq!(windows_command_line_escape(r#"a"b"#), r#""a\"b""#); + assert_eq!(windows_command_line_escape(r#"a\"b"#), r#""a\\\"b""#); + assert_eq!(windows_command_line_escape(r"dir\"), r#""dir\\""#); + assert_eq!(windows_command_line_escape(r"a\b"), r#""a\b""#); + } + + #[cfg(windows)] + #[test] + fn build_proxy_command_quotes_every_windows_argument() { + let cmd = build_proxy_command( + r"C:\Program Files\openshell.exe", + "gw", + "", + "ws & calc", + "tok", + "name", + ); + assert_eq!( + cmd, + r#""C:\Program Files\openshell.exe" ssh-proxy --gateway "gw" --sandbox "" --workspace "ws & calc" --token "tok" --gateway-name "name""# + ); + } + /// Helper: return the concatenation of characters that appear outside /// POSIX single-quoted runs. Used by the metacharacter assertions above. + #[cfg(not(windows))] fn outside_single_quotes(s: &str) -> String { let mut out = String::new(); let mut inside = false; diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index b2ae367bfd..0d31462aa2 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -42,6 +42,7 @@ const DEFAULT_IMAGE_SENTINEL: &str = "mxc:process-container"; const HOST_AUTH_BUNDLE_FILE: &str = "supervisor-auth.json"; /// Directory in the sandbox host state directory for the proxy CA files. const HOST_PROXY_TLS_DIR: &str = "proxy-tls"; +const HOST_SSH_SERVER_NAME: &str = "supervisor-ssh"; const HOST_RUNTIME_DESCRIPTOR_FILE: &str = "runtime-descriptor.json"; const BOUNDARY_CONFIG_FILE: &str = "boundary.json"; const BOUNDARY_TLS_CERT_FILE: &str = "sandbox.crt"; @@ -538,6 +539,119 @@ impl MxcComputeBackend { Ok(()) } + /// Restart a stopped sandbox by running its lifecycle again with the fresh + /// launch credentials the gateway supplies for this start. + /// + /// Stop tears the runtime down completely; for an isolation session that + /// includes deprovisioning the agent user. Restart therefore provisions a + /// new runtime generation (and, for an isolation session, a new agent user) + /// from the stored sandbox spec. Host paths named by the policy, such as the + /// workspace, are unaffected by stop and so persist across the restart. + pub async fn start_sandbox( + &self, + sandbox_id: &str, + sandbox_name: &str, + fresh_launch_authentication: Vec, + ) -> Result<(), tonic::Status> { + if fresh_launch_authentication.is_empty() { + return Err(tonic::Status::invalid_argument( + "launch authentication is required to start an MXC sandbox", + )); + } + let gate = { + let registry = self.registry.lock().await; + let entry = registry.get(sandbox_id).ok_or_else(|| { + tonic::Status::not_found(format!("sandbox {sandbox_name} not found")) + })?; + if entry.sandbox.name != sandbox_name { + return Err(tonic::Status::failed_precondition( + "sandbox_id did not match sandbox_name", + )); + } + entry.lifecycle_gate.clone() + }; + let startup_guard = gate.lock_owned().await; + + let (mut sandbox, previous_host, previous_boundary) = { + let registry = self.registry.lock().await; + let entry = registry.get(sandbox_id).ok_or_else(|| { + tonic::Status::not_found(format!("sandbox {sandbox_name} not found")) + })?; + match entry.phase_state { + PhaseState::Stopped | PhaseState::Failed(_) => {} + // A retried start of a sandbox that is already up is not an error. + PhaseState::Running | PhaseState::Starting => return Ok(()), + } + ( + entry.sandbox.clone(), + entry.host_state_dir.clone(), + entry.boundary_state_dir.clone(), + ) + }; + if let Some(spec) = sandbox.spec.as_mut() { + spec.launch_authentication = fresh_launch_authentication; + } else { + return Err(tonic::Status::failed_precondition( + "stored MXC sandbox has no spec to restart", + )); + } + let launch = launch_authentication(&sandbox)?; + launch + .validate() + .map_err(|error| tonic::Status::failed_precondition(error.to_string()))?; + let sandbox_config = sandbox_config(&sandbox)?; + let generation = safe_component(launch.supervisor.runtime_generation.as_str())?.to_string(); + let host_state_dir = self + .config + .state_dir + .join(safe_component(sandbox_id)?) + .join(&generation); + let boundary_state_dir = PathBuf::from(&sandbox_config.cwd) + .join(".openshell-runtime") + .join(&generation); + if previous_host != host_state_dir { + cleanup_runtime_directory(&previous_host); + } + if previous_boundary != boundary_state_dir { + cleanup_runtime_directory(&previous_boundary); + } + + let starting = make_sandbox_with_condition( + &sandbox, + &condition("Ready", "False", "Starting", "MXC runtime restarting"), + false, + ); + { + let mut registry = self.registry.lock().await; + let entry = registry.get_mut(sandbox_id).ok_or_else(|| { + tonic::Status::not_found(format!("sandbox {sandbox_name} not found")) + })?; + entry.sandbox = starting.clone(); + entry.phase_state = PhaseState::Starting; + entry.shutdown_tx = None; + entry.terminated_rx = None; + entry.host_state_dir.clone_from(&host_state_dir); + entry.boundary_state_dir.clone_from(&boundary_state_dir); + } + let _ = self.watch_tx.send(sandbox_event(starting)); + let context = LifecycleContext { + invoker: self.invoker.clone(), + config: self.config.clone(), + gateway: self.gateway.clone(), + registry: self.registry.clone(), + watch_tx: self.watch_tx.clone(), + attribution: self.attribution.clone(), + sandbox, + sandbox_config, + generation, + host_state_dir, + boundary_state_dir, + policy_mapper: self.policy_mapper.clone(), + }; + tokio::spawn(async move { run_lifecycle(context, startup_guard).await }); + Ok(()) + } + pub async fn delete_sandbox( &self, sandbox_id: &str, @@ -908,6 +1022,13 @@ fn supervisor_command( openshell_core::sandbox_env::PROXY_TLS_DIR, context.host_state_dir.join(HOST_PROXY_TLS_DIR), ) + // Enables the supervisor's SSH server for exec, connect, and file + // transfer. On Windows the server is reached in process, so this path + // only names the server; no socket is created. + .env( + openshell_core::sandbox_env::SSH_SOCKET_PATH, + context.host_state_dir.join(HOST_SSH_SERVER_NAME), + ) .env( openshell_core::sandbox_env::LOG_LEVEL, openshell_core::driver_utils::sandbox_log_level(&context.sandbox, "info"), @@ -1006,23 +1127,34 @@ async fn set_failed(context: &LifecycleContext, message: &str) { } fn sandbox_config(sandbox: &DriverSandbox) -> Result { - let config = sandbox + let driver_config = sandbox .spec .as_ref() .and_then(|spec| spec.template.as_ref()) - .and_then(|template| template.driver_config.as_ref()) - .ok_or_else(|| { - tonic::Status::invalid_argument( - "mxc requires template.driver_config.mxc with command and cwd", - ) - })?; - let config: MxcSandboxConfig = - serde_json::from_value(struct_to_json_value(config)).map_err(|error| { + .and_then(|template| template.driver_config.as_ref()); + let config = match driver_config { + // The MXC-specific `--driver-config-json` is the most deliberately + // targeted input, so it takes priority over the generic command. + Some(config) => serde_json::from_value(struct_to_json_value(config)).map_err(|error| { tonic::Status::invalid_argument(format!("invalid mxc driver_config: {error}")) - })?; + })?, + // The driver-agnostic `sandbox create -- ` syntax. The working + // directory also hosts boundary state, so it must be writable by the + // gateway user; use a per-sandbox directory under its temp folder. + None => MxcSandboxConfig { + command: sandbox + .spec + .as_ref() + .map(|spec| spec.command.clone()) + .unwrap_or_default(), + cwd: default_sandbox_cwd(&sandbox.id)?, + denied_paths: Vec::new(), + }, + }; if config.command.first().is_none_or(String::is_empty) { return Err(tonic::Status::invalid_argument( - "mxc driver_config.command must contain an executable", + "mxc sandbox command must contain an executable: set it with \ + `sandbox create -- ` or `--driver-config-json`", )); } let mut denied_paths = Vec::with_capacity(config.denied_paths.len()); @@ -1352,6 +1484,19 @@ fn path_key(path: &str) -> Vec { .collect() } +fn default_sandbox_cwd(sandbox_id: &str) -> Result { + let directory = std::env::temp_dir() + .join("openshell-mxc") + .join(safe_component(sandbox_id)?); + std::fs::create_dir_all(&directory).map_err(|error| { + tonic::Status::internal(format!( + "create default MXC working directory {}: {error}", + directory.display() + )) + })?; + Ok(directory.display().to_string()) +} + fn launch_authentication( sandbox: &DriverSandbox, ) -> Result { @@ -1660,6 +1805,37 @@ fn platform_event(sandbox_id: String, reason: &str, message: String) -> WatchSan // the driver. Keep lifecycle/security tests below during their migration. #[cfg(test)] mod tests { + #[test] + fn generic_command_is_used_without_driver_config() { + let sandbox = DriverSandbox { + id: "generic-command-sandbox".to_string(), + spec: Some(DriverSandboxSpec { + command: vec!["cmd.exe".into(), "/c".into(), "exit".into()], + ..Default::default() + }), + ..Default::default() + }; + let config = sandbox_config(&sandbox).expect("generic command is accepted"); + assert_eq!(config.command, vec!["cmd.exe", "/c", "exit"]); + let cwd = Path::new(&config.cwd); + assert!(cwd.is_absolute() && cwd.is_dir(), "{}", config.cwd); + assert!(cwd.ends_with("generic-command-sandbox")); + let _ = std::fs::remove_dir(cwd); + + let empty = DriverSandbox { + id: "empty-command-sandbox".to_string(), + spec: Some(DriverSandboxSpec::default()), + ..Default::default() + }; + let error = sandbox_config(&empty).expect_err("a command is required"); + assert!(error.message().contains("sandbox create -- ")); + let _ = std::fs::remove_dir( + std::env::temp_dir() + .join("openshell-mxc") + .join("empty-command-sandbox"), + ); + } + #[test] fn mxc_driver_command_reaches_supervisor_without_generic_command() { let command = vec![ diff --git a/crates/openshell-driver-mxc/src/grpc.rs b/crates/openshell-driver-mxc/src/grpc.rs index 4c21172c47..60b31012ab 100644 --- a/crates/openshell-driver-mxc/src/grpc.rs +++ b/crates/openshell-driver-mxc/src/grpc.rs @@ -127,11 +127,16 @@ impl ComputeDriver for ComputeDriverService { async fn start_sandbox( &self, - _request: Request, + request: Request, ) -> Result, Status> { - Err(Status::unimplemented( - "mxc driver does not support restarting stopped sandboxes", - )) + let req = request.into_inner(); + if req.sandbox_id.is_empty() || req.name.is_empty() { + return Err(Status::invalid_argument("sandbox_id and name are required")); + } + self.backend + .start_sandbox(&req.sandbox_id, &req.name, req.launch_authentication) + .await?; + Ok(Response::new(StartSandboxResponse::default())) } async fn delete_sandbox( @@ -185,7 +190,7 @@ mod tests { use crate::driver::MxcComputeConfig; #[tokio::test] - async fn start_sandbox_reports_one_shot_lifecycle() { + async fn start_sandbox_validates_the_request() { let service = ComputeDriverService::new(MxcComputeBackend::new( openshell_core::config::DEFAULT_GATEWAY_NAME, MxcComputeConfig::default(), @@ -194,10 +199,30 @@ mod tests { let error = service .start_sandbox(Request::new(StartSandboxRequest::default())) .await - .expect_err("MXC must not restart a stopped one-shot workload"); + .expect_err("a start without a sandbox identity is rejected"); + assert_eq!(error.code(), tonic::Code::InvalidArgument); - assert_eq!(error.code(), tonic::Code::Unimplemented); - assert!(error.message().contains("does not support restarting")); + let error = service + .start_sandbox(Request::new(StartSandboxRequest { + sandbox_id: "missing".to_string(), + name: "missing".to_string(), + launch_authentication: Vec::new(), + ..Default::default() + })) + .await + .expect_err("a start without fresh launch credentials is rejected"); + assert_eq!(error.code(), tonic::Code::InvalidArgument); + + let error = service + .start_sandbox(Request::new(StartSandboxRequest { + sandbox_id: "missing".to_string(), + name: "missing".to_string(), + launch_authentication: b"{}".to_vec(), + ..Default::default() + })) + .await + .expect_err("an unknown sandbox cannot be started"); + assert_eq!(error.code(), tonic::Code::NotFound); } #[tokio::test] diff --git a/crates/openshell-mxc-boundary/src/lib.rs b/crates/openshell-mxc-boundary/src/lib.rs index d06070d79c..ff894dfcb5 100644 --- a/crates/openshell-mxc-boundary/src/lib.rs +++ b/crates/openshell-mxc-boundary/src/lib.rs @@ -16,6 +16,8 @@ pub mod backend; mod identity; #[cfg(target_os = "windows")] pub mod launch; +#[cfg(any(target_os = "windows", test))] +mod posix_command; #[cfg(target_os = "windows")] pub mod socket_owner; diff --git a/crates/openshell-mxc-boundary/src/posix_command.rs b/crates/openshell-mxc-boundary/src/posix_command.rs new file mode 100644 index 0000000000..0326f58dc2 --- /dev/null +++ b/crates/openshell-mxc-boundary/src/posix_command.rs @@ -0,0 +1,222 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Recover argv from the POSIX command strings that reach a Windows boundary. +//! +//! The gateway and SSH server deliver `exec` requests as one POSIX shell +//! command string built by `shell_escape`: an optional `cd &&` prefix, +//! `KEY=` assignments, then the escaped argv. Windows has no POSIX +//! shell, and handing that string to `cmd.exe` would mangle its quoting. This +//! module parses exactly that form back into structured parts so the boundary +//! can spawn the program directly with the caller's original arguments. +//! Strings that use any other shell syntax are not parsed; the caller runs them +//! through `cmd.exe` instead. + +/// A command string reduced to the pieces the boundary spawns directly. +#[derive(Debug, PartialEq, Eq)] +pub struct PosixCommand { + pub workdir: Option, + pub env: Vec<(String, String)>, + pub argv: Vec, +} + +#[derive(Debug, PartialEq, Eq)] +enum Token { + Word { text: String, quoted: bool }, + And, +} + +/// Parse `[cd &&] [NAME=]... ...`. +/// +/// Returns `None` when the string uses shell syntax beyond that form +/// (pipes, redirection, variables, globbing, or command separators), or when +/// no program word remains. +pub fn parse(command: &str) -> Option { + let mut tokens = tokenize(command)?.into_iter().peekable(); + let workdir = if matches!(tokens.peek(), Some(Token::Word { text, quoted: false }) if text == "cd") + { + tokens.next(); + let Some(Token::Word { text, .. }) = tokens.next() else { + return None; + }; + if tokens.next() != Some(Token::And) { + return None; + } + Some(text) + } else { + None + }; + + let mut env = Vec::new(); + let mut argv = Vec::new(); + for token in tokens { + let Token::Word { text, quoted } = token else { + return None; + }; + if argv.is_empty() + && !quoted + && let Some((name, value)) = assignment(&text) + { + env.push((name, value)); + continue; + } + argv.push(text); + } + if argv.is_empty() { + return None; + } + Some(PosixCommand { workdir, env, argv }) +} + +/// An unquoted leading `NAME=` makes the word an environment assignment. +fn assignment(word: &str) -> Option<(String, String)> { + let (name, value) = word.split_once('=')?; + let mut chars = name.chars(); + let first = chars.next()?; + if !(first == '_' || first.is_ascii_alphabetic()) + || !chars.all(|c| c == '_' || c.is_ascii_alphanumeric()) + { + return None; + } + Some((name.to_string(), value.to_string())) +} + +/// Split on unquoted whitespace. A word's `quoted` flag records whether quoting +/// began before its first `=`, which keeps a quoted `'A=b'` an argument. +fn tokenize(command: &str) -> Option> { + let mut tokens = Vec::new(); + let mut chars = command.chars().peekable(); + loop { + while chars.peek().is_some_and(char::is_ascii_whitespace) { + chars.next(); + } + let Some(&first) = chars.peek() else { + return Some(tokens); + }; + if first == '&' { + chars.next(); + if chars.next() != Some('&') { + return None; + } + tokens.push(Token::And); + continue; + } + + let mut text = String::new(); + let mut quote_before_equals = false; + while let Some(&c) = chars.peek() { + if c.is_ascii_whitespace() { + break; + } + chars.next(); + match c { + '\'' => { + quote_before_equals |= !text.contains('='); + loop { + match chars.next()? { + '\'' => break, + other => text.push(other), + } + } + } + '"' => { + quote_before_equals |= !text.contains('='); + loop { + match chars.next()? { + '"' => break, + '\\' => match chars.next()? { + escaped @ ('"' | '\\') => text.push(escaped), + other => { + text.push('\\'); + text.push(other); + } + }, + '$' | '`' => return None, + other => text.push(other), + } + } + } + '\\' => text.push(chars.next()?), + '|' | '&' | ';' | '<' | '>' | '(' | ')' | '$' | '`' | '*' | '?' | '[' | '{' + | '}' | '~' | '#' => return None, + other => text.push(other), + } + } + tokens.push(Token::Word { + text, + quoted: quote_before_equals, + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn argv(values: &[&str]) -> Vec { + values.iter().map(|value| (*value).to_string()).collect() + } + + #[test] + fn plain_arguments_round_trip() { + let parsed = parse("cmd.exe /d /c echo hello").expect("parsed"); + assert_eq!(parsed.argv, argv(&["cmd.exe", "/d", "/c", "echo", "hello"])); + assert_eq!(parsed.workdir, None); + assert!(parsed.env.is_empty()); + } + + #[test] + fn gateway_escaping_is_reversed() { + // shell_escape("it's a C:\\path") and an empty argument. + let parsed = parse(r#"'C:\Program Files\x.exe' 'it'"'"'s' '' plain"#).expect("parsed"); + assert_eq!( + parsed.argv, + argv(&[r"C:\Program Files\x.exe", "it's", "", "plain"]) + ); + } + + #[test] + fn workdir_and_environment_prefixes_are_extracted() { + let parsed = + parse(r"cd 'C:\work dir' && A=1 B='two words' tool.exe --flag").expect("parsed"); + assert_eq!(parsed.workdir.as_deref(), Some(r"C:\work dir")); + assert_eq!( + parsed.env, + vec![ + ("A".to_string(), "1".to_string()), + ("B".to_string(), "two words".to_string()) + ] + ); + assert_eq!(parsed.argv, argv(&["tool.exe", "--flag"])); + } + + #[test] + fn quoted_assignment_is_an_argument() { + let parsed = parse("'A=1' tool").expect("parsed"); + assert!(parsed.env.is_empty()); + assert_eq!(parsed.argv, argv(&["A=1", "tool"])); + } + + #[test] + fn assignment_after_program_is_an_argument() { + let parsed = parse("tool A=1").expect("parsed"); + assert_eq!(parsed.argv, argv(&["tool", "A=1"])); + } + + #[test] + fn shell_syntax_is_not_parsed() { + for command in [ + "echo a | more", + "echo a > out.txt", + "echo a; echo b", + "echo $HOME", + "echo *.txt", + "echo a && echo b", + "'unterminated", + "A=1", + "", + ] { + assert_eq!(parse(command), None, "{command}"); + } + } +} diff --git a/crates/openshell-mxc-boundary/src/runtime.rs b/crates/openshell-mxc-boundary/src/runtime.rs index 9a75b4281f..51873ae02f 100644 --- a/crates/openshell-mxc-boundary/src/runtime.rs +++ b/crates/openshell-mxc-boundary/src/runtime.rs @@ -873,11 +873,13 @@ impl BoundaryRuntime { ); let process_id = format!("{}:main:0", self.config.generation); let (program, args) = agent_command(spec.clone()); + let (args, raw_command_tail) = cmd_verbatim_tail(&program, args); let process = match ManagedProcess::spawn( process_id.clone(), ProcessKindWire::Main, program, args, + raw_command_tail, spec.workdir.clone(), environment, ) @@ -901,10 +903,10 @@ impl BoundaryRuntime { } async fn start_exec(&self, envelope: &RequestEnvelope, spec: ExecSpecWire) -> Response { - if spec.shell.is_some() || spec.runtime_helper.is_some() { + if spec.runtime_helper.is_some() { return guest_error( BoundaryErrorKind::Invalid, - "MXC does not support shell exec intent or runtime helpers", + "MXC does not support runtime helpers", ); } if spec.pty { @@ -913,6 +915,8 @@ impl BoundaryRuntime { "Windows ConPTY is not enabled for MXC exec", ); } + let shell_request = spec.shell.is_some(); + let launch = windows_launch(spec); { let mut requests = lock(&self.exec_requests); if !requests.insert(envelope.request_id.clone()) { @@ -930,20 +934,37 @@ impl BoundaryRuntime { let environment = workload_environment( &self.config.child_env, &lock(&self.provider_environment).1, - &spec.env, + &launch.env, &self.proxy_url, lock(&self.ca_paths).as_ref(), ); - let process = match ManagedProcess::spawn( + let retry = shell_request.then(|| cmd_retry(&launch)).flatten(); + let spawned = ManagedProcess::spawn( id.clone(), ProcessKindWire::Exec, - spec.program, - spec.args, - spec.workdir, - environment, + launch.program, + launch.args, + launch.raw_command_tail, + launch.workdir, + environment.clone(), ) - .await - { + .await; + let spawned = match (spawned, retry) { + (Err(_), Some(retry)) => { + ManagedProcess::spawn( + id.clone(), + ProcessKindWire::Exec, + retry.program, + retry.args, + retry.raw_command_tail, + retry.workdir, + environment, + ) + .await + } + (spawned, _) => spawned, + }; + let process = match spawned { Ok(process) => Arc::new(process), Err(error) => return guest_error(BoundaryErrorKind::Process, error), }; @@ -1182,6 +1203,144 @@ fn install_ca_environment( } } +/// The concrete Windows process an exec request resolves to. +#[derive(Debug, PartialEq, Eq)] +struct WindowsLaunch { + program: String, + args: Vec, + raw_command_tail: Option, + workdir: Option, + env: Vec<(String, String)>, +} + +/// Map an exec request onto a Windows process. +/// +/// Shell intent carries one POSIX command string. When it is the plain form +/// the gateway builds from an argv, the program runs directly with the original +/// arguments, environment, and working directory. Any other shell syntax runs +/// through `cmd.exe /d /s /c`, and a shell with no command starts `cmd.exe` +/// reading stdin. The login-shell flag has no Windows equivalent. +fn windows_launch(spec: ExecSpecWire) -> WindowsLaunch { + let ExecSpecWire { + program, + args, + shell, + env, + workdir, + .. + } = spec; + let Some(shell) = shell else { + return WindowsLaunch { + program, + args, + raw_command_tail: None, + workdir, + env, + }; + }; + let Some(command) = shell.command.filter(|command| !command.trim().is_empty()) else { + return WindowsLaunch { + program: "cmd.exe".to_string(), + args: vec!["/d".to_string(), "/q".to_string()], + raw_command_tail: None, + workdir, + env, + }; + }; + if let Some(parsed) = crate::posix_command::parse(&command) { + let mut words = parsed.argv.into_iter(); + let program = words.next().unwrap_or_default(); + let mut merged_env = env; + merged_env.extend(parsed.env); + let (args, raw_command_tail) = cmd_verbatim_tail(&program, words.collect()); + return WindowsLaunch { + program, + args, + raw_command_tail, + workdir: parsed.workdir.or(workdir), + env: merged_env, + }; + } + WindowsLaunch { + program: "cmd.exe".to_string(), + args: vec!["/d".to_string(), "/s".to_string(), "/c".to_string()], + raw_command_tail: Some(format!("\"{command}\"")), + workdir, + env, + } +} + +/// `cmd.exe /c` treats the rest of its command line as command text and does not +/// apply the C runtime quoting rules, so a quoted path such as `>"C:\x.txt"` +/// would reach it as `\"`. Pass everything after `/c` or `/k` verbatim, wrapped +/// in `/s` quotes, as a terminal would. Other programs keep their arguments. +fn cmd_verbatim_tail(program: &str, args: Vec) -> (Vec, Option) { + let is_cmd = Path::new(program) + .file_stem() + .is_some_and(|stem| stem.eq_ignore_ascii_case("cmd")); + let command_flag = args + .iter() + .position(|arg| arg.eq_ignore_ascii_case("/c") || arg.eq_ignore_ascii_case("/k")); + let (true, Some(flag)) = (is_cmd, command_flag) else { + return (args, None); + }; + let mut args = args; + let text = args.split_off(flag + 1).join(" "); + args.insert(flag, "/s".to_string()); + (args, Some(format!("\"{text}\""))) +} + +/// The `cmd.exe` launch used when a shell request's program cannot be started +/// directly, as a POSIX shell would report `command not found` itself. It also +/// resolves `cmd.exe` built-ins such as `echo`, `dir`, and `type`, and returns +/// cmd's own error text and exit code for a missing program. Without this, a +/// spawn failure leaves the SSH client waiting for an exit status that never +/// arrives. +fn cmd_retry(launch: &WindowsLaunch) -> Option { + if launch.raw_command_tail.is_some() { + return None; + } + let line = std::iter::once(launch.program.as_str()) + .chain(launch.args.iter().map(String::as_str)) + .map(quote_windows_argument) + .collect::>() + .join(" "); + Some(WindowsLaunch { + program: "cmd.exe".to_string(), + args: vec!["/d".to_string(), "/s".to_string(), "/c".to_string()], + raw_command_tail: Some(format!("\"{line}\"")), + workdir: launch.workdir.clone(), + env: launch.env.clone(), + }) +} + +/// Quote one argument with the Microsoft C runtime command-line rules. +fn quote_windows_argument(argument: &str) -> String { + if !argument.is_empty() && !argument.contains([' ', '\t', '"']) { + return argument.to_string(); + } + let mut quoted = String::from("\""); + let mut backslashes = 0_usize; + for c in argument.chars() { + match c { + '\\' => backslashes += 1, + '"' => { + quoted.push_str(&"\\".repeat(backslashes * 2 + 1)); + quoted.push('"'); + backslashes = 0; + } + other => { + quoted.push_str(&"\\".repeat(backslashes)); + quoted.push(other); + backslashes = 0; + } + } + } + quoted.push_str(&"\\".repeat(backslashes * 2)); + quoted.push('"'); + quoted +} + fn workload_environment( child: &HashMap, providers: &HashMap, @@ -1274,6 +1433,7 @@ impl ManagedProcess { kind: ProcessKindWire, program: String, args: Vec, + raw_command_tail: Option, workdir: Option, environment: HashMap, ) -> Result { @@ -1287,6 +1447,11 @@ impl ManagedProcess { .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .envs(environment); + // cmd.exe parses its own command line, so the text after /c must not be + // re-quoted with the C runtime rules that `Command::args` applies. + if let Some(tail) = raw_command_tail { + command.raw_arg(tail); + } if let Some(workdir) = workdir.filter(|path| !path.trim().is_empty()) { command.current_dir(workdir); } @@ -1690,6 +1855,212 @@ fn lock(mutex: &Mutex) -> std::sync::MutexGuard<'_, T> { #[cfg(test)] mod tests { use super::*; + use openshell_sandbox_backend::boundary_protocol::ShellSpecWire; + + fn shell_exec(command: Option<&str>) -> ExecSpecWire { + ExecSpecWire { + program: String::new(), + args: Vec::new(), + shell: Some(ShellSpecWire { + command: command.map(str::to_string), + login: true, + }), + runtime_helper: None, + env: vec![("FROM_REQUEST".to_string(), "1".to_string())], + workdir: Some(r"C:\default".to_string()), + pty: false, + } + } + + #[test] + fn plain_shell_command_runs_the_program_directly() { + let launch = windows_launch(shell_exec(Some( + r#"cd 'C:\work dir' && A=1 'C:\Program Files\tool.exe' 'it'"'"'s'"#, + ))); + assert_eq!(launch.program, r"C:\Program Files\tool.exe"); + assert_eq!(launch.args, vec!["it's".to_string()]); + assert_eq!(launch.workdir.as_deref(), Some(r"C:\work dir")); + assert_eq!(launch.raw_command_tail, None); + assert_eq!( + launch.env, + vec![ + ("FROM_REQUEST".to_string(), "1".to_string()), + ("A".to_string(), "1".to_string()) + ] + ); + } + + #[test] + fn shell_syntax_runs_through_cmd() { + let launch = windows_launch(shell_exec(Some("echo a | findstr a"))); + assert_eq!(launch.program, "cmd.exe"); + assert_eq!(launch.args, vec!["/d", "/s", "/c"]); + assert_eq!( + launch.raw_command_tail.as_deref(), + Some("\"echo a | findstr a\"") + ); + assert_eq!(launch.workdir.as_deref(), Some(r"C:\default")); + } + + #[test] + fn shell_without_command_starts_cmd() { + let launch = windows_launch(shell_exec(None)); + assert_eq!(launch.program, "cmd.exe"); + assert_eq!(launch.args, vec!["/d", "/q"]); + } + + #[test] + fn cmd_command_text_is_passed_verbatim() { + let launch = windows_launch(shell_exec(Some( + r#"'C:\Windows\System32\cmd.exe' /v:on /c '>"C:\dir\out.txt" echo !value!'"#, + ))); + assert_eq!(launch.program, r"C:\Windows\System32\cmd.exe"); + assert_eq!(launch.args, vec!["/v:on", "/s", "/c"]); + assert_eq!( + launch.raw_command_tail.as_deref(), + Some(r#"">"C:\dir\out.txt" echo !value!""#) + ); + assert_eq!(cmd_retry(&launch), None, "cmd launches are not retried"); + } + + #[tokio::test] + async fn cmd_writes_a_quoted_path() { + let directory = tempfile::tempdir().expect("temporary directory"); + let path = directory.path().join("out file.txt"); + let command = format!( + "cmd.exe /c {}", + openshell_core::forward::shell_escape(&format!(">\"{}\" echo written", path.display())) + ); + let launch = windows_launch(shell_exec(Some(&command))); + let process = ManagedProcess::spawn( + "cmd-quoted-path".to_string(), + ProcessKindWire::Exec, + launch.program, + launch.args, + launch.raw_command_tail, + None, + HashMap::new(), + ) + .await + .expect("cmd starts"); + let status = tokio::time::timeout(Duration::from_secs(10), process.wait()) + .await + .expect("process exits"); + assert!(matches!(status, ExitStatusWire::Exited(0)), "{status:?}"); + assert_eq!( + std::fs::read_to_string(&path).expect("file written").trim(), + "written" + ); + } + + #[test] + fn argv_exec_is_unchanged() { + let mut spec = shell_exec(None); + spec.shell = None; + spec.program = "tool.exe".to_string(); + spec.args = vec!["a b".to_string()]; + let launch = windows_launch(spec); + assert_eq!(launch.program, "tool.exe"); + assert_eq!(launch.args, vec!["a b".to_string()]); + assert_eq!(launch.raw_command_tail, None); + } + + #[tokio::test] + async fn missing_program_retries_through_cmd() { + let launch = windows_launch(shell_exec(Some("no-such-program.exe 'a b'"))); + let direct = ManagedProcess::spawn( + "retry-direct".to_string(), + ProcessKindWire::Exec, + launch.program.clone(), + launch.args.clone(), + None, + None, + HashMap::new(), + ) + .await; + assert!(direct.is_err(), "the program does not exist"); + let retry = cmd_retry(&launch).expect("parsed launches can retry"); + assert_eq!( + retry.raw_command_tail.as_deref(), + Some("\"no-such-program.exe \"a b\"\"") + ); + let process = ManagedProcess::spawn( + "retry-cmd".to_string(), + ProcessKindWire::Exec, + retry.program, + retry.args, + retry.raw_command_tail, + None, + HashMap::new(), + ) + .await + .expect("cmd starts"); + let status = tokio::time::timeout(Duration::from_secs(10), process.wait()) + .await + .expect("process exits"); + assert!( + matches!(status, ExitStatusWire::Exited(code) if code != 0), + "{status:?}" + ); + } + + #[test] + fn cmd_fallback_launches_are_not_retried() { + let launch = windows_launch(shell_exec(Some("echo a | findstr a"))); + assert_eq!(cmd_retry(&launch), None); + } + + #[test] + fn windows_arguments_are_quoted_for_the_c_runtime() { + assert_eq!(quote_windows_argument("plain"), "plain"); + assert_eq!(quote_windows_argument(""), "\"\""); + assert_eq!(quote_windows_argument("a b"), "\"a b\""); + assert_eq!(quote_windows_argument(r#"say "hi""#), r#""say \"hi\"""#); + assert_eq!( + quote_windows_argument(r"C:\dir with space\"), + r#""C:\dir with space\\""# + ); + } + + #[tokio::test] + async fn cmd_fallback_runs_shell_syntax() { + let launch = windows_launch(shell_exec(Some("echo first & echo second"))); + let process = ManagedProcess::spawn( + "cmd-fallback-test".to_string(), + ProcessKindWire::Exec, + launch.program, + launch.args, + launch.raw_command_tail, + None, + HashMap::new(), + ) + .await + .expect("launch cmd fallback"); + let status = tokio::time::timeout(Duration::from_secs(10), process.wait()) + .await + .expect("process exits"); + assert!(matches!(status, ExitStatusWire::Exited(0))); + let mut cursor = process.output.cursor(); + let output = tokio::time::timeout(Duration::from_secs(10), async { + let mut output = Vec::new(); + loop { + let event = cursor.recv().await.expect("retained output event"); + if event.channel == STREAM_STDOUT { + output.extend(event.payload); + } + if event.channel == STREAM_EXIT { + return output; + } + } + }) + .await + .expect("stdout is retained"); + let output = String::from_utf8_lossy(&output); + assert!( + output.contains("first") && output.contains("second"), + "{output}" + ); + } #[test] fn exec_environment_keeps_proxy_and_ca_with_refreshed_providers() { @@ -2009,6 +2380,7 @@ mod tests { "echo relay-free launch".to_string(), ], None, + None, HashMap::new(), ) .await @@ -2055,6 +2427,7 @@ mod tests { "(for /L %i in (1,1,2000) do @echo output-line) & echo last-error 1>&2".into(), ], None, + None, HashMap::new(), ) .await diff --git a/crates/openshell-supervisor-process/src/delegated.rs b/crates/openshell-supervisor-process/src/delegated.rs index 9f61c6e3e2..659b72e9a2 100644 --- a/crates/openshell-supervisor-process/src/delegated.rs +++ b/crates/openshell-supervisor-process/src/delegated.rs @@ -11,10 +11,8 @@ use miette::Result; use openshell_isolation_interface::contract::{ BoundaryExec, BoundaryLoopbackConnector, BoundaryProcess, }; -#[cfg(unix)] use openshell_ocsf::{ActivityId, AppLifecycleBuilder, SeverityId, StatusId, ocsf_emit}; -#[cfg(unix)] fn ocsf_ctx() -> &'static openshell_ocsf::EventContext { openshell_ocsf::ctx::ctx() } @@ -142,7 +140,6 @@ pub async fn start_boundary_access( } /// The optional SSH adapter is the only platform-specific access component. -#[cfg(unix)] async fn start_optional_ssh( socket_path: Option, shared_socket: bool, @@ -198,24 +195,6 @@ async fn start_optional_ssh( } } -#[cfg(not(unix))] -async fn start_optional_ssh( - socket_path: Option, - _shared_socket: bool, - _ca_file_paths: Option<(std::path::PathBuf, std::path::PathBuf)>, - _boundary_exec: Arc, - _port_forward: Arc, - _main_session: Arc, - _host_key: Option, -) -> Result>> { - if socket_path.is_some() { - return Err(miette::miette!( - "SSH access sockets are unsupported on this host" - )); - } - Ok(None) -} - /// Report the canonical process exit until the gateway acknowledges it. pub async fn report_main_process_exit( endpoint: &str, @@ -384,7 +363,7 @@ mod tests { #[cfg(not(unix))] #[tokio::test] - async fn explicit_unix_ssh_socket_is_rejected() { + async fn in_process_ssh_requires_a_host_key() { let boundary = Arc::new(AccessBoundary); let result = start_boundary_access( None, @@ -402,9 +381,9 @@ mod tests { assert!( result .err() - .expect("Unix SSH unsupported") + .expect("SSH without a host key must be rejected") .to_string() - .contains("unsupported") + .contains("host key is missing") ); } diff --git a/crates/openshell-supervisor-process/src/lib.rs b/crates/openshell-supervisor-process/src/lib.rs index df0f21a519..b2c9ef22fb 100644 --- a/crates/openshell-supervisor-process/src/lib.rs +++ b/crates/openshell-supervisor-process/src/lib.rs @@ -12,7 +12,6 @@ pub mod delegated; pub mod log_push; pub mod main_session; pub mod skills; -#[cfg(unix)] pub mod ssh; pub mod supervisor_session; diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 58b3941df1..676b4b510d 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -6,16 +6,18 @@ mod input; mod upload; -#[cfg(test)] +#[cfg(all(test, unix))] mod exec_input_tests; -#[cfg(test)] +#[cfg(all(test, unix))] mod upload_tests; use crate::main_session::{MainOutput, MainSession}; #[cfg(unix)] use libc; -use miette::{IntoDiagnostic, Result}; +#[cfg(unix)] +use miette::IntoDiagnostic; +use miette::Result; use openshell_core::VERSION; use openshell_ocsf::{ ActionId, ActivityId, DispositionId, SeverityId, SshActivityBuilder, StatusId, ocsf_emit, @@ -25,9 +27,12 @@ use russh::server::{Auth, ChannelOpenHandle, Handle, Session}; use russh::{ChannelId, ChannelOpenFailure, Sig}; use std::borrow::Cow; use std::collections::HashMap; -use std::path::{Path, PathBuf}; +#[cfg(unix)] +use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +#[cfg(unix)] use tokio::net::UnixListener; use tracing::warn; @@ -40,7 +45,7 @@ const SSH_PEER_TIMEOUT: Duration = Duration::from_mins(1); mod peer_stream; -#[cfg(test)] +#[cfg(all(test, unix))] #[path = "ssh/reconnect_tests.rs"] mod reconnect_tests; @@ -70,6 +75,7 @@ fn filter_main_detach_sequence(prefix_pending: &mut bool, data: &[u8]) -> (Vec, @@ -91,12 +97,7 @@ pub fn parse_host_key(key: Option<&openshell_core::jwt::SecretSshHostKey>) -> Re Ok(key) } -fn ssh_server_init( - listen_path: &Path, - ca_file_paths: &Option<(PathBuf, PathBuf)>, - shared_socket: bool, - host_key: PrivateKey, -) -> Result { +fn ssh_server_config(host_key: PrivateKey) -> Arc { let mut config = russh::server::Config { server_id: russh::SshId::Standard(Cow::Owned(format!("SSH-2.0-OpenShell_{VERSION}"))), auth_rejection_time: Duration::from_secs(1), @@ -106,8 +107,17 @@ fn ssh_server_init( ..Default::default() }; config.keys.push(host_key); + Arc::new(config) +} - let config = Arc::new(config); +#[cfg(unix)] +fn ssh_server_init( + listen_path: &Path, + ca_file_paths: &Option<(PathBuf, PathBuf)>, + shared_socket: bool, + host_key: PrivateKey, +) -> Result { + let config = ssh_server_config(host_key); let ca_paths = ca_file_paths.as_ref().map(|p| Arc::new(p.clone())); // A driver may place the supervisor in another container, so an explicitly @@ -153,6 +163,7 @@ fn ssh_server_init( Ok((listener, config, ca_paths)) } +#[cfg(unix)] #[allow(clippy::too_many_arguments, clippy::implicit_hasher)] pub async fn run_ssh_server( listen_path: PathBuf, @@ -241,8 +252,10 @@ pub async fn run_ssh_server( } } +#[cfg(unix)] const MAX_CONSECUTIVE_UNKNOWN_SSH_ACCEPT_ERRORS: u32 = 10; +#[cfg(unix)] #[derive(Debug, PartialEq)] enum SshAcceptAction { Terminal, @@ -252,6 +265,7 @@ enum SshAcceptAction { }, } +#[cfg(unix)] fn classify_ssh_accept_error( error: &std::io::Error, consecutive_resource_errors: &mut u32, @@ -333,18 +347,208 @@ fn classify_ssh_accept_error( } } +/// Windows has no socket hop between the relay and the SSH server: both run in +/// the supervisor process, so each accepted relay is handed to the server as +/// one half of an in-memory duplex stream. Nothing outside the supervisor +/// process can reach the server. The configured socket path is only the +/// registry key that pairs a relay with its server. +#[cfg(not(unix))] +mod in_process { + use std::collections::HashMap; + use std::io; + use std::path::{Path, PathBuf}; + use std::sync::{Mutex, OnceLock, PoisonError}; + use tokio::io::DuplexStream; + use tokio::sync::mpsc; + + const DUPLEX_BUFFER_BYTES: usize = 256 * 1024; + const ACCEPT_BACKLOG: usize = 64; + + type Acceptors = Mutex>>; + + fn acceptors() -> &'static Acceptors { + static ACCEPTORS: OnceLock = OnceLock::new(); + ACCEPTORS.get_or_init(|| Mutex::new(HashMap::new())) + } + + /// Removes the server's acceptor when the server task ends, so later relays + /// fail instead of queueing on a dead server. + pub(super) struct Registration { + key: PathBuf, + } + + impl Drop for Registration { + fn drop(&mut self) { + acceptors() + .lock() + .unwrap_or_else(PoisonError::into_inner) + .remove(&self.key); + } + } + + pub(super) fn register( + key: &Path, + ) -> Result<(Registration, mpsc::Receiver), String> { + let mut map = acceptors().lock().unwrap_or_else(PoisonError::into_inner); + if map.contains_key(key) { + return Err(format!( + "an SSH server is already registered for {}", + key.display() + )); + } + let (sender, receiver) = mpsc::channel(ACCEPT_BACKLOG); + map.insert(key.to_path_buf(), sender); + Ok(( + Registration { + key: key.to_path_buf(), + }, + receiver, + )) + } + + /// Open a connection to the in-process SSH server registered under `key`. + pub async fn connect(key: &Path) -> io::Result { + let sender = acceptors() + .lock() + .unwrap_or_else(PoisonError::into_inner) + .get(key) + .cloned() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::NotFound, + "supervisor SSH server is not running", + ) + })?; + let (client, server) = tokio::io::duplex(DUPLEX_BUFFER_BYTES); + sender.send(server).await.map_err(|_| { + io::Error::new( + io::ErrorKind::ConnectionRefused, + "supervisor SSH server stopped", + ) + })?; + Ok(client) + } +} + +#[cfg(not(unix))] +pub use in_process::connect as connect_in_process; + +#[cfg(all(test, not(unix)))] +mod in_process_tests { + use super::in_process; + use std::path::Path; + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + + #[tokio::test] + async fn connections_reach_only_the_registered_server() { + let key = Path::new(r"C:\state\sandbox-a\supervisor-ssh"); + let (registration, mut accepted) = in_process::register(key).expect("register"); + assert!( + in_process::register(key).is_err(), + "a second server cannot claim the same key" + ); + + let mut client = in_process::connect(key).await.expect("connect"); + let mut server = accepted.recv().await.expect("server receives the stream"); + client.write_all(b"ping").await.unwrap(); + let mut buffer = [0_u8; 4]; + server.read_exact(&mut buffer).await.unwrap(); + assert_eq!(&buffer, b"ping"); + + let other = in_process::connect(Path::new(r"C:\state\sandbox-b\supervisor-ssh")).await; + assert_eq!( + other.err().map(|error| error.kind()), + Some(std::io::ErrorKind::NotFound) + ); + + drop(registration); + let stopped = in_process::connect(key).await; + assert_eq!( + stopped.err().map(|error| error.kind()), + Some(std::io::ErrorKind::NotFound), + "a stopped server no longer accepts connections" + ); + } +} + +#[cfg(not(unix))] +#[allow(clippy::too_many_arguments, clippy::implicit_hasher)] +pub async fn run_ssh_server( + listen_path: PathBuf, + ready_tx: tokio::sync::oneshot::Sender>, + _ca_file_paths: Option<(PathBuf, PathBuf)>, + _shared_socket: bool, + port_forward: Arc, + boundary_exec: Arc, + main_session: Option>, + host_key: PrivateKey, +) -> Result<()> { + let config = ssh_server_config(host_key); + let (_registration, mut accepted) = match in_process::register(&listen_path) { + Ok(registered) => { + let _ = ready_tx.send(Ok(())); + registered + } + Err(error) => { + let _ = ready_tx.send(Err(miette::miette!(error))); + return Ok(()); + } + }; + ocsf_emit!( + SshActivityBuilder::new(openshell_ocsf::ctx::ctx()) + .activity(ActivityId::Listen) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .message("SSH server listening on the supervisor in-process transport") + .build() + ); + + while let Some(stream) = accepted.recv().await { + let config = config.clone(); + let port_forward = port_forward.clone(); + let boundary_exec = boundary_exec.clone(); + let main_session = main_session.clone(); + tokio::spawn(async move { + if let Err(err) = handle_connection( + stream, + config, + port_forward, + boundary_exec, + main_session, + SSH_PEER_TIMEOUT, + ) + .await + { + ocsf_emit!( + SshActivityBuilder::new(openshell_ocsf::ctx::ctx()) + .activity(ActivityId::Fail) + .severity(SeverityId::Low) + .status(StatusId::Failure) + .message(format!("SSH connection failed: {err}")) + .build() + ); + } + }); + } + Ok(()) +} + #[allow(clippy::too_many_arguments)] -async fn handle_connection( - stream: tokio::net::UnixStream, +async fn handle_connection( + stream: S, config: Arc, port_forward: Arc, boundary_exec: Arc, main_session: Option>, peer_timeout: Duration, -) -> Result<()> { - // Access is gated by the Unix-socket filesystem permissions (root-only), - // not by an application-level preface. The supervisor bridges the - // gateway's RelayStream directly into this socket. +) -> Result<()> +where + S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin + Send + 'static, +{ + // Access is gated by the Unix-socket filesystem permissions (root-only) or, + // on Windows, by the stream never leaving the supervisor process. There is + // no application-level preface. The supervisor bridges the gateway's + // RelayStream directly into this stream. ocsf_emit!( SshActivityBuilder::new(openshell_ocsf::ctx::ctx()) .activity(ActivityId::Open) @@ -352,7 +556,11 @@ async fn handle_connection( .disposition(DispositionId::Allowed) .severity(SeverityId::Informational) .status(StatusId::Success) - .message("SSH connection accepted on supervisor Unix socket") + .message(if cfg!(unix) { + "SSH connection accepted on supervisor Unix socket" + } else { + "SSH connection accepted on supervisor in-process transport" + }) .build() ); @@ -1530,7 +1738,7 @@ fn direct_tcpip_target( openshell_isolation_interface::contract::LoopbackTarget::new(ip, port).ok() } -#[cfg(test)] +#[cfg(all(test, unix))] #[allow( clippy::doc_markdown, reason = "Test documentation references protocol and API identifiers." diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index 898af8672d..19240230e6 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -968,15 +968,19 @@ async fn open_target( expected_ssh_peer_pid: Option, ) -> Result, Box> { #[cfg(not(unix))] - let _ = (ssh_socket_path, expected_ssh_peer_pid); + let _ = expected_ssh_peer_pid; match relay_open.target.as_ref() { Some(relay_open::Target::Tcp(target)) => open_tcp_target(target, port_forward).await, Some(relay_open::Target::Ssh(_)) | None => { if ssh_socket_path.as_os_str().is_empty() { return Err("SSH access is not configured for this supervisor".into()); } + // The SSH server runs in this process, so there is no peer to + // authenticate: the stream never leaves the supervisor. #[cfg(not(unix))] - return Err("SSH relay targets are unsupported by the Windows supervisor".into()); + return Ok(Box::new( + crate::ssh::connect_in_process(ssh_socket_path).await?, + )); #[cfg(unix)] { let runtime_path = crate::unix_socket::runtime_path(ssh_socket_path); diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 212cdc5366..442c20bfac 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -769,7 +769,7 @@ async fn run_sandbox_with_backend( } = config; // The current client attachment adapter is SSH; retaining a process's I/O // internally does not make it reachable by a foreground client. - if await_main_process_attachment && (!cfg!(unix) || ssh_socket_path.is_none()) { + if await_main_process_attachment && ssh_socket_path.is_none() { return Err(miette::miette!( "foreground attachment to the main process requires a supported SSH access adapter; create the sandbox with --detach" )); @@ -819,7 +819,6 @@ async fn run_sandbox_with_backend( &auth_bundle, )?; let sandbox_bearer = openshell_core::grpc_client::install_supervisor_auth_bundle(&auth_bundle)?; - #[cfg(unix)] let ssh_host_key = if ssh_socket_path.is_some() { Some(openshell_supervisor_process::ssh::parse_host_key( auth_bundle.ssh_host_private_key.as_ref(), @@ -827,15 +826,6 @@ async fn run_sandbox_with_backend( } else { None }; - #[cfg(not(unix))] - let ssh_host_key = { - if ssh_socket_path.is_some() { - return Err(miette::miette!( - "SSH access sockets are unsupported on this host" - )); - } - None - }; // Startup joins the trace that created the sandbox when the driver passes // one, and ends once the access plane is up. let startup = tracing::info_span!(