Skip to content

Commit 6ca8a97

Browse files
committed
fix(desktop): remember folder permissions across chats
1 parent 8fab9e6 commit 6ca8a97

10 files changed

Lines changed: 323 additions & 181 deletions

File tree

‎apps/desktop/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,15 +182,15 @@ Copilot can inspect user-selected local directories through the ordinary VFS too
182182

183183
- **Explicit and read-only:** only a user click may open the native folder picker or revoke a grant; model tool calls cannot do either. There are no write/delete/execute/upload operations.
184184
- **Remembered securely:** grants are encrypted in Electron's private app data with OS-backed `safeStorage` and restored with the same opaque URI after a normal app restart. (A security-scoped bookmark is stored alongside each grant, but it is a no-op in the current Developer ID build — only the macOS App Sandbox consumes it — and is kept purely for forward-compatibility should a sandboxed/MAS build ever ship.) There is no plaintext fallback: when secure storage is unavailable, the returned mount has `remembered: false` and lasts only for that app session.
185-
- **Revocable:** Desktop settings removes one grant. All grants are removed on explicit sign-out or server-origin change so another Sim account or server cannot inherit them. Normal app quit only releases active OS handles and keeps the encrypted grants.
185+
- **Revocable:** File → Folder Access adds folders and removes individual grants. All grants are removed on explicit sign-out or server-origin change so another Sim account or server cannot inherit them. Normal app quit only releases active OS handles and keeps the encrypted grants.
186186
- **Opaque:** the model sees canonical paths such as `user-local/Project--<mount-id>/README.md`, never host paths or internal `localfs://` URIs. Electron resolves every request, checks lexical and realpath containment, and refuses symlink escapes.
187187
- **Desktop-only:** the web app advertises `desktopCapabilities.localFilesystem` only when the Electron bridge is present. Mothership adds the `user-local/` prompt surface and per-call client routing only for that capability, including delegated and resumed work.
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-
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.
191+
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.
192192

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.
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 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.
194194

195195
## Auto-update, channels, rollout, rollback
196196

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

Lines changed: 101 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type { DesktopLocalFileRequest, SimDesktopApi } from '@sim/desktop-bridge
1616

1717
const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url))
1818

