From c086b36b58837ca0b8aa5721d84de8b215ab3de1 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 24 Feb 2026 23:06:19 -0800 Subject: [PATCH] feat(ui): add network approval persistence plumbing (#12358) ## Summary - add TUI approval options for persistent network host rules - add app-server v2 approval payload plumbing for network approval context + proposed network policy amendments - add app-server handling to translate `applyNetworkPolicyAmendment` decisions back into core review decisions - update docs/test client output and generated app-server schemas/types --- ...CommandExecutionRequestApprovalParams.json | 34 +++++- ...mmandExecutionRequestApprovalResponse.json | 44 +++++++ .../schema/json/ServerRequest.json | 34 +++++- .../codex_app_server_protocol.schemas.json | 34 +++++- .../v2/CommandExecutionApprovalDecision.ts | 3 +- .../CommandExecutionRequestApprovalParams.ts | 9 +- .../typescript/v2/NetworkPolicyAmendment.ts | 6 + .../typescript/v2/NetworkPolicyRuleAction.ts | 5 + .../schema/typescript/v2/index.ts | 2 + .../src/protocol/common.rs | 3 + .../app-server-protocol/src/protocol/v2.rs | 44 ++++++- codex-rs/app-server-test-client/src/lib.rs | 4 + codex-rs/app-server/README.md | 6 +- .../app-server/src/bespoke_event_handling.rs | 25 ++++ codex-rs/app-server/src/transport.rs | 2 + codex-rs/core/tests/suite/approvals.rs | 30 ++++- .../tui/src/bottom_pane/approval_overlay.rs | 111 ++++++++++++++++-- codex-rs/tui/src/bottom_pane/mod.rs | 1 + ...l_overlay__tests__network_exec_prompt.snap | 38 ++++++ codex-rs/tui/src/chatwidget.rs | 1 + codex-rs/tui/src/history_cell.rs | 45 ++++--- 21 files changed, 436 insertions(+), 45 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyAmendment.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyRuleAction.ts create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__network_exec_prompt.snap diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index c8329d1b8..2122ad652 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -248,6 +248,28 @@ "socks5Udp" ], "type": "string" + }, + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" } }, "properties": { @@ -294,7 +316,7 @@ "type": "null" } ], - "description": "Optional context for managed-network approval prompts." + "description": "Optional context for a managed-network approval prompt." }, "proposedExecpolicyAmendment": { "description": "Optional proposed execpolicy amendment to allow similar commands without prompting.", @@ -306,6 +328,16 @@ "null" ] }, + "proposedNetworkPolicyAmendments": { + "description": "Optional proposed network policy amendments (allow/deny host) for future requests.", + "items": { + "$ref": "#/definitions/NetworkPolicyAmendment" + }, + "type": [ + "array", + "null" + ] + }, "reason": { "description": "Optional explanatory reason (e.g. request for network access).", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json index fcc3eba78..04e914a39 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json @@ -42,6 +42,28 @@ "title": "AcceptWithExecpolicyAmendmentCommandExecutionApprovalDecision", "type": "object" }, + { + "additionalProperties": false, + "description": "User chose a persistent network policy rule (allow/deny) for this host.", + "properties": { + "applyNetworkPolicyAmendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "applyNetworkPolicyAmendment" + ], + "title": "ApplyNetworkPolicyAmendmentCommandExecutionApprovalDecision", + "type": "object" + }, { "description": "User denied the command. The agent will continue the turn.", "enum": [ @@ -57,6 +79,28 @@ "type": "string" } ] + }, + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" } }, "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index f2f47eaa4..ba94b889c 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -313,7 +313,7 @@ "type": "null" } ], - "description": "Optional context for managed-network approval prompts." + "description": "Optional context for a managed-network approval prompt." }, "proposedExecpolicyAmendment": { "description": "Optional proposed execpolicy amendment to allow similar commands without prompting.", @@ -325,6 +325,16 @@ "null" ] }, + "proposedNetworkPolicyAmendments": { + "description": "Optional proposed network policy amendments (allow/deny host) for future requests.", + "items": { + "$ref": "#/definitions/NetworkPolicyAmendment" + }, + "type": [ + "array", + "null" + ] + }, "reason": { "description": "Optional explanatory reason (e.g. request for network access).", "type": [ @@ -568,6 +578,28 @@ ], "type": "string" }, + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 5aad0b098..563b41c1e 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -1401,6 +1401,28 @@ "title": "AcceptWithExecpolicyAmendmentCommandExecutionApprovalDecision", "type": "object" }, + { + "additionalProperties": false, + "description": "User chose a persistent network policy rule (allow/deny) for this host.", + "properties": { + "applyNetworkPolicyAmendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "applyNetworkPolicyAmendment" + ], + "title": "ApplyNetworkPolicyAmendmentCommandExecutionApprovalDecision", + "type": "object" + }, { "description": "User denied the command. The agent will continue the turn.", "enum": [ @@ -1463,7 +1485,7 @@ "type": "null" } ], - "description": "Optional context for managed-network approval prompts." + "description": "Optional context for a managed-network approval prompt." }, "proposedExecpolicyAmendment": { "description": "Optional proposed execpolicy amendment to allow similar commands without prompting.", @@ -1475,6 +1497,16 @@ "null" ] }, + "proposedNetworkPolicyAmendments": { + "description": "Optional proposed network policy amendments (allow/deny host) for future requests.", + "items": { + "$ref": "#/definitions/NetworkPolicyAmendment" + }, + "type": [ + "array", + "null" + ] + }, "reason": { "description": "Optional explanatory reason (e.g. request for network access).", "type": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalDecision.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalDecision.ts index 80df9bd02..c022030a1 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalDecision.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalDecision.ts @@ -2,5 +2,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; +import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; -export type CommandExecutionApprovalDecision = "accept" | "acceptForSession" | { "acceptWithExecpolicyAmendment": { execpolicy_amendment: ExecPolicyAmendment, } } | "decline" | "cancel"; +export type CommandExecutionApprovalDecision = "accept" | "acceptForSession" | { "acceptWithExecpolicyAmendment": { execpolicy_amendment: ExecPolicyAmendment, } } | { "applyNetworkPolicyAmendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "decline" | "cancel"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts index a7c143c6a..70cb8e152 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -5,6 +5,7 @@ import type { AdditionalPermissionProfile } from "./AdditionalPermissionProfile" import type { CommandAction } from "./CommandAction"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; +import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; export type CommandExecutionRequestApprovalParams = { threadId: string, turnId: string, itemId: string, /** @@ -22,7 +23,7 @@ approvalId?: string | null, */ reason?: string | null, /** - * Optional context for managed-network approval prompts. + * Optional context for a managed-network approval prompt. */ networkApprovalContext?: NetworkApprovalContext | null, /** @@ -44,4 +45,8 @@ additionalPermissions?: AdditionalPermissionProfile | null, /** * Optional proposed execpolicy amendment to allow similar commands without prompting. */ -proposedExecpolicyAmendment?: ExecPolicyAmendment | null, }; +proposedExecpolicyAmendment?: ExecPolicyAmendment | null, +/** + * Optional proposed network policy amendments (allow/deny host) for future requests. + */ +proposedNetworkPolicyAmendments?: Array | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyAmendment.ts b/codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyAmendment.ts new file mode 100644 index 000000000..4e5092e4d --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyAmendment.ts @@ -0,0 +1,6 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { NetworkPolicyRuleAction } from "./NetworkPolicyRuleAction"; + +export type NetworkPolicyAmendment = { host: string, action: NetworkPolicyRuleAction, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyRuleAction.ts b/codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyRuleAction.ts new file mode 100644 index 000000000..55ec70032 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/NetworkPolicyRuleAction.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type NetworkPolicyRuleAction = "allow" | "deny"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index d71980199..1a0b22343 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -106,6 +106,8 @@ export type { ModelReroutedNotification } from "./ModelReroutedNotification"; export type { NetworkAccess } from "./NetworkAccess"; export type { NetworkApprovalContext } from "./NetworkApprovalContext"; export type { NetworkApprovalProtocol } from "./NetworkApprovalProtocol"; +export type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; +export type { NetworkPolicyRuleAction } from "./NetworkPolicyRuleAction"; export type { NetworkRequirements } from "./NetworkRequirements"; export type { OverriddenMetadata } from "./OverriddenMetadata"; export type { PatchApplyStatus } from "./PatchApplyStatus"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 9e1e6f230..b3d3b4ae7 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -501,6 +501,7 @@ macro_rules! server_request_definitions { ) => { /// Request initiated from the server and sent to the client. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] + #[allow(clippy::large_enum_variant)] #[serde(tag = "method", rename_all = "camelCase")] pub enum ServerRequest { $( @@ -515,6 +516,7 @@ macro_rules! server_request_definitions { } #[derive(Debug, Clone, PartialEq, JsonSchema)] + #[allow(clippy::large_enum_variant)] pub enum ServerRequestPayload { $( $variant($params), )* } @@ -1393,6 +1395,7 @@ mod tests { macos: None, }), proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, }; let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms); assert_eq!( diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 41e545068..d17b0772a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -7,6 +7,8 @@ use codex_protocol::account::PlanType; use codex_protocol::approvals::ExecPolicyAmendment as CoreExecPolicyAmendment; use codex_protocol::approvals::NetworkApprovalContext as CoreNetworkApprovalContext; use codex_protocol::approvals::NetworkApprovalProtocol as CoreNetworkApprovalProtocol; +use codex_protocol::approvals::NetworkPolicyAmendment as CoreNetworkPolicyAmendment; +use codex_protocol::approvals::NetworkPolicyRuleAction as CoreNetworkPolicyRuleAction; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ForcedLoginMethod; @@ -686,6 +688,10 @@ pub enum CommandExecutionApprovalDecision { AcceptWithExecpolicyAmendment { execpolicy_amendment: ExecPolicyAmendment, }, + /// User chose a persistent network policy rule (allow/deny) for this host. + ApplyNetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment, + }, /// User denied the command. The agent will continue the turn. Decline, /// User denied the command. The turn will also be immediately interrupted. @@ -972,6 +978,38 @@ impl From for ExecPolicyAmendment { } } +v2_enum_from_core!( + pub enum NetworkPolicyRuleAction from CoreNetworkPolicyRuleAction { + Allow, Deny + } +); + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct NetworkPolicyAmendment { + pub host: String, + pub action: NetworkPolicyRuleAction, +} + +impl NetworkPolicyAmendment { + pub fn into_core(self) -> CoreNetworkPolicyAmendment { + CoreNetworkPolicyAmendment { + host: self.host, + action: self.action.to_core(), + } + } +} + +impl From for NetworkPolicyAmendment { + fn from(value: CoreNetworkPolicyAmendment) -> Self { + Self { + host: value.host, + action: NetworkPolicyRuleAction::from(value.action), + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type")] @@ -3442,7 +3480,7 @@ pub struct CommandExecutionRequestApprovalParams { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub reason: Option, - /// Optional context for managed-network approval prompts. + /// Optional context for a managed-network approval prompt. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub network_approval_context: Option, @@ -3467,6 +3505,10 @@ pub struct CommandExecutionRequestApprovalParams { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub proposed_execpolicy_amendment: Option, + /// Optional proposed network policy amendments (allow/deny host) for future requests. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional = nullable)] + pub proposed_network_policy_amendments: Option>, } impl CommandExecutionRequestApprovalParams { diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index f5a7fa9c4..ae147d5b4 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1538,6 +1538,7 @@ impl CodexClient { command_actions, additional_permissions, proposed_execpolicy_amendment, + proposed_network_policy_amendments, } = params; println!( @@ -1569,6 +1570,9 @@ impl CodexClient { if let Some(execpolicy_amendment) = proposed_execpolicy_amendment.as_ref() { println!("< proposed execpolicy amendment: {execpolicy_amendment:?}"); } + if let Some(network_policy_amendments) = proposed_network_policy_amendments.as_ref() { + println!("< proposed network policy amendments: {network_policy_amendments:?}"); + } let decision = match self.command_approval_behavior { CommandApprovalBehavior::AlwaysAccept => CommandExecutionApprovalDecision::Accept, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 72f686177..df026596f 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -660,15 +660,15 @@ When an upstream HTTP status is available (for example, from the Responses API o Certain actions (shell commands or modifying files) may require explicit user approval depending on the user's config. When `turn/start` is used, the app-server drives an approval flow by sending a server-initiated JSON-RPC request to the client. The client must respond to tell Codex whether to proceed. UIs should present these requests inline with the active turn so users can review the proposed command or diff before choosing. - Requests include `threadId` and `turnId`—use them to scope UI state to the active conversation. -- Respond with a single `{ "decision": "accept" | "decline" }` payload (plus optional `acceptSettings` on command executions). The server resumes or declines the work and ends the item with `item/completed`. +- Respond with a single `{ "decision": ... }` payload. Command approvals support `accept`, `acceptForSession`, `acceptWithExecpolicyAmendment`, `applyNetworkPolicyAmendment`, `decline`, or `cancel`. The server resumes or declines the work and ends the item with `item/completed`. ### Command execution approvals Order of messages: 1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action. -2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. -3. Client response — `{ "decision": "accept", "acceptSettings": { "forSession": false } }` or `{ "decision": "decline" }`. +2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. +3. Client response — for example `{ "decision": "accept" }`, `{ "decision": "acceptForSession" }`, `{ "decision": { "acceptWithExecpolicyAmendment": { "execpolicy_amendment": [...] } } }`, `{ "decision": { "applyNetworkPolicyAmendment": { "network_policy_amendment": { "host": "example.com", "action": "allow" } } } }`, `{ "decision": "decline" }`, or `{ "decision": "cancel" }`. 4. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result. ### File change approvals diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index d94e9a033..0b81a4c89 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -46,6 +46,8 @@ use codex_app_server_protocol::McpToolCallResult; use codex_app_server_protocol::McpToolCallStatus; use codex_app_server_protocol::ModelReroutedNotification; use codex_app_server_protocol::NetworkApprovalContext as V2NetworkApprovalContext; +use codex_app_server_protocol::NetworkPolicyAmendment as V2NetworkPolicyAmendment; +use codex_app_server_protocol::NetworkPolicyRuleAction as V2NetworkPolicyRuleAction; use codex_app_server_protocol::PatchApplyStatus; use codex_app_server_protocol::PlanDeltaNotification; use codex_app_server_protocol::RawResponseItemCompletedNotification; @@ -268,6 +270,7 @@ pub(crate) async fn apply_bespoke_event_handling( reason, network_approval_context, proposed_execpolicy_amendment, + proposed_network_policy_amendments, additional_permissions, parsed_cmd, .. @@ -331,6 +334,13 @@ pub(crate) async fn apply_bespoke_event_handling( }; let proposed_execpolicy_amendment_v2 = proposed_execpolicy_amendment.map(V2ExecPolicyAmendment::from); + let proposed_network_policy_amendments_v2 = proposed_network_policy_amendments + .map(|amendments| { + amendments + .into_iter() + .map(V2NetworkPolicyAmendment::from) + .collect() + }); let additional_permissions = additional_permissions.map(V2AdditionalPermissionProfile::from); @@ -346,6 +356,7 @@ pub(crate) async fn apply_bespoke_event_handling( command_actions, additional_permissions, proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2, + proposed_network_policy_amendments: proposed_network_policy_amendments_v2, }; let rx = outgoing .send_request(ServerRequestPayload::CommandExecutionRequestApproval( @@ -1915,6 +1926,20 @@ async fn on_command_execution_request_approval_response( }, None, ), + CommandExecutionApprovalDecision::ApplyNetworkPolicyAmendment { + network_policy_amendment, + } => { + let completion_status = match network_policy_amendment.action { + V2NetworkPolicyRuleAction::Allow => None, + V2NetworkPolicyRuleAction::Deny => Some(CommandExecutionStatus::Declined), + }; + ( + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: network_policy_amendment.into_core(), + }, + completion_status, + ) + } CommandExecutionApprovalDecision::Decline => ( ReviewDecision::Denied, Some(CommandExecutionStatus::Declined), diff --git a/codex-rs/app-server/src/transport.rs b/codex-rs/app-server/src/transport.rs index 70a251ad2..8c36df982 100644 --- a/codex-rs/app-server/src/transport.rs +++ b/codex-rs/app-server/src/transport.rs @@ -985,6 +985,7 @@ mod tests { }, ), proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, }, }), }, @@ -1045,6 +1046,7 @@ mod tests { }, ), proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, }, }), }, diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index cbe178093..34934b3d4 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -80,6 +80,10 @@ enum ActionKind { target: TargetPath, content: &'static str, }, + FetchUrlNoProxy { + endpoint: &'static str, + response_body: &'static str, + }, FetchUrl { endpoint: &'static str, response_body: &'static str, @@ -142,6 +146,28 @@ impl ActionKind { let event = shell_event(call_id, &command, 5_000, sandbox_permissions)?; Ok((event, Some(command))) } + ActionKind::FetchUrlNoProxy { + endpoint, + response_body, + } => { + Mock::given(method("GET")) + .and(path(*endpoint)) + .respond_with( + ResponseTemplate::new(200).set_body_string(response_body.to_string()), + ) + .mount(server) + .await; + + let url = format!("{}{}", server.uri(), endpoint); + let escaped_url = url.replace('\'', "\\'"); + let script = format!( + "import sys\nimport urllib.request\nurl = '{escaped_url}'\nopener = urllib.request.build_opener(urllib.request.ProxyHandler({{}}))\ntry:\n data = opener.open(url, timeout=2).read().decode()\n print('OK:' + data.strip())\nexcept Exception as exc:\n print('ERR:' + exc.__class__.__name__)\n sys.exit(1)", + ); + + let command = format!("python3 -c \"{script}\""); + let event = shell_event(call_id, &command, 5_000, sandbox_permissions)?; + Ok((event, Some(command))) + } ActionKind::RunCommand { command } => { let event = shell_event(call_id, command, 1_000, sandbox_permissions)?; Ok((event, Some(command.to_string()))) @@ -698,7 +724,7 @@ fn scenarios() -> Vec { name: "danger_full_access_on_request_allows_network", approval_policy: OnRequest, sandbox_policy: SandboxPolicy::DangerFullAccess, - action: ActionKind::FetchUrl { + action: ActionKind::FetchUrlNoProxy { endpoint: "/dfa/network", response_body: "danger-network-ok", }, @@ -714,7 +740,7 @@ fn scenarios() -> Vec { name: "danger_full_access_on_request_allows_network_gpt_5_1_no_exit", approval_policy: OnRequest, sandbox_policy: SandboxPolicy::DangerFullAccess, - action: ActionKind::FetchUrl { + action: ActionKind::FetchUrlNoProxy { endpoint: "/dfa/network", response_body: "danger-network-ok", }, diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 0cfd48bec..fd58baef4 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -23,6 +23,8 @@ use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; +use codex_protocol::protocol::NetworkPolicyAmendment; +use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use crossterm::event::KeyCode; @@ -46,6 +48,7 @@ pub(crate) enum ApprovalRequest { reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, + proposed_network_policy_amendments: Option>, additional_permissions: Option, }, ApplyPatch { @@ -114,11 +117,13 @@ impl ApprovalOverlay { ApprovalVariant::Exec { network_approval_context, proposed_execpolicy_amendment, + proposed_network_policy_amendments, additional_permissions, .. } => ( exec_options( proposed_execpolicy_amendment.clone(), + proposed_network_policy_amendments.clone(), network_approval_context.as_ref(), additional_permissions.as_ref(), ), @@ -362,6 +367,7 @@ impl From for ApprovalRequestState { reason, network_approval_context, proposed_execpolicy_amendment, + proposed_network_policy_amendments, additional_permissions, } => { let mut header: Vec> = Vec::new(); @@ -393,6 +399,7 @@ impl From for ApprovalRequestState { command, network_approval_context, proposed_execpolicy_amendment, + proposed_network_policy_amendments, additional_permissions, }, header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })), @@ -450,6 +457,7 @@ enum ApprovalVariant { command: Vec, network_approval_context: Option, proposed_execpolicy_amendment: Option, + proposed_network_policy_amendments: Option>, additional_permissions: Option, }, ApplyPatch { @@ -485,11 +493,12 @@ impl ApprovalOption { fn exec_options( proposed_execpolicy_amendment: Option, + proposed_network_policy_amendments: Option>, network_approval_context: Option<&NetworkApprovalContext>, additional_permissions: Option<&PermissionProfile>, ) -> Vec { if network_approval_context.is_some() { - return vec![ + let mut options = vec![ ApprovalOption { label: "Yes, just this once".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Approved), @@ -497,18 +506,36 @@ fn exec_options( additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], }, ApprovalOption { - label: "Yes, and allow this host for this session".to_string(), + label: "Yes, and allow this host for this conversation".to_string(), decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], }, - ApprovalOption { - label: "No, and tell Codex what to do differently".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Abort), - display_shortcut: Some(key_hint::plain(KeyCode::Esc)), - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }, ]; + for amendment in proposed_network_policy_amendments.unwrap_or_default() { + let (label, shortcut) = match amendment.action { + NetworkPolicyRuleAction::Allow => ( + "Yes, and allow this host in the future".to_string(), + KeyCode::Char('p'), + ), + NetworkPolicyRuleAction::Deny => continue, + }; + options.push(ApprovalOption { + label, + decision: ApprovalDecision::Review(ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: amendment, + }), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(shortcut)], + }); + } + options.push(ApprovalOption { + label: "No, and tell Codex what to do differently".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Abort), + display_shortcut: Some(key_hint::plain(KeyCode::Esc)), + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], + }); + return options; } if additional_permissions.is_some() { @@ -670,6 +697,7 @@ mod tests { reason: Some("reason".to_string()), network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, additional_permissions: None, } } @@ -716,6 +744,7 @@ mod tests { proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "echo".to_string(), ])), + proposed_network_policy_amendments: None, additional_permissions: None, }, tx, @@ -743,6 +772,43 @@ mod tests { ); } + #[test] + fn network_deny_forever_shortcut_is_not_bound() { + let (tx, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let mut view = ApprovalOverlay::new( + ApprovalRequest::Exec { + id: "test".to_string(), + command: vec!["curl".to_string(), "https://example.com".to_string()], + reason: None, + network_approval_context: Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }), + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: Some(vec![ + NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Deny, + }, + ]), + additional_permissions: None, + }, + tx, + Features::with_defaults(), + ); + view.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::NONE)); + + assert!( + rx.try_recv().is_err(), + "unexpected approval event emitted for hidden network deny shortcut" + ); + } + #[test] fn header_includes_command_snippet() { let (tx, _rx) = unbounded_channel::(); @@ -754,6 +820,7 @@ mod tests { reason: None, network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, additional_permissions: None, }; @@ -784,6 +851,16 @@ mod tests { }; let options = exec_options( Some(ExecPolicyAmendment::new(vec!["curl".to_string()])), + Some(vec![ + NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Deny, + }, + ]), Some(&network_context), None, ); @@ -793,7 +870,8 @@ mod tests { labels, vec![ "Yes, just this once".to_string(), - "Yes, and allow this host for this session".to_string(), + "Yes, and allow this host for this conversation".to_string(), + "Yes, and allow this host in the future".to_string(), "No, and tell Codex what to do differently".to_string(), ] ); @@ -808,7 +886,7 @@ mod tests { }), ..Default::default() }; - let options = exec_options(None, None, Some(&additional_permissions)); + let options = exec_options(None, None, None, Some(&additional_permissions)); let labels: Vec = options.into_iter().map(|option| option.label).collect(); assert_eq!( @@ -830,6 +908,7 @@ mod tests { reason: None, network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, additional_permissions: Some(PermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![PathBuf::from("/tmp/readme.txt")]), @@ -869,6 +948,7 @@ mod tests { reason: Some("need filesystem access".into()), network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, additional_permissions: Some(PermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![PathBuf::from("/tmp/readme.txt")]), @@ -898,12 +978,23 @@ mod tests { protocol: NetworkApprovalProtocol::Https, }), proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["curl".into()])), + proposed_network_policy_amendments: Some(vec![ + NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Deny, + }, + ]), additional_permissions: None, }; let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults()); let mut buf = Buffer::empty(Rect::new(0, 0, 100, view.desired_height(100))); view.render(Rect::new(0, 0, 100, view.desired_height(100)), &mut buf); + assert_snapshot!("network_exec_prompt", format!("{buf:?}")); let rendered: Vec = (0..buf.area.height) .map(|row| { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5781e055d..188ab87d6 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1096,6 +1096,7 @@ mod tests { reason: None, network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, additional_permissions: None, } } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__network_exec_prompt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__network_exec_prompt.snap new file mode 100644 index 000000000..612563fe1 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__network_exec_prompt.snap @@ -0,0 +1,38 @@ +--- +source: tui/src/bottom_pane/approval_overlay.rs +assertion_line: 821 +expression: "format!(\"{buf:?}\")" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 100, height: 12 }, + content: [ + " ", + " Do you want to approve network access to "example.com"? ", + " ", + " Reason: network request blocked ", + " ", + " ", + "› 1. Yes, just this once (y) ", + " 2. Yes, and allow this host for this conversation (a) ", + " 3. Yes, and allow this host in the future (p) ", + " 4. No, and tell Codex what to do differently (esc) ", + " ", + " Press enter to confirm or esc to cancel ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 57, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 10, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: ITALIC, + x: 33, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 6, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 28, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 53, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 54, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 45, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 46, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 48, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 51, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + ] +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9f4148b9d..b182d814d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2574,6 +2574,7 @@ impl ChatWidget { reason: ev.reason, network_approval_context: ev.network_approval_context, proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment, + proposed_network_policy_amendments: ev.proposed_network_policy_amendments, additional_permissions: ev.additional_permissions, }; self.bottom_pane diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 5faa1ed9a..78312db27 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -785,6 +785,7 @@ pub fn new_approval_decision_cell( command: Vec, decision: codex_protocol::protocol::ReviewDecision, ) -> Box { + use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::ReviewDecision::*; let (symbol, summary): (Span<'static>, Vec>) = match decision { @@ -830,29 +831,27 @@ pub fn new_approval_decision_cell( } NetworkPolicyAmendment { network_policy_amendment, - } => { - let host = Span::from(network_policy_amendment.host).dim(); - match network_policy_amendment.action { - codex_protocol::protocol::NetworkPolicyRuleAction::Allow => ( - "✔ ".green(), - vec![ - "You ".into(), - "approved".bold(), - " future network access to ".into(), - host, - ], - ), - codex_protocol::protocol::NetworkPolicyRuleAction::Deny => ( - "✗ ".red(), - vec![ - "You ".into(), - "blocked".bold(), - " future network access to ".into(), - host, - ], - ), - } - } + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => ( + "✔ ".green(), + vec![ + "You ".into(), + "persisted".bold(), + " Codex network access to ".into(), + Span::from(network_policy_amendment.host).dim(), + ], + ), + NetworkPolicyRuleAction::Deny => ( + "✗ ".red(), + vec![ + "You ".into(), + "denied".bold(), + " codex network access to ".into(), + Span::from(network_policy_amendment.host).dim(), + " and saved that rule".into(), + ], + ), + }, Denied => { let snippet = Span::from(exec_snippet(&command)).dim(); (