diff --git a/Dockerfile b/Dockerfile index 12d14ec..932e7dc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -48,5 +48,5 @@ ENV NODE_ENV=production \ USER bun EXPOSE 3000 HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \ - CMD bun -e "fetch('http://127.0.0.1:3000/api/health').then(r => process.exit(r.ok ? 0 : 1)).catch(() => process.exit(1))" + CMD bun -e "fetch('http://127.0.0.1:' + (process.env.STC_PORT || 3000) + '/api/health').then(r => process.exit(r.ok ? 0 : 1)).catch(() => process.exit(1))" CMD ["bun", "apps/server/src/main.ts"] diff --git a/apps/server/src/features.ts b/apps/server/src/features.ts index 10c7028..c9a521a 100644 --- a/apps/server/src/features.ts +++ b/apps/server/src/features.ts @@ -1,6 +1,6 @@ import type { FeatureFactory, ServerContext } from "@stc/contracts"; import type { PackageMigrations } from "@stc/db"; -import { aiFeature, createFsDraftSink, migrationsDirs as aiMigrations, type DraftSink } from "@stc/feature-ai"; +import { aiFeature, createFsDraftSink, migrationsDirs as aiMigrations, type AiFeatureOptions, type DraftSink } from "@stc/feature-ai"; import { importFeature, migrationsDirs as importMigrations } from "@stc/feature-import"; import { learnerFeature, migrationsDirs as learnerMigrations, userIdentity } from "@stc/feature-learner"; import { createDraftWriter, migrationsDirs as studioMigrations, studioFeature, type StudioFeature } from "@stc/feature-studio"; @@ -30,15 +30,24 @@ export function aiDraftSink(ctx: ServerContext, studio: Pick importFeature(ctx), (ctx) => (studio = studioFeature(ctx, { watch: opts.studioWatch ?? true })), - (ctx) => aiFeature(ctx, { draftSink: aiDraftSink(ctx, studio!) }), + (ctx) => { + if (!studio) throw new Error("productFeatures: the studio factory must run before the AI factory"); + return aiFeature(ctx, { draftSink: aiDraftSink(ctx, studio), ...(opts.aiProviderFactory ? { providerFactory: opts.aiProviderFactory } : {}) }); + }, ]; } diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts index 6a46694..ad5e6e4 100644 --- a/apps/server/src/main.ts +++ b/apps/server/src/main.ts @@ -28,6 +28,7 @@ export async function startServer(config: AppConfig, features: (Feature | Featur return { server, ctx: composed.ctx, + features: composed.features, async stop() { await server.stop(true); await composed.stop(); diff --git a/apps/server/test/e2e.test.ts b/apps/server/test/e2e.test.ts index 2952b6a..5cccb3a 100644 --- a/apps/server/test/e2e.test.ts +++ b/apps/server/test/e2e.test.ts @@ -1,61 +1,95 @@ -// End-to-end (plan Task I3): the real server with every feature, against the fixtures, on SQLite and Postgres. +// End-to-end (plan Task I3): the real server with every feature, wired exactly as in production +// (`productFeatures()` order and `packageMigrations`), against the fixtures plus a restricted copy, on SQLite and +// Postgres. It is also the visibility and cross-site regression suite for the composed server. import { describe, expect, test } from "bun:test"; -import { mkdtempSync, readFileSync } from "node:fs"; +import { cpSync, existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import type { AiProvider } from "@stc/contracts"; import { loadConfig } from "@stc/core"; -import { studioFeature } from "@stc/feature-studio"; import { createBrowserRunner } from "../../../packages/feature-learner/src/web/runner/runner"; -import { aiDraftSink, packageMigrations, productFeatures } from "../src/features"; +import { packageMigrations, productFeatures } from "../src/features"; import { startServer } from "../src/main"; import { PG, pgSchema } from "./pg"; const FIXTURES = resolve(import.meta.dir, "../../../fixtures/courses"); +const SAMPLE = join(FIXTURES, "sample"); const ORIGIN = "http://localhost:3000"; +const JPEG = new Uint8Array([0xff, 0xd8, 0xff, 0xe0, 0, 0x10, 0x4a, 0x46, 0x49, 0x46, 0, 1, 0xff, 0xd9]); const cookieFrom = (r: Response) => r.headers.getSetCookie().map((c) => c.split(";")[0]).filter((c) => !c.endsWith("=")).join("; "); +/** A canned AI provider: chat answers with text; generation answers with one file block. */ +const fakeAi: AiProvider = { + id: "fake", + async *chat(messages) { + const sys = messages[0]?.content ?? ""; + if (sys.includes("=== FILE:")) yield { text: "=== FILE: lessons/generated.md ===\n# Generated\n\nA draft.\n=== END FILE ===\n" }; + else yield { text: "An answer." }; + }, +}; + +/** A restricted copy of the sample course ("secret", titled "Hidden Vault"). */ +function restrictedCourseDir(): string { + const dir = mkdtempSync(join(tmpdir(), "stc-e2e-courses-")); + cpSync(SAMPLE, join(dir, "secret"), { recursive: true }); + const m = readFileSync(join(SAMPLE, "stcourse.yaml"), "utf8"); + writeFileSync(join(dir, "secret/stcourse.yaml"), m.replace("id: sample", "id: secret").replace("title: Sample course", "title: Hidden Vault").replace("visibility: public", "visibility: restricted")); + return dir; +} + async function run(databaseUrl: string) { const dataDir = mkdtempSync(join(tmpdir(), "stc-e2e-")); const config = loadConfig({ STC_PORT: "0", STC_DATABASE_URL: databaseUrl, STC_SECRET: "e2e-secret-0123456789abcdef0123456789", - STC_DATA_DIR: dataDir, STC_COURSE_DIRS: FIXTURES, STC_PUBLIC_URL: ORIGIN, + STC_DATA_DIR: dataDir, STC_COURSE_DIRS: `${FIXTURES}:${restrictedCourseDir()}`, STC_PUBLIC_URL: ORIGIN, }); - const s = await startServer(config, productFeatures({ studioWatch: false }), { migrations: packageMigrations }); + // The importer's cover candidate for the restricted course. + mkdirSync(join(config.cacheDir, "covers", "secret"), { recursive: true }); + writeFileSync(join(config.cacheDir, "covers", "secret", "frame-1.jpg"), JPEG); + const s = await startServer(config, productFeatures({ studioWatch: false, aiProviderFactory: () => fakeAi }), { migrations: packageMigrations }); const base = s.server.url.toString().replace(/\/$/, ""); - const call = async (method: string, path: string, opts: { cookie?: string; body?: unknown } = {}) => { - const headers: Record = { origin: ORIGIN }; + type Opts = { cookie?: string; body?: unknown; headers?: Record; origin?: string | null }; + const call = async (method: string, path: string, opts: Opts = {}) => { + const headers: Record = { ...opts.headers }; + if (opts.origin !== null) headers.origin = opts.origin ?? ORIGIN; if (opts.cookie) headers.cookie = opts.cookie; - if (opts.body !== undefined) headers["content-type"] = "application/json"; - return fetch(base + path, { method, headers, body: opts.body === undefined ? undefined : JSON.stringify(opts.body) }); + if (opts.body !== undefined) headers["content-type"] ??= "application/json"; + const body = opts.body === undefined ? undefined : typeof opts.body === "string" ? opts.body : JSON.stringify(opts.body); + return fetch(base + path, { method, headers, body }); }; + const json = async (method: string, path: string, opts: Opts = {}) => (await call(method, path, opts)).json() as Promise; const signUp = async (email: string, name: string) => { const r = await call("POST", "/api/auth/sign-up/email", { body: { email, name, password: "correct horse battery" } }); expect(r.status).toBe(200); return cookieFrom(r); }; + const until = async (f: () => Promise) => { for (let i = 0; i < 400; i++) { if (await f()) return; await Bun.sleep(25); } throw new Error("timeout"); }; + try { - // 1. the server is up with every feature and the fixture course - expect(await (await call("GET", "/api/health")).json()).toEqual({ ok: true, courses: 1, db: databaseUrl.startsWith("postgres") ? "postgres" : "sqlite" }); + // 1. the server is up with every feature, the fixture course and the restricted copy + expect(await json("GET", "/api/health")).toEqual({ ok: true, courses: 2, db: databaseUrl.startsWith("postgres") ? "postgres" : "sqlite" }); expect((await call("GET", "/c/sample")).headers.get("content-type")).toContain("text/html"); + expect(s.features.map((f) => f.name)).toEqual(["learner", "import", "studio", "ai"]); // 2. sign up: the first account is the owner const owner = await signUp("ada@example.org", "Ada Lovelace"); - const me = (await (await call("GET", "/api/me", { cookie: owner })).json()) as { user: { id: string; roles: string[] } }; - expect(me.user.roles).toContain("owner"); + const me = (await json("GET", "/api/me", { cookie: owner })) as { user: { id: string; roles: string[] } }; + expect(me.user.roles).toEqual(["owner"]); // 3. mark a lesson done expect((await call("PUT", "/api/courses/sample/progress/m1-intro", { cookie: owner, body: { state: "done" } })).status).toBe(200); - // 4. a slide note on the video lesson's deck + // 4. a slide note on the video lesson's deck, and a bodyless DELETE over the wire const note = await call("POST", "/api/courses/sample/notes", { cookie: owner, body: { lessonId: "m1-video", anchorKind: "slide", anchorValue: 2, deckId: "d1", body: "Slide two matters" } }); expect(note.status).toBe(201); + const scratch = (await json("POST", "/api/courses/sample/notes", { cookie: owner, body: { lessonId: "m1-intro", body: "scratch" } })) as { id: string }; + expect((await call("DELETE", `/api/notes/${scratch.id}`, { cookie: owner })).status).toBe(200); // 5. run the sample exercise with the learner's runner, then save the passing attempt - const lesson = (await (await call("GET", "/api/courses/sample/lessons/m2-ex-add", { cookie: owner })).json()) as { browser: { starter: string; tests: string } }; + const lesson = (await json("GET", "/api/courses/sample/lessons/m2-ex-add", { cookie: owner })) as { browser: { starter: string; tests: string } }; const runner = createBrowserRunner("js"); - const failing = await runner.run(lesson.browser.starter, lesson.browser.tests, 5000); - expect(failing.some((t) => !t.passed)).toBe(true); + expect((await runner.run(lesson.browser.starter, lesson.browser.tests, 5000)).some((t) => !t.passed)).toBe(true); const code = lesson.browser.starter.replace("a - b", "a + b"); const results = await runner.run(code, lesson.browser.tests, 5000); expect(results.length).toBeGreaterThan(0); @@ -63,55 +97,122 @@ async function run(databaseUrl: string) { expect((await call("PUT", "/api/courses/sample/lessons/m2-ex-add/attempt", { cookie: owner, body: { code, passed: true, result: results, track: "js" } })).status).toBe(200); // 6. read progress and notes back - const progress = (await (await call("GET", "/api/courses/sample/progress", { cookie: owner })).json()) as Record; + const progress = (await json("GET", "/api/courses/sample/progress", { cookie: owner })) as Record; expect(progress["m1-intro"]?.state).toBe("done"); expect(progress["m2-ex-add"]?.state).toBe("done"); - const notes = (await (await call("GET", "/api/courses/sample/notes", { cookie: owner })).json()) as { body: string; anchorKind: string; deckId: string }[]; + const notes = (await json("GET", "/api/courses/sample/notes", { cookie: owner })) as { body: string; anchorKind: string; deckId: string }[]; expect(notes.map((n) => [n.body, n.anchorKind, n.deckId])).toEqual([["Slide two matters", "slide", "d1"]]); // 7. a second account is a plain learner: own data only, and the owner/author features refuse it const lea = await signUp("lea@example.org", "Lea"); - expect(await (await call("GET", "/api/courses/sample/progress", { cookie: lea })).json()).toEqual({}); + expect(await json("GET", "/api/courses/sample/progress", { cookie: lea })).toEqual({}); expect((await call("GET", "/api/import/info", { cookie: lea })).status).toBe(403); expect((await call("GET", "/api/studio/courses", { cookie: lea })).status).toBe(403); expect((await call("GET", "/api/ai/admin/config", { cookie: lea })).status).toBe(403); expect((await call("GET", "/api/import/info", { cookie: owner })).status).toBe(200); expect((await call("GET", "/api/studio/courses", { cookie: owner })).status).toBe(200); - expect((await call("GET", "/api/ai/status", { cookie: owner })).status).toBe(200); - // 8. media: Range, nosniff - const clip = await call("GET", "/api/media/sample/media/clip.mp4", { cookie: lea }); - expect(clip.headers.get("x-content-type-options")).toBe("nosniff"); + // 8. media: a real Range request gives 206 with the right bytes; nosniff everywhere + const whole = new Uint8Array(readFileSync(join(SAMPLE, "media/clip.mp4"))); + const part = await call("GET", "/api/media/sample/media/clip.mp4", { cookie: lea, headers: { Range: "bytes=0-9" } }); + expect(part.status).toBe(206); + expect(part.headers.get("content-range")).toBe(`bytes 0-9/${whole.length}`); + expect(new Uint8Array(await part.arrayBuffer())).toEqual(whole.slice(0, 10)); + expect(part.headers.get("x-content-type-options")).toBe("nosniff"); + expect((await call("GET", "/api/media/sample/media/clip.mp4", { headers: { Range: `bytes=${whole.length}-` } })).status).toBe(416); - // 9. the AI draft sink: studio-managed course -> the author's draft branch, as that user; other courses -> files - const studio = studioFeature(s.ctx, { watch: false }); - try { - await studio.store.createCourse({ id: "gen-target", title: "Generated" }, { name: "Ada Lovelace", email: "ada@example.org" }); - const sink = aiDraftSink(s.ctx, studio); - const onStudio = await sink.writeDraftFiles("gen-target", me.user.id, [{ path: "lessons/q.md", content: "# Q\n" }], "AI draft"); - expect(onStudio.branch).toMatch(/^draft\//); - const repo = join(config.studioReposDir, "gen-target.git"); - const author = (await Bun.$`git --git-dir=${repo} log -1 ${"--format=%an <%ae>"} ${onStudio.branch!}`.text()).trim(); - expect(author).toBe("Ada Lovelace "); - const onFiles = await sink.writeDraftFiles("sample", me.user.id, [{ path: "q.md", content: "# Q\n" }], "AI draft"); - expect(onFiles.branch).toBeUndefined(); - expect(onFiles.location!.startsWith(resolve(dataDir, "drafts", "sample"))).toBe(true); - expect(readFileSync(join(onFiles.location!, "q.md"), "utf8")).toBe("# Q\n"); - } finally { - studio.stop(); + // 9. AI on (canned provider), so chat and retrieval can be checked + expect((await call("PUT", "/api/ai/admin/config", { cookie: owner, body: { provider: { kind: "anthropic", apiKey: "k" }, mcpServers: [] } })).status).toBe(200); + + // 10. the restricted course: the owner gets everything; a learner and anonymous users learn nothing + const restricted: [string, string, unknown?][] = [ + ["GET", "/api/courses/secret"], + ["GET", "/api/courses/secret/lessons/m1-intro"], + ["GET", "/api/media/secret/lessons/01-intro.md"], + ["GET", "/api/courses/secret/cover"], + ["GET", "/api/courses/secret/code/m1"], + ["GET", "/api/courses/secret/code/m1/zip"], + ["GET", "/api/courses/secret/notes"], + ["GET", "/api/courses/secret/progress"], + ["PUT", "/api/courses/secret/progress/m1-intro", { state: "done" }], + ["POST", "/api/courses/secret/notes", { lessonId: "m1-intro", body: "x" }], + ["PUT", "/api/courses/secret/lessons/m2-ex-add/attempt", { code: "x", passed: false }], + ["POST", "/api/ai/courses/secret/chat", { message: "What is in this course?" }], + ]; + for (const [method, path, body] of restricted) { + const asOwner = await call(method, path, { cookie: owner, body }); + expect([method, path, asOwner.status >= 200 && asOwner.status < 300]).toEqual([method, path, true]); + await asOwner.arrayBuffer(); + const asLearner = await call(method, path, { cookie: lea, body }); + expect([method, path, asLearner.status]).toEqual([method, path, 404]); + // Anonymous: 404 for content; 401 ("sign in first") on per-user routes, which answer the same for every course id. + const anon = await call(method, path, { body }); + expect([method, path, [401, 404].includes(anon.status)]).toEqual([method, path, true]); + const text = await anon.text(); + expect(text).not.toContain("Hidden Vault"); } + const ids = async (cookie?: string) => ((await json("GET", "/api/library", { cookie })) as { courses: { id: string }[] }).courses.map((c) => c.id).sort(); + expect(await ids(owner)).toEqual(["sample", "secret"]); + expect(await ids(lea)).toEqual(["sample"]); + expect(await ids()).toEqual(["sample"]); + const search = async (cookie?: string) => JSON.stringify(await json("GET", "/api/library/search?q=vault", { cookie })); + expect(await search(owner)).toContain("secret"); + expect(await search(lea)).toBe("[]"); + expect(await search()).toBe("[]"); + // AI retrieval: the course is unknown to the learner's assistant status, known to the owner's + expect((await json("GET", "/api/ai/status?courseId=secret", { cookie: owner })).course?.id).toBe("secret"); + expect((await json("GET", "/api/ai/status?courseId=secret", { cookie: lea })).course).toBeUndefined(); + expect((await json("GET", "/api/ai/status?courseId=secret")).course).toBeUndefined(); + + // 11. cross-site requests (CSRF): the owner's cookie is not enough + const pwned = join(dataDir, "PWNED"); + const mcp = { transport: "stdio", command: "sh", args: ["-c", `touch ${pwned}`] }; + const forged = await call("POST", "/api/ai/admin/mcp/test", { cookie: owner, origin: "http://evil.example", headers: { "content-type": "text/plain" }, body: JSON.stringify(mcp) }); + expect(forged.status).toBe(403); + const forgedJson = await call("POST", "/api/ai/admin/mcp/test", { cookie: owner, origin: "http://evil.example", body: mcp }); + expect(forgedJson.status).toBe(403); + const noOrigin = await call("POST", "/api/ai/admin/mcp/test", { cookie: owner, origin: null, body: mcp }); + expect(noOrigin.status).toBe(403); + await Bun.sleep(200); + expect(existsSync(pwned)).toBe(false); + expect((await call("POST", "/api/courses/sample/notes", { cookie: owner, origin: "http://evil.example", headers: { "content-type": "text/plain" }, body: JSON.stringify({ lessonId: "m1-intro", body: "forged" }) })).status).toBe(403); + expect((await call("POST", "/api/courses/sample/notes", { cookie: owner, origin: null, headers: { referer: `${ORIGIN}/c/sample/m1-intro` }, body: { lessonId: "m1-intro", body: "via referer" } })).status).toBe(201); + expect((await call("POST", "/api/auth/sign-in/email", { origin: "http://evil.example", body: { email: "ada@example.org", password: "correct horse battery" } })).status).toBe(403); + expect((await json("GET", "/api/courses/sample/notes", { cookie: owner })).map((n: { body: string }) => n.body)).not.toContain("forged"); + + // 12. AI drafts through the real wiring: a studio course gets a commit on the author's draft branch, as the + // author; a course the studio does not manage gets files under /drafts + expect((await call("POST", "/api/studio/courses", { cookie: owner, body: { id: "gen-target", title: "Generated" } })).status).toBe(201); + const generate = async (courseId: string, lessonId: string) => { + const r = await call("POST", "/api/ai/studio/generate", { cookie: owner, body: { courseId, kind: "document", style: "pure-snake", topic: "Adding", sources: [{ kind: "lesson", lessonId }] } }); + expect(r.status).toBe(202); + const { jobId } = (await r.json()) as { jobId: string }; + let job: any = null; + await until(async () => { job = await json("GET", `/api/ai/studio/jobs/${jobId}`, { cookie: owner }); return job.status === "done" || job.status === "failed"; }); + expect([job.status, job.error]).toEqual(["done", null]); + return job.result as { branch?: string; location: string; paths: string[] }; + }; + const onStudio = await generate("gen-target", "welcome"); + expect(onStudio.branch).toMatch(/^draft\//); + const repo = join(config.studioReposDir, "gen-target.git"); + expect((await Bun.$`git --git-dir=${repo} log -1 ${"--format=%an <%ae>"} ${onStudio.branch!}`.text()).trim()).toBe("Ada Lovelace "); + expect(await Bun.$`git --git-dir=${repo} show ${`${onStudio.branch}:lessons/generated.md`}`.text()).toContain("# Generated"); + const onFiles = await generate("sample", "m1-intro"); + expect(onFiles.branch).toBeUndefined(); + expect(onFiles.location.startsWith(resolve(dataDir, "drafts", "sample"))).toBe(true); + expect(readFileSync(join(onFiles.location, "lessons/generated.md"), "utf8")).toContain("# Generated"); } finally { await s.stop(); } } describe("end to end", () => { - test("sign up, mark done, slide note, exercise, progress [sqlite]", () => run("sqlite://:memory:"), 60_000); + test("sign up, mark done, slide note, exercise, progress, visibility, CSRF, AI drafts [sqlite]", () => run("sqlite://:memory:"), 90_000); const pg = PG; if (pg) { - test("sign up, mark done, slide note, exercise, progress [postgres]", async () => { + test("sign up, mark done, slide note, exercise, progress, visibility, CSRF, AI drafts [postgres]", async () => { const { url, drop } = await pgSchema(pg); try { await run(url); } finally { await drop(); } - }, 60_000); + }, 90_000); } }); diff --git a/docker-compose.yml b/docker-compose.yml index cc071ed..071b70b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,6 +22,27 @@ services: STC_UPLOAD_MAX_MB: ${STC_UPLOAD_MAX_MB:-100} STC_AI_ALLOWED_BASE_URLS: ${STC_AI_ALLOWED_BASE_URLS:-} STC_TRANSCRIBE_URL: ${STC_TRANSCRIBE_URL:-} + STC_TRANSCRIBE_KEY: ${STC_TRANSCRIBE_KEY:-} + STC_TRANSCRIBE_MODEL: ${STC_TRANSCRIBE_MODEL:-} + STC_ALLOWED_ORIGINS: ${STC_ALLOWED_ORIGINS:-} + # Sign-in providers (unset ones stay off). Social: STC_AUTH__ID/_SECRET. + STC_AUTH_GITHUB_ID: ${STC_AUTH_GITHUB_ID:-} + STC_AUTH_GITHUB_SECRET: ${STC_AUTH_GITHUB_SECRET:-} + STC_AUTH_GOOGLE_ID: ${STC_AUTH_GOOGLE_ID:-} + STC_AUTH_GOOGLE_SECRET: ${STC_AUTH_GOOGLE_SECRET:-} + STC_AUTH_MICROSOFT_ID: ${STC_AUTH_MICROSOFT_ID:-} + STC_AUTH_MICROSOFT_SECRET: ${STC_AUTH_MICROSOFT_SECRET:-} + STC_AUTH_GITLAB_ID: ${STC_AUTH_GITLAB_ID:-} + STC_AUTH_GITLAB_SECRET: ${STC_AUTH_GITLAB_SECRET:-} + STC_AUTH_DISCORD_ID: ${STC_AUTH_DISCORD_ID:-} + STC_AUTH_DISCORD_SECRET: ${STC_AUTH_DISCORD_SECRET:-} + STC_AUTH_OIDC_ISSUER: ${STC_AUTH_OIDC_ISSUER:-} + STC_AUTH_OIDC_ID: ${STC_AUTH_OIDC_ID:-} + STC_AUTH_OIDC_SECRET: ${STC_AUTH_OIDC_SECRET:-} + STC_AUTH_OIDC_NAME: ${STC_AUTH_OIDC_NAME:-} + STC_AUTH_OIDC_SCOPES: ${STC_AUTH_OIDC_SCOPES:-} + STC_AUTH_OIDC_DISCOVERY_URL: ${STC_AUTH_OIDC_DISCOVERY_URL:-} + STC_AUTH_GROUPS_CLAIM: ${STC_AUTH_GROUPS_CLAIM:-} depends_on: postgres: condition: service_healthy diff --git a/docs/operations.md b/docs/operations.md index 64c56da..673de68 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -50,11 +50,26 @@ Back up the `stc-data` volume (and the Postgres volume `stc-pg` when you use Pos | `STC_AI_ALLOWED_BASE_URLS` | unset | `,`-separated origins learners' own AI keys may use besides api.anthropic.com and api.openai.com | | `STC_AUTH__ID`, `_SECRET` | unset | social sign-in | | `STC_AUTH_OIDC_ISSUER`, `_ID`, `_SECRET`, `_NAME`, `_SCOPES`, `_DISCOVERY_URL`, `STC_AUTH_GROUPS_CLAIM` | unset | generic OIDC sign-in; group claims drive restricted visibility | -| `STC_TRUSTED_ORIGINS` | unset | extra `,`-separated origins allowed to call the sign-in API | +| `STC_ALLOWED_ORIGINS` | unset | extra `,`-separated origins (e.g. `http://192.168.1.5:3000`) allowed to send writes and sign in, besides `STC_PUBLIC_URL`'s origin. `STC_TRUSTED_ORIGINS` is read as an alias | Without Docker: `bun install`, then `STC_SECRET=… NODE_ENV=production bun apps/server/src/main.ts`. The importer needs `ffmpeg`, `ffprobe`, `git`, `unzip` and Python with PyMuPDF on the host. +## Cross-site requests and the session cookie + +- Every state-changing request (POST, PUT, PATCH, DELETE), sign-in included, must carry an `Origin` (or, without + one, a `Referer`) equal to `STC_PUBLIC_URL`'s origin or one in `STC_ALLOWED_ORIGINS`. Anything else gets 403. + Open STCourses at exactly that URL. If people reach it by several names (a LAN IP and a hostname), list the others + in `STC_ALLOWED_ORIGINS`. +- A write with a body must be `application/json` (uploads: `multipart/form-data`). HTML forms and `text/plain` + bodies, which another page can send without a CORS preflight, are refused. +- Exempt: the identity-provider callbacks (SAML ACS and SLO, OAuth/OIDC callbacks under `/api/auth`). The provider + posts those from its own origin, and Better Auth verifies the signed assertion or the OAuth state itself. +- The session cookie stays `SameSite=Lax` (Better Auth's default), not `Strict`. OAuth/OIDC sign-in returns from + the provider by a cross-site top-level navigation, and Better Auth needs its state cookie on that request. Under + `Strict` the browser withholds it, and social, OIDC and SAML sign-in break. The Origin check above covers the + same-site cases that `Lax` allows (other ports on the same host, sibling subdomains). + ## Security headers - The web app carries its Content-Security-Policy as a `` tag in `apps/web/index.html` (no inline script). diff --git a/packages/contracts/src/server.ts b/packages/contracts/src/server.ts index cf8377c..9fc427c 100644 --- a/packages/contracts/src/server.ts +++ b/packages/contracts/src/server.ts @@ -9,6 +9,13 @@ export interface Feature { jobs?: Record; // job kind -> handler auth?: AuthService; // replaces the anonymous AuthService (the learner package supplies it) stop?(): void | Promise; // called when the server stops (timers, watchers) + /** Route keys (as in `routes`) whose writes take multipart/form-data instead of JSON (uploads). */ + multipartRoutes?: string[]; + /** + * Path prefixes an outside identity provider posts to (SAML ACS/SLO, OAuth callbacks). Core's cross-site request + * guard skips them; the handler must verify the request itself (signed assertion, OAuth state). + */ + externalCallbacks?: string[]; } /** Features are built after the ServerContext exists. The job queue is built from the union of their `jobs`, so a * factory must not use `ctx.jobs` synchronously (only while a request or job runs). */ @@ -48,4 +55,9 @@ export interface AppConfig { transcribeUrl: string | null; /** STC_PYTHON, default python3: Python with PyMuPDF, used for PDF page images and text. */ python: string; + /** + * Origins allowed to send state-changing requests: the origin of `publicUrl` plus STC_ALLOWED_ORIGINS + * (comma-separated; STC_TRUSTED_ORIGINS is read as an alias). Core refuses writes from any other origin. + */ + allowedOrigins: string[]; } diff --git a/packages/contracts/test/types.test.ts b/packages/contracts/test/types.test.ts index e5cfbf0..a49a1fa 100644 --- a/packages/contracts/test/types.test.ts +++ b/packages/contracts/test/types.test.ts @@ -27,7 +27,7 @@ test("contract interfaces accept well-formed literals", async () => { secret: "s", publicUrl: "http://localhost:3000", importedCoursesDir: "./data/courses", studioReposDir: "./data/studio/repos", studioWorktreesDir: "./data/worktrees", cacheDir: "./data/cache", importRoots: ["./data/import-inbox"], uploadMaxMb: 100, aiAllowedBaseUrls: [], - transcribeUrl: null, python: "python3", + transcribeUrl: null, python: "python3", allowedOrigins: ["http://localhost:3000"], }; const ctx: Pick = { config, jobs: queue, auth }; diff --git a/packages/core/src/app.ts b/packages/core/src/app.ts index 931328c..baae3c3 100644 --- a/packages/core/src/app.ts +++ b/packages/core/src/app.ts @@ -22,6 +22,47 @@ export function canView(user: AuthUser | null, course: LoadedCourse, acl: { prin const json = (body: unknown, status = 200) => Response.json(body, { status }); +const WRITES = new Set(["POST", "PUT", "PATCH", "DELETE"]); + +/** The request's origin from `Origin`, else from `Referer`; null when neither names one ("null" counts as none). */ +function requestOrigin(req: Request): string | null { + const o = req.headers.get("origin"); + if (o && o !== "null") return o.toLowerCase(); + const r = req.headers.get("referer"); + if (r) { try { const u = new URL(r).origin; return u === "null" ? null : u; } catch { return null; } } + return null; +} + +const hasBody = (req: Request) => { + const len = req.headers.get("content-length"); + if (len !== null) return Number(len) > 0; + return req.headers.has("transfer-encoding") || req.body !== null; +}; + +/** + * Cross-site request guard for every state-changing request (POST, PUT, PATCH, DELETE), applied once here for all + * features, Better Auth included: + * - the `Origin` (or, without it, the `Referer`) must be one of `config.allowedOrigins` (publicUrl's origin plus + * STC_ALLOWED_ORIGINS); missing or foreign gives 403; + * - a request with a body must be `application/json`, or `multipart/form-data` on the feature's `multipartRoutes`. + * Forms and text/plain bodies (what a hostile page can send without a preflight) are refused with 403. + * Paths in a feature's `externalCallbacks` (identity-provider posts) are exempt and verify themselves. + * Returns the refusal, or null to proceed. + */ +export function crossSiteRefusal(req: Request, allowedOrigins: string[], opts: { multipart: boolean; external: boolean }): Response | null { + if (!WRITES.has(req.method) || opts.external) return null; + const origin = requestOrigin(req); + if (!origin || !allowedOrigins.includes(origin)) { + return json({ error: "cross-site request refused: the Origin must be this server's public URL (STC_PUBLIC_URL or STC_ALLOWED_ORIGINS)" }, 403); + } + if (hasBody(req)) { + const type = (req.headers.get("content-type") ?? "").split(";")[0]!.trim().toLowerCase(); + const ok = type === "application/json" || (opts.multipart && type === "multipart/form-data"); + if (!ok) return json({ error: opts.multipart ? "send multipart/form-data or application/json" : "send application/json" }, 403); + } + return null; +} + export function createApp(ctx: ServerContext, features: Feature[]): { routes: Routes; fetch(req: Request): Promise } { const acl = aclRepo(ctx.db); @@ -48,19 +89,25 @@ export function createApp(ctx: ServerContext, features: Feature[]): { routes: Ro }, }; - const wrap = (h: RouteHandler) => async (req: Request & { params?: Record }): Promise => { + // No configuration means no allowed origin: every write is refused (fail closed). + const allowed = ctx.config?.allowedOrigins ?? []; + const external = features.flatMap((f) => f.externalCallbacks ?? []); + const wrap = (h: RouteHandler, multipart: boolean) => async (req: Request & { params?: Record }): Promise => { const url = new URL(req.url); + const refused = crossSiteRefusal(req, allowed, { multipart, external: external.some((p) => url.pathname.startsWith(p)) }); + if (refused) return refused; const rc: RequestContext = { user: await ctx.auth.userFromRequest(req), params: req.params ?? {}, url }; return h(req, rc); }; const routes: Routes = {}; for (const f of [core, ...features]) { + const multipart = new Set(f.multipartRoutes ?? []); for (const [path, methods] of Object.entries(f.routes)) { const slot = (routes[path] ??= {}); for (const [m, h] of Object.entries(methods) as [Method, RouteHandler][]) { if (slot[m]) throw new Error(`Duplicate route ${m} ${path} (feature "${f.name}")`); - slot[m] = wrap(h); + slot[m] = wrap(h, multipart.has(path)); } } } diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index aebf8b9..4e5f29f 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -8,6 +8,15 @@ export type ConfigInput = Pick & Partial (raw ?? "").split(":").filter(Boolean); +/** `scheme://host[:port]` of a URL, lower-cased by URL parsing; null when it does not parse. */ +export function originOf(u: string): string | null { + try { + const o = new URL(u).origin; + return o === "null" ? null : o; + } catch { + return null; + } +} const urls = (raw: string | undefined) => (raw ?? "").split(",").map((s) => s.trim()).filter(Boolean); /** @@ -24,13 +33,20 @@ export function resolveConfig(input: ConfigInput): AppConfig { if (!courseDirs.some((c) => resolve(c) === resolve(d))) courseDirs.push(d); } const port = input.port ?? 3000; + const publicUrl = input.publicUrl ?? `http://localhost:${port}`; + const origins: string[] = []; + for (const o of [publicUrl, ...(input.allowedOrigins ?? [])]) { + const n = originOf(o); + if (n && !origins.includes(n)) origins.push(n); + } return { port, databaseUrl: input.databaseUrl ?? "sqlite://./data/stcourses.db", dataDir, courseDirs, secret: input.secret ?? DEV_SECRET, - publicUrl: input.publicUrl ?? `http://localhost:${port}`, + publicUrl, + allowedOrigins: origins, importedCoursesDir, studioReposDir: input.studioReposDir ?? resolve(dataDir, "studio", "repos"), studioWorktreesDir, @@ -71,6 +87,7 @@ export function loadConfig(env: Record = process.env aiAllowedBaseUrls: urls(env.STC_AI_ALLOWED_BASE_URLS), transcribeUrl: env.STC_TRANSCRIBE_URL || null, python: env.STC_PYTHON || "python3", + allowedOrigins: [...urls(env.STC_ALLOWED_ORIGINS), ...urls(env.STC_TRUSTED_ORIGINS)], }); } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 11619c1..80df762 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,5 +1,5 @@ -export { loadConfig, resolveConfig, cachePaths, type ConfigInput } from "./config"; +export { loadConfig, resolveConfig, cachePaths, originOf, type ConfigInput } from "./config"; export { createJobQueue } from "./jobs"; export { serveMedia, contentType, contentDisposition, mediaSecurityHeaders, MEDIA_SANDBOX_CSP } from "./media"; -export { createApp, canView, type Routes } from "./app"; +export { createApp, canView, crossSiteRefusal, type Routes } from "./app"; export { composeServer, type ComposeOptions, type ComposedServer } from "./compose"; diff --git a/packages/core/test/compose.test.ts b/packages/core/test/compose.test.ts index 213877b..c7b2130 100644 --- a/packages/core/test/compose.test.ts +++ b/packages/core/test/compose.test.ts @@ -33,7 +33,7 @@ test("factories get the context; the queue is built from the union of their jobs try { expect(String(early)).toMatch(/not ready/); expect(await (await s.app.fetch(new Request("http://x/api/a"))).json()).toEqual({ who: "u", courses: 1 }); - const { id } = (await (await s.app.fetch(new Request("http://x/api/b", { method: "POST" }))).json()) as { id: string }; + const { id } = (await (await s.app.fetch(new Request("http://x/api/b", { method: "POST", headers: { origin: "http://localhost:3000" } }))).json()) as { id: string }; for (let i = 0; i < 200 && (await s.ctx.jobs.get(id))?.status !== "done"; i++) await Bun.sleep(10); expect((await s.ctx.jobs.get(id))?.status).toBe("done"); expect(seen).toEqual(['a:{"n":1}']); diff --git a/packages/core/test/csrf.test.ts b/packages/core/test/csrf.test.ts new file mode 100644 index 0000000..60b329d --- /dev/null +++ b/packages/core/test/csrf.test.ts @@ -0,0 +1,84 @@ +import { expect, test } from "bun:test"; +import type { AuthUser, Feature, ServerContext } from "@stc/contracts"; +import { createApp, resolveConfig } from "../src"; + +const owner: AuthUser = { id: "o", email: "o@x", name: "O", roles: ["owner"], groups: [] }; +const ran: string[] = []; +const feature: Feature = { + name: "t", + routes: { + "/api/do": { POST: async () => { ran.push("do"); return Response.json({ ok: true }); }, DELETE: async () => { ran.push("del"); return Response.json({ ok: true }); }, GET: () => Response.json({ ok: true }) }, + "/api/up": { POST: async () => { ran.push("up"); return Response.json({ ok: true }); } }, + "/api/auth/*": { POST: async () => { ran.push("auth"); return Response.json({ ok: true }); } }, + }, + multipartRoutes: ["/api/up"], + externalCallbacks: ["/api/auth/sso/saml2/sp/acs"], +}; +const config = resolveConfig({ dataDir: "/tmp/x", publicUrl: "https://courses.example.org", allowedOrigins: ["http://192.168.1.5:3000"] }); +const app = createApp({ config, auth: { userFromRequest: async () => owner } } as unknown as ServerContext, [feature]); +const send = (path: string, init: { method?: string; headers?: Record; body?: BodyInit } = {}) => + app.fetch(new Request(`https://courses.example.org${path}`, { method: init.method ?? "POST", headers: init.headers, body: init.body })); +const JSONH = { "content-type": "application/json" }; + +test("same-origin JSON writes run; GET needs no Origin", async () => { + ran.length = 0; + expect((await send("/api/do", { headers: { origin: "https://courses.example.org", ...JSONH }, body: "{}" })).status).toBe(200); + expect((await send("/api/do", { headers: { origin: "http://192.168.1.5:3000", ...JSONH }, body: "{}" })).status).toBe(200); // STC_ALLOWED_ORIGINS + expect((await send("/api/do", { headers: { referer: "https://courses.example.org/c/x", ...JSONH }, body: "{}" })).status).toBe(200); // Referer fallback + expect((await send("/api/do", { method: "DELETE", headers: { origin: "https://courses.example.org" } })).status).toBe(200); // no body: no type needed + expect((await send("/api/do", { method: "GET" })).status).toBe(200); + expect(ran).toEqual(["do", "do", "do", "del"]); +}); + +test("foreign, missing or opaque Origin is 403 and the handler never runs", async () => { + ran.length = 0; + for (const headers of []>[ + { origin: "http://evil.example", "content-type": "text/plain" }, + { origin: "http://evil.example", ...JSONH }, + { origin: "https://courses.example.org.evil.example", ...JSONH }, + { origin: "http://courses.example.org", ...JSONH }, // scheme differs + { origin: "null", ...JSONH }, + { referer: "http://evil.example/page", ...JSONH }, + { ...JSONH }, // neither Origin nor Referer + ]) { + const r = await send("/api/do", { headers, body: "{}" }); + expect([headers, r.status]).toEqual([headers, 403]); + } + expect((await send("/api/do", { method: "DELETE" })).status).toBe(403); + expect(ran).toEqual([]); +}); + +test("same-origin bodies must be JSON (multipart only on upload routes)", async () => { + ran.length = 0; + const o = { origin: "https://courses.example.org" }; + for (const type of ["text/plain", "application/x-www-form-urlencoded", "multipart/form-data; boundary=x", ""]) { + expect([type, (await send("/api/do", { headers: { ...o, ...(type ? { "content-type": type } : {}) }, body: "a=1" })).status]).toEqual([type, 403]); + } + const fd = new FormData(); + fd.set("file", new File(["x"], "a.txt")); + expect((await send("/api/up", { headers: o, body: fd })).status).toBe(200); + expect((await send("/api/up", { headers: { origin: "http://evil.example" }, body: fd })).status).toBe(403); + expect((await send("/api/up", { headers: { ...o, "content-type": "text/plain" }, body: "x" })).status).toBe(403); + expect(ran).toEqual(["up"]); +}); + +test("Better Auth routes are guarded too, except declared identity-provider callbacks", async () => { + ran.length = 0; + expect((await send("/api/auth/sign-in/email", { headers: { origin: "http://evil.example", ...JSONH }, body: "{}" })).status).toBe(403); + expect((await send("/api/auth/sign-in/email", { headers: { origin: "https://courses.example.org", ...JSONH }, body: "{}" })).status).toBe(200); + // a SAML IdP posts a form from its own origin; Better Auth verifies the signed assertion + expect((await send("/api/auth/sso/saml2/sp/acs/idp1", { headers: { origin: "https://idp.example", "content-type": "application/x-www-form-urlencoded" }, body: "SAMLResponse=x" })).status).toBe(200); + expect(ran).toEqual(["auth", "auth"]); +}); + +test("no configuration means every write is refused", async () => { + const bare = createApp({ auth: { userFromRequest: async () => owner } } as unknown as ServerContext, [feature]); + const r = await bare.fetch(new Request("http://x/api/do", { method: "POST", headers: { origin: "http://x", ...JSONH }, body: "{}" })); + expect(r.status).toBe(403); +}); + +test("loadConfig reads STC_ALLOWED_ORIGINS (and STC_TRUSTED_ORIGINS) as origins", async () => { + const { loadConfig } = await import("../src"); + const c = loadConfig({ STC_SECRET: "s", STC_PUBLIC_URL: "https://Courses.Example.org/app/", STC_ALLOWED_ORIGINS: "http://lan:3000/, junk", STC_TRUSTED_ORIGINS: "https://b.example" }); + expect(c.allowedOrigins).toEqual(["https://courses.example.org", "http://lan:3000", "https://b.example"]); +}); diff --git a/packages/feature-ai/test/feature.test.ts b/packages/feature-ai/test/feature.test.ts index 148ad52..84579dc 100644 --- a/packages/feature-ai/test/feature.test.ts +++ b/packages/feature-ai/test/feature.test.ts @@ -29,7 +29,7 @@ async function setup(db: Db, turns: Parameters[0] = [() => const keys: (string | null)[] = []; const ctx = { db, courses, - config: resolveConfig({ port: 0, databaseUrl: "", dataDir: join(dir, "data"), courseDirs: [], secret: "test-secret", publicUrl: "", ...configOverrides }), + config: resolveConfig({ port: 0, databaseUrl: "", dataDir: join(dir, "data"), courseDirs: [], secret: "test-secret", publicUrl: "http://x", ...configOverrides }), auth: { userFromRequest: async (req: Request) => USERS[req.headers.get("x-user") ?? ""] ?? null }, } as unknown as ServerContext; const feature = aiFeature(ctx, { providerFactory: (_cfg, key) => { keys.push(key); return fake.provider; }, ...extra }); @@ -37,7 +37,7 @@ async function setup(db: Db, turns: Parameters[0] = [() => ctx.jobs = jobs; const app = createApp(ctx, [feature]); const call = async (method: string, path: string, user?: string, body?: unknown) => { - const headers: Record = {}; + const headers: Record = { origin: "http://x" }; if (user) headers["x-user"] = user; if (body !== undefined) headers["content-type"] = "application/json"; return app.fetch(new Request(`http://x${path}`, { method, headers, body: body === undefined ? undefined : JSON.stringify(body) })); diff --git a/packages/feature-import/test/routes.test.ts b/packages/feature-import/test/routes.test.ts index 5fc3994..f55895a 100644 --- a/packages/feature-import/test/routes.test.ts +++ b/packages/feature-import/test/routes.test.ts @@ -17,7 +17,7 @@ async function setup(db: any, env: Record = {}) { const courses = await createCourseRepository([join(dataDir, "courses")]); const ctx = { config: resolveConfig({ - databaseUrl: "", dataDir, courseDirs: [join(dataDir, "courses")], secret: "t", publicUrl: "", + databaseUrl: "", dataDir, courseDirs: [join(dataDir, "courses")], secret: "t", publicUrl: "http://x", python: process.env.STC_PYTHON || "python3", importRoots: "STC_IMPORT_ROOTS" in env ? (env.STC_IMPORT_ROOTS ? [env.STC_IMPORT_ROOTS] : undefined) : [tmpdir()], transcribeUrl: env.STC_TRANSCRIBE_URL ?? null, @@ -31,7 +31,7 @@ async function setup(db: any, env: Record = {}) { const app = createApp(ctx, [feature]); const call = (path: string, init: RequestInit & { as?: string; json?: unknown } = {}) => app.fetch(new Request(`http://x${path}`, { - method: init.method, headers: { ...(init.as ? { "x-user": init.as } : {}), ...(init.json !== undefined ? { "content-type": "application/json" } : {}) }, + method: init.method, headers: { origin: "http://x", ...(init.as ? { "x-user": init.as } : {}), ...(init.json !== undefined ? { "content-type": "application/json" } : {}) }, body: init.json !== undefined ? JSON.stringify(init.json) : undefined, })); return { ctx, queue, call, dataDir }; diff --git a/packages/feature-learner/src/server/auth/index.ts b/packages/feature-learner/src/server/auth/index.ts index 29d028c..9a2fb18 100644 --- a/packages/feature-learner/src/server/auth/index.ts +++ b/packages/feature-learner/src/server/auth/index.ts @@ -1,4 +1,5 @@ import { betterAuth, type BetterAuthOptions } from "better-auth"; +import { APIError } from "better-auth/api"; import { genericOAuth } from "better-auth/plugins/generic-oauth"; import { sso } from "@better-auth/sso"; import type { AuthService, AuthUser, Role, ServerContext } from "@stc/contracts"; @@ -31,20 +32,67 @@ export async function groupsOf(db: Db, userId: string): Promise { return rows.map((r: { group_id: string }) => r.group_id); } +/** A unique or primary-key violation, on SQLite or Postgres: another sign-up claimed the owner key first. */ +export function isUniqueViolation(e: unknown): boolean { + const err = e as { code?: string; errno?: string | number; message?: string } | null; + if (!err) return false; + if (err.code === "23505") return true; // Postgres unique_violation + const text = `${err.code ?? ""} ${err.errno ?? ""} ${err.message ?? ""}`; + return /SQLITE_CONSTRAINT_(PRIMARYKEY|UNIQUE)|UNIQUE constraint failed|duplicate key value/i.test(text); +} + /** - * First user ever to be created becomes the owner. The claim is an INSERT into `learner_meta` - * keyed `owner`, so with concurrent sign-ups exactly one wins (primary-key conflict for the rest). + * First user ever to be created becomes the owner. Atomic and fail-closed: + * - one transaction claims the `owner` key in `learner_meta` only if no owner exists (the primary key settles + * races) and inserts the user's role row; either both land or neither does; + * - only a unique violation (a concurrent sign-up won the claim) means "learner": the role is then inserted in + * its own transaction; + * - any other error is rethrown. The caller fails the sign-up; nobody is silently made a learner while the + * owner key stays free. */ -export async function assignInitialRole(db: Db, userId: string): Promise { - let role: Role = "learner"; +export function assignInitialRole(db: Db, userId: string): Promise { + // Serialised per database handle: Bun's SQLite handle is one connection and refuses a second open transaction. + const prev = roleLocks.get(db) ?? Promise.resolve(); + const run = prev.catch(() => {}).then(() => claimRole(db, userId)); + roleLocks.set(db, run.catch(() => {})); + return run; +} + +const roleLocks = new WeakMap>(); + +async function claimRole(db: Db, userId: string): Promise { try { - await db.sql`INSERT INTO learner_meta (key, value) VALUES ('owner', ${userId})`; - role = "owner"; - } catch { - role = "learner"; + return await db.sql.begin(async (tx) => { + const claimed = await tx` + INSERT INTO learner_meta (key, value) + SELECT 'owner', ${userId} WHERE NOT EXISTS (SELECT 1 FROM learner_meta WHERE key = 'owner') + RETURNING key`; + const role: Role = claimed.length === 1 ? "owner" : "learner"; + await tx`INSERT INTO user_roles (user_id, role) VALUES (${userId}, ${role})`; + return role; + }); + } catch (e) { + if (!isUniqueViolation(e)) throw e; + await db.sql.begin(async (tx) => { await tx`INSERT INTO user_roles (user_id, role) VALUES (${userId}, 'learner')`; }); + return "learner"; } - await db.sql`INSERT INTO user_roles (user_id, role) VALUES (${userId}, ${role})`; - return role; +} + +/** + * Removes a user whose sign-up could not be completed (role assignment failed), so no account without a role + * is left behind and the sign-up can simply be retried. + */ +async function discardUser(db: Db, userId: string): Promise { + // Each step on its own: the failure that brought us here may affect one of these tables. + const steps = [ + () => db.sql`DELETE FROM auth_user WHERE id = ${userId}`, + () => db.sql`DELETE FROM auth_account WHERE user_id = ${userId}`, + () => db.sql`DELETE FROM auth_session WHERE user_id = ${userId}`, + () => db.sql`DELETE FROM user_roles WHERE user_id = ${userId}`, + ]; + const errors: unknown[] = []; + for (const s of steps) { try { await s(); } catch (e) { errors.push(e); } } + if (errors.length === steps.length) throw errors[0]; } export interface LearnerAuth { @@ -110,11 +158,24 @@ export function createAuth(ctx: ServerContext, ready: Promise, env: Env emailAndPassword: { enabled: true, minPasswordLength: 8, autoSignIn: true }, socialProviders, plugins, - trustedOrigins: [config.publicUrl, ...(env.STC_TRUSTED_ORIGINS ?? "").split(",").map((s) => s.trim()).filter(Boolean)], + // The same list core's cross-site guard enforces (publicUrl + STC_ALLOWED_ORIGINS / STC_TRUSTED_ORIGINS). + trustedOrigins: config.allowedOrigins?.length ? config.allowedOrigins : [config.publicUrl], telemetry: { enabled: false }, session: { cookieCache: { enabled: true, maxAge: 60 } }, databaseHooks: { - user: { create: { after: async (user) => { await assignInitialRole(db, user.id); } } }, + user: { + create: { + after: async (user) => { + try { + await assignInitialRole(db, user.id); + } catch (e) { + console.error("[learner] role assignment failed; the sign-up is refused", e); + await discardUser(db, user.id).catch((d) => console.error("[learner] could not remove the half-created user", d)); + throw new APIError("INTERNAL_SERVER_ERROR", { message: "Could not set up the account. Try again." }); + } + }, + }, + }, session: { create: { after: async (session) => { diff --git a/packages/feature-learner/src/server/code.ts b/packages/feature-learner/src/server/code.ts index 27c64a3..35a3a63 100644 --- a/packages/feature-learner/src/server/code.ts +++ b/packages/feature-learner/src/server/code.ts @@ -1,15 +1,16 @@ -import { lstatSync, readdirSync, readFileSync, realpathSync } from "node:fs"; +import { lstatSync, readdirSync, realpathSync } from "node:fs"; import { join, sep } from "node:path"; import type { Feature, RequestContext, ServerContext } from "@stc/contracts"; import { contentDisposition } from "@stc/core"; import type { CodeListing } from "../shared/types"; import { courseFor } from "./access"; import { json, notFound, param } from "./http"; -import { zip } from "./zip"; +import { toStream, zipStream } from "./zip"; const SKIP = new Set([".git", "node_modules", "__pycache__", ".DS_Store", ".venv", "target", "dist", ".idea", ".vscode"]); export const MAX_FILES = 2000; -export const MAX_ZIP_BYTES = 200 * 1024 * 1024; +/** The zip is streamed (bounded memory); this caps the work per download and keeps it under ZIP32's 4 GiB. */ +export const MAX_ZIP_BYTES = 1024 * 1024 * 1024; /** * Regular files under `dirAbs` (relative paths, sorted). Symlinks are followed only when their target is a @@ -69,11 +70,12 @@ export function codeRoutes(ctx: ServerContext): Feature["routes"] { if (!r) return notFound(); const { files } = listFiles(r.rootReal, r.dirAbs, 20_000); const total = files.reduce((s, f) => s + f.size, 0); - if (total > MAX_ZIP_BYTES) return json({ error: "this folder is too large to download as one zip" }, 413); + if (total > MAX_ZIP_BYTES) return json({ error: `this folder is larger than ${MAX_ZIP_BYTES / 1024 / 1024} MB; download its files one by one` }, 413); const folder = r.code.dir.split("/").filter(Boolean).pop() ?? r.mod.id; - const data = zip(files.map((f) => ({ name: `${folder}/${f.path}`, data: new Uint8Array(readFileSync(f.abs)), mtime: lstatSync(f.abs).mtime }))); - return new Response(data, { - headers: { "Content-Type": "application/zip", "Content-Length": String(data.length), "Content-Disposition": contentDisposition(`${folder}.zip`) }, + const entries = files.map((f) => ({ name: `${folder}/${f.path}`, path: f.abs, size: f.size, mtime: lstatSync(f.abs).mtime })); + // Streamed one entry (or one chunk of a large file) at a time; the length is not known up front. + return new Response(toStream(zipStream(entries)), { + headers: { "Content-Type": "application/zip", "Content-Disposition": contentDisposition(`${folder}.zip`), "Cache-Control": "private, no-store" }, }); }, }, diff --git a/packages/feature-learner/src/server/feature.ts b/packages/feature-learner/src/server/feature.ts index 186e158..4e29e8d 100644 --- a/packages/feature-learner/src/server/feature.ts +++ b/packages/feature-learner/src/server/feature.ts @@ -37,5 +37,9 @@ export function learnerFeature(ctx: ServerContext): Feature & { auth: AuthServic ...assetRoutes(), ...runnerFrameRoutes(), }; - return { name: "learner", routes, auth: auth.service }; + return { + name: "learner", routes, auth: auth.service, + // Posted by identity providers (SAML assertions, OAuth/OIDC redirects); Better Auth checks signatures and state. + externalCallbacks: ["/api/auth/sso/saml2/sp/acs", "/api/auth/sso/saml2/sp/slo", "/api/auth/sso/callback", "/api/auth/callback/", "/api/auth/oauth2/callback/"], + }; } diff --git a/packages/feature-learner/src/server/zip.ts b/packages/feature-learner/src/server/zip.ts index d69671b..3f0d282 100644 --- a/packages/feature-learner/src/server/zip.ts +++ b/packages/feature-learner/src/server/zip.ts @@ -68,3 +68,150 @@ export function zip(entries: ZipEntry[]): Uint8Array { for (const part of [...locals, ...centrals, end]) { out.set(part, p); p += part.length; } return out; } + +// ---------- streaming writer ---------- + +const CRC_TABLE = (() => { + const t = new Uint32Array(256); + for (let n = 0; n < 256; n++) { + let c = n; + for (let k = 0; k < 8; k++) c = c & 1 ? 0xedb88320 ^ (c >>> 1) : c >>> 1; + t[n] = c >>> 0; + } + return t; +})(); + +/** Incremental CRC-32 (the same polynomial as Bun.hash.crc32). Start with 0, pass the result back in. */ +export function crc32Update(crc: number, data: Uint8Array): number { + let c = ~crc >>> 0; + for (let i = 0; i < data.length; i++) c = CRC_TABLE[(c ^ data[i]!) & 0xff]! ^ (c >>> 8); + return ~c >>> 0; +} + +export interface ZipFileEntry { name: string; path: string; size: number; mtime?: Date } + +/** Files up to this size are read whole and deflated; larger ones are stored and streamed in chunks. */ +export const SMALL_FILE = 4 * 1024 * 1024; +export const CHUNK = 256 * 1024; +const ZIP32_LIMIT = 0xffffffff; + +/** + * A ZIP written as it is read: one entry at a time, never more than one small file or one chunk of a large file in + * memory, whatever the folder's size. Large entries use a data descriptor (flag bit 3) so their CRC and sizes come + * after the data. No ZIP64: the caller keeps the total under 4 GiB. + */ +export async function* zipStream(entries: ZipFileEntry[], readChunks: (path: string) => AsyncIterable = fileChunks): AsyncGenerator { + const enc = new TextEncoder(); + const centrals: Uint8Array[] = []; + let offset = 0; + for (const e of entries) { + const name = enc.encode(e.name); + const { time, date } = dosTime(e.mtime ?? new Date()); + let crc = 0, csize = 0, usize = 0, method = 0, flags = 0x0800; + const header = (known: boolean) => { + const local = new Uint8Array(30 + name.length); + const lv = new DataView(local.buffer); + lv.setUint32(0, 0x04034b50, true); + lv.setUint16(4, 20, true); + lv.setUint16(6, flags, true); + lv.setUint16(8, method, true); + lv.setUint16(10, time, true); + lv.setUint16(12, date, true); + lv.setUint32(14, known ? crc : 0, true); + lv.setUint32(18, known ? csize : 0, true); + lv.setUint32(22, known ? usize : 0, true); + lv.setUint16(26, name.length, true); + local.set(name, 30); + return local; + }; + const start = offset; + if (e.size <= SMALL_FILE) { + const parts: Uint8Array[] = []; + for await (const c of readChunks(e.path)) parts.push(c); + const data = new Uint8Array(parts.reduce((s, p) => s + p.length, 0)); + let p = 0; + for (const part of parts) { data.set(part, p); p += part.length; } + crc = Bun.hash.crc32(data) >>> 0; + usize = data.length; + const deflated = Bun.deflateSync(data); + const body = deflated.length < data.length ? deflated : data; + method = body === deflated ? 8 : 0; + csize = body.length; + const local = header(true); + yield local; + yield body; + offset += local.length + body.length; + } else { + flags |= 0x0008; // sizes and CRC follow the data + const local = header(false); + yield local; + offset += local.length; + for await (const c of readChunks(e.path)) { + crc = crc32Update(crc, c); + usize += c.length; + yield c; + } + csize = usize; + offset += usize; + const dd = new Uint8Array(16); + const dv = new DataView(dd.buffer); + dv.setUint32(0, 0x08074b50, true); + dv.setUint32(4, crc, true); + dv.setUint32(8, csize, true); + dv.setUint32(12, usize, true); + yield dd; + offset += dd.length; + } + if (offset > ZIP32_LIMIT) throw new Error("zip larger than 4 GiB (no ZIP64)"); + const central = new Uint8Array(46 + name.length); + const cv = new DataView(central.buffer); + cv.setUint32(0, 0x02014b50, true); + cv.setUint16(4, 0x0314, true); + cv.setUint16(6, 20, true); + cv.setUint16(8, flags, true); + cv.setUint16(10, method, true); + cv.setUint16(12, time, true); + cv.setUint16(14, date, true); + cv.setUint32(16, crc, true); + cv.setUint32(20, csize, true); + cv.setUint32(24, usize, true); + cv.setUint16(28, name.length, true); + cv.setUint32(38, (0o100644 << 16) >>> 0, true); + cv.setUint32(42, start, true); + central.set(name, 46); + centrals.push(central); + } + let cdSize = 0; + for (const c of centrals) { yield c; cdSize += c.length; } + const end = new Uint8Array(22); + const ev = new DataView(end.buffer); + ev.setUint32(0, 0x06054b50, true); + ev.setUint16(8, entries.length, true); + ev.setUint16(10, entries.length, true); + ev.setUint32(12, cdSize, true); + ev.setUint32(16, offset, true); + yield end; +} + +/** A file's bytes in chunks of at most CHUNK, read one at a time. */ +export async function* fileChunks(path: string): AsyncGenerator { + const f = Bun.file(path); + const size = f.size; + for (let pos = 0; pos < size; pos += CHUNK) yield new Uint8Array(await f.slice(pos, Math.min(size, pos + CHUNK)).arrayBuffer()); +} + +/** Wraps a chunk generator as a pull-based stream: the next chunk is produced only when the client takes one. */ +export function toStream(gen: AsyncGenerator): ReadableStream { + return new ReadableStream({ + async pull(controller) { + try { + const { done, value } = await gen.next(); + if (done) controller.close(); + else controller.enqueue(value); + } catch (e) { + controller.error(e); + } + }, + async cancel() { await gen.return(undefined); }, + }, { highWaterMark: 1 }); +} diff --git a/packages/feature-learner/test/auth.test.ts b/packages/feature-learner/test/auth.test.ts index 6df41bc..aa6053d 100644 --- a/packages/feature-learner/test/auth.test.ts +++ b/packages/feature-learner/test/auth.test.ts @@ -1,5 +1,5 @@ -import { expect, test } from "bun:test"; -import { assignInitialRole } from "../src/server/auth"; +import { expect, spyOn, test } from "bun:test"; +import { assignInitialRole, isUniqueViolation } from "../src/server/auth"; import { syncClaimGroups, groupsFromClaims } from "../src/server/auth/groups"; import { forEachDb } from "./helpers"; import { cookieFrom, makeApp } from "./app"; @@ -60,3 +60,43 @@ test("groupsFromClaims reads arrays and strings", () => { expect(groupsFromClaims({ extraFields: { groups: ["z"] } })).toEqual(["z"]); expect(groupsFromClaims(null)).toEqual([]); }); + +forEachDb("owner bootstrap fails closed: an injected failure refuses the sign-up, no silent learner, no free owner slot", async (db) => { + const t = await makeApp(db); + const count = async (q: string) => Number((await db.sql.unsafe(q))[0].n); + const err = spyOn(console, "error").mockImplementation(() => {}); + try { + // The role table is unreachable: role assignment fails with a non-unique error. + await db.sql.unsafe("ALTER TABLE user_roles RENAME TO user_roles_off"); + const signUp = (email: string) => t.call("POST", "/api/auth/sign-up/email", { body: { email, name: "X", password: "correct horse battery" } }); + const first = await signUp("first@example.com"); + expect(first.status).toBe(500); + // nothing is left: no account, no owner claim + expect(await count("SELECT COUNT(*) AS n FROM auth_user")).toBe(0); + expect(await count("SELECT COUNT(*) AS n FROM learner_meta WHERE key = 'owner'")).toBe(0); + // while the failure lasts, the next sign-up does not become owner either: it fails the same way + expect((await signUp("stranger@example.com")).status).toBe(500); + expect(await count("SELECT COUNT(*) AS n FROM auth_user")).toBe(0); + expect(await count("SELECT COUNT(*) AS n FROM learner_meta WHERE key = 'owner'")).toBe(0); + // once the database recovers, the first completed account is the owner, the next a learner + await db.sql.unsafe("ALTER TABLE user_roles_off RENAME TO user_roles"); + const own = await t.signUp("first@example.com"); + const lea = await t.signUp("stranger@example.com"); + expect((await (await t.call("GET", "/api/me", { cookie: own })).json()).user.roles).toEqual(["owner"]); + expect((await (await t.call("GET", "/api/me", { cookie: lea })).json()).user.roles).toEqual(["learner"]); + } finally { + err.mockRestore(); + } +}, 30_000); + +forEachDb("assignInitialRole rethrows anything but a unique violation, and rolls the owner claim back", async (db) => { + await db.sql.unsafe("ALTER TABLE user_roles RENAME TO user_roles_off"); + await expect(assignInitialRole(db, "u1")).rejects.toThrow(); + expect((await db.sql`SELECT COUNT(*) AS n FROM learner_meta WHERE key = 'owner'`)[0].n.toString()).toBe("0"); + await db.sql.unsafe("ALTER TABLE user_roles_off RENAME TO user_roles"); + expect(await assignInitialRole(db, "u1")).toBe("owner"); + expect(await assignInitialRole(db, "u2")).toBe("learner"); + expect(isUniqueViolation({ code: "23505" })).toBe(true); + expect(isUniqueViolation(new Error("UNIQUE constraint failed: learner_meta.key"))).toBe(true); + expect(isUniqueViolation(new Error("no such table: user_roles"))).toBe(false); +}); diff --git a/packages/feature-learner/test/zip-stream.test.ts b/packages/feature-learner/test/zip-stream.test.ts new file mode 100644 index 0000000..40851d8 --- /dev/null +++ b/packages/feature-learner/test/zip-stream.test.ts @@ -0,0 +1,79 @@ +import { expect, test } from "bun:test"; +import { closeSync, mkdirSync, mkdtempSync, openSync, writeFileSync, writeSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { CHUNK, SMALL_FILE, crc32Update, toStream, zipStream } from "../src/server/zip"; +import { forEachDb } from "./helpers"; +import { FIXTURES, makeApp } from "./app"; + +const rnd = (n: number) => { const b = new Uint8Array(n); for (let i = 0; i < n; i += 65536) crypto.getRandomValues(b.subarray(i, Math.min(n, i + 65536))); return b; }; + +test("incremental CRC-32 matches Bun.hash.crc32", () => { + const data = rnd(300_000); + let c = 0; + for (let i = 0; i < data.length; i += 7777) c = crc32Update(c, data.subarray(i, i + 7777)); + expect(c).toBe(Bun.hash.crc32(data) >>> 0); +}); + +test("the stream holds at most one chunk of a large file; unzip verifies every CRC", async () => { + const dir = mkdtempSync(join(tmpdir(), "stc-zs-")); + mkdirSync(join(dir, "Ünï sub")); + writeFileSync(join(dir, "big.bin"), rnd(SMALL_FILE * 3 + 12345)); // streamed, stored, data descriptor + writeFileSync(join(dir, "Ünï sub/small.txt"), "hello ".repeat(10_000)); // deflated + writeFileSync(join(dir, "empty.txt"), ""); + const entries = ["big.bin", "Ünï sub/small.txt", "empty.txt"].map((p) => ({ name: `pkg/${p}`, path: join(dir, p), size: Bun.file(join(dir, p)).size })); + const out = join(dir, "out.zip"); + const fd = openSync(out, "w"); + let largest = 0; + for await (const chunk of zipStream(entries)) { largest = Math.max(largest, chunk.length); writeSync(fd, chunk); } + closeSync(fd); + expect(largest).toBeLessThanOrEqual(Math.max(CHUNK, 60_000)); // never the whole 12 MB file + const t = await Bun.$`unzip -t ${out}`.nothrow().text(); + expect(t).toContain("No errors detected"); + const x = join(dir, "x"); + await Bun.$`unzip -q ${out} -d ${x}`; + expect(Bun.hash.crc32(new Uint8Array(await Bun.file(join(x, "pkg/big.bin")).arrayBuffer()))).toBe(Bun.hash.crc32(new Uint8Array(await Bun.file(join(dir, "big.bin")).arrayBuffer()))); + expect(await Bun.file(join(x, "pkg/Ünï sub/small.txt")).text()).toBe("hello ".repeat(10_000)); +}); + +test("toStream pulls one chunk at a time", async () => { + let produced = 0; + async function* gen() { for (let i = 0; i < 100; i++) { produced++; yield new Uint8Array(10); } } + const reader = toStream(gen()).getReader(); + await reader.read(); + await Bun.sleep(20); + expect(produced).toBeLessThanOrEqual(3); // not run ahead to 100 + await reader.cancel(); +}); + +forEachDb("a 300 MB code folder downloads as a valid streamed zip without buffering it", async (db) => { + const root = mkdtempSync(join(tmpdir(), "stc-bigcode-")); + mkdirSync(join(root, "code"), { recursive: true }); + // sparse file: no disk or memory cost to create, 300 MB to read + const f = openSync(join(root, "code/huge.bin"), "w"); + writeSync(f, new Uint8Array([1]), 0, 1, 300 * 1024 * 1024 - 1); + closeSync(f); + writeFileSync(join(root, "code/readme.md"), "# hi\n"); + writeFileSync(join(root, "stcourse.yaml"), `schema: 1\nid: bigcode\ntitle: Big\nvisibility: public\nmodules:\n - id: m1\n title: M\n code: { dir: code }\n lessons: [{ id: l1, title: L, type: text, file: code/readme.md }]\n`); + const t = await makeApp(db, [FIXTURES, root]); + Bun.gc(true); + const before = process.memoryUsage().rss; + let peak = before; + const r = await t.call("GET", "/api/courses/bigcode/code/m1/zip"); + expect(r.status).toBe(200); + expect(r.headers.get("content-length")).toBeNull(); // streamed + const out = join(root, "out.zip"); + const w = Bun.file(out).writer(); + const reader = r.body!.getReader(); + let n = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + w.write(value); + if (++n % 64 === 0) { await w.flush(); peak = Math.max(peak, process.memoryUsage().rss); } + } + await w.end(); + expect(Bun.file(out).size).toBeGreaterThan(300 * 1024 * 1024); + expect(peak - before).toBeLessThan(150 * 1024 * 1024); // a buffered zip would need > 300 MB + expect(await Bun.$`unzip -t ${out}`.nothrow().text()).toContain("No errors detected"); +}, 120_000); diff --git a/packages/feature-studio/src/server/feature.ts b/packages/feature-studio/src/server/feature.ts index 7a321ae..9d1d4ca 100644 --- a/packages/feature-studio/src/server/feature.ts +++ b/packages/feature-studio/src/server/feature.ts @@ -364,5 +364,5 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { watcher.start(); } - return { name: "studio", routes, store, watcher, draftWriter, stop: () => watcher.stop() }; + return { name: "studio", routes, multipartRoutes: ["/api/studio/courses/:courseId/upload"], store, watcher, draftWriter, stop: () => watcher.stop() }; } diff --git a/packages/feature-studio/test/app-helpers.ts b/packages/feature-studio/test/app-helpers.ts index 592bbca..ea53c32 100644 --- a/packages/feature-studio/test/app-helpers.ts +++ b/packages/feature-studio/test/app-helpers.ts @@ -40,7 +40,7 @@ export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite:/ const feature = studioFeature(ctx, { watch: false }); const app = createApp(ctx, [feature]); const call = async (as: string | null, method: string, path: string, body?: unknown) => { - const headers: Record = {}; + const headers: Record = { origin: "http://x" }; if (as) headers["x-user"] = as; let payload: BodyInit | undefined; if (body instanceof FormData) payload = body; diff --git a/packages/feature-studio/test/security.test.ts b/packages/feature-studio/test/security.test.ts index 9bb9061..e9b3690 100644 --- a/packages/feature-studio/test/security.test.ts +++ b/packages/feature-studio/test/security.test.ts @@ -202,7 +202,7 @@ describe("I2: uploads", () => { const ct = new Response(fd).headers.get("content-type")!; const bytes = new Uint8Array(await new Response(fd).arrayBuffer()); const stream = new ReadableStream({ start(c) { for (let i = 0; i < bytes.length; i += 65536) c.enqueue(bytes.slice(i, i + 65536)); c.close(); } }); - const r = await env.app.fetch(new Request("http://x/api/studio/courses/c1/upload", { method: "POST", headers: { "x-user": "ada", "content-type": ct }, body: stream, duplex: "half" } as RequestInit)); + const r = await env.app.fetch(new Request("http://x/api/studio/courses/c1/upload", { method: "POST", headers: { "x-user": "ada", origin: "http://x", "content-type": ct }, body: stream, duplex: "half" } as RequestInit)); expect(r.status).toBe(413); expect(existsSync(join(env.dataDir, "worktrees/c1/drafts/u-ada/big.zip"))).toBe(false); expect((await upload("small.zip", new Uint8Array(1000))).status).toBe(200);