fix(auth): prefer stored OAuth over env token by default (#646)

BYK · BYK · commit f76e87b972a2 · 2026-04-07T02:57:48.000Z
When SENTRY_AUTH_TOKEN is set by build tooling (e.g. the Sentry wizard)
but lacks the scopes needed for interactive CLI commands, the CLI now
prefers stored OAuth credentials by default.

OAuth-preferred default: when the user is logged in via OAuth, env
tokens are automatically skipped. This prevents wizard-generated build
tokens from silently overriding interactive credentials. Set
SENTRY_FORCE_ENV_TOKEN=1 to restore the old env-token-first priority.

Key changes:
- getEnvToken() returns undefined when stored OAuth credentials exist,
  cascading through all consumers (isEnvTokenActive, refreshToken, etc.)
- SENTRY_FORCE_ENV_TOKEN env var to force env token priority
- sentry auth login no longer blocks when an env token is present
- sentry auth status shows env token info (active vs bypassed)
- Enhanced 401/403 error messages when env token is the only auth source
diff --git a/src/lib/api/infrastructure.ts b/src/lib/api/infrastructure.ts
@@ -10,6 +10,7 @@
 import * as Sentry from "@sentry/node-core/light";
 import type { z } from "zod";
 
+import { getRawEnvToken } from "../db/auth.js";
 import { getEnv } from "../env.js";
 import { ApiError, AuthError, stringifyUnknown } from "../errors.js";
 import { resolveOrgRegion } from "../region.js";
@@ -57,6 +58,29 @@ export function throwApiError(
     error && typeof error === "object" && "detail" in error
       ? stringifyUnknown((error as { detail: unknown }).detail)
       : stringifyUnknown(error);
+
+  // When an env token is set and we get 401, the HTTP-layer fallback to
+  // stored OAuth already failed (no stored credentials). Convert to AuthError
+  // so the auto-login middleware in cli.ts can trigger interactive login.
+  if (status === 401 && getRawEnvToken()) {
+    throw new AuthError(
+      "not_authenticated",
+      `${context}: ${status} ${response.statusText ?? "Unknown"}.\n` +
+        "  SENTRY_AUTH_TOKEN is set but lacks permissions for this endpoint.\n" +
+        "  Run 'sentry auth login' to authenticate with OAuth."
+    );
+  }
+
+  // For 403 with env token, keep as ApiError but add guidance
+  if (status === 403 && getRawEnvToken()) {
+    throw new ApiError(
+      `${context}: ${status} ${response.statusText ?? "Unknown"}`,
+      status,
+      `${detail}\n\n  SENTRY_AUTH_TOKEN may lack permissions for this endpoint.\n` +
+        "  Run 'sentry auth login' to authenticate with OAuth."
+    );
+  }
+
   throw new ApiError(
     `${context}: ${status} ${response.statusText ?? "Unknown"}`,
     status,
diff --git a/src/lib/db/auth.ts b/src/lib/db/auth.ts
@@ -36,10 +36,34 @@ export type AuthConfig = {
   source: AuthSource;
 };
 
+/**
+ * Read the raw token string from environment variables, ignoring all filters.
+ *
+ * Unlike {@link getEnvToken}, this always returns the env token if set, even
+ * when stored OAuth credentials would normally take priority. Used by the HTTP
+ * layer to check "was an env token provided?" independent of whether it's being
+ * used, and by the per-endpoint permission cache.
+ */
+export function getRawEnvToken(): string | undefined {
+  const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
+  if (authToken) {
+    return authToken;
+  }
+  const sentryToken = getEnv().SENTRY_TOKEN?.trim();
+  if (sentryToken) {
+    return sentryToken;
+  }
+  return;
+}
+
 /**
  * Read token from environment variables.
  * `SENTRY_AUTH_TOKEN` takes priority over `SENTRY_TOKEN` (matches legacy sentry-cli).
  * Empty or whitespace-only values are treated as unset.
+ *
+ * This function is intentionally pure (no DB access). The "prefer stored OAuth
+ * over env token" logic lives in {@link getAuthToken} and {@link getAuthConfig}
+ * which check the DB first when `SENTRY_FORCE_ENV_TOKEN` is not set.
  */
 function getEnvToken(): { token: string; source: AuthSource } | undefined {
   const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
@@ -62,28 +86,39 @@ export function isEnvTokenActive(): boolean {
 }
 
 /**
- * Get the name of the active env var providing authentication.
+ * Get the name of the env var providing a token, for error messages.
  * Returns the specific variable name (e.g. "SENTRY_AUTH_TOKEN" or "SENTRY_TOKEN").
  *
- * **Important**: Call only after checking {@link isEnvTokenActive} returns true.
- * Falls back to "SENTRY_AUTH_TOKEN" if no env source is active, which is a safe
- * default for error messages but may be misleading if used unconditionally.
+ * Uses {@link getRawEnvToken} (not {@link getEnvToken}) so the result is
+ * independent of whether stored OAuth takes priority.
+ * Falls back to "SENTRY_AUTH_TOKEN" if no env var is set.
  */
 export function getActiveEnvVarName(): string {
-  const env = getEnvToken();
-  if (env) {
-    return env.source.slice(ENV_SOURCE_PREFIX.length);
+  const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
+  if (authToken) {
+    return "SENTRY_AUTH_TOKEN";
+  }
+  const sentryToken = getEnv().SENTRY_TOKEN?.trim();
+  if (sentryToken) {
+    return "SENTRY_TOKEN";
   }
   return "SENTRY_AUTH_TOKEN";
 }
 
 export function getAuthConfig(): AuthConfig | undefined {
-  const envToken = getEnvToken();
-  if (envToken) {
-    return { token: envToken.token, source: envToken.source };
+  // When SENTRY_FORCE_ENV_TOKEN is set, check env first (old behavior).
+  // Otherwise, check the DB first — stored OAuth takes priority over env tokens.
+  // This is the core fix for #646: wizard-generated build tokens no longer
+  // silently override the user's interactive login.
+  const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
+  if (forceEnv) {
+    const envToken = getEnvToken();
+    if (envToken) {
+      return { token: envToken.token, source: envToken.source };
+    }
   }
 
-  return withDbSpan("getAuthConfig", () => {
+  const dbConfig = withDbSpan("getAuthConfig", () => {
     const db = getDatabase();
     const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
       | AuthRow
@@ -101,16 +136,34 @@ export function getAuthConfig(): AuthConfig | undefined {
       source: "oauth" as const,
     };
   });
-}
+  if (dbConfig) {
+    return dbConfig;
+  }
 
-/** Get the active auth token. Checks env vars first, then falls back to SQLite. */
-export function getAuthToken(): string | undefined {
+  // No stored OAuth — fall back to env token
   const envToken = getEnvToken();
   if (envToken) {
-    return envToken.token;
+    return { token: envToken.token, source: envToken.source };
+  }
+  return;
+}
+
+/**
+ * Get the active auth token.
+ *
+ * Default: checks the DB first (stored OAuth wins), then falls back to env vars.
+ * With `SENTRY_FORCE_ENV_TOKEN=1`: checks env vars first (old behavior).
+ */
+export function getAuthToken(): string | undefined {
+  const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
+  if (forceEnv) {
+    const envToken = getEnvToken();
+    if (envToken) {
+      return envToken.token;
+    }
   }
 
-  return withDbSpan("getAuthToken", () => {
+  const dbToken = withDbSpan("getAuthToken", () => {
     const db = getDatabase();
     const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
       | AuthRow
@@ -126,6 +179,16 @@ export function getAuthToken(): string | undefined {
 
     return row.token;
   });
+  if (dbToken) {
+    return dbToken;
+  }
+
+  // No stored OAuth — fall back to env token
+  const envToken = getEnvToken();
+  if (envToken) {
+    return envToken.token;
+  }
+  return;
 }
 
 export function setAuthToken(
@@ -179,6 +242,32 @@ export function isAuthenticated(): boolean {
   return !!token;
 }
 
+/**
+ * Check if usable OAuth credentials are stored in the database.
+ *
+ * Returns true when the `auth` table has either:
+ * - A non-expired token, or
+ * - An expired token with a refresh token (will be refreshed on next use)
+ *
+ * Used by the login command to decide whether to prompt for re-authentication
+ * when an env token is present.
+ */
+export function hasStoredAuthCredentials(): boolean {
+  const db = getDatabase();
+  const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
+    | AuthRow
+    | undefined;
+  if (!row?.token) {
+    return false;
+  }
+  // Non-expired token
+  if (!row.expires_at || Date.now() <= row.expires_at) {
+    return true;
+  }
+  // Expired but has refresh token — will be refreshed on next use
+  return !!row.refresh_token;
+}
+
 export type RefreshTokenOptions = {
   /** Bypass threshold check and always refresh */
   force?: boolean;
@@ -229,10 +318,13 @@ async function performTokenRefresh(
 export async function refreshToken(
   options: RefreshTokenOptions = {}
 ): Promise<RefreshTokenResult> {
-  // Env var tokens are assumed valid — no refresh, no expiry check
-  const envToken = getEnvToken();
-  if (envToken) {
-    return { token: envToken.token, refreshed: false };
+  // With SENTRY_FORCE_ENV_TOKEN, env token takes priority (no refresh needed).
+  const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
+  if (forceEnv) {
+    const envToken = getEnvToken();
+    if (envToken) {
+      return { token: envToken.token, refreshed: false };
+    }
   }
 
   const { force = false } = options;
@@ -244,6 +336,11 @@ export async function refreshToken(
     | undefined;
 
   if (!row?.token) {
+    // No stored token — try env token as fallback
+    const envToken = getEnvToken();
+    if (envToken) {
+      return { token: envToken.token, refreshed: false };
+    }
     throw new AuthError("not_authenticated");
   }
 
diff --git a/test/lib/db/auth.property.test.ts b/test/lib/db/auth.property.test.ts
@@ -74,28 +74,34 @@ describe("property: env var priority", () => {
     );
   });
 
-  test("env var always wins over stored token", () => {
+  test("stored OAuth wins over env var (default behavior)", () => {
     fcAssert(
       property(tokenArb, tokenArb, (envToken, storedToken) => {
         setAuthToken(storedToken);
         process.env.SENTRY_AUTH_TOKEN = envToken;
 
-        expect(getAuthToken()).toBe(envToken.trim());
+        // Stored OAuth takes priority — env token is for build tooling
+        expect(getAuthToken()).toBe(storedToken);
+        expect(getAuthConfig()?.source).toBe("oauth" satisfies AuthSource);
       }),
       { numRuns: DEFAULT_NUM_RUNS }
     );
   });
 
-  test("SENTRY_TOKEN wins over stored token when SENTRY_AUTH_TOKEN is absent", () => {
+  test("SENTRY_FORCE_ENV_TOKEN overrides stored OAuth", () => {
     fcAssert(
       property(tokenArb, tokenArb, (envToken, storedToken) => {
         setAuthToken(storedToken);
-        process.env.SENTRY_TOKEN = envToken;
-
-        expect(getAuthToken()).toBe(envToken.trim());
-        expect(getAuthConfig()?.source).toBe(
-          "env:SENTRY_TOKEN" satisfies AuthSource
-        );
+        process.env.SENTRY_AUTH_TOKEN = envToken;
+        try {
+          process.env.SENTRY_FORCE_ENV_TOKEN = "1";
+          expect(getAuthToken()).toBe(envToken.trim());
+          expect(getAuthConfig()?.source).toBe(
+            "env:SENTRY_AUTH_TOKEN" satisfies AuthSource
+          );
+        } finally {
+          delete process.env.SENTRY_FORCE_ENV_TOKEN;
+        }
       }),
       { numRuns: DEFAULT_NUM_RUNS }
     );
@@ -145,37 +151,40 @@ describe("property: env tokens never trigger refresh", () => {
 });
 
 describe("property: isEnvTokenActive consistency", () => {
-  test("isEnvTokenActive matches whether getAuthConfig returns env source", () => {
+  test("when no env token, getAuthConfig never returns env source", () => {
     fcAssert(
-      property(
-        option(tokenArb),
-        option(tokenArb),
-        option(tokenArb),
-        (authTokenOpt, sentryTokenOpt, storedTokenOpt) => {
-          // Clean slate
-          delete process.env.SENTRY_AUTH_TOKEN;
-          delete process.env.SENTRY_TOKEN;
-
-          if (authTokenOpt !== null) {
-            process.env.SENTRY_AUTH_TOKEN = authTokenOpt;
-          }
-          if (sentryTokenOpt !== null) {
-            process.env.SENTRY_TOKEN = sentryTokenOpt;
-          }
-          if (storedTokenOpt !== null) {
-            setAuthToken(storedTokenOpt);
-          }
-
-          const config = getAuthConfig();
-          const envActive = isEnvTokenActive();
-
-          if (envActive) {
-            expect(config?.source).toMatch(/^env:/);
-          } else if (config) {
-            expect(config.source).toBe("oauth");
-          }
+      property(option(tokenArb), (storedTokenOpt) => {
+        // Clean slate — no env tokens
+        delete process.env.SENTRY_AUTH_TOKEN;
+        delete process.env.SENTRY_TOKEN;
+
+        if (storedTokenOpt !== null) {
+          setAuthToken(storedTokenOpt);
+        }
+
+        const config = getAuthConfig();
+        const envActive = isEnvTokenActive();
+
+        expect(envActive).toBe(false);
+        if (config) {
+          expect(config.source).toBe("oauth");
         }
-      ),
+      }),
+      { numRuns: DEFAULT_NUM_RUNS }
+    );
+  });
+
+  test("stored OAuth takes priority: getAuthConfig returns oauth even when env token is set", () => {
+    fcAssert(
+      property(tokenArb, tokenArb, (envToken, storedToken) => {
+        process.env.SENTRY_AUTH_TOKEN = envToken;
+        setAuthToken(storedToken);
+
+        const config = getAuthConfig();
+        expect(config?.source).toBe("oauth");
+        // But isEnvTokenActive is still true (env token exists)
+        expect(isEnvTokenActive()).toBe(true);
+      }),
       { numRuns: DEFAULT_NUM_RUNS }
     );
   });
diff --git a/test/lib/db/auth.test.ts b/test/lib/db/auth.test.ts
@@ -114,14 +114,34 @@ describe("env var auth: getActiveEnvVarName", () => {
 });
 
 describe("env var auth: refreshToken edge cases", () => {
-  test("env token skips stored token entirely", async () => {
-    setAuthToken("stored_token", -1, "refresh_token");
+  test("env token used when no stored OAuth exists", async () => {
     process.env.SENTRY_AUTH_TOKEN = "env_token";
     const result = await refreshToken();
     expect(result.token).toBe("env_token");
     expect(result.refreshed).toBe(false);
   });
 
+  test("stored OAuth preferred over env token", async () => {
+    setAuthToken("stored_token", 3600);
+    process.env.SENTRY_AUTH_TOKEN = "env_token";
+    const result = await refreshToken();
+    expect(result.token).toBe("stored_token");
+    expect(result.refreshed).toBe(false);
+  });
+
+  test("SENTRY_FORCE_ENV_TOKEN overrides stored OAuth in refreshToken", async () => {
+    setAuthToken("stored_token", 3600);
+    process.env.SENTRY_AUTH_TOKEN = "env_token";
+    try {
+      process.env.SENTRY_FORCE_ENV_TOKEN = "1";
+      const result = await refreshToken();
+      expect(result.token).toBe("env_token");
+      expect(result.refreshed).toBe(false);
+    } finally {
+      delete process.env.SENTRY_FORCE_ENV_TOKEN;
+    }
+  });
+
   test("has no expiresAt or expiresIn for env tokens", async () => {
     process.env.SENTRY_AUTH_TOKEN = "env_token";
     const result = await refreshToken();
diff --git a/test/lib/db/model-based.test.ts b/test/lib/db/model-based.test.ts
