Skip to content

Commit d0024f0

Browse files
authored
feat(cache): scope response cache per active identity (#785) (#788)
## Summary Cached GET responses now live in per-identity namespaces so switching accounts never serves another user's data. `buildCacheKey(method, url)` internally mixes in a fingerprint of the active bearer identity, producing distinct cache files per account for the same URL. **Identity fingerprint** — a memoized 16-char MD5 hex of `kind|secret`, derived (in order) from: - **OAuth** — the stored `refresh_token`. Hourly access-token rotation keeps the same refresh_token, so the cache stays hot across refreshes. If the OAuth server DOES rotate the refresh_token on refresh (per spec, allowed), the fingerprint changes and the cache naturally re-populates under the new identity. - **Env token** — the raw `SENTRY_AUTH_TOKEN` / `SENTRY_TOKEN` value. Env tokens don't rotate, so hashing the token is equivalent to hashing a stable identity. - **No token** — sentinel `"<anon>"` (angle brackets can't appear in a real hex fingerprint, so no collision with a computed hash is possible). Memoization is reset on `setAuthToken` and `clearAuth`. OAuth refresh goes through `setAuthToken` so the fingerprint rotates correctly. MD5 is intentional: this is a cache namespace, not authentication — collisions are benign (two colliding identities would share cache space, like the anonymous case). CodeQL flagged this as a weak-crypto/insufficient-hash finding; dismissed as "won't fix" with context. This fixes the stale-data bugs reported in #785: - **#3** `auth whoami` no longer returns the previous user after `auth login` / env-token swap. - **#2** Post-mutation cache staleness is now bounded to a single identity rather than leaking across accounts. **Mutation hooks call `invalidateCachedResponsesMatching` for same-identity freshness:** `project create`, `project delete`, `issue resolve`, `issue unresolve`, `issue merge` all flush the relevant detail and list endpoints immediately. The sweep gates on an `identity` field persisted in every cache entry so one identity's mutation can't evict another identity's cache on a shared cache directory. **Cache-key / query-param correctness:** `getIssue` / `getIssueInOrg` pass `?collapse=stats&collapse=lifetime&...` so cached URLs carry query strings. Using exact-match `invalidateCachedResponse(<base url without params>)` would silently miss them. Mutation paths now use prefix-sweep (`invalidateCachedResponsesMatching`) on the trailing-slash base URL — which catches every cached variant regardless of query params. **URL-construction helper:** Introduced `buildApiUrl(regionUrl, ...segments)` in `api/infrastructure.ts` to replace repeated `${base}/api/0/path/${encodeURIComponent(slug)}/` template-string building. Per-segment encoding means a slug containing a literal `/` (rare but possible) now produces a correctly-encoded URL instead of a silently-broken one. Cache-invalidation helpers in `api/issues.ts` and `api/projects.ts` are contractually no-throw — a post-mutation DB/FS failure inside the helper can't turn a successful mutation into a user-visible error. **Circular import note:** `db/auth.ts` previously imported `clearResponseCache` statically; now that `response-cache.ts` imports `getIdentityFingerprint` from `db/auth.ts`, that's resolved via a dynamic `await import()` inside the (already-async) `clearAuth()`. ## Test plan - `bun test test/lib/db/auth.test.ts` — `getIdentityFingerprint` coverage: stability across OAuth access-token rotation, distinct fingerprints per env token / per OAuth session, `SENTRY_FORCE_ENV_TOKEN` switching the source, `kind`-prefix isolation (env/oauth with same secret produce distinct fingerprints), expired access-only token fall-through to env. - `bun test test/lib/response-cache.test.ts` — identity isolation (two different auth states never share a cache key for the same URL; `invalidateCachedResponsesMatching` leaves other identities' cached entries alone) plus a regression test for the query-param-variant case. - `bun test test/lib/api/infrastructure.test.ts` — `buildApiUrl` composition, trailing-slash normalization, reserved-char encoding (slash, space), and the zero-segments edge case. - `bun run typecheck`, `bun run lint` — clean. - Full unit suite: 5527 passing. Part of #785 (addresses items #2, #3, and half of #7 — the other half is fixed by a Sentry backend change).
1 parent 2359f96 commit d0024f0

11 files changed

Lines changed: 672 additions & 111 deletions

AGENTS.md

Lines changed: 54 additions & 48 deletions
Large diffs are not rendered by default.

src/lib/api/infrastructure.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,27 @@ export async function getOrgSdkConfig(orgSlug: string) {
165165
return getSdkConfig(regionUrl);
166166
}
167167

168+
/**
169+
* Build a full Sentry API URL from a region base and path segments.
170+
*
171+
* Each segment is passed through `encodeURIComponent`, so callers can
172+
* pass user-supplied slugs directly without worrying about slashes,
173+
* spaces, or other reserved characters. The `/api/0/` prefix and the
174+
* required trailing slash are added automatically.
175+
*
176+
* @example
177+
* buildApiUrl(regionUrl, "organizations", orgSlug, "projects")
178+
* // → `${regionUrl.replace(/\/$/,"")}/api/0/organizations/<org>/projects/`
179+
*/
180+
export function buildApiUrl(regionUrl: string, ...segments: string[]): string {
181+
const base = regionUrl.endsWith("/") ? regionUrl.slice(0, -1) : regionUrl;
182+
if (segments.length === 0) {
183+
return `${base}/api/0/`;
184+
}
185+
const path = segments.map(encodeURIComponent).join("/");
186+
return `${base}/api/0/${path}/`;
187+
}
188+
168189
/**
169190
* Extract the value of a named attribute from a Link header segment.
170191
* Parses `key="value"` pairs using string operations instead of regex

src/lib/api/issues.ts

Lines changed: 101 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@ import type { SentryIssue } from "../../types/index.js";
1212
import { applyCustomHeaders } from "../custom-headers.js";
1313
import { ApiError, ValidationError } from "../errors.js";
1414
import { resolveOrgRegion } from "../region.js";
15+
import { invalidateCachedResponsesMatching } from "../response-cache.js";
16+
import { getApiBaseUrl } from "../sentry-client.js";
1517

1618
import {
1719
API_MAX_PER_PAGE,
1820
apiRequest,
1921
apiRequestToRegion,
22+
buildApiUrl,
2023
getOrgSdkConfig,
2124
MAX_PAGINATION_PAGES,
2225
type PaginatedResponse,
@@ -554,6 +557,7 @@ export async function updateIssueStatus(
554557
if (options?.statusDetails) {
555558
body.statusDetails = options.statusDetails;
556559
}
560+
557561
if (options?.orgSlug) {
558562
// Region-aware org-scoped endpoint — preferred when org is known.
559563
const regionUrl = await resolveOrgRegion(options.orgSlug);
@@ -562,13 +566,22 @@ export async function updateIssueStatus(
562566
`/organizations/${encodeURIComponent(options.orgSlug)}/issues/${encodeURIComponent(issueId)}/`,
563567
{ method: "PUT", body }
564568
);
569+
await invalidateIssueCaches(regionUrl, options.orgSlug, issueId);
565570
return data;
566571
}
567-
// Legacy global endpoint — works without org but not region-aware.
568-
return apiRequest<SentryIssue>(`/issues/${encodeURIComponent(issueId)}/`, {
569-
method: "PUT",
570-
body,
571-
});
572+
573+
// Legacy global endpoint — works without org but not region-aware,
574+
// so we can only flush the legacy issue-detail URL. Region-scoped
575+
// lists age out via their TTL. Prefix-sweep (not exact-match)
576+
// because `getIssue` caches under `.../issues/{id}/?collapse=...`.
577+
const legacyData = await apiRequest<SentryIssue>(
578+
`/issues/${encodeURIComponent(issueId)}/`,
579+
{ method: "PUT", body }
580+
);
581+
await invalidateCachedResponsesMatching(
582+
buildApiUrl(getApiBaseUrl(), "issues", issueId)
583+
);
584+
return legacyData;
572585
}
573586

574587
/** Result of a successful issue-merge operation. */
@@ -612,6 +625,15 @@ export async function mergeIssues(
612625
method: "PUT",
613626
body: { merge: 1 },
614627
});
628+
// Flush detail caches for every affected ID (detail-only avoids
629+
// N+1 list scans) then sweep the org-wide list once.
630+
const affectedIds = data.merge.children.toSpliced(0, 0, data.merge.parent);
631+
await Promise.all(
632+
affectedIds.map((id) =>
633+
invalidateIssueDetailCaches(regionUrl, orgSlug, id)
634+
)
635+
);
636+
await invalidateOrgIssueList(regionUrl, orgSlug);
615637
return data.merge;
616638
} catch (error) {
617639
// The bulk-mutate endpoint returns 204 when no matching issues are
@@ -671,3 +693,77 @@ export async function getSharedIssue(
671693

672694
return (await response.json()) as { groupID: string };
673695
}
696+
697+
/**
698+
* Flush both the org-scoped and legacy detail endpoints for one issue,
699+
* including all `collapse` query-param variants (`getIssueInOrg` caches
700+
* responses under URLs like `.../issues/{id}/?collapse=stats&...` so
701+
* exact-match invalidation would miss them). Does NOT sweep the
702+
* org-wide list — callers must call {@link invalidateOrgIssueList}
703+
* once per operation. Never throws.
704+
*/
705+
async function invalidateIssueDetailCaches(
706+
regionUrl: string,
707+
orgSlug: string,
708+
issueId: string
709+
): Promise<void> {
710+
try {
711+
await Promise.all([
712+
invalidateCachedResponsesMatching(
713+
buildApiUrl(regionUrl, "organizations", orgSlug, "issues", issueId)
714+
),
715+
// Legacy `/api/0/issues/{id}/` is stored under the non-region base
716+
// (see `apiRequest` → `getApiBaseUrl`), NOT the org's region URL.
717+
invalidateCachedResponsesMatching(
718+
buildApiUrl(getApiBaseUrl(), "issues", issueId)
719+
),
720+
]);
721+
} catch {
722+
/* best-effort: mutation already succeeded upstream */
723+
}
724+
}
725+
726+
/**
727+
* Flush detail + list caches for one issue. Use for single-issue
728+
* mutations (resolve, unresolve); batch mutations should use the
729+
* detail-only helper plus one final {@link invalidateOrgIssueList}.
730+
*
731+
* Minor redundancy: the org-scoped half of
732+
* {@link invalidateIssueDetailCaches} is already a prefix of the
733+
* {@link invalidateOrgIssueList} sweep. Accepted because the helpers
734+
* are each also used solo elsewhere, and the extra directory walk is
735+
* negligible.
736+
*
737+
* Never throws.
738+
*/
739+
async function invalidateIssueCaches(
740+
regionUrl: string,
741+
orgSlug: string,
742+
issueId: string
743+
): Promise<void> {
744+
try {
745+
await Promise.all([
746+
invalidateIssueDetailCaches(regionUrl, orgSlug, issueId),
747+
invalidateOrgIssueList(regionUrl, orgSlug),
748+
]);
749+
} catch {
750+
/* best-effort: mutation already succeeded upstream */
751+
}
752+
}
753+
754+
/**
755+
* Sweep every paginated variant of the org's issue-list endpoint.
756+
* Never throws.
757+
*/
758+
async function invalidateOrgIssueList(
759+
regionUrl: string,
760+
orgSlug: string
761+
): Promise<void> {
762+
try {
763+
await invalidateCachedResponsesMatching(
764+
buildApiUrl(regionUrl, "organizations", orgSlug, "issues")
765+
);
766+
} catch {
767+
/* best-effort: mutation already succeeded upstream */
768+
}
769+
}

src/lib/api/projects.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,19 @@ import { cacheProjectsForOrg } from "../db/project-cache.js";
2525
import { getCachedOrganizations } from "../db/regions.js";
2626
import { type AuthGuardSuccess, withAuthGuard } from "../errors.js";
2727
import { logger } from "../logger.js";
28+
import { resolveOrgRegion } from "../region.js";
29+
import {
30+
invalidateCachedResponse,
31+
invalidateCachedResponsesMatching,
32+
} from "../response-cache.js";
2833
import { getApiBaseUrl } from "../sentry-client.js";
2934
import { buildProjectUrl } from "../sentry-urls.js";
3035
import { isAllDigits } from "../utils.js";
3136

3237
import {
3338
API_MAX_PER_PAGE,
3439
apiRequestToRegion,
40+
buildApiUrl,
3541
getOrgSdkConfig,
3642
MAX_PAGINATION_PAGES,
3743
ORG_FANOUT_CONCURRENCY,
@@ -166,6 +172,7 @@ export async function createProject(
166172
body,
167173
});
168174
const data = unwrapResult(result, "Failed to create project");
175+
await invalidateOrgProjectCache(orgSlug);
169176
return data as unknown as SentryProject;
170177
}
171178

