From c8dc8e5fd5fdfe9c249bb98d691372d87e4b3cff Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 10 Jul 2026 16:46:38 +0000 Subject: [PATCH] Propagate workspace roots to exec-server sandboxes (#32214) ## What changed - Pass configured workspace roots from core to the exec server so filesystem and process sandbox permissions are materialized against the intended roots. - Preserve an explicitly empty workspace-root list instead of treating the sandbox working directory as an implicit root. - Initialize filesystem sandbox contexts with their working directory as the default workspace root. ## Testing - Add end-to-end coverage for patch and command writes inside and outside workspace roots. - Verify remote filesystem and process sandboxes do not grant access through an empty workspace-root list. GitOrigin-RevId: 6684c8f7de50970b45a29e2a6323df954c96f9f6 --- codex-rs/core/src/tools/sandboxing.rs | 6 +- codex-rs/core/src/tools/sandboxing_tests.rs | 2 +- codex-rs/core/tests/suite/mod.rs | 1 + codex-rs/core/tests/suite/workspace_roots.rs | 242 ++++++++++++++++++ codex-rs/exec-server/src/fs_sandbox.rs | 6 +- codex-rs/exec-server/src/process_sandbox.rs | 6 +- codex-rs/exec-server/tests/exec_process.rs | 66 ++++- .../exec-server/tests/file_system_unix.rs | 29 +++ codex-rs/file-system/src/lib.rs | 3 +- 9 files changed, 340 insertions(+), 21 deletions(-) create mode 100644 codex-rs/core/tests/suite/workspace_roots.rs diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 571c5ce6dfc2..a23bd5a00089 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -511,7 +511,11 @@ impl<'a> SandboxAttempt<'a> { exec_request.exec_server_sandbox = Some(FileSystemSandboxContext { permissions: exec_server_permissions.into(), cwd: Some(exec_request.windows_sandbox_policy_cwd.clone()), - workspace_roots: Vec::new(), + workspace_roots: self + .workspace_roots + .iter() + .map(PathUri::from_abs_path) + .collect(), windows_sandbox_level: self.windows_sandbox_level, windows_sandbox_private_desktop: self.windows_sandbox_private_desktop, use_legacy_landlock: self.use_legacy_landlock, diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index e52186f7721a..ee2530a3863d 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -264,7 +264,7 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { Some(codex_exec_server::FileSystemSandboxContext { permissions: exec_server_permissions.clone().into(), cwd: Some(cwd_uri.clone()), - workspace_roots: Vec::new(), + workspace_roots: vec![cwd_uri.clone()], windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, use_legacy_landlock: false, diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index f1fae54c3bdc..a916bf20f417 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -142,3 +142,4 @@ mod websocket_fallback; mod window_headers; #[cfg(target_os = "windows")] mod windows_sandbox; +mod workspace_roots; diff --git a/codex-rs/core/tests/suite/workspace_roots.rs b/codex-rs/core/tests/suite/workspace_roots.rs new file mode 100644 index 000000000000..bd031127b788 --- /dev/null +++ b/codex-rs/core/tests/suite/workspace_roots.rs @@ -0,0 +1,242 @@ +use anyhow::Context; +use anyhow::Result; +use codex_exec_server::RemoveOptions; +use codex_features::Feature; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_utils_path_uri::PathUri; +use core_test_support::TestTargetOs; +use core_test_support::responses::ResponseMock; +use core_test_support::responses::ev_apply_patch_custom_tool_call; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_target_windows; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::test_codex; +use core_test_support::test_target_os; +use serde_json::json; +use wiremock::MockServer; + +const PATCH_CALL_ID: &str = "workspace-root-patch"; +const COMMAND_CALL_ID: &str = "workspace-root-command"; + +fn workspace_roots_profile() -> PermissionProfile { + PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ) +} + +async fn workspace_roots_test(server: &MockServer) -> Result { + let mut builder = test_codex().with_config(|config| { + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config.workspace_roots = vec![config.cwd.clone()]; + }); + builder.build_with_auto_env(server).await +} + +fn outside_workspace_path(test: &TestCodex, file_name: &str) -> Result { + let file_name = format!("codex-workspace-roots-{}-{file_name}", std::process::id()); + PathUri::from_abs_path(&test.config.cwd) + .parent() + .context("test workspace should have a parent")? + .join(&file_name) + .map_err(Into::into) +} + +fn command_arguments(path: &str, contents: &str) -> Result { + let (shell, command) = match test_target_os() { + TestTargetOs::Linux | TestTargetOs::MacOs => { + ("bash", format!("printf %s '{contents}' > '{path}'")) + } + TestTargetOs::Windows => ( + "powershell", + format!("Set-Content -NoNewline -Path '{path}' -Value '{contents}'"), + ), + }; + Ok(serde_json::to_string(&json!({ + "cmd": command, + "shell": shell, + "login": false, + }))?) +} + +async fn mount_patch_and_command_calls( + server: &MockServer, + patch: &str, + command_path: &str, + command_contents: &str, +) -> Result { + let command_arguments = command_arguments(command_path, command_contents)?; + Ok(mount_sse_sequence( + server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(PATCH_CALL_ID, patch), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_function_call(COMMAND_CALL_ID, "exec_command", &command_arguments), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_response_created("resp-3"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-3"), + ]), + ], + ) + .await) +} + +async fn submit_workspace_turn(test: &TestCodex, prompt: &str) -> Result<()> { + test.submit_turn_with_permission_profile(prompt, workspace_roots_profile()) + .await +} + +async fn read_file(test: &TestCodex, path: &PathUri) -> Result { + Ok(String::from_utf8( + test.fs().read_file(path, /*sandbox*/ None).await?, + )?) +} + +async fn remove_files(test: &TestCodex, paths: &[&PathUri]) -> Result<()> { + for path in paths { + test.fs() + .remove( + path, + RemoveOptions { + recursive: false, + force: true, + }, + /*sandbox*/ None, + ) + .await?; + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn workspace_roots_allow_file_and_command_writes() -> Result<()> { + const PATCH_CONTENTS: &str = "workspace root patch access"; + const COMMAND_CONTENTS: &str = "workspace root command access"; + + skip_if_target_windows!( + Ok(()), + "sandboxed process launch is not supported by the exec-server Windows backend" + ); + + let server = start_mock_server().await; + let test = workspace_roots_test(&server).await?; + let cwd = PathUri::from_abs_path(&test.config.cwd); + let patch_path = cwd.join("workspace-root-patch.txt")?; + let command_path = cwd.join("workspace-root-command.txt")?; + let patch = format!( + "*** Begin Patch\n*** Add File: workspace-root-patch.txt\n+{PATCH_CONTENTS}\n*** End Patch\n" + ); + + let response_mock = mount_patch_and_command_calls( + &server, + &patch, + "workspace-root-command.txt", + COMMAND_CONTENTS, + ) + .await?; + submit_workspace_turn(&test, "write files inside the workspace roots").await?; + + let request = response_mock + .last_request() + .context("model should receive both workspace-root tool results")?; + let (_, patch_success) = request + .custom_tool_call_output_content_and_success(PATCH_CALL_ID) + .context("patch result should be present")?; + assert_ne!(patch_success, Some(false)); + + let (_, command_success) = request + .function_call_output_content_and_success(COMMAND_CALL_ID) + .context("command result should be present")?; + assert_ne!(command_success, Some(false)); + assert_eq!( + read_file(&test, &patch_path).await?, + format!("{PATCH_CONTENTS}\n") + ); + assert_eq!(read_file(&test, &command_path).await?, COMMAND_CONTENTS); + + remove_files(&test, &[&patch_path, &command_path]).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn workspace_roots_deny_file_and_command_writes_outside_roots() -> Result<()> { + const PATCH_CONTENTS: &str = "outside workspace root patch"; + const COMMAND_CONTENTS: &str = "outside workspace root command"; + + skip_if_target_windows!( + Ok(()), + "sandboxed process launch is not supported by the exec-server Windows backend" + ); + + let server = start_mock_server().await; + let test = workspace_roots_test(&server).await?; + let patch_path = outside_workspace_path(&test, "outside-patch.txt")?; + let command_path = outside_workspace_path(&test, "outside-command.txt")?; + let patch_path_display = patch_path.inferred_native_path_string(); + let command_path_display = command_path.inferred_native_path_string(); + let patch = format!( + "*** Begin Patch\n*** Add File: {patch_path_display}\n+{PATCH_CONTENTS}\n*** End Patch\n" + ); + + let response_mock = + mount_patch_and_command_calls(&server, &patch, &command_path_display, COMMAND_CONTENTS) + .await?; + submit_workspace_turn(&test, "try to write files outside the workspace roots").await?; + + let request = response_mock + .last_request() + .context("model should receive both denied tool results")?; + let (patch_output, patch_success) = request + .custom_tool_call_output_content_and_success(PATCH_CALL_ID) + .context("denied patch result should be present")?; + assert_ne!(patch_success, Some(true)); + assert!( + patch_output + .as_deref() + .is_some_and(|output| output.contains("outside of the project")), + "patch should be denied outside the workspace roots, got {patch_output:?}" + ); + + let (command_output, _) = request + .function_call_output_content_and_success(COMMAND_CALL_ID) + .context("denied command result should be present")?; + let command_output = command_output.context("denied command output should be present")?; + assert!( + command_output.contains(&command_path_display), + "denied command output should identify {command_path_display}, got {command_output:?}" + ); + assert!( + test.fs() + .read_file(&patch_path, /*sandbox*/ None) + .await + .is_err() + ); + assert!( + test.fs() + .read_file(&command_path, /*sandbox*/ None) + .await + .is_err() + ); + + Ok(()) +} diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 9dfbeba976ff..ab122f415d04 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -72,11 +72,7 @@ impl FileSystemSandboxRunner { .iter() .map(native_workspace_root) .collect::, _>>()?; - let workspace_roots = if native_workspace_roots.is_empty() { - std::slice::from_ref(&cwd.native) - } else { - native_workspace_roots.as_slice() - }; + let workspace_roots = native_workspace_roots.as_slice(); let native_permissions: PermissionProfile = sandbox.permissions.clone().try_into().map_err(|err| { invalid_request(format!("invalid sandbox permission path URI: {err}")) diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index 050e151e6e62..8354196d0623 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -56,11 +56,7 @@ pub(crate) fn prepare_exec_request( .iter() .map(|root| native_path(root, "sandbox workspace root")) .collect::, _>>()?; - let workspace_roots = if native_workspace_roots.is_empty() { - std::slice::from_ref(&native_sandbox_policy_cwd) - } else { - native_workspace_roots.as_slice() - }; + let workspace_roots = native_workspace_roots.as_slice(); let permissions = permissions.materialize_project_roots_with_workspace_roots(workspace_roots); let managed_mitm_ca_trust_bundle_path = params.managed_network.as_ref().and_then(|_| { CUSTOM_CA_ENV_KEYS.iter().find_map(|key| { diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index 5e5188277216..a4fdeb7a49e8 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -13,26 +13,26 @@ use codex_exec_server::ExecOutputStream; use codex_exec_server::ExecParams; use codex_exec_server::ExecProcess; use codex_exec_server::ExecProcessEvent; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::ProcessId; use codex_exec_server::ProcessSignal; use codex_exec_server::ReadResponse; use codex_exec_server::StartedExecProcess; use codex_exec_server::WriteStatus; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::models::PermissionProfile; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::permissions::FileSystemAccessMode; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::permissions::FileSystemPath; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::permissions::FileSystemSandboxEntry; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::permissions::FileSystemSandboxPolicy; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::permissions::FileSystemSpecialPath; -#[cfg(target_os = "linux")] +#[cfg(unix)] use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; @@ -240,6 +240,56 @@ async fn remote_tty_process_uses_configured_sandbox_helper_with_hostile_path() - Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_process_preserves_empty_workspace_roots() -> Result<()> { + if let Some(warning) = codex_sandboxing::system_bwrap_warning(&PermissionProfile::read_only()) { + eprintln!("skipping bwrap test: {warning}"); + return Ok(()); + } + + let context = create_process_context(/*use_remote*/ true).await?; + let tmp = TempDir::new()?; + let file = tmp.path().join("excluded.txt"); + std::fs::write(&file, b"excluded")?; + let cwd = PathUri::from_host_native_path(tmp.path())?; + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Read, + }]); + let mut sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd( + PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted), + cwd.clone(), + ); + sandbox.workspace_roots.clear(); + + let session = context + .backend + .start(ExecParams { + process_id: ProcessId::from("proc-empty-workspace-roots"), + argv: vec!["/bin/cat".to_string(), file.to_string_lossy().into_owned()], + cwd, + env_policy: None, + env: HashMap::new(), + tty: false, + pipe_stdin: false, + arg0: None, + sandbox: Some(sandbox), + enforce_managed_network: false, + managed_network: None, + }) + .await?; + let (stdout, _stderr, exit_code, closed) = + collect_process_output_from_events(session.process).await?; + + assert!(!stdout.contains("excluded"), "unexpected stdout: {stdout}"); + assert_ne!(exit_code, Some(0)); + assert!(closed); + Ok(()) +} + async fn read_process_until_change( session: Arc, wake_rx: &mut watch::Receiver, diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index bfcf9df601f8..878749acfa11 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -263,6 +263,35 @@ async fn remote_read_file_materializes_environment_workspace_roots() -> Result<( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_read_file_preserves_empty_workspace_roots() -> Result<()> { + let context = create_file_system_context(FileSystemImplementation::Remote).await?; + let file_system = context.file_system; + let tmp = TempDir::new()?; + let file = tmp.path().join("excluded.txt"); + std::fs::write(&file, b"excluded")?; + + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Read, + }]); + let mut sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd( + PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted), + PathUri::from_host_native_path(tmp.path())?, + ); + sandbox.workspace_roots.clear(); + + let error = file_system + .read_file(&PathUri::from_host_native_path(&file)?, Some(&sandbox)) + .await + .expect_err("empty workspace roots should not grant cwd access"); + assert_sandbox_denied(&error); + + Ok(()) +} + #[test_case(FileSystemImplementation::Local ; "local")] #[test_case(FileSystemImplementation::Remote ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 2ef3bcf7107a..e402ef3382c2 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -171,10 +171,11 @@ impl FileSystemSandboxContext { permissions: PermissionProfile, cwd: Option, ) -> Self { + let workspace_roots = cwd.iter().cloned().collect(); Self { permissions: permissions.into(), cwd, - workspace_roots: Vec::new(), + workspace_roots, windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, use_legacy_landlock: false,