diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 9e0405397..983801c7e 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -19,9 +19,10 @@ It validates the compositor behavior that unit tests cannot see: - the automation group loads the compiled production keyboard boundary and guest preload, then proves that initial page window handlers get first refusal, unhandled shortcuts synchronously suppress editable browser defaults before crossing the host - boundary, handlers registered after preload still get first refusal, focused iframes - share the same boundary, digit wildcard shortcuts cross, and background automation stays - in the guest. + boundary, shortcuts marked unavailable in editable targets retain the browser field's + native behavior, handlers registered after preload still get first refusal, focused + iframes share the same boundary, digit wildcard shortcuts cross, and background automation + stays in the guest. Run it with the repo Electron: diff --git a/packages/app/src/keyboard/browser-shortcuts.test.ts b/packages/app/src/keyboard/browser-shortcuts.test.ts index be0aa026c..9dd323946 100644 --- a/packages/app/src/keyboard/browser-shortcuts.test.ts +++ b/packages/app/src/keyboard/browser-shortcuts.test.ts @@ -178,6 +178,20 @@ describe("buildBrowserKeyboardPolicy", () => { } }); + it("marks editable-only exclusions for enforcement inside the guest", () => { + const bindings = buildEffectiveBindings({}); + const policy = buildBrowserKeyboardPolicy({ bindings, isMac: true, isDesktop: true }); + + expect(policy.prefixes).toContainEqual({ + alt: false, + code: "ArrowLeft", + control: false, + editable: false, + meta: true, + shift: true, + }); + }); + it("does not publish plain browser keys", () => { const bindings = buildEffectiveBindings({}); const policy = buildBrowserKeyboardPolicy({ bindings, isMac: false, isDesktop: true }); diff --git a/packages/app/src/keyboard/browser-shortcuts.ts b/packages/app/src/keyboard/browser-shortcuts.ts index 470005e71..003790802 100644 --- a/packages/app/src/keyboard/browser-shortcuts.ts +++ b/packages/app/src/keyboard/browser-shortcuts.ts @@ -11,6 +11,7 @@ export interface BrowserShortcutPrefix { code: string; codeFallback?: true; control: boolean; + editable?: false; key?: string; meta: boolean; repeat?: false; @@ -81,7 +82,11 @@ export function parseBrowserShortcutInput(value: unknown): BrowserShortcutInput }; } -function prefixFromCombo(combo: KeyCombo, isMac: boolean): BrowserShortcutPrefix | null { +function prefixFromCombo( + combo: KeyCombo, + isMac: boolean, + editable: false | undefined, +): BrowserShortcutPrefix | null { const prefix: BrowserShortcutPrefix = { alt: combo.alt === true, code: combo.code, @@ -92,6 +97,9 @@ function prefixFromCombo(combo: KeyCombo, isMac: boolean): BrowserShortcutPrefix if (combo.codeFallback === true) { prefix.codeFallback = true; } + if (editable === false) { + prefix.editable = false; + } if (combo.key) { prefix.key = combo.key; } @@ -117,7 +125,7 @@ function isBrowserNativeNavigationPrefix(prefix: BrowserShortcutPrefix, isMac: b function canCrossBrowserBoundary(binding: ParsedShortcutBinding, isMac: boolean): boolean { return binding.parsedChord.every((combo) => { - const prefix = prefixFromCombo(combo, isMac); + const prefix = prefixFromCombo(combo, isMac, binding.when?.editable); return prefix !== null && !isBrowserNativeNavigationPrefix(prefix, isMac); }); } @@ -128,6 +136,7 @@ function prefixKey(prefix: BrowserShortcutPrefix): string { prefix.key ?? "", prefix.shiftedKey ?? "", prefix.codeFallback ?? "", + prefix.editable ?? "", prefix.control, prefix.meta, prefix.alt, @@ -166,7 +175,7 @@ function buildBrowserShortcutPrefixes(input: BrowserShortcutPolicyInput): Browse if (!combo) { continue; } - const prefix = prefixFromCombo(combo, input.isMac); + const prefix = prefixFromCombo(combo, input.isMac, binding.when?.editable); if (!prefix) { continue; } diff --git a/packages/desktop/capture-harness/main.js b/packages/desktop/capture-harness/main.js index 889423ca4..e8eeff2bf 100644 --- a/packages/desktop/capture-harness/main.js +++ b/packages/desktop/capture-harness/main.js @@ -1588,8 +1588,11 @@ function sendContainedEnter(guest) { }); } -function automationBrowserShortcut(guest, keyCode = "B") { +function automationBrowserShortcut(guest, keyCode = "B", options = {}) { const modifiers = [process.platform === "darwin" ? "meta" : "control"]; + if (options.shift) { + modifiers.push("shift"); + } guest.sendInputEvent({ type: "keyDown", keyCode, @@ -1846,6 +1849,31 @@ async function verifyFocusedIframeShortcut({ guest, shortcutInputs, expectedInpu return { group: "automation", check: "browser-shortcut-focused-iframe", pass: true }; } +async function verifyEditableShortcutExclusion({ guest, shortcutInputs }) { + await guest.executeJavaScript( + `window.editableExcludedShortcut = null; + window.addEventListener("keydown", (event) => { + if (event.code === "ArrowLeft" && event.shiftKey) { + window.editableExcludedShortcut = { defaultPrevented: event.defaultPrevented }; + } + }, { once: true });`, + true, + ); + automationBrowserShortcut(guest, "Left", { shift: true }); + await delay(100); + const observedEvent = await guest.executeJavaScript("window.editableExcludedShortcut", true); + if ( + shortcutInputs.length !== 2 || + !isDeepStrictEqual(observedEvent, { defaultPrevented: false }) + ) { + fail( + `editable-only exclusion escaped guest: inputs=${JSON.stringify(shortcutInputs)} event=${JSON.stringify(observedEvent)}`, + ); + } + pass("automation editable-only shortcuts keep browser field ownership"); + return { group: "automation", check: "browser-shortcut-editable-exclusion", pass: true }; +} + async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, sentinel }) { const checks = []; const { shortcutInputs } = sentinel; @@ -1968,6 +1996,8 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, pass("automation guest preload owns an unhandled editable browser shortcut"); checks.push({ group: "automation", check: "browser-shortcut-editable-owned", pass: true }); + checks.push(await verifyEditableShortcutExclusion({ guest, shortcutInputs })); + automationBrowserShortcut(guest, "1"); await waitForBrowserShortcutInput(shortcutInputs, 3); const expectedDigitShortcutInput = { @@ -2137,6 +2167,15 @@ async function runAutomationGroup() { repeat: false, shift: false, }, + { + alt: false, + code: "ArrowLeft", + control: !usesMeta, + editable: false, + meta: usesMeta, + repeat: false, + shift: true, + }, { alt: false, code: "Digit", diff --git a/packages/desktop/src/features/browser-automation/ipc.test.ts b/packages/desktop/src/features/browser-automation/ipc.test.ts index aeec6065d..2cb168f4c 100644 --- a/packages/desktop/src/features/browser-automation/ipc.test.ts +++ b/packages/desktop/src/features/browser-automation/ipc.test.ts @@ -1,7 +1,7 @@ import type { Rectangle } from "electron"; import { describe, expect, test, vi } from "vitest"; import type { TabImage } from "./service.js"; -import { adaptWebContents } from "./ipc.js"; +import { adaptWebContents, HostSnapshotEngineRegistry } from "./ipc.js"; import type { IsolatedKeyboardInputEvent } from "./trusted-input.js"; class FakeImage implements TabImage { @@ -191,6 +191,19 @@ class FakeWebContents { } describe("browser automation IPC adapter", () => { + test("isolates snapshot refs by host window and releases them on destruction", () => { + const registry = new HostSnapshotEngineRegistry(); + const firstHost = new FakeHostWebContents(1); + const secondHost = new FakeHostWebContents(2); + + const firstEngine = registry.get(firstHost); + expect(registry.get(firstHost)).toBe(firstEngine); + expect(registry.get(secondHost)).not.toBe(firstEngine); + + firstHost.destroy(); + expect(registry.get(new FakeHostWebContents(1))).not.toBe(firstEngine); + }); + test("sends contained keyboard input directly to the guest", () => { const contents = new FakeWebContents(19); const tab = adaptWebContents(contents); @@ -560,6 +573,21 @@ describe("browser automation IPC adapter", () => { }); }); +class FakeHostWebContents { + private destroyedListener: (() => void) | null = null; + + public constructor(public readonly id: number) {} + + public once(event: "destroyed", listener: () => void): void { + expect(event).toBe("destroyed"); + this.destroyedListener = listener; + } + + public destroy(): void { + this.destroyedListener?.(); + } +} + async function flushMicrotasks(): Promise { await Promise.resolve(); await Promise.resolve(); diff --git a/packages/desktop/src/features/browser-automation/ipc.ts b/packages/desktop/src/features/browser-automation/ipc.ts index ac479178e..12f35b798 100644 --- a/packages/desktop/src/features/browser-automation/ipc.ts +++ b/packages/desktop/src/features/browser-automation/ipc.ts @@ -17,6 +17,7 @@ import { promptShimRestoreScript, } from "./dialog-handling.js"; import { executeAutomationCommand } from "./service.js"; +import { BrowserSnapshotEngine } from "./snapshot-engine.js"; import { listRegisteredPaseoBrowserIds, listRegisteredPaseoBrowserIdsForWorkspace, @@ -35,6 +36,36 @@ interface IpcHandlerRegistry { handle(channel: string, listener: (event: unknown, ...args: unknown[]) => unknown): void; } +interface HostWebContents { + readonly id: number; + once(event: "destroyed", listener: () => void): void; +} + +export class HostSnapshotEngineRegistry { + private readonly entries = new Map< + number, + { hostContents: HostWebContents; snapshotEngine: BrowserSnapshotEngine } + >(); + + public get(hostContents: HostWebContents): BrowserSnapshotEngine { + const existing = this.entries.get(hostContents.id); + if (existing) { + return existing.snapshotEngine; + } + const snapshotEngine = new BrowserSnapshotEngine(); + const entry = { hostContents, snapshotEngine }; + this.entries.set(hostContents.id, entry); + hostContents.once("destroyed", () => { + if (this.entries.get(hostContents.id) === entry) { + this.entries.delete(hostContents.id); + } + }); + return snapshotEngine; + } +} + +const hostSnapshotEngines = new HostSnapshotEngineRegistry(); + interface WebContentsDebugger { isAttached(): boolean; attach(protocolVersion?: string): void; @@ -362,8 +393,9 @@ export function registerBrowserAutomationIpc(options?: { ipc?: IpcHandlerRegistr const ipc = options?.ipc ?? ipcMain; ipc.handle("paseo:browser:execute-automation-command", async (event, rawRequest: unknown) => { - const hostWebContentsId = (event as { sender?: { id?: unknown } }).sender?.id; - if (typeof hostWebContentsId !== "number") { + const hostContents = (event as { sender?: HostWebContents }).sender; + const hostWebContentsId = hostContents?.id; + if (!hostContents || typeof hostWebContentsId !== "number") { return { requestId: readRequestId(rawRequest), ok: false as const, @@ -386,7 +418,9 @@ export function registerBrowserAutomationIpc(options?: { ipc?: IpcHandlerRegistr }, }; } - return executeAutomationCommand(parsed.data, registry); + return executeAutomationCommand(parsed.data, registry, { + snapshotEngine: hostSnapshotEngines.get(hostContents), + }); }); } diff --git a/packages/desktop/src/features/browser-keyboard/guest-preload.ts b/packages/desktop/src/features/browser-keyboard/guest-preload.ts index c4a178e19..c4e2bd15e 100644 --- a/packages/desktop/src/features/browser-keyboard/guest-preload.ts +++ b/packages/desktop/src/features/browser-keyboard/guest-preload.ts @@ -13,12 +13,14 @@ interface BrowserKeyboardPolicyPayload extends BrowserKeyboardPolicy { } function matchesPolicy(event: KeyboardEvent): boolean { + const editable = isEditableTarget(event.target); return policy.some((prefix) => { if ( prefix.alt !== event.altKey || prefix.control !== event.ctrlKey || prefix.meta !== event.metaKey || prefix.shift !== event.shiftKey || + (prefix.editable === false && editable) || (prefix.repeat === false && event.repeat) ) { return false; @@ -37,6 +39,18 @@ function matchesPolicy(event: KeyboardEvent): boolean { }); } +function isEditableTarget(target: EventTarget | null): boolean { + if (!(target instanceof Element)) { + return false; + } + const element = target as HTMLElement; + if (element.isContentEditable) { + return true; + } + const tag = element.tagName.toLowerCase(); + return tag === "input" || tag === "textarea" || tag === "select"; +} + function matchesCode(prefixCode: string, eventCode: string): boolean { if (prefixCode !== "Digit") { return prefixCode === eventCode; diff --git a/packages/desktop/src/features/browser-keyboard/policy.test.ts b/packages/desktop/src/features/browser-keyboard/policy.test.ts index 53b89377f..6e07a8ceb 100644 --- a/packages/desktop/src/features/browser-keyboard/policy.test.ts +++ b/packages/desktop/src/features/browser-keyboard/policy.test.ts @@ -101,6 +101,76 @@ describe("browser keyboard policy", () => { ).toBeNull(); }); + test("preserves editable exclusions and rejects permissive values", () => { + expect( + parseBrowserKeyboardPolicy({ + menuPrefixes: [], + prefixes: [ + { + alt: false, + code: "ArrowLeft", + control: false, + editable: false, + meta: true, + shift: true, + }, + ], + }), + ).toEqual({ + menuPrefixes: [], + prefixes: [ + { + alt: false, + code: "ArrowLeft", + control: false, + editable: false, + meta: true, + shift: true, + }, + ], + }); + expect( + parseBrowserKeyboardPolicy({ + menuPrefixes: [], + prefixes: [ + { + alt: false, + code: "ArrowLeft", + control: false, + editable: true, + meta: true, + shift: true, + }, + ], + }), + ).toBeNull(); + + const policy = parseBrowserKeyboardPolicy({ + menuPrefixes: [], + prefixes: [ + { + alt: false, + code: "ArrowLeft", + control: false, + editable: false, + meta: true, + shift: true, + }, + ], + }); + const input = { + alt: false, + code: "ArrowLeft", + control: false, + key: "ArrowLeft", + meta: true, + repeat: false, + shift: true, + }; + expect(matchesBrowserShortcutPolicy(policy!, { ...input, editable: false })).toBe(true); + expect(matchesBrowserShortcutPolicy(policy!, { ...input, editable: true })).toBe(false); + }); + test("keeps browser identities exact", () => { expect( parseBrowserShortcutInput({ diff --git a/packages/desktop/src/features/browser-keyboard/policy.ts b/packages/desktop/src/features/browser-keyboard/policy.ts index 2f972cbd4..0dae1630e 100644 --- a/packages/desktop/src/features/browser-keyboard/policy.ts +++ b/packages/desktop/src/features/browser-keyboard/policy.ts @@ -3,6 +3,7 @@ export interface BrowserShortcutPrefix { code: string; codeFallback?: true; control: boolean; + editable?: false; key?: string; meta: boolean; repeat?: false; @@ -30,6 +31,7 @@ export interface BrowserShortcutMatchInput { alt: boolean; code: string; control: boolean; + editable?: boolean; key: string; meta: boolean; repeat: boolean; @@ -42,6 +44,16 @@ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +function hasValidOptionalPrefixFields(value: Record): boolean { + return ( + (value.key === undefined || typeof value.key === "string") && + (value.shiftedKey === undefined || typeof value.shiftedKey === "string") && + (value.codeFallback === undefined || value.codeFallback === true) && + (value.editable === undefined || value.editable === false) && + (value.repeat === undefined || value.repeat === false) + ); +} + function parsePrefix(value: unknown): BrowserShortcutPrefix | null { if (!isRecord(value)) { return null; @@ -53,10 +65,7 @@ function parsePrefix(value: unknown): BrowserShortcutPrefix | null { typeof value.control !== "boolean" || typeof value.meta !== "boolean" || typeof value.shift !== "boolean" || - (value.key !== undefined && typeof value.key !== "string") || - (value.shiftedKey !== undefined && typeof value.shiftedKey !== "string") || - (value.codeFallback !== undefined && value.codeFallback !== true) || - (value.repeat !== undefined && value.repeat !== false) + !hasValidOptionalPrefixFields(value) ) { return null; } @@ -65,6 +74,7 @@ function parsePrefix(value: unknown): BrowserShortcutPrefix | null { code: value.code, ...(value.codeFallback === true ? { codeFallback: true } : {}), control: value.control, + ...(value.editable === false ? { editable: false } : {}), ...(typeof value.key === "string" ? { key: value.key.toLowerCase() } : {}), meta: value.meta, ...(value.repeat === false ? { repeat: false } : {}), @@ -138,6 +148,7 @@ function matchesPrefix(prefix: BrowserShortcutPrefix, input: BrowserShortcutMatc prefix.control !== input.control || prefix.meta !== input.meta || prefix.shift !== input.shift || + (prefix.editable === false && input.editable === true) || (prefix.repeat === false && input.repeat) ) { return false;