Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/providers/key-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -185,6 +195,18 @@ export function restoreProviderKeyFromKeychain(config: OcxConfig, name: string):
const pool = provider.apiKeyPool ?? [];
const resolved = new Map<string, string>();
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;
Expand Down
2 changes: 2 additions & 0 deletions src/server/management/config-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise<Respon
const webSearchCandidates = await webSearchCandidateRows(config);
return jsonResponse({
webSearch: {
enabled: ws.enabled !== false,
Comment thread
lidge-jun marked this conversation as resolved.
model: ws.model ?? "gpt-5.6-luna",
backend: ws.backend,
streamRoutedModelOutput: ws.streamRoutedModelOutput === true,
Expand Down Expand Up @@ -939,6 +940,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise<Respon
return jsonResponse({
ok: true,
webSearch: {
enabled: ws.enabled !== false,
model: ws.model ?? "gpt-5.6-luna",
backend: ws.backend,
streamRoutedModelOutput: ws.streamRoutedModelOutput === true,
Expand Down
32 changes: 31 additions & 1 deletion tests/providers/provider-key-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -192,4 +223,3 @@ describe("store / restore", () => {
}
});
});

12 changes: 12 additions & 0 deletions tests/vision/sidecar-settings-vision-controls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions tests/vision/vision-anthropic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ describe("Anthropic vision planning and management config", () => {
config,
);
const getBody = await get!.json() as Record<string, any>;
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",
Expand All @@ -363,7 +363,7 @@ describe("Anthropic vision planning and management config", () => {
);
expect(clear.status).toBe(200);
const clearBody = await clear.json() as Record<string, any>;
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",
Expand Down
Loading