Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion codex-rs/cli/src/debug_sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,11 @@ async fn run_command_under_sandbox(
network_sandbox_policy,
sandbox_policy_cwd: sandbox_policy_cwd.as_path(),
enforce_managed_network,
environment_id: None,
network: network.as_ref(),
extra_allow_unix_sockets: allow_unix_sockets,
});
})
.map_err(|err| anyhow::anyhow!(err))?;
spawn_debug_sandbox_child(
PathBuf::from("/usr/bin/sandbox-exec"),
args,
Expand Down
32 changes: 27 additions & 5 deletions codex-rs/core/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ fn select_process_exec_tool_sandbox_type(
)
}

fn network_proxy_environment_error(
network_environment_id: Option<&str>,
err: impl std::fmt::Display,
) -> CodexErr {
let environment_id = network_environment_id.unwrap_or("default");
CodexErr::Io(io::Error::other(format!(
"failed to prepare network proxy for environment `{environment_id}`: {err}"
)))
}

/// Mechanism to terminate an exec invocation before it finishes naturally.
#[derive(Clone, Debug)]
pub enum ExecExpiration {
Expand Down Expand Up @@ -345,7 +355,11 @@ pub fn build_exec_request(
tracing::debug!("Sandbox type: {sandbox_type:?}");

if let Some(network) = network.as_ref() {
network.apply_to_env(&mut env);
network
.apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref())
.map_err(|err| {
network_proxy_environment_error(network_environment_id.as_deref(), err)
})?;
}
let (program, args) = command.split_first().ok_or_else(|| {
CodexErr::Io(io::Error::new(
Expand Down Expand Up @@ -598,15 +612,19 @@ async fn exec_windows_sandbox(
cwd,
mut env,
network,
network_environment_id: _,
network_environment_id,
expiration,
capture_policy,
windows_sandbox_level,
windows_sandbox_private_desktop,
..
} = params;
if let Some(network) = network.as_ref() {
network.apply_to_env(&mut env);
network
.apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref())
.map_err(|err| {
network_proxy_environment_error(network_environment_id.as_deref(), err)
})?;
}

// Windows sandbox capture still receives timeout and cancellation separately.
Expand Down Expand Up @@ -947,7 +965,7 @@ async fn exec(
cwd,
mut env,
network,
network_environment_id: _,
network_environment_id,
arg0,
expiration,
capture_policy,
Expand All @@ -961,7 +979,11 @@ async fn exec(
justification: _,
} = params;
if let Some(network) = network.as_ref() {
network.apply_to_env(&mut env);
network
.apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref())
.map_err(|err| {
network_proxy_environment_error(network_environment_id.as_deref(), err)
})?;
}

let (program, args) = command.split_first().ok_or_else(|| {
Expand Down
5 changes: 4 additions & 1 deletion codex-rs/core/src/tools/handlers/shell/shell_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ impl ShellCommandHandler {
Some(thread_id),
),
network: turn_context.network.clone(),
network_environment_id: None,
network_environment_id: turn_context
.environments
.primary()
.map(|environment| environment.environment_id.clone()),
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
windows_sandbox_level: turn_context.windows_sandbox_level,
windows_sandbox_private_desktop: turn_context
Expand Down
125 changes: 105 additions & 20 deletions codex-rs/core/src/tools/network_approval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub(crate) struct NetworkApprovalSpec {
pub mode: NetworkApprovalMode,
pub trigger: GuardianNetworkAccessTrigger,
pub command: String,
pub environment_id: String,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -120,14 +121,20 @@ impl ActiveNetworkApproval {

#[derive(Clone, Debug, Eq, Hash, PartialEq)]
struct HostApprovalKey {
environment_id: String,
host: String,
protocol: &'static str,
port: u16,
}

impl HostApprovalKey {
fn from_request(request: &NetworkPolicyRequest, protocol: NetworkApprovalProtocol) -> Self {
fn from_request(
request: &NetworkPolicyRequest,
protocol: NetworkApprovalProtocol,
environment_id: String,
) -> Self {
Self {
environment_id,
host: request.host.to_ascii_lowercase(),
protocol: protocol_key_label(protocol),
port: request.port,
Expand Down Expand Up @@ -224,9 +231,16 @@ struct ActiveNetworkApprovalCall {
turn_id: String,
trigger: GuardianNetworkAccessTrigger,
command: String,
environment_id: String,
cancellation_token: CancellationToken,
}

enum ActiveNetworkApprovalAttribution {
None,
Single(Arc<ActiveNetworkApprovalCall>),
Ambiguous,
}

#[derive(Default)]
struct NetworkApprovalCallState {
active_calls: IndexMap<String, Arc<ActiveNetworkApprovalCall>>,
Expand Down Expand Up @@ -267,6 +281,7 @@ impl NetworkApprovalService {
turn_id: String,
trigger: GuardianNetworkAccessTrigger,
command: String,
environment_id: String,
cancellation_token: CancellationToken,
) {
let mut calls = self.calls.lock().await;
Expand All @@ -278,6 +293,7 @@ impl NetworkApprovalService {
turn_id,
trigger,
command,
environment_id,
cancellation_token,
}),
);
Expand All @@ -299,6 +315,18 @@ impl NetworkApprovalService {
None
}

async fn resolve_active_call_attribution(&self) -> ActiveNetworkApprovalAttribution {
let calls = self.calls.lock().await;
match calls.active_calls.len() {
0 => ActiveNetworkApprovalAttribution::None,
1 => calls.active_calls.values().next().cloned().map_or(
ActiveNetworkApprovalAttribution::None,
ActiveNetworkApprovalAttribution::Single,
),
_ => ActiveNetworkApprovalAttribution::Ambiguous,
}
}

async fn get_or_create_pending_approval(
&self,
key: HostApprovalKey,
Expand Down Expand Up @@ -384,7 +412,10 @@ impl NetworkApprovalService {
}

fn approval_id_for_key(key: &HostApprovalKey) -> String {
format!("network#{}#{}#{}", key.protocol, key.host, key.port)
format!(
"network#{}#{}#{}#{}",
key.environment_id, key.protocol, key.host, key.port
)
}

pub(crate) async fn handle_inline_policy_request(
Expand All @@ -400,7 +431,38 @@ impl NetworkApprovalService {
NetworkProtocol::Socks5Tcp => NetworkApprovalProtocol::Socks5Tcp,
NetworkProtocol::Socks5Udp => NetworkApprovalProtocol::Socks5Udp,
};
let key = HostApprovalKey::from_request(&request, protocol);
let (owner_call, active_environment_id) =
if let Some(environment_id) = request.environment_id.clone() {
let owner_call = match self.resolve_active_call_attribution().await {
ActiveNetworkApprovalAttribution::Single(call) => {
(call.environment_id == environment_id).then_some(call)
}
ActiveNetworkApprovalAttribution::None
| ActiveNetworkApprovalAttribution::Ambiguous => None,
Comment thread
jif-oai marked this conversation as resolved.
};
(owner_call, Some(environment_id))
} else {
match self.resolve_active_call_attribution().await {
ActiveNetworkApprovalAttribution::None => (None, None),
ActiveNetworkApprovalAttribution::Single(call) => {
let environment_id = call.environment_id.clone();
(Some(call), Some(environment_id))
}
ActiveNetworkApprovalAttribution::Ambiguous => {
return NetworkDecision::deny(REASON_NOT_ALLOWED);
}
}
};
let turn_context = Self::active_turn_context(session.as_ref()).await;
let Some(environment_id) = active_environment_id.or_else(|| {
turn_context
.as_ref()
.and_then(|turn_context| turn_context.environments.primary())
.map(|environment| environment.environment_id.clone())
}) else {
return NetworkDecision::deny(REASON_NOT_ALLOWED);
};
let key = HostApprovalKey::from_request(&request, protocol, environment_id.clone());

{
let denied_hosts = self.session_denied_hosts.lock().await;
Expand All @@ -426,35 +488,43 @@ 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) = Self::active_turn_context(session.as_ref()).await else {
let Some(turn_context) = turn_context else {
pending.set_decision(PendingApprovalDecision::Deny).await;
self.pending_host_approvals.lock().await.remove(&key);
self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy(
policy_denial_message,
))
.await;
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::DeniedByPolicy(policy_denial_message),
)
.await;
}
return NetworkDecision::deny(REASON_NOT_ALLOWED);
};
if !permission_profile_allows_network_approval_flow(&turn_context.permission_profile()) {
pending.set_decision(PendingApprovalDecision::Deny).await;
self.pending_host_approvals.lock().await.remove(&key);
self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy(
policy_denial_message,
))
.await;
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::DeniedByPolicy(policy_denial_message),
)
.await;
}
return NetworkDecision::deny(REASON_NOT_ALLOWED);
}
if !allows_network_approval_flow(turn_context.approval_policy.value()) {
pending.set_decision(PendingApprovalDecision::Deny).await;
self.pending_host_approvals.lock().await.remove(&key);
self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy(
policy_denial_message,
))
.await;
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::DeniedByPolicy(policy_denial_message),
)
.await;
}
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,
Expand Down Expand Up @@ -519,15 +589,28 @@ impl NetworkApprovalService {
.await
} 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
.iter()
.find(|environment| environment.environment_id == environment_id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of scope but wondering why this is an array rather than a map keyed by environment_id

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally need an ordering to decide how to resolve collisions

.and_then(|environment| environment.cwd().to_abs_path().ok())
.unwrap_or_else(|| {
#[allow(deprecated)]
turn_context.cwd.clone()
})
};
session
.request_command_approval(
turn_context.as_ref(),
guardian_approval_id,
/*approval_id*/ None,
/*environment_id*/ None,
Some(environment_id),
prompt_command,
#[allow(deprecated)]
turn_context.cwd.clone(),
cwd,
Some(prompt_reason),
Some(network_approval_context.clone()),
/*proposed_execpolicy_amendment*/ None,
Expand Down Expand Up @@ -712,6 +795,7 @@ pub(crate) async fn begin_network_approval(
mode,
trigger,
command,
environment_id,
} = spec?;
if !managed_network_active || network.is_none() {
return None;
Expand All @@ -727,6 +811,7 @@ pub(crate) async fn begin_network_approval(
turn_id.to_string(),
trigger,
command,
environment_id,
cancellation_token.clone(),
)
.await;
Expand Down
Loading
Loading