Skip to content

Commit 04a253d

Browse files
committed
fix(desktop): validate consent rechecks and exact folder identities
1 parent 0ec6620 commit 04a253d

15 files changed

Lines changed: 141 additions & 65 deletions

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
mkdtempSync,
55
readFileSync,
66
renameSync,
7+
rmSync,
78
writeFileSync,
89
} from 'node:fs'
910
import { tmpdir } from 'node:os'
@@ -307,16 +308,28 @@ test.describe('background executor', () => {
307308
if (existsSync(settings)) renameSync(settings, `${settings}.backup`)
308309
mkdirSync(settings)
309310

310-
await check('failed settings persistence does not skip independent grant cleanup', async () => {
311-
await launched.app.evaluate(({ Menu }) => {
312-
const item = Menu.getApplicationMenu()
313-
?.items.flatMap((entry) => entry.submenu?.items ?? [])
314-
.find((entry) => entry.label === 'Sign Out')
315-
if (!item) throw new Error('Missing Sign Out menu item')
316-
item.click()
317-
})
318-
await expect.poll(() => existsSync(grants)).toBe(false)
319-
})
311+
try {
312+
await check(
313+
'failed settings persistence does not skip independent grant cleanup',
314+
async () => {
315+
const failedSignOut = launched.app.waitForEvent('window')
316+
await launched.app.evaluate(({ Menu }) => {
317+
const item = Menu.getApplicationMenu()
318+
?.items.flatMap((entry) => entry.submenu?.items ?? [])
319+
.find((entry) => entry.label === 'Sign Out')
320+
if (!item) throw new Error('Missing Sign Out menu item')
321+
item.click()
322+
})
323+
const failure = await failedSignOut
324+
await failure.getByRole('button', { name: 'OK', exact: true }).click()
325+
expect(existsSync(join(userData, 'account-data-teardown-required.json'))).toBe(true)
326+
await expect.poll(() => existsSync(grants)).toBe(false)
327+
}
328+
)
329+
} finally {
330+
rmSync(settings, { recursive: true, force: true })
331+
if (existsSync(`${settings}.backup`)) renameSync(`${settings}.backup`, settings)
332+
}
320333
})
321334

