Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 53 additions & 22 deletions packages/core/src/filesystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,28 +186,27 @@ 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 {
return resolved
}
}

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) {
Expand All @@ -225,28 +224,60 @@ 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(windowsPathInput(path))
Comment thread
Astro-Han marked this conversation as resolved.
Outdated
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)))
Comment thread
Astro-Han marked this conversation as resolved.
}

function windowsPathInput(path: string): string {
return path
.replace(/^\/([a-zA-Z]):(?:[\\/]|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
.replace(/^\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
.replace(/^\/cygdrive\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
.replace(/^\/mnt\/([a-zA-Z])(?:\/|$)/, (_, drive) => `${drive.toUpperCase()}:/`)
}
Comment thread
Astro-Han marked this conversation as resolved.
Outdated

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 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 isRootedDriveless(p: string): boolean {
return /^[\\/](?![\\/])/.test(p)
}

function uppercaseDriveRoot(path: string): string {
return path.replace(/^([a-z]):/, (_, drive) => `${drive.toUpperCase()}:`)
}

function windowsDriveRoots(): string[] {
Expand Down
44 changes: 44 additions & 0 deletions packages/core/test/filesystem/normalize-path.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,50 @@ test("normalizePath is identity on non-Windows", () => {
expect(AppFileSystem.normalizePath(p)).toBe(p)
})

function withWin32Platform(fn: () => void) {
const original = process.platform
Object.defineProperty(process, "platform", { value: "win32" })
try {
fn()
} finally {
Object.defineProperty(process, "platform", { value: original })
}
}

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.skipIf(process.platform !== "win32")(
"normalizePath probes drive roots for rooted-but-driveless paths to existing files",
() => {
Expand Down
10 changes: 6 additions & 4 deletions packages/opencode/src/tool/apply_patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/src/tool/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Expand Down
10 changes: 5 additions & 5 deletions packages/opencode/src/tool/external-directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion packages/opencode/src/tool/glob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/opencode/src/tool/grep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const GrepTool = Tool.define(
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)))
const cwd = info?.type === "Directory" ? search : path.dirname(search)
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/src/tool/lsp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ export const LspTool = Tool.define(
parameters: Parameters,
execute: (args: Schema.Schema.Type<typeof Parameters>, 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 }
Expand Down
11 changes: 6 additions & 5 deletions packages/opencode/src/tool/read.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/src/tool/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: "" }
Expand Down
26 changes: 26 additions & 0 deletions packages/opencode/test/tool/external-directory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ const baseCtx: Omit<Tool.Context, "ask"> = {
const glob = (p: string) =>
process.platform === "win32" ? Filesystem.normalizePathPattern(p) : p.replaceAll("\\", "/")

function withWin32Platform(fn: () => Promise<void>) {
const original = process.platform
Object.defineProperty(process, "platform", { value: "win32" })
return fn().finally(() => {
Object.defineProperty(process, "platform", { value: original })
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function makeCtx() {
const requests: Array<Omit<Permission.Request, "id" | "sessionID" | "tool">> = []
const ctx: Tool.Context = {
Expand Down Expand Up @@ -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()
Expand Down
Loading