19-
test('native file tools require local consent and reuse only the approved chat and path', async () => {
19+
test('native file tools remember folder consent across chats and restarts until revoked', async () => {
2020
const root = mkdtempSync(join(tmpdir(), 'sim-native-files-e2e-'))
2121
const source = join(root, 'Reports')
2222
const outside = join(root, 'Reports-other')
@@ -28,7 +28,7 @@ test('native file tools require local consent and reuse only the approved chat a
2828
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII='
2929
writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64'))
3030
const claimed = new Set<string>()
31-
const authorizedCalls = new Set<string>()
31+
const expireAfterAuthorization = new Set<string>()
3232
let signedIn = true
3333
const calls: Record<
3434
string,
@@ -84,11 +84,11 @@ test('native file tools require local consent and reuse only the approved chat a
8484
response.writeHead(call ? 409 : 403, { 'Content-Type': 'application/json' }).end('{}')
8585
return
8686
}
87-
authorizedCalls.add(input.toolCallId)
8887
if (input.claim) claimed.add(input.toolCallId)
8988
response
9089
.writeHead(200, { 'Content-Type': 'application/json' })
9190
.end(JSON.stringify({ ...call, chatId: call.chatId ?? 'org-chat' }))
91+
if (expireAfterAuthorization.has(input.toolCallId)) calls[input.toolCallId] = undefined
9292
return
9393
}
9494
response
@@ -98,21 +98,27 @@ test('native file tools require local consent and reuse only the approved chat a
9898
? { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/' }
9999
: {}),
100100
})
101-
.end('<!doctype html><title>Local file fixture</title><h1>Local files</h1>')
101+
.end(`<!doctype html><title>Local file fixture</title><h1>Local files</h1>
102+
<button id="forget" onclick="window.simDesktop.localFilesystem({operation:'list_mounts'}).then(async result => {
103+
for (const mount of result.data.mounts) await window.simDesktop.localFilesystem({operation:'forget_mount', uri:mount.uri});
104+
this.textContent='Forgotten';
105+
})">Forget folders</button>`)
102106
})
103107
await new Promise<void>((resolve) => server?.listen(0, '127.0.0.1', resolve))
104108
const address = server.address()
105109
if (!address || typeof address === 'string') throw new Error('Missing fixture address')
106-
app = await electron.launch({
107-
args: ['.'],
108-
cwd: DESKTOP_DIR,
109-
env: {
110-
...process.env,
111-
SIM_DESKTOP_ORIGIN: `http://127.0.0.1:${address.port}`,
112-
SIM_DESKTOP_USER_DATA: join(root, 'profile'),
113-
},
114-
})
115-
const window = await app.firstWindow()
110+
const launch = () =>
111+
electron.launch({
112+
args: ['.', '--use-mock-keychain'],
113+
cwd: DESKTOP_DIR,
114+
env: {
115+
...process.env,
116+
SIM_DESKTOP_ORIGIN: `http://127.0.0.1:${address.port}`,
117+
SIM_DESKTOP_USER_DATA: join(root, 'profile'),
118+
},
119+
})
120+
app = await launch()
121+
let window = await app.firstWindow()
116122
await expect(window.getByRole('heading')).toHaveText('Local files')
117123
const invoke = (input: DesktopLocalFileRequest) =>
118124
window.evaluate(async (request) => {
@@ -135,6 +141,7 @@ test('native file tools require local consent and reuse only the approved chat a
135141
})
136142
const deniedPrompt = app.waitForEvent('window', { timeout: 10_000 })
137143
const deniedRead = invoke({ operation: 'read', toolCallId: 'text' })
144+
void deniedRead.catch(() => {})
138145
const denial = await deniedPrompt
139146
await expect(denial.getByRole('button', { name: "Don't allow", exact: true })).toBeFocused()
140147
await denial.screenshot({
@@ -147,12 +154,14 @@ test('native file tools require local consent and reuse only the approved chat a
147154

148155
const folderPrompt = app.waitForEvent('window')
149156
const folderRead = invoke({ operation: 'read', toolCallId: 'directory' })
157+
void folderRead.catch(() => {})
150158
const folderConsent = await folderPrompt
151159
const queuedRead = invoke({ operation: 'read', toolCallId: 'text' })
160+
void queuedRead.catch(() => {})
152161
expect(
153162
await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop)
154163
).toBe('undefined')
155-
await folderConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click()
164+
await folderConsent.getByRole('button', { name: 'Allow folder', exact: true }).click()
156165
expect(await folderRead).toMatchObject({ ok: true, data: { representation: 'directory' } })
157166
expect(await queuedRead).toMatchObject({ ok: true, data: { text: 'native file contents' } })
158167
const canonicalRequest = {
@@ -168,29 +177,36 @@ test('native file tools require local consent and reuse only the approved chat a
168177
ok: true,
169178
data: { observations: [{ mediaType: 'image/png', data: png }] },
170179
})
171-
const runningApp = app
172180
const requestPermission = async (request: DesktopLocalFileRequest) => {
173-
const shown = runningApp.waitForEvent('window', { timeout: 10_000 })
181+
if (!app) throw new Error('Desktop app is not running')
182+
const shown = app.waitForEvent('window', { timeout: 10_000 })
174183
const result = invoke(request)
175184
void result.catch(() => {})
176185
const prompt = await shown
177186
await expect(prompt.getByRole('button', { name: "Don't allow", exact: true })).toBeVisible()
178187
return { prompt, result }
179188
}
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+
await test.step('a folder grant works in another chat but does not permit symlink escapes', async () => {
190+
expect(await invoke({ operation: 'read', toolCallId: 'otherChat' })).toMatchObject({
191+
ok: true,
192+
data: { text: 'native file contents' },
193+
})
194+
writeFileSync(join(source, 'empty', 'new.txt'), 'new file in a subfolder')
195+
calls.nested = {
196+
toolName: 'read_local_file',
197+
args: { path: join(source, 'empty', 'new.txt') },
198+
chatId: 'another-chat',
199+
}
200+
expect(await invoke({ operation: 'read', toolCallId: 'nested' })).toMatchObject({
201+
ok: true,
202+
data: { text: 'new file in a subfolder' },
203+
})
204+
rmSync(join(source, 'empty', 'new.txt'))
189205
symlinkSync(join(outside, 'private.txt'), join(source, 'linked.txt'))
190206
calls.escape = { toolName: 'read_local_file', args: { path: join(source, 'linked.txt') } }
191207
const escapedRead = await requestPermission({ operation: 'read', toolCallId: 'escape' })
192208
await expect(escapedRead.prompt.getByRole('dialog')).toContainText(
193-
JSON.stringify(realpathSync(join(outside, 'private.txt')))
209+
JSON.stringify(realpathSync(outside))
194210
)
195211
await escapedRead.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
196212
expect(await escapedRead.result).toMatchObject({ ok: false })
@@ -202,21 +218,23 @@ test('native file tools require local consent and reuse only the approved chat a
202218
const stale = await requestPermission({ operation: 'read', toolCallId: 'stale' })
203219
if (changed) calls.stale.args.path = join(source, 'report.txt')
204220
else calls.stale = undefined
205-
await stale.prompt.getByRole('button', { name: 'Allow for this chat', exact: true }).click()
221+
await stale.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click()
206222
expect(await stale.result).toMatchObject({ ok: false })
207223
}
208224
})
209-
await test.step('a queued call is revalidated even when its folder is already approved', async () => {
225+
await test.step('an unanswered prompt does not block approved folders', async () => {
210226
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') } }
212227
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
228+
expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true })
217229
await blocker.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
218230
expect(await blocker.result).toMatchObject({ ok: false })
219-
expect(await queued).toMatchObject({ ok: false })
231+
})
232+
await test.step('cancelled calls cannot reuse an approved folder', async () => {
233+
calls.expired = { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } }
234+
expireAfterAuthorization.add('expired')
235+
expect(await invoke({ operation: 'read', toolCallId: 'expired' })).toMatchObject({
236+
ok: false,
237+
})
220238
})
221239
await test.step('replacing the proposed folder during consent does not expose its new target', async () => {
222240
const proposed = join(root, 'Proposed')
@@ -226,16 +244,10 @@ test('native file tools require local consent and reuse only the approved chat a
226244
renameSync(proposed, join(root, 'Original'))
227245
mkdirSync(proposed)
228246
writeFileSync(join(proposed, 'unapproved.txt'), 'replacement folder contents')
229-
await retargeted.prompt
230-
.getByRole('button', { name: 'Allow for this chat', exact: true })
231-
.click()
247+
await retargeted.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click()
232248
expect(await retargeted.result).toMatchObject({ ok: false })
233249
})
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
250+
const result = await invoke({ operation: 'manifest', toolCallId: 'import' })
239251
if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result))
240252
expect(result.data.targetWorkspaceId).toBe('target-workspace')
241253
expect(result.data.entries.map((entry) => entry.relativePath)).toEqual([
@@ -279,22 +291,59 @@ test('native file tools require local consent and reuse only the approved chat a
279291
ok: false,
280292
code: 'ALREADY_STARTED',
281293
})
282-
await test.step('import approval is bound to its destination workspace', async () => {
294+
await test.step('an approved folder permits imports without repeated destination prompts', async () => {
283295
calls.otherImport = {
284296
toolName: 'import_local_files',
285297
args: { path: source, targetWorkspaceId: 'other-workspace' },
286298
}
287-
const otherImport = await requestPermission({
288-
operation: 'manifest',
289-
toolCallId: 'otherImport',
299+
expect(await invoke({ operation: 'manifest', toolCallId: 'otherImport' })).toMatchObject({
300+
ok: true,
290301
})
291-
await otherImport.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
292-
expect(await otherImport.result).toMatchObject({ ok: false })
302+
})
303+
await test.step('folder permissions survive restarting the desktop app', async () => {
304+
await app?.close()
305+
app = await launch()
306+
window = await app.firstWindow()
307+
await expect(window.getByRole('heading')).toHaveText('Local files')
308+
expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true })
309+
})
310+
await test.step('forgetting a folder revokes native reads and survives restart', async () => {
311+
await window.getByRole('button', { name: 'Forget folders', exact: true }).click()
312+
await expect(window.getByRole('button', { name: 'Forgotten', exact: true })).toBeVisible()
313+
await app?.close()
314+
app = await launch()
315+
window = await app.firstWindow()
316+
await expect(window.getByRole('heading')).toHaveText('Local files')
317+
const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' })
318+
await revoked.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click()
319+
expect(await revoked.result).toMatchObject({ ok: true })
320+
})
321+
322+
await test.step('a remembered grant does not follow a replaced folder after restart', async () => {
323+
await app?.close()
324+
renameSync(source, join(root, 'Original-reports'))
325+
mkdirSync(source)
326+
writeFileSync(join(source, 'report.txt'), 'replacement contents')
327+
app = await launch()
328+
window = await app.firstWindow()
329+
await expect(window.getByRole('heading')).toHaveText('Local files')
330+
const replaced = await requestPermission({ operation: 'read', toolCallId: 'text' })
331+
await replaced.prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
332+
expect(await replaced.result).toMatchObject({ ok: false })
333+
rmSync(source, { recursive: true })
334+
renameSync(join(root, 'Original-reports'), source)
335+
const restored = await requestPermission({ operation: 'read', toolCallId: 'text' })
336+
await restored.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click()
337+
expect(await restored.result).toMatchObject({ ok: true })
293338
})
294339

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' })
340+
await test.step('sign-out revokes remembered grants before the next account session', async () => {
341+
await app?.evaluate(({ Menu }) => {
342+
const item = Menu.getApplicationMenu()
343+
?.items.flatMap((entry) => entry.submenu?.items ?? [])
344+
.find((entry) => entry.label === 'Sign Out')
345+
if (!item) throw new Error('Sign Out menu item missing')
346+
item.click()
298347
})
299348
await expect(window).toHaveURL(`http://127.0.0.1:${address.port}/login`)
300349
signedIn = true

‎apps/desktop/src/main/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,9 @@ function main(): void {
849849
allowHttpLocalhost,
850850
openSettings,
851851
openServerSettings: () => serverWindow.open(),
852+
openFolderAccess: (parent) => {
853+
if (accountDataAvailable()) localFilesystem.showAccessMenu(parent)
854+
},
852855
newWindow: () => void createAndLoadAppWindow(),
853856
newChat: () => void openMainWindowAt(newChatRoute(config.get('lastRoute'))),
854857
handleFocusedResourceShortcut: (win, shortcut) =>

0 commit comments

Comments
 (0)