From 24f47478ba8beecd9eb4d911484b4d5a060fb02e Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Thu, 17 Sep 2026 07:05:16 +1000 Subject: [PATCH 1/3] fix(cache)!: secure.wrap() fails closed when encryption is not configured (LAB-513) Both `cache.secure.wrap()` and the `withExecutionContext(ctx)` view's `secure.wrap()` were bare delegates to `wrap()`. Every intent is typed `SecureCache`, so on a cache built without `encryption` a "secure" registration silently cached plaintext (CWE-311) while the interface doc claimed it "always encrypts". One private guard, `CacheImpl.secureWrap`, now throws `ConfigurationError` at wrap time when no encryption manager is configured; both sites delegate to it. Encrypted caches are unchanged. Matches cachekit-py (raises at decoration time) and cachekit-rs (`secure()` returns `Err`). Deliberately no opt-in to run unencrypted: callers who want plaintext call `wrap()`. BREAKING CHANGE: `cache.secure.wrap()` and `withExecutionContext(ctx).secure.wrap()` now throw `ConfigurationError` at wrap time when the cache has no `encryption` configured, instead of silently caching plaintext. Use `createCache.secure()` / pass `encryption`, or call `wrap()` for unencrypted caching. --- packages/cachekit/src/cache-core.ts | 32 ++++- .../cachekit/src/cache.secure-wrap.test.ts | 110 ++++++++++++++++++ packages/cachekit/src/types/cache.ts | 6 +- 3 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 packages/cachekit/src/cache.secure-wrap.test.ts diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts index 67fd916..4d6afe7 100644 --- a/packages/cachekit/src/cache-core.ts +++ b/packages/cachekit/src/cache-core.ts @@ -1197,11 +1197,35 @@ export class CacheImpl implements SecureCache { wrap: ( fn: (...args: TArgs) => Promise, options: WrapOptions - ): ((...args: TArgs) => Promise) => { - return this.wrap(fn, options); - }, + ): ((...args: TArgs) => Promise) => this.secureWrap(fn, options), }; + /** + * `secure.wrap` for both the instance and the `withExecutionContext` view. + * Fails closed at wrap time: a cache built without `encryption` (plain + * `createCache({ backend })`, or the `minimal` / `production` / `io` + * intents — all typed `SecureCache`, so `.secure` is always present) used + * to store plaintext here with no error, warning, or type error (LAB-513, + * CWE-311). Python raises at decoration time and Rust's `secure()` returns + * `Err`; this is the same contract. Deliberately no opt-in to run + * unencrypted — any escape hatch under a security-labelled path is the + * downgrade this guard exists to close. Plaintext callers use `wrap()`. + */ + private secureWrap( + fn: (...args: TArgs) => Promise, + options: WrapOptions, + waitUntil?: WaitUntil + ): (...args: TArgs) => Promise { + if (!this.encryption) { + throw new ConfigurationError( + 'cache.secure.wrap() requires encryption, but this cache has none configured. ' + + 'Create it with createCache.secure() or pass `encryption` in CacheOptions; ' + + 'for unencrypted caching call cache.wrap() instead.' + ); + } + return this.wrap(fn, options, waitUntil); + } + /** * Bind a request's execution context, returning a request-scoped view of * this cache whose SWR background refreshes are registered with the @@ -1234,7 +1258,7 @@ export class CacheImpl implements SecureCache { exists: (key) => this.exists(key), wrap: wrapWith, with: (options) => (fn) => wrapWith(fn, options), - secure: { wrap: wrapWith }, + secure: { wrap: (fn, options) => this.secureWrap(fn, options, waitUntil) }, invalidate: (level, options) => this.invalidate(level, options), close: () => this.close(), }; diff --git a/packages/cachekit/src/cache.secure-wrap.test.ts b/packages/cachekit/src/cache.secure-wrap.test.ts new file mode 100644 index 0000000..d2c25bf --- /dev/null +++ b/packages/cachekit/src/cache.secure-wrap.test.ts @@ -0,0 +1,110 @@ +/** + * LAB-513 regression: `secure.wrap()` must never cache plaintext. + * + * Both `cache.secure.wrap` and the request-scoped + * `cache.withExecutionContext(ctx).secure.wrap` used to be bare delegates to + * `wrap()`. Every intent is typed `SecureCache`, so on a cache built without + * `encryption` a "secure" registration silently stored plaintext (CWE-311). + * cachekit-py raises at decoration time and cachekit-rs's `secure()` returns + * `Err`; TypeScript now throws `ConfigurationError` at wrap time at both sites. + * The view is exercised here in the Node lane because the guard lives in the + * shared CacheImpl — the Workers entrypoint reuses the same method. + */ + +import { createHash } from 'node:crypto'; +import { describe, it, expect, afterEach } from 'vitest'; +import { createCache } from './cache.js'; +import { ConfigurationError } from './errors.js'; +import type { CacheImpl } from './cache-core.js'; +import type { SecureCache } from './types/cache.js'; +import type { Backend } from './backends/types.js'; + +// Derived at runtime from a public fixture string — not a key literal a +// secret scanner should match. No assertion depends on its value. +const MASTER_KEY = createHash('sha256').update('cachekit LAB-513 test fixture').digest('hex'); +/** Distinctive enough that a substring search over stored bytes is conclusive. */ +const CANARY = 'ssn-000-00-0000-do-not-leak'; +const OPTIONS = { namespace: 'patients:records', ttl: 300 }; + +class InMemoryBackend implements Backend { + store = new Map(); + + async get(key: string): Promise { + 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 Workers request-scoped view. The method lives on CacheImpl, not on the Node type. */ +function viewOf(cache: SecureCache): SecureCache { + return (cache as unknown as CacheImpl).withExecutionContext({ waitUntil: () => {} }); +} + +describe('secure.wrap() fails closed without encryption (LAB-513)', () => { + const caches: SecureCache[] = []; + + function makeCache(encrypted: boolean): { cache: SecureCache; backend: InMemoryBackend } { + const backend = new InMemoryBackend(); + const cache = createCache({ + backend, + defaultTtl: 3600, + ...(encrypted ? { encryption: { masterKey: MASTER_KEY } } : {}), + }); + caches.push(cache); + return { cache, backend }; + } + + afterEach(async () => { + await Promise.all(caches.splice(0).map((c) => c.close())); + }); + + const sites: Array<[string, (cache: SecureCache) => SecureCache['secure']]> = [ + ['cache.secure', (cache) => cache.secure], + ['cache.withExecutionContext(ctx).secure', (cache) => viewOf(cache).secure], + ]; + + describe.each(sites)('%s', (_site, secureOf) => { + it('throws ConfigurationError at wrap time, before the function is ever called', () => { + const { cache, backend } = makeCache(false); + let calls = 0; + const fn = async (id: string) => { + calls++; + return { id, ssn: CANARY }; + }; + + expect(() => secureOf(cache).wrap(fn, OPTIONS)).toThrow(ConfigurationError); + expect(() => secureOf(cache).wrap(fn, OPTIONS)).toThrow(/createCache\.secure\(\)/); + expect(calls).toBe(0); + expect(backend.store.size).toBe(0); + }); + + it('passes through to wrap() when encryption is configured and stores only ciphertext', async () => { + const { cache, backend } = makeCache(true); + const getRecord = secureOf(cache).wrap(async (id: string) => ({ id, ssn: CANARY }), OPTIONS); + + expect(await getRecord('p1')).toEqual({ id: 'p1', ssn: CANARY }); + // Second call is a hit and still decrypts to the same value. + expect(await getRecord('p1')).toEqual({ id: 'p1', ssn: CANARY }); + + expect(backend.store.size).toBe(1); + for (const bytes of backend.store.values()) { + expect(new TextDecoder().decode(bytes)).not.toContain(CANARY); + } + }); + }); + + it('plain wrap() on an unencrypted cache is unaffected', async () => { + const { cache } = makeCache(false); + const getRecord = cache.wrap(async (id: string) => ({ id }), OPTIONS); + expect(await getRecord('p1')).toEqual({ id: 'p1' }); + }); +}); diff --git a/packages/cachekit/src/types/cache.ts b/packages/cachekit/src/types/cache.ts index 58bb272..e1d9ae5 100644 --- a/packages/cachekit/src/types/cache.ts +++ b/packages/cachekit/src/types/cache.ts @@ -343,7 +343,11 @@ export interface Cache { */ export interface SecureCache extends Cache { /** - * Secure version of wrap that always encrypts. + * Encrypting version of `wrap`. Fails closed: throws `ConfigurationError` + * at wrap time — not on first call — when the cache has no `encryption` + * configured, so a function registered as secure can never cache + * plaintext. With encryption configured it behaves exactly like `wrap`. + * There is no option to run it unencrypted; use `wrap` for that. */ secure: { wrap( From 5f53dc82a3c8cb234955c2db03a4daf0c49b2d31 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Thu, 17 Sep 2026 07:20:58 +1000 Subject: [PATCH 2/3] test(cache): pin secure.wrap waitUntil plumbing and de-fang LZ4 in the ciphertext probe (LAB-513) Review findings on the first commit: - The view's secure.wrap used to BE wrapWith and inherited the Workers SWR coverage; as its own closure, dropping the third argument passed every test while silently disabling SWR for secure wrappers on Workers. A spy on CacheImpl.prototype.wrap now pins the handle through to ctx.waitUntil. - With default compression on, LZ4 alone removed the canary substring from the stored bytes, so the "stores only ciphertext" assertion also passed on an unencrypted store. Compression is off in this suite and the plain wrap() case now doubles as the plaintext control. - SecureCache's outer doc said "with encryption" though every intent returns it; the secureWrap JSDoc repeated history that lives in the commit and test header. Both trimmed to the contract. `secure` is typed off the interface instead of restating the generics. --- packages/cachekit/src/cache-core.ts | 23 +++----- .../cachekit/src/cache.secure-wrap.test.ts | 58 ++++++++++++++----- packages/cachekit/src/types/cache.ts | 5 +- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts index 4d6afe7..adfa24a 100644 --- a/packages/cachekit/src/cache-core.ts +++ b/packages/cachekit/src/cache-core.ts @@ -1193,23 +1193,16 @@ export class CacheImpl implements SecureCache { this.wrap(fn, options); } - secure = { - wrap: ( - fn: (...args: TArgs) => Promise, - options: WrapOptions - ): ((...args: TArgs) => Promise) => this.secureWrap(fn, options), - }; + secure: SecureCache['secure'] = { wrap: (fn, options) => this.secureWrap(fn, options) }; /** - * `secure.wrap` for both the instance and the `withExecutionContext` view. - * Fails closed at wrap time: a cache built without `encryption` (plain - * `createCache({ backend })`, or the `minimal` / `production` / `io` - * intents — all typed `SecureCache`, so `.secure` is always present) used - * to store plaintext here with no error, warning, or type error (LAB-513, - * CWE-311). Python raises at decoration time and Rust's `secure()` returns - * `Err`; this is the same contract. Deliberately no opt-in to run - * unencrypted — any escape hatch under a security-labelled path is the - * downgrade this guard exists to close. Plaintext callers use `wrap()`. + * Both `secure.wrap` sites (instance and `withExecutionContext` view) route + * here. Fails closed at wrap time: every intent is typed `SecureCache`, so + * `.secure` exists on caches with no `encryption` configured, and this guard + * is all that stands between a "secure" registration and plaintext at rest + * (LAB-513). Deliberately no opt-in to run unencrypted — an escape hatch + * under a security-labelled path is the downgrade this closes. Plaintext + * callers use `wrap()`. */ private secureWrap( fn: (...args: TArgs) => Promise, diff --git a/packages/cachekit/src/cache.secure-wrap.test.ts b/packages/cachekit/src/cache.secure-wrap.test.ts index d2c25bf..2baf083 100644 --- a/packages/cachekit/src/cache.secure-wrap.test.ts +++ b/packages/cachekit/src/cache.secure-wrap.test.ts @@ -12,10 +12,10 @@ */ import { createHash } from 'node:crypto'; -import { describe, it, expect, afterEach } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import { createCache } from './cache.js'; import { ConfigurationError } from './errors.js'; -import type { CacheImpl } from './cache-core.js'; +import { CacheImpl, type ExecutionContextLike } from './cache-core.js'; import type { SecureCache } from './types/cache.js'; import type { Backend } from './backends/types.js'; @@ -45,8 +45,11 @@ class InMemoryBackend implements Backend { } /** The Workers request-scoped view. The method lives on CacheImpl, not on the Node type. */ -function viewOf(cache: SecureCache): SecureCache { - return (cache as unknown as CacheImpl).withExecutionContext({ waitUntil: () => {} }); +function viewOf( + cache: SecureCache, + ctx: ExecutionContextLike = { waitUntil: () => {} } +): SecureCache { + return (cache as unknown as CacheImpl).withExecutionContext(ctx); } describe('secure.wrap() fails closed without encryption (LAB-513)', () => { @@ -57,6 +60,12 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { const cache = createCache({ backend, defaultTtl: 3600, + // LZ4 alone already hides the canary substring in the stored bytes (a + // match token lands inside "000-00-0000"), which would let an + // unencrypted store pass the ciphertext assertion below. With + // compression off, only AES-GCM stands between MessagePack and the + // backend, so "canary absent" means "encrypted" and nothing else. + compression: false, ...(encrypted ? { encryption: { masterKey: MASTER_KEY } } : {}), }); caches.push(cache); @@ -64,6 +73,7 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { } afterEach(async () => { + vi.restoreAllMocks(); await Promise.all(caches.splice(0).map((c) => c.close())); }); @@ -74,17 +84,11 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { describe.each(sites)('%s', (_site, secureOf) => { it('throws ConfigurationError at wrap time, before the function is ever called', () => { - const { cache, backend } = makeCache(false); - let calls = 0; - const fn = async (id: string) => { - calls++; - return { id, ssn: CANARY }; - }; + const { cache } = makeCache(false); + const fn = async (id: string) => ({ id, ssn: CANARY }); expect(() => secureOf(cache).wrap(fn, OPTIONS)).toThrow(ConfigurationError); expect(() => secureOf(cache).wrap(fn, OPTIONS)).toThrow(/createCache\.secure\(\)/); - expect(calls).toBe(0); - expect(backend.store.size).toBe(0); }); it('passes through to wrap() when encryption is configured and stores only ciphertext', async () => { @@ -102,9 +106,31 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { }); }); - it('plain wrap() on an unencrypted cache is unaffected', async () => { - const { cache } = makeCache(false); - const getRecord = cache.wrap(async (id: string) => ({ id }), OPTIONS); - expect(await getRecord('p1')).toEqual({ id: 'p1' }); + it('forwards the request waitUntil handle to wrap() on the view (Workers SWR)', () => { + const { cache } = makeCache(true); + const ctx: ExecutionContextLike = { waitUntil: vi.fn() }; + const wrapSpy = vi.spyOn(CacheImpl.prototype, 'wrap'); + + viewOf(cache, ctx).secure.wrap(async (id: string) => ({ id }), OPTIONS); + + // Before LAB-513 the view's secure.wrap WAS wrapWith and inherited its + // waitUntil; now it is its own closure, so pin the plumbing explicitly. + const handle = wrapSpy.mock.lastCall?.[2]; + expect(handle).toBeTypeOf('function'); + handle?.(Promise.resolve()); + expect(ctx.waitUntil).toHaveBeenCalledOnce(); + }); + + it('plain wrap() on an unencrypted cache is unaffected — and is the plaintext control', async () => { + const { cache, backend } = makeCache(false); + const getRecord = cache.wrap(async (id: string) => ({ id, ssn: CANARY }), OPTIONS); + expect(await getRecord('p1')).toEqual({ id: 'p1', ssn: CANARY }); + + // Proves the canary probe can see plaintext when it is there, so the + // ciphertext assertions above are not vacuous. + expect(backend.store.size).toBe(1); + for (const bytes of backend.store.values()) { + expect(new TextDecoder().decode(bytes)).toContain(CANARY); + } }); }); diff --git a/packages/cachekit/src/types/cache.ts b/packages/cachekit/src/types/cache.ts index e1d9ae5..78c93f4 100644 --- a/packages/cachekit/src/types/cache.ts +++ b/packages/cachekit/src/types/cache.ts @@ -338,8 +338,9 @@ export interface Cache { } /** - * Secure cache interface with encryption. - * Extends Cache with secure-only wrap method. + * `Cache` plus `secure.wrap`. Every `createCache()` call and intent returns + * this type whether or not `encryption` is configured; encryption is enforced + * by `secure.wrap` at wrap time, not by the type. */ export interface SecureCache extends Cache { /** From b3e44bc57cb469b1cb6ada1ae347fb552a25ff9a Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Thu, 17 Sep 2026 07:27:45 +1000 Subject: [PATCH 3/3] test(cache): drive the SWR refresh through view.secure.wrap; narrow with instanceof (LAB-513) Replaces the CacheImpl.prototype.wrap spy with the real stale-while-revalidate path: an always-stale L1 on an encrypted cache, wrapped through withExecutionContext(ctx).secure.wrap, must hand its refresh promise to ctx.waitUntil and that promise must be the recompute. Fails when the waitUntil forward in secureWrap is dropped. The viewOf helper narrows with instanceof CacheImpl instead of a double cast. --- .../cachekit/src/cache.secure-wrap.test.ts | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/packages/cachekit/src/cache.secure-wrap.test.ts b/packages/cachekit/src/cache.secure-wrap.test.ts index 2baf083..962a2be 100644 --- a/packages/cachekit/src/cache.secure-wrap.test.ts +++ b/packages/cachekit/src/cache.secure-wrap.test.ts @@ -16,7 +16,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { createCache } from './cache.js'; import { ConfigurationError } from './errors.js'; import { CacheImpl, type ExecutionContextLike } from './cache-core.js'; -import type { SecureCache } from './types/cache.js'; +import type { CacheOptions, SecureCache } from './types/cache.js'; import type { Backend } from './backends/types.js'; // Derived at runtime from a public fixture string — not a key literal a @@ -25,6 +25,8 @@ const MASTER_KEY = createHash('sha256').update('cachekit LAB-513 test fixture'). /** Distinctive enough that a substring search over stored bytes is conclusive. */ const CANARY = 'ssn-000-00-0000-do-not-leak'; const OPTIONS = { namespace: 'patients:records', ttl: 300 }; +/** Threshold ratio above 1 makes every L1 entry stale on its first hit. */ +const ALWAYS_STALE_L1 = { swrEnabled: true, swrThresholdRatio: 2 }; class InMemoryBackend implements Backend { store = new Map(); @@ -49,13 +51,19 @@ function viewOf( cache: SecureCache, ctx: ExecutionContextLike = { waitUntil: () => {} } ): SecureCache { - return (cache as unknown as CacheImpl).withExecutionContext(ctx); + if (!(cache instanceof CacheImpl)) { + throw new Error('createCache() returned something other than CacheImpl'); + } + return cache.withExecutionContext(ctx); } describe('secure.wrap() fails closed without encryption (LAB-513)', () => { const caches: SecureCache[] = []; - function makeCache(encrypted: boolean): { cache: SecureCache; backend: InMemoryBackend } { + function makeCache( + encrypted: boolean, + l1?: CacheOptions['l1'] + ): { cache: SecureCache; backend: InMemoryBackend } { const backend = new InMemoryBackend(); const cache = createCache({ backend, @@ -66,6 +74,7 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { // compression off, only AES-GCM stands between MessagePack and the // backend, so "canary absent" means "encrypted" and nothing else. compression: false, + ...(l1 ? { l1 } : {}), ...(encrypted ? { encryption: { masterKey: MASTER_KEY } } : {}), }); caches.push(cache); @@ -73,7 +82,6 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { } afterEach(async () => { - vi.restoreAllMocks(); await Promise.all(caches.splice(0).map((c) => c.close())); }); @@ -106,19 +114,26 @@ describe('secure.wrap() fails closed without encryption (LAB-513)', () => { }); }); - it('forwards the request waitUntil handle to wrap() on the view (Workers SWR)', () => { - const { cache } = makeCache(true); - const ctx: ExecutionContextLike = { waitUntil: vi.fn() }; - const wrapSpy = vi.spyOn(CacheImpl.prototype, 'wrap'); - - viewOf(cache, ctx).secure.wrap(async (id: string) => ({ id }), OPTIONS); - + it('hands the SWR refresh to the request waitUntil through view.secure.wrap (Workers contract)', async () => { // Before LAB-513 the view's secure.wrap WAS wrapWith and inherited its - // waitUntil; now it is its own closure, so pin the plumbing explicitly. - const handle = wrapSpy.mock.lastCall?.[2]; - expect(handle).toBeTypeOf('function'); - handle?.(Promise.resolve()); + // waitUntil plumbing; now it is its own closure, so drive the real + // stale-while-revalidate path and observe the handle being used. + const { cache } = makeCache(true, ALWAYS_STALE_L1); + const ctx = { waitUntil: vi.fn<(refresh: Promise) => void>() }; + let calls = 0; + const getRecord = viewOf(cache, ctx).secure.wrap( + async (id: string) => ({ id, gen: ++calls }), + OPTIONS + ); + + expect(await getRecord('p1')).toEqual({ id: 'p1', gen: 1 }); // miss: compute + store + expect(await getRecord('p1')).toEqual({ id: 'p1', gen: 1 }); // stale hit: serve, schedule refresh + expect(ctx.waitUntil).toHaveBeenCalledOnce(); + const refresh = ctx.waitUntil.mock.calls[0][0]; + expect(refresh).toBeInstanceOf(Promise); + await refresh; + expect(calls).toBe(2); // the promise handed to ctx was the refresh itself }); it('plain wrap() on an unencrypted cache is unaffected — and is the plaintext control', async () => {