diff --git a/src/core/auto-approval/WorkspaceRootCanonicalizer.ts b/src/core/auto-approval/WorkspaceRootCanonicalizer.ts new file mode 100644 index 0000000000..c406537be9 --- /dev/null +++ b/src/core/auto-approval/WorkspaceRootCanonicalizer.ts @@ -0,0 +1,43 @@ +import type { WorkspaceRootStrategy } from "./strategies/WorkspaceRootStrategy" + +/** Translate only a verified workspace-root spelling, never a file symlink. */ +export class WorkspaceRootCanonicalizer { + private constructor( + private readonly root: string | undefined, + private readonly canonicalRoot: string | undefined, + private readonly strategy: WorkspaceRootStrategy, + ) {} + + static forStrategy( + cwd: string | undefined, + canonicalCwd: string | undefined, + strategy: WorkspaceRootStrategy, + ): WorkspaceRootCanonicalizer { + const root = strategy.normalizeAbsoluteRoot(cwd) + const canonicalRoot = strategy.normalizeAbsoluteRoot(canonicalCwd) + return new WorkspaceRootCanonicalizer(root, canonicalRoot, strategy) + } + + canonicalize(value: string): string { + if (this.root === undefined || this.canonicalRoot === undefined) return value + + return this.replaceWorkspaceRootPrefix(value, this.root, this.canonicalRoot) + } + + /** Keep both verified spellings so globs spanning the root retain their meaning. */ + equivalentPaths(value: string): string[] { + if (this.root === undefined || this.canonicalRoot === undefined) return [value] + + const canonical = this.canonicalize(value) + const equivalent = + canonical !== value ? canonical : this.replaceWorkspaceRootPrefix(value, this.canonicalRoot, this.root) + return equivalent === value ? [value] : [value, equivalent] + } + + private replaceWorkspaceRootPrefix(value: string, root: string, canonicalRoot: string): string { + const hasRootBoundary = value.length === root.length || value[root.length] === "/" + return hasRootBoundary && this.strategy.pathsEqual(value.slice(0, root.length), root) + ? `${canonicalRoot}${value.slice(root.length)}` + : value + } +} diff --git a/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts b/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts new file mode 100644 index 0000000000..8be4a28813 --- /dev/null +++ b/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts @@ -0,0 +1,199 @@ +import { PosixWorkspaceRootStrategy } from "../strategies/PosixWorkspaceRootStrategy" +import { WindowsWorkspaceRootStrategy } from "../strategies/WindowsWorkspaceRootStrategy" +import { WorkspaceRootStrategy } from "../strategies/WorkspaceRootStrategy" +import { WorkspaceRootCanonicalizer } from "../WorkspaceRootCanonicalizer" + +afterEach(() => vi.restoreAllMocks()) + +it("normalizes roots once in the factory before any path comparisons", () => { + const strategy = new PosixWorkspaceRootStrategy() + const normalizeAbsoluteRoot = vi.spyOn(strategy, "normalizeAbsoluteRoot") + const pathsEqual = vi.spyOn(strategy, "pathsEqual") + + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy("/aliases/project/", "/storage/project/", strategy) + + expect(normalizeAbsoluteRoot).toHaveBeenCalledTimes(2) + expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(1, "/aliases/project/") + expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(2, "/storage/project/") + expect(pathsEqual).not.toHaveBeenCalled() + + expect(canonicalizer.canonicalize("/aliases/project/docs/file.md")).toBe("/storage/project/docs/file.md") + expect(canonicalizer.canonicalize("/aliases/project/notes.md")).toBe("/storage/project/notes.md") + expect(normalizeAbsoluteRoot).toHaveBeenCalledTimes(2) +}) + +describe.each([ + { + platform: "POSIX", + isWindows: false, + cwd: "/aliases/project", + canonicalCwd: "/storage/project", + root: "/aliases/project", + canonicalRoot: "/storage/project", + }, + { + platform: "Windows", + isWindows: true, + cwd: "C:\\Aliases\\Project", + canonicalCwd: "D:\\Storage\\Project", + root: "/C/Aliases/Project", + canonicalRoot: "/D/Storage/Project", + }, +])("workspace-root canonicalization on $platform", ({ isWindows, cwd, canonicalCwd, root, canonicalRoot }) => { + const strategy = WorkspaceRootStrategy.forPlatform(isWindows) + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy(cwd, canonicalCwd, strategy) + + it("translates an exact root and preserves descendant spelling and globs", () => { + expect(canonicalizer.canonicalize(root)).toBe(canonicalRoot) + expect(canonicalizer.canonicalize(`${root}/Docs/NOTES.md`)).toBe(`${canonicalRoot}/Docs/NOTES.md`) + expect(canonicalizer.canonicalize(`${root}/docs/**`)).toBe(`${canonicalRoot}/docs/**`) + }) + + it("does not translate similarly prefixed siblings or lexical parents", () => { + for (const value of [`${root}-other/docs/file.md`, `${root}2/file.md`, "/elsewhere/docs/file.md"]) { + expect(canonicalizer.canonicalize(value)).toBe(value) + } + }) + + it("leaves canonical-root paths and relative values unchanged", () => { + for (const value of [canonicalRoot, `${canonicalRoot}/docs/file.md`, "docs/file.md", "../shared/file.md"]) { + expect(canonicalizer.canonicalize(value)).toBe(value) + } + }) + + it.each([undefined, "", "relative/project", "C:", "C:relative"])( + "does not infer equivalence from an unknown or relative root: %s", + (invalidRoot) => { + const value = `${root}/docs/file.md` + expect( + WorkspaceRootCanonicalizer.forStrategy(invalidRoot, canonicalCwd, strategy).canonicalize(value), + ).toBe(value) + expect(WorkspaceRootCanonicalizer.forStrategy(cwd, invalidRoot, strategy).canonicalize(value)).toBe(value) + }, + ) +}) + +describe("POSIX workspace-root strategy", () => { + it("compares case-sensitively and keeps backslashes as filename characters", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "/aliases/My\\Project", + "/storage/My\\Project", + new PosixWorkspaceRootStrategy(), + ) + expect(canonicalizer.canonicalize("/aliases/My\\Project/docs/file.md")).toBe( + "/storage/My\\Project/docs/file.md", + ) + expect(canonicalizer.canonicalize("/aliases/my\\project/docs/file.md")).toBe( + "/aliases/my\\project/docs/file.md", + ) + expect(canonicalizer.canonicalize("/aliases/My/Project/docs/file.md")).toBe("/aliases/My/Project/docs/file.md") + }) + + it("normalizes root dot segments and trailing separators", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "/aliases/temporary/../project/", + "/storage/./project/", + new PosixWorkspaceRootStrategy(), + ) + expect(canonicalizer.canonicalize("/aliases/project/docs/file.md")).toBe("/storage/project/docs/file.md") + }) + + it("does not interpret Windows drive paths as absolute roots", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "C:/aliases/project", + "D:/storage/project", + new PosixWorkspaceRootStrategy(), + ) + expect(canonicalizer.canonicalize("/C/aliases/project/docs/file.md")).toBe("/C/aliases/project/docs/file.md") + }) +}) + +describe("Windows workspace-root strategy", () => { + it.each([ + ["\u0130", "i\u0307"], + ["i\u0307", "\u0130"], + ["\u212a", "K"], + ["K", "\u212a"], + ] as const)("does not fabricate an alias for distinct roots %s and %s", (root, sibling) => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + `C:/aliases/${root}`, + "D:/storage/project", + new WindowsWorkspaceRootStrategy(), + ) + const unrelated = `/C/aliases/${sibling}/Docs/NOTES.md` + + expect(canonicalizer.canonicalize(unrelated)).toBe(unrelated) + expect(canonicalizer.equivalentPaths(unrelated)).toEqual([unrelated]) + expect(canonicalizer.canonicalize(`/C/aliases/${root}/Docs/NOTES.md`)).toBe("/D/storage/project/Docs/NOTES.md") + }) + + it("compares the original root boundary and preserves the entire descendant suffix", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "C:/aliases/\u03a3", + "D:/storage/project", + new WindowsWorkspaceRootStrategy(), + ) + + expect(canonicalizer.canonicalize("/C/aliases/\u03c2/Docs/NOTES.md")).toBe("/D/storage/project/Docs/NOTES.md") + for (const suffix of ["-other", "\n", "2"]) { + const unrelated = `/C/aliases/\u03c2${suffix}/Docs/NOTES.md` + expect(canonicalizer.canonicalize(unrelated)).toBe(unrelated) + expect(canonicalizer.equivalentPaths(unrelated)).toEqual([unrelated]) + } + }) + + it("compares case-insensitively across drives without changing descendant case", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "C:\\Aliases\\Project", + "D:\\Storage\\Project", + new WindowsWorkspaceRootStrategy(), + ) + expect(canonicalizer.canonicalize("/c/ALIASES/project/Docs/NOTES.md")).toBe("/D/Storage/Project/Docs/NOTES.md") + expect(canonicalizer.canonicalize("/E/Aliases/Project/Docs/NOTES.md")).toBe("/E/Aliases/Project/Docs/NOTES.md") + }) + + it("normalizes mixed root separators, dot segments, and trailing separators", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "C:\\Aliases/temporary/../Project\\", + "D:/Storage/./Project/", + new WindowsWorkspaceRootStrategy(), + ) + expect(canonicalizer.canonicalize("/C/Aliases/Project/docs/file.md")).toBe("/D/Storage/Project/docs/file.md") + }) + + it("supports drive roots and drive-less absolute roots in matcher notation", () => { + const strategy = new WindowsWorkspaceRootStrategy() + expect(WorkspaceRootCanonicalizer.forStrategy("C:/", "D:/", strategy).canonicalize("/C/docs/file.md")).toBe( + "/D/docs/file.md", + ) + expect( + WorkspaceRootCanonicalizer.forStrategy("\\Aliases\\Project", "/Storage/Project", strategy).canonicalize( + "/ROOT/Aliases/Project/docs/file.md", + ), + ).toBe("/ROOT/Storage/Project/docs/file.md") + }) + + it("normalizes network roots into the shared UNC matcher namespace", () => { + const canonicalizer = WorkspaceRootCanonicalizer.forStrategy( + "\\\\server\\share\\project", + "\\\\server\\storage\\project", + new WindowsWorkspaceRootStrategy(), + ) + expect(canonicalizer.canonicalize("/UNC/SERVER/SHARE/project/docs/file.md")).toBe( + "/UNC/server/storage/project/docs/file.md", + ) + }) +}) + +it.each([false, true])("preserves filesystem-root equivalence (Windows=%s)", (isWindows) => { + const prefix = isWindows ? "/ROOT" : "" + const strategy = WorkspaceRootStrategy.forPlatform(isWindows) + expect( + WorkspaceRootCanonicalizer.forStrategy("/", "/storage", strategy).canonicalize(`${prefix}/docs/file.md`), + ).toBe(`${prefix}/storage/docs/file.md`) + expect( + WorkspaceRootCanonicalizer.forStrategy("/aliases", "/", strategy).canonicalize( + `${prefix}/aliases/docs/file.md`, + ), + ).toBe(`${prefix}/docs/file.md`) +}) diff --git a/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts new file mode 100644 index 0000000000..d4f0d74f6c --- /dev/null +++ b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts @@ -0,0 +1,74 @@ +import { isFileMatchedByPatterns } from "../filePatterns" +import { PosixWorkspaceRootStrategy } from "../strategies/PosixWorkspaceRootStrategy" +import { WorkspaceRootStrategy } from "../strategies/WorkspaceRootStrategy" + +describe("WorkspaceRootStrategy.forPlatform", () => { + afterEach(() => vi.restoreAllMocks()) + + it("selects POSIX behavior without interpreting drives, network prefixes, or backslashes", () => { + const strategy = WorkspaceRootStrategy.forPlatform(false) + + expect(strategy.toMatcherAbsolutePath("/tmp/notes.md", "/C")).toBe("/tmp/notes.md") + expect(strategy.toMatcherAbsolutePath("//server/share/notes.md", "/C")).toBe("//server/share/notes.md") + expect(strategy.toMatcherAbsolutePath("/tmp/my\\file.md", "/C")).toBe("/tmp/my\\file.md") + expect(strategy.pathsEqual("/tmp/NOTES.md", "/tmp/NOTES.md")).toBe(true) + expect(strategy.pathsEqual("/tmp/NOTES.md", "/tmp/notes.md")).toBe(false) + }) + + it("selects Windows behavior for drives, rooted paths, and the distinct UNC namespace", () => { + const strategy = WorkspaceRootStrategy.forPlatform(true) + + expect(strategy.toMatcherAbsolutePath("D:/tmp/notes.md", "/C")).toBe("/D/tmp/notes.md") + expect(strategy.toMatcherAbsolutePath("/tmp/notes.md", "/C")).toBe("/C/tmp/notes.md") + expect(strategy.toMatcherAbsolutePath("//server/share/notes.md", "/C")).toBe("/UNC/server/share/notes.md") + expect(strategy.toMatcherAbsolutePath("//C/share/notes.md", "/C")).not.toBe("/C/share/notes.md") + expect(strategy.pathsEqual("/D/Tmp/NOTES.md", "/d/tmp/notes.md")).toBe(true) + }) + + it.each([ + ["\u0130", "i\u0307", false], + ["\u212a", "K", false], + ["\u00df", "SS", false], + ["\u017f", "S", false], + ["\u03a3", "\u03c2", true], + ["\u03a3", "\u03c3", true], + ["Project[1]", "Project1", false], + ["Project(1)", "Project1", false], + ["Project+", "Projecttt", false], + ["Project", "Project\n", false], + ] as const)("compares Windows root names %s and %s without rewriting their identity", (left, right, equal) => { + const strategy = WorkspaceRootStrategy.forPlatform(true) + + expect(strategy.pathsEqual(`/C/aliases/${left}`, `/c/ALIASES/${right}`)).toBe(equal) + expect(strategy.pathsEqual(`/c/ALIASES/${right}`, `/C/aliases/${left}`)).toBe(equal) + }) + + it("uses one selected strategy for candidates, every pattern, and both workspace roots", () => { + const strategy = new PosixWorkspaceRootStrategy() + const toMatcherAbsolutePath = vi.spyOn(strategy, "toMatcherAbsolutePath") + const normalizeAbsoluteRoot = vi.spyOn(strategy, "normalizeAbsoluteRoot") + const pathsEqual = vi.spyOn(strategy, "pathsEqual") + const selectStrategy = vi.spyOn(WorkspaceRootStrategy, "forPlatform").mockReturnValue(strategy) + + expect( + isFileMatchedByPatterns({ + filePath: "/storage/project/docs/notes.md", + cwd: "/aliases/project", + canonicalCwd: "/storage/project", + patterns: ["docs/**", "!/aliases/project/docs/private.md", "/aliases/project/docs/notes.md"], + isWindows: false, + }), + ).toBe(true) + + expect(selectStrategy).toHaveBeenCalledOnce() + expect(selectStrategy).toHaveBeenCalledWith(false) + expect(toMatcherAbsolutePath).toHaveBeenCalledWith("/storage/project/docs/notes.md", "") + expect(toMatcherAbsolutePath).toHaveBeenCalledWith("/aliases/project", "") + expect(toMatcherAbsolutePath).toHaveBeenCalledWith("/aliases/project/docs/private.md", "") + expect(toMatcherAbsolutePath).toHaveBeenCalledWith("/aliases/project/docs/notes.md", "") + expect(normalizeAbsoluteRoot).toHaveBeenCalledTimes(2) + expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(1, "/aliases/project") + expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(2, "/storage/project") + expect(pathsEqual).toHaveBeenCalled() + }) +}) diff --git a/src/core/auto-approval/__tests__/filePatterns.spec.ts b/src/core/auto-approval/__tests__/filePatterns.spec.ts index 6b1c3ff6b9..23ceaa1089 100644 --- a/src/core/auto-approval/__tests__/filePatterns.spec.ts +++ b/src/core/auto-approval/__tests__/filePatterns.spec.ts @@ -86,7 +86,7 @@ describe("toMatcherPattern", () => { // name, so `C:/tmp/notes.md` names a file inside it, relative to the workspace. describe("on Windows", () => { // The colon is dropped, leaving the drive as an ordinary leading path - // segment; see `toPosixAbsolutePath` for why it cannot stay. + // segment: the Windows strategy avoids drive-rooted paths rejected by `ignore`. it("keeps the drive of an absolute pattern as its first path segment", () => { expect(toWindowsPattern("D:/tmp/notes.md")).toBe("/D/tmp/notes.md") }) diff --git a/src/core/auto-approval/__tests__/posixPaths.spec.ts b/src/core/auto-approval/__tests__/posixPaths.spec.ts index 5e9da6312f..bb697dc0cd 100644 --- a/src/core/auto-approval/__tests__/posixPaths.spec.ts +++ b/src/core/auto-approval/__tests__/posixPaths.spec.ts @@ -20,6 +20,25 @@ describe("matching off Windows", () => { const matches = (filePath: string, patterns: string[]) => matcher().isFileMatchedByPatterns({ filePath, cwd: CWD, patterns }) + it("keeps backslashes in a workspace root literal", () => { + const rootMatch = (filePath: string) => + matcher().isFileMatchedByPatterns({ + filePath, + cwd: "/aliases/my\\project", + canonicalCwd: "/storage/my\\project", + patterns: ["docs/**"], + }) + expect(rootMatch("/aliases/my\\project/docs/notes.md")).toBe(true) + expect(rootMatch("/storage/my\\project/docs/notes.md")).toBe(true) + expect(rootMatch("/aliases/my/project/docs/notes.md")).toBe(false) + }) + + it("matches a relative grant at the filesystem root", () => { + expect(matcher().isFileMatchedByPatterns({ filePath: "/docs/notes.md", cwd: "/", patterns: ["docs/**"] })).toBe( + true, + ) + }) + // A backslash is a legal filename character here, where `my\file` is one file // rather than `file` inside `my`. it("reads a backslash as part of the filename, not as a separator", () => { @@ -39,6 +58,18 @@ describe("matching off Windows", () => { expect(matches("/tmp/notes.md", ["C:/tmp/notes.md"])).toBe(false) }) + it("keeps drive-relative-looking names usable as literal POSIX paths", () => { + expect(matches("C:notes.md", ["C:notes.md"])).toBe(true) + expect(matches("D:docs/notes.md", ["D:docs/**"])).toBe(true) + expect( + matcher().isFileMatchedByPatterns({ + filePath: "docs/notes.md", + cwd: "/workspace/C:", + patterns: ["docs/**"], + }), + ).toBe(true) + }) + it("matches a bare pattern anywhere in the workspace, and nowhere else", () => { expect(matches("notes.md", ["notes.md"])).toBe(true) expect(matches("deeply/nested/notes.md", ["notes.md"])).toBe(true) diff --git a/src/core/auto-approval/__tests__/windowsDriveRelativeApproval.spec.ts b/src/core/auto-approval/__tests__/windowsDriveRelativeApproval.spec.ts new file mode 100644 index 0000000000..990eeb7f59 --- /dev/null +++ b/src/core/auto-approval/__tests__/windowsDriveRelativeApproval.spec.ts @@ -0,0 +1,56 @@ +import * as fs from "fs/promises" +import { baseState } from "./fixtures" +import { matcherForPlatform } from "./matcherForPlatform" + +vi.mock("fs/promises", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, realpath: vi.fn() } +}) + +describe("approval with a drive-relative Windows workspace", () => { + matcherForPlatform("win32") + + beforeEach(() => { + vi.mocked(fs.realpath).mockReset().mockResolvedValue("C:/repo") + }) + + it.each(["readFile", "newFileCreated"] as const)( + "does not approve %s outside the actual workspace through relative grants", + async (tool) => { + const { checkAutoApproval } = await import("../index") + const state = { ...baseState, allowedReadFiles: ["docs/**"], allowedWriteFiles: ["docs/**"] } + for (const path of ["C:/docs/private.md", "../docs/private.md"]) { + expect( + await checkAutoApproval({ + state, + cwd: "C:", + ask: "tool", + text: JSON.stringify({ tool, path, isOutsideWorkspace: true }), + }), + ).toEqual({ decision: "ask" }) + } + + const text = JSON.stringify({ tool, path: "C:/docs/private.md", isOutsideWorkspace: true }) + expect( + await checkAutoApproval({ + state: { ...state, allowedReadFiles: ["C:/docs/**"], allowedWriteFiles: ["C:/docs/**"] }, + cwd: "C:", + ask: "tool", + text, + }), + ).toEqual({ decision: "approve" }) + expect( + await checkAutoApproval({ + state: { + ...state, + allowedReadFiles: ["C:/docs/**", "!C:/docs/private.md"], + allowedWriteFiles: ["C:/docs/**", "!C:/docs/private.md"], + }, + cwd: "C:", + ask: "tool", + text, + }), + ).toEqual({ decision: "ask" }) + }, + ) +}) diff --git a/src/core/auto-approval/__tests__/windowsPaths.spec.ts b/src/core/auto-approval/__tests__/windowsPaths.spec.ts index acedfa726c..c8cf0644ef 100644 --- a/src/core/auto-approval/__tests__/windowsPaths.spec.ts +++ b/src/core/auto-approval/__tests__/windowsPaths.spec.ts @@ -22,6 +22,15 @@ import { matcherForPlatform } from "./matcherForPlatform" const WINDOWS_CWD = "C:\\path\\to\\repo" const WINDOWS_HOME = "C:\\Users\\me" +const ROOTED_EXCLUSIONS = [ + "!/**/.env", + "!///**/.env", + "!////**/.env", + "!\\**\\.env", + "!\\\\\\**\\.env", + "!\\\\\\\\**\\.env", +] + describe("matching on Windows", () => { const matcher = matcherForPlatform("win32") @@ -30,6 +39,398 @@ describe("matching on Windows", () => { const matches = (filePath: string, patterns: string[], cwd: string | undefined = WINDOWS_CWD) => matcher().isFileMatchedByPatterns({ filePath, cwd, patterns, homeDir: WINDOWS_HOME }) + it.each([ + "///server/share/docs/**", + "////server/share/docs/**", + "\\\\\\server\\share\\docs\\**", + "\\\\\\\\server\\share\\docs\\**", + ])("treats extra leading separators in a grant as drive-rooted, not UNC: %s", (pattern) => { + expect(matches("//server/share/docs/private.md", [pattern], "C:/repo")).toBe(false) + expect(matches("C:/server/share/docs/private.md", [pattern], "C:/repo")).toBe(true) + expect(matches("//server/share/docs/private.md", ["//server/share/docs/**"], "C:/repo")).toBe(true) + }) + + it.each([ + "///server/share/docs/private.md", + "////server/share/docs/private.md", + "\\\\\\server\\share\\docs\\private.md", + "\\\\\\\\server\\share\\docs\\private.md", + ])("treats extra leading separators in a candidate as drive-rooted, not UNC: %s", (filePath) => { + expect(matches(filePath, ["//server/share/docs/**"], "C:/repo")).toBe(false) + expect(matches(filePath, ["C:/server/share/docs/**"], "C:/repo")).toBe(true) + expect(matches(filePath, ["///server/share/docs/**"], "C:/repo")).toBe(true) + }) + + it.each(["//server//share", "//server///share", "\\\\server\\\\share", "\\\\server\\\\\\share"])( + "normalizes repeated separators between a UNC server and share: %s", + (root) => { + expect(matches("//server/share/docs/private.md", [`${root}/docs/**`], "C:/repo")).toBe(true) + expect(matches(`${root}/docs/private.md`, ["//server/share/docs/**"], "C:/repo")).toBe(true) + expect(matches("//server/share/docs/private.md", ["docs/**"], root)).toBe(true) + expect(matches("docs/private.md", ["//server/share/docs/**"], root)).toBe(true) + expect(matches("docs/private.md", ["C:/server/share/docs/**"], root)).toBe(false) + expect(matches("C:/server/share/docs/private.md", [`${root}/docs/**`], "C:/repo")).toBe(false) + }, + ) + + it.each(["//server", "\\\\server"])("treats an incomplete UNC name as drive-rooted: %s", (filePath) => { + expect(matches(filePath, ["C:/server"], "C:/repo")).toBe(true) + expect(matches(filePath, ["D:/server"], "C:/repo")).toBe(false) + expect(matches("C:/server", [filePath], "C:/repo")).toBe(true) + expect(matches("//server/share/docs/private.md", [filePath], "C:/repo")).toBe(false) + }) + + it.each([ + { + cwd: "//server/share/../project", + canonicalCwd: "//server/share/project", + otherVolume: "//server/project", + }, + { + cwd: "\\\\server\\share\\..\\project", + canonicalCwd: "//server/share/project", + otherVolume: "//server/project", + }, + { cwd: "C:/../D/project", canonicalCwd: "C:/D/project", otherVolume: "D:/project" }, + { cwd: "C:\\..\\D\\project", canonicalCwd: "C:/D/project", otherVolume: "D:/project" }, + ])( + "clamps parent traversal at the volume when normalizing equivalent workspace roots: $cwd", + ({ cwd, canonicalCwd, otherVolume }) => { + const input = { + cwd, + canonicalCwd, + filePath: `${canonicalCwd}/docs/private.md`, + } + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: [`${otherVolume}/docs/**`] })).toBe(false) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["docs/**"] })).toBe(true) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: [`${canonicalCwd}/docs/**`] })).toBe(true) + expect( + matcher().isFileMatchedByPatterns({ + ...input, + patterns: ["docs/**", `!${canonicalCwd}/docs/private.md`], + }), + ).toBe(false) + expect( + matcher().isFileMatchedByPatterns({ + ...input, + patterns: [`!${canonicalCwd}/docs/private.md`, "docs/**"], + }), + ).toBe(true) + }, + ) + + it.each( + [undefined, "/ws", "\\ws", "//server/share/ws", "\\\\server\\share\\ws"].flatMap((cwd) => + ROOTED_EXCLUSIONS.map((exclusion) => ({ cwd, exclusion })), + ), + )("preserves exclusion $exclusion and its order with workspace $cwd", ({ cwd, exclusion }) => { + const input = { filePath: "C:/x/.env", cwd, homeDir: WINDOWS_HOME } + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["C:/**", exclusion] })).toBe(false) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: [exclusion, "C:/**"] })).toBe(true) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["C:/**", exclusion, "C:/x/.env"] })).toBe(true) + expect( + matcher().isFileMatchedByPatterns({ + ...input, + filePath: "C:/x/notes.md", + patterns: ["C:/**", exclusion], + }), + ).toBe(true) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["/**"] })).toBe(false) + }) + + it.each(ROOTED_EXCLUSIONS)("keeps rooted exclusion %s scoped to a known workspace drive", (exclusion) => { + expect(matches("C:/x/.env", ["C:/**", exclusion], "D:/ws")).toBe(true) + expect(matches("D:/x/.env", ["D:/**", exclusion], "D:/ws")).toBe(false) + expect(matches("D:/x/notes.md", ["D:/**", exclusion], "D:/ws")).toBe(true) + }) + + it.each(["!//C/x/.env", "!\\\\C\\x\\.env"])("keeps explicit UNC exclusion %s separate from drives", (exclusion) => { + const input = { cwd: undefined, patterns: ["C:/**", "//C/x/**", "//other/x/**", exclusion] } + expect(matcher().isFileMatchedByPatterns({ ...input, filePath: "C:/x/.env" })).toBe(true) + expect(matcher().isFileMatchedByPatterns({ ...input, filePath: "//C/x/.env" })).toBe(false) + expect(matcher().isFileMatchedByPatterns({ ...input, filePath: "//other/x/.env" })).toBe(true) + }) + + it.each([ + { + scenario: "an incomplete UNC exclusion retains rooted semantics", + cwd: undefined, + filePath: "C:/x/.env", + patterns: ["C:/**", "!//**"], + allowed: false, + }, + { + scenario: "a drive-relative workspace preserves exclusions on explicit grants", + cwd: "C:", + filePath: "C:/x/.env", + patterns: ["C:/**", "!/**/.env"], + allowed: false, + }, + { + scenario: "a rooted exclusion cannot address the internal UNC prefix", + cwd: undefined, + filePath: "//server/share/x/.env", + patterns: ["//server/share/**", "!/UNC/**/.env"], + allowed: true, + }, + { + scenario: "a rooted exclusion cannot address the internal ROOT prefix", + cwd: undefined, + filePath: "/x/.env", + patterns: ["/**", "!/ROOT/**/.env"], + allowed: true, + }, + { + scenario: "an explicit UNC exclusion stays separate from drives", + cwd: undefined, + filePath: "C:/x/.env", + patterns: ["C:/**", "!//C/x/.env"], + allowed: true, + }, + { + scenario: "an explicit drive exclusion still blocks that drive", + cwd: "//server/share/ws", + filePath: "C:/x/.env", + patterns: ["C:/**", "!C:/x/.env"], + allowed: false, + }, + { + scenario: "an explicit drive exclusion does not block a different drive", + cwd: "//server/share/ws", + filePath: "D:/x/.env", + patterns: ["D:/**", "!C:/x/.env"], + allowed: true, + }, + { + scenario: "backslash exclusions preserve cross-volume restrictions", + cwd: "\\\\server\\share\\ws", + filePath: "C:\\x\\.env", + patterns: ["C:\\**", "!\\**\\.env"], + allowed: false, + }, + ])("preserves exclusion scope: $scenario", ({ cwd, filePath, patterns, allowed }) => { + expect(matcher().isFileMatchedByPatterns({ cwd, filePath, patterns })).toBe(allowed) + }) + + it.each(ROOTED_EXCLUSIONS)( + "checks exclusion %s against an equivalent drive path for a UNC candidate", + (exclusion) => { + expect( + matcher().isFileMatchedByPatterns({ + cwd: "//server/share/Project", + canonicalCwd: "C:/Storage/Project", + filePath: "//server/share/Project/docs/.env", + patterns: ["docs/**", exclusion.replace("**", "C/Storage/**")], + }), + ).toBe(false) + }, + ) + + it.each(["//server/sh[a]re/project", "\\\\server\\sh[a]re\\project"])( + "keeps an inherited UNC volume literal in rooted grants and exclusions: %s", + (cwd) => { + expect(matches("//server/sh[a]re/docs/notes.md", ["/docs/**"], cwd)).toBe(true) + expect(matches("\\\\server\\sh[a]re\\docs\\notes.md", ["/docs/**"], cwd)).toBe(true) + expect(matches("//server/share/docs/notes.md", ["/docs/**"], cwd)).toBe(false) + expect(matches("//server/shaare/docs/notes.md", ["/docs/**"], cwd)).toBe(false) + expect(matches("docs/private.md", ["docs/**", "!/project/docs/private.md"], cwd)).toBe(false) + expect(matches("docs/notes.md", ["docs/**", "!/project/docs/private.md"], cwd)).toBe(true) + }, + ) + + it.each(["//server/sh[a]re/docs/**", "\\\\server\\sh[a]re\\docs\\**"])( + "preserves user-authored globs in an explicit UNC pattern: %s", + (pattern) => { + expect(matches("//server/share/docs/notes.md", [pattern])).toBe(true) + expect(matches("//server/sh[a]re/docs/notes.md", [pattern])).toBe(false) + expect(matches("//server/other/docs/notes.md", [pattern])).toBe(false) + }, + ) + + it("keeps share-rooted paths separate from the internal UNC namespace", () => { + const cwd = "\\\\server\\share\\project" + const rooted = "/UNC/server/share/project/docs/**" + expect(matches("docs/private.md", [rooted], cwd)).toBe(false) + expect(matches("//server/share/project/docs/private.md", [rooted], cwd)).toBe(false) + expect(matches("//server/share/UNC/server/share/project/docs/private.md", [rooted], cwd)).toBe(true) + expect(matches("/UNC/server/share/project/docs/private.md", ["docs/**"], cwd)).toBe(false) + expect(matches("docs/private.md", ["docs/**", `!${rooted}`], cwd)).toBe(true) + }) + + it("resolves share-rooted paths against the workspace share", () => { + const cwd = "\\\\server\\share\\project" + expect(matches("//server/share/docs/private.md", ["/docs/**"], cwd)).toBe(true) + expect(matches("/docs/private.md", ["//server/share/docs/**"], cwd)).toBe(true) + expect(matches("//other/share/docs/private.md", ["/docs/**"], cwd)).toBe(false) + }) + + it.each(["C:/", "//server/share", "//server/share/"])("matches relative grants at a volume root: %s", (cwd) => { + const absolutePath = `${cwd.replace(/\/$/, "")}/docs/private.md` + expect(matches("docs/private.md", ["docs/**"], cwd)).toBe(true) + expect(matches(absolutePath, ["docs/**"], cwd)).toBe(true) + expect(matches("docs/private.md", ["docs/**", "!docs/private.md"], cwd)).toBe(false) + expect(matches(absolutePath, ["docs/**", "!docs/private.md"], cwd)).toBe(false) + }) + + it.each(["C:", "c:", "C:repo"])("does not anchor relative grants to a drive-relative workspace: %s", (cwd) => { + expect(matcher().toMatcherPattern("docs/**", cwd)).toBeUndefined() + for (const canonicalCwd of [undefined, "C:/repo"]) { + for (const filePath of [ + "docs/private.md", + "../docs/private.md", + "C:/docs/private.md", + "C:/repo/docs/private.md", + ]) { + expect(matcher().isFileMatchedByPatterns({ filePath, cwd, canonicalCwd, patterns: ["docs/**"] })).toBe( + false, + ) + } + expect( + matcher().isFileMatchedByPatterns({ + filePath: "C:/docs/notes.md", + cwd, + canonicalCwd, + patterns: ["C:/docs/**"], + }), + ).toBe(true) + expect( + matcher().isFileMatchedByPatterns({ + filePath: "C:/docs/private.md", + cwd, + canonicalCwd, + patterns: ["C:/docs/**", "!C:/docs/private.md"], + }), + ).toBe(false) + expect( + matcher().isFileMatchedByPatterns({ + filePath: "../docs/notes.md", + cwd, + canonicalCwd, + patterns: ["C:/docs/**"], + }), + ).toBe(false) + } + }) + + it.each(["C:", "C:docs/private.md", "D:docs/private.md"])( + "does not guess the location of a drive-relative candidate or pattern: %s", + (filePath) => { + expect(matches(filePath, ["**"])).toBe(false) + expect(matcher().toMatcherPattern(filePath, WINDOWS_CWD)).toBeUndefined() + }, + ) + + it("clamps lexical parents at drive and share roots without crossing matcher namespaces", () => { + const cwd = "\\\\server\\share\\project" + expect(matches("//server/share/docs/private.md", ["../../../docs/**"], cwd)).toBe(true) + expect(matches("C:/docs/private.md", ["../../../../C/docs/**"], cwd)).toBe(false) + expect(matches("../../../../C/docs/private.md", ["C:/docs/**"], cwd)).toBe(false) + expect(matches("C:/docs/private.md", ["../../../docs/**"], "C:/project")).toBe(true) + expect(matches("//server/share/docs/private.md", ["../../UNC/server/share/docs/**"], "C:/project")).toBe(false) + }) + + it.each(["//server/sh[a]re/shared/x.md", "\\\\server\\sh[a]re\\shared\\x.md"])( + "keeps UNC share names literal when resolving a lexical-parent grant: %s", + (filePath) => { + const cwd = "\\\\server\\sh[a]re\\project" + const patterns = ["../shared/x.md"] + expect(matches(filePath, patterns, cwd)).toBe(true) + expect(matches("../shared/x.md", patterns, cwd)).toBe(true) + for (const share of ["share", "shaare"]) { + expect(matches(`//server/${share}/shared/x.md`, patterns, cwd)).toBe(false) + } + }, + ) + + it("keeps unknown-volume rooted paths separate from explicit UNC paths", () => { + expect( + matcher().isFileMatchedByPatterns({ + filePath: "//server/share/docs/private.md", + patterns: ["/UNC/server/share/docs/**"], + }), + ).toBe(false) + expect( + matcher().isFileMatchedByPatterns({ + filePath: "/UNC/server/share/docs/private.md", + patterns: ["//server/share/docs/**"], + }), + ).toBe(false) + }) + + it("preserves wildcard exclusions spanning drive and UNC workspace roots", () => { + const rootMatch = (filePath: string, patterns: string[]) => + matcher().isFileMatchedByPatterns({ + filePath, + cwd: "C:/Aliases/Project", + canonicalCwd: "//server/share/Project", + patterns, + }) + for (const filePath of ["docs/private.md", "//server/share/Project/docs/private.md"]) { + for (const exclusion of ["!C:/Aliases/**/private.md", "!//server/share/Proj*/docs/private.md"]) { + expect(rootMatch(filePath, ["docs/**", exclusion])).toBe(false) + expect(rootMatch(filePath, [exclusion, "docs/**"])).toBe(true) + expect(rootMatch(filePath, ["docs/**", exclusion, "C:/Aliases/Project/docs/private.md"])).toBe(true) + } + } + }) + + it.each(["\\\\server\\share\\project\\docs\\notes.md", "//server/share/project/docs/notes.md"])( + "matches an absolute UNC candidate against a relative grant: %s", + (filePath) => { + expect(matches(filePath, ["docs/**"], "\\\\server\\share\\project")).toBe(true) + }, + ) + + it("matches explicit UNC patterns from a local-drive workspace and without a workspace", () => { + const filePath = "\\\\server\\share\\project\\docs\\notes.md" + const patterns = ["//server/share/project/docs/**"] + expect(matches(filePath, patterns)).toBe(true) + expect(matcher().isFileMatchedByPatterns({ filePath, patterns, homeDir: WINDOWS_HOME })).toBe(true) + }) + + it.each([ + { cwd: "\\\\alias\\share\\project", canonicalCwd: "\\\\server\\storage\\project" }, + { cwd: "C:\\Aliases\\Project", canonicalCwd: "\\\\server\\storage\\project" }, + { cwd: "\\\\alias\\share\\project", canonicalCwd: "D:\\Storage\\Project" }, + ])("matches equivalent local and UNC roots ($cwd -> $canonicalCwd)", ({ cwd, canonicalCwd }) => { + const filePath = `${canonicalCwd}\\DOCS\\NOTES.md` + const rootMatch = (patterns: string[]) => + matcher().isFileMatchedByPatterns({ filePath, cwd, canonicalCwd, patterns, homeDir: WINDOWS_HOME }) + expect(rootMatch(["docs/**"])).toBe(true) + expect(rootMatch([`${cwd}/docs/**`])).toBe(true) + expect(rootMatch([`${canonicalCwd}/docs/**`])).toBe(true) + expect(rootMatch(["docs/**", `!${cwd}/docs/notes.md`])).toBe(false) + expect(rootMatch(["docs/**", `!${canonicalCwd}/docs/notes.md`])).toBe(false) + expect(rootMatch(["docs/**", `!${cwd}/docs/notes.md`, `${canonicalCwd}/docs/notes.md`])).toBe(true) + }) + + it("honors UNC exclusions for both relative and absolute candidates", () => { + const cwd = "\\\\server\\share\\project" + const patterns = ["docs/**", "!//server/share/project/docs/private.md"] + expect(matches("docs/private.md", patterns, cwd)).toBe(false) + expect(matches(`${cwd}\\docs\\private.md`, patterns, cwd)).toBe(false) + expect(matches(`${cwd}\\docs\\notes.md`, patterns, cwd)).toBe(true) + expect(matches("docs/private.md", [...patterns, "//server/share/project/docs/private.md"], cwd)).toBe(true) + }) + + it("keeps different UNC servers, shares, and similarly prefixed siblings separate", () => { + const cwd = "\\\\server\\share\\project" + for (const filePath of [ + "\\\\other-server\\share\\project\\docs\\notes.md", + "\\\\server\\other-share\\project\\docs\\notes.md", + "\\\\server\\share\\project-other\\docs\\notes.md", + ]) { + expect(matches(filePath, ["docs/**"], cwd)).toBe(false) + } + }) + + it("does not confuse a UNC server named C with the local C drive", () => { + expect(matches("C:\\share\\project\\docs\\notes.md", ["docs/**"], "\\\\C\\share\\project")).toBe(false) + expect(matches("\\\\C\\share\\project\\docs\\notes.md", ["docs/**"], "C:\\share\\project")).toBe(false) + expect(matches("C:\\share\\project\\docs\\notes.md", ["//C/share/project/docs/**"])).toBe(false) + expect(matches("\\\\C\\share\\project\\docs\\notes.md", ["C:/share/project/docs/**"])).toBe(false) + }) + it("matches a workspace-relative path against a bare pattern", () => { expect(matches("notes.md", ["notes.md"])).toBe(true) expect(matches("docs\\notes.md", ["notes.md"])).toBe(true) @@ -99,4 +500,34 @@ describe("matching on Windows", () => { expect(matches("C:/path/to/repo/etc/passwd", ["passwd"])).toBe(true) expect(matches("C:/etc/passwd", ["passwd"])).toBe(false) }) + + it("matches equivalent workspace roots across drives with Windows case and ordered exclusions", () => { + const rootMatch = (patterns: string[]) => + matcher().isFileMatchedByPatterns({ + filePath: "d:\\storage\\project\\DOCS\\NOTES.md", + cwd: "C:\\Aliases\\Project", + canonicalCwd: "D:\\Storage\\Project", + patterns, + }) + expect(rootMatch(["docs/**"])).toBe(true) + expect(rootMatch(["c:/aliases/PROJECT/docs/**"])).toBe(true) + expect(rootMatch(["d:/STORAGE/project/docs/**"])).toBe(true) + expect(rootMatch(["docs/**", "!c:/aliases/project/docs/notes.md"])).toBe(false) + expect(rootMatch(["docs/**", "!D:/Storage/Project/docs/notes.md"])).toBe(false) + }) + + it("keeps Windows siblings and lexical-parent outside patterns separate from canonical roots", () => { + const rootMatch = (filePath: string, patterns: string[]) => + matcher().isFileMatchedByPatterns({ + filePath, + cwd: "C:\\Aliases\\Project", + canonicalCwd: "D:\\Storage\\Project", + patterns, + }) + expect(rootMatch("C:/Aliases/shared/file.md", ["../shared/file.md"])).toBe(true) + expect(rootMatch("D:/Storage/shared/file.md", ["../shared/file.md"])).toBe(false) + expect(rootMatch("D:/Storage/shared/file.md", ["D:/Storage/shared/file.md"])).toBe(true) + expect(rootMatch("D:/Storage/Project-other/docs/file.md", ["docs/**"])).toBe(false) + expect(rootMatch("D:/Storage/Project-other/docs/file.md", ["C:/Aliases/Project-other/docs/**"])).toBe(false) + }) }) diff --git a/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts new file mode 100644 index 0000000000..c74fbf9493 --- /dev/null +++ b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts @@ -0,0 +1,200 @@ +import * as fs from "fs/promises" +import type { ClineSayTool } from "@roo-code/types" +import { baseState, type State } from "./fixtures" +import { matcherForPlatform } from "./matcherForPlatform" + +vi.mock("fs/promises", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, realpath: vi.fn() } +}) + +const approvalRoutes = [ + { name: "read allowlist", tool: "readFile", setting: "allowedReadFiles" }, + { name: "write allowlist granting reads", tool: "readFile", setting: "allowedWriteFiles" }, + { name: "write allowlist", tool: "newFileCreated", setting: "allowedWriteFiles" }, +] as const + +const ROOTED_EXCLUSIONS = [ + "!/**/.env", + "!///**/.env", + "!////**/.env", + "!\\**\\.env", + "!\\\\\\**\\.env", + "!\\\\\\\\**\\.env", +] + +async function approval( + tool: "readFile" | "newFileCreated", + path: string, + cwd: string | undefined, + state: Partial, + isProtected = false, +) { + const { checkAutoApproval } = await import("../index") + return checkAutoApproval({ + state: { ...baseState, ...state }, + cwd, + ask: "tool", + text: JSON.stringify({ tool, path, isOutsideWorkspace: true } satisfies ClineSayTool), + isProtected, + }) +} + +describe("Windows rooted approval rules", () => { + matcherForPlatform("win32") + + beforeEach(() => { + // An unresolved root keeps these checks on the lexical Windows paths. + vi.mocked(fs.realpath).mockReset().mockRejectedValue(new Error("workspace unavailable")) + vi.spyOn(console, "error").mockImplementation(() => {}) + }) + + afterEach(() => vi.restoreAllMocks()) + + describe.each(approvalRoutes)("$name", ({ tool, setting }) => { + it.each(["///server/share/docs/**", "\\\\\\server\\share\\docs\\**"])( + "keeps the rooted pattern %s separate from an explicit UNC grant", + async (pattern) => { + const cwd = "C:/repo" + const uncFile = "//server/share/docs/private.md" + expect(await approval(tool, uncFile, cwd, { [setting]: [pattern] })).toEqual({ decision: "ask" }) + expect(await approval(tool, "C:/server/share/docs/private.md", cwd, { [setting]: [pattern] })).toEqual({ + decision: "approve", + }) + expect(await approval(tool, uncFile, cwd, { [setting]: ["//server/share/docs/**"] })).toEqual({ + decision: "approve", + }) + expect( + await approval(tool, uncFile, cwd, { [setting]: ["//server/share/docs/**", `!${pattern}`] }), + ).toEqual({ decision: "approve" }) + }, + ) + + it.each( + [undefined, "/ws", "//server/share/ws"].flatMap((cwd) => + ROOTED_EXCLUSIONS.map((exclusion) => ({ cwd, exclusion })), + ), + )("applies ordered exclusion $exclusion when cwd is $cwd", async ({ cwd, exclusion }) => { + const patterns = ["C:/**", exclusion] + expect(await approval(tool, "C:/x/notes.md", cwd, { [setting]: patterns })).toEqual({ + decision: "approve", + }) + expect(await approval(tool, "C:/x/.env", cwd, { [setting]: patterns })).toEqual({ decision: "ask" }) + expect(await approval(tool, "C:/x/.env", cwd, { [setting]: [exclusion, "C:/**"] })).toEqual({ + decision: "approve", + }) + expect(await approval(tool, "C:/x/.env", cwd, { [setting]: [...patterns, "C:/x/.env"] })).toEqual({ + decision: "approve", + }) + }) + + it.each(ROOTED_EXCLUSIONS)("keeps exclusion %s on a known workspace drive", async (exclusion) => { + const state = { [setting]: ["C:/**", "D:/**", exclusion] } + expect(await approval(tool, "C:/x/.env", "D:/ws", state)).toEqual({ decision: "approve" }) + expect(await approval(tool, "D:/x/.env", "D:/ws", state)).toEqual({ decision: "ask" }) + expect(await approval(tool, "D:/x/notes.md", "D:/ws", state)).toEqual({ decision: "approve" }) + }) + + it.each(["!//C/x/.env", "!\\\\C\\x\\.env"])( + "keeps explicit UNC exclusion %s on its share", + async (exclusion) => { + const state = { [setting]: ["C:/**", "//C/x/**", "//other/x/**", exclusion] } + expect(await approval(tool, "C:/x/.env", undefined, state)).toEqual({ decision: "approve" }) + expect(await approval(tool, "//C/x/.env", undefined, state)).toEqual({ decision: "ask" }) + expect(await approval(tool, "//other/x/.env", undefined, state)).toEqual({ decision: "approve" }) + }, + ) + + it.each(["//server/sh[a]re/project", "\\\\server\\sh[a]re\\project"])( + "keeps the inferred UNC share literal for cwd %s", + async (cwd) => { + const ownFile = "//server/sh[a]re/project/docs/notes.md" + const neighborFile = "//server/share/project/docs/notes.md" + const rootedGrant = { [setting]: ["/project/docs/**"] } + expect(await approval(tool, ownFile, cwd, rootedGrant)).toEqual({ decision: "approve" }) + expect(await approval(tool, neighborFile, cwd, rootedGrant)).toEqual({ decision: "ask" }) + + const patterns = ["//server/**", "!/project/docs/.env"] + expect( + await approval(tool, "//server/sh[a]re/project/docs/.env", cwd, { [setting]: patterns }), + ).toEqual({ decision: "ask" }) + expect(await approval(tool, "//server/share/project/docs/.env", cwd, { [setting]: patterns })).toEqual({ + decision: "approve", + }) + expect( + await approval(tool, "//server/sh[a]re/project/docs/.env", cwd, { + [setting]: [...patterns, "/project/docs/.env"], + }), + ).toEqual({ decision: "approve" }) + }, + ) + }) + + it.each(["allowedReadFiles", "allowedWriteFiles"] as const)( + "keeps UNC parent traversal within the share when %s grants reads", + async (setting) => { + const cwd = "//server/share/../project" + const file = "//server/share/project/docs/private.md" + vi.mocked(fs.realpath).mockResolvedValue("//server/share/project") + expect(await approval("readFile", file, cwd, { [setting]: ["//server/project/docs/**"] })).toEqual({ + decision: "ask", + }) + expect(await approval("readFile", file, cwd, { [setting]: ["//server/share/project/docs/**"] })).toEqual({ + decision: "approve", + }) + expect( + await approval("readFile", file, cwd, { + [setting]: ["//server/share/project/docs/**", "!//server/share/project/docs/private.md"], + }), + ).toEqual({ decision: "ask" }) + }, + ) + + it.each(ROOTED_EXCLUSIONS)("applies exclusion %s to reads through the verified drive alias", async (exclusion) => { + const cwd = "//server/sh[a]re/project" + vi.mocked(fs.realpath).mockResolvedValue("D:/storage/project") + const patterns = ["/project/docs/**", exclusion.replace("**", "D/storage/**")] + expect( + await approval("readFile", "D:/storage/project/docs/notes.md", cwd, { allowedReadFiles: patterns }), + ).toEqual({ + decision: "approve", + }) + expect(await approval("readFile", "D:/storage/project/docs/.env", cwd, { allowedReadFiles: patterns })).toEqual( + { + decision: "ask", + }, + ) + expect( + await approval("readFile", "D:/storage/project/docs/.env", cwd, { + allowedReadFiles: [...patterns, "/project/docs/.env"], + }), + ).toEqual({ decision: "approve" }) + }) + + it.each(["allowedReadFiles", "allowedWriteFiles"] as const)( + "does not let an exclusion in %s cancel the other list's read grant", + async (excludedSetting) => { + const state = { allowedReadFiles: ["C:/**"], allowedWriteFiles: ["C:/**"] } + state[excludedSetting] = ["C:/**", "!/**/.env"] + expect(await approval("readFile", "C:/x/.env", "//server/share/ws", state)).toEqual({ + decision: "approve", + }) + expect( + await approval("readFile", "C:/x/.env", "//server/share/ws", { + allowedReadFiles: ["C:/**", "!/**/.env"], + allowedWriteFiles: ["C:/**", "!/**/.env"], + }), + ).toEqual({ decision: "ask" }) + }, + ) + + it("requires protected-write permission even when a rooted UNC grant matches", async () => { + const cwd = "//server/sh[a]re/project" + const path = "//server/sh[a]re/project/AGENTS.md" + const state = { allowedWriteFiles: ["/project/*.md"] } + expect(await approval("newFileCreated", path, cwd, state, true)).toEqual({ decision: "ask" }) + expect( + await approval("newFileCreated", path, cwd, { ...state, alwaysAllowWriteProtected: true }, true), + ).toEqual({ decision: "approve" }) + }) +}) diff --git a/src/core/auto-approval/__tests__/windowsUnicodeRoots.spec.ts b/src/core/auto-approval/__tests__/windowsUnicodeRoots.spec.ts new file mode 100644 index 0000000000..6e8553314c --- /dev/null +++ b/src/core/auto-approval/__tests__/windowsUnicodeRoots.spec.ts @@ -0,0 +1,130 @@ +import * as fs from "fs/promises" +import { baseState } from "./fixtures" +import { matcherForPlatform } from "./matcherForPlatform" + +vi.mock("fs/promises", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, realpath: vi.fn() } +}) + +const DISTINCT_ROOT_NAMES = [ + { root: "\u0130", sibling: "i\u0307" }, + { root: "i\u0307", sibling: "\u0130" }, + { root: "\u212a", sibling: "K" }, + { root: "K", sibling: "\u212a" }, +] as const + +const CANONICAL_CWD = "D:/storage/project" + +describe("Windows Unicode workspace-root identity", () => { + const matcher = matcherForPlatform("win32") + + beforeEach(() => { + vi.mocked(fs.realpath).mockReset().mockResolvedValue(CANONICAL_CWD) + }) + + it.each(DISTINCT_ROOT_NAMES)( + "does not translate distinct lexical roots $root and $sibling", + ({ root, sibling }) => { + const cwd = `C:/aliases/${root}` + const matches = (filePath: string) => + matcher().isFileMatchedByPatterns({ + cwd, + canonicalCwd: CANONICAL_CWD, + filePath, + patterns: ["D:/storage/**"], + }) + + // A broad grant in the canonical parent must not approve another lexical + // directory through a case fold that expands or changes the root name. + expect(matches(`C:/aliases/${sibling}/docs/private.md`)).toBe(false) + expect(matches(`${cwd}-other/docs/private.md`)).toBe(false) + expect(matches(`${cwd}/Docs/NOTES.md`)).toBe(true) + expect(matches(`${CANONICAL_CWD}/Docs/NOTES.md`)).toBe(true) + }, + ) + + it.each(DISTINCT_ROOT_NAMES)( + "does not translate distinct canonical roots $root and $sibling", + ({ root, sibling }) => { + const cwd = "C:/aliases/project" + const canonicalCwd = `D:/storage/${root}` + const matches = (filePath: string) => + matcher().isFileMatchedByPatterns({ cwd, canonicalCwd, filePath, patterns: ["C:/aliases/**"] }) + + expect(matches(`D:/storage/${sibling}/docs/private.md`)).toBe(false) + expect(matches(`${canonicalCwd}-other/docs/private.md`)).toBe(false) + expect(matches(`${canonicalCwd}/Docs/NOTES.md`)).toBe(true) + expect(matches(`${cwd}/Docs/NOTES.md`)).toBe(true) + }, + ) + + it.each(DISTINCT_ROOT_NAMES)( + "asks before reading the distinct sibling $sibling of workspace $root", + async ({ root, sibling }) => { + const { checkAutoApproval } = await import("../index") + const cwd = `C:/aliases/${root}` + const approve = (path: string) => + checkAutoApproval({ + cwd, + state: { ...baseState, allowedReadFiles: ["D:/storage/**"] }, + ask: "tool", + text: JSON.stringify({ tool: "readFile", path, isOutsideWorkspace: true }), + }) + + expect(await approve(`C:/aliases/${sibling}/docs/private.md`)).toEqual({ decision: "ask" }) + expect(await approve(`${cwd}/docs/notes.md`)).toEqual({ decision: "approve" }) + expect(await approve(`${CANONICAL_CWD}/docs/notes.md`)).toEqual({ decision: "approve" }) + }, + ) + + it.each(["\u03c3", "\u03c2"])("keeps sigma case matching and ordered canonical exclusions for %s", (sigma) => { + const cwd = "C:/aliases/\u03a3" + const filePath = `C:/aliases/${sigma}/Docs/private.md` + const matches = (patterns: string[]) => + matcher().isFileMatchedByPatterns({ cwd, canonicalCwd: CANONICAL_CWD, filePath, patterns }) + + expect(matches(["D:/storage/**"])).toBe(true) + expect(matches(["C:/aliases/**", `!${CANONICAL_CWD}/Docs/private.md`])).toBe(false) + expect(matches([`!${CANONICAL_CWD}/Docs/private.md`, "C:/aliases/**"])).toBe(true) + expect(matches(["C:/aliases/**", `!${CANONICAL_CWD}/Docs/private.md`, filePath])).toBe(true) + expect(matches([`${CANONICAL_CWD}/Docs/private.md`])).toBe(true) + }) + + it("keeps canonical exclusions effective for a sigma spelling through read approval", async () => { + const { checkAutoApproval } = await import("../index") + const cwd = "C:/aliases/\u03a3" + const path = "C:/aliases/\u03c2/Docs/private.md" + const approve = (allowedReadFiles: string[]) => + checkAutoApproval({ + cwd, + state: { ...baseState, allowedReadFiles }, + ask: "tool", + text: JSON.stringify({ tool: "readFile", path }), + }) + + expect(await approve(["C:/aliases/**", `!${CANONICAL_CWD}/Docs/private.md`])).toEqual({ decision: "ask" }) + expect(await approve(["C:/aliases/**", `!${CANONICAL_CWD}/Docs/private.md`, path])).toEqual({ + decision: "approve", + }) + }) + + it.each(["Project(1)", "Project+", "Project$", "Project^", "Project{2}", "Project|other"])( + "keeps regular-expression characters literal in the root %s", + (name) => { + const cwd = `C:/aliases/${name}` + const matches = (filePath: string) => + matcher().isFileMatchedByPatterns({ + cwd, + canonicalCwd: CANONICAL_CWD, + filePath, + patterns: ["D:/storage/**"], + }) + + expect(matches(`${cwd}/docs/notes.md`)).toBe(true) + for (const sibling of ["Project", "Project1", "Projectt", "other"]) { + expect(matches(`C:/aliases/${sibling}/docs/notes.md`)).toBe(false) + } + }, + ) +}) diff --git a/src/core/auto-approval/__tests__/workspaceRootEquivalence.spec.ts b/src/core/auto-approval/__tests__/workspaceRootEquivalence.spec.ts new file mode 100644 index 0000000000..a59ba0b7e4 --- /dev/null +++ b/src/core/auto-approval/__tests__/workspaceRootEquivalence.spec.ts @@ -0,0 +1,208 @@ +import * as fs from "fs/promises" +import type { ClineSayTool } from "@roo-code/types" +import { checkAutoApproval } from "../index" +import { isFileMatchedByPatterns } from "../filePatterns" +import { baseState, type State } from "./fixtures" + +vi.mock("fs/promises", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, realpath: vi.fn() } +}) + +const CWD = "/aliases/project" +const CANONICAL_CWD = "/storage/project" + +function matches(filePath: string, patterns: string[]) { + return isFileMatchedByPatterns({ filePath, cwd: CWD, canonicalCwd: CANONICAL_CWD, patterns, isWindows: false }) +} + +function approval(tool: ClineSayTool, state: Partial, cwd: string | undefined = CWD, isProtected = false) { + return checkAutoApproval({ + state: { ...baseState, ...state }, + cwd, + ask: "tool", + text: JSON.stringify(tool), + isProtected, + }) +} + +beforeEach(() => { + vi.mocked(fs.realpath).mockReset().mockResolvedValue(CANONICAL_CWD) +}) + +afterEach(() => vi.restoreAllMocks()) + +describe("read matcher workspace-root equivalence", () => { + it.each([CWD, CANONICAL_CWD, "docs/private.md"])( + "preserves wildcard exclusions spanning either root for %s", + (candidateRoot) => { + const candidate = candidateRoot === "docs/private.md" ? candidateRoot : `${candidateRoot}/docs/private.md` + for (const exclusion of ["!/aliases/**/private.md", "!/storage/proj*/docs/private.md"]) { + expect(matches(candidate, ["docs/**", exclusion])).toBe(false) + expect(matches(candidate, [exclusion, "docs/**"])).toBe(true) + expect(matches(candidate, ["docs/**", exclusion, `${CANONICAL_CWD}/docs/private.md`])).toBe(true) + } + }, + ) + + it.each(["project[1]", "project*", "project?"])("keeps the canonical root %s literal", (name) => { + const rootMatch = (filePath: string) => + isFileMatchedByPatterns({ + filePath, + cwd: CWD, + canonicalCwd: `/storage/${name}`, + patterns: ["docs/**"], + isWindows: false, + }) + expect(rootMatch(`/storage/${name}/docs/notes.md`)).toBe(true) + expect(rootMatch(`${CWD}/docs/notes.md`)).toBe(true) + for (const sibling of ["project1", "projectx", "project3", "projectf"]) { + expect(rootMatch(`/storage/${sibling}/docs/notes.md`)).toBe(false) + } + }) + + it.each(["project[1]", "project*", "project?"])("keeps the lexical root %s literal", (name) => { + const rootMatch = (filePath: string) => + isFileMatchedByPatterns({ + filePath, + cwd: `/aliases/${name}`, + canonicalCwd: CANONICAL_CWD, + patterns: ["docs/**"], + isWindows: false, + }) + expect(rootMatch(`/aliases/${name}/docs/notes.md`)).toBe(true) + expect(rootMatch(`${CANONICAL_CWD}/docs/notes.md`)).toBe(true) + for (const sibling of ["project1", "projectx", "project3", "projectf"]) { + expect(rootMatch(`/aliases/${sibling}/docs/notes.md`)).toBe(false) + } + }) + + it.each([CWD, CANONICAL_CWD])("honors %s exclusions and later re-inclusion in one ordered matcher", (root) => { + const candidate = `${CANONICAL_CWD}/docs/private.md` + const exclusion = `!${root}/docs/private.md` + + expect(matches(candidate, ["docs/**", exclusion])).toBe(false) + expect(matches(candidate, [exclusion, "docs/**"])).toBe(true) + expect(matches(candidate, ["docs/**", exclusion, `${CWD}/docs/private.md`])).toBe(true) + }) + + it("resolves escaping patterns against the lexical parent, not the canonical parent", () => { + expect(matches("/aliases/shared/file.md", ["../shared/file.md"])).toBe(true) + expect(matches("/storage/shared/file.md", ["../shared/file.md"])).toBe(false) + expect(matches("/storage/shared/file.md", ["/storage/shared/file.md"])).toBe(true) + expect(matches("/aliases/shared/file.md", ["/storage/shared/file.md"])).toBe(false) + expect(matches(`${CANONICAL_CWD}/docs/file.md`, ["../project/docs/file.md"])).toBe(true) + }) + + it("does not translate similarly prefixed siblings or permit relative rules to cover other roots", () => { + expect(matches(`${CANONICAL_CWD}-other/docs/file.md`, ["docs/**"])).toBe(false) + expect(matches(`${CWD}-other/docs/file.md`, ["docs/**"])).toBe(false) + expect(matches(`${CANONICAL_CWD}-other/docs/file.md`, [`${CWD}-other/docs/**`])).toBe(false) + expect(matches(`${CANONICAL_CWD}/docs/file.md`, [`${CWD}-other/docs/**`])).toBe(false) + expect(matches(`${CWD}-other/docs/file.md`, [`${CWD}-other/docs/**`])).toBe(true) + }) +}) + +describe("read-only root equivalence approval", () => { + it("keeps read and write allowlist exclusions independent while writes still imply reads", async () => { + const tool: ClineSayTool = { tool: "readFile", path: `${CANONICAL_CWD}/docs/private.md` } + const excludedRead = ["docs/**", `!${CWD}/docs/private.md`] + const excludedWrite = ["docs/**", `!${CANONICAL_CWD}/docs/private.md`] + + expect(await approval(tool, { allowedReadFiles: excludedRead, allowedWriteFiles: excludedWrite })).toEqual({ + decision: "ask", + }) + expect(await approval(tool, { allowedReadFiles: excludedRead, allowedWriteFiles: ["docs/**"] })).toEqual({ + decision: "approve", + }) + expect(await approval(tool, { allowedReadFiles: ["docs/**"], allowedWriteFiles: excludedWrite })).toEqual({ + decision: "approve", + }) + }) + + it("does not extend write matching or protected-file permission when roots are equivalent", async () => { + const canonicalTool: ClineSayTool = { tool: "newFileCreated", path: `${CANONICAL_CWD}/AGENTS.md` } + expect(await approval(canonicalTool, { allowedReadFiles: ["*.md"], allowedWriteFiles: ["*.md"] })).toEqual({ + decision: "ask", + }) + expect(await approval(canonicalTool, { allowedWriteFiles: [`${CANONICAL_CWD}/AGENTS.md`] }, CWD, true)).toEqual( + { decision: "ask" }, + ) + expect( + await approval({ tool: "newFileCreated", path: "AGENTS.md" }, { allowedWriteFiles: ["*.md"] }, CWD, true), + ).toEqual({ decision: "ask" }) + expect( + await approval( + { tool: "listFilesRecursive", path: `${CANONICAL_CWD}/docs` }, + { allowedReadFiles: ["docs/**"] }, + ), + ).toEqual({ decision: "ask" }) + expect(fs.realpath).not.toHaveBeenCalled() + }) + + it("requires every named batch target to match and refuses unnamed targets", async () => { + const state = { allowedReadFiles: ["docs/**", `!${CWD}/docs/private.md`] } + const listed = { path: `${CANONICAL_CWD}/docs/notes.md`, lineSnippet: "", key: "notes" } + const excluded = { path: `${CANONICAL_CWD}/docs/private.md`, lineSnippet: "", key: "private" } + const outside = { path: "/elsewhere/docs/notes.md", lineSnippet: "", key: "outside" } + + expect( + await approval( + { + tool: "readFile", + batchFiles: [listed, { path: `${CWD}/docs/other.md`, lineSnippet: "", key: "other" }], + }, + state, + ), + ).toEqual({ decision: "approve" }) + expect(await approval({ tool: "readFile", batchFiles: [listed, excluded] }, state)).toEqual({ decision: "ask" }) + expect(await approval({ tool: "readFile", batchFiles: [listed, outside] }, state)).toEqual({ decision: "ask" }) + expect(await approval({ tool: "readFile", batchFiles: [listed], additionalFileCount: 1 }, state)).toEqual({ + decision: "ask", + }) + }) + + it.each(["ENOENT", "EACCES"])( + "does not infer root equivalence when task root resolution fails with %s", + async (code) => { + const error = Object.assign(new Error("root unavailable"), { code }) + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}) + vi.mocked(fs.realpath).mockRejectedValue(error) + const tool: ClineSayTool = { + tool: "readFile", + path: `${CANONICAL_CWD}/docs/notes.md`, + isOutsideWorkspace: true, + } + + expect(await approval(tool, { allowedReadFiles: ["docs/**"] })).toEqual({ decision: "ask" }) + expect(await approval(tool, { allowedReadFiles: [`${CWD}/docs/**`] })).toEqual({ decision: "ask" }) + expect(await approval(tool, { allowedReadFiles: [`${CANONICAL_CWD}/docs/**`] })).toEqual({ + decision: "approve", + }) + expect(errorLog).toHaveBeenCalledTimes(3) + expect(errorLog).toHaveBeenCalledWith(`[auto-approval] Failed to resolve workspace root ${CWD}:`, error) + }, + ) + + it.each([undefined, ""])("does not resolve workspace-relative grants without a task root: %s", async (cwd) => { + const errorLog = vi.spyOn(console, "error").mockImplementation(() => {}) + const text = JSON.stringify({ + tool: "readFile", + path: `${CANONICAL_CWD}/docs/notes.md`, + isOutsideWorkspace: true, + }) + expect( + await checkAutoApproval({ state: { ...baseState, allowedReadFiles: ["docs/**"] }, cwd, ask: "tool", text }), + ).toEqual({ decision: "ask" }) + expect( + await checkAutoApproval({ + state: { ...baseState, allowedReadFiles: [`${CANONICAL_CWD}/docs/**`] }, + cwd, + ask: "tool", + text, + }), + ).toEqual({ decision: "approve" }) + expect(fs.realpath).not.toHaveBeenCalled() + expect(errorLog).not.toHaveBeenCalled() + }) +}) diff --git a/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts b/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts new file mode 100644 index 0000000000..ac48c9373e --- /dev/null +++ b/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts @@ -0,0 +1,81 @@ +import * as fs from "fs/promises" +import os from "os" +import path from "path" +import { getReadablePath } from "../../../utils/path" +import { checkAutoApproval } from "../index" +import { baseState } from "./fixtures" + +it("keeps permissions and wildcard exclusions scoped to a real symlinked workspace", async () => { + const temp = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-workspace-approval-")) + try { + const canonicalRoot = path.join(temp, "storage", "project[1]") + const lexicalParent = path.join(temp, "aliases") + const cwd = path.join(lexicalParent, "project") + const inside = path.join(canonicalRoot, "docs", "private.md") + const outside = path.join(temp, "storage", "project1", "docs", "private.md") + await fs.mkdir(path.dirname(inside), { recursive: true }) + await fs.mkdir(path.dirname(outside), { recursive: true }) + await fs.mkdir(lexicalParent) + await fs.writeFile(inside, "allowed workspace") + await fs.writeFile(outside, "unrelated workspace") + // A directory junction also works on Windows without symlink privileges. + await fs.symlink(canonicalRoot, cwd, "junction") + + const approval = async (filePath: string, patterns: string[]) => + checkAutoApproval({ + cwd, + state: { ...baseState, allowedReadFiles: patterns }, + ask: "tool", + text: JSON.stringify({ tool: "readFile", path: filePath, isOutsideWorkspace: true }), + }) + const canonicalInside = await fs.realpath(inside) + const canonicalOutside = await fs.realpath(outside) + expect(await approval(canonicalInside, ["docs/**"])).toEqual({ decision: "approve" }) + expect(await approval(canonicalOutside, ["docs/**"])).toEqual({ decision: "ask" }) + const exclusion = `!${lexicalParent}/**/private.md` + expect(await approval(canonicalInside, ["docs/**", exclusion])).toEqual({ decision: "ask" }) + expect(await approval("docs/private.md", ["docs/**", exclusion])).toEqual({ decision: "ask" }) + expect(await approval(canonicalInside, [exclusion, "docs/**"])).toEqual({ decision: "approve" }) + } finally { + await fs.rm(temp, { recursive: true, force: true }) + } +}) + +it("checks root identity after the same parent normalization used by file reads", async () => { + const temp = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-workspace-parent-approval-")) + try { + const canonicalTemp = await fs.realpath(temp) + const lexicalRoot = path.join(canonicalTemp, "lexical") + const targetRoot = path.join(canonicalTemp, "target") + const targetChild = path.join(targetRoot, "child") + const link = path.join(lexicalRoot, "link") + const relativePath = path.join("docs", "private.md") + await fs.mkdir(path.join(lexicalRoot, "docs"), { recursive: true }) + await fs.mkdir(path.join(targetRoot, "docs"), { recursive: true }) + await fs.mkdir(targetChild) + await fs.writeFile(path.join(lexicalRoot, relativePath), "lexical file") + await fs.writeFile(path.join(targetRoot, relativePath), "different target file") + await fs.symlink(targetChild, link, "junction") + + // Preserve the parent segment until the same path.resolve used by the reader. + // Native realpath may follow the link first and identify a different root. + const cwd = `${link}${path.sep}..` + const readPath = path.resolve(cwd, relativePath) + const toolPath = getReadablePath(cwd, relativePath) + expect(await fs.readFile(readPath, "utf8")).toBe("lexical file") + expect(await fs.readFile(path.join(targetRoot, relativePath), "utf8")).toBe("different target file") + + const approval = (patterns: string[]) => + checkAutoApproval({ + cwd, + state: { ...baseState, allowedReadFiles: patterns }, + ask: "tool", + text: JSON.stringify({ tool: "readFile", path: toolPath, isOutsideWorkspace: true }), + }) + expect(await approval([`${targetRoot}/docs/**`])).toEqual({ decision: "ask" }) + expect(await approval([`${lexicalRoot}/docs/**`])).toEqual({ decision: "approve" }) + expect(await approval(["docs/**"])).toEqual({ decision: "approve" }) + } finally { + await fs.rm(temp, { recursive: true, force: true }) + } +}) diff --git a/src/core/auto-approval/filePatterns.ts b/src/core/auto-approval/filePatterns.ts index 923d29815d..1a9a857f80 100644 --- a/src/core/auto-approval/filePatterns.ts +++ b/src/core/auto-approval/filePatterns.ts @@ -3,6 +3,10 @@ import path from "path" import ignore from "ignore" +import { WorkspaceRootCanonicalizer } from "./WorkspaceRootCanonicalizer" +import { WorkspaceRootStrategy } from "./strategies/WorkspaceRootStrategy" +import { WINDOWS_UNC_PREFIX } from "./strategies/WindowsWorkspaceRootStrategy" + /** * Matching of file paths against user-configured file patterns. * @@ -72,7 +76,7 @@ import ignore from "ignore" * Both the configured patterns and the path being checked are therefore * rewritten into a single form the library accepts: a path relative to the * filesystem root (`/`), with any Windows drive as its first segment, in the - * spelling `toPosixAbsolutePath` gives it. + * spelling supplied by the selected path strategy. * * An alternative implementation would be to remember for each pattern whether * it's a workspace-scoped pattern or an absolute-filesystem pattern, and then @@ -155,56 +159,35 @@ function pathsepsToPosix(value: string, isWindows: boolean): string { } /** - * A Windows drive at the root of a path: the `C:` in `C:/tmp/x`, or `C:` alone. + * A Windows drive at the root of an absolute path: the `C:` in `C:/tmp/x`. * - * The lookahead is what limits this to a drive *root*. Windows also reads `C:tmp` - * as naming a drive, but relative to the current directory on it, which is not a - * place this module can resolve. + * Require a separator after the colon. Both `C:` and `C:tmp` are relative to + * the current directory on that drive, which this module cannot safely infer. */ -const DRIVE_PREFIX = /^([a-zA-Z]):(?=\/|$)/ +const DRIVE_PREFIX = /^([a-zA-Z]):(?=\/)/ /** - * Rewrite an absolute path into the plain POSIX form the rest of this module - * works in, where a Windows drive is an ordinary first path segment. - * - * - `"C:/tmp/x"` -> `"/C/tmp/x"` - * - `"/tmp/x"` with `workspaceDrive = "/D"` -> `"/D/tmp/x"`, since Windows reads a - * path written without a drive as being on the current one - * - `"/tmp/x"` off Windows -> `"/tmp/x"`, where there are no drives to place it on - * - * The colon has to go, working around `ignore` behaviour: - * On Windows, `ignore` rejects any path that looks drive-rooted - * (it tests `/^[a-z]:\//i` and throws `RangeError`, - * "path should be a `path.relative()`d string"). - * - * The bare letter cannot be mistaken for a directory of the same name, because it - * only ever goes where Windows puts the drive, at the very root of the path: a - * directory called `C` always sits in a later segment. - * - * Its case is left as typed, since a drive only occurs on Windows, where the - * matcher ignores case anyway (see the case-sensitivity note at the top), so - * `C:/x` and `c:/x` already agree. - * - * A drive is only read as one when reading paths as Windows does. Elsewhere `C:` - * is an ordinary directory name (`mkdir 'C:'` succeeds on Linux), so `C:/notes.md` - * is a *relative* path naming a file in it, and taking it for a drive would anchor - * it to the filesystem root instead of the workspace. + * The Windows volume in matcher notation: /C for a drive, /UNC/server/share + * for a network share. Empty when no volume is known, and always off Windows. */ -function toPosixAbsolutePath(absolutePath: string, workspaceDrive: string, isWindows: boolean): string { +function volumeSegmentOf(absolutePath: string, isWindows: boolean): string { + if (isWindows) { + const unc = WINDOWS_UNC_PREFIX.exec(absolutePath) + if (unc) return `/UNC/${unc[1]}/${unc[2]}` + } const drive = isWindows ? DRIVE_PREFIX.exec(absolutePath) : null - return drive ? `/${drive[1]}${absolutePath.slice(drive[0].length)}` : `${workspaceDrive}${absolutePath}` + return drive ? `/${drive[1]}` : "" } -/** - * The drive a path is on, spelled as the path segment that `toPosixAbsolutePath` - * gives it: `"/C"` for `"C:/tmp/x"`. Empty when the path names no drive, as is - * always the case off Windows. - */ -function driveSegmentOf(absolutePath: string, isWindows: boolean): string { - const drive = isWindows ? DRIVE_PREFIX.exec(absolutePath) : null +/** Filesystem-provided prefixes are literal names, not user-authored globs. */ +function escapePatternLiteral(value: string): string { + return value.replace(/[\\*?[\]]/g, (character) => `\\${character}`) +} - return drive ? `/${drive[1]}` : "" +/** A Windows drive/share is a filesystem root; .. cannot climb above it. */ +function resolveMatcherPath(root: string, relativePath: string, volume: string): string { + return `${volume}${path.posix.resolve(root.slice(volume.length) || "/", relativePath)}`.replace(/\/$/, "") || "/" } /** @@ -215,6 +198,11 @@ function isAbsolutePath(value: string, isWindows: boolean): boolean { return value.startsWith("/") || (isWindows && DRIVE_PREFIX.test(value)) } +/** Windows drive-relative paths depend on per-drive process state, not the task workspace. */ +function isDriveRelativePath(value: string, isWindows: boolean): boolean { + return isWindows && /^[a-zA-Z]:(?!\/)/.test(value) +} + function escapesWorkspace(posixPath: string): boolean { return posixPath.split("/").includes("..") } @@ -270,7 +258,7 @@ function toRootRelativePath(posixAbsolutePath: string): string { * - `"../shared/notes.md"` -> `"/path/to/shared/notes.md"` * resolved against the workspace root, so it lands outside the workspace * - `"~/notes.md"` -> `"/home/me/notes.md"` - * - `"C:/tmp/notes.md"` on Windows -> `"/C/tmp/notes.md"`, see `toPosixAbsolutePath()` + * - `"C:/tmp/notes.md"` on Windows -> `"/C/tmp/notes.md"`, using the path strategy * - `"!docs/secret.md"` -> `"!/path/to/repo/docs/secret.md"` * - `"mydir/"` -> `undefined` (because it names a dir) * - `"~"` -> `undefined` (because it names a dir) @@ -284,6 +272,8 @@ function toRootRelativePath(posixAbsolutePath: string): string { * a backslash escapes the character after it. Only patterns consisting solely of * whitespace are rejected, since they name no file. * + * @internal Direct-test seam for pattern rewriting; runtime shares its strategy through toMatcherPatternWithStrategy. + * * @param pattern - Raw pattern as typed by the user. * @param cwd - Workspace root, used to resolve workspace-relative patterns. * @param isWindows - Whether to read paths by Windows' rules; see `pathsepsToPosix`. @@ -297,6 +287,16 @@ export function toMatcherPattern( cwd?: string, isWindows = runningOnWindows(), homeDir = os.homedir(), +): string | undefined { + return toMatcherPatternWithStrategy(pattern, cwd, isWindows, homeDir, WorkspaceRootStrategy.forPlatform(isWindows)) +} + +function toMatcherPatternWithStrategy( + pattern: string, + cwd: string | undefined, + isWindows: boolean, + homeDir: string, + strategy: WorkspaceRootStrategy, ): string | undefined { // Set gitignore's negation aside so the path is rewritten on its own merits, // then restore it, so that a negation is anchored exactly like the pattern it @@ -307,14 +307,20 @@ export function toMatcherPattern( // as gitignore's escape character and a legal filename character. let normalized = pathsepsToPosix(pattern.slice(negation.length), isWindows) - if (!normalized.trim() || normalized === "." || normalized === "~" || normalized.endsWith("/")) { + if ( + !normalized.trim() || + isDriveRelativePath(normalized, isWindows) || + normalized === "." || + normalized === "~" || + normalized.endsWith("/") + ) { return undefined } const workspacePosix = pathsepsToPosix(cwd ?? "", isWindows) - // A path written without a drive is read as being on the workspace's, which is - // the drive the tool call it came from would resolve it against. - const workspaceDrive = driveSegmentOf(workspacePosix, isWindows) + // A rooted Windows path belongs to the workspace's drive or network share. + const workspaceVolume = volumeSegmentOf(workspacePosix, isWindows) + const matcherVolume = strategy.toMatcherAbsolutePath("/", workspaceVolume).replace(/\/$/, "") if (normalized.startsWith("~/")) { // Left in the user's own spelling, drive and all, for the absolute branch @@ -323,21 +329,25 @@ export function toMatcherPattern( } if (isAbsolutePath(normalized, isWindows)) { - return `${negation}${toPosixAbsolutePath(normalized, workspaceDrive, isWindows)}` + // Only the inherited volume is a literal filesystem name; the user's + // absolute pattern can still contain globs, including an explicit UNC share. + return `${negation}${strategy.toMatcherAbsolutePath(normalized, escapePatternLiteral(workspaceVolume))}` } - if (!cwd) { - // No workspace for a relative pattern to be relative to; see the note above + if (!cwd || isDriveRelativePath(workspacePosix, isWindows)) { + // An unknown or drive-relative workspace cannot anchor a relative pattern; see the note above // on why this is not matched loosely instead. return undefined } // `resolve` also normalises, so that a `cwd` with a trailing slash or a `..` // segment does not reach the patterns below. - const workspaceRoot = path.posix.resolve(toPosixAbsolutePath(workspacePosix, workspaceDrive, isWindows)) + const workspaceRoot = escapePatternLiteral( + resolveMatcherPath(strategy.toMatcherAbsolutePath(workspacePosix, workspaceVolume), ".", matcherVolume), + ) if (escapesWorkspace(normalized)) { - return `${negation}${path.posix.resolve(workspaceRoot, normalized)}` + return `${negation}${resolveMatcherPath(workspaceRoot, normalized, escapePatternLiteral(matcherVolume))}` } // "./notes.md" names the workspace root explicitly. @@ -355,7 +365,7 @@ export function toMatcherPattern( // number of directories including none, restores its reach. const matchesInAnyDirectory = !isWorkspaceRootAnchored && !normalized.includes("/") - return `${negation}${workspaceRoot}/${matchesInAnyDirectory ? "**/" : ""}${normalized}` + return `${negation}${workspaceRoot.replace(/\/$/, "")}/${matchesInAnyDirectory ? "**/" : ""}${normalized}` } /** @@ -365,31 +375,37 @@ export function toMatcherPattern( * @returns The rewritten path, or `undefined` when it names no file, or when it * is relative and there is no workspace root to resolve it against. */ -function toMatcherPath(filePath: string, cwd: string | undefined, isWindows: boolean): string | undefined { +function toMatcherPath( + filePath: string, + cwd: string | undefined, + isWindows: boolean, + strategy: WorkspaceRootStrategy, +): string | undefined { // Not trimmed: whitespace can be part of a filename. const normalized = pathsepsToPosix(filePath, isWindows) - if (!normalized.trim() || normalized === ".") { + if (!normalized.trim() || isDriveRelativePath(normalized, isWindows) || normalized === ".") { return undefined } const workspacePosix = pathsepsToPosix(cwd ?? "", isWindows) - const workspaceDrive = driveSegmentOf(workspacePosix, isWindows) + const workspaceVolume = volumeSegmentOf(workspacePosix, isWindows) + const matcherVolume = strategy.toMatcherAbsolutePath("/", workspaceVolume).replace(/\/$/, "") if (isAbsolutePath(normalized, isWindows)) { - return toRootRelativePath(toPosixAbsolutePath(normalized, workspaceDrive, isWindows)) + return toRootRelativePath(strategy.toMatcherAbsolutePath(normalized, workspaceVolume)) } - if (!cwd) { - // A relative path cannot be placed on the filesystem without a root, and + if (!cwd || isDriveRelativePath(workspacePosix, isWindows)) { + // A relative path cannot be placed without a known, non-drive-relative root, and // the only patterns that survive without one are absolute, which such a // path could never match anyway. return undefined } - const workspaceRoot = toPosixAbsolutePath(workspacePosix, workspaceDrive, isWindows) + const workspaceRoot = strategy.toMatcherAbsolutePath(workspacePosix, workspaceVolume) - return toRootRelativePath(path.posix.resolve(workspaceRoot, normalized)) + return toRootRelativePath(resolveMatcherPath(workspaceRoot, normalized, matcherVolume)) } /** @@ -411,12 +427,15 @@ function toMatcherPath(filePath: string, cwd: string | undefined, isWindows: boo export function isFileMatchedByPatterns({ filePath, cwd, + canonicalCwd, patterns, isWindows = runningOnWindows(), homeDir = os.homedir(), }: { filePath?: string cwd?: string + /** Verified realpath of the lexically resolved cwd, used only by read approval. */ + canonicalCwd?: string patterns?: string[] isWindows?: boolean homeDir?: string @@ -425,24 +444,53 @@ export function isFileMatchedByPatterns({ return false } - const candidate = toMatcherPath(filePath, cwd, isWindows) + const strategy = WorkspaceRootStrategy.forPlatform(isWindows) + const candidate = toMatcherPath(filePath, cwd, isWindows, strategy) if (!candidate) { return false } - const matcherPatterns = patterns - .map((pattern) => toMatcherPattern(pattern, cwd, isWindows, homeDir)) - .filter((pattern): pattern is string => !!pattern) - - if (!matcherPatterns.length) { - return false - } + const workspaceRoot = WorkspaceRootCanonicalizer.forStrategy(cwd, canonicalCwd, strategy) + const candidates = workspaceRoot.equivalentPaths(`/${candidate}`).map(toRootRelativePath) + // Before UNC/unknown-root namespaces were introduced, rooted exclusions in + // a workspace without a drive also matched drive paths (e.g. !/**/.env). + // Preserve that restriction only for drives, never for the internal UNC/ROOT + // prefixes, which must not be addressable by a user's rooted path. + const driveCandidates = + isWindows && !DRIVE_PREFIX.test(pathsepsToPosix(cwd ?? "", isWindows)) + ? candidates.filter((value) => /^[a-zA-Z]\//.test(value)) + : [] try { - return ignore({ ignoreCase: isWindows }) - .add([...matcherPatterns, MATCH_DIRECTORIES_ONLY_PATTERN]) - .ignores(candidate) + let matched = false + for (const rawPattern of patterns) { + const pattern = toMatcherPatternWithStrategy(rawPattern, cwd, isWindows, homeDir, strategy) + if (!pattern) continue + + const negated = pattern.startsWith("!") + const alternatives = [{ pattern: negated ? pattern.slice(1) : pattern, candidates }] + if (negated && driveCandidates.length) { + const rootedExclusion = pathsepsToPosix(rawPattern.slice(1), isWindows) + if (rootedExclusion.startsWith("/") && !WINDOWS_UNC_PREFIX.test(rootedExclusion)) { + alternatives.push({ + pattern: rootedExclusion.replace(/^\/+/, "/"), + candidates: driveCandidates, + }) + } + } + // Apply each rule to both spellings BEFORE advancing to the next rule. + // OR-ing two complete matchers would let one spelling bypass an exclusion. + if ( + alternatives.some(({ pattern, candidates }) => { + const matcher = ignore({ ignoreCase: isWindows }).add([pattern, MATCH_DIRECTORIES_ONLY_PATTERN]) + return candidates.some((value) => matcher.ignores(value)) + }) + ) { + matched = !negated + } + } + return matched } catch (error) { // A path the matcher rejects cannot be confirmed as matching, so treat it // as unmatched. diff --git a/src/core/auto-approval/index.ts b/src/core/auto-approval/index.ts index 3539eef24d..bc4aa36917 100644 --- a/src/core/auto-approval/index.ts +++ b/src/core/auto-approval/index.ts @@ -9,6 +9,8 @@ import { } from "@roo-code/types" import type { DcgDecision } from "../../services/destructive-command-guard/runner" +import { realpath } from "fs/promises" +import path from "path" import { ClineAskResponse } from "../../shared/WebviewMessage" @@ -87,6 +89,19 @@ function areAllNamedFilesMatched(tool: ClineSayTool, matchFun: (filePath: string return paths.every((filePath) => matchFun(filePath)) } +async function resolveCanonicalWorkspaceRoot(cwd: string | undefined): Promise { + if (!cwd) return undefined + + try { + // Read tools resolve paths lexically before accessing the filesystem. + // Verify that same root: resolving symlink/.. first can identify a different directory. + return await realpath(path.resolve(cwd)) + } catch (error) { + console.error(`[auto-approval] Failed to resolve workspace root ${cwd}:`, error) + return undefined + } +} + /** * Whether a read-only tool action is fully covered by the read allowlist patterns. * @@ -105,20 +120,23 @@ function areAllNamedFilesMatched(tool: ClineSayTool, matchFun: (filePath: string * narrows the list it appears in. This also means that negating a read is * ineffective while a non-negated write pattern still matches the file. */ -function isReadAllowedByPatterns( +async function isReadAllowedByPatterns( tool: ClineSayTool, cwd: string | undefined, state: Pick, -): boolean { - if (tool.tool !== "readFile") { +): Promise { + if (tool.tool !== "readFile" || (!state.allowedReadFiles?.length && !state.allowedWriteFiles?.length)) { return false } + // Task-local root only. Unknown roots keep lexical matching; never resolve + // individual file aliases here, since those can point outside the workspace. + const canonicalCwd = await resolveCanonicalWorkspaceRoot(cwd) return areAllNamedFilesMatched( tool, (filePath) => - isFileMatchedByPatterns({ filePath, cwd, patterns: state.allowedReadFiles }) || - isFileMatchedByPatterns({ filePath, cwd, patterns: state.allowedWriteFiles }), + isFileMatchedByPatterns({ filePath, cwd, canonicalCwd, patterns: state.allowedReadFiles }) || + isFileMatchedByPatterns({ filePath, cwd, canonicalCwd, patterns: state.allowedWriteFiles }), ) } @@ -392,7 +410,7 @@ export async function checkAutoApproval({ // `alwaysAllowReadOnly` permission. Such a pattern names its // location, including outside the workspace, so it also stands in for // `alwaysAllowReadOnlyOutsideWorkspace`. - const isAllowedReadFile = isReadAllowedByPatterns(tool, cwd, state) + const isAllowedReadFile = await isReadAllowedByPatterns(tool, cwd, state) const isReadAllowed = isAllowedReadFile || diff --git a/src/core/auto-approval/strategies/PosixWorkspaceRootStrategy.ts b/src/core/auto-approval/strategies/PosixWorkspaceRootStrategy.ts new file mode 100644 index 0000000000..93097b6896 --- /dev/null +++ b/src/core/auto-approval/strategies/PosixWorkspaceRootStrategy.ts @@ -0,0 +1,19 @@ +import path from "path" + +import type { WorkspaceRootStrategy } from "./WorkspaceRootStrategy" + +export class PosixWorkspaceRootStrategy implements WorkspaceRootStrategy { + toMatcherAbsolutePath(absolutePath: string): string { + return absolutePath + } + + normalizeAbsoluteRoot(root: string | undefined): string | undefined { + if (!root?.startsWith("/")) return undefined + + return path.posix.resolve(this.toMatcherAbsolutePath(root)).replace(/\/$/, "") + } + + pathsEqual(left: string, right: string): boolean { + return left === right + } +} diff --git a/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts b/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts new file mode 100644 index 0000000000..723c4d60f0 --- /dev/null +++ b/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts @@ -0,0 +1,41 @@ +import path from "path" + +import type { WorkspaceRootStrategy } from "./WorkspaceRootStrategy" + +/** Windows UNC roots have exactly two leading separators and nonempty server/share names. */ +export const WINDOWS_UNC_PREFIX = /^\/\/([^/]+)\/+([^/]+)/ + +export class WindowsWorkspaceRootStrategy implements WorkspaceRootStrategy { + /** Drop drive colons rejected by the Windows matcher while keeping UNC paths in their own namespace. */ + toMatcherAbsolutePath(absolutePath: string, workspaceVolume: string): string { + // Keep UNC paths independent of the current drive and distinct from drive letters. + const unc = WINDOWS_UNC_PREFIX.exec(absolutePath) + if (unc) return `/UNC/${unc[1]}/${unc[2]}${absolutePath.slice(unc[0].length)}` + + const drive = /^([a-zA-Z]):(?=\/)/.exec(absolutePath) + // Rooted paths belong to the workspace drive/share. Without a known volume, + // keep them in a separate namespace so /UNC/... cannot impersonate a share. + return drive + ? `/${drive[1]}${absolutePath.slice(drive[0].length)}` + : `${workspaceVolume || "/ROOT"}${absolutePath.replace(/^\/+/, "/")}` + } + + normalizeAbsoluteRoot(root: string | undefined): string | undefined { + const normalized = (root ?? "").replace(/\\/g, "/") + const drive = /^([a-zA-Z]):(?=\/)/.exec(normalized) + if (!normalized.startsWith("/") && !drive) return undefined + + // Resolve parents while drives and UNC shares are still filesystem roots. + // POSIX resolution after conversion could consume the drive or share name. + const absolutePath = path.win32.normalize(normalized).replace(/\\/g, "/") + return this.toMatcherAbsolutePath(absolutePath, "").replace(/\/$/, "") + } + + pathsEqual(left: string, right: string): boolean { + // Match literal roots with the same case rules as ignore. Whole-string + // lowercasing can expand Unicode characters and corrupt prefix offsets. + if (left.length !== right.length) return false + const literalRight = right.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") + return new RegExp(`^${literalRight}$`, "i").test(left) + } +} diff --git a/src/core/auto-approval/strategies/WorkspaceRootStrategy.ts b/src/core/auto-approval/strategies/WorkspaceRootStrategy.ts new file mode 100644 index 0000000000..d1bb91fecd --- /dev/null +++ b/src/core/auto-approval/strategies/WorkspaceRootStrategy.ts @@ -0,0 +1,13 @@ +import { PosixWorkspaceRootStrategy } from "./PosixWorkspaceRootStrategy" +import { WindowsWorkspaceRootStrategy } from "./WindowsWorkspaceRootStrategy" + +export abstract class WorkspaceRootStrategy { + static forPlatform(isWindows: boolean): WorkspaceRootStrategy { + return isWindows ? new WindowsWorkspaceRootStrategy() : new PosixWorkspaceRootStrategy() + } + + /** Convert a slash-normalized absolute path into the matcher's platform-specific namespace. */ + abstract toMatcherAbsolutePath(absolutePath: string, workspaceVolume: string): string + abstract normalizeAbsoluteRoot(root: string | undefined): string | undefined + abstract pathsEqual(left: string, right: string): boolean +}