feat(core): add requirements-gated network approval context and TUI prompt

This commit is contained in:
viyatb-oai
2026-02-11 13:14:20 -08:00
parent de6f2ef746
commit ddb4c2b28c
14 changed files with 304 additions and 9 deletions
@@ -205,6 +205,7 @@ pub(crate) async fn apply_bespoke_event_handling(
reason,
proposed_execpolicy_amendment,
parsed_cmd,
network_approval_context: _,
}) => match api_version {
ApiVersion::V1 => {
let params = ExecCommandApprovalParams {
+3
View File
@@ -169,6 +169,7 @@ use crate::protocol::Event;
use crate::protocol::EventMsg;
use crate::protocol::ExecApprovalRequestEvent;
use crate::protocol::McpServerRefreshConfig;
use crate::protocol::NetworkApprovalContext;
use crate::protocol::Op;
use crate::protocol::PlanDeltaEvent;
use crate::protocol::RateLimitSnapshot;
@@ -1964,6 +1965,7 @@ impl Session {
command: Vec<String>,
cwd: PathBuf,
reason: Option<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
) -> ReviewDecision {
// Add the tx_approve callback to the map before sending the request.
@@ -1990,6 +1992,7 @@ impl Session {
command,
cwd,
reason,
network_approval_context,
proposed_execpolicy_amendment,
parsed_cmd,
});
+2
View File
@@ -315,6 +315,7 @@ async fn handle_exec_approval(
command,
cwd,
reason,
network_approval_context,
proposed_execpolicy_amendment,
..
} = event;
@@ -326,6 +327,7 @@ async fn handle_exec_approval(
command,
cwd,
reason,
network_approval_context,
proposed_execpolicy_amendment,
);
let decision =
+161 -6
View File
@@ -20,13 +20,18 @@ use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::default_exec_approval_requirement;
use codex_otel::ToolDecisionSource;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use serde::Deserialize;
pub(crate) struct ToolOrchestrator {
sandbox: SandboxManager,
}
const NETWORK_POLICY_DECISION_PREFIX: &str = "CODEX_NETWORK_POLICY_DECISION ";
impl ToolOrchestrator {
pub fn new() -> Self {
Self {
@@ -70,6 +75,7 @@ impl ToolOrchestrator {
turn: turn_ctx,
call_id: &tool_ctx.call_id,
retry_reason: reason,
network_approval_context: None,
};
let decision = tool.start_approval_async(req, approval_ctx).await;
@@ -139,12 +145,16 @@ impl ToolOrchestrator {
// Ask for approval before retrying with the escalated sandbox.
if !tool.should_bypass_approval(approval_policy, already_approved) {
let reason_msg = build_denial_reason_from_output(output.as_ref());
let retry_details = build_denial_reason_from_output(
output.as_ref(),
should_prompt_for_network_approval(turn_ctx),
);
let approval_ctx = ApprovalCtx {
session: tool_ctx.session,
turn: turn_ctx,
call_id: &tool_ctx.call_id,
retry_reason: Some(reason_msg),
retry_reason: Some(retry_details.reason),
network_approval_context: retry_details.network_approval_context,
};
let decision = tool.start_approval_async(req, approval_ctx).await;
@@ -179,8 +189,153 @@ impl ToolOrchestrator {
}
}
fn build_denial_reason_from_output(_output: &ExecToolCallOutput) -> String {
// Keep approval reason terse and stable for UX/tests, but accept the
// output so we can evolve heuristics later without touching call sites.
"command failed; retry without sandbox?".to_string()
#[derive(Debug)]
struct RetryApprovalDetails {
reason: String,
network_approval_context: Option<NetworkApprovalContext>,
}
fn build_denial_reason_from_output(
output: &ExecToolCallOutput,
network_prompting_enabled: bool,
) -> RetryApprovalDetails {
let network_approval_context = if network_prompting_enabled {
extract_network_approval_context(output)
} else {
None
};
let reason = if let Some(network_approval_context) = network_approval_context.as_ref() {
format!(
"Network access to \"{}\" is blocked by policy.",
network_approval_context.host
)
} else {
// Keep approval reason terse and stable for UX/tests, but accept the
// output so we can evolve heuristics later without touching call sites.
"command failed; retry without sandbox?".to_string()
};
RetryApprovalDetails {
reason,
network_approval_context,
}
}
fn should_prompt_for_network_approval(turn_ctx: &crate::codex::TurnContext) -> bool {
matches!(
turn_ctx
.config
.config_layer_stack
.requirements_toml()
.network
.as_ref()
.and_then(|network| network.enabled),
Some(true)
)
}
fn extract_network_approval_context(output: &ExecToolCallOutput) -> Option<NetworkApprovalContext> {
[
output.stderr.text.as_str(),
output.stdout.text.as_str(),
output.aggregated_output.text.as_str(),
]
.into_iter()
.find_map(extract_network_approval_context_from_text)
}
fn extract_network_approval_context_from_text(text: &str) -> Option<NetworkApprovalContext> {
text.lines()
.find_map(|line| line.strip_prefix(NETWORK_POLICY_DECISION_PREFIX))
.and_then(|payload| serde_json::from_str::<NetworkPolicyDecisionPayload>(payload).ok())
.and_then(|payload| {
if payload.decision != "ask" || payload.source != "decider" {
return None;
}
let protocol = match payload.protocol.as_str() {
"http" => NetworkApprovalProtocol::Http,
"https" | "https_connect" => NetworkApprovalProtocol::Https,
_ => return None,
};
let host = payload.host.trim();
if host.is_empty() {
return None;
}
Some(NetworkApprovalContext {
host: host.to_string(),
protocol,
})
})
}
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
struct NetworkPolicyDecisionPayload {
decision: String,
source: String,
protocol: String,
host: String,
}
#[cfg(test)]
mod tests {
use super::*;
use crate::exec::StreamOutput;
use pretty_assertions::assert_eq;
fn output_with_stderr(stderr: &str) -> ExecToolCallOutput {
ExecToolCallOutput {
stderr: StreamOutput::new(stderr.to_string()),
..ExecToolCallOutput::default()
}
}
#[test]
fn build_denial_reason_extracts_network_context_when_enabled() {
let output = output_with_stderr(
"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nblocked",
);
let details = build_denial_reason_from_output(&output, true);
assert_eq!(
details.network_approval_context,
Some(NetworkApprovalContext {
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
})
);
assert_eq!(
details.reason,
"Network access to \"example.com\" is blocked by policy."
);
}
#[test]
fn build_denial_reason_skips_network_context_when_disabled() {
let output = output_with_stderr(
"CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nblocked",
);
let details = build_denial_reason_from_output(&output, false);
assert_eq!(details.network_approval_context, None);
assert_eq!(details.reason, "command failed; retry without sandbox?");
}
#[test]
fn extract_network_approval_context_ignores_non_ask_payloads() {
let text = "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"deny\",\"source\":\"decider\",\"protocol\":\"http\",\"host\":\"example.com\",\"port\":80}";
assert_eq!(extract_network_approval_context_from_text(text), None);
}
#[test]
fn extract_network_approval_context_ignores_non_decider_payloads() {
let text = "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"baseline_policy\",\"protocol\":\"http\",\"host\":\"example.com\",\"port\":80}";
assert_eq!(extract_network_approval_context_from_text(text), None);
}
}
@@ -109,6 +109,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
command,
cwd,
reason,
ctx.network_approval_context.clone(),
req.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
@@ -110,6 +110,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
command,
cwd,
reason,
ctx.network_approval_context.clone(),
req.exec_approval_requirement
.proposed_execpolicy_amendment()
.cloned(),
+2
View File
@@ -14,6 +14,7 @@ use crate::sandboxing::SandboxTransformError;
use crate::state::SessionServices;
use codex_network_proxy::NetworkProxy;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use std::collections::HashMap;
@@ -110,6 +111,7 @@ pub(crate) struct ApprovalCtx<'a> {
pub turn: &'a TurnContext,
pub call_id: &'a str,
pub retry_reason: Option<String>,
pub network_approval_context: Option<NetworkApprovalContext>,
}
// Specifies what tool orchestrator should do with a given tool call.
@@ -221,6 +221,7 @@ async fn run_codex_tool_session_inner(
reason: _,
proposed_execpolicy_amendment: _,
parsed_cmd,
network_approval_context: _,
}) => {
handle_exec_approval_request(
command,
+17
View File
@@ -37,6 +37,19 @@ impl From<Vec<String>> for ExecPolicyAmendment {
}
}
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
pub enum NetworkApprovalProtocol {
Http,
Https,
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
pub struct NetworkApprovalContext {
pub host: String,
pub protocol: NetworkApprovalProtocol,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
pub struct ExecApprovalRequestEvent {
/// Identifier for the associated exec call, if available.
@@ -52,6 +65,10 @@ pub struct ExecApprovalRequestEvent {
/// Optional human-readable reason for the approval (e.g. retry without sandbox).
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
/// Optional network context for a blocked request that can be approved.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub network_approval_context: Option<NetworkApprovalContext>,
/// Proposed execpolicy amendment that can be applied to allow future runs.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
+2
View File
@@ -53,6 +53,8 @@ pub use crate::approvals::ApplyPatchApprovalRequestEvent;
pub use crate::approvals::ElicitationAction;
pub use crate::approvals::ExecApprovalRequestEvent;
pub use crate::approvals::ExecPolicyAmendment;
pub use crate::approvals::NetworkApprovalContext;
pub use crate::approvals::NetworkApprovalProtocol;
pub use crate::request_user_input::RequestUserInputEvent;
/// Open/close tags for special user-input blocks. Used across crates to avoid
@@ -20,6 +20,7 @@ use codex_core::features::Features;
use codex_core::protocol::ElicitationAction;
use codex_core::protocol::ExecPolicyAmendment;
use codex_core::protocol::FileChange;
use codex_core::protocol::NetworkApprovalContext;
use codex_core::protocol::Op;
use codex_core::protocol::ReviewDecision;
use codex_protocol::mcp::RequestId;
@@ -42,6 +43,7 @@ pub(crate) enum ApprovalRequest {
id: String,
command: Vec<String>,
reason: Option<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
},
ApplyPatch {
@@ -108,11 +110,23 @@ impl ApprovalOverlay {
) -> (Vec<ApprovalOption>, SelectionViewParams) {
let (options, title) = match &variant {
ApprovalVariant::Exec {
network_approval_context,
proposed_execpolicy_amendment,
..
} => (
exec_options(proposed_execpolicy_amendment.clone()),
"Would you like to run the following command?".to_string(),
exec_options(
proposed_execpolicy_amendment.clone(),
network_approval_context.as_ref(),
),
network_approval_context.as_ref().map_or_else(
|| "Would you like to run the following command?".to_string(),
|network_approval_context| {
format!(
"Do you want to approve access to \"{}\"?",
network_approval_context.host
)
},
),
),
ApprovalVariant::ApplyPatch { .. } => (
patch_options(),
@@ -342,6 +356,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
id,
command,
reason,
network_approval_context,
proposed_execpolicy_amendment,
} => {
let mut header: Vec<Line<'static>> = Vec::new();
@@ -359,6 +374,7 @@ impl From<ApprovalRequest> for ApprovalRequestState {
variant: ApprovalVariant::Exec {
id,
command,
network_approval_context,
proposed_execpolicy_amendment,
},
header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })),
@@ -414,6 +430,7 @@ enum ApprovalVariant {
Exec {
id: String,
command: Vec<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
},
ApplyPatch {
@@ -447,7 +464,27 @@ impl ApprovalOption {
}
}
fn exec_options(proposed_execpolicy_amendment: Option<ExecPolicyAmendment>) -> Vec<ApprovalOption> {
fn exec_options(
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
network_approval_context: Option<&NetworkApprovalContext>,
) -> Vec<ApprovalOption> {
if network_approval_context.is_some() {
return vec![
ApprovalOption {
label: "Yes, proceed".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Approved),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
},
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'))],
},
];
}
vec![ApprovalOption {
label: "Yes, proceed".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Approved),
@@ -531,6 +568,7 @@ fn elicitation_options() -> Vec<ApprovalOption> {
mod tests {
use super::*;
use crate::app_event::AppEvent;
use codex_core::protocol::NetworkApprovalProtocol;
use pretty_assertions::assert_eq;
use tokio::sync::mpsc::unbounded_channel;
@@ -539,6 +577,7 @@ mod tests {
id: "test".to_string(),
command: vec!["echo".to_string(), "hi".to_string()],
reason: Some("reason".to_string()),
network_approval_context: None,
proposed_execpolicy_amendment: None,
}
}
@@ -581,6 +620,7 @@ mod tests {
id: "test".to_string(),
command: vec!["echo".to_string()],
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![
"echo".to_string(),
])),
@@ -619,6 +659,7 @@ mod tests {
id: "test".into(),
command,
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
};
@@ -641,6 +682,66 @@ mod tests {
);
}
#[test]
fn network_exec_options_use_expected_labels_and_hide_execpolicy_amendment() {
let network_context = NetworkApprovalContext {
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
};
let options = exec_options(
Some(ExecPolicyAmendment::new(vec!["curl".to_string()])),
Some(&network_context),
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
assert_eq!(
labels,
vec![
"Yes, proceed".to_string(),
"No, and tell Codex what to do differently".to_string(),
]
);
}
#[test]
fn network_exec_prompt_title_includes_host() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx);
let exec_request = ApprovalRequest::Exec {
id: "test".into(),
command: vec!["curl".into(), "https://example.com".into()],
reason: Some("network request blocked".into()),
network_approval_context: Some(NetworkApprovalContext {
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
}),
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["curl".into()])),
};
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);
let rendered: Vec<String> = (0..buf.area.height)
.map(|row| {
(0..buf.area.width)
.map(|col| buf[(col, row)].symbol().to_string())
.collect()
})
.collect();
assert!(
rendered
.iter()
.any(|line| line.contains("Do you want to approve access to \"example.com\"?")),
"expected network title to include host, got {rendered:?}"
);
assert!(
!rendered.iter().any(|line| line.contains("don't ask again")),
"network prompt should not show execpolicy option, got {rendered:?}"
);
}
#[test]
fn exec_history_cell_wraps_with_two_space_indent() {
let command = vec![
+1
View File
@@ -989,6 +989,7 @@ mod tests {
id: "1".to_string(),
command: vec!["echo".into(), "ok".into()],
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
}
}
+1
View File
@@ -2393,6 +2393,7 @@ impl ChatWidget {
id: ev.call_id,
command: ev.command,
reason: ev.reason,
network_approval_context: ev.network_approval_context,
proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment,
};
self.bottom_pane
+7
View File
@@ -1934,6 +1934,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
"this is a test reason such as one that would be produced by the model".into(),
),
proposed_execpolicy_amendment: None,
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -1978,6 +1979,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
"this is a test reason such as one that would be produced by the model".into(),
),
proposed_execpolicy_amendment: None,
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -2028,6 +2030,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None,
proposed_execpolicy_amendment: None,
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -4614,6 +4617,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
"hello".into(),
"world".into(),
])),
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -4667,6 +4671,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
"hello".into(),
"world".into(),
])),
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -4707,6 +4712,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -5066,6 +5072,7 @@ async fn status_widget_and_approval_modal_snapshot() {
"echo".into(),
"hello world".into(),
])),
network_approval_context: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {