Support per-server MCP OAuth callback ports (#38448)

## What changed

- Add `oauth.callback_port` to MCP server configuration and preserve it when
  serializing configuration edits.
- Accept `oauth.callbackPort` from plugin MCP declarations and skill dependency
  metadata.
- Prefer the server-specific callback port over `mcp_oauth_callback_port` for
  CLI login, app-server, plugin installation, executor, and skill dependency
  OAuth flows.

## Testing

- Cover configuration parsing, serialization, and fallback behavior.
- Verify plugin, executor, and skill OAuth registrations use their configured
  callback ports instead of the global port.

GitOrigin-RevId: 7f65e5e7869358307e49779f1b75e8672b607736
This commit is contained in:
willwang-openai
2026-08-13 22:42:12 +00:00
committed by copyberry
parent ca83f7908c
commit 1da59ad257
22 changed files with 205 additions and 37 deletions
@@ -210,7 +210,7 @@ impl McpRequestProcessor {
client_registration,
server.oauth_resource.as_deref(),
timeout_secs,
mcp_config.mcp_oauth_callback_port,
server.oauth_callback_port(mcp_config.mcp_oauth_callback_port),
mcp_config.mcp_oauth_callback_url.as_deref(),
http_client,
redirect_mode,
@@ -1980,7 +1980,7 @@ impl PluginRequestProcessor {
let store_mode = config.mcp_oauth_credentials_store_mode;
let keyring_backend_kind = config.auth_keyring_backend_kind();
let callback_port = config.mcp_oauth_callback_port;
let callback_port = server.oauth_callback_port(config.mcp_oauth_callback_port);
let callback_url = config.mcp_oauth_callback_url.clone();
let outgoing = Arc::clone(&self.outgoing);
let notification_name = name.clone();
@@ -150,11 +150,17 @@ async fn selected_executor_plugin_exposes_its_mcps_only_to_that_thread() -> Resu
let http_server_handle = tokio::spawn(async move {
let _ = axum::serve(http_listener, http_router).await;
});
let plugin_callback_listener = TcpListener::bind("127.0.0.1:0").await?;
let plugin_callback_port = plugin_callback_listener.local_addr()?.port();
let global_callback_listener = TcpListener::bind("127.0.0.1:0").await?;
let global_callback_port = global_callback_listener.local_addr()?.port();
drop(plugin_callback_listener);
let codex_home = TempDir::new()?;
let root_config = format!(
"compact_prompt = \"compact\"\nmodel_auto_compact_token_limit = 1024\nmcp_oauth_credentials_store = \"file\"\nmcp_oauth_callback_port = {global_callback_port}"
);
MockResponsesConfig::new(&responses_server.uri())
.with_root_config(
"compact_prompt = \"compact\"\nmodel_auto_compact_token_limit = 1024\nmcp_oauth_credentials_store = \"file\"",
)
.with_root_config(&root_config)
.with_provider_config("supports_websockets = false")
.write(codex_home.path())?;
let executor_config: codex_config::types::McpServerConfig = serde_json::from_value(json!({
@@ -221,12 +227,16 @@ HTTP_PROXY = {http_proxy}
(OAUTH_MCP_SERVER_NAME): {
"url": EXECUTOR_OAUTH_MCP_URL,
"environment_id": "local",
"oauth": {"callbackPort": plugin_callback_port},
"startup_timeout_sec": 10,
},
(PRE_REGISTERED_OAUTH_MCP_SERVER_NAME): {
"url": EXECUTOR_OAUTH_MCP_URL,
"environment_id": "local",
"oauth": {"clientId": "configured-client"},
"oauth": {
"clientId": "configured-client",
"callbackPort": plugin_callback_port,
},
"startup_timeout_sec": 10,
}
}
@@ -362,6 +372,7 @@ startup_timeout_sec = 10
json!([redirect_uri.clone()])
);
let mut callback_url = reqwest::Url::parse(&redirect_uri)?;
assert_eq!(callback_url.port(), Some(plugin_callback_port));
callback_url
.query_pairs_mut()
.append_pair("code", "executor-test-code")
@@ -1818,10 +1818,16 @@ async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Resu
.mount(&proxy)
.await;
let plugin_callback_listener = TcpListener::bind("127.0.0.1:0").await?;
let plugin_callback_port = plugin_callback_listener.local_addr()?.port();
let global_callback_listener = TcpListener::bind("127.0.0.1:0").await?;
let global_callback_port = global_callback_listener.local_addr()?.port();
drop(plugin_callback_listener);
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join("config.toml"),
"[features]\nplugins = true\n",
format!("mcp_oauth_callback_port = {global_callback_port}\n\n[features]\nplugins = true\n"),
)?;
let repo_root = TempDir::new()?;
write_plugin_marketplace(
@@ -1833,7 +1839,18 @@ async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Resu
/*auth_policy*/ None,
)?;
write_plugin_source(repo_root.path(), "sample-plugin", &[])?;
write_plugin_mcp_config(repo_root.path(), "sample-plugin", resource_url)?;
std::fs::write(
repo_root.path().join("sample-plugin/.mcp.json"),
serde_json::to_vec_pretty(&json!({
"mcpServers": {
"sample-mcp": {
"type": "http",
"url": format!("{resource_url}/mcp"),
"oauth": {"callbackPort": plugin_callback_port},
}
}
}))?,
)?;
let marketplace_path =
AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?;
@@ -1871,13 +1888,27 @@ async fn plugin_install_starts_mcp_oauth_through_configured_http_proxy() -> Resu
)
.await?;
let resource_metadata_requested = proxy
.received_requests()
.await
.unwrap_or_default()
let requests = proxy.received_requests().await.unwrap_or_default();
let resource_metadata_requested = requests
.iter()
.any(|request| request.url.path() == "/oauth-resource");
assert!(resource_metadata_requested);
let registration_request = requests
.iter()
.find(|request| request.url.path() == "/oauth/register")
.expect("OAuth client registration request");
let registration: serde_json::Value = serde_json::from_slice(&registration_request.body)?;
let redirect_uri: Uri = registration["redirect_uris"][0]
.as_str()
.expect("OAuth client registration redirect URI")
.parse()?;
assert_eq!(
redirect_uri
.authority()
.and_then(axum::http::uri::Authority::port_u16),
Some(plugin_callback_port)
);
Ok(())
}
+2 -1
View File
@@ -429,6 +429,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
.clone()
.map(|client_id| McpServerOAuthConfig {
client_id: Some(client_id),
callback_port: None,
}),
oauth_resource: oauth_resource.clone(),
tools: HashMap::new(),
@@ -593,7 +594,7 @@ async fn run_login(config: &Config, login_args: LoginArgs) -> Result<()> {
server.oauth_client_id(),
client_registration,
server.oauth_resource.as_deref(),
config.mcp_oauth_callback_port,
server.oauth_callback_port(config.mcp_oauth_callback_port),
config.mcp_oauth_callback_url.as_deref(),
http_client,
)
+4 -6
View File
@@ -266,12 +266,10 @@ fn normalize_plugin_mcp_server_value(
}
if let Some(JsonValue::Object(mut oauth)) = object.remove("oauth") {
if oauth.remove("callbackPort").is_some() {
let plugin_display = source.display();
warn!(
plugin = %plugin_display,
"plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings"
);
if let Some(callback_port) = oauth.remove("callbackPort") {
oauth
.entry("callback_port".to_string())
.or_insert(callback_port);
}
if let Some(client_id) = oauth.remove("clientId") {
@@ -582,6 +582,7 @@ fn declared_placement_preserves_local_plugin_normalization() {
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("client-id".to_string()),
callback_port: Some(9876),
}),
oauth_resource: None,
tools: HashMap::new(),
+16
View File
@@ -145,6 +145,10 @@ pub struct McpServerOAuthConfig {
/// Explicit OAuth client identifier to present during authorization and token exchange.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub client_id: Option<String>,
/// Fixed callback port that takes precedence over Codex's global OAuth callback port.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub callback_port: Option<u16>,
}
/// Authentication flow Codex attempts after resolving an HTTP MCP server's
@@ -272,6 +276,18 @@ impl McpServerConfig {
.as_ref()
.and_then(|oauth| oauth.client_id.as_deref())
}
pub fn oauth_callback_port(&self, global_callback_port: Option<u16>) -> Option<u16> {
let callback_port = self.oauth.as_ref().and_then(|oauth| oauth.callback_port);
if let Some(callback_port) = callback_port {
tracing::info!(
callback_port,
?global_callback_port,
"using plugin-specific MCP OAuth callback port instead of the global callback port"
);
}
callback_port.or(global_callback_port)
}
}
/// Raw MCP config shape used for deserialization and supported-field JSON
+30
View File
@@ -364,6 +364,7 @@ fn deserialize_streamable_http_server_config_with_oauth_client_id() {
[oauth]
client_id = "eci-prd-pub-codex-123"
callback_port = 9876
"#,
)
.expect("should deserialize http config with oauth client id");
@@ -372,10 +373,39 @@ fn deserialize_streamable_http_server_config_with_oauth_client_id() {
cfg.oauth,
Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
callback_port: Some(9876),
})
);
}
#[test]
fn oauth_callback_port_prefers_server_port_over_global_port() {
let cfg: McpServerConfig = toml::from_str(
r#"
url = "https://example.com/mcp"
[oauth]
callback_port = 9876
"#,
)
.expect("should deserialize http config with oauth callback port");
assert_eq!(cfg.oauth_callback_port(Some(4321)), Some(9876));
}
#[test]
fn oauth_callback_port_falls_back_to_global_port() {
let cfg: McpServerConfig = toml::from_str(
r#"
url = "https://example.com/mcp"
"#,
)
.expect("should deserialize http config without oauth callback port");
assert_eq!(cfg.oauth_callback_port(Some(4321)), Some(4321));
assert_eq!(cfg.oauth_callback_port(/*global_callback_port*/ None), None);
}
#[test]
fn deserialize_server_config_with_tool_filters() {
let cfg: McpServerConfig = toml::from_str(
@@ -969,6 +969,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("client-id".to_string()),
callback_port: Some(3118),
}),
oauth_resource: None,
tools: HashMap::new(),
+6
View File
@@ -1592,6 +1592,12 @@
"additionalProperties": false,
"description": "OAuth client settings used when Codex launches an MCP OAuth flow.",
"properties": {
"callback_port": {
"description": "Fixed callback port that takes precedence over Codex's global OAuth callback port.",
"format": "uint16",
"minimum": 0.0,
"type": "integer"
},
"client_id": {
"description": "Explicit OAuth client identifier to present during authorization and token exchange.",
"type": "string"
+1
View File
@@ -7449,6 +7449,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
callback_port: None,
}),
oauth_resource: Some("https://resource.example.com".to_string()),
tools: HashMap::new(),
@@ -147,14 +147,20 @@ fn serialize_mcp_server_table(config: &McpServerConfig) -> TomlTable {
{
entry["scopes"] = array_from_iter(scopes.iter().cloned());
}
if let Some(oauth) = &config.oauth
&& let Some(client_id) = &oauth.client_id
&& !client_id.is_empty()
{
if let Some(oauth) = &config.oauth {
let mut oauth_table = TomlTable::new();
oauth_table.set_implicit(false);
oauth_table["client_id"] = value(client_id.clone());
entry["oauth"] = TomlItem::Table(oauth_table);
if let Some(client_id) = &oauth.client_id
&& !client_id.is_empty()
{
oauth_table["client_id"] = value(client_id.clone());
}
if let Some(callback_port) = oauth.callback_port {
oauth_table["callback_port"] = value(i64::from(callback_port));
}
if !oauth_table.is_empty() {
entry["oauth"] = TomlItem::Table(oauth_table);
}
}
if let Some(resource) = &config.oauth_resource
&& !resource.is_empty()
+2
View File
@@ -1056,6 +1056,7 @@ fn blocking_replace_mcp_servers_round_trips() {
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
callback_port: Some(9876),
}),
oauth_resource: Some("https://resource.example.com".to_string()),
tools: HashMap::new(),
@@ -1084,6 +1085,7 @@ Z-Header = \"z\"
[mcp_servers.http.oauth]
client_id = \"eci-prd-pub-codex-123\"
callback_port = 9876
[mcp_servers.stdio]
command = \"cmd\"
+10 -3
View File
@@ -3,6 +3,7 @@ use std::collections::HashSet;
use std::sync::Arc;
use codex_config::McpServerConfig;
use codex_config::McpServerOAuthConfig;
use codex_config::McpServerTransportConfig;
use codex_config::load_global_mcp_servers;
use codex_login::default_client::is_first_party_originator;
@@ -174,6 +175,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
);
let oauth_client_id = server_config.oauth_client_id();
let oauth_credential_name = server_config.oauth_credential_name(&name);
let callback_port = server_config.oauth_callback_port(config.mcp_oauth_callback_port);
let first_attempt = perform_oauth_login(
oauth_credential_name.as_ref(),
&oauth_config.url,
@@ -185,7 +187,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
oauth_client_id,
McpOAuthClientRegistration::Auto,
server_config.oauth_resource.as_deref(),
config.mcp_oauth_callback_port,
callback_port,
config.mcp_oauth_callback_url.as_deref(),
Arc::clone(&http_client),
)
@@ -204,7 +206,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
oauth_client_id,
McpOAuthClientRegistration::Auto,
server_config.oauth_resource.as_deref(),
config.mcp_oauth_callback_port,
callback_port,
config.mcp_oauth_callback_url.as_deref(),
Arc::clone(&http_client),
)
@@ -405,7 +407,12 @@ fn mcp_dependency_to_server_config(
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth: dependency
.oauth_callback_port
.map(|callback_port| McpServerOAuthConfig {
client_id: None,
callback_port: Some(callback_port),
}),
oauth_resource: None,
tools: HashMap::new(),
});
@@ -5,6 +5,7 @@ use std::time::Duration;
use anyhow::Result;
use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
use codex_config::McpServerConfig;
use codex_config::McpServerOAuthConfig;
use codex_config::McpServerTransportConfig;
use codex_exec_server::CreateDirectoryOptions;
use codex_features::Feature;
@@ -25,7 +26,9 @@ use core_test_support::wait_for_mcp_server;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use tokio::net::TcpListener;
use tokio::process::Command;
use url::Url;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
@@ -33,6 +36,8 @@ use wiremock::matchers::method;
use wiremock::matchers::path;
const PROXY_TEST_SUBPROCESS_ENV_VAR: &str = "CODEX_MCP_HTTP_PROXY_TEST_SUBPROCESS";
const SKILL_CALLBACK_PORT_ENV_VAR: &str = "CODEX_MCP_SKILL_CALLBACK_PORT";
const SKILL_GLOBAL_CALLBACK_PORT_ENV_VAR: &str = "CODEX_MCP_SKILL_GLOBAL_CALLBACK_PORT";
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";
@@ -215,6 +220,12 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
.mount(&proxy)
.await;
let skill_callback_listener = TcpListener::bind("127.0.0.1:0").await?;
let skill_callback_port = skill_callback_listener.local_addr()?.port();
let global_callback_listener = TcpListener::bind("127.0.0.1:0").await?;
let global_callback_port = global_callback_listener.local_addr()?.port();
drop(skill_callback_listener);
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 {
@@ -222,6 +233,11 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
}
command
.env(PROXY_TEST_SUBPROCESS_ENV_VAR, "1")
.env(SKILL_CALLBACK_PORT_ENV_VAR, skill_callback_port.to_string())
.env(
SKILL_GLOBAL_CALLBACK_PORT_ENV_VAR,
global_callback_port.to_string(),
)
.env("HTTP_PROXY", proxy.uri())
.env("http_proxy", proxy.uri())
.env("NO_PROXY", codex_network_proxy::DEFAULT_NO_PROXY_VALUE)
@@ -238,22 +254,36 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr),
);
assert!(
requests.iter().any(|request| {
let registration_request = requests
.iter()
.find(|request| {
request.method == "POST" && request.url.path() == "/oauth/register"
}),
"skill dependency OAuth registration should use the configured proxy: {requests:#?}"
);
})
.unwrap_or_else(|| {
panic!(
"skill dependency OAuth registration should use the configured proxy: {requests:#?}"
)
});
let registration: Value = serde_json::from_slice(&registration_request.body)?;
let redirect_uri = Url::parse(
registration["redirect_uris"][0]
.as_str()
.expect("OAuth client registration redirect URI"),
)?;
assert_eq!(redirect_uri.port(), Some(skill_callback_port));
return Ok(());
}
let skill_callback_port = std::env::var(SKILL_CALLBACK_PORT_ENV_VAR)?.parse::<u16>()?;
let global_callback_port = std::env::var(SKILL_GLOBAL_CALLBACK_PORT_ENV_VAR)?.parse::<u16>()?;
let responses_server = responses::start_mock_server().await;
let mut builder = test_codex()
.with_config(|config| {
.with_config(move |config| {
config
.features
.enable(Feature::SkillMcpDependencyInstall)
.expect("test config should allow skill MCP dependency installation");
config.mcp_oauth_callback_port = Some(global_callback_port);
if cfg!(target_os = "linux") {
config
.features
@@ -262,7 +292,7 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
config.respect_system_proxy = true;
}
})
.with_workspace_setup(|cwd, fs| async move {
.with_workspace_setup(move |cwd, fs| async move {
let skill_dir = cwd.join(".agents/skills/proxy-skill");
let agents_dir = skill_dir.join("agents");
fs.create_directory(
@@ -278,7 +308,7 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
)
.await?;
let metadata = format!(
"dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n"
"dependencies:\n tools:\n - type: mcp\n value: {SERVER_NAME}\n transport: streamable_http\n url: {SERVER_URL}\n oauth:\n callbackPort: {skill_callback_port}\n"
);
fs.write_file(
&PathUri::from_host_native_path(agents_dir.join("openai.yaml"))?,
@@ -340,6 +370,14 @@ async fn skill_mcp_dependency_oauth_uses_configured_http_client() -> Result<()>
.await;
let servers = codex_config::load_global_mcp_servers(&fixture.config.codex_home).await?;
assert!(servers.contains_key(SERVER_NAME));
assert_eq!(
servers
.get(SERVER_NAME)
.and_then(|server| server.oauth.as_ref()),
Some(&McpServerOAuthConfig {
client_id: None,
callback_port: Some(skill_callback_port),
})
);
Ok(())
}
@@ -76,6 +76,7 @@ fn connector_dependencies_make_generic_skills_discoverable() {
transport: None,
command: None,
url: None,
oauth_callback_port: None,
}],
};
let documents = [
@@ -14,6 +14,7 @@ fn dependency_names_outrank_description_only_matches() {
transport: None,
command: None,
url: None,
oauth_callback_port: None,
}],
};
let documents = [
@@ -80,6 +81,7 @@ fn dependency_records_are_bounded() {
transport: None,
command: None,
url: None,
oauth_callback_port: None,
})
.collect::<Vec<_>>();
tools.push(SkillToolDependency {
@@ -89,6 +91,7 @@ fn dependency_records_are_bounded() {
transport: None,
command: None,
url: None,
oauth_callback_port: None,
});
let dependencies = SkillDependencies { tools };
let document = SkillSelectionDocument {
@@ -68,6 +68,7 @@ policy:
transport: Some("stdio".to_string()),
command: Some("deploy-mcp".to_string()),
url: None,
oauth_callback_port: None,
}],
}),
policy: Some(SkillPolicy {
@@ -117,6 +117,8 @@ async fn loads_host_frontmatter_dependencies_and_policy() {
- type: mcp
value: demo-tool
description: Demo tool
oauth:
callbackPort: 3118
policy:
allow_implicit_invocation: false
products: [codex, CHATGPT, atlas]
@@ -141,6 +143,7 @@ policy:
transport: None,
command: None,
url: None,
oauth_callback_port: Some(3118),
}],
}),
policy: Some(SkillPolicy {
@@ -62,6 +62,14 @@ struct DependencyTool {
transport: Option<String>,
command: Option<String>,
url: Option<String>,
oauth: Option<DependencyOAuth>,
}
#[derive(Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
struct DependencyOAuth {
#[serde(alias = "callback_port")]
callback_port: Option<u16>,
}
pub(super) async fn load_host_skill_metadata(
@@ -186,6 +194,7 @@ fn resolve_dependency_tool(tool: DependencyTool) -> Option<SkillToolDependency>
"dependencies.tools.command",
);
let url = resolve_str(tool.url, MAX_DEPENDENCY_URL_LEN, "dependencies.tools.url");
let oauth_callback_port = tool.oauth.and_then(|oauth| oauth.callback_port);
Some(SkillToolDependency {
r#type,
@@ -194,6 +203,7 @@ fn resolve_dependency_tool(tool: DependencyTool) -> Option<SkillToolDependency>
transport,
command,
url,
oauth_callback_port,
})
}
+1
View File
@@ -90,6 +90,7 @@ pub struct SkillToolDependency {
pub transport: Option<String>,
pub command: Option<String>,
pub url: Option<String>,
pub oauth_callback_port: Option<u16>,
}
fn matches_product_restriction(