Compare commits

...

7 Commits

Author SHA1 Message Date
kevin zhao
16faa24118 . 2025-12-03 18:01:15 -08:00
kevin zhao
e7c8a14457 Add PR description file for execpolicy refactor 2025-12-03 15:56:41 -08:00
kevin zhao
f767635d3a Add PR description 2025-12-03 15:40:18 -08:00
kevin zhao
b363899543 Refactor execpolicy fallback evaluation 2025-12-03 15:40:17 -08:00
kevin zhao
31c5e1116b . 2025-12-03 23:14:35 +00:00
kevin zhao
174a8ddec5 add back line 2025-12-03 17:46:30 +00:00
kevin zhao
b6dc1be5dd Add approval allow-prefix flow in core and tui
Add explicit prefix-approval decision and wire it through execpolicy/UI snapshots

update doc

mutating in memory policy instead of reloading

using RW locks

clippy

refactor: adding allow_prefix into ApprovedAllowPrefix

fmt

do not send allow_prefix if execpolicy is disabled

moving args around

cleanup exec_policy getters

undo diff

fixing rw lock bug causing tui to hang

updating phrasing

integration test

.

fix compile

fix flaky test

fix compile error

running test with single thread

fixup allow_prefix_if_applicable

fix formatting

fix approvals test

only cloning when needed

docs

add docstring

fix rebase bug

fixing rebase issues

Revert "fixing rebase issues"

This reverts commit 79ce7e1f2fc0378c2c0b362408e2e544566540fd.

fix rebase errors
2025-12-02 22:01:35 -05:00
35 changed files with 1000 additions and 234 deletions

View File

