Skip to content

Commit dc39947

Browse files
authored
fix(desktop): require folder permission for native file tools (#8345)
* fix(desktop): ask before accessing local files * fix(desktop): remember folder permissions across chats * fix(desktop): cancel pending file consent and recheck access * fix(desktop): enumerate approved directory descriptors * fix(desktop): reject replaced directory listings * fix(desktop): share pending folder consent decisions * fix(desktop): complete file consent and add full file access * fix(desktop): revalidate folder consent and preserve exact identities
1 parent 1ed08d8 commit dc39947

34 files changed

Lines changed: 2262 additions & 277 deletions

‎apps/desktop/README.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ src/main/ # main process (bundled to dist/main.cjs)
3838
src/preload/ # isolated renderer bridges
3939
index.ts # hosted-app contextBridge IPC bridge (dist/preload.cjs)
4040
browser/ # minimal agent-browser credential helper (dist/browser-preload.cjs)
41-
native/ # Node-API/AppKit bridge for native macOS Help docs search
41+
native/ # Node-API bridges for directory enumeration and macOS Help docs search
4242
static/ # bundled local pages (offline.html, server.html), served over sim-shell:
4343
e2e/ # Playwright _electron smoke suite
4444
```
@@ -182,13 +182,17 @@ 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-
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. 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. Concurrent requests for the same folder share one allow or deny decision. 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.
192+
193+
**Desktop settings → Local files → Full file access** bypasses folder prompts for authorized native reads and imports. It is off by default, persists across ordinary restarts, and resets on sign-out or server changes. Turning it off restores folder consent checks. Call authorization, cancellation, file identity, and resource limits still apply, and VFS access continues to use explicit mounts. A failed settings write reports an error and leaves full access disabled in the running app.
194+
195+
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. Directory enumeration uses `fdopendir` on the verified descriptor, so replacing a parent path cannot redirect the listing. Listings scan at most 1,001 entries and return up to 1,000 sorted names with an explicit truncation flag; the cap bounds both memory and filesystem work, rather than promising the globally first 1,000 names in an arbitrarily large directory. Imports reject truncated listings. 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.
192196

193197
## Auto-update, channels, rollout, rollback
194198

‎apps/desktop/e2e/background-executor.spec.ts‎

Lines changed: 179 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
1+
import {
2+
existsSync,
3+
mkdirSync,
4+
mkdtempSync,
5+
readFileSync,
6+
renameSync,
7+
rmSync,
8+
writeFileSync,
9+
} from 'node:fs'
210
import { tmpdir } from 'node:os'
311
import { join } from 'node:path'
412
import { type ElectronApplication, expect, test } from '@playwright/test'
@@ -104,7 +112,9 @@ test.describe('background executor', () => {
104112
args: { command: `sleep 1; echo B-${n} >> '${marker}'`, waitSeconds: 30 },
105113
})
106114
)
115+
const readConsent = launched.app.waitForEvent('window')
107116
const localRead = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
117+
await (await readConsent).getByRole('button', { name: 'Allow folder', exact: true }).click()
108118

109119
await window.goto(`${sim.origin}/workspace/ws-other/home`)
110120
await window.reload()
@@ -157,6 +167,171 @@ test.describe('background executor', () => {
157167
})
158168
})
159169

170+
test('background file reads require consent and Stop cancels pending permission', async () => {
171+
const userData = mkdtempSync(join(tmpdir(), 'sim-executor-consent-'))
172+
const launched = await launch(sim, userData)
173+
app = launched.app
174+
const deviceId = await registeredDevice(sim)
175+
const readable = join(userData, 'private.txt')
176+
writeFileSync(readable, 'background consent fixture')
177+
178+
await check('background read stays pending until folder consent', async () => {
179+
const shown = launched.app.waitForEvent('window', { timeout: 10_000 })
180+
const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
181+
const prompt = await shown
182+
await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible()
183+
expect(sim.requireCall(call).completions).toHaveLength(0)
184+
await prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
185+
const completion = await settled(sim, call)
186+
expect(completion.status).toBe('error')
187+
expect(JSON.stringify(completion)).not.toContain('background consent fixture')
188+
})
189+
190+
await check('Stop dismisses background consent without granting access', async () => {
191+
const shown = launched.app.waitForEvent('window', { timeout: 10_000 })
192+
const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
193+
const prompt = await shown
194+
await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible()
195+
sim.stopCall(call)
196+
await expect.poll(() => prompt.isClosed()).toBe(true)
197+
await settled(sim, call)
198+
expect(sim.requireCall(call).completions[0]?.outcome).toBe('superseded')
199+
})
200+
201+
await check('approved background reads reuse the shared folder grant', async () => {
202+
const shown = launched.app.waitForEvent('window', { timeout: 10_000 })
203+
const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
204+
const prompt = await shown
205+
await prompt.getByRole('button', { name: 'Allow folder', exact: true }).click()
206+
expect((await settled(sim, call)).status).toBe('success')
207+
const next = sim.issue(deviceId, CHAT_A, 'read_local_file', { path: readable })
208+
expect(JSON.stringify((await settled(sim, next)).data)).toContain(
209+
'background consent fixture'
210+
)
211+
})
212+
})
213+
214+
test('background consent cannot grant access when Sim cannot verify the call', async () => {
215+
const userData = mkdtempSync(join(tmpdir(), 'sim-executor-offline-consent-'))
216+
const launched = await launch(sim, userData)
217+
app = launched.app
218+
const deviceId = await registeredDevice(sim)
219+
const readable = join(userData, 'private.txt')
220+
writeFileSync(readable, 'offline consent fixture')
221+
222+
await check('offline approval returns an error without remembering a grant', async () => {
223+
const shown = launched.app.waitForEvent('window')
224+
const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
225+
const prompt = await shown
226+
await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible()
227+
sim.disconnect()
228+
await prompt.getByRole('button', { name: 'Allow folder', exact: true }).click()
229+
await expect
230+
.poll(() =>
231+
launched.app.evaluate(({ safeStorage }, toolCallId) => {
232+
const fs = process.getBuiltinModule('node:fs')
233+
const path = `${process.env.SIM_DESKTOP_USER_DATA}/desktop-executor-journal.json`
234+
const envelope = JSON.parse(fs.readFileSync(path, 'utf8'))
235+
const journal = JSON.parse(
236+
safeStorage.decryptString(Buffer.from(envelope.ciphertext, 'base64'))
237+
) as { entries: { toolCallId: string; state: string }[] }
238+
return journal.entries.find((entry) => entry.toolCallId === toolCallId)?.state
239+
}, call)
240+
)
241+
.toBe('result')
242+
sim.reconnect()
243+
const completion = await settled(sim, call)
244+
expect(completion.status).toBe('error')
245+
expect(JSON.stringify(completion)).not.toContain('offline consent fixture')
246+
const nextPrompt = launched.app.waitForEvent('window')
247+
const next = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
248+
await (await nextPrompt).getByRole('button', { name: "Don't allow", exact: true }).click()
249+
expect((await settled(sim, next)).status).toBe('error')
250+
})
251+
})
252+
253+
test('background consent does not reopen the main app after its windows are closed', async () => {
254+
test.skip(process.platform !== 'darwin', 'The macOS app remains running without a window.')
255+
const userData = mkdtempSync(join(tmpdir(), 'sim-executor-windowless-consent-'))
256+
const launched = await launch(sim, userData)
257+
app = launched.app
258+
const deviceId = await registeredDevice(sim)
259+
const readable = join(userData, 'private.txt')
260+
writeFileSync(readable, 'windowless consent fixture')
261+
await launched.window.close()
262+
263+
await check('only the standalone consent window opens for a background read', async () => {
264+
const shown = launched.app.waitForEvent('window')
265+
const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
266+
const prompt = await shown
267+
await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible()
268+
expect(
269+
await launched.app.evaluate(({ BrowserWindow }) =>
270+
BrowserWindow.getAllWindows().map((window) => window.getParentWindow() === null)
271+
)
272+
).toEqual([true])
273+
await prompt.getByRole('button', { name: "Don't allow", exact: true }).click()
274+
expect((await settled(sim, call)).status).toBe('error')
275+
})
276+
})
277+
278+
test('sign-out clears folder grants even when desktop settings cannot be saved', async () => {
279+
const userData = mkdtempSync(join(tmpdir(), 'sim-executor-grant-cleanup-'))
280+
const launched = await launch(sim, userData)
281+
app = launched.app
282+
const deviceId = await registeredDevice(sim)
283+
const readable = join(userData, 'private.txt')
284+
writeFileSync(readable, 'cleanup consent fixture')
285+
const shown = launched.app.waitForEvent('window')
286+
const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable })
287+
await (await shown).getByRole('button', { name: 'Allow folder', exact: true }).click()
288+
expect((await settled(sim, call)).status).toBe('success')
289+
const grants = join(userData, 'local-filesystem-grants.json')
290+
expect(existsSync(grants)).toBe(true)
291+
await launched.window.evaluate(() => {
292+
const button = document.createElement('button')
293+
button.textContent = 'Enable full file access'
294+
button.onclick = async () => {
295+
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
296+
await api.settings.setFullFileAccess?.(true)
297+
button.textContent = 'Full file access enabled'
298+
}
299+
document.body.append(button)
300+
})
301+
await launched.window
302+
.getByRole('button', { name: 'Enable full file access', exact: true })
303+
.click()
304+
await expect(
305+
launched.window.getByRole('button', { name: 'Full file access enabled', exact: true })
306+
).toBeVisible()
307+
const settings = join(userData, 'settings.json')
308+
if (existsSync(settings)) renameSync(settings, `${settings}.backup`)
309+
mkdirSync(settings)
310+
311+
try {
312+
await check(
313+
'failed settings persistence does not skip independent grant cleanup',
314+
async () => {
315+
const failedSignOut = launched.app.waitForEvent('window')
316+
await launched.app.evaluate(({ Menu }) => {
317+
const item = Menu.getApplicationMenu()
318+
?.items.flatMap((entry) => entry.submenu?.items ?? [])
319+
.find((entry) => entry.label === 'Sign Out')
320+
if (!item) throw new Error('Missing Sign Out menu item')
321+
item.click()
322+
})
323+
const failure = await failedSignOut
324+
await failure.getByRole('button', { name: 'OK', exact: true }).click()
325+
expect(existsSync(join(userData, 'account-data-teardown-required.json'))).toBe(true)
326+
await expect.poll(() => existsSync(grants)).toBe(false)
327+
}
328+
)
329+
} finally {
330+
rmSync(settings, { recursive: true, force: true })
331+
if (existsSync(`${settings}.backup`)) renameSync(`${settings}.backup`, settings)
332+
}
333+
})
334+
160335
test('B: a result produced while the network is cut is delivered once after reconnecting', async () => {
161336
const userData = mkdtempSync(join(tmpdir(), 'sim-executor-b-'))
162337
app = (await launch(sim, userData)).app
@@ -237,12 +412,15 @@ test.describe('background executor', () => {
237412
writeFileSync(join(source, 'q3', 'export.bin'), large)
238413
await launched.window.goto(`${sim.origin}/workspace/${WORKSPACE}/chat/${CHAT_C}`)
239414

415+
const importConsent = launched.app.waitForEvent('window')
240416
const call = sim.issue(deviceId, CHAT_B, 'import_local_files', {
241417
path: source,
242418
targetWorkspaceId: WORKSPACE,
243419
folderId: 'folder-e2e',
244420
})
245421

422+
await (await importConsent).getByRole('button', { name: 'Allow folder', exact: true }).click()
423+
246424
await check('D: the import completes with every entry it stored', async () => {
247425
const completion = await settled(sim, call, 60_000)
248426
expect(completion.status).toBe('success')

‎apps/desktop/e2e/browser-focus.spec.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,33 @@ test('browser focus and shortcuts stay with the surface the user is using', asyn
308308
await clickMenu('New Tab')
309309
await expect.poll(tabCount).toBe(before + 1)
310310
})
311+
await check(
312+
'a revealed page resumes throttling in its own chat after switching chats',
313+
async () => {
314+
await panelAction({ action: 'switch-tab', tabId: '1' })
315+
await shell.evaluate(async (scope) => {
316+
const api = (globalThis as Bridge).simDesktop.browserAgent
317+
api.setPanelBounds(
318+
{ x: 0, y: 120, width: innerWidth, height: innerHeight - 120 },
319+
null,
320+
scope
321+
)
322+
await api.activateScope('browser-focus-other-chat')
323+
}, SCOPE)
324+
await expect
325+
.poll(() =>
326+
shellApp.evaluate(
327+
({ webContents }, url) =>
328+
webContents
329+
.getAllWebContents()
330+
.find((contents) => contents.getURL() === url)
331+
?.getBackgroundThrottling(),
332+
`${site}/five`
333+
)
334+
)
335+
.toBe(true)
336+
}
337+
)
311338
passed = true
312339
} finally {
313340
mkdirSync(dirname(reportPath), { recursive: true })

‎apps/desktop/e2e/desktop-tools-live-sim.spec.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,12 @@ test.describe('desktop tools against a live Sim', () => {
245245
})
246246
const page = await app.firstWindow({ timeout })
247247
pageErrors = []
248+
app.on('window', (permission) => {
249+
void permission
250+
.getByRole('button', { name: 'Allow folder', exact: true })
251+
.click({ timeout: 10_000 })
252+
.catch((error) => pageErrors.push(`Folder approval failed: ${String(error)}`))
253+
})
248254
page.on('pageerror', (error) => pageErrors.push(error.message))
249255
page.on('console', (message) => {
250256
if (message.type() === 'error') pageErrors.push(message.text())

0 commit comments

Comments
 (0)