From 1e4ac5755f873836c5bc39867aff6844409a72aa Mon Sep 17 00:00:00 2001 From: betegon Date: Thu, 6 Aug 2026 21:30:37 +0200 Subject: [PATCH 1/5] fix(auth): request team admin OAuth scope --- apps/cli-docs/src/content/docs/self-hosted.md | 2 +- packages/cli/DEVELOPMENT.md | 2 +- packages/cli/src/cli.ts | 96 +--------- packages/cli/src/lib/api-scope.ts | 14 ++ packages/cli/src/lib/oauth.ts | 1 + packages/cli/src/lib/scope-recovery.ts | 124 +++++++++++++ packages/cli/test/lib/api-scope.test.ts | 10 ++ packages/cli/test/lib/oauth.test.ts | 4 + packages/cli/test/lib/scope-recovery.test.ts | 164 ++++++++++++++++++ 9 files changed, 326 insertions(+), 91 deletions(-) create mode 100644 packages/cli/src/lib/scope-recovery.ts create mode 100644 packages/cli/test/lib/scope-recovery.test.ts diff --git a/apps/cli-docs/src/content/docs/self-hosted.md b/apps/cli-docs/src/content/docs/self-hosted.md index 021622fd41..4301ef43ea 100644 --- a/apps/cli-docs/src/content/docs/self-hosted.md +++ b/apps/cli-docs/src/content/docs/self-hosted.md @@ -56,7 +56,7 @@ If your instance is on an older version or you prefer not to create an OAuth app 1. Go to **Settings → Developer Settings → Personal Tokens** in your Sentry instance (or visit `https://sentry.example.com/settings/account/api/auth-tokens/new-token/`) 2. Create a new token with the following scopes: -`project:read`, `project:write`, `project:admin`, `org:read`, `event:read`, `event:write`, `member:read`, `team:read`, `team:write`, `alerts:read`, `alerts:write` +`project:read`, `project:write`, `project:admin`, `org:read`, `event:read`, `event:write`, `member:read`, `team:read`, `team:write`, `team:admin`, `alerts:read`, `alerts:write` 3. Pass it to the CLI: diff --git a/packages/cli/DEVELOPMENT.md b/packages/cli/DEVELOPMENT.md index 98a3630f92..4bc3f3a3d5 100644 --- a/packages/cli/DEVELOPMENT.md +++ b/packages/cli/DEVELOPMENT.md @@ -69,7 +69,7 @@ When creating your Sentry OAuth application: - `org:read` - `event:read`, `event:write` - `member:read` - - `team:read`, `team:write` + - `team:read`, `team:write`, `team:admin` - `alerts:read`, `alerts:write` diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 9d9169f318..d7e8a811b4 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -243,9 +243,7 @@ export async function runCli(cliArgs: string[]): Promise { ); const { error } = await import("./lib/formatters/colors.js"); const { runInteractiveLogin } = await import("./lib/interactive-login.js"); - const { assertAutoLoginHostTrusted, recoverWithAutoLogin } = await import( - "./lib/auto-auth.js" - ); + const { recoverWithAutoLogin } = await import("./lib/auto-auth.js"); const { getEnvLogLevel, setLogLevel } = await import("./lib/logger.js"); const { isTrialEligible, promptAndStartTrial } = await import( "./lib/seer-trial.js" @@ -435,97 +433,17 @@ export async function runCli(cliArgs: string[]): Promise { } }; - /** - * Check whether a caught error is a recoverable 403 missing-scope error. - * - * Returns the extracted scope names when all conditions are met: - * - Interactive TTY (stdin) - * - Error is an `ApiError` with status 403 - * - Token is an OAuth token (not env-var — those can't be re-scoped via CLI) - * - The 403 detail mentions specific missing scopes - * - * Returns `null` when recovery is not possible, signaling the caller to - * re-throw. - */ - async function extractRecoverableScopes( - err: unknown - ): Promise { - if (!isatty(0)) { - return null; - } - const { ApiError } = await import("./lib/errors.js"); - if (!(err instanceof ApiError) || err.status !== 403) { - return null; - } - const { isEnvTokenActive } = await import("./lib/db/auth.js"); - if (isEnvTokenActive()) { - return null; - } - const { extractRequiredScopes } = await import("./lib/api-scope.js"); - const scopes = extractRequiredScopes(err.detail); - return scopes.length > 0 ? scopes : null; - } - /** * Scope recovery middleware. * - * Catches 403 Forbidden errors for OAuth tokens (not env-var tokens) in - * interactive TTYs. When specific missing scopes are detected in the API - * response, offers to re-authenticate with those scopes and retries the - * command — mirroring `gh auth refresh -s `. - * - * Env-var tokens are excluded: the user must regenerate those manually - * via the Sentry web UI (the 403 enrichment already directs them there). + * Existing stored OAuth grants may predate the current standard scope set. + * On a scope-specific 403, offer one refresh with today's defaults and retry + * exactly once. Non-interactive and explicitly unattended commands never + * enter a device flow. */ const scopeRecoveryMiddleware: ErrorMiddleware = async (next, argv) => { - try { - await next(argv); - } catch (err) { - const scopes = await extractRecoverableScopes(err); - if (!scopes) { - throw err; - } - - // Same host-trust gate as auto-login: re-authenticating to add scopes - // also runs the OAuth device flow, so refuse an unconfirmed self-hosted - // host before prompting (an injected env.SENTRY_URL must not steer the - // browser to an attacker's login page). - assertAutoLoginHostTrusted(); - - const scopeList = scopes.map((s) => `'${s}'`).join(", "); - const { logger: logModule } = await import("./lib/logger.js"); - const confirmed = await logModule - .withTag("auth") - .prompt( - `Missing scope(s): ${scopeList}. Re-authenticate with default scopes?`, - { type: "confirm", initial: true } - ); - - // Symbol(clack:cancel) is truthy — strict equality check - if (confirmed !== true) { - throw err; - } - - process.stderr.write("\n"); - // Merge missing scopes with the default set so the new token retains - // all previously-held scopes plus the ones the API requested. - const { OAUTH_SCOPES, resolveOAuthScopeString } = await import( - "./lib/oauth.js" - ); - const merged = [...new Set([...OAUTH_SCOPES, ...scopes])]; - const scope = resolveOAuthScopeString({ scopes: merged }); - const loginSuccess = await runInteractiveLogin({ scope }); - - if (loginSuccess) { - process.stderr.write("\nRetrying command...\n\n"); - await next(argv); - return; - } - - // Login failed or was cancelled — re-throw so the user sees the - // original 403 message with the scope hint. - throw err; - } + const { runWithScopeRecovery } = await import("./lib/scope-recovery.js"); + await runWithScopeRecovery(next, argv, runInteractiveLogin); }; /** diff --git a/packages/cli/src/lib/api-scope.ts b/packages/cli/src/lib/api-scope.ts index a5049d5f1d..3228669c09 100644 --- a/packages/cli/src/lib/api-scope.ts +++ b/packages/cli/src/lib/api-scope.ts @@ -61,6 +61,11 @@ export function extractRequiredScopes(detail: unknown): string[] { if (!detail) { return []; } + const serializedDetail = + typeof detail === "string" ? detail : JSON.stringify(detail); + if (isMemberProjectCreationPolicy(serializedDetail)) { + return []; + } if (typeof detail === "object") { const fromFields = extractFromRecord(detail as Record); if (fromFields.length > 0) { @@ -75,6 +80,15 @@ export function extractRequiredScopes(detail: unknown): string[] { return []; } +/** A role/policy denial can mention scope names without a token lacking them. */ +function isMemberProjectCreationPolicy(detail: string): boolean { + const normalized = detail.toLowerCase(); + return ( + normalized.includes("disabled this feature for members") || + normalized.includes("org-level policy setting, not an auth issue") + ); +} + function extractFromRecord(record: Record): string[] { for (const field of SCOPE_FIELD_NAMES) { const value = record[field]; diff --git a/packages/cli/src/lib/oauth.ts b/packages/cli/src/lib/oauth.ts index b86cbfb2ab..05e54d9207 100644 --- a/packages/cli/src/lib/oauth.ts +++ b/packages/cli/src/lib/oauth.ts @@ -90,6 +90,7 @@ export const OAUTH_SCOPES: readonly string[] = [ "member:read", "team:read", "team:write", + "team:admin", "alerts:read", "alerts:write", ]; diff --git a/packages/cli/src/lib/scope-recovery.ts b/packages/cli/src/lib/scope-recovery.ts new file mode 100644 index 0000000000..91133094fd --- /dev/null +++ b/packages/cli/src/lib/scope-recovery.ts @@ -0,0 +1,124 @@ +/** + * One-time recovery for stored OAuth grants that predate the CLI's current + * standard scope set. + */ + +import { isatty } from "node:tty"; +import { extractRequiredScopes } from "./api-scope.js"; +import { assertAutoLoginHostTrusted } from "./auto-auth.js"; +import { type AuthSource, getAuthConfig } from "./db/auth.js"; +import { ApiError } from "./errors.js"; +import type { + InteractiveLoginOptions, + LoginResult, +} from "./interactive-login.js"; +import { interactivePromptsAllowed } from "./interactive-prompts.js"; +import { logger } from "./logger.js"; +import { OAUTH_SCOPES, resolveOAuthScopeString } from "./oauth.js"; + +type InteractiveLogin = ( + options?: InteractiveLoginOptions +) => Promise; + +export type ScopeRecoveryRuntime = { + assertTrustedHost: () => void; + confirm: (message: string) => Promise; + getAuthSource: () => AuthSource | undefined; + inputIsTty: () => boolean; + promptsAllowed: () => boolean; + write: (message: string) => void; +}; + +const defaultRuntime: ScopeRecoveryRuntime = { + assertTrustedHost: assertAutoLoginHostTrusted, + confirm: (message) => + logger.withTag("auth").prompt(message, { + type: "confirm", + initial: true, + }), + getAuthSource: () => getAuthConfig()?.source, + inputIsTty: () => isatty(0), + promptsAllowed: interactivePromptsAllowed, + write: (message) => { + process.stderr.write(message); + }, +}; + +function disablesInteractiveRecovery(argv: string[]): boolean { + return argv.some( + (arg) => + arg === "--yes" || + arg === "-y" || + arg.startsWith("--yes=") || + arg === "--dry-run" || + arg.startsWith("--dry-run=") + ); +} + +function recoverableScopes( + error: unknown, + argv: string[], + runtime: ScopeRecoveryRuntime +): string[] | null { + let authSource: AuthSource | undefined; + try { + authSource = runtime.getAuthSource(); + } catch { + // Recovery must never replace the command's original 403 with a local + // credential-store read failure. + return null; + } + + if ( + !(runtime.inputIsTty() && runtime.promptsAllowed()) || + disablesInteractiveRecovery(argv) || + authSource !== "oauth" || + !(error instanceof ApiError) || + error.status !== 403 + ) { + return null; + } + + const scopes = extractRequiredScopes(error.detail); + return scopes.length > 0 ? scopes : null; +} + +/** + * Run a command once and, for an old interactive OAuth grant, refresh it with + * the current standard scopes before retrying the command exactly once. + */ +export async function runWithScopeRecovery( + proceed: (commandArgs: string[]) => Promise, + argv: string[], + runInteractiveLogin: InteractiveLogin, + runtime: ScopeRecoveryRuntime = defaultRuntime +): Promise { + try { + await proceed(argv); + } catch (error) { + const scopes = recoverableScopes(error, argv, runtime); + if (!scopes) { + throw error; + } + + runtime.assertTrustedHost(); + const scopeList = scopes.map((scopeName) => `'${scopeName}'`).join(", "); + const confirmed = await runtime.confirm( + `Your existing CLI authorization is missing standard scope(s) ${scopeList}. Refresh it with the current defaults?` + ); + if (confirmed !== true) { + throw error; + } + + runtime.write("\n"); + const merged = [...new Set([...OAUTH_SCOPES, ...scopes])]; + const requestedScope = resolveOAuthScopeString({ scopes: merged }); + const loginResult = await runInteractiveLogin({ scope: requestedScope }); + if (!loginResult) { + throw error; + } + + runtime.write("\nRetrying command...\n\n"); + await proceed(argv); + } +} diff --git a/packages/cli/test/lib/api-scope.test.ts b/packages/cli/test/lib/api-scope.test.ts index db2e7d50a6..e983e843ec 100644 --- a/packages/cli/test/lib/api-scope.test.ts +++ b/packages/cli/test/lib/api-scope.test.ts @@ -21,6 +21,16 @@ describe("extractRequiredScopes", () => { ).toEqual([]); }); + test("ignores role scopes mentioned in member-project policy guidance", () => { + expect( + extractRequiredScopes( + "Your organization has disabled this feature for members. " + + "This is an org-level policy setting, not an auth issue. " + + "You need org:admin/manager/owner role, or team:admin role on the team." + ) + ).toEqual([]); + }); + test("extracts a single scope from a detail string", () => { expect( extractRequiredScopes( diff --git a/packages/cli/test/lib/oauth.test.ts b/packages/cli/test/lib/oauth.test.ts index e0a990db2d..4d32db281f 100644 --- a/packages/cli/test/lib/oauth.test.ts +++ b/packages/cli/test/lib/oauth.test.ts @@ -23,6 +23,10 @@ import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; const knownScopeArb = constantFrom(...SENTRY_SCOPES); describe("resolveOAuthScopeString", () => { + test("default scopes include Team Admin for project creation", () => { + expect(OAUTH_SCOPES).toContain("team:admin"); + }); + test("default (no selection) returns the full OAUTH_SCOPES set", () => { expect(resolveOAuthScopeString()).toBe(OAUTH_SCOPES.join(" ")); expect(resolveOAuthScopeString({})).toBe(OAUTH_SCOPES.join(" ")); diff --git a/packages/cli/test/lib/scope-recovery.test.ts b/packages/cli/test/lib/scope-recovery.test.ts new file mode 100644 index 0000000000..2e5a2b6f45 --- /dev/null +++ b/packages/cli/test/lib/scope-recovery.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, test, vi } from "vitest"; +import { ApiError } from "../../src/lib/errors.js"; +import { + runWithScopeRecovery, + type ScopeRecoveryRuntime, +} from "../../src/lib/scope-recovery.js"; + +function missingScopeError(): ApiError { + return new ApiError( + "Forbidden", + 403, + "You do not have the required scope: team:admin" + ); +} + +function runtime( + overrides: Partial = {} +): ScopeRecoveryRuntime { + return { + assertTrustedHost: vi.fn(), + confirm: vi.fn().mockResolvedValue(true), + getAuthSource: () => "oauth", + inputIsTty: () => true, + promptsAllowed: () => true, + write: vi.fn(), + ...overrides, + }; +} + +describe("runWithScopeRecovery", () => { + test("refreshes an old OAuth grant with current scopes and retries once", async () => { + const originalError = missingScopeError(); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(originalError) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ + method: "oauth", + configPath: "/tmp/config", + }); + const testRuntime = runtime(); + + await runWithScopeRecovery( + proceed, + ["project", "create"], + login, + testRuntime + ); + + expect(proceed).toHaveBeenCalledTimes(2); + expect(testRuntime.assertTrustedHost).toHaveBeenCalledOnce(); + expect(testRuntime.confirm).toHaveBeenCalledOnce(); + expect(login).toHaveBeenCalledOnce(); + const scope = login.mock.calls[0]?.[0]?.scope; + expect(scope?.split(" ")).toEqual( + expect.arrayContaining(["org:read", "project:write", "team:admin"]) + ); + }); + + test.each([ + [["init", "--yes"], "oauth" as const], + [["init", "-y"], "oauth" as const], + [["init", "--dry-run"], "oauth" as const], + [["project", "create"], "env:SENTRY_AUTH_TOKEN" as const], + ])("does not refresh unattended commands or env tokens", async (argv, source) => { + const originalError = missingScopeError(); + const proceed = vi.fn().mockRejectedValue(originalError); + const login = vi.fn(); + + await expect( + runWithScopeRecovery( + proceed, + argv, + login, + runtime({ getAuthSource: () => source }) + ) + ).rejects.toBe(originalError); + + expect(proceed).toHaveBeenCalledOnce(); + expect(login).not.toHaveBeenCalled(); + }); + + test("preserves the original error when the credential store cannot be read", async () => { + const originalError = missingScopeError(); + const proceed = vi.fn().mockRejectedValue(originalError); + const login = vi.fn(); + + await expect( + runWithScopeRecovery( + proceed, + [], + login, + runtime({ + getAuthSource: () => { + throw new Error("database unavailable"); + }, + }) + ) + ).rejects.toBe(originalError); + expect(login).not.toHaveBeenCalled(); + }); + + test.each([ + { inputIsTty: () => false }, + { promptsAllowed: () => false }, + ])("does not refresh outside an interactive prompt context", async (overrides) => { + const originalError = missingScopeError(); + const proceed = vi.fn().mockRejectedValue(originalError); + const login = vi.fn(); + + await expect( + runWithScopeRecovery(proceed, [], login, runtime(overrides)) + ).rejects.toBe(originalError); + expect(login).not.toHaveBeenCalled(); + }); + + test("preserves the original error when refresh is declined", async () => { + const originalError = missingScopeError(); + const proceed = vi.fn().mockRejectedValue(originalError); + const login = vi.fn(); + + await expect( + runWithScopeRecovery( + proceed, + [], + login, + runtime({ confirm: vi.fn().mockResolvedValue(false) }) + ) + ).rejects.toBe(originalError); + expect(login).not.toHaveBeenCalled(); + }); + + test("preserves the original error when login is cancelled", async () => { + const originalError = missingScopeError(); + const proceed = vi.fn().mockRejectedValue(originalError); + const login = vi.fn().mockResolvedValue(null); + + await expect( + runWithScopeRecovery(proceed, [], login, runtime()) + ).rejects.toBe(originalError); + expect(proceed).toHaveBeenCalledOnce(); + }); + + test("does not attempt a second recovery when the retry fails", async () => { + const firstError = missingScopeError(); + const retryError = missingScopeError(); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(firstError) + .mockRejectedValueOnce(retryError); + const login = vi.fn().mockResolvedValue({ + method: "oauth", + configPath: "/tmp/config", + }); + const testRuntime = runtime(); + + await expect( + runWithScopeRecovery(proceed, [], login, testRuntime) + ).rejects.toBe(retryError); + expect(proceed).toHaveBeenCalledTimes(2); + expect(testRuntime.confirm).toHaveBeenCalledOnce(); + expect(login).toHaveBeenCalledOnce(); + }); +}); From 792e023b493ee8f7371cbad51247b9902eedfe00 Mon Sep 17 00:00:00 2001 From: betegon Date: Mon, 10 Aug 2026 17:48:19 +0200 Subject: [PATCH 2/5] fix(auth): generalize OAuth scope recovery --- packages/cli/src/commands/alert/list-utils.ts | 10 +- .../cli/src/commands/dashboard/resolve.ts | 12 +- packages/cli/src/commands/issue/list.ts | 28 ++-- packages/cli/src/commands/project/create.ts | 23 ++- packages/cli/src/lib/api-scope.ts | 137 ++++++++++++++++++ packages/cli/src/lib/api/infrastructure.ts | 44 ++++-- packages/cli/src/lib/errors.ts | 71 ++++++++- packages/cli/src/lib/init/preflight.ts | 64 ++++++-- .../lib/init/tools/create-sentry-project.ts | 45 +++++- packages/cli/src/lib/init/tools/registry.ts | 4 + packages/cli/src/lib/init/types.ts | 2 + packages/cli/src/lib/init/wizard-runner.ts | 15 +- packages/cli/src/lib/resolve-team.ts | 59 ++++++-- packages/cli/src/lib/scope-recovery.ts | 13 +- .../cli/test/commands/project/create.test.ts | 54 ++++++- packages/cli/test/lib/api-scope.test.ts | 84 ++++++++++- .../cli/test/lib/api/infrastructure.test.ts | 69 +++++++++ packages/cli/test/lib/errors.test.ts | 20 +++ packages/cli/test/lib/init/preflight.test.ts | 20 +++ .../init/tools/create-sentry-project.test.ts | 97 ++++++++++++- .../cli/test/lib/init/wizard-runner.test.ts | 33 +++++ packages/cli/test/lib/resolve-team.test.ts | 43 ++++++ packages/cli/test/lib/scope-recovery.test.ts | 52 ++++++- 23 files changed, 910 insertions(+), 89 deletions(-) diff --git a/packages/cli/src/commands/alert/list-utils.ts b/packages/cli/src/commands/alert/list-utils.ts index 5330713756..cb0c34dcee 100644 --- a/packages/cli/src/commands/alert/list-utils.ts +++ b/packages/cli/src/commands/alert/list-utils.ts @@ -1,6 +1,6 @@ /** Shared helpers for alert list commands. */ -import { ApiError, ValidationError } from "../../lib/errors.js"; +import { ApiError, cloneApiError, ValidationError } from "../../lib/errors.js"; import { LIST_MAX_LIMIT } from "../../lib/list-command.js"; import { distributeFetchBudget, type FetchResult } from "../../lib/org-list.js"; @@ -42,13 +42,7 @@ export function throwAlertListFetchFailure( if (!(error instanceof ApiError)) { throw new Error(`${prefix}: ${error.message}`); } - throw new ApiError( - `${prefix}: ${error.message}`, - error.status, - error.detail, - error.endpoint, - error.enriched403 - ); + throw cloneApiError(error, { message: `${prefix}: ${error.message}` }); } export function buildAlertListFailureErrors( diff --git a/packages/cli/src/commands/dashboard/resolve.ts b/packages/cli/src/commands/dashboard/resolve.ts index d86e44eb11..cc2aed4cea 100644 --- a/packages/cli/src/commands/dashboard/resolve.ts +++ b/packages/cli/src/commands/dashboard/resolve.ts @@ -14,6 +14,7 @@ import type { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { ApiError, ContextError, + cloneApiError, ResolutionError, ValidationError, } from "../../lib/errors.js"; @@ -792,13 +793,12 @@ export async function enrichDashboardError( error.status === 400 && (ctx.operation === "create" || ctx.operation === "update") ) { - throw new ApiError( - `Dashboard ${ctx.operation} failed in ${org}`, - error.status, - error.detail ?? + throw cloneApiError(error, { + message: `Dashboard ${ctx.operation} failed in ${org}`, + detail: + error.detail ?? "The API rejected the request. Check widget configuration.", - error.endpoint - ); + }); } throw error; diff --git a/packages/cli/src/commands/issue/list.ts b/packages/cli/src/commands/issue/list.ts index 2f8205a27e..1054111a0d 100644 --- a/packages/cli/src/commands/issue/list.ts +++ b/packages/cli/src/commands/issue/list.ts @@ -41,6 +41,7 @@ import { createDsnFingerprint } from "../../lib/dsn/index.js"; import { ApiError, ContextError, + cloneApiError, toSearchQueryError, ValidationError, withAuthGuard, @@ -917,12 +918,9 @@ function enrichIssueListError( } if (error instanceof ApiError) { if (error.status === 400) { - throw new ApiError( - error.message, - error.status, - build400Detail(error.detail, flags), - error.endpoint - ); + throw cloneApiError(error, { + detail: build400Detail(error.detail, flags), + }); } if (error.status === 403) { // Centralized 403 enrichment (infrastructure.ts) already added @@ -930,13 +928,7 @@ function enrichIssueListError( const detail = error.enriched403 ? appendProjectMembershipHint(error.detail) : build403Detail(error.detail); - throw new ApiError( - error.message, - error.status, - detail, - error.endpoint, - true - ); + throw cloneApiError(error, { detail, enriched403: true }); } } throw error; @@ -1154,13 +1146,11 @@ async function handleResolvedTargets( ? appendProjectMembershipHint(first.detail) : build403Detail(first.detail); } - throw new ApiError( - `${prefix}: ${first.message}`, - first.status, + throw cloneApiError(first, { + message: `${prefix}: ${first.message}`, detail, - first.endpoint, - first.enriched403 || first.status === 403 - ); + enriched403: first.enriched403 || first.status === 403, + }); } throw new Error(`${prefix}: ${first.message}`); diff --git a/packages/cli/src/commands/project/create.ts b/packages/cli/src/commands/project/create.ts index 9c34b7a47d..9777bb5c67 100644 --- a/packages/cli/src/commands/project/create.ts +++ b/packages/cli/src/commands/project/create.ts @@ -32,9 +32,11 @@ import { ApiError, CliError, ContextError, + cloneApiError, ResolutionError, ValidationError, withAuthGuard, + withRequiredScopes, } from "../../lib/errors.js"; import { formatProjectCreateOutput, @@ -283,6 +285,8 @@ type CreateProjectOpts = CreateProjectBaseOpts & { teamSlug: string; /** Source used to resolve the organization, when auto-detected. */ detectedFrom?: string; + /** Scopes granted by the user's effective role on the resolved team. */ + teamRoleScopes?: readonly string[]; }; /** @@ -366,12 +370,9 @@ function handleCreateApiError( // error-reporting.ts applies — e.g. a 403 "feature disabled for members" is a // permission issue, not a CLI bug. 5xx and network errors still get captured. // The message is kept short — ApiError.format() appends detail/endpoint. - throw new ApiError( - `Failed to create project '${name}' in ${orgSlug} (HTTP ${error.status}).`, - error.status, - error.detail, - error.endpoint - ); + throw cloneApiError(error, { + message: `Failed to create project '${name}' in ${orgSlug} (HTTP ${error.status}).`, + }); } /** @@ -391,7 +392,14 @@ async function createProjectWithErrors( if (error.status === 404) { return await handleCreateProject404(opts); } - return handleCreateApiError(error, opts); + const scopedError = + error.status === 403 && + error.requiredScopes.length === 0 && + opts.teamRoleScopes?.includes("team:admin") && + error.detail?.includes(MEMBER_PROJECT_CREATION_DISABLED_DETAIL) + ? withRequiredScopes(error, ["team:admin"]) + : error; + return handleCreateApiError(scopedError, opts); } } @@ -594,6 +602,7 @@ async function createOneProject(opts: { name, platform, detectedFrom, + teamRoleScopes: team.roleScopes, }); } catch (error) { // 403 means the user lacks permission to create or access teams, or to diff --git a/packages/cli/src/lib/api-scope.ts b/packages/cli/src/lib/api-scope.ts index 3228669c09..dff34b92a3 100644 --- a/packages/cli/src/lib/api-scope.ts +++ b/packages/cli/src/lib/api-scope.ts @@ -44,6 +44,19 @@ export const SENTRY_SCOPES = [ "alerts:write", ] as const; +const OAUTH_SCOPE_TOKEN_RE = /^[\x21\x23-\x5b\x5d-\x7e]+$/; +const BEARER_CHALLENGE_RE = /^Bearer\s+(.+)$/i; +const AUTH_SCHEME_TOKEN_CHAR_RE = /^[A-Za-z0-9!#$%&'*+.^_`|~-]$/; +const SCOPE_SEPARATOR_RE = /\s+/; +const MISSING_BEFORE_SCOPE_RE = + /(?:missing|lacks?|without|insufficient|required)[^.\n]{0,80}scopes?/; +const SCOPE_BEFORE_MISSING_RE = + /scopes?[^.\n]{0,80}(?:missing|required|insufficient)/; +const TOKEN_MISSING_RE = + /(?:token|authorization|oauth)[^.\n]{0,120}(?:missing|lacks?|without|insufficient)/; +const EXPLICIT_SCOPE_REQUIREMENT_RE = + /(?:required\s*:|obtaining[^.\n]{0,80}scope)/; + // Explicit alternation (not `:` product) rejects nonexistent // combinations like `release:write` or `alerts:admin`. `:` is not a // regex metachar so no escaping needed. @@ -80,6 +93,117 @@ export function extractRequiredScopes(detail: unknown): string[] { return []; } +/** + * Extract required scopes from an RFC 6750 Bearer challenge. + * + * Unlike the legacy response-body parser above, this accepts valid scopes + * that a newer Sentry server may introduce after this CLI was released. The + * challenge is authoritative, so recovery can remain generic when the CLI's + * standard OAuth scope set grows in the future. + * + * @param header - The `WWW-Authenticate` response header + * @returns Deduplicated, source-ordered scopes for `insufficient_scope` + */ +export function extractRequiredScopesFromWwwAuthenticate( + header: string | null | undefined +): string[] { + if (!header) { + return []; + } + for (const challenge of splitAuthenticateChallenges(header)) { + const bearer = BEARER_CHALLENGE_RE.exec(challenge); + if (!bearer?.[1]) { + continue; + } + const params = bearer[1]; + const error = extractAuthParam(params, "error"); + if (error?.toLowerCase() !== "insufficient_scope") { + continue; + } + const scope = extractAuthParam(params, "scope"); + if (!scope) { + continue; + } + return [ + ...new Set( + scope + .split(SCOPE_SEPARATOR_RE) + .filter((candidate) => OAUTH_SCOPE_TOKEN_RE.test(candidate)) + ), + ]; + } + return []; +} + +function splitAuthenticateChallenges(header: string): string[] { + const challenges: string[] = []; + let challengeStart = 0; + let inQuotes = false; + let escaped = false; + + for (let index = 0; index < header.length; index += 1) { + const char = header[index]; + if (escaped) { + escaped = false; + continue; + } + if (char === "\\" && inQuotes) { + escaped = true; + continue; + } + if (char === '"') { + inQuotes = !inQuotes; + continue; + } + if (char !== "," || inQuotes) { + continue; + } + + const nextSchemeStart = findNextAuthSchemeStart(header, index + 1); + if (nextSchemeStart === undefined) { + continue; + } + challenges.push(header.slice(challengeStart, index).trim()); + challengeStart = nextSchemeStart; + } + + challenges.push(header.slice(challengeStart).trim()); + return challenges.filter(Boolean); +} + +function findNextAuthSchemeStart( + header: string, + offset: number +): number | undefined { + let cursor = offset; + while (header[cursor] === " " || header[cursor] === "\t") { + cursor += 1; + } + const tokenStart = cursor; + while ( + cursor < header.length && + AUTH_SCHEME_TOKEN_CHAR_RE.test(header[cursor] as string) + ) { + cursor += 1; + } + if ( + cursor > tokenStart && + (header[cursor] === " " || header[cursor] === "\t") + ) { + return tokenStart; + } + return; +} + +function extractAuthParam(header: string, name: string): string | undefined { + const pattern = new RegExp( + `(?:^|[,\\s])${name}\\s*=\\s*(?:"([^"]*)"|([^,\\s]+))`, + "i" + ); + const match = pattern.exec(header); + return match?.[1] ?? match?.[2]; +} + /** A role/policy denial can mention scope names without a token lacking them. */ function isMemberProjectCreationPolicy(detail: string): boolean { const normalized = detail.toLowerCase(); @@ -138,9 +262,22 @@ function matchesKnownScope(scope: string): boolean { } function extractFromText(text: string): string[] { + if (!describesMissingTokenScope(text)) { + return []; + } const matches = text.match(KNOWN_SCOPE_RE); if (!matches) { return []; } return [...new Set(matches.map((m) => m.toLowerCase()))]; } + +function describesMissingTokenScope(text: string): boolean { + const normalized = text.toLowerCase(); + return ( + MISSING_BEFORE_SCOPE_RE.test(normalized) || + SCOPE_BEFORE_MISSING_RE.test(normalized) || + TOKEN_MISSING_RE.test(normalized) || + EXPLICIT_SCOPE_REQUIREMENT_RE.test(normalized) + ); +} diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 9664e5a57c..bdec9b22ef 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -13,7 +13,10 @@ import { parseSentryLinkHeader } from "@sentry/api"; import * as Sentry from "@sentry/node-core/light"; import type { z } from "zod"; -import { extractRequiredScopes } from "../api-scope.js"; +import { + extractRequiredScopes, + extractRequiredScopesFromWwwAuthenticate, +} from "../api-scope.js"; import { getActiveEnvVarName, isEnvTokenActive } from "../db/auth.js"; import { getEnv } from "../env.js"; import { ApiError, AuthError, stringifyUnknown } from "../errors.js"; @@ -38,9 +41,15 @@ import { * - env-var tokens → suggest checking token scopes * - OAuth tokens → suggest re-authentication */ -function enrich403Detail(rawDetail: string | undefined): string { +function enrich403Detail( + rawDetail: string | undefined, + requiredScopes: readonly string[] = [] +): string { // Org-level policy — re-auth and token scope advice do not apply here. - if (rawDetail?.includes("disabled this feature")) { + if ( + requiredScopes.length === 0 && + rawDetail?.includes("disabled this feature") + ) { return [ rawDetail, "", @@ -54,7 +63,10 @@ function enrich403Detail(rawDetail: string | undefined): string { lines.push(rawDetail, ""); } - const scopes = extractRequiredScopes(rawDetail); + const scopes = + requiredScopes.length > 0 + ? [...requiredScopes] + : extractRequiredScopes(rawDetail); if (isEnvTokenActive()) { if (scopes.length > 0) { @@ -149,10 +161,14 @@ export function enrich401Detail(rawDetail: string | undefined): string { function enrichDetail( status: number, detail: string | undefined, - hasUsableDetail: boolean + hasUsableDetail: boolean, + requiredScopes: readonly string[] = [] ): string | undefined { if (status === 403) { - return enrich403Detail(hasUsableDetail ? detail : undefined); + return enrich403Detail( + hasUsableDetail ? detail : undefined, + requiredScopes + ); } if (status === 401) { return enrich401Detail(hasUsableDetail ? detail : undefined); @@ -230,12 +246,16 @@ export function throwApiError( : stringifyUnknown(error); const is403 = status === 403; + const requiredScopes = extractRequiredScopesFromWwwAuthenticate( + response.headers.get("www-authenticate") + ); throw new ApiError( `${context}: ${status} ${response.statusText ?? "Unknown"}`, status, - enrichDetail(status, detail, hasUsableDetail), + enrichDetail(status, detail, hasUsableDetail, requiredScopes), undefined, - is403 + is403, + requiredScopes ); } @@ -592,12 +612,16 @@ async function throwRawApiError( "www-authenticate": response.headers.get("www-authenticate"), }); const is403 = response.status === 403; + const requiredScopes = extractRequiredScopesFromWwwAuthenticate( + response.headers.get("www-authenticate") + ); throw new ApiError( `API request failed: ${response.status} ${response.statusText}`, response.status, - enrichDetail(response.status, detail, detail !== undefined), + enrichDetail(response.status, detail, detail !== undefined, requiredScopes), endpoint, - is403 + is403, + requiredScopes ); } diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index 666b297852..03beb42202 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -23,6 +23,7 @@ * @see https://cli.sentry.dev/exit-codes/ for full reference */ +import { extractRequiredScopes } from "./api-scope.js"; import { buildBillingUrl, buildOrgSettingsUrl, @@ -171,6 +172,8 @@ export class HostScopeError extends CliError { * @param status - HTTP status code * @param detail - Detailed error message from API response * @param endpoint - API endpoint that failed + * @param enriched403 - Whether centralized API handling enriched this 403 + * @param requiredScopes - OAuth scopes reported by an insufficient-scope challenge */ export class ApiError extends CliError { readonly status: number; @@ -184,13 +187,17 @@ export class ApiError extends CliError { */ readonly enriched403: boolean; - // biome-ignore lint/nursery/useMaxParams: established 4-param shape; enriched403 is a defaulted extension + /** Trusted scope-recovery metadata from a server challenge or endpoint context. */ + readonly requiredScopes: readonly string[]; + + // biome-ignore lint/nursery/useMaxParams: established positional shape; new metadata is defaulted for compatibility constructor( message: string, status: number, detail?: string, endpoint?: string, - enriched403 = false + enriched403 = false, + requiredScopes: readonly string[] = [] ) { super(message, EXIT.API); this.name = "ApiError"; @@ -198,6 +205,7 @@ export class ApiError extends CliError { this.detail = detail; this.endpoint = endpoint; this.enriched403 = enriched403; + this.requiredScopes = [...requiredScopes]; } override format(): string { @@ -212,6 +220,65 @@ export class ApiError extends CliError { } } +/** Metadata overrides accepted when cloning an API error. */ +export type ApiErrorOverrides = { + message?: string; + detail?: string; + enriched403?: boolean; + requiredScopes?: readonly string[]; +}; + +/** Clone an API error while preserving all recovery metadata by default. */ +export function cloneApiError( + error: ApiError, + overrides: ApiErrorOverrides = {} +): ApiError { + return new ApiError( + overrides.message ?? error.message, + error.status, + overrides.detail ?? error.detail, + error.endpoint, + overrides.enriched403 ?? error.enriched403, + overrides.requiredScopes ?? error.requiredScopes + ); +} + +/** + * Return an API error carrying additional authoritative OAuth scope metadata. + * + * Some endpoint-specific permission checks cannot emit an RFC 6750 challenge, + * but their caller may have enough role context to identify the missing token + * scope. Cloning keeps that contextual hint structured for the central + * recovery layer without rewriting response text. + * + * @param error - Original API error + * @param scopes - Required OAuth scopes established by the caller + * @returns An equivalent API error with merged scope metadata + */ +export function withRequiredScopes( + error: ApiError, + scopes: readonly string[] +): ApiError { + return cloneApiError(error, { + requiredScopes: [...new Set([...error.requiredScopes, ...scopes])], + }); +} + +/** Resolve authoritative or legacy required-scope metadata from an API error. */ +export function getRequiredOAuthScopes(error: unknown): string[] { + if (!(error instanceof ApiError && error.status === 403)) { + return []; + } + return error.requiredScopes.length > 0 + ? [...error.requiredScopes] + : extractRequiredScopes(error.detail); +} + +/** Whether an API error can be retried after expanding an OAuth grant. */ +export function isRecoverableOAuthScopeError(error: unknown): boolean { + return getRequiredOAuthScopes(error).length > 0; +} + export type AuthErrorReason = "not_authenticated" | "expired" | "invalid"; /** Options for AuthError */ diff --git a/packages/cli/src/lib/init/preflight.ts b/packages/cli/src/lib/init/preflight.ts index 347174ab70..d90528e0af 100644 --- a/packages/cli/src/lib/init/preflight.ts +++ b/packages/cli/src/lib/init/preflight.ts @@ -5,7 +5,11 @@ import { listTeams, } from "../api-client.js"; import { getAuthToken } from "../db/auth.js"; -import { ApiError, WizardError } from "../errors.js"; +import { + ApiError, + isRecoverableOAuthScopeError, + WizardError, +} from "../errors.js"; import { buildOrgNotFoundError, resolveOrCreateTeam } from "../resolve-team.js"; import { slugify } from "../utils.js"; import { WizardCancelledError } from "./clack-utils.js"; @@ -38,6 +42,8 @@ type ProjectSelection = Pick< "project" | "existingProject" >; +type TeamSelection = Pick; + /** * Resolve org, project, team, and auth state before the init workflow starts. */ @@ -62,9 +68,14 @@ export async function resolveInitContext( return null; } - const team = await resolveTeam(org, initial, ui); + const teamSelection = await resolveTeam(org, initial, ui); - return buildResolvedInitContext(initial, org, team, projectSelection); + return buildResolvedInitContext( + initial, + org, + teamSelection, + projectSelection + ); }); } @@ -86,6 +97,9 @@ async function withPreflightHandling( ui.log.error(message); ui.cancel("Setup failed."); ui.feedback("failed"); + if (isRecoverableOAuthScopeError(error)) { + throw error; + } throw error instanceof WizardError ? error : new WizardError(message); } } @@ -93,7 +107,7 @@ async function withPreflightHandling( function buildResolvedInitContext( initial: WizardOptions, org: string, - team: string | undefined, + teamSelection: TeamSelection, selection: ProjectSelection ): ResolvedInitContext { return { @@ -102,7 +116,7 @@ function buildResolvedInitContext( dryRun: initial.dryRun, features: initial.features, org, - team, + ...teamSelection, isExplicitTeam: Boolean(initial.team), project: selection.project, app: initial.app, @@ -331,7 +345,7 @@ async function resolveTeam( org: string, initial: WizardOptions, ui: WizardUI -): Promise { +): Promise { if (!initial.team) { return await resolveImplicitTeam(org, initial, ui); } @@ -343,13 +357,18 @@ async function resolveTeam( dryRun: initial.dryRun, deferAutoCreateOnEmptyOrg: true, }); - return result.source === "deferred" ? undefined : result.slug; + return result.source === "deferred" + ? {} + : { team: result.slug, teamRoleScopes: result.roleScopes }; } catch (error) { if (error instanceof WizardCancelledError) { throw error; } if (error instanceof ApiError && error.status === 403) { - return; + if (isRecoverableOAuthScopeError(error)) { + throw error; + } + return {}; } throw toPreflightWizardError(error); } @@ -410,6 +429,9 @@ async function listTeamsForImplicitInit( // 403 from listTeams means the user cannot inspect team access. Continue // without a team so init mirrors onboarding's org-scoped auto-team path. if (error instanceof ApiError && error.status === 403) { + if (isRecoverableOAuthScopeError(error)) { + throw error; + } await assertOrgScopedCreationCanProceed(org); return; } @@ -424,19 +446,23 @@ async function resolveImplicitTeam( org: string, initial: WizardOptions, ui: WizardUI -): Promise { +): Promise { const teams = await listTeamsForImplicitInit(org); if (!teams) { - return; + return {}; } const candidateTeams = teams.filter(canCreateProjectInTeam); if (candidateTeams.length === 0) { await assertOrgScopedCreationCanProceed(org); - return; + return {}; } if (candidateTeams.length === 1 || initial.yes) { - return (candidateTeams[0] as SentryTeam).slug; + const team = candidateTeams[0] as SentryTeam; + return { + team: team.slug, + teamRoleScopes: Array.isArray(team.access) ? [...team.access] : undefined, + }; } const selected = await ui.select({ @@ -450,7 +476,16 @@ async function resolveImplicitTeam( if (isCancelled(selected)) { throw new WizardCancelledError(); } - return selected; + const selectedTeam = candidateTeams.find( + (candidate) => candidate.slug === selected + ); + return { + team: selected, + teamRoleScopes: + selectedTeam && Array.isArray(selectedTeam.access) + ? [...selectedTeam.access] + : undefined, + }; } /** @@ -462,6 +497,9 @@ async function resolveImplicitTeam( * re-authenticating will. */ function handleOrgListError(error: unknown): { ok: false; error: string } { + if (isRecoverableOAuthScopeError(error)) { + throw error; + } if (error instanceof ApiError && error.status === 403) { const lines: string[] = ["Could not list organizations (403 Forbidden)."]; if (error.detail) { diff --git a/packages/cli/src/lib/init/tools/create-sentry-project.ts b/packages/cli/src/lib/init/tools/create-sentry-project.ts index 4328d9666a..0003ab6281 100644 --- a/packages/cli/src/lib/init/tools/create-sentry-project.ts +++ b/packages/cli/src/lib/init/tools/create-sentry-project.ts @@ -13,7 +13,11 @@ import { createProjectWithDsn, MEMBER_PROJECT_CREATION_DISABLED_DETAIL, } from "../../api-client.js"; -import { ApiError } from "../../errors.js"; +import { + ApiError, + isRecoverableOAuthScopeError, + withRequiredScopes, +} from "../../errors.js"; import { resolveOrCreateTeam } from "../../resolve-team.js"; import { slugify } from "../../utils.js"; import { tryGetExistingProjectData } from "../existing-project.js"; @@ -184,6 +188,26 @@ async function validateTeamForDryRun( } } +function getProjectScopeRecoveryError( + error: unknown, + context: Pick +): ApiError | undefined { + if (!(error instanceof ApiError && error.status === 403)) { + return; + } + if (isRecoverableOAuthScopeError(error)) { + return error; + } + if ( + context.team && + context.teamRoleScopes?.includes("team:admin") && + error.detail?.includes(MEMBER_PROJECT_CREATION_DISABLED_DETAIL) + ) { + return withRequiredScopes(error, ["team:admin"]); + } + return; +} + /** * Create a new Sentry project using the org that preflight already resolved. * When preflight does not resolve a Team Admin team, creation uses the same @@ -200,7 +224,13 @@ export async function createSentryProject( payload: CreateSentryProjectPayload | EnsureSentryProjectPayload, context: Pick< ToolContext, - "dryRun" | "existingProject" | "isExplicitTeam" | "org" | "team" | "project" + | "dryRun" + | "existingProject" + | "isExplicitTeam" + | "org" + | "team" + | "teamRoleScopes" + | "project" > ): Promise { const name = context.project ?? payload.params.name; @@ -268,6 +298,17 @@ export async function createSentryProject( }, }; } catch (error) { + // Let the command-level OAuth recovery restart the wizard after a fresh + // authorization. Standard insufficient-scope responses already carry + // structured metadata from WWW-Authenticate. For an implicitly selected + // team, preflight only supplies `context.team` when the user's effective + // role grants team:admin, so the otherwise ambiguous policy 403 identifies + // an older OAuth grant that is missing that scope. + const recoveryError = getProjectScopeRecoveryError(error, context); + if (recoveryError) { + throw recoveryError; + } + // Org-level policy: member project creation is disabled on this org. // Surface a clear message with the escape hatch. if ( diff --git a/packages/cli/src/lib/init/tools/registry.ts b/packages/cli/src/lib/init/tools/registry.ts index c2ca1a92d2..56bc62424f 100644 --- a/packages/cli/src/lib/init/tools/registry.ts +++ b/packages/cli/src/lib/init/tools/registry.ts @@ -1,3 +1,4 @@ +import { isRecoverableOAuthScopeError } from "../../errors.js"; import type { ToolOperation, ToolPayload, ToolResult } from "../types.js"; import { applyPatchsetTool } from "./apply-patchset.js"; import { @@ -62,6 +63,9 @@ export async function executeTool( try { return await tool.execute(payload as never, context); } catch (error) { + if (isRecoverableOAuthScopeError(error)) { + throw error; + } return { ok: false, error: formatToolError(error) }; } } diff --git a/packages/cli/src/lib/init/types.ts b/packages/cli/src/lib/init/types.ts index 7ea6a87219..0c9b30f24c 100644 --- a/packages/cli/src/lib/init/types.ts +++ b/packages/cli/src/lib/init/types.ts @@ -47,6 +47,8 @@ export type ResolvedInitContext = { * Omitted when init defers empty-org auto-creation until project creation. */ team?: string; + /** Scopes granted by the user's effective role on the resolved team. */ + teamRoleScopes?: readonly string[]; /** * True only when `team` was supplied via the `--team` CLI flag. * False/absent when the team was auto-selected by preflight. diff --git a/packages/cli/src/lib/init/wizard-runner.ts b/packages/cli/src/lib/init/wizard-runner.ts index 10aa98c733..db37bd66b9 100644 --- a/packages/cli/src/lib/init/wizard-runner.ts +++ b/packages/cli/src/lib/init/wizard-runner.ts @@ -25,7 +25,12 @@ import { formatBanner } from "../banner.js"; import { CLI_VERSION } from "../constants.js"; import { customFetch } from "../custom-ca.js"; import { detectAgent } from "../detect-agent.js"; -import { ApiError, EXIT, WizardError } from "../errors.js"; +import { + ApiError, + EXIT, + isRecoverableOAuthScopeError, + WizardError, +} from "../errors.js"; import { renderInlineMarkdown, stripColorTags, @@ -1093,6 +1098,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise { } } catch (err) { const isAuthFailure = err instanceof ApiError && err.status === 401; + const isScopeFailure = isRecoverableOAuthScopeError(err); // A running spinner owns a live interval, so stop it before any early // return or rethrow to avoid leaving the event loop artificially busy. if (spinState.running) { @@ -1103,6 +1109,8 @@ export async function runWizard(initialOptions: WizardOptions): Promise { code = 0; } else if (isAuthFailure) { label = INIT_SERVICE_AUTH_FAILED_LABEL; + } else if (isScopeFailure) { + label = "Authorization update required"; } spin.stop(label, code); spinState.running = false; @@ -1125,6 +1133,11 @@ export async function runWizard(initialOptions: WizardOptions): Promise { setTag("wizard.outcome", "errored"); throw err; } + if (isScopeFailure) { + showFailedFeedback(ui, "Authorization update required"); + setTag("wizard.outcome", "errored"); + throw err; + } if (err instanceof WizardError) { showFailedFeedback(ui); setTag("wizard.outcome", "errored"); diff --git a/packages/cli/src/lib/resolve-team.ts b/packages/cli/src/lib/resolve-team.ts index 1a15b94da5..dd50e377b7 100644 --- a/packages/cli/src/lib/resolve-team.ts +++ b/packages/cli/src/lib/resolve-team.ts @@ -6,7 +6,7 @@ * * ## Resolution flow * - * 1. Explicit `--team` flag → use as-is, no validation + * 1. Explicit `--team` flag → use as-is, with best-effort role lookup * 2. Fetch org teams via `listTeams` * - On 404: org doesn't exist → resolve effective org via cache, show org list * - On other errors: surface status + generic hint @@ -28,6 +28,7 @@ import { AuthError, CliError, ContextError, + isRecoverableOAuthScopeError, ResolutionError, } from "./errors.js"; import { resolveEffectiveOrg } from "./region.js"; @@ -91,6 +92,8 @@ export type ResolvedConcreteTeam = { slug: string; /** How the team was determined */ source: "explicit" | "auto-selected" | "auto-created"; + /** Scopes granted by the user's effective role on this team, when known. */ + roleScopes?: readonly string[]; }; /** Result of init-specific deferred team resolution for empty organizations. */ @@ -167,7 +170,7 @@ export async function resolveOrCreateTeam( options: ResolveTeamOptions ): Promise { if (options.team) { - return { slug: options.team, source: "explicit" }; + return await resolveExplicitTeam(orgSlug, options.team); } let teams: SentryTeam[]; @@ -184,7 +187,7 @@ export async function resolveOrCreateTeam( // Single team — auto-select if (teams.length === 1) { - return { slug: (teams[0] as SentryTeam).slug, source: "auto-selected" }; + return resolvedListedTeam(teams[0] as SentryTeam); } // Multiple teams — prefer teams the user belongs to @@ -192,16 +195,16 @@ export async function resolveOrCreateTeam( const candidates = memberTeams.length > 0 ? memberTeams : teams; if (candidates.length === 1) { - return { - slug: (candidates[0] as SentryTeam).slug, - source: "auto-selected", - }; + return resolvedListedTeam(candidates[0] as SentryTeam); } // Multiple candidates — let caller choose or throw if (options.onAmbiguous) { const slug = await options.onAmbiguous(candidates); - return { slug, source: "auto-selected" }; + const selected = candidates.find((team) => team.slug === slug); + return selected + ? resolvedListedTeam(selected) + : { slug, source: "auto-selected" }; } const label = @@ -218,6 +221,40 @@ export async function resolveOrCreateTeam( ); } +function resolvedListedTeam( + team: SentryTeam, + source: ResolvedConcreteTeam["source"] = "auto-selected" +): ResolvedConcreteTeam { + return { + slug: team.slug, + source, + ...(Array.isArray(team.access) ? { roleScopes: [...team.access] } : {}), + }; +} + +async function resolveExplicitTeam( + orgSlug: string, + teamSlug: string +): Promise { + const fallback: ResolvedConcreteTeam = { + slug: teamSlug, + source: "explicit", + }; + try { + const teams = await listTeams(orgSlug); + const team = teams.find((candidate) => candidate.slug === teamSlug); + return team ? resolvedListedTeam(team, "explicit") : fallback; + } catch (error) { + if (isRecoverableOAuthScopeError(error)) { + throw error; + } + // Explicit team resolution historically made no list request. Capability + // lookup is best-effort so missing team:read or a transient list failure + // cannot prevent the create endpoint from returning its precise result. + return fallback; + } +} + /** * Handle the case when an org has zero teams. * Either defers init-specific creation, auto-creates a team, returns a dry-run @@ -253,7 +290,11 @@ async function autoCreateTeam( ): Promise { try { const team = await createTeam(orgSlug, slug); - return { slug: team.slug, source: "auto-created" }; + return { + slug: team.slug, + source: "auto-created", + ...(Array.isArray(team.access) ? { roleScopes: [...team.access] } : {}), + }; } catch (error) { // Let auth errors propagate so the central handler can trigger auto-login if (error instanceof AuthError) { diff --git a/packages/cli/src/lib/scope-recovery.ts b/packages/cli/src/lib/scope-recovery.ts index 91133094fd..6a81f7881b 100644 --- a/packages/cli/src/lib/scope-recovery.ts +++ b/packages/cli/src/lib/scope-recovery.ts @@ -4,17 +4,16 @@ */ import { isatty } from "node:tty"; -import { extractRequiredScopes } from "./api-scope.js"; import { assertAutoLoginHostTrusted } from "./auto-auth.js"; import { type AuthSource, getAuthConfig } from "./db/auth.js"; -import { ApiError } from "./errors.js"; +import { ApiError, getRequiredOAuthScopes } from "./errors.js"; import type { InteractiveLoginOptions, LoginResult, } from "./interactive-login.js"; import { interactivePromptsAllowed } from "./interactive-prompts.js"; import { logger } from "./logger.js"; -import { OAUTH_SCOPES, resolveOAuthScopeString } from "./oauth.js"; +import { OAUTH_SCOPES } from "./oauth.js"; type InteractiveLogin = ( options?: InteractiveLoginOptions @@ -79,7 +78,7 @@ function recoverableScopes( return null; } - const scopes = extractRequiredScopes(error.detail); + const scopes = getRequiredOAuthScopes(error); return scopes.length > 0 ? scopes : null; } @@ -112,7 +111,11 @@ export async function runWithScopeRecovery( runtime.write("\n"); const merged = [...new Set([...OAUTH_SCOPES, ...scopes])]; - const requestedScope = resolveOAuthScopeString({ scopes: merged }); + // Structured scopes come from either the configured host's RFC 6750 + // challenge or trusted endpoint-specific context. Do not validate them + // against this CLI's baked-in list: a newer server may introduce a scope + // before the CLI is released again, and recovery should still work. + const requestedScope = merged.join(" "); const loginResult = await runInteractiveLogin({ scope: requestedScope }); if (!loginResult) { throw error; diff --git a/packages/cli/test/commands/project/create.test.ts b/packages/cli/test/commands/project/create.test.ts index 55609bfad2..cd6b626262 100644 --- a/packages/cli/test/commands/project/create.test.ts +++ b/packages/cli/test/commands/project/create.test.ts @@ -273,15 +273,14 @@ describe("project create", () => { ); }); - test("passes --team to skip team auto-detection", async () => { + test("passes --team after a best-effort role capability lookup", async () => { listTeamsSpy.mockResolvedValue([sampleTeam, sampleTeam2]); const { context } = createMockContext(); const func = await createCommand.loader(); await func.call(context, { team: "mobile", json: false }, "my-app:go"); - // listTeams should NOT be called when --team is explicit - expect(listTeamsSpy).not.toHaveBeenCalled(); + expect(listTeamsSpy).toHaveBeenCalledOnce(); expect(createProjectWithDsnSpy).toHaveBeenCalledWith( "acme-corp", "mobile", @@ -676,6 +675,55 @@ describe("project create", () => { expect(err.message).toContain("disabled project creation for members"); }); + test("marks a Team Admin policy 403 as a recoverable OAuth scope error", async () => { + listTeamsSpy.mockResolvedValueOnce([ + { ...sampleTeam, access: ["team:admin"] }, + ]); + createProjectWithDsnSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "Your organization has disabled this feature for members." + ) + ); + + const { context } = createMockContext(); + const func = await createCommand.loader(); + const error = (await func + .call(context, { json: false }, "my-app:node") + .catch((caught: unknown) => caught)) as ApiError; + + expect(error).toBeInstanceOf(ApiError); + expect(error.requiredScopes).toEqual(["team:admin"]); + expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); + }); + + test("recovers a known Team Admin selected with --team", async () => { + listTeamsSpy.mockResolvedValueOnce([ + { + ...sampleTeam, + slug: "platform", + access: ["team:read", "team:admin"], + }, + ]); + createProjectWithDsnSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "Your organization has disabled this feature for members." + ) + ); + + const { context } = createMockContext(); + const func = await createCommand.loader(); + const error = (await func + .call(context, { json: false, team: "platform" }, "my-app:node") + .catch((caught: unknown) => caught)) as ApiError; + + expect(error).toBeInstanceOf(ApiError); + expect(error.requiredScopes).toEqual(["team:admin"]); + }); + test("outputs JSON when --json flag is set", async () => { const { context, stdoutWrite } = createMockContext(); const func = await createCommand.loader(); diff --git a/packages/cli/test/lib/api-scope.test.ts b/packages/cli/test/lib/api-scope.test.ts index e983e843ec..cfdb5044ec 100644 --- a/packages/cli/test/lib/api-scope.test.ts +++ b/packages/cli/test/lib/api-scope.test.ts @@ -5,7 +5,83 @@ */ import { describe, expect, test } from "vitest"; -import { extractRequiredScopes } from "../../src/lib/api-scope.js"; +import { + extractRequiredScopes, + extractRequiredScopesFromWwwAuthenticate, +} from "../../src/lib/api-scope.js"; + +describe("extractRequiredScopesFromWwwAuthenticate", () => { + test("extracts scopes from an RFC 6750 insufficient-scope challenge", () => { + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Bearer error="insufficient_scope", scope="team:admin project:write"' + ) + ).toEqual(["team:admin", "project:write"]); + }); + + test("accepts a future scope because the server challenge is authoritative", () => { + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Bearer error="insufficient_scope", scope="project:new-capability"' + ) + ).toEqual(["project:new-capability"]); + }); + + test("ignores unrelated or incomplete authentication challenges", () => { + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Bearer error="invalid_token", scope="team:admin"' + ) + ).toEqual([]); + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Basic realm="sentry", error="insufficient_scope", scope="team:admin"' + ) + ).toEqual([]); + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Bearer error="insufficient_scope"' + ) + ).toEqual([]); + }); + + test("deduplicates scopes while preserving server order", () => { + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Bearer scope="team:admin team:admin org:read", error=insufficient_scope' + ) + ).toEqual(["team:admin", "org:read"]); + }); + + test.each([ + [ + 'Basic realm="legacy", Bearer error="insufficient_scope", scope="team:admin"', + ], + [ + 'Bearer error="insufficient_scope", scope="team:admin", Basic realm="legacy"', + ], + ])("isolates Bearer parameters in combined challenges", (header) => { + expect(extractRequiredScopesFromWwwAuthenticate(header)).toEqual([ + "team:admin", + ]); + }); + + test("does not borrow a scope from a later authentication challenge", () => { + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Bearer error="insufficient_scope", Custom scope="team:admin"' + ) + ).toEqual([]); + }); + + test("ignores challenge-like text inside a quoted parameter", () => { + expect( + extractRequiredScopesFromWwwAuthenticate( + 'Basic realm="legacy, Bearer bogus", Bearer error="insufficient_scope", scope="project:new-capability"' + ) + ).toEqual(["project:new-capability"]); + }); +}); describe("extractRequiredScopes", () => { test("returns [] for undefined or null detail", () => { @@ -31,6 +107,12 @@ describe("extractRequiredScopes", () => { ).toEqual([]); }); + test("ignores a role requirement that merely names a scope", () => { + expect( + extractRequiredScopes("This action requires the team:admin role.") + ).toEqual([]); + }); + test("extracts a single scope from a detail string", () => { expect( extractRequiredScopes( diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 0211b0b556..83e3749255 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { + apiRequestToRegion, isTextualContentType, rawApiRequest, throwApiError, @@ -73,6 +74,31 @@ describe("throwApiError", () => { } }); + test("preserves required scopes from WWW-Authenticate as structured metadata", () => { + const mockResponse = new Response("", { + status: 403, + statusText: "Forbidden", + headers: { + "www-authenticate": + 'Bearer error="insufficient_scope", scope="team:admin"', + }, + }); + + try { + throwApiError( + { detail: "You do not have permission to perform this action." }, + mockResponse, + "Failed to create project" + ); + } catch (error) { + const apiError = error as ApiError; + expect(apiError.requiredScopes).toEqual(["team:admin"]); + expect(apiError.detail).toContain( + "missing the required scope(s) 'team:admin'" + ); + } + }); + test("HTTP error without detail uses stringified error", () => { const mockResponse = new Response("", { status: 500, @@ -670,3 +696,46 @@ describe("rawApiRequest binary handling", () => { expect(result.body).toBe("not json"); }); }); + +describe("apiRequestToRegion error metadata", () => { + useTestConfigDir("regional-api-scope-"); + + let originalFetch: typeof globalThis.fetch; + + beforeEach(async () => { + originalFetch = globalThis.fetch; + await setAuthToken("test-token"); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + }); + + test("preserves an insufficient-scope challenge from a raw API response", async () => { + globalThis.fetch = mockFetch( + async () => + new Response( + JSON.stringify({ + detail: "You do not have permission to perform this action.", + }), + { + status: 403, + statusText: "Forbidden", + headers: { + "content-type": "application/json", + "www-authenticate": + 'Bearer error="insufficient_scope", scope="team:admin"', + }, + } + ) + ); + + const error = await apiRequestToRegion( + "https://sentry.io", + "organizations/acme/projects/" + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); + }); +}); diff --git a/packages/cli/test/lib/errors.test.ts b/packages/cli/test/lib/errors.test.ts index ea91399cd5..072a9391fc 100644 --- a/packages/cli/test/lib/errors.test.ts +++ b/packages/cli/test/lib/errors.test.ts @@ -7,6 +7,7 @@ import { CliError, ConfigError, ContextError, + cloneApiError, DeviceFlowError, EXIT, formatError, @@ -97,6 +98,25 @@ describe("ApiError", () => { const err = new ApiError("Request failed", 503); expect(err.format()).toBe("Request failed"); }); + + test("cloneApiError preserves structured scope metadata", () => { + const original = new ApiError( + "Forbidden", + 403, + "Insufficient scope", + "/api/0/projects/", + true, + ["project:new-capability"] + ); + + const cloned = cloneApiError(original, { message: "Project list failed" }); + + expect(cloned.message).toBe("Project list failed"); + expect(cloned.requiredScopes).toEqual(["project:new-capability"]); + expect(cloned.detail).toBe(original.detail); + expect(cloned.endpoint).toBe(original.endpoint); + expect(cloned.enriched403).toBe(true); + }); }); describe("ConfigError", () => { diff --git a/packages/cli/test/lib/init/preflight.test.ts b/packages/cli/test/lib/init/preflight.test.ts index 8d82c3734e..993fdcde9b 100644 --- a/packages/cli/test/lib/init/preflight.test.ts +++ b/packages/cli/test/lib/init/preflight.test.ts @@ -525,6 +525,7 @@ describe("resolveInitContext", () => { resolveOrCreateTeamSpy.mockResolvedValue({ slug: "backend", source: "explicit", + roleScopes: ["team:read", "team:admin"], } as any); const { ui } = createMockUI(); @@ -535,6 +536,7 @@ describe("resolveInitContext", () => { expect(context?.isExplicitTeam).toBe(true); expect(context?.team).toBe("backend"); + expect(context?.teamRoleScopes).toEqual(["team:read", "team:admin"]); }); test("sets isExplicitTeam:false when no --team flag is provided", async () => { @@ -559,6 +561,24 @@ describe("resolveInitContext", () => { expect(resolveOrCreateTeamSpy).not.toHaveBeenCalled(); }); + test("preserves a structured insufficient-scope error from preflight", async () => { + const scopeError = new ApiError( + "Forbidden", + 403, + "Insufficient scope", + undefined, + true, + ["team:read"] + ); + listTeamsSpy.mockRejectedValueOnce(scopeError); + + const { ui } = createMockUI(); + + await expect(resolveInitContext(makeOptions(), ui)).rejects.toBe( + scopeError + ); + }); + test("preserves rich org-not-found guidance when implicit team lookup returns 404", async () => { resolveOrgPrefetchedSpy.mockResolvedValueOnce({ org: "missing-org" }); listOrganizationsSpy.mockResolvedValueOnce([ diff --git a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts index 64680db448..d8d64dfbad 100644 --- a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts +++ b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts @@ -18,6 +18,7 @@ import { createSentryProject, createSentryProjectTool, } from "../../../../src/lib/init/tools/create-sentry-project.js"; +import { executeTool } from "../../../../src/lib/init/tools/registry.js"; import type { CreateSentryProjectPayload, EnsureSentryProjectPayload, @@ -344,7 +345,76 @@ describe("createSentryProject", () => { expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); }); - test("does not fall back on team-scoped policy 403", async () => { + test("surfaces an implicit Team Admin policy 403 for OAuth recovery", async () => { + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithDsnSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "Your organization has disabled this feature for members." + ) + ); + + const error = await createSentryProject(makePayload(), { + dryRun: false, + org: "acme", + team: "platform", + teamRoleScopes: ["team:read", "team:admin"], + project: undefined, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); + expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); + }); + + test("lets a recoverable scope error escape the real tool registry", async () => { + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithDsnSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "Your organization has disabled this feature for members." + ) + ); + + const error = await executeTool(makePayload(), { + directory: "/tmp/test", + yes: false, + dryRun: false, + org: "acme", + team: "platform", + teamRoleScopes: ["team:read", "team:admin"], + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); + }); + + test("lets a legacy detail-only scope error escape the real tool registry", async () => { + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithDsnSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "You do not have the required scope: project:admin" + ) + ); + + const error = await executeTool(makePayload(), { + directory: "/tmp/test", + yes: false, + dryRun: false, + org: "acme", + team: "platform", + isExplicitTeam: true, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).detail).toContain("project:admin"); + }); + + test("keeps an explicit-team policy 403 as a role error", async () => { getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); createProjectWithDsnSpy.mockRejectedValueOnce( new ApiError( @@ -358,14 +428,37 @@ describe("createSentryProject", () => { dryRun: false, org: "acme", team: "platform", + isExplicitTeam: true, project: undefined, }); expect(result.ok).toBe(false); - expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); expect(result.error).toContain("disabled for members"); }); + test("recovers an explicit team when its known role grants Team Admin", async () => { + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithDsnSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "Your organization has disabled this feature for members." + ) + ); + + const error = await createSentryProject(makePayload(), { + dryRun: false, + org: "acme", + team: "platform", + teamRoleScopes: ["team:read", "team:admin"], + isExplicitTeam: true, + project: undefined, + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); + }); + test("surfaces friendly 409 error when fallback project already exists", async () => { createProjectWithAutoTeamSpy.mockRejectedValueOnce( new ApiError("Conflict", 409, "Slug already in use") diff --git a/packages/cli/test/lib/init/wizard-runner.test.ts b/packages/cli/test/lib/init/wizard-runner.test.ts index 47346a619a..5aebeb7d61 100644 --- a/packages/cli/test/lib/init/wizard-runner.test.ts +++ b/packages/cli/test/lib/init/wizard-runner.test.ts @@ -721,6 +721,39 @@ describe("runWizard", () => { expect(lastFeedbackOutcome()).toBe("failed"); }); + test("preserves recoverable scope errors thrown by a tool", async () => { + const payload: ToolPayload = { + type: "tool", + operation: "create-sentry-project", + cwd: "/tmp/test", + params: { name: "my-app", platform: "javascript-react" }, + }; + mockStartResult = { + status: "suspended", + suspended: [["ensure-sentry-project"]], + steps: { + "ensure-sentry-project": { suspendPayload: payload }, + }, + }; + const scopeError = new ApiError( + "Forbidden", + 403, + "Insufficient scope", + undefined, + true, + ["team:admin"] + ); + executeToolSpy.mockRejectedValue(scopeError); + + await expect(runWizard(makeOptions())).rejects.toBe(scopeError); + + expect(spinnerMock.stop).toHaveBeenCalledWith( + "Authorization update required", + 1 + ); + expect(lastCancelMessage()).toBe("Authorization update required"); + }); + test("tears down forwarding and stops the spinner on cancellation", async () => { const captureSpy = vi.spyOn(Sentry, "captureException"); const payload: ToolPayload = { diff --git a/packages/cli/test/lib/resolve-team.test.ts b/packages/cli/test/lib/resolve-team.test.ts index 5c98c94154..b34a2ba02d 100644 --- a/packages/cli/test/lib/resolve-team.test.ts +++ b/packages/cli/test/lib/resolve-team.test.ts @@ -40,4 +40,47 @@ describe("resolveOrCreateTeam", () => { expect(error.status).toBe(401); expect(error.detail).toContain("over its member limit"); }); + + test("preserves effective team role scopes for permission recovery", async () => { + listTeamsSpy.mockResolvedValueOnce([ + { + id: "1", + slug: "platform", + name: "Platform", + access: ["team:read", "team:admin"], + }, + ]); + + const result = await resolveOrCreateTeam("acme", { + usageHint: "sentry project create", + }); + + expect(result).toEqual({ + slug: "platform", + source: "auto-selected", + roleScopes: ["team:read", "team:admin"], + }); + }); + + test("best-effort resolves role scopes for an explicit team", async () => { + listTeamsSpy.mockResolvedValueOnce([ + { + id: "1", + slug: "platform", + name: "Platform", + access: ["team:read", "team:admin"], + }, + ]); + + const result = await resolveOrCreateTeam("acme", { + team: "platform", + usageHint: "sentry project create", + }); + + expect(result).toEqual({ + slug: "platform", + source: "explicit", + roleScopes: ["team:read", "team:admin"], + }); + }); }); diff --git a/packages/cli/test/lib/scope-recovery.test.ts b/packages/cli/test/lib/scope-recovery.test.ts index 2e5a2b6f45..92aa25cc91 100644 --- a/packages/cli/test/lib/scope-recovery.test.ts +++ b/packages/cli/test/lib/scope-recovery.test.ts @@ -9,7 +9,10 @@ function missingScopeError(): ApiError { return new ApiError( "Forbidden", 403, - "You do not have the required scope: team:admin" + "You do not have permission to perform this action.", + undefined, + true, + ["team:admin"] ); } @@ -57,6 +60,53 @@ describe("runWithScopeRecovery", () => { ); }); + test("keeps response-detail parsing as a legacy server fallback", async () => { + const originalError = new ApiError( + "Forbidden", + 403, + "You do not have the required scope: project:admin" + ); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(originalError) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ + method: "oauth", + configPath: "/tmp/config", + }); + + await runWithScopeRecovery(proceed, [], login, runtime()); + + expect(login.mock.calls[0]?.[0]?.scope?.split(" ")).toContain( + "project:admin" + ); + }); + + test("can recover a scope introduced by a newer Sentry server", async () => { + const originalError = new ApiError( + "Forbidden", + 403, + "Insufficient scope", + undefined, + true, + ["project:new-capability"] + ); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(originalError) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ + method: "oauth", + configPath: "/tmp/config", + }); + + await runWithScopeRecovery(proceed, [], login, runtime()); + + expect(login.mock.calls[0]?.[0]?.scope?.split(" ")).toContain( + "project:new-capability" + ); + }); + test.each([ [["init", "--yes"], "oauth" as const], [["init", "-y"], "oauth" as const], From 7580e340e428d302ee1e3f9f117b2e9460e67f8b Mon Sep 17 00:00:00 2001 From: betegon Date: Mon, 10 Aug 2026 19:14:13 +0200 Subject: [PATCH 3/5] fix(auth): verify OAuth grants before reauthorization --- packages/cli/src/cli.ts | 7 +- packages/cli/src/commands/alert/list-utils.ts | 10 +- packages/cli/src/commands/cli/setup.ts | 22 ++ packages/cli/src/commands/cli/upgrade.ts | 47 ++- .../cli/src/commands/dashboard/resolve.ts | 12 +- packages/cli/src/commands/issue/list.ts | 28 +- packages/cli/src/commands/project/create.ts | 23 +- packages/cli/src/lib/api-scope.ts | 137 -------- packages/cli/src/lib/api/auth.ts | 21 ++ packages/cli/src/lib/api/infrastructure.ts | 44 +-- packages/cli/src/lib/errors.ts | 71 +---- packages/cli/src/lib/init/preflight.ts | 79 ++--- .../lib/init/tools/create-sentry-project.ts | 50 +-- packages/cli/src/lib/init/tools/registry.ts | 7 +- packages/cli/src/lib/init/types.ts | 2 - packages/cli/src/lib/init/wizard-runner.ts | 18 +- packages/cli/src/lib/resolve-team.ts | 59 +--- packages/cli/src/lib/scope-recovery.ts | 166 ++++++---- packages/cli/test/commands/cli/setup.test.ts | 45 +++ .../cli/test/commands/cli/upgrade.test.ts | 54 +++- .../cli/test/commands/project/create.test.ts | 54 +--- packages/cli/test/lib/api-scope.test.ts | 84 +---- packages/cli/test/lib/api/auth.test.ts | 53 ++++ .../cli/test/lib/api/infrastructure.test.ts | 69 ---- packages/cli/test/lib/errors.test.ts | 20 -- packages/cli/test/lib/init/preflight.test.ts | 32 +- .../init/tools/create-sentry-project.test.ts | 91 +----- .../cli/test/lib/init/wizard-runner.test.ts | 15 +- packages/cli/test/lib/resolve-team.test.ts | 43 --- packages/cli/test/lib/scope-recovery.test.ts | 300 +++++++++++------- 30 files changed, 680 insertions(+), 983 deletions(-) create mode 100644 packages/cli/src/lib/api/auth.ts create mode 100644 packages/cli/test/lib/api/auth.test.ts diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index d7e8a811b4..e589c0da2c 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -436,10 +436,9 @@ export async function runCli(cliArgs: string[]): Promise { /** * Scope recovery middleware. * - * Existing stored OAuth grants may predate the current standard scope set. - * On a scope-specific 403, offer one refresh with today's defaults and retry - * exactly once. Non-interactive and explicitly unattended commands never - * enter a device flow. + * After a 401/403, compare a stored OAuth grant with the CLI's current scope + * set. Re-authorize stale grants and retry exactly once. Non-interactive and + * explicitly unattended commands never enter a device flow. */ const scopeRecoveryMiddleware: ErrorMiddleware = async (next, argv) => { const { runWithScopeRecovery } = await import("./lib/scope-recovery.js"); diff --git a/packages/cli/src/commands/alert/list-utils.ts b/packages/cli/src/commands/alert/list-utils.ts index cb0c34dcee..5330713756 100644 --- a/packages/cli/src/commands/alert/list-utils.ts +++ b/packages/cli/src/commands/alert/list-utils.ts @@ -1,6 +1,6 @@ /** Shared helpers for alert list commands. */ -import { ApiError, cloneApiError, ValidationError } from "../../lib/errors.js"; +import { ApiError, ValidationError } from "../../lib/errors.js"; import { LIST_MAX_LIMIT } from "../../lib/list-command.js"; import { distributeFetchBudget, type FetchResult } from "../../lib/org-list.js"; @@ -42,7 +42,13 @@ export function throwAlertListFetchFailure( if (!(error instanceof ApiError)) { throw new Error(`${prefix}: ${error.message}`); } - throw cloneApiError(error, { message: `${prefix}: ${error.message}` }); + throw new ApiError( + `${prefix}: ${error.message}`, + error.status, + error.detail, + error.endpoint, + error.enriched403 + ); } export function buildAlertListFailureErrors( diff --git a/packages/cli/src/commands/cli/setup.ts b/packages/cli/src/commands/cli/setup.ts index f930a97d92..55b0842de3 100644 --- a/packages/cli/src/commands/cli/setup.ts +++ b/packages/cli/src/commands/cli/setup.ts @@ -51,6 +51,7 @@ type SetupFlags = { readonly "no-modify-path": boolean; readonly "no-completions": boolean; readonly "no-agent-skills": boolean; + readonly "ensure-auth-scopes": boolean; readonly quiet: boolean; }; @@ -499,6 +500,12 @@ export const setupCommand = buildCommand({ brief: "Skip agent skill installation for AI coding assistants", default: false, }, + "ensure-auth-scopes": { + kind: "boolean", + brief: "Refresh an outdated stored OAuth authorization", + default: false, + hidden: true as const, + }, quiet: { kind: "boolean", brief: "Suppress output (for scripted usage)", @@ -556,6 +563,21 @@ export const setupCommand = buildCommand({ warn, }); + if (flags["ensure-auth-scopes"]) { + await bestEffort( + "Authorization", + async () => { + const [{ runInteractiveLogin }, { ensureCurrentOAuthScopes }] = + await Promise.all([ + import("../../lib/interactive-login.js"), + import("../../lib/scope-recovery.js"), + ]); + await ensureCurrentOAuthScopes(runInteractiveLogin); + }, + warn + ); + } + // 5. Print welcome message only on fresh install — upgrades are silent // since the upgrade command itself prints a success message. if (!flags.quiet && freshInstall) { diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index 2b3fa1afb2..7fb997803d 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -54,6 +54,7 @@ import { VERSION_PREFIX_REGEX, versionExists, } from "../../lib/upgrade.js"; +import { whichSync } from "../../lib/which.js"; const log = logger.withTag("cli.upgrade"); @@ -428,7 +429,7 @@ async function spawnWithRetry( for (let attempt = 1; attempt <= SPAWN_MAX_ATTEMPTS; attempt++) { try { const proc = spawn(binaryPath, args, { - stdio: ["ignore", "inherit", "inherit"], + stdio: "inherit", env, }); return await new Promise((resolve, reject) => { @@ -493,6 +494,8 @@ type SetupOptions = { install: boolean; /** Pin the install directory (prevents relocation during upgrade) */ installDir?: string; + /** Ask the new binary to refresh a stored OAuth grant when scopes changed. */ + ensureAuthScopes: boolean; }; /** @@ -508,7 +511,8 @@ type SetupOptions = { * updates completions, agent skills, and records metadata. */ async function runSetupOnNewBinary(opts: SetupOptions): Promise { - const { binaryPath, method, channel, install, installDir } = opts; + const { binaryPath, method, channel, install, installDir, ensureAuthScopes } = + opts; const args = [ "cli", "setup", @@ -522,6 +526,9 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { if (install) { args.push("--install"); } + if (ensureAuthScopes) { + args.push("--ensure-auth-scopes"); + } const env = installDir ? { ...process.env, SENTRY_INSTALL_DIR: installDir } @@ -536,6 +543,14 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { } } +function resolveUpdatedCliPath( + execPath: string, + entryPath: string | undefined, + pathEnv: string | undefined +): string { + return whichSync("sentry", { PATH: pathEnv }) ?? entryPath ?? execPath; +} + /** * Execute the standard upgrade path: download via curl or package manager, * then run setup on the new binary. @@ -546,10 +561,22 @@ async function executeStandardUpgrade(opts: { versionArg: string | undefined; target: string; execPath: string; + entryPath?: string; + pathEnv?: string; offline?: OfflineMode; json?: boolean; }): Promise { - const { method, channel, versionArg, target, execPath, offline, json } = opts; + const { + method, + channel, + versionArg, + target, + execPath, + entryPath, + pathEnv, + offline, + json, + } = opts; // Use the rolling "nightly" tag only when upgrading to latest nightly // (no specific version was requested). A specific version arg always @@ -584,18 +611,21 @@ async function executeStandardUpgrade(opts: { channel, install: true, installDir: currentInstallDir, + ensureAuthScopes: !json, }); } finally { releaseLock(downloadResult.lockPath); } - } else if (method !== "brew") { - // Package manager: binary already in place, just run setup. - // Skip brew — Homebrew's post_install hook already runs setup. + } else { + // Package managers replace their PATH entry in place. Resolve it after the + // install so setup runs with the new CLI, not Node's process.execPath or a + // removed Homebrew keg path. await runSetupOnNewBinary({ - binaryPath: execPath, + binaryPath: resolveUpdatedCliPath(execPath, entryPath, pathEnv), method, channel, install: false, + ensureAuthScopes: !json, }); } } @@ -656,6 +686,7 @@ async function migrateToStandaloneForNightly( channel: "nightly", install: true, installDir, + ensureAuthScopes: !json, }); } finally { releaseLock(downloadResult.lockPath); @@ -923,6 +954,8 @@ export const upgradeCommand = buildCommand({ versionArg, target, execPath: this.process.execPath, + entryPath: this.process.argv?.[1], + pathEnv: this.process.env.PATH, offline, json: flags.json, }); diff --git a/packages/cli/src/commands/dashboard/resolve.ts b/packages/cli/src/commands/dashboard/resolve.ts index cc2aed4cea..d86e44eb11 100644 --- a/packages/cli/src/commands/dashboard/resolve.ts +++ b/packages/cli/src/commands/dashboard/resolve.ts @@ -14,7 +14,6 @@ import type { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { ApiError, ContextError, - cloneApiError, ResolutionError, ValidationError, } from "../../lib/errors.js"; @@ -793,12 +792,13 @@ export async function enrichDashboardError( error.status === 400 && (ctx.operation === "create" || ctx.operation === "update") ) { - throw cloneApiError(error, { - message: `Dashboard ${ctx.operation} failed in ${org}`, - detail: - error.detail ?? + throw new ApiError( + `Dashboard ${ctx.operation} failed in ${org}`, + error.status, + error.detail ?? "The API rejected the request. Check widget configuration.", - }); + error.endpoint + ); } throw error; diff --git a/packages/cli/src/commands/issue/list.ts b/packages/cli/src/commands/issue/list.ts index 1054111a0d..2f8205a27e 100644 --- a/packages/cli/src/commands/issue/list.ts +++ b/packages/cli/src/commands/issue/list.ts @@ -41,7 +41,6 @@ import { createDsnFingerprint } from "../../lib/dsn/index.js"; import { ApiError, ContextError, - cloneApiError, toSearchQueryError, ValidationError, withAuthGuard, @@ -918,9 +917,12 @@ function enrichIssueListError( } if (error instanceof ApiError) { if (error.status === 400) { - throw cloneApiError(error, { - detail: build400Detail(error.detail, flags), - }); + throw new ApiError( + error.message, + error.status, + build400Detail(error.detail, flags), + error.endpoint + ); } if (error.status === 403) { // Centralized 403 enrichment (infrastructure.ts) already added @@ -928,7 +930,13 @@ function enrichIssueListError( const detail = error.enriched403 ? appendProjectMembershipHint(error.detail) : build403Detail(error.detail); - throw cloneApiError(error, { detail, enriched403: true }); + throw new ApiError( + error.message, + error.status, + detail, + error.endpoint, + true + ); } } throw error; @@ -1146,11 +1154,13 @@ async function handleResolvedTargets( ? appendProjectMembershipHint(first.detail) : build403Detail(first.detail); } - throw cloneApiError(first, { - message: `${prefix}: ${first.message}`, + throw new ApiError( + `${prefix}: ${first.message}`, + first.status, detail, - enriched403: first.enriched403 || first.status === 403, - }); + first.endpoint, + first.enriched403 || first.status === 403 + ); } throw new Error(`${prefix}: ${first.message}`); diff --git a/packages/cli/src/commands/project/create.ts b/packages/cli/src/commands/project/create.ts index 9777bb5c67..9c34b7a47d 100644 --- a/packages/cli/src/commands/project/create.ts +++ b/packages/cli/src/commands/project/create.ts @@ -32,11 +32,9 @@ import { ApiError, CliError, ContextError, - cloneApiError, ResolutionError, ValidationError, withAuthGuard, - withRequiredScopes, } from "../../lib/errors.js"; import { formatProjectCreateOutput, @@ -285,8 +283,6 @@ type CreateProjectOpts = CreateProjectBaseOpts & { teamSlug: string; /** Source used to resolve the organization, when auto-detected. */ detectedFrom?: string; - /** Scopes granted by the user's effective role on the resolved team. */ - teamRoleScopes?: readonly string[]; }; /** @@ -370,9 +366,12 @@ function handleCreateApiError( // error-reporting.ts applies — e.g. a 403 "feature disabled for members" is a // permission issue, not a CLI bug. 5xx and network errors still get captured. // The message is kept short — ApiError.format() appends detail/endpoint. - throw cloneApiError(error, { - message: `Failed to create project '${name}' in ${orgSlug} (HTTP ${error.status}).`, - }); + throw new ApiError( + `Failed to create project '${name}' in ${orgSlug} (HTTP ${error.status}).`, + error.status, + error.detail, + error.endpoint + ); } /** @@ -392,14 +391,7 @@ async function createProjectWithErrors( if (error.status === 404) { return await handleCreateProject404(opts); } - const scopedError = - error.status === 403 && - error.requiredScopes.length === 0 && - opts.teamRoleScopes?.includes("team:admin") && - error.detail?.includes(MEMBER_PROJECT_CREATION_DISABLED_DETAIL) - ? withRequiredScopes(error, ["team:admin"]) - : error; - return handleCreateApiError(scopedError, opts); + return handleCreateApiError(error, opts); } } @@ -602,7 +594,6 @@ async function createOneProject(opts: { name, platform, detectedFrom, - teamRoleScopes: team.roleScopes, }); } catch (error) { // 403 means the user lacks permission to create or access teams, or to diff --git a/packages/cli/src/lib/api-scope.ts b/packages/cli/src/lib/api-scope.ts index dff34b92a3..3228669c09 100644 --- a/packages/cli/src/lib/api-scope.ts +++ b/packages/cli/src/lib/api-scope.ts @@ -44,19 +44,6 @@ export const SENTRY_SCOPES = [ "alerts:write", ] as const; -const OAUTH_SCOPE_TOKEN_RE = /^[\x21\x23-\x5b\x5d-\x7e]+$/; -const BEARER_CHALLENGE_RE = /^Bearer\s+(.+)$/i; -const AUTH_SCHEME_TOKEN_CHAR_RE = /^[A-Za-z0-9!#$%&'*+.^_`|~-]$/; -const SCOPE_SEPARATOR_RE = /\s+/; -const MISSING_BEFORE_SCOPE_RE = - /(?:missing|lacks?|without|insufficient|required)[^.\n]{0,80}scopes?/; -const SCOPE_BEFORE_MISSING_RE = - /scopes?[^.\n]{0,80}(?:missing|required|insufficient)/; -const TOKEN_MISSING_RE = - /(?:token|authorization|oauth)[^.\n]{0,120}(?:missing|lacks?|without|insufficient)/; -const EXPLICIT_SCOPE_REQUIREMENT_RE = - /(?:required\s*:|obtaining[^.\n]{0,80}scope)/; - // Explicit alternation (not `:` product) rejects nonexistent // combinations like `release:write` or `alerts:admin`. `:` is not a // regex metachar so no escaping needed. @@ -93,117 +80,6 @@ export function extractRequiredScopes(detail: unknown): string[] { return []; } -/** - * Extract required scopes from an RFC 6750 Bearer challenge. - * - * Unlike the legacy response-body parser above, this accepts valid scopes - * that a newer Sentry server may introduce after this CLI was released. The - * challenge is authoritative, so recovery can remain generic when the CLI's - * standard OAuth scope set grows in the future. - * - * @param header - The `WWW-Authenticate` response header - * @returns Deduplicated, source-ordered scopes for `insufficient_scope` - */ -export function extractRequiredScopesFromWwwAuthenticate( - header: string | null | undefined -): string[] { - if (!header) { - return []; - } - for (const challenge of splitAuthenticateChallenges(header)) { - const bearer = BEARER_CHALLENGE_RE.exec(challenge); - if (!bearer?.[1]) { - continue; - } - const params = bearer[1]; - const error = extractAuthParam(params, "error"); - if (error?.toLowerCase() !== "insufficient_scope") { - continue; - } - const scope = extractAuthParam(params, "scope"); - if (!scope) { - continue; - } - return [ - ...new Set( - scope - .split(SCOPE_SEPARATOR_RE) - .filter((candidate) => OAUTH_SCOPE_TOKEN_RE.test(candidate)) - ), - ]; - } - return []; -} - -function splitAuthenticateChallenges(header: string): string[] { - const challenges: string[] = []; - let challengeStart = 0; - let inQuotes = false; - let escaped = false; - - for (let index = 0; index < header.length; index += 1) { - const char = header[index]; - if (escaped) { - escaped = false; - continue; - } - if (char === "\\" && inQuotes) { - escaped = true; - continue; - } - if (char === '"') { - inQuotes = !inQuotes; - continue; - } - if (char !== "," || inQuotes) { - continue; - } - - const nextSchemeStart = findNextAuthSchemeStart(header, index + 1); - if (nextSchemeStart === undefined) { - continue; - } - challenges.push(header.slice(challengeStart, index).trim()); - challengeStart = nextSchemeStart; - } - - challenges.push(header.slice(challengeStart).trim()); - return challenges.filter(Boolean); -} - -function findNextAuthSchemeStart( - header: string, - offset: number -): number | undefined { - let cursor = offset; - while (header[cursor] === " " || header[cursor] === "\t") { - cursor += 1; - } - const tokenStart = cursor; - while ( - cursor < header.length && - AUTH_SCHEME_TOKEN_CHAR_RE.test(header[cursor] as string) - ) { - cursor += 1; - } - if ( - cursor > tokenStart && - (header[cursor] === " " || header[cursor] === "\t") - ) { - return tokenStart; - } - return; -} - -function extractAuthParam(header: string, name: string): string | undefined { - const pattern = new RegExp( - `(?:^|[,\\s])${name}\\s*=\\s*(?:"([^"]*)"|([^,\\s]+))`, - "i" - ); - const match = pattern.exec(header); - return match?.[1] ?? match?.[2]; -} - /** A role/policy denial can mention scope names without a token lacking them. */ function isMemberProjectCreationPolicy(detail: string): boolean { const normalized = detail.toLowerCase(); @@ -262,22 +138,9 @@ function matchesKnownScope(scope: string): boolean { } function extractFromText(text: string): string[] { - if (!describesMissingTokenScope(text)) { - return []; - } const matches = text.match(KNOWN_SCOPE_RE); if (!matches) { return []; } return [...new Set(matches.map((m) => m.toLowerCase()))]; } - -function describesMissingTokenScope(text: string): boolean { - const normalized = text.toLowerCase(); - return ( - MISSING_BEFORE_SCOPE_RE.test(normalized) || - SCOPE_BEFORE_MISSING_RE.test(normalized) || - TOKEN_MISSING_RE.test(normalized) || - EXPLICIT_SCOPE_REQUIREMENT_RE.test(normalized) - ); -} diff --git a/packages/cli/src/lib/api/auth.ts b/packages/cli/src/lib/api/auth.ts new file mode 100644 index 0000000000..142bec4740 --- /dev/null +++ b/packages/cli/src/lib/api/auth.ts @@ -0,0 +1,21 @@ +import { z } from "zod"; + +import { getControlSiloUrl } from "../sentry-client.js"; +import { apiRequestToRegion } from "./infrastructure.js"; + +const AuthStatusSchema = z.object({ + auth: z + .object({ + scopes: z.array(z.string()), + }) + .nullable(), +}); + +export async function getCurrentAuthScopes(): Promise< + readonly string[] | null +> { + const { data } = await apiRequestToRegion(getControlSiloUrl(), "", { + schema: AuthStatusSchema, + }); + return data.auth?.scopes ?? null; +} diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index bdec9b22ef..9664e5a57c 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -13,10 +13,7 @@ import { parseSentryLinkHeader } from "@sentry/api"; import * as Sentry from "@sentry/node-core/light"; import type { z } from "zod"; -import { - extractRequiredScopes, - extractRequiredScopesFromWwwAuthenticate, -} from "../api-scope.js"; +import { extractRequiredScopes } from "../api-scope.js"; import { getActiveEnvVarName, isEnvTokenActive } from "../db/auth.js"; import { getEnv } from "../env.js"; import { ApiError, AuthError, stringifyUnknown } from "../errors.js"; @@ -41,15 +38,9 @@ import { * - env-var tokens → suggest checking token scopes * - OAuth tokens → suggest re-authentication */ -function enrich403Detail( - rawDetail: string | undefined, - requiredScopes: readonly string[] = [] -): string { +function enrich403Detail(rawDetail: string | undefined): string { // Org-level policy — re-auth and token scope advice do not apply here. - if ( - requiredScopes.length === 0 && - rawDetail?.includes("disabled this feature") - ) { + if (rawDetail?.includes("disabled this feature")) { return [ rawDetail, "", @@ -63,10 +54,7 @@ function enrich403Detail( lines.push(rawDetail, ""); } - const scopes = - requiredScopes.length > 0 - ? [...requiredScopes] - : extractRequiredScopes(rawDetail); + const scopes = extractRequiredScopes(rawDetail); if (isEnvTokenActive()) { if (scopes.length > 0) { @@ -161,14 +149,10 @@ export function enrich401Detail(rawDetail: string | undefined): string { function enrichDetail( status: number, detail: string | undefined, - hasUsableDetail: boolean, - requiredScopes: readonly string[] = [] + hasUsableDetail: boolean ): string | undefined { if (status === 403) { - return enrich403Detail( - hasUsableDetail ? detail : undefined, - requiredScopes - ); + return enrich403Detail(hasUsableDetail ? detail : undefined); } if (status === 401) { return enrich401Detail(hasUsableDetail ? detail : undefined); @@ -246,16 +230,12 @@ export function throwApiError( : stringifyUnknown(error); const is403 = status === 403; - const requiredScopes = extractRequiredScopesFromWwwAuthenticate( - response.headers.get("www-authenticate") - ); throw new ApiError( `${context}: ${status} ${response.statusText ?? "Unknown"}`, status, - enrichDetail(status, detail, hasUsableDetail, requiredScopes), + enrichDetail(status, detail, hasUsableDetail), undefined, - is403, - requiredScopes + is403 ); } @@ -612,16 +592,12 @@ async function throwRawApiError( "www-authenticate": response.headers.get("www-authenticate"), }); const is403 = response.status === 403; - const requiredScopes = extractRequiredScopesFromWwwAuthenticate( - response.headers.get("www-authenticate") - ); throw new ApiError( `API request failed: ${response.status} ${response.statusText}`, response.status, - enrichDetail(response.status, detail, detail !== undefined, requiredScopes), + enrichDetail(response.status, detail, detail !== undefined), endpoint, - is403, - requiredScopes + is403 ); } diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index 03beb42202..666b297852 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -23,7 +23,6 @@ * @see https://cli.sentry.dev/exit-codes/ for full reference */ -import { extractRequiredScopes } from "./api-scope.js"; import { buildBillingUrl, buildOrgSettingsUrl, @@ -172,8 +171,6 @@ export class HostScopeError extends CliError { * @param status - HTTP status code * @param detail - Detailed error message from API response * @param endpoint - API endpoint that failed - * @param enriched403 - Whether centralized API handling enriched this 403 - * @param requiredScopes - OAuth scopes reported by an insufficient-scope challenge */ export class ApiError extends CliError { readonly status: number; @@ -187,17 +184,13 @@ export class ApiError extends CliError { */ readonly enriched403: boolean; - /** Trusted scope-recovery metadata from a server challenge or endpoint context. */ - readonly requiredScopes: readonly string[]; - - // biome-ignore lint/nursery/useMaxParams: established positional shape; new metadata is defaulted for compatibility + // biome-ignore lint/nursery/useMaxParams: established 4-param shape; enriched403 is a defaulted extension constructor( message: string, status: number, detail?: string, endpoint?: string, - enriched403 = false, - requiredScopes: readonly string[] = [] + enriched403 = false ) { super(message, EXIT.API); this.name = "ApiError"; @@ -205,7 +198,6 @@ export class ApiError extends CliError { this.detail = detail; this.endpoint = endpoint; this.enriched403 = enriched403; - this.requiredScopes = [...requiredScopes]; } override format(): string { @@ -220,65 +212,6 @@ export class ApiError extends CliError { } } -/** Metadata overrides accepted when cloning an API error. */ -export type ApiErrorOverrides = { - message?: string; - detail?: string; - enriched403?: boolean; - requiredScopes?: readonly string[]; -}; - -/** Clone an API error while preserving all recovery metadata by default. */ -export function cloneApiError( - error: ApiError, - overrides: ApiErrorOverrides = {} -): ApiError { - return new ApiError( - overrides.message ?? error.message, - error.status, - overrides.detail ?? error.detail, - error.endpoint, - overrides.enriched403 ?? error.enriched403, - overrides.requiredScopes ?? error.requiredScopes - ); -} - -/** - * Return an API error carrying additional authoritative OAuth scope metadata. - * - * Some endpoint-specific permission checks cannot emit an RFC 6750 challenge, - * but their caller may have enough role context to identify the missing token - * scope. Cloning keeps that contextual hint structured for the central - * recovery layer without rewriting response text. - * - * @param error - Original API error - * @param scopes - Required OAuth scopes established by the caller - * @returns An equivalent API error with merged scope metadata - */ -export function withRequiredScopes( - error: ApiError, - scopes: readonly string[] -): ApiError { - return cloneApiError(error, { - requiredScopes: [...new Set([...error.requiredScopes, ...scopes])], - }); -} - -/** Resolve authoritative or legacy required-scope metadata from an API error. */ -export function getRequiredOAuthScopes(error: unknown): string[] { - if (!(error instanceof ApiError && error.status === 403)) { - return []; - } - return error.requiredScopes.length > 0 - ? [...error.requiredScopes] - : extractRequiredScopes(error.detail); -} - -/** Whether an API error can be retried after expanding an OAuth grant. */ -export function isRecoverableOAuthScopeError(error: unknown): boolean { - return getRequiredOAuthScopes(error).length > 0; -} - export type AuthErrorReason = "not_authenticated" | "expired" | "invalid"; /** Options for AuthError */ diff --git a/packages/cli/src/lib/init/preflight.ts b/packages/cli/src/lib/init/preflight.ts index d90528e0af..16f9c49587 100644 --- a/packages/cli/src/lib/init/preflight.ts +++ b/packages/cli/src/lib/init/preflight.ts @@ -5,12 +5,9 @@ import { listTeams, } from "../api-client.js"; import { getAuthToken } from "../db/auth.js"; -import { - ApiError, - isRecoverableOAuthScopeError, - WizardError, -} from "../errors.js"; +import { ApiError, WizardError } from "../errors.js"; import { buildOrgNotFoundError, resolveOrCreateTeam } from "../resolve-team.js"; +import { currentOAuthGrantNeedsRefresh } from "../scope-recovery.js"; import { slugify } from "../utils.js"; import { WizardCancelledError } from "./clack-utils.js"; import { tryGetExistingProjectData } from "./existing-project.js"; @@ -42,8 +39,6 @@ type ProjectSelection = Pick< "project" | "existingProject" >; -type TeamSelection = Pick; - /** * Resolve org, project, team, and auth state before the init workflow starts. */ @@ -68,14 +63,9 @@ export async function resolveInitContext( return null; } - const teamSelection = await resolveTeam(org, initial, ui); + const team = await resolveTeam(org, initial, ui); - return buildResolvedInitContext( - initial, - org, - teamSelection, - projectSelection - ); + return buildResolvedInitContext(initial, org, team, projectSelection); }); } @@ -93,13 +83,17 @@ async function withPreflightHandling( return null; } + if ( + error instanceof ApiError && + (error.status === 401 || error.status === 403) + ) { + throw error; + } + const message = error instanceof Error ? error.message : String(error); ui.log.error(message); ui.cancel("Setup failed."); ui.feedback("failed"); - if (isRecoverableOAuthScopeError(error)) { - throw error; - } throw error instanceof WizardError ? error : new WizardError(message); } } @@ -107,7 +101,7 @@ async function withPreflightHandling( function buildResolvedInitContext( initial: WizardOptions, org: string, - teamSelection: TeamSelection, + team: string | undefined, selection: ProjectSelection ): ResolvedInitContext { return { @@ -116,7 +110,7 @@ function buildResolvedInitContext( dryRun: initial.dryRun, features: initial.features, org, - ...teamSelection, + team, isExplicitTeam: Boolean(initial.team), project: selection.project, app: initial.app, @@ -345,7 +339,7 @@ async function resolveTeam( org: string, initial: WizardOptions, ui: WizardUI -): Promise { +): Promise { if (!initial.team) { return await resolveImplicitTeam(org, initial, ui); } @@ -357,18 +351,16 @@ async function resolveTeam( dryRun: initial.dryRun, deferAutoCreateOnEmptyOrg: true, }); - return result.source === "deferred" - ? {} - : { team: result.slug, teamRoleScopes: result.roleScopes }; + return result.source === "deferred" ? undefined : result.slug; } catch (error) { if (error instanceof WizardCancelledError) { throw error; } if (error instanceof ApiError && error.status === 403) { - if (isRecoverableOAuthScopeError(error)) { + if (await currentOAuthGrantNeedsRefresh()) { throw error; } - return {}; + return; } throw toPreflightWizardError(error); } @@ -429,7 +421,7 @@ async function listTeamsForImplicitInit( // 403 from listTeams means the user cannot inspect team access. Continue // without a team so init mirrors onboarding's org-scoped auto-team path. if (error instanceof ApiError && error.status === 403) { - if (isRecoverableOAuthScopeError(error)) { + if (await currentOAuthGrantNeedsRefresh()) { throw error; } await assertOrgScopedCreationCanProceed(org); @@ -446,23 +438,19 @@ async function resolveImplicitTeam( org: string, initial: WizardOptions, ui: WizardUI -): Promise { +): Promise { const teams = await listTeamsForImplicitInit(org); if (!teams) { - return {}; + return; } const candidateTeams = teams.filter(canCreateProjectInTeam); if (candidateTeams.length === 0) { await assertOrgScopedCreationCanProceed(org); - return {}; + return; } if (candidateTeams.length === 1 || initial.yes) { - const team = candidateTeams[0] as SentryTeam; - return { - team: team.slug, - teamRoleScopes: Array.isArray(team.access) ? [...team.access] : undefined, - }; + return (candidateTeams[0] as SentryTeam).slug; } const selected = await ui.select({ @@ -476,16 +464,7 @@ async function resolveImplicitTeam( if (isCancelled(selected)) { throw new WizardCancelledError(); } - const selectedTeam = candidateTeams.find( - (candidate) => candidate.slug === selected - ); - return { - team: selected, - teamRoleScopes: - selectedTeam && Array.isArray(selectedTeam.access) - ? [...selectedTeam.access] - : undefined, - }; + return selected; } /** @@ -496,8 +475,14 @@ async function resolveImplicitTeam( * directly. 401: token is invalid/expired — supplying an org won't help, only * re-authenticating will. */ -function handleOrgListError(error: unknown): { ok: false; error: string } { - if (isRecoverableOAuthScopeError(error)) { +async function handleOrgListError( + error: unknown +): Promise<{ ok: false; error: string }> { + if ( + error instanceof ApiError && + (error.status === 401 || error.status === 403) && + (await currentOAuthGrantNeedsRefresh()) + ) { throw error; } if (error instanceof ApiError && error.status === 403) { @@ -537,7 +522,7 @@ async function resolveOrgSlug( try { orgs = await listOrganizations(); } catch (error) { - return handleOrgListError(error); + return await handleOrgListError(error); } if (orgs.length === 0) { return { diff --git a/packages/cli/src/lib/init/tools/create-sentry-project.ts b/packages/cli/src/lib/init/tools/create-sentry-project.ts index 0003ab6281..4431a19aed 100644 --- a/packages/cli/src/lib/init/tools/create-sentry-project.ts +++ b/packages/cli/src/lib/init/tools/create-sentry-project.ts @@ -13,12 +13,9 @@ import { createProjectWithDsn, MEMBER_PROJECT_CREATION_DISABLED_DETAIL, } from "../../api-client.js"; -import { - ApiError, - isRecoverableOAuthScopeError, - withRequiredScopes, -} from "../../errors.js"; +import { ApiError } from "../../errors.js"; import { resolveOrCreateTeam } from "../../resolve-team.js"; +import { currentOAuthGrantNeedsRefresh } from "../../scope-recovery.js"; import { slugify } from "../../utils.js"; import { tryGetExistingProjectData } from "../existing-project.js"; import { formatMemberProjectCreationDisabledError } from "../project-creation-errors.js"; @@ -188,24 +185,12 @@ async function validateTeamForDryRun( } } -function getProjectScopeRecoveryError( - error: unknown, - context: Pick -): ApiError | undefined { - if (!(error instanceof ApiError && error.status === 403)) { - return; - } - if (isRecoverableOAuthScopeError(error)) { - return error; - } - if ( - context.team && - context.teamRoleScopes?.includes("team:admin") && - error.detail?.includes(MEMBER_PROJECT_CREATION_DISABLED_DETAIL) - ) { - return withRequiredScopes(error, ["team:admin"]); - } - return; +async function shouldRefreshOAuth(error: unknown): Promise { + return ( + error instanceof ApiError && + (error.status === 401 || error.status === 403) && + (await currentOAuthGrantNeedsRefresh()) + ); } /** @@ -224,13 +209,7 @@ export async function createSentryProject( payload: CreateSentryProjectPayload | EnsureSentryProjectPayload, context: Pick< ToolContext, - | "dryRun" - | "existingProject" - | "isExplicitTeam" - | "org" - | "team" - | "teamRoleScopes" - | "project" + "dryRun" | "existingProject" | "isExplicitTeam" | "org" | "team" | "project" > ): Promise { const name = context.project ?? payload.params.name; @@ -298,15 +277,8 @@ export async function createSentryProject( }, }; } catch (error) { - // Let the command-level OAuth recovery restart the wizard after a fresh - // authorization. Standard insufficient-scope responses already carry - // structured metadata from WWW-Authenticate. For an implicitly selected - // team, preflight only supplies `context.team` when the user's effective - // role grants team:admin, so the otherwise ambiguous policy 403 identifies - // an older OAuth grant that is missing that scope. - const recoveryError = getProjectScopeRecoveryError(error, context); - if (recoveryError) { - throw recoveryError; + if (await shouldRefreshOAuth(error)) { + throw error; } // Org-level policy: member project creation is disabled on this org. diff --git a/packages/cli/src/lib/init/tools/registry.ts b/packages/cli/src/lib/init/tools/registry.ts index 56bc62424f..63892af392 100644 --- a/packages/cli/src/lib/init/tools/registry.ts +++ b/packages/cli/src/lib/init/tools/registry.ts @@ -1,4 +1,4 @@ -import { isRecoverableOAuthScopeError } from "../../errors.js"; +import { ApiError } from "../../errors.js"; import type { ToolOperation, ToolPayload, ToolResult } from "../types.js"; import { applyPatchsetTool } from "./apply-patchset.js"; import { @@ -63,7 +63,10 @@ export async function executeTool( try { return await tool.execute(payload as never, context); } catch (error) { - if (isRecoverableOAuthScopeError(error)) { + if ( + error instanceof ApiError && + (error.status === 401 || error.status === 403) + ) { throw error; } return { ok: false, error: formatToolError(error) }; diff --git a/packages/cli/src/lib/init/types.ts b/packages/cli/src/lib/init/types.ts index 0c9b30f24c..7ea6a87219 100644 --- a/packages/cli/src/lib/init/types.ts +++ b/packages/cli/src/lib/init/types.ts @@ -47,8 +47,6 @@ export type ResolvedInitContext = { * Omitted when init defers empty-org auto-creation until project creation. */ team?: string; - /** Scopes granted by the user's effective role on the resolved team. */ - teamRoleScopes?: readonly string[]; /** * True only when `team` was supplied via the `--team` CLI flag. * False/absent when the team was auto-selected by preflight. diff --git a/packages/cli/src/lib/init/wizard-runner.ts b/packages/cli/src/lib/init/wizard-runner.ts index db37bd66b9..5ef492164c 100644 --- a/packages/cli/src/lib/init/wizard-runner.ts +++ b/packages/cli/src/lib/init/wizard-runner.ts @@ -25,12 +25,7 @@ import { formatBanner } from "../banner.js"; import { CLI_VERSION } from "../constants.js"; import { customFetch } from "../custom-ca.js"; import { detectAgent } from "../detect-agent.js"; -import { - ApiError, - EXIT, - isRecoverableOAuthScopeError, - WizardError, -} from "../errors.js"; +import { ApiError, EXIT, WizardError } from "../errors.js"; import { renderInlineMarkdown, stripColorTags, @@ -1098,7 +1093,8 @@ export async function runWizard(initialOptions: WizardOptions): Promise { } } catch (err) { const isAuthFailure = err instanceof ApiError && err.status === 401; - const isScopeFailure = isRecoverableOAuthScopeError(err); + const isPermissionFailure = + err instanceof ApiError && (err.status === 401 || err.status === 403); // A running spinner owns a live interval, so stop it before any early // return or rethrow to avoid leaving the event loop artificially busy. if (spinState.running) { @@ -1109,8 +1105,8 @@ export async function runWizard(initialOptions: WizardOptions): Promise { code = 0; } else if (isAuthFailure) { label = INIT_SERVICE_AUTH_FAILED_LABEL; - } else if (isScopeFailure) { - label = "Authorization update required"; + } else if (isPermissionFailure) { + label = "Sentry API request denied"; } spin.stop(label, code); spinState.running = false; @@ -1133,8 +1129,8 @@ export async function runWizard(initialOptions: WizardOptions): Promise { setTag("wizard.outcome", "errored"); throw err; } - if (isScopeFailure) { - showFailedFeedback(ui, "Authorization update required"); + if (isPermissionFailure) { + showFailedFeedback(ui, "Sentry API request denied"); setTag("wizard.outcome", "errored"); throw err; } diff --git a/packages/cli/src/lib/resolve-team.ts b/packages/cli/src/lib/resolve-team.ts index dd50e377b7..1a15b94da5 100644 --- a/packages/cli/src/lib/resolve-team.ts +++ b/packages/cli/src/lib/resolve-team.ts @@ -6,7 +6,7 @@ * * ## Resolution flow * - * 1. Explicit `--team` flag → use as-is, with best-effort role lookup + * 1. Explicit `--team` flag → use as-is, no validation * 2. Fetch org teams via `listTeams` * - On 404: org doesn't exist → resolve effective org via cache, show org list * - On other errors: surface status + generic hint @@ -28,7 +28,6 @@ import { AuthError, CliError, ContextError, - isRecoverableOAuthScopeError, ResolutionError, } from "./errors.js"; import { resolveEffectiveOrg } from "./region.js"; @@ -92,8 +91,6 @@ export type ResolvedConcreteTeam = { slug: string; /** How the team was determined */ source: "explicit" | "auto-selected" | "auto-created"; - /** Scopes granted by the user's effective role on this team, when known. */ - roleScopes?: readonly string[]; }; /** Result of init-specific deferred team resolution for empty organizations. */ @@ -170,7 +167,7 @@ export async function resolveOrCreateTeam( options: ResolveTeamOptions ): Promise { if (options.team) { - return await resolveExplicitTeam(orgSlug, options.team); + return { slug: options.team, source: "explicit" }; } let teams: SentryTeam[]; @@ -187,7 +184,7 @@ export async function resolveOrCreateTeam( // Single team — auto-select if (teams.length === 1) { - return resolvedListedTeam(teams[0] as SentryTeam); + return { slug: (teams[0] as SentryTeam).slug, source: "auto-selected" }; } // Multiple teams — prefer teams the user belongs to @@ -195,16 +192,16 @@ export async function resolveOrCreateTeam( const candidates = memberTeams.length > 0 ? memberTeams : teams; if (candidates.length === 1) { - return resolvedListedTeam(candidates[0] as SentryTeam); + return { + slug: (candidates[0] as SentryTeam).slug, + source: "auto-selected", + }; } // Multiple candidates — let caller choose or throw if (options.onAmbiguous) { const slug = await options.onAmbiguous(candidates); - const selected = candidates.find((team) => team.slug === slug); - return selected - ? resolvedListedTeam(selected) - : { slug, source: "auto-selected" }; + return { slug, source: "auto-selected" }; } const label = @@ -221,40 +218,6 @@ export async function resolveOrCreateTeam( ); } -function resolvedListedTeam( - team: SentryTeam, - source: ResolvedConcreteTeam["source"] = "auto-selected" -): ResolvedConcreteTeam { - return { - slug: team.slug, - source, - ...(Array.isArray(team.access) ? { roleScopes: [...team.access] } : {}), - }; -} - -async function resolveExplicitTeam( - orgSlug: string, - teamSlug: string -): Promise { - const fallback: ResolvedConcreteTeam = { - slug: teamSlug, - source: "explicit", - }; - try { - const teams = await listTeams(orgSlug); - const team = teams.find((candidate) => candidate.slug === teamSlug); - return team ? resolvedListedTeam(team, "explicit") : fallback; - } catch (error) { - if (isRecoverableOAuthScopeError(error)) { - throw error; - } - // Explicit team resolution historically made no list request. Capability - // lookup is best-effort so missing team:read or a transient list failure - // cannot prevent the create endpoint from returning its precise result. - return fallback; - } -} - /** * Handle the case when an org has zero teams. * Either defers init-specific creation, auto-creates a team, returns a dry-run @@ -290,11 +253,7 @@ async function autoCreateTeam( ): Promise { try { const team = await createTeam(orgSlug, slug); - return { - slug: team.slug, - source: "auto-created", - ...(Array.isArray(team.access) ? { roleScopes: [...team.access] } : {}), - }; + return { slug: team.slug, source: "auto-created" }; } catch (error) { // Let auth errors propagate so the central handler can trigger auto-login if (error instanceof AuthError) { diff --git a/packages/cli/src/lib/scope-recovery.ts b/packages/cli/src/lib/scope-recovery.ts index 6a81f7881b..31616c1626 100644 --- a/packages/cli/src/lib/scope-recovery.ts +++ b/packages/cli/src/lib/scope-recovery.ts @@ -1,27 +1,22 @@ -/** - * One-time recovery for stored OAuth grants that predate the CLI's current - * standard scope set. - */ - import { isatty } from "node:tty"; +import { getCurrentAuthScopes } from "./api/auth.js"; import { assertAutoLoginHostTrusted } from "./auto-auth.js"; import { type AuthSource, getAuthConfig } from "./db/auth.js"; -import { ApiError, getRequiredOAuthScopes } from "./errors.js"; -import type { - InteractiveLoginOptions, - LoginResult, -} from "./interactive-login.js"; +import { ApiError, AuthError } from "./errors.js"; +import type { LoginResult } from "./interactive-login.js"; import { interactivePromptsAllowed } from "./interactive-prompts.js"; -import { logger } from "./logger.js"; import { OAUTH_SCOPES } from "./oauth.js"; -type InteractiveLogin = ( - options?: InteractiveLoginOptions -) => Promise; +type InteractiveLogin = () => Promise; + +type OAuthScopeState = + | { kind: "current" } + | { kind: "invalid" } + | { kind: "missing"; scopes: string[] }; export type ScopeRecoveryRuntime = { assertTrustedHost: () => void; - confirm: (message: string) => Promise; + getAuthScopes: () => Promise; getAuthSource: () => AuthSource | undefined; inputIsTty: () => boolean; promptsAllowed: () => boolean; @@ -30,17 +25,11 @@ export type ScopeRecoveryRuntime = { const defaultRuntime: ScopeRecoveryRuntime = { assertTrustedHost: assertAutoLoginHostTrusted, - confirm: (message) => - logger.withTag("auth").prompt(message, { - type: "confirm", - initial: true, - }), + getAuthScopes: getCurrentAuthScopes, getAuthSource: () => getAuthConfig()?.source, inputIsTty: () => isatty(0), promptsAllowed: interactivePromptsAllowed, - write: (message) => { - process.stderr.write(message); - }, + write: (message) => process.stderr.write(message), }; function disablesInteractiveRecovery(argv: string[]): boolean { @@ -54,70 +43,111 @@ function disablesInteractiveRecovery(argv: string[]): boolean { ); } -function recoverableScopes( - error: unknown, - argv: string[], - runtime: ScopeRecoveryRuntime -): string[] | null { - let authSource: AuthSource | undefined; +async function inspectOAuthScopes( + runtime: ScopeRecoveryRuntime, + startedWithOAuth = false +): Promise { try { - authSource = runtime.getAuthSource(); - } catch { - // Recovery must never replace the command's original 403 with a local - // credential-store read failure. - return null; + const source = runtime.getAuthSource(); + if (source !== "oauth") { + if (startedWithOAuth) { + return { kind: "invalid" }; + } + return; + } + const granted = await runtime.getAuthScopes(); + if (!granted) { + return { kind: "invalid" }; + } + const grantedSet = new Set(granted); + const missing = OAUTH_SCOPES.filter((scope) => !grantedSet.has(scope)); + return missing.length > 0 + ? { kind: "missing", scopes: missing } + : { kind: "current" }; + } catch (error) { + if ( + (error instanceof ApiError && error.status === 401) || + (error instanceof AuthError && + (error.reason === "expired" || error.reason === "not_authenticated")) + ) { + return { kind: "invalid" }; + } + return; + } +} + +/** Whether the active stored OAuth token is invalid or lacks a current CLI scope. */ +export async function currentOAuthGrantNeedsRefresh( + runtime: ScopeRecoveryRuntime = defaultRuntime +): Promise { + const state = await inspectOAuthScopes(runtime); + return Boolean(state && state.kind !== "current"); +} + +async function refreshOAuthScopes( + state: Exclude, + runInteractiveLogin: InteractiveLogin, + runtime: ScopeRecoveryRuntime +): Promise { + if (!(runtime.inputIsTty() && runtime.promptsAllowed())) { + return false; } - if ( - !(runtime.inputIsTty() && runtime.promptsAllowed()) || - disablesInteractiveRecovery(argv) || - authSource !== "oauth" || - !(error instanceof ApiError) || - error.status !== 403 - ) { - return null; + runtime.assertTrustedHost(); + if (state.kind === "missing") { + runtime.write( + `Your CLI authorization is missing ${state.scopes.join(", ")}. Starting authorization...\n\n` + ); + } else { + runtime.write( + "Your CLI authorization is no longer valid. Starting authorization...\n\n" + ); } + return Boolean(await runInteractiveLogin()); +} - const scopes = getRequiredOAuthScopes(error); - return scopes.length > 0 ? scopes : null; +/** Refresh a stored OAuth grant when it lacks any scope requested by this CLI. */ +export async function ensureCurrentOAuthScopes( + runInteractiveLogin: InteractiveLogin, + runtime: ScopeRecoveryRuntime = defaultRuntime +): Promise { + const state = await inspectOAuthScopes(runtime); + if (!state || state.kind === "current") { + return false; + } + return await refreshOAuthScopes(state, runInteractiveLogin, runtime); } -/** - * Run a command once and, for an old interactive OAuth grant, refresh it with - * the current standard scopes before retrying the command exactly once. - */ +/** Check an OAuth token after a 401/403, re-authorize if needed, and retry once. */ export async function runWithScopeRecovery( proceed: (commandArgs: string[]) => Promise, argv: string[], runInteractiveLogin: InteractiveLogin, runtime: ScopeRecoveryRuntime = defaultRuntime ): Promise { + let startedWithOAuth = false; + try { + startedWithOAuth = runtime.getAuthSource() === "oauth"; + } catch { + // The original command remains authoritative if the credential store fails. + } try { await proceed(argv); } catch (error) { - const scopes = recoverableScopes(error, argv, runtime); - if (!scopes) { - throw error; - } - - runtime.assertTrustedHost(); - const scopeList = scopes.map((scopeName) => `'${scopeName}'`).join(", "); - const confirmed = await runtime.confirm( - `Your existing CLI authorization is missing standard scope(s) ${scopeList}. Refresh it with the current defaults?` - ); - if (confirmed !== true) { + if ( + !(error instanceof ApiError) || + (error.status !== 401 && error.status !== 403) + ) { throw error; } - runtime.write("\n"); - const merged = [...new Set([...OAUTH_SCOPES, ...scopes])]; - // Structured scopes come from either the configured host's RFC 6750 - // challenge or trusted endpoint-specific context. Do not validate them - // against this CLI's baked-in list: a newer server may introduce a scope - // before the CLI is released again, and recovery should still work. - const requestedScope = merged.join(" "); - const loginResult = await runInteractiveLogin({ scope: requestedScope }); - if (!loginResult) { + const state = await inspectOAuthScopes(runtime, startedWithOAuth); + if ( + !state || + state.kind === "current" || + disablesInteractiveRecovery(argv) || + !(await refreshOAuthScopes(state, runInteractiveLogin, runtime)) + ) { throw error; } diff --git a/packages/cli/test/commands/cli/setup.test.ts b/packages/cli/test/commands/cli/setup.test.ts index 2127f3c40a..2c14669460 100644 --- a/packages/cli/test/commands/cli/setup.test.ts +++ b/packages/cli/test/commands/cli/setup.test.ts @@ -12,9 +12,28 @@ import { readFile } from "node:fs/promises"; import { join } from "node:path"; import { run } from "@stricli/core"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +vi.mock("../../../src/lib/interactive-login.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/lib/interactive-login.js") + >()), + runInteractiveLogin: vi.fn(), +})); + +vi.mock("../../../src/lib/scope-recovery.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/lib/scope-recovery.js") + >()), + ensureCurrentOAuthScopes: vi.fn(), +})); + import { app } from "../../../src/app.js"; import type { SentryContext } from "../../../src/context.js"; import { getReleaseChannel } from "../../../src/lib/db/release-channel.js"; +// biome-ignore lint/performance/noNamespaceImport: dynamic setup imports are mocked at the module boundary +import * as interactiveLogin from "../../../src/lib/interactive-login.js"; +// biome-ignore lint/performance/noNamespaceImport: dynamic setup imports are mocked at the module boundary +import * as scopeRecovery from "../../../src/lib/scope-recovery.js"; import { useTestConfigDir } from "../../helpers.js"; /** Store original fetch for restoration */ @@ -122,12 +141,14 @@ describe("sentry cli setup", () => { `setup-test-${Date.now()}-${Math.random().toString(36).slice(2)}` ); mkdirSync(testDir, { recursive: true }); + vi.mocked(scopeRecovery.ensureCurrentOAuthScopes).mockResolvedValue(false); }); afterEach(() => { restoreStderr?.(); restoreStderr = undefined; rmSync(testDir, { recursive: true, force: true }); + vi.mocked(scopeRecovery.ensureCurrentOAuthScopes).mockReset(); }); test("runs with --quiet and skips all output", async () => { @@ -153,6 +174,30 @@ describe("sentry cli setup", () => { expect(getOutput()).toBe(""); }); + test("checks OAuth scopes when invoked by the upgrade command", async () => { + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + await run( + app, + [ + "cli", + "setup", + "--quiet", + "--no-modify-path", + "--no-completions", + "--no-agent-skills", + "--ensure-auth-scopes", + ], + context + ); + + expect(scopeRecovery.ensureCurrentOAuthScopes).toHaveBeenCalledOnce(); + expect(scopeRecovery.ensureCurrentOAuthScopes).toHaveBeenCalledWith( + interactiveLogin.runInteractiveLogin + ); + }); + test("produces no welcome or completion output without --install", async () => { // Without --install, setup is being called for an upgrade or manual re-run. // Output is suppressed — the upgrade command itself prints success. diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 305cea75d2..f166c010a0 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -11,7 +11,7 @@ // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as child_process from "node:child_process"; -import { mkdirSync, rmSync } from "node:fs"; +import { chmodSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import { unlink } from "node:fs/promises"; import { join } from "node:path"; import { gzipSync } from "node:zlib"; @@ -61,6 +61,7 @@ function createMockContext( homeDir: string; env: Record; execPath: string; + argv: string[]; }> = {} ): { context: SentryContext; @@ -118,6 +119,7 @@ function createMockContext( env, cwd: () => "/tmp", execPath: overrides.execPath ?? "/usr/local/bin/sentry", + argv: overrides.argv ?? ["/usr/local/bin/sentry"], exit: vi.fn(() => { // no-op for tests }), @@ -778,6 +780,56 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy expect(setupCall?.args).toContain("--method"); expect(setupCall?.args).toContain("curl"); expect(setupCall?.args).toContain("--install"); + expect(setupCall?.args).toContain("--ensure-auth-scopes"); + }); + + test("does not launch interactive auth from JSON upgrades", async () => { + mockBinaryDownloadWithVersion("99.99.99"); + + const { context, restore } = createMockContext({ homeDir: testDir }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "--method", "curl", "--json"], context); + + const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); + expect(setupCall).toBeDefined(); + expect(setupCall?.args).not.toContain("--ensure-auth-scopes"); + }); + + test("runs setup through the CLI entrypoint after an npm upgrade", async () => { + mockGitHubVersion("99.99.99"); + const entryPath = "/npm/global/node_modules/sentry/dist/bin.cjs"; + const { context, restore } = createMockContext({ + homeDir: testDir, + execPath: "/usr/bin/node", + argv: ["/usr/bin/node", entryPath], + }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "--method", "npm"], context); + + const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); + expect(setupCall?.cmd).toBe(entryPath); + expect(setupCall?.args).toContain("--ensure-auth-scopes"); + }); + + test("runs the new Homebrew binary and keeps JSON upgrades non-interactive", async () => { + mockGitHubVersion("99.99.99"); + const binaryPath = join(testDir, "sentry"); + writeFileSync(binaryPath, "#!/bin/sh\n"); + chmodSync(binaryPath, 0o755); + const { context, restore } = createMockContext({ + homeDir: testDir, + execPath: "/opt/homebrew/Cellar/sentry/old/bin/sentry", + env: { PATH: testDir }, + }); + restoreStderr = restore; + + await run(app, ["cli", "upgrade", "--method", "brew", "--json"], context); + + const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); + expect(setupCall?.cmd).toBe(binaryPath); + expect(setupCall?.args).not.toContain("--ensure-auth-scopes"); }); test("reports setup failure when spawn exits non-zero", async () => { diff --git a/packages/cli/test/commands/project/create.test.ts b/packages/cli/test/commands/project/create.test.ts index cd6b626262..55609bfad2 100644 --- a/packages/cli/test/commands/project/create.test.ts +++ b/packages/cli/test/commands/project/create.test.ts @@ -273,14 +273,15 @@ describe("project create", () => { ); }); - test("passes --team after a best-effort role capability lookup", async () => { + test("passes --team to skip team auto-detection", async () => { listTeamsSpy.mockResolvedValue([sampleTeam, sampleTeam2]); const { context } = createMockContext(); const func = await createCommand.loader(); await func.call(context, { team: "mobile", json: false }, "my-app:go"); - expect(listTeamsSpy).toHaveBeenCalledOnce(); + // listTeams should NOT be called when --team is explicit + expect(listTeamsSpy).not.toHaveBeenCalled(); expect(createProjectWithDsnSpy).toHaveBeenCalledWith( "acme-corp", "mobile", @@ -675,55 +676,6 @@ describe("project create", () => { expect(err.message).toContain("disabled project creation for members"); }); - test("marks a Team Admin policy 403 as a recoverable OAuth scope error", async () => { - listTeamsSpy.mockResolvedValueOnce([ - { ...sampleTeam, access: ["team:admin"] }, - ]); - createProjectWithDsnSpy.mockRejectedValueOnce( - new ApiError( - "Forbidden", - 403, - "Your organization has disabled this feature for members." - ) - ); - - const { context } = createMockContext(); - const func = await createCommand.loader(); - const error = (await func - .call(context, { json: false }, "my-app:node") - .catch((caught: unknown) => caught)) as ApiError; - - expect(error).toBeInstanceOf(ApiError); - expect(error.requiredScopes).toEqual(["team:admin"]); - expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); - }); - - test("recovers a known Team Admin selected with --team", async () => { - listTeamsSpy.mockResolvedValueOnce([ - { - ...sampleTeam, - slug: "platform", - access: ["team:read", "team:admin"], - }, - ]); - createProjectWithDsnSpy.mockRejectedValueOnce( - new ApiError( - "Forbidden", - 403, - "Your organization has disabled this feature for members." - ) - ); - - const { context } = createMockContext(); - const func = await createCommand.loader(); - const error = (await func - .call(context, { json: false, team: "platform" }, "my-app:node") - .catch((caught: unknown) => caught)) as ApiError; - - expect(error).toBeInstanceOf(ApiError); - expect(error.requiredScopes).toEqual(["team:admin"]); - }); - test("outputs JSON when --json flag is set", async () => { const { context, stdoutWrite } = createMockContext(); const func = await createCommand.loader(); diff --git a/packages/cli/test/lib/api-scope.test.ts b/packages/cli/test/lib/api-scope.test.ts index cfdb5044ec..e983e843ec 100644 --- a/packages/cli/test/lib/api-scope.test.ts +++ b/packages/cli/test/lib/api-scope.test.ts @@ -5,83 +5,7 @@ */ import { describe, expect, test } from "vitest"; -import { - extractRequiredScopes, - extractRequiredScopesFromWwwAuthenticate, -} from "../../src/lib/api-scope.js"; - -describe("extractRequiredScopesFromWwwAuthenticate", () => { - test("extracts scopes from an RFC 6750 insufficient-scope challenge", () => { - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Bearer error="insufficient_scope", scope="team:admin project:write"' - ) - ).toEqual(["team:admin", "project:write"]); - }); - - test("accepts a future scope because the server challenge is authoritative", () => { - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Bearer error="insufficient_scope", scope="project:new-capability"' - ) - ).toEqual(["project:new-capability"]); - }); - - test("ignores unrelated or incomplete authentication challenges", () => { - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Bearer error="invalid_token", scope="team:admin"' - ) - ).toEqual([]); - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Basic realm="sentry", error="insufficient_scope", scope="team:admin"' - ) - ).toEqual([]); - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Bearer error="insufficient_scope"' - ) - ).toEqual([]); - }); - - test("deduplicates scopes while preserving server order", () => { - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Bearer scope="team:admin team:admin org:read", error=insufficient_scope' - ) - ).toEqual(["team:admin", "org:read"]); - }); - - test.each([ - [ - 'Basic realm="legacy", Bearer error="insufficient_scope", scope="team:admin"', - ], - [ - 'Bearer error="insufficient_scope", scope="team:admin", Basic realm="legacy"', - ], - ])("isolates Bearer parameters in combined challenges", (header) => { - expect(extractRequiredScopesFromWwwAuthenticate(header)).toEqual([ - "team:admin", - ]); - }); - - test("does not borrow a scope from a later authentication challenge", () => { - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Bearer error="insufficient_scope", Custom scope="team:admin"' - ) - ).toEqual([]); - }); - - test("ignores challenge-like text inside a quoted parameter", () => { - expect( - extractRequiredScopesFromWwwAuthenticate( - 'Basic realm="legacy, Bearer bogus", Bearer error="insufficient_scope", scope="project:new-capability"' - ) - ).toEqual(["project:new-capability"]); - }); -}); +import { extractRequiredScopes } from "../../src/lib/api-scope.js"; describe("extractRequiredScopes", () => { test("returns [] for undefined or null detail", () => { @@ -107,12 +31,6 @@ describe("extractRequiredScopes", () => { ).toEqual([]); }); - test("ignores a role requirement that merely names a scope", () => { - expect( - extractRequiredScopes("This action requires the team:admin role.") - ).toEqual([]); - }); - test("extracts a single scope from a detail string", () => { expect( extractRequiredScopes( diff --git a/packages/cli/test/lib/api/auth.test.ts b/packages/cli/test/lib/api/auth.test.ts new file mode 100644 index 0000000000..2a134e9ea1 --- /dev/null +++ b/packages/cli/test/lib/api/auth.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +vi.mock("../../../src/lib/api/infrastructure.js", () => ({ + apiRequestToRegion: vi.fn(), +})); + +import { getCurrentAuthScopes } from "../../../src/lib/api/auth.js"; +import { apiRequestToRegion } from "../../../src/lib/api/infrastructure.js"; + +describe("getCurrentAuthScopes", () => { + beforeEach(() => vi.mocked(apiRequestToRegion).mockReset()); + + test("reads effective token scopes from the API index", async () => { + vi.mocked(apiRequestToRegion).mockResolvedValue({ + data: { auth: { scopes: ["org:read", "team:admin"] } }, + headers: new Headers(), + }); + + await expect(getCurrentAuthScopes()).resolves.toEqual([ + "org:read", + "team:admin", + ]); + expect(apiRequestToRegion).toHaveBeenCalledWith( + expect.any(String), + "", + expect.objectContaining({ schema: expect.any(Object) }) + ); + }); + + test("returns null when the API index reports no authenticated token", async () => { + vi.mocked(apiRequestToRegion).mockResolvedValue({ + data: { auth: null }, + headers: new Headers(), + }); + + await expect(getCurrentAuthScopes()).resolves.toBeNull(); + }); + + test("preserves a 401 from an invalid bearer", async () => { + const error = new Error("401 Unauthorized"); + vi.mocked(apiRequestToRegion).mockImplementationOnce(async () => { + throw error; + }); + + let thrown: unknown; + try { + await getCurrentAuthScopes(); + } catch (caught) { + thrown = caught; + } + expect(thrown).toBe(error); + }); +}); diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 83e3749255..0211b0b556 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -1,6 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { - apiRequestToRegion, isTextualContentType, rawApiRequest, throwApiError, @@ -74,31 +73,6 @@ describe("throwApiError", () => { } }); - test("preserves required scopes from WWW-Authenticate as structured metadata", () => { - const mockResponse = new Response("", { - status: 403, - statusText: "Forbidden", - headers: { - "www-authenticate": - 'Bearer error="insufficient_scope", scope="team:admin"', - }, - }); - - try { - throwApiError( - { detail: "You do not have permission to perform this action." }, - mockResponse, - "Failed to create project" - ); - } catch (error) { - const apiError = error as ApiError; - expect(apiError.requiredScopes).toEqual(["team:admin"]); - expect(apiError.detail).toContain( - "missing the required scope(s) 'team:admin'" - ); - } - }); - test("HTTP error without detail uses stringified error", () => { const mockResponse = new Response("", { status: 500, @@ -696,46 +670,3 @@ describe("rawApiRequest binary handling", () => { expect(result.body).toBe("not json"); }); }); - -describe("apiRequestToRegion error metadata", () => { - useTestConfigDir("regional-api-scope-"); - - let originalFetch: typeof globalThis.fetch; - - beforeEach(async () => { - originalFetch = globalThis.fetch; - await setAuthToken("test-token"); - }); - - afterEach(() => { - globalThis.fetch = originalFetch; - }); - - test("preserves an insufficient-scope challenge from a raw API response", async () => { - globalThis.fetch = mockFetch( - async () => - new Response( - JSON.stringify({ - detail: "You do not have permission to perform this action.", - }), - { - status: 403, - statusText: "Forbidden", - headers: { - "content-type": "application/json", - "www-authenticate": - 'Bearer error="insufficient_scope", scope="team:admin"', - }, - } - ) - ); - - const error = await apiRequestToRegion( - "https://sentry.io", - "organizations/acme/projects/" - ).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(ApiError); - expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); - }); -}); diff --git a/packages/cli/test/lib/errors.test.ts b/packages/cli/test/lib/errors.test.ts index 072a9391fc..ea91399cd5 100644 --- a/packages/cli/test/lib/errors.test.ts +++ b/packages/cli/test/lib/errors.test.ts @@ -7,7 +7,6 @@ import { CliError, ConfigError, ContextError, - cloneApiError, DeviceFlowError, EXIT, formatError, @@ -98,25 +97,6 @@ describe("ApiError", () => { const err = new ApiError("Request failed", 503); expect(err.format()).toBe("Request failed"); }); - - test("cloneApiError preserves structured scope metadata", () => { - const original = new ApiError( - "Forbidden", - 403, - "Insufficient scope", - "/api/0/projects/", - true, - ["project:new-capability"] - ); - - const cloned = cloneApiError(original, { message: "Project list failed" }); - - expect(cloned.message).toBe("Project list failed"); - expect(cloned.requiredScopes).toEqual(["project:new-capability"]); - expect(cloned.detail).toBe(original.detail); - expect(cloned.endpoint).toBe(original.endpoint); - expect(cloned.enriched403).toBe(true); - }); }); describe("ConfigError", () => { diff --git a/packages/cli/test/lib/init/preflight.test.ts b/packages/cli/test/lib/init/preflight.test.ts index 993fdcde9b..18737c9e93 100644 --- a/packages/cli/test/lib/init/preflight.test.ts +++ b/packages/cli/test/lib/init/preflight.test.ts @@ -16,6 +16,10 @@ vi.mock("../../../src/lib/api-client.js", async (importOriginal) => { ); }); +vi.mock("../../../src/lib/scope-recovery.js", () => ({ + currentOAuthGrantNeedsRefresh: vi.fn(), +})); + // biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference import * as apiClient from "../../../src/lib/api-client.js"; @@ -66,6 +70,8 @@ import * as prefetch from "../../../src/lib/init/org-prefetch.js"; import { resolveInitContext } from "../../../src/lib/init/preflight.js"; import type { WizardOptions } from "../../../src/lib/init/types.js"; import { CANCELLED } from "../../../src/lib/init/ui/types.js"; +// biome-ignore lint/performance/noNamespaceImport: scope decision is mocked at the module boundary +import * as scopeRecovery from "../../../src/lib/scope-recovery.js"; vi.mock("../../../src/lib/resolve-target.js", async (importOriginal) => { const actual = @@ -125,6 +131,9 @@ let detectDsnSpy: ReturnType; let resolveDsnByPublicKeySpy: ReturnType; beforeEach(() => { + vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockResolvedValue( + false + ); resolveOrgPrefetchedSpy = vi .spyOn(prefetch, "resolveOrgPrefetched") .mockResolvedValue({ org: "acme" }); @@ -185,6 +194,7 @@ afterEach(() => { resolveOrCreateTeamSpy.mockRestore(); detectDsnSpy.mockRestore(); resolveDsnByPublicKeySpy.mockRestore(); + vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockReset(); process.exitCode = 0; }); @@ -525,7 +535,6 @@ describe("resolveInitContext", () => { resolveOrCreateTeamSpy.mockResolvedValue({ slug: "backend", source: "explicit", - roleScopes: ["team:read", "team:admin"], } as any); const { ui } = createMockUI(); @@ -536,7 +545,6 @@ describe("resolveInitContext", () => { expect(context?.isExplicitTeam).toBe(true); expect(context?.team).toBe("backend"); - expect(context?.teamRoleScopes).toEqual(["team:read", "team:admin"]); }); test("sets isExplicitTeam:false when no --team flag is provided", async () => { @@ -561,22 +569,16 @@ describe("resolveInitContext", () => { expect(resolveOrCreateTeamSpy).not.toHaveBeenCalled(); }); - test("preserves a structured insufficient-scope error from preflight", async () => { - const scopeError = new ApiError( - "Forbidden", - 403, - "Insufficient scope", - undefined, - true, - ["team:read"] - ); - listTeamsSpy.mockRejectedValueOnce(scopeError); + test("preserves a 403 when the stored OAuth grant is stale", async () => { + const error = new ApiError("Forbidden", 403, "No team:read access"); + listTeamsSpy.mockRejectedValueOnce(error); + vi.mocked( + scopeRecovery.currentOAuthGrantNeedsRefresh + ).mockResolvedValueOnce(true); const { ui } = createMockUI(); - await expect(resolveInitContext(makeOptions(), ui)).rejects.toBe( - scopeError - ); + await expect(resolveInitContext(makeOptions(), ui)).rejects.toBe(error); }); test("preserves rich org-not-found guidance when implicit team lookup returns 404", async () => { diff --git a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts index d8d64dfbad..6d2bd00381 100644 --- a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts +++ b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts @@ -11,6 +11,10 @@ vi.mock("../../../../src/lib/api-client.js", async (importOriginal) => { ); }); +vi.mock("../../../../src/lib/scope-recovery.js", () => ({ + currentOAuthGrantNeedsRefresh: vi.fn(), +})); + // biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference import * as apiClient from "../../../../src/lib/api-client.js"; import { ApiError } from "../../../../src/lib/errors.js"; @@ -23,6 +27,8 @@ import type { CreateSentryProjectPayload, EnsureSentryProjectPayload, } from "../../../../src/lib/init/types.js"; +// biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference +import * as scopeRecovery from "../../../../src/lib/scope-recovery.js"; vi.mock("../../../../src/lib/resolve-team.js", async (importOriginal) => { const actual = @@ -85,6 +91,9 @@ let tryGetPrimaryDsnSpy: ReturnType; let resolveOrCreateTeamSpy: ReturnType; beforeEach(() => { + vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockResolvedValue( + false + ); createProjectWithDsnSpy = vi .spyOn(apiClient, "createProjectWithDsn") .mockResolvedValue({ @@ -125,6 +134,7 @@ afterEach(() => { getProjectSpy.mockRestore(); tryGetPrimaryDsnSpy.mockRestore(); resolveOrCreateTeamSpy.mockRestore(); + vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockReset(); }); describe("createSentryProject", () => { @@ -345,30 +355,10 @@ describe("createSentryProject", () => { expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); }); - test("surfaces an implicit Team Admin policy 403 for OAuth recovery", async () => { - getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); - createProjectWithDsnSpy.mockRejectedValueOnce( - new ApiError( - "Forbidden", - 403, - "Your organization has disabled this feature for members." - ) - ); - - const error = await createSentryProject(makePayload(), { - dryRun: false, - org: "acme", - team: "platform", - teamRoleScopes: ["team:read", "team:admin"], - project: undefined, - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(ApiError); - expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); - expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); - }); - - test("lets a recoverable scope error escape the real tool registry", async () => { + test("lets a 403 from an outdated OAuth grant escape the real tool registry", async () => { + vi.mocked( + scopeRecovery.currentOAuthGrantNeedsRefresh + ).mockResolvedValueOnce(true); getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); createProjectWithDsnSpy.mockRejectedValueOnce( new ApiError( @@ -384,37 +374,13 @@ describe("createSentryProject", () => { dryRun: false, org: "acme", team: "platform", - teamRoleScopes: ["team:read", "team:admin"], }).catch((caught: unknown) => caught); expect(error).toBeInstanceOf(ApiError); - expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); + expect((error as ApiError).status).toBe(403); }); - test("lets a legacy detail-only scope error escape the real tool registry", async () => { - getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); - createProjectWithDsnSpy.mockRejectedValueOnce( - new ApiError( - "Forbidden", - 403, - "You do not have the required scope: project:admin" - ) - ); - - const error = await executeTool(makePayload(), { - directory: "/tmp/test", - yes: false, - dryRun: false, - org: "acme", - team: "platform", - isExplicitTeam: true, - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(ApiError); - expect((error as ApiError).detail).toContain("project:admin"); - }); - - test("keeps an explicit-team policy 403 as a role error", async () => { + test("does not fall back on team-scoped policy 403", async () => { getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); createProjectWithDsnSpy.mockRejectedValueOnce( new ApiError( @@ -428,37 +394,14 @@ describe("createSentryProject", () => { dryRun: false, org: "acme", team: "platform", - isExplicitTeam: true, project: undefined, }); expect(result.ok).toBe(false); + expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); expect(result.error).toContain("disabled for members"); }); - test("recovers an explicit team when its known role grants Team Admin", async () => { - getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); - createProjectWithDsnSpy.mockRejectedValueOnce( - new ApiError( - "Forbidden", - 403, - "Your organization has disabled this feature for members." - ) - ); - - const error = await createSentryProject(makePayload(), { - dryRun: false, - org: "acme", - team: "platform", - teamRoleScopes: ["team:read", "team:admin"], - isExplicitTeam: true, - project: undefined, - }).catch((caught: unknown) => caught); - - expect(error).toBeInstanceOf(ApiError); - expect((error as ApiError).requiredScopes).toEqual(["team:admin"]); - }); - test("surfaces friendly 409 error when fallback project already exists", async () => { createProjectWithAutoTeamSpy.mockRejectedValueOnce( new ApiError("Conflict", 409, "Slug already in use") diff --git a/packages/cli/test/lib/init/wizard-runner.test.ts b/packages/cli/test/lib/init/wizard-runner.test.ts index 5aebeb7d61..690848f6c3 100644 --- a/packages/cli/test/lib/init/wizard-runner.test.ts +++ b/packages/cli/test/lib/init/wizard-runner.test.ts @@ -721,7 +721,7 @@ describe("runWizard", () => { expect(lastFeedbackOutcome()).toBe("failed"); }); - test("preserves recoverable scope errors thrown by a tool", async () => { + test("preserves 403 errors thrown for command-level scope inspection", async () => { const payload: ToolPayload = { type: "tool", operation: "create-sentry-project", @@ -735,23 +735,16 @@ describe("runWizard", () => { "ensure-sentry-project": { suspendPayload: payload }, }, }; - const scopeError = new ApiError( - "Forbidden", - 403, - "Insufficient scope", - undefined, - true, - ["team:admin"] - ); + const scopeError = new ApiError("Forbidden", 403); executeToolSpy.mockRejectedValue(scopeError); await expect(runWizard(makeOptions())).rejects.toBe(scopeError); expect(spinnerMock.stop).toHaveBeenCalledWith( - "Authorization update required", + "Sentry API request denied", 1 ); - expect(lastCancelMessage()).toBe("Authorization update required"); + expect(lastCancelMessage()).toBe("Sentry API request denied"); }); test("tears down forwarding and stops the spinner on cancellation", async () => { diff --git a/packages/cli/test/lib/resolve-team.test.ts b/packages/cli/test/lib/resolve-team.test.ts index b34a2ba02d..5c98c94154 100644 --- a/packages/cli/test/lib/resolve-team.test.ts +++ b/packages/cli/test/lib/resolve-team.test.ts @@ -40,47 +40,4 @@ describe("resolveOrCreateTeam", () => { expect(error.status).toBe(401); expect(error.detail).toContain("over its member limit"); }); - - test("preserves effective team role scopes for permission recovery", async () => { - listTeamsSpy.mockResolvedValueOnce([ - { - id: "1", - slug: "platform", - name: "Platform", - access: ["team:read", "team:admin"], - }, - ]); - - const result = await resolveOrCreateTeam("acme", { - usageHint: "sentry project create", - }); - - expect(result).toEqual({ - slug: "platform", - source: "auto-selected", - roleScopes: ["team:read", "team:admin"], - }); - }); - - test("best-effort resolves role scopes for an explicit team", async () => { - listTeamsSpy.mockResolvedValueOnce([ - { - id: "1", - slug: "platform", - name: "Platform", - access: ["team:read", "team:admin"], - }, - ]); - - const result = await resolveOrCreateTeam("acme", { - team: "platform", - usageHint: "sentry project create", - }); - - expect(result).toEqual({ - slug: "platform", - source: "explicit", - roleScopes: ["team:read", "team:admin"], - }); - }); }); diff --git a/packages/cli/test/lib/scope-recovery.test.ts b/packages/cli/test/lib/scope-recovery.test.ts index 92aa25cc91..80e1922dd4 100644 --- a/packages/cli/test/lib/scope-recovery.test.ts +++ b/packages/cli/test/lib/scope-recovery.test.ts @@ -1,27 +1,19 @@ import { describe, expect, test, vi } from "vitest"; -import { ApiError } from "../../src/lib/errors.js"; +import { ApiError, AuthError } from "../../src/lib/errors.js"; +import { OAUTH_SCOPES } from "../../src/lib/oauth.js"; import { + currentOAuthGrantNeedsRefresh, + ensureCurrentOAuthScopes, runWithScopeRecovery, type ScopeRecoveryRuntime, } from "../../src/lib/scope-recovery.js"; -function missingScopeError(): ApiError { - return new ApiError( - "Forbidden", - 403, - "You do not have permission to perform this action.", - undefined, - true, - ["team:admin"] - ); -} - function runtime( overrides: Partial = {} ): ScopeRecoveryRuntime { return { assertTrustedHost: vi.fn(), - confirm: vi.fn().mockResolvedValue(true), + getAuthScopes: vi.fn().mockResolvedValue(OAUTH_SCOPES), getAuthSource: () => "oauth", inputIsTty: () => true, promptsAllowed: () => true, @@ -31,17 +23,20 @@ function runtime( } describe("runWithScopeRecovery", () => { - test("refreshes an old OAuth grant with current scopes and retries once", async () => { - const originalError = missingScopeError(); + test("checks the token after a 403, re-authorizes a stale grant, and retries", async () => { + const error = new ApiError("Forbidden", 403); const proceed = vi .fn<(argv: string[]) => Promise>() - .mockRejectedValueOnce(originalError) + .mockRejectedValueOnce(error) .mockResolvedValueOnce(); - const login = vi.fn().mockResolvedValue({ - method: "oauth", - configPath: "/tmp/config", + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + const testRuntime = runtime({ + getAuthScopes: vi + .fn() + .mockResolvedValue( + OAUTH_SCOPES.filter((scope) => scope !== "team:admin") + ), }); - const testRuntime = runtime(); await runWithScopeRecovery( proceed, @@ -51,164 +46,243 @@ describe("runWithScopeRecovery", () => { ); expect(proceed).toHaveBeenCalledTimes(2); + expect(testRuntime.getAuthScopes).toHaveBeenCalledOnce(); expect(testRuntime.assertTrustedHost).toHaveBeenCalledOnce(); - expect(testRuntime.confirm).toHaveBeenCalledOnce(); - expect(login).toHaveBeenCalledOnce(); - const scope = login.mock.calls[0]?.[0]?.scope; - expect(scope?.split(" ")).toEqual( - expect.arrayContaining(["org:read", "project:write", "team:admin"]) + expect(login).toHaveBeenCalledWith(); + expect(testRuntime.write).toHaveBeenCalledWith( + expect.stringContaining("team:admin") ); }); - test("keeps response-detail parsing as a legacy server fallback", async () => { - const originalError = new ApiError( - "Forbidden", - 403, - "You do not have the required scope: project:admin" - ); + test("does not re-authorize a role or policy 403 when the token has every scope", async () => { + const error = new ApiError("Forbidden", 403); + const proceed = vi.fn().mockRejectedValue(error); + const login = vi.fn(); + + await expect( + runWithScopeRecovery(proceed, [], login, runtime()) + ).rejects.toBe(error); + + expect(login).not.toHaveBeenCalled(); + }); + + test("re-authorizes after a 401 when the API reports no active token", async () => { + const error = new ApiError("Unauthorized", 401); const proceed = vi .fn<(argv: string[]) => Promise>() - .mockRejectedValueOnce(originalError) + .mockRejectedValueOnce(error) .mockResolvedValueOnce(); - const login = vi.fn().mockResolvedValue({ - method: "oauth", - configPath: "/tmp/config", - }); - - await runWithScopeRecovery(proceed, [], login, runtime()); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); - expect(login.mock.calls[0]?.[0]?.scope?.split(" ")).toContain( - "project:admin" + await runWithScopeRecovery( + proceed, + [], + login, + runtime({ getAuthScopes: vi.fn().mockResolvedValue(null) }) ); + + expect(login).toHaveBeenCalledOnce(); + expect(proceed).toHaveBeenCalledTimes(2); }); - test("can recover a scope introduced by a newer Sentry server", async () => { - const originalError = new ApiError( - "Forbidden", - 403, - "Insufficient scope", - undefined, - true, - ["project:new-capability"] - ); + test("re-authorizes when scope inspection rejects an invalid token", async () => { + const error = new ApiError("Unauthorized", 401); const proceed = vi .fn<(argv: string[]) => Promise>() - .mockRejectedValueOnce(originalError) + .mockRejectedValueOnce(error) .mockResolvedValueOnce(); - const login = vi.fn().mockResolvedValue({ - method: "oauth", - configPath: "/tmp/config", - }); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); - await runWithScopeRecovery(proceed, [], login, runtime()); + await runWithScopeRecovery( + proceed, + [], + login, + runtime({ + getAuthScopes: vi + .fn() + .mockRejectedValue(new ApiError("Invalid token", 401)), + }) + ); + + expect(login).toHaveBeenCalledOnce(); + expect(proceed).toHaveBeenCalledTimes(2); + }); + + test("re-authorizes when a failed refresh clears the stored OAuth row", async () => { + const error = new ApiError("Unauthorized", 401); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(error) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + const getAuthSource = vi + .fn<() => "oauth" | undefined>() + .mockReturnValueOnce("oauth") + .mockReturnValueOnce(undefined); + const getAuthScopes = vi.fn(); - expect(login.mock.calls[0]?.[0]?.scope?.split(" ")).toContain( - "project:new-capability" + await runWithScopeRecovery( + proceed, + [], + login, + runtime({ getAuthSource, getAuthScopes }) ); + + expect(login).toHaveBeenCalledOnce(); + expect(getAuthScopes).not.toHaveBeenCalled(); }); test.each([ [["init", "--yes"], "oauth" as const], - [["init", "-y"], "oauth" as const], [["init", "--dry-run"], "oauth" as const], - [["project", "create"], "env:SENTRY_AUTH_TOKEN" as const], - ])("does not refresh unattended commands or env tokens", async (argv, source) => { - const originalError = missingScopeError(); - const proceed = vi.fn().mockRejectedValue(originalError); + [[], "env:SENTRY_AUTH_TOKEN" as const], + ])("does not launch OAuth for unattended commands or env tokens", async (argv, source) => { + const error = new ApiError("Forbidden", 403); const login = vi.fn(); + const getAuthScopes = vi.fn().mockResolvedValue([]); await expect( runWithScopeRecovery( - proceed, + vi.fn().mockRejectedValue(error), argv, login, - runtime({ getAuthSource: () => source }) + runtime({ getAuthSource: () => source, getAuthScopes }) ) - ).rejects.toBe(originalError); + ).rejects.toBe(error); - expect(proceed).toHaveBeenCalledOnce(); expect(login).not.toHaveBeenCalled(); + if (source !== "oauth") { + expect(getAuthScopes).not.toHaveBeenCalled(); + } }); - test("preserves the original error when the credential store cannot be read", async () => { - const originalError = missingScopeError(); - const proceed = vi.fn().mockRejectedValue(originalError); + test("checks scopes but does not launch OAuth without an interactive TTY", async () => { + const error = new ApiError("Forbidden", 403); + const getAuthScopes = vi.fn().mockResolvedValue([]); const login = vi.fn(); await expect( runWithScopeRecovery( - proceed, + vi.fn().mockRejectedValue(error), [], login, - runtime({ - getAuthSource: () => { - throw new Error("database unavailable"); - }, - }) + runtime({ getAuthScopes, inputIsTty: () => false }) ) - ).rejects.toBe(originalError); + ).rejects.toBe(error); + + expect(getAuthScopes).toHaveBeenCalledOnce(); expect(login).not.toHaveBeenCalled(); }); - test.each([ - { inputIsTty: () => false }, - { promptsAllowed: () => false }, - ])("does not refresh outside an interactive prompt context", async (overrides) => { - const originalError = missingScopeError(); - const proceed = vi.fn().mockRejectedValue(originalError); + test("does not launch OAuth when JSON output disables prompts", async () => { + const error = new ApiError("Forbidden", 403); + const getAuthScopes = vi.fn().mockResolvedValue([]); const login = vi.fn(); await expect( - runWithScopeRecovery(proceed, [], login, runtime(overrides)) - ).rejects.toBe(originalError); + runWithScopeRecovery( + vi.fn().mockRejectedValue(error), + ["--json"], + login, + runtime({ getAuthScopes, promptsAllowed: () => false }) + ) + ).rejects.toBe(error); + + expect(getAuthScopes).toHaveBeenCalledOnce(); expect(login).not.toHaveBeenCalled(); }); - test("preserves the original error when refresh is declined", async () => { - const originalError = missingScopeError(); - const proceed = vi.fn().mockRejectedValue(originalError); + test("preserves the original error when scope inspection fails", async () => { + const error = new ApiError("Forbidden", 403); const login = vi.fn(); await expect( runWithScopeRecovery( - proceed, + vi.fn().mockRejectedValue(error), [], login, - runtime({ confirm: vi.fn().mockResolvedValue(false) }) + runtime({ + getAuthScopes: vi.fn().mockRejectedValue(new Error("offline")), + }) ) - ).rejects.toBe(originalError); + ).rejects.toBe(error); expect(login).not.toHaveBeenCalled(); }); - test("preserves the original error when login is cancelled", async () => { - const originalError = missingScopeError(); - const proceed = vi.fn().mockRejectedValue(originalError); - const login = vi.fn().mockResolvedValue(null); - - await expect( - runWithScopeRecovery(proceed, [], login, runtime()) - ).rejects.toBe(originalError); - expect(proceed).toHaveBeenCalledOnce(); - }); - test("does not attempt a second recovery when the retry fails", async () => { - const firstError = missingScopeError(); - const retryError = missingScopeError(); + const first = new ApiError("Forbidden", 403); + const second = new ApiError("Still forbidden", 403); const proceed = vi .fn<(argv: string[]) => Promise>() - .mockRejectedValueOnce(firstError) - .mockRejectedValueOnce(retryError); - const login = vi.fn().mockResolvedValue({ - method: "oauth", - configPath: "/tmp/config", - }); - const testRuntime = runtime(); + .mockRejectedValueOnce(first) + .mockRejectedValueOnce(second); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); await expect( - runWithScopeRecovery(proceed, [], login, testRuntime) - ).rejects.toBe(retryError); + runWithScopeRecovery( + proceed, + [], + login, + runtime({ getAuthScopes: vi.fn().mockResolvedValue([]) }) + ) + ).rejects.toBe(second); + expect(login).toHaveBeenCalledOnce(); expect(proceed).toHaveBeenCalledTimes(2); - expect(testRuntime.confirm).toHaveBeenCalledOnce(); + }); +}); + +describe("upgrade scope check", () => { + test("re-authorizes a stale OAuth grant", async () => { + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + const refreshed = await ensureCurrentOAuthScopes( + login, + runtime({ getAuthScopes: vi.fn().mockResolvedValue(["org:read"]) }) + ); + + expect(refreshed).toBe(true); + expect(login).toHaveBeenCalledOnce(); + }); + + test("re-authorizes when the stored token is rejected during upgrade", async () => { + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + const refreshed = await ensureCurrentOAuthScopes( + login, + runtime({ + getAuthScopes: vi + .fn() + .mockRejectedValue(new ApiError("Invalid token", 401)), + }) + ); + + expect(refreshed).toBe(true); + expect(login).toHaveBeenCalledOnce(); + }); + + test("re-authorizes when upgrade scope inspection reports expired auth", async () => { + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + const refreshed = await ensureCurrentOAuthScopes( + login, + runtime({ + getAuthScopes: vi.fn().mockRejectedValue(new AuthError("expired")), + }) + ); + + expect(refreshed).toBe(true); expect(login).toHaveBeenCalledOnce(); }); + + test("does nothing when the stored grant is current", async () => { + const login = vi.fn(); + expect(await ensureCurrentOAuthScopes(login, runtime())).toBe(false); + expect(login).not.toHaveBeenCalled(); + }); + + test("exposes the same scope decision to init error boundaries", async () => { + expect( + await currentOAuthGrantNeedsRefresh( + runtime({ getAuthScopes: vi.fn().mockResolvedValue([]) }) + ) + ).toBe(true); + expect(await currentOAuthGrantNeedsRefresh(runtime())).toBe(false); + }); }); From 77ba525cf86ea2f087b9333e9ef8991361376afb Mon Sep 17 00:00:00 2001 From: betegon Date: Mon, 10 Aug 2026 19:25:24 +0200 Subject: [PATCH 4/5] fix(auth): propagate API auth failures to recovery --- packages/cli/src/app.ts | 27 +++++------ .../cli/test/lib/app-scope-recovery.test.ts | 46 +++++++++++++++++++ 2 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 packages/cli/test/lib/app-scope-recovery.test.ts diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index d2b7febf36..d34c3212b4 100644 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -65,6 +65,7 @@ import { import { CLI_VERSION } from "./lib/constants.js"; import { reportCliError } from "./lib/error-reporting.js"; import { + ApiError, AuthError, CliError, getExitCode, @@ -274,6 +275,16 @@ function formatSynonymError( return `${prefix} ${exc.format()}\n${tip}`; } +function escapesToOuterMiddleware(exc: unknown): boolean { + if (exc instanceof OutputError) { + return true; + } + if (exc instanceof AuthError) { + return exc.reason === "not_authenticated" || exc.reason === "expired"; + } + return exc instanceof ApiError && (exc.status === 401 || exc.status === 403); +} + /** * Custom error formatting for CLI errors. * @@ -347,19 +358,9 @@ const customText: ApplicationText = { return base; }, exceptionWhileRunningCommand: (exc: unknown, ansiColor: boolean): string => { - // OutputError: data was already rendered to stdout — just re-throw - // so the exit code propagates without Stricli printing an error message. - if (exc instanceof OutputError) { - throw exc; - } - - // Re-throw AuthError for auto-login flow in bin.ts - // Don't capture to Sentry - it's an expected state (user not logged in or token expired), not an error - // Note: skipAutoAuth is checked in bin.ts, not here — all auth errors must escape Sentry capture - if ( - exc instanceof AuthError && - (exc.reason === "not_authenticated" || exc.reason === "expired") - ) { + // These errors are handled outside Stricli: OutputError has already been + // rendered, while auth errors may trigger login and a single retry. + if (escapesToOuterMiddleware(exc)) { throw exc; } diff --git a/packages/cli/test/lib/app-scope-recovery.test.ts b/packages/cli/test/lib/app-scope-recovery.test.ts new file mode 100644 index 0000000000..73bada9808 --- /dev/null +++ b/packages/cli/test/lib/app-scope-recovery.test.ts @@ -0,0 +1,46 @@ +import { homedir } from "node:os"; +import { run } from "@stricli/core"; +import { afterEach, describe, expect, test } from "vitest"; +import { app } from "../../src/app.js"; +import type { SentryContext } from "../../src/context.js"; +import { getConfigDir } from "../../src/lib/db/index.js"; +import { ApiError } from "../../src/lib/errors.js"; + +const originalFetch = globalThis.fetch; + +function context(): SentryContext { + return { + process, + env: process.env, + cwd: process.cwd(), + homeDir: homedir(), + configDir: getConfigDir(), + stdout: { write: () => true }, + stderr: { write: () => true }, + stdin: process.stdin, + }; +} + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe("scope-recovery error boundary", () => { + test.each([401, 403])("lets API %i escape Stricli", async (status) => { + globalThis.fetch = (async () => + new Response(JSON.stringify({ detail: "Denied" }), { + status, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + + let thrown: unknown; + try { + await run(app, ["org", "list", "--fresh"], context()); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(ApiError); + expect((thrown as ApiError).status).toBe(status); + }); +}); From 0d83597e21a3db8c58006912a3bb8489962300a5 Mon Sep 17 00:00:00 2001 From: betegon Date: Mon, 10 Aug 2026 22:21:59 +0200 Subject: [PATCH 5/5] fix(auth): preserve init fallbacks during scope recovery Only delegate init API errors when interactive OAuth recovery can actually run, so policy and unattended flows keep their actionable fallbacks. --- packages/cli/src/lib/command.ts | 13 ++- packages/cli/src/lib/init/preflight.ts | 51 ++++++---- .../lib/init/tools/create-sentry-project.ts | 30 +++--- packages/cli/src/lib/interactive-prompts.ts | 9 +- packages/cli/src/lib/scope-recovery.ts | 64 ++++++++---- packages/cli/test/lib/command.test.ts | 74 ++++++++++++++ packages/cli/test/lib/init/preflight.test.ts | 97 +++++++++++++++++-- .../init/tools/create-sentry-project.test.ts | 95 +++++++++++++++--- packages/cli/test/lib/scope-recovery.test.ts | 86 ++++++++++++---- 9 files changed, 410 insertions(+), 109 deletions(-) diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 9b5efea99f..16f6e8cc6d 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -707,9 +707,16 @@ export function buildCommand< } } - // Suppress interactive prompts (e.g. the org/project picker) in JSON mode so - // a prompt never blocks a scripted run or interleaves with stdout JSON. - setInteractivePromptsAllowed(!cleanFlags.json); + // Suppress prompts when parsed command flags require unattended or + // machine-readable execution. Using parsed flags avoids confusing aliases + // such as list command `-n` (limit) with init's `-n` (dry-run). + setInteractivePromptsAllowed( + !( + cleanFlags.json === true || + cleanFlags.yes === true || + cleanFlags["dry-run"] === true + ) + ); const stdout = (this as unknown as { stdout: Writer }).stdout; diff --git a/packages/cli/src/lib/init/preflight.ts b/packages/cli/src/lib/init/preflight.ts index 88f03e1e37..3b41673063 100644 --- a/packages/cli/src/lib/init/preflight.ts +++ b/packages/cli/src/lib/init/preflight.ts @@ -7,7 +7,7 @@ import { import { getAuthToken } from "../db/auth.js"; import { ApiError, AuthError, HostScopeError, WizardError } from "../errors.js"; import { buildOrgNotFoundError, resolveOrCreateTeam } from "../resolve-team.js"; -import { currentOAuthGrantNeedsRefresh } from "../scope-recovery.js"; +import { captureOAuthScopeRecoveryGate } from "../scope-recovery.js"; import { slugify } from "../utils.js"; import { WizardCancelledError } from "./clack-utils.js"; import { tryGetExistingProjectData } from "./existing-project.js"; @@ -349,6 +349,7 @@ async function resolveTeam( return await resolveImplicitTeam(org, initial, ui); } + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { const result = await resolveOrCreateTeam(org, { team: initial.team, @@ -361,10 +362,16 @@ async function resolveTeam( if (error instanceof WizardCancelledError) { throw error; } + if ( + error instanceof ApiError && + (error.status === 401 || error.status === 403) && + (await scopeRecovery.shouldDelegate(error, { + unattended: initial.yes || initial.dryRun, + })) + ) { + throw error; + } if (error instanceof ApiError && error.status === 403) { - if (await currentOAuthGrantNeedsRefresh()) { - throw error; - } return; } throw toPreflightWizardError(error); @@ -418,17 +425,23 @@ async function assertOrgScopedCreationCanProceed(org: string): Promise { } async function listTeamsForImplicitInit( - org: string + org: string, + unattended: boolean ): Promise { + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { return await listTeams(org); } catch (error) { // 403 from listTeams means the user cannot inspect team access. Continue // without a team so init mirrors onboarding's org-scoped auto-team path. + if ( + error instanceof ApiError && + (error.status === 401 || error.status === 403) && + (await scopeRecovery.shouldDelegate(error, { unattended })) + ) { + throw error; + } if (error instanceof ApiError && error.status === 403) { - if (await currentOAuthGrantNeedsRefresh()) { - throw error; - } await assertOrgScopedCreationCanProceed(org); return; } @@ -444,7 +457,10 @@ async function resolveImplicitTeam( initial: WizardOptions, ui: WizardUI ): Promise { - const teams = await listTeamsForImplicitInit(org); + const teams = await listTeamsForImplicitInit( + org, + initial.yes || initial.dryRun + ); if (!teams) { return; } @@ -482,16 +498,7 @@ async function resolveImplicitTeam( * directly. 401: token is invalid/expired — supplying an org won't help, only * re-authenticating will. */ -async function handleOrgListError( - error: unknown -): Promise<{ ok: false; error: string }> { - if ( - error instanceof ApiError && - (error.status === 401 || error.status === 403) && - (await currentOAuthGrantNeedsRefresh()) - ) { - throw error; - } +function handleOrgListError(error: unknown): { ok: false; error: string } { if (error instanceof ApiError && error.status === 403) { const lines: string[] = ["Could not list organizations (403 Forbidden)."]; if (error.detail) { @@ -526,10 +533,14 @@ async function resolveOrgSlug( } let orgs: Awaited>; + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { orgs = await listOrganizations(); } catch (error) { - return await handleOrgListError(error); + if (await scopeRecovery.shouldDelegate(error, { unattended: yes })) { + throw error; + } + return handleOrgListError(error); } orgs.sort( (left, right) => diff --git a/packages/cli/src/lib/init/tools/create-sentry-project.ts b/packages/cli/src/lib/init/tools/create-sentry-project.ts index 4431a19aed..c75e2cc310 100644 --- a/packages/cli/src/lib/init/tools/create-sentry-project.ts +++ b/packages/cli/src/lib/init/tools/create-sentry-project.ts @@ -15,7 +15,7 @@ import { } from "../../api-client.js"; import { ApiError } from "../../errors.js"; import { resolveOrCreateTeam } from "../../resolve-team.js"; -import { currentOAuthGrantNeedsRefresh } from "../../scope-recovery.js"; +import { captureOAuthScopeRecoveryGate } from "../../scope-recovery.js"; import { slugify } from "../../utils.js"; import { tryGetExistingProjectData } from "../existing-project.js"; import { formatMemberProjectCreationDisabledError } from "../project-creation-errors.js"; @@ -185,14 +185,6 @@ async function validateTeamForDryRun( } } -async function shouldRefreshOAuth(error: unknown): Promise { - return ( - error instanceof ApiError && - (error.status === 401 || error.status === 403) && - (await currentOAuthGrantNeedsRefresh()) - ); -} - /** * Create a new Sentry project using the org that preflight already resolved. * When preflight does not resolve a Team Admin team, creation uses the same @@ -209,7 +201,13 @@ export async function createSentryProject( payload: CreateSentryProjectPayload | EnsureSentryProjectPayload, context: Pick< ToolContext, - "dryRun" | "existingProject" | "isExplicitTeam" | "org" | "team" | "project" + | "dryRun" + | "existingProject" + | "isExplicitTeam" + | "org" + | "team" + | "project" + | "yes" > ): Promise { const name = context.project ?? payload.params.name; @@ -229,6 +227,7 @@ export async function createSentryProject( }; } + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { const existingProject = await tryGetExistingProjectData(context.org, slug); if (existingProject) { @@ -277,10 +276,6 @@ export async function createSentryProject( }, }; } catch (error) { - if (await shouldRefreshOAuth(error)) { - throw error; - } - // Org-level policy: member project creation is disabled on this org. // Surface a clear message with the escape hatch. if ( @@ -293,6 +288,13 @@ export async function createSentryProject( error: formatMemberProjectCreationDisabledError(context.org), }; } + if ( + await scopeRecovery.shouldDelegate(error, { + unattended: context.yes || context.dryRun, + }) + ) { + throw error; + } // 409: project already exists (from either the team-scoped or org-scoped // endpoint — both propagate here). Surface a friendly message with a view // hint rather than the raw API error text. diff --git a/packages/cli/src/lib/interactive-prompts.ts b/packages/cli/src/lib/interactive-prompts.ts index e7ef9eec03..5a2068d71d 100644 --- a/packages/cli/src/lib/interactive-prompts.ts +++ b/packages/cli/src/lib/interactive-prompts.ts @@ -1,12 +1,11 @@ /** * Process-wide gate for interactive prompts. * - * Commands that emit machine-readable output (`--json`, or `SENTRY_OUTPUT_FORMAT=json`) - * must never block on an interactive prompt or interleave prompt UI with JSON on - * stdout. The command wrapper disables prompts for such runs via + * Commands that emit machine-readable output or explicitly run unattended + * (`--json`, `--yes`, or `--dry-run`) must never block on an interactive prompt. + * The command wrapper disables prompts from parsed flag values via * {@link setInteractivePromptsAllowed}, and prompt sites (e.g. the org/project - * picker in `resolve-target.ts`) consult {@link interactivePromptsAllowed} - * before showing a prompt. + * picker in `resolve-target.ts`) consult {@link interactivePromptsAllowed}. * * Defaults to `true` so code paths that run outside the command wrapper (tests, * library callers) fall back to their own TTY checks rather than being silently diff --git a/packages/cli/src/lib/scope-recovery.ts b/packages/cli/src/lib/scope-recovery.ts index 31616c1626..1079528699 100644 --- a/packages/cli/src/lib/scope-recovery.ts +++ b/packages/cli/src/lib/scope-recovery.ts @@ -32,17 +32,6 @@ const defaultRuntime: ScopeRecoveryRuntime = { write: (message) => process.stderr.write(message), }; -function disablesInteractiveRecovery(argv: string[]): boolean { - return argv.some( - (arg) => - arg === "--yes" || - arg === "-y" || - arg.startsWith("--yes=") || - arg === "--dry-run" || - arg.startsWith("--dry-run=") - ); -} - async function inspectOAuthScopes( runtime: ScopeRecoveryRuntime, startedWithOAuth = false @@ -77,13 +66,52 @@ async function inspectOAuthScopes( } /** Whether the active stored OAuth token is invalid or lacks a current CLI scope. */ -export async function currentOAuthGrantNeedsRefresh( - runtime: ScopeRecoveryRuntime = defaultRuntime +async function currentOAuthGrantNeedsRefresh( + runtime: ScopeRecoveryRuntime, + startedWithOAuth: boolean ): Promise { - const state = await inspectOAuthScopes(runtime); + const state = await inspectOAuthScopes(runtime, startedWithOAuth); return Boolean(state && state.kind !== "current"); } +function hasActiveOAuthGrant( + runtime: ScopeRecoveryRuntime = defaultRuntime +): boolean { + try { + return runtime.getAuthSource() === "oauth"; + } catch { + return false; + } +} + +export type OAuthScopeRecoveryGate = { + readonly shouldDelegate: ( + error: unknown, + options: { unattended: boolean } + ) => Promise; +}; + +/** Preserve command-specific fallbacks unless central OAuth recovery can own the error. */ +export function captureOAuthScopeRecoveryGate( + runtime: ScopeRecoveryRuntime = defaultRuntime +): OAuthScopeRecoveryGate { + const startedWithOAuth = hasActiveOAuthGrant(runtime); + return { + shouldDelegate: async (error, options) => { + if ( + !(error instanceof ApiError) || + (error.status !== 401 && error.status !== 403) || + options.unattended || + !runtime.inputIsTty() || + !runtime.promptsAllowed() + ) { + return false; + } + return await currentOAuthGrantNeedsRefresh(runtime, startedWithOAuth); + }, + }; +} + async function refreshOAuthScopes( state: Exclude, runInteractiveLogin: InteractiveLogin, @@ -125,12 +153,7 @@ export async function runWithScopeRecovery( runInteractiveLogin: InteractiveLogin, runtime: ScopeRecoveryRuntime = defaultRuntime ): Promise { - let startedWithOAuth = false; - try { - startedWithOAuth = runtime.getAuthSource() === "oauth"; - } catch { - // The original command remains authoritative if the credential store fails. - } + const startedWithOAuth = hasActiveOAuthGrant(runtime); try { await proceed(argv); } catch (error) { @@ -145,7 +168,6 @@ export async function runWithScopeRecovery( if ( !state || state.kind === "current" || - disablesInteractiveRecovery(argv) || !(await refreshOAuthScopes(state, runInteractiveLogin, runtime)) ) { throw error; diff --git a/packages/cli/test/lib/command.test.ts b/packages/cli/test/lib/command.test.ts index 981a5fb569..a251151e59 100644 --- a/packages/cli/test/lib/command.test.ts +++ b/packages/cli/test/lib/command.test.ts @@ -28,7 +28,12 @@ import { } from "../../src/lib/command.js"; import { EXIT, OutputError } from "../../src/lib/errors.js"; import { CommandOutput } from "../../src/lib/formatters/output.js"; +import { + interactivePromptsAllowed, + setInteractivePromptsAllowed, +} from "../../src/lib/interactive-prompts.js"; import { LOG_LEVEL_NAMES, logger, setLogLevel } from "../../src/lib/logger.js"; +import { DRY_RUN_FLAG, YES_FLAG } from "../../src/lib/mutate-command.js"; import { resolveOrgAndProject } from "../../src/lib/resolve-target.js"; import { buildRouteMap } from "../../src/lib/route-map.js"; @@ -75,6 +80,75 @@ function createTestContext() { }; } +test("derives prompt availability from parsed flags instead of ambiguous aliases", async () => { + type InitPromptFlags = { yes: boolean; "dry-run": boolean }; + type DataFlags = { limit?: number; row?: number }; + const promptStates: boolean[] = []; + + const initCommand = buildCommand({ + auth: false, + docs: { brief: "Init" }, + parameters: { + flags: { yes: YES_FLAG, "dry-run": DRY_RUN_FLAG }, + aliases: { y: "yes", n: "dry-run" }, + }, + // biome-ignore lint/correctness/useYield: test command only observes wrapper state + async *func() { + promptStates.push(interactivePromptsAllowed()); + }, + }); + const dataCommand = buildCommand({ + auth: false, + docs: { brief: "Data" }, + parameters: { + flags: { + limit: { + kind: "parsed", + parse: numberParser, + brief: "Limit", + optional: true, + }, + row: { + kind: "parsed", + parse: numberParser, + brief: "Row", + optional: true, + }, + }, + aliases: { n: "limit", y: "row" }, + }, + // biome-ignore lint/correctness/useYield: test command only observes wrapper state + async *func() { + promptStates.push(interactivePromptsAllowed()); + }, + }); + const routeMap = buildRouteMap({ + routes: { init: initCommand, data: dataCommand }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const ctx = createTestContext(); + const cases: Array<{ + args: string[]; + expected: boolean; + }> = [ + { args: ["init", "-y"], expected: false }, + { args: ["init", "-n"], expected: false }, + { args: ["init", "--yes=false"], expected: true }, + { args: ["data", "-n", "10"], expected: true }, + { args: ["data", "-y", "2"], expected: true }, + ]; + + try { + for (const testCase of cases) { + await run(app, testCase.args, ctx as TestContext); + expect(promptStates.at(-1)).toBe(testCase.expected); + } + } finally { + setInteractivePromptsAllowed(true); + } +}); + describe("buildCommand", () => { test("builds a valid command object", () => { const command = buildCommand({ diff --git a/packages/cli/test/lib/init/preflight.test.ts b/packages/cli/test/lib/init/preflight.test.ts index 5150d48b9b..b471a84b16 100644 --- a/packages/cli/test/lib/init/preflight.test.ts +++ b/packages/cli/test/lib/init/preflight.test.ts @@ -17,7 +17,7 @@ vi.mock("../../../src/lib/api-client.js", async (importOriginal) => { }); vi.mock("../../../src/lib/scope-recovery.js", () => ({ - currentOAuthGrantNeedsRefresh: vi.fn(), + captureOAuthScopeRecoveryGate: vi.fn(), })); // biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference @@ -134,11 +134,13 @@ let getAuthTokenSpy: ReturnType; let resolveOrCreateTeamSpy: ReturnType; let detectDsnSpy: ReturnType; let resolveDsnByPublicKeySpy: ReturnType; +let shouldDelegateScopeRecovery: ReturnType; beforeEach(() => { - vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockResolvedValue( - false - ); + shouldDelegateScopeRecovery = vi.fn().mockResolvedValue(false); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReturnValue({ + shouldDelegate: shouldDelegateScopeRecovery, + }); resolveOrgPrefetchedSpy = vi .spyOn(prefetch, "resolveOrgPrefetched") .mockResolvedValue({ org: "acme" }); @@ -199,7 +201,7 @@ afterEach(() => { resolveOrCreateTeamSpy.mockRestore(); detectDsnSpy.mockRestore(); resolveDsnByPublicKeySpy.mockRestore(); - vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockReset(); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReset(); process.exitCode = 0; }); @@ -544,6 +546,23 @@ describe("resolveInitContext", () => { expect(errorCall?.message).toContain("sentry init /"); }); + test("preserves an organization-list 403 when OAuth recovery can run", async () => { + const error = new ApiError( + "Failed to list organizations", + 403, + "Missing org:read" + ); + resolveOrgPrefetchedSpy.mockResolvedValue(null); + listOrganizationsSpy.mockRejectedValueOnce(error); + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); + + const { ui } = createMockUI(); + + await expect( + resolveInitContext(makeOptions({ yes: false }), ui) + ).rejects.toBe(error); + }); + test("surfaces 401 guidance when listOrganizations is unauthorized", async () => { resolveOrgPrefetchedSpy.mockResolvedValue(null); listOrganizationsSpy.mockRejectedValueOnce( @@ -593,6 +612,32 @@ describe("resolveInitContext", () => { expect(context?.isExplicitTeam).toBe(false); }); + test("keeps the org-scoped fallback for an unattended explicit-team 403", async () => { + resolveOrCreateTeamSpy.mockRejectedValueOnce( + new ApiError("Forbidden", 403, "No team:admin access") + ); + + const { ui } = createMockUI(); + const context = await resolveInitContext( + makeOptions({ team: "backend", yes: true }), + ui + ); + + expect(context?.team).toBeUndefined(); + }); + + test("preserves an explicit-team 403 when OAuth recovery can run", async () => { + const error = new ApiError("Forbidden", 403, "No team:admin access"); + resolveOrCreateTeamSpy.mockRejectedValueOnce(error); + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); + + const { ui } = createMockUI(); + + await expect( + resolveInitContext(makeOptions({ team: "backend", yes: false }), ui) + ).rejects.toBe(error); + }); + test("swallows 403 from listTeams and resolves context with team:undefined", async () => { listTeamsSpy.mockRejectedValueOnce( new ApiError("Forbidden", 403, "No team:read access") @@ -608,16 +653,48 @@ describe("resolveInitContext", () => { expect(resolveOrCreateTeamSpy).not.toHaveBeenCalled(); }); - test("preserves a 403 when the stored OAuth grant is stale", async () => { + test("preserves a 403 when OAuth scope recovery can run", async () => { const error = new ApiError("Forbidden", 403, "No team:read access"); listTeamsSpy.mockRejectedValueOnce(error); - vi.mocked( - scopeRecovery.currentOAuthGrantNeedsRefresh - ).mockResolvedValueOnce(true); + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); const { ui } = createMockUI(); - await expect(resolveInitContext(makeOptions(), ui)).rejects.toBe(error); + await expect( + resolveInitContext(makeOptions({ yes: false }), ui) + ).rejects.toBe(error); + expect(shouldDelegateScopeRecovery).toHaveBeenCalledWith(error, { + unattended: false, + }); + }); + + test("preserves a recoverable 401 from team lookup", async () => { + const error = new ApiError("Unauthorized", 401, "Invalid token"); + listTeamsSpy.mockRejectedValueOnce(error); + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); + + const { ui } = createMockUI(); + + await expect( + resolveInitContext(makeOptions({ yes: false }), ui) + ).rejects.toBe(error); + }); + + test("keeps the org-scoped fallback when OAuth recovery is unattended", async () => { + listTeamsSpy.mockRejectedValueOnce( + new ApiError("Forbidden", 403, "No team:read access") + ); + + const { ui } = createMockUI(); + const context = await resolveInitContext(makeOptions({ yes: true }), ui); + + expect(context?.team).toBeUndefined(); + expect(shouldDelegateScopeRecovery).toHaveBeenCalledWith( + expect.any(ApiError), + { + unattended: true, + } + ); }); test("preserves rich org-not-found guidance when implicit team lookup returns 404", async () => { diff --git a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts index 6d2bd00381..9f898ca4d5 100644 --- a/packages/cli/test/lib/init/tools/create-sentry-project.test.ts +++ b/packages/cli/test/lib/init/tools/create-sentry-project.test.ts @@ -12,7 +12,7 @@ vi.mock("../../../../src/lib/api-client.js", async (importOriginal) => { }); vi.mock("../../../../src/lib/scope-recovery.js", () => ({ - currentOAuthGrantNeedsRefresh: vi.fn(), + captureOAuthScopeRecoveryGate: vi.fn(), })); // biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference @@ -89,11 +89,13 @@ let createProjectWithAutoTeamSpy: ReturnType; let getProjectSpy: ReturnType; let tryGetPrimaryDsnSpy: ReturnType; let resolveOrCreateTeamSpy: ReturnType; +let shouldDelegateScopeRecovery: ReturnType; beforeEach(() => { - vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockResolvedValue( - false - ); + shouldDelegateScopeRecovery = vi.fn().mockResolvedValue(false); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReturnValue({ + shouldDelegate: shouldDelegateScopeRecovery, + }); createProjectWithDsnSpy = vi .spyOn(apiClient, "createProjectWithDsn") .mockResolvedValue({ @@ -134,13 +136,14 @@ afterEach(() => { getProjectSpy.mockRestore(); tryGetPrimaryDsnSpy.mockRestore(); resolveOrCreateTeamSpy.mockRestore(); - vi.mocked(scopeRecovery.currentOAuthGrantNeedsRefresh).mockReset(); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReset(); }); describe("createSentryProject", () => { test("returns the pre-resolved existing project without creating", async () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: "my-app", @@ -162,6 +165,7 @@ describe("createSentryProject", () => { test("accepts the legacy ensure-sentry-project alias", async () => { const result = await createSentryProject(makeEnsurePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: "my-app", @@ -182,6 +186,7 @@ describe("createSentryProject", () => { test("returns error when project name produces an empty slug", async () => { const result = await createSentryProject(makePayload({ name: "---" }), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -197,6 +202,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -216,6 +222,7 @@ describe("createSentryProject", () => { test("re-checks for an existing project before creating when the slug is known", async () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -232,6 +239,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -247,6 +255,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: true, + yes: true, org: "acme", team: "platform", project: undefined, @@ -267,6 +276,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -295,6 +305,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -325,6 +336,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -345,6 +357,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "backend", isExplicitTeam: true, @@ -355,17 +368,11 @@ describe("createSentryProject", () => { expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); }); - test("lets a 403 from an outdated OAuth grant escape the real tool registry", async () => { - vi.mocked( - scopeRecovery.currentOAuthGrantNeedsRefresh - ).mockResolvedValueOnce(true); + test("lets a recoverable 403 escape the real tool registry", async () => { + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); - createProjectWithDsnSpy.mockRejectedValueOnce( - new ApiError( - "Forbidden", - 403, - "Your organization has disabled this feature for members." - ) + createProjectWithAutoTeamSpy.mockRejectedValueOnce( + new ApiError("Forbidden", 403, "No project:write access") ); const error = await executeTool(makePayload(), { @@ -373,11 +380,64 @@ describe("createSentryProject", () => { yes: false, dryRun: false, org: "acme", - team: "platform", + team: undefined, }).catch((caught: unknown) => caught); expect(error).toBeInstanceOf(ApiError); expect((error as ApiError).status).toBe(403); + expect(shouldDelegateScopeRecovery).toHaveBeenCalledWith( + expect.any(ApiError), + { + unattended: false, + } + ); + }); + + test("keeps the tool fallback when OAuth recovery is unattended", async () => { + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithAutoTeamSpy.mockRejectedValueOnce( + new ApiError("Forbidden", 403, "No project:write access") + ); + + const result = await createSentryProject(makePayload(), { + dryRun: false, + yes: true, + org: "acme", + team: undefined, + project: undefined, + }); + + expect(result.ok).toBe(false); + expect(shouldDelegateScopeRecovery).toHaveBeenCalledWith( + expect.any(ApiError), + { + unattended: true, + } + ); + }); + + test("keeps the policy-specific error when OAuth scopes are stale", async () => { + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithAutoTeamSpy.mockRejectedValueOnce( + new ApiError( + "Forbidden", + 403, + "Your organization has disabled this feature for members." + ) + ); + + const result = await createSentryProject(makePayload(), { + dryRun: false, + yes: false, + org: "acme", + team: undefined, + project: undefined, + }); + + expect(result.ok).toBe(false); + expect(result.error).toContain("disabled for members"); + expect(shouldDelegateScopeRecovery).not.toHaveBeenCalled(); }); test("does not fall back on team-scoped policy 403", async () => { @@ -392,6 +452,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -410,6 +471,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -426,6 +488,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: true, + yes: true, org: "acme", team: undefined, project: undefined, diff --git a/packages/cli/test/lib/scope-recovery.test.ts b/packages/cli/test/lib/scope-recovery.test.ts index 80e1922dd4..11ff67e1ef 100644 --- a/packages/cli/test/lib/scope-recovery.test.ts +++ b/packages/cli/test/lib/scope-recovery.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test, vi } from "vitest"; import { ApiError, AuthError } from "../../src/lib/errors.js"; import { OAUTH_SCOPES } from "../../src/lib/oauth.js"; import { - currentOAuthGrantNeedsRefresh, + captureOAuthScopeRecoveryGate, ensureCurrentOAuthScopes, runWithScopeRecovery, type ScopeRecoveryRuntime, @@ -132,11 +132,7 @@ describe("runWithScopeRecovery", () => { expect(getAuthScopes).not.toHaveBeenCalled(); }); - test.each([ - [["init", "--yes"], "oauth" as const], - [["init", "--dry-run"], "oauth" as const], - [[], "env:SENTRY_AUTH_TOKEN" as const], - ])("does not launch OAuth for unattended commands or env tokens", async (argv, source) => { + test("does not launch OAuth for environment tokens", async () => { const error = new ApiError("Forbidden", 403); const login = vi.fn(); const getAuthScopes = vi.fn().mockResolvedValue([]); @@ -144,16 +140,17 @@ describe("runWithScopeRecovery", () => { await expect( runWithScopeRecovery( vi.fn().mockRejectedValue(error), - argv, + [], login, - runtime({ getAuthSource: () => source, getAuthScopes }) + runtime({ + getAuthSource: () => "env:SENTRY_AUTH_TOKEN", + getAuthScopes, + }) ) ).rejects.toBe(error); expect(login).not.toHaveBeenCalled(); - if (source !== "oauth") { - expect(getAuthScopes).not.toHaveBeenCalled(); - } + expect(getAuthScopes).not.toHaveBeenCalled(); }); test("checks scopes but does not launch OAuth without an interactive TTY", async () => { @@ -231,6 +228,64 @@ describe("runWithScopeRecovery", () => { }); }); +describe("scope recovery availability", () => { + test("only gives init errors to recovery when authorization can run", async () => { + const error = new ApiError("Forbidden", 403); + const availableRuntime = runtime({ + getAuthScopes: vi.fn().mockResolvedValue([]), + }); + expect( + await captureOAuthScopeRecoveryGate(availableRuntime).shouldDelegate( + error, + { unattended: false } + ) + ).toBe(true); + + const blockedCases = [ + { + unattended: true, + runtime: runtime(), + }, + { + unattended: false, + runtime: runtime({ inputIsTty: () => false }), + }, + { + unattended: false, + runtime: runtime({ promptsAllowed: () => false }), + }, + ]; + + for (const blocked of blockedCases) { + const getAuthScopes = vi.mocked(blocked.runtime.getAuthScopes); + expect( + await captureOAuthScopeRecoveryGate(blocked.runtime).shouldDelegate( + error, + { unattended: blocked.unattended } + ) + ).toBe(false); + expect(getAuthScopes).not.toHaveBeenCalled(); + } + }); + + test("retains OAuth provenance when a failed refresh clears stored auth", async () => { + const getAuthSource = vi + .fn<() => "oauth" | undefined>() + .mockReturnValueOnce("oauth") + .mockReturnValueOnce(undefined); + const getAuthScopes = vi.fn(); + const testRuntime = runtime({ getAuthSource, getAuthScopes }); + const scopeRecovery = captureOAuthScopeRecoveryGate(testRuntime); + + expect( + await scopeRecovery.shouldDelegate(new ApiError("Unauthorized", 401), { + unattended: false, + }) + ).toBe(true); + expect(getAuthScopes).not.toHaveBeenCalled(); + }); +}); + describe("upgrade scope check", () => { test("re-authorizes a stale OAuth grant", async () => { const login = vi.fn().mockResolvedValue({ method: "oauth" }); @@ -276,13 +331,4 @@ describe("upgrade scope check", () => { expect(await ensureCurrentOAuthScopes(login, runtime())).toBe(false); expect(login).not.toHaveBeenCalled(); }); - - test("exposes the same scope decision to init error boundaries", async () => { - expect( - await currentOAuthGrantNeedsRefresh( - runtime({ getAuthScopes: vi.fn().mockResolvedValue([]) }) - ) - ).toBe(true); - expect(await currentOAuthGrantNeedsRefresh(runtime())).toBe(false); - }); });