From 65f729c417e1f073e62b04d70826e64fb8a3446c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 26 Sep 2026 14:48:43 -0700 Subject: [PATCH 1/8] fix(desktop): ask before accessing local files --- apps/desktop/README.md | 4 +- apps/desktop/e2e/local-files.spec.ts | 201 ++++++++++++++++-- apps/desktop/src/main/ipc.test.ts | 65 ------ apps/desktop/src/main/ipc.ts | 47 +++- .../src/main/local-file-permissions.ts | 175 +++++++++++++++ apps/desktop/src/main/local-files.test.ts | 15 +- apps/desktop/src/main/local-files.ts | 83 +++++--- packages/desktop-bridge/src/local-files.ts | 2 +- 8 files changed, 479 insertions(+), 113 deletions(-) create mode 100644 apps/desktop/src/main/local-file-permissions.ts diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 4d0f879d94b..ccbe6925687 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -188,7 +188,9 @@ 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. -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. +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. + +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. ## Auto-update, channels, rollout, rollback diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 0a9a08f0678..7093e3c9d6c 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -1,4 +1,12 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { + mkdirSync, + mkdtempSync, + realpathSync, + renameSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' import { createServer, type Server } from 'node:http' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -8,18 +16,31 @@ import type { DesktopLocalFileRequest, SimDesktopApi } from '@sim/desktop-bridge const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url)) -test('native file tools read and import through the installed preload without Sim folder grants', async () => { +test('native file tools require local consent and reuse only the approved chat and path', async () => { const root = mkdtempSync(join(tmpdir(), 'sim-native-files-e2e-')) const source = join(root, 'Reports') + const outside = join(root, 'Reports-other') + mkdirSync(outside) + writeFileSync(join(outside, 'private.txt'), 'outside contents') mkdirSync(join(source, 'empty'), { recursive: true }) writeFileSync(join(source, 'report.txt'), 'native file contents') const png = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII=' writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64')) - /** Calls the server saw claimed; like the server, only an import refuses a second claim. */ const claimed = new Set() - const calls: Record }> = { + const authorizedCalls = new Set() + let signedIn = true + const calls: Record< + string, + { toolName: string; args: Record; chatId?: string } | undefined + > = { + directory: { toolName: 'read_local_file', args: { path: source } }, text: { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } }, + otherChat: { + toolName: 'read_local_file', + args: { path: join(source, 'report.txt') }, + chatId: 'other-chat', + }, image: { toolName: 'read_local_file', args: { path: join(source, 'image.png') } }, import: { toolName: 'import_local_files', @@ -33,13 +54,27 @@ test('native file tools read and import through the installed preload without Si const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname if (path === '/api/auth/get-session') { response.writeHead(200, { 'Content-Type': 'application/json' }).end( - JSON.stringify({ - user: { id: 'local-file-user' }, - session: { id: 'local-file-session' }, - }) + JSON.stringify( + signedIn + ? { + user: { id: 'local-file-user' }, + session: { id: 'local-file-session' }, + } + : null + ) ) return } + if (path === '/api/auth/sign-out') { + signedIn = false + response + .writeHead(200, { + 'Content-Type': 'application/json', + 'Set-Cookie': 'better-auth.session_token=; HttpOnly; SameSite=Lax; Path=/; Max-Age=0', + }) + .end('{}') + return + } if (path === '/api/desktop/tool/authorize') { let body = '' for await (const chunk of request) body += chunk.toString() @@ -52,16 +87,19 @@ test('native file tools read and import through the installed preload without Si response.writeHead(call ? 409 : 403, { 'Content-Type': 'application/json' }).end('{}') return } + authorizedCalls.add(input.toolCallId) if (input.claim) claimed.add(input.toolCallId) response .writeHead(200, { 'Content-Type': 'application/json' }) - .end(JSON.stringify({ ...call, chatId: 'org-chat' })) + .end(JSON.stringify({ ...call, chatId: call.chatId ?? 'org-chat' })) return } response .writeHead(200, { 'Content-Type': 'text/html', - 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/', + ...(signedIn + ? { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/' } + : {}), }) .end('Local file fixture

Local files