322335
test('B: a result produced while the network is cut is delivered once after reconnecting', async () => {

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

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

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@ createRoot(document.getElementById('settings')).render(
232232
const folderConsent = await folderPrompt
233233
const queuedRead = invoke({ operation: 'read', toolCallId: 'text' })
234234
void queuedRead.catch(() => {})
235+
await expect(
236+
folderConsent.getByRole('button', { name: 'Allow folder', exact: true })
237+
).toBeVisible()
235238
expect(
236239
await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop)
237240
).toBe('undefined')
@@ -432,12 +435,12 @@ createRoot(document.getElementById('settings')).render(
432435
openApproved(
433436
root: string,
434437
relative: string,
435-
dev: number,
436-
ino: number,
438+
dev: bigint,
439+
ino: bigint,
437440
directory: boolean
438441
): Promise<number>
439442
}
440-
const root = await stat(paths.source)
443+
const root = await stat(paths.source, { bigint: true })
441444
const denied = async (path: string, ino = root.ino) => {
442445
try {
443446
const fd = await native.openApproved(paths.source, path, root.dev, ino, false)
@@ -460,12 +463,19 @@ createRoot(document.getElementById('settings')).render(
460463
text,
461464
ancestor: await denied('native-link/private.txt'),
462465
traversal: await denied('../Reports-other/private.txt'),
463-
replaced: await denied('native-parent/inside.txt', root.ino + 1),
466+
replaced: await denied('native-parent/inside.txt', root.ino + 1n),
467+
overflow: await denied('native-parent/inside.txt', root.ino + (1n << 64n)),
464468
}
465469
},
466470
{ source: realpathSync(source) }
467471
)
468-
expect(result).toEqual({ text: 'inside', ancestor: true, traversal: true, replaced: true })
472+
expect(result).toEqual({
473+
text: 'inside',
474+
ancestor: true,
475+
traversal: true,
476+
replaced: true,
477+
overflow: true,
478+
})
469479
} finally {
470480
rmSync(linked)
471481
rmSync(parent, { recursive: true })

‎apps/desktop/native/directory.cc‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <cerrno>
99
#include <climits>
1010
#include <cmath>
11+
#include <cstdint>
1112
#include <cstring>
1213
#include <string>
1314
#include <vector>
@@ -150,8 +151,8 @@ struct ApprovedOpen {
150151
napi_deferred deferred = nullptr;
151152
std::string root;
152153
std::string relative;
153-
double dev = 0;
154-
double ino = 0;
154+
uint64_t dev = 0;
155+
uint64_t ino = 0;
155156
bool directory = false;
156157
int descriptor = -1;
157158
std::string error;
@@ -162,8 +163,8 @@ static void OpenApprovedPath(napi_env, void* data) {
162163
int descriptor = open(request->root.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
163164
struct stat metadata;
164165
if (descriptor < 0 || fstat(descriptor, &metadata) != 0 ||
165-
static_cast<double>(metadata.st_dev) != request->dev ||
166-
static_cast<double>(metadata.st_ino) != request->ino) {
166+
static_cast<uint64_t>(metadata.st_dev) != request->dev ||
167+
static_cast<uint64_t>(metadata.st_ino) != request->ino) {
167168
if (descriptor >= 0) close(descriptor);
168169
request->error = "The approved folder changed. Request access again.";
169170
return;
@@ -232,13 +233,15 @@ static napi_value OpenApproved(napi_env env, napi_callback_info info) {
232233
size_t count = 5;
233234
napi_value arguments[5];
234235
auto* request = new ApprovedOpen();
236+
bool devLossless = false;
237+
bool inoLossless = false;
235238
if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 5 ||
236239
!ReadPath(env, arguments[0], request->root) || request->root.empty() || request->root[0] != '/' ||
237240
!ReadPath(env, arguments[1], request->relative) ||
238241
(!request->relative.empty() && (request->relative.front() == '/' || request->relative.back() == '/')) ||
239-
napi_get_value_double(env, arguments[2], &request->dev) != napi_ok ||
240-
napi_get_value_double(env, arguments[3], &request->ino) != napi_ok ||
241-
!std::isfinite(request->dev) || !std::isfinite(request->ino) ||
242+
napi_get_value_bigint_uint64(env, arguments[2], &request->dev, &devLossless) != napi_ok ||
243+
napi_get_value_bigint_uint64(env, arguments[3], &request->ino, &inoLossless) != napi_ok ||
244+
!devLossless || !inoLossless ||
242245
napi_get_value_bool(env, arguments[4], &request->directory) != napi_ok) {
243246
delete request;
244247
napi_throw_type_error(env, nullptr, "Expected a granted root, relative path, identity, and path kind.");

‎apps/desktop/src/main/browser-agent/panel.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { getErrorMessage } from '@sim/utils/errors'
2020
import type { BrowserWindow, WebContentsView } from 'electron'
2121
import { zoomPercentOf } from '@/main/browser-agent/context-menu'
2222
import type { AgentTab } from '@/main/browser-agent/session'
23-
import { reassertTabThrottling } from '@/main/browser-agent/session'
2423

2524
const logger = createLogger('BrowserAgentPanel')
2625

@@ -49,6 +48,8 @@ export interface PanelHost {
4948
onGeometryChanged?: () => void
5049
/** Runs after each layout that leaves the active view attached and visible. */
5150
onViewShown?: (view: WebContentsView) => void
51+
/** Restores a revealed view's own chat policy after its initial paint. */
52+
restoreTabThrottling?: (view: WebContentsView) => void
5253
}
5354

5455
let host: PanelHost = {
@@ -547,7 +548,7 @@ export function layout(): void {
547548
contents.setBackgroundThrottling(false)
548549
contents.invalidate()
549550
setTimeout(() => {
550-
if (!contents.isDestroyed()) reassertTabThrottling()
551+
if (!contents.isDestroyed()) host.restoreTabThrottling?.(active.view)
551552
}, 1_000)
552553
}
553554
}

‎apps/desktop/src/main/browser-agent/session.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,10 @@ export function initSession(
10831083
const scopeId = browserScopeIdForView(view)
10841084
if (scopeId) withBrowserScope(scopeId, () => applyPendingUserFocus(view))
10851085
},
1086+
restoreTabThrottling: (view) => {
1087+
const scopeId = browserScopeIdForView(view)
1088+
if (scopeId) withBrowserScope(scopeId, applyAutomationTabPolicy)
1089+
},
10861090
onViewDetached: (view) => {
10871091
if (!view) return
10881092
const scopeId = browserScopeIdForView(view)
@@ -2808,15 +2812,6 @@ export function setAutomationNeedsAttention(needsAttention: boolean): void {
28082812
events?.onTabsChanged()
28092813
}
28102814

2811-
/**
2812-
* Re-applies the tab throttling policy after a caller temporarily suspended it
2813-
* (the panel's reveal pulse). Exempts the automation-active tab exactly as the
2814-
* internal policy does.
2815-
*/
2816-
export function reassertTabThrottling(): void {
2817-
applyAutomationTabPolicy()
2818-
}
2819-
28202815
/**
28212816
* Unthrottles the automation tab while automation is active, throttles every other tab, and
28222817
* keeps the automation tab composited while no panel shows it. Call after anything that changes

‎apps/desktop/src/main/desktop-executor/runner.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ function runner(overrides: Partial<DesktopToolRunnerDeps> = {}) {
5252
resolve: realpath,
5353
open: async (path, directory = false) => {
5454
const root = await realpath(dirname(String(call.args.path)))
55-
return openNativeFile(root, relative(root, path), await stat(root), directory)
55+
return openNativeFile(
56+
root,
57+
relative(root, path),
58+
await stat(root, { bigint: true }),
59+
directory
60+
)
5661
},
5762
}
5863
),

‎apps/desktop/src/main/desktop-executor/service.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
} from '@/main/desktop-executor/executor'
3636
import { createExecutorJournal } from '@/main/desktop-executor/journal'
3737
import {
38+
type ClaimedDesktopCall,
3839
DESKTOP_EXECUTOR_PROTOCOL_VERSION,
3940
type DesktopExecutorTiming,
4041
type DesktopImportEntryRequest,
@@ -79,6 +80,8 @@ export interface DesktopExecutorService {
7980
* journal, whenever it is asked; empty when the journal cannot be read.
8081
*/
8182
pendingResults(): Promise<Set<string>>
83+
/** Confirms that Sim still authorizes this device to execute the claimed call. */
84+
revalidateCall(call: ClaimedDesktopCall): Promise<boolean>
8285
/** Stores one entry of a claimed import, as this device's registered session. */
8386
importEntry(
8487
request: DesktopImportEntryRequest,
@@ -487,6 +490,12 @@ export function createDesktopExecutorService(
487490
pendingResults() {
488491
return pendingResultsSnapshot()
489492
},
493+
async revalidateCall(call) {
494+
const current = client
495+
if (!current || !deps.accountDataAvailable()) return false
496+
await current.renewLease(call.toolCallId, call.executionToken)
497+
return client === current && deps.accountDataAvailable()
498+
},
490499
importEntry(request, signal) {
491500
if (!client) throw new Error('The Sim desktop app is not signed in to Sim.')
492501
return client.importEntry(request, signal)

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,9 @@ function main(): void {
176176
)
177177
const clearLocalFileAccess = async () => {
178178
config.set('fullFileAccess', false)
179-
if (!config.flush()) throw new Error('Full file access could not be disabled')
179+
const saved = config.flush()
180180
await localFilesystem.forgetAll()
181+
if (!saved) throw new Error('Full file access could not be disabled')
181182
}
182183
const scopeEvents = new ScopedEventRouter()
183184
const terminal = new TerminalRegistry(
@@ -645,15 +646,15 @@ function main(): void {
645646
const authorization = { toolName: call.toolName, args: call.args }
646647
try {
647648
const access = await localFilePermissions.authorize(authorization, {
648-
parent: ensureMainWindow,
649+
parent: async () => getMainWindow(),
649650
origin,
650651
generation,
651652
signal,
652653
isCurrent: () =>
653654
isAccountDataGenerationCurrent(generation) &&
654655
accountDataAvailable() &&
655656
appOrigin() === origin,
656-
revalidate: async () => !signal.aborted,
657+
revalidate: () => desktopExecutor.revalidateCall(call),
657658
})
658659
return await executeLocalFileRequest(request, authorization, access)
659660
} catch (error) {

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { openNativeFile } from '@/main/native-directory'
1111
const MAX_PENDING_REQUESTS = 32
1212

1313
interface LocalFilePermissionContext {
14-
parent: () => Promise<BrowserWindow>
14+
parent: () => Promise<BrowserWindow | null>
1515
origin: string
1616
generation: number
1717
signal: AbortSignal
@@ -73,7 +73,7 @@ export class LocalFilePermissions {
7373
if (this.fullFileAccess()) {
7474
const info = await stat(path)
7575
const folder = info.isDirectory() ? path : dirname(path)
76-
const identity = await stat(folder)
76+
const identity = await stat(folder, { bigint: true })
7777
return this.authorizedAccess(
7878
{
7979
path,
@@ -171,7 +171,7 @@ export class LocalFilePermissions {
171171
): Promise<void> {
172172
const context = await this.currentContext(contexts, signal)
173173
if (await this.filesystem.nativeAccess(folder)) return
174-
const root = await lstat(folder)
174+
const root = await lstat(folder, { bigint: true })
175175
if (!root.isDirectory()) throw new Error('The folder is no longer available.')
176176
const displayedPath = JSON.stringify(folder).replace(
177177
/\p{Bidi_Control}/gu,
@@ -180,15 +180,16 @@ export class LocalFilePermissions {
180180
signal.throwIfAborted()
181181
const parent = await context.parent()
182182
await this.currentContext(contexts, signal)
183-
const result = await showShellDialog(parent, {
183+
const options = {
184184
signal,
185185
title: 'Allow access to this folder?',
186186
message: displayedPath,
187187
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.`,
188188
buttons: ['Allow folder', "Don't allow"],
189189
defaultId: 1,
190190
cancelId: 1,
191-
})
191+
}
192+
const result = await (parent ? showShellDialog(parent, options) : showShellDialog(options))
192193
signal.throwIfAborted()
193194
if (result.response !== 0) throw new Error('The user did not allow this local file access.')
194195
const current = await this.currentContext(contexts, signal)

0 commit comments

Comments
 (0)