diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index a49af2181..13f330133 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -186,9 +186,9 @@ export namespace AppFileSystem { return lookup(path) || "application/octet-stream" } - export function normalizePath(path: string): string { + export function normalizePath(path: string, options?: WindowsPathOptions): string { if (process.platform !== "win32") return path - const resolved = normalizeWindowsAbsolutePath(windowsPath(path)) + const resolved = normalizeWindowsPath(path, options) try { return realpathSync.native(resolved) } catch { @@ -196,18 +196,17 @@ export namespace AppFileSystem { } } - export function normalizePathPattern(path: string): string { + export function normalizePathPattern(path: string, options?: WindowsPathOptions): string { if (process.platform !== "win32") return path if (path === "*") return path const match = path.match(/^(.*)[\\/]\*$/) - if (!match) return normalizePath(path) + if (!match) return normalizePath(path, options) const dir = /^[A-Za-z]:$/.test(match[1]) ? match[1] + "\\" : match[1] - return join(normalizePath(dir), "*") + return join(normalizePath(dir, options), "*") } - export function resolve(path: string): string { - const resolved = - process.platform === "win32" ? normalizeWindowsAbsolutePath(windowsPath(path)) : pathResolve(path) + export function resolve(path: string, options?: WindowsPathOptions): string { + const resolved = process.platform === "win32" ? normalizeWindowsPath(path, options) : pathResolve(path) try { return normalizePath(realpathSync(resolved)) } catch (error: any) { @@ -218,6 +217,10 @@ export namespace AppFileSystem { export function windowsPath(path: string): string { if (process.platform !== "win32") return path + return normalizeWindowsShellInput(path) + } + + function normalizeWindowsShellInput(path: string): string { return path .replace(/^\/([a-zA-Z]):(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`) .replace(/^\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`) @@ -225,28 +228,65 @@ export namespace AppFileSystem { .replace(/^\/mnt\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`) } + export type WindowsPathOptions = { + base?: string + driveRoots?: string[] + exists?: (path: string) => boolean + } + // Rooted but driveless paths (e.g. "/users/runner/...") arrive when callers // strip the drive letter before handing the path back. path.resolve would // bind such a path to the cwd's drive, which silently mismatches when the - // file actually lives on a different drive. Probe each known drive root for - // an existing file and fall back to win32.resolve only when none match. - // - // Important: this only repairs *existing* rooted-driveless paths. Targets - // that have not been created yet (e.g. write destinations) still fall back - // to the cwd-drive answer that win32.resolve produces. Callers that need - // a stable drive for a future path should pre-resolve via the project root. - function normalizeWindowsAbsolutePath(p: string): string { - const existing = resolveRootedWindowsVariant(p) - return win32.normalize(existing ?? win32.resolve(p)) + // file actually lives on a different drive. Probe known drive roots for an + // existing file; if none match, bind future targets to the explicit base + // drive when one is available. + export function normalizeWindowsPath(path: string, options: WindowsPathOptions = {}): string { + const p = stripExtendedLengthPrefix(normalizeWindowsShellInput(path)) + const existing = resolveRootedWindowsVariant(p, options) + if (existing) return uppercaseDriveRoot(win32.normalize(existing)) + if (isRootedDriveless(p) && options.base) return uppercaseDriveRoot(win32.resolve(options.base, p)) + return uppercaseDriveRoot(win32.normalize(win32.resolve(p))) } - function resolveRootedWindowsVariant(p: string): string | undefined { - if (!/^[\\/](?![\\/])/.test(p)) return + function stripExtendedLengthPrefix(path: string): string { + if (/^\\\\\?\\UNC\\/i.test(path)) return path.replace(/^\\\\\?\\UNC\\/i, "\\\\") + if (/^\\\\\?\\[A-Za-z]:\\/i.test(path)) return path.slice(4) + return path + } + + function resolveRootedWindowsVariant(p: string, options: WindowsPathOptions): string | undefined { + if (!isRootedDriveless(p)) return const suffix = p.replace(/^[\\/]+/, "").replaceAll("/", "\\") - for (const root of windowsDriveRoots()) { + const matches: string[] = [] + for (const root of uniqueWindowsDriveRoots(options.driveRoots ?? windowsDriveRoots())) { const candidate = win32.join(root, suffix) - if (existsSync(candidate)) return candidate + if ((options.exists ?? existsSync)(candidate)) matches.push(candidate) + } + if (matches.length > 1) { + throw new Error(`Ambiguous Windows path ${p}; use a drive-qualified path.`) + } + return matches[0] + } + + function uniqueWindowsDriveRoots(roots: string[]): string[] { + const result: string[] = [] + const seen = new Set() + for (const root of roots) { + const normalized = uppercaseDriveRoot(win32.normalize(root)) + const key = normalized.toUpperCase() + if (seen.has(key)) continue + seen.add(key) + result.push(normalized) } + return result + } + + function isRootedDriveless(p: string): boolean { + return /^[\\/](?![\\/])/.test(p) + } + + function uppercaseDriveRoot(path: string): string { + return path.replace(/^([a-z]):/, (_, drive) => `${drive.toUpperCase()}:`) } function windowsDriveRoots(): string[] { diff --git a/packages/core/test/filesystem/normalize-path.test.ts b/packages/core/test/filesystem/normalize-path.test.ts index d81fb49d6..557db5dea 100644 --- a/packages/core/test/filesystem/normalize-path.test.ts +++ b/packages/core/test/filesystem/normalize-path.test.ts @@ -14,6 +14,59 @@ test("normalizePath is identity on non-Windows", () => { expect(AppFileSystem.normalizePath(p)).toBe(p) }) +function withWin32Platform(fn: () => void) { + const descriptor = Object.getOwnPropertyDescriptor(process, "platform") + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + try { + fn() + } finally { + if (descriptor) Object.defineProperty(process, "platform", descriptor) + } +} + +test("normalizePath folds extended-length drive paths on Windows", () => { + withWin32Platform(() => { + expect(AppFileSystem.normalizePath("\\\\?\\D:\\Users\\Ada\\file.txt")).toBe("D:\\Users\\Ada\\file.txt") + }) +}) + +test("normalizePath folds extended-length UNC paths on Windows", () => { + withWin32Platform(() => { + expect(AppFileSystem.normalizePath("\\\\?\\UNC\\server\\share\\dir\\file.txt")).toBe( + "\\\\server\\share\\dir\\file.txt", + ) + }) +}) + +test("normalizeWindowsPath resolves non-existing rooted-driveless paths from an explicit base drive", () => { + expect(AppFileSystem.normalizeWindowsPath("\\future\\file.txt", { base: "D:\\project\\work" })).toBe( + "D:\\future\\file.txt", + ) +}) + +test("normalizeWindowsPath uppercases drive letters for non-existing drive-qualified paths", () => { + expect(AppFileSystem.normalizeWindowsPath("d:\\future\\file.txt")).toBe("D:\\future\\file.txt") +}) + +test("normalizeWindowsPath rejects ambiguous rooted-driveless existing paths", () => { + expect(() => + AppFileSystem.normalizeWindowsPath("\\shared\\file.txt", { + driveRoots: ["C:\\", "D:\\"], + exists: (candidate) => + candidate.toLowerCase() === "c:\\shared\\file.txt" || candidate.toLowerCase() === "d:\\shared\\file.txt", + }), + ).toThrow("Ambiguous Windows path") +}) + +test("normalizeWindowsPath de-duplicates caller-supplied drive roots before ambiguity checks", () => { + expect( + AppFileSystem.normalizeWindowsPath("\\shared\\file.txt", { + driveRoots: ["C:\\", "c:\\"], + exists: (candidate) => candidate.toLowerCase() === "c:\\shared\\file.txt", + }), + ).toBe("C:\\shared\\file.txt") +}) + test.skipIf(process.platform !== "win32")( "normalizePath probes drive roots for rooted-but-driveless paths to existing files", () => { diff --git a/packages/opencode/src/tool/apply_patch.ts b/packages/opencode/src/tool/apply_patch.ts index c8ddf7784..e60d38906 100644 --- a/packages/opencode/src/tool/apply_patch.ts +++ b/packages/opencode/src/tool/apply_patch.ts @@ -99,8 +99,8 @@ export const ApplyPatchTool = Tool.define( let totalDiff = "" for (const hunk of hunks) { - const filePath = path.resolve(Instance.directory, hunk.path) - yield* assertExternalDirectoryEffect(ctx, filePath) + const rawFilePath = path.resolve(Instance.directory, hunk.path) + const filePath = (yield* assertExternalDirectoryEffect(ctx, rawFilePath)) ?? rawFilePath switch (hunk.type) { case "add": { @@ -172,8 +172,10 @@ export const ApplyPatchTool = Tool.define( if (change.removed) deletions += change.count || 0 } - const movePath = hunk.move_path ? path.resolve(Instance.directory, hunk.move_path) : undefined - yield* assertExternalDirectoryEffect(ctx, movePath) + const rawMovePath = hunk.move_path ? path.resolve(Instance.directory, hunk.move_path) : undefined + const movePath = rawMovePath + ? ((yield* assertExternalDirectoryEffect(ctx, rawMovePath)) ?? rawMovePath) + : undefined const moveBefore = movePath ? yield* Bom.readFile(afs, movePath).pipe(Effect.catchIf(notFound, () => Effect.succeed(undefined))) : undefined diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 0ac819420..d7541fa05 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -79,10 +79,10 @@ export const EditTool = Tool.define( throw new Error("No changes to apply: oldString and newString are identical.") } - const filePath = path.isAbsolute(params.filePath) + const rawFilePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) - yield* assertExternalDirectoryEffect(ctx, filePath) + const filePath = (yield* assertExternalDirectoryEffect(ctx, rawFilePath)) ?? rawFilePath const relativeFilePath = path.relative(Instance.worktree, filePath) let diff = "" diff --git a/packages/opencode/src/tool/external-directory.ts b/packages/opencode/src/tool/external-directory.ts index 0dd9a1af3..e0cb20706 100644 --- a/packages/opencode/src/tool/external-directory.ts +++ b/packages/opencode/src/tool/external-directory.ts @@ -20,17 +20,16 @@ export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirec ) { if (!target) return - if (options?.bypass) return - const ins = yield* InstanceState.context - const full = process.platform === "win32" ? AppFileSystem.normalizePath(target) : target - if (Instance.containsPath(full, ins)) return + const full = process.platform === "win32" ? AppFileSystem.normalizePath(target, { base: ins.directory }) : target + if (options?.bypass) return full + if (Instance.containsPath(full, ins)) return full const kind = options?.kind ?? "file" const dir = kind === "directory" ? full : path.dirname(full) const glob = process.platform === "win32" - ? AppFileSystem.normalizePathPattern(path.join(dir, "*")) + ? AppFileSystem.normalizePathPattern(path.join(dir, "*"), { base: ins.directory }) : path.join(dir, "*").replaceAll("\\", "/") yield* ctx.ask({ @@ -42,6 +41,7 @@ export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirec parentDir: dir, }, }) + return full }) export async function assertExternalDirectory(ctx: Tool.Context, target?: string, options?: Options) { diff --git a/packages/opencode/src/tool/glob.ts b/packages/opencode/src/tool/glob.ts index 984c13d41..5fbdccec7 100644 --- a/packages/opencode/src/tool/glob.ts +++ b/packages/opencode/src/tool/glob.ts @@ -39,11 +39,12 @@ export const GlobTool = Tool.define( let search = params.path ?? ins.directory search = path.isAbsolute(search) ? search : path.resolve(ins.directory, search) + if (process.platform === "win32") search = AppFileSystem.normalizePath(search, { base: ins.directory }) const info = yield* fs.stat(search).pipe(Effect.catch(() => Effect.succeed(undefined))) if (info?.type === "File") { throw new Error(`glob path must be a directory: ${search}`) } - yield* assertExternalDirectoryEffect(ctx, search, { kind: "directory" }) + search = (yield* assertExternalDirectoryEffect(ctx, search, { kind: "directory" })) ?? search const limit = 100 let truncated = false diff --git a/packages/opencode/src/tool/grep.ts b/packages/opencode/src/tool/grep.ts index 406829777..c7c648272 100644 --- a/packages/opencode/src/tool/grep.ts +++ b/packages/opencode/src/tool/grep.ts @@ -52,17 +52,19 @@ export const GrepTool = Tool.define( }) const ins = yield* InstanceState.context - const search = AppFileSystem.resolve( + let search = AppFileSystem.resolve( path.isAbsolute(params.path ?? ins.directory) ? (params.path ?? ins.directory) : path.join(ins.directory, params.path ?? "."), + { base: ins.directory }, ) const info = yield* fs.stat(search).pipe(Effect.catch(() => Effect.succeed(undefined))) + search = + (yield* assertExternalDirectoryEffect(ctx, search, { + kind: info?.type === "Directory" ? "directory" : "file", + })) ?? search const cwd = info?.type === "Directory" ? search : path.dirname(search) const file = info?.type === "Directory" ? undefined : [path.relative(cwd, search)] - yield* assertExternalDirectoryEffect(ctx, search, { - kind: info?.type === "Directory" ? "directory" : "file", - }) const result = yield* rg.search({ cwd, diff --git a/packages/opencode/src/tool/lsp.ts b/packages/opencode/src/tool/lsp.ts index 84a8f9c5c..6d2c9010e 100644 --- a/packages/opencode/src/tool/lsp.ts +++ b/packages/opencode/src/tool/lsp.ts @@ -44,8 +44,8 @@ export const LspTool = Tool.define( parameters: Parameters, execute: (args: Schema.Schema.Type, ctx: Tool.Context) => Effect.gen(function* () { - const file = path.isAbsolute(args.filePath) ? args.filePath : path.join(Instance.directory, args.filePath) - yield* assertExternalDirectoryEffect(ctx, file) + const rawFile = path.isAbsolute(args.filePath) ? args.filePath : path.join(Instance.directory, args.filePath) + const file = (yield* assertExternalDirectoryEffect(ctx, rawFile)) ?? rawFile const meta = args.operation === "workspaceSymbol" ? { operation: args.operation } diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 6c7176e79..95e55c8cc 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -167,7 +167,7 @@ export const ReadTool = Tool.define( filepath = path.resolve(Instance.directory, filepath) } if (process.platform === "win32") { - filepath = AppFileSystem.normalizePath(filepath) + filepath = AppFileSystem.normalizePath(filepath, { base: Instance.directory }) } const title = path.relative(Instance.worktree, filepath) @@ -178,10 +178,11 @@ export const ReadTool = Tool.define( ), ) - yield* assertExternalDirectoryEffect(ctx, filepath, { - bypass: Boolean(ctx.extra?.["bypassCwdCheck"]), - kind: stat?.type === "Directory" ? "directory" : "file", - }) + filepath = + (yield* assertExternalDirectoryEffect(ctx, filepath, { + bypass: Boolean(ctx.extra?.["bypassCwdCheck"]), + kind: stat?.type === "Directory" ? "directory" : "file", + })) ?? filepath yield* ctx.ask({ permission: "read", diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index a01d34f56..d7be4393d 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -40,10 +40,10 @@ export const WriteTool = Tool.define( parameters: Parameters, execute: (params: { content: string; filePath: string }, ctx: Tool.Context) => Effect.gen(function* () { - const filepath = path.isAbsolute(params.filePath) + const rawFilepath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) - yield* assertExternalDirectoryEffect(ctx, filepath) + const filepath = (yield* assertExternalDirectoryEffect(ctx, rawFilepath)) ?? rawFilepath const exists = yield* fs.existsSafe(filepath) const source = exists ? yield* Bom.readFile(fs, filepath) : { bom: false, text: "" } diff --git a/packages/opencode/test/tool/external-directory.test.ts b/packages/opencode/test/tool/external-directory.test.ts index 931ca41e6..9381d1981 100644 --- a/packages/opencode/test/tool/external-directory.test.ts +++ b/packages/opencode/test/tool/external-directory.test.ts @@ -22,6 +22,14 @@ const baseCtx: Omit = { const glob = (p: string) => process.platform === "win32" ? Filesystem.normalizePathPattern(p) : p.replaceAll("\\", "/") +function withWin32Platform(fn: () => Promise) { + const descriptor = Object.getOwnPropertyDescriptor(process, "platform") + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + return fn().finally(() => { + if (descriptor) Object.defineProperty(process, "platform", descriptor) + }) +} + function makeCtx() { const requests: Array> = [] const ctx: Tool.Context = { @@ -114,6 +122,24 @@ describe("tool.assertExternalDirectory", () => { expect(requests.length).toBe(0) }) + test("returns the canonical Windows target used for permission metadata", async () => { + await withWin32Platform(async () => { + const { requests, ctx } = makeCtx() + + await Instance.provide({ + directory: "D:\\project", + fn: async () => { + const target = await assertExternalDirectory(ctx, "\\\\?\\D:\\outside\\file.txt") + expect(target).toBe("D:\\outside\\file.txt") + }, + }) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.metadata.filepath).toBe("D:\\outside\\file.txt") + }) + }) + if (process.platform === "win32") { test("normalizes Windows path variants to one glob", async () => { const { requests, ctx } = makeCtx()