Skip to content

Commit d43d45a

Browse files
committed
fix(desktop): ask before accessing local files
1 parent 7a36d74 commit d43d45a

8 files changed

Lines changed: 482 additions & 114 deletions

File tree

‎apps/desktop/README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,9 @@ Copilot can inspect user-selected local directories through the ordinary VFS too
188188
- **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.
189189
- **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.
190190

191-
Raw local file bytes are never exposed through the preload bridge and cannot be staged or uploaded by a model. Bounded text read/search results are returned to the active Copilot request; a user must use the normal attachment UI when they want the file itself to leave the device.
191+
The native `read_local_file` and `import_local_files` tools also accept absolute or `~/` paths. Before accessing a new file or folder, Electron displays a bundled, isolated permission dialog showing the resolved path and the connected server. **Allow for this chat** grants access to that file, or that folder and its contents, for the current desktop session. Closing or declining the dialog returns no contents. Grants are scoped to the server, account generation, chat, and operation; import grants also bind the destination workspace and folder. A read grant never authorizes an import. Grants are not persisted and expire on sign-out, server changes, or app restart.
192+
193+
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 the pending call after consent, 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.
192194

193195
## Auto-update, channels, rollout, rollback
194196

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

Lines changed: 190 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
1+
import {
2+
mkdirSync,
3+
mkdtempSync,
4+
realpathSync,
5+
renameSync,
6+
rmSync,
7+
symlinkSync,
8+
writeFileSync,
9+
} from 'node:fs'
210
import { createServer, type Server } from 'node:http'
311
import { tmpdir } from 'node:os'
412
import { join } from 'node:path'
@@ -8,17 +16,31 @@ import type { DesktopLocalFileRequest, SimDesktopApi } from '@sim/desktop-bridge
816

917
const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url))
1018

