fix(gateway): harden control-ui avatar reads

This commit is contained in:
Peter Steinberger
2026-02-22 22:30:33 +01:00
parent e16f93af0c
commit 6970c2c2db
2 changed files with 114 additions and 13 deletions

View File

@@ -4,7 +4,7 @@ import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
import { CONTROL_UI_BOOTSTRAP_CONFIG_PATH } from "./control-ui-contract.js";
import { handleControlUiHttpRequest } from "./control-ui.js";
import { handleControlUiAvatarRequest, handleControlUiHttpRequest } from "./control-ui.js";
import { makeMockHttpResponse } from "./test-http-response.js";
describe("handleControlUiHttpRequest", () => {
@@ -58,6 +58,24 @@ describe("handleControlUiHttpRequest", () => {
return { res, end, handled };
}
function runAvatarRequest(params: {
url: string;
method: "GET" | "HEAD";
resolveAvatar: Parameters<typeof handleControlUiAvatarRequest>[2]["resolveAvatar"];
basePath?: string;
}) {
const { res, end } = makeMockHttpResponse();
const handled = handleControlUiAvatarRequest(
{ url: params.url, method: params.method } as IncomingMessage,
res,
{
...(params.basePath ? { basePath: params.basePath } : {}),
resolveAvatar: params.resolveAvatar,
},
);
return { res, end, handled };
}
async function writeAssetFile(rootPath: string, filename: string, contents: string) {
const assetsDir = path.join(rootPath, "assets");
await fs.mkdir(assetsDir, { recursive: true });
@@ -179,6 +197,48 @@ describe("handleControlUiHttpRequest", () => {
});
});
it("serves local avatar bytes through hardened avatar handler", async () => {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-avatar-http-"));
try {
const avatarPath = path.join(tmp, "main.png");
await fs.writeFile(avatarPath, "avatar-bytes\n");
const { res, end, handled } = runAvatarRequest({
url: "/avatar/main",
method: "GET",
resolveAvatar: () => ({ kind: "local", filePath: avatarPath }),
});
expect(handled).toBe(true);
expect(res.statusCode).toBe(200);
expect(String(end.mock.calls[0]?.[0] ?? "")).toBe("avatar-bytes\n");
} finally {
await fs.rm(tmp, { recursive: true, force: true });
}
});
it("rejects avatar symlink paths from resolver", async () => {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-avatar-http-link-"));
const outside = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-avatar-http-outside-"));
try {
const outsideFile = path.join(outside, "secret.txt");
await fs.writeFile(outsideFile, "outside-secret\n");
const linkPath = path.join(tmp, "avatar-link.png");
await fs.symlink(outsideFile, linkPath);
const { res, end, handled } = runAvatarRequest({
url: "/avatar/main",
method: "GET",
resolveAvatar: () => ({ kind: "local", filePath: linkPath }),
});
expectNotFoundResponse({ handled, res, end });
} finally {
await fs.rm(tmp, { recursive: true, force: true });
await fs.rm(outside, { recursive: true, force: true });
}
});
it("rejects symlinked assets that resolve outside control-ui root", async () => {
await withControlUiRoot({
fn: async (tmp) => {

View File

@@ -4,6 +4,7 @@ import path from "node:path";
import type { OpenClawConfig } from "../config/config.js";
import { resolveControlUiRootSync } from "../infra/control-ui-assets.js";
import { isWithinDir } from "../infra/path-safety.js";
import { AVATAR_MAX_BYTES } from "../shared/avatar-policy.js";
import { DEFAULT_ASSISTANT_IDENTITY, resolveAssistantIdentity } from "./assistant-identity.js";
import {
CONTROL_UI_BOOTSTRAP_CONFIG_PATH,
@@ -162,16 +163,25 @@ export function handleControlUiAvatarRequest(
return true;
}
if (req.method === "HEAD") {
res.statusCode = 200;
res.setHeader("Content-Type", contentTypeForExt(path.extname(resolved.filePath).toLowerCase()));
res.setHeader("Cache-Control", "no-cache");
res.end();
const safeAvatar = resolveSafeAvatarFile(resolved.filePath);
if (!safeAvatar) {
respondNotFound(res);
return true;
}
try {
if (req.method === "HEAD") {
res.statusCode = 200;
res.setHeader("Content-Type", contentTypeForExt(path.extname(safeAvatar.path).toLowerCase()));
res.setHeader("Cache-Control", "no-cache");
res.end();
return true;
}
serveFile(res, resolved.filePath);
return true;
serveResolvedFile(res, safeAvatar.path, fs.readFileSync(safeAvatar.fd));
return true;
} finally {
fs.closeSync(safeAvatar.fd);
}
}
function respondNotFound(res: ServerResponse) {
@@ -188,11 +198,6 @@ function setStaticFileHeaders(res: ServerResponse, filePath: string) {
res.setHeader("Cache-Control", "no-cache");
}
function serveFile(res: ServerResponse, filePath: string) {
setStaticFileHeaders(res, filePath);
res.end(fs.readFileSync(filePath));
}
function serveResolvedFile(res: ServerResponse, filePath: string, body: Buffer) {
setStaticFileHeaders(res, filePath);
res.end(body);
@@ -219,6 +224,42 @@ function areSameFileIdentity(preOpen: fs.Stats, opened: fs.Stats): boolean {
return preOpen.dev === opened.dev && preOpen.ino === opened.ino;
}
function resolveSafeAvatarFile(filePath: string): { path: string; fd: number } | null {
let fd: number | null = null;
try {
const candidateStat = fs.lstatSync(filePath);
if (candidateStat.isSymbolicLink()) {
return null;
}
const fileReal = fs.realpathSync(filePath);
const preOpenStat = fs.lstatSync(fileReal);
if (!preOpenStat.isFile() || preOpenStat.size > AVATAR_MAX_BYTES) {
return null;
}
const openFlags =
fs.constants.O_RDONLY |
(typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : 0);
fd = fs.openSync(fileReal, openFlags);
const openedStat = fs.fstatSync(fd);
if (
!openedStat.isFile() ||
openedStat.size > AVATAR_MAX_BYTES ||
!areSameFileIdentity(preOpenStat, openedStat)
) {
return null;
}
const safeFile = { path: fileReal, fd };
fd = null;
return safeFile;
} catch {
return null;
} finally {
if (fd !== null) {
fs.closeSync(fd);
}
}
}
function resolveSafeControlUiFile(
rootReal: string,
filePath: string,