diff --git a/src/lib/server/secretproviders/probe-cache.ts b/src/lib/server/secretproviders/probe-cache.ts index 9ffc7b7..dbc76ab 100644 --- a/src/lib/server/secretproviders/probe-cache.ts +++ b/src/lib/server/secretproviders/probe-cache.ts @@ -1,10 +1,14 @@ /** * Short-lived cache for live provider probes. * - * The stack editor probes the bound provider on every debounced keystroke to - * classify a compose ${VAR} as present ("IN VAULT") or missing. Without a cache - * a fast typist would hammer the provider's API. This caches ONLY the KEY NAMES - * of a bulk selector for a few seconds - never the secret values. + * The stack editor probes the bound provider to classify a compose ${VAR} as + * present ("IN VAULT") or missing. Without protection a busy editor page (debounced + * keystrokes + the list auto-refresh) fires overlapping probes, and each one would + * hit the provider: wasted 1Password/Vault API calls, and for the CLI providers a + * burst of keepassxc-cli/bws spawns (each re-deriving the key). Two guards: + * - a ~30s result cache (KEY NAMES only, never values), and + * - single-flight: probes for the same (providerId, selector) that arrive while a + * call is already in flight share that one Promise instead of spawning their own. */ import type { SecretProvider, SecretProviderConfig } from './shared'; @@ -16,14 +20,20 @@ interface Entry { at: number; } -// Keyed by `${providerId}:${selector}`. Bounded by (provider count * distinct -// selectors seen in the TTL window); stale entries are dropped lazily on read. +// Keyed by `${providerId}:${selector}`. A stale entry is deleted when read, so the +// map self-trims to selectors probed within the TTL window rather than growing +// forever across a live editing session (every keystroke is a distinct selector). const cache = new Map(); +// In-flight probes, keyed the same way, so concurrent callers coalesce onto one +// provider call. Cleared when the call settles (success or failure). +const inflight = new Map>(); + /** * Returns the KEY NAMES available under a provider's bulk selector, cached for * ~30s per (providerId, selector). Values are discarded immediately - only the - * names are stored and returned. Propagates the provider's errors to the caller. + * names are stored and returned. Concurrent callers for the same key coalesce onto + * a single provider call. Propagates the provider's errors to the caller. */ export async function probeBulkKeysCached( providerId: number, @@ -34,11 +44,24 @@ export async function probeBulkKeysCached( const cacheKey = `${providerId}:${selector}`; const now = Date.now(); const hit = cache.get(cacheKey); - if (hit && now - hit.at < TTL_MS) { - return hit.keys; + if (hit) { + if (now - hit.at < TTL_MS) return hit.keys; + cache.delete(cacheKey); // stale: drop it so the map doesn't grow unbounded } - const bulk = await provider.resolveBulk(config, selector); - const keys = Object.keys(bulk); - cache.set(cacheKey, { keys, at: now }); - return keys; + + // Coalesce onto an existing in-flight call for the same key. + const pending = inflight.get(cacheKey); + if (pending) return pending; + + const call = (async () => { + const bulk = await provider.resolveBulk(config, selector); + const keys = Object.keys(bulk); + cache.set(cacheKey, { keys, at: Date.now() }); + return keys; + })().finally(() => { + inflight.delete(cacheKey); + }); + + inflight.set(cacheKey, call); + return call; } diff --git a/tests/secret-provider-probe-cache.test.ts b/tests/secret-provider-probe-cache.test.ts new file mode 100644 index 0000000..d1e50e7 --- /dev/null +++ b/tests/secret-provider-probe-cache.test.ts @@ -0,0 +1,90 @@ +/** + * probeBulkKeysCached: result cache (~30s, names only) + single-flight coalescing. + * The single-flight guard stops overlapping stack-editor probes from each hitting the + * provider (wasted 1Password/Vault API calls; a burst of keepassxc-cli/bws spawns). + */ +import { describe, it, expect } from 'bun:test'; +import { probeBulkKeysCached } from '../src/lib/server/secretproviders/probe-cache'; + +// A provider whose resolveBulk is slow and counts its own calls. Cast to the +// SecretProvider shape (only resolveBulk is exercised here). +function countingProvider(delayMs = 30) { + let calls = 0; + const provider: any = { + supportsBulk: true, + async resolveBulk() { + calls++; + await new Promise((r) => setTimeout(r, delayMs)); + return { KEY_A: 'a', KEY_B: 'b' }; + } + }; + return { provider, calls: () => calls }; +} + +describe('probeBulkKeysCached', () => { + it('coalesces concurrent probes for the same (provider, selector) into ONE call', async () => { + const { provider, calls } = countingProvider(); + const id = 1001; // unique id so cache/inflight don't collide with other tests + const results = await Promise.all( + Array.from({ length: 10 }, () => probeBulkKeysCached(id, provider, {} as any, 'grp')) + ); + expect(calls()).toBe(1); // single-flight: 10 concurrent -> 1 provider call + // every caller still gets the same key names + for (const r of results) expect(r.sort()).toEqual(['KEY_A', 'KEY_B']); + }); + + it('serves the cached result on a later call without re-calling the provider', async () => { + const { provider, calls } = countingProvider(); + const id = 1002; + await probeBulkKeysCached(id, provider, {} as any, 'grp'); + await probeBulkKeysCached(id, provider, {} as any, 'grp'); + expect(calls()).toBe(1); // second call hit the ~30s cache + }); + + it('does NOT coalesce different selectors', async () => { + const { provider, calls } = countingProvider(); + const id = 1003; + await Promise.all([ + probeBulkKeysCached(id, provider, {} as any, 'grp-a'), + probeBulkKeysCached(id, provider, {} as any, 'grp-b') + ]); + expect(calls()).toBe(2); // distinct keys -> distinct calls + }); + + it('re-probes (does not serve a stale entry) once the TTL has elapsed', async () => { + // The cache TTL is 30s; we can't wait that long, so drive it by advancing the + // clock via a stubbed Date.now. A stale hit must be dropped and re-fetched. + const { provider, calls } = countingProvider(); + const id = 1005; + const realNow = Date.now; + try { + let t = 1_000_000; + (Date as any).now = () => t; + await probeBulkKeysCached(id, provider, {} as any, 'grp'); // call 1, cached at t + t += 31_000; // jump past the 30s TTL + await probeBulkKeysCached(id, provider, {} as any, 'grp'); // stale -> call 2 + expect(calls()).toBe(2); + } finally { + (Date as any).now = realNow; + } + }); + + it('does not cache a failure, and a later probe retries', async () => { + let calls = 0; + const provider: any = { + supportsBulk: true, + async resolveBulk() { + calls++; + if (calls === 1) throw new Error('transient'); + return { OK: 'v' }; + } + }; + const id = 1004; + await expect(probeBulkKeysCached(id, provider, {} as any, 'grp')).rejects.toThrow('transient'); + // the in-flight entry must have been cleared, so this retries rather than + // returning a stale/failed promise + const keys = await probeBulkKeysCached(id, provider, {} as any, 'grp'); + expect(keys).toEqual(['OK']); + expect(calls).toBe(2); + }); +});