diff --git a/codex-rs/network-proxy/src/certs.rs b/codex-rs/network-proxy/src/certs.rs index 8497f1f1d2..60502092f5 100644 --- a/codex-rs/network-proxy/src/certs.rs +++ b/codex-rs/network-proxy/src/certs.rs @@ -131,7 +131,7 @@ pub(crate) fn managed_ca_trust_bundle_path(env: &HashMap) -> Res .parent() .ok_or_else(|| anyhow!("managed MITM CA cert path is missing a parent"))? .join(MANAGED_MITM_CA_TRUST_BUNDLE); - let trust_bundle = build_managed_ca_trust_bundle(&cert_path, env)?; + let trust_bundle = build_managed_ca_trust_bundle(&cert_path, &trust_bundle_path, env)?; write_atomic_replace( &trust_bundle_path, trust_bundle.as_bytes(), @@ -148,6 +148,7 @@ pub(crate) fn managed_ca_trust_bundle_path(env: &HashMap) -> Res fn build_managed_ca_trust_bundle( managed_ca_cert_path: &Path, + trust_bundle_path: &Path, env: &HashMap, ) -> Result { let mut trust_bundle = String::new(); @@ -169,13 +170,21 @@ fn build_managed_ca_trust_bundle( continue; }; let path = PathBuf::from(path); - if path == managed_ca_cert_path || custom_ca_paths.contains(&path) { + if path == managed_ca_cert_path + || path == trust_bundle_path + || custom_ca_paths.contains(&path) + { continue; } custom_ca_paths.push(path); } for path in custom_ca_paths { - append_pem_file(&mut trust_bundle, &path)?; + if let Err(err) = append_pem_file(&mut trust_bundle, &path) { + warn!( + path = %path.display(), + "failed to append inherited custom CA bundle; continuing without it: {err}" + ); + } } append_pem_file(&mut trust_bundle, managed_ca_cert_path)?; Ok(trust_bundle) @@ -337,6 +346,12 @@ fn write_atomic_create_new(path: &Path, contents: &[u8], mode: u32) -> Result<() } fn write_atomic_replace(path: &Path, contents: &[u8], mode: u32) -> Result<()> { + if fs::symlink_metadata(path) + .ok() + .is_some_and(|metadata| metadata.file_type().is_symlink()) + { + return Err(anyhow!("refusing to overwrite symlink {}", path.display())); + } if fs::read(path).ok().as_deref() == Some(contents) { return Ok(()); } @@ -345,12 +360,6 @@ fn write_atomic_replace(path: &Path, contents: &[u8], mode: u32) -> Result<()> { .parent() .ok_or_else(|| anyhow!("missing parent directory"))?; fs::create_dir_all(parent).with_context(|| format!("failed to create {}", parent.display()))?; - if fs::symlink_metadata(path) - .ok() - .is_some_and(|metadata| metadata.file_type().is_symlink()) - { - return Err(anyhow!("refusing to overwrite symlink {}", path.display())); - } let nanos = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -441,13 +450,53 @@ fn open_create_new_with_mode(path: &Path, _mode: u32) -> Result { .with_context(|| format!("failed to create {}", path.display())) } -#[cfg(all(test, unix))] +#[cfg(test)] mod tests { use super::*; + #[cfg(unix)] + use pretty_assertions::assert_eq; + #[cfg(unix)] use std::os::unix::fs::PermissionsExt; use tempfile::tempdir; + #[test] + fn build_managed_ca_trust_bundle_skips_missing_inherited_bundle() { + let dir = tempdir().unwrap(); + let managed_ca_cert_path = dir.path().join("ca.pem"); + let trust_bundle_path = dir.path().join("ca-bundle.pem"); + fs::write(&managed_ca_cert_path, "managed ca\n").unwrap(); + let env = HashMap::from([( + "SSL_CERT_FILE".to_string(), + dir.path().join("missing.pem").display().to_string(), + )]); + + let trust_bundle = + build_managed_ca_trust_bundle(&managed_ca_cert_path, &trust_bundle_path, &env).unwrap(); + + assert!(trust_bundle.contains("managed ca")); + } + + #[test] + fn build_managed_ca_trust_bundle_skips_existing_managed_bundle() { + let dir = tempdir().unwrap(); + let managed_ca_cert_path = dir.path().join("ca.pem"); + let trust_bundle_path = dir.path().join("ca-bundle.pem"); + fs::write(&managed_ca_cert_path, "managed ca\n").unwrap(); + fs::write(&trust_bundle_path, "stale managed bundle\n").unwrap(); + let env = HashMap::from([( + "SSL_CERT_FILE".to_string(), + trust_bundle_path.display().to_string(), + )]); + + let trust_bundle = + build_managed_ca_trust_bundle(&managed_ca_cert_path, &trust_bundle_path, &env).unwrap(); + + assert!(trust_bundle.contains("managed ca")); + assert!(!trust_bundle.contains("stale managed bundle")); + } + + #[cfg(unix)] #[test] fn validate_existing_ca_key_file_rejects_group_world_permissions() { let dir = tempdir().unwrap(); @@ -462,6 +511,7 @@ mod tests { ); } + #[cfg(unix)] #[test] fn validate_existing_ca_key_file_rejects_symlink() { use std::os::unix::fs::symlink; @@ -479,6 +529,7 @@ mod tests { ); } + #[cfg(unix)] #[test] fn validate_existing_ca_key_file_allows_private_permissions() { let dir = tempdir().unwrap(); @@ -488,4 +539,23 @@ mod tests { validate_existing_ca_key_file(&key_path).unwrap(); } + + #[cfg(unix)] + #[test] + fn write_atomic_replace_rejects_matching_symlink_target() { + use std::os::unix::fs::symlink; + + let dir = tempdir().unwrap(); + let target = dir.path().join("real-bundle.pem"); + let link = dir.path().join("ca-bundle.pem"); + fs::write(&target, "bundle").unwrap(); + symlink(&target, &link).unwrap(); + + let err = write_atomic_replace(&link, b"bundle", /*mode*/ 0o644).unwrap_err(); + + assert_eq!( + err.to_string(), + format!("refusing to overwrite symlink {}", link.display()) + ); + } } diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index deef932b0b..8a5237afaa 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -378,7 +378,7 @@ const ELECTRON_GET_USE_PROXY_ENV_KEY: &str = "ELECTRON_GET_USE_PROXY"; const NODE_USE_ENV_PROXY_ENV_KEY: &str = "NODE_USE_ENV_PROXY"; #[cfg(any(target_os = "macos", test))] const GIT_SSH_COMMAND_ENV_KEY: &str = "GIT_SSH_COMMAND"; -const BASE_PROXY_ENV_KEYS: [&str; 35] = [ +pub const PROXY_ENV_KEYS: &[&str] = &[ PROXY_ACTIVE_ENV_KEY, ALLOW_LOCAL_BINDING_ENV_KEY, ELECTRON_GET_USE_PROXY_ENV_KEY, @@ -415,7 +415,6 @@ const BASE_PROXY_ENV_KEYS: [&str; 35] = [ "FTP_PROXY", "ftp_proxy", ]; -pub const PROXY_ENV_KEYS: &[&str] = &concat_proxy_env_keys(); #[cfg(target_os = "macos")] pub const PROXY_GIT_SSH_COMMAND_ENV_KEY: &str = GIT_SSH_COMMAND_ENV_KEY; @@ -470,24 +469,6 @@ fn set_env_keys(env: &mut HashMap, keys: &[&str], value: &str) { } } -const fn concat_proxy_env_keys() --> [&'static str; BASE_PROXY_ENV_KEYS.len() + crate::certs::CUSTOM_CA_ENV_KEYS.len()] { - let mut keys = [""; BASE_PROXY_ENV_KEYS.len() + crate::certs::CUSTOM_CA_ENV_KEYS.len()]; - let mut index = 0; - while index < BASE_PROXY_ENV_KEYS.len() { - keys[index] = BASE_PROXY_ENV_KEYS[index]; - index += 1; - } - - let mut custom_ca_index = 0; - while custom_ca_index < crate::certs::CUSTOM_CA_ENV_KEYS.len() { - keys[index + custom_ca_index] = crate::certs::CUSTOM_CA_ENV_KEYS[custom_ca_index]; - custom_ca_index += 1; - } - - keys -} - #[cfg(target_os = "macos")] fn codex_proxy_git_ssh_command(socks_addr: SocketAddr) -> String { format!("{CODEX_PROXY_GIT_SSH_COMMAND_PREFIX}{socks_addr}{CODEX_PROXY_GIT_SSH_COMMAND_SUFFIX}") @@ -1116,6 +1097,13 @@ mod tests { } } + #[test] + fn custom_ca_env_keys_are_not_generic_proxy_env_keys() { + for key in crate::certs::CUSTOM_CA_ENV_KEYS { + assert!(!PROXY_ENV_KEYS.contains(&key)); + } + } + #[test] fn apply_proxy_env_overrides_uses_http_for_all_proxy_without_socks() { let mut env = HashMap::new();