fix(desktop): cancel pending file consent and recheck access

Author: waleedlatif1

apps/desktop/README.md

@@ -188,7 +188,7 @@ Copilot can inspect user-selected local directories through the ordinary VFS too
 - **Bound to a live Copilot call:** before a native read/search or browser action, Electron asks the authenticated Sim origin for the pending tool-call record. Local requests must exactly match its persisted operation, path, and options; browser actions run with the persisted arguments rather than renderer-supplied ones. Completed, failed, and aborted runs are rejected.
 - **Abort-aware and bounded:** stop/cancel propagates to active native scans and reads. File size, aggregate grep bytes, line, result, traversal-depth, and scan-count limits remain enforced in Electron, and unsafe regular expressions are rejected before execution.
 
-The native `read_local_file` and `import_local_files` tools also accept absolute or `~/` paths. They reuse the same remembered folder grants as the VFS tools. For an unapproved path, Electron displays a bundled, isolated dialog showing the canonical folder and connected server. **Allow folder** grants read and import access to that folder and its subfolders across chats and normal app restarts. A file request proposes its containing folder explicitly; no wider folder is approved silently. Closing or declining the dialog returns no contents. Users can add or forget folders through **File → Folder Access**. As with VFS grants, sign-out and server changes clear access, and unavailable secure storage limits persistence to the app session. New consent prompts are serialized, but reads of approved folders proceed independently.
+The native `read_local_file` and `import_local_files` tools also accept absolute or `~/` paths. They reuse the same remembered folder grants as the VFS tools. For an unapproved path, Electron displays a bundled, isolated dialog showing the canonical folder and connected server. **Allow folder** grants read and import access to that folder and its subfolders across chats and normal app restarts. A file request proposes its containing folder explicitly; no wider folder is approved silently. Closing or declining the dialog returns no contents. Users can add or forget folders through **File → Folder Access**. As with VFS grants, sign-out and server changes clear access, and unavailable secure storage limits persistence to the app session. New consent prompts are serialized, but reads of approved folders proceed independently. Existing encrypted path-based approvals retain their scope and acquire folder-identity metadata on their first restore.
 
 Approved native reads can return bounded text, directory listings, images, or PDFs to the chat. Approved imports transfer file bytes to Workspace Files. Electron revalidates every pending call before using a grant, including remembered grants, checks canonical containment and grant identity throughout the operation, and opens files with no-follow and descriptor identity checks. A model or hosted renderer cannot answer the local consent dialog. These permissions govern the native file tools; the separately enabled terminal still runs with the user's OS privileges.
 

apps/desktop/e2e/local-files.spec.ts

@@ -222,6 +222,98 @@ test('native file tools remember folder consent across chats and restarts until
         expect(await stale.result).toMatchObject({ ok: false })
       }
     })
