From ed59a6c1cdf5e6fc96351fd46dfc1ef8a16db385 Mon Sep 17 00:00:00 2001 From: zm-oai Date: Wed, 7 Oct 2026 00:18:40 +0000 Subject: [PATCH] Preserve the CLI MXC preference in executor config reads (#51525) ## Why Executor config reads omitted the startup `features.prefer_mxc` override, so clients could not see the CLI sandbox preference. ## What changed - Include the explicit CLI `features.prefer_mxc` value in `environmentConfig/read` as a `session-flags` layer, above project config and below legacy managed file and MDM config. - Retain only this preference from startup overrides. - Expose `windows_mxc_allowed_by_config` to share MXC eligibility checks after callers resolve the active permission profile. ## Testing Add CLI and remote executor coverage for preference propagation and filtering, plus a unit test verifying that both legacy managed config sources retain precedence. GitOrigin-RevId: 5ac6f5257f1e807c2f197dcf5dcb2385cedca0ea --- codex-rs/cli/src/exec_server_command.rs | 11 ++- codex-rs/cli/tests/exec_server.rs | 73 ++++++++++++++++++- codex-rs/core/src/config/mod.rs | 1 + .../core/src/config/windows_sandbox_config.rs | 26 ++++++- .../tests/suite/mcp_executor_context_tests.rs | 2 +- codex-rs/exec-server/src/environment.rs | 8 +- .../exec-server/src/environment_config.rs | 42 ++++++++++- .../src/environment_config_tests.rs | 52 +++++++++++++ codex-rs/exec-server/src/runtime_options.rs | 3 + codex-rs/exec-server/src/server/handler.rs | 16 ++-- codex-rs/exec-server/tests/common/mod.rs | 10 ++- .../exec-server/tests/environment_config.rs | 29 +++++++- codex-rs/ext/extension-api/tests/registry.rs | 2 + 13 files changed, 256 insertions(+), 19 deletions(-) create mode 100644 codex-rs/exec-server/src/environment_config_tests.rs diff --git a/codex-rs/cli/src/exec_server_command.rs b/codex-rs/cli/src/exec_server_command.rs index 588a1f05b4..568ceaf98c 100644 --- a/codex-rs/cli/src/exec_server_command.rs +++ b/codex-rs/cli/src/exec_server_command.rs @@ -174,12 +174,21 @@ impl ExecServerCommand { .codex_self_exe .clone() .ok_or_else(|| anyhow::anyhow!("Codex executable path is not configured"))?; - let runtime_paths = ExecServerRuntimeOptions::new( + let mut runtime_paths = ExecServerRuntimeOptions::new( codex_self_exe, arg0_paths.codex_linux_sandbox_exe.clone(), )? .with_linux_sandbox_pid_namespace(self.linux_sandbox_pid_namespace) .with_proxy_private_ips_via_upstream(self.proxy_private_ips_via_upstream); + // Config loading below owns error handling (non-strict stdio can proceed + // without config). Never retain other startup flags in the executor. + if let Ok(overrides) = root_config_overrides.parse_overrides() { + let cli_flags = codex_config::build_cli_overrides_layer(&overrides); + runtime_paths.prefer_mxc = cli_flags + .get("features") + .and_then(|features| features.get("prefer_mxc")) + .and_then(toml::Value::as_bool); + } if let Some(base_url) = self.remote.take() { let environment_id = self.environment_id.take().ok_or_else(|| { anyhow::anyhow!("--environment-id is required when --remote is set") diff --git a/codex-rs/cli/tests/exec_server.rs b/codex-rs/cli/tests/exec_server.rs index 36c16bb565..8f4f8b742e 100644 --- a/codex-rs/cli/tests/exec_server.rs +++ b/codex-rs/cli/tests/exec_server.rs @@ -97,6 +97,75 @@ fn local_exec_server_ignores_invalid_config_without_strict_config() -> Result<() Ok(()) } +/// Real global CLI flags must reach executor config reads; unrelated startup flags stay private. +#[tokio::test] +async fn local_exec_server_projects_global_mxc_preference() -> Result<()> { + let home = TempDir::new()?; + let cwd = url::Url::from_directory_path(home.path()) + .map_err(|()| anyhow::anyhow!("could not convert home to file URL"))?; + let mut child = tokio::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) + .env("CODEX_HOME", home.path()) + .args([ + "-c", + "features.prefer_mxc=true", + "-c", + "model='private-startup-model'", + "exec-server", + "--listen", + "stdio", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .kill_on_drop(true) + .spawn()?; + let mut stdin = child.stdin.take().context("exec-server stdin")?; + let mut stdout = BufReader::new(child.stdout.take().context("exec-server stdout")?); + let result = tokio::time::timeout(Duration::from_secs(30), async { + send_json_line( + &mut stdin, + &serde_json::json!({"id": 1, "method": "initialize", + "params": {"clientName": "mxc-test", "resumeSessionId": null}}), + ) + .await?; + wait_for_response(&mut stdout, /*expected_id*/ 1).await?; + send_json_line( + &mut stdin, + &serde_json::json!({"method": "initialized", "params": {}}), + ) + .await?; + send_json_line( + &mut stdin, + &serde_json::json!({"id": 2, "method": "environmentConfig/read", "params": { + "cwd": cwd, "configPaths": [["features", "prefer_mxc"], ["model"]], + "requirementsPaths": [] + }}), + ) + .await?; + wait_for_response(&mut stdout, /*expected_id*/ 2).await + }) + .await + .context("executor CLI config read timed out")??; + let layers = result["result"]["config"]["layers"] + .as_array() + .context("config layers")?; + let session = layers + .iter() + .find(|l| l["source"] == "session-flags") + .context("session flags")?; + let value: toml::Value = toml::from_str(session["toml"].as_str().context("session TOML")?)?; + assert_eq!( + value, + toml::Value::Table(toml::toml! { [features] prefer_mxc = true }) + ); + drop(stdin); + assert!( + tokio::time::timeout(Duration::from_secs(30), child.wait()) + .await?? + .success() + ); + Ok(()) +} + /// The standalone exec-server accepts an explicit per-connection concurrency limit. #[test] fn local_exec_server_accepts_concurrent_requests_flag() -> Result<()> { @@ -617,7 +686,7 @@ fn local_exec_server_exits_successfully_on_sigterm() -> Result<()> { async fn wait_for_response( stdout: &mut (impl tokio::io::AsyncBufRead + Unpin), expected_id: i64, -) -> Result<()> { +) -> Result { loop { let mut line = String::new(); if stdout.read_line(&mut line).await? == 0 { @@ -629,7 +698,7 @@ async fn wait_for_response( message.get("error").is_none(), "exec-server request {expected_id} failed: {message}" ); - return Ok(()); + return Ok(message); } } } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 15971c70e1..1eb2cdf83f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -221,6 +221,7 @@ pub use windows_sandbox_config::PreparedWindowsSandboxConfig; use windows_sandbox_config::config_allows_mxc; pub use windows_sandbox_config::prepare_windows_sandbox_config; use windows_sandbox_config::resolve_windows_sandbox_type; +pub use windows_sandbox_config::windows_mxc_allowed_by_config; const DEFAULT_IGNORE_LARGE_UNTRACKED_DIRS: i64 = 200; const DEFAULT_IGNORE_LARGE_UNTRACKED_FILES: i64 = 10 * 1024 * 1024; diff --git a/codex-rs/core/src/config/windows_sandbox_config.rs b/codex-rs/core/src/config/windows_sandbox_config.rs index 75ff2013c5..dbe88ba9f1 100644 --- a/codex-rs/core/src/config/windows_sandbox_config.rs +++ b/codex-rs/core/src/config/windows_sandbox_config.rs @@ -120,6 +120,30 @@ pub(super) fn config_allows_mxc( } else { None }; + Ok(windows_mxc_allowed_by_config( + windows_sandbox_mode, + network_requirements, + features, + enable_network_proxy, + profile_local_binding, + )) +} + +/// Eligibility shared with remote selection after the caller resolves its active profile. +/// Managed network requirements outrank feature and profile binding restrictions. +pub fn windows_mxc_allowed_by_config( + windows_sandbox_mode: &ConstrainedWithSource>, + network_requirements: Option<&Sourced>, + features: Option<&FeaturesToml>, + enable_network_proxy: bool, + profile_local_binding: Option, +) -> bool { + if windows_sandbox_mode + .can_set(&Some(WindowsSandboxModeToml::Mxc)) + .is_err() + { + return false; + } let allow_local_binding = network_requirements .and_then(|requirements| requirements.value.allow_local_binding) .or_else(|| { @@ -128,7 +152,7 @@ pub(super) fn config_allows_mxc( .and_then(|config| config.allow_local_binding) }) .or(profile_local_binding); - Ok(allow_local_binding != Some(false)) + allow_local_binding != Some(false) } #[cfg(test)] diff --git a/codex-rs/core/tests/suite/mcp_executor_context_tests.rs b/codex-rs/core/tests/suite/mcp_executor_context_tests.rs index 642e369052..d780d5ec4f 100644 --- a/codex-rs/core/tests/suite/mcp_executor_context_tests.rs +++ b/codex-rs/core/tests/suite/mcp_executor_context_tests.rs @@ -106,7 +106,7 @@ async fn thread_projection_preserves_executor_order_and_unavailable_selections() let selections = requests .iter() .cloned() - .map(TurnEnvironmentSelection::new) + .map(|request| TurnEnvironmentSelection::new(request, &[])) .collect::>(); recorder .observed diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 2bf43abb8c..baa0f22049 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -1008,9 +1008,11 @@ impl Environment { ) -> Result { match &self.remote_client { Some(client) => client.get().await?.read_environment_config(params).await, - None => read_environment_config(self.filesystem.as_ref(), params) - .await - .map_err(|error| ExecServerError::Protocol(error.to_string())), + None => { + read_environment_config(self.filesystem.as_ref(), params, /*prefer_mxc*/ None) + .await + .map_err(|error| ExecServerError::Protocol(error.to_string())) + } } } diff --git a/codex-rs/exec-server/src/environment_config.rs b/codex-rs/exec-server/src/environment_config.rs index b53208b63a..e738a5d135 100644 --- a/codex-rs/exec-server/src/environment_config.rs +++ b/codex-rs/exec-server/src/environment_config.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use codex_config::CONFIG_TOML_FILE; +use codex_config::ConfigLayerSource; use codex_config::McpServerConfig; use codex_config::McpServerDisabledReason; use codex_config::RequirementSource; @@ -8,6 +9,8 @@ use codex_config::RequirementsLayerEntry; use codex_config::compose_requirements_for_hostname; use codex_config::format_config_layer_source; use codex_config::host_name; +use codex_config::loader::LocalConfigLayers; +use codex_config::loader::LocalTomlLayer; use codex_config::loader::LocalTomlLayerStack; use codex_config::loader::load_local_config_layers; use codex_exec_server_protocol::EnvironmentConfigLayer; @@ -15,6 +18,7 @@ use codex_exec_server_protocol::EnvironmentConfigLayerStack; use codex_exec_server_protocol::EnvironmentConfigReadParams; use codex_exec_server_protocol::EnvironmentConfigReadResponse; use codex_file_system::ExecutorFileSystem; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_home_dir::find_codex_home; use codex_utils_path_uri::PathUri; @@ -32,6 +36,7 @@ pub(crate) enum ReadEnvironmentConfigError { pub(crate) async fn read_environment_config( file_system: &dyn ExecutorFileSystem, params: EnvironmentConfigReadParams, + prefer_mxc: Option, ) -> Result { validate_paths(¶ms)?; let cwd = params @@ -41,14 +46,15 @@ pub(crate) async fn read_environment_config( let codex_home = find_codex_home().map_err(|error| { ReadEnvironmentConfigError::Internal(format!("failed to find Codex home: {error}")) })?; - let layers = load_local_config_layers(file_system, codex_home.as_path(), &cwd) + let mut layers = load_local_config_layers(file_system, codex_home.as_path(), &cwd) .await .map_err(|error| { ReadEnvironmentConfigError::Internal(format!( "failed to load executor-local config: {error}" )) - })? - .project(¶ms.config_paths, ¶ms.requirements_paths); + })?; + include_startup_preference(&mut layers, &cwd, prefer_mxc); + let layers = layers.project(¶ms.config_paths, ¶ms.requirements_paths); Ok(EnvironmentConfigReadResponse { user_home_dir: dirs::home_dir() @@ -62,6 +68,32 @@ pub(crate) async fn read_environment_config( }) } +fn include_startup_preference( + layers: &mut LocalConfigLayers, + cwd: &AbsolutePathBuf, + prefer_mxc: Option, +) { + if let Some(prefer_mxc) = prefer_mxc { + // CLI flags follow project layers, but never outrank legacy managed/MDM. + // Insert before projection, which also adjusts the cloud insertion index. + let index = layers + .config + .layers + .partition_point(|layer| layer.source <= ConfigLayerSource::SessionFlags); + layers.config.layers.insert( + index, + LocalTomlLayer { + source: ConfigLayerSource::SessionFlags, + base_dir: cwd.clone(), + toml: codex_config::build_cli_overrides_layer(&[( + "features.prefer_mxc".to_string(), + toml::Value::Boolean(prefer_mxc), + )]), + }, + ); + } +} + impl Environment { /// Reads executor-owned HTTP MCP servers from the selected project's current config. pub async fn discover_http_mcp_servers( @@ -186,3 +218,7 @@ fn serialize_layer_stack( cloud_insertion_index: stack.cloud_insertion_index, }) } + +#[cfg(test)] +#[path = "environment_config_tests.rs"] +mod tests; diff --git a/codex-rs/exec-server/src/environment_config_tests.rs b/codex-rs/exec-server/src/environment_config_tests.rs new file mode 100644 index 0000000000..e7c5b05eeb --- /dev/null +++ b/codex-rs/exec-server/src/environment_config_tests.rs @@ -0,0 +1,52 @@ +use super::*; +use pretty_assertions::assert_eq; + +/// Legacy file and MDM values must still win after a CLI preference is projected. +#[test] +fn startup_preference_stays_below_both_managed_layers() -> anyhow::Result<()> { + let temp = tempfile::tempdir()?; + let cwd = AbsolutePathBuf::from_absolute_path(temp.path())?; + let project = ConfigLayerSource::Project { + dot_codex_folder: cwd.join(".codex"), + }; + let file = ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: cwd.join("managed_config.toml"), + }; + for managed in [file, ConfigLayerSource::LegacyManagedConfigTomlFromMdm] { + let mut layers = LocalConfigLayers { + config: LocalTomlLayerStack { + layers: vec![project.clone(), managed.clone()] + .into_iter() + .map(|source| LocalTomlLayer { + source, + base_dir: cwd.clone(), + toml: toml::Value::Table(toml::toml! { [features] prefer_mxc = false }), + }) + .collect(), + cloud_insertion_index: 0, + }, + requirements: LocalTomlLayerStack { + layers: Vec::new(), + cloud_insertion_index: 0, + }, + }; + include_startup_preference(&mut layers, &cwd, Some(true)); + let projected = layers.project(&[vec!["features".into(), "prefer_mxc".into()]], &[]); + assert_eq!( + projected + .config + .layers + .iter() + .map(|l| &l.source) + .collect::>(), + [&project, &ConfigLayerSource::SessionFlags, &managed], + ); + let mut effective = toml::Value::Table(toml::map::Map::new()); + for layer in projected.config.layers { + codex_config::merge_toml_values(&mut effective, &layer.toml); + } + assert_eq!(effective["features"]["prefer_mxc"].as_bool(), Some(false)); + assert_eq!(projected.config.cloud_insertion_index, 0); + } + Ok(()) +} diff --git a/codex-rs/exec-server/src/runtime_options.rs b/codex-rs/exec-server/src/runtime_options.rs index 812e09553f..e3146c6e57 100644 --- a/codex-rs/exec-server/src/runtime_options.rs +++ b/codex-rs/exec-server/src/runtime_options.rs @@ -17,6 +17,8 @@ pub struct ExecServerRuntimeOptions { pub linux_sandbox_pid_namespace: LinuxSandboxPidNamespace, /// Trusted startup routing; request policy still controls destination access. pub proxy_private_ips_via_upstream: bool, + /// Only the sandbox preference from CLI flags, never other startup config or secrets. + pub prefer_mxc: Option, /// User-config opt-out of writable-root symlink checks beneath this host's home. #[cfg(target_os = "macos")] pub allowed_symlinked_codex_home: Option, @@ -43,6 +45,7 @@ impl ExecServerRuntimeOptions { Ok(Self { linux_sandbox_pid_namespace: LinuxSandboxPidNamespace::default(), proxy_private_ips_via_upstream: false, + prefer_mxc: None, codex_self_exe: absolute_path(codex_self_exe)?, codex_linux_sandbox_exe: codex_linux_sandbox_exe.map(absolute_path).transpose()?, #[cfg(target_os = "macos")] diff --git a/codex-rs/exec-server/src/server/handler.rs b/codex-rs/exec-server/src/server/handler.rs index e80c8cc471..d4ecb1b07f 100644 --- a/codex-rs/exec-server/src/server/handler.rs +++ b/codex-rs/exec-server/src/server/handler.rs @@ -209,12 +209,16 @@ impl ExecServerHandler { params: EnvironmentConfigReadParams, ) -> Result { self.require_initialized_for("environment config")?; - read_environment_config(crate::LOCAL_FS.as_ref(), params) - .await - .map_err(|error| match error { - ReadEnvironmentConfigError::InvalidParams(message) => invalid_params(message), - ReadEnvironmentConfigError::Internal(message) => internal_error(message), - }) + read_environment_config( + crate::LOCAL_FS.as_ref(), + params, + self.runtime_paths.prefer_mxc, + ) + .await + .map_err(|error| match error { + ReadEnvironmentConfigError::InvalidParams(message) => invalid_params(message), + ReadEnvironmentConfigError::Internal(message) => internal_error(message), + }) } pub(crate) fn environment_status(&self) -> Result { diff --git a/codex-rs/exec-server/tests/common/mod.rs b/codex-rs/exec-server/tests/common/mod.rs index 0d7661c66f..d9affe4fdb 100644 --- a/codex-rs/exec-server/tests/common/mod.rs +++ b/codex-rs/exec-server/tests/common/mod.rs @@ -214,7 +214,7 @@ fn maybe_run_exec_server_from_test_binary(guard: Option<&TestBinaryDispatchGuard std::process::exit(1); } }; - let runtime_paths = match ExecServerRuntimeOptions::new( + let mut runtime_paths = match ExecServerRuntimeOptions::new( current_exe.clone(), linux_sandbox_exe(guard, ¤t_exe), ) { @@ -224,6 +224,14 @@ fn maybe_run_exec_server_from_test_binary(guard: Option<&TestBinaryDispatchGuard std::process::exit(1); } }; + runtime_paths.prefer_mxc = env::var("CODEX_EXEC_SERVER_TEST_PREFER_MXC") + .ok() + .map(|value| { + value.parse().unwrap_or_else(|error| { + eprintln!("invalid test MXC preference: {error}"); + std::process::exit(1); + }) + }); let runtime = match tokio::runtime::Builder::new_multi_thread() .enable_all() .build() diff --git a/codex-rs/exec-server/tests/environment_config.rs b/codex-rs/exec-server/tests/environment_config.rs index ed456f505d..2dd977c5a9 100644 --- a/codex-rs/exec-server/tests/environment_config.rs +++ b/codex-rs/exec-server/tests/environment_config.rs @@ -13,11 +13,13 @@ use codex_exec_server::ExecServerError; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use common::exec_server::exec_server; +use common::exec_server::exec_server_with_env; use pretty_assertions::assert_eq; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn remote_environment_reads_projected_executor_config() -> anyhow::Result<()> { - let mut server = exec_server().await?; + let mut server = + exec_server_with_env([("CODEX_EXEC_SERVER_TEST_PREFER_MXC", "true")], &[]).await?; let codex_home = AbsolutePathBuf::from_absolute_path(std::fs::canonicalize(server.codex_home())?)?; let config_file = codex_home.join(CONFIG_TOML_FILE); @@ -41,6 +43,9 @@ async fn remote_environment_reads_projected_executor_config() -> anyhow::Result< [future_environment] relative_path = "./executor-relative" unselected = "do not return" +[features] +prefer_mxc = false +private_startup_value = "unselected-private-value" "#, ) .await?; @@ -116,6 +121,28 @@ unselected = "do not return" .await?; assert_eq!(projects, expected_projects); + let preference = environment + .read_environment_config(EnvironmentConfigReadParams { + cwd: PathUri::from_abs_path(&project), + config_paths: vec![vec!["features".to_string(), "prefer_mxc".to_string()]], + requirements_paths: Vec::new(), + }) + .await?; + assert_eq!(preference.config.cloud_insertion_index, 0); + assert_eq!(preference.config.layers.len(), 2); + let configured = &preference.config.layers[0]; + let startup = &preference.config.layers[1]; + assert_eq!(configured.base_dir, PathUri::from_abs_path(&dot_codex)); + assert_eq!(startup.source, "session-flags"); + assert_eq!(startup.base_dir, PathUri::from_abs_path(&project)); + assert_eq!( + toml::from_str::(&configured.toml)?, + toml::Value::Table(toml::toml! { [features] prefer_mxc = false }), + ); + assert_eq!( + toml::from_str::(&startup.toml)?, + toml::Value::Table(toml::toml! { [features] prefer_mxc = true }), + ); server.shutdown().await?; Ok(()) } diff --git a/codex-rs/ext/extension-api/tests/registry.rs b/codex-rs/ext/extension-api/tests/registry.rs index 5f959ad866..90c6bad8b9 100644 --- a/codex-rs/ext/extension-api/tests/registry.rs +++ b/codex-rs/ext/extension-api/tests/registry.rs @@ -78,12 +78,14 @@ fn mcp_context_preserves_selected_executor_order_and_unavailability() { cwd: PathUri::parse("file:///workspace").expect("workspace URI"), workspace_roots: Vec::new(), config: EnvironmentConfigState::Pending, + selected_capability_roots: Default::default(), }, TurnEnvironmentSelection { environment_id: "ready-secondary".to_owned(), cwd: PathUri::parse("file:///other").expect("workspace URI"), workspace_roots: Vec::new(), config: EnvironmentConfigState::FromThread, + selected_capability_roots: Default::default(), }, ]; let context = McpServerContributionContext::global(&config);