mirror of
https://github.com/openai/codex.git
synced 2026-09-14 11:57:03 +00:00
add unit tests
This commit is contained in:
@@ -2408,6 +2408,69 @@ fn map_file_change_approval_decision(
|
||||
}
|
||||
}
|
||||
|
||||
fn map_command_execution_approval_decision(
|
||||
decision: CommandExecutionApprovalDecision,
|
||||
allow_override_command: bool,
|
||||
) -> (ReviewDecision, Option<CommandExecutionStatus>) {
|
||||
match decision {
|
||||
CommandExecutionApprovalDecision::Accept => (ReviewDecision::Approved, None),
|
||||
CommandExecutionApprovalDecision::AcceptWithOverrideCommand { command } => {
|
||||
if !allow_override_command {
|
||||
error!(
|
||||
"failed to deserialize CommandExecutionRequestApprovalResponse: override command is not allowed for network-only approvals"
|
||||
);
|
||||
(
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
)
|
||||
} else if command.is_empty() {
|
||||
error!(
|
||||
"failed to deserialize CommandExecutionRequestApprovalResponse: override command cannot be empty"
|
||||
);
|
||||
(
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
)
|
||||
} else {
|
||||
(ReviewDecision::ApprovedOverrideCommand { command }, None)
|
||||
}
|
||||
}
|
||||
CommandExecutionApprovalDecision::AcceptForSession => {
|
||||
(ReviewDecision::ApprovedForSession, None)
|
||||
}
|
||||
CommandExecutionApprovalDecision::AcceptWithExecpolicyAmendment {
|
||||
execpolicy_amendment,
|
||||
} => (
|
||||
ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: execpolicy_amendment.into_core(),
|
||||
},
|
||||
None,
|
||||
),
|
||||
CommandExecutionApprovalDecision::ApplyNetworkPolicyAmendment {
|
||||
network_policy_amendment,
|
||||
} => {
|
||||
let completion_status = match network_policy_amendment.action {
|
||||
V2NetworkPolicyRuleAction::Allow => None,
|
||||
V2NetworkPolicyRuleAction::Deny => Some(CommandExecutionStatus::Declined),
|
||||
};
|
||||
(
|
||||
ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: network_policy_amendment.into_core(),
|
||||
},
|
||||
completion_status,
|
||||
)
|
||||
}
|
||||
CommandExecutionApprovalDecision::Decline => (
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
),
|
||||
CommandExecutionApprovalDecision::Cancel => (
|
||||
ReviewDecision::Abort,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn on_file_change_request_approval_response(
|
||||
event_turn_id: String,
|
||||
@@ -2502,67 +2565,12 @@ async fn on_command_execution_request_approval_response(
|
||||
}
|
||||
});
|
||||
|
||||
let decision = response.decision;
|
||||
|
||||
let (decision, completion_status) = match decision {
|
||||
CommandExecutionApprovalDecision::Accept => (ReviewDecision::Approved, None),
|
||||
CommandExecutionApprovalDecision::AcceptWithOverrideCommand { command } => {
|
||||
// Network-only approval prompts omit the command, so they
|
||||
// must not allow clients to steer execution via override.
|
||||
if completion_item.is_none() {
|
||||
error!(
|
||||
"failed to deserialize CommandExecutionRequestApprovalResponse: override command is not allowed for network-only approvals"
|
||||
);
|
||||
(
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
)
|
||||
} else if command.is_empty() {
|
||||
error!(
|
||||
"failed to deserialize CommandExecutionRequestApprovalResponse: override command cannot be empty"
|
||||
);
|
||||
(
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
)
|
||||
} else {
|
||||
(ReviewDecision::ApprovedOverrideCommand { command }, None)
|
||||
}
|
||||
}
|
||||
CommandExecutionApprovalDecision::AcceptForSession => {
|
||||
(ReviewDecision::ApprovedForSession, None)
|
||||
}
|
||||
CommandExecutionApprovalDecision::AcceptWithExecpolicyAmendment {
|
||||
execpolicy_amendment,
|
||||
} => (
|
||||
ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
proposed_execpolicy_amendment: execpolicy_amendment.into_core(),
|
||||
},
|
||||
None,
|
||||
),
|
||||
CommandExecutionApprovalDecision::ApplyNetworkPolicyAmendment {
|
||||
network_policy_amendment,
|
||||
} => {
|
||||
let completion_status = match network_policy_amendment.action {
|
||||
V2NetworkPolicyRuleAction::Allow => None,
|
||||
V2NetworkPolicyRuleAction::Deny => Some(CommandExecutionStatus::Declined),
|
||||
};
|
||||
(
|
||||
ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: network_policy_amendment.into_core(),
|
||||
},
|
||||
completion_status,
|
||||
)
|
||||
}
|
||||
CommandExecutionApprovalDecision::Decline => (
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
),
|
||||
CommandExecutionApprovalDecision::Cancel => (
|
||||
ReviewDecision::Abort,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
),
|
||||
};
|
||||
let (decision, completion_status) = map_command_execution_approval_decision(
|
||||
response.decision,
|
||||
// Network-only prompts omit the completion item, and they
|
||||
// must not allow command override responses.
|
||||
completion_item.is_some(),
|
||||
);
|
||||
(decision, completion_status)
|
||||
}
|
||||
Ok(Err(err)) if is_turn_transition_server_request_error(&err) => return,
|
||||
@@ -2907,6 +2915,19 @@ mod tests {
|
||||
assert_eq!(completion_status, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_execution_override_is_rejected_for_network_only_prompts() {
|
||||
let (decision, completion_status) = map_command_execution_approval_decision(
|
||||
CommandExecutionApprovalDecision::AcceptWithOverrideCommand {
|
||||
command: vec!["echo".to_string(), "hi".to_string()],
|
||||
},
|
||||
false,
|
||||
);
|
||||
|
||||
assert_eq!(decision, ReviewDecision::Denied);
|
||||
assert_eq!(completion_status, Some(CommandExecutionStatus::Declined));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_server_elicitation_turn_transition_error_maps_to_cancel() {
|
||||
let error = JSONRPCErrorError {
|
||||
|
||||
@@ -36,6 +36,7 @@ use crate::tools::sandboxing::sandbox_override_for_first_attempt;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use futures::future::BoxFuture;
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -34,6 +34,7 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_match;
|
||||
use core_test_support::wait_for_event_with_timeout;
|
||||
use core_test_support::zsh_fork::build_zsh_fork_test;
|
||||
use core_test_support::zsh_fork::restrictive_workspace_write_policy;
|
||||
@@ -1815,6 +1816,102 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approved_override_command_updates_exec_events() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let sandbox_policy_for_config = sandbox_policy.clone();
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "override-command-events";
|
||||
let original_command = r#"cat < "hello" > /var/test.txt"#;
|
||||
let override_payload = "printf 'override-output'";
|
||||
let (event, expected_command) = ActionKind::RunCommand {
|
||||
command: original_command,
|
||||
}
|
||||
.prepare(
|
||||
&test,
|
||||
&server,
|
||||
call_id,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
)
|
||||
.await?;
|
||||
let expected_command =
|
||||
expected_command.expect("override command scenario should produce a shell command");
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-override-command-1"),
|
||||
event,
|
||||
ev_completed("resp-override-command-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let results_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-override-command-1", "done"),
|
||||
ev_completed("resp-override-command-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"override-command-events",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let approval = expect_exec_approval(&test, expected_command.as_str()).await;
|
||||
let mut override_command = approval.command.clone();
|
||||
*override_command
|
||||
.last_mut()
|
||||
.expect("shell approval command should include the shell payload") =
|
||||
override_payload.to_string();
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::ApprovedOverrideCommand {
|
||||
command: override_command.clone(),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
|
||||
let begin_event = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::ExecCommandBegin(ev) if ev.call_id == call_id => Some(ev.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
let end_event = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::ExecCommandEnd(ev) if ev.call_id == call_id => Some(ev.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
assert_eq!(begin_event.command, override_command);
|
||||
assert_eq!(end_event.command, override_command);
|
||||
assert_eq!(end_event.stdout, "override-output");
|
||||
|
||||
let output_item = results_mock.single_request().function_call_output(call_id);
|
||||
let result = parse_result(&output_item);
|
||||
assert_eq!(result.stdout, "override-output");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> {
|
||||
|
||||
@@ -330,3 +330,66 @@ pub struct ApplyPatchApprovalRequestEvent {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub grant_root: Option<PathBuf>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn command_prompts_advertise_override_decision() {
|
||||
let command = vec!["echo".to_string(), "hi".to_string()];
|
||||
|
||||
let decisions =
|
||||
ExecApprovalRequestEvent::default_available_decisions(&command, None, None, None, None);
|
||||
|
||||
assert_eq!(
|
||||
decisions,
|
||||
vec![
|
||||
ReviewDecision::Approved,
|
||||
ReviewDecision::ApprovedOverrideCommand {
|
||||
command: command.clone(),
|
||||
},
|
||||
ReviewDecision::Abort,
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_only_prompts_do_not_advertise_override_decision() {
|
||||
let decisions = ExecApprovalRequestEvent::default_available_decisions(
|
||||
&["echo".to_string(), "hi".to_string()],
|
||||
Some(&NetworkApprovalContext {
|
||||
host: "example.com".to_string(),
|
||||
protocol: NetworkApprovalProtocol::Https,
|
||||
}),
|
||||
None,
|
||||
Some(&[
|
||||
NetworkPolicyAmendment {
|
||||
host: "example.com".to_string(),
|
||||
action: NetworkPolicyRuleAction::Allow,
|
||||
},
|
||||
NetworkPolicyAmendment {
|
||||
host: "example.com".to_string(),
|
||||
action: NetworkPolicyRuleAction::Deny,
|
||||
},
|
||||
]),
|
||||
None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
decisions,
|
||||
vec![
|
||||
ReviewDecision::Approved,
|
||||
ReviewDecision::ApprovedForSession,
|
||||
ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: NetworkPolicyAmendment {
|
||||
host: "example.com".to_string(),
|
||||
action: NetworkPolicyRuleAction::Allow,
|
||||
},
|
||||
},
|
||||
ReviewDecision::Abort,
|
||||
]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user