11-
test('native file tools read and import through the installed preload without Sim folder grants', async () => {
19+
test('native file tools require local consent and reuse only the approved chat and path', async () => {
1220
const root = mkdtempSync(join(tmpdir(), 'sim-native-files-e2e-'))
1321
const source = join(root, 'Reports')
22+
const outside = join(root, 'Reports-other')
23+
mkdirSync(outside)
24+
writeFileSync(join(outside, 'private.txt'), 'outside contents')
1425
mkdirSync(join(source, 'empty'), { recursive: true })
1526
writeFileSync(join(source, 'report.txt'), 'native file contents')
1627
const png =
1728
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII='
1829
writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64'))
19-
let claimed = false
20-
const calls: Record<string, { toolName: string; args: Record<string, unknown> }> = {
30+
const claimed = new Set<string>()
31+
const authorizedCalls = new Set<string>()
32+
let signedIn = true
33+
const calls: Record<
34+
string,
35+
{ toolName: string; args: Record<string, unknown>; chatId?: string } | undefined
36+
> = {
37+
directory: { toolName: 'read_local_file', args: { path: source } },
2138
text: { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } },
39+
otherChat: {
40+
toolName: 'read_local_file',
41+
args: { path: join(source, 'report.txt') },
42+
chatId: 'other-chat',
43+
},
2244
image: { toolName: 'read_local_file', args: { path: join(source, 'image.png') } },
2345
import: {
2446
toolName: 'import_local_files',
@@ -32,32 +54,49 @@ test('native file tools read and import through the installed preload without Si
3254
const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname
3355
if (path === '/api/auth/get-session') {
3456
response.writeHead(200, { 'Content-Type': 'application/json' }).end(
35-
JSON.stringify({
36-
user: { id: 'local-file-user' },
37-
session: { id: 'local-file-session' },
38-
})
57+
JSON.stringify(
58+
signedIn
59+
? {
60+
user: { id: 'local-file-user' },
61+
session: { id: 'local-file-session' },
62+
}
63+
: null
64+
)
3965
)
4066
return
4167
}
68+
if (path === '/api/auth/sign-out') {
69+
signedIn = false
70+
response
71+
.writeHead(200, {
72+
'Content-Type': 'application/json',
73+
'Set-Cookie': 'better-auth.session_token=; HttpOnly; SameSite=Lax; Path=/; Max-Age=0',
74+
})
75+
.end('{}')
76+
return
77+
}
4278
if (path === '/api/desktop/tool/authorize') {
4379
let body = ''
4480
for await (const chunk of request) body += chunk.toString()
4581
const input = JSON.parse(body)
4682
const call = calls[input.toolCallId]
47-
if (!call || (input.claim && claimed)) {
83+
if (!call || (input.claim && claimed.has(input.toolCallId))) {
4884
response.writeHead(call ? 409 : 403, { 'Content-Type': 'application/json' }).end('{}')
4985
return
5086
}
51-
if (input.claim) claimed = true
87+
authorizedCalls.add(input.toolCallId)
88+
if (input.claim) claimed.add(input.toolCallId)
5289
response
5390
.writeHead(200, { 'Content-Type': 'application/json' })
54-
.end(JSON.stringify({ ...call, chatId: 'org-chat' }))
91+
.end(JSON.stringify({ ...call, chatId: call.chatId ?? 'org-chat' }))
5592
return
5693
}
5794
response
5895
.writeHead(200, {
5996
'Content-Type': 'text/html',
60-
'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/',
97+
...(signedIn
98+
? { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/' }
99+
: {}),
61100
})
62101
.end('<!doctype html><title>Local file fixture</title><h1>Local files</h1>')
63102
})
@@ -82,17 +121,121 @@ test('native file tools read and import through the installed preload without Si
82121
return api.localFiles(request)
83122
}, input)
84123
await expect
85-
.poll(async () => (await invoke({ operation: 'read', toolCallId: 'text' })).ok)
124+
.poll(() =>
125+
window.evaluate(async () => {
126+
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
127+
return (await api.localFilesystem?.({ operation: 'list_mounts' }))?.ok
128+
})
129+
)
86130
.toBe(true)
87-
expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({
131+
await window.evaluate(async () => {
132+
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
133+
await api.settings.setPreference('browserEnabled', false)
134+
await api.settings.setPreference('terminalEnabled', false)
135+
})
136+
const deniedPrompt = app.waitForEvent('window', { timeout: 10_000 })
137+
const deniedRead = invoke({ operation: 'read', toolCallId: 'text' })
138+
const denial = await deniedPrompt
139+
await expect(denial.getByRole('button', { name: "Don't allow", exact: true })).toBeFocused()
140+
await denial.screenshot({
141+
path:
142+
process.env.DESKTOP_LOCAL_FILES_REPORT_PATH ??
143+
test.info().outputPath('local-file-consent.png'),
144+
})
145+
await denial.getByRole('button', { name: "Don't allow", exact: true }).click()
146+
expect(await deniedRead).toMatchObject({ ok: false })
147+
148+
const folderPrompt = app.waitForEvent('window')
149+
const folderRead = invoke({ operation: 'read', toolCallId: 'directory' })
150+
const folderConsent = await folderPrompt
151+
const queuedRead = invoke({ operation: 'read', toolCallId: 'text' })
152+
expect(
153+
await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop)
154+
).toBe('undefined')
155+
await folderConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click()
156+
expect(await folderRead).toMatchObject({ ok: true, data: { representation: 'directory' } })
157+
expect(await queuedRead).toMatchObject({ ok: true, data: { text: 'native file contents' } })
158+
const canonicalRequest = {
159+
operation: 'read' as const,
160+
toolCallId: 'text',
161+
path: join(outside, 'private.txt'),
162+
}
163+
expect(await invoke(canonicalRequest)).toMatchObject({
88164
ok: true,
89165
data: { representation: 'text', text: 'native file contents' },
90166
})
91167
expect(await invoke({ operation: 'read', toolCallId: 'image' })).toMatchObject({
92168
ok: true,
93169
data: { observations: [{ mediaType: 'image/png', data: png }] },
94170
})
95-
const result = await invoke({ operation: 'manifest', toolCallId: 'import' })
171+
const runningApp = app
172+
const requestPermission = async (request: DesktopLocalFileRequest) => {
173+
const shown = runningApp.waitForEvent('window', { timeout: 10_000 })
174+
const result = invoke(request)
175+
void result.catch(() => {})
176+
const prompt = await shown
177+
await expect(prompt.getByRole('button', { name: "Don't allow", exact: true })).toBeVisible()
178+
return { prompt, result }
179+
}
180+
await test.step('a folder grant does not authorize another chat or a symlink escape', async () => {
181+
const otherChat = await requestPermission({ operation: 'read', toolCallId: 'otherChat' })
182+
const dismissed = otherChat.prompt.waitForEvent('close')
183+
await otherChat.prompt
184+
.getByRole('button', { name: "Don't allow", exact: true })
185+
.press('Escape')
186+
.catch(() => {})
187+
await dismissed
188+
expect(await otherChat.result).toMatchObject({ ok: false })
189+
symlinkSync(join(outside, 'private.txt'), join(source, 'linked.txt'))
190+
calls.escape = { toolName: 'read_local_file', args: { path: join(source, 'linked.txt') } }
191+
const escapedRead = await requestPermission({ operation: 'read', toolCallId: 'escape' })
192+
await expect(escapedRead.prompt.getByRole('dialog')).toContainText(
193+
JSON.stringify(realpathSync(join(outside, 'private.txt')))
194+
)
195+
await escapedRead.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
196+
expect(await escapedRead.result).toMatchObject({ ok: false })
197+
rmSync(join(source, 'linked.txt'))
198+
})
199+
await test.step('cancelled calls and changed arguments cannot acquire a grant', async () => {
200+
for (const changed of [false, true]) {
201+
calls.stale = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } }
202+
const stale = await requestPermission({ operation: 'read', toolCallId: 'stale' })
203+
if (changed) calls.stale.args.path = join(source, 'report.txt')
204+
else calls.stale = undefined
205+
await stale.prompt.getByRole('button', { name: 'Allow for this chat', exact: true }).click()
206+
expect(await stale.result).toMatchObject({ ok: false })
207+
}
208+
})
209+
await test.step('a queued call is revalidated even when its folder is already approved', async () => {
210+
calls.blocker = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } }
211+
calls.queued = { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } }
212+
const blocker = await requestPermission({ operation: 'read', toolCallId: 'blocker' })
213+
const queued = invoke({ operation: 'read', toolCallId: 'queued' })
214+
void queued.catch(() => {})
215+
await expect.poll(() => authorizedCalls.has('queued')).toBe(true)
216+
calls.queued = undefined
217+
await blocker.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
218+
expect(await blocker.result).toMatchObject({ ok: false })
219+
expect(await queued).toMatchObject({ ok: false })
220+
})
221+
await test.step('replacing the proposed folder during consent does not expose its new target', async () => {
222+
const proposed = join(root, 'Proposed')
223+
mkdirSync(proposed)
224+
calls.retargeted = { toolName: 'read_local_file', args: { path: proposed } }
225+
const retargeted = await requestPermission({ operation: 'read', toolCallId: 'retargeted' })
226+
renameSync(proposed, join(root, 'Original'))
227+
mkdirSync(proposed)
228+
writeFileSync(join(proposed, 'unapproved.txt'), 'replacement folder contents')
229+
await retargeted.prompt
230+
.getByRole('button', { name: 'Allow for this chat', exact: true })
231+
.click()
232+
expect(await retargeted.result).toMatchObject({ ok: false })
233+
})
234+
const importPrompt = app.waitForEvent('window')
235+
const importing = invoke({ operation: 'manifest', toolCallId: 'import' })
236+
const importConsent = await importPrompt
237+
await importConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click()
238+
const result = await importing
96239
if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result))
97240
expect(result.data.targetWorkspaceId).toBe('target-workspace')
98241
expect(result.data.entries.map((entry) => entry.relativePath)).toEqual([
@@ -136,6 +279,38 @@ test('native file tools read and import through the installed preload without Si
136279
ok: false,
137280
code: 'ALREADY_STARTED',
138281
})
282+
await test.step('import approval is bound to its destination workspace', async () => {
283+
calls.otherImport = {
284+
toolName: 'import_local_files',
285+
args: { path: source, targetWorkspaceId: 'other-workspace' },
286+
}
287+
const otherImport = await requestPermission({
288+
operation: 'manifest',
289+
toolCallId: 'otherImport',
290+
})
291+
await otherImport.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
292+
expect(await otherImport.result).toMatchObject({ ok: false })
293+
})
294+
295+
await test.step('sign-out revokes chat grants before the next account session', async () => {
296+
await window.evaluate(async () => {
297+
await fetch('/api/auth/sign-out', { method: 'POST' })
298+
})
299+
await expect(window).toHaveURL(`http://127.0.0.1:${address.port}/login`)
300+
signedIn = true
301+
await window.goto(`http://127.0.0.1:${address.port}/`)
302+
await expect
303+
.poll(() =>
304+
window.evaluate(async () => {
305+
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
306+
return (await api.localFilesystem?.({ operation: 'list_mounts' }))?.ok
307+
})
308+
)
309+
.toBe(true)
310+
const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' })
311+
await revoked.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
312+
expect(await revoked.result).toMatchObject({ ok: false })
313+
})
139314
} finally {
140315
await app?.close()
141316
server?.close()

‎apps/desktop/src/main/ipc.test.ts‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -458,71 +458,6 @@ describe('registerIpcHandlers', () => {
458458
})
459459
})
460460

