fix(isolation): scope the legacy code fallback and sketch writes to the identity

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 | – | – |

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 */

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<Server> {
    * 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<codeHash, CompilationResult>
   const compilationCache = new CompilationCache(config.compilation.resultCacheMaxEntries);
@@ -173,56 +175,7 @@ export async function registerRoutes(app: Express): Promise<Server> {
     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<Server> {
     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<Server> {
     getSimulationRateLimiter,
     getSimulationAdmissionController,
     shouldSendSimulationEndMessage,
-    getLastCompiledCode: () => lastCompiledCode,
+    getLastCompiledCode: (subject: string) => lastCompiledCode.get(subject),
     logger,
     runnerPool,
     trust: config.trust,

server/routes/compiler.routes.ts

@@ -20,7 +20,7 @@ type CompilerDeps = {
   compilationCache: Map<string, { result: CompilationResult; timestamp: number }>;
   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);

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<typeof getSandboxRunnerPool>;
   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");

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" });
+    }
+  });
+}

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<string, string>();
+
+  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;
+  }
+}

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<string, string>();
+
+  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<Sketch[]> {
+    return (await this.storage.getAllSketches()).filter(({ id }) => this.isVisible(owner, id));
+  }
+
+  async get(owner: string | undefined, id: string): Promise<Sketch | undefined> {
+    const sketch = await this.storage.getSketch(id);
+    return sketch && this.isVisible(owner, id) ? sketch : undefined;
+  }
+
+  async create(owner: string | undefined, sketch: InsertSketch): Promise<Sketch> {
+    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<InsertSketch>): Promise<Sketch | undefined> {
+    return this.isOwned(owner, id) ? this.storage.updateSketch(id, sketch) : undefined;
+  }
+
+  async delete(owner: string | undefined, id: string): Promise<boolean> {
+    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);

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 () => {