Skip to content

Commit 13461e3

Browse files
authored
improvement(desktop): make file consent clearer and easier to control (#8835)
* test(desktop): cover quiet navigation and full access consent * improvement(desktop): add explicit full file access from folder consent * fix(desktop): clarify consent actions and await native test readiness
1 parent 3c87849 commit 13461e3

11 files changed

Lines changed: 206 additions & 33 deletions

File tree

‎apps/desktop/e2e/browser-page-dialogs.spec.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,48 @@ createRoot(document.getElementById('root')).render(createElement(Fixture));`,
364364
await panelAction({ action: 'reload' })
365365
await expect.poll(() => pageTitle()).toBe('form')
366366

367+
await check('clean reloads and navigation never ask to discard changes', async () => {
368+
await inPage("document.documentElement.dataset.reloadProbe = 'before'")
369+
await panelAction({ action: 'reload' })
370+
await expect
371+
.poll(() => inPage<string | undefined>('document.documentElement.dataset.reloadProbe'))
372+
.toBeUndefined()
373+
await expect.poll(() => pageTitle()).toBe('form')
374+
expect(await pageDialog()).toBeNull()
375+
await panelAction({ action: 'navigate', url: `${site}/next` })
376+
await expect.poll(pageUrl).toBe(`${site}/next`)
377+
expect(await pageDialog()).toBeNull()
378+
await panelAction({ action: 'back' })
379+
await expect.poll(pageUrl).toBe(`${site}/form`)
380+
expect(await pageDialog()).toBeNull()
381+
})
382+
383+
await check('clearing a draft removes the leave warning', async () => {
384+
await userInput('#draft', 'temporary draft')
385+
await inPage("document.getElementById('draft').value = ''")
386+
await panelAction({ action: 'navigate', url: `${site}/next` })
387+
await expect.poll(pageUrl).toBe(`${site}/next`)
388+
expect(await pageDialog()).toBeNull()
389+
await panelAction({ action: 'navigate', url: `${site}/form` })
390+
await expect.poll(pageUrl).toBe(`${site}/form`)
391+
})
392+
393+
await check('same-page navigation preserves a draft without a leave warning', async () => {
394+
await userInput('#draft', 'same-page draft')
395+
await panelAction({ action: 'navigate', url: `${site}/form#section` })
396+
await expect.poll(pageUrl).toBe(`${site}/form#section`)
397+
expect(await pageDialog()).toBeNull()
398+
expect(await inPage<string>("document.getElementById('draft').value")).toBe('same-page draft')
399+
await panelAction({ action: 'back' })
400+
await expect.poll(pageUrl).toBe(`${site}/form`)
401+
expect(await pageDialog()).toBeNull()
402+
await inPage(`setTimeout(() => { location.href = ${JSON.stringify(`${site}/next`)} })`)
403+
await expect.poll(pageUrl).toBe(`${site}/next`)
404+
expect(await pageDialog()).toBeNull()
405+
await panelAction({ action: 'navigate', url: `${site}/form` })
406+
await expect.poll(pageUrl).toBe(`${site}/form`)
407+
})
408+
367409
await check('leaving a draft from the URL bar asks, and Stay keeps it', async () => {
368410
await userInput('#draft', 'draft')
369411
await expect
@@ -391,6 +433,12 @@ createRoot(document.getElementById('root')).render(createElement(Fixture));`,
391433
await userInput('#draft')
392434
await panelAction({ action: 'reload' })
393435
await expect(shell.getByRole('button', { name: 'Stay', exact: true })).toBeFocused()
436+
await shell.getByRole('dialog').screenshot({
437+
path: test.info().outputPath('leave-page-modal.png'),
438+
animations: 'allow',
439+
caret: 'initial',
440+
})
441+
await expect(shell.getByRole('button', { name: 'Stay', exact: true })).toBeFocused()
394442
await shell.keyboard.press('Escape')
395443
await expect.poll(pageDialog).toBeNull()
396444
await expect

‎apps/desktop/e2e/executor-sim.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ export async function launch(
462462
app.context().pages().forEach(leaveDialogsToDesktop)
463463
app.context().on('page', leaveDialogsToDesktop)
464464
const window = await app.firstWindow()
465+
await window.waitForURL((url) => url.origin === sim.origin, { waitUntil: 'load' })
465466
return { app, window }
466467
}
467468

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

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,36 @@ createRoot(document.getElementById('settings')).render(
304304
.click({ noWaitAfter: true })
305305
expect(await survivor).toMatchObject({ ok: true, data: { text: 'shared contents' } })
306306
})
307+
await test.step('expired requests cannot enable Full file access from a confirmation', async () => {
308+
if (!app) throw new Error('Desktop app is not running')
309+
calls.expiringFullAccess = {
310+
toolName: 'read_local_file',
311+
args: { path: join(outside, 'private.txt') },
312+
}
313+
const permission = await requestPermission({
314+
operation: 'read',
315+
toolCallId: 'expiringFullAccess',
316+
})
317+
const shown = app.waitForEvent('window')
318+
await permission.prompt
319+
.getByRole('button', { name: 'Full file access', exact: true })
320+
.click({ noWaitAfter: true })
321+
const confirmation = await shown
322+
calls.expiringFullAccess = undefined
323+
await confirmation
324+
.getByRole('button', { name: 'Enable', exact: true })
325+
.click({ noWaitAfter: true })
326+
expect(await permission.result).toMatchObject({ ok: false })
327+
expect(
328+
await window.evaluate(async () =>
329+
(
330+
globalThis as typeof globalThis & { simDesktop: SimDesktopApi }
331+
).simDesktop.settings.getPreferences()
332+
)
333+
).toMatchObject({ fullFileAccess: false })
334+
})
307335
await test.step('Full file access is opt-in, survives restart, and stops granting access when disabled', async () => {
336+
if (!app) throw new Error('Desktop app is not running')
308337
const fullFolder = join(root, 'Full access')
309338
mkdirSync(fullFolder)
310339
writeFileSync(join(fullFolder, 'file.txt'), 'full access contents')
@@ -315,7 +344,40 @@ createRoot(document.getElementById('settings')).render(
315344
await expect(
316345
window.getByRole('switch', { name: 'Full file access', exact: true })
317346
).not.toBeChecked()
318-
await window.getByRole('switch', { name: 'Full file access', exact: true }).click()
347+
const permission = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' })
348+
const confirmationShown = app.waitForEvent('window')
349+
await permission.prompt
350+
.getByRole('button', { name: 'Full file access', exact: true })
351+
.click({ noWaitAfter: true })
352+
const confirmation = await confirmationShown
353+
await expect(confirmation.getByRole('button', { name: 'Cancel', exact: true })).toBeFocused()
354+
const returnedPrompt = app.waitForEvent('window')
355+
await confirmation
356+
.getByRole('button', { name: 'Cancel', exact: true })
357+
.click({ noWaitAfter: true })
358+
const folderPrompt = await returnedPrompt
359+
expect(
360+
await window.evaluate(async () =>
361+
(
362+
globalThis as typeof globalThis & { simDesktop: SimDesktopApi }
363+
).simDesktop.settings.getPreferences()
364+
)
365+
).toMatchObject({ fullFileAccess: false })
366+
const acceptedConfirmation = app.waitForEvent('window')
367+
await folderPrompt
368+
.getByRole('button', { name: 'Full file access', exact: true })
369+
.click({ noWaitAfter: true })
370+
const allowAll = await acceptedConfirmation
371+
await allowAll.screenshot({
372+
path: test.info().outputPath('full-file-access-confirmation.png'),
373+
})
374+
await allowAll
375+
.getByRole('button', { name: 'Enable', exact: true })
376+
.click({ noWaitAfter: true })
377+
expect(await permission.result).toMatchObject({
378+
ok: true,
379+
data: { text: 'full access contents' },
380+
})
319381
await expect(
320382
window.getByRole('switch', { name: 'Full file access', exact: true })
321383
).toBeChecked()
@@ -383,11 +445,11 @@ createRoot(document.getElementById('settings')).render(
383445
await expect(
384446
window.getByRole('switch', { name: 'Full file access', exact: true })
385447
).not.toBeChecked()
386-
const permission = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' })
387-
await permission.prompt
448+
const revoked = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' })
449+
await revoked.prompt
388450
.getByRole('button', { name: "Don't allow", exact: true })
389451
.click({ noWaitAfter: true })
390-
expect(await permission.result).toMatchObject({ ok: false })
452+
expect(await revoked.result).toMatchObject({ ok: false })
391453
})
392454
await test.step('a folder grant works in another chat but does not permit symlink escapes', async () => {
393455
expect(await invoke({ operation: 'read', toolCallId: 'otherChat' })).toMatchObject({
@@ -877,8 +939,12 @@ createRoot(document.getElementById('settings')).render(
877939
try {
878940
await toggle.click()
879941
await expect(
880-
window.getByText('Could not update file access', { exact: true })
942+
window.getByText(
943+
'Could not save file access settings. Your previous setting may return after restarting Sim.',
944+
{ exact: true }
945+
)
881946
).toBeVisible()
947+
await expect(toggle).not.toBeChecked()
882948
expect(
883949
await window.evaluate(async () =>
884950
(

‎apps/desktop/e2e/smoke.spec.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ test.describe('desktop shell smoke', () => {
138138
test('OAuth popups share the session without inheriting the privileged preload', async () => {
139139
app = await launchApp(origin)
140140
const window = await app.firstWindow()
141+
await window.waitForURL(`${origin}/home`, { waitUntil: 'load' })
141142
await window.evaluate(() => {
142143
document.cookie = 'sim-e2e-session=shared; Path=/; SameSite=Lax'
143144
})
@@ -319,7 +320,7 @@ test.describe('desktop shell smoke', () => {
319320
await window.locator('#server').click()
320321
const picker = await pickerPromise
321322

322-
expect(picker.url()).toBe('sim-shell://pages/server.html')
323+
await expect(picker).toHaveURL('sim-shell://pages/server.html')
323324
await expect(picker.getByRole('dialog', { name: 'Sim server', exact: true })).toBeVisible()
324325
await expect(picker.getByLabel('Server URL')).toHaveValue('http://127.0.0.1:1')
325326
await expect(picker.getByLabel('Server URL')).toBeFocused()

‎apps/desktop/src/main/desktop-settings.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export interface DesktopSettingsService {
5353

5454
interface DesktopSettingsServiceDeps {
5555
config: ConfigStore
56+
onFullFileAccessChanged?: (preferences: DesktopPreferences) => void
5657
getMainWindow: () => BrowserWindow | null
5758
openMainWindowAt: (route?: string) => void
5859
setAutoDownloadUpdates: (enabled: boolean) => void
@@ -185,9 +186,14 @@ export function createDesktopSettingsService(
185186
deps.config.set('fullFileAccess', enabled)
186187
if (!deps.config.flush()) {
187188
deps.config.set('fullFileAccess', false)
188-
throw new Error('Could not save file access settings')
189+
deps.onFullFileAccessChanged?.(read())
190+
throw new Error(
191+
'Could not save file access settings. Your previous setting may return after restarting Sim.'
192+
)
189193
}
190-
return read()
194+
const preferences = read()
195+
deps.onFullFileAccessChanged?.(preferences)
196+
return preferences
191197
},
192198
setPreventSleepWhileRunning(enabled) {
193199
deps.config.set('preventSleepWhileRunning', enabled)

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,10 @@ function main(): void {
172172
})
173173
const localFilePermissions = new LocalFilePermissions(
174174
localFilesystem,
175-
() => config.get('fullFileAccess') === true
175+
() => config.get('fullFileAccess') === true,
176+
() => {
177+
desktopSettings.setFullFileAccess(true)
178+
}
176179
)
177180
const clearLocalFileAccess = async () => {
178181
config.set('fullFileAccess', false)
@@ -546,6 +549,9 @@ function main(): void {
546549

547550
const desktopSettings = createDesktopSettingsService({
548551
config,
552+
onFullFileAccessChanged: (preferences) => {
553+
broadcast('desktop:settings:full-file-access-changed', preferences)
554+
},
549555
getMainWindow,
550556
openMainWindowAt: (route) => void openMainWindowAt(route),
551557
setAutoDownloadUpdates: (enabled) => updater?.setAutoDownload(enabled),

‎apps/desktop/src/main/local-file-permissions.ts‎

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ export class LocalFilePermissions {
5454

5555
constructor(
5656
private readonly filesystem: LocalFilesystemService,
57-
private readonly fullFileAccess: () => boolean = () => false
57+
private readonly fullFileAccess: () => boolean = () => false,
58+
private readonly enableFullFileAccess?: () => void
5859
) {}
5960

6061
async authorize(
@@ -71,18 +72,7 @@ export class LocalFilePermissions {
7172
throw new Error('A valid destination workspace and folder are required for imports.')
7273
const path = await realpath(nativePath(authorization.args.path))
7374
if (this.fullFileAccess()) {
74-
const info = await stat(path)
75-
const folder = info.isDirectory() ? path : dirname(path)
76-
const identity = await stat(folder, { bigint: true })
77-
return this.authorizedAccess(
78-
{
79-
path,
80-
resolve: realpath,
81-
open: (requested, directory = false) =>
82-
openNativeFile(folder, relative(folder, requested), identity, directory),
83-
},
84-
{ ...context, isCurrent: () => context.isCurrent() && this.fullFileAccess() }
85-
)
75+
return this.unrestrictedAccess(path, context)
8676
}
8777
const existing = await this.filesystem.nativeAccess(path)
8878
if (existing) return this.authorizedAccess(existing, context)
@@ -114,11 +104,30 @@ export class LocalFilePermissions {
114104
}
115105
pending.contexts.add(context)
116106
await this.waitForDecision(pending, context)
107+
if (this.fullFileAccess()) return this.unrestrictedAccess(path, context)
117108
const access = await this.filesystem.nativeAccess(path)
118109
if (!access) throw new Error('The approved folder is no longer available.')
119110
return this.authorizedAccess(access, context)
120111
}
121112

113+
private async unrestrictedAccess(
114+
path: string,
115+
context: LocalFilePermissionContext
116+
): Promise<LocalFileAccess> {
117+
const info = await stat(path)
118+
const folder = info.isDirectory() ? path : dirname(path)
119+
const identity = await stat(folder, { bigint: true })
120+
return this.authorizedAccess(
121+
{
122+
path,
123+
resolve: realpath,
124+
open: (requested, directory = false) =>
125+
openNativeFile(folder, relative(folder, requested), identity, directory),
126+
},
127+
{ ...context, isCurrent: () => context.isCurrent() && this.fullFileAccess() }
128+
)
129+
}
130+
122131
private waitForDecision(
123132
pending: PendingFolderDecision,
124133
context: LocalFilePermissionContext
@@ -170,7 +179,7 @@ export class LocalFilePermissions {
170179
signal: AbortSignal
171180
): Promise<void> {
172181
const context = await this.currentContext(contexts, signal)
173-
if (await this.filesystem.nativeAccess(folder)) return
182+
if (this.fullFileAccess() || (await this.filesystem.nativeAccess(folder))) return
174183
const root = await lstat(folder, { bigint: true })
175184
if (!root.isDirectory()) throw new Error('The folder is no longer available.')
176185
const displayedPath = JSON.stringify(folder).replace(
@@ -184,16 +193,42 @@ export class LocalFilePermissions {
184193
signal,
185194
title: 'Allow access to this folder?',
186195
message: displayedPath,
187-
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.`,
188-
buttons: ['Allow folder', "Don't allow"],
196+
detail: `Sim can read and import files from this folder and its subfolders across chats on ${context.origin}.\n\nManage access in File → Folder Access.`,
197+
buttons: [
198+
'Allow folder',
199+
"Don't allow",
200+
...(this.enableFullFileAccess ? ['Full file access'] : []),
201+
],
189202
defaultId: 1,
190203
cancelId: 1,
191204
}
192-
const result = await (parent ? showShellDialog(parent, options) : showShellDialog(options))
193-
signal.throwIfAborted()
194-
if (result.response !== 0) throw new Error('The user did not allow this local file access.')
195-
const current = await this.currentContext(contexts, signal)
196-
await this.filesystem.grantDirectory({ path: folder }, current.generation, root)
205+
while (true) {
206+
await this.currentContext(contexts, signal)
207+
const result = await (parent ? showShellDialog(parent, options) : showShellDialog(options))
208+
signal.throwIfAborted()
209+
if (result.response === 2 && this.enableFullFileAccess) {
210+
const confirmation = {
211+
signal,
212+
title: 'Enable full file access?',
213+
message: `Sim can read and import files from any folder on this computer across chats on ${context.origin}.`,
214+
detail: 'Turn this off in Settings → Desktop → Full file access.',
215+
buttons: ['Enable', 'Cancel'],
216+
defaultId: 1,
217+
cancelId: 1,
218+
}
219+
const answer = await (parent
220+
? showShellDialog(parent, confirmation)
221+
: showShellDialog(confirmation))
222+
await this.currentContext(contexts, signal)
223+
if (answer.response !== 0) continue
224+
this.enableFullFileAccess()
225+
return
226+
}
227+
if (result.response !== 0) throw new Error('The user did not allow this local file access.')
228+
const current = await this.currentContext(contexts, signal)
229+
await this.filesystem.grantDirectory({ path: folder }, current.generation, root)
230+
return
231+
}
197232
}
198233

