From c7b9e1f98eb24b4043f59bf7d20d8b8e2ac60c55 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 05:20:43 +0100 Subject: [PATCH] fix(learner): review round 1: sanitize-html, sandboxed runner frame, one visibility rule, no Tab capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1: lesson HTML goes through a sanitize-html allowlist on the fully rendered output (no on* attributes, no style, no svg/math/iframe/object/embed/form; URLs limited to http(s), mailto, relative and anchors). Raw tags outside the allowlist are escaped as text. The SPA document is served with a CSP (script-src 'self', no inline script) through spaWithCsp in main.ts; contract request §5 asks core to own it. C2: exercises run in ", + ].join("\n\n"); + const before = hits.length; + const html = await chrome.eval(`stcRenderLesson(${JSON.stringify(md)}, ${JSON.stringify(" { for (const a of document.querySelectorAll("#root a, #root summary")) { try { a.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true })); } catch (_) {} } })()`); + await Bun.sleep(800); + expect(await chrome.eval("window.__xss ?? null")).toBeNull(); + expect(chrome.dialogs).toEqual([]); + expect(hits.slice(before).filter((h) => h.path === "/xss")).toEqual([]); + expect(html).toContain("Next paragraph"); + expect(html).not.toMatch(/<[a-z][^>]*\son[a-z]+=/i); + }, 30_000); +}); diff --git a/packages/feature-learner/test/chrome.ts b/packages/feature-learner/test/chrome.ts new file mode 100644 index 0000000..f9dd787 --- /dev/null +++ b/packages/feature-learner/test/chrome.ts @@ -0,0 +1,68 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/** Path of a usable Chrome, or null (the browser tests are skipped without one). */ +export const chromePath = Bun.which("google-chrome") ?? Bun.which("chromium") ?? Bun.which("chromium-browser"); + +/** A minimal Chrome DevTools Protocol driver for headless end-to-end checks. */ +export class Chrome { + private ws!: WebSocket; + private id = 0; + private pending = new Map void>(); + readonly console: string[] = []; + readonly dialogs: string[] = []; + private proc!: Bun.Subprocess; + private dir!: string; + + static async launch(): Promise { + const c = new Chrome(); + c.dir = mkdtempSync(join(tmpdir(), "stc-chrome-")); + const port = 9400 + Math.floor(Math.random() * 500); + c.proc = Bun.spawn([chromePath!, "--headless=new", "--no-sandbox", "--disable-gpu", "--no-first-run", `--remote-debugging-port=${port}`, `--user-data-dir=${c.dir}`, "about:blank"], { stdout: "ignore", stderr: "ignore" }); + let url: string | null = null; + for (let i = 0; i < 100 && !url; i++) { + await Bun.sleep(100); + try { + const list = (await (await fetch(`http://127.0.0.1:${port}/json`)).json()) as { type: string; webSocketDebuggerUrl: string }[]; + url = list.find((t) => t.type === "page")?.webSocketDebuggerUrl ?? null; + } catch { /* not up yet */ } + } + if (!url) throw new Error("chrome did not start"); + c.ws = new WebSocket(url); + await new Promise((r) => c.ws.addEventListener("open", r, { once: true })); + c.ws.addEventListener("message", (ev) => { + const m = JSON.parse(String(ev.data)); + if (m.id && c.pending.has(m.id)) { c.pending.get(m.id)!(m); c.pending.delete(m.id); } + if (m.method === "Runtime.consoleAPICalled") c.console.push(`${m.params.type}: ${m.params.args.map((a: any) => a.value ?? a.description).join(" ")}`); + if (m.method === "Runtime.exceptionThrown") c.console.push(`exception: ${m.params.exceptionDetails.exception?.description ?? m.params.exceptionDetails.text}`); + if (m.method === "Page.javascriptDialogOpening") { c.dialogs.push(m.params.message); void c.send("Page.handleJavaScriptDialog", { accept: true }); } + }); + await c.send("Page.enable"); + await c.send("Runtime.enable"); + return c; + } + + send(method: string, params: object = {}): Promise<{ result?: any; error?: any }> { + return new Promise((r) => { const i = ++this.id; this.pending.set(i, r); this.ws.send(JSON.stringify({ id: i, method, params })); }); + } + + async goto(url: string, settleMs = 1500) { + await this.send("Page.navigate", { url }); + await Bun.sleep(settleMs); + } + + /** Evaluates `js` in the page (awaiting promises) and returns the JSON value. */ + async eval(js: string): Promise { + const r = await this.send("Runtime.evaluate", { expression: js, awaitPromise: true, returnByValue: true }); + if (r.result?.exceptionDetails) throw new Error(r.result.exceptionDetails.exception?.description ?? "evaluation failed"); + return r.result?.result?.value as T; + } + + async close() { + try { this.ws.close(); } catch { /* closed */ } + this.proc.kill(); + await this.proc.exited; + rmSync(this.dir, { recursive: true, force: true }); + } +} diff --git a/packages/feature-learner/test/e2e/harness.tsx b/packages/feature-learner/test/e2e/harness.tsx new file mode 100644 index 0000000..d44ff19 --- /dev/null +++ b/packages/feature-learner/test/e2e/harness.tsx @@ -0,0 +1,39 @@ +// Browser harness for the headless-Chrome tests: the real runner and the real LessonView, bundled for the browser. +import { createRoot } from "react-dom/client"; +import { createBrowserRunner } from "../../src/web/runner/runner"; +import { WORKER_SOURCE } from "../../src/web/runner/worker-source"; +import { LessonView } from "../../src/web/lesson/LessonView"; +import type { LessonPayload } from "../../src/shared/types"; + +declare global { + interface Window { + stcRun(code: string, tests: string, timeoutMs: number): Promise; + /** Negative control only: the old design, a same-origin blob worker in the page. */ + stcRunPlain(code: string, tests: string, timeoutMs: number): Promise; + stcRenderLesson(markdown: string, hintBody: string): Promise; + stcReady: boolean; + } +} + +window.stcRun = (code, tests, timeoutMs) => createBrowserRunner("js").run(code, tests, timeoutMs); +window.stcRunPlain = (code, tests, timeoutMs) => createBrowserRunner("js", { + createWorker: () => new Worker(URL.createObjectURL(new Blob([WORKER_SOURCE], { type: "text/javascript" })), { type: "module" }), +}).run(code, tests, timeoutMs); + +window.stcRenderLesson = async (markdown, hintBody) => { + const lesson: LessonPayload = { + courseId: "c", courseTitle: "C", lesson: { id: "x", title: "Exploit", type: "exercise", minutes: null }, + module: { id: "m", title: "M", index: 0 }, locked: false, markdown, baseFile: "course/x.md", trackMarkdown: {}, tracks: [], + video: null, transcript: null, hints: [{ label: "nudge", body: hintBody }], decks: [], code: null, attachments: [], browser: null, + localCommand: null, prev: null, next: null, lessonFiles: {}, + }; + const el = document.getElementById("root")!; + createRoot(el).render(); + await new Promise((r) => setTimeout(r, 300)); + // Open every fold so nested content is live in the DOM too. + for (const d of document.querySelectorAll("details")) d.open = true; + await new Promise((r) => setTimeout(r, 500)); + return el.innerHTML; +}; + +window.stcReady = true; diff --git a/packages/feature-learner/test/lesson-view.test.tsx b/packages/feature-learner/test/lesson-view.test.tsx index 60f8478..fbd89f6 100644 --- a/packages/feature-learner/test/lesson-view.test.tsx +++ b/packages/feature-learner/test/lesson-view.test.tsx @@ -106,3 +106,12 @@ test("helpers: local command per track, note timestamps, code tree", () => { expect(t.dirs.get("src")!.files.map((f) => f.path)).toEqual(["src/b.ts"]); expect(t.dirs.get("src")!.dirs.get("x")!.files[0]!.path).toBe("src/x/c.ts"); }); + +test("the exercise editor: no Tab capture, Enter keeps the indentation", async () => { + const { newlineWithIndent } = await import("../src/web/lesson/Exercise"); + expect(newlineWithIndent("function f() {\n return 1", 26)).toBe("\n "); + expect(newlineWithIndent("x", 1)).toBe("\n"); + expect(newlineWithIndent("\t\tx", 3)).toBe("\n\t\t"); + const src = await Bun.file(new URL("../src/web/lesson/Exercise.tsx", import.meta.url)).text(); + expect(src).not.toMatch(/e\.key === "Tab"/); +}); diff --git a/packages/feature-learner/test/markdown.test.ts b/packages/feature-learner/test/markdown.test.ts index 604b444..a3b3630 100644 --- a/packages/feature-learner/test/markdown.test.ts +++ b/packages/feature-learner/test/markdown.test.ts @@ -100,11 +100,12 @@ test("systemic-design hint numbering forms one group", () => { }); test("raw HTML is reduced to safe tags", () => { - expect(sanitizeHtml('x
')).toBe("<script>alert(1)</script>x<img src=x onerror=alert(1)>
"); + expect(sanitizeHtml('x
')).toBe('<script>alert(1)</script>x
'); const html = renderLessonHtml('
hi
\n\n[x](javascript:alert(1)) s', opts); - expect(html).not.toContain("hi"); + expect(html).not.toContain("onclick"); expect(html).not.toContain("javascript:"); - expect(html).not.toContain(' { @@ -114,7 +115,7 @@ test("links and images resolve inside the course", () => { expect(resolveHref("https://example.com", opts, "link")).toEqual({ url: "https://example.com", external: true }); expect(resolveHref("img/a.png", opts, "image")).toEqual({ url: "/api/media/pure-snake/course/img/a.png", external: false }); const html = renderLessonHtml("![board](img/board.png) and [ext](https://x.org)", opts); - expect(html).toContain('board'); + expect(html).toContain('board'); expect(html).toContain('target="_blank" rel="noopener noreferrer"'); }); diff --git a/packages/feature-learner/test/spa.test.ts b/packages/feature-learner/test/spa.test.ts new file mode 100644 index 0000000..7e78e56 --- /dev/null +++ b/packages/feature-learner/test/spa.test.ts @@ -0,0 +1,65 @@ +import { expect, test } from "bun:test"; +import { inlineScriptHashes, spaCsp, spaWithCsp } from "../src/server/spa"; +import { RUNNER_FRAME_CSP, RUNNER_FRAME_PATH } from "../src/server/runnerFrame"; +import { connect } from "@stc/db"; +import { runMigrations } from "../src/server/migrate"; +import { makeApp } from "./app"; + +const scriptSrc = (csp: string) => csp.split(";").map((d) => d.trim()).find((d) => d.startsWith("script-src"))!; + +test("the SPA CSP allows scripts from 'self' only, never inline", () => { + const s = scriptSrc(spaCsp()); + expect(s).toBe("script-src 'self'"); + expect(spaCsp()).toContain("object-src 'none'"); + expect(spaCsp()).toContain("frame-ancestors 'self'"); + expect(inlineScriptHashes('')).toEqual([ + `sha256-${new Bun.CryptoHasher("sha256").update("x()").digest("base64")}`, + ]); +}); + +// A stand-in for the bundled index.html (bundling inside bun test is unreliable); the real bundle is checked +// against the dev server (see wp-learner-report.md, fix round 1). +const DOC = '
'; +const upstream = (req: Request) => new URL(req.url).pathname.endsWith(".js") + ? new Response("console.log(1)", { headers: { "Content-Type": "text/javascript" } }) + : new Response(DOC, { headers: { "Content-Type": "text/html;charset=utf-8" } }); + +for (const development of [false, true]) { + test(`the SPA document is served with the CSP header (development: ${development})`, async () => { + const spa = spaWithCsp(upstream, { development }); + const server = Bun.serve({ hostname: "127.0.0.1", port: 0, routes: { "/api/x": () => new Response("api"), ...spa.routes } }); + try { + const r = await fetch(`http://127.0.0.1:${server.port}/c/sample/m1-intro`); + expect(r.status).toBe(200); + const csp = r.headers.get("content-security-policy")!; + expect(await r.text()).toBe(DOC); + const s = scriptSrc(csp); + expect(s).not.toContain("unsafe-inline"); + expect(s).not.toContain("unsafe-eval"); + // Production never allows inline script; development allows only the exact injected helper, by hash. + expect(s).toBe(development ? `script-src 'self' '${inlineScriptHashes(DOC)[0]}'` : "script-src 'self'"); + const asset = await fetch(`http://127.0.0.1:${server.port}/_bun/client/app.js`); + expect(asset.status).toBe(200); + expect(asset.headers.get("content-security-policy")).toBeNull(); + expect(await (await fetch(`http://127.0.0.1:${server.port}/api/x`)).text()).toBe("api"); + } finally { + server.stop(true); + spa.stop(); + } + }); +} + +test("the runner frame is served sandboxed, with no connections and only its own script", async () => { + const db = await connect("sqlite://:memory:"); + await runMigrations(db); + const { call } = await makeApp(db); + const r = await call("GET", RUNNER_FRAME_PATH); + expect(r.status).toBe(200); + const csp = r.headers.get("content-security-policy")!; + expect(csp).toBe(RUNNER_FRAME_CSP); + expect(csp).toContain("connect-src 'none'"); + expect(csp).toContain("sandbox allow-scripts"); + expect(csp).not.toContain("allow-same-origin"); + expect(scriptSrc(csp)).toMatch(/^script-src 'sha256-[A-Za-z0-9+/=]+' blob:$/); + await db.close(); +}); diff --git a/packages/feature-learner/test/visibility.test.ts b/packages/feature-learner/test/visibility.test.ts new file mode 100644 index 0000000..1d9ec5f --- /dev/null +++ b/packages/feature-learner/test/visibility.test.ts @@ -0,0 +1,27 @@ +import { expect } from "bun:test"; +import { forEachDb } from "./helpers"; +import { FIXTURES, makeApp } from "./app"; +import { extraCourses } from "./courses"; + +forEachDb("one visibility rule: the global author role alone does not open a restricted course (I1)", async (db) => { + const dir = await extraCourses(); + const { call, signUp } = await makeApp(db, [FIXTURES, dir]); + const owner = await signUp("owner@x.test"); + const author = await signUp("author@x.test"); + const coAuthor = await signUp("co@x.test"); + const users = (await (await call("GET", "/api/admin/users", { cookie: owner })).json()) as { id: string; email: string }[]; + const id = (e: string) => users.find((u) => u.email === e)!.id; + expect((await call("PUT", `/api/admin/users/${id("author@x.test")}/roles`, { cookie: owner, body: { roles: ["author"] } })).status).toBe(200); + expect((await call("PUT", "/api/admin/courses/secret/access", { cookie: owner, body: { authors: [id("co@x.test")] } })).status).toBe(200); + + const paths = ["/api/courses/secret", "/api/courses/secret/lessons/s1", "/api/media/secret/a.md"]; + for (const p of paths) { + expect([p, (await call("GET", p, { cookie: author })).status]).toEqual([p, 404]); + expect([p, (await call("GET", p, { cookie: coAuthor })).status]).toEqual([p, 200]); + } + const lib = async (c: string) => ((await (await call("GET", "/api/library", { cookie: c })).json()) as { courses: { id: string }[] }).courses.map((x) => x.id); + expect(await lib(author)).not.toContain("secret"); + expect(await lib(coAuthor)).toContain("secret"); + const search = (await (await call("GET", "/api/library/search?q=hidden", { cookie: author })).json()) as unknown[]; + expect(search).toEqual([]); +}); diff --git a/packages/feature-learner/test/xss.test.ts b/packages/feature-learner/test/xss.test.ts new file mode 100644 index 0000000..eaa5727 --- /dev/null +++ b/packages/feature-learner/test/xss.test.ts @@ -0,0 +1,84 @@ +import { expect, test } from "bun:test"; +import { renderHintsHtml, renderLessonHtml, sanitizeHtml } from "../src/web/markdown"; + +const opts = { courseId: "c", baseFile: "course/l.md", tracks: [], lessonFiles: {} }; + +const decode = (s: string) => s.replace(/&#x([0-9a-f]+);?/gi, (_, h) => String.fromCodePoint(parseInt(h, 16))) + .replace(/&#(\d+);?/g, (_, d) => String.fromCodePoint(Number(d))).replace(/:/gi, ":").replace(/&tab;/gi, "\t").replace(/&newline;/gi, "\n").replace(/&/g, "&"); + +/** Fails when real tags in `html` carry event handlers, style, a dangerous URL, or are script-capable elements. */ +function expectSafe(html: string) { + const tags = html.match(/<[a-zA-Z][^>]*>/g) ?? []; + for (const t of tags) { + expect(t).not.toMatch(/\son[a-z]+\s*=/i); + expect(t).not.toMatch(/\sstyle\s*=/i); + expect(t).not.toMatch(/^<(script|style|svg|math|iframe|object|embed|form|input|button|textarea|select|link|meta|base|frame|frameset)\b/i); + for (const m of t.matchAll(/\s(?:href|src|action|formaction|xlink:href)\s*=\s*"([^"]*)"/gi)) { + const url = decode(m[1]!).replace(/[\u0000- \u007f-\u009f]/g, "").toLowerCase(); + expect(url).not.toMatch(/^(javascript|vbscript|data|file):/); + } + } + // An unterminated tag must never swallow following markup into its attributes. + expect(html).not.toMatch(/]*

or

", () => { + for (const wrapper of ["details", "div"]) { + const md = `<${wrapper}>\nThink first\n { + for (const md of [ + "bold", + "
a\n\n", + "ok'); +}); + +test("
keeps the fold but drops the handler", () => { + const html = renderLessonHtml('
Hint 1 — nudge\n\ntext\n\n
', opts); + expectSafe(html); + expect(html).toContain('
", body: " { + const html = renderLessonHtml("A value of type IO or Stream.", opts); + expect(html).toContain("IO<A>"); + expect(html).toContain("Stream<Action>"); +});