Skip to content

Commit d8a6b6b

Browse files
authored
improvement(desktop): harden browser agent (#7311)
* improvement(desktop): harden browser agent * fix(desktop): close browser review gaps
1 parent b64a138 commit d8a6b6b

41 files changed

Lines changed: 7487 additions & 488 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/desktop-release.yml

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ jobs:
5555
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
5656
with:
5757
fetch-depth: 0
58+
persist-credentials: false
5859
ref: ${{ github.event_name == 'workflow_dispatch' && inputs.version || github.sha }}
5960

6061
# Prerelease versions carry their environment in the tag: -dev.N is a
@@ -340,10 +341,18 @@ jobs:
340341
exit 1
341342
fi
342343
export GH_TOKEN
343-
if ! gh release view "$VERSION" --repo "$RELEASE_REPOSITORY" >/dev/null; then
344+
if ! RELEASE_ID="$(
345+
gh release view "$VERSION" --repo "$RELEASE_REPOSITORY" \
346+
--json databaseId --jq '.databaseId'
347+
)"; then
344348
echo "::error::Release $VERSION does not exist in $RELEASE_REPOSITORY."
345349
exit 1
346350
fi
351+
if ! [[ "$RELEASE_ID" =~ ^[0-9]+$ ]]; then
352+
echo "::error::Release $VERSION returned an invalid database ID."
353+
exit 1
354+
fi
355+
RELEASE_JSON="$(gh api "repos/${RELEASE_REPOSITORY}/releases/${RELEASE_ID}")"
347356
SEMVER="${VERSION#v}"
348357
ARTIFACTS=(
349358
"apps/desktop/release/Sim-${SEMVER}-universal.dmg"
@@ -354,11 +363,10 @@ jobs:
354363
)
355364
upload_or_verify() {
356365
local ARTIFACT="$1"
357-
local NAME SIZE DIGEST RELEASE_JSON REMOTE REMOTE_SIZE REMOTE_DIGEST
366+
local NAME SIZE DIGEST REMOTE REMOTE_SIZE REMOTE_DIGEST
358367
NAME="$(basename "$ARTIFACT")"
359368
SIZE="$(stat -f%z "$ARTIFACT")"
360369
DIGEST="sha256:$(shasum -a 256 "$ARTIFACT" | awk '{print $1}')"
361-
RELEASE_JSON="$(gh api "repos/${RELEASE_REPOSITORY}/releases/tags/${VERSION}")"
362370
REMOTE="$(jq -c --arg name "$NAME" '.assets[] | select(.name == $name)' <<< "$RELEASE_JSON")"
363371
if [ -n "$REMOTE" ]; then
364372
REMOTE_SIZE="$(jq -r '.size' <<< "$REMOTE")"

apps/desktop/README.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ src/main/ # main process (bundled to dist/main.cjs)
3434
browser-credentials/ # saved passwords, OS-auth gated, safeStorage at rest
3535
browser-sites/ # imported site directory, safeStorage at rest
3636
browser-import/ # one-shot import of profiles, cookies and passwords
37-
src/preload/ # contextBridge IPC bridge (bundled to dist/preload.cjs)
37+
src/preload/ # isolated renderer bridges
38+
index.ts # hosted-app contextBridge IPC bridge (dist/preload.cjs)
39+
browser/ # minimal agent-browser credential helper (dist/browser-preload.cjs)
3840
native/ # Node-API/AppKit bridge for native macOS Help docs search
3941
static/ # bundled local pages (offline.html)
4042
e2e/ # Playwright _electron smoke suite
@@ -54,7 +56,7 @@ SIM_DESKTOP_ORIGIN=http://localhost:3000 bun run dev # against local sim
5456
- `bun run type-check` / `lint:check` — standard workspace checks; CI picks these up automatically via `turbo run`.
5557
- `SIM_DESKTOP_USER_DATA=<dir>` isolates settings/partition state (used by e2e).
5658

57-
Everything is bundled by esbuild into `dist/main.cjs` + `dist/preload.cjs`including `electron-updater` and the `@sim/*` packages — so the packaged app has **no runtime node_modules** and `electron-builder` needs no lockfile/npmRebuild step (this is the deliberate workaround for Bun ↔ electron-builder friction; there is no `package-lock.json`).
59+
The main process and two preloads are bundled by esbuild into `dist/main.cjs`, `dist/preload.cjs`, and `dist/browser-preload.cjs`, including `electron-updater` and the `@sim/*` packages. The native `@lydell/node-pty` packages stay external so Electron can load their architecture-specific prebuilds from the packaged runtime `node_modules`; `npmRebuild` remains disabled because those Node-API prebuilds are already ABI-stable. There is no `package-lock.json`.
5860

5961
## Auth model (read before touching auth)
6062

@@ -99,7 +101,7 @@ Overall this is **within normal thin-wrapper coupling** — every item is either
99101

100102
Local unsigned build: `bun run package:dir` (app in `release/mac-universal/`). Signed: `bun run package:mac` with `CSC_LINK`/`CSC_KEY_PASSWORD` exported.
101103

102-
Pre-release share (no Developer ID yet): `SIM_DESKTOP_DEFAULT_ORIGIN=https://www.dev.sim.ai bun run package:share` builds a DMG whose fresh installs default to that origin (baked at build time; official builds leave it unset → prod) and skips per-file signature timestamps. Recipients must clear quarantine once: `xattr -cr /Applications/Sim.app`.
104+
Local unsigned pre-release share: `SIM_DESKTOP_DEFAULT_ORIGIN=https://www.dev.sim.ai bun run package:share` builds a DMG whose fresh installs default to that origin (baked at build time; official builds leave it unset → prod) and skips per-file signature timestamps. Recipients must clear quarantine once: `xattr -cr /Applications/Sim.app`.
103105

104106
The build also derives the app icon from `SIM_DESKTOP_DEFAULT_ORIGIN`. Every channel uses the exact production icon with its white background and black `sim` mark. Non-production channels add a thin outline using existing platform colors: dev uses orange, staging uses Loop blue, and localhost uses Workflow violet. The macOS menu-bar icon also carries a compact `D`, `S`, or `L` subscript for those environments; production remains unmarked. Native Icon Composer assets live in `build/`; `scripts/build.ts` copies the selected variant to the ignored `build/generated-icon.icon` path consumed by electron-builder. Electron-builder compiles it to `Assets.car` and derives the legacy `.icns` fallback from the same source. Matching 512px PNGs in `static/` provide the Dock icon for unpackaged runs.
105107

@@ -185,10 +187,11 @@ Raw local file bytes are never exposed through the preload bridge and cannot be
185187

186188
## Known caveats
187189

188-
- Microphone and camera are denied by design (the permission matrix grants only sanitized clipboard writes to the app origin).
190+
- The hosted Sim renderer may request microphone access for voice input from the configured app origin; camera access remains denied. On macOS the shell also requires the operating-system microphone grant. Separately, a page in the isolated agent browser may request microphone or camera only from its main frame after a recent native user gesture; Sim then requires an explicit document-scoped prompt and the operating-system grant where applicable.
191+
- The built-in agent browser is not a general-purpose download manager. Its dedicated partition applies the same bounded policy to every download, including one started by a direct user click: at most 2 GiB per file, two active downloads per task, six app-wide, and a 1 GiB free-disk reserve. A rejected download appears in the browser's downloads menu; use a normal browser for an intentionally larger transfer.
189192
- Default Electron ships H.264/AAC/MP3 — do not swap in the codec-free ffmpeg build.
190193
- Third-party web analytics (GTM/GA) are blocked at the network layer by default (`blockThirdPartyAnalytics`); first-party PostHog `/ingest` is untouched.
191-
- `Cmd+F` find-in-page overlay is not implemented (Monaco and tables ship their own finds); revisit if users ask.
194+
- `Cmd+F` opens the native find overlay in built-in browser tabs. The hosted Sim workspace continues to use Monaco- and table-specific find surfaces.
192195
- Sign-in uses only the `127.0.0.1` loopback callback, which needs no OS registration — so it completes identically under `bun run dev` (unpackaged) and in a packaged build. There is no custom URL scheme.
193196

194197
## Electron upgrades

apps/desktop/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"@sim/tsconfig": "workspace:*",
5151
"@types/micromatch": "4.0.10",
5252
"@types/node": "24.2.1",
53-
"electron": "43.1.1",
53+
"electron": "43.5.0",
5454
"electron-builder": "26.15.3",
5555
"esbuild": "0.28.1",
5656
"typescript": "^7.0.2",

apps/desktop/src/main/browser-agent/driver-profile.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ const mocks = vi.hoisted(() => ({
1010
vi.mock('@/main/browser-agent/session', () => ({
1111
clearProfileStorage: mocks.clearProfileStorage,
1212
initSession: vi.fn(),
13+
isBrowserScopeSuspended: vi.fn(() => false),
14+
resolveBrowserScopeId: vi.fn((scopeId: string) => scopeId),
1315
}))
1416

1517
vi.mock('@/main/browser-credentials', () => ({
@@ -18,7 +20,12 @@ vi.mock('@/main/browser-credentials', () => ({
1820
initFillCoordinator: vi.fn(),
1921
}))
2022

21-
import { clearBrowserProfile, initDriver } from '@/main/browser-agent/driver'
23+
import {
24+
captureBrowserToolQueueBoundary,
25+
clearBrowserProfile,
26+
executeTool,
27+
initDriver,
28+
} from '@/main/browser-agent/driver'
2229
import type { ConfigStore } from '@/main/config'
2330

2431
describe('clearBrowserProfile', () => {
@@ -52,4 +59,35 @@ describe('clearBrowserProfile', () => {
5259
expect(mocks.clearCredentials).toHaveBeenCalledTimes(2)
5360
expect(config.flush).toHaveBeenCalledTimes(2)
5461
})
62+
63+
it('invalidates pre-wipe authorization and retires live work before profile teardown', async () => {
64+
initDriver(
65+
{
66+
onPageState: vi.fn(),
67+
onTabsState: vi.fn(),
68+
onSessionStatus: vi.fn(),
69+
onFillAvailability: vi.fn(),
70+
},
71+
() => null
72+
)
73+
const boundary = captureBrowserToolQueueBoundary('chat-before-wipe')
74+
expect(boundary).not.toBeNull()
75+
if (!boundary) throw new Error('Expected browser tool authorization admission')
76+
77+
await clearBrowserProfile()
78+
const staleExecution = await executeTool(
79+
'chat-before-wipe',
80+
'browser_list_sessions',
81+
{},
82+
'tool-authorized-before-wipe',
83+
boundary
84+
)
85+
86+
expect(mocks.clearProfileStorage).toHaveBeenCalledOnce()
87+
expect(mocks.clearCredentials).toHaveBeenCalledOnce()
88+
expect(staleExecution).toMatchObject({
89+
ok: false,
90+
error: expect.stringContaining('cancelled before it started'),
91+
})
92+
})
5593
})

0 commit comments

Comments
 (0)