From 9ea975a2dc88d039512313da3e332013e8bd911e Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 28 Jul 2026 17:56:56 +0000 Subject: [PATCH] Route MCP OAuth through configured HTTP clients (#35806) ## What changed - Pass resolved, route-aware HTTP clients through MCP OAuth discovery and login so CLI commands, plugin installation, and skill dependency setup honor configured proxies and execution environments. - Apply per-plugin MCP server configuration and requirements before starting OAuth during installation, and skip disabled servers or servers assigned to unowned environments. - Preserve configured MCP server policies when merging remotely installed plugin metadata. ## Testing - Cover proxy-routed OAuth for `codex mcp add`, `codex mcp login`, plugin installation, and skill MCP dependencies. - Cover plugin-install OAuth filtering for disabled servers, plugin requirements, and unowned environments. GitOrigin-RevId: f84c88820e24a627faa78d6bed1b371682ecdc2f --- codex-rs/app-server/src/request_processors.rs | 1 - .../src/request_processors/plugins.rs | 58 ++- .../tests/suite/v2/plugin_install.rs | 340 +++++++++++++++++- codex-rs/cli/src/mcp_cmd.rs | 50 ++- codex-rs/cli/tests/mcp_add_remove.rs | 107 ++++++ codex-rs/cli/tests/mcp_list.rs | 1 + codex-rs/core-plugins/src/loader.rs | 55 ++- codex-rs/core-plugins/src/manager_tests.rs | 70 ++++ codex-rs/core/src/mcp_skill_dependencies.rs | 27 +- .../suite/mcp_startup_refresh_http_proxy.rs | 187 ++++++++++ .../rmcp-client/src/perform_oauth_login.rs | 96 ++++- 11 files changed, 947 insertions(+), 45 deletions(-) diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 13fbf124c5..62ec16992e 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -363,7 +363,6 @@ use codex_core_plugins::PluginReadRequest; use codex_core_plugins::PluginUninstallError as CorePluginUninstallError; use codex_core_plugins::PluginsManager; use codex_core_plugins::loader::load_plugin_apps; -use codex_core_plugins::loader::load_plugin_mcp_servers; use codex_core_plugins::manifest::PluginManifestInterface; use codex_core_plugins::marketplace::MarketplaceError; use codex_core_plugins::marketplace::MarketplacePluginSource; diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 0285e16667..a77c64d339 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -11,6 +11,7 @@ use codex_config::types::McpServerConfig; use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core_plugins::PluginListBackgroundTaskOptions; use codex_core_plugins::is_openai_curated_marketplace_name; +use codex_core_plugins::loader::load_configured_plugin_mcp_servers; use codex_core_plugins::remote::REMOTE_CREATED_BY_ME_MARKETPLACE_NAME; use codex_core_plugins::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use codex_core_plugins::remote::REMOTE_WORKSPACE_MARKETPLACE_NAME; @@ -24,11 +25,13 @@ use codex_core_plugins::remote::is_valid_remote_plugin_id; use codex_core_plugins::remote::validate_remote_plugin_id; use codex_core_plugins::remote_bundle::RemotePluginBundleInstallError; use codex_mcp::McpOAuthLoginSupport; -use codex_mcp::oauth_login_support; +use codex_mcp::McpRuntimeContext; +use codex_mcp::oauth_login_support_with_http_client; use codex_mcp::should_retry_without_scopes; use codex_plugin::PluginId; use codex_plugin::PluginTelemetryMetadata; use codex_protocol::auth::AuthMode as DomainAuthMode; +use codex_rmcp_client::OAuthDiscoveryTimeout; use codex_rmcp_client::perform_oauth_login_silent; #[derive(Clone)] @@ -1520,13 +1523,16 @@ impl PluginRequestProcessor { self.on_effective_plugins_changed(); - let plugin_mcp_servers = load_plugin_mcp_servers( + let plugin_mcp_servers = load_configured_plugin_mcp_servers( result.installed_path.as_path(), auth.as_ref().map(CodexAuth::auth_mode), + &result.plugin_id, + &config.config_layer_stack, + config.codex_home.as_path(), ) .await; if !plugin_mcp_servers.is_empty() { - self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) + self.start_plugin_mcp_oauth_logins(&config, &result.plugin_id, plugin_mcp_servers) .await; } @@ -1694,13 +1700,16 @@ impl PluginRequestProcessor { self.analytics_events_client .track_plugin_installed(plugin_metadata); - let plugin_mcp_servers = load_plugin_mcp_servers( + let plugin_mcp_servers = load_configured_plugin_mcp_servers( result.installed_path.as_path(), auth.as_ref().map(CodexAuth::auth_mode), + &result.plugin_id, + &config.config_layer_stack, + config.codex_home.as_path(), ) .await; if !plugin_mcp_servers.is_empty() { - self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) + self.start_plugin_mcp_oauth_logins(&config, &result.plugin_id, plugin_mcp_servers) .await; } @@ -1850,10 +1859,42 @@ impl PluginRequestProcessor { async fn start_plugin_mcp_oauth_logins( &self, config: &Config, - plugin_mcp_servers: HashMap, + plugin_id: &PluginId, + mut plugin_mcp_servers: HashMap, ) { + let plugin_id = plugin_id.as_key(); + config.apply_plugin_mcp_server_requirements(&plugin_id, &mut plugin_mcp_servers); + let runtime_context = McpRuntimeContext::new( + self.thread_manager.environment_manager(), + config.cwd.to_path_buf(), + ); for (name, server) in plugin_mcp_servers { - let oauth_config = match oauth_login_support(&server.transport).await { + if !server.enabled { + continue; + } + if !server.is_local_environment() { + warn!( + plugin = %plugin_id, + server = %name, + environment_id = %server.environment_id, + "skipping plugin MCP OAuth for an unowned environment" + ); + continue; + } + let http_client = match runtime_context.resolve_http_client(&name, &server) { + Ok(http_client) => http_client, + Err(err) => { + warn!("failed to resolve MCP runtime for plugin install {name}: {err}"); + continue; + } + }; + let login_support = oauth_login_support_with_http_client( + &server.transport, + Arc::clone(&http_client), + OAuthDiscoveryTimeout::LOCAL, + ) + .await; + let oauth_config = match login_support { McpOAuthLoginSupport::Supported(config) => config, McpOAuthLoginSupport::Unsupported => continue, McpOAuthLoginSupport::Unknown(err) => { @@ -1877,6 +1918,7 @@ impl PluginRequestProcessor { let outgoing = Arc::clone(&self.outgoing); let notification_name = name.clone(); let thread_manager = Arc::clone(&self.thread_manager); + let http_client = Arc::clone(&http_client); tokio::spawn(async move { let oauth_client_id = server.oauth_client_id(); @@ -1892,6 +1934,7 @@ impl PluginRequestProcessor { server.oauth_resource.as_deref(), callback_port, callback_url.as_deref(), + Arc::clone(&http_client), ) .await; @@ -1909,6 +1952,7 @@ impl PluginRequestProcessor { server.oauth_resource.as_deref(), callback_port, callback_url.as_deref(), + http_client, ) .await } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 88e5134102..690ea366f2 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -48,6 +48,7 @@ use rmcp::transport::StreamableHttpService; use rmcp::transport::streamable_http_server::session::local::LocalSessionManager; use serde_json::json; use tempfile::TempDir; +use tokio::io::AsyncBufReadExt; use tokio::net::TcpListener; use tokio::task::JoinHandle; use tokio::time::timeout; @@ -1330,6 +1331,215 @@ async fn plugin_install_skips_mcp_oauth_for_chatgpt_dual_surface_plugin() -> Res Ok(()) } +#[tokio::test] +async fn plugin_install_skips_mcp_oauth_disabled_by_plugin_requirements() -> Result<()> { + let oauth_server = MockServer::start().await; + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + "[features]\nplugins = true\n", + )?; + std::fs::write( + codex_home.path().join("requirements.toml"), + r#"[plugins."sample-plugin@debug".mcp_servers.allowed.identity] +url = "https://example.com/allowed-mcp" +"#, + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + /*install_policy*/ None, + /*auth_policy*/ None, + )?; + write_plugin_source(repo_root.path(), "sample-plugin", &[])?; + write_plugin_mcp_config(repo_root.path(), "sample-plugin", &oauth_server.uri())?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let _: PluginInstallResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + + assert!( + oauth_server + .received_requests() + .await + .unwrap_or_default() + .is_empty() + ); + Ok(()) +} + +#[tokio::test] +async fn plugin_install_skips_mcp_oauth_disabled_by_plugin_config() -> Result<()> { + let oauth_server = MockServer::start().await; + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true + +[plugins."sample-plugin@debug".mcp_servers.sample-mcp] +enabled = false +"#, + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + /*install_policy*/ None, + /*auth_policy*/ None, + )?; + write_plugin_source(repo_root.path(), "sample-plugin", &[])?; + write_plugin_mcp_config(repo_root.path(), "sample-plugin", &oauth_server.uri())?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let _: PluginInstallResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + + assert!( + oauth_server + .received_requests() + .await + .unwrap_or_default() + .is_empty() + ); + let persisted_config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; + let persisted_config = toml::from_str::(&persisted_config)?; + assert_eq!( + persisted_config + .get("plugins") + .and_then(|plugins| plugins.get("sample-plugin@debug")) + .and_then(|plugin| plugin.get("mcp_servers")) + .and_then(|servers| servers.get("sample-mcp")) + .and_then(|server| server.get("enabled")) + .and_then(toml::Value::as_bool), + Some(false) + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn plugin_install_skips_mcp_oauth_for_unowned_environment() -> Result<()> { + const UNOWNED_ENVIRONMENT_ID: &str = "plugin-unowned-executor"; + + let oauth_server = MockServer::start().await; + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + "[features]\nplugins = true\n", + )?; + let mut executor = + tokio::process::Command::new(codex_utils_cargo_bin::cargo_bin("exec-server")?) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .kill_on_drop(true) + .spawn()?; + let executor_stdout = executor + .stdout + .take() + .ok_or_else(|| anyhow::anyhow!("exec-server fixture stdout was not captured"))?; + let mut executor_stdout_lines = tokio::io::BufReader::new(executor_stdout).lines(); + let executor_url = timeout(DEFAULT_TIMEOUT, executor_stdout_lines.next_line()) + .await?? + .ok_or_else(|| anyhow::anyhow!("exec-server fixture did not emit its WebSocket URL"))?; + let executor_url = toml::Value::String(executor_url); + std::fs::write( + codex_home.path().join("environments.toml"), + format!( + r#"include_local = true + +[[environments]] +id = "{UNOWNED_ENVIRONMENT_ID}" +url = {executor_url} +"# + ), + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + /*install_policy*/ None, + /*auth_policy*/ None, + )?; + write_plugin_source(repo_root.path(), "sample-plugin", &[])?; + std::fs::write( + repo_root.path().join("sample-plugin/.mcp.json"), + serde_json::to_vec_pretty(&json!({ + "mcpServers": { + "sample-mcp": { + "type": "http", + "url": format!("{}/mcp", oauth_server.uri()), + "environment_id": UNOWNED_ENVIRONMENT_ID, + }, + }, + }))?, + )?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let _: PluginInstallResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + + assert!( + oauth_server + .received_requests() + .await + .unwrap_or_default() + .is_empty() + ); + Ok(()) +} + #[tokio::test] async fn plugin_install_starts_mcp_oauth_with_formerly_disallowed_plugin_app() -> Result<()> { let (apps_server_url, apps_server_handle, _apps_server_control) = @@ -1405,41 +1615,42 @@ async fn plugin_install_starts_mcp_oauth_with_formerly_disallowed_plugin_app() - } #[tokio::test] -async fn plugin_install_starts_mcp_oauth_through_protected_resource_metadata() -> Result<()> { - let resource_server = MockServer::start().await; - let authorization_server = MockServer::start().await; - let resource_metadata_url = format!("{}/oauth-resource", resource_server.uri()); +async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Result<()> { + let proxy = MockServer::start().await; + let resource_url = "http://plugin-mcp.invalid"; + let authorization_url = "http://plugin-oauth.invalid"; + let resource_metadata_url = format!("{resource_url}/oauth-resource"); let challenge = format!("Bearer resource_metadata=\"{resource_metadata_url}\""); Mock::given(method("GET")) .and(path("/mcp")) .respond_with( ResponseTemplate::new(401).insert_header("WWW-Authenticate", challenge.as_str()), ) - .mount(&resource_server) + .mount(&proxy) .await; Mock::given(method("GET")) .and(path("/oauth-resource")) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "resource": resource_server.uri(), - "authorization_servers": [authorization_server.uri()], + "resource": resource_url, + "authorization_servers": [authorization_url], }))) - .mount(&resource_server) + .mount(&proxy) .await; Mock::given(method("GET")) .and(path("/.well-known/oauth-authorization-server")) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "authorization_endpoint": format!("{}/oauth/authorize", authorization_server.uri()), - "token_endpoint": format!("{}/oauth/token", authorization_server.uri()), - "registration_endpoint": format!("{}/oauth/register", authorization_server.uri()), + "authorization_endpoint": format!("{authorization_url}/oauth/authorize"), + "token_endpoint": format!("{authorization_url}/oauth/token"), + "registration_endpoint": format!("{authorization_url}/oauth/register"), "response_types_supported": ["code"], "code_challenge_methods_supported": ["S256"], }))) - .mount(&authorization_server) + .mount(&proxy) .await; Mock::given(method("POST")) .and(path("/oauth/register")) .respond_with(ResponseTemplate::new(400)) - .mount(&authorization_server) + .mount(&proxy) .await; let codex_home = TempDir::new()?; @@ -1457,13 +1668,24 @@ async fn plugin_install_starts_mcp_oauth_through_protected_resource_metadata() - /*auth_policy*/ None, )?; write_plugin_source(repo_root.path(), "sample-plugin", &[])?; - write_plugin_mcp_config(repo_root.path(), "sample-plugin", &resource_server.uri())?; + write_plugin_mcp_config(repo_root.path(), "sample-plugin", resource_url)?; let marketplace_path = AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + let proxy_uri = proxy.uri(); let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() + .with_env_overrides(&[ + ("HTTP_PROXY", Some(proxy_uri.as_str())), + ("http_proxy", Some(proxy_uri.as_str())), + ("HTTPS_PROXY", None), + ("https_proxy", None), + ("ALL_PROXY", None), + ("all_proxy", None), + ("NO_PROXY", None), + ("no_proxy", None), + ]) .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; @@ -1477,14 +1699,14 @@ async fn plugin_install_starts_mcp_oauth_through_protected_resource_metadata() - let _: PluginInstallResponse = timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; wait_for_remote_plugin_request_count( - &authorization_server, + &proxy, "POST", "/oauth/register", /*expected_count*/ 1, ) .await?; - let resource_metadata_requested = resource_server + let resource_metadata_requested = proxy .received_requests() .await .unwrap_or_default() @@ -1589,6 +1811,92 @@ async fn plugin_install_starts_remote_mcp_oauth_for_install_response_only_app() Ok(()) } +#[tokio::test] +async fn plugin_install_skips_remote_mcp_oauth_disabled_by_requirements() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + let oauth_server = MockServer::start().await; + let bundle_url = mount_remote_plugin_bundle( + &server, + /*status_code*/ 200, + remote_plugin_bundle_tar_gz_bytes_with_mcp_config("linear", &oauth_server.uri())?, + ) + .await; + configure_remote_plugin_with_apps_test(codex_home.path(), &server)?; + std::fs::write( + codex_home.path().join("requirements.toml"), + "[mcp_servers]\n", + )?; + mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await; + mount_empty_remote_installed_plugins(&server).await; + mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))]) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let request_id = send_remote_plugin_install_request(&mut mcp, REMOTE_PLUGIN_ID).await?; + let _: PluginInstallResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + + assert!( + oauth_server + .received_requests() + .await + .unwrap_or_default() + .is_empty() + ); + Ok(()) +} + +#[tokio::test] +async fn plugin_install_skips_remote_mcp_oauth_disabled_by_plugin_config() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + let oauth_server = MockServer::start().await; + let bundle_url = mount_remote_plugin_bundle( + &server, + /*status_code*/ 200, + remote_plugin_bundle_tar_gz_bytes_with_mcp_config("linear", &oauth_server.uri())?, + ) + .await; + configure_remote_plugin_with_apps_test(codex_home.path(), &server)?; + let config_path = codex_home.path().join("config.toml"); + let existing_config = std::fs::read_to_string(&config_path)?; + std::fs::write( + config_path, + format!( + "{existing_config}\n[plugins.\"linear@openai-curated-remote\".mcp_servers.sample-mcp]\nenabled = false\n" + ), + )?; + mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await; + mount_empty_remote_installed_plugins(&server).await; + mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))]) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let request_id = send_remote_plugin_install_request(&mut mcp, REMOTE_PLUGIN_ID).await?; + let _: PluginInstallResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + + assert!( + oauth_server + .received_requests() + .await + .unwrap_or_default() + .is_empty() + ); + Ok(()) +} + #[tokio::test] async fn plugin_install_skips_remote_mcp_oauth_for_bundled_same_name_app() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index b2be52d13d..7857edf670 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -19,16 +19,19 @@ use codex_core::config::find_codex_home; use codex_core::config::load_global_mcp_servers; use codex_core_plugins::PluginsManager; use codex_exec_server::EnvironmentManager; +use codex_exec_server::HttpClient; +use codex_exec_server::RouteAwareHttpClient; use codex_login::AuthManager; use codex_mcp::McpOAuthLoginSupport; use codex_mcp::McpRuntimeContext; use codex_mcp::ResolvedMcpOAuthScopes; use codex_mcp::compute_auth_statuses; -use codex_mcp::discover_supported_scopes; -use codex_mcp::oauth_login_support; +use codex_mcp::discover_supported_scopes_with_http_client; +use codex_mcp::oauth_login_support_with_http_client; use codex_mcp::resolve_oauth_scopes; use codex_mcp::should_retry_without_scopes; use codex_protocol::protocol::McpAuthStatus; +use codex_rmcp_client::OAuthDiscoveryTimeout; use codex_rmcp_client::delete_oauth_tokens; use codex_rmcp_client::perform_oauth_login; use codex_utils_cli::CliConfigOverrides; @@ -224,6 +227,7 @@ async fn perform_oauth_login_retry_without_scopes( oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, + http_client: Arc, ) -> Result<()> { match perform_oauth_login( name, @@ -237,6 +241,7 @@ async fn perform_oauth_login_retry_without_scopes( oauth_resource, callback_port, callback_url, + Arc::clone(&http_client), ) .await { @@ -255,6 +260,7 @@ async fn perform_oauth_login_retry_without_scopes( oauth_resource, callback_port, callback_url, + http_client, ) .await } @@ -381,7 +387,17 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re println!("Added global MCP server '{name}'."); - match oauth_login_support(&transport).await { + // `mcp add` assigns new servers to the local environment, so its immediate + // OAuth discovery uses the local route-aware HTTP client. + let http_client: Arc = + Arc::new(RouteAwareHttpClient::new(config.http_client_factory())); + let login_support = oauth_login_support_with_http_client( + &transport, + Arc::clone(&http_client), + OAuthDiscoveryTimeout::LOCAL, + ) + .await; + match login_support { McpOAuthLoginSupport::Supported(oauth_config) => { println!("Detected OAuth support. Starting OAuth flow…"); let resolved_scopes = resolve_oauth_scopes( @@ -401,6 +417,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), + http_client, ) .await?; println!("Successfully logged in."); @@ -479,9 +496,18 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) _ => bail!("OAuth login is only supported for streamable HTTP servers."), }; + // Standalone `mcp login` runs OAuth from the local CLI process; execution + // environment routing belongs to app-server and session MCP flows. + let http_client: Arc = + Arc::new(RouteAwareHttpClient::new(config.http_client_factory())); let explicit_scopes = (!scopes.is_empty()).then_some(scopes); let discovered_scopes = if explicit_scopes.is_none() && server.scopes.is_none() { - discover_supported_scopes(&server.transport).await + discover_supported_scopes_with_http_client( + &server.transport, + Arc::clone(&http_client), + OAuthDiscoveryTimeout::LOCAL, + ) + .await } else { None }; @@ -500,6 +526,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) server.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), + http_client, ) .await?; println!("Successfully logged in to MCP server '{name}'."); @@ -557,17 +584,20 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let mut entries: Vec<_> = mcp_servers.iter().collect(); entries.sort_by_key(|(name, _)| *name); + // Standalone `mcp list` only discovers auth through local HTTP; it has + // never initialized or routed through remote execution environments. + let runtime_context = McpRuntimeContext::new( + Arc::new(EnvironmentManager::without_environments( + config.http_client_factory(), + )), + config.cwd.to_path_buf(), + ); let auth_statuses = compute_auth_statuses( effective_mcp_servers.iter(), config.mcp_oauth_credentials_store_mode, config.auth_keyring_backend_kind(), auth.as_ref(), - &McpRuntimeContext::new( - Arc::new(EnvironmentManager::without_environments( - config.http_client_factory(), - )), - config.cwd.to_path_buf(), - ), + &runtime_context, ) .await; diff --git a/codex-rs/cli/tests/mcp_add_remove.rs b/codex-rs/cli/tests/mcp_add_remove.rs index d0fc5f327d..9ae1d5fd11 100644 --- a/codex-rs/cli/tests/mcp_add_remove.rs +++ b/codex-rs/cli/tests/mcp_add_remove.rs @@ -6,6 +6,11 @@ use codex_core::config::load_global_mcp_servers; use predicates::str::contains; use pretty_assertions::assert_eq; use tempfile::TempDir; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; fn codex_command(codex_home: &Path) -> Result { let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); @@ -68,6 +73,108 @@ async fn add_and_remove_server_updates_global_config() -> Result<()> { Ok(()) } +#[tokio::test] +async fn add_and_login_discover_oauth_through_configured_http_proxy() -> Result<()> { + let codex_home = TempDir::new()?; + let proxy = MockServer::start().await; + let resource_url = "http://cli-mcp.invalid"; + let challenge = "Bearer resource_metadata=\"http://cli-mcp.invalid/oauth-resource\""; + Mock::given(method("GET")) + .and(path("/mcp")) + .respond_with(ResponseTemplate::new(401).insert_header("WWW-Authenticate", challenge)) + .mount(&proxy) + .await; + Mock::given(method("GET")) + .and(path("/oauth-resource")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "resource": format!("{resource_url}/mcp"), + "authorization_servers": [resource_url], + }))) + .mount(&proxy) + .await; + Mock::given(method("GET")) + .and(path("/.well-known/oauth-authorization-server")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "authorization_endpoint": format!("{resource_url}/oauth/authorize"), + "token_endpoint": format!("{resource_url}/oauth/token"), + "registration_endpoint": format!("{resource_url}/oauth/register"), + "response_types_supported": ["code"], + "code_challenge_methods_supported": ["S256"], + }))) + .mount(&proxy) + .await; + Mock::given(method("POST")) + .and(path("/oauth/register")) + .respond_with(ResponseTemplate::new(400)) + .mount(&proxy) + .await; + + let mut add = codex_command(codex_home.path())?; + add.env("HTTP_PROXY", proxy.uri()) + .env("http_proxy", proxy.uri()) + .env_remove("HTTPS_PROXY") + .env_remove("https_proxy") + .env_remove("ALL_PROXY") + .env_remove("all_proxy") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .args([ + "-c", + "mcp_oauth_credentials_store=\"file\"", + "mcp", + "add", + "oauth", + "--url", + "http://cli-mcp.invalid/mcp", + ]); + let add_output = tokio::task::spawn_blocking(move || add.output()).await??; + assert!( + !add_output.status.success(), + "mock OAuth registration should terminate the automatic login" + ); + assert!( + load_global_mcp_servers(codex_home.path()) + .await? + .contains_key("oauth") + ); + + // Local OAuth login does not require the execution-environment registry. + std::fs::write(codex_home.path().join("environments.toml"), "invalid = [")?; + + let mut login = codex_command(codex_home.path())?; + login + .env("HTTP_PROXY", proxy.uri()) + .env("http_proxy", proxy.uri()) + .env_remove("HTTPS_PROXY") + .env_remove("https_proxy") + .env_remove("ALL_PROXY") + .env_remove("all_proxy") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .args([ + "-c", + "mcp_oauth_credentials_store=\"file\"", + "mcp", + "login", + "oauth", + ]); + let login_output = tokio::task::spawn_blocking(move || login.output()).await??; + assert!( + !login_output.status.success(), + "mock OAuth registration should terminate the explicit login" + ); + + let registrations = proxy + .received_requests() + .await + .expect("mock proxy should record OAuth requests") + .iter() + .filter(|request| request.method == "POST" && request.url.path() == "/oauth/register") + .count(); + assert_eq!(registrations, 2); + Ok(()) +} + #[tokio::test] async fn profile_mcp_reports_legacy_profile_migration() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/cli/tests/mcp_list.rs b/codex-rs/cli/tests/mcp_list.rs index 7590153a6d..9bbc9a6a41 100644 --- a/codex-rs/cli/tests/mcp_list.rs +++ b/codex-rs/cli/tests/mcp_list.rs @@ -114,6 +114,7 @@ enabled = true async fn list_discovers_local_oauth_server_through_environment_proxy() -> Result<()> { let codex_home = TempDir::new()?; configure_http_oauth_server(codex_home.path(), "http://mcp-proxy.invalid/mcp").await?; + std::fs::write(codex_home.path().join("environments.toml"), "invalid = [")?; let listener = TcpListener::bind("127.0.0.1:0")?; listener.set_nonblocking(true)?; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 8cd57bb4c7..5371327e83 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -239,7 +239,9 @@ fn merge_configured_plugins_with_remote_installed( Ok(plugin_id) => plugin_id.marketplace_name != crate::OPENAI_CURATED_MARKETPLACE_NAME, Err(_) => true, }); - configured_plugins.extend(extra_plugins); + for (plugin_key, plugin_config) in extra_plugins { + merge_remote_plugin_config(&mut configured_plugins, plugin_key, plugin_config); + } return configured_plugins; } @@ -273,7 +275,7 @@ fn merge_configured_plugins_with_remote_installed( continue; } - configured_plugins.insert(plugin_key, plugin_config); + merge_remote_plugin_config(&mut configured_plugins, plugin_key, plugin_config); } configured_plugins @@ -301,6 +303,19 @@ pub(crate) fn plugin_is_eligible_for_target_marketplace( } } +fn merge_remote_plugin_config( + configured_plugins: &mut HashMap, + plugin_key: String, + mut remote_plugin_config: PluginConfig, +) { + if let Some(configured_plugin) = configured_plugins.get(&plugin_key) { + remote_plugin_config + .mcp_servers + .clone_from(&configured_plugin.mcp_servers); + } + configured_plugins.insert(plugin_key, remote_plugin_config); +} + fn installed_plugin_name_for_marketplace( plugin_key: &str, marketplace_name: &str, @@ -1362,11 +1377,37 @@ pub async fn plugin_capability_summary_from_root( }) } +/// Loads plugin MCP servers without applying user-specific policy overrides. pub async fn load_plugin_mcp_servers( plugin_root: &Path, auth_mode: Option, ) -> HashMap { - let mut mcp_servers = load_declared_plugin_mcp_servers(plugin_root).await; + load_plugin_mcp_servers_with_policy(plugin_root, auth_mode, /*plugin_policy*/ None).await +} + +/// Loads plugin MCP servers with the effective user policy for an installed plugin. +pub async fn load_configured_plugin_mcp_servers( + plugin_root: &Path, + auth_mode: Option, + plugin_id: &PluginId, + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> HashMap { + let configured_plugins = configured_plugins_from_stack(config_layer_stack, codex_home); + let plugin_id = plugin_id.as_key(); + let plugin_policy = configured_plugins + .get(&plugin_id) + .map(|plugin| &plugin.mcp_servers); + + load_plugin_mcp_servers_with_policy(plugin_root, auth_mode, plugin_policy).await +} + +async fn load_plugin_mcp_servers_with_policy( + plugin_root: &Path, + auth_mode: Option, + plugin_policy: Option<&HashMap>, +) -> HashMap { + let mut mcp_servers = load_declared_plugin_mcp_servers(plugin_root, plugin_policy).await; if !apps_route_available(auth_mode) || mcp_servers.is_empty() { return mcp_servers; } @@ -1381,13 +1422,15 @@ pub async fn load_plugin_mcp_servers( mcp_servers } -async fn load_declared_plugin_mcp_servers(plugin_root: &Path) -> HashMap { +async fn load_declared_plugin_mcp_servers( + plugin_root: &Path, + plugin_policy: Option<&HashMap>, +) -> HashMap { let Some(manifest) = load_plugin_manifest(plugin_root) else { return HashMap::new(); }; - load_plugin_mcp_servers_from_manifest(plugin_root, &manifest.paths, /*plugin_policy*/ None) - .await + load_plugin_mcp_servers_from_manifest(plugin_root, &manifest.paths, plugin_policy).await } pub(crate) async fn load_plugin_mcp_servers_from_manifest( diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 77cc8bdf49..33127535ca 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -1120,6 +1120,76 @@ approval_mode = "approve" ); } +#[tokio::test] +async fn remote_installed_plugin_preserves_configured_mcp_server_policy() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home + .path() + .join("plugins/cache/openai-curated-remote/linear/local"); + write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear"); + write_file( + &plugin_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "linear": { + "type": "http", + "url": "https://linear.example/mcp" + } + } +}"#, + ); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true + +[plugins."linear@openai-curated-remote"] +enabled = false + +[plugins."linear@openai-curated-remote".mcp_servers.linear] +enabled = false +default_tools_approval_mode = "approve" +enabled_tools = ["search"] +disabled_tools = ["delete"] + +[plugins."linear@openai-curated-remote".mcp_servers.linear.tools.search] +approval_mode = "approve" +"#, + ); + + let config = load_config(codex_home.path(), codex_home.path()).await; + let manager = PluginsManager::new_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + manager.write_remote_installed_plugins_cache(vec![remote_installed_linear_plugin()]); + + let outcome = manager.plugins_for_config(&config).await; + let plugin = outcome + .plugins() + .iter() + .find(|plugin| plugin.config_name == "linear@openai-curated-remote") + .expect("remote plugin should be loaded"); + let expected_server = serde_json::from_value::(serde_json::json!({ + "url": "https://linear.example/mcp", + "enabled": false, + "default_tools_approval_mode": "approve", + "enabled_tools": ["search"], + "disabled_tools": ["delete"], + "tools": { + "search": { "approval_mode": "approve" } + } + })) + .expect("valid expected MCP server"); + + assert!(plugin.enabled); + assert_eq!( + plugin.mcp_servers, + HashMap::from([("linear".to_string(), expected_server)]) + ); +} + #[tokio::test] async fn remote_installed_cache_ignores_plugins_missing_local_cache() { let codex_home = TempDir::new().unwrap(); diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 61d5c54a59..2f3bfbc909 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::sync::Arc; use codex_config::ConfigEditsBuilder; use codex_config::McpServerConfig; @@ -11,6 +12,7 @@ use codex_protocol::request_user_input::RequestUserInputArgs; use codex_protocol::request_user_input::RequestUserInputQuestion; use codex_protocol::request_user_input::RequestUserInputQuestionOption; use codex_protocol::request_user_input::RequestUserInputResponse; +use codex_rmcp_client::OAuthDiscoveryTimeout; use codex_rmcp_client::perform_oauth_login; use tokio_util::sync::CancellationToken; use tracing::warn; @@ -23,7 +25,7 @@ use codex_mcp::ElicitationReviewerHandle; use codex_mcp::McpOAuthLoginSupport; use codex_mcp::McpPermissionPromptAutoApproveContext; use codex_mcp::mcp_permission_prompt_is_auto_approved; -use codex_mcp::oauth_login_support; +use codex_mcp::oauth_login_support_with_http_client; use codex_mcp::resolve_oauth_scopes; use codex_mcp::should_retry_without_scopes; @@ -132,8 +134,27 @@ pub(crate) async fn maybe_install_mcp_dependencies( return; } + let (_, runtime_context) = sess.runtime_mcp_config_and_context(config).await; for (name, server_config) in added { - let oauth_config = match oauth_login_support(&server_config.transport).await { + let http_client = match runtime_context.resolve_http_client(&name, &server_config) { + Ok(http_client) => http_client, + Err(err) => { + warn!("failed to resolve MCP dependency runtime for {name}: {err}"); + continue; + } + }; + let discovery_timeout = if server_config.is_local_environment() { + OAuthDiscoveryTimeout::LOCAL + } else { + OAuthDiscoveryTimeout::Requested + }; + let login_support = oauth_login_support_with_http_client( + &server_config.transport, + Arc::clone(&http_client), + discovery_timeout, + ) + .await; + let oauth_config = match login_support { McpOAuthLoginSupport::Supported(config) => config, McpOAuthLoginSupport::Unsupported => continue, McpOAuthLoginSupport::Unknown(err) => { @@ -160,6 +181,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( server_config.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), + Arc::clone(&http_client), ) .await; @@ -177,6 +199,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( server_config.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), + Arc::clone(&http_client), ) .await { diff --git a/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs b/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs index 70f0dd2ff9..e0a6dc4bdc 100644 --- a/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs +++ b/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs @@ -5,20 +5,36 @@ use anyhow::Result; use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use codex_exec_server::CreateDirectoryOptions; use codex_features::Feature; +use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::user_input::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::responses; use core_test_support::skip_if_no_network; +use core_test_support::skip_if_target_windows; +use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; use tokio::process::Command; +use wiremock::Mock; use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; const PROXY_TEST_SUBPROCESS_ENV_VAR: &str = "CODEX_MCP_HTTP_PROXY_TEST_SUBPROCESS"; const TEST_NAME: &str = "suite::mcp_startup_refresh_http_proxy::local_mcp_startup_and_refresh_use_configured_http_client"; +const SKILL_TEST_NAME: &str = + "suite::mcp_startup_refresh_http_proxy::skill_mcp_dependency_oauth_uses_configured_http_client"; const SERVER_NAME: &str = "proxied_mcp"; const SERVER_URL: &str = "http://mcp-proxy.invalid/api/codex/ps/mcp"; @@ -156,3 +172,174 @@ async fn local_mcp_startup_and_refresh_use_configured_http_client() -> Result<() assert_eq!(result.is_error, Some(false)); Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()> { + skip_if_target_windows!(Ok(()), "requires native cross-OS skill paths"); + skip_if_no_network!(Ok(())); + + if std::env::var_os(PROXY_TEST_SUBPROCESS_ENV_VAR).is_none() { + let proxy = MockServer::start().await; + let _apps_server = AppsTestServer::mount(&proxy).await?; + let challenge = "Bearer resource_metadata=\"http://mcp-proxy.invalid/oauth-resource\""; + Mock::given(method("GET")) + .and(path("/api/codex/ps/mcp")) + .respond_with(ResponseTemplate::new(401).insert_header("WWW-Authenticate", challenge)) + .mount(&proxy) + .await; + Mock::given(method("GET")) + .and(path("/oauth-resource")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "resource": SERVER_URL, + "authorization_servers": ["http://mcp-proxy.invalid"], + }))) + .mount(&proxy) + .await; + Mock::given(method("GET")) + .and(path("/.well-known/oauth-authorization-server")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "authorization_endpoint": "http://mcp-proxy.invalid/oauth/authorize", + "token_endpoint": "http://mcp-proxy.invalid/oauth/token", + "registration_endpoint": "http://mcp-proxy.invalid/oauth/register", + "response_types_supported": ["code"], + "code_challenge_methods_supported": ["S256"], + }))) + .mount(&proxy) + .await; + Mock::given(method("POST")) + .and(path("/oauth/register")) + .respond_with(ResponseTemplate::new(400)) + .mount(&proxy) + .await; + + let mut command = Command::new(std::env::current_exe()?); + command.arg("--exact").arg(SKILL_TEST_NAME); + for &key in codex_network_proxy::PROXY_ENV_KEYS { + command.env_remove(key); + } + command + .env(PROXY_TEST_SUBPROCESS_ENV_VAR, "1") + .env("HTTP_PROXY", proxy.uri()) + .env("http_proxy", proxy.uri()) + .env("NO_PROXY", codex_network_proxy::DEFAULT_NO_PROXY_VALUE) + .env("no_proxy", codex_network_proxy::DEFAULT_NO_PROXY_VALUE); + + let output = command.output().await?; + let requests = proxy + .received_requests() + .await + .expect("mock proxy should record MCP OAuth requests"); + assert!( + output.status.success(), + "subprocess test `{SKILL_TEST_NAME}` failed\nstdout:\n{}\nstderr:\n{}\nproxy requests:\n{requests:#?}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + assert!( + requests.iter().any(|request| { + request.method == "POST" && request.url.path() == "/oauth/register" + }), + "skill dependency OAuth registration should use the configured proxy: {requests:#?}" + ); + return Ok(()); + } + + let responses_server = responses::start_mock_server().await; + let mut builder = test_codex() + .with_config(|config| { + config + .features + .enable(Feature::SkillMcpDependencyInstall) + .expect("test config should allow skill MCP dependency installation"); + if cfg!(target_os = "linux") { + config + .features + .enable(Feature::RespectSystemProxy) + .expect("test config should allow the system proxy feature"); + config.respect_system_proxy = true; + } + }) + .with_workspace_setup(|cwd, fs| async move { + let skill_dir = cwd.join(".agents/skills/proxy-skill"); + let agents_dir = skill_dir.join("agents"); + fs.create_directory( + &PathUri::from_host_native_path(&agents_dir)?, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + fs.write_file( + &PathUri::from_host_native_path(skill_dir.join("SKILL.md"))?, + b"---\nname: proxy-skill\ndescription: Uses a proxied MCP server.\n---\n".to_vec(), + /*sandbox*/ None, + ) + .await?; + let metadata = format!( + "dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n" + ); + fs.write_file( + &PathUri::from_host_native_path(agents_dir.join("openai.yaml"))?, + metadata.into_bytes(), + /*sandbox*/ None, + ) + .await?; + Ok(()) + }); + let fixture = builder.build_with_auto_env(&responses_server).await?; + responses::mount_sse_once( + &responses_server, + responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("msg-1", "done"), + responses::ev_completed("resp-1"), + ]), + ) + .await; + + let skill_path = fixture + .config + .cwd + .join(".agents/skills/proxy-skill/SKILL.md") + .canonicalize() + .unwrap_or_else(|_| { + fixture + .config + .cwd + .join(".agents/skills/proxy-skill/SKILL.md") + }); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, fixture.config.cwd.as_path()); + fixture + .codex + .submit(Op::UserInput { + items: vec![ + UserInput::Text { + text: "please use $proxy-skill".to_string(), + text_elements: Vec::new(), + }, + UserInput::Skill { + name: "proxy-skill".to_string(), + path: skill_path.to_path_buf(), + }, + ], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + environments: Some(local_selections(fixture.config.cwd.clone())), + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + ..Default::default() + }, + }) + .await?; + core_test_support::wait_for_event(fixture.codex.as_ref(), |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let servers = codex_config::load_global_mcp_servers(&fixture.config.codex_home).await?; + assert!(servers.contains_key(SERVER_NAME)); + Ok(()) +} diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index ad493a2956..d61ad311c2 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -96,6 +96,7 @@ pub async fn perform_oauth_login( oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, + http_client: Arc, ) -> Result<()> { perform_oauth_login_with_browser_output( server_name, @@ -109,6 +110,7 @@ pub async fn perform_oauth_login( oauth_resource, callback_port, callback_url, + http_client, /*emit_browser_url*/ true, ) .await @@ -127,6 +129,7 @@ pub async fn perform_oauth_login_silent( oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, + http_client: Arc, ) -> Result<()> { perform_oauth_login_with_browser_output( server_name, @@ -140,6 +143,7 @@ pub async fn perform_oauth_login_silent( oauth_resource, callback_port, callback_url, + http_client, /*emit_browser_url*/ false, ) .await @@ -158,14 +162,13 @@ async fn perform_oauth_login_with_browser_output( oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, + http_client: Arc, emit_browser_url: bool, ) -> Result<()> { let http_context = OAuthHttpContext { http_headers, env_http_headers, - http_client: Arc::new(RouteAwareHttpClient::new(HttpClientFactory::new( - OutboundProxyPolicy::ReqwestDefault, - ))), + http_client, }; OauthLoginFlow::new( server_name, @@ -743,9 +746,17 @@ mod tests { use axum::Router; use axum::routing::get; use axum::routing::post; + use codex_config::types::AuthKeyringBackendKind; + use codex_config::types::OAuthCredentialsStoreMode; + use codex_exec_server::ExecServerError; + use codex_exec_server::HttpClient; + use codex_exec_server::HttpRequestParams; + use codex_exec_server::HttpRequestResponse; + use codex_exec_server::HttpResponseBodyStream; use codex_exec_server::RouteAwareHttpClient; use codex_http_client::HttpClientFactory; use codex_http_client::OutboundProxyPolicy; + use futures::future::BoxFuture; use pretty_assertions::assert_eq; use reqwest::Url; use reqwest::header::HeaderMap; @@ -760,8 +771,41 @@ mod tests { use super::callback_id_from_server_url; use super::callback_path_from_redirect_uri; use super::parse_oauth_callback; + use super::perform_oauth_login; + use super::perform_oauth_login_silent; use super::start_authorization; + #[derive(Default)] + struct RecordingHttpClient { + requests: AtomicUsize, + } + + impl HttpClient for RecordingHttpClient { + fn http_request( + &self, + _params: HttpRequestParams, + ) -> BoxFuture<'_, Result> { + Box::pin(async { + Err(ExecServerError::HttpRequest( + "unexpected buffered OAuth request".to_string(), + )) + }) + } + + fn http_request_stream( + &self, + _params: HttpRequestParams, + ) -> BoxFuture<'_, Result<(HttpRequestResponse, HttpResponseBodyStream), ExecServerError>> + { + self.requests.fetch_add(1, Ordering::SeqCst); + Box::pin(async { + Err(ExecServerError::HttpRequest( + "configured OAuth client was used".to_string(), + )) + }) + } + } + async fn spawn_oauth_metadata_server() -> String { let listener = TcpListener::bind("127.0.0.1:0") .await @@ -930,6 +974,52 @@ mod tests { } } + #[tokio::test] + async fn interactive_oauth_login_uses_supplied_http_client() { + let http_client = Arc::new(RecordingHttpClient::default()); + perform_oauth_login( + "configured-client", + "http://127.0.0.1:1/mcp", + OAuthCredentialsStoreMode::default(), + AuthKeyringBackendKind::default(), + /*http_headers*/ None, + /*env_http_headers*/ None, + &[], + /*oauth_client_id*/ None, + /*oauth_resource*/ None, + /*callback_port*/ None, + /*callback_url*/ None, + http_client.clone(), + ) + .await + .expect_err("OAuth metadata discovery should fail through the supplied client"); + + assert!(http_client.requests.load(Ordering::SeqCst) > 0); + } + + #[tokio::test] + async fn silent_oauth_login_uses_supplied_http_client() { + let http_client = Arc::new(RecordingHttpClient::default()); + perform_oauth_login_silent( + "configured-client", + "http://127.0.0.1:1/mcp", + OAuthCredentialsStoreMode::default(), + AuthKeyringBackendKind::default(), + /*http_headers*/ None, + /*env_http_headers*/ None, + &[], + /*oauth_client_id*/ None, + /*oauth_resource*/ None, + /*callback_port*/ None, + /*callback_url*/ None, + http_client.clone(), + ) + .await + .expect_err("OAuth metadata discovery should fail through the supplied client"); + + assert!(http_client.requests.load(Ordering::SeqCst) > 0); + } + #[test] fn parse_oauth_callback_accepts_default_path() { let parsed = parse_oauth_callback("/callback?code=abc&state=xyz", "/callback");