pjandro/OmniRoute
1
0
Fork 0
mirror of https://github.com/diegosouzapw/OmniRoute.git synced 2026-09-15 19:32:20 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): empty whitelist allows through to prevent admin lockout (#13534)

Browse Source 
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!
This commit is contained in:
Koosha Paridehpour
2026-09-14 19:55:21 -07:00
committed by GitHub
parent 918546acf2
commit fb3f298489
2 changed files with 24 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
open-sse/services/ipFilter.ts
View File

@@ -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" };
}

17
tests/unit/ip-filter.test.ts
View File

@@ -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 ─────────────────────────────────────────────────────