199234
private async authorizedAccess(

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,13 @@ const api: SimDesktopApi = {
181181
ipcRenderer.invoke('desktop:settings:set', key, value),
182182
setFullFileAccess: (enabled: boolean): Promise<DesktopPreferences> =>
183183
ipcRenderer.invoke('desktop:settings:set-full-file-access', enabled),
184+
onFullFileAccessChanged: (
185+
callback: (preferences: DesktopPreferences) => void
186+
): (() => void) => {
187+
const listener = (_event: unknown, preferences: DesktopPreferences) => callback(preferences)
188+
ipcRenderer.on('desktop:settings:full-file-access-changed', listener)
189+
return () => ipcRenderer.removeListener('desktop:settings:full-file-access-changed', listener)
190+
},
184191
setPreventSleepWhileRunning: (enabled: boolean): Promise<DesktopPreferences> =>
185192
ipcRenderer.invoke('desktop:settings:set-prevent-sleep', enabled),
186193
setBrowserSearchSuggestionsEnabled: (enabled: boolean): Promise<DesktopPreferences> =>

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-dialog.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ export function BrowserPageDialogModal({ dialog, open, onAnswer }: BrowserPageDi
1717
<ChipConfirmModal
1818
open={open}
1919
onOpenChange={(nextOpen) => !nextOpen && answer(false)}
20-
title='Leave site?'
21-
text='Changes you made may not be saved.'
20+
title='Leave page?'
21+
text='Changes on this page may not be saved.'
2222
defaultAction='dismiss'
2323
dismissLabel='Stay'
2424
confirm={{ label: 'Leave', onClick: () => answer(true) }}

0 commit comments

Comments
 (0)