From cb2c8cc50d84e40b071a8513e9acc4f417412a0a Mon Sep 17 00:00:00 2001 From: betegon Date: Fri, 12 Jun 2026 21:11:26 +0200 Subject: [PATCH] feat(auth): attempt browser login flow in non-TTY The OAuth device flow (which opens the browser) was gated on a stdin TTY check, so an unauthenticated command run in a non-TTY (piped output, redirected stdin, CI) failed immediately instead of trying to authenticate. Attempt the device flow regardless of TTY. It's already TTY-agnostic: openBrowser is best-effort and falls back to printing the verification URL + QR code, and the copy-key listener stays gated on process.stdin.isTTY. If login doesn't complete (e.g. nobody authorizes), a non-TTY re-throws the original auth error so the standard "Not authenticated" message and exit code still surface. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/cli.ts | 77 ++++++++++++++++++++++++++++++------------- test/e2e/auth.test.ts | 18 ++++++++++ 2 files changed, 72 insertions(+), 23 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 9a74aadbc..24770e3f7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -444,41 +444,72 @@ export async function runCli(cliArgs: string[]): Promise { } }; + /** + * Run the OAuth device flow to recover from an auth error, retrying the + * command on success. + * + * On failure the outcome depends on interactivity: an interactive terminal + * exits 1 (the device flow has already reported why), while a non-TTY + * re-throws the original error so its standard message and exit code reach the + * user — matching the behavior before auto-auth was attempted in non-TTY + * contexts. + * + * @param err - Auth error that triggered recovery; re-thrown on non-TTY failure + * @param proceed - Re-runs the inner middleware chain to retry the command + * @param retryArgs - Argv passed to {@link proceed} on retry + */ + async function recoverWithAutoAuth( + err: InstanceType, + proceed: (cmdInput: string[]) => Promise, + retryArgs: string[] + ): Promise { + // Direct fd check; used only to pick the failure behavior on a failed login. + const interactive = isatty(0); + + process.stderr.write( + err.reason === "expired" + ? "Authentication expired. Starting login flow...\n\n" + : "Authentication required. Starting login flow...\n\n" + ); + + const loginSuccess = await runInteractiveLogin(); + if (loginSuccess) { + process.stderr.write("\nRetrying command...\n\n"); + await proceed(retryArgs); + return; + } + + if (interactive) { + process.exitCode = 1; + return; + } + throw err; + } + /** * Auto-authentication middleware. * - * Catches auth errors (not_authenticated, expired) in interactive TTYs - * and runs the login flow. On success, retries through the full middleware - * chain so inner middlewares (e.g., trial prompt) also apply to the retry. + * Catches auth errors (not_authenticated, expired) and runs the OAuth device + * flow via {@link recoverWithAutoAuth}, retrying through the full middleware + * chain on success so inner middlewares (e.g., trial prompt) also apply. + * + * The flow is attempted in non-TTY contexts too (piped output, the + * Bun-compiled-binary `isTTY===undefined` case, CI): the device flow is + * TTY-agnostic — `openBrowser` falls back to printing the verification URL + + * QR code, and the copy-key listener is gated on `process.stdin.isTTY` inside + * `runInteractiveLogin`. Auth commands opt out via `skipAutoAuth` + * (e.g. `auth status`). */ const autoAuthMiddleware: ErrorMiddleware = async (next, argv) => { try { await next(argv); } catch (err) { - // Use isatty(0) for reliable stdin TTY detection (process.stdin.isTTY can be undefined in Bun) - // Errors can opt-out via skipAutoAuth (e.g., auth status command) if ( err instanceof AuthError && (err.reason === "not_authenticated" || err.reason === "expired") && - !err.skipAutoAuth && - isatty(0) + !err.skipAutoAuth ) { - process.stderr.write( - err.reason === "expired" - ? "Authentication expired. Starting login flow...\n\n" - : "Authentication required. Starting login flow...\n\n" - ); - - const loginSuccess = await runInteractiveLogin(); - - if (loginSuccess) { - process.stderr.write("\nRetrying command...\n\n"); - await next(argv); - return; - } - - // Login failed or was cancelled - process.exitCode = 1; + await recoverWithAutoAuth(err, next, argv); return; } diff --git a/test/e2e/auth.test.ts b/test/e2e/auth.test.ts index 14ccefec0..3ad6b2242 100644 --- a/test/e2e/auth.test.ts +++ b/test/e2e/auth.test.ts @@ -76,6 +76,24 @@ describe("sentry auth status", () => { }); }); +describe("non-TTY auto-auth", () => { + // Auth-required commands attempt the OAuth device flow even in a non-TTY + // (the test subprocess has no TTY on stdin). The device-code request fails + // fast against the mock (no /oauth/device/code/ route → 404), so the command + // still exits 10 with the standard not-authenticated message — but only + // after attempting to start the login flow. + test("attempts login flow then exits not-authenticated", async () => { + const result = await ctx.run(["api", "organizations/"]); + + const output = result.stdout + result.stderr; + // The login flow was attempted (gate is no longer TTY-only)... + expect(output).toMatch(/starting login flow/i); + // ...but it failed, so the standard not-authenticated path still wins. + expect(output).toMatch(/not authenticated|login/i); + expect(result.exitCode).toBe(EXIT.AUTH_NOT_AUTHENTICATED); + }); +}); + describe("sentry auth login --token", () => { test("stores valid API token", { timeout: 10_000 }, async () => { const result = await ctx.run([