feat: remove OPENAI_API_KEY from env and mlock if set

This commit is contained in:
Michael Bolin
2025-09-25 12:13:50 -07:00
parent 103adcdf2d
commit 4670b0c561
8 changed files with 129 additions and 11 deletions
+1
View File
@@ -605,6 +605,7 @@ dependencies = [
"codex-core",
"codex-linux-sandbox",
"dotenvy",
"libc",
"tempfile",
"tokio",
]
+1
View File
@@ -16,5 +16,6 @@ codex-apply-patch = { workspace = true }
codex-core = { workspace = true }
codex-linux-sandbox = { workspace = true }
dotenvy = { workspace = true }
libc = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }
+31 -5
View File
@@ -7,10 +7,28 @@ use codex_core::CODEX_APPLY_PATCH_ARG1;
use std::os::unix::fs::symlink;
use tempfile::TempDir;
mod openai_api_key_env_var;
use openai_api_key_env_var::extract_locked_openai_api_key;
const LINUX_SANDBOX_ARG0: &str = "codex-linux-sandbox";
const APPLY_PATCH_ARG0: &str = "apply_patch";
const MISSPELLED_APPLY_PATCH_ARG0: &str = "applypatch";
/// Arguments supplied to the async entrypoint invoked by [`arg0_dispatch_or_else`].
///
#[derive(Debug)]
pub struct PreMainArgs {
pub codex_linux_sandbox_exe: Option<PathBuf>,
/// Value of the `OPENAI_API_KEY` environment variable that was set at
/// startup, if any.
///
/// If `Some`, the key has already been removed from the environment and an
/// attempt was made to `mlock(2)` the string's allocation to keep it
/// resident in memory.
pub openai_api_key: Option<String>,
}
/// While we want to deploy the Codex CLI as a single executable for simplicity,
/// we also want to expose some of its functionality as distinct CLIs, so we use
/// the "arg0 trick" to determine which CLI to dispatch. This effectively allows
@@ -26,15 +44,15 @@ const MISSPELLED_APPLY_PATCH_ARG0: &str = "applypatch";
/// 3. Derive the path to the current executable (so children can re-invoke the
/// sandbox) when running on Linux.
/// 4. Execute the provided async `main_fn` inside that runtime, forwarding any
/// error. Note that `main_fn` receives `codex_linux_sandbox_exe:
/// Option<PathBuf>`, as an argument, which is generally needed as part of
/// constructing [`codex_core::config::Config`].
/// error. The closure receives [`PreMainArgs`], which includes the optional
/// path to the linux sandbox helper as well as the OpenAI API key if the
/// caller requested it.
///
/// This function should be used to wrap any `main()` function in binary crates
/// in this workspace that depends on these helper CLIs.
pub fn arg0_dispatch_or_else<F, Fut>(main_fn: F) -> anyhow::Result<()>
where
F: FnOnce(Option<PathBuf>) -> Fut,
F: FnOnce(PreMainArgs) -> Fut,
Fut: Future<Output = anyhow::Result<()>>,
{
// Determine if we were invoked via the special alias.
@@ -76,6 +94,9 @@ where
// before creating any threads/the Tokio runtime.
load_dotenv();
// Perform the OPENAI_API_KEY check after loading the .env file.
let openai_api_key = extract_locked_openai_api_key();
// Retain the TempDir so it exists for the lifetime of the invocation of
// this executable. Admittedly, we could invoke `keep()` on it, but it
// would be nice to avoid leaving temporary directories behind, if possible.
@@ -99,7 +120,12 @@ where
None
};
main_fn(codex_linux_sandbox_exe).await
let args = PreMainArgs {
codex_linux_sandbox_exe,
openai_api_key,
};
main_fn(args).await
})
}
@@ -0,0 +1,69 @@
const OPENAI_API_KEY_ENV_VAR: &str = "OPENAI_API_KEY";
pub(crate) fn extract_locked_openai_api_key() -> Option<String> {
match std::env::var(OPENAI_API_KEY_ENV_VAR) {
Ok(key) => {
if key.is_empty() {
return None;
}
mlock_str(&key);
// Safety: modifying environment variables is only done before new
// threads are spawned.
unsafe {
std::env::remove_var(OPENAI_API_KEY_ENV_VAR);
}
Some(key)
}
Err(std::env::VarError::NotPresent) => None,
Err(std::env::VarError::NotUnicode(_)) => {
// Cannot possibly be a valid API key, so ignore it.
None
}
}
}
#[cfg(unix)]
fn mlock_str(value: &str) {
use libc::_SC_PAGESIZE;
use libc::c_void;
use libc::mlock;
use libc::sysconf;
if value.is_empty() {
return;
}
// Safety: we only read the pointer and length for mlock bookkeeping.
let page_size = unsafe { sysconf(_SC_PAGESIZE) };
if page_size <= 0 {
return;
}
let page_size = page_size as usize;
if page_size == 0 {
return;
}
let addr = value.as_ptr() as usize;
let len = value.len();
let start = addr & !(page_size - 1);
let addr_end = match addr.checked_add(len) {
Some(v) => match v.checked_add(page_size - 1) {
Some(total) => total,
None => return,
},
None => return,
};
let end = addr_end & !(page_size - 1);
let size = end.saturating_sub(start);
if size == 0 {
return;
}
// Best-effort; ignore failures because mlock may require privileges.
let _ = unsafe { mlock(start as *const c_void, size) };
}
#[cfg(not(unix))]
fn mlock_str(_value: &str) {}
+9 -3
View File
@@ -2,6 +2,7 @@ use clap::CommandFactory;
use clap::Parser;
use clap_complete::Shell;
use clap_complete::generate;
use codex_arg0::PreMainArgs;
use codex_arg0::arg0_dispatch_or_else;
use codex_chatgpt::apply_command::ApplyCommand;
use codex_chatgpt::apply_command::run_apply_command;
@@ -224,13 +225,18 @@ fn pre_main_hardening() {
}
fn main() -> anyhow::Result<()> {
arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move {
cli_main(codex_linux_sandbox_exe).await?;
arg0_dispatch_or_else(|pre_main_args| async move {
cli_main(pre_main_args).await?;
Ok(())
})
}
async fn cli_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()> {
async fn cli_main(pre_main_args: PreMainArgs) -> anyhow::Result<()> {
let PreMainArgs {
codex_linux_sandbox_exe,
openai_api_key: _,
} = pre_main_args;
let MultitoolCli {
config_overrides: root_config_overrides,
mut interactive,
+6 -1
View File
@@ -10,6 +10,7 @@
//! This allows us to ship a completely separate set of functionality as part
//! of the `codex-exec` binary.
use clap::Parser;
use codex_arg0::PreMainArgs;
use codex_arg0::arg0_dispatch_or_else;
use codex_common::CliConfigOverrides;
use codex_exec::Cli;
@@ -25,7 +26,11 @@ struct TopCli {
}
fn main() -> anyhow::Result<()> {
arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move {
arg0_dispatch_or_else(|pre_main_args| async move {
let PreMainArgs {
codex_linux_sandbox_exe,
openai_api_key: _,
} = pre_main_args;
let top_cli = TopCli::parse();
// Merge root-level overrides into inner CLI struct so downstream logic remains unchanged.
let mut inner = top_cli.inner;
+6 -1
View File
@@ -1,9 +1,14 @@
use codex_arg0::PreMainArgs;
use codex_arg0::arg0_dispatch_or_else;
use codex_common::CliConfigOverrides;
use codex_mcp_server::run_main;
fn main() -> anyhow::Result<()> {
arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move {
arg0_dispatch_or_else(|pre_main_args| async move {
let PreMainArgs {
codex_linux_sandbox_exe,
openai_api_key: _,
} = pre_main_args;
run_main(codex_linux_sandbox_exe, CliConfigOverrides::default()).await?;
Ok(())
})
+6 -1
View File
@@ -1,4 +1,5 @@
use clap::Parser;
use codex_arg0::PreMainArgs;
use codex_arg0::arg0_dispatch_or_else;
use codex_common::CliConfigOverrides;
use codex_tui::Cli;
@@ -14,7 +15,11 @@ struct TopCli {
}
fn main() -> anyhow::Result<()> {
arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move {
arg0_dispatch_or_else(|pre_main_args| async move {
let PreMainArgs {
codex_linux_sandbox_exe,
openai_api_key: _,
} = pre_main_args;
let top_cli = TopCli::parse();
let mut inner = top_cli.inner;
inner