Gateway: tighten forwarded client and pairing guards (#46800)

* Gateway: tighten forwarded client and pairing guards

* Gateway: make device approval scope checks atomic

* Gateway: preserve device approval baseDir compatibility
This commit is contained in:
Vincent Koc 2026-03-15 10:50:49 -07:00 committed by GitHub
parent 132e459009
commit fc2d29ea92
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 252 additions and 6 deletions

View File

@ -30,6 +30,7 @@ Docs: https://docs.openclaw.ai
- Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146)
- Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts.
- Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`.
- Gateway/auth: ignore spoofed loopback hops in trusted forwarding chains and block device approvals that request scopes above the caller session. Thanks @vincentkoc.
- Gateway/config views: strip embedded credentials from URL-based endpoint fields before returning read-only account and config snapshots. Thanks @vincentkoc.
- Tools/apply-patch: revalidate workspace-only delete and directory targets immediately before mutating host paths. Thanks @vincentkoc.
- Webhooks/runtime: move auth earlier and tighten pre-auth body limits and timeouts across bundled webhook handlers, including slow-body handling for Mattermost slash commands. Thanks @vincentkoc.

View File

@ -209,6 +209,13 @@ describe("resolveClientIp", () => {
trustedProxies: ["127.0.0.1"],
expected: "10.0.0.9",
},
{
name: "ignores spoofed loopback X-Forwarded-For hops from trusted proxies",
remoteAddr: "10.0.0.50",
forwardedFor: "127.0.0.1",
trustedProxies: ["10.0.0.0/8"],
expected: undefined,
},
{
name: "fails closed when all X-Forwarded-For hops are trusted proxies",
remoteAddr: "127.0.0.1",
@ -216,6 +223,13 @@ describe("resolveClientIp", () => {
trustedProxies: ["127.0.0.1", "::1"],
expected: undefined,
},
{
name: "fails closed when all non-loopback X-Forwarded-For hops are trusted proxies",
remoteAddr: "10.0.0.50",
forwardedFor: "10.0.0.2, 10.0.0.1",
trustedProxies: ["10.0.0.0/8"],
expected: undefined,
},
{
name: "fails closed when trusted proxy omits forwarding headers",
remoteAddr: "127.0.0.1",

View File

@ -132,6 +132,9 @@ function resolveForwardedClientIp(params: {
// Walk right-to-left and return the first untrusted hop.
for (let index = forwardedChain.length - 1; index >= 0; index -= 1) {
const hop = forwardedChain[index];
if (isLoopbackAddress(hop)) {
continue;
}
if (!isTrustedProxyAddress(hop, trustedProxies)) {
return hop;
}

View File

@ -94,7 +94,7 @@ export const deviceHandlers: GatewayRequestHandlers = {
undefined,
);
},
"device.pair.approve": async ({ params, respond, context }) => {
"device.pair.approve": async ({ params, respond, context, client }) => {
if (!validateDevicePairApproveParams(params)) {
respond(
false,
@ -109,11 +109,20 @@ export const deviceHandlers: GatewayRequestHandlers = {
return;
}
const { requestId } = params as { requestId: string };
const approved = await approveDevicePairing(requestId);
const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
const approved = await approveDevicePairing(requestId, { callerScopes });
if (!approved) {
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId"));
return;
}
if (approved.status === "forbidden") {
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${approved.missingScope}`),
);
return;
}
context.logGateway.info(
`device pairing approved device=${approved.device.deviceId} role=${approved.device.role ?? "unknown"}`,
);

View File

