From 238b0fc76fa79ce9953007a8367b3c3c4b79d1ed Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Fri, 15 May 2026 14:51:38 +0530 Subject: [PATCH] fix(canvas): validate snapshot response formats [AI] (#81881) * fix: validate canvas snapshot formats * addressing codex review * docs: add changelog entry for PR merge --- CHANGELOG.md | 1 + extensions/canvas/src/cli-helpers.test.ts | 47 ++++++++++++++++++++++- extensions/canvas/src/cli-helpers.ts | 34 ++++++++++++++-- extensions/canvas/src/cli.test.ts | 22 +++++++++++ extensions/canvas/src/tool.test.ts | 15 ++++++++ extensions/canvas/src/tool.ts | 21 +--------- 6 files changed, 116 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ae1b20c298..3eacbe1ee1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(canvas): validate snapshot response formats [AI]. (#81881) Thanks @pgondhi987. - Constrain provider catalog entry paths [AI]. (#81884) Thanks @pgondhi987. - Require canonical node platform IDs [AI]. (#81880) Thanks @pgondhi987. - Agents/Azure OpenAI Responses: default unset Azure OpenAI API versions to `preview` so `/openai/v1/responses` calls use Azure's current Responses API route. (#82026) Thanks @leoge007. diff --git a/extensions/canvas/src/cli-helpers.test.ts b/extensions/canvas/src/cli-helpers.test.ts index 699b5264c23..54b957db6bc 100644 --- a/extensions/canvas/src/cli-helpers.test.ts +++ b/extensions/canvas/src/cli-helpers.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { parseCanvasSnapshotPayload } from "./cli-helpers.js"; +import { + canvasSnapshotTempPath, + normalizeCanvasSnapshotFileExtension, + parseCanvasSnapshotPayload, +} from "./cli-helpers.js"; describe("canvas CLI helpers", () => { it("parses canvas.snapshot payload", () => { @@ -14,4 +18,45 @@ describe("canvas CLI helpers", () => { /invalid canvas\.snapshot payload/i, ); }); + + it.each([{ base64: "aGk=" }, { format: 42, base64: "aGk=" }])( + "rejects invalid canvas.snapshot format fields", + (payload) => { + expect(() => parseCanvasSnapshotPayload(payload)).toThrow( + /invalid canvas\.snapshot payload/i, + ); + }, + ); + + it.each(["/../../target.sh", "../target.sh", "png/../../target.sh", "image/png", ""])( + "rejects unsafe canvas.snapshot formats from responses: %s", + (format) => { + expect(() => parseCanvasSnapshotPayload({ format, base64: "aGk=" })).toThrow( + /invalid canvas\.snapshot payload/i, + ); + }, + ); + + it("normalizes supported snapshot file extensions", () => { + expect(normalizeCanvasSnapshotFileExtension("png")).toBe("png"); + expect(normalizeCanvasSnapshotFileExtension(".jpeg")).toBe("jpg"); + expect(normalizeCanvasSnapshotFileExtension(" JPG ")).toBe("jpg"); + }); + + it("rejects unsafe snapshot temp path parts", () => { + expect(() => + canvasSnapshotTempPath({ + tmpDir: "/tmp/openclaw-canvas-test", + id: "snapshot", + ext: "/../../target.sh", + }), + ).toThrow(/invalid canvas\.snapshot format/i); + expect(() => + canvasSnapshotTempPath({ + tmpDir: "/tmp/openclaw-canvas-test", + id: "../../snapshot", + ext: "png", + }), + ).toThrow(/invalid canvas snapshot id/i); + }); }); diff --git a/extensions/canvas/src/cli-helpers.ts b/extensions/canvas/src/cli-helpers.ts index 2e159134385..649db0e6f5d 100644 --- a/extensions/canvas/src/cli-helpers.ts +++ b/extensions/canvas/src/cli-helpers.ts @@ -5,13 +5,32 @@ import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/security-run import { asRecord, readStringValue } from "openclaw/plugin-sdk/string-coerce-runtime"; type CanvasSnapshotPayload = { - format: string; + format: CanvasSnapshotFormat; base64: string; }; +type CanvasSnapshotFormat = "png" | "jpg" | "jpeg"; +type CanvasSnapshotFileExtension = "png" | "jpg"; + +function normalizeCanvasSnapshotFormat(value: string | undefined): CanvasSnapshotFormat | null { + const format = value?.trim().toLowerCase() ?? ""; + if (format === "png" || format === "jpg" || format === "jpeg") { + return format; + } + return null; +} + +export function normalizeCanvasSnapshotFileExtension(value: string): CanvasSnapshotFileExtension { + const format = normalizeCanvasSnapshotFormat(value.startsWith(".") ? value.slice(1) : value); + if (!format) { + throw new Error("invalid canvas.snapshot format"); + } + return format === "jpeg" ? "jpg" : format; +} + export function parseCanvasSnapshotPayload(value: unknown): CanvasSnapshotPayload { const obj = asRecord(value); - const format = readStringValue(obj.format); + const format = normalizeCanvasSnapshotFormat(readStringValue(obj.format)); const base64 = readStringValue(obj.base64); if (!format || !base64) { throw new Error("invalid canvas.snapshot payload"); @@ -23,6 +42,13 @@ function resolveCliName(): string { return "openclaw"; } +function resolveCanvasSnapshotId(id: string): string { + if (!/^[A-Za-z0-9_-]+$/.test(id)) { + throw new Error("invalid canvas snapshot id"); + } + return id; +} + function resolveTempPathParts(opts: { ext: string; tmpDir?: string; id?: string }) { const tmpDir = opts.tmpDir ?? resolvePreferredOpenClawTmpDir(); if (!opts.tmpDir) { @@ -30,8 +56,8 @@ function resolveTempPathParts(opts: { ext: string; tmpDir?: string; id?: string } return { tmpDir, - id: opts.id ?? randomUUID(), - ext: opts.ext.startsWith(".") ? opts.ext : `.${opts.ext}`, + id: resolveCanvasSnapshotId(opts.id ?? randomUUID()), + ext: `.${normalizeCanvasSnapshotFileExtension(opts.ext)}`, }; } diff --git a/extensions/canvas/src/cli.test.ts b/extensions/canvas/src/cli.test.ts index 06284d8f7e0..68cab1da1fa 100644 --- a/extensions/canvas/src/cli.test.ts +++ b/extensions/canvas/src/cli.test.ts @@ -91,4 +91,26 @@ describe("canvas CLI", () => { expect(mediaMessage?.startsWith("MEDIA:")).toBe(true); expect(mediaMessage?.endsWith(".png")).toBe(true); }); + + it("rejects node-controlled snapshot formats before writing", async () => { + const program = new Command(); + program.exitOverride(); + const nodes = program.command("nodes"); + const { deps, writtenFiles } = createCanvasCliDeps(); + vi.mocked(deps.callGatewayCli).mockResolvedValueOnce({ + payload: { + format: "/../../target.sh", + base64: "aGk=", + }, + }); + + registerNodesCanvasCommands(nodes, deps); + + await expect( + program.parseAsync(["nodes", "canvas", "snapshot", "--node", "ios-node"], { + from: "user", + }), + ).rejects.toThrow(/invalid canvas\.snapshot payload/i); + expect(writtenFiles).toHaveLength(0); + }); }); diff --git a/extensions/canvas/src/tool.test.ts b/extensions/canvas/src/tool.test.ts index f66e34c9e81..0da45934a3d 100644 --- a/extensions/canvas/src/tool.test.ts +++ b/extensions/canvas/src/tool.test.ts @@ -96,4 +96,19 @@ describe("Canvas tool", () => { expect(imageResultParams?.details).toEqual({ format: "png" }); expect(imageResultParams?.imageSanitization).toEqual({ maxDimensionPx: 1600 }); }); + + it("rejects node-controlled snapshot formats before creating image results", async () => { + mocks.callGatewayTool.mockResolvedValue({ + payload: { + format: "/../../target.sh", + base64: Buffer.from("not-a-real-png").toString("base64"), + }, + }); + const tool = createCanvasTool(); + + await expect(tool.execute("tool-call-1", { action: "snapshot" })).rejects.toThrow( + /invalid canvas\.snapshot payload/i, + ); + expect(mocks.imageResultFromFile).not.toHaveBeenCalled(); + }); }); diff --git a/extensions/canvas/src/tool.ts b/extensions/canvas/src/tool.ts index 0a966ccd3d0..b19752c99f6 100644 --- a/extensions/canvas/src/tool.ts +++ b/extensions/canvas/src/tool.ts @@ -13,6 +13,7 @@ import { } from "openclaw/plugin-sdk/channel-actions"; import type { AnyAgentTool, OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry"; import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; +import { normalizeCanvasSnapshotFileExtension, parseCanvasSnapshotPayload } from "./cli-helpers.js"; import { CanvasToolSchema } from "./tool-schema.js"; type CanvasToolOptions = { @@ -20,11 +21,6 @@ type CanvasToolOptions = { workspaceDir?: string; }; -type CanvasSnapshotPayload = { - format: string; - base64: string; -}; - type CanvasImageSanitizationLimits = { maxDimensionPx?: number; }; @@ -45,23 +41,10 @@ async function resolveNodeId( return resolveNodeIdFromList(await listNodes(opts), query, allowDefault); } -function parseCanvasSnapshotPayload(value: unknown): CanvasSnapshotPayload { - if (!value || typeof value !== "object" || Array.isArray(value)) { - throw new Error("invalid canvas.snapshot payload"); - } - const record = value as Record; - const format = typeof record.format === "string" ? record.format : ""; - const base64 = typeof record.base64 === "string" ? record.base64 : ""; - if (!format || !base64) { - throw new Error("invalid canvas.snapshot payload"); - } - return { format, base64 }; -} - async function writeBase64ToTempFile(params: { base64: string; ext: string }): Promise { const dir = resolvePreferredOpenClawTmpDir(); await fs.mkdir(dir, { recursive: true, mode: 0o700 }); - const ext = params.ext.startsWith(".") ? params.ext : `.${params.ext}`; + const ext = `.${normalizeCanvasSnapshotFileExtension(params.ext)}`; const filePath = path.join(dir, `openclaw-canvas-snapshot-${randomUUID()}${ext}`); await fs.writeFile(filePath, Buffer.from(params.base64, "base64")); return filePath;