') }) @@ -86,9 +124,46 @@ test('native file tools read and import through the installed preload without Si return api.localFiles(request) }, input) await expect - .poll(async () => (await invoke({ operation: 'read', toolCallId: 'text' })).ok) + .poll(() => + window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + return (await api.localFilesystem?.({ operation: 'list_mounts' }))?.ok + }) + ) .toBe(true) - expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ + await window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + await api.settings.setPreference('browserEnabled', false) + await api.settings.setPreference('terminalEnabled', false) + }) + const deniedPrompt = app.waitForEvent('window', { timeout: 10_000 }) + const deniedRead = invoke({ operation: 'read', toolCallId: 'text' }) + const denial = await deniedPrompt + await expect(denial.getByRole('button', { name: "Don't allow", exact: true })).toBeFocused() + await denial.screenshot({ + path: + process.env.DESKTOP_LOCAL_FILES_REPORT_PATH ?? + test.info().outputPath('local-file-consent.png'), + }) + await denial.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await deniedRead).toMatchObject({ ok: false }) + + const folderPrompt = app.waitForEvent('window') + const folderRead = invoke({ operation: 'read', toolCallId: 'directory' }) + const folderConsent = await folderPrompt + const queuedRead = invoke({ operation: 'read', toolCallId: 'text' }) + expect( + await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop) + ).toBe('undefined') + await folderConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click() + expect(await folderRead).toMatchObject({ ok: true, data: { representation: 'directory' } }) + expect(await queuedRead).toMatchObject({ ok: true, data: { text: 'native file contents' } }) + const canonicalRequest = { + operation: 'read' as const, + toolCallId: 'text', + path: join(outside, 'private.txt'), + } + expect(await invoke(canonicalRequest)).toMatchObject({ ok: true, data: { representation: 'text', text: 'native file contents' }, }) @@ -96,8 +171,74 @@ test('native file tools read and import through the installed preload without Si ok: true, data: { observations: [{ mediaType: 'image/png', data: png }] }, }) - expect([...claimed]).toEqual(['text', 'image']) - const result = await invoke({ operation: 'manifest', toolCallId: 'import' }) + const runningApp = app + const requestPermission = async (request: DesktopLocalFileRequest) => { + const shown = runningApp.waitForEvent('window', { timeout: 10_000 }) + const result = invoke(request) + void result.catch(() => {}) + const prompt = await shown + await expect(prompt.getByRole('button', { name: "Don't allow", exact: true })).toBeVisible() + return { prompt, result } + } + await test.step('a folder grant does not authorize another chat or a symlink escape', async () => { + const otherChat = await requestPermission({ operation: 'read', toolCallId: 'otherChat' }) + const dismissed = otherChat.prompt.waitForEvent('close') + await otherChat.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .press('Escape') + .catch(() => {}) + await dismissed + expect(await otherChat.result).toMatchObject({ ok: false }) + symlinkSync(join(outside, 'private.txt'), join(source, 'linked.txt')) + calls.escape = { toolName: 'read_local_file', args: { path: join(source, 'linked.txt') } } + const escapedRead = await requestPermission({ operation: 'read', toolCallId: 'escape' }) + await expect(escapedRead.prompt.getByRole('dialog')).toContainText( + JSON.stringify(realpathSync(join(outside, 'private.txt'))) + ) + await escapedRead.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await escapedRead.result).toMatchObject({ ok: false }) + rmSync(join(source, 'linked.txt')) + }) + await test.step('cancelled calls and changed arguments cannot acquire a grant', async () => { + for (const changed of [false, true]) { + calls.stale = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } } + const stale = await requestPermission({ operation: 'read', toolCallId: 'stale' }) + if (changed) calls.stale.args.path = join(source, 'report.txt') + else calls.stale = undefined + await stale.prompt.getByRole('button', { name: 'Allow for this chat', exact: true }).click() + expect(await stale.result).toMatchObject({ ok: false }) + } + }) + await test.step('a queued call is revalidated even when its folder is already approved', async () => { + calls.blocker = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } } + calls.queued = { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } } + const blocker = await requestPermission({ operation: 'read', toolCallId: 'blocker' }) + const queued = invoke({ operation: 'read', toolCallId: 'queued' }) + void queued.catch(() => {}) + await expect.poll(() => authorizedCalls.has('queued')).toBe(true) + calls.queued = undefined + await blocker.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await blocker.result).toMatchObject({ ok: false }) + expect(await queued).toMatchObject({ ok: false }) + }) + await test.step('replacing the proposed folder during consent does not expose its new target', async () => { + const proposed = join(root, 'Proposed') + mkdirSync(proposed) + calls.retargeted = { toolName: 'read_local_file', args: { path: proposed } } + const retargeted = await requestPermission({ operation: 'read', toolCallId: 'retargeted' }) + renameSync(proposed, join(root, 'Original')) + mkdirSync(proposed) + writeFileSync(join(proposed, 'unapproved.txt'), 'replacement folder contents') + await retargeted.prompt + .getByRole('button', { name: 'Allow for this chat', exact: true }) + .click() + expect(await retargeted.result).toMatchObject({ ok: false }) + }) + const importPrompt = app.waitForEvent('window') + const importing = invoke({ operation: 'manifest', toolCallId: 'import' }) + const importConsent = await importPrompt + await importConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click() + const result = await importing if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result)) expect(result.data.targetWorkspaceId).toBe('target-workspace') expect(result.data.entries.map((entry) => entry.relativePath)).toEqual([ @@ -141,6 +282,38 @@ test('native file tools read and import through the installed preload without Si ok: false, code: 'ALREADY_STARTED', }) + await test.step('import approval is bound to its destination workspace', async () => { + calls.otherImport = { + toolName: 'import_local_files', + args: { path: source, targetWorkspaceId: 'other-workspace' }, + } + const otherImport = await requestPermission({ + operation: 'manifest', + toolCallId: 'otherImport', + }) + await otherImport.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await otherImport.result).toMatchObject({ ok: false }) + }) + + await test.step('sign-out revokes chat grants before the next account session', async () => { + await window.evaluate(async () => { + await fetch('/api/auth/sign-out', { method: 'POST' }) + }) + await expect(window).toHaveURL(`http://127.0.0.1:${address.port}/login`) + signedIn = true + await window.goto(`http://127.0.0.1:${address.port}/`) + await expect + .poll(() => + window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + return (await api.localFilesystem?.({ operation: 'list_mounts' }))?.ok + }) + ) + .toBe(true) + const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await revoked.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await revoked.result).toMatchObject({ ok: false }) + }) } finally { await app?.close() server?.close() diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 0c037b5a5b0..14ea1e16483 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -462,71 +462,6 @@ describe('registerIpcHandlers', () => { }) }) - it('reads a native file through canonical IPC arguments without folder grants or user activation', async () => { - const { invoke } = collectHandlers() - const handler = invoke.get('desktop:local-files') - const path = fileURLToPath(import.meta.url) - const fetchAuthorization = vi.fn(async () => - Response.json({ chatId: 'chat-1', toolName: 'read_local_file', args: { path, limit: 64 } }) - ) - const authorizedEvent = { - senderFrame: { url: `${APP}/o/org/home` }, - sender: { session: { fetch: fetchAuthorization } }, - } - const mounts = vi.spyOn(deps.localFilesystem, 'handle') - expect( - await handler?.(authorizedEvent, { - operation: 'read', - toolCallId: 'tool-native', - path: '/not/the/canonical/path', - }) - ).toMatchObject({ - ok: true, - data: { kind: 'read', path, text: readFileSync(path, 'utf8').slice(0, 64) }, - }) - expect(mounts).not.toHaveBeenCalled() - expect(fetchAuthorization).toHaveBeenCalledWith( - `${APP}/api/desktop/tool/authorize`, - expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native', claim: true }) }) - ) - expect( - await handler?.(evilEvent, { operation: 'read', toolCallId: 'tool-native' }) - ).toMatchObject({ ok: false }) - }) - - it('claims native imports at IPC before traversal and rejects a replay', async () => { - const { invoke } = collectHandlers() - const handler = invoke.get('desktop:local-files') - const fetchAuthorization = vi - .fn() - .mockResolvedValueOnce( - Response.json({ - chatId: 'chat-1', - toolName: 'import_local_files', - args: { path: fileURLToPath(import.meta.url), targetWorkspaceId: 'workspace' }, - }) - ) - .mockResolvedValueOnce(Response.json({ error: 'already started' }, { status: 409 })) - const event = { - senderFrame: { url: `${APP}/o/org/home` }, - sender: { session: { fetch: fetchAuthorization } }, - } - const request = { operation: 'manifest', toolCallId: 'tool-import' } - expect(await handler?.(event, request)).toMatchObject({ - ok: true, - data: { - kind: 'manifest', - targetWorkspaceId: 'workspace', - entries: [{ relativePath: '', kind: 'file' }], - }, - }) - expect(fetchAuthorization).toHaveBeenCalledWith( - `${APP}/api/desktop/tool/authorize`, - expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-import', claim: true }) }) - ) - expect(await handler?.(event, request)).toMatchObject({ ok: false, code: 'ALREADY_STARTED' }) - }) - it('requires server authorization for every privileged filesystem tool request', async () => { const { invoke } = collectHandlers() const handler = invoke.get('desktop:local-filesystem') diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 41d9b409650..84dea6fdf31 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -1,3 +1,4 @@ +import { isDeepStrictEqual } from 'node:util' import { BROWSER_TOOL_AUTHORIZATION_TIMEOUT_MS, type BrowserPanelAction, @@ -32,6 +33,10 @@ import { isRecordLike, toRecord } from '@sim/utils/object' import { PASTE_LIMITS, utf8ByteLength } from '@sim/utils/paste' import type { BrowserWindow, IpcMainEvent, IpcMainInvokeEvent, WebContents } from 'electron' import { clipboard, ipcMain, shell } from 'electron' +import { + captureAccountDataGeneration, + isAccountDataGenerationCurrent, +} from '@/main/account-data-generation' import { type BrowserToolQueueBoundary, cancelActiveTool, @@ -83,6 +88,7 @@ import { isSafeInternalPath } from '@/main/config' import type { DesktopSettingsService } from '@/main/desktop-settings' import { isDesktopPreferenceKey } from '@/main/desktop-settings' import { hasRecentDeliberateInput, hasRecentDiscreteInput } from '@/main/input-activity' +import { type LocalFileAccess, LocalFilePermissions } from '@/main/local-file-permissions' import { executeLocalFileRequest } from '@/main/local-files' import type { LocalFilesystemService } from '@/main/local-filesystem' import { isAppOrigin, openExternalSafe } from '@/main/navigation' @@ -607,6 +613,7 @@ async function authorizeLocalFilesystemTool( * unvalidated args they must parse themselves. */ export function registerIpcHandlers(deps: IpcDeps): void { + const localFilePermissions = new LocalFilePermissions() const browserScopeBySender = new WeakMap() const terminalScopeBySender = new WeakMap() const browserPendingScopesBySender = new WeakMap>() @@ -766,8 +773,12 @@ export function registerIpcHandlers(deps: IpcDeps): void { requiresAccountData: true, passSender: true, denied: { ok: false, error: 'Local file tools are unavailable from this page.' }, - handler: (_sender, request, authorization) => - executeLocalFileRequest(request, authorization as DesktopToolAuthorization), + handler: (_sender, request, authorization, access) => + executeLocalFileRequest( + request, + authorization as DesktopToolAuthorization, + access as LocalFileAccess + ), }, 'desktop:local-filesystem': { kind: 'invoke', @@ -2133,6 +2144,8 @@ export function registerIpcHandlers(deps: IpcDeps): void { } } if (channel === 'desktop:local-files') { + 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 @@ -2156,7 +2169,35 @@ export function registerIpcHandlers(deps: IpcDeps): void { !['read_local_file', 'import_local_files'].includes(authorization.toolName) ) return { ok: false, error: 'This is not an authorized pending local file tool call.' } - handlerArgs = [request, authorization] + 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.' } + try { + const access = await localFilePermissions.authorize(authorization, { + parent, + origin, + generation, + isCurrent: () => + isAccountDataGenerationCurrent(generation) && + deps.accountDataAvailable() && + deps.appOrigin() === origin && + isAppOriginSender(event, origin), + revalidate: async () => + isDeepStrictEqual( + authorization, + await fetchDesktopToolAuthorization(event, deps, request.toolCallId) + ), + }) + handlerArgs = [request, authorization, access] + } catch (error) { + return { ok: false, error: getErrorMessage(error) } + } } if (spec.passSender) { handlerArgs = [event.sender, ...handlerArgs] diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts new file mode 100644 index 00000000000..0e3b1338ff9 --- /dev/null +++ b/apps/desktop/src/main/local-file-permissions.ts @@ -0,0 +1,175 @@ +import { lstat, realpath, stat } from 'node:fs/promises' +import { homedir } from 'node:os' +import { isAbsolute, join, relative, resolve, sep } from 'node:path' +import { isDesktopScopeId } from '@sim/desktop-bridge' +import type { BrowserWindow } from 'electron' +import { showShellDialog } from '@/main/dialogs' +import type { LocalFileAuthorization } from '@/main/local-files' + +const MAX_GRANTS = 256 +const MAX_PENDING_REQUESTS = 32 + +interface LocalFilePermissionContext { + parent: BrowserWindow + origin: string + generation: number + isCurrent: () => boolean + revalidate: () => Promise +} + +interface LocalFileGrant { + scope: string + path: string + directory: boolean + dev: number + ino: number +} + +export interface LocalFileAccess { + path: string + resolve: (path: string) => Promise +} + +function nativePath(value: unknown): string { + if (typeof value !== 'string' || value.length > 4096 || value.includes('\0')) + throw new Error('A native absolute path or ~/ path is required.') + const path = + value === '~' ? homedir() : value.startsWith('~/') ? join(homedir(), value.slice(2)) : value + if (!isAbsolute(path)) throw new Error('Use an absolute path or ~/ path.') + return resolve(path) +} + +function contains(grant: LocalFileGrant, path: string): boolean { + if (path === grant.path) return true + if (!grant.directory) return false + const rel = relative(grant.path, path) + return !isAbsolute(rel) && rel !== '..' && !rel.startsWith(`..${sep}`) +} + +function assertCurrent(context: LocalFilePermissionContext): void { + if (context.parent.isDestroyed() || !context.isCurrent()) + throw new Error('This local file request expired. Ask again in the current chat.') +} + +/** Chat grants live only in this desktop session and are never writable by the hosted renderer. */ +export class LocalFilePermissions { + private grants: LocalFileGrant[] = [] + private generation = -1 + private queue: Promise = Promise.resolve() + private pending = 0 + + async authorize( + authorization: LocalFileAuthorization & { chatId: string }, + context: LocalFilePermissionContext + ): Promise { + if (this.pending >= MAX_PENDING_REQUESTS) + throw new Error('Too many local file requests are waiting for permission. Try again later.') + const wasQueued = this.pending > 0 + this.pending++ + const pending = this.queue.then(() => this.authorizeNext(authorization, context, wasQueued)) + this.queue = pending.then( + () => undefined, + () => undefined + ) + try { + return await pending + } finally { + this.pending-- + } + } + + private async authorizeNext( + authorization: LocalFileAuthorization & { chatId: string }, + context: LocalFilePermissionContext, + wasQueued: boolean + ): Promise { + assertCurrent(context) + if (this.generation !== context.generation) { + this.grants = [] + this.generation = context.generation + } + const importing = authorization.toolName === 'import_local_files' + if ( + importing && + (!isDesktopScopeId(authorization.args.targetWorkspaceId) || + (authorization.args.folderId !== undefined && + !isDesktopScopeId(authorization.args.folderId))) + ) + throw new Error('A valid destination workspace and folder are required for imports.') + const scope = JSON.stringify([ + context.origin, + authorization.chatId, + authorization.toolName, + ...(importing ? [authorization.args.targetWorkspaceId, authorization.args.folderId] : []), + ]) + const path = await realpath(nativePath(authorization.args.path)) + const info = await stat(path) + if (!info.isFile() && !info.isDirectory()) + throw new Error('The path is not a regular file or directory.') + let grant = this.grants.find((entry) => entry.scope === scope && contains(entry, path)) + if (grant) { + if (wasQueued && !(await context.revalidate())) + throw new Error('This local file tool call is no longer pending or its arguments changed.') + const root = await lstat(grant.path) + if (root.dev !== grant.dev || root.ino !== grant.ino || root.isSymbolicLink()) { + this.grants = this.grants.filter((entry) => entry !== grant) + grant = undefined + } + } + if (!grant) { + if (this.grants.length >= MAX_GRANTS) + throw new Error( + 'Restart Sim to clear this session’s local file permissions before adding more.' + ) + assertCurrent(context) + const kind = info.isDirectory() ? 'folder' : 'file' + const displayedPath = JSON.stringify(path).replace( + /[\u202a-\u202e\u2066-\u2069]/g, + (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}` + ) + const result = await showShellDialog(context.parent, { + title: importing ? `Import this ${kind}?` : `Read this ${kind}?`, + message: displayedPath, + detail: [ + importing + ? `Sim will upload this ${kind}${info.isDirectory() ? ' and its contents' : ''} to your workspace on ${context.origin}.` + : `Sim will read this ${kind}${info.isDirectory() ? ' and its contents' : ''} and send the results to ${context.origin} for this chat.`, + ...(importing ? [`Destination workspace: ${authorization.args.targetWorkspaceId}`] : []), + 'This permission applies only to this chat and ends when Sim closes.', + ].join('\n\n'), + buttons: ['Allow for this chat', "Don't allow"], + defaultId: 1, + cancelId: 1, + }) + assertCurrent(context) + if (result.response !== 0) throw new Error('The user did not allow this local file access.') + if (!(await context.revalidate())) + throw new Error('This local file tool call is no longer pending or its arguments changed.') + grant = { scope, path, directory: info.isDirectory(), dev: info.dev, ino: info.ino } + await this.resolve(grant, path, context) + this.grants.push(grant) + } + await this.resolve(grant, path, context) + const approvedGrant = grant + return { path, resolve: (candidate) => this.resolve(approvedGrant, candidate, context) } + } + + private async resolve( + grant: LocalFileGrant, + candidate: string, + context: LocalFilePermissionContext + ): Promise { + assertCurrent(context) + const root = await lstat(grant.path) + if ( + root.dev !== grant.dev || + root.ino !== grant.ino || + (grant.directory ? !root.isDirectory() : !root.isFile()) + ) + throw new Error('The approved file or folder changed. Ask again to request access.') + const path = await realpath(candidate) + if (!contains(grant, path)) throw new Error('This path is outside the approved file or folder.') + assertCurrent(context) + return path + } +} diff --git a/apps/desktop/src/main/local-files.test.ts b/apps/desktop/src/main/local-files.test.ts index 1e01abebfd7..a7dd2c87d07 100644 --- a/apps/desktop/src/main/local-files.test.ts +++ b/apps/desktop/src/main/local-files.test.ts @@ -1,8 +1,19 @@ -import { mkdir, mkdtemp, rm, symlink, truncate, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, symlink, truncate, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, expect, it } from 'vitest' -import { executeLocalFileRequest } from '@/main/local-files' +import { + executeLocalFileRequest as executeApprovedLocalFileRequest, + type LocalFileAuthorization, +} from '@/main/local-files' + +/** Parser and import invariants run with explicit fixture access; consent is covered through Electron. */ +function executeLocalFileRequest(request: unknown, authorization: LocalFileAuthorization) { + return executeApprovedLocalFileRequest(request, authorization, { + path: String(authorization.args.path), + resolve: realpath, + }) +} let root: string beforeEach(async () => { diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index a4b9cd9a42c..10cfa6674e5 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -1,5 +1,5 @@ -import { open, readdir, realpath, stat } from 'node:fs/promises' -import { homedir } from 'node:os' +import { constants } from 'node:fs' +import { lstat, open, readdir, stat } from 'node:fs/promises' import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' import type { DesktopLocalFileEntry, @@ -11,6 +11,7 @@ import { MAX_DESKTOP_IMPORT_FILE_BYTES } from '@sim/desktop-bridge' import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' import { PDFDocument } from 'pdf-lib' +import type { LocalFileAccess } from '@/main/local-file-permissions' const CHUNK_BYTES = 8 * 1024 * 1024 const MAX_ENTRIES = 1000 @@ -21,16 +22,6 @@ export interface LocalFileAuthorization { args: Record } -/** Resolve normal native paths; macOS, not Sim folder grants, owns filesystem access. */ -function nativePath(value: unknown): string { - if (typeof value !== 'string' || value.length > 4096 || value.includes('\0')) - throw new Error('A native absolute path or ~/ path is required.') - const path = - value === '~' ? homedir() : value.startsWith('~/') ? join(homedir(), value.slice(2)) : value - if (!isAbsolute(path)) throw new Error('Use an absolute path or ~/ path.') - return resolve(path) -} - function revision(info: Awaited>): string { return `${info.dev}:${info.ino}:${info.size}:${info.mtimeMs}` } @@ -48,7 +39,35 @@ function assertImportPath(root: string, candidate: string): void { throw new Error('The file is outside this import source.') } -async function inspect(path: string, args: Record): Promise { +async function openApprovedFile(path: string, access: LocalFileAccess) { + const canonical = await access.resolve(path) + const file = await open( + canonical, + constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK + ) + try { + const info = await file.stat() + const verified = await access.resolve(path) + const current = await lstat(verified) + if ( + !info.isFile() || + canonical !== verified || + info.dev !== current.dev || + info.ino !== current.ino + ) + throw new Error('The local file changed while it was being opened. Try again.') + return file + } catch (error) { + await file.close() + throw error + } +} + +async function inspect( + path: string, + args: Record, + access: LocalFileAccess +): Promise { const info = await stat(path) if (info.isDirectory()) { const entries = await readdir(path, { withFileTypes: true }) @@ -73,8 +92,9 @@ async function inspect(path: string, args: Record): Promise): Promise + args: Record, + access: LocalFileAccess ): Promise { if (typeof args.targetWorkspaceId !== 'string') throw new Error('A target workspace is required.') - const root = await realpath(path) + const root = await access.resolve(path) const entries: DesktopLocalFileEntry[] = [] async function walk(current: string, ancestors: ReadonlySet): Promise { if (entries.length >= MAX_ENTRIES) throw new Error( 'The directory exceeds 1,000 entries. Import smaller subdirectories separately.' ) - const canonical = await realpath(current) + const canonical = await access.resolve(current) assertImportPath(root, canonical) const info = await stat(canonical) if (!info.isDirectory() && !info.isFile()) @@ -210,20 +231,27 @@ async function manifest( } } -/** Calls have already been authorized against the pending server record by the IPC boundary. */ +/** Requires both a pending server call and a main-process grant before returning local data. */ export async function executeLocalFileRequest( request: unknown, - authorization: LocalFileAuthorization + authorization: LocalFileAuthorization, + access: LocalFileAccess ): Promise { try { if (!isRecordLike(request)) throw new Error('Invalid local file request.') - const path = nativePath(authorization.args.path) - if (request.operation === 'read' && authorization.toolName === 'read_local_file') - return { ok: true, data: await inspect(path, authorization.args) } + const path = await access.resolve(access.path) + if (request.operation === 'read' && authorization.toolName === 'read_local_file') { + const data = await inspect(path, authorization.args, access) + await access.resolve(path) + return { ok: true, data } + } if (authorization.toolName !== 'import_local_files') throw new Error('The operation does not match the pending tool call.') - if (request.operation === 'manifest') - return { ok: true, data: await manifest(path, authorization.args) } + if (request.operation === 'manifest') { + const data = await manifest(path, authorization.args, access) + await access.resolve(path) + return { ok: true, data } + } if ( request.operation !== 'chunk' || typeof request.relativePath !== 'string' || @@ -232,11 +260,11 @@ export async function executeLocalFileRequest( throw new Error('Invalid file chunk request.') const child = resolve(path, request.relativePath) assertImportPath(path, child) - const root = await realpath(path) - const canonical = await realpath(child) + const root = await access.resolve(path) + const canonical = await access.resolve(child) assertImportPath(root, canonical) const offset = boundedInteger(request.offset, 0, Number.MAX_SAFE_INTEGER) - const file = await open(canonical, 'r') + const file = await openApprovedFile(canonical, access) try { const info = await file.stat() if (!info.isFile() || revision(info) !== request.revision) @@ -248,6 +276,7 @@ export async function executeLocalFileRequest( const { bytesRead } = await file.read(buffer, 0, buffer.length, offset) if (revision(await file.stat()) !== request.revision) throw new Error('The source file changed during import.') + await access.resolve(child) return { ok: true, data: { diff --git a/packages/desktop-bridge/src/local-files.ts b/packages/desktop-bridge/src/local-files.ts index 6a76b2b1aa0..b1e6ca22278 100644 --- a/packages/desktop-bridge/src/local-files.ts +++ b/packages/desktop-bridge/src/local-files.ts @@ -21,7 +21,7 @@ export function isStorableImportName(name: string): boolean { ) } -/** Native desktop file operations use OS permissions and canonical pending chat calls. */ +/** Native desktop file operations require local consent and canonical pending chat calls. */ export type DesktopLocalFileRequest = | { operation: 'read' | 'manifest'; toolCallId: string } | { From 2b7f931ccb392d1b9091ffee264ca8b7f3e0f642 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 26 Sep 2026 15:07:25 -0700 Subject: [PATCH 2/8] fix(desktop): remember folder permissions across chats --- apps/desktop/README.md | 6 +- apps/desktop/e2e/local-files.spec.ts | 153 ++++++++++----- apps/desktop/src/main/index.ts | 3 + apps/desktop/src/main/ipc.ts | 6 +- .../src/main/local-file-permissions.ts | 180 +++++++----------- apps/desktop/src/main/local-files.ts | 2 +- .../src/main/local-filesystem-grant-store.ts | 9 + apps/desktop/src/main/local-filesystem.ts | 135 ++++++++++++- apps/desktop/src/main/menu.test.ts | 1 + apps/desktop/src/main/menu.ts | 9 + 10 files changed, 323 insertions(+), 181 deletions(-) diff --git a/apps/desktop/README.md b/apps/desktop/README.md index ccbe6925687..83bff675d09 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -182,15 +182,15 @@ Copilot can inspect user-selected local directories through the ordinary VFS too - **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. - **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. -- **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. +- **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. - **Opaque:** the model sees canonical paths such as `user-local/Project--/README.md`, never host paths or internal `localfs://` URIs. Electron resolves every request, checks lexical and realpath containment, and refuses symlink escapes. - **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. - **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. 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. +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. -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. +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. ## Auto-update, channels, rollout, rollback diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 7093e3c9d6c..e5e7d34827a 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -16,7 +16,7 @@ import type { DesktopLocalFileRequest, SimDesktopApi } from '@sim/desktop-bridge const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url)) -test('native file tools require local consent and reuse only the approved chat and path', async () => { +test('native file tools remember folder consent across chats and restarts until revoked', async () => { const root = mkdtempSync(join(tmpdir(), 'sim-native-files-e2e-')) const source = join(root, 'Reports') const outside = join(root, 'Reports-other') @@ -28,7 +28,7 @@ test('native file tools require local consent and reuse only the approved chat a 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII=' writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64')) const claimed = new Set() - const authorizedCalls = new Set() + const expireAfterAuthorization = new Set() let signedIn = true const calls: Record< string, @@ -87,11 +87,11 @@ test('native file tools require local consent and reuse only the approved chat a response.writeHead(call ? 409 : 403, { 'Content-Type': 'application/json' }).end('{}') return } - authorizedCalls.add(input.toolCallId) if (input.claim) claimed.add(input.toolCallId) response .writeHead(200, { 'Content-Type': 'application/json' }) .end(JSON.stringify({ ...call, chatId: call.chatId ?? 'org-chat' })) + if (expireAfterAuthorization.has(input.toolCallId)) calls[input.toolCallId] = undefined return } response @@ -101,21 +101,27 @@ test('native file tools require local consent and reuse only the approved chat a ? { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/' } : {}), }) - .end('Local file fixture

Local files

') + .end(`Local file fixture

Local files

+ `) }) await new Promise((resolve) => server?.listen(0, '127.0.0.1', resolve)) const address = server.address() if (!address || typeof address === 'string') throw new Error('Missing fixture address') - app = await electron.launch({ - args: ['.'], - cwd: DESKTOP_DIR, - env: { - ...process.env, - SIM_DESKTOP_ORIGIN: `http://127.0.0.1:${address.port}`, - SIM_DESKTOP_USER_DATA: join(root, 'profile'), - }, - }) - const window = await app.firstWindow() + const launch = () => + electron.launch({ + args: ['.', '--use-mock-keychain'], + cwd: DESKTOP_DIR, + env: { + ...process.env, + SIM_DESKTOP_ORIGIN: `http://127.0.0.1:${address.port}`, + SIM_DESKTOP_USER_DATA: join(root, 'profile'), + }, + }) + app = await launch() + let window = await app.firstWindow() await expect(window.getByRole('heading')).toHaveText('Local files') const invoke = (input: DesktopLocalFileRequest) => window.evaluate(async (request) => { @@ -138,6 +144,7 @@ test('native file tools require local consent and reuse only the approved chat a }) const deniedPrompt = app.waitForEvent('window', { timeout: 10_000 }) const deniedRead = invoke({ operation: 'read', toolCallId: 'text' }) + void deniedRead.catch(() => {}) const denial = await deniedPrompt await expect(denial.getByRole('button', { name: "Don't allow", exact: true })).toBeFocused() await denial.screenshot({ @@ -150,12 +157,14 @@ test('native file tools require local consent and reuse only the approved chat a const folderPrompt = app.waitForEvent('window') const folderRead = invoke({ operation: 'read', toolCallId: 'directory' }) + void folderRead.catch(() => {}) const folderConsent = await folderPrompt const queuedRead = invoke({ operation: 'read', toolCallId: 'text' }) + void queuedRead.catch(() => {}) expect( await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop) ).toBe('undefined') - await folderConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click() + await folderConsent.getByRole('button', { name: 'Allow folder', exact: true }).click() expect(await folderRead).toMatchObject({ ok: true, data: { representation: 'directory' } }) expect(await queuedRead).toMatchObject({ ok: true, data: { text: 'native file contents' } }) const canonicalRequest = { @@ -171,29 +180,36 @@ test('native file tools require local consent and reuse only the approved chat a ok: true, data: { observations: [{ mediaType: 'image/png', data: png }] }, }) - const runningApp = app const requestPermission = async (request: DesktopLocalFileRequest) => { - const shown = runningApp.waitForEvent('window', { timeout: 10_000 }) + if (!app) throw new Error('Desktop app is not running') + const shown = app.waitForEvent('window', { timeout: 10_000 }) const result = invoke(request) void result.catch(() => {}) const prompt = await shown await expect(prompt.getByRole('button', { name: "Don't allow", exact: true })).toBeVisible() return { prompt, result } } - await test.step('a folder grant does not authorize another chat or a symlink escape', async () => { - const otherChat = await requestPermission({ operation: 'read', toolCallId: 'otherChat' }) - const dismissed = otherChat.prompt.waitForEvent('close') - await otherChat.prompt - .getByRole('button', { name: "Don't allow", exact: true }) - .press('Escape') - .catch(() => {}) - await dismissed - expect(await otherChat.result).toMatchObject({ ok: false }) + await test.step('a folder grant works in another chat but does not permit symlink escapes', async () => { + expect(await invoke({ operation: 'read', toolCallId: 'otherChat' })).toMatchObject({ + ok: true, + data: { text: 'native file contents' }, + }) + writeFileSync(join(source, 'empty', 'new.txt'), 'new file in a subfolder') + calls.nested = { + toolName: 'read_local_file', + args: { path: join(source, 'empty', 'new.txt') }, + chatId: 'another-chat', + } + expect(await invoke({ operation: 'read', toolCallId: 'nested' })).toMatchObject({ + ok: true, + data: { text: 'new file in a subfolder' }, + }) + rmSync(join(source, 'empty', 'new.txt')) symlinkSync(join(outside, 'private.txt'), join(source, 'linked.txt')) calls.escape = { toolName: 'read_local_file', args: { path: join(source, 'linked.txt') } } const escapedRead = await requestPermission({ operation: 'read', toolCallId: 'escape' }) await expect(escapedRead.prompt.getByRole('dialog')).toContainText( - JSON.stringify(realpathSync(join(outside, 'private.txt'))) + JSON.stringify(realpathSync(outside)) ) await escapedRead.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() expect(await escapedRead.result).toMatchObject({ ok: false }) @@ -205,21 +221,23 @@ test('native file tools require local consent and reuse only the approved chat a const stale = await requestPermission({ operation: 'read', toolCallId: 'stale' }) if (changed) calls.stale.args.path = join(source, 'report.txt') else calls.stale = undefined - await stale.prompt.getByRole('button', { name: 'Allow for this chat', exact: true }).click() + await stale.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() expect(await stale.result).toMatchObject({ ok: false }) } }) - await test.step('a queued call is revalidated even when its folder is already approved', async () => { + await test.step('an unanswered prompt does not block approved folders', async () => { calls.blocker = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } } - calls.queued = { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } } const blocker = await requestPermission({ operation: 'read', toolCallId: 'blocker' }) - const queued = invoke({ operation: 'read', toolCallId: 'queued' }) - void queued.catch(() => {}) - await expect.poll(() => authorizedCalls.has('queued')).toBe(true) - calls.queued = undefined + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true }) await blocker.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() expect(await blocker.result).toMatchObject({ ok: false }) - expect(await queued).toMatchObject({ ok: false }) + }) + await test.step('cancelled calls cannot reuse an approved folder', async () => { + calls.expired = { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } } + expireAfterAuthorization.add('expired') + expect(await invoke({ operation: 'read', toolCallId: 'expired' })).toMatchObject({ + ok: false, + }) }) await test.step('replacing the proposed folder during consent does not expose its new target', async () => { const proposed = join(root, 'Proposed') @@ -229,16 +247,10 @@ test('native file tools require local consent and reuse only the approved chat a renameSync(proposed, join(root, 'Original')) mkdirSync(proposed) writeFileSync(join(proposed, 'unapproved.txt'), 'replacement folder contents') - await retargeted.prompt - .getByRole('button', { name: 'Allow for this chat', exact: true }) - .click() + await retargeted.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() expect(await retargeted.result).toMatchObject({ ok: false }) }) - const importPrompt = app.waitForEvent('window') - const importing = invoke({ operation: 'manifest', toolCallId: 'import' }) - const importConsent = await importPrompt - await importConsent.getByRole('button', { name: 'Allow for this chat', exact: true }).click() - const result = await importing + const result = await invoke({ operation: 'manifest', toolCallId: 'import' }) if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result)) expect(result.data.targetWorkspaceId).toBe('target-workspace') expect(result.data.entries.map((entry) => entry.relativePath)).toEqual([ @@ -282,22 +294,59 @@ test('native file tools require local consent and reuse only the approved chat a ok: false, code: 'ALREADY_STARTED', }) - await test.step('import approval is bound to its destination workspace', async () => { + await test.step('an approved folder permits imports without repeated destination prompts', async () => { calls.otherImport = { toolName: 'import_local_files', args: { path: source, targetWorkspaceId: 'other-workspace' }, } - const otherImport = await requestPermission({ - operation: 'manifest', - toolCallId: 'otherImport', + expect(await invoke({ operation: 'manifest', toolCallId: 'otherImport' })).toMatchObject({ + ok: true, }) - await otherImport.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() - expect(await otherImport.result).toMatchObject({ ok: false }) + }) + await test.step('folder permissions survive restarting the desktop app', async () => { + await app?.close() + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading')).toHaveText('Local files') + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true }) + }) + await test.step('forgetting a folder revokes native reads and survives restart', async () => { + await window.getByRole('button', { name: 'Forget folders', exact: true }).click() + await expect(window.getByRole('button', { name: 'Forgotten', exact: true })).toBeVisible() + await app?.close() + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading')).toHaveText('Local files') + const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await revoked.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + expect(await revoked.result).toMatchObject({ ok: true }) + }) + + await test.step('a remembered grant does not follow a replaced folder after restart', async () => { + await app?.close() + renameSync(source, join(root, 'Original-reports')) + mkdirSync(source) + writeFileSync(join(source, 'report.txt'), 'replacement contents') + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading')).toHaveText('Local files') + const replaced = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await replaced.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await replaced.result).toMatchObject({ ok: false }) + rmSync(source, { recursive: true }) + renameSync(join(root, 'Original-reports'), source) + const restored = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await restored.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + expect(await restored.result).toMatchObject({ ok: true }) }) - await test.step('sign-out revokes chat grants before the next account session', async () => { - await window.evaluate(async () => { - await fetch('/api/auth/sign-out', { method: 'POST' }) + await test.step('sign-out revokes remembered grants before the next account session', async () => { + await app?.evaluate(({ Menu }) => { + const item = Menu.getApplicationMenu() + ?.items.flatMap((entry) => entry.submenu?.items ?? []) + .find((entry) => entry.label === 'Sign Out') + if (!item) throw new Error('Sign Out menu item missing') + item.click() }) await expect(window).toHaveURL(`http://127.0.0.1:${address.port}/login`) signedIn = true diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 63edf362b29..ad203c7a54e 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -980,6 +980,9 @@ function main(): void { allowHttpLocalhost, openSettings, openServerSettings: () => serverWindow.open(), + openFolderAccess: (parent) => { + if (accountDataAvailable()) localFilesystem.showAccessMenu(parent) + }, newWindow: () => void createAndLoadAppWindow(), newChat: () => void openMainWindowAt(newChatRoute(config.get('lastRoute'))), handleFocusedResourceShortcut: (win, shortcut) => diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 84dea6fdf31..b1ee3fcc379 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -88,9 +88,9 @@ import { isSafeInternalPath } from '@/main/config' import type { DesktopSettingsService } from '@/main/desktop-settings' import { isDesktopPreferenceKey } from '@/main/desktop-settings' import { hasRecentDeliberateInput, hasRecentDiscreteInput } from '@/main/input-activity' -import { type LocalFileAccess, LocalFilePermissions } from '@/main/local-file-permissions' +import { LocalFilePermissions } from '@/main/local-file-permissions' import { executeLocalFileRequest } from '@/main/local-files' -import type { LocalFilesystemService } from '@/main/local-filesystem' +import type { LocalFileAccess, LocalFilesystemService } from '@/main/local-filesystem' import { isAppOrigin, openExternalSafe } from '@/main/navigation' import type { ScopedEventRouter } from '@/main/scoped-event-router' import type { TerminalRegistry } from '@/main/terminal/registry' @@ -613,7 +613,7 @@ async function authorizeLocalFilesystemTool( * unvalidated args they must parse themselves. */ export function registerIpcHandlers(deps: IpcDeps): void { - const localFilePermissions = new LocalFilePermissions() + const localFilePermissions = new LocalFilePermissions(deps.localFilesystem) const browserScopeBySender = new WeakMap() const terminalScopeBySender = new WeakMap() const browserPendingScopesBySender = new WeakMap>() diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts index 0e3b1338ff9..1893cc98a11 100644 --- a/apps/desktop/src/main/local-file-permissions.ts +++ b/apps/desktop/src/main/local-file-permissions.ts @@ -1,12 +1,12 @@ import { lstat, realpath, stat } from 'node:fs/promises' import { homedir } from 'node:os' -import { isAbsolute, join, relative, resolve, sep } from 'node:path' +import { dirname, isAbsolute, join, resolve } from 'node:path' import { isDesktopScopeId } from '@sim/desktop-bridge' import type { BrowserWindow } from 'electron' import { showShellDialog } from '@/main/dialogs' import type { LocalFileAuthorization } from '@/main/local-files' +import type { LocalFileAccess, LocalFilesystemService } from '@/main/local-filesystem' -const MAX_GRANTS = 256 const MAX_PENDING_REQUESTS = 32 interface LocalFilePermissionContext { @@ -17,19 +17,6 @@ interface LocalFilePermissionContext { revalidate: () => Promise } -interface LocalFileGrant { - scope: string - path: string - directory: boolean - dev: number - ino: number -} - -export interface LocalFileAccess { - path: string - resolve: (path: string) => Promise -} - function nativePath(value: unknown): string { if (typeof value !== 'string' || value.length > 4096 || value.includes('\0')) throw new Error('A native absolute path or ~/ path is required.') @@ -39,34 +26,44 @@ function nativePath(value: unknown): string { return resolve(path) } -function contains(grant: LocalFileGrant, path: string): boolean { - if (path === grant.path) return true - if (!grant.directory) return false - const rel = relative(grant.path, path) - return !isAbsolute(rel) && rel !== '..' && !rel.startsWith(`..${sep}`) -} - function assertCurrent(context: LocalFilePermissionContext): void { if (context.parent.isDestroyed() || !context.isCurrent()) throw new Error('This local file request expired. Ask again in the current chat.') } -/** Chat grants live only in this desktop session and are never writable by the hosted renderer. */ +async function revalidate(context: LocalFilePermissionContext): Promise { + assertCurrent(context) + if (!(await context.revalidate())) + throw new Error('This local file tool call is no longer pending or its arguments changed.') + assertCurrent(context) +} + +/** Serializes new consent prompts while remembered folder access remains concurrent. */ export class LocalFilePermissions { - private grants: LocalFileGrant[] = [] - private generation = -1 private queue: Promise = Promise.resolve() private pending = 0 + constructor(private readonly filesystem: LocalFilesystemService) {} + async authorize( - authorization: LocalFileAuthorization & { chatId: string }, + authorization: LocalFileAuthorization, context: LocalFilePermissionContext ): Promise { + assertCurrent(context) + if ( + authorization.toolName === 'import_local_files' && + (!isDesktopScopeId(authorization.args.targetWorkspaceId) || + (authorization.args.folderId !== undefined && + !isDesktopScopeId(authorization.args.folderId))) + ) + throw new Error('A valid destination workspace and folder are required for imports.') + const path = await realpath(nativePath(authorization.args.path)) + const existing = await this.filesystem.nativeAccess(path) + if (existing) return this.authorizedAccess(existing, context) if (this.pending >= MAX_PENDING_REQUESTS) throw new Error('Too many local file requests are waiting for permission. Try again later.') - const wasQueued = this.pending > 0 this.pending++ - const pending = this.queue.then(() => this.authorizeNext(authorization, context, wasQueued)) + const pending = this.queue.then(() => this.requestFolder(path, context)) this.queue = pending.then( () => undefined, () => undefined @@ -78,98 +75,53 @@ export class LocalFilePermissions { } } - private async authorizeNext( - authorization: LocalFileAuthorization & { chatId: string }, - context: LocalFilePermissionContext, - wasQueued: boolean + private async requestFolder( + path: string, + context: LocalFilePermissionContext ): Promise { - assertCurrent(context) - if (this.generation !== context.generation) { - this.grants = [] - this.generation = context.generation - } - const importing = authorization.toolName === 'import_local_files' - if ( - importing && - (!isDesktopScopeId(authorization.args.targetWorkspaceId) || - (authorization.args.folderId !== undefined && - !isDesktopScopeId(authorization.args.folderId))) - ) - throw new Error('A valid destination workspace and folder are required for imports.') - const scope = JSON.stringify([ - context.origin, - authorization.chatId, - authorization.toolName, - ...(importing ? [authorization.args.targetWorkspaceId, authorization.args.folderId] : []), - ]) - const path = await realpath(nativePath(authorization.args.path)) + await revalidate(context) + const existing = await this.filesystem.nativeAccess(path) + if (existing) return this.authorizedAccess(existing, context) const info = await stat(path) if (!info.isFile() && !info.isDirectory()) throw new Error('The path is not a regular file or directory.') - let grant = this.grants.find((entry) => entry.scope === scope && contains(entry, path)) - if (grant) { - if (wasQueued && !(await context.revalidate())) - throw new Error('This local file tool call is no longer pending or its arguments changed.') - const root = await lstat(grant.path) - if (root.dev !== grant.dev || root.ino !== grant.ino || root.isSymbolicLink()) { - this.grants = this.grants.filter((entry) => entry !== grant) - grant = undefined - } - } - if (!grant) { - if (this.grants.length >= MAX_GRANTS) - throw new Error( - 'Restart Sim to clear this session’s local file permissions before adding more.' - ) - assertCurrent(context) - const kind = info.isDirectory() ? 'folder' : 'file' - const displayedPath = JSON.stringify(path).replace( - /[\u202a-\u202e\u2066-\u2069]/g, - (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}` - ) - const result = await showShellDialog(context.parent, { - title: importing ? `Import this ${kind}?` : `Read this ${kind}?`, - message: displayedPath, - detail: [ - importing - ? `Sim will upload this ${kind}${info.isDirectory() ? ' and its contents' : ''} to your workspace on ${context.origin}.` - : `Sim will read this ${kind}${info.isDirectory() ? ' and its contents' : ''} and send the results to ${context.origin} for this chat.`, - ...(importing ? [`Destination workspace: ${authorization.args.targetWorkspaceId}`] : []), - 'This permission applies only to this chat and ends when Sim closes.', - ].join('\n\n'), - buttons: ['Allow for this chat', "Don't allow"], - defaultId: 1, - cancelId: 1, - }) - assertCurrent(context) - if (result.response !== 0) throw new Error('The user did not allow this local file access.') - if (!(await context.revalidate())) - throw new Error('This local file tool call is no longer pending or its arguments changed.') - grant = { scope, path, directory: info.isDirectory(), dev: info.dev, ino: info.ino } - await this.resolve(grant, path, context) - this.grants.push(grant) - } - await this.resolve(grant, path, context) - const approvedGrant = grant - return { path, resolve: (candidate) => this.resolve(approvedGrant, candidate, context) } + const folder = info.isDirectory() ? path : dirname(path) + 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, + (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}` + ) + assertCurrent(context) + const result = await showShellDialog(context.parent, { + 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.`, + buttons: ['Allow folder', "Don't allow"], + defaultId: 1, + cancelId: 1, + }) + assertCurrent(context) + if (result.response !== 0) throw new Error('The user did not allow this local file access.') + await revalidate(context) + await this.filesystem.grantDirectory({ path: folder }, context.generation, root) + const access = await this.filesystem.nativeAccess(path) + if (!access) throw new Error('The approved folder is no longer available.') + return this.authorizedAccess(access, context) } - private async resolve( - grant: LocalFileGrant, - candidate: string, + private async authorizedAccess( + access: LocalFileAccess, context: LocalFilePermissionContext - ): Promise { - assertCurrent(context) - const root = await lstat(grant.path) - if ( - root.dev !== grant.dev || - root.ino !== grant.ino || - (grant.directory ? !root.isDirectory() : !root.isFile()) - ) - throw new Error('The approved file or folder changed. Ask again to request access.') - const path = await realpath(candidate) - if (!contains(grant, path)) throw new Error('This path is outside the approved file or folder.') - assertCurrent(context) - return path + ): Promise { + await revalidate(context) + const resolveApproved = async (path: string): Promise => { + assertCurrent(context) + const resolved = await access.resolve(path) + assertCurrent(context) + return resolved + } + await resolveApproved(access.path) + return { path: access.path, resolve: resolveApproved } } } diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index 10cfa6674e5..e76c86448b8 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -11,7 +11,7 @@ import { MAX_DESKTOP_IMPORT_FILE_BYTES } from '@sim/desktop-bridge' import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' import { PDFDocument } from 'pdf-lib' -import type { LocalFileAccess } from '@/main/local-file-permissions' +import type { LocalFileAccess } from '@/main/local-filesystem' const CHUNK_BYTES = 8 * 1024 * 1024 const MAX_ENTRIES = 1000 diff --git a/apps/desktop/src/main/local-filesystem-grant-store.ts b/apps/desktop/src/main/local-filesystem-grant-store.ts index 67216976840..188c2469b60 100644 --- a/apps/desktop/src/main/local-filesystem-grant-store.ts +++ b/apps/desktop/src/main/local-filesystem-grant-store.ts @@ -21,6 +21,8 @@ export interface PersistedLocalFilesystemGrant { id: string name: string rootPath: string + dev?: number + ino?: number bookmark?: string } @@ -55,6 +57,13 @@ function isPersistedGrant(value: unknown): value is PersistedLocalFilesystemGran grant.rootPath.length > 0 && grant.rootPath.length <= MAX_GRANT_PATH_LENGTH && !grant.rootPath.includes('\0') && + ((grant.dev === undefined && grant.ino === undefined) || + (typeof grant.dev === 'number' && + Number.isSafeInteger(grant.dev) && + grant.dev >= 0 && + typeof grant.ino === 'number' && + Number.isSafeInteger(grant.ino) && + grant.ino >= 0)) && (grant.bookmark === undefined || (typeof grant.bookmark === 'string' && grant.bookmark.length > 0 && diff --git a/apps/desktop/src/main/local-filesystem.ts b/apps/desktop/src/main/local-filesystem.ts index a83abf50881..51b1598bbdf 100644 --- a/apps/desktop/src/main/local-filesystem.ts +++ b/apps/desktop/src/main/local-filesystem.ts @@ -17,10 +17,12 @@ import { MAX_GREP_RESULTS, MAX_READ_LINES, } from '@sim/desktop-bridge/local-filesystem-limits' +import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { escapeRegExp, stripTrailingSlashes, truncate } from '@sim/utils/string' -import { app, dialog, shell } from 'electron' +import type { BrowserWindow } from 'electron' +import { app, dialog, Menu, shell } from 'electron' import micromatch from 'micromatch' import safeRegex from 'safe-regex2' import { @@ -30,6 +32,7 @@ import { runAccountDataMutation, waitForAccountDataMutations, } from '@/main/account-data-generation' +import { showShellDialog } from '@/main/dialogs' import type { LocalFilesystemGrantStore, PersistedLocalFilesystemGrant, @@ -93,6 +96,8 @@ const GRANT_SCOPED_OPERATIONS: ReadonlySet = new Set([ interface GrantedMount extends LocalFilesystemMount { rootPath: string + dev: number + ino: number bookmark?: string stopAccessing?: () => void } @@ -346,6 +351,11 @@ async function selectDirectoryEntries( return { entries, truncated: seen > entries.length } } +export interface LocalFileAccess { + path: string + resolve: (path: string) => Promise +} + export class LocalFilesystemService { private readonly mounts = new Map() private readonly activeRequests = new Map() @@ -629,7 +639,18 @@ export class LocalFilesystemService { throw new LocalFilesystemError('CANCELLED', 'The folder request expired during sign-out.') } - const selected = typeof selection === 'string' ? { path: selection } : selection + return this.grantDirectory( + typeof selection === 'string' ? { path: selection } : selection, + generation + ) + } + + /** Records a folder selected through trusted desktop UI, with the identity shown at consent. */ + async grantDirectory( + selected: SelectedDirectory, + generation: number, + expected?: { dev: number; ino: number } + ): Promise { const stopAccessing = selected.bookmark ? this.startAccessingBookmark(selected.bookmark) : undefined @@ -652,7 +673,24 @@ export class LocalFilesystemService { throw new LocalFilesystemError('NOT_A_DIRECTORY', 'The selected item is not a directory.') } + if ( + expected && + (rootPath !== selected.path || + rootStat.dev !== expected.dev || + rootStat.ino !== expected.ino) + ) { + throw new LocalFilesystemError( + 'ACCESS_DENIED', + 'The folder changed while awaiting permission. Request access again.' + ) + } const existing = [...this.mounts.values()].find((mount) => mount.rootPath === rootPath) + if (!existing && this.mounts.size >= 256) { + throw new LocalFilesystemError( + 'INVALID_REQUEST', + 'Forget an unused folder in File → Folder Access before adding more.' + ) + } const id = existing?.id ?? generateId() const bookmark = selected.bookmark ?? existing?.bookmark const nextStopAccessing = selected.bookmark @@ -667,6 +705,8 @@ export class LocalFilesystemService { name: basename(rootPath) || 'Local files', uri: localUri(id), rootPath, + dev: rootStat.dev, + ino: rootStat.ino, remembered: existing?.remembered ?? false, ...(bookmark ? { bookmark } : {}), ...(nextStopAccessing ? { stopAccessing: nextStopAccessing } : {}), @@ -691,6 +731,75 @@ export class LocalFilesystemService { } } + /** Resolves native tool paths through the same remembered grants as VFS reads. */ + async nativeAccess(candidate: string): Promise { + const path = await realpath(candidate) + for (const mount of this.mounts.values()) { + if (!isWithinRoot(mount.rootPath, path)) continue + try { + await this.assertMountCurrent(mount) + } catch { + continue + } + const resolveGranted = async (requested: string): Promise => { + await this.assertMountCurrent(mount) + const resolved = await realpath(requested) + if (!isWithinRoot(mount.rootPath, resolved)) { + throw new LocalFilesystemError( + 'ACCESS_DENIED', + 'This path is outside the approved folder.' + ) + } + await this.assertMountCurrent(mount) + return resolved + } + return { path, resolve: resolveGranted } + } + return null + } + + private async assertMountCurrent(mount: GrantedMount): Promise { + const root = await lstat(mount.rootPath) + if ( + this.mounts.get(mount.id) !== mount || + !root.isDirectory() || + root.dev !== mount.dev || + root.ino !== mount.ino + ) { + throw new LocalFilesystemError( + 'ACCESS_DENIED', + 'Folder access was removed or the folder changed. Request access again.' + ) + } + } + + /** Native controls share the same grant store and revocation path as the desktop bridge. */ + showAccessMenu(parent: BrowserWindow): void { + const generation = captureAccountDataGeneration() + const run = (operation: () => Promise) => { + if (!isAccountDataGenerationCurrent(generation) || parent.isDestroyed()) return + void operation().catch((error) => + showShellDialog(parent, { + title: 'Folder access', + message: getErrorMessage(error), + buttons: ['OK'], + }) + ) + } + Menu.buildFromTemplate([ + { label: 'Add Folder…', click: () => run(() => this.mountDirectory()) }, + { type: 'separator' }, + ...[...this.mounts.values()].map((mount) => ({ + label: mount.rootPath, + submenu: [ + { label: 'Show Folder', click: () => run(async () => this.revealMount(mount.uri)) }, + { label: 'Forget Folder', click: () => run(() => this.forgetMount(mount.uri)) }, + ], + })), + ...(this.mounts.size === 0 ? [{ label: 'No folders allowed', enabled: false }] : []), + ]).popup({ window: parent }) + } + private listMounts(): LocalFilesystemData { return { mounts: [...this.mounts.values()].map((mount) => this.publicMount(mount)) } } @@ -709,11 +818,11 @@ export class LocalFilesystemService { const generation = captureAccountDataGeneration() const grants = await this.grantStore.load() if (!isAccountDataGenerationCurrent(generation)) return - let skipped = false + let needsPersist = false for (const grant of grants) { if (!/^[a-zA-Z0-9-]{1,128}$/.test(grant.id) || this.mounts.has(grant.id)) { - skipped = true + needsPersist = true continue } const stopAccessing = grant.bookmark ? this.startAccessingBookmark(grant.bookmark) : undefined @@ -724,27 +833,34 @@ export class LocalFilesystemService { stopAccessing?.() return } - if (!rootStat.isDirectory()) { + if ( + !rootStat.isDirectory() || + rootPath !== grant.rootPath || + (grant.dev !== undefined && (grant.dev !== rootStat.dev || grant.ino !== rootStat.ino)) + ) { stopAccessing?.() - skipped = true + needsPersist = true continue } + if (grant.dev === undefined) needsPersist = true this.mounts.set(grant.id, { id: grant.id, name: basename(rootPath) || grant.name || 'Local files', uri: localUri(grant.id), rootPath, + dev: rootStat.dev, + ino: rootStat.ino, remembered: true, ...(grant.bookmark ? { bookmark: grant.bookmark } : {}), ...(stopAccessing ? { stopAccessing } : {}), }) } catch { stopAccessing?.() - skipped = true + needsPersist = true } } - if (skipped) { + if (needsPersist) { await runAccountDataMutation(generation, () => this.persistMounts()) } } @@ -754,6 +870,8 @@ export class LocalFilesystemService { id: mount.id, name: mount.name, rootPath: mount.rootPath, + dev: mount.dev, + ino: mount.ino, ...(mount.bookmark ? { bookmark: mount.bookmark } : {}), })) } @@ -940,6 +1058,7 @@ export class LocalFilesystemService { private async resolveUri(uri: string): Promise { const { mount, relativePath } = this.parseUri(uri) + await this.assertMountCurrent(mount) const lexicalPath = resolve(mount.rootPath, ...relativePath.split('/').filter(Boolean)) if (!isWithinRoot(mount.rootPath, lexicalPath)) { throw new LocalFilesystemError( diff --git a/apps/desktop/src/main/menu.test.ts b/apps/desktop/src/main/menu.test.ts index 7364b5aaa39..2eb8862dc32 100644 --- a/apps/desktop/src/main/menu.test.ts +++ b/apps/desktop/src/main/menu.test.ts @@ -20,6 +20,7 @@ function makeDeps(origin = 'https://sim.ai'): MenuDeps { allowHttpLocalhost: vi.fn(() => false), openSettings: vi.fn(), openServerSettings: vi.fn(), + openFolderAccess: vi.fn(), newWindow: vi.fn(), newChat: vi.fn(), handleFocusedResourceShortcut: vi.fn(() => false), diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index b2234f74d5b..a88c51de480 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -19,6 +19,7 @@ export interface MenuDeps { openSettings: () => void /** Opens the native server picker (see main/server-window.ts). */ openServerSettings: () => void + openFolderAccess: (parent: BrowserWindow) => void newWindow: () => void newChat: () => void /** @@ -212,6 +213,14 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] { label: 'File', submenu: [ + { + label: 'Folder Access…', + click: (_item, focusedWindow) => { + const win = focusedMainOrFallback(focusedWindow) + if (win) deps.openFolderAccess(win) + }, + }, + { type: 'separator' }, { label: 'New Window', accelerator: 'CmdOrCtrl+Shift+N', From 260c7e650cb9cee54224ad0d7f9c0050b133b655 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 26 Sep 2026 15:31:11 -0700 Subject: [PATCH 3/8] fix(desktop): cancel pending file consent and recheck access --- apps/desktop/README.md | 2 +- apps/desktop/e2e/local-files.spec.ts | 92 ++++++++++++++++ apps/desktop/src/main/ipc.ts | 81 +++++++++----- .../src/main/local-file-permissions.ts | 5 +- apps/desktop/src/main/local-files.ts | 101 ++++++++++++++---- .../desktop/src/main/local-filesystem.test.ts | 36 ++++++- apps/desktop/src/main/local-filesystem.ts | 6 +- .../mothership/tools/client/native-files.ts | 18 +++- packages/desktop-bridge/src/local-files.ts | 2 +- 9 files changed, 283 insertions(+), 60 deletions(-) diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 83bff675d09..30a0eaeb035 100644 --- a/apps/desktop/README.md +++ b/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. diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index e5e7d34827a..54164f0b175 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -225,6 +225,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) => { + 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 (operation: () => Promise): Promise => { + 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) => + 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' }) diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index b1ee3fcc379..19c83aaa087 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -614,6 +614,8 @@ async function authorizeLocalFilesystemTool( */ export function registerIpcHandlers(deps: IpcDeps): void { const localFilePermissions = new LocalFilePermissions(deps.localFilesystem) + const activeLocalFiles = new Map>() + let activeLocalFileCount = 0 const browserScopeBySender = new WeakMap() const terminalScopeBySender = new WeakMap() const browserPendingScopesBySender = new WeakMap>() @@ -2147,42 +2149,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' || request.operation === 'read', - (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() + controllers.add(controller) + activeLocalFiles.set(key, controllers) + activeLocalFileCount++ try { + let failureStatus: number | undefined + const authorization = await fetchDesktopToolAuthorization( + event, + deps, + request.toolCallId, + request.operation === 'manifest' || request.operation === 'read', + (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() && @@ -2194,9 +2213,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) { diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts index 1893cc98a11..bfc7133f0f2 100644 --- a/apps/desktop/src/main/local-file-permissions.ts +++ b/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 } @@ -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.`, diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index e76c86448b8..d9179636ed4 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -1,5 +1,5 @@ -import { constants } from 'node:fs' -import { lstat, open, readdir, stat } from 'node:fs/promises' +import { constants, type Dirent } from 'node:fs' +import { lstat, open, opendir, stat } from 'node:fs/promises' import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' import type { DesktopLocalFileEntry, @@ -10,6 +10,7 @@ import type { import { MAX_DESKTOP_IMPORT_FILE_BYTES } from '@sim/desktop-bridge' import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' +import { compareStrings } from '@sim/utils/string' import { PDFDocument } from 'pdf-lib' import type { LocalFileAccess } from '@/main/local-filesystem' @@ -39,18 +40,23 @@ function assertImportPath(root: string, candidate: string): void { throw new Error('The file is outside this import source.') } -async function openApprovedFile(path: string, access: LocalFileAccess) { +async function openApprovedPath(path: string, access: LocalFileAccess, directory = false) { const canonical = await access.resolve(path) + if (canonical !== path) + throw new Error('The local path changed while it was being opened. Try again.') const file = await open( canonical, - constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK + constants.O_RDONLY | + constants.O_NOFOLLOW | + constants.O_NONBLOCK | + (directory ? constants.O_DIRECTORY : 0) ) try { const info = await file.stat() const verified = await access.resolve(path) const current = await lstat(verified) if ( - !info.isFile() || + (directory ? !info.isDirectory() : !info.isFile()) || canonical !== verified || info.dev !== current.dev || info.ino !== current.ino @@ -63,6 +69,53 @@ async function openApprovedFile(path: string, access: LocalFileAccess) { } } +async function readApprovedDirectory(path: string, access: LocalFileAccess) { + const handle = await openApprovedPath(path, access, true) + try { + const before = await handle.stat({ bigint: true }) + const verify = async () => { + const canonical = await access.resolve(path) + const current = await lstat(canonical, { bigint: true }) + const after = await handle.stat({ bigint: true }) + if ( + canonical !== path || + !current.isDirectory() || + current.dev !== before.dev || + current.ino !== before.ino || + current.ctimeNs !== before.ctimeNs || + current.mtimeNs !== before.mtimeNs || + after.ctimeNs !== before.ctimeNs || + after.mtimeNs !== before.mtimeNs + ) { + throw new Error('The local directory changed while it was being read. Try again.') + } + } + const directory = await opendir(path) + try { + await verify() + const entries: Dirent[] = [] + let count = 0 + const sort = () => entries.sort((left, right) => compareStrings(left.name, right.name)) + for (let entry = await directory.read(); entry; entry = await directory.read()) { + entries.push(entry) + count++ + if (entries.length === MAX_ENTRIES * 2) { + sort() + entries.length = MAX_ENTRIES + await access.resolve(path) + } + } + await verify() + sort() + return { entries: entries.slice(0, MAX_ENTRIES), truncated: count > MAX_ENTRIES } + } finally { + await directory.close() + } + } finally { + await handle.close() + } +} + async function inspect( path: string, args: Record, @@ -70,29 +123,26 @@ async function inspect( ): Promise { const info = await stat(path) if (info.isDirectory()) { - const entries = await readdir(path, { withFileTypes: true }) + const { entries, truncated } = await readApprovedDirectory(path, access) return { kind: 'read', path, representation: 'directory', - truncated: entries.length > MAX_ENTRIES, - entries: entries - .sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)) - .slice(0, MAX_ENTRIES) - .map((entry) => ({ - name: entry.name, - kind: entry.isFile() - ? 'file' - : entry.isDirectory() - ? 'directory' - : entry.isSymbolicLink() - ? 'symlink' - : 'other', - })), + truncated, + entries: entries.map((entry) => ({ + name: entry.name, + kind: entry.isFile() + ? 'file' + : entry.isDirectory() + ? 'directory' + : entry.isSymbolicLink() + ? 'symlink' + : 'other', + })), } } if (!info.isFile()) throw new Error('The path is not a regular file or directory.') - const file = await openApprovedFile(path, access) + const file = await openApprovedPath(path, access) try { const info = await file.stat() const header = Buffer.alloc(16) @@ -218,7 +268,12 @@ async function manifest( }) if (info.isDirectory()) { const next = new Set([...ancestors, canonical]) - for (const name of (await readdir(current)).sort()) await walk(join(current, name), next) + const children = await readApprovedDirectory(canonical, access) + if (children.truncated) + throw new Error( + 'The directory exceeds 1,000 entries. Import smaller subdirectories separately.' + ) + for (const entry of children.entries) await walk(join(current, entry.name), next) } } await walk(path, new Set()) @@ -264,7 +319,7 @@ export async function executeLocalFileRequest( const canonical = await access.resolve(child) assertImportPath(root, canonical) const offset = boundedInteger(request.offset, 0, Number.MAX_SAFE_INTEGER) - const file = await openApprovedFile(canonical, access) + const file = await openApprovedPath(canonical, access) try { const info = await file.stat() if (!info.isFile() || revision(info) !== request.revision) diff --git a/apps/desktop/src/main/local-filesystem.test.ts b/apps/desktop/src/main/local-filesystem.test.ts index f4d0b5a35a6..d0418a67e05 100644 --- a/apps/desktop/src/main/local-filesystem.test.ts +++ b/apps/desktop/src/main/local-filesystem.test.ts @@ -3,6 +3,7 @@ import { mkdir, mkdtemp, open, + readFile, realpath, rename, rm, @@ -16,7 +17,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, open: vi.fn(actual.open) } + return { + ...actual, + open: vi.fn(actual.open), + realpath: vi.fn(actual.realpath), + readFile: vi.fn(actual.readFile), + } }) import type { LocalFilesystemMount, LocalFilesystemResponse } from '@sim/desktop-bridge' @@ -379,6 +385,34 @@ describe('LocalFilesystemService', () => { } ) + it.each(['resolution', 'read'] as const)( + 'returns no VFS contents when access is revoked during %s', + async (stage) => { + const granted = await mount(service) + const revoke = () => service.handle({ operation: 'forget_mount', uri: granted.uri }) + if (stage === 'resolution') { + const original = vi.mocked(realpath).getMockImplementation() + if (!original) throw new Error('Missing real filesystem implementation') + vi.mocked(realpath).mockImplementationOnce(async (...args) => { + const result = await original(...args) + await revoke() + return result + }) + } else { + const original = vi.mocked(readFile).getMockImplementation() + if (!original) throw new Error('Missing real filesystem implementation') + vi.mocked(readFile).mockImplementationOnce(async (...args) => { + const result = await original(...args) + await revoke() + return result + }) + } + expect( + await service.handle({ operation: 'read', uri: `${granted.uri}README.md` }) + ).toMatchObject({ ok: false, code: 'ACCESS_DENIED' }) + } + ) + it('rejects lexical traversal before URL normalization can reinterpret it', async () => { const granted = await mount(service) const traversal = await service.handle({ diff --git a/apps/desktop/src/main/local-filesystem.ts b/apps/desktop/src/main/local-filesystem.ts index 51b1598bbdf..469482035a3 100644 --- a/apps/desktop/src/main/local-filesystem.ts +++ b/apps/desktop/src/main/local-filesystem.ts @@ -498,12 +498,15 @@ export class LocalFilesystemService { 'Local filesystem operation is not supported.' ) } + if (grant) { + if (this.mounts.get(grant.id)?.rootPath !== grant.rootPath) throw mountNotFound() + await this.assertMountCurrent(grant) + } } finally { if (requestId) { this.activeRequests.delete(requestId) } } - if (grant && this.mounts.get(grant.id)?.rootPath !== grant.rootPath) throw mountNotFound() return { ok: true, data } } catch (error) { const safe = safeError(error) @@ -1073,6 +1076,7 @@ export class LocalFilesystemService { 'The requested path is outside the selected folder.' ) } + await this.assertMountCurrent(mount) return { mount, relativePath, lexicalPath, realPath } } diff --git a/apps/sim/lib/mothership/tools/client/native-files.ts b/apps/sim/lib/mothership/tools/client/native-files.ts index e26daa5e3f4..315ddc07a02 100644 --- a/apps/sim/lib/mothership/tools/client/native-files.ts +++ b/apps/sim/lib/mothership/tools/client/native-files.ts @@ -38,9 +38,21 @@ async function invoke( signal?.throwIfAborted() const bridge = getDesktopBridge() if (!bridge?.localFiles) throw new Error('Update the Sim desktop app to use native file tools.') - const response = await bridge.localFiles(request) - signal?.throwIfAborted() - return response + const onAbort = () => { + void bridge + .localFiles?.({ operation: 'cancel', toolCallId: request.toolCallId }) + .catch((error) => + logger.warn('Could not cancel native file access', { error: getErrorMessage(error) }) + ) + } + signal?.addEventListener('abort', onAbort, { once: true }) + try { + const response = await bridge.localFiles(request) + signal?.throwIfAborted() + return response + } finally { + signal?.removeEventListener('abort', onAbort) + } } /** The manifest is produced from canonical pending-tool arguments in Electron, never renderer paths. */ diff --git a/packages/desktop-bridge/src/local-files.ts b/packages/desktop-bridge/src/local-files.ts index b1e6ca22278..e3c2109954d 100644 --- a/packages/desktop-bridge/src/local-files.ts +++ b/packages/desktop-bridge/src/local-files.ts @@ -23,7 +23,7 @@ export function isStorableImportName(name: string): boolean { /** Native desktop file operations require local consent and canonical pending chat calls. */ export type DesktopLocalFileRequest = - | { operation: 'read' | 'manifest'; toolCallId: string } + | { operation: 'read' | 'manifest' | 'cancel'; toolCallId: string } | { operation: 'chunk' toolCallId: string From 436d535636567ee2a4998c99bd59252bd4216862 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 26 Sep 2026 15:51:39 -0700 Subject: [PATCH 4/8] fix(desktop): enumerate approved directory descriptors --- apps/desktop/README.md | 4 +- apps/desktop/e2e/local-files.spec.ts | 123 +++++++++++++---- apps/desktop/native/directory.cc | 154 ++++++++++++++++++++++ apps/desktop/package.json | 4 +- apps/desktop/scripts/build-native.ts | 62 +++++++++ apps/desktop/scripts/build.ts | 52 +------- apps/desktop/src/main/local-files.test.ts | 8 +- apps/desktop/src/main/local-files.ts | 60 ++------- apps/desktop/src/main/native-directory.ts | 25 ++++ 9 files changed, 362 insertions(+), 130 deletions(-) create mode 100644 apps/desktop/native/directory.cc create mode 100644 apps/desktop/scripts/build-native.ts create mode 100644 apps/desktop/src/main/native-directory.ts diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 30a0eaeb035..61b1cee9504 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -38,7 +38,7 @@ src/main/ # main process (bundled to dist/main.cjs) src/preload/ # isolated renderer bridges index.ts # hosted-app contextBridge IPC bridge (dist/preload.cjs) browser/ # minimal agent-browser credential helper (dist/browser-preload.cjs) -native/ # Node-API/AppKit bridge for native macOS Help docs search +native/ # Node-API bridges for directory enumeration and macOS Help docs search static/ # bundled local pages (offline.html, server.html), served over sim-shell: e2e/ # Playwright _electron smoke suite ``` @@ -190,7 +190,7 @@ Copilot can inspect user-selected local directories through the ordinary VFS too 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. +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. 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. ## Auto-update, channels, rollout, rollback diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 54164f0b175..7fe39192b71 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -256,40 +256,113 @@ test('native file tools remember folder consent across chats and restarts until rmSync(other) } }) - await test.step('a directory swapped out and back cannot leak outside entries', async () => { + await test.step('swapping an ancestor cannot redirect directory enumeration', async () => { + const parent = join(realpathSync(source), 'parent') + const child = join(parent, 'child') + const backup = join(realpathSync(source), 'parent-backup') + const otherParent = join(outside, 'parent') + mkdirSync(child, { recursive: true }) + mkdirSync(join(otherParent, 'child'), { recursive: true }) + writeFileSync(join(child, 'allowed.txt'), 'allowed') + writeFileSync(join(otherParent, 'child', 'private.txt'), 'outside') + calls.ancestorRace = { toolName: 'read_local_file', args: { path: child } } 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 (operation: () => Promise): Promise => { - 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) + const originalStat = fs.lstat + const originalRealpath = fs.realpath + let swapped = false + const restore = async () => { + fs.lstat = originalStat + fs.realpath = originalRealpath + if (swapped) { + await fs.rm(paths.parent) + await fs.rename(paths.backup, paths.parent) + swapped = false } } - fs.opendir = (path, options) => - path === paths.source - ? swapped(() => originalOpen(path, options)) - : originalOpen(path, options) - fs.readdir = ((...args: Parameters) => - args[0] === paths.source - ? swapped(() => originalRead(...args)) - : originalRead(...args)) as typeof fs.readdir + ;( + globalThis as typeof globalThis & { restoreLocalFileRace?: () => Promise } + ).restoreLocalFileRace = restore + fs.lstat = (async (...args: Parameters) => { + const result = await originalStat(...args) + if (args[0] === paths.child) { + fs.lstat = originalStat + await fs.rename(paths.parent, paths.backup) + await fs.symlink(paths.otherParent, paths.parent) + swapped = true + } + return result + }) as typeof fs.lstat + fs.realpath = (async (...args: Parameters) => { + if (swapped && args[0] === paths.child) await restore() + return originalRealpath(...args) + }) as typeof fs.realpath }, - { source: realpathSync(source), backup: join(root, 'reports-backup'), outside } + { parent, child, backup, otherParent } ) - expect(await invoke({ operation: 'read', toolCallId: 'directory' })).toMatchObject({ - ok: false, - }) + try { + expect(await invoke({ operation: 'read', toolCallId: 'ancestorRace' })).toMatchObject({ + ok: true, + data: { entries: [{ name: 'allowed.txt', kind: 'file' }] }, + }) + } finally { + await app?.evaluate(async () => { + const runtime = globalThis as typeof globalThis & { + restoreLocalFileRace?: () => Promise + } + await runtime.restoreLocalFileRace?.() + runtime.restoreLocalFileRace = undefined + }) + rmSync(parent, { recursive: true, force: true }) + rmSync(otherParent, { recursive: true, force: true }) + } + }) + await test.step('cancelling after a file opens prevents its contents from returning', async () => { + await app?.evaluate( + (_electron, path) => { + const fs = process.getBuiltinModule( + 'node:fs/promises' + ) as typeof import('node:fs/promises') + const original = fs.open + fs.open = async (...args: Parameters) => { + const handle = await original(...args) + if (args[0] === path) { + fs.open = original + await new Promise((resolve) => { + ;( + globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } + ).releaseLocalFileRead = resolve + }) + } + return handle + } + }, + realpathSync(join(source, 'report.txt')) + ) + const reading = invoke({ operation: 'read', toolCallId: 'text' }) + void reading.catch(() => {}) + try { + await expect + .poll(() => + app?.evaluate( + () => + typeof (globalThis as typeof globalThis & { releaseLocalFileRead?: () => void }) + .releaseLocalFileRead === 'function' + ) + ) + .toBe(true) + await invoke({ operation: 'cancel', toolCallId: 'text' }) + } finally { + await app?.evaluate(() => { + const runtime = globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } + runtime.releaseLocalFileRead?.() + runtime.releaseLocalFileRead = undefined + }) + } + expect(await reading).toMatchObject({ ok: false }) }) await test.step('cancelling in the renderer closes consent without remembering access', async () => { calls.cancelled = { diff --git a/apps/desktop/native/directory.cc b/apps/desktop/native/directory.cc new file mode 100644 index 00000000000..0e40e20c07e --- /dev/null +++ b/apps/desktop/native/directory.cc @@ -0,0 +1,154 @@ +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +struct Entry { + std::string name; + const char* kind; +}; + +struct DirectoryRead { + napi_async_work work = nullptr; + napi_deferred deferred = nullptr; + int descriptor = -1; + size_t limit = 0; + bool truncated = false; + std::string error; + std::vector entries; +}; + +static const char* EntryKind(DIR* directory, const dirent* entry) { + switch (entry->d_type) { + case DT_REG: return "file"; + case DT_DIR: return "directory"; + case DT_LNK: return "symlink"; + case DT_UNKNOWN: { + struct stat metadata; + if (fstatat(dirfd(directory), entry->d_name, &metadata, AT_SYMLINK_NOFOLLOW) == 0) { + if (S_ISREG(metadata.st_mode)) return "file"; + if (S_ISDIR(metadata.st_mode)) return "directory"; + if (S_ISLNK(metadata.st_mode)) return "symlink"; + } + return "other"; + } + default: return "other"; + } +} + +static void ReadEntries(napi_env, void* data) { + auto* read = static_cast(data); + DIR* directory = fdopendir(read->descriptor); + if (!directory) { + close(read->descriptor); + read->descriptor = -1; + read->error = "Could not enumerate the approved directory."; + return; + } + read->descriptor = -1; + while (true) { + errno = 0; + const dirent* entry = readdir(directory); + if (!entry) { + if (errno != 0) read->error = "Could not finish reading the approved directory."; + break; + } + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue; + if (read->entries.size() == read->limit) { + read->truncated = true; + break; + } + read->entries.push_back({entry->d_name, EntryKind(directory, entry)}); + } + closedir(directory); +} + +static napi_value String(napi_env env, const char* value) { + napi_value result; + napi_create_string_utf8(env, value, NAPI_AUTO_LENGTH, &result); + return result; +} + +static void Complete(napi_env env, napi_status status, void* data) { + auto* read = static_cast(data); + if (read->descriptor >= 0) close(read->descriptor); + if (status != napi_ok || !read->error.empty()) { + napi_value error; + napi_create_error(env, nullptr, + String(env, read->error.empty() ? "Directory read cancelled." : read->error.c_str()), + &error); + napi_reject_deferred(env, read->deferred, error); + } else { + napi_value result; + napi_value entries; + napi_value truncated; + napi_create_object(env, &result); + napi_create_array_with_length(env, read->entries.size(), &entries); + for (size_t index = 0; index < read->entries.size(); index++) { + napi_value entry; + napi_create_object(env, &entry); + napi_set_named_property(env, entry, "name", String(env, read->entries[index].name.c_str())); + napi_set_named_property(env, entry, "kind", String(env, read->entries[index].kind)); + napi_set_element(env, entries, index, entry); + } + napi_get_boolean(env, read->truncated, &truncated); + napi_set_named_property(env, result, "entries", entries); + napi_set_named_property(env, result, "truncated", truncated); + napi_resolve_deferred(env, read->deferred, result); + } + napi_delete_async_work(env, read->work); + delete read; +} + +/** The descriptor is duplicated before scheduling; no pathname is reopened by the worker. */ +static napi_value ReadDirectory(napi_env env, napi_callback_info info) { + size_t count = 2; + napi_value arguments[2]; + double descriptor = -1; + double limit = 0; + if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 2 || + napi_get_value_double(env, arguments[0], &descriptor) != napi_ok || + napi_get_value_double(env, arguments[1], &limit) != napi_ok || + !std::isfinite(descriptor) || descriptor < 0 || descriptor > INT_MAX || + descriptor != std::floor(descriptor) || limit < 1 || limit > 1000 || limit != std::floor(limit)) { + napi_throw_type_error(env, nullptr, "Expected a directory descriptor and an entry limit from 1 to 1000."); + return nullptr; + } + auto* read = new DirectoryRead(); + read->limit = static_cast(limit); + read->descriptor = fcntl(static_cast(descriptor), F_DUPFD_CLOEXEC, 0); + if (read->descriptor < 0) { + delete read; + napi_throw_error(env, nullptr, "Could not retain the approved directory descriptor."); + return nullptr; + } + napi_value promise; + if (napi_create_promise(env, &read->deferred, &promise) != napi_ok || + napi_create_async_work(env, nullptr, String(env, "ReadApprovedDirectory"), ReadEntries, + Complete, read, &read->work) != napi_ok || + napi_queue_async_work(env, read->work) != napi_ok) { + close(read->descriptor); + if (read->work) napi_delete_async_work(env, read->work); + delete read; + napi_throw_error(env, nullptr, "Could not schedule the directory read."); + return nullptr; + } + return promise; +} + +NAPI_MODULE_INIT() { + napi_property_descriptor property = { + "readDirectory", nullptr, ReadDirectory, nullptr, nullptr, nullptr, napi_default, nullptr + }; + napi_define_properties(env, exports, 1, &property); + return exports; +} diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 3a6dbd01c5e..f52bcfe37de 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -25,8 +25,8 @@ "lint:check": "biome check .", "format": "biome format --write .", "format:check": "biome format .", - "test": "vitest run", - "test:watch": "vitest", + "test": "bun run scripts/build-native.ts && vitest run", + "test:watch": "bun run scripts/build-native.ts && vitest", "test:e2e": "playwright test" }, "dependencies": { diff --git a/apps/desktop/scripts/build-native.ts b/apps/desktop/scripts/build-native.ts new file mode 100644 index 00000000000..8980683f31f --- /dev/null +++ b/apps/desktop/scripts/build-native.ts @@ -0,0 +1,62 @@ +import { execFileSync } from 'node:child_process' +import { existsSync, mkdirSync, statSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { createLogger } from '@sim/logger' + +const logger = createLogger('DesktopNativeBuild') +if (process.platform !== 'darwin' && process.platform !== 'linux') { + throw new Error('Native desktop modules require macOS or Linux.') +} +const nodeExecutable = execFileSync('node', ['-p', 'process.execPath'], { encoding: 'utf8' }).trim() +const includeDirectory = join(dirname(nodeExecutable), '..', 'include', 'node') +if (!existsSync(join(includeDirectory, 'node_api.h'))) { + throw new Error(`Could not find Node-API headers in ${includeDirectory}`) +} +const outputDirectory = 'dist/native' +mkdirSync(outputDirectory, { recursive: true }) +const modules = [ + { name: 'directory', source: 'native/directory.cc', appKit: false }, + ...(process.platform === 'darwin' + ? [{ name: 'help-search', source: 'native/help-search.mm', appKit: true }] + : []), +] +for (const module of modules) { + const output = join(outputDirectory, `${module.name}.node`) + if ( + existsSync(output) && + statSync(output).mtimeMs >= + Math.max(statSync(module.source).mtimeMs, statSync(import.meta.filename).mtimeMs) + ) + continue + const macOS = process.platform === 'darwin' + execFileSync( + macOS ? 'xcrun' : 'c++', + [ + ...(macOS ? ['clang++'] : []), + '-std=c++17', + '-DNAPI_VERSION=8', + ...(macOS + ? [ + '-bundle', + '-undefined', + 'dynamic_lookup', + '-mmacosx-version-min=12.0', + '-arch', + 'arm64', + '-arch', + 'x86_64', + ] + : ['-shared', '-fPIC']), + '-I', + includeDirectory, + ...(module.appKit + ? ['-fobjc-arc', '-fblocks', '-framework', 'AppKit', '-framework', 'Foundation'] + : []), + '-o', + output, + module.source, + ], + { stdio: 'inherit' } + ) + logger.info('Compiled native desktop module', { module: module.name }) +} diff --git a/apps/desktop/scripts/build.ts b/apps/desktop/scripts/build.ts index b15dc2fb354..59527b6c616 100644 --- a/apps/desktop/scripts/build.ts +++ b/apps/desktop/scripts/build.ts @@ -1,6 +1,6 @@ import { execFileSync } from 'node:child_process' -import { cpSync, existsSync, mkdirSync, readFileSync, rmSync } from 'node:fs' -import { dirname, join, resolve } from 'node:path' +import { cpSync, readFileSync, rmSync } from 'node:fs' +import { dirname, resolve } from 'node:path' import { type BuildOptions, build } from 'esbuild' import postcss from 'postcss' import loadPostcssConfig from 'postcss-load-config' @@ -34,52 +34,6 @@ rmSync(generatedIcon, { force: true, recursive: true }) cpSync(appIcon, generatedIcon, { recursive: true }) console.log(`• Selecting desktop icon: ${appIcon}`) -function compileNativeHelpSearch(): void { - const outputDirectory = 'dist/native' - rmSync(outputDirectory, { force: true, recursive: true }) - if (process.platform !== 'darwin') return - - const nodeExecutable = execFileSync('node', ['-p', 'process.execPath'], { - encoding: 'utf8', - }).trim() - const nodeIncludeDirectory = join(dirname(nodeExecutable), '..', 'include', 'node') - const nodeApiHeader = join(nodeIncludeDirectory, 'node_api.h') - if (!existsSync(nodeApiHeader)) { - throw new Error(`Could not find Node-API headers at ${nodeApiHeader}`) - } - - mkdirSync(outputDirectory, { recursive: true }) - execFileSync( - 'xcrun', - [ - 'clang++', - '-std=c++17', - '-DNAPI_VERSION=8', - '-fobjc-arc', - '-fblocks', - '-bundle', - '-undefined', - 'dynamic_lookup', - '-mmacosx-version-min=12.0', - '-arch', - 'arm64', - '-arch', - 'x86_64', - '-I', - nodeIncludeDirectory, - '-framework', - 'AppKit', - '-framework', - 'Foundation', - '-o', - join(outputDirectory, 'help-search.node'), - 'native/help-search.mm', - ], - { stdio: 'inherit' } - ) - console.log('• Compiled native macOS documentation Help search') -} - const common = { bundle: true, platform: 'node' as const, @@ -138,7 +92,7 @@ const renderer: BuildOptions = { } async function run(): Promise { - compileNativeHelpSearch() + execFileSync(process.execPath, ['run', 'scripts/build-native.ts'], { stdio: 'inherit' }) if (watch) { const { context } = await import('esbuild') const rendererCtx = await context(renderer) diff --git a/apps/desktop/src/main/local-files.test.ts b/apps/desktop/src/main/local-files.test.ts index a7dd2c87d07..4ef5f6fdb46 100644 --- a/apps/desktop/src/main/local-files.test.ts +++ b/apps/desktop/src/main/local-files.test.ts @@ -1,7 +1,12 @@ import { mkdir, mkdtemp, realpath, rm, symlink, truncate, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, expect, it } from 'vitest' +import { fileURLToPath } from 'node:url' +import { app } from 'electron' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +vi.mock('electron', () => import('@/test/electron-mock')) + import { executeLocalFileRequest as executeApprovedLocalFileRequest, type LocalFileAuthorization, @@ -17,6 +22,7 @@ function executeLocalFileRequest(request: unknown, authorization: LocalFileAutho let root: string beforeEach(async () => { + vi.mocked(app.getAppPath).mockReturnValue(fileURLToPath(new URL('../..', import.meta.url))) root = await mkdtemp(join(tmpdir(), 'sim-native-files-')) }) afterEach(async () => { diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index d9179636ed4..4e5ae157fae 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -1,5 +1,5 @@ -import { constants, type Dirent } from 'node:fs' -import { lstat, open, opendir, stat } from 'node:fs/promises' +import { constants } from 'node:fs' +import { lstat, open, stat } from 'node:fs/promises' import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' import type { DesktopLocalFileEntry, @@ -13,6 +13,7 @@ import { isRecordLike } from '@sim/utils/object' import { compareStrings } from '@sim/utils/string' import { PDFDocument } from 'pdf-lib' import type { LocalFileAccess } from '@/main/local-filesystem' +import { readNativeDirectory } from '@/main/native-directory' const CHUNK_BYTES = 8 * 1024 * 1024 const MAX_ENTRIES = 1000 @@ -72,45 +73,11 @@ async function openApprovedPath(path: string, access: LocalFileAccess, directory async function readApprovedDirectory(path: string, access: LocalFileAccess) { const handle = await openApprovedPath(path, access, true) try { - const before = await handle.stat({ bigint: true }) - const verify = async () => { - const canonical = await access.resolve(path) - const current = await lstat(canonical, { bigint: true }) - const after = await handle.stat({ bigint: true }) - if ( - canonical !== path || - !current.isDirectory() || - current.dev !== before.dev || - current.ino !== before.ino || - current.ctimeNs !== before.ctimeNs || - current.mtimeNs !== before.mtimeNs || - after.ctimeNs !== before.ctimeNs || - after.mtimeNs !== before.mtimeNs - ) { - throw new Error('The local directory changed while it was being read. Try again.') - } - } - const directory = await opendir(path) - try { - await verify() - const entries: Dirent[] = [] - let count = 0 - const sort = () => entries.sort((left, right) => compareStrings(left.name, right.name)) - for (let entry = await directory.read(); entry; entry = await directory.read()) { - entries.push(entry) - count++ - if (entries.length === MAX_ENTRIES * 2) { - sort() - entries.length = MAX_ENTRIES - await access.resolve(path) - } - } - await verify() - sort() - return { entries: entries.slice(0, MAX_ENTRIES), truncated: count > MAX_ENTRIES } - } finally { - await directory.close() - } + const listing = await readNativeDirectory(handle.fd, MAX_ENTRIES) + if ((await access.resolve(path)) !== path) + throw new Error('The local directory changed while it was being read. Try again.') + listing.entries.sort((left, right) => compareStrings(left.name, right.name)) + return listing } finally { await handle.close() } @@ -129,16 +96,7 @@ async function inspect( path, representation: 'directory', truncated, - entries: entries.map((entry) => ({ - name: entry.name, - kind: entry.isFile() - ? 'file' - : entry.isDirectory() - ? 'directory' - : entry.isSymbolicLink() - ? 'symlink' - : 'other', - })), + entries, } } if (!info.isFile()) throw new Error('The path is not a regular file or directory.') diff --git a/apps/desktop/src/main/native-directory.ts b/apps/desktop/src/main/native-directory.ts new file mode 100644 index 00000000000..4babef7bde9 --- /dev/null +++ b/apps/desktop/src/main/native-directory.ts @@ -0,0 +1,25 @@ +import { join } from 'node:path' +import type { DesktopLocalFileRead } from '@sim/desktop-bridge' +import { app } from 'electron' + +interface NativeDirectoryListing { + entries: NonNullable + truncated: boolean +} + +interface NativeDirectoryBridge { + readDirectory: (descriptor: number, limit: number) => Promise +} + +let bridge: NativeDirectoryBridge | undefined + +/** Enumerates the already-validated descriptor without resolving a pathname again. */ +export function readNativeDirectory( + descriptor: number, + limit: number +): Promise { + bridge ??= require( + join(app.getAppPath(), 'dist', 'native', 'directory.node') + ) as NativeDirectoryBridge + return bridge.readDirectory(descriptor, limit) +} From 9d3e234ed81a14c2ed72cd4f7561b851f06c07c8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 26 Sep 2026 15:59:00 -0700 Subject: [PATCH 5/8] fix(desktop): reject replaced directory listings --- apps/desktop/README.md | 2 +- apps/desktop/e2e/local-files.spec.ts | 34 ++++++++++++++++++++++++++++ apps/desktop/src/main/local-files.ts | 5 +++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 61b1cee9504..37f89a0c4ad 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -190,7 +190,7 @@ Copilot can inspect user-selected local directories through the ordinary VFS too 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. Directory enumeration uses `fdopendir` on the verified descriptor, so replacing a parent path cannot redirect the listing. 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. +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. ## Auto-update, channels, rollout, rollback diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 7fe39192b71..d9edcd058f1 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -320,6 +320,40 @@ test('native file tools remember folder consent across chats and restarts until rmSync(otherParent, { recursive: true, force: true }) } }) + await test.step('a replaced directory cannot return a listing for its old contents', async () => { + const directory = join(realpathSync(source), 'replace-during-read') + const backup = join(realpathSync(source), 'previous-directory') + mkdirSync(directory) + writeFileSync(join(directory, 'old.txt'), 'old contents') + calls.directoryReplaced = { toolName: 'read_local_file', args: { path: directory } } + await app?.evaluate( + (_electron, paths) => { + const fs = process.getBuiltinModule( + 'node:fs/promises' + ) as typeof import('node:fs/promises') + const original = fs.lstat + fs.lstat = (async (...args: Parameters) => { + const result = await original(...args) + if (args[0] === paths.directory) { + fs.lstat = original + await fs.rename(paths.directory, paths.backup) + await fs.mkdir(paths.directory) + await fs.writeFile(`${paths.directory}/new.txt`, 'new contents') + } + return result + }) as typeof fs.lstat + }, + { directory, backup } + ) + try { + expect(await invoke({ operation: 'read', toolCallId: 'directoryReplaced' })).toMatchObject({ + ok: false, + }) + } finally { + rmSync(directory, { recursive: true, force: true }) + rmSync(backup, { recursive: true, force: true }) + } + }) await test.step('cancelling after a file opens prevents its contents from returning', async () => { await app?.evaluate( (_electron, path) => { diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index 4e5ae157fae..77f1146cfe7 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -74,7 +74,10 @@ async function readApprovedDirectory(path: string, access: LocalFileAccess) { const handle = await openApprovedPath(path, access, true) try { const listing = await readNativeDirectory(handle.fd, MAX_ENTRIES) - if ((await access.resolve(path)) !== path) + const canonical = await access.resolve(path) + const current = await lstat(canonical) + const opened = await handle.stat() + if (canonical !== path || current.dev !== opened.dev || current.ino !== opened.ino) throw new Error('The local directory changed while it was being read. Try again.') listing.entries.sort((left, right) => compareStrings(left.name, right.name)) return listing From cab5ad9f2e2db724145c065a30ebca6e2d420b36 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 5 Oct 2026 16:13:39 -0700 Subject: [PATCH 6/8] fix(desktop): share pending folder consent decisions --- apps/desktop/README.md | 2 +- apps/desktop/e2e/local-files.spec.ts | 14 +++-- .../src/main/local-file-permissions.ts | 51 +++++++++---------- 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 37f89a0c4ad..65e32d29f90 100644 --- a/apps/desktop/README.md +++ b/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. Existing encrypted path-based approvals retain their scope and acquire folder-identity metadata on their first restore. +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. 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. diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index d9edcd058f1..3ed50866f6f 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -143,8 +143,12 @@ test('native file tools remember folder consent across chats and restarts until await api.settings.setPreference('terminalEnabled', false) }) const deniedPrompt = app.waitForEvent('window', { timeout: 10_000 }) - const deniedRead = invoke({ operation: 'read', toolCallId: 'text' }) - void deniedRead.catch(() => {}) + const deniedReads = ['text', 'otherChat'].map((toolCallId) => + invoke({ operation: 'read', toolCallId }) + ) + const deniedResults: unknown[] = [] + for (const read of deniedReads) + void read.then((result) => deniedResults.push(result)).catch(() => {}) const denial = await deniedPrompt await expect(denial.getByRole('button', { name: "Don't allow", exact: true })).toBeFocused() await denial.screenshot({ @@ -153,7 +157,11 @@ test('native file tools remember folder consent across chats and restarts until test.info().outputPath('local-file-consent.png'), }) await denial.getByRole('button', { name: "Don't allow", exact: true }).click() - expect(await deniedRead).toMatchObject({ ok: false }) + await expect.poll(() => deniedResults.length).toBe(2) + expect(deniedResults).toEqual([ + { ok: false, error: expect.any(String) }, + { ok: false, error: expect.any(String) }, + ]) const folderPrompt = app.waitForEvent('window') const folderRead = invoke({ operation: 'read', toolCallId: 'directory' }) diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts index bfc7133f0f2..73a0600287f 100644 --- a/apps/desktop/src/main/local-file-permissions.ts +++ b/apps/desktop/src/main/local-file-permissions.ts @@ -40,10 +40,10 @@ async function revalidate(context: LocalFilePermissionContext): Promise { assertCurrent(context) } -/** Serializes new consent prompts while remembered folder access remains concurrent. */ +/** Shares each pending folder decision while remembered access remains concurrent. */ export class LocalFilePermissions { private queue: Promise = Promise.resolve() - private pending = 0 + private readonly pending = new Map>() constructor(private readonly filesystem: LocalFilesystemService) {} @@ -62,32 +62,32 @@ export class LocalFilePermissions { const path = await realpath(nativePath(authorization.args.path)) const existing = await this.filesystem.nativeAccess(path) if (existing) return this.authorizedAccess(existing, context) - if (this.pending >= MAX_PENDING_REQUESTS) - throw new Error('Too many local file requests are waiting for permission. Try again later.') - this.pending++ - const pending = this.queue.then(() => this.requestFolder(path, context)) - this.queue = pending.then( - () => undefined, - () => undefined - ) - try { - return await pending - } finally { - this.pending-- - } - } - - private async requestFolder( - path: string, - context: LocalFilePermissionContext - ): Promise { - await revalidate(context) - const existing = await this.filesystem.nativeAccess(path) - if (existing) return this.authorizedAccess(existing, context) const info = await stat(path) if (!info.isFile() && !info.isDirectory()) throw new Error('The path is not a regular file or directory.') const folder = info.isDirectory() ? path : dirname(path) + const key = JSON.stringify([context.generation, context.origin, folder]) + let pending = this.pending.get(key) + if (!pending) { + if (this.pending.size >= MAX_PENDING_REQUESTS) + throw new Error('Too many local file requests are waiting for permission. Try again later.') + const decision = this.queue.then(() => this.requestFolder(folder, context)) + this.queue = decision.then( + () => undefined, + () => undefined + ) + pending = decision.finally(() => this.pending.delete(key)) + this.pending.set(key, pending) + } + await pending + const access = await this.filesystem.nativeAccess(path) + if (!access) throw new Error('The approved folder is no longer available.') + return this.authorizedAccess(access, context) + } + + private async requestFolder(folder: string, context: LocalFilePermissionContext): Promise { + await revalidate(context) + if (await this.filesystem.nativeAccess(folder)) return const root = await lstat(folder) if (!root.isDirectory()) throw new Error('The folder is no longer available.') const displayedPath = JSON.stringify(folder).replace( @@ -108,9 +108,6 @@ export class LocalFilePermissions { if (result.response !== 0) throw new Error('The user did not allow this local file access.') await revalidate(context) await this.filesystem.grantDirectory({ path: folder }, context.generation, root) - const access = await this.filesystem.nativeAccess(path) - if (!access) throw new Error('The approved folder is no longer available.') - return this.authorizedAccess(access, context) } private async authorizedAccess( From 16a4dce855b89f8c597a54c6c2d8c25dc10acc34 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 8 Oct 2026 17:19:35 -0700 Subject: [PATCH 7/8] fix(desktop): complete file consent and add full file access --- apps/desktop/README.md | 2 + apps/desktop/e2e/background-executor.spec.ts | 49 +++ .../e2e/desktop-tools-live-sim.spec.ts | 6 + apps/desktop/e2e/local-files.spec.ts | 353 ++++++++++++++++-- apps/desktop/native/directory.cc | 177 ++++++++- apps/desktop/src/main/config.ts | 2 + .../src/main/desktop-executor/runner.test.ts | 28 +- .../src/main/desktop-executor/runner.ts | 18 +- apps/desktop/src/main/desktop-settings.ts | 10 + apps/desktop/src/main/index.ts | 42 ++- apps/desktop/src/main/ipc.test.ts | 10 +- apps/desktop/src/main/ipc.ts | 19 +- .../src/main/local-file-permissions.ts | 135 ++++++- apps/desktop/src/main/local-files.test.ts | 9 +- apps/desktop/src/main/local-files.ts | 38 +- .../desktop/src/main/local-filesystem.test.ts | 2 +- apps/desktop/src/main/local-filesystem.ts | 34 +- apps/desktop/src/main/native-directory.ts | 54 ++- apps/desktop/src/preload/index.ts | 2 + .../settings/components/desktop/desktop.tsx | 40 +- packages/desktop-bridge/src/index.ts | 4 + 21 files changed, 897 insertions(+), 137 deletions(-) diff --git a/apps/desktop/README.md b/apps/desktop/README.md index 65e32d29f90..25097cef29b 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -190,6 +190,8 @@ Copilot can inspect user-selected local directories through the ordinary VFS too 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. +**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. + 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. ## Auto-update, channels, rollout, rollback diff --git a/apps/desktop/e2e/background-executor.spec.ts b/apps/desktop/e2e/background-executor.spec.ts index 06cc2dbcfd7..a9adadc7c1a 100644 --- a/apps/desktop/e2e/background-executor.spec.ts +++ b/apps/desktop/e2e/background-executor.spec.ts @@ -104,7 +104,9 @@ test.describe('background executor', () => { args: { command: `sleep 1; echo B-${n} >> '${marker}'`, waitSeconds: 30 }, }) ) + const readConsent = launched.app.waitForEvent('window') const localRead = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + await (await readConsent).getByRole('button', { name: 'Allow folder', exact: true }).click() await window.goto(`${sim.origin}/workspace/ws-other/home`) await window.reload() @@ -157,6 +159,50 @@ test.describe('background executor', () => { }) }) + test('background file reads require consent and Stop cancels pending permission', async () => { + const userData = mkdtempSync(join(tmpdir(), 'sim-executor-consent-')) + const launched = await launch(sim, userData) + app = launched.app + const deviceId = await registeredDevice(sim) + const readable = join(userData, 'private.txt') + writeFileSync(readable, 'background consent fixture') + + await check('background read stays pending until folder consent', async () => { + const shown = launched.app.waitForEvent('window', { timeout: 10_000 }) + const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + const prompt = await shown + await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible() + expect(sim.requireCall(call).completions).toHaveLength(0) + await prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + const completion = await settled(sim, call) + expect(completion.status).toBe('error') + expect(JSON.stringify(completion)).not.toContain('background consent fixture') + }) + + await check('Stop dismisses background consent without granting access', async () => { + const shown = launched.app.waitForEvent('window', { timeout: 10_000 }) + const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + const prompt = await shown + await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible() + sim.stopCall(call) + await expect.poll(() => prompt.isClosed()).toBe(true) + await settled(sim, call) + expect(sim.requireCall(call).completions[0]?.outcome).toBe('superseded') + }) + + await check('approved background reads reuse the shared folder grant', async () => { + const shown = launched.app.waitForEvent('window', { timeout: 10_000 }) + const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + const prompt = await shown + await prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + expect((await settled(sim, call)).status).toBe('success') + const next = sim.issue(deviceId, CHAT_A, 'read_local_file', { path: readable }) + expect(JSON.stringify((await settled(sim, next)).data)).toContain( + 'background consent fixture' + ) + }) + }) + test('B: a result produced while the network is cut is delivered once after reconnecting', async () => { const userData = mkdtempSync(join(tmpdir(), 'sim-executor-b-')) app = (await launch(sim, userData)).app @@ -237,12 +283,15 @@ test.describe('background executor', () => { writeFileSync(join(source, 'q3', 'export.bin'), large) await launched.window.goto(`${sim.origin}/workspace/${WORKSPACE}/chat/${CHAT_C}`) + const importConsent = launched.app.waitForEvent('window') const call = sim.issue(deviceId, CHAT_B, 'import_local_files', { path: source, targetWorkspaceId: WORKSPACE, folderId: 'folder-e2e', }) + await (await importConsent).getByRole('button', { name: 'Allow folder', exact: true }).click() + await check('D: the import completes with every entry it stored', async () => { const completion = await settled(sim, call, 60_000) expect(completion.status).toBe('success') diff --git a/apps/desktop/e2e/desktop-tools-live-sim.spec.ts b/apps/desktop/e2e/desktop-tools-live-sim.spec.ts index 6a4fe359933..20b2262b17b 100644 --- a/apps/desktop/e2e/desktop-tools-live-sim.spec.ts +++ b/apps/desktop/e2e/desktop-tools-live-sim.spec.ts @@ -245,6 +245,12 @@ test.describe('desktop tools against a live Sim', () => { }) const page = await app.firstWindow({ timeout }) pageErrors = [] + app.on('window', (permission) => { + void permission + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ timeout: 10_000 }) + .catch((error) => pageErrors.push(`Folder approval failed: ${String(error)}`)) + }) page.on('pageerror', (error) => pageErrors.push(error.message)) page.on('console', (message) => { if (message.type() === 'error') pageErrors.push(message.text()) diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 3ed50866f6f..10c19ebac31 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -1,6 +1,7 @@ import { mkdirSync, mkdtempSync, + readFileSync, realpathSync, renameSync, rmSync, @@ -13,10 +14,15 @@ import { join } from 'node:path' import { fileURLToPath } from 'node:url' import { type ElectronApplication, _electron as electron, expect, test } from '@playwright/test' import type { DesktopLocalFileRequest, SimDesktopApi } from '@sim/desktop-bridge' +import { build } from 'esbuild' +import postcss from 'postcss' +import loadPostcssConfig from 'postcss-load-config' const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url)) +const SIM_DIR = fileURLToPath(new URL('../../sim/', import.meta.url)) test('native file tools remember folder consent across chats and restarts until revoked', async () => { + test.setTimeout(180_000) const root = mkdtempSync(join(tmpdir(), 'sim-native-files-e2e-')) const source = join(root, 'Reports') const outside = join(root, 'Reports-other') @@ -50,8 +56,55 @@ test('native file tools remember folder consent across chats and restarts until let server: Server | undefined let app: ElectronApplication | undefined try { + const bundle = await build({ + stdin: { + contents: `import { createRoot } from 'react-dom/client'; +import { ToastProvider } from '@sim/emcn'; +import { AppRouterContext } from 'next/dist/shared/lib/app-router-context.shared-runtime'; +import { PathParamsContext } from 'next/dist/shared/lib/hooks-client-context.shared-runtime'; +import { Desktop } from '@/app/workspace/[workspaceId]/settings/components/desktop/desktop'; +import { SettingsHeaderProvider, SettingsHeaderShell } from '@/components/settings/settings-header'; +import { SettingsSectionProvider } from '@/components/settings/settings-panel'; +const router = { bfcacheId: 'fixture', back: () => history.back(), forward: () => history.forward(), refresh: () => location.reload(), push: url => location.assign(url), replace: url => location.replace(url), prefetch: () => {} }; +createRoot(document.getElementById('settings')).render( + + + + + + + +);`, + resolveDir: SIM_DIR, + loader: 'tsx', + }, + bundle: true, + jsx: 'automatic', + write: false, + outfile: test.info().outputPath('settings.js'), + external: ['node:async_hooks', 'postgres'], + banner: { js: 'var process={env:{NODE_ENV:"development"},browser:true};' }, + format: 'iife', + platform: 'browser', + tsconfig: join(SIM_DIR, 'tsconfig.json'), + define: { 'process.env.NODE_ENV': '"development"' }, + }) + const config = await loadPostcssConfig({}, SIM_DIR) + const cssPath = join(SIM_DIR, 'app/_styles/globals.css') + const css = await postcss(config.plugins).process(readFileSync(cssPath, 'utf8'), { + from: cssPath, + }) server = createServer(async (request, response) => { const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname + if (path === '/settings.js' || path === '/settings.css') { + response.setHeader('Content-Type', path.endsWith('.js') ? 'text/javascript' : 'text/css') + response.end( + path.endsWith('.js') + ? bundle.outputFiles.find((file) => file.path.endsWith('.js'))?.text + : css.css + ) + return + } if (path === '/api/auth/get-session') { response.writeHead(200, { 'Content-Type': 'application/json' }).end( JSON.stringify( @@ -101,11 +154,12 @@ test('native file tools remember folder consent across chats and restarts until ? { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/' } : {}), }) - .end(`Local file fixture

Local files

+ .end(`Local file fixture

Local files

`) + })">Forget folders +
`) }) await new Promise((resolve) => server?.listen(0, '127.0.0.1', resolve)) const address = server.address() @@ -122,7 +176,16 @@ test('native file tools remember folder consent across chats and restarts until }) app = await launch() let window = await app.firstWindow() - await expect(window.getByRole('heading')).toHaveText('Local files') + await app.evaluate(({ app, BrowserWindow }) => { + const host = BrowserWindow.getAllWindows()[0] + if (!host) throw new Error('Missing host window') + host.webContents.setBackgroundThrottling(false) + app.focus({ steal: true }) + host.focus() + }) + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) const invoke = (input: DesktopLocalFileRequest) => window.evaluate(async (request) => { const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop @@ -197,6 +260,124 @@ test('native file tools remember folder consent across chats and restarts until await expect(prompt.getByRole('button', { name: "Don't allow", exact: true })).toBeVisible() return { prompt, result } } + await test.step('Stop cancels only its own request while chats share a folder prompt', async () => { + const sharedFolder = join(root, 'Shared') + mkdirSync(sharedFolder) + writeFileSync(join(sharedFolder, 'shared.txt'), 'shared contents') + for (const toolCallId of ['sharedLeader', 'sharedFollower', 'sharedSurvivor']) { + calls[toolCallId] = { + toolName: 'read_local_file', + args: { path: join(sharedFolder, 'shared.txt') }, + chatId: toolCallId, + } + } + const leader = await requestPermission({ operation: 'read', toolCallId: 'sharedLeader' }) + let followerResult: unknown + const follower = invoke({ operation: 'read', toolCallId: 'sharedFollower' }).then( + (result) => { + followerResult = result + } + ) + void follower.catch(() => {}) + await expect.poll(() => claimed.has('sharedFollower')).toBe(true) + await leader.prompt.getByRole('dialog').hover() + await invoke({ operation: 'cancel', toolCallId: 'sharedFollower' }) + await expect.poll(() => followerResult).toMatchObject({ ok: false }) + await follower + await expect(leader.prompt.getByRole('dialog')).toBeVisible() + const survivor = invoke({ operation: 'read', toolCallId: 'sharedSurvivor' }) + void survivor.catch(() => {}) + await expect.poll(() => claimed.has('sharedSurvivor')).toBe(true) + await leader.prompt.getByRole('dialog').hover() + await invoke({ operation: 'cancel', toolCallId: 'sharedLeader' }) + expect(await leader.result).toMatchObject({ ok: false }) + await expect(leader.prompt.getByRole('dialog')).toBeVisible() + await leader.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + expect(await survivor).toMatchObject({ ok: true, data: { text: 'shared contents' } }) + }) + await test.step('Full file access is opt-in, survives restart, and stops granting access when disabled', async () => { + const fullFolder = join(root, 'Full access') + mkdirSync(fullFolder) + writeFileSync(join(fullFolder, 'file.txt'), 'full access contents') + calls.fullAccess = { + toolName: 'read_local_file', + args: { path: join(fullFolder, 'file.txt') }, + } + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).not.toBeChecked() + await window.getByRole('switch', { name: 'Full file access', exact: true }).click() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeChecked() + expect(await invoke({ operation: 'read', toolCallId: 'fullAccess' })).toMatchObject({ + ok: true, + data: { text: 'full access contents' }, + }) + calls.fullImport = { + toolName: 'import_local_files', + args: { path: fullFolder, targetWorkspaceId: 'target-workspace' }, + } + const manifest = await invoke({ operation: 'manifest', toolCallId: 'fullImport' }) + if (!manifest.ok || manifest.data.kind !== 'manifest') + throw new Error('Missing full-access manifest') + const imported = manifest.data.entries.find((entry) => entry.relativePath === 'file.txt') + if (!imported) throw new Error('Missing full-access file') + const chunk = await invoke({ + operation: 'chunk', + toolCallId: 'fullImport', + relativePath: imported.relativePath, + revision: imported.revision, + offset: 0, + }) + if (!chunk.ok || chunk.data.kind !== 'chunk') throw new Error('Missing full-access contents') + expect(Object.values(chunk.data.bytes)).toEqual([...Buffer.from('full access contents')]) + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeChecked() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeEnabled() + await window.screenshot({ + path: test.info().outputPath('desktop-settings-full-access.png'), + animations: 'disabled', + }) + await window.evaluate(() => document.documentElement.classList.add('dark')) + await window.screenshot({ + path: test.info().outputPath('desktop-settings-full-access-dark.png'), + animations: 'disabled', + }) + await window.evaluate(() => document.documentElement.classList.remove('dark')) + expect(await invoke({ operation: 'read', toolCallId: 'notAuthorized' })).toMatchObject({ + ok: false, + }) + await app?.close() + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) + await expect + .poll(() => + window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ) + .toMatchObject({ fullFileAccess: true }) + expect(await invoke({ operation: 'read', toolCallId: 'fullAccess' })).toMatchObject({ + ok: true, + data: { text: 'full access contents' }, + }) + await window.getByRole('switch', { name: 'Full file access', exact: true }).click() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).not.toBeChecked() + const permission = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' }) + await permission.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect(await permission.result).toMatchObject({ ok: false }) + }) await test.step('a folder grant works in another chat but does not permit symlink escapes', async () => { expect(await invoke({ operation: 'read', toolCallId: 'otherChat' })).toMatchObject({ ok: true, @@ -233,6 +414,63 @@ test('native file tools remember folder consent across chats and restarts until expect(await stale.result).toMatchObject({ ok: false }) } }) + await test.step('native traversal rejects redirected ancestors and replaced grant identities', async () => { + const parent = join(realpathSync(source), 'native-parent') + mkdirSync(parent) + writeFileSync(join(parent, 'inside.txt'), 'inside') + const linked = join(realpathSync(source), 'native-link') + symlinkSync(outside, linked) + try { + const result = await app?.evaluate( + async ({ app }, paths) => { + const { createRequire } = process.getBuiltinModule('node:module') + const { stat } = process.getBuiltinModule('node:fs/promises') + const fs = process.getBuiltinModule('node:fs') + const native = createRequire(`${app.getAppPath()}/package.json`)( + './dist/native/directory.node' + ) as { + openApproved( + root: string, + relative: string, + dev: number, + ino: number, + directory: boolean + ): Promise + } + const root = await stat(paths.source) + const denied = async (path: string, ino = root.ino) => { + try { + const fd = await native.openApproved(paths.source, path, root.dev, ino, false) + fs.closeSync(fd) + return false + } catch { + return true + } + } + const valid = await native.openApproved( + paths.source, + 'native-parent/inside.txt', + root.dev, + root.ino, + false + ) + const text = fs.readFileSync(valid, 'utf8') + fs.closeSync(valid) + return { + text, + ancestor: await denied('native-link/private.txt'), + traversal: await denied('../Reports-other/private.txt'), + replaced: await denied('native-parent/inside.txt', root.ino + 1), + } + }, + { source: realpathSync(source) } + ) + expect(result).toEqual({ text: 'inside', ancestor: true, traversal: true, replaced: true }) + } finally { + rmSync(linked) + rmSync(parent, { recursive: true }) + } + }) 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') @@ -341,14 +579,13 @@ test('native file tools remember folder consent across chats and restarts until ) as typeof import('node:fs/promises') const original = fs.lstat fs.lstat = (async (...args: Parameters) => { - const result = await original(...args) if (args[0] === paths.directory) { fs.lstat = original await fs.rename(paths.directory, paths.backup) await fs.mkdir(paths.directory) await fs.writeFile(`${paths.directory}/new.txt`, 'new contents') } - return result + return original(...args) }) as typeof fs.lstat }, { directory, backup } @@ -362,29 +599,26 @@ test('native file tools remember folder consent across chats and restarts until rmSync(backup, { recursive: true, force: true }) } }) - await test.step('cancelling after a file opens prevents its contents from returning', async () => { - await app?.evaluate( - (_electron, path) => { - const fs = process.getBuiltinModule( - 'node:fs/promises' - ) as typeof import('node:fs/promises') - const original = fs.open - fs.open = async (...args: Parameters) => { - const handle = await original(...args) - if (args[0] === path) { - fs.open = original - await new Promise((resolve) => { - ;( - globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } - ).releaseLocalFileRead = resolve - }) - } - return handle + await test.step('cancelling a directory read prevents its contents from returning', async () => { + const path = realpathSync(join(source, 'empty')) + calls.directoryCancel = { toolName: 'read_local_file', args: { path } } + await app?.evaluate((_electron, path) => { + const fs = process.getBuiltinModule('node:fs/promises') as typeof import('node:fs/promises') + const original = fs.lstat + fs.lstat = (async (...args: Parameters) => { + const handle = await original(...args) + if (args[0] === path) { + fs.lstat = original + await new Promise((resolve) => { + ;( + globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } + ).releaseLocalFileRead = resolve + }) } - }, - realpathSync(join(source, 'report.txt')) - ) - const reading = invoke({ operation: 'read', toolCallId: 'text' }) + return handle + }) as typeof fs.lstat + }, path) + const reading = invoke({ operation: 'read', toolCallId: 'directoryCancel' }) void reading.catch(() => {}) try { await expect @@ -396,7 +630,7 @@ test('native file tools remember folder consent across chats and restarts until ) ) .toBe(true) - await invoke({ operation: 'cancel', toolCallId: 'text' }) + await invoke({ operation: 'cancel', toolCallId: 'directoryCancel' }) } finally { await app?.evaluate(() => { const runtime = globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } @@ -514,7 +748,9 @@ test('native file tools remember folder consent across chats and restarts until await app?.close() app = await launch() window = await app.firstWindow() - await expect(window.getByRole('heading')).toHaveText('Local files') + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true }) }) await test.step('forgetting a folder revokes native reads and survives restart', async () => { @@ -523,7 +759,9 @@ test('native file tools remember folder consent across chats and restarts until await app?.close() app = await launch() window = await app.firstWindow() - await expect(window.getByRole('heading')).toHaveText('Local files') + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) await revoked.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() expect(await revoked.result).toMatchObject({ ok: true }) @@ -536,7 +774,9 @@ test('native file tools remember folder consent across chats and restarts until writeFileSync(join(source, 'report.txt'), 'replacement contents') app = await launch() window = await app.firstWindow() - await expect(window.getByRole('heading')).toHaveText('Local files') + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) const replaced = await requestPermission({ operation: 'read', toolCallId: 'text' }) await replaced.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() expect(await replaced.result).toMatchObject({ ok: false }) @@ -547,7 +787,11 @@ test('native file tools remember folder consent across chats and restarts until expect(await restored.result).toMatchObject({ ok: true }) }) - await test.step('sign-out revokes remembered grants before the next account session', async () => { + await test.step('sign-out revokes remembered grants and Full file access before the next account session', async () => { + await window.getByRole('switch', { name: 'Full file access', exact: true }).click() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeChecked() await app?.evaluate(({ Menu }) => { const item = Menu.getApplicationMenu() ?.items.flatMap((entry) => entry.submenu?.items ?? []) @@ -566,10 +810,55 @@ test('native file tools remember folder consent across chats and restarts until }) ) .toBe(true) + await expect + .poll(() => + window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ) + .toMatchObject({ fullFileAccess: false }) const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) await revoked.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() expect(await revoked.result).toMatchObject({ ok: false }) }) + await test.step('a failed settings write reports the error and leaves Full file access disabled', async () => { + const settingsPath = join(root, 'profile', 'settings.json') + const saved = JSON.parse(readFileSync(settingsPath, 'utf8')) + for (const previous of [false, true]) { + await app?.close() + rmSync(settingsPath, { recursive: true, force: true }) + writeFileSync(settingsPath, JSON.stringify({ ...saved, fullFileAccess: previous })) + app = await launch() + window = await app.firstWindow() + const toggle = window.getByRole('switch', { name: 'Full file access', exact: true }) + await expect(toggle).toBeChecked({ checked: previous }) + renameSync(settingsPath, `${settingsPath}.backup`) + mkdirSync(settingsPath) + try { + await toggle.click() + await expect( + window.getByText('Could not update file access', { exact: true }) + ).toBeVisible() + expect( + await window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ).toMatchObject({ fullFileAccess: false }) + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ + ok: false, + }) + } finally { + await app.close() + app = undefined + rmSync(settingsPath, { recursive: true, force: true }) + renameSync(`${settingsPath}.backup`, settingsPath) + } + } + }) } finally { await app?.close() server?.close() diff --git a/apps/desktop/native/directory.cc b/apps/desktop/native/directory.cc index 0e40e20c07e..22ab5a113d1 100644 --- a/apps/desktop/native/directory.cc +++ b/apps/desktop/native/directory.cc @@ -145,10 +145,181 @@ static napi_value ReadDirectory(napi_env env, napi_callback_info info) { return promise; } +struct ApprovedOpen { + napi_async_work work = nullptr; + napi_deferred deferred = nullptr; + std::string root; + std::string relative; + double dev = 0; + double ino = 0; + bool directory = false; + int descriptor = -1; + std::string error; +}; + +static void OpenApprovedPath(napi_env, void* data) { + auto* request = static_cast(data); + int descriptor = open(request->root.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + struct stat metadata; + if (descriptor < 0 || fstat(descriptor, &metadata) != 0 || + static_cast(metadata.st_dev) != request->dev || + static_cast(metadata.st_ino) != request->ino) { + if (descriptor >= 0) close(descriptor); + request->error = "The approved folder changed. Request access again."; + return; + } + size_t start = 0; + while (start < request->relative.size()) { + const size_t end = request->relative.find('/', start); + const bool last = end == std::string::npos; + const std::string component = request->relative.substr(start, last ? end : end - start); + if (component.empty() || component == "." || component == "..") { + close(descriptor); + request->error = "The path must stay within the approved folder."; + return; + } + const int next = openat(descriptor, component.c_str(), + O_RDONLY | O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK | + ((!last || request->directory) ? O_DIRECTORY : 0)); + close(descriptor); + if (next < 0) { + request->error = "Could not open the path within the approved folder."; + return; + } + descriptor = next; + if (last) break; + start = end + 1; + } + if (fstat(descriptor, &metadata) != 0 || + (request->directory ? !S_ISDIR(metadata.st_mode) : !S_ISREG(metadata.st_mode))) { + close(descriptor); + request->error = "The approved path is not a regular file or directory."; + return; + } + request->descriptor = descriptor; +} + +static void CompleteOpen(napi_env env, napi_status status, void* data) { + auto* request = static_cast(data); + if (status != napi_ok || !request->error.empty()) { + if (request->descriptor >= 0) close(request->descriptor); + napi_value error; + napi_create_error(env, nullptr, String(env, request->error.empty() + ? "File open cancelled." : request->error.c_str()), &error); + napi_reject_deferred(env, request->deferred, error); + } else { + napi_value descriptor; + napi_create_int32(env, request->descriptor, &descriptor); + napi_resolve_deferred(env, request->deferred, descriptor); + } + napi_delete_async_work(env, request->work); + delete request; +} + +static bool ReadPath(napi_env env, napi_value value, std::string& path) { + size_t length = 0; + if (napi_get_value_string_utf8(env, value, nullptr, 0, &length) != napi_ok || length > 4096) + return false; + std::vector buffer(length + 1); + if (napi_get_value_string_utf8(env, value, buffer.data(), buffer.size(), &length) != napi_ok) + return false; + path.assign(buffer.data(), length); + return path.find('\0') == std::string::npos; +} + +/** Opens each component relative to the verified grant descriptor; no ancestor is followed. */ +static napi_value OpenApproved(napi_env env, napi_callback_info info) { + size_t count = 5; + napi_value arguments[5]; + auto* request = new ApprovedOpen(); + if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 5 || + !ReadPath(env, arguments[0], request->root) || request->root.empty() || request->root[0] != '/' || + !ReadPath(env, arguments[1], request->relative) || + (!request->relative.empty() && (request->relative.front() == '/' || request->relative.back() == '/')) || + napi_get_value_double(env, arguments[2], &request->dev) != napi_ok || + napi_get_value_double(env, arguments[3], &request->ino) != napi_ok || + !std::isfinite(request->dev) || !std::isfinite(request->ino) || + napi_get_value_bool(env, arguments[4], &request->directory) != napi_ok) { + delete request; + napi_throw_type_error(env, nullptr, "Expected a granted root, relative path, identity, and path kind."); + return nullptr; + } + napi_value promise; + if (napi_create_promise(env, &request->deferred, &promise) != napi_ok || + napi_create_async_work(env, nullptr, String(env, "OpenApprovedPath"), OpenApprovedPath, + CompleteOpen, request, &request->work) != napi_ok || + napi_queue_async_work(env, request->work) != napi_ok) { + if (request->work) napi_delete_async_work(env, request->work); + delete request; + napi_throw_error(env, nullptr, "Could not schedule the approved file open."); + return nullptr; + } + return promise; +} + +struct DescriptorClose { + napi_async_work work = nullptr; + napi_deferred deferred = nullptr; + int descriptor = -1; + bool failed = false; +}; + +static void CloseDescriptor(napi_env, void* data) { + auto* request = static_cast(data); + request->failed = close(request->descriptor) != 0; + request->descriptor = -1; +} + +static void CompleteClose(napi_env env, napi_status status, void* data) { + auto* request = static_cast(data); + if (request->descriptor >= 0) CloseDescriptor(env, request); + if (status != napi_ok || request->failed) { + napi_value error; + napi_create_error(env, nullptr, String(env, "Could not close the approved file."), &error); + napi_reject_deferred(env, request->deferred, error); + } else { + napi_value result; + napi_get_undefined(env, &result); + napi_resolve_deferred(env, request->deferred, result); + } + napi_delete_async_work(env, request->work); + delete request; +} + +/** Native opens retain native ownership through close, including inside Node workers. */ +static napi_value CloseFile(napi_env env, napi_callback_info info) { + size_t count = 1; + napi_value argument; + double descriptor = -1; + if (napi_get_cb_info(env, info, &count, &argument, nullptr, nullptr) != napi_ok || count != 1 || + napi_get_value_double(env, argument, &descriptor) != napi_ok || + !std::isfinite(descriptor) || descriptor < 0 || descriptor > INT_MAX || + descriptor != std::floor(descriptor)) { + napi_throw_type_error(env, nullptr, "Expected an approved file descriptor."); + return nullptr; + } + auto* request = new DescriptorClose(); + request->descriptor = static_cast(descriptor); + napi_value promise; + if (napi_create_promise(env, &request->deferred, &promise) != napi_ok || + napi_create_async_work(env, nullptr, String(env, "CloseApprovedFile"), CloseDescriptor, + CompleteClose, request, &request->work) != napi_ok || + napi_queue_async_work(env, request->work) != napi_ok) { + close(request->descriptor); + if (request->work) napi_delete_async_work(env, request->work); + delete request; + napi_throw_error(env, nullptr, "Could not schedule the approved file close."); + return nullptr; + } + return promise; +} + NAPI_MODULE_INIT() { - napi_property_descriptor property = { - "readDirectory", nullptr, ReadDirectory, nullptr, nullptr, nullptr, napi_default, nullptr + napi_property_descriptor properties[] = { + {"readDirectory", nullptr, ReadDirectory, nullptr, nullptr, nullptr, napi_default, nullptr}, + {"openApproved", nullptr, OpenApproved, nullptr, nullptr, nullptr, napi_default, nullptr}, + {"closeFile", nullptr, CloseFile, nullptr, nullptr, nullptr, napi_default, nullptr}, }; - napi_define_properties(env, exports, 1, &property); + napi_define_properties(env, exports, 3, properties); return exports; } diff --git a/apps/desktop/src/main/config.ts b/apps/desktop/src/main/config.ts index d4b5497bb5c..68e5a87cad5 100644 --- a/apps/desktop/src/main/config.ts +++ b/apps/desktop/src/main/config.ts @@ -104,6 +104,7 @@ export interface DesktopSettings { /** Whether omnibox typing may request live Google search completions. */ browserSearchSuggestionsEnabled?: boolean terminalEnabled?: boolean + fullFileAccess?: boolean /** Keep the machine awake while a chat is running desktop work in the background. */ preventSleepWhileRunning?: boolean /** Device-wide browser page appearance; `app` follows Sim. */ @@ -238,6 +239,7 @@ const DEFAULT_SETTINGS: DesktopSettings = { browserEnabled: true, browserSearchSuggestionsEnabled: true, terminalEnabled: true, + fullFileAccess: false, preventSleepWhileRunning: true, } diff --git a/apps/desktop/src/main/desktop-executor/runner.test.ts b/apps/desktop/src/main/desktop-executor/runner.test.ts index f10ac5a3748..cf45c449232 100644 --- a/apps/desktop/src/main/desktop-executor/runner.test.ts +++ b/apps/desktop/src/main/desktop-executor/runner.test.ts @@ -1,8 +1,10 @@ -import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join, relative } from 'node:path' +import { fileURLToPath } from 'node:url' import type { TerminalToolResponse } from '@sim/terminal-protocol' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { app } from 'electron' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { DeviceRequestError } from '@/main/desktop-executor/client' import type { ClaimedDesktopCall, @@ -10,6 +12,13 @@ import type { } from '@/main/desktop-executor/protocol' import { createDesktopToolRunner, type DesktopToolRunnerDeps } from '@/main/desktop-executor/runner' import { executeLocalFileRequest } from '@/main/local-files' +import { openNativeFile } from '@/main/native-directory' + +vi.mock('electron', () => import('@/test/electron-mock')) + +beforeEach(() => { + vi.mocked(app.getAppPath).mockReturnValue(fileURLToPath(new URL('../../..', import.meta.url))) +}) function terminalCall(toolCallId: string, operation: string): ClaimedDesktopCall { return { @@ -35,7 +44,18 @@ function runner(overrides: Partial = {}) { terminal: { executeTool: vi.fn(), cancelTool: vi.fn(async () => true) }, localFiles: { request: (call, request) => - executeLocalFileRequest(request, { toolName: call.toolName, args: call.args }), + executeLocalFileRequest( + request, + { toolName: call.toolName, args: call.args }, + { + path: String(call.args.path), + resolve: realpath, + open: async (path, directory = false) => { + const root = await realpath(dirname(String(call.args.path))) + return openNativeFile(root, relative(root, path), await stat(root), directory) + }, + } + ), }, imports: { importEntry: vi.fn() }, localFilesystem: { handle: vi.fn(), vfsRoot: () => 'user-local/x--1' }, diff --git a/apps/desktop/src/main/desktop-executor/runner.ts b/apps/desktop/src/main/desktop-executor/runner.ts index 1bb3663d765..8f1f789e365 100644 --- a/apps/desktop/src/main/desktop-executor/runner.ts +++ b/apps/desktop/src/main/desktop-executor/runner.ts @@ -106,7 +106,8 @@ export interface DesktopToolRunnerDeps { localFiles: { request( call: ClaimedDesktopCall, - request: DesktopLocalFileRequest + request: DesktopLocalFileRequest, + signal: AbortSignal ): Promise } imports: { @@ -289,7 +290,8 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo const folders: DesktopLocalFileImportResult['folders'] = [] const targetWorkspaceId = typeof call.args.targetWorkspaceId === 'string' ? call.args.targetWorkspaceId : '' - const read = (request: DesktopLocalFileRequest) => deps.localFiles.request(call, request) + const read = (request: DesktopLocalFileRequest) => + deps.localFiles.request(call, request, signal) try { const response = await read({ operation: 'manifest', toolCallId: call.toolCallId }) if (!response.ok) throw new Error(response.error) @@ -337,10 +339,14 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo if (!deps.accountDataAvailable()) return localAccessUnavailable() if (call.toolName === 'read_local_file') { return localFileReadCompletion( - await deps.localFiles.request(call, { - operation: 'read', - toolCallId: call.toolCallId, - }) + await deps.localFiles.request( + call, + { + operation: 'read', + toolCallId: call.toolCallId, + }, + signal + ) ) } if (call.toolName === 'import_local_files') return await runImport(call, signal) diff --git a/apps/desktop/src/main/desktop-settings.ts b/apps/desktop/src/main/desktop-settings.ts index 37c88b27056..af8119fd943 100644 --- a/apps/desktop/src/main/desktop-settings.ts +++ b/apps/desktop/src/main/desktop-settings.ts @@ -37,6 +37,7 @@ export interface DesktopSettingsService { getPreferences(): DesktopPreferences setPreference(key: DesktopPreferenceKey, value: boolean): DesktopPreferences setBrowserSearchSuggestionsEnabled(enabled: boolean): DesktopPreferences + setFullFileAccess(enabled: boolean): DesktopPreferences setPreventSleepWhileRunning(enabled: boolean): DesktopPreferences setAppearancePreference( key: DesktopAppearanceSettingKey, @@ -96,6 +97,7 @@ function readPreferences( browserEnabled: config.get('browserEnabled') ?? true, browserSearchSuggestionsEnabled: config.get('browserSearchSuggestionsEnabled') ?? true, terminalEnabled: config.get('terminalEnabled') ?? true, + fullFileAccess: config.get('fullFileAccess') === true, preventSleepWhileRunning: config.get('preventSleepWhileRunning') ?? true, browserTheme: isDesktopAppearanceTheme(browserTheme) ? browserTheme : 'app', browserDefaultZoom: isDesktopZoomPercent(browserDefaultZoom) ? browserDefaultZoom : 100, @@ -179,6 +181,14 @@ export function createDesktopSettingsService( deps.config.flush() return read() }, + setFullFileAccess(enabled) { + deps.config.set('fullFileAccess', enabled) + if (!deps.config.flush()) { + deps.config.set('fullFileAccess', false) + throw new Error('Could not save file access settings') + } + return read() + }, setPreventSleepWhileRunning(enabled) { deps.config.set('preventSleepWhileRunning', enabled) deps.config.flush() diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index ad203c7a54e..b61fda583ba 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -15,10 +15,12 @@ import { } from 'electron' import { beginAccountDataTeardown, + captureAccountDataGeneration, completeDeploymentScopedTeardown, getAccountDataTeardownKind, getAccountDataTeardownOrigin, initializeAccountDataRecovery, + isAccountDataGenerationCurrent, isAccountDataTeardownRequired, prepareAccountDataTeardownForQuit, retryAccountDataTeardown, @@ -74,6 +76,7 @@ import { } from '@/main/help-search' import { registerIpcHandlers } from '@/main/ipc' import { attachLoadHealth, type LoadHealthHandle } from '@/main/load-health' +import { LocalFilePermissions } from '@/main/local-file-permissions' import { executeLocalFileRequest } from '@/main/local-files' import { LocalFilesystemService, mountVfsRoot } from '@/main/local-filesystem' import { createEncryptedLocalFilesystemGrantStore } from '@/main/local-filesystem-grant-store' @@ -167,6 +170,15 @@ function main(): void { join(userDataPath, 'local-filesystem-grants.json') ), }) + const localFilePermissions = new LocalFilePermissions( + localFilesystem, + () => config.get('fullFileAccess') === true + ) + const clearLocalFileAccess = async () => { + config.set('fullFileAccess', false) + if (!config.flush()) throw new Error('Full file access could not be disabled') + await localFilesystem.forgetAll() + } const scopeEvents = new ScopedEventRouter() const terminal = new TerminalRegistry( { @@ -349,7 +361,7 @@ function main(): void { }, }, { label: 'task resource state', clear: clearDesktopChatSessions }, - { label: 'local filesystem grants', clear: () => localFilesystem.forgetAll() }, + { label: 'local filesystem grants', clear: clearLocalFileAccess }, ] const outcomes = await Promise.allSettled( stores.map(({ clear }) => Promise.resolve().then(clear)) @@ -627,8 +639,27 @@ function main(): void { }, terminal, localFiles: { - request: (call, request) => - executeLocalFileRequest(request, { toolName: call.toolName, args: call.args }), + request: async (call, request, signal) => { + const generation = captureAccountDataGeneration() + const origin = appOrigin() + const authorization = { toolName: call.toolName, args: call.args } + try { + const access = await localFilePermissions.authorize(authorization, { + parent: ensureMainWindow, + origin, + generation, + signal, + isCurrent: () => + isAccountDataGenerationCurrent(generation) && + accountDataAvailable() && + appOrigin() === origin, + revalidate: async () => !signal.aborted, + }) + return await executeLocalFileRequest(request, authorization, access) + } catch (error) { + return { ok: false, error: getErrorMessage(error) } + } + }, }, imports: { importEntry: (request, signal) => desktopExecutor.importEntry(request, signal), @@ -654,7 +685,7 @@ function main(): void { // that would have cleared fine still holding the outgoing deployment's // access. Each failure is named so the picker can say what survived. const stores = [ - { label: 'local file access', clear: () => localFilesystem.forgetAll() }, + { label: 'local file access', clear: clearLocalFileAccess }, { label: 'built-in browser sessions', clear: () => clearAgentBrowserProfile({ settingsPersistence: 'server-repair' }), @@ -772,7 +803,7 @@ function main(): void { } const stores = [ { label: 'built-in browser sessions', clear: () => clearAgentBrowserProfile() }, - { label: 'local filesystem grants', clear: () => localFilesystem.forgetAll() }, + { label: 'local filesystem grants', clear: clearLocalFileAccess }, { label: 'browser site history', clear: () => { @@ -891,6 +922,7 @@ function main(): void { if (win) loadHealthByWindow.get(win)?.retry() }, localFilesystem, + localFilePermissions, terminal, settings: desktopSettings, getWindowState: (sender) => ({ diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 14ea1e16483..d38cea1ec4d 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -134,6 +134,7 @@ import { import { getSearchSuggestions } from '@/main/browser-search/suggestions' import { trackInputActivity } from '@/main/input-activity' import { type IpcDeps, registerIpcHandlers } from '@/main/ipc' +import { LocalFilePermissions } from '@/main/local-file-permissions' import { LocalFilesystemService } from '@/main/local-filesystem' import { isLocalPageUrl } from '@/main/local-pages' import { TerminalRegistry } from '@/main/terminal/registry' @@ -287,6 +288,9 @@ describe('registerIpcHandlers', () => { mockCoordinator.showChooser.mockClear() mockCoordinator.listFillOptions.mockClear() mockCoordinator.fillCredential.mockClear() + const localFilesystem = new LocalFilesystemService({ + chooseDirectory: vi.fn(async () => null), + }) deps = { appOrigin: () => APP, getExecutorDevice: () => null, @@ -297,9 +301,8 @@ describe('registerIpcHandlers', () => { beginOAuthConnect: vi.fn(async () => true), prepareSourceConnect: vi.fn(() => 's'.repeat(32)), cancelSourceConnect: vi.fn(() => true), - localFilesystem: new LocalFilesystemService({ - chooseDirectory: vi.fn(async () => null), - }), + localFilesystem, + localFilePermissions: new LocalFilePermissions(localFilesystem), terminal: new TerminalRegistry(), scopeEvents: { activateBrowser: vi.fn(), @@ -311,6 +314,7 @@ describe('registerIpcHandlers', () => { getPreferences: vi.fn(() => DEFAULT_DESKTOP_PREFERENCES), setPreference: vi.fn(), setBrowserSearchSuggestionsEnabled: vi.fn(), + setFullFileAccess: vi.fn(), setPreventSleepWhileRunning: vi.fn(), setAppearancePreference: vi.fn(), setBrowserDefaultZoom: vi.fn(), diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 19c83aaa087..6a1068b2336 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -88,7 +88,7 @@ import { isSafeInternalPath } from '@/main/config' import type { DesktopSettingsService } from '@/main/desktop-settings' import { isDesktopPreferenceKey } from '@/main/desktop-settings' import { hasRecentDeliberateInput, hasRecentDiscreteInput } from '@/main/input-activity' -import { LocalFilePermissions } from '@/main/local-file-permissions' +import type { LocalFilePermissions } from '@/main/local-file-permissions' import { executeLocalFileRequest } from '@/main/local-files' import type { LocalFileAccess, LocalFilesystemService } from '@/main/local-filesystem' import { isAppOrigin, openExternalSafe } from '@/main/navigation' @@ -347,6 +347,7 @@ export interface IpcDeps { isLocalPageUrl: (url: string) => boolean retryLoad: (sender: WebContents) => void localFilesystem: LocalFilesystemService + localFilePermissions: LocalFilePermissions terminal: TerminalRegistry scopeEvents: Pick< ScopedEventRouter, @@ -613,7 +614,6 @@ async function authorizeLocalFilesystemTool( * unvalidated args they must parse themselves. */ export function registerIpcHandlers(deps: IpcDeps): void { - const localFilePermissions = new LocalFilePermissions(deps.localFilesystem) const activeLocalFiles = new Map>() let activeLocalFileCount = 0 const browserScopeBySender = new WeakMap() @@ -824,6 +824,17 @@ export function registerIpcHandlers(deps: IpcDeps): void { ? deps.settings.setBrowserSearchSuggestionsEnabled(enabled) : deps.settings.getPreferences(), }, + 'desktop:settings:set-full-file-access': { + kind: 'invoke', + gate: 'app-origin', + needsUserActivation: true, + requiresAccountData: true, + denied: null, + handler: (enabled) => + typeof enabled === 'boolean' + ? deps.settings.setFullFileAccess(enabled) + : deps.settings.getPreferences(), + }, 'desktop:settings:set-prevent-sleep': { kind: 'invoke', gate: 'app-origin', @@ -2197,8 +2208,8 @@ export function registerIpcHandlers(deps: IpcDeps): void { 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, + const access = await deps.localFilePermissions.authorize(authorization, { + parent: async () => parent, origin, generation, signal: controller.signal, diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts index 73a0600287f..dd72a290661 100644 --- a/apps/desktop/src/main/local-file-permissions.ts +++ b/apps/desktop/src/main/local-file-permissions.ts @@ -1,16 +1,17 @@ import { lstat, realpath, stat } from 'node:fs/promises' import { homedir } from 'node:os' -import { dirname, isAbsolute, join, resolve } from 'node:path' +import { dirname, isAbsolute, join, relative, resolve } from 'node:path' import { isDesktopScopeId } from '@sim/desktop-bridge' import type { BrowserWindow } from 'electron' import { showShellDialog } from '@/main/dialogs' import type { LocalFileAuthorization } from '@/main/local-files' import type { LocalFileAccess, LocalFilesystemService } from '@/main/local-filesystem' +import { openNativeFile } from '@/main/native-directory' const MAX_PENDING_REQUESTS = 32 interface LocalFilePermissionContext { - parent: BrowserWindow + parent: () => Promise origin: string generation: number signal: AbortSignal @@ -18,6 +19,12 @@ interface LocalFilePermissionContext { revalidate: () => Promise } +interface PendingFolderDecision { + contexts: Set + controller: AbortController + decision: Promise +} + function nativePath(value: unknown): string { if (typeof value !== 'string' || value.length > 4096 || value.includes('\0')) throw new Error('A native absolute path or ~/ path is required.') @@ -29,7 +36,7 @@ function nativePath(value: unknown): string { function assertCurrent(context: LocalFilePermissionContext): void { context.signal.throwIfAborted() - if (context.parent.isDestroyed() || !context.isCurrent()) + if (!context.isCurrent()) throw new Error('This local file request expired. Ask again in the current chat.') } @@ -43,9 +50,12 @@ async function revalidate(context: LocalFilePermissionContext): Promise { /** Shares each pending folder decision while remembered access remains concurrent. */ export class LocalFilePermissions { private queue: Promise = Promise.resolve() - private readonly pending = new Map>() + private readonly pending = new Map() - constructor(private readonly filesystem: LocalFilesystemService) {} + constructor( + private readonly filesystem: LocalFilesystemService, + private readonly fullFileAccess: () => boolean = () => false + ) {} async authorize( authorization: LocalFileAuthorization, @@ -60,6 +70,20 @@ export class LocalFilePermissions { ) throw new Error('A valid destination workspace and folder are required for imports.') const path = await realpath(nativePath(authorization.args.path)) + if (this.fullFileAccess()) { + const info = await stat(path) + const folder = info.isDirectory() ? path : dirname(path) + const identity = await stat(folder) + return this.authorizedAccess( + { + path, + resolve: realpath, + open: (requested, directory = false) => + openNativeFile(folder, relative(folder, requested), identity, directory), + }, + { ...context, isCurrent: () => context.isCurrent() && this.fullFileAccess() } + ) + } const existing = await this.filesystem.nativeAccess(path) if (existing) return this.authorizedAccess(existing, context) const info = await stat(path) @@ -68,25 +92,84 @@ export class LocalFilePermissions { const folder = info.isDirectory() ? path : dirname(path) const key = JSON.stringify([context.generation, context.origin, folder]) let pending = this.pending.get(key) + if (pending?.controller.signal.aborted) pending = undefined if (!pending) { if (this.pending.size >= MAX_PENDING_REQUESTS) throw new Error('Too many local file requests are waiting for permission. Try again later.') - const decision = this.queue.then(() => this.requestFolder(folder, context)) + const contexts = new Set() + const controller = new AbortController() + const decision = this.queue.then(() => + this.requestFolder(folder, contexts, controller.signal) + ) this.queue = decision.then( () => undefined, () => undefined ) - pending = decision.finally(() => this.pending.delete(key)) - this.pending.set(key, pending) + const request = { contexts, controller, decision } + pending = request + this.pending.set(key, request) + void this.queue.then(() => { + if (this.pending.get(key) === request) this.pending.delete(key) + }) } - await pending + pending.contexts.add(context) + await this.waitForDecision(pending, context) const access = await this.filesystem.nativeAccess(path) if (!access) throw new Error('The approved folder is no longer available.') return this.authorizedAccess(access, context) } - private async requestFolder(folder: string, context: LocalFilePermissionContext): Promise { - await revalidate(context) + private waitForDecision( + pending: PendingFolderDecision, + context: LocalFilePermissionContext + ): Promise { + return new Promise((resolve, reject) => { + const leave = () => { + context.signal.removeEventListener('abort', cancel) + pending.contexts.delete(context) + if (pending.contexts.size === 0) pending.controller.abort() + } + const cancel = () => { + leave() + reject(context.signal.reason) + } + context.signal.addEventListener('abort', cancel, { once: true }) + pending.decision.then( + () => { + leave() + resolve() + }, + (error) => { + leave() + reject(error) + } + ) + if (context.signal.aborted) cancel() + }) + } + + private async currentContext( + contexts: Set, + signal: AbortSignal + ): Promise { + signal.throwIfAborted() + for (const context of contexts) { + try { + await revalidate(context) + if (contexts.has(context)) return context + } catch { + signal.throwIfAborted() + } + } + throw new Error('The local file requests are no longer pending. Ask again in the current chat.') + } + + private async requestFolder( + folder: string, + contexts: Set, + signal: AbortSignal + ): Promise { + const context = await this.currentContext(contexts, signal) if (await this.filesystem.nativeAccess(folder)) return const root = await lstat(folder) if (!root.isDirectory()) throw new Error('The folder is no longer available.') @@ -94,9 +177,11 @@ export class LocalFilePermissions { /\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, + signal.throwIfAborted() + const parent = await context.parent() + await this.currentContext(contexts, signal) + const result = await showShellDialog(parent, { + 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.`, @@ -104,10 +189,10 @@ export class LocalFilePermissions { defaultId: 1, cancelId: 1, }) - assertCurrent(context) + signal.throwIfAborted() if (result.response !== 0) throw new Error('The user did not allow this local file access.') - await revalidate(context) - await this.filesystem.grantDirectory({ path: folder }, context.generation, root) + const current = await this.currentContext(contexts, signal) + await this.filesystem.grantDirectory({ path: folder }, current.generation, root) } private async authorizedAccess( @@ -122,6 +207,20 @@ export class LocalFilePermissions { return resolved } await resolveApproved(access.path) - return { path: access.path, resolve: resolveApproved } + return { + path: access.path, + resolve: resolveApproved, + open: async (path, directory) => { + assertCurrent(context) + const file = await access.open(path, directory) + try { + assertCurrent(context) + return file + } catch (error) { + await file.close() + throw error + } + }, + } } } diff --git a/apps/desktop/src/main/local-files.test.ts b/apps/desktop/src/main/local-files.test.ts index 4ef5f6fdb46..fb829c6720d 100644 --- a/apps/desktop/src/main/local-files.test.ts +++ b/apps/desktop/src/main/local-files.test.ts @@ -1,6 +1,6 @@ -import { mkdir, mkdtemp, realpath, rm, symlink, truncate, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, stat, symlink, truncate, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { join, relative } from 'node:path' import { fileURLToPath } from 'node:url' import { app } from 'electron' import { afterEach, beforeEach, expect, it, vi } from 'vitest' @@ -11,19 +11,22 @@ import { executeLocalFileRequest as executeApprovedLocalFileRequest, type LocalFileAuthorization, } from '@/main/local-files' +import { openNativeFile } from '@/main/native-directory' /** Parser and import invariants run with explicit fixture access; consent is covered through Electron. */ function executeLocalFileRequest(request: unknown, authorization: LocalFileAuthorization) { return executeApprovedLocalFileRequest(request, authorization, { path: String(authorization.args.path), resolve: realpath, + open: async (path, directory = false) => + openNativeFile(root, relative(root, path), await stat(root), directory), }) } let root: string beforeEach(async () => { vi.mocked(app.getAppPath).mockReturnValue(fileURLToPath(new URL('../..', import.meta.url))) - root = await mkdtemp(join(tmpdir(), 'sim-native-files-')) + root = await realpath(await mkdtemp(join(tmpdir(), 'sim-native-files-'))) }) afterEach(async () => { await rm(root, { recursive: true, force: true }) diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index 77f1146cfe7..840dd6c10f5 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -1,5 +1,4 @@ -import { constants } from 'node:fs' -import { lstat, open, stat } from 'node:fs/promises' +import { lstat, stat } from 'node:fs/promises' import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' import type { DesktopLocalFileEntry, @@ -41,37 +40,8 @@ function assertImportPath(root: string, candidate: string): void { throw new Error('The file is outside this import source.') } -async function openApprovedPath(path: string, access: LocalFileAccess, directory = false) { - const canonical = await access.resolve(path) - if (canonical !== path) - throw new Error('The local path changed while it was being opened. Try again.') - const file = await open( - canonical, - constants.O_RDONLY | - constants.O_NOFOLLOW | - constants.O_NONBLOCK | - (directory ? constants.O_DIRECTORY : 0) - ) - try { - const info = await file.stat() - const verified = await access.resolve(path) - const current = await lstat(verified) - if ( - (directory ? !info.isDirectory() : !info.isFile()) || - canonical !== verified || - info.dev !== current.dev || - info.ino !== current.ino - ) - throw new Error('The local file changed while it was being opened. Try again.') - return file - } catch (error) { - await file.close() - throw error - } -} - async function readApprovedDirectory(path: string, access: LocalFileAccess) { - const handle = await openApprovedPath(path, access, true) + const handle = await access.open(path, true) try { const listing = await readNativeDirectory(handle.fd, MAX_ENTRIES) const canonical = await access.resolve(path) @@ -103,7 +73,7 @@ async function inspect( } } if (!info.isFile()) throw new Error('The path is not a regular file or directory.') - const file = await openApprovedPath(path, access) + const file = await access.open(path) try { const info = await file.stat() const header = Buffer.alloc(16) @@ -280,7 +250,7 @@ export async function executeLocalFileRequest( const canonical = await access.resolve(child) assertImportPath(root, canonical) const offset = boundedInteger(request.offset, 0, Number.MAX_SAFE_INTEGER) - const file = await openApprovedPath(canonical, access) + const file = await access.open(canonical) try { const info = await file.stat() if (!info.isFile() || revision(info) !== request.revision) diff --git a/apps/desktop/src/main/local-filesystem.test.ts b/apps/desktop/src/main/local-filesystem.test.ts index d0418a67e05..81a1bbc2c6e 100644 --- a/apps/desktop/src/main/local-filesystem.test.ts +++ b/apps/desktop/src/main/local-filesystem.test.ts @@ -409,7 +409,7 @@ describe('LocalFilesystemService', () => { } expect( await service.handle({ operation: 'read', uri: `${granted.uri}README.md` }) - ).toMatchObject({ ok: false, code: 'ACCESS_DENIED' }) + ).toMatchObject({ ok: false, code: 'MOUNT_NOT_FOUND' }) } ) diff --git a/apps/desktop/src/main/local-filesystem.ts b/apps/desktop/src/main/local-filesystem.ts index 469482035a3..4863355992c 100644 --- a/apps/desktop/src/main/local-filesystem.ts +++ b/apps/desktop/src/main/local-filesystem.ts @@ -37,6 +37,7 @@ import type { LocalFilesystemGrantStore, PersistedLocalFilesystemGrant, } from '@/main/local-filesystem-grant-store' +import { openNativeFile } from '@/main/native-directory' const MAX_URI_LENGTH = 4096 const MAX_LIST_ENTRIES = 500 @@ -354,6 +355,7 @@ async function selectDirectoryEntries( export interface LocalFileAccess { path: string resolve: (path: string) => Promise + open: (path: string, directory?: boolean) => ReturnType } export class LocalFilesystemService { @@ -498,10 +500,7 @@ export class LocalFilesystemService { 'Local filesystem operation is not supported.' ) } - if (grant) { - if (this.mounts.get(grant.id)?.rootPath !== grant.rootPath) throw mountNotFound() - await this.assertMountCurrent(grant) - } + if (grant) await this.assertMountCurrent(grant) } finally { if (requestId) { this.activeRequests.delete(requestId) @@ -756,15 +755,38 @@ export class LocalFilesystemService { await this.assertMountCurrent(mount) return resolved } - return { path, resolve: resolveGranted } + return { + path, + resolve: resolveGranted, + open: async (requested, directory = false) => { + const canonical = await resolveGranted(requested) + if (canonical !== requested) throw new Error('The local path changed. Try again.') + const file = await openNativeFile( + mount.rootPath, + relative(mount.rootPath, canonical), + mount, + directory + ) + try { + await this.assertMountCurrent(mount) + return file + } catch (error) { + await file.close() + throw error + } + }, + } } return null } private async assertMountCurrent(mount: GrantedMount): Promise { const root = await lstat(mount.rootPath) + const current = this.mounts.get(mount.id) + if (!current || current.rootPath !== mount.rootPath) throw mountNotFound() if ( - this.mounts.get(mount.id) !== mount || + current.dev !== mount.dev || + current.ino !== mount.ino || !root.isDirectory() || root.dev !== mount.dev || root.ino !== mount.ino diff --git a/apps/desktop/src/main/native-directory.ts b/apps/desktop/src/main/native-directory.ts index 4babef7bde9..12362d30d3d 100644 --- a/apps/desktop/src/main/native-directory.ts +++ b/apps/desktop/src/main/native-directory.ts @@ -1,4 +1,6 @@ +import { fstat, read } from 'node:fs' import { join } from 'node:path' +import { promisify } from 'node:util' import type { DesktopLocalFileRead } from '@sim/desktop-bridge' import { app } from 'electron' @@ -8,18 +10,62 @@ interface NativeDirectoryListing { } interface NativeDirectoryBridge { + closeFile: (descriptor: number) => Promise readDirectory: (descriptor: number, limit: number) => Promise + openApproved: ( + root: string, + relativePath: string, + dev: number, + ino: number, + directory: boolean + ) => Promise } let bridge: NativeDirectoryBridge | undefined +const statDescriptor = promisify(fstat) +const readDescriptor = promisify(read) + +function nativeBridge(): NativeDirectoryBridge { + bridge ??= require( + join(app.getAppPath(), 'dist', 'native', 'directory.node') + ) as NativeDirectoryBridge + return bridge +} + +/** Owns a descriptor opened beneath the identity of a granted directory. */ +export async function openNativeFile( + root: string, + relativePath: string, + identity: { dev: number; ino: number }, + directory: boolean +) { + let descriptor = await nativeBridge().openApproved( + root, + relativePath, + identity.dev, + identity.ino, + directory + ) + return { + get fd() { + return descriptor + }, + stat: () => statDescriptor(descriptor), + read: (buffer: Buffer, offset: number, length: number, position: number) => + readDescriptor(descriptor, buffer, offset, length, position), + close: async () => { + if (descriptor < 0) return + const closing = descriptor + descriptor = -1 + await nativeBridge().closeFile(closing) + }, + } +} /** Enumerates the already-validated descriptor without resolving a pathname again. */ export function readNativeDirectory( descriptor: number, limit: number ): Promise { - bridge ??= require( - join(app.getAppPath(), 'dist', 'native', 'directory.node') - ) as NativeDirectoryBridge - return bridge.readDirectory(descriptor, limit) + return nativeBridge().readDirectory(descriptor, limit) } diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 6071925e7dd..3a181a33eee 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -179,6 +179,8 @@ const api: SimDesktopApi = { getPreferences: (): Promise => ipcRenderer.invoke('desktop:settings:get'), setPreference: (key: DesktopPreferenceKey, value: boolean): Promise => ipcRenderer.invoke('desktop:settings:set', key, value), + setFullFileAccess: (enabled: boolean): Promise => + ipcRenderer.invoke('desktop:settings:set-full-file-access', enabled), setPreventSleepWhileRunning: (enabled: boolean): Promise => ipcRenderer.invoke('desktop:settings:set-prevent-sleep', enabled), setBrowserSearchSuggestionsEnabled: (enabled: boolean): Promise => diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/desktop/desktop.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/desktop/desktop.tsx index b6a19bf86ca..9d9591eef17 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/desktop/desktop.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/desktop/desktop.tsx @@ -73,6 +73,13 @@ export function Desktop() { setPreferences ) + const { pending: fullFileAccessPending, mutate: setFullFileAccess } = + useDesktopPreferenceMutation( + async (bridge, enabled: boolean) => bridge.settings.setFullFileAccess?.(enabled), + 'Could not update file access', + setPreferences + ) + if (!preferences) { return null } @@ -106,20 +113,13 @@ export function Desktop() { onCheckedChange={(checked) => void updatePreference('launchAtLogin', checked)} /> {supportsPreventSleep && ( -
-
- -

- Closing the lid still puts your computer to sleep -

-
- void setPreventSleep(checked)} - /> -
+ void setPreventSleep(checked)} + /> )} + {getDesktopBridge()?.settings.setFullFileAccess && ( + + void setFullFileAccess(checked)} + /> + + )} +
+ /** Optional for installed shells that predate the explicit full-file-access setting. */ + setFullFileAccess?(enabled: boolean): Promise notify(payload: DesktopNotificationPayload): Promise /** Overrides the appearance requested by browser pages. */ setBrowserTheme(theme: DesktopAppearanceTheme): Promise From 52827358f457aec51971778867aa7f4dd04eca18 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 8 Oct 2026 17:43:58 -0700 Subject: [PATCH 8/8] fix(desktop): revalidate folder consent and preserve exact identities --- apps/desktop/e2e/background-executor.spec.ts | 131 +++++++++++++++++- apps/desktop/e2e/browser-focus.spec.ts | 27 ++++ apps/desktop/e2e/local-files.spec.ts | 76 +++++++--- apps/desktop/native/directory.cc | 17 ++- apps/desktop/src/main/browser-agent/panel.ts | 5 +- .../desktop/src/main/browser-agent/session.ts | 13 +- .../src/main/desktop-executor/runner.test.ts | 7 +- .../src/main/desktop-executor/service.ts | 9 ++ apps/desktop/src/main/index.ts | 7 +- .../src/main/local-file-permissions.ts | 11 +- apps/desktop/src/main/local-files.test.ts | 2 +- .../main/local-filesystem-grant-store.test.ts | 8 +- .../src/main/local-filesystem-grant-store.ts | 20 +-- apps/desktop/src/main/local-filesystem.ts | 21 +-- apps/desktop/src/main/native-directory.ts | 6 +- 15 files changed, 290 insertions(+), 70 deletions(-) diff --git a/apps/desktop/e2e/background-executor.spec.ts b/apps/desktop/e2e/background-executor.spec.ts index a9adadc7c1a..6b01639f9de 100644 --- a/apps/desktop/e2e/background-executor.spec.ts +++ b/apps/desktop/e2e/background-executor.spec.ts @@ -1,4 +1,12 @@ -import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { type ElectronApplication, expect, test } from '@playwright/test' @@ -203,6 +211,127 @@ test.describe('background executor', () => { }) }) + test('background consent cannot grant access when Sim cannot verify the call', async () => { + const userData = mkdtempSync(join(tmpdir(), 'sim-executor-offline-consent-')) + const launched = await launch(sim, userData) + app = launched.app + const deviceId = await registeredDevice(sim) + const readable = join(userData, 'private.txt') + writeFileSync(readable, 'offline consent fixture') + + await check('offline approval returns an error without remembering a grant', async () => { + const shown = launched.app.waitForEvent('window') + const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + const prompt = await shown + await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible() + sim.disconnect() + await prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + await expect + .poll(() => + launched.app.evaluate(({ safeStorage }, toolCallId) => { + const fs = process.getBuiltinModule('node:fs') + const path = `${process.env.SIM_DESKTOP_USER_DATA}/desktop-executor-journal.json` + const envelope = JSON.parse(fs.readFileSync(path, 'utf8')) + const journal = JSON.parse( + safeStorage.decryptString(Buffer.from(envelope.ciphertext, 'base64')) + ) as { entries: { toolCallId: string; state: string }[] } + return journal.entries.find((entry) => entry.toolCallId === toolCallId)?.state + }, call) + ) + .toBe('result') + sim.reconnect() + const completion = await settled(sim, call) + expect(completion.status).toBe('error') + expect(JSON.stringify(completion)).not.toContain('offline consent fixture') + const nextPrompt = launched.app.waitForEvent('window') + const next = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + await (await nextPrompt).getByRole('button', { name: "Don't allow", exact: true }).click() + expect((await settled(sim, next)).status).toBe('error') + }) + }) + + test('background consent does not reopen the main app after its windows are closed', async () => { + test.skip(process.platform !== 'darwin', 'The macOS app remains running without a window.') + const userData = mkdtempSync(join(tmpdir(), 'sim-executor-windowless-consent-')) + const launched = await launch(sim, userData) + app = launched.app + const deviceId = await registeredDevice(sim) + const readable = join(userData, 'private.txt') + writeFileSync(readable, 'windowless consent fixture') + await launched.window.close() + + await check('only the standalone consent window opens for a background read', async () => { + const shown = launched.app.waitForEvent('window') + const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + const prompt = await shown + await expect(prompt.getByRole('button', { name: 'Allow folder', exact: true })).toBeVisible() + expect( + await launched.app.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows().map((window) => window.getParentWindow() === null) + ) + ).toEqual([true]) + await prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + expect((await settled(sim, call)).status).toBe('error') + }) + }) + + test('sign-out clears folder grants even when desktop settings cannot be saved', async () => { + const userData = mkdtempSync(join(tmpdir(), 'sim-executor-grant-cleanup-')) + const launched = await launch(sim, userData) + app = launched.app + const deviceId = await registeredDevice(sim) + const readable = join(userData, 'private.txt') + writeFileSync(readable, 'cleanup consent fixture') + const shown = launched.app.waitForEvent('window') + const call = sim.issue(deviceId, CHAT_B, 'read_local_file', { path: readable }) + await (await shown).getByRole('button', { name: 'Allow folder', exact: true }).click() + expect((await settled(sim, call)).status).toBe('success') + const grants = join(userData, 'local-filesystem-grants.json') + expect(existsSync(grants)).toBe(true) + await launched.window.evaluate(() => { + const button = document.createElement('button') + button.textContent = 'Enable full file access' + button.onclick = async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + await api.settings.setFullFileAccess?.(true) + button.textContent = 'Full file access enabled' + } + document.body.append(button) + }) + await launched.window + .getByRole('button', { name: 'Enable full file access', exact: true }) + .click() + await expect( + launched.window.getByRole('button', { name: 'Full file access enabled', exact: true }) + ).toBeVisible() + const settings = join(userData, 'settings.json') + if (existsSync(settings)) renameSync(settings, `${settings}.backup`) + mkdirSync(settings) + + try { + await check( + 'failed settings persistence does not skip independent grant cleanup', + async () => { + const failedSignOut = launched.app.waitForEvent('window') + await launched.app.evaluate(({ Menu }) => { + const item = Menu.getApplicationMenu() + ?.items.flatMap((entry) => entry.submenu?.items ?? []) + .find((entry) => entry.label === 'Sign Out') + if (!item) throw new Error('Missing Sign Out menu item') + item.click() + }) + const failure = await failedSignOut + await failure.getByRole('button', { name: 'OK', exact: true }).click() + expect(existsSync(join(userData, 'account-data-teardown-required.json'))).toBe(true) + await expect.poll(() => existsSync(grants)).toBe(false) + } + ) + } finally { + rmSync(settings, { recursive: true, force: true }) + if (existsSync(`${settings}.backup`)) renameSync(`${settings}.backup`, settings) + } + }) + test('B: a result produced while the network is cut is delivered once after reconnecting', async () => { const userData = mkdtempSync(join(tmpdir(), 'sim-executor-b-')) app = (await launch(sim, userData)).app diff --git a/apps/desktop/e2e/browser-focus.spec.ts b/apps/desktop/e2e/browser-focus.spec.ts index 1f740a6b7fe..60bd25a345e 100644 --- a/apps/desktop/e2e/browser-focus.spec.ts +++ b/apps/desktop/e2e/browser-focus.spec.ts @@ -308,6 +308,33 @@ test('browser focus and shortcuts stay with the surface the user is using', asyn await clickMenu('New Tab') await expect.poll(tabCount).toBe(before + 1) }) + await check( + 'a revealed page resumes throttling in its own chat after switching chats', + async () => { + await panelAction({ action: 'switch-tab', tabId: '1' }) + await shell.evaluate(async (scope) => { + const api = (globalThis as Bridge).simDesktop.browserAgent + api.setPanelBounds( + { x: 0, y: 120, width: innerWidth, height: innerHeight - 120 }, + null, + scope + ) + await api.activateScope('browser-focus-other-chat') + }, SCOPE) + await expect + .poll(() => + shellApp.evaluate( + ({ webContents }, url) => + webContents + .getAllWebContents() + .find((contents) => contents.getURL() === url) + ?.getBackgroundThrottling(), + `${site}/five` + ) + ) + .toBe(true) + } + ) passed = true } finally { mkdirSync(dirname(reportPath), { recursive: true }) diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 10c19ebac31..bc55bbc2514 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -219,7 +219,9 @@ createRoot(document.getElementById('settings')).render( process.env.DESKTOP_LOCAL_FILES_REPORT_PATH ?? test.info().outputPath('local-file-consent.png'), }) - await denial.getByRole('button', { name: "Don't allow", exact: true }).click() + await denial + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) await expect.poll(() => deniedResults.length).toBe(2) expect(deniedResults).toEqual([ { ok: false, error: expect.any(String) }, @@ -232,10 +234,15 @@ createRoot(document.getElementById('settings')).render( const folderConsent = await folderPrompt const queuedRead = invoke({ operation: 'read', toolCallId: 'text' }) void queuedRead.catch(() => {}) + await expect( + folderConsent.getByRole('button', { name: 'Allow folder', exact: true }) + ).toBeVisible() expect( await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop) ).toBe('undefined') - await folderConsent.getByRole('button', { name: 'Allow folder', exact: true }).click() + await folderConsent + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) expect(await folderRead).toMatchObject({ ok: true, data: { representation: 'directory' } }) expect(await queuedRead).toMatchObject({ ok: true, data: { text: 'native file contents' } }) const canonicalRequest = { @@ -292,7 +299,9 @@ createRoot(document.getElementById('settings')).render( await invoke({ operation: 'cancel', toolCallId: 'sharedLeader' }) expect(await leader.result).toMatchObject({ ok: false }) await expect(leader.prompt.getByRole('dialog')).toBeVisible() - await leader.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + await leader.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) expect(await survivor).toMatchObject({ ok: true, data: { text: 'shared contents' } }) }) await test.step('Full file access is opt-in, survives restart, and stops granting access when disabled', async () => { @@ -375,7 +384,9 @@ createRoot(document.getElementById('settings')).render( window.getByRole('switch', { name: 'Full file access', exact: true }) ).not.toBeChecked() const permission = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' }) - await permission.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + await permission.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) expect(await permission.result).toMatchObject({ ok: false }) }) await test.step('a folder grant works in another chat but does not permit symlink escapes', async () => { @@ -400,7 +411,9 @@ createRoot(document.getElementById('settings')).render( await expect(escapedRead.prompt.getByRole('dialog')).toContainText( JSON.stringify(realpathSync(outside)) ) - await escapedRead.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + await escapedRead.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) expect(await escapedRead.result).toMatchObject({ ok: false }) rmSync(join(source, 'linked.txt')) }) @@ -410,7 +423,9 @@ createRoot(document.getElementById('settings')).render( const stale = await requestPermission({ operation: 'read', toolCallId: 'stale' }) if (changed) calls.stale.args.path = join(source, 'report.txt') else calls.stale = undefined - await stale.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + await stale.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) expect(await stale.result).toMatchObject({ ok: false }) } }) @@ -432,12 +447,12 @@ createRoot(document.getElementById('settings')).render( openApproved( root: string, relative: string, - dev: number, - ino: number, + dev: bigint, + ino: bigint, directory: boolean ): Promise } - const root = await stat(paths.source) + const root = await stat(paths.source, { bigint: true }) const denied = async (path: string, ino = root.ino) => { try { const fd = await native.openApproved(paths.source, path, root.dev, ino, false) @@ -460,12 +475,19 @@ createRoot(document.getElementById('settings')).render( text, ancestor: await denied('native-link/private.txt'), traversal: await denied('../Reports-other/private.txt'), - replaced: await denied('native-parent/inside.txt', root.ino + 1), + replaced: await denied('native-parent/inside.txt', root.ino + 1n), + overflow: await denied('native-parent/inside.txt', root.ino + (1n << 64n)), } }, { source: realpathSync(source) } ) - expect(result).toEqual({ text: 'inside', ancestor: true, traversal: true, replaced: true }) + expect(result).toEqual({ + text: 'inside', + ancestor: true, + traversal: true, + replaced: true, + overflow: true, + }) } finally { rmSync(linked) rmSync(parent, { recursive: true }) @@ -654,7 +676,9 @@ createRoot(document.getElementById('settings')).render( 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() + await again.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) expect(await again.result).toMatchObject({ ok: false }) }) await test.step('consent escapes direction controls in folder names', async () => { @@ -663,14 +687,18 @@ createRoot(document.getElementById('settings')).render( 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() + await bidi.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) 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' }) expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true }) - await blocker.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + await blocker.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) expect(await blocker.result).toMatchObject({ ok: false }) }) await test.step('cancelled calls cannot reuse an approved folder', async () => { @@ -688,7 +716,9 @@ createRoot(document.getElementById('settings')).render( renameSync(proposed, join(root, 'Original')) mkdirSync(proposed) writeFileSync(join(proposed, 'unapproved.txt'), 'replacement folder contents') - await retargeted.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + await retargeted.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) expect(await retargeted.result).toMatchObject({ ok: false }) }) const result = await invoke({ operation: 'manifest', toolCallId: 'import' }) @@ -763,7 +793,9 @@ createRoot(document.getElementById('settings')).render( 'Local files' ) const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) - await revoked.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + await revoked.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) expect(await revoked.result).toMatchObject({ ok: true }) }) @@ -778,12 +810,16 @@ createRoot(document.getElementById('settings')).render( 'Local files' ) const replaced = await requestPermission({ operation: 'read', toolCallId: 'text' }) - await replaced.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + await replaced.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) expect(await replaced.result).toMatchObject({ ok: false }) rmSync(source, { recursive: true }) renameSync(join(root, 'Original-reports'), source) const restored = await requestPermission({ operation: 'read', toolCallId: 'text' }) - await restored.prompt.getByRole('button', { name: 'Allow folder', exact: true }).click() + await restored.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) expect(await restored.result).toMatchObject({ ok: true }) }) @@ -820,7 +856,9 @@ createRoot(document.getElementById('settings')).render( ) .toMatchObject({ fullFileAccess: false }) const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) - await revoked.prompt.getByRole('button', { name: "Don't allow", exact: true }).click() + await revoked.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) expect(await revoked.result).toMatchObject({ ok: false }) }) await test.step('a failed settings write reports the error and leaves Full file access disabled', async () => { diff --git a/apps/desktop/native/directory.cc b/apps/desktop/native/directory.cc index 22ab5a113d1..3cf0d536ebb 100644 --- a/apps/desktop/native/directory.cc +++ b/apps/desktop/native/directory.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -150,8 +151,8 @@ struct ApprovedOpen { napi_deferred deferred = nullptr; std::string root; std::string relative; - double dev = 0; - double ino = 0; + uint64_t dev = 0; + uint64_t ino = 0; bool directory = false; int descriptor = -1; std::string error; @@ -162,8 +163,8 @@ static void OpenApprovedPath(napi_env, void* data) { int descriptor = open(request->root.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); struct stat metadata; if (descriptor < 0 || fstat(descriptor, &metadata) != 0 || - static_cast(metadata.st_dev) != request->dev || - static_cast(metadata.st_ino) != request->ino) { + static_cast(metadata.st_dev) != request->dev || + static_cast(metadata.st_ino) != request->ino) { if (descriptor >= 0) close(descriptor); request->error = "The approved folder changed. Request access again."; return; @@ -232,13 +233,15 @@ static napi_value OpenApproved(napi_env env, napi_callback_info info) { size_t count = 5; napi_value arguments[5]; auto* request = new ApprovedOpen(); + bool devLossless = false; + bool inoLossless = false; if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 5 || !ReadPath(env, arguments[0], request->root) || request->root.empty() || request->root[0] != '/' || !ReadPath(env, arguments[1], request->relative) || (!request->relative.empty() && (request->relative.front() == '/' || request->relative.back() == '/')) || - napi_get_value_double(env, arguments[2], &request->dev) != napi_ok || - napi_get_value_double(env, arguments[3], &request->ino) != napi_ok || - !std::isfinite(request->dev) || !std::isfinite(request->ino) || + napi_get_value_bigint_uint64(env, arguments[2], &request->dev, &devLossless) != napi_ok || + napi_get_value_bigint_uint64(env, arguments[3], &request->ino, &inoLossless) != napi_ok || + !devLossless || !inoLossless || napi_get_value_bool(env, arguments[4], &request->directory) != napi_ok) { delete request; napi_throw_type_error(env, nullptr, "Expected a granted root, relative path, identity, and path kind."); diff --git a/apps/desktop/src/main/browser-agent/panel.ts b/apps/desktop/src/main/browser-agent/panel.ts index 304e7e22ade..0a3d9a5174c 100644 --- a/apps/desktop/src/main/browser-agent/panel.ts +++ b/apps/desktop/src/main/browser-agent/panel.ts @@ -20,7 +20,6 @@ import { getErrorMessage } from '@sim/utils/errors' import type { BrowserWindow, WebContentsView } from 'electron' import { zoomPercentOf } from '@/main/browser-agent/context-menu' import type { AgentTab } from '@/main/browser-agent/session' -import { reassertTabThrottling } from '@/main/browser-agent/session' const logger = createLogger('BrowserAgentPanel') @@ -49,6 +48,8 @@ export interface PanelHost { onGeometryChanged?: () => void /** Runs after each layout that leaves the active view attached and visible. */ onViewShown?: (view: WebContentsView) => void + /** Restores a revealed view's own chat policy after its initial paint. */ + restoreTabThrottling?: (view: WebContentsView) => void } let host: PanelHost = { @@ -547,7 +548,7 @@ export function layout(): void { contents.setBackgroundThrottling(false) contents.invalidate() setTimeout(() => { - if (!contents.isDestroyed()) reassertTabThrottling() + if (!contents.isDestroyed()) host.restoreTabThrottling?.(active.view) }, 1_000) } } diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index 7b62998bfef..752fca5bec9 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -1083,6 +1083,10 @@ export function initSession( const scopeId = browserScopeIdForView(view) if (scopeId) withBrowserScope(scopeId, () => applyPendingUserFocus(view)) }, + restoreTabThrottling: (view) => { + const scopeId = browserScopeIdForView(view) + if (scopeId) withBrowserScope(scopeId, applyAutomationTabPolicy) + }, onViewDetached: (view) => { if (!view) return const scopeId = browserScopeIdForView(view) @@ -2808,15 +2812,6 @@ export function setAutomationNeedsAttention(needsAttention: boolean): void { events?.onTabsChanged() } -/** - * Re-applies the tab throttling policy after a caller temporarily suspended it - * (the panel's reveal pulse). Exempts the automation-active tab exactly as the - * internal policy does. - */ -export function reassertTabThrottling(): void { - applyAutomationTabPolicy() -} - /** * Unthrottles the automation tab while automation is active, throttles every other tab, and * keeps the automation tab composited while no panel shows it. Call after anything that changes diff --git a/apps/desktop/src/main/desktop-executor/runner.test.ts b/apps/desktop/src/main/desktop-executor/runner.test.ts index cf45c449232..e3df0c0bcf9 100644 --- a/apps/desktop/src/main/desktop-executor/runner.test.ts +++ b/apps/desktop/src/main/desktop-executor/runner.test.ts @@ -52,7 +52,12 @@ function runner(overrides: Partial = {}) { resolve: realpath, open: async (path, directory = false) => { const root = await realpath(dirname(String(call.args.path))) - return openNativeFile(root, relative(root, path), await stat(root), directory) + return openNativeFile( + root, + relative(root, path), + await stat(root, { bigint: true }), + directory + ) }, } ), diff --git a/apps/desktop/src/main/desktop-executor/service.ts b/apps/desktop/src/main/desktop-executor/service.ts index 95b856be5e3..b51d5afcde2 100644 --- a/apps/desktop/src/main/desktop-executor/service.ts +++ b/apps/desktop/src/main/desktop-executor/service.ts @@ -35,6 +35,7 @@ import { } from '@/main/desktop-executor/executor' import { createExecutorJournal } from '@/main/desktop-executor/journal' import { + type ClaimedDesktopCall, DESKTOP_EXECUTOR_PROTOCOL_VERSION, type DesktopExecutorTiming, type DesktopImportEntryRequest, @@ -79,6 +80,8 @@ export interface DesktopExecutorService { * journal, whenever it is asked; empty when the journal cannot be read. */ pendingResults(): Promise> + /** Confirms that Sim still authorizes this device to execute the claimed call. */ + revalidateCall(call: ClaimedDesktopCall): Promise /** Stores one entry of a claimed import, as this device's registered session. */ importEntry( request: DesktopImportEntryRequest, @@ -487,6 +490,12 @@ export function createDesktopExecutorService( pendingResults() { return pendingResultsSnapshot() }, + async revalidateCall(call) { + const current = client + if (!current || !deps.accountDataAvailable()) return false + await current.renewLease(call.toolCallId, call.executionToken) + return client === current && deps.accountDataAvailable() + }, importEntry(request, signal) { if (!client) throw new Error('The Sim desktop app is not signed in to Sim.') return client.importEntry(request, signal) diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index b61fda583ba..8bac14e70ee 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -176,8 +176,9 @@ function main(): void { ) const clearLocalFileAccess = async () => { config.set('fullFileAccess', false) - if (!config.flush()) throw new Error('Full file access could not be disabled') + const saved = config.flush() await localFilesystem.forgetAll() + if (!saved) throw new Error('Full file access could not be disabled') } const scopeEvents = new ScopedEventRouter() const terminal = new TerminalRegistry( @@ -645,7 +646,7 @@ function main(): void { const authorization = { toolName: call.toolName, args: call.args } try { const access = await localFilePermissions.authorize(authorization, { - parent: ensureMainWindow, + parent: async () => getMainWindow(), origin, generation, signal, @@ -653,7 +654,7 @@ function main(): void { isAccountDataGenerationCurrent(generation) && accountDataAvailable() && appOrigin() === origin, - revalidate: async () => !signal.aborted, + revalidate: () => desktopExecutor.revalidateCall(call), }) return await executeLocalFileRequest(request, authorization, access) } catch (error) { diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts index dd72a290661..fd9490cba4e 100644 --- a/apps/desktop/src/main/local-file-permissions.ts +++ b/apps/desktop/src/main/local-file-permissions.ts @@ -11,7 +11,7 @@ import { openNativeFile } from '@/main/native-directory' const MAX_PENDING_REQUESTS = 32 interface LocalFilePermissionContext { - parent: () => Promise + parent: () => Promise origin: string generation: number signal: AbortSignal @@ -73,7 +73,7 @@ export class LocalFilePermissions { if (this.fullFileAccess()) { const info = await stat(path) const folder = info.isDirectory() ? path : dirname(path) - const identity = await stat(folder) + const identity = await stat(folder, { bigint: true }) return this.authorizedAccess( { path, @@ -171,7 +171,7 @@ export class LocalFilePermissions { ): Promise { const context = await this.currentContext(contexts, signal) if (await this.filesystem.nativeAccess(folder)) return - const root = await lstat(folder) + const root = await lstat(folder, { bigint: true }) if (!root.isDirectory()) throw new Error('The folder is no longer available.') const displayedPath = JSON.stringify(folder).replace( /\p{Bidi_Control}/gu, @@ -180,7 +180,7 @@ export class LocalFilePermissions { signal.throwIfAborted() const parent = await context.parent() await this.currentContext(contexts, signal) - const result = await showShellDialog(parent, { + const options = { signal, title: 'Allow access to this folder?', message: displayedPath, @@ -188,7 +188,8 @@ export class LocalFilePermissions { buttons: ['Allow folder', "Don't allow"], defaultId: 1, cancelId: 1, - }) + } + const result = await (parent ? showShellDialog(parent, options) : showShellDialog(options)) signal.throwIfAborted() if (result.response !== 0) throw new Error('The user did not allow this local file access.') const current = await this.currentContext(contexts, signal) diff --git a/apps/desktop/src/main/local-files.test.ts b/apps/desktop/src/main/local-files.test.ts index fb829c6720d..fd7fe391d4d 100644 --- a/apps/desktop/src/main/local-files.test.ts +++ b/apps/desktop/src/main/local-files.test.ts @@ -19,7 +19,7 @@ function executeLocalFileRequest(request: unknown, authorization: LocalFileAutho path: String(authorization.args.path), resolve: realpath, open: async (path, directory = false) => - openNativeFile(root, relative(root, path), await stat(root), directory), + openNativeFile(root, relative(root, path), await stat(root, { bigint: true }), directory), }) } diff --git a/apps/desktop/src/main/local-filesystem-grant-store.test.ts b/apps/desktop/src/main/local-filesystem-grant-store.test.ts index 7fef1bd965d..8b9fc5e8540 100644 --- a/apps/desktop/src/main/local-filesystem-grant-store.test.ts +++ b/apps/desktop/src/main/local-filesystem-grant-store.test.ts @@ -16,7 +16,11 @@ function testEncryption(available = true) { } describe('createEncryptedLocalFilesystemGrantStore', () => { - it('encrypts grants at rest and restores them', async () => { + it.each([ + {}, + { dev: 1, ino: Number.MAX_SAFE_INTEGER }, + { dev: '1', ino: '18446744073709551615' }, + ])('encrypts grants at rest and restores their exact identity %j', async (identity) => { const directory = await mkdtemp(join(tmpdir(), 'sim-localfs-store-')) const filePath = join(directory, 'grants.json') const encryption = testEncryption() @@ -27,6 +31,7 @@ describe('createEncryptedLocalFilesystemGrantStore', () => { name: 'project', rootPath: '/Users/example/private-project', bookmark: 'security-scoped-bookmark', + ...identity, }, ] @@ -35,7 +40,6 @@ describe('createEncryptedLocalFilesystemGrantStore', () => { const raw = await readFile(filePath, 'utf8') expect(raw).not.toContain(grants[0].rootPath) expect(raw).not.toContain(grants[0].bookmark) - expect(encryption.encryptString).toHaveBeenCalledOnce() await expect(store.load()).resolves.toEqual(grants) await store.clear() diff --git a/apps/desktop/src/main/local-filesystem-grant-store.ts b/apps/desktop/src/main/local-filesystem-grant-store.ts index 188c2469b60..3b7dc887198 100644 --- a/apps/desktop/src/main/local-filesystem-grant-store.ts +++ b/apps/desktop/src/main/local-filesystem-grant-store.ts @@ -21,8 +21,8 @@ export interface PersistedLocalFilesystemGrant { id: string name: string rootPath: string - dev?: number - ino?: number + dev?: number | string + ino?: number | string bookmark?: string } @@ -43,6 +43,15 @@ interface EncryptedGrantEnvelope { ciphertext: string } +function isStoredIdentity(value: unknown): value is number | string { + if (typeof value === 'number') return Number.isSafeInteger(value) && value >= 0 + return ( + typeof value === 'string' && + /^(0|[1-9][0-9]{0,19})$/.test(value) && + BigInt(value) <= 18446744073709551615n + ) +} + function isPersistedGrant(value: unknown): value is PersistedLocalFilesystemGrant { if (!value || typeof value !== 'object' || Array.isArray(value)) return false const grant = value as Record @@ -58,12 +67,7 @@ function isPersistedGrant(value: unknown): value is PersistedLocalFilesystemGran grant.rootPath.length <= MAX_GRANT_PATH_LENGTH && !grant.rootPath.includes('\0') && ((grant.dev === undefined && grant.ino === undefined) || - (typeof grant.dev === 'number' && - Number.isSafeInteger(grant.dev) && - grant.dev >= 0 && - typeof grant.ino === 'number' && - Number.isSafeInteger(grant.ino) && - grant.ino >= 0)) && + (isStoredIdentity(grant.dev) && isStoredIdentity(grant.ino))) && (grant.bookmark === undefined || (typeof grant.bookmark === 'string' && grant.bookmark.length > 0 && diff --git a/apps/desktop/src/main/local-filesystem.ts b/apps/desktop/src/main/local-filesystem.ts index 4863355992c..f823c60c315 100644 --- a/apps/desktop/src/main/local-filesystem.ts +++ b/apps/desktop/src/main/local-filesystem.ts @@ -97,8 +97,8 @@ const GRANT_SCOPED_OPERATIONS: ReadonlySet = new Set([ interface GrantedMount extends LocalFilesystemMount { rootPath: string - dev: number - ino: number + dev: bigint + ino: bigint bookmark?: string stopAccessing?: () => void } @@ -651,7 +651,7 @@ export class LocalFilesystemService { async grantDirectory( selected: SelectedDirectory, generation: number, - expected?: { dev: number; ino: number } + expected?: { dev: bigint; ino: bigint } ): Promise { const stopAccessing = selected.bookmark ? this.startAccessingBookmark(selected.bookmark) @@ -667,7 +667,7 @@ export class LocalFilesystemService { try { const rootPath = await realpath(selected.path) - const rootStat = await stat(rootPath) + const rootStat = await stat(rootPath, { bigint: true }) if (!isAccountDataGenerationCurrent(generation)) { throw new LocalFilesystemError('CANCELLED', 'The folder request expired during sign-out.') } @@ -781,7 +781,7 @@ export class LocalFilesystemService { } private async assertMountCurrent(mount: GrantedMount): Promise { - const root = await lstat(mount.rootPath) + const root = await lstat(mount.rootPath, { bigint: true }) const current = this.mounts.get(mount.id) if (!current || current.rootPath !== mount.rootPath) throw mountNotFound() if ( @@ -853,7 +853,7 @@ export class LocalFilesystemService { const stopAccessing = grant.bookmark ? this.startAccessingBookmark(grant.bookmark) : undefined try { const rootPath = await realpath(grant.rootPath) - const rootStat = await stat(rootPath) + const rootStat = await stat(rootPath, { bigint: true }) if (!isAccountDataGenerationCurrent(generation)) { stopAccessing?.() return @@ -861,7 +861,10 @@ export class LocalFilesystemService { if ( !rootStat.isDirectory() || rootPath !== grant.rootPath || - (grant.dev !== undefined && (grant.dev !== rootStat.dev || grant.ino !== rootStat.ino)) + (grant.dev !== undefined && + (grant.ino === undefined || + BigInt(grant.dev) !== rootStat.dev || + BigInt(grant.ino) !== rootStat.ino)) ) { stopAccessing?.() needsPersist = true @@ -895,8 +898,8 @@ export class LocalFilesystemService { id: mount.id, name: mount.name, rootPath: mount.rootPath, - dev: mount.dev, - ino: mount.ino, + dev: mount.dev.toString(), + ino: mount.ino.toString(), ...(mount.bookmark ? { bookmark: mount.bookmark } : {}), })) } diff --git a/apps/desktop/src/main/native-directory.ts b/apps/desktop/src/main/native-directory.ts index 12362d30d3d..224d16b37dd 100644 --- a/apps/desktop/src/main/native-directory.ts +++ b/apps/desktop/src/main/native-directory.ts @@ -15,8 +15,8 @@ interface NativeDirectoryBridge { openApproved: ( root: string, relativePath: string, - dev: number, - ino: number, + dev: bigint, + ino: bigint, directory: boolean ) => Promise } @@ -36,7 +36,7 @@ function nativeBridge(): NativeDirectoryBridge { export async function openNativeFile( root: string, relativePath: string, - identity: { dev: number; ino: number }, + identity: { dev: bigint; ino: bigint }, directory: boolean ) { let descriptor = await nativeBridge().openApproved(