fix(security): fail-close node camera URL downloads

This commit is contained in:
Peter Steinberger
2026-03-02 23:23:30 +00:00
parent 7365aefa19
commit 3bf19d6f40
9 changed files with 302 additions and 74 deletions

View File

@@ -95,22 +95,39 @@ describe("nodes camera helpers", () => {
it("writes camera clip payload from url", async () => {
stubFetchResponse(new Response("url-clip", { status: 200 }));
await withCameraTempDir(async (dir) => {
const expectedHost = "198.51.100.42";
const out = await writeCameraClipPayloadToFile({
payload: {
format: "mp4",
url: "https://example.com/clip.mp4",
url: `https://${expectedHost}/clip.mp4`,
durationMs: 200,
hasAudio: false,
},
facing: "back",
tmpDir: dir,
id: "clip2",
expectedHost,
});
expect(out).toBe(path.join(dir, "openclaw-camera-clip-back-clip2.mp4"));
await expect(fs.readFile(out, "utf8")).resolves.toBe("url-clip");
});
});
it("rejects camera clip url payloads without node remoteIp", async () => {
stubFetchResponse(new Response("url-clip", { status: 200 }));
await expect(
writeCameraClipPayloadToFile({
payload: {
format: "mp4",
url: "https://198.51.100.42/clip.mp4",
durationMs: 200,
hasAudio: false,
},
facing: "back",
}),
).rejects.toThrow(/node remoteip/i);
});
it("writes base64 to file", async () => {
await withCameraTempDir(async (dir) => {
const out = path.join(dir, "x.bin");
@@ -127,11 +144,22 @@ describe("nodes camera helpers", () => {
stubFetchResponse(new Response("url-content", { status: 200 }));
await withCameraTempDir(async (dir) => {
const out = path.join(dir, "x.bin");
await writeUrlToFile(out, "https://example.com/clip.mp4");
await writeUrlToFile(out, "https://198.51.100.42/clip.mp4", {
expectedHost: "198.51.100.42",
});
await expect(fs.readFile(out, "utf8")).resolves.toBe("url-content");
});
});
it("rejects url host mismatches", async () => {
stubFetchResponse(new Response("url-content", { status: 200 }));
await expect(
writeUrlToFile("/tmp/ignored", "https://198.51.100.42/clip.mp4", {
expectedHost: "198.51.100.43",
}),
).rejects.toThrow(/must match node host/i);
});
it("rejects invalid url payload responses", async () => {
const cases: Array<{
name: string;
@@ -141,12 +169,12 @@ describe("nodes camera helpers", () => {
}> = [
{
name: "non-https url",
url: "http://example.com/x.bin",
url: "http://198.51.100.42/x.bin",
expectedMessage: /only https/i,
},
{
name: "oversized content-length",
url: "https://example.com/huge.bin",
url: "https://198.51.100.42/huge.bin",
response: new Response("tiny", {
status: 200,
headers: { "content-length": String(999_999_999) },
@@ -155,13 +183,13 @@ describe("nodes camera helpers", () => {
},
{
name: "non-ok status",
url: "https://example.com/down.bin",
url: "https://198.51.100.42/down.bin",
response: new Response("down", { status: 503, statusText: "Service Unavailable" }),
expectedMessage: /503/i,
},
{
name: "empty response body",
url: "https://example.com/empty.bin",
url: "https://198.51.100.42/empty.bin",
response: new Response(null, { status: 200 }),
expectedMessage: /empty response body/i,
},
@@ -171,9 +199,10 @@ describe("nodes camera helpers", () => {
if (testCase.response) {
stubFetchResponse(testCase.response);
}
await expect(writeUrlToFile("/tmp/ignored", testCase.url), testCase.name).rejects.toThrow(
testCase.expectedMessage,
);
await expect(
writeUrlToFile("/tmp/ignored", testCase.url, { expectedHost: "198.51.100.42" }),
testCase.name,
).rejects.toThrow(testCase.expectedMessage);
}
});
@@ -188,9 +217,9 @@ describe("nodes camera helpers", () => {
await withCameraTempDir(async (dir) => {
const out = path.join(dir, "broken.bin");
await expect(writeUrlToFile(out, "https://example.com/broken.bin")).rejects.toThrow(
/stream exploded/i,
);
await expect(
writeUrlToFile(out, "https://198.51.100.42/broken.bin", { expectedHost: "198.51.100.42" }),
).rejects.toThrow(/stream exploded/i);
await expect(fs.stat(out)).rejects.toThrow();
});
});

View File

@@ -1,5 +1,7 @@
import * as fs from "node:fs/promises";
import * as path from "node:path";
import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js";
import { normalizeHostname } from "../infra/net/hostname.js";
import { resolveCliName } from "./cli-name.js";
import {
asBoolean,
@@ -72,64 +74,103 @@ export function cameraTempPath(opts: {
return path.join(tmpDir, `${cliName}-camera-${opts.kind}${facingPart}-${id}${ext}`);
}
export async function writeUrlToFile(filePath: string, url: string) {
export async function writeUrlToFile(
filePath: string,
url: string,
opts: { expectedHost: string },
) {
const parsed = new URL(url);
if (parsed.protocol !== "https:") {
throw new Error(`writeUrlToFile: only https URLs are allowed, got ${parsed.protocol}`);
}
const res = await fetch(url);
if (!res.ok) {
throw new Error(`failed to download ${url}: ${res.status} ${res.statusText}`);
const expectedHost = normalizeHostname(opts.expectedHost);
if (!expectedHost) {
throw new Error("writeUrlToFile: expectedHost is required");
}
const contentLengthRaw = res.headers.get("content-length");
const contentLength = contentLengthRaw ? Number.parseInt(contentLengthRaw, 10) : undefined;
if (
typeof contentLength === "number" &&
Number.isFinite(contentLength) &&
contentLength > MAX_CAMERA_URL_DOWNLOAD_BYTES
) {
if (normalizeHostname(parsed.hostname) !== expectedHost) {
throw new Error(
`writeUrlToFile: content-length ${contentLength} exceeds max ${MAX_CAMERA_URL_DOWNLOAD_BYTES}`,
`writeUrlToFile: url host ${parsed.hostname} must match node host ${opts.expectedHost}`,
);
}
const body = res.body;
if (!body) {
throw new Error(`failed to download ${url}: empty response body`);
}
const policy = {
allowPrivateNetwork: true,
allowedHostnames: [expectedHost],
hostnameAllowlist: [expectedHost],
};
const fileHandle = await fs.open(filePath, "w");
let release: () => Promise<void> = async () => {};
let bytes = 0;
let thrown: unknown;
try {
const reader = body.getReader();
while (true) {
const { done, value } = await reader.read();
if (done) {
break;
}
if (!value || value.byteLength === 0) {
continue;
}
bytes += value.byteLength;
if (bytes > MAX_CAMERA_URL_DOWNLOAD_BYTES) {
throw new Error(
`writeUrlToFile: downloaded ${bytes} bytes, exceeds max ${MAX_CAMERA_URL_DOWNLOAD_BYTES}`,
);
}
await fileHandle.write(value);
const guarded = await fetchWithSsrFGuard({
url,
auditContext: "writeUrlToFile",
policy,
});
release = guarded.release;
const finalUrl = new URL(guarded.finalUrl);
if (finalUrl.protocol !== "https:") {
throw new Error(`writeUrlToFile: redirect resolved to non-https URL ${guarded.finalUrl}`);
}
if (normalizeHostname(finalUrl.hostname) !== expectedHost) {
throw new Error(
`writeUrlToFile: redirect host ${finalUrl.hostname} must match node host ${opts.expectedHost}`,
);
}
const res = guarded.response;
if (!res.ok) {
throw new Error(`failed to download ${url}: ${res.status} ${res.statusText}`);
}
} catch (err) {
thrown = err;
} finally {
await fileHandle.close();
}
if (thrown) {
await fs.unlink(filePath).catch(() => {});
throw thrown;
const contentLengthRaw = res.headers.get("content-length");
const contentLength = contentLengthRaw ? Number.parseInt(contentLengthRaw, 10) : undefined;
if (
typeof contentLength === "number" &&
Number.isFinite(contentLength) &&
contentLength > MAX_CAMERA_URL_DOWNLOAD_BYTES
) {
throw new Error(
`writeUrlToFile: content-length ${contentLength} exceeds max ${MAX_CAMERA_URL_DOWNLOAD_BYTES}`,
);
}
const body = res.body;
if (!body) {
throw new Error(`failed to download ${url}: empty response body`);
}
const fileHandle = await fs.open(filePath, "w");
let thrown: unknown;
try {
const reader = body.getReader();
while (true) {
const { done, value } = await reader.read();
if (done) {
break;
}
if (!value || value.byteLength === 0) {
continue;
}
bytes += value.byteLength;
if (bytes > MAX_CAMERA_URL_DOWNLOAD_BYTES) {
throw new Error(
`writeUrlToFile: downloaded ${bytes} bytes, exceeds max ${MAX_CAMERA_URL_DOWNLOAD_BYTES}`,
);
}
await fileHandle.write(value);
}
} catch (err) {
thrown = err;
} finally {
await fileHandle.close();
}
if (thrown) {
await fs.unlink(filePath).catch(() => {});
throw thrown;
}
} finally {
await release();
}
return { path: filePath, bytes };
@@ -146,6 +187,7 @@ export async function writeCameraClipPayloadToFile(params: {
facing: CameraFacing;
tmpDir?: string;
id?: string;
expectedHost?: string;
}): Promise<string> {
const filePath = cameraTempPath({
kind: "clip",
@@ -155,7 +197,10 @@ export async function writeCameraClipPayloadToFile(params: {
id: params.id,
});
if (params.payload.url) {
await writeUrlToFile(filePath, params.payload.url);
if (!params.expectedHost) {
throw new Error("camera URL payload requires node remoteIp");
}
await writeUrlToFile(filePath, params.payload.url, { expectedHost: params.expectedHost });
} else if (params.payload.base64) {
await writeBase64ToFile(filePath, params.payload.base64);
} else {

View File

@@ -13,7 +13,13 @@ import {
} from "../nodes-camera.js";
import { parseDurationMs } from "../parse-duration.js";
import { getNodesTheme, runNodesCommand } from "./cli-utils.js";
import { buildNodeInvokeParams, callGatewayCli, nodesCallOpts, resolveNodeId } from "./rpc.js";
import {
buildNodeInvokeParams,
callGatewayCli,
nodesCallOpts,
resolveNode,
resolveNodeId,
} from "./rpc.js";
import type { NodesRpcOpts } from "./types.js";
const parseFacing = (value: string): CameraFacing => {
@@ -102,7 +108,8 @@ export function registerNodesCameraCommands(nodes: Command) {
.option("--invoke-timeout <ms>", "Node invoke timeout in ms (default 20000)", "20000")
.action(async (opts: NodesRpcOpts) => {
await runNodesCommand("camera snap", async () => {
const nodeId = await resolveNodeId(opts, String(opts.node ?? ""));
const node = await resolveNode(opts, String(opts.node ?? ""));
const nodeId = node.nodeId;
const facingOpt = String(opts.facing ?? "both")
.trim()
.toLowerCase();
@@ -160,7 +167,10 @@ export function registerNodesCameraCommands(nodes: Command) {
ext: payload.format === "jpeg" ? "jpg" : payload.format,
});
if (payload.url) {
await writeUrlToFile(filePath, payload.url);
if (!node.remoteIp) {
throw new Error("camera URL payload requires node remoteIp");
}
await writeUrlToFile(filePath, payload.url, { expectedHost: node.remoteIp });
} else if (payload.base64) {
await writeBase64ToFile(filePath, payload.base64);
}
@@ -198,7 +208,8 @@ export function registerNodesCameraCommands(nodes: Command) {
.option("--invoke-timeout <ms>", "Node invoke timeout in ms (default 90000)", "90000")
.action(async (opts: NodesRpcOpts & { audio?: boolean }) => {
await runNodesCommand("camera clip", async () => {
const nodeId = await resolveNodeId(opts, String(opts.node ?? ""));
const node = await resolveNode(opts, String(opts.node ?? ""));
const nodeId = node.nodeId;
const facing = parseFacing(String(opts.facing ?? "front"));
const durationMs = parseDurationMs(String(opts.duration ?? "3000"));
const includeAudio = opts.audio !== false;
@@ -226,6 +237,7 @@ export function registerNodesCameraCommands(nodes: Command) {
const filePath = await writeCameraClipPayloadToFile({
payload,
facing,
expectedHost: node.remoteIp,
});
if (opts.json) {

View File

@@ -73,6 +73,10 @@ export function unauthorizedHintForMessage(message: string): string | null {
}
export async function resolveNodeId(opts: NodesRpcOpts, query: string) {
return (await resolveNode(opts, query)).nodeId;
}
export async function resolveNode(opts: NodesRpcOpts, query: string): Promise<NodeListNode> {
const q = String(query ?? "").trim();
if (!q) {
throw new Error("node required");
@@ -93,5 +97,6 @@ export async function resolveNodeId(opts: NodesRpcOpts, query: string) {
remoteIp: n.remoteIp,
}));
}
return resolveNodeIdFromCandidates(nodes, q);
const nodeId = resolveNodeIdFromCandidates(nodes, q);
return nodes.find((node) => node.nodeId === nodeId) ?? { nodeId };
}

View File

@@ -325,7 +325,7 @@ describe("cli program (nodes media)", () => {
command: "camera.snap" as const,
payload: {
format: "jpg",
url: "https://example.com/photo.jpg",
url: `https://${IOS_NODE.remoteIp}/photo.jpg`,
width: 640,
height: 480,
},
@@ -337,7 +337,7 @@ describe("cli program (nodes media)", () => {
command: "camera.clip" as const,
payload: {
format: "mp4",
url: "https://example.com/clip.mp4",
url: `https://${IOS_NODE.remoteIp}/clip.mp4`,
durationMs: 5000,
hasAudio: true,
},