-
Notifications
You must be signed in to change notification settings - Fork 0
fix(cache)!: secure.wrap() fails closed when encryption is not configured (LAB-513) #123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
27Bslash6
wants to merge
3
commits into
main
Choose a base branch
from
lab-513-secure-wrap-fails-closed
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+185
−12
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
24f4747
fix(cache)!: secure.wrap() fails closed when encryption is not config…
27Bslash6 5f53dc8
test(cache): pin secure.wrap waitUntil plumbing and de-fang LZ4 in th…
27Bslash6 b3e44bc
test(cache): drive the SWR refresh through view.secure.wrap; narrow w…
27Bslash6 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| /** | ||
| * 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, vi } from 'vitest'; | ||
| import { createCache } from './cache.js'; | ||
| import { ConfigurationError } from './errors.js'; | ||
| import { CacheImpl, type ExecutionContextLike } from './cache-core.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 | ||
| // 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 }; | ||
| /** 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<string, Uint8Array>(); | ||
|
|
||
| async get(key: string): Promise<Uint8Array | null> { | ||
| 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 Workers request-scoped view. The method lives on CacheImpl, not on the Node type. */ | ||
| function viewOf( | ||
| cache: SecureCache, | ||
| ctx: ExecutionContextLike = { waitUntil: () => {} } | ||
| ): SecureCache { | ||
| 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, | ||
| l1?: CacheOptions['l1'] | ||
| ): { cache: SecureCache; backend: InMemoryBackend } { | ||
| const backend = new InMemoryBackend(); | ||
| 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, | ||
| ...(l1 ? { l1 } : {}), | ||
| ...(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 } = 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\(\)/); | ||
| }); | ||
|
|
||
| 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('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 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<unknown>) => 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 () => { | ||
| 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); | ||
| } | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.