From d626dc38950fb40a1a5ad0a8ffab2485e3348c53 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Sun, 12 Apr 2026 18:36:03 -0700 Subject: [PATCH] Run exec-server fs operations through sandbox helper (#17294) ## Summary - run exec-server filesystem RPCs requiring sandboxing through a `codex-fs` arg0 helper over stdin/stdout - keep direct local filesystem execution for `DangerFullAccess` and external sandbox policies - remove the standalone exec-server binary path in favor of top-level arg0 dispatch/runtime paths - add sandbox escape regression coverage for local and remote filesystem paths ## Validation - `just fmt` - `git diff --check` - remote devbox: `cd codex-rs && bazel test --bes_backend= --bes_results_url= //codex-rs/exec-server:all` (6/6 passed) --------- Co-authored-by: Codex --- codex-rs/Cargo.lock | 2 +- codex-rs/app-server-client/src/lib.rs | 1 + codex-rs/app-server/src/fs_api.rs | 11 +- codex-rs/app-server/src/lib.rs | 8 +- codex-rs/apply-patch/src/invocation.rs | 27 +- codex-rs/apply-patch/src/lib.rs | 114 ++-- .../apply-patch/src/standalone_executable.rs | 1 + codex-rs/arg0/src/lib.rs | 9 +- codex-rs/cli/src/main.rs | 19 +- codex-rs/core/src/codex.rs | 17 + codex-rs/core/src/project_doc.rs | 8 +- .../core/src/tools/handlers/apply_patch.rs | 20 +- .../src/tools/handlers/apply_patch_tests.rs | 18 +- .../core/src/tools/handlers/view_image.rs | 7 +- .../core/src/tools/runtimes/apply_patch.rs | 4 + codex-rs/core/tests/common/test_codex.rs | 31 +- codex-rs/core/tests/suite/agents_md.rs | 36 +- .../core/tests/suite/hierarchical_agents.rs | 3 +- codex-rs/core/tests/suite/mod.rs | 7 +- codex-rs/core/tests/suite/remote_env.rs | 7 +- codex-rs/core/tests/suite/unified_exec.rs | 6 +- codex-rs/core/tests/suite/view_image.rs | 16 +- codex-rs/exec-server/BUILD.bazel | 4 + codex-rs/exec-server/Cargo.toml | 6 +- codex-rs/exec-server/README.md | 150 +++-- .../exec-server/src/bin/codex-exec-server.rs | 18 - codex-rs/exec-server/src/environment.rs | 80 ++- codex-rs/exec-server/src/file_system.rs | 95 +-- codex-rs/exec-server/src/fs_helper.rs | 299 ++++++++++ codex-rs/exec-server/src/fs_helper_main.rs | 45 ++ codex-rs/exec-server/src/fs_sandbox.rs | 546 +++++++++++++++++ codex-rs/exec-server/src/lib.rs | 11 +- codex-rs/exec-server/src/local_file_system.rs | 554 ++++++++---------- codex-rs/exec-server/src/protocol.rs | 16 +- .../exec-server/src/remote_file_system.rs | 177 +----- codex-rs/exec-server/src/runtime_paths.rs | 43 ++ .../exec-server/src/sandboxed_file_system.rs | 239 ++++++++ codex-rs/exec-server/src/server.rs | 9 +- .../src/server/file_system_handler.rs | 133 +++-- codex-rs/exec-server/src/server/handler.rs | 4 +- .../exec-server/src/server/handler/tests.rs | 15 + codex-rs/exec-server/src/server/processor.rs | 35 +- codex-rs/exec-server/src/server/transport.rs | 10 +- .../exec-server/tests/common/exec_server.rs | 4 +- codex-rs/exec-server/tests/file_system.rs | 294 ++++------ codex-rs/exec/src/lib.rs | 9 +- codex-rs/mcp-server/src/lib.rs | 8 +- codex-rs/tui/src/lib.rs | 8 +- justfile | 2 +- scripts/run_tui_with_exec_server.sh | 6 +- scripts/start-codex-exec.sh | 4 +- scripts/test-remote-env.sh | 12 +- 52 files changed, 2313 insertions(+), 895 deletions(-) delete mode 100644 codex-rs/exec-server/src/bin/codex-exec-server.rs create mode 100644 codex-rs/exec-server/src/fs_helper.rs create mode 100644 codex-rs/exec-server/src/fs_helper_main.rs create mode 100644 codex-rs/exec-server/src/fs_sandbox.rs create mode 100644 codex-rs/exec-server/src/runtime_paths.rs create mode 100644 codex-rs/exec-server/src/sandboxed_file_system.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 6c101b940..533c28d6f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2097,9 +2097,9 @@ dependencies = [ "arc-swap", "async-trait", "base64 0.22.1", - "clap", "codex-app-server-protocol", "codex-protocol", + "codex-sandboxing", "codex-utils-absolute-path", "codex-utils-cargo-bin", "codex-utils-pty", diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 4eadb0924..eb3d98496 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -44,6 +44,7 @@ use codex_core::config::Config; use codex_core::config_loader::CloudRequirementsLoader; use codex_core::config_loader::LoaderOverrides; pub use codex_exec_server::EnvironmentManager; +pub use codex_exec_server::ExecServerRuntimePaths; use codex_feedback::CodexFeedback; use codex_protocol::protocol::SessionSource; use serde::de::DeserializeOwned; diff --git a/codex-rs/app-server/src/fs_api.rs b/codex-rs/app-server/src/fs_api.rs index 57b355f81..8540b9210 100644 --- a/codex-rs/app-server/src/fs_api.rs +++ b/codex-rs/app-server/src/fs_api.rs @@ -46,7 +46,7 @@ impl FsApi { ) -> Result { let bytes = self .file_system - .read_file(¶ms.path) + .read_file(¶ms.path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsReadFileResponse { @@ -64,7 +64,7 @@ impl FsApi { )) })?; self.file_system - .write_file(¶ms.path, bytes) + .write_file(¶ms.path, bytes, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsWriteFileResponse {}) @@ -80,6 +80,7 @@ impl FsApi { CreateDirectoryOptions { recursive: params.recursive.unwrap_or(true), }, + /*sandbox*/ None, ) .await .map_err(map_fs_error)?; @@ -92,7 +93,7 @@ impl FsApi { ) -> Result { let metadata = self .file_system - .get_metadata(¶ms.path) + .get_metadata(¶ms.path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsGetMetadataResponse { @@ -109,7 +110,7 @@ impl FsApi { ) -> Result { let entries = self .file_system - .read_directory(¶ms.path) + .read_directory(¶ms.path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsReadDirectoryResponse { @@ -135,6 +136,7 @@ impl FsApi { recursive: params.recursive.unwrap_or(true), force: params.force.unwrap_or(true), }, + /*sandbox*/ None, ) .await .map_err(map_fs_error)?; @@ -152,6 +154,7 @@ impl FsApi { CopyOptions { recursive: params.recursive, }, + /*sandbox*/ None, ) .await .map_err(map_fs_error)?; diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index bfc87251a..bdd24274c 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -44,6 +44,7 @@ use codex_core::check_execpolicy_for_warnings; use codex_core::config_loader::ConfigLoadError; use codex_core::config_loader::TextRange as CoreTextRange; use codex_exec_server::EnvironmentManager; +use codex_exec_server::ExecServerRuntimePaths; use codex_feedback::CodexFeedback; use codex_protocol::protocol::SessionSource; use codex_state::log_db; @@ -360,7 +361,12 @@ pub async fn run_main_with_transport( session_source: SessionSource, auth: AppServerWebsocketAuthSettings, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::from_env()); + let environment_manager = Arc::new(EnvironmentManager::from_env_with_runtime_paths(Some( + ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?, + ))); let (transport_event_tx, mut transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(CHANNEL_CAPACITY); diff --git a/codex-rs/apply-patch/src/invocation.rs b/codex-rs/apply-patch/src/invocation.rs index 3b0db2fa9..075c94c60 100644 --- a/codex-rs/apply-patch/src/invocation.rs +++ b/codex-rs/apply-patch/src/invocation.rs @@ -135,6 +135,7 @@ pub async fn maybe_parse_apply_patch_verified( argv: &[String], cwd: &AbsolutePathBuf, fs: &dyn ExecutorFileSystem, + sandbox: Option<&codex_exec_server::FileSystemSandboxContext>, ) -> MaybeApplyPatchVerified { // Detect a raw patch body passed directly as the command or as the body of a shell // script. In these cases, report an explicit error rather than applying the patch. @@ -170,7 +171,7 @@ pub async fn maybe_parse_apply_patch_verified( ); } Hunk::DeleteFile { .. } => { - let content = match fs.read_file_text(&path).await { + let content = match fs.read_file_text(&path, sandbox).await { Ok(content) => content, Err(e) => { return MaybeApplyPatchVerified::CorrectnessError( @@ -192,7 +193,7 @@ pub async fn maybe_parse_apply_patch_verified( let ApplyPatchFileUpdate { unified_diff, content: contents, - } = match unified_diff_from_chunks(&path, &chunks, fs).await { + } = match unified_diff_from_chunks(&path, &chunks, fs, sandbox).await { Ok(diff) => diff, Err(e) => { return MaybeApplyPatchVerified::CorrectnessError(e); @@ -467,7 +468,8 @@ mod tests { maybe_parse_apply_patch_verified( &args, &AbsolutePathBuf::from_absolute_path(dir.path()).unwrap(), - LOCAL_FS.as_ref() + LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await, MaybeApplyPatchVerified::CorrectnessError(ApplyPatchError::ImplicitInvocation) @@ -483,7 +485,8 @@ mod tests { maybe_parse_apply_patch_verified( &args, &AbsolutePathBuf::from_absolute_path(dir.path()).unwrap(), - LOCAL_FS.as_ref() + LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await, MaybeApplyPatchVerified::CorrectnessError(ApplyPatchError::ImplicitInvocation) @@ -693,9 +696,10 @@ PATCH"#, }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = + unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref(), /*sandbox*/ None) + .await + .unwrap(); let expected_diff = r#"@@ -2,2 +2,2 @@ bar -baz @@ -731,9 +735,10 @@ PATCH"#, }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = + unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref(), /*sandbox*/ None) + .await + .unwrap(); let expected_diff = r#"@@ -3 +3,2 @@ baz +quux @@ -770,6 +775,7 @@ PATCH"#, &argv, &AbsolutePathBuf::from_absolute_path(session_dir.path()).unwrap(), LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await; @@ -823,6 +829,7 @@ PATCH"#, &argv, &AbsolutePathBuf::from_absolute_path(session_dir.path()).unwrap(), LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await; let action = match result { diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 6b6aba2df..8be5ef0f4 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -12,6 +12,7 @@ use anyhow::Context; use anyhow::Result; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::RemoveOptions; use codex_utils_absolute_path::AbsolutePathBuf; pub use parser::Hunk; @@ -184,6 +185,7 @@ pub async fn apply_patch( stdout: &mut impl std::io::Write, stderr: &mut impl std::io::Write, fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, ) -> Result<(), ApplyPatchError> { let hunks = match parse_patch(patch) { Ok(source) => source.hunks, @@ -207,7 +209,7 @@ pub async fn apply_patch( } }; - apply_hunks(&hunks, cwd, stdout, stderr, fs).await?; + apply_hunks(&hunks, cwd, stdout, stderr, fs, sandbox).await?; Ok(()) } @@ -219,9 +221,10 @@ pub async fn apply_hunks( stdout: &mut impl std::io::Write, stderr: &mut impl std::io::Write, fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, ) -> Result<(), ApplyPatchError> { // Delegate to a helper that applies each hunk to the filesystem. - match apply_hunks_to_files(hunks, cwd, fs).await { + match apply_hunks_to_files(hunks, cwd, fs, sandbox).await { Ok(affected) => { print_summary(&affected, stdout).map_err(ApplyPatchError::from)?; Ok(()) @@ -257,6 +260,7 @@ async fn apply_hunks_to_files( hunks: &[Hunk], cwd: &AbsolutePathBuf, fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, ) -> anyhow::Result { if hunks.is_empty() { anyhow::bail!("No files were modified."); @@ -271,23 +275,27 @@ async fn apply_hunks_to_files( match hunk { Hunk::AddFile { contents, .. } => { if let Some(parent_abs) = path_abs.parent() { - fs.create_directory(&parent_abs, CreateDirectoryOptions { recursive: true }) - .await - .with_context(|| { - format!( - "Failed to create parent directories for {}", - path_abs.display() - ) - })?; + fs.create_directory( + &parent_abs, + CreateDirectoryOptions { recursive: true }, + sandbox, + ) + .await + .with_context(|| { + format!( + "Failed to create parent directories for {}", + path_abs.display() + ) + })?; } - fs.write_file(&path_abs, contents.clone().into_bytes()) + fs.write_file(&path_abs, contents.clone().into_bytes(), sandbox) .await .with_context(|| format!("Failed to write file {}", path_abs.display()))?; added.push(affected_path); } Hunk::DeleteFile { .. } => { let result: io::Result<()> = async { - let metadata = fs.get_metadata(&path_abs).await?; + let metadata = fs.get_metadata(&path_abs, sandbox).await?; if metadata.is_directory { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -300,6 +308,7 @@ async fn apply_hunks_to_files( recursive: false, force: false, }, + sandbox, ) .await } @@ -311,13 +320,14 @@ async fn apply_hunks_to_files( move_path, chunks, .. } => { let AppliedPatch { new_contents, .. } = - derive_new_contents_from_chunks(&path_abs, chunks, fs).await?; + derive_new_contents_from_chunks(&path_abs, chunks, fs, sandbox).await?; if let Some(dest) = move_path { let dest_abs = AbsolutePathBuf::resolve_path_against_base(dest, cwd); if let Some(parent_abs) = dest_abs.parent() { fs.create_directory( &parent_abs, CreateDirectoryOptions { recursive: true }, + sandbox, ) .await .with_context(|| { @@ -327,11 +337,11 @@ async fn apply_hunks_to_files( ) })?; } - fs.write_file(&dest_abs, new_contents.into_bytes()) + fs.write_file(&dest_abs, new_contents.into_bytes(), sandbox) .await .with_context(|| format!("Failed to write file {}", dest_abs.display()))?; let result: io::Result<()> = async { - let metadata = fs.get_metadata(&path_abs).await?; + let metadata = fs.get_metadata(&path_abs, sandbox).await?; if metadata.is_directory { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -344,6 +354,7 @@ async fn apply_hunks_to_files( recursive: false, force: false, }, + sandbox, ) .await } @@ -353,7 +364,7 @@ async fn apply_hunks_to_files( })?; modified.push(affected_path); } else { - fs.write_file(&path_abs, new_contents.into_bytes()) + fs.write_file(&path_abs, new_contents.into_bytes(), sandbox) .await .with_context(|| format!("Failed to write file {}", path_abs.display()))?; modified.push(affected_path); @@ -379,8 +390,9 @@ async fn derive_new_contents_from_chunks( path_abs: &AbsolutePathBuf, chunks: &[UpdateFileChunk], fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, ) -> std::result::Result { - let original_contents = fs.read_file_text(path_abs).await.map_err(|err| { + let original_contents = fs.read_file_text(path_abs, sandbox).await.map_err(|err| { ApplyPatchError::IoError(IoError { context: format!("Failed to read file to update {}", path_abs.display()), source: err, @@ -540,8 +552,9 @@ pub async fn unified_diff_from_chunks( path_abs: &AbsolutePathBuf, chunks: &[UpdateFileChunk], fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, ) -> std::result::Result { - unified_diff_from_chunks_with_context(path_abs, chunks, /*context*/ 1, fs).await + unified_diff_from_chunks_with_context(path_abs, chunks, /*context*/ 1, fs, sandbox).await } pub async fn unified_diff_from_chunks_with_context( @@ -549,11 +562,12 @@ pub async fn unified_diff_from_chunks_with_context( chunks: &[UpdateFileChunk], context: usize, fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, ) -> std::result::Result { let AppliedPatch { original_contents, new_contents, - } = derive_new_contents_from_chunks(path_abs, chunks, fs).await?; + } = derive_new_contents_from_chunks(path_abs, chunks, fs, sandbox).await?; let text_diff = TextDiff::from_lines(&original_contents, &new_contents); let unified_diff = text_diff.unified_diff().context_radius(context).to_string(); Ok(ApplyPatchFileUpdate { @@ -614,6 +628,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -667,9 +682,16 @@ mod tests { let mut stdout = Vec::new(); let mut stderr = Vec::new(); - apply_patch(&patch, &cwd, &mut stdout, &mut stderr, LOCAL_FS.as_ref()) - .await - .unwrap(); + apply_patch( + &patch, + &cwd, + &mut stdout, + &mut stderr, + LOCAL_FS.as_ref(), + /*sandbox*/ None, + ) + .await + .unwrap(); assert_eq!(fs::read_to_string(&relative_add).unwrap(), "relative add\n"); assert_eq!(fs::read_to_string(&absolute_add).unwrap(), "absolute add\n"); @@ -709,6 +731,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -744,6 +767,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -783,6 +807,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -831,6 +856,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -888,6 +914,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -931,6 +958,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -973,6 +1001,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -1019,9 +1048,14 @@ mod tests { _ => panic!("Expected a single UpdateFile hunk"), }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, update_file_chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = unified_diff_from_chunks( + &path_abs, + update_file_chunks, + LOCAL_FS.as_ref(), + /*sandbox*/ None, + ) + .await + .unwrap(); let expected_diff = r#"@@ -1,4 +1,4 @@ foo -bar @@ -1061,9 +1095,10 @@ mod tests { }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = + unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref(), /*sandbox*/ None) + .await + .unwrap(); let expected_diff = r#"@@ -1,2 +1,2 @@ -foo +FOO @@ -1101,9 +1136,10 @@ mod tests { }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = + unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref(), /*sandbox*/ None) + .await + .unwrap(); let expected_diff = r#"@@ -2,2 +2,2 @@ bar -baz @@ -1139,9 +1175,10 @@ mod tests { }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = + unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref(), /*sandbox*/ None) + .await + .unwrap(); let expected_diff = r#"@@ -3 +3,2 @@ baz +quux @@ -1188,9 +1225,10 @@ mod tests { }; let path_abs = path.as_path().abs(); - let diff = unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref()) - .await - .unwrap(); + let diff = + unified_diff_from_chunks(&path_abs, chunks, LOCAL_FS.as_ref(), /*sandbox*/ None) + .await + .unwrap(); let expected_diff = r#"@@ -1,6 +1,7 @@ a @@ -1219,6 +1257,7 @@ mod tests { &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await .unwrap(); @@ -1258,6 +1297,7 @@ g &mut stdout, &mut stderr, LOCAL_FS.as_ref(), + /*sandbox*/ None, ) .await; assert!(result.is_err()); diff --git a/codex-rs/apply-patch/src/standalone_executable.rs b/codex-rs/apply-patch/src/standalone_executable.rs index 149bfd338..093bda543 100644 --- a/codex-rs/apply-patch/src/standalone_executable.rs +++ b/codex-rs/apply-patch/src/standalone_executable.rs @@ -71,6 +71,7 @@ pub fn run_main() -> i32 { &mut stdout, &mut stderr, codex_exec_server::LOCAL_FS.as_ref(), + /*sandbox*/ None, )) { Ok(()) => { // Flush to ensure output ordering when used in pipelines. diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index f8b61796b..deb18fb99 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -4,6 +4,7 @@ use std::path::Path; use std::path::PathBuf; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; +use codex_exec_server::CODEX_FS_HELPER_ARG1; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_utils_home_dir::find_codex_home; #[cfg(unix)] @@ -93,6 +94,9 @@ pub fn arg0_dispatch() -> Option { } let argv1 = args.next().unwrap_or_default(); + if argv1 == CODEX_FS_HELPER_ARG1 { + codex_exec_server::run_fs_helper_main(); + } if argv1 == CODEX_CORE_APPLY_PATCH_ARG1 { let patch_arg = args.next().and_then(|s| s.to_str().map(str::to_owned)); let exit_code = match patch_arg { @@ -116,6 +120,7 @@ pub fn arg0_dispatch() -> Option { &mut stdout, &mut stderr, codex_exec_server::LOCAL_FS.as_ref(), + /*sandbox*/ None, )) { Ok(()) => 0, Err(_) => 1, @@ -325,13 +330,13 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result anyhow::Result<()> { root_remote_auth_token_env.as_deref(), "exec-server", )?; - run_exec_server_command(cmd).await?; + run_exec_server_command(cmd, &arg0_paths).await?; } Some(Subcommand::Features(FeaturesCli { sub })) => match sub { FeaturesSubcommand::List => { @@ -1103,8 +1103,19 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { Ok(()) } -async fn run_exec_server_command(cmd: ExecServerCommand) -> anyhow::Result<()> { - codex_exec_server::run_main_with_listen_url(&cmd.listen) +async fn run_exec_server_command( + cmd: ExecServerCommand, + arg0_paths: &Arg0DispatchPaths, +) -> anyhow::Result<()> { + let codex_self_exe = arg0_paths + .codex_self_exe + .clone() + .ok_or_else(|| anyhow::anyhow!("Codex executable path is not configured"))?; + let runtime_paths = codex_exec_server::ExecServerRuntimePaths::new( + codex_self_exe, + arg0_paths.codex_linux_sandbox_exe.clone(), + )?; + codex_exec_server::run_main(&cmd.listen, runtime_paths) .await .map_err(anyhow::Error::from_boxed) } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index c19c4b48f..a3a674cb6 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -62,6 +62,7 @@ use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; +use codex_exec_server::FileSystemSandboxContext; use codex_features::FEATURES; use codex_features::Feature; use codex_features::unstable_features_warning_event; @@ -1040,6 +1041,22 @@ impl TurnContext { .map_or_else(|| self.cwd.clone(), |path| self.cwd.join(path)) } + pub(crate) fn file_system_sandbox_context( + &self, + additional_permissions: Option, + ) -> FileSystemSandboxContext { + FileSystemSandboxContext { + sandbox_policy: self.sandbox_policy.get().clone(), + windows_sandbox_level: self.windows_sandbox_level, + windows_sandbox_private_desktop: self + .config + .permissions + .windows_sandbox_private_desktop, + use_legacy_landlock: self.features.use_legacy_landlock(), + additional_permissions, + } + } + pub(crate) fn compact_prompt(&self) -> &str { self.compact_prompt .as_deref() diff --git a/codex-rs/core/src/project_doc.rs b/codex-rs/core/src/project_doc.rs index e7321695a..2234ad48f 100644 --- a/codex-rs/core/src/project_doc.rs +++ b/codex-rs/core/src/project_doc.rs @@ -169,14 +169,14 @@ async fn read_project_docs_with_fs( break; } - match fs.get_metadata(&p).await { + match fs.get_metadata(&p, /*sandbox*/ None).await { Ok(metadata) if !metadata.is_file => continue, Ok(_) => {} Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) => return Err(err), } - let mut data = match fs.read_file(&p).await { + let mut data = match fs.read_file(&p, /*sandbox*/ None).await { Ok(data) => data, Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) => return Err(err), @@ -249,7 +249,7 @@ pub async fn discover_project_doc_paths( for ancestor in dir.ancestors() { for marker in &project_root_markers { let marker_path = AbsolutePathBuf::try_from(ancestor.join(marker))?; - let marker_exists = match fs.get_metadata(&marker_path).await { + let marker_exists = match fs.get_metadata(&marker_path, /*sandbox*/ None).await { Ok(_) => true, Err(err) if err.kind() == io::ErrorKind::NotFound => false, Err(err) => return Err(err), @@ -289,7 +289,7 @@ pub async fn discover_project_doc_paths( for d in search_dirs { for name in &candidate_filenames { let candidate = d.join(name); - match fs.get_metadata(&candidate).await { + match fs.get_metadata(&candidate, /*sandbox*/ None).await { Ok(md) if md.is_file => { found.push(candidate); break; diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 281c3aa6e..3ab1c1454 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -176,7 +176,16 @@ impl ToolHandler for ApplyPatchHandler { )); }; let fs = environment.get_filesystem(); - match codex_apply_patch::maybe_parse_apply_patch_verified(&command, &cwd, fs.as_ref()).await + let sandbox = environment + .is_remote() + .then(|| turn.file_system_sandbox_context(/*additional_permissions*/ None)); + match codex_apply_patch::maybe_parse_apply_patch_verified( + &command, + &cwd, + fs.as_ref(), + sandbox.as_ref(), + ) + .await { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { let (file_paths, effective_additional_permissions, file_system_sandbox_policy) = @@ -273,7 +282,14 @@ pub(crate) async fn intercept_apply_patch( call_id: &str, tool_name: &str, ) -> Result, FunctionCallError> { - match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs).await { + let sandbox = turn + .environment + .as_ref() + .filter(|env| env.is_remote()) + .map(|_| turn.file_system_sandbox_context(/*additional_permissions*/ None)); + match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, sandbox.as_ref()) + .await + { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { session .record_model_warning( diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index afe2e09db..86e05fb8a 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -24,13 +24,17 @@ async fn approval_keys_include_move_destination() { +new content *** End Patch"#; let argv = vec!["apply_patch".to_string(), patch.to_string()]; - let action = - match codex_apply_patch::maybe_parse_apply_patch_verified(&argv, &cwd, LOCAL_FS.as_ref()) - .await - { - MaybeApplyPatchVerified::Body(action) => action, - other => panic!("expected patch body, got: {other:?}"), - }; + let action = match codex_apply_patch::maybe_parse_apply_patch_verified( + &argv, + &cwd, + LOCAL_FS.as_ref(), + /*sandbox*/ None, + ) + .await + { + MaybeApplyPatchVerified::Body(action) => action, + other => panic!("expected patch body, got: {other:?}"), + }; let keys = file_paths_for_action(&action); assert_eq!(keys.len(), 2); diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 5e39ae618..1c9ddd315 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -92,10 +92,13 @@ impl ToolHandler for ViewImageHandler { "view_image is unavailable in this session".to_string(), )); }; + let sandbox = environment + .is_remote() + .then(|| turn.file_system_sandbox_context(/*additional_permissions*/ None)); let metadata = environment .get_filesystem() - .get_metadata(&abs_path) + .get_metadata(&abs_path, sandbox.as_ref()) .await .map_err(|error| { FunctionCallError::RespondToModel(format!( @@ -112,7 +115,7 @@ impl ToolHandler for ViewImageHandler { } let file_bytes = environment .get_filesystem() - .read_file(&abs_path) + .read_file(&abs_path, sandbox.as_ref()) .await .map_err(|error| { FunctionCallError::RespondToModel(format!( diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 51f0cc83d..8324f365b 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -218,6 +218,9 @@ impl ToolRuntime for ApplyPatchRuntime { if let Some(environment) = ctx.turn.environment.as_ref().filter(|env| env.is_remote()) { let started_at = Instant::now(); let fs = environment.get_filesystem(); + let sandbox = ctx + .turn + .file_system_sandbox_context(req.additional_permissions.clone()); let mut stdout = Vec::new(); let mut stderr = Vec::new(); let result = codex_apply_patch::apply_patch( @@ -226,6 +229,7 @@ impl ToolRuntime for ApplyPatchRuntime { &mut stdout, &mut stderr, fs.as_ref(), + Some(&sandbox), ) .await; let stdout = String::from_utf8_lossy(&stdout).into_owned(); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index a3b11d71b..33289522a 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -148,7 +148,11 @@ pub async fn test_env() -> Result { let cwd = remote_aware_cwd_path(); environment .get_filesystem() - .create_directory(&cwd, CreateDirectoryOptions { recursive: true }) + .create_directory( + &cwd, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) .await?; remote_process.process.register_cleanup_path(cwd.as_path()); Ok(TestEnv { @@ -170,10 +174,9 @@ struct RemoteExecServerStart { fn start_remote_exec_server(remote_env: &RemoteEnvConfig) -> Result { let container_name = remote_env.container_name.as_str(); let instance_id = remote_exec_server_instance_id(); - let remote_exec_server_path = format!("/tmp/codex-exec-server-{instance_id}"); + let remote_exec_server_path = format!("/tmp/codex-{instance_id}"); let stdout_path = format!("/tmp/codex-exec-server-{instance_id}.stdout"); - let local_binary = codex_utils_cargo_bin::cargo_bin("codex-exec-server") - .context("resolve codex-exec-server binary")?; + let local_binary = codex_utils_cargo_bin::cargo_bin("codex").context("resolve codex binary")?; let local_binary = local_binary.to_string_lossy().to_string(); let remote_binary = format!("{container_name}:{remote_exec_server_path}"); @@ -188,7 +191,7 @@ fn start_remote_exec_server(remote_env: &RemoteEnvConfig) -> Result {stdout_path} 2>&1 & \ +nohup {remote_exec_server_path} exec-server --listen ws://0.0.0.0:0 > {stdout_path} 2>&1 & \ echo $!" ); let pid_output = @@ -836,18 +839,26 @@ impl TestCodexHarness { if let Some(parent) = abs_path.parent() { self.test .fs() - .create_directory(&parent, CreateDirectoryOptions { recursive: true }) + .create_directory( + &parent, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) .await?; } self.test .fs() - .write_file(&abs_path, contents.as_ref().to_vec()) + .write_file(&abs_path, contents.as_ref().to_vec(), /*sandbox*/ None) .await?; Ok(()) } pub async fn read_file_text(&self, rel: impl AsRef) -> Result { - Ok(self.test.fs().read_file_text(&self.path_abs(rel)).await?) + Ok(self + .test + .fs() + .read_file_text(&self.path_abs(rel), /*sandbox*/ None) + .await?) } pub async fn create_dir_all(&self, rel: impl AsRef) -> Result<()> { @@ -856,6 +867,7 @@ impl TestCodexHarness { .create_directory( &self.path_abs(rel), CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, ) .await?; Ok(()) @@ -874,13 +886,14 @@ impl TestCodexHarness { recursive: false, force: true, }, + /*sandbox*/ None, ) .await?; Ok(()) } pub async fn abs_path_exists(&self, path: &AbsolutePathBuf) -> Result { - match self.test.fs().get_metadata(path).await { + match self.test.fs().get_metadata(path, /*sandbox*/ None).await { Ok(_) => Ok(true), Err(err) if err.kind() == ErrorKind::NotFound => Ok(false), Err(err) => Err(err.into()), diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index ed62c9038..724b85239 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -33,9 +33,14 @@ async fn agents_override_is_preferred_over_agents_md() -> Result<()> { agents_instructions(test_codex().with_workspace_setup(|cwd, fs| async move { let agents_md = cwd.join("AGENTS.md"); let override_md = cwd.join("AGENTS.override.md"); - fs.write_file(&agents_md, b"base doc".to_vec()).await?; - fs.write_file(&override_md, b"override doc".to_vec()) + fs.write_file(&agents_md, b"base doc".to_vec(), /*sandbox*/ None) .await?; + fs.write_file( + &override_md, + b"override doc".to_vec(), + /*sandbox*/ None, + ) + .await?; Ok::<(), anyhow::Error>(()) })) .await?; @@ -62,9 +67,14 @@ async fn configured_fallback_is_used_when_agents_candidate_is_directory() -> Res .with_workspace_setup(|cwd, fs| async move { let agents_dir = cwd.join("AGENTS.md"); let fallback = cwd.join("WORKFLOW.md"); - fs.create_directory(&agents_dir, CreateDirectoryOptions { recursive: true }) + fs.create_directory( + &agents_dir, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + fs.write_file(&fallback, b"fallback doc".to_vec(), /*sandbox*/ None) .await?; - fs.write_file(&fallback, b"fallback doc".to_vec()).await?; Ok::<(), anyhow::Error>(()) }), ) @@ -95,12 +105,22 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { let git_marker = root.join(".git"); let nested_agents = nested.join("AGENTS.md"); - fs.create_directory(&nested, CreateDirectoryOptions { recursive: true }) + fs.create_directory( + &nested, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + fs.write_file(&root_agents, b"root doc".to_vec(), /*sandbox*/ None) .await?; - fs.write_file(&root_agents, b"root doc".to_vec()).await?; - fs.write_file(&git_marker, b"gitdir: /tmp/mock-git-dir\n".to_vec()) + fs.write_file( + &git_marker, + b"gitdir: /tmp/mock-git-dir\n".to_vec(), + /*sandbox*/ None, + ) + .await?; + fs.write_file(&nested_agents, b"child doc".to_vec(), /*sandbox*/ None) .await?; - fs.write_file(&nested_agents, b"child doc".to_vec()).await?; Ok::<(), anyhow::Error>(()) }), ) diff --git a/codex-rs/core/tests/suite/hierarchical_agents.rs b/codex-rs/core/tests/suite/hierarchical_agents.rs index f0960c072..212303ed0 100644 --- a/codex-rs/core/tests/suite/hierarchical_agents.rs +++ b/codex-rs/core/tests/suite/hierarchical_agents.rs @@ -27,7 +27,8 @@ async fn hierarchical_agents_appends_to_project_doc_in_user_instructions() { }) .with_workspace_setup(|cwd, fs| async move { let agents_md = cwd.join("AGENTS.md"); - fs.write_file(&agents_md, b"be nice".to_vec()).await?; + fs.write_file(&agents_md, b"be nice".to_vec(), /*sandbox*/ None) + .await?; Ok::<(), anyhow::Error>(()) }); let test = builder diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index a553cbced..cb6f5b817 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -30,9 +30,14 @@ pub static CODEX_ALIASES_TEMP_DIR: Option = { .and_then(|name| name.to_str()) .unwrap_or(""); let argv1 = args.next().unwrap_or_default(); + if argv1 == CODEX_CORE_APPLY_PATCH_ARG1 { + let _ = arg0_dispatch(); + return None; + } + // Helper re-execs inherit this ctor too, but they may run inside a sandbox // where creating another CODEX_HOME tempdir under /tmp is not allowed. - if exe_name == CODEX_LINUX_SANDBOX_ARG0 || argv1 == CODEX_CORE_APPLY_PATCH_ARG1 { + if exe_name == CODEX_LINUX_SANDBOX_ARG0 { return None; } diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 4cd9568a5..0307dc511 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -21,9 +21,11 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> { let payload = b"remote-test-env-ok".to_vec(); file_system - .write_file(&file_path_abs, payload.clone()) + .write_file(&file_path_abs, payload.clone(), /*sandbox*/ None) + .await?; + let actual = file_system + .read_file(&file_path_abs, /*sandbox*/ None) .await?; - let actual = file_system.read_file(&file_path_abs).await?; assert_eq!(actual, payload); file_system @@ -33,6 +35,7 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> { recursive: false, force: true, }, + /*sandbox*/ None, ) .await?; diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 7470717a0..772f3cc0a 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -191,7 +191,11 @@ async fn create_workspace_directory( ) -> Result { let abs_path = test.config.cwd.join(rel_path.as_ref()); test.fs() - .create_directory(&abs_path, CreateDirectoryOptions { recursive: true }) + .create_directory( + &abs_path, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) .await?; Ok(abs_path.into_path_buf()) } diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 06dec12b4..c2dab9d17 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -87,7 +87,11 @@ fn png_bytes(width: u32, height: u32, rgba: [u8; 4]) -> anyhow::Result> async fn create_workspace_directory(test: &TestCodex, rel_path: &str) -> anyhow::Result { let abs_path = test.config.cwd.join(rel_path); test.fs() - .create_directory(&abs_path, CreateDirectoryOptions { recursive: true }) + .create_directory( + &abs_path, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) .await?; Ok(abs_path.into_path_buf()) } @@ -100,10 +104,16 @@ async fn write_workspace_file( let abs_path = test.config.cwd.join(rel_path); if let Some(parent) = abs_path.parent() { test.fs() - .create_directory(&parent, CreateDirectoryOptions { recursive: true }) + .create_directory( + &parent, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) .await?; } - test.fs().write_file(&abs_path, contents).await?; + test.fs() + .write_file(&abs_path, contents, /*sandbox*/ None) + .await?; Ok(abs_path.into_path_buf()) } diff --git a/codex-rs/exec-server/BUILD.bazel b/codex-rs/exec-server/BUILD.bazel index 5d62c68ca..ea464aec3 100644 --- a/codex-rs/exec-server/BUILD.bazel +++ b/codex-rs/exec-server/BUILD.bazel @@ -3,5 +3,9 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "exec-server", crate_name = "codex_exec_server", + extra_binaries = [ + "//codex-rs/cli:codex", + "//codex-rs/linux-sandbox:codex-linux-sandbox", + ], test_tags = ["no-sandbox"], ) diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 6bf5528d3..25570dc71 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -7,10 +7,6 @@ license.workspace = true [lib] doctest = false -[[bin]] -name = "codex-exec-server" -path = "src/bin/codex-exec-server.rs" - [lints] workspace = true @@ -18,9 +14,9 @@ workspace = true arc-swap = { workspace = true } async-trait = { workspace = true } base64 = { workspace = true } -clap = { workspace = true, features = ["derive"] } codex-app-server-protocol = { workspace = true } codex-protocol = { workspace = true } +codex-sandboxing = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-pty = { workspace = true } futures = { workspace = true } diff --git a/codex-rs/exec-server/README.md b/codex-rs/exec-server/README.md index 3c71dfa19..0047449d3 100644 --- a/codex-rs/exec-server/README.md +++ b/codex-rs/exec-server/README.md @@ -1,27 +1,25 @@ # codex-exec-server -`codex-exec-server` is a small standalone JSON-RPC server for spawning -and controlling subprocesses through `codex-utils-pty`. +`codex-exec-server` is the library backing `codex exec-server`, a small +JSON-RPC server for spawning and controlling subprocesses through +`codex-utils-pty`. -This PR intentionally lands only the standalone binary, client, wire protocol, -and docs. Exec and filesystem methods are stubbed server-side here and are -implemented in follow-up PRs. +It provides: -It currently provides: - -- a standalone binary: `codex-exec-server` +- a CLI entrypoint: `codex exec-server` - a Rust client: `ExecServerClient` - a small protocol module with shared request/response types -This crate is intentionally narrow. It is not wired into the main Codex CLI or -unified-exec in this PR; it is only the standalone transport layer. +This crate owns the transport, protocol, and filesystem/process handlers. The +top-level `codex` binary owns hidden helper dispatch for sandboxed +filesystem operations and `codex-linux-sandbox`. ## Transport The server speaks the shared `codex-app-server-protocol` message envelope on the wire. -The standalone binary supports: +The CLI entrypoint supports: - `ws://IP:PORT` (default) @@ -36,7 +34,7 @@ Each connection follows this sequence: 1. Send `initialize`. 2. Wait for the `initialize` response. 3. Send `initialized`. -4. Call exec or filesystem RPCs once the follow-up implementation PRs land. +4. Call process or filesystem RPCs. If the server receives any notification other than `initialized`, it replies with an error using request id `-1`. @@ -72,7 +70,7 @@ Handshake acknowledgement notification sent by the client after a successful Params are currently ignored. Sending any other notification method is treated as an invalid request. -### `command/exec` +### `process/start` Starts a new managed process. @@ -87,7 +85,6 @@ Request params: "PATH": "/usr/bin:/bin" }, "tty": true, - "outputBytesCap": 16384, "arg0": null } ``` @@ -100,31 +97,61 @@ Field definitions: - `env`: environment variables passed to the child process. - `tty`: when `true`, spawn a PTY-backed interactive process; when `false`, spawn a pipe-backed process with closed stdin. -- `outputBytesCap`: maximum retained stdout/stderr bytes per stream for the - in-memory buffer. Defaults to `codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP`. - `arg0`: optional argv0 override forwarded to `codex-utils-pty`. Response: ```json { - "processId": "proc-1", - "running": true, - "exitCode": null, - "stdout": null, - "stderr": null + "processId": "proc-1" } ``` Behavior notes: - Reusing an existing `processId` is rejected. -- PTY-backed processes accept later writes through `command/exec/write`. +- PTY-backed processes accept later writes through `process/write`. - Pipe-backed processes are launched with stdin closed and reject writes. -- Output is streamed asynchronously via `command/exec/outputDelta`. -- Exit is reported asynchronously via `command/exec/exited`. +- Output is streamed asynchronously via `process/output`. +- Exit is reported asynchronously via `process/exited`. -### `command/exec/write` +### `process/read` + +Reads buffered output and terminal state for a managed process. + +Request params: + +```json +{ + "processId": "proc-1", + "afterSeq": null, + "maxBytes": 65536, + "waitMs": 1000 +} +``` + +Field definitions: + +- `processId`: managed process id returned by `process/start`. +- `afterSeq`: optional sequence number cursor; when present, only newer chunks + are returned. +- `maxBytes`: optional response byte budget. +- `waitMs`: optional long-poll timeout in milliseconds. + +Response: + +```json +{ + "chunks": [], + "nextSeq": 1, + "exited": false, + "exitCode": null, + "closed": false, + "failure": null +} +``` + +### `process/write` Writes raw bytes to a running PTY-backed process stdin. @@ -143,7 +170,7 @@ Response: ```json { - "accepted": true + "status": "accepted" } ``` @@ -152,7 +179,7 @@ Behavior notes: - Writes to an unknown `processId` are rejected. - Writes to a non-PTY process are rejected because stdin is already closed. -### `command/exec/terminate` +### `process/terminate` Terminates a running managed process. @@ -182,7 +209,7 @@ If the process is already unknown or already removed, the server responds with: ## Notifications -### `command/exec/outputDelta` +### `process/output` Streaming output chunk from a running process. @@ -191,6 +218,7 @@ Params: ```json { "processId": "proc-1", + "seq": 1, "stream": "stdout", "chunk": "aGVsbG8K" } @@ -199,10 +227,11 @@ Params: Fields: - `processId`: process identifier -- `stream`: `"stdout"` or `"stderr"` +- `seq`: per-process output sequence number +- `stream`: `"stdout"`, `"stderr"`, or `"pty"` - `chunk`: base64-encoded output bytes -### `command/exec/exited` +### `process/exited` Final process exit notification. @@ -211,10 +240,43 @@ Params: ```json { "processId": "proc-1", + "seq": 2, "exitCode": 0 } ``` +### `process/closed` + +Notification emitted after process output is closed and the process handle is +removed. + +Params: + +```json +{ + "processId": "proc-1" +} +``` + +## Filesystem RPCs + +Filesystem methods use absolute paths and return JSON-RPC errors for invalid +or unavailable paths: + +- `fs/readFile` +- `fs/writeFile` +- `fs/createDirectory` +- `fs/getMetadata` +- `fs/readDirectory` +- `fs/remove` +- `fs/copy` + +Each filesystem request accepts an optional `sandbox` object. When `sandbox` +contains a `ReadOnly` or `WorkspaceWrite` policy, the operation runs in a +hidden helper process launched from the top-level `codex` executable and +prepared through the shared sandbox transform path. Helper requests and +responses are passed over stdin/stdout. + ## Errors The server returns JSON-RPC errors with these codes: @@ -231,6 +293,7 @@ Typical error cases: - duplicate `processId` - writes to unknown processes - writes to non-PTY processes +- sandbox-denied filesystem operations ## Rust surface @@ -240,10 +303,14 @@ The crate exports: - `ExecServerError` - `ExecServerClientConnectOptions` - `RemoteExecServerConnectArgs` -- protocol structs `InitializeParams` and `InitializeResponse` +- protocol request/response structs for process and filesystem RPCs - `DEFAULT_LISTEN_URL` and `ExecServerListenUrlParseError` -- `run_main_with_listen_url()` -- `run_main()` for embedding the websocket server in a binary +- `ExecServerRuntimePaths` +- `run_main()` for embedding the websocket server + +Callers must pass `ExecServerRuntimePaths` to `run_main()`. The top-level +`codex exec-server` command builds these paths from the `codex` arg0 dispatch +state. ## Example session @@ -258,23 +325,24 @@ Initialize: Start a process: ```json -{"id":2,"method":"command/exec","params":{"processId":"proc-1","argv":["bash","-lc","printf 'ready\\n'; while IFS= read -r line; do printf 'echo:%s\\n' \"$line\"; done"],"cwd":"/tmp","env":{"PATH":"/usr/bin:/bin"},"tty":true,"outputBytesCap":4096,"arg0":null}} -{"id":2,"result":{"processId":"proc-1","running":true,"exitCode":null,"stdout":null,"stderr":null}} -{"method":"command/exec/outputDelta","params":{"processId":"proc-1","stream":"stdout","chunk":"cmVhZHkK"}} +{"id":2,"method":"process/start","params":{"processId":"proc-1","argv":["bash","-lc","printf 'ready\\n'; while IFS= read -r line; do printf 'echo:%s\\n' \"$line\"; done"],"cwd":"/tmp","env":{"PATH":"/usr/bin:/bin"},"tty":true,"arg0":null}} +{"id":2,"result":{"processId":"proc-1"}} +{"method":"process/output","params":{"processId":"proc-1","seq":1,"stream":"stdout","chunk":"cmVhZHkK"}} ``` Write to the process: ```json -{"id":3,"method":"command/exec/write","params":{"processId":"proc-1","chunk":"aGVsbG8K"}} -{"id":3,"result":{"accepted":true}} -{"method":"command/exec/outputDelta","params":{"processId":"proc-1","stream":"stdout","chunk":"ZWNobzpoZWxsbwo="}} +{"id":3,"method":"process/write","params":{"processId":"proc-1","chunk":"aGVsbG8K"}} +{"id":3,"result":{"status":"accepted"}} +{"method":"process/output","params":{"processId":"proc-1","seq":2,"stream":"stdout","chunk":"ZWNobzpoZWxsbwo="}} ``` Terminate it: ```json -{"id":4,"method":"command/exec/terminate","params":{"processId":"proc-1"}} +{"id":4,"method":"process/terminate","params":{"processId":"proc-1"}} {"id":4,"result":{"running":true}} -{"method":"command/exec/exited","params":{"processId":"proc-1","exitCode":0}} +{"method":"process/exited","params":{"processId":"proc-1","seq":3,"exitCode":0}} +{"method":"process/closed","params":{"processId":"proc-1"}} ``` diff --git a/codex-rs/exec-server/src/bin/codex-exec-server.rs b/codex-rs/exec-server/src/bin/codex-exec-server.rs deleted file mode 100644 index 82fa9ec00..000000000 --- a/codex-rs/exec-server/src/bin/codex-exec-server.rs +++ /dev/null @@ -1,18 +0,0 @@ -use clap::Parser; - -#[derive(Debug, Parser)] -struct ExecServerArgs { - /// Transport endpoint URL. Supported values: `ws://IP:PORT` (default). - #[arg( - long = "listen", - value_name = "URL", - default_value = codex_exec_server::DEFAULT_LISTEN_URL - )] - listen: String, -} - -#[tokio::main] -async fn main() -> Result<(), Box> { - let args = ExecServerArgs::parse(); - codex_exec_server::run_main_with_listen_url(&args.listen).await -} diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 00b7fbb0c..a118468f5 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -4,6 +4,7 @@ use tokio::sync::OnceCell; use crate::ExecServerClient; use crate::ExecServerError; +use crate::ExecServerRuntimePaths; use crate::RemoteExecServerConnectArgs; use crate::file_system::ExecutorFileSystem; use crate::local_file_system::LocalFileSystem; @@ -21,6 +22,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; #[derive(Debug)] pub struct EnvironmentManager { exec_server_url: Option, + local_runtime_paths: Option, disabled: bool, current_environment: OnceCell>>, } @@ -34,9 +36,19 @@ impl Default for EnvironmentManager { impl EnvironmentManager { /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value. pub fn new(exec_server_url: Option) -> Self { + Self::new_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None) + } + + /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local + /// runtime paths used when creating local filesystem helpers. + pub fn new_with_runtime_paths( + exec_server_url: Option, + local_runtime_paths: Option, + ) -> Self { let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); Self { exec_server_url, + local_runtime_paths, disabled, current_environment: OnceCell::new(), } @@ -44,7 +56,18 @@ impl EnvironmentManager { /// Builds a manager from process environment variables. pub fn from_env() -> Self { - Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok()) + Self::from_env_with_runtime_paths(/*local_runtime_paths*/ None) + } + + /// Builds a manager from process environment variables and local runtime + /// paths used when creating local filesystem helpers. + pub fn from_env_with_runtime_paths( + local_runtime_paths: Option, + ) -> Self { + Self::new_with_runtime_paths( + std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + local_runtime_paths, + ) } /// Builds a manager from the currently selected environment, or from the @@ -53,11 +76,13 @@ impl EnvironmentManager { match environment { Some(environment) => Self { exec_server_url: environment.exec_server_url().map(str::to_owned), + local_runtime_paths: environment.local_runtime_paths().cloned(), disabled: false, current_environment: OnceCell::new(), }, None => Self { exec_server_url: None, + local_runtime_paths: None, disabled: true, current_environment: OnceCell::new(), }, @@ -82,7 +107,11 @@ impl EnvironmentManager { Ok(None) } else { Ok(Some(Arc::new( - Environment::create(self.exec_server_url.clone()).await?, + Environment::create_with_runtime_paths( + self.exec_server_url.clone(), + self.local_runtime_paths.clone(), + ) + .await?, ))) } }) @@ -101,6 +130,7 @@ pub struct Environment { exec_server_url: Option, remote_exec_server_client: Option, exec_backend: Arc, + local_runtime_paths: Option, } impl Default for Environment { @@ -109,6 +139,7 @@ impl Default for Environment { exec_server_url: None, remote_exec_server_client: None, exec_backend: Arc::new(LocalProcess::default()), + local_runtime_paths: None, } } } @@ -124,6 +155,15 @@ impl std::fmt::Debug for Environment { impl Environment { /// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value. pub async fn create(exec_server_url: Option) -> Result { + Self::create_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None).await + } + + /// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value and + /// local runtime paths used when creating local filesystem helpers. + pub async fn create_with_runtime_paths( + exec_server_url: Option, + local_runtime_paths: Option, + ) -> Result { let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); if disabled { return Err(ExecServerError::Protocol( @@ -157,6 +197,7 @@ impl Environment { exec_server_url, remote_exec_server_client, exec_backend, + local_runtime_paths, }) } @@ -169,6 +210,10 @@ impl Environment { self.exec_server_url.as_deref() } + pub fn local_runtime_paths(&self) -> Option<&ExecServerRuntimePaths> { + self.local_runtime_paths.as_ref() + } + pub fn get_exec_backend(&self) -> Arc { Arc::clone(&self.exec_backend) } @@ -176,7 +221,10 @@ impl Environment { pub fn get_filesystem(&self) -> Arc { match self.remote_exec_server_client.clone() { Some(client) => Arc::new(RemoteFileSystem::new(client)), - None => Arc::new(LocalFileSystem), + None => match self.local_runtime_paths.clone() { + Some(runtime_paths) => Arc::new(LocalFileSystem::with_runtime_paths(runtime_paths)), + None => Arc::new(LocalFileSystem::unsandboxed()), + }, } } } @@ -194,6 +242,7 @@ mod tests { use super::Environment; use super::EnvironmentManager; + use crate::ExecServerRuntimePaths; use crate::ProcessId; use pretty_assertions::assert_eq; @@ -246,6 +295,31 @@ mod tests { assert!(Arc::ptr_eq(&first, &second)); } + #[tokio::test] + async fn environment_manager_carries_local_runtime_paths() { + let runtime_paths = ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let manager = EnvironmentManager::new_with_runtime_paths( + /*exec_server_url*/ None, + Some(runtime_paths.clone()), + ); + + let environment = manager + .current() + .await + .expect("get current environment") + .expect("local environment"); + + assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); + assert_eq!( + EnvironmentManager::from_environment(Some(&environment)).local_runtime_paths, + Some(runtime_paths) + ); + } + #[tokio::test] async fn disabled_environment_manager_has_no_current_environment() { let manager = EnvironmentManager::new(Some("none".to_string())); diff --git a/codex-rs/exec-server/src/file_system.rs b/codex-rs/exec-server/src/file_system.rs index 8e68e1ab0..5786082e4 100644 --- a/codex-rs/exec-server/src/file_system.rs +++ b/codex-rs/exec-server/src/file_system.rs @@ -1,4 +1,6 @@ use async_trait::async_trait; +use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use tokio::io; @@ -34,86 +36,95 @@ pub struct ReadDirectoryEntry { pub is_file: bool, } +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FileSystemSandboxContext { + pub sandbox_policy: SandboxPolicy, + pub windows_sandbox_level: WindowsSandboxLevel, + #[serde(default)] + pub windows_sandbox_private_desktop: bool, + #[serde(default)] + pub use_legacy_landlock: bool, + pub additional_permissions: Option, +} + +impl FileSystemSandboxContext { + pub fn new(sandbox_policy: SandboxPolicy) -> Self { + Self { + sandbox_policy, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + windows_sandbox_private_desktop: false, + use_legacy_landlock: false, + additional_permissions: None, + } + } + + pub fn should_run_in_sandbox(&self) -> bool { + matches!( + self.sandbox_policy, + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } + ) + } +} + pub type FileSystemResult = io::Result; #[async_trait] pub trait ExecutorFileSystem: Send + Sync { - async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult>; + async fn read_file( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult>; /// Reads a file and decodes it as UTF-8 text. - async fn read_file_text(&self, path: &AbsolutePathBuf) -> FileSystemResult { - let bytes = self.read_file(path).await?; + async fn read_file_text( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + let bytes = self.read_file(path, sandbox).await?; String::from_utf8(bytes).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err)) } - async fn read_file_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult>; - - async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec) -> FileSystemResult<()>; - - async fn write_file_with_sandbox_policy( + async fn write_file( &self, path: &AbsolutePathBuf, contents: Vec, - sandbox_policy: Option<&SandboxPolicy>, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; async fn create_directory( - &self, - path: &AbsolutePathBuf, - options: CreateDirectoryOptions, - ) -> FileSystemResult<()>; - - async fn create_directory_with_sandbox_policy( &self, path: &AbsolutePathBuf, create_directory_options: CreateDirectoryOptions, - sandbox_policy: Option<&SandboxPolicy>, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; - async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult; - - async fn get_metadata_with_sandbox_policy( + async fn get_metadata( &self, path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult; async fn read_directory( &self, path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult>; - async fn read_directory_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult>; - - async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()>; - - async fn remove_with_sandbox_policy( + async fn remove( &self, path: &AbsolutePathBuf, remove_options: RemoveOptions, - sandbox_policy: Option<&SandboxPolicy>, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; async fn copy( - &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, - options: CopyOptions, - ) -> FileSystemResult<()>; - - async fn copy_with_sandbox_policy( &self, source_path: &AbsolutePathBuf, destination_path: &AbsolutePathBuf, copy_options: CopyOptions, - sandbox_policy: Option<&SandboxPolicy>, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; } diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs new file mode 100644 index 000000000..b4f50c75a --- /dev/null +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -0,0 +1,299 @@ +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD; +use codex_app_server_protocol::JSONRPCErrorError; +use serde::Deserialize; +use serde::Serialize; +use tokio::io; + +use crate::CopyOptions; +use crate::CreateDirectoryOptions; +use crate::ExecutorFileSystem; +use crate::RemoveOptions; +use crate::local_file_system::DirectFileSystem; +use crate::protocol::FS_COPY_METHOD; +use crate::protocol::FS_CREATE_DIRECTORY_METHOD; +use crate::protocol::FS_GET_METADATA_METHOD; +use crate::protocol::FS_READ_DIRECTORY_METHOD; +use crate::protocol::FS_READ_FILE_METHOD; +use crate::protocol::FS_REMOVE_METHOD; +use crate::protocol::FS_WRITE_FILE_METHOD; +use crate::protocol::FsCopyParams; +use crate::protocol::FsCopyResponse; +use crate::protocol::FsCreateDirectoryParams; +use crate::protocol::FsCreateDirectoryResponse; +use crate::protocol::FsGetMetadataParams; +use crate::protocol::FsGetMetadataResponse; +use crate::protocol::FsReadDirectoryEntry; +use crate::protocol::FsReadDirectoryParams; +use crate::protocol::FsReadDirectoryResponse; +use crate::protocol::FsReadFileParams; +use crate::protocol::FsReadFileResponse; +use crate::protocol::FsRemoveParams; +use crate::protocol::FsRemoveResponse; +use crate::protocol::FsWriteFileParams; +use crate::protocol::FsWriteFileResponse; +use crate::rpc::internal_error; +use crate::rpc::invalid_request; +use crate::rpc::not_found; + +pub const CODEX_FS_HELPER_ARG1: &str = "--codex-run-as-fs-helper"; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "operation", content = "params")] +pub(crate) enum FsHelperRequest { + #[serde(rename = "fs/readFile")] + ReadFile(FsReadFileParams), + #[serde(rename = "fs/writeFile")] + WriteFile(FsWriteFileParams), + #[serde(rename = "fs/createDirectory")] + CreateDirectory(FsCreateDirectoryParams), + #[serde(rename = "fs/getMetadata")] + GetMetadata(FsGetMetadataParams), + #[serde(rename = "fs/readDirectory")] + ReadDirectory(FsReadDirectoryParams), + #[serde(rename = "fs/remove")] + Remove(FsRemoveParams), + #[serde(rename = "fs/copy")] + Copy(FsCopyParams), +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(tag = "status", content = "payload", rename_all = "camelCase")] +pub(crate) enum FsHelperResponse { + Ok(FsHelperPayload), + Error(JSONRPCErrorError), +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "operation", content = "response")] +pub(crate) enum FsHelperPayload { + #[serde(rename = "fs/readFile")] + ReadFile(FsReadFileResponse), + #[serde(rename = "fs/writeFile")] + WriteFile(FsWriteFileResponse), + #[serde(rename = "fs/createDirectory")] + CreateDirectory(FsCreateDirectoryResponse), + #[serde(rename = "fs/getMetadata")] + GetMetadata(FsGetMetadataResponse), + #[serde(rename = "fs/readDirectory")] + ReadDirectory(FsReadDirectoryResponse), + #[serde(rename = "fs/remove")] + Remove(FsRemoveResponse), + #[serde(rename = "fs/copy")] + Copy(FsCopyResponse), +} + +impl FsHelperPayload { + fn operation(&self) -> &'static str { + match self { + Self::ReadFile(_) => FS_READ_FILE_METHOD, + Self::WriteFile(_) => FS_WRITE_FILE_METHOD, + Self::CreateDirectory(_) => FS_CREATE_DIRECTORY_METHOD, + Self::GetMetadata(_) => FS_GET_METADATA_METHOD, + Self::ReadDirectory(_) => FS_READ_DIRECTORY_METHOD, + Self::Remove(_) => FS_REMOVE_METHOD, + Self::Copy(_) => FS_COPY_METHOD, + } + } + + pub(crate) fn expect_read_file(self) -> Result { + match self { + Self::ReadFile(response) => Ok(response), + other => Err(unexpected_response(FS_READ_FILE_METHOD, other.operation())), + } + } + + pub(crate) fn expect_write_file(self) -> Result { + match self { + Self::WriteFile(response) => Ok(response), + other => Err(unexpected_response(FS_WRITE_FILE_METHOD, other.operation())), + } + } + + pub(crate) fn expect_create_directory( + self, + ) -> Result { + match self { + Self::CreateDirectory(response) => Ok(response), + other => Err(unexpected_response( + FS_CREATE_DIRECTORY_METHOD, + other.operation(), + )), + } + } + + pub(crate) fn expect_get_metadata(self) -> Result { + match self { + Self::GetMetadata(response) => Ok(response), + other => Err(unexpected_response( + FS_GET_METADATA_METHOD, + other.operation(), + )), + } + } + + pub(crate) fn expect_read_directory( + self, + ) -> Result { + match self { + Self::ReadDirectory(response) => Ok(response), + other => Err(unexpected_response( + FS_READ_DIRECTORY_METHOD, + other.operation(), + )), + } + } + + pub(crate) fn expect_remove(self) -> Result { + match self { + Self::Remove(response) => Ok(response), + other => Err(unexpected_response(FS_REMOVE_METHOD, other.operation())), + } + } + + pub(crate) fn expect_copy(self) -> Result { + match self { + Self::Copy(response) => Ok(response), + other => Err(unexpected_response(FS_COPY_METHOD, other.operation())), + } + } +} + +fn unexpected_response(expected: &str, actual: &str) -> JSONRPCErrorError { + internal_error(format!( + "unexpected fs sandbox helper response: expected {expected}, got {actual}" + )) +} + +pub(crate) async fn run_direct_request( + request: FsHelperRequest, +) -> Result { + let file_system = DirectFileSystem; + match request { + FsHelperRequest::ReadFile(params) => { + let data = file_system + .read_file(¶ms.path, /*sandbox*/ None) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::ReadFile(FsReadFileResponse { + data_base64: STANDARD.encode(data), + })) + } + FsHelperRequest::WriteFile(params) => { + let bytes = STANDARD.decode(params.data_base64).map_err(|err| { + invalid_request(format!( + "{FS_WRITE_FILE_METHOD} requires valid base64 dataBase64: {err}" + )) + })?; + file_system + .write_file(¶ms.path, bytes, /*sandbox*/ None) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::WriteFile(FsWriteFileResponse {})) + } + FsHelperRequest::CreateDirectory(params) => { + file_system + .create_directory( + ¶ms.path, + CreateDirectoryOptions { + recursive: params.recursive.unwrap_or(true), + }, + /*sandbox*/ None, + ) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::CreateDirectory( + FsCreateDirectoryResponse {}, + )) + } + FsHelperRequest::GetMetadata(params) => { + let metadata = file_system + .get_metadata(¶ms.path, /*sandbox*/ None) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::GetMetadata(FsGetMetadataResponse { + is_directory: metadata.is_directory, + is_file: metadata.is_file, + created_at_ms: metadata.created_at_ms, + modified_at_ms: metadata.modified_at_ms, + })) + } + FsHelperRequest::ReadDirectory(params) => { + let entries = file_system + .read_directory(¶ms.path, /*sandbox*/ None) + .await + .map_err(map_fs_error)? + .into_iter() + .map(|entry| FsReadDirectoryEntry { + file_name: entry.file_name, + is_directory: entry.is_directory, + is_file: entry.is_file, + }) + .collect(); + Ok(FsHelperPayload::ReadDirectory(FsReadDirectoryResponse { + entries, + })) + } + FsHelperRequest::Remove(params) => { + file_system + .remove( + ¶ms.path, + RemoveOptions { + recursive: params.recursive.unwrap_or(true), + force: params.force.unwrap_or(true), + }, + /*sandbox*/ None, + ) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::Remove(FsRemoveResponse {})) + } + FsHelperRequest::Copy(params) => { + file_system + .copy( + ¶ms.source_path, + ¶ms.destination_path, + CopyOptions { + recursive: params.recursive, + }, + /*sandbox*/ None, + ) + .await + .map_err(map_fs_error)?; + Ok(FsHelperPayload::Copy(FsCopyResponse {})) + } + } +} + +fn map_fs_error(err: io::Error) -> JSONRPCErrorError { + match err.kind() { + io::ErrorKind::NotFound => not_found(err.to_string()), + io::ErrorKind::InvalidInput | io::ErrorKind::PermissionDenied => { + invalid_request(err.to_string()) + } + _ => internal_error(err.to_string()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn helper_requests_use_fs_method_names() -> serde_json::Result<()> { + assert_eq!( + serde_json::to_value(FsHelperRequest::WriteFile(FsWriteFileParams { + path: std::env::current_dir() + .expect("cwd") + .join("file") + .as_path() + .try_into() + .expect("absolute path"), + data_base64: String::new(), + sandbox: None, + }))?["operation"], + FS_WRITE_FILE_METHOD, + ); + Ok(()) + } +} diff --git a/codex-rs/exec-server/src/fs_helper_main.rs b/codex-rs/exec-server/src/fs_helper_main.rs new file mode 100644 index 000000000..d9639dd1f --- /dev/null +++ b/codex-rs/exec-server/src/fs_helper_main.rs @@ -0,0 +1,45 @@ +use std::error::Error; + +use tokio::io; +use tokio::io::AsyncReadExt; +use tokio::io::AsyncWriteExt; + +use crate::fs_helper::FsHelperRequest; +use crate::fs_helper::FsHelperResponse; +use crate::fs_helper::run_direct_request; + +pub fn main() -> ! { + let exit_code = match tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + { + Ok(runtime) => match runtime.block_on(run_main()) { + Ok(()) => 0, + Err(err) => { + eprintln!("fs sandbox helper failed: {err}"); + 1 + } + }, + Err(err) => { + eprintln!("failed to start fs sandbox helper runtime: {err}"); + 1 + } + }; + std::process::exit(exit_code); +} + +async fn run_main() -> Result<(), Box> { + let mut input = Vec::new(); + io::stdin().read_to_end(&mut input).await?; + let request: FsHelperRequest = serde_json::from_slice(&input)?; + let response = match run_direct_request(request).await { + Ok(payload) => FsHelperResponse::Ok(payload), + Err(error) => FsHelperResponse::Error(error), + }; + let mut stdout = io::stdout(); + stdout + .write_all(serde_json::to_string(&response)?.as_bytes()) + .await?; + stdout.write_all(b"\n").await?; + Ok(()) +} diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs new file mode 100644 index 000000000..995bfbd91 --- /dev/null +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -0,0 +1,546 @@ +use std::collections::HashMap; +use std::path::PathBuf; + +use codex_app_server_protocol::JSONRPCErrorError; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::ReadOnlyAccess; +use codex_protocol::protocol::SandboxPolicy; +use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxExecRequest; +use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxTransformRequest; +use codex_sandboxing::SandboxablePreference; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::canonicalize_preserving_symlinks; +use tokio::io::AsyncWriteExt; +use tokio::process::Command; + +use crate::ExecServerRuntimePaths; +use crate::FileSystemSandboxContext; +use crate::fs_helper::CODEX_FS_HELPER_ARG1; +use crate::fs_helper::FsHelperPayload; +use crate::fs_helper::FsHelperRequest; +use crate::fs_helper::FsHelperResponse; +use crate::local_file_system::current_sandbox_cwd; +use crate::local_file_system::resolve_existing_path; +use crate::protocol::FsCopyParams; +use crate::protocol::FsCreateDirectoryParams; +use crate::protocol::FsGetMetadataParams; +use crate::protocol::FsReadDirectoryParams; +use crate::protocol::FsReadFileParams; +use crate::protocol::FsRemoveParams; +use crate::protocol::FsWriteFileParams; +use crate::rpc::internal_error; +use crate::rpc::invalid_request; + +#[derive(Clone, Debug)] +pub(crate) struct FileSystemSandboxRunner { + runtime_paths: ExecServerRuntimePaths, +} + +impl FileSystemSandboxRunner { + pub(crate) fn new(runtime_paths: ExecServerRuntimePaths) -> Self { + Self { runtime_paths } + } + + pub(crate) async fn run( + &self, + sandbox: &FileSystemSandboxContext, + request: FsHelperRequest, + ) -> Result { + let request_sandbox_policy = + normalize_sandbox_policy_root_aliases(sandbox.sandbox_policy.clone()); + let helper_sandbox_policy = normalize_sandbox_policy_root_aliases( + sandbox_policy_with_helper_runtime_defaults(&sandbox.sandbox_policy), + ); + let cwd = current_sandbox_cwd().map_err(io_error)?; + let cwd = AbsolutePathBuf::from_absolute_path(cwd.as_path()) + .map_err(|err| invalid_request(format!("current directory is not absolute: {err}")))?; + let request_file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &request_sandbox_policy, + cwd.as_path(), + ); + let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &helper_sandbox_policy, + cwd.as_path(), + ); + let request = resolve_request_paths(request, &request_file_system_policy, &cwd)?; + let network_policy = NetworkSandboxPolicy::Restricted; + let command = self.sandbox_exec_request( + &helper_sandbox_policy, + &file_system_policy, + network_policy, + &cwd, + sandbox, + )?; + let request_json = serde_json::to_vec(&request).map_err(json_error)?; + run_command(command, request_json).await + } + + fn sandbox_exec_request( + &self, + sandbox_policy: &SandboxPolicy, + file_system_policy: &FileSystemSandboxPolicy, + network_policy: NetworkSandboxPolicy, + cwd: &AbsolutePathBuf, + sandbox_context: &FileSystemSandboxContext, + ) -> Result { + let helper = &self.runtime_paths.codex_self_exe; + let sandbox_manager = SandboxManager::new(); + let sandbox = sandbox_manager.select_initial( + file_system_policy, + network_policy, + SandboxablePreference::Auto, + sandbox_context.windows_sandbox_level, + /*has_managed_network_requirements*/ false, + ); + let command = SandboxCommand { + program: helper.as_path().as_os_str().to_owned(), + args: vec![CODEX_FS_HELPER_ARG1.to_string()], + cwd: cwd.clone(), + env: HashMap::new(), + additional_permissions: Some( + self.helper_permissions(sandbox_context.additional_permissions.as_ref()), + ), + }; + sandbox_manager + .transform(SandboxTransformRequest { + command, + policy: sandbox_policy, + file_system_policy, + network_policy, + sandbox, + enforce_managed_network: false, + network: None, + sandbox_policy_cwd: cwd.as_path(), + codex_linux_sandbox_exe: self.runtime_paths.codex_linux_sandbox_exe.as_deref(), + use_legacy_landlock: sandbox_context.use_legacy_landlock, + windows_sandbox_level: sandbox_context.windows_sandbox_level, + windows_sandbox_private_desktop: sandbox_context.windows_sandbox_private_desktop, + }) + .map_err(|err| invalid_request(format!("failed to prepare fs sandbox: {err}"))) + } + + fn helper_permissions( + &self, + additional_permissions: Option<&PermissionProfile>, + ) -> PermissionProfile { + PermissionProfile { + network: None, + file_system: additional_permissions + .and_then(|permissions| permissions.file_system.clone()), + } + } +} + +fn resolve_request_paths( + request: FsHelperRequest, + file_system_policy: &FileSystemSandboxPolicy, + cwd: &AbsolutePathBuf, +) -> Result { + match request { + FsHelperRequest::ReadFile(FsReadFileParams { path, sandbox }) => { + let path = resolve_sandbox_path(&path, PreserveTerminalSymlink::No)?; + ensure_path_access(file_system_policy, cwd, &path, FileSystemAccessMode::Read)?; + Ok(FsHelperRequest::ReadFile(FsReadFileParams { + path, + sandbox, + })) + } + FsHelperRequest::WriteFile(FsWriteFileParams { + path, + data_base64, + sandbox, + }) => Ok(FsHelperRequest::WriteFile(FsWriteFileParams { + path: { + let path = resolve_sandbox_path(&path, PreserveTerminalSymlink::No)?; + ensure_path_access(file_system_policy, cwd, &path, FileSystemAccessMode::Write)?; + path + }, + data_base64, + sandbox, + })), + FsHelperRequest::CreateDirectory(FsCreateDirectoryParams { + path, + recursive, + sandbox, + }) => Ok(FsHelperRequest::CreateDirectory(FsCreateDirectoryParams { + path: { + let path = resolve_sandbox_path(&path, PreserveTerminalSymlink::No)?; + ensure_path_access(file_system_policy, cwd, &path, FileSystemAccessMode::Write)?; + path + }, + recursive, + sandbox, + })), + FsHelperRequest::GetMetadata(FsGetMetadataParams { path, sandbox }) => { + let path = resolve_sandbox_path(&path, PreserveTerminalSymlink::No)?; + ensure_path_access(file_system_policy, cwd, &path, FileSystemAccessMode::Read)?; + Ok(FsHelperRequest::GetMetadata(FsGetMetadataParams { + path, + sandbox, + })) + } + FsHelperRequest::ReadDirectory(FsReadDirectoryParams { path, sandbox }) => { + let path = resolve_sandbox_path(&path, PreserveTerminalSymlink::No)?; + ensure_path_access(file_system_policy, cwd, &path, FileSystemAccessMode::Read)?; + Ok(FsHelperRequest::ReadDirectory(FsReadDirectoryParams { + path, + sandbox, + })) + } + FsHelperRequest::Remove(FsRemoveParams { + path, + recursive, + force, + sandbox, + }) => Ok(FsHelperRequest::Remove(FsRemoveParams { + path: { + let path = resolve_sandbox_path(&path, PreserveTerminalSymlink::Yes)?; + ensure_path_access(file_system_policy, cwd, &path, FileSystemAccessMode::Write)?; + path + }, + recursive, + force, + sandbox, + })), + FsHelperRequest::Copy(FsCopyParams { + source_path, + destination_path, + recursive, + sandbox, + }) => Ok(FsHelperRequest::Copy(FsCopyParams { + source_path: { + let source_path = resolve_sandbox_path(&source_path, PreserveTerminalSymlink::Yes)?; + ensure_path_access( + file_system_policy, + cwd, + &source_path, + FileSystemAccessMode::Read, + )?; + source_path + }, + destination_path: { + let destination_path = + resolve_sandbox_path(&destination_path, PreserveTerminalSymlink::No)?; + ensure_path_access( + file_system_policy, + cwd, + &destination_path, + FileSystemAccessMode::Write, + )?; + destination_path + }, + recursive, + sandbox, + })), + } +} + +#[derive(Clone, Copy)] +enum PreserveTerminalSymlink { + Yes, + No, +} + +fn resolve_sandbox_path( + path: &AbsolutePathBuf, + preserve_terminal_symlink: PreserveTerminalSymlink, +) -> Result { + if matches!(preserve_terminal_symlink, PreserveTerminalSymlink::Yes) + && std::fs::symlink_metadata(path.as_path()) + .map(|metadata| metadata.file_type().is_symlink()) + .unwrap_or(false) + { + return Ok(normalize_top_level_alias(path.clone())); + } + + let resolved = resolve_existing_path(path.as_path()).map_err(io_error)?; + absolute_path(resolved) +} + +fn normalize_sandbox_policy_root_aliases(sandbox_policy: SandboxPolicy) -> SandboxPolicy { + let mut sandbox_policy = sandbox_policy; + match &mut sandbox_policy { + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { readable_roots, .. }, + .. + } => { + normalize_root_aliases(readable_roots); + } + SandboxPolicy::WorkspaceWrite { + writable_roots, + read_only_access, + .. + } => { + normalize_root_aliases(writable_roots); + if let ReadOnlyAccess::Restricted { readable_roots, .. } = read_only_access { + normalize_root_aliases(readable_roots); + } + } + _ => {} + } + sandbox_policy +} + +fn normalize_root_aliases(paths: &mut Vec) { + for path in paths { + *path = normalize_top_level_alias(path.clone()); + } +} + +fn normalize_top_level_alias(path: AbsolutePathBuf) -> AbsolutePathBuf { + let raw_path = path.to_path_buf(); + for ancestor in raw_path.ancestors() { + if std::fs::symlink_metadata(ancestor).is_err() { + continue; + } + let Ok(normalized_ancestor) = canonicalize_preserving_symlinks(ancestor) else { + continue; + }; + if normalized_ancestor == ancestor { + continue; + } + let Ok(suffix) = raw_path.strip_prefix(ancestor) else { + continue; + }; + if let Ok(normalized_path) = + AbsolutePathBuf::from_absolute_path(normalized_ancestor.join(suffix)) + { + return normalized_path; + } + } + path +} + +fn absolute_path(path: PathBuf) -> Result { + AbsolutePathBuf::from_absolute_path(path.as_path()) + .map_err(|err| invalid_request(format!("resolved sandbox path is not absolute: {err}"))) +} + +fn ensure_path_access( + file_system_policy: &FileSystemSandboxPolicy, + cwd: &AbsolutePathBuf, + path: &AbsolutePathBuf, + required_access: FileSystemAccessMode, +) -> Result<(), JSONRPCErrorError> { + let actual_access = file_system_policy.resolve_access_with_cwd(path.as_path(), cwd.as_path()); + let permitted = match required_access { + FileSystemAccessMode::Read => actual_access.can_read(), + FileSystemAccessMode::Write => actual_access.can_write(), + FileSystemAccessMode::None => true, + }; + if permitted { + return Ok(()); + } + + Err(invalid_request(format!( + "{} is not permitted by filesystem sandbox", + path.display() + ))) +} + +async fn run_command( + command: SandboxExecRequest, + request_json: Vec, +) -> Result { + let mut child = spawn_command(command)?; + let mut stdin = child + .stdin + .take() + .ok_or_else(|| internal_error("failed to open fs sandbox helper stdin".to_string()))?; + stdin.write_all(&request_json).await.map_err(io_error)?; + stdin.shutdown().await.map_err(io_error)?; + drop(stdin); + + let output = child.wait_with_output().await.map_err(io_error)?; + if !output.status.success() { + return Err(internal_error(format!( + "fs sandbox helper failed with status {status}: {stderr}", + status = output.status, + stderr = String::from_utf8_lossy(&output.stderr).trim() + ))); + } + let response: FsHelperResponse = serde_json::from_slice(&output.stdout).map_err(json_error)?; + match response { + FsHelperResponse::Ok(payload) => Ok(payload), + FsHelperResponse::Error(error) => Err(error), + } +} + +fn spawn_command( + SandboxExecRequest { + command: argv, + cwd, + env, + arg0, + .. + }: SandboxExecRequest, +) -> Result { + let Some((program, args)) = argv.split_first() else { + return Err(invalid_request("fs sandbox command was empty".to_string())); + }; + let mut command = Command::new(program); + #[cfg(unix)] + if let Some(arg0) = arg0 { + command.arg0(arg0); + } + #[cfg(not(unix))] + let _ = arg0; + command.args(args); + command.current_dir(cwd.as_path()); + command.env_clear(); + command.envs(env); + command.stdin(std::process::Stdio::piped()); + command.stdout(std::process::Stdio::piped()); + command.stderr(std::process::Stdio::piped()); + command.spawn().map_err(io_error) +} + +fn sandbox_policy_with_helper_runtime_defaults(sandbox_policy: &SandboxPolicy) -> SandboxPolicy { + let mut sandbox_policy = sandbox_policy.clone(); + match &mut sandbox_policy { + SandboxPolicy::ReadOnly { access, .. } => enable_platform_defaults(access), + SandboxPolicy::WorkspaceWrite { + read_only_access, .. + } => enable_platform_defaults(read_only_access), + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {} + } + sandbox_policy +} + +fn enable_platform_defaults(access: &mut ReadOnlyAccess) { + if let ReadOnlyAccess::Restricted { + include_platform_defaults, + .. + } = access + { + *include_platform_defaults = true; + } +} + +fn io_error(err: std::io::Error) -> JSONRPCErrorError { + internal_error(err.to_string()) +} + +fn json_error(err: serde_json::Error) -> JSONRPCErrorError { + internal_error(format!( + "failed to encode or decode fs sandbox helper message: {err}" + )) +} + +#[cfg(test)] +mod tests { + use codex_protocol::models::FileSystemPermissions; + use codex_protocol::models::NetworkPermissions; + use codex_protocol::models::PermissionProfile; + use codex_protocol::protocol::ReadOnlyAccess; + use codex_protocol::protocol::SandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + + use crate::ExecServerRuntimePaths; + + use super::FileSystemSandboxRunner; + use super::sandbox_policy_with_helper_runtime_defaults; + + #[test] + fn helper_sandbox_policy_enables_platform_defaults_for_read_only_access() { + let sandbox_policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + }; + + let updated = sandbox_policy_with_helper_runtime_defaults(&sandbox_policy); + + assert_eq!( + updated, + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: false, + } + ); + } + + #[test] + fn helper_sandbox_policy_enables_platform_defaults_for_workspace_read_access() { + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let updated = sandbox_policy_with_helper_runtime_defaults(&sandbox_policy); + + assert_eq!( + updated, + SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + } + ); + } + + #[test] + fn helper_permissions_strip_network_grants() { + let codex_self_exe = std::env::current_exe().expect("current exe"); + let runtime_paths = ExecServerRuntimePaths::new( + codex_self_exe.clone(), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let runner = FileSystemSandboxRunner::new(runtime_paths); + let readable = AbsolutePathBuf::from_absolute_path( + codex_self_exe.parent().expect("current exe parent"), + ) + .expect("absolute readable path"); + let writable = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + .expect("absolute writable path"); + + let permissions = runner.helper_permissions(Some(&PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + file_system: Some(FileSystemPermissions { + read: Some(vec![readable.clone()]), + write: Some(vec![writable.clone()]), + }), + })); + + assert_eq!(permissions.network, None); + assert_eq!( + permissions + .file_system + .as_ref() + .and_then(|fs| fs.write.clone()), + Some(vec![writable]) + ); + assert_eq!( + permissions + .file_system + .as_ref() + .and_then(|fs| fs.read.clone()), + Some(vec![readable]) + ); + } +} diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index eccb9c91b..81bb8a6cd 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -3,6 +3,9 @@ mod client_api; mod connection; mod environment; mod file_system; +mod fs_helper; +mod fs_helper_main; +mod fs_sandbox; mod local_file_system; mod local_process; mod process; @@ -11,6 +14,8 @@ mod protocol; mod remote_file_system; mod remote_process; mod rpc; +mod runtime_paths; +mod sandboxed_file_system; mod server; pub use client::ExecServerClient; @@ -25,9 +30,13 @@ pub use file_system::CreateDirectoryOptions; pub use file_system::ExecutorFileSystem; pub use file_system::FileMetadata; pub use file_system::FileSystemResult; +pub use file_system::FileSystemSandboxContext; pub use file_system::ReadDirectoryEntry; pub use file_system::RemoveOptions; +pub use fs_helper::CODEX_FS_HELPER_ARG1; +pub use fs_helper_main::main as run_fs_helper_main; pub use local_file_system::LOCAL_FS; +pub use local_file_system::LocalFileSystem; pub use process::ExecBackend; pub use process::ExecProcess; pub use process::StartedExecProcess; @@ -62,7 +71,7 @@ pub use protocol::TerminateResponse; pub use protocol::WriteParams; pub use protocol::WriteResponse; pub use protocol::WriteStatus; +pub use runtime_paths::ExecServerRuntimePaths; pub use server::DEFAULT_LISTEN_URL; pub use server::ExecServerListenUrlParseError; pub use server::run_main; -pub use server::run_main_with_listen_url; diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 10df5faa3..1c2b0f79e 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -1,7 +1,4 @@ use async_trait::async_trait; -use codex_protocol::permissions::FileSystemPath; -use codex_protocol::permissions::FileSystemSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use std::path::Path; use std::path::PathBuf; @@ -13,23 +10,240 @@ use tokio::io; use crate::CopyOptions; use crate::CreateDirectoryOptions; +use crate::ExecServerRuntimePaths; use crate::ExecutorFileSystem; use crate::FileMetadata; use crate::FileSystemResult; +use crate::FileSystemSandboxContext; use crate::ReadDirectoryEntry; use crate::RemoveOptions; +use crate::sandboxed_file_system::SandboxedFileSystem; const MAX_READ_FILE_BYTES: u64 = 512 * 1024 * 1024; pub static LOCAL_FS: LazyLock> = - LazyLock::new(|| -> Arc { Arc::new(LocalFileSystem) }); + LazyLock::new(|| -> Arc { Arc::new(LocalFileSystem::unsandboxed()) }); #[derive(Clone, Default)] -pub(crate) struct LocalFileSystem; +pub(crate) struct DirectFileSystem; + +#[derive(Clone, Default)] +pub(crate) struct UnsandboxedFileSystem { + file_system: DirectFileSystem, +} + +#[derive(Clone, Default)] +pub struct LocalFileSystem { + unsandboxed: UnsandboxedFileSystem, + sandboxed: Option, +} + +impl LocalFileSystem { + pub fn unsandboxed() -> Self { + Self { + unsandboxed: UnsandboxedFileSystem::default(), + sandboxed: None, + } + } + + pub fn with_runtime_paths(runtime_paths: ExecServerRuntimePaths) -> Self { + Self { + unsandboxed: UnsandboxedFileSystem::default(), + sandboxed: Some(SandboxedFileSystem::new(runtime_paths)), + } + } + + fn sandboxed(&self) -> io::Result<&SandboxedFileSystem> { + self.sandboxed.as_ref().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "sandboxed filesystem operations require configured runtime paths", + ) + }) + } + + fn file_system_for<'a>( + &'a self, + sandbox: Option<&'a FileSystemSandboxContext>, + ) -> io::Result<( + &'a dyn ExecutorFileSystem, + Option<&'a FileSystemSandboxContext>, + )> { + if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) { + Ok((self.sandboxed()?, sandbox)) + } else { + Ok((&self.unsandboxed, sandbox)) + } + } +} #[async_trait] impl ExecutorFileSystem for LocalFileSystem { - async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + async fn read_file( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.read_file(path, sandbox).await + } + + async fn write_file( + &self, + path: &AbsolutePathBuf, + contents: Vec, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.write_file(path, contents, sandbox).await + } + + async fn create_directory( + &self, + path: &AbsolutePathBuf, + options: CreateDirectoryOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.create_directory(path, options, sandbox).await + } + + async fn get_metadata( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.get_metadata(path, sandbox).await + } + + async fn read_directory( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.read_directory(path, sandbox).await + } + + async fn remove( + &self, + path: &AbsolutePathBuf, + options: RemoveOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system.remove(path, options, sandbox).await + } + + async fn copy( + &self, + source_path: &AbsolutePathBuf, + destination_path: &AbsolutePathBuf, + options: CopyOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let (file_system, sandbox) = self.file_system_for(sandbox)?; + file_system + .copy(source_path, destination_path, options, sandbox) + .await + } +} + +#[async_trait] +impl ExecutorFileSystem for UnsandboxedFileSystem { + async fn read_file( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + reject_platform_sandbox_context(sandbox)?; + self.file_system.read_file(path, /*sandbox*/ None).await + } + + async fn write_file( + &self, + path: &AbsolutePathBuf, + contents: Vec, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + reject_platform_sandbox_context(sandbox)?; + self.file_system + .write_file(path, contents, /*sandbox*/ None) + .await + } + + async fn create_directory( + &self, + path: &AbsolutePathBuf, + options: CreateDirectoryOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + reject_platform_sandbox_context(sandbox)?; + self.file_system + .create_directory(path, options, /*sandbox*/ None) + .await + } + + async fn get_metadata( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + reject_platform_sandbox_context(sandbox)?; + self.file_system.get_metadata(path, /*sandbox*/ None).await + } + + async fn read_directory( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + reject_platform_sandbox_context(sandbox)?; + self.file_system + .read_directory(path, /*sandbox*/ None) + .await + } + + async fn remove( + &self, + path: &AbsolutePathBuf, + options: RemoveOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + reject_platform_sandbox_context(sandbox)?; + self.file_system + .remove(path, options, /*sandbox*/ None) + .await + } + + async fn copy( + &self, + source_path: &AbsolutePathBuf, + destination_path: &AbsolutePathBuf, + options: CopyOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + reject_platform_sandbox_context(sandbox)?; + self.file_system + .copy( + source_path, + destination_path, + options, + /*sandbox*/ None, + ) + .await + } +} + +#[async_trait] +impl ExecutorFileSystem for DirectFileSystem { + async fn read_file( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + reject_sandbox_context(sandbox)?; let metadata = tokio::fs::metadata(path.as_path()).await?; if metadata.len() > MAX_READ_FILE_BYTES { return Err(io::Error::new( @@ -40,34 +254,23 @@ impl ExecutorFileSystem for LocalFileSystem { tokio::fs::read(path.as_path()).await } - async fn read_file_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult> { - enforce_read_access(path, sandbox_policy)?; - self.read_file(path).await - } - - async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec) -> FileSystemResult<()> { - tokio::fs::write(path.as_path(), contents).await - } - - async fn write_file_with_sandbox_policy( + async fn write_file( &self, path: &AbsolutePathBuf, contents: Vec, - sandbox_policy: Option<&SandboxPolicy>, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { - enforce_write_access(path, sandbox_policy)?; - self.write_file(path, contents).await + reject_sandbox_context(sandbox)?; + tokio::fs::write(path.as_path(), contents).await } async fn create_directory( &self, path: &AbsolutePathBuf, options: CreateDirectoryOptions, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { + reject_sandbox_context(sandbox)?; if options.recursive { tokio::fs::create_dir_all(path.as_path()).await?; } else { @@ -76,17 +279,12 @@ impl ExecutorFileSystem for LocalFileSystem { Ok(()) } - async fn create_directory_with_sandbox_policy( + async fn get_metadata( &self, path: &AbsolutePathBuf, - create_directory_options: CreateDirectoryOptions, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - enforce_write_access(path, sandbox_policy)?; - self.create_directory(path, create_directory_options).await - } - - async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult { + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + reject_sandbox_context(sandbox)?; let metadata = tokio::fs::metadata(path.as_path()).await?; Ok(FileMetadata { is_directory: metadata.is_dir(), @@ -96,19 +294,12 @@ impl ExecutorFileSystem for LocalFileSystem { }) } - async fn get_metadata_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult { - enforce_read_access(path, sandbox_policy)?; - self.get_metadata(path).await - } - async fn read_directory( &self, path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { + reject_sandbox_context(sandbox)?; let mut entries = Vec::new(); let mut read_dir = tokio::fs::read_dir(path.as_path()).await?; while let Some(entry) = read_dir.next_entry().await? { @@ -122,16 +313,13 @@ impl ExecutorFileSystem for LocalFileSystem { Ok(entries) } - async fn read_directory_with_sandbox_policy( + async fn remove( &self, path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult> { - enforce_read_access(path, sandbox_policy)?; - self.read_directory(path).await - } - - async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()> { + options: RemoveOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + reject_sandbox_context(sandbox)?; match tokio::fs::symlink_metadata(path.as_path()).await { Ok(metadata) => { let file_type = metadata.file_type(); @@ -151,22 +339,14 @@ impl ExecutorFileSystem for LocalFileSystem { } } - async fn remove_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - remove_options: RemoveOptions, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - enforce_write_access_preserving_leaf(path, sandbox_policy)?; - self.remove(path, remove_options).await - } - async fn copy( &self, source_path: &AbsolutePathBuf, destination_path: &AbsolutePathBuf, options: CopyOptions, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { + reject_sandbox_context(sandbox)?; let source_path = source_path.to_path_buf(); let destination_path = destination_path.to_path_buf(); tokio::task::spawn_blocking(move || -> FileSystemResult<()> { @@ -211,164 +391,26 @@ impl ExecutorFileSystem for LocalFileSystem { .await .map_err(|err| io::Error::other(format!("filesystem task failed: {err}")))? } - - async fn copy_with_sandbox_policy( - &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, - copy_options: CopyOptions, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - enforce_copy_source_read_access(source_path, sandbox_policy)?; - enforce_write_access(destination_path, sandbox_policy)?; - self.copy(source_path, destination_path, copy_options).await - } } -fn enforce_read_access( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, -) -> FileSystemResult<()> { - enforce_access_for_current_dir( - path, - sandbox_policy, - FileSystemSandboxPolicy::can_read_path_with_cwd, - "read", - AccessPathMode::ResolveAll, - ) -} - -fn enforce_write_access( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, -) -> FileSystemResult<()> { - enforce_access_for_current_dir( - path, - sandbox_policy, - FileSystemSandboxPolicy::can_write_path_with_cwd, - "write", - AccessPathMode::ResolveAll, - ) -} - -fn enforce_write_access_preserving_leaf( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, -) -> FileSystemResult<()> { - enforce_access_for_current_dir( - path, - sandbox_policy, - FileSystemSandboxPolicy::can_write_path_with_cwd, - "write", - AccessPathMode::PreserveLeaf, - ) -} - -fn enforce_copy_source_read_access( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, -) -> FileSystemResult<()> { - let path_mode = match std::fs::symlink_metadata(path.as_path()) { - Ok(metadata) if metadata.file_type().is_symlink() => AccessPathMode::PreserveLeaf, - _ => AccessPathMode::ResolveAll, - }; - enforce_access_for_current_dir( - path, - sandbox_policy, - FileSystemSandboxPolicy::can_read_path_with_cwd, - "read", - path_mode, - ) -} - -#[cfg(all(test, unix))] -fn enforce_read_access_for_cwd( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - sandbox_cwd: &AbsolutePathBuf, -) -> FileSystemResult<()> { - enforce_access_for_cwd( - path, - sandbox_policy, - sandbox_cwd, - FileSystemSandboxPolicy::can_read_path_with_cwd, - "read", - AccessPathMode::ResolveAll, - ) -} - -fn enforce_access_for_current_dir( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool, - access_kind: &str, - path_mode: AccessPathMode, -) -> FileSystemResult<()> { - let Some(sandbox_policy) = sandbox_policy else { - return Ok(()); - }; - let cwd = current_sandbox_cwd()?; - enforce_access( - path, - sandbox_policy, - cwd.as_path(), - is_allowed, - access_kind, - path_mode, - ) -} - -#[cfg(all(test, unix))] -fn enforce_access_for_cwd( - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - sandbox_cwd: &AbsolutePathBuf, - is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool, - access_kind: &str, - path_mode: AccessPathMode, -) -> FileSystemResult<()> { - let Some(sandbox_policy) = sandbox_policy else { - return Ok(()); - }; - let cwd = resolve_existing_path(sandbox_cwd.as_path())?; - enforce_access( - path, - sandbox_policy, - cwd.as_path(), - is_allowed, - access_kind, - path_mode, - ) -} - -fn enforce_access( - path: &AbsolutePathBuf, - sandbox_policy: &SandboxPolicy, - sandbox_cwd: &Path, - is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool, - access_kind: &str, - path_mode: AccessPathMode, -) -> FileSystemResult<()> { - let resolved_path = resolve_path_for_access_check(path.as_path(), path_mode)?; - let file_system_policy = - canonicalize_file_system_policy_paths(FileSystemSandboxPolicy::from(sandbox_policy))?; - if is_allowed(&file_system_policy, resolved_path.as_path(), sandbox_cwd) { - Ok(()) - } else { - Err(io::Error::new( +fn reject_sandbox_context(sandbox: Option<&FileSystemSandboxContext>) -> io::Result<()> { + if sandbox.is_some() { + return Err(io::Error::new( io::ErrorKind::InvalidInput, - format!( - "fs/{access_kind} is not permitted for path {}", - path.as_path().display() - ), - )) + "direct filesystem operations do not accept sandbox context", + )); } + Ok(()) } -#[derive(Clone, Copy)] -enum AccessPathMode { - ResolveAll, - PreserveLeaf, +fn reject_platform_sandbox_context(sandbox: Option<&FileSystemSandboxContext>) -> io::Result<()> { + if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "sandboxed filesystem operations require configured runtime paths", + )); + } + Ok(()) } fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> { @@ -395,28 +437,11 @@ fn destination_is_same_or_descendant_of_source( destination: &Path, ) -> io::Result { let source = std::fs::canonicalize(source)?; - let destination = resolve_path_for_access_check(destination, AccessPathMode::ResolveAll)?; + let destination = resolve_existing_path(destination)?; Ok(destination.starts_with(&source)) } -fn resolve_path_for_access_check(path: &Path, path_mode: AccessPathMode) -> io::Result { - match path_mode { - AccessPathMode::ResolveAll => resolve_existing_path(path), - AccessPathMode::PreserveLeaf => preserve_leaf_path_for_access_check(path), - } -} - -fn preserve_leaf_path_for_access_check(path: &Path) -> io::Result { - let Some(file_name) = path.file_name() else { - return resolve_existing_path(path); - }; - let parent = path.parent().unwrap_or_else(|| Path::new("/")); - let mut resolved_parent = resolve_existing_path(parent)?; - resolved_parent.push(file_name); - Ok(resolved_parent) -} - -fn resolve_existing_path(path: &Path) -> io::Result { +pub(crate) fn resolve_existing_path(path: &Path) -> io::Result { let mut unresolved_suffix = Vec::new(); let mut existing_path = path; while !existing_path.exists() { @@ -437,33 +462,12 @@ fn resolve_existing_path(path: &Path) -> io::Result { Ok(resolved) } -fn current_sandbox_cwd() -> io::Result { +pub(crate) fn current_sandbox_cwd() -> io::Result { let cwd = std::env::current_dir() .map_err(|err| io::Error::other(format!("failed to read current dir: {err}")))?; resolve_existing_path(cwd.as_path()) } -fn canonicalize_file_system_policy_paths( - mut file_system_policy: FileSystemSandboxPolicy, -) -> io::Result { - for entry in &mut file_system_policy.entries { - if let FileSystemPath::Path { path } = &mut entry.path { - *path = canonicalize_absolute_path(path)?; - } - } - Ok(file_system_policy) -} - -fn canonicalize_absolute_path(path: &AbsolutePathBuf) -> io::Result { - let resolved = resolve_existing_path(path.as_path())?; - AbsolutePathBuf::from_absolute_path(resolved.as_path()).map_err(|err| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("path must stay absolute after canonicalization: {err}"), - ) - }) -} - fn copy_symlink(source: &Path, target: &Path) -> io::Result<()> { let link_target = std::fs::read_link(source)?; #[cfg(unix)] @@ -508,29 +512,11 @@ fn system_time_to_unix_ms(time: SystemTime) -> i64 { #[cfg(all(test, unix))] mod tests { use super::*; - use codex_protocol::protocol::ReadOnlyAccess; use pretty_assertions::assert_eq; use std::os::unix::fs::symlink; - fn absolute_path(path: PathBuf) -> AbsolutePathBuf { - match AbsolutePathBuf::try_from(path) { - Ok(path) => path, - Err(err) => panic!("absolute path: {err}"), - } - } - - fn read_only_sandbox_policy(readable_roots: Vec) -> SandboxPolicy { - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::Restricted { - include_platform_defaults: false, - readable_roots: readable_roots.into_iter().map(absolute_path).collect(), - }, - network_access: false, - } - } - #[test] - fn resolve_path_for_access_check_rejects_symlink_parent_dotdot_escape() -> io::Result<()> { + fn resolve_existing_path_handles_symlink_parent_dotdot_escape() -> io::Result<()> { let temp_dir = tempfile::TempDir::new()?; let allowed_dir = temp_dir.path().join("allowed"); let outside_dir = temp_dir.path().join("outside"); @@ -538,13 +524,12 @@ mod tests { std::fs::create_dir_all(&outside_dir)?; symlink(&outside_dir, allowed_dir.join("link"))?; - let resolved = resolve_path_for_access_check( + let resolved = resolve_existing_path( allowed_dir .join("link") .join("..") .join("secret.txt") .as_path(), - AccessPathMode::ResolveAll, )?; assert_eq!( @@ -553,29 +538,6 @@ mod tests { ); Ok(()) } - - #[test] - fn enforce_read_access_uses_explicit_sandbox_cwd() -> io::Result<()> { - let temp_dir = tempfile::TempDir::new()?; - let workspace_dir = temp_dir.path().join("workspace"); - let other_dir = temp_dir.path().join("other"); - let note_path = workspace_dir.join("note.txt"); - std::fs::create_dir_all(&workspace_dir)?; - std::fs::create_dir_all(&other_dir)?; - std::fs::write(¬e_path, "hello")?; - - let sandbox_policy = read_only_sandbox_policy(vec![]); - let sandbox_cwd = absolute_path(workspace_dir); - let other_cwd = absolute_path(other_dir); - let note_path = absolute_path(note_path); - - enforce_read_access_for_cwd(¬e_path, Some(&sandbox_policy), &sandbox_cwd)?; - - let error = enforce_read_access_for_cwd(¬e_path, Some(&sandbox_policy), &other_cwd) - .expect_err("read should be rejected outside provided cwd"); - assert_eq!(error.kind(), io::ErrorKind::InvalidInput); - Ok(()) - } } #[cfg(all(test, windows))] diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 54034bdc5..b35363762 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -1,8 +1,8 @@ use std::collections::HashMap; use std::path::PathBuf; +use crate::FileSystemSandboxContext; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; @@ -141,7 +141,7 @@ pub struct TerminateResponse { #[serde(rename_all = "camelCase")] pub struct FsReadFileParams { pub path: AbsolutePathBuf, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -155,7 +155,7 @@ pub struct FsReadFileResponse { pub struct FsWriteFileParams { pub path: AbsolutePathBuf, pub data_base64: String, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -167,7 +167,7 @@ pub struct FsWriteFileResponse {} pub struct FsCreateDirectoryParams { pub path: AbsolutePathBuf, pub recursive: Option, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -178,7 +178,7 @@ pub struct FsCreateDirectoryResponse {} #[serde(rename_all = "camelCase")] pub struct FsGetMetadataParams { pub path: AbsolutePathBuf, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -194,7 +194,7 @@ pub struct FsGetMetadataResponse { #[serde(rename_all = "camelCase")] pub struct FsReadDirectoryParams { pub path: AbsolutePathBuf, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -217,7 +217,7 @@ pub struct FsRemoveParams { pub path: AbsolutePathBuf, pub recursive: Option, pub force: Option, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -230,7 +230,7 @@ pub struct FsCopyParams { pub source_path: AbsolutePathBuf, pub destination_path: AbsolutePathBuf, pub recursive: bool, - pub sandbox_policy: Option, + pub sandbox: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index c92b460a8..ff4d8a4ce 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -1,7 +1,6 @@ use async_trait::async_trait; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; -use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use tokio::io; use tracing::trace; @@ -13,6 +12,7 @@ use crate::ExecServerError; use crate::ExecutorFileSystem; use crate::FileMetadata; use crate::FileSystemResult; +use crate::FileSystemSandboxContext; use crate::ReadDirectoryEntry; use crate::RemoveOptions; use crate::protocol::FsCopyParams; @@ -40,13 +40,17 @@ impl RemoteFileSystem { #[async_trait] impl ExecutorFileSystem for RemoteFileSystem { - async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult> { + async fn read_file( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { trace!("remote fs read_file"); let response = self .client .fs_read_file(FsReadFileParams { path: path.clone(), - sandbox_policy: None, + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; @@ -58,53 +62,18 @@ impl ExecutorFileSystem for RemoteFileSystem { }) } - async fn read_file_with_sandbox_policy( + async fn write_file( &self, path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult> { - trace!("remote fs read_file_with_sandbox_policy"); - let response = self - .client - .fs_read_file(FsReadFileParams { - path: path.clone(), - sandbox_policy: sandbox_policy.cloned(), - }) - .await - .map_err(map_remote_error)?; - STANDARD.decode(response.data_base64).map_err(|err| { - io::Error::new( - io::ErrorKind::InvalidData, - format!("remote fs/readFile returned invalid base64 dataBase64: {err}"), - ) - }) - } - - async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec) -> FileSystemResult<()> { + contents: Vec, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { trace!("remote fs write_file"); self.client .fs_write_file(FsWriteFileParams { path: path.clone(), data_base64: STANDARD.encode(contents), - sandbox_policy: None, - }) - .await - .map_err(map_remote_error)?; - Ok(()) - } - - async fn write_file_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - contents: Vec, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - trace!("remote fs write_file_with_sandbox_policy"); - self.client - .fs_write_file(FsWriteFileParams { - path: path.clone(), - data_base64: STANDARD.encode(contents), - sandbox_policy: sandbox_policy.cloned(), + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; @@ -115,66 +84,31 @@ impl ExecutorFileSystem for RemoteFileSystem { &self, path: &AbsolutePathBuf, options: CreateDirectoryOptions, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs create_directory"); self.client .fs_create_directory(FsCreateDirectoryParams { path: path.clone(), recursive: Some(options.recursive), - sandbox_policy: None, + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; Ok(()) } - async fn create_directory_with_sandbox_policy( + async fn get_metadata( &self, path: &AbsolutePathBuf, - create_directory_options: CreateDirectoryOptions, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - trace!("remote fs create_directory_with_sandbox_policy"); - self.client - .fs_create_directory(FsCreateDirectoryParams { - path: path.clone(), - recursive: Some(create_directory_options.recursive), - sandbox_policy: sandbox_policy.cloned(), - }) - .await - .map_err(map_remote_error)?; - Ok(()) - } - - async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult { + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { trace!("remote fs get_metadata"); let response = self .client .fs_get_metadata(FsGetMetadataParams { path: path.clone(), - sandbox_policy: None, - }) - .await - .map_err(map_remote_error)?; - Ok(FileMetadata { - is_directory: response.is_directory, - is_file: response.is_file, - created_at_ms: response.created_at_ms, - modified_at_ms: response.modified_at_ms, - }) - } - - async fn get_metadata_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult { - trace!("remote fs get_metadata_with_sandbox_policy"); - let response = self - .client - .fs_get_metadata(FsGetMetadataParams { - path: path.clone(), - sandbox_policy: sandbox_policy.cloned(), + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; @@ -189,13 +123,14 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn read_directory( &self, path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_directory"); let response = self .client .fs_read_directory(FsReadDirectoryParams { path: path.clone(), - sandbox_policy: None, + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; @@ -210,58 +145,19 @@ impl ExecutorFileSystem for RemoteFileSystem { .collect()) } - async fn read_directory_with_sandbox_policy( + async fn remove( &self, path: &AbsolutePathBuf, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult> { - trace!("remote fs read_directory_with_sandbox_policy"); - let response = self - .client - .fs_read_directory(FsReadDirectoryParams { - path: path.clone(), - sandbox_policy: sandbox_policy.cloned(), - }) - .await - .map_err(map_remote_error)?; - Ok(response - .entries - .into_iter() - .map(|entry| ReadDirectoryEntry { - file_name: entry.file_name, - is_directory: entry.is_directory, - is_file: entry.is_file, - }) - .collect()) - } - - async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()> { + options: RemoveOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { trace!("remote fs remove"); self.client .fs_remove(FsRemoveParams { path: path.clone(), recursive: Some(options.recursive), force: Some(options.force), - sandbox_policy: None, - }) - .await - .map_err(map_remote_error)?; - Ok(()) - } - - async fn remove_with_sandbox_policy( - &self, - path: &AbsolutePathBuf, - remove_options: RemoveOptions, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - trace!("remote fs remove_with_sandbox_policy"); - self.client - .fs_remove(FsRemoveParams { - path: path.clone(), - recursive: Some(remove_options.recursive), - force: Some(remove_options.force), - sandbox_policy: sandbox_policy.cloned(), + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; @@ -273,6 +169,7 @@ impl ExecutorFileSystem for RemoteFileSystem { source_path: &AbsolutePathBuf, destination_path: &AbsolutePathBuf, options: CopyOptions, + sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs copy"); self.client @@ -280,27 +177,7 @@ impl ExecutorFileSystem for RemoteFileSystem { source_path: source_path.clone(), destination_path: destination_path.clone(), recursive: options.recursive, - sandbox_policy: None, - }) - .await - .map_err(map_remote_error)?; - Ok(()) - } - - async fn copy_with_sandbox_policy( - &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, - copy_options: CopyOptions, - sandbox_policy: Option<&SandboxPolicy>, - ) -> FileSystemResult<()> { - trace!("remote fs copy_with_sandbox_policy"); - self.client - .fs_copy(FsCopyParams { - source_path: source_path.clone(), - destination_path: destination_path.clone(), - recursive: copy_options.recursive, - sandbox_policy: sandbox_policy.cloned(), + sandbox: sandbox.cloned(), }) .await .map_err(map_remote_error)?; diff --git a/codex-rs/exec-server/src/runtime_paths.rs b/codex-rs/exec-server/src/runtime_paths.rs new file mode 100644 index 000000000..1d3713b1c --- /dev/null +++ b/codex-rs/exec-server/src/runtime_paths.rs @@ -0,0 +1,43 @@ +use std::path::PathBuf; + +use codex_utils_absolute_path::AbsolutePathBuf; + +/// Runtime paths needed by exec-server child processes. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ExecServerRuntimePaths { + /// Stable path to the Codex executable used to launch hidden helper modes. + pub codex_self_exe: AbsolutePathBuf, + /// 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, +} + +impl ExecServerRuntimePaths { + pub fn from_optional_paths( + codex_self_exe: Option, + codex_linux_sandbox_exe: Option, + ) -> std::io::Result { + let codex_self_exe = codex_self_exe.ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "Codex executable path is not configured", + ) + })?; + Self::new(codex_self_exe, codex_linux_sandbox_exe) + } + + pub fn new( + codex_self_exe: PathBuf, + codex_linux_sandbox_exe: Option, + ) -> std::io::Result { + Ok(Self { + codex_self_exe: absolute_path(codex_self_exe)?, + codex_linux_sandbox_exe: codex_linux_sandbox_exe.map(absolute_path).transpose()?, + }) + } +} + +fn absolute_path(path: PathBuf) -> std::io::Result { + AbsolutePathBuf::from_absolute_path(path.as_path()) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidInput, err)) +} diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs new file mode 100644 index 000000000..1079b22b4 --- /dev/null +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -0,0 +1,239 @@ +use async_trait::async_trait; +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_utils_absolute_path::AbsolutePathBuf; +use tokio::io; + +use crate::CopyOptions; +use crate::CreateDirectoryOptions; +use crate::ExecServerRuntimePaths; +use crate::ExecutorFileSystem; +use crate::FileMetadata; +use crate::FileSystemResult; +use crate::FileSystemSandboxContext; +use crate::ReadDirectoryEntry; +use crate::RemoveOptions; +use crate::fs_helper::FsHelperPayload; +use crate::fs_helper::FsHelperRequest; +use crate::fs_sandbox::FileSystemSandboxRunner; +use crate::protocol::FsCopyParams; +use crate::protocol::FsCreateDirectoryParams; +use crate::protocol::FsGetMetadataParams; +use crate::protocol::FsReadDirectoryParams; +use crate::protocol::FsReadFileParams; +use crate::protocol::FsRemoveParams; +use crate::protocol::FsWriteFileParams; + +#[derive(Clone)] +pub struct SandboxedFileSystem { + sandbox_runner: FileSystemSandboxRunner, +} + +impl SandboxedFileSystem { + pub fn new(runtime_paths: ExecServerRuntimePaths) -> Self { + Self { + sandbox_runner: FileSystemSandboxRunner::new(runtime_paths), + } + } + + async fn run_sandboxed( + &self, + sandbox: &FileSystemSandboxContext, + request: FsHelperRequest, + ) -> FileSystemResult { + self.sandbox_runner + .run(sandbox, request) + .await + .map_err(map_sandbox_error) + } +} + +#[async_trait] +impl ExecutorFileSystem for SandboxedFileSystem { + async fn read_file( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + let sandbox = require_platform_sandbox(sandbox)?; + let response = self + .run_sandboxed( + sandbox, + FsHelperRequest::ReadFile(FsReadFileParams { + path: path.clone(), + sandbox: None, + }), + ) + .await? + .expect_read_file() + .map_err(map_sandbox_error)?; + STANDARD.decode(response.data_base64).map_err(|err| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("fs/readFile returned invalid base64 dataBase64: {err}"), + ) + }) + } + + async fn write_file( + &self, + path: &AbsolutePathBuf, + contents: Vec, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let sandbox = require_platform_sandbox(sandbox)?; + self.run_sandboxed( + sandbox, + FsHelperRequest::WriteFile(FsWriteFileParams { + path: path.clone(), + data_base64: STANDARD.encode(contents), + sandbox: None, + }), + ) + .await? + .expect_write_file() + .map_err(map_sandbox_error)?; + Ok(()) + } + + async fn create_directory( + &self, + path: &AbsolutePathBuf, + options: CreateDirectoryOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let sandbox = require_platform_sandbox(sandbox)?; + self.run_sandboxed( + sandbox, + FsHelperRequest::CreateDirectory(FsCreateDirectoryParams { + path: path.clone(), + recursive: Some(options.recursive), + sandbox: None, + }), + ) + .await? + .expect_create_directory() + .map_err(map_sandbox_error)?; + Ok(()) + } + + async fn get_metadata( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + let sandbox = require_platform_sandbox(sandbox)?; + let response = self + .run_sandboxed( + sandbox, + FsHelperRequest::GetMetadata(FsGetMetadataParams { + path: path.clone(), + sandbox: None, + }), + ) + .await? + .expect_get_metadata() + .map_err(map_sandbox_error)?; + Ok(FileMetadata { + is_directory: response.is_directory, + is_file: response.is_file, + created_at_ms: response.created_at_ms, + modified_at_ms: response.modified_at_ms, + }) + } + + async fn read_directory( + &self, + path: &AbsolutePathBuf, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + let sandbox = require_platform_sandbox(sandbox)?; + let response = self + .run_sandboxed( + sandbox, + FsHelperRequest::ReadDirectory(FsReadDirectoryParams { + path: path.clone(), + sandbox: None, + }), + ) + .await? + .expect_read_directory() + .map_err(map_sandbox_error)?; + Ok(response + .entries + .into_iter() + .map(|entry| ReadDirectoryEntry { + file_name: entry.file_name, + is_directory: entry.is_directory, + is_file: entry.is_file, + }) + .collect()) + } + + async fn remove( + &self, + path: &AbsolutePathBuf, + remove_options: RemoveOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let sandbox = require_platform_sandbox(sandbox)?; + self.run_sandboxed( + sandbox, + FsHelperRequest::Remove(FsRemoveParams { + path: path.clone(), + recursive: Some(remove_options.recursive), + force: Some(remove_options.force), + sandbox: None, + }), + ) + .await? + .expect_remove() + .map_err(map_sandbox_error)?; + Ok(()) + } + + async fn copy( + &self, + source_path: &AbsolutePathBuf, + destination_path: &AbsolutePathBuf, + options: CopyOptions, + sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + let sandbox = require_platform_sandbox(sandbox)?; + self.run_sandboxed( + sandbox, + FsHelperRequest::Copy(FsCopyParams { + source_path: source_path.clone(), + destination_path: destination_path.clone(), + recursive: options.recursive, + sandbox: None, + }), + ) + .await? + .expect_copy() + .map_err(map_sandbox_error)?; + Ok(()) + } +} + +fn require_platform_sandbox( + sandbox: Option<&FileSystemSandboxContext>, +) -> FileSystemResult<&FileSystemSandboxContext> { + sandbox + .filter(|sandbox| sandbox.should_run_in_sandbox()) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "sandboxed filesystem operations require ReadOnly or WorkspaceWrite sandbox policy", + ) + }) +} + +fn map_sandbox_error(error: JSONRPCErrorError) -> io::Error { + match error.code { + -32004 => io::Error::new(io::ErrorKind::NotFound, error.message), + -32600 => io::Error::new(io::ErrorKind::InvalidInput, error.message), + _ => io::Error::other(error.message), + } +} diff --git a/codex-rs/exec-server/src/server.rs b/codex-rs/exec-server/src/server.rs index 44dc0a5d0..62c178738 100644 --- a/codex-rs/exec-server/src/server.rs +++ b/codex-rs/exec-server/src/server.rs @@ -10,12 +10,11 @@ pub(crate) use handler::ExecServerHandler; pub use transport::DEFAULT_LISTEN_URL; pub use transport::ExecServerListenUrlParseError; -pub async fn run_main() -> Result<(), Box> { - run_main_with_listen_url(DEFAULT_LISTEN_URL).await -} +use crate::ExecServerRuntimePaths; -pub async fn run_main_with_listen_url( +pub async fn run_main( listen_url: &str, + runtime_paths: ExecServerRuntimePaths, ) -> Result<(), Box> { - transport::run_transport(listen_url).await + transport::run_transport(listen_url, runtime_paths).await } diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index 84dbd4f29..d254ef624 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -6,9 +6,11 @@ use codex_app_server_protocol::JSONRPCErrorError; use crate::CopyOptions; use crate::CreateDirectoryOptions; +use crate::ExecServerRuntimePaths; use crate::ExecutorFileSystem; use crate::RemoveOptions; use crate::local_file_system::LocalFileSystem; +use crate::protocol::FS_WRITE_FILE_METHOD; use crate::protocol::FsCopyParams; use crate::protocol::FsCopyResponse; use crate::protocol::FsCreateDirectoryParams; @@ -28,19 +30,25 @@ use crate::rpc::internal_error; use crate::rpc::invalid_request; use crate::rpc::not_found; -#[derive(Clone, Default)] +#[derive(Clone)] pub(crate) struct FileSystemHandler { file_system: LocalFileSystem, } impl FileSystemHandler { + pub(crate) fn new(runtime_paths: ExecServerRuntimePaths) -> Self { + Self { + file_system: LocalFileSystem::with_runtime_paths(runtime_paths), + } + } + pub(crate) async fn read_file( &self, params: FsReadFileParams, ) -> Result { let bytes = self .file_system - .read_file_with_sandbox_policy(¶ms.path, params.sandbox_policy.as_ref()) + .read_file(¶ms.path, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsReadFileResponse { @@ -54,11 +62,11 @@ impl FileSystemHandler { ) -> Result { let bytes = STANDARD.decode(params.data_base64).map_err(|err| { invalid_request(format!( - "fs/writeFile requires valid base64 dataBase64: {err}" + "{FS_WRITE_FILE_METHOD} requires valid base64 dataBase64: {err}" )) })?; self.file_system - .write_file_with_sandbox_policy(¶ms.path, bytes, params.sandbox_policy.as_ref()) + .write_file(¶ms.path, bytes, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsWriteFileResponse {}) @@ -68,13 +76,12 @@ impl FileSystemHandler { &self, params: FsCreateDirectoryParams, ) -> Result { + let recursive = params.recursive.unwrap_or(true); self.file_system - .create_directory_with_sandbox_policy( + .create_directory( ¶ms.path, - CreateDirectoryOptions { - recursive: params.recursive.unwrap_or(true), - }, - params.sandbox_policy.as_ref(), + CreateDirectoryOptions { recursive }, + params.sandbox.as_ref(), ) .await .map_err(map_fs_error)?; @@ -87,7 +94,7 @@ impl FileSystemHandler { ) -> Result { let metadata = self .file_system - .get_metadata_with_sandbox_policy(¶ms.path, params.sandbox_policy.as_ref()) + .get_metadata(¶ms.path, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsGetMetadataResponse { @@ -104,33 +111,30 @@ impl FileSystemHandler { ) -> Result { let entries = self .file_system - .read_directory_with_sandbox_policy(¶ms.path, params.sandbox_policy.as_ref()) + .read_directory(¶ms.path, params.sandbox.as_ref()) .await - .map_err(map_fs_error)?; - Ok(FsReadDirectoryResponse { - entries: entries - .into_iter() - .map(|entry| FsReadDirectoryEntry { - file_name: entry.file_name, - is_directory: entry.is_directory, - is_file: entry.is_file, - }) - .collect(), - }) + .map_err(map_fs_error)? + .into_iter() + .map(|entry| FsReadDirectoryEntry { + file_name: entry.file_name, + is_directory: entry.is_directory, + is_file: entry.is_file, + }) + .collect(); + Ok(FsReadDirectoryResponse { entries }) } pub(crate) async fn remove( &self, params: FsRemoveParams, ) -> Result { + let recursive = params.recursive.unwrap_or(true); + let force = params.force.unwrap_or(true); self.file_system - .remove_with_sandbox_policy( + .remove( ¶ms.path, - RemoveOptions { - recursive: params.recursive.unwrap_or(true), - force: params.force.unwrap_or(true), - }, - params.sandbox_policy.as_ref(), + RemoveOptions { recursive, force }, + params.sandbox.as_ref(), ) .await .map_err(map_fs_error)?; @@ -142,13 +146,13 @@ impl FileSystemHandler { params: FsCopyParams, ) -> Result { self.file_system - .copy_with_sandbox_policy( + .copy( ¶ms.source_path, ¶ms.destination_path, CopyOptions { recursive: params.recursive, }, - params.sandbox_policy.as_ref(), + params.sandbox.as_ref(), ) .await .map_err(map_fs_error)?; @@ -157,11 +161,68 @@ impl FileSystemHandler { } fn map_fs_error(err: io::Error) -> JSONRPCErrorError { - if err.kind() == io::ErrorKind::NotFound { - not_found(err.to_string()) - } else if err.kind() == io::ErrorKind::InvalidInput { - invalid_request(err.to_string()) - } else { - internal_error(err.to_string()) + match err.kind() { + io::ErrorKind::NotFound => not_found(err.to_string()), + io::ErrorKind::InvalidInput | io::ErrorKind::PermissionDenied => { + invalid_request(err.to_string()) + } + _ => internal_error(err.to_string()), + } +} + +#[cfg(test)] +mod tests { + use codex_protocol::protocol::NetworkAccess; + use codex_protocol::protocol::SandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + + use super::*; + use crate::FileSystemSandboxContext; + use crate::protocol::FsReadFileParams; + use crate::protocol::FsWriteFileParams; + + #[tokio::test] + async fn no_platform_sandbox_policies_do_not_require_configured_sandbox_helper() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let runtime_paths = ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let handler = FileSystemHandler::new(runtime_paths); + + for (file_name, sandbox_policy) in [ + ("danger.txt", SandboxPolicy::DangerFullAccess), + ( + "external.txt", + SandboxPolicy::ExternalSandbox { + network_access: NetworkAccess::Restricted, + }, + ), + ] { + let path = + AbsolutePathBuf::from_absolute_path(temp_dir.path().join(file_name).as_path()) + .expect("absolute path"); + + handler + .write_file(FsWriteFileParams { + path: path.clone(), + data_base64: STANDARD.encode("ok"), + sandbox: Some(FileSystemSandboxContext::new(sandbox_policy.clone())), + }) + .await + .expect("write file"); + + let response = handler + .read_file(FsReadFileParams { + path, + sandbox: Some(FileSystemSandboxContext::new(sandbox_policy)), + }) + .await + .expect("read file"); + + assert_eq!(response.data_base64, STANDARD.encode("ok")); + } } } diff --git a/codex-rs/exec-server/src/server/handler.rs b/codex-rs/exec-server/src/server/handler.rs index 65bfc15fd..46f7af90a 100644 --- a/codex-rs/exec-server/src/server/handler.rs +++ b/codex-rs/exec-server/src/server/handler.rs @@ -5,6 +5,7 @@ use std::sync::atomic::Ordering; use codex_app_server_protocol::JSONRPCErrorError; +use crate::ExecServerRuntimePaths; use crate::protocol::ExecParams; use crate::protocol::ExecResponse; use crate::protocol::FsCopyParams; @@ -48,12 +49,13 @@ impl ExecServerHandler { pub(crate) fn new( session_registry: Arc, notifications: RpcNotificationSender, + runtime_paths: ExecServerRuntimePaths, ) -> Self { Self { session_registry, notifications, session: StdMutex::new(None), - file_system: FileSystemHandler::default(), + file_system: FileSystemHandler::new(runtime_paths), initialize_requested: AtomicBool::new(false), initialized: AtomicBool::new(false), } diff --git a/codex-rs/exec-server/src/server/handler/tests.rs b/codex-rs/exec-server/src/server/handler/tests.rs index c0b20172c..547409946 100644 --- a/codex-rs/exec-server/src/server/handler/tests.rs +++ b/codex-rs/exec-server/src/server/handler/tests.rs @@ -7,6 +7,7 @@ use tokio::sync::mpsc; use uuid::Uuid; use super::ExecServerHandler; +use crate::ExecServerRuntimePaths; use crate::ProcessId; use crate::protocol::ExecParams; use crate::protocol::InitializeParams; @@ -64,12 +65,21 @@ fn windows_command_processor() -> String { std::env::var("COMSPEC").unwrap_or_else(|_| "cmd.exe".to_string()) } +fn test_runtime_paths() -> ExecServerRuntimePaths { + ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths") +} + async fn initialized_handler() -> Arc { let (outgoing_tx, _outgoing_rx) = mpsc::channel(16); let registry = SessionRegistry::new(); let handler = Arc::new(ExecServerHandler::new( registry, RpcNotificationSender::new(outgoing_tx), + test_runtime_paths(), )); let initialize_response = handler .initialize(InitializeParams { @@ -147,6 +157,7 @@ async fn long_poll_read_fails_after_session_resume() { let first_handler = Arc::new(ExecServerHandler::new( Arc::clone(®istry), RpcNotificationSender::new(first_tx), + test_runtime_paths(), )); let initialize_response = first_handler .initialize(InitializeParams { @@ -187,6 +198,7 @@ async fn long_poll_read_fails_after_session_resume() { let second_handler = Arc::new(ExecServerHandler::new( registry, RpcNotificationSender::new(second_tx), + test_runtime_paths(), )); second_handler .initialize(InitializeParams { @@ -219,6 +231,7 @@ async fn active_session_resume_is_rejected() { let first_handler = Arc::new(ExecServerHandler::new( Arc::clone(®istry), RpcNotificationSender::new(first_tx), + test_runtime_paths(), )); let initialize_response = first_handler .initialize(InitializeParams { @@ -232,6 +245,7 @@ async fn active_session_resume_is_rejected() { let second_handler = Arc::new(ExecServerHandler::new( registry, RpcNotificationSender::new(second_tx), + test_runtime_paths(), )); let err = second_handler .initialize(InitializeParams { @@ -259,6 +273,7 @@ async fn output_and_exit_are_retained_after_notification_receiver_closes() { let handler = Arc::new(ExecServerHandler::new( SessionRegistry::new(), RpcNotificationSender::new(outgoing_tx), + test_runtime_paths(), )); handler .initialize(InitializeParams { diff --git a/codex-rs/exec-server/src/server/processor.rs b/codex-rs/exec-server/src/server/processor.rs index bd30a9819..6ea6a55bd 100644 --- a/codex-rs/exec-server/src/server/processor.rs +++ b/codex-rs/exec-server/src/server/processor.rs @@ -4,6 +4,7 @@ use tokio::sync::mpsc; use tracing::debug; use tracing::warn; +use crate::ExecServerRuntimePaths; use crate::connection::CHANNEL_CAPACITY; use crate::connection::JsonRpcConnection; use crate::connection::JsonRpcConnectionEvent; @@ -19,28 +20,43 @@ use crate::server::session_registry::SessionRegistry; #[derive(Clone)] pub(crate) struct ConnectionProcessor { session_registry: Arc, + runtime_paths: ExecServerRuntimePaths, } impl ConnectionProcessor { - pub(crate) fn new() -> Self { + pub(crate) fn new(runtime_paths: ExecServerRuntimePaths) -> Self { Self { session_registry: SessionRegistry::new(), + runtime_paths, } } pub(crate) async fn run_connection(&self, connection: JsonRpcConnection) { - run_connection(connection, Arc::clone(&self.session_registry)).await; + run_connection( + connection, + Arc::clone(&self.session_registry), + self.runtime_paths.clone(), + ) + .await; } } -async fn run_connection(connection: JsonRpcConnection, session_registry: Arc) { +async fn run_connection( + connection: JsonRpcConnection, + session_registry: Arc, + runtime_paths: ExecServerRuntimePaths, +) { let router = Arc::new(build_router()); let (json_outgoing_tx, mut incoming_rx, mut disconnected_rx, connection_tasks) = connection.into_parts(); let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(CHANNEL_CAPACITY); let notifications = RpcNotificationSender::new(outgoing_tx.clone()); - let handler = Arc::new(ExecServerHandler::new(session_registry, notifications)); + let handler = Arc::new(ExecServerHandler::new( + session_registry, + notifications, + runtime_paths, + )); let outbound_task = tokio::spawn(async move { while let Some(message) = outgoing_rx.recv().await { @@ -184,6 +200,7 @@ mod tests { use tokio::time::timeout; use super::run_connection; + use crate::ExecServerRuntimePaths; use crate::ProcessId; use crate::connection::JsonRpcConnection; use crate::protocol::EXEC_METHOD; @@ -298,10 +315,18 @@ mod tests { let (server_writer, client_reader) = duplex(1 << 20); let connection = JsonRpcConnection::from_stdio(server_reader, server_writer, label.to_string()); - let task = tokio::spawn(run_connection(connection, registry)); + let task = tokio::spawn(run_connection(connection, registry, test_runtime_paths())); (client_writer, BufReader::new(client_reader).lines(), task) } + fn test_runtime_paths() -> ExecServerRuntimePaths { + ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths") + } + async fn send_request( writer: &mut DuplexStream, id: i64, diff --git a/codex-rs/exec-server/src/server/transport.rs b/codex-rs/exec-server/src/server/transport.rs index de94b1af6..b8a5a086b 100644 --- a/codex-rs/exec-server/src/server/transport.rs +++ b/codex-rs/exec-server/src/server/transport.rs @@ -1,9 +1,10 @@ +use std::io::Write as _; use std::net::SocketAddr; - use tokio::net::TcpListener; use tokio_tungstenite::accept_async; use tracing::warn; +use crate::ExecServerRuntimePaths; use crate::connection::JsonRpcConnection; use crate::server::processor::ConnectionProcessor; @@ -48,19 +49,22 @@ pub(crate) fn parse_listen_url( pub(crate) async fn run_transport( listen_url: &str, + runtime_paths: ExecServerRuntimePaths, ) -> Result<(), Box> { let bind_address = parse_listen_url(listen_url)?; - run_websocket_listener(bind_address).await + run_websocket_listener(bind_address, runtime_paths).await } async fn run_websocket_listener( bind_address: SocketAddr, + runtime_paths: ExecServerRuntimePaths, ) -> Result<(), Box> { let listener = TcpListener::bind(bind_address).await?; let local_addr = listener.local_addr()?; - let processor = ConnectionProcessor::new(); + let processor = ConnectionProcessor::new(runtime_paths); tracing::info!("codex-exec-server listening on ws://{local_addr}"); println!("ws://{local_addr}"); + std::io::stdout().flush()?; loop { let (stream, peer_addr) = listener.accept().await?; diff --git a/codex-rs/exec-server/tests/common/exec_server.rs b/codex-rs/exec-server/tests/common/exec_server.rs index 65d6f1ec6..0b6eec544 100644 --- a/codex-rs/exec-server/tests/common/exec_server.rs +++ b/codex-rs/exec-server/tests/common/exec_server.rs @@ -41,9 +41,9 @@ impl Drop for ExecServerHarness { } pub(crate) async fn exec_server() -> anyhow::Result { - let binary = cargo_bin("codex-exec-server")?; + let binary = cargo_bin("codex")?; let mut child = Command::new(binary); - child.args(["--listen", "ws://127.0.0.1:0"]); + child.args(["exec-server", "--listen", "ws://127.0.0.1:0"]); child.stdin(Stdio::null()); child.stdout(Stdio::piped()); child.stderr(Stdio::inherit()); diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index a3272d4b3..ae8c09ca2 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -11,7 +11,10 @@ use anyhow::Result; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::Environment; +use codex_exec_server::ExecServerRuntimePaths; use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::FileSystemSandboxContext; +use codex_exec_server::LocalFileSystem; use codex_exec_server::ReadDirectoryEntry; use codex_exec_server::RemoveOptions; use codex_protocol::protocol::ReadOnlyAccess; @@ -38,9 +41,15 @@ async fn create_file_system_context(use_remote: bool) -> Result AbsolutePathBuf { } } -fn read_only_sandbox_policy(readable_root: std::path::PathBuf) -> SandboxPolicy { - SandboxPolicy::ReadOnly { +fn read_only_sandbox(readable_root: std::path::PathBuf) -> FileSystemSandboxContext { + FileSystemSandboxContext::new(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::Restricted { include_platform_defaults: false, readable_roots: vec![absolute_path(readable_root)], }, network_access: false, - } + }) } -fn workspace_write_sandbox_policy(writable_root: std::path::PathBuf) -> SandboxPolicy { - SandboxPolicy::WorkspaceWrite { +fn workspace_write_sandbox(writable_root: std::path::PathBuf) -> FileSystemSandboxContext { + FileSystemSandboxContext::new(SandboxPolicy::WorkspaceWrite { writable_roots: vec![absolute_path(writable_root)], read_only_access: ReadOnlyAccess::Restricted { include_platform_defaults: false, @@ -78,6 +87,42 @@ fn workspace_write_sandbox_policy(writable_root: std::path::PathBuf) -> SandboxP network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, + }) +} + +fn assert_sandbox_denied(error: &std::io::Error) { + assert!( + matches!( + error.kind(), + std::io::ErrorKind::InvalidInput | std::io::ErrorKind::PermissionDenied + ), + "unexpected sandbox error kind: {error:?}", + ); + let message = error.to_string(); + assert!( + message.contains("is not permitted") + || message.contains("Operation not permitted") + || message.contains("Permission denied"), + "unexpected sandbox error message: {message}", + ); +} + +fn assert_normalized_path_rejected(error: &std::io::Error) { + match error.kind() { + std::io::ErrorKind::NotFound => assert!( + error.to_string().contains("No such file or directory"), + "unexpected not-found message: {error}", + ), + std::io::ErrorKind::InvalidInput | std::io::ErrorKind::PermissionDenied => { + let message = error.to_string(); + assert!( + message.contains("is not permitted") + || message.contains("Operation not permitted") + || message.contains("Permission denied"), + "unexpected rejection message: {message}", + ); + } + other => panic!("unexpected normalized-path error kind: {other:?}: {error:?}"), } } @@ -93,7 +138,7 @@ async fn file_system_get_metadata_returns_expected_fields(use_remote: bool) -> R std::fs::write(&file_path, "hello")?; let metadata = file_system - .get_metadata(&absolute_path(file_path)) + .get_metadata(&absolute_path(file_path), /*sandbox*/ None) .await .with_context(|| format!("mode={use_remote}"))?; assert_eq!(metadata.is_directory, false); @@ -122,6 +167,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> .create_directory( &absolute_path(nested_dir.clone()), CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -130,6 +176,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> .write_file( &absolute_path(nested_file.clone()), b"hello from trait".to_vec(), + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -137,18 +184,19 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> .write_file( &absolute_path(source_file.clone()), b"hello from source root".to_vec(), + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; let nested_file_contents = file_system - .read_file(&absolute_path(nested_file.clone())) + .read_file(&absolute_path(nested_file.clone()), /*sandbox*/ None) .await .with_context(|| format!("mode={use_remote}"))?; assert_eq!(nested_file_contents, b"hello from trait"); let nested_file_text = file_system - .read_file_text(&absolute_path(nested_file.clone())) + .read_file_text(&absolute_path(nested_file.clone()), /*sandbox*/ None) .await .with_context(|| format!("mode={use_remote}"))?; assert_eq!(nested_file_text, "hello from trait"); @@ -158,6 +206,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> &absolute_path(nested_file), &absolute_path(copied_file.clone()), CopyOptions { recursive: false }, + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -168,6 +217,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> &absolute_path(source_dir.clone()), &absolute_path(copied_dir.clone()), CopyOptions { recursive: true }, + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -177,7 +227,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> ); let mut entries = file_system - .read_directory(&absolute_path(source_dir)) + .read_directory(&absolute_path(source_dir), /*sandbox*/ None) .await .with_context(|| format!("mode={use_remote}"))?; entries.sort_by(|left, right| left.file_name.cmp(&right.file_name)); @@ -204,6 +254,7 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()> recursive: true, force: true, }, + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -228,6 +279,7 @@ async fn file_system_copy_rejects_directory_without_recursive(use_remote: bool) &absolute_path(source_dir), &absolute_path(tmp.path().join("dest")), CopyOptions { recursive: false }, + /*sandbox*/ None, ) .await; let error = match error { @@ -246,7 +298,7 @@ async fn file_system_copy_rejects_directory_without_recursive(use_remote: bool) #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_read_with_sandbox_policy_allows_readable_root(use_remote: bool) -> Result<()> { +async fn file_system_sandboxed_read_allows_readable_root(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -255,10 +307,10 @@ async fn file_system_read_with_sandbox_policy_allows_readable_root(use_remote: b let file_path = allowed_dir.join("note.txt"); std::fs::create_dir_all(&allowed_dir)?; std::fs::write(&file_path, "sandboxed hello")?; - let sandbox_policy = read_only_sandbox_policy(allowed_dir); + let sandbox = read_only_sandbox(allowed_dir); let contents = file_system - .read_file_with_sandbox_policy(&absolute_path(file_path), Some(&sandbox_policy)) + .read_file(&absolute_path(file_path), Some(&sandbox)) .await .with_context(|| format!("mode={use_remote}"))?; assert_eq!(contents, b"sandboxed hello"); @@ -269,9 +321,7 @@ async fn file_system_read_with_sandbox_policy_allows_readable_root(use_remote: b #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_write_with_sandbox_policy_rejects_unwritable_path( - use_remote: bool, -) -> Result<()> { +async fn file_system_sandboxed_write_rejects_unwritable_path(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -280,26 +330,19 @@ async fn file_system_write_with_sandbox_policy_rejects_unwritable_path( let blocked_path = tmp.path().join("blocked.txt"); std::fs::create_dir_all(&allowed_dir)?; - let sandbox_policy = read_only_sandbox_policy(allowed_dir); + let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .write_file_with_sandbox_policy( + .write_file( &absolute_path(blocked_path.clone()), b"nope".to_vec(), - Some(&sandbox_policy), + Some(&sandbox), ) .await { Ok(()) => anyhow::bail!("write should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/write is not permitted for path {}", - blocked_path.display() - ) - ); + assert_sandbox_denied(&error); assert!(!blocked_path.exists()); Ok(()) @@ -308,9 +351,7 @@ async fn file_system_write_with_sandbox_policy_rejects_unwritable_path( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_read_with_sandbox_policy_rejects_symlink_escape( - use_remote: bool, -) -> Result<()> { +async fn file_system_sandboxed_read_rejects_symlink_escape(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -323,25 +364,15 @@ async fn file_system_read_with_sandbox_policy_rejects_symlink_escape( symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = allowed_dir.join("link").join("secret.txt"); - let sandbox_policy = read_only_sandbox_policy(allowed_dir); + let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .read_file_with_sandbox_policy( - &absolute_path(requested_path.clone()), - Some(&sandbox_policy), - ) + .read_file(&absolute_path(requested_path.clone()), Some(&sandbox)) .await { Ok(_) => anyhow::bail!("read should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/read is not permitted for path {}", - requested_path.display() - ) - ); + assert_sandbox_denied(&error); Ok(()) } @@ -349,7 +380,7 @@ async fn file_system_read_with_sandbox_policy_rejects_symlink_escape( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_read_with_sandbox_policy_rejects_symlink_parent_dotdot_escape( +async fn file_system_sandboxed_read_rejects_symlink_parent_dotdot_escape( use_remote: bool, ) -> Result<()> { let context = create_file_system_context(use_remote).await?; @@ -365,15 +396,17 @@ async fn file_system_read_with_sandbox_policy_rejects_symlink_parent_dotdot_esca symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = absolute_path(allowed_dir.join("link").join("..").join("secret.txt")); - let sandbox_policy = read_only_sandbox_policy(allowed_dir); - let error = match file_system - .read_file_with_sandbox_policy(&requested_path, Some(&sandbox_policy)) - .await - { + let sandbox = read_only_sandbox(allowed_dir); + let error = match file_system.read_file(&requested_path, Some(&sandbox)).await { Ok(_) => anyhow::bail!("read should fail after path normalization"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::NotFound); + // AbsolutePathBuf normalizes `link/../secret.txt` to `allowed/secret.txt` + // before the request reaches the filesystem layer. Depending on whether + // the platform/runtime resolves that normalized path through a top-level + // symlink alias, the request can surface as either "missing file" or an + // upfront sandbox rejection. + assert_normalized_path_rejected(&error); Ok(()) } @@ -381,9 +414,7 @@ async fn file_system_read_with_sandbox_policy_rejects_symlink_parent_dotdot_esca #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_write_with_sandbox_policy_rejects_symlink_escape( - use_remote: bool, -) -> Result<()> { +async fn file_system_sandboxed_write_rejects_symlink_escape(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -395,26 +426,19 @@ async fn file_system_write_with_sandbox_policy_rejects_symlink_escape( symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = allowed_dir.join("link").join("blocked.txt"); - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir); + let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system - .write_file_with_sandbox_policy( + .write_file( &absolute_path(requested_path.clone()), b"nope".to_vec(), - Some(&sandbox_policy), + Some(&sandbox), ) .await { Ok(()) => anyhow::bail!("write should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/write is not permitted for path {}", - requested_path.display() - ) - ); + assert_sandbox_denied(&error); assert!(!outside_dir.join("blocked.txt").exists()); Ok(()) @@ -423,9 +447,7 @@ async fn file_system_write_with_sandbox_policy_rejects_symlink_escape( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_create_directory_with_sandbox_policy_rejects_symlink_escape( - use_remote: bool, -) -> Result<()> { +async fn file_system_create_directory_rejects_symlink_escape(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -437,26 +459,19 @@ async fn file_system_create_directory_with_sandbox_policy_rejects_symlink_escape symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = allowed_dir.join("link").join("created"); - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir); + let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system - .create_directory_with_sandbox_policy( + .create_directory( &absolute_path(requested_path.clone()), CreateDirectoryOptions { recursive: false }, - Some(&sandbox_policy), + Some(&sandbox), ) .await { Ok(()) => anyhow::bail!("create_directory should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/write is not permitted for path {}", - requested_path.display() - ) - ); + assert_sandbox_denied(&error); assert!(!outside_dir.join("created").exists()); Ok(()) @@ -465,9 +480,7 @@ async fn file_system_create_directory_with_sandbox_policy_rejects_symlink_escape #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_get_metadata_with_sandbox_policy_rejects_symlink_escape( - use_remote: bool, -) -> Result<()> { +async fn file_system_get_metadata_rejects_symlink_escape(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -480,25 +493,15 @@ async fn file_system_get_metadata_with_sandbox_policy_rejects_symlink_escape( symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = allowed_dir.join("link").join("secret.txt"); - let sandbox_policy = read_only_sandbox_policy(allowed_dir); + let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .get_metadata_with_sandbox_policy( - &absolute_path(requested_path.clone()), - Some(&sandbox_policy), - ) + .get_metadata(&absolute_path(requested_path.clone()), Some(&sandbox)) .await { Ok(_) => anyhow::bail!("get_metadata should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/read is not permitted for path {}", - requested_path.display() - ) - ); + assert_sandbox_denied(&error); Ok(()) } @@ -506,9 +509,7 @@ async fn file_system_get_metadata_with_sandbox_policy_rejects_symlink_escape( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_read_directory_with_sandbox_policy_rejects_symlink_escape( - use_remote: bool, -) -> Result<()> { +async fn file_system_read_directory_rejects_symlink_escape(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -521,25 +522,15 @@ async fn file_system_read_directory_with_sandbox_policy_rejects_symlink_escape( symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = allowed_dir.join("link"); - let sandbox_policy = read_only_sandbox_policy(allowed_dir); + let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .read_directory_with_sandbox_policy( - &absolute_path(requested_path.clone()), - Some(&sandbox_policy), - ) + .read_directory(&absolute_path(requested_path.clone()), Some(&sandbox)) .await { Ok(_) => anyhow::bail!("read_directory should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/read is not permitted for path {}", - requested_path.display() - ) - ); + assert_sandbox_denied(&error); Ok(()) } @@ -547,9 +538,7 @@ async fn file_system_read_directory_with_sandbox_policy_rejects_symlink_escape( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_copy_with_sandbox_policy_rejects_symlink_escape_destination( - use_remote: bool, -) -> Result<()> { +async fn file_system_copy_rejects_symlink_escape_destination(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -562,27 +551,20 @@ async fn file_system_copy_with_sandbox_policy_rejects_symlink_escape_destination symlink(&outside_dir, allowed_dir.join("link"))?; let requested_destination = allowed_dir.join("link").join("copied.txt"); - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir.clone()); + let sandbox = workspace_write_sandbox(allowed_dir.clone()); let error = match file_system - .copy_with_sandbox_policy( + .copy( &absolute_path(allowed_dir.join("source.txt")), &absolute_path(requested_destination.clone()), CopyOptions { recursive: false }, - Some(&sandbox_policy), + Some(&sandbox), ) .await { Ok(()) => anyhow::bail!("copy should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/write is not permitted for path {}", - requested_destination.display() - ) - ); + assert_sandbox_denied(&error); assert!(!outside_dir.join("copied.txt").exists()); Ok(()) @@ -591,9 +573,7 @@ async fn file_system_copy_with_sandbox_policy_rejects_symlink_escape_destination #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_remove_with_sandbox_policy_removes_symlink_not_target( - use_remote: bool, -) -> Result<()> { +async fn file_system_remove_removes_symlink_not_target(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -607,15 +587,15 @@ async fn file_system_remove_with_sandbox_policy_removes_symlink_not_target( let symlink_path = allowed_dir.join("link"); symlink(&outside_file, &symlink_path)?; - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir); + let sandbox = workspace_write_sandbox(allowed_dir); file_system - .remove_with_sandbox_policy( + .remove( &absolute_path(symlink_path.clone()), RemoveOptions { recursive: false, force: false, }, - Some(&sandbox_policy), + Some(&sandbox), ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -630,9 +610,7 @@ async fn file_system_remove_with_sandbox_policy_removes_symlink_not_target( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_copy_with_sandbox_policy_preserves_symlink_source( - use_remote: bool, -) -> Result<()> { +async fn file_system_copy_preserves_symlink_source(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -647,13 +625,13 @@ async fn file_system_copy_with_sandbox_policy_preserves_symlink_source( std::fs::write(&outside_file, "outside")?; symlink(&outside_file, &source_symlink)?; - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir.clone()); + let sandbox = workspace_write_sandbox(allowed_dir.clone()); file_system - .copy_with_sandbox_policy( + .copy( &absolute_path(source_symlink), &absolute_path(copied_symlink.clone()), CopyOptions { recursive: false }, - Some(&sandbox_policy), + Some(&sandbox), ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -668,9 +646,7 @@ async fn file_system_copy_with_sandbox_policy_preserves_symlink_source( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_remove_with_sandbox_policy_rejects_symlink_escape( - use_remote: bool, -) -> Result<()> { +async fn file_system_remove_rejects_symlink_escape(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -684,29 +660,22 @@ async fn file_system_remove_with_sandbox_policy_rejects_symlink_escape( symlink(&outside_dir, allowed_dir.join("link"))?; let requested_path = allowed_dir.join("link").join("secret.txt"); - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir); + let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system - .remove_with_sandbox_policy( + .remove( &absolute_path(requested_path.clone()), RemoveOptions { recursive: false, force: false, }, - Some(&sandbox_policy), + Some(&sandbox), ) .await { Ok(()) => anyhow::bail!("remove should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/write is not permitted for path {}", - requested_path.display() - ) - ); + assert_sandbox_denied(&error); assert_eq!(std::fs::read_to_string(outside_file)?, "outside"); Ok(()) @@ -715,9 +684,7 @@ async fn file_system_remove_with_sandbox_policy_rejects_symlink_escape( #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_copy_with_sandbox_policy_rejects_symlink_escape_source( - use_remote: bool, -) -> Result<()> { +async fn file_system_copy_rejects_symlink_escape_source(use_remote: bool) -> Result<()> { let context = create_file_system_context(use_remote).await?; let file_system = context.file_system; @@ -732,27 +699,20 @@ async fn file_system_copy_with_sandbox_policy_rejects_symlink_escape_source( symlink(&outside_dir, allowed_dir.join("link"))?; let requested_source = allowed_dir.join("link").join("secret.txt"); - let sandbox_policy = workspace_write_sandbox_policy(allowed_dir); + let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system - .copy_with_sandbox_policy( + .copy( &absolute_path(requested_source.clone()), &absolute_path(requested_destination.clone()), CopyOptions { recursive: false }, - Some(&sandbox_policy), + Some(&sandbox), ) .await { Ok(()) => anyhow::bail!("copy should be blocked"), Err(error) => error, }; - assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); - assert_eq!( - error.to_string(), - format!( - "fs/read is not permitted for path {}", - requested_source.display() - ) - ); + assert_sandbox_denied(&error); assert!(!requested_destination.exists()); Ok(()) @@ -776,6 +736,7 @@ async fn file_system_copy_rejects_copying_directory_into_descendant( &absolute_path(source_dir.clone()), &absolute_path(source_dir.join("nested").join("copy")), CopyOptions { recursive: true }, + /*sandbox*/ None, ) .await; let error = match error { @@ -810,6 +771,7 @@ async fn file_system_copy_preserves_symlinks_in_recursive_copy(use_remote: bool) &absolute_path(source_dir), &absolute_path(copied_dir.clone()), CopyOptions { recursive: true }, + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -855,6 +817,7 @@ async fn file_system_copy_ignores_unknown_special_files_in_recursive_copy( &absolute_path(source_dir), &absolute_path(copied_dir.clone()), CopyOptions { recursive: true }, + /*sandbox*/ None, ) .await .with_context(|| format!("mode={use_remote}"))?; @@ -891,6 +854,7 @@ async fn file_system_copy_rejects_standalone_fifo_source(use_remote: bool) -> Re &absolute_path(fifo_path), &absolute_path(tmp.path().join("copied")), CopyOptions { recursive: false }, + /*sandbox*/ None, ) .await; let error = match error { diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 31099e8d6..548eef183 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -15,6 +15,7 @@ pub use cli::Command; pub use cli::ReviewArgs; use codex_app_server_client::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; use codex_app_server_client::EnvironmentManager; +use codex_app_server_client::ExecServerRuntimePaths; use codex_app_server_client::InProcessAppServerClient; use codex_app_server_client::InProcessClientStartArgs; use codex_app_server_client::InProcessServerEvent; @@ -469,6 +470,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result range: None, }) .collect(); + let local_runtime_paths = ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?; let in_process_start_args = InProcessClientStartArgs { arg0_paths, config: std::sync::Arc::new(config.clone()), @@ -476,7 +481,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result loader_overrides: run_loader_overrides, cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), - environment_manager: std::sync::Arc::new(EnvironmentManager::from_env()), + environment_manager: std::sync::Arc::new(EnvironmentManager::from_env_with_runtime_paths( + Some(local_runtime_paths), + )), config_warnings, session_source: SessionSource::Exec, enable_codex_api_key_env: true, diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index ccaaba3cd..1320fd1b6 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use codex_arg0::Arg0DispatchPaths; use codex_core::config::Config; use codex_exec_server::EnvironmentManager; +use codex_exec_server::ExecServerRuntimePaths; use codex_login::default_client::set_default_client_residency_requirement; use codex_utils_cli::CliConfigOverrides; @@ -58,7 +59,12 @@ pub async fn run_main( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::from_env()); + let environment_manager = Arc::new(EnvironmentManager::from_env_with_runtime_paths(Some( + ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?, + ))); // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. let cli_kv_overrides = cli_config_overrides.parse_overrides().map_err(|e| { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 1e3b58452..f2f355b9f 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -39,6 +39,7 @@ use codex_app_server_protocol::ThreadSortKey as AppServerThreadSortKey; use codex_app_server_protocol::ThreadSourceKind; use codex_cloud_requirements::cloud_requirements_loader_for_storage; use codex_exec_server::EnvironmentManager; +use codex_exec_server::ExecServerRuntimePaths; use codex_login::AuthConfig; use codex_login::default_client::set_default_client_residency_requirement; use codex_login::enforce_login_restrictions; @@ -722,7 +723,12 @@ pub async fn run_main( } }; - let environment_manager = Arc::new(EnvironmentManager::from_env()); + let environment_manager = Arc::new(EnvironmentManager::from_env_with_runtime_paths(Some( + ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?, + ))); let cwd = cli.cwd.clone(); let config_cwd = config_cwd_for_app_server_target(cwd.as_deref(), &app_server_target, &environment_manager)?; diff --git a/justfile b/justfile index 43afbf93d..f0f14f419 100644 --- a/justfile +++ b/justfile @@ -14,7 +14,7 @@ codex *args: exec *args: cargo run --bin codex -- exec "$@" -# Start codex-exec-server and run codex-tui. +# Start `codex exec-server` and run codex-tui. [no-cd] tui-with-exec-server *args: ./scripts/run_tui_with_exec_server.sh "$@" diff --git a/scripts/run_tui_with_exec_server.sh b/scripts/run_tui_with_exec_server.sh index 669cdee59..db926fc74 100755 --- a/scripts/run_tui_with_exec_server.sh +++ b/scripts/run_tui_with_exec_server.sh @@ -24,7 +24,7 @@ trap cleanup EXIT INT TERM HUP ( cd "$cargo_root" - cargo run -p codex-exec-server -- --listen "$listen_url" + cargo run -p codex-cli --bin codex -- exec-server --listen "$listen_url" ) >"$stdout_log" 2>"$stderr_log" & server_pid="$!" @@ -40,7 +40,7 @@ for _ in $(seq 1 "$((start_timeout_seconds * 20))"); do if ! kill -0 "$server_pid" >/dev/null 2>&1; then cat "$stderr_log" >&2 || true cat "$stdout_log" >&2 || true - echo "failed to start codex-exec-server" >&2 + echo "failed to start codex exec-server" >&2 exit 1 fi @@ -50,7 +50,7 @@ done if [[ -z "$exec_server_url" ]]; then cat "$stderr_log" >&2 || true cat "$stdout_log" >&2 || true - echo "timed out waiting ${start_timeout_seconds}s for codex-exec-server to report its websocket URL" >&2 + echo "timed out waiting ${start_timeout_seconds}s for codex exec-server to report its websocket URL" >&2 exit 1 fi diff --git a/scripts/start-codex-exec.sh b/scripts/start-codex-exec.sh index 7102d1d08..5699c13ef 100755 --- a/scripts/start-codex-exec.sh +++ b/scripts/start-codex-exec.sh @@ -105,10 +105,10 @@ remote_repo_root="$HOME/code/codex-sync" remote_codex_rs="$remote_repo_root/codex-rs" cd "${remote_codex_rs}" -cargo build -p codex-exec-server --bin codex-exec-server +cargo build -p codex-cli --bin codex rm -f "${remote_exec_server_log_path}" "${remote_exec_server_pid_path}" -nohup ./target/debug/codex-exec-server --listen ws://127.0.0.1:0 \ +nohup ./target/debug/codex exec-server --listen ws://127.0.0.1:0 \ >"${remote_exec_server_log_path}" 2>&1 & remote_exec_server_pid="$!" echo "${remote_exec_server_pid}" >"${remote_exec_server_pid_path}" diff --git a/scripts/test-remote-env.sh b/scripts/test-remote-env.sh index 833be978c..bdccf4d7d 100755 --- a/scripts/test-remote-env.sh +++ b/scripts/test-remote-env.sh @@ -17,10 +17,10 @@ is_sourced() { setup_remote_env() { local container_name - local codex_exec_server_binary_path + local codex_binary_path container_name="${CODEX_TEST_REMOTE_ENV_CONTAINER_NAME:-codex-remote-test-env-local-$(date +%s)-${RANDOM}}" - codex_exec_server_binary_path="${REPO_ROOT}/codex-rs/target/debug/codex-exec-server" + codex_binary_path="${REPO_ROOT}/codex-rs/target/debug/codex" if ! command -v docker >/dev/null 2>&1; then echo "docker is required (Colima or Docker Desktop)" >&2 @@ -33,17 +33,17 @@ setup_remote_env() { fi if ! command -v cargo >/dev/null 2>&1; then - echo "cargo is required to build codex-exec-server" >&2 + echo "cargo is required to build codex" >&2 return 1 fi ( cd "${REPO_ROOT}/codex-rs" - cargo build -p codex-exec-server --bin codex-exec-server + cargo build -p codex-cli --bin codex ) - if [[ ! -f "${codex_exec_server_binary_path}" ]]; then - echo "codex-exec-server binary not found at ${codex_exec_server_binary_path}" >&2 + if [[ ! -f "${codex_binary_path}" ]]; then + echo "codex binary not found at ${codex_binary_path}" >&2 return 1 fi