fix(compiler): key the compile cache by headers and keep the HEX out of the REST response

docs/UNOSIM_REFACTORING_OPL.md

@@ -16,7 +16,7 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet.
 |---|---|---|---|---|---|---|---|---|---|---|---|
 | 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 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` | – |
+| R6 | Einheitlicher Compile-Hash inkl. Header (Worker-Identität); kein Binary in REST-Payload/LRU | Korrektheit/Performance/Security | A4, A5, S1-INCBIN [Code] | mittel | keine veralteten Cache-Treffer; Worker und direkter Pfad teilen Cache-Einträge; Antwort 59.653 → 562 Byte (Blink-Sketch, Worker-Pfad) | klein | – | fix/compile-cache-key-and-payload | DONE | RED→GREEN: `arduino-compiler-cache-key.test.ts` (Header-Änderung kompiliert neu; gleiche Identität wie der Worker), `compiler-binary-payload.test.ts` (frisch, gecacht, LRU ohne `binary`) | PR-Merge siehe Verlauf |
 | 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 | – |
 | R3b | Reset-Ownership in `runner.resetForReuse()` | Kapselung | A6 [Code] | mittel | Reset an einer Stelle | mittel | R3a | refactor/runner-reset-ownership | OPEN | Pool-/Isolationstests | – |
@@ -35,16 +35,16 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet.
 |---|---|---|---|---|---|---|---|---|---|---|---|
 | S1 | Absolutes/traversierendes `#include` lässt arduino-cli eine beliebige lesbare Datei lesen und in stderr ausgeben | Security | [Code] verifiziert: synthetische Sentinel-Datei erschien vollständig in der REST-Compile-Antwort | hoch | – | – | – | R1 | DONE | `compile-include-boundary.test.ts` | durch R1 geschlossen |
 | 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-INCBIN | `.incbin` im Inline-Assembler bettet reguläre Dateien ins HEX ein; das HEX ging im REST-JSON an den Client | Security | [Code] verifiziert (Marker-Datei im Objekt) | mittel | – | – | – | R6 | DONE | Response ohne `binary` | HEX verlässt den Server nicht mehr |
 | 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] 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 | – | – |
 | A3 | Fallback umgeht Lastgrenze, keine Worker-Recovery, unbegrenzte Queue | Concurrency | [Code] | mittel | – | – | – | R5b | OPEN | – | – |
-| A4 | Compile-Hash ohne Header im direkten Compiler | Korrektheit | [Code] | mittel | – | – | – | R6 | OPEN | – | – |
-| A5 | HEX-Binary in REST-JSON und LRU | Performance | [Code] | gering | – | – | – | R6 | OPEN | – | – |
+| A4 | Compile-Hash ohne Header im direkten Compiler | Korrektheit | [Code] bestätigt | mittel | – | – | – | R6 | DONE | Header-only-Test | `libraries` bleibt außerhalb des Hashes: arduino-cli erhält sie nicht |
+| A5 | HEX-Binary in REST-JSON und LRU | Performance | [Code] bestätigt, [gemessen] 59.653 statt 562 Byte | gering | – | – | – | R6 | DONE | Payload-Test | Simulation nutzt das REST-Binary nicht |
 | A6 | Pool setzt private Runner-Felder zurück | Kapselung | [Code] | mittel | – | – | – | R3b | OPEN | – | – |
 | A7 | Kein Orphan-Sweep, kein Heartbeat, keine Serialisierung, kein Message-Limit | Lifecycle | [Code] | mittel | – | – | – | R4a/R4b | OPEN | – | – |
 | P1 | Globaler API-Limiter pro IP (Campus-NAT) | Skalierung | [Code] | mittel | – | – | – | R7 | OPEN | – | – |
@@ -79,4 +79,5 @@ Abweichungen werden unter „Reihenfolge-Änderungen“ begründet.
 |---|---|---|---|
 | #159 | Audit-Bericht und diese OPL | `0863ba1f` | PR-CI 5/5 grün; Post-Merge-CI siehe nächster Eintrag |
 | #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 | – | – |
+| #161 | R2: Per-Identity-Isolation von Code-Fallback und Sketch-CRUD | `18c7d89b` | PR-CI 5/5 grün; Post-Merge-CI von #160 grün |
+| R6 | Einheitlicher Compile-Hash, kein Binary in der REST-Antwort | – | – |

server/routes/compiler.routes.ts

