diff --git a/lib/__tests__/plugin-host-checker.test.ts b/lib/__tests__/plugin-host-checker.test.ts index 968c83c98..eb7067123 100644 --- a/lib/__tests__/plugin-host-checker.test.ts +++ b/lib/__tests__/plugin-host-checker.test.ts @@ -31,14 +31,30 @@ beforeEach(() => resetPluginRateLimits()); afterEach(() => resetCheckers()); describe('compose:check', () => { - it('is a high-risk permission, and compose:read/write do not grant checking', async () => { + it('is a high-risk permission, and compose:read/write do not grant checking while typing', async () => { expect(HIGH_RISK_PERMISSIONS.has('compose:check')).toBe(true); - for (const method of ['composer.registerChecker', 'composer.setAnnotations', 'composer.getCheckText', 'composer.recheck']) { - await expect(call(READ_WRITE, method, [DATA])).rejects.toThrow('lacks permission "compose:check"'); + await expect(call(READ_WRITE, 'composer.registerChecker', [DATA])).rejects.toThrow('lacks permission "compose:check"'); + await expect(call(plugin([]), 'composer.registerChecker', [{ ...DATA, mode: 'on-demand' }])).rejects.toThrow('lacks permission'); + for (const method of ['composer.setAnnotations', 'composer.getCheckText', 'composer.recheck']) { + await expect(call(READ_WRITE, method, ['composer-1', 'lt'])).rejects.toThrow(); } expect(allCheckers()).toHaveLength(0); }); + it('lets a plugin with compose:read check when the user asks, never while typing', async () => { + await call(READ_WRITE, 'composer.registerChecker', [{ ...DATA, mode: 'on-demand' }]); + expect(getChecker('lt', 'lt')).toMatchObject({ mode: 'on-demand' }); + const target = { getCheckText: vi.fn(() => ({ revision: 'r1' })), setAnnotations: vi.fn(() => 1) }; + const unregister = registerCheckTarget('composer-1', target); + expect(await call(READ_WRITE, 'composer.getCheckText', ['composer-1', 'lt'])).toEqual({ revision: 'r1' }); + expect(await call(READ_WRITE, 'composer.setAnnotations', ['composer-1', 'lt', 'r1', []])).toBe(1); + unregister(); + // Re-registering as a typing checker needs compose:check. + await expect(call(READ_WRITE, 'composer.registerChecker', [{ ...DATA, mode: 'typing' }])).rejects.toThrow('compose:check'); + // A permission taken away stops the calls of a checker already registered. + await expect(call(plugin([]), 'composer.recheck', ['lt'])).rejects.toThrow('lacks permission'); + }); + it('registers a checker from the background frame, with defaults filled in', async () => { await call(CHECK, 'composer.registerChecker', [DATA]); expect(getChecker('lt', 'lt')).toMatchObject({ label: 'LanguageTool', kinds: ['spelling', 'grammar'], debounceMs: 800, mode: 'typing', warnBeforeSend: false, pluginName: 'LanguageTool' }); diff --git a/lib/__tests__/plugin-permission-coverage.test.ts b/lib/__tests__/plugin-permission-coverage.test.ts index 1ac16d526..adfa971ca 100644 --- a/lib/__tests__/plugin-permission-coverage.test.ts +++ b/lib/__tests__/plugin-permission-coverage.test.ts @@ -20,6 +20,7 @@ const root = path.resolve(__dirname, '../..'); */ const OTHER_GATES: Record = { 'ui:keyboard': { file: 'lib/plugin-sandbox/loader.ts', call: "pluginHasPermission(plugin, 'ui:keyboard')" }, + 'compose:check': { file: 'lib/plugin-sandbox/host-api-checker.ts', call: "pluginHasPermission(plugin, 'compose:check')" }, }; /** diff --git a/lib/plugin-sandbox/host-api-checker.ts b/lib/plugin-sandbox/host-api-checker.ts index 251132915..ae2bc49cf 100644 --- a/lib/plugin-sandbox/host-api-checker.ts +++ b/lib/plugin-sandbox/host-api-checker.ts @@ -1,4 +1,10 @@ -// Host side of the text checker calls (compose:check). See checkers.ts. +// Host side of the text checker calls. See checkers.ts. +// +// A checker the host sends text to while the user types needs compose:check: +// it reads every message being written, without the user pointing it at +// one. A checker that checks only when the user asks (an assistant's "check +// spelling" button, with the composer id that click hands over) needs no +// more than compose:read, like the plugin's other reads of that message. // // composer.registerChecker(data) background frame only // composer.updateChecker(checkerId, patch) @@ -13,6 +19,7 @@ import { removePluginChecker, setPluginChecker, updatePluginChecker, type CheckerSpec, } from './checkers'; import { readId, withinRate } from './host-api-input'; +import { pluginHasPermission } from './permissions'; const CALLS_PER_MINUTE = 60; @@ -20,9 +27,18 @@ function rate(method: string, plugin: InstalledPlugin): void { if (!withinRate('composer-check', plugin.id, CALLS_PER_MINUTE)) throw new Error(`${method}: at most ${CALLS_PER_MINUTE} calls a minute`); } +/** compose:check, or for a checker that checks only when asked, compose:read. */ +function requirePermission(method: string, plugin: InstalledPlugin, mode: 'typing' | 'on-demand'): void { + if (pluginHasPermission(plugin, 'compose:check')) return; + if (mode === 'on-demand' && pluginHasPermission(plugin, 'compose:read')) return; + throw new Error(`Plugin "${plugin.id}" lacks permission "compose:check"${mode === 'on-demand' ? ' or "compose:read"' : ''} for ${method}`); +} + function checkerOf(method: string, plugin: InstalledPlugin, checkerId: unknown): CheckerSpec { const spec = getChecker(plugin.id, checkerId); if (!spec) throw new Error(`${method}: no checker "${String(checkerId)}" registered by this plugin`); + // Checked again on every call, in case the permission went since. + requirePermission(method, plugin, spec.mode); return spec; } @@ -30,8 +46,10 @@ export function doRegisterChecker(plugin: InstalledPlugin, data: unknown, frame: // The host calls a checker in the plugin's background frame; a slot // frame comes and goes with the screen. if (frame === 'slot') throw new Error('composer.registerChecker: register checkers from activate(), not from a slot'); + const checked = checkCheckerData(data); + requirePermission('composer.registerChecker', plugin, checked.mode); rate('composer.registerChecker', plugin); - setPluginChecker(plugin.id, plugin.name, checkCheckerData(data)); + setPluginChecker(plugin.id, plugin.name, checked); return true; } diff --git a/lib/plugin-sandbox/host-api.ts b/lib/plugin-sandbox/host-api.ts index 20fe6cc7d..6b51b8c7f 100644 --- a/lib/plugin-sandbox/host-api.ts +++ b/lib/plugin-sandbox/host-api.ts @@ -326,14 +326,15 @@ export const PERM_PER_METHOD: Readonly> = { 'composer.replaceBody': 'compose:write', // Attach a file the plugin uploaded (jmap.uploadBlob). 'composer.attach': 'email:blob-write', - // Text checkers: the host sends what the user writes in every composer - // where checking is on (checkers.ts). - 'composer.registerChecker': 'compose:check', - 'composer.updateChecker': 'compose:check', - 'composer.recheck': 'compose:check', - 'composer.unregisterChecker': 'compose:check', - 'composer.getCheckText': 'compose:check', - 'composer.setAnnotations': 'compose:check', + // Text checkers (checkers.ts): compose:check for one the host sends text + // to while the user types, compose:read for one that checks when asked. + // host-api-checker.ts checks which, by the checker's mode. + 'composer.registerChecker': null, + 'composer.updateChecker': null, + 'composer.recheck': null, + 'composer.unregisterChecker': null, + 'composer.getCheckText': null, + 'composer.setAnnotations': null, }; // ─── Cross-origin allow-list (mirrors lib/plugin-api.ts) ────── diff --git a/lib/plugin-types.ts b/lib/plugin-types.ts index 1a77c0e55..f75078686 100644 --- a/lib/plugin-types.ts +++ b/lib/plugin-types.ts @@ -1559,6 +1559,7 @@ export const ALL_PERMISSIONS = [ // writes, without the user pointing it at a composer, and draws its // results as underlines. Only where the user (or the admin) turned // proofreading on, and never for a message that will be signed or encrypted. + // A checker that checks only when asked needs compose:read instead. 'compose:check', // Forward a message of the account with its attachments, without the // composer (api.email.forward). The host asks the user once per address.