mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
address review issues
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -2334,6 +2334,7 @@ dependencies = [
|
||||
"rustls-pki-types",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"sha2 0.10.9",
|
||||
"tempfile",
|
||||
"thiserror 2.0.18",
|
||||
"tokio",
|
||||
|
||||
@@ -20,6 +20,7 @@ use codex_config::CloudRequirementsLoadErrorCode;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_config::types::NetworkConfigToml;
|
||||
use codex_core::util::backoff;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
@@ -734,13 +735,47 @@ pub async fn cloud_requirements_loader_for_storage(
|
||||
credentials_store_mode: AuthCredentialsStoreMode,
|
||||
chatgpt_base_url: String,
|
||||
) -> CloudRequirementsLoader {
|
||||
let auth_manager = AuthManager::shared(
|
||||
codex_home.clone(),
|
||||
cloud_requirements_loader_for_storage_with_network_config(
|
||||
codex_home,
|
||||
enable_codex_api_key_env,
|
||||
credentials_store_mode,
|
||||
Some(chatgpt_base_url.clone()),
|
||||
chatgpt_base_url,
|
||||
/*network_config*/ None,
|
||||
)
|
||||
.await;
|
||||
.await
|
||||
}
|
||||
|
||||
/// Loads cloud requirements while allowing resolved startup network config to reach auth recovery.
|
||||
pub async fn cloud_requirements_loader_for_storage_with_network_config(
|
||||
codex_home: PathBuf,
|
||||
enable_codex_api_key_env: bool,
|
||||
credentials_store_mode: AuthCredentialsStoreMode,
|
||||
chatgpt_base_url: String,
|
||||
network_config: Option<&NetworkConfigToml>,
|
||||
) -> CloudRequirementsLoader {
|
||||
let outbound_proxy_config =
|
||||
codex_login::windows_system_proxy_config_from_network_config(network_config);
|
||||
let auth_manager = if let Some(outbound_proxy_config) = outbound_proxy_config {
|
||||
// Keep the existing startup auth path unless Windows system proxy discovery is selected.
|
||||
Arc::new(
|
||||
AuthManager::new_with_proxy_config(
|
||||
codex_home.clone(),
|
||||
enable_codex_api_key_env,
|
||||
credentials_store_mode,
|
||||
Some(chatgpt_base_url.clone()),
|
||||
Some(outbound_proxy_config),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
} else {
|
||||
AuthManager::shared(
|
||||
codex_home.clone(),
|
||||
enable_codex_api_key_env,
|
||||
credentials_store_mode,
|
||||
Some(chatgpt_base_url.clone()),
|
||||
)
|
||||
.await
|
||||
};
|
||||
cloud_requirements_loader(auth_manager, chatgpt_base_url, codex_home)
|
||||
}
|
||||
|
||||
|
||||
@@ -44,15 +44,28 @@ pub fn normalize_base_url(input: &str) -> String {
|
||||
pub async fn load_auth_manager(chatgpt_base_url: Option<String>) -> Option<AuthManager> {
|
||||
// TODO: pass in cli overrides once cloud tasks properly support them.
|
||||
let config = Config::load_with_cli_overrides(Vec::new()).await.ok()?;
|
||||
Some(
|
||||
let outbound_proxy_config =
|
||||
codex_login::windows_system_proxy_config_from_network_config(config.network.as_ref());
|
||||
let chatgpt_base_url = chatgpt_base_url.or(Some(config.chatgpt_base_url));
|
||||
Some(if let Some(outbound_proxy_config) = outbound_proxy_config {
|
||||
// Keep the existing cloud auth path unless Windows system proxy discovery is selected.
|
||||
AuthManager::new_with_proxy_config(
|
||||
config.codex_home.to_path_buf(),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
config.cli_auth_credentials_store_mode,
|
||||
chatgpt_base_url,
|
||||
Some(outbound_proxy_config),
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
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)),
|
||||
chatgpt_base_url,
|
||||
)
|
||||
.await,
|
||||
)
|
||||
.await
|
||||
})
|
||||
}
|
||||
|
||||
/// Build headers for ChatGPT-backed requests: `User-Agent`, optional `Authorization`,
|
||||
|
||||
@@ -26,6 +26,7 @@ codex-utils-rustls-provider = { workspace = true }
|
||||
zstd = { workspace = true }
|
||||
|
||||
[target.'cfg(target_os = "windows")'.dependencies]
|
||||
sha2 = { workspace = true }
|
||||
windows-sys = { version = "0.52", features = [
|
||||
"Win32_Foundation",
|
||||
"Win32_Networking_WinHttp",
|
||||
|
||||
@@ -51,7 +51,7 @@ use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnStartedNotification;
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_cloud_config::cloud_requirements_loader_for_storage;
|
||||
use codex_cloud_config::cloud_requirements_loader_for_storage_with_network_config;
|
||||
use codex_config::ConfigLoadError;
|
||||
use codex_config::ConfigLoadOptions;
|
||||
use codex_config::LoaderOverrides;
|
||||
@@ -364,11 +364,12 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
.clone()
|
||||
.unwrap_or_else(|| "https://chatgpt.com/backend-api/".to_string());
|
||||
// TODO(gt): Make cloud requirements failures blocking once we can fail-closed.
|
||||
let cloud_requirements = cloud_requirements_loader_for_storage(
|
||||
let cloud_requirements = cloud_requirements_loader_for_storage_with_network_config(
|
||||
codex_home.to_path_buf(),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
config_toml.cli_auth_credentials_store.unwrap_or_default(),
|
||||
chatgpt_base_url,
|
||||
config_toml.network.as_ref(),
|
||||
)
|
||||
.await;
|
||||
let run_cli_overrides = cli_kv_overrides.clone();
|
||||
|
||||
@@ -50,4 +50,5 @@ pub use auth::read_openai_api_key_from_env;
|
||||
pub use auth::save_auth;
|
||||
pub use auth_env_telemetry::AuthEnvTelemetry;
|
||||
pub use auth_env_telemetry::collect_auth_env_telemetry;
|
||||
pub use outbound_proxy::windows_system_proxy_config_from_network_config;
|
||||
pub use token_data::TokenData;
|
||||
|
||||
@@ -21,6 +21,22 @@ pub(crate) fn outbound_proxy_config_from_network_config(
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the auth proxy config for the explicit Windows system-proxy startup path.
|
||||
pub fn windows_system_proxy_config_from_network_config(
|
||||
network: Option<&NetworkConfigToml>,
|
||||
) -> Option<OutboundProxyConfig> {
|
||||
let network = network?;
|
||||
if !system_proxy_mode_enabled()
|
||||
|| network.proxy_mode != Some(NetworkProxyMode::System)
|
||||
|| network.proxy_url.is_some()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
// Legacy startup auth builders only need the explicit Windows system-proxy selection here.
|
||||
Some(outbound_proxy_config_from_network_config(network))
|
||||
}
|
||||
|
||||
const fn system_proxy_mode_enabled() -> bool {
|
||||
cfg!(target_os = "windows")
|
||||
}
|
||||
@@ -55,4 +71,36 @@ mod tests {
|
||||
};
|
||||
assert_eq!(config.mode, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_system_proxy_config_is_scoped_to_windows_pac_path() {
|
||||
let network = NetworkConfigToml {
|
||||
proxy_mode: Some(NetworkProxyMode::System),
|
||||
proxy_url: None,
|
||||
};
|
||||
|
||||
let config = windows_system_proxy_config_from_network_config(Some(&network));
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(
|
||||
config,
|
||||
Some(outbound_proxy_config_from_network_config(&network))
|
||||
);
|
||||
} else {
|
||||
assert_eq!(config, None);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn startup_system_proxy_config_skips_concrete_proxy_override() {
|
||||
let network = NetworkConfigToml {
|
||||
proxy_mode: Some(NetworkProxyMode::System),
|
||||
proxy_url: Some("http://proxy.example.test:8080".to_string()),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
windows_system_proxy_config_from_network_config(Some(&network)),
|
||||
None
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,7 +38,7 @@ use codex_app_server_protocol::ThreadListCwdFilter;
|
||||
use codex_app_server_protocol::ThreadListParams;
|
||||
use codex_app_server_protocol::ThreadSortKey as AppServerThreadSortKey;
|
||||
use codex_app_server_protocol::ThreadSourceKind;
|
||||
use codex_cloud_config::cloud_requirements_loader_for_storage;
|
||||
use codex_cloud_config::cloud_requirements_loader_for_storage_with_network_config;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::ConfigLoadError;
|
||||
use codex_config::LoaderOverrides;
|
||||
@@ -1011,11 +1011,12 @@ pub async fn run_main(
|
||||
.chatgpt_base_url
|
||||
.clone()
|
||||
.unwrap_or_else(|| "https://chatgpt.com/backend-api/".to_string());
|
||||
let cloud_requirements = cloud_requirements_loader_for_storage(
|
||||
let cloud_requirements = cloud_requirements_loader_for_storage_with_network_config(
|
||||
codex_home.to_path_buf(),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
config_toml.cli_auth_credentials_store.unwrap_or_default(),
|
||||
chatgpt_base_url,
|
||||
config_toml.network.as_ref(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1455,11 +1456,12 @@ async fn run_ratatui_app(
|
||||
// rebuild config. This avoids missing newly available cloud requirements due to login
|
||||
// status detection edge cases.
|
||||
if show_login_screen && !uses_remote_workspace {
|
||||
cloud_requirements = cloud_requirements_loader_for_storage(
|
||||
cloud_requirements = cloud_requirements_loader_for_storage_with_network_config(
|
||||
initial_config.codex_home.to_path_buf(),
|
||||
/*enable_codex_api_key_env*/ false,
|
||||
initial_config.cli_auth_credentials_store_mode,
|
||||
initial_config.chatgpt_base_url.clone(),
|
||||
initial_config.network.as_ref(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user