From ecfd91260bc9ebeac46c1bc96e58f74ed67c10de Mon Sep 17 00:00:00 2001
From: Ray Walker <ray@27b.io>
Date: Sat, 8 Aug 2026 01:09:49 +1000
Subject: [PATCH 1/5] fix(security): store ciphertext in L1 for encrypted
 caches (LAB-238)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

L1 held post-decrypt plaintext for encrypted caches, so any heap dump, core
dump, or Node diagnostic report yielded the entire L1 working set in the clear
for its full TTL — and that plaintext outlived the key zeroization in close(),
because L1 entries are held independently of tenant keys. It also broke parity
with cachekit-py, whose L1Cache stores bytes and decrypts at read time, and
cachekit-rs, which keeps ciphertext across every layer.

All three population sites now store what L2 stores:

- getEntry() writes the backend bytes it read, not the value it decoded
- setEntry() writes the ciphertext it produced, not the caller's value
- the SWR refresh writes what the L2 write handed back, not the factory result

To close the third site, the persist callback returns an L1Write ({ l1 })
instead of void. The wrapper is what distinguishes "store this" from "there is
nothing to store": a degraded write on a secure cache has no verified
ciphertext to show for itself, so the refresh cancels and L1 keeps the stale
entry rather than falling back to the plaintext it just computed. Plaintext
caches still get their value back on a degraded write, so their SWR behaviour
is unchanged.

Every L1 hit path — get, wrap's SWR read, and wrap's no-waitUntil fallback —
decrypts and AAD-verifies against the cache key. An entry that fails to verify
is dropped and the read falls through to L2, mirroring cachekit-py's L1
handler, which invalidates before applying its fail policy so a poisoned L1
copy cannot outlive remediation of L2. The failure is logged, not swallowed.

Two hazards that came with holding bytes in L1:

- estimateSize measured with JSON.stringify, which renders a Uint8Array as
  {"0":171,...} — ~14x its real size. Unfixed, the first encrypted entry would
  have evicted most of L1.
- a Node Buffer from the backend is a window onto a shared 8 KiB pool slab, so
  retaining one for the entry's TTL pins the slab. Copy when the view is
  narrower than its buffer; cachekit-py refuses memoryview/bytearray in
  L1Cache.put for the same reason.

Non-encrypted caches keep storing decoded values, unchanged.
---
 packages/cachekit/README.md                   |   5 +-
 packages/cachekit/src/cache-core.ts           | 198 ++++++++++++++----
 .../cachekit/src/cache.encryption-l1.test.ts  | 194 +++++++++++++++++
 .../src/cache/background-refresh.test.ts      |  59 +++++-
 .../cachekit/src/cache/background-refresh.ts  |  43 +++-
 packages/cachekit/src/l1/lru-cache.test.ts    |  19 ++
 packages/cachekit/src/l1/lru-cache.ts         |   6 +
 7 files changed, 471 insertions(+), 53 deletions(-)
 create mode 100644 packages/cachekit/src/cache.encryption-l1.test.ts

