Skip to content

Commit 8b4a628

Browse files
committed
fix(auth): prefer stored OAuth over env token by default (#646)
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
1 parent ca14e7c commit 8b4a628

File tree

11 files changed

+281
-73
lines changed

11 files changed

+281
-73
lines changed

src/commands/auth/login.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { buildCommand, numberParser } from "../../lib/command.js";
99
import {
1010
clearAuth,
1111
getActiveEnvVarName,
12+
hasStoredAuthCredentials,
1213
isAuthenticated,
1314
isEnvTokenActive,
1415
setAuthToken,
@@ -71,11 +72,15 @@ type LoginFlags = {
7172
async function handleExistingAuth(force: boolean): Promise<boolean> {
7273
if (isEnvTokenActive()) {
7374
const envVar = getActiveEnvVarName();
74-
log.info(
75-
`Authentication is provided via ${envVar} environment variable. ` +
76-
`Unset ${envVar} to use OAuth-based login instead.`
75+
log.warn(
76+
`${envVar} is set in your environment (likely from build tooling).\n` +
77+
" OAuth credentials will be stored separately and used for CLI commands."
7778
);
78-
return false;
79+
// If no stored OAuth token exists, proceed directly to login
80+
if (!hasStoredAuthCredentials()) {
81+
return true;
82+
}
83+
// Fall through to the re-auth confirmation logic below
7984
}
8085

8186
if (!force) {

src/commands/auth/status.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@ import {
1111
type AuthConfig,
1212
type AuthSource,
1313
ENV_SOURCE_PREFIX,
14+
getActiveEnvVarName,
1415
getAuthConfig,
16+
getRawEnvToken,
1517
isAuthenticated,
18+
isEnvTokenActive,
1619
} from "../../lib/db/auth.js";
1720
import {
1821
getDefaultOrganization,
@@ -77,6 +80,13 @@ export type AuthStatusData = {
7780
/** Error message if verification failed */
7881
error?: string;
7982
};
83+
/** Environment variable token info (present when SENTRY_AUTH_TOKEN or SENTRY_TOKEN is set) */
84+
envToken?: {
85+
/** Name of the env var (e.g., "SENTRY_AUTH_TOKEN") */
86+
envVar: string;
87+
/** Whether the env token is the effective auth source (vs bypassed for OAuth) */
88+
active: boolean;
89+
};
8090
};
8191

8292
/**
@@ -186,6 +196,13 @@ export const statusCommand = buildCommand({
186196
: undefined;
187197
}
188198

199+
// Check for env token regardless of whether it's the active source
200+
// (it may be set but bypassed because stored OAuth takes priority)
201+
const rawEnv = getRawEnvToken();
202+
const envTokenData: AuthStatusData["envToken"] = rawEnv
203+
? { envVar: getActiveEnvVarName(), active: isEnvTokenActive() }
204+
: undefined;
205+
189206
const data: AuthStatusData = {
190207
authenticated: true,
191208
source: auth?.source ?? "oauth",
@@ -194,6 +211,7 @@ export const statusCommand = buildCommand({
194211
token: collectTokenInfo(auth, flags["show-token"]),
195212
defaults: collectDefaults(),
196213
verification: await verifyCredentials(),
214+
envToken: envTokenData,
197215
};
198216

199217
yield new CommandOutput(data);

src/lib/api/infrastructure.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import * as Sentry from "@sentry/node-core/light";
1111
import type { z } from "zod";
1212

13+
import { getRawEnvToken } from "../db/auth.js";
1314
import { getEnv } from "../env.js";
1415
import { ApiError, AuthError, stringifyUnknown } from "../errors.js";
1516
import { resolveOrgRegion } from "../region.js";
@@ -57,6 +58,29 @@ export function throwApiError(
5758
error && typeof error === "object" && "detail" in error
5859
? stringifyUnknown((error as { detail: unknown }).detail)
5960
: stringifyUnknown(error);
61+
62+
// When an env token is set and we get 401, the HTTP-layer fallback to
63+
// stored OAuth already failed (no stored credentials). Convert to AuthError
64+
// so the auto-login middleware in cli.ts can trigger interactive login.
65+
if (status === 401 && getRawEnvToken()) {
66+
throw new AuthError(
67+
"not_authenticated",
68+
`${context}: ${status} ${response.statusText ?? "Unknown"}.\n` +
69+
" SENTRY_AUTH_TOKEN is set but lacks permissions for this endpoint.\n" +
70+
" Run 'sentry auth login' to authenticate with OAuth."
71+
);
72+
}
73+
74+
// For 403 with env token, keep as ApiError but add guidance
75+
if (status === 403 && getRawEnvToken()) {
76+
throw new ApiError(
77+
`${context}: ${status} ${response.statusText ?? "Unknown"}`,
78+
status,
79+
`${detail}\n\n SENTRY_AUTH_TOKEN may lack permissions for this endpoint.\n` +
80+
" Run 'sentry auth login' to authenticate with OAuth."
81+
);
82+
}
83+
6084
throw new ApiError(
6185
`${context}: ${status} ${response.statusText ?? "Unknown"}`,
6286
status,

src/lib/db/auth.ts

Lines changed: 113 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,19 @@ type AuthRow = {
2525
/** Prefix for environment variable auth sources in {@link AuthSource} */
2626
export const ENV_SOURCE_PREFIX = "env:";
2727

28+
/**
29+
* Process-level cache for the hasStoredAuthCredentials result.
30+
* Keyed by config dir so it auto-resets when tests change SENTRY_CONFIG_DIR.
31+
* Also reset on clearAuth() (logout) and setAuthToken() (login).
32+
*/
33+
const _credentialCache = { value: null as boolean | null, configDir: "" };
34+
35+
/** Reset the credential cache. Called from closeDatabase() during test teardown. */
36+
export function resetCredentialCache(): void {
37+
_credentialCache.value = null;
38+
_credentialCache.configDir = "";
39+
}
40+
2841
/** Where the auth token originated */
2942
export type AuthSource = "env:SENTRY_AUTH_TOKEN" | "env:SENTRY_TOKEN" | "oauth";
3043

@@ -36,21 +49,75 @@ export type AuthConfig = {
3649
source: AuthSource;
3750
};
3851

52+
/**
53+
* Read the raw token string from environment variables, ignoring all filters.
54+
*
55+
* Unlike {@link getEnvToken}, this always returns the env token if set, even
56+
* when stored OAuth credentials would normally take priority. Used by the HTTP
57+
* layer to check "was an env token provided?" independent of whether it's being
58+
* used, and by the per-endpoint permission cache.
59+
*/
60+
export function getRawEnvToken(): string | undefined {
61+
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
62+
if (authToken) {
63+
return authToken;
64+
}
65+
const sentryToken = getEnv().SENTRY_TOKEN?.trim();
66+
if (sentryToken) {
67+
return sentryToken;
68+
}
69+
return;
70+
}
71+
3972
/**
4073
* Read token from environment variables.
4174
* `SENTRY_AUTH_TOKEN` takes priority over `SENTRY_TOKEN` (matches legacy sentry-cli).
4275
* Empty or whitespace-only values are treated as unset.
76+
*
77+
* **Default behavior**: When the user is logged in (stored OAuth credentials
78+
* exist in the database), env tokens are skipped so OAuth takes priority. This
79+
* prevents wizard-generated build tokens from silently overriding interactive
80+
* CLI credentials. Set `SENTRY_FORCE_ENV_TOKEN=1` to override this and force
81+
* the env token to take priority (e.g., for CI pipelines that intentionally
82+
* use an env token alongside stored credentials).
4383
*/
4484
function getEnvToken(): { token: string; source: AuthSource } | undefined {
85+
const raw = getRawEnvToken();
86+
if (!raw) {
87+
return;
88+
}
89+
90+
// When OAuth credentials are stored and env token is not forced, prefer OAuth.
91+
// This is the core fix for #646: wizard-generated tokens no longer silently
92+
// override the user's interactive login.
93+
//
94+
// The result is cached per-process to avoid repeated DB queries and to prevent
95+
// opening the DB as a side effect of reading an env var on the very first call.
96+
// Cache is invalidated by setAuthToken() and clearAuth().
97+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
98+
if (!forceEnv) {
99+
const currentDir = getEnv().SENTRY_CONFIG_DIR ?? "";
100+
if (
101+
_credentialCache.value === null ||
102+
_credentialCache.configDir !== currentDir
103+
) {
104+
_credentialCache.configDir = currentDir;
105+
try {
106+
_credentialCache.value = hasStoredAuthCredentials();
107+
} catch {
108+
_credentialCache.value = false;
109+
}
110+
}
111+
if (_credentialCache.value) {
112+
return;
113+
}
114+
}
115+
45116
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
46117
if (authToken) {
47118
return { token: authToken, source: "env:SENTRY_AUTH_TOKEN" };
48119
}
49-
const sentryToken = getEnv().SENTRY_TOKEN?.trim();
50-
if (sentryToken) {
51-
return { token: sentryToken, source: "env:SENTRY_TOKEN" };
52-
}
53-
return;
120+
return { token: raw, source: "env:SENTRY_TOKEN" };
54121
}
55122

56123
/**
@@ -62,17 +129,21 @@ export function isEnvTokenActive(): boolean {
62129
}
63130

64131
/**
65-
* Get the name of the active env var providing authentication.
132+
* Get the name of the env var providing a token, for error messages.
66133
* Returns the specific variable name (e.g. "SENTRY_AUTH_TOKEN" or "SENTRY_TOKEN").
67134
*
68-
* **Important**: Call only after checking {@link isEnvTokenActive} returns true.
69-
* Falls back to "SENTRY_AUTH_TOKEN" if no env source is active, which is a safe
70-
* default for error messages but may be misleading if used unconditionally.
135+
* Uses {@link getRawEnvToken} (not {@link getEnvToken}) so the result is
136+
* independent of whether stored OAuth takes priority.
137+
* Falls back to "SENTRY_AUTH_TOKEN" if no env var is set.
71138
*/
72139
export function getActiveEnvVarName(): string {
73-
const env = getEnvToken();
74-
if (env) {
75-
return env.source.slice(ENV_SOURCE_PREFIX.length);
140+
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
141+
if (authToken) {
142+
return "SENTRY_AUTH_TOKEN";
143+
}
144+
const sentryToken = getEnv().SENTRY_TOKEN?.trim();
145+
if (sentryToken) {
146+
return "SENTRY_TOKEN";
76147
}
77148
return "SENTRY_AUTH_TOKEN";
78149
}
@@ -133,6 +204,7 @@ export function setAuthToken(
133204
expiresIn?: number,
134205
newRefreshToken?: string
135206
): void {
207+
_credentialCache.value = true; // Invalidate: we're storing credentials
136208
withDbSpan("setAuthToken", () => {
137209
const db = getDatabase();
138210
const now = Date.now();
@@ -156,6 +228,7 @@ export function setAuthToken(
156228
}
157229

158230
export async function clearAuth(): Promise<void> {
231+
_credentialCache.value = null; // Invalidate: credentials being cleared
159232
withDbSpan("clearAuth", () => {
160233
const db = getDatabase();
161234
db.query("DELETE FROM auth WHERE id = 1").run();
@@ -179,6 +252,33 @@ export function isAuthenticated(): boolean {
179252
return !!token;
180253
}
181254

255+
/**
256+
* Check if usable OAuth credentials are stored in the database.
257+
*
258+
* Returns true when the `auth` table has either:
259+
* - A non-expired token, or
260+
* - An expired token with a refresh token (will be refreshed on next use)
261+
*
262+
* This is the gate for the "OAuth-preferred" default: when this returns true,
263+
* {@link getEnvToken} skips the env token so OAuth takes priority. Also used
264+
* by the login command to decide whether to prompt for re-authentication.
265+
*/
266+
export function hasStoredAuthCredentials(): boolean {
267+
const db = getDatabase();
268+
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
269+
| AuthRow
270+
| undefined;
271+
if (!row?.token) {
272+
return false;
273+
}
274+
// Non-expired token
275+
if (!row.expires_at || Date.now() <= row.expires_at) {
276+
return true;
277+
}
278+
// Expired but has refresh token — will be refreshed on next use
279+
return !!row.refresh_token;
280+
}
281+
182282
export type RefreshTokenOptions = {
183283
/** Bypass threshold check and always refresh */
184284
force?: boolean;
@@ -229,7 +329,7 @@ async function performTokenRefresh(
229329
export async function refreshToken(
230330
options: RefreshTokenOptions = {}
231331
): Promise<RefreshTokenResult> {
232-
// Env var tokens are assumed valid — no refresh, no expiry check
332+
// Env var tokens are assumed valid — no refresh, no expiry check.
233333
const envToken = getEnvToken();
234334
if (envToken) {
235335
return { token: envToken.token, refreshed: false };

src/lib/db/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { Database } from "bun:sqlite";
77
import { chmodSync, mkdirSync } from "node:fs";
88
import { join } from "node:path";
99
import { getEnv } from "../env.js";
10+
import { resetCredentialCache } from "./auth.js";
1011
import { migrateFromJson } from "./migration.js";
1112
import { initSchema, runMigrations } from "./schema.js";
1213

@@ -130,6 +131,7 @@ export function closeDatabase(): void {
130131
rawDb = null;
131132
dbOpenedPath = null;
132133
}
134+
resetCredentialCache();
133135
}
134136

135137
/**

src/lib/formatters/human.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,6 +1826,10 @@ export function formatAuthStatus(data: AuthStatusData): string {
18261826
lines.push(mdKvTable(authRows));
18271827
}
18281828

1829+
if (data.envToken) {
1830+
lines.push(formatEnvTokenSection(data.envToken));
1831+
}
1832+
18291833
if (data.defaults) {
18301834
lines.push(formatDefaultsSection(data.defaults));
18311835
}
@@ -1837,6 +1841,24 @@ export function formatAuthStatus(data: AuthStatusData): string {
18371841
return renderMarkdown(lines.join("\n"));
18381842
}
18391843

1844+
/**
1845+
* Format the env token status section.
1846+
* Shows whether the env token is active or bypassed, and how many endpoints
1847+
* have been marked insufficient.
1848+
*/
1849+
function formatEnvTokenSection(
1850+
envToken: NonNullable<AuthStatusData["envToken"]>
1851+
): string {
1852+
const status = envToken.active
1853+
? "active"
1854+
: "set but not used (using OAuth credentials)";
1855+
const rows: [string, string][] = [
1856+
["Env var", safeCodeSpan(envToken.envVar)],
1857+
["Status", status],
1858+
];
1859+
return `\n${mdKvTable(rows, "Environment Token")}`;
1860+
}
1861+
18401862
// Project Creation Formatting
18411863

18421864
/** Input for the project-created success formatter */

0 commit comments

Comments
 (0)