From a5df3503b75692faa65b3cfe0d1b0f6dbc08cca1 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 24 Apr 2026 17:52:54 +0000 Subject: [PATCH 01/24] security: scope tokens to hosts to prevent credential exfiltration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes three CVE-class vulnerabilities where untrusted inputs could redirect credentialed requests to an attacker-controlled host: 1. URL argument: `sentry issue view https://evil.com/...` previously wrote `env.SENTRY_HOST=https://evil.com` unconditionally, causing every subsequent authenticated fetch and OAuth refresh to send credentials (Bearer + refresh token) to the attacker. 2. `.sentryclirc` injection: a committed `.sentryclirc` file in a cloned repo could set `[defaults] url = https://evil.com`. With a `SENTRY_AUTH_TOKEN` in the environment and `SENTRY_HOST` unset, the env-shim wrote the attacker's URL and preserved the real token, leaking credentials on the next API call. 3. Share URL custom headers: `getSharedIssue(baseUrl, shareId)` calls `applyCustomHeaders` when `isSelfHosted()` is true. A crafted share URL on a non-SaaS host would attach the user's `SENTRY_CUSTOM_HEADERS` (IAP tokens, mTLS headers, etc.) to the outbound request. ## Fix: host-scoped tokens Tokens are bound to a specific Sentry host at issuance time. The fetch layer (and `.sentryclirc` / URL-arg entry points) check each request's destination against the token's recorded host and refuse to attach credentials when they don't match. - `auth` table gains a `host TEXT` column (schema v16). Lazy migration backfills NULL hosts from `getConfiguredSentryUrl()` on first access. - New `src/lib/token-host.ts` normalizes origins, implements SaaS equivalence (`sentry.io` matches `*.sentry.io` for regional silos and org subdomains, exact origin for non-SaaS), and exposes `getActiveTokenHost()`. - New `src/lib/env-token-host.ts` captures the env-token's scope from `SENTRY_HOST`/`SENTRY_URL` at boot, BEFORE `.sentryclirc` shim can write to the env. `.sentryclirc` values are intentionally NOT trusted for scoping — repo-local files and home dirs on CI runners have weaker integrity than the token they would scope. - `sentry auth login --url ` is the only command that establishes trust for a new host. URL arguments and config files are rejected when they don't match the currently-authenticated host. ## Enforcement Primary: entry-point guards in `applySentryUrlContext` (URL args) and `applySentryCliRcEnvShim` (rc files) throw `CliError` on token-host mismatch before any env var is written. Defense in depth: `prepareHeaders` in `sentry-client.ts` and `refreshAccessToken` in `oauth.ts` re-check origin match before attaching credentials. `applyCustomHeaders` now takes a mandatory request-URL argument and skips header attachment when the URL doesn't match the trusted host — closing the share-URL IAP-token leak. ## Threat model (explicit non-goals) - `SENTRY_AUTH_TOKEN` + `SENTRY_HOST=https://evil.com` set in env together is NOT our vulnerability: anyone who can set env vars can already read the token. - `.envrc` / direnv are out of scope (not loaded by this CLI). ## Tests - `test/lib/security/` — four attack regression tests for all three CVEs plus the fetch-layer defense-in-depth. - `test/lib/token-host.{test,property.test}.ts` — SaaS equivalence, subdomain/lookalike attack resistance, origin normalization. - `test/lib/env-token-host.test.ts` — snapshot isolation from `.sentryclirc`-injected env. - `test/lib/db/auth.host.test.ts` — persistence, NULL-host migration. - Updated existing tests in `sentry-url-parser`, `sentryclirc`, `arg-parsing`, `commands/event/view`, `custom-headers`, `commands/auth/login` to reflect the new reject-outright semantics. Plan at .opencode/plans/1777023782662-proud-circuit.md documents the full rationale, trust model, and test matrix. --- .../skills/sentry-cli/references/auth.md | 1 + src/cli.ts | 10 + src/commands/auth/login.ts | 75 +++++++- src/lib/api/issues.ts | 4 +- src/lib/custom-headers.ts | 70 ++++++- src/lib/db/auth.ts | 139 +++++++++++++- src/lib/db/schema.ts | 16 +- src/lib/env-token-host.ts | 100 ++++++++++ src/lib/oauth.ts | 68 +++++-- src/lib/sentry-client.ts | 28 ++- src/lib/sentry-url-parser.ts | 62 +++++- src/lib/sentryclirc.ts | 28 +++ src/lib/token-host.ts | 136 +++++++++++++ test/commands/auth/login.test.ts | 14 +- test/commands/event/view.test.ts | 19 +- test/lib/arg-parsing.test.ts | 36 +++- test/lib/custom-headers.test.ts | 45 ++++- test/lib/db/auth.host.test.ts | 139 ++++++++++++++ test/lib/env-token-host.test.ts | 103 ++++++++++ test/lib/security/fetch-layer-guard.test.ts | 111 +++++++++++ .../lib/security/refresh-token-poison.test.ts | 115 +++++++++++ .../security/sentryclirc-url-poison.test.ts | 149 +++++++++++++++ test/lib/security/url-arg-poison.test.ts | 114 +++++++++++ test/lib/sentry-url-parser.test.ts | 55 +++++- test/lib/sentryclirc.test.ts | 25 ++- test/lib/token-host.property.test.ts | 167 ++++++++++++++++ test/lib/token-host.test.ts | 178 ++++++++++++++++++ 27 files changed, 1944 insertions(+), 63 deletions(-) create mode 100644 src/lib/env-token-host.ts create mode 100644 src/lib/token-host.ts create mode 100644 test/lib/db/auth.host.test.ts create mode 100644 test/lib/env-token-host.test.ts create mode 100644 test/lib/security/fetch-layer-guard.test.ts create mode 100644 test/lib/security/refresh-token-poison.test.ts create mode 100644 test/lib/security/sentryclirc-url-poison.test.ts create mode 100644 test/lib/security/url-arg-poison.test.ts create mode 100644 test/lib/token-host.property.test.ts create mode 100644 test/lib/token-host.test.ts diff --git a/plugins/sentry-cli/skills/sentry-cli/references/auth.md b/plugins/sentry-cli/skills/sentry-cli/references/auth.md index a20e5cadf..976e1418b 100644 --- a/plugins/sentry-cli/skills/sentry-cli/references/auth.md +++ b/plugins/sentry-cli/skills/sentry-cli/references/auth.md @@ -19,6 +19,7 @@ Authenticate with Sentry - `--token - Authenticate using an API token instead of OAuth` - `--timeout - Timeout for OAuth flow in seconds (default: 900) - (default: "900")` - `--force - Re-authenticate without prompting` +- `--url - Sentry instance URL to authenticate against (e.g. https://sentry.example.com). Required for self-hosted; defaults to SaaS (https://sentry.io).` **Examples:** diff --git a/src/cli.ts b/src/cli.ts index 9524a29d9..9bf2446f5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -11,6 +11,7 @@ */ import { getEnv } from "./lib/env.js"; +import { captureEnvTokenHost } from "./lib/env-token-host.js"; import { applySentryCliRcEnvShim } from "./lib/sentryclirc.js"; /** @@ -20,6 +21,15 @@ import { applySentryCliRcEnvShim } from "./lib/sentryclirc.js"; * to `findProjectRoot` and `loadSentryCliRc` are cache hits. */ async function preloadProjectContext(cwd: string): Promise { + // CRITICAL: Snapshot the env-token host BEFORE anything that could mutate + // `env.SENTRY_HOST`/`env.SENTRY_URL` (the .sentryclirc shim or the default + // URL fallback below). This pins the trust scope for env-var tokens like + // SENTRY_AUTH_TOKEN to whatever the user's shell actually set — not to a + // value injected by a repo-local config file. See + // src/lib/env-token-host.ts and the plan at + // .opencode/plans/1777023782662-proud-circuit.md for rationale. + captureEnvTokenHost(); + // Dynamic import keeps the heavy DSN/DB modules out of the completion fast-path const [{ findProjectRoot }, { setCachedProjectRoot }] = await Promise.all([ import("./lib/dsn/project-root.js"), diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index 7f9d6c087..8f481259c 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -6,6 +6,7 @@ import { listOrganizationsUncached, } from "../../lib/api-client.js"; import { buildCommand, numberParser } from "../../lib/command.js"; +import { DEFAULT_SENTRY_URL, normalizeUrl } from "../../lib/constants.js"; import { clearAuth, getActiveEnvVarName, @@ -16,7 +17,8 @@ import { } from "../../lib/db/auth.js"; import { getDbPath } from "../../lib/db/index.js"; import { getUserInfo, setUserInfo } from "../../lib/db/user.js"; -import { AuthError } from "../../lib/errors.js"; +import { getEnv } from "../../lib/env.js"; +import { AuthError, ValidationError } from "../../lib/errors.js"; import { success } from "../../lib/formatters/colors.js"; import { formatDuration, @@ -30,6 +32,7 @@ import { } from "../../lib/interactive-login.js"; import { logger } from "../../lib/logger.js"; import { clearResponseCache } from "../../lib/response-cache.js"; +import { normalizeOrigin } from "../../lib/token-host.js"; const log = logger.withTag("auth.login"); @@ -56,8 +59,50 @@ type LoginFlags = { readonly token?: string; readonly timeout: number; readonly force: boolean; + readonly url?: string; }; +/** + * Normalize and validate the `--url` flag value. + * + * Accepts bare hostnames (`sentry.example.com`) and full URLs + * (`https://sentry.example.com`). Returns the normalized origin + * (`https://sentry.example.com`). Throws {@link ValidationError} on + * unparseable input. + * + * Exported indirectly via the command's `parse` callback. + */ +function parseLoginUrl(raw: string): string { + const prefixed = normalizeUrl(raw); + if (!prefixed) { + throw new ValidationError("--url cannot be empty", "url"); + } + const origin = normalizeOrigin(prefixed); + if (!origin) { + throw new ValidationError(`--url is not a valid URL: ${raw}`, "url"); + } + return origin; +} + +/** + * When `--url` is passed, set `env.SENTRY_HOST` and `env.SENTRY_URL` so the + * device-flow and token-refresh endpoints hit the requested host. Returns + * the normalized URL so callers can record it with {@link setAuthToken}. + */ +function applyLoginUrl(url: string | undefined): string { + if (!url) { + // Preserve existing env / .sentryclirc resolution + const env = getEnv(); + return ( + normalizeOrigin(env.SENTRY_HOST || env.SENTRY_URL) ?? DEFAULT_SENTRY_URL + ); + } + const env = getEnv(); + env.SENTRY_HOST = url; + env.SENTRY_URL = url; + return url; +} + /** * Handle the case where the user is already authenticated. * @@ -118,7 +163,11 @@ export const loginCommand = buildCommand({ fullDescription: "Log in to Sentry using OAuth or an API token.\n\n" + "The OAuth flow uses a device code - you'll be given a code to enter at a URL.\n" + - "Alternatively, use --token to authenticate with an existing API token.", + "Alternatively, use --token to authenticate with an existing API token.\n\n" + + "For self-hosted Sentry, pass --url to authenticate against that\n" + + "instance. This is the ONLY way to trust a new Sentry host — URL\n" + + "arguments and config files are refused when they don't match the\n" + + "currently-authenticated host.", }, parameters: { flags: { @@ -140,10 +189,24 @@ export const loginCommand = buildCommand({ brief: "Re-authenticate without prompting", default: false, }, + url: { + kind: "parsed", + parse: parseLoginUrl, + brief: + "Sentry instance URL to authenticate against (e.g. https://sentry.example.com). " + + "Required for self-hosted; defaults to SaaS (https://sentry.io).", + optional: true, + }, }, }, output: { human: formatLoginResult }, async *func(this: SentryContext, flags: LoginFlags) { + // Apply `--url` first so all downstream auth code (device flow, token + // refresh, token validation) targets the requested instance. The + // effective host is also passed to `setAuthToken` so the stored token + // is scoped correctly. + const effectiveHost = applyLoginUrl(flags.url); + // Check if already authenticated and handle re-authentication if (isAuthenticated()) { const shouldProceed = await handleExistingAuth(flags.force); @@ -161,8 +224,10 @@ export const loginCommand = buildCommand({ // Token-based authentication if (flags.token) { - // Save token first, then validate by fetching user regions - await setAuthToken(flags.token); + // Save token first (with host scope), then validate by fetching user regions + await setAuthToken(flags.token, undefined, undefined, { + host: effectiveHost, + }); // Validate token by fetching user regions try { @@ -201,7 +266,7 @@ export const loginCommand = buildCommand({ return yield new CommandOutput(result); } - // OAuth device flow + // OAuth device flow (host scope recorded via completeOAuthFlow → setAuthToken) const result = await runInteractiveLogin({ timeout: flags.timeout * 1000, }); diff --git a/src/lib/api/issues.ts b/src/lib/api/issues.ts index 950139ed2..679ee4370 100644 --- a/src/lib/api/issues.ts +++ b/src/lib/api/issues.ts @@ -669,7 +669,9 @@ export async function getSharedIssue( ): Promise<{ groupID: string }> { const url = `${baseUrl}/api/0/shared/issues/${encodeURIComponent(shareId)}/`; const headers = new Headers({ "Content-Type": "application/json" }); - applyCustomHeaders(headers); + // URL-scoped: headers only attach when `url`'s origin matches the trusted + // host, so IAP tokens etc. can't leak to an attacker-controlled share URL. + applyCustomHeaders(headers, url); const response = await fetch(url, { headers }); if (!response.ok) { diff --git a/src/lib/custom-headers.ts b/src/lib/custom-headers.ts index 48c2f414d..c814faf4b 100644 --- a/src/lib/custom-headers.ts +++ b/src/lib/custom-headers.ts @@ -21,12 +21,13 @@ * ``` */ -import { getConfiguredSentryUrl } from "./constants.js"; +import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "./constants.js"; import { getDefaultHeaders } from "./db/defaults.js"; import { getEnv } from "./env.js"; import { ConfigError } from "./errors.js"; import { logger } from "./logger.js"; import { isSentrySaasUrl } from "./sentry-urls.js"; +import { getActiveTokenHost, isHostTrusted } from "./token-host.js"; const log = logger.withTag("custom-headers"); @@ -66,6 +67,12 @@ let cachedRawSource: string | undefined; /** Whether the SaaS warning has already been logged this session. */ let saasWarningLogged = false; +/** + * Whether the untrusted-destination warning has already been logged this + * session. Prevents log spam when a command fans out many requests. + */ +let untrustedDestinationWarningLogged = false; + /** * Parse a raw custom headers string into validated name/value pairs. * @@ -195,15 +202,67 @@ export function getCustomHeaders(): readonly [string, string][] { } /** - * Apply custom headers to a `Headers` instance. + * Resolve the origin we treat as "ours" for custom-header scoping. + * + * Precedence: + * 1. Active token's scoped host (from stored OAuth or env-token snapshot) + * 2. Currently-configured `SENTRY_HOST`/`SENTRY_URL` + * 3. SaaS default (`https://sentry.io`) + * + * Custom headers are unauthenticated from the CLI's perspective — they're + * attached regardless of whether we have a bearer token — but they contain + * trust-bearing material (IAP tokens, mTLS headers, etc.) that must not leak + * to a non-trusted host. Without an active token, we still have to pick a + * trust anchor; the configured host is the next-best signal. + */ +function getTrustedHostForCustomHeaders(): string { + return getActiveTokenHost() ?? getConfiguredSentryUrl() ?? DEFAULT_SENTRY_URL; +} + +/** + * Apply custom headers to a `Headers` instance, scoped to a request URL. * - * Reads from the env var or SQLite defaults, validates, and sets each header. - * No-op when no custom headers are configured or when targeting SaaS. + * Reads from the env var or SQLite defaults, validates, and sets each header, + * but ONLY when `requestUrl`'s origin matches the trusted host (active token + * scope or configured `SENTRY_HOST`, with SaaS equivalence). + * + * This is the third-report CVE fix: `getSharedIssue` fetches from a `baseUrl` + * extracted from user-provided share URLs, and previously would attach + * `SENTRY_CUSTOM_HEADERS` (e.g. IAP tokens) to any host matching the + * non-SaaS / self-hosted heuristic. The URL-arg entry-point guard + * (`applySentryUrlContext`) is the primary fix, but scoping headers at their + * attachment point defends against any code path that bypasses that guard. + * + * No-op when no custom headers are configured, when targeting SaaS, or when + * the request URL doesn't match the trusted host. * * @param headers - The `Headers` instance to modify in-place + * @param requestUrl - The destination URL (string, URL, or Request) whose + * origin will be compared to the trusted host. When this parameter is + * omitted (legacy callers), the headers are NOT applied — we fail closed + * to avoid unintentional leaks from refactors that forgot to pass the URL. */ -export function applyCustomHeaders(headers: Headers): void { +export function applyCustomHeaders( + headers: Headers, + requestUrl: string | URL | Request +): void { const customHeaders = getCustomHeaders(); + if (customHeaders.length === 0) { + return; + } + + const trustedHost = getTrustedHostForCustomHeaders(); + if (!isHostTrusted(requestUrl, trustedHost)) { + if (!untrustedDestinationWarningLogged) { + untrustedDestinationWarningLogged = true; + log.warn( + `Skipping custom headers for request to untrusted host (expected ${trustedHost}). ` + + "If this is legitimate, configure SENTRY_HOST or log in against the intended instance." + ); + } + return; + } + for (const [name, value] of customHeaders) { headers.set(name, value); } @@ -217,4 +276,5 @@ export function _resetCustomHeadersCache(): void { cachedHeaders = undefined; cachedRawSource = undefined; saasWarningLogged = false; + untrustedDestinationWarningLogged = false; } diff --git a/src/lib/db/auth.ts b/src/lib/db/auth.ts index 0e8e9260e..334c5aa0b 100644 --- a/src/lib/db/auth.ts +++ b/src/lib/db/auth.ts @@ -3,8 +3,11 @@ */ import { createHash } from "node:crypto"; +import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "../constants.js"; import { getEnv } from "../env.js"; +import { logger } from "../logger.js"; import { withDbSpan } from "../telemetry.js"; +import { normalizeOrigin } from "../token-host.js"; import { getDatabase } from "./index.js"; import { clearAllIssueOrgCache } from "./issue-org-cache.js"; import { runUpsert } from "./utils.js"; @@ -21,8 +24,48 @@ type AuthRow = { expires_at: number | null; issued_at: number | null; updated_at: number; + /** + * Origin URL the token was issued against (e.g., `https://sentry.io` or + * `https://sentry.example.com`). NULL for rows written before schema v16; + * lazily migrated by `migrateNullHostIfPresent` on first access. + */ + host: string | null; }; +const log = logger.withTag("auth"); + +/** + * Lazy migration for rows created before schema v16 (NULL `host`). + * + * Writes the currently-configured host (`SENTRY_HOST`/`SENTRY_URL`, or SaaS + * default) into the row. One-time per row, logged at `info` so users see it + * in their first command after upgrade. + * + * Users who had `SENTRY_HOST` correctly configured at upgrade time are + * migrated cleanly. Users with the wrong host configured can recover with + * `sentry auth logout && sentry auth login` against the intended instance. + * + * Returns the migrated host string (never NULL on return). + */ +function migrateNullHost(row: AuthRow): string { + const configured = getConfiguredSentryUrl(); + const migratedHost = normalizeOrigin(configured ?? DEFAULT_SENTRY_URL); + const host = migratedHost ?? DEFAULT_SENTRY_URL; + try { + withDbSpan("migrateAuthHost", () => { + const db = getDatabase(); + db.query("UPDATE auth SET host = ? WHERE id = 1").run(host); + }); + log.info(`Migrated stored credentials to host-scoped model: ${host}`); + } catch { + // Non-fatal: if the migration write fails, callers still get a + // well-formed host from this function. The migration will retry + // on the next access. + } + row.host = host; + return host; +} + /** Prefix for environment variable auth sources in {@link AuthSource} */ export const ENV_SOURCE_PREFIX = "env:"; @@ -153,6 +196,68 @@ export function getAuthConfig(): AuthConfig | undefined { return; } +/** + * Read the host the stored OAuth token is scoped to. + * + * Lazy-migrates NULL hosts (rows from before schema v16) to the currently- + * configured host on first access. Returns `undefined` when no stored token + * exists — callers should fall through to the env-token host snapshot. + * + * This function is intentionally tolerant of "no DB" errors (tests that + * bypass DB init). On DB failure, returns `undefined` and trust-scoping + * falls back to the caller's default behavior. + */ +export function getStoredAuthHost(): string | undefined { + try { + return withDbSpan("getStoredAuthHost", () => { + const db = getDatabase(); + const row = db.query("SELECT * FROM auth WHERE id = 1").get() as + | AuthRow + | undefined; + if (!row?.token) { + return; + } + if (row.host) { + return row.host; + } + // Lazy migration for pre-v16 rows + return migrateNullHost(row); + }); + } catch { + return; + } +} + +/** + * Check whether a usable stored token exists in the auth row. + * + * Mirrors the "usable" criteria in `getAuthConfig`: a token is usable if it + * has a bearer value AND (no expiry, OR not expired, OR expired-with-refresh). + * + * Used by {@link getActiveTokenHost} to decide whether to prefer stored + * OAuth's host over the env-token snapshot. + */ +export function hasUsableStoredToken(): boolean { + try { + return withDbSpan("hasUsableStoredToken", () => { + const db = getDatabase(); + const row = db.query("SELECT * FROM auth WHERE id = 1").get() as + | AuthRow + | undefined; + if (!row?.token) { + return false; + } + // Match getAuthConfig's filter: expired-no-refresh rows are unusable + if (row.expires_at && Date.now() > row.expires_at && !row.refresh_token) { + return false; + } + return true; + }); + } catch { + return false; + } +} + /** Memoized token. Wrapper distinguishes "not cached" from "cached as undefined". */ let cachedAuthToken: { value: string | undefined } | undefined; @@ -233,10 +338,23 @@ export function resetAuthRowCache(): void { cachedAuthRow = undefined; } +/** + * Options for persisting a token. + * + * @property host - Origin URL the token was issued against. When omitted on + * an update (e.g., access-token refresh), the existing row's host is + * preserved. When omitted on a fresh write, defaults to the + * currently-configured host (`SENTRY_HOST`/`SENTRY_URL`) or `DEFAULT_SENTRY_URL`. + */ +export type SetAuthTokenOptions = { + host?: string; +}; + export function setAuthToken( token: string, expiresIn?: number, - newRefreshToken?: string + newRefreshToken?: string, + options?: SetAuthTokenOptions ): void { withDbSpan("setAuthToken", () => { const db = getDatabase(); @@ -244,6 +362,24 @@ export function setAuthToken( const expiresAt = expiresIn ? now + expiresIn * 1000 : null; const issuedAt = expiresIn ? now : null; + // Host resolution precedence: + // 1. Explicit `options.host` (login command, tests) + // 2. Existing row's `host` (refresh flow preserves the original scope) + // 3. Currently-configured host (getConfiguredSentryUrl) + // 4. SaaS default + // Always normalized to scheme+host[+port] via normalizeOrigin. + const existingHost = ( + db.query("SELECT host FROM auth WHERE id = 1").get() as + | { host: string | null } + | undefined + )?.host; + const rawHost = + options?.host ?? + existingHost ?? + getConfiguredSentryUrl() ?? + DEFAULT_SENTRY_URL; + const host = normalizeOrigin(rawHost) ?? DEFAULT_SENTRY_URL; + runUpsert( db, "auth", @@ -254,6 +390,7 @@ export function setAuthToken( expires_at: expiresAt, issued_at: issuedAt, updated_at: now, + host, }, ["id"] ); diff --git a/src/lib/db/schema.ts b/src/lib/db/schema.ts index 353fb120b..e4ee65c97 100644 --- a/src/lib/db/schema.ts +++ b/src/lib/db/schema.ts @@ -16,7 +16,7 @@ import { getEnv } from "../env.js"; import { stringifyUnknown } from "../errors.js"; import { logger } from "../logger.js"; -export const CURRENT_SCHEMA_VERSION = 15; +export const CURRENT_SCHEMA_VERSION = 16; /** Environment variable to disable auto-repair */ const NO_AUTO_REPAIR_ENV = "SENTRY_CLI_NO_AUTO_REPAIR"; @@ -66,6 +66,11 @@ export const TABLE_SCHEMAS: Record = { notNull: true, default: "(unixepoch() * 1000)", }, + // Origin URL (scheme://host[:port]) this token was issued against. + // Enforced at the fetch layer: credentials are only attached to requests + // whose origin matches this host (with SaaS equivalence). Nullable for + // rows created before schema v16; migrated lazily in getAuthConfig. + host: { type: "TEXT", addedInVersion: 16 }, }, }, @@ -839,6 +844,15 @@ export function runMigrations(db: Database): void { db.exec(EXPECTED_TABLES.issue_org_cache as string); } + // Migration 15 -> 16: Add host column to auth table for host-scoped tokens. + // The column is NULL for existing rows; getAuthConfig lazily backfills it + // with the currently-configured host on first access after upgrade, so + // users who already have SENTRY_HOST/SENTRY_URL set at upgrade time are + // migrated cleanly to the host-scoped model. + if (currentVersion < 16) { + addColumnIfMissing(db, "auth", "host", "TEXT"); + } + if (currentVersion < CURRENT_SCHEMA_VERSION) { db.query("UPDATE schema_version SET version = ?").run( CURRENT_SCHEMA_VERSION diff --git a/src/lib/env-token-host.ts b/src/lib/env-token-host.ts new file mode 100644 index 000000000..0e1142d17 --- /dev/null +++ b/src/lib/env-token-host.ts @@ -0,0 +1,100 @@ +/** + * Env-Token Host Snapshot + * + * Captures the host an env-var auth token (`SENTRY_AUTH_TOKEN` / + * `SENTRY_TOKEN`) is scoped to, **before** any post-boot code path can mutate + * the env (specifically, before `applySentryCliRcEnvShim` writes + * `env.SENTRY_URL` from a `.sentryclirc` file). + * + * Trust model rationale: the only signal we can trust for env-token scoping + * is other env vars set by the same shell session that launched us. Anyone + * with shell access to set `SENTRY_HOST` already has access to read + * `SENTRY_AUTH_TOKEN`, so env-level attacks are symmetric (and out of scope + * per the plan's threat model). `.sentryclirc` files, by contrast, are + * writable by repo authors or CI environments with weaker integrity than the + * token — they are NOT consulted for scoping. + * + * Default: `DEFAULT_SENTRY_URL` (SaaS) — matches where personal access tokens + * from `sentry.io` settings can be used by default. + * + * Idempotent: calling `captureEnvTokenHost()` twice is a no-op. Exposed as + * `reset...ForTesting()` for unit tests that need to re-capture after + * mutating env vars. + * + * Boot ordering (see `src/cli.ts::preloadProjectContext`): + * 1. captureEnvTokenHost() ← this module, synchronous, reads env only + * 2. findProjectRoot ← populates .sentryclirc cache (no env writes) + * 3. applySentryCliRcEnvShim ← may write env.SENTRY_URL + * 4. getDefaultUrl() fallback ← may write env.SENTRY_URL + */ + +import { DEFAULT_SENTRY_URL, normalizeUrl } from "./constants.js"; +import { getEnv } from "./env.js"; +import { normalizeOrigin } from "./token-host.js"; + +/** + * Pinned host, or `undefined` if not yet captured. + * + * Using a wrapper pattern so we can distinguish "not captured yet" (the value + * is `undefined`) from "captured as default" (the value is a string). + */ +let pinnedHost: string | undefined; + +/** + * Read `env.SENTRY_HOST` then `env.SENTRY_URL` directly (NOT via + * `getConfiguredSentryUrl`, which also consults `.sentryclirc`-sourced values + * we haven't yet decided to trust). Bare hostnames (e.g. + * `SENTRY_HOST=sentry.example.com`) are prefixed with `https://` via + * `normalizeUrl`, matching the rest of the code base. + */ +function readEnvHost(): string | undefined { + const env = getEnv(); + const raw = env.SENTRY_HOST?.trim() || env.SENTRY_URL?.trim(); + return normalizeUrl(raw); +} + +/** + * Snapshot the env-token's scoping host from `SENTRY_HOST`/`SENTRY_URL`. + * + * Must be called before any code path that writes to `env.SENTRY_HOST` or + * `env.SENTRY_URL` — notably `applySentryCliRcEnvShim` and the + * `getDefaultUrl` fallback in `preloadProjectContext`. Idempotent: second + * and subsequent calls are no-ops. + * + * Defaults to `DEFAULT_SENTRY_URL` (SaaS) when neither env var is set. + */ +export function captureEnvTokenHost(): void { + if (pinnedHost !== undefined) { + return; + } + const fromEnv = readEnvHost(); + const host = fromEnv ? normalizeOrigin(fromEnv) : undefined; + pinnedHost = host ?? DEFAULT_SENTRY_URL; +} + +/** + * Return the pinned env-token host, auto-capturing on first call. + * + * Auto-capture is a safety net — the standard boot path in `src/cli.ts` calls + * `captureEnvTokenHost()` explicitly to snapshot before `.sentryclirc` shim + * runs. For code paths that bypass that boot (e.g. direct library usage in + * tests), this auto-capture still produces the correct result because it + * reads env-only. + */ +export function getEnvTokenHost(): string { + if (pinnedHost === undefined) { + captureEnvTokenHost(); + } + // captureEnvTokenHost always assigns a non-undefined value + return pinnedHost as string; +} + +/** + * Reset the pinned host. Tests only — call between cases that mutate + * `SENTRY_HOST`/`SENTRY_URL` to re-capture on the next access. + * + * @internal + */ +export function resetEnvTokenHostForTesting(): void { + pinnedHost = undefined; +} diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index bcf79cc0f..ea063606f 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -12,11 +12,22 @@ import { TokenResponseSchema, } from "../types/index.js"; import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "./constants.js"; -import { getCustomHeaders } from "./custom-headers.js"; +import { applyCustomHeaders } from "./custom-headers.js"; import { setAuthToken } from "./db/auth.js"; import { getEnv } from "./env.js"; -import { ApiError, AuthError, ConfigError, DeviceFlowError } from "./errors.js"; +import { + ApiError, + AuthError, + CliError, + ConfigError, + DeviceFlowError, +} from "./errors.js"; import { withHttpSpan } from "./telemetry.js"; +import { + getActiveTokenHost, + isHostTrusted, + normalizeOrigin, +} from "./token-host.js"; /** * Get the Sentry instance URL for OAuth endpoints. @@ -86,16 +97,13 @@ async function fetchWithConnectionError( url: string, init: RequestInit ): Promise { - // Inject custom headers for self-hosted proxies (IAP, mTLS, etc.) - let effectiveInit = init; - const customHeaders = getCustomHeaders(); - if (customHeaders.length > 0) { - const merged = new Headers(init.headers); - for (const [name, value] of customHeaders) { - merged.set(name, value); - } - effectiveInit = { ...init, headers: merged }; - } + // Inject custom headers for self-hosted proxies (IAP, mTLS, etc.). + // `applyCustomHeaders` is URL-scoped — headers only attach when `url` + // matches the trusted host, so IAP tokens can't leak to an attacker's + // host even on unauthenticated OAuth endpoints. + const merged = new Headers(init.headers); + applyCustomHeaders(merged, url); + const effectiveInit: RequestInit = { ...init, headers: merged }; try { return await fetch(url, effectiveInit); @@ -117,6 +125,28 @@ async function fetchWithConnectionError( } } +/** + * Guard the OAuth token refresh path: refuse to POST a refresh token to a + * host that doesn't match the active token's scope. + * + * Defense-in-depth — the URL-arg / rc-shim entry points already reject + * mismatched hosts before we get here. This is the belt-and-suspenders layer + * that catches any future code path that writes SENTRY_HOST/SENTRY_URL + * without going through the entry-point guards. + */ +function assertRefreshHostTrusted(): void { + const tokenHost = getActiveTokenHost(); + const refreshUrl = getSentryUrl(); + const refreshOrigin = normalizeOrigin(refreshUrl); + if (tokenHost && !isHostTrusted(refreshOrigin, tokenHost)) { + throw new CliError( + `Refusing to send OAuth refresh token to ${refreshOrigin ?? ""}: active token is scoped to ${tokenHost}.\n` + + "Run 'sentry auth login --url ' against the intended instance, " + + "or unset SENTRY_HOST to use credentials for a different host." + ); + } +} + /** Request a device code from Sentry's device authorization endpoint */ function requestDeviceCode() { const clientId = getClientId(); @@ -320,6 +350,11 @@ export async function performDeviceFlow( /** * Complete the OAuth flow by storing the token in the database. * + * The token is scoped to the host the OAuth flow targeted (read via + * {@link getSentryUrl}, which reflects `SENTRY_HOST`/`SENTRY_URL` at this + * moment — set by `auth login --url` or user env). Scoping the token at + * issuance time is what makes the fetch-layer trust check work. + * * @param tokenResponse - The token response from performDeviceFlow */ export async function completeOAuthFlow( @@ -328,7 +363,8 @@ export async function completeOAuthFlow( await setAuthToken( tokenResponse.access_token, tokenResponse.expires_in, - tokenResponse.refresh_token + tokenResponse.refresh_token, + { host: getSentryUrl() } ); } @@ -340,7 +376,7 @@ export async function completeOAuthFlow( * @param token - The API token to store */ export async function setApiToken(token: string): Promise { - await setAuthToken(token); + await setAuthToken(token, undefined, undefined, { host: getSentryUrl() }); } /** Refresh an access token using a refresh token. */ @@ -355,6 +391,10 @@ export function refreshAccessToken( ); } + // Defense-in-depth host scoping: refuse to send the refresh token to a + // host the active token isn't scoped for. See assertRefreshHostTrusted. + assertRefreshHostTrusted(); + return withHttpSpan("POST", "/oauth/token/", async () => { const response = await fetchWithConnectionError( `${getSentryUrl()}/oauth/token/`, diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index 32cb03f68..797df50fb 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -18,7 +18,7 @@ import { } from "./constants.js"; import { applyCustomHeaders } from "./custom-headers.js"; import { getAuthToken, refreshToken } from "./db/auth.js"; -import { TimeoutError } from "./errors.js"; +import { CliError, TimeoutError } from "./errors.js"; import { logger } from "./logger.js"; import { getCachedResponse, @@ -26,6 +26,11 @@ import { storeCachedResponse, } from "./response-cache.js"; import { withTracingSpan } from "./telemetry.js"; +import { + getActiveTokenHost, + isHostTrusted, + normalizeOrigin, +} from "./token-host.js"; const log = logger.withTag("http"); @@ -100,6 +105,21 @@ function prepareHeaders( init: RequestInit | undefined, token: string ): Headers { + // Host-scoping guard (defense in depth). Primary rejection happens at the + // URL-arg / rc-shim entry points, but any future code path that writes + // SENTRY_HOST/SENTRY_URL without going through those guards is still caught + // here: we refuse to attach `Authorization` to a request whose origin + // doesn't match the active token's scoped host. See token-host.ts. + const tokenHost = getActiveTokenHost(); + const requestOrigin = normalizeOrigin(input); + if (tokenHost && !isHostTrusted(requestOrigin, tokenHost)) { + throw new CliError( + `Refusing to send credentials to ${requestOrigin ?? ""}: active token is scoped to ${tokenHost}.\n` + + "Run 'sentry auth login --url ' against the intended instance, " + + "or unset SENTRY_AUTH_TOKEN/SENTRY_HOST to use credentials for a different host." + ); + } + // When the SDK calls fetch(request) with no init, read headers from the Request // object to preserve Content-Type. On Node.js, fetch(request, {headers}) replaces // the Request's headers entirely per spec, so we must carry them forward explicitly. @@ -123,8 +143,10 @@ function prepareHeaders( headers.set("baggage", traceData.baggage); } - // Inject user-configured custom headers for self-hosted proxies (IAP, mTLS, etc.) - applyCustomHeaders(headers); + // Inject user-configured custom headers for self-hosted proxies (IAP, mTLS, + // etc.). Scoped to the request URL so IAP tokens don't leak to an attacker's + // host even on unauthenticated endpoints (see CVE in plan). + applyCustomHeaders(headers, input); return headers; } diff --git a/src/lib/sentry-url-parser.ts b/src/lib/sentry-url-parser.ts index 65414e951..6eece5a35 100644 --- a/src/lib/sentry-url-parser.ts +++ b/src/lib/sentry-url-parser.ts @@ -10,7 +10,9 @@ import { DEFAULT_SENTRY_HOST } from "./constants.js"; import { getEnv } from "./env.js"; +import { CliError } from "./errors.js"; import { isSentrySaasUrl } from "./sentry-urls.js"; +import { getActiveTokenHost, isHostTrusted } from "./token-host.js"; /** * Components extracted from a Sentry web URL. @@ -212,13 +214,26 @@ export function parseSentryUrl(input: string): ParsedSentryUrl | null { /** * Configure `SENTRY_URL` for self-hosted instances detected from a parsed URL. * - * Sets the env var when the URL is NOT a Sentry SaaS domain (*.sentry.io), - * since SaaS uses multi-region routing instead. + * Host-scoping trust check (see plan + * `.opencode/plans/1777023782662-proud-circuit.md`): * - * The parsed URL always takes precedence over any existing `SENTRY_URL` value - * because an explicit URL argument is the strongest signal of user intent. + * - SaaS URLs (`*.sentry.io`) always proceed. Credentials scoped to SaaS are + * valid for any regional silo or org subdomain, so there's nothing to leak. + * - Non-SaaS URLs require the trust check: if any active token exists, its + * scoped host MUST match `baseUrl` (with SaaS equivalence for tokens that + * happen to be SaaS, though non-SaaS baseUrl never matches SaaS tokens). + * Mismatch or no-token-at-all → throw `CliError`. Only `sentry auth login + * --url ` establishes trust for a new host. + * + * This closes the reported CVE class where a URL argument like + * `sentry issue view https://evil.com/...` would unconditionally set + * `env.SENTRY_HOST`, causing the subsequent authenticated request to leak + * the bearer (and potentially refresh) token to the attacker. * * @param baseUrl - The scheme + host extracted from the URL (e.g., "https://sentry.example.com") + * @throws {CliError} When `baseUrl` is non-SaaS and doesn't match the active + * token's scoped host, or when no token is stored/configured and the URL + * is non-SaaS. */ export function applySentryUrlContext(baseUrl: string): void { const env = getEnv(); @@ -231,6 +246,45 @@ export function applySentryUrlContext(baseUrl: string): void { delete env.SENTRY_URL; return; } + + // Non-SaaS URL — enforce host-scoping. + const tokenHost = getActiveTokenHost(); + if (!(tokenHost && isHostTrusted(baseUrl, tokenHost))) { + throw new CliError( + buildUrlMismatchMessage("URL argument", baseUrl, tokenHost) + ); + } + env.SENTRY_HOST = baseUrl; env.SENTRY_URL = baseUrl; } + +/** + * Build the standard "host mismatch" error message used by + * {@link applySentryUrlContext} and (re-exported for) the `.sentryclirc` shim. + * + * @param source - Short human-readable description of where the URL came from + * (e.g., `"URL argument"`, `"Config at /path/to/.sentryclirc"`). + * @param baseUrl - The URL that doesn't match the active token. + * @param tokenHost - The active token's scoped host, or `undefined` when no + * token exists (the "no token" case needs a different action hint). + */ +export function buildUrlMismatchMessage( + source: string, + baseUrl: string, + tokenHost: string | undefined +): string { + if (tokenHost === undefined) { + return ( + `${source}: ${baseUrl}\n` + + "Refusing to route requests to this host because no Sentry credentials are configured for it.\n" + + `To use this host, run: sentry auth login --url ${baseUrl}` + ); + } + return ( + `${source}: ${baseUrl}\n` + + `Refusing to route requests here because it doesn't match the host your Sentry credentials are for (${tokenHost}).\n` + + `To use this host, run: sentry auth login --url ${baseUrl}\n` + + "To keep using your current credentials, remove this URL override." + ); +} diff --git a/src/lib/sentryclirc.ts b/src/lib/sentryclirc.ts index 2d9f37046..2850d3393 100644 --- a/src/lib/sentryclirc.ts +++ b/src/lib/sentryclirc.ts @@ -21,8 +21,12 @@ import { homedir } from "node:os"; import { join } from "node:path"; import { getConfigDir } from "./db/index.js"; import { getEnv } from "./env.js"; +import { CliError } from "./errors.js"; import { parseIni } from "./ini.js"; import { logger } from "./logger.js"; +import { buildUrlMismatchMessage } from "./sentry-url-parser.js"; +import { isSentrySaasUrl } from "./sentry-urls.js"; +import { getActiveTokenHost, isHostTrusted } from "./token-host.js"; import { walkUpFrom } from "./walk-up.js"; const log = logger.withTag("sentryclirc"); @@ -342,6 +346,30 @@ export async function applySentryCliRcEnvShim(cwd: string): Promise { } if (config.url && !env.SENTRY_HOST?.trim() && !env.SENTRY_URL?.trim()) { + // Host-scoping trust check (see plan + // .opencode/plans/1777023782662-proud-circuit.md). `.sentryclirc` files + // — both repo-local and global `~/.sentryclirc` — are untrusted as + // trust-establishment sources (repos ship them; CI writes home dirs). + // A URL in any rc file is honored only when it matches the active + // token's scoped host. Otherwise we refuse: silently routing requests + // to a host the credentials don't match either leaks credentials (if + // the fetch layer didn't also guard them) or produces confusing 401s + // at best. Users who need to trust a new host must run + // `sentry auth login --url ` explicitly. + // + // SaaS URLs in rc files are always honored — nothing to leak. + if (!isSentrySaasUrl(config.url)) { + const tokenHost = getActiveTokenHost(); + if (!(tokenHost && isHostTrusted(config.url, tokenHost))) { + const source = config.sources.url + ? `Config at ${config.sources.url}` + : `${CONFIG_FILENAME} [defaults] url`; + throw new CliError( + buildUrlMismatchMessage(source, config.url, tokenHost) + ); + } + } + log.debug( `Setting SENTRY_URL from ${CONFIG_FILENAME} (${config.sources.url})` ); diff --git a/src/lib/token-host.ts b/src/lib/token-host.ts new file mode 100644 index 000000000..0cc8b9696 --- /dev/null +++ b/src/lib/token-host.ts @@ -0,0 +1,136 @@ +/** + * Host-Scoped Token Trust Model + * + * Tokens (env or stored OAuth) are bound to a specific Sentry host. The fetch + * layer (and the `.sentryclirc` / URL-arg entry points) check the destination + * of every authenticated request against the token's recorded host and refuse + * to attach credentials when they don't match. + * + * This prevents the CVE class where untrusted inputs (URL arguments, committed + * `.sentryclirc` files) can redirect credentialed requests to an attacker's + * host. Routing decisions are decoupled from credential decisions: credentials + * simply aren't attached when destination ≠ token host, so an attacker's host + * gets an unauthenticated request and nothing leaks. + * + * Host equivalence: + * - Exact origin match (normalized scheme + host + explicit port). + * - SaaS equivalence class: a token scoped to `https://sentry.io` is valid for + * any `*.sentry.io` subdomain (regional silos, org subdomains). This is the + * only equivalence class — non-SaaS hosts match exactly. + * + * See `.opencode/plans/1777023782662-proud-circuit.md` for the full rationale. + */ + +import { DEFAULT_SENTRY_URL } from "./constants.js"; +import { + getRawEnvToken, + getStoredAuthHost, + hasUsableStoredToken, +} from "./db/auth.js"; +import { getEnv } from "./env.js"; +import { getEnvTokenHost } from "./env-token-host.js"; +import { isSentrySaasUrl } from "./sentry-urls.js"; + +/** + * Normalize a URL (or fetch input) to its canonical origin form. + * + * Returns `scheme://host[:port]` with: + * - lowercase scheme and host + * - explicit port only when non-default + * - no trailing slash or path/query/fragment + * + * Returns `undefined` for strings that don't parse as URLs. + */ +export function normalizeOrigin( + input: string | URL | Request | undefined | null +): string | undefined { + if (input === null || input === undefined) { + return; + } + let raw: string; + if (typeof input === "string") { + raw = input; + } else if (input instanceof URL) { + raw = input.href; + } else { + // Request object + raw = input.url; + } + try { + return new URL(raw).origin; + } catch { + return; + } +} + +/** + * Check whether `candidate` matches `trusted` under the host-scoping trust + * model. + * + * - SaaS tokens (scoped to `https://sentry.io`) match any `*.sentry.io` + * candidate (e.g., `us.sentry.io`, `myorg.sentry.io`). + * - Non-SaaS tokens must match exact origin (scheme + host + port). No + * subdomain suffix matching — a `sentry.acme.com` token does NOT match + * `sentry.acme.evil.com`. + * + * Returns `false` when either argument fails to parse. The caller should treat + * an unparseable candidate as an untrusted destination. + */ +export function isHostTrusted( + candidate: string | URL | Request | undefined | null, + trusted: string | undefined | null +): boolean { + if (!trusted) { + return false; + } + const candidateOrigin = normalizeOrigin(candidate); + const trustedOrigin = normalizeOrigin(trusted); + if (!(candidateOrigin && trustedOrigin)) { + return false; + } + if (candidateOrigin === trustedOrigin) { + return true; + } + // SaaS equivalence: if the trusted host is SaaS and the candidate is also + // SaaS, they share the same trust class. Non-SaaS must match exactly. + if (isSentrySaasUrl(trustedOrigin) && isSentrySaasUrl(candidateOrigin)) { + return true; + } + return false; +} + +/** + * Resolve the origin of the currently active Sentry token, if any. + * + * Precedence mirrors {@link getAuthConfig}: + * 1. `SENTRY_FORCE_ENV_TOKEN` + env token present → env-token host snapshot + * 2. Stored OAuth row (with lazy NULL-host migration) → row host + * 3. Env token present → env-token host snapshot + * 4. No token → `undefined` + * + * Returns `undefined` when no token is active. Host values are always + * normalized origins; the DB and snapshot helpers guarantee this. + * + * Implementation is isolated from `db/auth.ts` to keep that module focused on + * storage. This indirection also avoids circular imports between + * `sentry-client` (fetch layer) and auth. + */ +export function getActiveTokenHost(): string | undefined { + // 1. Forced env-token precedence + const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim(); + if (forceEnv && getRawEnvToken()) { + return getEnvTokenHost(); + } + + // 2. Stored OAuth (with lazy migration) takes precedence when present + if (hasUsableStoredToken()) { + return getStoredAuthHost() ?? DEFAULT_SENTRY_URL; + } + + // 3. Env token as fallback + if (getRawEnvToken()) { + return getEnvTokenHost(); + } + + return; +} diff --git a/test/commands/auth/login.test.ts b/test/commands/auth/login.test.ts index ae9208c66..d3843dfb6 100644 --- a/test/commands/auth/login.test.ts +++ b/test/commands/auth/login.test.ts @@ -240,7 +240,13 @@ describe("loginCommand.func --token path", () => { timeout: 900, }); - expect(setAuthTokenSpy).toHaveBeenCalledWith("my-token"); + // Token stored with host scope (host resolved from SENTRY_HOST/SENTRY_URL + // or default SaaS — see setAuthToken in db/auth.ts). + expect(setAuthTokenSpy).toHaveBeenCalled(); + expect(setAuthTokenSpy.mock.calls[0]?.[0]).toBe("my-token"); + expect(setAuthTokenSpy.mock.calls[0]?.[3]).toMatchObject({ + host: expect.any(String), + }); expect(getCurrentUserSpy).toHaveBeenCalled(); expect(setUserInfoSpy).toHaveBeenCalledWith({ userId: "42", @@ -385,7 +391,11 @@ describe("loginCommand.func --token path", () => { }); expect(clearAuthSpy).toHaveBeenCalled(); - expect(setAuthTokenSpy).toHaveBeenCalledWith("new-token"); + expect(setAuthTokenSpy).toHaveBeenCalled(); + expect(setAuthTokenSpy.mock.calls[0]?.[0]).toBe("new-token"); + expect(setAuthTokenSpy.mock.calls[0]?.[3]).toMatchObject({ + host: expect.any(String), + }); expect(getStdout()).toContain("Authenticated"); }); diff --git a/test/commands/event/view.test.ts b/test/commands/event/view.test.ts index ba30682eb..a89bc01cd 100644 --- a/test/commands/event/view.test.ts +++ b/test/commands/event/view.test.ts @@ -258,19 +258,25 @@ describe("parsePositionalArgs", () => { }); }); - // URL integration tests — applySentryUrlContext may set SENTRY_HOST/SENTRY_URL as a side effect + // URL integration tests — applySentryUrlContext may set SENTRY_HOST/SENTRY_URL as a side effect. + // Host-scoping: self-hosted URLs now require the token to be scoped to the + // same host. Tests seed SENTRY_HOST before parsing so env-token-host matches. describe("Sentry URL inputs", () => { let savedSentryUrl: string | undefined; let savedSentryHost: string | undefined; - beforeEach(() => { + beforeEach(async () => { savedSentryUrl = process.env.SENTRY_URL; savedSentryHost = process.env.SENTRY_HOST; delete process.env.SENTRY_URL; delete process.env.SENTRY_HOST; + const { resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); - afterEach(() => { + afterEach(async () => { if (savedSentryUrl !== undefined) { process.env.SENTRY_URL = savedSentryUrl; } else { @@ -281,6 +287,10 @@ describe("parsePositionalArgs", () => { } else { delete process.env.SENTRY_HOST; } + const { resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); test("event URL extracts eventId and passes org as OrgAll target", () => { @@ -291,7 +301,8 @@ describe("parsePositionalArgs", () => { expect(result.targetArg).toBe("my-org/"); }); - test("self-hosted event URL extracts eventId, passes org, sets SENTRY_URL", () => { + test("self-hosted event URL extracts eventId, passes org, sets SENTRY_URL (requires matching token host)", () => { + process.env.SENTRY_HOST = "https://sentry.example.com"; const result = parsePositionalArgs([ "https://sentry.example.com/organizations/acme/issues/999/events/deadbeef/", ]); diff --git a/test/lib/arg-parsing.test.ts b/test/lib/arg-parsing.test.ts index 6d9361ad0..2bfa884f0 100644 --- a/test/lib/arg-parsing.test.ts +++ b/test/lib/arg-parsing.test.ts @@ -110,19 +110,26 @@ describe("parseOrgProjectArg", () => { }); }); - // URL integration tests — applySentryUrlContext may set SENTRY_HOST/SENTRY_URL as a side effect + // URL integration tests — applySentryUrlContext may set SENTRY_HOST/SENTRY_URL as a side effect. + // Host-scoping: non-SaaS URLs now require the token to be scoped to the + // same host. Tests that pass self-hosted URLs must set SENTRY_HOST before + // running so the env-token-host snapshot matches. describe("Sentry URL inputs", () => { let savedSentryUrl: string | undefined; let savedSentryHost: string | undefined; - beforeEach(() => { + beforeEach(async () => { savedSentryUrl = process.env.SENTRY_URL; savedSentryHost = process.env.SENTRY_HOST; delete process.env.SENTRY_URL; delete process.env.SENTRY_HOST; + const { resetEnvTokenHostForTesting } = await import( + "../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); - afterEach(() => { + afterEach(async () => { if (savedSentryUrl !== undefined) { process.env.SENTRY_URL = savedSentryUrl; } else { @@ -133,6 +140,10 @@ describe("parseOrgProjectArg", () => { } else { delete process.env.SENTRY_HOST; } + const { resetEnvTokenHostForTesting } = await import( + "../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); test("issue URL returns org-all", () => { @@ -167,7 +178,9 @@ describe("parseOrgProjectArg", () => { }); }); - test("self-hosted URL extracts org", () => { + test("self-hosted URL extracts org when token is scoped to that host", () => { + // Pin env-token to sentry.example.com so the URL-arg's host matches. + process.env.SENTRY_HOST = "https://sentry.example.com"; expect( parseOrgProjectArg( "https://sentry.example.com/organizations/acme-corp/issues/99/" @@ -177,6 +190,15 @@ describe("parseOrgProjectArg", () => { org: "acme-corp", }); }); + + test("self-hosted URL throws when token is scoped to a different host", () => { + // No SENTRY_HOST set → env-token defaults to SaaS → mismatch on self-hosted URL. + expect(() => + parseOrgProjectArg( + "https://sentry.example.com/organizations/acme-corp/issues/99/" + ) + ).toThrow(/does not match|sentry auth login --url/); + }); }); describe("space handling (no normalization)", () => { @@ -422,7 +444,8 @@ describe("parseIssueArg", () => { }); }); - test("self-hosted issue URL with query params", () => { + test("self-hosted issue URL with query params (requires matching token host)", () => { + process.env.SENTRY_HOST = "https://sentry.example.com"; expect( parseIssueArg( "https://sentry.example.com/organizations/acme/issues/32886/?project=2" @@ -503,7 +526,8 @@ describe("parseIssueArg", () => { }); }); - test("self-hosted share URL returns share type", () => { + test("self-hosted share URL returns share type (requires matching token host)", () => { + process.env.SENTRY_HOST = "https://sentry.example.com"; expect( parseIssueArg( "https://sentry.example.com/share/issue/aabbccdd11223344aabbccdd11223344/" diff --git a/test/lib/custom-headers.test.ts b/test/lib/custom-headers.test.ts index 9399984a9..3f8c083aa 100644 --- a/test/lib/custom-headers.test.ts +++ b/test/lib/custom-headers.test.ts @@ -251,13 +251,17 @@ describe("applyCustomHeaders", () => { let savedHeaders: string | undefined; let savedHost: string | undefined; - beforeEach(() => { + beforeEach(async () => { savedHeaders = process.env.SENTRY_CUSTOM_HEADERS; savedHost = process.env.SENTRY_HOST; _resetCustomHeadersCache(); + const { resetEnvTokenHostForTesting } = await import( + "../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); - afterEach(() => { + afterEach(async () => { if (savedHeaders !== undefined) { process.env.SENTRY_CUSTOM_HEADERS = savedHeaders; } else { @@ -269,14 +273,22 @@ describe("applyCustomHeaders", () => { delete process.env.SENTRY_HOST; } _resetCustomHeadersCache(); + const { resetEnvTokenHostForTesting } = await import( + "../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); - test("applies custom headers to Headers instance", () => { + test("applies custom headers to Headers instance when request host matches", () => { process.env.SENTRY_CUSTOM_HEADERS = "X-Test: hello; X-Other: world"; + // Pin env-token scope to the self-hosted instance so headers apply. process.env.SENTRY_HOST = "https://sentry.example.com"; const headers = new Headers({ Accept: "application/json" }); - applyCustomHeaders(headers); + applyCustomHeaders( + headers, + "https://sentry.example.com/api/0/organizations/" + ); expect(headers.get("X-Test")).toBe("hello"); expect(headers.get("X-Other")).toBe("world"); @@ -287,7 +299,10 @@ describe("applyCustomHeaders", () => { process.env.SENTRY_HOST = "https://sentry.example.com"; const headers = new Headers({ Accept: "application/json" }); - applyCustomHeaders(headers); + applyCustomHeaders( + headers, + "https://sentry.example.com/api/0/organizations/" + ); expect(headers.get("Accept")).toBe("application/json"); expect(headers.get("X-Test")).toBe("hello"); @@ -298,10 +313,28 @@ describe("applyCustomHeaders", () => { delete process.env.SENTRY_HOST; const headers = new Headers({ Accept: "application/json" }); - applyCustomHeaders(headers); + applyCustomHeaders(headers, "https://sentry.io/api/0/"); // Only the original header const keys = [...headers.keys()]; expect(keys).toEqual(["accept"]); }); + + test("skips custom headers when request URL host does not match trusted host (CVE defense)", () => { + // Self-hosted user has IAP token configured for their proxy: + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: secret"; + process.env.SENTRY_HOST = "https://sentry.example.com"; + + // A share URL from an attacker resolves to evil.example.com. Even though + // applyCustomHeaders is on a non-SaaS codepath (isSelfHosted()==true for + // this host-config), the request's destination is the untrusted URL, so + // the IAP token must NOT attach. + const headers = new Headers({ Accept: "application/json" }); + applyCustomHeaders( + headers, + "https://evil.example.com/api/0/shared/issues/deadbeef/" + ); + + expect(headers.get("X-IAP-Token")).toBeNull(); + }); }); diff --git a/test/lib/db/auth.host.test.ts b/test/lib/db/auth.host.test.ts new file mode 100644 index 000000000..733af0072 --- /dev/null +++ b/test/lib/db/auth.host.test.ts @@ -0,0 +1,139 @@ +/** + * Tests for host-scoped auth: setAuthToken persistence, getStoredAuthHost, + * NULL-host lazy migration, host preservation across refresh-style updates. + */ + +import { describe, expect, test } from "bun:test"; +import { + getStoredAuthHost, + hasUsableStoredToken, + setAuthToken, +} from "../../../src/lib/db/auth.js"; +import { getDatabase } from "../../../src/lib/db/index.js"; +import { useTestConfigDir } from "../../helpers.js"; + +describe("db/auth host scoping", () => { + useTestConfigDir("auth-host-test-"); + + test("setAuthToken persists explicit host", () => { + setAuthToken("tok-1", undefined, undefined, { + host: "https://sentry.acme.com", + }); + expect(getStoredAuthHost()).toBe("https://sentry.acme.com"); + }); + + test("setAuthToken normalizes host (lowercases + strips trailing slash)", () => { + setAuthToken("tok-1", undefined, undefined, { + host: "https://SENTRY.Acme.com/", + }); + // normalizeOrigin lowercases + strips trailing slash + expect(getStoredAuthHost()).toBe("https://sentry.acme.com"); + }); + + test("setAuthToken without host explicit uses configured host", () => { + const prevHost = process.env.SENTRY_HOST; + process.env.SENTRY_HOST = "https://env-host.example.com"; + try { + setAuthToken("tok-2"); + expect(getStoredAuthHost()).toBe("https://env-host.example.com"); + } finally { + if (prevHost === undefined) { + delete process.env.SENTRY_HOST; + } else { + process.env.SENTRY_HOST = prevHost; + } + } + }); + + test("setAuthToken without host falls back to DEFAULT_SENTRY_URL", () => { + const prevHost = process.env.SENTRY_HOST; + const prevUrl = process.env.SENTRY_URL; + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + try { + setAuthToken("tok-3"); + expect(getStoredAuthHost()).toBe("https://sentry.io"); + } finally { + if (prevHost !== undefined) { + process.env.SENTRY_HOST = prevHost; + } + if (prevUrl !== undefined) { + process.env.SENTRY_URL = prevUrl; + } + } + }); + + test("refresh-style update preserves existing host when options.host omitted", () => { + setAuthToken("tok-initial", 3600, "refresh-tok", { + host: "https://sentry.acme.com", + }); + expect(getStoredAuthHost()).toBe("https://sentry.acme.com"); + + // Simulate a refresh: new access token + same refresh token, no host + setAuthToken("tok-refreshed", 3600, "refresh-tok"); + expect(getStoredAuthHost()).toBe("https://sentry.acme.com"); + }); + + test("lazy migration: NULL host is backfilled from configured host on first access", () => { + // Simulate a pre-v16 row: direct INSERT bypassing setAuthToken + const db = getDatabase(); + db.query( + "INSERT OR REPLACE INTO auth (id, token, host, updated_at) VALUES (1, 'legacy-token', NULL, ?)" + ).run(Date.now()); + + const prevHost = process.env.SENTRY_HOST; + process.env.SENTRY_HOST = "https://legacy-configured.example.com"; + try { + expect(getStoredAuthHost()).toBe("https://legacy-configured.example.com"); + // Second call reads the now-populated host + expect(getStoredAuthHost()).toBe("https://legacy-configured.example.com"); + // Verify it was actually written to the DB + const row = db.query("SELECT host FROM auth WHERE id = 1").get() as { + host: string | null; + }; + expect(row.host).toBe("https://legacy-configured.example.com"); + } finally { + if (prevHost === undefined) { + delete process.env.SENTRY_HOST; + } else { + process.env.SENTRY_HOST = prevHost; + } + } + }); + + test("lazy migration: NULL host with no configured host falls back to SaaS", () => { + const db = getDatabase(); + db.query( + "INSERT OR REPLACE INTO auth (id, token, host, updated_at) VALUES (1, 'legacy-token', NULL, ?)" + ).run(Date.now()); + + const prevHost = process.env.SENTRY_HOST; + const prevUrl = process.env.SENTRY_URL; + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + try { + expect(getStoredAuthHost()).toBe("https://sentry.io"); + } finally { + if (prevHost !== undefined) { + process.env.SENTRY_HOST = prevHost; + } + if (prevUrl !== undefined) { + process.env.SENTRY_URL = prevUrl; + } + } + }); + + test("getStoredAuthHost returns undefined when no stored token", () => { + // Nothing persisted + expect(getStoredAuthHost()).toBeUndefined(); + }); + + test("hasUsableStoredToken reflects stored row status", () => { + expect(hasUsableStoredToken()).toBe(false); + + setAuthToken("tok-usable", 3600, "refresh", { + host: "https://sentry.io", + }); + expect(hasUsableStoredToken()).toBe(true); + }); +}); diff --git a/test/lib/env-token-host.test.ts b/test/lib/env-token-host.test.ts new file mode 100644 index 000000000..f6e685ed8 --- /dev/null +++ b/test/lib/env-token-host.test.ts @@ -0,0 +1,103 @@ +/** + * Unit tests for env-token-host snapshot capture. + * + * Asserts: + * - Snapshot reads env-only (not .sentryclirc-injected values later). + * - Default is `DEFAULT_SENTRY_URL` when env is unset. + * - `captureEnvTokenHost` is idempotent. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { DEFAULT_SENTRY_URL } from "../../src/lib/constants.js"; +import { + captureEnvTokenHost, + getEnvTokenHost, + resetEnvTokenHostForTesting, +} from "../../src/lib/env-token-host.js"; + +const ENV_KEYS = ["SENTRY_HOST", "SENTRY_URL"] as const; + +describe("env-token-host", () => { + let saved: Record; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + for (const k of ENV_KEYS) { + delete process.env[k]; + } + resetEnvTokenHostForTesting(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + const v = saved[k]; + if (v !== undefined) { + process.env[k] = v; + } else { + delete process.env[k]; + } + } + resetEnvTokenHostForTesting(); + }); + + test("defaults to SaaS when neither SENTRY_HOST nor SENTRY_URL is set", () => { + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe(DEFAULT_SENTRY_URL); + }); + + test("captures SENTRY_HOST when set", () => { + process.env.SENTRY_HOST = "https://sentry.acme.com"; + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe("https://sentry.acme.com"); + }); + + test("captures SENTRY_URL when SENTRY_HOST is unset", () => { + process.env.SENTRY_URL = "https://sentry.acme.com"; + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe("https://sentry.acme.com"); + }); + + test("prefers SENTRY_HOST over SENTRY_URL when both are set", () => { + process.env.SENTRY_HOST = "https://host.example.com"; + process.env.SENTRY_URL = "https://url.example.com"; + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe("https://host.example.com"); + }); + + test("normalizes bare hostname to https://", () => { + process.env.SENTRY_HOST = "sentry.acme.com"; + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe("https://sentry.acme.com"); + }); + + test("is idempotent — second capture is a no-op", () => { + process.env.SENTRY_HOST = "https://first.example.com"; + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe("https://first.example.com"); + + // Change env AFTER initial capture — snapshot should NOT update + process.env.SENTRY_HOST = "https://second.example.com"; + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe("https://first.example.com"); + }); + + test("does NOT consult values added to env after capture (the .sentryclirc simulation)", () => { + // Simulates: captureEnvTokenHost() at boot, then + // applySentryCliRcEnvShim() writes SENTRY_URL. The env-token-host + // snapshot must NOT reflect the shim write. + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe(DEFAULT_SENTRY_URL); + + // Simulate shim write + process.env.SENTRY_URL = "https://injected-by-sentryclirc.com"; + // Second call is a no-op (idempotent) + captureEnvTokenHost(); + expect(getEnvTokenHost()).toBe(DEFAULT_SENTRY_URL); + }); + + test("auto-captures on first getEnvTokenHost() call without explicit capture", () => { + process.env.SENTRY_HOST = "https://auto.example.com"; + // Never call captureEnvTokenHost explicitly + expect(getEnvTokenHost()).toBe("https://auto.example.com"); + }); +}); diff --git a/test/lib/security/fetch-layer-guard.test.ts b/test/lib/security/fetch-layer-guard.test.ts new file mode 100644 index 000000000..6ff560c36 --- /dev/null +++ b/test/lib/security/fetch-layer-guard.test.ts @@ -0,0 +1,111 @@ +/** + * Defense-in-depth regression tests for the fetch layer. + * + * Even if a future code path bypasses the URL-arg / .sentryclirc entry-point + * guards and writes `SENTRY_HOST`/`SENTRY_URL` directly, the fetch layer + * must still refuse to attach credentials to a request whose origin + * doesn't match the active token's scope. + * + * This file simulates the bypass by directly calling the lower-level + * primitives (`apiRequest`, `applyCustomHeaders`, `refreshAccessToken`) + * with mismatched hosts. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + _resetCustomHeadersCache, + applyCustomHeaders, +} from "../../../src/lib/custom-headers.js"; +import { resetEnvTokenHostForTesting } from "../../../src/lib/env-token-host.js"; + +const ENV_KEYS = [ + "SENTRY_HOST", + "SENTRY_URL", + "SENTRY_CUSTOM_HEADERS", +] as const; + +describe("CVE defense-in-depth: fetch layer refuses mismatched hosts", () => { + let saved: Record; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + for (const k of ENV_KEYS) { + delete process.env[k]; + } + resetEnvTokenHostForTesting(); + _resetCustomHeadersCache(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + const v = saved[k]; + if (v !== undefined) { + process.env[k] = v; + } else { + delete process.env[k]; + } + } + resetEnvTokenHostForTesting(); + _resetCustomHeadersCache(); + }); + + test("applyCustomHeaders: IAP token does NOT leak to untrusted URL (CVE #3)", () => { + // Reproduce the share-URL attack directly at the header layer. + // Even if something bypasses applySentryUrlContext and arrives at + // applyCustomHeaders with a mismatched URL, the header attach must + // refuse. + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: sensitive-iap-value"; + process.env.SENTRY_HOST = "https://sentry.example.com"; + + const headers = new Headers(); + applyCustomHeaders( + headers, + "https://evil.com/api/0/shared/issues/deadbeef/" + ); + + expect(headers.get("X-IAP-Token")).toBeNull(); + // Verify no other custom headers leaked either + expect([...headers.keys()]).toHaveLength(0); + }); + + test("applyCustomHeaders: IAP token attaches to trusted URL", () => { + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: sensitive-iap-value"; + process.env.SENTRY_HOST = "https://sentry.example.com"; + + const headers = new Headers(); + applyCustomHeaders( + headers, + "https://sentry.example.com/api/0/organizations/" + ); + + expect(headers.get("X-IAP-Token")).toBe("sensitive-iap-value"); + }); + + test("applyCustomHeaders: does NOT attach to a look-alike SaaS host (prefix/suffix attack)", () => { + // Critical: sentry.io.evil.com is NOT a sentry.io subdomain, so even + // a SaaS-scoped token shouldn't grant trust to it. + // SENTRY_CUSTOM_HEADERS is an IAP/proxy feature that only applies to + // self-hosted instances (getCustomHeaders short-circuits on SaaS), + // so we set SENTRY_HOST to enable it for this test. + process.env.SENTRY_CUSTOM_HEADERS = "X-Custom: value"; + process.env.SENTRY_HOST = "https://sentry.acme.com"; + + const headers = new Headers(); + applyCustomHeaders(headers, "https://sentry.acme.com.evil.com/api/0/"); + + expect(headers.get("X-Custom")).toBeNull(); + }); + + test("applyCustomHeaders: subdomain-attack on self-hosted is refused", () => { + // Token scoped to sentry.acme.com must not authorize sub.sentry.acme.com + // when it's actually an attacker-controlled subdomain takeover. + // (Non-SaaS trust class requires EXACT origin match.) + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: secret"; + process.env.SENTRY_HOST = "https://sentry.acme.com"; + + const headers = new Headers(); + applyCustomHeaders(headers, "https://attacker.sentry.acme.com/api/0/"); + + expect(headers.get("X-IAP-Token")).toBeNull(); + }); +}); diff --git a/test/lib/security/refresh-token-poison.test.ts b/test/lib/security/refresh-token-poison.test.ts new file mode 100644 index 000000000..f3f381bee --- /dev/null +++ b/test/lib/security/refresh-token-poison.test.ts @@ -0,0 +1,115 @@ +/** + * CVE defense-in-depth: OAuth refresh-token credential exfiltration. + * + * Attack: if something bypasses the entry-point guards and poisons + * `env.SENTRY_URL` before the next OAuth refresh fires, the refresh token + * would previously be POSTed to the attacker's `/oauth/token/` endpoint. + * + * Fix: `refreshAccessToken` calls `assertRefreshHostTrusted()` before + * building the request body, which throws `CliError` on mismatch. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + captureEnvTokenHost, + resetEnvTokenHostForTesting, +} from "../../../src/lib/env-token-host.js"; +import { refreshAccessToken } from "../../../src/lib/oauth.js"; + +const ENV_KEYS = ["SENTRY_HOST", "SENTRY_URL", "SENTRY_CLIENT_ID"] as const; + +function extractUrl(input: RequestInfo | URL): string { + if (typeof input === "string") { + return input; + } + if (input instanceof URL) { + return input.href; + } + return input.url; +} + +describe("CVE defense-in-depth: refresh token", () => { + let saved: Record; + let fetchCalls: string[]; + let originalFetch: typeof globalThis.fetch; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + for (const k of ENV_KEYS) { + delete process.env[k]; + } + // A client ID is required for refreshAccessToken to proceed past the + // config check. We want it to reach (and fail at) the host assertion. + process.env.SENTRY_CLIENT_ID = "test-client-id"; + resetEnvTokenHostForTesting(); + // Intercept fetch to detect any outbound request attempt. + fetchCalls = []; + originalFetch = globalThis.fetch; + globalThis.fetch = (async (input: RequestInfo | URL) => { + fetchCalls.push(extractUrl(input)); + throw new Error("test: unexpected fetch"); + }) as typeof fetch; + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + const v = saved[k]; + if (v !== undefined) { + process.env[k] = v; + } else { + delete process.env[k]; + } + } + resetEnvTokenHostForTesting(); + globalThis.fetch = originalFetch; + }); + + test("refreshAccessToken throws before fetch when env.SENTRY_URL is poisoned after boot", async () => { + // Step 1: simulate boot — capture env-token-host with no SENTRY_URL set + // (defaults to SaaS, matching a user who got SENTRY_AUTH_TOKEN from their + // shell without configuring SENTRY_HOST). + resetEnvTokenHostForTesting(); + captureEnvTokenHost(); // snapshots → SaaS default + + // Step 2: simulate the bypass — something writes env.SENTRY_URL AFTER + // the snapshot. This is the attack shape: env got poisoned by a + // code path that skipped the URL-arg / rc-shim guards. + process.env.SENTRY_URL = "https://evil.com"; + + // `refreshAccessToken` throws synchronously from its host-scope guard + // (before returning the promise from withHttpSpan). Handle both shapes. + let thrown: unknown; + try { + await refreshAccessToken("fake-refresh-token"); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(Error); + expect((thrown as Error).message).toMatch( + /does not match|sentry auth login --url/ + ); + + // Critical: zero outbound requests to evil.com (or anywhere). + expect(fetchCalls).toEqual([]); + }); + + test("refreshAccessToken proceeds when URL matches token scope", async () => { + // Pin env-token to the self-hosted instance BEFORE the url is used. + process.env.SENTRY_HOST = "https://sentry.example.com"; + resetEnvTokenHostForTesting(); + captureEnvTokenHost(); + // Also set SENTRY_URL so getSentryUrl() returns the same host + process.env.SENTRY_URL = "https://sentry.example.com"; + + // Should NOT throw at the host-assertion; the actual fetch will fail + // with the mock "test: unexpected fetch" error, which is fine — the + // important thing is that the pre-fetch assertion let us through. + await expect(refreshAccessToken("fake-refresh-token")).rejects.toThrow( + /unexpected fetch|Cannot connect|fetch failed/ + ); + + // A request was attempted, and it went to the correct host + expect(fetchCalls).toHaveLength(1); + expect(fetchCalls[0]).toBe("https://sentry.example.com/oauth/token/"); + }); +}); diff --git a/test/lib/security/sentryclirc-url-poison.test.ts b/test/lib/security/sentryclirc-url-poison.test.ts new file mode 100644 index 000000000..26fca9cd2 --- /dev/null +++ b/test/lib/security/sentryclirc-url-poison.test.ts @@ -0,0 +1,149 @@ +/** + * CVE regression: .sentryclirc URL injection attack. + * + * Attack: a committed `.sentryclirc` file in a cloned repo sets + * `[defaults] url = https://evil.com`. The env-shim previously wrote + * `env.SENTRY_URL=https://evil.com` when neither `SENTRY_HOST` nor + * `SENTRY_URL` was set, independent of whether `SENTRY_AUTH_TOKEN` was + * present — so a developer's real token got sent to the attacker on every + * CLI command. + * + * Fix (this PR): the shim consults `getActiveTokenHost()` (from env-only + * snapshot, not .sentryclirc itself) and throws `CliError` when the rc + * url doesn't match the active token's scope. SaaS URLs bypass the check + * (no credentials can leak to SaaS). + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { closeDatabase } from "../../../src/lib/db/index.js"; +import { + captureEnvTokenHost, + resetEnvTokenHostForTesting, +} from "../../../src/lib/env-token-host.js"; +import { + applySentryCliRcEnvShim, + CONFIG_FILENAME, + clearSentryCliRcCache, +} from "../../../src/lib/sentryclirc.js"; +import { cleanupTestDir, createTestConfigDir } from "../../helpers.js"; + +const ENV_KEYS = [ + "SENTRY_AUTH_TOKEN", + "SENTRY_TOKEN", + "SENTRY_HOST", + "SENTRY_URL", + "SENTRY_CONFIG_DIR", +] as const; + +function writeRc(dir: string, content: string): void { + writeFileSync(join(dir, CONFIG_FILENAME), content, "utf-8"); +} + +describe("CVE: .sentryclirc URL credential exfiltration", () => { + let testDir: string; + let saved: Record; + + beforeEach(async () => { + clearSentryCliRcCache(); + closeDatabase(); + saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + testDir = await createTestConfigDir("sentryclirc-cve-", { + isolateProjectRoot: true, + }); + process.env.SENTRY_CONFIG_DIR = testDir; + resetEnvTokenHostForTesting(); + }); + + afterEach(async () => { + clearSentryCliRcCache(); + closeDatabase(); + for (const k of ENV_KEYS) { + const v = saved[k]; + if (v !== undefined) { + process.env[k] = v; + } else { + delete process.env[k]; + } + } + resetEnvTokenHostForTesting(); + await cleanupTestDir(testDir); + }); + + test("repo-local .sentryclirc with attacker URL throws (SENTRY_AUTH_TOKEN default SaaS scope)", async () => { + // Attack setup: user has SENTRY_AUTH_TOKEN (scoped to SaaS by default + // because no SENTRY_HOST is set). Attacker repo ships .sentryclirc + // pointing at evil.com. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + // SENTRY_AUTH_TOKEN is already set by test/preload.ts + writeRc(testDir, "[defaults]\nurl = https://evil.com\n"); + + await expect(applySentryCliRcEnvShim(testDir)).rejects.toThrow( + /does not match|sentry auth login --url/ + ); + + // Critical: env must remain unpoisoned so no credentialed request + // will be sent to evil.com. + expect(process.env.SENTRY_URL).toBeUndefined(); + expect(process.env.SENTRY_HOST).toBeUndefined(); + }); + + test("global-style .sentryclirc with attacker URL is rejected too (no 'global is trusted' bypass)", async () => { + // The plan explicitly rejects 'global rc is trusted' because CI runners + // can write ~/.sentryclirc. Writing to the config dir (treated as + // global) must NOT be a trust-establishment pathway. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + // Write the rc in the config dir (SENTRY_CONFIG_DIR is set above). + // That's treated as a "global" path by the shim's scope tagging. + writeRc(testDir, "[defaults]\nurl = https://evil.com\n"); + + await expect(applySentryCliRcEnvShim(testDir)).rejects.toThrow( + /does not match|sentry auth login --url/ + ); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + + test("SaaS URL in .sentryclirc proceeds (no credential leak possible to SaaS)", async () => { + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + writeRc(testDir, "[defaults]\nurl = https://sentry.io\n"); + + await applySentryCliRcEnvShim(testDir); + expect(process.env.SENTRY_URL).toBe("https://sentry.io"); + }); + + test("matching self-hosted URL proceeds when env-token is scoped to that host", async () => { + // Simulate boot ordering: user sets SENTRY_HOST via env (shell export), + // captureEnvTokenHost() pins the token's scope to that host, THEN the + // env gets cleared (simulating e.g. a test isolation or a shell that + // only exported SENTRY_HOST for a particular subcommand). The rc file + // then proposes the same host the token was scoped to — match. + process.env.SENTRY_HOST = "https://sentry.example.com"; + resetEnvTokenHostForTesting(); + captureEnvTokenHost(); // pin to sentry.example.com + // Now simulate the shim running when both env vars are unset so the + // shim's "only write if both unset" guard admits the rc-sourced URL. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + writeRc(testDir, "[defaults]\nurl = https://sentry.example.com\n"); + + await applySentryCliRcEnvShim(testDir); + expect(process.env.SENTRY_URL).toBe("https://sentry.example.com"); + }); + + test("existing SENTRY_HOST is never overridden by rc (shim has its own guard)", async () => { + // Pre-existing SENTRY_HOST from user env — rc must not override it + // regardless of what the rc says. + process.env.SENTRY_HOST = "https://sentry.example.com"; + resetEnvTokenHostForTesting(); + writeRc(testDir, "[defaults]\nurl = https://evil.com\n"); + + // Shim's own "only write if both unset" guard kicks in first → silent no-op + await applySentryCliRcEnvShim(testDir); + expect(process.env.SENTRY_HOST).toBe("https://sentry.example.com"); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); +}); diff --git a/test/lib/security/url-arg-poison.test.ts b/test/lib/security/url-arg-poison.test.ts new file mode 100644 index 000000000..f2743cd04 --- /dev/null +++ b/test/lib/security/url-arg-poison.test.ts @@ -0,0 +1,114 @@ +/** + * CVE regression: URL argument attack. + * + * Attack: `sentry issue view https://evil.com/organizations/x/issues/1/` + * previously wrote `env.SENTRY_HOST=https://evil.com` with no validation, + * causing every subsequent authenticated fetch + OAuth refresh to send + * credentials to the attacker. + * + * Fix (this PR): `applySentryUrlContext` rejects non-SaaS URLs that don't + * match the active token's scoped host. Only `sentry auth login --url ` + * can establish trust for a new host. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { parsePositionalArgs } from "../../../src/commands/event/view.js"; +import { + parseIssueArg, + parseOrgProjectArg, +} from "../../../src/lib/arg-parsing.js"; +import { resetEnvTokenHostForTesting } from "../../../src/lib/env-token-host.js"; + +const ENV_KEYS = ["SENTRY_HOST", "SENTRY_URL"] as const; + +describe("CVE: URL argument credential exfiltration", () => { + let saved: Record; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + for (const k of ENV_KEYS) { + delete process.env[k]; + } + resetEnvTokenHostForTesting(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + const v = saved[k]; + if (v !== undefined) { + process.env[k] = v; + } else { + delete process.env[k]; + } + } + resetEnvTokenHostForTesting(); + }); + + test("parseIssueArg with attacker URL throws before env is poisoned (SaaS-scoped token)", () => { + // preload sets SENTRY_AUTH_TOKEN; env-token defaults to SaaS. + expect(() => + parseIssueArg("https://evil.com/organizations/target-org/issues/12345/") + ).toThrow(/does not match|sentry auth login --url/); + + // Env untouched — no routing poison. + expect(process.env.SENTRY_HOST).toBeUndefined(); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + + test("parseOrgProjectArg with attacker URL throws before env is poisoned", () => { + expect(() => + parseOrgProjectArg( + "https://evil.com/organizations/target-org/issues/12345/" + ) + ).toThrow(/does not match|sentry auth login --url/); + + expect(process.env.SENTRY_HOST).toBeUndefined(); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + + test("event view with attacker URL throws before env is poisoned", () => { + expect(() => + parsePositionalArgs([ + "https://evil.com/organizations/acme/issues/999/events/deadbeef/", + ]) + ).toThrow(/does not match|sentry auth login --url/); + + expect(process.env.SENTRY_HOST).toBeUndefined(); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + + test("share URL from attacker host throws (CVE #3: custom-headers leak)", () => { + // The share-URL variant of the CVE. parseIssueArg still runs + // applySentryUrlContext, which throws before getSharedIssue is invoked. + expect(() => + parseIssueArg( + "https://evil.com/share/issue/deadbeef12345678deadbeef12345678/" + ) + ).toThrow(/does not match|sentry auth login --url/); + + expect(process.env.SENTRY_HOST).toBeUndefined(); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + + test("SaaS URL arg proceeds even when SENTRY_HOST is currently set to self-hosted", () => { + // SaaS URLs always proceed — they're part of the SaaS trust class + // when the active token is SaaS-scoped, and they clear env to route + // correctly. + process.env.SENTRY_HOST = "https://old-self-hosted.example.com"; + process.env.SENTRY_URL = "https://old-self-hosted.example.com"; + parseIssueArg("https://sentry.io/organizations/acme/issues/1234/"); + // env cleared so default SaaS routing takes over + expect(process.env.SENTRY_HOST).toBeUndefined(); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + + test("matching self-hosted URL arg is honored when token scoped to that host", () => { + process.env.SENTRY_HOST = "https://sentry.example.com"; + resetEnvTokenHostForTesting(); + parseOrgProjectArg( + "https://sentry.example.com/organizations/acme/issues/1/" + ); + expect(process.env.SENTRY_HOST).toBe("https://sentry.example.com"); + expect(process.env.SENTRY_URL).toBe("https://sentry.example.com"); + }); +}); diff --git a/test/lib/sentry-url-parser.test.ts b/test/lib/sentry-url-parser.test.ts index 3e688d77b..b9400444a 100644 --- a/test/lib/sentry-url-parser.test.ts +++ b/test/lib/sentry-url-parser.test.ts @@ -382,17 +382,32 @@ describe("parseSentryUrl", () => { }); describe("applySentryUrlContext", () => { + // Host-scoping: applySentryUrlContext honors non-SaaS URLs ONLY when the + // destination matches the active token's scoped host (with SaaS + // equivalence). Mismatches throw CliError so credentials can't leak to an + // attacker-chosen host. + // + // The test preload (test/preload.ts) sets `SENTRY_AUTH_TOKEN` scoped to + // SaaS by default. To simulate a self-hosted-authenticated user, set + // `SENTRY_HOST` BEFORE the module loads (which pins the env-token's scope) + // and use `resetEnvTokenHostForTesting()` between cases. let originalSentryUrl: string | undefined; let originalSentryHost: string | undefined; - beforeEach(() => { + beforeEach(async () => { originalSentryUrl = process.env.SENTRY_URL; originalSentryHost = process.env.SENTRY_HOST; delete process.env.SENTRY_URL; delete process.env.SENTRY_HOST; + // Reset env-token-host capture so each test can re-pin based on the + // SENTRY_HOST they set (or leave unset → SaaS default). + const { resetEnvTokenHostForTesting } = await import( + "../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); - afterEach(() => { + afterEach(async () => { if (originalSentryUrl !== undefined) { process.env.SENTRY_URL = originalSentryUrl; } else { @@ -403,14 +418,31 @@ describe("applySentryUrlContext", () => { } else { delete process.env.SENTRY_HOST; } + const { resetEnvTokenHostForTesting } = await import( + "../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); }); - test("sets both SENTRY_HOST and SENTRY_URL for self-hosted instance", () => { + test("writes env when non-SaaS URL matches token-scoped host", () => { + // Pin env-token to the self-hosted instance via SENTRY_HOST before + // calling captureEnvTokenHost() (implicit on first getEnvTokenHost call). + process.env.SENTRY_HOST = "https://sentry.example.com"; applySentryUrlContext("https://sentry.example.com"); expect(process.env.SENTRY_HOST).toBe("https://sentry.example.com"); expect(process.env.SENTRY_URL).toBe("https://sentry.example.com"); }); + test("throws CliError for non-SaaS URL that does not match token host", () => { + // Env-token defaults to SaaS (no SENTRY_HOST set), so a self-hosted URL + // is a host-scope mismatch → CliError, env untouched. + expect(() => applySentryUrlContext("https://sentry.example.com")).toThrow( + /does not match|sentry auth login --url/ + ); + expect(process.env.SENTRY_HOST).toBeUndefined(); + expect(process.env.SENTRY_URL).toBeUndefined(); + }); + test("does not set SENTRY_HOST or SENTRY_URL for SaaS (sentry.io)", () => { applySentryUrlContext("https://sentry.io"); expect(process.env.SENTRY_HOST).toBeUndefined(); @@ -423,15 +455,22 @@ describe("applySentryUrlContext", () => { expect(process.env.SENTRY_URL).toBeUndefined(); }); - test("overrides existing env vars (parsed URL takes precedence)", () => { + test("throws on mismatch even when SENTRY_HOST is pre-set to a different host", () => { + // Token scoped to existing.example.com; URL-arg points at sentry.other.com. + // Primary guard refuses to re-scope by writing the new host — only + // `sentry auth login --url` may change scope. process.env.SENTRY_HOST = "https://existing.example.com"; process.env.SENTRY_URL = "https://existing.example.com"; - applySentryUrlContext("https://sentry.other.com"); - expect(process.env.SENTRY_HOST).toBe("https://sentry.other.com"); - expect(process.env.SENTRY_URL).toBe("https://sentry.other.com"); + expect(() => applySentryUrlContext("https://sentry.other.com")).toThrow( + /does not match|sentry auth login --url/ + ); + // Existing env left intact — throw happens before any write. + expect(process.env.SENTRY_HOST).toBe("https://existing.example.com"); + expect(process.env.SENTRY_URL).toBe("https://existing.example.com"); }); - test("sets both env vars for self-hosted with port", () => { + test("writes env for self-hosted URL with port when it matches token host", () => { + process.env.SENTRY_HOST = "https://sentry.acme.internal:9000"; applySentryUrlContext("https://sentry.acme.internal:9000"); expect(process.env.SENTRY_HOST).toBe("https://sentry.acme.internal:9000"); expect(process.env.SENTRY_URL).toBe("https://sentry.acme.internal:9000"); diff --git a/test/lib/sentryclirc.test.ts b/test/lib/sentryclirc.test.ts index 4a3a17f72..62f6037b7 100644 --- a/test/lib/sentryclirc.test.ts +++ b/test/lib/sentryclirc.test.ts @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { closeDatabase } from "../../src/lib/db/index.js"; +import { resetEnvTokenHostForTesting } from "../../src/lib/env-token-host.js"; import { applySentryCliRcEnvShim, CONFIG_FILENAME, @@ -38,6 +39,7 @@ beforeEach(async () => { }); // Point config dir at the test dir so getConfigDir() returns a predictable path process.env.SENTRY_CONFIG_DIR = testDir; + resetEnvTokenHostForTesting(); }); afterEach(async () => { @@ -54,6 +56,7 @@ afterEach(async () => { delete process.env[key]; } } + resetEnvTokenHostForTesting(); await cleanupTestDir(testDir); }); @@ -260,13 +263,29 @@ describe("applySentryCliRcEnvShim", () => { expect(readEnv("SENTRY_TOKEN")).toBe("env-fallback-token"); }); - test("sets SENTRY_URL when neither SENTRY_HOST nor SENTRY_URL is set", async () => { + test("sets SENTRY_URL for SaaS rc url (no credential risk, no scoping check)", async () => { delete process.env.SENTRY_HOST; delete process.env.SENTRY_URL; - writeRcFile(testDir, "[defaults]\nurl = https://sentry.example.com\n"); + writeRcFile(testDir, "[defaults]\nurl = https://sentry.io\n"); await applySentryCliRcEnvShim(testDir); - expect(readEnv("SENTRY_URL")).toBe("https://sentry.example.com"); + expect(readEnv("SENTRY_URL")).toBe("https://sentry.io"); + }); + + test("throws CliError when non-SaaS rc url does not match active token's scoped host", async () => { + // Env-token defaults to SaaS (no SENTRY_HOST set at capture time). + // Any non-SaaS rc url is therefore a mismatch → CliError, env untouched. + // This closes the CVE where a committed .sentryclirc could redirect + // requests + token to an attacker host. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + writeRcFile(testDir, "[defaults]\nurl = https://evil.example.com\n"); + + await expect(applySentryCliRcEnvShim(testDir)).rejects.toThrow( + /does not match|sentry auth login --url/ + ); + expect(readEnv("SENTRY_URL")).toBeUndefined(); + expect(readEnv("SENTRY_HOST")).toBeUndefined(); }); test("does not set SENTRY_URL when SENTRY_HOST is set", async () => { diff --git a/test/lib/token-host.property.test.ts b/test/lib/token-host.property.test.ts new file mode 100644 index 000000000..713b99103 --- /dev/null +++ b/test/lib/token-host.property.test.ts @@ -0,0 +1,167 @@ +/** + * Property-based tests for host-scoping trust model. + * + * Verifies invariants that must hold for ANY input: + * - Trust is symmetric within the SaaS equivalence class. + * - Non-SaaS hosts only match exact origin (no subdomain suffix tricks). + * - Normalization is idempotent. + * - Requests with unparseable URLs are never trusted. + * + * Unit tests for specific edge cases live in test/lib/token-host.test.ts. + */ + +import { describe, expect, test } from "bun:test"; +import { + constantFrom, + assert as fcAssert, + property, + stringMatching, + tuple, +} from "fast-check"; +import { isHostTrusted, normalizeOrigin } from "../../src/lib/token-host.js"; +import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; + +// Arbitraries + +/** Host chars: lowercase alphanumerics + hyphen */ +const HOST_LABEL = stringMatching(/^[a-z0-9][a-z0-9-]{0,20}$/); + +/** Two-label lowercase host (e.g. "example.com") */ +const simpleHostArb = tuple(HOST_LABEL, HOST_LABEL).map( + ([a, b]) => `${a}.${b}` +); + +/** Non-SaaS host: never ends with `sentry.io` */ +const nonSaasHostArb = simpleHostArb.filter( + (h) => h !== "sentry.io" && !h.endsWith(".sentry.io") +); + +/** SaaS host: any subdomain of sentry.io (including bare `sentry.io`) */ +const saasSubdomainArb = HOST_LABEL.map((label) => `${label}.sentry.io`); + +/** Protocol */ +const protoArb = constantFrom("http", "https"); + +describe("property: normalizeOrigin is idempotent", () => { + test("normalize(normalize(x)) === normalize(x)", () => { + fcAssert( + property(protoArb, simpleHostArb, (proto, host) => { + const url = `${proto}://${host}/some/path?q=1`; + const once = normalizeOrigin(url); + if (once === undefined) { + return; + } + expect(normalizeOrigin(once)).toBe(once); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); + +describe("property: SaaS equivalence is reflexive + symmetric", () => { + test("any sentry.io subdomain matches any other sentry.io subdomain", () => { + fcAssert( + property(saasSubdomainArb, saasSubdomainArb, (a, b) => { + // Both a and b are https://