From fb3f298489f48cca02faf3ba7c08bdf0a3d8ae40 Mon Sep 17 00:00:00 2001 From: Koosha Paridehpour <42529354+KooshaPari@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:55:21 -0700 Subject: [PATCH] fix(security): empty whitelist allows through to prevent admin lockout (#13534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In `whitelist` mode, an empty whitelist now lets requests through instead of blocking everyone. Switching to whitelist mode before adding any IP no longer locks the admin out of the dashboard; enforcement starts with the first entry (#13176). Approved by the maintainer as a policy change. Maintainer note: the existing `addToWhitelist/removeFromWhitelist: dynamic updates` test asserted the old fail-closed contract (remove the only entry → blocked). It now uses two entries to keep checking that removal blocks, plus an explicit assertion that removing the last entry returns to the new open state. That is the operator-visible consequence of this change: clearing the list disables enforcement. IP-filter suites: 53 cases green. Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green. Thanks @KooshaPari! --- open-sse/services/ipFilter.ts | 8 +++++++- tests/unit/ip-filter.test.ts | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/open-sse/services/ipFilter.ts b/open-sse/services/ipFilter.ts index e54930c5b1..7516d56546 100644 --- a/open-sse/services/ipFilter.ts +++ b/open-sse/services/ipFilter.ts @@ -133,7 +133,13 @@ export function checkIP(ip) { switch (_config.mode) { case "whitelist": - // Only whitelisted IPs allowed + // Only whitelisted IPs allowed — but if the whitelist is empty the admin + // has just switched to this mode and has not added IPs yet. Allow the + // request through so the admin can still reach the dashboard to populate + // the list. Once at least one entry exists, enforcement kicks in. + if (_config.whitelist.size === 0) { + return { allowed: true }; + } if (!matchesAny(normalizedIP, _config.whitelist)) { return { allowed: false, reason: "IP not in whitelist" }; } diff --git a/tests/unit/ip-filter.test.ts b/tests/unit/ip-filter.test.ts index 77bad1ea64..9e02abad82 100644 --- a/tests/unit/ip-filter.test.ts +++ b/tests/unit/ip-filter.test.ts @@ -77,6 +77,18 @@ test("whitelist: CIDR match", () => { assert.equal(checkIP("11.0.0.1").allowed, false); }); +test("whitelist: empty whitelist allows all IPs (admin can still reach dashboard)", () => { + configureIPFilter({ enabled: true, mode: "whitelist", whitelist: [] }); + assert.equal(checkIP("1.2.3.4").allowed, true); + assert.equal(checkIP("5.6.7.8").allowed, true); +}); + +test("whitelist: non-empty whitelist blocks unlisted IPs", () => { + configureIPFilter({ enabled: true, mode: "whitelist", whitelist: ["1.2.3.4"] }); + assert.equal(checkIP("1.2.3.4").allowed, true); + assert.equal(checkIP("5.6.7.8").allowed, false); +}); + // ─── Whitelist Priority Mode ──────────────────────────────────────────────── test("whitelist-priority: whitelist overrides blacklist", () => { @@ -120,9 +132,14 @@ test("addToBlacklist/removeFromBlacklist: dynamic updates", () => { test("addToWhitelist/removeFromWhitelist: dynamic updates", () => { configureIPFilter({ enabled: true, mode: "whitelist" }); addToWhitelist("1.1.1.1"); + addToWhitelist("2.2.2.2"); assert.equal(checkIP("1.1.1.1").allowed, true); removeFromWhitelist("1.1.1.1"); assert.equal(checkIP("1.1.1.1").allowed, false); + // #13534: removing the last entry leaves an empty whitelist, which no longer + // enforces (so an admin who has not populated the list yet is not locked out). + removeFromWhitelist("2.2.2.2"); + assert.equal(checkIP("1.1.1.1").allowed, true); }); // ─── IPv6 Normalization ─────────────────────────────────────────────────────