mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-20 22:22:57 +03:00
fix(auth): IP blacklist now blocks on direct connections via trusted peer stamp and re-reads config without restart (#9033) (#9385)
Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
9fcefcce9f
commit
5e344a3a99
1
changelog.d/fixes/9033-fix.plan.md
Normal file
1
changelog.d/fixes/9033-fix.plan.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(auth): IP blacklist now blocks on direct connections via trusted peer stamp and re-reads config without restart (#9033)
|
||||
@@ -22,15 +22,16 @@ let _config = {
|
||||
// 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.
|
||||
//
|
||||
// D2 (#9033): the _loaded one-shot gate was removed so a config persisted by the
|
||||
// dashboard settings route (a separate module instance, since @omniroute/open-sse
|
||||
// is bundled per-entry via transpilePackages) propagates to the proxy runtime
|
||||
// without a restart. A DB failure still degrades to the in-memory defaults, and
|
||||
// tempBans remain in-memory-only as before.
|
||||
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 = ?")
|
||||
@@ -235,9 +236,17 @@ export function createIPFilterMiddleware() {
|
||||
|
||||
/**
|
||||
* For Next.js App Router — check IP from request object
|
||||
*
|
||||
* D1 (#9033): accepts an optional trustedPeerIp (resolved from the authenticated
|
||||
* peer stamp, available on direct connections where the proxy runtime has no
|
||||
* socket). When provided, it is checked FIRST before falling through to the
|
||||
* forwarding headers, so a blacklisted IP on a direct connection (no XFF, no
|
||||
* socket) is blocked. When behind a reverse proxy (via-proxy marker set), the
|
||||
* caller passes null so the XFF path continues to work.
|
||||
*/
|
||||
export function checkRequestIP(request) {
|
||||
export function checkRequestIP(request, trustedPeerIp) {
|
||||
const ip =
|
||||
pickFirstValidIp(trustedPeerIp || null) ||
|
||||
pickFirstValidIp(request.headers?.get?.("cf-connecting-ip")) ||
|
||||
pickFirstValidIp(request.headers?.get?.("x-forwarded-for")) ||
|
||||
pickFirstValidIp(request.headers?.get?.("x-real-ip")) ||
|
||||
@@ -329,7 +338,6 @@ function extractClientIP(req) {
|
||||
* Reset config (for testing)
|
||||
*/
|
||||
export function resetIPFilter() {
|
||||
_loaded = false;
|
||||
_config = {
|
||||
enabled: false,
|
||||
mode: "blacklist",
|
||||
|
||||
@@ -8,7 +8,11 @@ import { applyCorsHeaders } from "../cors/origins";
|
||||
import { validateBrowserMutationOrigin } from "../origin/publicOrigin";
|
||||
import { classifyRoute } from "./classify";
|
||||
import { validateDashboardCsrfToken } from "./csrf";
|
||||
import { classifyStampedPeerLocality } from "./peerStamp";
|
||||
import {
|
||||
classifyStampedPeerLocality,
|
||||
resolveStampedPeer,
|
||||
resolveStampedViaProxy,
|
||||
} from "./peerStamp";
|
||||
import { checkRequestIP } from "@omniroute/open-sse/services/ipFilter.ts";
|
||||
import { clientApiPolicy } from "./policies/clientApi";
|
||||
import { managementPolicy } from "./policies/management";
|
||||
@@ -347,8 +351,23 @@ export async function runAuthzPipeline(
|
||||
// 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.
|
||||
//
|
||||
// D1 (#9033): on a direct connection the proxy runtime has no socket, so
|
||||
// checkRequestIP reads only forwarding headers + undefined request.ip and
|
||||
// falls to "unknown", never blocking the blacklisted client. Resolve the
|
||||
// trusted peer IP from the authenticated stamp and pass it to checkRequestIP,
|
||||
// but only when NOT behind a reverse proxy (the via-proxy marker means the
|
||||
// peer IP is the proxy hop, e.g. 127.0.0.1, and the real client is in XFF).
|
||||
if (peerLocality !== "loopback") {
|
||||
const ipVerdict = checkRequestIP(request);
|
||||
const trustedPeerIp = resolveStampedPeer(
|
||||
request.headers.get(PEER_IP_HEADER),
|
||||
process.env.OMNIROUTE_PEER_STAMP_TOKEN
|
||||
);
|
||||
const viaProxy = resolveStampedViaProxy(
|
||||
request.headers.get(VIA_PROXY_HEADER),
|
||||
process.env.OMNIROUTE_PEER_STAMP_TOKEN
|
||||
);
|
||||
const ipVerdict = checkRequestIP(request, viaProxy ? null : trustedPeerIp);
|
||||
if (!ipVerdict.allowed) {
|
||||
const blocked = NextResponse.json(
|
||||
{ error: ipVerdict.reason || "Access denied" },
|
||||
|
||||
@@ -108,7 +108,7 @@ export const resetStatsActionSchema = z.object({
|
||||
action: z.literal("reset-stats"),
|
||||
});
|
||||
|
||||
export const ipFilterModeSchema = z.enum(["blacklist", "whitelist"]);
|
||||
export const ipFilterModeSchema = z.enum(["blacklist", "whitelist", "whitelist-priority"]);
|
||||
|
||||
export const tempBanSchema = z.object({
|
||||
ip: z.string().trim().min(1),
|
||||
|
||||
@@ -55,11 +55,7 @@ test("D1: blacklisted IP is blocked on a DIRECT connection (trusted peer stamp,
|
||||
{ enforce: true }
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
res.status,
|
||||
403,
|
||||
`direct blacklisted IP must be blocked, got status=${res.status}`
|
||||
);
|
||||
assert.equal(res.status, 403, `direct blacklisted IP must be blocked, got status=${res.status}`);
|
||||
});
|
||||
|
||||
test("D2: persisted config written after first load is honored WITHOUT restart", async () => {
|
||||
@@ -74,9 +70,7 @@ test("D2: persisted config written after first load is honored WITHOUT restart",
|
||||
// Now simulate a "settings route" write: write directly to the DB key_value table
|
||||
// with a DIFFERENT config (e.g. empty blacklist, effectively "allow all").
|
||||
const db = core.getDbInstance();
|
||||
db.prepare(
|
||||
"INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)"
|
||||
).run(
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
"ipFilter",
|
||||
"config",
|
||||
JSON.stringify({ enabled: true, mode: "blacklist", blacklist: [], whitelist: [] })
|
||||
@@ -116,13 +110,11 @@ test("D3: behind reverse proxy (peer stamp=loopback + via-proxy marker + XFF=bla
|
||||
});
|
||||
|
||||
test("Bonus: ipFilterModeSchema accepts whitelist-priority", async () => {
|
||||
const { ipFilterModeSchema } = await import(
|
||||
"../../../src/shared/validation/schemas/misc.ts"
|
||||
);
|
||||
const { ipFilterModeSchema } = await import("../../../src/shared/validation/schemas/misc.ts");
|
||||
const result = ipFilterModeSchema.safeParse("whitelist-priority");
|
||||
assert.equal(
|
||||
result.success,
|
||||
true,
|
||||
`ipFilterModeSchema must accept "whitelist-priority", got: ${JSON.stringify(result)}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user