mirror of
https://github.com/openai/codex.git
synced 2026-11-12 22:42:36 +00:00
Harden managed proxy setup for sandboxed executions (#34641)
## What changed - Make the generated Linux proxy socket directory readable inside restricted `bubblewrap` sandboxes so the proxy bridge can connect. - Route `WS_PROXY` and `WSS_PROXY` through the Linux managed proxy bridge. - Remove inherited proxy attribution tokens from unscoped executions while continuing to replace them with the current token for scoped executions. ## Testing - Exercise `WSS_PROXY` routing with a minimal filesystem policy. - Cover attribution-token removal and replacement during environment setup. GitOrigin-RevId: 2f14ddb26baab0147a786bc354d93eb5aad059f3
This commit is contained in:
1 parent
bdd3118c71
commit
c5eb33aed1
5 files changed
+80
-13
No files matched your search
@@ -5,6 +5,7 @@ use std::time::Duration;
|
|||||||
#[cfg(target_os = "macos")]
|
#[cfg(target_os = "macos")]
|
||||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||||
use codex_network_proxy::NetworkProxyConfig;
|
use codex_network_proxy::NetworkProxyConfig;
|
||||||
|
use codex_network_proxy::PROXY_ATTRIBUTION_TOKEN_ENV_KEY;
|
||||||
use codex_network_proxy::RemoteNetworkProxyConfig;
|
use codex_network_proxy::RemoteNetworkProxyConfig;
|
||||||
use codex_network_proxy::RemoteNetworkProxyLaunchConfig;
|
use codex_network_proxy::RemoteNetworkProxyLaunchConfig;
|
||||||
#[cfg(windows)]
|
#[cfg(windows)]
|
||||||
@@ -264,6 +265,10 @@ async fn native_request_handles_remote_proxy_config_for_platform() {
|
|||||||
let env = HashMap::from([
|
let env = HashMap::from([
|
||||||
("HTTP_PROXY".to_string(), stale_proxy.clone()),
|
("HTTP_PROXY".to_string(), stale_proxy.clone()),
|
||||||
("TEST_ENV".to_string(), "value".to_string()),
|
("TEST_ENV".to_string(), "value".to_string()),
|
||||||
|
(
|
||||||
|
PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(),
|
||||||
|
"foreign-token".to_string(),
|
||||||
|
),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
let prepared = prepare_exec_request(¶ms, env, /*runtime_paths*/ None)
|
let prepared = prepare_exec_request(¶ms, env, /*runtime_paths*/ None)
|
||||||
@@ -280,6 +285,7 @@ async fn native_request_handles_remote_proxy_config_for_platform() {
|
|||||||
let http_proxy = prepared.env.get("HTTP_PROXY").expect("HTTP proxy env");
|
let http_proxy = prepared.env.get("HTTP_PROXY").expect("HTTP proxy env");
|
||||||
assert_ne!(http_proxy, &stale_proxy);
|
assert_ne!(http_proxy, &stale_proxy);
|
||||||
assert!(http_proxy.starts_with("http://127.0.0.1:"));
|
assert!(http_proxy.starts_with("http://127.0.0.1:"));
|
||||||
|
assert!(!prepared.env.contains_key(PROXY_ATTRIBUTION_TOKEN_ENV_KEY));
|
||||||
let proxy_addr: SocketAddr = http_proxy
|
let proxy_addr: SocketAddr = http_proxy
|
||||||
.strip_prefix("http://")
|
.strip_prefix("http://")
|
||||||
.expect("HTTP proxy scheme")
|
.expect("HTTP proxy scheme")
|
||||||
|
|||||||
@@ -167,7 +167,7 @@ pub fn run_main() -> ! {
|
|||||||
ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_legacy_landlock);
|
ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_legacy_landlock);
|
||||||
let EffectivePermissions {
|
let EffectivePermissions {
|
||||||
permission_profile,
|
permission_profile,
|
||||||
file_system_sandbox_policy,
|
mut file_system_sandbox_policy,
|
||||||
network_sandbox_policy,
|
network_sandbox_policy,
|
||||||
} = resolve_permission_profile(permission_profile).unwrap_or_else(|err| panic!("{err}"));
|
} = resolve_permission_profile(permission_profile).unwrap_or_else(|err| panic!("{err}"));
|
||||||
ensure_legacy_landlock_mode_supports_policy(
|
ensure_legacy_landlock_mode_supports_policy(
|
||||||
@@ -218,14 +218,17 @@ pub fn run_main() -> ! {
|
|||||||
// Outer stage: bubblewrap first, then re-enter this binary in the
|
// Outer stage: bubblewrap first, then re-enter this binary in the
|
||||||
// sandboxed environment to apply seccomp. This path never falls back
|
// sandboxed environment to apply seccomp. This path never falls back
|
||||||
// to legacy Landlock on failure.
|
// to legacy Landlock on failure.
|
||||||
let proxy_route_spec =
|
let proxy_route_spec = if allow_network_for_proxy {
|
||||||
if allow_network_for_proxy {
|
let (proxy_route_spec, socket_dir) = prepare_host_proxy_route_spec()
|
||||||
Some(prepare_host_proxy_route_spec().unwrap_or_else(|err| {
|
.unwrap_or_else(|err| panic!("failed to prepare host proxy routing bridge: {err}"));
|
||||||
panic!("failed to prepare host proxy routing bridge: {err}")
|
file_system_sandbox_policy = file_system_sandbox_policy.with_additional_readable_roots(
|
||||||
}))
|
&sandbox_policy_cwd,
|
||||||
} else {
|
std::slice::from_ref(&socket_dir),
|
||||||
None
|
);
|
||||||
};
|
Some(proxy_route_spec)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
let inner = build_inner_seccomp_command(InnerSeccompCommandArgs {
|
let inner = build_inner_seccomp_command(InnerSeccompCommandArgs {
|
||||||
sandbox_policy_cwd: &sandbox_policy_cwd,
|
sandbox_policy_cwd: &sandbox_policy_cwd,
|
||||||
command_cwd: command_cwd.as_deref(),
|
command_cwd: command_cwd.as_deref(),
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
use codex_network_proxy::PROXY_ATTRIBUTION_TOKEN_ENV_KEY;
|
use codex_network_proxy::PROXY_ATTRIBUTION_TOKEN_ENV_KEY;
|
||||||
use codex_network_proxy::write_attribution_frame;
|
use codex_network_proxy::write_attribution_frame;
|
||||||
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde::Serialize;
|
use serde::Serialize;
|
||||||
use std::collections::BTreeMap;
|
use std::collections::BTreeMap;
|
||||||
@@ -29,6 +30,8 @@ use url::Url;
|
|||||||
const PROXY_ENV_KEYS: &[&str] = &[
|
const PROXY_ENV_KEYS: &[&str] = &[
|
||||||
"HTTP_PROXY",
|
"HTTP_PROXY",
|
||||||
"HTTPS_PROXY",
|
"HTTPS_PROXY",
|
||||||
|
"WS_PROXY",
|
||||||
|
"WSS_PROXY",
|
||||||
"ALL_PROXY",
|
"ALL_PROXY",
|
||||||
"FTP_PROXY",
|
"FTP_PROXY",
|
||||||
"YARN_HTTP_PROXY",
|
"YARN_HTTP_PROXY",
|
||||||
@@ -72,7 +75,7 @@ struct ProxyRoutePlan {
|
|||||||
has_proxy_config: bool,
|
has_proxy_config: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn prepare_host_proxy_route_spec() -> io::Result<String> {
|
pub(crate) fn prepare_host_proxy_route_spec() -> io::Result<(String, AbsolutePathBuf)> {
|
||||||
let (attribution_token, plan) = extract_attribution_token_and_plan(std::env::vars().collect());
|
let (attribution_token, plan) = extract_attribution_token_and_plan(std::env::vars().collect());
|
||||||
// SAFETY: the sandbox helper is single-threaded here, before it forks bridge workers or
|
// SAFETY: the sandbox helper is single-threaded here, before it forks bridge workers or
|
||||||
// executes the user command.
|
// executes the user command.
|
||||||
@@ -93,6 +96,7 @@ pub(crate) fn prepare_host_proxy_route_spec() -> io::Result<String> {
|
|||||||
let _ = cleanup_stale_proxy_socket_dirs_in(socket_parent_dir.as_path());
|
let _ = cleanup_stale_proxy_socket_dirs_in(socket_parent_dir.as_path());
|
||||||
|
|
||||||
let socket_dir = create_proxy_socket_dir()?;
|
let socket_dir = create_proxy_socket_dir()?;
|
||||||
|
let readable_socket_dir = AbsolutePathBuf::relative_to_current_dir(&socket_dir)?;
|
||||||
let mut socket_by_endpoint: BTreeMap<SocketAddr, PathBuf> = BTreeMap::new();
|
let mut socket_by_endpoint: BTreeMap<SocketAddr, PathBuf> = BTreeMap::new();
|
||||||
let mut next_index = 0usize;
|
let mut next_index = 0usize;
|
||||||
for route in &plan.routes {
|
for route in &plan.routes {
|
||||||
@@ -128,7 +132,8 @@ pub(crate) fn prepare_host_proxy_route_spec() -> io::Result<String> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
serde_json::to_string(&ProxyRouteSpec { routes }).map_err(io::Error::other)
|
let spec = serde_json::to_string(&ProxyRouteSpec { routes }).map_err(io::Error::other)?;
|
||||||
|
Ok((spec, readable_socket_dir))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn extract_attribution_token_and_plan(
|
fn extract_attribution_token_and_plan(
|
||||||
@@ -732,6 +737,8 @@ mod tests {
|
|||||||
fn recognizes_proxy_env_keys_case_insensitively() {
|
fn recognizes_proxy_env_keys_case_insensitively() {
|
||||||
assert_eq!(is_proxy_env_key("HTTP_PROXY"), true);
|
assert_eq!(is_proxy_env_key("HTTP_PROXY"), true);
|
||||||
assert_eq!(is_proxy_env_key("http_proxy"), true);
|
assert_eq!(is_proxy_env_key("http_proxy"), true);
|
||||||
|
assert_eq!(is_proxy_env_key("WS_PROXY"), true);
|
||||||
|
assert_eq!(is_proxy_env_key("wss_proxy"), true);
|
||||||
assert_eq!(is_proxy_env_key("PATH"), false);
|
assert_eq!(is_proxy_env_key("PATH"), false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,13 @@
|
|||||||
use codex_core::exec_env::create_env;
|
use codex_core::exec_env::create_env;
|
||||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||||
use codex_protocol::models::PermissionProfile;
|
use codex_protocol::models::PermissionProfile;
|
||||||
|
use codex_protocol::permissions::FileSystemAccessMode;
|
||||||
|
use codex_protocol::permissions::FileSystemPath;
|
||||||
|
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||||
|
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||||
|
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||||
|
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||||
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::io::Read;
|
use std::io::Read;
|
||||||
@@ -28,6 +35,8 @@ const MANAGED_PROXY_PERMISSION_ERR_SNIPPETS: &[&str] = &[
|
|||||||
const PROXY_ENV_KEYS: &[&str] = &[
|
const PROXY_ENV_KEYS: &[&str] = &[
|
||||||
"HTTP_PROXY",
|
"HTTP_PROXY",
|
||||||
"HTTPS_PROXY",
|
"HTTPS_PROXY",
|
||||||
|
"WS_PROXY",
|
||||||
|
"WSS_PROXY",
|
||||||
"ALL_PROXY",
|
"ALL_PROXY",
|
||||||
"FTP_PROXY",
|
"FTP_PROXY",
|
||||||
"YARN_HTTP_PROXY",
|
"YARN_HTTP_PROXY",
|
||||||
@@ -209,14 +218,40 @@ async fn managed_proxy_mode_routes_through_bridge_and_blocks_direct_egress() {
|
|||||||
"HTTP_PROXY".to_string(),
|
"HTTP_PROXY".to_string(),
|
||||||
format!("http://127.0.0.1:{proxy_port}"),
|
format!("http://127.0.0.1:{proxy_port}"),
|
||||||
);
|
);
|
||||||
|
env.insert(
|
||||||
|
"WSS_PROXY".to_string(),
|
||||||
|
format!("http://127.0.0.1:{proxy_port}"),
|
||||||
|
);
|
||||||
|
|
||||||
|
let sandbox_helper_dir = std::path::Path::new(env!("CARGO_BIN_EXE_codex-linux-sandbox"))
|
||||||
|
.parent()
|
||||||
|
.expect("sandbox helper should have a parent");
|
||||||
|
let file_system_sandbox_policy =
|
||||||
|
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||||
|
path: FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::Minimal,
|
||||||
|
},
|
||||||
|
access: FileSystemAccessMode::Read,
|
||||||
|
missing_path_behavior: None,
|
||||||
|
}])
|
||||||
|
.with_additional_readable_roots(
|
||||||
|
std::env::current_dir()
|
||||||
|
.expect("current directory should exist")
|
||||||
|
.as_path(),
|
||||||
|
&[AbsolutePathBuf::try_from(sandbox_helper_dir).expect("absolute helper dir")],
|
||||||
|
);
|
||||||
|
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||||
|
&file_system_sandbox_policy,
|
||||||
|
NetworkSandboxPolicy::Restricted,
|
||||||
|
);
|
||||||
|
|
||||||
let routed_output = run_linux_sandbox_direct(
|
let routed_output = run_linux_sandbox_direct(
|
||||||
&[
|
&[
|
||||||
"bash",
|
"bash",
|
||||||
"-c",
|
"-c",
|
||||||
"proxy=\"${HTTP_PROXY#*://}\"; host=\"${proxy%%:*}\"; port=\"${proxy##*:}\"; exec 3<>/dev/tcp/${host}/${port}; printf 'GET http://example.com/ HTTP/1.1\\r\\nHost: example.com\\r\\n\\r\\n' >&3; IFS= read -r line <&3; printf '%s\\n' \"$line\"",
|
"proxy=\"${WSS_PROXY#*://}\"; host=\"${proxy%%:*}\"; port=\"${proxy##*:}\"; exec 3<>/dev/tcp/${host}/${port}; printf 'GET http://example.com/ HTTP/1.1\\r\\nHost: example.com\\r\\n\\r\\n' >&3; IFS= read -r line <&3; printf '%s\\n' \"$line\"",
|
||||||
],
|
],
|
||||||
&PermissionProfile::Disabled,
|
&permission_profile,
|
||||||
/*allow_network_for_proxy*/ true,
|
/*allow_network_for_proxy*/ true,
|
||||||
env.clone(),
|
env.clone(),
|
||||||
NETWORK_TIMEOUT_MS,
|
NETWORK_TIMEOUT_MS,
|
||||||
|
|||||||
@@ -902,6 +902,8 @@ impl NetworkProxy {
|
|||||||
PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(),
|
PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(),
|
||||||
execution_scope.attribution_token.clone(),
|
execution_scope.attribution_token.clone(),
|
||||||
);
|
);
|
||||||
|
} else {
|
||||||
|
env.remove(PROXY_ATTRIBUTION_TOKEN_ENV_KEY);
|
||||||
}
|
}
|
||||||
let expose_socks_port = self.socks_enabled;
|
let expose_socks_port = self.socks_enabled;
|
||||||
#[cfg(target_os = "windows")]
|
#[cfg(target_os = "windows")]
|
||||||
@@ -1712,9 +1714,23 @@ mod tests {
|
|||||||
|
|
||||||
let scoped = proxy.for_execution("remote-env", "execution-1", "token-1".to_string())?;
|
let scoped = proxy.for_execution("remote-env", "execution-1", "token-1".to_string())?;
|
||||||
let launch = scoped.remote_launch_config().await?;
|
let launch = scoped.remote_launch_config().await?;
|
||||||
|
let prepared = scoped.prepare_for_optional_environment(
|
||||||
|
HashMap::from([(
|
||||||
|
PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(),
|
||||||
|
"foreign-token".to_string(),
|
||||||
|
)]),
|
||||||
|
/*environment_id*/ None,
|
||||||
|
)?;
|
||||||
|
|
||||||
assert_eq!(launch.environment_id.as_deref(), Some("remote-env"));
|
assert_eq!(launch.environment_id.as_deref(), Some("remote-env"));
|
||||||
assert_eq!(launch.execution_id.as_deref(), Some("execution-1"));
|
assert_eq!(launch.execution_id.as_deref(), Some("execution-1"));
|
||||||
|
assert_eq!(
|
||||||
|
prepared
|
||||||
|
.env
|
||||||
|
.get(PROXY_ATTRIBUTION_TOKEN_ENV_KEY)
|
||||||
|
.map(String::as_str),
|
||||||
|
Some("token-1")
|
||||||
|
);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user