mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Read approval policy from the current turn configuration (#36912)
## Why Thread settings can update the approval policy after a turn context is created. Keeping a separate copy on `TurnContext` could leave tool approval checks using the previous policy. ## What changed - Remove the duplicated approval-policy field from `TurnContext`. - Resolve the policy through the turn's current configuration everywhere it is needed, including tool execution, Guardian routing, MCP handling, and permission requests. ## Testing - Update the shell escalation test to apply a thread-level policy override and verify that the next turn rejects an escalation using the updated policy. GitOrigin-RevId: e5966ba08f179d53fd76871ad904762958c0f5ea
This commit is contained in:
@@ -25,7 +25,7 @@ pub(crate) fn prepare_apply_patch(
|
||||
) -> Result<ApplyPatchRuntimeInvocation, FunctionCallError> {
|
||||
match assess_patch_safety(
|
||||
&action,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.approval_policy(),
|
||||
&turn_context.permission_profile(),
|
||||
file_system_sandbox_policy,
|
||||
&action.cwd,
|
||||
|
||||
@@ -430,7 +430,8 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
|
||||
let mut config = (*parent_ctx.config).clone();
|
||||
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
parent_ctx.config = Arc::new(config);
|
||||
parent_ctx
|
||||
Arc::make_mut(&mut parent_ctx.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("set on-request policy");
|
||||
@@ -553,7 +554,8 @@ async fn delegated_mcp_guardian_abort_returns_synthetic_decline_answer() {
|
||||
let mut config = (*parent_ctx.config).clone();
|
||||
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
parent_ctx.config = Arc::new(config);
|
||||
parent_ctx
|
||||
Arc::make_mut(&mut parent_ctx.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("set on-request policy");
|
||||
|
||||
@@ -179,7 +179,7 @@ pub(crate) fn routes_approval_to_guardian_with_reviewer(
|
||||
turn: &TurnContext,
|
||||
approvals_reviewer: ApprovalsReviewer,
|
||||
) -> bool {
|
||||
routes_approval_policy_to_guardian(turn.approval_policy.value(), approvals_reviewer)
|
||||
routes_approval_policy_to_guardian(turn.approval_policy(), approvals_reviewer)
|
||||
}
|
||||
|
||||
/// Whether an exact approval policy and reviewer should route through Guardian.
|
||||
|
||||
@@ -1242,7 +1242,9 @@ async fn routes_approval_to_guardian_allows_granular_review_policy() {
|
||||
let mut config = (*turn.config).clone();
|
||||
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
turn.config = Arc::new(config);
|
||||
turn.approval_policy
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
|
||||
@@ -770,7 +770,7 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str);
|
||||
}
|
||||
|
||||
fn hook_permission_mode(turn_context: &TurnContext) -> String {
|
||||
match turn_context.approval_policy.value() {
|
||||
match turn_context.approval_policy() {
|
||||
AskForApproval::Never => "bypassPermissions",
|
||||
AskForApproval::UnlessTrusted | AskForApproval::OnRequest | AskForApproval::Granular(_) => {
|
||||
"default"
|
||||
|
||||
@@ -234,7 +234,7 @@ async fn should_install_mcp_dependencies(
|
||||
cancellation_token: &CancellationToken,
|
||||
) -> bool {
|
||||
if mcp_permission_prompt_is_auto_approved(
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.approval_policy(),
|
||||
&turn_context.permission_profile(),
|
||||
McpPermissionPromptAutoApproveContext::default(),
|
||||
) {
|
||||
|
||||
@@ -87,7 +87,6 @@ fn approval_metadata(
|
||||
|
||||
fn approval_config(turn_context: &TurnContext) -> codex_mcp::McpConfig {
|
||||
let mut config = (*mcp_config_for_test(&turn_context.config)).clone();
|
||||
config.approval_policy = turn_context.approval_policy.clone();
|
||||
config.permission_profile = turn_context.permission_profile.clone();
|
||||
config
|
||||
}
|
||||
@@ -1371,7 +1370,7 @@ async fn codex_apps_auth_elicitation_feature_disabled_returns_original_result()
|
||||
let returned = maybe_request_codex_apps_auth_elicitation(
|
||||
&session,
|
||||
&turn_context,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.approval_policy(),
|
||||
"call_123",
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
Some(&metadata),
|
||||
@@ -1415,7 +1414,8 @@ async fn codex_apps_auth_elicitation_granular_mcp_disabled_returns_original_resu
|
||||
features.enable(Feature::AuthElicitation);
|
||||
let mutable_turn_context = Arc::get_mut(&mut turn_context).expect("single turn context ref");
|
||||
Arc::make_mut(&mut mutable_turn_context.config).features = ManagedFeatures::from(features);
|
||||
mutable_turn_context
|
||||
Arc::make_mut(&mut mutable_turn_context.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
@@ -1431,7 +1431,7 @@ async fn codex_apps_auth_elicitation_granular_mcp_disabled_returns_original_resu
|
||||
let returned = maybe_request_codex_apps_auth_elicitation(
|
||||
&session,
|
||||
&turn_context,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.approval_policy(),
|
||||
"call_123",
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
Some(&metadata),
|
||||
@@ -1457,7 +1457,7 @@ async fn codex_apps_auth_elicitation_enabled_by_default_requests_elicitation() {
|
||||
maybe_request_codex_apps_auth_elicitation(
|
||||
&session,
|
||||
&turn_context,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.approval_policy(),
|
||||
"call_123",
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
Some(&metadata),
|
||||
@@ -2379,7 +2379,8 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
|
||||
.await;
|
||||
|
||||
let (mut session, mut turn_context) = make_session_and_context().await;
|
||||
turn_context
|
||||
Arc::make_mut(&mut turn_context.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -2678,7 +2679,8 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
|
||||
.await;
|
||||
|
||||
let (mut session, mut turn_context) = make_session_and_context().await;
|
||||
turn_context
|
||||
Arc::make_mut(&mut turn_context.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -2807,7 +2809,8 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
|
||||
#[tokio::test]
|
||||
async fn full_access_mode_skips_mcp_tool_approval_for_all_approval_modes() {
|
||||
let (session, mut turn_context) = make_session_and_context().await;
|
||||
turn_context
|
||||
Arc::make_mut(&mut turn_context.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -2907,7 +2910,8 @@ async fn approve_mode_skips_guardian_in_every_permission_mode() {
|
||||
turn_context.auth_manager = Some(crate::test_support::auth_manager_from_auth(
|
||||
codex_login::CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
));
|
||||
turn_context
|
||||
Arc::make_mut(&mut turn_context.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(approval_policy)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
|
||||
@@ -2408,7 +2408,7 @@ impl Session {
|
||||
environment: TurnEnvironmentSelection,
|
||||
cancellation_token: CancellationToken,
|
||||
) -> Option<RequestPermissionsResponse> {
|
||||
match turn_context.as_ref().approval_policy.value() {
|
||||
match turn_context.as_ref().approval_policy() {
|
||||
AskForApproval::Never => {
|
||||
return Some(RequestPermissionsResponse {
|
||||
permissions: RequestPermissionProfile::default(),
|
||||
|
||||
@@ -134,7 +134,6 @@ pub(super) async fn spawn_review_thread(
|
||||
.clone(),
|
||||
multi_agent_version: MultiAgentVersion::Disabled,
|
||||
personality: parent_turn_context.personality,
|
||||
approval_policy: parent_turn_context.approval_policy.clone(),
|
||||
permission_profile: parent_turn_context.permission_profile(),
|
||||
network: parent_turn_context.network.clone(),
|
||||
windows_sandbox_level: parent_turn_context.windows_sandbox_level,
|
||||
|
||||
@@ -186,7 +186,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -233,7 +233,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -1296,7 +1296,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -1385,7 +1385,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -1417,7 +1417,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -1547,7 +1547,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -1674,7 +1674,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -1844,7 +1844,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
|
||||
@@ -3426,7 +3426,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: turn_context.sandbox_policy(),
|
||||
permission_profile: None,
|
||||
@@ -6362,8 +6362,9 @@ fn strict_auto_review_session_scope_grants_no_permissions() {
|
||||
async fn request_permissions_emits_event_when_granular_policy_allows_requests() {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("single thread settings ref")
|
||||
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
|
||||
Arc::make_mut(&mut turn_context_mut.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
@@ -6458,7 +6459,8 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
|
||||
};
|
||||
std::fs::create_dir_all(environment_cwd.as_path()).expect("create environment cwd");
|
||||
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
|
||||
turn_context_mut
|
||||
Arc::make_mut(&mut turn_context_mut.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
@@ -6606,8 +6608,9 @@ async fn request_permissions_tool_rejects_unknown_environment_id() {
|
||||
async fn request_permissions_response_materializes_session_cwd_grants_before_recording() {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("single thread settings ref")
|
||||
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
|
||||
Arc::make_mut(&mut turn_context_mut.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
@@ -6717,8 +6720,9 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
|
||||
async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_requests() {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("single thread settings ref")
|
||||
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
|
||||
Arc::make_mut(&mut turn_context_mut.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
@@ -8147,7 +8151,7 @@ async fn refresh_mcp_servers_uses_latest_state_for_existing_turns() {
|
||||
async fn refreshed_mcp_binding_captures_current_approval_authority() {
|
||||
let (session, old_turn) = make_session_and_context().await;
|
||||
let session = Arc::new(session);
|
||||
let previous_policy = old_turn.approval_policy.value();
|
||||
let previous_policy = old_turn.approval_policy();
|
||||
assert_ne!(previous_policy, AskForApproval::Never);
|
||||
assert_eq!(
|
||||
old_turn.config.permissions.approval_policy.value(),
|
||||
@@ -8184,14 +8188,14 @@ async fn refreshed_mcp_binding_captures_current_approval_authority() {
|
||||
ApprovalsReviewer::AutoReview,
|
||||
)
|
||||
);
|
||||
assert_eq!(old_turn.approval_policy.value(), previous_policy);
|
||||
assert_eq!(old_turn.approval_policy(), previous_policy);
|
||||
assert_eq!(
|
||||
old_turn.config.permissions.approval_policy.value(),
|
||||
previous_policy
|
||||
);
|
||||
|
||||
let new_turn = session.new_default_turn().await;
|
||||
assert_eq!(new_turn.approval_policy.value(), AskForApproval::Never);
|
||||
assert_eq!(new_turn.approval_policy(), AskForApproval::Never);
|
||||
assert_eq!(
|
||||
new_turn.config.permissions.approval_policy.value(),
|
||||
AskForApproval::Never
|
||||
@@ -11150,7 +11154,8 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
|
||||
let (session, mut turn_context_raw) = make_session_and_context().await;
|
||||
// Ensure policy is NOT OnRequest so the early rejection path triggers
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -11199,7 +11204,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
|
||||
let expected = format!(
|
||||
"approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}",
|
||||
policy = turn_context.approval_policy.value()
|
||||
policy = turn_context.approval_policy()
|
||||
);
|
||||
|
||||
pretty_assertions::assert_eq!(output, expected);
|
||||
@@ -11225,7 +11230,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
permission_profile: turn_context.permission_profile(),
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
@@ -11316,7 +11321,8 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
|
||||
let (session, mut turn_context_raw) = make_session_and_context().await;
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -11353,7 +11359,7 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
|
||||
|
||||
let expected = format!(
|
||||
"approval policy is {policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {policy:?}",
|
||||
policy = turn_context.approval_policy.value()
|
||||
policy = turn_context.approval_policy()
|
||||
);
|
||||
|
||||
pretty_assertions::assert_eq!(output, expected);
|
||||
|
||||
@@ -92,7 +92,8 @@ async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() {
|
||||
|
||||
let (mut session, mut turn_context_raw) = make_session_and_context().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -180,7 +181,8 @@ async fn request_permissions_guardian_review_stops_when_cancelled() {
|
||||
let (mut session, mut turn_context, rx_event) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
let turn_context_raw = Arc::get_mut(&mut turn_context).expect("single turn context ref");
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -293,7 +295,8 @@ async fn guardian_allows_shell_command_additional_permissions_requests_past_poli
|
||||
.await;
|
||||
|
||||
let (mut session, mut turn_context_raw) = make_session_and_context().await;
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -412,7 +415,8 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_
|
||||
)
|
||||
.await;
|
||||
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::Never)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -476,7 +480,8 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_
|
||||
#[tokio::test]
|
||||
async fn guardian_allows_unified_exec_additional_permissions_requests_past_policy_validation() {
|
||||
let (mut session, mut turn_context_raw) = make_session_and_context().await;
|
||||
turn_context_raw
|
||||
Arc::make_mut(&mut turn_context_raw.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("test setup should allow updating approval policy");
|
||||
|
||||
@@ -969,7 +969,7 @@ async fn track_turn_resolved_config_analytics(
|
||||
.service_tier
|
||||
.as_deref()
|
||||
.and_then(ServiceTier::from_request_value),
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
approval_policy: turn_context.approval_policy(),
|
||||
approvals_reviewer: turn_context.config.approvals_reviewer,
|
||||
sandbox_network_access: turn_context.network_sandbox_policy().is_enabled(),
|
||||
collaboration_mode: turn_context.mode,
|
||||
@@ -2161,7 +2161,7 @@ async fn try_run_sampling_request(
|
||||
) -> CodexResult<SamplingRequestResult> {
|
||||
feedback_tags!(
|
||||
model = turn_context.model_info.slug.clone(),
|
||||
approval_policy = turn_context.approval_policy.value(),
|
||||
approval_policy = turn_context.approval_policy(),
|
||||
sandbox_policy = &turn_context.sandbox_policy(),
|
||||
effort = turn_context.reasoning_effort,
|
||||
auth_mode = sess.services.auth_manager.auth_mode(),
|
||||
|
||||
@@ -136,7 +136,6 @@ pub struct TurnContext {
|
||||
pub(crate) collaboration_mode_developer_instructions: Option<String>,
|
||||
pub(crate) multi_agent_version: MultiAgentVersion,
|
||||
pub(crate) personality: Option<Personality>,
|
||||
pub(crate) approval_policy: Constrained<AskForApproval>,
|
||||
pub(crate) permission_profile: PermissionProfile,
|
||||
pub(crate) network: Option<NetworkProxy>,
|
||||
pub(crate) windows_sandbox_level: WindowsSandboxLevel,
|
||||
@@ -186,6 +185,10 @@ impl TurnContext {
|
||||
.resolve_attribution(command, cwd)
|
||||
}
|
||||
|
||||
pub(crate) fn approval_policy(&self) -> AskForApproval {
|
||||
self.config.permissions.approval_policy.value()
|
||||
}
|
||||
|
||||
pub(crate) fn permission_profile(&self) -> PermissionProfile {
|
||||
self.permission_profile.clone()
|
||||
}
|
||||
@@ -310,7 +313,6 @@ impl TurnContext {
|
||||
.clone(),
|
||||
multi_agent_version: self.multi_agent_version,
|
||||
personality: self.personality,
|
||||
approval_policy: self.approval_policy.clone(),
|
||||
permission_profile: self.permission_profile.clone(),
|
||||
network: self.network.clone(),
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
@@ -380,7 +382,7 @@ impl TurnContext {
|
||||
workspace_roots: (!workspace_roots.is_empty()).then_some(workspace_roots),
|
||||
current_date: self.current_date.clone(),
|
||||
timezone: self.timezone.clone(),
|
||||
approval_policy: self.approval_policy.value(),
|
||||
approval_policy: self.approval_policy(),
|
||||
approvals_reviewer: Some(self.config.approvals_reviewer),
|
||||
sandbox_policy: self.sandbox_policy(),
|
||||
permission_profile: Some(self.permission_profile()),
|
||||
@@ -579,7 +581,6 @@ impl Session {
|
||||
.clone(),
|
||||
multi_agent_version,
|
||||
personality: session_configuration.personality,
|
||||
approval_policy: session_configuration.approval_policy.clone(),
|
||||
permission_profile,
|
||||
network,
|
||||
windows_sandbox_level: session_configuration.windows_sandbox_level,
|
||||
|
||||
@@ -118,7 +118,7 @@ impl Session {
|
||||
let exec_policy = self.services.exec_policy.current();
|
||||
world_state.add_section(PermissionsState::new(
|
||||
&permission_profile,
|
||||
turn_context.approval_policy.value(),
|
||||
turn_context.approval_policy(),
|
||||
ApprovalPromptContext::new(
|
||||
turn_context.config.approvals_reviewer,
|
||||
model_messages.and_then(|messages| messages.approvals.as_ref()),
|
||||
|
||||
@@ -572,7 +572,7 @@ async fn execute_verified_patch(
|
||||
&request,
|
||||
&tool_ctx,
|
||||
tool_ctx.turn.as_ref(),
|
||||
tool_ctx.turn.approval_policy.value(),
|
||||
tool_ctx.turn.approval_policy(),
|
||||
)
|
||||
.await
|
||||
.map(|result| result.output);
|
||||
|
||||
@@ -237,7 +237,7 @@ pub(crate) fn apply_spawn_agent_runtime_overrides(
|
||||
config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(turn.approval_policy.value())
|
||||
.set(turn.approval_policy())
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!("approval_policy is invalid: {err}"))
|
||||
})?;
|
||||
|
||||
@@ -273,9 +273,6 @@ async fn spawn_agent_uses_explorer_role_and_preserves_approval_policy() {
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy should be set");
|
||||
turn.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy should be set");
|
||||
turn.provider = create_model_provider(provider_info, turn.auth_manager.clone());
|
||||
turn.config = Arc::new(config);
|
||||
|
||||
@@ -2258,7 +2255,9 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
|
||||
&expected_file_system_sandbox_policy,
|
||||
expected_network_sandbox_policy,
|
||||
);
|
||||
turn.approval_policy
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy should be set");
|
||||
let mut config = (*turn.config).clone();
|
||||
@@ -4440,7 +4439,9 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
|
||||
network_sandbox_policy,
|
||||
);
|
||||
turn.permission_profile = permission_profile.clone();
|
||||
turn.approval_policy
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy set");
|
||||
|
||||
@@ -4474,7 +4475,9 @@ async fn build_agent_resume_config_clears_base_instructions() {
|
||||
let mut base_config = (*turn.config).clone();
|
||||
base_config.base_instructions = Some("caller-base".to_string());
|
||||
turn.config = Arc::new(base_config);
|
||||
turn.approval_policy
|
||||
Arc::make_mut(&mut turn.config)
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("approval policy set");
|
||||
|
||||
|
||||
@@ -108,7 +108,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
|| {
|
||||
normalize_and_validate_additional_permissions(
|
||||
additional_permissions_allowed,
|
||||
turn.approval_policy.value(),
|
||||
turn.approval_policy(),
|
||||
effective_additional_permissions.sandbox_permissions,
|
||||
effective_additional_permissions.additional_permissions,
|
||||
effective_additional_permissions.permissions_preapproved,
|
||||
@@ -127,11 +127,11 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
.requests_sandbox_override()
|
||||
&& !effective_additional_permissions.permissions_preapproved
|
||||
&& !matches!(
|
||||
turn.approval_policy.value(),
|
||||
turn.approval_policy(),
|
||||
codex_protocol::protocol::AskForApproval::OnRequest
|
||||
)
|
||||
{
|
||||
let approval_policy = turn.approval_policy.value();
|
||||
let approval_policy = turn.approval_policy();
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"approval policy is {approval_policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {approval_policy:?}"
|
||||
)));
|
||||
@@ -177,7 +177,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
approval_policy: turn.approval_policy(),
|
||||
permission_profile: turn.permission_profile(),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
@@ -217,13 +217,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
tool_name,
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(
|
||||
&mut runtime,
|
||||
&req,
|
||||
&tool_ctx,
|
||||
&turn,
|
||||
turn.approval_policy.value(),
|
||||
)
|
||||
.run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy())
|
||||
.await
|
||||
.map(|result| result.output);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
|
||||
@@ -277,11 +277,11 @@ impl ExecCommandHandler {
|
||||
.requests_sandbox_override()
|
||||
&& !effective_additional_permissions.permissions_preapproved
|
||||
&& !matches!(
|
||||
context.turn.approval_policy.value(),
|
||||
context.turn.approval_policy(),
|
||||
codex_protocol::protocol::AskForApproval::OnRequest
|
||||
)
|
||||
{
|
||||
let approval_policy = context.turn.approval_policy.value();
|
||||
let approval_policy = context.turn.approval_policy();
|
||||
manager.release_process_id(process_id).await;
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
|
||||
@@ -297,7 +297,7 @@ impl ExecCommandHandler {
|
||||
|| {
|
||||
normalize_and_validate_additional_permissions(
|
||||
additional_permissions_allowed,
|
||||
context.turn.approval_policy.value(),
|
||||
context.turn.approval_policy(),
|
||||
effective_additional_permissions.sandbox_permissions,
|
||||
effective_additional_permissions.additional_permissions,
|
||||
effective_additional_permissions.permissions_preapproved,
|
||||
|
||||
@@ -722,7 +722,7 @@ impl NetworkApprovalService {
|
||||
pending_owner.complete(PendingApprovalDecision::Deny);
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
}
|
||||
if !allows_network_approval_flow(turn_context.approval_policy.value()) {
|
||||
if !allows_network_approval_flow(turn_context.approval_policy()) {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
|
||||
@@ -233,7 +233,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
call_id: ctx.call_id.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: ctx.turn.approval_policy.value(),
|
||||
approval_policy: ctx.turn.approval_policy(),
|
||||
permission_profile: command_executor.permission_profile.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
approval_sandbox_permissions,
|
||||
@@ -314,7 +314,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
call_id: ctx.call_id.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
tool_name: GuardianCommandSource::UnifiedExec,
|
||||
approval_policy: ctx.turn.approval_policy.value(),
|
||||
approval_policy: ctx.turn.approval_policy(),
|
||||
permission_profile: exec_request.permission_profile.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
approval_sandbox_permissions: approval_sandbox_permissions(
|
||||
|
||||
@@ -547,7 +547,9 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul
|
||||
..HooksConfig::default()
|
||||
})));
|
||||
|
||||
turn_context.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
Arc::make_mut(&mut turn_context.config)
|
||||
.permissions
|
||||
.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
turn_context.permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&read_only_file_system_sandbox_policy(),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
|
||||
@@ -1192,7 +1192,7 @@ impl UnifiedExecProcessManager {
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
approval_policy: context.turn.approval_policy(),
|
||||
permission_profile: context.turn.permission_profile(),
|
||||
windows_sandbox_level: context.turn.windows_sandbox_level,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
@@ -1241,7 +1241,7 @@ impl UnifiedExecProcessManager {
|
||||
&req,
|
||||
&tool_ctx,
|
||||
&context.turn,
|
||||
context.turn.approval_policy.value(),
|
||||
context.turn.approval_policy(),
|
||||
)
|
||||
.await
|
||||
.map(|result| (result.output, result.deferred_network_approval))
|
||||
|
||||
@@ -7,6 +7,7 @@ use std::time::Instant;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -16,6 +17,7 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use core_test_support::assert_regex_match;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
@@ -30,6 +32,7 @@ use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::responses::strip_response_item_ids_from_json;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_sandbox;
|
||||
use core_test_support::submit_thread_settings;
|
||||
use core_test_support::test_codex::local;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use regex_lite::Regex;
|
||||
@@ -383,8 +386,21 @@ async fn shell_command_escalated_permissions_rejected_then_ok() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_model("test-gpt-5-codex");
|
||||
let mut builder = test_codex()
|
||||
.with_model("test-gpt-5-codex")
|
||||
.with_config(|config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
submit_thread_settings(
|
||||
&test.codex,
|
||||
ThreadSettingsOverrides {
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
permission_profile: Some(PermissionProfile::Disabled),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
let command = "echo shell ok";
|
||||
let call_id_blocked = "shell-command-blocked";
|
||||
@@ -437,12 +453,8 @@ async fn shell_command_escalated_permissions_rejected_then_ok() -> Result<()> {
|
||||
)
|
||||
.await;
|
||||
|
||||
test.submit_turn_with_approval_and_permission_profile(
|
||||
"run the shell_command script",
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
test.submit_text_turn("run the shell_command script")
|
||||
.await?;
|
||||
|
||||
let policy = AskForApproval::Never;
|
||||
let expected_message = format!(
|
||||
|
||||
Reference in New Issue
Block a user