From e8bf3ea63e7f633ff88573d4a52b993c3fedd469 Mon Sep 17 00:00:00 2001
From: Merge Sim <sim@local>
Date: Mon, 7 Sep 2026 13:11:35 -0700
Subject: [PATCH] fix(native-chat): authorize external attachment paths before
 preview

---
 .../native-chat-composer-drop-scope.test.tsx  | 133 +++++++++++++++---
 ...-native-chat-external-attachments.test.tsx |  48 ++++++-
 .../use-native-chat-external-attachments.ts   |  18 ++-
 3 files changed, 180 insertions(+), 19 deletions(-)

diff --git a/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx b/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx
index 8e12ec6fbe8..e535a59fad0 100644
--- a/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx
+++ b/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx
@@ -1,9 +1,12 @@
 // @vitest-environment happy-dom
 
 import { EventEmitter } from 'node:events'
-import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
-import { act, cleanup, render } from '@testing-library/react'
+import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
+import { act, cleanup, render, screen } from '@testing-library/react'
 import { useRef } from 'react'
+import { useNativeChatExternalAttachments } from './use-native-chat-external-attachments'
+import { NativeChatImageAttachmentPreview } from './NativeChatImageAttachmentPreview'
+import { resetLocalImageSrcStateForTests } from '../editor/useLocalImageSrc'
 import { useComposerDropListener } from '../../hooks/composer-state/composer-drop-listener'
 import type { NativeFileDropPayload } from '../../../../shared/native-file-drop'
 import { useNativeChatFileAttachmentActions } from './use-native-chat-file-attachment-actions'
@@ -20,6 +23,18 @@ const electron = vi.hoisted(() => ({
   getPathForFile: vi.fn((file: File) => `/repro/${file.name}`)
 }))
 
