From fbfe16534acdd17ecffb73d58b0a6d0411ac6d74 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 20:26:09 +0100 Subject: [PATCH 1/7] docs: v1.1 work package A plan (AI contract requests 3-5, MCP secrets) Co-Authored-By: Claude Opus 5.5 --- docs/superpowers/plans/2026-10-30-v11-ai.md | 94 +++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-30-v11-ai.md diff --git a/docs/superpowers/plans/2026-10-30-v11-ai.md b/docs/superpowers/plans/2026-10-30-v11-ai.md new file mode 100644 index 0000000..845f6d4 --- /dev/null +++ b/docs/superpowers/plans/2026-10-30-v11-ai.md @@ -0,0 +1,94 @@ +# STCourses v1.1 — Work package A: deferred AI contract requests 3–5 + +> **For agentic workers:** executed inline with superpowers:executing-plans and TDD. Steps use checkbox (`- [ ]`) syntax. One commit per task. + +**Goal:** settle AI contract requests 3, 4 and 5 (`docs/contract-requests/ai.md`) and the v1 deferred minor "MCP `env`/`headers` in plaintext": +- (3) structured tool calls on `ChatMessage`, with the Anthropic and OpenAI providers mapping them natively; +- (4) per-course mentor toggles in their own table, migrated from `ai_settings.config_json`; +- (5) a citations column on `ai_messages`, migrated from the v1 `sources` rows; +- MCP `env` and `headers` encrypted at rest (same AES-GCM/HKDF box as BYOK keys) and only ever returned masked. + +**Spec:** section 9.2. **Parent plan:** `docs/superpowers/plans/2026-10-30-v1-1.md` (Global Constraints, package A). + +## Global Constraints + +- Everything in `CLAUDE.md` "Project rules" and the v1.1 Global Constraints applies. +- Every DB change is a new portable migration in `packages/feature-ai/migrations/` that runs unchanged on SQLite and Postgres, and upgrades a database already on the v1 schema. File names carry the `ai` prefix (core `migrate()` requires unique names across packages). +- An upgrade test seeds v1 rows (core `0001_init.sql` only), runs the AI migrations and the data upgrade, and asserts the result, on SQLite and on Postgres (`STC_TEST_PG_URL`). +- v1 safeguards that must keep holding (each already has tests; they stay green and unchanged in meaning): + - BYOK keys only reach the official endpoints or allowlisted origins, without redirects; + - error bodies from BYOK calls are never echoed to the user; + - stdio MCP processes get PATH, HOME, LANG plus exactly the configured variables; + - the solution guard works on canonical paths; + - the banner text is exactly "Mentor mode off: I'll give full answers"; + - tests use fakes only, no network. +- Gates per task: `bun test`, `bun run typecheck`, and the Postgres run with `STC_TEST_PG_URL=postgres://postgres:stc@127.0.0.1:55433/postgres`. +- Manual check: dev server on port 3493 with `STC_DATA_DIR=/tmp/stc-v11-ai`. Never port 3480 or `/home/user/STCourses/data`. + +## Design decisions + +1. **Schema vs data migrations.** Pure-SQL migrations cover what portable SQL can do: the new table, the new column, and folding v1 `sources` rows into `citations_json` (a correlated subquery that runs on both engines). Two upgrades cannot be portable SQL: reading `config_json.mentorOff` needs engine-specific JSON functions, and encrypting MCP secrets needs `STC_SECRET`. These run in code, in `upgradeAiData(db, box)`: one transaction, idempotent, recorded in `schema_migrations` under the name `ai_data_v11` (no `.sql` suffix, so it cannot clash with a file). The settings repo awaits it once (memoised; retried after a failure) before its first read or write, and `aiFeature` starts it at construction so plaintext does not linger until the first request. +2. **Citations column name.** `ai_messages.citations_json TEXT NULL` (the parent plan asks for "a citations column"; a typed name beats a generic `meta_json`). v1 `sources` rows are folded into the assistant row immediately before them (by id order, the order v1 reads in) and then deleted. +3. **Mentor table.** `ai_mentor_prefs (user_id, course_id, off INTEGER, updated_at, PK (user_id, course_id))`. A row exists only while the mentor is off (turning it back on deletes the row). `ai_settings.mentor_off` stays reserved for "off everywhere" and is not read. The upgrade copies every `mentorOff[courseId] === true` and removes the `mentorOff` key from the user's `config_json`. +4. **Tool calls on `ChatMessage`.** Contract change in `@stc/contracts`: + - `ToolCall = { id, name, input }`; + - `ChatMessage.toolCalls?: ToolCall[]` (assistant turn) and `ChatMessage.providerData?: unknown` (opaque, round-tripped by the provider that produced it); + - `AiProvider.chat` may also yield `{ providerData }` once per turn. + The assistant loop appends one assistant message per model turn (`content` = the turn's text, `toolCalls`, `providerData`), then one `tool` message per call (`toolCallId`, `name`). The v1 one-message-per-call encoding (`assistant` + `toolCallId`) is removed. + - Anthropic: `providerData = { anthropic: }` (thinking with signature, text, tool_use). An assistant message carrying it is replayed verbatim; otherwise it maps to text + `tool_use` blocks. The in-memory `rawByToolId` map goes away, so a provider instance holds no conversation state. + - OpenAI: assistant `toolCalls` map to `tool_calls` (`function.arguments` = JSON of input) with `content` = text or null. No `providerData`. +5. **MCP secrets at rest.** In `config_json` each MCP server keeps its non-secret fields; `env` and `headers` move into `secretCipher` = `box.encrypt(JSON.stringify({ env, headers }))`. The admin GET returns `env` and `headers` with the keys and masked values: `••••` plus the last 4 characters when the value has at least 12 characters, otherwise `••••••••`. On save, a posted value equal to the mask of the stored value for the same server id and key keeps the stored value; any other value starting with `••••` is refused with 400 ("re-enter the value"), so a mask is never stored as a secret. The MCP test endpoint resolves masks the same way. Only server code gets full values (`mcpServersWithSecrets()`). If the secret cannot be decrypted (changed `STC_SECRET`), the server is listed without `env`/`headers` and the client starts it without them. + +## Tasks + +### Task 1: AI migrations folder, test helper and citations column + +**Files:** +- Create: `packages/feature-ai/migrations/0001_ai_message_citations.sql` +- Modify: `packages/feature-ai/src/index.ts` (`migrationsDirs`), `packages/feature-ai/scripts/dev.ts` (migrate with the AI folder), `packages/feature-ai/src/feature.ts` (write/read `citations_json`, no more `sources` rows) +- Test: `packages/feature-ai/test/upgrade.test.ts` (new), `test/helpers.ts` (`forEachAiDb`), `test/feature.test.ts` + +- [ ] Write `upgrade.test.ts`: on a v1 database (core migrations only) seed a conversation with user, assistant, sources, user, assistant (no sources), and an orphan sources row; run `migrate(db, { "feature-ai": migrationsDirs })`; assert the first assistant row has the citations JSON, the second has NULL, no `sources` rows remain, and a second migrate applies nothing. Add a feature test that a stored assistant message has `citations_json` set and no `sources` row is written. +- [ ] Run: fails (no migrations folder). +- [ ] Implement the SQL (`ALTER TABLE ai_messages ADD COLUMN citations_json TEXT`; correlated `UPDATE` picking the next message by id when it is a `sources` row; `DELETE ... WHERE role = 'sources'`), export the folder, switch `feature.ts` to the column, switch AI tests that touch the DB to `forEachAiDb`. +- [ ] Run tests on SQLite and Postgres; typecheck; commit `feat(ai): citations column on ai_messages (contract request 5)`. + +### Task 2: Mentor preferences table and the data upgrade runner + +**Files:** +- Create: `packages/feature-ai/migrations/0002_ai_mentor_prefs.sql`, `packages/feature-ai/src/upgrade.ts` +- Modify: `packages/feature-ai/src/settings.ts`, `src/feature.ts`, `src/index.ts` +- Test: `test/upgrade.test.ts`, `test/settings.test.ts` + +- [ ] Tests: v1 user rows with `mentorOff: { sample: true, other: false }` plus a BYOK provider become one `ai_mentor_prefs` row (`u1`, `sample`) and a `config_json` without `mentorOff` but with `byok` intact; `isMentorOff` reads the table; running the upgrade twice changes nothing; a row with invalid JSON is left alone. Settings test: toggling writes and deletes table rows and never touches `config_json.mentorOff`. +- [ ] Run: fails. +- [ ] Implement table SQL, `upgradeAiData(db, box)` (mentor part plus the `ai_data_v11` marker), repo `ready()` memo, mentor methods on the table, `getUser().mentorOff` from the table. +- [ ] Gates; commit `feat(ai): per-course mentor preferences table (contract request 4)`. + +### Task 3: Encrypt MCP env and headers at rest, masked in the UI + +**Files:** +- Modify: `src/settings.ts`, `src/upgrade.ts`, `src/feature.ts` (admin PUT 400 on stale masks, chat uses `mcpServersWithSecrets`, MCP test route resolves masks), `src/web/AdminAiPage.tsx` (one-line hint) +- Test: `test/upgrade.test.ts`, `test/settings.test.ts`, `test/feature.test.ts` + +- [ ] Tests: v1 server row with plaintext `env`/`headers` is encrypted by the upgrade (raw `config_json` no longer contains the values; `mcpServersWithSecrets()` returns them); `getServer()` returns masked values only; saving the masked values back keeps the stored secrets; saving a new value replaces it; a stray `••••` value is refused; the admin config API never contains a secret value; the MCP test route with masked values reaches the server with the real header (fake fetch). +- [ ] Run: fails. +- [ ] Implement. +- [ ] Gates; commit `feat(ai): encrypt MCP env and headers at rest and mask them in the admin API`. + +### Task 4: Structured tool calls on ChatMessage + +**Files:** +- Modify: `packages/contracts/src/ai.ts`, `packages/contracts/test/types.test.ts`, `packages/feature-ai/src/assistant/loop.ts`, `src/providers/anthropic.ts`, `src/providers/openai.ts` +- Test: `test/anthropic.test.ts`, `test/openai.test.ts`, `test/assistant.test.ts`, `test/helpers.ts` + +- [ ] Tests: the loop appends one assistant message with `toolCalls` (two calls in one turn) and the provider's `providerData`, then a tool message per call; Anthropic replays `providerData` blocks verbatim (thinking signature intact) and maps a message without it to text + `tool_use` blocks; a fresh provider instance replays history it did not produce (no in-memory state); OpenAI maps `toolCalls` to one assistant message with `tool_calls` and text content. +- [ ] Run: fails. +- [ ] Implement contract fields, provider mapping, loop change; remove `rawByToolId` and the legacy encoding. +- [ ] Gates; commit `feat(ai): structured tool calls on ChatMessage (contract request 3)`. + +### Task 5: Docs and manual check + +- [ ] Mark requests 3–5 settled in `docs/contract-requests/ai.md`; update `settings.ts`/`index.ts` comments. +- [ ] Dev server on 3493 with `STC_DATA_DIR=/tmp/stc-v11-ai`: migrations apply on a fresh DB, admin config shows masked MCP values. +- [ ] Full gates; commit `docs(ai): contract requests 3-5 settled in v1.1`. From d34a5c1f0c7f83246570aa4dd31799181c898cef Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 20:28:23 +0100 Subject: [PATCH 2/7] feat(ai): citations column on ai_messages (contract request 5) New packages/feature-ai/migrations/0001_ai_message_citations.sql adds ai_messages.citations_json and folds v1 'sources' rows into the assistant row before them. The feature writes and reads the column directly. Co-Authored-By: Claude Opus 5.5 --- .../migrations/0001_ai_message_citations.sql | 10 +++++ packages/feature-ai/scripts/dev.ts | 3 +- packages/feature-ai/src/feature.ts | 32 +++++++-------- packages/feature-ai/src/index.ts | 3 +- packages/feature-ai/src/migrations.ts | 7 ++++ packages/feature-ai/test/feature.test.ts | 7 +++- packages/feature-ai/test/helpers.ts | 11 +++++ packages/feature-ai/test/settings.test.ts | 2 +- packages/feature-ai/test/upgrade.test.ts | 41 +++++++++++++++++++ 9 files changed, 94 insertions(+), 22 deletions(-) create mode 100644 packages/feature-ai/migrations/0001_ai_message_citations.sql create mode 100644 packages/feature-ai/src/migrations.ts create mode 100644 packages/feature-ai/test/upgrade.test.ts diff --git a/packages/feature-ai/migrations/0001_ai_message_citations.sql b/packages/feature-ai/migrations/0001_ai_message_citations.sql new file mode 100644 index 0000000..4e04b03 --- /dev/null +++ b/packages/feature-ai/migrations/0001_ai_message_citations.sql @@ -0,0 +1,10 @@ +-- v1.1 (contract request 5): an assistant message's citations live in its own column. +-- v1 stored them as an extra row with role 'sources' right after the assistant row (ids sort in write order). +-- Fold each such row into the assistant row before it, then drop every 'sources' row. +ALTER TABLE ai_messages ADD COLUMN citations_json TEXT; +UPDATE ai_messages SET citations_json = ( + SELECT s.content FROM ai_messages s + WHERE s.conversation_id = ai_messages.conversation_id AND s.role = 'sources' + AND s.id = (SELECT MIN(n.id) FROM ai_messages n WHERE n.conversation_id = ai_messages.conversation_id AND n.id > ai_messages.id) +) WHERE role = 'assistant'; +DELETE FROM ai_messages WHERE role = 'sources'; diff --git a/packages/feature-ai/scripts/dev.ts b/packages/feature-ai/scripts/dev.ts index a31fe02..55e0af2 100644 --- a/packages/feature-ai/scripts/dev.ts +++ b/packages/feature-ai/scripts/dev.ts @@ -7,6 +7,7 @@ import { createCourseRepository } from "@stc/course-format"; import { connect, migrate } from "@stc/db"; import indexHtml from "../../../apps/web/index.html"; import { aiFeature } from "../src/feature"; +import { migrationsDirs } from "../src/migrations"; const config = loadConfig(); const who = process.env.STC_AI_DEV_USER ?? "owner"; @@ -32,7 +33,7 @@ const fake: AiProvider = { }; const db = await connect(config.databaseUrl); -await migrate(db); +await migrate(db, { "feature-ai": migrationsDirs }); const courses = await createCourseRepository(config.courseDirs); const ctx = { config, db, courses, auth } as ServerContext; const feature = aiFeature(ctx, process.env.STC_AI_DEV_FAKE ? { providerFactory: () => fake } : {}); diff --git a/packages/feature-ai/src/feature.ts b/packages/feature-ai/src/feature.ts index 13b5180..e283fc1 100644 --- a/packages/feature-ai/src/feature.ts +++ b/packages/feature-ai/src/feature.ts @@ -198,22 +198,23 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const rows = await ctx.db.sql`SELECT id, user_id, course_id, title, created_at FROM ai_conversations WHERE id = ${id} AND user_id = ${userId}`; return rows[0] as { id: string; user_id: string; course_id: string; title: string | null; created_at: string } | undefined; }; - // Roles: "user", "assistant", and "sources" (JSON citations for the assistant message before it). - const addMessage = async (conversationId: string, role: "user" | "assistant" | "sources", content: string) => { - await ctx.db.sql`INSERT INTO ai_messages (id, conversation_id, role, content, created_at) VALUES (${Bun.randomUUIDv7()}, ${conversationId}, ${role}, ${content}, ${new Date().toISOString()})`; + /** Roles "user" and "assistant"; an assistant message's citations go in `citations_json` (contract request 5). */ + const addMessage = async (conversationId: string, role: "user" | "assistant", content: string, citations?: unknown[]) => { + const cites = citations ? JSON.stringify(citations) : null; + await ctx.db.sql`INSERT INTO ai_messages (id, conversation_id, role, content, citations_json, created_at) VALUES (${Bun.randomUUIDv7()}, ${conversationId}, ${role}, ${content}, ${cites}, ${new Date().toISOString()})`; }; - const messagesOf = async (conversationId: string) => - (await ctx.db.sql`SELECT id, role, content, created_at FROM ai_messages WHERE conversation_id = ${conversationId}`) - .map((r: { id: string; role: string; content: string; created_at: string }) => ({ id: r.id, role: r.role, content: r.content, createdAt: r.created_at })) - .sort((a: { id: string }, b: { id: string }) => (a.id < b.id ? -1 : 1)) as { id: string; role: "user" | "assistant" | "sources"; content: string; createdAt: string }[]; - /** Chat turns with each assistant message's stored citations attached. */ + /** Chat turns in write order (UUIDv7 ids), each assistant message with its stored citations. */ const turnsOf = async (conversationId: string) => { + const rows = (await ctx.db.sql`SELECT id, role, content, citations_json, created_at FROM ai_messages WHERE conversation_id = ${conversationId}`) as + { id: string; role: string; content: string; citations_json: string | null; created_at: string }[]; const out: { id: string; role: "user" | "assistant"; content: string; createdAt: string; citations?: unknown[] }[] = []; - for (const m of await messagesOf(conversationId)) { - if (m.role === "sources") { - const last = out[out.length - 1]; - if (last?.role === "assistant") { try { last.citations = JSON.parse(m.content); } catch {} } - } else out.push({ ...m, role: m.role }); + for (const r of rows.sort((a, b) => (a.id < b.id ? -1 : 1))) { + if (r.role !== "user" && r.role !== "assistant") continue; + const turn: (typeof out)[number] = { id: r.id, role: r.role, content: r.content, createdAt: r.created_at }; + if (r.role === "assistant" && r.citations_json) { + try { const c = JSON.parse(r.citations_json); if (Array.isArray(c)) turn.citations = c; } catch {} + } + out.push(turn); } return out; }; @@ -282,10 +283,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat send({ type: "done", text: ev.text, citations: cited }); } else send(ev); } - if (answer) { - await addMessage(convId, "assistant", answer); - await addMessage(convId, "sources", JSON.stringify(cited)); - } + if (answer) await addMessage(convId, "assistant", answer, cited); } catch (e) { const m = (e as Error).message; if (resolved.source === "byok") log(`chat error for user ${user.id} (own key): ${m}`); diff --git a/packages/feature-ai/src/index.ts b/packages/feature-ai/src/index.ts index 405c90e..66c642d 100644 --- a/packages/feature-ai/src/index.ts +++ b/packages/feature-ai/src/index.ts @@ -1,7 +1,6 @@ export { createSecretBox, type SecretBox } from "./crypto"; -/** Migration folders for this package. Empty: the AI package uses the ai_* tables from 0001_init.sql. */ -export const migrationsDirs: string[] = []; +export { migrationsDirs } from "./migrations"; export * from "./settings"; export * from "./providers"; export * from "./retrieval/course-index"; diff --git a/packages/feature-ai/src/migrations.ts b/packages/feature-ai/src/migrations.ts new file mode 100644 index 0000000..4c24074 --- /dev/null +++ b/packages/feature-ai/src/migrations.ts @@ -0,0 +1,7 @@ +import { join } from "node:path"; + +/** + * Migration folders for this package; the server passes them to core `migrate()`. The files upgrade the v1 ai_* + * tables of core's 0001_init.sql. Data that portable SQL cannot move runs in `upgradeAiData` (src/upgrade.ts). + */ +export const migrationsDirs: string[] = [join(import.meta.dir, "..", "migrations")]; diff --git a/packages/feature-ai/test/feature.test.ts b/packages/feature-ai/test/feature.test.ts index 84579dc..5042444 100644 --- a/packages/feature-ai/test/feature.test.ts +++ b/packages/feature-ai/test/feature.test.ts @@ -6,7 +6,7 @@ import type { AppConfig, AuthUser, JobRecord, ServerContext } from "@stc/contrac import { createApp, createJobQueue, resolveConfig } from "@stc/core"; import { createCourseRepository } from "@stc/course-format"; import type { Db } from "@stc/db"; -import { forEachDb } from "../../db/test/helpers"; +import { forEachAiDb as forEachDb } from "./helpers"; import { aiFeature, type AiFeatureOptions } from "../src/feature"; import { fakeProvider } from "./helpers"; @@ -104,6 +104,11 @@ forEachDb("chat cites course files, applies mentor rules, and stores the convers expect(conv.messages.map((m: { role: string }) => m.role)).toEqual(["user", "assistant"]); expect(conv.messages[1].citations.some((c: { path: string }) => c.path === "lessons/04-exercise.md")).toBe(true); expect((await call("GET", `/api/ai/conversations/${convs[0].id}`, "bob")).status).toBe(404); + // citations live in the assistant row's column, not in a separate "sources" row + const stored = await db.sql`SELECT role, citations_json FROM ai_messages WHERE conversation_id = ${convs[0].id}`; + expect(stored.map((m: { role: string }) => m.role).sort()).toEqual(["assistant", "user"]); + const assistantRow = stored.find((m: { role: string }) => m.role === "assistant"); + expect(JSON.parse(assistantRow.citations_json).some((c: { path: string }) => c.path === "lessons/04-exercise.md")).toBe(true); // Learner switches mentor mode off for this course: rules leave the prompt. expect((await call("PUT", "/api/ai/courses/sample/mentor", "ann", { off: true })).status).toBe(200); diff --git a/packages/feature-ai/test/helpers.ts b/packages/feature-ai/test/helpers.ts index d54648f..cc3846a 100644 --- a/packages/feature-ai/test/helpers.ts +++ b/packages/feature-ai/test/helpers.ts @@ -1,4 +1,7 @@ import type { AiProvider, ChatMessage, ToolSpec } from "@stc/contracts"; +import { migrate, type Db } from "@stc/db"; +import { forEachDb } from "../../db/test/helpers"; +import { migrationsDirs } from "../src/migrations"; /** SSE body from a list of [event, data] pairs, split into awkward chunks to exercise buffering. */ export function sseBody(events: [string | null, unknown][], chunk = 7): ReadableStream { @@ -51,3 +54,11 @@ export function fakeProvider(turns: ((messages: ChatMessage[], tools?: ToolSpec[ }; return { provider, seen }; } + +/** forEachDb with the AI package's migrations applied on top of core (the schema the server runs). */ +export function forEachAiDb(name: string, fn: (db: Db) => Promise): void { + forEachDb(name, async (db) => { + await migrate(db, { "feature-ai": migrationsDirs }); + await fn(db); + }); +} diff --git a/packages/feature-ai/test/settings.test.ts b/packages/feature-ai/test/settings.test.ts index 6dec14a..3febe6c 100644 --- a/packages/feature-ai/test/settings.test.ts +++ b/packages/feature-ai/test/settings.test.ts @@ -1,5 +1,5 @@ import { expect } from "bun:test"; -import { forEachDb } from "../../db/test/helpers"; +import { forEachAiDb as forEachDb } from "./helpers"; import { createSecretBox } from "../src/crypto"; import { aiSettingsRepo } from "../src/settings"; diff --git a/packages/feature-ai/test/upgrade.test.ts b/packages/feature-ai/test/upgrade.test.ts new file mode 100644 index 0000000..56fe1a4 --- /dev/null +++ b/packages/feature-ai/test/upgrade.test.ts @@ -0,0 +1,41 @@ +import { expect } from "bun:test"; +import { migrate, type Db } from "@stc/db"; +// Core migrations only: the ai_* tables exactly as v1 shipped them. +import { forEachDb as forEachV1Db } from "../../db/test/helpers"; +import { migrationsDirs } from "../src/migrations"; + +const T = "2026-10-01T00:00:00.000Z"; +const aiMigrations = { "feature-ai": migrationsDirs }; + +async function seedMessage(db: Db, id: string, conv: string, role: string, content: string) { + await db.sql`INSERT INTO ai_messages (id, conversation_id, role, content, created_at) VALUES (${id}, ${conv}, ${role}, ${content}, ${T})`; +} + +forEachV1Db("v1 upgrade: sources rows fold into citations_json on the assistant row before them", async (db) => { + const cites = JSON.stringify([{ path: "lessons/04-exercise.md", title: "Adding numbers", lessonId: "m2-ex-add" }]); + await db.sql`INSERT INTO ai_conversations (id, user_id, course_id, title, created_at) VALUES ('c1', 'u1', 'sample', 'Hi', ${T})`; + await db.sql`INSERT INTO ai_conversations (id, user_id, course_id, title, created_at) VALUES ('c2', 'u1', 'sample', 'Two', ${T})`; + // ids sort in the order v1 wrote them (UUIDv7); c1: user, assistant, sources, user, assistant (no sources) + await seedMessage(db, "0001-a", "c1", "user", "How do I add?"); + await seedMessage(db, "0001-b", "c1", "assistant", "Which operator?"); + await seedMessage(db, "0001-c", "c1", "sources", cites); + await seedMessage(db, "0001-d", "c1", "user", "And then?"); + await seedMessage(db, "0001-e", "c1", "assistant", "Try it."); + // c2: an orphan sources row (no assistant before it) and an assistant whose next row belongs to c1's ids range + await seedMessage(db, "0002-a", "c2", "sources", "[]"); + await seedMessage(db, "0002-b", "c2", "assistant", "Alone."); + + const applied = await migrate(db, aiMigrations); + expect(applied).toContain("0001_ai_message_citations.sql"); + + const rows = await db.sql`SELECT id, role, content, citations_json FROM ai_messages ORDER BY id`; + expect(rows.map((r: { id: string; role: string }) => `${r.id}:${r.role}`)).toEqual([ + "0001-a:user", "0001-b:assistant", "0001-d:user", "0001-e:assistant", "0002-b:assistant", + ]); + const byId = Object.fromEntries(rows.map((r: { id: string; citations_json: string | null }) => [r.id, r.citations_json])); + expect(JSON.parse(byId["0001-b"]!)).toEqual(JSON.parse(cites)); + expect(byId["0001-e"]).toBeNull(); + expect(byId["0002-b"]).toBeNull(); + expect(byId["0001-a"]).toBeNull(); + expect(await migrate(db, aiMigrations)).toEqual([]); +}); From a995900fba01ae243e77d7319807e1306f32a8d6 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 20:30:59 +0100 Subject: [PATCH 3/7] feat(ai): per-course mentor preferences table (contract request 4) 0002_ai_mentor_prefs.sql adds ai_mentor_prefs(user_id, course_id, off). upgradeAiData copies v1 config_json.mentorOff toggles into it once (marked ai_data_v11 in schema_migrations); the settings repo awaits the upgrade before first use and aiFeature starts it at construction. Co-Authored-By: Claude Opus 5.5 --- .../migrations/0002_ai_mentor_prefs.sql | 7 +++ packages/feature-ai/src/feature.ts | 2 + packages/feature-ai/src/index.ts | 1 + packages/feature-ai/src/settings.ts | 45 +++++++++++++------ packages/feature-ai/src/upgrade.ts | 45 +++++++++++++++++++ packages/feature-ai/test/settings.test.ts | 14 ++++++ packages/feature-ai/test/upgrade.test.ts | 43 ++++++++++++++++++ 7 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 packages/feature-ai/migrations/0002_ai_mentor_prefs.sql create mode 100644 packages/feature-ai/src/upgrade.ts diff --git a/packages/feature-ai/migrations/0002_ai_mentor_prefs.sql b/packages/feature-ai/migrations/0002_ai_mentor_prefs.sql new file mode 100644 index 0000000..13da201 --- /dev/null +++ b/packages/feature-ai/migrations/0002_ai_mentor_prefs.sql @@ -0,0 +1,7 @@ +-- v1.1 (contract request 4): per-course mentor toggles get their own table. A row exists only while the learner +-- has switched the course's mentor rules off. v1 kept them in ai_settings.config_json.mentorOff; reading that JSON +-- is not portable SQL, so upgradeAiData (src/upgrade.ts) copies it on first start. +CREATE TABLE ai_mentor_prefs ( + user_id TEXT NOT NULL, course_id TEXT NOT NULL, off INTEGER NOT NULL, updated_at TEXT NOT NULL, + PRIMARY KEY (user_id, course_id) +); diff --git a/packages/feature-ai/src/feature.ts b/packages/feature-ai/src/feature.ts index e283fc1..3582386 100644 --- a/packages/feature-ai/src/feature.ts +++ b/packages/feature-ai/src/feature.ts @@ -145,6 +145,8 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const progress = progressRepo(ctx.db); const sink = opts.draftSink ?? createFsDraftSink(ctx.config.dataDir); const log = opts.log ?? ((m: string) => console.error(`[ai] ${m}`)); + // The v1 data upgrade (mentor toggles, MCP secrets) starts now; every settings call also waits for it. + settings.ready().catch((e) => log(`v1.1 data upgrade failed (retried on next use): ${(e as Error).message}`)); // AppConfig.aiAllowedBaseUrls (STC_AI_ALLOWED_BASE_URLS) plus the admin's list in the server AI settings. const configOrigins = parseAllowedOrigins((ctx.config.aiAllowedBaseUrls ?? []).join(",")); const allowedOrigins = async () => [...new Set([...configOrigins, ...(await settings.getServer()).allowedBaseUrls])]; diff --git a/packages/feature-ai/src/index.ts b/packages/feature-ai/src/index.ts index 66c642d..2582896 100644 --- a/packages/feature-ai/src/index.ts +++ b/packages/feature-ai/src/index.ts @@ -2,6 +2,7 @@ export { createSecretBox, type SecretBox } from "./crypto"; export { migrationsDirs } from "./migrations"; export * from "./settings"; +export { upgradeAiData, AI_DATA_UPGRADE } from "./upgrade"; export * from "./providers"; export * from "./retrieval/course-index"; export { Bm25, tokenize } from "./retrieval/bm25"; diff --git a/packages/feature-ai/src/settings.ts b/packages/feature-ai/src/settings.ts index ecc9776..b0dfa04 100644 --- a/packages/feature-ai/src/settings.ts +++ b/packages/feature-ai/src/settings.ts @@ -1,5 +1,6 @@ import type { Db } from "@stc/db"; import type { SecretBox } from "./crypto"; +import { upgradeAiData } from "./upgrade"; export type ProviderKind = "anthropic" | "openai"; export type Effort = "low" | "medium" | "high" | "xhigh" | "max"; @@ -30,7 +31,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 UserRow = { byok: ProviderConfig | null; mentorOff: Record }; +type UserRow = { byok: ProviderConfig | null }; const pickProvider = (p: ProviderConfig): ProviderConfig => { const out: ProviderConfig = { kind: p.kind }; @@ -46,14 +47,18 @@ const parse = (s: string | undefined, fallback: T): T => { }; /** - * AI settings in the ai_settings and user_api_keys tables of 0001_init.sql. + * 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. - * - ('user', ) holds the BYOK provider choice and per-course mentor toggles. The frozen primary key - * is (scope, owner_id), so the per-course flag lives in config_json; the mentor_off column means - * "off for every course" and stays 0 (see docs/contract-requests/ai.md). + * - ('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. + * 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) { + let upgraded: Promise | null = null; + const ready = () => + (upgraded ??= upgradeAiData(db, box).catch((e) => { upgraded = null; throw e; })); + const read = async (scope: "server" | "user", owner: string): Promise => { const rows = await db.sql`SELECT config_json FROM ai_settings WHERE scope = ${scope} AND owner_id = ${owner}`; return rows[0]?.config_json; @@ -68,16 +73,23 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { }); }; const serverRow = async (): Promise => parse(await read("server", ""), { provider: null, apiKeyCipher: null, mcpServers: [] }); - const userRow = async (userId: string): Promise => parse(await read("user", userId), { byok: null, mentorOff: {} }); + const userRow = async (userId: string): Promise => { + const { mentorOff: _legacy, ...row } = parse(await read("user", userId), { byok: null } as UserRow & { mentorOff?: unknown }); + return row; + }; 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; return { + /** Resolves once the v1 data upgrade is done (aiFeature starts it at construction). */ + ready: async (): Promise => { await ready(); }, 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 ?? [] }; }, async saveServer(input: ServerSettingsInput): Promise { + await ready(); const prev = await serverRow(); let apiKeyCipher: string | null = null; if (input.provider) { @@ -92,15 +104,19 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { }); }, async serverApiKey(): Promise { + await ready(); const r = await serverRow(); return r.provider && r.apiKeyCipher ? box.decrypt(r.apiKeyCipher) : null; }, async getUser(userId: string): Promise { + await ready(); const r = await userRow(userId); const k = await keyRow(userId); - return { byok: r.byok && k ? { ...r.byok, hasApiKey: true } : null, mentorOff: r.mentorOff ?? {} }; + const prefs = (await db.sql`SELECT course_id FROM ai_mentor_prefs WHERE user_id = ${userId} AND off = 1`) as { course_id: string }[]; + return { byok: r.byok && k ? { ...r.byok, hasApiKey: true } : null, mentorOff: Object.fromEntries(prefs.map((p) => [p.course_id, true])) }; }, async saveUserKey(userId: string, cfg: ProviderConfig & { apiKey: string }): Promise { + await ready(); const ciphertext = await box.encrypt(cfg.apiKey); const now = new Date().toISOString(); await db.sql.begin(async (tx) => { @@ -111,23 +127,26 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { await write("user", userId, { ...r, byok: pickProvider(cfg) }); }, async removeUserKey(userId: string): Promise { + await ready(); await db.sql`DELETE FROM user_api_keys WHERE user_id = ${userId}`; const r = await userRow(userId); await write("user", userId, { ...r, byok: null }); }, async userApiKey(userId: string): Promise { + await ready(); const k = await keyRow(userId); return k ? box.decrypt(k.ciphertext) : null; }, async setMentorOff(userId: string, courseId: string, off: boolean): Promise { - const r = await userRow(userId); - const mentorOff = { ...r.mentorOff }; - if (off) mentorOff[courseId] = true; - else delete mentorOff[courseId]; - await write("user", userId, { ...r, mentorOff }); + await ready(); + await db.sql.begin(async (tx) => { + await tx`DELETE FROM ai_mentor_prefs WHERE user_id = ${userId} AND course_id = ${courseId}`; + if (off) await tx`INSERT INTO ai_mentor_prefs (user_id, course_id, off, updated_at) VALUES (${userId}, ${courseId}, 1, ${new Date().toISOString()})`; + }); }, async isMentorOff(userId: string, courseId: string): Promise { - return (await userRow(userId)).mentorOff?.[courseId] === true; + await ready(); + return (await db.sql`SELECT 1 AS x FROM ai_mentor_prefs WHERE user_id = ${userId} AND course_id = ${courseId} AND off = 1`).length > 0; }, }; } diff --git a/packages/feature-ai/src/upgrade.ts b/packages/feature-ai/src/upgrade.ts new file mode 100644 index 0000000..26767c2 --- /dev/null +++ b/packages/feature-ai/src/upgrade.ts @@ -0,0 +1,45 @@ +import type { Db } from "@stc/db"; +import type { SecretBox } from "./crypto"; + +/** 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"; + +type Tx = Bun.TransactionSQL; + +const parseObject = (s: string): Record | null => { + try { + const v = JSON.parse(s); + return v && typeof v === "object" && !Array.isArray(v) ? v : null; + } catch { return null; } +}; + +/** v1 kept per-course toggles in the user row's config_json.mentorOff; move them to ai_mentor_prefs. */ +async function moveMentorToggles(tx: Tx, now: string) { + const rows = (await tx`SELECT owner_id, config_json FROM ai_settings WHERE scope = 'user'`) as { owner_id: string; config_json: string }[]; + for (const r of rows) { + const cfg = parseObject(r.config_json); + if (!cfg || !("mentorOff" in cfg)) continue; + const off = cfg.mentorOff && typeof cfg.mentorOff === "object" ? (cfg.mentorOff as Record) : {}; + for (const [courseId, v] of Object.entries(off)) { + if (v !== true || !courseId) continue; + const exists = await tx`SELECT 1 AS x FROM ai_mentor_prefs WHERE user_id = ${r.owner_id} AND course_id = ${courseId}`; + if (!exists.length) await tx`INSERT INTO ai_mentor_prefs (user_id, course_id, off, updated_at) VALUES (${r.owner_id}, ${courseId}, 1, ${now})`; + } + const { mentorOff: _drop, ...rest } = cfg; + await tx`UPDATE ai_settings SET config_json = ${JSON.stringify(rest)}, updated_at = ${now} WHERE scope = 'user' AND owner_id = ${r.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 { + 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 tx`INSERT INTO schema_migrations (name, applied_at) VALUES (${AI_DATA_UPGRADE}, ${now})`; + return true; + }); +} diff --git a/packages/feature-ai/test/settings.test.ts b/packages/feature-ai/test/settings.test.ts index 3febe6c..6edf375 100644 --- a/packages/feature-ai/test/settings.test.ts +++ b/packages/feature-ai/test/settings.test.ts @@ -61,3 +61,17 @@ forEachDb("mentor mode is on by default and toggles per course", async (db) => { 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); +}); diff --git a/packages/feature-ai/test/upgrade.test.ts b/packages/feature-ai/test/upgrade.test.ts index 56fe1a4..dd8ba37 100644 --- a/packages/feature-ai/test/upgrade.test.ts +++ b/packages/feature-ai/test/upgrade.test.ts @@ -2,9 +2,13 @@ import { expect } from "bun:test"; import { migrate, type Db } from "@stc/db"; // Core migrations only: the ai_* tables exactly as v1 shipped them. import { forEachDb as forEachV1Db } from "../../db/test/helpers"; +import { createSecretBox } from "../src/crypto"; import { migrationsDirs } from "../src/migrations"; +import { aiSettingsRepo } from "../src/settings"; +import { upgradeAiData } from "../src/upgrade"; const T = "2026-10-01T00:00:00.000Z"; +const box = createSecretBox("test-secret"); const aiMigrations = { "feature-ai": migrationsDirs }; async function seedMessage(db: Db, id: string, conv: string, role: string, content: string) { @@ -39,3 +43,42 @@ forEachV1Db("v1 upgrade: sources rows fold into citations_json on the assistant expect(byId["0001-a"]).toBeNull(); expect(await migrate(db, aiMigrations)).toEqual([]); }); + +async function seedSettings(db: Db, scope: "server" | "user", owner: string, config: unknown) { + const json = typeof config === "string" ? config : JSON.stringify(config); + await db.sql`INSERT INTO ai_settings (scope, owner_id, mentor_off, config_json, updated_at) VALUES (${scope}, ${owner}, 0, ${json}, ${T})`; +} + +forEachV1Db("v1 upgrade: per-course mentor toggles move from config_json to ai_mentor_prefs", async (db) => { + await seedSettings(db, "user", "u1", { byok: { kind: "anthropic" }, mentorOff: { sample: true, other: false } }); + await seedSettings(db, "user", "u2", { byok: null, mentorOff: {} }); + await seedSettings(db, "user", "u3", "{not json"); + await db.sql`INSERT INTO user_api_keys (user_id, provider, ciphertext, created_at) VALUES ('u1', 'anthropic', ${await box.encrypt("sk-u1")}, ${T})`; + + expect(await migrate(db, aiMigrations)).toContain("0002_ai_mentor_prefs.sql"); + expect(await upgradeAiData(db, box)).toBe(true); + + const prefs = await db.sql`SELECT user_id, course_id, off FROM ai_mentor_prefs ORDER BY user_id, course_id`; + expect(prefs.map((r: { user_id: string; course_id: string; off: number }) => [r.user_id, r.course_id, Number(r.off)])).toEqual([["u1", "sample", 1]]); + const cfg = async (u: string) => (await db.sql`SELECT config_json FROM ai_settings WHERE scope = 'user' AND owner_id = ${u}`)[0].config_json as string; + expect(JSON.parse(await cfg("u1"))).toEqual({ byok: { kind: "anthropic" } }); + expect(JSON.parse(await cfg("u2"))).toEqual({ byok: null }); + expect(await cfg("u3")).toBe("{not json"); + + const repo = aiSettingsRepo(db, box); + expect(await repo.isMentorOff("u1", "sample")).toBe(true); + expect(await repo.isMentorOff("u1", "other")).toBe(false); + expect(await repo.getUser("u1")).toEqual({ byok: { kind: "anthropic", hasApiKey: true }, mentorOff: { sample: true } }); + + // idempotent: the marker stops a second run + expect(await upgradeAiData(db, box)).toBe(false); + expect((await db.sql`SELECT COUNT(*) AS n FROM ai_mentor_prefs`)[0].n.toString()).toBe("1"); + expect((await db.sql`SELECT name FROM schema_migrations WHERE name = 'ai_data_v11'`)).toHaveLength(1); +}); + +forEachV1Db("v1 upgrade: the settings repo upgrades on first use", async (db) => { + await seedSettings(db, "user", "u1", { byok: null, mentorOff: { sample: true } }); + await migrate(db, aiMigrations); + expect(await aiSettingsRepo(db, box).isMentorOff("u1", "sample")).toBe(true); + expect(JSON.parse((await db.sql`SELECT config_json FROM ai_settings WHERE owner_id = 'u1'`)[0].config_json)).toEqual({ byok: null }); +}); From ddc66f5be5c4d6d0a8bcbd64ae5e0332483d5902 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 20:33:56 +0100 Subject: [PATCH 4/7] feat(ai): encrypt MCP env and headers at rest and mask them in the admin API env/headers are sealed into secretCipher with the BYOK AES-GCM/HKDF box. The admin GET returns masked values; a mask posted back for the same server and key keeps the stored value, any other mask is a 400. The chat and the test button use the stored values. upgradeAiData seals plaintext v1 rows. Co-Authored-By: Claude Opus 5.5 --- packages/feature-ai/src/feature.ts | 17 +++- packages/feature-ai/src/mcp/secrets.ts | 87 +++++++++++++++++++++ packages/feature-ai/src/settings.ts | 34 ++++++-- packages/feature-ai/src/upgrade.ts | 15 +++- packages/feature-ai/src/web/AdminAiPage.tsx | 2 +- packages/feature-ai/test/feature.test.ts | 38 +++++++++ packages/feature-ai/test/settings.test.ts | 48 +++++++++++- packages/feature-ai/test/upgrade.test.ts | 32 ++++++++ 8 files changed, 261 insertions(+), 12 deletions(-) create mode 100644 packages/feature-ai/src/mcp/secrets.ts 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" ? ( - +