mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Route network access through the shared approval pipeline (#38299)
## What changed - Represent blocked network requests as approval actions so permission hooks, automatic review, and user review use the common approval flow. - Route network requests using the active turn's review settings, including for background terminals started by an earlier turn. - Record the final applied network decision in tool telemetry without exposing the destination or assigning an approval source. - Persist deny amendments and keep the blocked request denied. ## Testing Added coverage for strict automatic review, cross-turn background network requests, deny amendment persistence, and destination-safe telemetry. GitOrigin-RevId: a2a9d106962f407ed93f4d198f40ced15f090b8e
This commit is contained in:
@@ -1,10 +1,13 @@
|
||||
//! Central approval policy-stage execution and reviewer routing.
|
||||
|
||||
use crate::command_canonicalization::canonicalize_command_for_approval;
|
||||
use crate::guardian::GuardianNetworkAccessTrigger;
|
||||
use crate::guardian::GuardianReviewContext;
|
||||
use crate::guardian::GuardianReviewOptions;
|
||||
use crate::guardian::guardian_timeout_message;
|
||||
use crate::guardian::new_guardian_review_id;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::guardian::review_approval_request_with_cancel;
|
||||
use crate::guardian::routes_approval_policy_to_guardian;
|
||||
use crate::hook_runtime::run_permission_request_hooks;
|
||||
use crate::mcp_tool_call::request_mcp_tool_user_approval;
|
||||
@@ -20,6 +23,7 @@ use crate::tools::sandboxing::ApprovalRequestReasons;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_analytics::GuardianApprovalRequestSource;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_otel::ToolDecisionSource;
|
||||
@@ -27,6 +31,7 @@ use codex_protocol::approvals::ExecPolicyAmendment;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::approvals::GuardianCommandSource;
|
||||
use codex_protocol::approvals::NetworkApprovalContext;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
@@ -40,7 +45,9 @@ use codex_utils_path_uri::PathUri;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::error;
|
||||
use tracing::warn;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ApprovalContext {
|
||||
@@ -118,6 +125,20 @@ pub(crate) enum ApprovalAction {
|
||||
allow_session_remember: bool,
|
||||
allow_persistent_approval: bool,
|
||||
},
|
||||
NetworkAccess {
|
||||
id: String,
|
||||
turn_id: String,
|
||||
environment_id: String,
|
||||
target: String,
|
||||
host: String,
|
||||
protocol: NetworkApprovalProtocol,
|
||||
port: u16,
|
||||
trigger: Option<GuardianNetworkAccessTrigger>,
|
||||
hook_command: String,
|
||||
hook_run_id: String,
|
||||
command: Vec<String>,
|
||||
cwd: AbsolutePathBuf,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Eq, Hash, PartialEq, serde::Serialize)]
|
||||
@@ -160,6 +181,14 @@ impl ApprovalAction {
|
||||
.clone()
|
||||
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
|
||||
},
|
||||
Self::NetworkAccess {
|
||||
hook_command,
|
||||
target,
|
||||
..
|
||||
} => PermissionRequestPayload::bash(
|
||||
hook_command.clone(),
|
||||
Some(format!("network-access {target}")),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -197,7 +226,7 @@ impl ApprovalAction {
|
||||
})],
|
||||
#[cfg(unix)]
|
||||
Self::Execve { .. } => Vec::new(),
|
||||
Self::McpToolCall { .. } => Vec::new(),
|
||||
Self::McpToolCall { .. } | Self::NetworkAccess { .. } => Vec::new(),
|
||||
Self::ApplyPatch {
|
||||
environment_id,
|
||||
files,
|
||||
@@ -312,6 +341,24 @@ impl ApprovalAction {
|
||||
tool_description,
|
||||
annotations,
|
||||
},
|
||||
Self::NetworkAccess {
|
||||
id,
|
||||
turn_id,
|
||||
target,
|
||||
host,
|
||||
protocol,
|
||||
port,
|
||||
trigger,
|
||||
..
|
||||
} => crate::guardian::GuardianApprovalRequest::NetworkAccess {
|
||||
id,
|
||||
turn_id,
|
||||
target,
|
||||
host,
|
||||
protocol,
|
||||
port,
|
||||
trigger,
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -408,9 +455,11 @@ impl Session {
|
||||
ctx: ApprovalContext,
|
||||
) -> Result<ReviewDecision, ToolError> {
|
||||
let is_mcp_tool_call = matches!(&action, ApprovalAction::McpToolCall { .. });
|
||||
let is_network_approval = matches!(&action, ApprovalAction::NetworkAccess { .. });
|
||||
let permission_request_run_id = match &action {
|
||||
#[cfg(unix)]
|
||||
ApprovalAction::Execve { approval_id, .. } => approval_id.clone(),
|
||||
ApprovalAction::NetworkAccess { hook_run_id, .. } => hook_run_id.clone(),
|
||||
_ if ctx.retry_reason.is_some() => format!("{}:retry", ctx.call_id),
|
||||
_ => ctx.call_id.clone(),
|
||||
};
|
||||
@@ -436,10 +485,31 @@ impl Session {
|
||||
},
|
||||
None => self.request_reviewer_approval(action, &ctx).await,
|
||||
};
|
||||
record_resolution(&ctx, &resolution);
|
||||
// Network approvals record their final telemetry after validation and persistence.
|
||||
if !is_network_approval {
|
||||
record_resolution(&ctx, &resolution);
|
||||
}
|
||||
if is_mcp_tool_call && resolution.decision == ReviewDecision::ApprovedMcpPolicyAmendment {
|
||||
return Ok(resolution.decision);
|
||||
}
|
||||
if is_network_approval {
|
||||
match (&resolution.decision, resolution.source) {
|
||||
(
|
||||
ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment,
|
||||
},
|
||||
_,
|
||||
) if network_policy_amendment.action == NetworkPolicyRuleAction::Deny => {
|
||||
return Ok(resolution.decision);
|
||||
}
|
||||
(ReviewDecision::Abort, ApprovalResolutionSource::Guardian) => {
|
||||
return Err(ToolError::Rejected(
|
||||
"automatic approval review was cancelled".to_string(),
|
||||
));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
resolution.into_tool_result()
|
||||
}
|
||||
|
||||
@@ -477,6 +547,7 @@ impl Session {
|
||||
action: ApprovalAction,
|
||||
ctx: &ApprovalContext,
|
||||
) -> ReviewDecision {
|
||||
let is_network_approval = matches!(&action, ApprovalAction::NetworkAccess { .. });
|
||||
let review_id = new_guardian_review_id();
|
||||
let action = match action.into_guardian_request() {
|
||||
Ok(action) => action,
|
||||
@@ -488,17 +559,46 @@ impl Session {
|
||||
}
|
||||
};
|
||||
|
||||
review_approval_request(
|
||||
self,
|
||||
ctx.review_context.clone(),
|
||||
review_id,
|
||||
action,
|
||||
ApprovalRequestReasons {
|
||||
approval: ctx.approval_reason.clone(),
|
||||
retry: ctx.retry_reason.clone(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
if is_network_approval {
|
||||
let review_cancel = CancellationToken::new();
|
||||
let review_cancel_guard = review_cancel.clone().drop_guard();
|
||||
let review_session = Arc::clone(self);
|
||||
let review_context = ctx.review_context.clone();
|
||||
let retry_reason = ctx.retry_reason.clone();
|
||||
let review = tokio::spawn(async move {
|
||||
review_approval_request_with_cancel(
|
||||
&review_session,
|
||||
review_context,
|
||||
review_id,
|
||||
action,
|
||||
retry_reason,
|
||||
GuardianReviewOptions {
|
||||
plugin_attribution_override: None,
|
||||
approval_request_source: GuardianApprovalRequestSource::MainTurn,
|
||||
external_cancel: Some(review_cancel),
|
||||
},
|
||||
)
|
||||
.await
|
||||
});
|
||||
let decision = review.await.unwrap_or_else(|err| {
|
||||
warn!("network Guardian review task failed: {err}");
|
||||
ReviewDecision::denied("automatic approval review could not complete")
|
||||
});
|
||||
drop(review_cancel_guard.disarm());
|
||||
decision
|
||||
} else {
|
||||
review_approval_request(
|
||||
self,
|
||||
ctx.review_context.clone(),
|
||||
review_id,
|
||||
action,
|
||||
ApprovalRequestReasons {
|
||||
approval: ctx.approval_reason.clone(),
|
||||
retry: ctx.retry_reason.clone(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
async fn request_user_approval(
|
||||
@@ -540,7 +640,9 @@ impl Session {
|
||||
#[cfg(unix)]
|
||||
ApprovalAction::Execve { .. } => unreachable!("matched command approval"),
|
||||
ApprovalAction::ApplyPatch { .. } => unreachable!("matched command approval"),
|
||||
ApprovalAction::McpToolCall { .. } => unreachable!("matched command approval"),
|
||||
ApprovalAction::McpToolCall { .. } | ApprovalAction::NetworkAccess { .. } => {
|
||||
unreachable!("matched command approval")
|
||||
}
|
||||
};
|
||||
let reason = ctx
|
||||
.retry_reason
|
||||
@@ -640,6 +742,28 @@ impl Session {
|
||||
)
|
||||
.await
|
||||
}
|
||||
ApprovalAction::NetworkAccess {
|
||||
environment_id,
|
||||
command,
|
||||
cwd,
|
||||
..
|
||||
} => {
|
||||
self.request_command_approval(
|
||||
ctx.review_context.turn(),
|
||||
ctx.call_id.clone(),
|
||||
/*approval_id*/ None,
|
||||
Some(environment_id.clone()),
|
||||
command.clone(),
|
||||
cwd.clone(),
|
||||
ctx.approval_reason.clone(),
|
||||
ctx.network_approval_context.clone(),
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
/*additional_permissions*/ None,
|
||||
/*available_decisions*/ None,
|
||||
/*plugin_attribution_override*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -655,7 +779,7 @@ fn record_resolution(ctx: &ApprovalContext, resolution: &ApprovalResolution) {
|
||||
tool_name.as_ref(),
|
||||
&ctx.call_id,
|
||||
&resolution.decision,
|
||||
source,
|
||||
Some(source),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,18 +1,12 @@
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianNetworkAccessTrigger;
|
||||
use crate::guardian::GuardianReviewOptions;
|
||||
use crate::guardian::new_guardian_review_id;
|
||||
use crate::guardian::review_approval_request_with_cancel;
|
||||
use crate::guardian::routes_approval_to_guardian;
|
||||
use crate::hook_runtime::run_permission_request_hooks;
|
||||
use crate::guardian::GuardianReviewContext;
|
||||
use crate::network_policy_decision::denied_network_policy_message;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::approvals::ApprovalAction;
|
||||
use crate::tools::approvals::ApprovalContext;
|
||||
use crate::tools::events::truncate_rejection_message;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use codex_analytics::GuardianApprovalRequestSource;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_network_proxy::BlockedRequest;
|
||||
use codex_network_proxy::BlockedRequestObserver;
|
||||
use codex_network_proxy::NetworkDecision;
|
||||
@@ -30,6 +24,7 @@ use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use codex_sandboxing::record_network_sandbox_violation;
|
||||
use codex_tools::ToolName;
|
||||
use indexmap::IndexMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
@@ -569,16 +564,6 @@ impl NetworkApprovalService {
|
||||
owner_call.cancellation_token.cancel();
|
||||
}
|
||||
|
||||
async fn active_turn_context(
|
||||
session: &Session,
|
||||
) -> Option<Arc<crate::session::turn_context::TurnContext>> {
|
||||
let active_turn = session.active_turn.lock().await;
|
||||
active_turn
|
||||
.as_ref()
|
||||
.and_then(|turn| turn.task.as_ref())
|
||||
.map(|task| Arc::clone(&task.turn_context))
|
||||
}
|
||||
|
||||
fn format_network_target(protocol: &str, host: &str, port: u16) -> String {
|
||||
format!("{protocol}://{host}:{port}")
|
||||
}
|
||||
@@ -614,11 +599,11 @@ impl NetworkApprovalService {
|
||||
else {
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
};
|
||||
let turn_context = Self::active_turn_context(session.as_ref()).await;
|
||||
let active_turn = session.active_turn_context_and_strict_auto_review().await;
|
||||
let Some(environment_id) = active_environment_id.or_else(|| {
|
||||
turn_context
|
||||
active_turn
|
||||
.as_ref()
|
||||
.and_then(|turn_context| turn_context.environments.primary())
|
||||
.and_then(|(turn_context, _)| turn_context.environments.primary())
|
||||
.map(|environment| environment.environment_id.clone())
|
||||
}) else {
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
@@ -645,7 +630,7 @@ impl NetworkApprovalService {
|
||||
format!("Network access to \"{target}\" was blocked by policy.");
|
||||
let prompt_reason = format!("{} is not in the allowed_domains", request.host);
|
||||
|
||||
let Some(turn_context) = turn_context else {
|
||||
let Some((turn_context, strict_auto_review)) = active_turn else {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(&owner_call.registration_id, policy_denial_message)
|
||||
.await;
|
||||
@@ -714,99 +699,97 @@ impl NetworkApprovalService {
|
||||
let command = owner_call
|
||||
.as_ref()
|
||||
.map_or_else(|| prompt_command.join(" "), |call| call.command.clone());
|
||||
let hook_approval_decision = match run_permission_request_hooks(
|
||||
&session,
|
||||
&turn_context,
|
||||
&hook_run_id_suffix,
|
||||
PermissionRequestPayload::bash(command, Some(format!("network-access {target}"))),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Some(PermissionRequestDecision::Allow) => Some(ReviewDecision::Approved),
|
||||
Some(PermissionRequestDecision::Deny { message }) => {
|
||||
let cwd = if let Some(owner_call) = owner_call.as_ref() {
|
||||
owner_call.trigger.cwd.clone()
|
||||
} else {
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
turn_context.cwd.clone()
|
||||
})
|
||||
};
|
||||
let approval_call_id = format!("{guardian_approval_id}#{}", Uuid::new_v4());
|
||||
let telemetry_call_id = owner_call.as_ref().map_or_else(
|
||||
|| Uuid::new_v4().to_string(),
|
||||
|call| call.trigger.call_id.clone(),
|
||||
);
|
||||
let telemetry_tool_name = owner_call.as_ref().map_or_else(
|
||||
|| "network_access".to_string(),
|
||||
|call| call.trigger.tool_name.clone(),
|
||||
);
|
||||
let action = ApprovalAction::NetworkAccess {
|
||||
id: guardian_approval_id,
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
environment_id,
|
||||
target,
|
||||
host: request.host.clone(),
|
||||
protocol,
|
||||
port: key.port,
|
||||
trigger: owner_call.as_ref().map(|call| call.trigger.clone()),
|
||||
hook_command: command,
|
||||
hook_run_id: hook_run_id_suffix,
|
||||
command: prompt_command,
|
||||
cwd,
|
||||
};
|
||||
let approval_context = ApprovalContext {
|
||||
review_context: GuardianReviewContext::from(&turn_context),
|
||||
call_id: approval_call_id,
|
||||
tool_name: ToolName::plain(telemetry_tool_name.clone()),
|
||||
strict_auto_review,
|
||||
approval_reason: Some(prompt_reason),
|
||||
retry_reason: Some(policy_denial_message.clone()),
|
||||
network_approval_context: Some(network_approval_context.clone()),
|
||||
};
|
||||
let approval_decision = match session.request_approval(action, approval_context).await {
|
||||
Ok(decision) => decision,
|
||||
Err(ToolError::Rejected(rejection)) => {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(&owner_call.registration_id, message)
|
||||
self.record_call_outcome(&owner_call.registration_id, rejection)
|
||||
.await;
|
||||
}
|
||||
turn_context.session_telemetry.tool_decision(
|
||||
&telemetry_tool_name,
|
||||
&telemetry_call_id,
|
||||
&ReviewDecision::denied("network approval was rejected"),
|
||||
/*source*/ None,
|
||||
);
|
||||
pending_owner.complete(PendingApprovalDecision::Deny);
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
}
|
||||
Err(ToolError::Codex(err)) => {
|
||||
let telemetry_decision = if matches!(
|
||||
err.details(),
|
||||
codex_protocol::error::CodexErrorDetails::TurnAborted
|
||||
) {
|
||||
ReviewDecision::Abort
|
||||
} else {
|
||||
ReviewDecision::denied("network approval failed")
|
||||
};
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
let rejection = if matches!(
|
||||
err.details(),
|
||||
codex_protocol::error::CodexErrorDetails::TurnAborted
|
||||
) {
|
||||
"rejected by user".to_string()
|
||||
} else {
|
||||
format!("Error while requesting approval: {err}")
|
||||
};
|
||||
self.record_call_outcome(&owner_call.registration_id, rejection)
|
||||
.await;
|
||||
}
|
||||
turn_context.session_telemetry.tool_decision(
|
||||
&telemetry_tool_name,
|
||||
&telemetry_call_id,
|
||||
&telemetry_decision,
|
||||
/*source*/ None,
|
||||
);
|
||||
pending_owner.complete(PendingApprovalDecision::Deny);
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
let use_guardian = routes_approval_to_guardian(&turn_context);
|
||||
let guardian_review_id = use_guardian.then(new_guardian_review_id);
|
||||
let approval_decision = if let Some(hook_approval_decision) = hook_approval_decision {
|
||||
hook_approval_decision
|
||||
} else if let Some(review_id) = guardian_review_id.clone() {
|
||||
let review_cancel = CancellationToken::new();
|
||||
let review_cancel_guard = review_cancel.clone().drop_guard();
|
||||
let review_session = Arc::clone(&session);
|
||||
let review_turn = Arc::clone(&turn_context);
|
||||
let review_request = GuardianApprovalRequest::NetworkAccess {
|
||||
id: guardian_approval_id.clone(),
|
||||
turn_id: owner_call
|
||||
.as_ref()
|
||||
.map_or_else(|| turn_context.sub_id.clone(), |call| call.turn_id.clone()),
|
||||
target: target.clone(),
|
||||
host: request.host.clone(),
|
||||
protocol,
|
||||
port: key.port,
|
||||
trigger: owner_call.as_ref().map(|call| call.trigger.clone()),
|
||||
};
|
||||
let retry_reason = Some(policy_denial_message.clone());
|
||||
let review = tokio::spawn(async move {
|
||||
review_approval_request_with_cancel(
|
||||
&review_session,
|
||||
&review_turn,
|
||||
review_id,
|
||||
review_request,
|
||||
retry_reason,
|
||||
GuardianReviewOptions {
|
||||
plugin_attribution_override: None,
|
||||
approval_request_source: GuardianApprovalRequestSource::MainTurn,
|
||||
external_cancel: Some(review_cancel),
|
||||
},
|
||||
)
|
||||
.await
|
||||
});
|
||||
let decision = review.await.unwrap_or_else(|err| {
|
||||
warn!("network Guardian review task failed: {err}");
|
||||
ReviewDecision::denied("automatic approval review could not complete")
|
||||
});
|
||||
drop(review_cancel_guard.disarm());
|
||||
decision
|
||||
} else {
|
||||
let available_decisions = None;
|
||||
let cwd = if let Some(owner_call) = owner_call.as_ref() {
|
||||
owner_call.trigger.cwd.clone()
|
||||
} else {
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
turn_context.cwd.clone()
|
||||
})
|
||||
};
|
||||
let approval_call_id = format!("{guardian_approval_id}#{}", Uuid::new_v4());
|
||||
session
|
||||
.request_command_approval(
|
||||
turn_context.as_ref(),
|
||||
approval_call_id,
|
||||
/*approval_id*/ None,
|
||||
Some(environment_id),
|
||||
prompt_command,
|
||||
cwd,
|
||||
Some(prompt_reason),
|
||||
Some(network_approval_context.clone()),
|
||||
/*proposed_execpolicy_amendment*/ None,
|
||||
/*additional_permissions*/ None,
|
||||
available_decisions,
|
||||
/*plugin_attribution_override*/ None,
|
||||
)
|
||||
.await
|
||||
};
|
||||
|
||||
let _session_policy_commit_guard = if matches!(
|
||||
@@ -820,6 +803,8 @@ impl NetworkApprovalService {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let mut telemetry_decision = approval_decision.clone();
|
||||
let mut network_policy_amendment_applied = false;
|
||||
let resolved = match approval_decision {
|
||||
ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
|
||||
if self.session_denied_hosts.lock().await.contains(&key) {
|
||||
@@ -866,6 +851,7 @@ impl NetworkApprovalService {
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
network_policy_amendment_applied = true;
|
||||
session
|
||||
.record_network_policy_amendment_message(
|
||||
&turn_context.sub_id,
|
||||
@@ -915,6 +901,7 @@ impl NetworkApprovalService {
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
network_policy_amendment_applied = true;
|
||||
session
|
||||
.record_network_policy_amendment_message(
|
||||
&turn_context.sub_id,
|
||||
@@ -949,8 +936,11 @@ impl NetworkApprovalService {
|
||||
PendingApprovalDecision::Deny
|
||||
}
|
||||
},
|
||||
ReviewDecision::ApprovedMcpPolicyAmendment => {
|
||||
error!("Network approval received ApprovedMcpPolicyAmendment");
|
||||
ReviewDecision::ApprovedMcpPolicyAmendment
|
||||
| ReviewDecision::Denied { .. }
|
||||
| ReviewDecision::TimedOut
|
||||
| ReviewDecision::Abort => {
|
||||
error!("centralized network approval returned an invalid decision");
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
@@ -960,44 +950,32 @@ impl NetworkApprovalService {
|
||||
}
|
||||
PendingApprovalDecision::Deny
|
||||
}
|
||||
ReviewDecision::Denied { rejection } => {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(&owner_call.registration_id, rejection)
|
||||
.await;
|
||||
}
|
||||
PendingApprovalDecision::Deny
|
||||
}
|
||||
ReviewDecision::TimedOut => {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
crate::guardian::guardian_timeout_message(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
PendingApprovalDecision::Deny
|
||||
}
|
||||
ReviewDecision::Abort => {
|
||||
if use_guardian {
|
||||
if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
"automatic approval review was cancelled".to_string(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
} else if let Some(owner_call) = owner_call.as_ref() {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
"rejected by user".to_string(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
PendingApprovalDecision::Deny
|
||||
}
|
||||
};
|
||||
pending_owner.set_decision_on_drop(resolved);
|
||||
|
||||
let decision_was_network_policy_amendment = matches!(
|
||||
&telemetry_decision,
|
||||
ReviewDecision::NetworkPolicyAmendment { .. }
|
||||
);
|
||||
if decision_was_network_policy_amendment && !network_policy_amendment_applied {
|
||||
telemetry_decision = match resolved {
|
||||
PendingApprovalDecision::AllowOnce => ReviewDecision::Approved,
|
||||
PendingApprovalDecision::AllowForSession => ReviewDecision::ApprovedForSession,
|
||||
PendingApprovalDecision::Deny => {
|
||||
ReviewDecision::denied("network approval was not applied")
|
||||
}
|
||||
};
|
||||
} else if matches!(resolved, PendingApprovalDecision::Deny)
|
||||
&& !decision_was_network_policy_amendment
|
||||
{
|
||||
telemetry_decision = ReviewDecision::denied("network approval was not applied");
|
||||
}
|
||||
turn_context.session_telemetry.tool_decision(
|
||||
&telemetry_tool_name,
|
||||
&telemetry_call_id,
|
||||
&telemetry_decision,
|
||||
/*source*/ None,
|
||||
);
|
||||
pending_owner.complete(resolved);
|
||||
|
||||
resolved.to_network_decision()
|
||||
|
||||
@@ -195,7 +195,7 @@ impl ToolOrchestrator {
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
&ReviewDecision::Approved,
|
||||
ToolDecisionSource::Config,
|
||||
Some(ToolDecisionSource::Config),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ use codex_protocol::approvals::NetworkPolicyRuleAction;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -26,6 +27,9 @@ use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::PathBufExt;
|
||||
@@ -204,6 +208,120 @@ async fn guardian_network_approval_preserves_action_and_outcome_routing() -> Res
|
||||
.find_map(|request| request.function_call_output_text(second_call_id))
|
||||
.context("expected denied network tool output")?;
|
||||
assert!(denied_output.contains(denial));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg_attr(
|
||||
not(target_os = "linux"),
|
||||
ignore = "requires the trusted Linux proxy bridge"
|
||||
)]
|
||||
async fn strict_auto_review_routes_network_approval_to_guardian_when_user_reviewer_is_selected()
|
||||
-> Result<()> {
|
||||
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
||||
skip_if_host_windows!(Ok(()));
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let test = managed_network_unified_exec_test_with_features(
|
||||
&server,
|
||||
&[Feature::RequestPermissionsTool],
|
||||
)
|
||||
.await?;
|
||||
let permission_call_id = "strict-network-permissions";
|
||||
let network_call_id = "strict-network-access";
|
||||
let requested_permissions = RequestPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-strict-network-permissions"),
|
||||
ev_function_call(
|
||||
permission_call_id,
|
||||
"request_permissions",
|
||||
&serde_json::to_string(&json!({
|
||||
"reason": "Require automatic review for the rest of this turn",
|
||||
"permissions": requested_permissions,
|
||||
}))?,
|
||||
),
|
||||
ev_completed("resp-strict-network-permissions"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-strict-network-command"),
|
||||
ev_function_call(
|
||||
network_call_id,
|
||||
"exec_command",
|
||||
&serde_json::to_string(&network_fetch_args(LOCAL_ENVIRONMENT_ID))?,
|
||||
),
|
||||
ev_completed("resp-strict-network-command"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-strict-command-guardian"),
|
||||
ev_assistant_message(
|
||||
"msg-strict-command-guardian",
|
||||
r#"{"risk_level":"low","user_authorization":"high","outcome":"allow","rationale":"The strict-review command is safe."}"#,
|
||||
),
|
||||
ev_completed("resp-strict-command-guardian"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-strict-network-guardian"),
|
||||
ev_assistant_message(
|
||||
"msg-strict-network-guardian",
|
||||
r#"{"risk_level":"low","user_authorization":"high","outcome":"allow","rationale":"The strict-review network request is safe."}"#,
|
||||
),
|
||||
ev_completed("resp-strict-network-guardian"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-strict-network-complete"),
|
||||
ev_assistant_message("msg-strict-network-complete", "reviewed"),
|
||||
ev_completed("resp-strict-network-complete"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_managed_network_turn(
|
||||
&test,
|
||||
"grant turn permissions, then automatically review network access",
|
||||
vec![local(test.config.cwd.clone())],
|
||||
ApprovalsReviewer::User,
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
let EventMsg::RequestPermissions(request) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::RequestPermissions(_))
|
||||
})
|
||||
.await
|
||||
else {
|
||||
unreachable!("matched request permissions event")
|
||||
};
|
||||
assert_eq!(request.call_id, permission_call_id);
|
||||
test.codex
|
||||
.submit(Op::RequestPermissionsResponse {
|
||||
id: permission_call_id.to_string(),
|
||||
response: RequestPermissionsResponse {
|
||||
permissions: request.permissions,
|
||||
scope: PermissionGrantScope::Turn,
|
||||
strict_auto_review: true,
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion_without_network_prompt(&test).await;
|
||||
|
||||
let actions = guardian_network_actions(&responses)?;
|
||||
assert_eq!(actions.len(), 1);
|
||||
assert_eq!(
|
||||
actions[0]
|
||||
.pointer("/trigger/callId")
|
||||
.and_then(Value::as_str),
|
||||
Some(network_call_id)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -412,6 +530,194 @@ async fn timed_out_guardian_network_review_uses_timeout_outcome_without_user_fal
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg_attr(
|
||||
not(target_os = "linux"),
|
||||
ignore = "requires the trusted Linux proxy bridge"
|
||||
)]
|
||||
async fn background_network_approval_uses_active_turn_after_original_turn_completes() -> Result<()>
|
||||
{
|
||||
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
||||
skip_if_host_windows!(Ok(()));
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let test = managed_network_unified_exec_test_with_features(
|
||||
&server,
|
||||
&[Feature::RequestPermissionsTool],
|
||||
)
|
||||
.await?;
|
||||
let start_call_id = "cross-turn-network-start";
|
||||
let permission_call_id = "cross-turn-network-permissions";
|
||||
let stdin_call_id = "cross-turn-network-stdin";
|
||||
let requested_permissions = RequestPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let command = format!(
|
||||
"read _; python3 -c \"import urllib.request; urllib.request.build_opener(urllib.request.ProxyHandler()).open('{NETWORK_TEST_TARGET}', timeout=2).read()\"; echo CROSS-TURN-NETWORK-COMPLETE; read _"
|
||||
);
|
||||
let mut start_args = network_exec_args(&command);
|
||||
start_args["environment_id"] = json!(LOCAL_ENVIRONMENT_ID);
|
||||
start_args["tty"] = json!(true);
|
||||
start_args["yield_time_ms"] = json!(250);
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-cross-turn-network-start"),
|
||||
ev_function_call(
|
||||
start_call_id,
|
||||
"exec_command",
|
||||
&serde_json::to_string(&start_args)?,
|
||||
),
|
||||
ev_completed("resp-cross-turn-network-start"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-cross-turn-network-first-complete"),
|
||||
ev_assistant_message("msg-cross-turn-network-first-complete", "terminal started"),
|
||||
ev_completed("resp-cross-turn-network-first-complete"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-cross-turn-network-permissions"),
|
||||
ev_function_call(
|
||||
permission_call_id,
|
||||
"request_permissions",
|
||||
&serde_json::to_string(&json!({
|
||||
"reason": "Automatically review the existing terminal's network access",
|
||||
"permissions": requested_permissions,
|
||||
}))?,
|
||||
),
|
||||
ev_completed("resp-cross-turn-network-permissions"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-cross-turn-network-stdin"),
|
||||
ev_function_call(
|
||||
stdin_call_id,
|
||||
"write_stdin",
|
||||
&serde_json::to_string(&json!({
|
||||
"session_id": 1000,
|
||||
"chars": "continue\n",
|
||||
"yield_time_ms": 1_000,
|
||||
}))?,
|
||||
),
|
||||
ev_completed("resp-cross-turn-network-stdin"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-cross-turn-network-guardian"),
|
||||
ev_assistant_message(
|
||||
"msg-cross-turn-network-guardian",
|
||||
r#"{"risk_level":"low","user_authorization":"high","outcome":"allow","rationale":"The existing terminal's network request is safe."}"#,
|
||||
),
|
||||
ev_completed("resp-cross-turn-network-guardian"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-cross-turn-network-second-complete"),
|
||||
ev_assistant_message(
|
||||
"msg-cross-turn-network-second-complete",
|
||||
"network request approved",
|
||||
),
|
||||
ev_completed("resp-cross-turn-network-second-complete"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_managed_network_turn(
|
||||
&test,
|
||||
"start a background terminal that waits before requesting network access",
|
||||
vec![local(test.config.cwd.clone())],
|
||||
ApprovalsReviewer::User,
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
let EventMsg::TurnComplete(first_turn) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await
|
||||
else {
|
||||
unreachable!("matched first turn completion")
|
||||
};
|
||||
assert_eq!(test.codex.list_background_terminals().await.len(), 1);
|
||||
|
||||
submit_managed_network_turn(
|
||||
&test,
|
||||
"allow the existing background terminal to request network access",
|
||||
vec![local(test.config.cwd.clone())],
|
||||
ApprovalsReviewer::User,
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
let EventMsg::TurnStarted(active_turn) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnStarted(_))
|
||||
})
|
||||
.await
|
||||
else {
|
||||
unreachable!("matched second turn start")
|
||||
};
|
||||
let EventMsg::RequestPermissions(request) = wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::RequestPermissions(_))
|
||||
})
|
||||
.await
|
||||
else {
|
||||
unreachable!("matched request permissions event")
|
||||
};
|
||||
assert_eq!(request.call_id, permission_call_id);
|
||||
test.codex
|
||||
.submit(Op::RequestPermissionsResponse {
|
||||
id: permission_call_id.to_string(),
|
||||
response: RequestPermissionsResponse {
|
||||
permissions: request.permissions,
|
||||
scope: PermissionGrantScope::Turn,
|
||||
strict_auto_review: true,
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
let assessment = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::GuardianAssessment(assessment)
|
||||
if assessment.status == GuardianAssessmentStatus::Approved
|
||||
) || matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
let EventMsg::GuardianAssessment(assessment) = assessment else {
|
||||
panic!("expected Guardian to approve the background terminal's network request");
|
||||
};
|
||||
assert_eq!(assessment.turn_id, active_turn.turn_id);
|
||||
assert_ne!(assessment.turn_id, first_turn.turn_id);
|
||||
wait_for_turn_complete(&test).await;
|
||||
|
||||
let actions = guardian_network_actions(&responses)?;
|
||||
assert_eq!(actions.len(), 1);
|
||||
assert_eq!(
|
||||
actions[0]
|
||||
.pointer("/trigger/callId")
|
||||
.and_then(Value::as_str),
|
||||
Some(start_call_id)
|
||||
);
|
||||
let stdin_output = responses
|
||||
.requests()
|
||||
.iter()
|
||||
.find_map(|request| request.function_call_output_text(stdin_call_id))
|
||||
.context("expected background terminal network request output")?;
|
||||
assert!(!stdin_output.contains("blocked by policy"));
|
||||
assert_eq!(
|
||||
test.codex.list_background_terminals().await.len(),
|
||||
1,
|
||||
"approved network access must not terminate the background process"
|
||||
);
|
||||
test.codex.submit(Op::CleanBackgroundTerminals).await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg_attr(
|
||||
not(target_os = "linux"),
|
||||
@@ -753,7 +1059,6 @@ async fn allowing_network_policy_amendment_persists_context_and_bypasses_prompt(
|
||||
"Allowed network rule saved in execpolicy (allowlist): codex-network-test.invalid",
|
||||
)
|
||||
}));
|
||||
|
||||
mount_exec_network_turn(
|
||||
&server,
|
||||
"resp-network-amendment-2",
|
||||
@@ -774,6 +1079,62 @@ async fn allowing_network_policy_amendment_persists_context_and_bypasses_prompt(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg_attr(
|
||||
not(target_os = "linux"),
|
||||
ignore = "requires the trusted Linux proxy bridge"
|
||||
)]
|
||||
async fn denying_network_policy_amendment_persists_and_blocks_request() -> Result<()> {
|
||||
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
||||
skip_if_host_windows!(Ok(()));
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let test = managed_network_unified_exec_test(&server).await?;
|
||||
let responses = mount_exec_network_turn(
|
||||
&server,
|
||||
"resp-network-deny-amendment",
|
||||
"network-deny-amendment",
|
||||
network_fetch_args(LOCAL_ENVIRONMENT_ID),
|
||||
)
|
||||
.await?;
|
||||
submit_managed_network_turn(
|
||||
&test,
|
||||
"persist a deny rule for this host",
|
||||
vec![local(test.config.cwd.clone())],
|
||||
ApprovalsReviewer::User,
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
let approval = expect_network_approval(&test, LOCAL_ENVIRONMENT_ID).await?;
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: Some(approval.turn_id),
|
||||
decision: ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: NetworkPolicyAmendment {
|
||||
host: NETWORK_TEST_HOST.to_string(),
|
||||
action: NetworkPolicyRuleAction::Deny,
|
||||
},
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_turn_complete(&test).await;
|
||||
|
||||
let policy = fs::read_to_string(test.home.path().join("rules/default.rules"))?;
|
||||
assert!(policy.contains(
|
||||
r#"network_rule(host="codex-network-test.invalid", protocol="http", decision="deny""#
|
||||
));
|
||||
let output = responses
|
||||
.requests()
|
||||
.iter()
|
||||
.find_map(|request| request.function_call_output_text("network-deny-amendment"))
|
||||
.context("expected denied network tool output")?;
|
||||
assert!(output.contains("rejected by user"));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[cfg_attr(
|
||||
not(target_os = "linux"),
|
||||
@@ -824,7 +1185,6 @@ async fn failed_network_policy_amendment_denies_request_and_does_not_approve_hos
|
||||
.find_map(|request| request.function_call_output_text("network-failed-amendment-1"))
|
||||
.context("expected the failed policy amendment to reject the network request")?;
|
||||
assert!(denied_output.contains("blocked by policy"));
|
||||
|
||||
mount_exec_network_turn(
|
||||
&server,
|
||||
"resp-network-failed-amendment-2",
|
||||
@@ -1550,6 +1910,13 @@ async fn approved_network_host_for_one_environment_still_prompts_in_another() ->
|
||||
}
|
||||
|
||||
async fn managed_network_unified_exec_test(server: &wiremock::MockServer) -> Result<TestCodex> {
|
||||
managed_network_unified_exec_test_with_features(server, &[]).await
|
||||
}
|
||||
|
||||
async fn managed_network_unified_exec_test_with_features(
|
||||
server: &wiremock::MockServer,
|
||||
features: &[Feature],
|
||||
) -> Result<TestCodex> {
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
fs::write(
|
||||
home.path().join("config.toml"),
|
||||
@@ -1572,6 +1939,7 @@ allow_local_binding = true
|
||||
/*exclude_slash_tmp*/ false,
|
||||
);
|
||||
let permission_profile_for_config = permission_profile.clone();
|
||||
let features = features.to_vec();
|
||||
let mut builder = test_codex()
|
||||
.with_home(home)
|
||||
.with_cloud_config_bundle(managed_network_requirements_loader())
|
||||
@@ -1581,6 +1949,12 @@ allow_local_binding = true
|
||||
.features
|
||||
.enable(Feature::UnifiedExec)
|
||||
.expect("test config should allow feature update");
|
||||
for feature in &features {
|
||||
config
|
||||
.features
|
||||
.enable(*feature)
|
||||
.expect("test config should allow feature update");
|
||||
}
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config
|
||||
.permissions
|
||||
@@ -1790,6 +2164,10 @@ fn guardian_network_actions(responses: &ResponseMock) -> Result<Vec<Value>> {
|
||||
.into_iter()
|
||||
.filter(|request| {
|
||||
request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian")
|
||||
&& request
|
||||
.message_input_texts("user")
|
||||
.iter()
|
||||
.any(|text| text.contains("\"tool\": \"network_access\""))
|
||||
})
|
||||
.map(|request| {
|
||||
let user_texts = request.message_input_texts("user");
|
||||
|
||||
@@ -4,6 +4,8 @@ use codex_features::Feature;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_otel::TelemetryAuthMode;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::approvals::NetworkPolicyAmendment;
|
||||
use codex_protocol::approvals::NetworkPolicyRuleAction;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
@@ -1123,6 +1125,73 @@ fn sandbox_outcome_assertion<'a>(
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[traced_test]
|
||||
fn network_policy_decisions_omit_source_and_destination() {
|
||||
let telemetry = SessionTelemetry::new(
|
||||
ThreadId::new(),
|
||||
"gpt-5.5",
|
||||
"gpt-5.5",
|
||||
/*account_id*/ None,
|
||||
/*account_email*/ None,
|
||||
Some(TelemetryAuthMode::ApiKey),
|
||||
"Codex_Desktop".to_string(),
|
||||
/*log_user_prompts*/ false,
|
||||
"tty".to_string(),
|
||||
SessionSource::Cli,
|
||||
);
|
||||
|
||||
for (call_id, action, expected_decision) in [
|
||||
(
|
||||
"network-allow",
|
||||
NetworkPolicyRuleAction::Allow,
|
||||
"approved_with_network_policy_allow",
|
||||
),
|
||||
(
|
||||
"network-deny",
|
||||
NetworkPolicyRuleAction::Deny,
|
||||
"denied_with_network_policy_deny",
|
||||
),
|
||||
] {
|
||||
telemetry.tool_decision(
|
||||
"exec_command",
|
||||
call_id,
|
||||
&ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: NetworkPolicyAmendment {
|
||||
host: "private.example.com".to_string(),
|
||||
action,
|
||||
},
|
||||
},
|
||||
/*source*/ None,
|
||||
);
|
||||
|
||||
logs_assert(|lines: &[&str]| {
|
||||
let line = lines
|
||||
.iter()
|
||||
.find(|line| {
|
||||
line.contains("codex.tool_decision")
|
||||
&& line.contains(&format!("call_id={call_id}"))
|
||||
})
|
||||
.ok_or_else(|| format!("missing network tool decision for {call_id}"))?;
|
||||
|
||||
if !line.contains("tool_name=exec_command") {
|
||||
return Err("missing triggering network tool name".to_string());
|
||||
}
|
||||
if !line.contains(&format!("decision={expected_decision}")) {
|
||||
return Err(format!("unexpected network tool decision for {call_id}"));
|
||||
}
|
||||
if line.contains("source=") {
|
||||
return Err("network tool decision unexpectedly included a source".to_string());
|
||||
}
|
||||
if line.contains("private.example.com") {
|
||||
return Err("network tool decision exposed the destination host".to_string());
|
||||
}
|
||||
|
||||
Ok(())
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[traced_test]
|
||||
fn sandbox_outcome_event_records_outcome() {
|
||||
@@ -1327,7 +1396,7 @@ async fn handle_shell_command_user_approved_for_session_records_tool_decision()
|
||||
|
||||
logs_assert(tool_decision_assertion(
|
||||
"user_approved_session_call",
|
||||
"approvedforsession",
|
||||
"approved_for_session",
|
||||
"user",
|
||||
));
|
||||
}
|
||||
@@ -1519,7 +1588,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision()
|
||||
|
||||
logs_assert(tool_decision_assertion(
|
||||
"sandbox_session_call",
|
||||
"approvedforsession",
|
||||
"approved_for_session",
|
||||
"user",
|
||||
));
|
||||
}
|
||||
|
||||
@@ -992,16 +992,25 @@ impl SessionTelemetry {
|
||||
tool_name: &str,
|
||||
call_id: &str,
|
||||
decision: &ReviewDecision,
|
||||
source: ToolDecisionSource,
|
||||
source: Option<ToolDecisionSource>,
|
||||
) {
|
||||
log_event!(
|
||||
self,
|
||||
event.name = "codex.tool_decision",
|
||||
tool_name = %tool_name,
|
||||
call_id = %call_id,
|
||||
decision = %decision.clone().to_string().to_lowercase(),
|
||||
source = %source.to_string(),
|
||||
);
|
||||
match source {
|
||||
Some(source) => log_event!(
|
||||
self,
|
||||
event.name = "codex.tool_decision",
|
||||
tool_name = %tool_name,
|
||||
call_id = %call_id,
|
||||
decision = %decision.to_opaque_string(),
|
||||
source = %source.to_string(),
|
||||
),
|
||||
None => log_event!(
|
||||
self,
|
||||
event.name = "codex.tool_decision",
|
||||
tool_name = %tool_name,
|
||||
call_id = %call_id,
|
||||
decision = %decision.to_opaque_string(),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn sandbox_outcome(
|
||||
|
||||
Reference in New Issue
Block a user