mirror of
https://github.com/openai/codex.git
synced 2026-11-11 22:33:20 +00:00
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
This commit is contained in:
1 parent
1c9f890c05
commit
d7ba5ff955
6 files changed
+23
-84
No files matched your search
@@ -1832,9 +1832,6 @@ impl App {
|
|||||||
AppEvent::SkipNextWorldWritableScan => {
|
AppEvent::SkipNextWorldWritableScan => {
|
||||||
self.windows_sandbox.skip_world_writable_scan_once = true;
|
self.windows_sandbox.skip_world_writable_scan_once = true;
|
||||||
}
|
}
|
||||||
AppEvent::UpdateFullAccessWarningAcknowledged(ack) => {
|
|
||||||
self.chat_widget.set_full_access_warning_acknowledged(ack);
|
|
||||||
}
|
|
||||||
AppEvent::UpdateWorldWritableWarningAcknowledged(ack) => {
|
AppEvent::UpdateWorldWritableWarningAcknowledged(ack) => {
|
||||||
self.chat_widget
|
self.chat_widget
|
||||||
.set_world_writable_warning_acknowledged(ack);
|
.set_world_writable_warning_acknowledged(ack);
|
||||||
@@ -1847,21 +1844,6 @@ impl App {
|
|||||||
self.sync_active_thread_plan_mode_reasoning_setting(app_server)
|
self.sync_active_thread_plan_mode_reasoning_setting(app_server)
|
||||||
.await;
|
.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 => {
|
AppEvent::PersistWorldWritableWarningAcknowledged => {
|
||||||
if let Err(err) = ConfigEditsBuilder::for_config(&self.config)
|
if let Err(err) = ConfigEditsBuilder::for_config(&self.config)
|
||||||
.set_hide_world_writable_warning(/*acknowledged*/ true)
|
.set_hide_world_writable_warning(/*acknowledged*/ true)
|
||||||
|
|||||||
@@ -864,9 +864,6 @@ pub(crate) enum AppEvent {
|
|||||||
/// Clear all persisted local memory artifacts via the app-server.
|
/// Clear all persisted local memory artifacts via the app-server.
|
||||||
ResetMemories,
|
ResetMemories,
|
||||||
|
|
||||||
/// Update whether the full access warning prompt has been acknowledged.
|
|
||||||
UpdateFullAccessWarningAcknowledged(bool),
|
|
||||||
|
|
||||||
/// Update whether the world-writable directories warning has been acknowledged.
|
/// Update whether the world-writable directories warning has been acknowledged.
|
||||||
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
||||||
UpdateWorldWritableWarningAcknowledged(bool),
|
UpdateWorldWritableWarningAcknowledged(bool),
|
||||||
@@ -877,9 +874,6 @@ pub(crate) enum AppEvent {
|
|||||||
/// Update the Plan-mode-specific reasoning effort in memory.
|
/// Update the Plan-mode-specific reasoning effort in memory.
|
||||||
UpdatePlanModeReasoningEffort(Option<ReasoningEffort>),
|
UpdatePlanModeReasoningEffort(Option<ReasoningEffort>),
|
||||||
|
|
||||||
/// Persist the acknowledgement flag for the full access warning prompt.
|
|
||||||
PersistFullAccessWarningAcknowledged,
|
|
||||||
|
|
||||||
/// Persist the acknowledgement flag for the world-writable directories warning.
|
/// Persist the acknowledgement flag for the world-writable directories warning.
|
||||||
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
||||||
PersistWorldWritableWarningAcknowledged,
|
PersistWorldWritableWarningAcknowledged,
|
||||||
|
|||||||
@@ -304,13 +304,8 @@ impl ChatWidget {
|
|||||||
Self::permission_profile_selection_actions,
|
Self::permission_profile_selection_actions,
|
||||||
)
|
)
|
||||||
};
|
};
|
||||||
let requires_confirmation = approvals_reviewer == ApprovalsReviewer::User
|
let requires_confirmation =
|
||||||
&& preset.id == "full-access"
|
approvals_reviewer == ApprovalsReviewer::User && preset.id == "full-access";
|
||||||
&& !self
|
|
||||||
.config
|
|
||||||
.notices
|
|
||||||
.hide_full_access_warning
|
|
||||||
.unwrap_or(false);
|
|
||||||
if requires_confirmation {
|
if requires_confirmation {
|
||||||
let preset = preset.clone();
|
let preset = preset.clone();
|
||||||
return vec![Box::new(move |tx| {
|
return vec![Box::new(move |tx| {
|
||||||
@@ -426,23 +421,7 @@ impl ChatWidget {
|
|||||||
));
|
));
|
||||||
let header = ColumnRenderable::with(header_children);
|
let header = ColumnRenderable::with(header_children);
|
||||||
|
|
||||||
let mut accept_actions = profile_selection.clone().map_or_else(
|
let accept_actions = profile_selection.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(
|
|
||||||
|| {
|
|| {
|
||||||
Self::approval_preset_actions(
|
Self::approval_preset_actions(
|
||||||
approval,
|
approval,
|
||||||
@@ -454,10 +433,6 @@ impl ChatWidget {
|
|||||||
},
|
},
|
||||||
Self::permission_profile_selection_actions,
|
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<SelectionAction> = vec![Box::new(move |tx| {
|
let deny_actions: Vec<SelectionAction> = vec![Box::new(move |tx| {
|
||||||
if return_to_permissions {
|
if return_to_permissions {
|
||||||
@@ -475,13 +450,6 @@ impl ChatWidget {
|
|||||||
dismiss_on_select: true,
|
dismiss_on_select: true,
|
||||||
..Default::default()
|
..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 {
|
SelectionItem {
|
||||||
name: "Cancel".to_string(),
|
name: "Cancel".to_string(),
|
||||||
description: Some("Go back without enabling full access".to_string()),
|
description: Some("Go back without enabling full access".to_string()),
|
||||||
|
|||||||
@@ -122,10 +122,6 @@ impl ChatWidget {
|
|||||||
self.refresh_status_surfaces();
|
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) {
|
pub(crate) fn set_world_writable_warning_acknowledged(&mut self, acknowledged: bool) {
|
||||||
self.config.notices.hide_world_writable_warning = Some(acknowledged);
|
self.config.notices.hide_world_writable_warning = Some(acknowledged);
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-3
@@ -8,8 +8,7 @@ expression: popup
|
|||||||
enabling full access. This significantly increases the risk of data loss,
|
enabling full access. This significantly increases the risk of data loss,
|
||||||
leaks, or unexpected behavior.
|
leaks, or unexpected behavior.
|
||||||
|
|
||||||
› 1. Yes, continue anyway Apply full access for this session
|
› 1. Yes, continue anyway Apply full access for this session
|
||||||
2. Yes, and don't ask again Enable full access and remember this choice
|
2. Cancel Go back without enabling full access
|
||||||
3. Cancel Go back without enabling full access
|
|
||||||
|
|
||||||
Press enter to confirm or esc to go back
|
Press enter to confirm or esc to go back
|
||||||
@@ -77,7 +77,6 @@ async fn approvals_selection_popup_snapshot() {
|
|||||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||||
|
|
||||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
||||||
chat.config.notices.hide_full_access_warning = None;
|
|
||||||
chat.open_approvals_popup();
|
chat.open_approvals_popup();
|
||||||
|
|
||||||
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||||
@@ -260,11 +259,11 @@ async fn profile_permissions_selection_emits_auto_review_mode_event() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[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;
|
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||||
chat.config.explicit_permission_profile_mode = true;
|
chat.config.explicit_permission_profile_mode = true;
|
||||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
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.open_permissions_popup();
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Up));
|
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() {
|
async fn approvals_selection_popup_snapshot_windows_degraded_sandbox() {
|
||||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
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::WindowsSandbox, /*enabled*/ true);
|
||||||
chat.set_feature_enabled(Feature::WindowsSandboxElevated, /*enabled*/ false);
|
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.config.notices.hide_world_writable_warning = Some(true);
|
||||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
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.open_permissions_popup();
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
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_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
||||||
chat.config.notices.hide_full_access_warning = Some(true);
|
|
||||||
|
|
||||||
chat.open_permissions_popup();
|
chat.open_permissions_popup();
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||||
#[cfg(target_os = "windows")]
|
#[cfg(target_os = "windows")]
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
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);
|
let cells = drain_insert_history(&mut rx);
|
||||||
assert_eq!(cells.len(), 1, "expected one mode-switch history cell");
|
assert_eq!(cells.len(), 1, "expected one mode-switch history cell");
|
||||||
assert_chatwidget_snapshot!(
|
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.config.notices.hide_world_writable_warning = Some(true);
|
||||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.config.notices.hide_full_access_warning = Some(true);
|
|
||||||
chat.config
|
chat.config
|
||||||
.permissions
|
.permissions
|
||||||
.approval_policy
|
.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_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
||||||
chat.config.notices.hide_full_access_warning = Some(true);
|
|
||||||
|
|
||||||
chat.open_permissions_popup();
|
chat.open_permissions_popup();
|
||||||
let popup = render_bottom_popup(&chat, /*width*/ 120);
|
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_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
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.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||||
chat.config
|
chat.config
|
||||||
.permissions
|
.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.config.notices.hide_world_writable_warning = Some(true);
|
||||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.config.notices.hide_full_access_warning = Some(true);
|
|
||||||
let _ = chat
|
let _ = chat
|
||||||
.config
|
.config
|
||||||
.features
|
.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.config.notices.hide_world_writable_warning = Some(true);
|
||||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.config.notices.hide_full_access_warning = Some(true);
|
|
||||||
let _ = chat
|
let _ = chat
|
||||||
.config
|
.config
|
||||||
.features
|
.features
|
||||||
@@ -1028,7 +1031,6 @@ async fn permissions_selection_can_disable_auto_review() {
|
|||||||
chat.config.notices.hide_world_writable_warning = Some(true);
|
chat.config.notices.hide_world_writable_warning = Some(true);
|
||||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
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.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true);
|
||||||
chat.config
|
chat.config
|
||||||
.permissions
|
.permissions
|
||||||
@@ -1039,6 +1041,7 @@ async fn permissions_selection_can_disable_auto_review() {
|
|||||||
.permissions
|
.permissions
|
||||||
.set_permission_profile(PermissionProfile::workspace_write())
|
.set_permission_profile(PermissionProfile::workspace_write())
|
||||||
.expect("set permission profile");
|
.expect("set permission profile");
|
||||||
|
chat.set_approvals_reviewer(ApprovalsReviewer::AutoReview);
|
||||||
|
|
||||||
chat.open_permissions_popup();
|
chat.open_permissions_popup();
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Up));
|
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.config.notices.hide_world_writable_warning = Some(true);
|
||||||
chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
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.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true);
|
||||||
chat.config
|
chat.config
|
||||||
.permissions
|
.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_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated));
|
||||||
}
|
}
|
||||||
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
||||||
chat.config.notices.hide_full_access_warning = None;
|
|
||||||
|
|
||||||
chat.open_permissions_popup();
|
chat.open_permissions_popup();
|
||||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||||
#[cfg(target_os = "windows")]
|
#[cfg(target_os = "windows")]
|
||||||
|
|||||||
Reference in new issue
Block a user