Merge a96e08da33 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-04-22 14:06:45 -07:00
committed by GitHub
7 changed files with 220 additions and 40 deletions
+41 -9
View File
@@ -54,6 +54,7 @@ pub(crate) enum GuardianApprovalRequest {
host: String,
protocol: NetworkApprovalProtocol,
port: u16,
trigger: Option<GuardianNetworkAccessTrigger>,
},
McpToolCall {
id: String,
@@ -75,6 +76,22 @@ pub(crate) enum GuardianApprovalRequest {
},
}
#[derive(Debug, Clone, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct GuardianNetworkAccessTrigger {
pub(crate) call_id: String,
pub(crate) tool_name: String,
pub(crate) command: Vec<String>,
pub(crate) cwd: AbsolutePathBuf,
pub(crate) sandbox_permissions: crate::sandboxing::SandboxPermissions,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) additional_permissions: Option<PermissionProfile>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) justification: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) tty: Option<bool>,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub(crate) struct GuardianMcpAnnotations {
#[serde(skip_serializing_if = "Option::is_none")]
@@ -131,6 +148,18 @@ struct McpToolCallApprovalAction<'a> {
annotations: Option<&'a GuardianMcpAnnotations>,
}
#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
struct NetworkAccessApprovalAction<'a> {
tool: &'static str,
target: &'a str,
host: &'a str,
protocol: NetworkApprovalProtocol,
port: u16,
#[serde(skip_serializing_if = "Option::is_none")]
trigger: Option<&'a GuardianNetworkAccessTrigger>,
}
#[derive(Serialize)]
struct RequestPermissionsApprovalAction<'a> {
tool: &'static str,
@@ -297,13 +326,15 @@ pub(crate) fn guardian_approval_request_to_json(
host,
protocol,
port,
} => Ok(serde_json::json!({
"tool": "network_access",
"target": target,
"host": host,
"protocol": protocol,
"port": port,
})),
trigger,
} => serialize_guardian_action(NetworkAccessApprovalAction {
tool: "network_access",
target,
host,
protocol: *protocol,
port: *port,
trigger: trigger.as_ref(),
}),
GuardianApprovalRequest::McpToolCall {
id: _,
server,
@@ -371,12 +402,13 @@ pub(crate) fn guardian_assessment_action(
}
}
GuardianApprovalRequest::NetworkAccess {
id: _,
turn_id: _,
id: _id,
turn_id: _turn_id,
target,
host,
protocol,
port,
trigger: _trigger,
} => GuardianAssessmentAction::NetworkAccess {
target: target.clone(),
host: host.clone(),
+1
View File
@@ -25,6 +25,7 @@ use serde::Serialize;
pub(crate) use approval_request::GuardianApprovalRequest;
pub(crate) use approval_request::GuardianMcpAnnotations;
pub(crate) use approval_request::GuardianNetworkAccessTrigger;
pub(crate) use approval_request::guardian_approval_request_to_json;
pub(crate) use review::guardian_rejection_message;
pub(crate) use review::guardian_timeout_message;
+70
View File
@@ -12,6 +12,7 @@ use crate::config_loader::NetworkDomainPermissionToml;
use crate::config_loader::NetworkDomainPermissionsToml;
use crate::config_loader::RequirementSource;
use crate::config_loader::Sourced;
use crate::guardian::approval_request::guardian_request_target_item_id;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::test_support;
@@ -727,6 +728,50 @@ fn guardian_approval_request_to_json_renders_mcp_tool_call_shape() -> serde_json
Ok(())
}
#[test]
fn guardian_approval_request_to_json_renders_network_access_trigger() -> serde_json::Result<()> {
let cwd = test_path_buf("/repo").abs();
let action = GuardianApprovalRequest::NetworkAccess {
id: "network-1".to_string(),
turn_id: "turn-1".to_string(),
target: "https://example.com:443".to_string(),
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
port: 443,
trigger: Some(GuardianNetworkAccessTrigger {
call_id: "call-1".to_string(),
tool_name: "shell".to_string(),
command: vec!["curl".to_string(), "https://example.com".to_string()],
cwd: cwd.clone(),
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
additional_permissions: None,
justification: Some("Fetch the release metadata.".to_string()),
tty: None,
}),
};
assert_eq!(
guardian_approval_request_to_json(&action)?,
serde_json::json!({
"tool": "network_access",
"target": "https://example.com:443",
"host": "example.com",
"protocol": "https",
"port": 443,
"trigger": {
"callId": "call-1",
"toolName": "shell",
"command": ["curl", "https://example.com"],
"cwd": cwd.to_string_lossy().to_string(),
"sandboxPermissions": "use_default",
"justification": "Fetch the release metadata.",
},
})
);
Ok(())
}
#[test]
fn guardian_assessment_action_redacts_apply_patch_patch_text() {
let cwd = test_path_buf("/tmp").abs();
@@ -758,6 +803,7 @@ fn guardian_request_turn_id_prefers_network_access_owner_turn() {
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
port: 443,
trigger: None,
};
let apply_patch = GuardianApprovalRequest::ApplyPatch {
id: "patch-1".to_string(),
@@ -777,6 +823,30 @@ fn guardian_request_turn_id_prefers_network_access_owner_turn() {
);
}
#[test]
fn guardian_request_target_item_id_omits_network_access_trigger_call_id() {
let network_access = GuardianApprovalRequest::NetworkAccess {
id: "network-1".to_string(),
turn_id: "owner-turn".to_string(),
target: "https://example.com:443".to_string(),
host: "example.com".to_string(),
protocol: NetworkApprovalProtocol::Https,
port: 443,
trigger: Some(GuardianNetworkAccessTrigger {
call_id: "call-1".to_string(),
tool_name: "shell".to_string(),
command: vec!["curl".to_string(), "https://example.com".to_string()],
cwd: test_path_buf("/repo").abs(),
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
tty: None,
}),
};
assert_eq!(guardian_request_target_item_id(&network_access), None);
}
#[tokio::test]
async fn cancelled_guardian_review_emits_terminal_abort_without_warning() {
let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await;
+27 -6
View File
@@ -1,4 +1,5 @@
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianNetworkAccessTrigger;
use crate::guardian::guardian_rejection_message;
use crate::guardian::guardian_timeout_message;
use crate::guardian::new_guardian_review_id;
@@ -47,6 +48,7 @@ pub(crate) enum NetworkApprovalMode {
pub(crate) struct NetworkApprovalSpec {
pub network: Option<NetworkProxy>,
pub mode: NetworkApprovalMode,
pub trigger: GuardianNetworkAccessTrigger,
pub command: String,
}
@@ -177,6 +179,7 @@ impl PendingHostApproval {
struct ActiveNetworkApprovalCall {
registration_id: String,
turn_id: String,
trigger: GuardianNetworkAccessTrigger,
command: String,
}
@@ -210,7 +213,13 @@ impl NetworkApprovalService {
other_approved_hosts.extend(approved_hosts.iter().cloned());
}
async fn register_call(&self, registration_id: String, turn_id: String, command: String) {
async fn register_call(
&self,
registration_id: String,
turn_id: String,
trigger: GuardianNetworkAccessTrigger,
command: String,
) {
let mut active_calls = self.active_calls.lock().await;
let key = registration_id.clone();
active_calls.insert(
@@ -218,6 +227,7 @@ impl NetworkApprovalService {
Arc::new(ActiveNetworkApprovalCall {
registration_id,
turn_id,
trigger,
command,
}),
);
@@ -369,11 +379,11 @@ impl NetworkApprovalService {
return NetworkDecision::deny(REASON_NOT_ALLOWED);
}
let owner_call = self.resolve_single_active_call().await;
let network_approval_context = NetworkApprovalContext {
host: request.host.clone(),
protocol,
};
let owner_call = self.resolve_single_active_call().await;
let guardian_approval_id = Self::approval_id_for_key(&key);
let prompt_command = vec!["network-access".to_string(), target.clone()];
let command = owner_call
@@ -431,6 +441,7 @@ impl NetworkApprovalService {
host: request.host,
protocol,
port: key.port,
trigger: owner_call.as_ref().map(|call| call.trigger.clone()),
},
Some(policy_denial_message.clone()),
)
@@ -623,8 +634,13 @@ pub(crate) async fn begin_network_approval(
managed_network_active: bool,
spec: Option<NetworkApprovalSpec>,
) -> Option<ActiveNetworkApproval> {
let spec = spec?;
if !managed_network_active || spec.network.is_none() {
let NetworkApprovalSpec {
network,
mode,
trigger,
command,
} = spec?;
if !managed_network_active || network.is_none() {
return None;
}
@@ -632,12 +648,17 @@ pub(crate) async fn begin_network_approval(
session
.services
.network_approval
.register_call(registration_id.clone(), turn_id.to_string(), spec.command)
.register_call(
registration_id.clone(),
turn_id.to_string(),
trigger,
command,
)
.await;
Some(ActiveNetworkApproval {
registration_id: Some(registration_id),
mode: spec.mode,
mode,
})
}
@@ -1,7 +1,10 @@
use super::*;
use crate::sandboxing::SandboxPermissions;
use codex_network_proxy::BlockedRequestArgs;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use core_test_support::PathBufExt;
use core_test_support::test_path_buf;
use pretty_assertions::assert_eq;
#[tokio::test]
@@ -207,17 +210,66 @@ fn denied_blocked_request(host: &str) -> BlockedRequest {
})
}
async fn register_call_with_default_shell_trigger(
service: &NetworkApprovalService,
registration_id: &str,
) {
service
.register_call(
registration_id.to_string(),
"turn-1".to_string(),
GuardianNetworkAccessTrigger {
call_id: "call-1".to_string(),
tool_name: "shell".to_string(),
command: vec!["curl".to_string(), "https://example.com".to_string()],
cwd: test_path_buf("/tmp").abs(),
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: None,
tty: None,
},
"curl https://example.com".to_string(),
)
.await;
}
#[tokio::test]
async fn record_blocked_request_sets_policy_outcome_for_owner_call() {
async fn active_call_preserves_triggering_command_context() {
let service = NetworkApprovalService::default();
let expected = GuardianNetworkAccessTrigger {
call_id: "call-1".to_string(),
tool_name: "shell".to_string(),
command: vec!["curl".to_string(), "https://example.com".to_string()],
cwd: test_path_buf("/repo").abs(),
sandbox_permissions: SandboxPermissions::UseDefault,
additional_permissions: None,
justification: Some("fetch release metadata".to_string()),
tty: None,
};
service
.register_call(
"registration-1".to_string(),
"turn-1".to_string(),
"curl http://example.com".to_string(),
expected.clone(),
"curl https://example.com".to_string(),
)
.await;
let call = service
.resolve_single_active_call()
.await
.expect("single active call should resolve");
assert_eq!(&call.trigger, &expected);
assert_eq!(call.command, "curl https://example.com");
}
#[tokio::test]
async fn record_blocked_request_sets_policy_outcome_for_owner_call() {
let service = NetworkApprovalService::default();
register_call_with_default_shell_trigger(&service, "registration-1").await;
service
.record_blocked_request(denied_blocked_request("example.com"))
.await;
@@ -233,13 +285,7 @@ async fn record_blocked_request_sets_policy_outcome_for_owner_call() {
#[tokio::test]
async fn blocked_request_policy_does_not_override_user_denial_outcome() {
let service = NetworkApprovalService::default();
service
.register_call(
"registration-1".to_string(),
"turn-1".to_string(),
"curl http://example.com".to_string(),
)
.await;
register_call_with_default_shell_trigger(&service, "registration-1").await;
service
.record_call_outcome("registration-1", NetworkApprovalOutcome::DeniedByUser)
@@ -257,20 +303,8 @@ async fn blocked_request_policy_does_not_override_user_denial_outcome() {
#[tokio::test]
async fn record_blocked_request_ignores_ambiguous_unattributed_blocked_requests() {
let service = NetworkApprovalService::default();
service
.register_call(
"registration-1".to_string(),
"turn-1".to_string(),
"curl http://example.com".to_string(),
)
.await;
service
.register_call(
"registration-2".to_string(),
"turn-1".to_string(),
"gh api /foo".to_string(),
)
.await;
register_call_with_default_shell_trigger(&service, "registration-1").await;
register_call_with_default_shell_trigger(&service, "registration-2").await;
service
.record_blocked_request(denied_blocked_request("example.com"))
+12 -1
View File
@@ -11,6 +11,7 @@ pub(crate) mod zsh_fork_backend;
use crate::command_canonicalization::canonicalize_command_for_approval;
use crate::exec::ExecCapturePolicy;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianNetworkAccessTrigger;
use crate::guardian::review_approval_request;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::SandboxPermissions;
@@ -217,12 +218,22 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
fn network_approval_spec(
&self,
req: &ShellRequest,
_ctx: &ToolCtx,
ctx: &ToolCtx,
) -> Option<NetworkApprovalSpec> {
req.network.as_ref()?;
Some(NetworkApprovalSpec {
network: req.network.clone(),
mode: NetworkApprovalMode::Immediate,
trigger: GuardianNetworkAccessTrigger {
call_id: ctx.call_id.clone(),
tool_name: ctx.tool_name.clone(),
command: req.command.clone(),
cwd: req.cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
tty: None,
},
command: req.hook_command.clone(),
})
}
@@ -8,6 +8,7 @@ use crate::command_canonicalization::canonicalize_command_for_approval;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecExpiration;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianNetworkAccessTrigger;
use crate::guardian::review_approval_request;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecServerEnvConfig;
@@ -202,12 +203,22 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
fn network_approval_spec(
&self,
req: &UnifiedExecRequest,
_ctx: &ToolCtx,
ctx: &ToolCtx,
) -> Option<NetworkApprovalSpec> {
req.network.as_ref()?;
Some(NetworkApprovalSpec {
network: req.network.clone(),
mode: NetworkApprovalMode::Deferred,
trigger: GuardianNetworkAccessTrigger {
call_id: ctx.call_id.clone(),
tool_name: ctx.tool_name.clone(),
command: req.command.clone(),
cwd: req.cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
additional_permissions: req.additional_permissions.clone(),
justification: req.justification.clone(),
tty: Some(req.tty),
},
command: req.hook_command.clone(),
})
}