@@ -179,6 +179,7 @@ pub(crate) async fn apply_bespoke_event_handling(
cwd,
reason,
risk,
proposed_execpolicy_amendment: _proposed_execpolicy_amendment,
parsed_cmd,
}) => match api_version {
ApiVersion::V1 => {

View File

@@ -40,17 +40,15 @@ prefix_rule(
assert_eq!(
result,
json!({
"match": {
"decision": "forbidden",
"matchedRules": [
{
"prefixRuleMatch": {
"matchedPrefix": ["git", "push"],
"decision": "forbidden"
}
"decision": "forbidden",
"matchedRules": [
{
"prefixRuleMatch": {
"matchedPrefix": ["git", "push"],
"decision": "forbidden"
}
]
}
}
]
})
);

View File

@@ -70,7 +70,9 @@ pub(crate) async fn apply_patch(
)
.await;
match rx_approve.await.unwrap_or_default() {
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => {
InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec {
action,
user_explicitly_approved_this_action: true,

View File

@@ -71,6 +71,7 @@ use crate::error::CodexErr;
use crate::error::Result as CodexResult;
#[cfg(test)]
use crate::exec::StreamOutput;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::mcp::auth::compute_auth_statuses;
use crate::mcp_connection_manager::McpConnectionManager;
use crate::model_family::find_family_for_model;
@@ -288,7 +289,7 @@ pub(crate) struct TurnContext {
pub(crate) final_output_json_schema: Option<Value>,
pub(crate) codex_linux_sandbox_exe: Option<PathBuf>,
pub(crate) tool_call_gate: Arc<ReadinessFlag>,
pub(crate) exec_policy: Arc<ExecPolicy>,
pub(crate) exec_policy: Arc<RwLock<ExecPolicy>>,
pub(crate) truncation_policy: TruncationPolicy,
}
@@ -346,7 +347,7 @@ pub(crate) struct SessionConfiguration {
/// Set of feature flags for this session
features: Features,
/// Execpolicy policy, applied only when enabled by feature flag.
exec_policy: Arc<ExecPolicy>,
exec_policy: Arc<RwLock<ExecPolicy>>,
// TODO(pakrym): Remove config from here
original_config_do_not_use: Arc<Config>,
@@ -861,11 +862,44 @@ impl Session {
.await
}
pub(crate) async fn persist_execpolicy_amendment(
&self,
amendment: &[String],
) -> Result<(), ExecPolicyUpdateError> {
let (features, codex_home, current_policy) = {
let state = self.state.lock().await;
(
state.session_configuration.features.clone(),
state
.session_configuration
.original_config_do_not_use
.codex_home
.clone(),
state.session_configuration.exec_policy.clone(),
)
};
if !features.enabled(Feature::ExecPolicy) {
error!("attempted to append execpolicy rule while execpolicy feature is disabled");
return Err(ExecPolicyUpdateError::FeatureDisabled);
}
crate::exec_policy::append_execpolicy_amendment_and_update(
&codex_home,
&current_policy,
amendment,
)
.await?;
Ok(())
}
/// Emit an exec approval request event and await the user's decision.
///
/// The request is keyed by `sub_id`/`call_id` so matching responses are delivered
/// to the correct in-flight turn. If the task is aborted, this returns the
/// default `ReviewDecision` (`Denied`).
#[allow(clippy::too_many_arguments)]
pub async fn request_command_approval(
&self,
turn_context: &TurnContext,
@@ -874,6 +908,7 @@ impl Session {
cwd: PathBuf,
reason: Option<String>,
risk: Option<SandboxCommandAssessment>,
proposed_execpolicy_amendment: Option<Vec<String>>,
) -> ReviewDecision {
let sub_id = turn_context.sub_id.clone();
// Add the tx_approve callback to the map before sending the request.
@@ -901,6 +936,7 @@ impl Session {
cwd,
reason,
risk,
proposed_execpolicy_amendment,
parsed_cmd,
});
self.send_event(turn_context, event).await;
@@ -1075,6 +1111,15 @@ impl Session {
.enabled(feature)
}
pub(crate) async fn features(&self) -> Features {
self.state
.lock()
.await
.session_configuration
.features
.clone()
}
async fn send_raw_response_items(&self, turn_context: &TurnContext, items: &[ResponseItem]) {
for item in items {
self.send_event(
@@ -1508,6 +1553,7 @@ mod handlers {
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::ReviewRequest;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::WarningEvent;
use codex_protocol::user_input::UserInput;
use codex_rmcp_client::ElicitationAction;
@@ -1622,7 +1668,25 @@ mod handlers {
}
}
/// Propagate a user's exec approval decision to the session.
/// Also optionally applies an execpolicy amendment.
pub async fn exec_approval(sess: &Arc<Session>, id: String, decision: ReviewDecision) {
if let ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
} = &decision
&& let Err(err) = sess
.persist_execpolicy_amendment(proposed_execpolicy_amendment)
.await
{
let message = format!("Failed to apply execpolicy amendment: {err}");
tracing::warn!("{message}");
let warning = EventMsg::Warning(WarningEvent { message });
sess.send_event_raw(Event {
id: id.clone(),
msg: warning,
})
.await;
}
match decision {
ReviewDecision::Abort => {
sess.interrupt_task().await;
@@ -2578,7 +2642,7 @@ mod tests {
cwd: config.cwd.clone(),
original_config_do_not_use: Arc::clone(&config),
features: Features::default(),
exec_policy: Arc::new(ExecPolicy::empty()),
exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())),
session_source: SessionSource::Exec,
};
@@ -2777,7 +2841,7 @@ mod tests {
cwd: config.cwd.clone(),
original_config_do_not_use: Arc::clone(&config),
features: Features::default(),
exec_policy: Arc::new(ExecPolicy::empty()),
exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())),
session_source: SessionSource::Exec,
};
@@ -2855,7 +2919,7 @@ mod tests {
cwd: config.cwd.clone(),
original_config_do_not_use: Arc::clone(&config),
features: Features::default(),
exec_policy: Arc::new(ExecPolicy::empty()),
exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())),
session_source: SessionSource::Exec,
};

View File

@@ -275,6 +275,7 @@ async fn handle_exec_approval(
event.cwd,
event.reason,
event.risk,
event.proposed_execpolicy_amendment,
);
let decision = await_approval_with_cancel(
approval_fut,

View File

@@ -4,14 +4,20 @@ use std::path::PathBuf;
use std::sync::Arc;
use crate::command_safety::is_dangerous_command::requires_initial_appoval;
use codex_execpolicy::AmendError;
use codex_execpolicy::Decision;
use codex_execpolicy::Error as ExecPolicyRuleError;
use codex_execpolicy::Evaluation;
use codex_execpolicy::Policy;
use codex_execpolicy::PolicyParser;
use codex_execpolicy::RuleMatch;
use codex_execpolicy::blocking_append_allow_prefix_rule;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use thiserror::Error;
use tokio::fs;
use tokio::sync::RwLock;
use tokio::task::spawn_blocking;
use crate::bash::parse_shell_lc_plain_commands;
use crate::features::Feature;
@@ -20,9 +26,12 @@ use crate::sandboxing::SandboxPermissions;
use crate::tools::sandboxing::ApprovalRequirement;
const FORBIDDEN_REASON: &str = "execpolicy forbids this command";
const PROMPT_CONFLICT_REASON: &str =
"execpolicy requires approval for this command, but AskForApproval is set to Never";
const PROMPT_REASON: &str = "execpolicy requires approval for this command";
const POLICY_DIR_NAME: &str = "policy";
const POLICY_EXTENSION: &str = "codexpolicy";
const DEFAULT_POLICY_FILE: &str = "default.codexpolicy";
#[derive(Debug, Error)]
pub enum ExecPolicyError {
@@ -45,12 +54,30 @@ pub enum ExecPolicyError {
},
}
#[derive(Debug, Error)]
pub enum ExecPolicyUpdateError {
#[error("failed to update execpolicy file {path}: {source}")]
AppendRule { path: PathBuf, source: AmendError },
#[error("failed to join blocking execpolicy update task: {source}")]
JoinBlockingTask { source: tokio::task::JoinError },
#[error("failed to update in-memory execpolicy: {source}")]
AddRule {
#[from]
source: ExecPolicyRuleError,
},
#[error("cannot append execpolicy rule because execpolicy feature is disabled")]
FeatureDisabled,
}
pub(crate) async fn exec_policy_for(
features: &Features,
codex_home: &Path,
) -> Result<Arc<Policy>, ExecPolicyError> {
) -> Result<Arc<RwLock<Policy>>, ExecPolicyError> {
if !features.enabled(Feature::ExecPolicy) {
return Ok(Arc::new(Policy::empty()));
return Ok(Arc::new(RwLock::new(Policy::empty())));
}
let policy_dir = codex_home.join(POLICY_DIR_NAME);
@@ -74,7 +101,7 @@ pub(crate) async fn exec_policy_for(
})?;
}
let policy = Arc::new(parser.build());
let policy = Arc::new(RwLock::new(parser.build()));
tracing::debug!(
"loaded execpolicy from {} files in {}",
policy_paths.len(),
@@ -84,62 +111,143 @@ pub(crate) async fn exec_policy_for(
Ok(policy)
}
fn evaluate_with_policy(
policy: &Policy,
command: &[String],
approval_policy: AskForApproval,
) -> Option<ApprovalRequirement> {
let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]);
let evaluation = policy.check_multiple(commands.iter());
pub(crate) fn default_policy_path(codex_home: &Path) -> PathBuf {
codex_home.join(POLICY_DIR_NAME).join(DEFAULT_POLICY_FILE)
}
match evaluation {
Evaluation::Match { decision, .. } => match decision {
Decision::Forbidden => Some(ApprovalRequirement::Forbidden {
reason: FORBIDDEN_REASON.to_string(),
}),
Decision::Prompt => {
let reason = PROMPT_REASON.to_string();
if matches!(approval_policy, AskForApproval::Never) {
Some(ApprovalRequirement::Forbidden { reason })
} else {
Some(ApprovalRequirement::NeedsApproval {
reason: Some(reason),
})
pub(crate) async fn append_execpolicy_amendment_and_update(
codex_home: &Path,
current_policy: &Arc<RwLock<Policy>>,
prefix: &[String],
) -> Result<(), ExecPolicyUpdateError> {
let policy_path = default_policy_path(codex_home);
let prefix = prefix.to_vec();
spawn_blocking({
let policy_path = policy_path.clone();
let prefix = prefix.clone();
move || blocking_append_allow_prefix_rule(&policy_path, &prefix)
})
.await
.map_err(|source| ExecPolicyUpdateError::JoinBlockingTask { source })?
.map_err(|source| ExecPolicyUpdateError::AppendRule {
path: policy_path,
source,
})?;
current_policy
.write()
.await
.add_prefix_rule(&prefix, Decision::Allow)?;
Ok(())
}
/// Returns a proposed execpolicy amendment only when heuristics caused
/// the prompt decision, so we can offer to apply that amendment for future runs.
///
/// The amendment uses the first command heuristics marked as `Prompt`. If any explicit
/// execpolicy rule also prompts, we return `None` because applying the amendment would not
/// skip that policy requirement.
///
/// Examples:
/// - execpolicy: empty. Command: `["python"]`. Heuristics prompt -> `Some(vec!["python"])`.
/// - execpolicy: empty. Command: `["bash", "-c", "cd /some/folder && prog1 --option1 arg1 && prog2 --option2 arg2"]`.
/// Parsed commands include `cd /some/folder`, `prog1 --option1 arg1`, and `prog2 --option2 arg2`. If heuristics allow `cd` but prompt
/// on `prog1`, we return `Some(vec!["prog1", "--option1", "arg1"])`.
/// - execpolicy: contains a `prompt for prefix ["prog2"]` rule. For the same command as above,
/// we return `None` because an execpolicy prompt still applies even if we amend execpolicy to allow ["prog1", "--option1", "arg1"].
fn proposed_execpolicy_amendment(
evaluation: &Evaluation,
features: &Features,
) -> Option<Vec<String>> {
if !features.enabled(Feature::ExecPolicy) {
return None;
}
if evaluation.decision != Decision::Prompt {
return None;
}
let mut first_prompt_from_heuristics: Option<Vec<String>> = None;
for rule_match in &evaluation.matched_rules {
match rule_match {
RuleMatch::HeuristicsRuleMatch { command, decision } => {
if *decision == Decision::Prompt && first_prompt_from_heuristics.is_none() {
first_prompt_from_heuristics = Some(command.clone());
}
}
Decision::Allow => Some(ApprovalRequirement::Skip {
bypass_sandbox: true,
}),
},
Evaluation::NoMatch { .. } => None,
_ if rule_match.decision() == Decision::Prompt => {
return None;
}
_ => {}
}
}
first_prompt_from_heuristics
}
pub(crate) async fn create_approval_requirement_for_command(
policy: &Policy,
exec_policy: &Arc<RwLock<Policy>>,
features: &Features,
command: &[String],
approval_policy: AskForApproval,
sandbox_policy: &SandboxPolicy,
sandbox_permissions: SandboxPermissions,
) -> ApprovalRequirement {
if let Some(requirement) = evaluate_with_policy(policy, command, approval_policy) {
return requirement;
}
if requires_initial_appoval(
approval_policy,
sandbox_policy,
command,
sandbox_permissions,
) {
ApprovalRequirement::NeedsApproval { reason: None }
} else {
ApprovalRequirement::Skip {
bypass_sandbox: false,
let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]);
let heuristics_fallback = |cmd: &[String]| {
if requires_initial_appoval(approval_policy, sandbox_policy, cmd, sandbox_permissions) {
Decision::Prompt
} else {
Decision::Allow
}
};
let policy = exec_policy.read().await;
let evaluation = policy.check_multiple(commands.iter(), &heuristics_fallback);
let has_policy_allow = evaluation.matched_rules.iter().any(|rule_match| {
!matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. })
&& rule_match.decision() == Decision::Allow
});
match evaluation.decision {
Decision::Forbidden => ApprovalRequirement::Forbidden {
reason: FORBIDDEN_REASON.to_string(),
},
Decision::Prompt => {
let prompt_reason = derive_prompt_reason(&evaluation);
if matches!(approval_policy, AskForApproval::Never) {
ApprovalRequirement::Forbidden {
reason: PROMPT_CONFLICT_REASON.to_string(),
}
} else {
ApprovalRequirement::NeedsApproval {
reason: prompt_reason,
proposed_execpolicy_amendment: proposed_execpolicy_amendment(
&evaluation,
features,
),
}
}
}
Decision::Allow => ApprovalRequirement::Skip {
bypass_sandbox: has_policy_allow,
},
}
}
/// Only return PROMPT_REASON when an execpolicy rule drove the prompt decision
fn derive_prompt_reason(evaluation: &Evaluation) -> Option<String> {
evaluation.matched_rules.iter().find_map(|rule_match| {
if !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. })
&& rule_match.decision() == Decision::Prompt
{
Some(PROMPT_REASON.to_string())
} else {
None
}
})
}
async fn collect_policy_files(dir: &Path) -> Result<Vec<PathBuf>, ExecPolicyError> {
let mut read_dir = match fs::read_dir(dir).await {
Ok(read_dir) => read_dir,
@@ -195,6 +303,7 @@ mod tests {
use codex_protocol::protocol::SandboxPolicy;
use pretty_assertions::assert_eq;
use std::fs;
use std::sync::Arc;
use tempfile::tempdir;
#[tokio::test]
@@ -208,10 +317,19 @@ mod tests {
.expect("policy result");
let commands = [vec!["rm".to_string()]];
assert!(matches!(
policy.check_multiple(commands.iter()),
Evaluation::NoMatch { .. }
));
assert_eq!(
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec!["rm".to_string()],
decision: Decision::Allow
}],
},
policy
.read()
.await
.check_multiple(commands.iter(), &|_| Decision::Allow)
);
assert!(!temp_dir.path().join(POLICY_DIR_NAME).exists());
}
@@ -242,10 +360,19 @@ mod tests {
.await
.expect("policy result");
let command = [vec!["rm".to_string()]];
assert!(matches!(
policy.check_multiple(command.iter()),
Evaluation::Match { .. }
));
assert_eq!(
Evaluation {
decision: Decision::Forbidden,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["rm".to_string()],
decision: Decision::Forbidden
}],
},
policy
.read()
.await
.check_multiple(command.iter(), &|_| Decision::Allow)
);
}
#[tokio::test]
@@ -261,14 +388,23 @@ mod tests {
.await
.expect("policy result");
let command = [vec!["ls".to_string()]];
assert!(matches!(
policy.check_multiple(command.iter()),
Evaluation::NoMatch { .. }
));
assert_eq!(
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec!["ls".to_string()],
decision: Decision::Allow
}],
},
policy
.read()
.await
.check_multiple(command.iter(), &|_| Decision::Allow)
);
}
#[test]
fn evaluates_bash_lc_inner_commands() {
#[tokio::test]
async fn evaluates_bash_lc_inner_commands() {
let policy_src = r#"
prefix_rule(pattern=["rm"], decision="forbidden")
"#;
@@ -276,7 +412,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = parser.build();
let policy = Arc::new(RwLock::new(parser.build()));
let forbidden_script = vec![
"bash".to_string(),
@@ -284,9 +420,15 @@ prefix_rule(pattern=["rm"], decision="forbidden")
"rm -rf /tmp".to_string(),
];
let requirement =
evaluate_with_policy(&policy, &forbidden_script, AskForApproval::OnRequest)
.expect("expected match for forbidden command");
let requirement = create_approval_requirement_for_command(
&policy,
&Features::with_defaults(),
&forbidden_script,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
)
.await;
assert_eq!(
requirement,
@@ -303,11 +445,12 @@ prefix_rule(pattern=["rm"], decision="forbidden")
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = parser.build();
let policy = Arc::new(RwLock::new(parser.build()));
let command = vec!["rm".to_string()];
let requirement = create_approval_requirement_for_command(
&policy,
&Features::with_defaults(),
&command,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
@@ -318,7 +461,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval {
reason: Some(PROMPT_REASON.to_string())
reason: Some(PROMPT_REASON.to_string()),
proposed_execpolicy_amendment: None,
}
);
}
@@ -330,11 +474,12 @@ prefix_rule(pattern=["rm"], decision="forbidden")
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = parser.build();
let policy = Arc::new(RwLock::new(parser.build()));
let command = vec!["rm".to_string()];
let requirement = create_approval_requirement_for_command(
&policy,
&Features::with_defaults(),
&command,
AskForApproval::Never,
&SandboxPolicy::DangerFullAccess,
@@ -345,7 +490,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
assert_eq!(
requirement,
ApprovalRequirement::Forbidden {
reason: PROMPT_REASON.to_string()
reason: PROMPT_CONFLICT_REASON.to_string()
}
);
}
@@ -354,9 +499,10 @@ prefix_rule(pattern=["rm"], decision="forbidden")
async fn approval_requirement_falls_back_to_heuristics() {
let command = vec!["python".to_string()];
let empty_policy = Policy::empty();
let empty_policy = Arc::new(RwLock::new(Policy::empty()));
let requirement = create_approval_requirement_for_command(
&empty_policy,
&Features::with_defaults(),
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
@@ -366,7 +512,200 @@ prefix_rule(pattern=["rm"], decision="forbidden")
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval { reason: None }
ApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(command)
}
);
}
#[tokio::test]
async fn heuristics_apply_when_other_commands_match_policy() {
let policy_src = r#"prefix_rule(pattern=["apple"], decision="allow")"#;
let mut parser = PolicyParser::new();
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = Arc::new(RwLock::new(parser.build()));
let command = vec![
"bash".to_string(),
"-lc".to_string(),
"apple | orange".to_string(),
];
assert_eq!(
create_approval_requirement_for_command(
&policy,
&Features::with_defaults(),
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
)
.await,
ApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(vec!["orange".to_string()])
}
);
}
#[tokio::test]
async fn append_execpolicy_amendment_updates_policy_and_file() {
let codex_home = tempdir().expect("create temp dir");
let current_policy = Arc::new(RwLock::new(Policy::empty()));
let prefix = vec!["echo".to_string(), "hello".to_string()];
append_execpolicy_amendment_and_update(codex_home.path(), &current_policy, &prefix)
.await
.expect("update policy");
let evaluation = current_policy.read().await.check(
&["echo".to_string(), "hello".to_string(), "world".to_string()],
&|_| Decision::Allow,
);
assert!(matches!(
evaluation,
Evaluation {
decision: Decision::Allow,
..
}
));
let contents = fs::read_to_string(default_policy_path(codex_home.path()))
.expect("policy file should have been created");
assert_eq!(
contents,
r#"prefix_rule(pattern=["echo", "hello"], decision="allow")
"#
);
}
#[tokio::test]
async fn append_execpolicy_amendment_rejects_empty_prefix() {
let codex_home = tempdir().expect("create temp dir");
let current_policy = Arc::new(RwLock::new(Policy::empty()));
let result =
append_execpolicy_amendment_and_update(codex_home.path(), &current_policy, &[]).await;
assert!(matches!(
result,
Err(ExecPolicyUpdateError::AppendRule {
source: AmendError::EmptyPrefix,
..
})
));
}
#[tokio::test]
async fn proposed_execpolicy_amendment_is_present_for_single_command_without_policy_match() {
let command = vec!["python".to_string()];
let empty_policy = Arc::new(RwLock::new(Policy::empty()));
let requirement = create_approval_requirement_for_command(
&empty_policy,
&Features::with_defaults(),
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
SandboxPermissions::UseDefault,
)
.await;
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(command)
}
);
}
#[tokio::test]
async fn proposed_execpolicy_amendment_is_disabled_when_execpolicy_feature_disabled() {
let command = vec!["python".to_string()];
let mut features = Features::with_defaults();
features.disable(Feature::ExecPolicy);
let requirement = create_approval_requirement_for_command(
&Arc::new(RwLock::new(Policy::empty())),
&features,
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
SandboxPermissions::UseDefault,
)
.await;
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
}
);
}
#[tokio::test]
async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
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 = Arc::new(RwLock::new(parser.build()));
let command = vec!["rm".to_string()];
let requirement = create_approval_requirement_for_command(
&policy,
&Features::with_defaults(),
&command,
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
)
.await;
assert_eq!(
requirement,
ApprovalRequirement::NeedsApproval {
reason: Some(PROMPT_REASON.to_string()),
proposed_execpolicy_amendment: None,
}
);
}
#[tokio::test]
async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scripts() {
let policy_src = r#"prefix_rule(pattern=["cat"], decision="allow")"#;
let mut parser = PolicyParser::new();
parser
.parse("test.codexpolicy", policy_src)
.expect("parse policy");
let policy = Arc::new(RwLock::new(parser.build()));
let command = vec![
"bash".to_string(),
"-lc".to_string(),
"cat && apple".to_string(),
];
assert_eq!(
create_approval_requirement_for_command(
&policy,
&Features::with_defaults(),
&command,
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
SandboxPermissions::UseDefault,
)
.await,
ApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(vec!["apple".to_string()]),
}
);
}
}

