Skip to content

Commit 238b0fc

Browse files
pgondhi987pgondhi987
authored
fix(canvas): validate snapshot response formats [AI] (#81881)
* fix: validate canvas snapshot formats * addressing codex review * docs: add changelog entry for PR merge
1 parent e30be46 commit 238b0fc

6 files changed

Lines changed: 116 additions & 24 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ Docs: https://docs.openclaw.ai
3434

3535
### Fixes
3636

37+
- fix(canvas): validate snapshot response formats [AI]. (#81881) Thanks @pgondhi987.
3738
- Constrain provider catalog entry paths [AI]. (#81884) Thanks @pgondhi987.
3839
- Require canonical node platform IDs [AI]. (#81880) Thanks @pgondhi987.
3940
- 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.

‎extensions/canvas/src/cli-helpers.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it } from "vitest";
2-
import { parseCanvasSnapshotPayload } from "./cli-helpers.js";
2+
import {
3+
canvasSnapshotTempPath,
4+
normalizeCanvasSnapshotFileExtension,
5+
parseCanvasSnapshotPayload,
6+
} from "./cli-helpers.js";
37

48
describe("canvas CLI helpers", () => {
59
it("parses canvas.snapshot payload", () => {
@@ -14,4 +18,45 @@ describe("canvas CLI helpers", () => {
1418
/invalid canvas\.snapshot payload/i,
1519
);
1620
});
21+
22+
it.each([{ base64: "aGk=" }, { format: 42, base64: "aGk=" }])(
23+
"rejects invalid canvas.snapshot format fields",
24+
(payload) => {
25+
expect(() => parseCanvasSnapshotPayload(payload)).toThrow(
26+
/invalid canvas\.snapshot payload/i,
27+
);
28+
},
29+
);
30+
31+
it.each(["/../../target.sh", "../target.sh", "png/../../target.sh", "image/png", ""])(
32+
"rejects unsafe canvas.snapshot formats from responses: %s",
33+
(format) => {
34+
expect(() => parseCanvasSnapshotPayload({ format, base64: "aGk=" })).toThrow(
35+
/invalid canvas\.snapshot payload/i,
36+
);
37+
},
38+
);
39+
40+
it("normalizes supported snapshot file extensions", () => {
41+
expect(normalizeCanvasSnapshotFileExtension("png")).toBe("png");
42+
expect(normalizeCanvasSnapshotFileExtension(".jpeg")).toBe("jpg");
43+
expect(normalizeCanvasSnapshotFileExtension(" JPG ")).toBe("jpg");
44+
});
45+
46+
it("rejects unsafe snapshot temp path parts", () => {
47+
expect(() =>
48+
canvasSnapshotTempPath({
49+
tmpDir: "/tmp/openclaw-canvas-test",
50+
id: "snapshot",
51+
ext: "/../../target.sh",
52+
}),
53+
).toThrow(/invalid canvas\.snapshot format/i);
54+
expect(() =>
55+
canvasSnapshotTempPath({
56+
tmpDir: "/tmp/openclaw-canvas-test",
57+
id: "../../snapshot",
58+
ext: "png",
59+
}),
60+
).toThrow(/invalid canvas snapshot id/i);
61+
});
1762
});

‎extensions/canvas/src/cli-helpers.ts‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,32 @@ import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/security-run
55
import { asRecord, readStringValue } from "openclaw/plugin-sdk/string-coerce-runtime";
66

77
type CanvasSnapshotPayload = {
8-
format: string;
8+
format: CanvasSnapshotFormat;
99
base64: string;
1010
};
1111

12+
type CanvasSnapshotFormat = "png" | "jpg" | "jpeg";
13+
type CanvasSnapshotFileExtension = "png" | "jpg";
14+
15+
function normalizeCanvasSnapshotFormat(value: string | undefined): CanvasSnapshotFormat | null {
16+
const format = value?.trim().toLowerCase() ?? "";
17+
if (format === "png" || format === "jpg" || format === "jpeg") {
18+
return format;
19+
}
20+
return null;
21+
}
22+
23+
export function normalizeCanvasSnapshotFileExtension(value: string): CanvasSnapshotFileExtension {
24+
const format = normalizeCanvasSnapshotFormat(value.startsWith(".") ? value.slice(1) : value);
25+
if (!format) {
26+
throw new Error("invalid canvas.snapshot format");
27+
}
28+
return format === "jpeg" ? "jpg" : format;
29+
}
30+
1231
export function parseCanvasSnapshotPayload(value: unknown): CanvasSnapshotPayload {
1332
const obj = asRecord(value);
14-
const format = readStringValue(obj.format);
33+
const format = normalizeCanvasSnapshotFormat(readStringValue(obj.format));
1534
const base64 = readStringValue(obj.base64);
1635
if (!format || !base64) {
1736
throw new Error("invalid canvas.snapshot payload");
@@ -23,15 +42,22 @@ function resolveCliName(): string {
2342
return "openclaw";
2443
}
2544

45+
function resolveCanvasSnapshotId(id: string): string {
46+
if (!/^[A-Za-z0-9_-]+$/.test(id)) {
47+
throw new Error("invalid canvas snapshot id");
48+
}
49+
return id;
50+
}
51+
2652
function resolveTempPathParts(opts: { ext: string; tmpDir?: string; id?: string }) {
2753
const tmpDir = opts.tmpDir ?? resolvePreferredOpenClawTmpDir();
2854
if (!opts.tmpDir) {
2955
fs.mkdirSync(tmpDir, { recursive: true, mode: 0o700 });
3056
}
3157
return {
3258
tmpDir,
33-
id: opts.id ?? randomUUID(),
34-
ext: opts.ext.startsWith(".") ? opts.ext : `.${opts.ext}`,
59+
id: resolveCanvasSnapshotId(opts.id ?? randomUUID()),
60+
ext: `.${normalizeCanvasSnapshotFileExtension(opts.ext)}`,
3561
};
3662
}
3763

‎extensions/canvas/src/cli.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,26 @@ describe("canvas CLI", () => {
9191
expect(mediaMessage?.startsWith("MEDIA:")).toBe(true);
9292
expect(mediaMessage?.endsWith(".png")).toBe(true);
9393
});
94+
95+
it("rejects node-controlled snapshot formats before writing", async () => {
96+
const program = new Command();
97+
program.exitOverride();
98+
const nodes = program.command("nodes");
99+
const { deps, writtenFiles } = createCanvasCliDeps();
100+
vi.mocked(deps.callGatewayCli).mockResolvedValueOnce({
101+
payload: {
102+
format: "/../../target.sh",
103+
base64: "aGk=",
104+
},
105+
});
106+
107+
registerNodesCanvasCommands(nodes, deps);
108+
109+
await expect(
110+
program.parseAsync(["nodes", "canvas", "snapshot", "--node", "ios-node"], {
111+
from: "user",
112+
}),
113+
).rejects.toThrow(/invalid canvas\.snapshot payload/i);
114+
expect(writtenFiles).toHaveLength(0);
115+
});
94116
});

‎extensions/canvas/src/tool.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,19 @@ describe("Canvas tool", () => {
9696
expect(imageResultParams?.details).toEqual({ format: "png" });
9797
expect(imageResultParams?.imageSanitization).toEqual({ maxDimensionPx: 1600 });
9898
});
99+
100+
it("rejects node-controlled snapshot formats before creating image results", async () => {
101+
mocks.callGatewayTool.mockResolvedValue({
102+
payload: {
103+
format: "/../../target.sh",
104+
base64: Buffer.from("not-a-real-png").toString("base64"),
105+
},
106+
});
107+
const tool = createCanvasTool();
108+
109+
await expect(tool.execute("tool-call-1", { action: "snapshot" })).rejects.toThrow(
110+
/invalid canvas\.snapshot payload/i,
111+
);
112+
expect(mocks.imageResultFromFile).not.toHaveBeenCalled();
113+
});
99114
});

‎extensions/canvas/src/tool.ts‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,14 @@ import {
1313
} from "openclaw/plugin-sdk/channel-actions";
1414
import type { AnyAgentTool, OpenClawConfig } from "openclaw/plugin-sdk/plugin-entry";
1515
import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path";
16+
import { normalizeCanvasSnapshotFileExtension, parseCanvasSnapshotPayload } from "./cli-helpers.js";
1617
import { CanvasToolSchema } from "./tool-schema.js";
1718

1819
type CanvasToolOptions = {
1920
config?: OpenClawConfig;
2021
workspaceDir?: string;
2122
};
2223

23-
type CanvasSnapshotPayload = {
24-
format: string;
25-
base64: string;
26-
};
27-
2824
type CanvasImageSanitizationLimits = {
2925
maxDimensionPx?: number;
3026
};
@@ -45,23 +41,10 @@ async function resolveNodeId(
4541
return resolveNodeIdFromList(await listNodes(opts), query, allowDefault);
4642
}
4743

48-
function parseCanvasSnapshotPayload(value: unknown): CanvasSnapshotPayload {
49-
if (!value || typeof value !== "object" || Array.isArray(value)) {
50-
throw new Error("invalid canvas.snapshot payload");
51-
}
52-
const record = value as Record<string, unknown>;
53-
const format = typeof record.format === "string" ? record.format : "";
54-
const base64 = typeof record.base64 === "string" ? record.base64 : "";
55-
if (!format || !base64) {
56-
throw new Error("invalid canvas.snapshot payload");
57-
}
58-
return { format, base64 };
59-
}
60-
6144
async function writeBase64ToTempFile(params: { base64: string; ext: string }): Promise<string> {
6245
const dir = resolvePreferredOpenClawTmpDir();
6346
await fs.mkdir(dir, { recursive: true, mode: 0o700 });
64-
const ext = params.ext.startsWith(".") ? params.ext : `.${params.ext}`;
47+
const ext = `.${normalizeCanvasSnapshotFileExtension(params.ext)}`;
6548
const filePath = path.join(dir, `openclaw-canvas-snapshot-${randomUUID()}${ext}`);
6649
await fs.writeFile(filePath, Buffer.from(params.base64, "base64"));
6750
return filePath;

0 commit comments

Comments
 (0)