diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index f4cdf6626..58ca63306 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -4014,6 +4014,7 @@ impl CodexMessageProcessor { scopes.as_deref().unwrap_or_default(), timeout_secs, config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await { diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 052af5619..be3678e07 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -273,6 +273,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re oauth_config.env_http_headers, &Vec::new(), config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await?; println!("Successfully logged in."); @@ -356,6 +357,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) env_http_headers, &scopes, config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await?; println!("Successfully logged in to MCP server '{name}'."); diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 9515ad038..16f5c1455 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1601,6 +1601,10 @@ "minimum": 0.0, "type": "integer" }, + "mcp_oauth_callback_url": { + "description": "Optional redirect URI to use during MCP OAuth login. When set, this URI is used in the OAuth authorization request instead of the local listener address. The local callback listener still binds to 127.0.0.1 (using `mcp_oauth_callback_port` when provided).", + "type": "string" + }, "mcp_oauth_credentials_store": { "allOf": [ { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index f11026ede..3d3a714fa 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -295,6 +295,13 @@ pub struct Config { /// When unset, Codex will bind to an ephemeral port chosen by the OS. pub mcp_oauth_callback_port: Option, + /// Optional redirect URI to use during MCP OAuth login. + /// + /// When set, this URI is used in the OAuth authorization request instead + /// of the local listener address. The local callback listener still binds + /// to 127.0.0.1 (using `mcp_oauth_callback_port` when provided). + pub mcp_oauth_callback_url: Option, + /// Combined provider map (defaults merged with user-defined overrides). pub model_providers: HashMap, @@ -1005,6 +1012,12 @@ pub struct ConfigToml { /// When unset, Codex will bind to an ephemeral port chosen by the OS. pub mcp_oauth_callback_port: Option, + /// Optional redirect URI to use during MCP OAuth login. + /// When set, this URI is used in the OAuth authorization request instead + /// of the local listener address. The local callback listener still binds + /// to 127.0.0.1 (using `mcp_oauth_callback_port` when provided). + pub mcp_oauth_callback_url: Option, + /// User-defined provider entries that extend/override the built-in list. #[serde(default)] pub model_providers: HashMap, @@ -1937,6 +1950,7 @@ impl Config { // is important in code to differentiate the mode from the store implementation. mcp_oauth_credentials_store_mode: cfg.mcp_oauth_credentials_store.unwrap_or_default(), mcp_oauth_callback_port: cfg.mcp_oauth_callback_port, + mcp_oauth_callback_url: cfg.mcp_oauth_callback_url.clone(), model_providers, project_doc_max_bytes: cfg.project_doc_max_bytes.unwrap_or(PROJECT_DOC_MAX_BYTES), project_doc_fallback_filenames: cfg @@ -4407,6 +4421,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -4524,6 +4539,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -4639,6 +4655,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -4740,6 +4757,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -5269,6 +5287,17 @@ trust_level = "untrusted" assert_eq!(cfg.mcp_oauth_callback_port, Some(4321)); } + #[test] + fn config_toml_deserializes_mcp_oauth_callback_url() { + let toml = r#"mcp_oauth_callback_url = "https://example.com/callback""#; + let cfg: ConfigToml = + toml::from_str(toml).expect("TOML deserialization should succeed for callback URL"); + assert_eq!( + cfg.mcp_oauth_callback_url.as_deref(), + Some("https://example.com/callback") + ); + } + #[test] fn config_loads_mcp_oauth_callback_port_from_toml() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -5289,6 +5318,29 @@ mcp_oauth_callback_port = 5678 Ok(()) } + #[test] + fn config_loads_mcp_oauth_callback_url_from_toml() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let toml = r#" +model = "gpt-5.1" +mcp_oauth_callback_url = "https://example.com/callback" +"#; + let cfg: ConfigToml = + toml::from_str(toml).expect("TOML deserialization should succeed for callback URL"); + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config.mcp_oauth_callback_url.as_deref(), + Some("https://example.com/callback") + ); + Ok(()) + } + #[test] fn test_untrusted_project_gets_unless_trusted_approval_policy() -> anyhow::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index 0e6d61055..e8b2d4f7b 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -242,6 +242,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_config.env_http_headers, &[], config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await { diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 426807733..4c8cd563a 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -840,8 +840,31 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { /// This test writes to a fallback credentials file in CODEX_HOME. /// Ideally, we wouldn't need to serialize the test but it's much more cumbersome to wire CODEX_HOME through the code. #[serial(codex_home)] -#[tokio::test(flavor = "multi_thread", worker_threads = 1)] -async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { +#[test] +fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { + const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024; + + let handle = std::thread::Builder::new() + .name("streamable_http_with_oauth_round_trip".to_string()) + .stack_size(TEST_STACK_SIZE_BYTES) + .spawn(|| -> anyhow::Result<()> { + let runtime = tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) + .enable_all() + .build()?; + runtime.block_on(streamable_http_with_oauth_round_trip_impl()) + })?; + + match handle.join() { + Ok(result) => result, + Err(_) => Err(anyhow::anyhow!( + "streamable_http_with_oauth_round_trip thread panicked" + )), + } +} + +#[allow(clippy::expect_used)] +async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index b5007fd58..6a0983a6e 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -535,6 +535,10 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { }) .await?; + let snapshot_path = wait_for_snapshot(&codex_home).await?; + let snapshot_content = fs::read_to_string(&snapshot_path).await?; + assert_posix_snapshot_sections(&snapshot_content); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; assert_eq!( @@ -542,10 +546,6 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { "hello from snapshot\n" ); - let snapshot_path = wait_for_snapshot(&codex_home).await?; - let snapshot_content = fs::read_to_string(&snapshot_path).await?; - assert_posix_snapshot_sections(&snapshot_content); - Ok(()) } diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index 09b746837..62a9c3b01 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -8,6 +8,7 @@ use anyhow::Result; use anyhow::anyhow; use anyhow::bail; use reqwest::ClientBuilder; +use reqwest::Url; use rmcp::transport::auth::OAuthState; use tiny_http::Response; use tiny_http::Server; @@ -38,6 +39,7 @@ impl Drop for CallbackServerGuard { } } +#[allow(clippy::too_many_arguments)] pub async fn perform_oauth_login( server_name: &str, server_url: &str, @@ -46,6 +48,7 @@ pub async fn perform_oauth_login( env_http_headers: Option>, scopes: &[String], callback_port: Option, + callback_url: Option<&str>, ) -> Result<()> { let headers = OauthHeaders { http_headers, @@ -59,6 +62,7 @@ pub async fn perform_oauth_login( scopes, true, callback_port, + callback_url, None, ) .await? @@ -76,6 +80,7 @@ pub async fn perform_oauth_login_return_url( scopes: &[String], timeout_secs: Option, callback_port: Option, + callback_url: Option<&str>, ) -> Result { let headers = OauthHeaders { http_headers, @@ -89,6 +94,7 @@ pub async fn perform_oauth_login_return_url( scopes, false, callback_port, + callback_url, timeout_secs, ) .await?; @@ -99,11 +105,15 @@ pub async fn perform_oauth_login_return_url( Ok(OauthLoginHandle::new(authorization_url, completion)) } -fn spawn_callback_server(server: Arc, tx: oneshot::Sender<(String, String)>) { +fn spawn_callback_server( + server: Arc, + tx: oneshot::Sender<(String, String)>, + expected_callback_path: String, +) { tokio::task::spawn_blocking(move || { while let Ok(request) = server.recv() { let path = request.url().to_string(); - match parse_oauth_callback(&path) { + match parse_oauth_callback(&path, &expected_callback_path) { CallbackOutcome::Success(OauthCallbackResult { code, state }) => { let response = Response::from_string( "Authentication complete. You may close this window.", @@ -146,11 +156,11 @@ enum CallbackOutcome { Invalid, } -fn parse_oauth_callback(path: &str) -> CallbackOutcome { +fn parse_oauth_callback(path: &str, expected_callback_path: &str) -> CallbackOutcome { let Some((route, query)) = path.split_once('?') else { return CallbackOutcome::Invalid; }; - if route != "/callback" { + if route != expected_callback_path { return CallbackOutcome::Invalid; } @@ -238,6 +248,53 @@ fn resolve_callback_port(callback_port: Option) -> Result> { Ok(None) } +fn local_redirect_uri(server: &Server) -> Result { + match server.server_addr() { + tiny_http::ListenAddr::IP(std::net::SocketAddr::V4(addr)) => { + let ip = addr.ip(); + let port = addr.port(); + Ok(format!("http://{ip}:{port}/callback")) + } + tiny_http::ListenAddr::IP(std::net::SocketAddr::V6(addr)) => { + let ip = addr.ip(); + let port = addr.port(); + Ok(format!("http://[{ip}]:{port}/callback")) + } + #[cfg(not(target_os = "windows"))] + _ => Err(anyhow!("unable to determine callback address")), + } +} + +fn resolve_redirect_uri(server: &Server, callback_url: Option<&str>) -> Result { + let Some(callback_url) = callback_url else { + return local_redirect_uri(server); + }; + Url::parse(callback_url) + .with_context(|| format!("invalid MCP OAuth callback URL `{callback_url}`"))?; + Ok(callback_url.to_string()) +} + +fn callback_path_from_redirect_uri(redirect_uri: &str) -> Result { + let parsed = Url::parse(redirect_uri) + .with_context(|| format!("invalid redirect URI `{redirect_uri}`"))?; + Ok(parsed.path().to_string()) +} + +fn callback_bind_host(callback_url: Option<&str>) -> &'static str { + let Some(callback_url) = callback_url else { + return "127.0.0.1"; + }; + + let Ok(parsed) = Url::parse(callback_url) else { + return "127.0.0.1"; + }; + + match parsed.host_str() { + Some("localhost" | "127.0.0.1" | "::1") | None => "127.0.0.1", + Some(_) => "0.0.0.0", + } +} + impl OauthLoginFlow { #[allow(clippy::too_many_arguments)] async fn new( @@ -248,14 +305,16 @@ impl OauthLoginFlow { scopes: &[String], launch_browser: bool, callback_port: Option, + callback_url: Option<&str>, timeout_secs: Option, ) -> Result { const DEFAULT_OAUTH_TIMEOUT_SECS: i64 = 300; + let bind_host = callback_bind_host(callback_url); let callback_port = resolve_callback_port(callback_port)?; let bind_addr = match callback_port { - Some(port) => format!("127.0.0.1:{port}"), - None => "127.0.0.1:0".to_string(), + Some(port) => format!("{bind_host}:{port}"), + None => format!("{bind_host}:0"), }; let server = Arc::new(Server::http(&bind_addr).map_err(|err| anyhow!(err))?); @@ -263,23 +322,11 @@ impl OauthLoginFlow { server: Arc::clone(&server), }; - let redirect_uri = match server.server_addr() { - tiny_http::ListenAddr::IP(std::net::SocketAddr::V4(addr)) => { - let ip = addr.ip(); - let port = addr.port(); - format!("http://{ip}:{port}/callback") - } - tiny_http::ListenAddr::IP(std::net::SocketAddr::V6(addr)) => { - let ip = addr.ip(); - let port = addr.port(); - format!("http://[{ip}]:{port}/callback") - } - #[cfg(not(target_os = "windows"))] - _ => return Err(anyhow!("unable to determine callback address")), - }; + let redirect_uri = resolve_redirect_uri(&server, callback_url)?; + let callback_path = callback_path_from_redirect_uri(&redirect_uri)?; let (tx, rx) = oneshot::channel(); - spawn_callback_server(server, tx); + spawn_callback_server(server, tx, callback_path); let OauthHeaders { http_headers, @@ -383,3 +430,35 @@ impl OauthLoginFlow { rx } } + +#[cfg(test)] +mod tests { + use super::CallbackOutcome; + use super::callback_path_from_redirect_uri; + use super::parse_oauth_callback; + + #[test] + fn parse_oauth_callback_accepts_default_path() { + let parsed = parse_oauth_callback("/callback?code=abc&state=xyz", "/callback"); + assert!(matches!(parsed, CallbackOutcome::Success(_))); + } + + #[test] + fn parse_oauth_callback_accepts_custom_path() { + let parsed = parse_oauth_callback("/oauth/callback?code=abc&state=xyz", "/oauth/callback"); + assert!(matches!(parsed, CallbackOutcome::Success(_))); + } + + #[test] + fn parse_oauth_callback_rejects_wrong_path() { + let parsed = parse_oauth_callback("/callback?code=abc&state=xyz", "/oauth/callback"); + assert!(matches!(parsed, CallbackOutcome::Invalid)); + } + + #[test] + fn callback_path_comes_from_redirect_uri() { + let path = callback_path_from_redirect_uri("https://example.com/oauth/callback") + .expect("redirect URI should parse"); + assert_eq!(path, "/oauth/callback"); + } +}