mirror of
https://github.com/openai/codex.git
synced 2026-02-02 06:57:03 +00:00
Compare commits
7 Commits
queue/stee
...
dev/zhao/4
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
16faa24118 | ||
|
|
e7c8a14457 | ||
|
|
f767635d3a | ||
|
|
b363899543 | ||
|
|
31c5e1116b | ||
|
|
174a8ddec5 | ||
|
|
b6dc1be5dd |
@@ -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 => {
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
);
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
¤t_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,
|
||||
};
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(), ¤t_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(), ¤t_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()]),
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -127,6 +127,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
cwd,
|
||||
Some(reason),
|
||||
risk,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
} else if user_explicitly_approved {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
@@ -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`).
|
||||
|
||||
|
||||
@@ -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>,
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
);
|
||||
}
|
||||
|
||||
@@ -180,6 +180,7 @@ async fn run_codex_tool_session_inner(
|
||||
call_id,
|
||||
reason: _,
|
||||
risk,
|
||||
proposed_execpolicy_amendment: _,
|
||||
parsed_cmd,
|
||||
}) => {
|
||||
handle_exec_approval_request(
|
||||
|
||||
@@ -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(),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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>,
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -563,6 +563,7 @@ mod tests {
|
||||
command: vec!["echo".into(), "ok".into()],
|
||||
reason: None,
|
||||
risk: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 "
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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();
|
||||
(
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user