From cec94cd9a5db4c1c57628b016e6875df18ec00bc Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Wed, 14 Oct 2026 15:13:39 +0300 Subject: [PATCH 1/5] fix(auto-approval): match symlinked workspace roots Recognize lexical and verified canonical workspace-root spellings without resolving individual file aliases. Preserve ordered grants, exclusions, literal root names, and lexical-parent rules across POSIX, Windows drives, and UNC shares. Reject ambiguous Windows drive-relative inputs without widening read or write permissions. Centralize platform selection in WorkspaceRootStrategy.forPlatform and cover literal UNC share names with regression tests. --- .../WorkspaceRootCanonicalizer.ts | 44 ++++ .../WorkspaceRootCanonicalizer.spec.ts | 171 ++++++++++++++ .../__tests__/WorkspaceRootStrategy.spec.ts | 57 +++++ .../__tests__/filePatterns.spec.ts | 2 +- .../__tests__/posixPaths.spec.ts | 31 +++ .../windowsDriveRelativeApproval.spec.ts | 56 +++++ .../__tests__/windowsPaths.spec.ts | 214 ++++++++++++++++++ .../workspaceRootEquivalence.spec.ts | 208 +++++++++++++++++ .../workspaceRootSafety.integration.spec.ts | 41 ++++ src/core/auto-approval/filePatterns.ts | 158 +++++++------ src/core/auto-approval/index.ts | 27 ++- .../strategies/PosixWorkspaceRootStrategy.ts | 19 ++ .../WindowsWorkspaceRootStrategy.ts | 32 +++ .../strategies/WorkspaceRootStrategy.ts | 13 ++ 14 files changed, 1001 insertions(+), 72 deletions(-) create mode 100644 src/core/auto-approval/WorkspaceRootCanonicalizer.ts create mode 100644 src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts create mode 100644 src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts create mode 100644 src/core/auto-approval/__tests__/windowsDriveRelativeApproval.spec.ts create mode 100644 src/core/auto-approval/__tests__/workspaceRootEquivalence.spec.ts create mode 100644 src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts create mode 100644 src/core/auto-approval/strategies/PosixWorkspaceRootStrategy.ts create mode 100644 src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts create mode 100644 src/core/auto-approval/strategies/WorkspaceRootStrategy.ts diff --git a/src/core/auto-approval/WorkspaceRootCanonicalizer.ts b/src/core/auto-approval/WorkspaceRootCanonicalizer.ts new file mode 100644 index 0000000000..d78579a1f9 --- /dev/null +++ b/src/core/auto-approval/WorkspaceRootCanonicalizer.ts @@ -0,0 +1,44 @@ +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 comparableValue = this.strategy.comparisonKey(value) + const comparableRoot = this.strategy.comparisonKey(root) + return comparableValue === comparableRoot || comparableValue.startsWith(`${comparableRoot}/`) + ? `${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..8a46b4b6f4 --- /dev/null +++ b/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts @@ -0,0 +1,171 @@ +import { PosixWorkspaceRootStrategy } from "../strategies/PosixWorkspaceRootStrategy" +import { WindowsWorkspaceRootStrategy } from "../strategies/WindowsWorkspaceRootStrategy" +import { WorkspaceRootStrategy } from "../strategies/WorkspaceRootStrategy" +import { WorkspaceRootCanonicalizer } from "../WorkspaceRootCanonicalizer" + +it("normalizes roots once in the factory before any path comparisons", () => { + const normalizeAbsoluteRoot = vi + .fn() + .mockReturnValueOnce("/aliases/project") + .mockReturnValueOnce("/storage/project") + const comparisonKey = vi.fn((value: string) => value) + const strategy: WorkspaceRootStrategy = { + normalizeAbsoluteRoot, + comparisonKey, + toMatcherAbsolutePath: vi.fn((value: string) => value), + } + + 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(comparisonKey).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("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..db0c8080cc --- /dev/null +++ b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts @@ -0,0 +1,57 @@ +import { isFileMatchedByPatterns } from "../filePatterns" +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.comparisonKey("/tmp/NOTES.md")).toBe("/tmp/NOTES.md") + }) + + 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.comparisonKey("/D/Tmp/NOTES.md")).toBe("/d/tmp/notes.md") + }) + + it("uses one selected strategy for candidates, every pattern, and both workspace roots", () => { + const toMatcherAbsolutePath = vi.fn((value: string) => value) + const normalizeAbsoluteRoot = vi + .fn() + .mockReturnValueOnce("/aliases/project") + .mockReturnValueOnce("/storage/project") + const comparisonKey = vi.fn((value: string) => value) + const strategy: WorkspaceRootStrategy = { toMatcherAbsolutePath, normalizeAbsoluteRoot, comparisonKey } + 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(comparisonKey).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..fc94221b03 100644 --- a/src/core/auto-approval/__tests__/windowsPaths.spec.ts +++ b/src/core/auto-approval/__tests__/windowsPaths.spec.ts @@ -30,6 +30,190 @@ describe("matching on Windows", () => { const matches = (filePath: string, patterns: string[], cwd: string | undefined = WINDOWS_CWD) => matcher().isFileMatchedByPatterns({ filePath, cwd, patterns, homeDir: WINDOWS_HOME }) + 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 +283,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__/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..4d0643d336 --- /dev/null +++ b/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts @@ -0,0 +1,41 @@ +import * as fs from "fs/promises" +import os from "os" +import path from "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 }) + } +}) diff --git a/src/core/auto-approval/filePatterns.ts b/src/core/auto-approval/filePatterns.ts index 923d29815d..075a61d03c 100644 --- a/src/core/auto-approval/filePatterns.ts +++ b/src/core/auto-approval/filePatterns.ts @@ -3,6 +3,9 @@ import path from "path" import ignore from "ignore" +import { WorkspaceRootCanonicalizer } from "./WorkspaceRootCanonicalizer" +import { WorkspaceRootStrategy } from "./strategies/WorkspaceRootStrategy" + /** * Matching of file paths against user-configured file patterns. * @@ -72,7 +75,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 +158,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 = /^\/\/([^/]+)\/([^/]+)/.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 +197,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 +257,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 +271,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 +286,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 +306,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 +328,23 @@ export function toMatcherPattern( } if (isAbsolutePath(normalized, isWindows)) { - return `${negation}${toPosixAbsolutePath(normalized, workspaceDrive, isWindows)}` + return `${negation}${strategy.toMatcherAbsolutePath(normalized, 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 +362,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 +372,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 +424,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 cwd, used only by read approval. */ + canonicalCwd?: string patterns?: string[] isWindows?: boolean homeDir?: string @@ -425,14 +441,17 @@ 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 workspaceRoot = WorkspaceRootCanonicalizer.forStrategy(cwd, canonicalCwd, strategy) + const candidates = workspaceRoot.equivalentPaths(`/${candidate}`).map(toRootRelativePath) const matcherPatterns = patterns - .map((pattern) => toMatcherPattern(pattern, cwd, isWindows, homeDir)) + .map((pattern) => toMatcherPatternWithStrategy(pattern, cwd, isWindows, homeDir, strategy)) .filter((pattern): pattern is string => !!pattern) if (!matcherPatterns.length) { @@ -440,9 +459,18 @@ export function isFileMatchedByPatterns({ } try { - return ignore({ ignoreCase: isWindows }) - .add([...matcherPatterns, MATCH_DIRECTORIES_ONLY_PATTERN]) - .ignores(candidate) + let matched = false + for (const pattern of matcherPatterns) { + const negated = pattern.startsWith("!") + const matcher = ignore({ ignoreCase: isWindows }).add([ + negated ? pattern.slice(1) : pattern, + MATCH_DIRECTORIES_ONLY_PATTERN, + ]) + // 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 (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..9352eb4281 100644 --- a/src/core/auto-approval/index.ts +++ b/src/core/auto-approval/index.ts @@ -9,6 +9,7 @@ import { } from "@roo-code/types" import type { DcgDecision } from "../../services/destructive-command-guard/runner" +import { realpath } from "fs/promises" import { ClineAskResponse } from "../../shared/WebviewMessage" @@ -87,6 +88,17 @@ 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 { + return await realpath(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 +117,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 +407,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..799f3e75c8 --- /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(/\/$/, "") + } + + comparisonKey(value: string): string { + return value + } +} diff --git a/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts b/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts new file mode 100644 index 0000000000..06d10e9eab --- /dev/null +++ b/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts @@ -0,0 +1,32 @@ +import path from "path" + +import type { WorkspaceRootStrategy } from "./WorkspaceRootStrategy" + +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. + if (absolutePath.startsWith("//")) return `/UNC/${absolutePath.replace(/^\/+/, "")}` + + 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}` + } + + normalizeAbsoluteRoot(root: string | undefined): string | undefined { + const normalized = (root ?? "").replace(/\\/g, "/") + const drive = /^([a-zA-Z]):(?=\/)/.exec(normalized) + if (!normalized.startsWith("/") && !drive) return undefined + + // Match the allowlist representation: a Windows drive is a POSIX path segment. + const absolutePath = this.toMatcherAbsolutePath(normalized, "") + return path.posix.resolve(absolutePath).replace(/\/$/, "") + } + + comparisonKey(value: string): string { + return value.toLowerCase() + } +} diff --git a/src/core/auto-approval/strategies/WorkspaceRootStrategy.ts b/src/core/auto-approval/strategies/WorkspaceRootStrategy.ts new file mode 100644 index 0000000000..301a3f0768 --- /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 comparisonKey(value: string): string +} From d191fea50d63e0fa7035f2bca098a51a58cc7af5 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sat, 14 Nov 2026 15:15:02 +0300 Subject: [PATCH 2/5] fix(auto-approval): preserve Windows rooted exclusions Preserve cross-drive exclusions when the workspace has no drive. Escape inherited UNC volumes without changing user-authored globs. Add matcher and approval regressions for reads, writes, aliases, and rule order. Part of #1948. --- .../__tests__/windowsPaths.spec.ts | 120 ++++++++++++++++ .../__tests__/windowsRootedApproval.spec.ts | 132 ++++++++++++++++++ src/core/auto-approval/filePatterns.ts | 44 ++++-- 3 files changed, 282 insertions(+), 14 deletions(-) create mode 100644 src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts diff --git a/src/core/auto-approval/__tests__/windowsPaths.spec.ts b/src/core/auto-approval/__tests__/windowsPaths.spec.ts index fc94221b03..9910011245 100644 --- a/src/core/auto-approval/__tests__/windowsPaths.spec.ts +++ b/src/core/auto-approval/__tests__/windowsPaths.spec.ts @@ -30,6 +30,126 @@ 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([undefined, "/ws", "\\ws", "//server/share/ws", "\\\\server\\share\\ws"])( + "preserves cross-volume wildcard exclusions and their order with workspace %s", + (cwd) => { + const input = { filePath: "C:/x/.env", cwd, homeDir: WINDOWS_HOME } + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["C:/**", "!/**/.env"] })).toBe(false) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["!/**/.env", "C:/**"] })).toBe(true) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["C:/**", "!/**/.env", "C:/x/.env"] })).toBe( + true, + ) + expect( + matcher().isFileMatchedByPatterns({ + ...input, + filePath: "C:/x/notes.md", + patterns: ["C:/**", "!/**/.env"], + }), + ).toBe(true) + expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["/**"] })).toBe(false) + }, + ) + + it.each([ + { + scenario: "a known workspace drive keeps the exclusion on that drive", + cwd: "D:/ws", + filePath: "C:/x/.env", + patterns: ["C:/**", "!/**/.env"], + allowed: true, + }, + { + scenario: "a known workspace drive still excludes its own files", + cwd: "D:/ws", + filePath: "D:/x/.env", + patterns: ["D:/**", "!/**/.env"], + 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("checks cross-volume exclusions against an equivalent drive path for a UNC candidate", () => { + expect( + matcher().isFileMatchedByPatterns({ + cwd: "//server/share/Project", + canonicalCwd: "C:/Storage/Project", + filePath: "//server/share/Project/docs/.env", + patterns: ["docs/**", "!/C/Storage/**/.env"], + }), + ).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/**" 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..2d00c1d0f0 --- /dev/null +++ b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts @@ -0,0 +1,132 @@ +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 + +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([undefined, "/ws", "//server/share/ws"])( + "applies ordered global exclusions when cwd is %s", + async (cwd) => { + const patterns = ["C:/**", "!/**/.env"] + 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]: [...patterns, "C:/x/.env"] })).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("applies rooted UNC rules to reads through the verified drive alias", async () => { + const cwd = "//server/sh[a]re/project" + vi.mocked(fs.realpath).mockResolvedValue("D:/storage/project") + const patterns = ["/project/docs/**", "!/**/.env"] + 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/filePatterns.ts b/src/core/auto-approval/filePatterns.ts index 075a61d03c..27af8bbaa4 100644 --- a/src/core/auto-approval/filePatterns.ts +++ b/src/core/auto-approval/filePatterns.ts @@ -328,7 +328,9 @@ function toMatcherPatternWithStrategy( } if (isAbsolutePath(normalized, isWindows)) { - return `${negation}${strategy.toMatcherAbsolutePath(normalized, workspaceVolume)}` + // 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 || isDriveRelativePath(workspacePosix, isWindows)) { @@ -450,25 +452,39 @@ export function isFileMatchedByPatterns({ const workspaceRoot = WorkspaceRootCanonicalizer.forStrategy(cwd, canonicalCwd, strategy) const candidates = workspaceRoot.equivalentPaths(`/${candidate}`).map(toRootRelativePath) - const matcherPatterns = patterns - .map((pattern) => toMatcherPatternWithStrategy(pattern, cwd, isWindows, homeDir, strategy)) - .filter((pattern): pattern is string => !!pattern) - - if (!matcherPatterns.length) { - return false - } + // 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 { let matched = false - for (const pattern of matcherPatterns) { + for (const rawPattern of patterns) { + const pattern = toMatcherPatternWithStrategy(rawPattern, cwd, isWindows, homeDir, strategy) + if (!pattern) continue + const negated = pattern.startsWith("!") - const matcher = ignore({ ignoreCase: isWindows }).add([ - negated ? pattern.slice(1) : pattern, - MATCH_DIRECTORIES_ONLY_PATTERN, - ]) + const alternatives = [{ pattern: negated ? pattern.slice(1) : pattern, candidates }] + if (negated && driveCandidates.length) { + const rootedExclusion = pathsepsToPosix(rawPattern.slice(1), isWindows) + if (rootedExclusion.startsWith("/") && !rootedExclusion.startsWith("//")) { + alternatives.push({ pattern: rootedExclusion, 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 (candidates.some((value) => matcher.ignores(value))) matched = !negated + 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) { From cd46b0fb2b168cb57fbb451177d29648648cd564 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sat, 14 Nov 2026 15:24:22 +0300 Subject: [PATCH 3/5] fix(auto-approval): preserve Windows volume boundaries Distinguish UNC prefixes from rooted paths with extra separators. Normalize workspace roots with Windows path rules before conversion so parent segments cannot consume drive or share names. Add matcher and approval regressions for both separator spellings. Part of #1948. --- .../__tests__/windowsPaths.spec.ts | 80 +++++++++++++++++++ .../__tests__/windowsRootedApproval.spec.ts | 38 +++++++++ src/core/auto-approval/filePatterns.ts | 3 +- .../WindowsWorkspaceRootStrategy.ts | 15 ++-- 4 files changed, 130 insertions(+), 6 deletions(-) diff --git a/src/core/auto-approval/__tests__/windowsPaths.spec.ts b/src/core/auto-approval/__tests__/windowsPaths.spec.ts index 9910011245..a01528a74d 100644 --- a/src/core/auto-approval/__tests__/windowsPaths.spec.ts +++ b/src/core/auto-approval/__tests__/windowsPaths.spec.ts @@ -30,6 +30,86 @@ 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"])( "preserves cross-volume wildcard exclusions and their order with workspace %s", (cwd) => { diff --git a/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts index 2d00c1d0f0..35d66f2ce8 100644 --- a/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts +++ b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts @@ -43,6 +43,24 @@ describe("Windows rooted approval rules", () => { 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"])( "applies ordered global exclusions when cwd is %s", async (cwd) => { @@ -82,6 +100,26 @@ describe("Windows rooted approval rules", () => { ) }) + 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("applies rooted UNC rules to reads through the verified drive alias", async () => { const cwd = "//server/sh[a]re/project" vi.mocked(fs.realpath).mockResolvedValue("D:/storage/project") diff --git a/src/core/auto-approval/filePatterns.ts b/src/core/auto-approval/filePatterns.ts index 27af8bbaa4..6272e84b6f 100644 --- a/src/core/auto-approval/filePatterns.ts +++ b/src/core/auto-approval/filePatterns.ts @@ -5,6 +5,7 @@ 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. @@ -171,7 +172,7 @@ const DRIVE_PREFIX = /^([a-zA-Z]):(?=\/)/ */ function volumeSegmentOf(absolutePath: string, isWindows: boolean): string { if (isWindows) { - const unc = /^\/\/([^/]+)\/([^/]+)/.exec(absolutePath) + const unc = WINDOWS_UNC_PREFIX.exec(absolutePath) if (unc) return `/UNC/${unc[1]}/${unc[2]}` } const drive = isWindows ? DRIVE_PREFIX.exec(absolutePath) : null diff --git a/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts b/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts index 06d10e9eab..750364e9ff 100644 --- a/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts +++ b/src/core/auto-approval/strategies/WindowsWorkspaceRootStrategy.ts @@ -2,18 +2,22 @@ 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. - if (absolutePath.startsWith("//")) return `/UNC/${absolutePath.replace(/^\/+/, "")}` + 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}` + : `${workspaceVolume || "/ROOT"}${absolutePath.replace(/^\/+/, "/")}` } normalizeAbsoluteRoot(root: string | undefined): string | undefined { @@ -21,9 +25,10 @@ export class WindowsWorkspaceRootStrategy implements WorkspaceRootStrategy { const drive = /^([a-zA-Z]):(?=\/)/.exec(normalized) if (!normalized.startsWith("/") && !drive) return undefined - // Match the allowlist representation: a Windows drive is a POSIX path segment. - const absolutePath = this.toMatcherAbsolutePath(normalized, "") - return path.posix.resolve(absolutePath).replace(/\/$/, "") + // 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(/\/$/, "") } comparisonKey(value: string): string { From b8d496107c14dde793cb2ccce386603a89a5398e Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sun, 15 Nov 2026 12:10:28 +0300 Subject: [PATCH 4/5] fix(auto-approval): verify workspace root identity Use one UNC classification for rooted exclusions. Verify the lexically resolved workspace before realpath. Compare literal Windows roots without Unicode expansions or changed prefix offsets. Add permission regressions and real-filesystem coverage. Part of #1948. --- .../WorkspaceRootCanonicalizer.ts | 5 +- .../WorkspaceRootCanonicalizer.spec.ts | 39 +++++- .../__tests__/WorkspaceRootStrategy.spec.ts | 29 +++- .../__tests__/windowsPaths.spec.ts | 95 +++++++------ .../__tests__/windowsRootedApproval.spec.ts | 56 ++++++-- .../__tests__/windowsUnicodeRoots.spec.ts | 130 ++++++++++++++++++ .../workspaceRootSafety.integration.spec.ts | 40 ++++++ src/core/auto-approval/filePatterns.ts | 9 +- src/core/auto-approval/index.ts | 5 +- .../strategies/PosixWorkspaceRootStrategy.ts | 4 +- .../WindowsWorkspaceRootStrategy.ts | 8 +- .../strategies/WorkspaceRootStrategy.ts | 2 +- 12 files changed, 350 insertions(+), 72 deletions(-) create mode 100644 src/core/auto-approval/__tests__/windowsUnicodeRoots.spec.ts diff --git a/src/core/auto-approval/WorkspaceRootCanonicalizer.ts b/src/core/auto-approval/WorkspaceRootCanonicalizer.ts index d78579a1f9..c406537be9 100644 --- a/src/core/auto-approval/WorkspaceRootCanonicalizer.ts +++ b/src/core/auto-approval/WorkspaceRootCanonicalizer.ts @@ -35,9 +35,8 @@ export class WorkspaceRootCanonicalizer { } private replaceWorkspaceRootPrefix(value: string, root: string, canonicalRoot: string): string { - const comparableValue = this.strategy.comparisonKey(value) - const comparableRoot = this.strategy.comparisonKey(root) - return comparableValue === comparableRoot || comparableValue.startsWith(`${comparableRoot}/`) + 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 index 8a46b4b6f4..2dc218b3f8 100644 --- a/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts +++ b/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts @@ -8,10 +8,10 @@ it("normalizes roots once in the factory before any path comparisons", () => { .fn() .mockReturnValueOnce("/aliases/project") .mockReturnValueOnce("/storage/project") - const comparisonKey = vi.fn((value: string) => value) + const pathsEqual = vi.fn((left: string, right: string) => left === right) const strategy: WorkspaceRootStrategy = { normalizeAbsoluteRoot, - comparisonKey, + pathsEqual, toMatcherAbsolutePath: vi.fn((value: string) => value), } @@ -20,7 +20,7 @@ it("normalizes roots once in the factory before any path comparisons", () => { expect(normalizeAbsoluteRoot).toHaveBeenCalledTimes(2) expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(1, "/aliases/project/") expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(2, "/storage/project/") - expect(comparisonKey).not.toHaveBeenCalled() + 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") @@ -114,6 +114,39 @@ describe("POSIX workspace-root strategy", () => { }) 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", diff --git a/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts index db0c8080cc..87a62dd848 100644 --- a/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts +++ b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts @@ -10,7 +10,8 @@ describe("WorkspaceRootStrategy.forPlatform", () => { 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.comparisonKey("/tmp/NOTES.md")).toBe("/tmp/NOTES.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", () => { @@ -20,7 +21,25 @@ describe("WorkspaceRootStrategy.forPlatform", () => { 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.comparisonKey("/D/Tmp/NOTES.md")).toBe("/d/tmp/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", () => { @@ -29,8 +48,8 @@ describe("WorkspaceRootStrategy.forPlatform", () => { .fn() .mockReturnValueOnce("/aliases/project") .mockReturnValueOnce("/storage/project") - const comparisonKey = vi.fn((value: string) => value) - const strategy: WorkspaceRootStrategy = { toMatcherAbsolutePath, normalizeAbsoluteRoot, comparisonKey } + const pathsEqual = vi.fn((left: string, right: string) => left === right) + const strategy: WorkspaceRootStrategy = { toMatcherAbsolutePath, normalizeAbsoluteRoot, pathsEqual } const selectStrategy = vi.spyOn(WorkspaceRootStrategy, "forPlatform").mockReturnValue(strategy) expect( @@ -52,6 +71,6 @@ describe("WorkspaceRootStrategy.forPlatform", () => { expect(normalizeAbsoluteRoot).toHaveBeenCalledTimes(2) expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(1, "/aliases/project") expect(normalizeAbsoluteRoot).toHaveBeenNthCalledWith(2, "/storage/project") - expect(comparisonKey).toHaveBeenCalled() + expect(pathsEqual).toHaveBeenCalled() }) }) diff --git a/src/core/auto-approval/__tests__/windowsPaths.spec.ts b/src/core/auto-approval/__tests__/windowsPaths.spec.ts index a01528a74d..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") @@ -110,39 +119,44 @@ describe("matching on Windows", () => { }, ) - it.each([undefined, "/ws", "\\ws", "//server/share/ws", "\\\\server\\share\\ws"])( - "preserves cross-volume wildcard exclusions and their order with workspace %s", - (cwd) => { - const input = { filePath: "C:/x/.env", cwd, homeDir: WINDOWS_HOME } - expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["C:/**", "!/**/.env"] })).toBe(false) - expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["!/**/.env", "C:/**"] })).toBe(true) - expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["C:/**", "!/**/.env", "C:/x/.env"] })).toBe( - true, - ) - expect( - matcher().isFileMatchedByPatterns({ - ...input, - filePath: "C:/x/notes.md", - patterns: ["C:/**", "!/**/.env"], - }), - ).toBe(true) - expect(matcher().isFileMatchedByPatterns({ ...input, patterns: ["/**"] })).toBe(false) - }, - ) + 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: "a known workspace drive keeps the exclusion on that drive", - cwd: "D:/ws", + scenario: "an incomplete UNC exclusion retains rooted semantics", + cwd: undefined, filePath: "C:/x/.env", - patterns: ["C:/**", "!/**/.env"], - allowed: true, - }, - { - scenario: "a known workspace drive still excludes its own files", - cwd: "D:/ws", - filePath: "D:/x/.env", - patterns: ["D:/**", "!/**/.env"], + patterns: ["C:/**", "!//**"], allowed: false, }, { @@ -198,16 +212,19 @@ describe("matching on Windows", () => { expect(matcher().isFileMatchedByPatterns({ cwd, filePath, patterns })).toBe(allowed) }) - it("checks cross-volume exclusions against an equivalent drive path for a UNC candidate", () => { - expect( - matcher().isFileMatchedByPatterns({ - cwd: "//server/share/Project", - canonicalCwd: "C:/Storage/Project", - filePath: "//server/share/Project/docs/.env", - patterns: ["docs/**", "!/C/Storage/**/.env"], - }), - ).toBe(false) - }) + 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", diff --git a/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts index 35d66f2ce8..c74fbf9493 100644 --- a/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts +++ b/src/core/auto-approval/__tests__/windowsRootedApproval.spec.ts @@ -14,6 +14,15 @@ const approvalRoutes = [ { 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, @@ -61,17 +70,38 @@ describe("Windows rooted approval rules", () => { }, ) - it.each([undefined, "/ws", "//server/share/ws"])( - "applies ordered global exclusions when cwd is %s", - async (cwd) => { - const patterns = ["C:/**", "!/**/.env"] - 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]: [...patterns, "C:/x/.env"] })).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" }) }, ) @@ -120,10 +150,10 @@ describe("Windows rooted approval rules", () => { }, ) - it("applies rooted UNC rules to reads through the verified drive alias", async () => { + 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/**", "!/**/.env"] + const patterns = ["/project/docs/**", exclusion.replace("**", "D/storage/**")] expect( await approval("readFile", "D:/storage/project/docs/notes.md", cwd, { allowedReadFiles: patterns }), ).toEqual({ 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__/workspaceRootSafety.integration.spec.ts b/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts index 4d0643d336..ac48c9373e 100644 --- a/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts +++ b/src/core/auto-approval/__tests__/workspaceRootSafety.integration.spec.ts @@ -1,6 +1,7 @@ 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" @@ -39,3 +40,42 @@ it("keeps permissions and wildcard exclusions scoped to a real symlinked workspa 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 6272e84b6f..1a9a857f80 100644 --- a/src/core/auto-approval/filePatterns.ts +++ b/src/core/auto-approval/filePatterns.ts @@ -434,7 +434,7 @@ export function isFileMatchedByPatterns({ }: { filePath?: string cwd?: string - /** Verified realpath of cwd, used only by read approval. */ + /** Verified realpath of the lexically resolved cwd, used only by read approval. */ canonicalCwd?: string patterns?: string[] isWindows?: boolean @@ -472,8 +472,11 @@ export function isFileMatchedByPatterns({ const alternatives = [{ pattern: negated ? pattern.slice(1) : pattern, candidates }] if (negated && driveCandidates.length) { const rootedExclusion = pathsepsToPosix(rawPattern.slice(1), isWindows) - if (rootedExclusion.startsWith("/") && !rootedExclusion.startsWith("//")) { - alternatives.push({ pattern: rootedExclusion, candidates: driveCandidates }) + 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. diff --git a/src/core/auto-approval/index.ts b/src/core/auto-approval/index.ts index 9352eb4281..bc4aa36917 100644 --- a/src/core/auto-approval/index.ts +++ b/src/core/auto-approval/index.ts @@ -10,6 +10,7 @@ import { import type { DcgDecision } from "../../services/destructive-command-guard/runner" import { realpath } from "fs/promises" +import path from "path" import { ClineAskResponse } from "../../shared/WebviewMessage" @@ -92,7 +93,9 @@ async function resolveCanonicalWorkspaceRoot(cwd: string | undefined): Promise Date: Sun, 15 Nov 2026 12:33:15 +0300 Subject: [PATCH 5/5] test(auto-approval): exercise real workspace strategies --- .../__tests__/WorkspaceRootCanonicalizer.spec.ts | 15 +++++---------- .../__tests__/WorkspaceRootStrategy.spec.ts | 12 +++++------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts b/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts index 2dc218b3f8..8be4a28813 100644 --- a/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts +++ b/src/core/auto-approval/__tests__/WorkspaceRootCanonicalizer.spec.ts @@ -3,17 +3,12 @@ import { WindowsWorkspaceRootStrategy } from "../strategies/WindowsWorkspaceRoot import { WorkspaceRootStrategy } from "../strategies/WorkspaceRootStrategy" import { WorkspaceRootCanonicalizer } from "../WorkspaceRootCanonicalizer" +afterEach(() => vi.restoreAllMocks()) + it("normalizes roots once in the factory before any path comparisons", () => { - const normalizeAbsoluteRoot = vi - .fn() - .mockReturnValueOnce("/aliases/project") - .mockReturnValueOnce("/storage/project") - const pathsEqual = vi.fn((left: string, right: string) => left === right) - const strategy: WorkspaceRootStrategy = { - normalizeAbsoluteRoot, - pathsEqual, - toMatcherAbsolutePath: vi.fn((value: string) => value), - } + 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) diff --git a/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts index 87a62dd848..d4f0d74f6c 100644 --- a/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts +++ b/src/core/auto-approval/__tests__/WorkspaceRootStrategy.spec.ts @@ -1,4 +1,5 @@ import { isFileMatchedByPatterns } from "../filePatterns" +import { PosixWorkspaceRootStrategy } from "../strategies/PosixWorkspaceRootStrategy" import { WorkspaceRootStrategy } from "../strategies/WorkspaceRootStrategy" describe("WorkspaceRootStrategy.forPlatform", () => { @@ -43,13 +44,10 @@ describe("WorkspaceRootStrategy.forPlatform", () => { }) it("uses one selected strategy for candidates, every pattern, and both workspace roots", () => { - const toMatcherAbsolutePath = vi.fn((value: string) => value) - const normalizeAbsoluteRoot = vi - .fn() - .mockReturnValueOnce("/aliases/project") - .mockReturnValueOnce("/storage/project") - const pathsEqual = vi.fn((left: string, right: string) => left === right) - const strategy: WorkspaceRootStrategy = { toMatcherAbsolutePath, normalizeAbsoluteRoot, pathsEqual } + 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(