diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d7bb37928..a03e10907 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3665,6 +3665,7 @@ dependencies = [ "codex-secrets", "codex-utils-cargo-bin", "codex-utils-home-dir", + "codex-utils-path-uri", "codex-utils-pty", "futures", "keyring", diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index d744a72c9..a44cc5253 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -57,6 +57,7 @@ use codex_protocol::protocol::ExecCommandSource; use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::approx_token_count; +use codex_utils_path_uri::PathUri; const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [ ("NO_COLOR", "1"), @@ -156,7 +157,7 @@ fn exec_server_params_for_request( codex_exec_server::ExecParams { process_id: exec_server_process_id(process_id).into(), argv: request.command.clone(), - cwd: request.cwd.to_path_buf(), + cwd: PathUri::from_abs_path(&request.cwd), env_policy, env, tty, diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index 9aa0e7381..d9b3a771b 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -66,7 +66,7 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() { } #[test] -fn exec_server_params_use_env_policy_overlay_contract() { +fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() { let cwd: codex_utils_absolute_path::AbsolutePathBuf = std::env::current_dir() .expect("current dir") .try_into() @@ -115,6 +115,7 @@ fn exec_server_params_use_env_policy_overlay_contract() { exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true); assert_eq!(params.process_id.as_str(), "123"); + assert_eq!(params.cwd, PathUri::from_abs_path(&request.cwd)); assert!(params.env_policy.is_some()); assert_eq!( params.env, diff --git a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs index aa70ab1a5..b505227b6 100644 --- a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs +++ b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs @@ -7,8 +7,6 @@ use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::ExecCommandSource; -use codex_protocol::protocol::ExecCommandStatus; use codex_protocol::protocol::Op; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::protocol::TurnEnvironmentSelections; @@ -23,7 +21,6 @@ use core_test_support::responses::start_mock_server; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; -use pretty_assertions::assert_eq; use serde_json::json; use tokio::io::AsyncBufReadExt; use tokio::io::BufReader; @@ -33,7 +30,7 @@ const CALL_ID: &str = "wine-cmd-smoke"; const COMMAND: &str = "echo WINE_BAZEL_OK&&cd"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn windows_exec_server_records_host_shell_mismatch() -> Result<()> { +async fn windows_exec_server_rejects_non_native_cwd_uri() -> Result<()> { let executable = codex_utils_cargo_bin::cargo_bin("wine-windows-exec-server")?; let mut exec_server = WineTestCommand::new(executable) .env("CODEX_HOME", r"C:\codex-home") @@ -124,79 +121,36 @@ async fn windows_exec_server_records_host_shell_mismatch() -> Result<()> { }) .await?; - let mut begin = None; - let mut end = None; + let mut saw_exec_event = false; loop { match wait_for_event(&test.codex, |_| true).await { EventMsg::ExecCommandBegin(event) if event.call_id == CALL_ID => { - begin = Some(event) + saw_exec_event = true } EventMsg::ExecCommandEnd(event) if event.call_id == CALL_ID => { - end = Some(event) + saw_exec_event = true } EventMsg::TurnComplete(_) => break, _ => {} } } - let begin = begin.context("exec_command should emit a begin event")?; - let expected_commands = [ - vec![ - "/bin/bash".to_string(), - "-c".to_string(), - COMMAND.to_string(), - ], - vec!["/bin/sh".to_string(), "-c".to_string(), COMMAND.to_string()], - ]; - // This intentionally records the current cross-OS failure mode: the Linux - // orchestrator resolves its own shell before sending the command to the - // Windows exec-server, where that Unix shell cannot start. assert!( - expected_commands.contains(&begin.command), - "unexpected command: {:?}", - begin.command, - ); - assert_eq!( - (begin.cwd.clone(), begin.source), - ( - test.config.cwd.clone(), - ExecCommandSource::UnifiedExecStartup, - ), - ); - - let end = end.context("exec_command should emit an end event")?; - assert_eq!( - ( - end.command, - end.cwd, - end.source, - end.stdout, - end.stderr, - end.aggregated_output, - end.exit_code, - end.status, - ), - ( - begin.command, - test.config.cwd.clone(), - ExecCommandSource::UnifiedExecStartup, - String::new(), - String::new(), - String::new(), - -1, - ExecCommandStatus::Failed, - ), + !saw_exec_event, + "a non-native cwd should be rejected before process lifecycle events", ); let request = response_mock .last_request() - .context("model should receive the failed command output")?; + .context("model should receive the rejected command output")?; let (output, success) = request .function_call_output_content_and_success(CALL_ID) - .context("failed command output should be present")?; - let output = output.context("failed command output should contain text")?; + .context("rejected command output should be present")?; + let output = output.context("rejected command output should contain text")?; assert!( - output.contains("Process exited with code -1"), + output.contains("exec-server rejected request (-32602)") + && output.contains("cwd URI") + && output.contains("is not valid on this exec-server host"), "unexpected command output: {output:?}", ); assert_ne!(success, Some(true)); diff --git a/codex-rs/exec-server/README.md b/codex-rs/exec-server/README.md index 208e53bc2..54b00ea92 100644 --- a/codex-rs/exec-server/README.md +++ b/codex-rs/exec-server/README.md @@ -157,7 +157,7 @@ Request params: { "processId": "proc-1", "argv": ["bash", "-lc", "printf 'hello\\n'"], - "cwd": "/absolute/working/directory", + "cwd": "file:///absolute/working/directory", "env": { "PATH": "/usr/bin:/bin" }, @@ -171,7 +171,7 @@ Field definitions: - `processId`: caller-chosen stable id for this process within the connection. - `argv`: command vector. It must be non-empty. -- `cwd`: absolute working directory used for the child process. +- `cwd`: `file:` URI for the child process working directory. - `env`: environment variables passed to the child process. - `tty`: when `true`, spawn a PTY-backed interactive process. - `pipeStdin`: when `true`, keep non-PTY stdin writable via `process/write`. @@ -409,7 +409,7 @@ Initialize: Start a process: ```json -{"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,"pipeStdin":false,"arg0":null}} +{"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":"file:///tmp","env":{"PATH":"/usr/bin:/bin"},"tty":true,"pipeStdin":false,"arg0":null}} {"id":2,"result":{"processId":"proc-1"}} {"method":"process/output","params":{"processId":"proc-1","seq":1,"stream":"stdout","chunk":"cmVhZHkK"}} ``` diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 688c421dd..d4cfef454 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -498,6 +498,7 @@ mod tests { use crate::ProcessId; use crate::environment_provider::EnvironmentDefault; use crate::environment_provider::EnvironmentProviderSnapshot; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; fn test_runtime_paths() -> ExecServerRuntimePaths { @@ -862,7 +863,8 @@ mod tests { .start(crate::ExecParams { process_id: ProcessId::from("default-env-proc"), argv: vec!["true".to_string()], - cwd: std::env::current_dir().expect("read current dir"), + cwd: PathUri::from_path(std::env::current_dir().expect("read current dir")) + .expect("cwd URI"), env_policy: None, env: Default::default(), tty: false, diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 60e6b8d91..78a53897e 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -156,6 +156,12 @@ impl LocalProcess { .argv .split_first() .ok_or_else(|| invalid_params("argv must not be empty".to_string()))?; + let native_cwd = params.cwd.to_abs_path().map_err(|err| { + invalid_params(format!( + "cwd URI `{}` is not valid on this exec-server host: {err}", + params.cwd + )) + })?; { let mut process_map = self.inner.processes.lock().await; @@ -172,7 +178,7 @@ impl LocalProcess { codex_utils_pty::spawn_pty_process( program, args, - params.cwd.as_path(), + native_cwd.as_path(), &env, ¶ms.arg0, TerminalSize::default(), @@ -182,7 +188,7 @@ impl LocalProcess { codex_utils_pty::spawn_pipe_process( program, args, - params.cwd.as_path(), + native_cwd.as_path(), &env, ¶ms.arg0, ) @@ -191,7 +197,7 @@ impl LocalProcess { codex_utils_pty::spawn_pipe_process_no_stdin( program, args, - params.cwd.as_path(), + native_cwd.as_path(), &env, ¶ms.arg0, ) @@ -783,6 +789,7 @@ fn notification_sender(inner: &Inner) -> Option { mod tests { use super::*; use codex_protocol::config_types::ShellEnvironmentPolicyInherit; + use codex_utils_path_uri::PathUri; use codex_utils_pty::ProcessDriver; use pretty_assertions::assert_eq; use tokio::sync::oneshot; @@ -792,7 +799,7 @@ mod tests { ExecParams { process_id: ProcessId::from("env-test"), argv: vec!["true".to_string()], - cwd: std::path::PathBuf::from("/tmp"), + cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"), env_policy: None, env, tty: false, @@ -801,6 +808,30 @@ mod tests { } } + #[tokio::test] + async fn start_process_rejects_non_native_cwd_before_launch() { + #[cfg(unix)] + let uri = "file://server/share/checkout"; + #[cfg(windows)] + let uri = "file:///usr/local/checkout"; + let cwd = PathUri::parse(uri).expect("non-native cwd URI"); + let source = cwd + .to_abs_path() + .expect_err("cwd should not be native to this host"); + let expected = invalid_params(format!( + "cwd URI `{cwd}` is not valid on this exec-server host: {source}" + )); + let mut params = test_exec_params(HashMap::new()); + params.cwd = cwd; + + let result = LocalProcess::default().start_process(params).await; + let Err(error) = result else { + panic!("non-native cwd should be rejected"); + }; + + assert_eq!(error, expected); + } + #[test] fn child_env_defaults_to_exact_env() { let params = test_exec_params(HashMap::from([("ONLY_THIS".to_string(), "1".to_string())])); diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 175d23fe0..ee116ef08 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::path::PathBuf; use crate::FileSystemSandboxContext; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; @@ -77,7 +76,8 @@ pub struct EnvironmentInfo { pub struct ShellInfo { /// Stable shell name, for example `zsh`, `bash`, `powershell`, `sh`, or `cmd`. pub name: String, - /// Path the exec server would use for that shell. + /// Target-native shell executable path or command name. Fallbacks such as `cmd.exe` need not + /// be absolute, so this is not a [`PathUri`]. pub path: String, } @@ -88,7 +88,8 @@ pub struct ExecParams { /// This is a protocol key, not an OS pid. pub process_id: ProcessId, pub argv: Vec, - pub cwd: PathBuf, + /// Working directory URI, interpreted using the exec-server host's path rules at launch time. + pub cwd: PathUri, #[serde(default)] pub env_policy: Option, pub env: HashMap, @@ -96,6 +97,8 @@ pub struct ExecParams { /// Keep non-tty stdin writable through `process/write`. #[serde(default)] pub pipe_stdin: bool, + /// Optional process-visible argv0 override. Values such as `codex-linux-sandbox` are command + /// names rather than paths, so this is not a [`PathUri`]. pub arg0: Option, } diff --git a/codex-rs/exec-server/src/server/handler/tests.rs b/codex-rs/exec-server/src/server/handler/tests.rs index 6b632fe89..97bfba534 100644 --- a/codex-rs/exec-server/src/server/handler/tests.rs +++ b/codex-rs/exec-server/src/server/handler/tests.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tokio::sync::mpsc; use uuid::Uuid; @@ -26,7 +27,7 @@ fn exec_params_with_argv(process_id: &str, argv: Vec) -> ExecParams { ExecParams { process_id: ProcessId::from(process_id), argv, - cwd: std::env::current_dir().expect("cwd"), + cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"), env_policy: None, env: inherited_path_env(), tty: false, diff --git a/codex-rs/exec-server/src/server/processor.rs b/codex-rs/exec-server/src/server/processor.rs index 6fc0723f0..952f41421 100644 --- a/codex-rs/exec-server/src/server/processor.rs +++ b/codex-rs/exec-server/src/server/processor.rs @@ -195,6 +195,7 @@ mod tests { use codex_app_server_protocol::JSONRPCRequest; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; + use codex_utils_path_uri::PathUri; use serde::Serialize; use serde::de::DeserializeOwned; use tokio::io::AsyncBufReadExt; @@ -396,7 +397,7 @@ mod tests { ExecParams { process_id, argv: sleep_then_print_argv(), - cwd: std::env::current_dir().expect("cwd"), + cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"), env_policy: None, env, tty: false, diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index af8e722c6..e5522f407 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -15,6 +15,7 @@ use codex_exec_server::ProcessSignal; use codex_exec_server::ReadResponse; use codex_exec_server::StartedExecProcess; use codex_exec_server::WriteStatus; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tempfile::TempDir; use test_case::test_case; @@ -72,7 +73,7 @@ async fn assert_exec_process_starts_and_exits(use_remote: bool) -> Result<()> { .start(ExecParams { process_id: ProcessId::from("proc-1"), argv: vec!["true".to_string()], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -213,7 +214,7 @@ async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> { "-c".to_string(), "sleep 0.05; printf 'session output\\n'".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -244,7 +245,7 @@ async fn assert_exec_process_pushes_events(use_remote: bool) -> Result<()> { "-c".to_string(), "printf 'event output\\n'; sleep 0.1; printf 'event err\\n' >&2; sleep 0.1; exit 7".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -291,7 +292,7 @@ async fn assert_exec_process_replays_events_after_close(use_remote: bool) -> Res "-c".to_string(), "printf 'late one\\n'; printf 'late two\\n'".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -339,7 +340,7 @@ async fn assert_exec_process_retains_output_after_exit_until_streams_close( DELAYED_OUTPUT_AFTER_EXIT_PARENT_ARG.to_string(), release_path.to_string_lossy().into_owned(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -412,7 +413,7 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> { "-c".to_string(), "IFS= read line; printf 'from-stdin:%s\\n' \"$line\"".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: true, @@ -449,7 +450,7 @@ async fn assert_exec_process_write_then_read_without_tty(use_remote: bool) -> Re "-c".to_string(), "IFS= read line; printf 'from-stdin:%s\\n' \"$line\"".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -482,7 +483,7 @@ async fn assert_exec_process_rejects_write_without_pipe_stdin(use_remote: bool) "-c".to_string(), "sleep 0.3; if IFS= read -r line; then printf 'read:%s\\n' \"$line\"; else printf 'eof\\n'; fi".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -516,7 +517,7 @@ async fn assert_exec_process_signal_interrupts_process(use_remote: bool) -> Resu "-c".to_string(), "trap 'printf \"signal:2\\n\"; exit 7' INT; printf 'ready\\n'; while :; do :; done".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -569,7 +570,7 @@ async fn assert_exec_process_signal_reports_unsupported_on_windows(use_remote: b "/C".to_string(), "echo ready && ping -n 30 127.0.0.1 >NUL".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -609,7 +610,7 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe( "-c".to_string(), "printf 'queued output\\n'".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -644,7 +645,7 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> { "-c".to_string(), "sleep 10".to_string(), ], - cwd: std::env::current_dir()?, + cwd: PathUri::from_path(std::env::current_dir()?)?, env_policy: /*env_policy*/ None, env: Default::default(), tty: false, diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index c355e0ee6..44f3eb139 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -22,6 +22,7 @@ codex-exec-server = { workspace = true } codex-keyring-store = { workspace = true } codex-protocol = { workspace = true } codex-secrets = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-home-dir = { workspace = true } bytes = { workspace = true } diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 8b3250669..8ada806ff 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -35,6 +35,7 @@ use codex_exec_server::ExecEnvPolicy; use codex_exec_server::ExecParams; use codex_exec_server::ExecProcess; use codex_protocol::config_types::ShellEnvironmentPolicyInherit; +use codex_utils_path_uri::PathUri; #[cfg(unix)] use codex_utils_pty::process_group::kill_process_group; #[cfg(unix)] @@ -487,6 +488,7 @@ impl ExecutorStdioServerLauncher { // before sending an executor request. let argv = Self::process_api_argv(&program, &args).map_err(io::Error::other)?; let env = Self::process_api_env(envs).map_err(io::Error::other)?; + let cwd = PathUri::from_path(cwd)?; let process_id = ExecutorProcessTransport::next_process_id(); // Start the MCP server process on the executor with raw pipes. `tty=false` // keeps stdout as a clean protocol stream, while `pipe_stdin=true` lets