Review fix round 1. - Secret box v2 tokens: one HKDF key per purpose (byok, server-api-key, mcp) and AES-GCM AAD bound to the row (user id + provider; server id + field). MCP env and headers are sealed per field. open() refuses v1 tokens; the data upgrade re-seals v1 keys, v1 plaintext MCP values and the draft secretCipher, and marks ai_secrets_v2 only when nothing legacy is left (retried until the previous secret is back). - Unreadable secrets are reported (admin: unreadableSecrets with server and field; user: byok.keyUnreadable), logged at startup, and kept on save unless replaced or cleared (clearApiKey, clearSecrets). The admin and account pages show the warnings. - allowedOrigins and provider resolution no longer open MCP secrets. Co-Authored-By: Claude Opus 5.5 <[email protected]>
209 lines
14 KiB
TypeScript
209 lines
14 KiB
TypeScript
import { expect } from "bun:test";
|
|
import { forEachAiDb as forEachDb } from "./helpers";
|
|
import { createSecretBox } from "../src/crypto";
|
|
import { aiSettingsRepo, maskSecret, MaskedSecretError } from "../src/settings";
|
|
|
|
const box = createSecretBox("test-secret");
|
|
|
|
forEachDb("server config is empty by default and hides the key", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
expect(await r.getServer()).toEqual({ provider: null, mcpServers: [], allowedBaseUrls: [], unreadableSecrets: [] });
|
|
expect(await r.serverApiKey()).toBeNull();
|
|
|
|
await r.saveServer({ provider: { kind: "anthropic", model: "claude-opus-5-5", apiKey: "sk-ant-SECRET" }, mcpServers: [] });
|
|
const got = await r.getServer();
|
|
expect(got.provider).toEqual({ kind: "anthropic", model: "claude-opus-5-5", hasApiKey: true });
|
|
expect(JSON.stringify(got)).not.toContain("SECRET");
|
|
const raw = await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`;
|
|
expect(raw[0].config_json).not.toContain("SECRET");
|
|
expect(await r.serverApiKey()).toBe("sk-ant-SECRET");
|
|
|
|
// Saving without apiKey keeps the old key; clearApiKey removes it.
|
|
await r.saveServer({ provider: { kind: "anthropic", model: "other" }, mcpServers: [{ id: "fs", name: "Files", transport: "stdio", command: "x" }] });
|
|
expect(await r.serverApiKey()).toBe("sk-ant-SECRET");
|
|
expect((await r.getServer()).mcpServers).toHaveLength(1);
|
|
await r.saveServer({ provider: { kind: "openai", baseUrl: "http://ollama:11434/v1", model: "llama3", clearApiKey: true }, mcpServers: [] });
|
|
expect(await r.serverApiKey()).toBeNull();
|
|
expect((await r.getServer()).provider).toEqual({ kind: "openai", baseUrl: "http://ollama:11434/v1", model: "llama3", hasApiKey: false });
|
|
await r.saveServer({ provider: null, mcpServers: [] });
|
|
expect((await r.getServer()).provider).toBeNull();
|
|
});
|
|
|
|
forEachDb("BYOK keys are encrypted, per user and removable", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
expect((await r.getUser("u1")).byok).toBeNull();
|
|
await r.saveUserKey("u1", { kind: "anthropic", apiKey: "sk-user-SECRET" });
|
|
expect((await r.getUser("u1")).byok).toEqual({ kind: "anthropic", hasApiKey: true });
|
|
expect((await r.getUser("u2")).byok).toBeNull();
|
|
const raw = await db.sql`SELECT ciphertext FROM user_api_keys WHERE user_id = 'u1'`;
|
|
expect(raw[0].ciphertext).not.toContain("SECRET");
|
|
expect(await r.userApiKey("u1")).toBe("sk-user-SECRET");
|
|
expect(await r.userApiKey("u2")).toBeNull();
|
|
// replacing with another provider drops the old key
|
|
await r.saveUserKey("u1", { kind: "openai", baseUrl: "https://api.example.com/v1", model: "m", apiKey: "k2" });
|
|
expect(await r.userApiKey("u1")).toBe("k2");
|
|
expect((await db.sql`SELECT COUNT(*) AS n FROM user_api_keys WHERE user_id = 'u1'`)[0].n.toString()).toBe("1");
|
|
await r.removeUserKey("u1");
|
|
expect((await r.getUser("u1")).byok).toBeNull();
|
|
expect(await r.userApiKey("u1")).toBeNull();
|
|
});
|
|
|
|
forEachDb("mentor mode is on by default and toggles per course", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
expect(await r.isMentorOff("u1", "sample")).toBe(false);
|
|
await r.setMentorOff("u1", "sample", true);
|
|
expect(await r.isMentorOff("u1", "sample")).toBe(true);
|
|
expect(await r.isMentorOff("u1", "pure-snake")).toBe(false);
|
|
expect(await r.isMentorOff("u2", "sample")).toBe(false);
|
|
await r.saveUserKey("u1", { kind: "anthropic", apiKey: "k" });
|
|
expect(await r.isMentorOff("u1", "sample")).toBe(true);
|
|
await r.setMentorOff("u1", "sample", false);
|
|
expect(await r.isMentorOff("u1", "sample")).toBe(false);
|
|
expect((await r.getUser("u1")).byok?.kind).toBe("anthropic");
|
|
});
|
|
|
|
forEachDb("mentor toggles are rows in ai_mentor_prefs, not config_json", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
await r.saveUserKey("u1", { kind: "anthropic", apiKey: "k" });
|
|
await r.setMentorOff("u1", "sample", true);
|
|
await r.setMentorOff("u1", "sample", true); // twice is fine
|
|
const rows = await db.sql`SELECT user_id, course_id, off FROM ai_mentor_prefs`;
|
|
expect(rows.map((x: { user_id: string; course_id: string; off: number }) => [x.user_id, x.course_id, Number(x.off)])).toEqual([["u1", "sample", 1]]);
|
|
const cfg = (await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'user' AND owner_id = 'u1'`)[0].config_json;
|
|
expect(cfg).not.toContain("mentorOff");
|
|
expect((await r.getUser("u1")).mentorOff).toEqual({ sample: true });
|
|
await r.setMentorOff("u1", "sample", false);
|
|
expect(await db.sql`SELECT user_id FROM ai_mentor_prefs`).toHaveLength(0);
|
|
});
|
|
|
|
forEachDb("MCP env and headers are encrypted, masked when read and kept when the mask is sent back", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
await r.saveServer({ provider: null, mcpServers: [
|
|
{ id: "fs", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "tok-SECRET-123456", SHORT: "abc" } },
|
|
{ id: "web", name: "Web", transport: "http", url: "https://mcp.example.com/mcp", headers: { authorization: "Bearer hdr-SECRET-abcdef" } },
|
|
] });
|
|
const raw = (await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`)[0].config_json;
|
|
expect(raw).not.toContain("SECRET");
|
|
expect(raw).not.toContain("abc\"");
|
|
|
|
const shown = await r.getServer();
|
|
expect(JSON.stringify(shown)).not.toContain("SECRET");
|
|
expect(shown.mcpServers[0]!.env).toEqual({ FS_TOKEN: "••••3456", SHORT: "••••••••" });
|
|
expect(shown.mcpServers[1]!.headers).toEqual({ authorization: "••••cdef" });
|
|
expect(maskSecret("abc")).toBe("••••••••");
|
|
|
|
// The admin page sends the masks back unchanged (plus a new variable): stored values are kept.
|
|
await r.saveServer({ provider: null, mcpServers: [
|
|
{ ...shown.mcpServers[0]!, env: { ...shown.mcpServers[0]!.env, NEW: "plain-new-value" } },
|
|
shown.mcpServers[1]!,
|
|
] });
|
|
const full = await r.mcpServersWithSecrets();
|
|
expect(full[0]!.env).toEqual({ FS_TOKEN: "tok-SECRET-123456", SHORT: "abc", NEW: "plain-new-value" });
|
|
expect(full[1]!.headers).toEqual({ authorization: "Bearer hdr-SECRET-abcdef" });
|
|
|
|
// A new value replaces the old one; a removed key is gone.
|
|
await r.saveServer({ provider: null, mcpServers: [{ id: "fs", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "tok-ROTATED-999999" } }] });
|
|
expect((await r.mcpServersWithSecrets())).toEqual([{ id: "fs", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "tok-ROTATED-999999" } }]);
|
|
|
|
// A mask that does not match the stored value (other server id, or a stale mask) is refused, never stored.
|
|
await expect(r.saveServer({ provider: null, mcpServers: [{ id: "fs2", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "••••9999" } }] })).rejects.toThrow(MaskedSecretError);
|
|
await expect(r.saveServer({ provider: null, mcpServers: [{ id: "fs", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "••••0000" } }] })).rejects.toThrow(/Re-enter/);
|
|
expect((await r.mcpServersWithSecrets())[0]!.env).toEqual({ FS_TOKEN: "tok-ROTATED-999999" });
|
|
|
|
// resolveMcpMasks: the admin "test" button sends a masked draft; the server fills in the stored values.
|
|
expect(await r.resolveMcpMasks({ id: "fs", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "••••9999" } }))
|
|
.toEqual({ id: "fs", name: "Files", transport: "stdio", command: "mcp-fs", env: { FS_TOKEN: "tok-ROTATED-999999" } });
|
|
});
|
|
|
|
forEachDb("unreadable MCP secrets are reported, kept on save, and recover when the old STC_SECRET returns", async (db) => {
|
|
const old = aiSettingsRepo(db, createSecretBox("old-secret"));
|
|
await old.saveServer({ provider: null, mcpServers: [
|
|
{ id: "fs", name: "Files", transport: "stdio", command: "x", env: { T: "tok-SECRET-123456" } },
|
|
{ id: "web", name: "Web", transport: "http", url: "https://mcp.example.com/mcp", headers: { authorization: "Bearer hdr-SECRET-abcdef" } },
|
|
] });
|
|
const r = aiSettingsRepo(db, box); // STC_SECRET changed
|
|
const shown = await r.getServer();
|
|
expect(shown.mcpServers).toEqual([
|
|
{ id: "fs", name: "Files", transport: "stdio", command: "x" },
|
|
{ id: "web", name: "Web", transport: "http", url: "https://mcp.example.com/mcp" },
|
|
]);
|
|
expect(shown.unreadableSecrets).toEqual([
|
|
{ kind: "mcp", serverId: "fs", name: "Files", field: "env" },
|
|
{ kind: "mcp", serverId: "web", name: "Web", field: "headers" },
|
|
]);
|
|
expect(await r.mcpServersWithSecrets()).toEqual(shown.mcpServers);
|
|
|
|
// The admin saves the page for another reason: the unreadable ciphers survive.
|
|
const before = JSON.parse((await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`)[0].config_json);
|
|
await r.saveServer({ provider: { kind: "openai", baseUrl: "http://ollama:11434/v1", model: "llama3" }, mcpServers: shown.mcpServers });
|
|
const after = JSON.parse((await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`)[0].config_json);
|
|
expect(after.mcpServers[0].envCipher).toBe(before.mcpServers[0].envCipher);
|
|
expect(after.mcpServers[1].headersCipher).toBe(before.mcpServers[1].headersCipher);
|
|
expect((await r.getServer()).unreadableSecrets).toHaveLength(2);
|
|
|
|
// Restoring the previous secret brings them back.
|
|
expect((await aiSettingsRepo(db, createSecretBox("old-secret")).mcpServersWithSecrets()).map((m) => m.env ?? m.headers)).toEqual([
|
|
{ T: "tok-SECRET-123456" }, { authorization: "Bearer hdr-SECRET-abcdef" },
|
|
]);
|
|
|
|
// Explicit replace (new values) and explicit clear (clearSecrets) are the only ways to drop them.
|
|
await r.saveServer({ provider: null, mcpServers: [
|
|
{ id: "fs", name: "Files", transport: "stdio", command: "x", env: { T: "tok-NEW-0000000000" } },
|
|
{ id: "web", name: "Web", transport: "http", url: "https://mcp.example.com/mcp", clearSecrets: ["headers"] },
|
|
] });
|
|
expect(await r.mcpServersWithSecrets()).toEqual([
|
|
{ id: "fs", name: "Files", transport: "stdio", command: "x", env: { T: "tok-NEW-0000000000" } },
|
|
{ id: "web", name: "Web", transport: "http", url: "https://mcp.example.com/mcp" },
|
|
]);
|
|
expect((await r.getServer()).unreadableSecrets).toEqual([]);
|
|
expect((await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`)[0].config_json).not.toContain("clearSecrets");
|
|
});
|
|
|
|
forEachDb("an MCP cipher swapped onto another server or field does not open", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
await r.saveServer({ provider: null, mcpServers: [
|
|
{ id: "a", name: "A", transport: "http", url: "https://a.example.com/mcp", headers: { authorization: "Bearer A-SECRET-111111" } },
|
|
{ id: "b", name: "B", transport: "http", url: "https://b.example.com/mcp", headers: { authorization: "Bearer B-SECRET-222222" } },
|
|
{ id: "c", name: "C", transport: "stdio", command: "x", env: { T: "C-SECRET-33333333" } },
|
|
] });
|
|
const row = JSON.parse((await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`)[0].config_json);
|
|
// swap a <-> b, and move c's env cipher into a headers slot of c
|
|
[row.mcpServers[0].headersCipher, row.mcpServers[1].headersCipher] = [row.mcpServers[1].headersCipher, row.mcpServers[0].headersCipher];
|
|
row.mcpServers[2].headersCipher = row.mcpServers[2].envCipher;
|
|
delete row.mcpServers[2].envCipher;
|
|
await db.sql`UPDATE ai_settings SET config_json = ${JSON.stringify(row)} WHERE scope = 'server'`;
|
|
const full = await r.mcpServersWithSecrets();
|
|
expect(full.every((m) => !m.headers && !m.env)).toBe(true);
|
|
expect((await r.getServer()).unreadableSecrets.map((u) => u.kind === "mcp" && `${u.serverId}.${u.field}`)).toEqual(["a.headers", "b.headers", "c.headers"]);
|
|
});
|
|
|
|
forEachDb("a BYOK ciphertext copied to another user, or into an MCP slot, does not open", async (db) => {
|
|
const r = aiSettingsRepo(db, box);
|
|
await r.saveUserKey("u1", { kind: "anthropic", apiKey: "sk-u1-SECRET" });
|
|
await r.saveUserKey("u2", { kind: "anthropic", apiKey: "sk-u2-other" });
|
|
const u1 = (await db.sql`SELECT ciphertext FROM user_api_keys WHERE user_id = 'u1'`)[0].ciphertext as string;
|
|
await db.sql`UPDATE user_api_keys SET ciphertext = ${u1} WHERE user_id = 'u2'`;
|
|
expect(await r.userApiKey("u2")).toBeNull();
|
|
expect((await r.getUser("u2")).byok).toEqual({ kind: "anthropic", hasApiKey: true, keyUnreadable: true });
|
|
expect(await r.userApiKey("u1")).toBe("sk-u1-SECRET");
|
|
expect((await r.getUser("u1")).byok).toEqual({ kind: "anthropic", hasApiKey: true });
|
|
// into the server API key and an MCP field: no oracle
|
|
await r.saveServer({ provider: { kind: "anthropic", apiKey: "sk-server" }, mcpServers: [{ id: "m", name: "M", transport: "http", url: "https://m.example.com/mcp", headers: { x: "y-123456789012" } }] });
|
|
const row = JSON.parse((await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'server'`)[0].config_json);
|
|
row.apiKeyCipher = u1;
|
|
row.mcpServers[0].headersCipher = u1;
|
|
await db.sql`UPDATE ai_settings SET config_json = ${JSON.stringify(row)} WHERE scope = 'server'`;
|
|
expect(await r.serverApiKey()).toBeNull();
|
|
expect((await r.mcpServersWithSecrets())[0]!.headers).toBeUndefined();
|
|
expect((await r.getServer()).unreadableSecrets).toEqual([{ kind: "api-key" }, { kind: "mcp", serverId: "m", name: "M", field: "headers" }]);
|
|
});
|
|
|
|
forEachDb("an unreadable server API key is reported and kept on save", async (db) => {
|
|
await aiSettingsRepo(db, createSecretBox("old-secret")).saveServer({ provider: { kind: "anthropic", apiKey: "sk-ant-OLD" }, mcpServers: [] });
|
|
const r = aiSettingsRepo(db, box);
|
|
expect((await r.getServer()).unreadableSecrets).toEqual([{ kind: "api-key" }]);
|
|
expect(await r.serverApiKey()).toBeNull();
|
|
await r.saveServer({ provider: { kind: "anthropic", model: "claude-opus-5-5" }, mcpServers: [] });
|
|
expect(await aiSettingsRepo(db, createSecretBox("old-secret")).serverApiKey()).toBe("sk-ant-OLD");
|
|
});
|