fix(read-file): bind approved reads to verified targets

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
+	}
+}

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<WorkspaceRootStrategy["normalizeAbsoluteRoot"]>()
+		.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`)
+})

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<WorkspaceRootStrategy["normalizeAbsoluteRoot"]>()
+			.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()
+	})
+})

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")
 		})

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)

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<typeof import("fs/promises")>()
+	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" })
+		},
+	)
+})