From a13cc66ebb30770362731276a306618e41ba974d Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 04:47:25 +0100 Subject: [PATCH] fix(ai): BYOK endpoints limited to official presets and admin-allowed origins (SSRF) - A user's own key goes only to https://api.anthropic.com, https://api.openai.com/v1 or an origin listed in STC_AI_ALLOWED_BASE_URLS / the server config's allowedBaseUrls; anything else is a 400, and a stored key whose origin was later removed stops being used. - User-scope provider calls never follow redirects, and their errors reach the user only as the HTTP status plus a fixed message; the full text is logged. - The admin's server-scope provider may still use any base URL. - Admin UI: allowed origins field; MCP stdio env editable, HTTP headers kept. Co-Authored-By: Claude Opus 5.5 --- packages/feature-ai/src/endpoints.ts | 92 +++++++++++++++++++ packages/feature-ai/src/feature.ts | 56 ++++++++--- packages/feature-ai/src/settings.ts | 10 +- packages/feature-ai/src/web/AccountAiPage.tsx | 2 +- packages/feature-ai/src/web/AdminAiPage.tsx | 34 +++++-- packages/feature-ai/test/feature.test.ts | 68 +++++++++++++- packages/feature-ai/test/settings.test.ts | 2 +- 7 files changed, 236 insertions(+), 28 deletions(-) create mode 100644 packages/feature-ai/src/endpoints.ts diff --git a/packages/feature-ai/src/endpoints.ts b/packages/feature-ai/src/endpoints.ts new file mode 100644 index 0000000..e0368a4 --- /dev/null +++ b/packages/feature-ai/src/endpoints.ts @@ -0,0 +1,92 @@ +import type { AiProvider } from "@stc/contracts"; +import type { ProviderKind } from "./settings"; + +/** + * Where a user's own key (BYOK) may be sent. Learners must not be able to point the server at arbitrary hosts + * (SSRF), so BYOK uses the official endpoint for its kind, or an origin the admin allowed explicitly through + * STC_AI_ALLOWED_BASE_URLS (comma-separated origins) or the server AI config's `allowedBaseUrls`. + * The admin's own server-scope provider may use any base URL (Ollama and the like). + */ +export const PRESET_BASE_URLS: Record = { + anthropic: "https://api.anthropic.com", + openai: "https://api.openai.com/v1", +}; + +const trimSlash = (s: string) => s.replace(/\/+$/, ""); + +/** The origin of an http(s) URL, or null. "Exact origins" means scheme, host and port must all match. */ +export function originOf(u: string): string | null { + try { + const url = new URL(u.trim()); + if (url.protocol !== "http:" && url.protocol !== "https:") return null; + if (url.username || url.password) return null; + return url.origin; + } catch { + return null; + } +} + +/** Parses an allowlist (array or comma-separated string) into origins; entries that are not plain origins are dropped. */ +export function parseAllowedOrigins(v: unknown): string[] { + const items = Array.isArray(v) ? v.map(String) : typeof v === "string" ? v.split(",") : []; + const out = new Set(); + for (const raw of items) { + const s = trimSlash(raw.trim()); + if (!s) continue; + const o = originOf(s); + if (o && o === s) out.add(o); + } + return [...out]; +} + +/** + * The base URL a BYOK config may use, or null when it is not allowed. + * - empty: the official endpoint for the kind; + * - the official endpoint for the kind (with or without a trailing slash); + * - any URL whose origin is on the allowlist. + */ +export function byokBaseUrl(kind: ProviderKind, baseUrl: string | undefined, allowed: string[]): string | null { + const preset = PRESET_BASE_URLS[kind]; + if (!baseUrl || !baseUrl.trim()) return preset; + const clean = trimSlash(baseUrl.trim()); + if (clean === preset) return preset; + const origin = originOf(clean); + if (!origin) return null; + const url = new URL(clean); + if (url.search || url.hash) return null; + return allowed.includes(origin) ? clean : null; +} + +/** fetch for user-scope calls: never follows redirects (an allowed host must not bounce the request elsewhere). */ +export function noRedirectFetch(base: typeof fetch): typeof fetch { + const f = ((input: RequestInfo | URL, init?: RequestInit) => base(input, { ...init, redirect: "manual" })) as typeof fetch; + return f; +} + +export const USER_PROVIDER_ERROR = "The AI provider returned an error. Check your key, model and endpoint under Account, AI."; + +/** The message shown for a failed user-scope provider call: the HTTP status, if any, and a fixed sentence. */ +export function publicProviderError(message: string): string { + const status = /(?:^|\s)([1-5]\d{2}):/.exec(message)?.[1]; + return status ? `${status}: ${USER_PROVIDER_ERROR}` : USER_PROVIDER_ERROR; +} + +/** + * Wraps a provider created with a user's key: any error is logged server-side with its full text and rethrown + * as the status plus a fixed message, so upstream response bodies never reach the browser. + */ +export function sanitizeProviderErrors(p: AiProvider, log: (msg: string) => void): AiProvider { + return { + id: p.id, + async *chat(messages, opts) { + try { + yield* p.chat(messages, opts); + } catch (e) { + const raw = (e as Error)?.message ?? String(e); + if ((e as Error)?.name === "AbortError") throw e; + log(raw); + throw new Error(publicProviderError(raw)); + } + }, + }; +} diff --git a/packages/feature-ai/src/feature.ts b/packages/feature-ai/src/feature.ts index 625fa23..ad521f8 100644 --- a/packages/feature-ai/src/feature.ts +++ b/packages/feature-ai/src/feature.ts @@ -12,6 +12,7 @@ import { createFsDraftSink, type DraftSink } from "./generator/draft-sink"; import { generateDraft, type GenerateRequest, type GenerateSource } from "./generator/generate"; 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 { aiSettingsRepo, type McpServerConfig, type ProviderConfig } from "./settings"; @@ -23,6 +24,10 @@ export interface AiFeatureOptions { /** Overrides provider construction (tests use a fake AiProvider). */ providerFactory?: (cfg: ProviderConfig, apiKey: string | null) => AiProvider; mcpTimeoutMs?: number; + /** Environment to read STC_AI_ALLOWED_BASE_URLS from. Default: process.env. */ + env?: Record; + /** Server-side log for provider errors that are not shown to users. Default: console.error. */ + log?: (message: string) => void; } const json = (body: unknown, status = 200) => Response.json(body, { status }); @@ -40,7 +45,7 @@ async function body(req: Request): Promise | null> { } const str = (v: unknown): string | undefined => (typeof v === "string" && v.trim() ? v.trim() : undefined); -function parseProvider(v: unknown): (ProviderConfig & { apiKey?: string; clearApiKey?: boolean }) | null | "invalid" { +function parseProvider(v: unknown, scope: "server" | "user"): (ProviderConfig & { apiKey?: string; clearApiKey?: boolean }) | null | "invalid" { if (v === null || v === undefined) return null; if (typeof v !== "object") return "invalid"; const p = v as Record; @@ -55,7 +60,8 @@ function parseProvider(v: unknown): (ProviderConfig & { apiKey?: string; clearAp if (str(p.effort)) out.effort = str(p.effort) as ProviderConfig["effort"]; if (str(p.apiKey)) out.apiKey = str(p.apiKey); if (p.clearApiKey === true) out.clearApiKey = true; - if (out.kind === "openai" && (!out.baseUrl || !out.model)) return "invalid"; + // Server scope: any endpoint, but OpenAI-compatible needs one. User scope: an empty base URL means the official endpoint. + if (out.kind === "openai" && (!out.model || (scope === "server" && !out.baseUrl))) return "invalid"; return out; } @@ -140,15 +146,28 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const notes = notesRepo(ctx.db); const progress = progressRepo(ctx.db); const sink = opts.draftSink ?? createFsDraftSink(ctx.config.dataDir); - const makeProvider = opts.providerFactory ?? ((cfg: ProviderConfig, key: string | null) => createProvider(cfg, key, opts.fetch)); + const log = opts.log ?? ((m: string) => console.error(`[ai] ${m}`)); + const env = opts.env ?? process.env; + const allowedOrigins = async () => [...new Set([...parseAllowedOrigins(env.STC_AI_ALLOWED_BASE_URLS ?? ""), ...(await settings.getServer()).allowedBaseUrls])]; + /** + * Providers for a user's own key never follow redirects, and their errors reach the user only as the HTTP + * status plus a fixed sentence (the full text is logged here). Server-scope providers are the admin's own. + */ + const makeProvider = (r: { source: "byok" | "server"; cfg: ProviderConfig; key: string | null }, userId: string): AiProvider => { + if (r.source === "server") return opts.providerFactory ? opts.providerFactory(r.cfg, r.key) : createProvider(r.cfg, r.key, opts.fetch); + const p = opts.providerFactory ? opts.providerFactory(r.cfg, r.key) : createProvider(r.cfg, r.key, noRedirectFetch(opts.fetch ?? fetch)); + return sanitizeProviderErrors(p, (m) => log(`provider error for user ${userId} (own key): ${m}`)); + }; /** BYOK first ("used only for their own requests"), then the server provider. */ const resolveProvider = async (userId: string | null): Promise<{ source: "byok" | "server"; cfg: ProviderConfig; key: string | null } | null> => { if (userId) { const u = await settings.getUser(userId); if (u.byok) { - const key = await settings.userApiKey(userId).catch(() => null); - if (key) return { source: "byok", cfg: u.byok, key }; + // Re-checked on every use: an origin the admin removed from the allowlist stops working at once. + const base = byokBaseUrl(u.byok.kind, u.byok.baseUrl, await allowedOrigins()); + const key = base ? await settings.userApiKey(userId).catch(() => null) : null; + if (key && base) return { source: "byok", cfg: { ...u.byok, baseUrl: base }, key }; } } const s = await settings.getServer(); @@ -208,7 +227,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const resolved = await resolveProvider(user.id); if (!resolved) return err(409, "AI is not set up. An admin can configure a server provider, or you can add your own key under Account, AI."); let provider: AiProvider; - try { provider = makeProvider(resolved.cfg, resolved.key); } catch (e) { return err(409, (e as Error).message); } + try { provider = makeProvider(resolved, user.id); } catch (e) { return err(409, (e as Error).message); } let conversationId = str(b?.conversationId); if (conversationId) { @@ -261,7 +280,9 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat await addMessage(convId, "sources", JSON.stringify(cited)); } } catch (e) { - send({ type: "error", message: (e as Error).message }); + const m = (e as Error).message; + if (resolved.source === "byok") log(`chat error for user ${user.id} (own key): ${m}`); + send({ type: "error", message: resolved.source === "byok" ? publicProviderError(m) : m }); } finally { await tools?.close?.().catch(() => {}); try { controller.close(); } catch {} @@ -275,7 +296,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const p = payload as { req: GenerateRequest; userId: string }; const resolved = await resolveProvider(p.userId); if (!resolved) throw new Error("AI is not set up for this user."); - const provider = makeProvider(resolved.cfg, resolved.key); + const provider = makeProvider(resolved, p.userId); return generateDraft({ provider, repo: ctx.courses, req: p.req, sink, authorId: p.userId, dataDir: ctx.config.dataDir, signal: jc.signal, onProgress: (f, m) => jc.reportProgress(f, m) }); }; @@ -305,11 +326,18 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat if (deny) return deny; const b = await body(req); if (!b) return err(400, "Invalid JSON."); - const provider = parseProvider(b.provider); + const provider = parseProvider(b.provider, "server"); const mcpServers = parseMcp(b.mcpServers); if (provider === "invalid") return err(400, "Invalid provider. OpenAI-compatible endpoints need a base URL and a model."); if (mcpServers === "invalid") return err(400, "Invalid MCP server list."); - await settings.saveServer({ provider, mcpServers }); + let allowedBaseUrls: string[] | undefined; + if (b.allowedBaseUrls !== undefined) { + const list = Array.isArray(b.allowedBaseUrls) ? b.allowedBaseUrls.map(String).map((x) => x.trim()).filter(Boolean) : null; + if (!list) return err(400, "allowedBaseUrls must be a list of origins."); + 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 }); return json(await settings.getServer()); }, }, @@ -340,9 +368,11 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat PUT: async (req, rc) => { if (!rc.user) return err(401, "Sign in first."); const b = await body(req); - const p = b ? parseProvider(b) : "invalid"; - if (!p || p === "invalid" || !p.apiKey) return err(400, "Choose a provider and paste your key. OpenAI-compatible endpoints also need a base URL and a model."); - await settings.saveUserKey(rc.user.id, { ...p, apiKey: p.apiKey }); + const p = b ? parseProvider(b, "user") : "invalid"; + if (!p || p === "invalid" || !p.apiKey) return err(400, "Choose a provider and paste your key. OpenAI-compatible endpoints also need a model."); + const base = byokBaseUrl(p.kind, p.baseUrl, await allowedOrigins()); + if (!base) return err(400, "That base URL is not allowed for your own key. Use the official endpoint, or ask an admin to allow it."); + await settings.saveUserKey(rc.user.id, { ...p, baseUrl: base === PRESET_BASE_URLS[p.kind] ? undefined : base, apiKey: p.apiKey }); return json((await settings.getUser(rc.user.id)).byok); }, DELETE: async (_req, rc) => { diff --git a/packages/feature-ai/src/settings.ts b/packages/feature-ai/src/settings.ts index 0d1360c..ecc9776 100644 --- a/packages/feature-ai/src/settings.ts +++ b/packages/feature-ai/src/settings.ts @@ -19,14 +19,17 @@ export interface McpServerConfig { courseIds?: string[]; } -export interface ServerSettings { provider: PublicProviderConfig | null; mcpServers: McpServerConfig[] } +/** `allowedBaseUrls`: origins learners' own keys (BYOK) may be sent to, besides the official endpoints. */ +export interface ServerSettings { provider: PublicProviderConfig | null; mcpServers: McpServerConfig[]; allowedBaseUrls: string[] } export interface ServerSettingsInput { provider: (ProviderConfig & { apiKey?: string; clearApiKey?: boolean }) | null; mcpServers: McpServerConfig[]; + /** Omitted: keep the saved list. */ + allowedBaseUrls?: string[]; } export interface UserSettings { byok: PublicProviderConfig | null; mentorOff: Record } -type ServerRow = { provider: ProviderConfig | null; apiKeyCipher: string | null; mcpServers: McpServerConfig[] }; +type ServerRow = { provider: ProviderConfig | null; apiKeyCipher: string | null; mcpServers: McpServerConfig[]; allowedBaseUrls?: string[] }; type UserRow = { byok: ProviderConfig | null; mentorOff: Record }; const pickProvider = (p: ProviderConfig): ProviderConfig => { @@ -72,7 +75,7 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { return { async getServer(): Promise { const r = await serverRow(); - return { provider: r.provider ? { ...r.provider, hasApiKey: !!r.apiKeyCipher } : null, mcpServers: r.mcpServers ?? [] }; + return { provider: r.provider ? { ...r.provider, hasApiKey: !!r.apiKeyCipher } : null, mcpServers: r.mcpServers ?? [], allowedBaseUrls: r.allowedBaseUrls ?? [] }; }, async saveServer(input: ServerSettingsInput): Promise { const prev = await serverRow(); @@ -85,6 +88,7 @@ export function aiSettingsRepo(db: Db, box: SecretBox) { provider: input.provider ? pickProvider(input.provider) : null, apiKeyCipher, mcpServers: input.mcpServers ?? [], + allowedBaseUrls: input.allowedBaseUrls ?? prev.allowedBaseUrls ?? [], }); }, async serverApiKey(): Promise { diff --git a/packages/feature-ai/src/web/AccountAiPage.tsx b/packages/feature-ai/src/web/AccountAiPage.tsx index 0b0d41f..137cfe8 100644 --- a/packages/feature-ai/src/web/AccountAiPage.tsx +++ b/packages/feature-ai/src/web/AccountAiPage.tsx @@ -66,7 +66,7 @@ export default function AccountAiPage() { {kind === "openai" ? ( - + setBaseUrl(e.target.value)} placeholder="https://api.openai.com/v1" /> ) : null} diff --git a/packages/feature-ai/src/web/AdminAiPage.tsx b/packages/feature-ai/src/web/AdminAiPage.tsx index 0c3d0ca..21c4ad5 100644 --- a/packages/feature-ai/src/web/AdminAiPage.tsx +++ b/packages/feature-ai/src/web/AdminAiPage.tsx @@ -5,14 +5,19 @@ import { Field, PageHead } from "./components"; type Kind = "" | "anthropic" | "openai"; interface ProviderOut { kind: "anthropic" | "openai"; model?: string; baseUrl?: string; effort?: string; hasApiKey: boolean } -interface McpServer { id: string; name: string; transport: "stdio" | "http"; command?: string; args?: string[]; url?: string; courseIds?: string[] } -interface Config { provider: ProviderOut | null; mcpServers: McpServer[] } +interface McpServer { id: string; name: string; transport: "stdio" | "http"; command?: string; args?: string[]; env?: Record; url?: string; headers?: Record; courseIds?: string[] } +interface Config { provider: ProviderOut | null; mcpServers: McpServer[]; allowedBaseUrls: string[] } -type ServerDraft = { id: string; name: string; transport: "stdio" | "http"; command: string; args: string; url: string; courses: string; test?: string }; -const toDraft = (s: McpServer): ServerDraft => ({ id: s.id, name: s.name, transport: s.transport, command: s.command ?? "", args: (s.args ?? []).join(" "), url: s.url ?? "", courses: (s.courseIds ?? []).join(", ") }); +type ServerDraft = { id: string; name: string; transport: "stdio" | "http"; command: string; args: string; env: string; url: string; headers?: Record; courses: string; test?: string }; +const toDraft = (s: McpServer): ServerDraft => ({ + id: s.id, name: s.name, transport: s.transport, command: s.command ?? "", args: (s.args ?? []).join(" "), + env: Object.entries(s.env ?? {}).map(([k, v]) => `${k}=${v}`).join("\n"), url: s.url ?? "", headers: s.headers, courses: (s.courseIds ?? []).join(", "), +}); +const parseEnv = (text: string): Record => + Object.fromEntries(text.split("\n").map((l) => l.trim()).filter((l) => l.includes("=")).map((l) => [l.slice(0, l.indexOf("=")).trim(), l.slice(l.indexOf("=") + 1)])); const fromDraft = (d: ServerDraft): McpServer => ({ id: d.id, name: d.name || d.id, transport: d.transport, - ...(d.transport === "stdio" ? { command: d.command, args: d.args.trim() ? d.args.trim().split(/\s+/) : [] } : { url: d.url }), + ...(d.transport === "stdio" ? { command: d.command, args: d.args.trim() ? d.args.trim().split(/\s+/) : [], env: parseEnv(d.env) } : { url: d.url, ...(d.headers ? { headers: d.headers } : {}) }), courseIds: d.courses.split(",").map((s) => s.trim()).filter(Boolean), }); @@ -26,6 +31,7 @@ export default function AdminAiPage() { const [apiKey, setApiKey] = useState(""); const [clearKey, setClearKey] = useState(false); const [servers, setServers] = useState([]); + const [allowed, setAllowed] = useState(""); const [msg, setMsg] = useState<{ ok: boolean; text: string } | null>(null); const apply = (c: Config) => { @@ -37,6 +43,7 @@ export default function AdminAiPage() { setApiKey(""); setClearKey(false); setServers(c.mcpServers.map(toDraft)); + setAllowed((c.allowedBaseUrls ?? []).join("\n")); }; useEffect(() => { api("/api/ai/admin/config").then(apply, (e: ApiError) => setDenied(e.status === 401 ? "Sign in as the owner to change AI settings." : e.message)); @@ -46,7 +53,7 @@ export default function AdminAiPage() { setMsg(null); try { const provider = kind ? { kind, model, baseUrl, effort, apiKey: apiKey || undefined, clearApiKey: clearKey } : null; - apply(await api("/api/ai/admin/config", { method: "PUT", body: { provider, mcpServers: servers.map(fromDraft) } })); + apply(await api("/api/ai/admin/config", { method: "PUT", body: { provider, mcpServers: servers.map(fromDraft), allowedBaseUrls: allowed.split(/[\s,]+/).filter(Boolean) } })); setMsg({ ok: true, text: kind ? "Saved. The assistant is available to learners." : "Saved. AI is off for everyone without their own key." }); } catch (e) { setMsg({ ok: false, text: (e as Error).message }); @@ -118,6 +125,14 @@ export default function AdminAiPage() { ) : null} +
+

Endpoints for learners' own keys

+ Own keys go to api.anthropic.com or api.openai.com only. To let people use another endpoint, such as a shared Ollama, list its origin here (one per line, for example http://ollama.internal:11434). STC_AI_ALLOWED_BASE_URLS adds more. + +