diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 9257aec839..95b647a5d0 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -110,6 +110,28 @@ "type": "object" } ] + }, + "CommandExecutionNetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/CommandExecutionNetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "CommandExecutionNetworkApprovalProtocol": { + "enum": [ + "http", + "https" + ], + "type": "string" } }, "properties": { @@ -140,6 +162,17 @@ "itemId": { "type": "string" }, + "networkApprovalContext": { + "anyOf": [ + { + "$ref": "#/definitions/CommandExecutionNetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional context for host-scoped network approvals." + }, "proposedExecpolicyAmendment": { "description": "Optional proposed execpolicy amendment to allow similar commands without prompting.", "items": { diff --git a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json index 977b1626a0..4c97cd3a11 100644 --- a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json @@ -1,6 +1,28 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "ExecCommandNetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/ExecCommandNetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "ExecCommandNetworkApprovalProtocol": { + "enum": [ + "http", + "https" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { @@ -133,6 +155,16 @@ "cwd": { "type": "string" }, + "networkApprovalContext": { + "anyOf": [ + { + "$ref": "#/definitions/ExecCommandNetworkApprovalContext" + }, + { + "type": "null" + } + ] + }, "parsedCmd": { "items": { "$ref": "#/definitions/ParsedCommand" diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 3fa487385d..1e83bd57f6 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -177,6 +177,28 @@ } ] }, + "CommandExecutionNetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/CommandExecutionNetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "CommandExecutionNetworkApprovalProtocol": { + "enum": [ + "http", + "https" + ], + "type": "string" + }, "CommandExecutionRequestApprovalParams": { "properties": { "command": { @@ -206,6 +228,17 @@ "itemId": { "type": "string" }, + "networkApprovalContext": { + "anyOf": [ + { + "$ref": "#/definitions/CommandExecutionNetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional context for host-scoped network approvals." + }, "proposedExecpolicyAmendment": { "description": "Optional proposed execpolicy amendment to allow similar commands without prompting.", "items": { @@ -280,6 +313,16 @@ "cwd": { "type": "string" }, + "networkApprovalContext": { + "anyOf": [ + { + "$ref": "#/definitions/ExecCommandNetworkApprovalContext" + }, + { + "type": "null" + } + ] + }, "parsedCmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -302,6 +345,28 @@ ], "type": "object" }, + "ExecCommandNetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/ExecCommandNetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "ExecCommandNetworkApprovalProtocol": { + "enum": [ + "http", + "https" + ], + "type": "string" + }, "FileChange": { "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 5be11b4093..b3166b078a 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 @@ -2048,6 +2048,28 @@ } ] }, + "CommandExecutionNetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/CommandExecutionNetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "CommandExecutionNetworkApprovalProtocol": { + "enum": [ + "http", + "https" + ], + "type": "string" + }, "CommandExecutionRequestApprovalParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -2078,6 +2100,17 @@ "itemId": { "type": "string" }, + "networkApprovalContext": { + "anyOf": [ + { + "$ref": "#/definitions/CommandExecutionNetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional context for host-scoped network approvals." + }, "proposedExecpolicyAmendment": { "description": "Optional proposed execpolicy amendment to allow similar commands without prompting.", "items": { @@ -4926,6 +4959,16 @@ "cwd": { "type": "string" }, + "networkApprovalContext": { + "anyOf": [ + { + "$ref": "#/definitions/ExecCommandNetworkApprovalContext" + }, + { + "type": "null" + } + ] + }, "parsedCmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -4962,6 +5005,28 @@ "title": "ExecCommandApprovalResponse", "type": "object" }, + "ExecCommandNetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/ExecCommandNetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "ExecCommandNetworkApprovalProtocol": { + "enum": [ + "http", + "https" + ], + "type": "string" + }, "ExecCommandSource": { "enum": [ "agent", diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts index b427337a84..68e82fb14f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts @@ -1,6 +1,7 @@ // 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 { ExecCommandNetworkApprovalContext } from "./ExecCommandNetworkApprovalContext"; import type { ParsedCommand } from "./ParsedCommand"; import type { ThreadId } from "./ThreadId"; @@ -9,4 +10,4 @@ export type ExecCommandApprovalParams = { conversationId: ThreadId, * Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] * and [codex_core::protocol::ExecCommandEndEvent]. */ -callId: string, command: Array, cwd: string, reason: string | null, parsedCmd: Array, }; +callId: string, command: Array, cwd: string, reason: string | null, networkApprovalContext: ExecCommandNetworkApprovalContext | null, parsedCmd: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecCommandNetworkApprovalContext.ts b/codex-rs/app-server-protocol/schema/typescript/ExecCommandNetworkApprovalContext.ts new file mode 100644 index 0000000000..9a721b41a9 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/ExecCommandNetworkApprovalContext.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 { ExecCommandNetworkApprovalProtocol } from "./ExecCommandNetworkApprovalProtocol"; + +export type ExecCommandNetworkApprovalContext = { host: string, protocol: ExecCommandNetworkApprovalProtocol, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecCommandNetworkApprovalProtocol.ts b/codex-rs/app-server-protocol/schema/typescript/ExecCommandNetworkApprovalProtocol.ts new file mode 100644 index 0000000000..21a1229dd9 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/ExecCommandNetworkApprovalProtocol.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 ExecCommandNetworkApprovalProtocol = "http" | "https"; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 4780a82576..48b57ebc77 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -60,6 +60,8 @@ export type { ExecCommandApprovalParams } from "./ExecCommandApprovalParams"; export type { ExecCommandApprovalResponse } from "./ExecCommandApprovalResponse"; export type { ExecCommandBeginEvent } from "./ExecCommandBeginEvent"; export type { ExecCommandEndEvent } from "./ExecCommandEndEvent"; +export type { ExecCommandNetworkApprovalContext } from "./ExecCommandNetworkApprovalContext"; +export type { ExecCommandNetworkApprovalProtocol } from "./ExecCommandNetworkApprovalProtocol"; export type { ExecCommandOutputDeltaEvent } from "./ExecCommandOutputDeltaEvent"; export type { ExecCommandSource } from "./ExecCommandSource"; export type { ExecCommandStatus } from "./ExecCommandStatus"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionNetworkApprovalContext.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionNetworkApprovalContext.ts new file mode 100644 index 0000000000..b9e8048430 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionNetworkApprovalContext.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 { CommandExecutionNetworkApprovalProtocol } from "./CommandExecutionNetworkApprovalProtocol"; + +export type CommandExecutionNetworkApprovalContext = { host: string, protocol: CommandExecutionNetworkApprovalProtocol, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionNetworkApprovalProtocol.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionNetworkApprovalProtocol.ts new file mode 100644 index 0000000000..ebea6ce106 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionNetworkApprovalProtocol.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 CommandExecutionNetworkApprovalProtocol = "http" | "https"; 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 12b2521431..2898ecab4e 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -2,6 +2,7 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { CommandAction } from "./CommandAction"; +import type { CommandExecutionNetworkApprovalContext } from "./CommandExecutionNetworkApprovalContext"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; export type CommandExecutionRequestApprovalParams = { threadId: string, turnId: string, itemId: string, @@ -9,6 +10,10 @@ export type CommandExecutionRequestApprovalParams = { threadId: string, turnId: * Optional explanatory reason (e.g. request for network access). */ reason?: string | null, +/** + * Optional context for host-scoped network approvals. + */ +networkApprovalContext?: CommandExecutionNetworkApprovalContext | null, /** * The command to be executed. */ 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 500b1d930f..722d4d05f9 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -29,6 +29,8 @@ export type { CommandAction } from "./CommandAction"; export type { CommandExecParams } from "./CommandExecParams"; export type { CommandExecResponse } from "./CommandExecResponse"; export type { CommandExecutionApprovalDecision } from "./CommandExecutionApprovalDecision"; +export type { CommandExecutionNetworkApprovalContext } from "./CommandExecutionNetworkApprovalContext"; +export type { CommandExecutionNetworkApprovalProtocol } from "./CommandExecutionNetworkApprovalProtocol"; export type { CommandExecutionOutputDeltaNotification } from "./CommandExecutionOutputDeltaNotification"; export type { CommandExecutionRequestApprovalParams } from "./CommandExecutionRequestApprovalParams"; export type { CommandExecutionRequestApprovalResponse } from "./CommandExecutionRequestApprovalResponse"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index bc91341166..bcdc53f2bf 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -995,6 +995,10 @@ mod tests { command: vec!["echo".to_string(), "hello".to_string()], cwd: PathBuf::from("/tmp"), reason: Some("because tests".to_string()), + network_approval_context: Some(v1::ExecCommandNetworkApprovalContext { + host: "example.com".to_string(), + protocol: v1::ExecCommandNetworkApprovalProtocol::Https, + }), parsed_cmd: vec![ParsedCommand::Unknown { cmd: "echo hello".to_string(), }], @@ -1014,6 +1018,10 @@ mod tests { "command": ["echo", "hello"], "cwd": "/tmp", "reason": "because tests", + "networkApprovalContext": { + "host": "example.com", + "protocol": "https" + }, "parsedCmd": [ { "type": "unknown", diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 25e60ca751..7c3a01e70c 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -251,6 +251,20 @@ pub struct ApplyPatchApprovalResponse { pub decision: ReviewDecision, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +pub struct ExecCommandNetworkApprovalContext { + pub host: String, + pub protocol: ExecCommandNetworkApprovalProtocol, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum ExecCommandNetworkApprovalProtocol { + Http, + Https, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] pub struct ExecCommandApprovalParams { @@ -261,6 +275,7 @@ pub struct ExecCommandApprovalParams { pub command: Vec, pub cwd: PathBuf, pub reason: Option, + pub network_approval_context: Option, pub parsed_cmd: Vec, } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 7cff3e2def..4053458e35 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -3056,6 +3056,22 @@ pub struct ContextCompactedNotification { pub turn_id: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct CommandExecutionNetworkApprovalContext { + pub host: String, + pub protocol: CommandExecutionNetworkApprovalProtocol, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +pub enum CommandExecutionNetworkApprovalProtocol { + Http, + Https, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -3066,6 +3082,9 @@ pub struct CommandExecutionRequestApprovalParams { /// Optional explanatory reason (e.g. request for network access). #[ts(optional = nullable)] pub reason: Option, + /// Optional context for host-scoped network approvals. + #[ts(optional = nullable)] + pub network_approval_context: Option, /// The command to be executed. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 91012572ac..4dc102f576 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1005,6 +1005,7 @@ impl CodexClient { turn_id, item_id, reason, + network_approval_context, command, cwd, command_actions, @@ -1017,6 +1018,9 @@ impl CodexClient { if let Some(reason) = reason.as_deref() { println!("< reason: {reason}"); } + if let Some(network_approval_context) = network_approval_context.as_ref() { + println!("< network approval context: {network_approval_context:?}"); + } if let Some(command) = command.as_deref() { println!("< command: {command}"); } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index a683891fb6..2f72f62158 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -17,6 +17,8 @@ use codex_app_server_protocol::CollabAgentTool; use codex_app_server_protocol::CollabAgentToolCallStatus as V2CollabToolCallStatus; use codex_app_server_protocol::CommandAction as V2ParsedCommand; use codex_app_server_protocol::CommandExecutionApprovalDecision; +use codex_app_server_protocol::CommandExecutionNetworkApprovalContext as V2CommandExecutionNetworkApprovalContext; +use codex_app_server_protocol::CommandExecutionNetworkApprovalProtocol as V2CommandExecutionNetworkApprovalProtocol; use codex_app_server_protocol::CommandExecutionOutputDeltaNotification; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; @@ -27,6 +29,8 @@ use codex_app_server_protocol::DynamicToolCallParams; use codex_app_server_protocol::ErrorNotification; use codex_app_server_protocol::ExecCommandApprovalParams; use codex_app_server_protocol::ExecCommandApprovalResponse; +use codex_app_server_protocol::ExecCommandNetworkApprovalContext as V1ExecCommandNetworkApprovalContext; +use codex_app_server_protocol::ExecCommandNetworkApprovalProtocol as V1ExecCommandNetworkApprovalProtocol; use codex_app_server_protocol::ExecPolicyAmendment as V2ExecPolicyAmendment; use codex_app_server_protocol::FileChangeApprovalDecision; use codex_app_server_protocol::FileChangeOutputDeltaNotification; @@ -86,6 +90,8 @@ use codex_core::protocol::TurnDiffEvent; use codex_core::review_format::format_review_findings_block; use codex_core::review_prompts; use codex_protocol::ThreadId; +use codex_protocol::approvals::NetworkApprovalContext as CoreNetworkApprovalContext; +use codex_protocol::approvals::NetworkApprovalProtocol as CoreNetworkApprovalProtocol; use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem as CoreDynamicToolCallOutputContentItem; use codex_protocol::dynamic_tools::DynamicToolResponse as CoreDynamicToolResponse; use codex_protocol::plan_tool::UpdatePlanArgs; @@ -205,7 +211,7 @@ pub(crate) async fn apply_bespoke_event_handling( reason, proposed_execpolicy_amendment, parsed_cmd, - network_approval_context: _, + network_approval_context, }) => match api_version { ApiVersion::V1 => { let params = ExecCommandApprovalParams { @@ -214,6 +220,9 @@ pub(crate) async fn apply_bespoke_event_handling( command, cwd, reason, + network_approval_context: network_approval_context + .clone() + .map(v1_network_approval_context_from_core), parsed_cmd, }; let rx = outgoing @@ -241,6 +250,8 @@ pub(crate) async fn apply_bespoke_event_handling( // and emit the corresponding EventMsg, we repurpose the call_id as the item_id. item_id: item_id.clone(), reason, + network_approval_context: network_approval_context + .map(v2_network_approval_context_from_core), command: Some(command_string.clone()), cwd: Some(cwd.clone()), command_actions: Some(command_actions.clone()), @@ -1608,6 +1619,32 @@ fn format_file_change_diff(change: &CoreFileChange) -> String { } } +fn v1_network_approval_context_from_core( + value: CoreNetworkApprovalContext, +) -> V1ExecCommandNetworkApprovalContext { + let protocol = match value.protocol { + CoreNetworkApprovalProtocol::Http => V1ExecCommandNetworkApprovalProtocol::Http, + CoreNetworkApprovalProtocol::Https => V1ExecCommandNetworkApprovalProtocol::Https, + }; + V1ExecCommandNetworkApprovalContext { + host: value.host, + protocol, + } +} + +fn v2_network_approval_context_from_core( + value: CoreNetworkApprovalContext, +) -> V2CommandExecutionNetworkApprovalContext { + let protocol = match value.protocol { + CoreNetworkApprovalProtocol::Http => V2CommandExecutionNetworkApprovalProtocol::Http, + CoreNetworkApprovalProtocol::Https => V2CommandExecutionNetworkApprovalProtocol::Https, + }; + V2CommandExecutionNetworkApprovalContext { + host: value.host, + protocol, + } +} + fn map_file_change_approval_decision( decision: FileChangeApprovalDecision, ) -> (ReviewDecision, Option) { diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 863a7bfc4f..4cc02ee378 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -301,6 +301,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { command: format_with_current_shell("python3 -c 'print(42)'"), cwd: working_directory.clone(), reason: None, + network_approval_context: None, parsed_cmd: vec![ParsedCommand::Unknown { cmd: "python3 -c 'print(42)'".to_string() }], diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 412aefa9f1..9e47d82c30 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -533,6 +533,12 @@ struct NetworkApprovalAttempt { command: Vec, cwd: PathBuf, approved_hosts: Mutex>, + outcome: Mutex>, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum NetworkApprovalOutcome { + DeniedByUser, } const SEARCH_TOOL_DEVELOPER_INSTRUCTIONS: &str = @@ -2137,6 +2143,7 @@ impl Session { command, cwd, approved_hosts: Mutex::new(HashSet::new()), + outcome: Mutex::new(None), }), ); } @@ -2146,6 +2153,18 @@ impl Session { attempts.remove(attempt_id); } + pub(crate) async fn take_network_approval_outcome( + &self, + attempt_id: &str, + ) -> Option { + let attempt = { + let attempts = self.network_approval_attempts.lock().await; + attempts.get(attempt_id).cloned() + }?; + let mut outcome = attempt.outcome.lock().await; + outcome.take() + } + async fn resolve_network_approval_attempt( &self, request: &NetworkPolicyRequest, @@ -2166,22 +2185,19 @@ impl Session { ); } - if attempts.len() == 1 - && let Some(attempt) = attempts.values().next().cloned() - { + if attempts.len() == 1 { tracing::debug!( - "inline network approval decider falling back to only active attempt for host {}", + "inline network approval decider falling back to sole active attempt for host {}", request.host ); - return Some(attempt); - } else { - tracing::debug!( - "inline network approval decider cannot disambiguate attempt for host {} (active_attempts={})", - request.host, - attempts.len() - ); + return attempts.values().next().cloned(); } + tracing::debug!( + "inline network approval decider cannot disambiguate attempt for host {} (active_attempts={})", + request.host, + attempts.len() + ); None } @@ -2255,6 +2271,8 @@ impl Session { NetworkDecision::Allow } ReviewDecision::Denied | ReviewDecision::Abort => { + let mut outcome = attempt.outcome.lock().await; + *outcome = Some(NetworkApprovalOutcome::DeniedByUser); NetworkDecision::deny(REASON_NOT_ALLOWED) } } @@ -7248,7 +7266,7 @@ mod tests { } #[tokio::test] - async fn resolve_network_approval_attempt_falls_back_to_only_active_attempt() { + async fn resolve_network_approval_attempt_falls_back_to_single_active_attempt() { let (session, _turn_context) = make_session_and_context().await; session .register_network_approval_attempt( @@ -7274,13 +7292,61 @@ mod tests { ); let resolved = session.resolve_network_approval_attempt(&request).await; - assert!(resolved.is_some()); + let resolved = resolved.expect("single active attempt should be used as fallback"); + assert_eq!(resolved.call_id, "call-1"); session .unregister_network_approval_attempt("attempt-1") .await; } + #[tokio::test] + async fn resolve_network_approval_attempt_returns_exact_attempt_match() { + let (session, _turn_context) = make_session_and_context().await; + session + .register_network_approval_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "google.com".to_string()], + std::env::temp_dir(), + ) + .await; + session + .register_network_approval_attempt( + "attempt-2".to_string(), + "turn-2".to_string(), + "call-2".to_string(), + vec!["curl".to_string(), "openai.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let request = codex_network_proxy::NetworkPolicyRequest::new( + codex_network_proxy::NetworkPolicyRequestArgs { + protocol: codex_network_proxy::NetworkProtocol::Http, + host: "openai.com".to_string(), + port: 80, + client_addr: None, + method: Some("GET".to_string()), + command: None, + exec_policy_hint: None, + attempt_id: Some("attempt-2".to_string()), + }, + ); + + let resolved = session.resolve_network_approval_attempt(&request).await; + let resolved = resolved.expect("attempt-2 should resolve"); + assert_eq!(resolved.call_id, "call-2"); + + session + .unregister_network_approval_attempt("attempt-1") + .await; + session + .unregister_network_approval_attempt("attempt-2") + .await; + } + #[tokio::test] async fn resolve_network_approval_attempt_returns_none_when_ambiguous() { let (session, _turn_context) = make_session_and_context().await; @@ -7353,6 +7419,45 @@ mod tests { assert_eq!(decision, codex_network_proxy::NetworkDecision::Allow); } + #[tokio::test] + async fn take_network_approval_outcome_clears_stored_value() { + let (session, _turn_context) = make_session_and_context().await; + session + .register_network_approval_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "google.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let attempt = { + let attempts = session.network_approval_attempts.lock().await; + attempts + .get("attempt-1") + .cloned() + .expect("attempt should exist") + }; + { + let mut outcome = attempt.outcome.lock().await; + *outcome = Some(NetworkApprovalOutcome::DeniedByUser); + } + + assert_eq!( + session.take_network_approval_outcome("attempt-1").await, + Some(NetworkApprovalOutcome::DeniedByUser) + ); + assert_eq!( + session.take_network_approval_outcome("attempt-1").await, + None + ); + + session + .unregister_network_approval_attempt("attempt-1") + .await; + } + #[tokio::test] async fn record_model_warning_appends_user_message() { let (mut session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 35c523f204..0287857ef7 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -21,7 +21,6 @@ use crate::error::CodexErr; use crate::error::Result; use crate::error::SandboxErr; use crate::get_platform_sandbox; -use crate::network_policy_decision::NetworkPolicyDecisionPayload; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; @@ -253,16 +252,6 @@ pub(crate) async fn execute_exec_env( let network_attempt_id = network_attempt_id.or_else(|| network.as_ref().map(|_| Uuid::new_v4().to_string())); - let blocked_cursor = match network.as_ref() { - Some(network) => match network.blocked_requests_cursor().await { - Ok(cursor) => Some(cursor), - Err(err) => { - tracing::debug!("failed to read blocked telemetry cursor before exec: {err:#}"); - None - } - }, - None => None, - }; let params = ExecParams { command, @@ -280,63 +269,7 @@ pub(crate) async fn execute_exec_env( let start = Instant::now(); let raw_output_result = exec(params, sandbox, sandbox_policy, stdout_stream).await; let duration = start.elapsed(); - let finalized = finalize_exec_result(raw_output_result, sandbox, duration); - let network_policy_decision_for_exec = match network.as_ref() { - Some(network) => { - blocking_network_policy_decision_from_attempt_or_cursor( - network, - network_attempt_id.as_deref(), - blocked_cursor, - sandbox_policy, - ) - .await - } - None => None, - }; - match finalized { - Ok(exec_output) => { - if let Some(policy_decision) = network_policy_decision_for_exec { - tracing::debug!( - "promoting successful exec result to sandbox denied based on structured network telemetry (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", - policy_decision.decision, - policy_decision.source, - policy_decision.host, - policy_decision.protocol, - policy_decision.port - ); - return Err(CodexErr::Sandbox(SandboxErr::Denied { - output: Box::new(exec_output), - network_policy_decision: Some(policy_decision), - })); - } - Ok(exec_output) - } - Err(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision, - })) => { - let merged_decision = network_policy_decision.or(network_policy_decision_for_exec); - if let Some(payload) = merged_decision.as_ref() { - tracing::debug!( - "sandbox-denied exec result includes structured network decision (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", - payload.decision, - payload.source, - payload.host, - payload.protocol, - payload.port - ); - } else { - tracing::debug!( - "sandbox-denied exec result had no structured network decision payload" - ); - } - Err(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision: merged_decision, - })) - } - Err(err) => Err(err), - } + finalize_exec_result(raw_output_result, sandbox, duration) } #[cfg(target_os = "windows")] @@ -664,148 +597,6 @@ pub(crate) fn is_likely_sandbox_denied( false } -pub(crate) async fn blocking_network_policy_decision_from_attempt( - network: &NetworkProxy, - attempt_id: &str, - sandbox_policy: &SandboxPolicy, -) -> Option { - let entry = match network.latest_blocked_request_for_attempt(attempt_id).await { - Ok(entry) => entry, - Err(err) => { - tracing::debug!( - "failed to load blocked telemetry for network attempt {attempt_id}: {err:#}" - ); - return None; - } - }; - let payload = entry - .as_ref() - .and_then(|entry| network_policy_decision_from_blocked_entry(entry, sandbox_policy)); - if let Some(payload) = payload.as_ref() { - tracing::debug!( - "selected telemetry network decision for attempt {attempt_id} (decision={}, source={}, host={:?}, protocol={:?}, port={:?})", - payload.decision, - payload.source, - payload.host, - payload.protocol, - payload.port - ); - } else { - tracing::debug!("no blocked telemetry entry found for network attempt {attempt_id}"); - } - payload -} - -pub(crate) async fn blocking_network_policy_decision_from_attempt_or_cursor( - network: &NetworkProxy, - attempt_id: Option<&str>, - blocked_cursor: Option, - sandbox_policy: &SandboxPolicy, -) -> Option { - if let Some(attempt_id) = attempt_id - && let Some(payload) = - blocking_network_policy_decision_from_attempt(network, attempt_id, sandbox_policy).await - { - return Some(payload); - } - - let blocked_cursor = blocked_cursor?; - blocking_network_policy_decision_from_blocked_queue(network, blocked_cursor, sandbox_policy) - .await -} - -pub(crate) async fn blocking_network_policy_decision_from_blocked_queue( - network: &NetworkProxy, - cursor: u64, - sandbox_policy: &SandboxPolicy, -) -> Option { - let entries = match network.blocked_requests_since(cursor).await { - Ok(entries) => entries, - Err(err) => { - tracing::debug!( - "failed to load blocked telemetry entries since cursor {cursor}: {err:#}" - ); - return None; - } - }; - select_network_policy_decision_from_blocked_entries(&entries, sandbox_policy) -} - -fn select_network_policy_decision_from_blocked_entries( - entries: &[codex_network_proxy::BlockedRequest], - sandbox_policy: &SandboxPolicy, -) -> Option { - let mapped: Vec = entries - .iter() - .filter_map(|entry| network_policy_decision_from_blocked_entry(entry, sandbox_policy)) - .collect(); - mapped - .iter() - .rev() - .find(|payload| payload.is_ask_from_decider()) - .cloned() - .or_else(|| mapped.last().cloned()) -} - -fn network_policy_decision_from_blocked_entry( - entry: &codex_network_proxy::BlockedRequest, - sandbox_policy: &SandboxPolicy, -) -> Option { - let (decision, source) = derive_network_policy_decision( - entry.reason.as_str(), - entry.source.as_deref(), - sandbox_policy, - )?; - let protocol = match entry.protocol.as_str() { - "http-connect" => Some("https_connect".to_string()), - "socks5" => Some("socks5_tcp".to_string()), - "socks5-udp" => Some("socks5_udp".to_string()), - "http" | "https" | "https_connect" | "socks5_tcp" | "socks5_udp" => { - Some(entry.protocol.clone()) - } - _ => None, - }?; - Some(NetworkPolicyDecisionPayload { - decision, - source, - protocol: Some(protocol), - host: Some(entry.host.clone()), - reason: Some(entry.reason.clone()), - port: entry.port, - }) -} - -fn derive_network_policy_decision( - reason: &str, - source: Option<&str>, - sandbox_policy: &SandboxPolicy, -) -> Option<(String, String)> { - const REASON_NOT_ALLOWED: &str = "not_allowed"; - const REASON_NOT_ALLOWED_LOCAL: &str = "not_allowed_local"; - const REASON_DENIED: &str = "denied"; - const REASON_METHOD_NOT_ALLOWED: &str = "method_not_allowed"; - - let source_or = |fallback: &str| source.unwrap_or(fallback).to_string(); - - match reason { - REASON_NOT_ALLOWED if should_ask_on_allowlist_miss(sandbox_policy) => { - Some(("ask".to_string(), "decider".to_string())) - } - REASON_NOT_ALLOWED - | REASON_NOT_ALLOWED_LOCAL - | REASON_DENIED - | REASON_METHOD_NOT_ALLOWED => Some(("deny".to_string(), source_or("baseline_policy"))), - _ => Some(("deny".to_string(), source_or("proxy_state"))), - } -} - -fn should_ask_on_allowlist_miss(sandbox_policy: &SandboxPolicy) -> bool { - matches!( - sandbox_policy, - SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } - ) -} - #[derive(Debug, Clone)] pub struct StreamOutput { pub text: T, @@ -1129,22 +920,10 @@ fn synthetic_exit_status(code: i32) -> ExitStatus { #[cfg(test)] mod tests { use super::*; - use codex_network_proxy::BlockedRequest; - use codex_protocol::protocol::ReadOnlyAccess; use pretty_assertions::assert_eq; use std::time::Duration; use tokio::io::AsyncWriteExt; - fn restricted_sandbox_policy() -> SandboxPolicy { - SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - read_only_access: ReadOnlyAccess::FullAccess, - network_access: true, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - } - } - fn make_exec_output( exit_code: i32, stdout: &str, @@ -1231,198 +1010,6 @@ mod tests { )); } - #[test] - fn network_policy_decision_maps_fresh_entries() { - let entry = BlockedRequest { - host: "google.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http-connect".to_string(), - attempt_id: None, - decision: Some("ask".to_string()), - source: Some("decider".to_string()), - port: Some(443), - timestamp: 200, - }; - - let payload = - network_policy_decision_from_blocked_entry(&entry, &restricted_sandbox_policy()) - .expect("blocked entry should map to structured decision"); - assert_eq!( - payload, - NetworkPolicyDecisionPayload { - decision: "ask".to_string(), - source: "decider".to_string(), - protocol: Some("https_connect".to_string()), - host: Some("google.com".to_string()), - reason: Some("not_allowed".to_string()), - port: Some(443), - } - ); - } - - #[test] - fn network_policy_decision_ignores_allow_entries() { - let entry = BlockedRequest { - host: "google.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: None, - decision: Some("allow".to_string()), - source: Some("decider".to_string()), - port: Some(80), - timestamp: 200, - }; - - let payload = - network_policy_decision_from_blocked_entry(&entry, &restricted_sandbox_policy()) - .expect("restricted sandbox should derive ask decision on allowlist miss"); - assert_eq!(payload.decision, "ask"); - assert_eq!(payload.source, "decider"); - } - - #[test] - fn network_policy_decision_derives_source_when_missing() { - let entry = BlockedRequest { - host: "google.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: None, - decision: Some("ask".to_string()), - source: None, - port: Some(80), - timestamp: 100, - }; - - let payload = - network_policy_decision_from_blocked_entry(&entry, &restricted_sandbox_policy()) - .expect("missing source should still map to structured decision"); - assert_eq!(payload.decision, "ask"); - assert_eq!(payload.source, "decider"); - } - - #[test] - fn yolo_sandbox_denies_allowlist_miss() { - let entry = BlockedRequest { - host: "google.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: None, - decision: Some("ask".to_string()), - source: Some("decider".to_string()), - port: Some(80), - timestamp: 100, - }; - - let payload = - network_policy_decision_from_blocked_entry(&entry, &SandboxPolicy::DangerFullAccess) - .expect("allowlist miss should still map to structured decision in yolo"); - assert_eq!( - payload, - NetworkPolicyDecisionPayload { - decision: "deny".to_string(), - source: "decider".to_string(), - protocol: Some("http".to_string()), - host: Some("google.com".to_string()), - reason: Some("not_allowed".to_string()), - port: Some(80), - } - ); - } - - #[test] - fn select_network_policy_decision_prefers_ask_from_decider() { - let entries = vec![ - BlockedRequest { - host: "google.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: Some("attempt-1".to_string()), - decision: Some("ask".to_string()), - source: Some("decider".to_string()), - port: Some(80), - timestamp: 100, - }, - BlockedRequest { - host: "example.com".to_string(), - reason: "denied".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: Some("attempt-2".to_string()), - decision: Some("deny".to_string()), - source: Some("baseline_policy".to_string()), - port: Some(80), - timestamp: 200, - }, - ]; - - let selected = select_network_policy_decision_from_blocked_entries( - &entries, - &restricted_sandbox_policy(), - ) - .expect("expected a structured decision from blocked entries"); - assert_eq!(selected.decision, "ask"); - assert_eq!(selected.source, "decider"); - assert_eq!(selected.host.as_deref(), Some("google.com")); - } - - #[test] - fn select_network_policy_decision_uses_newest_when_no_ask() { - let entries = vec![ - BlockedRequest { - host: "old.example.com".to_string(), - reason: "denied".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: None, - decision: Some("deny".to_string()), - source: Some("baseline_policy".to_string()), - port: Some(80), - timestamp: 100, - }, - BlockedRequest { - host: "new.example.com".to_string(), - reason: "method_not_allowed".to_string(), - client: None, - method: Some("CONNECT".to_string()), - mode: None, - protocol: "http-connect".to_string(), - attempt_id: None, - decision: Some("deny".to_string()), - source: Some("mode_guard".to_string()), - port: Some(443), - timestamp: 200, - }, - ]; - - let selected = select_network_policy_decision_from_blocked_entries( - &entries, - &SandboxPolicy::DangerFullAccess, - ) - .expect("expected a structured decision from blocked entries"); - assert_eq!(selected.decision, "deny"); - assert_eq!(selected.host.as_deref(), Some("new.example.com")); - assert_eq!(selected.protocol.as_deref(), Some("https_connect")); - } - #[tokio::test] async fn read_capped_limits_retained_bytes() { let (mut writer, reader) = tokio::io::duplex(1024); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index a4e9866dbb..c000f78234 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -142,6 +142,12 @@ impl ToolOrchestrator { } else { tracing::debug!("sandbox denied without structured network decision payload"); } + if network_policy_decision.is_some() { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + }))); + } if !tool.escalate_on_failure() { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index ecd0d273ba..4df2c80137 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -4,6 +4,7 @@ Runtime: shell Executes shell requests under the orchestrator: asks for approval when needed, builds a CommandSpec, and runs it under the current SandboxAttempt. */ +use crate::codex::NetworkApprovalOutcome; use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecToolCallOutput; use crate::features::Feature; @@ -190,6 +191,11 @@ impl ToolRuntime for ShellRuntime { let out = execute_env(env, attempt.policy, Self::stdout_stream(ctx)) .await .map_err(ToolError::Codex); + let approval_outcome = if let Some(attempt_id) = network_attempt_id.as_deref() { + ctx.session.take_network_approval_outcome(attempt_id).await + } else { + None + }; if let Some(attempt_id) = network_attempt_id.as_deref() { ctx.session @@ -197,6 +203,10 @@ impl ToolRuntime for ShellRuntime { .await; } + if approval_outcome == Some(NetworkApprovalOutcome::DeniedByUser) { + return Err(ToolError::Rejected("rejected by user".to_string())); + } + let out = out?; Ok(out) } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 8eb1367e62..423f372771 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -13,13 +13,10 @@ use tokio::time::Instant; use tokio_util::sync::CancellationToken; use uuid::Uuid; -use crate::exec::blocking_network_policy_decision_from_attempt_or_cursor; +use crate::codex::NetworkApprovalOutcome; use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; -use crate::network_policy_decision::NetworkPolicyDecisionPayload; -use crate::network_policy_decision::network_approval_context_from_payload; use crate::protocol::ExecCommandSource; -use crate::protocol::ReviewDecision; use crate::sandboxing::ExecRequest; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; @@ -54,7 +51,6 @@ use crate::unified_exec::process::OutputBuffer; use crate::unified_exec::process::OutputHandles; use crate::unified_exec::process::UnifiedExecProcess; use crate::unified_exec::resolve_max_tokens; -use codex_protocol::approvals::NetworkApprovalContext; const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [ ("NO_COLOR", "1"), @@ -94,16 +90,6 @@ fn apply_unified_exec_env(mut env: HashMap) -> HashMap, -) -> Option { - if retried_after_network_approval { - return None; - } - network_policy_decision.and_then(network_approval_context_from_payload) -} - struct PreparedProcessHandles { writer_tx: mpsc::Sender>, output_buffer: OutputBuffer, @@ -175,237 +161,104 @@ impl UnifiedExecProcessManager { emitter.emit(event_ctx, ToolEventStage::Begin).await; let max_tokens = resolve_max_tokens(request.max_output_tokens); let yield_time_ms = clamp_yield_time(request.yield_time_ms); - let mut retried_after_network_approval = false; - let mut retry_network_context: Option = None; - - loop { - let temporary_allowed_host = if let Some(network_context) = retry_network_context.take() - { - if let Some(network) = request.network.as_ref() { - let granted_host = network - .grant_temporary_allowed_host(&network_context.host) - .await; - if granted_host.is_none() { - tracing::warn!( - host = %network_context.host, - "failed to grant temporary network host allowance for unified exec retry" - ); - } - granted_host.map(|host| (network.clone(), host)) - } else { - tracing::warn!( - host = %network_context.host, - "network approval context is present but no managed network proxy is available for unified exec retry" - ); - None - } - } else { - None - }; - - let network_attempt_id = request.network.as_ref().map(|_| Uuid::new_v4().to_string()); - let blocked_cursor = match request.network.as_ref() { - Some(network) => match network.blocked_requests_cursor().await { - Ok(cursor) => Some(cursor), - Err(err) => { - tracing::debug!( - "failed to read blocked telemetry cursor before unified exec: {err:#}" - ); - None - } - }, - None => None, - }; - if let Some(attempt_id) = network_attempt_id.as_ref() { - context - .session - .register_network_approval_attempt( - attempt_id.clone(), - context.turn.sub_id.clone(), - context.call_id.clone(), - request.command.clone(), - cwd.clone(), - ) - .await; - } - - let process = self - .open_session_with_sandbox( - &request, - cwd.clone(), - context, - retried_after_network_approval, - network_attempt_id.clone(), - ) - .await; - - let process = match process { - Ok(process) => Arc::new(process), - Err(err) => { - if let Some(attempt_id) = network_attempt_id.as_deref() { - context - .session - .unregister_network_approval_attempt(attempt_id) - .await; - } - if let Some((network, host)) = temporary_allowed_host { - network.revoke_temporary_allowed_host(&host).await; - } - self.release_process_id(&request.process_id).await; - return Err(err); - } - }; - - let transcript = Arc::new(tokio::sync::Mutex::new(HeadTailBuffer::default())); - start_streaming_output(&process, context, Arc::clone(&transcript)); - - let start = Instant::now(); - // For the initial exec_command call, we both stream output to events - // (via start_streaming_output above) and collect a snapshot here for - // the tool response body. - let OutputHandles { - output_buffer, - output_notify, - output_closed, - output_closed_notify, - cancellation_token, - } = process.output_handles(); - let deadline = start + Duration::from_millis(yield_time_ms); - let collected = Self::collect_output_until_deadline( - &output_buffer, - &output_notify, - &output_closed, - &output_closed_notify, - &cancellation_token, - deadline, - ) - .await; - let wall_time = Instant::now().saturating_duration_since(start); - - let text = String::from_utf8_lossy(&collected).to_string(); - let output = formatted_truncate_text(&text, TruncationPolicy::Tokens(max_tokens)); - let exit_code = process.exit_code(); - let has_exited = process.has_exited() || exit_code.is_some(); - let chunk_id = generate_chunk_id(); - let process_id = request.process_id.clone(); - - if has_exited { - // Short‑lived command: emit ExecCommandEnd immediately using the - // same helper as the background watcher, so all end events share - // one implementation. - let exit = exit_code.unwrap_or(-1); - emit_exec_end_for_unified_exec( - Arc::clone(&context.session), - Arc::clone(&context.turn), + let network_attempt_id = request.network.as_ref().map(|_| Uuid::new_v4().to_string()); + if let Some(attempt_id) = network_attempt_id.as_ref() { + context + .session + .register_network_approval_attempt( + attempt_id.clone(), + context.turn.sub_id.clone(), context.call_id.clone(), request.command.clone(), cwd.clone(), - Some(process_id), - Arc::clone(&transcript), - output.clone(), - exit, - wall_time, ) .await; + } - let network_policy_decision = match request.network.as_ref() { - Some(network) => { - blocking_network_policy_decision_from_attempt_or_cursor( - network, - network_attempt_id.as_deref(), - blocked_cursor, - &context.turn.sandbox_policy, - ) - .await - } - None => None, - }; + let process = self + .open_session_with_sandbox( + &request, + cwd.clone(), + context, + false, + network_attempt_id.clone(), + ) + .await; + + let process = match process { + Ok(process) => Arc::new(process), + Err(err) => { if let Some(attempt_id) = network_attempt_id.as_deref() { context .session .unregister_network_approval_attempt(attempt_id) .await; } - - if let Some((network, host)) = temporary_allowed_host { - network.revoke_temporary_allowed_host(&host).await; - } - - if let Some(network_approval_context) = next_network_retry_context( - retried_after_network_approval, - network_policy_decision.as_ref(), - ) { - let approval_decision = context - .session - .request_command_approval( - context.turn.as_ref(), - context.call_id.clone(), - request.command.clone(), - cwd.clone(), - Some(format!( - "Network access to \"{}\" is blocked by policy.", - network_approval_context.host - )), - Some(network_approval_context.clone()), - None, - ) - .await; - - match approval_decision { - ReviewDecision::Approved - | ReviewDecision::ApprovedExecpolicyAmendment { .. } - | ReviewDecision::ApprovedForSession => { - retried_after_network_approval = true; - retry_network_context = Some(network_approval_context); - continue; - } - ReviewDecision::Denied | ReviewDecision::Abort => { - self.release_process_id(&request.process_id).await; - return Err(UnifiedExecError::create_process( - "rejected by user".to_string(), - )); - } - } - } - self.release_process_id(&request.process_id).await; - process.check_for_sandbox_denial_with_text(&text).await?; - - let original_token_count = approx_token_count(&text); - let response = UnifiedExecResponse { - event_call_id: context.call_id.clone(), - chunk_id, - wall_time, - output, - raw_output: collected, - process_id: None, - exit_code, - original_token_count: Some(original_token_count), - session_command: Some(request.command.clone()), - }; - - return Ok(response); + return Err(err); } + }; - // Long‑lived command: persist the process so write_stdin can reuse - // it, and register a background watcher that will emit - // ExecCommandEnd when the PTY eventually exits (even if no further - // tool calls are made). - self.store_process( - Arc::clone(&process), - context, - &request.command, + let transcript = Arc::new(tokio::sync::Mutex::new(HeadTailBuffer::default())); + start_streaming_output(&process, context, Arc::clone(&transcript)); + + let start = Instant::now(); + // For the initial exec_command call, we both stream output to events + // (via start_streaming_output above) and collect a snapshot here for + // the tool response body. + let OutputHandles { + output_buffer, + output_notify, + output_closed, + output_closed_notify, + cancellation_token, + } = process.output_handles(); + let deadline = start + Duration::from_millis(yield_time_ms); + let collected = Self::collect_output_until_deadline( + &output_buffer, + &output_notify, + &output_closed, + &output_closed_notify, + &cancellation_token, + deadline, + ) + .await; + let wall_time = Instant::now().saturating_duration_since(start); + + let text = String::from_utf8_lossy(&collected).to_string(); + let output = formatted_truncate_text(&text, TruncationPolicy::Tokens(max_tokens)); + let exit_code = process.exit_code(); + let has_exited = process.has_exited() || exit_code.is_some(); + let chunk_id = generate_chunk_id(); + let process_id = request.process_id.clone(); + + if has_exited { + // Short‑lived command: emit ExecCommandEnd immediately using the + // same helper as the background watcher, so all end events share + // one implementation. + let exit = exit_code.unwrap_or(-1); + emit_exec_end_for_unified_exec( + Arc::clone(&context.session), + Arc::clone(&context.turn), + context.call_id.clone(), + request.command.clone(), cwd.clone(), - start, - process_id, - request.tty, + Some(process_id), Arc::clone(&transcript), + output.clone(), + exit, + wall_time, ) .await; - if let Some((network, host)) = temporary_allowed_host { - network.revoke_temporary_allowed_host(&host).await; - } + let approval_outcome = if let Some(attempt_id) = network_attempt_id.as_deref() { + context + .session + .take_network_approval_outcome(attempt_id) + .await + } else { + None + }; if let Some(attempt_id) = network_attempt_id.as_deref() { context .session @@ -413,6 +266,14 @@ impl UnifiedExecProcessManager { .await; } + self.release_process_id(&request.process_id).await; + if approval_outcome == Some(NetworkApprovalOutcome::DeniedByUser) { + return Err(UnifiedExecError::create_process( + "rejected by user".to_string(), + )); + } + process.check_for_sandbox_denial_with_text(&text).await?; + let original_token_count = approx_token_count(&text); let response = UnifiedExecResponse { event_call_id: context.call_id.clone(), @@ -420,7 +281,7 @@ impl UnifiedExecProcessManager { wall_time, output, raw_output: collected, - process_id: Some(request.process_id.clone()), + process_id: None, exit_code, original_token_count: Some(original_token_count), session_command: Some(request.command.clone()), @@ -428,6 +289,59 @@ impl UnifiedExecProcessManager { return Ok(response); } + + let approval_outcome = if let Some(attempt_id) = network_attempt_id.as_deref() { + context + .session + .take_network_approval_outcome(attempt_id) + .await + } else { + None + }; + if let Some(attempt_id) = network_attempt_id.as_deref() { + context + .session + .unregister_network_approval_attempt(attempt_id) + .await; + } + if approval_outcome == Some(NetworkApprovalOutcome::DeniedByUser) { + process.terminate(); + self.release_process_id(&request.process_id).await; + return Err(UnifiedExecError::create_process( + "rejected by user".to_string(), + )); + } + + // Long‑lived command: persist the process so write_stdin can reuse + // it, and register a background watcher that will emit + // ExecCommandEnd when the PTY eventually exits (even if no further + // tool calls are made). + self.store_process( + Arc::clone(&process), + context, + &request.command, + cwd.clone(), + start, + process_id, + request.tty, + Arc::clone(&transcript), + ) + .await; + + let original_token_count = approx_token_count(&text); + let response = UnifiedExecResponse { + event_call_id: context.call_id.clone(), + chunk_id, + wall_time, + output, + raw_output: collected, + process_id: Some(request.process_id.clone()), + exit_code, + original_token_count: Some(original_token_count), + session_command: Some(request.command.clone()), + }; + + Ok(response) } pub(crate) async fn write_stdin( @@ -905,7 +819,6 @@ enum ProcessStatus { #[cfg(test)] mod tests { use super::*; - use codex_protocol::approvals::NetworkApprovalProtocol; use pretty_assertions::assert_eq; use tokio::time::Duration; use tokio::time::Instant; @@ -1007,42 +920,4 @@ mod tests { // (10) is exited but among the last 8; we should drop the LRU outside that set. assert_eq!(candidate, Some(id(1))); } - - #[test] - fn next_network_retry_context_extracts_context_for_ask_decider() { - let payload = NetworkPolicyDecisionPayload { - decision: "ask".to_string(), - source: "decider".to_string(), - protocol: Some("http".to_string()), - host: Some("google.com".to_string()), - reason: Some("not_allowed".to_string()), - port: Some(80), - }; - - let context = next_network_retry_context(false, Some(&payload)); - - assert_eq!( - context, - Some(NetworkApprovalContext { - host: "google.com".to_string(), - protocol: NetworkApprovalProtocol::Http, - }) - ); - } - - #[test] - fn next_network_retry_context_is_none_after_retry() { - let payload = NetworkPolicyDecisionPayload { - decision: "ask".to_string(), - source: "decider".to_string(), - protocol: Some("http".to_string()), - host: Some("google.com".to_string()), - reason: Some("not_allowed".to_string()), - port: Some(80), - }; - - let context = next_network_retry_context(true, Some(&payload)); - - assert_eq!(context, None); - } }