From 158c9cb0656d306a805e85701e377baa0191c854 Mon Sep 17 00:00:00 2001 From: Akram Date: Thu, 3 Sep 2026 01:28:07 +0400 Subject: [PATCH 1/3] fix(sandbox): detect an available login shell instead of hardcoding /bin/bash The built-in default sandbox command and the interactive SSH session hardcoded /bin/bash. Minimal images such as Alpine ship only /bin/sh (BusyBox ash), so sandbox startup failed with an opaque "No such file or directory (os error 2)" that never named the missing binary. Add openshell-core::shell with shell-path constants and a runtime detect_login_shell() that resolves a shell present in the sandbox image ($SHELL if executable, then bash, then /bin/sh). Use it for: - the built-in default command (only the default is remapped; explicit user commands are never rewritten), resolved in the supervisor so it inspects the sandbox filesystem rather than the gateway's - the SSH interactive shell - the SHELL environment variable Also name the program in the spawn error so a missing shell/binary is diagnosable instead of a bare ENOENT. Refs #3146 Signed-off-by: Akram --- crates/openshell-core/src/lib.rs | 1 + crates/openshell-core/src/shell.rs | 98 +++++++++++++++++++ crates/openshell-sandbox/src/main.rs | 76 ++++++++++---- .../src/process.rs | 17 +++- .../openshell-supervisor-process/src/ssh.rs | 12 ++- 5 files changed, 180 insertions(+), 24 deletions(-) create mode 100644 crates/openshell-core/src/shell.rs diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index 03ae8a30fd..7acb72dd6f 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -45,6 +45,7 @@ pub mod provider_credentials; pub mod sandbox_env; pub mod secrets; pub mod settings; +pub mod shell; pub mod spiffe; pub mod telemetry; pub mod time; diff --git a/crates/openshell-core/src/shell.rs b/crates/openshell-core/src/shell.rs new file mode 100644 index 0000000000..b22e2ae356 --- /dev/null +++ b/crates/openshell-core/src/shell.rs @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Login-shell resolution for sandbox images. +//! +//! The default sandbox command and the interactive SSH session need a shell, +//! but not every base image ships the same one. Debian-based images provide +//! `bash`; minimal images such as Alpine only provide `/bin/sh` (`BusyBox` +//! `ash`). Hard-coding `/bin/bash` makes sandbox startup fail on those images +//! with an opaque `No such file or directory`. +//! +//! These helpers resolve a shell that actually exists in the current root +//! filesystem. They must run inside the sandbox (i.e. in the supervisor), not +//! on the gateway, because the answer depends on the sandbox image's contents. + +/// Preferred interactive shell when the image provides it. +pub const BASH: &str = "/bin/bash"; + +/// POSIX shell. Guaranteed on virtually every image, including Alpine/`BusyBox`. +/// Used as the ultimate fallback. +pub const POSIX_SH: &str = "/bin/sh"; + +/// Shell paths tried, in preference order, by [`detect_login_shell`]. +pub const SHELL_CANDIDATES: &[&str] = &[BASH, "/usr/bin/bash", POSIX_SH]; + +/// Return `true` if `path` is a regular, executable file in the current root +/// filesystem. +#[must_use] +pub fn is_executable(path: &str) -> bool { + let Ok(meta) = std::fs::metadata(path) else { + return false; + }; + if !meta.is_file() { + return false; + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + meta.permissions().mode() & 0o111 != 0 + } + #[cfg(not(unix))] + { + true + } +} + +/// Resolve a login shell that exists in the current root filesystem. +/// +/// Resolution order: `$SHELL` (when set and executable), then +/// [`SHELL_CANDIDATES`], falling back to [`POSIX_SH`]. Because this inspects +/// the filesystem, call it from the supervisor (inside the sandbox), never on +/// the gateway. +#[must_use] +pub fn detect_login_shell() -> String { + if let Ok(shell) = std::env::var("SHELL") + && is_executable(&shell) + { + return shell; + } + SHELL_CANDIDATES + .iter() + .find(|candidate| is_executable(candidate)) + .map_or_else( + || POSIX_SH.to_string(), + |candidate| (*candidate).to_string(), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn posix_sh_exists_on_test_host() { + // /bin/sh is present on all supported CI hosts (Linux and macOS). + assert!(is_executable(POSIX_SH)); + } + + #[test] + fn missing_path_is_not_executable() { + assert!(!is_executable("/nonexistent/definitely/not/here")); + } + + #[test] + fn non_file_is_not_executable() { + // A directory is not an executable shell. + assert!(!is_executable("/")); + } + + #[test] + fn detect_returns_an_executable_shell() { + let shell = detect_login_shell(); + assert!( + is_executable(&shell), + "detected shell {shell} is not executable" + ); + } +} diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 7108378654..9516436ea0 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -111,7 +111,9 @@ impl std::str::FromStr for Mode { #[command(about = "Process sandbox and monitor", long_about = None)] struct Args { /// Command to execute in the sandbox. - /// Defaults to `/bin/bash -l` if neither this nor the driver specification is provided. + /// Defaults to a login shell if neither this nor the driver specification is + /// provided: `/bin/bash -l` when available, otherwise a shell detected in the + /// sandbox image (e.g. `/bin/sh` on Alpine). #[arg(trailing_var_arg = true)] command: Vec, @@ -659,23 +661,41 @@ fn main() -> Result<()> { // drivers otherwise provide a versioned JSON transport so argument // boundaries are never reconstructed with shell parsing. let workdir = args.workdir.clone(); - let (command, interactive, await_main_process_attachment) = if !args.command.is_empty() { - (args.command, args.interactive, false) - } else if let Ok(json) = std::env::var(openshell_core::sandbox_env::MAIN_PROCESS_SPEC) { - let config = openshell_core::sandbox_env::MainProcessConfig::decode(&json) - .map_err(|error| miette::miette!("{error}"))?; - ( - config.command, - config.tty, - config.await_main_process_attachment, - ) + let (command, interactive, await_main_process_attachment, is_default_command) = + if !args.command.is_empty() { + (args.command, args.interactive, false, false) + } else if let Ok(json) = std::env::var(openshell_core::sandbox_env::MAIN_PROCESS_SPEC) { + let config = openshell_core::sandbox_env::MainProcessConfig::decode(&json) + .map_err(|error| miette::miette!("{error}"))?; + // The driver bakes the built-in default (`scratch`) into the spec + // when the user supplies no command. Detect that case so the + // default shell can be resolved against the sandbox image below. + let is_default = + config == openshell_core::sandbox_env::MainProcessConfig::scratch(); + ( + config.command, + config.tty, + config.await_main_process_attachment, + is_default, + ) + } else { + let config = openshell_core::sandbox_env::MainProcessConfig::scratch(); + ( + config.command, + config.tty, + config.await_main_process_attachment, + true, + ) + }; + + // The built-in default command is a login shell (`/bin/bash -l`), but + // minimal images (e.g. Alpine) don't ship bash. Only the default is + // remapped — an explicit user command is never rewritten. This runs in + // the supervisor, so it resolves against the sandbox image's filesystem. + let command = if is_default_command { + resolve_default_shell_command(command) } else { - let config = openshell_core::sandbox_env::MainProcessConfig::scratch(); - ( - config.command, - config.tty, - config.await_main_process_attachment, - ) + command }; info!(command = ?command, "Starting sandbox"); @@ -718,6 +738,28 @@ fn main() -> Result<()> { std::process::exit(exit_code); } +/// Remap the built-in default shell command to a shell that exists in the +/// sandbox image. The default program (`/bin/bash`) is absent on minimal images +/// such as Alpine; falling back to a detected login shell lets the sandbox +/// start instead of failing with an opaque `No such file or directory`. Only the +/// built-in default is remapped — explicit user commands never reach this path. +fn resolve_default_shell_command(mut command: Vec) -> Vec { + let Some(program) = command.first().cloned() else { + return command; + }; + if openshell_core::shell::is_executable(&program) { + return command; + } + let shell = openshell_core::shell::detect_login_shell(); + warn!( + missing = %program, + fallback = %shell, + "default shell not found in sandbox image; falling back to a detected shell" + ); + command[0] = shell; + command +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 52557493cd..691a87f471 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -225,11 +225,14 @@ fn apply_canonical_process_environment( user_environment: &HashMap, ) { let (session_user, session_home) = session_user_and_home(policy, workspace.home()); + // Resolve a shell present in the sandbox image (minimal images such as + // Alpine ship only `/bin/sh`, not bash). Runs in the supervisor. + let shell = openshell_core::shell::detect_login_shell(); for (key, value) in [ ("HOME", session_home.as_str()), ("USER", session_user.as_str()), - ("SHELL", "/bin/bash"), + ("SHELL", shell.as_str()), ( "TERM", if interactive { @@ -900,15 +903,23 @@ impl ProcessHandle { } } + // Name the program in the error: a bare "No such file or directory" + // here is otherwise indistinguishable from a missing working directory + // or interpreter, and is a common failure on images that lack the + // requested shell/binary (e.g. bash on Alpine). #[cfg(target_os = "linux")] let mut child = spawn_command_with_supervisor_identity_namespace(cmd) .into_diagnostic() - .wrap_err("failed to spawn sandbox entrypoint process")?; + .wrap_err_with(|| { + format!("failed to spawn sandbox entrypoint process '{program}'") + })?; #[cfg(not(target_os = "linux"))] let mut child = cmd .spawn() .into_diagnostic() - .wrap_err("failed to spawn sandbox entrypoint process")?; + .wrap_err_with(|| { + format!("failed to spawn sandbox entrypoint process '{program}'") + })?; let pid = child.id().unwrap_or(0); managed_children::register(pid); diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 357969fb52..830bc9cd55 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -1208,7 +1208,7 @@ fn apply_child_env( .env(openshell_core::sandbox_env::SANDBOX, "1") .env("HOME", session_home) .env("USER", session_user) - .env("SHELL", "/bin/bash") + .env("SHELL", openshell_core::shell::detect_login_shell()) .env("PATH", &path) .env("TERM", term); @@ -1435,18 +1435,22 @@ fn spawn_pipe_exec( resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> anyhow::Result>> { + // Resolve a shell present in the sandbox image; minimal images (e.g. Alpine) + // don't ship bash, only `/bin/sh`. Runs in the supervisor, so it inspects + // the sandbox filesystem. + let shell = openshell_core::shell::detect_login_shell(); let mut cmd = command.map_or_else( || { // No command — read from stdin. Do *not* pass `-i`; interactive // mode reads .bashrc, writes prompts to stderr, and can introduce // just enough latency for VS Code Remote-SSH's platform detection - // to time out and fall back to "windows". Plain `bash` with piped + // to time out and fall back to "windows". Plain shell with piped // stdin already reads commands line-by-line (script mode), which is // exactly what VS Code's local server expects. - Command::new("/bin/bash") + Command::new(&shell) }, |command| { - let mut c = Command::new("/bin/bash"); + let mut c = Command::new(&shell); // Login shell (-l) sources .profile/.bashrc so tool env vars // (VIRTUAL_ENV, etc.) are available. Callers that need a predictable // environment opt out via OPENSHELL_NO_LOGIN_SHELL → plain -c. From c37f287c632a1864b7dcdf8ac149f662be018088 Mon Sep 17 00:00:00 2001 From: Akram Date: Thu, 3 Sep 2026 05:55:13 +0400 Subject: [PATCH 2/3] fix(sandbox): drop $SHELL preference in shell detection $SHELL is image/user-controlled and the detected shell is later invoked with `-lc`, so an executable that is not a compatible shell (e.g. SHELL=/bin/false) would pass the executable check and then break command execution even when /bin/sh is available. Resolve only from known shell paths instead. Also add a USR_BASH constant for /usr/bin/bash rather than a string literal in SHELL_CANDIDATES. Refs #3146 Signed-off-by: Akram --- crates/openshell-core/src/shell.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/openshell-core/src/shell.rs b/crates/openshell-core/src/shell.rs index b22e2ae356..7a22856d3d 100644 --- a/crates/openshell-core/src/shell.rs +++ b/crates/openshell-core/src/shell.rs @@ -16,12 +16,16 @@ /// Preferred interactive shell when the image provides it. pub const BASH: &str = "/bin/bash"; +/// Preferred interactive shell on `usr`-merged images where `/bin` is not a +/// top-level directory. +pub const USR_BASH: &str = "/usr/bin/bash"; + /// POSIX shell. Guaranteed on virtually every image, including Alpine/`BusyBox`. /// Used as the ultimate fallback. pub const POSIX_SH: &str = "/bin/sh"; /// Shell paths tried, in preference order, by [`detect_login_shell`]. -pub const SHELL_CANDIDATES: &[&str] = &[BASH, "/usr/bin/bash", POSIX_SH]; +pub const SHELL_CANDIDATES: &[&str] = &[BASH, USR_BASH, POSIX_SH]; /// Return `true` if `path` is a regular, executable file in the current root /// filesystem. @@ -46,17 +50,17 @@ pub fn is_executable(path: &str) -> bool { /// Resolve a login shell that exists in the current root filesystem. /// -/// Resolution order: `$SHELL` (when set and executable), then -/// [`SHELL_CANDIDATES`], falling back to [`POSIX_SH`]. Because this inspects -/// the filesystem, call it from the supervisor (inside the sandbox), never on -/// the gateway. +/// Tries [`SHELL_CANDIDATES`] in order and falls back to [`POSIX_SH`]. Because +/// this inspects the filesystem, call it from the supervisor (inside the +/// sandbox), never on the gateway. +/// +/// `$SHELL` is intentionally not consulted: it is image/user-controlled, the +/// result is later invoked with `-lc`, and an executable that is not a +/// compatible shell (e.g. `SHELL=/bin/false`) would pass the executable check +/// yet break command execution. Resolving only from known shell paths avoids +/// that footgun. #[must_use] pub fn detect_login_shell() -> String { - if let Ok(shell) = std::env::var("SHELL") - && is_executable(&shell) - { - return shell; - } SHELL_CANDIDATES .iter() .find(|candidate| is_executable(candidate)) From 89f8783e4fd01b1e154300b4e586791c4d3b271e Mon Sep 17 00:00:00 2001 From: Akram Date: Thu, 3 Sep 2026 06:14:11 +0400 Subject: [PATCH 3/3] fix(sandbox): resolve the default login shell in the supervisor (empty command = default) Addresses review: interactive PTY SSH now uses the detected shell, the shell tests are portable across the Windows lane, and default-shell provenance is carried without a new spec field. An omitted command is left empty end to end and resolved in the supervisor, which is the only place that sees the sandbox image: - The CLI forwards the command as-is; the gateway persists an omitted command as empty (no baked /bin/bash -l) and requests a TTY. - MainProcessConfig carries the command empty (the transport now allows it); the supervisor resolves a login shell that exists in the sandbox image (bash when present, otherwise /bin/sh on minimal images like Alpine) and logs the resolved shell. - Interactive PTY SSH (spawn_pty_shell) uses the detected shell; a shared build_ssh_shell_command helper covers the PTY and non-PTY paths, with a deterministic sh-only regression test. - Unix-only shell tests are gated with cfg(unix). An explicit command is always run verbatim. Refs #3146 Signed-off-by: Akram --- crates/openshell-cli/src/run.rs | 10 +- crates/openshell-core/src/sandbox_env.rs | 54 +++++++++- crates/openshell-core/src/shell.rs | 7 +- crates/openshell-driver-docker/src/tests.rs | 4 +- crates/openshell-sandbox/src/main.rs | 82 ++++++-------- crates/openshell-server/src/grpc/sandbox.rs | 9 +- .../src/process.rs | 13 ++- .../openshell-supervisor-process/src/ssh.rs | 102 ++++++++++++------ docs/sandboxes/manage-sandboxes.mdx | 8 +- 9 files changed, 182 insertions(+), 107 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 4cd4e34a76..972d04f21e 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -593,11 +593,11 @@ pub async fn sandbox_create( let main_terminal = tty_override .unwrap_or_else(|| std::io::stdin().is_terminal() && std::io::stdout().is_terminal()); - let main_command = if command.is_empty() { - vec!["/bin/bash".to_string(), "-l".to_string()] - } else { - command.to_vec() - }; + // Forward the command as-is. When empty, the gateway persists it empty and + // the supervisor resolves the default login shell against the sandbox image + // (bash when present, otherwise /bin/sh on minimal images like Alpine). + // Baking a shell here would force a shell the image may not ship. + let main_command = command.to_vec(); let persist = sandbox_should_persist(keep, forward.as_ref()); let create_detaches = detach || (persist diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index 99ac55fe6e..2ce8e4b058 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -38,6 +38,9 @@ const MAIN_PROCESS_SPEC_BASE64URL_PREFIX: &str = "base64url:"; #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct MainProcessConfig { pub version: u32, + /// Canonical command. Empty means "no command supplied": the supervisor + /// resolves the default login shell against the sandbox image. A non-empty + /// command is the exact program+args and is run verbatim. pub command: Vec, pub tty: bool, #[serde(default)] @@ -47,11 +50,15 @@ pub struct MainProcessConfig { impl MainProcessConfig { pub const VERSION: u32 = 1; + /// Default config for a sandbox created without a command. The command is + /// left empty on purpose: the supervisor picks a login shell that exists in + /// the sandbox image (bash when present, otherwise `/bin/sh`). A TTY is + /// requested because the default is an interactive login shell. #[must_use] pub fn scratch() -> Self { Self { version: Self::VERSION, - command: vec!["/bin/bash".to_string(), "-l".to_string()], + command: Vec::new(), tty: true, await_main_process_attachment: false, } @@ -91,8 +98,13 @@ impl MainProcessConfig { config.version )); } - if config.command.is_empty() || config.command[0].is_empty() { - return Err(format!("{MAIN_PROCESS_SPEC} command must not be empty")); + // An empty command is valid: it means "no command supplied", and the + // supervisor resolves the default login shell. Only a present-but-blank + // program is rejected. + if !config.command.is_empty() && config.command[0].is_empty() { + return Err(format!( + "{MAIN_PROCESS_SPEC} command program must not be empty" + )); } Ok(config) } @@ -283,4 +295,40 @@ mod tests { let encoded = serde_json::to_string(&config).unwrap(); assert_eq!(MainProcessConfig::decode(&encoded).unwrap(), config); } + + #[test] + fn omitted_command_stays_empty_for_supervisor_resolution() { + // No command supplied → empty command; the supervisor resolves the + // default login shell against the sandbox image. + let empty = crate::proto::compute::v1::DriverSandboxSpec::default(); + assert!( + MainProcessConfig::from_driver_spec(Some(&empty)) + .command + .is_empty() + ); + assert!(MainProcessConfig::from_driver_spec(None).command.is_empty()); + + // An explicit command is preserved verbatim and never rewritten. + let explicit = crate::proto::compute::v1::DriverSandboxSpec { + command: vec!["/bin/bash".into(), "-l".into()], + tty: true, + ..Default::default() + }; + assert_eq!( + MainProcessConfig::from_driver_spec(Some(&explicit)).command, + vec!["/bin/bash".to_string(), "-l".to_string()] + ); + + // An empty command survives the transport round-trip. + let encoded = serde_json::to_string(&MainProcessConfig::scratch()).unwrap(); + assert!( + MainProcessConfig::decode(&encoded) + .unwrap() + .command + .is_empty() + ); + + // A present-but-blank program is still rejected. + assert!(MainProcessConfig::decode(r#"{"version":1,"command":[""],"tty":false}"#).is_err()); + } } diff --git a/crates/openshell-core/src/shell.rs b/crates/openshell-core/src/shell.rs index 7a22856d3d..09610afe1a 100644 --- a/crates/openshell-core/src/shell.rs +++ b/crates/openshell-core/src/shell.rs @@ -74,9 +74,12 @@ pub fn detect_login_shell() -> String { mod tests { use super::*; + // These assume a Unix root filesystem (`/bin/sh`, POSIX permission bits) and + // are skipped on the Windows workspace lane where openshell-core also builds. + #[cfg(unix)] #[test] fn posix_sh_exists_on_test_host() { - // /bin/sh is present on all supported CI hosts (Linux and macOS). + // /bin/sh is present on all supported Unix CI hosts (Linux and macOS). assert!(is_executable(POSIX_SH)); } @@ -85,12 +88,14 @@ mod tests { assert!(!is_executable("/nonexistent/definitely/not/here")); } + #[cfg(unix)] #[test] fn non_file_is_not_executable() { // A directory is not an executable shell. assert!(!is_executable("/")); } + #[cfg(unix)] #[test] fn detect_returns_an_executable_shell() { let shell = detect_login_shell(); diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 07fb2b0ffb..1d6727b7ac 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -1092,7 +1092,9 @@ fn build_environment_sets_docker_tls_paths() { }) .expect("main-process transport"); let main = openshell_core::sandbox_env::MainProcessConfig::decode(&encoded).unwrap(); - assert_eq!(main.command, vec!["/bin/bash", "-l"]); + // An omitted command is forwarded empty; the supervisor resolves the default + // login shell against the sandbox image at startup. + assert!(main.command.is_empty()); assert!(main.tty); } diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 9516436ea0..e7a0948718 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -661,43 +661,31 @@ fn main() -> Result<()> { // drivers otherwise provide a versioned JSON transport so argument // boundaries are never reconstructed with shell parsing. let workdir = args.workdir.clone(); - let (command, interactive, await_main_process_attachment, is_default_command) = - if !args.command.is_empty() { - (args.command, args.interactive, false, false) - } else if let Ok(json) = std::env::var(openshell_core::sandbox_env::MAIN_PROCESS_SPEC) { - let config = openshell_core::sandbox_env::MainProcessConfig::decode(&json) - .map_err(|error| miette::miette!("{error}"))?; - // The driver bakes the built-in default (`scratch`) into the spec - // when the user supplies no command. Detect that case so the - // default shell can be resolved against the sandbox image below. - let is_default = - config == openshell_core::sandbox_env::MainProcessConfig::scratch(); - ( - config.command, - config.tty, - config.await_main_process_attachment, - is_default, - ) - } else { - let config = openshell_core::sandbox_env::MainProcessConfig::scratch(); - ( - config.command, - config.tty, - config.await_main_process_attachment, - true, - ) - }; - - // The built-in default command is a login shell (`/bin/bash -l`), but - // minimal images (e.g. Alpine) don't ship bash. Only the default is - // remapped — an explicit user command is never rewritten. This runs in - // the supervisor, so it resolves against the sandbox image's filesystem. - let command = if is_default_command { - resolve_default_shell_command(command) + let (command, interactive, await_main_process_attachment) = if !args.command.is_empty() { + (args.command, args.interactive, false) + } else if let Ok(json) = std::env::var(openshell_core::sandbox_env::MAIN_PROCESS_SPEC) { + let config = openshell_core::sandbox_env::MainProcessConfig::decode(&json) + .map_err(|error| miette::miette!("{error}"))?; + ( + config.command, + config.tty, + config.await_main_process_attachment, + ) } else { - command + let config = openshell_core::sandbox_env::MainProcessConfig::scratch(); + ( + config.command, + config.tty, + config.await_main_process_attachment, + ) }; + // An omitted command (the gateway leaves the default empty rather than + // baking a shell it cannot verify) is resolved to a login shell here, in + // the supervisor, so it matches the sandbox image: bash when present, + // otherwise /bin/sh (e.g. Alpine). An explicit command is used verbatim. + let command = resolve_default_command(command); + info!(command = ?command, "Starting sandbox"); // Note: "Starting sandbox" stays as plain info!() since the OCSF context // is not yet initialized at this point (run_sandbox hasn't been called). @@ -738,26 +726,18 @@ fn main() -> Result<()> { std::process::exit(exit_code); } -/// Remap the built-in default shell command to a shell that exists in the -/// sandbox image. The default program (`/bin/bash`) is absent on minimal images -/// such as Alpine; falling back to a detected login shell lets the sandbox -/// start instead of failing with an opaque `No such file or directory`. Only the -/// built-in default is remapped — explicit user commands never reach this path. -fn resolve_default_shell_command(mut command: Vec) -> Vec { - let Some(program) = command.first().cloned() else { - return command; - }; - if openshell_core::shell::is_executable(&program) { +/// Resolve an omitted canonical command to a login shell that exists in this +/// sandbox image. Empty means "use the default": the gateway leaves an omitted +/// command empty rather than persisting a shell it cannot verify, so the +/// supervisor picks one here against the real sandbox filesystem (bash when +/// present, otherwise `/bin/sh`). An explicit command is returned unchanged. +fn resolve_default_command(command: Vec) -> Vec { + if !command.is_empty() { return command; } let shell = openshell_core::shell::detect_login_shell(); - warn!( - missing = %program, - fallback = %shell, - "default shell not found in sandbox image; falling back to a detected shell" - ); - command[0] = shell; - command + info!(shell = %shell, "no command specified; resolved default login shell"); + vec![shell, "-l".to_string()] } #[cfg(test)] diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index d4d08da251..b941502c62 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -297,11 +297,12 @@ async fn handle_create_sandbox_inner( (resolved, Some(provenance)) }; - // Every newly persisted sandbox has one explicit canonical process. This - // portable default also preserves compatibility with callers compiled - // before the main-process field was introduced. + // Leave an omitted command empty rather than persisting a concrete shell: + // the supervisor resolves the default login shell against the sandbox image + // (bash when present, otherwise /bin/sh on minimal images like Alpine), + // which the gateway cannot do since it does not see the sandbox filesystem. + // The default is an interactive login shell, so request a TTY. if spec.command.is_empty() { - spec.command = vec!["/bin/bash".to_string(), "-l".to_string()]; spec.tty = true; } diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 691a87f471..0ab1dd3187 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -910,16 +910,12 @@ impl ProcessHandle { #[cfg(target_os = "linux")] let mut child = spawn_command_with_supervisor_identity_namespace(cmd) .into_diagnostic() - .wrap_err_with(|| { - format!("failed to spawn sandbox entrypoint process '{program}'") - })?; + .wrap_err_with(|| format!("failed to spawn sandbox entrypoint process '{program}'"))?; #[cfg(not(target_os = "linux"))] let mut child = cmd .spawn() .into_diagnostic() - .wrap_err_with(|| { - format!("failed to spawn sandbox entrypoint process '{program}'") - })?; + .wrap_err_with(|| format!("failed to spawn sandbox entrypoint process '{program}'"))?; let pid = child.id().unwrap_or(0); managed_children::register(pid); @@ -2486,7 +2482,10 @@ mod tests { Some(¤t_user.dir.to_string_lossy().as_ref()) ); assert_eq!(variables.get("USER"), Some(¤t_user.name.as_str())); - assert_eq!(variables.get("SHELL"), Some(&"/bin/bash")); + // SHELL is the shell detected in the current root filesystem, not a + // hardcoded path (bash-less images resolve to /bin/sh). + let expected_shell = openshell_core::shell::detect_login_shell(); + assert_eq!(variables.get("SHELL"), Some(&expected_shell.as_str())); assert_eq!(variables.get("TERM"), Some(&"xterm-256color")); } diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 830bc9cd55..fbd6d9275b 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -1242,6 +1242,33 @@ const fn login_shell_flag(no_login_shell: bool) -> &'static str { if no_login_shell { "-c" } else { "-lc" } } +/// Build the shell command for an SSH session using a shell that exists in the +/// sandbox image (minimal images such as Alpine ship only `/bin/sh`, not bash). +/// +/// `no_command_arg` is appended only when no explicit command is given: `-i` +/// for an interactive PTY session, or `None` for the non-PTY stdin path (a +/// bare shell already reads piped stdin line-by-line). With an explicit +/// command the login-shell flag is used per `no_login_shell`. +fn build_ssh_shell_command( + shell: &str, + command: Option, + no_login_shell: bool, + no_command_arg: Option<&str>, +) -> Command { + let mut cmd = Command::new(shell); + match command { + None => { + if let Some(arg) = no_command_arg { + cmd.arg(arg); + } + } + Some(command) => { + cmd.arg(login_shell_flag(no_login_shell)).arg(command); + } + } + cmd +} + #[allow(clippy::too_many_arguments)] fn spawn_pty_shell( policy: &SandboxPolicy, @@ -1276,18 +1303,11 @@ fn spawn_pty_shell( let mut reader = master.try_clone()?; let mut writer = master.try_clone()?; - let mut cmd = command.map_or_else( - || { - let mut c = Command::new("/bin/bash"); - c.arg("-i"); - c - }, - |command| { - let mut c = Command::new("/bin/bash"); - c.arg(login_shell_flag(no_login_shell)).arg(command); - c - }, - ); + // Resolve a shell present in the sandbox image; interactive PTY sessions + // pass `-i` when no command is given. Runs in the supervisor, so it + // inspects the sandbox filesystem. + let shell = openshell_core::shell::detect_login_shell(); + let mut cmd = build_ssh_shell_command(&shell, command, no_login_shell, Some("-i")); let term = if pty.term.is_empty() { "xterm-256color" @@ -1437,27 +1457,13 @@ fn spawn_pipe_exec( ) -> anyhow::Result>> { // Resolve a shell present in the sandbox image; minimal images (e.g. Alpine) // don't ship bash, only `/bin/sh`. Runs in the supervisor, so it inspects - // the sandbox filesystem. + // the sandbox filesystem. No command → read from stdin with no `-i`: + // interactive mode reads .bashrc, writes prompts to stderr, and can add + // just enough latency for VS Code Remote-SSH's platform detection to time + // out and fall back to "windows". A plain shell with piped stdin already + // reads commands line-by-line (script mode), which is what VS Code expects. let shell = openshell_core::shell::detect_login_shell(); - let mut cmd = command.map_or_else( - || { - // No command — read from stdin. Do *not* pass `-i`; interactive - // mode reads .bashrc, writes prompts to stderr, and can introduce - // just enough latency for VS Code Remote-SSH's platform detection - // to time out and fall back to "windows". Plain shell with piped - // stdin already reads commands line-by-line (script mode), which is - // exactly what VS Code's local server expects. - Command::new(&shell) - }, - |command| { - let mut c = Command::new(&shell); - // Login shell (-l) sources .profile/.bashrc so tool env vars - // (VIRTUAL_ENV, etc.) are available. Callers that need a predictable - // environment opt out via OPENSHELL_NO_LOGIN_SHELL → plain -c. - c.arg(login_shell_flag(no_login_shell)).arg(command); - c - }, - ); + let mut cmd = build_ssh_shell_command(&shell, command, no_login_shell, None); let (session_user, session_home) = session_user_and_home(policy, workspace.home()); apply_child_env( @@ -1801,8 +1807,40 @@ fn is_loopback_host(host: &str) -> bool { )] mod tests { use super::*; + use std::ffi::OsStr; use std::process::Stdio; + /// Regression test: SSH sessions run the shell they are given, never a + /// hardcoded bash, so sh-only images (e.g. Alpine) work. Covers both the + /// interactive PTY path (`-i` when no command) and the non-PTY path. + #[test] + fn build_ssh_shell_command_uses_given_shell() { + // PTY, no command → given shell + interactive flag. + let cmd = build_ssh_shell_command("/bin/sh", None, false, Some("-i")); + assert_eq!(cmd.get_program(), OsStr::new("/bin/sh")); + assert_eq!(cmd.get_args().collect::>(), vec![OsStr::new("-i")]); + + // Non-PTY, no command → bare shell, no args (reads piped stdin). + let cmd = build_ssh_shell_command("/bin/sh", None, false, None); + assert_eq!(cmd.get_program(), OsStr::new("/bin/sh")); + assert_eq!(cmd.get_args().count(), 0); + + // Explicit command → login-shell flag + command, still on the given shell. + let cmd = build_ssh_shell_command("/bin/sh", Some("echo hi".into()), false, Some("-i")); + assert_eq!(cmd.get_program(), OsStr::new("/bin/sh")); + assert_eq!( + cmd.get_args().collect::>(), + vec![OsStr::new("-lc"), OsStr::new("echo hi")] + ); + + // OPENSHELL_NO_LOGIN_SHELL → plain -c. + let cmd = build_ssh_shell_command("/bin/sh", Some("echo hi".into()), true, None); + assert_eq!( + cmd.get_args().collect::>(), + vec![OsStr::new("-c"), OsStr::new("echo hi")] + ); + } + /// Regression test: the direct-tcpip connect path sets `TCP_NODELAY`. #[tokio::test] async fn connect_in_netns_sets_tcp_nodelay() { diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index 96538eeeaa..31c7182e5f 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -23,9 +23,11 @@ openshell sandbox create -- claude The trailing command is the sandbox's canonical main process. OpenShell starts it once, streams its output, and returns its exit status. Exit code 0 leaves a retained sandbox in `Completed`; a nonzero exit leaves it in `Error` with a -`MainProcessFailed` condition. With no trailing command, OpenShell -starts `/bin/bash -l` in a retained pseudo-terminal. Add `--detach` to create -the sandbox without attaching: +`MainProcessFailed` condition. With no trailing command, OpenShell starts a +login shell in a retained pseudo-terminal: `/bin/bash -l` when the image +provides bash, otherwise a shell detected in the image such as `/bin/sh` on +minimal bases like Alpine. Add `--detach` to create the sandbox without +attaching: ```shell openshell sandbox create --name worker --detach -- ./worker