fix(permissions): canonicalize Windows absolute rules in analyzer (#974)

Co-authored-by: Letta <noreply@letta.com>

src/permissions/analyzer.ts

@@ -2,7 +2,7 @@
 // Analyze tool executions and recommend appropriate permission rules
 
 import { homedir } from "node:os";
-import { dirname, resolve } from "node:path";
+import { dirname, relative, resolve, win32 } from "node:path";
 
 export interface ApprovalContext {
   // What rule should be saved if user clicks "approve always"
@@ -29,6 +29,70 @@ export interface ApprovalContext {
  */
 type ToolArgs = Record<string, unknown>;
 
+function normalizeOsPath(path: string): string {
+  return path.replace(/\\/g, "/");
+}
+
+function isWindowsPath(path: string): boolean {
+  return /^[a-zA-Z]:[\\/]/.test(path) || /^\\\\/.test(path);
+}
+
+function resolvePathForContext(basePath: string, targetPath: string): string {
+  const windows = isWindowsPath(basePath) || isWindowsPath(targetPath);
+  return windows
+    ? win32.resolve(basePath, targetPath)
+    : resolve(basePath, targetPath);
+}
+
+function relativePathForContext(basePath: string, targetPath: string): string {
+  const windows = isWindowsPath(basePath) || isWindowsPath(targetPath);
+  return windows
+    ? win32.relative(basePath, targetPath)
+    : relative(basePath, targetPath);
+}
+
+function isPathWithinDirectory(path: string, directory: string): boolean {
+  const windows = isWindowsPath(path) || isWindowsPath(directory);
+  const normalizedPath = normalizeOsPath(path);
+  const normalizedDirectory = normalizeOsPath(directory);
+
+  const relativePath = normalizeOsPath(
+    windows
+      ? win32.relative(
+          normalizedDirectory.toLowerCase(),
+          normalizedPath.toLowerCase(),
+        )
+      : relativePathForContext(normalizedDirectory, normalizedPath),
+  );
+
+  if (relativePath === "") {
+    return true;
+  }
+
+  return (
+    !relativePath.startsWith("../") &&
+    relativePath !== ".." &&
+    !relativePath.startsWith("/") &&
+    !/^[a-zA-Z]:\//.test(relativePath)
+  );
+}
+
+function dirnameForContext(path: string): string {
+  return isWindowsPath(path) ? win32.dirname(path) : dirname(path);
+}
+
+function formatAbsoluteRulePath(path: string): string {
+  const normalized = normalizeOsPath(path).replace(/\/+$/, "");
+  if (/^[a-zA-Z]:\//.test(normalized) || normalized.startsWith("//")) {
+    return normalized;
+  }
+  return `//${normalized.replace(/^\/+/, "")}`;
+}
+
+function formatDisplayPath(path: string): string {
+  return normalizeOsPath(path).replace(normalizeOsPath(homedir()), "~");
+}
+
 export function analyzeApprovalContext(
   toolName: string,
   toolArgs: ToolArgs,
@@ -97,15 +161,15 @@ function analyzeReadApproval(
   filePath: string,
   workingDir: string,
 ): ApprovalContext {
-  const absolutePath = resolve(workingDir, filePath);
+  const absolutePath = resolvePathForContext(workingDir, filePath);
 
   // If outside working directory, generalize to parent directory
-  if (!absolutePath.startsWith(workingDir)) {
-    const dirPath = dirname(absolutePath);
-    const displayPath = dirPath.replace(require("node:os").homedir(), "~");
+  if (!isPathWithinDirectory(absolutePath, workingDir)) {
+    const dirPath = dirnameForContext(absolutePath);
+    const displayPath = formatDisplayPath(dirPath);
 
     return {
-      recommendedRule: `Read(/${dirPath}/**)`,
+      recommendedRule: `Read(${formatAbsoluteRulePath(dirPath)}/**)`,
       ruleDescription: `reading from ${displayPath}/`,
       approveAlwaysText: `Yes, allow reading from ${displayPath}/ in this project`,
       defaultScope: "project",
@@ -115,9 +179,12 @@ function analyzeReadApproval(
   }
 
   // Inside working directory - use relative path
-  const relativePath = absolutePath.slice(workingDir.length + 1);
+  const relativePath = normalizeOsPath(
+    relativePathForContext(workingDir, absolutePath),
+  );
   const relativeDir = dirname(relativePath);
-  const pattern = relativeDir === "." ? "**" : `${relativeDir}/**`;
+  const pattern =
+    relativeDir === "." || relativeDir === "" ? "**" : `${relativeDir}/**`;
 
   return {
     recommendedRule: `Read(${pattern})`,
@@ -157,14 +224,14 @@ function analyzeEditApproval(
 ): ApprovalContext {
   // Edit is safer than Write (file must exist)
   // Can offer project-level for specific directories
-  const absolutePath = resolve(workingDir, filePath);
-  const dirPath = dirname(absolutePath);
+  const absolutePath = resolvePathForContext(workingDir, filePath);
+  const dirPath = dirnameForContext(absolutePath);
 
-  // If outside working directory, use absolute path with // prefix
-  if (!dirPath.startsWith(workingDir)) {
-    const displayPath = dirPath.replace(require("node:os").homedir(), "~");
+  // If outside working directory, use canonical absolute path pattern
+  if (!isPathWithinDirectory(dirPath, workingDir)) {
+    const displayPath = formatDisplayPath(dirPath);
     return {
-      recommendedRule: `Edit(/${dirPath}/**)`,
+      recommendedRule: `Edit(${formatAbsoluteRulePath(dirPath)}/**)`,
       ruleDescription: `editing files in ${displayPath}/`,
       approveAlwaysText: `Yes, allow editing files in ${displayPath}/ in this project`,
       defaultScope: "project",
@@ -174,8 +241,13 @@ function analyzeEditApproval(
   }
 
   // Inside working directory, use relative path
-  const relativeDirPath = dirPath.slice(workingDir.length + 1);
-  const pattern = relativeDirPath === "" ? "**" : `${relativeDirPath}/**`;
+  const relativeDirPath = normalizeOsPath(
+    relativePathForContext(workingDir, dirPath),
+  );
+  const pattern =
+    relativeDirPath === "" || relativeDirPath === "."
+      ? "**"
+      : `${relativeDirPath}/**`;
 
   return {
     recommendedRule: `Edit(${pattern})`,
@@ -651,13 +723,13 @@ function analyzeSearchApproval(
   searchPath: string,
   workingDir: string,
 ): ApprovalContext {
-  const absolutePath = resolve(workingDir, searchPath);
+  const absolutePath = resolvePathForContext(workingDir, searchPath);
 
-  if (!absolutePath.startsWith(workingDir)) {
-    const displayPath = absolutePath.replace(require("node:os").homedir(), "~");
+  if (!isPathWithinDirectory(absolutePath, workingDir)) {
+    const displayPath = formatDisplayPath(absolutePath);
 
     return {
-      recommendedRule: `${toolName}(/${absolutePath}/**)`,
+      recommendedRule: `${toolName}(${formatAbsoluteRulePath(absolutePath)}/**)`,
       ruleDescription: `searching in ${displayPath}/`,
       approveAlwaysText: `Yes, allow searching in ${displayPath}/ in this project`,
       defaultScope: "project",

src/tests/permissions-analyzer.test.ts

@@ -417,6 +417,60 @@ test("Grep outside working directory suggests directory pattern", () => {
   expect(context.approveAlwaysText).toContain("/Users/test/docs/");
 });
 
+test("Read outside Windows working directory emits canonical Windows absolute rule", () => {
+  const context = analyzeApprovalContext(
+    "Read",
+    { file_path: "C:\\Users\\Test\\docs\\api.md" },
+    "C:\\Users\\Test\\project",
+  );
+
+  expect(context.recommendedRule).toBe("Read(C:/Users/Test/docs/**)");
+  expect(context.approveAlwaysText).toContain("C:/Users/Test/docs/");
+});
+
+test("Edit outside Windows working directory emits canonical Windows absolute rule", () => {
+  const context = analyzeApprovalContext(
+    "Edit",
+    { file_path: "C:\\Users\\Test\\docs\\note.md" },
+    "C:\\Users\\Test\\project",
+  );
+
+  expect(context.recommendedRule).toBe("Edit(C:/Users/Test/docs/**)");
+  expect(context.approveAlwaysText).toContain("C:/Users/Test/docs/");
+});
+
+test("Read inside Windows working directory handles drive-letter case differences", () => {
+  const context = analyzeApprovalContext(
+    "Read",
+    { file_path: "c:\\users\\test\\project\\src\\index.ts" },
+    "C:\\Users\\Test\\project",
+  );
+
+  expect(context.recommendedRule).toBe("Read(src/**)");
+});
+
+test("Glob outside Windows working directory emits canonical Windows absolute rule", () => {
+  const context = analyzeApprovalContext(
+    "Glob",
+    { path: "C:\\Users\\Test\\docs" },
+    "C:\\Users\\Test\\project",
+  );
+
+  expect(context.recommendedRule).toBe("Glob(C:/Users/Test/docs/**)");
+  expect(context.approveAlwaysText).toContain("C:/Users/Test/docs/");
+});
+
+test("Grep outside Windows working directory emits canonical Windows absolute rule", () => {
+  const context = analyzeApprovalContext(
+    "Grep",
+    { path: "C:\\Users\\Test\\docs" },
+    "C:\\Users\\Test\\project",
+  );
+
+  expect(context.recommendedRule).toBe("Grep(C:/Users/Test/docs/**)");
+  expect(context.approveAlwaysText).toContain("C:/Users/Test/docs/");
+});
+
 // ============================================================================
 // WebFetch Analysis Tests
 // ============================================================================