fix(diffs): harden viewer proxy access (#57912)

* fix(diffs): harden viewer proxy access

* fix(diffs): restore mapped loopback access
This commit is contained in:
Agustin Rivera
2026-03-30 13:17:27 -07:00
committed by GitHub
parent 910134b702
commit 30a1690323
4 changed files with 84 additions and 22 deletions

View File

@@ -38,6 +38,8 @@ export default definePluginEntry({
store,
logger: api.logger,
allowRemoteViewer: security.allowRemoteViewer,
trustedProxies: api.config.gateway?.trustedProxies,
allowRealIpFallback: api.config.gateway?.allowRealIpFallback === true,
}),
});
api.on("before_prompt_build", async () => ({

View File

@@ -0,0 +1 @@
export { resolveRequestClientIp } from "openclaw/plugin-sdk/webhook-ingress";

View File

@@ -1,5 +1,6 @@
import type { IncomingMessage, ServerResponse } from "node:http";
import type { PluginLogger } from "../api.js";
import { resolveRequestClientIp } from "../runtime-api.js";
import type { DiffArtifactStore } from "./store.js";
import { DIFF_ARTIFACT_ID_PATTERN, DIFF_ARTIFACT_TOKEN_PATTERN } from "./types.js";
import { VIEWER_ASSET_PREFIX, getServedViewerAsset } from "./viewer-assets.js";
@@ -25,6 +26,8 @@ export function createDiffsHttpHandler(params: {
store: DiffArtifactStore;
logger?: PluginLogger;
allowRemoteViewer?: boolean;
trustedProxies?: readonly string[];
allowRealIpFallback?: boolean;
}) {
const viewerFailureLimiter = new ViewerFailureLimiter();
@@ -42,7 +45,10 @@ export function createDiffsHttpHandler(params: {
return false;
}
const access = resolveViewerAccess(req);
const access = resolveViewerAccess(req, {
trustedProxies: params.trustedProxies,
allowRealIpFallback: params.allowRealIpFallback,
});
if (!access.localRequest && params.allowRemoteViewer !== true) {
respondText(res, 404, "Diff not found");
return true;
@@ -186,12 +192,30 @@ function hasProxyForwardingHints(req: IncomingMessage): boolean {
);
}
function resolveViewerAccess(req: IncomingMessage): {
function resolveViewerAccess(
req: IncomingMessage,
params: {
trustedProxies?: readonly string[];
allowRealIpFallback?: boolean;
},
): {
remoteKey: string;
localRequest: boolean;
} {
const remoteKey = normalizeRemoteClientKey(req.socket?.remoteAddress);
const localRequest = isLoopbackClientIp(remoteKey) && !hasProxyForwardingHints(req);
const proxyHintsPresent = hasProxyForwardingHints(req);
const clientIp =
proxyHintsPresent || (params.trustedProxies?.length ?? 0) > 0
? // Reuse gateway proxy trust rules and fail closed when a trusted proxy hop
// does not provide usable client-origin headers.
resolveRequestClientIp(
req,
params.trustedProxies ? [...params.trustedProxies] : undefined,
params.allowRealIpFallback === true,
)
: req.socket?.remoteAddress;
const remoteKey = normalizeRemoteClientKey(clientIp ?? req.socket?.remoteAddress);
const localRequest =
!proxyHintsPresent && typeof clientIp === "string" && isLoopbackClientIp(remoteKey);
return { remoteKey, localRequest };
}

View File

@@ -308,6 +308,18 @@ describe("createDiffsHttpHandler", () => {
});
it.each([
{
name: "allows direct loopback viewer access by default",
request: localReq,
allowRemoteViewer: false,
expectedStatusCode: 200,
},
{
name: "allows ipv4-mapped ipv6 loopback viewer access by default",
request: ipv4MappedLoopbackReq,
allowRemoteViewer: false,
expectedStatusCode: 200,
},
{
name: "blocks non-loopback viewer access by default",
request: remoteReq,
@@ -321,6 +333,13 @@ describe("createDiffsHttpHandler", () => {
allowRemoteViewer: false,
expectedStatusCode: 404,
},
{
name: "blocks trusted-proxy loopback requests without client-origin headers by default",
request: localReq,
trustedProxies: ["127.0.0.1"],
allowRemoteViewer: false,
expectedStatusCode: 404,
},
{
name: "allows remote access when allowRemoteViewer is enabled",
request: remoteReq,
@@ -331,29 +350,33 @@ describe("createDiffsHttpHandler", () => {
name: "allows proxied loopback requests when allowRemoteViewer is enabled",
request: localReq,
headers: { "x-forwarded-for": "203.0.113.10" },
trustedProxies: ["127.0.0.1"],
allowRemoteViewer: true,
expectedStatusCode: 200,
},
])("$name", async ({ request, headers, allowRemoteViewer, expectedStatusCode }) => {
const artifact = await createViewerArtifact(store);
])(
"$name",
async ({ request, headers, trustedProxies, allowRemoteViewer, expectedStatusCode }) => {
const artifact = await createViewerArtifact(store);
const handler = createDiffsHttpHandler({ store, allowRemoteViewer });
const res = createMockServerResponse();
const handled = await handler(
request({
method: "GET",
url: artifact.viewerPath,
headers,
}),
res,
);
const handler = createDiffsHttpHandler({ store, allowRemoteViewer, trustedProxies });
const res = createMockServerResponse();
const handled = await handler(
request({
method: "GET",
url: artifact.viewerPath,
headers,
}),
res,
);
expect(handled).toBe(true);
expect(res.statusCode).toBe(expectedStatusCode);
if (expectedStatusCode === 200) {
expect(res.body).toBe("<html>viewer</html>");
}
});
expect(handled).toBe(true);
expect(res.statusCode).toBe(expectedStatusCode);
if (expectedStatusCode === 200) {
expect(res.body).toBe("<html>viewer</html>");
}
},
);
it("rate-limits repeated remote misses", async () => {
const handler = createDiffsHttpHandler({ store, allowRemoteViewer: true });
@@ -414,3 +437,15 @@ function remoteReq(input: {
socket: { remoteAddress: "203.0.113.10" },
} as unknown as IncomingMessage;
}
function ipv4MappedLoopbackReq(input: {
method: string;
url: string;
headers?: Record<string, string>;
}): IncomingMessage {
return {
...input,
headers: input.headers ?? {},
socket: { remoteAddress: "::ffff:127.0.0.1" },
} as unknown as IncomingMessage;
}