mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
agentydragon(tasks): implement granular auto‑approval predicates: parse [[auto_allow]] config, evaluate predicate scripts with short‑circuit voting, integrate into shell flow, update docs and add tests
This commit is contained in:
@@ -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<AutoAllowPredicate>` 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.
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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<String>,
|
||||
approval_policy: AskForApproval,
|
||||
/// External predicate scripts for auto-approval or rejection of shell commands.
|
||||
pub auto_allow: Vec<AutoAllowPredicate>,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
shell_environment_policy: ShellEnvironmentPolicy,
|
||||
writable_roots: Mutex<Vec<PathBuf>>,
|
||||
@@ -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,
|
||||
|
||||
@@ -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<AutoAllowPredicate>,
|
||||
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
|
||||
@@ -238,6 +247,10 @@ pub struct ConfigToml {
|
||||
/// Default approval policy for executing commands.
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
|
||||
/// Auto-approval predicate scripts that cast votes on each shell command.
|
||||
#[serde(default)]
|
||||
pub auto_allow: Vec<AutoAllowPredicate>,
|
||||
|
||||
#[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
|
||||
|
||||
@@ -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<String>,
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
pub disable_response_storage: Option<bool>,
|
||||
/// External predicate scripts for auto-approval or rejection of shell commands.
|
||||
pub auto_allow: Option<Vec<AutoAllowPredicate>>,
|
||||
}
|
||||
|
||||
@@ -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<SandboxType> {
|
||||
}
|
||||
}
|
||||
|
||||
/// 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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user