From aed2b36eda6b03f4eb036774035495f9fb294a18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Cabanillas?= <76893296+Nicoo01x@users.noreply.github.com> Date: Sat, 21 Feb 2026 14:16:05 -0300 Subject: [PATCH] fix(opencode): prevent cross-drive path bypass in Filesystem.contains on Windows On Windows, path.relative() across different drive letters returns the absolute child path instead of a ".." prefixed relative path, causing Filesystem.contains() to incorrectly report that a path on a different drive is inside the project directory. Add a drive letter comparison guard before the path.relative() check. Remove resolved TODO comments from file/index.ts. Add Windows-specific tests for cross-drive, same-drive, and case-insensitive drive letter scenarios. Closes #14579 --- packages/opencode/src/file/index.ts | 2 -- packages/opencode/src/util/filesystem.ts | 8 +++++++ .../opencode/test/file/path-traversal.test.ts | 22 +++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/file/index.ts b/packages/opencode/src/file/index.ts index b7daddc5fb8..60694521c50 100644 --- a/packages/opencode/src/file/index.ts +++ b/packages/opencode/src/file/index.ts @@ -497,7 +497,6 @@ export namespace File { const full = path.join(Instance.directory, file) // TODO: Filesystem.contains is lexical only - symlinks inside the project can escape. - // TODO: On Windows, cross-drive paths bypass this check. Consider realpath canonicalization. if (!Instance.containsPath(full)) { throw new Error(`Access denied: path escapes project directory`) } @@ -573,7 +572,6 @@ export namespace File { const resolved = dir ? path.join(Instance.directory, dir) : Instance.directory // TODO: Filesystem.contains is lexical only - symlinks inside the project can escape. - // TODO: On Windows, cross-drive paths bypass this check. Consider realpath canonicalization. if (!Instance.containsPath(resolved)) { throw new Error(`Access denied: path escapes project directory`) } diff --git a/packages/opencode/src/util/filesystem.ts b/packages/opencode/src/util/filesystem.ts index 3a1e8b8ec15..b6f68f3a71c 100644 --- a/packages/opencode/src/util/filesystem.ts +++ b/packages/opencode/src/util/filesystem.ts @@ -120,6 +120,14 @@ export namespace Filesystem { } export function contains(parent: string, child: string) { + // On Windows, path.relative across different drive letters (e.g. D:\project vs C:\evil) + // returns the absolute child path instead of a ".." prefixed relative path, + // which causes a false positive (the check thinks the child is inside the parent). + if (process.platform === "win32") { + const parentRoot = parent.slice(0, 3).toUpperCase() + const childRoot = child.slice(0, 3).toUpperCase() + if (parentRoot !== childRoot) return false + } return !relative(parent, child).startsWith("..") } diff --git a/packages/opencode/test/file/path-traversal.test.ts b/packages/opencode/test/file/path-traversal.test.ts index 44ae8f15435..3dfe2684ea7 100644 --- a/packages/opencode/test/file/path-traversal.test.ts +++ b/packages/opencode/test/file/path-traversal.test.ts @@ -29,6 +29,28 @@ describe("Filesystem.contains", () => { expect(Filesystem.contains("/project", "/project-other/file")).toBe(false) expect(Filesystem.contains("/project", "/projectfile")).toBe(false) }) + + if (process.platform === "win32") { + test("blocks cross-drive paths on Windows", () => { + expect(Filesystem.contains("D:\\project", "C:\\evil\\file.txt")).toBe(false) + expect(Filesystem.contains("D:\\project", "C:\\Windows\\System32\\config")).toBe(false) + expect(Filesystem.contains("C:\\Users\\user\\project", "D:\\other\\file")).toBe(false) + }) + + test("allows same-drive paths on Windows", () => { + expect(Filesystem.contains("D:\\project", "D:\\project\\src\\file.ts")).toBe(true) + expect(Filesystem.contains("C:\\Users\\project", "C:\\Users\\project\\file.ts")).toBe(true) + }) + + test("handles case-insensitive drive letters on Windows", () => { + expect(Filesystem.contains("d:\\project", "D:\\project\\file.ts")).toBe(true) + expect(Filesystem.contains("D:\\project", "d:\\project\\file.ts")).toBe(true) + }) + + test("blocks cross-drive with forward slashes on Windows", () => { + expect(Filesystem.contains("D:/project", "C:/evil/file.txt")).toBe(false) + }) + } }) /*