ci: pin the review follow-ups of the CI workflow (#33)

- Forbid `continue-on-error` in ci.yml: that mutation left every test green,
  and a failing step would turn the commit status green in silence.
- boot-from-copy.sh refuses a port already held by something else (any curl
  exit other than 7). Without it, the wait loop could reach the neighbour and
  assert on a server it never started — the new test reproduces exactly that
  when the check is removed.
- Pin every require of the COPY'd runtime modules at column 0, with no
  dynamic import: the boot check only calls /health, so it only proves the
  modules loaded at startup.

Fixes #33

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CLAUDE.md

@@ -60,22 +60,22 @@ curl -H "Authorization: Bearer $(cat ~/.coolify-token)" \
 ## Tests
 
 - **CI** : `.forgejo/workflows/ci.yml` (issue #24), sur toute PR (pas de filtre `branches:`, pour que les maillons d'une pile soient batis) et sur `push` vers `main`. Un seul job en `container: node:22-alpine`, la base du Dockerfile : `npm ci`, `npm test`, `bash test-curl.sh`, puis `sh scripts/boot-from-copy.sh`. Verdict a lire sur le statut de commit (`GET /repos/maximus/vps-health-api/commits/<sha>/status`), l'API Actions repondant 404 sur cette instance.
-- `npm test` (vitest) — 308 cas
+- `npm test` (vitest) — 313 cas
   - `__tests__/findings.test.js` — `/defenseurs/findings` (14 cas : auth, validation, filtres severity/category, asymetrie INFO, scan clean vs no_data, JSON corrompu)
   - `__tests__/state.test.js` — `/defenseurs/state` + non-regression `/defenseurs` apres extraction de `serveJsonFile()` (9 cas : corps identique octet pour octet sur un JSON non canonique, no_data, isolation des deux chemins, 401 sans token / mauvais token / token d'ingestion, 404 sur POST)
   - `__tests__/health.test.js` — module `metrics.js` + `/health` (6 cas : exports, payload champ pour champ, garde de latence)
   - `__tests__/auth.test.js` — filet de securite auth sur les routes de lecture (67 cas : matrice route x mode d'echec, en-tetes malformes (dont le separateur `Bearer` autre qu'un espace), quasi-rates du token sur les 6 routes de lecture, fail-closed, **2 cas `404 (not 401)` qui figent l'ordre routage -> auth**, **un espion sur `timingSafeEqual`** qui exige deux empreintes de 32 octets a chaque requete, et **des epingles sur le source** : aucune egalite contre une chaine `Bearer`, un seul `tokenMatches()` et un seul `return null` dans `checkAuth()`, egalites limitees a une liste blanche, `tokenMatches()` qui hache les deux cotes sans test de longueur)
   - `__tests__/hosts.test.js` — matrice exhaustive des endpoints postes (155 cas : regex de route comme controle anti-traversee, 401 avant 403, cloisonnement des deux tokens, 503 fail-closed, reconstruction par liste blanche, 413, `neverSeen`, fraicheur serveur sous horloge figee, degradation de lecture)
   - `__tests__/agent.test.js` — agent poste (50 cas : contrat de payload verifie contre `sanitizeSnapshot()`, une seule tentative sans rejeu, timeout, codes de sortie, wrapper shell, et **le token absent de toute sortie d'echec, fragments de 5 caracteres compris**)
-  - `__tests__/ci.test.js` — epingles du workflow (7 cas : image du `container:` identique au `FROM` du Dockerfile, les quatre etapes presentes, aucun filtre `branches:` sous `pull_request`, `push` limite a `main`, ni `defaults:` ni `working-directory` sur une etape `uses:`, et ce que le boot check suppose du Dockerfile : `USER node`, `CMD` en forme exec)
+  - `__tests__/ci.test.js` — epingles du workflow (12 cas : image du `container:` identique au `FROM` du Dockerfile, les quatre etapes presentes, aucun filtre `branches:` sous `pull_request`, `push` limite a `main`, ni `defaults:` ni `working-directory` sur une etape `uses:`, **aucun `continue-on-error`** (une etape rouge passerait au vert en silence), ce que le boot check suppose du Dockerfile : `USER node`, `CMD` en forme exec, le **refus d'un port deja occupe** (voisin HTTP reel, script lance en `spawn` asynchrone), et **des `require` en colonne 0 seulement, sans `import()`**, dans chaque `.js` du `COPY`)
 - Runtime reste 0-dep ; vitest en devDep uniquement
-- `sh scripts/boot-from-copy.sh` — demarre le `CMD` depuis les seules sources du `COPY`, copiees dans un dossier vide, et exige 401 sans token, 200 avec, et des metriques disque/memoire non nulles. Tient lieu de `docker build` en CI : un vrai build demanderait le socket Docker dans le runner d'instance, donc root sur le VPS de prod pour tout workflow de tout depot. Il n'accepte qu'une ligne `COPY` en forme shell sans option, et un `CMD` en forme exec ; toute autre forme echoue bruyamment plutot que de verifier du vide.
+- `sh scripts/boot-from-copy.sh` — demarre le `CMD` depuis les seules sources du `COPY`, copiees dans un dossier vide, et exige 401 sans token, 200 avec, et des metriques disque/memoire non nulles. Tient lieu de `docker build` en CI : un vrai build demanderait le socket Docker dans le runner d'instance, donc root sur le VPS de prod pour tout workflow de tout depot. Il n'accepte qu'une ligne `COPY` en forme shell sans option, et un `CMD` en forme exec ; toute autre forme echoue bruyamment plutot que de verifier du vide. Port 3097 par defaut (`BOOT_PORT` pour en changer) ; il refuse de demarrer si le port est deja pris (tout code de sortie curl autre que 7, « connexion refusee »), sans quoi ses assertions pourraient interroger un voisin. Il n'appelle que `/health` : il ne prouve que les modules charges au demarrage, d'ou l'epingle « `require` en colonne 0 » dans `ci.test.js`.
 - `bash test-curl.sh` — passe au niveau socket, complementaire et non redondante (45 cas). Vitest couvre la logique ; ce script couvre ce que seuls un vrai client et un vrai TCP peuvent montrer : le 413 rendu **avant** que la connexion soit coupee, les deux tokens qui se refusent mutuellement leurs routes, le fichier servi octet pour octet par `/defenseurs/state`, et le 503 d'une instance demarree sans `HOSTS_INGEST_TOKEN` (elle tourne sur un second port, le temps du script). Il boote ses propres serveurs sur 3099 et 3098 — aucun service exterieur requis.
 
 ## Gotchas
 
 - Pas d'Express — HTTP natif Node.js uniquement
-- Le `COPY` du Dockerfile liste les fichiers un par un (`package.json index.js metrics.js`) : tout nouveau module runtime doit y etre ajoute, sinon le conteneur plante au demarrage sur un `MODULE_NOT_FOUND` que `npm test` ne voit pas (seul `scripts/boot-from-copy.sh`, en CI, le rattrape). Corollaire : `agent/` n'entre PAS dans l'image — c'est du code de poste, livre par copie de fichiers (voir `agent/README.md`), et un test epingle que le `COPY` ne le mentionne jamais.
+- Le `COPY` du Dockerfile liste les fichiers un par un (`package.json index.js metrics.js`) : tout nouveau module runtime doit y etre ajoute, sinon le conteneur plante au demarrage sur un `MODULE_NOT_FOUND` que `npm test` ne voit pas (seul `scripts/boot-from-copy.sh`, en CI, le rattrape). Un `require` place dans un handler de route echapperait au boot check (qui n'appelle que `/health`) : garder tous les `require` au niveau module, une epingle de `ci.test.js` l'exige. Corollaire : `agent/` n'entre PAS dans l'image — c'est du code de poste, livre par copie de fichiers (voir `agent/README.md`), et un test epingle que le `COPY` ne le mentionne jamais.
 - `agent/` tourne sur le ThinkPad, pas sur le VPS, et sa sortie part dans `logger -t host-agent` : tout ce qu'il imprime finit dans `/var/log/syslog` et journald pour de bon. Le chemin d'erreur ne rend donc que `err.code` et le code HTTP — jamais un objet d'erreur, jamais les options de requete, jamais un en-tete. Et jamais de `set -x` dans `run-push.sh` : le shell echoerait le token en sourcant le fichier d'env.
 - `getHealth()` garde `Promise.all([collectMetrics(), getLogtoHealth()])` : l'echantillon CPU de 500 ms et le check Logto (timeout 3 s) sont deliberement concurrents. Les serialiser ferait monter le p99 de `/health` a ~3,5 s. Garde de non-regression : le test de latence dans `__tests__/health.test.js`.
 - L'API n'est plus read-only depuis l'issue #13. `status.json`, `agents-map.json` et les rapports de scan restent ecrits par le Sergent defenseurs et lus seulement ici ; en revanche `POST /hosts/<id>` ecrit dans `HOSTS_DIR`. Consequence : ce montage-la doit etre en lecture-ecriture pour l'uid 1000, contrairement aux montages `/data/defenseurs`.

__tests__/ci.test.js

@@ -1,5 +1,7 @@
 const fs = require("node:fs");
 const path = require("node:path");
+const http = require("node:http");
+const { spawn } = require("node:child_process");
 
 // Pins on the CI workflow and on what the boot check assumes of the Dockerfile.
 // The runtime stays 0-dependency and vitest ships no YAML parser, so these read
@@ -75,6 +77,11 @@ describe("CI workflow", () => {
       if (/^\s*uses:|\n\s*uses:/.test(step)) expect(step).not.toMatch(/working-directory:/);
     }
   });
+  test("never lets a failing step pass", () => {
+    // `continue-on-error: true` turns a red step green without a trace in the
+    // commit status. Anchored on the key position, so only a real key counts.
+    expect(ciCode).not.toMatch(/^\s*(- )?continue-on-error\s*:/m);
+  });
 });
 
 describe("what the boot check assumes of the Dockerfile", () => {
@@ -91,3 +98,63 @@ describe("what the boot check assumes of the Dockerfile", () => {
     expect(argv.length).toBeGreaterThan(0);
   });
 });
+
+// Runs the script with a listener already on its port: without the pre-check,
+// the wait loop could reach this neighbour and assert on a server it never
+// started. Async spawn, so the listener keeps answering while the script runs.
+function runBootScript(env) {
+  return new Promise((resolve, reject) => {
+    const child = spawn("sh", ["scripts/boot-from-copy.sh"], {
+      cwd: ROOT,
+      env: { ...process.env, ...env },
+    });
+    let stderr = "";
+    child.stderr.on("data", (d) => (stderr += d));
+    child.on("error", reject);
+    child.on("close", (code) => resolve({ code, stderr }));
+  });
+}
+
+describe("boot-from-copy.sh", () => {
+  test("refuses a port something else already holds", async () => {
+    const neighbour = http.createServer((req, res) => res.end("not the API"));
+    await new Promise((resolve) => neighbour.listen(0, "127.0.0.1", resolve));
+    try {
+      const { port } = neighbour.address();
+      const { code, stderr } = await runBootScript({ BOOT_PORT: String(port) });
+
+      expect(code).toBe(1);
+      expect(stderr).toMatch(new RegExp(`port ${port} is already taken`));
+    } finally {
+      await new Promise((resolve) => neighbour.close(resolve));
+    }
+  }, 15000);
+});
+
+// The boot check only calls /health, so it proves the existence of the modules
+// loaded at startup, nothing more. A require inside a route handler would load
+// only when that route runs and escape it. Heuristic stand-in for "loaded at
+// startup": every require starts at column 0 (a top-level statement), and there
+// is no dynamic import. Comment lines are skipped, so an explanation quoting
+// require( cannot break the pin.
+describe("runtime modules are all loaded at startup", () => {
+  const copyLine = dockerfile.match(/^\s*COPY\s+(.+)$/m);
+  const sources = copyLine ? copyLine[1].trim().split(/\s+/).slice(0, -1) : [];
+  const runtimeJs = sources.filter((f) => f.endsWith(".js"));
+
+  test("the COPY line lists runtime modules", () => {
+    expect(runtimeJs.length).toBeGreaterThan(0);
+  });
+
+  test.each(runtimeJs)("%s requires only at module level", (file) => {
+    const lines = fs.readFileSync(path.join(ROOT, file), "utf-8").split("\n");
+    const offenders = [];
+    lines.forEach((line, i) => {
+      if (/^\s*(\/\/|\/?\*)/.test(line)) return;
+      if (/\brequire\(/.test(line) && /^\s/.test(line)) offenders.push(`${file}:${i + 1}: ${line.trim()}`);
+      if (/\bimport\(/.test(line)) offenders.push(`${file}:${i + 1}: ${line.trim()}`);
+    });
+
+    expect(offenders).toEqual([]);
+  });
+});

scripts/boot-from-copy.sh

@@ -14,7 +14,9 @@
 # image's own base.
 #
 # Usage : sh scripts/boot-from-copy.sh   (from the repository root)
-# Exit 0 if the server boots and answers, 1 otherwise.
+#         BOOT_PORT=<port> sh scripts/boot-from-copy.sh   (default port 3097)
+# Exit 0 if the server boots and answers, 1 otherwise — including when the port
+# is already held by something else (pick another one with BOOT_PORT).
 
 set -eu
 
@@ -57,6 +59,19 @@ CMD_SH=$(printf '%s' "$CMD_LINE" | node -e '
   });
 ') || fail "CMD must be an exec-form JSON array of plain words: $CMD_LINE"
 
+# --- Port must be free -----------------------------------------------------
+# If something already listens on $PORT, our server dies of EADDRINUSE, but the
+# wait loop below can reach the neighbour first and the assertions would then
+# check a server we never started. So refuse up front: the port counts as free
+# only when curl gets "connection refused" (exit 7). Any other exit — an HTTP
+# answer (0), a listener that accepts and stalls (28), a non-HTTP service
+# (52, 56) — means it is taken. --noproxy keeps an http_proxy from answering
+# in the port's place.
+command -v curl >/dev/null 2>&1 || fail "curl is required"
+RC=0
+curl -s -o /dev/null --noproxy '*' --max-time 2 "http://127.0.0.1:$PORT/" || RC=$?
+[ "$RC" = "7" ] || fail "port $PORT is already taken (curl exit $RC); set BOOT_PORT to a free port"
+
 # --- Build the image's file set ---------------------------------------------
 APP_DIR=$(mktemp -d)
 LOG="$APP_DIR.log"