From bea04da9871a62ea379aaa8c87f2e6ce4a2eab42 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 02:10:05 +0000 Subject: [PATCH 1/7] perf(cachekit): decrypt an encrypted L1 hit on Node.js without awaiting On Node.js the NAPI decrypt is synchronous, yet an encrypted L1 hit awaited four promises around it: the decode, the L1 decode, decrypt() and its ensureInitialized(). Once the keys are derived the manager's new decryptNow() decrypts in the call, decodeEntry and decodeL1Entry return a value or a Promise, and the get(), exists() and wrap() hit sites await only a Promise. Workers, where WebCrypto decrypts asynchronously, and a manager not yet initialised keep the await. The first-hit copy and trust, drop-on-failure and the degradation policy are unchanged. Adds the secure-get-l1-hit bench workload. --- packages/cachekit/bench/ir-workload.mjs | 22 ++ packages/cachekit/src/cache-core.ts | 192 +++++++++++------ .../cachekit/src/cache.encryption-l1.test.ts | 195 +++++++++++++++++- packages/cachekit/src/cache.interop.test.ts | 6 +- .../cachekit/src/cache.l1-trusted.test.ts | 17 +- packages/cachekit/src/cache.test.ts | 12 +- .../src/encryption/manager-core.test.ts | 46 +++++ .../cachekit/src/encryption/manager-core.ts | 49 ++++- packages/cachekit/src/encryption/manager.ts | 4 +- 9 files changed, 464 insertions(+), 79 deletions(-) diff --git a/packages/cachekit/bench/ir-workload.mjs b/packages/cachekit/bench/ir-workload.mjs index 06f1f5b3..a94efb4e 100644 --- a/packages/cachekit/bench/ir-workload.mjs +++ b/packages/cachekit/bench/ir-workload.mjs @@ -465,6 +465,28 @@ export const WORKLOADS = { }; }, }, + // get() served from an encrypted L1, which holds ciphertext: the lookup, + // then decrypt, unpack and decode of 100 B of msgpack, with no key + // generation, unlike secure-wrap-l1-hit-100b. Setup checks the get is served + // by L1. 4,000 warm-up ops, as for get-l1-hit and secure-wrap-l1-hit-100b. + 'secure-get-l1-hit': { + n: 20_000, + warmup: 4_000, + setup: async () => { + const { createCache } = await load('index.js'); + const backend = new MemoryBackend(); + const cache = createCache({ backend, encryption: { masterKey: MASTER_KEY }, metrics: false }); + let reads = 0; + const read = backend.get.bind(backend); + backend.get = (key) => (reads++, read(key)); + await cache.set(KEY, rowsOf(3), { ttl: 3600 }); + sink += (await cache.get(KEY)).rows.length; + if (reads !== 0) throw new Error('the get read L2: L1 did not keep the entry'); + return async () => { + sink += (await cache.get(KEY)).rows.length; + }; + }, + }, // wrap() with an object among the arguments, which the key memo does not // keep: keygen + L1 lookup, where a plaintext L1 holds the decoded value 'wrap-l1-hit': { diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts index ae0dd59d..c8b87113 100644 --- a/packages/cachekit/src/cache-core.ts +++ b/packages/cachekit/src/cache-core.ts @@ -116,6 +116,19 @@ type DeferredWrite = */ type L1Mode = 'fill' | 'skip' | 'refresh'; +/** A stored entry decoded (see decodeEntry). */ +interface DecodedEntry { + value: T; + serializedSize: number; + count: ObjectCount; +} + +/** + * An L1 entry decodeL1Entry served: wrapped, because a cached value may be + * null; null itself means the entry would not decode and was dropped. + */ +type L1Read = { value: T } | null; + /** A key's idle-key ops in flight (see keysInFlight). */ interface InFlight { count: number; @@ -262,7 +275,12 @@ export interface ByteStorageLike { /** Encryption surface CacheImpl drives (see EncryptionManagerCore). */ export interface EncryptionLike { encrypt(data: Uint8Array, cacheKey: string, compressed?: boolean): Promise; - decrypt(ciphertext: Uint8Array, cacheKey: string, compressed?: boolean): Promise; + /** The plaintext when the decrypt need not suspend (NAPI), else a Promise of it. */ + decryptNow( + ciphertext: Uint8Array, + cacheKey: string, + compressed?: boolean + ): Uint8Array | Promise; /** Throws ConfigurationError for a key encrypt/decrypt would reject for size. */ validateKey(cacheKey: string, compressed?: boolean): void; dispose(): void; @@ -1017,39 +1035,54 @@ export class CacheImpl implements SecureCache { * * `trusted` drops the depth pre-scan, and in auto mode validateDepth. Only * decodeL1Entry passes it, for a copy marked in trustedL1. + * + * Returns a Promise only when the decrypt suspends (see + * EncryptionLike.decryptNow): on Node.js an entry decodes in this call, so + * an L1 hit awaits nothing. A failure throws, or rejects that Promise. */ - private async decodeEntry( + private decodeEntry( bytes: Uint8Array, key: string, interop: boolean, trusted = false - ): Promise<{ value: T; serializedSize: number; count: ObjectCount }> { + ): DecodedEntry | Promise> { const useEnvelope = this.useEnvelope(interop); - - let plaintext = bytes; - if (this.encryption) { - // Refuse ciphertext longer than any plaintext this cache would decode - // before the codec copies it in: junk of any length otherwise reaches - // decrypt, which allocates for all of it before the tag check fails. - // The AAD binds useEnvelope, so a compression-off entry that decrypts - // is a plain serialized value, which decode() caps at maxDecodedSize. - const maxPlaintext = interop - ? DEFAULT_MAX_DECODED_SIZE // decodeInteropValue's fixed input cap - : useEnvelope - ? maxEnvelopeInputSize(this.serializerConfig.maxDecodedSize) - : this.serializerConfig.maxDecodedSize; - if (plaintext.length > maxPlaintext + AEAD_OVERHEAD_BYTES) { - throw new ValueTooLargeError( - `Ciphertext size ${plaintext.length} exceeds max ${maxPlaintext + AEAD_OVERHEAD_BYTES}` - ); - } - // The AAD binds useEnvelope (frozen v0x03 set, protocol#12), so an entry - // written under the other compression setting fails here — a loud, - // counted decrypt failure, never a silent wrong decode. We deliberately - // do NOT retry with the flipped flag: that would reintroduce exactly the - // envelope-mode ambiguity the AAD binding exists to rule out. - plaintext = await this.encryption.decrypt(plaintext, this.aadKey(key), useEnvelope); + if (!this.encryption) return this.decodePlaintext(bytes, key, interop, useEnvelope, trusted); + + // Refuse ciphertext longer than any plaintext this cache would decode + // before the codec copies it in: junk of any length otherwise reaches + // decrypt, which allocates for all of it before the tag check fails. + // The AAD binds useEnvelope, so a compression-off entry that decrypts + // is a plain serialized value, which decode() caps at maxDecodedSize. + const maxPlaintext = interop + ? DEFAULT_MAX_DECODED_SIZE // decodeInteropValue's fixed input cap + : useEnvelope + ? maxEnvelopeInputSize(this.serializerConfig.maxDecodedSize) + : this.serializerConfig.maxDecodedSize; + if (bytes.length > maxPlaintext + AEAD_OVERHEAD_BYTES) { + throw new ValueTooLargeError( + `Ciphertext size ${bytes.length} exceeds max ${maxPlaintext + AEAD_OVERHEAD_BYTES}` + ); } + // The AAD binds useEnvelope (frozen v0x03 set, protocol#12), so an entry + // written under the other compression setting fails here — a loud, + // counted decrypt failure, never a silent wrong decode. We deliberately + // do NOT retry with the flipped flag: that would reintroduce exactly the + // envelope-mode ambiguity the AAD binding exists to rule out. + const plaintext = this.encryption.decryptNow(bytes, this.aadKey(key), useEnvelope); + return plaintext instanceof Promise + ? plaintext.then((p) => this.decodePlaintext(p, key, interop, useEnvelope, trusted)) + : this.decodePlaintext(plaintext, key, interop, useEnvelope, trusted); + } + + /** decodeEntry after the decrypt: unpack the envelope, then deserialize. */ + private decodePlaintext( + plaintext: Uint8Array, + key: string, + interop: boolean, + useEnvelope: boolean, + trusted: boolean + ): DecodedEntry { if (useEnvelope) { const verdict = envelopeVerdict(plaintext, this.serializerConfig.maxDecodedSize); if (verdict !== 'unpack') { @@ -1133,14 +1166,20 @@ export class CacheImpl implements SecureCache { * 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. + * + * Returns a Promise only when the decrypt suspends (see decodeEntry), so on + * Node.js the hit paths await nothing; with degradation off a failure then + * throws rather than rejects. */ - private async decodeL1Entry( + private decodeL1Entry( key: string, stored: unknown, interop: boolean - ): Promise<{ value: T } | null> { + ): L1Read | Promise> { if (!this.encryption) return { value: stored as T }; + let own: Uint8Array | null = null; + let decoded: DecodedEntry | Promise>; try { if (!(stored instanceof Uint8Array)) { throw new Error( @@ -1148,25 +1187,51 @@ export class CacheImpl implements SecureCache { ); } if (this.trustedL1.get(stored) === interop) { - return { value: (await this.decodeEntry(stored, key, interop, true)).value }; + decoded = this.decodeEntry(stored, key, interop, true); + } else { + // Copied before the decode, so the copy holds exactly the bytes the walks pass. + own = new Uint8Array(stored); + decoded = this.decodeEntry(own, key, interop); + } + if (!(decoded instanceof Promise)) { + return this.servedFromL1(key, stored, own, interop, decoded.value); } - // Copied before the decode, so the copy holds exactly the bytes the walks pass. - const own = new Uint8Array(stored); - const { value } = await this.decodeEntry(own, key, interop); - // A rewrite, refresh, invalidation or eviction since l1.get wins: then - // nothing is swapped and the copy is dropped. - if (this.l1?.replace(key, stored, own)) this.trustedL1.set(own, interop); - return { value }; } 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; + return this.dropL1Entry(key, error); } + // The same steps once the decrypt settles (WebCrypto on Workers). + return decoded + .then(({ value }) => this.servedFromL1(key, stored, own, interop, value)) + .catch((error: unknown) => this.dropL1Entry(key, error)); + } + + /** + * decodeL1Entry's result once the entry decoded. `own` is the copy a first + * hit decoded: it is swapped into L1 in place of `stored` and marked in + * trustedL1, unless a rewrite, refresh, invalidation or eviction since + * l1.get won, in which case nothing is swapped and the copy is dropped. + */ + private servedFromL1( + key: string, + stored: Uint8Array, + own: Uint8Array | null, + interop: boolean, + value: T + ): { value: T } { + if (own !== null && this.l1?.replace(key, stored, own)) this.trustedL1.set(own, interop); + return { value }; + } + + /** decodeL1Entry's failure: drop the entry, count and log it, then apply the fail policy. */ + private dropL1Entry(key: string, error: unknown): null { + 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; } /** @@ -1203,9 +1268,11 @@ export class CacheImpl implements SecureCache { if (l1) { const l1Result = l1.get(key); if (l1Result !== null) { - const decoded = this.encryption - ? await this.decodeL1Entry(key, l1Result, interop) + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.encryption + ? this.decodeL1Entry(key, l1Result, interop) : { value: l1Result as T }; + const decoded = read instanceof Promise ? await read : read; if (decoded !== null) { if (probe) probe.servedBy = 'l1'; else this.recordHit('l1'); @@ -1850,12 +1917,17 @@ export class CacheImpl implements SecureCache { // cannot disagree, and so the poisoned entry is dropped here too. if (this.l1) { const l1Value = this.l1.get(key); - if ( - l1Value !== null && - (!this.encryption || (await this.decodeL1Entry(key, l1Value, false)) !== null) - ) { - this.recordHit('l1'); - return true; + if (l1Value !== null) { + let present = true; + if (this.encryption) { + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.decodeL1Entry(key, l1Value, false); + present = (read instanceof Promise ? await read : read) !== null; + } + if (present) { + this.recordHit('l1'); + return true; + } } } @@ -2039,9 +2111,11 @@ export class CacheImpl implements SecureCache { if (this.swrRequiresWaitUntil && !waitUntil) { const l1Value = this.l1.get(cacheKey); if (l1Value !== null) { - const decoded = this.encryption - ? await this.decodeL1Entry(cacheKey, l1Value, interop) + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.encryption + ? this.decodeL1Entry(cacheKey, l1Value, interop) : { value: l1Value as TResult }; + const decoded = read instanceof Promise ? await read : read; if (decoded !== null) { this.recordHit('l1'); if (timedFrom !== undefined) this.stopL1Timer(timedFrom); @@ -2058,11 +2132,13 @@ export class CacheImpl implements SecureCache { // 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; + let decoded: L1Read = null; try { - decoded = this.encryption - ? await this.decodeL1Entry(cacheKey, swrResult.value, interop) + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.encryption + ? this.decodeL1Entry(cacheKey, swrResult.value, interop) : { value: swrResult.value as TResult }; + decoded = read instanceof Promise ? await read : read; } finally { // Decode failed — by rethrow (degradation off) or by null // (degradation on) — so the entry is dropped either way, and the diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts index e270e3cc..7cd66c27 100644 --- a/packages/cachekit/src/cache.encryption-l1.test.ts +++ b/packages/cachekit/src/cache.encryption-l1.test.ts @@ -17,10 +17,18 @@ import { createHash } from 'node:crypto'; import { describe, it, expect, afterEach, vi } from 'vitest'; -import { ByteStorage } from '@cachekit-io/cachekit-core-ts'; +import { + ByteStorage, + decryptWithTenantKeys, + deriveTenantKeys, + encryptWithTenantKeys, + type TenantKeys, +} from '@cachekit-io/cachekit-core-ts'; import { createCache } from './cache.js'; import { CacheImpl, type CacheRuntime } from './cache-core.js'; import { EncryptionManager } from './encryption/manager.js'; +import { EncryptionManagerCore } from './encryption/manager-core.js'; +import { EncryptionError } from './errors.js'; import { generateKey } from './serialization/key-generator.js'; import type { SecureCache } from './types/cache.js'; import type { Backend } from './backends/types.js'; @@ -364,3 +372,188 @@ describe('L1 zero-knowledge for encrypted caches', () => { expect(l1Entry(cache, generateKey('items', [9]))).toEqual({ plain: true }); }); }); + +// On Node.js the decrypt is a synchronous NAPI call, so once the keys are +// derived an encrypted L1 hit has nothing to await, and awaiting even a +// settled value costs the hit a microtask turn: the promise it hands back +// must already be settled. On Workers WebCrypto decrypts asynchronously, so +// there the hit awaits the decrypt. +describe('an encrypted L1 hit awaits only a decrypt that suspends', () => { + const caches: SecureCache[] = []; + + afterEach(async () => { + vi.restoreAllMocks(); + await Promise.all(caches.splice(0).map((c) => c.close())); + }); + + async function turnsToSettle(promise: Promise): Promise { + let settled = false; + promise.then( + () => (settled = true), + () => (settled = true) + ); + let turns = 0; + while (!settled) { + await null; + turns++; + } + return turns; + } + + /** + * An encrypted cache with L1 on. `suspends` gives it bindings that answer + * with Promises, as the Workers bindings do, over the NAPI core, so the + * bytes are the real ones. + */ + function makeCache( + backend: InMemoryBackend, + { suspends = false, degradation = true, swrRequiresWaitUntil = false } = {} + ): SecureCache { + const runtime: CacheRuntime = { + resolveBackend: () => backend, + createByteStorage: () => new ByteStorage(), + createEncryption: (config) => + suspends + ? new EncryptionManagerCore(config.masterKey, config.tenantId, async () => ({ + deriveTenantKeys, + encryptWithTenantKeys: async (plaintext, aad, keys) => + encryptWithTenantKeys(plaintext, aad, keys as TenantKeys), + decryptWithTenantKeys: async (ciphertext, aad, keys) => + decryptWithTenantKeys(ciphertext, aad, keys as TenantKeys), + })) + : new EncryptionManager(config.masterKey, config.tenantId), + swrRequiresWaitUntil, + }; + const cache = new CacheImpl( + { + backend, + defaultTtl: 3600, + l1: { enabled: true, maxEntries: 100 }, + encryption: { masterKey: MASTER_KEY, tenantId: TENANT }, + metrics: false, + reliability: { degradation }, + }, + runtime + ); + caches.push(cache); + return cache; + } + + it('on Node.js, get() and exists() hand back a settled promise, on the first hit and on later ones', async () => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend); + await cache.set('users:1', { ssn: LEAK_CANARY }); + await cache.set('users:2', { ssn: LEAK_CANARY }); + const alreadySettled = await turnsToSettle(Promise.resolve()); + const reads = backend.gets; + + // Each key's first hit walks a copy and swaps it into L1; the second + // trusts that copy. + for (const read of [ + () => cache.get('users:1'), + () => cache.get('users:1'), + () => cache.exists('users:2'), + () => cache.exists('users:2'), + ]) { + expect(await turnsToSettle(read())).toBe(alreadySettled); + } + expect(l1Entry(cache, 'users:1')).not.toBe(backend.store.get('users:1')); + expect(await cache.get('users:1')).toEqual({ ssn: LEAK_CANARY }); + expect(await cache.exists('users:2')).toBe(true); + expect(backend.gets).toBe(reads); + }); + + it.each([ + ['with SWR', false], + ['on a runtime whose SWR needs waitUntil, with none bound', true], + ])( + 'on Node.js, wrap() hands back a settled promise %s, on the first hit and on later ones', + async (_, swrRequiresWaitUntil) => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend, { swrRequiresWaitUntil }); + const fn = vi.fn(async (id: number) => ({ ssn: LEAK_CANARY, id })); + const load = cache.wrap(fn, { namespace: 'users', ttl: 3600 }); + await load(1); + const alreadySettled = await turnsToSettle(Promise.resolve()); + const reads = backend.gets; + + for (let i = 0; i < 2; i++) { + const hit = load(1); + expect(await turnsToSettle(hit)).toBe(alreadySettled); + expect(await hit).toEqual({ ssn: LEAK_CANARY, id: 1 }); + } + expect(fn).toHaveBeenCalledTimes(1); + expect(backend.gets).toBe(reads); + } + ); + + it('a decrypt that suspends, as on Workers, is awaited, and the first hit still swaps in its copy', async () => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend, { suspends: true }); + await cache.set('users:1', { ssn: LEAK_CANARY }); + const load = cache.wrap(async (id: number) => ({ id }), { namespace: 'users', ttl: 3600 }); + await load(1); + const alreadySettled = await turnsToSettle(Promise.resolve()); + const reads = backend.gets; + expect(l1Entry(cache, 'users:1')).toBe(backend.store.get('users:1')); + + for (const [read, value] of [ + [() => cache.get('users:1'), { ssn: LEAK_CANARY }], + [() => cache.get('users:1'), { ssn: LEAK_CANARY }], + [() => cache.exists('users:1'), true], + [() => load(1), { id: 1 }], + [() => load(1), { id: 1 }], + ] as const) { + const hit = read(); + expect(await turnsToSettle(hit)).toBeGreaterThan(alreadySettled); + expect(await hit).toEqual(value); + } + const copy = l1Entry(cache, 'users:1'); + expect(copy).not.toBe(backend.store.get('users:1')); + expect(copy).toEqual(backend.store.get('users:1')); + expect(backend.gets).toBe(reads); + }); + + it.each([ + { suspends: false, degradation: true }, + { suspends: false, degradation: false }, + { suspends: true, degradation: true }, + { suspends: true, degradation: false }, + ])( + 'a substituted entry is dropped and logged, under degradation $degradation, when the decrypt suspends: $suspends', + async ({ suspends, degradation }) => { + const backend = new InMemoryBackend(); + const cache = makeCache(backend, { suspends, degradation }); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const fn = vi.fn(async (id: number) => ({ ssn: LEAK_CANARY, id })); + const load = cache.wrap(fn, { namespace: 'users', ttl: 3600 }); + await load(1); + await load(2); + const victim = generateKey('users', [1]); + // A valid ciphertext sealed under another key: the AAD binds the key. + const stolen = backend.store.get(generateKey('users', [2]))!; + + for (const [read, value] of [ + [() => cache.get(victim), { ssn: LEAK_CANARY, id: 1 }], + [() => cache.exists(victim), true], + [() => load(1), { ssn: LEAK_CANARY, id: 1 }], + ] as const) { + l1Of(cache).set(victim, stolen, 3600_000, 'users'); + if (degradation) { + // Dropped, then served from L2 (get() and wrap() refill L1 from it). + expect(await read()).toEqual(value); + expect(l1Entry(cache, victim)).not.toBe(stolen); + } else { + await expect(read()).rejects.toThrow(EncryptionError); + expect(l1Entry(cache, victim)).toBeNull(); + } + } + expect(consoleSpy).toHaveBeenCalledTimes(3); + expect(consoleSpy).toHaveBeenCalledWith( + '[cachekit] L1 decrypt failed — entry dropped:', + expect.stringContaining('Authentication verification failed') + ); + expect(fn).toHaveBeenCalledTimes(2); + } + ); +}); diff --git a/packages/cachekit/src/cache.interop.test.ts b/packages/cachekit/src/cache.interop.test.ts index 493296a1..6731152f 100644 --- a/packages/cachekit/src/cache.interop.test.ts +++ b/packages/cachekit/src/cache.interop.test.ts @@ -466,11 +466,11 @@ describe('cache.wrap interop mode', () => { // Junk one byte past the fixed cap + AEAD overhead never reaches decrypt. const impl = cache as unknown as { getEntry: (key: string, interop: boolean) => Promise; - encryption: { decrypt: (...args: unknown[]) => Promise }; + encryption: { decryptNow: (...args: unknown[]) => Uint8Array | Promise }; }; let decrypts = 0; - const realDecrypt = impl.encryption.decrypt.bind(impl.encryption); - impl.encryption.decrypt = (...args) => { + const realDecrypt = impl.encryption.decryptNow.bind(impl.encryption); + impl.encryption.decryptNow = (...args) => { decrypts++; return realDecrypt(...args); }; diff --git a/packages/cachekit/src/cache.l1-trusted.test.ts b/packages/cachekit/src/cache.l1-trusted.test.ts index a9148d25..f7ca3bff 100644 --- a/packages/cachekit/src/cache.l1-trusted.test.ts +++ b/packages/cachekit/src/cache.l1-trusted.test.ts @@ -13,8 +13,9 @@ */ import { createHash } from 'node:crypto'; -import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; +import { describe, it, expect, afterEach, beforeEach, onTestFinished, vi } from 'vitest'; import { createCache } from './cache.js'; +import { EncryptionManagerCore } from './encryption/manager-core.js'; import { generateKey } from './serialization/key-generator.js'; import { generateInteropKey } from './serialization/interop.js'; import { decodeCounted, decodeTrusted } from './serialization/serializer.js'; @@ -206,9 +207,19 @@ describe('encrypted L1 hits: walk once, on a copy L1 keeps', () => { const rewritten = other.store.get('users:5')!; vi.clearAllMocks(); - // The hit copies and starts decrypting before it yields; the rewrite - // lands while it decodes. + // On Node.js the hit decodes inside the get() call, which leaves no room + // for a race. A decrypt that suspends, as WebCrypto's does on Workers, + // does: the hit copies and starts decrypting before it yields, and the + // rewrite lands while it decodes. + const decryptNow = EncryptionManagerCore.prototype.decryptNow; + const suspends = vi + .spyOn(EncryptionManagerCore.prototype, 'decryptNow') + .mockImplementationOnce(async function (this: EncryptionManagerCore, ...args) { + return decryptNow.apply(this, args); + }); + onTestFinished(() => suspends.mockRestore()); const hit = cache.get('users:5'); + expect(suspends).toHaveBeenCalledTimes(1); l1Of(cache).set('users:5', rewritten, 3600_000, 'users'); expect(await hit).toEqual({ id: 5 }); diff --git a/packages/cachekit/src/cache.test.ts b/packages/cachekit/src/cache.test.ts index 1e5e7b96..eb1e9372 100644 --- a/packages/cachekit/src/cache.test.ts +++ b/packages/cachekit/src/cache.test.ts @@ -911,11 +911,11 @@ describe('Cache Integration', () => { reliability: { degradation: false, retry: { maxAttempts: 1 } }, }); const impl = cache as unknown as { - encryption: { decrypt: (...args: unknown[]) => Promise }; + encryption: { decryptNow: (...args: unknown[]) => Uint8Array | Promise }; }; let decrypts = 0; - const realDecrypt = impl.encryption.decrypt.bind(impl.encryption); - impl.encryption.decrypt = (...args) => { + const realDecrypt = impl.encryption.decryptNow.bind(impl.encryption); + impl.encryption.decryptNow = (...args) => { decrypts++; return realDecrypt(...args); }; @@ -2018,7 +2018,7 @@ describe('Cache Integration', () => { const c = secureCache(backend, KEY_A); await c.set('ns:k', 'v'); const backendGet = vi.spyOn(backend, 'get'); - vi.spyOn(EncryptionManagerCore.prototype, 'decrypt').mockRejectedValueOnce( + vi.spyOn(EncryptionManagerCore.prototype, 'decryptNow').mockRejectedValueOnce( new Error('native binding failed to load') ); @@ -2092,7 +2092,7 @@ describe('Cache Integration', () => { // Seeded, so a read that skipped the pre-flight would reach decrypt. await backend.set(overLimitKey, new Uint8Array([1]), 60); const backendGet = vi.spyOn(backend, 'get'); - const decrypt = vi.spyOn(EncryptionManagerCore.prototype, 'decrypt'); + const decrypt = vi.spyOn(EncryptionManagerCore.prototype, 'decryptNow'); const c = secureCache(backend); await expect(c.get(overLimitKey)).rejects.toThrow(ConfigurationError); @@ -2246,7 +2246,7 @@ describe('Cache Integration', () => { 'keyPrefix $keyPrefix binds $bound at encrypt, decrypt and the AAD-size pre-flight', async ({ keyPrefix, bound }) => { const encrypt = vi.spyOn(EncryptionManagerCore.prototype, 'encrypt'); - const decrypt = vi.spyOn(EncryptionManagerCore.prototype, 'decrypt'); + const decrypt = vi.spyOn(EncryptionManagerCore.prototype, 'decryptNow'); const validateKey = vi.spyOn(EncryptionManagerCore.prototype, 'validateKey'); const c = secureOver(new PrefixingBackend(keyPrefix, new Map())); diff --git a/packages/cachekit/src/encryption/manager-core.test.ts b/packages/cachekit/src/encryption/manager-core.test.ts index 4ef43bdb..3c02306a 100644 --- a/packages/cachekit/src/encryption/manager-core.test.ts +++ b/packages/cachekit/src/encryption/manager-core.test.ts @@ -398,6 +398,32 @@ describe('EncryptionManagerCore keyring config (previousMasterKeys)', () => { await expect(inFlight).rejects.toThrow(EncryptionError); }); + + it('decryptNow decrypts in the call once the keys are derived, and throws what decrypt() rejects with', async () => { + const { bindings } = mockBindings(); + const manager = new TestManager(async () => bindings); + + // Before the keys are derived it loads the bindings first: a Promise. + const first = manager.decryptNow(new Uint8Array([1]), 'ns:k'); + expect(first).toBeInstanceOf(Promise); + expect(await first).toEqual(new Uint8Array([2])); + + const plaintext = manager.decryptNow(new Uint8Array([1]), 'ns:k', true); + expect(plaintext).not.toBeInstanceOf(Promise); + expect(plaintext).toEqual(new Uint8Array([2])); + vi.mocked(bindings.decryptWithTenantKeys).mockImplementationOnce(() => { + throw new Error('Authentication verification failed'); + }); + expect(() => manager.decryptNow(new Uint8Array(28), 'ns:k')).toThrow( + new EncryptionError('Decryption failed: Authentication verification failed') + ); + + // After dispose() it rejects, as decrypt() does. + manager.dispose(); + await expect(manager.decryptNow(new Uint8Array([1]), 'ns:k')).rejects.toThrow( + 'EncryptionManager has been disposed' + ); + }); }); // The Workers bindings are asynchronous (WebCrypto); the seam accepts a @@ -440,6 +466,26 @@ describe('EncryptionManagerCore over asynchronous bindings', () => { manager.dispose(); }); + it('decryptNow returns the binding’s Promise once the keys are derived, its rejection mapped to EncryptionError', async () => { + let decrypts = 0; + const { bindings } = mockBindings({ + decryptWithTenantKeys: vi.fn(async () => { + if (decrypts++ > 0) throw new Error('Authentication verification failed'); + return new Uint8Array([8]); + }), + }); + const manager = new TestManager(async () => bindings); + await manager.encrypt(new Uint8Array([1]), 'ns:k'); // derives the keys + + const plaintext = manager.decryptNow(new Uint8Array([1]), 'ns:k'); + expect(plaintext).toBeInstanceOf(Promise); + expect(await plaintext).toEqual(new Uint8Array([8])); + await expect(manager.decryptNow(new Uint8Array(28), 'ns:k')).rejects.toThrow( + new EncryptionError('Decryption failed: Authentication verification failed') + ); + manager.dispose(); + }); + it('returns the value an asynchronous encrypt and decrypt resolve to', async () => { const { bindings } = mockBindings({ encryptWithTenantKeys: vi.fn(async () => new Uint8Array([7])), diff --git a/packages/cachekit/src/encryption/manager-core.ts b/packages/cachekit/src/encryption/manager-core.ts index b58d0cff..45e73533 100644 --- a/packages/cachekit/src/encryption/manager-core.ts +++ b/packages/cachekit/src/encryption/manager-core.ts @@ -26,6 +26,14 @@ function isThenable(value: T | PromiseLike): value is PromiseLike { return typeof (value as { then?: unknown }).then === 'function'; } +/** decrypt()'s error for a binding failure, thrown or rejected. */ +function decryptionFailed(error: unknown): EncryptionError { + return new EncryptionError( + `Decryption failed: ${error instanceof Error ? error.message : 'Unknown error'}`, + { cause: error instanceof Error ? error : undefined } + ); +} + function concatBytes(...parts: Uint8Array[]): Uint8Array { const out = new Uint8Array(parts.reduce((sum, p) => sum + p.length, 0)); let offset = 0; @@ -374,17 +382,46 @@ export class EncryptionManagerCore { */ async decrypt(ciphertext: Uint8Array, cacheKey: string, compressed = false): Promise { await this.ensureInitialized(); + return this.decryptWithKeys(ciphertext, cacheKey, compressed); + } + + /** + * decrypt() without its awaits, for the cache's read path. Once the keys are + * derived, a binding that returns a plain value (NAPI on Node.js) decrypts + * in this call: it returns the plaintext, or throws the EncryptionError + * decrypt() would reject with. Awaiting even a settled value would cost an + * L1 hit a microtask turn. Otherwise it returns a Promise: decrypt()'s + * before the keys are derived or after dispose(), and the binding's on + * Workers, where WebCrypto decrypts asynchronously. Package-internal. + * @internal + */ + decryptNow( + ciphertext: Uint8Array, + cacheKey: string, + compressed = false + ): Uint8Array | Promise { + return this.tenantKeys + ? this.decryptWithKeys(ciphertext, cacheKey, compressed) + : this.decrypt(ciphertext, cacheKey, compressed); + } + /** decrypt() once the keys are derived: a failure throws, or rejects if the binding's Promise does. */ + private decryptWithKeys( + ciphertext: Uint8Array, + cacheKey: string, + compressed: boolean + ): Uint8Array | Promise { + let result: Uint8Array | Promise; try { const aad = this.buildAAD(cacheKey, compressed); - const result = this.native!.decryptWithTenantKeys(ciphertext, aad, this.tenantKeys!); - return isThenable(result) ? await result : result; + result = this.native!.decryptWithTenantKeys(ciphertext, aad, this.tenantKeys!); } catch (error) { - throw new EncryptionError( - `Decryption failed: ${error instanceof Error ? error.message : 'Unknown error'}`, - { cause: error instanceof Error ? error : undefined } - ); + throw decryptionFailed(error); } + if (!isThenable(result)) return result; + return Promise.resolve(result).catch((error: unknown) => { + throw decryptionFailed(error); + }); } /** diff --git a/packages/cachekit/src/encryption/manager.ts b/packages/cachekit/src/encryption/manager.ts index bba436dd..cdf78689 100644 --- a/packages/cachekit/src/encryption/manager.ts +++ b/packages/cachekit/src/encryption/manager.ts @@ -33,8 +33,8 @@ async function loadNapiBindings(): Promise { * @example * ```typescript * const manager = new EncryptionManager(process.env.CACHEKIT_MASTER_KEY!, 'tenant-123'); - * const ciphertext = manager.encrypt(data, 'cache:my-key'); - * const plaintext = manager.decrypt(ciphertext, 'cache:my-key'); + * const ciphertext = await manager.encrypt(data, 'cache:my-key'); + * const plaintext = await manager.decrypt(ciphertext, 'cache:my-key'); * manager.dispose(); // Clean up when done * ``` */ From 7c893c9ad91f4c9f694972c9c7d4a3c5ddd3bd00 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 02:37:44 +0000 Subject: [PATCH 2/7] perf(cachekit): keep decrypt() as it was; document the sync encrypted hit decryptNow() takes the binding call into its own body instead of sharing one with decrypt(): routing decrypt() through it changed how V8 inlined a caller awaiting decrypt() and encrypt(), +5.6% instructions on napi-encrypted. States in the README that an encrypted L1 hit on Node.js awaits no promise, lists secure-get-l1-hit in CONTRIBUTING and gives it an ir-seconds row. --- CONTRIBUTING.md | 2 +- packages/cachekit/README.md | 2 +- packages/cachekit/bench/ir-seconds.json | 1 + .../cachekit/src/encryption/manager-core.ts | 31 ++++++++++--------- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d096318..2fb41a00 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,7 +147,7 @@ Six harnesses measure the client. **Call shape (runs in `pnpm test`).** `packages/cachekit/test/transport/call-shape.test.ts` runs the CacheKit Cloud backend against a local TLS fake of CacheKit Cloud. It asserts the exact requests each op sends (a wrap miss is `GET` then `PUT`, and with `writeBehind` the caller waits for the `GET` only; a locked miss is `GET`, lock, `PUT`, unlock) and the connections 100 ops use. Each extra request is a round trip, and each extra connection is a TCP and TLS handshake. So if you change one of these numbers, change the expectation in the same PR and say why. The fake mints a throwaway certificate with the `openssl` CLI, and it needs Node 22.19+ or 24.5+ for `tls.setDefaultCACertificates`. Where either is missing, the tests fail with a message that names the requirement. -**Instructions per op (`pnpm --filter @cachekit-io/cachekit bench:ir`).** This counts the main-thread instructions of each hot path under callgrind: key generation through the NAPI hash (`keygen`, `keygen-1kb`) and the wasm hash (`keygen-wasm`, `keygen-wasm-1kb`), at 45 B and 1 KB of arguments, the serializer, the NAPI and wasm envelopes (1 KB, plus 40 B under the core's 64 B literals-only cutover and 64 KiB), both encrypted paths (1 KB, plus 64 KiB of incompressible bytes), and the L1 hit through `get()` (`get-l1-hit`) and through `wrap()`, once with an object among the arguments and once with the single integer argument the README benchmark passes, and the encrypted `wrap()` L1 hit at 100 B, 10 KB and 1 MB. Unlike wall-clock timing, instruction counts repeat on a busy machine. The suite pins V8's flags and counts only the measured ops. It also lowers V8's TurboFan threshold, so no optimizing compile lands in any workload's window. For all but the two 1 MB workloads, the window counts the code V8 runs in steady state. Those run so few ops that the `set()` and `wrap()` code around each 1 MB encode or decode stays in V8's interpreter for the whole run, which costs too little to show beside the encode or decode. A workload that tiers up later sets its own `warmup`; `bench/ir-workload.mjs` says how to check one with `--trace-opt`. The 100,000-entry L1 at-capacity workload collects garbage just before its window, so V8 does not finish collecting the entries its setup made inside it: that had been two thirds of its count, and it moved with whatever the build allocated at load. The 1,000- and 10,000-entry ones do not collect: at 10,000 a collection made the count move 2.4% with heap layout. On Node 22 and 24, every path repeats within 0.01%. On Node 26, paths through the native cores repeat within about 0.3%, but `napi-encrypted` and the encrypted `wrap()` hits at 100 B and 1 MB sometimes have a run up to 1.7% off the rest, which makes them read inconclusive. Gate on Node 22 or 24. It needs Linux and valgrind. It imports `dist/` and the wasm build, so run `pnpm build` and `pnpm --filter @cachekit-io/cachekit-core-wasm build:wasm` first; the second needs the `wasm32-unknown-unknown` target, `wasm-bindgen` and `wasm-opt`, and without it pass `--only` to skip the seven wasm workloads (`wasm-envelope`, `wasm-envelope-40b`, `wasm-envelope-64kb`, `wasm-encrypted`, `wasm-encrypted-64kb`, `keygen-wasm`, `keygen-wasm-1kb`). To A/B a change, run it on both builds on the same machine: +**Instructions per op (`pnpm --filter @cachekit-io/cachekit bench:ir`).** This counts the main-thread instructions of each hot path under callgrind: key generation through the NAPI hash (`keygen`, `keygen-1kb`) and the wasm hash (`keygen-wasm`, `keygen-wasm-1kb`), at 45 B and 1 KB of arguments, the serializer, the NAPI and wasm envelopes (1 KB, plus 40 B under the core's 64 B literals-only cutover and 64 KiB), both encrypted paths (1 KB, plus 64 KiB of incompressible bytes), and the L1 hit through `get()` (`get-l1-hit`) and through `wrap()`, once with an object among the arguments and once with the single integer argument the README benchmark passes, the encrypted `wrap()` L1 hit at 100 B, 10 KB and 1 MB, and the encrypted `get()` L1 hit at 100 B (`secure-get-l1-hit`). Unlike wall-clock timing, instruction counts repeat on a busy machine. The suite pins V8's flags and counts only the measured ops. It also lowers V8's TurboFan threshold, so no optimizing compile lands in any workload's window. For all but the two 1 MB workloads, the window counts the code V8 runs in steady state. Those run so few ops that the `set()` and `wrap()` code around each 1 MB encode or decode stays in V8's interpreter for the whole run, which costs too little to show beside the encode or decode. A workload that tiers up later sets its own `warmup`; `bench/ir-workload.mjs` says how to check one with `--trace-opt`. The 100,000-entry L1 at-capacity workload collects garbage just before its window, so V8 does not finish collecting the entries its setup made inside it: that had been two thirds of its count, and it moved with whatever the build allocated at load. The 1,000- and 10,000-entry ones do not collect: at 10,000 a collection made the count move 2.4% with heap layout. On Node 22 and 24, every path repeats within 0.01%. On Node 26, paths through the native cores repeat within about 0.3%, but `napi-encrypted` and the encrypted `wrap()` hits at 100 B and 1 MB sometimes have a run up to 1.7% off the rest, which makes them read inconclusive. Gate on Node 22 or 24. It needs Linux and valgrind. It imports `dist/` and the wasm build, so run `pnpm build` and `pnpm --filter @cachekit-io/cachekit-core-wasm build:wasm` first; the second needs the `wasm32-unknown-unknown` target, `wasm-bindgen` and `wasm-opt`, and without it pass `--only` to skip the seven wasm workloads (`wasm-envelope`, `wasm-envelope-40b`, `wasm-envelope-64kb`, `wasm-encrypted`, `wasm-encrypted-64kb`, `keygen-wasm`, `keygen-wasm-1kb`). To A/B a change, run it on both builds on the same machine: ```bash git checkout -- packages/cachekit/bench # in the base checkout: your bench/, as CI does diff --git a/packages/cachekit/README.md b/packages/cachekit/README.md index fc6b690e..2b590569 100644 --- a/packages/cachekit/README.md +++ b/packages/cachekit/README.md @@ -1189,7 +1189,7 @@ What one call costs in the client, on an in-memory backend, so no figure include - **`wrap()` L1 hit**: the hit returns the stored object as it is, so the value's size barely matters. A wrapped function remembers the key of each call whose arguments are all strings, numbers, booleans, `null` or `undefined`, and the L1 entry that call was last served from. A repeat call is served from that entry on one check, with no key generation and no lookup by key, while L1 still holds the entry and it is short of its stale-while-revalidate point (the top of that point's ±10% jitter band); once L1 evicts or expires it, or it reaches that band, the call looks its key up in L1 again. `delete()`, `invalidate()` and an invalidation event empty the memo too, so the next call generates its key first, and after `close()` every call is rejected. It remembers a call only if it has at most 8 arguments, no `NaN`, and strings whose `length` adds up to 256 or less. Every other call generates its key, which adds about the cost of the key generation row: an object, array, `Date`, `Map`, `Set` or binary argument, a call past those limits, a call with `skipL1` or under `ttl: 0`, every `interop` call, and every call on a cache that encrypts or has L1 off, or on Cloudflare Workers. - **Key memo**: a wrapped function remembers at most 1,024 keys and starts again when full: about 160 KiB when each call passes one number, and at most about 1 MiB. It holds copies of the arguments, so a cache that encrypts keeps no memo, and `delete()`, `invalidate()`, `close()` and, with `invalidation` configured, an invalidation event from another instance empty every memo of the cache. A timer also empties a memo one second after its first key, or one `ttl` after it if that is sooner, even if the function is not called again, so it is set to drop an argument no later than L1 stops serving a value stored with that `ttl`. The timer can run late, for example when the event loop is busy or the process is suspended, and the memo keeps its arguments until it runs. A call that comes one `ttl` or more after the first key, before the timer has run, empties the memo first. The call that stores the first key sets that timer, which keeps the call's async context, `AsyncLocalStorage` stores included, until it runs: one second or less after it was set, unless it runs late. So a function called again and again with the same arguments generates their key about once a second, or once per `ttl` if that is shorter. Wrap a function once, at module scope: a function wrapped inside each request keeps that request's async context and its memo until the memo's timer runs, one second later or one `ttl` later if that is sooner. With a `ttl` of a second or more, at 1,000 requests a second with a 16 KiB store, that is about 16 MB. A wrap under `ttl: 0`, which never expires, keeps no memo, and nor does a Worker, where a timer can be cancelled when its request ends. -- **Encrypted L1 hit**: L1 holds the same ciphertext as L2, so every hit also generates the key, decrypts, verifies the AAD and decodes the value. That cost grows with the value. A read from L2 also walks the document to check its nesting depth, before the decode and again after it. So does the first hit on an entry, which then keeps its own copy of the ciphertext in L1; later hits on that copy skip the walks (an `interop` entry keeps the check after the decode). +- **Encrypted L1 hit**: L1 holds the same ciphertext as L2, so every hit also generates the key, decrypts, verifies the AAD and decodes the value. That cost grows with the value. A read from L2 also walks the document to check its nesting depth, before the decode and again after it. So does the first hit on an entry, which then keeps its own copy of the ciphertext in L1; later hits on that copy skip the walks (an `interop` entry keeps the check after the decode). On Node.js the decrypt is a synchronous call into the native core, so once the cache has derived its keys, at its first encrypt or decrypt, a hit through `get()`, `exists()` or `wrap()` awaits no promise. On Cloudflare Workers, WebCrypto decrypts asynchronously, so the hit waits for it. - **L2 hit and `set`**: L1 is off, so each call reads or writes the backend and decodes or encodes the value. A real backend adds its round trip, which for Redis or CacheKit Cloud is a network call. To print this table for your own machine, run `pnpm install` and `pnpm build`, then `node packages/cachekit/bench/readme.mjs`, from a clone of the repository, or run the **Bench** workflow from its Actions tab. Across 11 runs on this CPU model, the 100 B hit took 290-360 ns, 320 ns at the median, and every other cell agreed within 10%. The encrypted L1 hit row is the median of four later runs on this CPU model, which read 9.0-9.7 µs and 119-125 µs. Hosted runners also come with other CPU models, on which the 100 B hit took from 160 ns (an EPYC 9V45) to 330 ns (an Intel Xeon Platinum 8370C or 8573C) in the same batch of runs, and other machines give other figures. diff --git a/packages/cachekit/bench/ir-seconds.json b/packages/cachekit/bench/ir-seconds.json index 186fee65..f1459199 100644 --- a/packages/cachekit/bench/ir-seconds.json +++ b/packages/cachekit/bench/ir-seconds.json @@ -26,6 +26,7 @@ "wasm-encrypted-64kb": 124, "secure-set-get": 71, "get-l1-hit": 14, + "secure-get-l1-hit": 35, "wrap-l1-hit": 18, "wrap-l1-hit-metrics": 22, "get-l2-hit-metrics": 62, diff --git a/packages/cachekit/src/encryption/manager-core.ts b/packages/cachekit/src/encryption/manager-core.ts index 45e73533..96c8279d 100644 --- a/packages/cachekit/src/encryption/manager-core.ts +++ b/packages/cachekit/src/encryption/manager-core.ts @@ -26,7 +26,7 @@ function isThenable(value: T | PromiseLike): value is PromiseLike { return typeof (value as { then?: unknown }).then === 'function'; } -/** decrypt()'s error for a binding failure, thrown or rejected. */ +/** The EncryptionError decrypt() builds for a binding failure, for decryptNow() to throw or reject with. */ function decryptionFailed(error: unknown): EncryptionError { return new EncryptionError( `Decryption failed: ${error instanceof Error ? error.message : 'Unknown error'}`, @@ -382,7 +382,20 @@ export class EncryptionManagerCore { */ async decrypt(ciphertext: Uint8Array, cacheKey: string, compressed = false): Promise { await this.ensureInitialized(); - return this.decryptWithKeys(ciphertext, cacheKey, compressed); + + // decryptNow() repeats this body rather than sharing it: routing decrypt() + // through a shared helper changed how V8 inlined a caller awaiting + // decrypt() and encrypt(), +5.6% instructions on bench:ir napi-encrypted. + try { + const aad = this.buildAAD(cacheKey, compressed); + const result = this.native!.decryptWithTenantKeys(ciphertext, aad, this.tenantKeys!); + return isThenable(result) ? await result : result; + } catch (error) { + throw new EncryptionError( + `Decryption failed: ${error instanceof Error ? error.message : 'Unknown error'}`, + { cause: error instanceof Error ? error : undefined } + ); + } } /** @@ -400,21 +413,11 @@ export class EncryptionManagerCore { cacheKey: string, compressed = false ): Uint8Array | Promise { - return this.tenantKeys - ? this.decryptWithKeys(ciphertext, cacheKey, compressed) - : this.decrypt(ciphertext, cacheKey, compressed); - } - - /** decrypt() once the keys are derived: a failure throws, or rejects if the binding's Promise does. */ - private decryptWithKeys( - ciphertext: Uint8Array, - cacheKey: string, - compressed: boolean - ): Uint8Array | Promise { + if (!this.tenantKeys) return this.decrypt(ciphertext, cacheKey, compressed); let result: Uint8Array | Promise; try { const aad = this.buildAAD(cacheKey, compressed); - result = this.native!.decryptWithTenantKeys(ciphertext, aad, this.tenantKeys!); + result = this.native!.decryptWithTenantKeys(ciphertext, aad, this.tenantKeys); } catch (error) { throw decryptionFailed(error); } From e5cc7144992e8d0c6648895d7605804e5680b2b5 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 03:01:14 +0000 Subject: [PATCH 3/7] fix(cachekit): await a decode's Promise even when the global Promise is replaced The value-or-Promise checks used instanceof Promise. An async function returns the engine's own Promise, which fails instanceof once a library such as zone.js or Bluebird has replaced the global, so a cache's first decrypt, before its keys are derived, was taken for a value: an L1 hit served undefined and an L2 read failed to decode. The checks now test for a thenable, through one isThenable shared with the manager. Also pins decryptNow()'s AAD against the protocol vectors, names the trust and failure helpers for what they do, shares the settle-turn test helper, and drops the estimated ir-seconds row: a workload with no row counts as the median. --- packages/cachekit/bench/ir-seconds.json | 1 - packages/cachekit/src/cache-core.ts | 47 +++++++----- .../cachekit/src/cache.encryption-l1.test.ts | 75 +++++++++++++++---- packages/cachekit/src/cache.test.ts | 15 +--- .../cachekit/src/encryption/manager-core.ts | 9 +-- packages/cachekit/src/utils/thenable.ts | 9 +++ packages/cachekit/test/fixtures/settle.ts | 18 +++++ .../test/protocol/encryption.protocol.test.ts | 18 ++++- 8 files changed, 131 insertions(+), 61 deletions(-) create mode 100644 packages/cachekit/src/utils/thenable.ts create mode 100644 packages/cachekit/test/fixtures/settle.ts diff --git a/packages/cachekit/bench/ir-seconds.json b/packages/cachekit/bench/ir-seconds.json index f1459199..186fee65 100644 --- a/packages/cachekit/bench/ir-seconds.json +++ b/packages/cachekit/bench/ir-seconds.json @@ -26,7 +26,6 @@ "wasm-encrypted-64kb": 124, "secure-set-get": 71, "get-l1-hit": 14, - "secure-get-l1-hit": 35, "wrap-l1-hit": 18, "wrap-l1-hit-metrics": 22, "get-l2-hit-metrics": 62, diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts index c8b87113..291c4ada 100644 --- a/packages/cachekit/src/cache-core.ts +++ b/packages/cachekit/src/cache-core.ts @@ -14,6 +14,7 @@ import type { InvalidationEvent } from './l1/types.js'; import type { MetricsCollector, MetricsConfig } from './metrics/prometheus.js'; import { checkDefaultLabels } from './metrics/labels.js'; import { logError } from './logger.js'; +import { isThenable } from './utils/thenable.js'; import { L1Cache, type L1Entry } from './l1/lru-cache.js'; import { ReliabilityExecutor } from './reliability/executor.js'; import { @@ -1037,8 +1038,9 @@ export class CacheImpl implements SecureCache { * decodeL1Entry passes it, for a copy marked in trustedL1. * * Returns a Promise only when the decrypt suspends (see - * EncryptionLike.decryptNow): on Node.js an entry decodes in this call, so - * an L1 hit awaits nothing. A failure throws, or rejects that Promise. + * EncryptionLike.decryptNow): on Node.js, once the keys are derived, an + * entry decodes in this call, so an L1 hit awaits nothing. A failure throws, + * or rejects that Promise. */ private decodeEntry( bytes: Uint8Array, @@ -1070,7 +1072,7 @@ export class CacheImpl implements SecureCache { // do NOT retry with the flipped flag: that would reintroduce exactly the // envelope-mode ambiguity the AAD binding exists to rule out. const plaintext = this.encryption.decryptNow(bytes, this.aadKey(key), useEnvelope); - return plaintext instanceof Promise + return isThenable(plaintext) ? plaintext.then((p) => this.decodePlaintext(p, key, interop, useEnvelope, trusted)) : this.decodePlaintext(plaintext, key, interop, useEnvelope, trusted); } @@ -1140,7 +1142,8 @@ export class CacheImpl implements SecureCache { * 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, so the * hit paths call this only when the cache encrypts and use a plaintext L1 - * value as it is: awaiting even a settled promise costs a hit a microtask. + * value as it is, which leaves the plaintext hit with no call and no + * thenable check. * * Verify once, on the first hit. The bytes a fill gives L1 can be a buffer * the backend still holds (the one it returned, or the one setEntry gave @@ -1193,25 +1196,26 @@ export class CacheImpl implements SecureCache { own = new Uint8Array(stored); decoded = this.decodeEntry(own, key, interop); } - if (!(decoded instanceof Promise)) { - return this.servedFromL1(key, stored, own, interop, decoded.value); + if (!isThenable(decoded)) { + return this.trustL1Copy(key, stored, own, interop, decoded.value); } } catch (error) { - return this.dropL1Entry(key, error); + return this.failL1Read(key, error); } // The same steps once the decrypt settles (WebCrypto on Workers). return decoded - .then(({ value }) => this.servedFromL1(key, stored, own, interop, value)) - .catch((error: unknown) => this.dropL1Entry(key, error)); + .then(({ value }) => this.trustL1Copy(key, stored, own, interop, value)) + .catch((error: unknown) => this.failL1Read(key, error)); } /** - * decodeL1Entry's result once the entry decoded. `own` is the copy a first - * hit decoded: it is swapped into L1 in place of `stored` and marked in - * trustedL1, unless a rewrite, refresh, invalidation or eviction since - * l1.get won, in which case nothing is swapped and the copy is dropped. + * decodeL1Entry's result once the entry decoded, and the one place L1 trust + * is granted. `own` is the copy a first hit decoded: it is swapped into L1 + * in place of `stored` and marked in trustedL1, unless a rewrite, refresh, + * invalidation or eviction since l1.get won, in which case nothing is + * swapped and the copy is dropped. */ - private servedFromL1( + private trustL1Copy( key: string, stored: Uint8Array, own: Uint8Array | null, @@ -1222,8 +1226,11 @@ export class CacheImpl implements SecureCache { return { value }; } - /** decodeL1Entry's failure: drop the entry, count and log it, then apply the fail policy. */ - private dropL1Entry(key: string, error: unknown): null { + /** + * decodeL1Entry's failure: drop the entry, count and log it, then apply the + * fail policy, rethrowing with degradation off and returning null with it on. + */ + private failL1Read(key: string, error: unknown): null { this.l1?.invalidateByKey(key); this.recordFailure('l1_decrypt', error); logError( @@ -1272,7 +1279,7 @@ export class CacheImpl implements SecureCache { const read = this.encryption ? this.decodeL1Entry(key, l1Result, interop) : { value: l1Result as T }; - const decoded = read instanceof Promise ? await read : read; + const decoded = isThenable(read) ? await read : read; if (decoded !== null) { if (probe) probe.servedBy = 'l1'; else this.recordHit('l1'); @@ -1922,7 +1929,7 @@ export class CacheImpl implements SecureCache { if (this.encryption) { // Awaited only when the decrypt suspends (see decodeL1Entry). const read = this.decodeL1Entry(key, l1Value, false); - present = (read instanceof Promise ? await read : read) !== null; + present = (isThenable(read) ? await read : read) !== null; } if (present) { this.recordHit('l1'); @@ -2115,7 +2122,7 @@ export class CacheImpl implements SecureCache { const read = this.encryption ? this.decodeL1Entry(cacheKey, l1Value, interop) : { value: l1Value as TResult }; - const decoded = read instanceof Promise ? await read : read; + const decoded = isThenable(read) ? await read : read; if (decoded !== null) { this.recordHit('l1'); if (timedFrom !== undefined) this.stopL1Timer(timedFrom); @@ -2138,7 +2145,7 @@ export class CacheImpl implements SecureCache { const read = this.encryption ? this.decodeL1Entry(cacheKey, swrResult.value, interop) : { value: swrResult.value as TResult }; - decoded = read instanceof Promise ? await read : read; + decoded = isThenable(read) ? await read : read; } finally { // Decode failed — by rethrow (degradation off) or by null // (degradation on) — so the entry is dropped either way, and the diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts index 7cd66c27..e5e416bf 100644 --- a/packages/cachekit/src/cache.encryption-l1.test.ts +++ b/packages/cachekit/src/cache.encryption-l1.test.ts @@ -30,6 +30,7 @@ import { EncryptionManager } from './encryption/manager.js'; import { EncryptionManagerCore } from './encryption/manager-core.js'; import { EncryptionError } from './errors.js'; import { generateKey } from './serialization/key-generator.js'; +import { turnsToSettle } from '../test/fixtures/settle.js'; import type { SecureCache } from './types/cache.js'; import type { Backend } from './backends/types.js'; import type { L1Cache } from './l1/lru-cache.js'; @@ -386,20 +387,6 @@ describe('an encrypted L1 hit awaits only a decrypt that suspends', () => { await Promise.all(caches.splice(0).map((c) => c.close())); }); - async function turnsToSettle(promise: Promise): Promise { - let settled = false; - promise.then( - () => (settled = true), - () => (settled = true) - ); - let turns = 0; - while (!settled) { - await null; - turns++; - } - return turns; - } - /** * An encrypted cache with L1 on. `suspends` gives it bindings that answer * with Promises, as the Workers bindings do, over the NAPI core, so the @@ -514,6 +501,66 @@ describe('an encrypted L1 hit awaits only a decrypt that suspends', () => { expect(backend.gets).toBe(reads); }); + // Before its keys are derived a cache decrypts through an async function, + // whose Promise is the engine's own: `instanceof` against a replaced global + // Promise misses it, and a Promise taken for a value is never awaited. + async function withReplacedPromise(op: () => Promise): Promise { + const NativePromise = globalThis.Promise; + class ReplacedPromise extends NativePromise {} + globalThis.Promise = ReplacedPromise as PromiseConstructor; + try { + return await op(); + } finally { + globalThis.Promise = NativePromise; + } + } + + const VALUE = { ssn: LEAK_CANARY, id: 1 }; + type Read = (cache: SecureCache, load: (id: number) => Promise) => Promise; + const KEY = generateKey('users', [1]); + const getKey: Read = (cache) => cache.get(KEY); + const existsKey: Read = (cache) => cache.exists(KEY); + const wrapped: Read = (_, load) => load(1); + it.each([ + ['get() from L1', getKey, VALUE, true, false], + ['get() from L2', getKey, VALUE, false, false], + ['exists() from L1', existsKey, true, true, false], + ['wrap() with SWR from L1', wrapped, VALUE, true, false], + ['wrap() with no waitUntil bound from L1', wrapped, VALUE, true, true], + ] as const)( + '%s decodes when a library has replaced the global Promise, as zone.js does', + async (_, read, expected, fromL1, noWaitUntil) => { + const backend = new InMemoryBackend(); + await makeCache(backend).set(KEY, VALUE); + const fresh = makeCache(backend, { swrRequiresWaitUntil: noWaitUntil }); + if (fromL1) l1Of(fresh).set(KEY, backend.store.get(KEY)!, 3600_000, 'users'); + const load = fresh.wrap(async (id: number) => ({ ssn: 'computed', id }), { + namespace: 'users', + ttl: 3600, + }); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(await withReplacedPromise(() => read(fresh, load))).toEqual(expected); + expect(consoleSpy).not.toHaveBeenCalled(); + } + ); + + it('exists() drops a substituted L1 entry when a library has replaced the global Promise', async () => { + const backend = new InMemoryBackend(); + const writer = makeCache(backend); + await writer.set(KEY, VALUE); + await writer.set('users:other', { other: true }); + const fresh = makeCache(backend); + l1Of(fresh).set(KEY, backend.store.get('users:other')!, 3600_000, 'users'); + backend.store.delete(KEY); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + expect(await withReplacedPromise(() => fresh.exists(KEY))).toBe(false); + expect(consoleSpy).toHaveBeenCalledWith( + '[cachekit] L1 decrypt failed — entry dropped:', + expect.stringContaining('Authentication verification failed') + ); + }); + it.each([ { suspends: false, degradation: true }, { suspends: false, degradation: false }, diff --git a/packages/cachekit/src/cache.test.ts b/packages/cachekit/src/cache.test.ts index eb1e9372..80024dcf 100644 --- a/packages/cachekit/src/cache.test.ts +++ b/packages/cachekit/src/cache.test.ts @@ -16,6 +16,7 @@ import { ValueTooLargeError, } from './errors.js'; import { forgedEnvelope } from '../test/fixtures/forged-envelope.js'; +import { turnsToSettle } from '../test/fixtures/settle.js'; import { MessagePackSerializer } from './serialization/serializer.js'; import { EncryptionManagerCore } from './encryption/manager-core.js'; import type { SecureCache, WrapOptionsBase } from './types/cache.js'; @@ -159,20 +160,6 @@ describe('Cache Integration', () => { // and awaiting even a settled promise costs the hit a microtask turn: the // promise a hit hands back must already be settled. describe('a plaintext L1 hit returns an already-settled promise', () => { - const turnsToSettle = async (promise: Promise): Promise => { - let settled = false; - promise.then( - () => (settled = true), - () => (settled = true) - ); - let turns = 0; - while (!settled) { - await null; - turns++; - } - return turns; - }; - it('from get() and exists()', async () => { const alreadySettled = await turnsToSettle(Promise.resolve()); await cache.set('test:settled', { data: 'value' }); diff --git a/packages/cachekit/src/encryption/manager-core.ts b/packages/cachekit/src/encryption/manager-core.ts index 96c8279d..b7fc67f1 100644 --- a/packages/cachekit/src/encryption/manager-core.ts +++ b/packages/cachekit/src/encryption/manager-core.ts @@ -6,6 +6,7 @@ import { MASTER_KEY_BYTES, MASTER_KEY_HEX_LENGTH, } from '../constants.js'; +import { isThenable } from '../utils/thenable.js'; const utf8 = new TextEncoder(); @@ -18,14 +19,6 @@ function aadComponent(text: string): Uint8Array { return component; } -/** - * Whether a binding returned a Promise. The NAPI bindings return plain values, - * and awaiting one would still cost a microtask hop on every Node operation. - */ -function isThenable(value: T | PromiseLike): value is PromiseLike { - return typeof (value as { then?: unknown }).then === 'function'; -} - /** The EncryptionError decrypt() builds for a binding failure, for decryptNow() to throw or reject with. */ function decryptionFailed(error: unknown): EncryptionError { return new EncryptionError( diff --git a/packages/cachekit/src/utils/thenable.ts b/packages/cachekit/src/utils/thenable.ts new file mode 100644 index 00000000..3dce231f --- /dev/null +++ b/packages/cachekit/src/utils/thenable.ts @@ -0,0 +1,9 @@ +/** + * Whether a value is a Promise or another thenable. The value-or-Promise paths + * test this, not `instanceof Promise`: an async function returns the engine's + * own Promise, which fails `instanceof` once a library (zone.js, Bluebird) has + * replaced the global Promise, and a Promise taken for a value is never awaited. + */ +export function isThenable(value: unknown): value is PromiseLike { + return typeof (value as { then?: unknown } | null | undefined)?.then === 'function'; +} diff --git a/packages/cachekit/test/fixtures/settle.ts b/packages/cachekit/test/fixtures/settle.ts new file mode 100644 index 00000000..9697a5e5 --- /dev/null +++ b/packages/cachekit/test/fixtures/settle.ts @@ -0,0 +1,18 @@ +/** + * Microtask turns until `promise` settles. A promise that was already settled + * when it was handed back takes as many as `Promise.resolve()`: each await on + * the way to a value costs one more. + */ +export async function turnsToSettle(promise: Promise): Promise { + let settled = false; + promise.then( + () => (settled = true), + () => (settled = true) + ); + let turns = 0; + while (!settled) { + await null; + turns++; + } + return turns; +} diff --git a/packages/cachekit/test/protocol/encryption.protocol.test.ts b/packages/cachekit/test/protocol/encryption.protocol.test.ts index c7c96ad4..4a46db12 100644 --- a/packages/cachekit/test/protocol/encryption.protocol.test.ts +++ b/packages/cachekit/test/protocol/encryption.protocol.test.ts @@ -571,8 +571,10 @@ describe('encryption.json decrypted_container (protocol 1.5.0, ENC-3) — the co describe('encryption.json aad_hex — the shipped AAD builder', () => { // The Workers lane proves the builder only end to end, by decrypting; this - // pins its bytes. Mock bindings capture the AAD the manager hands encrypt - // and decrypt, so no key material or native code is involved. + // pins its bytes. Mock bindings capture the AAD the manager hands encrypt, + // decrypt() and decryptNow() (the cache's read path once the keys are + // derived, which builds its own binding call), so no key material or native + // code is involved. async function managerAad( tenantId: string | undefined, cacheKey: string, @@ -596,6 +598,9 @@ describe('encryption.json aad_hex — the shipped AAD builder', () => { try { await manager.encrypt(new Uint8Array([1]), cacheKey, compressed); await manager.decrypt(new Uint8Array([1]), cacheKey, compressed); + expect(manager.decryptNow(new Uint8Array([1]), cacheKey, compressed)).toEqual( + new Uint8Array([2]) + ); } finally { manager.dispose(); } @@ -630,11 +635,12 @@ describe('encryption.json aad_hex — the shipped AAD builder', () => { }); it.each(rows.map((v) => [v.name, v] as const))( - '%s: encrypt and decrypt hand the binding aad_hex', + '%s: encrypt, decrypt() and decryptNow() hand the binding aad_hex', async (_name, v) => { expect(await managerAad(v.tenantId, v.cache_key, v.compressed)).toEqual([ v.aad_hex, v.aad_hex, + v.aad_hex, ]); } ); @@ -664,7 +670,11 @@ describe('encryption.json aad_hex — the shipped AAD builder', () => { 'tenant $tenantId, key $cacheKey, compressed=$compressed matches the v0x03 layout', async ({ tenantId, cacheKey, compressed }) => { const expected = layoutHex(tenantId, cacheKey, compressed); - expect(await managerAad(tenantId, cacheKey, compressed)).toEqual([expected, expected]); + expect(await managerAad(tenantId, cacheKey, compressed)).toEqual([ + expected, + expected, + expected, + ]); } ); }); From 6c7aea460f1d8d124fd4770297dbf6af439d72fb Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 03:07:40 +0000 Subject: [PATCH 4/7] perf(cachekit): keep the plaintext L1 hit free of the thenable check The thenable check ran on the plaintext branch of the get() and wrap() hit sites too, +9 instructions on get-l1-hit. Each site now decodes and checks only when the cache encrypts, as exists() already did. --- packages/cachekit/src/cache-core.ts | 35 ++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts index 291c4ada..3f04ae52 100644 --- a/packages/cachekit/src/cache-core.ts +++ b/packages/cachekit/src/cache-core.ts @@ -1275,11 +1275,13 @@ export class CacheImpl implements SecureCache { if (l1) { const l1Result = l1.get(key); if (l1Result !== null) { - // Awaited only when the decrypt suspends (see decodeL1Entry). - const read = this.encryption - ? this.decodeL1Entry(key, l1Result, interop) - : { value: l1Result as T }; - const decoded = isThenable(read) ? await read : read; + let decoded: L1Read; + if (!this.encryption) decoded = { value: l1Result as T }; + else { + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.decodeL1Entry(key, l1Result, interop); + decoded = isThenable(read) ? await read : read; + } if (decoded !== null) { if (probe) probe.servedBy = 'l1'; else this.recordHit('l1'); @@ -2118,11 +2120,13 @@ export class CacheImpl implements SecureCache { if (this.swrRequiresWaitUntil && !waitUntil) { const l1Value = this.l1.get(cacheKey); if (l1Value !== null) { - // Awaited only when the decrypt suspends (see decodeL1Entry). - const read = this.encryption - ? this.decodeL1Entry(cacheKey, l1Value, interop) - : { value: l1Value as TResult }; - const decoded = isThenable(read) ? await read : read; + let decoded: L1Read; + if (!this.encryption) decoded = { value: l1Value as TResult }; + else { + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.decodeL1Entry(cacheKey, l1Value, interop); + decoded = isThenable(read) ? await read : read; + } if (decoded !== null) { this.recordHit('l1'); if (timedFrom !== undefined) this.stopL1Timer(timedFrom); @@ -2141,11 +2145,12 @@ export class CacheImpl implements SecureCache { // about to recompute the same key. let decoded: L1Read = null; try { - // Awaited only when the decrypt suspends (see decodeL1Entry). - const read = this.encryption - ? this.decodeL1Entry(cacheKey, swrResult.value, interop) - : { value: swrResult.value as TResult }; - decoded = isThenable(read) ? await read : read; + if (!this.encryption) decoded = { value: swrResult.value as TResult }; + else { + // Awaited only when the decrypt suspends (see decodeL1Entry). + const read = this.decodeL1Entry(cacheKey, swrResult.value, interop); + decoded = isThenable(read) ? await read : read; + } } finally { // Decode failed — by rethrow (degradation off) or by null // (degradation on) — so the entry is dropped either way, and the From d3d4e04f0067c9b6dbd40bf7d2f4300fb9c32118 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 03:13:45 +0000 Subject: [PATCH 5/7] test(cachekit): type the suspending bindings' key handle instead of casting it --- packages/cachekit/src/cache.encryption-l1.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts index e5e416bf..b774317e 100644 --- a/packages/cachekit/src/cache.encryption-l1.test.ts +++ b/packages/cachekit/src/cache.encryption-l1.test.ts @@ -403,10 +403,10 @@ describe('an encrypted L1 hit awaits only a decrypt that suspends', () => { suspends ? new EncryptionManagerCore(config.masterKey, config.tenantId, async () => ({ deriveTenantKeys, - encryptWithTenantKeys: async (plaintext, aad, keys) => - encryptWithTenantKeys(plaintext, aad, keys as TenantKeys), - decryptWithTenantKeys: async (ciphertext, aad, keys) => - decryptWithTenantKeys(ciphertext, aad, keys as TenantKeys), + encryptWithTenantKeys: async (plaintext, aad, keys: TenantKeys) => + encryptWithTenantKeys(plaintext, aad, keys), + decryptWithTenantKeys: async (ciphertext, aad, keys: TenantKeys) => + decryptWithTenantKeys(ciphertext, aad, keys), })) : new EncryptionManager(config.masterKey, config.tenantId), swrRequiresWaitUntil, From 9fd45bdfc1605207b33773a0f5438e3011dbf089 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 03:24:19 +0000 Subject: [PATCH 6/7] test(cachekit): drop a replaced-Promise row exists() passes either way; say why the thenable check precedes the await --- .../cachekit/src/cache.encryption-l1.test.ts | 16 ++++++++-------- packages/cachekit/src/utils/thenable.ts | 9 ++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts index b774317e..e3b24638 100644 --- a/packages/cachekit/src/cache.encryption-l1.test.ts +++ b/packages/cachekit/src/cache.encryption-l1.test.ts @@ -519,17 +519,17 @@ describe('an encrypted L1 hit awaits only a decrypt that suspends', () => { type Read = (cache: SecureCache, load: (id: number) => Promise) => Promise; const KEY = generateKey('users', [1]); const getKey: Read = (cache) => cache.get(KEY); - const existsKey: Read = (cache) => cache.exists(KEY); const wrapped: Read = (_, load) => load(1); + // exists() would report present off an unawaited Promise too, so the test + // after this one pins it with an entry that must not decode. it.each([ - ['get() from L1', getKey, VALUE, true, false], - ['get() from L2', getKey, VALUE, false, false], - ['exists() from L1', existsKey, true, true, false], - ['wrap() with SWR from L1', wrapped, VALUE, true, false], - ['wrap() with no waitUntil bound from L1', wrapped, VALUE, true, true], + ['get() from L1', getKey, true, false], + ['get() from L2', getKey, false, false], + ['wrap() with SWR from L1', wrapped, true, false], + ['wrap() with no waitUntil bound from L1', wrapped, true, true], ] as const)( '%s decodes when a library has replaced the global Promise, as zone.js does', - async (_, read, expected, fromL1, noWaitUntil) => { + async (_, read, fromL1, noWaitUntil) => { const backend = new InMemoryBackend(); await makeCache(backend).set(KEY, VALUE); const fresh = makeCache(backend, { swrRequiresWaitUntil: noWaitUntil }); @@ -539,7 +539,7 @@ describe('an encrypted L1 hit awaits only a decrypt that suspends', () => { ttl: 3600, }); const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - expect(await withReplacedPromise(() => read(fresh, load))).toEqual(expected); + expect(await withReplacedPromise(() => read(fresh, load))).toEqual(VALUE); expect(consoleSpy).not.toHaveBeenCalled(); } ); diff --git a/packages/cachekit/src/utils/thenable.ts b/packages/cachekit/src/utils/thenable.ts index 3dce231f..5309b9da 100644 --- a/packages/cachekit/src/utils/thenable.ts +++ b/packages/cachekit/src/utils/thenable.ts @@ -1,8 +1,11 @@ /** * Whether a value is a Promise or another thenable. The value-or-Promise paths - * test this, not `instanceof Promise`: an async function returns the engine's - * own Promise, which fails `instanceof` once a library (zone.js, Bluebird) has - * replaced the global Promise, and a Promise taken for a value is never awaited. + * check this before awaiting, because awaiting even a plain value costs a + * microtask turn, which on Node.js every encrypt, decrypt and L1 hit would pay. + * They test this, not `instanceof Promise`: an async function returns the + * engine's own Promise, which fails `instanceof` once a library (zone.js, + * Bluebird) has replaced the global Promise, and a Promise taken for a value is + * never awaited. */ export function isThenable(value: unknown): value is PromiseLike { return typeof (value as { then?: unknown } | null | undefined)?.then === 'function'; From 0409a5c9fa3183174c374778563c9c8762941a0d Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 7 Nov 2026 03:29:30 +0000 Subject: [PATCH 7/7] test(cachekit): bound the settle-turn loop so a timer-bound read fails instead of hanging --- packages/cachekit/src/utils/thenable.ts | 3 ++- packages/cachekit/test/fixtures/settle.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/cachekit/src/utils/thenable.ts b/packages/cachekit/src/utils/thenable.ts index 5309b9da..46138daf 100644 --- a/packages/cachekit/src/utils/thenable.ts +++ b/packages/cachekit/src/utils/thenable.ts @@ -1,7 +1,8 @@ /** * Whether a value is a Promise or another thenable. The value-or-Promise paths * check this before awaiting, because awaiting even a plain value costs a - * microtask turn, which on Node.js every encrypt, decrypt and L1 hit would pay. + * microtask turn, which on Node.js every encrypt, decrypt and encrypted L1 hit + * would pay. * They test this, not `instanceof Promise`: an async function returns the * engine's own Promise, which fails `instanceof` once a library (zone.js, * Bluebird) has replaced the global Promise, and a Promise taken for a value is diff --git a/packages/cachekit/test/fixtures/settle.ts b/packages/cachekit/test/fixtures/settle.ts index 9697a5e5..74456eae 100644 --- a/packages/cachekit/test/fixtures/settle.ts +++ b/packages/cachekit/test/fixtures/settle.ts @@ -1,7 +1,12 @@ +const MAX_TURNS = 1_000; + /** * Microtask turns until `promise` settles. A promise that was already settled * when it was handed back takes as many as `Promise.resolve()`: each await on * the way to a value costs one more. + * + * Throws after MAX_TURNS: a promise that waits on a timer or I/O never settles + * while this loop holds the microtask queue, and nor would the test's timeout. */ export async function turnsToSettle(promise: Promise): Promise { let settled = false; @@ -11,6 +16,11 @@ export async function turnsToSettle(promise: Promise): Promise ); let turns = 0; while (!settled) { + if (turns === MAX_TURNS) { + throw new Error( + `still pending after ${MAX_TURNS} microtask turns: it waits on a timer or I/O` + ); + } await null; turns++; }