diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cf4c30f18..5e039b13a 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -240,7 +240,7 @@ impl Codex { ) .await; - let exec_policy = ExecPolicyManager::load(&config.features, &config.codex_home) + let exec_policy = ExecPolicyManager::load(&config.features, &config.config_layer_stack) .await .map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy: {err}")))?; diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 7d98db708..be336bd05 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -28,6 +28,7 @@ pub use config_requirements::ConfigRequirements; pub use merge::merge_toml_values; pub use state::ConfigLayerEntry; pub use state::ConfigLayerStack; +pub use state::ConfigLayerStackOrdering; pub use state::LoaderOverrides; /// On Unix systems, load requirements from this file path, if present. diff --git a/codex-rs/core/src/config_loader/state.rs b/codex-rs/core/src/config_loader/state.rs index 5afa30080..bd00e7724 100644 --- a/codex-rs/core/src/config_loader/state.rs +++ b/codex-rs/core/src/config_loader/state.rs @@ -50,6 +50,25 @@ impl ConfigLayerEntry { config: serde_json::to_value(&self.config).unwrap_or(JsonValue::Null), } } + + // Get the `.codex/` folder associated with this config layer, if any. + pub fn config_folder(&self) -> Option { + match &self.name { + ConfigLayerSource::Mdm { .. } => None, + ConfigLayerSource::System { .. } => None, + ConfigLayerSource::User { file } => file.parent(), + ConfigLayerSource::Project { dot_codex_folder } => Some(dot_codex_folder.clone()), + ConfigLayerSource::SessionFlags => None, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => None, + ConfigLayerSource::LegacyManagedConfigTomlFromMdm => None, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConfigLayerStackOrdering { + LowestPrecedenceFirst, + HighestPrecedenceFirst, } #[derive(Debug, Clone, Default, PartialEq)] @@ -156,7 +175,16 @@ impl ConfigLayerStack { /// Returns the highest-precedence to lowest-precedence layers, so /// `ConfigLayerSource::SessionFlags` would be first, if present. pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> { - self.layers.iter().rev().collect() + self.get_layers(ConfigLayerStackOrdering::HighestPrecedenceFirst) + } + + /// Returns the highest-precedence to lowest-precedence layers, so + /// `ConfigLayerSource::SessionFlags` would be first, if present. + pub fn get_layers(&self, ordering: ConfigLayerStackOrdering) -> Vec<&ConfigLayerEntry> { + match ordering { + ConfigLayerStackOrdering::HighestPrecedenceFirst => self.layers.iter().rev().collect(), + ConfigLayerStackOrdering::LowestPrecedenceFirst => self.layers.iter().collect(), + } } } diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 6bbde29d3..5caf7d9b8 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -6,6 +6,8 @@ use std::sync::Arc; use arc_swap::ArcSwap; use crate::command_safety::is_dangerous_command::requires_initial_appoval; +use crate::config_loader::ConfigLayerStack; +use crate::config_loader::ConfigLayerStackOrdering; use codex_execpolicy::AmendError; use codex_execpolicy::Decision; use codex_execpolicy::Error as ExecPolicyRuleError; @@ -94,9 +96,9 @@ impl ExecPolicyManager { pub(crate) async fn load( features: &Features, - codex_home: &Path, + config_stack: &ConfigLayerStack, ) -> Result { - let policy = load_exec_policy_for_features(features, codex_home).await?; + let policy = load_exec_policy_for_features(features, config_stack).await?; Ok(Self::new(Arc::new(policy))) } @@ -194,18 +196,28 @@ impl Default for ExecPolicyManager { async fn load_exec_policy_for_features( features: &Features, - codex_home: &Path, + config_stack: &ConfigLayerStack, ) -> Result { if !features.enabled(Feature::ExecPolicy) { Ok(Policy::empty()) } else { - load_exec_policy(codex_home).await + load_exec_policy(config_stack).await } } -pub async fn load_exec_policy(codex_home: &Path) -> Result { - let policy_dir = codex_home.join(RULES_DIR_NAME); - let policy_paths = collect_policy_files(&policy_dir).await?; +pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result { + // Iterate the layers in increasing order of precedence, adding the *.rules + // from each layer, so that higher-precedence layers can override + // rules defined in lower-precedence ones. + let mut policy_paths = Vec::new(); + for layer in config_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst) { + if let Some(config_folder) = layer.config_folder() { + #[expect(clippy::expect_used)] + let policy_dir = config_folder.join(RULES_DIR_NAME).expect("safe join"); + let layer_policy_paths = collect_policy_files(&policy_dir).await?; + policy_paths.extend(layer_policy_paths); + } + } let mut parser = PolicyParser::new(); for policy_path in &policy_paths { @@ -226,11 +238,7 @@ pub async fn load_exec_policy(codex_home: &Path) -> Result Option { }) } -async fn collect_policy_files(dir: &Path) -> Result, ExecPolicyError> { +async fn collect_policy_files(dir: impl AsRef) -> Result, ExecPolicyError> { + let dir = dir.as_ref(); let mut read_dir = match fs::read_dir(dir).await { Ok(read_dir) => read_dir, Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Vec::new()), @@ -345,28 +354,51 @@ async fn collect_policy_files(dir: &Path) -> Result, ExecPolicyErro policy_paths.sort(); + tracing::debug!( + "loaded {} .rules files in {}", + policy_paths.len(), + dir.display() + ); Ok(policy_paths) } #[cfg(test)] mod tests { use super::*; + use crate::config_loader::ConfigLayerEntry; + use crate::config_loader::ConfigLayerStack; + use crate::config_loader::ConfigRequirements; use crate::features::Feature; use crate::features::Features; + use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; + use std::path::Path; use std::sync::Arc; use tempfile::tempdir; + use toml::Value as TomlValue; + + fn config_stack_for_dot_codex_folder(dot_codex_folder: &Path) -> ConfigLayerStack { + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(dot_codex_folder) + .expect("absolute dot_codex_folder"); + let layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + TomlValue::Table(Default::default()), + ); + ConfigLayerStack::new(vec![layer], ConfigRequirements::default()).expect("ConfigLayerStack") + } #[tokio::test] async fn returns_empty_policy_when_feature_disabled() { let mut features = Features::with_defaults(); features.disable(Feature::ExecPolicy); let temp_dir = tempdir().expect("create temp dir"); + let config_stack = config_stack_for_dot_codex_folder(temp_dir.path()); - let manager = ExecPolicyManager::load(&features, temp_dir.path()) + let manager = ExecPolicyManager::load(&features, &config_stack) .await .expect("manager result"); let policy = manager.current(); @@ -400,6 +432,7 @@ mod tests { #[tokio::test] async fn loads_policies_from_policy_subdirectory() { let temp_dir = tempdir().expect("create temp dir"); + let config_stack = config_stack_for_dot_codex_folder(temp_dir.path()); let policy_dir = temp_dir.path().join(RULES_DIR_NAME); fs::create_dir_all(&policy_dir).expect("create policy dir"); fs::write( @@ -408,7 +441,7 @@ mod tests { ) .expect("write policy file"); - let policy = load_exec_policy(temp_dir.path()) + let policy = load_exec_policy(&config_stack) .await .expect("policy result"); let command = [vec!["rm".to_string()]]; @@ -427,13 +460,14 @@ mod tests { #[tokio::test] async fn ignores_policies_outside_policy_dir() { let temp_dir = tempdir().expect("create temp dir"); + let config_stack = config_stack_for_dot_codex_folder(temp_dir.path()); fs::write( temp_dir.path().join("root.rules"), r#"prefix_rule(pattern=["ls"], decision="prompt")"#, ) .expect("write policy file"); - let policy = load_exec_policy(temp_dir.path()) + let policy = load_exec_policy(&config_stack) .await .expect("policy result"); let command = [vec!["ls".to_string()]]; @@ -449,6 +483,69 @@ mod tests { ); } + #[tokio::test] + async fn loads_policies_from_multiple_config_layers() -> anyhow::Result<()> { + let user_dir = tempdir()?; + let project_dir = tempdir()?; + + let user_policy_dir = user_dir.path().join(RULES_DIR_NAME); + fs::create_dir_all(&user_policy_dir)?; + fs::write( + user_policy_dir.join("user.rules"), + r#"prefix_rule(pattern=["rm"], decision="forbidden")"#, + )?; + + let project_policy_dir = project_dir.path().join(RULES_DIR_NAME); + fs::create_dir_all(&project_policy_dir)?; + fs::write( + project_policy_dir.join("project.rules"), + r#"prefix_rule(pattern=["ls"], decision="prompt")"#, + )?; + + let user_config_toml = + AbsolutePathBuf::from_absolute_path(user_dir.path().join("config.toml"))?; + let project_dot_codex_folder = AbsolutePathBuf::from_absolute_path(project_dir.path())?; + let layers = vec![ + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: user_config_toml, + }, + TomlValue::Table(Default::default()), + ), + ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: project_dot_codex_folder, + }, + TomlValue::Table(Default::default()), + ), + ]; + let config_stack = ConfigLayerStack::new(layers, ConfigRequirements::default())?; + + let policy = load_exec_policy(&config_stack).await?; + + assert_eq!( + Evaluation { + decision: Decision::Forbidden, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["rm".to_string()], + decision: Decision::Forbidden + }], + }, + policy.check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow) + ); + assert_eq!( + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["ls".to_string()], + decision: Decision::Prompt + }], + }, + policy.check_multiple([vec!["ls".to_string()]].iter(), &|_| Decision::Allow) + ); + Ok(()) + } + #[tokio::test] async fn evaluates_bash_lc_inner_commands() { let policy_src = r#" diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index ba481264e..12d0055cd 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -230,7 +230,21 @@ fn format_program_name(path: &Path, preserve_program_paths: bool) -> Option anyhow::Result { let codex_home = find_codex_home().context("failed to resolve codex_home for execpolicy")?; - codex_core::load_exec_policy(&codex_home) + + // TODO(mbolin): At a minimum, `cwd` should be configurable via + // `codex/sandbox-state/update` or some other custom MCP call. + let cwd = None; + let cli_overrides = Vec::new(); + let overrides = codex_core::config_loader::LoaderOverrides::default(); + let config_layer_stack = codex_core::config_loader::load_config_layers_state( + &codex_home, + cwd, + &cli_overrides, + overrides, + ) + .await?; + + codex_core::load_exec_policy(&config_layer_stack) .await .map_err(anyhow::Error::from) } diff --git a/codex-rs/utils/absolute-path/src/lib.rs b/codex-rs/utils/absolute-path/src/lib.rs index 808e03183..2eb7d4875 100644 --- a/codex-rs/utils/absolute-path/src/lib.rs +++ b/codex-rs/utils/absolute-path/src/lib.rs @@ -43,6 +43,13 @@ impl AbsolutePathBuf { Self::resolve_path_against_base(path, &self.0) } + pub fn parent(&self) -> Option { + self.0.parent().map(|p| { + #[expect(clippy::expect_used)] + Self::from_absolute_path(p).expect("parent of AbsolutePathBuf must be absolute") + }) + } + pub fn as_path(&self) -> &Path { &self.0 }