From 0d32239389a1e7003a64df4533933cabbd80d99b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 31 Mar 2026 16:58:58 +0200 Subject: [PATCH 1/7] feat(auth): enforce auth by default in buildCommand (#611) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary All commands now require authentication by default. `buildCommand` checks `isAuthenticated()` before invoking the command function and throws `AuthError("not_authenticated")` if no token exists — this feeds into the existing auto-auth middleware that prompts login and retries. Previously each command had to manually guard auth or rely on an API call failing deep in execution. `sentry init` in particular had no auth check at all and would proceed into the wizard flow before eventually failing. ## Changes Added `auth?: boolean` to `buildCommand` options (defaults `true`). The check runs before the generator, so the auto-auth middleware in `cli.ts` catches it and kicks off the interactive login flow on unauthenticated runs. Commands that intentionally work without a token opt out with `auth: false`: - `auth login`, `auth logout`, `auth refresh`, `auth status` - `help`, `schema` - `cli fix`, `cli setup`, `cli upgrade`, `cli feedback` With this in place, the redundant explicit `isAuthenticated()` checks in `auth whoami` and `auth token` were removed. ## Test Plan - `sentry init` (unauthenticated) → should immediately prompt "Authentication required. Starting login flow..." - `sentry auth login` (unauthenticated) → should work normally - `sentry auth status` (unauthenticated) → should show "not authenticated" cleanly, no login prompt - `sentry auth logout` (unauthenticated) → should return "Not currently authenticated." - `SENTRY_AUTH_TOKEN=xxx sentry init` → no auth prompt, proceeds normally --------- Co-authored-by: Claude Sonnet 4.6 --- src/commands/auth/login.ts | 1 + src/commands/auth/logout.ts | 1 + src/commands/auth/refresh.ts | 1 + src/commands/auth/status.ts | 1 + src/commands/auth/token.ts | 1 - src/commands/auth/whoami.ts | 6 ------ src/commands/cli/feedback.ts | 1 + src/commands/cli/fix.ts | 1 + src/commands/cli/setup.ts | 1 + src/commands/cli/upgrade.ts | 1 + src/commands/help.ts | 1 + src/commands/schema.ts | 1 + src/lib/command.ts | 30 +++++++++++++++++++++++++++++- src/lib/list-command.ts | 1 + 14 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index 68c3cc350..fc108515d 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -107,6 +107,7 @@ async function handleExistingAuth(force: boolean): Promise { } export const loginCommand = buildCommand({ + auth: false, docs: { brief: "Authenticate with Sentry", fullDescription: diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index 5a4588ff8..669be0ec6 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -28,6 +28,7 @@ export type LogoutResult = { }; export const logoutCommand = buildCommand({ + auth: false, docs: { brief: "Log out of Sentry", fullDescription: diff --git a/src/commands/auth/refresh.ts b/src/commands/auth/refresh.ts index 73d7127a2..cee651b5c 100644 --- a/src/commands/auth/refresh.ts +++ b/src/commands/auth/refresh.ts @@ -39,6 +39,7 @@ function formatRefreshResult(data: RefreshOutput): string { } export const refreshCommand = buildCommand({ + auth: false, docs: { brief: "Refresh your authentication token", fullDescription: ` diff --git a/src/commands/auth/status.ts b/src/commands/auth/status.ts index 2f6de80b5..3592103ee 100644 --- a/src/commands/auth/status.ts +++ b/src/commands/auth/status.ts @@ -138,6 +138,7 @@ async function verifyCredentials(): Promise { } export const statusCommand = buildCommand({ + auth: false, docs: { brief: "View authentication status", fullDescription: diff --git a/src/commands/auth/token.ts b/src/commands/auth/token.ts index 190b6cf7a..04d8090d8 100644 --- a/src/commands/auth/token.ts +++ b/src/commands/auth/token.ts @@ -27,7 +27,6 @@ export const tokenCommand = buildCommand({ if (!token) { throw new AuthError("not_authenticated"); } - return yield new CommandOutput(token); }, }); diff --git a/src/commands/auth/whoami.ts b/src/commands/auth/whoami.ts index b7ae06d87..2051b64e0 100644 --- a/src/commands/auth/whoami.ts +++ b/src/commands/auth/whoami.ts @@ -9,9 +9,7 @@ import type { SentryContext } from "../../context.js"; import { getCurrentUser } from "../../lib/api-client.js"; import { buildCommand } from "../../lib/command.js"; -import { isAuthenticated } from "../../lib/db/auth.js"; import { setUserInfo } from "../../lib/db/user.js"; -import { AuthError } from "../../lib/errors.js"; import { formatUserIdentity } from "../../lib/formatters/index.js"; import { CommandOutput } from "../../lib/formatters/output.js"; import { @@ -46,10 +44,6 @@ export const whoamiCommand = buildCommand({ async *func(this: SentryContext, flags: WhoamiFlags) { applyFreshFlag(flags); - if (!isAuthenticated()) { - throw new AuthError("not_authenticated"); - } - const user = await getCurrentUser(); // Keep cached user info up to date. Non-fatal: display must succeed even diff --git a/src/commands/cli/feedback.ts b/src/commands/cli/feedback.ts index 55d115f33..90e7a6d5e 100644 --- a/src/commands/cli/feedback.ts +++ b/src/commands/cli/feedback.ts @@ -25,6 +25,7 @@ export type FeedbackResult = { }; export const feedbackCommand = buildCommand({ + auth: false, docs: { brief: "Send feedback about the CLI", fullDescription: diff --git a/src/commands/cli/fix.ts b/src/commands/cli/fix.ts index e76ca32b5..99d8ed008 100644 --- a/src/commands/cli/fix.ts +++ b/src/commands/cli/fix.ts @@ -650,6 +650,7 @@ function safeHandleSchemaIssues( } export const fixCommand = buildCommand({ + auth: false, docs: { brief: "Diagnose and repair CLI database issues", fullDescription: diff --git a/src/commands/cli/setup.ts b/src/commands/cli/setup.ts index c08f156de..5e264d5b2 100644 --- a/src/commands/cli/setup.ts +++ b/src/commands/cli/setup.ts @@ -428,6 +428,7 @@ async function runConfigurationSteps(opts: ConfigStepOptions) { } export const setupCommand = buildCommand({ + auth: false, docs: { brief: "Configure shell integration", fullDescription: diff --git a/src/commands/cli/upgrade.ts b/src/commands/cli/upgrade.ts index 98e9a2efb..f046b9ff8 100644 --- a/src/commands/cli/upgrade.ts +++ b/src/commands/cli/upgrade.ts @@ -643,6 +643,7 @@ async function buildCheckResultWithChangelog(opts: { } export const upgradeCommand = buildCommand({ + auth: false, docs: { brief: "Update the Sentry CLI to the latest version", fullDescription: diff --git a/src/commands/help.ts b/src/commands/help.ts index 3330df59b..bdf9c8efa 100644 --- a/src/commands/help.ts +++ b/src/commands/help.ts @@ -20,6 +20,7 @@ import { } from "../lib/help.js"; export const helpCommand = buildCommand({ + auth: false, docs: { brief: "Display help for a command", fullDescription: diff --git a/src/commands/schema.ts b/src/commands/schema.ts index c47b0951e..93a1f57fe 100644 --- a/src/commands/schema.ts +++ b/src/commands/schema.ts @@ -260,6 +260,7 @@ type SchemaFlags = { }; export const schemaCommand = buildCommand({ + auth: false, docs: { brief: "Browse the Sentry API schema", fullDescription: diff --git a/src/lib/command.ts b/src/lib/command.ts index cc62d83c4..95e59b890 100644 --- a/src/lib/command.ts +++ b/src/lib/command.ts @@ -37,7 +37,8 @@ import { numberParser as stricliNumberParser, } from "@stricli/core"; import type { Writer } from "../types/index.js"; -import { CliError, OutputError } from "./errors.js"; +import { getAuthConfig } from "./db/auth.js"; +import { AuthError, CliError, OutputError } from "./errors.js"; import { warning } from "./formatters/colors.js"; import { parseFieldsList } from "./formatters/json.js"; import { @@ -150,6 +151,19 @@ type LocalCommandBuilderArguments< */ // biome-ignore lint/suspicious/noExplicitAny: Variance erasure — OutputConfig.human is contravariant in T, but the builder erases T because it doesn't know the output type. Using `any` allows commands to declare OutputConfig while the wrapper handles it generically. readonly output?: OutputConfig; + /** + * Whether the command requires authentication. Defaults to `true`. + * + * When `true` (the default), the command throws `AuthError("not_authenticated")` + * before executing if no credentials exist at all (no token or refresh token + * in the DB or env vars). Expired tokens with a valid refresh token pass the + * guard — the API client handles silent refresh. The auto-auth middleware in + * `cli.ts` catches the error and triggers the login flow. + * + * Set to `false` for commands that intentionally work without a token + * (e.g. `auth login`, `auth logout`, `auth status`, `help`, `cli upgrade`). + */ + readonly auth?: boolean; }; // --------------------------------------------------------------------------- @@ -252,6 +266,10 @@ export function applyLoggingFlags( * 4. Captures flag values and positional arguments as Sentry telemetry context * 5. When `output` has an {@link OutputConfig}, injects `--json` and `--fields` * flags, pre-parses `--fields`, and auto-renders the command's `{ data }` return + * 6. Enforces authentication by default — throws `AuthError("not_authenticated")` + * before the command runs if no credentials exist at all (expired tokens with + * a refresh token pass through so the API client can silently refresh). Opt out with `auth: false` + * for commands that intentionally work without a token (e.g. `auth login`, `help`) * * When a command already defines its own `verbose` flag (e.g. the `api` command * uses `--verbose` for HTTP request/response output), the injected `VERBOSE_FLAG` @@ -324,6 +342,7 @@ export function buildCommand< ): Command { const originalFunc = builderArgs.func; const outputConfig = builderArgs.output; + const requiresAuth = builderArgs.auth !== false; // Merge logging flags into the command's flag definitions. // Quoted keys produce kebab-case CLI flags: "log-level" → --log-level @@ -557,7 +576,16 @@ export function buildCommand< // Iterate the generator using manual .next() instead of for-await-of // so we can capture the return value (done: true result). The return // value carries the final `hint` — for-await-of discards it. + // + // Auth guard is inside the try block so that maybeRecoverWithHelp can + // intercept the AuthError when "help" appears as a positional arg (e.g. + // `sentry issue list help`). Without this, the auth prompt would fire + // before the help-recovery path could show the command's help text. try { + if (requiresAuth && !getAuthConfig()) { + throw new AuthError("not_authenticated"); + } + const generator = originalFunc.call( this, cleanFlags as FLAGS, diff --git a/src/lib/list-command.ts b/src/lib/list-command.ts index d814ed97c..5ccfbbbe3 100644 --- a/src/lib/list-command.ts +++ b/src/lib/list-command.ts @@ -464,6 +464,7 @@ export function buildListCommand< readonly func: ListCommandFunction; // biome-ignore lint/suspicious/noExplicitAny: OutputConfig is generic but type is erased at the builder level readonly output?: OutputConfig; + readonly auth?: boolean; }, options?: ListCommandOptions ): Command { From fc146ae1c16c6b3d04ca6524c7f373e7b567a6ff Mon Sep 17 00:00:00 2001 From: mathuraditya724 Date: Tue, 31 Mar 2026 21:31:47 +0530 Subject: [PATCH 2/7] fix(test): adapt tests for buildCommand auth guard (#611) The auth guard added in #611 calls getAuthConfig() before every command, which broke ~200 tests that had no auth token in the test environment. - Set a fake SENTRY_AUTH_TOKEN in test preload so the auth guard passes globally (real API calls are blocked by the global fetch mock) - Add auth: false to command.test.ts test commands (they test framework behavior, not authentication) - Clear the env token in tests that specifically verify unauthenticated or SENTRY_TOKEN-priority behavior (logout, refresh, whoami, project list) --- test/commands/auth/logout.test.ts | 6 +++++ test/commands/auth/refresh.test.ts | 6 +++++ test/commands/auth/whoami.test.ts | 20 ++++++++++++++++ test/commands/project/list.test.ts | 38 ++++++++++++++++++++++-------- test/lib/command.test.ts | 29 +++++++++++++++++++++++ test/preload.ts | 6 +++++ 6 files changed, 95 insertions(+), 10 deletions(-) diff --git a/test/commands/auth/logout.test.ts b/test/commands/auth/logout.test.ts index 8953cb448..8bf8282a9 100644 --- a/test/commands/auth/logout.test.ts +++ b/test/commands/auth/logout.test.ts @@ -126,6 +126,9 @@ describe("logoutCommand.func", () => { isAuthenticatedSpy.mockReturnValue(true); isEnvTokenActiveSpy.mockReturnValue(true); // Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken() + // Clear SENTRY_AUTH_TOKEN so SENTRY_TOKEN takes priority + const savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; process.env.SENTRY_TOKEN = "sntrys_token_456"; const { context } = createContext(); @@ -138,6 +141,9 @@ describe("logoutCommand.func", () => { expect(msg).toContain("SENTRY_TOKEN"); } finally { delete process.env.SENTRY_TOKEN; + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } } expect(clearAuthSpy).not.toHaveBeenCalled(); }); diff --git a/test/commands/auth/refresh.test.ts b/test/commands/auth/refresh.test.ts index 94c786a26..9a744ef4e 100644 --- a/test/commands/auth/refresh.test.ts +++ b/test/commands/auth/refresh.test.ts @@ -85,6 +85,9 @@ describe("refreshCommand.func", () => { test("env token (SENTRY_TOKEN): throws AuthError with SENTRY_TOKEN in message", async () => { isEnvTokenActiveSpy.mockReturnValue(true); // Set env var directly — getActiveEnvVarName() reads env vars via getEnvToken() + // Clear SENTRY_AUTH_TOKEN so SENTRY_TOKEN takes priority + const savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; process.env.SENTRY_TOKEN = "sntrys_token_456"; const { context } = createContext(); @@ -99,6 +102,9 @@ describe("refreshCommand.func", () => { expect((err as AuthError).message).not.toContain("SENTRY_AUTH_TOKEN"); } finally { delete process.env.SENTRY_TOKEN; + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } } expect(refreshTokenSpy).not.toHaveBeenCalled(); diff --git a/test/commands/auth/whoami.test.ts b/test/commands/auth/whoami.test.ts index dd44e52dc..0ed717736 100644 --- a/test/commands/auth/whoami.test.ts +++ b/test/commands/auth/whoami.test.ts @@ -84,6 +84,26 @@ describe("whoamiCommand.func", () => { }); describe("unauthenticated", () => { + let getAuthConfigSpy: ReturnType; + let savedAuthToken: string | undefined; + + beforeEach(() => { + // Clear env token and mock getAuthConfig so buildCommand's auth guard + // sees no credentials — this tests the unauthenticated path end-to-end. + savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; + getAuthConfigSpy = spyOn(dbAuth, "getAuthConfig").mockReturnValue( + undefined + ); + }); + + afterEach(() => { + getAuthConfigSpy.mockRestore(); + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } + }); + test("throws AuthError(not_authenticated) when no token stored", async () => { isAuthenticatedSpy.mockReturnValue(false); diff --git a/test/commands/project/list.test.ts b/test/commands/project/list.test.ts index 870f45ae9..dbc7b9976 100644 --- a/test/commands/project/list.test.ts +++ b/test/commands/project/list.test.ts @@ -969,8 +969,17 @@ describe("fetchOrgProjectsSafe", () => { test("propagates AuthError when not authenticated", async () => { // Clear auth token so the API client throws AuthError before making any request await clearAuth(); - - await expect(fetchOrgProjectsSafe("myorg")).rejects.toThrow(AuthError); + // Also clear env token — preload sets a fake one for the auth guard + const savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; + + try { + await expect(fetchOrgProjectsSafe("myorg")).rejects.toThrow(AuthError); + } finally { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } + } }); }); @@ -1233,14 +1242,23 @@ describe("handleAutoDetect", () => { setDefaults("test-org"); // Clear auth so getAuthToken() throws AuthError before any fetch await clearAuth(); - - await expect( - handleAutoDetect("/tmp/test-project", { - limit: 30, - json: true, - fresh: false, - }) - ).rejects.toThrow(AuthError); + // Also clear env token — preload sets a fake one for the auth guard + const savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; + + try { + await expect( + handleAutoDetect("/tmp/test-project", { + limit: 30, + json: true, + fresh: false, + }) + ).rejects.toThrow(AuthError); + } finally { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } + } }); test("slow path: uses full fetch when platform filter is active", async () => { diff --git a/test/lib/command.test.ts b/test/lib/command.test.ts index e0883d380..2495186c3 100644 --- a/test/lib/command.test.ts +++ b/test/lib/command.test.ts @@ -76,6 +76,7 @@ describe("buildCommand", () => { test("builds a valid command object", () => { const command = buildCommand({ docs: { brief: "Test command" }, + auth: false, parameters: { flags: { verbose: { kind: "boolean", brief: "Verbose", default: false }, @@ -91,6 +92,7 @@ describe("buildCommand", () => { test("handles commands with empty parameters", () => { const command = buildCommand({ docs: { brief: "Simple command" }, + auth: false, parameters: {}, async *func() { // no-op @@ -128,6 +130,7 @@ describe("buildCommand telemetry integration", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { verbose: { kind: "boolean", brief: "Verbose", default: false }, @@ -168,6 +171,7 @@ describe("buildCommand telemetry integration", () => { test("skips false boolean flags in telemetry", async () => { const command = buildCommand<{ json: boolean }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { json: { kind: "boolean", brief: "JSON output", default: false }, @@ -199,6 +203,7 @@ describe("buildCommand telemetry integration", () => { const command = buildCommand, [string], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: { positional: { kind: "tuple", @@ -236,6 +241,7 @@ describe("buildCommand telemetry integration", () => { const command = buildCommand, [], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: {}, // biome-ignore lint/correctness/useYield: test command — no output to yield async *func(this: TestContext) { @@ -261,6 +267,7 @@ describe("buildCommand telemetry integration", () => { const command = buildCommand<{ delay: number }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { delay: { @@ -371,6 +378,7 @@ describe("buildCommand", () => { test("builds a valid command object", () => { const command = buildCommand({ docs: { brief: "Test command" }, + auth: false, parameters: { flags: { json: { kind: "boolean", brief: "JSON output", default: false }, @@ -388,6 +396,7 @@ describe("buildCommand", () => { const command = buildCommand<{ json: boolean }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { json: { kind: "boolean", brief: "JSON output", default: false }, @@ -499,6 +508,7 @@ describe("buildCommand", () => { const command = buildCommand<{ limit: number }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { limit: { @@ -547,6 +557,7 @@ describe("buildCommand", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { verbose: { @@ -603,6 +614,7 @@ describe("buildCommand", () => { try { const command = buildCommand<{ verbose: boolean }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, parameters: { flags: { verbose: { @@ -674,6 +686,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: { flags: { @@ -717,6 +730,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: {}, // biome-ignore lint/correctness/useYield: test command — no output to yield @@ -756,6 +770,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: {}, // biome-ignore lint/correctness/useYield: test command — no output to yield @@ -794,6 +809,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: {}, // biome-ignore lint/correctness/useYield: test command — no output to yield @@ -858,6 +874,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: { flags: { @@ -901,6 +918,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: {}, // biome-ignore lint/correctness/useYield: test command — no output to yield @@ -942,6 +960,7 @@ describe("buildCommand output config", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused" }, parameters: { flags: { @@ -996,6 +1015,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { name: string; role: string }) => `${d.name} (${d.role})`, }, @@ -1024,6 +1044,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { name: string; role: string }) => `${d.name} (${d.role})`, }, @@ -1053,6 +1074,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { id: number; name: string; role: string }) => `${d.name}`, }, @@ -1084,6 +1106,7 @@ describe("buildCommand return-based output", () => { const makeCommand = () => buildCommand<{ json: boolean; fields?: string[] }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { value: number }) => `Value: ${d.value}`, }, @@ -1131,6 +1154,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: () => "unused", }, @@ -1188,6 +1212,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { name: string }) => `Hello, ${d.name}!`, }, @@ -1218,6 +1243,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: Array<{ id: number }>) => d.map(((x) => x.id).join(", ")), }, @@ -1247,6 +1273,7 @@ describe("buildCommand return-based output", () => { const makeCommand = () => buildCommand<{ json: boolean; fields?: string[] }, [], TestContext>({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { org: string }) => `Org: ${d.org}`, }, @@ -1285,6 +1312,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { error: string }) => `Error: ${d.error}`, }, @@ -1333,6 +1361,7 @@ describe("buildCommand return-based output", () => { TestContext >({ docs: { brief: "Test" }, + auth: false, output: { human: (d: { error: string }) => `Error: ${d.error}`, }, diff --git a/test/preload.ts b/test/preload.ts index e9aaa9dca..7d77e7d86 100644 --- a/test/preload.ts +++ b/test/preload.ts @@ -93,6 +93,12 @@ delete process.env.SENTRY_HOST; delete process.env.SENTRY_ORG; delete process.env.SENTRY_PROJECT; +// Set a fake auth token so buildCommand's auth guard passes in tests. +// Real API calls are blocked by the global fetch mock below. +// Tests that specifically verify unauthenticated behavior (e.g., auth status) +// mock getAuthConfig to return undefined. +process.env.SENTRY_AUTH_TOKEN = "sntrys_test-token-for-unit-tests_000000"; + // Disable telemetry and background update checks in tests // This prevents Sentry SDK from keeping the process alive and making external calls process.env.SENTRY_CLI_NO_TELEMETRY = "1"; From 275ca890cbda5e6c3549746225bdfa1554668db4 Mon Sep 17 00:00:00 2001 From: mathuraditya724 Date: Tue, 31 Mar 2026 22:10:48 +0530 Subject: [PATCH 3/7] fix(telemetry): clean up SDK beforeExit handlers on re-init Sentry.init() registers internal beforeExit handlers (ProcessSession, client report flusher, log flusher) and a setInterval for client reports. When initSentry is called multiple times (e.g., in tests), these handlers accumulate and keep the event loop alive, preventing the Bun test runner from exiting. Fix: snapshot beforeExit listeners before Sentry.init() and remove any new ones added by the SDK afterward. Also call client.close(0) on the previous client to clear its internal setInterval. We manage our own beforeExit handler explicitly for session lifecycle. --- src/lib/telemetry.ts | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 340042a21..101e21690 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -355,6 +355,22 @@ export function initSentry( const libraryMode = options?.libraryMode ?? false; const environment = getEnv().NODE_ENV ?? "development"; + // Close the previous client to clean up its internal timers and beforeExit + // handlers (client report flusher interval, log flush listener). Without + // this, re-initializing the SDK (e.g., in tests) leaks setInterval handles + // that keep the event loop alive and prevent the process from exiting. + const previousClient = Sentry.getClient(); + if (previousClient) { + previousClient.close(0); + } + + // Snapshot beforeExit listeners so we can detect any new ones added by + // Sentry.init() — particularly the anonymous handler from the + // ProcessSession integration, which registers via setupOnce and has no + // cleanup mechanism. We remove SDK-added listeners after init and manage + // our own beforeExit handler explicitly (see below). + const listenersBefore = new Set(process.rawListeners("beforeExit")); + const client = Sentry.init({ dsn: SENTRY_CLI_DSN, enabled, @@ -405,12 +421,23 @@ export function initSentry( }, }); - // Always remove the previous handler on re-init. The removal must happen - // unconditionally — not only when enabled=true — so that calling - // initSentry(false) to disable telemetry (e.g. in test afterEach) actually - // cleans up the handler registered by a prior initSentry(true) call. - // Without this, the stale handler keeps the event loop alive after all tests - // finish, preventing the process from exiting. + // Remove all beforeExit listeners added by Sentry.init(). This covers: + // - ProcessSession integration (anonymous handler, setupOnce, no cleanup) + // - Client report flusher (_clientReportOnExitFlushListener) + // - Log flusher (_logOnExitFlushListener) + // We manage our own beforeExit handler explicitly below and don't need + // the SDK's handlers — they keep the event loop alive in tests and on + // re-initialization (e.g., auto-auth retry). + for (const listener of process.rawListeners("beforeExit")) { + if (!listenersBefore.has(listener)) { + process.removeListener( + "beforeExit", + listener as (...args: unknown[]) => void + ); + } + } + + // Always remove our own previous handler on re-init. if (currentBeforeExitHandler) { process.removeListener("beforeExit", currentBeforeExitHandler); currentBeforeExitHandler = null; From 9bf574d29fa66ea29b4cd3a1a27e8d4c5a7a873f Mon Sep 17 00:00:00 2001 From: mathuraditya724 Date: Tue, 31 Mar 2026 22:34:22 +0530 Subject: [PATCH 4/7] fix(telemetry): use module-level snapshot to catch ProcessSession handler The ProcessSession integration registers its beforeExit handler via setupOnce, meaning it only fires on the first Sentry.init() call and persists across all subsequent re-inits. The previous per-call snapshot missed it on re-init because it was already in the snapshot. Fix: capture the pre-SDK listener set once (before the first initSentry call) and use it for all subsequent removals. This ensures the ProcessSession handler is always identified as SDK-owned and removed. --- src/lib/telemetry.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 62ef0842c..f5fe40c33 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -304,6 +304,13 @@ const hasGetSystemErrorMap = (() => { /** Current beforeExit handler, tracked so it can be replaced on re-init */ let currentBeforeExitHandler: (() => void) | null = null; +/** + * Listeners that existed on `process.beforeExit` before the first `initSentry` + * call. Used to distinguish SDK-added handlers from application/test handlers. + */ +// biome-ignore lint/complexity/noBannedTypes: rawListeners returns Function[] +let preInitListeners: Set | null = null; + /** Match all SaaS regional URLs (us.sentry.io, de.sentry.io, o1234.ingest.us.sentry.io, etc.) */ const SENTRY_SAAS_SUBDOMAIN_RE = /^https:\/\/[^/]*\.sentry\.io(\/|$)/; @@ -362,12 +369,14 @@ export function initSentry( // close(0) removes listeners synchronously; we don't need to await the flush. Sentry.getClient()?.close(0); - // Snapshot beforeExit listeners so we can detect any new ones added by - // Sentry.init() — particularly the anonymous handler from the - // ProcessSession integration, which registers via setupOnce and has no - // cleanup mechanism. We remove SDK-added listeners after init and manage - // our own beforeExit handler explicitly (see below). - const listenersBefore = new Set(process.rawListeners("beforeExit")); + // Snapshot beforeExit listeners that existed before the first initSentry call. + // These are non-SDK listeners (e.g., from the app or test framework) that must + // be preserved. All others — including the ProcessSession handler (registered + // via setupOnce on the first Sentry.init(), never cleaned up by close()) and + // any client-internal handlers — will be removed after init. + if (!preInitListeners) { + preInitListeners = new Set(process.rawListeners("beforeExit")); + } const client = Sentry.init({ dsn: SENTRY_CLI_DSN, @@ -427,7 +436,7 @@ export function initSentry( // the SDK's handlers — they keep the event loop alive in tests and on // re-initialization (e.g., auto-auth retry). for (const listener of process.rawListeners("beforeExit")) { - if (!listenersBefore.has(listener)) { + if (!preInitListeners?.has(listener)) { process.removeListener( "beforeExit", listener as (...args: unknown[]) => void From abfdab93a77697ba888e66aed029f0351c541353 Mon Sep 17 00:00:00 2001 From: mathuraditya724 Date: Tue, 31 Mar 2026 22:50:03 +0530 Subject: [PATCH 5/7] fix(telemetry): exclude ProcessSession integration and disable logs/reports when disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three sources of leaked beforeExit handlers prevented the Bun test runner from exiting: 1. ProcessSession integration registers an anonymous beforeExit handler via setupOnce that has no cleanup mechanism. Excluded it from integrations — release health session tracking is unnecessary for a short-lived CLI. 2. enableLogs creates a log flush beforeExit handler even when enabled=false. Now gated on enabled flag. 3. sendClientReports creates a setInterval + beforeExit handler even when enabled=false. Now disabled when telemetry is off. Combined with the existing client.close(0) call, this ensures zero leaked beforeExit listeners after initSentry(false), allowing the process to exit. --- src/lib/telemetry.ts | 47 ++++++++++---------------------------------- 1 file changed, 10 insertions(+), 37 deletions(-) diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index f5fe40c33..c25bafb5e 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -262,6 +262,10 @@ const EXCLUDED_INTEGRATIONS = new Set([ "ContextLines", // Reads source files - we rely on uploaded sourcemaps instead "LocalVariables", // Captures local variables - adds significant overhead "Modules", // Lists all loaded modules - unnecessary for CLI telemetry + // ProcessSession registers an anonymous beforeExit handler via setupOnce + // that has no cleanup mechanism. For a short-lived CLI process this is + // unnecessary and it prevents the Bun test runner from exiting. + "ProcessSession", ]); /** @@ -275,7 +279,6 @@ const LIBRARY_EXCLUDED_INTEGRATIONS = new Set([ ...EXCLUDED_INTEGRATIONS, "OnUncaughtException", // process.on('uncaughtException') "OnUnhandledRejection", // process.on('unhandledRejection') - "ProcessSession", // process.on('beforeExit') "Http", // diagnostics_channel + trace headers "NodeFetch", // diagnostics_channel + trace headers "FunctionToString", // wraps Function.prototype.toString @@ -304,13 +307,6 @@ const hasGetSystemErrorMap = (() => { /** Current beforeExit handler, tracked so it can be replaced on re-init */ let currentBeforeExitHandler: (() => void) | null = null; -/** - * Listeners that existed on `process.beforeExit` before the first `initSentry` - * call. Used to distinguish SDK-added handlers from application/test handlers. - */ -// biome-ignore lint/complexity/noBannedTypes: rawListeners returns Function[] -let preInitListeners: Set | null = null; - /** Match all SaaS regional URLs (us.sentry.io, de.sentry.io, o1234.ingest.us.sentry.io, etc.) */ const SENTRY_SAAS_SUBDOMAIN_RE = /^https:\/\/[^/]*\.sentry\.io(\/|$)/; @@ -369,15 +365,6 @@ export function initSentry( // close(0) removes listeners synchronously; we don't need to await the flush. Sentry.getClient()?.close(0); - // Snapshot beforeExit listeners that existed before the first initSentry call. - // These are non-SDK listeners (e.g., from the app or test framework) that must - // be preserved. All others — including the ProcessSession handler (registered - // via setupOnce on the first Sentry.init(), never cleaned up by close()) and - // any client-internal handlers — will be removed after init. - if (!preInitListeners) { - preInitListeners = new Set(process.rawListeners("beforeExit")); - } - const client = Sentry.init({ dsn: SENTRY_CLI_DSN, enabled, @@ -397,10 +384,12 @@ export function initSentry( }, environment, // Enable Sentry structured logs for non-exception telemetry (e.g., unexpected input warnings). - // Disabled in library mode — logs use timers/beforeExit that pollute the host process. - enableLogs: !libraryMode, - // Disable client reports in library mode — they use timers/beforeExit. - ...(libraryMode && { sendClientReports: false }), + // Disabled when telemetry is off or in library mode — the SDK registers + // beforeExit handlers for log flushing that keep the event loop alive. + enableLogs: enabled && !libraryMode, + // Disable client reports when telemetry is off or in library mode — the SDK + // registers a setInterval + beforeExit handler that keep the event loop alive. + ...((libraryMode || !enabled) && { sendClientReports: false }), // Sample all events for CLI telemetry (low volume) tracesSampleRate: 1, sampleRate: 1, @@ -428,22 +417,6 @@ export function initSentry( }, }); - // Remove all beforeExit listeners added by Sentry.init(). This covers: - // - ProcessSession integration (anonymous handler, setupOnce, no cleanup) - // - Client report flusher (_clientReportOnExitFlushListener) - // - Log flusher (_logOnExitFlushListener) - // We manage our own beforeExit handler explicitly below and don't need - // the SDK's handlers — they keep the event loop alive in tests and on - // re-initialization (e.g., auto-auth retry). - for (const listener of process.rawListeners("beforeExit")) { - if (!preInitListeners?.has(listener)) { - process.removeListener( - "beforeExit", - listener as (...args: unknown[]) => void - ); - } - } - // Always remove our own previous handler on re-init. if (currentBeforeExitHandler) { process.removeListener("beforeExit", currentBeforeExitHandler); From f3feb951f10bf60ab7dacd951a6e58a87ce41de5 Mon Sep 17 00:00:00 2001 From: mathuraditya724 Date: Tue, 31 Mar 2026 23:28:46 +0530 Subject: [PATCH 6/7] fix(test): clear preload auth token in tests that verify auth behavior The preload SENTRY_AUTH_TOKEN (added for the auth guard) takes precedence over DB-stored tokens in getAuthToken/getAuthConfig/isAuthenticated. Tests that verify auth storage, token refresh, unauthenticated paths, or specific SENTRY_TOKEN behavior need the env token cleared so they exercise the intended code paths. Files fixed: - config.test.ts: auth storage tests need DB path, not env token - api-client.test.ts: 401 retry tests need DB token for refresh flow - resolve-target.test.ts: fetchProjectId AuthError test needs no token - wizard-runner.test.ts: clear env token + mock isAuthenticated to prevent unmocked API calls whose retry timers kept the event loop alive (the root cause of the CI hang for this test file) - local-ops.create-sentry-project.test.ts: isAuthenticated/getAuthConfig need to use mocked values, not env token --- test/lib/api-client.test.ts | 11 +++++++++ test/lib/config.test.ts | 17 ++++++++++++- .../local-ops.create-sentry-project.test.ts | 14 +++++++++++ test/lib/init/wizard-runner.test.ts | 24 ++++++++++++++++++- test/lib/resolve-target.test.ts | 15 +++++++++--- 5 files changed, 76 insertions(+), 5 deletions(-) diff --git a/test/lib/api-client.test.ts b/test/lib/api-client.test.ts index 07f66ac51..a4a930120 100644 --- a/test/lib/api-client.test.ts +++ b/test/lib/api-client.test.ts @@ -107,6 +107,17 @@ function createMockFetch( describe("401 retry behavior", () => { // Note: These tests use rawApiRequest which goes to control silo (sentry.io) // and supports 401 retry with token refresh. + // Clear preload SENTRY_AUTH_TOKEN so the DB-stored token is used. + let savedAuthToken: string | undefined; + beforeEach(() => { + savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; + }); + afterEach(() => { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } + }); test("retries request with new token on 401 response", async () => { const requests: RequestLog[] = []; diff --git a/test/lib/config.test.ts b/test/lib/config.test.ts index ebd9af4ed..1d4730ae6 100644 --- a/test/lib/config.test.ts +++ b/test/lib/config.test.ts @@ -4,7 +4,7 @@ * Integration tests for SQLite-based config storage. */ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { writeFileSync } from "node:fs"; import { join } from "node:path"; import { @@ -47,6 +47,21 @@ import { useTestConfigDir } from "../helpers.js"; */ const getConfigDir = useTestConfigDir("test-config-"); +/** + * Clear the preload SENTRY_AUTH_TOKEN so auth tests exercise the DB path + * (getAuthToken/getAuthConfig check env vars first, which would bypass DB storage). + */ +let savedAuthToken: string | undefined; +beforeEach(() => { + savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; +}); +afterEach(() => { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } +}); + describe("auth token management", () => { test("setAuthToken stores token", async () => { await setAuthToken("test-token-123"); diff --git a/test/lib/init/local-ops.create-sentry-project.test.ts b/test/lib/init/local-ops.create-sentry-project.test.ts index deb034578..6c83fe6dd 100644 --- a/test/lib/init/local-ops.create-sentry-project.test.ts +++ b/test/lib/init/local-ops.create-sentry-project.test.ts @@ -61,6 +61,20 @@ const sampleProject: SentryProject = { dateCreated: "2026-03-04T00:00:00Z", }; +// Clear the preload SENTRY_AUTH_TOKEN so isAuthenticated() / getAuthConfig() +// use the DB path. Without this, the env token causes handleLocalOp to skip +// the "Not authenticated" branch and hit unexpected code paths. +let savedAuthToken: string | undefined; +beforeEach(() => { + savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; +}); +afterEach(() => { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } +}); + describe("create-sentry-project", () => { let resolveOrgSpy: ReturnType; let listOrgsSpy: ReturnType; diff --git a/test/lib/init/wizard-runner.test.ts b/test/lib/init/wizard-runner.test.ts index 24deee61c..efdd450b0 100644 --- a/test/lib/init/wizard-runner.test.ts +++ b/test/lib/init/wizard-runner.test.ts @@ -121,6 +121,22 @@ function setupWorkflowSpy() { return { mockWorkflow }; } +// ── Auth env override ─────────────────────────────────────────────────────── +// Clear the preload SENTRY_AUTH_TOKEN so runWizard's internal isAuthenticated +// check uses the mocked getAuthToken. Without this, the env token passes auth +// and runWizard tries to resolve orgs via real API calls, whose retry timers +// keep the event loop alive and prevent the process from exiting. +let savedAuthToken: string | undefined; +beforeEach(() => { + savedAuthToken = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; +}); +afterEach(() => { + if (savedAuthToken !== undefined) { + process.env.SENTRY_AUTH_TOKEN = savedAuthToken; + } +}); + // ── Setup / Teardown ──────────────────────────────────────────────────────── beforeEach(() => { @@ -149,8 +165,14 @@ beforeEach(() => { // git spy — default: pass all checks checkGitStatusSpy = spyOn(git, "checkGitStatus").mockResolvedValue(true); - // dep spies + // dep spies — mock the full auth surface so runWizard sees valid auth + // without hitting real APIs (env token is cleared above). getAuthTokenSpy = spyOn(auth, "getAuthToken").mockReturnValue("fake-token"); + spyOn(auth, "getAuthConfig").mockReturnValue({ + token: "fake-token", + source: "oauth" as const, + }); + spyOn(auth, "isAuthenticated").mockReturnValue(true); formatBannerSpy = spyOn(banner, "formatBanner").mockReturnValue("BANNER"); formatResultSpy = spyOn(fmt, "formatResult").mockImplementation(noop); formatErrorSpy = spyOn(fmt, "formatError").mockImplementation(noop); diff --git a/test/lib/resolve-target.test.ts b/test/lib/resolve-target.test.ts index 809853130..bdcdcce6c 100644 --- a/test/lib/resolve-target.test.ts +++ b/test/lib/resolve-target.test.ts @@ -438,11 +438,20 @@ describe("fetchProjectId", () => { test("rethrows AuthError when not authenticated", async () => { // No auth token set — refreshToken() will throw AuthError + // Clear preload env token so there's truly no auth + const saved = process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_AUTH_TOKEN; setOrgRegion("test-org", DEFAULT_SENTRY_URL); - expect(fetchProjectId("test-org", "test-project")).rejects.toThrow( - AuthError - ); + try { + await expect(fetchProjectId("test-org", "test-project")).rejects.toThrow( + AuthError + ); + } finally { + if (saved !== undefined) { + process.env.SENTRY_AUTH_TOKEN = saved; + } + } }); test("returns undefined on transient server error", async () => { From b65a918ee2f3c871df3c77802df6f0a8d7d38557 Mon Sep 17 00:00:00 2001 From: mathuraditya724 Date: Tue, 31 Mar 2026 23:34:13 +0530 Subject: [PATCH 7/7] fix(telemetry): restore ProcessSession integration for production session tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving ProcessSession to EXCLUDED_INTEGRATIONS broke release health monitoring — no session data (crash rates, health) was being transmitted in production. The integration was excluded as a blunt fix for the test runner hang, but it needs to run in normal CLI mode. Fix: move ProcessSession back to LIBRARY_EXCLUDED_INTEGRATIONS (library mode only) and instead clean up its beforeExit handler in the test file. telemetry.test.ts now snapshots beforeExit listeners before any test and removes SDK-added ones in afterAll, preventing the handler leak that kept the Bun test runner alive. --- src/lib/telemetry.ts | 5 +---- test/lib/telemetry.test.ts | 27 ++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index c25bafb5e..c9567c39b 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -262,10 +262,6 @@ const EXCLUDED_INTEGRATIONS = new Set([ "ContextLines", // Reads source files - we rely on uploaded sourcemaps instead "LocalVariables", // Captures local variables - adds significant overhead "Modules", // Lists all loaded modules - unnecessary for CLI telemetry - // ProcessSession registers an anonymous beforeExit handler via setupOnce - // that has no cleanup mechanism. For a short-lived CLI process this is - // unnecessary and it prevents the Bun test runner from exiting. - "ProcessSession", ]); /** @@ -279,6 +275,7 @@ const LIBRARY_EXCLUDED_INTEGRATIONS = new Set([ ...EXCLUDED_INTEGRATIONS, "OnUncaughtException", // process.on('uncaughtException') "OnUnhandledRejection", // process.on('unhandledRejection') + "ProcessSession", // process.on('beforeExit') — anonymous handler, no cleanup "Http", // diagnostics_channel + trace headers "NodeFetch", // diagnostics_channel + trace headers "FunctionToString", // wraps Function.prototype.toString diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index da7442fa3..46f36c3c9 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -5,7 +5,15 @@ */ import { Database } from "bun:sqlite"; -import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeEach, + describe, + expect, + spyOn, + test, +} from "bun:test"; import { chmodSync, mkdirSync, rmSync } from "node:fs"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as Sentry from "@sentry/node-core/light"; @@ -30,6 +38,23 @@ import { withTracingSpan, } from "../../src/lib/telemetry.js"; +// Snapshot beforeExit listeners before any test calls initSentry(true). +// The ProcessSession integration registers an anonymous handler via setupOnce +// that has no cleanup mechanism. After all tests, we remove any listeners +// that weren't present before to prevent the Bun test runner from hanging. +const preTestListeners = new Set(process.rawListeners("beforeExit")); + +afterAll(() => { + for (const listener of process.rawListeners("beforeExit")) { + if (!preTestListeners.has(listener)) { + process.removeListener( + "beforeExit", + listener as (...args: unknown[]) => void + ); + } + } +}); + describe("initSentry", () => { test("returns client with enabled=false when disabled", () => { const client = initSentry(false);