@@ -215,6 +222,46 @@ export async function deleteProject(
215222
},
216223
});
217224
unwrapResult(result, "Failed to delete project");
225+
await invalidateProjectCaches(orgSlug, projectSlug);
226+
}
227+
228+
/**
229+
* Flush the project-detail GET and the org-wide project list so
230+
* follow-up `project list` / `project view` reads don't see the
231+
* deleted project. Never throws.
232+
*/
233+
async function invalidateProjectCaches(
234+
orgSlug: string,
235+
projectSlug: string
236+
): Promise<void> {
237+
try {
238+
const regionUrl = await resolveOrgRegion(orgSlug);
239+
await Promise.all([
240+
invalidateCachedResponse(
241+
buildApiUrl(regionUrl, "projects", orgSlug, projectSlug)
242+
),
243+
invalidateCachedResponsesMatching(
244+
buildApiUrl(regionUrl, "organizations", orgSlug, "projects")
245+
),
246+
]);
247+
} catch {
248+
/* best-effort: mutation already succeeded */
249+
}
250+
}
251+
252+
/**
253+
* Sweep every paginated variant of the org's project-list endpoint.
254+
* Used by `project create`. Never throws.
255+
*/
256+
async function invalidateOrgProjectCache(orgSlug: string): Promise<void> {
257+
try {
258+
const regionUrl = await resolveOrgRegion(orgSlug);
259+
await invalidateCachedResponsesMatching(
260+
buildApiUrl(regionUrl, "organizations", orgSlug, "projects")
261+
);
262+
} catch {
263+
/* best-effort: mutation already succeeded */
264+
}
218265
}
219266

