From 479bbb240f257be9342e533d21770c58e3c01178 Mon Sep 17 00:00:00 2001 From: XIN PENG Date: Tue, 17 Feb 2026 08:58:41 -0800 Subject: [PATCH] fix: avoid shell interpolation in image conversion commands --- src/tools/look-at/image-converter.test.ts | 60 +++++++++++++++++++++++ src/tools/look-at/image-converter.ts | 6 +-- 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 src/tools/look-at/image-converter.test.ts diff --git a/src/tools/look-at/image-converter.test.ts b/src/tools/look-at/image-converter.test.ts new file mode 100644 index 00000000..37c9cdf2 --- /dev/null +++ b/src/tools/look-at/image-converter.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test, mock, beforeEach } from "bun:test" +import { existsSync, mkdtempSync, writeFileSync, unlinkSync, rmSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" + +const originalChildProcess = await import("node:child_process") + +const execFileSyncMock = mock((_command: string, _args: string[]) => "") +const execSyncMock = mock(() => { + throw new Error("execSync should not be called") +}) + +mock.module("node:child_process", () => ({ + ...originalChildProcess, + execFileSync: execFileSyncMock, + execSync: execSyncMock, +})) + +const { convertImageToJpeg } = await import("./image-converter") + +describe("image-converter command execution safety", () => { + beforeEach(() => { + execFileSyncMock.mockReset() + execSyncMock.mockReset() + }) + + test("uses execFileSync with argument arrays for conversion commands", () => { + const testDir = mkdtempSync(join(tmpdir(), "img-converter-test-")) + const inputPath = join(testDir, "evil$(touch_pwn).heic") + writeFileSync(inputPath, "fake-heic-data") + + execFileSyncMock.mockImplementation((command: string, args: string[]) => { + if (command === "sips") { + const outIndex = args.indexOf("--out") + const outputPath = outIndex >= 0 ? args[outIndex + 1] : undefined + if (outputPath) writeFileSync(outputPath, "jpeg") + } else if (command === "convert") { + writeFileSync(args[1], "jpeg") + } + return "" + }) + + const outputPath = convertImageToJpeg(inputPath, "image/heic") + + expect(execSyncMock).not.toHaveBeenCalled() + expect(execFileSyncMock).toHaveBeenCalled() + + const [firstCommand, firstArgs] = execFileSyncMock.mock.calls[0] as [string, string[]] + expect(typeof firstCommand).toBe("string") + expect(Array.isArray(firstArgs)).toBe(true) + expect(firstArgs).toContain(inputPath) + expect(firstArgs.join(" ")).not.toContain(`\"${inputPath}\"`) + + expect(existsSync(outputPath)).toBe(true) + + if (existsSync(outputPath)) unlinkSync(outputPath) + if (existsSync(inputPath)) unlinkSync(inputPath) + rmSync(testDir, { recursive: true, force: true }) + }) +}) diff --git a/src/tools/look-at/image-converter.ts b/src/tools/look-at/image-converter.ts index 6718bd0f..e95237ba 100644 --- a/src/tools/look-at/image-converter.ts +++ b/src/tools/look-at/image-converter.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process" +import { execFileSync } from "node:child_process" import { existsSync, mkdtempSync, unlinkSync, writeFileSync, readFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" @@ -57,7 +57,7 @@ export function convertImageToJpeg(inputPath: string, mimeType: string): string try { if (process.platform === "darwin") { try { - execSync(`sips -s format jpeg "${inputPath}" --out "${outputPath}"`, { + execFileSync("sips", ["-s", "format", "jpeg", inputPath, "--out", outputPath], { stdio: "pipe", encoding: "utf-8", }) @@ -72,7 +72,7 @@ export function convertImageToJpeg(inputPath: string, mimeType: string): string } try { - execSync(`convert "${inputPath}" "${outputPath}"`, { + execFileSync("convert", [inputPath, outputPath], { stdio: "pipe", encoding: "utf-8", })