diff --git a/src/providers/cursor/auth.rs b/src/providers/cursor/auth.rs index bcbbdc5b..8ec56ea9 100644 --- a/src/providers/cursor/auth.rs +++ b/src/providers/cursor/auth.rs @@ -196,7 +196,11 @@ pub fn run_cursor_login() -> anyhow::Result> { println!("Open this URL to authenticate with Cursor:"); println!("{}", login.login_url); println!(); - if let Err(err) = open_cursor_login_url(&login.login_url) { + if let Err(err) = open_cursor_login_url( + &login.login_url, + std::env::consts::OS, + std::process::Command::status, + ) { println!("Could not open browser automatically: {err}"); } println!("Waiting for Cursor login..."); @@ -337,22 +341,39 @@ fn parse_jwt_claims(token: &str) -> Option { serde_json::from_slice(&decoded).ok() } -fn open_cursor_login_url(url: &str) -> anyhow::Result<()> { - let status = if cfg!(target_os = "macos") { - std::process::Command::new("open").arg(url).status()? - } else if cfg!(target_os = "windows") { - std::process::Command::new("cmd") - .args(["/c", "start", "", url]) - .status()? +fn open_cursor_login_url( + url: &str, + platform: &str, + execute: impl FnOnce(&mut std::process::Command) -> std::io::Result, +) -> anyhow::Result<()> { + let command = if platform == "macos" { + ("open", vec![url]) + } else if platform == "windows" { + windows_browser_command(url) } else { - std::process::Command::new("xdg-open").arg(url).status()? + ("xdg-open", vec![url]) }; + let status = execute(std::process::Command::new(command.0).args(command.1))?; if !status.success() { anyhow::bail!("open command exited with {status}"); } Ok(()) } +fn windows_browser_command(url: &str) -> (&str, Vec<&str>) { + // Keep URL metacharacters out of a command interpreter. + ("rundll32.exe", vec!["url.dll,FileProtocolHandler", url]) +} + +#[cfg(test)] +use open_cursor_login_url as open_login_url; + +// Run the shared assertions against each provider's private launcher. +#[cfg(test)] +#[allow(clippy::duplicate_mod)] +#[path = "../../../tests/support/oauth_browser_launch.rs"] +mod browser_launch_tests; + fn random_base64_url(len: usize) -> String { let mut bytes = vec![0u8; len]; rand::thread_rng().fill_bytes(&mut bytes); diff --git a/src/providers/grok/auth/login.rs b/src/providers/grok/auth/login.rs index b7b7e74c..a8efe5e6 100644 --- a/src/providers/grok/auth/login.rs +++ b/src/providers/grok/auth/login.rs @@ -47,7 +47,13 @@ pub fn login>(store: &GrokTokenStore) -> anyhow::R let auth_url = authorize_url(&discovery, &redirect_uri, &pkce, &state)?; println!("Open this URL in your browser to authorize:\n\n {auth_url}\n"); - open_browser(&auth_url); + if let Err(err) = open_browser( + &auth_url, + std::env::consts::OS, + std::process::Command::spawn, + ) { + println!("Could not open browser automatically: {err}"); + } let code = wait_for_callback(&listener, &state, LOGIN_TIMEOUT)?; let tokens = exchange_code(&client, &discovery, &code, &pkce, &redirect_uri)?; validate_tokens(&tokens)?; @@ -280,21 +286,35 @@ fn respond(stream: &mut TcpStream, status: &str, body: &str) { let _ = stream.write_all(response.as_bytes()); } -fn open_browser(url: &str) { - #[cfg(target_os = "macos")] - let command = ("open", vec![url]); - #[cfg(target_os = "linux")] - let command = ("xdg-open", vec![url]); - #[cfg(target_os = "windows")] - let command = ("cmd", vec!["/C", "start", "", url]); - #[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] - { - let _ = std::process::Command::new(command.0) - .args(command.1) - .spawn(); - } +fn open_browser( + url: &str, + platform: &str, + execute: impl FnOnce(&mut std::process::Command) -> std::io::Result, +) -> anyhow::Result<()> { + let command = match platform { + "macos" => ("open", vec![url]), + "linux" => ("xdg-open", vec![url]), + "windows" => windows_browser_command(url), + _ => return Ok(()), + }; + execute(std::process::Command::new(command.0).args(command.1))?; + Ok(()) } +fn windows_browser_command(url: &str) -> (&str, Vec<&str>) { + // Keep URL metacharacters out of a command interpreter. + ("rundll32.exe", vec!["url.dll,FileProtocolHandler", url]) +} + +#[cfg(test)] +use open_browser as open_login_url; + +// Run the shared assertions against each provider's private launcher. +#[cfg(test)] +#[allow(clippy::duplicate_mod)] +#[path = "../../../../tests/support/oauth_browser_launch.rs"] +mod browser_launch_tests; + fn now_ms() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/tests/support/oauth_browser_launch.rs b/tests/support/oauth_browser_launch.rs new file mode 100644 index 00000000..f90a3c4f --- /dev/null +++ b/tests/support/oauth_browser_launch.rs @@ -0,0 +1,95 @@ +use super::open_login_url; + +const LOGIN_URLS: [&str; 2] = [ + "https://auth.x.ai/oauth2/authorize?response_type=code&client_id=fixture-client&redirect_uri=http%3A%2F%2F127.0.0.1%3A12345%2Fcallback&scope=openid+profile&code_challenge=ab^cd|ef(gh)&code_challenge_method=S256&state=a%26b", + "https://cursor.com/loginDeepControl?challenge=ab^cd|ef(gh)&uuid=12345678-abcd-4321-abcd-123456789abc&mode=login&redirectTarget=cli%3A%2F%2Fcallback", +]; + +#[test] +fn oauth_browser_launch_keeps_urls_out_of_cmd() { + for url in LOGIN_URLS { + let error = open_login_url(url, "windows", |command| { + assert_eq!( + command.get_program(), + "rundll32.exe", + "OAuth URLs must bypass cmd" + ); + assert_eq!( + command.get_args().collect::>(), + ["url.dll,FileProtocolHandler", url] + ); + Err(std::io::Error::other("browser launch intercepted")) + }) + .unwrap_err(); + assert_eq!(error.to_string(), "browser launch intercepted"); + } +} + +#[test] +fn oauth_browser_launch_preserves_unix_arguments() { + for (platform, program) in [("macos", "open"), ("linux", "xdg-open")] { + for url in LOGIN_URLS { + let error = open_login_url(url, platform, |command| { + assert_eq!(command.get_program(), program); + assert_eq!(command.get_args().collect::>(), [url]); + Err(std::io::Error::other("browser launch intercepted")) + }) + .unwrap_err(); + assert_eq!(error.to_string(), "browser launch intercepted"); + } + } +} + +#[cfg(windows)] +#[test] +fn oauth_browser_launch_roundtrips_through_native_process() { + // Substitute a native argv recorder for rundll32 so no browser is opened. + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("argv.rs"); + let recorder = temp.path().join("argv.exe"); + std::fs::write( + &source, + r#"fn main() { + for arg in std::env::args().skip(1) { + println!("{arg}"); + } + }"#, + ) + .unwrap(); + let compile = std::process::Command::new("rustc") + .arg(&source) + .arg("-o") + .arg(&recorder) + .output() + .unwrap(); + assert!( + compile.status.success(), + "argv recorder did not compile: {}", + String::from_utf8_lossy(&compile.stderr) + ); + for url in LOGIN_URLS { + let error = open_login_url(url, "windows", |command| { + assert_eq!( + command.get_program(), + "rundll32.exe", + "OAuth URLs must bypass cmd" + ); + assert_eq!( + command.get_args().collect::>(), + ["url.dll,FileProtocolHandler", url] + ); + let output = std::process::Command::new(&recorder) + .args(command.get_args()) + .output()?; + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert_eq!( + stdout.lines().collect::>(), + ["url.dll,FileProtocolHandler", url] + ); + Err(std::io::Error::other("browser launch intercepted")) + }) + .unwrap_err(); + assert_eq!(error.to_string(), "browser launch intercepted"); + } +}