From 98a23c8e95760c359fcc49bb2557493019271e0e Mon Sep 17 00:00:00 2001 From: ttbombadil Date: Sat, 3 Oct 2026 22:54:09 +0200 Subject: [PATCH] fix(isolation): scope the legacy code fallback and sketch writes to the identity A code-less start_simulation ran whatever any user compiled last, and any user could overwrite or delete the shared start sketch. Both now hold per subject; the REST and WebSocket contracts keep their shape and status codes. Co-Authored-By: Claude Opus 5.5 --- docs/UNOSIM_REFACTORING_OPL.md | 13 ++- server/config.ts | 2 + server/routes.ts | 65 ++----------- server/routes/compiler.routes.ts | 10 +- server/routes/simulation.ws.ts | 4 +- server/routes/sketches.routes.ts | 59 +++++++++++ server/services/last-compiled-code-store.ts | 24 +++++ server/storage.ts | 51 ++++++++++ tests/server/routes/compiler.routes.test.ts | 2 +- tests/server/routes/routes-core.test.ts | 34 ++++--- .../simulation-last-compiled-code.test.ts | 97 +++++++++++++++++++ tests/server/routes/sketches.routes.test.ts | 64 ++++++++++++ .../services/last-compiled-code-store.test.ts | 27 ++++++ vitest.config.ts | 2 + 14 files changed, 377 insertions(+), 77 deletions(-) create mode 100644 server/routes/sketches.routes.ts create mode 100644 server/services/last-compiled-code-store.ts create mode 100644 tests/server/routes/simulation-last-compiled-code.test.ts create mode 100644 tests/server/routes/sketches.routes.test.ts create mode 100644 tests/server/services/last-compiled-code-store.test.ts diff --git a/docs/UNOSIM_REFACTORING_OPL.md b/docs/UNOSIM_REFACTORING_OPL.md index 31f32a7c4..abb7ff311 100644 --- a/docs/UNOSIM_REFACTORING_OPL.md +++ b/docs/UNOSIM_REFACTORING_OPL.md @@ -15,7 +15,7 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet. | ID | Befund | Kategorie | Evidenz | Risiko | Nutzen | Aufwand | Abhängigkeiten | Ziel-PR | Status | Verifikation | Merge-SHA/Ergebnis | |---|---|---|---|---|---|---|---|---|---|---|---| | R1 | Compile-Pfad absichern: sichere Include-Grenze vor arduino-cli (Env-Allowlist entfällt, siehe S1-ENV) | Security | S1 [Code, mit Sentinel verifiziert] | hoch | schließt den Kanal, über den eine beliebige lesbare Datei vollständig in Diagnosen erscheint | klein | – | fix/compile-include-guard | DONE | RED→GREEN: `include-guard.test.ts` (25), `arduino-compiler-include-guard.test.ts` (2), Toolchain-Sentinel-Test `compile-include-boundary.test.ts` (echte arduino-cli; vorher Sentinel in der Antwort, nachher nicht); Unit 2697 grün | PR-Merge siehe Verlauf | -| R2 | `lastCompiledCode`-Fallback und globales schreibbares Sketch-CRUD entfernen | Isolation | S2, S3 [Code] | mittel–hoch | Nutzerisolation | klein | – | fix/remove-global-legacy-state | OPEN | Start ohne Code → Fehler; Sketch-Schreibrouten weg | – | +| R2 | `lastCompiledCode`-Fallback und Sketch-CRUD pro Identität absichern (nicht entfernen) | Isolation | S2, S3 [Code] | mittel–hoch | Nutzerisolation ohne Bruch des REST-/WS-Vertrags | klein | – | fix/isolate-legacy-global-state | DONE | RED→GREEN: `simulation-last-compiled-code.test.ts` (fremder Code wird nie ausgeführt, eigener Fallback bleibt), `last-compiled-code-store.test.ts`, `sketches.routes.test.ts` (Seed read-only, fremde Sketches 404); zwei Bestandstests an Identität angepasst (Begründung im PR) | PR-Merge siehe Verlauf | | R6 | Einheitlicher Compile-Hash inkl. Header; kein Binary in REST-Payload/LRU | Korrektheit/Performance | A4, A5 [Code] | mittel | keine veralteten Cache-Treffer, kleinere Antworten | klein | – | fix/compile-cache-key-and-payload | OPEN | Header-only-Änderung → frischer Compile; Response ohne `binary` | – | | R7 | Globaler API-Limiter im Gateway-Modus nach `subject` statt IP | Skalierung | P1 [Code] | mittel (topologieabhängig) | keine klassenweiten 429 hinter NAT | klein | – | fix/api-rate-limit-identity | OPEN | Route-Tests beider Trust-Modi | – | | R3a | Lauf-Generation + Abbruch im Runner-Lifecycle | Isolation/Lifecycle | S4 [plausibel] | hoch | keine fremde Ausgabe, keine verwaisten Container | mittel | – | fix/runner-run-generation | OPEN | deterministischer Race-Test A wartet → A stoppt → B übernimmt | – | @@ -37,8 +37,8 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet. | S1-ENV | Env-Leak über geerbte Prozessumgebung und `/proc/*/environ` | Security | g++ 12 (Sandbox-Image) und avr-g++ 7.3 (Backend-Image) lesen `/proc/self/environ` und `/proc/1/environ` per `#include` und `.incbin` als leer (Sentinel-Env-Variable nicht sichtbar) | – | – | – | – | – | FALSIFIED | Docker-Experiment mit synthetischer Variable | keine Env-Allowlist umgesetzt | | S1-INCBIN | `.incbin` im Inline-Assembler bettet reguläre Dateien ins HEX ein; das HEX geht derzeit im REST-JSON an den Client | Security | [Code] verifiziert (Marker-Datei im Objekt) | mittel | – | – | – | R6 | OPEN | Response ohne `binary` | Kanal schließt mit R6 | | S1-ASM | GAS `.include` im Inline-Assembler gibt die ersten ~10 Zeichen je Zeile einer beliebigen Datei als Fehlermeldung aus; per String-Konkatenation nicht robust textuell filterbar | Security | [Code] verifiziert (Marker-Präfix in der Assemblermeldung) | mittel (Teilinhalt; Default-Deployment hält Secrets nur in Env) | – | groß | – | – | BLOCKED_DECISION | – | Robuster Fix = REST-Compile ohne Zugriff auf Backend-Dateien (Sandbox/Namespace); Architekturentscheidung | -| S2 | Globaler `lastCompiledCode` | Isolation | [Code] | mittel–hoch | – | – | – | R2 | OPEN | – | – | -| S3 | Sketch-CRUD ohne Besitzer | Isolation | [Code] | mittel | – | – | – | R2 | OPEN | – | – | +| S2 | Globaler `lastCompiledCode` | Isolation | [Code] bestätigt | mittel–hoch | – | – | – | R2 | DONE | WS-Test zweier Subjects | Fallback jetzt pro Subject (LRU, 1000 Subjects) | +| S3 | Sketch-CRUD ohne Besitzer | Isolation | [Code] bestätigt | mittel | – | – | – | R2 | DONE | Route-Test zweier Identitäten | Schreiben nur auf eigene Sketches, Seed schreibgeschützt | | S4 | Runner-Reuse-Race | Isolation/Lifecycle | [plausibel] | hoch | – | – | – | R3a | OPEN | Race-Test | – | | A1 | Überlappende Concurrency-Mechanismen, vermischte Statusmetriken | Concurrency | [Code] | mittel | – | – | – | R5b | OPEN | – | – | | A2 | Gatekeeper-TTL ohne Queue-Fortsetzung, tote Cache-Locks | Concurrency | [Code] | mittel | – | – | – | R5a | OPEN | – | – | @@ -66,6 +66,10 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet. ## Reihenfolge-Änderungen +- R2 sichert statt zu entfernen: Der Code deklariert den Code-losen Start als + Kompatibilität bis zum nächsten Protokoll-Major, und ARCHITECTURE verlangt für + inkompatible REST-Änderungen eine neue Major-Version. Die Isolation pro + Identität erreicht das Ziel ohne Vertragsbruch. - R1 umfasst nur die Include-Grenze. Der `.incbin`-Kanal (S1-INCBIN) wird mit R6 geschlossen, weil dort das Binary aus der REST-Antwort entfernt wird. @@ -74,4 +78,5 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet. | PR | Inhalt | Merge-SHA | CI | |---|---|---|---| | #159 | Audit-Bericht und diese OPL | `0863ba1f` | PR-CI 5/5 grün; Post-Merge-CI siehe nächster Eintrag | -| R1 | Include-Grenze für den REST-Compiler | – | – | +| #160 | R1: Include-Grenze für den REST-Compiler | `1c2995d9` | PR-CI 5/5 grün; Post-Merge-CI von #159 grün | +| R2 | Per-Identity-Isolation von Code-Fallback und Sketch-CRUD | – | – | diff --git a/server/config.ts b/server/config.ts index eecde2bc9..535ef47ab 100644 --- a/server/config.ts +++ b/server/config.ts @@ -532,6 +532,8 @@ export const config = { resultCacheMaxEntries: 100, /** Time-to-live for compile result cache entries */ resultCacheTtlMs: 5 * 60 * 1000, + /** Subjects whose last compiled code is kept for code-less simulation starts */ + lastCompiledCodeMaxSubjects: 1000, /** Max queued compile requests in the unified gatekeeper */ gatekeeperMaxQueueSize: 500, /** Bypass gatekeeper in E2E tests */ diff --git a/server/routes.ts b/server/routes.ts index e056996df..33eaf2f0f 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -5,7 +5,7 @@ import type { WebSocketServer } from "ws"; import { createServer, type Server } from "node:http"; import { createHash } from "node:crypto"; -import { storage } from "./storage"; +import { ownedSketches } from "./storage"; import { getCompilerWithFallback } from "./services/compiler-with-fallback"; import { SandboxRunner } from "./services/sandbox-runner"; import { @@ -18,7 +18,6 @@ import { getSandboxRunnerPool, initializeSandboxRunnerPool, } from "./services/sandbox-runner-pool"; -import { insertSketchSchema } from "@shared/schema"; import { Logger } from "@shared/logger"; // Pfad ggf. anpassen @@ -30,6 +29,8 @@ import { registerConfigRoutes } from "./routes/config.routes"; import { registerTestResetRoute } from "./routes/test-reset.routes"; import { registerExamplesRoutes } from "./routes/examples.routes"; import { registerTutorRoutes } from "./routes/tutor.routes"; +import { registerSketchRoutes } from "./routes/sketches.routes"; +import { LastCompiledCodeStore } from "./services/last-compiled-code-store"; import { ExamplesRepository } from "./services/examples/examples-repository"; import { config } from "./config"; import { createUserAuthorizationMiddleware } from "./security/access-control"; @@ -139,8 +140,9 @@ export async function registerRoutes(app: Express): Promise { * Legacy compatibility fallback for clients that omit code in * start_simulation. New clients must send the compiled code per session. * Planned for removal after the legacy protocol sunset (next major release). + * Kept per subject: a start never runs code that another user compiled. */ - let lastCompiledCode: string | null = null; + const lastCompiledCode = new LastCompiledCodeStore(config.compilation.lastCompiledCodeMaxSubjects); // Compilation Cache: Map const compilationCache = new CompilationCache(config.compilation.resultCacheMaxEntries); @@ -173,56 +175,7 @@ export async function registerRoutes(app: Express): Promise { disableRateLimit: config.server.disableRateLimit, courseContent: examplesRepository, }); - // --- Sketch CRUD routes (leicht gekürzt) --- - app.get("/api/sketches", async (_req, res) => { - try { - const sketches = await storage.getAllSketches(); - res.json(sketches); - } catch { - res.status(500).json({ error: "Failed to fetch sketches" }); - } - }); - - app.get("/api/sketches/:id", async (req, res) => { - try { - const sketch = await storage.getSketch(req.params.id); - if (!sketch) return res.status(404).json({ error: "Sketch not found" }); - res.json(sketch); - } catch { - res.status(500).json({ error: "Failed to fetch sketch" }); - } - }); - - app.post("/api/sketches", async (req, res) => { - try { - const validatedData = insertSketchSchema.parse(req.body); - const sketch = await storage.createSketch(validatedData); - res.status(201).json(sketch); - } catch { - res.status(400).json({ error: "Invalid sketch data" }); - } - }); - - app.put("/api/sketches/:id", async (req, res) => { - try { - const validatedData = insertSketchSchema.partial().parse(req.body); - const sketch = await storage.updateSketch(req.params.id, validatedData); - if (!sketch) return res.status(404).json({ error: "Sketch not found" }); - res.json(sketch); - } catch { - res.status(400).json({ error: "Invalid sketch data" }); - } - }); - - app.delete("/api/sketches/:id", async (req, res) => { - try { - const deleted = await storage.deleteSketch(req.params.id); - if (!deleted) return res.status(404).json({ error: "Sketch not found" }); - res.status(204).send(); - } catch { - res.status(500).json({ error: "Failed to delete sketch" }); - } - }); + registerSketchRoutes(app, ownedSketches); // --- COMPILATION (moved to modular route) --- // Delegate the /api/compile handler to the compiler module and inject @@ -236,8 +189,8 @@ export async function registerRoutes(app: Express): Promise { compilationCache, hashCode, CACHE_TTL, - setLastCompiledCode: (code: string | null) => { - lastCompiledCode = code; + setLastCompiledCode: (subject: string, code: string) => { + lastCompiledCode.set(subject, code); }, logger, compileRateLimiter: getCompileRateLimiter(), @@ -253,7 +206,7 @@ export async function registerRoutes(app: Express): Promise { getSimulationRateLimiter, getSimulationAdmissionController, shouldSendSimulationEndMessage, - getLastCompiledCode: () => lastCompiledCode, + getLastCompiledCode: (subject: string) => lastCompiledCode.get(subject), logger, runnerPool, trust: config.trust, diff --git a/server/routes/compiler.routes.ts b/server/routes/compiler.routes.ts index 72c74e418..d849ef883 100644 --- a/server/routes/compiler.routes.ts +++ b/server/routes/compiler.routes.ts @@ -20,7 +20,7 @@ type CompilerDeps = { compilationCache: Map; hashCode: (code: string, headers?: CompilerHeader[], options?: CompileRequestOptions) => string; CACHE_TTL: number; - setLastCompiledCode: (code: string | null) => void; + setLastCompiledCode: (subject: string, code: string) => void; logger: Logger; compileRateLimiter?: { checkLimit: (identity: string) => RateLimitResult }; disableRateLimit?: boolean; @@ -130,6 +130,10 @@ function getCachedCompilation( export function registerCompilerRoutes(app: Express, deps: CompilerDeps) { const { compiler, compilationCache, hashCode, CACHE_TTL, setLastCompiledCode, logger } = deps; + const rememberCompiledCode = (res: Response, code: string) => { + const identity = res.locals.unosimIdentity as RequestIdentity | undefined; + if (identity) setLastCompiledCode(identity.subject, code); + }; app.post("/api/compile", async (req, res) => { let compileStartTime: number | null = null; @@ -157,7 +161,7 @@ export function registerCompilerRoutes(app: Express, deps: CompilerDeps) { ); if (cachedResult) { logger.info(`✅ Cache hit for code (age: ${cachedResult.ageMs}ms)`); - setLastCompiledCode(code); + rememberCompiledCode(res, code); return res.json({ ...cachedResult.result, cached: true }); } @@ -184,7 +188,7 @@ export function registerCompilerRoutes(app: Express, deps: CompilerDeps) { compilationCache.set(codeHash, { result, timestamp: Date.now() }); logger.info(`✅ Cached compilation result for code`); } - setLastCompiledCode(code); + rememberCompiledCode(res, code); } res.json(result); diff --git a/server/routes/simulation.ws.ts b/server/routes/simulation.ws.ts index b206e0f33..77de4f73c 100644 --- a/server/routes/simulation.ws.ts +++ b/server/routes/simulation.ws.ts @@ -158,7 +158,7 @@ type SimulationDeps = { }; getSimulationAdmissionController?: () => SimulationAdmissionController; shouldSendSimulationEndMessage: (compileFailed: boolean) => boolean; - getLastCompiledCode: () => string | null; + getLastCompiledCode: (subject: string) => string | null; logger: Logger; runnerPool?: ReturnType; trust: TrustConfig; @@ -524,7 +524,7 @@ export function registerSimulationWebSocket( typeof data.code === "string" && data.code.trim().length > 0 ? data.code - : getLastCompiledCode(); + : getLastCompiledCode(clientState.subject); if (!code) { if (clientState.runner) { await safeReleaseRunner(clientState, "missing-compiled-code"); diff --git a/server/routes/sketches.routes.ts b/server/routes/sketches.routes.ts new file mode 100644 index 000000000..042a7f478 --- /dev/null +++ b/server/routes/sketches.routes.ts @@ -0,0 +1,59 @@ +import type { Express, Response } from "express"; +import { insertSketchSchema } from "@shared/schema"; +import type { RequestIdentity } from "../security/access-control"; +import type { OwnedSketchStore } from "../storage"; + +function subjectOf(res: Response): string | undefined { + return (res.locals.unosimIdentity as RequestIdentity | undefined)?.subject; +} + +export function registerSketchRoutes(app: Express, sketches: OwnedSketchStore): void { + app.get("/api/sketches", async (_req, res) => { + try { + res.json(await sketches.list(subjectOf(res))); + } catch { + res.status(500).json({ error: "Failed to fetch sketches" }); + } + }); + + app.get("/api/sketches/:id", async (req, res) => { + try { + const sketch = await sketches.get(subjectOf(res), req.params.id); + if (!sketch) return res.status(404).json({ error: "Sketch not found" }); + res.json(sketch); + } catch { + res.status(500).json({ error: "Failed to fetch sketch" }); + } + }); + + app.post("/api/sketches", async (req, res) => { + try { + const validatedData = insertSketchSchema.parse(req.body); + const sketch = await sketches.create(subjectOf(res), validatedData); + res.status(201).json(sketch); + } catch { + res.status(400).json({ error: "Invalid sketch data" }); + } + }); + + app.put("/api/sketches/:id", async (req, res) => { + try { + const validatedData = insertSketchSchema.partial().parse(req.body); + const sketch = await sketches.update(subjectOf(res), req.params.id, validatedData); + if (!sketch) return res.status(404).json({ error: "Sketch not found" }); + res.json(sketch); + } catch { + res.status(400).json({ error: "Invalid sketch data" }); + } + }); + + app.delete("/api/sketches/:id", async (req, res) => { + try { + const deleted = await sketches.delete(subjectOf(res), req.params.id); + if (!deleted) return res.status(404).json({ error: "Sketch not found" }); + res.status(204).send(); + } catch { + res.status(500).json({ error: "Failed to delete sketch" }); + } + }); +} diff --git a/server/services/last-compiled-code-store.ts b/server/services/last-compiled-code-store.ts new file mode 100644 index 000000000..afc007b65 --- /dev/null +++ b/server/services/last-compiled-code-store.ts @@ -0,0 +1,24 @@ +/** + * Last successfully compiled code per authenticated subject, for the legacy + * `start_simulation` without `code`. Keyed by subject so that a start never + * runs code another user compiled; bounded so idle subjects cannot grow it. + */ +export class LastCompiledCodeStore { + private readonly codeBySubject = new Map(); + + constructor(private readonly maxSubjects: number) {} + + set(subject: string, code: string): void { + this.codeBySubject.delete(subject); + this.codeBySubject.set(subject, code); + while (this.codeBySubject.size > this.maxSubjects) { + const oldest = this.codeBySubject.keys().next().value; + if (oldest === undefined) break; + this.codeBySubject.delete(oldest); + } + } + + get(subject: string): string | null { + return this.codeBySubject.get(subject) ?? null; + } +} diff --git a/server/storage.ts b/server/storage.ts index b03d3e2ba..b1e763c47 100644 --- a/server/storage.ts +++ b/server/storage.ts @@ -81,3 +81,54 @@ void loop() { } export const storage = new MemStorage(); + +/** + * Per-identity view of the sketch storage. Sketches created through the API + * belong to the creating subject and are invisible to everyone else; the + * seeded default sketch is shared and read-only. Unknown and foreign sketches + * look the same (`undefined`/`false`), so ids leak no ownership information. + */ +export class OwnedSketchStore { + private readonly ownerById = new Map(); + + constructor(private readonly storage: IStorage) {} + + private isVisible(owner: string | undefined, id: string): boolean { + const sketchOwner = this.ownerById.get(id); + return sketchOwner === undefined || (owner !== undefined && sketchOwner === owner); + } + + private isOwned(owner: string | undefined, id: string): boolean { + return owner !== undefined && this.ownerById.get(id) === owner; + } + + async list(owner: string | undefined): Promise { + return (await this.storage.getAllSketches()).filter(({ id }) => this.isVisible(owner, id)); + } + + async get(owner: string | undefined, id: string): Promise { + const sketch = await this.storage.getSketch(id); + return sketch && this.isVisible(owner, id) ? sketch : undefined; + } + + async create(owner: string | undefined, sketch: InsertSketch): Promise { + if (owner === undefined) throw new Error("Creating a sketch requires an identity"); + const created = await this.storage.createSketch(sketch); + this.ownerById.set(created.id, owner); + return created; + } + + async update(owner: string | undefined, id: string, sketch: Partial): Promise { + return this.isOwned(owner, id) ? this.storage.updateSketch(id, sketch) : undefined; + } + + async delete(owner: string | undefined, id: string): Promise { + const existing = await this.storage.getSketch(id); + if (!existing || !this.isOwned(owner, id)) return false; + const deleted = await this.storage.deleteSketch(id); + if (deleted) this.ownerById.delete(id); + return deleted; + } +} + +export const ownedSketches = new OwnedSketchStore(storage); diff --git a/tests/server/routes/compiler.routes.test.ts b/tests/server/routes/compiler.routes.test.ts index 26d505d3b..088f32255 100644 --- a/tests/server/routes/compiler.routes.test.ts +++ b/tests/server/routes/compiler.routes.test.ts @@ -213,7 +213,7 @@ describe("compiler.routes - /api/compile", () => { undefined, { fqbn: undefined, libraries: undefined }, ); - expect(deps.setLastCompiledCode).toHaveBeenCalledWith("void setup(){}"); + expect(deps.setLastCompiledCode).toHaveBeenCalledWith("student-a", "void setup(){}"); }); it("forwards an explicit logical entryFile and nested header paths", async () => { diff --git a/tests/server/routes/routes-core.test.ts b/tests/server/routes/routes-core.test.ts index 61f9ac6a0..886147657 100644 --- a/tests/server/routes/routes-core.test.ts +++ b/tests/server/routes/routes-core.test.ts @@ -72,8 +72,13 @@ function listen(app: express.Express): Promise<{ baseUrl: string; server: http.S }); } -async function request(baseUrl: string, method: string, route: string, body?: unknown) { - return new Promise<{ status: number; body: unknown }>((resolve, reject) => { +async function request(baseUrl: string, method: string, route: string, body?: unknown, cookie?: string) { + const { setCookie: _setCookie, ...response } = await sessionRequest(baseUrl, method, route, body, cookie); + return response; +} + +async function sessionRequest(baseUrl: string, method: string, route: string, body?: unknown, cookie?: string) { + return new Promise<{ status: number; body: unknown; setCookie?: string[] }>((resolve, reject) => { const url = new URL(route, baseUrl); const payload = body === undefined ? undefined : JSON.stringify(body); const req = http.request({ @@ -81,13 +86,17 @@ async function request(baseUrl: string, method: string, route: string, body?: un port: url.port, path: `${url.pathname}${url.search}`, method, - headers: payload ? { "content-type": "application/json", "content-length": Buffer.byteLength(payload) } : undefined, + headers: { + ...(payload ? { "content-type": "application/json", "content-length": Buffer.byteLength(payload) } : {}), + ...(cookie ? { cookie } : {}), + }, }, (res) => { let data = ""; res.on("data", (chunk) => { data += chunk; }); res.on("end", () => { - try { resolve({ status: res.statusCode ?? 0, body: JSON.parse(data) }); } - catch { resolve({ status: res.statusCode ?? 0, body: data }); } + const setCookie = res.headers["set-cookie"]; + try { resolve({ status: res.statusCode ?? 0, body: JSON.parse(data), ...(setCookie ? { setCookie } : {}) }); } + catch { resolve({ status: res.statusCode ?? 0, body: data, ...(setCookie ? { setCookie } : {}) }); } }); }); req.on("error", reject); @@ -140,21 +149,24 @@ describe("registerRoutes core HTTP behavior", () => { it("creates, reads, updates, lists, and deletes sketches through the public API", async () => { await startServer(); - const created = await request(baseUrl, "POST", "/api/sketches", { + // Sketches belong to the creating identity; reuse its session cookie like a browser does. + const created = await sessionRequest(baseUrl, "POST", "/api/sketches", { name: "routes-test.ino", content: "void setup(){} void loop(){}", }); expect(created.status).toBe(201); const id = (created.body as { id: string }).id; + const session = created.setCookie?.[0]?.split(";")[0]; + expect(session).toMatch(/^unosim_local_session=/); - await expect(request(baseUrl, "GET", `/api/sketches/${id}`)).resolves.toMatchObject({ status: 200, body: { id } }); - await expect(request(baseUrl, "PUT", `/api/sketches/${id}`, { name: "updated.ino" })).resolves.toMatchObject({ + await expect(request(baseUrl, "GET", `/api/sketches/${id}`, undefined, session)).resolves.toMatchObject({ status: 200, body: { id } }); + await expect(request(baseUrl, "PUT", `/api/sketches/${id}`, { name: "updated.ino" }, session)).resolves.toMatchObject({ status: 200, body: { id, name: "updated.ino" }, }); - expect((await request(baseUrl, "GET", "/api/sketches")).status).toBe(200); - await expect(request(baseUrl, "DELETE", `/api/sketches/${id}`)).resolves.toMatchObject({ status: 204 }); - await expect(request(baseUrl, "GET", `/api/sketches/${id}`)).resolves.toEqual({ + expect((await request(baseUrl, "GET", "/api/sketches", undefined, session)).status).toBe(200); + await expect(request(baseUrl, "DELETE", `/api/sketches/${id}`, undefined, session)).resolves.toMatchObject({ status: 204 }); + await expect(request(baseUrl, "GET", `/api/sketches/${id}`, undefined, session)).resolves.toEqual({ status: 404, body: { error: "Sketch not found" }, }); diff --git a/tests/server/routes/simulation-last-compiled-code.test.ts b/tests/server/routes/simulation-last-compiled-code.test.ts new file mode 100644 index 000000000..54e649bde --- /dev/null +++ b/tests/server/routes/simulation-last-compiled-code.test.ts @@ -0,0 +1,97 @@ +import { createServer, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { WebSocket } from "ws"; +import type { Logger } from "@shared/logger"; +import type { ServerToClientWSMessage } from "@shared/schema"; +import { registerSimulationWebSocket } from "../../../server/routes/simulation.ws"; +import type { SandboxRunner } from "../../../server/services/sandbox-runner"; +import type { SandboxRunnerPool } from "../../../server/services/sandbox-runner-pool"; +import { SimulationAdmissionController } from "../../../server/services/simulation-admission-controller"; + +const SECRET = "a-secure-gateway-secret-with-32-characters"; +const ORIGIN = "https://classroom.example"; +const servers: Server[] = []; +const clients: WebSocket[] = []; + +async function waitFor(predicate: () => boolean, label: string): Promise { + const deadline = Date.now() + 2_000; + while (!predicate()) { + if (Date.now() >= deadline) throw new Error(`Timed out waiting for ${label}`); + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + +async function createHarness(lastCompiled: Record) { + const runSketch = vi.fn(async () => false); + const pool = { + acquireRunner: vi.fn(async () => ({ runSketch, stop: vi.fn().mockResolvedValue(undefined) })), + releaseRunner: vi.fn().mockResolvedValue(undefined), + getRunnerIndex: vi.fn(() => 0), + getStats: vi.fn(() => ({ availableRunners: 5, totalRunners: 5, maxRunners: 5, queuedRequests: 0 })), + }; + const server = createServer(); + registerSimulationWebSocket(server, { + SandboxRunner: class {} as typeof SandboxRunner, + getSimulationRateLimiter: () => ({ checkLimit: () => ({ allowed: true }) }), + getSimulationAdmissionController: () => new SimulationAdmissionController(25), + shouldSendSimulationEndMessage: () => true, + getLastCompiledCode: (subject: string) => lastCompiled[subject] ?? null, + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } as unknown as Logger, + runnerPool: pool as unknown as SandboxRunnerPool, + trust: { mode: "gateway", gatewaySecret: SECRET, trustedProxy: "127.0.0.1" }, + allowedWebSocketOrigins: [ORIGIN], + disableRateLimit: true, + }); + servers.push(server); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + + async function connect(subject: string) { + const socket = new WebSocket(`ws://127.0.0.1:${port}/ws`, { + headers: { Origin: ORIGIN, "X-UnoSim-Gateway-Secret": SECRET, "X-UnoSim-Subject": subject, "X-UnoSim-Roles": "user" }, + }); + clients.push(socket); + const messages: ServerToClientWSMessage[] = []; + socket.on("message", (raw) => messages.push(JSON.parse(raw.toString()) as ServerToClientWSMessage)); + await new Promise((resolve, reject) => { + socket.once("open", resolve); + socket.once("error", reject); + }); + return { socket, messages }; + } + + return { connect, runSketch }; +} + +afterEach(async () => { + await Promise.all(clients.splice(0).map((client) => new Promise((resolve) => { + if (client.readyState === WebSocket.CLOSED) return resolve(); + client.once("close", () => resolve()); + client.close(); + }))); + await Promise.all(servers.splice(0).map((server) => new Promise((resolve) => server.close(() => resolve())))); +}); + +describe("start_simulation without code (legacy fallback)", () => { + it("never runs code that another subject compiled", async () => { + const harness = await createHarness({ alice: "void setup(){} void loop(){} // alice" }); + const bob = await harness.connect("bob"); + + bob.socket.send(JSON.stringify({ type: "start_simulation" })); + await waitFor(() => bob.messages.some((m) => m.type === "serial_output"), "missing-code message"); + + expect(harness.runSketch).not.toHaveBeenCalled(); + expect(bob.messages).toContainEqual({ type: "serial_output", data: "[ERR] No compiled code available. Please compile first.\n" }); + }); + + it("keeps the fallback to the same subject's last compiled code", async () => { + const harness = await createHarness({ alice: "void setup(){} void loop(){} // alice" }); + const alice = await harness.connect("alice"); + + alice.socket.send(JSON.stringify({ type: "start_simulation" })); + await waitFor(() => harness.runSketch.mock.calls.length === 1, "alice run"); + + expect(harness.runSketch).toHaveBeenCalledWith(expect.objectContaining({ code: "void setup(){} void loop(){} // alice" })); + }); +}); diff --git a/tests/server/routes/sketches.routes.test.ts b/tests/server/routes/sketches.routes.test.ts new file mode 100644 index 000000000..87c92cf8f --- /dev/null +++ b/tests/server/routes/sketches.routes.test.ts @@ -0,0 +1,64 @@ +import express from "express"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterEach, describe, expect, it } from "vitest"; +import { registerSketchRoutes } from "../../../server/routes/sketches.routes"; +import { MemStorage, OwnedSketchStore } from "../../../server/storage"; + +const servers: http.Server[] = []; + +async function startApp() { + const app = express(); + app.use(express.json()); + app.use((req, res, next) => { + res.locals.unosimIdentity = { subject: req.header("x-test-subject"), roles: ["user"] }; + next(); + }); + registerSketchRoutes(app, new OwnedSketchStore(new MemStorage())); + const server = app.listen(0, "127.0.0.1"); + servers.push(server); + await new Promise((resolve) => server.once("listening", resolve)); + const { port } = server.address() as AddressInfo; + return async (subject: string, method: string, route: string, body?: unknown) => { + const response = await fetch(`http://127.0.0.1:${port}${route}`, { + method, + headers: { "content-type": "application/json", "x-test-subject": subject }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); + const text = await response.text(); + return { status: response.status, body: text ? JSON.parse(text) : undefined }; + }; +} + +afterEach(async () => { + await Promise.all(servers.splice(0).map((server) => new Promise((resolve) => server.close(() => resolve())))); +}); + +describe("sketch routes isolate writes per identity", () => { + it("lets each identity see the shared seed sketch but not change it", async () => { + const request = await startApp(); + const list = await request("alice", "GET", "/api/sketches"); + expect(list.status).toBe(200); + const seed = list.body[0] as { id: string; content: string }; + + expect((await request("alice", "PUT", `/api/sketches/${seed.id}`, { content: "tampered" })).status).toBe(404); + expect((await request("alice", "DELETE", `/api/sketches/${seed.id}`)).status).toBe(404); + expect((await request("bob", "GET", `/api/sketches/${seed.id}`)).body).toMatchObject({ content: seed.content }); + }); + + it("keeps a created sketch private to its creator", async () => { + const request = await startApp(); + const created = await request("alice", "POST", "/api/sketches", { name: "a.ino", content: "void setup(){} void loop(){}" }); + expect(created.status).toBe(201); + const id = created.body.id as string; + + expect((await request("bob", "GET", `/api/sketches/${id}`)).status).toBe(404); + expect((await request("bob", "PUT", `/api/sketches/${id}`, { content: "x" })).status).toBe(404); + expect((await request("bob", "DELETE", `/api/sketches/${id}`)).status).toBe(404); + expect((await request("bob", "GET", "/api/sketches")).body.map((sketch: { id: string }) => sketch.id)).not.toContain(id); + + expect((await request("alice", "PUT", `/api/sketches/${id}`, { name: "b.ino" })).body).toMatchObject({ id, name: "b.ino" }); + expect((await request("alice", "GET", "/api/sketches")).body.map((sketch: { id: string }) => sketch.id)).toContain(id); + expect((await request("alice", "DELETE", `/api/sketches/${id}`)).status).toBe(204); + }); +}); diff --git a/tests/server/services/last-compiled-code-store.test.ts b/tests/server/services/last-compiled-code-store.test.ts new file mode 100644 index 000000000..1d170d7c8 --- /dev/null +++ b/tests/server/services/last-compiled-code-store.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { LastCompiledCodeStore } from "../../../server/services/last-compiled-code-store"; + +describe("LastCompiledCodeStore", () => { + it("keeps the last compiled code per subject", () => { + const store = new LastCompiledCodeStore(10); + store.set("alice", "a1"); + store.set("bob", "b1"); + store.set("alice", "a2"); + + expect(store.get("alice")).toBe("a2"); + expect(store.get("bob")).toBe("b1"); + expect(store.get("carol")).toBeNull(); + }); + + it("evicts the least recently compiled subject beyond its bound", () => { + const store = new LastCompiledCodeStore(2); + store.set("alice", "a"); + store.set("bob", "b"); + store.set("alice", "a"); + store.set("carol", "c"); + + expect(store.get("bob")).toBeNull(); + expect(store.get("alice")).toBe("a"); + expect(store.get("carol")).toBe("c"); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 028fb57d4..f414a4e7e 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -24,6 +24,8 @@ const serializedHttpUnitTests = [ "tests/server/routes/server-status-observability.test.ts", "tests/server/routes/server-status.test.ts", "tests/server/routes/simulation-admission.test.ts", + "tests/server/routes/simulation-last-compiled-code.test.ts", + "tests/server/routes/sketches.routes.test.ts", "tests/server/routes/simulation-start-readiness.test.ts", "tests/server/routes/shutdown-websocket.test.ts", "tests/server/routes/test-reset.routes.test.ts",