+const intake = vi.hoisted(() => ({
+  owner: { kind: 'local' } as { kind: string; connectionId?: string },
+  authorizeExternalPath: vi.fn(),
+  readFile: vi.fn(),
+  upload: vi.fn()
+}))
+vi.mock('@/store', () => ({ useAppStore: { getState: () => ({}) } }))
+vi.mock('./native-chat-attachment-upload', () => ({
+  resolveNativeChatAttachmentOwner: () => intake.owner,
+  uploadNativeChatAttachmentPaths: intake.upload
+}))
+
 vi.mock('electron', () => ({
   ipcRenderer: electron,
   webUtils: { getPathForFile: electron.getPathForFile }
@@ -47,7 +62,13 @@ function ComposerProbe({ pane, hidden = false }: { pane: string; hidden?: boolea
     setDraft: () => {},
     setNotice: () => {}
   })
-  useNativeChatFileAttachmentActions(pane, attachments.attachResolvedPaths)
+  const { attachExternalPaths } = useNativeChatExternalAttachments({
+    terminalTabId: pane,
+    disabled: false,
+    attachResolvedPaths: attachments.attachResolvedPaths,
+    setNotice: () => {}
+  })
+  useNativeChatFileAttachmentActions(pane, attachExternalPaths)
   return (
     <div data-pane={pane} style={{ display: hidden ? 'none' : 'block' }}>
       <textarea
@@ -55,12 +76,19 @@ function ComposerProbe({ pane, hidden = false }: { pane: string; hidden?: boolea
         data-native-file-drop-target="composer"
         data-composer-scope-key={pane}
       />
+      {attachments.imageAttachments.map((attachment) => (
+        <NativeChatImageAttachmentPreview
+          key={attachment.id}
+          attachment={attachment}
+          onRemove={() => {}}
+        />
+      ))}
       <output>{JSON.stringify(attachments.imageAttachments.map(({ path }) => path))}</output>
     </div>
   )
 }
 
-function dropTwoImages(target: Element): void {
+async function dropTwoImages(target: Element): Promise<void> {
   const event = new Event('drop', { bubbles: true, cancelable: true })
   Object.defineProperty(event, 'dataTransfer', {
     value: {
@@ -68,7 +96,9 @@ function dropTwoImages(target: Element): void {
       files: [new File(['a'], 'first.png'), new File(['b'], 'second.png')]
     }
   })
-  act(() => target.dispatchEvent(event))
+  await act(async () => {
+    target.dispatchEvent(event)
+  })
 }
 
 function WorkspaceComposerProbe({ onDrop }: { onDrop: (paths: string[]) => void }) {
@@ -91,18 +121,28 @@ describe('native chat composer drop scoping', () => {
     })
     Object.defineProperty(window, 'api', {
       configurable: true,
-      value: { ui: { onFileDrop: subscribeNativeFileDrop } }
+      value: { ui: { onFileDrop: subscribeNativeFileDrop }, fs: intake }
     })
     installNativeFileDropHandlers()
   })
 
+  beforeEach(() => {
+    intake.owner = { kind: 'local' }
+    intake.authorizeExternalPath.mockReset().mockResolvedValue(undefined)
+    intake.readFile.mockReset().mockResolvedValue({ content: '', isBinary: false })
+    intake.upload.mockReset()
+    vi.stubGlobal('IntersectionObserver', undefined)
+  })
+
   afterEach(() => {
     cleanup()
+    resetLocalImageSrcStateForTests()
+    vi.unstubAllGlobals()
     clearNativeChatAttachmentCacheForTests()
     electron.send.mockClear()
   })
 
-  it('attaches only to the dropped pane and leaves a hidden pane clean on remount', () => {
+  it('attaches only to the dropped pane and leaves a hidden pane clean on remount', async () => {
     const view = render(
       <>
         <ComposerProbe pane="chat-a" />
@@ -113,7 +153,7 @@ describe('native chat composer drop scoping', () => {
     expect(readNativeChatAttachmentCache('chat-b')).toEqual([])
 
     const target = view.container.querySelector('[data-pane="chat-a"] textarea')!
-    dropTwoImages(target)
+    await dropTwoImages(target)
 
     expect(electron.send).toHaveBeenCalledExactlyOnceWith('terminal:file-dropped-from-preload', {
       target: 'composer',
@@ -131,7 +171,7 @@ describe('native chat composer drop scoping', () => {
     expect(returned.container.querySelector('output')?.textContent).toBe('[]')
   })
 
-  it('keeps a drop into an unscoped composer out of every chat pane', () => {
+  it('keeps a drop into an unscoped composer out of every chat pane', async () => {
     const view = render(
       <>
         <ComposerProbe pane="chat-a" />
@@ -139,7 +179,7 @@ describe('native chat composer drop scoping', () => {
         <div data-native-file-drop-target="composer" data-unscoped-composer="true" />
       </>
     )
-    dropTwoImages(view.container.querySelector('[data-unscoped-composer="true"]')!)
+    await dropTwoImages(view.container.querySelector('[data-unscoped-composer="true"]')!)
     expect(electron.send).toHaveBeenCalledExactlyOnceWith('terminal:file-dropped-from-preload', {
       target: 'composer',
       paths: ['/repro/first.png', '/repro/second.png']
@@ -148,7 +188,7 @@ describe('native chat composer drop scoping', () => {
     expect(readNativeChatAttachmentCache('chat-b')).toEqual([])
   })
 
-  it('isolates native chat drops from the workspace composer while preserving workspace drops', () => {
+  it('isolates native chat drops from the workspace composer while preserving workspace drops', async () => {
     const workspaceDrop = vi.fn()
     const view = render(
       <>
@@ -158,7 +198,7 @@ describe('native chat composer drop scoping', () => {
       </>
     )
 
-    dropTwoImages(view.container.querySelector('[data-pane="chat-a"] textarea')!)
+    await dropTwoImages(view.container.querySelector('[data-pane="chat-a"] textarea')!)
     expect(workspaceDrop).not.toHaveBeenCalled()
     expect(readNativeChatAttachmentCache('chat-b')).toEqual([])
     expect(readNativeChatAttachmentCache('chat-a').map(({ path }) => path)).toEqual([
@@ -166,7 +206,7 @@ describe('native chat composer drop scoping', () => {
       '/repro/second.png'
     ])
 
-    dropTwoImages(view.container.querySelector('[data-workspace-composer="true"]')!)
+    await dropTwoImages(view.container.querySelector('[data-workspace-composer="true"]')!)
     expect(workspaceDrop).toHaveBeenCalledExactlyOnceWith(
       ['/repro/first.png', '/repro/second.png'],
       expect.any(Function)
@@ -178,8 +218,67 @@ describe('native chat composer drop scoping', () => {
     expect(readNativeChatAttachmentCache('chat-b')).toEqual([])
   })
 
+  it('authorizes only dropped files before preview reads and leaves the other pane untouched', async () => {
+    const authorized = new Set<string>()
+    intake.authorizeExternalPath.mockImplementation(
+      async ({ targetPath }: { targetPath: string }) => {
+        authorized.add(targetPath)
+      }
+    )
+    intake.readFile.mockImplementation(async ({ filePath }: { filePath: string }) => {
+      if (!authorized.has(filePath)) {
+        throw new Error('Access denied: path resolves outside allowed directories')
+      }
+      return { content: 'AA==', isBinary: true, mimeType: 'image/png' }
+    })
+    await expect(intake.readFile({ filePath: '/repro/first.png' })).rejects.toThrow('Access denied')
+    intake.readFile.mockClear()
+    const view = render(
+      <>
+        <ComposerProbe pane="chat-a" />
+        <ComposerProbe pane="chat-b" hidden />
+      </>
+    )
+    await dropTwoImages(view.container.querySelector('[data-pane="chat-a"] textarea')!)
+    expect(await screen.findByRole('img', { name: 'first.png' })).toBeTruthy()
+    expect(await screen.findByRole('img', { name: 'second.png' })).toBeTruthy()
+    expect(intake.authorizeExternalPath.mock.calls).toEqual([
+      [{ targetPath: '/repro/first.png' }],
+      [{ targetPath: '/repro/second.png' }]
+    ])
+    expect(intake.readFile).toHaveBeenCalledTimes(2)
+    expect(intake.upload).not.toHaveBeenCalled()
+    expect(readNativeChatAttachmentCache('chat-a')).toHaveLength(2)
+    expect(readNativeChatAttachmentCache('chat-b')).toEqual([])
+    await expect(intake.readFile({ filePath: '/repro/sibling.png' })).rejects.toThrow(
+      'Access denied'
+    )
+  })
+
+  it('uploads once for the SSH drop owner without authorizing remote paths locally', async () => {
+    intake.owner = { kind: 'ssh', connectionId: 'conn-1' }
+    intake.upload.mockResolvedValue(['/remote/first.png', '/remote/second.png'])
+    const view = render(
+      <>
+        <ComposerProbe pane="chat-a" />
+        <ComposerProbe pane="chat-b" hidden />
+      </>
+    )
+    await dropTwoImages(view.container.querySelector('[data-pane="chat-a"] textarea')!)
+    expect(intake.upload).toHaveBeenCalledExactlyOnceWith(
+      ['/repro/first.png', '/repro/second.png'],
+      intake.owner
+    )
+    expect(intake.authorizeExternalPath).not.toHaveBeenCalled()
+    expect(readNativeChatAttachmentCache('chat-a').map(({ path }) => path)).toEqual([
+      '/remote/first.png',
+      '/remote/second.png'
+    ])
+    expect(readNativeChatAttachmentCache('chat-b')).toEqual([])
+  })
+
   // Mirrors the terminal target, whose leaf id sits inside its drop-target marker.
-  it('reads a scope key published inside the drop-target marker', () => {
+  it('reads a scope key published inside the drop-target marker', async () => {
     const view = render(
       <div data-native-file-drop-target="composer">
         <div data-composer-scope-key="chat-a">
@@ -187,7 +286,7 @@ describe('native chat composer drop scoping', () => {
         </div>
       </div>
     )
-    dropTwoImages(view.container.querySelector('[data-inner-drop-point="true"]')!)
+    await dropTwoImages(view.container.querySelector('[data-inner-drop-point="true"]')!)
     expect(electron.send).toHaveBeenCalledExactlyOnceWith('terminal:file-dropped-from-preload', {
       target: 'composer',
       scopeKey: 'chat-a',
@@ -195,7 +294,7 @@ describe('native chat composer drop scoping', () => {
     })
   })
 
-  it('control: an editor-targeted drop does not attach images to either chat', () => {
+  it('control: an editor-targeted drop does not attach images to either chat', async () => {
     const view = render(
       <>
         <ComposerProbe pane="chat-a" />
@@ -203,7 +302,7 @@ describe('native chat composer drop scoping', () => {
         <div data-native-file-drop-target="editor" />
       </>
     )
-    dropTwoImages(view.container.querySelector('[data-native-file-drop-target="editor"]')!)
+    await dropTwoImages(view.container.querySelector('[data-native-file-drop-target="editor"]')!)
     expect(electron.send).toHaveBeenCalledExactlyOnceWith('terminal:file-dropped-from-preload', {
       target: 'editor',
       paths: ['/repro/first.png', '/repro/second.png']
diff --git a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx
index ab801b27c0a..679c13fab51 100644
--- a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx
+++ b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx
@@ -1,9 +1,10 @@
 // @vitest-environment happy-dom
-import { afterEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { act, createElement } from 'react'
 import { createRoot, type Root } from 'react-dom/client'
 
 const mocks = vi.hoisted(() => ({
+  authorizeExternalPath: vi.fn(),
   resolveNativeChatAttachmentOwner: vi.fn(),
   uploadNativeChatAttachmentPaths: vi.fn()
 }))
@@ -91,6 +92,13 @@ async function renderProbe(args: {
   }
 }
 
+beforeEach(() => {
+  mocks.authorizeExternalPath.mockReset().mockResolvedValue(undefined)
+  window.api = {
+    fs: { authorizeExternalPath: mocks.authorizeExternalPath }
+  } as unknown as Window['api']
+})
+
 afterEach(() => {
   root?.unmount()
   root = null
@@ -105,10 +113,47 @@ describe('useNativeChatExternalAttachments', () => {
     await act(async () => {
       probe.latest().attachExternalPaths(['/local/a.txt'])
     })
+    expect(mocks.authorizeExternalPath).toHaveBeenCalledExactlyOnceWith({
+      targetPath: '/local/a.txt'
+    })
     expect(attachResolvedPaths).toHaveBeenCalledWith(['/local/a.txt'])
     expect(mocks.uploadNativeChatAttachmentPaths).not.toHaveBeenCalled()
   })
 
+  it('waits for local authorization and skips rejected paths without blocking other files', async () => {
+    mocks.resolveNativeChatAttachmentOwner.mockReturnValue({ kind: 'local' })
+    const authorization = deferred<void>()
+    mocks.authorizeExternalPath
+      .mockReturnValueOnce(authorization.promise)
+      .mockRejectedValueOnce(new Error('denied'))
+    const attachResolvedPaths = vi.fn()
+    const probe = await renderProbe({ attachResolvedPaths })
+    act(() =>
+      probe.latest().attachExternalPaths(['/external/a.png', '/external/b.png', '/external/c.png'])
+    )
+    expect(attachResolvedPaths).not.toHaveBeenCalled()
+    expect(mocks.authorizeExternalPath).toHaveBeenCalledTimes(1)
+    await act(async () => authorization.resolve())
+    expect(attachResolvedPaths).toHaveBeenCalledExactlyOnceWith([
+      '/external/a.png',
+      '/external/c.png'
+    ])
+    expect(mocks.authorizeExternalPath).toHaveBeenCalledTimes(3)
+  })
+
+  it('does not attach local paths when disabled during authorization', async () => {
+    mocks.resolveNativeChatAttachmentOwner.mockReturnValue({ kind: 'local' })
+    const authorization = deferred<void>()
+    mocks.authorizeExternalPath.mockReturnValueOnce(authorization.promise)
+    const attachResolvedPaths = vi.fn()
+    const probe = await renderProbe({ attachResolvedPaths })
+    act(() => probe.latest().attachExternalPaths(['/external/a.png', '/external/b.png']))
+    await probe.setDisabled(true)
+    await act(async () => authorization.resolve())
+    expect(attachResolvedPaths).not.toHaveBeenCalled()
+    expect(mocks.authorizeExternalPath).toHaveBeenCalledTimes(1)
+  })
+
   it('uploads SSH worktree paths and attaches the remote results', async () => {
     mocks.resolveNativeChatAttachmentOwner.mockReturnValue({
       kind: 'ssh',
@@ -133,6 +178,7 @@ describe('useNativeChatExternalAttachments', () => {
       expectedSshConnectionGeneration: 4
     })
     expect(attachResolvedPaths).toHaveBeenCalledWith(['/remote/wt/.orca/drops/a.txt'], 'conn-1')
+    expect(mocks.authorizeExternalPath).not.toHaveBeenCalled()
   })
 
   it('delivers concurrent SSH resolutions in order without deduplicating paths', async () => {
diff --git a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts
index 0341395eb61..d395e54d137 100644
--- a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts
+++ b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts
@@ -62,7 +62,23 @@ export function useNativeChatExternalAttachments({
         return
       }
       if (owner.kind !== 'ssh') {
-        attachResolvedPaths(paths)
+        void (async () => {
+          const authorizedPaths: string[] = []
+          for (const targetPath of paths) {
+            if (disabledRef.current) {
+              return
+            }
+            try {
+              await window.api.fs.authorizeExternalPath({ targetPath })
+              authorizedPaths.push(targetPath)
+            } catch {
+              // Skip unreadable paths, matching workspace composer drops.
+            }
+          }
+          if (authorizedPaths.length > 0 && !disabledRef.current) {
+            attachResolvedPaths(authorizedPaths)
+          }
+        })()
         return
       }
       void (async () => {