From faa127e0f4df346949acc474d2d7a9b85d6c0298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Fri, 21 Aug 2026 22:32:46 +0200 Subject: [PATCH] fix(upgrade): signal scope refresh via env var, not a version-skewed flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cli upgrade` spawns the *target* binary's `cli setup` and appended `--ensure-auth-scopes` to its args. That flag is brand new (#1373, merged to main but unreleased), so any target that predates it — a downgrade, or a nightly upgrading to the current stable (0.42.2) — hits a strict argument parser that aborts on the unknown flag, failing the whole upgrade with "No flag registered for --ensure-auth-scopes" / exit 252. This already breaks every nightly user trying to move to stable, and will break any downgrade once 0.43.0 ships. Route the intent through the `SENTRY_ENSURE_AUTH_SCOPES` env var instead (same channel already used for `SENTRY_INSTALL_DIR`). An unknown env var is silently ignored by older binaries, whereas an unknown flag is fatal, so the upgrade path is now robust across arbitrary version skew in both directions. `cli setup` honors the env var and still accepts the legacy `--ensure-auth-scopes` flag, so already-deployed pre-fix binaries that pass the flag don't fail their parse when upgrading to a fixed binary. Co-Authored-By: Claude Opus 4.8 --- packages/cli/src/commands/cli/setup.ts | 14 ++++++-- packages/cli/src/commands/cli/upgrade.ts | 22 +++++++++---- packages/cli/test/commands/cli/setup.test.ts | 33 ++++++++++++++++++- .../cli/test/commands/cli/upgrade.test.ts | 30 +++++++++++++---- 4 files changed, 83 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/commands/cli/setup.ts b/packages/cli/src/commands/cli/setup.ts index 7a5ce6b0e..34f4bdcea 100644 --- a/packages/cli/src/commands/cli/setup.ts +++ b/packages/cli/src/commands/cli/setup.ts @@ -516,9 +516,14 @@ export const setupCommand = buildCommand({ brief: "Skip agent skill installation for AI coding assistants", default: false, }, + // Legacy internal flag. `cli upgrade` now signals this intent through the + // SENTRY_ENSURE_AUTH_SCOPES env var (see upgrade.ts) so version-skewed + // spawns don't crash on an unknown flag. Retained (still honored below) + // only so that already-deployed pre-fix binaries, which still pass this + // flag, don't fail their argument parse when upgrading to this binary. "ensure-auth-scopes": { kind: "boolean", - brief: "Refresh an outdated stored OAuth authorization", + brief: "Refresh an outdated stored OAuth authorization (legacy)", default: false, hidden: true as const, }, @@ -579,7 +584,12 @@ export const setupCommand = buildCommand({ warn, }); - if (flags["ensure-auth-scopes"]) { + // Honor either the env var (the mechanism `cli upgrade` uses now) or the + // legacy flag (still passed by pre-fix binaries upgrading to this one). + const ensureAuthScopes = + flags["ensure-auth-scopes"] || + process.env.SENTRY_ENSURE_AUTH_SCOPES === "1"; + if (ensureAuthScopes) { await bestEffort( "Authorization", async () => { diff --git a/packages/cli/src/commands/cli/upgrade.ts b/packages/cli/src/commands/cli/upgrade.ts index f760f80ba..494f976c8 100644 --- a/packages/cli/src/commands/cli/upgrade.ts +++ b/packages/cli/src/commands/cli/upgrade.ts @@ -536,16 +536,26 @@ async function runSetupOnNewBinary(opts: SetupOptions): Promise { if (install) { args.push("--install"); } - if (ensureAuthScopes) { - args.push("--ensure-auth-scopes"); - } if (noAgentSkills) { args.push("--no-agent-skills"); } - const env = installDir - ? { ...process.env, SENTRY_INSTALL_DIR: installDir } - : undefined; + // Signal "refresh OAuth scopes" through an env var, never a CLI flag. The + // spawned binary may be an ARBITRARY version — a downgrade, or a nightly + // upgrading to a stable release that predates this feature — and its strict + // argument parser aborts (non-zero exit) on any flag it doesn't recognize, + // failing the whole upgrade. An unknown env var is silently ignored, so only + // binaries that understand SENTRY_ENSURE_AUTH_SCOPES act on it. Any future + // setup signal that isn't guaranteed to exist in every upgradeable-from/-to + // version must travel this same way. + const childEnv: NodeJS.ProcessEnv = { ...process.env }; + if (installDir) { + childEnv.SENTRY_INSTALL_DIR = installDir; + } + if (ensureAuthScopes) { + childEnv.SENTRY_ENSURE_AUTH_SCOPES = "1"; + } + const env = installDir || ensureAuthScopes ? childEnv : undefined; const exitCode = await spawnWithRetry(binaryPath, args, env); if (exitCode !== 0) { diff --git a/packages/cli/test/commands/cli/setup.test.ts b/packages/cli/test/commands/cli/setup.test.ts index ade184939..7f017261e 100644 --- a/packages/cli/test/commands/cli/setup.test.ts +++ b/packages/cli/test/commands/cli/setup.test.ts @@ -178,7 +178,38 @@ describe("sentry cli setup", () => { expect(getOutput()).toBe(""); }); - test("checks OAuth scopes when invoked by the upgrade command", async () => { + test("checks OAuth scopes when SENTRY_ENSURE_AUTH_SCOPES is set", async () => { + // This is the mechanism `cli upgrade` uses now: an env var, so a + // version-skewed target binary that predates the feature ignores it + // instead of aborting on an unknown flag. + const { context, restore } = createMockContext({ + homeDir: testDir, + env: { SENTRY_ENSURE_AUTH_SCOPES: "1" }, + }); + restoreStderr = restore; + + await run( + app, + [ + "cli", + "setup", + "--quiet", + "--no-modify-path", + "--no-completions", + "--no-agent-skills", + ], + context + ); + + expect(scopeRecovery.ensureCurrentOAuthScopes).toHaveBeenCalledOnce(); + expect(scopeRecovery.ensureCurrentOAuthScopes).toHaveBeenCalledWith( + interactiveLogin.runInteractiveLogin + ); + }); + + test("still honors the legacy --ensure-auth-scopes flag from pre-fix binaries", async () => { + // Retained for backward compat: already-deployed upgrade binaries pass this + // flag, and their target (this binary) must not fail its argument parse. const { context, restore } = createMockContext({ homeDir: testDir }); restoreStderr = restore; diff --git a/packages/cli/test/commands/cli/upgrade.test.ts b/packages/cli/test/commands/cli/upgrade.test.ts index 47f4f372f..b8af34ab1 100644 --- a/packages/cli/test/commands/cli/upgrade.test.ts +++ b/packages/cli/test/commands/cli/upgrade.test.ts @@ -679,7 +679,11 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy useTestConfigDir("test-upgrade-spawn-"); let testDir: string; - let spawnedArgs: Array<{ cmd: string; args: string[] }>; + let spawnedArgs: Array<{ + cmd: string; + args: string[]; + env?: NodeJS.ProcessEnv; + }>; let spawnSpy: ReturnType; let restoreStderr: (() => void) | undefined; @@ -709,10 +713,16 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy // Spy on child_process.spawn — captures args and resolves with exit 0 spawnSpy = vi .spyOn(child_process, "spawn") - .mockImplementation((cmd: string, args?: readonly string[]) => { - spawnedArgs.push({ cmd, args: [...(args ?? [])] }); - return fakeChildProcess(0); - }); + .mockImplementation( + ( + cmd: string, + args?: readonly string[], + options?: { env?: NodeJS.ProcessEnv } + ) => { + spawnedArgs.push({ cmd, args: [...(args ?? [])], env: options?.env }); + return fakeChildProcess(0); + } + ); }); afterEach(async () => { @@ -780,7 +790,10 @@ 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"); + // Scope-refresh intent travels via env var, not a flag a version-skewed + // target binary could reject. + expect(setupCall?.args).not.toContain("--ensure-auth-scopes"); + expect(setupCall?.env?.SENTRY_ENSURE_AUTH_SCOPES).toBe("1"); }); test("does not launch interactive auth from JSON upgrades", async () => { @@ -794,6 +807,7 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); expect(setupCall).toBeDefined(); expect(setupCall?.args).not.toContain("--ensure-auth-scopes"); + expect(setupCall?.env?.SENTRY_ENSURE_AUTH_SCOPES).toBeUndefined(); }); test("does not pass --no-agent-skills to setup by default", async () => { @@ -840,7 +854,8 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); expect(setupCall?.cmd).toBe(entryPath); - expect(setupCall?.args).toContain("--ensure-auth-scopes"); + expect(setupCall?.args).not.toContain("--ensure-auth-scopes"); + expect(setupCall?.env?.SENTRY_ENSURE_AUTH_SCOPES).toBe("1"); }); test("runs the new Homebrew binary and keeps JSON upgrades non-interactive", async () => { @@ -860,6 +875,7 @@ describe("sentry cli upgrade — curl full upgrade path (child_process.spawn spy const setupCall = spawnedArgs.find((entry) => entry.args.includes("setup")); expect(setupCall?.cmd).toBe(binaryPath); expect(setupCall?.args).not.toContain("--ensure-auth-scopes"); + expect(setupCall?.env?.SENTRY_ENSURE_AUTH_SCOPES).toBeUndefined(); }); test("reports setup failure when spawn exits non-zero", async () => {