clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 14:45:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

precompute approval_requirement

Browse Source
This commit is contained in:
kevin zhao
2025-11-17 16:16:44 -08:00
parent f18fdc97b3
commit 3990d90e10
7 changed files with 131 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
codex-rs/core/src/exec_policy.rs
View File

@@ -3,11 +3,13 @@ use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
use codex_execpolicy2::Decision;
use codex_execpolicy2::Evaluation;
use codex_execpolicy2::Policy;
use codex_execpolicy2::PolicyParser;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use thiserror::Error;
use crate::bash::parse_shell_lc_plain_commands;
@@ -133,12 +135,38 @@ pub(crate) fn evaluate_with_policy(
}
}
pub(crate) fn approval_requirement_for_command(
policy: Option<&Policy>,
command: &[String],
approval_policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
with_escalated_permissions: bool,
) -> ApprovalRequirement {
if let Some(policy) = policy
&& let Some(requirement) = evaluate_with_policy(policy, command, approval_policy)
{
return requirement;
}
if requires_initial_appoval(
approval_policy,
sandbox_policy,
command,
with_escalated_permissions,
) {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::features::Feature;
use crate::features::Features;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
@@ -192,4 +220,48 @@ prefix_rule(pattern=["rm"], decision="forbidden")
}
);
}
#[test]
fn approval_requirement_prefers_execpolicy_match() {
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
let mut parser = PolicyParser::new();
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = parser.build();
let command = vec!["rm".to_string()];
let requirement = approval_requirement_for_command(
Some(&policy),
&command,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
false,
);
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval {
reason: Some(PROMPT_REASON.to_string())
}
);
}
#[test]
fn approval_requirement_falls_back_to_heuristics() {
let command = vec!["ls".to_string()];
let requirement = approval_requirement_for_command(
None,
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
false,
);
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval { reason: None }
);
}
}

14
codex-rs/core/src/tools/handlers/shell.rs
View File

