From e94b77ddb754a3060d88859e67cb40919476efa6 Mon Sep 17 00:00:00 2001 From: Jarek Krochmalski Date: Wed, 9 Sep 2026 09:37:27 +0530 Subject: [PATCH] cache 1password reference probes so the stack editor stops re-billing the api limit --- src/lib/server/secretproviders/probe-cache.ts | 55 ++++++++ .../secret-providers/[id]/probe/+server.ts | 5 +- tests/secret-provider-probe-cache.test.ts | 119 +++++++++++++++++- 3 files changed, 175 insertions(+), 4 deletions(-) diff --git a/src/lib/server/secretproviders/probe-cache.ts b/src/lib/server/secretproviders/probe-cache.ts index dbc76ab..d16ce05 100644 --- a/src/lib/server/secretproviders/probe-cache.ts +++ b/src/lib/server/secretproviders/probe-cache.ts @@ -29,6 +29,21 @@ const cache = new Map(); // provider call. Cleared when the call settles (success or failure). const inflight = new Map>(); +// Same two guards for the INLINE op:// reference probe (resolveSecretReferences), +// keyed by (providerId + the sorted ref set). Separate maps from the bulk ones so a +// bulk selector and a set of refs never collide. This is the path that drained the +// 1Password limit "just by opening the editor" (#1436): runProbe fires on open and +// on every debounced edit, and each fire resolved every op:// ref again with no cache. +const refsCache = new Map(); +const refsInflight = new Map>(); + +/** Stable cache key for a ref set: order-independent (sorted) so the same refs typed + * in a different order hit one entry, and JSON-encoded so a ref value that itself + * contains the separator can't collide two different sets onto one key. */ +function refsKey(providerId: number, refs: string[]): string { + return `${providerId}:${JSON.stringify([...refs].sort())}`; +} + /** * Returns the KEY NAMES available under a provider's bulk selector, cached for * ~30s per (providerId, selector). Values are discarded immediately - only the @@ -65,3 +80,43 @@ export async function probeBulkKeysCached( inflight.set(cacheKey, call); return call; } + +/** + * Returns which of the given inline op:// (etc.) references currently RESOLVE in the + * provider, cached ~30s per (providerId, ref set) with the same single-flight guard as + * the bulk path. Only the resolved reference STRINGS are returned - secret values are + * discarded immediately, never stored. This is what keeps the editor's live probe from + * re-billing the provider's rate limit on every open and every debounced keystroke. + * Propagates the provider's errors to the caller. + */ +export async function probeRefsCached( + providerId: number, + provider: SecretProvider, + config: SecretProviderConfig, + refs: string[] +): Promise { + if (refs.length === 0) return []; + + const cacheKey = refsKey(providerId, refs); + const now = Date.now(); + const hit = refsCache.get(cacheKey); + if (hit) { + if (now - hit.at < TTL_MS) return hit.keys; + refsCache.delete(cacheKey); + } + + const pending = refsInflight.get(cacheKey); + if (pending) return pending; + + const call = (async () => { + const resolved = await provider.resolveSecretReferences(config, refs); + const names = [...resolved.keys()]; // ref strings only; values dropped + refsCache.set(cacheKey, { keys: names, at: Date.now() }); + return names; + })().finally(() => { + refsInflight.delete(cacheKey); + }); + + refsInflight.set(cacheKey, call); + return call; +} diff --git a/src/routes/api/secret-providers/[id]/probe/+server.ts b/src/routes/api/secret-providers/[id]/probe/+server.ts index 57779fd..baed890 100644 --- a/src/routes/api/secret-providers/[id]/probe/+server.ts +++ b/src/routes/api/secret-providers/[id]/probe/+server.ts @@ -4,7 +4,7 @@ import { getSecretProviderById } from '$lib/server/db'; import { authorize } from '$lib/server/authorize'; import { getProvider } from '$lib/server/secretproviders'; import { UnsupportedOperationError, parseProviderError } from '$lib/server/secretproviders/shared'; -import { probeBulkKeysCached } from '$lib/server/secretproviders/probe-cache'; +import { probeBulkKeysCached, probeRefsCached } from '$lib/server/secretproviders/probe-cache'; /** * Live probe of a stored provider: given a bulk selector and/or inline op:// @@ -75,8 +75,7 @@ export const POST: RequestHandler = async ({ params, cookies, request }) => { if (refs.length && provider.supportsReferences) { try { - const resolved = await provider.resolveSecretReferences(row.config, refs); - resolvedRefs = [...resolved.keys()]; + resolvedRefs = await probeRefsCached(id, provider, row.config, refs); } catch (e) { if (!(e instanceof UnsupportedOperationError)) { return json({ ok: false, error: shortError(e) }, { status: 200 }); diff --git a/tests/secret-provider-probe-cache.test.ts b/tests/secret-provider-probe-cache.test.ts index d1e50e7..1c58422 100644 --- a/tests/secret-provider-probe-cache.test.ts +++ b/tests/secret-provider-probe-cache.test.ts @@ -4,7 +4,7 @@ * 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'; +import { probeBulkKeysCached, probeRefsCached } 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). @@ -88,3 +88,120 @@ describe('probeBulkKeysCached', () => { expect(calls).toBe(2); }); }); + +// The inline-reference probe (op:// etc.) is the path that drained the 1Password +// limit "just by opening the editor" (#1436): runProbe fires on open and on every +// debounced edit, and each fire resolved every reference again with no cache. +function countingRefsProvider(delayMs = 30) { + let calls = 0; + const provider: any = { + supportsReferences: true, + async resolveSecretReferences(_config: any, refs: string[]) { + calls++; + await new Promise((r) => setTimeout(r, delayMs)); + // echo back a resolution for every ref (value discarded by the cache) + return new Map(refs.map((r) => [r, 'secret-value'])); + } + }; + return { provider, calls: () => calls }; +} + +describe('probeRefsCached', () => { + const REFS = ['op://vault/db/password', 'op://vault/api/token']; + + it('returns [] without calling the provider when there are no refs', async () => { + const { provider, calls } = countingRefsProvider(); + expect(await probeRefsCached(2001, provider, {} as any, [])).toEqual([]); + expect(calls()).toBe(0); + }); + + it('coalesces concurrent probes for the same (provider, refs) into ONE call', async () => { + const { provider, calls } = countingRefsProvider(); + const id = 2002; + const results = await Promise.all( + Array.from({ length: 10 }, () => probeRefsCached(id, provider, {} as any, REFS)) + ); + expect(calls()).toBe(1); // 10 concurrent editor probes -> 1 provider call + for (const r of results) expect(r.sort()).toEqual([...REFS].sort()); + }); + + it('serves the cached result on a later call without re-billing the provider', async () => { + const { provider, calls } = countingRefsProvider(); + const id = 2003; + await probeRefsCached(id, provider, {} as any, REFS); + await probeRefsCached(id, provider, {} as any, REFS); // reopen / next keystroke + expect(calls()).toBe(1); // second call hit the ~30s cache + }); + + it('is order-independent: the same refs in a different order share one cache entry', async () => { + const { provider, calls } = countingRefsProvider(); + const id = 2004; + await probeRefsCached(id, provider, {} as any, REFS); + await probeRefsCached(id, provider, {} as any, [...REFS].reverse()); + expect(calls()).toBe(1); + }); + + it('does NOT coalesce different ref sets', async () => { + const { provider, calls } = countingRefsProvider(); + const id = 2005; + await Promise.all([ + probeRefsCached(id, provider, {} as any, ['op://v/a/x']), + probeRefsCached(id, provider, {} as any, ['op://v/b/y']) + ]); + expect(calls()).toBe(2); + }); + + it('returns (and caches) ONLY the refs that resolved, not the full input set', async () => { + // The real resolveSecretReferences returns just the refs that resolved; the + // missing ones drive the editor's MISSING marker. The cache must store that + // resolved SUBSET (never the input refs), or a missing secret would show a + // false "IN VAULT". Provider resolves only the first ref. + let calls = 0; + const provider: any = { + supportsReferences: true, + async resolveSecretReferences(_c: any, r: string[]) { + calls++; + return new Map([[r[0], 'v']]); // second ref does NOT resolve + } + }; + const id = 2008; + const first = await probeRefsCached(id, provider, {} as any, REFS); + expect(first).toEqual([REFS[0]]); // only the resolved ref, not both + const second = await probeRefsCached(id, provider, {} as any, REFS); + expect(second).toEqual([REFS[0]]); // cached subset, still not the full input + expect(calls).toBe(1); + }); + + it('re-probes once the TTL has elapsed', async () => { + const { provider, calls } = countingRefsProvider(); + const id = 2006; + const realNow = Date.now; + try { + let t = 2_000_000; + (Date as any).now = () => t; + await probeRefsCached(id, provider, {} as any, REFS); + t += 31_000; // past the 30s TTL + await probeRefsCached(id, provider, {} as any, REFS); + 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 = { + supportsReferences: true, + async resolveSecretReferences(_c: any, refs: string[]) { + calls++; + if (calls === 1) throw new Error('rate limit exceeded'); + return new Map(refs.map((r) => [r, 'v'])); + } + }; + const id = 2007; + await expect(probeRefsCached(id, provider, {} as any, REFS)).rejects.toThrow('rate limit'); + const names = await probeRefsCached(id, provider, {} as any, REFS); + expect(names.sort()).toEqual([...REFS].sort()); + expect(calls).toBe(2); + }); +});