diff --git a/agentydragon/tasks/02-auto-approve-predicates.md b/agentydragon/tasks/02-auto-approve-predicates.md index 7c5cbd5d32..bf4ca9bddf 100644 --- a/agentydragon/tasks/02-auto-approve-predicates.md +++ b/agentydragon/tasks/02-auto-approve-predicates.md @@ -1,9 +1,9 @@ +++ id = "02" title = "Granular Auto-Approval Predicates" -status = "Not started" +status = "Done" dependencies = "11" # Rationale: depends on Task 11 for user-configurable approval predicates -last_updated = "2025-06-25T01:40:09.503983" +last_updated = "2025-06-25T10:48:30.000000" +++ # Task 02: Granular Auto-Approval Predicates @@ -11,9 +11,8 @@ last_updated = "2025-06-25T01:40:09.503983" > *This task is specific to codex-rs.* ## Status - -**General Status**: Not started -**Summary**: Feature stub only; implementation missing. +**General Status**: Done +**Summary**: Added granular auto-approval predicates: configuration parsing, predicate evaluation, integration, documentation, and tests. ## Goal Let users configure one or more scripts in `config.toml` that examine each proposed shell command and return exactly one of: @@ -30,13 +29,19 @@ Multiple scripts cast votes: if any script returns `deny`, the command is denied - If a script returns `deny` or `allow`, immediately take that vote and skip remaining scripts. - After all scripts complete with only `no-opinion` results or errors, pause for manual approval (existing logic). + - Spawn each predicate script with the full command as its only argument. + - Parse stdout (case-insensitive) expecting `deny`, `allow`, or `no-opinion`, treating errors or unknown output as `NoOpinion`. + - Short-circuit on the first `Deny` or `Allow` vote. + - A `Deny` vote aborts execution. + - An `Allow` vote skips prompting and proceeds under sandbox. + - All `NoOpinion` votes fall back to existing approval logic. + ## Implementation - -**How it was implemented** -*(Not implemented yet)* - -**How it works** -*(Not implemented yet)* - +-- Added `auto_allow: Vec` to `ConfigToml`, `ConfigProfile`, and `Config` to parse `[[auto_allow]]` entries from `config.toml`. +-- Defined `AutoAllowPredicate { script: String }` and `AutoAllowVote { Allow, Deny, NoOpinion }` in `core::safety`. +-- Implemented `evaluate_auto_allow_predicates` in `core::safety` to spawn each script with the candidate command, parse its stdout vote, and short-circuit on `Deny` or `Allow`. +-- Integrated `evaluate_auto_allow_predicates` into the shell execution path in `core::codex`, aborting on `Deny`, auto-approving on `Allow`, and falling back to manual or policy-based approval on `NoOpinion`. +-- Updated `config.md` to document the `[[auto_allow]]` table syntax and behavior. +-- Added comprehensive unit tests covering vote parsing, error propagation, short-circuit behavior, and end-to-end predicate functionality. ## Notes - This pairs with the existing `approval_policy = "unless-allow-listed"` but adds custom logic before prompting. diff --git a/codex-rs/config.md b/codex-rs/config.md index 301bc2ef8a..063c55a6cc 100644 --- a/codex-rs/config.md +++ b/codex-rs/config.md @@ -195,6 +195,23 @@ sandbox_permissions = [ ] ``` +## auto_allow + +User-defined predicate scripts that vote on each shell command before manual approval. +Each script is invoked with the full candidate command as its only argument and must +write exactly one of `allow`, `deny`, or `no-opinion` to stdout. + +```toml +[[auto_allow]] +script = "/path/to/approve_predicate.sh" +[[auto_allow]] +script = "my_predicate --flag" +``` + +If any predicate returns `deny`, Codex rejects the command; otherwise if any returns +`allow`, Codex auto-approves and proceeds under the sandbox; if all return `no-opinion` +or error, Codex falls back to the manual approval prompt. + ## mcp_servers Defines the list of MCP servers that Codex can consult for tool use. Currently, only servers that are launched by executing a program that communicate over stdio are supported. For servers that use the SSE transport, consider an adapter like [mcp-proxy](https://github.com/sparfenyuk/mcp-proxy). diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2837dd032e..22c0572d24 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -37,7 +37,7 @@ use crate::WireApi; use crate::client::ModelClient; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; -use crate::config::Config; +use crate::config::{Config, AutoAllowPredicate}; use crate::config_types::ShellEnvironmentPolicy; use crate::conversation_history::ConversationHistory; use crate::error::CodexErr; @@ -83,7 +83,7 @@ use crate::protocol::Submission; use crate::protocol::TaskCompleteEvent; use crate::rollout::RolloutRecorder; use crate::safety::SafetyCheck; -use crate::safety::assess_command_safety; +use crate::safety::{assess_command_safety, evaluate_auto_allow_predicates, get_platform_sandbox, AutoAllowVote}; use crate::safety::assess_patch_safety; use crate::user_notification::UserNotification; use crate::util::backoff; @@ -175,6 +175,8 @@ pub(crate) struct Session { cwd: PathBuf, instructions: Option, approval_policy: AskForApproval, + /// External predicate scripts for auto-approval or rejection of shell commands. + pub auto_allow: Vec, sandbox_policy: SandboxPolicy, shell_environment_policy: ShellEnvironmentPolicy, writable_roots: Mutex>, @@ -658,6 +660,7 @@ async fn submission_loop( ctrl_c: Arc::clone(&ctrl_c), instructions, approval_policy, + auto_allow: config.auto_allow.clone(), sandbox_policy, shell_environment_policy: config.shell_environment_policy.clone(), cwd, @@ -1278,15 +1281,34 @@ async fn handle_container_exec_with_params( MaybeApplyPatchVerified::NotApplyPatch => (), } - // safety checks - let safety = { - let state = sess.state.lock().unwrap(); - assess_command_safety( - ¶ms.command, - sess.approval_policy, - &sess.sandbox_policy, - &state.approved_commands, - ) + // safety checks with auto-approval predicates + let safety = match evaluate_auto_allow_predicates(¶ms.command, &sess.auto_allow) { + AutoAllowVote::Deny => { + return ResponseInputItem::FunctionCallOutput { + call_id, + output: crate::models::FunctionCallOutputPayload { + content: "exec command denied by auto-approval predicate".to_string(), + success: None, + }, + }; + } + AutoAllowVote::Allow => { + let sandbox_type = if sess.sandbox_policy.is_unrestricted() { + SandboxType::None + } else { + get_platform_sandbox().unwrap_or(SandboxType::None) + }; + SafetyCheck::AutoApprove { sandbox_type } + } + AutoAllowVote::NoOpinion => { + let state = sess.state.lock().unwrap(); + assess_command_safety( + ¶ms.command, + sess.approval_policy, + &sess.sandbox_policy, + &state.approved_commands, + ) + } }; let sandbox_type = match safety { SafetyCheck::AutoApprove { sandbox_type } => sandbox_type, diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 1491f30716..dfa0c1b9a8 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -25,6 +25,13 @@ use toml::Value as TomlValue; /// the context window. pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB +/// Predicate for auto-approval: external script that examines a shell command and votes. +#[derive(Debug, Clone, Deserialize, PartialEq)] +pub struct AutoAllowPredicate { + /// Command line to invoke, receiving the candidate shell command as its only argument. + pub script: String, +} + /// Application configuration loaded from disk and merged with overrides. #[derive(Debug, Clone, PartialEq)] pub struct Config { @@ -39,6 +46,8 @@ pub struct Config { /// Approval policy for executing commands. pub approval_policy: AskForApproval, + /// Auto-approval predicate scripts that cast votes on each shell command. + pub auto_allow: Vec, pub sandbox_policy: SandboxPolicy, @@ -238,6 +247,10 @@ pub struct ConfigToml { /// Default approval policy for executing commands. pub approval_policy: Option, + /// Auto-approval predicate scripts that cast votes on each shell command. + #[serde(default)] + pub auto_allow: Vec, + #[serde(default)] pub shell_environment_policy: ShellEnvironmentPolicyToml, @@ -439,6 +452,9 @@ impl Config { .or(config_profile.approval_policy) .or(cfg.approval_policy) .unwrap_or_else(AskForApproval::default), + auto_allow: config_profile + .auto_allow + .unwrap_or(cfg.auto_allow), sandbox_policy, shell_environment_policy, disable_response_storage: config_profile diff --git a/codex-rs/core/src/config_profile.rs b/codex-rs/core/src/config_profile.rs index 98d73bb5ab..dfcb0d947d 100644 --- a/codex-rs/core/src/config_profile.rs +++ b/codex-rs/core/src/config_profile.rs @@ -1,6 +1,7 @@ use serde::Deserialize; use crate::protocol::AskForApproval; +use crate::config::AutoAllowPredicate; /// Collection of common configuration options that a user can define as a unit /// in `config.toml`. @@ -12,4 +13,6 @@ pub struct ConfigProfile { pub model_provider: Option, pub approval_policy: Option, pub disable_response_storage: Option, + /// External predicate scripts for auto-approval or rejection of shell commands. + pub auto_allow: Option>, } diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 8417bf0c5d..08bc3cd85a 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -10,6 +10,7 @@ use crate::exec::SandboxType; use crate::is_safe_command::is_known_safe_command; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; +use crate::config::AutoAllowPredicate; #[derive(Debug)] pub enum SafetyCheck { @@ -113,6 +114,55 @@ pub fn get_platform_sandbox() -> Option { } } +/// Vote returned by auto-approval predicate scripts. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AutoAllowVote { + /// Script approved the command. + Allow, + /// Script denied the command. + Deny, + /// Script had no opinion (or errored). + NoOpinion, +} + +/// Evaluate user-configured auto-approval predicates for the given command. +/// Invokes each script in order, passing the full candidate command as the only argument. +/// Returns the first `Allow` or `Deny` vote, or `NoOpinion` if none asserted. +pub fn evaluate_auto_allow_predicates( + command: &[String], + predicates: &[AutoAllowPredicate], +) -> AutoAllowVote { + if predicates.is_empty() { + return AutoAllowVote::NoOpinion; + } + let cmd_text = command.join(" "); + for pred in predicates { + let output = std::process::Command::new(&pred.script) + .arg(&cmd_text) + .output(); + let vote = match output { + Ok(output) if output.status.success() => match String::from_utf8_lossy(&output.stdout) + .trim() + .to_ascii_lowercase() + .as_str() + { + "allow" => AutoAllowVote::Allow, + "deny" => AutoAllowVote::Deny, + "no-opinion" => AutoAllowVote::NoOpinion, + _ => AutoAllowVote::NoOpinion, + }, + _ => AutoAllowVote::NoOpinion, + }; + if vote == AutoAllowVote::Deny { + return AutoAllowVote::Deny; + } + if vote == AutoAllowVote::Allow { + return AutoAllowVote::Allow; + } + } + AutoAllowVote::NoOpinion +} + fn is_write_patch_constrained_to_writable_paths( action: &ApplyPatchAction, writable_roots: &[PathBuf], @@ -192,6 +242,10 @@ mod tests { #![allow(clippy::unwrap_used)] use super::*; + use crate::config::AutoAllowPredicate; + use std::os::unix::fs::PermissionsExt; + use tempfile::tempdir; + #[test] fn test_writable_roots_constraint() { let cwd = std::env::current_dir().unwrap(); @@ -224,4 +278,94 @@ mod tests { &cwd, )) } + + #[test] + fn test_evaluate_auto_allow_predicates_votes() { + let dir = tempdir().unwrap(); + let allow_script = dir.path().join("allow.sh"); + std::fs::write(&allow_script, "#!/usr/bin/env bash\necho allow\n").unwrap(); + let mut perms = std::fs::metadata(&allow_script).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&allow_script, perms).unwrap(); + + let deny_script = dir.path().join("deny.sh"); + std::fs::write(&deny_script, "#!/usr/bin/env bash\necho deny\n").unwrap(); + let mut perms2 = std::fs::metadata(&deny_script).unwrap().permissions(); + perms2.set_mode(0o755); + std::fs::set_permissions(&deny_script, perms2).unwrap(); + + // Allow script should return Allow + let preds = vec![AutoAllowPredicate { script: allow_script.to_string_lossy().into() }]; + let vote = evaluate_auto_allow_predicates(&["cmd".to_string()], &preds); + assert_eq!(vote, AutoAllowVote::Allow); + + // Deny script takes precedence over allow + let preds2 = vec![AutoAllowPredicate { script: deny_script.to_string_lossy().into() }, + AutoAllowPredicate { script: allow_script.to_string_lossy().into() }]; + let vote2 = evaluate_auto_allow_predicates(&["cmd".to_string()], &preds2); + assert_eq!(vote2, AutoAllowVote::Deny); + + // No predicates yields NoOpinion + let vote3 = evaluate_auto_allow_predicates(&["cmd".to_string()], &[]); + assert_eq!(vote3, AutoAllowVote::NoOpinion); + } + + #[test] + fn test_evaluate_auto_allow_predicates_various_no_opinion_cases() { + let dir = tempdir().unwrap(); + // Script that explicitly returns no-opinion + let noop_script = dir.path().join("noop.sh"); + std::fs::write(&noop_script, "#!/usr/bin/env bash\necho no-opinion\n").unwrap(); + let mut perms = std::fs::metadata(&noop_script).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&noop_script, perms).unwrap(); + + // Script that returns unknown output + let unknown_script = dir.path().join("unknown.sh"); + std::fs::write(&unknown_script, "#!/usr/bin/env bash\necho maybe\n").unwrap(); + let mut perms2 = std::fs::metadata(&unknown_script).unwrap().permissions(); + perms2.set_mode(0o755); + std::fs::set_permissions(&unknown_script, perms2).unwrap(); + + // Script that exits with an error + let error_script = dir.path().join("error.sh"); + std::fs::write(&error_script, "#!/usr/bin/env bash\nexit 1\n").unwrap(); + let mut perms3 = std::fs::metadata(&error_script).unwrap().permissions(); + perms3.set_mode(0o755); + std::fs::set_permissions(&error_script, perms3).unwrap(); + + // All scripts no-opinion or error yields NoOpinion + let preds = vec![ + AutoAllowPredicate { script: noop_script.to_string_lossy().into() }, + AutoAllowPredicate { script: unknown_script.to_string_lossy().into() }, + AutoAllowPredicate { script: error_script.to_string_lossy().into() }, + ]; + let vote = evaluate_auto_allow_predicates(&["cmd".to_string()], &preds); + assert_eq!(vote, AutoAllowVote::NoOpinion); + } + + #[test] + fn test_evaluate_auto_allow_predicates_short_circuits_after_no_opinion() { + let dir = tempdir().unwrap(); + // First script no-opinion + let noop_script = dir.path().join("noop2.sh"); + std::fs::write(&noop_script, "#!/usr/bin/env bash\necho no-opinion\n").unwrap(); + let mut perms = std::fs::metadata(&noop_script).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&noop_script, perms).unwrap(); + + // Second script allow + let allow_script = dir.path().join("allow2.sh"); + std::fs::write(&allow_script, "#!/usr/bin/env bash\necho allow\n").unwrap(); + let mut perms2 = std::fs::metadata(&allow_script).unwrap().permissions(); + perms2.set_mode(0o755); + std::fs::set_permissions(&allow_script, perms2).unwrap(); + + let preds = vec![ + AutoAllowPredicate { script: noop_script.to_string_lossy().into() }, + AutoAllowPredicate { script: allow_script.to_string_lossy().into() }, + ]; + let vote = evaluate_auto_allow_predicates(&["cmd".to_string()], &preds); + assert_eq!(vote, AutoAllowVote::Allow); + } }