diff --git a/lib/__tests__/plugin-sandbox-loader.test.ts b/lib/__tests__/plugin-sandbox-loader.test.ts index 307a5f2a5..1df200de3 100644 --- a/lib/__tests__/plugin-sandbox-loader.test.ts +++ b/lib/__tests__/plugin-sandbox-loader.test.ts @@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => ({ saveCode: vi.fn(), downloadManagedBundle: vi.fn(), createBackgroundInstance: vi.fn(), + registerShortcuts: vi.fn((..._args: unknown[]) => () => {}), })); vi.mock('../plugin-storage', () => ({ @@ -27,7 +28,7 @@ vi.mock('../plugin-sandbox/host-api', () => ({ cancelPluginDialogs: vi.fn(), })); vi.mock('../plugin-sandbox/shortcuts', () => ({ - registerShortcuts: () => () => {}, + registerShortcuts: (...args: unknown[]) => mocks.registerShortcuts(...args), })); import { loadSandboxedPlugin, unloadSandboxedPlugin, setSandboxStoreAccessor } from '../plugin-sandbox/loader'; @@ -177,3 +178,30 @@ describe('loadSandboxedPlugin bundle sourcing', () => { expect(lastStatus()).toMatchObject({ status: 'error', error: expect.stringMatching(/integrity mismatch/) }); }); }); + +describe('plugin shortcuts', () => { + const SHORTCUT = [{ id: 'new', keys: 'Alt+N', label: 'New' }]; + function withShortcuts() { + return { ...fakeBackground(), initPromise: Promise.resolve({ hooks: [], slots: [], shortcuts: SHORTCUT }) }; + } + + it('does not install shortcuts for a plugin without ui:keyboard', async () => { + mocks.getCode.mockResolvedValue(CODE); + mocks.createBackgroundInstance.mockImplementation(withShortcuts); + mocks.registerShortcuts.mockClear(); + + await loadSandboxedPlugin(plugin()); + + expect(mocks.registerShortcuts).not.toHaveBeenCalled(); + }); + + it('installs them with ui:keyboard', async () => { + mocks.getCode.mockResolvedValue(CODE); + mocks.createBackgroundInstance.mockImplementation(withShortcuts); + mocks.registerShortcuts.mockClear(); + + await loadSandboxedPlugin(plugin({ permissions: ['ui:keyboard'] })); + + expect(mocks.registerShortcuts).toHaveBeenCalledWith(expect.anything(), SHORTCUT); + }); +}); diff --git a/lib/__tests__/plugin-shortcuts.test.ts b/lib/__tests__/plugin-shortcuts.test.ts new file mode 100644 index 000000000..3344609cb --- /dev/null +++ b/lib/__tests__/plugin-shortcuts.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { registerShortcuts, isAllowedPluginCombo } from '../plugin-sandbox/shortcuts'; +import type { SandboxInstance } from '../plugin-sandbox/host-bridge'; + +function instance(pluginId: string) { + return { pluginId, invokeHook: vi.fn(async () => undefined) } as unknown as SandboxInstance & { invokeHook: ReturnType }; +} + +const disposers: Array<() => void> = []; +afterEach(() => { + while (disposers.length) disposers.pop()!(); +}); + +function press(key: string, mods: Partial> = {}) { + const ev = new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...mods }); + const outer = vi.fn(); + document.body.addEventListener('keydown', outer); + document.body.dispatchEvent(ev); + document.body.removeEventListener('keydown', outer); + return { defaultPrevented: ev.defaultPrevented, reachedApp: outer.mock.calls.length > 0 }; +} + +describe('plugin keyboard shortcuts', () => { + it.each(['Escape', 'c', 'Shift+G', 'Ctrl+K', 'Ctrl+F', 'Ctrl+P', 'Cmd+W', 'Ctrl+Shift+T', 'Tab', 'Alt+Tab'])( + 'refuses %s, which belongs to the app or the browser', + (combo) => { + expect(isAllowedPluginCombo(combo)).toBe(false); + }, + ); + + it.each(['Alt+N', 'Alt+Shift+N', 'Ctrl+Shift+L', 'Ctrl+Alt+S', 'Cmd+Shift+Y'])('allows %s', (combo) => { + expect(isAllowedPluginCombo(combo)).toBe(true); + }); + + it('does not take Escape away from the app', async () => { + const plugin = instance('grabber'); + disposers.push(registerShortcuts(plugin, [{ id: 'esc', keys: 'Escape', label: 'x' }])); + + const result = press('Escape'); + + expect(plugin.invokeHook).not.toHaveBeenCalled(); + expect(result.defaultPrevented).toBe(false); + expect(result.reachedApp).toBe(true); + }); + + it('runs an allowed shortcut without hiding the key from the rest of the app', async () => { + const plugin = instance('notes'); + disposers.push(registerShortcuts(plugin, [{ id: 'new', keys: 'Alt+N', label: 'New note' }])); + + const result = press('n', { altKey: true }); + + expect(plugin.invokeHook).toHaveBeenCalledWith('shortcut:new', []); + expect(result.reachedApp).toBe(true); + }); + + it('keeps the first plugin that claimed a combo', async () => { + const first = instance('first'); + const second = instance('second'); + disposers.push(registerShortcuts(first, [{ id: 'a', keys: 'Alt+J', label: 'a' }])); + disposers.push(registerShortcuts(second, [{ id: 'b', keys: 'Alt+J', label: 'b' }])); + + press('j', { altKey: true }); + + expect(first.invokeHook).toHaveBeenCalled(); + expect(second.invokeHook).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/plugin-sandbox/loader.ts b/lib/plugin-sandbox/loader.ts index a01b0f6a0..679661ae4 100644 --- a/lib/plugin-sandbox/loader.ts +++ b/lib/plugin-sandbox/loader.ts @@ -21,7 +21,7 @@ import { resolvePluginTier } from './tier'; import { register as registerActive, deregister as deregisterActive, all as allActiveEntries } from './registry'; import { cancelPluginDialogs } from './host-api'; import { registerShortcuts } from './shortcuts'; -import { mayOfferSlot, mayRegisterHook } from './permissions'; +import { mayOfferSlot, mayRegisterHook, pluginHasPermission } from './permissions'; // ─── Hook-bus lookup (one flat map for name → bus) ──────────── @@ -193,9 +193,16 @@ export async function loadSandboxedPlugin(plugin: InstalledPlugin): Promise unknown)); } - // Install plugin-declared keyboard shortcuts. - const shortcutDispose = registerShortcuts(bg, info.shortcuts ?? []); - hookDisposables.push({ dispose: shortcutDispose }); + // Install plugin-declared keyboard shortcuts. A shortcut sees every + // key press outside text fields, so it needs ui:keyboard like the hooks. + if ((info.shortcuts ?? []).length > 0) { + if (pluginHasPermission(plugin, 'ui:keyboard')) { + const shortcutDispose = registerShortcuts(bg, info.shortcuts ?? []); + hookDisposables.push({ dispose: shortcutDispose }); + } else { + console.warn(`[plugin-sandbox] Plugin "${plugin.id}" declares shortcuts without the ui:keyboard permission; not registered`); + } + } const slotOffers = info.slots.filter((offer) => { if (mayOfferSlot(plugin, offer.name)) return true; diff --git a/lib/plugin-sandbox/shortcuts.ts b/lib/plugin-sandbox/shortcuts.ts index 1b1408270..3e69ce5e7 100644 --- a/lib/plugin-sandbox/shortcuts.ts +++ b/lib/plugin-sandbox/shortcuts.ts @@ -49,6 +49,31 @@ function normaliseCombo(combo: string): NormalisedCombo | null { return { ctrl, shift, alt, meta, key }; } +// Keys a plugin may never bind: the app and the browser need them. +const RESERVED_KEYS = new Set(['escape', 'tab', 'enter', 'f1', 'f5', 'f11', 'f12', 'delete', 'backspace']); +// Ctrl/Cmd+Shift chords browsers use (reopen tab, new private window, +// devtools, …). +const BROWSER_SHIFT_CHORDS = new Set(['t', 'n', 'w', 'q', 'i', 'j', 'c', 'r', 'p', 'b', 'o', 'delete', 'tab', 'enter', 'k']); + +/** + * Whether a plugin may bind this combo. The app uses bare keys (j, k, c, /, + * Shift+G…) and Escape, and browsers own plain Ctrl/Cmd chords, so a plugin + * needs Alt, Ctrl/Cmd+Shift or Ctrl/Cmd+Alt. + */ +export function isAllowedPluginCombo(keys: string): boolean { + const combo = normaliseCombo(keys); + if (!combo) return false; + if (RESERVED_KEYS.has(combo.key)) return false; + const primary = combo.ctrl || combo.meta; + if (combo.alt) return true; + if (primary && combo.shift) return !BROWSER_SHIFT_CHORDS.has(combo.key); + return false; +} + +function comboId(combo: NormalisedCombo): string { + return `${combo.ctrl || combo.meta ? 'C' : ''}${combo.shift ? 'S' : ''}${combo.alt ? 'A' : ''}+${combo.key}`; +} + function eventMatches(ev: KeyboardEvent, combo: NormalisedCombo): boolean { if (combo.ctrl !== (ev.ctrlKey || ev.metaKey ? ev.ctrlKey : false)) { // Treat Ctrl and Cmd as equivalent: a binding declaring Ctrl matches a @@ -70,8 +95,8 @@ function onKeyDown(ev: KeyboardEvent): void { const combo = normaliseCombo(binding.keys); if (!combo) continue; if (eventMatches(ev, combo)) { + // Not stopPropagation: the app's own listeners still see the key. ev.preventDefault(); - ev.stopPropagation(); void binding.invoke(); return; } @@ -90,8 +115,22 @@ export function registerShortcuts( ): () => void { ensureListener(); const keys: string[] = []; + const taken = new Set([...bindings.values()].map((b) => { + const c = normaliseCombo(b.keys); + return c ? comboId(c) : ''; + })); for (const sc of shortcuts) { const key = `${instance.pluginId}:${sc.id}`; + const combo = normaliseCombo(sc.keys); + if (!combo || !isAllowedPluginCombo(sc.keys)) { + console.warn(`[plugin-shortcuts] "${instance.pluginId}" may not bind ${sc.keys}; use Alt, Ctrl+Shift or Ctrl+Alt`); + continue; + } + if (taken.has(comboId(combo))) { + console.warn(`[plugin-shortcuts] ${sc.keys} is already used by another plugin; "${instance.pluginId}" does not get it`); + continue; + } + taken.add(comboId(combo)); bindings.set(key, { pluginId: instance.pluginId, shortcutId: sc.id,