View File

@@ -231,8 +231,10 @@ impl ShellHandler {
let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None);
emitter.begin(event_ctx).await;
let features = session.features().await;
let approval_requirement = create_approval_requirement_for_command(
&turn.exec_policy,
&features,
&exec_params.command,
turn.approval_policy,
&turn.sandbox_policy,

View File

@@ -59,12 +59,12 @@ impl ToolOrchestrator {
});
match requirement {
ApprovalRequirement::Skip { .. } => {
otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg);
otel.tool_decision(otel_tn, otel_ci, &ReviewDecision::Approved, otel_cfg);
}
ApprovalRequirement::Forbidden { reason } => {
return Err(ToolError::Rejected(reason));
}
ApprovalRequirement::NeedsApproval { reason } => {
ApprovalRequirement::NeedsApproval { reason, .. } => {
let mut risk = None;
if let Some(metadata) = req.sandbox_retry_data() {
@@ -88,13 +88,15 @@ impl ToolOrchestrator {
};
let decision = tool.start_approval_async(req, approval_ctx).await;
otel.tool_decision(otel_tn, otel_ci, decision, otel_user.clone());
otel.tool_decision(otel_tn, otel_ci, &decision, otel_user.clone());
match decision {
ReviewDecision::Denied | ReviewDecision::Abort => {
return Err(ToolError::Rejected("rejected by user".to_string()));
}
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {}
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => {}
}
already_approved = true;
}
@@ -119,25 +121,20 @@ impl ToolOrchestrator {
};
match tool.run(req, &initial_attempt, tool_ctx).await {
Ok(out) => {
// We have a successful initial result
Ok(out)
}
Ok(out) => Ok(out),
Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) => {
if !tool.escalate_on_failure() {
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
})));
}
// Under `Never` or `OnRequest`, do not retry without sandbox; surface a concise
// sandbox denial that preserves the original output.
if !tool.wants_no_sandbox_approval(approval_policy) {
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
output,
})));
}
// Ask for approval before retrying without sandbox.
if !tool.should_bypass_approval(approval_policy, already_approved) {
let mut risk = None;
@@ -169,13 +166,15 @@ impl ToolOrchestrator {
};
let decision = tool.start_approval_async(req, approval_ctx).await;
otel.tool_decision(otel_tn, otel_ci, decision, otel_user);
otel.tool_decision(otel_tn, otel_ci, &decision, otel_user);
match decision {
ReviewDecision::Denied | ReviewDecision::Abort => {
return Err(ToolError::Rejected("rejected by user".to_string()));
}
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {}
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => {}
}
}
@@ -187,8 +186,7 @@ impl ToolOrchestrator {
codex_linux_sandbox_exe: None,
};
// Second attempt.
(*tool).run(req, &escalated_attempt, tool_ctx).await
tool.run(req, &escalated_attempt, tool_ctx).await
}
other => other,
}