+    await test.step('a symlink replacement cannot redirect an inspected file', async () => {
+      const file = realpathSync(join(source, 'report.txt'))
+      const backup = join(source, 'original-report.txt')
+      const other = join(source, 'other.txt')
+      writeFileSync(other, 'different file contents')
+      await app?.evaluate(
+        (_electron, paths) => {
+          const fs = process.getBuiltinModule(
+            'node:fs/promises'
+          ) as typeof import('node:fs/promises')
+          const original = fs.stat
+          fs.stat = (async (...args: Parameters<typeof fs.stat>) => {
+            const result = await original(...args)
+            if (args[0] === paths.file) {
+              fs.stat = original
+              await fs.rename(paths.file, paths.backup)
+              await fs.symlink(paths.other, paths.file)
+            }
+            return result
+          }) as typeof fs.stat
+        },
+        { file, backup, other }
+      )
+      try {
+        expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: false })
+      } finally {
+        rmSync(file)
+        renameSync(backup, file)
+        rmSync(other)
+      }
+    })
+    await test.step('a directory swapped out and back cannot leak outside entries', async () => {
+      await app?.evaluate(
+        (_electron, paths) => {
+          const fs = process.getBuiltinModule(
+            'node:fs/promises'
+          ) as typeof import('node:fs/promises')
+          const originalOpen = fs.opendir
+          const originalRead = fs.readdir
+          const swapped = async <T>(operation: () => Promise<T>): Promise<T> => {
+            fs.opendir = originalOpen
+            fs.readdir = originalRead
+            await fs.rename(paths.source, paths.backup)
+            await fs.symlink(paths.outside, paths.source)
+            try {
+              return await operation()
+            } finally {
+              await fs.rm(paths.source)
+              await fs.rename(paths.backup, paths.source)
+            }
+          }
+          fs.opendir = (path, options) =>
+            path === paths.source
+              ? swapped(() => originalOpen(path, options))
+              : originalOpen(path, options)
+          fs.readdir = ((...args: Parameters<typeof fs.readdir>) =>
+            args[0] === paths.source
+              ? swapped(() => originalRead(...args))
+              : originalRead(...args)) as typeof fs.readdir
+        },
+        { source: realpathSync(source), backup: join(root, 'reports-backup'), outside }
+      )
+      expect(await invoke({ operation: 'read', toolCallId: 'directory' })).toMatchObject({
+        ok: false,
+      })
+    })
+    await test.step('cancelling in the renderer closes consent without remembering access', async () => {
+      calls.cancelled = {
+        toolName: 'read_local_file',
+        args: { path: join(outside, 'private.txt') },
+      }
+      const cancelled = await requestPermission({ operation: 'read', toolCallId: 'cancelled' })
+      const closed = cancelled.prompt.waitForEvent('close', { timeout: 5000 })
+      await window.evaluate(async () => {
+        const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
+        await api.localFiles?.({ operation: 'cancel', toolCallId: 'cancelled' })
+      })
+      await closed
+      expect(await cancelled.result).toMatchObject({ ok: false })
+      const again = await requestPermission({ operation: 'read', toolCallId: 'cancelled' })
+      await again.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
+      expect(await again.result).toMatchObject({ ok: false })
+    })
+    await test.step('consent escapes direction controls in folder names', async () => {
+      const folder = join(root, 'Bidi\u061c\u200e\u200f')
+      mkdirSync(folder)
+      calls.bidi = { toolName: 'read_local_file', args: { path: folder } }
+      const bidi = await requestPermission({ operation: 'read', toolCallId: 'bidi' })
+      await expect(bidi.prompt.getByRole('dialog')).toContainText('Bidi\\u061c\\u200e\\u200f')
+      await bidi.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
+      expect(await bidi.result).toMatchObject({ ok: false })
+    })
     await test.step('an unanswered prompt does not block approved folders', async () => {
       calls.blocker = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } }
       const blocker = await requestPermission({ operation: 'read', toolCallId: 'blocker' })

apps/desktop/src/main/ipc.ts

