security: constrain READ_FILE to granted dirs; validate command paths before basename

src/main/ipc/content-handlers.ts

@@ -9,6 +9,8 @@ import type { MessageAttachment } from '../../shared/types'
 import { ConversationModel } from '../database/models/conversation'
 import { MessageModel } from '../database/models/message'
 import { validateFilePath } from '../security/input-validator'
+import { getFileAccessManager } from '../file-access/file-access-manager'
+import { isPathWithinAllowedDirs } from '../../shared/file-access-types'
 import { wrapHandler, requireFeature } from './ipc-utils'
 
 export async function registerContentHandlers(): Promise<void> {
@@ -197,6 +199,13 @@ export async function registerContentHandlers(): Promise<void> {
       if (!pathValidation.valid) {
         return { success: false, error: pathValidation.error }
       }
+      // Constrain reads to the granted-directory model — validateFilePath only
+      // blocks '..'/length, so without this any absolute path was readable
+      // (security audit 2026-08-21).
+      const allowedDirs = getFileAccessManager().getAllowedDirectories().map((d) => d.path)
+      if (!isPathWithinAllowedDirs(filePath, allowedDirs)) {
+        return { success: false, error: 'Access denied: file is not within a granted directory' }
+      }
       const content = await fs.readFile(filePath, 'utf-8')
       return { success: true, content }
     } catch (error) {

src/main/utils/mcp-sanitizer.ts

@@ -63,19 +63,23 @@ export function validateCommand(command: string): SanitizeResult {
 
   // Extract base command name (handle paths like /usr/bin/python3)
   const baseName = trimmed.split('/').pop()?.split('\\').pop() || ''
+  const isPath = trimmed.includes('/') || trimmed.includes('\\')
 
-  // Allow whitelisted commands
-  if (ALLOWED_COMMANDS.has(baseName)) {
+  // The basename allowlist applies ONLY to bare command names. A PATH must be
+  // validated as a path — a renamed binary in an unsafe directory
+  // (e.g. /tmp/evil/python3, ./evil/python3) must not pass just because its
+  // basename is allowlisted (security audit 2026-08-21).
+  if (!isPath && ALLOWED_COMMANDS.has(baseName)) {
     return { valid: true }
   }
 
-  // Allow absolute paths only from safe directories
-  if (trimmed.startsWith('/') || trimmed.startsWith('~') || /^[A-Z]:\\/.test(trimmed)) {
-    const resolved = trimmed.startsWith('~')
-      ? trimmed // tilde paths resolve at spawn time; validate structure only
-      : trimmed
-    const inSafeDir = SAFE_COMMAND_DIRS.some(dir => resolved.startsWith(dir))
-    if (!inSafeDir && !trimmed.startsWith('~')) {
+  // Allow absolute paths only from safe directories.
+  if (isPath && (trimmed.startsWith('/') || trimmed.startsWith('~') || /^[A-Z]:\\/.test(trimmed))) {
+    if (trimmed.startsWith('~')) {
+      return { valid: true } // tilde resolves at spawn time; validate structure only
+    }
+    const inSafeDir = SAFE_COMMAND_DIRS.some(dir => trimmed.startsWith(dir))
+    if (!inSafeDir) {
       return {
         valid: false,
         error: `Absolute path "${trimmed}" is outside safe directories. Allowed: ${SAFE_COMMAND_DIRS.join(', ')}`

src/shared/file-access-types.ts

@@ -99,3 +99,11 @@ export function isPathSafe(requestedPath: string, allowedDir: string): boolean {
   // the grant for /a/photos (security audit 2026-08-21).
   return resolved === allowed || resolved.startsWith(allowed + path.sep)
 }
+
+/**
+ * Security: is the path inside ANY of the granted directories?
+ * Used to constrain raw file reads to the user's granted-directory model.
+ */
+export function isPathWithinAllowedDirs(requestedPath: string, allowedDirPaths: string[]): boolean {
+  return allowedDirPaths.some((dir) => isPathSafe(requestedPath, dir))
+}

tests/unit/file-access-types.test.ts

@@ -21,3 +21,19 @@ describe('isPathSafe', () => {
     expect(isPathSafe('/a/photos/../etc/passwd', '/a/photos')).toBe(false)
   })
 })
+
+import { isPathWithinAllowedDirs } from '../../src/shared/file-access-types'
+describe('isPathWithinAllowedDirs', () => {
+  it('accepts a path inside one of the granted dirs', () => {
+    expect(isPathWithinAllowedDirs('/a/photos/x', ['/a/docs', '/a/photos'])).toBe(true)
+  })
+  it('rejects a path outside every granted dir', () => {
+    expect(isPathWithinAllowedDirs('/etc/passwd', ['/a/docs', '/a/photos'])).toBe(false)
+  })
+  it('rejects a sibling-prefix escape of a granted dir', () => {
+    expect(isPathWithinAllowedDirs('/a/photos-evil/x', ['/a/photos'])).toBe(false)
+  })
+  it('rejects everything when no directory is granted', () => {
+    expect(isPathWithinAllowedDirs('/a/photos/x', [])).toBe(false)
+  })
+})

tests/unit/mcp-sanitizer.test.ts

@@ -160,3 +160,15 @@ describe('MCP Sanitizer', () => {
     })
   })
 })
+
+// Security regression (audit 2026-08-21): the basename allowlist was checked
+// BEFORE the safe-directory rule, so a renamed binary passed on basename alone.
+describe('validateCommand — path validated before basename allowlist', () => {
+  it('rejects an allowlisted basename in an UNSAFE absolute dir', () => {
+    expect(validateCommand('/tmp/evil/python3').valid).toBe(false)
+  })
+  it('rejects an allowlisted basename via a RELATIVE path', () => {
+    expect(validateCommand('./evil/python3').valid).toBe(false)
+    expect(validateCommand('evil/python3').valid).toBe(false)
+  })
+})