Skip to content

fix(windows): stop cmd from truncating OAuth browser login URLs (grok, cursor) - #192

Open
Nitjsefnie wants to merge 3 commits into
raine:mainfrom
Nitjsefnie-OSC:fix/106-windows-oauth-url
Open

Nitjsefnie wants to merge 3 commits into
raine:mainfrom
Nitjsefnie-OSC:fix/106-windows-oauth-url

Conversation

@Nitjsefnie

Copy link
Copy Markdown
Contributor

Summary

On Windows, both providers handed the OAuth login URL to cmd /C start unquoted, so cmd split the URL at the first & and the browser got only the first parameter (Grok: "Missing or invalid client_id"). Both providers now launch the URL through rundll32 url.dll,FileProtocolHandler, which passes the complete URL as a single argument and never involves a command interpreter.

Related Issues and Pull Requests

Fixes #106

Changes

  • Grok's open_browser and Cursor's open_cursor_login_url build their Windows command with a small provider-local helper returning rundll32.exe + ["url.dll,FileProtocolHandler", url] — the URL stays one argument, no cmd interpreter in the path
  • Grok's open_browser now propagates spawn errors instead of discarding them; the login flow prints Could not open browser automatically: {err} (Cursor's existing pattern), so a failed launch is visible
  • Each launcher takes its platform and process-execution operation as parameters (production passes std::env::consts::OS and the real Command method); public signatures unchanged, no new dependency
  • Shared test source (tests/support/oauth_browser_launch.rs) included into both providers' lib test modules: tests drive the real private launchers with only process execution intercepted, assert the executable and exact ordered argv for both providers' real URL shapes (including raw & ^ | < > ( ) values), cover the macOS/Linux argument paths, and a #[cfg(windows)] recorder test drives the real launcher's argv through a native subprocess

Testing

  • cargo test --lib oauth_browser_launch: 4 tests green at head 21a4431c340b7015e056c04d2638662d8b08aca7; reverting either launcher's Windows branch to cmd /C start (helper untouched) fails the covering test by assertion, verified separately per provider; on windows-latest the same filter runs 6 tests (the cfg(windows) recorder included), all green
  • Fork CI on that exact head: osc-manual.yml run 39844448273 (just format/clippy/build/test/clean-tree) and osc-adv-windows.yml run 39844444292 (windows-latest, targeted regression tests) — see PR checks for results
  • The shared test source is intentionally included into both providers' test modules; those two items carry a scoped #[allow(clippy::duplicate_mod)] with a justification comment
  • macOS/Linux launch expressions are byte-identical to main; no live browser login was run (no Windows machine with provider credentials available); url.dll's handler does not return browser-open errors (documented by Microsoft), so launch-failure visibility is at the process-spawn level

Footer

Generated by GPT-6.1 Sol (implementation, testing), GPT-6 Astra (review), GLM-5.3-Flash (brief, review)

Nitjsefnie and others added 3 commits October 22, 2026 21:08
Bypass the command interpreter for Grok and Cursor browser launches. Report Grok spawn errors and cover adversarial query parameters in the production Windows argument builders.

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Inject platform selection and process execution into the private launchers so regression tests cover their actual Windows branches. Keep native recorder coverage on those paths and verify unchanged Unix arguments and launch error propagation.

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Scope the duplicate module lint allowance to the shared test includes, which exercise each provider's private launcher.

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@Nitjsefnie

Copy link
Copy Markdown
Contributor Author

Heads-up: this branch lives in an organization-owned fork, and GitHub doesn't let maintainers push to those. If anything needs changing, just say so here and I'll make the change on this branch right away.

@raine

raine commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR

Heads-up: this branch lives in an organization-owned fork, and GitHub doesn't let maintainers push to those. If anything needs changing, just say so here and I'll make the change on this branch right away.

No time to coordinate like this, will rather take the PRs changes and fix them outside it if needed

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: cmd /C start truncates the OAuth URL at the first &, so Grok browser login fails with "Missing or invalid client_id"

2 participants