diff --git a/codex-rs/responses-api-proxy/README.md b/codex-rs/responses-api-proxy/README.md index 4b74fe10..31ac9c45 100644 --- a/codex-rs/responses-api-proxy/README.md +++ b/codex-rs/responses-api-proxy/README.md @@ -4,12 +4,12 @@ A strict HTTP proxy that only forwards `POST` requests to `/v1/responses` to the ## Expected Usage -**IMPORTANT:** `codex-responses-api-proxy` is designed to be run by a privileged user with access to `OPENAI_API_KEY` so that an unprivileged user cannot inspect or tamper with the process. Though if `--http-shutdown` is specified, an unprivileged user _can_ make a `GET` request to `/shutdown` to shutdown the server, as an unprivileged could not send `SIGTERM` to kill the process. +**IMPORTANT:** `codex-responses-api-proxy` is designed to be run by a privileged user with access to `OPENAI_API_KEY` so that an unprivileged user cannot inspect or tamper with the process. Though if `--http-shutdown` is specified, an unprivileged user _can_ make a `GET` request to `/shutdown` to shutdown the server, as an unprivileged user could not send `SIGTERM` to kill the process. A privileged user (i.e., `root` or a user with `sudo`) who has access to `OPENAI_API_KEY` would run the following to start the server, as `codex-responses-api-proxy` reads the auth token from `stdin`: ```shell -printenv OPENAI_API_KEY | codex-responses-api-proxy --http-shutdown --server-info /tmp/server-info.json +printenv OPENAI_API_KEY | env -u OPENAI_API_KEY codex-responses-api-proxy --http-shutdown --server-info /tmp/server-info.json ``` A non-privileged user would then run Codex as follows, specifying the `model_provider` dynamically: @@ -35,7 +35,7 @@ curl --fail --silent --show-error "${PROXY_BASE_URL}/shutdown" - Listens on the provided port or an ephemeral port if `--port` is not specified. - Accepts exactly `POST /v1/responses` (no query string). The request body is forwarded to `https://api.openai.com/v1/responses` with `Authorization: Bearer ` set. All original request headers (except any incoming `Authorization`) are forwarded upstream. For other requests, it responds with `403`. - Optionally writes a single-line JSON file with server info, currently `{ "port": }`. -- Optional `--http-shutdown` enables `GET /shutdown` to terminate the process with exit code 0. This allows one user (e.g., `root`) to start the proxy and another unprivileged user on the host to shut it down. +- Optional `--http-shutdown` enables `GET /shutdown` to terminate the process with exit code `0`. This allows one user (e.g., `root`) to start the proxy and another unprivileged user on the host to shut it down. ## CLI @@ -44,7 +44,7 @@ codex-responses-api-proxy [--port ] [--server-info ] [--http-shutdow ``` - `--port `: Port to bind on `127.0.0.1`. If omitted, an ephemeral port is chosen. -- `--server-info `: If set, the proxy writes a single line of JSON with `{ "port": }` once listening. +- `--server-info `: If set, the proxy writes a single line of JSON with `{ "port": , "pid": }` once listening. - `--http-shutdown`: If set, enables `GET /shutdown` to exit the process with code `0`. ## Notes 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 1dda92f3..f3950b54 100644 --- a/codex-rs/responses-api-proxy/src/read_api_key.rs +++ b/codex-rs/responses-api-proxy/src/read_api_key.rs @@ -1,7 +1,6 @@ use anyhow::Context; use anyhow::Result; use anyhow::anyhow; -use std::io::Read; use zeroize::Zeroize; /// Use a generous buffer size to avoid truncation and to allow for longer API @@ -13,13 +12,66 @@ 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) +} + +#[cfg(windows)] +pub(crate) fn read_auth_header_from_stdin() -> Result<&'static str> { + 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 + // 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)) } -fn read_auth_header_with(read_fn: F) -> Result<&'static str> +/// We perform a low-level read with `read(2)` because `stdio::io::stdin()` has +/// an internal BufReader: +/// +/// https://github.com/rust-lang/rust/blob/bcbbdcb8522fd3cb4a8dde62313b251ab107694d/library/std/src/io/stdio.rs#L250-L252 +/// +/// that can end up retaining a copy of stdin data in memory with no way to zero +/// 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 { + use libc::c_void; + use libc::read; + + // Perform a single read(2) call into the provided buffer slice. + // Looping and newline/EOF handling are managed by the caller. + loop { + let result = unsafe { + read( + libc::STDIN_FILENO, + buffer.as_mut_ptr().cast::(), + buffer.len(), + ) + }; + + if result == 0 { + return Ok(0); + } + + if result < 0 { + let err = std::io::Error::last_os_error(); + if err.kind() == std::io::ErrorKind::Interrupted { + continue; + } + return Err(err); + } + + return Ok(result as usize); + } +} + +fn read_auth_header_with(mut read_fn: F) -> Result<&'static str> where - F: FnOnce(&mut [u8]) -> std::io::Result, + F: FnMut(&mut [u8]) -> std::io::Result, { // TAKE CARE WHEN MODIFYING THIS CODE!!! // @@ -31,19 +83,50 @@ where let mut buf = [0u8; BUFFER_SIZE]; buf[..AUTH_HEADER_PREFIX.len()].copy_from_slice(AUTH_HEADER_PREFIX); - let read = read_fn(&mut buf[AUTH_HEADER_PREFIX.len()..]).inspect_err(|_err| { - buf.zeroize(); - })?; + let prefix_len = AUTH_HEADER_PREFIX.len(); + let capacity = buf.len() - prefix_len; + let mut total_read = 0usize; // number of bytes read into the token region + let mut saw_newline = false; + let mut saw_eof = false; - if read == buf.len() - AUTH_HEADER_PREFIX.len() { + while total_read < capacity { + let slice = &mut buf[prefix_len + total_read..]; + let read = match read_fn(slice) { + Ok(n) => n, + Err(err) => { + buf.zeroize(); + return Err(err.into()); + } + }; + + if read == 0 { + saw_eof = true; + break; + } + + // Search only the newly written region for a newline. + let newly_written = &slice[..read]; + if let Some(pos) = newly_written.iter().position(|&b| b == b'\n') { + total_read += pos + 1; // include the newline for trimming below + saw_newline = true; + break; + } + + total_read += read; + + // Continue loop; if buffer fills without newline/EOF we'll error below. + } + + // If buffer filled and we did not see newline or EOF, error out. + if total_read == capacity && !saw_newline && !saw_eof { buf.zeroize(); return Err(anyhow!( "OPENAI_API_KEY is too large to fit in the 512-byte buffer" )); } - let mut total = AUTH_HEADER_PREFIX.len() + read; - while total > AUTH_HEADER_PREFIX.len() && (buf[total - 1] == b'\n' || buf[total - 1] == b'\r') { + let mut total = prefix_len + total_read; + while total > prefix_len && (buf[total - 1] == b'\n' || buf[total - 1] == b'\r') { total -= 1; } @@ -138,13 +221,19 @@ fn validate_auth_header_bytes(key_bytes: &[u8]) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use std::collections::VecDeque; use std::io; #[test] fn reads_key_with_no_newlines() { + let mut sent = false; let result = read_auth_header_with(|buf| { + if sent { + return Ok(0); + } let data = b"sk-abc123"; buf[..data.len()].copy_from_slice(data); + sent = true; Ok(data.len()) }) .unwrap(); @@ -152,11 +241,32 @@ mod tests { assert_eq!(result, "Bearer sk-abc123"); } + #[test] + fn reads_key_with_short_reads() { + let mut chunks: VecDeque<&[u8]> = + VecDeque::from(vec![b"sk-".as_ref(), b"abc".as_ref(), b"123\n".as_ref()]); + let result = read_auth_header_with(|buf| match chunks.pop_front() { + Some(chunk) if !chunk.is_empty() => { + buf[..chunk.len()].copy_from_slice(chunk); + Ok(chunk.len()) + } + _ => Ok(0), + }) + .unwrap(); + + assert_eq!(result, "Bearer sk-abc123"); + } + #[test] fn reads_key_and_trims_newlines() { + let mut sent = false; let result = read_auth_header_with(|buf| { + if sent { + return Ok(0); + } let data = b"sk-abc123\r\n"; buf[..data.len()].copy_from_slice(data); + sent = true; Ok(data.len()) }) .unwrap(); @@ -194,9 +304,14 @@ mod tests { #[test] fn errors_on_invalid_utf8() { + let mut sent = false; let err = read_auth_header_with(|buf| { + if sent { + return Ok(0); + } let data = b"sk-abc\xff"; buf[..data.len()].copy_from_slice(data); + sent = true; Ok(data.len()) }) .unwrap_err(); @@ -209,9 +324,14 @@ mod tests { #[test] fn errors_on_invalid_characters() { + let mut sent = false; let err = read_auth_header_with(|buf| { + if sent { + return Ok(0); + } let data = b"sk-abc!23"; buf[..data.len()].copy_from_slice(data); + sent = true; Ok(data.len()) }) .unwrap_err();