mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
[codex] close workload identity review gaps [ci changed_files]
This commit is contained in:
Generated
+1
@@ -3246,6 +3246,7 @@ dependencies = [
|
||||
"codex-keyring-store",
|
||||
"codex-model-provider-info",
|
||||
"codex-otel",
|
||||
"codex-process-hardening",
|
||||
"codex-protocol",
|
||||
"codex-secrets",
|
||||
"codex-terminal-detection",
|
||||
|
||||
@@ -780,12 +780,11 @@ impl AccountRequestProcessor {
|
||||
requires_openai_auth: Some(false),
|
||||
}
|
||||
} else {
|
||||
let auth = if do_refresh {
|
||||
Ok(self.auth_manager.auth_cached())
|
||||
} else {
|
||||
self.auth_manager.auth().await
|
||||
}
|
||||
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?;
|
||||
let auth = self
|
||||
.auth_manager
|
||||
.auth()
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?;
|
||||
match auth {
|
||||
Some(auth) => {
|
||||
let permanent_refresh_failure =
|
||||
|
||||
@@ -2,6 +2,7 @@ use chrono::DateTime;
|
||||
use chrono::Local;
|
||||
use chrono::Utc;
|
||||
use reqwest::header::HeaderMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core::config::Config;
|
||||
use codex_login::AuthManager;
|
||||
@@ -41,19 +42,13 @@ pub fn normalize_base_url(input: &str) -> String {
|
||||
base_url
|
||||
}
|
||||
|
||||
pub async fn load_auth_manager(chatgpt_base_url: Option<String>) -> Option<AuthManager> {
|
||||
pub async fn load_auth_manager(chatgpt_base_url: Option<String>) -> Option<Arc<AuthManager>> {
|
||||
// TODO: pass in cli overrides once cloud tasks properly support them.
|
||||
let config = Config::load_with_cli_overrides(Vec::new()).await.ok()?;
|
||||
Some(
|
||||
AuthManager::new(
|
||||
config.codex_home.to_path_buf(),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
config.cli_auth_credentials_store_mode,
|
||||
chatgpt_base_url.or(Some(config.chatgpt_base_url.clone())),
|
||||
config.auth_keyring_backend_kind(),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
let mut config = Config::load_with_cli_overrides(Vec::new()).await.ok()?;
|
||||
if let Some(chatgpt_base_url) = chatgpt_base_url {
|
||||
config.chatgpt_base_url = chatgpt_base_url;
|
||||
}
|
||||
Some(AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await)
|
||||
}
|
||||
|
||||
/// Build headers for ChatGPT-backed requests: `User-Agent`, optional `Authorization`,
|
||||
|
||||
@@ -30,6 +30,7 @@ codex-config = { workspace = true }
|
||||
codex-keyring-store = { workspace = true }
|
||||
codex-model-provider-info = { workspace = true }
|
||||
codex-otel = { workspace = true }
|
||||
codex-process-hardening = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-secrets = { workspace = true }
|
||||
codex-terminal-detection = { workspace = true }
|
||||
|
||||
@@ -643,27 +643,54 @@ async fn external_chatgpt_auth_resolves_and_refreshes_without_writing_auth_file(
|
||||
)
|
||||
.await;
|
||||
manager.set_external_auth(external.clone());
|
||||
let mut auth_changes = manager.auth_change_receiver();
|
||||
|
||||
assert_eq!(manager.auth_mode(), Some(AuthMode::Chatgpt));
|
||||
assert_eq!(
|
||||
manager.get_api_auth_mode(),
|
||||
Some(AuthMode::ChatgptAuthTokens)
|
||||
);
|
||||
assert_eq!(manager.auth_mode(), None);
|
||||
assert_eq!(manager.get_api_auth_mode(), None);
|
||||
let auth = manager
|
||||
.auth()
|
||||
.await
|
||||
.expect("external auth resolution should succeed")
|
||||
.expect("external auth should resolve");
|
||||
assert!(
|
||||
auth_changes
|
||||
.has_changed()
|
||||
.expect("auth watch should be open")
|
||||
);
|
||||
drop(auth_changes.borrow_and_update());
|
||||
assert!(manager.is_external_chatgpt_auth_active());
|
||||
assert_eq!(manager.auth_mode(), Some(AuthMode::Chatgpt));
|
||||
assert_eq!(
|
||||
manager.get_api_auth_mode(),
|
||||
Some(AuthMode::ChatgptAuthTokens)
|
||||
);
|
||||
assert_eq!(auth.get_token().unwrap(), token);
|
||||
assert_eq!(auth.get_account_id().as_deref(), Some(WORKSPACE_ID_ALLOWED));
|
||||
assert!(!get_auth_file(codex_home.path()).exists());
|
||||
|
||||
let repeated = manager
|
||||
.auth()
|
||||
.await
|
||||
.expect("repeated external auth resolution should succeed")
|
||||
.expect("repeated external auth should resolve");
|
||||
assert_eq!(repeated.get_token().unwrap(), token);
|
||||
assert!(
|
||||
!auth_changes
|
||||
.has_changed()
|
||||
.expect("auth watch should be open"),
|
||||
"unchanged external tokens must not publish an auth change"
|
||||
);
|
||||
|
||||
manager
|
||||
.refresh_token()
|
||||
.await
|
||||
.expect("external auth refresh");
|
||||
assert_eq!(external.refresh_count.load(Ordering::Relaxed), 1);
|
||||
assert!(
|
||||
auth_changes
|
||||
.has_changed()
|
||||
.expect("auth watch should be open")
|
||||
);
|
||||
assert_eq!(
|
||||
manager.auth_cached().unwrap().get_token().unwrap(),
|
||||
refreshed_token
|
||||
@@ -731,6 +758,11 @@ async fn auth_result_preserves_external_auth_resolution_errors() {
|
||||
.await;
|
||||
manager.set_external_auth(Arc::new(FailingExternalChatgptAuth));
|
||||
|
||||
assert_eq!(manager.auth_mode(), Some(AuthMode::Chatgpt));
|
||||
assert_eq!(
|
||||
manager.get_api_auth_mode(),
|
||||
Some(ApiAuthMode::ChatgptAuthTokens)
|
||||
);
|
||||
let error = manager
|
||||
.auth_result()
|
||||
.await
|
||||
|
||||
@@ -1787,7 +1787,15 @@ impl AuthManager {
|
||||
(ApiAuthMode::ApiKey, ApiAuthMode::ApiKey) => a.api_key() == b.api_key(),
|
||||
(ApiAuthMode::Chatgpt, ApiAuthMode::Chatgpt)
|
||||
| (ApiAuthMode::ChatgptAuthTokens, ApiAuthMode::ChatgptAuthTokens) => {
|
||||
a.get_current_auth_json() == b.get_current_auth_json()
|
||||
let mut a = a.get_current_auth_json();
|
||||
let mut b = b.get_current_auth_json();
|
||||
if let Some(a) = a.as_mut() {
|
||||
a.last_refresh = None;
|
||||
}
|
||||
if let Some(b) = b.as_mut() {
|
||||
b.last_refresh = None;
|
||||
}
|
||||
a == b
|
||||
}
|
||||
(ApiAuthMode::AgentIdentity, ApiAuthMode::AgentIdentity) => match (a, b) {
|
||||
(CodexAuth::AgentIdentity(a), CodexAuth::AgentIdentity(b)) => {
|
||||
@@ -1973,9 +1981,18 @@ impl AuthManager {
|
||||
}
|
||||
|
||||
fn external_auth_mode(&self) -> Option<AuthMode> {
|
||||
self.external_auth()
|
||||
.as_ref()
|
||||
.map(|external_auth| external_auth.auth_mode())
|
||||
let external_auth = self.external_auth()?;
|
||||
let mode = external_auth.auth_mode();
|
||||
if mode == AuthMode::Chatgpt
|
||||
&& !external_auth.requires_successful_resolution()
|
||||
&& !self
|
||||
.auth_cached()
|
||||
.as_ref()
|
||||
.is_some_and(CodexAuth::is_external_chatgpt_tokens)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(mode)
|
||||
}
|
||||
|
||||
fn has_external_api_key_auth(&self) -> bool {
|
||||
|
||||
@@ -8,14 +8,24 @@ use super::ExternalAuthTokens;
|
||||
|
||||
pub(super) struct WorkloadIdentityExternalAuth {
|
||||
client: ConfiguredWorkloadIdentityClient,
|
||||
process_isolation_error: Option<String>,
|
||||
}
|
||||
|
||||
impl WorkloadIdentityExternalAuth {
|
||||
pub(super) fn new(client: ConfiguredWorkloadIdentityClient) -> Self {
|
||||
Self { client }
|
||||
let process_isolation_error = codex_process_hardening::disable_process_inspection()
|
||||
.err()
|
||||
.map(|error| format!("workload identity process isolation failed: {error}"));
|
||||
Self {
|
||||
client,
|
||||
process_isolation_error,
|
||||
}
|
||||
}
|
||||
|
||||
async fn tokens(&self, force_refresh: bool) -> std::io::Result<ExternalAuthTokens> {
|
||||
if let Some(error) = self.process_isolation_error.as_ref() {
|
||||
return Err(std::io::Error::other(error.clone()));
|
||||
}
|
||||
let token = if force_refresh {
|
||||
self.client.refresh().await
|
||||
} else {
|
||||
|
||||
@@ -3,6 +3,18 @@ use std::ffi::OsString;
|
||||
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))]
|
||||
use std::sync::OnceLock;
|
||||
|
||||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))]
|
||||
#[derive(Clone, Copy)]
|
||||
enum ProcessInspectionHardening {
|
||||
Applied,
|
||||
Failed(Option<i32>),
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))]
|
||||
static PROCESS_INSPECTION_HARDENING: OnceLock<ProcessInspectionHardening> = OnceLock::new();
|
||||
|
||||
/// This is designed to be called pre-main() (using `#[ctor::ctor]`) to perform
|
||||
/// various process hardening steps, such as
|
||||
@@ -43,12 +55,8 @@ const SET_RLIMIT_CORE_FAILED_EXIT_CODE: i32 = 7;
|
||||
#[cfg(any(target_os = "linux", target_os = "android"))]
|
||||
pub(crate) fn pre_main_hardening_linux() {
|
||||
// Disable ptrace attach / mark process non-dumpable.
|
||||
let ret_code = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) };
|
||||
if ret_code != 0 {
|
||||
eprintln!(
|
||||
"ERROR: prctl(PR_SET_DUMPABLE, 0) failed: {}",
|
||||
std::io::Error::last_os_error()
|
||||
);
|
||||
if let Err(error) = disable_process_inspection() {
|
||||
eprintln!("ERROR: prctl(PR_SET_DUMPABLE, 0) failed: {error}");
|
||||
std::process::exit(PRCTL_FAILED_EXIT_CODE);
|
||||
}
|
||||
|
||||
@@ -63,12 +71,7 @@ pub(crate) fn pre_main_hardening_linux() {
|
||||
/// Mark the current Linux process non-dumpable so same-user processes cannot attach with ptrace.
|
||||
#[cfg(target_os = "linux")]
|
||||
pub fn disable_process_dumping() -> std::io::Result<()> {
|
||||
let ret_code = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) };
|
||||
if ret_code == 0 {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(std::io::Error::last_os_error())
|
||||
}
|
||||
disable_process_inspection()
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "freebsd", target_os = "openbsd"))]
|
||||
@@ -82,12 +85,8 @@ pub(crate) fn pre_main_hardening_bsd() {
|
||||
#[cfg(target_os = "macos")]
|
||||
pub(crate) fn pre_main_hardening_macos() {
|
||||
// Prevent debuggers from attaching to this process.
|
||||
let ret_code = unsafe { libc::ptrace(libc::PT_DENY_ATTACH, 0, std::ptr::null_mut(), 0) };
|
||||
if ret_code == -1 {
|
||||
eprintln!(
|
||||
"ERROR: ptrace(PT_DENY_ATTACH) failed: {}",
|
||||
std::io::Error::last_os_error()
|
||||
);
|
||||
if let Err(error) = disable_process_inspection() {
|
||||
eprintln!("ERROR: ptrace(PT_DENY_ATTACH) failed: {error}");
|
||||
std::process::exit(PTRACE_DENY_ATTACH_FAILED_EXIT_CODE);
|
||||
}
|
||||
|
||||
@@ -121,6 +120,55 @@ pub(crate) fn pre_main_hardening_windows() {
|
||||
// TODO(mbolin): Perform the appropriate configuration for Windows.
|
||||
}
|
||||
|
||||
/// Prevent same-user child processes from inspecting this process's environment or memory.
|
||||
///
|
||||
/// This is idempotent because auth managers can be rebuilt as configuration changes. Callers that
|
||||
/// hold in-memory credentials should fail closed when the current platform cannot provide this
|
||||
/// boundary.
|
||||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))]
|
||||
pub fn disable_process_inspection() -> std::io::Result<()> {
|
||||
match *PROCESS_INSPECTION_HARDENING.get_or_init(apply_process_inspection_hardening) {
|
||||
ProcessInspectionHardening::Applied => Ok(()),
|
||||
ProcessInspectionHardening::Failed(Some(error)) => {
|
||||
Err(std::io::Error::from_raw_os_error(error))
|
||||
}
|
||||
ProcessInspectionHardening::Failed(None) => Err(std::io::Error::other(
|
||||
"failed to disable process inspection",
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "macos")))]
|
||||
pub fn disable_process_inspection() -> std::io::Result<()> {
|
||||
Err(std::io::Error::new(
|
||||
std::io::ErrorKind::Unsupported,
|
||||
"process inspection hardening is unsupported on this platform",
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))]
|
||||
fn apply_process_inspection_hardening() -> ProcessInspectionHardening {
|
||||
#[cfg(any(target_os = "linux", target_os = "android"))]
|
||||
{
|
||||
let result = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) };
|
||||
if result == 0 {
|
||||
ProcessInspectionHardening::Applied
|
||||
} else {
|
||||
ProcessInspectionHardening::Failed(std::io::Error::last_os_error().raw_os_error())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
{
|
||||
let result = unsafe { libc::ptrace(libc::PT_DENY_ATTACH, 0, std::ptr::null_mut(), 0) };
|
||||
if result == 0 {
|
||||
ProcessInspectionHardening::Applied
|
||||
} else {
|
||||
ProcessInspectionHardening::Failed(std::io::Error::last_os_error().raw_os_error())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn remove_env_vars_with_prefix(prefix: &[u8]) {
|
||||
for key in env_keys_with_prefix(std::env::vars_os(), prefix) {
|
||||
@@ -190,4 +238,13 @@ mod tests {
|
||||
assert_eq!(keys.len(), 1);
|
||||
assert_eq!(keys[0].as_os_str(), ld_test_var);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn disable_process_inspection_marks_process_non_dumpable() {
|
||||
disable_process_inspection().expect("process inspection hardening should succeed");
|
||||
|
||||
let dumpable = unsafe { libc::prctl(libc::PR_GET_DUMPABLE) };
|
||||
assert_eq!(dumpable, 0);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user