220267
/** Result of searching for projects by slug across all organizations. */

src/lib/db/auth.ts

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
* Authentication credential storage (single-row table pattern).
33
*/
44

5+
import { createHash } from "node:crypto";
56
import { getEnv } from "../env.js";
6-
import { clearResponseCache } from "../response-cache.js";
77
import { withDbSpan } from "../telemetry.js";
88
import { getDatabase } from "./index.js";
99
import { clearAllIssueOrgCache } from "./issue-org-cache.js";
@@ -221,6 +221,9 @@ export function setAuthToken(
221221
["id"]
222222
);
223223
});
224+
// Auth row changed — drop the memoized fingerprint so the next
225+
// `getIdentityFingerprint()` call reflects the new row.
226+
resetIdentityFingerprintCache();
224227
}
225228

226229
export async function clearAuth(): Promise<void> {
@@ -235,10 +238,11 @@ export async function clearAuth(): Promise<void> {
235238
db.query("DELETE FROM pagination_cursors").run();
236239
clearAllIssueOrgCache();
237240
});
241+
resetIdentityFingerprintCache();
238242

239-
// Clear cached API responses — they are tied to the current user's permissions.
240-
// Awaited so cache is fully removed before the process exits.
243+
// Dynamic import avoids the auth→response-cache→auth cycle.
241244
try {
245+
const { clearResponseCache } = await import("../response-cache.js");
242246
await clearResponseCache();
243247
} catch {
244248
// Non-fatal: cache directory may not exist yet
@@ -250,6 +254,88 @@ export function isAuthenticated(): boolean {
250254
return !!token;
251255
}
252256

257+
/** Fingerprint returned when no token is present (logged out, no env var). */
258+
export const ANON_IDENTITY = "<anon>";
259+
260+
/** Memoized fingerprint. Identity doesn't change within a single CLI run. */
261+
let cachedFingerprint: string | undefined;
262+
263+
/**
264+
* Opaque fingerprint of the active bearer identity, used to namespace
265+
* response-cache keys so entries never leak across accounts. Mirrors
266+
* `getAuthConfig` precedence: forced env token > stored OAuth
267+
* (refresh_token preferred for stability across access-token rotation,
268+
* falling through expired access-only rows) > env token > anonymous.
269+
*
270+
* Memoized. Reset on every mutation point (`setAuthToken`,
271+
* `clearAuth`), so both the common case (OAuth access-token refresh
272+
* with a stable refresh_token — fingerprint unchanged in practice)
273+
* and the uncommon case (server-rotated refresh_token — fingerprint
274+
* changes, cache naturally re-populates under the new identity) work
275+
* correctly. Tests that mutate auth state between cases call
276+
* {@link resetIdentityFingerprintCache}.
277+
*/
278+
export function getIdentityFingerprint(): string {
279+
if (cachedFingerprint === undefined) {
280+
cachedFingerprint = computeIdentityFingerprint();
281+
}
282+
return cachedFingerprint;
283+
}
284+
285+
/** Reset the memoized fingerprint. Tests only — call between auth-state mutations. */
286+
export function resetIdentityFingerprintCache(): void {
287+
cachedFingerprint = undefined;
288+
}
289+
290+
function computeIdentityFingerprint(): string {
291+
// Forced env-token: matches what `refreshToken()` will actually send.
292+
if (getEnv().SENTRY_FORCE_ENV_TOKEN?.trim()) {
293+
const envToken = getRawEnvToken();
294+
if (envToken) {
295+
return hashIdentity("env", envToken);
296+
}
297+
}
298+
299+
const row = withDbSpan("getIdentityFingerprint", () => {
300+
const db = getDatabase();
301+
return db
302+
.query("SELECT token, refresh_token, expires_at FROM auth WHERE id = 1")
303+
.get() as
304+
| {
305+
token: string | null;
306+
refresh_token: string | null;
307+
expires_at: number | null;
308+
}
309+
| undefined;
310+
});
311+
// Prefer refresh_token: stable across access-token rotation.
312+
if (row?.refresh_token) {
313+
return hashIdentity("oauth", row.refresh_token);
314+
}
315+
// Access-only row: skip if expired (mirrors getAuthConfig).
316+
if (row?.token && !(row.expires_at && Date.now() > row.expires_at)) {
317+
return hashIdentity("oauth-access", row.token);
318+
}
319+
320+
const envToken = getRawEnvToken();
321+
if (envToken) {
322+
return hashIdentity("env", envToken);
323+
}
324+
return ANON_IDENTITY;
325+
}
326+
327+
/**
328+
* 16-char MD5 hex of `kind|secret`. Not used for auth — just a cheap
329+
* cache namespace. Collisions are benign (identities would share a
330+
* cache slot, same as the anonymous case).
331+
*/
332+
function hashIdentity(kind: string, secret: string): string {
333+
return createHash("md5")
334+
.update(`${kind}|${secret}`)
335+
.digest("hex")
336+
.slice(0, 16);
337+
}
338+
253339
/**
254340
* Check if usable OAuth credentials are stored in the database.
255341
*

src/lib/db/migration.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ export function migrateFromJson(db: Database): void {
141141

142142
try {
143143
if (oldConfig.auth?.token) {
144+
// Direct write (not via setAuthToken) — safe only because migration
145+
// runs during DB bootstrap, before getIdentityFingerprint() memoizes.
144146
db.query(`
145147
INSERT OR REPLACE INTO auth (id, token, refresh_token, expires_at, issued_at, updated_at)
146148
VALUES (1, ?, ?, ?, ?, ?)

0 commit comments

Comments
 (0)