View File

@@ -127,6 +127,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
cwd,
Some(reason),
risk,
None,
)
.await
} else if user_explicitly_approved {

View File

@@ -107,7 +107,17 @@ impl Approvable<ShellRequest> for ShellRuntime {
Box::pin(async move {
with_cached_approval(&session.services, key, move || async move {
session
.request_command_approval(turn, call_id, command, cwd, reason, risk)
.request_command_approval(
turn,
call_id,
command,
cwd,
reason,
risk,
req.approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
)
.await
})
.await

View File

@@ -125,7 +125,17 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
Box::pin(async move {
with_cached_approval(&session.services, key, || async move {
session
.request_command_approval(turn, call_id, command, cwd, reason, risk)
.request_command_approval(
turn,
call_id,
command,
cwd,
reason,
risk,
req.approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
)
.await
})
.await

View File

@@ -96,11 +96,28 @@ pub(crate) enum ApprovalRequirement {
bypass_sandbox: bool,
},
/// Approval required for this tool call
NeedsApproval { reason: Option<String> },
NeedsApproval {
reason: Option<String>,
/// Proposed execpolicy amendment to skip future approvals for similar commands
/// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined.
proposed_execpolicy_amendment: Option<Vec<String>>,
},
/// Execution forbidden for this tool call
Forbidden { reason: String },
}
impl ApprovalRequirement {
pub fn proposed_execpolicy_amendment(&self) -> Option<&Vec<String>> {
match self {
Self::NeedsApproval {
proposed_execpolicy_amendment: Some(prefix),
..
} => Some(prefix),
_ => None,
}
}
}
/// - Never, OnFailure: do not ask
/// - OnRequest: ask unless sandbox policy is DangerFullAccess
/// - UnlessTrusted: always ask
@@ -115,7 +132,10 @@ pub(crate) fn default_approval_requirement(
};
if needs_approval {
ApprovalRequirement::NeedsApproval { reason: None }
ApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
}
} else {
ApprovalRequirement::Skip {
bypass_sandbox: false,

View File

@@ -552,10 +552,12 @@ impl UnifiedExecSessionManager {
context: &UnifiedExecContext,
) -> Result<UnifiedExecSession, UnifiedExecError> {
let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
let features = context.session.features().await;
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = UnifiedExecRuntime::new(self);
let approval_requirement = create_approval_requirement_for_command(
&context.turn.exec_policy,
&features,
command,
context.turn.approval_policy,
&context.turn.sandbox_policy,

View File

@@ -1523,7 +1523,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
test.codex
.submit(Op::ExecApproval {
id: "0".into(),
decision: *decision,
decision: decision.clone(),
})
.await?;
wait_for_completion(&test).await;
@@ -1544,7 +1544,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
test.codex
.submit(Op::PatchApproval {
id: "0".into(),
decision: *decision,
decision: decision.clone(),
})
.await?;
wait_for_completion(&test).await;
@@ -1557,3 +1557,140 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> {
let server = start_mock_server().await;
let approval_policy = AskForApproval::UnlessTrusted;
let sandbox_policy = SandboxPolicy::DangerFullAccess;
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.approval_policy = approval_policy;
config.sandbox_policy = sandbox_policy_for_config;
});
let test = builder.build(&server).await?;
let call_id_first = "allow-prefix-first";
let (first_event, expected_command) = ActionKind::RunCommand {
command: "printf allow-prefix-ok",
}
.prepare(&test, &server, call_id_first, false)
.await?;
let expected_command =
expected_command.expect("execpolicy amendment scenario should produce a shell command");
let expected_execpolicy_amendment = expected_command
.split_whitespace()
.map(ToString::to_string)
.collect::<Vec<_>>();
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-allow-prefix-1"),
first_event,
ev_completed("resp-allow-prefix-1"),
]),
)
.await;
let first_results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-allow-prefix-1", "done"),
ev_completed("resp-allow-prefix-2"),
]),
)
.await;
submit_turn(
&test,
"allow-prefix-first",
approval_policy,
sandbox_policy.clone(),
)
.await?;
let approval = expect_exec_approval(&test, expected_command.as_str()).await;
assert_eq!(
approval.proposed_execpolicy_amendment,
Some(expected_execpolicy_amendment.clone())
);
test.codex
.submit(Op::ExecApproval {
id: "0".into(),
decision: ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: expected_execpolicy_amendment.clone(),
},
})
.await?;
wait_for_completion(&test).await;
let policy_path = test.home.path().join("policy").join("default.codexpolicy");
let policy_contents = fs::read_to_string(&policy_path)?;
assert!(
policy_contents
.contains(r#"prefix_rule(pattern=["printf", "allow-prefix-ok"], decision="allow")"#),
"unexpected policy contents: {policy_contents}"
);
let first_output = parse_result(
&first_results
.single_request()
.function_call_output(call_id_first),
);
assert_eq!(first_output.exit_code.unwrap_or(0), 0);
assert!(
first_output.stdout.contains("allow-prefix-ok"),
"unexpected stdout: {}",
first_output.stdout
);
let call_id_second = "allow-prefix-second";
let (second_event, second_command) = ActionKind::RunCommand {
command: "printf allow-prefix-ok",
}
.prepare(&test, &server, call_id_second, false)
.await?;
assert_eq!(second_command.as_deref(), Some(expected_command.as_str()));
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-allow-prefix-3"),
second_event,
ev_completed("resp-allow-prefix-3"),
]),
)
.await;
let second_results = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-allow-prefix-2", "done"),
ev_completed("resp-allow-prefix-4"),
]),
)
.await;
submit_turn(
&test,
"allow-prefix-second",
approval_policy,
sandbox_policy.clone(),
)
.await?;
wait_for_completion_without_approval(&test).await;
let second_output = parse_result(
&second_results
.single_request()
.function_call_output(call_id_second),
);
assert_eq!(second_output.exit_code.unwrap_or(0), 0);
assert!(
second_output.stdout.contains("allow-prefix-ok"),
"unexpected stdout: {}",
second_output.stdout
);
Ok(())
}

