From 92098d36e8490cad66eade1ce56d10917f7eb7a3 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 12 Dec 2025 20:01:24 +0000 Subject: [PATCH 01/79] feat: clean config loading and config api (#7924) Check the README of the `config_loader` for details --- codex-rs/Cargo.lock | 2 - codex-rs/app-server/Cargo.toml | 2 - .../app-server/src/codex_message_processor.rs | 25 +- codex-rs/app-server/src/config_api.rs | 1096 +---------------- codex-rs/core/src/config/mod.rs | 84 +- codex-rs/core/src/config/service.rs | 935 ++++++++++++++ codex-rs/core/src/config_loader/README.md | 64 + .../core/src/config_loader/fingerprint.rs | 67 + codex-rs/core/src/config_loader/layer_io.rs | 100 ++ codex-rs/core/src/config_loader/merge.rs | 18 + codex-rs/core/src/config_loader/mod.rs | 369 +----- codex-rs/core/src/config_loader/overrides.rs | 55 + codex-rs/core/src/config_loader/state.rs | 128 ++ codex-rs/core/src/config_loader/tests.rs | 138 +++ 14 files changed, 1596 insertions(+), 1487 deletions(-) create mode 100644 codex-rs/core/src/config/service.rs create mode 100644 codex-rs/core/src/config_loader/README.md create mode 100644 codex-rs/core/src/config_loader/fingerprint.rs create mode 100644 codex-rs/core/src/config_loader/layer_io.rs create mode 100644 codex-rs/core/src/config_loader/merge.rs create mode 100644 codex-rs/core/src/config_loader/overrides.rs create mode 100644 codex-rs/core/src/config_loader/state.rs create mode 100644 codex-rs/core/src/config_loader/tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 24eb0e4ba..ec5911a4b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -846,12 +846,10 @@ dependencies = [ "serde", "serde_json", "serial_test", - "sha2", "shlex", "tempfile", "tokio", "toml 0.9.5", - "toml_edit", "tracing", "tracing-subscriber", "uuid", diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 948facdea..cb3f51bb8 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -31,11 +31,9 @@ codex-utils-json-to-toml = { workspace = true } chrono = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -sha2 = { workspace = true } mcp-types = { workspace = true } tempfile = { workspace = true } toml = { workspace = true } -toml_edit = { workspace = true } tokio = { workspace = true, features = [ "io-std", "macros", diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 9f4227b2d..53fabd22e 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -117,10 +117,9 @@ use codex_core::auth::CLIENT_ID; use codex_core::auth::login_with_api_key; use codex_core::config::Config; use codex_core::config::ConfigOverrides; -use codex_core::config::ConfigToml; +use codex_core::config::ConfigService; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::config::types::McpServerTransportConfig; -use codex_core::config_loader::load_config_as_toml; use codex_core::default_client::get_codex_user_agent; use codex_core::exec::ExecParams; use codex_core::exec_env::create_env; @@ -1108,25 +1107,13 @@ impl CodexMessageProcessor { } async fn get_user_saved_config(&self, request_id: RequestId) { - let toml_value = match load_config_as_toml(&self.config.codex_home).await { - Ok(val) => val, - Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to load config.toml: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } - }; - - let cfg: ConfigToml = match toml_value.try_into() { - Ok(cfg) => cfg, + let service = ConfigService::new(self.config.codex_home.clone(), Vec::new()); + let user_saved_config: UserSavedConfig = match service.load_user_saved_config().await { + Ok(config) => config, Err(err) => { let error = JSONRPCErrorError { code: INTERNAL_ERROR_CODE, - message: format!("failed to parse config.toml: {err}"), + message: err.to_string(), data: None, }; self.outgoing.send_error(request_id, error).await; @@ -1134,8 +1121,6 @@ impl CodexMessageProcessor { } }; - let user_saved_config: UserSavedConfig = cfg.into(); - let response = GetUserSavedConfigResponse { config: user_saved_config, }; diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index c1eaf62d2..98e0f108e 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -1,66 +1,27 @@ use crate::error_code::INTERNAL_ERROR_CODE; use crate::error_code::INVALID_REQUEST_ERROR_CODE; -use codex_app_server_protocol::Config; use codex_app_server_protocol::ConfigBatchWriteParams; -use codex_app_server_protocol::ConfigLayer; -use codex_app_server_protocol::ConfigLayerMetadata; -use codex_app_server_protocol::ConfigLayerName; use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::ConfigWriteErrorCode; use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::JSONRPCErrorError; -use codex_app_server_protocol::MergeStrategy; -use codex_app_server_protocol::OverriddenMetadata; -use codex_app_server_protocol::WriteStatus; -use codex_core::config::ConfigToml; -use codex_core::config::edit::ConfigEdit; -use codex_core::config::edit::ConfigEditsBuilder; -use codex_core::config_loader::LoadedConfigLayers; -use codex_core::config_loader::LoaderOverrides; -use codex_core::config_loader::load_config_layers_with_overrides; -use codex_core::config_loader::merge_toml_values; -use serde_json::Value as JsonValue; +use codex_core::config::ConfigService; +use codex_core::config::ConfigServiceError; use serde_json::json; -use sha2::Digest; -use sha2::Sha256; -use std::collections::HashMap; -use std::path::Path; use std::path::PathBuf; use toml::Value as TomlValue; -use toml_edit::Item as TomlItem; - -const SESSION_FLAGS_SOURCE: &str = "--config"; -const MDM_SOURCE: &str = "com.openai.codex/config_toml_base64"; -const CONFIG_FILE_NAME: &str = "config.toml"; #[derive(Clone)] pub(crate) struct ConfigApi { - codex_home: PathBuf, - cli_overrides: Vec<(String, TomlValue)>, - loader_overrides: LoaderOverrides, + service: ConfigService, } impl ConfigApi { pub(crate) fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self { Self { - codex_home, - cli_overrides, - loader_overrides: LoaderOverrides::default(), - } - } - - #[cfg(test)] - fn with_overrides( - codex_home: PathBuf, - cli_overrides: Vec<(String, TomlValue)>, - loader_overrides: LoaderOverrides, - ) -> Self { - Self { - codex_home, - cli_overrides, - loader_overrides, + service: ConfigService::new(codex_home, cli_overrides), } } @@ -68,696 +29,32 @@ impl ConfigApi { &self, params: ConfigReadParams, ) -> Result { - let layers = self - .load_layers_state() - .await - .map_err(|err| internal_error("failed to read configuration layers", err))?; - - let effective = layers.effective_config(); - validate_config(&effective).map_err(|err| internal_error("invalid configuration", err))?; - - let config: Config = serde_json::from_value(to_json_value(&effective)) - .map_err(|err| internal_error("failed to deserialize configuration", err))?; - let response = ConfigReadResponse { - config, - origins: layers.origins(), - layers: params.include_layers.then(|| layers.layers_high_to_low()), - }; - - Ok(response) + self.service.read(params).await.map_err(map_error) } pub(crate) async fn write_value( &self, params: ConfigValueWriteParams, ) -> Result { - let edits = vec![(params.key_path, params.value, params.merge_strategy)]; - self.apply_edits(params.file_path, params.expected_version, edits) - .await + self.service.write_value(params).await.map_err(map_error) } pub(crate) async fn batch_write( &self, params: ConfigBatchWriteParams, ) -> Result { - let edits = params - .edits - .into_iter() - .map(|edit| (edit.key_path, edit.value, edit.merge_strategy)) - .collect(); - - self.apply_edits(params.file_path, params.expected_version, edits) - .await - } - - async fn apply_edits( - &self, - file_path: Option, - expected_version: Option, - edits: Vec<(String, JsonValue, MergeStrategy)>, - ) -> Result { - let allowed_path = self.codex_home.join(CONFIG_FILE_NAME); - let provided_path = file_path - .as_ref() - .map(PathBuf::from) - .unwrap_or_else(|| allowed_path.clone()); - - if !paths_match(&allowed_path, &provided_path) { - return Err(config_write_error( - ConfigWriteErrorCode::ConfigLayerReadonly, - "Only writes to the user config are allowed", - )); - } - - let layers = self - .load_layers_state() - .await - .map_err(|err| internal_error("failed to load configuration", err))?; - - if let Some(expected) = expected_version.as_deref() - && expected != layers.user.version - { - return Err(config_write_error( - ConfigWriteErrorCode::ConfigVersionConflict, - "Configuration was modified since last read. Fetch latest version and retry.", - )); - } - - let mut user_config = layers.user.config.clone(); - let mut parsed_segments = Vec::new(); - let mut config_edits = Vec::new(); - - for (key_path, value, strategy) in edits.into_iter() { - let segments = parse_key_path(&key_path).map_err(|message| { - config_write_error(ConfigWriteErrorCode::ConfigValidationError, message) - })?; - let original_value = value_at_path(&user_config, &segments).cloned(); - let parsed_value = parse_value(value).map_err(|message| { - config_write_error(ConfigWriteErrorCode::ConfigValidationError, message) - })?; - - apply_merge(&mut user_config, &segments, parsed_value.as_ref(), strategy).map_err( - |err| match err { - MergeError::PathNotFound => config_write_error( - ConfigWriteErrorCode::ConfigPathNotFound, - "Path not found", - ), - MergeError::Validation(message) => { - config_write_error(ConfigWriteErrorCode::ConfigValidationError, message) - } - }, - )?; - - let updated_value = value_at_path(&user_config, &segments).cloned(); - if original_value != updated_value { - let edit = match updated_value { - Some(value) => ConfigEdit::SetPath { - segments: segments.clone(), - value: toml_value_to_item(&value) - .map_err(|err| internal_error("failed to build config edits", err))?, - }, - None => ConfigEdit::ClearPath { - segments: segments.clone(), - }, - }; - config_edits.push(edit); - } - - parsed_segments.push(segments); - } - - validate_config(&user_config).map_err(|err| { - config_write_error( - ConfigWriteErrorCode::ConfigValidationError, - format!("Invalid configuration: {err}"), - ) - })?; - - let updated_layers = layers.with_user_config(user_config.clone()); - let effective = updated_layers.effective_config(); - validate_config(&effective).map_err(|err| { - config_write_error( - ConfigWriteErrorCode::ConfigValidationError, - format!("Invalid configuration: {err}"), - ) - })?; - - if !config_edits.is_empty() { - ConfigEditsBuilder::new(&self.codex_home) - .with_edits(config_edits) - .apply() - .await - .map_err(|err| internal_error("failed to persist config.toml", err))?; - } - - let overridden = first_overridden_edit(&updated_layers, &effective, &parsed_segments); - let status = overridden - .as_ref() - .map(|_| WriteStatus::OkOverridden) - .unwrap_or(WriteStatus::Ok); - - let file_path = provided_path - .canonicalize() - .unwrap_or(provided_path.clone()) - .display() - .to_string(); - - Ok(ConfigWriteResponse { - status, - version: updated_layers.user.version.clone(), - file_path, - overridden_metadata: overridden, - }) - } - - async fn load_layers_state(&self) -> std::io::Result { - let LoadedConfigLayers { - base, - managed_config, - managed_preferences, - } = load_config_layers_with_overrides(&self.codex_home, self.loader_overrides.clone()) - .await?; - - let user = LayerState::new( - ConfigLayerName::User, - self.codex_home.join(CONFIG_FILE_NAME), - base, - ); - - let session_flags = LayerState::new( - ConfigLayerName::SessionFlags, - PathBuf::from(SESSION_FLAGS_SOURCE), - { - let mut root = TomlValue::Table(toml::map::Map::new()); - for (path, value) in self.cli_overrides.iter() { - apply_override(&mut root, path, value.clone()); - } - root - }, - ); - - let system = managed_config.map(|cfg| { - LayerState::new( - ConfigLayerName::System, - system_config_path(&self.codex_home), - cfg, - ) - }); - - let mdm = managed_preferences - .map(|cfg| LayerState::new(ConfigLayerName::Mdm, PathBuf::from(MDM_SOURCE), cfg)); - - Ok(LayersState { - user, - session_flags, - system, - mdm, - }) - } -} - -fn parse_value(value: JsonValue) -> Result, String> { - if value.is_null() { - return Ok(None); - } - - serde_json::from_value::(value) - .map(Some) - .map_err(|err| format!("invalid value: {err}")) -} - -fn parse_key_path(path: &str) -> Result, String> { - if path.trim().is_empty() { - return Err("keyPath must not be empty".to_string()); - } - Ok(path - .split('.') - .map(std::string::ToString::to_string) - .collect()) -} - -fn apply_override(target: &mut TomlValue, path: &str, value: TomlValue) { - use toml::value::Table; - - let segments: Vec<&str> = path.split('.').collect(); - let mut current = target; - - for (idx, segment) in segments.iter().enumerate() { - let is_last = idx == segments.len() - 1; - - if is_last { - match current { - TomlValue::Table(table) => { - table.insert(segment.to_string(), value); - } - _ => { - let mut table = Table::new(); - table.insert(segment.to_string(), value); - *current = TomlValue::Table(table); - } - } - return; - } - - match current { - TomlValue::Table(table) => { - current = table - .entry((*segment).to_string()) - .or_insert_with(|| TomlValue::Table(Table::new())); - } - _ => { - *current = TomlValue::Table(Table::new()); - if let TomlValue::Table(tbl) = current { - current = tbl - .entry((*segment).to_string()) - .or_insert_with(|| TomlValue::Table(Table::new())); - } - } - } - } -} - -#[derive(Debug)] -enum MergeError { - PathNotFound, - Validation(String), -} - -fn apply_merge( - root: &mut TomlValue, - segments: &[String], - value: Option<&TomlValue>, - strategy: MergeStrategy, -) -> Result { - let Some(value) = value else { - return clear_path(root, segments); - }; - - let Some((last, parents)) = segments.split_last() else { - return Err(MergeError::Validation( - "keyPath must not be empty".to_string(), - )); - }; - - let mut current = root; - - for segment in parents { - match current { - TomlValue::Table(table) => { - current = table - .entry(segment.clone()) - .or_insert_with(|| TomlValue::Table(toml::map::Map::new())); - } - _ => { - *current = TomlValue::Table(toml::map::Map::new()); - if let TomlValue::Table(table) = current { - current = table - .entry(segment.clone()) - .or_insert_with(|| TomlValue::Table(toml::map::Map::new())); - } - } - } - } - - let table = current.as_table_mut().ok_or_else(|| { - MergeError::Validation("cannot set value on non-table parent".to_string()) - })?; - - if matches!(strategy, MergeStrategy::Upsert) - && let Some(existing) = table.get_mut(last) - && matches!(existing, TomlValue::Table(_)) - && matches!(value, TomlValue::Table(_)) - { - merge_toml_values(existing, value); - return Ok(true); - } - - let changed = table - .get(last) - .map(|existing| Some(existing) != Some(value)) - .unwrap_or(true); - table.insert(last.clone(), value.clone()); - Ok(changed) -} - -fn clear_path(root: &mut TomlValue, segments: &[String]) -> Result { - let Some((last, parents)) = segments.split_last() else { - return Err(MergeError::Validation( - "keyPath must not be empty".to_string(), - )); - }; - - let mut current = root; - for segment in parents { - match current { - TomlValue::Table(table) => { - current = table.get_mut(segment).ok_or(MergeError::PathNotFound)?; - } - _ => return Err(MergeError::PathNotFound), - } - } - - let Some(parent) = current.as_table_mut() else { - return Err(MergeError::PathNotFound); - }; - - Ok(parent.remove(last).is_some()) -} - -fn toml_value_to_item(value: &TomlValue) -> anyhow::Result { - match value { - TomlValue::Table(table) => { - let mut table_item = toml_edit::Table::new(); - table_item.set_implicit(false); - for (key, val) in table { - table_item.insert(key, toml_value_to_item(val)?); - } - Ok(TomlItem::Table(table_item)) - } - other => Ok(TomlItem::Value(toml_value_to_value(other)?)), - } -} - -fn toml_value_to_value(value: &TomlValue) -> anyhow::Result { - match value { - TomlValue::String(val) => Ok(toml_edit::Value::from(val.clone())), - TomlValue::Integer(val) => Ok(toml_edit::Value::from(*val)), - TomlValue::Float(val) => Ok(toml_edit::Value::from(*val)), - TomlValue::Boolean(val) => Ok(toml_edit::Value::from(*val)), - TomlValue::Datetime(val) => Ok(toml_edit::Value::from(*val)), - TomlValue::Array(items) => { - let mut array = toml_edit::Array::new(); - for item in items { - array.push(toml_value_to_value(item)?); - } - Ok(toml_edit::Value::Array(array)) - } - TomlValue::Table(table) => { - let mut inline = toml_edit::InlineTable::new(); - for (key, val) in table { - inline.insert(key, toml_value_to_value(val)?); - } - Ok(toml_edit::Value::InlineTable(inline)) - } - } -} - -#[derive(Clone)] -struct LayerState { - name: ConfigLayerName, - source: PathBuf, - config: TomlValue, - version: String, -} - -impl LayerState { - fn new(name: ConfigLayerName, source: PathBuf, config: TomlValue) -> Self { - let version = version_for_toml(&config); - Self { - name, - source, - config, - version, - } - } - - fn metadata(&self) -> ConfigLayerMetadata { - ConfigLayerMetadata { - name: self.name.clone(), - source: self.source.display().to_string(), - version: self.version.clone(), - } - } - - fn as_layer(&self) -> ConfigLayer { - ConfigLayer { - name: self.name.clone(), - source: self.source.display().to_string(), - version: self.version.clone(), - config: to_json_value(&self.config), - } - } -} - -#[derive(Clone)] -struct LayersState { - user: LayerState, - session_flags: LayerState, - system: Option, - mdm: Option, -} - -impl LayersState { - fn with_user_config(self, user_config: TomlValue) -> Self { - Self { - user: LayerState::new(self.user.name, self.user.source, user_config), - session_flags: self.session_flags, - system: self.system, - mdm: self.mdm, - } - } - - fn effective_config(&self) -> TomlValue { - let mut merged = self.user.config.clone(); - merge_toml_values(&mut merged, &self.session_flags.config); - if let Some(system) = &self.system { - merge_toml_values(&mut merged, &system.config); - } - if let Some(mdm) = &self.mdm { - merge_toml_values(&mut merged, &mdm.config); - } - merged - } - - fn origins(&self) -> HashMap { - let mut origins = HashMap::new(); - let mut path = Vec::new(); - - record_origins( - &self.user.config, - &self.user.metadata(), - &mut path, - &mut origins, - ); - record_origins( - &self.session_flags.config, - &self.session_flags.metadata(), - &mut path, - &mut origins, - ); - if let Some(system) = &self.system { - record_origins(&system.config, &system.metadata(), &mut path, &mut origins); - } - if let Some(mdm) = &self.mdm { - record_origins(&mdm.config, &mdm.metadata(), &mut path, &mut origins); - } - - origins - } - - fn layers_high_to_low(&self) -> Vec { - let mut layers = Vec::new(); - if let Some(mdm) = &self.mdm { - layers.push(mdm.as_layer()); - } - if let Some(system) = &self.system { - layers.push(system.as_layer()); - } - layers.push(self.session_flags.as_layer()); - layers.push(self.user.as_layer()); - layers + self.service.batch_write(params).await.map_err(map_error) } } -fn record_origins( - value: &TomlValue, - meta: &ConfigLayerMetadata, - path: &mut Vec, - origins: &mut HashMap, -) { - match value { - TomlValue::Table(table) => { - for (key, val) in table { - path.push(key.clone()); - record_origins(val, meta, path, origins); - path.pop(); - } - } - TomlValue::Array(items) => { - for (idx, item) in items.iter().enumerate() { - path.push(idx.to_string()); - record_origins(item, meta, path, origins); - path.pop(); - } - } - _ => { - if !path.is_empty() { - origins.insert(path.join("."), meta.clone()); - } - } +fn map_error(err: ConfigServiceError) -> JSONRPCErrorError { + if let Some(code) = err.write_error_code() { + return config_write_error(code, err.to_string()); } -} - -fn to_json_value(value: &TomlValue) -> JsonValue { - serde_json::to_value(value).unwrap_or(JsonValue::Null) -} - -fn validate_config(value: &TomlValue) -> Result<(), toml::de::Error> { - let _: ConfigToml = value.clone().try_into()?; - Ok(()) -} - -fn version_for_toml(value: &TomlValue) -> String { - let json = to_json_value(value); - let canonical = canonical_json(&json); - let serialized = serde_json::to_vec(&canonical).unwrap_or_default(); - let mut hasher = Sha256::new(); - hasher.update(serialized); - let hash = hasher.finalize(); - let hex = hash - .iter() - .map(|byte| format!("{byte:02x}")) - .collect::(); - format!("sha256:{hex}") -} -fn canonical_json(value: &JsonValue) -> JsonValue { - match value { - JsonValue::Object(map) => { - let mut sorted = serde_json::Map::new(); - let mut keys = map.keys().cloned().collect::>(); - keys.sort(); - for key in keys { - if let Some(val) = map.get(&key) { - sorted.insert(key, canonical_json(val)); - } - } - JsonValue::Object(sorted) - } - JsonValue::Array(items) => JsonValue::Array(items.iter().map(canonical_json).collect()), - other => other.clone(), - } -} - -fn paths_match(expected: &Path, provided: &Path) -> bool { - if let (Ok(expanded_expected), Ok(expanded_provided)) = - (expected.canonicalize(), provided.canonicalize()) - { - return expanded_expected == expanded_provided; - } - - expected == provided -} - -fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a TomlValue> { - let mut current = root; - for segment in segments { - match current { - TomlValue::Table(table) => { - current = table.get(segment)?; - } - TomlValue::Array(items) => { - let idx: usize = segment.parse().ok()?; - current = items.get(idx)?; - } - _ => return None, - } - } - Some(current) -} - -fn override_message(layer: &ConfigLayerName) -> String { - match layer { - ConfigLayerName::Mdm => "Overridden by managed policy (mdm)".to_string(), - ConfigLayerName::System => "Overridden by managed config (system)".to_string(), - ConfigLayerName::SessionFlags => "Overridden by session flags".to_string(), - ConfigLayerName::User => "Overridden by user config".to_string(), - } -} - -fn compute_override_metadata( - layers: &LayersState, - effective: &TomlValue, - segments: &[String], -) -> Option { - let user_value = value_at_path(&layers.user.config, segments); - let effective_value = value_at_path(effective, segments); - - if user_value.is_some() && user_value == effective_value { - return None; - } - - if user_value.is_none() && effective_value.is_none() { - return None; - } - - let effective_layer = find_effective_layer(layers, segments); - let overriding_layer = effective_layer.unwrap_or_else(|| layers.user.metadata()); - let message = override_message(&overriding_layer.name); - - Some(OverriddenMetadata { - message, - overriding_layer, - effective_value: effective_value - .map(to_json_value) - .unwrap_or(JsonValue::Null), - }) -} - -fn first_overridden_edit( - layers: &LayersState, - effective: &TomlValue, - edits: &[Vec], -) -> Option { - for segments in edits { - if let Some(meta) = compute_override_metadata(layers, effective, segments) { - return Some(meta); - } - } - None -} - -fn find_effective_layer(layers: &LayersState, segments: &[String]) -> Option { - let check = - |state: &LayerState| value_at_path(&state.config, segments).map(|_| state.metadata()); - - if let Some(mdm) = &layers.mdm - && let Some(meta) = check(mdm) - { - return Some(meta); - } - if let Some(system) = &layers.system - && let Some(meta) = check(system) - { - return Some(meta); - } - if let Some(meta) = check(&layers.session_flags) { - return Some(meta); - } - check(&layers.user) -} - -fn system_config_path(codex_home: &Path) -> PathBuf { - if let Ok(path) = std::env::var("CODEX_MANAGED_CONFIG_PATH") { - return PathBuf::from(path); - } - - #[cfg(unix)] - { - let _ = codex_home; - PathBuf::from("/etc/codex/managed_config.toml") - } - - #[cfg(not(unix))] - { - codex_home.join("managed_config.toml") - } -} - -fn internal_error(context: &str, err: E) -> JSONRPCErrorError { JSONRPCErrorError { code: INTERNAL_ERROR_CODE, - message: format!("{context}: {err}"), + message: err.to_string(), data: None, } } @@ -771,374 +68,3 @@ fn config_write_error(code: ConfigWriteErrorCode, message: impl Into) -> })), } } - -#[cfg(test)] -mod tests { - use super::*; - use anyhow::Result; - use codex_app_server_protocol::AskForApproval; - use pretty_assertions::assert_eq; - use tempfile::tempdir; - - #[test] - fn toml_value_to_item_handles_nested_config_tables() { - let config = r#" -[mcp_servers.docs] -command = "docs-server" - -[mcp_servers.docs.http_headers] -X-Doc = "42" -"#; - - let value: TomlValue = toml::from_str(config).expect("parse config example"); - let item = toml_value_to_item(&value).expect("convert to toml_edit item"); - - let root = item.as_table().expect("root table"); - assert!(!root.is_implicit(), "root table should be explicit"); - - let mcp_servers = root - .get("mcp_servers") - .and_then(TomlItem::as_table) - .expect("mcp_servers table"); - assert!( - !mcp_servers.is_implicit(), - "mcp_servers table should be explicit" - ); - - let docs = mcp_servers - .get("docs") - .and_then(TomlItem::as_table) - .expect("docs table"); - assert_eq!( - docs.get("command") - .and_then(TomlItem::as_value) - .and_then(toml_edit::Value::as_str), - Some("docs-server") - ); - - let http_headers = docs - .get("http_headers") - .and_then(TomlItem::as_table) - .expect("http_headers table"); - assert_eq!( - http_headers - .get("X-Doc") - .and_then(TomlItem::as_value) - .and_then(toml_edit::Value::as_str), - Some("42") - ); - } - - #[tokio::test] - async fn write_value_preserves_comments_and_order() -> Result<()> { - let tmp = tempdir().expect("tempdir"); - let original = r#"# Codex user configuration -model = "gpt-5" -approval_policy = "on-request" - -[notice] -# Preserve this comment -hide_full_access_warning = true - -[features] -unified_exec = true -"#; - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), original)?; - - let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]); - api.write_value(ConfigValueWriteParams { - file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), - key_path: "features.remote_compaction".to_string(), - value: json!(true), - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect("write succeeds"); - - let updated = - std::fs::read_to_string(tmp.path().join(CONFIG_FILE_NAME)).expect("read config"); - let expected = r#"# Codex user configuration -model = "gpt-5" -approval_policy = "on-request" - -[notice] -# Preserve this comment -hide_full_access_warning = true - -[features] -unified_exec = true -remote_compaction = true -"#; - assert_eq!(updated, expected); - Ok(()) - } - - #[tokio::test] - async fn read_includes_origins_and_layers() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "model = \"user\"").unwrap(); - - let managed_path = tmp.path().join("managed_config.toml"); - std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - - let api = ConfigApi::with_overrides( - tmp.path().to_path_buf(), - vec![], - LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }, - ); - - let response = api - .read(ConfigReadParams { - include_layers: true, - }) - .await - .expect("response"); - - assert_eq!(response.config.approval_policy, Some(AskForApproval::Never)); - - assert_eq!( - response - .origins - .get("approval_policy") - .expect("origin") - .name, - ConfigLayerName::System - ); - let layers = response.layers.expect("layers present"); - assert_eq!(layers.first().unwrap().name, ConfigLayerName::System); - assert_eq!(layers.get(1).unwrap().name, ConfigLayerName::SessionFlags); - assert_eq!(layers.last().unwrap().name, ConfigLayerName::User); - } - - #[tokio::test] - async fn write_value_reports_override() { - let tmp = tempdir().expect("tempdir"); - std::fs::write( - tmp.path().join(CONFIG_FILE_NAME), - "approval_policy = \"on-request\"", - ) - .unwrap(); - - let managed_path = tmp.path().join("managed_config.toml"); - std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - - let api = ConfigApi::with_overrides( - tmp.path().to_path_buf(), - vec![], - LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }, - ); - - let result = api - .write_value(ConfigValueWriteParams { - file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), - key_path: "approval_policy".to_string(), - value: json!("never"), - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect("result"); - - let read_after = api - .read(ConfigReadParams { - include_layers: true, - }) - .await - .expect("read"); - assert_eq!( - read_after.config.approval_policy, - Some(AskForApproval::Never) - ); - assert_eq!( - read_after - .origins - .get("approval_policy") - .expect("origin") - .name, - ConfigLayerName::System - ); - assert_eq!(result.status, WriteStatus::Ok); - assert!(result.overridden_metadata.is_none()); - } - - #[tokio::test] - async fn version_conflict_rejected() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "model = \"user\"").unwrap(); - - let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]); - let error = api - .write_value(ConfigValueWriteParams { - file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), - key_path: "model".to_string(), - value: json!("gpt-5"), - merge_strategy: MergeStrategy::Replace, - expected_version: Some("sha256:bogus".to_string()), - }) - .await - .expect_err("should fail"); - - assert_eq!(error.code, INVALID_REQUEST_ERROR_CODE); - assert_eq!( - error - .data - .as_ref() - .and_then(|d| d.get("config_write_error_code")) - .and_then(serde_json::Value::as_str), - Some("configVersionConflict") - ); - } - - #[tokio::test] - async fn write_value_defaults_to_user_config_path() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "").unwrap(); - - let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]); - api.write_value(ConfigValueWriteParams { - file_path: None, - key_path: "model".to_string(), - value: json!("gpt-new"), - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect("write succeeds"); - - let contents = - std::fs::read_to_string(tmp.path().join(CONFIG_FILE_NAME)).expect("read config"); - assert!( - contents.contains("model = \"gpt-new\""), - "config.toml should be updated even when file_path is omitted" - ); - } - - #[tokio::test] - async fn invalid_user_value_rejected_even_if_overridden_by_managed() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "model = \"user\"").unwrap(); - - let managed_path = tmp.path().join("managed_config.toml"); - std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - - let api = ConfigApi::with_overrides( - tmp.path().to_path_buf(), - vec![], - LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }, - ); - - let error = api - .write_value(ConfigValueWriteParams { - file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), - key_path: "approval_policy".to_string(), - value: json!("bogus"), - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect_err("should fail validation"); - - assert_eq!(error.code, INVALID_REQUEST_ERROR_CODE); - assert_eq!( - error - .data - .as_ref() - .and_then(|d| d.get("config_write_error_code")) - .and_then(serde_json::Value::as_str), - Some("configValidationError") - ); - - let contents = - std::fs::read_to_string(tmp.path().join(CONFIG_FILE_NAME)).expect("read config"); - assert_eq!(contents.trim(), "model = \"user\""); - } - - #[tokio::test] - async fn read_reports_managed_overrides_user_and_session_flags() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "model = \"user\"").unwrap(); - - let managed_path = tmp.path().join("managed_config.toml"); - std::fs::write(&managed_path, "model = \"system\"").unwrap(); - - let cli_overrides = vec![( - "model".to_string(), - TomlValue::String("session".to_string()), - )]; - - let api = ConfigApi::with_overrides( - tmp.path().to_path_buf(), - cli_overrides, - LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }, - ); - - let response = api - .read(ConfigReadParams { - include_layers: true, - }) - .await - .expect("response"); - - assert_eq!(response.config.model.as_deref(), Some("system")); - assert_eq!( - response.origins.get("model").expect("origin").name, - ConfigLayerName::System - ); - let layers = response.layers.expect("layers"); - assert_eq!(layers.first().unwrap().name, ConfigLayerName::System); - assert_eq!(layers.get(1).unwrap().name, ConfigLayerName::SessionFlags); - assert_eq!(layers.get(2).unwrap().name, ConfigLayerName::User); - } - - #[tokio::test] - async fn write_value_reports_managed_override() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "").unwrap(); - - let managed_path = tmp.path().join("managed_config.toml"); - std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - - let api = ConfigApi::with_overrides( - tmp.path().to_path_buf(), - vec![], - LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }, - ); - - let result = api - .write_value(ConfigValueWriteParams { - file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), - key_path: "approval_policy".to_string(), - value: json!("on-request"), - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect("result"); - - assert_eq!(result.status, WriteStatus::OkOverridden); - let overridden = result.overridden_metadata.expect("overridden metadata"); - assert_eq!(overridden.overriding_layer.name, ConfigLayerName::System); - assert_eq!(overridden.effective_value, json!("never")); - } -} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 4c1a073c5..584708156 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -12,10 +12,7 @@ use crate::config::types::ShellEnvironmentPolicy; use crate::config::types::ShellEnvironmentPolicyToml; use crate::config::types::Tui; use crate::config::types::UriBasedFileOpener; -use crate::config_loader::LoadedConfigLayers; -use crate::config_loader::load_config_as_toml; -use crate::config_loader::load_config_layers_with_overrides; -use crate::config_loader::merge_toml_values; +use crate::config_loader::load_config_layers_state; use crate::features::Feature; use crate::features::FeatureOverrides; use crate::features::Features; @@ -59,8 +56,12 @@ use toml_edit::DocumentMut; pub mod edit; pub mod profile; +pub mod service; pub mod types; +pub use service::ConfigService; +pub use service::ConfigServiceError; + const OPENAI_DEFAULT_REVIEW_MODEL: &str = "gpt-5.1-codex-max"; /// Maximum number of bytes of the documentation that will be embedded. Larger @@ -342,29 +343,8 @@ async fn load_resolved_config( cli_overrides: Vec<(String, TomlValue)>, overrides: crate::config_loader::LoaderOverrides, ) -> std::io::Result { - let layers = load_config_layers_with_overrides(codex_home, overrides).await?; - Ok(apply_overlays(layers, cli_overrides)) -} - -fn apply_overlays( - layers: LoadedConfigLayers, - cli_overrides: Vec<(String, TomlValue)>, -) -> TomlValue { - let LoadedConfigLayers { - mut base, - managed_config, - managed_preferences, - } = layers; - - for (path, value) in cli_overrides.into_iter() { - apply_toml_override(&mut base, &path, value); - } - - for overlay in [managed_config, managed_preferences].into_iter().flatten() { - merge_toml_values(&mut base, &overlay); - } - - base + let layers = load_config_layers_state(codex_home, &cli_overrides, overrides).await?; + Ok(layers.effective_config()) } fn deserialize_config_toml_with_base( @@ -382,7 +362,12 @@ fn deserialize_config_toml_with_base( pub async fn load_global_mcp_servers( codex_home: &Path, ) -> std::io::Result> { - let root_value = load_config_as_toml(codex_home).await?; + let root_value = load_resolved_config( + codex_home, + Vec::new(), + crate::config_loader::LoaderOverrides::default(), + ) + .await?; let Some(servers_value) = root_value.get("mcp_servers") else { return Ok(BTreeMap::new()); }; @@ -541,49 +526,6 @@ pub fn set_default_oss_provider(codex_home: &Path, provider: &str) -> std::io::R Ok(()) } -/// Apply a single dotted-path override onto a TOML value. -fn apply_toml_override(root: &mut TomlValue, path: &str, value: TomlValue) { - use toml::value::Table; - - let segments: Vec<&str> = path.split('.').collect(); - let mut current = root; - - for (idx, segment) in segments.iter().enumerate() { - let is_last = idx == segments.len() - 1; - - if is_last { - match current { - TomlValue::Table(table) => { - table.insert(segment.to_string(), value); - } - _ => { - let mut table = Table::new(); - table.insert(segment.to_string(), value); - *current = TomlValue::Table(table); - } - } - return; - } - - // Traverse or create intermediate object. - match current { - TomlValue::Table(table) => { - current = table - .entry(segment.to_string()) - .or_insert_with(|| TomlValue::Table(Table::new())); - } - _ => { - *current = TomlValue::Table(Table::new()); - if let TomlValue::Table(tbl) = current { - current = tbl - .entry(segment.to_string()) - .or_insert_with(|| TomlValue::Table(Table::new())); - } - } - } - } -} - /// Base config deserialized from ~/.codex/config.toml. #[derive(Deserialize, Debug, Clone, Default, PartialEq)] pub struct ConfigToml { diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs new file mode 100644 index 000000000..6f9d0e4d6 --- /dev/null +++ b/codex-rs/core/src/config/service.rs @@ -0,0 +1,935 @@ +use super::CONFIG_TOML_FILE; +use super::ConfigToml; +use crate::config::edit::ConfigEdit; +use crate::config::edit::ConfigEditsBuilder; +use crate::config_loader::ConfigLayerEntry; +use crate::config_loader::ConfigLayerStack; +use crate::config_loader::LoaderOverrides; +use crate::config_loader::load_config_layers_state; +use crate::config_loader::merge_toml_values; +use codex_app_server_protocol::Config as ApiConfig; +use codex_app_server_protocol::ConfigBatchWriteParams; +use codex_app_server_protocol::ConfigLayerMetadata; +use codex_app_server_protocol::ConfigLayerName; +use codex_app_server_protocol::ConfigReadParams; +use codex_app_server_protocol::ConfigReadResponse; +use codex_app_server_protocol::ConfigValueWriteParams; +use codex_app_server_protocol::ConfigWriteErrorCode; +use codex_app_server_protocol::ConfigWriteResponse; +use codex_app_server_protocol::MergeStrategy; +use codex_app_server_protocol::OverriddenMetadata; +use codex_app_server_protocol::WriteStatus; +use serde_json::Value as JsonValue; +use std::path::Path; +use std::path::PathBuf; +use thiserror::Error; +use toml::Value as TomlValue; +use toml_edit::Item as TomlItem; + +#[derive(Debug, Error)] +pub enum ConfigServiceError { + #[error("{message}")] + Write { + code: ConfigWriteErrorCode, + message: String, + }, + + #[error("{context}: {source}")] + Io { + context: &'static str, + #[source] + source: std::io::Error, + }, + + #[error("{context}: {source}")] + Json { + context: &'static str, + #[source] + source: serde_json::Error, + }, + + #[error("{context}: {source}")] + Toml { + context: &'static str, + #[source] + source: toml::de::Error, + }, + + #[error("{context}: {source}")] + Anyhow { + context: &'static str, + #[source] + source: anyhow::Error, + }, +} + +impl ConfigServiceError { + fn write(code: ConfigWriteErrorCode, message: impl Into) -> Self { + Self::Write { + code, + message: message.into(), + } + } + + fn io(context: &'static str, source: std::io::Error) -> Self { + Self::Io { context, source } + } + + fn json(context: &'static str, source: serde_json::Error) -> Self { + Self::Json { context, source } + } + + fn toml(context: &'static str, source: toml::de::Error) -> Self { + Self::Toml { context, source } + } + + fn anyhow(context: &'static str, source: anyhow::Error) -> Self { + Self::Anyhow { context, source } + } + + pub fn write_error_code(&self) -> Option { + match self { + Self::Write { code, .. } => Some(code.clone()), + _ => None, + } + } +} + +#[derive(Clone)] +pub struct ConfigService { + codex_home: PathBuf, + cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, +} + +impl ConfigService { + pub fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self { + Self { + codex_home, + cli_overrides, + loader_overrides: LoaderOverrides::default(), + } + } + + #[cfg(test)] + fn with_overrides( + codex_home: PathBuf, + cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, + ) -> Self { + Self { + codex_home, + cli_overrides, + loader_overrides, + } + } + + pub async fn read( + &self, + params: ConfigReadParams, + ) -> Result { + let layers = self + .load_layers_state() + .await + .map_err(|err| ConfigServiceError::io("failed to read configuration layers", err))?; + + let effective = layers.effective_config(); + validate_config(&effective) + .map_err(|err| ConfigServiceError::toml("invalid configuration", err))?; + + let json_value = serde_json::to_value(&effective) + .map_err(|err| ConfigServiceError::json("failed to serialize configuration", err))?; + let config: ApiConfig = serde_json::from_value(json_value) + .map_err(|err| ConfigServiceError::json("failed to deserialize configuration", err))?; + + Ok(ConfigReadResponse { + config, + origins: layers.origins(), + layers: params.include_layers.then(|| layers.layers_high_to_low()), + }) + } + + pub async fn write_value( + &self, + params: ConfigValueWriteParams, + ) -> Result { + let edits = vec![(params.key_path, params.value, params.merge_strategy)]; + self.apply_edits(params.file_path, params.expected_version, edits) + .await + } + + pub async fn batch_write( + &self, + params: ConfigBatchWriteParams, + ) -> Result { + let edits = params + .edits + .into_iter() + .map(|edit| (edit.key_path, edit.value, edit.merge_strategy)) + .collect(); + + self.apply_edits(params.file_path, params.expected_version, edits) + .await + } + + pub async fn load_user_saved_config( + &self, + ) -> Result { + let layers = self + .load_layers_state() + .await + .map_err(|err| ConfigServiceError::io("failed to load configuration", err))?; + + let toml_value = layers.effective_config(); + let cfg: ConfigToml = toml_value + .try_into() + .map_err(|err| ConfigServiceError::toml("failed to parse config.toml", err))?; + Ok(cfg.into()) + } + + async fn apply_edits( + &self, + file_path: Option, + expected_version: Option, + edits: Vec<(String, JsonValue, MergeStrategy)>, + ) -> Result { + let allowed_path = self.codex_home.join(CONFIG_TOML_FILE); + let provided_path = file_path + .as_ref() + .map(PathBuf::from) + .unwrap_or_else(|| allowed_path.clone()); + + if !paths_match(&allowed_path, &provided_path) { + return Err(ConfigServiceError::write( + ConfigWriteErrorCode::ConfigLayerReadonly, + "Only writes to the user config are allowed", + )); + } + + let layers = self + .load_layers_state() + .await + .map_err(|err| ConfigServiceError::io("failed to load configuration", err))?; + + if let Some(expected) = expected_version.as_deref() + && expected != layers.user.version + { + return Err(ConfigServiceError::write( + ConfigWriteErrorCode::ConfigVersionConflict, + "Configuration was modified since last read. Fetch latest version and retry.", + )); + } + + let mut user_config = layers.user.config.clone(); + let mut parsed_segments = Vec::new(); + let mut config_edits = Vec::new(); + + for (key_path, value, strategy) in edits.into_iter() { + let segments = parse_key_path(&key_path).map_err(|message| { + ConfigServiceError::write(ConfigWriteErrorCode::ConfigValidationError, message) + })?; + let original_value = value_at_path(&user_config, &segments).cloned(); + let parsed_value = parse_value(value).map_err(|message| { + ConfigServiceError::write(ConfigWriteErrorCode::ConfigValidationError, message) + })?; + + apply_merge(&mut user_config, &segments, parsed_value.as_ref(), strategy).map_err( + |err| match err { + MergeError::PathNotFound => ConfigServiceError::write( + ConfigWriteErrorCode::ConfigPathNotFound, + "Path not found", + ), + MergeError::Validation(message) => ConfigServiceError::write( + ConfigWriteErrorCode::ConfigValidationError, + message, + ), + }, + )?; + + let updated_value = value_at_path(&user_config, &segments).cloned(); + if original_value != updated_value { + let edit = match updated_value { + Some(value) => ConfigEdit::SetPath { + segments: segments.clone(), + value: toml_value_to_item(&value).map_err(|err| { + ConfigServiceError::anyhow("failed to build config edits", err) + })?, + }, + None => ConfigEdit::ClearPath { + segments: segments.clone(), + }, + }; + config_edits.push(edit); + } + + parsed_segments.push(segments); + } + + validate_config(&user_config).map_err(|err| { + ConfigServiceError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("Invalid configuration: {err}"), + ) + })?; + + let updated_layers = layers.with_user_config(user_config.clone()); + let effective = updated_layers.effective_config(); + validate_config(&effective).map_err(|err| { + ConfigServiceError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("Invalid configuration: {err}"), + ) + })?; + + if !config_edits.is_empty() { + ConfigEditsBuilder::new(&self.codex_home) + .with_edits(config_edits) + .apply() + .await + .map_err(|err| ConfigServiceError::anyhow("failed to persist config.toml", err))?; + } + + let overridden = first_overridden_edit(&updated_layers, &effective, &parsed_segments); + let status = overridden + .as_ref() + .map(|_| WriteStatus::OkOverridden) + .unwrap_or(WriteStatus::Ok); + + let file_path = provided_path + .canonicalize() + .unwrap_or(provided_path.clone()) + .display() + .to_string(); + + Ok(ConfigWriteResponse { + status, + version: updated_layers.user.version.clone(), + file_path, + overridden_metadata: overridden, + }) + } + + async fn load_layers_state(&self) -> std::io::Result { + load_config_layers_state( + &self.codex_home, + &self.cli_overrides, + self.loader_overrides.clone(), + ) + .await + } +} + +fn parse_value(value: JsonValue) -> Result, String> { + if value.is_null() { + return Ok(None); + } + + serde_json::from_value::(value) + .map(Some) + .map_err(|err| format!("invalid value: {err}")) +} + +fn parse_key_path(path: &str) -> Result, String> { + if path.trim().is_empty() { + return Err("keyPath must not be empty".to_string()); + } + Ok(path + .split('.') + .map(std::string::ToString::to_string) + .collect()) +} + +#[derive(Debug)] +enum MergeError { + PathNotFound, + Validation(String), +} + +fn apply_merge( + root: &mut TomlValue, + segments: &[String], + value: Option<&TomlValue>, + strategy: MergeStrategy, +) -> Result { + let Some(value) = value else { + return clear_path(root, segments); + }; + + let Some((last, parents)) = segments.split_last() else { + return Err(MergeError::Validation( + "keyPath must not be empty".to_string(), + )); + }; + + let mut current = root; + + for segment in parents { + match current { + TomlValue::Table(table) => { + current = table + .entry(segment.clone()) + .or_insert_with(|| TomlValue::Table(toml::map::Map::new())); + } + _ => { + *current = TomlValue::Table(toml::map::Map::new()); + if let TomlValue::Table(table) = current { + current = table + .entry(segment.clone()) + .or_insert_with(|| TomlValue::Table(toml::map::Map::new())); + } + } + } + } + + let table = current.as_table_mut().ok_or_else(|| { + MergeError::Validation("cannot set value on non-table parent".to_string()) + })?; + + if matches!(strategy, MergeStrategy::Upsert) + && let Some(existing) = table.get_mut(last) + && matches!(existing, TomlValue::Table(_)) + && matches!(value, TomlValue::Table(_)) + { + merge_toml_values(existing, value); + return Ok(true); + } + + let changed = table + .get(last) + .map(|existing| Some(existing) != Some(value)) + .unwrap_or(true); + table.insert(last.clone(), value.clone()); + Ok(changed) +} + +fn clear_path(root: &mut TomlValue, segments: &[String]) -> Result { + let Some((last, parents)) = segments.split_last() else { + return Err(MergeError::Validation( + "keyPath must not be empty".to_string(), + )); + }; + + let mut current = root; + for segment in parents { + match current { + TomlValue::Table(table) => { + current = table.get_mut(segment).ok_or(MergeError::PathNotFound)?; + } + _ => return Err(MergeError::PathNotFound), + } + } + + let Some(parent) = current.as_table_mut() else { + return Err(MergeError::PathNotFound); + }; + + Ok(parent.remove(last).is_some()) +} + +fn toml_value_to_item(value: &TomlValue) -> anyhow::Result { + match value { + TomlValue::Table(table) => { + let mut table_item = toml_edit::Table::new(); + table_item.set_implicit(false); + for (key, val) in table { + table_item.insert(key, toml_value_to_item(val)?); + } + Ok(TomlItem::Table(table_item)) + } + other => Ok(TomlItem::Value(toml_value_to_value(other)?)), + } +} + +fn toml_value_to_value(value: &TomlValue) -> anyhow::Result { + match value { + TomlValue::String(val) => Ok(toml_edit::Value::from(val.clone())), + TomlValue::Integer(val) => Ok(toml_edit::Value::from(*val)), + TomlValue::Float(val) => Ok(toml_edit::Value::from(*val)), + TomlValue::Boolean(val) => Ok(toml_edit::Value::from(*val)), + TomlValue::Datetime(val) => Ok(toml_edit::Value::from(*val)), + TomlValue::Array(items) => { + let mut array = toml_edit::Array::new(); + for item in items { + array.push(toml_value_to_value(item)?); + } + Ok(toml_edit::Value::Array(array)) + } + TomlValue::Table(table) => { + let mut inline = toml_edit::InlineTable::new(); + for (key, val) in table { + inline.insert(key, toml_value_to_value(val)?); + } + Ok(toml_edit::Value::InlineTable(inline)) + } + } +} + +fn validate_config(value: &TomlValue) -> Result<(), toml::de::Error> { + let _: ConfigToml = value.clone().try_into()?; + Ok(()) +} + +fn paths_match(expected: &Path, provided: &Path) -> bool { + if let (Ok(expanded_expected), Ok(expanded_provided)) = + (expected.canonicalize(), provided.canonicalize()) + { + return expanded_expected == expanded_provided; + } + + expected == provided +} + +fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a TomlValue> { + let mut current = root; + for segment in segments { + match current { + TomlValue::Table(table) => { + current = table.get(segment)?; + } + TomlValue::Array(items) => { + let idx = segment.parse::().ok()?; + let idx = usize::try_from(idx).ok()?; + current = items.get(idx)?; + } + _ => return None, + } + } + Some(current) +} + +fn override_message(layer: &ConfigLayerName) -> String { + match layer { + ConfigLayerName::Mdm => "Overridden by managed policy (mdm)".to_string(), + ConfigLayerName::System => "Overridden by managed config (system)".to_string(), + ConfigLayerName::SessionFlags => "Overridden by session flags".to_string(), + ConfigLayerName::User => "Overridden by user config".to_string(), + } +} + +fn compute_override_metadata( + layers: &ConfigLayerStack, + effective: &TomlValue, + segments: &[String], +) -> Option { + let user_value = value_at_path(&layers.user.config, segments); + let effective_value = value_at_path(effective, segments); + + if user_value.is_some() && user_value == effective_value { + return None; + } + + if user_value.is_none() && effective_value.is_none() { + return None; + } + + let effective_layer = find_effective_layer(layers, segments); + let overriding_layer = effective_layer.unwrap_or_else(|| layers.user.metadata()); + let message = override_message(&overriding_layer.name); + + Some(OverriddenMetadata { + message, + overriding_layer, + effective_value: effective_value + .and_then(|value| serde_json::to_value(value).ok()) + .unwrap_or(JsonValue::Null), + }) +} + +fn first_overridden_edit( + layers: &ConfigLayerStack, + effective: &TomlValue, + edits: &[Vec], +) -> Option { + for segments in edits { + if let Some(meta) = compute_override_metadata(layers, effective, segments) { + return Some(meta); + } + } + None +} + +fn find_effective_layer( + layers: &ConfigLayerStack, + segments: &[String], +) -> Option { + let check = + |state: &ConfigLayerEntry| value_at_path(&state.config, segments).map(|_| state.metadata()); + + if let Some(mdm) = &layers.mdm + && let Some(meta) = check(mdm) + { + return Some(meta); + } + if let Some(system) = &layers.system + && let Some(meta) = check(system) + { + return Some(meta); + } + if let Some(meta) = check(&layers.session_flags) { + return Some(meta); + } + check(&layers.user) +} + +#[cfg(test)] +mod tests { + use super::*; + use anyhow::Result; + use codex_app_server_protocol::AskForApproval; + use pretty_assertions::assert_eq; + use tempfile::tempdir; + + #[test] + fn toml_value_to_item_handles_nested_config_tables() { + let config = r#" +[mcp_servers.docs] +command = "docs-server" + +[mcp_servers.docs.http_headers] +X-Doc = "42" +"#; + + let value: TomlValue = toml::from_str(config).expect("parse config example"); + let item = toml_value_to_item(&value).expect("convert to toml_edit item"); + + let root = item.as_table().expect("root table"); + assert!(!root.is_implicit(), "root table should be explicit"); + + let mcp_servers = root + .get("mcp_servers") + .and_then(TomlItem::as_table) + .expect("mcp_servers table"); + assert!( + !mcp_servers.is_implicit(), + "mcp_servers table should be explicit" + ); + + let docs = mcp_servers + .get("docs") + .and_then(TomlItem::as_table) + .expect("docs table"); + assert_eq!( + docs.get("command") + .and_then(TomlItem::as_value) + .and_then(toml_edit::Value::as_str), + Some("docs-server") + ); + + let http_headers = docs + .get("http_headers") + .and_then(TomlItem::as_table) + .expect("http_headers table"); + assert_eq!( + http_headers + .get("X-Doc") + .and_then(TomlItem::as_value) + .and_then(toml_edit::Value::as_str), + Some("42") + ); + } + + #[tokio::test] + async fn write_value_preserves_comments_and_order() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let original = r#"# Codex user configuration +model = "gpt-5" +approval_policy = "on-request" + +[notice] +# Preserve this comment +hide_full_access_warning = true + +[features] +unified_exec = true +"#; + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), original)?; + + let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "features.remote_compaction".to_string(), + value: serde_json::json!(true), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("write succeeds"); + + let updated = + std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"# Codex user configuration +model = "gpt-5" +approval_policy = "on-request" + +[notice] +# Preserve this comment +hide_full_access_warning = true + +[features] +unified_exec = true +remote_compaction = true +"#; + assert_eq!(updated, expected); + Ok(()) + } + + #[tokio::test] + async fn read_includes_origins_and_layers() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "model = \"user\"").unwrap(); + + let managed_path = tmp.path().join("managed_config.toml"); + std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); + + let service = ConfigService::with_overrides( + tmp.path().to_path_buf(), + vec![], + LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }, + ); + + let response = service + .read(ConfigReadParams { + include_layers: true, + }) + .await + .expect("response"); + + assert_eq!(response.config.approval_policy, Some(AskForApproval::Never)); + + assert_eq!( + response + .origins + .get("approval_policy") + .expect("origin") + .name, + ConfigLayerName::System + ); + let layers = response.layers.expect("layers present"); + assert_eq!(layers.first().unwrap().name, ConfigLayerName::System); + assert_eq!(layers.get(1).unwrap().name, ConfigLayerName::SessionFlags); + assert_eq!(layers.last().unwrap().name, ConfigLayerName::User); + } + + #[tokio::test] + async fn write_value_reports_override() { + let tmp = tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + "approval_policy = \"on-request\"", + ) + .unwrap(); + + let managed_path = tmp.path().join("managed_config.toml"); + std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); + + let service = ConfigService::with_overrides( + tmp.path().to_path_buf(), + vec![], + LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }, + ); + + let result = service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "approval_policy".to_string(), + value: serde_json::json!("never"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("result"); + + let read_after = service + .read(ConfigReadParams { + include_layers: true, + }) + .await + .expect("read"); + assert_eq!( + read_after.config.approval_policy, + Some(AskForApproval::Never) + ); + assert_eq!( + read_after + .origins + .get("approval_policy") + .expect("origin") + .name, + ConfigLayerName::System + ); + assert_eq!(result.status, WriteStatus::Ok); + assert!(result.overridden_metadata.is_none()); + } + + #[tokio::test] + async fn version_conflict_rejected() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "model = \"user\"").unwrap(); + + let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let error = service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "model".to_string(), + value: serde_json::json!("gpt-5"), + merge_strategy: MergeStrategy::Replace, + expected_version: Some("sha256:bogus".to_string()), + }) + .await + .expect_err("should fail"); + + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigVersionConflict) + ); + } + + #[tokio::test] + async fn write_value_defaults_to_user_config_path() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); + + let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + service + .write_value(ConfigValueWriteParams { + file_path: None, + key_path: "model".to_string(), + value: serde_json::json!("gpt-new"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("write succeeds"); + + let contents = + std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config"); + assert!( + contents.contains("model = \"gpt-new\""), + "config.toml should be updated even when file_path is omitted" + ); + } + + #[tokio::test] + async fn invalid_user_value_rejected_even_if_overridden_by_managed() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "model = \"user\"").unwrap(); + + let managed_path = tmp.path().join("managed_config.toml"); + std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); + + let service = ConfigService::with_overrides( + tmp.path().to_path_buf(), + vec![], + LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }, + ); + + let error = service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "approval_policy".to_string(), + value: serde_json::json!("bogus"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect_err("should fail validation"); + + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigValidationError) + ); + + let contents = + std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents.trim(), "model = \"user\""); + } + + #[tokio::test] + async fn read_reports_managed_overrides_user_and_session_flags() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "model = \"user\"").unwrap(); + + let managed_path = tmp.path().join("managed_config.toml"); + std::fs::write(&managed_path, "model = \"system\"").unwrap(); + + let cli_overrides = vec![( + "model".to_string(), + TomlValue::String("session".to_string()), + )]; + + let service = ConfigService::with_overrides( + tmp.path().to_path_buf(), + cli_overrides, + LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }, + ); + + let response = service + .read(ConfigReadParams { + include_layers: true, + }) + .await + .expect("response"); + + assert_eq!(response.config.model.as_deref(), Some("system")); + assert_eq!( + response.origins.get("model").expect("origin").name, + ConfigLayerName::System + ); + let layers = response.layers.expect("layers"); + assert_eq!(layers.first().unwrap().name, ConfigLayerName::System); + assert_eq!(layers.get(1).unwrap().name, ConfigLayerName::SessionFlags); + assert_eq!(layers.get(2).unwrap().name, ConfigLayerName::User); + } + + #[tokio::test] + async fn write_value_reports_managed_override() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); + + let managed_path = tmp.path().join("managed_config.toml"); + std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); + + let service = ConfigService::with_overrides( + tmp.path().to_path_buf(), + vec![], + LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }, + ); + + let result = service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "approval_policy".to_string(), + value: serde_json::json!("on-request"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("result"); + + assert_eq!(result.status, WriteStatus::OkOverridden); + let overridden = result.overridden_metadata.expect("overridden metadata"); + assert_eq!(overridden.overriding_layer.name, ConfigLayerName::System); + assert_eq!(overridden.effective_value, serde_json::json!("never")); + } +} diff --git a/codex-rs/core/src/config_loader/README.md b/codex-rs/core/src/config_loader/README.md new file mode 100644 index 000000000..da427c454 --- /dev/null +++ b/codex-rs/core/src/config_loader/README.md @@ -0,0 +1,64 @@ +# `codex-core` config loader + +This module is the canonical place to **load and describe Codex configuration layers** (user config, CLI/session overrides, managed config, and MDM-managed preferences) and to produce: + +- An **effective merged** TOML config. +- **Per-key origins** metadata (which layer “wins” for a given key). +- **Per-layer versions** (stable fingerprints) used for optimistic concurrency / conflict detection. + +## Public surface + +Exported from `codex_core::config_loader`: + +- `load_config_layers_state(codex_home, cli_overrides, overrides) -> ConfigLayerStack` +- `ConfigLayerStack` + - `effective_config() -> toml::Value` + - `origins() -> HashMap` + - `layers_high_to_low() -> Vec` + - `with_user_config(user_config) -> ConfigLayerStack` +- `ConfigLayerEntry` (one layer’s `{name, source, config, version}`) +- `LoaderOverrides` (test/override hooks for managed config sources) +- `merge_toml_values(base, overlay)` (public helper used elsewhere) + +## Layering model + +Precedence is **top overrides bottom**: + +1. **MDM** managed preferences (macOS only) +2. **System** managed config (e.g. `managed_config.toml`) +3. **Session flags** (CLI overrides, applied as dotted-path TOML writes) +4. **User** config (`config.toml`) + +This is what `ConfigLayerStack::effective_config()` implements. + +## Typical usage + +Most callers want the effective config plus metadata: + +```rust +use codex_core::config_loader::{load_config_layers_state, LoaderOverrides}; +use toml::Value as TomlValue; + +let cli_overrides: Vec<(String, TomlValue)> = Vec::new(); +let layers = load_config_layers_state( + &codex_home, + &cli_overrides, + LoaderOverrides::default(), +).await?; + +let effective = layers.effective_config(); +let origins = layers.origins(); +let layers_for_ui = layers.layers_high_to_low(); +``` + +## Internal layout + +Implementation is split by concern: + +- `state.rs`: public types (`ConfigLayerEntry`, `ConfigLayerStack`) + merge/origins convenience methods. +- `layer_io.rs`: reading `config.toml`, managed config, and managed preferences inputs. +- `overrides.rs`: CLI dotted-path overrides → TOML “session flags” layer. +- `merge.rs`: recursive TOML merge. +- `fingerprint.rs`: stable per-layer hashing and per-key origins traversal. +- `macos.rs`: managed preferences integration (macOS only). + diff --git a/codex-rs/core/src/config_loader/fingerprint.rs b/codex-rs/core/src/config_loader/fingerprint.rs new file mode 100644 index 000000000..c072134fd --- /dev/null +++ b/codex-rs/core/src/config_loader/fingerprint.rs @@ -0,0 +1,67 @@ +use codex_app_server_protocol::ConfigLayerMetadata; +use serde_json::Value as JsonValue; +use sha2::Digest; +use sha2::Sha256; +use std::collections::HashMap; +use toml::Value as TomlValue; + +pub(super) fn record_origins( + value: &TomlValue, + meta: &ConfigLayerMetadata, + path: &mut Vec, + origins: &mut HashMap, +) { + match value { + TomlValue::Table(table) => { + for (key, val) in table { + path.push(key.clone()); + record_origins(val, meta, path, origins); + path.pop(); + } + } + TomlValue::Array(items) => { + for (idx, item) in (0_i32..).zip(items.iter()) { + path.push(idx.to_string()); + record_origins(item, meta, path, origins); + path.pop(); + } + } + _ => { + if !path.is_empty() { + origins.insert(path.join("."), meta.clone()); + } + } + } +} + +pub(super) fn version_for_toml(value: &TomlValue) -> String { + let json = serde_json::to_value(value).unwrap_or(JsonValue::Null); + let canonical = canonical_json(&json); + let serialized = serde_json::to_vec(&canonical).unwrap_or_default(); + let mut hasher = Sha256::new(); + hasher.update(serialized); + let hash = hasher.finalize(); + let hex = hash + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + format!("sha256:{hex}") +} + +fn canonical_json(value: &JsonValue) -> JsonValue { + match value { + JsonValue::Object(map) => { + let mut sorted = serde_json::Map::new(); + let mut keys = map.keys().cloned().collect::>(); + keys.sort(); + for key in keys { + if let Some(val) = map.get(&key) { + sorted.insert(key, canonical_json(val)); + } + } + JsonValue::Object(sorted) + } + JsonValue::Array(items) => JsonValue::Array(items.iter().map(canonical_json).collect()), + other => other.clone(), + } +} diff --git a/codex-rs/core/src/config_loader/layer_io.rs b/codex-rs/core/src/config_loader/layer_io.rs new file mode 100644 index 000000000..1e9052f21 --- /dev/null +++ b/codex-rs/core/src/config_loader/layer_io.rs @@ -0,0 +1,100 @@ +use super::LoaderOverrides; +use super::macos::load_managed_admin_config_layer; +use super::overrides::default_empty_table; +use crate::config::CONFIG_TOML_FILE; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use tokio::fs; +use toml::Value as TomlValue; + +#[cfg(unix)] +const CODEX_MANAGED_CONFIG_SYSTEM_PATH: &str = "/etc/codex/managed_config.toml"; + +#[derive(Debug, Clone)] +pub(super) struct LoadedConfigLayers { + pub base: TomlValue, + pub managed_config: Option, + pub managed_preferences: Option, +} + +pub(super) async fn load_config_layers_internal( + codex_home: &Path, + overrides: LoaderOverrides, +) -> io::Result { + #[cfg(target_os = "macos")] + let LoaderOverrides { + managed_config_path, + managed_preferences_base64, + } = overrides; + + #[cfg(not(target_os = "macos"))] + let LoaderOverrides { + managed_config_path, + } = overrides; + + let managed_config_path = + managed_config_path.unwrap_or_else(|| managed_config_default_path(codex_home)); + + let user_config_path = codex_home.join(CONFIG_TOML_FILE); + let user_config = read_config_from_path(&user_config_path, true).await?; + let managed_config = read_config_from_path(&managed_config_path, false).await?; + + #[cfg(target_os = "macos")] + let managed_preferences = + load_managed_admin_config_layer(managed_preferences_base64.as_deref()).await?; + + #[cfg(not(target_os = "macos"))] + let managed_preferences = load_managed_admin_config_layer(None).await?; + + Ok(LoadedConfigLayers { + base: user_config.unwrap_or_else(default_empty_table), + managed_config, + managed_preferences, + }) +} + +pub(super) async fn read_config_from_path( + path: &Path, + log_missing_as_info: bool, +) -> io::Result> { + match fs::read_to_string(path).await { + Ok(contents) => match toml::from_str::(&contents) { + Ok(value) => Ok(Some(value)), + Err(err) => { + tracing::error!("Failed to parse {}: {err}", path.display()); + Err(io::Error::new(io::ErrorKind::InvalidData, err)) + } + }, + Err(err) if err.kind() == io::ErrorKind::NotFound => { + if log_missing_as_info { + tracing::info!("{} not found, using defaults", path.display()); + } else { + tracing::debug!("{} not found", path.display()); + } + Ok(None) + } + Err(err) => { + tracing::error!("Failed to read {}: {err}", path.display()); + Err(err) + } + } +} + +/// Return the default managed config path (honoring `CODEX_MANAGED_CONFIG_PATH`). +pub(super) fn managed_config_default_path(codex_home: &Path) -> PathBuf { + if let Ok(path) = std::env::var("CODEX_MANAGED_CONFIG_PATH") { + return PathBuf::from(path); + } + + #[cfg(unix)] + { + let _ = codex_home; + PathBuf::from(CODEX_MANAGED_CONFIG_SYSTEM_PATH) + } + + #[cfg(not(unix))] + { + codex_home.join("managed_config.toml") + } +} diff --git a/codex-rs/core/src/config_loader/merge.rs b/codex-rs/core/src/config_loader/merge.rs new file mode 100644 index 000000000..eacbbcb78 --- /dev/null +++ b/codex-rs/core/src/config_loader/merge.rs @@ -0,0 +1,18 @@ +use toml::Value as TomlValue; + +/// Merge config `overlay` into `base`, giving `overlay` precedence. +pub fn merge_toml_values(base: &mut TomlValue, overlay: &TomlValue) { + if let TomlValue::Table(overlay_table) = overlay + && let TomlValue::Table(base_table) = base + { + for (key, value) in overlay_table { + if let Some(existing) = base_table.get_mut(key) { + merge_toml_values(existing, value); + } else { + base_table.insert(key.clone(), value.clone()); + } + } + } else { + *base = overlay.clone(); + } +} diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 4cd541d3e..c7c522008 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -1,319 +1,74 @@ +mod fingerprint; +mod layer_io; mod macos; +mod merge; +mod overrides; +mod state; + +#[cfg(test)] +mod tests; use crate::config::CONFIG_TOML_FILE; -use macos::load_managed_admin_config_layer; +use codex_app_server_protocol::ConfigLayerName; use std::io; use std::path::Path; use std::path::PathBuf; -use tokio::fs; use toml::Value as TomlValue; -#[cfg(unix)] -const CODEX_MANAGED_CONFIG_SYSTEM_PATH: &str = "/etc/codex/managed_config.toml"; - -#[derive(Debug, Clone)] -pub struct LoadedConfigLayers { - pub base: TomlValue, - pub managed_config: Option, - pub managed_preferences: Option, -} - -#[derive(Debug, Default, Clone)] -pub struct LoaderOverrides { - pub managed_config_path: Option, - #[cfg(target_os = "macos")] - pub managed_preferences_base64: Option, -} - -// Configuration layering pipeline (top overrides bottom): -// -// +-------------------------+ -// | Managed preferences (*) | -// +-------------------------+ -// ^ -// | -// +-------------------------+ -// | managed_config.toml | -// +-------------------------+ -// ^ -// | -// +-------------------------+ -// | config.toml (base) | -// +-------------------------+ -// -// (*) Only available on macOS via managed device profiles. - -pub async fn load_config_as_toml(codex_home: &Path) -> io::Result { - load_config_as_toml_with_overrides(codex_home, LoaderOverrides::default()).await -} - -pub async fn load_config_layers(codex_home: &Path) -> io::Result { - load_config_layers_with_overrides(codex_home, LoaderOverrides::default()).await -} - -fn default_empty_table() -> TomlValue { - TomlValue::Table(Default::default()) -} - -pub async fn load_config_layers_with_overrides( +pub use merge::merge_toml_values; +pub use state::ConfigLayerEntry; +pub use state::ConfigLayerStack; +pub use state::LoaderOverrides; + +const SESSION_FLAGS_SOURCE: &str = "--config"; +const MDM_SOURCE: &str = "com.openai.codex/config_toml_base64"; + +/// Configuration layering pipeline (top overrides bottom): +/// +/// +-------------------------+ +/// | Managed preferences (*) | +/// +-------------------------+ +/// ^ +/// | +/// +-------------------------+ +/// | managed_config.toml | +/// +-------------------------+ +/// ^ +/// | +/// +-------------------------+ +/// | config.toml (base) | +/// +-------------------------+ +/// +/// (*) Only available on macOS via managed device profiles. +pub async fn load_config_layers_state( codex_home: &Path, + cli_overrides: &[(String, TomlValue)], overrides: LoaderOverrides, -) -> io::Result { - load_config_layers_internal(codex_home, overrides).await -} - -async fn load_config_as_toml_with_overrides( - codex_home: &Path, - overrides: LoaderOverrides, -) -> io::Result { - let layers = load_config_layers_internal(codex_home, overrides).await?; - Ok(apply_managed_layers(layers)) -} - -async fn load_config_layers_internal( - codex_home: &Path, - overrides: LoaderOverrides, -) -> io::Result { - #[cfg(target_os = "macos")] - let LoaderOverrides { - managed_config_path, - managed_preferences_base64, - } = overrides; - - #[cfg(not(target_os = "macos"))] - let LoaderOverrides { - managed_config_path, - } = overrides; - - let managed_config_path = - managed_config_path.unwrap_or_else(|| managed_config_default_path(codex_home)); - - let user_config_path = codex_home.join(CONFIG_TOML_FILE); - let user_config = read_config_from_path(&user_config_path, true).await?; - let managed_config = read_config_from_path(&managed_config_path, false).await?; - - #[cfg(target_os = "macos")] - let managed_preferences = - load_managed_admin_config_layer(managed_preferences_base64.as_deref()).await?; - - #[cfg(not(target_os = "macos"))] - let managed_preferences = load_managed_admin_config_layer(None).await?; - - Ok(LoadedConfigLayers { - base: user_config.unwrap_or_else(default_empty_table), - managed_config, - managed_preferences, +) -> io::Result { + let managed_config_path = overrides + .managed_config_path + .clone() + .unwrap_or_else(|| layer_io::managed_config_default_path(codex_home)); + + let layers = layer_io::load_config_layers_internal(codex_home, overrides).await?; + let cli_overrides = overrides::build_cli_overrides_layer(cli_overrides); + + Ok(ConfigLayerStack { + user: ConfigLayerEntry::new( + ConfigLayerName::User, + codex_home.join(CONFIG_TOML_FILE), + layers.base, + ), + session_flags: ConfigLayerEntry::new( + ConfigLayerName::SessionFlags, + PathBuf::from(SESSION_FLAGS_SOURCE), + cli_overrides, + ), + system: layers.managed_config.map(|cfg| { + ConfigLayerEntry::new(ConfigLayerName::System, managed_config_path.clone(), cfg) + }), + mdm: layers + .managed_preferences + .map(|cfg| ConfigLayerEntry::new(ConfigLayerName::Mdm, PathBuf::from(MDM_SOURCE), cfg)), }) } - -async fn read_config_from_path( - path: &Path, - log_missing_as_info: bool, -) -> io::Result> { - match fs::read_to_string(path).await { - Ok(contents) => match toml::from_str::(&contents) { - Ok(value) => Ok(Some(value)), - Err(err) => { - tracing::error!("Failed to parse {}: {err}", path.display()); - Err(io::Error::new(io::ErrorKind::InvalidData, err)) - } - }, - Err(err) if err.kind() == io::ErrorKind::NotFound => { - if log_missing_as_info { - tracing::info!("{} not found, using defaults", path.display()); - } else { - tracing::debug!("{} not found", path.display()); - } - Ok(None) - } - Err(err) => { - tracing::error!("Failed to read {}: {err}", path.display()); - Err(err) - } - } -} - -/// Merge config `overlay` into `base`, giving `overlay` precedence. -pub fn merge_toml_values(base: &mut TomlValue, overlay: &TomlValue) { - if let TomlValue::Table(overlay_table) = overlay - && let TomlValue::Table(base_table) = base - { - for (key, value) in overlay_table { - if let Some(existing) = base_table.get_mut(key) { - merge_toml_values(existing, value); - } else { - base_table.insert(key.clone(), value.clone()); - } - } - } else { - *base = overlay.clone(); - } -} - -fn managed_config_default_path(codex_home: &Path) -> PathBuf { - if let Ok(path) = std::env::var("CODEX_MANAGED_CONFIG_PATH") { - return PathBuf::from(path); - } - - #[cfg(unix)] - { - let _ = codex_home; - PathBuf::from(CODEX_MANAGED_CONFIG_SYSTEM_PATH) - } - - #[cfg(not(unix))] - { - codex_home.join("managed_config.toml") - } -} - -fn apply_managed_layers(layers: LoadedConfigLayers) -> TomlValue { - let LoadedConfigLayers { - mut base, - managed_config, - managed_preferences, - } = layers; - - for overlay in [managed_config, managed_preferences].into_iter().flatten() { - merge_toml_values(&mut base, &overlay); - } - - base -} - -#[cfg(test)] -mod tests { - use super::*; - use tempfile::tempdir; - - #[tokio::test] - async fn merges_managed_config_layer_on_top() { - let tmp = tempdir().expect("tempdir"); - let managed_path = tmp.path().join("managed_config.toml"); - - std::fs::write( - tmp.path().join(CONFIG_TOML_FILE), - r#"foo = 1 - -[nested] -value = "base" -"#, - ) - .expect("write base"); - std::fs::write( - &managed_path, - r#"foo = 2 - -[nested] -value = "managed_config" -extra = true -"#, - ) - .expect("write managed config"); - - let overrides = LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }; - - let loaded = load_config_as_toml_with_overrides(tmp.path(), overrides) - .await - .expect("load config"); - let table = loaded.as_table().expect("top-level table expected"); - - assert_eq!(table.get("foo"), Some(&TomlValue::Integer(2))); - let nested = table - .get("nested") - .and_then(|v| v.as_table()) - .expect("nested"); - assert_eq!( - nested.get("value"), - Some(&TomlValue::String("managed_config".to_string())) - ); - assert_eq!(nested.get("extra"), Some(&TomlValue::Boolean(true))); - } - - #[tokio::test] - async fn returns_empty_when_all_layers_missing() { - let tmp = tempdir().expect("tempdir"); - let managed_path = tmp.path().join("managed_config.toml"); - let overrides = LoaderOverrides { - managed_config_path: Some(managed_path), - #[cfg(target_os = "macos")] - managed_preferences_base64: None, - }; - - let layers = load_config_layers_with_overrides(tmp.path(), overrides) - .await - .expect("load layers"); - let base_table = layers.base.as_table().expect("base table expected"); - assert!( - base_table.is_empty(), - "expected empty base layer when configs missing" - ); - assert!( - layers.managed_config.is_none(), - "managed config layer should be absent when file missing" - ); - - #[cfg(not(target_os = "macos"))] - { - let loaded = load_config_as_toml(tmp.path()).await.expect("load config"); - let table = loaded.as_table().expect("top-level table expected"); - assert!( - table.is_empty(), - "expected empty table when configs missing" - ); - } - } - - #[cfg(target_os = "macos")] - #[tokio::test] - async fn managed_preferences_take_highest_precedence() { - use base64::Engine; - - let managed_payload = r#" -[nested] -value = "managed" -flag = false -"#; - let encoded = base64::prelude::BASE64_STANDARD.encode(managed_payload.as_bytes()); - let tmp = tempdir().expect("tempdir"); - let managed_path = tmp.path().join("managed_config.toml"); - - std::fs::write( - tmp.path().join(CONFIG_TOML_FILE), - r#"[nested] -value = "base" -"#, - ) - .expect("write base"); - std::fs::write( - &managed_path, - r#"[nested] -value = "managed_config" -flag = true -"#, - ) - .expect("write managed config"); - - let overrides = LoaderOverrides { - managed_config_path: Some(managed_path), - managed_preferences_base64: Some(encoded), - }; - - let loaded = load_config_as_toml_with_overrides(tmp.path(), overrides) - .await - .expect("load config"); - let nested = loaded - .get("nested") - .and_then(|v| v.as_table()) - .expect("nested table"); - assert_eq!( - nested.get("value"), - Some(&TomlValue::String("managed".to_string())) - ); - assert_eq!(nested.get("flag"), Some(&TomlValue::Boolean(false))); - } -} diff --git a/codex-rs/core/src/config_loader/overrides.rs b/codex-rs/core/src/config_loader/overrides.rs new file mode 100644 index 000000000..e2ae6375a --- /dev/null +++ b/codex-rs/core/src/config_loader/overrides.rs @@ -0,0 +1,55 @@ +use toml::Value as TomlValue; + +pub(super) fn default_empty_table() -> TomlValue { + TomlValue::Table(Default::default()) +} + +pub(super) fn build_cli_overrides_layer(cli_overrides: &[(String, TomlValue)]) -> TomlValue { + let mut root = default_empty_table(); + for (path, value) in cli_overrides { + apply_toml_override(&mut root, path, value.clone()); + } + root +} + +/// Apply a single dotted-path override onto a TOML value. +fn apply_toml_override(root: &mut TomlValue, path: &str, value: TomlValue) { + use toml::value::Table; + + let mut current = root; + let mut segments_iter = path.split('.').peekable(); + + while let Some(segment) = segments_iter.next() { + let is_last = segments_iter.peek().is_none(); + + if is_last { + match current { + TomlValue::Table(table) => { + table.insert(segment.to_string(), value); + } + _ => { + let mut table = Table::new(); + table.insert(segment.to_string(), value); + *current = TomlValue::Table(table); + } + } + return; + } + + match current { + TomlValue::Table(table) => { + current = table + .entry(segment.to_string()) + .or_insert_with(|| TomlValue::Table(Table::new())); + } + _ => { + *current = TomlValue::Table(Table::new()); + if let TomlValue::Table(tbl) = current { + current = tbl + .entry(segment.to_string()) + .or_insert_with(|| TomlValue::Table(Table::new())); + } + } + } + } +} diff --git a/codex-rs/core/src/config_loader/state.rs b/codex-rs/core/src/config_loader/state.rs new file mode 100644 index 000000000..e23cc243c --- /dev/null +++ b/codex-rs/core/src/config_loader/state.rs @@ -0,0 +1,128 @@ +use super::fingerprint::record_origins; +use super::fingerprint::version_for_toml; +use super::merge::merge_toml_values; +use codex_app_server_protocol::ConfigLayer; +use codex_app_server_protocol::ConfigLayerMetadata; +use codex_app_server_protocol::ConfigLayerName; +use serde_json::Value as JsonValue; +use std::collections::HashMap; +use std::path::PathBuf; +use toml::Value as TomlValue; + +#[derive(Debug, Default, Clone)] +pub struct LoaderOverrides { + pub managed_config_path: Option, + #[cfg(target_os = "macos")] + pub managed_preferences_base64: Option, +} + +#[derive(Debug, Clone)] +pub struct ConfigLayerEntry { + pub name: ConfigLayerName, + pub source: PathBuf, + pub config: TomlValue, + pub version: String, +} + +impl ConfigLayerEntry { + pub fn new(name: ConfigLayerName, source: PathBuf, config: TomlValue) -> Self { + let version = version_for_toml(&config); + Self { + name, + source, + config, + version, + } + } + + pub fn metadata(&self) -> ConfigLayerMetadata { + ConfigLayerMetadata { + name: self.name.clone(), + source: self.source.display().to_string(), + version: self.version.clone(), + } + } + + pub fn as_layer(&self) -> ConfigLayer { + ConfigLayer { + name: self.name.clone(), + source: self.source.display().to_string(), + version: self.version.clone(), + config: serde_json::to_value(&self.config).unwrap_or(JsonValue::Null), + } + } +} + +#[derive(Debug, Clone)] +pub struct ConfigLayerStack { + pub user: ConfigLayerEntry, + pub session_flags: ConfigLayerEntry, + pub system: Option, + pub mdm: Option, +} + +impl ConfigLayerStack { + pub fn with_user_config(&self, user_config: TomlValue) -> Self { + Self { + user: ConfigLayerEntry::new( + self.user.name.clone(), + self.user.source.clone(), + user_config, + ), + session_flags: self.session_flags.clone(), + system: self.system.clone(), + mdm: self.mdm.clone(), + } + } + + pub fn effective_config(&self) -> TomlValue { + let mut merged = self.user.config.clone(); + merge_toml_values(&mut merged, &self.session_flags.config); + if let Some(system) = &self.system { + merge_toml_values(&mut merged, &system.config); + } + if let Some(mdm) = &self.mdm { + merge_toml_values(&mut merged, &mdm.config); + } + merged + } + + pub fn origins(&self) -> HashMap { + let mut origins = HashMap::new(); + let mut path = Vec::new(); + + record_origins( + &self.user.config, + &self.user.metadata(), + &mut path, + &mut origins, + ); + record_origins( + &self.session_flags.config, + &self.session_flags.metadata(), + &mut path, + &mut origins, + ); + if let Some(system) = &self.system { + record_origins(&system.config, &system.metadata(), &mut path, &mut origins); + } + if let Some(mdm) = &self.mdm { + record_origins(&mdm.config, &mdm.metadata(), &mut path, &mut origins); + } + + origins + } + + pub fn layers_high_to_low(&self) -> Vec { + let mut layers = Vec::new(); + if let Some(mdm) = &self.mdm { + layers.push(mdm.as_layer()); + } + if let Some(system) = &self.system { + layers.push(system.as_layer()); + } + layers.push(self.session_flags.as_layer()); + layers.push(self.user.as_layer()); + layers + } +} diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs new file mode 100644 index 000000000..121ef736d --- /dev/null +++ b/codex-rs/core/src/config_loader/tests.rs @@ -0,0 +1,138 @@ +use super::LoaderOverrides; +use super::load_config_layers_state; +use crate::config::CONFIG_TOML_FILE; +use tempfile::tempdir; +use toml::Value as TomlValue; + +#[tokio::test] +async fn merges_managed_config_layer_on_top() { + let tmp = tempdir().expect("tempdir"); + let managed_path = tmp.path().join("managed_config.toml"); + + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#"foo = 1 + +[nested] +value = "base" +"#, + ) + .expect("write base"); + std::fs::write( + &managed_path, + r#"foo = 2 + +[nested] +value = "managed_config" +extra = true +"#, + ) + .expect("write managed config"); + + let overrides = LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }; + + let state = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides) + .await + .expect("load config"); + let loaded = state.effective_config(); + let table = loaded.as_table().expect("top-level table expected"); + + assert_eq!(table.get("foo"), Some(&TomlValue::Integer(2))); + let nested = table + .get("nested") + .and_then(|v| v.as_table()) + .expect("nested"); + assert_eq!( + nested.get("value"), + Some(&TomlValue::String("managed_config".to_string())) + ); + assert_eq!(nested.get("extra"), Some(&TomlValue::Boolean(true))); +} + +#[tokio::test] +async fn returns_empty_when_all_layers_missing() { + let tmp = tempdir().expect("tempdir"); + let managed_path = tmp.path().join("managed_config.toml"); + let overrides = LoaderOverrides { + managed_config_path: Some(managed_path), + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + }; + + let layers = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides) + .await + .expect("load layers"); + let base_table = layers.user.config.as_table().expect("base table expected"); + assert!( + base_table.is_empty(), + "expected empty base layer when configs missing" + ); + assert!( + layers.system.is_none(), + "managed config layer should be absent when file missing" + ); + + #[cfg(not(target_os = "macos"))] + { + let effective = layers.effective_config(); + let table = effective.as_table().expect("top-level table expected"); + assert!( + table.is_empty(), + "expected empty table when configs missing" + ); + } +} + +#[cfg(target_os = "macos")] +#[tokio::test] +async fn managed_preferences_take_highest_precedence() { + use base64::Engine; + + let managed_payload = r#" +[nested] +value = "managed" +flag = false +"#; + let encoded = base64::prelude::BASE64_STANDARD.encode(managed_payload.as_bytes()); + let tmp = tempdir().expect("tempdir"); + let managed_path = tmp.path().join("managed_config.toml"); + + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#"[nested] +value = "base" +"#, + ) + .expect("write base"); + std::fs::write( + &managed_path, + r#"[nested] +value = "managed_config" +flag = true +"#, + ) + .expect("write managed config"); + + let overrides = LoaderOverrides { + managed_config_path: Some(managed_path), + managed_preferences_base64: Some(encoded), + }; + + let state = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides) + .await + .expect("load config"); + let loaded = state.effective_config(); + let nested = loaded + .get("nested") + .and_then(|v| v.as_table()) + .expect("nested table"); + assert_eq!( + nested.get("value"), + Some(&TomlValue::String("managed".to_string())) + ); + assert_eq!(nested.get("flag"), Some(&TomlValue::Boolean(false))); +} From 570eb5fe7884ae73a684944a1c17077a6d7845fd Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Fri, 12 Dec 2025 12:21:53 -0800 Subject: [PATCH 02/79] chore(prompt) Remove truncation details (#7941) Fixes #7867 and #7906 ## Summary Update truncation details. --- codex-rs/core/gpt_5_1_prompt.md | 2 +- codex-rs/core/gpt_5_2_prompt.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/gpt_5_1_prompt.md b/codex-rs/core/gpt_5_1_prompt.md index 3201ffeb6..a4492c6ac 100644 --- a/codex-rs/core/gpt_5_1_prompt.md +++ b/codex-rs/core/gpt_5_1_prompt.md @@ -319,7 +319,7 @@ For casual greetings, acknowledgements, or other one-off conversational messages When using the shell, you must adhere to the following guidelines: - When searching for text or files, prefer using `rg` or `rg --files` respectively because `rg` is much faster than alternatives like `grep`. (If the `rg` command is not found, then use alternatives.) -- Read files in chunks with a max chunk size of 250 lines. Do not use python scripts to attempt to output larger chunks of a file. Command line output will be truncated after 10 kilobytes or 256 lines of output, regardless of the command used. +- Do not use python scripts to attempt to output larger chunks of a file. ## apply_patch diff --git a/codex-rs/core/gpt_5_2_prompt.md b/codex-rs/core/gpt_5_2_prompt.md index 99a3f49bb..cfbb22084 100644 --- a/codex-rs/core/gpt_5_2_prompt.md +++ b/codex-rs/core/gpt_5_2_prompt.md @@ -285,7 +285,7 @@ For casual greetings, acknowledgements, or other one-off conversational messages When using the shell, you must adhere to the following guidelines: - When searching for text or files, prefer using `rg` or `rg --files` respectively because `rg` is much faster than alternatives like `grep`. (If the `rg` command is not found, then use alternatives.) -- Do not use python scripts to attempt to output larger chunks of a file. Command line output will be truncated after 10 kilobytes, regardless of the command used. +- Do not use python scripts to attempt to output larger chunks of a file. - Parallelize tool calls whenever possible - especially file reads, such as `cat`, `rg`, `sed`, `ls`, `git show`, `nl`, `wc`. Use `multi_tool_use.parallel` to parallelize tool calls and only this. ## apply_patch From 677732ff6541fad141823698f7e85920f036b2e1 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Fri, 12 Dec 2025 12:30:38 -0800 Subject: [PATCH 03/79] Elevated Sandbox 4 (#7889) --- codex-rs/Cargo.lock | 31 +- codex-rs/cli/src/debug_sandbox.rs | 34 +- codex-rs/core/src/config/mod.rs | 7 +- codex-rs/core/src/exec.rs | 33 +- codex-rs/core/src/features.rs | 8 + codex-rs/core/src/safety.rs | 22 + .../windows-sandbox-rs/src/elevated_impl.rs | 588 ++++++++++++++++++ codex-rs/windows-sandbox-rs/src/lib.rs | 67 +- codex-rs/windows-sandbox-rs/src/process.rs | 35 +- codex-rs/windows-sandbox-rs/src/winutil.rs | 42 ++ 10 files changed, 739 insertions(+), 128 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/elevated_impl.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ec5911a4b..1979b1fda 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1253,7 +1253,7 @@ dependencies = [ "allocative", "anyhow", "clap", - "derive_more 2.1.0", + "derive_more 2.0.1", "env_logger", "log", "multimap", @@ -1545,7 +1545,7 @@ dependencies = [ "codex-windows-sandbox", "color-eyre", "crossterm", - "derive_more 2.1.0", + "derive_more 2.0.1", "diffy", "dirs", "dunce", @@ -1614,7 +1614,7 @@ dependencies = [ "codex-windows-sandbox", "color-eyre", "crossterm", - "derive_more 2.1.0", + "derive_more 2.0.1", "diffy", "dirs", "dunce", @@ -1835,9 +1835,9 @@ dependencies = [ [[package]] name = "convert_case" -version = "0.10.0" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "633458d4ef8c78b72454de2d54fd6ab2e60f9e02be22f3c6104cdc8a4e0fceb9" +checksum = "bb402b8d4c85569410425650ce3eddc7d698ed96d39a73f941b08fb63082f1e7" dependencies = [ "unicode-segmentation", ] @@ -2176,11 +2176,11 @@ dependencies = [ [[package]] name = "derive_more" -version = "2.1.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "10b768e943bed7bf2cab53df09f4bc34bfd217cdb57d971e769874c9a6710618" +checksum = "093242cf7570c207c83073cf82f79706fe7b8317e98620a47d5be7c3d8497678" dependencies = [ - "derive_more-impl 2.1.0", + "derive_more-impl 2.0.1", ] [[package]] @@ -2198,14 +2198,13 @@ dependencies = [ [[package]] name = "derive_more-impl" -version = "2.1.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d286bfdaf75e988b4a78e013ecd79c581e06399ab53fbacd2d916c2f904f30b" +checksum = "bda628edc44c4bb645fbe0f758797143e4e07926f7ebf4e9bdfbd3d2ce621df3" dependencies = [ - "convert_case 0.10.0", + "convert_case 0.7.1", "proc-macro2", "quote", - "rustc_version", "syn 2.0.104", "unicode-xid", ] @@ -6989,9 +6988,9 @@ checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" [[package]] name = "ts-rs" -version = "11.1.0" +version = "11.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4994acea2522cd2b3b85c1d9529a55991e3ad5e25cdcd3de9d505972c4379424" +checksum = "6ef1b7a6d914a34127ed8e1fa927eb7088903787bcded4fa3eef8f85ee1568be" dependencies = [ "serde_json", "thiserror 2.0.17", @@ -7001,9 +7000,9 @@ dependencies = [ [[package]] name = "ts-rs-macros" -version = "11.1.0" +version = "11.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee6ff59666c9cbaec3533964505d39154dc4e0a56151fdea30a09ed0301f62e2" +checksum = "e9d4ed7b4c18cc150a6a0a1e9ea1ecfa688791220781af6e119f9599a8502a0a" dependencies = [ "proc-macro2", "quote", diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 26fecd55c..c298a3511 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -136,7 +136,9 @@ async fn run_command_under_sandbox( if let SandboxType::Windows = sandbox_type { #[cfg(target_os = "windows")] { + use codex_core::features::Feature; use codex_windows_sandbox::run_windows_sandbox_capture; + use codex_windows_sandbox::run_windows_sandbox_capture_elevated; let policy_str = serde_json::to_string(&config.sandbox_policy)?; @@ -145,18 +147,32 @@ async fn run_command_under_sandbox( let env_map = env.clone(); let command_vec = command.clone(); let base_dir = config.codex_home.clone(); + let use_elevated = config.features.enabled(Feature::WindowsSandbox) + && config.features.enabled(Feature::WindowsSandboxElevated); // Preflight audit is invoked elsewhere at the appropriate times. let res = tokio::task::spawn_blocking(move || { - run_windows_sandbox_capture( - policy_str.as_str(), - &sandbox_cwd, - base_dir.as_path(), - command_vec, - &cwd_clone, - env_map, - None, - ) + if use_elevated { + run_windows_sandbox_capture_elevated( + policy_str.as_str(), + &sandbox_cwd, + base_dir.as_path(), + command_vec, + &cwd_clone, + env_map, + None, + ) + } else { + run_windows_sandbox_capture( + policy_str.as_str(), + &sandbox_cwd, + base_dir.as_path(), + command_vec, + &cwd_clone, + env_map, + None, + ) + } }) .await; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 584708156..a6febb55c 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -956,7 +956,12 @@ impl Config { let features = Features::from_config(&cfg, &config_profile, feature_overrides); #[cfg(target_os = "windows")] { - crate::safety::set_windows_sandbox_enabled(features.enabled(Feature::WindowsSandbox)); + // Base flag controls sandbox on/off; elevated only applies when base is enabled. + let sandbox_enabled = features.enabled(Feature::WindowsSandbox); + crate::safety::set_windows_sandbox_enabled(sandbox_enabled); + let elevated_enabled = + sandbox_enabled && features.enabled(Feature::WindowsSandboxElevated); + crate::safety::set_windows_elevated_sandbox_enabled(elevated_enabled); } let resolved_cwd = { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 596f32505..da113ae42 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -220,7 +220,9 @@ async fn exec_windows_sandbox( sandbox_policy: &SandboxPolicy, ) -> Result { use crate::config::find_codex_home; + use crate::safety::is_windows_elevated_sandbox_enabled; use codex_windows_sandbox::run_windows_sandbox_capture; + use codex_windows_sandbox::run_windows_sandbox_capture_elevated; let ExecParams { command, @@ -244,16 +246,29 @@ async fn exec_windows_sandbox( "windows sandbox: failed to resolve codex_home: {err}" ))) })?; + let use_elevated = is_windows_elevated_sandbox_enabled(); let spawn_res = tokio::task::spawn_blocking(move || { - run_windows_sandbox_capture( - policy_str.as_str(), - &sandbox_cwd, - codex_home.as_ref(), - command, - &cwd, - env, - timeout_ms, - ) + if use_elevated { + run_windows_sandbox_capture_elevated( + policy_str.as_str(), + &sandbox_cwd, + codex_home.as_ref(), + command, + &cwd, + env, + timeout_ms, + ) + } else { + run_windows_sandbox_capture( + policy_str.as_str(), + &sandbox_cwd, + codex_home.as_ref(), + command, + &cwd, + env, + timeout_ms, + ) + } }) .await; diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index a011884fc..20f405bf2 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -50,6 +50,8 @@ pub enum Feature { ExecPolicy, /// Enable Windows sandbox (restricted token) on Windows. WindowsSandbox, + /// Use the elevated Windows sandbox pipeline (setup + runner). + WindowsSandboxElevated, /// Remote compaction enabled (only for ChatGPT auth) RemoteCompaction, /// Refresh remote models and emit AppReady once the list is available. @@ -327,6 +329,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Experimental, default_enabled: false, }, + FeatureSpec { + id: Feature::WindowsSandboxElevated, + key: "enable_elevated_windows_sandbox", + stage: Stage::Experimental, + default_enabled: false, + }, FeatureSpec { id: Feature::RemoteCompaction, key: "remote_compaction", diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index e82e10745..8ab120ca6 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -18,6 +18,8 @@ use std::sync::atomic::Ordering; #[cfg(target_os = "windows")] static WINDOWS_SANDBOX_ENABLED: AtomicBool = AtomicBool::new(false); +#[cfg(target_os = "windows")] +static WINDOWS_ELEVATED_SANDBOX_ENABLED: AtomicBool = AtomicBool::new(false); #[cfg(target_os = "windows")] pub fn set_windows_sandbox_enabled(enabled: bool) { @@ -28,6 +30,26 @@ pub fn set_windows_sandbox_enabled(enabled: bool) { #[allow(dead_code)] pub fn set_windows_sandbox_enabled(_enabled: bool) {} +#[cfg(target_os = "windows")] +pub fn set_windows_elevated_sandbox_enabled(enabled: bool) { + WINDOWS_ELEVATED_SANDBOX_ENABLED.store(enabled, Ordering::Relaxed); +} + +#[cfg(not(target_os = "windows"))] +#[allow(dead_code)] +pub fn set_windows_elevated_sandbox_enabled(_enabled: bool) {} + +#[cfg(target_os = "windows")] +pub fn is_windows_elevated_sandbox_enabled() -> bool { + WINDOWS_ELEVATED_SANDBOX_ENABLED.load(Ordering::Relaxed) +} + +#[cfg(not(target_os = "windows"))] +#[allow(dead_code)] +pub fn is_windows_elevated_sandbox_enabled() -> bool { + false +} + #[derive(Debug, PartialEq)] pub enum SafetyCheck { AutoApprove { diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs new file mode 100644 index 000000000..b72ee7597 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -0,0 +1,588 @@ +mod windows_impl { + use crate::acl::allow_null_device; + use crate::allow::compute_allow_paths; + use crate::allow::AllowDenyPaths; + use crate::cap::cap_sid_file; + use crate::cap::load_or_create_cap_sids; + use crate::env::ensure_non_interactive_pager; + use crate::env::inherit_path_env; + use crate::env::normalize_null_device_env; + use crate::identity::require_logon_sandbox_creds; + use crate::logging::debug_log; + use crate::logging::log_failure; + use crate::logging::log_note; + use crate::logging::log_start; + use crate::logging::log_success; + use crate::policy::parse_policy; + use crate::policy::SandboxPolicy; + use crate::token::convert_string_sid_to_sid; + use crate::winutil::format_last_error; + use crate::winutil::quote_windows_arg; + use crate::winutil::to_wide; + use anyhow::Result; + use rand::rngs::SmallRng; + use rand::Rng; + use rand::SeedableRng; + use std::collections::HashMap; + use std::ffi::c_void; + use std::fs; + use std::io; + use std::path::Path; + use std::path::PathBuf; + use std::ptr; + use windows_sys::Win32::Foundation::CloseHandle; + use windows_sys::Win32::Foundation::GetLastError; + use windows_sys::Win32::Foundation::HANDLE; + use windows_sys::Win32::Security::Authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW; + use windows_sys::Win32::Security::PSECURITY_DESCRIPTOR; + use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; + use windows_sys::Win32::System::Diagnostics::Debug::SetErrorMode; + use windows_sys::Win32::System::Pipes::ConnectNamedPipe; + use windows_sys::Win32::System::Pipes::CreateNamedPipeW; + // PIPE_ACCESS_DUPLEX is 0x00000003; not exposed in windows-sys 0.52, so use the value directly. + const PIPE_ACCESS_DUPLEX: u32 = 0x0000_0003; + use windows_sys::Win32::System::Pipes::PIPE_READMODE_BYTE; + use windows_sys::Win32::System::Pipes::PIPE_TYPE_BYTE; + use windows_sys::Win32::System::Pipes::PIPE_WAIT; + use windows_sys::Win32::System::Threading::CreateProcessWithLogonW; + use windows_sys::Win32::System::Threading::GetExitCodeProcess; + use windows_sys::Win32::System::Threading::WaitForSingleObject; + use windows_sys::Win32::System::Threading::INFINITE; + use windows_sys::Win32::System::Threading::LOGON_WITH_PROFILE; + use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; + use windows_sys::Win32::System::Threading::STARTUPINFOW; + + /// Ensures the parent directory of a path exists before writing to it. + fn ensure_dir(p: &Path) -> Result<()> { + if let Some(d) = p.parent() { + std::fs::create_dir_all(d)?; + } + Ok(()) + } + + /// Walks upward from `start` to locate the git worktree root, following gitfile redirects. + fn find_git_root(start: &Path) -> Option { + let mut cur = dunce::canonicalize(start).ok()?; + loop { + let marker = cur.join(".git"); + if marker.is_dir() { + return Some(cur); + } + if marker.is_file() { + if let Ok(txt) = std::fs::read_to_string(&marker) { + if let Some(rest) = txt.trim().strip_prefix("gitdir:") { + let gitdir = rest.trim(); + let resolved = if Path::new(gitdir).is_absolute() { + PathBuf::from(gitdir) + } else { + cur.join(gitdir) + }; + return resolved.parent().map(|p| p.to_path_buf()).or(Some(cur)); + } + } + return Some(cur); + } + let parent = cur.parent()?; + if parent == cur { + return None; + } + cur = parent.to_path_buf(); + } + } + + /// Creates the sandbox user's Codex home directory if it does not already exist. + fn ensure_codex_home_exists(p: &Path) -> Result<()> { + std::fs::create_dir_all(p)?; + Ok(()) + } + + /// Adds a git safe.directory entry to the environment when running inside a repository. + /// git will not otherwise allow the Sandbox user to run git commands on the repo directory + /// which is owned by the primary user. + fn inject_git_safe_directory( + env_map: &mut HashMap, + cwd: &Path, + logs_base_dir: Option<&Path>, + ) { + if let Some(git_root) = find_git_root(cwd) { + let mut cfg_count: usize = env_map + .get("GIT_CONFIG_COUNT") + .and_then(|v| v.parse::().ok()) + .unwrap_or(0); + let git_path = git_root.to_string_lossy().replace("\\\\", "/"); + env_map.insert( + format!("GIT_CONFIG_KEY_{cfg_count}"), + "safe.directory".to_string(), + ); + env_map.insert(format!("GIT_CONFIG_VALUE_{cfg_count}"), git_path); + cfg_count += 1; + env_map.insert("GIT_CONFIG_COUNT".to_string(), cfg_count.to_string()); + log_note( + &format!("injected git safe.directory for {}", git_root.display()), + logs_base_dir, + ); + } + } + + /// Locates `codex-command-runner.exe` next to the current binary. + fn find_runner_exe() -> PathBuf { + if let Ok(exe) = std::env::current_exe() { + if let Some(dir) = exe.parent() { + let candidate = dir.join("codex-command-runner.exe"); + if candidate.exists() { + return candidate; + } + } + } + PathBuf::from("codex-command-runner.exe") + } + + /// Generates a unique named-pipe path used to communicate with the runner process. + fn pipe_name(suffix: &str) -> String { + let mut rng = SmallRng::from_entropy(); + format!(r"\\.\pipe\codex-runner-{:x}-{}", rng.gen::(), suffix) + } + + /// Creates a named pipe with permissive ACLs so the sandbox user can connect. + fn create_named_pipe(name: &str, access: u32) -> io::Result { + // Allow sandbox users to connect by granting Everyone full access on the pipe. + let sddl = to_wide("D:(A;;GA;;;WD)"); + let mut sd: PSECURITY_DESCRIPTOR = ptr::null_mut(); + let ok = unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + 1, // SDDL_REVISION_1 + &mut sd, + ptr::null_mut(), + ) + }; + if ok == 0 { + return Err(io::Error::from_raw_os_error(unsafe { + GetLastError() as i32 + })); + } + let mut sa = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: sd, + bInheritHandle: 0, + }; + let wide = to_wide(name); + let h = unsafe { + CreateNamedPipeW( + wide.as_ptr(), + access, + PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + 1, + 65536, + 65536, + 0, + &mut sa as *mut SECURITY_ATTRIBUTES, + ) + }; + if h == 0 || h == windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE { + return Err(io::Error::from_raw_os_error(unsafe { + GetLastError() as i32 + })); + } + Ok(h) + } + + /// Waits for a client connection on the named pipe, tolerating an existing connection. + fn connect_pipe(h: HANDLE) -> io::Result<()> { + let ok = unsafe { ConnectNamedPipe(h, ptr::null_mut()) }; + if ok == 0 { + let err = unsafe { GetLastError() }; + const ERROR_PIPE_CONNECTED: u32 = 535; + if err != ERROR_PIPE_CONNECTED { + return Err(io::Error::from_raw_os_error(err as i32)); + } + } + Ok(()) + } + + pub use crate::windows_impl::CaptureResult; + + #[derive(serde::Serialize)] + struct RunnerPayload { + policy_json_or_preset: String, + sandbox_policy_cwd: PathBuf, + // Writable log dir for sandbox user (.codex in sandbox profile). + codex_home: PathBuf, + // Real user's CODEX_HOME for shared data (caps, config). + real_codex_home: PathBuf, + cap_sid: String, + request_file: Option, + command: Vec, + cwd: PathBuf, + env_map: HashMap, + timeout_ms: Option, + stdin_pipe: String, + stdout_pipe: String, + stderr_pipe: String, + } + + /// Launches the command runner under the sandbox user and captures its output. + pub fn run_windows_sandbox_capture( + policy_json_or_preset: &str, + sandbox_policy_cwd: &Path, + codex_home: &Path, + command: Vec, + cwd: &Path, + mut env_map: HashMap, + timeout_ms: Option, + ) -> Result { + let policy = parse_policy(policy_json_or_preset)?; + normalize_null_device_env(&mut env_map); + ensure_non_interactive_pager(&mut env_map); + inherit_path_env(&mut env_map); + inject_git_safe_directory(&mut env_map, cwd, None); + let current_dir = cwd.to_path_buf(); + // Use a temp-based log dir that the sandbox user can write. + let sandbox_base = codex_home.join(".sandbox"); + ensure_codex_home_exists(&sandbox_base)?; + + let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path()); + log_start(&command, logs_base_dir); + let sandbox_creds = + require_logon_sandbox_creds(&policy, sandbox_policy_cwd, cwd, &env_map, codex_home)?; + log_note("cli creds ready", logs_base_dir); + let cap_sid_path = cap_sid_file(codex_home); + + // Build capability SID for ACL grants. + let (psid_to_use, cap_sid_str) = match &policy { + SandboxPolicy::ReadOnly => { + let caps = load_or_create_cap_sids(codex_home); + ensure_dir(&cap_sid_path)?; + fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?; + ( + unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() }, + caps.readonly.clone(), + ) + } + SandboxPolicy::WorkspaceWrite { .. } => { + let caps = load_or_create_cap_sids(codex_home); + ensure_dir(&cap_sid_path)?; + fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?; + ( + unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() }, + caps.workspace.clone(), + ) + } + SandboxPolicy::DangerFullAccess => { + anyhow::bail!("DangerFullAccess is not supported for sandboxing") + } + }; + + let AllowDenyPaths { allow, deny } = + compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map); + // Deny/allow ACEs are now applied during setup; avoid per-command churn. + log_note( + &format!( + "cli skipping per-command ACL grants (allow_count={} deny_count={})", + allow.len(), + deny.len() + ), + logs_base_dir, + ); + unsafe { + allow_null_device(psid_to_use); + } + + // Prepare named pipes for runner. + let stdin_name = pipe_name("stdin"); + let stdout_name = pipe_name("stdout"); + let stderr_name = pipe_name("stderr"); + let h_stdin_pipe = create_named_pipe( + &stdin_name, + PIPE_ACCESS_DUPLEX | PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + )?; + let h_stdout_pipe = create_named_pipe( + &stdout_name, + PIPE_ACCESS_DUPLEX | PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + )?; + let h_stderr_pipe = create_named_pipe( + &stderr_name, + PIPE_ACCESS_DUPLEX | PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, + )?; + log_note( + &format!( + "cli pipes created stdin={} stdout={} stderr={}", + stdin_name, stdout_name, stderr_name + ), + logs_base_dir, + ); + + // Launch runner as sandbox user via CreateProcessWithLogonW. + let runner_exe = find_runner_exe(); + let runner_cmdline = runner_exe + .to_str() + .map(|s| s.to_string()) + .unwrap_or_else(|| "codex-command-runner.exe".to_string()); + // Write request to a file under the sandbox base dir for the runner to read. + // TODO(iceweasel) - use a different mechanism for invoking the runner. + let base_tmp = sandbox_base.join("requests"); + std::fs::create_dir_all(&base_tmp)?; + let mut rng = SmallRng::from_entropy(); + let req_file = base_tmp.join(format!("request-{:x}.json", rng.gen::())); + let payload = RunnerPayload { + policy_json_or_preset: policy_json_or_preset.to_string(), + sandbox_policy_cwd: sandbox_policy_cwd.to_path_buf(), + codex_home: sandbox_base.clone(), + real_codex_home: codex_home.to_path_buf(), + cap_sid: cap_sid_str.clone(), + request_file: Some(req_file.clone()), + command: command.clone(), + cwd: cwd.to_path_buf(), + env_map: env_map.clone(), + timeout_ms, + stdin_pipe: stdin_name.clone(), + stdout_pipe: stdout_name.clone(), + stderr_pipe: stderr_name.clone(), + }; + let payload_json = serde_json::to_string(&payload)?; + if let Err(e) = fs::write(&req_file, &payload_json) { + log_note( + &format!("error writing request file {}: {}", req_file.display(), e), + logs_base_dir, + ); + return Err(e.into()); + } + log_note( + &format!("cli request file written path={}", req_file.display()), + logs_base_dir, + ); + let runner_full_cmd = format!( + "{} {}", + quote_windows_arg(&runner_cmdline), + quote_windows_arg(&format!("--request-file={}", req_file.display())) + ); + let mut cmdline_vec: Vec = to_wide(&runner_full_cmd); + let exe_w: Vec = to_wide(&runner_cmdline); + let cwd_w: Vec = to_wide(cwd); + log_note( + &format!("cli prep done request_file={}", req_file.display()), + logs_base_dir, + ); + log_note( + &format!("cli about to launch runner cmd={}", runner_full_cmd), + logs_base_dir, + ); + + // Minimal CPWL launch: inherit env, no desktop override, no handle inheritance. + let env_block: Option> = None; + let mut si: STARTUPINFOW = unsafe { std::mem::zeroed() }; + si.cb = std::mem::size_of::() as u32; + let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; + let user_w = to_wide(&sandbox_creds.username); + let domain_w = to_wide("."); + let password_w = to_wide(&sandbox_creds.password); + // Suppress WER/UI popups from the runner process so we can collect exit codes. + let _ = unsafe { SetErrorMode(0x0001 | 0x0002) }; // SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX + + // Ensure command line buffer is mutable and includes the exe as argv[0]. + let spawn_res = unsafe { + CreateProcessWithLogonW( + user_w.as_ptr(), + domain_w.as_ptr(), + password_w.as_ptr(), + LOGON_WITH_PROFILE, + exe_w.as_ptr(), + cmdline_vec.as_mut_ptr(), + windows_sys::Win32::System::Threading::CREATE_NO_WINDOW + | windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT, + env_block + .as_ref() + .map(|b| b.as_ptr() as *const c_void) + .unwrap_or(ptr::null()), + cwd_w.as_ptr(), + &si, + &mut pi, + ) + }; + if spawn_res == 0 { + let err = unsafe { GetLastError() } as i32; + let dbg = format!( + "CreateProcessWithLogonW failed: {} ({}) | cwd={} | cmd={} | req_file={} | env=inherit | si_flags={}", + err, + format_last_error(err), + cwd.display(), + runner_full_cmd, + req_file.display(), + si.dwFlags, + ); + debug_log(&dbg, logs_base_dir); + log_note(&dbg, logs_base_dir); + return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {}", err)); + } + log_note( + &format!("cli runner launched pid={}", pi.hProcess), + logs_base_dir, + ); + + // Pipes are no longer passed as std handles; no stdin payload is sent. + connect_pipe(h_stdin_pipe)?; + connect_pipe(h_stdout_pipe)?; + connect_pipe(h_stderr_pipe)?; + unsafe { + CloseHandle(h_stdin_pipe); + } + + // Read stdout/stderr. + let (tx_out, rx_out) = std::sync::mpsc::channel::>(); + let (tx_err, rx_err) = std::sync::mpsc::channel::>(); + let t_out = std::thread::spawn(move || { + let mut buf = Vec::new(); + let mut tmp = [0u8; 8192]; + loop { + let mut read_bytes: u32 = 0; + let ok = unsafe { + windows_sys::Win32::Storage::FileSystem::ReadFile( + h_stdout_pipe, + tmp.as_mut_ptr(), + tmp.len() as u32, + &mut read_bytes, + std::ptr::null_mut(), + ) + }; + if ok == 0 || read_bytes == 0 { + break; + } + buf.extend_from_slice(&tmp[..read_bytes as usize]); + } + let _ = tx_out.send(buf); + }); + let t_err = std::thread::spawn(move || { + let mut buf = Vec::new(); + let mut tmp = [0u8; 8192]; + loop { + let mut read_bytes: u32 = 0; + let ok = unsafe { + windows_sys::Win32::Storage::FileSystem::ReadFile( + h_stderr_pipe, + tmp.as_mut_ptr(), + tmp.len() as u32, + &mut read_bytes, + std::ptr::null_mut(), + ) + }; + if ok == 0 || read_bytes == 0 { + break; + } + buf.extend_from_slice(&tmp[..read_bytes as usize]); + } + let _ = tx_err.send(buf); + }); + + let timeout = timeout_ms.map(|ms| ms as u32).unwrap_or(INFINITE); + let res = unsafe { WaitForSingleObject(pi.hProcess, timeout) }; + let timed_out = res == 0x0000_0102; + let mut exit_code_u32: u32 = 1; + if !timed_out { + unsafe { + GetExitCodeProcess(pi.hProcess, &mut exit_code_u32); + } + } else { + unsafe { + windows_sys::Win32::System::Threading::TerminateProcess(pi.hProcess, 1); + } + } + + unsafe { + if pi.hThread != 0 { + CloseHandle(pi.hThread); + } + if pi.hProcess != 0 { + CloseHandle(pi.hProcess); + } + CloseHandle(h_stdout_pipe); + CloseHandle(h_stderr_pipe); + } + let _ = t_out.join(); + let _ = t_err.join(); + let stdout = rx_out.recv().unwrap_or_default(); + let stderr = rx_err.recv().unwrap_or_default(); + let exit_code = if timed_out { + 128 + 64 + } else { + exit_code_u32 as i32 + }; + + if exit_code == 0 { + log_success(&command, logs_base_dir); + } else { + log_failure(&command, &format!("exit code {}", exit_code), logs_base_dir); + } + + Ok(CaptureResult { + exit_code, + stdout, + stderr, + timed_out, + }) + } + + #[cfg(test)] + mod tests { + use crate::policy::SandboxPolicy; + + fn workspace_policy(network_access: bool) -> SandboxPolicy { + SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + network_access, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + } + } + + #[test] + fn applies_network_block_when_access_is_disabled() { + assert!(!workspace_policy(false).has_full_network_access()); + } + + #[test] + fn skips_network_block_when_access_is_allowed() { + assert!(workspace_policy(true).has_full_network_access()); + } + + #[test] + fn applies_network_block_for_read_only() { + assert!(!SandboxPolicy::ReadOnly.has_full_network_access()); + } + } +} + +#[cfg(target_os = "windows")] +pub use windows_impl::run_windows_sandbox_capture; + +#[cfg(not(target_os = "windows"))] +mod stub { + use anyhow::bail; + use anyhow::Result; + use codex_protocol::protocol::SandboxPolicy; + use std::collections::HashMap; + use std::path::Path; + + #[derive(Debug, Default)] + pub struct CaptureResult { + pub exit_code: i32, + pub stdout: Vec, + pub stderr: Vec, + pub timed_out: bool, + } + + /// Stub implementation for non-Windows targets; sandboxing only works on Windows. + pub fn run_windows_sandbox_capture( + _policy_json_or_preset: &str, + _sandbox_policy_cwd: &Path, + _codex_home: &Path, + _command: Vec, + _cwd: &Path, + _env_map: HashMap, + _timeout_ms: Option, + ) -> Result { + bail!("Windows sandbox is only available on Windows") + } +} + +#[cfg(not(target_os = "windows"))] +pub use stub::run_windows_sandbox_capture; diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index c4367e612..1bcfa1207 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -12,6 +12,9 @@ windows_modules!( #[path = "setup_orchestrator.rs"] mod setup; +#[cfg(target_os = "windows")] +mod elevated_impl; + #[cfg(target_os = "windows")] pub use acl::allow_null_device; #[cfg(target_os = "windows")] @@ -29,6 +32,8 @@ pub use dpapi::protect as dpapi_protect; #[cfg(target_os = "windows")] pub use dpapi::unprotect as dpapi_unprotect; #[cfg(target_os = "windows")] +pub use elevated_impl::run_windows_sandbox_capture as run_windows_sandbox_capture_elevated; +#[cfg(target_os = "windows")] pub use identity::require_logon_sandbox_creds; #[cfg(target_os = "windows")] pub use logging::log_note; @@ -91,8 +96,10 @@ mod windows_impl { use super::logging::log_success; use super::policy::parse_policy; use super::policy::SandboxPolicy; + use super::process::make_env_block; use super::token::convert_string_sid_to_sid; use super::winutil::format_last_error; + use super::winutil::quote_windows_arg; use super::winutil::to_wide; use anyhow::Result; use std::collections::HashMap; @@ -135,66 +142,6 @@ mod windows_impl { Ok(()) } - fn make_env_block(env: &HashMap) -> Vec { - let mut items: Vec<(String, String)> = - env.iter().map(|(k, v)| (k.clone(), v.clone())).collect(); - items.sort_by(|a, b| { - a.0.to_uppercase() - .cmp(&b.0.to_uppercase()) - .then(a.0.cmp(&b.0)) - }); - let mut w: Vec = Vec::new(); - for (k, v) in items { - let mut s = to_wide(format!("{}={}", k, v)); - s.pop(); - w.extend_from_slice(&s); - w.push(0); - } - w.push(0); - w - } - - // Quote a single Windows command-line argument following the rules used by - // CommandLineToArgvW/CRT so that spaces, quotes, and backslashes are preserved. - // Reference behavior matches Rust std::process::Command on Windows. - fn quote_windows_arg(arg: &str) -> String { - let needs_quotes = arg.is_empty() - || arg - .chars() - .any(|c| matches!(c, ' ' | '\t' | '\n' | '\r' | '"')); - if !needs_quotes { - return arg.to_string(); - } - - let mut quoted = String::with_capacity(arg.len() + 2); - quoted.push('"'); - let mut backslashes = 0; - for ch in arg.chars() { - match ch { - '\\' => { - backslashes += 1; - } - '"' => { - quoted.push_str(&"\\".repeat(backslashes * 2 + 1)); - quoted.push('"'); - backslashes = 0; - } - _ => { - if backslashes > 0 { - quoted.push_str(&"\\".repeat(backslashes)); - backslashes = 0; - } - quoted.push(ch); - } - } - } - if backslashes > 0 { - quoted.push_str(&"\\".repeat(backslashes * 2)); - } - quoted.push('"'); - quoted - } - unsafe fn setup_stdio_pipes() -> io::Result { let mut in_r: HANDLE = 0; let mut in_w: HANDLE = 0; diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index 978dcfe60..786fe416a 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -1,5 +1,6 @@ use crate::logging; use crate::winutil::format_last_error; +use crate::winutil::quote_windows_arg; use crate::winutil::to_wide; use anyhow::anyhow; use anyhow::Result; @@ -50,38 +51,6 @@ pub fn make_env_block(env: &HashMap) -> Vec { w } -#[allow(dead_code)] -fn quote_arg(a: &str) -> String { - let needs_quote = a.is_empty() || a.chars().any(|ch| ch.is_whitespace() || ch == '"'); - if !needs_quote { - return a.to_string(); - } - let mut out = String::from("\""); - let mut bs: usize = 0; - for ch in a.chars() { - if (ch as u32) == 92 { - bs += 1; - continue; - } - if ch == '"' { - out.push_str(&"\\".repeat(bs * 2 + 1)); - out.push('"'); - bs = 0; - continue; - } - if bs > 0 { - out.push_str(&"\\".repeat(bs * 2)); - bs = 0; - } - out.push(ch); - } - if bs > 0 { - out.push_str(&"\\".repeat(bs * 2)); - } - out.push('"'); - out -} - #[allow(dead_code)] unsafe fn ensure_inheritable_stdio(si: &mut STARTUPINFOW) -> Result<()> { for kind in [STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE] { @@ -114,7 +83,7 @@ pub unsafe fn create_process_as_user( ) -> Result<(PROCESS_INFORMATION, STARTUPINFOW)> { let cmdline_str = argv .iter() - .map(|a| quote_arg(a)) + .map(|a| quote_windows_arg(a)) .collect::>() .join(" "); let mut cmdline: Vec = to_wide(&cmdline_str); diff --git a/codex-rs/windows-sandbox-rs/src/winutil.rs b/codex-rs/windows-sandbox-rs/src/winutil.rs index a81956126..02f7a8cce 100644 --- a/codex-rs/windows-sandbox-rs/src/winutil.rs +++ b/codex-rs/windows-sandbox-rs/src/winutil.rs @@ -14,6 +14,48 @@ pub fn to_wide>(s: S) -> Vec { v } +/// Quote a single Windows command-line argument following the rules used by +/// CommandLineToArgvW/CRT so that spaces, quotes, and backslashes are preserved. +/// Reference behavior matches Rust std::process::Command on Windows. +#[cfg(target_os = "windows")] +pub fn quote_windows_arg(arg: &str) -> String { + let needs_quotes = arg.is_empty() + || arg + .chars() + .any(|c| matches!(c, ' ' | '\t' | '\n' | '\r' | '"')); + if !needs_quotes { + return arg.to_string(); + } + + let mut quoted = String::with_capacity(arg.len() + 2); + quoted.push('"'); + let mut backslashes = 0; + for ch in arg.chars() { + match ch { + '\\' => { + backslashes += 1; + } + '"' => { + quoted.push_str(&"\\".repeat(backslashes * 2 + 1)); + quoted.push('"'); + backslashes = 0; + } + _ => { + if backslashes > 0 { + quoted.push_str(&"\\".repeat(backslashes)); + backslashes = 0; + } + quoted.push(ch); + } + } + } + if backslashes > 0 { + quoted.push_str(&"\\".repeat(backslashes * 2)); + } + quoted.push('"'); + quoted +} + // Produce a readable description for a Win32 error code. pub fn format_last_error(err: i32) -> String { unsafe { From 26d0d822a2fcff0975f6ca244632671993e3cfa1 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Fri, 12 Dec 2025 12:50:49 -0800 Subject: [PATCH 04/79] chore(prompt) Update base prompt (#7943) ## Summary Update base prompt --- codex-rs/core/prompt.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/prompt.md b/codex-rs/core/prompt.md index e4590c386..d8bebc371 100644 --- a/codex-rs/core/prompt.md +++ b/codex-rs/core/prompt.md @@ -297,7 +297,7 @@ For casual greetings, acknowledgements, or other one-off conversational messages When using the shell, you must adhere to the following guidelines: - When searching for text or files, prefer using `rg` or `rg --files` respectively because `rg` is much faster than alternatives like `grep`. (If the `rg` command is not found, then use alternatives.) -- Read files in chunks with a max chunk size of 250 lines. Do not use python scripts to attempt to output larger chunks of a file. Command line output will be truncated after 10 kilobytes or 256 lines of output, regardless of the command used. +- Do not use python scripts to attempt to output larger chunks of a file. ## `update_plan` From 9009490357fc3843c14cab1c3e0132acc70487ed Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 12 Dec 2025 13:06:49 -0800 Subject: [PATCH 05/79] fix: use PowerShell to parse PowerShell (#7607) Previous to this PR, we used a hand-rolled PowerShell parser in `windows_safe_commands.rs` to take a `&str` of PowerShell script see if it is equivalent to a list of `execvp(3)` invocations, and if so, we then test each using `is_safe_powershell_command()` to determine if the overall command is safe: https://github.com/openai/codex/blob/6e6338aa876bb4258abe25b02ac6417b8ea9dff0/codex-rs/core/src/command_safety/windows_safe_commands.rs#L89-L98 Unfortunately, our PowerShell parser did not recognize `@(...)` as a special construct, so it was treated as an ordinary token. This meant that the following would erroneously be considered "safe:" ```powershell ls @(calc.exe) ``` The fix introduced in this PR is to do something comparable what we do for Bash/Zsh, which is to use a "proper" parser to derive the list of `execvp(3)` calls. For Bash/Zsh, we rely on https://crates.io/crates/tree-sitter-bash, but there does not appear to be a crate of comparable quality for parsing PowerShell statically (https://github.com/airbus-cert/tree-sitter-powershell/ is the best thing I found). Instead, in this PR, we use a PowerShell script to parse the input PowerShell program to produce the AST. --- .../src/command_safety/powershell_parser.ps1 | 201 +++++++++ .../command_safety/windows_safe_commands.rs | 398 +++++++++++++----- codex-rs/core/src/powershell.rs | 10 +- codex-rs/core/src/tools/handlers/shell.rs | 25 +- 4 files changed, 506 insertions(+), 128 deletions(-) create mode 100644 codex-rs/core/src/command_safety/powershell_parser.ps1 diff --git a/codex-rs/core/src/command_safety/powershell_parser.ps1 b/codex-rs/core/src/command_safety/powershell_parser.ps1 new file mode 100644 index 000000000..af71cb7f3 --- /dev/null +++ b/codex-rs/core/src/command_safety/powershell_parser.ps1 @@ -0,0 +1,201 @@ +$ErrorActionPreference = 'Stop' + +$payload = $env:CODEX_POWERSHELL_PAYLOAD +if ([string]::IsNullOrEmpty($payload)) { + Write-Output '{"status":"parse_failed"}' + exit 0 +} + +try { + $source = + [System.Text.Encoding]::Unicode.GetString( + [System.Convert]::FromBase64String($payload) + ) +} catch { + Write-Output '{"status":"parse_failed"}' + exit 0 +} + +$tokens = $null +$errors = $null + +$ast = $null +try { + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + $source, + [ref]$tokens, + [ref]$errors + ) +} catch { + Write-Output '{"status":"parse_failed"}' + exit 0 +} + +if ($errors.Count -gt 0) { + Write-Output '{"status":"parse_errors"}' + exit 0 +} + +function Convert-CommandElement { + param($element) + + if ($element -is [System.Management.Automation.Language.StringConstantExpressionAst]) { + return @($element.Value) + } + + if ($element -is [System.Management.Automation.Language.ExpandableStringExpressionAst]) { + if ($element.NestedExpressions.Count -gt 0) { + return $null + } + return @($element.Value) + } + + if ($element -is [System.Management.Automation.Language.ConstantExpressionAst]) { + return @($element.Value.ToString()) + } + + if ($element -is [System.Management.Automation.Language.CommandParameterAst]) { + if ($element.Argument -eq $null) { + return @('-' + $element.ParameterName) + } + + if ($element.Argument -is [System.Management.Automation.Language.StringConstantExpressionAst]) { + return @('-' + $element.ParameterName, $element.Argument.Value) + } + + if ($element.Argument -is [System.Management.Automation.Language.ConstantExpressionAst]) { + return @('-' + $element.ParameterName, $element.Argument.Value.ToString()) + } + + return $null + } + + return $null +} + +function Convert-PipelineElement { + param($element) + + if ($element -is [System.Management.Automation.Language.CommandAst]) { + if ($element.Redirections.Count -gt 0) { + return $null + } + + if ( + $element.InvocationOperator -ne $null -and + $element.InvocationOperator -ne [System.Management.Automation.Language.TokenKind]::Unknown + ) { + return $null + } + + $parts = @() + foreach ($commandElement in $element.CommandElements) { + $converted = Convert-CommandElement $commandElement + if ($converted -eq $null) { + return $null + } + $parts += $converted + } + return $parts + } + + if ($element -is [System.Management.Automation.Language.CommandExpressionAst]) { + if ($element.Redirections.Count -gt 0) { + return $null + } + + if ($element.Expression -is [System.Management.Automation.Language.ParenExpressionAst]) { + $innerPipeline = $element.Expression.Pipeline + if ($innerPipeline -and $innerPipeline.PipelineElements.Count -eq 1) { + return Convert-PipelineElement $innerPipeline.PipelineElements[0] + } + } + + return $null + } + + return $null +} + +function Add-CommandsFromPipelineAst { + param($pipeline, $commands) + + if ($pipeline.PipelineElements.Count -eq 0) { + return $false + } + + foreach ($element in $pipeline.PipelineElements) { + $words = Convert-PipelineElement $element + if ($words -eq $null -or $words.Count -eq 0) { + return $false + } + $null = $commands.Add($words) + } + + return $true +} + +function Add-CommandsFromPipelineChain { + param($chain, $commands) + + if (-not (Add-CommandsFromPipelineBase $chain.LhsPipelineChain $commands)) { + return $false + } + + if (-not (Add-CommandsFromPipelineAst $chain.RhsPipeline $commands)) { + return $false + } + + return $true +} + +function Add-CommandsFromPipelineBase { + param($pipeline, $commands) + + if ($pipeline -is [System.Management.Automation.Language.PipelineAst]) { + return Add-CommandsFromPipelineAst $pipeline $commands + } + + if ($pipeline -is [System.Management.Automation.Language.PipelineChainAst]) { + return Add-CommandsFromPipelineChain $pipeline $commands + } + + return $false +} + +$commands = [System.Collections.ArrayList]::new() + +foreach ($statement in $ast.EndBlock.Statements) { + if (-not (Add-CommandsFromPipelineBase $statement $commands)) { + $commands = $null + break + } +} + +if ($commands -ne $null) { + $normalized = [System.Collections.ArrayList]::new() + foreach ($cmd in $commands) { + if ($cmd -is [string]) { + $null = $normalized.Add(@($cmd)) + continue + } + + if ($cmd -is [System.Array] -or $cmd -is [System.Collections.IEnumerable]) { + $null = $normalized.Add(@($cmd)) + continue + } + + $normalized = $null + break + } + + $commands = $normalized +} + +$result = if ($commands -eq $null) { + @{ status = 'unsupported' } +} else { + @{ status = 'ok'; commands = $commands } +} + +,$result | ConvertTo-Json -Depth 3 diff --git a/codex-rs/core/src/command_safety/windows_safe_commands.rs b/codex-rs/core/src/command_safety/windows_safe_commands.rs index a1d3b297f..ac479a4d2 100644 --- a/codex-rs/core/src/command_safety/windows_safe_commands.rs +++ b/codex-rs/core/src/command_safety/windows_safe_commands.rs @@ -1,30 +1,38 @@ -use shlex::split as shlex_split; +use base64::Engine; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use serde::Deserialize; use std::path::Path; +use std::process::Command; +use std::sync::LazyLock; + +const POWERSHELL_PARSER_SCRIPT: &str = include_str!("powershell_parser.ps1"); /// On Windows, we conservatively allow only clearly read-only PowerShell invocations /// that match a small safelist. Anything else (including direct CMD commands) is unsafe. pub fn is_safe_command_windows(command: &[String]) -> bool { if let Some(commands) = try_parse_powershell_command_sequence(command) { - return commands + commands .iter() - .all(|cmd| is_safe_powershell_command(cmd.as_slice())); + .all(|cmd| is_safe_powershell_command(cmd.as_slice())) + } else { + // Only PowerShell invocations are allowed on Windows for now; anything else is unsafe. + false } - // Only PowerShell invocations are allowed on Windows for now; anything else is unsafe. - false } /// Returns each command sequence if the invocation starts with a PowerShell binary. /// For example, the tokens from `pwsh Get-ChildItem | Measure-Object` become two sequences. fn try_parse_powershell_command_sequence(command: &[String]) -> Option>> { let (exe, rest) = command.split_first()?; - if !is_powershell_executable(exe) { - return None; + if is_powershell_executable(exe) { + parse_powershell_invocation(exe, rest) + } else { + None } - parse_powershell_invocation(rest) } /// Parses a PowerShell invocation into discrete command vectors, rejecting unsafe patterns. -fn parse_powershell_invocation(args: &[String]) -> Option>> { +fn parse_powershell_invocation(executable: &str, args: &[String]) -> Option>> { if args.is_empty() { // Examples rejected here: "pwsh" and "powershell.exe" with no additional arguments. return None; @@ -42,7 +50,7 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { // Examples rejected here: "pwsh -Command foo bar" and "powershell -c ls extra". return None; } - return parse_powershell_script(script); + return parse_powershell_script(executable, script); } _ if lower.starts_with("-command:") || lower.starts_with("/command:") => { if idx + 1 != args.len() { @@ -51,7 +59,7 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { return None; } let script = arg.split_once(':')?.1; - return parse_powershell_script(script); + return parse_powershell_script(executable, script); } // Benign, no-arg flags we tolerate. @@ -77,7 +85,8 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { // This happens if powershell is invoked without -Command, e.g. // ["pwsh", "-NoLogo", "git", "-c", "core.pager=cat", "status"] _ => { - return split_into_commands(args[idx..].to_vec()); + let script = join_arguments_as_script(&args[idx..]); + return parse_powershell_script(executable, &script); } } } @@ -88,46 +97,14 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { /// Tokenizes an inline PowerShell script and delegates to the command splitter. /// Examples of when this is called: pwsh.exe -Command '