From ce2765973d989841c37915f378011ee2b504861e Mon Sep 17 00:00:00 2001 From: Ignat Remizov Date: Sat, 10 Jan 2026 00:35:03 +0200 Subject: [PATCH] fix(execpolicy): add missing config support for auto-allow prefixes PR #7033 added TUI infrastructure for prefix allowance but left the config mechanism unimplemented. Users have been requesting configurable auto-approved commands for over a year (#1260) but had no way to set prefix allowances. This completes the implementation by: - Add execpolicy.auto_allow_prefixes config array for shell-style command prefixes - Parse prefixes with shlex and apply as allow-prefix rules at session start - Support config layer precedence (project config overrides user/global) - Add comprehensive tests for prefix matching, layer precedence, and error handling - Document usage in config.md with spec in execpolicy_auto_allow_prefixes.md Also includes cleanup: refactor ScrollInputMode and ReadMode to use derive Default with #[default] attribute. Example config: ```toml [execpolicy] auto_allow_prefixes = [ "git status", "cargo test", "PGPASSWORD=pass psql -h 127.0.0.1 -p 5432 -U my_user -d local_db -c", ] --- codex-rs/core/src/config/mod.rs | 4 + codex-rs/core/src/config/types.rs | 13 +- codex-rs/core/src/exec_policy.rs | 231 +++++++++++++++++- codex-rs/core/src/shell.rs | 9 +- codex-rs/core/src/tools/handlers/read_file.rs | 8 +- docs/config.md | 16 ++ docs/execpolicy_auto_allow_prefixes.md | 53 ++++ 7 files changed, 317 insertions(+), 17 deletions(-) create mode 100644 docs/execpolicy_auto_allow_prefixes.md diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 7b483f944f2..b8f93596778 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1,5 +1,6 @@ use crate::auth::AuthCredentialsStoreMode; use crate::config::types::DEFAULT_OTEL_ENVIRONMENT; +use crate::config::types::ExecPolicyConfigToml; use crate::config::types::History; use crate::config::types::McpServerConfig; use crate::config::types::Notice; @@ -701,6 +702,9 @@ pub struct ConfigToml { /// Default approval policy for executing commands. pub approval_policy: Option, + /// Optional execpolicy configuration. + pub execpolicy: Option, + #[serde(default)] pub shell_environment_policy: ShellEnvironmentPolicyToml, diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 2b41c3c52fb..2e1b935b93a 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -18,6 +18,11 @@ use serde::de::Error as SerdeError; pub const DEFAULT_OTEL_ENVIRONMENT: &str = "dev"; +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)] +pub struct ExecPolicyConfigToml { + pub auto_allow_prefixes: Option>, +} + #[derive(Serialize, Debug, Clone, PartialEq)] pub struct McpServerConfig { #[serde(flatten)] @@ -389,8 +394,10 @@ impl Default for Notifications { /// infer wheel vs trackpad per stream, or forces a specific behavior. #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq)] #[serde(rename_all = "snake_case")] +#[derive(Default)] pub enum ScrollInputMode { /// Infer wheel vs trackpad behavior per scroll stream. + #[default] Auto, /// Always treat scroll events as mouse-wheel input (fixed lines per tick). Wheel, @@ -398,12 +405,6 @@ pub enum ScrollInputMode { Trackpad, } -impl Default for ScrollInputMode { - fn default() -> Self { - Self::Auto - } -} - /// Collection of settings that are specific to the TUI. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)] pub struct Tui { diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index d8880a9b3c4..18cab1ab9d8 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use arc_swap::ArcSwap; use crate::command_safety::is_dangerous_command::requires_initial_appoval; +use crate::config::types::ExecPolicyConfigToml; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; use codex_execpolicy::AmendError; @@ -28,6 +29,7 @@ use crate::features::Feature; use crate::features::Features; use crate::sandboxing::SandboxPermissions; use crate::tools::sandboxing::ExecApprovalRequirement; +use shlex::split as shlex_split; use shlex::try_join as shlex_try_join; const PROMPT_CONFLICT_REASON: &str = @@ -200,7 +202,9 @@ async fn load_exec_policy_for_features( if !features.enabled(Feature::ExecPolicy) { Ok(Policy::empty()) } else { - load_exec_policy(config_stack).await + let mut policy = load_exec_policy(config_stack).await?; + apply_auto_allow_prefixes(&mut policy, config_stack); + Ok(policy) } } @@ -242,6 +246,57 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result { + tracing::warn!( + prefix = %prefix, + "execpolicy auto-allow prefix resolved to empty argv; skipping" + ); + } + Some(argv) => { + if let Err(err) = policy.add_prefix_rule(&argv, Decision::Allow) { + tracing::warn!( + prefix = %prefix, + error = %err, + "failed to add execpolicy auto-allow prefix" + ); + } + } + None => { + tracing::warn!( + prefix = %prefix, + "execpolicy auto-allow prefix failed to parse; skipping" + ); + } + } + } +} + +fn execpolicy_auto_allow_prefixes(config_stack: &ConfigLayerStack) -> Vec { + let Some(execpolicy) = config_stack.effective_config().get("execpolicy").cloned() else { + return Vec::new(); + }; + + let execpolicy_config: ExecPolicyConfigToml = match execpolicy.try_into() { + Ok(config) => config, + Err(err) => { + tracing::warn!( + error = %err, + "failed to parse execpolicy config; ignoring auto-allow prefixes" + ); + return Vec::new(); + } + }; + + execpolicy_config.auto_allow_prefixes.unwrap_or_default() +} + fn default_policy_path(codex_home: &Path) -> PathBuf { codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE) } @@ -418,6 +473,7 @@ async fn collect_policy_files(dir: impl AsRef) -> Result, Exe #[cfg(test)] mod tests { use super::*; + use crate::config::CONFIG_TOML_FILE; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; @@ -450,6 +506,11 @@ mod tests { .expect("ConfigLayerStack") } + fn config_entry_from_str(source: ConfigLayerSource, toml: &str) -> ConfigLayerEntry { + let config: TomlValue = toml::from_str(toml).expect("parse toml"); + ConfigLayerEntry::new(source, config) + } + #[tokio::test] async fn returns_empty_policy_when_feature_disabled() { let mut features = Features::with_defaults(); @@ -517,6 +578,174 @@ mod tests { ); } + #[tokio::test] + async fn auto_allow_prefixes_allow_exec_commands() { + let temp_dir = tempdir().expect("create temp dir"); + let dot_codex_folder = temp_dir.path().join(".codex"); + let config_stack = ConfigLayerStack::new( + vec![config_entry_from_str( + ConfigLayerSource::Project { + dot_codex_folder: AbsolutePathBuf::from_absolute_path(&dot_codex_folder) + .expect("absolute dot_codex_folder"), + }, + r#" +[execpolicy] +auto_allow_prefixes = ["git status"] +"#, + )], + ConfigRequirements::default(), + ) + .expect("ConfigLayerStack"); + + let manager = ExecPolicyManager::load(&Features::with_defaults(), &config_stack) + .await + .expect("manager"); + let policy = manager.current(); + + let command = vec![ + "git".to_string(), + "status".to_string(), + "--short".to_string(), + ]; + let evaluation = policy.check(&command, &|_| Decision::Prompt); + + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["git".to_string(), "status".to_string()], + decision: Decision::Allow, + justification: None, + }], + } + ); + } + + #[tokio::test] + async fn auto_allow_prefixes_respect_layer_precedence() { + let temp_dir = tempdir().expect("create temp dir"); + let user_file = AbsolutePathBuf::from_absolute_path(temp_dir.path().join(CONFIG_TOML_FILE)) + .expect("absolute user config file"); + let project_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path().join(".codex")) + .expect("absolute dot_codex_folder"); + + let config_stack = ConfigLayerStack::new( + vec![ + config_entry_from_str( + ConfigLayerSource::User { file: user_file }, + r#" +[execpolicy] +auto_allow_prefixes = ["git status"] +"#, + ), + config_entry_from_str( + ConfigLayerSource::Project { + dot_codex_folder: project_folder, + }, + r#" +[execpolicy] +auto_allow_prefixes = ["cargo test"] +"#, + ), + ], + ConfigRequirements::default(), + ) + .expect("ConfigLayerStack"); + + let manager = ExecPolicyManager::load(&Features::with_defaults(), &config_stack) + .await + .expect("manager"); + let policy = manager.current(); + + let git_command = vec![ + "git".to_string(), + "status".to_string(), + "--short".to_string(), + ]; + let git_evaluation = policy.check(&git_command, &|_| Decision::Prompt); + assert_eq!( + git_evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::HeuristicsRuleMatch { + command: git_command, + decision: Decision::Prompt, + }], + } + ); + + let cargo_command = vec!["cargo".to_string(), "test".to_string(), "--all".to_string()]; + let cargo_evaluation = policy.check(&cargo_command, &|_| Decision::Prompt); + assert_eq!( + cargo_evaluation, + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["cargo".to_string(), "test".to_string()], + decision: Decision::Allow, + justification: None, + }], + } + ); + } + + #[tokio::test] + async fn auto_allow_prefixes_ignores_empty_and_invalid_entries() { + let temp_dir = tempdir().expect("create temp dir"); + let dot_codex_folder = temp_dir.path().join(".codex"); + let config_stack = ConfigLayerStack::new( + vec![config_entry_from_str( + ConfigLayerSource::Project { + dot_codex_folder: AbsolutePathBuf::from_absolute_path(&dot_codex_folder) + .expect("absolute dot_codex_folder"), + }, + r#" +[execpolicy] +auto_allow_prefixes = [" ", "git status", "git \"status"] +"#, + )], + ConfigRequirements::default(), + ) + .expect("ConfigLayerStack"); + + let manager = ExecPolicyManager::load(&Features::with_defaults(), &config_stack) + .await + .expect("manager"); + let policy = manager.current(); + + let git_command = vec![ + "git".to_string(), + "status".to_string(), + "--short".to_string(), + ]; + let git_evaluation = policy.check(&git_command, &|_| Decision::Prompt); + assert_eq!( + git_evaluation, + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["git".to_string(), "status".to_string()], + decision: Decision::Allow, + justification: None, + }], + } + ); + + let invalid_command = vec!["git".to_string(), "\"status".to_string()]; + let invalid_evaluation = policy.check(&invalid_command, &|_| Decision::Prompt); + assert_eq!( + invalid_evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::HeuristicsRuleMatch { + command: invalid_command, + decision: Decision::Prompt, + }], + } + ); + } + #[tokio::test] async fn ignores_policies_outside_policy_dir() { let temp_dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index d22b6543a9f..3e84a10eb5c 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -340,6 +340,7 @@ mod detect_shell_type_tests { #[cfg(unix)] mod tests { use super::*; + use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -369,9 +370,9 @@ mod tests { let shell_path = bash_shell.shell_path; assert!( - shell_path == PathBuf::from("/bin/bash") - || shell_path == PathBuf::from("/usr/bin/bash") - || shell_path == PathBuf::from("/usr/local/bin/bash"), + shell_path == Path::new("/bin/bash") + || shell_path == Path::new("/usr/bin/bash") + || shell_path == Path::new("/usr/local/bin/bash"), "shell path: {shell_path:?}", ); } @@ -381,7 +382,7 @@ mod tests { let sh_shell = get_shell(ShellType::Sh, None).unwrap(); let shell_path = sh_shell.shell_path; assert!( - shell_path == PathBuf::from("/bin/sh") || shell_path == PathBuf::from("/usr/bin/sh"), + shell_path == Path::new("/bin/sh") || shell_path == Path::new("/usr/bin/sh"), "shell path: {shell_path:?}", ); } diff --git a/codex-rs/core/src/tools/handlers/read_file.rs b/codex-rs/core/src/tools/handlers/read_file.rs index 4f187540a6e..b95e4a07402 100644 --- a/codex-rs/core/src/tools/handlers/read_file.rs +++ b/codex-rs/core/src/tools/handlers/read_file.rs @@ -42,7 +42,9 @@ struct ReadFileArgs { #[derive(Deserialize)] #[serde(rename_all = "snake_case")] +#[derive(Default)] enum ReadMode { + #[default] Slice, Indentation, } @@ -461,12 +463,6 @@ mod defaults { } } - impl Default for ReadMode { - fn default() -> Self { - Self::Slice - } - } - pub fn offset() -> usize { 1 } diff --git a/docs/config.md b/docs/config.md index 2b64253d309..a0a9e3eef3d 100644 --- a/docs/config.md +++ b/docs/config.md @@ -17,3 +17,19 @@ Codex can connect to MCP servers configured in `~/.codex/config.toml`. See the c Codex can run a notification hook when the agent finishes a turn. See the configuration reference for the latest notification settings: - https://developers.openai.com/codex/config-reference + +## Exec policy auto-allow prefixes + +You can pre-approve exec command prefixes to avoid repeated approvals. Prefixes are tokenized with shell rules and applied to exec approvals only. See the configuration reference for the latest options: + +Example: + +```toml +[execpolicy] +auto_allow_prefixes = [ + "PGPASSWORD=example_password psql -h 127.0.0.1 -p 5432 -U example_user -d example_db -c", + "git status", +] +``` + +Note: this affects exec approvals only; other tools (like apply_patch) are unchanged. diff --git a/docs/execpolicy_auto_allow_prefixes.md b/docs/execpolicy_auto_allow_prefixes.md new file mode 100644 index 00000000000..294f0a23f0d --- /dev/null +++ b/docs/execpolicy_auto_allow_prefixes.md @@ -0,0 +1,53 @@ +STATUS: ready-for-implementation +## Execpolicy Auto-Allow Prefixes + +### Goals +- Allow users to pre-approve exec command prefixes via config so repeated approvals are avoided. +- Support both global config and per-project overrides. +- Ensure prefix matches allow trailing argument changes (prefix match, not exact match). +- Limit scope to exec approvals only (no effect on apply_patch or non-exec tools). + +### Non-Goals +- No changes to existing execpolicy file format or rule parsing. +- No auto-generation of rules for apply_patch or other tool classes. +- No wildcard or regex matching beyond execpolicy prefix semantics. + +### Config Schema +- New config key (available in any config layer): `execpolicy.auto_allow_prefixes = [, ...]` +- Each string is a shell-style prefix that is tokenized using shlex rules into argv. +- The resulting argv prefix is applied as an execpolicy allow-prefix rule. +- Empty/whitespace-only strings are ignored and do not produce rules. +- If tokenization fails for a string, log a warning and skip that entry (do not fail startup). + +### Precedence Rules +- Config layers are merged in precedence order (lowest to highest): system config, user config, project layers (`./config.toml`, `.codex/config.toml` from repo root to cwd), session flags, then legacy managed config. +- `execpolicy.auto_allow_prefixes` is an array; higher-precedence layers replace lower-precedence values (no implicit concatenation). +- Within project layers, the closest config to the cwd wins for the key. + +### Persistence Behavior +- Prefixes are applied to the in-memory execpolicy at session start. +- No execpolicy rule files are written for these prefixes (no persistence beyond the current session). + +### Examples (config.toml) +```toml +[execpolicy] +auto_allow_prefixes = [ + "PGPASSWORD=example_password psql -h 127.0.0.1 -p 5432 -U example_user -d example_db -c", + "git status", +] +``` + +```toml +# .codex/config.toml (project override example) +[execpolicy] +auto_allow_prefixes = [ + "PGPASSWORD=project_password psql -h 127.0.0.1 -p 5432 -U project_user -d project_db -c", +] +``` + +### Acceptance Criteria +- A configured prefix auto-allows exec commands whose argv starts with that prefix, even when trailing args change. +- Exec approvals are skipped for matching prefixes; apply_patch behavior is unchanged. +- Project config layers can override the global `execpolicy.auto_allow_prefixes` value. +- Prefixes are loaded from config at session start without writing to execpolicy rule files. +- Invalid prefix strings do not prevent startup; they are logged and ignored.