@@ -9,6 +9,7 @@ use crate::apply_patch::convert_apply_patch_to_protocol;
use crate::codex::TurnContext;
use crate::exec::ExecParams;
use crate::exec_env::create_env;
use crate::exec_policy::approval_requirement_for_command;
use crate::function_tool::FunctionCallError;
use crate::is_safe_command::is_known_safe_command;
use crate::protocol::ExecCommandSource;
@@ -24,6 +25,7 @@ use crate::tools::runtimes::apply_patch::ApplyPatchRequest;
use crate::tools::runtimes::apply_patch::ApplyPatchRuntime;
use crate::tools::runtimes::shell::ShellRequest;
use crate::tools::runtimes::shell::ShellRuntime;
use crate::tools::sandboxing::ApprovalRequirement;
use crate::tools::sandboxing::ToolCtx;
pub struct ShellHandler;
@@ -294,10 +296,16 @@ impl ShellHandler {
env: exec_params.env.clone(),
with_escalated_permissions: exec_params.with_escalated_permissions,
justification: exec_params.justification.clone(),
exec_policy: if is_user_shell_command {
None
approval_requirement: if is_user_shell_command {
ApprovalRequirement::Skip
} else {
turn.exec_policy_v2.clone()
approval_requirement_for_command(
turn.exec_policy_v2.as_deref(),
&exec_params.command,
turn.approval_policy,
&turn.sandbox_policy,
exec_params.with_escalated_permissions.unwrap_or(false),
)
},
};
let mut orchestrator = ToolOrchestrator::new();

6
codex-rs/core/src/tools/orchestrator.rs
View File

@@ -17,6 +17,7 @@ use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::default_approval_requirement;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
@@ -52,7 +53,10 @@ impl ToolOrchestrator {
// 1) Approval
let mut already_approved = false;
match tool.approval_requirement(req, approval_policy, &turn_ctx.sandbox_policy) {
let requirement = tool.approval_requirement(req).unwrap_or_else(|| {
default_approval_requirement(approval_policy, &turn_ctx.sandbox_policy)
});
match requirement {
ApprovalRequirement::Skip => {
otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg);
}

112
codex-rs/core/src/tools/runtimes/shell.rs
View File

@@ -4,10 +4,7 @@ Runtime: shell
Executes shell requests under the orchestrator: asks for approval when needed,
builds a CommandSpec, and runs it under the current SandboxAttempt.
*/
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
use crate::exec::ExecToolCallOutput;
use crate::exec_policy::evaluate_with_policy;
use crate::protocol::SandboxPolicy;
use crate::sandboxing::execute_env;
use crate::tools::runtimes::build_command_spec;
use crate::tools::sandboxing::Approvable;
@@ -22,12 +19,9 @@ use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::with_cached_approval;
use codex_execpolicy2::Policy as ExecPolicyV2;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use futures::future::BoxFuture;
use std::path::PathBuf;
use std::sync::Arc;
#[derive(Clone, Debug)]
pub struct ShellRequest {
@@ -37,7 +31,7 @@ pub struct ShellRequest {
pub env: std::collections::HashMap<String, String>,
pub with_escalated_permissions: Option<bool>,
pub justification: Option<String>,
pub exec_policy: Option<Arc<ExecPolicyV2>>,
pub approval_requirement: ApprovalRequirement,
}
impl ProvidesSandboxRetryData for ShellRequest {
@@ -119,26 +113,8 @@ impl Approvable<ShellRequest> for ShellRuntime {
})
}
fn approval_requirement(
&self,
req: &ShellRequest,
policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
) -> ApprovalRequirement {
if let Some(exec_policy) = &req.exec_policy
&& let Some(requirement) = evaluate_with_policy(exec_policy, &req.command, policy)
{
requirement
} else if requires_initial_appoval(
policy,
sandbox_policy,
&req.command,
req.with_escalated_permissions.unwrap_or(false),
) {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
fn approval_requirement(&self, req: &ShellRequest) -> Option<ApprovalRequirement> {
Some(req.approval_requirement.clone())
}
fn wants_escalated_first_attempt(&self, req: &ShellRequest) -> bool {
@@ -170,85 +146,3 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
Ok(out)
}
}
#[cfg(test)]
mod tests {
use super::*;
use codex_execpolicy2::PolicyParser;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
fn parse_policy(src: &str) -> Arc<ExecPolicyV2> {
let mut parser = PolicyParser::new();
parser
.parse("test.codexpolicy", src)
.expect("parse execpolicy2 file");
Arc::new(parser.build())
}
fn shell_request(command: &[&str], exec_policy: Option<Arc<ExecPolicyV2>>) -> ShellRequest {
ShellRequest {
command: command.iter().map(ToString::to_string).collect(),
cwd: PathBuf::from("."),
timeout_ms: None,
env: HashMap::new(),
with_escalated_permissions: None,
justification: None,
exec_policy,
}
}
#[test]
fn prompt_decision_requires_approval() {
let policy = parse_policy(r#"prefix_rule(pattern=["echo"], decision="prompt")"#);
let req = shell_request(&["echo", "hi"], Some(policy));
let runtime = ShellRuntime::new();
let requirement = runtime.approval_requirement(
&req,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
);
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval {
reason: Some("execpolicy requires approval for this command".to_string())
}
);
}
#[test]
fn prompt_blocked_when_approval_disabled() {
let policy = parse_policy(r#"prefix_rule(pattern=["echo"], decision="prompt")"#);
let req = shell_request(&["echo", "hi"], Some(policy));
let runtime = ShellRuntime::new();
let requirement = runtime.approval_requirement(
&req,
AskForApproval::Never,
&SandboxPolicy::DangerFullAccess,
);
assert_eq!(
requirement,
ApprovalRequirement::Forbidden {
reason: "execpolicy requires approval for this command".to_string()
}
);
}
#[test]
fn user_shell_commands_skip_execpolicy() {
let req = shell_request(&["echo", "hi"], None);
let runtime = ShellRuntime::new();
let requirement = runtime.approval_requirement(
&req,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
);
assert_eq!(requirement, ApprovalRequirement::Skip);
}
}

34
codex-rs/core/src/tools/runtimes/unified_exec.rs
View File

@@ -1,5 +1,3 @@
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
use crate::exec_policy::evaluate_with_policy;
/*
Runtime: unified exec
@@ -24,23 +22,19 @@ use crate::tools::sandboxing::with_cached_approval;
use crate::unified_exec::UnifiedExecError;
use crate::unified_exec::UnifiedExecSession;
use crate::unified_exec::UnifiedExecSessionManager;
use codex_execpolicy2::Policy as ExecPolicyV2;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use futures::future::BoxFuture;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
#[derive(Clone, Debug)]
pub struct UnifiedExecRequest {
pub command: Vec<String>,
pub cwd: PathBuf,
pub env: HashMap<String, String>,
pub exec_policy: Option<Arc<ExecPolicyV2>>,
pub with_escalated_permissions: Option<bool>,
pub justification: Option<String>,
pub approval_requirement: ApprovalRequirement,
}
impl ProvidesSandboxRetryData for UnifiedExecRequest {
@@ -68,17 +62,17 @@ impl UnifiedExecRequest {
command: Vec<String>,
cwd: PathBuf,
env: HashMap<String, String>,
exec_policy: Option<Arc<ExecPolicyV2>>,
with_escalated_permissions: Option<bool>,
justification: Option<String>,
approval_requirement: ApprovalRequirement,
) -> Self {
Self {
command,
cwd,
env,
exec_policy,
with_escalated_permissions,
justification,
approval_requirement,
}
}
}
@@ -136,26 +130,8 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
})
}
fn approval_requirement(
&self,
req: &UnifiedExecRequest,
policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
) -> ApprovalRequirement {
if let Some(exec_policy) = &req.exec_policy
&& let Some(requirement) = evaluate_with_policy(exec_policy, &req.command, policy)
{
requirement
} else if requires_initial_appoval(
policy,
sandbox_policy,
&req.command,
req.with_escalated_permissions.unwrap_or(false),
) {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
fn approval_requirement(&self, req: &UnifiedExecRequest) -> Option<ApprovalRequirement> {
Some(req.approval_requirement.clone())
}
fn wants_escalated_first_attempt(&self, req: &UnifiedExecRequest) -> bool {

49
codex-rs/core/src/tools/sandboxing.rs
View File

@@ -93,6 +93,28 @@ pub(crate) enum ApprovalRequirement {
Forbidden { reason: String },
}
/// Decide whether an initial user approval should be requested before the
/// first attempt. Defaults to the orchestrator's behavior (pre‑refactor):
/// - Never, OnFailure: do not ask
/// - OnRequest: ask unless sandbox policy is DangerFullAccess
/// - UnlessTrusted: always ask
pub(crate) fn default_approval_requirement(
policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
) -> ApprovalRequirement {
let needs_approval = match policy {
AskForApproval::Never | AskForApproval::OnFailure => false,
AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess),
AskForApproval::UnlessTrusted => true,
};
if needs_approval {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
}
pub(crate) trait Approvable<Req> {
type ApprovalKey: Hash + Eq + Clone + Debug + Serialize;
@@ -113,28 +135,11 @@ pub(crate) trait Approvable<Req> {
matches!(policy, AskForApproval::Never)
}
/// Decide whether an initial user approval should be requested before the
/// first attempt. Defaults to the orchestrator's behavior (pre‑refactor):
/// - Never, OnFailure: do not ask
/// - OnRequest: ask unless sandbox policy is DangerFullAccess
/// - UnlessTrusted: always ask
fn approval_requirement(
&self,
_req: &Req,
policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
) -> ApprovalRequirement {
let needs_approval = match policy {
AskForApproval::Never | AskForApproval::OnFailure => false,
AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess),
AskForApproval::UnlessTrusted => true,
};
if needs_approval {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip
}
/// Override the default approval requirement. Return `Some(_)` to specify
/// a custom requirement, or `None` to fall back to
/// policy-based default.
fn approval_requirement(&self, _req: &Req) -> Option<ApprovalRequirement> {
None
}
/// Decide we can request an approval for no-sandbox execution.

9
codex-rs/core/src/unified_exec/session_manager.rs
View File

@@ -11,6 +11,7 @@ use crate::codex::TurnContext;
use crate::exec::ExecToolCallOutput;
use crate::exec::StreamOutput;
use crate::exec_env::create_env;
use crate::exec_policy::approval_requirement_for_command;
use crate::protocol::BackgroundEventEvent;
use crate::protocol::EventMsg;
use crate::protocol::ExecCommandSource;
@@ -447,9 +448,15 @@ impl UnifiedExecSessionManager {
command.to_vec(),
cwd,
create_env(&context.turn.shell_environment_policy),
context.turn.exec_policy_v2.clone(),
with_escalated_permissions,
justification,
approval_requirement_for_command(
context.turn.exec_policy_v2.as_deref(),
command,
context.turn.approval_policy,
&context.turn.sandbox_policy,
with_escalated_permissions.unwrap_or(false),
),
);
let tool_ctx = ToolCtx {
session: context.session.as_ref(),