fix(learner): review round 1: sanitize-html, sandboxed runner frame, one visibility rule, no Tab capture
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 <iframe sandbox="allow-scripts"> (opaque origin, no cookies) loaded from /api/learner/runner-frame, whose CSP allows only its own script (by hash) and blob: workers, and connect-src 'none'. postMessage only, both sides check event.source; the timeout removes the frame. The worker stubs now also cover Worker, SharedWorker and caches. I1: learner visibility calls core canView with per-course authorship only, like /api/media (contract request §6). I2: Tab and Shift+Tab move focus; Enter keeps the previous line's indentation. Tests: XSS unit cases, headless-Chrome checks for the exploit lesson and for fetch, import, Cache.add, nested Worker, img, EventSource, WebSocket, XHR and sendBeacon from learner and test code (plus a negative control), visibility on both dialects, SPA and runner-frame CSP headers. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
061ae7a2d9
commit
c7b9e1f98e
@@ -28,10 +28,14 @@ export function canEdit(user: AuthUser | null, courseId: string, idx: AccessInde
|
||||
return !!user && (user.roles.includes("owner") || user.roles.includes("author") || idx.authorOf.has(courseId));
|
||||
}
|
||||
|
||||
/** Learners see courses with a manifest that `canView` allows; owners also see broken ones. */
|
||||
/**
|
||||
* Learners see courses with a manifest that core `canView` allows; owners also see broken ones. The author input
|
||||
* is per-course authorship only, the same as core's media route, so every route applies one rule. (The global
|
||||
* `author` role grants viewing only once Phase 2 extends `canView`; contract request learner §6.)
|
||||
*/
|
||||
export function visible(user: AuthUser | null, c: LoadedCourse, idx: AccessIndex): boolean {
|
||||
if (!c.manifest) return isOwner(user);
|
||||
return canView(user, c, idx.acl.get(c.id) ?? [], canEdit(user, c.id, idx));
|
||||
return canView(user, c, idx.acl.get(c.id) ?? [], idx.authorOf.has(c.id));
|
||||
}
|
||||
|
||||
/** The course if this user may read it, else null (callers answer 404, never 403, so hidden courses stay hidden). */
|
||||
|
||||
@@ -9,6 +9,7 @@ import { userDataRoutes } from "./userdata";
|
||||
import { codeRoutes } from "./code";
|
||||
import { transpileRoutes } from "./transpile";
|
||||
import { assetRoutes } from "./assets";
|
||||
import { runnerFrameRoutes } from "./runnerFrame";
|
||||
|
||||
/**
|
||||
* The learner feature: auth, library, reader, notes, progress and exercises.
|
||||
@@ -34,6 +35,7 @@ export function learnerFeature(ctx: ServerContext): Feature & { auth: AuthServic
|
||||
...codeRoutes(ctx),
|
||||
...transpileRoutes(),
|
||||
...assetRoutes(),
|
||||
...runnerFrameRoutes(),
|
||||
};
|
||||
// Every handler waits for the learner migrations (a no-op once Phase 2 runs them in migrate()).
|
||||
for (const methods of Object.values(routes)) {
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
import type { Feature } from "@stc/contracts";
|
||||
import { WORKER_SOURCE } from "../web/runner/worker-source";
|
||||
|
||||
/**
|
||||
* The exercise runner frame. The page loads it in `<iframe sandbox="allow-scripts">` (never allow-same-origin),
|
||||
* so it has an opaque origin: no cookies, no storage, no access to the page. Its own CSP allows only this one
|
||||
* inline script (by hash) and blob: scripts and workers, and no connections at all (`connect-src 'none'`), which
|
||||
* also covers dynamic import of remote URLs, Cache.add, nested workers, EventSource and WebSocket. The worker
|
||||
* inherits that policy. The page talks to the frame only with postMessage, and both sides check `event.source`.
|
||||
*/
|
||||
export const RUNNER_FRAME_PATH = "/api/learner/runner-frame";
|
||||
|
||||
export const FRAME_SCRIPT = `(() => {
|
||||
"use strict";
|
||||
const SRC = ${JSON.stringify(WORKER_SOURCE)};
|
||||
let worker = null;
|
||||
const send = (msg) => parent.postMessage(msg, "*");
|
||||
addEventListener("message", (ev) => {
|
||||
if (ev.source !== parent) return;
|
||||
const m = ev.data;
|
||||
if (!m || m.type !== "run" || worker) return;
|
||||
const url = URL.createObjectURL(new Blob([SRC], { type: "text/javascript" }));
|
||||
// A classic worker: module workers from blob: URLs fail to start in an opaque-origin frame in Chrome.
|
||||
// The worker still loads the learner and test modules with import() of blob: URLs.
|
||||
worker = new Worker(url);
|
||||
worker.onmessage = (e) => send({ type: "worker", data: e.data });
|
||||
worker.onerror = (e) => { e.preventDefault(); send({ type: "crash", message: e.message || "The exercise worker crashed" }); };
|
||||
worker.postMessage({ code: String(m.code), tests: String(m.tests) });
|
||||
});
|
||||
send({ type: "ready" });
|
||||
})();`;
|
||||
|
||||
const hash = new Bun.CryptoHasher("sha256").update(FRAME_SCRIPT).digest("base64");
|
||||
|
||||
export const RUNNER_FRAME_CSP = [
|
||||
"default-src 'none'",
|
||||
`script-src 'sha256-${hash}' blob:`,
|
||||
"worker-src blob:",
|
||||
"connect-src 'none'",
|
||||
"img-src 'none'",
|
||||
"style-src 'none'",
|
||||
"font-src 'none'",
|
||||
"media-src 'none'",
|
||||
"object-src 'none'",
|
||||
"base-uri 'none'",
|
||||
"form-action 'none'",
|
||||
"frame-ancestors 'self'",
|
||||
"sandbox allow-scripts",
|
||||
].join("; ");
|
||||
|
||||
const HTML = `<!doctype html><html><head><meta charset="utf-8"><title>Exercise runner</title></head><body><script>${FRAME_SCRIPT}</script></body></html>`;
|
||||
|
||||
export function runnerFrameRoutes(): Feature["routes"] {
|
||||
return {
|
||||
[RUNNER_FRAME_PATH]: {
|
||||
GET: async () => new Response(HTML, {
|
||||
headers: {
|
||||
"Content-Type": "text/html; charset=utf-8",
|
||||
"Content-Security-Policy": RUNNER_FRAME_CSP,
|
||||
"X-Content-Type-Options": "nosniff",
|
||||
"Referrer-Policy": "no-referrer",
|
||||
"Cache-Control": "no-store",
|
||||
},
|
||||
}),
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,80 @@
|
||||
import type { HTMLBundle } from "bun";
|
||||
|
||||
/**
|
||||
* Content-Security-Policy for the SPA document. No inline script: `script-src 'self'` only (plus, in development,
|
||||
* the hashes of the inline helper scripts Bun's dev server injects). Style attributes are allowed because React
|
||||
* components set them; styles cannot run script. The exercise runner frame is a same-origin URL (`frame-src
|
||||
* 'self'`) with its own, stricter policy, see `runnerFrame.ts`.
|
||||
*/
|
||||
export function spaCsp(inlineScriptHashes: string[] = []): string {
|
||||
const hashes = inlineScriptHashes.map((h) => ` '${h}'`).join("");
|
||||
return [
|
||||
"default-src 'self'",
|
||||
`script-src 'self'${hashes}`,
|
||||
"style-src 'self' 'unsafe-inline'",
|
||||
"img-src 'self' data: blob: https:",
|
||||
"media-src 'self' blob:",
|
||||
"font-src 'self' data:",
|
||||
"connect-src 'self'",
|
||||
"worker-src 'self' blob:",
|
||||
"frame-src 'self'",
|
||||
"object-src 'none'",
|
||||
"base-uri 'self'",
|
||||
"form-action 'self'",
|
||||
"frame-ancestors 'self'",
|
||||
].join("; ");
|
||||
}
|
||||
|
||||
/** `sha256-…` CSP sources for every inline `<script>` (no `src`) in `html`. */
|
||||
export function inlineScriptHashes(html: string): string[] {
|
||||
const out: string[] = [];
|
||||
for (const m of html.matchAll(/<script\b(?![^>]*\bsrc\s*=)[^>]*>([\s\S]*?)<\/script>/gi)) {
|
||||
out.push(`sha256-${new Bun.CryptoHasher("sha256").update(m[1]!).digest("base64")}`);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Serves the SPA (`index.html` bundle) with a CSP header. Bun.serve cannot add headers to an HTML import route,
|
||||
* so the bundle is served by an internal server bound to 127.0.0.1 on a random port, and these routes proxy to
|
||||
* it, adding the header to HTML responses. Phase 2 can replace this once core sets the CSP globally
|
||||
* (docs/contract-requests/learner.md §5).
|
||||
*/
|
||||
export function spaWithCsp(indexHtml: HTMLBundle | ((req: Request) => Response | Promise<Response>), opts: { development: boolean }) {
|
||||
const inner = Bun.serve({
|
||||
hostname: "127.0.0.1",
|
||||
port: 0,
|
||||
routes: { "/*": indexHtml },
|
||||
// HMR needs a websocket on the public server; it is off, and the bundle still rebuilds per request in dev.
|
||||
development: opts.development ? { hmr: false, console: false } : false,
|
||||
fetch: () => new Response("Not found", { status: 404 }),
|
||||
});
|
||||
const base = `http://127.0.0.1:${inner.port}`;
|
||||
|
||||
async function handle(req: Request): Promise<Response> {
|
||||
const u = new URL(req.url);
|
||||
const headers = new Headers(req.headers);
|
||||
headers.delete("host");
|
||||
headers.set("accept-encoding", "identity");
|
||||
const r = await fetch(base + u.pathname + u.search, {
|
||||
method: req.method, headers, body: req.method === "GET" || req.method === "HEAD" ? undefined : await req.arrayBuffer(), redirect: "manual",
|
||||
});
|
||||
const out = new Headers(r.headers);
|
||||
out.delete("content-encoding");
|
||||
out.delete("content-length");
|
||||
out.delete("transfer-encoding");
|
||||
if ((out.get("content-type") ?? "").includes("text/html")) {
|
||||
const html = await r.text();
|
||||
out.set("Content-Security-Policy", spaCsp(opts.development ? inlineScriptHashes(html) : []));
|
||||
out.set("X-Content-Type-Options", "nosniff");
|
||||
out.set("Referrer-Policy", "same-origin");
|
||||
return new Response(html, { status: r.status, headers: out });
|
||||
}
|
||||
return new Response(r.body, { status: r.status, headers: out });
|
||||
}
|
||||
|
||||
return {
|
||||
routes: { "/*": handle } as Record<string, (req: Request) => Promise<Response>>,
|
||||
stop: () => inner.stop(true),
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user