mirror of
https://github.com/openai/codex.git
synced 2026-11-08 08:37:30 +00:00
Tie network approval reviews to their originating execution (#44575)
## Why Background commands can outlive their launching turn. Their network reviews need the original execution environment even when current turn settings change, and pending reviews must stop when the execution is cancelled. ## What changed - Capture the execution's environment snapshot for Guardian network reviews while continuing to use current review settings. - Deny requests from already cancelled executions before checking session host grants, and resolve pending approvals as denied when execution cancellation interrupts review. - Preserve accepted review results and session grants when cancellation happens after acceptance. ## Testing Add regression coverage for same-turn and later-turn settings changes, the original review working directory, cancellation before and after review acceptance, late callbacks, background process termination, and reuse of accepted session grants by new executions. GitOrigin-RevId: 29487488da9ab43700ba6684c61eea86f30a03a8
This commit is contained in:
1 parent
6bb5be869f
commit
bfca0335fa
7 files changed
+586
-172
No files matched your search
@@ -94,13 +94,14 @@ impl GuardianReviewContext {
|
|||||||
pub(crate) fn from_resolved_settings(
|
pub(crate) fn from_resolved_settings(
|
||||||
turn: Arc<TurnContext>,
|
turn: Arc<TurnContext>,
|
||||||
settings: &ResolvedStepSettings,
|
settings: &ResolvedStepSettings,
|
||||||
|
environments: &TurnEnvironmentSnapshot,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
parent_response_id: turn
|
parent_response_id: turn
|
||||||
.extension_data
|
.extension_data
|
||||||
.get::<codex_api::ResponseId>()
|
.get::<codex_api::ResponseId>()
|
||||||
.map(|id| id.0.clone()),
|
.map(|id| id.0.clone()),
|
||||||
environments: turn.environments.clone(),
|
environments: environments.clone(),
|
||||||
model_info: Arc::clone(&settings.model_info),
|
model_info: Arc::clone(&settings.model_info),
|
||||||
reasoning_effort: settings.reasoning_effort().cloned(),
|
reasoning_effort: settings.reasoning_effort().cloned(),
|
||||||
reasoning_summary: settings.reasoning_summary,
|
reasoning_summary: settings.reasoning_summary,
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
use crate::config::NetworkProxySpec;
|
use crate::config::NetworkProxySpec;
|
||||||
|
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||||
use crate::guardian::GuardianNetworkAccessTrigger;
|
use crate::guardian::GuardianNetworkAccessTrigger;
|
||||||
use crate::guardian::GuardianReviewContext;
|
use crate::guardian::GuardianReviewContext;
|
||||||
use crate::network_policy_decision::denied_network_policy_message;
|
use crate::network_policy_decision::denied_network_policy_message;
|
||||||
@@ -241,6 +242,7 @@ struct ActiveNetworkApprovalCall {
|
|||||||
command: String,
|
command: String,
|
||||||
environment_id: String,
|
environment_id: String,
|
||||||
permission_profile: PermissionProfile,
|
permission_profile: PermissionProfile,
|
||||||
|
environments: TurnEnvironmentSnapshot,
|
||||||
cancellation_token: CancellationToken,
|
cancellation_token: CancellationToken,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -639,6 +641,12 @@ impl NetworkApprovalService {
|
|||||||
else {
|
else {
|
||||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||||
};
|
};
|
||||||
|
if owner_call
|
||||||
|
.as_ref()
|
||||||
|
.is_some_and(|call| call.cancellation_token.is_cancelled())
|
||||||
|
{
|
||||||
|
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||||
|
}
|
||||||
let active_turn = session.active_turn_context_and_strict_auto_review().await;
|
let active_turn = session.active_turn_context_and_strict_auto_review().await;
|
||||||
let Some(environment_id) = active_environment_id.or_else(|| {
|
let Some(environment_id) = active_environment_id.or_else(|| {
|
||||||
active_turn
|
active_turn
|
||||||
@@ -720,6 +728,11 @@ impl NetworkApprovalService {
|
|||||||
let review_context = GuardianReviewContext::from_resolved_settings(
|
let review_context = GuardianReviewContext::from_resolved_settings(
|
||||||
Arc::clone(&turn_context),
|
Arc::clone(&turn_context),
|
||||||
&step_settings,
|
&step_settings,
|
||||||
|
// Review this new request under current settings, in its originating execution's
|
||||||
|
// environments. Completing the original turn does not transfer process ownership.
|
||||||
|
owner_call
|
||||||
|
.as_ref()
|
||||||
|
.map_or(&turn_context.environments, |call| &call.environments),
|
||||||
);
|
);
|
||||||
if !allows_network_approval_flow(review_context.approval_policy) {
|
if !allows_network_approval_flow(review_context.approval_policy) {
|
||||||
if let Some(owner_call) = owner_call.as_ref() {
|
if let Some(owner_call) = owner_call.as_ref() {
|
||||||
@@ -791,7 +804,22 @@ impl NetworkApprovalService {
|
|||||||
retry_reason: Some(policy_denial_message.clone()),
|
retry_reason: Some(policy_denial_message.clone()),
|
||||||
network_approval_context: Some(network_approval_context.clone()),
|
network_approval_context: Some(network_approval_context.clone()),
|
||||||
};
|
};
|
||||||
let approval_decision = match session.request_approval(action, approval_context).await {
|
let approval = session.request_approval(action, approval_context);
|
||||||
|
let approval_result = if let Some(owner_call) = &owner_call {
|
||||||
|
// A ready review result takes precedence over concurrent cancellation.
|
||||||
|
let Some(result) = owner_call
|
||||||
|
.cancellation_token
|
||||||
|
.run_until_cancelled(approval)
|
||||||
|
.await
|
||||||
|
else {
|
||||||
|
pending_owner.complete(PendingApprovalDecision::Deny);
|
||||||
|
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||||
|
};
|
||||||
|
result
|
||||||
|
} else {
|
||||||
|
approval.await
|
||||||
|
};
|
||||||
|
let approval_decision = match approval_result {
|
||||||
Ok(decision) => decision,
|
Ok(decision) => decision,
|
||||||
Err(ToolError::Rejected(rejection)) => {
|
Err(ToolError::Rejected(rejection)) => {
|
||||||
if let Some(owner_call) = owner_call.as_ref() {
|
if let Some(owner_call) = owner_call.as_ref() {
|
||||||
@@ -1067,6 +1095,7 @@ pub(crate) fn build_network_policy_decider(
|
|||||||
pub(crate) async fn begin_network_approval(
|
pub(crate) async fn begin_network_approval(
|
||||||
session: &Arc<Session>,
|
session: &Arc<Session>,
|
||||||
turn: &TurnContext,
|
turn: &TurnContext,
|
||||||
|
environments: &TurnEnvironmentSnapshot,
|
||||||
managed_network_active: bool,
|
managed_network_active: bool,
|
||||||
spec: Option<NetworkApprovalSpec>,
|
spec: Option<NetworkApprovalSpec>,
|
||||||
) -> Result<Option<ActiveNetworkApproval>, ToolError> {
|
) -> Result<Option<ActiveNetworkApproval>, ToolError> {
|
||||||
@@ -1181,6 +1210,7 @@ pub(crate) async fn begin_network_approval(
|
|||||||
command,
|
command,
|
||||||
environment_id,
|
environment_id,
|
||||||
permission_profile,
|
permission_profile,
|
||||||
|
environments: environments.clone(),
|
||||||
cancellation_token: cancellation_token.clone(),
|
cancellation_token: cancellation_token.clone(),
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
|
|||||||
@@ -10,9 +10,155 @@ use core_test_support::test_path_buf;
|
|||||||
use futures::poll;
|
use futures::poll;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
use test_case::test_case;
|
||||||
use tokio::time::timeout;
|
use tokio::time::timeout;
|
||||||
use tokio_util::sync::CancellationToken;
|
use tokio_util::sync::CancellationToken;
|
||||||
|
|
||||||
|
enum ExecutionCancellationTiming {
|
||||||
|
PendingReview,
|
||||||
|
AcceptedReview,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test_case(ExecutionCancellationTiming::PendingReview; "pending_review")]
|
||||||
|
#[test_case(ExecutionCancellationTiming::AcceptedReview; "accepted_review")]
|
||||||
|
#[tokio::test]
|
||||||
|
#[allow(
|
||||||
|
clippy::await_holding_invalid_type,
|
||||||
|
reason = "hold the commit lock to cancel execution after the review decision is accepted"
|
||||||
|
)]
|
||||||
|
async fn execution_cancellation_respects_network_approval_boundary(
|
||||||
|
timing: ExecutionCancellationTiming,
|
||||||
|
) {
|
||||||
|
use crate::session::tests::make_session_and_context_with_rx;
|
||||||
|
use crate::tasks::SessionTask;
|
||||||
|
|
||||||
|
struct PendingTask;
|
||||||
|
impl SessionTask for PendingTask {
|
||||||
|
fn kind(&self) -> crate::state::TaskKind {
|
||||||
|
crate::state::TaskKind::Regular
|
||||||
|
}
|
||||||
|
|
||||||
|
fn span_name(&self) -> &'static str {
|
||||||
|
"network_approval_test"
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn run(
|
||||||
|
self: Arc<Self>,
|
||||||
|
_session: Arc<Session>,
|
||||||
|
_ctx: Arc<crate::session::turn_context::TurnContext>,
|
||||||
|
_input: Vec<crate::session::TurnInput>,
|
||||||
|
cancellation_token: CancellationToken,
|
||||||
|
) -> crate::tasks::SessionTaskResult {
|
||||||
|
cancellation_token.cancelled().await;
|
||||||
|
Ok(None)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let (session, _, events) = make_session_and_context_with_rx().await;
|
||||||
|
let (turn, _) = session
|
||||||
|
.new_turn_with_sub_id(
|
||||||
|
"active-turn".to_string(),
|
||||||
|
crate::session::SessionSettingsUpdate {
|
||||||
|
step_settings: crate::session::step_settings::StepSettingsUpdate {
|
||||||
|
approval_policy: Some(AskForApproval::OnRequest),
|
||||||
|
approvals_reviewer: Some(codex_config::types::ApprovalsReviewer::User),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
crate::session::turn_context::NewTurnContextOptions::default(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
session.start_task(turn, Vec::new(), PendingTask).await;
|
||||||
|
let service = &session.services.network_approval;
|
||||||
|
register_call_with_default_shell_trigger(service, "execution-1").await;
|
||||||
|
let request = NetworkPolicyRequest {
|
||||||
|
protocol: NetworkProtocol::Http,
|
||||||
|
host: "example.com".to_string(),
|
||||||
|
port: 80,
|
||||||
|
environment_id: Some("local".to_string()),
|
||||||
|
execution_id: Some("execution-1".to_string()),
|
||||||
|
client_addr: None,
|
||||||
|
method: None,
|
||||||
|
command: None,
|
||||||
|
exec_policy_hint: None,
|
||||||
|
disconnect: None,
|
||||||
|
cancellation: None,
|
||||||
|
};
|
||||||
|
let decision = service.handle_inline_policy_request(Arc::clone(&session), request.clone());
|
||||||
|
tokio::pin!(decision);
|
||||||
|
let approval = timeout(Duration::from_secs(5), async {
|
||||||
|
loop {
|
||||||
|
tokio::select! {
|
||||||
|
result = &mut decision => panic!("expected pending approval, got {result:?}"),
|
||||||
|
event = events.recv() => {
|
||||||
|
if let EventMsg::ExecApprovalRequest(approval) = event.unwrap().msg {
|
||||||
|
break approval;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let expected_decision = match timing {
|
||||||
|
ExecutionCancellationTiming::PendingReview => {
|
||||||
|
service.record_call_outcome("execution-1", "original execution denial".to_string());
|
||||||
|
NetworkDecision::deny("not_allowed")
|
||||||
|
}
|
||||||
|
ExecutionCancellationTiming::AcceptedReview => {
|
||||||
|
let commit_guard = service.session_policy_commit_lock.lock().await;
|
||||||
|
session
|
||||||
|
.notify_approval(
|
||||||
|
&approval.effective_approval_id(),
|
||||||
|
ReviewDecision::ApprovedForSession,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
// Consume the approval and stop at the held policy commit lock.
|
||||||
|
assert!(poll!(decision.as_mut()).is_pending());
|
||||||
|
service.record_call_outcome("execution-1", "original execution denial".to_string());
|
||||||
|
assert!(poll!(decision.as_mut()).is_pending());
|
||||||
|
drop(commit_guard);
|
||||||
|
NetworkDecision::Allow
|
||||||
|
}
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
timeout(Duration::from_secs(5), &mut decision)
|
||||||
|
.await
|
||||||
|
.expect("finish cancelled or accepted review"),
|
||||||
|
expected_decision,
|
||||||
|
);
|
||||||
|
let key =
|
||||||
|
HostApprovalKey::from_request(&request, NetworkApprovalProtocol::Http, "local".to_string());
|
||||||
|
assert_eq!(
|
||||||
|
service.session_approved_hosts.lock().await.contains(&key),
|
||||||
|
expected_decision == NetworkDecision::Allow,
|
||||||
|
);
|
||||||
|
// A late callback must not resurrect the cancelled execution, even with a host grant.
|
||||||
|
service.session_approved_hosts.lock().await.insert(key);
|
||||||
|
assert_eq!(
|
||||||
|
service
|
||||||
|
.handle_inline_policy_request(Arc::clone(&session), request)
|
||||||
|
.await,
|
||||||
|
NetworkDecision::deny("not_allowed"),
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
service.take_call_outcome("execution-1").await,
|
||||||
|
Some("original execution denial".to_string())
|
||||||
|
);
|
||||||
|
assert!(service.pending_host_approvals.lock().unwrap().is_empty());
|
||||||
|
while let Ok(event) = events.try_recv() {
|
||||||
|
assert!(!matches!(
|
||||||
|
event.msg,
|
||||||
|
EventMsg::ExecApprovalRequest(_) | EventMsg::GuardianAssessment(_)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
session
|
||||||
|
.abort_all_tasks(codex_protocol::protocol::TurnAbortReason::Interrupted)
|
||||||
|
.await;
|
||||||
|
}
|
||||||
|
|
||||||
fn pending_key(host: HostApprovalKey, turn_id: &str, execution_id: &str) -> PendingHostApprovalKey {
|
fn pending_key(host: HostApprovalKey, turn_id: &str, execution_id: &str) -> PendingHostApprovalKey {
|
||||||
PendingHostApprovalKey {
|
PendingHostApprovalKey {
|
||||||
host,
|
host,
|
||||||
@@ -492,6 +638,7 @@ async fn register_call_with_default_shell_trigger(
|
|||||||
command: "curl https://example.com".to_string(),
|
command: "curl https://example.com".to_string(),
|
||||||
environment_id: "local".to_string(),
|
environment_id: "local".to_string(),
|
||||||
permission_profile: PermissionProfile::workspace_write(),
|
permission_profile: PermissionProfile::workspace_write(),
|
||||||
|
environments: TurnEnvironmentSnapshot::default(),
|
||||||
cancellation_token: cancellation_token.clone(),
|
cancellation_token: cancellation_token.clone(),
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
@@ -522,6 +669,7 @@ async fn active_call_preserves_triggering_command_context() {
|
|||||||
command: "curl https://example.com".to_string(),
|
command: "curl https://example.com".to_string(),
|
||||||
environment_id: "remote".to_string(),
|
environment_id: "remote".to_string(),
|
||||||
permission_profile: PermissionProfile::workspace_write(),
|
permission_profile: PermissionProfile::workspace_write(),
|
||||||
|
environments: TurnEnvironmentSnapshot::default(),
|
||||||
cancellation_token: CancellationToken::new(),
|
cancellation_token: CancellationToken::new(),
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
|
|||||||
@@ -62,6 +62,7 @@ impl ToolOrchestrator {
|
|||||||
let network_approval = match begin_network_approval(
|
let network_approval = match begin_network_approval(
|
||||||
&tool_ctx.session,
|
&tool_ctx.session,
|
||||||
&tool_ctx.step_context.turn,
|
&tool_ctx.step_context.turn,
|
||||||
|
&tool_ctx.step_context.environments,
|
||||||
attempt.enforce_managed_network,
|
attempt.enforce_managed_network,
|
||||||
network_approval_spec,
|
network_approval_spec,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -293,8 +293,9 @@ impl CoreShellActionProvider {
|
|||||||
})?;
|
})?;
|
||||||
let approval_ctx = ApprovalContext {
|
let approval_ctx = ApprovalContext {
|
||||||
review_context: GuardianReviewContext::from_resolved_settings(
|
review_context: GuardianReviewContext::from_resolved_settings(
|
||||||
turn_context,
|
Arc::clone(&turn_context),
|
||||||
&step_settings,
|
&step_settings,
|
||||||
|
&turn_context.environments,
|
||||||
),
|
),
|
||||||
// The running process can outlive its launching tool or code-mode cell.
|
// The running process can outlive its launching tool or code-mode cell.
|
||||||
cancellation_token: None,
|
cancellation_token: None,
|
||||||
|
|||||||
@@ -34,7 +34,6 @@ use codex_protocol::models::ResponseItem;
|
|||||||
use codex_protocol::openai_models::AutoReviewMessages;
|
use codex_protocol::openai_models::AutoReviewMessages;
|
||||||
use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER;
|
use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER;
|
||||||
use codex_protocol::openai_models::ModelTokenBudgetConfig;
|
use codex_protocol::openai_models::ModelTokenBudgetConfig;
|
||||||
use codex_protocol::openai_models::ModelVisibility;
|
|
||||||
use codex_protocol::openai_models::ModelsResponse;
|
use codex_protocol::openai_models::ModelsResponse;
|
||||||
use codex_protocol::permissions::FileSystemAccessMode;
|
use codex_protocol::permissions::FileSystemAccessMode;
|
||||||
use codex_protocol::permissions::FileSystemPath;
|
use codex_protocol::permissions::FileSystemPath;
|
||||||
@@ -92,6 +91,7 @@ use wiremock::http::Method;
|
|||||||
use wiremock::matchers::method;
|
use wiremock::matchers::method;
|
||||||
use wiremock::matchers::path_regex;
|
use wiremock::matchers::path_regex;
|
||||||
|
|
||||||
|
use super::network_approval::guardian_parent_catalog;
|
||||||
use super::rmcp_client::remote_aware_environment_id;
|
use super::rmcp_client::remote_aware_environment_id;
|
||||||
use super::rmcp_client::remote_aware_stdio_server_bin;
|
use super::rmcp_client::remote_aware_stdio_server_bin;
|
||||||
|
|
||||||
@@ -951,44 +951,6 @@ async fn guardian_node_repl_policy_follows_production_approval_path(
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
fn guardian_parent_catalog() -> ModelsResponse {
|
|
||||||
let template = codex_models_manager::bundled_models_response()
|
|
||||||
.expect("bundled model catalog")
|
|
||||||
.models
|
|
||||||
.into_iter()
|
|
||||||
.find(|model| model.slug == "gpt-5.4")
|
|
||||||
.expect("gpt-5.4 in bundled catalog");
|
|
||||||
// Keep safety settings compatible so active publication can switch A to B.
|
|
||||||
ModelsResponse {
|
|
||||||
models: ["guardian-parent-a", "guardian-parent-b"]
|
|
||||||
.into_iter()
|
|
||||||
.map(|slug| {
|
|
||||||
let mut model = template.clone();
|
|
||||||
model.slug = slug.to_string();
|
|
||||||
model.visibility = ModelVisibility::List;
|
|
||||||
model.auto_review_model_override = None;
|
|
||||||
model.supported_reasoning_levels.retain(|level| {
|
|
||||||
level.effort != codex_protocol::openai_models::ReasoningEffort::Low
|
|
||||||
});
|
|
||||||
model
|
|
||||||
.model_messages
|
|
||||||
.as_mut()
|
|
||||||
.expect("model messages")
|
|
||||||
.auto_review = Some(AutoReviewMessages {
|
|
||||||
policy: Some("captured policy".to_string()),
|
|
||||||
policy_template: Some(
|
|
||||||
"captured template: {{ tenant_policy_config }}".to_string(),
|
|
||||||
),
|
|
||||||
node_repl_policy: None,
|
|
||||||
rejection_instructions: None,
|
|
||||||
timeout_instructions: None,
|
|
||||||
});
|
|
||||||
model
|
|
||||||
})
|
|
||||||
.collect(),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn guardian_reviews_delayed_and_new_actions_after_catalog_refresh() -> Result<()> {
|
async fn guardian_reviews_delayed_and_new_actions_after_catalog_refresh() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|||||||
@@ -27,6 +27,9 @@ use codex_protocol::config_types::WindowsSandboxLevel;
|
|||||||
use codex_protocol::models::NetworkPermissions;
|
use codex_protocol::models::NetworkPermissions;
|
||||||
use codex_protocol::models::PermissionProfile;
|
use codex_protocol::models::PermissionProfile;
|
||||||
use codex_protocol::models::PermissionProfileSnapshot;
|
use codex_protocol::models::PermissionProfileSnapshot;
|
||||||
|
use codex_protocol::openai_models::AutoReviewMessages;
|
||||||
|
use codex_protocol::openai_models::ModelVisibility;
|
||||||
|
use codex_protocol::openai_models::ModelsResponse;
|
||||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||||
use codex_protocol::protocol::AskForApproval;
|
use codex_protocol::protocol::AskForApproval;
|
||||||
use codex_protocol::protocol::EnvironmentConfigState;
|
use codex_protocol::protocol::EnvironmentConfigState;
|
||||||
@@ -64,6 +67,7 @@ use core_test_support::skip_if_no_remote_env;
|
|||||||
use core_test_support::skip_if_sandbox;
|
use core_test_support::skip_if_sandbox;
|
||||||
use core_test_support::skip_if_target_windows;
|
use core_test_support::skip_if_target_windows;
|
||||||
use core_test_support::test_codex::TestCodex;
|
use core_test_support::test_codex::TestCodex;
|
||||||
|
use core_test_support::test_codex::TestCodexBuilder;
|
||||||
use core_test_support::test_codex::local;
|
use core_test_support::test_codex::local;
|
||||||
use core_test_support::test_codex::test_codex;
|
use core_test_support::test_codex::test_codex;
|
||||||
use core_test_support::test_codex::turn_permission_fields;
|
use core_test_support::test_codex::turn_permission_fields;
|
||||||
@@ -661,23 +665,31 @@ async fn timed_out_guardian_network_review_uses_timeout_outcome_without_user_fal
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test_case(true; "same_turn")]
|
||||||
|
#[test_case(false; "later_turn")]
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
#[cfg_attr(
|
#[cfg_attr(
|
||||||
not(target_os = "linux"),
|
not(target_os = "linux"),
|
||||||
ignore = "requires the trusted Linux proxy bridge"
|
ignore = "requires the trusted Linux proxy bridge"
|
||||||
)]
|
)]
|
||||||
async fn background_network_approval_uses_active_turn_after_original_turn_completes() -> Result<()>
|
async fn background_network_approval_uses_current_review_settings_and_original_execution(
|
||||||
{
|
same_turn: bool,
|
||||||
|
) -> Result<()> {
|
||||||
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
||||||
skip_if_host_windows!(Ok(()));
|
skip_if_host_windows!(Ok(()));
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
skip_if_sandbox!(Ok(()));
|
skip_if_sandbox!(Ok(()));
|
||||||
|
|
||||||
let server = start_mock_server().await;
|
let server = start_mock_server().await;
|
||||||
let test = managed_network_unified_exec_test_with_features(
|
let test = managed_network_unified_exec_builder(&[
|
||||||
&server,
|
Feature::RequestPermissionsTool,
|
||||||
&[Feature::RequestPermissionsTool],
|
Feature::StepModelSwitching,
|
||||||
)
|
])?
|
||||||
|
.with_model("guardian-parent-a")
|
||||||
|
.with_config(|config| {
|
||||||
|
config.model_catalog = Some(guardian_parent_catalog());
|
||||||
|
})
|
||||||
|
.build_with_remote_and_local_env(&server)
|
||||||
.await?;
|
.await?;
|
||||||
let start_call_id = "cross-turn-network-start";
|
let start_call_id = "cross-turn-network-start";
|
||||||
let permission_call_id = "cross-turn-network-permissions";
|
let permission_call_id = "cross-turn-network-permissions";
|
||||||
@@ -689,73 +701,84 @@ async fn background_network_approval_uses_active_turn_after_original_turn_comple
|
|||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
let command = format!(
|
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 _"
|
"read _; {}; echo CROSS-TURN-NETWORK-COMPLETE; read _",
|
||||||
|
remote_network_proxy_request_command("NETWORK-RESULT")
|
||||||
);
|
);
|
||||||
let mut start_args = network_exec_args(&command);
|
let mut start_args = network_exec_args(&command);
|
||||||
start_args["environment_id"] = json!(LOCAL_ENVIRONMENT_ID);
|
start_args["environment_id"] = json!(LOCAL_ENVIRONMENT_ID);
|
||||||
start_args["tty"] = json!(true);
|
start_args["tty"] = json!(true);
|
||||||
start_args["yield_time_ms"] = json!(250);
|
start_args["yield_time_ms"] = json!(250);
|
||||||
let responses = mount_sse_sequence(
|
let mut events = vec![
|
||||||
&server,
|
sse(vec![
|
||||||
vec![
|
ev_response_created("resp-cross-turn-network-start"),
|
||||||
sse(vec![
|
ev_function_call(
|
||||||
ev_response_created("resp-cross-turn-network-start"),
|
start_call_id,
|
||||||
ev_function_call(
|
"exec_command",
|
||||||
start_call_id,
|
&serde_json::to_string(&start_args)?,
|
||||||
"exec_command",
|
),
|
||||||
&serde_json::to_string(&start_args)?,
|
ev_completed("resp-cross-turn-network-start"),
|
||||||
),
|
]),
|
||||||
ev_completed("resp-cross-turn-network-start"),
|
sse(vec![
|
||||||
]),
|
ev_response_created("resp-cross-turn-network-first-complete"),
|
||||||
sse(vec![
|
ev_assistant_message("msg-cross-turn-network-first-complete", "terminal started"),
|
||||||
ev_response_created("resp-cross-turn-network-first-complete"),
|
ev_completed("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"),
|
||||||
sse(vec![
|
ev_function_call(
|
||||||
ev_response_created("resp-cross-turn-network-permissions"),
|
permission_call_id,
|
||||||
ev_function_call(
|
"request_permissions",
|
||||||
permission_call_id,
|
&serde_json::to_string(&json!({
|
||||||
"request_permissions",
|
"reason": "Automatically review the existing terminal's network access",
|
||||||
&serde_json::to_string(&json!({
|
"permissions": requested_permissions,
|
||||||
"reason": "Automatically review the existing terminal's network access",
|
}))?,
|
||||||
"permissions": requested_permissions,
|
),
|
||||||
}))?,
|
ev_completed("resp-cross-turn-network-permissions"),
|
||||||
),
|
]),
|
||||||
ev_completed("resp-cross-turn-network-permissions"),
|
sse(vec![
|
||||||
]),
|
ev_response_created("resp-cross-turn-network-stdin"),
|
||||||
sse(vec![
|
ev_function_call(
|
||||||
ev_response_created("resp-cross-turn-network-stdin"),
|
stdin_call_id,
|
||||||
ev_function_call(
|
"write_stdin",
|
||||||
stdin_call_id,
|
&serde_json::to_string(&json!({
|
||||||
"write_stdin",
|
"session_id": 1000,
|
||||||
&serde_json::to_string(&json!({
|
"chars": "continue\n",
|
||||||
"session_id": 1000,
|
"yield_time_ms": 10_000,
|
||||||
"chars": "continue\n",
|
}))?,
|
||||||
"yield_time_ms": 1_000,
|
),
|
||||||
}))?,
|
ev_completed("resp-cross-turn-network-stdin"),
|
||||||
),
|
]),
|
||||||
ev_completed("resp-cross-turn-network-stdin"),
|
sse(vec![
|
||||||
]),
|
ev_response_created("resp-network-guardian-environment"),
|
||||||
sse(vec![
|
ev_function_call(
|
||||||
ev_response_created("resp-cross-turn-network-guardian"),
|
"guardian-pwd",
|
||||||
ev_assistant_message(
|
"exec_command",
|
||||||
"msg-cross-turn-network-guardian",
|
&network_exec_args("pwd").to_string(),
|
||||||
r#"{"risk_level":"low","user_authorization":"high","outcome":"allow","rationale":"The existing terminal's network request is safe."}"#,
|
),
|
||||||
),
|
ev_completed("resp-network-guardian-environment"),
|
||||||
ev_completed("resp-cross-turn-network-guardian"),
|
]),
|
||||||
]),
|
sse(vec![
|
||||||
sse(vec![
|
ev_response_created("resp-cross-turn-network-guardian"),
|
||||||
ev_response_created("resp-cross-turn-network-second-complete"),
|
ev_assistant_message(
|
||||||
ev_assistant_message(
|
"msg-cross-turn-network-guardian",
|
||||||
"msg-cross-turn-network-second-complete",
|
r#"{"risk_level":"low","user_authorization":"high","outcome":"allow","rationale":"The existing terminal's network request is safe."}"#,
|
||||||
"network request approved",
|
),
|
||||||
),
|
ev_completed("resp-cross-turn-network-guardian"),
|
||||||
ev_completed("resp-cross-turn-network-second-complete"),
|
]),
|
||||||
]),
|
sse(vec![
|
||||||
],
|
ev_response_created("resp-cross-turn-network-second-complete"),
|
||||||
)
|
ev_assistant_message(
|
||||||
.await;
|
"msg-cross-turn-network-second-complete",
|
||||||
|
"network request approved",
|
||||||
|
),
|
||||||
|
ev_completed("resp-cross-turn-network-second-complete"),
|
||||||
|
]),
|
||||||
|
];
|
||||||
|
if same_turn {
|
||||||
|
// Omit the first completion so the original turn stays active.
|
||||||
|
events.remove(/*index*/ 1);
|
||||||
|
}
|
||||||
|
let responses = mount_sse_sequence(&server, events).await;
|
||||||
|
|
||||||
submit_managed_network_turn(
|
submit_managed_network_turn(
|
||||||
&test,
|
&test,
|
||||||
@@ -765,29 +788,54 @@ async fn background_network_approval_uses_active_turn_after_original_turn_comple
|
|||||||
AskForApproval::OnRequest,
|
AskForApproval::OnRequest,
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
let EventMsg::TurnComplete(first_turn) = wait_for_event(&test.codex, |event| {
|
let EventMsg::TurnStarted(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(_))
|
matches!(event, EventMsg::TurnStarted(_))
|
||||||
})
|
})
|
||||||
.await
|
.await
|
||||||
else {
|
else {
|
||||||
unreachable!("matched second turn start")
|
unreachable!("matched first turn start")
|
||||||
|
};
|
||||||
|
let current_cwd = test.cwd.path().join("later-turn");
|
||||||
|
fs::create_dir(¤t_cwd)?;
|
||||||
|
let active_turn_id = if same_turn {
|
||||||
|
first_turn.turn_id.clone()
|
||||||
|
} else {
|
||||||
|
wait_for_turn_complete(&test).await;
|
||||||
|
assert_eq!(test.codex.list_background_terminals().await.len(), 1);
|
||||||
|
test.codex
|
||||||
|
.start_or_steer_turn(
|
||||||
|
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||||
|
text: "review the existing terminal under B".to_string(),
|
||||||
|
text_elements: Vec::new(),
|
||||||
|
}])
|
||||||
|
.with_thread_settings(ThreadSettingsOverrides {
|
||||||
|
environments: Some(TurnEnvironmentSelections::new(
|
||||||
|
current_cwd.abs(),
|
||||||
|
vec![local(current_cwd.abs())],
|
||||||
|
)),
|
||||||
|
collaboration_mode: Some(CollaborationMode {
|
||||||
|
mode: ModeKind::Default,
|
||||||
|
settings: Settings {
|
||||||
|
model: "guardian-parent-b".to_string(),
|
||||||
|
reasoning_effort: Some(
|
||||||
|
codex_protocol::openai_models::ReasoningEffort::Medium,
|
||||||
|
),
|
||||||
|
developer_instructions: None,
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
..Default::default()
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
let EventMsg::TurnStarted(active_turn) = wait_for_event(&test.codex, |event| {
|
||||||
|
matches!(event, EventMsg::TurnStarted(_))
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
else {
|
||||||
|
unreachable!("matched second turn start")
|
||||||
|
};
|
||||||
|
assert_ne!(active_turn.turn_id, first_turn.turn_id);
|
||||||
|
active_turn.turn_id
|
||||||
};
|
};
|
||||||
let EventMsg::RequestPermissions(request) = wait_for_event(&test.codex, |event| {
|
let EventMsg::RequestPermissions(request) = wait_for_event(&test.codex, |event| {
|
||||||
matches!(event, EventMsg::RequestPermissions(_))
|
matches!(event, EventMsg::RequestPermissions(_))
|
||||||
@@ -797,6 +845,24 @@ async fn background_network_approval_uses_active_turn_after_original_turn_comple
|
|||||||
unreachable!("matched request permissions event")
|
unreachable!("matched request permissions event")
|
||||||
};
|
};
|
||||||
assert_eq!(request.call_id, permission_call_id);
|
assert_eq!(request.call_id, permission_call_id);
|
||||||
|
if same_turn {
|
||||||
|
let (reply, outcome) = tokio::sync::oneshot::channel();
|
||||||
|
test.codex
|
||||||
|
.submit(Op::TurnSettings {
|
||||||
|
turn_id: active_turn_id.clone(),
|
||||||
|
update: codex_protocol::protocol::TurnSettingsUpdate {
|
||||||
|
model: Some("guardian-parent-b".to_string()),
|
||||||
|
effort: Some(Some(codex_protocol::openai_models::ReasoningEffort::Medium)),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
reply,
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
assert_eq!(
|
||||||
|
tokio::time::timeout(Duration::from_secs(10), outcome).await??,
|
||||||
|
codex_protocol::protocol::TurnSettingsUpdateOutcome::Applied,
|
||||||
|
);
|
||||||
|
}
|
||||||
test.codex
|
test.codex
|
||||||
.submit(Op::RequestPermissionsResponse {
|
.submit(Op::RequestPermissionsResponse {
|
||||||
id: permission_call_id.to_string(),
|
id: permission_call_id.to_string(),
|
||||||
@@ -819,26 +885,77 @@ async fn background_network_approval_uses_active_turn_after_original_turn_comple
|
|||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
let EventMsg::GuardianAssessment(assessment) = assessment else {
|
let EventMsg::GuardianAssessment(assessment) = assessment else {
|
||||||
panic!("expected Guardian to approve the background terminal's network request");
|
panic!("expected Guardian approval, got {assessment:?}");
|
||||||
};
|
};
|
||||||
assert_eq!(assessment.turn_id, active_turn.turn_id);
|
assert_eq!(assessment.turn_id, active_turn_id);
|
||||||
assert_ne!(assessment.turn_id, first_turn.turn_id);
|
|
||||||
wait_for_turn_complete(&test).await;
|
wait_for_turn_complete(&test).await;
|
||||||
|
|
||||||
let actions = guardian_network_actions(&responses)?;
|
let actions = guardian_network_actions(&responses)?;
|
||||||
assert_eq!(actions.len(), 1);
|
assert_eq!(actions.len(), 2);
|
||||||
|
assert_eq!(actions[0], actions[1]);
|
||||||
|
let original_command = get_shell(ShellType::Sh)
|
||||||
|
.context("expected local sh")?
|
||||||
|
.derive_exec_args(&command, /*use_login_shell*/ false);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
actions[0]
|
actions[0]["trigger"],
|
||||||
.pointer("/trigger/callId")
|
json!({
|
||||||
.and_then(Value::as_str),
|
"callId": start_call_id,
|
||||||
Some(start_call_id)
|
"toolName": "exec_command",
|
||||||
|
"command": original_command,
|
||||||
|
"cwd": test.config.cwd,
|
||||||
|
"sandboxPermissions": "use_default",
|
||||||
|
"tty": true,
|
||||||
|
})
|
||||||
);
|
);
|
||||||
|
let requests = responses.requests();
|
||||||
|
let reviews = requests
|
||||||
|
.iter()
|
||||||
|
.filter(|request| request.body_json()["client_metadata"]["x-openai-subagent"] == "guardian")
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
assert_eq!(reviews.len(), 2);
|
||||||
|
for review in &reviews {
|
||||||
|
assert_eq!(review.body_json()["model"], "guardian-parent-b");
|
||||||
|
assert_eq!(review.body_json()["reasoning"]["effort"], "medium");
|
||||||
|
}
|
||||||
|
let parent_requests = requests
|
||||||
|
.iter()
|
||||||
|
.filter(|request| request.body_json()["client_metadata"]["x-openai-subagent"] != "guardian")
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
assert_eq!(parent_requests[0].body_json()["model"], "guardian-parent-a");
|
||||||
|
let final_parent = parent_requests
|
||||||
|
.last()
|
||||||
|
.context("parent completion request")?;
|
||||||
|
assert_eq!(final_parent.body_json()["model"], "guardian-parent-b");
|
||||||
|
let metadata: Value = serde_json::from_str(
|
||||||
|
final_parent.body_json()["client_metadata"]["x-codex-turn-metadata"]
|
||||||
|
.as_str()
|
||||||
|
.context("Responses turn metadata")?,
|
||||||
|
)?;
|
||||||
|
// Strict review does not change this field's policy-routing meaning.
|
||||||
|
assert_eq!(metadata["auto_review_enabled"], false);
|
||||||
|
let pwd_output = reviews[1]
|
||||||
|
.function_call_output_text("guardian-pwd")
|
||||||
|
.context("Guardian pwd output")?;
|
||||||
|
let original_cwd = test
|
||||||
|
.config
|
||||||
|
.cwd
|
||||||
|
.as_path()
|
||||||
|
.to_str()
|
||||||
|
.context("UTF-8 original cwd")?;
|
||||||
|
let current_cwd = current_cwd.to_str().context("UTF-8 current cwd")?;
|
||||||
|
assert!(pwd_output.contains(original_cwd));
|
||||||
|
assert!(!pwd_output.contains(current_cwd));
|
||||||
let stdin_output = responses
|
let stdin_output = responses
|
||||||
.requests()
|
.requests()
|
||||||
.iter()
|
.iter()
|
||||||
.find_map(|request| request.function_call_output_text(stdin_call_id))
|
.find_map(|request| request.function_call_output_text(stdin_call_id))
|
||||||
.context("expected background terminal network request output")?;
|
.context("expected background terminal network request output")?;
|
||||||
assert!(!stdin_output.contains("blocked by policy"));
|
assert!(!stdin_output.contains("blocked by policy"));
|
||||||
|
assert!(
|
||||||
|
stdin_output.contains("NETWORK-RESULT:HTTP/1.1 502")
|
||||||
|
|| stdin_output.contains("NETWORK-RESULT:HTTP/1.1 200")
|
||||||
|
);
|
||||||
|
assert!(stdin_output.contains("CROSS-TURN-NETWORK-COMPLETE"));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
test.codex.list_background_terminals().await.len(),
|
test.codex.list_background_terminals().await.len(),
|
||||||
1,
|
1,
|
||||||
@@ -1036,12 +1153,24 @@ async fn user_network_approval_once_session_and_denial_semantics() -> Result<()>
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Clone, Copy)]
|
||||||
|
enum OtherNetworkReview {
|
||||||
|
Denied,
|
||||||
|
Pending,
|
||||||
|
ApprovedForSession,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test_case(OtherNetworkReview::Denied; "latest_rejection")]
|
||||||
|
#[test_case(OtherNetworkReview::Pending; "pending_review")]
|
||||||
|
#[test_case(OtherNetworkReview::ApprovedForSession; "accepted_review")]
|
||||||
#[tokio::test(flavor = "current_thread")]
|
#[tokio::test(flavor = "current_thread")]
|
||||||
#[cfg_attr(
|
#[cfg_attr(
|
||||||
not(target_os = "linux"),
|
not(target_os = "linux"),
|
||||||
ignore = "requires the trusted Linux proxy bridge"
|
ignore = "requires the trusted Linux proxy bridge"
|
||||||
)]
|
)]
|
||||||
async fn latest_network_rejection_wins_for_multiple_reviews_of_one_execution() -> Result<()> {
|
async fn network_rejection_preserves_execution_and_review_outcomes(
|
||||||
|
other_review: OtherNetworkReview,
|
||||||
|
) -> Result<()> {
|
||||||
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture");
|
||||||
skip_if_host_windows!(Ok(()));
|
skip_if_host_windows!(Ok(()));
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
@@ -1049,20 +1178,62 @@ async fn latest_network_rejection_wins_for_multiple_reviews_of_one_execution() -
|
|||||||
|
|
||||||
let server = start_mock_server().await;
|
let server = start_mock_server().await;
|
||||||
let test = managed_network_unified_exec_test(&server).await?;
|
let test = managed_network_unified_exec_test(&server).await?;
|
||||||
|
let local_cwd = test.cwd.path().abs();
|
||||||
let call_id = "network-multiple-reviews";
|
let call_id = "network-multiple-reviews";
|
||||||
|
let poll_call_id = "network-multiple-reviews-poll";
|
||||||
let second_target = format!("http://{NETWORK_TEST_HOST}:81");
|
let second_target = format!("http://{NETWORK_TEST_HOST}:81");
|
||||||
let command = format!(
|
let command = format!(
|
||||||
"python3 -c \"import threading,urllib.request; threads=[threading.Thread(target=urllib.request.build_opener(urllib.request.ProxyHandler()).open,args=(url,),kwargs={{'timeout': 10}}) for url in ('{NETWORK_TEST_TARGET}','{second_target}')]; [thread.start() for thread in threads]; [thread.join() for thread in threads]\""
|
r#"read _; python3 - <<'PY'
|
||||||
|
import fcntl, os, pathlib, socket, threading, urllib.parse
|
||||||
|
process_lock = open('network-process-lock', 'w')
|
||||||
|
fcntl.flock(process_lock, fcntl.LOCK_EX)
|
||||||
|
proxy = urllib.parse.urlparse(os.environ['HTTP_PROXY'])
|
||||||
|
def fetch(port):
|
||||||
|
with socket.create_connection((proxy.hostname, proxy.port), timeout=30) as sock:
|
||||||
|
sock.sendall(f'GET http://{NETWORK_TEST_HOST}:{{port}}/ HTTP/1.1\r\nHost: {NETWORK_TEST_HOST}:{{port}}\r\nConnection: close\r\n\r\n'.encode())
|
||||||
|
status = sock.makefile('rb').readline().decode().strip()
|
||||||
|
result = pathlib.Path(f'network-result-{{port}}')
|
||||||
|
result.with_suffix('.tmp').write_text(status)
|
||||||
|
result.with_suffix('.tmp').replace(result)
|
||||||
|
threads = [threading.Thread(target=fetch, args=(port,)) for port in (80, 81)]
|
||||||
|
for thread in threads:
|
||||||
|
thread.start()
|
||||||
|
for thread in threads:
|
||||||
|
thread.join()
|
||||||
|
threading.Event().wait()
|
||||||
|
PY"#
|
||||||
);
|
);
|
||||||
let mut args = network_exec_args(&command);
|
let mut args = network_exec_args(&command);
|
||||||
args["yield_time_ms"] = json!(10_000);
|
args["yield_time_ms"] = json!(250);
|
||||||
let responses =
|
args["tty"] = json!(true);
|
||||||
mount_exec_network_turn(&server, "resp-network-multiple-reviews", call_id, args).await?;
|
let responses = mount_sse_sequence(
|
||||||
|
&server,
|
||||||
|
vec![
|
||||||
|
sse(vec![
|
||||||
|
ev_function_call(call_id, "exec_command", &args.to_string()),
|
||||||
|
ev_completed("network-start"),
|
||||||
|
]),
|
||||||
|
sse(vec![
|
||||||
|
ev_function_call(
|
||||||
|
poll_call_id,
|
||||||
|
"write_stdin",
|
||||||
|
&json!({"session_id": 1000, "chars": "start\n", "yield_time_ms": 10_000})
|
||||||
|
.to_string(),
|
||||||
|
),
|
||||||
|
ev_completed("network-poll"),
|
||||||
|
]),
|
||||||
|
sse(vec![
|
||||||
|
ev_assistant_message("network-done", "done"),
|
||||||
|
ev_completed("network-done"),
|
||||||
|
]),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
submit_managed_network_turn(
|
submit_managed_network_turn(
|
||||||
&test,
|
&test,
|
||||||
"review both network requests from one execution",
|
"review both network requests from one execution",
|
||||||
vec![local(test.config.cwd.clone())],
|
vec![local(local_cwd.clone())],
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
AskForApproval::OnRequest,
|
AskForApproval::OnRequest,
|
||||||
)
|
)
|
||||||
@@ -1086,40 +1257,107 @@ async fn latest_network_rejection_wins_for_multiple_reviews_of_one_execution() -
|
|||||||
};
|
};
|
||||||
approvals.push(approval);
|
approvals.push(approval);
|
||||||
}
|
}
|
||||||
|
assert_eq!(test.codex.list_background_terminals().await.len(), 1);
|
||||||
|
// The child holds this lock until exit, including across the sandbox's PID namespace.
|
||||||
|
let process_lock = fs::File::open(local_cwd.join("network-process-lock"))?;
|
||||||
|
assert!(matches!(
|
||||||
|
process_lock.try_lock(),
|
||||||
|
Err(fs::TryLockError::WouldBlock)
|
||||||
|
));
|
||||||
assert_eq!(approvals[0].turn_id, approvals[1].turn_id);
|
assert_eq!(approvals[0].turn_id, approvals[1].turn_id);
|
||||||
let mut actual_targets = approvals
|
approvals.sort_by_key(|approval| approval.command[1].clone());
|
||||||
|
let actual_targets = approvals
|
||||||
.iter()
|
.iter()
|
||||||
.map(|approval| approval.command[1].clone())
|
.map(|approval| approval.command[1].clone())
|
||||||
.collect::<Vec<_>>();
|
.collect::<Vec<_>>();
|
||||||
actual_targets.sort();
|
|
||||||
let mut expected_targets = vec![NETWORK_TEST_TARGET.to_string(), second_target];
|
let mut expected_targets = vec![NETWORK_TEST_TARGET.to_string(), second_target];
|
||||||
expected_targets.sort();
|
expected_targets.sort();
|
||||||
assert_eq!(actual_targets, expected_targets);
|
assert_eq!(actual_targets, expected_targets);
|
||||||
|
|
||||||
let first_rejection = "first network approval was rejected";
|
let first_rejection = "first network approval was rejected";
|
||||||
let latest_rejection = "latest network approval was rejected";
|
let latest_rejection = "latest network approval was rejected";
|
||||||
for (approval, rejection) in approvals
|
let first_approval = &approvals[0];
|
||||||
.into_iter()
|
let first_decision = match other_review {
|
||||||
.zip([first_rejection, latest_rejection])
|
OtherNetworkReview::Denied => Some(ReviewDecision::denied(first_rejection)),
|
||||||
{
|
OtherNetworkReview::Pending => None,
|
||||||
|
OtherNetworkReview::ApprovedForSession => Some(ReviewDecision::ApprovedForSession),
|
||||||
|
};
|
||||||
|
if let Some(decision) = first_decision {
|
||||||
test.codex
|
test.codex
|
||||||
.submit(Op::ExecApproval {
|
.submit(Op::ExecApproval {
|
||||||
id: approval.effective_approval_id(),
|
id: first_approval.effective_approval_id(),
|
||||||
turn_id: Some(approval.turn_id),
|
turn_id: Some(first_approval.turn_id.clone()),
|
||||||
decision: ReviewDecision::denied(rejection),
|
decision,
|
||||||
})
|
})
|
||||||
.await?;
|
.await?;
|
||||||
}
|
}
|
||||||
wait_for_turn_complete(&test).await;
|
if matches!(other_review, OtherNetworkReview::ApprovedForSession) {
|
||||||
|
// Wait for the process to observe the proxy response before cancelling its execution.
|
||||||
|
let result_path = local_cwd.join("network-result-80");
|
||||||
|
wait_for_paths(&[&result_path]).await?;
|
||||||
|
let result = fs::read_to_string(result_path)?;
|
||||||
|
assert!(result.starts_with("HTTP/1.1 200") || result.starts_with("HTTP/1.1 502"));
|
||||||
|
}
|
||||||
|
let final_approval = &approvals[1];
|
||||||
|
test.codex
|
||||||
|
.submit(Op::ExecApproval {
|
||||||
|
id: final_approval.effective_approval_id(),
|
||||||
|
turn_id: Some(final_approval.turn_id.clone()),
|
||||||
|
decision: ReviewDecision::denied(latest_rejection),
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
wait_for_completion_without_network_prompt(&test).await;
|
||||||
|
tokio::time::timeout(Duration::from_secs(5), async {
|
||||||
|
loop {
|
||||||
|
match process_lock.try_lock() {
|
||||||
|
Ok(()) => return Ok(()),
|
||||||
|
Err(fs::TryLockError::WouldBlock) => {
|
||||||
|
tokio::time::sleep(Duration::from_millis(10)).await;
|
||||||
|
}
|
||||||
|
Err(fs::TryLockError::Error(error)) => return Err(error),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.context("background process remained alive after network rejection")??;
|
||||||
|
assert!(test.codex.list_background_terminals().await.is_empty());
|
||||||
|
|
||||||
let output = responses
|
let output = responses
|
||||||
.requests()
|
.requests()
|
||||||
.iter()
|
.iter()
|
||||||
.find_map(|request| request.function_call_output_text(call_id))
|
.find_map(|request| request.function_call_output_text(poll_call_id))
|
||||||
.context("expected output from the execution with multiple network reviews")?;
|
.context("expected output from the execution with multiple network reviews")?;
|
||||||
assert!(output.contains(latest_rejection), "{output}");
|
assert!(output.contains(latest_rejection), "{output}");
|
||||||
assert!(!output.contains(first_rejection), "{output}");
|
assert!(!output.contains(first_rejection), "{output}");
|
||||||
|
|
||||||
|
if matches!(other_review, OtherNetworkReview::ApprovedForSession) {
|
||||||
|
let probe_call_id = "network-session-grant-probe";
|
||||||
|
let probe = mount_exec_network_turn(
|
||||||
|
&server,
|
||||||
|
"network-session-grant-probe",
|
||||||
|
probe_call_id,
|
||||||
|
network_exec_args(&remote_network_proxy_request_command("REUSED-GRANT")),
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
submit_managed_network_turn(
|
||||||
|
&test,
|
||||||
|
"reuse the accepted host grant after the original execution was cancelled",
|
||||||
|
vec![local(local_cwd.clone())],
|
||||||
|
ApprovalsReviewer::User,
|
||||||
|
AskForApproval::OnRequest,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
wait_for_completion_without_network_prompt(&test).await;
|
||||||
|
let output = probe
|
||||||
|
.function_call_output_text(probe_call_id)
|
||||||
|
.context("expected network output from the new execution")?;
|
||||||
|
assert!(
|
||||||
|
output.contains("REUSED-GRANT:HTTP/1.1 200")
|
||||||
|
|| output.contains("REUSED-GRANT:HTTP/1.1 502"),
|
||||||
|
"{output}",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2349,6 +2587,44 @@ async fn approved_network_host_for_one_environment_still_prompts_in_another() ->
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn guardian_parent_catalog() -> ModelsResponse {
|
||||||
|
let template = codex_models_manager::bundled_models_response()
|
||||||
|
.expect("bundled model catalog")
|
||||||
|
.models
|
||||||
|
.into_iter()
|
||||||
|
.find(|model| model.slug == "gpt-5.4")
|
||||||
|
.expect("gpt-5.4 in bundled catalog");
|
||||||
|
// Keep safety settings compatible so active publication can switch A to B.
|
||||||
|
ModelsResponse {
|
||||||
|
models: ["guardian-parent-a", "guardian-parent-b"]
|
||||||
|
.into_iter()
|
||||||
|
.map(|slug| {
|
||||||
|
let mut model = template.clone();
|
||||||
|
model.slug = slug.to_string();
|
||||||
|
model.visibility = ModelVisibility::List;
|
||||||
|
model.auto_review_model_override = None;
|
||||||
|
model.supported_reasoning_levels.retain(|level| {
|
||||||
|
level.effort != codex_protocol::openai_models::ReasoningEffort::Low
|
||||||
|
});
|
||||||
|
model
|
||||||
|
.model_messages
|
||||||
|
.as_mut()
|
||||||
|
.expect("model messages")
|
||||||
|
.auto_review = Some(AutoReviewMessages {
|
||||||
|
policy: Some("captured policy".to_string()),
|
||||||
|
policy_template: Some(
|
||||||
|
"captured template: {{ tenant_policy_config }}".to_string(),
|
||||||
|
),
|
||||||
|
node_repl_policy: None,
|
||||||
|
rejection_instructions: None,
|
||||||
|
timeout_instructions: None,
|
||||||
|
});
|
||||||
|
model
|
||||||
|
})
|
||||||
|
.collect(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async fn managed_network_unified_exec_test(server: &wiremock::MockServer) -> Result<TestCodex> {
|
async fn managed_network_unified_exec_test(server: &wiremock::MockServer) -> Result<TestCodex> {
|
||||||
managed_network_unified_exec_test_with_features(server, &[]).await
|
managed_network_unified_exec_test_with_features(server, &[]).await
|
||||||
}
|
}
|
||||||
@@ -2357,6 +2633,16 @@ async fn managed_network_unified_exec_test_with_features(
|
|||||||
server: &wiremock::MockServer,
|
server: &wiremock::MockServer,
|
||||||
features: &[Feature],
|
features: &[Feature],
|
||||||
) -> Result<TestCodex> {
|
) -> Result<TestCodex> {
|
||||||
|
let test = managed_network_unified_exec_builder(features)?
|
||||||
|
.build_with_remote_and_local_env(server)
|
||||||
|
.await?;
|
||||||
|
assert!(test.config.managed_network_requirements_enabled());
|
||||||
|
assert!(test.config.permissions.network.is_some());
|
||||||
|
assert!(test.session_configured.network_proxy.is_some());
|
||||||
|
Ok(test)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn managed_network_unified_exec_builder(features: &[Feature]) -> Result<TestCodexBuilder> {
|
||||||
let home = Arc::new(TempDir::new()?);
|
let home = Arc::new(TempDir::new()?);
|
||||||
fs::write(
|
fs::write(
|
||||||
home.path().join("config.toml"),
|
home.path().join("config.toml"),
|
||||||
@@ -2378,9 +2664,9 @@ allow_local_binding = true
|
|||||||
/*exclude_tmpdir_env_var*/ false,
|
/*exclude_tmpdir_env_var*/ false,
|
||||||
/*exclude_slash_tmp*/ false,
|
/*exclude_slash_tmp*/ false,
|
||||||
);
|
);
|
||||||
let permission_profile_for_config = permission_profile.clone();
|
let permission_profile_for_config = permission_profile;
|
||||||
let features = features.to_vec();
|
let features = features.to_vec();
|
||||||
let mut builder = test_codex()
|
Ok(test_codex()
|
||||||
.with_home(home)
|
.with_home(home)
|
||||||
.with_cloud_config_bundle(managed_network_requirements_loader())
|
.with_cloud_config_bundle(managed_network_requirements_loader())
|
||||||
.with_config(move |config| {
|
.with_config(move |config| {
|
||||||
@@ -2395,22 +2681,7 @@ allow_local_binding = true
|
|||||||
.permissions
|
.permissions
|
||||||
.set_permission_profile(permission_profile_for_config)
|
.set_permission_profile(permission_profile_for_config)
|
||||||
.expect("set permission profile");
|
.expect("set permission profile");
|
||||||
});
|
}))
|
||||||
let test = builder.build_with_remote_and_local_env(server).await?;
|
|
||||||
assert!(
|
|
||||||
test.config.managed_network_requirements_enabled(),
|
|
||||||
"expected managed network requirements to be enabled"
|
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
test.config.permissions.network.is_some(),
|
|
||||||
"expected managed network proxy config to be present"
|
|
||||||
);
|
|
||||||
test.session_configured
|
|
||||||
.network_proxy
|
|
||||||
.as_ref()
|
|
||||||
.expect("expected runtime managed network proxy addresses");
|
|
||||||
|
|
||||||
Ok(test)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn mount_exec_network_turn(
|
async fn mount_exec_network_turn(
|
||||||
|
|||||||
Reference in new issue
Block a user