diff --git a/packages/cachekit/README.md b/packages/cachekit/README.md index 37f8d8a..13aa52b 100644 --- a/packages/cachekit/README.md +++ b/packages/cachekit/README.md @@ -116,7 +116,10 @@ const cache = createCache({ maxMemory: 50 * 1024 * 1024, // 50MB }, - // Optional: Client-side encryption + // Optional: Client-side encryption. Zero-knowledge covers every layer — + // L1 holds the same ciphertext L2 does, so an L1 hit costs a decrypt and + // AAD verify rather than being free, and no plaintext is resident in the + // heap between reads. Matches cachekit-py and cachekit-rs. encryption: { masterKey: process.env.CACHEKIT_MASTER_KEY!, // hex-encoded, 32+ bytes tenantId: 'tenant-123', // for multi-tenant key isolation diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts index 71bdbe5..b73346c 100644 --- a/packages/cachekit/src/cache-core.ts +++ b/packages/cachekit/src/cache-core.ts @@ -14,7 +14,11 @@ import type { MetricsCollector, MetricsConfig } from './metrics/prometheus.js'; import { logError } from './logger.js'; import { L1Cache } from './l1/lru-cache.js'; import { ReliabilityExecutor } from './reliability/executor.js'; -import { BackgroundRefreshManager, type WaitUntil } from './cache/background-refresh.js'; +import { + BackgroundRefreshManager, + type WaitUntil, + type L1Write, +} from './cache/background-refresh.js'; import { MessagePackSerializer } from './serialization/serializer.js'; import { generateKey, @@ -178,6 +182,12 @@ export class CacheImpl implements SecureCache { // l1Telemetry hook below). private readonly telemetry = { l1Hits: 0, l2Hits: 0, misses: 0 }; private readonly swrRequiresWaitUntil: boolean; + /** + * Mirrors ReliabilityExecutor's own default. Read directly so the L1 decrypt + * path can honour the same fail-open/fail-closed choice the L2 decrypt path + * gets for free by running inside the executor. + */ + private readonly degradationEnabled: boolean; private closed = false; /** One in-flight cold-miss resolution per cache key (single-flight, LAB-519). */ private readonly inflight = new Map>(); @@ -251,6 +261,7 @@ export class CacheImpl implements SecureCache { retry: options.reliability?.retry, degradation: options.reliability?.degradation, }); + this.degradationEnabled = options.reliability?.degradation !== false; // Initialize background refresh manager (SWR) this.backgroundRefresh = new BackgroundRefreshManager(); @@ -380,6 +391,108 @@ export class CacheImpl implements SecureCache { return this.getEntry(key, false); } + /** + * Does this entry ride the ByteStorage envelope? Interop entries never do — + * they are plain MessagePack with AAD compressed=False regardless of the + * cache-level `compression` option, so py/rs can read them byte-for-byte. + */ + private useEnvelope(interop: boolean): boolean { + return !interop && this.byteStorage !== null; + } + + /** + * What L1 should hold for an entry: the same ciphertext L2 holds when the + * cache is encrypted, the decoded value otherwise. + * + * Zero-knowledge is a property of every layer, not just the backend + * (LAB-238) — it is what cachekit-py does (its L1Cache stores bytes and + * decrypts at read time) and cachekit-rs with it. Holding post-decrypt + * plaintext here would put the entire L1 working set into any heap dump, + * core dump, or Node diagnostic report for the full TTL, and that plaintext + * would outlive the key zeroization in close(), since L1 entries are held + * independently of tenant keys. + * + * The bytes are copied when they are a window into a larger buffer: a Node + * Buffer from the backend is a view onto a shared 8 KiB pool slab, and + * retaining one for the entry's TTL pins the whole slab. cachekit-py guards + * the same edge by refusing memoryview/bytearray in L1Cache.put. This guards + * slab pinning only — it does NOT defend against a backend that mutates a + * buffer it already handed over. Every in-tree backend returns an owned, + * exact-size Uint8Array (redis, memcached, cachekitio, workers-kv, + * workers-cache-api) and file.ts's narrower header view lands in the copy + * branch; a third-party Backend that recycles buffers must copy on its side. + */ + private l1Payload(value: unknown, bytes: Uint8Array): unknown { + if (!this.encryption) return value; + return bytes.byteLength === bytes.buffer.byteLength ? bytes : new Uint8Array(bytes); + } + + /** + * Ciphertext (or envelope) bytes to the value they carry: decrypt + + * AAD-verify against the cache key, unpack the ByteStorage envelope, then + * deserialize. Shared by the L2 read and the L1 read so the two tiers can + * never drift into decoding the same entry differently — a real hazard once + * both paths handle AAD (protocol#12 freezes the v0x03 component set). + */ + private async decodeEntry(bytes: Uint8Array, key: string, interop: boolean): Promise { + const useEnvelope = this.useEnvelope(interop); + + let plaintext = bytes; + if (this.encryption) { + plaintext = await this.encryption.decrypt(plaintext, key, useEnvelope); + } + if (useEnvelope) { + plaintext = this.byteStorage!.unpack(plaintext); + } + return interop ? decodeInteropValue(plaintext) : this.serializer.decode(plaintext); + } + + /** + * Decode a value served from L1. For a secure cache that is a decrypt + + * AAD-verify against the cache key followed by the same unpack/deserialize + * the L2 path runs — an L1 hit is no longer free, which is the price of not + * keeping plaintext resident. For a plaintext cache it is a cast. + * + * Drops the L1 entry and returns null when a secure entry will not decrypt + * (rotated key, tampered heap, an entry written under a different envelope + * mode), so a poisoned L1 copy cannot outlive remediation of L2 — cachekit-py + * invalidates before applying its fail policy for the same reason. The result + * is wrapped because a cached value may legitimately BE null: without the + * wrapper a secure cache holding null would read as a decrypt failure on + * every hit, invalidating and re-fetching a perfectly good entry forever. + * + * Fail policy follows `reliability.degradation`, the same lever that governs + * an L2 decrypt failure (which happens inside run()): degradation on absorbs + * the failure and falls through to L2, degradation off rethrows so a tamper + * signal reaches the caller. Either way it is counted and logged, never + * silently swallowed. + */ + private async decodeL1Entry( + key: string, + stored: unknown, + interop: boolean + ): Promise<{ value: T } | null> { + if (!this.encryption) return { value: stored as T }; + + try { + if (!(stored instanceof Uint8Array)) { + throw new Error( + `L1 entry for a secure cache is not ciphertext bytes (got ${typeof stored})` + ); + } + return { value: await this.decodeEntry(stored, key, interop) }; + } catch (error) { + this.l1?.invalidateByKey(key); + this.recordFailure('l1_decrypt', error); + logError( + '[cachekit] L1 decrypt failed — entry dropped:', + error instanceof Error ? error.message : 'Unknown error' + ); + if (!this.degradationEnabled) throw error; + return null; + } + } + /** * L1 + L2 read. Interop entries (interop=true) are plain MessagePack — * no ByteStorage envelope and AAD compressed=False — regardless of the @@ -390,17 +503,20 @@ export class CacheImpl implements SecureCache { private async getEntry(key: string, interop: boolean, ttlSeconds?: number): Promise { this.ensureNotClosed(); - // Check L1 first + // Check L1 first. A secure cache holds ciphertext here, so the hit costs a + // decrypt + AAD verify; an entry that fails to verify is dropped and this + // falls through to L2 rather than serving or throwing. if (this.l1) { const l1Result = this.l1.get(key); if (l1Result !== null) { - this.recordHit('l1'); - return l1Result as T; + const decoded = await this.decodeL1Entry(key, l1Result, interop); + if (decoded !== null) { + this.recordHit('l1'); + return decoded.value; + } } } - const useEnvelope = !interop && this.byteStorage !== null; - // Fetch from L2 (backend) return this.run('get', null, async (): Promise => { const data = await this.backend.get(key); @@ -409,28 +525,18 @@ export class CacheImpl implements SecureCache { return null; } - // Decrypt if encryption enabled - let plaintext = data; - if (this.encryption) { - plaintext = await this.encryption.decrypt(data, key, useEnvelope); - } - - // Decompress with ByteStorage (after decryption) - if (useEnvelope) { - plaintext = this.byteStorage!.unpack(plaintext); - } - - // Deserialize - const value = interop - ? decodeInteropValue(plaintext) - : this.serializer.decode(plaintext); + // Decrypt, unpack, deserialize — the same sequence an L1 hit runs. + const value = await this.decodeEntry(data, key, interop); - // Populate L1. Interop keys are {namespace}:{operation}:{hash} — group - // under the user-facing namespace segment so namespace-level - // invalidation matches entries written through wrap(). + // Populate L1 with `data` — the bytes the backend returned, still + // encrypted — not the plaintext `value` decoded above. Interop keys are + // {namespace}:{operation}:{hash} — group under the user-facing namespace + // segment so namespace-level invalidation matches entries written + // through wrap(). if (this.l1) { const namespace = interop ? key.slice(0, key.indexOf(':')) : extractNamespace(key); - this.l1.set(key, value, (ttlSeconds ?? this.defaultTtl) * 1000, namespace); + const ttlMs = (ttlSeconds ?? this.defaultTtl) * 1000; + this.l1.set(key, this.l1Payload(value, data), ttlMs, namespace); this.publishL1Stats(); } @@ -440,13 +546,18 @@ export class CacheImpl implements SecureCache { } async set(key: string, value: T, options?: SetOptions): Promise { - return this.setEntry(key, value, options, false); + await this.setEntry(key, value, options, false); } /** * L1 + L2 write. Interop entries (interop=true) serialize to canonical * plain MessagePack — never the ByteStorage envelope — and encrypt with * AAD compressed=False, matching the Python and Rust SDKs byte-for-byte. + * + * Returns the payload L1 should hold for this entry, so the SWR refresh + * path (which defers its L1 write to completeRefresh's version check) can + * store the ciphertext this produced instead of the caller's plaintext. + * Null only when nothing storable was ever produced — see `l1Write`. */ private async setEntry( key: string, @@ -454,7 +565,7 @@ export class CacheImpl implements SecureCache { options: SetOptions | undefined, interop: boolean, updateL1 = true - ): Promise { + ): Promise { this.ensureNotClosed(); const ttl = options?.ttl ?? this.defaultTtl; @@ -465,7 +576,18 @@ export class CacheImpl implements SecureCache { } const namespace = options?.namespace ?? extractNamespace(key); - const useEnvelope = !interop && this.byteStorage !== null; + const useEnvelope = this.useEnvelope(interop); + + // What L1 should hold, captured as soon as it exists rather than returned + // from the closure — a degraded backend write (which `run` swallows) must + // still yield it. Otherwise an encrypted cache's SWR refresh gets nothing + // to store, and since a cancelled refresh leaves `expiresAt` untouched the + // entry stays stale and EVERY later read re-arms the refresh: a backend + // outage becomes an origin stampede on exactly the encrypted caches that + // matter (measured 15 origin calls over 15 reads, vs 1 for plaintext). + // Seeded with the plaintext value so a plaintext cache repopulates L1 on a + // degraded write exactly as it did before this change. + let l1Write: L1Write | null = this.encryption ? null : { l1: value }; // Interop model rejection is a deterministic caller error (spec: values // outside the data model MUST error) — it surfaces synchronously and @@ -475,7 +597,7 @@ export class CacheImpl implements SecureCache { // (existing degrade semantics unchanged). const interopSerialized = interop ? encodeInteropValue(value) : null; - return this.run('set', undefined, async (): Promise => { + await this.run('set', undefined, async (): Promise => { // Serialize const serialized = interopSerialized ?? this.serializer.encode(value); @@ -486,24 +608,28 @@ export class CacheImpl implements SecureCache { if (this.encryption) { data = await this.encryption.encrypt(data, key, useEnvelope); } + l1Write = { l1: this.l1Payload(value, data) }; // Store in backend await this.backend.set(key, data, ttl); - // Update L1 for direct writes. The SWR refresh path passes - // updateL1=false and writes L1 only through completeRefresh, whose - // version token discards the refresh if an explicit write or - // invalidation landed meanwhile — the guard is authoritative for L1 - // ONLY. The backend.set above is unconditional last-write-wins: an - // interleaved explicit set() survives in L1 but is overwritten in L2 - // by the refresh's value until the entry next expires or refreshes - // (a conditional L2 write would need CAS the Backend contract doesn't - // have). + // Update L1 for direct writes, with `data` (the ciphertext just written + // to the backend) rather than the caller's plaintext `value` whenever + // the cache is encrypted. The SWR refresh path passes updateL1=false and + // writes L1 only through completeRefresh, whose version token discards + // the refresh if an explicit write or invalidation landed meanwhile — + // the guard is authoritative for L1 ONLY. The backend.set above is + // unconditional last-write-wins: an interleaved explicit set() survives + // in L1 but is overwritten in L2 by the refresh's value until the entry + // next expires or refreshes (a conditional L2 write would need CAS the + // Backend contract doesn't have). if (updateL1 && this.l1) { - this.l1.set(key, value, ttl * 1000, namespace); + this.l1.set(key, l1Write!.l1, ttl * 1000, namespace); this.publishL1Stats(); } }); + + return l1Write; } async delete(key: string): Promise { @@ -526,10 +652,14 @@ export class CacheImpl implements SecureCache { async exists(key: string): Promise { this.ensureNotClosed(); - // Check L1 first + // Check L1 first. Presence alone is not an answer for a secure cache: L1 + // holds ciphertext, and after a key rotation every resident entry is + // undecryptable, so a bare `get() !== null` would report present for + // entries get() verifies, rejects and drops. Decode so exists() and get() + // cannot disagree, and so the poisoned entry is dropped here too. if (this.l1) { const l1Value = this.l1.get(key); - if (l1Value !== null) { + if (l1Value !== null && (await this.decodeL1Entry(key, l1Value, false)) !== null) { this.recordHit('l1'); return true; } @@ -668,35 +798,52 @@ export class CacheImpl implements SecureCache { if (this.swrRequiresWaitUntil && !waitUntil) { const l1Value = this.l1.get(cacheKey); if (l1Value !== null) { - this.recordHit('l1'); - return l1Value as TResult; + const decoded = await this.decodeL1Entry(cacheKey, l1Value, interop); + if (decoded !== null) { + this.recordHit('l1'); + return decoded.value; + } } } else { const swrResult = this.l1.getWithSwr(cacheKey); if (swrResult.value !== null) { - // Trigger background refresh if needed - if (swrResult.shouldRefresh) { - this.backgroundRefresh.scheduleRefresh( - cacheKey, - () => fn(...args), - { ttl: options.ttl, namespace: options.namespace }, - swrResult.versionToken, - this.l1, - async (key, value, opts) => { - await this.setEntry( - key, - value, - { ttl: opts.ttl, namespace: opts.namespace }, - interop, - false - ); - }, - waitUntil - ); + // Decode before scheduling: an entry that will not decrypt is + // already gone, so refreshing it would race the cold path that is + // about to recompute the same key. + let decoded: { value: TResult } | null = null; + try { + decoded = await this.decodeL1Entry(cacheKey, swrResult.value, interop); + } finally { + // Decode failed — by rethrow (degradation off) or by null + // (degradation on) — so the entry is dropped either way, and the + // marker getWithSwr took on our behalf must not outlive it: + // release the slot rather than strand it for the marker TTL. + if (decoded === null && swrResult.shouldRefresh) this.l1.cancelRefresh(cacheKey); + } + if (decoded !== null) { + // Trigger background refresh if needed + if (swrResult.shouldRefresh) { + this.backgroundRefresh.scheduleRefresh( + cacheKey, + () => fn(...args), + { ttl: options.ttl, namespace: options.namespace }, + swrResult.versionToken, + this.l1, + (key, value, opts) => + this.setEntry( + key, + value, + { ttl: opts.ttl, namespace: opts.namespace }, + interop, + false + ), + waitUntil + ); + } + this.recordHit('l1'); + return decoded.value; } - this.recordHit('l1'); - return swrResult.value as TResult; } } } diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts new file mode 100644 index 0000000..3b87db7 --- /dev/null +++ b/packages/cachekit/src/cache.encryption-l1.test.ts @@ -0,0 +1,312 @@ +/** + * LAB-238 regression: L1 must never hold plaintext for an encrypted cache. + * + * Zero-knowledge is a property of every layer. cachekit-py's L1Cache stores + * bytes (`get() -> tuple[bool, Optional[bytes]]`) and decrypts at read time; + * cachekit-rs stores ciphertext across all layers. TypeScript stored the + * post-decrypt value, so a heap dump, core dump, or Node diagnostic report + * yielded the whole L1 working set in plaintext for its full TTL — and that + * plaintext survived the key zeroization in close(), because L1 entries are + * held independently of tenant keys. + * + * These drive the REAL encryption path (AES-256-GCM via the Rust core, AAD + * bound to the cache key) through createCache end-to-end — no mocks on the + * crypto path — and cover all three L1 population sites: set, get, and the SWR + * background refresh. + */ + +import { createHash } from 'node:crypto'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { createCache } from './cache.js'; +import { generateKey } from './serialization/key-generator.js'; +import type { SecureCache } from './types/cache.js'; +import type { Backend } from './backends/types.js'; +import type { L1Cache } from './l1/lru-cache.js'; + +// Deterministic (reproducible runs, no random source) yet derived at runtime +// from a public fixture string — not a hardcoded key literal a secret scanner +// should ever match. No assertion depends on the key's value. +const MASTER_KEY = createHash('sha256').update('cachekit LAB-238 test fixture').digest('hex'); +const TENANT = 'lab-238'; + +/** Distinctive enough that a substring search over any dump is conclusive. */ +const LEAK_CANARY = 'ssn-000-00-0000-do-not-leak'; + +class InMemoryBackend implements Backend { + store = new Map(); + gets = 0; + + async get(key: string): Promise { + this.gets++; + return this.store.get(key) ?? null; + } + async set(key: string, value: Uint8Array): Promise { + this.store.set(key, value); + } + async delete(key: string): Promise { + return this.store.delete(key); + } + async exists(key: string): Promise { + return this.store.has(key); + } + async close(): Promise {} +} + +/** + * The L1 instance CacheImpl holds. Private on purpose — this suite asserts on + * exactly the bytes an attacker with a heap dump would find, which is the one + * thing a public API cannot show us. + */ +function l1Of(cache: SecureCache): L1Cache { + return (cache as unknown as { l1: L1Cache }).l1; +} + +/** What is resident in L1 for this key, as-is. */ +function l1Entry(cache: SecureCache, key: string): unknown { + return l1Of(cache).get(key); +} + +function expectCiphertext(stored: unknown, backendBytes: Uint8Array | undefined): void { + if (!(stored instanceof Uint8Array)) { + throw new Error(`L1 entry is not ciphertext bytes (got ${typeof stored})`); + } + // Byte-identical to what L2 holds: the same envelope, still sealed. + expect(stored).toEqual(backendBytes); + // And the canary is nowhere in the resident bytes. + expect(new TextDecoder().decode(stored)).not.toContain(LEAK_CANARY); +} + +describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => { + const caches: SecureCache[] = []; + + function makeCache(backend: Backend, encrypted = true): SecureCache { + const cache = createCache({ + backend, + defaultTtl: 3600, + l1: { enabled: true, maxEntries: 100 }, + ...(encrypted ? { encryption: { masterKey: MASTER_KEY, tenantId: TENANT } } : {}), + }); + caches.push(cache); + return cache; + } + + afterEach(async () => { + await Promise.all(caches.splice(0).map((c) => c.close())); + }); + + it('set() populates L1 with ciphertext, and the L1 hit decrypts it', async () => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend); + const key = 'users:1'; + + await cache.set(key, { ssn: LEAK_CANARY }); + + expectCiphertext(l1Entry(cache, key), backend.store.get(key)); + + // The hit is served from L1 (no second backend read) and still returns + // the plaintext value — decrypt happens on read, as in cachekit-py. + const before = backend.gets; + expect(await cache.get(key)).toEqual({ ssn: LEAK_CANARY }); + expect(backend.gets).toBe(before); + }); + + it('get() repopulates L1 with the ciphertext it read, not the value it decoded', async () => { + const backend = new InMemoryBackend(); + const writer = makeCache(backend); + await writer.set('users:2', { ssn: LEAK_CANARY }); + + // A second cache over the same L2: cold L1, warm backend. + const reader = makeCache(backend); + expect(l1Entry(reader, 'users:2')).toBeNull(); + + expect(await reader.get('users:2')).toEqual({ ssn: LEAK_CANARY }); + + expectCiphertext(l1Entry(reader, 'users:2'), backend.store.get('users:2')); + }); + + it('the SWR background refresh writes ciphertext to L1, not the factory result', async () => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend); + + let generation = 0; + const load = cache.wrap( + async (_id: number) => ({ ssn: LEAK_CANARY, generation: ++generation }), + // 2s TTL, read at 1.4s: 600ms remaining against a 0.5 threshold of + // 900-1100ms (±10% jitter), so the read is stale for every jitter draw + // and still has 600ms of headroom before the entry expires outright — + // a loaded CI box takes the stale path, not the cold path. + { namespace: 'users', ttl: 2 } + ); + + await load(3); + const key = generateKey('users', [3]); + expect(generation).toBe(1); + const firstCiphertext = l1Entry(cache, key); + + await new Promise((r) => setTimeout(r, 1400)); + + // Stale hit: served from L1, refresh scheduled in the background. + expect(await load(3)).toEqual({ ssn: LEAK_CANARY, generation: 1 }); + + // Wait for the refresh to actually land in L1 — the factory running is not + // enough, completeRefresh only runs once the L2 write has resolved. + await vi.waitFor(() => { + // Present AND changed: a bare not.toEqual would also pass on null once + // the TTL expires, misreporting a slow refresh as a format failure below. + const entry = l1Entry(cache, key); + expect(entry).toBeInstanceOf(Uint8Array); + expect(entry).not.toEqual(firstCiphertext); + }); + + expectCiphertext(l1Entry(cache, key), backend.store.get(key)); + expect(await load(3)).toEqual({ ssn: LEAK_CANARY, generation: 2 }); + }); + + it('rejects a substituted L1 entry — AAD is verified against the cache key', async () => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + try { + await cache.set('users:victim', { ssn: LEAK_CANARY }); + await cache.set('users:attacker', { ssn: 'attacker-controlled' }); + + // Swap in a valid ciphertext sealed under a DIFFERENT key. Without an + // AAD check on the L1 path this would be served as the victim's value. + const attackerBytes = backend.store.get('users:attacker')!; + l1Of(cache).set('users:victim', attackerBytes, 3600_000, 'users'); + + expect(await cache.get('users:victim')).toEqual({ ssn: LEAK_CANARY }); + // AES-GCM tag verification is what rejected it, not a decode error. + expect(consoleSpy).toHaveBeenCalledWith( + '[cachekit] L1 decrypt failed — entry dropped:', + expect.stringContaining('Authentication verification failed') + ); + // The poisoned entry was dropped, then refilled from L2 on the way back. + expectCiphertext(l1Entry(cache, 'users:victim'), backend.store.get('users:victim')); + } finally { + consoleSpy.mockRestore(); + } + }); + + it('serves a legitimately-null cached value from L1 without treating it as tampering', async () => { + // decodeL1Entry wraps its result because null is a valid cached value: an + // unwrapped null would read as a decrypt failure on every hit, so the + // entry would be invalidated and re-fetched from L2 (a billed miss on a + // metered backend) for its whole TTL. + const backend = new InMemoryBackend(); + const cache = makeCache(backend); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + try { + await cache.set('users:null', null); + const before = backend.gets; + + expect(await cache.get('users:null')).toBeNull(); + expect(await cache.get('users:null')).toBeNull(); + + // Served from L1 both times, and nothing was logged as a decrypt failure. + expect(backend.gets).toBe(before); + expect(consoleSpy).not.toHaveBeenCalled(); + expect(l1Entry(cache, 'users:null')).toBeInstanceOf(Uint8Array); + } finally { + consoleSpy.mockRestore(); + } + }); + + it('exists() verifies the L1 entry rather than trusting its presence', async () => { + // L1 holds ciphertext, so presence alone is not an answer: after a key + // rotation every resident entry is undecryptable and exists() would + // otherwise report present for entries get() rejects and drops. + const backend = new InMemoryBackend(); + const cache = makeCache(backend); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + try { + await cache.set('users:5', { ssn: LEAK_CANARY }); + expect(await cache.exists('users:5')).toBe(true); + + // Poison L1 with a ciphertext sealed under a different key, then drop + // the L2 copy: exists() must not report present off the bad L1 entry. + await cache.set('users:6', { other: true }); + l1Of(cache).set('users:5', backend.store.get('users:6')!, 3600_000, 'users'); + backend.store.delete('users:5'); + + expect(await cache.exists('users:5')).toBe(false); + expect(consoleSpy).toHaveBeenCalled(); + } finally { + consoleSpy.mockRestore(); + } + }); + + it('does not stampede the origin when the L2 write is degraded', async () => { + // A degraded write still hands back the ciphertext it produced, so the SWR + // refresh repopulates L1 and resets expiresAt. Without that, cancelRefresh + // would leave the entry permanently stale and every read would re-arm the + // refresh — turning a backend outage into an origin stampede on exactly + // the encrypted caches that carry PII. + const backend = new InMemoryBackend(); + let writesFail = false; + const flaky: Backend = { + ...backend, + get: (k) => backend.get(k), + set: async (k, v, t) => { + if (writesFail) throw new Error('backend down'); + return backend.set(k, v, t); + }, + delete: (k) => backend.delete(k), + exists: (k) => backend.exists(k), + close: () => backend.close(), + }; + const cache = makeCache(flaky); + + let originCalls = 0; + const load = cache.wrap(async (_id: number) => ({ ssn: LEAK_CANARY, n: ++originCalls }), { + namespace: 'users', + // 4s TTL read from 2.4s: stale against the 1.8-2.2s threshold (0.5 + // ratio, ±10% jitter) for every jitter draw, with 1.6s of lifetime left + // so the whole read loop below finishes well before the entry expires — + // an expiry mid-loop would send every later read down the cold path and + // count origin calls that have nothing to do with stampede control. + ttl: 4, + }); + + await load(7); + expect(originCalls).toBe(1); + + writesFail = true; + await new Promise((r) => setTimeout(r, 2400)); + + // Stale reads during the outage, spaced so each scheduled refresh + // settles before the next read — otherwise the in-flight marker masks the + // behaviour under test. A refresh that stored something resets the entry's + // freshness, so the origin is touched a couple of times; one that stored + // nothing and released its marker would be re-armed by every single read. + const READS = 10; + for (let i = 0; i < READS; i++) { + await load(7); + await new Promise((r) => setTimeout(r, 10)); + } + + await vi.waitFor(() => { + expect(originCalls).toBeGreaterThan(1); + }); + // The signal is "far fewer origin calls than reads", not an absolute count. + expect(originCalls).toBeLessThan(READS / 2); + }); + + it('leaves plaintext caches storing decoded values (unchanged, non-goal)', async () => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend, false); + + await cache.set('users:4', { ssn: LEAK_CANARY }); + expect(l1Entry(cache, 'users:4')).toEqual({ ssn: LEAK_CANARY }); + + const load = cache.wrap(async (_id: number) => ({ plain: true }), { + namespace: 'items', + ttl: 60, + }); + await load(9); + expect(l1Entry(cache, generateKey('items', [9]))).toEqual({ plain: true }); + }); +}); diff --git a/packages/cachekit/src/cache/background-refresh.test.ts b/packages/cachekit/src/cache/background-refresh.test.ts index cc6b95b..18e8b7d 100644 --- a/packages/cachekit/src/cache/background-refresh.test.ts +++ b/packages/cachekit/src/cache/background-refresh.test.ts @@ -2,6 +2,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { BackgroundRefreshManager } from './background-refresh.js'; import { L1Cache } from '../l1/lru-cache.js'; +// Pin the SWR jitter draw so tests are deterministic. 0.5 lands the +// jitter factor at exactly 1.0, so the refresh threshold is exactly +// originalTtl * swrThresholdRatio — no probabilistic window in any test here. +vi.mock('../utils/random.js', () => ({ + secureRandomFloat: () => 0.5, +})); + describe('BackgroundRefreshManager', () => { let manager: BackgroundRefreshManager; let l1Cache: L1Cache; @@ -11,11 +18,14 @@ describe('BackgroundRefreshManager', () => { beforeEach(() => { manager = new BackgroundRefreshManager(); l1Cache = new L1Cache({ maxEntries: 100 }); - persistToL2 = vi.fn().mockResolvedValue(undefined); + // The L2 write hands back what L1 should hold. For a plaintext cache that + // is the value itself; a secure cache would return its ciphertext here. + persistToL2 = vi.fn(async (_key: string, value: unknown) => ({ l1: value })); consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); }); afterEach(() => { + vi.useRealTimers(); consoleSpy.mockRestore(); manager.close(); l1Cache.clear(); @@ -109,34 +119,121 @@ describe('BackgroundRefreshManager', () => { expect(cached).toEqual({ data: 'refreshed' }); }); + it('stores what the L2 write returned, not the plaintext it computed (LAB-238)', async () => { + // A secure cache's persist callback returns the ciphertext it wrote to + // L2. That, and never the factory's plaintext result, is what lands in + // L1 — otherwise the SWR refresh re-poisons L1 on every revalidation. + const staleCiphertext = new Uint8Array([0xca, 0xfe, 0xba, 0xbe]); + const refreshedCiphertext = new Uint8Array([0xde, 0xad, 0xbe, 0xef]); + const secretPersist = vi.fn(async () => ({ l1: refreshedCiphertext })); + const computeFn = vi.fn().mockResolvedValue({ ssn: '000-00-0000' }); + + l1Cache.set('key1', staleCiphertext, 3600000, 'test'); + const { versionToken } = l1Cache.getWithSwr('key1'); + + let refreshDone!: Promise; + manager.scheduleRefresh( + 'key1', + computeFn, + { ttl: 3600, namespace: 'test' }, + versionToken, + l1Cache, + secretPersist, + (promise) => { + refreshDone = promise; + } + ); + // The refresh promise settles only after the L1 update landed. + await refreshDone; + + // Reference identity to the persist callback's ciphertext is the whole + // proof: neither the stale entry nor the factory's plaintext object can + // be this Uint8Array — so the refresh demonstrably replaced L1's entry. + expect(l1Cache.get('key1')).toBe(refreshedCiphertext); + }); + + it('holds the refresh marker when the write returns nothing storable', async () => { + // No ciphertext to show for the write, so L1 must keep the stale entry + // rather than fall back to the plaintext. Critically the marker is NOT + // released: cancelling frees it immediately while expiresAt stays put, + // so every later read would re-arm the refresh and hammer the origin. + // Letting it lapse via SWR_REFRESH_MARKER_TTL_MS throttles to 1/min/key. + const degraded = vi.fn(async () => null); + const computeFn = vi.fn().mockResolvedValue({ data: 'fresh' }); + + // Frozen clock + pinned jitter (vi.mock at top of file): with a 4s TTL + // the refresh threshold is exactly 2s, so a read at t=2.4s is + // deterministically stale and the entry deterministically alive until + // t=4s. Time only moves when this test says so — no wall-clock races. + vi.useFakeTimers(); + l1Cache.set('key1', { data: 'stale' }, 4000, 'test'); + vi.advanceTimersByTime(2400); + const stale = l1Cache.getWithSwr('key1'); + expect(stale.shouldRefresh).toBe(true); + + let refreshDone!: Promise; + manager.scheduleRefresh( + 'key1', + computeFn, + { ttl: 3600, namespace: 'test' }, + stale.versionToken, + l1Cache, + degraded, + (promise) => { + refreshDone = promise; + } + ); + // The refresh is pure microtasks (no timers) — awaiting its promise + // needs no timer advancement and settles after the degraded write. + await refreshDone; + expect(degraded).toHaveBeenCalled(); + + expect(l1Cache.get('key1')).toEqual({ data: 'stale' }); + expect(l1Cache.stats.refreshing).toBe(1); + // A second read must NOT schedule another refresh while the marker holds. + expect(l1Cache.getWithSwr('key1').shouldRefresh).toBe(false); + }); + it('should log error and cancel L1 refresh on failure', async () => { const error = new Error('Compute failed'); const computeFn = vi.fn().mockRejectedValue(error); - // Set initial value in L1 - l1Cache.set('key1', { data: 'old' }, 3600000, 'test'); - - // Trigger SWR to mark as refreshing - l1Cache.getWithSwr('key1'); - + // A fresh entry never takes a refresh marker, which would leave the + // cancel half of this test asserting nothing — so make the entry + // deterministically stale first (same frozen-clock setup as above). + vi.useFakeTimers(); + l1Cache.set('key1', { data: 'old' }, 4000, 'test'); + vi.advanceTimersByTime(2400); + const stale = l1Cache.getWithSwr('key1'); + expect(stale.shouldRefresh).toBe(true); + expect(l1Cache.stats.refreshing).toBe(1); + + let refreshDone!: Promise; manager.scheduleRefresh( 'key1', computeFn, { ttl: 3600, namespace: 'test' }, - 0, + stale.versionToken, l1Cache, - persistToL2 + persistToL2, + (promise) => { + refreshDone = promise; + } ); + await refreshDone; - await vi.waitFor(() => { - expect(consoleSpy).toHaveBeenCalledWith( - '[cachekit] Background refresh failed:', - 'Compute failed' - ); - }); + expect(consoleSpy).toHaveBeenCalledWith( + '[cachekit] Background refresh failed:', + 'Compute failed' + ); // persistToL2 should not be called on error expect(persistToL2).not.toHaveBeenCalled(); + + // The failed refresh released its marker via cancelRefresh — unlike + // the degraded-write case above, a compute error must allow the next + // read to re-arm a refresh immediately. + expect(l1Cache.stats.refreshing).toBe(0); }); it('should skip refresh if manager is closed', async () => { diff --git a/packages/cachekit/src/cache/background-refresh.ts b/packages/cachekit/src/cache/background-refresh.ts index db14c84..fc5b360 100644 --- a/packages/cachekit/src/cache/background-refresh.ts +++ b/packages/cachekit/src/cache/background-refresh.ts @@ -19,9 +19,28 @@ export interface RefreshOptions { } /** - * Callback for persisting refreshed values to L2 cache. + * What L1 should hold for an entry, as produced by the L2 write that just + * landed: the ciphertext for a secure cache, the plain value otherwise. + * + * Wrapped rather than returned bare because a cached value may legitimately be + * null or undefined — the wrapper is what distinguishes "store this" from + * "there is nothing to store" (LAB-238). + */ +export interface L1Write { + readonly l1: unknown; +} + +/** + * Callback for persisting refreshed values to L2 cache. Returns the payload + * L1 should hold for the entry, or null when the write produced nothing L1 may + * hold (a degraded write on a secure cache — no ciphertext to store, and the + * refresh must not fall back to the plaintext it computed). */ -export type PersistCallback = (key: string, value: T, options: RefreshOptions) => Promise; +export type PersistCallback = ( + key: string, + value: T, + options: RefreshOptions +) => Promise; /** * Manages stale-while-revalidate (SWR) background refreshes. @@ -76,13 +95,33 @@ export class BackgroundRefreshManager { const result = await computeFn(); - // Update L2 FIRST (before version check) - await persistToL2(key, result, options); + // Update L2 FIRST (before version check). The write hands back what L1 + // should hold — for a secure cache the ciphertext it just produced, + // never the plaintext `result` computed above (LAB-238). + const persisted = await persistToL2(key, result, options); // Then complete L1 refresh with version check // If version changed during L2 update, L1 update is rejected (stale data protection) if (l1Cache) { - l1Cache.completeRefresh(key, result, options.ttl * 1000, versionToken, options.namespace); + if (!persisted) { + // Nothing storable came back — the value could not even be + // encrypted (nonce exhausted, manager disposed). Deliberately do + // NOT cancelRefresh: a cancel frees the marker immediately while + // leaving the stale entry's expiresAt untouched, so the next read + // re-arms shouldRefresh and the one after that, hammering the + // origin for the rest of the TTL. Letting the marker lapse via + // SWR_REFRESH_MARKER_TTL_MS throttles retries to one per key per + // minute, and hasRefreshSlot sweeps expired markers so no refresh + // slot is wedged. + } else { + l1Cache.completeRefresh( + key, + persisted.l1, + options.ttl * 1000, + versionToken, + options.namespace + ); + } } } catch (error) { // Log error for observability diff --git a/packages/cachekit/src/l1/lru-cache.test.ts b/packages/cachekit/src/l1/lru-cache.test.ts index 4d2cd92..f06c5af 100644 --- a/packages/cachekit/src/l1/lru-cache.test.ts +++ b/packages/cachekit/src/l1/lru-cache.test.ts @@ -132,6 +132,25 @@ describe('L1Cache', () => { expect(smallCache.stats.entries).toBeLessThan(3); expect(smallCache.stats.memoryUsed).toBeLessThanOrEqual(400); }); + + it('sizes byte payloads by their buffer, not their JSON form (LAB-238)', () => { + // Secure caches store the L2 ciphertext here. JSON.stringify of a + // Uint8Array yields {"0":12,"1":34,…} — roughly 14x the real size — so + // measuring that way would evict most of L1 on the first entry. + const bytesCache = new L1Cache({ maxEntries: 100, maxMemory: 4096 }); + const ciphertext = new Uint8Array(256).fill(0xab); + + bytesCache.set('a', ciphertext, 10000, 'test'); + + expect(bytesCache.stats.memoryUsed).toBe(256); + + // 16 x 256B fits in a 4 KiB budget; under JSON sizing the second entry + // would already have evicted the first. + for (let i = 0; i < 15; i++) { + bytesCache.set(`k${i}`, new Uint8Array(256).fill(i), 10000, 'test'); + } + expect(bytesCache.stats.entries).toBe(16); + }); }); describe('SWR', () => { diff --git a/packages/cachekit/src/l1/lru-cache.ts b/packages/cachekit/src/l1/lru-cache.ts index d665288..c42d45b 100644 --- a/packages/cachekit/src/l1/lru-cache.ts +++ b/packages/cachekit/src/l1/lru-cache.ts @@ -412,6 +412,12 @@ export class L1Cache { } private estimateSize(value: unknown): number { + // Secure caches store the L2 ciphertext here (LAB-238), so the common + // entry is a Uint8Array. JSON.stringify turns one into {"0":12,"1":34,…} — + // roughly 14x its real size — which would blow the memory budget and evict + // most of L1 on the first encrypted entry. Count the buffer instead. + if (ArrayBuffer.isView(value)) return value.byteLength; + // Rough estimation - JSON stringify length as proxy // m2 Fix: Track visited objects to prevent infinite recursion on circular refs const visited = new WeakSet(); diff --git a/packages/cachekit/src/logger.test.ts b/packages/cachekit/src/logger.test.ts index 9d8eb93..8915ce5 100644 --- a/packages/cachekit/src/logger.test.ts +++ b/packages/cachekit/src/logger.test.ts @@ -65,7 +65,8 @@ describe('pluggable logger (LAB-517)', () => { { ttl: 60, namespace: 'ns' }, 0, null, - async () => {} + // PersistCallback contract: null = "nothing for L1 to hold". + async () => null ); await vi.waitFor(() => {