feat: let a checker that checks only when asked work with compose:read #1397

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' });

lib/__tests__/plugin-permission-coverage.test.ts

@@ -20,6 +20,7 @@ const root = path.resolve(__dirname, '../..');
  */
 const OTHER_GATES: Record<string, { file: string; call: string }> = {
   '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')" },
 };
 
 /**

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;
 }
 

lib/plugin-sandbox/host-api.ts

@@ -326,14 +326,15 @@ export const PERM_PER_METHOD: Readonly<Record<string, Permission | null>> = {
   '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) ──────

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.