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/app.ts b/packages/cli/src/app.ts index cfc747ceb1..fa057a7a0f 100644 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -67,6 +67,7 @@ import { import { CLI_VERSION } from "./lib/constants.js"; import { reportCliError } from "./lib/error-reporting.js"; import { + ApiError, AuthError, CliError, getExitCode, @@ -280,6 +281,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. * @@ -353,19 +364,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/src/cli.ts b/packages/cli/src/cli.ts index 508ae271af..d4870bf051 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 { scheduleInitForceExitIfRequested } = await import( "./lib/init/force-exit.js" @@ -438,97 +436,16 @@ 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). + * 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) => { - 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/commands/cli/setup.ts b/packages/cli/src/commands/cli/setup.ts index c7a7bf1747..c04370b1f2 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/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/api/auth.ts b/packages/cli/src/lib/api/auth.ts new file mode 100644 index 0000000000..06a757f456 --- /dev/null +++ b/packages/cli/src/lib/api/auth.ts @@ -0,0 +1,21 @@ +import { array, nullable, object, string } from "valibot"; + +import { getControlSiloUrl } from "../sentry-client.js"; +import { apiRequestToRegion } from "./infrastructure.js"; + +const AuthStatusSchema = object({ + auth: nullable( + object({ + scopes: array(string()), + }) + ), +}); + +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/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 ced0864769..3b41673063 100644 --- a/packages/cli/src/lib/init/preflight.ts +++ b/packages/cli/src/lib/init/preflight.ts @@ -7,6 +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 { captureOAuthScopeRecoveryGate } from "../scope-recovery.js"; import { slugify } from "../utils.js"; import { WizardCancelledError } from "./clack-utils.js"; import { tryGetExistingProjectData } from "./existing-project.js"; @@ -82,7 +83,12 @@ async function withPreflightHandling( return null; } - if (error instanceof AuthError || error instanceof HostScopeError) { + if ( + error instanceof AuthError || + error instanceof HostScopeError || + (error instanceof ApiError && + (error.status === 401 || error.status === 403)) + ) { throw error; } @@ -343,6 +349,7 @@ async function resolveTeam( return await resolveImplicitTeam(org, initial, ui); } + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { const result = await resolveOrCreateTeam(org, { team: initial.team, @@ -355,6 +362,15 @@ 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) { return; } @@ -409,13 +425,22 @@ 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) { await assertOrgScopedCreationCanProceed(org); return; @@ -432,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; } @@ -505,9 +533,13 @@ async function resolveOrgSlug( } let orgs: Awaited>; + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { orgs = await listOrganizations(); } catch (error) { + if (await scopeRecovery.shouldDelegate(error, { unattended: yes })) { + throw error; + } return handleOrgListError(error); } orgs.sort( 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..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,6 +15,7 @@ import { } from "../../api-client.js"; import { ApiError } from "../../errors.js"; import { resolveOrCreateTeam } from "../../resolve-team.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"; @@ -200,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; @@ -220,6 +227,7 @@ export async function createSentryProject( }; } + const scopeRecovery = captureOAuthScopeRecoveryGate(); try { const existingProject = await tryGetExistingProjectData(context.org, slug); if (existingProject) { @@ -280,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/init/tools/registry.ts b/packages/cli/src/lib/init/tools/registry.ts index c2ca1a92d2..63892af392 100644 --- a/packages/cli/src/lib/init/tools/registry.ts +++ b/packages/cli/src/lib/init/tools/registry.ts @@ -1,3 +1,4 @@ +import { ApiError } from "../../errors.js"; import type { ToolOperation, ToolPayload, ToolResult } from "../types.js"; import { applyPatchsetTool } from "./apply-patchset.js"; import { @@ -62,6 +63,12 @@ export async function executeTool( try { return await tool.execute(payload as never, context); } catch (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/wizard-runner.ts b/packages/cli/src/lib/init/wizard-runner.ts index 10aa98c733..5ef492164c 100644 --- a/packages/cli/src/lib/init/wizard-runner.ts +++ b/packages/cli/src/lib/init/wizard-runner.ts @@ -1093,6 +1093,8 @@ export async function runWizard(initialOptions: WizardOptions): Promise { } } catch (err) { const isAuthFailure = err instanceof ApiError && err.status === 401; + 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) { @@ -1103,6 +1105,8 @@ export async function runWizard(initialOptions: WizardOptions): Promise { code = 0; } else if (isAuthFailure) { label = INIT_SERVICE_AUTH_FAILED_LABEL; + } else if (isPermissionFailure) { + label = "Sentry API request denied"; } spin.stop(label, code); spinState.running = false; @@ -1125,6 +1129,11 @@ export async function runWizard(initialOptions: WizardOptions): Promise { setTag("wizard.outcome", "errored"); throw err; } + if (isPermissionFailure) { + showFailedFeedback(ui, "Sentry API request denied"); + setTag("wizard.outcome", "errored"); + throw err; + } if (err instanceof WizardError) { showFailedFeedback(ui); setTag("wizard.outcome", "errored"); 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/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..1079528699 --- /dev/null +++ b/packages/cli/src/lib/scope-recovery.ts @@ -0,0 +1,179 @@ +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, AuthError } from "./errors.js"; +import type { LoginResult } from "./interactive-login.js"; +import { interactivePromptsAllowed } from "./interactive-prompts.js"; +import { OAUTH_SCOPES } from "./oauth.js"; + +type InteractiveLogin = () => Promise; + +type OAuthScopeState = + | { kind: "current" } + | { kind: "invalid" } + | { kind: "missing"; scopes: string[] }; + +export type ScopeRecoveryRuntime = { + assertTrustedHost: () => void; + getAuthScopes: () => Promise; + getAuthSource: () => AuthSource | undefined; + inputIsTty: () => boolean; + promptsAllowed: () => boolean; + write: (message: string) => void; +}; + +const defaultRuntime: ScopeRecoveryRuntime = { + assertTrustedHost: assertAutoLoginHostTrusted, + getAuthScopes: getCurrentAuthScopes, + getAuthSource: () => getAuthConfig()?.source, + inputIsTty: () => isatty(0), + promptsAllowed: interactivePromptsAllowed, + write: (message) => process.stderr.write(message), +}; + +async function inspectOAuthScopes( + runtime: ScopeRecoveryRuntime, + startedWithOAuth = false +): Promise { + try { + 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. */ +async function currentOAuthGrantNeedsRefresh( + runtime: ScopeRecoveryRuntime, + startedWithOAuth: boolean +): Promise { + 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, + runtime: ScopeRecoveryRuntime +): Promise { + if (!(runtime.inputIsTty() && runtime.promptsAllowed())) { + return false; + } + + 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()); +} + +/** 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); +} + +/** 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 { + const startedWithOAuth = hasActiveOAuthGrant(runtime); + try { + await proceed(argv); + } catch (error) { + if ( + !(error instanceof ApiError) || + (error.status !== 401 && error.status !== 403) + ) { + throw error; + } + + const state = await inspectOAuthScopes(runtime, startedWithOAuth); + if ( + !state || + state.kind === "current" || + !(await refreshOAuthScopes(state, runInteractiveLogin, runtime)) + ) { + throw error; + } + + runtime.write("\nRetrying command...\n\n"); + await proceed(argv); + } +} diff --git a/packages/cli/test/commands/cli/setup.test.ts b/packages/cli/test/commands/cli/setup.test.ts index 6591a67fed..6a9c17207d 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/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/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/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); + }); +}); 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 1f65205363..b471a84b16 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", () => ({ + captureOAuthScopeRecoveryGate: vi.fn(), +})); + // biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference import * as apiClient from "../../../src/lib/api-client.js"; @@ -71,6 +75,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 = @@ -128,8 +134,13 @@ let getAuthTokenSpy: ReturnType; let resolveOrCreateTeamSpy: ReturnType; let detectDsnSpy: ReturnType; let resolveDsnByPublicKeySpy: ReturnType; +let shouldDelegateScopeRecovery: ReturnType; beforeEach(() => { + shouldDelegateScopeRecovery = vi.fn().mockResolvedValue(false); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReturnValue({ + shouldDelegate: shouldDelegateScopeRecovery, + }); resolveOrgPrefetchedSpy = vi .spyOn(prefetch, "resolveOrgPrefetched") .mockResolvedValue({ org: "acme" }); @@ -190,6 +201,7 @@ afterEach(() => { resolveOrCreateTeamSpy.mockRestore(); detectDsnSpy.mockRestore(); resolveDsnByPublicKeySpy.mockRestore(); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReset(); process.exitCode = 0; }); @@ -534,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( @@ -583,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") @@ -598,6 +653,50 @@ describe("resolveInitContext", () => { expect(resolveOrCreateTeamSpy).not.toHaveBeenCalled(); }); + test("preserves a 403 when OAuth scope recovery can run", async () => { + const error = new ApiError("Forbidden", 403, "No team:read access"); + listTeamsSpy.mockRejectedValueOnce(error); + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); + + const { ui } = createMockUI(); + + 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 () => { 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..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 @@ -11,6 +11,10 @@ vi.mock("../../../../src/lib/api-client.js", async (importOriginal) => { ); }); +vi.mock("../../../../src/lib/scope-recovery.js", () => ({ + captureOAuthScopeRecoveryGate: 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"; @@ -18,10 +22,13 @@ 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, } 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 = @@ -82,8 +89,13 @@ let createProjectWithAutoTeamSpy: ReturnType; let getProjectSpy: ReturnType; let tryGetPrimaryDsnSpy: ReturnType; let resolveOrCreateTeamSpy: ReturnType; +let shouldDelegateScopeRecovery: ReturnType; beforeEach(() => { + shouldDelegateScopeRecovery = vi.fn().mockResolvedValue(false); + vi.mocked(scopeRecovery.captureOAuthScopeRecoveryGate).mockReturnValue({ + shouldDelegate: shouldDelegateScopeRecovery, + }); createProjectWithDsnSpy = vi .spyOn(apiClient, "createProjectWithDsn") .mockResolvedValue({ @@ -124,12 +136,14 @@ afterEach(() => { getProjectSpy.mockRestore(); tryGetPrimaryDsnSpy.mockRestore(); resolveOrCreateTeamSpy.mockRestore(); + 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", @@ -151,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", @@ -171,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, @@ -186,6 +202,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -205,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, @@ -221,6 +239,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -236,6 +255,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: true, + yes: true, org: "acme", team: "platform", project: undefined, @@ -256,6 +276,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -284,6 +305,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -314,6 +336,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -334,6 +357,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "backend", isExplicitTeam: true, @@ -344,6 +368,78 @@ describe("createSentryProject", () => { expect(createProjectWithAutoTeamSpy).not.toHaveBeenCalled(); }); + test("lets a recoverable 403 escape the real tool registry", async () => { + shouldDelegateScopeRecovery.mockResolvedValueOnce(true); + getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); + createProjectWithAutoTeamSpy.mockRejectedValueOnce( + new ApiError("Forbidden", 403, "No project:write access") + ); + + const error = await executeTool(makePayload(), { + directory: "/tmp/test", + yes: false, + dryRun: false, + org: "acme", + 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 () => { getProjectSpy.mockRejectedValueOnce(new ApiError("Not found", 404)); createProjectWithDsnSpy.mockRejectedValueOnce( @@ -356,6 +452,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: "platform", project: undefined, @@ -374,6 +471,7 @@ describe("createSentryProject", () => { const result = await createSentryProject(makePayload(), { dryRun: false, + yes: false, org: "acme", team: undefined, project: undefined, @@ -390,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/init/wizard-runner.test.ts b/packages/cli/test/lib/init/wizard-runner.test.ts index 47346a619a..690848f6c3 100644 --- a/packages/cli/test/lib/init/wizard-runner.test.ts +++ b/packages/cli/test/lib/init/wizard-runner.test.ts @@ -721,6 +721,32 @@ describe("runWizard", () => { expect(lastFeedbackOutcome()).toBe("failed"); }); + test("preserves 403 errors thrown for command-level scope inspection", 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); + executeToolSpy.mockRejectedValue(scopeError); + + await expect(runWizard(makeOptions())).rejects.toBe(scopeError); + + expect(spinnerMock.stop).toHaveBeenCalledWith( + "Sentry API request denied", + 1 + ); + expect(lastCancelMessage()).toBe("Sentry API request denied"); + }); + 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/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..11ff67e1ef --- /dev/null +++ b/packages/cli/test/lib/scope-recovery.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, test, vi } from "vitest"; +import { ApiError, AuthError } from "../../src/lib/errors.js"; +import { OAUTH_SCOPES } from "../../src/lib/oauth.js"; +import { + captureOAuthScopeRecoveryGate, + ensureCurrentOAuthScopes, + runWithScopeRecovery, + type ScopeRecoveryRuntime, +} from "../../src/lib/scope-recovery.js"; + +function runtime( + overrides: Partial = {} +): ScopeRecoveryRuntime { + return { + assertTrustedHost: vi.fn(), + getAuthScopes: vi.fn().mockResolvedValue(OAUTH_SCOPES), + getAuthSource: () => "oauth", + inputIsTty: () => true, + promptsAllowed: () => true, + write: vi.fn(), + ...overrides, + }; +} + +describe("runWithScopeRecovery", () => { + 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(error) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + const testRuntime = runtime({ + getAuthScopes: vi + .fn() + .mockResolvedValue( + OAUTH_SCOPES.filter((scope) => scope !== "team:admin") + ), + }); + + await runWithScopeRecovery( + proceed, + ["project", "create"], + login, + testRuntime + ); + + expect(proceed).toHaveBeenCalledTimes(2); + expect(testRuntime.getAuthScopes).toHaveBeenCalledOnce(); + expect(testRuntime.assertTrustedHost).toHaveBeenCalledOnce(); + expect(login).toHaveBeenCalledWith(); + expect(testRuntime.write).toHaveBeenCalledWith( + expect.stringContaining("team: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(error) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + + await runWithScopeRecovery( + proceed, + [], + login, + runtime({ getAuthScopes: vi.fn().mockResolvedValue(null) }) + ); + + expect(login).toHaveBeenCalledOnce(); + expect(proceed).toHaveBeenCalledTimes(2); + }); + + 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(error) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + + 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(); + + await runWithScopeRecovery( + proceed, + [], + login, + runtime({ getAuthSource, getAuthScopes }) + ); + + expect(login).toHaveBeenCalledOnce(); + expect(getAuthScopes).not.toHaveBeenCalled(); + }); + + test("does not launch OAuth for environment tokens", async () => { + const error = new ApiError("Forbidden", 403); + const login = vi.fn(); + const getAuthScopes = vi.fn().mockResolvedValue([]); + + await expect( + runWithScopeRecovery( + vi.fn().mockRejectedValue(error), + [], + login, + runtime({ + getAuthSource: () => "env:SENTRY_AUTH_TOKEN", + getAuthScopes, + }) + ) + ).rejects.toBe(error); + + expect(login).not.toHaveBeenCalled(); + expect(getAuthScopes).not.toHaveBeenCalled(); + }); + + 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( + vi.fn().mockRejectedValue(error), + [], + login, + runtime({ getAuthScopes, inputIsTty: () => false }) + ) + ).rejects.toBe(error); + + expect(getAuthScopes).toHaveBeenCalledOnce(); + expect(login).not.toHaveBeenCalled(); + }); + + 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( + 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 scope inspection fails", async () => { + const error = new ApiError("Forbidden", 403); + const login = vi.fn(); + + await expect( + runWithScopeRecovery( + vi.fn().mockRejectedValue(error), + [], + login, + runtime({ + getAuthScopes: vi.fn().mockRejectedValue(new Error("offline")), + }) + ) + ).rejects.toBe(error); + expect(login).not.toHaveBeenCalled(); + }); + + test("does not attempt a second recovery when the retry fails", async () => { + const first = new ApiError("Forbidden", 403); + const second = new ApiError("Still forbidden", 403); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(first) + .mockRejectedValueOnce(second); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + + await expect( + runWithScopeRecovery( + proceed, + [], + login, + runtime({ getAuthScopes: vi.fn().mockResolvedValue([]) }) + ) + ).rejects.toBe(second); + expect(login).toHaveBeenCalledOnce(); + expect(proceed).toHaveBeenCalledTimes(2); + }); +}); + +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" }); + 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(); + }); +});