From 924b65799d32f22e1ec0bdb8cc216897bc8527ef Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:02:32 +0900 Subject: [PATCH 1/2] fix(providers): refuse keychain restore across provider ownership [skip ci] (cherry picked from commit 73abec9a995d157e3acb55edfd7d8f082cc11aae) Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/providers/key-store.ts | 22 +++++++++++++++ tests/providers/provider-key-store.test.ts | 32 +++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) 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/tests/providers/provider-key-store.test.ts b/tests/providers/provider-key-store.test.ts index 645920e32c..1197a1fea9 100644 --- a/tests/providers/provider-key-store.test.ts +++ b/tests/providers/provider-key-store.test.ts @@ -156,6 +156,37 @@ describe("store / restore", () => { 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", () => { } }); }); - From 6eadb165873a33d8589e46bf5256a61524d82ec7 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 18:18:33 +0900 Subject: [PATCH 2/2] fix(management): expose the effective web-search sidecar enabled state GET and PUT /api/sidecar-settings return webSearch.model/backend/stream flags but omitted webSearch.enabled, so the dashboard could not tell whether the sidecar master switch was off. Serialize the effective value (unset reads as enabled) on both responses. Reimplements https://github.com/lidge-jun/opencodex/pull/2033 on current dev (the original branch is 1364 commits behind and its test moved to tests/vision/ under the test-layout split). Co-authored-by: louis-tepe <161487332+louis-tepe@users.noreply.github.com> --- src/server/management/config-routes.ts | 2 ++ .../vision/sidecar-settings-vision-controls.test.ts | 12 ++++++++++++ tests/vision/vision-anthropic.test.ts | 4 ++-- 3 files changed, 16 insertions(+), 2 deletions(-) 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(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",