diff --git a/packages/feature-ai/src/feature.ts b/packages/feature-ai/src/feature.ts index 3582386..2803f81 100644 --- a/packages/feature-ai/src/feature.ts +++ b/packages/feature-ai/src/feature.ts @@ -14,6 +14,7 @@ import { connectMcp, mcpServersFor } from "./mcp/client"; import { createProvider } from "./providers"; import { PRESET_BASE_URLS, byokBaseUrl, noRedirectFetch, parseAllowedOrigins, publicProviderError, sanitizeProviderErrors } from "./endpoints"; import { getCourseIndex, noteChunks } from "./retrieval/course-index"; +import { MaskedSecretError } from "./mcp/secrets"; import { aiSettingsRepo, type McpServerConfig, type ProviderConfig } from "./settings"; export interface AiFeatureOptions { @@ -267,7 +268,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat let tools: ToolSet | null = null; try { const course_ = courseTools({ repo: ctx.courses, course, index, listNotes: (lid) => notes.list(user.id, course.id, lid), getProgress: () => progress.get(user.id, course.id) }); - const servers = mcpServersFor(course.id, (await settings.getServer()).mcpServers); + const servers = mcpServersFor(course.id, await settings.mcpServersWithSecrets()); const mcp = servers.length ? await mcpTools(servers, { fetch: opts.fetch, timeoutMs: opts.mcpTimeoutMs }) : null; for (const w of mcp?.warnings ?? []) send({ type: "notice", message: w }); tools = combineTools(mcp ? [course_, mcp] : [course_]); @@ -344,7 +345,12 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat allowedBaseUrls = parseAllowedOrigins(list); if (allowedBaseUrls.length !== new Set(list.map((x) => x.replace(/\/+$/, ""))).size) return err(400, "Each allowed base URL must be an origin such as http://ollama.internal:11434."); } - await settings.saveServer({ provider, mcpServers, allowedBaseUrls }); + try { + await settings.saveServer({ provider, mcpServers, allowedBaseUrls }); + } catch (e) { + if (e instanceof MaskedSecretError) return err(400, e.message); + throw e; + } return json(await settings.getServer()); }, }, @@ -355,8 +361,13 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const b = await body(req); const parsed = parseMcp(b ? [b] : null); if (parsed === "invalid" || !parsed.length) return err(400, "Invalid MCP server."); + let cfg: McpServerConfig; + try { cfg = await settings.resolveMcpMasks(parsed[0]!); } catch (e) { + if (e instanceof MaskedSecretError) return err(400, e.message); + throw e; + } try { - const c = await connectMcp(parsed[0]!, { fetch: opts.fetch, timeoutMs: opts.mcpTimeoutMs ?? 15_000 }); + const c = await connectMcp(cfg, { fetch: opts.fetch, timeoutMs: opts.mcpTimeoutMs ?? 15_000 }); try { return json({ ok: true, tools: (await c.tools()).map((t) => ({ name: t.name, description: t.description })) }); } finally { await c.close(); } } catch (e) { return json({ ok: false, error: (e as Error).message }); diff --git a/packages/feature-ai/src/mcp/secrets.ts b/packages/feature-ai/src/mcp/secrets.ts new file mode 100644 index 0000000..210dc1f --- /dev/null +++ b/packages/feature-ai/src/mcp/secrets.ts @@ -0,0 +1,87 @@ +import type { SecretBox } from "../crypto"; +import type { McpServerConfig } from "../settings"; + +/** + * MCP `env` and `headers` at rest: both move into `secretCipher`, sealed with the same AES-GCM/HKDF box as API + * keys. The admin API only ever sees masked values; server code opens them to start or call an MCP server. + */ +export type StoredMcpServer = Omit & { secretCipher?: string; env?: Record; headers?: Record }; +type Secrets = { env?: Record; headers?: Record }; + +export const MASK_PREFIX = "••••"; + +/** `••••` plus the last 4 characters for values of 12 characters or more, otherwise `••••••••`. */ +export function maskSecret(value: string): string { + return value.length >= 12 ? `${MASK_PREFIX}${value.slice(-4)}` : `${MASK_PREFIX}${MASK_PREFIX}`; +} + +/** A posted value looks like a mask but matches no stored value for that server and key. */ +export class MaskedSecretError extends Error { + constructor(serverId: string, key: string) { + super(`Re-enter the value of ${key} for MCP server "${serverId}": a masked value cannot be saved.`); + this.name = "MaskedSecretError"; + } +} + +const nonEmpty = (r: Record | undefined) => (r && Object.keys(r).length ? r : undefined); + +/** Strips `env`/`headers` from a config and seals them (if any) into `secretCipher`. */ +export async function sealMcpServer(box: SecretBox, cfg: McpServerConfig | StoredMcpServer): Promise { + const { env, headers, secretCipher: _old, ...rest } = cfg as StoredMcpServer; + const secrets: Secrets = {}; + if (nonEmpty(env)) secrets.env = env; + if (nonEmpty(headers)) secrets.headers = headers; + return secrets.env || secrets.headers ? { ...rest, secretCipher: await box.encrypt(JSON.stringify(secrets)) } : rest; +} + +/** + * The full config for server code. A value that cannot be opened (STC_SECRET changed) is dropped with the server + * still listed, so a broken secret never crashes the assistant. Plaintext left over from v1 is passed through. + */ +export async function openMcpServer(box: SecretBox, s: StoredMcpServer): Promise { + const { secretCipher, env, headers, ...rest } = s; + let secrets: Secrets = {}; + if (secretCipher) { + try { secrets = JSON.parse(await box.decrypt(secretCipher)) as Secrets; } catch { secrets = {}; } + } + const out: McpServerConfig = { ...rest }; + const e = nonEmpty(secrets.env) ?? nonEmpty(env); + const h = nonEmpty(secrets.headers) ?? nonEmpty(headers); + if (e) out.env = e; + if (h) out.headers = h; + return out; +} + +const maskRecord = (r: Record | undefined) => (r ? Object.fromEntries(Object.entries(r).map(([k, v]) => [k, maskSecret(v)])) : undefined); + +/** What the admin API returns: keys visible, values masked. */ +export function maskMcpServer(s: McpServerConfig): McpServerConfig { + const out: McpServerConfig = { ...s }; + if (s.env) out.env = maskRecord(s.env); + if (s.headers) out.headers = maskRecord(s.headers); + return out; +} + +/** + * Replaces each posted value equal to the mask of the stored value (same server id, same key) with the stored + * value. Any other value starting with the mask prefix throws MaskedSecretError, so a mask is never saved or sent. + */ +export function unmaskMcpServer(posted: McpServerConfig, previous: McpServerConfig[]): McpServerConfig { + const prev = previous.find((p) => p.id === posted.id); + const fill = (field: "env" | "headers") => { + const r = posted[field]; + if (!r) return undefined; + return Object.fromEntries(Object.entries(r).map(([k, v]) => { + if (!v.startsWith(MASK_PREFIX)) return [k, v]; + const old = prev?.[field]?.[k]; + if (old !== undefined && maskSecret(old) === v) return [k, old]; + throw new MaskedSecretError(posted.id, k); + })); + }; + const out: McpServerConfig = { ...posted }; + const env = fill("env"); + const headers = fill("headers"); + if (env) out.env = env; + if (headers) out.headers = headers; + return out; +} diff --git a/packages/feature-ai/src/settings.ts b/packages/feature-ai/src/settings.ts index b0dfa04..c571df4 100644 --- a/packages/feature-ai/src/settings.ts +++ b/packages/feature-ai/src/settings.ts @@ -1,7 +1,10 @@ import type { Db } from "@stc/db"; import type { SecretBox } from "./crypto"; +import { maskMcpServer, openMcpServer, sealMcpServer, unmaskMcpServer, type StoredMcpServer } from "./mcp/secrets"; import { upgradeAiData } from "./upgrade"; +export { maskSecret, MaskedSecretError } from "./mcp/secrets"; + export type ProviderKind = "anthropic" | "openai"; export type Effort = "low" | "medium" | "high" | "xhigh" | "max"; export interface ProviderConfig { kind: ProviderKind; model?: string; baseUrl?: string; effort?: Effort } @@ -20,7 +23,10 @@ export interface McpServerConfig { courseIds?: string[]; } -/** `allowedBaseUrls`: origins learners' own keys (BYOK) may be sent to, besides the official endpoints. */ +/** + * `allowedBaseUrls`: origins learners' own keys (BYOK) may be sent to, besides the official endpoints. + * `mcpServers` here carry masked `env`/`headers` values (see mcp/secrets.ts); full values: mcpServersWithSecrets(). + */ export interface ServerSettings { provider: PublicProviderConfig | null; mcpServers: McpServerConfig[]; allowedBaseUrls: string[] } export interface ServerSettingsInput { provider: (ProviderConfig & { apiKey?: string; clearApiKey?: boolean }) | null; @@ -30,7 +36,7 @@ export interface ServerSettingsInput { } export interface UserSettings { byok: PublicProviderConfig | null; mentorOff: Record } -type ServerRow = { provider: ProviderConfig | null; apiKeyCipher: string | null; mcpServers: McpServerConfig[]; allowedBaseUrls?: string[] }; +type ServerRow = { provider: ProviderConfig | null; apiKeyCipher: string | null; mcpServers: StoredMcpServer[]; allowedBaseUrls?: string[] }; type UserRow = { byok: ProviderConfig | null }; const pickProvider = (p: ProviderConfig): ProviderConfig => { @@ -48,10 +54,11 @@ const parse = (s: string | undefined, fallback: T): T => { /** * AI settings in ai_settings and user_api_keys (core 0001_init.sql) and ai_mentor_prefs (this package's 0002). - * - ('server', '') holds the admin provider (key encrypted) and MCP servers. + * - ('server', '') holds the admin provider (key encrypted) and MCP servers (env and headers encrypted). * - ('user', ) holds the BYOK provider choice. The mentor_off column ("off for every course") stays 0. * - ai_mentor_prefs has a row per (user, course) whose mentor rules the learner switched off. - * Keys never leave this module except through serverApiKey/userApiKey, which only server code calls. + * Keys and MCP secrets leave this module only through serverApiKey, userApiKey and mcpServersWithSecrets, which + * only server code calls. * Every method first awaits the one-time v1 data upgrade (upgradeAiData); a failed attempt is retried next call. */ export function aiSettingsRepo(db: Db, box: SecretBox) { @@ -77,6 +84,7 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { const { mentorOff: _legacy, ...row } = parse(await read("user", userId), { byok: null } as UserRow & { mentorOff?: unknown }); return row; }; + const openServers = async (r: ServerRow) => Promise.all((r.mcpServers ?? []).map((m) => openMcpServer(box, m))); const keyRow = async (userId: string) => (await db.sql`SELECT provider, ciphertext FROM user_api_keys WHERE user_id = ${userId}`)[0] as { provider: string; ciphertext: string } | undefined; @@ -86,7 +94,18 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { async getServer(): Promise { await ready(); const r = await serverRow(); - return { provider: r.provider ? { ...r.provider, hasApiKey: !!r.apiKeyCipher } : null, mcpServers: r.mcpServers ?? [], allowedBaseUrls: r.allowedBaseUrls ?? [] }; + const mcpServers = (await openServers(r)).map(maskMcpServer); + return { provider: r.provider ? { ...r.provider, hasApiKey: !!r.apiKeyCipher } : null, mcpServers, allowedBaseUrls: r.allowedBaseUrls ?? [] }; + }, + /** Server code only: MCP servers with their real env and headers. */ + async mcpServersWithSecrets(): Promise { + await ready(); + return openServers(await serverRow()); + }, + /** A posted (masked) MCP config with stored values filled in; throws MaskedSecretError for a stray mask. */ + async resolveMcpMasks(cfg: McpServerConfig): Promise { + await ready(); + return unmaskMcpServer(cfg, await openServers(await serverRow())); }, async saveServer(input: ServerSettingsInput): Promise { await ready(); @@ -96,10 +115,13 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { if (input.provider.apiKey) apiKeyCipher = await box.encrypt(input.provider.apiKey); else if (!input.provider.clearApiKey) apiKeyCipher = prev.apiKeyCipher; } + const previous = await openServers(prev); + const unmasked = (input.mcpServers ?? []).map((m) => unmaskMcpServer(m, previous)); + const mcpServers = await Promise.all(unmasked.map((m) => sealMcpServer(box, m))); await write("server", "", { provider: input.provider ? pickProvider(input.provider) : null, apiKeyCipher, - mcpServers: input.mcpServers ?? [], + mcpServers, allowedBaseUrls: input.allowedBaseUrls ?? prev.allowedBaseUrls ?? [], }); }, diff --git a/packages/feature-ai/src/upgrade.ts b/packages/feature-ai/src/upgrade.ts index 26767c2..1a33600 100644 --- a/packages/feature-ai/src/upgrade.ts +++ b/packages/feature-ai/src/upgrade.ts @@ -1,5 +1,6 @@ import type { Db } from "@stc/db"; import type { SecretBox } from "./crypto"; +import { sealMcpServer, type StoredMcpServer } from "./mcp/secrets"; /** Recorded in schema_migrations once the data upgrade has run (no `.sql` suffix, so no file can share it). */ export const AI_DATA_UPGRADE = "ai_data_v11"; @@ -30,15 +31,27 @@ async function moveMentorToggles(tx: Tx, now: string) { } } +/** v1 stored MCP `env`/`headers` in plaintext in the server row; seal them like API keys. */ +async function sealMcpSecrets(tx: Tx, box: SecretBox, now: string) { + const rows = (await tx`SELECT config_json FROM ai_settings WHERE scope = 'server' AND owner_id = ''`) as { config_json: string }[]; + const cfg = rows[0] ? parseObject(rows[0].config_json) : null; + if (!cfg || !Array.isArray(cfg.mcpServers)) return; + const servers = cfg.mcpServers as StoredMcpServer[]; + if (!servers.some((s) => s && (s.env || s.headers))) return; + const mcpServers = await Promise.all(servers.map((s) => (s && (s.env || s.headers) ? sealMcpServer(box, s) : s))); + await tx`UPDATE ai_settings SET config_json = ${JSON.stringify({ ...cfg, mcpServers })}, updated_at = ${now} WHERE scope = 'server' AND owner_id = ''`; +} + /** * Data the v1.1 SQL migrations cannot move portably. Runs after the package's migrations, in one transaction, * once (marked in schema_migrations). Returns true if it ran now. The settings repo awaits it before first use. */ -export async function upgradeAiData(db: Db, _box: SecretBox): Promise { +export async function upgradeAiData(db: Db, box: SecretBox): Promise { return db.sql.begin(async (tx) => { if ((await tx`SELECT name FROM schema_migrations WHERE name = ${AI_DATA_UPGRADE}`).length) return false; const now = new Date().toISOString(); await moveMentorToggles(tx, now); + await sealMcpSecrets(tx, box, now); await tx`INSERT INTO schema_migrations (name, applied_at) VALUES (${AI_DATA_UPGRADE}, ${now})`; return true; }); diff --git a/packages/feature-ai/src/web/AdminAiPage.tsx b/packages/feature-ai/src/web/AdminAiPage.tsx index 21c4ad5..22b60a4 100644 --- a/packages/feature-ai/src/web/AdminAiPage.tsx +++ b/packages/feature-ai/src/web/AdminAiPage.tsx @@ -157,7 +157,7 @@ export default function AdminAiPage() { )} {s.transport === "stdio" ? ( - +