View File

@@ -30,32 +30,24 @@ codex execpolicy check --policy path/to/policy.codexpolicy git status
cargo run -p codex-execpolicy -- check --policy path/to/policy.codexpolicy git status
```
- Example outcomes:
- Match: `{"match": { ... "decision": "allow" ... }}`
- No match: `{"noMatch": {}}`
- Match: `{"matchedRules":[{...}],"decision":"allow"}`
- No match: `{"matchedRules":[]}`
## Response shapes
- Match:
## Response shape
```json
{
"match": {
"decision": "allow|prompt|forbidden",
"matchedRules": [
{
"prefixRuleMatch": {
"matchedPrefix": ["<token>", "..."],
"decision": "allow|prompt|forbidden"
}
"matchedRules": [
{
"prefixRuleMatch": {
"matchedPrefix": ["<token>", "..."],
"decision": "allow|prompt|forbidden"
}
]
}
}
],
"decision": "allow|prompt|forbidden"
}
```
- No match:
```json
{"noMatch": {}}
```
- When no rules match, `matchedRules` is an empty array and `decision` is omitted.
- `matchedRules` lists every rule whose prefix matched the command; `matchedPrefix` is the exact prefix that matched.
- The effective `decision` is the strictest severity across all matches (`forbidden` > `prompt` > `allow`).

View File

@@ -4,10 +4,12 @@ use std::path::PathBuf;
use anyhow::Context;
use anyhow::Result;
use clap::Parser;
use serde::Serialize;
use crate::Evaluation;
use crate::Decision;
use crate::Policy;
use crate::PolicyParser;
use crate::RuleMatch;
/// Arguments for evaluating a command against one or more execpolicy files.
#[derive(Debug, Parser, Clone)]
@@ -34,20 +36,25 @@ impl ExecPolicyCheckCommand {
/// Load the policies for this command, evaluate the command, and render JSON output.
pub fn run(&self) -> Result<()> {
let policy = load_policies(&self.policies)?;
let evaluation = policy.check(&self.command);
let matched_rules = policy.matches_for_command(&self.command, None);
let json = format_evaluation_json(&evaluation, self.pretty)?;
let json = format_matches_json(&matched_rules, self.pretty)?;
println!("{json}");
Ok(())
}
}
pub fn format_evaluation_json(evaluation: &Evaluation, pretty: bool) -> Result<String> {
pub fn format_matches_json(matched_rules: &[RuleMatch], pretty: bool) -> Result<String> {
let output = ExecPolicyCheckOutput {
matched_rules,
decision: matched_rules.iter().map(RuleMatch::decision).max(),
};
if pretty {
serde_json::to_string_pretty(evaluation).map_err(Into::into)
serde_json::to_string_pretty(&output).map_err(Into::into)
} else {
serde_json::to_string(evaluation).map_err(Into::into)
serde_json::to_string(&output).map_err(Into::into)
}
}
@@ -65,3 +72,12 @@ pub fn load_policies(policy_paths: &[PathBuf]) -> Result<Policy> {
Ok(parser.build())
}
#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
struct ExecPolicyCheckOutput<'a> {
#[serde(rename = "matchedRules")]
matched_rules: &'a [RuleMatch],
#[serde(skip_serializing_if = "Option::is_none")]
decision: Option<Decision>,
}

View File

@@ -1,6 +1,6 @@
use anyhow::Result;
use clap::Parser;
use codex_execpolicy::ExecPolicyCheckCommand;
use codex_execpolicy::execpolicycheck::ExecPolicyCheckCommand;
/// CLI for evaluating exec policies
#[derive(Parser)]
@@ -13,10 +13,6 @@ enum Cli {
fn main() -> Result<()> {
let cli = Cli::parse();
match cli {
Cli::Check(cmd) => cmd_check(cmd),
Cli::Check(cmd) => cmd.run(),
}
}
fn cmd_check(cmd: ExecPolicyCheckCommand) -> Result<()> {
cmd.run()
}

View File

@@ -11,6 +11,8 @@ use serde::Deserialize;
use serde::Serialize;
use std::sync::Arc;
type HeuristicsFallback<'a> = Option<&'a dyn Fn(&[String]) -> Decision>;
#[derive(Clone, Debug)]
pub struct Policy {
rules_by_program: MultiMap<String, RuleRef>,
@@ -50,62 +52,84 @@ impl Policy {
Ok(())
}
pub fn check(&self, cmd: &[String]) -> Evaluation {
let rules = match cmd.first() {
Some(first) => match self.rules_by_program.get_vec(first) {
Some(rules) => rules,
None => return Evaluation::NoMatch {},
},
None => return Evaluation::NoMatch {},
};
let matched_rules: Vec<RuleMatch> =
rules.iter().filter_map(|rule| rule.matches(cmd)).collect();
match matched_rules.iter().map(RuleMatch::decision).max() {
Some(decision) => Evaluation::Match {
decision,
matched_rules,
},
None => Evaluation::NoMatch {},
}
pub fn check<F>(&self, cmd: &[String], heuristics_fallback: &F) -> Evaluation
where
F: Fn(&[String]) -> Decision,
{
let matched_rules = self.matches_for_command(cmd, Some(heuristics_fallback));
Evaluation::from_matches(matched_rules)
}
pub fn check_multiple<Commands>(&self, commands: Commands) -> Evaluation
pub fn check_multiple<Commands, F>(
&self,
commands: Commands,
heuristics_fallback: &F,
) -> Evaluation
where
Commands: IntoIterator,
Commands::Item: AsRef<[String]>,
F: Fn(&[String]) -> Decision,
{
let matched_rules: Vec<RuleMatch> = commands
.into_iter()
.flat_map(|command| match self.check(command.as_ref()) {
Evaluation::Match { matched_rules, .. } => matched_rules,
Evaluation::NoMatch { .. } => Vec::new(),
.flat_map(|command| {
self.matches_for_command(command.as_ref(), Some(heuristics_fallback))
})
.collect();
match matched_rules.iter().map(RuleMatch::decision).max() {
Some(decision) => Evaluation::Match {
decision,
matched_rules,
},
None => Evaluation::NoMatch {},
Evaluation::from_matches(matched_rules)
}
pub fn matches_for_command(
&self,
cmd: &[String],
heuristics_fallback: HeuristicsFallback<'_>,
) -> Vec<RuleMatch> {
let mut matched_rules: Vec<RuleMatch> = match cmd.first() {
Some(first) => self
.rules_by_program
.get_vec(first)
.map(|rules| rules.iter().filter_map(|rule| rule.matches(cmd)).collect())
.unwrap_or_default(),
None => Vec::new(),
};
if let (true, Some(heuristics_fallback)) = (matched_rules.is_empty(), heuristics_fallback) {
matched_rules.push(RuleMatch::HeuristicsRuleMatch {
command: cmd.to_vec(),
decision: heuristics_fallback(cmd),
});
}
matched_rules
}
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub enum Evaluation {
NoMatch {},
Match {
decision: Decision,
#[serde(rename = "matchedRules")]
matched_rules: Vec<RuleMatch>,
},
pub struct Evaluation {
pub decision: Decision,
#[serde(rename = "matchedRules")]
pub matched_rules: Vec<RuleMatch>,
}
impl Evaluation {
pub fn is_match(&self) -> bool {
matches!(self, Self::Match { .. })
self.matched_rules
.iter()
.any(|rule_match| !matches!(rule_match, RuleMatch::HeuristicsRuleMatch { .. }))
}
fn from_matches(matched_rules: Vec<RuleMatch>) -> Self {
let decision = matched_rules
.iter()
.map(RuleMatch::decision)
.max()
.unwrap_or(Decision::Allow);
Self {
decision,
matched_rules,
}
}
}

View File

@@ -64,12 +64,17 @@ pub enum RuleMatch {
matched_prefix: Vec<String>,
decision: Decision,
},
HeuristicsRuleMatch {
command: Vec<String>,
decision: Decision,
},
}
impl RuleMatch {
pub fn decision(&self) -> Decision {
match self {
Self::PrefixRuleMatch { decision, .. } => *decision,
Self::HeuristicsRuleMatch { decision, .. } => *decision,
}
}
}

View File

@@ -19,6 +19,14 @@ fn tokens(cmd: &[&str]) -> Vec<String> {
cmd.iter().map(std::string::ToString::to_string).collect()
}
fn allow_all(_: &[String]) -> Decision {
Decision::Allow
}
fn prompt_all(_: &[String]) -> Decision {
Decision::Prompt
}
#[derive(Clone, Debug, Eq, PartialEq)]
enum RuleSnapshot {
Prefix(PrefixRule),
@@ -49,9 +57,9 @@ prefix_rule(
parser.parse("test.codexpolicy", policy_src)?;
let policy = parser.build();
let cmd = tokens(&["git", "status"]);
let evaluation = policy.check(&cmd);
let evaluation = policy.check(&cmd, &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
@@ -80,9 +88,9 @@ fn add_prefix_rule_extends_policy() -> Result<()> {
rules
);
let evaluation = policy.check(&tokens(&["ls", "-l", "/tmp"]));
let evaluation = policy.check(&tokens(&["ls", "-l", "/tmp"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["ls", "-l"]),
@@ -146,9 +154,9 @@ prefix_rule(
git_rules
);
let status_eval = policy.check(&tokens(&["git", "status"]));
let status_eval = policy.check(&tokens(&["git", "status"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git"]),
@@ -158,9 +166,9 @@ prefix_rule(
status_eval
);
let commit_eval = policy.check(&tokens(&["git", "commit", "-m", "hi"]));
let commit_eval = policy.check(&tokens(&["git", "commit", "-m", "hi"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Forbidden,
matched_rules: vec![
RuleMatch::PrefixRuleMatch {
@@ -217,9 +225,9 @@ prefix_rule(
sh_rules
);
let bash_eval = policy.check(&tokens(&["bash", "-c", "echo", "hi"]));
let bash_eval = policy.check(&tokens(&["bash", "-c", "echo", "hi"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["bash", "-c"]),
@@ -229,9 +237,9 @@ prefix_rule(
bash_eval
);
let sh_eval = policy.check(&tokens(&["sh", "-l", "echo", "hi"]));
let sh_eval = policy.check(&tokens(&["sh", "-l", "echo", "hi"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["sh", "-l"]),
@@ -273,9 +281,9 @@ prefix_rule(
rules
);
let npm_i = policy.check(&tokens(&["npm", "i", "--legacy-peer-deps"]));
let npm_i = policy.check(&tokens(&["npm", "i", "--legacy-peer-deps"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["npm", "i", "--legacy-peer-deps"]),
@@ -285,9 +293,12 @@ prefix_rule(
npm_i
);
let npm_install = policy.check(&tokens(&["npm", "install", "--no-save", "leftpad"]));
let npm_install = policy.check(
&tokens(&["npm", "install", "--no-save", "leftpad"]),
&allow_all,
);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["npm", "install", "--no-save"]),
@@ -314,9 +325,9 @@ prefix_rule(
let mut parser = PolicyParser::new();
parser.parse("test.codexpolicy", policy_src)?;
let policy = parser.build();
let match_eval = policy.check(&tokens(&["git", "status"]));
let match_eval = policy.check(&tokens(&["git", "status"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: tokens(&["git", "status"]),
@@ -326,13 +337,20 @@ prefix_rule(
match_eval
);
let no_match_eval = policy.check(&tokens(&[
"git",
"--config",
"color.status=always",
"status",
]));
assert_eq!(Evaluation::NoMatch {}, no_match_eval);
let no_match_eval = policy.check(
&tokens(&["git", "--config", "color.status=always", "status"]),
&allow_all,
);
assert_eq!(
Evaluation {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: tokens(&["git", "--config", "color.status=always", "status",]),
decision: Decision::Allow,
}],
},
no_match_eval
);
Ok(())
}
@@ -352,9 +370,9 @@ prefix_rule(
parser.parse("test.codexpolicy", policy_src)?;
let policy = parser.build();
let commit = policy.check(&tokens(&["git", "commit", "-m", "hi"]));
let commit = policy.check(&tokens(&["git", "commit", "-m", "hi"]), &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Forbidden,
matched_rules: vec![
RuleMatch::PrefixRuleMatch {
@@ -393,9 +411,9 @@ prefix_rule(
tokens(&["git", "commit", "-m", "hi"]),
];
let evaluation = policy.check_multiple(&commands);
let evaluation = policy.check_multiple(&commands, &allow_all);
assert_eq!(
Evaluation::Match {
Evaluation {
decision: Decision::Forbidden,
matched_rules: vec![
RuleMatch::PrefixRuleMatch {
@@ -416,3 +434,21 @@ prefix_rule(
);
Ok(())
}
#[test]
fn heuristics_match_is_returned_when_no_policy_matches() {
let policy = Policy::empty();
let command = tokens(&["python"]);
let evaluation = policy.check(&command, &prompt_all);
assert_eq!(
Evaluation {
decision: Decision::Prompt,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command,
decision: Decision::Prompt,
}],
},
evaluation
);
}

View File

@@ -180,6 +180,7 @@ async fn run_codex_tool_session_inner(
call_id,
reason: _,
risk,
proposed_execpolicy_amendment: _,
parsed_cmd,
}) => {
handle_exec_approval_request(

View File

@@ -352,7 +352,7 @@ impl OtelEventManager {
&self,
tool_name: &str,
call_id: &str,
decision: ReviewDecision,
decision: &ReviewDecision,
source: ToolDecisionSource,
) {
tracing::event!(
@@ -369,7 +369,7 @@ impl OtelEventManager {
slug = %self.metadata.slug,
tool_name = %tool_name,
call_id = %call_id,
decision = %decision.to_string().to_lowercase(),
decision = %decision.clone().to_string().to_lowercase(),
source = %source.to_string(),
);
}

View File

@@ -51,6 +51,10 @@ pub struct ExecApprovalRequestEvent {
/// Optional model-provided risk assessment describing the blocked command.
#[serde(skip_serializing_if = "Option::is_none")]
pub risk: Option<SandboxCommandAssessment>,
/// Proposed execpolicy amendment that can be applied to allow future runs.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional, type = "Array<string>")]
pub proposed_execpolicy_amendment: Option<Vec<String>>,
pub parsed_cmd: Vec<ParsedCommand>,
}

View File

@@ -1646,14 +1646,18 @@ pub struct SessionConfiguredEvent {
}
/// User's decision in response to an ExecApprovalRequest.
#[derive(
Debug, Default, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, Display, JsonSchema, TS,
)]
#[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, Eq, Display, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
pub enum ReviewDecision {
/// User has approved this command and the agent should execute it.
Approved,
/// User has approved this command and wants to apply the proposed execpolicy
/// amendment so future matching commands are permitted.
ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: Vec<String>,
},
/// User has approved this command and wants to automatically approve any
/// future identical instances (`command` and `cwd` match exactly) for the
/// remainder of the session.

View File

@@ -43,6 +43,7 @@ pub(crate) enum ApprovalRequest {
command: Vec<String>,
reason: Option<String>,
risk: Option<SandboxCommandAssessment>,
proposed_execpolicy_amendment: Option<Vec<String>>,
},
ApplyPatch {
id: String,
@@ -104,8 +105,11 @@ impl ApprovalOverlay {
header: Box<dyn Renderable>,
) -> (Vec<ApprovalOption>, SelectionViewParams) {
let (options, title) = match &variant {
ApprovalVariant::Exec { .. } => (
exec_options(),
ApprovalVariant::Exec {
proposed_execpolicy_amendment,
..
} => (
exec_options(proposed_execpolicy_amendment.clone()),
"Would you like to run the following command?".to_string(),
),
ApprovalVariant::ApplyPatch { .. } => (
@@ -160,12 +164,12 @@ impl ApprovalOverlay {
return;
};
if let Some(variant) = self.current_variant.as_ref() {
match (&variant, &option.decision) {
(ApprovalVariant::Exec { id, command }, ApprovalDecision::Review(decision)) => {
self.handle_exec_decision(id, command, *decision);
match (variant, &option.decision) {
(ApprovalVariant::Exec { id, command, .. }, ApprovalDecision::Review(decision)) => {
self.handle_exec_decision(id, command, decision.clone());
}
(ApprovalVariant::ApplyPatch { id, .. }, ApprovalDecision::Review(decision)) => {
self.handle_patch_decision(id, *decision);
self.handle_patch_decision(id, decision.clone());
}
(
ApprovalVariant::McpElicitation {
@@ -185,7 +189,7 @@ impl ApprovalOverlay {
}
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision);
let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision.clone());
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval {
id: id.to_string(),
@@ -273,7 +277,7 @@ impl BottomPaneView for ApprovalOverlay {
&& let Some(variant) = self.current_variant.as_ref()
{
match &variant {
ApprovalVariant::Exec { id, command } => {
ApprovalVariant::Exec { id, command, .. } => {
self.handle_exec_decision(id, command, ReviewDecision::Abort);
}
ApprovalVariant::ApplyPatch { id, .. } => {
@@ -336,6 +340,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
command,
reason,
risk,
proposed_execpolicy_amendment,
} => {
let reason = reason.filter(|item| !item.is_empty());
let has_reason = reason.is_some();
@@ -355,7 +360,11 @@ impl From<ApprovalRequest> for ApprovalRequestState {
}
header.extend(full_cmd_lines);
Self {
variant: ApprovalVariant::Exec { id, command },
variant: ApprovalVariant::Exec {
id,
command,
proposed_execpolicy_amendment,
},
header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })),
}
}
@@ -431,6 +440,7 @@ enum ApprovalVariant {
Exec {
id: String,
command: Vec<String>,
proposed_execpolicy_amendment: Option<Vec<String>>,
},
ApplyPatch {
id: String,
@@ -463,7 +473,7 @@ impl ApprovalOption {
}
}
fn exec_options() -> Vec<ApprovalOption> {
fn exec_options(proposed_execpolicy_amendment: Option<Vec<String>>) -> Vec<ApprovalOption> {
vec![
ApprovalOption {
label: "Yes, proceed".to_string(),
@@ -472,18 +482,28 @@ fn exec_options() -> Vec<ApprovalOption> {
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
},
ApprovalOption {
label: "Yes, and don't ask again for this command".to_string(),
label: "Yes, and don't ask again this session".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
},
ApprovalOption {
label: "No, and tell Codex what to do differently".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Abort),
display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
},
]
.into_iter()
.chain(proposed_execpolicy_amendment.map(|prefix| ApprovalOption {
label: "Yes, and don't ask again for commands with this prefix".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: prefix,
}),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
}))
.chain([ApprovalOption {
label: "No, and tell Codex what to do differently".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Abort),
display_shortcut: Some(key_hint::plain(KeyCode::Esc)),
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
}])
.collect()
}
fn patch_options() -> Vec<ApprovalOption> {
@@ -539,6 +559,7 @@ mod tests {
command: vec!["echo".to_string(), "hi".to_string()],
reason: Some("reason".to_string()),
risk: None,
proposed_execpolicy_amendment: None,
}
}
@@ -571,6 +592,40 @@ mod tests {
assert!(saw_op, "expected approval decision to emit an op");
}
#[test]
fn exec_prefix_option_emits_execpolicy_amendment() {
let (tx, mut rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx);
let mut view = ApprovalOverlay::new(
ApprovalRequest::Exec {
id: "test".to_string(),
command: vec!["echo".to_string()],
reason: None,
risk: None,
proposed_execpolicy_amendment: Some(vec!["echo".to_string()]),
},
tx,
);
view.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::NONE));
let mut saw_op = false;
while let Ok(ev) = rx.try_recv() {
if let AppEvent::CodexOp(Op::ExecApproval { decision, .. }) = ev {
assert_eq!(
decision,
ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: vec!["echo".to_string()]
}
);
saw_op = true;
break;
}
}
assert!(
saw_op,
"expected approval decision to emit an op with allow prefix"
);
}
#[test]
fn header_includes_command_snippet() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
@@ -581,6 +636,7 @@ mod tests {
command,
reason: None,
risk: None,
proposed_execpolicy_amendment: None,
};
let view = ApprovalOverlay::new(exec_request, tx);

View File

@@ -563,6 +563,7 @@ mod tests {
command: vec!["echo".into(), "ok".into()],
reason: None,
risk: None,
proposed_execpolicy_amendment: None,
}
}

View File

@@ -1069,6 +1069,7 @@ impl ChatWidget {
command: ev.command,
reason: ev.reason,
risk: ev.risk,
proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment,
};
self.bottom_pane.push_approval_request(request);
self.request_redraw();

View File

@@ -10,7 +10,8 @@ expression: terminal.backend().vt100().screen().contents()
$ echo hello world
1. Yes, proceed (y)
2. Yes, and don't ask again for this command (a)
3. No, and tell Codex what to do differently (esc)
2. Yes, and don't ask again this session (a)
3. Yes, and don't ask again for commands with this prefix (p)
4. No, and tell Codex what to do differently (esc)
Press enter to confirm or esc to cancel

View File

@@ -7,7 +7,8 @@ expression: terminal.backend().vt100().screen().contents()
$ echo hello world
1. Yes, proceed (y)
2. Yes, and don't ask again for this command (a)
3. No, and tell Codex what to do differently (esc)
2. Yes, and don't ask again this session (a)
3. Yes, and don't ask again for commands with this prefix (p)
4. No, and tell Codex what to do differently (esc)
Press enter to confirm or esc to cancel

View File

@@ -15,7 +15,7 @@ Buffer {
" $ echo hello world ",
" ",
" 1. Yes, proceed (y) ",
" 2. Yes, and don't ask again for this command (a) ",
" 2. Yes, and don't ask again this session (a) ",
" 3. No, and tell Codex what to do differently (esc) ",
" ",
" Press enter to confirm or esc to cancel ",
@@ -30,8 +30,8 @@ Buffer {
x: 7, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
x: 0, y: 9, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD,
x: 21, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
x: 48, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
x: 49, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
x: 44, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
x: 45, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
x: 48, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
x: 51, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
x: 2, y: 13, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,

View File

@@ -1,6 +1,5 @@
---
source: tui/src/chatwidget/tests.rs
assertion_line: 1548
expression: terminal.backend()
---
" "
@@ -13,7 +12,8 @@ expression: terminal.backend()
" $ echo 'hello world' "
" "
" 1. Yes, proceed (y) "
" 2. Yes, and don't ask again for this command (a) "
" 3. No, and tell Codex what to do differently (esc) "
" 2. Yes, and don't ask again this session (a) "
" 3. Yes, and don't ask again for commands with this prefix (p) "
" 4. No, and tell Codex what to do differently (esc) "
" "
" Press enter to confirm or esc to cancel "

View File

@@ -677,6 +677,7 @@ fn exec_approval_emits_proposed_command_and_decision_history() {
"this is a test reason such as one that would be produced by the model".into(),
),
risk: None,
proposed_execpolicy_amendment: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -721,6 +722,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() {
"this is a test reason such as one that would be produced by the model".into(),
),
risk: None,
proposed_execpolicy_amendment: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -771,6 +773,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() {
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None,
risk: None,
proposed_execpolicy_amendment: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -1969,6 +1972,7 @@ fn approval_modal_exec_snapshot() {
"this is a test reason such as one that would be produced by the model".into(),
),
risk: None,
proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello".into(), "world".into()]),
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -2015,6 +2019,7 @@ fn approval_modal_exec_without_reason_snapshot() {
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None,
risk: None,
proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello".into(), "world".into()]),
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -2228,6 +2233,7 @@ fn status_widget_and_approval_modal_snapshot() {
"this is a test reason such as one that would be produced by the model".into(),
),
risk: None,
proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello world".into()]),
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {

View File

@@ -408,6 +408,19 @@ pub fn new_approval_decision_cell(
],
)
}
ApprovedExecpolicyAmendment { .. } => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"".green(),
vec![
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" and applied the execpolicy amendment".bold(),
],
)
}
ApprovedForSession => {
let snippet = Span::from(exec_snippet(&command)).dim();
(

View File

@@ -603,7 +603,7 @@ metadata above):
- `codex.tool_decision`
- `tool_name`
- `call_id`
- `decision` (`approved`, `approved_for_session`, `denied`, or `abort`)
- `decision` (`approved`, `approved_execpolicy_amendment`, `approved_for_session`, `denied`, or `abort`)
- `source` (`config` or `user`)
- `codex.tool_result`
- `tool_name`

View File

@@ -33,6 +33,30 @@ codex execpolicy check --policy ~/.codex/policy/default.codexpolicy git push ori
Pass multiple `--policy` flags to test how several files combine, and use `--pretty` for formatted JSON output. See the [`codex-rs/execpolicy` README](../codex-rs/execpolicy/README.md) for a more detailed walkthrough of the available syntax.
Example output when a rule matches:
```json
{
"matchedRules": [
{
"prefixRuleMatch": {
"matchedPrefix": ["git", "push"],
"decision": "prompt"
}
}
],
"decision": "prompt"
}
```
When no rules match, `matchedRules` is an empty array and `decision` is omitted.
```json
{
"matchedRules": []
}
```
## Status
`execpolicy` commands are still in preview. The API may have breaking changes in the future.