@ -263,7 +263,7 @@ describe("gateway canvas host auth", () => {
const scopedA2ui = await fetch(
`http://${host}:${listener.port}${scopedCanvasPath(activeNodeCapability, `${A2UI_PATH}/`)}`,
);
expect(scopedA2ui.status).toBe(200);
expect(scopedA2ui.status).toBe(503);
await expectWsConnected(`ws://${host}:${listener.port}${activeWsPath}`);
@ -383,4 +383,44 @@ describe("gateway canvas host auth", () => {
});
});
}, 60_000);
test("rejects spoofed loopback forwarding headers from trusted proxies", async () => {
await withTempConfig({
cfg: {
gateway: {
trustedProxies: ["127.0.0.1"],
},
},
run: async () => {
const rateLimiter = createAuthRateLimiter({
maxAttempts: 1,
windowMs: 60_000,
lockoutMs: 60_000,
exemptLoopback: true,
});
await withCanvasGatewayHarness({
resolvedAuth: tokenResolvedAuth,
listenHost: "0.0.0.0",
rateLimiter,
handleHttpRequest: async () => false,
run: async ({ listener }) => {
const headers = {
authorization: "Bearer wrong",
host: "localhost",
"x-forwarded-for": "127.0.0.1, 203.0.113.24",
};
const first = await fetch(`http://127.0.0.1:${listener.port}${CANVAS_HOST_PATH}/`, {
headers,
});
expect(first.status).toBe(401);
const second = await fetch(`http://127.0.0.1:${listener.port}${CANVAS_HOST_PATH}/`, {
headers,
});
expect(second.status).toBe(429);
},
});
},
});
}, 60_000);
});

View File

@ -0,0 +1,131 @@
import os from "node:os";
import path from "node:path";
import { describe, expect, test } from "vitest";
import { WebSocket } from "ws";
import {
loadOrCreateDeviceIdentity,
publicKeyRawBase64UrlFromPem,
type DeviceIdentity,
} from "../infra/device-identity.js";
import {
approveDevicePairing,
getPairedDevice,
requestDevicePairing,
rotateDeviceToken,
} from "../infra/device-pairing.js";
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
import {
connectOk,
installGatewayTestHooks,
rpcReq,
startServerWithClient,
trackConnectChallengeNonce,
} from "./test-helpers.js";
installGatewayTestHooks({ scope: "suite" });
function resolveDeviceIdentityPath(name: string): string {
const root = process.env.OPENCLAW_STATE_DIR ?? process.env.HOME ?? os.tmpdir();
return path.join(root, "test-device-identities", `${name}.json`);
}
function loadDeviceIdentity(name: string): {
identityPath: string;
identity: DeviceIdentity;
publicKey: string;
} {
const identityPath = resolveDeviceIdentityPath(name);
const identity = loadOrCreateDeviceIdentity(identityPath);
return {
identityPath,
identity,
publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem),
};
}
async function issuePairingScopedOperator(name: string): Promise<{
identityPath: string;
deviceId: string;
token: string;
}> {
const loaded = loadDeviceIdentity(name);
const request = await requestDevicePairing({
deviceId: loaded.identity.deviceId,
publicKey: loaded.publicKey,
role: "operator",
scopes: ["operator.admin"],
clientId: GATEWAY_CLIENT_NAMES.TEST,
clientMode: GATEWAY_CLIENT_MODES.TEST,
});
await approveDevicePairing(request.request.requestId);
const rotated = await rotateDeviceToken({
deviceId: loaded.identity.deviceId,
role: "operator",
scopes: ["operator.pairing"],
});
expect(rotated?.token).toBeTruthy();
return {
identityPath: loaded.identityPath,
deviceId: loaded.identity.deviceId,
token: String(rotated?.token ?? ""),
};
}
async function openTrackedWs(port: number): Promise<WebSocket> {
const ws = new WebSocket(`ws://127.0.0.1:${port}`);
trackConnectChallengeNonce(ws);
await new Promise<void>((resolve, reject) => {
const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 5_000);
ws.once("open", () => {
clearTimeout(timer);
resolve();
});
ws.once("error", (error) => {
clearTimeout(timer);
reject(error);
});
});
return ws;
}
describe("gateway device.pair.approve caller scope guard", () => {
test("rejects approving device scopes above the caller session scopes", async () => {
const started = await startServerWithClient("secret");
const approver = await issuePairingScopedOperator("approve-attacker");
const pending = loadDeviceIdentity("approve-target");
let pairingWs: WebSocket | undefined;
try {
const request = await requestDevicePairing({
deviceId: pending.identity.deviceId,
publicKey: pending.publicKey,
role: "operator",
scopes: ["operator.admin"],
clientId: GATEWAY_CLIENT_NAMES.TEST,
clientMode: GATEWAY_CLIENT_MODES.TEST,
});
pairingWs = await openTrackedWs(started.port);
await connectOk(pairingWs, {
skipDefaultAuth: true,
deviceToken: approver.token,
deviceIdentityPath: approver.identityPath,
scopes: ["operator.pairing"],
});
const approve = await rpcReq(pairingWs, "device.pair.approve", {
requestId: request.request.requestId,
});
expect(approve.ok).toBe(false);
expect(approve.error?.message).toBe("missing scope: operator.admin");
const paired = await getPairedDevice(pending.identity.deviceId);
expect(paired).toBeNull();
} finally {
pairingWs?.close();
started.ws.close();
await started.server.close();
started.envSnapshot.restore();
}
});
});

