diff --git a/src/providers/key-store.ts b/src/providers/key-store.ts index 12e4ce6cb7..614fd3372f 100644 --- a/src/providers/key-store.ts +++ b/src/providers/key-store.ts @@ -64,6 +64,16 @@ function keychainAccount(reference: string): string { return reference.slice(KEYCHAIN_REFERENCE_PREFIX.length); } +/** + * A reference belongs to `name` only when its account is that provider's own active account + * or one of its pool accounts. `storeProviderKeyInKeychain` writes exactly those two shapes, + * so anything else in a provider's config names another provider's secret. + */ +function keychainReferenceBelongsToProvider(reference: string, name: string): boolean { + const account = keychainAccount(reference); + return account === name || account.startsWith(`${name}/`); +} + function readKeychain(account: string): string | undefined { const cached = resolvedCache.get(account); if (cached !== undefined) return cached; @@ -185,6 +195,18 @@ export function restoreProviderKeyFromKeychain(config: OcxConfig, name: string): const pool = provider.apiKeyPool ?? []; const resolved = new Map(); const refs = [provider.apiKey, ...pool.map(e => e.key)].filter(isKeychainReference); + // Restore reads a secret out of the keychain, writes it back to config as plaintext, and then + // DELETES the keychain item. Following a reference to another provider's account would both + // disclose that secret through this provider's config and destroy the real owner's credential, + // so refuse before anything is read or removed. + const foreign = refs.filter(ref => !keychainReferenceBelongsToProvider(ref, name)); + if (foreign.length > 0) { + return { + ok: false, + error: `provider "${name}" references a keychain account it does not own (${foreign.length} reference(s)); config left unchanged`, + status: 400, + }; + } for (const ref of refs) { const account = keychainAccount(ref); if (resolved.has(account)) continue; diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 4d551a886d..08f4b85d27 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -701,6 +701,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise { expect(probeProviderKeychain().available).toBe(false); }); + test("restore refuses a reference to another provider's keychain account", () => { + const { store, factory } = fakeKeychain(); + setProviderKeychainEntryFactoryForTests(factory); + const config = loadConfig(); + config.providers.other = { adapter: "openai-chat", baseUrl: "https://other.example/v1", apiKey: POOL_SECRET }; + expect(storeProviderKeyInKeychain(config, "other")).toEqual({ ok: true, moved: 1 }); + expect(config.providers.other!.apiKey).toBe("keychain:other"); + + // Point "relay" at the account "other" owns. Restore would otherwise read that secret, + // write it into relay's config as plaintext, and delete the owner's keychain item. + config.providers.relay!.apiKey = "keychain:other"; + const result = restoreProviderKeyFromKeychain(config, "relay"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + + expect(config.providers.relay!.apiKey).toBe("keychain:other"); + expect(readFileSync(join(testDir, "config.json"), "utf8")).not.toContain(POOL_SECRET); + // The real owner's secret is still in the keychain and still resolves for that provider. + expect(store.size).toBe(1); + expect(resolveProviderApiKey(config.providers.other!.apiKey)).toBe(POOL_SECRET); + }); + + test("restore still accepts a provider's own active and pool accounts", () => { + const { factory } = fakeKeychain(); + setProviderKeychainEntryFactoryForTests(factory); + const config = loadConfig(); + config.providers.relay!.apiKeyPool = [{ id: "a1", key: SECRET }, { id: "b2", key: POOL_SECRET }]; + expect(storeProviderKeyInKeychain(config, "relay")).toEqual({ ok: true, moved: 2 }); + expect(restoreProviderKeyFromKeychain(config, "relay")).toEqual({ ok: true, restored: 2 }); + }); + test("management route: GET reports store kind, POST store/restore round-trips", async () => { const { factory } = fakeKeychain(); setProviderKeychainEntryFactoryForTests(factory); @@ -192,4 +223,3 @@ describe("store / restore", () => { } }); }); - diff --git a/tests/vision/sidecar-settings-vision-controls.test.ts b/tests/vision/sidecar-settings-vision-controls.test.ts index 4fb4028ee7..a57b646f78 100644 --- a/tests/vision/sidecar-settings-vision-controls.test.ts +++ b/tests/vision/sidecar-settings-vision-controls.test.ts @@ -222,6 +222,18 @@ describe("sidecar-settings remaining vision controls", () => { expect(config.visionSidecar).toEqual({ ...FULL_VISION, enabled: false }); }); + test("GET and PUT expose the effective web-search enabled state", async () => { + const unset = await getSidecarSettings(emptyConfig()); + expect((await unset.json() as { webSearch: { enabled: boolean } }).webSearch.enabled).toBe(true); + + const config = emptyConfig({ webSearchSidecar: { enabled: false } }); + const disabled = await getSidecarSettings(config); + expect((await disabled.json() as { webSearch: { enabled: boolean } }).webSearch.enabled).toBe(false); + + const response = await putSidecarSettings(config, { webSearch: { streamRoutedModelOutput: true } }); + expect((await response.json() as { webSearch: { enabled: boolean } }).webSearch.enabled).toBe(false); + }); + test("timeoutMs validation reuses the runtime bounds rather than a second contract", async () => { expect(resolveVisionTimeoutMs(undefined)).toBe(DEFAULT_VISION_TIMEOUT_MS); expect(resolveVisionTimeoutMs(MIN_VISION_TIMEOUT_MS)).toBe(MIN_VISION_TIMEOUT_MS); diff --git a/tests/vision/vision-anthropic.test.ts b/tests/vision/vision-anthropic.test.ts index ee4b01b421..086e5df7bb 100644 --- a/tests/vision/vision-anthropic.test.ts +++ b/tests/vision/vision-anthropic.test.ts @@ -339,7 +339,7 @@ describe("Anthropic vision planning and management config", () => { config, ); const getBody = await get!.json() as Record; - expect(getBody.webSearch).toEqual({ model: "claude-haiku-4-5", backend: "anthropic", streamRoutedModelOutput: false }); + expect(getBody.webSearch).toEqual({ enabled: true, model: "claude-haiku-4-5", backend: "anthropic", streamRoutedModelOutput: false }); expect(getBody.vision).toEqual({ enabled: true, model: "claude-sonnet-5", @@ -363,7 +363,7 @@ describe("Anthropic vision planning and management config", () => { ); expect(clear.status).toBe(200); const clearBody = await clear.json() as Record; - expect(clearBody.webSearch).toEqual({ model: "gpt-5.6-luna", streamRoutedModelOutput: false }); + expect(clearBody.webSearch).toEqual({ enabled: true, model: "gpt-5.6-luna", streamRoutedModelOutput: false }); expect(clearBody.vision).toEqual({ enabled: true, model: "gpt-5.4-mini",