From cbc31b9a2ea6e491f98e9b6724933fc734629c47 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Wed, 25 Mar 2026 12:09:29 -0700 Subject: [PATCH] codex: always confirm admin-required full access Co-authored-by: Codex --- codex-rs/tui/src/chatwidget.rs | 55 ++++++++++--------- ...ion_popup_with_required_justification.snap | 5 +- codex-rs/tui/src/chatwidget/tests.rs | 12 ++-- codex-rs/tui_app_server/src/chatwidget.rs | 55 ++++++++++--------- ...ion_popup_with_required_justification.snap | 5 +- .../tui_app_server/src/chatwidget/tests.rs | 12 ++-- 6 files changed, 74 insertions(+), 70 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 2f35d39492..0a86549163 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -7139,11 +7139,12 @@ impl ChatWidget { .clone() .or_else(|| guardian_disabled_reason(false)); let requires_confirmation = preset.id == "full-access" - && !self - .config - .notices - .hide_full_access_warning - .unwrap_or(false); + && (self.config.require_full_access_justification + || !self + .config + .notices + .hide_full_access_warning + .unwrap_or(false)); let default_actions: Vec = if requires_confirmation { let preset_clone = preset.clone(); vec![Box::new(move |tx| { @@ -7502,10 +7503,6 @@ impl ChatWidget { /*persist_warning_acknowledged*/ false, ); - let accept_and_remember_actions = self.full_access_preset_actions( - preset, /*acknowledge_warning*/ true, /*persist_warning_acknowledged*/ true, - ); - let deny_actions: Vec = vec![Box::new(move |tx| { if return_to_permissions { tx.send(AppEvent::OpenPermissionsPopup); @@ -7514,29 +7511,33 @@ impl ChatWidget { } })]; - let items = vec![ - SelectionItem { - name: "Yes, continue anyway".to_string(), - description: Some("Apply full access for this session".to_string()), - actions: accept_actions, - dismiss_on_select: true, - ..Default::default() - }, - SelectionItem { + let mut items = vec![SelectionItem { + name: "Yes, continue anyway".to_string(), + description: Some("Apply full access for this session".to_string()), + actions: accept_actions, + dismiss_on_select: true, + ..Default::default() + }]; + if !self.config.require_full_access_justification { + let accept_and_remember_actions = self.full_access_preset_actions( + preset, /*acknowledge_warning*/ true, + /*persist_warning_acknowledged*/ true, + ); + items.push(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()), - actions: deny_actions, - dismiss_on_select: true, - ..Default::default() - }, - ]; + }); + } + items.push(SelectionItem { + name: "Cancel".to_string(), + description: Some("Go back without enabling full access".to_string()), + actions: deny_actions, + dismiss_on_select: true, + ..Default::default() + }); self.bottom_pane.show_selection_view(SelectionViewParams { footer_hint: Some(standard_popup_hint_line()), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap index 0753b511b1..a23cec0d73 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap @@ -9,8 +9,7 @@ expression: popup leaks, or unexpected behavior. Your admin requires a written justification before full access is enabled. -› 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.rs b/codex-rs/tui/src/chatwidget/tests.rs index c76c0d7aa6..79a67a519c 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -159,11 +159,13 @@ use toml::Value as TomlValue; async fn test_config() -> Config { // Use base defaults to avoid depending on host state. let codex_home = std::env::temp_dir(); - ConfigBuilder::default() + let mut config = ConfigBuilder::default() .codex_home(codex_home.clone()) .build() .await - .expect("config") + .expect("config"); + config.require_full_access_justification = false; + config } fn invalid_value(candidate: impl Into, allowed: impl Into) -> ConstraintError { @@ -9972,7 +9974,7 @@ async fn permissions_full_access_history_cell_emitted_only_after_confirmation() } #[tokio::test] -async fn permissions_full_access_requires_justification_when_warning_hidden() { +async fn permissions_full_access_still_confirms_when_warning_hidden_but_justification_required() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; #[cfg(target_os = "windows")] { @@ -9990,8 +9992,8 @@ async fn permissions_full_access_requires_justification_when_warning_hidden() { assert!( events .iter() - .any(|event| matches!(event, AppEvent::OpenFullAccessJustificationPrompt { .. })), - "expected full access selection to request a justification prompt: {events:?}" + .any(|event| matches!(event, AppEvent::OpenFullAccessConfirmation { .. })), + "expected full access selection to request a confirmation popup: {events:?}" ); assert!( !events diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index c018b66c5c..4ee53ac9e0 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -8319,11 +8319,12 @@ impl ChatWidget { .clone() .or_else(|| guardian_disabled_reason(false)); let requires_confirmation = preset.id == "full-access" - && !self - .config - .notices - .hide_full_access_warning - .unwrap_or(false); + && (self.config.require_full_access_justification + || !self + .config + .notices + .hide_full_access_warning + .unwrap_or(false)); let default_actions: Vec = if requires_confirmation { let preset_clone = preset.clone(); vec![Box::new(move |tx| { @@ -8685,10 +8686,6 @@ impl ChatWidget { /*persist_warning_acknowledged*/ false, ); - let accept_and_remember_actions = self.full_access_preset_actions( - preset, /*acknowledge_warning*/ true, /*persist_warning_acknowledged*/ true, - ); - let deny_actions: Vec = vec![Box::new(move |tx| { if return_to_permissions { tx.send(AppEvent::OpenPermissionsPopup); @@ -8697,29 +8694,33 @@ impl ChatWidget { } })]; - let items = vec![ - SelectionItem { - name: "Yes, continue anyway".to_string(), - description: Some("Apply full access for this session".to_string()), - actions: accept_actions, - dismiss_on_select: true, - ..Default::default() - }, - SelectionItem { + let mut items = vec![SelectionItem { + name: "Yes, continue anyway".to_string(), + description: Some("Apply full access for this session".to_string()), + actions: accept_actions, + dismiss_on_select: true, + ..Default::default() + }]; + if !self.config.require_full_access_justification { + let accept_and_remember_actions = self.full_access_preset_actions( + preset, /*acknowledge_warning*/ true, + /*persist_warning_acknowledged*/ true, + ); + items.push(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()), - actions: deny_actions, - dismiss_on_select: true, - ..Default::default() - }, - ]; + }); + } + items.push(SelectionItem { + name: "Cancel".to_string(), + description: Some("Go back without enabling full access".to_string()), + actions: deny_actions, + dismiss_on_select: true, + ..Default::default() + }); self.bottom_pane.show_selection_view(SelectionViewParams { footer_hint: Some(standard_popup_hint_line()), diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap index 7c65c38d5a..77026269cb 100644 --- a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__full_access_confirmation_popup_with_required_justification.snap @@ -9,8 +9,7 @@ expression: popup leaks, or unexpected behavior. Your admin requires a written justification before full access is enabled. -› 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_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index 5c4d395153..be8b9ce3ff 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -183,11 +183,13 @@ use toml::Value as TomlValue; async fn test_config() -> Config { // Use base defaults to avoid depending on host state. let codex_home = std::env::temp_dir(); - ConfigBuilder::default() + let mut config = ConfigBuilder::default() .codex_home(codex_home.clone()) .build() .await - .expect("config") + .expect("config"); + config.require_full_access_justification = false; + config } fn invalid_value(candidate: impl Into, allowed: impl Into) -> ConstraintError { @@ -10513,7 +10515,7 @@ async fn permissions_full_access_history_cell_emitted_only_after_confirmation() } #[tokio::test] -async fn permissions_full_access_requires_justification_when_warning_hidden() { +async fn permissions_full_access_still_confirms_when_warning_hidden_but_justification_required() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; #[cfg(target_os = "windows")] { @@ -10533,8 +10535,8 @@ async fn permissions_full_access_requires_justification_when_warning_hidden() { assert!( events .iter() - .any(|event| matches!(event, AppEvent::OpenFullAccessJustificationPrompt { .. })), - "expected full access selection to request a justification prompt: {events:?}" + .any(|event| matches!(event, AppEvent::OpenFullAccessConfirmation { .. })), + "expected full access selection to request a confirmation popup: {events:?}" ); assert!( !events