From 619531225b29dcf6437ec6571537f9da9da002bc Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 23 Oct 2026 19:46:07 +0000 Subject: [PATCH] Add scoped request-body secret substitution to MITM hooks (#57174) ## What changed - Add `body_secret_substitution` actions to version 2 MITM configuration, replacing exact opaque placeholders with secrets from environment variables or files within the matching hook's scope. - Bound body size, expansion, replacement count, and search work. Reject overlapping matches, compressed payloads, trailers, and supplied payload-integrity headers. - Recompute `Content-Length` and substitute before SigV4 signing so signatures cover the transmitted body. ## Testing Add coverage for markers split across chunks, expansion limits, invalid payloads, overlapping bindings, secret redaction, and search limits. Add a TLS forwarding test that verifies hook scope and independently checks the signature of the substituted body. GitOrigin-RevId: fa95c9ae0c8fc8f695e03c0e87a0a4875d8418fd --- codex-rs/config/src/permissions_toml.rs | 28 +- codex-rs/config/src/permissions_toml_tests.rs | 19 +- codex-rs/core/config.schema.json | 52 ++++ codex-rs/core/src/config/config_tests.rs | 2 + codex-rs/network-proxy/README.md | 18 +- codex-rs/network-proxy/src/lib.rs | 3 + codex-rs/network-proxy/src/mitm.rs | 10 + .../src/mitm_forwarding_tests.rs | 112 ++++++++ codex-rs/network-proxy/src/mitm_hook.rs | 18 ++ codex-rs/network-proxy/src/mitm_tests.rs | 2 + .../network-proxy/src/secret_substitution.rs | 264 ++++++++++++++++++ .../src/secret_substitution_tests.rs | 261 +++++++++++++++++ 12 files changed, 784 insertions(+), 5 deletions(-) create mode 100644 codex-rs/network-proxy/src/secret_substitution.rs create mode 100644 codex-rs/network-proxy/src/secret_substitution_tests.rs diff --git a/codex-rs/config/src/permissions_toml.rs b/codex-rs/config/src/permissions_toml.rs index 00446f0419..66843625ef 100644 --- a/codex-rs/config/src/permissions_toml.rs +++ b/codex-rs/config/src/permissions_toml.rs @@ -397,6 +397,7 @@ enum NetworkModeSchema { #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[serde(default, deny_unknown_fields)] pub struct NetworkMitmActionToml { + pub body_secret_substitution: Option, pub aws_sigv4: Option, pub strip_request_headers: Vec, pub inject_request_headers: Vec, @@ -446,7 +447,11 @@ impl NetworkMitmToml { .inject_request_headers .iter() .any(|header| header.placeholder.is_some()); - if self.version.is_none() && (uses_placeholder || action.aws_sigv4.is_some()) { + if self.version.is_none() + && (uses_placeholder + || action.aws_sigv4.is_some() + || action.body_secret_substitution.is_some()) + { return Err("scoped MITM actions require network.mitm.version = 2".to_string()); } if action.is_empty() { @@ -477,6 +482,21 @@ impl NetworkMitmToml { "network.mitm.hooks.{hook_name} must select at most one AWS signing action" )); } + let substitution_count = hook + .action + .iter() + .filter(|name| { + self.actions + .as_ref() + .and_then(|actions| actions.get(*name)) + .is_some_and(|action| action.body_secret_substitution.is_some()) + }) + .count(); + if substitution_count > 1 { + return Err(format!( + "network.mitm.hooks.{hook_name} must select at most one body substitution action" + )); + } if self.version.is_none() && (!hook.schemes.is_empty() || !hook.ports.is_empty()) { return Err("scoped MITM hooks require network.mitm.version = 2".to_string()); } @@ -508,7 +528,8 @@ impl NetworkMitmToml { impl NetworkMitmActionToml { pub fn is_empty(&self) -> bool { - self.aws_sigv4.is_none() + self.body_secret_substitution.is_none() + && self.aws_sigv4.is_none() && self.strip_request_headers.is_empty() && self.inject_request_headers.is_empty() } @@ -608,6 +629,9 @@ impl NetworkMitmHookToml { let mut selected = MitmHookActionsConfig::default(); for action_name in &self.action { if let Some(action) = actions_by_name.get(action_name) { + if action.body_secret_substitution.is_some() { + selected.body_secret_substitution = action.body_secret_substitution.clone(); + } if action.aws_sigv4.is_some() { selected.aws_sigv4 = action.aws_sigv4.clone(); } diff --git a/codex-rs/config/src/permissions_toml_tests.rs b/codex-rs/config/src/permissions_toml_tests.rs index f3149d4ed8..ed39445748 100644 --- a/codex-rs/config/src/permissions_toml_tests.rs +++ b/codex-rs/config/src/permissions_toml_tests.rs @@ -19,13 +19,18 @@ name = "x-api-key" secret_env_var = "PROXY_API_KEY" placeholder = "dummy-key" +[actions.body.body_secret_substitution] +port = 443 +max_body_bytes = 1024 +replacements = [{ placeholder = "codex-secret-0123456789abcdef", secret = { env_var = "BODY_SECRET" } }] + [hooks.bucket] host = "bucket.s3.amazonaws.com" schemes = ["https"] ports = [443] methods = ["PUT"] path_prefixes = ["/workspace/"] -action = ["substitute", "aws"] +action = ["substitute", "body", "aws"] "#; #[test] @@ -35,6 +40,16 @@ fn resolves_native_signing_and_substitution_actions_from_toml() { assert_eq!(hooks[0].matcher.schemes, vec!["https"]); assert_eq!(hooks[0].matcher.ports, vec![443]); let signer = hooks[0].actions.aws_sigv4.as_ref().unwrap(); + assert_eq!( + hooks[0] + .actions + .body_secret_substitution + .as_ref() + .unwrap() + .replacements[0] + .secret, + ProxySecretSource::EnvVar("BODY_SECRET".into()) + ); assert_eq!( signer.access_key_id, ProxySecretSource::EnvVar("PROXY_ACCESS_KEY".into()) @@ -110,7 +125,7 @@ fn rejects_misspelled_header_placeholder() { fn rejects_multiple_signing_actions_and_unknown_signing_parameters() { assert!( toml::from_str::(&SIGNING.replace( - "action = [\"substitute\", \"aws\"]", + "action = [\"substitute\", \"body\", \"aws\"]", "action = [\"aws\", \"aws\"]" )) .is_err() diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 2c9b7a3f51..264e19c8e0 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -566,6 +566,50 @@ ], "type": "object" }, + "BodySecretReplacement": { + "additionalProperties": false, + "properties": { + "placeholder": { + "type": "string" + }, + "secret": { + "$ref": "#/definitions/ProxySecretSource" + } + }, + "required": [ + "placeholder", + "secret" + ], + "type": "object" + }, + "BodySecretSubstitutionConfig": { + "additionalProperties": false, + "description": "Exact opaque placeholders scoped by their enclosing host/method/path MITM hook.", + "properties": { + "max_body_bytes": { + "default": 8388608, + "format": "uint", + "minimum": 0.0, + "type": "integer" + }, + "port": { + "default": 443, + "format": "uint16", + "minimum": 0.0, + "type": "integer" + }, + "replacements": { + "items": { + "$ref": "#/definitions/BodySecretReplacement" + }, + "type": "array" + } + }, + "required": [ + "replacements" + ], + "type": "object" + }, "BrowserUseConfigToml": { "additionalProperties": false, "properties": { @@ -3216,6 +3260,14 @@ ], "default": null }, + "body_secret_substitution": { + "allOf": [ + { + "$ref": "#/definitions/BodySecretSubstitutionConfig" + } + ], + "default": null + }, "inject_request_headers": { "default": [], "items": { diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index ebbdec8aa1..aa9c3831ed 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1683,6 +1683,7 @@ strip_request_headers = ["authorization"] actions: Some(IndexMap::from([( "strip_auth".to_string(), NetworkMitmActionToml { + body_secret_substitution: None, aws_sigv4: None, strip_request_headers: vec!["authorization".to_string()], inject_request_headers: Vec::new(), @@ -1766,6 +1767,7 @@ fn permissions_profile_network_to_proxy_config_preserves_mitm_hooks() { actions: Some(IndexMap::from([( "strip_auth".to_string(), NetworkMitmActionToml { + body_secret_substitution: None, aws_sigv4: None, strip_request_headers: vec!["authorization".to_string()], inject_request_headers: Vec::new(), diff --git a/codex-rs/network-proxy/README.md b/codex-rs/network-proxy/README.md index 89a0271d7c..6556281f73 100644 --- a/codex-rs/network-proxy/README.md +++ b/codex-rs/network-proxy/README.md @@ -309,7 +309,23 @@ An injected header can additionally set `placeholder` to require one exact whole-header value before substituting the secret. Missing, mismatched, or duplicate values fail closed. This form requires HTTPS port 443 and cannot modify routing or framing headers. Resolved secret headers and signing headers -are marked sensitive in diagnostics. +are marked sensitive in diagnostics. Body substitution is available as `body_secret_substitution` with `replacements` +(each containing an exact `placeholder` and a trusted `secret` source), `port`, +and `max_body_bytes`. Markers must be opaque ASCII values of 16–256 bytes. +Input and expanded output are bounded (8 MiB default, 16 MiB maximum), each +resolved credential is capped at 64 KiB, and at most 65,536 occurrences may be +replaced. Search work is capped at 256 MiB of worst-case byte comparisons; +requests may exceed this limit before reaching the body-size limit. +Compressed payloads, trailers, supplied payload-integrity headers, and AWS +streaming signatures fail closed. Markers split across HTTP body chunks are +supported; overlapping +bindings or occurrences are rejected. Replacement is byte-exact and does not +perform JSON/form escaping or regex interpretation. The caller must choose +markers appropriate for the payload encoding. Content length is recomputed, +and SigV4 runs after substitution so it hashes the actual transmitted body. +At most one signing action and one body substitution action may match a request; +combine body bindings in one action to avoid ambiguous transformation order. + All native buffering stages share eight request slots, with a five-second bounded wait. Each lease follows the prepared body until consumed or dropped; the 16 MiB body cap bounds retained memory even when upstream connections are slow. diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 62e3bff2e8..30cd34bcb3 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -26,6 +26,7 @@ mod request_cancellation; mod request_disconnect; mod responses; mod runtime; +mod secret_substitution; mod socket_path; mod socks5; mod state; @@ -126,6 +127,8 @@ pub use runtime::ConfigReloaderFuture; pub use runtime::ConfigState; pub use runtime::LocalBindingPolicy; pub use runtime::NetworkProxyState; +pub use secret_substitution::BodySecretReplacement; +pub use secret_substitution::BodySecretSubstitutionConfig; pub use state::NetworkProxyAuditMetadata; pub use state::NetworkProxyConstraintError; pub use state::NetworkProxyConstraints; diff --git a/codex-rs/network-proxy/src/mitm.rs b/codex-rs/network-proxy/src/mitm.rs index a90045a875..01784c7be5 100644 --- a/codex-rs/network-proxy/src/mitm.rs +++ b/codex-rs/network-proxy/src/mitm.rs @@ -281,6 +281,7 @@ async fn handle_mitm_request( request_ctx: Arc, ) -> Result { crate::aws_sigv4::preserve_signing_headers(&mut req); + crate::secret_substitution::preserve_body_substitution_headers(&mut req); let response = match (request_ctx.policy.scheme == Scheme::HTTPS) .then(RemoveRequestHeaderLayer::hop_by_hop) .into_layer(service_fn(move |req| { @@ -369,6 +370,15 @@ async fn forward_request(req: Request, request_ctx: &MitmRequestContext) -> Resu }; let mut upstream_req = Request::from_parts(parts, body); + if let Some(substitution) = hook_actions + .as_ref() + .and_then(|actions| actions.body_secret_substitution.as_ref()) + { + let Ok(substituted) = substitution.apply(upstream_req).await else { + return Ok(request_ctx.policy.deny_hook(&method, client).await); + }; + upstream_req = substituted; + } if let Some((signer, guest)) = signing { let Ok(signed) = signer.sign(upstream_req, &guest).await else { return Ok(request_ctx.policy.deny_hook(&method, client).await); diff --git a/codex-rs/network-proxy/src/mitm_forwarding_tests.rs b/codex-rs/network-proxy/src/mitm_forwarding_tests.rs index e920c22537..057806f369 100644 --- a/codex-rs/network-proxy/src/mitm_forwarding_tests.rs +++ b/codex-rs/network-proxy/src/mitm_forwarding_tests.rs @@ -36,6 +36,15 @@ async fn tls_origin( dir: &Path, service: &str, alpn: &[u8], +) -> (MitmRequestContext, TcpListener, TlsAcceptor) { + tls_origin_with_hook(dir, service, alpn, |_, _| {}).await +} + +async fn tls_origin_with_hook( + dir: &Path, + service: &str, + alpn: &[u8], + configure: impl FnOnce(&mut MitmHookConfig, u16), ) -> (MitmRequestContext, TcpListener, TlsAcceptor) { ensure_rustls_crypto_provider(); let key = KeyPair::generate().expect("generate origin key"); @@ -94,6 +103,7 @@ async fn tls_origin( }], ..Default::default() }; + configure(&mut config.mitm_hooks[0], port); config.set_credential_broker_enabled(/*enabled*/ true); config.set_allowed_domains(vec!["localhost".to_string()]); let app_state = Arc::new(network_proxy_state_for_policy(config)); @@ -479,3 +489,105 @@ async fn drained_signed_bodies_release_capacity_before_response_headers() { .expect("drained bodies must release buffer slots before response headers"); assert_eq!(response.status(), StatusCode::OK); } + +#[tokio::test] +async fn forwards_substituted_body_before_signing() { + let marker = b"codex-secret-0123456789abcdef"; + let dir = tempdir().expect("TLS fixture directory"); + let secret = dir.path().join("body-secret"); + std::fs::write(&secret, "expanded-payload").unwrap(); + let configure = |hook: &mut MitmHookConfig, port| { + hook.matcher.path_prefixes = vec!["/allowed/".into()]; + hook.actions.strip_request_headers.push("digest".into()); + hook.actions.body_secret_substitution = Some(crate::BodySecretSubstitutionConfig { + replacements: vec![crate::BodySecretReplacement { + placeholder: String::from_utf8(marker.to_vec()).unwrap(), + secret: ProxySecretSource::File(secret.display().to_string()), + }], + port, + max_body_bytes: 64, + }); + }; + let (context, listener, acceptor) = + tls_origin_with_hook(dir.path(), "s3", b"http/1.1", configure).await; + let context = Arc::new(context); + let signed_headers = "content-length;cookie;host;x-amz-date"; + let make_request = |path: &str| { + let mut req = request(&context, "s3", "POST", path, marker); + *req.version_mut() = rama_http::Version::HTTP_2; + let headers = req.headers_mut(); + headers.remove("x-amz-content-sha256"); + headers.insert( + "authorization", + HeaderValue::from_str(&guest_authorization("s3", signed_headers)).unwrap(), + ); + headers.insert("content-length", HeaderValue::from(marker.len())); + headers.append("cookie", HeaderValue::from_static("a=1")); + headers.append("cookie", HeaderValue::from_static("b=2")); + req + }; + let denied = handle_mitm_request(make_request("/denied"), context.clone()) + .await + .unwrap(); + assert_eq!(denied.status(), StatusCode::FORBIDDEN); + let mut invalid = make_request("/allowed/item"); + invalid + .headers_mut() + .insert("digest", HeaderValue::from_static("sha-256=guest")); + let denied = handle_mitm_request(invalid, context.clone()).await.unwrap(); + assert_eq!(denied.status(), StatusCode::FORBIDDEN); + let origin = tokio::spawn(async move { + let (socket, _) = listener.accept().await.unwrap(); + let mut stream = acceptor.accept(socket).await.unwrap(); + let wire = read_request(&mut stream).await; + stream.write_all(RESPONSE).await.unwrap(); + wire + }); + let req = make_request("/allowed/item"); + let response = timeout(FORWARD_TIMEOUT, handle_mitm_request(req, context)) + .await + .unwrap() + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let wire = origin.await.unwrap(); + assert_eq!(wire.body.as_slice(), b"expanded-payload"); + assert_eq!(wire.path, "/allowed/item"); + let headers = wire.headers; + assert_eq!(headers["cookie"], "a=1; b=2"); + let expected_hash = format!("{:x}", sha2::Sha256::digest(&wire.body)); + assert_eq!(headers["x-amz-content-sha256"], expected_hash); + // Rebuild the canonical request from the received HTTP/1.1 headers, without + // the signer's request preparation or the SDK's header canonicalization. + let canonical_headers = signed_headers + .split(';') + .map(|name| format!("{name}:{}\n", headers[name])) + .collect::(); + let canonical_request = + format!("POST\n/allowed/item\n\n{canonical_headers}\n{signed_headers}\n{expected_hash}"); + let timestamp = &headers["x-amz-date"]; + let signing_time = chrono::NaiveDateTime::parse_from_str(timestamp, "%Y%m%dT%H%M%SZ") + .unwrap() + .and_utc(); + let scope = format!( + "{}/us-east-1/s3/aws4_request", + signing_time.format("%Y%m%d") + ); + let string_to_sign = format!( + "AWS4-HMAC-SHA256\n{timestamp}\n{scope}\n{:x}", + sha2::Sha256::digest(canonical_request.as_bytes()) + ); + let signing_key = aws_sigv4::sign::v4::generate_signing_key( + "test-secret-key", + signing_time.into(), + "us-east-1", + "s3", + ); + let signature = + aws_sigv4::sign::v4::calculate_signature(signing_key, string_to_sign.as_bytes()); + assert_eq!( + headers["authorization"], + format!( + "AWS4-HMAC-SHA256 Credential=AKIDEXAMPLE/{scope}, SignedHeaders={signed_headers}, Signature={signature}" + ) + ); +} diff --git a/codex-rs/network-proxy/src/mitm_hook.rs b/codex-rs/network-proxy/src/mitm_hook.rs index 61e385949d..ca1cde1b8f 100644 --- a/codex-rs/network-proxy/src/mitm_hook.rs +++ b/codex-rs/network-proxy/src/mitm_hook.rs @@ -5,6 +5,8 @@ use crate::aws_sigv4::AwsSigV4Config; use crate::aws_sigv4::AwsSigV4Signer; use crate::config::NetworkProxyConfig; use crate::policy::normalize_host; +use crate::secret_substitution::BodySecretSubstitution; +use crate::secret_substitution::BodySecretSubstitutionConfig; use anyhow::Context as _; use anyhow::Result; use anyhow::anyhow; @@ -52,6 +54,7 @@ pub struct MitmHookMatchConfig { #[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)] #[serde(default)] pub struct MitmHookActionsConfig { + pub body_secret_substitution: Option, pub aws_sigv4: Option, pub strip_request_headers: Vec, pub inject_request_headers: Vec, @@ -104,6 +107,7 @@ pub struct HeaderConstraint { #[derive(Debug, Clone, PartialEq, Eq)] pub struct MitmHookActions { + pub(crate) body_secret_substitution: Option>, pub(crate) aws_sigv4: Option>, pub strip_request_headers: Vec, pub inject_request_headers: Vec, @@ -245,6 +249,9 @@ pub(crate) fn validate_mitm_hook_config(config: &NetworkProxyConfig) -> Result<( if let Some(signing) = &hook.actions.aws_sigv4 { signing.validate()?; } + if let Some(substitution) = &hook.actions.body_secret_substitution { + substitution.validate()?; + } validate_injected_headers(&hook.actions.inject_request_headers).with_context(|| { format!("invalid network.mitm_hooks[{hook_index}].actions.inject_request_headers") })?; @@ -373,6 +380,16 @@ where body: None, }, actions: MitmHookActions { + body_secret_substitution: hook + .actions + .body_secret_substitution + .as_ref() + .map(|substitution| { + substitution + .compile(&resolve_env_var, &read_secret_file) + .map(Arc::new) + }) + .transpose()?, aws_sigv4: hook .actions .aws_sigv4 @@ -803,6 +820,7 @@ mod tests { ..MitmHookMatchConfig::default() }, actions: MitmHookActionsConfig { + body_secret_substitution: None, aws_sigv4: None, strip_request_headers: vec!["authorization".to_string()], inject_request_headers: vec![InjectedHeaderConfig { diff --git a/codex-rs/network-proxy/src/mitm_tests.rs b/codex-rs/network-proxy/src/mitm_tests.rs index f3e74e3a31..d4fcd34ce9 100644 --- a/codex-rs/network-proxy/src/mitm_tests.rs +++ b/codex-rs/network-proxy/src/mitm_tests.rs @@ -29,6 +29,7 @@ fn github_write_hook() -> crate::mitm_hook::MitmHookConfig { ..crate::mitm_hook::MitmHookMatchConfig::default() }, actions: crate::mitm_hook::MitmHookActionsConfig { + body_secret_substitution: None, aws_sigv4: None, strip_request_headers: vec!["authorization".to_string()], inject_request_headers: vec![crate::mitm_hook::InjectedHeaderConfig { @@ -572,6 +573,7 @@ fn request_credentials_apply_hook_after_brokerage() { let mut secret = HeaderValue::from_static("Bearer secret-token"); secret.set_sensitive(true); let actions = crate::mitm_hook::MitmHookActions { + body_secret_substitution: None, aws_sigv4: None, strip_request_headers: vec![HeaderName::from_static("authorization")], inject_request_headers: vec![crate::mitm_hook::ResolvedInjectedHeader { diff --git a/codex-rs/network-proxy/src/secret_substitution.rs b/codex-rs/network-proxy/src/secret_substitution.rs new file mode 100644 index 0000000000..ff80820952 --- /dev/null +++ b/codex-rs/network-proxy/src/secret_substitution.rs @@ -0,0 +1,264 @@ +//! Bounded byte-exact secret substitution after destination authorization, before signing. +//! Placeholder bindings and resolved secret values are private to the trusted proxy. + +use crate::aws_sigv4::ProxySecretSource; +use anyhow::Result; +use anyhow::anyhow; +use codex_utils_absolute_path::AbsolutePathBuf; +use rama_core::extensions::ExtensionsMut; +use rama_core::extensions::ExtensionsRef; +use rama_http::Body; +use rama_http::HeaderMap; +use rama_http::HeaderValue; +use rama_http::Request; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; +use std::time::Duration; + +// Bound the worst-case number of bytes compared by the placeholder window scans. +const MAX_SUBSTITUTION_SEARCH_BYTES: usize = 256 * 1024 * 1024; + +#[derive(Clone, Copy, Debug)] +struct OriginalBodyHeadersSupported(bool); + +pub(crate) fn preserve_body_substitution_headers(req: &mut Request) { + if req + .extensions() + .get::() + .is_none() + { + let supported = body_substitution_headers_supported(req.headers()); + req.extensions_mut() + .insert(OriginalBodyHeadersSupported(supported)); + } +} + +fn body_substitution_headers_supported(headers: &HeaderMap) -> bool { + let identity_encoding = headers.get_all("content-encoding").iter().all(|value| { + value.to_str().is_ok_and(|value| { + value + .split(',') + .all(|coding| coding.trim().eq_ignore_ascii_case("identity")) + }) + }); + identity_encoding + && crate::body_budget::supported_transfer_framing(headers) + && !crate::body_budget::is_amazon_event_stream(headers) + && !headers.contains_key("trailer") + && !headers.contains_key("x-amz-trailer") + && !headers.contains_key("x-amz-decoded-content-length") + && !headers.keys().any(|name| { + matches!( + name.as_str(), + "content-md5" + | "digest" + | "content-digest" + | "repr-digest" + | "x-amz-content-sha256" + ) || name.as_str().starts_with("x-amz-checksum-") + }) +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct BodySecretReplacement { + pub placeholder: String, + pub secret: ProxySecretSource, +} + +/// Exact opaque placeholders scoped by their enclosing host/method/path MITM hook. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct BodySecretSubstitutionConfig { + pub replacements: Vec, + #[serde(default = "https_port")] + pub port: u16, + #[serde(default = "default_max_body_bytes")] + pub max_body_bytes: usize, +} + +pub(crate) fn https_port() -> u16 { + 443 +} +pub(crate) fn default_max_body_bytes() -> usize { + 8 * 1024 * 1024 +} + +#[derive(Clone, PartialEq, Eq)] +pub(crate) struct BodySecretSubstitution { + config: BodySecretSubstitutionConfig, + secrets: Vec>, +} + +impl std::fmt::Debug for BodySecretSubstitution { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("BodySecretSubstitution") + .field("config", &self.config) + .finish_non_exhaustive() + } +} + +impl BodySecretSubstitutionConfig { + pub(crate) fn validate(&self) -> Result<()> { + if self.port == 0 + || self.max_body_bytes > 16 * 1024 * 1024 + || self.replacements.is_empty() + || self.replacements.len() > 32 + { + return Err(anyhow!( + "invalid body substitution port, bindings, or limit (maximum 16 MiB)" + )); + } + for (index, replacement) in self.replacements.iter().enumerate() { + if !(16..=256).contains(&replacement.placeholder.len()) + || !replacement.placeholder.is_ascii() + { + return Err(anyhow!( + "body placeholders must be opaque ASCII markers of 16 to 256 bytes" + )); + } + replacement.secret.validate()?; + if self.replacements[..index].iter().any(|previous| { + previous.placeholder.contains(&replacement.placeholder) + || replacement.placeholder.contains(&previous.placeholder) + }) { + return Err(anyhow!("body substitution bindings overlap")); + } + } + Ok(()) + } + + pub(crate) fn compile( + &self, + env: &impl Fn(&str) -> Option, + file: &impl Fn(&AbsolutePathBuf) -> Result, + ) -> Result { + self.validate()?; + let secrets = self + .replacements + .iter() + .map(|replacement| { + let secret = replacement.secret.resolve(env, file)?; + if secret.len() > self.max_body_bytes + || self + .replacements + .iter() + .any(|binding| secret.contains(&binding.placeholder)) + { + return Err(anyhow!( + "body substitution secret exceeds limit or contains a placeholder" + )); + } + Ok(secret.into_bytes()) + }) + .collect::>>()?; + Ok(BodySecretSubstitution { + config: self.clone(), + secrets, + }) + } +} + +impl BodySecretSubstitution { + pub(crate) async fn apply(&self, mut req: Request) -> Result { + if req.uri().scheme_str() != Some("https") + || req.uri().port_u16().unwrap_or(443) != self.config.port + { + return Err(anyhow!( + "body substitution requires its configured HTTPS port" + )); + } + if req + .extensions() + .get::() + .is_some_and(|original| !original.0) + || !body_substitution_headers_supported(req.headers()) + { + return Err(anyhow!( + "body substitution does not support encoded, streaming-signed, or integrity-protected payloads" + )); + } + let _permit = crate::body_budget::reserve(&mut req).await?; + let (mut parts, body) = req.into_parts(); + let (body, trailers) = tokio::time::timeout( + Duration::from_secs(30), + crate::body_budget::collect_body(body, self.config.max_body_bytes), + ) + .await + .map_err(|_| anyhow!("body substitution read timed out"))? + .map_err(|_| anyhow!("body substitution exceeds limit or cannot read payload"))?; + if trailers.is_some() { + return Err(anyhow!( + "body substitution does not support request trailers" + )); + } + if !crate::body_budget::content_length_matches(&parts.headers, body.len()) { + return Err(anyhow!( + "body substitution content length does not match the request body" + )); + } + // Charge every window for its full length before searching, including failed matches. + let search_bytes = self + .config + .replacements + .iter() + .try_fold(0usize, |work, binding| { + let windows = body.len().saturating_sub(binding.placeholder.len() - 1); + work.checked_add(windows.checked_mul(binding.placeholder.len())?) + }); + if search_bytes.is_none_or(|work| work > MAX_SUBSTITUTION_SEARCH_BYTES) { + return Err(anyhow!("body substitution search exceeds work limit")); + } + // Search the original bytes only. Replacements cannot recursively expand other bindings. + let mut matches = Vec::new(); + for (index, binding) in self.config.replacements.iter().enumerate() { + for (offset, window) in body.windows(binding.placeholder.len()).enumerate() { + if window == binding.placeholder.as_bytes() { + if matches.len() >= 65_536 { + return Err(anyhow!("too many body substitution occurrences")); + } + matches.push((offset, offset + binding.placeholder.len(), index)); + } + } + } + matches.sort_unstable(); + let mut cursor = 0; + let mut size = body.len(); + for &(start, end, index) in &matches { + if start < cursor { + return Err(anyhow!("body substitution matches overlap")); + } + size = size + .checked_sub(end - start) + .and_then(|size| size.checked_add(self.secrets[index].len())) + .ok_or_else(|| anyhow!("expanded body substitution exceeds limit"))?; + cursor = end; + } + if size > self.config.max_body_bytes { + return Err(anyhow!("expanded body substitution exceeds limit")); + } + let mut output = Vec::with_capacity(size); + cursor = 0; + for (start, end, index) in matches { + output.extend_from_slice(&body[cursor..start]); + output.extend_from_slice(&self.secrets[index]); + cursor = end; + } + output.extend_from_slice(&body[cursor..]); + parts.headers.remove("transfer-encoding"); + parts.headers.remove("trailer"); + parts.headers.insert( + "content-length", + HeaderValue::from_str(&output.len().to_string())?, + ); + Ok(crate::body_budget::retain(Request::from_parts( + parts, + Body::from(output), + ))) + } +} + +#[cfg(test)] +#[path = "secret_substitution_tests.rs"] +mod tests; diff --git a/codex-rs/network-proxy/src/secret_substitution_tests.rs b/codex-rs/network-proxy/src/secret_substitution_tests.rs new file mode 100644 index 0000000000..8d432105bc --- /dev/null +++ b/codex-rs/network-proxy/src/secret_substitution_tests.rs @@ -0,0 +1,261 @@ +//! Exact substitution preserves chunk boundaries, authorization scope, and signed wire bytes. + +use super::*; +use pretty_assertions::assert_eq; +use rama_core::bytes::Bytes; +use rama_core::futures::stream; +use rama_http::body::util::BodyExt; + +const MARKER: &str = "codex-secret-0123456789abcdef"; + +fn config() -> BodySecretSubstitutionConfig { + BodySecretSubstitutionConfig { + replacements: vec![BodySecretReplacement { + placeholder: MARKER.into(), + secret: ProxySecretSource::EnvVar("SECRET".into()), + }], + port: 443, + max_body_bytes: 1024, + } +} + +fn compile(config: BodySecretSubstitutionConfig, secret: &str) -> BodySecretSubstitution { + config + .compile(&|_| Some(secret.into()), &|_| { + panic!("unexpected file read") + }) + .unwrap() +} + +#[tokio::test] +async fn substitutes_split_markers_and_updates_framing() { + let body = Body::from_stream(stream::iter([ + Ok::<_, std::io::Error>(Bytes::from_static(b"before:codex-secret-0123")), + Ok(Bytes::from_static(b"456789abcdef:after")), + ])); + let request = Request::builder() + .method("PUT") + .uri("https://bucket.s3.amazonaws.com/object") + .header("transfer-encoding", "chunked") + .header("content-encoding", "Identity, identity") + .header("content-encoding", "IDENTITY") + .body(body) + .unwrap(); + let substituted = compile(config(), "actual-secret") + .apply(request) + .await + .unwrap(); + assert_eq!(substituted.headers()["content-length"], "26"); + assert!(!substituted.headers().contains_key("transfer-encoding")); + let payload = b"before:actual-secret:after"; + assert_eq!( + substituted + .into_body() + .collect() + .await + .unwrap() + .to_bytes() + .as_ref(), + payload + ); +} + +#[tokio::test] +async fn scope_overflow_encoding_trailers_and_ambiguous_matches_fail_closed() { + let mut small = config(); + small.max_body_bytes = 40; + let transform = compile(small, "long-replacement-value"); + for (uri, body) in [ + ("http://example.com/", Body::from(MARKER)), + ("https://example.com:444/", Body::from(MARKER)), + ("https://example.com/", Body::from(vec![b'x'; 41])), + ( + "https://example.com/", + Body::empty().with_trailer_headers(rama_http::HeaderMap::new()), + ), + ] { + let req = Request::builder().uri(uri).body(body).unwrap(); + assert!(transform.apply(req).await.is_err()); + } + // Cover expansion and contraction in both orders at the final byte limit. + let expanding_marker = "0123456789abcdef"; + let shrinking_marker = "fedcba9876543210"; + let mut mixed = config(); + mixed.max_body_bytes = 32; + mixed.replacements[0].placeholder = expanding_marker.into(); + mixed.replacements.push(BodySecretReplacement { + placeholder: shrinking_marker.into(), + secret: ProxySecretSource::EnvVar("SHRINKING".into()), + }); + for expanding_secret in ["x".repeat(30), "x".repeat(32)] { + let transform = mixed + .compile( + &|name| match name { + "SECRET" => Some(expanding_secret.clone()), + "SHRINKING" => Some("y".into()), + _ => None, + }, + &|_| panic!("unexpected file read"), + ) + .unwrap(); + for (body, expected) in [ + ( + format!("{expanding_marker}{shrinking_marker}"), + format!("{expanding_secret}y"), + ), + ( + format!("{shrinking_marker}{expanding_marker}"), + format!("y{expanding_secret}"), + ), + ] { + let result = transform + .apply( + Request::builder() + .uri("https://example.com/") + .header("content-length", "32") + .body(Body::from(body)) + .unwrap(), + ) + .await; + if expected.len() > mixed.max_body_bytes { + assert_eq!( + result.unwrap_err().to_string(), + "expanded body substitution exceeds limit" + ); + continue; + } + let substituted = result.unwrap(); + assert_eq!(substituted.headers()["content-length"], "31"); + assert!(!substituted.headers().contains_key("transfer-encoding")); + assert_eq!( + substituted + .into_body() + .collect() + .await + .unwrap() + .to_bytes() + .as_ref(), + expected.as_bytes() + ); + } + } + + for coding in [None, Some("chunked")] { + let mut req = Request::builder() + .uri("https://example.com/") + .header("content-length", "1"); + if let Some(coding) = coding { + req = req.header("transfer-encoding", coding); + } + assert!( + transform + .apply(req.body(Body::from(MARKER)).unwrap()) + .await + .is_err() + ); + } + + let mut overlaps = config(); + overlaps.replacements[0].placeholder = "0123456789abcdef0".into(); + let transform = compile(overlaps, "replacement"); + assert!( + transform + .apply( + Request::builder() + .uri("https://example.com/") + .body(Body::from("0123456789abcdef0123456789abcdef0")) + .unwrap() + ) + .await + .is_err() + ); +} + +#[test] +fn rejects_duplicate_bindings_recursive_values_and_redacts_secrets() { + let mut duplicate = config(); + duplicate + .replacements + .push(duplicate.replacements[0].clone()); + assert!(duplicate.validate().is_err()); + assert!( + config() + .compile(&|_| Some(MARKER.into()), &|_| panic!("unexpected file")) + .is_err() + ); + let compiled = compile(config(), "do-not-log-this-secret"); + assert!(!format!("{compiled:?}").contains("do-not-log-this-secret")); +} + +#[tokio::test] +async fn rejects_integrity_headers_and_preserves_original_validation() { + let transform = compile(config(), "secret"); + for (name, value) in [ + ( + "content-type", + "Application/Vnd.Amazon.Eventstream; version=1", + ), + ("content-encoding", "gzip"), + ("transfer-encoding", "gzip, chunked"), + ("trailer", "digest"), + ("x-amz-trailer", "x-amz-checksum-sha256"), + ("x-amz-decoded-content-length", "32"), + ("content-md5", "placeholder-digest"), + ("digest", "sha-256=placeholder-digest"), + ("content-digest", "sha-256=:placeholder-digest:"), + ("repr-digest", "sha-256=:placeholder-digest:"), + ("x-amz-content-sha256", "placeholder-digest"), + ("x-amz-checksum-sha256", "placeholder-digest"), + ("x-amz-checksum-crc32", "placeholder-digest"), + ] { + for strip in [false, true] { + let mut request = Request::builder() + .uri("https://example.com/") + .header(name, value) + .body(Body::from(MARKER)) + .unwrap(); + preserve_body_substitution_headers(&mut request); + if strip { + request.headers_mut().remove(name); + preserve_body_substitution_headers(&mut request); + } + assert!( + transform.apply(request).await.is_err(), + "{name}, strip={strip}" + ); + } + } + let mut request = Request::builder() + .uri("https://example.com/") + .body(Body::from(MARKER)) + .unwrap(); + preserve_body_substitution_headers(&mut request); + request + .headers_mut() + .insert("content-encoding", HeaderValue::from_static("gzip")); + assert!(transform.apply(request).await.is_err()); +} + +#[tokio::test] +async fn rejects_excessive_search_work_before_scanning_shared_prefixes() { + let mut bounded = config(); + bounded.max_body_bytes = 1024 * 1024; + bounded.replacements = (0..32) + .map(|index| BodySecretReplacement { + placeholder: format!("{}{:02}", "x".repeat(254), index), + secret: ProxySecretSource::EnvVar("SECRET".into()), + }) + .collect(); + let request = Request::builder() + .uri("https://example.com/") + .body(Body::from(vec![b'x'; bounded.max_body_bytes])) + .unwrap(); + assert_eq!( + compile(bounded, "secret") + .apply(request) + .await + .unwrap_err() + .to_string(), + "body substitution search exceeds work limit" + ); +}