openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-05 20:10:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): preserve browser network access in auto review

Browse Source
This commit is contained in:
Felipe Coury
2026-07-07 09:29:46 -03:00
parent 798e0a2311
commit 79034a9e01
11 changed files with 187 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app/config_persistence.rs
+35 -24
View File
@@ -431,9 +431,9 @@ impl App {
}
if feature == Feature::GuardianApproval && effective_enabled {
// The feature flag alone is not enough for the live session.
// We also align approval policy + sandbox to the Auto-review
// preset so enabling the experiment immediately
// makes guardian review observable in the current thread.
// We also align the approval policy with Auto-review. Preserve
// an existing workspace-shaped profile so enabling the reviewer
// does not discard configured roots or network access.
if !self.try_set_approval_policy_on_config(
&mut feature_config,
auto_review_preset.approval_policy,
@@ -442,30 +442,34 @@ impl App {
) {
continue;
}
let Some(permission_profile) = self
.try_set_builtin_active_permission_profile_on_config(
&mut feature_config,
auto_review_preset.active_permission_profile.clone(),
"Failed to enable Approve for me",
"failed to set auto-review permission profile on staged config",
)
else {
continue;
};
feature_edits.extend([
crate::config_update::replace_config_value(
"approval_policy",
serde_json::json!("on-request"),
),
crate::config_update::replace_config_value(
feature_edits.push(crate::config_update::replace_config_value(
"approval_policy",
serde_json::json!("on-request"),
));
let preserve_workspace_profile = ChatWidget::workspace_profile_matches_auto_mode(
feature_config.permissions.permission_profile(),
feature_config.cwd.as_path(),
);
if !preserve_workspace_profile {
let Some(permission_profile) = self
.try_set_builtin_active_permission_profile_on_config(
&mut feature_config,
auto_review_preset.active_permission_profile.clone(),
"Failed to enable Approve for me",
"failed to set auto-review permission profile on staged config",
)
else {
continue;
};
feature_edits.push(crate::config_update::replace_config_value(
"sandbox_mode",
serde_json::json!("workspace-write"),
),
]);
));
permission_profile_override = Some(permission_profile);
active_permission_profile_override =
Some(auto_review_preset.active_permission_profile.clone());
}
approval_policy_override = Some(auto_review_preset.approval_policy);
permission_profile_override = Some(permission_profile);
active_permission_profile_override =
Some(auto_review_preset.active_permission_profile.clone());
}
next_config = feature_config;
feature_updates_to_apply.push((feature, effective_enabled));
@@ -882,6 +886,13 @@ impl App {
return;
}
if ChatWidget::workspace_profile_matches_auto_mode(
self.config.permissions.permission_profile(),
self.config.cwd.as_path(),
) {
return;
}
let auto_review_preset = auto_review_mode();
let mut config = self.config.clone();
let Some(permission_profile) = self.try_set_builtin_active_permission_profile_on_config(
codex-rs/tui/src/app/tests.rs
+59
View File
@@ -95,6 +95,7 @@ use codex_protocol::models::ActivePermissionProfile;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::user_input::TextElement;
@@ -2062,6 +2063,64 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result<
Ok(())
}
#[tokio::test]
async fn terminal_browser_enabling_guardian_preserves_network_enabled_workspace_profile()
-> Result<()> {
let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await;
let codex_home = tempdir()?;
app.config.codex_home = codex_home.path().to_path_buf().abs();
let permission_profile = PermissionProfile::workspace_write_with(
&[],
NetworkSandboxPolicy::Enabled,
/*exclude_tmpdir_env_var*/ false,
/*exclude_slash_tmp*/ false,
);
app.config
.permissions
.set_permission_profile(permission_profile.clone())?;
app.chat_widget
.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::legacy(
permission_profile.clone(),
))?;
let mut app_server = start_config_write_test_app_server(&app).await?;
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, true)])
.await;
assert_eq!(
app.config.permissions.permission_profile(),
&permission_profile
);
assert_eq!(
app.chat_widget
.config_ref()
.permissions
.permission_profile(),
&permission_profile
);
assert_eq!(
op_rx.try_recv(),
Ok(Op::OverrideTurnContext {
cwd: None,
approval_policy: Some(AskForApproval::OnRequest),
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
permission_profile: None,
active_permission_profile: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
})
);
let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
assert!(!config.contains("sandbox_mode ="));
app_server.shutdown().await?;
Ok(())
}
#[tokio::test]
async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restores_default()
-> Result<()> {
codex-rs/tui/src/app_server_session.rs
+3 -5
View File
@@ -14,7 +14,6 @@ use crate::session_state::SessionNetworkProxyRuntime;
use crate::session_state::ThreadSessionState;
use crate::status::StatusAccountDisplay;
use crate::status::plan_type_display_name;
use crate::terminal_browser::TerminalBrowserNetworkAvailability;
use crate::terminal_browser::dynamic_tool_specs;
use crate::terminal_visualization_instructions::with_terminal_visualization_instructions;
use codex_app_server_client::AppServerClient;
@@ -1411,8 +1410,7 @@ fn thread_start_params_from_config(
) -> ThreadStartParams {
let dynamic_tools = (matches!(thread_params_mode, ThreadParamsMode::Embedded)
&& cfg!(any(target_os = "macos", target_os = "linux"))
&& config.features.enabled(Feature::TerminalBrowser)
&& TerminalBrowserNetworkAvailability::dynamic_tools_supported(config))
&& config.features.enabled(Feature::TerminalBrowser))
.then(dynamic_tool_specs);
let permissions = permissions_selection_from_config(config, thread_params_mode);
let sandbox = permissions
@@ -1968,7 +1966,7 @@ mod tests {
#[cfg(any(target_os = "macos", target_os = "linux"))]
#[tokio::test]
async fn terminal_browser_dynamic_tools_allow_non_mitm_managed_network() {
async fn terminal_browser_dynamic_tools_stay_visible_when_network_is_restricted() {
let direct_config = build_terminal_browser_config(/*managed_network*/ false).await;
let managed_config = build_terminal_browser_config(/*managed_network*/ true).await;
assert_eq!(
@@ -2016,7 +2014,7 @@ mod tests {
"network restricted",
&restricted_config,
ThreadParamsMode::Embedded,
false,
true,
),
(
"managed network",
codex-rs/tui/src/chatwidget.rs
+9
View File
@@ -480,6 +480,15 @@ const DEFAULT_OPENAI_BASE_URL: &str = "https://api.openai.com/v1";
const DEFAULT_STATUS_LINE_ITEMS: [&str; 2] = ["model-with-reasoning", "current-dir"];
const MAX_AGENT_COPY_HISTORY: usize = 32;
#[derive(Clone)]
enum PermissionPresetProfileUpdate {
Preserve,
Replace {
permission_profile: PermissionProfile,
active_permission_profile: ActivePermissionProfile,
},
}
/// Common initialization parameters shared by all `ChatWidget` constructors.
pub(crate) struct ChatWidgetInit {
pub(crate) config: Config,
codex-rs/tui/src/chatwidget/permission_popups.rs
+54 -23
View File
@@ -240,18 +240,27 @@ impl ChatWidget {
pub(super) fn approval_preset_actions(
approval: AskForApproval,
permission_profile: PermissionProfile,
active_permission_profile: ActivePermissionProfile,
profile_update: PermissionPresetProfileUpdate,
label: String,
approvals_reviewer: ApprovalsReviewer,
) -> Vec<SelectionAction> {
vec![Box::new(move |tx| {
let (permission_profile, active_permission_profile) = match &profile_update {
PermissionPresetProfileUpdate::Preserve => (None, None),
PermissionPresetProfileUpdate::Replace {
permission_profile,
active_permission_profile,
} => (
Some(permission_profile.clone()),
Some(active_permission_profile.clone()),
),
};
tx.send(AppEvent::CodexOp(AppCommand::override_turn_context(
/*cwd*/ None,
Some(approval),
Some(approvals_reviewer),
Some(permission_profile.clone()),
Some(active_permission_profile.clone()),
permission_profile,
active_permission_profile,
/*windows_sandbox_level*/ None,
/*model*/ None,
/*effort*/ None,
@@ -261,9 +270,15 @@ impl ChatWidget {
/*personality*/ None,
)));
tx.send(AppEvent::UpdateAskForApprovalPolicy(approval));
tx.send(AppEvent::UpdateActivePermissionProfile(
active_permission_profile.clone(),
));
if let PermissionPresetProfileUpdate::Replace {
active_permission_profile,
..
} = &profile_update
{
tx.send(AppEvent::UpdateActivePermissionProfile(
active_permission_profile.clone(),
));
}
tx.send(AppEvent::UpdateApprovalsReviewer(approvals_reviewer));
tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_info_event(
@@ -290,13 +305,24 @@ impl ChatWidget {
profile_selection: Option<PermissionProfileSelection>,
return_to_permissions: bool,
) -> Vec<SelectionAction> {
let preserve_workspace_profile = preset.id == "auto"
&& Self::workspace_profile_matches_auto_mode(
self.config.permissions.permission_profile(),
self.config.cwd.as_path(),
);
let apply_actions = || {
profile_selection.clone().map_or_else(
|| {
Self::approval_preset_actions(
AskForApproval::from(preset.approval),
preset.permission_profile.clone(),
preset.active_permission_profile.clone(),
if preserve_workspace_profile {
PermissionPresetProfileUpdate::Preserve
} else {
PermissionPresetProfileUpdate::Replace {
permission_profile: preset.permission_profile.clone(),
active_permission_profile: preset.active_permission_profile.clone(),
}
},
label.clone(),
approvals_reviewer,
)
@@ -390,20 +416,21 @@ impl ChatWidget {
&& current_permission_profile.network_sandbox_policy()
== preset.permission_profile.network_sandbox_policy()
}
"auto" => {
let file_system_policy = current_permission_profile.file_system_sandbox_policy();
matches!(
current_permission_profile,
PermissionProfile::Managed { .. }
) && file_system_policy.can_write_path_with_cwd(cwd, cwd)
&& !file_system_policy.has_full_disk_write_access()
&& current_permission_profile.network_sandbox_policy()
== preset.permission_profile.network_sandbox_policy()
}
"auto" => Self::workspace_profile_matches_auto_mode(current_permission_profile, cwd),
_ => current_permission_profile == &preset.permission_profile,
}
}
pub(crate) fn workspace_profile_matches_auto_mode(
permission_profile: &PermissionProfile,
cwd: &std::path::Path,
) -> bool {
let file_system_policy = permission_profile.file_system_sandbox_policy();
matches!(permission_profile, PermissionProfile::Managed { .. })
&& file_system_policy.can_write_path_with_cwd(cwd, cwd)
&& !file_system_policy.has_full_disk_write_access()
}
pub(crate) fn open_full_access_confirmation(
&mut self,
preset: ApprovalPreset,
@@ -430,8 +457,10 @@ impl ChatWidget {
|| {
Self::approval_preset_actions(
approval,
preset.permission_profile.clone(),
preset.active_permission_profile.clone(),
PermissionPresetProfileUpdate::Replace {
permission_profile: preset.permission_profile.clone(),
active_permission_profile: preset.active_permission_profile.clone(),
},
selected_name.clone(),
ApprovalsReviewer::User,
)
@@ -446,8 +475,10 @@ impl ChatWidget {
|| {
Self::approval_preset_actions(
approval,
preset.permission_profile,
preset.active_permission_profile,
PermissionPresetProfileUpdate::Replace {
permission_profile: preset.permission_profile,
active_permission_profile: preset.active_permission_profile,
},
selected_name,
ApprovalsReviewer::User,
)
codex-rs/tui/src/chatwidget/tests.rs
-1
View File
@@ -144,7 +144,6 @@ pub(super) use codex_protocol::config_types::Personality;
pub(super) use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE;
pub(super) use codex_protocol::config_types::ServiceTier;
pub(super) use codex_protocol::models::ActivePermissionProfile;
pub(super) use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
pub(super) use codex_protocol::models::FileSystemPermissions;
pub(super) use codex_protocol::models::MessagePhase;
pub(super) use codex_protocol::models::NetworkPermissions;
codex-rs/tui/src/chatwidget/tests/permissions.rs
+17 -17
View File
@@ -1061,7 +1061,7 @@ async fn permissions_selection_can_disable_auto_review() {
}
#[tokio::test]
async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context() {
async fn terminal_browser_auto_review_preserves_network_enabled_workspace_profile() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
#[cfg(target_os = "windows")]
{
@@ -1075,9 +1075,15 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context
.approval_policy
.set(AskForApproval::OnRequest.to_core())
.expect("set approval policy");
let permission_profile = PermissionProfile::workspace_write_with(
&[],
NetworkSandboxPolicy::Enabled,
/*exclude_tmpdir_env_var*/ false,
/*exclude_slash_tmp*/ false,
);
chat.config
.permissions
.set_permission_profile(PermissionProfile::workspace_write())
.set_permission_profile(permission_profile.clone())
.expect("set permission profile");
chat.set_approvals_reviewer(ApprovalsReviewer::User);
@@ -1113,10 +1119,8 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context
cwd: None,
approval_policy: Some(AskForApproval::OnRequest),
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
permission_profile: Some(PermissionProfile::workspace_write()),
active_permission_profile: Some(ActivePermissionProfile::new(
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
)),
permission_profile: None,
active_permission_profile: None,
windows_sandbox_level: None,
model: None,
effort: None,
@@ -1127,18 +1131,14 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context
}
);
let active_permission_profile_update = std::iter::from_fn(|| rx.try_recv().ok())
.find_map(|event| match event {
AppEvent::UpdateActivePermissionProfile(active_permission_profile) => {
Some(active_permission_profile)
}
_ => None,
})
.expect("expected UpdateActivePermissionProfile event");
assert_eq!(
active_permission_profile_update,
ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE)
chat.config.permissions.permission_profile(),
&permission_profile
);
assert!(
std::iter::from_fn(|| rx.try_recv().ok())
.all(|event| !matches!(event, AppEvent::UpdateActivePermissionProfile(_))),
"reviewer-only changes must not replace the effective workspace profile"
);
}
codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs
+8 -4
View File
@@ -153,8 +153,10 @@ impl ChatWidget {
) {
accept_actions.extend(Self::approval_preset_actions(
approval,
permission_profile,
active_permission_profile,
PermissionPresetProfileUpdate::Replace {
permission_profile,
active_permission_profile,
},
mode_label.to_string(),
ApprovalsReviewer::User,
));
@@ -173,8 +175,10 @@ impl ChatWidget {
{
accept_and_remember_actions.extend(Self::approval_preset_actions(
approval,
permission_profile,
active_permission_profile,
PermissionPresetProfileUpdate::Replace {
permission_profile,
active_permission_profile,
},
mode_label.to_string(),
ApprovalsReviewer::User,
));
codex-rs/tui/src/terminal_browser/network.rs
-9
View File
@@ -44,15 +44,6 @@ impl TerminalBrowserNetworkAvailability {
}
}
pub(crate) fn dynamic_tools_supported(config: &Config) -> bool {
config.permissions.network_sandbox_policy().is_enabled()
&& config
.permissions
.network
.as_ref()
.is_none_or(|network| !network.mitm_enabled())
}
pub(crate) fn unavailable_message(self) -> Option<&'static str> {
match self {
Self::Direct | Self::ManagedProxy { .. } => None,
codex-rs/tui/src/terminal_browser/network_tests.rs
-6
View File
@@ -53,18 +53,12 @@ async fn direct_network_does_not_require_runtime_proxy_details() {
),
TerminalBrowserNetworkAvailability::Direct
);
assert!(TerminalBrowserNetworkAvailability::dynamic_tools_supported(
&config
));
}
#[tokio::test]
async fn managed_network_requires_a_non_mitm_loopback_runtime_proxy() {
let config = terminal_browser_config(/*managed_network*/ true).await;
assert!(config.permissions.network.is_some());
assert!(TerminalBrowserNetworkAvailability::dynamic_tools_supported(
&config
));
assert_eq!(
TerminalBrowserNetworkAvailability::from_config_and_runtime(
codex-rs/tui/src/terminal_browser/tools.rs
+2 -1
View File
@@ -13,7 +13,8 @@ pub(crate) const TERMINAL_BROWSER_NAMESPACE: &str = "terminal_browser";
pub(crate) fn dynamic_tool_specs() -> Vec<DynamicToolSpec> {
vec![DynamicToolSpec::Namespace(DynamicToolNamespaceSpec {
name: TERMINAL_BROWSER_NAMESPACE.to_string(),
description: "Control a browser rendered inside the terminal.".to_string(),
description: "Control the browser rendered inside the terminal. Use these tools when the user asks for the terminal browser; do not substitute shell text browsers."
.to_string(),
tools: vec![
tool(
"open",