diff --git a/packages/opencode/src/patch/index.ts b/packages/opencode/src/patch/index.ts index b87ad555286..c56f38c229a 100644 --- a/packages/opencode/src/patch/index.ts +++ b/packages/opencode/src/patch/index.ts @@ -3,6 +3,7 @@ import * as path from "path" import * as fs from "fs/promises" import { readFileSync } from "fs" import { Log } from "../util/log" +import { Filesystem } from "../util/filesystem" export namespace Patch { const log = Log.create({ service: "patch" }) @@ -528,6 +529,7 @@ export namespace Patch { switch (hunk.type) { case "add": // Create parent directories + Filesystem.assertSafeWindowsPath(hunk.path) const addDir = path.dirname(hunk.path) if (addDir !== "." && addDir !== "/") { await fs.mkdir(addDir, { recursive: true }) @@ -539,16 +541,19 @@ export namespace Patch { break case "delete": + Filesystem.assertSafeWindowsPath(hunk.path) await fs.unlink(hunk.path) deleted.push(hunk.path) log.info(`Deleted file: ${hunk.path}`) break case "update": + Filesystem.assertSafeWindowsPath(hunk.path) const fileUpdate = deriveNewContentsFromChunks(hunk.path, hunk.chunks) if (hunk.move_path) { // Handle file move + Filesystem.assertSafeWindowsPath(hunk.move_path) const moveDir = path.dirname(hunk.move_path) if (moveDir !== "." && moveDir !== "/") { await fs.mkdir(moveDir, { recursive: true }) diff --git a/packages/opencode/src/tool/apply_patch.ts b/packages/opencode/src/tool/apply_patch.ts index 06293b6eba6..945304f0ba1 100644 --- a/packages/opencode/src/tool/apply_patch.ts +++ b/packages/opencode/src/tool/apply_patch.ts @@ -59,6 +59,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", { for (const hunk of hunks) { const filePath = path.resolve(Instance.directory, hunk.path) + Filesystem.assertSafeWindowsPath(filePath) await assertExternalDirectory(ctx, filePath) switch (hunk.type) { @@ -117,6 +118,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", { } const movePath = hunk.move_path ? path.resolve(Instance.directory, hunk.move_path) : undefined + if (movePath) Filesystem.assertSafeWindowsPath(movePath) await assertExternalDirectory(ctx, movePath) fileChanges.push({ diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 109a665363b..f7bef43dbf3 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -21,8 +21,82 @@ import { Plugin } from "@/plugin" const MAX_METADATA_LENGTH = 30_000 const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000 +interface ShellNodeLike { + type: string + text: string + namedChildCount: number + namedChild(index: number): ShellNodeLike | null + descendantsOfType(type: string): ShellNodeLike[] +} + export const log = Log.create({ service: "bash-tool" }) +export function isWindowsPosixShell(shell: string) { + const basename = + shell + .split(/[\\/]+/) + .pop() + ?.toLowerCase() ?? "" + return !["cmd", "cmd.exe", "powershell", "powershell.exe", "pwsh", "pwsh.exe"].includes(basename) +} + +function literalShellWord(node: ShellNodeLike): string | undefined { + switch (node.type) { + case "word": + return node.text.replace(/\\(.)/g, "$1") + case "string_content": + return node.text.replace(/\\(["$`\\\n])/g, "$1") + case "raw_string": + return node.text.replace(/^'/, "").replace(/'$/, "") + case "string": + case "concatenation": { + if (node.namedChildCount === 0) { + return node.text.replace(/^['"]/, "").replace(/['"]$/, "") + } + let result = "" + for (let i = 0; i < node.namedChildCount; i++) { + const child = node.namedChild(i) + if (!child) continue + const text = literalShellWord(child) + if (text === undefined) return undefined + result += text + } + return result + } + default: + return undefined + } +} + +function literalRedirectTarget(node: ShellNodeLike): string | undefined { + const target = node.namedChild(node.namedChildCount - 1) + if (!target) return undefined + return literalShellWord(target) +} + +export function hasLiteralWindowsReservedRedirect(rootNode: ShellNodeLike) { + for (const redirect of rootNode.descendantsOfType("file_redirect")) { + const target = literalRedirectTarget(redirect) + if (!target) continue + if (Filesystem.hasReservedWindowsBasename(target)) return true + } + return false +} + +export async function hasLiteralWindowsReservedRedirectInCommand(command: string) { + const tree = await parser().then((p) => p.parse(command)) + return tree ? hasLiteralWindowsReservedRedirect(tree.rootNode as ShellNodeLike) : false +} + +export function assertWindowsLiteralRedirectCompatibility(rootNode: ShellNodeLike, shell: string) { + if (process.platform !== "win32") return + if (!isWindowsPosixShell(shell)) return + if (!hasLiteralWindowsReservedRedirect(rootNode)) return + throw new Error( + "On Windows, the bash tool may run in a POSIX shell such as Git Bash. Redirecting output to a reserved Windows name like `NUL` can create a literal artifact. Use `/dev/null` instead, or wrap the command with `cmd /c` if you need cmd.exe semantics.", + ) +} + const resolveWasm = (asset: string) => { if (asset.startsWith("file://")) return fileURLToPath(asset) if (asset.startsWith("/") || /^[a-z]:/i.test(asset)) return asset @@ -85,6 +159,7 @@ export const BashTool = Tool.define("bash", async () => { if (!tree) { throw new Error("Failed to parse command") } + assertWindowsLiteralRedirectCompatibility(tree.rootNode as ShellNodeLike, shell) const directories = new Set() if (!Instance.containsPath(cwd)) directories.add(cwd) const patterns = new Set() diff --git a/packages/opencode/src/tool/bash.txt b/packages/opencode/src/tool/bash.txt index baafb00810a..9773fe59154 100644 --- a/packages/opencode/src/tool/bash.txt +++ b/packages/opencode/src/tool/bash.txt @@ -17,6 +17,7 @@ Before executing the command, please follow these steps: - mkdir /Users/name/My Documents (incorrect - will fail) - python "/path/with spaces/script.py" (correct) - python /path/with spaces/script.py (incorrect - will fail) + - On Windows, commands may run in Git Bash (or another POSIX shell) rather than `cmd.exe`. Redirect discarded output to `/dev/null`, not `NUL`. - After ensuring proper quoting, execute the command. - Capture the output of the command. diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 1a7614fc17f..63a811ef006 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -51,6 +51,7 @@ export const EditTool = Tool.define("edit", { } const filePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) + Filesystem.assertSafeWindowsPath(filePath) await assertExternalDirectory(ctx, filePath) let diff = "" diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index 83474a543ca..0182c521ff6 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -24,6 +24,7 @@ export const WriteTool = Tool.define("write", { }), async execute(params, ctx) { const filepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) + Filesystem.assertSafeWindowsPath(filepath) await assertExternalDirectory(ctx, filepath) const exists = await Filesystem.exists(filepath) diff --git a/packages/opencode/src/util/filesystem.ts b/packages/opencode/src/util/filesystem.ts index 37f00c6b9c8..c0d790071e7 100644 --- a/packages/opencode/src/util/filesystem.ts +++ b/packages/opencode/src/util/filesystem.ts @@ -8,6 +8,12 @@ import { pipeline } from "stream/promises" import { Glob } from "./glob" export namespace Filesystem { + const WINDOWS_RESERVED_DEVICE_INDEX = "[1-9\u00b9\u00b2\u00b3]" + const WINDOWS_RESERVED_BASENAME = new RegExp( + `^(con|prn|aux|nul|com${WINDOWS_RESERVED_DEVICE_INDEX}|lpt${WINDOWS_RESERVED_DEVICE_INDEX})(?:\\..*)?$`, + "i", + ) + // Fast sync version for metadata checks export async function exists(p: string): Promise { return existsSync(p) @@ -51,7 +57,28 @@ export namespace Filesystem { return typeof e === "object" && e !== null && "code" in e && (e as { code: string }).code === "ENOENT" } + export function hasReservedWindowsBasename(p: string): boolean { + if (process.platform !== "win32") return false + const normalized = windowsPath(p).replace(/^\\\\\?\\/, "") + return normalized + .split(/[\\/]+/) + .filter(Boolean) + .some((segment) => { + if (/^[a-zA-Z]:$/.test(segment)) return false + const trimmed = segment.replace(/[ .]+$/g, "") + return trimmed.length > 0 && WINDOWS_RESERVED_BASENAME.test(trimmed) + }) + } + + export function assertSafeWindowsPath(p: string): void { + if (!hasReservedWindowsBasename(p)) return + throw new Error( + `Path contains a reserved Windows name: ${p}. Names like NUL, CON, PRN, AUX, COM1-COM9, and LPT1-LPT9 are not allowed.`, + ) + } + export async function write(p: string, content: string | Buffer | Uint8Array, mode?: number): Promise { + assertSafeWindowsPath(p) try { if (mode) { await writeFile(p, content, { mode }) @@ -81,6 +108,7 @@ export namespace Filesystem { stream: ReadableStream | Readable, mode?: number, ): Promise { + assertSafeWindowsPath(p) const dir = dirname(p) if (!existsSync(dir)) { await mkdir(dir, { recursive: true }) diff --git a/packages/opencode/test/tool/apply_patch.test.ts b/packages/opencode/test/tool/apply_patch.test.ts index 4e276517f10..f339c901ed5 100644 --- a/packages/opencode/test/tool/apply_patch.test.ts +++ b/packages/opencode/test/tool/apply_patch.test.ts @@ -75,6 +75,22 @@ describe("tool.apply_patch freeform", () => { await expect(execute({ patchText: emptyPatch }, ctx)).rejects.toThrow("patch rejected: empty patch") }) + test("rejects reserved Windows filenames", async () => { + if (process.platform !== "win32") return + await using fixture = await tmpdir({ git: true }) + const { ctx } = makeCtx() + + await Instance.provide({ + directory: fixture.path, + fn: async () => { + const addPatch = "*** Begin Patch\n*** Add File: NUL\n+bad\n*** End Patch" + const deletePatch = "*** Begin Patch\n*** Delete File: NUL\n*** End Patch" + await expect(execute({ patchText: addPatch }, ctx)).rejects.toThrow("reserved Windows name") + await expect(execute({ patchText: deletePatch }, ctx)).rejects.toThrow("reserved Windows name") + }, + }) + }) + test("applies add/update/delete in one patch", async () => { await using fixture = await tmpdir({ git: true }) const { ctx, calls } = makeCtx() diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index f947398b37e..cfb0c00c538 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test" import os from "os" import path from "path" -import { BashTool } from "../../src/tool/bash" +import { BashTool, hasLiteralWindowsReservedRedirectInCommand, isWindowsPosixShell } from "../../src/tool/bash" import { Instance } from "../../src/project/instance" import { Filesystem } from "../../src/util/filesystem" import { tmpdir } from "../fixture/fixture" @@ -40,6 +40,27 @@ describe("tool.bash", () => { }, }) }) + + test("detects Windows POSIX shells", () => { + if (process.platform !== "win32") return + expect(isWindowsPosixShell("C:/Program Files/Git/bin/bash.exe")).toBe(true) + expect(isWindowsPosixShell("C:/Windows/System32/cmd.exe")).toBe(false) + }) + + test("detects literal reserved redirect targets", async () => { + if (process.platform !== "win32") return + expect(await hasLiteralWindowsReservedRedirectInCommand("dir >NUL")).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand("dir 2>NUL")).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand('dir >"NUL"')).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand('dir >N"UL"')).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand("dir >NU\\L")).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand('dir >"./NUL"')).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand('printf x > "./NUL/out.txt"')).toBe(true) + expect(await hasLiteralWindowsReservedRedirectInCommand('dir >"NU\\L"')).toBe(false) + expect(await hasLiteralWindowsReservedRedirectInCommand('cmd /c "dir >NUL 2>&1"')).toBe(false) + expect(await hasLiteralWindowsReservedRedirectInCommand('dir >"$OUT"')).toBe(false) + expect(await hasLiteralWindowsReservedRedirectInCommand('printf x >"$(printf NUL)"')).toBe(false) + }) }) describe("tool.bash permissions", () => { diff --git a/packages/opencode/test/tool/edit.test.ts b/packages/opencode/test/tool/edit.test.ts index 7b6784cf49a..c383ecdedeb 100644 --- a/packages/opencode/test/tool/edit.test.ts +++ b/packages/opencode/test/tool/edit.test.ts @@ -73,6 +73,29 @@ describe("tool.edit", () => { }) }) + test("rejects reserved Windows filenames when creating files", async () => { + if (process.platform !== "win32") return + await using tmp = await tmpdir() + const filepath = path.join(tmp.path, "NUL") + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const edit = await EditTool.init() + await expect( + edit.execute( + { + filePath: filepath, + oldString: "", + newString: "new content", + }, + ctx, + ), + ).rejects.toThrow("reserved Windows name") + }, + }) + }) + test("emits add event for new files", async () => { await using tmp = await tmpdir() const filepath = path.join(tmp.path, "new.txt") diff --git a/packages/opencode/test/tool/write.test.ts b/packages/opencode/test/tool/write.test.ts index af002a39100..459c02aff92 100644 --- a/packages/opencode/test/tool/write.test.ts +++ b/packages/opencode/test/tool/write.test.ts @@ -86,6 +86,28 @@ describe("tool.write", () => { }, }) }) + + test("rejects reserved Windows filenames", async () => { + if (process.platform !== "win32") return + await using tmp = await tmpdir() + const filepath = path.join(tmp.path, "NUL") + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const write = await WriteTool.init() + await expect( + write.execute( + { + filePath: filepath, + content: "Hello, World!", + }, + ctx, + ), + ).rejects.toThrow("reserved Windows name") + }, + }) + }) }) describe("existing file overwrite", () => { diff --git a/packages/opencode/test/util/filesystem.test.ts b/packages/opencode/test/util/filesystem.test.ts index aea0b1db87f..7c1af257eaf 100644 --- a/packages/opencode/test/util/filesystem.test.ts +++ b/packages/opencode/test/util/filesystem.test.ts @@ -555,4 +555,26 @@ describe("filesystem", () => { expect(() => Filesystem.resolve(path.join(file, "child"))).toThrow() }) }) + + describe("Windows reserved names", () => { + test("detects reserved Windows basenames", () => { + if (process.platform !== "win32") return + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\NUL")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\nul.txt")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\NUL.")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\NUL ")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\COM¹")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\LPT².txt")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\nested\\COM1.log")).toBe(true) + expect(Filesystem.hasReservedWindowsBasename("C:\\temp\\notes.txt")).toBe(false) + }) + + test("rejects reserved Windows basenames", () => { + if (process.platform !== "win32") return + expect(() => Filesystem.assertSafeWindowsPath("C:\\temp\\NUL")).toThrow("reserved Windows name") + expect(() => Filesystem.assertSafeWindowsPath("C:\\temp\\NUL.")).toThrow("reserved Windows name") + expect(() => Filesystem.assertSafeWindowsPath("C:\\temp\\COM³")).toThrow("reserved Windows name") + expect(() => Filesystem.assertSafeWindowsPath("C:\\temp\\dir\\LPT1.txt")).toThrow("reserved Windows name") + }) + }) })