From c283f87ab06c713960a84a6e94c0a338d0e0189a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 11:34:56 +0100 Subject: [PATCH] refactor: clarify strict loopback proxy audit rules --- src/security/audit.test.ts | 52 +++++++++++++++++++------------------- src/security/audit.ts | 15 +++++------ 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index c8703341ccb..02060d49d7b 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -974,6 +974,20 @@ describe("security audit", () => { }); it("scores X-Real-IP fallback risk by gateway exposure", async () => { + const trustedProxyCfg = (trustedProxies: string[]): OpenClawConfig => ({ + gateway: { + bind: "loopback", + allowRealIpFallback: true, + trustedProxies, + auth: { + mode: "trusted-proxy", + trustedProxy: { + userHeader: "x-forwarded-user", + }, + }, + }, + }); + const cases: Array<{ name: string; cfg: OpenClawConfig; @@ -1011,36 +1025,22 @@ describe("security audit", () => { }, { name: "loopback trusted-proxy with loopback-only proxies", - cfg: { - gateway: { - bind: "loopback", - allowRealIpFallback: true, - trustedProxies: ["127.0.0.1"], - auth: { - mode: "trusted-proxy", - trustedProxy: { - userHeader: "x-forwarded-user", - }, - }, - }, - }, + cfg: trustedProxyCfg(["127.0.0.1"]), expectedSeverity: "warn", }, { name: "loopback trusted-proxy with non-loopback proxy range", - cfg: { - gateway: { - bind: "loopback", - allowRealIpFallback: true, - trustedProxies: ["127.0.0.1", "10.0.0.0/8"], - auth: { - mode: "trusted-proxy", - trustedProxy: { - userHeader: "x-forwarded-user", - }, - }, - }, - }, + cfg: trustedProxyCfg(["127.0.0.1", "10.0.0.0/8"]), + expectedSeverity: "critical", + }, + { + name: "loopback trusted-proxy with 127.0.0.2", + cfg: trustedProxyCfg(["127.0.0.2"]), + expectedSeverity: "critical", + }, + { + name: "loopback trusted-proxy with 127.0.0.0/8 range", + cfg: trustedProxyCfg(["127.0.0.0/8"]), expectedSeverity: "critical", }, ]; diff --git a/src/security/audit.ts b/src/security/audit.ts index c02191cf32e..651fb619b25 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -9,7 +9,6 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolveConfigPath, resolveStateDir } from "../config/paths.js"; import { resolveGatewayAuth } from "../gateway/auth.js"; import { buildGatewayConnectionDetails } from "../gateway/call.js"; -import { isLoopbackAddress } from "../gateway/net.js"; import { resolveGatewayProbeAuth } from "../gateway/probe-auth.js"; import { probeGateway } from "../gateway/probe.js"; import { collectChannelSecurityFindings } from "./audit-channel.js"; @@ -340,7 +339,7 @@ function collectGatewayConfigFindings( if (allowRealIpFallback) { const hasNonLoopbackTrustedProxy = trustedProxies.some( - (proxy) => !isLoopbackOnlyTrustedProxyEntry(proxy), + (proxy) => !isStrictLoopbackTrustedProxyEntry(proxy), ); const exposed = bind !== "loopback" || (auth.mode === "trusted-proxy" && hasNonLoopbackTrustedProxy); @@ -508,13 +507,15 @@ function collectGatewayConfigFindings( return findings; } -function isLoopbackOnlyTrustedProxyEntry(entry: string): boolean { +// Keep this stricter than isLoopbackAddress on purpose: this check is for +// trust boundaries, so only explicit localhost proxy hops are treated as local. +function isStrictLoopbackTrustedProxyEntry(entry: string): boolean { const candidate = entry.trim(); if (!candidate) { return false; } if (!candidate.includes("/")) { - return isLoopbackAddress(candidate); + return candidate === "127.0.0.1" || candidate.toLowerCase() === "::1"; } const [rawIp, rawPrefix] = candidate.split("/", 2); @@ -527,11 +528,7 @@ function isLoopbackOnlyTrustedProxyEntry(entry: string): boolean { return false; } if (ipVersion === 4) { - if (prefix < 8 || prefix > 32) { - return false; - } - const firstOctet = Number.parseInt(rawIp.trim().split(".")[0] ?? "", 10); - return firstOctet === 127; + return rawIp.trim() === "127.0.0.1" && prefix === 32; } if (ipVersion === 6) { return prefix === 128 && rawIp.trim().toLowerCase() === "::1";