@@ -55,6 +55,12 @@ function parseCompileRequest(body: unknown): ParsedCompileRequest {
   return { success: true, data: parsedRequest.data };
 }
 
+/** The HEX stays on the server: the client never uses it and it inflates every response. */
+function withoutBinary(result: CompilationResult): CompilationResult {
+  const { binary: _binary, ...clientResult } = result;
+  return clientResult;
+}
+
 function isTimedOutCompileResult(result: CompilationResult): boolean {
   return !result.success && `${result.stderr ?? ""} ${result.errors.map((err) => err.message).join(" ")}`.toLowerCase().includes("timeout");
 }
@@ -183,15 +189,16 @@ export function registerCompilerRoutes(app: Express, deps: CompilerDeps) {
 
       recordCompileMetricIfNeeded(compiler, compileStartTime, result);
 
+      const clientResult = withoutBinary(result);
       if (result.success) {
         if (!cacheDisabled) {
-          compilationCache.set(codeHash, { result, timestamp: Date.now() });
+          compilationCache.set(codeHash, { result: clientResult, timestamp: Date.now() });
           logger.info(`✅ Cached compilation result for code`);
         }
         rememberCompiledCode(res, code);
       }
 
-      res.json(result);
+      res.json(clientResult);
     } catch (error) {
       recordCompileErrorIfNeeded(compiler, compileStartTime, error);
       logger.error(`[Compiler Route] Error during /api/compile: ${error instanceof Error ? error.message : String(error)}`);

server/services/arduino-compiler.ts

@@ -2,7 +2,7 @@
 
 import { mkdtemp, mkdir, writeFile } from "node:fs/promises";
 import { join } from "node:path";
-import { randomUUID, createHash } from "node:crypto";
+import { randomUUID } from "node:crypto";
 import { Logger } from "@shared/logger";
 import { ParserMessage, IOPinRecord } from "@shared/schema";
 import { CodeParser } from "@shared/code-parser";
@@ -29,6 +29,7 @@ import {
 import { processHeaderIncludes } from "./compiler/header-processor";
 import { findUnsafeIncludes } from "./compiler/include-guard";
 import { compileWithArduinoCli, type CLICompileConfig } from "./compiler/cli-runner";
+import { buildSketchHash } from "./workers/compile-worker-utils";
 
 // Re-export for backwards compatibility
 export type { CompilationError } from "./compiler/compiler-output-parser";
@@ -91,20 +92,14 @@ export class ArduinoCompiler {
     return instance;
   }
 
+  /** Same identity as the compile worker, so both paths share cache entries and headers count. */
   private buildSketchHash(
     code: string,
+    headers: Array<{ name: string; content: string }> | undefined,
     options?: CompileRequestOptions,
   ): string {
-    if (options?.sketchHash) {
-      return options.sketchHash;
-    }
-
-    const payload = JSON.stringify({
-      code,
-      fqbn: options?.fqbn || this.defaultFqbn,
-      entryFile: options?.entryFile || "sketch.ino",
-    });
-    return createHash("sha256").update(payload).digest("hex");
+    return options?.sketchHash
+      ?? buildSketchHash({ code, headers, entryFile: options?.entryFile }, options?.fqbn || this.defaultFqbn);
   }
 
   /**
@@ -194,7 +189,7 @@ export class ArduinoCompiler {
     const reservedNameMessages = reservedNamesValidator.validateReservedNames(code);
     const allParserMessages = [...parserMessages, ...reservedNameMessages];
     const ioRegistry: IOPinRecord[] = [];
-    const sketchHash = this.buildSketchHash(code, options);
+    const sketchHash = this.buildSketchHash(code, headers, options);
     const hexCacheDir = options?.hexCacheDir || this.defaultHexCacheDir;
     const compileStartedAt = process.hrtime.bigint();
 

tests/server/routes/compiler-binary-payload.test.ts

@@ -0,0 +1,63 @@
+import express from "express";
+import http from "node:http";
+import type { AddressInfo } from "node:net";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { registerCompilerRoutes } from "../../../server/routes/compiler.routes";
+
+const BINARY = Buffer.from(":100000000C9434000C9446000C9446000C944600A2\n");
+
+describe("compile response payload", () => {
+  let server: http.Server;
+  let baseUrl: string;
+  let compilationCache: Map<string, { result: unknown; timestamp: number }>;
+
+  beforeEach(async () => {
+    const app = express();
+    app.use(express.json());
+    app.use((_req, res, next) => {
+      res.locals.unosimIdentity = { subject: "student-a", roles: ["user"] };
+      next();
+    });
+    compilationCache = new Map();
+    registerCompilerRoutes(app, {
+      compiler: { compile: vi.fn().mockResolvedValue({ success: true, output: "ok", errors: [], arduinoCliStatus: "success", binary: BINARY }) },
+      compilationCache: compilationCache as never,
+      hashCode: (code: string) => `hash:${code}`,
+      CACHE_TTL: 60_000,
+      setLastCompiledCode: vi.fn(),
+      logger: { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() } as never,
+    });
+    server = app.listen(0, "127.0.0.1");
+    await new Promise<void>((resolve) => server.once("listening", resolve));
+    baseUrl = `http://127.0.0.1:${(server.address() as AddressInfo).port}`;
+  });
+
+  afterEach(() => new Promise<void>((resolve) => server.close(() => resolve())));
+
+  async function compile() {
+    const response = await fetch(`${baseUrl}/api/compile`, {
+      method: "POST",
+      headers: { "content-type": "application/json" },
+      body: JSON.stringify({ code: "void setup(){} void loop(){}" }),
+    });
+    return response.json() as Promise<Record<string, unknown>>;
+  }
+
+  it("does not send the compiled binary to the client, fresh or cached", async () => {
+    const fresh = await compile();
+    const cached = await compile();
+
+    expect(fresh).toMatchObject({ success: true, output: "ok" });
+    expect(fresh).not.toHaveProperty("binary");
+    expect(cached).toMatchObject({ success: true, cached: true });
+    expect(cached).not.toHaveProperty("binary");
+  });
+
+  it("does not keep the binary in the in-memory result cache", async () => {
+    await compile();
+
+    expect([...compilationCache.values()]).toEqual([
+      expect.objectContaining({ result: expect.not.objectContaining({ binary: expect.anything() }) }),
+    ]);
+  });
+});

tests/server/services/arduino-compiler-cache-key.test.ts

@@ -0,0 +1,43 @@
+import { randomUUID } from "node:crypto";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { ArduinoCompiler } from "../../../server/services/arduino-compiler";
+import * as cliRunner from "../../../server/services/compiler/cli-runner";
+import { buildSketchHash } from "../../../server/services/workers/compile-worker-utils";
+
+describe("ArduinoCompiler cache key", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it("compiles again when only a header changed", async () => {
+    const cli = vi.spyOn(cliRunner, "compileWithArduinoCli").mockResolvedValue({
+      success: true,
+      output: "Sketch uses 1 bytes.\n\nBoard: Arduino UNO",
+      binary: Buffer.from(":00000001FF\n"),
+    });
+    const compiler = await ArduinoCompiler.create();
+    // Unique code keeps this test independent of earlier on-disk cache entries.
+    const code = `#include "pins.h"\n// ${randomUUID()}\nvoid setup(){}\nvoid loop(){}\n`;
+
+    await compiler.compile(code, [{ name: "pins.h", content: "#define PIN 5" }]);
+    await compiler.compile(code, [{ name: "pins.h", content: "#define PIN 6" }]);
+
+    expect(cli).toHaveBeenCalledTimes(2);
+  });
+
+  it("uses the same sketch identity as the compile worker", async () => {
+    const cli = vi.spyOn(cliRunner, "compileWithArduinoCli").mockResolvedValue({
+      success: true,
+      output: "Board: Arduino UNO",
+      binary: Buffer.from(":00000001FF\n"),
+    });
+    const compiler = await ArduinoCompiler.create();
+    const code = `// ${randomUUID()}\nvoid setup(){}\nvoid loop(){}\n`;
+    const headers = [{ name: "pins.h", content: "#define PIN 5" }];
+
+    await compiler.compile(code, headers, undefined, { sketchHash: buildSketchHash({ code, headers }, "arduino:avr:uno") });
+    await compiler.compile(code, headers);
+
+    expect(cli).toHaveBeenCalledOnce();
+  });
+});

vitest.config.ts

@@ -19,6 +19,7 @@ const serializedHttpUnitTests = [
   "tests/server/cli-label-isolation.test.ts",
   "tests/server/dev-entrypoint.test.ts",
   "tests/server/routes/compiler.routes.test.ts",
+  "tests/server/routes/compiler-binary-payload.test.ts",
   "tests/server/routes/examples.routes.test.ts",
   "tests/server/routes/routes-core.test.ts",
   "tests/server/routes/server-status-observability.test.ts",