mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Encapsulate Responses API proxy authentication (#55296)
## What changed Introduce a shared `Auth` abstraction that supplies sensitive authorization headers and the client's redirect policy. Preserve the existing static API key allocation and default redirect behavior, and return an empty `502` response if header acquisition fails. Unify the stdin authentication entry point around platform-specific `read_from_stdin` helpers. ## Testing Add a test verifying that both compact and pretty `Debug` output redact API keys. GitOrigin-RevId: 6a924534f00c1b209b24e8b0af117091cfa3ec07
This commit is contained in:
@@ -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<AuthHeader> {
|
||||
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;
|
||||
@@ -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]"));
|
||||
}
|
||||
}
|
||||
@@ -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::<Duration>)
|
||||
.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());
|
||||
|
||||
|
||||
@@ -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<usize> {
|
||||
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<usize> {
|
||||
pub(crate) fn read_from_stdin(buffer: &mut [u8]) -> std::io::Result<usize> {
|
||||
use libc::c_void;
|
||||
use libc::read;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user