From 64e57306e5e7740c8bef85e3b8cbef4e643c74a9 Mon Sep 17 00:00:00 2001 From: Michael Fan Date: Wed, 25 Mar 2026 19:47:46 -0400 Subject: [PATCH] codex: restore fixed create-api-key callback port Use the Hydra-registered localhost:5000 redirect URI for project API key creation, but fail fast on port conflicts instead of sending /cancel to a potentially unrelated local service. Validation: - cargo test -p codex-login - just fmt - just fix -p codex-login - just argument-comment-lint Co-authored-by: Codex --- codex-rs/login/src/create_api_key.rs | 6 +++- codex-rs/login/src/oauth_callback_server.rs | 38 +++++++++++++++++++-- codex-rs/login/src/server.rs | 4 ++- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/codex-rs/login/src/create_api_key.rs b/codex-rs/login/src/create_api_key.rs index 3d9daf1f48..5433f4f043 100644 --- a/codex-rs/login/src/create_api_key.rs +++ b/codex-rs/login/src/create_api_key.rs @@ -9,6 +9,7 @@ use serde::Deserialize; use url::Url; use crate::oauth_callback_server::AuthorizationCodeServer; +use crate::oauth_callback_server::PortConflictStrategy; use crate::oauth_callback_server::start_authorization_code_server; use crate::pkce::PkceCodes; @@ -16,7 +17,9 @@ const AUTH_ISSUER: &str = "https://auth.openai.com"; const PLATFORM_HYDRA_CLIENT_ID: &str = "app_2SKx67EdpoN0G6j64rFvigXD"; const PLATFORM_AUDIENCE: &str = "https://api.openai.com/v1"; const API_BASE: &str = "https://api.openai.com"; -const CALLBACK_PORT: u16 = 0; +// This client is registered with Hydra for http://localhost:5000/auth/callback, +// so the browser redirect must stay on port 5000. +const CALLBACK_PORT: u16 = 5000; const CALLBACK_PATH: &str = "/auth/callback"; const SCOPE: &str = "openid email profile offline_access"; const APP: &str = "api"; @@ -104,6 +107,7 @@ pub fn start_create_api_key() -> Result let client = build_http_client()?; let callback_server = start_authorization_code_server( options.callback_port, + PortConflictStrategy::Fail, CALLBACK_PATH, /*force_state*/ None, |redirect_uri, pkce, state| { diff --git a/codex-rs/login/src/oauth_callback_server.rs b/codex-rs/login/src/oauth_callback_server.rs index 4270367898..0fd83279ab 100644 --- a/codex-rs/login/src/oauth_callback_server.rs +++ b/codex-rs/login/src/oauth_callback_server.rs @@ -24,6 +24,15 @@ use tiny_http::StatusCode; use crate::pkce::PkceCodes; use crate::pkce::generate_pkce; +/// Strategy for handling a callback port that is already in use. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PortConflictStrategy { + /// Attempt to cancel a previous callback server on the same port and retry. + CancelPrevious, + /// Return an error immediately without sending any request to the occupied port. + Fail, +} + /// Handle used to signal the callback server loop to exit. #[derive(Clone, Debug)] pub struct ShutdownHandle { @@ -84,6 +93,7 @@ impl AuthorizationCodeServer { pub(crate) fn start_authorization_code_server( port: u16, + port_conflict_strategy: PortConflictStrategy, callback_path: &str, force_state: Option, auth_url_builder: F, @@ -95,7 +105,7 @@ where let state = force_state.unwrap_or_else(generate_state); let callback_path = callback_path.to_string(); - let (server, actual_port, rx) = bind_server_with_request_channel(port)?; + let (server, actual_port, rx) = bind_server_with_request_channel(port, port_conflict_strategy)?; let redirect_uri = format!("http://localhost:{actual_port}{callback_path}"); let auth_url = match auth_url_builder(&redirect_uri, &pkce, &state) { Ok(auth_url) => auth_url, @@ -139,8 +149,9 @@ pub(crate) enum HandledRequest { pub(crate) fn bind_server_with_request_channel( port: u16, + port_conflict_strategy: PortConflictStrategy, ) -> io::Result<(Arc, u16, tokio::sync::mpsc::Receiver)> { - let server = bind_server(port)?; + let server = bind_server(port, port_conflict_strategy)?; let actual_port = match server.server_addr().to_ip() { Some(addr) => addr.port(), None => { @@ -311,7 +322,7 @@ fn send_cancel_request(port: u16) -> io::Result<()> { Ok(()) } -fn bind_server(port: u16) -> io::Result { +fn bind_server(port: u16, port_conflict_strategy: PortConflictStrategy) -> io::Result { let bind_address = format!("127.0.0.1:{port}"); let mut cancel_attempted = false; let mut attempts = 0; @@ -331,6 +342,13 @@ fn bind_server(port: u16) -> io::Result { // If the address is in use, there is probably another instance of the callback // server running. Attempt to cancel it and retry. if is_addr_in_use { + if port_conflict_strategy == PortConflictStrategy::Fail { + return Err(io::Error::new( + io::ErrorKind::AddrInUse, + format!("Port {bind_address} is already in use"), + )); + } + if !cancel_attempted { cancel_attempted = true; if let Err(cancel_err) = send_cancel_request(port) { @@ -454,6 +472,20 @@ fn authorization_code_error_message(error_code: &str, error_description: Option< mod tests { use super::*; + #[test] + fn bind_server_fails_without_canceling_when_port_conflict_strategy_is_fail() { + let listener = + std::net::TcpListener::bind("127.0.0.1:0").expect("bind ephemeral test listener"); + let port = listener.local_addr().expect("read local addr").port(); + + let error = match bind_server(port, PortConflictStrategy::Fail) { + Ok(_) => panic!("expected occupied port to fail immediately"), + Err(error) => error, + }; + + assert_eq!(error.kind(), io::ErrorKind::AddrInUse); + } + #[test] fn process_authorization_code_request_keeps_server_running_on_state_mismatch() { let response = process_authorization_code_request( diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 0cf05b25d0..a5221ed198 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -36,6 +36,7 @@ use tracing::info; use tracing::warn; use crate::oauth_callback_server::HandledRequest; +use crate::oauth_callback_server::PortConflictStrategy; use crate::oauth_callback_server::ShutdownHandle; use crate::oauth_callback_server::bind_server_with_request_channel; use crate::oauth_callback_server::generate_state; @@ -110,7 +111,8 @@ pub fn run_login_server(opts: ServerOptions) -> io::Result { let pkce = generate_pkce(); let state = opts.force_state.clone().unwrap_or_else(generate_state); - let (server, actual_port, rx) = bind_server_with_request_channel(opts.port)?; + let (server, actual_port, rx) = + bind_server_with_request_channel(opts.port, PortConflictStrategy::CancelPrevious)?; let redirect_uri = format!("http://localhost:{actual_port}/auth/callback"); let auth_url = build_authorize_url( &opts.issuer,