diff --git a/packages/cachekit/README.md b/packages/cachekit/README.md
index 37f8d8a..13aa52b 100644
--- a/packages/cachekit/README.md
+++ b/packages/cachekit/README.md
@@ -116,7 +116,10 @@ const cache = createCache({
     maxMemory: 50 * 1024 * 1024, // 50MB
   },
 
-  // Optional: Client-side encryption
+  // Optional: Client-side encryption. Zero-knowledge covers every layer —
+  // L1 holds the same ciphertext L2 does, so an L1 hit costs a decrypt and
+  // AAD verify rather than being free, and no plaintext is resident in the
+  // heap between reads. Matches cachekit-py and cachekit-rs.
   encryption: {
     masterKey: process.env.CACHEKIT_MASTER_KEY!, // hex-encoded, 32+ bytes
     tenantId: 'tenant-123', // for multi-tenant key isolation
diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts
index 71bdbe5..7a15337 100644
--- a/packages/cachekit/src/cache-core.ts
+++ b/packages/cachekit/src/cache-core.ts
@@ -14,7 +14,11 @@ import type { MetricsCollector, MetricsConfig } from './metrics/prometheus.js';
 import { logError } from './logger.js';
 import { L1Cache } from './l1/lru-cache.js';
 import { ReliabilityExecutor } from './reliability/executor.js';
-import { BackgroundRefreshManager, type WaitUntil } from './cache/background-refresh.js';
+import {
+  BackgroundRefreshManager,
+  type WaitUntil,
+  type L1Write,
+} from './cache/background-refresh.js';
 import { MessagePackSerializer } from './serialization/serializer.js';
 import {
   generateKey,
@@ -380,6 +384,73 @@ export class CacheImpl implements SecureCache {
     return this.getEntry(key, false);
   }
 
+  /**
+   * Does this entry ride the ByteStorage envelope? Interop entries never do —
+   * they are plain MessagePack with AAD compressed=False regardless of the
+   * cache-level `compression` option, so py/rs can read them byte-for-byte.
+   */
+  private useEnvelope(interop: boolean): boolean {
+    return !interop && this.byteStorage !== null;
+  }
+
+  /**
+   * What L1 should hold for an entry: the same ciphertext L2 holds when the
+   * cache is encrypted, the decoded value otherwise.
+   *
+   * Zero-knowledge is a property of every layer, not just the backend
+   * (LAB-238) — it is what cachekit-py does (its L1Cache stores bytes and
+   * decrypts at read time) and cachekit-rs with it. Holding post-decrypt
+   * plaintext here would put the entire L1 working set into any heap dump,
+   * core dump, or Node diagnostic report for the full TTL, and that plaintext
+   * would outlive the key zeroization in close(), since L1 entries are held
+   * independently of tenant keys.
+   *
+   * The bytes are copied when they are a window into a larger buffer: a Node
+   * Buffer from the backend is a view onto a shared 8 KiB pool slab, and
+   * retaining one for the entry's TTL pins the whole slab. cachekit-py guards
+   * the same edge by refusing memoryview/bytearray in L1Cache.put.
+   */
+  private l1Payload<T>(value: T, bytes: Uint8Array): unknown {
+    if (!this.encryption) return value;
+    return bytes.byteLength === bytes.buffer.byteLength ? bytes : new Uint8Array(bytes);
+  }
+
+  /**
+   * Decode a value served from L1. For a secure cache that is a decrypt +
+   * AAD-verify against the cache key followed by the same unpack/deserialize
+   * the L2 path runs — an L1 hit is no longer free, which is the price of not
+   * keeping plaintext resident. For a plaintext cache it is a cast.
+   *
+   * Returns null when a secure entry will not decrypt (rotated key, tampered
+   * heap, an entry written under a different envelope mode). The entry is
+   * dropped first so a poisoned L1 copy cannot outlive remediation of L2, and
+   * the caller falls through to L2, which re-verifies against the same key and
+   * either repopulates L1 or fails on its own path. This mirrors cachekit-py's
+   * L1 handler, which invalidates before applying its fail policy. The failure
+   * is logged rather than swallowed: on this SDK an L2 decrypt failure is
+   * already absorbed by the degradation layer, so a throw here would make an
+   * L1 hit noisier than the same corruption seen one layer down.
+   */
+  private async readL1<T>(key: string, stored: unknown, interop: boolean): Promise<T | null> {
+    if (!this.encryption) return stored as T;
+
+    try {
+      const useEnvelope = this.useEnvelope(interop);
+      let plaintext = await this.encryption.decrypt(stored as Uint8Array, key, useEnvelope);
+      if (useEnvelope) {
+        plaintext = this.byteStorage!.unpack(plaintext);
+      }
+      return interop ? decodeInteropValue<T>(plaintext) : this.serializer.decode<T>(plaintext);
+    } catch (error) {
+      this.l1?.invalidateByKey(key);
+      logError(
+        '[cachekit] L1 decrypt failed — entry dropped, falling through to L2:',
+        error instanceof Error ? error.message : 'Unknown error'
+      );
+      return null;
+    }
+  }
+
   /**
    * L1 + L2 read. Interop entries (interop=true) are plain MessagePack —
    * no ByteStorage envelope and AAD compressed=False — regardless of the
@@ -390,16 +461,21 @@ export class CacheImpl implements SecureCache {
   private async getEntry<T>(key: string, interop: boolean, ttlSeconds?: number): Promise<T | null> {
     this.ensureNotClosed();
 
-    // Check L1 first
+    // Check L1 first. A secure cache holds ciphertext here, so the hit costs a
+    // decrypt + AAD verify; an entry that fails to verify is dropped and this
+    // falls through to L2 rather than serving or throwing.
     if (this.l1) {
       const l1Result = this.l1.get(key);
       if (l1Result !== null) {
-        this.recordHit('l1');
-        return l1Result as T;
+        const decoded = await this.readL1<T>(key, l1Result, interop);
+        if (decoded !== null) {
+          this.recordHit('l1');
+          return decoded;
+        }
       }
     }
 
-    const useEnvelope = !interop && this.byteStorage !== null;
+    const useEnvelope = this.useEnvelope(interop);
 
     // Fetch from L2 (backend)
     return this.run('get', null, async (): Promise<T | null> => {
@@ -425,12 +501,15 @@ export class CacheImpl implements SecureCache {
         ? decodeInteropValue<T>(plaintext)
         : this.serializer.decode<T>(plaintext);
 
-      // Populate L1. Interop keys are {namespace}:{operation}:{hash} — group
-      // under the user-facing namespace segment so namespace-level
-      // invalidation matches entries written through wrap().
+      // Populate L1 with `data` — the bytes the backend returned, still
+      // encrypted — not the plaintext `value` decoded above. Interop keys are
+      // {namespace}:{operation}:{hash} — group under the user-facing namespace
+      // segment so namespace-level invalidation matches entries written
+      // through wrap().
       if (this.l1) {
         const namespace = interop ? key.slice(0, key.indexOf(':')) : extractNamespace(key);
-        this.l1.set(key, value, (ttlSeconds ?? this.defaultTtl) * 1000, namespace);
+        const ttlMs = (ttlSeconds ?? this.defaultTtl) * 1000;
+        this.l1.set(key, this.l1Payload(value, data), ttlMs, namespace);
         this.publishL1Stats();
       }
 
@@ -440,13 +519,18 @@ export class CacheImpl implements SecureCache {
   }
 
   async set<T>(key: string, value: T, options?: SetOptions): Promise<void> {
-    return this.setEntry(key, value, options, false);
+    await this.setEntry(key, value, options, false);
   }
 
   /**
    * L1 + L2 write. Interop entries (interop=true) serialize to canonical
    * plain MessagePack — never the ByteStorage envelope — and encrypt with
    * AAD compressed=False, matching the Python and Rust SDKs byte-for-byte.
+   *
+   * Returns the payload L1 should hold for this entry, so the SWR refresh
+   * path (which defers its L1 write to completeRefresh's version check) can
+   * store the ciphertext this produced instead of the caller's plaintext.
+   * Null when the write degraded with nothing L1 may hold — see `degradedL1`.
    */
   private async setEntry<T>(
     key: string,
@@ -454,7 +538,7 @@ export class CacheImpl implements SecureCache {
     options: SetOptions | undefined,
     interop: boolean,
     updateL1 = true
-  ): Promise<void> {
+  ): Promise<L1Write | null> {
     this.ensureNotClosed();
 
     const ttl = options?.ttl ?? this.defaultTtl;
@@ -465,7 +549,14 @@ export class CacheImpl implements SecureCache {
     }
 
     const namespace = options?.namespace ?? extractNamespace(key);
-    const useEnvelope = !interop && this.byteStorage !== null;
+    const useEnvelope = this.useEnvelope(interop);
+
+    // What a degraded write (backend down, degradation swallowing it) leaves
+    // for the SWR refresh to put in L1. A plaintext cache still has the value
+    // in hand, so the refresh repopulates L1 exactly as it did before; a
+    // secure cache has no verified ciphertext to show for a write that never
+    // landed, so it declines and L1 keeps the stale entry until it expires.
+    const degradedL1: L1Write | null = this.encryption ? null : { l1: value };
 
     // Interop model rejection is a deterministic caller error (spec: values
     // outside the data model MUST error) — it surfaces synchronously and
@@ -475,7 +566,7 @@ export class CacheImpl implements SecureCache {
     // (existing degrade semantics unchanged).
     const interopSerialized = interop ? encodeInteropValue(value) : null;
 
-    return this.run('set', undefined, async (): Promise<void> => {
+    return this.run('set', degradedL1, async (): Promise<L1Write> => {
       // Serialize
       const serialized = interopSerialized ?? this.serializer.encode(value);
 
@@ -490,19 +581,22 @@ export class CacheImpl implements SecureCache {
       // Store in backend
       await this.backend.set(key, data, ttl);
 
-      // Update L1 for direct writes. The SWR refresh path passes
-      // updateL1=false and writes L1 only through completeRefresh, whose
-      // version token discards the refresh if an explicit write or
-      // invalidation landed meanwhile — the guard is authoritative for L1
-      // ONLY. The backend.set above is unconditional last-write-wins: an
-      // interleaved explicit set() survives in L1 but is overwritten in L2
-      // by the refresh's value until the entry next expires or refreshes
-      // (a conditional L2 write would need CAS the Backend contract doesn't
-      // have).
+      // Update L1 for direct writes, with `data` (the ciphertext just written
+      // to the backend) rather than the caller's plaintext `value` whenever
+      // the cache is encrypted. The SWR refresh path passes updateL1=false and
+      // writes L1 only through completeRefresh, whose version token discards
+      // the refresh if an explicit write or invalidation landed meanwhile —
+      // the guard is authoritative for L1 ONLY. The backend.set above is
+      // unconditional last-write-wins: an interleaved explicit set() survives
+      // in L1 but is overwritten in L2 by the refresh's value until the entry
+      // next expires or refreshes (a conditional L2 write would need CAS the
+      // Backend contract doesn't have).
+      const l1 = this.l1Payload(value, data);
       if (updateL1 && this.l1) {
-        this.l1.set(key, value, ttl * 1000, namespace);
+        this.l1.set(key, l1, ttl * 1000, namespace);
         this.publishL1Stats();
       }
+      return { l1 };
     });
   }
 
@@ -668,35 +762,47 @@ export class CacheImpl implements SecureCache {
         if (this.swrRequiresWaitUntil && !waitUntil) {
           const l1Value = this.l1.get(cacheKey);
           if (l1Value !== null) {
-            this.recordHit('l1');
-            return l1Value as TResult;
+            const decoded = await this.readL1<TResult>(cacheKey, l1Value, interop);
+            if (decoded !== null) {
+              this.recordHit('l1');
+              return decoded;
+            }
           }
         } else {
           const swrResult = this.l1.getWithSwr(cacheKey);
 
           if (swrResult.value !== null) {
-            // Trigger background refresh if needed
-            if (swrResult.shouldRefresh) {
-              this.backgroundRefresh.scheduleRefresh(
-                cacheKey,
-                () => fn(...args),
-                { ttl: options.ttl, namespace: options.namespace },
-                swrResult.versionToken,
-                this.l1,
-                async (key, value, opts) => {
-                  await this.setEntry(
-                    key,
-                    value,
-                    { ttl: opts.ttl, namespace: opts.namespace },
-                    interop,
-                    false
-                  );
-                },
-                waitUntil
-              );
+            // Decode before scheduling: an entry that will not decrypt is
+            // already gone, so refreshing it would race the cold path that is
+            // about to recompute the same key.
+            const decoded = await this.readL1<TResult>(cacheKey, swrResult.value, interop);
+            if (decoded === null) {
+              // getWithSwr took the refresh marker on our behalf — release it
+              // so the slot is not held for a key we are dropping.
+              if (swrResult.shouldRefresh) this.l1.cancelRefresh(cacheKey);
+            } else {
+              // Trigger background refresh if needed
+              if (swrResult.shouldRefresh) {
+                this.backgroundRefresh.scheduleRefresh(
+                  cacheKey,
+                  () => fn(...args),
+                  { ttl: options.ttl, namespace: options.namespace },
+                  swrResult.versionToken,
+                  this.l1,
+                  (key, value, opts) =>
+                    this.setEntry(
+                      key,
+                      value,
+                      { ttl: opts.ttl, namespace: opts.namespace },
+                      interop,
+                      false
+                    ),
+                  waitUntil
+                );
+              }
+              this.recordHit('l1');
+              return decoded;
             }
-            this.recordHit('l1');
-            return swrResult.value as TResult;
           }
         }
       }
diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts
new file mode 100644
index 0000000..77d057f
--- /dev/null
+++ b/packages/cachekit/src/cache.encryption-l1.test.ts
@@ -0,0 +1,194 @@
+/**
+ * LAB-238 regression: L1 must never hold plaintext for an encrypted cache.
+ *
+ * Zero-knowledge is a property of every layer. cachekit-py's L1Cache stores
+ * bytes (`get() -> tuple[bool, Optional[bytes]]`) and decrypts at read time;
+ * cachekit-rs stores ciphertext across all layers. TypeScript stored the
+ * post-decrypt value, so a heap dump, core dump, or Node diagnostic report
+ * yielded the whole L1 working set in plaintext for its full TTL — and that
+ * plaintext survived the key zeroization in close(), because L1 entries are
+ * held independently of tenant keys.
+ *
+ * These drive the REAL encryption path (AES-256-GCM via the Rust core, AAD
+ * bound to the cache key) through createCache end-to-end — no mocks on the
+ * crypto path — and cover all three L1 population sites: set, get, and the SWR
+ * background refresh.
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import { createCache } from './cache.js';
+import { generateKey } from './serialization/key-generator.js';
+import type { SecureCache } from './types/cache.js';
+import type { Backend } from './backends/types.js';
+import type { L1Cache } from './l1/lru-cache.js';
+
+const MASTER_KEY = '61'.repeat(32);
+const TENANT = 'lab-238';
+
+/** Distinctive enough that a substring search over any dump is conclusive. */
+const LEAK_CANARY = 'ssn-000-00-0000-do-not-leak';
+
+class InMemoryBackend implements Backend {
+  store = new Map<string, Uint8Array>();
+  gets = 0;
+
+  async get(key: string): Promise<Uint8Array | null> {
+    this.gets++;
+    return this.store.get(key) ?? null;
+  }
+  async set(key: string, value: Uint8Array): Promise<void> {
+    this.store.set(key, value);
+  }
+  async delete(key: string): Promise<boolean> {
+    return this.store.delete(key);
+  }
+  async exists(key: string): Promise<boolean> {
+    return this.store.has(key);
+  }
+  async close(): Promise<void> {}
+}
+
+/**
+ * The L1 instance CacheImpl holds. Private on purpose — this suite asserts on
+ * exactly the bytes an attacker with a heap dump would find, which is the one
+ * thing a public API cannot show us.
+ */
+function l1Of(cache: SecureCache): L1Cache {
+  return (cache as unknown as { l1: L1Cache }).l1;
+}
+
+/** What is resident in L1 for this key, as-is. */
+function l1Entry(cache: SecureCache, key: string): unknown {
+  return l1Of(cache).get(key);
+}
+
+function expectCiphertext(stored: unknown, backendBytes: Uint8Array | undefined): void {
+  expect(stored).toBeInstanceOf(Uint8Array);
+  // Byte-identical to what L2 holds: the same envelope, still sealed.
+  expect(stored).toEqual(backendBytes);
+  // And the secret is nowhere in the resident representation.
+  expect(JSON.stringify(Array.from(stored as Uint8Array))).not.toContain(LEAK_CANARY);
+  expect(new TextDecoder().decode(stored as Uint8Array)).not.toContain(LEAK_CANARY);
+}
+
+describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
+  const caches: SecureCache[] = [];
+
+  function makeCache(backend: Backend, encrypted = true): SecureCache {
+    const cache = createCache({
+      backend,
+      defaultTtl: 3600,
+      l1: { enabled: true, maxEntries: 100 },
+      ...(encrypted ? { encryption: { masterKey: MASTER_KEY, tenantId: TENANT } } : {}),
+    });
+    caches.push(cache);
+    return cache;
+  }
+
+  afterEach(async () => {
+    await Promise.all(caches.splice(0).map((c) => c.close()));
+  });
+
+  it('set() populates L1 with ciphertext, and the L1 hit decrypts it', async () => {
+    const backend = new InMemoryBackend();
+    const cache = makeCache(backend);
+    const key = 'users:1';
+
+    await cache.set(key, { ssn: LEAK_CANARY });
+
+    expectCiphertext(l1Entry(cache, key), backend.store.get(key));
+
+    // The hit is served from L1 (no second backend read) and still returns
+    // the plaintext value — decrypt happens on read, as in cachekit-py.
+    const before = backend.gets;
+    expect(await cache.get(key)).toEqual({ ssn: LEAK_CANARY });
+    expect(backend.gets).toBe(before);
+  });
+
+  it('get() repopulates L1 with the ciphertext it read, not the value it decoded', async () => {
+    const backend = new InMemoryBackend();
+    const writer = makeCache(backend);
+    await writer.set('users:2', { ssn: LEAK_CANARY });
+
+    // A second cache over the same L2: cold L1, warm backend.
+    const reader = makeCache(backend);
+    expect(l1Entry(reader, 'users:2')).toBeNull();
+
+    expect(await reader.get('users:2')).toEqual({ ssn: LEAK_CANARY });
+
+    expectCiphertext(l1Entry(reader, 'users:2'), backend.store.get('users:2'));
+  });
+
+  it('the SWR background refresh writes ciphertext to L1, not the factory result', async () => {
+    const backend = new InMemoryBackend();
+    const cache = makeCache(backend);
+
+    let generation = 0;
+    const load = cache.wrap(
+      async (_id: number) => ({ ssn: LEAK_CANARY, generation: ++generation }),
+      // 1s TTL: past the 0.5 default SWR threshold (±10% jitter) after the
+      // wait below, so the next read serves stale and schedules a refresh.
+      { namespace: 'users', ttl: 1 }
+    );
+
+    await load(3);
+    const key = generateKey('users', [3]);
+    expect(generation).toBe(1);
+    const firstCiphertext = l1Entry(cache, key);
+
+    await new Promise((r) => setTimeout(r, 700));
+
+    // Stale hit: served from L1, refresh scheduled in the background.
+    expect(await load(3)).toEqual({ ssn: LEAK_CANARY, generation: 1 });
+
+    // Wait for the refresh to actually land in L1 — the factory running is not
+    // enough, completeRefresh only runs once the L2 write has resolved.
+    await vi.waitFor(() => {
+      expect(l1Entry(cache, key)).not.toEqual(firstCiphertext);
+    });
+
+    expectCiphertext(l1Entry(cache, key), backend.store.get(key));
+    expect(await load(3)).toEqual({ ssn: LEAK_CANARY, generation: 2 });
+  });
+
+  it('rejects a substituted L1 entry — AAD is verified against the cache key', async () => {
+    const backend = new InMemoryBackend();
+    const cache = makeCache(backend);
+    const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    try {
+      await cache.set('users:victim', { ssn: LEAK_CANARY });
+      await cache.set('users:attacker', { ssn: 'attacker-controlled' });
+
+      // Swap in a valid ciphertext sealed under a DIFFERENT key. Without an
+      // AAD check on the L1 path this would be served as the victim's value.
+      const attackerBytes = backend.store.get('users:attacker')!;
+      l1Of(cache).set('users:victim', attackerBytes, 3600_000, 'users');
+
+      expect(await cache.get('users:victim')).toEqual({ ssn: LEAK_CANARY });
+      expect(consoleSpy).toHaveBeenCalledWith(
+        '[cachekit] L1 decrypt failed — entry dropped, falling through to L2:',
+        expect.any(String)
+      );
+      // The poisoned entry was dropped, then refilled from L2 on the way back.
+      expectCiphertext(l1Entry(cache, 'users:victim'), backend.store.get('users:victim'));
+    } finally {
+      consoleSpy.mockRestore();
+    }
+  });
+
+  it('leaves plaintext caches storing decoded values (unchanged, non-goal)', async () => {
+    const backend = new InMemoryBackend();
+    const cache = makeCache(backend, false);
+
+    await cache.set('users:4', { ssn: LEAK_CANARY });
+    expect(l1Entry(cache, 'users:4')).toEqual({ ssn: LEAK_CANARY });
+
+    const load = cache.wrap(async (_id: number) => ({ plain: true }), {
+      namespace: 'items',
+      ttl: 60,
+    });
+    await load(9);
+    expect(l1Entry(cache, generateKey('items', [9]))).toEqual({ plain: true });
+  });
+});
diff --git a/packages/cachekit/src/cache/background-refresh.test.ts b/packages/cachekit/src/cache/background-refresh.test.ts
index cc6b95b..6b0580e 100644
--- a/packages/cachekit/src/cache/background-refresh.test.ts
+++ b/packages/cachekit/src/cache/background-refresh.test.ts
@@ -11,7 +11,9 @@ describe('BackgroundRefreshManager', () => {
   beforeEach(() => {
     manager = new BackgroundRefreshManager();
     l1Cache = new L1Cache({ maxEntries: 100 });
-    persistToL2 = vi.fn().mockResolvedValue(undefined);
+    // The L2 write hands back what L1 should hold. For a plaintext cache that
+    // is the value itself; a secure cache would return its ciphertext here.
+    persistToL2 = vi.fn(async (_key: string, value: unknown) => ({ l1: value }));
     consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
   });
 
@@ -109,6 +111,61 @@ describe('BackgroundRefreshManager', () => {
       expect(cached).toEqual({ data: 'refreshed' });
     });
 
+    it('stores what the L2 write returned, not the plaintext it computed (LAB-238)', async () => {
+      // A secure cache's persist callback returns the ciphertext it wrote to
+      // L2. That, and never the factory's plaintext result, is what lands in
+      // L1 — otherwise the SWR refresh re-poisons L1 on every revalidation.
+      const ciphertext = new Uint8Array([0xde, 0xad, 0xbe, 0xef]);
+      const secretPersist = vi.fn(async () => ({ l1: ciphertext }));
+      const computeFn = vi.fn().mockResolvedValue({ ssn: '000-00-0000' });
+
+      l1Cache.set('key1', ciphertext, 3600000, 'test');
+      const { versionToken } = l1Cache.getWithSwr('key1');
+
+      manager.scheduleRefresh(
+        'key1',
+        computeFn,
+        { ttl: 3600, namespace: 'test' },
+        versionToken,
+        l1Cache,
+        secretPersist
+      );
+
+      await vi.waitFor(() => {
+        expect(secretPersist).toHaveBeenCalled();
+      });
+
+      expect(l1Cache.get('key1')).toBe(ciphertext);
+      expect(JSON.stringify(l1Cache.get('key1'))).not.toContain('000-00-0000');
+    });
+
+    it('cancels the refresh when the write returns nothing storable', async () => {
+      // Degraded L2 write on a secure cache: no ciphertext to show for it, so
+      // L1 must keep the stale entry rather than fall back to the plaintext.
+      const degraded = vi.fn(async () => null);
+      const computeFn = vi.fn().mockResolvedValue({ data: 'fresh' });
+
+      l1Cache.set('key1', { data: 'stale' }, 3600000, 'test');
+      const { versionToken } = l1Cache.getWithSwr('key1');
+
+      manager.scheduleRefresh(
+        'key1',
+        computeFn,
+        { ttl: 3600, namespace: 'test' },
+        versionToken,
+        l1Cache,
+        degraded
+      );
+
+      await vi.waitFor(() => {
+        expect(degraded).toHaveBeenCalled();
+      });
+
+      expect(l1Cache.get('key1')).toEqual({ data: 'stale' });
+      // Marker released, so the key can be refreshed again on a later read.
+      expect(l1Cache.stats.refreshing).toBe(0);
+    });
+
     it('should log error and cancel L1 refresh on failure', async () => {
       const error = new Error('Compute failed');
       const computeFn = vi.fn().mockRejectedValue(error);
diff --git a/packages/cachekit/src/cache/background-refresh.ts b/packages/cachekit/src/cache/background-refresh.ts
index db14c84..0ea8563 100644
--- a/packages/cachekit/src/cache/background-refresh.ts
+++ b/packages/cachekit/src/cache/background-refresh.ts
@@ -19,9 +19,28 @@ export interface RefreshOptions {
 }
 
 /**
- * Callback for persisting refreshed values to L2 cache.
+ * What L1 should hold for an entry, as produced by the L2 write that just
+ * landed: the ciphertext for a secure cache, the plain value otherwise.
+ *
+ * Wrapped rather than returned bare because a cached value may legitimately be
+ * null or undefined — the wrapper is what distinguishes "store this" from
+ * "there is nothing to store" (LAB-238).
+ */
+export interface L1Write {
+  readonly l1: unknown;
+}
+
+/**
+ * Callback for persisting refreshed values to L2 cache. Returns the payload
+ * L1 should hold for the entry, or null when the write produced nothing L1 may
+ * hold (a degraded write on a secure cache — no ciphertext to store, and the
+ * refresh must not fall back to the plaintext it computed).
  */
-export type PersistCallback<T> = (key: string, value: T, options: RefreshOptions) => Promise<void>;
+export type PersistCallback<T> = (
+  key: string,
+  value: T,
+  options: RefreshOptions
+) => Promise<L1Write | null>;
 
 /**
  * Manages stale-while-revalidate (SWR) background refreshes.
@@ -76,13 +95,27 @@ export class BackgroundRefreshManager {
 
         const result = await computeFn();
 
-        // Update L2 FIRST (before version check)
-        await persistToL2(key, result, options);
+        // Update L2 FIRST (before version check). The write hands back what L1
+        // should hold — for a secure cache the ciphertext it just produced,
+        // never the plaintext `result` computed above (LAB-238).
+        const persisted = await persistToL2(key, result, options);
 
         // Then complete L1 refresh with version check
         // If version changed during L2 update, L1 update is rejected (stale data protection)
         if (l1Cache) {
-          l1Cache.completeRefresh(key, result, options.ttl * 1000, versionToken, options.namespace);
+          if (!persisted) {
+            // Nothing storable came back (degraded write on a secure cache):
+            // release the marker and leave the stale entry to expire.
+            l1Cache.cancelRefresh(key);
+          } else {
+            l1Cache.completeRefresh(
+              key,
+              persisted.l1,
+              options.ttl * 1000,
+              versionToken,
+              options.namespace
+            );
+          }
         }
       } catch (error) {
         // Log error for observability
diff --git a/packages/cachekit/src/l1/lru-cache.test.ts b/packages/cachekit/src/l1/lru-cache.test.ts
index 4d2cd92..f06c5af 100644
--- a/packages/cachekit/src/l1/lru-cache.test.ts
+++ b/packages/cachekit/src/l1/lru-cache.test.ts
@@ -132,6 +132,25 @@ describe('L1Cache', () => {
       expect(smallCache.stats.entries).toBeLessThan(3);
       expect(smallCache.stats.memoryUsed).toBeLessThanOrEqual(400);
     });
+
+    it('sizes byte payloads by their buffer, not their JSON form (LAB-238)', () => {
+      // Secure caches store the L2 ciphertext here. JSON.stringify of a
+      // Uint8Array yields {"0":12,"1":34,…} — roughly 14x the real size — so
+      // measuring that way would evict most of L1 on the first entry.
+      const bytesCache = new L1Cache<Uint8Array>({ maxEntries: 100, maxMemory: 4096 });
+      const ciphertext = new Uint8Array(256).fill(0xab);
+
+      bytesCache.set('a', ciphertext, 10000, 'test');
+
+      expect(bytesCache.stats.memoryUsed).toBe(256);
+
+      // 16 x 256B fits in a 4 KiB budget; under JSON sizing the second entry
+      // would already have evicted the first.
+      for (let i = 0; i < 15; i++) {
+        bytesCache.set(`k${i}`, new Uint8Array(256).fill(i), 10000, 'test');
+      }
+      expect(bytesCache.stats.entries).toBe(16);
+    });
   });
 
   describe('SWR', () => {
diff --git a/packages/cachekit/src/l1/lru-cache.ts b/packages/cachekit/src/l1/lru-cache.ts
index d665288..c42d45b 100644
--- a/packages/cachekit/src/l1/lru-cache.ts
+++ b/packages/cachekit/src/l1/lru-cache.ts
@@ -412,6 +412,12 @@ export class L1Cache<T = unknown> {
   }
 
   private estimateSize(value: unknown): number {
+    // Secure caches store the L2 ciphertext here (LAB-238), so the common
+    // entry is a Uint8Array. JSON.stringify turns one into {"0":12,"1":34,…} —
+    // roughly 14x its real size — which would blow the memory budget and evict
+    // most of L1 on the first encrypted entry. Count the buffer instead.
+    if (ArrayBuffer.isView(value)) return value.byteLength;
+
     // Rough estimation - JSON stringify length as proxy
     // m2 Fix: Track visited objects to prevent infinite recursion on circular refs
     const visited = new WeakSet<object>();

From 7f1cea465d8a9d459b5c67fca4d58290caa79cb6 Mon Sep 17 00:00:00 2001
From: Ray Walker <ray@27b.io>
Date: Sat, 8 Aug 2026 01:32:03 +1000
Subject: [PATCH 2/5] fix(security): apply expert-panel findings on L1
 ciphertext (LAB-238)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Panel review of ecfd912 at critical stakes (per ray's LAB-131 gate: any
encryption/AAD diff in a cachekit repo needs one). Five findings applied, four
rejected with reasons.

CRIT — degraded L2 write became an origin stampede. Found independently by two
reviewers and measured: with the backend down, 11 reads of an encrypted key
drove 11 origin calls, against 1 for plaintext. Returning null for "nothing
storable" made every SWR refresh end in cancelRefresh, which frees the refresh
marker while leaving expiresAt untouched — so the entry stayed stale and every
subsequent read re-armed the refresh, on exactly the encrypted caches that
carry PII, and invisibly to any plaintext load test. Two changes: the
ciphertext is now captured the moment encrypt() produces it, before the
backend write that may fail, so a degraded write still yields an L1 payload and
the refresh resets freshness as it always did; and the residual null case
(encrypt itself failed — nonce exhausted, manager disposed) deliberately leaves
the marker to lapse via SWR_REFRESH_MARKER_TTL_MS, throttling retries to one
per key per minute instead of one per read.

MAJ — a cached null read as a decrypt failure. readL1 overloaded null as both
"AEAD verification failed" and "the value is null", so a secure cache holding
null invalidated and re-fetched a perfectly good entry on every hit — a billed
miss per read on a metered backend. The same commit added the L1Write wrapper
for this exact reason on the write side and left the read side unwrapped;
decodeL1Entry now returns { value } | null.

MAJ — exists() trusted L1 presence without decrypting, so after a key rotation
it reported present for entries get() verifies, rejects and drops. It now
decodes, so exists() and get() cannot disagree.

MAJ — an AES-GCM tag failure is the canonical tamper signal and reached only a
log line. It now goes through recordFailure so operators can alert on it.

MAJ — reliability.degradation governs an L2 decrypt failure (it runs inside the
executor) but not the new L1 one. decodeL1Entry now honours the same lever:
degradation off rethrows instead of falling through.

Also: extracted decodeEntry so the L1 and L2 tiers cannot drift into decoding
the same entry differently; renamed readL1 to decodeL1Entry (it decodes an
already-read value and evicts on failure); dropped a vestigial type parameter;
documented that the buffer-copy rule guards slab pinning, not a backend that
mutates buffers it handed over.

Rejected: a new encryption.failClosed option (new public API, the existing
degradation lever covers it); copying L1 bytes unconditionally (every in-tree
backend was verified to return an owned exact-size buffer, and the copy is real
cost on large values); a synchronous fast path for plaintext L1 hits
(unmeasured microtask against three duplicated ternaries on a crypto path); and
re-checking the L1 version token after the decrypt await to close a
sub-millisecond read-vs-invalidate window (needs new L1 API, cannot be tested
deterministically, and the authoritative delete already went to L2).

Tests: three new regressions, each verified to fail against ecfd912 — the
stampede (11 origin calls vs 2), the null round-trip, and exists() verification.
Deleted a dead assertion that stringified bytes to digits and so could never
have caught a leak. Widened the SWR test's stale window from 300ms to 600ms of
headroom so a loaded CI box cannot take the cold path.
---
 packages/cachekit/src/cache-core.ts           | 143 +++++++++++-------
 .../cachekit/src/cache.encryption-l1.test.ts  | 117 +++++++++++++-
 .../src/cache/background-refresh.test.ts      |  24 ++-
 .../cachekit/src/cache/background-refresh.ts  |  12 +-
 4 files changed, 221 insertions(+), 75 deletions(-)

diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts
index 7a15337..b63e259 100644
--- a/packages/cachekit/src/cache-core.ts
+++ b/packages/cachekit/src/cache-core.ts
@@ -182,6 +182,12 @@ export class CacheImpl implements SecureCache {
   // l1Telemetry hook below).
   private readonly telemetry = { l1Hits: 0, l2Hits: 0, misses: 0 };
   private readonly swrRequiresWaitUntil: boolean;
+  /**
+   * Mirrors ReliabilityExecutor's own default. Read directly so the L1 decrypt
+   * path can honour the same fail-open/fail-closed choice the L2 decrypt path
+   * gets for free by running inside the executor.
+   */
+  private readonly degradationEnabled: boolean;
   private closed = false;
   /** One in-flight cold-miss resolution per cache key (single-flight, LAB-519). */
   private readonly inflight = new Map<string, Promise<unknown>>();
@@ -255,6 +261,7 @@ export class CacheImpl implements SecureCache {
       retry: options.reliability?.retry,
       degradation: options.reliability?.degradation,
     });
+    this.degradationEnabled = options.reliability?.degradation !== false;
 
     // Initialize background refresh manager (SWR)
     this.backgroundRefresh = new BackgroundRefreshManager();
@@ -408,45 +415,75 @@ export class CacheImpl implements SecureCache {
    * The bytes are copied when they are a window into a larger buffer: a Node
    * Buffer from the backend is a view onto a shared 8 KiB pool slab, and
    * retaining one for the entry's TTL pins the whole slab. cachekit-py guards
-   * the same edge by refusing memoryview/bytearray in L1Cache.put.
+   * the same edge by refusing memoryview/bytearray in L1Cache.put. This guards
+   * slab pinning only — it does NOT defend against a backend that mutates a
+   * buffer it already handed over. Every in-tree backend returns an owned,
+   * exact-size Uint8Array (redis, memcached, cachekitio, workers-kv,
+   * workers-cache-api) and file.ts's narrower header view lands in the copy
+   * branch; a third-party Backend that recycles buffers must copy on its side.
    */
-  private l1Payload<T>(value: T, bytes: Uint8Array): unknown {
+  private l1Payload(value: unknown, bytes: Uint8Array): unknown {
     if (!this.encryption) return value;
     return bytes.byteLength === bytes.buffer.byteLength ? bytes : new Uint8Array(bytes);
   }
 
+  /**
+   * Ciphertext (or envelope) bytes to the value they carry: decrypt +
+   * AAD-verify against the cache key, unpack the ByteStorage envelope, then
+   * deserialize. Shared by the L2 read and the L1 read so the two tiers can
+   * never drift into decoding the same entry differently — a real hazard once
+   * both paths handle AAD (protocol#12 freezes the v0x03 component set).
+   */
+  private async decodeEntry<T>(bytes: Uint8Array, key: string, interop: boolean): Promise<T> {
+    const useEnvelope = this.useEnvelope(interop);
+
+    let plaintext = bytes;
+    if (this.encryption) {
+      plaintext = await this.encryption.decrypt(plaintext, key, useEnvelope);
+    }
+    if (useEnvelope) {
+      plaintext = this.byteStorage!.unpack(plaintext);
+    }
+    return interop ? decodeInteropValue<T>(plaintext) : this.serializer.decode<T>(plaintext);
+  }
+
   /**
    * Decode a value served from L1. For a secure cache that is a decrypt +
    * AAD-verify against the cache key followed by the same unpack/deserialize
    * the L2 path runs — an L1 hit is no longer free, which is the price of not
    * keeping plaintext resident. For a plaintext cache it is a cast.
    *
-   * Returns null when a secure entry will not decrypt (rotated key, tampered
-   * heap, an entry written under a different envelope mode). The entry is
-   * dropped first so a poisoned L1 copy cannot outlive remediation of L2, and
-   * the caller falls through to L2, which re-verifies against the same key and
-   * either repopulates L1 or fails on its own path. This mirrors cachekit-py's
-   * L1 handler, which invalidates before applying its fail policy. The failure
-   * is logged rather than swallowed: on this SDK an L2 decrypt failure is
-   * already absorbed by the degradation layer, so a throw here would make an
-   * L1 hit noisier than the same corruption seen one layer down.
+   * Drops the L1 entry and returns null when a secure entry will not decrypt
+   * (rotated key, tampered heap, an entry written under a different envelope
+   * mode), so a poisoned L1 copy cannot outlive remediation of L2 — cachekit-py
+   * invalidates before applying its fail policy for the same reason. The result
+   * is wrapped because a cached value may legitimately BE null: without the
+   * wrapper a secure cache holding null would read as a decrypt failure on
+   * every hit, invalidating and re-fetching a perfectly good entry forever.
+   *
+   * Fail policy follows `reliability.degradation`, the same lever that governs
+   * an L2 decrypt failure (which happens inside run()): degradation on absorbs
+   * the failure and falls through to L2, degradation off rethrows so a tamper
+   * signal reaches the caller. Either way it is counted and logged, never
+   * silently swallowed.
    */
-  private async readL1<T>(key: string, stored: unknown, interop: boolean): Promise<T | null> {
-    if (!this.encryption) return stored as T;
+  private async decodeL1Entry<T>(
+    key: string,
+    stored: unknown,
+    interop: boolean
+  ): Promise<{ value: T } | null> {
+    if (!this.encryption) return { value: stored as T };
 
     try {
-      const useEnvelope = this.useEnvelope(interop);
-      let plaintext = await this.encryption.decrypt(stored as Uint8Array, key, useEnvelope);
-      if (useEnvelope) {
-        plaintext = this.byteStorage!.unpack(plaintext);
-      }
-      return interop ? decodeInteropValue<T>(plaintext) : this.serializer.decode<T>(plaintext);
+      return { value: await this.decodeEntry<T>(stored as Uint8Array, key, interop) };
     } catch (error) {
       this.l1?.invalidateByKey(key);
+      this.recordFailure('l1_decrypt', error);
       logError(
-        '[cachekit] L1 decrypt failed — entry dropped, falling through to L2:',
+        '[cachekit] L1 decrypt failed — entry dropped:',
         error instanceof Error ? error.message : 'Unknown error'
       );
+      if (!this.degradationEnabled) throw error;
       return null;
     }
   }
@@ -467,16 +504,14 @@ export class CacheImpl implements SecureCache {
     if (this.l1) {
       const l1Result = this.l1.get(key);
       if (l1Result !== null) {
-        const decoded = await this.readL1<T>(key, l1Result, interop);
+        const decoded = await this.decodeL1Entry<T>(key, l1Result, interop);
         if (decoded !== null) {
           this.recordHit('l1');
-          return decoded;
+          return decoded.value;
         }
       }
     }
 
-    const useEnvelope = this.useEnvelope(interop);
-
     // Fetch from L2 (backend)
     return this.run('get', null, async (): Promise<T | null> => {
       const data = await this.backend.get(key);
@@ -485,21 +520,8 @@ export class CacheImpl implements SecureCache {
         return null;
       }
 
-      // Decrypt if encryption enabled
-      let plaintext = data;
-      if (this.encryption) {
-        plaintext = await this.encryption.decrypt(data, key, useEnvelope);
-      }
-
-      // Decompress with ByteStorage (after decryption)
-      if (useEnvelope) {
-        plaintext = this.byteStorage!.unpack(plaintext);
-      }
-
-      // Deserialize
-      const value = interop
-        ? decodeInteropValue<T>(plaintext)
-        : this.serializer.decode<T>(plaintext);
+      // Decrypt, unpack, deserialize — the same sequence an L1 hit runs.
+      const value = await this.decodeEntry<T>(data, key, interop);
 
       // Populate L1 with `data` — the bytes the backend returned, still
       // encrypted — not the plaintext `value` decoded above. Interop keys are
@@ -530,7 +552,7 @@ export class CacheImpl implements SecureCache {
    * Returns the payload L1 should hold for this entry, so the SWR refresh
    * path (which defers its L1 write to completeRefresh's version check) can
    * store the ciphertext this produced instead of the caller's plaintext.
-   * Null when the write degraded with nothing L1 may hold — see `degradedL1`.
+   * Null only when nothing storable was ever produced — see `l1Write`.
    */
   private async setEntry<T>(
     key: string,
@@ -551,12 +573,16 @@ export class CacheImpl implements SecureCache {
     const namespace = options?.namespace ?? extractNamespace(key);
     const useEnvelope = this.useEnvelope(interop);
 
-    // What a degraded write (backend down, degradation swallowing it) leaves
-    // for the SWR refresh to put in L1. A plaintext cache still has the value
-    // in hand, so the refresh repopulates L1 exactly as it did before; a
-    // secure cache has no verified ciphertext to show for a write that never
-    // landed, so it declines and L1 keeps the stale entry until it expires.
-    const degradedL1: L1Write | null = this.encryption ? null : { l1: value };
+    // What L1 should hold, captured as soon as it exists rather than returned
+    // from the closure — a degraded backend write (which `run` swallows) must
+    // still yield it. Otherwise an encrypted cache's SWR refresh gets nothing
+    // to store, and since a cancelled refresh leaves `expiresAt` untouched the
+    // entry stays stale and EVERY later read re-arms the refresh: a backend
+    // outage becomes an origin stampede on exactly the encrypted caches that
+    // matter (measured 15 origin calls over 15 reads, vs 1 for plaintext).
+    // Seeded with the plaintext value so a plaintext cache repopulates L1 on a
+    // degraded write exactly as it did before this change.
+    let l1Write: L1Write | null = this.encryption ? null : { l1: value };
 
     // Interop model rejection is a deterministic caller error (spec: values
     // outside the data model MUST error) — it surfaces synchronously and
@@ -566,7 +592,7 @@ export class CacheImpl implements SecureCache {
     // (existing degrade semantics unchanged).
     const interopSerialized = interop ? encodeInteropValue(value) : null;
 
-    return this.run('set', degradedL1, async (): Promise<L1Write> => {
+    await this.run('set', undefined, async (): Promise<void> => {
       // Serialize
       const serialized = interopSerialized ?? this.serializer.encode(value);
 
@@ -577,6 +603,7 @@ export class CacheImpl implements SecureCache {
       if (this.encryption) {
         data = await this.encryption.encrypt(data, key, useEnvelope);
       }
+      l1Write = { l1: this.l1Payload(value, data) };
 
       // Store in backend
       await this.backend.set(key, data, ttl);
@@ -591,13 +618,13 @@ export class CacheImpl implements SecureCache {
       // in L1 but is overwritten in L2 by the refresh's value until the entry
       // next expires or refreshes (a conditional L2 write would need CAS the
       // Backend contract doesn't have).
-      const l1 = this.l1Payload(value, data);
       if (updateL1 && this.l1) {
-        this.l1.set(key, l1, ttl * 1000, namespace);
+        this.l1.set(key, l1Write!.l1, ttl * 1000, namespace);
         this.publishL1Stats();
       }
-      return { l1 };
     });
+
+    return l1Write;
   }
 
   async delete(key: string): Promise<boolean> {
@@ -620,10 +647,14 @@ export class CacheImpl implements SecureCache {
   async exists(key: string): Promise<boolean> {
     this.ensureNotClosed();
 
-    // Check L1 first
+    // Check L1 first. Presence alone is not an answer for a secure cache: L1
+    // holds ciphertext, and after a key rotation every resident entry is
+    // undecryptable, so a bare `get() !== null` would report present for
+    // entries get() verifies, rejects and drops. Decode so exists() and get()
+    // cannot disagree, and so the poisoned entry is dropped here too.
     if (this.l1) {
       const l1Value = this.l1.get(key);
-      if (l1Value !== null) {
+      if (l1Value !== null && (await this.decodeL1Entry(key, l1Value, false)) !== null) {
         this.recordHit('l1');
         return true;
       }
@@ -762,10 +793,10 @@ export class CacheImpl implements SecureCache {
         if (this.swrRequiresWaitUntil && !waitUntil) {
           const l1Value = this.l1.get(cacheKey);
           if (l1Value !== null) {
-            const decoded = await this.readL1<TResult>(cacheKey, l1Value, interop);
+            const decoded = await this.decodeL1Entry<TResult>(cacheKey, l1Value, interop);
             if (decoded !== null) {
               this.recordHit('l1');
-              return decoded;
+              return decoded.value;
             }
           }
         } else {
@@ -775,7 +806,7 @@ 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.
-            const decoded = await this.readL1<TResult>(cacheKey, swrResult.value, interop);
+            const decoded = await this.decodeL1Entry<TResult>(cacheKey, swrResult.value, interop);
             if (decoded === null) {
               // getWithSwr took the refresh marker on our behalf — release it
               // so the slot is not held for a key we are dropping.
@@ -801,7 +832,7 @@ export class CacheImpl implements SecureCache {
                 );
               }
               this.recordHit('l1');
-              return decoded;
+              return decoded.value;
             }
           }
         }
diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts
index 77d057f..27526b7 100644
--- a/packages/cachekit/src/cache.encryption-l1.test.ts
+++ b/packages/cachekit/src/cache.encryption-l1.test.ts
@@ -66,8 +66,7 @@ function expectCiphertext(stored: unknown, backendBytes: Uint8Array | undefined)
   expect(stored).toBeInstanceOf(Uint8Array);
   // Byte-identical to what L2 holds: the same envelope, still sealed.
   expect(stored).toEqual(backendBytes);
-  // And the secret is nowhere in the resident representation.
-  expect(JSON.stringify(Array.from(stored as Uint8Array))).not.toContain(LEAK_CANARY);
+  // And the canary is nowhere in the resident bytes.
   expect(new TextDecoder().decode(stored as Uint8Array)).not.toContain(LEAK_CANARY);
 }
 
@@ -126,9 +125,11 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
     let generation = 0;
     const load = cache.wrap(
       async (_id: number) => ({ ssn: LEAK_CANARY, generation: ++generation }),
-      // 1s TTL: past the 0.5 default SWR threshold (±10% jitter) after the
-      // wait below, so the next read serves stale and schedules a refresh.
-      { namespace: 'users', ttl: 1 }
+      // 2s TTL, read at 1.4s: 600ms remaining against a 0.5 threshold of
+      // 900-1100ms (±10% jitter), so the read is stale for every jitter draw
+      // and still has 600ms of headroom before the entry expires outright —
+      // a loaded CI box takes the stale path, not the cold path.
+      { namespace: 'users', ttl: 2 }
     );
 
     await load(3);
@@ -136,7 +137,7 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
     expect(generation).toBe(1);
     const firstCiphertext = l1Entry(cache, key);
 
-    await new Promise((r) => setTimeout(r, 700));
+    await new Promise((r) => setTimeout(r, 1400));
 
     // Stale hit: served from L1, refresh scheduled in the background.
     expect(await load(3)).toEqual({ ssn: LEAK_CANARY, generation: 1 });
@@ -166,9 +167,10 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
       l1Of(cache).set('users:victim', attackerBytes, 3600_000, 'users');
 
       expect(await cache.get('users:victim')).toEqual({ ssn: LEAK_CANARY });
+      // AES-GCM tag verification is what rejected it, not a decode error.
       expect(consoleSpy).toHaveBeenCalledWith(
-        '[cachekit] L1 decrypt failed — entry dropped, falling through to L2:',
-        expect.any(String)
+        '[cachekit] L1 decrypt failed — entry dropped:',
+        expect.stringContaining('Authentication verification failed')
       );
       // The poisoned entry was dropped, then refilled from L2 on the way back.
       expectCiphertext(l1Entry(cache, 'users:victim'), backend.store.get('users:victim'));
@@ -177,6 +179,105 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
     }
   });
 
+  it('serves a legitimately-null cached value from L1 without treating it as tampering', async () => {
+    // decodeL1Entry wraps its result because null is a valid cached value: an
+    // unwrapped null would read as a decrypt failure on every hit, so the
+    // entry would be invalidated and re-fetched from L2 (a billed miss on a
+    // metered backend) for its whole TTL.
+    const backend = new InMemoryBackend();
+    const cache = makeCache(backend);
+    const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    try {
+      await cache.set('users:null', null);
+      const before = backend.gets;
+
+      expect(await cache.get('users:null')).toBeNull();
+      expect(await cache.get('users:null')).toBeNull();
+
+      // Served from L1 both times, and nothing was logged as a decrypt failure.
+      expect(backend.gets).toBe(before);
+      expect(consoleSpy).not.toHaveBeenCalled();
+      expect(l1Entry(cache, 'users:null')).toBeInstanceOf(Uint8Array);
+    } finally {
+      consoleSpy.mockRestore();
+    }
+  });
+
+  it('exists() verifies the L1 entry rather than trusting its presence', async () => {
+    // L1 holds ciphertext, so presence alone is not an answer: after a key
+    // rotation every resident entry is undecryptable and exists() would
+    // otherwise report present for entries get() rejects and drops.
+    const backend = new InMemoryBackend();
+    const cache = makeCache(backend);
+    const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    try {
+      await cache.set('users:5', { ssn: LEAK_CANARY });
+      expect(await cache.exists('users:5')).toBe(true);
+
+      // Poison L1 with a ciphertext sealed under a different key, then drop
+      // the L2 copy: exists() must not report present off the bad L1 entry.
+      await cache.set('users:6', { other: true });
+      l1Of(cache).set('users:5', backend.store.get('users:6')!, 3600_000, 'users');
+      backend.store.delete('users:5');
+
+      expect(await cache.exists('users:5')).toBe(false);
+      expect(consoleSpy).toHaveBeenCalled();
+    } finally {
+      consoleSpy.mockRestore();
+    }
+  });
+
+  it('does not stampede the origin when the L2 write is degraded', async () => {
+    // A degraded write still hands back the ciphertext it produced, so the SWR
+    // refresh repopulates L1 and resets expiresAt. Without that, cancelRefresh
+    // would leave the entry permanently stale and every read would re-arm the
+    // refresh — turning a backend outage into an origin stampede on exactly
+    // the encrypted caches that carry PII.
+    const backend = new InMemoryBackend();
+    let writesFail = false;
+    const flaky: Backend = {
+      ...backend,
+      get: (k) => backend.get(k),
+      set: async (k, v, t) => {
+        if (writesFail) throw new Error('backend down');
+        return backend.set(k, v, t);
+      },
+      delete: (k) => backend.delete(k),
+      exists: (k) => backend.exists(k),
+      close: () => backend.close(),
+    };
+    const cache = makeCache(flaky);
+
+    let originCalls = 0;
+    const load = cache.wrap(async (_id: number) => ({ ssn: LEAK_CANARY, n: ++originCalls }), {
+      namespace: 'users',
+      ttl: 2,
+    });
+
+    await load(7);
+    expect(originCalls).toBe(1);
+
+    writesFail = true;
+    await new Promise((r) => setTimeout(r, 1400));
+
+    // Ten stale reads during the outage, spaced so each scheduled refresh
+    // settles before the next read — otherwise the in-flight marker masks the
+    // behaviour under test. A refresh that stored something resets the entry's
+    // freshness, so the origin is touched a couple of times; one that stored
+    // nothing and released its marker would be re-armed by every single read.
+    for (let i = 0; i < 10; i++) {
+      await load(7);
+      await new Promise((r) => setTimeout(r, 10));
+    }
+
+    await vi.waitFor(() => {
+      expect(originCalls).toBeGreaterThan(1);
+    });
+    expect(originCalls).toBeLessThan(5);
+  });
+
   it('leaves plaintext caches storing decoded values (unchanged, non-goal)', async () => {
     const backend = new InMemoryBackend();
     const cache = makeCache(backend, false);
diff --git a/packages/cachekit/src/cache/background-refresh.test.ts b/packages/cachekit/src/cache/background-refresh.test.ts
index 6b0580e..1731783 100644
--- a/packages/cachekit/src/cache/background-refresh.test.ts
+++ b/packages/cachekit/src/cache/background-refresh.test.ts
@@ -139,20 +139,27 @@ describe('BackgroundRefreshManager', () => {
       expect(JSON.stringify(l1Cache.get('key1'))).not.toContain('000-00-0000');
     });
 
-    it('cancels the refresh when the write returns nothing storable', async () => {
-      // Degraded L2 write on a secure cache: no ciphertext to show for it, so
-      // L1 must keep the stale entry rather than fall back to the plaintext.
+    it('holds the refresh marker when the write returns nothing storable', async () => {
+      // No ciphertext to show for the write, so L1 must keep the stale entry
+      // rather than fall back to the plaintext. Critically the marker is NOT
+      // released: cancelling frees it immediately while expiresAt stays put,
+      // so every later read would re-arm the refresh and hammer the origin.
+      // Letting it lapse via SWR_REFRESH_MARKER_TTL_MS throttles to 1/min/key.
       const degraded = vi.fn(async () => null);
       const computeFn = vi.fn().mockResolvedValue({ data: 'fresh' });
 
-      l1Cache.set('key1', { data: 'stale' }, 3600000, 'test');
-      const { versionToken } = l1Cache.getWithSwr('key1');
+      // ttl well past the SWR threshold so the read below is stale and takes
+      // the marker.
+      l1Cache.set('key1', { data: 'stale' }, 1000, 'test');
+      await new Promise((r) => setTimeout(r, 600));
+      const stale = l1Cache.getWithSwr('key1');
+      expect(stale.shouldRefresh).toBe(true);
 
       manager.scheduleRefresh(
         'key1',
         computeFn,
         { ttl: 3600, namespace: 'test' },
-        versionToken,
+        stale.versionToken,
         l1Cache,
         degraded
       );
@@ -162,8 +169,9 @@ describe('BackgroundRefreshManager', () => {
       });
 
       expect(l1Cache.get('key1')).toEqual({ data: 'stale' });
-      // Marker released, so the key can be refreshed again on a later read.
-      expect(l1Cache.stats.refreshing).toBe(0);
+      expect(l1Cache.stats.refreshing).toBe(1);
+      // A second read must NOT schedule another refresh while the marker holds.
+      expect(l1Cache.getWithSwr('key1').shouldRefresh).toBe(false);
     });
 
     it('should log error and cancel L1 refresh on failure', async () => {
diff --git a/packages/cachekit/src/cache/background-refresh.ts b/packages/cachekit/src/cache/background-refresh.ts
index 0ea8563..fc5b360 100644
--- a/packages/cachekit/src/cache/background-refresh.ts
+++ b/packages/cachekit/src/cache/background-refresh.ts
@@ -104,9 +104,15 @@ export class BackgroundRefreshManager {
         // If version changed during L2 update, L1 update is rejected (stale data protection)
         if (l1Cache) {
           if (!persisted) {
-            // Nothing storable came back (degraded write on a secure cache):
-            // release the marker and leave the stale entry to expire.
-            l1Cache.cancelRefresh(key);
+            // Nothing storable came back — the value could not even be
+            // encrypted (nonce exhausted, manager disposed). Deliberately do
+            // NOT cancelRefresh: a cancel frees the marker immediately while
+            // leaving the stale entry's expiresAt untouched, so the next read
+            // re-arms shouldRefresh and the one after that, hammering the
+            // origin for the rest of the TTL. Letting the marker lapse via
+            // SWR_REFRESH_MARKER_TTL_MS throttles retries to one per key per
+            // minute, and hasRefreshSlot sweeps expired markers so no refresh
+            // slot is wedged.
           } else {
             l1Cache.completeRefresh(
               key,

From ebc082dc74ee7d07e37ac39a3611de4d4a6563c3 Mon Sep 17 00:00:00 2001
From: Ray Walker <ray@27b.io>
Date: Sat, 8 Aug 2026 05:14:44 +1000
Subject: [PATCH 3/5] fix(review): replace unsafe assertions with runtime
 guards, randomize test master key (LAB-238)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Kody round 2026-08-07: the encrypted L1 decode path now instanceof-guards
the stored entry before decrypt — a non-bytes entry rides the existing
invalidate/degradation path instead of failing inside the native decrypt.
The test helper narrows via instanceof instead of casting, and the test
master key is generated per-run rather than embedded as a literal.
---
 packages/cachekit/src/cache-core.ts               | 7 ++++++-
 packages/cachekit/src/cache.encryption-l1.test.ts | 9 ++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts
index b63e259..4a2400e 100644
--- a/packages/cachekit/src/cache-core.ts
+++ b/packages/cachekit/src/cache-core.ts
@@ -475,7 +475,12 @@ export class CacheImpl implements SecureCache {
     if (!this.encryption) return { value: stored as T };
 
     try {
-      return { value: await this.decodeEntry<T>(stored as Uint8Array, key, interop) };
+      if (!(stored instanceof Uint8Array)) {
+        throw new Error(
+          `L1 entry for a secure cache is not ciphertext bytes (got ${typeof stored})`
+        );
+      }
+      return { value: await this.decodeEntry<T>(stored, key, interop) };
     } catch (error) {
       this.l1?.invalidateByKey(key);
       this.recordFailure('l1_decrypt', error);
diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts
index 27526b7..24c2e88 100644
--- a/packages/cachekit/src/cache.encryption-l1.test.ts
+++ b/packages/cachekit/src/cache.encryption-l1.test.ts
@@ -15,6 +15,7 @@
  * background refresh.
  */
 
+import { randomBytes } from 'node:crypto';
 import { describe, it, expect, afterEach, vi } from 'vitest';
 import { createCache } from './cache.js';
 import { generateKey } from './serialization/key-generator.js';
@@ -22,7 +23,7 @@ import type { SecureCache } from './types/cache.js';
 import type { Backend } from './backends/types.js';
 import type { L1Cache } from './l1/lru-cache.js';
 
-const MASTER_KEY = '61'.repeat(32);
+const MASTER_KEY = randomBytes(32).toString('hex');
 const TENANT = 'lab-238';
 
 /** Distinctive enough that a substring search over any dump is conclusive. */
@@ -63,11 +64,13 @@ function l1Entry(cache: SecureCache, key: string): unknown {
 }
 
 function expectCiphertext(stored: unknown, backendBytes: Uint8Array | undefined): void {
-  expect(stored).toBeInstanceOf(Uint8Array);
+  if (!(stored instanceof Uint8Array)) {
+    throw new Error(`L1 entry is not ciphertext bytes (got ${typeof stored})`);
+  }
   // Byte-identical to what L2 holds: the same envelope, still sealed.
   expect(stored).toEqual(backendBytes);
   // And the canary is nowhere in the resident bytes.
-  expect(new TextDecoder().decode(stored as Uint8Array)).not.toContain(LEAK_CANARY);
+  expect(new TextDecoder().decode(stored)).not.toContain(LEAK_CANARY);
 }
 
 describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {

From 3e47ca4f31b23bf09cef7e7b5b0abd21737457f7 Mon Sep 17 00:00:00 2001
From: Ray Walker <ray@27b.io>
Date: Sat, 8 Aug 2026 07:27:22 +1000
Subject: [PATCH 4/5] fix(review): release SWR marker on decode failure via
 try/finally, deterministic test key, deflake timing tests (LAB-238)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- getWithSwr: a decode that rethrows (degradation off) now releases the
  refresh marker it holds, same as the null path — one release site via
  null-sentinel try/finally instead of a stranded slot for the marker TTL
- test master key: sha256-derived fixture — deterministic runs without a
  scanner-matchable key literal
- SWR timing tests: 4s TTL / 2.4s sleep clears the 1.8-2.2s jittered
  threshold on every draw with 1.6s expiry headroom; stampede bound derived
  from READS instead of a magic 5
- refresh wait asserts presence AND change (null after expiry no longer
  false-passes); dropped an unfalsifiable JSON.stringify assertion
- logger test persist callback returns null per PersistCallback contract
---
 packages/cachekit/src/cache-core.ts           | 17 +++++++----
 .../cachekit/src/cache.encryption-l1.test.ts  | 30 ++++++++++++++-----
 .../src/cache/background-refresh.test.ts      | 13 ++++----
 packages/cachekit/src/logger.test.ts          |  3 +-
 4 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/packages/cachekit/src/cache-core.ts b/packages/cachekit/src/cache-core.ts
index 4a2400e..b73346c 100644
--- a/packages/cachekit/src/cache-core.ts
+++ b/packages/cachekit/src/cache-core.ts
@@ -811,12 +811,17 @@ 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.
-            const decoded = await this.decodeL1Entry<TResult>(cacheKey, swrResult.value, interop);
-            if (decoded === null) {
-              // getWithSwr took the refresh marker on our behalf — release it
-              // so the slot is not held for a key we are dropping.
-              if (swrResult.shouldRefresh) this.l1.cancelRefresh(cacheKey);
-            } else {
+            let decoded: { value: TResult } | null = null;
+            try {
+              decoded = await this.decodeL1Entry<TResult>(cacheKey, swrResult.value, interop);
+            } finally {
+              // Decode failed — by rethrow (degradation off) or by null
+              // (degradation on) — so the entry is dropped either way, and the
+              // marker getWithSwr took on our behalf must not outlive it:
+              // release the slot rather than strand it for the marker TTL.
+              if (decoded === null && swrResult.shouldRefresh) this.l1.cancelRefresh(cacheKey);
+            }
+            if (decoded !== null) {
               // Trigger background refresh if needed
               if (swrResult.shouldRefresh) {
                 this.backgroundRefresh.scheduleRefresh(
diff --git a/packages/cachekit/src/cache.encryption-l1.test.ts b/packages/cachekit/src/cache.encryption-l1.test.ts
index 24c2e88..3b87db7 100644
--- a/packages/cachekit/src/cache.encryption-l1.test.ts
+++ b/packages/cachekit/src/cache.encryption-l1.test.ts
@@ -15,7 +15,7 @@
  * background refresh.
  */
 
-import { randomBytes } from 'node:crypto';
+import { createHash } from 'node:crypto';
 import { describe, it, expect, afterEach, vi } from 'vitest';
 import { createCache } from './cache.js';
 import { generateKey } from './serialization/key-generator.js';
@@ -23,7 +23,10 @@ import type { SecureCache } from './types/cache.js';
 import type { Backend } from './backends/types.js';
 import type { L1Cache } from './l1/lru-cache.js';
 
-const MASTER_KEY = randomBytes(32).toString('hex');
+// Deterministic (reproducible runs, no random source) yet derived at runtime
+// from a public fixture string — not a hardcoded key literal a secret scanner
+// should ever match. No assertion depends on the key's value.
+const MASTER_KEY = createHash('sha256').update('cachekit LAB-238 test fixture').digest('hex');
 const TENANT = 'lab-238';
 
 /** Distinctive enough that a substring search over any dump is conclusive. */
@@ -148,7 +151,11 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
     // Wait for the refresh to actually land in L1 — the factory running is not
     // enough, completeRefresh only runs once the L2 write has resolved.
     await vi.waitFor(() => {
-      expect(l1Entry(cache, key)).not.toEqual(firstCiphertext);
+      // Present AND changed: a bare not.toEqual would also pass on null once
+      // the TTL expires, misreporting a slow refresh as a format failure below.
+      const entry = l1Entry(cache, key);
+      expect(entry).toBeInstanceOf(Uint8Array);
+      expect(entry).not.toEqual(firstCiphertext);
     });
 
     expectCiphertext(l1Entry(cache, key), backend.store.get(key));
@@ -256,21 +263,27 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
     let originCalls = 0;
     const load = cache.wrap(async (_id: number) => ({ ssn: LEAK_CANARY, n: ++originCalls }), {
       namespace: 'users',
-      ttl: 2,
+      // 4s TTL read from 2.4s: stale against the 1.8-2.2s threshold (0.5
+      // ratio, ±10% jitter) for every jitter draw, with 1.6s of lifetime left
+      // so the whole read loop below finishes well before the entry expires —
+      // an expiry mid-loop would send every later read down the cold path and
+      // count origin calls that have nothing to do with stampede control.
+      ttl: 4,
     });
 
     await load(7);
     expect(originCalls).toBe(1);
 
     writesFail = true;
-    await new Promise((r) => setTimeout(r, 1400));
+    await new Promise((r) => setTimeout(r, 2400));
 
-    // Ten stale reads during the outage, spaced so each scheduled refresh
+    // Stale reads during the outage, spaced so each scheduled refresh
     // settles before the next read — otherwise the in-flight marker masks the
     // behaviour under test. A refresh that stored something resets the entry's
     // freshness, so the origin is touched a couple of times; one that stored
     // nothing and released its marker would be re-armed by every single read.
-    for (let i = 0; i < 10; i++) {
+    const READS = 10;
+    for (let i = 0; i < READS; i++) {
       await load(7);
       await new Promise((r) => setTimeout(r, 10));
     }
@@ -278,7 +291,8 @@ describe('L1 zero-knowledge for encrypted caches (LAB-238)', () => {
     await vi.waitFor(() => {
       expect(originCalls).toBeGreaterThan(1);
     });
-    expect(originCalls).toBeLessThan(5);
+    // The signal is "far fewer origin calls than reads", not an absolute count.
+    expect(originCalls).toBeLessThan(READS / 2);
   });
 
   it('leaves plaintext caches storing decoded values (unchanged, non-goal)', async () => {
diff --git a/packages/cachekit/src/cache/background-refresh.test.ts b/packages/cachekit/src/cache/background-refresh.test.ts
index 1731783..7a7f170 100644
--- a/packages/cachekit/src/cache/background-refresh.test.ts
+++ b/packages/cachekit/src/cache/background-refresh.test.ts
@@ -135,8 +135,9 @@ describe('BackgroundRefreshManager', () => {
         expect(secretPersist).toHaveBeenCalled();
       });
 
+      // Reference identity to the persist callback's ciphertext is the whole
+      // proof: the factory's plaintext object cannot be this Uint8Array.
       expect(l1Cache.get('key1')).toBe(ciphertext);
-      expect(JSON.stringify(l1Cache.get('key1'))).not.toContain('000-00-0000');
     });
 
     it('holds the refresh marker when the write returns nothing storable', async () => {
@@ -148,10 +149,12 @@ describe('BackgroundRefreshManager', () => {
       const degraded = vi.fn(async () => null);
       const computeFn = vi.fn().mockResolvedValue({ data: 'fresh' });
 
-      // ttl well past the SWR threshold so the read below is stale and takes
-      // the marker.
-      l1Cache.set('key1', { data: 'stale' }, 1000, 'test');
-      await new Promise((r) => setTimeout(r, 600));
+      // Stale but far from expiry: 4s TTL read at 2.4s is past the 1.8-2.2s
+      // threshold (0.5 ratio, ±10% jitter) for every jitter draw, and the
+      // 1.6s of remaining lifetime dwarfs the refresh round-trip — a loaded
+      // box must not expire the entry before the final read asserts on it.
+      l1Cache.set('key1', { data: 'stale' }, 4000, 'test');
+      await new Promise((r) => setTimeout(r, 2400));
       const stale = l1Cache.getWithSwr('key1');
       expect(stale.shouldRefresh).toBe(true);
 
diff --git a/packages/cachekit/src/logger.test.ts b/packages/cachekit/src/logger.test.ts
index 9d8eb93..8915ce5 100644
--- a/packages/cachekit/src/logger.test.ts
+++ b/packages/cachekit/src/logger.test.ts
@@ -65,7 +65,8 @@ describe('pluggable logger (LAB-517)', () => {
       { ttl: 60, namespace: 'ns' },
       0,
       null,
-      async () => {}
+      // PersistCallback contract: null = "nothing for L1 to hold".
+      async () => null
     );
 
     await vi.waitFor(() => {

From 3fffbb5afd8aa494b662eb571fd83169326eb398 Mon Sep 17 00:00:00 2001
From: Ray Walker <ray@27b.io>
Date: Sat, 8 Aug 2026 08:20:11 +1000
Subject: [PATCH 5/5] =?UTF-8?q?test(review):=20deterministic=20SWR=20refre?=
 =?UTF-8?q?sh=20tests=20=E2=80=94=20fake=20timers,=20pinned=20jitter,=20aw?=
 =?UTF-8?q?aited=20refresh=20promises=20(LAB-238)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- ciphertext-residency test now uses distinct stale/refreshed ciphertexts
  and awaits the refresh promise (settles after the L1 update), so it
  fails if the refresh does not actually replace L1's entry
- marker-hold test runs on a frozen clock with the jitter draw pinned to
  the midpoint: threshold is exactly 2s on a 4s TTL, no wall-clock races
- compute-failure test now takes a real refresh marker first and asserts
  cancelRefresh released it — previously the cancel half was unfalsifiable
---
 .../src/cache/background-refresh.test.ts      | 95 ++++++++++++-------
 1 file changed, 62 insertions(+), 33 deletions(-)

diff --git a/packages/cachekit/src/cache/background-refresh.test.ts b/packages/cachekit/src/cache/background-refresh.test.ts
index 7a7f170..18e8b7d 100644
--- a/packages/cachekit/src/cache/background-refresh.test.ts
+++ b/packages/cachekit/src/cache/background-refresh.test.ts
@@ -2,6 +2,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { BackgroundRefreshManager } from './background-refresh.js';
 import { L1Cache } from '../l1/lru-cache.js';
 
+// Pin the SWR jitter draw so tests are deterministic. 0.5 lands the
+// jitter factor at exactly 1.0, so the refresh threshold is exactly
+// originalTtl * swrThresholdRatio — no probabilistic window in any test here.
+vi.mock('../utils/random.js', () => ({
+  secureRandomFloat: () => 0.5,
+}));
+
 describe('BackgroundRefreshManager', () => {
   let manager: BackgroundRefreshManager;
   let l1Cache: L1Cache;
@@ -18,6 +25,7 @@ describe('BackgroundRefreshManager', () => {
   });
 
   afterEach(() => {
+    vi.useRealTimers();
     consoleSpy.mockRestore();
     manager.close();
     l1Cache.clear();
@@ -115,29 +123,33 @@ describe('BackgroundRefreshManager', () => {
       // A secure cache's persist callback returns the ciphertext it wrote to
       // L2. That, and never the factory's plaintext result, is what lands in
       // L1 — otherwise the SWR refresh re-poisons L1 on every revalidation.
-      const ciphertext = new Uint8Array([0xde, 0xad, 0xbe, 0xef]);
-      const secretPersist = vi.fn(async () => ({ l1: ciphertext }));
+      const staleCiphertext = new Uint8Array([0xca, 0xfe, 0xba, 0xbe]);
+      const refreshedCiphertext = new Uint8Array([0xde, 0xad, 0xbe, 0xef]);
+      const secretPersist = vi.fn(async () => ({ l1: refreshedCiphertext }));
       const computeFn = vi.fn().mockResolvedValue({ ssn: '000-00-0000' });
 
-      l1Cache.set('key1', ciphertext, 3600000, 'test');
+      l1Cache.set('key1', staleCiphertext, 3600000, 'test');
       const { versionToken } = l1Cache.getWithSwr('key1');
 
+      let refreshDone!: Promise<unknown>;
       manager.scheduleRefresh(
         'key1',
         computeFn,
         { ttl: 3600, namespace: 'test' },
         versionToken,
         l1Cache,
-        secretPersist
+        secretPersist,
+        (promise) => {
+          refreshDone = promise;
+        }
       );
-
-      await vi.waitFor(() => {
-        expect(secretPersist).toHaveBeenCalled();
-      });
+      // The refresh promise settles only after the L1 update landed.
+      await refreshDone;
 
       // Reference identity to the persist callback's ciphertext is the whole
-      // proof: the factory's plaintext object cannot be this Uint8Array.
-      expect(l1Cache.get('key1')).toBe(ciphertext);
+      // proof: neither the stale entry nor the factory's plaintext object can
+      // be this Uint8Array — so the refresh demonstrably replaced L1's entry.
+      expect(l1Cache.get('key1')).toBe(refreshedCiphertext);
     });
 
     it('holds the refresh marker when the write returns nothing storable', async () => {
@@ -149,27 +161,32 @@ describe('BackgroundRefreshManager', () => {
       const degraded = vi.fn(async () => null);
       const computeFn = vi.fn().mockResolvedValue({ data: 'fresh' });
 
-      // Stale but far from expiry: 4s TTL read at 2.4s is past the 1.8-2.2s
-      // threshold (0.5 ratio, ±10% jitter) for every jitter draw, and the
-      // 1.6s of remaining lifetime dwarfs the refresh round-trip — a loaded
-      // box must not expire the entry before the final read asserts on it.
+      // Frozen clock + pinned jitter (vi.mock at top of file): with a 4s TTL
+      // the refresh threshold is exactly 2s, so a read at t=2.4s is
+      // deterministically stale and the entry deterministically alive until
+      // t=4s. Time only moves when this test says so — no wall-clock races.
+      vi.useFakeTimers();
       l1Cache.set('key1', { data: 'stale' }, 4000, 'test');
-      await new Promise((r) => setTimeout(r, 2400));
+      vi.advanceTimersByTime(2400);
       const stale = l1Cache.getWithSwr('key1');
       expect(stale.shouldRefresh).toBe(true);
 
+      let refreshDone!: Promise<unknown>;
       manager.scheduleRefresh(
         'key1',
         computeFn,
         { ttl: 3600, namespace: 'test' },
         stale.versionToken,
         l1Cache,
-        degraded
+        degraded,
+        (promise) => {
+          refreshDone = promise;
+        }
       );
-
-      await vi.waitFor(() => {
-        expect(degraded).toHaveBeenCalled();
-      });
+      // The refresh is pure microtasks (no timers) — awaiting its promise
+      // needs no timer advancement and settles after the degraded write.
+      await refreshDone;
+      expect(degraded).toHaveBeenCalled();
 
       expect(l1Cache.get('key1')).toEqual({ data: 'stale' });
       expect(l1Cache.stats.refreshing).toBe(1);
@@ -181,30 +198,42 @@ describe('BackgroundRefreshManager', () => {
       const error = new Error('Compute failed');
       const computeFn = vi.fn().mockRejectedValue(error);
 
-      // Set initial value in L1
-      l1Cache.set('key1', { data: 'old' }, 3600000, 'test');
-
-      // Trigger SWR to mark as refreshing
-      l1Cache.getWithSwr('key1');
+      // A fresh entry never takes a refresh marker, which would leave the
+      // cancel half of this test asserting nothing — so make the entry
+      // deterministically stale first (same frozen-clock setup as above).
+      vi.useFakeTimers();
+      l1Cache.set('key1', { data: 'old' }, 4000, 'test');
+      vi.advanceTimersByTime(2400);
+      const stale = l1Cache.getWithSwr('key1');
+      expect(stale.shouldRefresh).toBe(true);
+      expect(l1Cache.stats.refreshing).toBe(1);
 
+      let refreshDone!: Promise<unknown>;
       manager.scheduleRefresh(
         'key1',
         computeFn,
         { ttl: 3600, namespace: 'test' },
-        0,
+        stale.versionToken,
         l1Cache,
-        persistToL2
+        persistToL2,
+        (promise) => {
+          refreshDone = promise;
+        }
       );
+      await refreshDone;
 
-      await vi.waitFor(() => {
-        expect(consoleSpy).toHaveBeenCalledWith(
-          '[cachekit] Background refresh failed:',
-          'Compute failed'
-        );
-      });
+      expect(consoleSpy).toHaveBeenCalledWith(
+        '[cachekit] Background refresh failed:',
+        'Compute failed'
+      );
 
       // persistToL2 should not be called on error
       expect(persistToL2).not.toHaveBeenCalled();
+
+      // The failed refresh released its marker via cancelRefresh — unlike
+      // the degraded-write case above, a compute error must allow the next
+      // read to re-arm a refresh immediately.
+      expect(l1Cache.stats.refreshing).toBe(0);
     });
 
     it('should skip refresh if manager is closed', async () => {