461-
it('reads a native file through canonical IPC arguments without folder grants or user activation', async () => {
462-
const { invoke } = collectHandlers()
463-
const handler = invoke.get('desktop:local-files')
464-
const path = fileURLToPath(import.meta.url)
465-
const fetchAuthorization = vi.fn(async () =>
466-
Response.json({ chatId: 'chat-1', toolName: 'read_local_file', args: { path, limit: 64 } })
467-
)
468-
const authorizedEvent = {
469-
senderFrame: { url: `${APP}/o/org/home` },
470-
sender: { session: { fetch: fetchAuthorization } },
471-
}
472-
const mounts = vi.spyOn(deps.localFilesystem, 'handle')
473-
expect(
474-
await handler?.(authorizedEvent, {
475-
operation: 'read',
476-
toolCallId: 'tool-native',
477-
path: '/not/the/canonical/path',
478-
})
479-
).toMatchObject({
480-
ok: true,
481-
data: { kind: 'read', path, text: readFileSync(path, 'utf8').slice(0, 64) },
482-
})
483-
expect(mounts).not.toHaveBeenCalled()
484-
expect(fetchAuthorization).toHaveBeenCalledWith(
485-
`${APP}/api/desktop/tool/authorize`,
486-
expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native' }) })
487-
)
488-
expect(
489-
await handler?.(evilEvent, { operation: 'read', toolCallId: 'tool-native' })
490-
).toMatchObject({ ok: false })
491-
})
492-
493-
it('claims native imports at IPC before traversal and rejects a replay', async () => {
494-
const { invoke } = collectHandlers()
495-
const handler = invoke.get('desktop:local-files')
496-
const fetchAuthorization = vi
497-
.fn()
498-
.mockResolvedValueOnce(
499-
Response.json({
500-
chatId: 'chat-1',
501-
toolName: 'import_local_files',
502-
args: { path: fileURLToPath(import.meta.url), targetWorkspaceId: 'workspace' },
503-
})
504-
)
505-
.mockResolvedValueOnce(Response.json({ error: 'already started' }, { status: 409 }))
506-
const event = {
507-
senderFrame: { url: `${APP}/o/org/home` },
508-
sender: { session: { fetch: fetchAuthorization } },
509-
}
510-
const request = { operation: 'manifest', toolCallId: 'tool-import' }
511-
expect(await handler?.(event, request)).toMatchObject({
512-
ok: true,
513-
data: {
514-
kind: 'manifest',
515-
targetWorkspaceId: 'workspace',
516-
entries: [{ relativePath: '', kind: 'file' }],
517-
},
518-
})
519-
expect(fetchAuthorization).toHaveBeenCalledWith(
520-
`${APP}/api/desktop/tool/authorize`,
521-
expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-import', claim: true }) })
522-
)
523-
expect(await handler?.(event, request)).toMatchObject({ ok: false, code: 'ALREADY_STARTED' })
524-
})
525-
526461
it('requires server authorization for every privileged filesystem tool request', async () => {
527462
const { invoke } = collectHandlers()
528463
const handler = invoke.get('desktop:local-filesystem')

0 commit comments

Comments
 (0)