Merge pull request #69 from y574444354/test-pattern-with-root

test: add unit tests for patternWithRoot with Windows path improvements

src/utils/permissions/__tests__/filesystem.test.ts

@@ -0,0 +1,99 @@
+import { describe, expect, mock, test } from 'bun:test'
+import { logMock } from '../../../../tests/mocks/log'
+
+mock.module('src/utils/log.ts', logMock)
+
+const { patternWithRoot } = await import('../filesystem')
+
+describe('patternWithRoot', () => {
+  // ── Linux / macOS ────────────────────────────────────────────
+
+  describe('relative paths', () => {
+    test('plain relative path → root=null', () => {
+      const r = patternWithRoot('src/config.json', 'localSettings')
+      expect(r.root).toBeNull()
+      expect(r.relativePattern).toBe('src/config.json')
+    })
+
+    test('strips ./ prefix → root=null', () => {
+      const r = patternWithRoot('./config.json', 'localSettings')
+      expect(r.root).toBeNull()
+      expect(r.relativePattern).toBe('config.json')
+    })
+
+    test('glob pattern → root=null', () => {
+      const r = patternWithRoot('src/**/*.ts', 'localSettings')
+      expect(r.root).toBeNull()
+      expect(r.relativePattern).toBe('src/**/*.ts')
+    })
+
+    test('leading dotfile → root=null', () => {
+      const r = patternWithRoot('.env', 'localSettings')
+      expect(r.root).toBeNull()
+      expect(r.relativePattern).toBe('.env')
+    })
+  })
+
+  describe('home directory', () => {
+    test('~/.config/app.json → homedir root', () => {
+      const r = patternWithRoot('~/.config/app.json', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/.config/app.json')
+    })
+
+    test('~ only (no /) → falls to root=null', () => {
+      const r = patternWithRoot('~', 'localSettings')
+      expect(r.root).toBeNull()
+      expect(r.relativePattern).toBe('~')
+    })
+  })
+
+  describe('absolute paths', () => {
+    test('/home/user/config.json', () => {
+      const r = patternWithRoot('/home/user/config.json', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/home/user/config.json')
+    })
+
+    test('/etc/hosts', () => {
+      const r = patternWithRoot('/etc/hosts', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/etc/hosts')
+    })
+
+    test('/tmp/output.log', () => {
+      const r = patternWithRoot('/tmp/output.log', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/tmp/output.log')
+    })
+
+    test('/Users/shared/data.json (userSettings source)', () => {
+      const r = patternWithRoot('/Users/shared/data.json', 'userSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/Users/shared/data.json')
+    })
+  })
+
+  describe('// prefix (filesystem root)', () => {
+    test('//etc/hosts → root=/, relative=/etc/hosts', () => {
+      const r = patternWithRoot('//etc/hosts', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/etc/hosts')
+    })
+
+    test('//home/user/file.txt', () => {
+      const r = patternWithRoot('//home/user/file.txt', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.relativePattern).toBe('/home/user/file.txt')
+    })
+  })
+
+  describe('/c/... NOT treated as Windows drive on Linux', () => {
+    test('/c/Users/me/config.json → settings-root (no drive letter)', () => {
+      const r = patternWithRoot('/c/Users/me/config.json', 'localSettings')
+      expect(r.root).not.toBeNull()
+      expect(r.root).not.toMatch(/^[A-Za-z]:/)
+      expect(r.relativePattern).toBe('/c/Users/me/config.json')
+    })
+  })
+})

src/utils/permissions/__tests__/filesystem.windows.test.ts

@@ -0,0 +1,144 @@
+import { describe, expect, mock, test } from 'bun:test'
+import type { ToolPermissionContext } from '../../../Tool'
+import { logMock } from '../../../../tests/mocks/log'
+
+mock.module('src/utils/log.ts', logMock)
+mock.module('src/utils/platform.ts', () => ({
+  getPlatform: () => 'windows',
+}))
+mock.module('src/utils/windowsPaths.ts', () => ({
+  windowsPathToPosixPath: (p: string) =>
+    p.startsWith('\\\\')
+      ? p.replace(/\\/g, '/')
+      : p
+          .replace(
+            /^([A-Za-z]):/,
+            (_m: string, d: string) => `/${d.toLowerCase()}`,
+          )
+          .replace(/\\/g, '/'),
+}))
+mock.module('src/utils/path.ts', () => ({
+  containsPathTraversal: (path: string) =>
+    /(?:^|[\\/])\.\.(?:[\\/]|$)/.test(path),
+  expandPath: (path: string) => path,
+  getDirectoryForPath: (path: string) => {
+    const slash = Math.max(path.lastIndexOf('/'), path.lastIndexOf('\\'))
+    return slash === -1 ? path : path.slice(0, slash)
+  },
+  sanitizePath: (path: string) => path,
+}))
+
+const { matchingRuleForInput, patternWithRoot } = await import('../filesystem')
+
+function makePermissionContext(
+  rules: Partial<
+    Pick<
+      ToolPermissionContext,
+      'alwaysAllowRules' | 'alwaysDenyRules' | 'alwaysAskRules'
+    >
+  >,
+): ToolPermissionContext {
+  return {
+    mode: 'default',
+    additionalWorkingDirectories: new Map(),
+    alwaysAllowRules: rules.alwaysAllowRules ?? {},
+    alwaysDenyRules: rules.alwaysDenyRules ?? {},
+    alwaysAskRules: rules.alwaysAskRules ?? {},
+    isBypassPermissionsModeAvailable: true,
+  }
+}
+
+describe('patternWithRoot (Windows)', () => {
+  test('C:\\Users\\me\\config.json → root=C:\\', () => {
+    const r = patternWithRoot('C:\\Users\\me\\config.json', 'localSettings')
+    expect(r.root).toBe('C:\\')
+    expect(r.relativePattern).toBe('Users/me/config.json')
+  })
+
+  test('C:/Users/me/config.json → root=C:\\', () => {
+    const r = patternWithRoot('C:/Users/me/config.json', 'localSettings')
+    expect(r.root).toBe('C:\\')
+    expect(r.relativePattern).toBe('Users/me/config.json')
+  })
+
+  test('D:\\projects\\app\\main.ts → root=D:\\', () => {
+    const r = patternWithRoot('D:\\projects\\app\\main.ts', 'localSettings')
+    expect(r.root).toBe('D:\\')
+    expect(r.relativePattern).toBe('projects/app/main.ts')
+  })
+
+  test('/c/Users/me/config.json (POSIX) → drive root', () => {
+    const r = patternWithRoot('/c/Users/me/config.json', 'localSettings')
+    expect(r.root).toBe('C:\\')
+    expect(r.relativePattern).toBe('Users/me/config.json')
+  })
+
+  test('/d/projects/main.ts → D: drive root', () => {
+    const r = patternWithRoot('/d/projects/main.ts', 'localSettings')
+    expect(r.root).toBe('D:\\')
+    expect(r.relativePattern).toBe('projects/main.ts')
+  })
+
+  test('src/config.ts → root=null (relative on Windows too)', () => {
+    const r = patternWithRoot('src/config.ts', 'localSettings')
+    expect(r.root).toBeNull()
+    expect(r.relativePattern).toBe('src/config.ts')
+  })
+})
+
+describe('matchingRuleForInput (Windows)', () => {
+  test('matches deny rule written with backslash drive path', () => {
+    const context = makePermissionContext({
+      alwaysDenyRules: {
+        localSettings: ['Read(C:\\Users\\me\\config.json)'],
+      },
+    })
+
+    const rule = matchingRuleForInput(
+      'C:\\Users\\me\\config.json',
+      context,
+      'read',
+      'deny',
+    )
+
+    expect(rule).not.toBeNull()
+    expect(rule?.ruleBehavior).toBe('deny')
+    expect(rule?.ruleValue.ruleContent).toBe('C:\\Users\\me\\config.json')
+  })
+
+  test('matches deny rule written with slash drive path', () => {
+    const context = makePermissionContext({
+      alwaysDenyRules: {
+        localSettings: ['Read(C:/Users/me/config.json)'],
+      },
+    })
+
+    const rule = matchingRuleForInput(
+      'C:\\Users\\me\\config.json',
+      context,
+      'read',
+      'deny',
+    )
+
+    expect(rule).not.toBeNull()
+    expect(rule?.ruleBehavior).toBe('deny')
+    expect(rule?.ruleValue.ruleContent).toBe('C:/Users/me/config.json')
+  })
+
+  test('does not match same relative path on a different drive', () => {
+    const context = makePermissionContext({
+      alwaysDenyRules: {
+        localSettings: ['Read(C:\\Users\\me\\config.json)'],
+      },
+    })
+
+    const rule = matchingRuleForInput(
+      'D:\\Users\\me\\config.json',
+      context,
+      'read',
+      'deny',
+    )
+
+    expect(rule).toBeNull()
+  })
+})

src/utils/permissions/filesystem.ts

@@ -850,13 +850,21 @@ export function getFileReadIgnorePatterns(
   return result
 }
 
-function patternWithRoot(
-  pattern: string,
+export function patternWithRoot(
+  originalPattern: string,
   source: PermissionRuleSource,
 ): {
   relativePattern: string
   root: string | null
 } {
+  // On Windows, normalize the pattern to POSIX so that
+  // Read(C:\Users\...\config.json) and Read(C:/Users/.../config.json)
+  // both work — previously only //C:/Users/... was recognized.
+  let pattern = originalPattern
+  if (getPlatform() === 'windows' && /^[A-Za-z]:[\\/]/.test(originalPattern)) {
+    pattern = windowsPathToPosixPath(originalPattern)
+  }
+
   if (pattern.startsWith(`${DIR_SEP}${DIR_SEP}`)) {
     // Patterns starting with // resolve relative to /
     const patternWithoutDoubleSlash = pattern.slice(1)
@@ -868,18 +876,12 @@ function patternWithRoot(
       getPlatform() === 'windows' &&
       patternWithoutDoubleSlash.match(/^\/[a-z]\//i)
     ) {
-      // Convert POSIX path to Windows format
-      // The pattern is like /c/Users/... so we convert it to C:\Users\...
       const driveLetter = patternWithoutDoubleSlash[1]?.toUpperCase() ?? 'C'
-      // Keep the pattern in POSIX format since relativePath returns POSIX paths
       const pathAfterDrive = patternWithoutDoubleSlash.slice(2)
-
-      // Extract the drive root (C:\) and the rest of the pattern
       const driveRoot = `${driveLetter}:\\`
       const relativeFromDrive = pathAfterDrive.startsWith('/')
         ? pathAfterDrive.slice(1)
         : pathAfterDrive
-
       return {
         relativePattern: relativeFromDrive,
         root: driveRoot,
@@ -890,28 +892,45 @@ function patternWithRoot(
       relativePattern: patternWithoutDoubleSlash,
       root: DIR_SEP,
     }
-  } else if (pattern.startsWith(`~${DIR_SEP}`)) {
+  }
+
+  // On Windows, recognize POSIX-ified absolute drive paths like /c/Users/...
+  // (produced by windowsPathToPosixPath above) as absolute, using the drive
+  // as root. Must come BEFORE the general pattern.startsWith('/') check so
+  // the drive letter is used as root, not rootPathForSource(source).
+  if (getPlatform() === 'windows' && /^\/[a-z](\/|$)/i.test(pattern)) {
+    const driveLetter = pattern[1]!.toUpperCase()
+    const relativePattern = pattern.slice(3) // skip "/c/"
+    return {
+      relativePattern,
+      root: `${driveLetter}:\\`,
+    }
+  }
+
+  if (pattern.startsWith(`~${DIR_SEP}`)) {
     // Patterns starting with ~/ resolve relative to homedir
     return {
       relativePattern: pattern.slice(1),
       root: homedir().normalize('NFC'),
     }
-  } else if (pattern.startsWith(DIR_SEP)) {
-    // Patterns starting with / resolve relative to the directory where settings are stored (without .claude/)
+  }
+
+  if (pattern.startsWith(DIR_SEP)) {
+    // Patterns starting with / resolve relative to the directory where settings are stored
     return {
       relativePattern: pattern,
       root: rootPathForSource(source),
     }
   }
-  // No root specified, put it with all the other patterns
-  // Normalize patterns that start with "./" to remove the prefix
-  // This ensures that patterns like "./.env" match files like ".env"
-  let normalizedPattern = pattern
+
+  // No root specified, put it with all the other patterns.
+  // Normalize patterns that start with "./" to remove the prefix.
+  let relativePattern = pattern
   if (pattern.startsWith(`.${DIR_SEP}`)) {
-    normalizedPattern = pattern.slice(2)
+    relativePattern = pattern.slice(2)
   }
   return {
-    relativePattern: normalizedPattern,
+    relativePattern,
     root: null,
   }
 }