Skip to content

fix(win32): normalize paths at permission boundaries #14738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hona merged 4 commits into from
Feb 23, 2026
Merged

fix(win32): normalize paths at permission boundaries #14738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b3247b
fix(win32): normalize paths at permission boundaries
Hona Feb 23, 2026
5a9d823
fix(win32): make wildcard matching case-insensitive for windows paths
Hona Feb 23, 2026
ac7cfbd
fix(win32): scope wildcard case-insensitivity to windows only
Hona Feb 23, 2026
97f83a2
test(win32): add coverage for wildcard path normalization and case-in…
Hona Feb 23, 2026
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
2 changes: 1 addition & 1 deletion packages/opencode/src/tool/external-directory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string

const kind = options?.kind ?? "file"
const parentDir = kind === "directory" ? target : path.dirname(target)
const glob = path.join(parentDir, "*")
const glob = path.join(parentDir, "*").replaceAll("\\", "/")

await ctx.ask({
permission: "external_directory",
Expand Down
5 changes: 4 additions & 1 deletion packages/opencode/src/util/wildcard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { sortBy, pipe } from "remeda"

export namespace Wildcard {
export function match(str: string, pattern: string) {
if (str) str = str.replaceAll("\\", "/")
if (pattern) pattern = pattern.replaceAll("\\", "/")
Comment on lines +5 to +6
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition if (str) will evaluate to false for empty strings, which are valid inputs for the match function. This could cause incorrect behavior when comparing empty strings. Consider using if (str !== undefined && str !== null) or a more explicit check instead.

Copilot uses AI. Check for mistakes.
let escaped = pattern
.replace(/[.+^${}()|[\]\\]/g, "\\$&") // escape special regex chars
.replace(/\*/g, ".*") // * becomes .*
Expand All @@ -13,7 +15,8 @@ export namespace Wildcard {
escaped = escaped.slice(0, -3) + "( .*)?"
}

return new RegExp("^" + escaped + "$", "s").test(str)
const flags = process.platform === "win32" ? "si" : "s"
return new RegExp("^" + escaped + "$", flags).test(str)
}

export function all(input: string, patterns: Record<string, any>) {
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/test/tool/bash.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ describe("tool.bash permissions", () => {

await bash.execute(
{
command: "rm tmpfile",
description: "Remove tmpfile",
command: `rm -rf ${path.join(tmp.path, "nested")}`,
description: "remove nested dir",
Comment on lines +206 to +207
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test creates a file at line 202 (tmpfile) but the command at line 206 attempts to remove a directory (nested). This inconsistency suggests that either the file creation or the rm command is incorrect. If the test is meant to verify that rm doesn't trigger external_directory permission for internal paths, consider either removing the file creation line or changing the rm command to target the created file.

Suggested change
command: `rm -rf ${path.join(tmp.path, "nested")}`,
description: "remove nested dir",
command: `rm -rf ${path.join(tmp.path, "tmpfile")}`,
description: "remove tmp file",

Copilot uses AI. Check for mistakes.
},
testCtx,
)
Expand Down
4 changes: 2 additions & 2 deletions packages/opencode/test/tool/read.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe("tool.read external_directory permission", () => {
await read.execute({ filePath: path.join(outerTmp.path, "secret.txt") }, testCtx)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeDefined()
expect(extDirReq!.patterns.some((p) => p.includes(outerTmp.path))).toBe(true)
expect(extDirReq!.patterns.some((p) => p.includes(outerTmp.path.replaceAll("\\", "/")))).toBe(true)
},
})
})
Expand All @@ -100,7 +100,7 @@ describe("tool.read external_directory permission", () => {
await read.execute({ filePath: path.join(outerTmp.path, "external") }, testCtx)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeDefined()
expect(extDirReq!.patterns).toContain(path.join(outerTmp.path, "external", "*"))
expect(extDirReq!.patterns).toContain(path.join(outerTmp.path, "external", "*").replaceAll("\\", "/"))
},
})
})
Expand Down
15 changes: 15 additions & 0 deletions packages/opencode/test/util/wildcard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,18 @@ test("allStructured handles sed flags", () => {
expect(Wildcard.allStructured({ head: "sed", tail: ["-n", "1p", "file"] }, rules)).toBe("allow")
expect(Wildcard.allStructured({ head: "sed", tail: ["-i", "-n", "/./p", "myfile.txt"] }, rules)).toBe("ask")
})

test("match normalizes slashes for cross-platform globbing", () => {
expect(Wildcard.match("C:\\Windows\\System32\\*", "C:/Windows/System32/*")).toBe(true)
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test appears to be checking path normalization, but the first argument to Wildcard.match contains a literal asterisk at the end. After normalization, both arguments become "C:/Windows/System32/*", and the pattern's asterisk will be converted to a wildcard regex .*. This means the test is actually checking if a literal asterisk in the string matches a wildcard pattern, which may not be the intended behavior for a test named "match normalizes slashes for cross-platform globbing". Consider using a concrete path like "C:\\Windows\\System32\\drivers" for the first argument to better test slash normalization without the ambiguity of the literal asterisk.

Suggested change
expect(Wildcard.match("C:\\Windows\\System32\\*", "C:/Windows/System32/*")).toBe(true)
expect(Wildcard.match("C:\\Windows\\System32\\drivers", "C:/Windows/System32/*")).toBe(true)

Copilot uses AI. Check for mistakes.
expect(Wildcard.match("C:/Windows/System32/drivers", "C:\\Windows\\System32\\*")).toBe(true)
})

test("match handles case-insensitivity on Windows", () => {
if (process.platform === "win32") {
expect(Wildcard.match("C:\\windows\\system32\\hosts", "C:/Windows/System32/*")).toBe(true)
expect(Wildcard.match("c:/windows/system32/hosts", "C:\\Windows\\System32\\*")).toBe(true)
} else {
// Unix paths are case-sensitive
expect(Wildcard.match("/users/test/file", "/Users/test/*")).toBe(false)
}
})
Loading