@@ -598,6 +598,8 @@ async function authorizeLocalFilesystemTool(
  */
 export function registerIpcHandlers(deps: IpcDeps): void {
   const localFilePermissions = new LocalFilePermissions(deps.localFilesystem)
+  const activeLocalFiles = new Map<string, Set<AbortController>>()
+  let activeLocalFileCount = 0
   const browserScopeBySender = new WeakMap<WebContents, string>()
   const terminalScopeBySender = new WeakMap<WebContents, string>()
   const browserPendingScopesBySender = new WeakMap<WebContents, Set<string>>()
@@ -2087,42 +2089,59 @@ export function registerIpcHandlers(deps: IpcDeps): void {
           const generation = captureAccountDataGeneration()
           const origin = deps.appOrigin()
           const request = args[0]
-          if (!isRecordLike(request)) return { ok: false, error: 'Invalid local file request.' }
-          let failureStatus: number | undefined
-          const authorization = await fetchDesktopToolAuthorization(
-            event,
-            deps,
-            request.toolCallId,
-            request.operation === 'manifest',
-            (status) => {
-              failureStatus = status
-            }
-          )
-          if (failureStatus === 409)
+          if (!isRecordLike(request) || !isDesktopToolCallId(request.toolCallId))
+            return { ok: false, error: 'Invalid local file request.' }
+          const key = JSON.stringify([event.sender.id, request.toolCallId])
+          if (request.operation === 'cancel') {
+            for (const pending of activeLocalFiles.get(key) ?? []) pending.abort()
+            return { ok: false, error: 'Local file operation cancelled.' }
+          }
+          if (activeLocalFileCount >= 128)
             return {
               ok: false,
-              code: 'ALREADY_STARTED',
-              error: 'This import is already running or was already started.',
+              error: 'Too many local file operations are running. Try again later.',
             }
-          if (
-            !authorization ||
-            !['read_local_file', 'import_local_files'].includes(authorization.toolName)
-          )
-            return { ok: false, error: 'This is not an authorized pending local file tool call.' }
-          if (
-            authorization.toolName === 'read_local_file'
-              ? request.operation !== 'read'
-              : request.operation !== 'manifest' && request.operation !== 'chunk'
-          )
-            return { ok: false, error: 'The operation does not match the pending tool call.' }
-          const parent = deps.getWindowForContents(event.sender)
-          if (!parent)
-            return { ok: false, error: 'A desktop window is required to approve file access.' }
+          const controller = new AbortController()
+          const controllers = activeLocalFiles.get(key) ?? new Set<AbortController>()
+          controllers.add(controller)
+          activeLocalFiles.set(key, controllers)
+          activeLocalFileCount++
           try {
+            let failureStatus: number | undefined
+            const authorization = await fetchDesktopToolAuthorization(
+              event,
+              deps,
+              request.toolCallId,
+              request.operation === 'manifest',
+              (status) => {
+                failureStatus = status
+              }
+            )
+            if (failureStatus === 409)
+              return {
+                ok: false,
+                code: 'ALREADY_STARTED',
+                error: 'This import is already running or was already started.',
+              }
+            if (
+              !authorization ||
+              !['read_local_file', 'import_local_files'].includes(authorization.toolName)
+            )
+              return { ok: false, error: 'This is not an authorized pending local file tool call.' }
+            if (
+              authorization.toolName === 'read_local_file'
+                ? request.operation !== 'read'
+                : request.operation !== 'manifest' && request.operation !== 'chunk'
+            )
+              return { ok: false, error: 'The operation does not match the pending tool call.' }
+            const parent = deps.getWindowForContents(event.sender)
+            if (!parent)
+              return { ok: false, error: 'A desktop window is required to approve file access.' }
             const access = await localFilePermissions.authorize(authorization, {
               parent,
               origin,
               generation,
+              signal: controller.signal,
               isCurrent: () =>
                 isAccountDataGenerationCurrent(generation) &&
                 deps.accountDataAvailable() &&
@@ -2134,9 +2153,13 @@ export function registerIpcHandlers(deps: IpcDeps): void {
                   await fetchDesktopToolAuthorization(event, deps, request.toolCallId)
                 ),
             })
-            handlerArgs = [request, authorization, access]
+            return await spec.handler(event.sender, request, authorization, access)
           } catch (error) {
             return { ok: false, error: getErrorMessage(error) }
+          } finally {
+            controllers.delete(controller)
+            if (controllers.size === 0) activeLocalFiles.delete(key)
+            activeLocalFileCount--
           }
         }
         if (spec.passSender) {

apps/desktop/src/main/local-file-permissions.ts

@@ -13,6 +13,7 @@ interface LocalFilePermissionContext {
   parent: BrowserWindow
   origin: string
   generation: number
+  signal: AbortSignal
   isCurrent: () => boolean
   revalidate: () => Promise<boolean>
 }
@@ -27,6 +28,7 @@ function nativePath(value: unknown): string {
 }
 
 function assertCurrent(context: LocalFilePermissionContext): void {
+  context.signal.throwIfAborted()
   if (context.parent.isDestroyed() || !context.isCurrent())
     throw new Error('This local file request expired. Ask again in the current chat.')
 }
@@ -89,11 +91,12 @@ export class LocalFilePermissions {
     const root = await lstat(folder)
     if (!root.isDirectory()) throw new Error('The folder is no longer available.')
     const displayedPath = JSON.stringify(folder).replace(
-      /[\u202a-\u202e\u2066-\u2069]/g,
+      /\p{Bidi_Control}/gu,
       (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}`
     )
     assertCurrent(context)
     const result = await showShellDialog(context.parent, {
+      signal: context.signal,
       title: 'Allow access to this folder?',
       message: displayedPath,
       detail: `Sim can read files in this folder and its subfolders, use them across chats, and import them into your workspaces on ${context.origin}.\n\nManage or remove access in File → Folder Access.`,