mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
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
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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<String, McpServerConfig>,
|
||||
plugin_id: &PluginId,
|
||||
mut plugin_mcp_servers: HashMap<String, McpServerConfig>,
|
||||
) {
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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::<toml::Value>(&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()?;
|
||||
|
||||
+40
-10
@@ -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<u16>,
|
||||
callback_url: Option<&str>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
) -> 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<dyn HttpClient> =
|
||||
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<dyn HttpClient> =
|
||||
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;
|
||||
|
||||
|
||||
@@ -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<assert_cmd::Command> {
|
||||
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()?;
|
||||
|
||||
@@ -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)?;
|
||||
|
||||
@@ -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<String, PluginConfig>,
|
||||
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<AuthMode>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
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<AuthMode>,
|
||||
plugin_id: &PluginId,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
codex_home: &Path,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
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<AuthMode>,
|
||||
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
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<String, McpServerConfig> {
|
||||
async fn load_declared_plugin_mcp_servers(
|
||||
plugin_root: &Path,
|
||||
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
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(
|
||||
|
||||
@@ -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::<McpServerConfig>(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();
|
||||
|
||||
@@ -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
|
||||
{
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
@@ -96,6 +96,7 @@ pub async fn perform_oauth_login(
|
||||
oauth_resource: Option<&str>,
|
||||
callback_port: Option<u16>,
|
||||
callback_url: Option<&str>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
) -> 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<u16>,
|
||||
callback_url: Option<&str>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
) -> 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<u16>,
|
||||
callback_url: Option<&str>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
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<HttpRequestResponse, ExecServerError>> {
|
||||
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");
|
||||
|
||||
Reference in New Issue
Block a user