From d7ba5ff9553a6aa0898a8e3bd5cb3bc00d0c9ddf Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Tue, 14 Jul 2026 06:50:10 +0000 Subject: [PATCH] Always confirm before enabling full access (#32989) ## Why Full access significantly increases the risk of data loss, leaks, or unexpected behavior, so users should explicitly confirm each time they select it. ## What changed - Always open the full access confirmation when user-reviewed approvals are active, regardless of `notices.hide_full_access_warning`. - Remove the persistent "don't ask again" option and its acknowledgement events. ## Testing - Update the TUI permission tests and confirmation popup snapshot to cover repeated confirmation. GitOrigin-RevId: 0ee827332441e13d9445349ed6a5a92dc8fef4b2 --- codex-rs/tui/src/app/event_dispatch.rs | 18 --------- codex-rs/tui/src/app_event.rs | 6 --- .../tui/src/chatwidget/permission_popups.rs | 38 ++----------------- codex-rs/tui/src/chatwidget/settings.rs | 4 -- ...tests__full_access_confirmation_popup.snap | 5 +-- .../tui/src/chatwidget/tests/permissions.rs | 36 +++++++++--------- 6 files changed, 23 insertions(+), 84 deletions(-) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index a5d408d4ca..4a2f0305e8 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1832,9 +1832,6 @@ impl App { AppEvent::SkipNextWorldWritableScan => { self.windows_sandbox.skip_world_writable_scan_once = true; } - AppEvent::UpdateFullAccessWarningAcknowledged(ack) => { - self.chat_widget.set_full_access_warning_acknowledged(ack); - } AppEvent::UpdateWorldWritableWarningAcknowledged(ack) => { self.chat_widget .set_world_writable_warning_acknowledged(ack); @@ -1847,21 +1844,6 @@ impl App { self.sync_active_thread_plan_mode_reasoning_setting(app_server) .await; } - AppEvent::PersistFullAccessWarningAcknowledged => { - if let Err(err) = ConfigEditsBuilder::for_config(&self.config) - .set_hide_full_access_warning(/*acknowledged*/ true) - .apply() - .await - { - tracing::error!( - error = %err, - "failed to persist full access warning acknowledgement" - ); - self.chat_widget.add_error_message(format!( - "Failed to save full access confirmation preference: {err}" - )); - } - } AppEvent::PersistWorldWritableWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::for_config(&self.config) .set_hide_world_writable_warning(/*acknowledged*/ true) diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 553a58d8d4..6661ae5af4 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -864,9 +864,6 @@ pub(crate) enum AppEvent { /// Clear all persisted local memory artifacts via the app-server. ResetMemories, - /// Update whether the full access warning prompt has been acknowledged. - UpdateFullAccessWarningAcknowledged(bool), - /// Update whether the world-writable directories warning has been acknowledged. #[cfg_attr(not(target_os = "windows"), allow(dead_code))] UpdateWorldWritableWarningAcknowledged(bool), @@ -877,9 +874,6 @@ pub(crate) enum AppEvent { /// Update the Plan-mode-specific reasoning effort in memory. UpdatePlanModeReasoningEffort(Option), - /// Persist the acknowledgement flag for the full access warning prompt. - PersistFullAccessWarningAcknowledged, - /// Persist the acknowledgement flag for the world-writable directories warning. #[cfg_attr(not(target_os = "windows"), allow(dead_code))] PersistWorldWritableWarningAcknowledged, diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index 1283a9e490..6c667864d3 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -304,13 +304,8 @@ impl ChatWidget { Self::permission_profile_selection_actions, ) }; - let requires_confirmation = approvals_reviewer == ApprovalsReviewer::User - && preset.id == "full-access" - && !self - .config - .notices - .hide_full_access_warning - .unwrap_or(false); + let requires_confirmation = + approvals_reviewer == ApprovalsReviewer::User && preset.id == "full-access"; if requires_confirmation { let preset = preset.clone(); return vec![Box::new(move |tx| { @@ -426,23 +421,7 @@ impl ChatWidget { )); let header = ColumnRenderable::with(header_children); - let mut accept_actions = profile_selection.clone().map_or_else( - || { - Self::approval_preset_actions( - approval, - preset.permission_profile.clone(), - preset.active_permission_profile.clone(), - selected_name.clone(), - ApprovalsReviewer::User, - ) - }, - Self::permission_profile_selection_actions, - ); - accept_actions.push(Box::new(|tx| { - tx.send(AppEvent::UpdateFullAccessWarningAcknowledged(true)); - })); - - let mut accept_and_remember_actions = profile_selection.map_or_else( + let accept_actions = profile_selection.map_or_else( || { Self::approval_preset_actions( approval, @@ -454,10 +433,6 @@ impl ChatWidget { }, Self::permission_profile_selection_actions, ); - accept_and_remember_actions.push(Box::new(|tx| { - tx.send(AppEvent::UpdateFullAccessWarningAcknowledged(true)); - tx.send(AppEvent::PersistFullAccessWarningAcknowledged); - })); let deny_actions: Vec = vec![Box::new(move |tx| { if return_to_permissions { @@ -475,13 +450,6 @@ impl ChatWidget { dismiss_on_select: true, ..Default::default() }, - SelectionItem { - name: "Yes, and don't ask again".to_string(), - description: Some("Enable full access and remember this choice".to_string()), - actions: accept_and_remember_actions, - dismiss_on_select: true, - ..Default::default() - }, SelectionItem { name: "Cancel".to_string(), description: Some("Go back without enabling full access".to_string()), diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index e5688e92f2..48224ea1dd 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -122,10 +122,6 @@ impl ChatWidget { self.refresh_status_surfaces(); } - pub(crate) fn set_full_access_warning_acknowledged(&mut self, acknowledged: bool) { - self.config.notices.hide_full_access_warning = Some(acknowledged); - } - pub(crate) fn set_world_writable_warning_acknowledged(&mut self, acknowledged: bool) { self.config.notices.hide_world_writable_warning = Some(acknowledged); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup.snap index d98ce2b772..c954ceec43 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup.snap @@ -8,8 +8,7 @@ expression: popup enabling full access. This significantly increases the risk of data loss, leaks, or unexpected behavior. -› 1. Yes, continue anyway Apply full access for this session - 2. Yes, and don't ask again Enable full access and remember this choice - 3. Cancel Go back without enabling full access +› 1. Yes, continue anyway Apply full access for this session + 2. Cancel Go back without enabling full access Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index ebac595671..0cdf5d67a5 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -77,7 +77,6 @@ async fn approvals_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); - chat.config.notices.hide_full_access_warning = None; chat.open_approvals_popup(); let popup = render_bottom_popup(&chat, /*width*/ 80); @@ -260,11 +259,11 @@ async fn profile_permissions_selection_emits_auto_review_mode_event() { } #[tokio::test] -async fn profile_permissions_full_access_opens_confirmation() { +async fn profile_permissions_full_access_always_opens_confirmation() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.config.explicit_permission_profile_mode = true; chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); - chat.config.notices.hide_full_access_warning = None; + chat.config.notices.hide_full_access_warning = Some(true); chat.open_permissions_popup(); chat.handle_key_event(KeyEvent::from(KeyCode::Up)); @@ -295,7 +294,6 @@ async fn profile_permissions_full_access_opens_confirmation() { async fn approvals_selection_popup_snapshot_windows_degraded_sandbox() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - chat.config.notices.hide_full_access_warning = None; chat.set_feature_enabled(Feature::WindowsSandbox, /*enabled*/ true); chat.set_feature_enabled(Feature::WindowsSandboxElevated, /*enabled*/ false); @@ -752,8 +750,7 @@ async fn permissions_selection_emits_history_cell_when_selection_changes() { chat.config.notices.hide_world_writable_warning = Some(true); chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } - chat.config.notices.hide_full_access_warning = Some(true); - + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); chat.open_permissions_popup(); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -780,14 +777,26 @@ async fn permissions_selection_history_snapshot_after_mode_switch() { chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); - chat.config.notices.hide_full_access_warning = Some(true); - chat.open_permissions_popup(); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); #[cfg(target_os = "windows")] chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + let (preset, return_to_permissions, profile_selection) = + std::iter::from_fn(|| rx.try_recv().ok()) + .find_map(|event| match event { + AppEvent::OpenFullAccessConfirmation { + preset, + return_to_permissions, + profile_selection, + } => Some((preset, return_to_permissions, profile_selection)), + _ => None, + }) + .expect("expected full access confirmation event"); + chat.open_full_access_confirmation(preset, return_to_permissions, profile_selection); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 1, "expected one mode-switch history cell"); assert_chatwidget_snapshot!( @@ -804,7 +813,6 @@ async fn permissions_selection_history_snapshot_full_access_to_default() { chat.config.notices.hide_world_writable_warning = Some(true); chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } - chat.config.notices.hide_full_access_warning = Some(true); chat.config .permissions .approval_policy @@ -882,8 +890,6 @@ async fn permissions_selection_hides_auto_review_when_feature_disabled() { chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); - chat.config.notices.hide_full_access_warning = Some(true); - chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, /*width*/ 120); @@ -903,7 +909,6 @@ async fn permissions_selection_hides_auto_review_when_feature_disabled_even_if_a chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); - chat.config.notices.hide_full_access_warning = Some(true); chat.config.approvals_reviewer = ApprovalsReviewer::AutoReview; chat.config .permissions @@ -932,7 +937,6 @@ async fn permissions_selection_marks_auto_review_current_after_session_configure chat.config.notices.hide_world_writable_warning = Some(true); chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } - chat.config.notices.hide_full_access_warning = Some(true); let _ = chat .config .features @@ -978,7 +982,6 @@ async fn permissions_selection_marks_auto_review_current_with_custom_workspace_w chat.config.notices.hide_world_writable_warning = Some(true); chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } - chat.config.notices.hide_full_access_warning = Some(true); let _ = chat .config .features @@ -1028,7 +1031,6 @@ async fn permissions_selection_can_disable_auto_review() { chat.config.notices.hide_world_writable_warning = Some(true); chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } - chat.config.notices.hide_full_access_warning = Some(true); chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); chat.config .permissions @@ -1039,6 +1041,7 @@ async fn permissions_selection_can_disable_auto_review() { .permissions .set_permission_profile(PermissionProfile::workspace_write()) .expect("set permission profile"); + chat.set_approvals_reviewer(ApprovalsReviewer::AutoReview); chat.open_permissions_popup(); chat.handle_key_event(KeyEvent::from(KeyCode::Up)); @@ -1068,7 +1071,6 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context chat.config.notices.hide_world_writable_warning = Some(true); chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } - chat.config.notices.hide_full_access_warning = Some(true); chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); chat.config .permissions @@ -1151,8 +1153,6 @@ async fn permissions_full_access_history_cell_emitted_only_after_confirmation() chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); } chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); - chat.config.notices.hide_full_access_warning = None; - chat.open_permissions_popup(); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); #[cfg(target_os = "windows")]