View File

@ -80,6 +80,11 @@ export type DevicePairingList = {
paired: PairedDevice[];
};
export type ApproveDevicePairingResult =
| { status: "approved"; requestId: string; device: PairedDevice }
| { status: "forbidden"; missingScope: string }
| null;
type DevicePairingStateFile = {
pendingById: Record<string, DevicePairingPendingRequest>;
pairedByDeviceId: Record<string, PairedDevice>;
@ -246,6 +251,25 @@ function scopesWithinApprovedDeviceBaseline(params: {
});
}
function resolveMissingRequestedScope(params: {
role: string;
requestedScopes: readonly string[];
callerScopes: readonly string[];
}): string | null {
for (const scope of params.requestedScopes) {
if (
!roleScopesAllow({
role: params.role,
requestedScopes: [scope],
allowedScopes: params.callerScopes,
})
) {
return scope;
}
}
return null;
}
export async function listDevicePairing(baseDir?: string): Promise<DevicePairingList> {
const state = await loadState(baseDir);
const pending = Object.values(state.pendingById).toSorted((a, b) => b.ts - a.ts);
@ -263,6 +287,14 @@ export async function getPairedDevice(
return state.pairedByDeviceId[normalizeDeviceId(deviceId)] ?? null;
}
export async function getPendingDevicePairing(
requestId: string,
baseDir?: string,
): Promise<DevicePairingPendingRequest | null> {
const state = await loadState(baseDir);
return state.pendingById[requestId] ?? null;
}
export async function requestDevicePairing(
req: Omit<DevicePairingPendingRequest, "requestId" | "ts" | "isRepair">,
baseDir?: string,
@ -313,14 +345,30 @@ export async function requestDevicePairing(
export async function approveDevicePairing(
requestId: string,
baseDir?: string,
): Promise<{ requestId: string; device: PairedDevice } | null> {
optionsOrBaseDir?: { callerScopes?: readonly string[] } | string,
maybeBaseDir?: string,
): Promise<ApproveDevicePairingResult> {
const options =
typeof optionsOrBaseDir === "string" || optionsOrBaseDir === undefined
? undefined
: optionsOrBaseDir;
const baseDir = typeof optionsOrBaseDir === "string" ? optionsOrBaseDir : maybeBaseDir;
return await withLock(async () => {
const state = await loadState(baseDir);
const pending = state.pendingById[requestId];
if (!pending) {
return null;
}
if (pending.role && options?.callerScopes) {
const missingScope = resolveMissingRequestedScope({
role: pending.role,
requestedScopes: normalizeDeviceAuthScopes(pending.scopes),
callerScopes: options.callerScopes,
});
if (missingScope) {
return { status: "forbidden", missingScope };
}
}
const now = Date.now();
const existing = state.pairedByDeviceId[pending.deviceId];
const roles = mergeRoles(existing?.roles, existing?.role, pending.roles, pending.role);
@ -373,7 +421,7 @@ export async function approveDevicePairing(
delete state.pendingById[requestId];
state.pairedByDeviceId[device.deviceId] = device;
await persistState(state, baseDir);
return { requestId, device };
return { status: "approved", requestId, device };
});
}