mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
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
This commit is contained in:
@@ -1008,9 +1008,11 @@ impl Environment {
|
||||
) -> Result<EnvironmentConfigReadResponse, ExecServerError> {
|
||||
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()))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<bool>,
|
||||
) -> Result<EnvironmentConfigReadResponse, ReadEnvironmentConfigError> {
|
||||
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<bool>,
|
||||
) {
|
||||
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<S>(
|
||||
cloud_insertion_index: stack.cloud_insertion_index,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "environment_config_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -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::<Vec<_>>(),
|
||||
[&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(())
|
||||
}
|
||||
@@ -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<bool>,
|
||||
/// User-config opt-out of writable-root symlink checks beneath this host's home.
|
||||
#[cfg(target_os = "macos")]
|
||||
pub allowed_symlinked_codex_home: Option<AbsolutePathBuf>,
|
||||
@@ -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")]
|
||||
|
||||
@@ -209,12 +209,16 @@ impl ExecServerHandler {
|
||||
params: EnvironmentConfigReadParams,
|
||||
) -> Result<EnvironmentConfigReadResponse, JSONRPCErrorError> {
|
||||
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<EnvironmentStatus, JSONRPCErrorError> {
|
||||
|
||||
Reference in New Issue
Block a user