From ef73346886368a1106f19e1e74e54fb73b3696ea Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:45:25 +0000 Subject: [PATCH] feat(mcp-cloudflare): Share allowlisted cache entries across isolates through R2 Add the R2 tier of the hosted response cache behind MCP_RESPONSE_CACHE=tiered. TieredStore reads memory, then R2, for organization event details (without averageColumn) and project trace-item details (without debug). It writes to R2 only when the response carries Cache-Control: private with a positive max-age, and expires objects at the lesser of max-age and 5 minutes, checked on read. Writes run in waitUntil, and a 1-day bucket lifecycle rule collects garbage. Authorization values kept for Vary matching are stored as SHA-256 hashes, never as tokens. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- docs/cloudflare/architecture.md | 60 +++- docs/operations/monitoring.md | 10 + .../src/server/lib/mcp-handler.ts | 1 + .../src/server/lib/r2-cache-store.test.ts | 282 ++++++++++++++++++ .../src/server/lib/r2-cache-store.ts | 228 ++++++++++++++ .../src/server/lib/response-cache.test.ts | 28 +- .../src/server/lib/response-cache.ts | 70 ++++- packages/mcp-cloudflare/src/server/types.ts | 2 + packages/mcp-cloudflare/wrangler.canary.jsonc | 9 + packages/mcp-cloudflare/wrangler.jsonc | 9 + packages/mcp-cloudflare/wrangler.test.jsonc | 6 + .../api-client/response-cache/policy.test.ts | 8 + .../src/api-client/response-cache/policy.ts | 7 +- .../src/api-client/response-cache/store.ts | 5 +- 14 files changed, 694 insertions(+), 31 deletions(-) create mode 100644 packages/mcp-cloudflare/src/server/lib/r2-cache-store.test.ts create mode 100644 packages/mcp-cloudflare/src/server/lib/r2-cache-store.ts diff --git a/docs/cloudflare/architecture.md b/docs/cloudflare/architecture.md index 158ff7792..a777fd381 100644 --- a/docs/cloudflare/architecture.md +++ b/docs/cloudflare/architecture.md @@ -145,7 +145,9 @@ const result = streamText({ - **Pages**: Static asset hosting for React app - **KV Namespace**: OAuth token storage - **AI Binding**: Access to Cloudflare AI models (AutoRAG for docs search) -- **R2**: File storage (future) +- **R2**: The response cache's tier for allowlisted detail endpoints + (`MCP_RESPONSE_CACHE_BUCKET`, see + [Sentry API Response Cache](#sentry-api-response-cache)) ### Environment Variables @@ -172,11 +174,11 @@ The MCP handler can cache Sentry API GET responses through the shared response cache in `@sentry/toolkit-core/api-client/response-cache`. It is behind `MCP_RESPONSE_CACHE`: -| Value | Behavior | -| ------------------- | -------------------------------------------------------------------------- | -| unset, `off`, other | No caching. The default in every deployed environment. | -| `memory` | Caches every tier in the isolate's memory. | -| `tiered` | Memory only for now; the R2 tier for allowlisted detail endpoints follows. | +| Value | Behavior | +| ------------------- | ---------------------------------------------------------------------------- | +| unset, `off`, other | No caching. The default in every deployed environment. | +| `memory` | Caches every tier in the isolate's memory. | +| `tiered` | `memory`, plus R2 for allowlisted detail endpoints (memory only without R2). | How it works (`src/server/lib/response-cache.ts`): @@ -197,9 +199,53 @@ How it works (`src/server/lib/response-cache.ts`): has zstd; a deployed Worker is not confirmed yet. The `app.response_cache.write` metric records the encoding each write used. +#### R2 tier + +`tiered` shares a few detail responses across isolates through the +`MCP_RESPONSE_CACHE_BUCKET` R2 binding (`src/server/lib/r2-cache-store.ts`): + +- **Allowlist.** Only organization event details + (`/api/0/organizations/{org}/events/{project}:{event_id}/`) without + `averageColumn`, and project trace-item details + (`/api/0/projects/{org}/{project}/trace-items/{id}/`) without `debug`. + Every other URL stays in memory. +- **Server opt-in.** An allowlisted response is written to R2 only when it + carries `Cache-Control: private` with a positive `max-age` (and no + `no-store` or `no-cache`). Until Sentry sends that header + (getsentry/sentry#128828), R2 stores nothing. +- **Expiry.** Each object records an expiry, the lesser of `max-age` and 5 + minutes, and reads treat a later object as a miss. An R2 hit is copied + into memory for the time it has left. +- **Keys.** Objects are keyed `/`, with the + identity hash above, and a read also checks the identity stored in the + entry. Authorization values the policy keeps for `Vary` are stored as + SHA-256 hashes, never as tokens. +- **Writes** run in `ctx.waitUntil()`. Mutation invalidation only touches + memory, so a write under the same path, such as deleting the project, does + not clear R2. The 5-minute expiry bounds how long a deleted item can still + be served. +- **Garbage collection.** A 1-day bucket lifecycle rule deletes old objects. + It only frees space; reads enforce the expiry. + +The buckets are `sentry-mcp-response-cache` (production) and +`sentry-mcp-canary-response-cache` (canary). Deploys run with +`--experimental-auto-create=false`, so each bucket must exist before a +config that binds it deploys: + +```bash +pnpm exec wrangler r2 bucket create sentry-mcp-response-cache +pnpm exec wrangler r2 bucket lifecycle add sentry-mcp-response-cache \ + expire-1d --expire-days 1 --force +pnpm exec wrangler r2 bucket create sentry-mcp-canary-response-cache +pnpm exec wrangler r2 bucket lifecycle add sentry-mcp-canary-response-cache \ + expire-1d --expire-days 1 --force +``` + +The binding is inert while `MCP_RESPONSE_CACHE` is not `tiered`. + Turning it on in production is a separate decision: revoked access or deleted data can still be served from the cache until the entry expires (up to 5 -minutes for stable entries). Leave `MCP_RESPONSE_CACHE` unset in production +minutes for stable entries, including from R2). Leave `MCP_RESPONSE_CACHE` unset in production until that window has privacy and security sign-off. Setting it back to `off` restores uncached behavior. Metrics are listed in [Monitoring](../operations/monitoring.md#response-cache-metrics). diff --git a/docs/operations/monitoring.md b/docs/operations/monitoring.md index c8aa899c3..101f7410f 100644 --- a/docs/operations/monitoring.md +++ b/docs/operations/monitoring.md @@ -355,6 +355,11 @@ off. charges against its budget, after each write - `app.response_cache.memory.entries` - Gauge of entries in the isolate's store, after each write +- `app.response_cache.r2.lookup` - Count of R2 reads after a memory miss on an + allowlisted URL (`MCP_RESPONSE_CACHE=tiered` only), with + `app.response_cache.result`: `hit`, `miss`, or `expired` (past its recorded + expiry) +- `app.response_cache.r2.write` - Count of entries written to R2 The first eviction in an isolate also logs `Response cache reached its memory budget` at info level, with the budget, bytes, and entry count. @@ -365,6 +370,11 @@ Interpretation: the unfiltered sum - A steady eviction rate with `memory.bytes` at the budget means the budget is too small for the traffic an isolate serves +- R2 hit rate: `app.response_cache.r2.lookup` filtered to `result=hit`, over + the unfiltered sum. No `r2.write` at all means Sentry is not sending + `Cache-Control: private, max-age` on the allowlisted endpoints. +- R2 read and write failures log `Response cache R2 read failed` and + `Response cache R2 write failed` at warning level; a failed read is a miss. ### Traces Configuration diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts index e6580f88a..0cb577298 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts @@ -451,6 +451,7 @@ async function handleAuthenticatedMcpRequest( auth.kind === "oauth" && auth.grantId ? await getResponseCache({ mode: resolveResponseCacheMode(env.MCP_RESPONSE_CACHE), + bucket: env.MCP_RESPONSE_CACHE_BUCKET, userId: auth.userId, grantId: auth.grantId, sentryHost, diff --git a/packages/mcp-cloudflare/src/server/lib/r2-cache-store.test.ts b/packages/mcp-cloudflare/src/server/lib/r2-cache-store.test.ts new file mode 100644 index 000000000..ed806d43e --- /dev/null +++ b/packages/mcp-cloudflare/src/server/lib/r2-cache-store.test.ts @@ -0,0 +1,282 @@ +import { env } from "cloudflare:test"; +import { + type CacheEntry, + createMemoryStore, +} from "@sentry/toolkit-core/api-client/response-cache"; +import { createApiTransport } from "@sentry/toolkit-core/api-client/transport"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { sentryMetricsCount } = vi.hoisted(() => ({ + sentryMetricsCount: vi.fn(), +})); + +vi.mock("@sentry/cloudflare", async (importOriginal) => ({ + ...(await importOriginal()), + metrics: { count: sentryMetricsCount, gauge: vi.fn() }, +})); + +import type { Env } from "../types"; +import { + createTieredStore, + isR2Allowlisted, + R2_MAX_TTL_MS, + r2TtlMs, +} from "./r2-cache-store"; +import { createResponseCache } from "./response-cache"; + +const bucket = (env as unknown as Env).MCP_RESPONSE_CACHE_BUCKET as R2Bucket; + +const API = "https://us.sentry.io/api/0"; +const ORG_EVENT_URL = `${API}/organizations/org/events/backend:0123456789abcdef0123456789abcdef/`; +const TRACE_ITEM_URL = `${API}/projects/org/backend/trace-items/0123456789abcdef/`; +const ISSUES_URL = `${API}/organizations/org/issues/?query=is%3Aunresolved`; +const PRIVATE_5M = "private, max-age=300"; +const TOKEN = "sntryu_secret-upstream-token"; + +function entryWithCacheControl(cacheControl?: string): CacheEntry { + return { + url: ORG_EVENT_URL, + identity: "identity-a", + tier: "stable", + status: 200, + headers: cacheControl ? { "cache-control": cacheControl } : {}, + body: { bytes: new Uint8Array(), encoding: "identity", rawSize: 0 }, + policy: {}, + createdAt: Date.now(), + }; +} + +describe("isR2Allowlisted", () => { + it.each([ + [ORG_EVENT_URL, true], + [TRACE_ITEM_URL, true], + [`${TRACE_ITEM_URL}?trace_id=abc&item_type=spans`, true], + [`${ORG_EVENT_URL}?averageColumn=span.duration`, false], + [`${TRACE_ITEM_URL}?debug=1`, false], + [`${API}/projects/org/backend/events/0123456789abcdef/`, false], + [`${API}/organizations/org/issues/1/events/latest/`, false], + [ISSUES_URL, false], + ["not a url", false], + ])("%s → %s", (url, allowed) => { + expect(isR2Allowlisted(url)).toBe(allowed); + }); +}); + +describe("r2TtlMs", () => { + it.each([ + [undefined, 0], + ["max-age=300", 0], + ["public, max-age=300", 0], + ["private", 0], + ["private, max-age=0", 0], + ["private, no-cache, max-age=300", 0], + ["private, max-age=60", 60_000], + [PRIVATE_5M, R2_MAX_TTL_MS], + ["Private, Max-Age=3600", R2_MAX_TTL_MS], + ])("%j → %d", (cacheControl, ttl) => { + expect(r2TtlMs(entryWithCacheControl(cacheControl))).toBe(ttl); + }); +}); + +describe("createTieredStore", () => { + let upstream: ReturnType>; + let pending: Promise[] = []; + const waitUntil = (promise: Promise) => { + pending.push(promise); + }; + + beforeEach(async () => { + vi.useFakeTimers({ toFake: ["Date"] }); + sentryMetricsCount.mockReset(); + pending = []; + for (const object of (await bucket.list()).objects) { + await bucket.delete(object.key); + } + upstream = vi.fn(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + /** Answer every request with `body` and these response headers. */ + function respondWith(body: unknown, headers: Record = {}) { + upstream.mockImplementation(async () => + Response.json(body, { headers: { ...headers } }), + ); + } + + /** + * One Worker isolate: its own memory store over the shared bucket, and a + * transport that caches for `identity`. + */ + function isolate(identity: string) { + const memory = createMemoryStore(); + const { cache } = createResponseCache({ + store: memory, + allowlistedStore: createTieredStore({ + memory, + bucket, + identity, + waitUntil, + }), + identity, + apiBaseUrl: "https://sentry.io", + waitUntil, + }); + const transport = createApiTransport({ + retry: { + maxRetries: 0, + baseDelayMs: 0, + maxDelayMs: 0, + maxTotalDelayMs: 0, + }, + credentials: { current: () => ({ token: TOKEN, identity }) }, + cache, + fetch: upstream, + }); + return { + memory, + async get(url: string): Promise { + const response = await transport.request(url); + const body = await response.json(); + // Writes finish in waitUntil, as on a Worker. + while (pending.length > 0) { + await Promise.all(pending.splice(0)); + } + return body; + }, + }; + } + + async function objectKeys(): Promise { + return (await bucket.list()).objects.map((object) => object.key); + } + + it("serves an allowlisted response from another isolate", async () => { + respondWith({ id: "event-1" }, { "Cache-Control": PRIVATE_5M }); + + expect(await isolate("identity-a").get(ORG_EVENT_URL)).toEqual({ + id: "event-1", + }); + expect(upstream).toHaveBeenCalledTimes(1); + const keys = await objectKeys(); + expect(keys).toHaveLength(1); + expect(keys[0]).toMatch(/^identity-a\/[0-9a-f]{64}$/); + + const second = isolate("identity-a"); + expect(await second.get(ORG_EVENT_URL)).toEqual({ id: "event-1" }); + expect(upstream).toHaveBeenCalledTimes(1); + // The R2 hit was copied into the second isolate's memory. + expect(second.memory.stats().entries).toBe(1); + expect(sentryMetricsCount).toHaveBeenCalledWith( + "app.response_cache.r2.lookup", + 1, + { attributes: { "app.response_cache.result": "hit" } }, + ); + }); + + it("never serves one identity's R2 entry to another", async () => { + respondWith({ id: "event-1" }, { "Cache-Control": PRIVATE_5M }); + await isolate("identity-a").get(ORG_EVENT_URL); + + await isolate("identity-b").get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(2); + expect((await objectKeys()).sort()).toEqual([ + expect.stringMatching(/^identity-a\//), + expect.stringMatching(/^identity-b\//), + ]); + }); + + it.each([ + ["no server cache headers", {}], + ["a public response", { "Cache-Control": "public, max-age=300" }], + ["a zero max-age", { "Cache-Control": "private, max-age=0" }], + ])("does not write to R2 for %s", async (_, headers) => { + respondWith({ id: "event-1" }, headers); + const first = isolate("identity-a"); + await first.get(ORG_EVENT_URL); + + expect(await objectKeys()).toEqual([]); + expect(upstream).toHaveBeenCalledTimes(1); + await isolate("identity-a").get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(2); + }); + + it("keeps an allowlisted response without server headers in memory", async () => { + respondWith({ id: "event-1" }); + const first = isolate("identity-a"); + await first.get(ORG_EVENT_URL); + await first.get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(1); + expect(await objectKeys()).toEqual([]); + }); + + it.each([ + ["an issue list", ISSUES_URL], + ["an event with averageColumn", `${ORG_EVENT_URL}?averageColumn=x`], + ["a trace item with debug", `${TRACE_ITEM_URL}?debug=1`], + ])("keeps %s in memory only", async (_, url) => { + respondWith({ id: "1" }, { "Cache-Control": PRIVATE_5M }); + const first = isolate("identity-a"); + await first.get(url); + await first.get(url); + + expect(upstream).toHaveBeenCalledTimes(1); + expect(await objectKeys()).toEqual([]); + await isolate("identity-a").get(url); + expect(upstream).toHaveBeenCalledTimes(2); + }); + + it("persists an allowlisted trace item", async () => { + respondWith({ itemId: "1" }, { "Cache-Control": PRIVATE_5M }); + await isolate("identity-a").get(TRACE_ITEM_URL); + expect(await objectKeys()).toHaveLength(1); + }); + + it("enforces the 5-minute cap on read, whatever max-age says", async () => { + respondWith( + { id: "event-1" }, + { "Cache-Control": "private, max-age=3600" }, + ); + await isolate("identity-a").get(ORG_EVENT_URL); + + vi.setSystemTime(Date.now() + R2_MAX_TTL_MS - 1000); + await isolate("identity-a").get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(1); + + vi.setSystemTime(Date.now() + 2000); + await isolate("identity-a").get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(2); + expect(sentryMetricsCount).toHaveBeenCalledWith( + "app.response_cache.r2.lookup", + 1, + { attributes: { "app.response_cache.result": "expired" } }, + ); + }); + + it("expires at a server max-age shorter than 5 minutes", async () => { + respondWith({ id: "event-1" }, { "Cache-Control": "private, max-age=60" }); + await isolate("identity-a").get(ORG_EVENT_URL); + + vi.setSystemTime(Date.now() + 61_000); + await isolate("identity-a").get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(2); + }); + + it("never persists the raw upstream token", async () => { + respondWith( + { id: "event-1" }, + { "Cache-Control": PRIVATE_5M, Vary: "Authorization" }, + ); + await isolate("identity-a").get(ORG_EVENT_URL); + + const [key] = await objectKeys(); + const object = await bucket.get(key ?? ""); + const stored = new TextDecoder().decode(await object?.arrayBuffer()); + expect(stored).not.toContain(TOKEN); + // The hashed Authorization still matches for the same token. + await isolate("identity-a").get(ORG_EVENT_URL); + expect(upstream).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/mcp-cloudflare/src/server/lib/r2-cache-store.ts b/packages/mcp-cloudflare/src/server/lib/r2-cache-store.ts new file mode 100644 index 000000000..ad28d5bef --- /dev/null +++ b/packages/mcp-cloudflare/src/server/lib/r2-cache-store.ts @@ -0,0 +1,228 @@ +/** + * The R2 tier of the hosted response cache, used when + * `MCP_RESPONSE_CACHE=tiered`. It shares a few detail responses across + * isolates: only URLs on {@link isR2Allowlisted}, and only when the server + * opted in with `Cache-Control: private` and a positive `max-age`. Every + * other entry stays in the isolate's memory. + * + * Objects are keyed `/`, and each records its expiry, + * the lesser of the server's `max-age` and 5 minutes, which reads enforce. + * A 1-day bucket lifecycle rule only collects garbage. + */ + +import * as Sentry from "@sentry/cloudflare"; +import { logWarn } from "@sentry/mcp-core/telem/logging"; +import type { + CacheEntry, + CacheStore, +} from "@sentry/toolkit-core/api-client/response-cache"; + +/** No R2 entry outlives this, whatever `max-age` the server sends. */ +export const R2_MAX_TTL_MS = 5 * 60 * 1000; + +/** Largest stored (compressed) body written to R2, as in the memory store. */ +const MAX_R2_ENTRY_BYTES = 1024 * 1024; + +/** Bumped when the object layout changes; other versions read as misses. */ +const FORMAT_VERSION = "1"; + +const LOGGER_SCOPE = ["cloudflare", "response-cache"]; +const R2_LOOKUP_METRIC = "app.response_cache.r2.lookup"; +const R2_WRITE_METRIC = "app.response_cache.r2.write"; + +/** + * Detail endpoints stable enough to share across isolates, each with the + * query parameters that make a response live or privileged. + */ +const R2_URL_ALLOWLIST: readonly { + path: RegExp; + excludedParams: readonly string[]; +}[] = [ + { + // Organization event details; `averageColumn` reads live aggregates. + path: /^\/api\/0\/organizations\/[^/]+\/events\/[^/:]+:[^/]+\/$/, + excludedParams: ["averageColumn"], + }, + { + // Project trace-item details; `debug` adds internal fields. + path: /^\/api\/0\/projects\/[^/]+\/[^/]+\/trace-items\/[^/]+\/$/, + excludedParams: ["debug"], + }, +]; + +const encoder = new TextEncoder(); +const decoder = new TextDecoder(); + +/** Whether `url` may be persisted to R2 at all. */ +export function isR2Allowlisted(url: string): boolean { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return false; + } + return R2_URL_ALLOWLIST.some( + ({ path, excludedParams }) => + path.test(parsed.pathname) && + !excludedParams.some((name) => parsed.searchParams.has(name)), + ); +} + +/** + * How long R2 may keep `entry`: the server's `max-age` capped at + * {@link R2_MAX_TTL_MS} when it sent `Cache-Control: private`, otherwise 0. + */ +export function r2TtlMs(entry: CacheEntry): number { + const directives = (entry.headers["cache-control"] ?? "") + .toLowerCase() + .split(",") + .map((directive) => directive.trim()); + if ( + !directives.includes("private") || + directives.includes("no-store") || + directives.includes("no-cache") + ) { + return 0; + } + const maxAge = directives.find((directive) => + directive.startsWith("max-age="), + ); + const seconds = Number(maxAge?.slice("max-age=".length)); + if (!(Number.isInteger(seconds) && seconds > 0)) { + return 0; + } + return Math.min(seconds * 1000, R2_MAX_TTL_MS); +} + +/** An entry as one object: a 4-byte metadata length, its JSON, then the body. */ +function encodeEntry(entry: CacheEntry): Uint8Array { + const { body, ...rest } = entry; + const meta = encoder.encode( + JSON.stringify({ ...rest, body: { ...body, bytes: undefined } }), + ); + const bytes = new Uint8Array(4 + meta.byteLength + body.bytes.byteLength); + new DataView(bytes.buffer).setUint32(0, meta.byteLength); + bytes.set(meta, 4); + bytes.set(body.bytes, 4 + meta.byteLength); + return bytes; +} + +function decodeEntry(bytes: Uint8Array): CacheEntry { + const metaLength = new DataView( + bytes.buffer, + bytes.byteOffset, + bytes.byteLength, + ).getUint32(0); + const meta = JSON.parse( + decoder.decode(bytes.subarray(4, 4 + metaLength)), + ) as Omit & { + body: Omit; + }; + return { + ...meta, + body: { ...meta.body, bytes: bytes.slice(4 + metaLength) }, + }; +} + +function recordR2Lookup(result: "hit" | "miss" | "expired"): void { + Sentry.metrics.count(R2_LOOKUP_METRIC, 1, { + attributes: { "app.response_cache.result": result }, + }); +} + +type TieredStoreOptions = { + memory: CacheStore; + bucket: R2Bucket; + /** The identity every key belongs to; it prefixes each object key. */ + identity: string; + /** Keeps R2 writes alive after the response is returned. */ + waitUntil: (promise: Promise) => void; +}; + +/** + * Memory first, then R2, for one identity's allowlisted URLs. The caller + * picks this store only for {@link isR2Allowlisted} URLs. An R2 hit is copied + * into memory for the time it has left. Writes go to memory, and also to R2 + * when the server opted in. Invalidation only touches memory: allowlisted + * entries are never invalidated by writes, and they expire in 5 minutes. + */ +export function createTieredStore({ + memory, + bucket, + identity, + waitUntil, +}: TieredStoreOptions): CacheStore { + const objectKey = (key: string) => `${identity}/${key}`; + + async function readR2(key: string): Promise { + const object = await bucket.get(objectKey(key)); + if (!object) { + recordR2Lookup("miss"); + return; + } + const remainingMs = Number(object.customMetadata?.expiresAt) - Date.now(); + if (object.customMetadata?.v !== FORMAT_VERSION || !(remainingMs > 0)) { + recordR2Lookup("expired"); + return; + } + const entry = decodeEntry(new Uint8Array(await object.arrayBuffer())); + if (entry.identity !== identity) { + recordR2Lookup("miss"); + return; + } + await memory.set(key, entry, remainingMs); + recordR2Lookup("hit"); + return entry; + } + + return { + async get(key) { + const cached = await memory.get(key); + if (cached) { + return cached; + } + try { + return await readR2(key); + } catch (error) { + logWarn("Response cache R2 read failed", { + loggerScope: LOGGER_SCOPE, + extra: { error: String(error) }, + }); + return; + } + }, + + async set(key, entry, ttlMs) { + await memory.set(key, entry, ttlMs); + const persistMs = r2TtlMs(entry); + if ( + persistMs === 0 || + entry.identity !== identity || + entry.body.bytes.byteLength > MAX_R2_ENTRY_BYTES + ) { + return; + } + const expiresAt = entry.createdAt + persistMs; + waitUntil( + bucket + .put(objectKey(key), encodeEntry(entry), { + customMetadata: { + v: FORMAT_VERSION, + expiresAt: String(expiresAt), + }, + }) + .then(() => Sentry.metrics.count(R2_WRITE_METRIC, 1)) + .catch((error: unknown) => { + logWarn("Response cache R2 write failed", { + loggerScope: LOGGER_SCOPE, + extra: { error: String(error) }, + }); + }), + ); + }, + + async deleteMatching(entryIdentity, urlPrefix) { + await memory.deleteMatching(entryIdentity, urlPrefix); + }, + }; +} diff --git a/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts b/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts index 3bf6aedcf..e75a6bc4f 100644 --- a/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/response-cache.test.ts @@ -5,15 +5,19 @@ import { import { ENTRY_OVERHEAD_BYTES } from "@sentry/toolkit-core/api-client/response-cache/memory-store"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -const { logInfo, sentryMetricsCount, sentryMetricsGauge } = vi.hoisted(() => ({ - logInfo: vi.fn(), - sentryMetricsCount: vi.fn(), - sentryMetricsGauge: vi.fn(), -})); +const { logInfo, logWarn, sentryMetricsCount, sentryMetricsGauge } = vi.hoisted( + () => ({ + logInfo: vi.fn(), + logWarn: vi.fn(), + sentryMetricsCount: vi.fn(), + sentryMetricsGauge: vi.fn(), + }), +); vi.mock("@sentry/mcp-core/telem/logging", async (importOriginal) => ({ ...(await importOriginal()), logInfo, + logWarn, })); vi.mock("@sentry/cloudflare", async (importOriginal) => ({ @@ -263,6 +267,7 @@ describe("createMeteredMemoryStore", () => { describe("getResponseCache", () => { const options = { + bucket: undefined, userId: "user-1", grantId: "grant-1", sentryHost: "sentry.io", @@ -273,6 +278,19 @@ describe("getResponseCache", () => { expect(await getResponseCache({ ...options, mode: "off" })).toBeUndefined(); }); + it("warns once when tiered has no bucket", async () => { + logWarn.mockReset(); + expect( + await getResponseCache({ ...options, mode: "tiered" }), + ).toBeDefined(); + await getResponseCache({ ...options, mode: "tiered" }); + expect(logWarn).toHaveBeenCalledTimes(1); + expect(logWarn).toHaveBeenCalledWith( + expect.stringContaining("without an R2 bucket"), + expect.anything(), + ); + }); + it("scopes the cache to the grant", async () => { const cache = await getResponseCache({ ...options, mode: "memory" }); expect(cache?.identity).toBe( diff --git a/packages/mcp-cloudflare/src/server/lib/response-cache.ts b/packages/mcp-cloudflare/src/server/lib/response-cache.ts index 8037b3268..9f32f1cf4 100644 --- a/packages/mcp-cloudflare/src/server/lib/response-cache.ts +++ b/packages/mcp-cloudflare/src/server/lib/response-cache.ts @@ -3,10 +3,11 @@ * `off` (the default, and any unknown value), `memory`, or `tiered`. * * Entries live in one byte-budgeted memory store per isolate, shared by every - * request it serves. Each entry is keyed by an identity hashed from the MCP - * grant (the wrapper token's user and grant IDs), so one grant never reads - * another's responses. Requests without a grant, such as Sentry-Bearer - * direct tokens, are not cached. + * request it serves. `tiered` also shares a few allowlisted detail responses + * across isolates through R2 (see `r2-cache-store.ts`). Each entry is keyed + * by an identity hashed from the MCP grant (the wrapper token's user and + * grant IDs), so one grant never reads another's responses. Requests + * without a grant, such as Sentry-Bearer direct tokens, are not cached. */ import * as Sentry from "@sentry/cloudflare"; @@ -23,6 +24,7 @@ import { storeResponse, } from "@sentry/toolkit-core/api-client/response-cache"; import { createBodyCodec } from "@sentry/toolkit-core/response-cache/codec"; +import { createTieredStore, isR2Allowlisted } from "./r2-cache-store"; export type ResponseCacheMode = "off" | "memory" | "tiered"; @@ -41,10 +43,11 @@ const codec = createBodyCodec(); /** The isolate's store, shared by every request it serves. */ let memoryStore: MemoryStore | undefined; +let missingBucketLogged = false; /** * The cache mode for `MCP_RESPONSE_CACHE`. Anything other than `memory` or - * `tiered` is `off`. `tiered` caches in memory only until the R2 tier lands. + * `tiered` is `off`. */ export function resolveResponseCacheMode( value: string | undefined, @@ -62,15 +65,30 @@ export async function getGrantCacheIdentity( userId: string, grantId: string, ): Promise { - const digest = await crypto.subtle.digest( - "SHA-256", - encoder.encode(JSON.stringify(["mcp-grant", userId, grantId])), - ); + return await sha256Hex(JSON.stringify(["mcp-grant", userId, grantId])); +} + +async function sha256Hex(value: string): Promise { + const digest = await crypto.subtle.digest("SHA-256", encoder.encode(value)); return Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, "0"), ).join(""); } +/** + * Request headers for `Vary` matching with the Authorization value hashed. + * A response that varies on Authorization makes the cache policy keep the + * request's headers, and an entry must never hold a raw token. + */ +async function hashAuthorization( + requestHeaders: Record, +): Promise> { + const { authorization, ...rest } = requestHeaders; + return authorization + ? { ...rest, authorization: `sha256:${await sha256Hex(authorization)}` } + : rest; +} + function recordLookup(result: "hit" | "miss" | "stale"): void { Sentry.metrics.count(LOOKUP_METRIC, 1, { attributes: { "app.response_cache.result": result }, @@ -119,6 +137,8 @@ function isJson(response: Response): boolean { export type ResponseCacheOptions = { store: CacheStore; + /** Used instead of `store` for R2-allowlisted URLs. */ + allowlistedStore?: CacheStore; identity: string; /** The Sentry host's base URL, for cross-endpoint invalidation rules. */ apiBaseUrl: string; @@ -132,10 +152,14 @@ export type ResponseCacheOptions = { */ export function createResponseCache({ store, + allowlistedStore, identity, apiBaseUrl, waitUntil, }: ResponseCacheOptions): ResponseCache { + const storeFor = (url: string) => + allowlistedStore && isR2Allowlisted(url) ? allowlistedStore : store; + async function write( url: string, requestHeaders: Record, @@ -148,12 +172,12 @@ export function createResponseCache({ } const key = await buildCacheKey("GET", url, identity); await storeResponse({ - store, + store: storeFor(url), codec, key, identity, url, - requestHeaders, + requestHeaders: await hashAuthorization(requestHeaders), response, }); } finally { @@ -174,11 +198,11 @@ export function createResponseCache({ try { const key = await buildCacheKey("GET", url, identity); const lookup = await lookupResponse({ - store, + store: storeFor(url), codec, key, url, - requestHeaders, + requestHeaders: await hashAuthorization(requestHeaders), }); recordLookup(lookup?.kind ?? "miss"); return lookup; @@ -217,16 +241,18 @@ export function createResponseCache({ /** * The response cache for one MCP request under `mode`, or `undefined` when - * caching is off. + * caching is off. `tiered` without a `bucket` caches in memory only. */ export async function getResponseCache({ mode, + bucket, userId, grantId, sentryHost, waitUntil, }: { mode: ResponseCacheMode; + bucket: R2Bucket | undefined; userId: string; grantId: string; sentryHost: string; @@ -235,9 +261,21 @@ export async function getResponseCache({ if (mode === "off") { return; } + if (mode === "tiered" && !bucket && !missingBucketLogged) { + missingBucketLogged = true; + logWarn("MCP_RESPONSE_CACHE=tiered without an R2 bucket; using memory", { + loggerScope: LOGGER_SCOPE, + }); + } + const memory = (memoryStore ??= createMeteredMemoryStore()); + const identity = await getGrantCacheIdentity(userId, grantId); return createResponseCache({ - store: (memoryStore ??= createMeteredMemoryStore()), - identity: await getGrantCacheIdentity(userId, grantId), + store: memory, + allowlistedStore: + mode === "tiered" && bucket + ? createTieredStore({ memory, bucket, identity, waitUntil }) + : undefined, + identity, apiBaseUrl: `https://${sentryHost}`, waitUntil, }); diff --git a/packages/mcp-cloudflare/src/server/types.ts b/packages/mcp-cloudflare/src/server/types.ts index 0b4fe1446..44b8ff655 100644 --- a/packages/mcp-cloudflare/src/server/types.ts +++ b/packages/mcp-cloudflare/src/server/types.ts @@ -90,4 +90,6 @@ export interface Env { * docs/cloudflare/architecture.md before enabling it in production. */ MCP_RESPONSE_CACHE?: string; + /** R2 tier for `MCP_RESPONSE_CACHE=tiered`; without it, `tiered` is memory only. */ + MCP_RESPONSE_CACHE_BUCKET?: R2Bucket; } diff --git a/packages/mcp-cloudflare/wrangler.canary.jsonc b/packages/mcp-cloudflare/wrangler.canary.jsonc index e1b6405db..ed7ae4b95 100644 --- a/packages/mcp-cloudflare/wrangler.canary.jsonc +++ b/packages/mcp-cloudflare/wrangler.canary.jsonc @@ -72,6 +72,15 @@ "id": "963817ecb25c4761a2710d3aedc6478d", }, ], + // R2 tier of the Sentry API response cache, read only when + // MCP_RESPONSE_CACHE=tiered. The bucket needs a 1-day lifecycle rule; see + // docs/cloudflare/architecture.md. + "r2_buckets": [ + { + "binding": "MCP_RESPONSE_CACHE_BUCKET", + "bucket_name": "sentry-mcp-canary-response-cache", + }, + ], "ai": { "binding": "AI", }, diff --git a/packages/mcp-cloudflare/wrangler.jsonc b/packages/mcp-cloudflare/wrangler.jsonc index 6ea648773..bd6d9c726 100644 --- a/packages/mcp-cloudflare/wrangler.jsonc +++ b/packages/mcp-cloudflare/wrangler.jsonc @@ -77,6 +77,15 @@ "id": "01b5b45e9f3c4edaa9ef357b4f9949db", }, ], + // R2 tier of the Sentry API response cache, read only when + // MCP_RESPONSE_CACHE=tiered. The bucket needs a 1-day lifecycle rule; see + // docs/cloudflare/architecture.md. + "r2_buckets": [ + { + "binding": "MCP_RESPONSE_CACHE_BUCKET", + "bucket_name": "sentry-mcp-response-cache", + }, + ], "ai": { "binding": "AI", }, diff --git a/packages/mcp-cloudflare/wrangler.test.jsonc b/packages/mcp-cloudflare/wrangler.test.jsonc index b972909b7..6faf99c15 100644 --- a/packages/mcp-cloudflare/wrangler.test.jsonc +++ b/packages/mcp-cloudflare/wrangler.test.jsonc @@ -24,6 +24,12 @@ "id": "test-oauth-kv", }, ], + "r2_buckets": [ + { + "binding": "MCP_RESPONSE_CACHE_BUCKET", + "bucket_name": "test-response-cache", + }, + ], // Disable observability to avoid OTel issues in tests "observability": { "enabled": false, diff --git a/packages/toolkit-core/src/api-client/response-cache/policy.test.ts b/packages/toolkit-core/src/api-client/response-cache/policy.test.ts index d86cce0a8..98fd046a0 100644 --- a/packages/toolkit-core/src/api-client/response-cache/policy.test.ts +++ b/packages/toolkit-core/src/api-client/response-cache/policy.test.ts @@ -134,6 +134,14 @@ describe("storeResponse", () => { }); }); + it("keeps Cache-Control on the entry for the store", async () => { + const cache = createMemoryStore(); + await store(cache, json(BODY, { "cache-control": "private, max-age=300" })); + + const entry = (await cache.get(KEY)) as CacheEntry; + expect(entry.headers).toEqual({ "cache-control": "private, max-age=300" }); + }); + it.each([ ["Cache-Control: no-store", json(BODY, { "cache-control": "no-store" })], ["a non-2xx status", json({ detail: "Not found" }, {}, 404)], diff --git a/packages/toolkit-core/src/api-client/response-cache/policy.ts b/packages/toolkit-core/src/api-client/response-cache/policy.ts index 9ebd3ac5b..21758327d 100644 --- a/packages/toolkit-core/src/api-client/response-cache/policy.ts +++ b/packages/toolkit-core/src/api-client/response-cache/policy.ts @@ -29,8 +29,11 @@ const POLICY_OPTIONS: CachePolicy.Options = { cacheHeuristic: 0.1, }; -/** Response headers kept for callers, such as `Link` for pagination. */ -const PRESERVED_HEADERS = ["link"]; +/** + * Response headers kept on the entry: `Link` for pagination, and + * `Cache-Control` for stores whose persistence depends on it. + */ +const PRESERVED_HEADERS = ["link", "cache-control"]; const encoder = new TextEncoder(); diff --git a/packages/toolkit-core/src/api-client/response-cache/store.ts b/packages/toolkit-core/src/api-client/response-cache/store.ts index 57f704086..23a212c30 100644 --- a/packages/toolkit-core/src/api-client/response-cache/store.ts +++ b/packages/toolkit-core/src/api-client/response-cache/store.ts @@ -17,7 +17,10 @@ export type CacheEntry = { tier: TtlTier; /** HTTP status of the cached response. */ status: number; - /** Response headers kept for callers, such as `Link` for pagination. */ + /** + * Response headers kept for callers and stores, such as `Link` for + * pagination and `Cache-Control`. + */ headers: Readonly>; /** The response body, encoded by the codec. */ body: EncodedBody;