diff --git a/codex-rs/responses-api-proxy/src/auth.rs b/codex-rs/responses-api-proxy/src/auth.rs new file mode 100644 index 0000000000..c18ca8b882 --- /dev/null +++ b/codex-rs/responses-api-proxy/src/auth.rs @@ -0,0 +1,49 @@ +//! Supplies sensitive request headers without copying the locked API key. + +use std::fmt; + +use anyhow::Result; +use reqwest::header::HeaderValue; + +pub(crate) enum Auth { + ApiKey(&'static str), +} + +impl fmt::Debug for Auth { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::ApiKey(_) => f.debug_tuple("ApiKey").field(&"[REDACTED]").finish(), + } + } +} + +#[derive(Clone, Debug)] +pub(crate) struct AuthHeader { + pub(crate) value: HeaderValue, +} + +impl Auth { + pub(crate) fn redirect_policy(&self) -> reqwest::redirect::Policy { + match self { + Self::ApiKey(_) => reqwest::redirect::Policy::default(), + } + } + + /// Returns a sensitive authorization header. API keys are validated when read, + /// so this currently always succeeds. The fallible return type allows later + /// credential providers to report acquisition failures. + pub(crate) fn header(&self) -> Result { + match self { + Self::ApiKey(value) => { + // Keep the existing locked, static API key allocation. + let mut header = HeaderValue::from_static(value); + header.set_sensitive(true); + Ok(AuthHeader { value: header }) + } + } + } +} + +#[cfg(test)] +#[path = "auth_tests.rs"] +mod tests; diff --git a/codex-rs/responses-api-proxy/src/auth_tests.rs b/codex-rs/responses-api-proxy/src/auth_tests.rs new file mode 100644 index 0000000000..c55a13ec40 --- /dev/null +++ b/codex-rs/responses-api-proxy/src/auth_tests.rs @@ -0,0 +1,14 @@ +//! Ensures authentication diagnostics do not expose credentials. + +use super::Auth; + +#[test] +fn debug_redacts_api_keys_in_both_formats() { + let auth = Auth::ApiKey("Bearer test-secret-for-proxy-auth"); + + for output in [format!("{auth:?}"), format!("{auth:#?}")] { + assert!(!output.contains("test-secret-for-proxy-auth")); + assert!(output.contains("ApiKey")); + assert!(output.contains("[REDACTED]")); + } +} diff --git a/codex-rs/responses-api-proxy/src/lib.rs b/codex-rs/responses-api-proxy/src/lib.rs index 307fa05bb8..d096d8bf68 100644 --- a/codex-rs/responses-api-proxy/src/lib.rs +++ b/codex-rs/responses-api-proxy/src/lib.rs @@ -28,8 +28,10 @@ use tiny_http::Response; use tiny_http::Server; use tiny_http::StatusCode; +mod auth; mod dump; mod read_api_key; +use auth::Auth; use dump::ExchangeDumper; use read_api_key::read_auth_header_from_stdin; @@ -71,7 +73,7 @@ struct ForwardConfig { /// Entry point for the library main, for parity with other crates. pub fn run_main(args: Args) -> Result<()> { - let auth_header = read_auth_header_from_stdin()?; + let auth = Arc::new(Auth::ApiKey(read_auth_header_from_stdin()?)); let upstream_url = Url::parse(&args.upstream_url).context("parsing --upstream-url")?; let host = match (upstream_url.host_str(), upstream_url.port()) { @@ -101,6 +103,7 @@ pub fn run_main(args: Args) -> Result<()> { .map_err(|err| anyhow!("creating HTTP server: {err}"))?; let client = Arc::new( Client::builder() + .redirect(auth.redirect_policy()) // Disable reqwest's 30s default so long-lived response streams keep flowing. .timeout(None::) .build() @@ -112,6 +115,7 @@ pub fn run_main(args: Args) -> Result<()> { let http_shutdown = args.http_shutdown; for request in server.incoming_requests() { let client = client.clone(); + let auth = auth.clone(); let forward_config = forward_config.clone(); let dump_dir = dump_dir.clone(); std::thread::spawn(move || { @@ -122,7 +126,7 @@ pub fn run_main(args: Args) -> Result<()> { if let Err(e) = forward_request( &client, - auth_header, + &auth, &forward_config, dump_dir.as_deref(), request, @@ -162,7 +166,7 @@ fn write_server_info(path: &Path, port: u16) -> Result<()> { fn forward_request( client: &Client, - auth_header: &'static str, + auth: &Auth, config: &ForwardConfig, dump_dir: Option<&ExchangeDumper>, mut req: Request, @@ -212,11 +216,15 @@ fn forward_request( } } - // As part of our effort to to keep `auth_header` secret, we use a - // combination of `from_static()` and `set_sensitive(true)`. - let mut auth_header_value = HeaderValue::from_static(auth_header); - auth_header_value.set_sensitive(true); - headers.insert(AUTHORIZATION, auth_header_value); + let auth_header = match auth.header() { + Ok(value) => value, + Err(err) => { + // No token, request URL or upstream error body is exposed to the caller. + let _ = req.respond(Response::new_empty(StatusCode(502))); + return Err(err); + } + }; + headers.insert(AUTHORIZATION, auth_header.value); headers.insert(HOST, config.host_header.clone()); diff --git a/codex-rs/responses-api-proxy/src/read_api_key.rs b/codex-rs/responses-api-proxy/src/read_api_key.rs index df5e2106c7..e9e9034c81 100644 --- a/codex-rs/responses-api-proxy/src/read_api_key.rs +++ b/codex-rs/responses-api-proxy/src/read_api_key.rs @@ -12,21 +12,20 @@ const AUTH_HEADER_PREFIX: &[u8] = b"Bearer "; /// value with the auth token used with `Bearer`. The header value is returned /// as a `&'static str` whose bytes are locked in memory to avoid accidental /// exposure. -#[cfg(unix)] pub(crate) fn read_auth_header_from_stdin() -> Result<&'static str> { - read_auth_header_with(read_from_unix_stdin) + read_auth_header_with(read_from_stdin) } #[cfg(windows)] -pub(crate) fn read_auth_header_from_stdin() -> Result<&'static str> { +pub(crate) fn read_from_stdin(buffer: &mut [u8]) -> std::io::Result { use std::io::Read; // Use of `stdio::io::stdin()` has the problem mentioned in the docstring on - // the UNIX version of `read_from_unix_stdin()`, so this should ultimately + // the UNIX version of `read_from_stdin()`, so this should ultimately // be replaced the low-level Windows equivalent. Because we do not have an // equivalent of mlock() on Windows right now, it is not pressing until we // address that issue. - read_auth_header_with(|buffer| std::io::stdin().read(buffer)) + std::io::stdin().read(buffer) } /// We perform a low-level read with `read(2)` because `stdio::io::stdin()` has @@ -38,7 +37,7 @@ pub(crate) fn read_auth_header_from_stdin() -> Result<&'static str> { /// it out, whereas we aim to guarantee there is exactly one copy of the API key /// in memory, protected by mlock(2). #[cfg(unix)] -fn read_from_unix_stdin(buffer: &mut [u8]) -> std::io::Result { +pub(crate) fn read_from_stdin(buffer: &mut [u8]) -> std::io::Result { use libc::c_void; use libc::read;