fix(ci): boot protocol E2E on the peer-stamped custom server with preserved open bootstrap (#11535)

changelog.d/fixes/11535-protocol-e2e-peer-stamped-server.md

@@ -0,0 +1 @@
+- **fix(ci):** protocol-clients E2E harness boots the peer-stamped custom server (`run-next.mjs dev`) instead of the bare `next` CLI, so `LOCAL_ONLY` locality resolves from the real TCP peer stamp and `/api/mcp/audit` answers 200 (open bootstrap preserved via an `OMNIROUTE_E2E_BOOTSTRAP_MODE=open` credential clear in `run-next.mjs`, pinned to loopback `HOST`) instead of a deterministic 403 (#11535)

scripts/dev/run-next.mjs

@@ -61,6 +61,25 @@ for (const [key, value] of Object.entries(mergedEnv)) {
   }
 }
 
+// E2E open-mode bootstrap (#11535). Test harnesses that boot THIS server (protocol
+// clients E2E) rely on an auth-disabled "open" bootstrap so management endpoints such
+// as /api/mcp/audit are genuinely exercised unauthenticated (200), not short-circuited
+// by a stray credential. bootstrap-env.mjs deliberately drops empty strings from
+// process.env/.env/server.env, so an INITIAL_PASSWORD="" injected by a harness cannot
+// survive the merge above and any INITIAL_PASSWORD persisted in .env or server.env
+// would leak back in (401 → green-shallow suite).
+// Cleared to EMPTY STRING (not deleted): Next's env loader re-reads the repo .env
+// during app prepare(), AFTER this point — an absent var would be re-populated from
+// the file and src/instrumentation-node.ts would bcrypt-persist it as a real login
+// (401s everywhere). An existing empty var is falsy to every consumer AND wins over
+// dotenv's no-override load, mirroring run-next-playwright.mjs's open-mode overrides.
+// Gated on the test-only env var so production boots are untouched.
+if (process.env.OMNIROUTE_E2E_BOOTSTRAP_MODE === "open") {
+  process.env.INITIAL_PASSWORD = "";
+  process.env.OMNIROUTE_E2E_PASSWORD = "";
+  process.env.OMNIROUTE_API_KEY = "";
+}
+
 // systemd sd_notify (Type=notify / WatchdogSec=): this process owns the
 // watchdog pings — if its event loop blocks (freeze), the pings stop and
 // systemd kills the service. No-op outside systemd (no NOTIFY_SOCKET).

scripts/dev/run-protocol-clients-tests.mjs

@@ -57,10 +57,24 @@ async function main() {
           OMNIROUTE_BASE_URL: baseUrl,
         }),
     OMNIROUTE_E2E_BOOTSTRAP_MODE: process.env.OMNIROUTE_E2E_BOOTSTRAP_MODE || "open",
