diff --git a/apps/desktop/package.json b/apps/desktop/package.json index f8c06f8..625a863 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -31,6 +31,7 @@ "@radix-ui/react-label": "^2.1.8", "@radix-ui/react-slot": "^1.2.4", "class-variance-authority": "^0.7.1", + "dbus-next": "^0.10.2", "dotenv": "^17.2.3", "linkify-react": "^4.3.2", "linkifyjs": "^4.3.2", diff --git a/apps/desktop/src/main/ipc/input.test.ts b/apps/desktop/src/main/ipc/input.test.ts index e7db45e..6044a6e 100644 --- a/apps/desktop/src/main/ipc/input.test.ts +++ b/apps/desktop/src/main/ipc/input.test.ts @@ -13,6 +13,8 @@ vi.mock('../input/injector', () => ({ backend: 'nut-js', backendSupported: true, stats: { received: 0, injected: 0, rejected: 0, errors: 0 }, + heldButtons: 0, + heldKeys: 0, }), updateScreenSize: vi.fn(), emergencyStop: vi.fn().mockResolvedValue(undefined), @@ -99,6 +101,8 @@ describe('IPC Input Handlers', () => { backend: 'nut-js', backendSupported: true, stats: { received: 0, injected: 0, rejected: 0, errors: 0 }, + heldButtons: 0, + heldKeys: 0, }); const handler = mockIpcMainHandlers.get('input:enable')!; @@ -132,6 +136,8 @@ describe('IPC Input Handlers', () => { backend: 'nut-js', backendSupported: true, stats: { received: 1, injected: 1, rejected: 0, errors: 0 }, + heldButtons: 0, + heldKeys: 0, }); const handler = mockIpcMainHandlers.get('input:status')!; diff --git a/apps/desktop/src/main/overlay/cursorOverlay.test.ts b/apps/desktop/src/main/overlay/cursorOverlay.test.ts new file mode 100644 index 0000000..bc8112b --- /dev/null +++ b/apps/desktop/src/main/overlay/cursorOverlay.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { canShowDesktopOverlay } from './cursorOverlay'; + +// The overlay is a fullscreen, always-on-top window over the host's real +// desktop. If it fails to be inert it takes input the host cannot get back, +// which is the most damaging failure this app has. So the question is not +// "does click-through usually work" but "can we show it without taking focus +// at all" — and on Wayland Electron documents showInactive() as unsupported. +describe('canShowDesktopOverlay', () => { + it('refuses Wayland, where showInactive is unsupported', () => { + expect(canShowDesktopOverlay('wayland')).toBe(false); + }); + + it('allows the display servers whose window APIs Electron supports', () => { + expect(canShowDesktopOverlay('x11')).toBe(true); + expect(canShowDesktopOverlay('macos')).toBe(true); + expect(canShowDesktopOverlay('windows')).toBe(true); + }); + + // An unknown display server is only reported on Linux when neither + // WAYLAND_DISPLAY nor DISPLAY is set — a headless or unusual session rather + // than a compositor known to mishandle the window. + it('allows an unknown display server', () => { + expect(canShowDesktopOverlay('unknown')).toBe(true); + }); +}); diff --git a/apps/desktop/src/main/overlay/cursorOverlay.ts b/apps/desktop/src/main/overlay/cursorOverlay.ts index a18bb18..d9fccbe 100644 --- a/apps/desktop/src/main/overlay/cursorOverlay.ts +++ b/apps/desktop/src/main/overlay/cursorOverlay.ts @@ -13,12 +13,44 @@ */ import { BrowserWindow, screen } from 'electron'; +import { detectDisplayServer } from '../platform'; let overlay: BrowserWindow | null = null; +/** + * Whether a desktop-wide overlay can be shown safely on this display server. + * + * False on Wayland, because Electron documents the operations this window is + * built out of as unsupported there: + * + * - `showInactive()` — "Not supported on Wayland (Linux)". This is the one + * that matters. It is how the overlay appears *without* taking focus; with + * it unavailable there is no way to put a fullscreen always-on-top window + * on screen and be sure it has not grabbed the user's input. + * - `setPosition()` — "Not supported on Wayland (Linux)", and `getBounds()` + * reports `{ x: 0, y: 0 }`, so the window cannot be reliably placed. + * - the `level` argument to `setAlwaysOnTop` is documented macOS/Windows only. + * + * More generally: "On Wayland (Linux) it is generally not possible to + * programmatically resize windows after creation, or to position, move, focus, + * or blur windows without user input." + * + * The failure mode if we show it anyway is the worst one this app has — a + * fullscreen window over the host's desktop that takes input they cannot get + * back. The in-app cursor still draws inside the PairUX window, so the guest's + * pointer stays visible where the video is; only the desktop-wide overlay is + * given up. + * + * https://www.electronjs.org/docs/latest/api/browser-window + */ +export function canShowDesktopOverlay(displayServer: string): boolean { + return displayServer !== 'wayland'; +} + /** Escape hatch, in case a compositor mishandles a click-through window. */ function isDisabled(): boolean { - return process.env.PAIRUX_DISABLE_CURSOR_OVERLAY === '1'; + if (process.env.PAIRUX_DISABLE_CURSOR_OVERLAY === '1') return true; + return !canShowDesktopOverlay(detectDisplayServer()); } function buildHtml(): string { diff --git a/packages/remote-input/src/injector.test.ts b/packages/remote-input/src/injector.test.ts index 4465d47..08d4ca0 100644 --- a/packages/remote-input/src/injector.test.ts +++ b/packages/remote-input/src/injector.test.ts @@ -419,6 +419,125 @@ describe('RemoteInputInjector held-input safety', () => { vi.useRealTimers(); } }); + + // The failure that took a host's machine away from them. + // + // A lost "up" leaves the button held. The guest carries on moving the mouse, + // every move resets the idle watchdog, and it never fires. A held button + // makes dispatch treat movement as a drag and inject it, so the guest's + // pointer starts driving the host's and the host cannot use their own + // machine until control is revoked. The absolute timer is the only thing + // that ends this, so it must not be resettable by incoming movement. + it('releases a hold whose "up" was lost even while moves keep arriving', async () => { + vi.useFakeTimers(); + try { + const backend = fakeBackend(); + const injector = new RemoteInputInjector({ + selection: { kind: 'nut-js', platform: 'linux', displayServer: 'x11' }, + createBackend: () => backend, + logger: silentLogger, + virtualCursor: false, + holdTimeoutMs: 1000, + maxHoldMs: 5000, + }); + injector.enable(); + + await injector.inject(down()); + + // Movement never stops, so the idle timer is reset over and over and + // would on its own keep the button held indefinitely. + for (let i = 0; i < 20; i += 1) { + await vi.advanceTimersByTimeAsync(400); + await injector.inject({ type: 'mouse', action: 'move', x: 0.5, y: 0.5 }); + } + + const ups = vi + .mocked(backend.inject) + .mock.calls.filter(([e]) => 'action' in e && e.action === 'up'); + expect(ups.length).toBeGreaterThan(0); + expect(injector.getDiagnostics().heldButtons).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + // The absolute timer must not turn into a guillotine for real drags either: + // it starts once per hold and is not restarted by each new press. + it('does not restart the absolute timer on every press', async () => { + vi.useFakeTimers(); + try { + const backend = fakeBackend(); + const injector = new RemoteInputInjector({ + selection: { kind: 'nut-js', platform: 'linux', displayServer: 'x11' }, + createBackend: () => backend, + logger: silentLogger, + virtualCursor: false, + holdTimeoutMs: 1000, + maxHoldMs: 5000, + }); + injector.enable(); + + await injector.inject(down()); + for (let i = 0; i < 10; i += 1) { + await vi.advanceTimersByTimeAsync(400); + await injector.inject(down()); + } + + // 4s of presses is inside both windows, so nothing has been released. + expect(injector.getDiagnostics().heldButtons).toBe(1); + + await vi.advanceTimersByTimeAsync(2000); + expect(injector.getDiagnostics().heldButtons).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + // A clean press/release must leave nothing armed, or the next hold inherits + // a timer that is already partway through its window. + it('clears the absolute timer when a button is released normally', async () => { + vi.useFakeTimers(); + try { + const backend = fakeBackend(); + const injector = new RemoteInputInjector({ + selection: { kind: 'nut-js', platform: 'linux', displayServer: 'x11' }, + createBackend: () => backend, + logger: silentLogger, + virtualCursor: false, + holdTimeoutMs: 1000, + maxHoldMs: 5000, + }); + injector.enable(); + + await injector.inject(down()); + await vi.advanceTimersByTimeAsync(4000); + await injector.inject({ + type: 'mouse', + action: 'up', + button: 'left', + x: 0.5, + y: 0.5, + }); + vi.mocked(backend.inject).mockClear(); + + // A fresh hold, kept alive so only the absolute timer is under test. + // It gets a full 5s window; had it inherited the previous hold's timer + // there would be 1s left and the button would be released during this. + await injector.inject(down()); + for (let i = 0; i < 10; i += 1) { + await vi.advanceTimersByTimeAsync(400); + await injector.inject({ type: 'mouse', action: 'move', x: 0.5, y: 0.5 }); + } + + const ups = vi + .mocked(backend.inject) + .mock.calls.filter(([e]) => 'action' in e && e.action === 'up'); + expect(ups).toHaveLength(0); + expect(injector.getDiagnostics().heldButtons).toBe(1); + } finally { + vi.useRealTimers(); + } + }); }); // Remote mouse movement must never spend the local pointer — that is what diff --git a/packages/remote-input/src/injector.ts b/packages/remote-input/src/injector.ts index 73f461c..12f5416 100644 --- a/packages/remote-input/src/injector.ts +++ b/packages/remote-input/src/injector.ts @@ -33,8 +33,22 @@ export interface RemoteInputInjectorOptions { /** * How long a button or key may stay held with no further input before it * is force-released. Guards against a viewer dropping mid-drag. Default 5s. + * + * Reset by every event, so an active drag is never cut short. */ holdTimeoutMs?: number; + /** + * How long a button or key may stay held in total, however much input keeps + * arriving. Default 30s. + * + * `holdTimeoutMs` alone cannot bound a hold whose release was lost: the + * guest carries on moving the mouse, every move resets the idle timer, and + * the button stays down forever. A held button makes `dispatch` treat all + * movement as a drag and inject it, so the guest ends up driving the host's + * pointer and the host cannot use their own machine. This is the backstop, + * and it is deliberately never reset. + */ + maxHoldMs?: number; /** * Keep the local and remote cursors independent (default true). * @@ -77,7 +91,9 @@ export class RemoteInputInjector { private readonly heldButtons = new Set(); private readonly heldKeys = new Set(); private holdWatchdog: ReturnType | null = null; + private maxHoldWatchdog: ReturnType | null = null; private readonly holdTimeoutMs: number; + private readonly maxHoldMs: number; // Two-cursor mode: remote movement never moves the local pointer, so both // people keep a usable cursor at the same time. @@ -99,6 +115,7 @@ export class RemoteInputInjector { this.makeBackend = options.createBackend ?? createInputBackend; this.rateLimiter = new InputRateLimiter(options.maxEventsPerSecond ?? 1000); this.holdTimeoutMs = options.holdTimeoutMs ?? 5000; + this.maxHoldMs = options.maxHoldMs ?? 30_000; this.virtualCursor = options.virtualCursor ?? true; this.edgeMarginPx = Math.max(0, options.edgeMarginPx ?? 0); this.onRejected = options.onRejected; @@ -198,7 +215,10 @@ export class RemoteInputInjector { const keys = [...this.heldKeys]; this.heldButtons.clear(); this.heldKeys.clear(); + // Both timers: the hold is over, and a stale absolute timer would fire + // partway through the next one. this.clearHoldWatchdog(); + this.clearMaxHoldWatchdog(); if (buttons.length === 0 && keys.length === 0) return; @@ -359,19 +379,53 @@ export class RemoteInputInjector { } /** - * While something is held, arm a timer to release it if no further input - * arrives. A viewer whose connection drops mid-drag would otherwise leave - * the button down indefinitely. + * Arm both hold watchdogs. Two are needed, and one alone is a trap. + * + * The idle timer catches a viewer who disappears mid-drag: no further input + * of any kind arrives, so nothing would ever release the button. It is reset + * by every event, because a drag that is still receiving movement is alive + * and must not be torn apart at an arbitrary deadline. + * + * That reset is exactly what makes the idle timer insufficient on its own. + * If a button's "up" is lost while the guest keeps moving the mouse, every + * move re-arms the idle timer and it never fires. The button stays held, so + * `dispatch` treats all subsequent movement as a drag and injects it — the + * guest's pointer starts driving the host's, and the host cannot use their + * own machine until control is revoked. + * + * So the absolute timer runs from the moment the first button or key went + * down and is never reset. It is the only thing that bounds a hold whose + * release was lost, and it is generous enough that no real drag reaches it. */ private armHoldWatchdog(): void { this.clearHoldWatchdog(); - if (this.heldButtons.size === 0 && this.heldKeys.size === 0) return; + + if (this.heldButtons.size === 0 && this.heldKeys.size === 0) { + this.clearMaxHoldWatchdog(); + return; + } this.holdWatchdog = setTimeout(() => { void this.releaseAll('no input while a button or key was held'); }, this.holdTimeoutMs); // Never keep a Node process alive just for this timer. this.holdWatchdog.unref(); + + // Started once per hold, then left alone: re-arming it here would + // reintroduce the very stall this timer exists to break. + if (this.maxHoldWatchdog === null) { + this.maxHoldWatchdog = setTimeout(() => { + void this.releaseAll('held past the maximum hold duration'); + }, this.maxHoldMs); + this.maxHoldWatchdog.unref(); + } + } + + private clearMaxHoldWatchdog(): void { + if (this.maxHoldWatchdog !== null) { + clearTimeout(this.maxHoldWatchdog); + this.maxHoldWatchdog = null; + } } private trackHeldState(event: InputEvent): void { @@ -483,6 +537,7 @@ export class RemoteInputInjector { if (this.pendingInject) await this.pendingInject; await this.releaseAll('shutting down'); this.clearHoldWatchdog(); + this.clearMaxHoldWatchdog(); // The last chance to give the host their mouse back. // @@ -510,6 +565,8 @@ export class RemoteInputInjector { backend: backend.name, backendSupported: backend.supported, stats: { ...this.stats }, + heldButtons: this.heldButtons.size, + heldKeys: this.heldKeys.size, }; if (backend.reason !== undefined) diagnostics.reason = backend.reason; diff --git a/packages/remote-input/src/types.ts b/packages/remote-input/src/types.ts index d77081b..ae9d1a9 100644 --- a/packages/remote-input/src/types.ts +++ b/packages/remote-input/src/types.ts @@ -131,4 +131,14 @@ export interface InputDiagnostics { reason?: string; details?: Record; stats: InputStats; + /** + * How many buttons and keys the injector believes are held right now. + * + * Worth surfacing because a stuck hold is not a quiet failure: a held button + * makes every remote move inject as a drag, so the guest's pointer drives + * the host's and the host loses their machine. Non-zero here while nobody is + * pressing anything is that state, and is otherwise invisible. + */ + heldButtons: number; + heldKeys: number; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ee59090..5c77bc3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -98,6 +98,9 @@ importers: class-variance-authority: specifier: ^0.7.1 version: 0.7.1 + dbus-next: + specifier: ^0.10.2 + version: 0.10.2 dotenv: specifier: ^17.2.3 version: 17.2.3 @@ -11552,8 +11555,7 @@ snapshots: '@nodelib/fs.scandir': 2.1.5 fastq: 1.20.1 - '@nornagon/put@0.0.8': - optional: true + '@nornagon/put@0.0.8': {} '@npmcli/agent@3.0.0': dependencies: @@ -13752,7 +13754,6 @@ snapshots: xml2js: 0.4.23 optionalDependencies: usocket: 0.3.0 - optional: true debug@2.6.9: dependencies: @@ -13909,8 +13910,7 @@ snapshots: es-errors: 1.3.0 gopd: 1.2.0 - duplexer@0.1.2: - optional: true + duplexer@0.1.2: {} eastasianwidth@0.2.0: {} @@ -14333,7 +14333,6 @@ snapshots: split: 0.3.3 stream-combiner: 0.0.4 through: 2.3.8 - optional: true event-target-shim@5.0.1: {} @@ -14770,8 +14769,7 @@ snapshots: fresh@0.5.2: {} - from@0.1.7: - optional: true + from@0.1.7: {} fs-constants@1.0.0: {} @@ -15060,8 +15058,7 @@ snapshots: dependencies: hermes-estree: 0.25.1 - hexy@0.2.11: - optional: true + hexy@0.2.11: {} hoist-non-react-statics@3.3.2: dependencies: @@ -15587,8 +15584,7 @@ snapshots: dependencies: argparse: 2.0.1 - jsbi@2.0.5: - optional: true + jsbi@2.0.5: {} jsbn@0.1.1: optional: true @@ -15927,8 +15923,7 @@ snapshots: loglevel@1.9.2: {} - long@4.0.0: - optional: true + long@4.0.0: {} loose-envify@1.4.0: dependencies: @@ -16019,8 +16014,7 @@ snapshots: map-obj@5.0.0: {} - map-stream@0.1.0: - optional: true + map-stream@0.1.0: {} marked@18.0.5: {} @@ -16770,7 +16764,6 @@ snapshots: pause-stream@0.0.11: dependencies: through: 2.3.8 - optional: true pe-library@0.4.1: {} @@ -17782,7 +17775,6 @@ snapshots: split@0.3.3: dependencies: through: 2.3.8 - optional: true sprintf-js@1.0.3: {} @@ -17844,7 +17836,6 @@ snapshots: stream-combiner@0.0.4: dependencies: duplexer: 0.1.2 - optional: true strict-uri-encode@2.0.0: {} @@ -18131,8 +18122,7 @@ snapshots: throat@5.0.0: {} - through@2.3.8: - optional: true + through@2.3.8: {} timm@1.7.1: {} @@ -18888,7 +18878,6 @@ snapshots: dependencies: sax: 1.6.0 xmlbuilder: 11.0.1 - optional: true xml2js@0.5.0: dependencies: