mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Add PR description file for execpolicy refactor
This commit is contained in:
@@ -1,6 +0,0 @@
|
||||
## Summary
|
||||
- thread fallback evaluation through execpolicy so pipelines evaluate the first unmatched command instead of stopping at the first allow match
|
||||
- update core exec_policy integration, execpolicycheck CLI, and docs to use the new fallback path and avoid skipping heuristics on later piped commands
|
||||
|
||||
## Testing
|
||||
- not run (not requested)
|
||||
@@ -179,7 +179,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
allow_prefix: _allow_prefix,
|
||||
proposed_execpolicy_amendment: _proposed_execpolicy_amendment,
|
||||
parsed_cmd,
|
||||
}) => match api_version {
|
||||
ApiVersion::V1 => {
|
||||
|
||||
@@ -71,7 +71,7 @@ pub(crate) async fn apply_patch(
|
||||
.await;
|
||||
match rx_approve.await.unwrap_or_default() {
|
||||
ReviewDecision::Approved
|
||||
| ReviewDecision::ApprovedAllowPrefix { .. }
|
||||
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
|
||||
| ReviewDecision::ApprovedForSession => {
|
||||
InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec {
|
||||
action,
|
||||
|
||||
+17
-15
@@ -862,13 +862,11 @@ impl Session {
|
||||
.await
|
||||
}
|
||||
|
||||
/// Adds a prefix rule to the exec policy
|
||||
///
|
||||
/// This mutates the in-memory execpolicy so the current conversation can use the new
|
||||
/// prefix and persists the change in default.execpolicy so new conversations will also allow the new prefix.
|
||||
pub(crate) async fn persist_command_allow_prefix(
|
||||
/// Adds an execpolicy amendment to both the in-memory and on-disk policies so future
|
||||
/// commands can use the newly approved prefix.
|
||||
pub(crate) async fn persist_execpolicy_amendment(
|
||||
&self,
|
||||
prefix: &[String],
|
||||
amendment: &[String],
|
||||
) -> Result<(), ExecPolicyUpdateError> {
|
||||
let (features, codex_home, current_policy) = {
|
||||
let state = self.state.lock().await;
|
||||
@@ -888,10 +886,10 @@ impl Session {
|
||||
return Err(ExecPolicyUpdateError::FeatureDisabled);
|
||||
}
|
||||
|
||||
crate::exec_policy::append_allow_prefix_rule_and_update(
|
||||
crate::exec_policy::append_execpolicy_amendment_and_update(
|
||||
&codex_home,
|
||||
¤t_policy,
|
||||
prefix,
|
||||
amendment,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -912,7 +910,7 @@ impl Session {
|
||||
cwd: PathBuf,
|
||||
reason: Option<String>,
|
||||
risk: Option<SandboxCommandAssessment>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
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.
|
||||
@@ -940,7 +938,7 @@ impl Session {
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
allow_prefix,
|
||||
proposed_execpolicy_amendment,
|
||||
parsed_cmd,
|
||||
});
|
||||
self.send_event(turn_context, event).await;
|
||||
@@ -1672,13 +1670,17 @@ mod handlers {
|
||||
}
|
||||
}
|
||||
|
||||
/// Propagate a user's exec approval decision to the session
|
||||
/// Also optionally whitelists command in execpolicy
|
||||
/// 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::ApprovedAllowPrefix { allow_prefix } = &decision
|
||||
&& let Err(err) = sess.persist_command_allow_prefix(allow_prefix).await
|
||||
if let ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment,
|
||||
} = &decision
|
||||
&& let Err(err) = sess
|
||||
.persist_execpolicy_amendment(proposed_execpolicy_amendment)
|
||||
.await
|
||||
{
|
||||
let message = format!("Failed to update execpolicy allow list: {err}");
|
||||
let message = format!("Failed to apply execpolicy amendment: {err}");
|
||||
tracing::warn!("{message}");
|
||||
let warning = EventMsg::Warning(WarningEvent { message });
|
||||
sess.send_event_raw(Event {
|
||||
|
||||
@@ -275,7 +275,7 @@ async fn handle_exec_approval(
|
||||
event.cwd,
|
||||
event.reason,
|
||||
event.risk,
|
||||
event.allow_prefix,
|
||||
event.proposed_execpolicy_amendment,
|
||||
);
|
||||
let decision = await_approval_with_cancel(
|
||||
approval_fut,
|
||||
|
||||
@@ -115,7 +115,7 @@ pub(crate) fn default_policy_path(codex_home: &Path) -> PathBuf {
|
||||
codex_home.join(POLICY_DIR_NAME).join(DEFAULT_POLICY_FILE)
|
||||
}
|
||||
|
||||
pub(crate) async fn append_allow_prefix_rule_and_update(
|
||||
pub(crate) async fn append_execpolicy_amendment_and_update(
|
||||
codex_home: &Path,
|
||||
current_policy: &Arc<RwLock<Policy>>,
|
||||
prefix: &[String],
|
||||
@@ -142,8 +142,24 @@ pub(crate) async fn append_allow_prefix_rule_and_update(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Return an allow-prefix option when a command needs approval and execpolicy did not drive the decision.
|
||||
fn allow_prefix_if_applicable(evaluation: &Evaluation, features: &Features) -> Option<Vec<String>> {
|
||||
/// 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) || evaluation.decision != Decision::Prompt {
|
||||
return None;
|
||||
}
|
||||
@@ -214,7 +230,10 @@ pub(crate) async fn create_exec_approval_requirement_for_command(
|
||||
} else {
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: derive_prompt_reason(&evaluation),
|
||||
allow_prefix: allow_prefix_if_applicable(&evaluation, features),
|
||||
proposed_execpolicy_amendment: proposed_execpolicy_amendment(
|
||||
&evaluation,
|
||||
features,
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -438,7 +457,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: Some(PROMPT_REASON.to_string()),
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -490,7 +509,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: Some(command)
|
||||
proposed_execpolicy_amendment: Some(command)
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -521,18 +540,18 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
.await,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: Some(vec!["orange".to_string()])
|
||||
proposed_execpolicy_amendment: Some(vec!["orange".to_string()])
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn append_allow_prefix_rule_updates_policy_and_file() {
|
||||
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_allow_prefix_rule_and_update(codex_home.path(), ¤t_policy, &prefix)
|
||||
append_execpolicy_amendment_and_update(codex_home.path(), ¤t_policy, &prefix)
|
||||
.await
|
||||
.expect("update policy");
|
||||
|
||||
@@ -558,12 +577,12 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn append_allow_prefix_rule_rejects_empty_prefix() {
|
||||
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_allow_prefix_rule_and_update(codex_home.path(), ¤t_policy, &[]).await;
|
||||
append_execpolicy_amendment_and_update(codex_home.path(), ¤t_policy, &[]).await;
|
||||
|
||||
assert!(matches!(
|
||||
result,
|
||||
@@ -575,7 +594,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn allow_prefix_is_present_for_single_command_without_policy_match() {
|
||||
async fn proposed_execpolicy_amendment_is_present_for_single_command_without_policy_match() {
|
||||
let command = vec!["cargo".to_string(), "build".to_string()];
|
||||
|
||||
let empty_policy = Arc::new(RwLock::new(Policy::empty()));
|
||||
@@ -593,13 +612,13 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: Some(command)
|
||||
proposed_execpolicy_amendment: Some(command)
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn allow_prefix_is_disabled_when_execpolicy_feature_disabled() {
|
||||
async fn proposed_execpolicy_amendment_is_disabled_when_execpolicy_feature_disabled() {
|
||||
let command = vec!["cargo".to_string(), "build".to_string()];
|
||||
|
||||
let mut features = Features::with_defaults();
|
||||
@@ -619,13 +638,13 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn allow_prefix_is_omitted_when_policy_prompts() {
|
||||
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
|
||||
@@ -648,13 +667,13 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: Some(PROMPT_REASON.to_string()),
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn allow_prefix_is_omitted_for_multi_command_scripts() {
|
||||
async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() {
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
@@ -674,14 +693,14 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: Some(vec!["cargo".to_string(), "build".to_string()]),
|
||||
proposed_execpolicy_amendment: Some(vec!["cargo".to_string(), "build".to_string()]),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn allow_prefix_uses_first_no_match_in_multi_command_scripts() {
|
||||
let policy_src = r#"prefix_rule(pattern=["python"], decision="allow")"#;
|
||||
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)
|
||||
@@ -691,7 +710,7 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
let command = vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"python && echo ok".to_string(),
|
||||
"cat && apple".to_string(),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
@@ -704,8 +723,9 @@ prefix_rule(pattern=["rm"], decision="forbidden")
|
||||
SandboxPermissions::UseDefault,
|
||||
)
|
||||
.await,
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: true
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(vec!["apple".to_string()]),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -241,6 +241,7 @@ impl ShellHandler {
|
||||
SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)),
|
||||
)
|
||||
.await;
|
||||
|
||||
let req = ShellRequest {
|
||||
command: exec_params.command.clone(),
|
||||
cwd: exec_params.cwd.clone(),
|
||||
|
||||
@@ -95,7 +95,7 @@ impl ToolOrchestrator {
|
||||
return Err(ToolError::Rejected("rejected by user".to_string()));
|
||||
}
|
||||
ReviewDecision::Approved
|
||||
| ReviewDecision::ApprovedAllowPrefix { .. }
|
||||
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
|
||||
| ReviewDecision::ApprovedForSession => {}
|
||||
}
|
||||
already_approved = true;
|
||||
@@ -178,7 +178,7 @@ impl ToolOrchestrator {
|
||||
return Err(ToolError::Rejected("rejected by user".to_string()));
|
||||
}
|
||||
ReviewDecision::Approved
|
||||
| ReviewDecision::ApprovedAllowPrefix { .. }
|
||||
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
|
||||
| ReviewDecision::ApprovedForSession => {}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -114,7 +114,9 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
req.exec_approval_requirement.allow_prefix().cloned(),
|
||||
req.exec_approval_requirement
|
||||
.proposed_execpolicy_amendment()
|
||||
.cloned(),
|
||||
)
|
||||
.await
|
||||
})
|
||||
|
||||
@@ -132,7 +132,9 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
cwd,
|
||||
reason,
|
||||
risk,
|
||||
req.exec_approval_requirement.allow_prefix().cloned(),
|
||||
req.exec_approval_requirement
|
||||
.proposed_execpolicy_amendment()
|
||||
.cloned(),
|
||||
)
|
||||
.await
|
||||
})
|
||||
|
||||
@@ -98,18 +98,19 @@ pub(crate) enum ExecApprovalRequirement {
|
||||
/// Approval required for this tool call.
|
||||
NeedsApproval {
|
||||
reason: Option<String>,
|
||||
/// Prefix that can be whitelisted via execpolicy to skip future approvals for similar commands
|
||||
allow_prefix: Option<Vec<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 ExecApprovalRequirement {
|
||||
pub fn allow_prefix(&self) -> Option<&Vec<String>> {
|
||||
pub fn proposed_execpolicy_amendment(&self) -> Option<&Vec<String>> {
|
||||
match self {
|
||||
Self::NeedsApproval {
|
||||
allow_prefix: Some(prefix),
|
||||
proposed_execpolicy_amendment: Some(prefix),
|
||||
..
|
||||
} => Some(prefix),
|
||||
_ => None,
|
||||
@@ -133,7 +134,7 @@ pub(crate) fn default_exec_approval_requirement(
|
||||
if needs_approval {
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
} else {
|
||||
ExecApprovalRequirement::Skip {
|
||||
|
||||
@@ -1560,7 +1560,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Result<()> {
|
||||
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::ReadOnly;
|
||||
@@ -1580,8 +1580,9 @@ async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Re
|
||||
.prepare(&test, &server, call_id_first, false)
|
||||
.await?;
|
||||
let expected_command =
|
||||
expected_command.expect("allow prefix scenario should produce a shell command");
|
||||
let expected_allow_prefix = vec!["touch".to_string(), "allow-prefix.txt".to_string()];
|
||||
expected_command.expect("execpolicy amendment scenario should produce a shell command");
|
||||
let expected_execpolicy_amendment =
|
||||
vec!["touch".to_string(), "allow-prefix.txt".to_string()];
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
@@ -1610,13 +1611,16 @@ async fn approving_allow_prefix_persists_policy_and_skips_future_prompts() -> Re
|
||||
.await?;
|
||||
|
||||
let approval = expect_exec_approval(&test, expected_command.as_str()).await;
|
||||
assert_eq!(approval.allow_prefix, Some(expected_allow_prefix.clone()));
|
||||
assert_eq!(
|
||||
approval.proposed_execpolicy_amendment,
|
||||
Some(expected_execpolicy_amendment.clone())
|
||||
);
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::ApprovedAllowPrefix {
|
||||
allow_prefix: expected_allow_prefix.clone(),
|
||||
decision: ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: expected_execpolicy_amendment.clone(),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -180,7 +180,7 @@ async fn run_codex_tool_session_inner(
|
||||
call_id,
|
||||
reason: _,
|
||||
risk,
|
||||
allow_prefix: _,
|
||||
proposed_execpolicy_amendment: _,
|
||||
parsed_cmd,
|
||||
}) => {
|
||||
handle_exec_approval_request(
|
||||
|
||||
@@ -51,10 +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>,
|
||||
/// Prefix rule that can be added to the user's execpolicy to allow future runs.
|
||||
/// 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 allow_prefix: Option<Vec<String>>,
|
||||
pub proposed_execpolicy_amendment: Option<Vec<String>>,
|
||||
pub parsed_cmd: Vec<ParsedCommand>,
|
||||
}
|
||||
|
||||
|
||||
@@ -1652,9 +1652,11 @@ pub enum ReviewDecision {
|
||||
/// User has approved this command and the agent should execute it.
|
||||
Approved,
|
||||
|
||||
/// User has approved this command and wants to add the command prefix to
|
||||
/// the execpolicy allow list so future matching commands are permitted.
|
||||
ApprovedAllowPrefix { allow_prefix: Vec<String> },
|
||||
/// 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
|
||||
|
||||
@@ -43,7 +43,7 @@ pub(crate) enum ApprovalRequest {
|
||||
command: Vec<String>,
|
||||
reason: Option<String>,
|
||||
risk: Option<SandboxCommandAssessment>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
proposed_execpolicy_amendment: Option<Vec<String>>,
|
||||
},
|
||||
ApplyPatch {
|
||||
id: String,
|
||||
@@ -105,8 +105,11 @@ impl ApprovalOverlay {
|
||||
header: Box<dyn Renderable>,
|
||||
) -> (Vec<ApprovalOption>, SelectionViewParams) {
|
||||
let (options, title) = match &variant {
|
||||
ApprovalVariant::Exec { allow_prefix, .. } => (
|
||||
exec_options(allow_prefix.clone()),
|
||||
ApprovalVariant::Exec {
|
||||
proposed_execpolicy_amendment,
|
||||
..
|
||||
} => (
|
||||
exec_options(proposed_execpolicy_amendment.clone()),
|
||||
"Would you like to run the following command?".to_string(),
|
||||
),
|
||||
ApprovalVariant::ApplyPatch { .. } => (
|
||||
@@ -337,7 +340,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
|
||||
command,
|
||||
reason,
|
||||
risk,
|
||||
allow_prefix,
|
||||
proposed_execpolicy_amendment,
|
||||
} => {
|
||||
let reason = reason.filter(|item| !item.is_empty());
|
||||
let has_reason = reason.is_some();
|
||||
@@ -360,7 +363,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
|
||||
variant: ApprovalVariant::Exec {
|
||||
id,
|
||||
command,
|
||||
allow_prefix,
|
||||
proposed_execpolicy_amendment,
|
||||
},
|
||||
header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })),
|
||||
}
|
||||
@@ -437,7 +440,7 @@ enum ApprovalVariant {
|
||||
Exec {
|
||||
id: String,
|
||||
command: Vec<String>,
|
||||
allow_prefix: Option<Vec<String>>,
|
||||
proposed_execpolicy_amendment: Option<Vec<String>>,
|
||||
},
|
||||
ApplyPatch {
|
||||
id: String,
|
||||
@@ -470,7 +473,7 @@ impl ApprovalOption {
|
||||
}
|
||||
}
|
||||
|
||||
fn exec_options(allow_prefix: Option<Vec<String>>) -> Vec<ApprovalOption> {
|
||||
fn exec_options(proposed_execpolicy_amendment: Option<Vec<String>>) -> Vec<ApprovalOption> {
|
||||
vec![
|
||||
ApprovalOption {
|
||||
label: "Yes, proceed".to_string(),
|
||||
@@ -486,10 +489,10 @@ fn exec_options(allow_prefix: Option<Vec<String>>) -> Vec<ApprovalOption> {
|
||||
},
|
||||
]
|
||||
.into_iter()
|
||||
.chain(allow_prefix.map(|prefix| ApprovalOption {
|
||||
.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::ApprovedAllowPrefix {
|
||||
allow_prefix: prefix,
|
||||
decision: ApprovalDecision::Review(ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: prefix,
|
||||
}),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
|
||||
@@ -556,7 +559,7 @@ mod tests {
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
reason: Some("reason".to_string()),
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -590,7 +593,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_prefix_option_emits_allow_prefix() {
|
||||
fn exec_prefix_option_emits_execpolicy_amendment() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = ApprovalOverlay::new(
|
||||
@@ -599,7 +602,7 @@ mod tests {
|
||||
command: vec!["echo".to_string()],
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: Some(vec!["echo".to_string()]),
|
||||
proposed_execpolicy_amendment: Some(vec!["echo".to_string()]),
|
||||
},
|
||||
tx,
|
||||
);
|
||||
@@ -609,8 +612,8 @@ mod tests {
|
||||
if let AppEvent::CodexOp(Op::ExecApproval { decision, .. }) = ev {
|
||||
assert_eq!(
|
||||
decision,
|
||||
ReviewDecision::ApprovedAllowPrefix {
|
||||
allow_prefix: vec!["echo".to_string()]
|
||||
ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: vec!["echo".to_string()]
|
||||
}
|
||||
);
|
||||
saw_op = true;
|
||||
@@ -633,7 +636,7 @@ mod tests {
|
||||
command,
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
};
|
||||
|
||||
let view = ApprovalOverlay::new(exec_request, tx);
|
||||
|
||||
@@ -563,7 +563,7 @@ mod tests {
|
||||
command: vec!["echo".into(), "ok".into()],
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1069,7 +1069,7 @@ impl ChatWidget {
|
||||
command: ev.command,
|
||||
reason: ev.reason,
|
||||
risk: ev.risk,
|
||||
allow_prefix: ev.allow_prefix,
|
||||
proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment,
|
||||
};
|
||||
self.bottom_pane.push_approval_request(request);
|
||||
self.request_redraw();
|
||||
|
||||
@@ -677,7 +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,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -722,7 +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,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -773,7 +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,
|
||||
allow_prefix: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -1972,7 +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,
|
||||
allow_prefix: Some(vec!["echo".into(), "hello".into(), "world".into()]),
|
||||
proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello".into(), "world".into()]),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -2019,7 +2019,7 @@ fn approval_modal_exec_without_reason_snapshot() {
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
reason: None,
|
||||
risk: None,
|
||||
allow_prefix: Some(vec!["echo".into(), "hello".into(), "world".into()]),
|
||||
proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello".into(), "world".into()]),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -2233,7 +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,
|
||||
allow_prefix: Some(vec!["echo".into(), "hello world".into()]),
|
||||
proposed_execpolicy_amendment: Some(vec!["echo".into(), "hello world".into()]),
|
||||
parsed_cmd: vec![],
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
|
||||
@@ -408,7 +408,7 @@ pub fn new_approval_decision_cell(
|
||||
],
|
||||
)
|
||||
}
|
||||
ApprovedAllowPrefix { .. } => {
|
||||
ApprovedExecpolicyAmendment { .. } => {
|
||||
let snippet = Span::from(exec_snippet(&command)).dim();
|
||||
(
|
||||
"✔ ".green(),
|
||||
@@ -417,7 +417,7 @@ pub fn new_approval_decision_cell(
|
||||
"approved".bold(),
|
||||
" codex to run ".into(),
|
||||
snippet,
|
||||
" and added its prefix to your allow list".bold(),
|
||||
" and applied the execpolicy amendment".bold(),
|
||||
],
|
||||
)
|
||||
}
|
||||
|
||||
+1
-1
@@ -603,7 +603,7 @@ metadata above):
|
||||
- `codex.tool_decision`
|
||||
- `tool_name`
|
||||
- `call_id`
|
||||
- `decision` (`approved`, `approved_allow_prefix`, `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`
|
||||
|
||||
+2
-2
@@ -53,10 +53,10 @@ When no rules match, `matchedRules` is an empty array and `decision` is omitted.
|
||||
|
||||
```json
|
||||
{
|
||||
"matchedRules": [],
|
||||
"matchedRules": []
|
||||
}
|
||||
```
|
||||
|
||||
## Status
|
||||
|
||||
`execpolicy` commands are still in preview. The API may have breaking changes in the future.
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user