From 6970c2c2db3ee069ef0fff0ade5cfbdd0134f9d2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 22:30:33 +0100 Subject: [PATCH] fix(gateway): harden control-ui avatar reads --- src/gateway/control-ui.http.test.ts | 62 ++++++++++++++++++++++++++- src/gateway/control-ui.ts | 65 +++++++++++++++++++++++------ 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/src/gateway/control-ui.http.test.ts b/src/gateway/control-ui.http.test.ts index d767cdc27..aa8c923ae 100644 --- a/src/gateway/control-ui.http.test.ts +++ b/src/gateway/control-ui.http.test.ts @@ -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[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) => { diff --git a/src/gateway/control-ui.ts b/src/gateway/control-ui.ts index 4dc5752d7..cd152720c 100644 --- a/src/gateway/control-ui.ts +++ b/src/gateway/control-ui.ts @@ -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,