github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 16:17:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Prevent brokered credential aliases from bypassing MITM hooks (#52661)

Browse Source 
## Why

A credential bound to a host with MITM hooks could also be used at an alias without hooks, allowing the proxy to inject the real credential without enforcing hook policy.

## What changed

Reject requests to unhooked aliases when their selected brokered credentials are also bound to a hooked host. Check both HTTP forwarding and MITM requests before replacing dummy credentials, returning `403` with `blocked-by-mitm-hook` for denied requests.

Use the same destination and credential-selection rules for alias checks and injection, including configured URL prefixes, safe paths, equivalent credentials, merged Basic authentication fields, and environment scope.

## Testing

Add regression coverage for built-in and configured provider aliases, IPv6 destinations, credential selection, and unrelated credentials. Add an agent-started proxy test that verifies credential masking and alias denial.

GitOrigin-RevId: 12be41654761cedcf87b170f7661d6566f7a864e
This commit is contained in:
viyatb-oai
2026-10-09 18:04:48 +00:00
committed by copyberry
parent b49e2211a5
commit 65b82cdf96
10 changed files with 649 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/tests/suite/network_approval.rs
+136
View File
@@ -16,6 +16,14 @@ use codex_exec_server::REMOTE_ENVIRONMENT_ID;
use codex_exec_server::RemoveOptions;
use codex_features::Feature;
use codex_history::RolloutItem;
#[cfg(unix)]
use codex_network_proxy::CredentialProviderConfig;
#[cfg(unix)]
use codex_network_proxy::MitmHookActionsConfig;
#[cfg(unix)]
use codex_network_proxy::MitmHookConfig;
#[cfg(unix)]
use codex_network_proxy::MitmHookMatchConfig;
use codex_network_proxy::NetworkProxyConfig;
use codex_network_proxy::PROXY_ACTIVE_ENV_KEY;
use codex_protocol::approvals::ExecApprovalKind;
@@ -3143,6 +3151,134 @@ async fn approved_network_host_for_one_environment_still_prompts_in_another() ->
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn agent_started_proxy_denies_brokered_provider_alias() -> Result<()> {
const REAL_CREDENTIAL: &str = "alias_aaaaaaaaaaaaaaaaaaaaaaaa";
let upstream = tokio::net::TcpListener::bind("127.0.0.1:0").await?;
let port = upstream.local_addr()?.port();
let mut network = NetworkProxyConfig {
enabled: true,
proxy_url: "http://127.0.0.1:0".to_string(),
socks_url: "http://127.0.0.1:0".to_string(),
// Local requests must stay behind the proxy for their credentials to be masked.
allow_local_binding: Some(false),
allow_upstream_proxy: false,
mitm_hooks: vec![MitmHookConfig {
host: "localhost".to_string(),
matcher: MitmHookMatchConfig {
methods: vec!["GET".to_string()],
path_prefixes: vec!["/v1".to_string()],
..MitmHookMatchConfig::default()
},
actions: MitmHookActionsConfig::default(),
}],
..NetworkProxyConfig::default()
};
network.set_credential_broker_enabled(/*enabled*/ true);
network.set_allowed_domains(vec!["localhost".to_string(), "127.0.0.1".to_string()]);
network.credential_providers.insert(
"alias_test".to_string(),
CredentialProviderConfig {
env: vec!["ALIAS_TEST_TOKEN".to_string()],
patterns: vec!["alias_[a-z]{24}".to_string()],
url_prefixes: vec![
format!("http://localhost:{port}/v1"),
format!("http://127.0.0.1:{port}/v2"),
],
..CredentialProviderConfig::default()
},
);
let permission_profile = PermissionProfile::workspace_write_with(
&[],
NetworkSandboxPolicy::Enabled,
/*exclude_tmpdir_env_var*/ false,
/*exclude_slash_tmp*/ false,
);
let server = start_mock_server().await;
let test = test_codex()
.with_config(move |config| {
for feature in [
Feature::UnifiedExec,
Feature::ShellSnapshot,
Feature::NetworkProxy,
] {
config
.features
.enable(feature)
.expect("enable test feature");
}
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
config
.permissions
.set_permission_profile(permission_profile.clone())
.expect("set permission profile");
config
.permissions
.shell_environment_policy
.r#set
.insert("ALIAS_TEST_TOKEN".to_string(), REAL_CREDENTIAL.to_string());
config.permissions.network = Some(
NetworkProxySpec::from_config_and_constraints(
network,
/*requirements*/ None,
&permission_profile,
)
.expect("build brokered managed network proxy"),
);
})
// Credential brokerage currently runs only in the local environment.
.build(&server)
.await?;
let proxy = test
.session_configured
.network_proxy
.as_ref()
.context("agent did not start its managed network proxy")?;
let command = r#"printf 'BROKERED:%s\nACTIVE:%s\n' "$ALIAS_TEST_TOKEN" "$CODEX_NETWORK_PROXY_CREDENTIAL_BROKER_ACTIVE""#;
let call_id = "exec-brokered-provider-alias";
let mut args = network_exec_args(command);
args["yield_time_ms"] = json!(30_000);
let responses =
mount_exec_network_turn(&server, "resp-brokered-provider-alias", call_id, args).await?;
test.submit_text_turn("read the brokered credential")
.await?;
let output = responses
.function_call_output_text(call_id)
.context("agent did not return command output")?;
assert!(output.contains("ACTIVE:1"), "{output}");
assert!(!output.contains(REAL_CREDENTIAL));
let token = output
.lines()
.find_map(|line| line.strip_prefix("BROKERED:"))
.context("agent did not expose its brokered dummy")?;
assert!(token.starts_with("alias_"));
// Probe after the command exits so the denial cannot interrupt the tool call.
let response = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), async {
let mut connection = tokio::net::TcpStream::connect(&proxy.http_addr).await?;
connection
.write_all(
format!(
"GET http://127.0.0.1:{port}/v2/resource HTTP/1.1\r\n\
Host: 127.0.0.1:{port}\r\nAuthorization: Bearer {token}\r\n\
Connection: close\r\n\r\n"
)
.as_bytes(),
)
.await?;
let mut response = String::new();
connection.read_to_string(&mut response).await?;
Ok::<_, std::io::Error>(response)
})
.await
.context("alias denial did not finish")??;
assert!(response.starts_with("HTTP/1.1 403"), "{response}");
assert!(response.contains("blocked-by-mitm-hook"), "{response}");
assert!(!response.contains(REAL_CREDENTIAL));
Ok(())
}
pub(super) fn guardian_parent_catalog() -> ModelsResponse {
let template = codex_models_manager::bundled_models_response()
.expect("bundled model catalog")
codex-rs/network-proxy/src/credential_broker.rs
+63 -33
View File
@@ -1280,11 +1280,29 @@ impl CredentialBroker {
headers: &mut HeaderMap,
environment_id: Option<&str>,
) {
let state = self.read_state();
if !state.enabled {
return;
}
let credentials =
select_credentials(headers, destination, &state.credentials, environment_id);
for (credential, header_name, header_value) in credentials {
credential
.provider
.insert_request_header(headers, header_name, header_value);
}
}
pub(crate) fn request_matches_hooked_host_alias(
&self,
destination: &str,
headers: &HeaderMap,
hooked_hosts: &[String],
environment_id: Option<&str>,
) -> bool {
let request = if destination.contains("://") {
let Ok(request) = Url::parse(destination) else {
return;
};
Some(request)
Url::parse(destination).ok()
} else {
None
};
@@ -1296,38 +1314,50 @@ impl CredentialBroker {
);
let state = self.read_state();
if !state.enabled {
return;
return false;
}
let credentials = select_credentials(
headers,
&normalized_host,
request.as_ref(),
&state.credentials,
environment_id,
);
if credentials.iter().any(|(credential, _, _)| {
matches!(
&credential.provider,
BrokeredCredentialProvider::Configured(_)
)
}) && let Some(request) = request.as_ref()
{
let Ok(raw_request) = destination.parse::<rama_http::Uri>() else {
return;
};
let raw_path = raw_request.path();
if raw_path != request.path()
|| !crate::authorization_path::is_safe_for_authorization(raw_path)
{
return;
}
}
for (credential, header_name, header_value) in credentials {
credential
.provider
.insert_request_header(headers, header_name, header_value);
let selected_headers =
select_credentials(headers, destination, &state.credentials, environment_id)
.into_iter()
.map(|(_, name, _)| name)
.collect::<Vec<_>>();
if selected_headers.is_empty() {
return false;
}
let matching_credentials = state
.credentials
.iter()
.filter(|credential| {
credential.belongs_to_environment(environment_id)
&& credential
.host_bindings()
.any(|binding| binding.matches_request(&normalized_host, request.as_ref()))
})
.collect::<Vec<_>>();
// Ambiguity removes the whole header, so every translation for a surviving header
// contributed through equivalent deduplication or Basic-field merging.
matching_credentials.iter().any(|credential| {
hooked_hosts.iter().any(|hooked_host| {
hooked_host != &normalized_host
&& credential
.host_bindings()
.any(|binding| binding.matches_hostname(hooked_host))
}) && matching_credentials.iter().any(|candidate| {
candidate.provider.same_provider(&credential.provider)
&& candidate.real_value == credential.real_value
&& credential
.provider
.translate_request_headers(
headers,
&candidate.dummy_value,
&credential.real_value,
)
.iter()
.any(|(name, _)| selected_headers.contains(name))
})
})
}
fn read_state(&self) -> std::sync::RwLockReadGuard<'_, CredentialBrokerState> {
codex-rs/network-proxy/src/credential_broker/destination.rs
+4 -3
View File
@@ -116,9 +116,10 @@ impl CredentialDestination {
}
pub(super) fn matches_host(&self, host: &str, port: u16) -> bool {
if self.port != port {
return false;
}
self.port == port && self.matches_hostname(host)
}
pub(super) fn matches_hostname(&self, host: &str) -> bool {
if self.wildcard {
host.strip_suffix(&self.host)
.is_some_and(|prefix| prefix.ends_with('.'))
codex-rs/network-proxy/src/credential_broker/providers.rs
+12
View File
@@ -113,6 +113,18 @@ impl CredentialHostBinding {
}
}
// Hooks apply to the host regardless of each credential destination's port or path.
pub(super) fn matches_hostname(&self, host: &str) -> bool {
match self {
Self::ConfiguredHosts(destinations) => destinations
.iter()
.any(|destination| destination.matches_hostname(host)),
Self::ExactHost(_) | Self::ExactHosts(_) | Self::HostPattern { .. } => {
self.matches_host(host, /*port*/ 443)
}
}
}
pub(super) fn requires_mitm(&self, host: &str, port: u16) -> bool {
match self {
Self::ConfiguredHosts(destinations) => destinations
codex-rs/network-proxy/src/credential_broker/registry.rs
+36 -5
View File
@@ -10,6 +10,7 @@ use super::matching::is_operational_path_match;
use super::providers;
use super::source_accepts_credential;
use super::source_tracks_credential;
use crate::policy::normalize_host;
use base64::Engine;
use rama_http::HeaderMap;
use rama_http::HeaderName;
@@ -189,17 +190,30 @@ pub(super) fn is_builtin_shaped_credential(value: &str) -> bool {
pub(super) fn select_credentials<'a>(
headers: &HeaderMap,
host: &str,
request: Option<&Url>,
destination: &str,
credentials: &'a [CredentialRecord],
environment_id: Option<&str>,
) -> Vec<(&'a CredentialRecord, HeaderName, HeaderValue)> {
let request = if destination.contains("://") {
let Ok(request) = Url::parse(destination) else {
return Vec::new();
};
Some(request)
} else {
None
};
let host = normalize_host(
request
.as_ref()
.and_then(Url::host_str)
.unwrap_or(destination),
);
let mut translated_matches = Vec::<(&CredentialRecord, HeaderName, HeaderValue)>::new();
for credential in credentials.iter().filter(|credential| {
credential.belongs_to_environment(environment_id)
&& credential
.host_bindings()
.any(|binding| binding.matches_request(host, request))
.any(|binding| binding.matches_request(&host, request.as_ref()))
}) {
for candidate in credentials.iter().filter(|candidate| {
candidate.belongs_to_environment(environment_id)
@@ -207,7 +221,7 @@ pub(super) fn select_credentials<'a>(
&& candidate.real_value == credential.real_value
&& candidate
.host_bindings()
.any(|binding| binding.matches_request(host, request))
.any(|binding| binding.matches_request(&host, request.as_ref()))
}) {
for (name, value) in credential.provider.translate_request_headers(
headers,
@@ -252,7 +266,7 @@ pub(super) fn select_credentials<'a>(
merge_basic_auth_fields(original, &selected[existing].2, &header_value)
})
{
// Keep the caller's configured-provider raw-path validation for either field.
// Keep configured-provider raw-path validation for either field.
if matches!(
credential.provider,
BrokeredCredentialProvider::Configured(_)
@@ -266,6 +280,23 @@ pub(super) fn select_credentials<'a>(
ambiguous_headers.push(header_name);
}
}
if selected.iter().any(|(credential, _, _)| {
matches!(
&credential.provider,
BrokeredCredentialProvider::Configured(_)
)
}) && let Some(request) = request.as_ref()
{
let Ok(raw_request) = destination.parse::<rama_http::Uri>() else {
return Vec::new();
};
let raw_path = raw_request.path();
if raw_path != request.path()
|| !crate::authorization_path::is_safe_for_authorization(raw_path)
{
return Vec::new();
}
}
selected
}
codex-rs/network-proxy/src/credential_broker_tests.rs
+72
View File
@@ -1435,6 +1435,15 @@ fn openai_credentials_bind_only_to_default_and_configured_trusted_hosts() {
("attacker.example", dummy.as_str()),
] {
let mut headers = headers_with_bearer(dummy);
assert_eq!(
broker.request_matches_hooked_host_alias(
host,
&headers,
&["api.openai.com".to_string()],
/*environment_id*/ None,
),
host != "api.openai.com" && expected_credential == "sk-real",
);
broker.inject_request_headers(host, &mut headers);
let expected = format!("Bearer {expected_credential}");
assert_eq!(authorization(&headers), Some(expected.as_str()), "{host}");
@@ -1457,6 +1466,69 @@ fn openai_credentials_bind_only_to_default_and_configured_trusted_hosts() {
);
}
#[test]
fn selected_openai_credential_matches_ipv6_alias_with_plaintext_fallback() {
let broker = CredentialBroker::new(/*enabled*/ true);
let mut env = env_map([
("OPENAI_API_KEY", "sk-real"),
("OPENAI_BASE_URL", "https://[fd00::1]/v1"),
]);
broker.virtualize_child_env(&mut env);
for destination in ["https://[fd00::1]/v1/models", "fd00::1"] {
assert!(broker.request_matches_hooked_host_alias(
destination,
&headers_with_bearer(&env["OPENAI_API_KEY"]),
&["api.openai.com".to_string()],
/*environment_id*/ None,
));
}
}
#[test]
fn selected_alias_checks_equivalent_and_merged_credential_bindings() {
let broker = CredentialBroker::new(/*enabled*/ true);
let register = |env_var: &str, token: &str, host: &str| {
let mut env = env_map([("GH_HOST", host), (env_var, token)]);
broker.virtualize_child_env_for_environment(&mut env, Some("selected"));
env.insert("GH_HOST".to_string(), "alias.example".to_string());
broker.virtualize_child_env_for_environment(&mut env, Some("selected"));
env[env_var].clone()
};
let first = register("GH_ENTERPRISE_TOKEN", "ghp-first-real", "alias.example");
let equivalent = register(
"GITHUB_ENTERPRISE_TOKEN",
"ghp-first-real",
"hooked.example",
);
let second = register(
"GITHUB_ENTERPRISE_TOKEN",
"ghp-second-real",
"second-hooked.example",
);
assert_ne!(first, equivalent);
assert!(broker.request_matches_hooked_host_alias(
"alias.example",
&headers_with_bearer(&equivalent),
&["hooked.example".to_string()],
Some("selected"),
));
let basic = STANDARD.encode(format!("{first}:{second}"));
let headers = headers_with_authorization(&format!("Basic {basic}"));
assert!(broker.request_matches_hooked_host_alias(
"alias.example",
&headers,
&["second-hooked.example".to_string()],
Some("selected"),
));
assert!(!broker.request_matches_hooked_host_alias(
"alias.example",
&headers,
&["second-hooked.example".to_string()],
Some("other"),
));
}
#[test]
fn github_cloud_credentials_match_ghe_com_host_hint() {
let broker = CredentialBroker::new(/*enabled*/ true);
codex-rs/network-proxy/src/http_proxy.rs
+140 -22
View File
@@ -3,6 +3,7 @@ use crate::config::NetworkMode;
use crate::connect_policy::TargetCheckedTcpConnector;
use crate::connection_lifecycle::CancelOnShutdown;
use crate::mitm;
use crate::mitm_hook::HookEvaluation;
use crate::network_policy::BlockDecisionAuditEventArgs;
use crate::network_policy::NetworkDecision;
use crate::network_policy::NetworkDecisionSource;
@@ -16,6 +17,7 @@ use crate::network_policy::emit_block_decision_audit_event;
use crate::network_policy::evaluate_host_policy;
use crate::policy::normalize_host;
use crate::reasons::REASON_METHOD_NOT_ALLOWED;
use crate::reasons::REASON_MITM_HOOK_DENIED;
use crate::reasons::REASON_MITM_REQUIRED;
use crate::reasons::REASON_NOT_ALLOWED;
use crate::reasons::REASON_PROXY_DISABLED;
@@ -908,13 +910,70 @@ async fn http_plain_proxy(
));
}
if let Err(err) =
inject_forward_request_credentials(app_state.as_ref(), &request_ctx, &mut req).await
let credential_destinations =
match forward_request_credential_destinations(app_state.as_ref(), &request_ctx, &req).await
{
Ok(destinations) => destinations,
Err(err) => {
return Ok(internal_error(
"failed to read credential injection config",
err,
));
}
};
// Evaluate the original Authorization header before replacing brokered dummy credentials.
// Otherwise an unhooked provider alias can receive the real credential without hook policy.
match app_state
.evaluate_mitm_hook_request(&host, &req, &credential_destinations)
.await
{
return Ok(internal_error(
"failed to read plaintext credential injection config",
err,
));
Ok(HookEvaluation::HookedHostNoMatch) => {
emit_http_block_decision_audit_event(
&app_state,
BlockDecisionAuditEventArgs {
source: NetworkDecisionSource::ModeGuard,
reason: REASON_MITM_HOOK_DENIED,
protocol: NetworkProtocol::Http,
server_address: host.as_str(),
server_port: port,
method: Some(req.method().as_str()),
client_addr: client.as_deref(),
},
);
let details = PolicyDecisionDetails {
decision: NetworkPolicyDecision::Deny,
reason: REASON_MITM_HOOK_DENIED,
source: NetworkDecisionSource::ModeGuard,
protocol: NetworkProtocol::Http,
host: &host,
port,
};
let _ = app_state
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_MITM_HOOK_DENIED.to_string(),
client: client.clone(),
method: Some(req.method().as_str().to_string()),
mode: None,
protocol: "http".to_string(),
decision: Some(details.decision.as_str().to_string()),
source: Some(details.source.as_str().to_string()),
port: Some(port),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!(
"request blocked by MITM hook policy (client={client}, host={host}, method={})",
req.method()
);
return Ok(json_blocked(&host, REASON_MITM_HOOK_DENIED, Some(&details)));
}
Ok(HookEvaluation::Matched { .. } | HookEvaluation::NoHooksForHost) => {}
Err(err) => return Ok(internal_error("failed to evaluate MITM hook policy", err)),
}
for destination in credential_destinations {
app_state.inject_request_credentials(&destination, req.headers_mut());
}
let client = client.as_deref().unwrap_or_default();
@@ -946,11 +1005,11 @@ async fn http_plain_proxy(
}
}
async fn inject_forward_request_credentials(
async fn forward_request_credential_destinations(
app_state: &NetworkProxyState,
context: &RequestContext,
req: &mut Request,
) -> Result<()> {
req: &Request,
) -> Result<Vec<String>> {
let authority = context.host_with_port();
let scheme = req.uri().scheme_str().unwrap_or("http");
// Server-wide OPTIONS may use root-scoped credentials without changing its wire target.
@@ -960,16 +1019,11 @@ async fn inject_forward_request_credentials(
.map(rama_http::uri::PathAndQuery::as_str)
.filter(|path| *path != "*")
.unwrap_or("/");
let destination = format!("{scheme}://{authority}{request_path}");
let unrestricted_plaintext = app_state.plaintext_credential_injection_enabled().await?;
app_state.inject_request_credentials(&destination, req.headers_mut());
if unrestricted_plaintext {
app_state.inject_request_credentials(
&normalize_host(&authority.host.to_string()),
req.headers_mut(),
);
let mut destinations = vec![format!("{scheme}://{authority}{request_path}")];
if app_state.plaintext_credential_injection_enabled().await? {
destinations.push(normalize_host(&authority.host.to_string()));
}
Ok(())
Ok(destinations)
}
async fn proxy_via_unix_socket(req: Request, socket_path: &str) -> Result<Response> {
@@ -1382,9 +1436,12 @@ mod tests {
.unwrap();
let context = RequestContext::try_from(&req).unwrap();
inject_forward_request_credentials(&state, &context, &mut req)
for destination in forward_request_credential_destinations(&state, &context, &req)
.await
.unwrap();
.unwrap()
{
state.inject_request_credentials(&destination, req.headers_mut());
}
let expected = if enabled { real_token } else { dummy };
assert_eq!(
@@ -1482,9 +1539,12 @@ mod tests {
.unwrap();
let context = RequestContext::try_from(&req).unwrap();
inject_forward_request_credentials(&state, &context, &mut req)
for destination in forward_request_credential_destinations(&state, &context, &req)
.await
.unwrap();
.unwrap()
{
state.inject_request_credentials(&destination, req.headers_mut());
}
let expected = if inject { real_token } else { dummy };
assert_eq!(
@@ -1497,6 +1557,64 @@ mod tests {
}
}
#[tokio::test]
async fn http_plain_proxy_blocks_brokered_provider_alias_before_credential_injection() {
let mut network = NetworkProxyConfig {
allow_local_binding: Some(true),
credential_broker: true,
dangerously_allow_plaintext_credential_injection: true,
mitm: true,
mitm_hooks: vec![crate::mitm_hook::MitmHookConfig {
host: "api.github.com".to_string(),
matcher: crate::mitm_hook::MitmHookMatchConfig {
methods: vec!["POST".to_string()],
path_prefixes: vec!["/repos/openai/".to_string()],
..crate::mitm_hook::MitmHookMatchConfig::default()
},
actions: crate::mitm_hook::MitmHookActionsConfig::default(),
}],
..NetworkProxyConfig::default()
};
network.set_allowed_domains(vec!["api.github.com".to_string(), "github.com".to_string()]);
let state = Arc::new(network_proxy_state_for_policy(network));
let mut env = HashMap::from([("GH_TOKEN".to_string(), "ghp-real".to_string())]);
state.virtualize_child_credentials(&mut env);
for scheme in ["http", "https"] {
let mut req = Request::builder()
.method(Method::GET)
.uri(format!("{scheme}://github.com/repos/openai/codex/issues"))
.header(header::HOST, "github.com")
.header(header::AUTHORIZATION, format!("Bearer {}", env["GH_TOKEN"]))
.body(Body::empty())
.expect("absolute-form provider-alias request should build");
req.extensions_mut().insert(state.clone());
let response = http_plain_proxy(
/*policy_decider*/ None, /*environment_id*/ None, req,
)
.await
.expect("provider-alias policy should return a response");
assert_eq!(response.status(), StatusCode::FORBIDDEN);
assert_eq!(
response.headers().get("x-proxy-error").unwrap(),
"blocked-by-mitm-hook"
);
}
let blocked = state.drain_blocked().await.unwrap();
assert_eq!(blocked.len(), 2);
assert!(blocked.iter().all(|request| {
request.host == "github.com"
&& request.reason == REASON_MITM_HOOK_DENIED
&& request.protocol == "http"
&& request.source.as_deref() == Some("mode_guard")
}));
assert_eq!(blocked[0].port, Some(80));
assert_eq!(blocked[1].port, Some(443));
}
#[tokio::test]
async fn http_connect_accept_blocks_hooked_host_in_full_mode_without_mitm_state() {
let mut policy = NetworkProxyConfig {
codex-rs/network-proxy/src/mitm.rs
+16 -7
View File
@@ -77,6 +77,16 @@ struct MitmPolicyContext {
app_state: Arc<NetworkProxyState>,
}
impl MitmPolicyContext {
fn credential_destination(&self, req: &Request) -> String {
let authority = authority_header_value(&self.target_host, self.target_port, &self.scheme);
let path = path_and_query(req.uri());
// Server-wide OPTIONS must not borrow authority from a narrower credential URL prefix.
let credential_path = if path == "*" { "/" } else { &path };
format!("{}://{authority}{credential_path}", self.scheme)
}
}
#[derive(Clone)]
struct MitmRequestContext {
policy: MitmPolicyContext,
@@ -248,6 +258,7 @@ async fn handle_mitm_request(
}
async fn forward_request(req: Request, request_ctx: &MitmRequestContext) -> Result<Response> {
let credential_destination = request_ctx.policy.credential_destination(&req);
let hook_actions = match evaluate_mitm_policy(&req, &request_ctx.policy).await? {
MitmPolicyDecision::Allow { hook_actions } => hook_actions,
MitmPolicyDecision::Block(response) => return Ok(response),
@@ -268,12 +279,6 @@ async fn forward_request(req: Request, request_ctx: &MitmRequestContext) -> Resu
.authority(authority.as_str())
.path_and_query(path.as_str())
.build()?;
// Server-wide OPTIONS must not borrow authority from a narrower credential URL prefix.
let credential_path = if path == "*" { "/" } else { &path };
let credential_destination = format!(
"{}://{authority}{credential_path}",
request_ctx.policy.scheme
);
request_ctx
.policy
.app_state
@@ -424,7 +429,11 @@ async fn evaluate_mitm_policy(
let hook_actions = match policy
.app_state
.evaluate_mitm_hook_request(&policy.target_host, req)
.evaluate_mitm_hook_request(
&policy.target_host,
req,
&[policy.credential_destination(req)],
)
.await?
{
HookEvaluation::Matched { actions } => Some(actions),
codex-rs/network-proxy/src/mitm_tests.rs
+147
View File
@@ -1,10 +1,13 @@
use super::*;
use crate::CredentialProviderConfig;
use crate::config::NetworkProxyConfig;
use crate::reasons::REASON_METHOD_NOT_ALLOWED;
use crate::reasons::REASON_MITM_HOOK_DENIED;
use crate::reasons::REASON_NOT_ALLOWED_LOCAL;
use crate::runtime::network_proxy_state_for_policy;
use base64::Engine as _;
use base64::engine::general_purpose::STANDARD;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use rama_http::Body;
@@ -14,6 +17,7 @@ use rama_http::Method;
use rama_http::Request;
use rama_http::StatusCode;
use rama_http::header::HeaderName;
use std::collections::HashMap;
use tempfile::NamedTempFile;
fn github_write_hook() -> crate::mitm_hook::MitmHookConfig {
@@ -229,6 +233,149 @@ async fn mitm_policy_allows_matching_hooked_write_in_full_mode() {
assert_eq!(app_state.blocked_snapshot().await.unwrap().len(), 0);
}
#[tokio::test]
async fn mitm_policy_enforces_brokered_aliases_without_blocking_unrelated_credentials() {
for hooked_hosts in [
vec!["api.github.com"],
vec!["api.github.com", "github.com"],
vec![],
] {
let network = NetworkProxyConfig {
credential_broker: true,
mitm: true,
allow_local_binding: Some(true),
mitm_hooks: hooked_hosts
.iter()
.map(|host| {
let mut hook = github_write_hook();
hook.host = (*host).to_string();
hook.matcher.methods = vec!["GET".to_string()];
hook.actions.inject_request_headers.clear();
hook
})
.collect(),
..NetworkProxyConfig::default()
};
let app_state = Arc::new(network_proxy_state_for_policy(network));
let mut env = HashMap::from([
("GH_TOKEN".to_string(), "ghp-real".to_string()),
("OPENAI_API_KEY".to_string(), "sk-real".to_string()),
("GH_HOST".to_string(), "enterprise.example".to_string()),
(
"GH_ENTERPRISE_TOKEN".to_string(),
"ghp-enterprise-real".to_string(),
),
]);
app_state.virtualize_child_credentials(&mut env);
let github = format!("Bearer {}", env["GH_TOKEN"]);
let token = format!("token {}", env["GH_TOKEN"]);
let basic = format!(
"Basic {}",
STANDARD.encode(format!("x-access-token:{}", env["GH_TOKEN"]))
);
let openai = format!("Bearer {}", env["OPENAI_API_KEY"]);
let enterprise = format!("Bearer {}", env["GH_ENTERPRISE_TOKEN"]);
for (host, authorization, uses_cloud_credential) in [
("github.com", Some(github.as_str()), true),
("api.astemu.ghe.com", Some(github.as_str()), true),
("github.com", Some(token.as_str()), true),
("github.com", Some(basic.as_str()), true),
("github.com", None, false),
("github.com", Some("Bearer unrelated-token"), false),
("github.com", Some(openai.as_str()), false),
("enterprise.example", Some(enterprise.as_str()), false),
] {
let ctx = policy_ctx(
app_state.clone(),
NetworkMode::Full,
host,
/*target_port*/ 443,
);
let mut req = Request::builder()
.method(Method::GET)
.uri("/repos/openai/codex/issues")
.header(HOST, host)
.body(Body::empty())
.unwrap();
if let Some(value) = authorization {
req.headers_mut()
.insert("authorization", HeaderValue::from_str(value).unwrap());
}
let denied =
uses_cloud_credential && !hooked_hosts.is_empty() && !hooked_hosts.contains(&host);
let response = mitm_blocking_response(&req, &ctx).await.unwrap();
assert_eq!(
response.as_ref().map(Response::status),
denied.then_some(StatusCode::FORBIDDEN),
"host: {host}, hooks: {hooked_hosts:?}"
);
if let Some(response) = response {
assert_eq!(response.headers()["x-proxy-error"], "blocked-by-mitm-hook");
}
}
}
}
#[tokio::test]
async fn mitm_policy_checks_configured_credential_aliases_at_the_request_destination() {
let mut hook = github_write_hook();
hook.host = "api.hooked.example".to_string();
hook.actions.inject_request_headers.clear();
let mut network = NetworkProxyConfig {
credential_broker: true,
mitm: true,
allow_local_binding: Some(true),
mitm_hooks: vec![hook],
..NetworkProxyConfig::default()
};
network.credential_providers.insert(
"custom".to_string(),
CredentialProviderConfig {
env: vec!["PROVIDER_TOKEN".to_string()],
patterns: vec!["provider_[a-z]{24}".to_string()],
// The hooked host need not use the alias's port or path prefix.
url_prefixes: vec![
"https://*.hooked.example/v1".to_string(),
"https://alias.example:8443/v2".to_string(),
],
..CredentialProviderConfig::default()
},
);
let app_state = Arc::new(network_proxy_state_for_policy(network));
let mut env = HashMap::from([(
"PROVIDER_TOKEN".to_string(),
"provider_abcdefghijklmnopqrstuvwx".to_string(),
)]);
app_state.virtualize_child_credentials(&mut env);
let ctx = policy_ctx(
app_state,
NetworkMode::Full,
"alias.example",
/*target_port*/ 8443,
);
for (path, denied) in [
("/v2/models", true),
("/v20/models", false),
("/v2/../v2/models", false),
] {
let req = Request::builder()
.uri(path)
.header(HOST, "alias.example:8443")
.header("authorization", format!("Bearer {}", env["PROVIDER_TOKEN"]))
.body(Body::empty())
.unwrap();
let response = mitm_blocking_response(&req, &ctx).await.unwrap();
assert_eq!(
response.as_ref().map(Response::status),
denied.then_some(StatusCode::FORBIDDEN),
"path: {path}"
);
if let Some(response) = response {
assert_eq!(response.headers()["x-proxy-error"], "blocked-by-mitm-hook");
}
}
}
#[tokio::test]
async fn mitm_policy_blocks_encoded_path_traversal_for_repository_allowlist() {
let mut hook = github_write_hook();
codex-rs/network-proxy/src/runtime.rs
+23 -2
View File
@@ -982,10 +982,31 @@ impl NetworkProxyState {
&self,
host: &str,
req: &rama_http::Request,
credential_destinations: &[String],
) -> Result<HookEvaluation> {
self.reload_if_needed().await?;
let guard = self.state.read().await;
Ok(evaluate_mitm_hooks(&guard.mitm_hooks, host, req))
let hooked_hosts = {
let guard = self.state.read().await;
match evaluate_mitm_hooks(&guard.mitm_hooks, host, req) {
HookEvaluation::NoHooksForHost if !guard.mitm_hooks.is_empty() => {
guard.mitm_hooks.keys().cloned().collect::<Vec<_>>()
}
evaluation => return Ok(evaluation),
}
};
if credential_destinations.iter().any(|destination| {
self.credential_broker.request_matches_hooked_host_alias(
destination,
req.headers(),
&hooked_hosts,
self.environment_id(),
)
}) {
Ok(HookEvaluation::HookedHostNoMatch)
} else {
Ok(HookEvaluation::NoHooksForHost)
}
}
pub(crate) async fn host_mitm_requirement(