Lucas WintherandClaude Opus 5.5
c7b9e1f98e
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]>
4.8 KiB
Contract requests from WP-A (learner)
1. Feature factories and the auth service (apps/server/src/main.ts, @stc/contracts)
Problem. Phase 0 passes Feature[] to startServer, but ServerContext is built inside startServer, so a feature that needs ctx (every Phase 1 package exports xFeature(ctx)) cannot be listed. There is also no way for a feature to replace the anonymous AuthService.
Workaround in feat/learner (applied, small): startServer accepts FeatureEntry = Feature | ((ctx) => Feature & { auth?: AuthService }). Factories run after ctx exists (with ctx.jobs set right after they return, so factories must not use jobs synchronously), and a feature carrying auth replaces ctx.auth. The main entry lists learnerFeature.
Proposal. Move both into the contract:
export type FeatureFactory = (ctx: ServerContext) => Feature;
export interface Feature { /* … */ auth?: AuthService; migrationsDirs?: string[] }
and let startServer take Array<Feature | FeatureFactory>.
2. Migrations per package
migrate() only reads packages/db/migrations. The learner package exports migrationsDirs and, until Phase 2 wires it in, runs its own migrations lazily inside learnerFeature(ctx) through runMigrations(db, dirs), which uses the same schema_migrations table and file-name keys. When migrate() learns about migrationsDirs, the lazy call becomes a no-op; it can then be removed.
3. Package entry for the browser
Problem. The package index exports both the server (learnerFeature, which pulls in Better Auth and @better-auth/sso) and the browser half (learnerWeb, LessonView). A browser bundle of the index fails: @better-auth/sso imports X509Certificate from node:crypto, which has no browser polyfill. The root tsconfig.json maps @stc/* to packages/*/src, so package.json exports subpaths are not used either.
Workaround in feat/learner (applied). LessonView and learnerWeb are exported from the index (for server-side and type use) and from a browser-only entry, @stc/feature-learner/web. That subpath resolves through the existing mapping to packages/feature-learner/web/src/index.ts, a one-line re-export of src/web. apps/web/src/features.ts imports learnerWeb from it, and the studio should import LessonView from it too.
Proposal. Add "@stc/*/web": ["packages/*/src/web"] ahead of the general mapping in tsconfig.json (Bun honours tsconfig paths), and let every feature package expose src/web/index.ts. The shim directory can then be deleted. This also settles the importer's relative import in features.ts.
4. LessonView for the studio preview
import { LessonView } from "@stc/feature-learner/web". Props: lesson: LessonPayload (the JSON from GET /api/courses/:id/lessons/:lessonId), plus optional look, track, onTrackChange, preview. With preview set, it makes no notes, attempt or progress requests; it still loads code listings and decks through the normal media routes. It injects its own stylesheet (scoped under .lr) and works with or without a router.
5. Content-Security-Policy on the SPA, set by core (review C1)
Problem. Lesson HTML is author content. The sanitizer (sanitize-html allowlist) is the first defence, and a CSP without inline script is the second. Bun.serve cannot attach headers to an HTML import route.
Workaround in feat/learner (applied). spaWithCsp(indexHtml, { development }) in @stc/feature-learner serves the bundle from an internal server bound to 127.0.0.1 and proxies it. HTML responses get Content-Security-Policy (script-src 'self', no 'unsafe-inline'; connect-src 'self'; object-src 'none'; frame-ancestors 'self'; see spaCsp()), plus X-Content-Type-Options: nosniff and Referrer-Policy: same-origin. main.ts uses it for "/*". HMR is off on the internal server, so Bun injects no inline script. If it ever does in development, only the hashes of those exact scripts are allowed.
Proposal. Core owns the SPA route and sets this CSP globally (and for every feature's HTML), replacing the proxy. Other features must then avoid inline scripts as well. The studio preview through LessonView already complies.
6. canView and the global author role (review I1)
Ruling applied. Learner routes now call core canView with per-course authorship only (course_authors), exactly like core's /api/media. A user with only the global author role gets a 404 for a restricted course they are not listed on, on every route.
Proposal for Phase 2. Extend core canView so the global author role can view every course (the studio and AI need the same rule), for example canView(user, course, acl, isCourseAuthor) returning true when user.roles.includes("author"). The learner then needs no change: it already delegates to canView.