diff --git a/codex-rs/cli/src/exec_server_command.rs b/codex-rs/cli/src/exec_server_command.rs index 505ea0fb81..de4b6643a4 100644 --- a/codex-rs/cli/src/exec_server_command.rs +++ b/codex-rs/cli/src/exec_server_command.rs @@ -43,6 +43,11 @@ pub(super) struct ExecServerCommand { )] pub(super) strict_config: bool, + /// Linux PID namespace: isolate (default) or inherit. Inherit allows signals to + /// other same-UID processes; enable only when provisioning a dedicated environment. + #[arg(long, value_name = "MODE", default_value = "isolate", global = true)] + linux_sandbox_pid_namespace: codex_sandboxing::LinuxSandboxPidNamespace, + /// Maximum number of requests to process concurrently on each connection. #[arg( long = "concurrent-requests", @@ -162,7 +167,8 @@ impl ExecServerCommand { let runtime_paths = ExecServerRuntimePaths::new( codex_self_exe, arg0_paths.codex_linux_sandbox_exe.clone(), - )?; + )? + .with_linux_sandbox_pid_namespace(self.linux_sandbox_pid_namespace); if let Some(base_url) = self.remote.take() { let environment_id = self.environment_id.take().ok_or_else(|| { anyhow::anyhow!("--environment-id is required when --remote is set") diff --git a/codex-rs/cli/src/snapshots/codex__exec_server_args_tests__exec_server_help_documents_remote_options.snap b/codex-rs/cli/src/snapshots/codex__exec_server_args_tests__exec_server_help_documents_remote_options.snap index 8ff0748607..44fb6b581e 100644 --- a/codex-rs/cli/src/snapshots/codex__exec_server_args_tests__exec_server_help_documents_remote_options.snap +++ b/codex-rs/cli/src/snapshots/codex__exec_server_args_tests__exec_server_help_documents_remote_options.snap @@ -1,5 +1,6 @@ --- source: cli/src/exec_server_args_tests.rs +assertion_line: 36 expression: help_text --- [EXPERIMENTAL] Run the standalone exec-server service @@ -28,15 +29,22 @@ Options: Error out when config.toml contains fields that are not recognized by this version of Codex + --disable + Disable a feature (repeatable). Equivalent to `-c + features.=false` + + --linux-sandbox-pid-namespace + Linux PID namespace: isolate (default) or inherit. Inherit allows + signals to other same-UID processes; enable only when provisioning a + dedicated environment + + [default: isolate] + --concurrent-requests Maximum number of requests to process concurrently on each connection [default: 1] - --disable - Disable a feature (repeatable). Equivalent to `-c - features.=false` - --listen Transport endpoint URL. Supported values: `ws://IP:PORT` (default), `stdio`, `stdio://` diff --git a/codex-rs/cli/tests/exec_server.rs b/codex-rs/cli/tests/exec_server.rs index 34d1f7e516..d16e9d95a8 100644 --- a/codex-rs/cli/tests/exec_server.rs +++ b/codex-rs/cli/tests/exec_server.rs @@ -1,3 +1,7 @@ +#[cfg(target_os = "linux")] +#[path = "exec_server/pid_namespace_tests.rs"] +mod pid_namespace_tests; + use std::collections::HashMap; #[cfg(unix)] use std::io::BufRead as _; diff --git a/codex-rs/cli/tests/exec_server/pid_namespace_tests.rs b/codex-rs/cli/tests/exec_server/pid_namespace_tests.rs new file mode 100644 index 0000000000..6129e7f0e2 --- /dev/null +++ b/codex-rs/cli/tests/exec_server/pid_namespace_tests.rs @@ -0,0 +1,154 @@ +//! End-to-end checks that only executor startup can opt into PID inheritance. + +use super::*; +use codex_exec_server::FileSystemSandboxContext; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::FileSystemSpecialPath; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use std::os::unix::fs::PermissionsExt; + +#[tokio::test] +async fn pid_inheritance_is_startup_only_for_process_and_filesystem_helpers() -> Result<()> { + let Some(bwrap) = codex_sandboxing::find_system_bwrap_in_path() else { + eprintln!("skipping PID namespace test: no system bubblewrap"); + return Ok(()); + }; + let available = tokio::process::Command::new(&bwrap) + .args(["--unshare-user", "--ro-bind", "/", "/", "--", "/bin/true"]) + .output() + .await?; + if !available.status.success() { + eprintln!( + "skipping PID namespace test: {}", + String::from_utf8_lossy(&available.stderr) + ); + return Ok(()); + } + tokio::time::timeout(Duration::from_secs(30), async { + let fixture = TempDir::new()?; + let wrapper = fixture.path().join("bwrap"); + std::os::unix::fs::symlink(bwrap, fixture.path().join("real-bwrap"))?; + std::fs::write(&wrapper, r#"#!/bin/sh +for arg in "$@"; do + [ "$arg" = "--" ] && break + if [ "$arg" = "--proc" ]; then + echo "bwrap: Can't mount proc on /newroot/proc: Operation not permitted" >&2 + exit 1 + fi +done +exec "${0%/*}/real-bwrap" "$@" +"#)?; + std::fs::set_permissions(&wrapper, std::fs::Permissions::from_mode(0o755))?; + let path = format!("{}:{}", fixture.path().display(), std::env::var("PATH")?); + // System bubblewrap discovery intentionally excludes executables under the command cwd. + let workspace = TempDir::new()?; + let file = workspace.path().join("readable"); + std::fs::write(&file, "ok")?; + let file_uri = url::Url::from_file_path(&file).unwrap(); + let cwd = url::Url::from_directory_path(workspace.path()).unwrap(); + let minimal_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { value: FileSystemSpecialPath::Minimal }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + AbsolutePathBuf::try_from(workspace.path())?.into(), FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + AbsolutePathBuf::from_absolute_path("/proc/version")?.into(), FileSystemAccessMode::Deny, + ), + ]); + let minimal_profile = PermissionProfile::from_runtime_permissions( + &minimal_policy, NetworkSandboxPolicy::Restricted, + ); + for (inherit, profile) in [ + (false, PermissionProfile::read_only()), + (true, PermissionProfile::read_only()), + (true, minimal_profile), + ] { + let minimal = profile.file_system_sandbox_policy().include_platform_defaults(); + let sandbox = FileSystemSandboxContext::from_permission_profile( + profile, cwd.as_str().parse()?, + ); + let codex_home = TempDir::new()?; + // Neither a config key nor executor/request environment variables may opt in. + std::fs::write(codex_home.path().join("config.toml"), + "linux_sandbox_pid_namespace = 'inherit'\n")?; + let mut command = tokio::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); + command.args(["exec-server", "--listen", "stdio"]) + .env("CODEX_HOME", codex_home.path()) + .env("PATH", &path) + .env("CODEX_LINUX_SANDBOX_PID_NAMESPACE", "inherit") + .stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()) + .kill_on_drop(true); + if inherit { + command.arg("--linux-sandbox-pid-namespace=inherit"); + } + let mut child = command.spawn()?; + let mut stdin = child.stdin.take().context("executor stdin")?; + let mut stdout = BufReader::new(child.stdout.take().context("executor stdout")?); + send_json_line(&mut stdin, &serde_json::json!({ + "id": 1, "method": "initialize", + "params": {"clientName": "pid-namespace-test", "resumeSessionId": null} + })).await?; + wait_for_response(&mut stdout, /*expected_id*/ 1).await?; + send_json_line(&mut stdin, &serde_json::json!({"method": "initialized", "params": {}})).await?; + send_json_line(&mut stdin, &serde_json::json!({ + "id": 2, "method": "process/start", "params": { + "processId": "pid-test", "cwd": cwd, "sandbox": sandbox, + "argv": ["/bin/bash", "-lc", r#" +set -e +IFS=' ' read -r pid rest < /proc/self/stat +test "$$" = "$pid" +test "$(cat <(printf shell-ok))" = shell-ok +if [ "$MINIMAL" = true ]; then + ! cat /proc/version +fi +"#], + "env": {"PATH": path, "MINIMAL": minimal.to_string(), "CODEX_LINUX_SANDBOX_PID_NAMESPACE": "inherit"}, + "tty": false, "pipeStdin": false, "arg0": null + } + })).await?; + // Notifications may precede the process/start response, so consume both. + let mut started = false; + let mut exit_code = None; + while !started || exit_code.is_none() { + let mut line = String::new(); + anyhow::ensure!(stdout.read_line(&mut line).await? > 0, "executor closed"); + let message: serde_json::Value = serde_json::from_str(&line)?; + if message["id"] == 2 { + anyhow::ensure!(message.get("error").is_none(), "start failed: {message}"); + started = true; + } + if message["method"] == "process/exited" { + exit_code = message["params"]["exitCode"].as_i64(); + } + } + assert_eq!(exit_code, Some(if inherit { 0 } else { 1 })); + send_json_line(&mut stdin, &serde_json::json!({ + "id": 3, "method": "fs/readFile", "params": {"path": file_uri, "sandbox": sandbox} + })).await?; + loop { + let mut line = String::new(); + anyhow::ensure!(stdout.read_line(&mut line).await? > 0, "executor closed"); + let message: serde_json::Value = serde_json::from_str(&line)?; + if message["id"] != 3 { continue; } + assert_eq!(message["result"], serde_json::json!({"dataBase64": "b2s="})); + break; + } + drop(stdin); + // Drain notifications while the executor shuts down. + let mut remaining = String::new(); + stdout.read_to_string(&mut remaining).await?; + let output = child.wait_with_output().await?; + anyhow::ensure!(output.status.success(), "executor failed: {}", String::from_utf8_lossy(&output.stderr)); + } + Ok(()) + }).await? +} diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 02d9dffeae..274874c1af 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1936,12 +1936,10 @@ async fn async_hook_finishing_while_idle_waits_for_the_next_turn( .await .context("timed out waiting for the async hook to start")?; - fs::write( - test.codex_home_path() - .join("async_user_prompt_submit_release"), - "ready", - ) - .context("release gated async hook")?; + let release_path = test + .codex_home_path() + .join("async_user_prompt_submit_release"); + fs::write(&release_path, "ready").context("release gated async hook")?; let finished_path = test .codex_home_path() @@ -1963,6 +1961,10 @@ async fn async_hook_finishing_while_idle_waits_for_the_next_turn( "an async hook result from the previous turn must not start a model turn" ); + // Keep the next prompt's hook pending so only the first hook's buffered + // output can contribute to the next model turn. + fs::remove_file(release_path).context("reset the async hook gate")?; + let next_prompt = "observe the buffered async context"; let next_turn = if automatic_continuation { TurnInputRequest::new(TurnInput::ResponseItem(responses::user_message_item( @@ -2040,6 +2042,7 @@ async fn async_hook_finishing_while_idle_waits_for_the_next_turn( "buffered async hook context should precede the next user prompt" ); + test.codex.shutdown_and_wait().await?; Ok(()) } diff --git a/codex-rs/core/tests/suite/mcp_optional_startup_grace.rs b/codex-rs/core/tests/suite/mcp_optional_startup_grace.rs index f5ef343ae4..0dbc563edd 100644 --- a/codex-rs/core/tests/suite/mcp_optional_startup_grace.rs +++ b/codex-rs/core/tests/suite/mcp_optional_startup_grace.rs @@ -226,8 +226,10 @@ async fn running_thread_uses_refreshed_optional_mcp_startup_grace( .await .context("optional MCP initialization should begin before the initial turn")?; + // Allow for turn setup on remote workers while still finishing before the + // five-second MCP startup timeout. The server remains gated throughout. tokio::time::timeout( - Duration::from_millis(500), + Duration::from_secs(2), fixture.submit_turn("show initial optional MCP tools"), ) .await diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index dbf4b1d152..73905a668b 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -150,7 +150,8 @@ impl FileSystemSandboxRunner { sandbox_context: &FileSystemSandboxContext, ) -> Result { let helper = &self.runtime_paths.codex_self_exe; - let sandbox_manager = SandboxManager::for_file_system_helpers(); + let sandbox_manager = SandboxManager::for_file_system_helpers() + .with_linux_sandbox_pid_namespace(self.runtime_paths.linux_sandbox_pid_namespace); #[cfg(target_os = "macos")] let sandbox_manager = sandbox_manager.with_allowed_symlinked_codex_home( self.runtime_paths.allowed_symlinked_codex_home.clone(), diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index a1a43323ba..567fb37081 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -211,7 +211,8 @@ pub(crate) async fn prepare_exec_request_with_telemetry( &file_system_policy, network_policy, ); - let sandbox_manager = SandboxManager::new(); + let sandbox_manager = SandboxManager::new() + .with_linux_sandbox_pid_namespace(runtime_paths.linux_sandbox_pid_namespace); #[cfg(target_os = "macos")] let sandbox_manager = sandbox_manager .with_allowed_symlinked_codex_home(runtime_paths.allowed_symlinked_codex_home.clone()); diff --git a/codex-rs/exec-server/src/runtime_paths.rs b/codex-rs/exec-server/src/runtime_paths.rs index 0947ed8772..46b3f524c9 100644 --- a/codex-rs/exec-server/src/runtime_paths.rs +++ b/codex-rs/exec-server/src/runtime_paths.rs @@ -1,5 +1,6 @@ use std::path::PathBuf; +use codex_sandboxing::LinuxSandboxPidNamespace; use codex_utils_absolute_path::AbsolutePathBuf; /// Paths and sandbox settings initialized when creating an executor. @@ -10,6 +11,8 @@ pub struct ExecServerRuntimePaths { /// Path to the Linux sandbox helper alias used when the platform sandbox /// needs to re-enter Codex by argv0. pub codex_linux_sandbox_exe: Option, + /// Trusted startup policy; requests and repository config cannot change PID isolation. + pub linux_sandbox_pid_namespace: LinuxSandboxPidNamespace, /// User-config opt-out of writable-root symlink checks beneath this host's home. #[cfg(target_os = "macos")] pub allowed_symlinked_codex_home: Option, @@ -34,6 +37,7 @@ impl ExecServerRuntimePaths { codex_linux_sandbox_exe: Option, ) -> std::io::Result { Ok(Self { + linux_sandbox_pid_namespace: LinuxSandboxPidNamespace::default(), codex_self_exe: absolute_path(codex_self_exe)?, codex_linux_sandbox_exe: codex_linux_sandbox_exe.map(absolute_path).transpose()?, #[cfg(target_os = "macos")] @@ -41,6 +45,12 @@ impl ExecServerRuntimePaths { }) } + /// Applies the PID namespace policy chosen by trusted executor provisioning. + pub fn with_linux_sandbox_pid_namespace(mut self, mode: LinuxSandboxPidNamespace) -> Self { + self.linux_sandbox_pid_namespace = mode; + self + } + /// Applies the symlink opt-in resolved by the execution host's config loader. #[cfg(target_os = "macos")] pub fn with_allowed_symlinked_codex_home( diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index e7904d3ed3..e1049361db 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -79,7 +79,7 @@ commands that would enter the bubblewrap path. writable roots are blocked by mounting `/dev/null` on the symlink or first missing component. - When bubblewrap is active, the helper explicitly isolates the user namespace via - `--unshare-user` and the PID namespace via `--unshare-pid`. + `--unshare-user`. By default it also creates a PID namespace via `--unshare-pid`. - When bubblewrap is active and network is restricted without proxy routing, the helper also isolates the network namespace via `--unshare-net`. - In managed proxy mode, the helper uses `--unshare-net` plus an internal @@ -87,8 +87,25 @@ commands that would enter the bubblewrap path. endpoints. - In managed proxy mode, after the bridge is live, seccomp blocks new AF_UNIX/socketpair creation for the user command. -- When bubblewrap is active, it mounts a fresh `/proc` via `--proc /proc` by default, but - you can skip this in restrictive container environments with `--no-proc`. +- When bubblewrap is active, it mounts a fresh `/proc` via `--proc /proc` by default. + If that mount is denied, it retains the inherited `/proc` and still creates a + PID namespace, preserving the existing fallback. `--no-proc` also retains the + inherited `/proc` without disabling PID isolation. In these cases, process IDs + inside the sandbox can differ from those exposed by `/proc`. Default invocations + send no new helper flags and remain compatible with older helpers. +- Trusted provisioning of a dedicated environment can start + `codex exec-server --linux-sandbox-pid-namespace=inherit`. This startup-only + setting applies to both process and filesystem helpers; repository config and + command environment variables cannot enable it. The helper receives the new + `--inherit-pid-namespace` option, which requires an updated helper; deploy the + helper and startup flag together. + Inheritance reuses the caller's PID namespace and `/proc` together, omitting + `--unshare-pid` and `--as-pid-1`. This preserves process lookups but allows + sandboxed commands to signal other same-UID processes, including the executor. + With `:minimal`, the existing `/proc` is bound read-only, preserving its + container masks; explicit filesystem denials are applied afterward. + Filesystem, user, IPC, network, seccomp, and existing container `/proc` masks + remain in force. The default `isolate` mode retains PID isolation. **Notes** - The CLI surface is `codex sandbox`; the host OS selects the sandbox backend. diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index bcc7dcbaa3..7added9ba6 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -65,11 +65,10 @@ pub(crate) const WSLG_DISTRO_ROOT: &str = "/mnt/wslg/distro"; /// Options that control how bubblewrap is invoked. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct BwrapOptions { - /// Whether to mount a fresh `/proc` inside the sandbox. - /// - /// This is the secure default, but some restrictive container environments - /// deny `--proc /proc`. + /// Whether to mount a fresh `/proc`; disabled by `--no-proc` or mount fallback. pub mount_proc: bool, + /// Explicitly reuse the caller's PID namespace instead of creating one. + pub inherit_pid_namespace: bool, /// How networking should be configured inside the bubblewrap sandbox. pub network_mode: BwrapNetworkMode, /// Hide the WSL Windows interop socket from commands with restricted filesystem access. @@ -88,6 +87,7 @@ impl Default for BwrapOptions { fn default() -> Self { Self { mount_proc: true, + inherit_pid_namespace: false, network_mode: BwrapNetworkMode::FullAccess, mask_wsl_interop: false, mask_wslg_distro: false, @@ -295,9 +295,11 @@ fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOption // Always enter a fresh user namespace so root inside a container does // not need ambient CAP_SYS_ADMIN to create the remaining namespaces. "--unshare-user".to_string(), - "--unshare-pid".to_string(), "--unshare-ipc".to_string(), ]; + if !options.inherit_pid_namespace { + args.push("--unshare-pid".to_string()); + } if options.network_mode.should_unshare_network() { args.push("--unshare-net".to_string()); } @@ -371,7 +373,9 @@ fn create_bwrap_flags( // This also blocks host procfs root/cwd/fd links through ptrace permission // checks, including when a container requires retaining the host procfs. args.push("--unshare-user".to_string()); - args.push("--unshare-pid".to_string()); + if !options.inherit_pid_namespace { + args.push("--unshare-pid".to_string()); + } args.push("--unshare-ipc".to_string()); if options.network_mode.should_unshare_network() { args.push("--unshare-net".to_string()); @@ -538,6 +542,11 @@ fn create_filesystem_args( .map(|path| PathBuf::from(*path)) .filter(|path| path.exists()), ); + if options.inherit_pid_namespace { + // Reuse the caller's procfs, including its submount masks. Bind it + // with the other read roots so explicit deny rules still win. + readable_roots.insert(PathBuf::from("/proc")); + } } // A restricted policy can still explicitly request `/`, which is @@ -1483,6 +1492,7 @@ mod wslg_tests; mod tests { use super::*; + use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::FileSystemAccessMode; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSandboxEntry; @@ -1497,6 +1507,45 @@ mod tests { assert_eq!(BwrapOptions::default().glob_scan_max_depth, None); } + #[test] + fn pid_inheritance_and_legacy_proc_modes_read_only() { + assert_pid_namespace_args(&PermissionProfile::read_only().file_system_sandbox_policy()); + } + + #[test] + fn pid_inheritance_and_legacy_proc_modes_full_filesystem() { + assert_pid_namespace_args(&FileSystemSandboxPolicy::unrestricted()); + } + + fn assert_pid_namespace_args(policy: &FileSystemSandboxPolicy) { + for (mount_proc, inherit_pid_namespace) in [(true, false), (false, false), (false, true)] { + let args = create_bwrap_command_args( + vec!["/bin/true".to_string()], + policy, + Path::new("/"), + Path::new("/"), + BwrapOptions { + mount_proc, + inherit_pid_namespace, + network_mode: BwrapNetworkMode::Isolated, + ..Default::default() + }, + ) + .expect("create bwrap args") + .args; + + assert_eq!( + args.iter().any(|arg| arg == "--unshare-pid"), + !inherit_pid_namespace + ); + assert_eq!(args.iter().any(|arg| arg == "--proc"), mount_proc); + assert!(args.iter().any(|arg| arg == "--unshare-user")); + assert!(args.iter().any(|arg| arg == "--unshare-ipc")); + assert!(args.iter().any(|arg| arg == "--unshare-net")); + assert!(args.windows(2).any(|args| args == ["--cap-drop", "ALL"])); + } + } + fn unreadable_glob_entry(pattern: String) -> FileSystemSandboxEntry { FileSystemSandboxEntry { path: FileSystemPath::GlobPattern { pattern }, @@ -1560,8 +1609,8 @@ mod tests { "/dev/shm".to_string(), "/dev/shm".to_string(), "--unshare-user".to_string(), - "--unshare-pid".to_string(), "--unshare-ipc".to_string(), + "--unshare-pid".to_string(), "--unshare-net".to_string(), "--proc".to_string(), "/proc".to_string(), diff --git a/codex-rs/linux-sandbox/src/launcher.rs b/codex-rs/linux-sandbox/src/launcher.rs index d4e9b521bf..1bcbb6bf31 100644 --- a/codex-rs/linux-sandbox/src/launcher.rs +++ b/codex-rs/linux-sandbox/src/launcher.rs @@ -36,7 +36,13 @@ struct SystemBwrapCapabilities { } pub(crate) fn exec_bwrap(mut argv: Vec, preserved_files: Vec) -> ! { - argv.insert(1, "--as-pid-1".to_string()); + if argv + .iter() + .take_while(|arg| arg.as_str() != "--") + .any(|arg| arg == "--unshare-pid") + { + argv.insert(1, "--as-pid-1".to_string()); + } match preferred_bwrap_launcher() { BubblewrapLauncher::System(launcher) => { diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 480de987c5..8ef516589c 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -147,6 +147,11 @@ pub struct LandlockCommand { #[arg(long = "no-proc", default_value_t = false)] pub no_proc: bool, + /// Reuse the caller's PID namespace and `/proc` together. + /// Only trusted executor startup may select this mode. + #[arg(long = "inherit-pid-namespace", hide = true)] + pub inherit_pid_namespace: bool, + /// Full command args to run under the Linux sandbox helper. #[arg(trailing_var_arg = true)] pub command: Vec, @@ -170,6 +175,7 @@ pub fn run_main() -> ! { proxy_route_spec, verify_fd_mounts, no_proc, + inherit_pid_namespace, command, } = LandlockCommand::parse(); let allow_network_for_proxy = managed_network.is_some(); @@ -298,7 +304,8 @@ pub fn run_main() -> ! { (None, Vec::new()) }; let options = BwrapOptions { - mount_proc: !no_proc, + mount_proc: !no_proc && !inherit_pid_namespace, + inherit_pid_namespace, network_mode: bwrap_network_mode(network_sandbox_policy, allow_network_for_proxy), mask_wsl_interop: !file_system_sandbox_policy.has_full_disk_write_access() && Path::new(WSL_INTEROP_DIR).is_dir(), diff --git a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs index 6aca07aef9..90580c64b6 100644 --- a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs +++ b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs @@ -308,6 +308,147 @@ async fn namespace_reaper_collects_orphaned_descendants() { assert_eq!(output.stdout, b"orphan reaped\n"); } +#[tokio::test] +async fn proc_mount_denial_preserves_legacy_fallback_and_explicit_pid_inheritance() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bubblewrap is unavailable"); + return; + } + + let Some(system_bwrap) = codex_sandboxing::find_system_bwrap_in_path() else { + eprintln!("skipping PID namespace test: no system bubblewrap is available"); + return; + }; + let tempdir = tempfile::tempdir().expect("create PID namespace fixture"); + let wrapper = tempdir.path().join("bwrap"); + std::fs::write( + &wrapper, + r#"#!/bin/sh +for arg in "$@"; do + [ "$arg" = "--" ] && break + if [ "$arg" = "--proc" ]; then + printf '%s\n' "bwrap: Can't mount proc on /newroot/proc: Operation not permitted" >&2 + exit 1 + fi +done +exec "$CODEX_TEST_REAL_BWRAP" "$@" +"#, + ) + .expect("write proc-denying bubblewrap wrapper"); + std::fs::set_permissions(&wrapper, std::fs::Permissions::from_mode(0o755)) + .expect("make bubblewrap wrapper executable"); + let protected_file = tempdir.path().join("protected"); + std::fs::write(&protected_file, "original").expect("write protected file"); + let pid_namespace = std::fs::read_link("/proc/self/ns/pid").expect("read caller PID namespace"); + let probe = r#" +set -eu +printf 'command started\n' +IFS=' ' read -r proc_pid rest < /proc/self/stat +test "$$" = "$proc_pid" +test "$(readlink /proc/self/ns/pid)" = "$CODEX_TEST_PID_NAMESPACE" +sleep 30 & +child=$! +trap 'kill "$child"; wait "$child" || :' EXIT +IFS=' ' read -r proc_pid rest < "/proc/$child/stat" +test "$child" = "$proc_pid" +if (printf changed > "$CODEX_TEST_PROTECTED_FILE") 2>/dev/null; then exit 1; fi +test "$(readlink /proc/self/ns/user)" != "$CODEX_TEST_USER_NAMESPACE" +test "$(readlink /proc/self/ns/ipc)" != "$CODEX_TEST_IPC_NAMESPACE" +test "$(readlink /proc/self/ns/net)" != "$CODEX_TEST_NET_NAMESPACE" +while read -r key value rest; do + case "$key" in + CapEff:) test "$value" = 0000000000000000 ;; + NoNewPrivs:) test "$value" = 1 ;; + Seccomp:) test "$value" = 2 ;; + esac +done < /proc/self/status +printf 'consistent PIDs; restrictions preserved\n' +"#; + + let legacy_probe = r#" +set -eu +printf 'command started\n' +IFS=' ' read -r proc_pid rest < /proc/self/stat +test "$$" != "$proc_pid" +printf 'legacy proc fallback\n' +"#; + + // Both modes still run when proc mounts are denied. Only explicit inheritance + // makes process IDs match the inherited proc view. + for inherit_pid_namespace in [false, true] { + let mut env = create_env_from_core_vars(); + strip_proxy_env(&mut env); + let original_path = env.get("PATH").cloned().unwrap_or_default(); + env.insert( + "PATH".to_string(), + format!("{}:{original_path}", tempdir.path().display()), + ); + env.insert( + "CODEX_TEST_REAL_BWRAP".to_string(), + system_bwrap.display().to_string(), + ); + env.insert( + "CODEX_TEST_PID_NAMESPACE".to_string(), + pid_namespace.display().to_string(), + ); + env.insert( + "CODEX_TEST_PROTECTED_FILE".to_string(), + protected_file.display().to_string(), + ); + let mut command = Command::new(env!("CARGO_BIN_EXE_codex-linux-sandbox")); + command + .arg("--sandbox-policy-cwd") + .arg(std::env::current_dir().expect("current directory should exist")) + .arg("--permission-profile") + .arg(serde_json::to_string(&PermissionProfile::read_only()).unwrap()); + for namespace in ["user", "ipc", "net"] { + env.insert( + format!("CODEX_TEST_{}_NAMESPACE", namespace.to_uppercase()), + std::fs::read_link(format!("/proc/self/ns/{namespace}")) + .unwrap() + .display() + .to_string(), + ); + } + if inherit_pid_namespace { + command.arg("--inherit-pid-namespace"); + } + let probe = if inherit_pid_namespace { + probe + } else { + legacy_probe + }; + command + .args(["--", "/bin/sh", "-c", probe]) + .env_clear() + .envs(env) + .kill_on_drop(true); + let output = + tokio::time::timeout(Duration::from_millis(NETWORK_TIMEOUT_MS), command.output()) + .await + .expect("proc probe should not time out") + .expect("proc probe should execute"); + assert!( + output.status.success(), + "stdout={}; stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + assert_eq!( + output.stdout, + if inherit_pid_namespace { + b"command started\nconsistent PIDs; restrictions preserved\n".as_slice() + } else { + b"command started\nlegacy proc fallback\n".as_slice() + } + ); + assert_eq!( + std::fs::read_to_string(&protected_file).unwrap(), + "original" + ); + } +} + #[tokio::test] async fn unsupported_system_bwrap_falls_back_to_bundled_bwrap() { if option_env!("CODEX_BWRAP_SHA256") diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index 3a1c883ce4..9e54454e3d 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -2,6 +2,7 @@ mod bwrap; mod denial; pub mod landlock; +mod linux_pid_namespace; mod manager; pub mod policy_transforms; #[cfg(target_os = "macos")] @@ -23,6 +24,7 @@ pub use codex_mxc_sandbox::run_main as run_windows_mxc_main; pub use codex_windows_sandbox::WindowsSandboxProxySettingsMode; pub use denial::is_likely_executor_managed_sandbox_denied; pub use denial::is_likely_sandbox_denied; +pub use linux_pid_namespace::LinuxSandboxPidNamespace; pub use manager::SandboxCommand; pub use manager::SandboxDirectSpawnTransformRequest; pub use manager::SandboxExecRequest; diff --git a/codex-rs/sandboxing/src/linux_pid_namespace.rs b/codex-rs/sandboxing/src/linux_pid_namespace.rs new file mode 100644 index 0000000000..b701131181 --- /dev/null +++ b/codex-rs/sandboxing/src/linux_pid_namespace.rs @@ -0,0 +1,23 @@ +//! Startup-owned Linux PID isolation policy, independent of request permissions. + +/// Selects the PID namespace policy for Linux bubblewrap sandboxes. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub enum LinuxSandboxPidNamespace { + /// Create a new PID namespace, retaining the legacy `/proc` mount fallback. + #[default] + Isolate, + /// Reuse the caller's PID namespace and `/proc`, allowing signals to other same-UID processes. + Inherit, +} + +impl std::str::FromStr for LinuxSandboxPidNamespace { + type Err = String; + + fn from_str(value: &str) -> Result { + match value { + "isolate" => Ok(Self::Isolate), + "inherit" => Ok(Self::Inherit), + _ => Err("expected 'isolate' or 'inherit'".to_string()), + } + } +} diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 783cef5e04..2bdcecfc52 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -1,3 +1,4 @@ +use crate::LinuxSandboxPidNamespace; #[cfg(target_os = "linux")] use crate::bwrap::WSL1_BWRAP_WARNING; #[cfg(target_os = "linux")] @@ -266,6 +267,7 @@ impl std::error::Error for SandboxTransformError { #[derive(Clone, Default)] pub struct SandboxManager { + linux_sandbox_pid_namespace: LinuxSandboxPidNamespace, #[cfg(target_os = "macos")] seatbelt_profile: MacosSeatbeltProfile, #[cfg(target_os = "macos")] @@ -280,6 +282,7 @@ impl SandboxManager { /// Creates a manager that applies the narrower runtime profile required by filesystem helpers. pub fn for_file_system_helpers() -> Self { Self { + linux_sandbox_pid_namespace: LinuxSandboxPidNamespace::default(), #[cfg(target_os = "macos")] seatbelt_profile: MacosSeatbeltProfile::FileSystemHelper, #[cfg(target_os = "macos")] @@ -287,6 +290,12 @@ impl SandboxManager { } } + /// Applies a trusted executor startup policy, never a command or repository setting. + pub fn with_linux_sandbox_pid_namespace(mut self, mode: LinuxSandboxPidNamespace) -> Self { + self.linux_sandbox_pid_namespace = mode; + self + } + /// Allows otherwise-authorized writable roots beneath the opted-in user home /// to follow symlinks, including targets outside that home. #[cfg(target_os = "macos")] @@ -500,6 +509,11 @@ impl SandboxManager { use_legacy_landlock, managed_network.as_ref(), ); + // Keep default invocations compatible with older helpers. Only the + // startup opt-in requires a helper that understands PID inheritance. + if self.linux_sandbox_pid_namespace == LinuxSandboxPidNamespace::Inherit { + args.insert(0, "--inherit-pid-namespace".to_string()); + } let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(os_string_to_command_component(exe.as_os_str().to_owned())); full_command.append(&mut args); diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 3afc65beea..3daac3d45e 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -380,8 +380,9 @@ fn managed_mitm_ca_bundle_becomes_readable_for_restricted_sandbox() { #[cfg(target_os = "linux")] fn transform_linux_seccomp_request( codex_linux_sandbox_exe: &std::path::Path, + mode: crate::LinuxSandboxPidNamespace, ) -> super::SandboxExecRequest { - let manager = SandboxManager::new(); + let manager = SandboxManager::new().with_linux_sandbox_pid_namespace(mode); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); let cwd_uri = PathUri::from_abs_path(&cwd); let permissions = PermissionProfile::Disabled; @@ -483,7 +484,10 @@ fn wsl1_allows_non_bubblewrap_linux_paths() { #[test] fn transform_linux_seccomp_preserves_helper_path_in_arg0_when_available() { let codex_linux_sandbox_exe = std::path::PathBuf::from("/tmp/codex-linux-sandbox"); - let exec_request = transform_linux_seccomp_request(&codex_linux_sandbox_exe); + let exec_request = transform_linux_seccomp_request( + &codex_linux_sandbox_exe, + crate::LinuxSandboxPidNamespace::default(), + ); assert_eq!( exec_request.arg0, @@ -495,9 +499,20 @@ fn transform_linux_seccomp_preserves_helper_path_in_arg0_when_available() { #[test] fn transform_linux_seccomp_uses_helper_alias_when_launcher_is_not_helper_path() { let codex_linux_sandbox_exe = std::path::PathBuf::from("/tmp/codex"); - let exec_request = transform_linux_seccomp_request(&codex_linux_sandbox_exe); - - assert_eq!(exec_request.arg0, Some("codex-linux-sandbox".to_string())); + for (mode, first_helper_arg) in [ + ( + crate::LinuxSandboxPidNamespace::Isolate, + "--sandbox-policy-cwd", + ), + ( + crate::LinuxSandboxPidNamespace::Inherit, + "--inherit-pid-namespace", + ), + ] { + let exec_request = transform_linux_seccomp_request(&codex_linux_sandbox_exe, mode); + assert_eq!(exec_request.arg0, Some("codex-linux-sandbox".to_string())); + assert_eq!(exec_request.command[1], first_helper_arg); + } } #[cfg(unix)]