diff --git a/CHANGELOG.md b/CHANGELOG.md index d0d6ec39a..4f7d00fbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Docs: https://docs.openclaw.ai - Security/Media: enforce inbound media byte limits during download/read across Discord, Telegram, Zalo, Microsoft Teams, and BlueBubbles to prevent oversized payload memory spikes before rejection. This ships in the next npm release. Thanks @tdjackey for reporting. - Media/Understanding: preserve `application/pdf` MIME classification during text-like file heuristics so PDF uploads use PDF extraction paths instead of being inlined as raw text. (#23191) Thanks @claudeplay2026-byte. - Security/Control UI: block symlink-based out-of-root static file reads by enforcing realpath containment and file-identity checks when serving Control UI assets and SPA fallback `index.html`. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Control UI: centralize avatar URL/path validation across gateway/config helpers and enforce a 2 MB max size for local agent avatar files before `/avatar` resolution, reducing oversized-avatar memory risk without changing supported avatar formats. - Security/MSTeams media: enforce allowlist checks for SharePoint reference attachment URLs and redirect targets during Graph-backed media fetches so redirect chains cannot escape configured media host boundaries. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/macOS discovery: fail closed for unresolved discovery endpoints by clearing stale remote selection values, use resolved service host only for SSH target derivation, and keep remote URL config aligned with resolved endpoint availability. (#21618) Thanks @bmendonca3. - Chat/Usage/TUI: strip synthetic inbound metadata blocks (including `Conversation info` and trailing `Untrusted context` channel metadata wrappers) from displayed conversation history so internal prompt context no longer leaks into user-visible logs. diff --git a/src/agents/identity-avatar.e2e.test.ts b/src/agents/identity-avatar.e2e.test.ts index fcfbf6ff4..4bb05bfe3 100644 --- a/src/agents/identity-avatar.e2e.test.ts +++ b/src/agents/identity-avatar.e2e.test.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +import { AVATAR_MAX_BYTES } from "../shared/avatar-policy.js"; import { resolveAgentAvatar } from "./identity-avatar.js"; async function writeFile(filePath: string, contents = "avatar") { @@ -127,6 +128,26 @@ describe("resolveAgentAvatar", () => { } }); + it("rejects local avatars larger than max bytes", async () => { + const root = await createTempAvatarRoot(); + const workspace = path.join(root, "work"); + const avatarPath = path.join(workspace, "avatars", "too-big.png"); + await fs.mkdir(path.dirname(avatarPath), { recursive: true }); + await fs.writeFile(avatarPath, Buffer.alloc(AVATAR_MAX_BYTES + 1)); + + const cfg: OpenClawConfig = { + agents: { + list: [{ id: "main", workspace, identity: { avatar: "avatars/too-big.png" } }], + }, + }; + + const resolved = resolveAgentAvatar(cfg, "main"); + expect(resolved.kind).toBe("none"); + if (resolved.kind === "none") { + expect(resolved.reason).toBe("too_large"); + } + }); + it("accepts remote and data avatars", () => { const cfg: OpenClawConfig = { agents: { diff --git a/src/agents/identity-avatar.ts b/src/agents/identity-avatar.ts index 1c9a82258..f30a5d334 100644 --- a/src/agents/identity-avatar.ts +++ b/src/agents/identity-avatar.ts @@ -1,6 +1,13 @@ import fs from "node:fs"; import path from "node:path"; import type { OpenClawConfig } from "../config/config.js"; +import { + AVATAR_MAX_BYTES, + isAvatarDataUrl, + isAvatarHttpUrl, + isPathWithinRoot, + isSupportedLocalAvatarExtension, +} from "../shared/avatar-policy.js"; import { resolveUserPath } from "../utils.js"; import { resolveAgentWorkspaceDir } from "./agent-scope.js"; import { loadAgentIdentityFromWorkspace } from "./identity-file.js"; @@ -12,8 +19,6 @@ export type AgentAvatarResolution = | { kind: "remote"; url: string } | { kind: "data"; url: string }; -const ALLOWED_AVATAR_EXTS = new Set([".png", ".jpg", ".jpeg", ".gif", ".webp", ".svg"]); - function normalizeAvatarValue(value: string | undefined | null): string | null { const trimmed = value?.trim(); return trimmed ? trimmed : null; @@ -29,15 +34,6 @@ function resolveAvatarSource(cfg: OpenClawConfig, agentId: string): string | nul return fromIdentity; } -function isRemoteAvatar(value: string): boolean { - const lower = value.toLowerCase(); - return lower.startsWith("http://") || lower.startsWith("https://"); -} - -function isDataAvatar(value: string): boolean { - return value.toLowerCase().startsWith("data:"); -} - function resolveExistingPath(value: string): string { try { return fs.realpathSync(value); @@ -46,14 +42,6 @@ function resolveExistingPath(value: string): string { } } -function isPathWithin(root: string, target: string): boolean { - const relative = path.relative(root, target); - if (!relative) { - return true; - } - return !relative.startsWith("..") && !path.isAbsolute(relative); -} - function resolveLocalAvatarPath(params: { raw: string; workspaceDir: string; @@ -65,17 +53,20 @@ function resolveLocalAvatarPath(params: { ? resolveUserPath(raw) : path.resolve(workspaceRoot, raw); const realPath = resolveExistingPath(resolved); - if (!isPathWithin(workspaceRoot, realPath)) { + if (!isPathWithinRoot(workspaceRoot, realPath)) { return { ok: false, reason: "outside_workspace" }; } - const ext = path.extname(realPath).toLowerCase(); - if (!ALLOWED_AVATAR_EXTS.has(ext)) { + if (!isSupportedLocalAvatarExtension(realPath)) { return { ok: false, reason: "unsupported_extension" }; } try { - if (!fs.statSync(realPath).isFile()) { + const stat = fs.statSync(realPath); + if (!stat.isFile()) { return { ok: false, reason: "missing" }; } + if (stat.size > AVATAR_MAX_BYTES) { + return { ok: false, reason: "too_large" }; + } } catch { return { ok: false, reason: "missing" }; } @@ -87,10 +78,10 @@ export function resolveAgentAvatar(cfg: OpenClawConfig, agentId: string): AgentA if (!source) { return { kind: "none", reason: "missing" }; } - if (isRemoteAvatar(source)) { + if (isAvatarHttpUrl(source)) { return { kind: "remote", url: source }; } - if (isDataAvatar(source)) { + if (isAvatarDataUrl(source)) { return { kind: "data", url: source }; } const workspaceDir = resolveAgentWorkspaceDir(cfg, agentId); diff --git a/src/config/validation.ts b/src/config/validation.ts index 29ebd8fa6..a9205a3ae 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -8,6 +8,13 @@ import { } from "../plugins/config-state.js"; import { loadPluginManifestRegistry } from "../plugins/manifest-registry.js"; import { validateJsonSchemaValue } from "../plugins/schema-validator.js"; +import { + hasAvatarUriScheme, + isAvatarDataUrl, + isAvatarHttpUrl, + isPathWithinRoot, + isWindowsAbsolutePath, +} from "../shared/avatar-policy.js"; import { isRecord } from "../utils.js"; import { findDuplicateAgentDirs, formatDuplicateAgentDirError } from "./agent-dirs.js"; import { applyAgentDefaults, applyModelDefaults, applySessionDefaults } from "./defaults.js"; @@ -15,22 +22,10 @@ import { findLegacyConfigIssues } from "./legacy.js"; import type { OpenClawConfig, ConfigValidationIssue } from "./types.js"; import { OpenClawSchema } from "./zod-schema.js"; -const AVATAR_SCHEME_RE = /^[a-z][a-z0-9+.-]*:/i; -const AVATAR_DATA_RE = /^data:/i; -const AVATAR_HTTP_RE = /^https?:\/\//i; -const WINDOWS_ABS_RE = /^[a-zA-Z]:[\\/]/; - function isWorkspaceAvatarPath(value: string, workspaceDir: string): boolean { const workspaceRoot = path.resolve(workspaceDir); const resolved = path.resolve(workspaceRoot, value); - const relative = path.relative(workspaceRoot, resolved); - if (relative === "") { - return true; - } - if (relative.startsWith("..")) { - return false; - } - return !path.isAbsolute(relative); + return isPathWithinRoot(workspaceRoot, resolved); } function validateIdentityAvatar(config: OpenClawConfig): ConfigValidationIssue[] { @@ -51,7 +46,7 @@ function validateIdentityAvatar(config: OpenClawConfig): ConfigValidationIssue[] if (!avatar) { continue; } - if (AVATAR_DATA_RE.test(avatar) || AVATAR_HTTP_RE.test(avatar)) { + if (isAvatarDataUrl(avatar) || isAvatarHttpUrl(avatar)) { continue; } if (avatar.startsWith("~")) { @@ -61,8 +56,8 @@ function validateIdentityAvatar(config: OpenClawConfig): ConfigValidationIssue[] }); continue; } - const hasScheme = AVATAR_SCHEME_RE.test(avatar); - if (hasScheme && !WINDOWS_ABS_RE.test(avatar)) { + const hasScheme = hasAvatarUriScheme(avatar); + if (hasScheme && !isWindowsAbsolutePath(avatar)) { issues.push({ path: `agents.list.${index}.identity.avatar`, message: "identity.avatar must be a workspace-relative path, http(s) URL, or data URI.", diff --git a/src/gateway/assistant-identity.ts b/src/gateway/assistant-identity.ts index 84ba6c7e6..d1a103e92 100644 --- a/src/gateway/assistant-identity.ts +++ b/src/gateway/assistant-identity.ts @@ -3,6 +3,11 @@ import { resolveAgentIdentity } from "../agents/identity.js"; import { loadAgentIdentity } from "../commands/agents.config.js"; import type { OpenClawConfig } from "../config/config.js"; import { normalizeAgentId } from "../routing/session-key.js"; +import { + isAvatarHttpUrl, + isAvatarImageDataUrl, + looksLikeAvatarPath, +} from "../shared/avatar-policy.js"; const MAX_ASSISTANT_NAME = 50; const MAX_ASSISTANT_AVATAR = 200; @@ -36,14 +41,7 @@ function coerceIdentityValue(value: string | undefined, maxLength: number): stri } function isAvatarUrl(value: string): boolean { - return /^https?:\/\//i.test(value) || /^data:image\//i.test(value); -} - -function looksLikeAvatarPath(value: string): boolean { - if (/[\\/]/.test(value)) { - return true; - } - return /\.(png|jpe?g|gif|webp|svg|ico)$/i.test(value); + return isAvatarHttpUrl(value) || isAvatarImageDataUrl(value); } function normalizeAvatarValue(value: string | undefined): string | undefined { diff --git a/src/gateway/control-ui-shared.ts b/src/gateway/control-ui-shared.ts index 8f27411f5..7ba4c61a0 100644 --- a/src/gateway/control-ui-shared.ts +++ b/src/gateway/control-ui-shared.ts @@ -1,3 +1,9 @@ +import { + isAvatarHttpUrl, + isAvatarImageDataUrl, + looksLikeAvatarPath, +} from "../shared/avatar-policy.js"; + const CONTROL_UI_AVATAR_PREFIX = "/avatar"; export function normalizeControlUiBasePath(basePath?: string): string { @@ -26,13 +32,6 @@ export function buildControlUiAvatarUrl(basePath: string, agentId: string): stri : `${CONTROL_UI_AVATAR_PREFIX}/${agentId}`; } -function looksLikeLocalAvatarPath(value: string): boolean { - if (/[\\/]/.test(value)) { - return true; - } - return /\.(png|jpe?g|gif|webp|svg|ico)$/i.test(value); -} - export function resolveAssistantAvatarUrl(params: { avatar?: string | null; agentId?: string | null; @@ -42,7 +41,7 @@ export function resolveAssistantAvatarUrl(params: { if (!avatar) { return undefined; } - if (/^https?:\/\//i.test(avatar) || /^data:image\//i.test(avatar)) { + if (isAvatarHttpUrl(avatar) || isAvatarImageDataUrl(avatar)) { return avatar; } @@ -60,7 +59,7 @@ export function resolveAssistantAvatarUrl(params: { if (!params.agentId) { return avatar; } - if (looksLikeLocalAvatarPath(avatar)) { + if (looksLikeAvatarPath(avatar)) { return buildControlUiAvatarUrl(basePath, params.agentId); } return avatar; diff --git a/src/gateway/session-utils.ts b/src/gateway/session-utils.ts index 3180b65ad..5f176361b 100644 --- a/src/gateway/session-utils.ts +++ b/src/gateway/session-utils.ts @@ -27,6 +27,14 @@ import { parseAgentSessionKey, } from "../routing/session-key.js"; import { isCronRunSessionKey } from "../sessions/session-key-utils.js"; +import { + AVATAR_MAX_BYTES, + isAvatarDataUrl, + isAvatarHttpUrl, + isPathWithinRoot, + isWorkspaceRelativeAvatarPath, + resolveAvatarMime, +} from "../shared/avatar-policy.js"; import { normalizeSessionDeliveryFields } from "../utils/delivery-context.js"; import { readSessionTitleFieldsFromTranscript } from "./session-utils.fs.js"; import type { @@ -58,43 +66,6 @@ export type { } from "./session-utils.types.js"; const DERIVED_TITLE_MAX_LEN = 60; -const AVATAR_MAX_BYTES = 2 * 1024 * 1024; - -const AVATAR_DATA_RE = /^data:/i; -const AVATAR_HTTP_RE = /^https?:\/\//i; -const AVATAR_SCHEME_RE = /^[a-z][a-z0-9+.-]*:/i; -const WINDOWS_ABS_RE = /^[a-zA-Z]:[\\/]/; - -const AVATAR_MIME_BY_EXT: Record = { - ".png": "image/png", - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".webp": "image/webp", - ".gif": "image/gif", - ".svg": "image/svg+xml", - ".bmp": "image/bmp", - ".tif": "image/tiff", - ".tiff": "image/tiff", -}; - -function resolveAvatarMime(filePath: string): string { - const ext = path.extname(filePath).toLowerCase(); - return AVATAR_MIME_BY_EXT[ext] ?? "application/octet-stream"; -} - -function isWorkspaceRelativePath(value: string): boolean { - if (!value) { - return false; - } - if (value.startsWith("~")) { - return false; - } - if (AVATAR_SCHEME_RE.test(value) && !WINDOWS_ABS_RE.test(value)) { - return false; - } - return true; -} - function resolveIdentityAvatarUrl( cfg: OpenClawConfig, agentId: string, @@ -107,17 +78,16 @@ function resolveIdentityAvatarUrl( if (!trimmed) { return undefined; } - if (AVATAR_DATA_RE.test(trimmed) || AVATAR_HTTP_RE.test(trimmed)) { + if (isAvatarDataUrl(trimmed) || isAvatarHttpUrl(trimmed)) { return trimmed; } - if (!isWorkspaceRelativePath(trimmed)) { + if (!isWorkspaceRelativeAvatarPath(trimmed)) { return undefined; } const workspaceDir = resolveAgentWorkspaceDir(cfg, agentId); const workspaceRoot = path.resolve(workspaceDir); const resolved = path.resolve(workspaceRoot, trimmed); - const relative = path.relative(workspaceRoot, resolved); - if (relative.startsWith("..") || path.isAbsolute(relative)) { + if (!isPathWithinRoot(workspaceRoot, resolved)) { return undefined; } try { diff --git a/src/shared/avatar-policy.test.ts b/src/shared/avatar-policy.test.ts new file mode 100644 index 000000000..81331a45b --- /dev/null +++ b/src/shared/avatar-policy.test.ts @@ -0,0 +1,43 @@ +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + isPathWithinRoot, + isSupportedLocalAvatarExtension, + isWorkspaceRelativeAvatarPath, + looksLikeAvatarPath, + resolveAvatarMime, +} from "./avatar-policy.js"; + +describe("avatar policy", () => { + it("accepts workspace-relative avatar paths and rejects URI schemes", () => { + expect(isWorkspaceRelativeAvatarPath("avatars/openclaw.png")).toBe(true); + expect(isWorkspaceRelativeAvatarPath("C:\\\\avatars\\\\openclaw.png")).toBe(true); + expect(isWorkspaceRelativeAvatarPath("https://example.com/avatar.png")).toBe(false); + expect(isWorkspaceRelativeAvatarPath("data:image/png;base64,AAAA")).toBe(false); + expect(isWorkspaceRelativeAvatarPath("~/avatar.png")).toBe(false); + }); + + it("checks path containment safely", () => { + const root = path.resolve("/tmp/root"); + expect(isPathWithinRoot(root, path.resolve("/tmp/root/avatars/a.png"))).toBe(true); + expect(isPathWithinRoot(root, path.resolve("/tmp/root/../outside.png"))).toBe(false); + }); + + it("detects avatar-like path strings", () => { + expect(looksLikeAvatarPath("avatars/openclaw.svg")).toBe(true); + expect(looksLikeAvatarPath("openclaw.webp")).toBe(true); + expect(looksLikeAvatarPath("A")).toBe(false); + }); + + it("supports expected local file extensions", () => { + expect(isSupportedLocalAvatarExtension("avatar.png")).toBe(true); + expect(isSupportedLocalAvatarExtension("avatar.svg")).toBe(true); + expect(isSupportedLocalAvatarExtension("avatar.ico")).toBe(false); + }); + + it("resolves mime type from extension", () => { + expect(resolveAvatarMime("a.svg")).toBe("image/svg+xml"); + expect(resolveAvatarMime("a.tiff")).toBe("image/tiff"); + expect(resolveAvatarMime("a.bin")).toBe("application/octet-stream"); + }); +}); diff --git a/src/shared/avatar-policy.ts b/src/shared/avatar-policy.ts new file mode 100644 index 000000000..7913ccc85 --- /dev/null +++ b/src/shared/avatar-policy.ts @@ -0,0 +1,83 @@ +import path from "node:path"; + +export const AVATAR_MAX_BYTES = 2 * 1024 * 1024; + +const LOCAL_AVATAR_EXTENSIONS = new Set([".png", ".jpg", ".jpeg", ".gif", ".webp", ".svg"]); + +const AVATAR_MIME_BY_EXT: Record = { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".webp": "image/webp", + ".gif": "image/gif", + ".svg": "image/svg+xml", + ".bmp": "image/bmp", + ".tif": "image/tiff", + ".tiff": "image/tiff", +}; + +export const AVATAR_DATA_RE = /^data:/i; +export const AVATAR_IMAGE_DATA_RE = /^data:image\//i; +export const AVATAR_HTTP_RE = /^https?:\/\//i; +export const AVATAR_SCHEME_RE = /^[a-z][a-z0-9+.-]*:/i; +export const WINDOWS_ABS_RE = /^[a-zA-Z]:[\\/]/; + +const AVATAR_PATH_EXT_RE = /\.(png|jpe?g|gif|webp|svg|ico)$/i; + +export function resolveAvatarMime(filePath: string): string { + const ext = path.extname(filePath).toLowerCase(); + return AVATAR_MIME_BY_EXT[ext] ?? "application/octet-stream"; +} + +export function isAvatarDataUrl(value: string): boolean { + return AVATAR_DATA_RE.test(value); +} + +export function isAvatarImageDataUrl(value: string): boolean { + return AVATAR_IMAGE_DATA_RE.test(value); +} + +export function isAvatarHttpUrl(value: string): boolean { + return AVATAR_HTTP_RE.test(value); +} + +export function hasAvatarUriScheme(value: string): boolean { + return AVATAR_SCHEME_RE.test(value); +} + +export function isWindowsAbsolutePath(value: string): boolean { + return WINDOWS_ABS_RE.test(value); +} + +export function isWorkspaceRelativeAvatarPath(value: string): boolean { + if (!value) { + return false; + } + if (value.startsWith("~")) { + return false; + } + if (hasAvatarUriScheme(value) && !isWindowsAbsolutePath(value)) { + return false; + } + return true; +} + +export function isPathWithinRoot(rootDir: string, targetPath: string): boolean { + const relative = path.relative(rootDir, targetPath); + if (relative === "") { + return true; + } + return !relative.startsWith("..") && !path.isAbsolute(relative); +} + +export function looksLikeAvatarPath(value: string): boolean { + if (/[\\/]/.test(value)) { + return true; + } + return AVATAR_PATH_EXT_RE.test(value); +} + +export function isSupportedLocalAvatarExtension(filePath: string): boolean { + const ext = path.extname(filePath).toLowerCase(); + return LOCAL_AVATAR_EXTENSIONS.has(ext); +}