mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 14:04:46 +00:00
Harden trusted-proxy source validation [AI] (#81290)
* fix: reject local-interface trusted-proxy peers * addressing claude review * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
b7572cc384
commit
26c7da2d02
@@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Harden trusted-proxy source validation [AI]. (#81290) Thanks @pgondhi987.
|
||||
- Agents: add permissive item schemas to array tool parameters before provider submission, preventing OpenAI-compatible schema validation from rejecting plugin tools that omit `items`. Fixes #81175. (#81217) Thanks @JARVIS-Glasses.
|
||||
- Agents: escalate LLM idle watchdog timeouts through profile rotation and configured model fallback instead of leaving agent turns stuck after a silent model stream. Fixes #76877. (#80449) Thanks @jimdawdy-hub.
|
||||
- ACPX: stop forwarding unsupported timeout config options to Claude ACP while preserving OpenClaw's own turn timeout. (#80812) Thanks @sxxtony.
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import os from "node:os";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { makeNetworkInterfacesSnapshot } from "../test-helpers/network-interfaces.js";
|
||||
import { createAuthRateLimiter, type AuthRateLimiter } from "./auth-rate-limit.js";
|
||||
import {
|
||||
assertGatewayAuthConfigured,
|
||||
@@ -558,6 +560,26 @@ describe("gateway auth", () => {
|
||||
});
|
||||
|
||||
describe("trusted-proxy auth", () => {
|
||||
function mockLocalInterfaces(nonLoopbackAddress = "10.0.0.2", family: "IPv4" | "IPv6" = "IPv4") {
|
||||
const spy = vi.isMockFunction(os.networkInterfaces)
|
||||
? vi.mocked(os.networkInterfaces)
|
||||
: vi.spyOn(os, "networkInterfaces");
|
||||
spy.mockReturnValue(
|
||||
makeNetworkInterfacesSnapshot({
|
||||
lo: [{ address: "127.0.0.1", family: "IPv4", internal: true }],
|
||||
eth0: [{ address: nonLoopbackAddress, family }],
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockLocalInterfaces();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
type GatewayConnectInput = Parameters<typeof authorizeGatewayConnect>[0];
|
||||
const trustedProxyConfig = {
|
||||
userHeader: "x-forwarded-user",
|
||||
@@ -602,6 +624,57 @@ describe("trusted-proxy auth", () => {
|
||||
expect(res.user).toBe("nick@example.com");
|
||||
});
|
||||
|
||||
it("rejects trusted-proxy headers from the host non-loopback interface address", async () => {
|
||||
mockLocalInterfaces("10.0.0.1");
|
||||
|
||||
const res = await authorizeTrustedProxy({
|
||||
trustedProxies: ["10.0.0.1"],
|
||||
remoteAddress: "10.0.0.1",
|
||||
headers: {
|
||||
"x-forwarded-user": "nick@example.com",
|
||||
"x-forwarded-proto": "https",
|
||||
"x-openclaw-proxy-auth": "present",
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.reason).toBe("trusted_proxy_local_interface_source");
|
||||
});
|
||||
|
||||
it("rejects trusted-proxy headers when local interface discovery fails", async () => {
|
||||
vi.mocked(os.networkInterfaces).mockImplementation(() => {
|
||||
throw new Error("interface discovery failed");
|
||||
});
|
||||
|
||||
const res = await authorizeTrustedProxy({
|
||||
trustedProxies: ["10.0.0.1"],
|
||||
remoteAddress: "10.0.0.1",
|
||||
headers: {
|
||||
"x-forwarded-user": "nick@example.com",
|
||||
"x-forwarded-proto": "https",
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.reason).toBe("trusted_proxy_local_interface_check_failed");
|
||||
});
|
||||
|
||||
it("rejects trusted-proxy headers from a host IPv6 interface address", async () => {
|
||||
mockLocalInterfaces("fd7a:115c:a1e0::1234", "IPv6");
|
||||
|
||||
const res = await authorizeTrustedProxy({
|
||||
trustedProxies: ["fd7a:115c:a1e0::1234"],
|
||||
remoteAddress: "fd7a:115c:a1e0::1234",
|
||||
headers: {
|
||||
"x-forwarded-user": "nick@example.com",
|
||||
"x-forwarded-proto": "https",
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.reason).toBe("trusted_proxy_local_interface_source");
|
||||
});
|
||||
|
||||
it("rejects trusted-proxy HTTP requests from origins outside the allowlist", async () => {
|
||||
await expect(
|
||||
authorizeHttpGatewayConnect({
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
import { type ResolvedGatewayAuth } from "./auth-resolve.js";
|
||||
import {
|
||||
isLoopbackAddress,
|
||||
resolveLocalInterfaceAddressMatch,
|
||||
resolveRequestClientIp,
|
||||
isTrustedProxyAddress,
|
||||
resolveClientIp,
|
||||
@@ -280,9 +281,19 @@ function authorizeTrustedProxy(params: {
|
||||
if (!remoteAddr || !isTrustedProxyAddress(remoteAddr, trustedProxies)) {
|
||||
return { reason: "trusted_proxy_untrusted_source" };
|
||||
}
|
||||
if (isLoopbackAddress(remoteAddr) && trustedProxyConfig.allowLoopback !== true) {
|
||||
const remoteIsLoopback = isLoopbackAddress(remoteAddr);
|
||||
if (remoteIsLoopback && trustedProxyConfig.allowLoopback !== true) {
|
||||
return { reason: "trusted_proxy_loopback_source" };
|
||||
}
|
||||
if (!remoteIsLoopback) {
|
||||
const localInterfaceMatch = resolveLocalInterfaceAddressMatch(remoteAddr);
|
||||
if (localInterfaceMatch === undefined) {
|
||||
return { reason: "trusted_proxy_local_interface_check_failed" };
|
||||
}
|
||||
if (localInterfaceMatch) {
|
||||
return { reason: "trusted_proxy_local_interface_source" };
|
||||
}
|
||||
}
|
||||
|
||||
const requiredHeaders = trustedProxyConfig.requiredHeaders ?? [];
|
||||
for (const header of requiredHeaders) {
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
__resetContainerCacheForTest,
|
||||
defaultGatewayBindMode,
|
||||
isContainerEnvironment,
|
||||
isLocalInterfaceAddress,
|
||||
isLocalishHost,
|
||||
isLoopbackHost,
|
||||
isPrivateOrLoopbackAddress,
|
||||
@@ -12,6 +13,7 @@ import {
|
||||
isSecureWebSocketUrl,
|
||||
isTrustedProxyAddress,
|
||||
pickPrimaryLanIPv4,
|
||||
resolveLocalInterfaceAddressMatch,
|
||||
resolveClientIp,
|
||||
resolveGatewayBindHost,
|
||||
resolveGatewayListenHosts,
|
||||
@@ -238,6 +240,36 @@ describe("isTrustedProxyAddress", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("isLocalInterfaceAddress", () => {
|
||||
const snapshot = makeNetworkInterfacesSnapshot({
|
||||
lo: [
|
||||
{ address: "127.0.0.1", family: "IPv4", internal: true },
|
||||
{ address: "::1", family: "IPv6", internal: true },
|
||||
],
|
||||
eth0: [{ address: "10.42.0.59", family: "IPv4" }],
|
||||
tailscale0: [{ address: "fd7a:115c:a1e0::1234", family: "IPv6" }],
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ input: "10.42.0.59", expected: true },
|
||||
{ input: "::ffff:10.42.0.59", expected: true },
|
||||
{ input: "fd7a:115c:a1e0::1234", expected: true },
|
||||
{ input: "127.0.0.1", expected: true },
|
||||
{ input: "10.42.0.60", expected: false },
|
||||
{ input: undefined, expected: false },
|
||||
] as const)("returns $expected for $input", ({ input, expected }) => {
|
||||
expect(isLocalInterfaceAddress(input, snapshot)).toBe(expected);
|
||||
});
|
||||
|
||||
it("returns false when interface discovery is unavailable", () => {
|
||||
expect(isLocalInterfaceAddress("10.42.0.59", undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it("reports an indeterminate match when interface discovery is unavailable", () => {
|
||||
expect(resolveLocalInterfaceAddressMatch("10.42.0.59", undefined)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveClientIp", () => {
|
||||
it.each([
|
||||
{
|
||||
|
||||
@@ -8,6 +8,8 @@ import {
|
||||
import {
|
||||
pickMatchingExternalInterfaceAddress,
|
||||
readNetworkInterfaces,
|
||||
safeNetworkInterfaces,
|
||||
type NetworkInterfacesSnapshot,
|
||||
} from "../infra/network-interfaces.js";
|
||||
import { pickPrimaryTailnetIPv4 } from "../infra/tailnet.js";
|
||||
import {
|
||||
@@ -57,6 +59,40 @@ export function isLoopbackAddress(ip: string | undefined): boolean {
|
||||
return isLoopbackIpAddress(ip);
|
||||
}
|
||||
|
||||
export function isLocalInterfaceAddress(
|
||||
ip: string | undefined,
|
||||
snapshot?: NetworkInterfacesSnapshot,
|
||||
): boolean {
|
||||
return (
|
||||
(arguments.length >= 2
|
||||
? resolveLocalInterfaceAddressMatch(ip, snapshot)
|
||||
: resolveLocalInterfaceAddressMatch(ip)) === true
|
||||
);
|
||||
}
|
||||
|
||||
export function resolveLocalInterfaceAddressMatch(
|
||||
ip: string | undefined,
|
||||
snapshot?: NetworkInterfacesSnapshot,
|
||||
): boolean | undefined {
|
||||
const normalized = normalizeIp(ip);
|
||||
if (!normalized) {
|
||||
return false;
|
||||
}
|
||||
const effectiveSnapshot = arguments.length >= 2 ? snapshot : safeNetworkInterfaces();
|
||||
if (!effectiveSnapshot) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
for (const entries of Object.values(effectiveSnapshot)) {
|
||||
for (const entry of entries ?? []) {
|
||||
if (normalizeIp(entry.address) === normalized) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if the IP belongs to a private or loopback network range.
|
||||
* Private ranges: RFC1918, link-local, ULA IPv6, and CGNAT (100.64/10), plus loopback.
|
||||
|
||||
Reference in New Issue
Block a user