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/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/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 new file mode 100644 index 0000000000..09610afe1a --- /dev/null +++ b/crates/openshell-core/src/shell.rs @@ -0,0 +1,107 @@ +// 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"; + +/// 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_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. +/// +/// 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 { + 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::*; + + // 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 Unix CI hosts (Linux and macOS). + assert!(is_executable(POSIX_SH)); + } + + #[test] + fn missing_path_is_not_executable() { + 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(); + assert!( + is_executable(&shell), + "detected shell {shell} is not executable" + ); + } +} 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 7108378654..e7a0948718 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, @@ -678,6 +680,12 @@ fn main() -> Result<()> { ) }; + // 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). @@ -718,6 +726,20 @@ fn main() -> Result<()> { std::process::exit(exit_code); } +/// 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(); + info!(shell = %shell, "no command specified; resolved default login shell"); + vec![shell, "-l".to_string()] +} + #[cfg(test)] mod tests { use super::*; 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 52557493cd..0ab1dd3187 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,19 @@ 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); @@ -2475,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 357969fb52..fbd6d9275b 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); @@ -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" @@ -1435,25 +1455,15 @@ fn spawn_pipe_exec( resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> anyhow::Result>> { - 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 - // stdin already reads commands line-by-line (script mode), which is - // exactly what VS Code's local server expects. - Command::new("/bin/bash") - }, - |command| { - let mut c = Command::new("/bin/bash"); - // 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 - }, - ); + // 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. 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 = 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( @@ -1797,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