Skip to content

Commit 5a30349

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 5a30349

File tree

11 files changed

+347
-116
lines changed

11 files changed

+347
-116
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: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import {
1111
type AuthConfig,
1212
type AuthSource,
1313
ENV_SOURCE_PREFIX,
14+
getActiveEnvVarName,
1415
getAuthConfig,
16+
getRawEnvToken,
1517
isAuthenticated,
1618
} from "../../lib/db/auth.js";
1719
import {
@@ -77,6 +79,13 @@ export type AuthStatusData = {
7779
/** Error message if verification failed */
7880
error?: string;
7981
};
82+
/** Environment variable token info (present when SENTRY_AUTH_TOKEN or SENTRY_TOKEN is set) */
83+
envToken?: {
84+
/** Name of the env var (e.g., "SENTRY_AUTH_TOKEN") */
85+
envVar: string;
86+
/** Whether the env token is the effective auth source (vs bypassed for OAuth) */
87+
active: boolean;
88+
};
8089
};
8190

8291
/**
@@ -186,6 +195,13 @@ export const statusCommand = buildCommand({
186195
: undefined;
187196
}
188197

198+
// Check for env token regardless of whether it's the active source
199+
// (it may be set but bypassed because stored OAuth takes priority)
200+
const rawEnv = getRawEnvToken();
201+
const envTokenData: AuthStatusData["envToken"] = rawEnv
202+
? { envVar: getActiveEnvVarName(), active: fromEnv }
203+
: undefined;
204+
189205
const data: AuthStatusData = {
190206
authenticated: true,
191207
source: auth?.source ?? "oauth",
@@ -194,6 +210,7 @@ export const statusCommand = buildCommand({
194210
token: collectTokenInfo(auth, flags["show-token"]),
195211
defaults: collectDefaults(),
196212
verification: await verifyCredentials(),
213+
envToken: envTokenData,
197214
};
198215

199216
yield new CommandOutput(data);

src/lib/api/infrastructure.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export function throwApiError(
5757
error && typeof error === "object" && "detail" in error
5858
? stringifyUnknown((error as { detail: unknown }).detail)
5959
: stringifyUnknown(error);
60+
6061
throw new ApiError(
6162
`${context}: ${status} ${response.statusText ?? "Unknown"}`,
6263
status,

src/lib/db/auth.ts

Lines changed: 116 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,34 @@ export type AuthConfig = {
3636
source: AuthSource;
3737
};
3838

39+
/**
40+
* Read the raw token string from environment variables, ignoring all filters.
41+
*
42+
* Unlike {@link getEnvToken}, this always returns the env token if set, even
43+
* when stored OAuth credentials would normally take priority. Used by the HTTP
44+
* layer to check "was an env token provided?" independent of whether it's being
45+
* used, and by the per-endpoint permission cache.
46+
*/
47+
export function getRawEnvToken(): string | undefined {
48+
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
49+
if (authToken) {
50+
return authToken;
51+
}
52+
const sentryToken = getEnv().SENTRY_TOKEN?.trim();
53+
if (sentryToken) {
54+
return sentryToken;
55+
}
56+
return;
57+
}
58+
3959
/**
4060
* Read token from environment variables.
4161
* `SENTRY_AUTH_TOKEN` takes priority over `SENTRY_TOKEN` (matches legacy sentry-cli).
4262
* Empty or whitespace-only values are treated as unset.
63+
*
64+
* This function is intentionally pure (no DB access). The "prefer stored OAuth
65+
* over env token" logic lives in {@link getAuthToken} and {@link getAuthConfig}
66+
* which check the DB first when `SENTRY_FORCE_ENV_TOKEN` is not set.
4367
*/
4468
function getEnvToken(): { token: string; source: AuthSource } | undefined {
4569
const authToken = getEnv().SENTRY_AUTH_TOKEN?.trim();
@@ -62,28 +86,36 @@ export function isEnvTokenActive(): boolean {
6286
}
6387

6488
/**
65-
* Get the name of the active env var providing authentication.
66-
* Returns the specific variable name (e.g. "SENTRY_AUTH_TOKEN" or "SENTRY_TOKEN").
67-
*
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.
89+
* Get the name of the env var providing a token, for error messages.
90+
* Returns the specific variable name (e.g. "SENTRY_AUTH_TOKEN" or "SENTRY_TOKEN")
91+
* by checking which env var {@link getRawEnvToken} would read.
92+
* Falls back to "SENTRY_AUTH_TOKEN" if no env var is set.
7193
*/
7294
export function getActiveEnvVarName(): string {
73-
const env = getEnvToken();
74-
if (env) {
75-
return env.source.slice(ENV_SOURCE_PREFIX.length);
95+
// Match getRawEnvToken() priority: SENTRY_AUTH_TOKEN first, then SENTRY_TOKEN
96+
if (getEnv().SENTRY_AUTH_TOKEN?.trim()) {
97+
return "SENTRY_AUTH_TOKEN";
98+
}
99+
if (getEnv().SENTRY_TOKEN?.trim()) {
100+
return "SENTRY_TOKEN";
76101
}
77102
return "SENTRY_AUTH_TOKEN";
78103
}
79104

80105
export function getAuthConfig(): AuthConfig | undefined {
81-
const envToken = getEnvToken();
82-
if (envToken) {
83-
return { token: envToken.token, source: envToken.source };
106+
// When SENTRY_FORCE_ENV_TOKEN is set, check env first (old behavior).
107+
// Otherwise, check the DB first — stored OAuth takes priority over env tokens.
108+
// This is the core fix for #646: wizard-generated build tokens no longer
109+
// silently override the user's interactive login.
110+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
111+
if (forceEnv) {
112+
const envToken = getEnvToken();
113+
if (envToken) {
114+
return { token: envToken.token, source: envToken.source };
115+
}
84116
}
85117

86-
return withDbSpan("getAuthConfig", () => {
118+
const dbConfig = withDbSpan("getAuthConfig", () => {
87119
const db = getDatabase();
88120
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
89121
| AuthRow
@@ -101,16 +133,34 @@ export function getAuthConfig(): AuthConfig | undefined {
101133
source: "oauth" as const,
102134
};
103135
});
104-
}
136+
if (dbConfig) {
137+
return dbConfig;
138+
}
105139

106-
/** Get the active auth token. Checks env vars first, then falls back to SQLite. */
107-
export function getAuthToken(): string | undefined {
140+
// No stored OAuth — fall back to env token
108141
const envToken = getEnvToken();
109142
if (envToken) {
110-
return envToken.token;
143+
return { token: envToken.token, source: envToken.source };
111144
}
145+
return;
146+
}
112147

113-
return withDbSpan("getAuthToken", () => {
148+
/**
149+
* Get the active auth token.
150+
*
151+
* Default: checks the DB first (stored OAuth wins), then falls back to env vars.
152+
* With `SENTRY_FORCE_ENV_TOKEN=1`: checks env vars first (old behavior).
153+
*/
154+
export function getAuthToken(): string | undefined {
155+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
156+
if (forceEnv) {
157+
const envToken = getEnvToken();
158+
if (envToken) {
159+
return envToken.token;
160+
}
161+
}
162+
163+
const dbToken = withDbSpan("getAuthToken", () => {
114164
const db = getDatabase();
115165
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
116166
| AuthRow
@@ -126,6 +176,16 @@ export function getAuthToken(): string | undefined {
126176

127177
return row.token;
128178
});
179+
if (dbToken) {
180+
return dbToken;
181+
}
182+
183+
// No stored OAuth — fall back to env token
184+
const envToken = getEnvToken();
185+
if (envToken) {
186+
return envToken.token;
187+
}
188+
return;
129189
}
130190

131191
export function setAuthToken(
@@ -179,6 +239,32 @@ export function isAuthenticated(): boolean {
179239
return !!token;
180240
}
181241

242+
/**
243+
* Check if usable OAuth credentials are stored in the database.
244+
*
245+
* Returns true when the `auth` table has either:
246+
* - A non-expired token, or
247+
* - An expired token with a refresh token (will be refreshed on next use)
248+
*
249+
* Used by the login command to decide whether to prompt for re-authentication
250+
* when an env token is present.
251+
*/
252+
export function hasStoredAuthCredentials(): boolean {
253+
const db = getDatabase();
254+
const row = db.query("SELECT * FROM auth WHERE id = 1").get() as
255+
| AuthRow
256+
| undefined;
257+
if (!row?.token) {
258+
return false;
259+
}
260+
// Non-expired token
261+
if (!row.expires_at || Date.now() <= row.expires_at) {
262+
return true;
263+
}
264+
// Expired but has refresh token — will be refreshed on next use
265+
return !!row.refresh_token;
266+
}
267+
182268
export type RefreshTokenOptions = {
183269
/** Bypass threshold check and always refresh */
184270
force?: boolean;
@@ -229,10 +315,13 @@ async function performTokenRefresh(
229315
export async function refreshToken(
230316
options: RefreshTokenOptions = {}
231317
): Promise<RefreshTokenResult> {
232-
// Env var tokens are assumed valid — no refresh, no expiry check
233-
const envToken = getEnvToken();
234-
if (envToken) {
235-
return { token: envToken.token, refreshed: false };
318+
// With SENTRY_FORCE_ENV_TOKEN, env token takes priority (no refresh needed).
319+
const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim();
320+
if (forceEnv) {
321+
const envToken = getEnvToken();
322+
if (envToken) {
323+
return { token: envToken.token, refreshed: false };
324+
}
236325
}
237326

238327
const { force = false } = options;
@@ -244,6 +333,11 @@ export async function refreshToken(
244333
| undefined;
245334

246335
if (!row?.token) {
336+
// No stored token — try env token as fallback
337+
const envToken = getEnvToken();
338+
if (envToken) {
339+
return { token: envToken.token, refreshed: false };
340+
}
247341
throw new AuthError("not_authenticated");
248342
}
249343

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 */

src/lib/sentry-client.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
getConfiguredSentryUrl,
1515
getUserAgent,
1616
} from "./constants.js";
17-
import { getAuthToken, isEnvTokenActive, refreshToken } from "./db/auth.js";
17+
import { getAuthToken, refreshToken } from "./db/auth.js";
1818
import { logger } from "./logger.js";
1919
import { getCachedResponse, storeCachedResponse } from "./response-cache.js";
2020
import { withHttpSpan } from "./telemetry.js";
@@ -108,10 +108,10 @@ async function handleUnauthorized(headers: Headers): Promise<boolean> {
108108
if (headers.get(RETRY_MARKER_HEADER)) {
109109
return false;
110110
}
111-
// Env var tokens can't be refreshed — let the 401 propagate
112-
if (isEnvTokenActive()) {
113-
return false;
114-
}
111+
// refreshToken handles the token selection: it refreshes OAuth when OAuth is
112+
// the effective auth source, or returns the env token without refresh when
113+
// SENTRY_FORCE_ENV_TOKEN is set. If the token can't be refreshed (env token,
114+
// no refresh token), `refreshed` is false and the 401 propagates.
115115
try {
116116
const { token: newToken, refreshed } = await refreshToken({ force: true });
117117
if (refreshed) {

0 commit comments

Comments
 (0)