+    // Pin the custom server's bind address to loopback (#11535): under the
+    // programmatic next() entry the middleware's nextUrl.hostname mirrors the
+    // configured HOST (default "0.0.0.0"), and apiAuth.isLoopbackRequest() reads
+    // nextUrl.hostname FIRST — an unpinned boot makes every request look remote,
+    // so the anonymous open-bootstrap allow never fires (401 green-shallow).
+    HOST: process.env.HOST || "127.0.0.1",
   };
 
   if (!(await isServerReady())) {
-    serverProcess = spawn(process.execPath, ["scripts/dev/run-next-playwright.mjs", "dev"], {
+    // Boot the REAL custom server (run-next.mjs), not the bare `next dev` CLI.
+    // Only the custom Node server stamps the trusted PEER_IP_HEADER from the TCP
+    // socket; without that stamp the authz middleware fails closed on locality and
+    // every LOCAL_ONLY route (e.g. /api/mcp/audit) answers 403 even from loopback
+    // (#11535). run-next.mjs honors OMNIROUTE_E2E_BOOTSTRAP_MODE=open by clearing
+    // bootstrap credentials after its env merge, keeping the audit assertions live
+    // (200) instead of masking them behind a 401. The Playwright webServer runner is
+    // intentionally left untouched — it serves the whole blocking test-e2e suite.
+    serverProcess = spawn(process.execPath, ["scripts/dev/run-next.mjs", "dev"], {
       stdio: "inherit",
       env: testEnv,
     });

tests/unit/protocol-e2e-server-stamping-11535.test.ts

@@ -0,0 +1,98 @@
+/**
+ * Issue #11535 — protocol-clients E2E must boot a peer-stamped, open-bootstrap server.
+ *
+ * The suite's harness (`scripts/dev/run-protocol-clients-tests.mjs`) used to spawn
+ * `run-next-playwright.mjs dev`, which runs the plain `next dev` CLI. That flavour of
+ * server never writes the trusted PEER_IP_HEADER stamp (only the custom Node server in
+ * `run-next.mjs` does), so `resolveStampedPeer()` returns null and the LOCAL_ONLY gate
+ * fails closed → every request to `/api/mcp/audit` answered 403 where the test accepts
+ * 200|401. Swapping the boot target to `run-next.mjs` fixes locality, but would go
+ * "green shallow": `bootstrap-env.mjs` deliberately drops empty strings from env, so an
+ * `.env`/server.env `INITIAL_PASSWORD` would leak back in and the audit block would
+ * silently never execute (401). These source contracts pin both halves of the fix:
+ *
+ *   1. the harness boots `run-next.mjs dev` (peer stamping present on this path);
+ *   2. `run-next.mjs` applies the E2E "open" bootstrap AFTER merging persisted/.env
+ *      credentials into process.env, clearing them so the suite exercises 200.
+ */
+
+import { readFileSync } from "node:fs";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { describe, it } from "node:test";
+import assert from "node:assert/strict";
+
+const here = dirname(fileURLToPath(import.meta.url));
+const repoRoot = join(here, "..", "..");
+const harnessSource = readFileSync(
+  join(repoRoot, "scripts", "dev", "run-protocol-clients-tests.mjs"),
+  "utf8"
+);
+const runNextSource = readFileSync(join(repoRoot, "scripts", "dev", "run-next.mjs"), "utf8");
+
+describe("protocol clients E2E harness (#11535)", () => {
+  it("boots the peer-stamped custom server (run-next.mjs), not the bare next CLI", () => {
+    assert.ok(
+      harnessSource.includes('"scripts/dev/run-next.mjs"'),
+      "harness must spawn scripts/dev/run-next.mjs so requests carry the trusted peer-IP stamp"
+    );
+    assert.ok(
+      !harnessSource.includes("run-next-playwright.mjs"),
+      "harness must not boot via run-next-playwright.mjs (plain next dev has no peer stamping)"
+    );
+  });
+
+  it("still passes the isolated port/data-dir/open-mode env to the spawned server", () => {
+    // The harness relies on these env vars to isolate the run and disable auth;
+    // swapping the boot target must not drop them.
+    for (const key of [
+      "DATA_DIR:",
+      "PORT:",
+      "DASHBOARD_PORT:",
+      "OMNIROUTE_E2E_BOOTSTRAP_MODE",
+      // Under the programmatic next() entry, middleware nextUrl.hostname mirrors the
+      // configured HOST — an unpinned "0.0.0.0" bind makes apiAuth treat loopback
+      // requests as remote, so the open-bootstrap anonymous allow never fires.
+      '"127.0.0.1"',
+    ]) {
+      assert.ok(harnessSource.includes(key), `testEnv must still define ${key}`);
+    }
+  });
+
+  it("run-next.mjs stamps the real TCP peer IP into its request listener", () => {
+    const listenerIdx = runNextSource.indexOf("http.createServer(");
+    const stampIdx = runNextSource.indexOf("stampPeerIp(req)");
+    assert.notEqual(listenerIdx, -1, "custom http.createServer listener expected");
+    assert.notEqual(stampIdx, -1, "stampPeerIp(req) call expected");
+    assert.ok(
+      stampIdx > listenerIdx,
+      "stampPeerIp must be wired inside/after the request listener setup"
+    );
+    assert.ok(runNextSource.includes("ensurePeerStampToken()"), "per-process token required");
+  });
+
+  it("run-next.mjs applies the E2E open-mode credential clear AFTER the env merge", () => {
+    const mergeLoopIdx = runNextSource.indexOf("Object.entries(mergedEnv)");
+    const openModeIdx = runNextSource.indexOf('OMNIROUTE_E2E_BOOTSTRAP_MODE === "open"');
+    assert.notEqual(mergeLoopIdx, -1, "mergedEnv application loop expected");
+    assert.notEqual(openModeIdx, -1, "open-mode bootstrap hook missing");
+
+    const openModeBlock = runNextSource.slice(Math.max(0, openModeIdx - 200));
+    for (const key of ["INITIAL_PASSWORD", "OMNIROUTE_E2E_PASSWORD", "OMNIROUTE_API_KEY"]) {
+      // Empty-string assignment, NOT delete: Next's env loader re-reads the repo .env
+      // during app prepare() (after this hook), so an absent var would be re-populated
+      // from the file and bcrypt-persisted by instrumentation — 401s everywhere.
+      const cleared = new RegExp(`process\\.env\\.${key} = "";`).exec(openModeBlock);
+      assert.ok(cleared, `open mode must set ${key} to "" after the bootstrap merge`);
+      const deleted = new RegExp(`delete process\\.env\\.${key}`).exec(openModeBlock);
+      assert.ok(!deleted, `open mode must not merely delete ${key} (dotenv would restore it)`);
+    }
+    // bootstrap-env.mjs filters empty strings, so the hook must run after the merge
+    // loop that copies persisted/.env values into process.env — otherwise a CI .env
+    // INITIAL_PASSWORD=CHANGEME would survive and the suite would go green shallow.
+    assert.ok(
+      openModeIdx > mergeLoopIdx,
+      "open-mode hook must be positioned after the mergedEnv -> process.env loop"
+    );
+  });
+});