From e422f623bcea819204d3df856da17b097e4e62a0 Mon Sep 17 00:00:00 2001 From: Mr P-Tech Date: Tue, 11 Aug 2026 11:53:40 +0100 Subject: [PATCH 1/2] fix(control): restore virtual cursor safety --- apps/desktop/src/main/input/injector.test.ts | 18 +++ apps/desktop/src/main/input/injector.ts | 12 +- apps/desktop/src/main/window.test.ts | 18 +++ apps/desktop/src/main/window.ts | 36 +++-- packages/remote-input/README.md | 16 +-- packages/remote-input/src/injector.test.ts | 35 +++-- packages/remote-input/src/injector.ts | 131 ++++++++----------- 7 files changed, 147 insertions(+), 119 deletions(-) diff --git a/apps/desktop/src/main/input/injector.test.ts b/apps/desktop/src/main/input/injector.test.ts index b9c8b09..70106b1 100644 --- a/apps/desktop/src/main/input/injector.test.ts +++ b/apps/desktop/src/main/input/injector.test.ts @@ -10,6 +10,10 @@ vi.mock('../platform', () => ({ detectDisplayServer: vi.fn().mockReturnValue('x11'), })); +vi.mock('../capture/captureDisplay', () => ({ + resolveCaptureBoundsForSource: vi.fn().mockResolvedValue(null), +})); + // Mock @nut-tree-fork/nut-js before imports // Note: All values must be defined inside the factory since vi.mock is hoisted vi.mock('@nut-tree-fork/nut-js', () => { @@ -93,6 +97,7 @@ vi.mock('@nut-tree-fork/nut-js', () => { }); import { mouse, keyboard, Button, Key, screen } from '@nut-tree-fork/nut-js'; +import { resolveCaptureBoundsForSource } from '../capture/captureDisplay'; import { initInputInjector, enableInjection, @@ -101,11 +106,14 @@ import { updateScreenSize, injectInput, emergencyStop, + resetInputInjector, + setCaptureSource, } from './injector'; describe('Input Injector', () => { beforeEach(async () => { vi.clearAllMocks(); + resetInputInjector(); // Reset injection state and screen size disableInjection(); // Reset to default screen size for consistent tests @@ -130,6 +138,16 @@ describe('Input Injector', () => { // Should not throw await expect(initInputInjector()).resolves.not.toThrow(); }); + + it('re-resolves a source selected before backend geometry was initialized', async () => { + await setCaptureSource('screen:second'); + await initInputInjector(); + + expect(resolveCaptureBoundsForSource).toHaveBeenLastCalledWith('screen:second', { + width: 1920, + height: 1080, + }); + }); }); describe('enableInjection / disableInjection', () => { diff --git a/apps/desktop/src/main/input/injector.ts b/apps/desktop/src/main/input/injector.ts index a1ab794..37869f5 100644 --- a/apps/desktop/src/main/input/injector.ts +++ b/apps/desktop/src/main/input/injector.ts @@ -43,6 +43,7 @@ function getInjector(): RemoteInputInjector { export function resetInputInjector(): void { injector = null; backendPrimary = null; + captureSourceId = null; } /** @@ -54,11 +55,14 @@ export function resetInputInjector(): void { * and scale every subsequent resolution against the wrong reference. */ let backendPrimary: { width: number; height: number } | null = null; +/** The latest source requested, retained until backend dimensions are known. */ +let captureSourceId: string | null = null; export async function initInputInjector(): Promise { const injector = getInjector(); await injector.init(); backendPrimary = injector.getScreenSize(); + await updateCaptureBounds(); } /** @@ -69,7 +73,13 @@ export async function initInputInjector(): Promise { * single-monitor host has always had. */ export async function setCaptureSource(sourceId: string | null): Promise { - const bounds = await resolveCaptureBoundsForSource(sourceId, backendPrimary); + captureSourceId = sourceId; + await updateCaptureBounds(); +} + +/** Re-resolve once both the selected source and backend geometry are available. */ +async function updateCaptureBounds(): Promise { + const bounds = await resolveCaptureBoundsForSource(captureSourceId, backendPrimary); getInjector().updateCaptureBounds(bounds); } diff --git a/apps/desktop/src/main/window.test.ts b/apps/desktop/src/main/window.test.ts index 30f36c7..a37c016 100644 --- a/apps/desktop/src/main/window.test.ts +++ b/apps/desktop/src/main/window.test.ts @@ -60,6 +60,24 @@ beforeEach(() => { }); describe('createMainWindow', () => { + it('allows pointer lock through both Electron permission paths', async () => { + const { createMainWindow } = await import('./window'); + await createMainWindow(false); + + const requestHandler = vi + .mocked(session.defaultSession.setPermissionRequestHandler) + .mock.calls.at(-1)?.[0]; + const checkHandler = vi + .mocked(session.defaultSession.setPermissionCheckHandler) + .mock.calls.at(-1)?.[0]; + const callback = vi.fn(); + + requestHandler?.({} as Electron.WebContents, 'pointerLock', callback, {}); + + expect(callback).toHaveBeenCalledWith(true); + expect(checkHandler?.({} as Electron.WebContents, 'pointerLock', '', {})).toBe(true); + }); + it('keeps the renderer unthrottled in the background', async () => { const { createMainWindow } = await import('./window'); await createMainWindow(false); diff --git a/apps/desktop/src/main/window.ts b/apps/desktop/src/main/window.ts index 95d09e0..171f128 100644 --- a/apps/desktop/src/main/window.ts +++ b/apps/desktop/src/main/window.ts @@ -119,20 +119,24 @@ export async function createMainWindow(isWayland: boolean): Promise { - const allowedPermissions = [ - 'media', - 'display-capture', - 'mediaKeySystem', - 'geolocation', - 'notifications', - 'fullscreen', - 'clipboard-sanitized-write', - 'clipboard-read', - ]; - - if (allowedPermissions.includes(permission)) { + if (allowedPermissions.has(permission)) { console.log('[Main] Permission granted:', permission); callback(true); } else { @@ -141,6 +145,12 @@ export async function createMainWindow(isWayland: boolean): Promise { + const allowed = allowedPermissions.has(permission); + console.log(`[Main] Permission ${allowed ? 'granted' : 'denied'}:`, permission); + return allowed; + }); + // Mirror renderer console output into the main process stdout. // // Session logic (WebRTC, control state, audio routing) all lives in the diff --git a/packages/remote-input/README.md b/packages/remote-input/README.md index 2b50979..ec4cc3a 100644 --- a/packages/remote-input/README.md +++ b/packages/remote-input/README.md @@ -124,14 +124,14 @@ system cursor directly. ## Platform support -| Platform | Backend | Two cursors | Requirements | -| ----------------------- | ----------------- | ---------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | -| macOS | `nut-js` | Full — local pointer restored | Accessibility permission (see below) | -| Windows | `nut-js` | Full — local pointer restored | None. Admin only to drive elevated windows. | -| Linux / X11 | `nut-js` | Full — local pointer restored | None | -| Linux / Wayland (KDE) | `wayland-ydotool` | Full via `KWinCursorProvider` — falls back to leaving the pointer where the click landed | `ydotool` + running `ydotoold` with `/dev/uinput`; `gdbus` for cursor reporting | -| Linux / Wayland (other) | `wayland-ydotool` | Partial — movement never hijacked, but a click leaves the pointer where it landed | `ydotool` + a running `ydotoold` with `/dev/uinput` | -| Linux / Wayland | `wayland-portal` | n/a | Diagnostic only — reports why control is unavailable | +| Platform | Backend | Two cursors | Requirements | +| ----------------------- | ----------------- | ------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | +| macOS | `nut-js` | Full — local pointer restored | Accessibility permission (see below) | +| Windows | `nut-js` | Full — local pointer restored | None. Admin only to drive elevated windows. | +| Linux / X11 | `nut-js` | Full — local pointer restored | None | +| Linux / Wayland (KDE) | `wayland-ydotool` | Full via `KWinCursorProvider` — without it, movement remains virtual and a click leaves the pointer where it landed | `ydotool` + running `ydotoold` with `/dev/uinput`; `gdbus` for cursor reporting | +| Linux / Wayland (other) | `wayland-ydotool` | Partial — movement never hijacked, but a click leaves the pointer where it landed | `ydotool` + a running `ydotoold` with `/dev/uinput` | +| Linux / Wayland | `wayland-portal` | n/a | Diagnostic only — reports why control is unavailable | > This package injects into a real OS, so it runs only where one exists. A > browser cannot be the _controlled_ machine; a browser-based client can only diff --git a/packages/remote-input/src/injector.test.ts b/packages/remote-input/src/injector.test.ts index 239b14e..67d27a7 100644 --- a/packages/remote-input/src/injector.test.ts +++ b/packages/remote-input/src/injector.test.ts @@ -640,13 +640,7 @@ describe('RemoteInputInjector two-cursor mode', () => { expect(moves(backend)).toHaveLength(0); }); - // Regression. Two cursors rest on one absolute positioning call per click - // with nothing in between to correct it, which is only trustworthy where the - // pointer can be read back. On a host that cannot report it — Wayland with - // no compositor helper — that made every click a single unverified "move - // there, now press": the guest saw their cursor move and nothing respond. - // Where the pointer cannot be read, movement must drive the real cursor. - it('drives the cursor directly when the host cannot report the pointer', async () => { + it('keeps movement virtual when the host cannot report the pointer', async () => { const backend = fakeBackend(); delete (backend as { getCursorPosition?: unknown }).getCursorPosition; const injector = twoCursorInjector(backend); @@ -654,14 +648,12 @@ describe('RemoteInputInjector two-cursor mode', () => { await injector.inject({ type: 'mouse', action: 'move', x: 0.3, y: 0.4 }); - expect(backend.inject).toHaveBeenCalledWith( - expect.objectContaining({ action: 'move', x: 0.3, y: 0.4 }) - ); + expect(moves(backend)).toHaveLength(0); }); - // The method can exist and still refuse to answer (KWin not reporting), so - // the first null has to switch the strategy rather than being shrugged off. - it('switches to driving the cursor once a position read comes back empty', async () => { + // The method can exist and still refuse to answer (KWin not reporting). A + // click then cannot be restored, but remote movement must remain virtual. + it('does not hijack movement once a position read comes back empty', async () => { const backend = fakeBackend({ getCursorPosition: vi.fn().mockResolvedValue(null) }); const injector = twoCursorInjector(backend); injector.enable(); @@ -674,9 +666,10 @@ describe('RemoteInputInjector two-cursor mode', () => { await injector.inject({ type: 'mouse', action: 'down', button: 'left', x: 0.2, y: 0.2 }); await injector.inject({ type: 'mouse', action: 'up', button: 'left', x: 0.2, y: 0.2 }); - // From here movement drives the real cursor, so clicks land where aimed. + // From here a later click cannot restore the local pointer, but movement + // must not continuously warp it away from the host. await injector.inject({ type: 'mouse', action: 'move', x: 0.7, y: 0.8 }); - expect(moves(backend).at(-1)?.[0]).toMatchObject({ x: 0.7, y: 0.8 }); + expect(moves(backend)).toHaveLength(0); }); // A host that *can* report the pointer must keep both cursors, or the fix @@ -878,7 +871,7 @@ describe('move coalescing', () => { await Promise.all(promises); } - it('drops moves that a newer position has already replaced', async () => { + it('keeps the newest superseding move when the queue drains', async () => { const { backend, release, injected } = blockingBackend(); const injector = makeInjector(backend); injector.enable(); @@ -890,10 +883,11 @@ describe('move coalescing', () => { await drain(release, [inFlight, ...queued]); - // The middle position is gone: it was obsolete before it ever ran. + // The middle position is gone, but the final position is injected rather + // than silently lost with it. const xs = injected.filter((e) => 'x' in e).map((e) => (e as { x: number }).x); expect(xs).not.toContain(0.2); - expect(injected.length).toBeLessThan(3); + expect(xs).toContain(0.3); }); it('never drops a click, however far behind the queue is', async () => { @@ -932,8 +926,8 @@ describe('move coalescing', () => { expect(xs).toContain(0.3); }); - it('reports where the viewer stopped even when the last move was dropped', async () => { - const { backend, release } = blockingBackend(); + it('reports and injects where the viewer stopped after coalescing', async () => { + const { backend, release, injected } = blockingBackend(); const injector = makeInjector(backend); injector.enable(); @@ -945,6 +939,7 @@ describe('move coalescing', () => { // The overlay still has to draw the guest's cursor where they left it. expect(injector.getRemoteCursorPosition().x).toBe(0.9); + expect(injected.some((event) => 'x' in event && event.x === 0.9)).toBe(true); }); it('counts what it dropped, so a laggy host can be told apart from a busy one', async () => { diff --git a/packages/remote-input/src/injector.ts b/packages/remote-input/src/injector.ts index eabfd71..a937e85 100644 --- a/packages/remote-input/src/injector.ts +++ b/packages/remote-input/src/injector.ts @@ -113,35 +113,16 @@ export class RemoteInputInjector { /** Where the local pointer was before a remote click borrowed it. */ private borrowedFrom: { x: number; y: number } | null = null; - /** - * Whether this host can actually keep two cursors apart. Null until known. - * - * Two-cursor mode rests on a single absolute positioning call per click with - * nothing in between to correct it. That is only trustworthy where the - * backend can also read the pointer back — the same platforms whose absolute - * positioning is exact. Where it cannot (Wayland with no compositor helper), - * a click is one unverified "move there, now press" with no feedback, and if - * the move does not land the click happens wherever the host left the - * pointer. The guest sees their cursor move and nothing respond. - * - * So where the pointer cannot be read back, remote movement drives the real - * cursor instead. The host's pointer gets borrowed for as long as the guest - * is steering, which is the lesser cost: a click that lands beats a cursor - * that stayed put. - */ - private cursorReportingWorks: boolean | null = null; - // Every async operation that results in an OS-level button press or release // chains through this promise. disable() and emergencyStop() wait on it so // releaseAll always runs *after* trackHeldState has recorded the press. private pendingInject: Promise | null = null; - /** - * Pointer moves waiting on the queue. See `supersededMove`. - * - * A counter rather than a flag: the in-flight move and the one behind it are - * different things, and only the second is safe to drop. - */ + /** Number of real pointer moves in the serialized injection queue. */ private queuedMoves = 0; + /** Newest non-drag move received while another move is still queued. */ + private pendingCoalescedMove: MouseMoveEvent | null = null; + /** Callers awaiting discarded moves complete when their replacement lands. */ + private pendingCoalescedMoveResolvers: (() => void)[] = []; constructor(options: RemoteInputInjectorOptions = {}) { this.selection = options.selection ?? getInputBackendSelection(); @@ -204,19 +185,12 @@ export class RemoteInputInjector { this.enabled = true; this.rateLimiter.reset(); - // A backend with no way to read the pointer at all can be ruled out now, - // before a single event arrives, rather than after the first misplaced - // click. Backends that have the method but may still fail to answer are - // settled on first use, in `dispatch`. - this.cursorReportingWorks = backend.getCursorPosition ? null : false; - - // Start cursor reporting so we can restore the host pointer after - // remote clicks. Where it cannot start, movement drives the real cursor - // instead so clicks keep landing — see `cursorReportingWorks`. + // Start cursor reporting so we can restore the host pointer after remote + // clicks. A failed report only means click restoration is unavailable; it + // must never make virtual movement take over the host's cursor. if (this.virtualCursor) { void backend.startCursorReporting?.().catch((error: unknown) => { this.logger.warn('[RemoteInput] Cursor reporting could not start', { error }); - this.cursorReportingWorks = false; }); } @@ -334,11 +308,10 @@ export class RemoteInputInjector { if (event.action === 'move') { this.remotePosition = { x: event.x, y: event.y }; - // Virtual by default. Two exceptions, both of which need real motion: - // a drag in progress, and a host whose pointer cannot be read back — - // there, keeping the cursors apart would stake every click on a single - // uncorrected positioning call. See `cursorReportingWorks`. - if (this.virtualCursor && this.cursorReportingWorks !== false && !dragging) return; + // Virtual movement must remain virtual even if cursor reporting fails. + // On that host a click cannot be restored, but constantly warping the + // host's pointer makes their UI unusable for the entire control session. + if (this.virtualCursor && !dragging) return; await backend.inject(this.withEdgeMargin(event)); return; @@ -350,13 +323,8 @@ export class RemoteInputInjector { // Borrow the real pointer, unless a previous press already borrowed it. if (!dragging) { const reported = (await backend.getCursorPosition?.()) ?? null; - // A null here means the compositor will not report the pointer, so two - // cursors cannot be kept apart accurately. Remember it: from now on - // movement drives the real cursor, and clicks stop depending on a single - // positioning call landing. - if (reported === null) this.cursorReportingWorks = false; - else this.cursorReportingWorks = true; - + // A null here means the compositor will not report the pointer, so this + // click cannot later restore it. Movement nevertheless remains virtual. this.borrowedFrom ??= reported; } @@ -544,31 +512,6 @@ export class RemoteInputInjector { this.onRejected?.(reason, event, detail); } - /** - * Is this move already obsolete? - * - * Every injection is serialized, and on some hosts each one costs a process - * spawn. A viewer streams pointer movement at their display's refresh rate, - * which arrives faster than that drains — so the queue grows without bound, - * the host's pointer trails further and further behind, and a click ends up - * stuck behind hundreds of stale positions before it is even attempted. The - * session reads as "laggy and I cannot click anything". - * - * Nothing is lost by dropping a superseded move: only the newest position - * matters, because a move is a statement about where the pointer *is*, not a - * step along a path. Clicks, scrolls and keystrokes are never dropped — - * those are discrete actions, and one of them going missing is a bug. - * - * A move that is part of a drag is also kept: there the intermediate motion - * is the point, since anything tracking a drag (text selection, canvas apps, - * file managers) needs to see the path and not just its endpoints. - */ - private supersededMove(event: InputEvent): event is MouseMoveEvent { - if (event.type !== 'mouse' || event.action !== 'move') return false; - if (this.heldButtons.size > 0) return false; - return this.queuedMoves > 0; - } - async inject(event: InputEvent): Promise { this.stats.received += 1; @@ -577,17 +520,34 @@ export class RemoteInputInjector { return; } - // Drop a move that a newer one has already replaced, before it can take a - // place in the queue. Tracked rather than inferred, because the queue is a - // promise chain with no length to inspect. const isMove = event.type === 'mouse' && event.action === 'move'; - if (this.supersededMove(event)) { - // Remember it anyway: if this turns out to be the last move the viewer - // sends, the cursor overlay still needs to know where they stopped. + if ( + event.type === 'mouse' && + event.action === 'move' && + this.heldButtons.size === 0 && + this.queuedMoves > 0 + ) { + // Preserve the latest position, rather than merely discarding a stale + // move. The queue flushes it before any following discrete input and + // once the in-flight move completes, so the cursor always settles where + // the viewer stopped. + this.pendingCoalescedMove = event; this.remotePosition = { x: event.x, y: event.y }; this.stats.coalesced += 1; + await new Promise((resolve) => this.pendingCoalescedMoveResolvers.push(resolve)); return; } + + // A click, scroll or key must run after the freshest pending move, not the + // stale one currently in flight. This is what keeps a rapid move-and-click + // sequence ordered without growing an unbounded move queue. + if (!isMove) this.flushCoalescedMove(); + await this.enqueue(event); + } + + /** Queue a real injection. Coalesced callers resolve when this one finishes. */ + private async enqueue(event: InputEvent, resolvers: (() => void)[] = []): Promise { + const isMove = event.type === 'mouse' && event.action === 'move'; if (isMove) this.queuedMoves += 1; // Serialize every injection so disable() and emergencyStop() can wait for @@ -633,9 +593,26 @@ export class RemoteInputInjector { } finally { if (isMove) this.queuedMoves = Math.max(0, this.queuedMoves - 1); resolve?.(); + for (const done of resolvers) done(); + + // With no later discrete event to flush the move, queue it as soon as + // the current move drains. `void` is safe: all waiting callers hold its + // resolvers, and errors are handled inside enqueue. + if (isMove) this.flushCoalescedMove(); } } + /** Move the latest coalesced position onto the serialized queue. */ + private flushCoalescedMove(): void { + const event = this.pendingCoalescedMove; + if (!event) return; + + this.pendingCoalescedMove = null; + const resolvers = this.pendingCoalescedMoveResolvers; + this.pendingCoalescedMoveResolvers = []; + void this.enqueue(event, resolvers); + } + /** * Disable injection and release every key and button the remote peer may * still be holding. Called on panic hotkeys and on disconnect, so a dropped From 8c3740167603fa94f586ea857cb7e24eefe204de Mon Sep 17 00:00:00 2001 From: Mr P-Tech Date: Tue, 11 Aug 2026 11:56:34 +0100 Subject: [PATCH 2/2] test(control): exercise pointer lock permissions --- apps/desktop/src/main/window.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/main/window.test.ts b/apps/desktop/src/main/window.test.ts index a37c016..7b82e59 100644 --- a/apps/desktop/src/main/window.test.ts +++ b/apps/desktop/src/main/window.test.ts @@ -62,6 +62,7 @@ beforeEach(() => { describe('createMainWindow', () => { it('allows pointer lock through both Electron permission paths', async () => { const { createMainWindow } = await import('./window'); + const { session } = await import('electron'); await createMainWindow(false); const requestHandler = vi @@ -72,10 +73,10 @@ describe('createMainWindow', () => { .mock.calls.at(-1)?.[0]; const callback = vi.fn(); - requestHandler?.({} as Electron.WebContents, 'pointerLock', callback, {}); + requestHandler?.({} as Electron.WebContents, 'pointerLock', callback, {} as never); expect(callback).toHaveBeenCalledWith(true); - expect(checkHandler?.({} as Electron.WebContents, 'pointerLock', '', {})).toBe(true); + expect(checkHandler?.({} as Electron.WebContents, 'pointerLock', '', {} as never)).toBe(true); }); it('keeps the renderer unthrottled in the background', async () => {