fix(security): persist IP filter config + enforce it in the authz pipeline (#6131) (#6132)

Integrated into release/v3.8.44 — IP filter persistence + authz-pipeline enforcement (closes #6131). HARD-neutro: validate-release-green on the merge shows the same 3 pre-existing base-reds as the release baseline (test-masking cycle-wide, unit red-herring, integration batch-E2E env); #6131's own tests + ip-filter/pipeline suites all green.

open-sse/services/ipFilter.ts

@@ -5,6 +5,7 @@
  */
 
 import { isIP } from "node:net";
+import { getDbInstance } from "../../src/lib/db/core.ts";
 
 // In-memory IP lists
 let _config = {
@@ -15,6 +16,57 @@ let _config = {
   tempBans: new Map(),
 };
 
+// Persistence (#6131): the config used to live in memory only, so every restart
+// (i.e. every OmniRoute update) reset it to Disabled + empty lists. It is now
+// persisted to the key_value table (namespace 'ipFilter', key 'config') and
+// lazily loaded on first access. better-sqlite3 is synchronous, so both the load
+// and the save stay in the sync hot path without extra startup wiring. tempBans
+// are intentionally NOT persisted — they are ephemeral, TTL-swept runtime state.
+const IP_FILTER_NAMESPACE = "ipFilter";
+const IP_FILTER_KEY = "config";
+let _loaded = false;
+
+function ensureLoaded() {
+  if (_loaded) return;
+  // Mark loaded up-front so a DB failure (build phase / cloud / migration not yet
+  // run) degrades to in-memory only instead of retrying on every request.
+  _loaded = true;
+  try {
+    const row = getDbInstance()
+      .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
+      .get(IP_FILTER_NAMESPACE, IP_FILTER_KEY) as { value?: string } | undefined;
+    if (!row?.value) return;
+    const parsed = JSON.parse(row.value) as {
+      enabled?: boolean;
+      mode?: string;
+      blacklist?: string[];
+      whitelist?: string[];
+    };
+    _config.enabled = parsed.enabled === true;
+    if (typeof parsed.mode === "string") _config.mode = parsed.mode;
+    _config.blacklist = new Set(Array.isArray(parsed.blacklist) ? parsed.blacklist : []);
+    _config.whitelist = new Set(Array.isArray(parsed.whitelist) ? parsed.whitelist : []);
+  } catch {
+    // No DB / table yet — keep the in-memory defaults.
+  }
+}
+
+function persist() {
+  try {
+    const payload = JSON.stringify({
+      enabled: _config.enabled,
+      mode: _config.mode,
+      blacklist: Array.from(_config.blacklist),
+      whitelist: Array.from(_config.whitelist),
+    });
+    getDbInstance()
+      .prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)")
+      .run(IP_FILTER_NAMESPACE, IP_FILTER_KEY, payload);
+  } catch {
+    // Best-effort persistence: never let a DB write failure break the request path.
+  }
+}
+
 const _tempBanSweep = setInterval(() => {
   const now = Date.now();
   const bans = _config.tempBans as Map<string, { until: number; reason: string }>;
@@ -30,16 +82,19 @@ if (typeof _tempBanSweep === "object" && "unref" in _tempBanSweep) {
  * Configure the IP filter
  */
 export function configureIPFilter(config) {
+  ensureLoaded();
   if (config.enabled !== undefined) _config.enabled = config.enabled;
   if (config.mode) _config.mode = config.mode;
   if (config.blacklist) _config.blacklist = new Set(config.blacklist);
   if (config.whitelist) _config.whitelist = new Set(config.whitelist);
+  persist();
 }
 
 /**
  * Get current IP filter config (for API)
  */
 export function getIPFilterConfig() {
+  ensureLoaded();
   return {
     enabled: _config.enabled,
     mode: _config.mode,
@@ -60,6 +115,7 @@ export function getIPFilterConfig() {
  * @returns {{ allowed: boolean, reason?: string }}
  */
 export function checkIP(ip) {
+  ensureLoaded();
   if (!_config.enabled) return { allowed: true };
   if (!ip) return { allowed: true };
 
@@ -124,28 +180,36 @@ export function removeTempBan(ip) {
  * Add IP to blacklist
  */
 export function addToBlacklist(ip) {
+  ensureLoaded();
   _config.blacklist.add(normalizeIP(ip));
+  persist();
 }
 
 /**
  * Remove IP from blacklist
  */
 export function removeFromBlacklist(ip) {
+  ensureLoaded();
   _config.blacklist.delete(normalizeIP(ip));
+  persist();
 }
 
 /**
  * Add IP to whitelist
  */
 export function addToWhitelist(ip) {
+  ensureLoaded();
   _config.whitelist.add(normalizeIP(ip));
+  persist();
 }
 
 /**
  * Remove IP from whitelist
  */
 export function removeFromWhitelist(ip) {
+  ensureLoaded();
   _config.whitelist.delete(normalizeIP(ip));
+  persist();
 }
 
 /**
@@ -265,6 +329,7 @@ function extractClientIP(req) {
  * Reset config (for testing)
  */
 export function resetIPFilter() {
+  _loaded = false;
   _config = {
     enabled: false,
     mode: "blacklist",

src/server/authz/pipeline.ts

@@ -9,6 +9,7 @@ import { validateBrowserMutationOrigin } from "../origin/publicOrigin";
 import { classifyRoute } from "./classify";
 import { validateDashboardCsrfToken } from "./csrf";
 import { classifyStampedPeerLocality } from "./peerStamp";
+import { checkRequestIP } from "@omniroute/open-sse/services/ipFilter.ts";
 import { clientApiPolicy } from "./policies/clientApi";
 import { managementPolicy } from "./policies/management";
 import { publicPolicy } from "./policies/public";
@@ -287,14 +288,12 @@ export async function runAuthzPipeline(
   // to "remote" so the LOCAL_ONLY gate is not bypassed by a request arriving
   // through an external reverse proxy (nginx / Caddy / Cloudflare Tunnel).
   // See peerStamp.ts and the upstream da667836 reference for the full rationale.
-  requestHeaders.set(
-    AUTHZ_HEADER_PEER_LOCALITY,
-    classifyStampedPeerLocality(
-      request.headers.get(PEER_IP_HEADER),
-      request.headers.get(VIA_PROXY_HEADER),
-      process.env.OMNIROUTE_PEER_STAMP_TOKEN
-    )
+  const peerLocality = classifyStampedPeerLocality(
+    request.headers.get(PEER_IP_HEADER),
+    request.headers.get(VIA_PROXY_HEADER),
+    process.env.OMNIROUTE_PEER_STAMP_TOKEN
   );
+  requestHeaders.set(AUTHZ_HEADER_PEER_LOCALITY, peerLocality);
 
   if (method === "OPTIONS") {
     const preflight = new NextResponse(null, { status: 204 });
@@ -312,6 +311,23 @@ export async function runAuthzPipeline(
     return response;
   }
 
+  // IP filter (#6131): enforce the operator's IP blacklist/whitelist on the
+  // external surface. Loopback is exempt so the local operator can never lock
+  // themselves out of the dashboard (they can always fix the list from
+  // localhost). checkIP is a no-op when the filter is disabled.
+  if (peerLocality !== "loopback") {
+    const ipVerdict = checkRequestIP(request);
+    if (!ipVerdict.allowed) {
+      const blocked = NextResponse.json(
+        { error: ipVerdict.reason || "Access denied" },
+        { status: 403 }
+      );
+      stampRouteResponse(blocked, requestId, classification.routeClass);
+      applyCorsHeaders(blocked, request, corsRelaxOrigin);
+      return blocked;
+    }
+  }
+
   const policy = POLICIES[classification.routeClass];
   const outcome = await policy.evaluate({ request, classification, requestId });
 

tests/unit/authz/ip-filter-enforcement-6131.test.ts

@@ -0,0 +1,92 @@
+// Regression for #6131 (Part B — enforcement): the IP blacklist was never wired
+// into the request pipeline, so blacklisted IPs were not actually blocked. This
+// locks that runAuthzPipeline blocks a blacklisted client IP with 403 before the
+// route policy runs, allows a clean IP through to the normal auth outcome, and
+// exempts loopback so the local operator can never lock themselves out.
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { NextRequest } from "next/server";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ipenforce-6131-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.JWT_SECRET = "test-secret-6131";
+
+const core = await import("../../../src/lib/db/core.ts");
+const ipFilter = await import("../../../open-sse/services/ipFilter.ts");
+const pipeline = await import("../../../src/server/authz/pipeline.ts");
+
+const ORIGINAL_STAMP_TOKEN = process.env.OMNIROUTE_PEER_STAMP_TOKEN;
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  if (ORIGINAL_STAMP_TOKEN === undefined) delete process.env.OMNIROUTE_PEER_STAMP_TOKEN;
+  else process.env.OMNIROUTE_PEER_STAMP_TOKEN = ORIGINAL_STAMP_TOKEN;
+});
+
+test.beforeEach(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+  ipFilter.resetIPFilter();
+  delete process.env.OMNIROUTE_PEER_STAMP_TOKEN;
+});
+
+const BLOCKED = "203.0.113.9";
+const CLEAN = "203.0.113.10";
+
+function req(xff: string, extraHeaders: Record<string, string> = {}) {
+  return new NextRequest("http://localhost/v1/models", {
+    headers: { "x-forwarded-for": xff, ...extraHeaders },
+  });
+}
+
+async function isIpBlocked(res: Response): Promise<boolean> {
+  if (res.status !== 403) return false;
+  const body = (await res.clone().json()) as { error?: unknown };
+  // The IP-filter block returns a plain string error; policy 403s use a nested object.
+  return typeof body.error === "string" && /blacklist|not in whitelist|banned/i.test(body.error);
+}
+
+test("#6131 blacklisted remote IP is blocked with 403 before the route policy", async () => {
+  ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" });
+  ipFilter.addToBlacklist(BLOCKED);
+
+  const res = await pipeline.runAuthzPipeline(req(BLOCKED), { enforce: true });
+  assert.equal(res.status, 403);
+  assert.equal(await isIpBlocked(res), true, "expected the IP-filter 403 block");
+});
+
+test("#6131 a clean remote IP passes the IP filter (reaches the normal auth outcome)", async () => {
+  ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" });
+  ipFilter.addToBlacklist(BLOCKED);
+
+  const res = await pipeline.runAuthzPipeline(req(CLEAN), { enforce: true });
+  // The IP filter must let it through to the normal route/auth outcome, whatever
+  // that is (the point is: NOT the IP-filter 403 block).
+  assert.equal(await isIpBlocked(res), false, "clean IP must not be blocked by the IP filter");
+});
+
+test("#6131 disabled filter never blocks (even a listed IP)", async () => {
+  ipFilter.configureIPFilter({ enabled: false, mode: "blacklist" });
+  ipFilter.addToBlacklist(BLOCKED);
+
+  const res = await pipeline.runAuthzPipeline(req(BLOCKED), { enforce: true });
+  assert.equal(await isIpBlocked(res), false, "disabled filter must not block");
+});
+
+test("#6131 loopback is exempt — operator can't lock themselves out locally", async () => {
+  process.env.OMNIROUTE_PEER_STAMP_TOKEN = "stamp-tok";
+  ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" });
+  ipFilter.addToBlacklist(BLOCKED);
+
+  // A trusted stamped loopback peer IP downgrades the request to "loopback".
+  const res = await pipeline.runAuthzPipeline(
+    req(BLOCKED, { "x-omniroute-peer-ip": "stamp-tok|127.0.0.1" }),
+    { enforce: true }
+  );
+  assert.equal(await isIpBlocked(res), false, "loopback must be exempt from the IP filter");
+});

tests/unit/ip-filter-persistence-6131.test.ts

@@ -0,0 +1,96 @@
+// Regression for #6131: the IP filter config lived in memory only, so every
+// restart (i.e. every OmniRoute update) reset it to Disabled + empty lists and
+// blacklisted IPs were never actually blocked. This locks the fix:
+//   1. configure/blacklist persists to the DB and survives a simulated restart;
+//   2. after the restart the blacklisted IP is still blocked by checkIP.
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ipfilter-6131-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const core = await import("../../src/lib/db/core.ts");
+const ipFilter = await import("../../open-sse/services/ipFilter.ts");
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+test.beforeEach(() => {
+  // Fresh DB per test + fresh in-memory module state.
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+  ipFilter.resetIPFilter();
+});
+
+// Simulate an OmniRoute restart: the module's in-memory state is wiped (as it
+// would be on a fresh import) but the DB file persists — exactly what happens
+// across an update/restart.
+function simulateRestart() {
+  ipFilter.resetIPFilter();
+}
+
+test("#6131 blacklist + enabled survive a restart (persisted to DB)", () => {
+  ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" });
+  ipFilter.addToBlacklist("203.0.113.7");
+  ipFilter.addToBlacklist("198.51.100.42");
+
+  simulateRestart();
+
+  const cfg = ipFilter.getIPFilterConfig();
+  assert.equal(cfg.enabled, true, "enabled must persist across restart");
+  assert.equal(cfg.mode, "blacklist");
+  assert.deepEqual(cfg.blacklist.sort(), ["198.51.100.42", "203.0.113.7"]);
+});
+
+test("#6131 blacklisted IP is still blocked after a restart", () => {
+  ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" });
+  ipFilter.addToBlacklist("203.0.113.7");
+
+  simulateRestart();
+
+  assert.equal(ipFilter.checkIP("203.0.113.7").allowed, false);
+  assert.equal(ipFilter.checkIP("203.0.113.8").allowed, true);
+});
+
+test("#6131 removing an IP and disabling also persist across restart", () => {
+  ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" });
+  ipFilter.addToBlacklist("203.0.113.7");
+  ipFilter.addToBlacklist("203.0.113.8");
+  ipFilter.removeFromBlacklist("203.0.113.7");
+  ipFilter.configureIPFilter({ enabled: false });
+
+  simulateRestart();
+
+  const cfg = ipFilter.getIPFilterConfig();
+  assert.equal(cfg.enabled, false);
+  assert.deepEqual(cfg.blacklist, ["203.0.113.8"]);
+  // Disabled → everything allowed regardless of the persisted blacklist.
+  assert.equal(ipFilter.checkIP("203.0.113.8").allowed, true);
+});
+
+test("#6131 whitelist mode persists across restart", () => {
+  ipFilter.configureIPFilter({ enabled: true, mode: "whitelist" });
+  ipFilter.addToWhitelist("203.0.113.7");
+
+  simulateRestart();
+
+  const cfg = ipFilter.getIPFilterConfig();
+  assert.equal(cfg.mode, "whitelist");
+  assert.deepEqual(cfg.whitelist, ["203.0.113.7"]);
+  assert.equal(ipFilter.checkIP("203.0.113.7").allowed, true);
+  assert.equal(ipFilter.checkIP("10.0.0.1").allowed, false);
+});
+
+test("#6131 defaults are safe when nothing was ever persisted (disabled, allow-all)", () => {
+  simulateRestart();
+  const cfg = ipFilter.getIPFilterConfig();
+  assert.equal(cfg.enabled, false);
+  assert.deepEqual(cfg.blacklist, []);
+  assert.equal(ipFilter.checkIP("203.0.113.7").allowed, true);
+});

tests/unit/ip-filter.test.ts

@@ -1,6 +1,16 @@
 import test from "node:test";
 import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
 
+// Isolate DATA_DIR before importing ipFilter: since #6131 the filter lazily
+// loads/persists its config to the DB, so an un-isolated run would touch the
+// real ~/.omniroute DB (side-effect + WAL-lock flake). Pin a throwaway dir.
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ipfilter-unit-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const core = await import("../../src/lib/db/core.ts");
 const {
   checkIP,
   configureIPFilter,
@@ -15,7 +25,17 @@ const {
   resetIPFilter,
 } = await import("../../open-sse/services/ipFilter.ts");
 
-test.beforeEach(() => resetIPFilter());
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+test.beforeEach(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+  resetIPFilter();
+});
 
 // ─── Disabled ───────────────────────────────────────────────────────────────