fix(security): gate plugin hook registration on granted permissions
`info.hooks` is self-reported by the sandboxed bundle, and the loader registered any recognised hook name without checking permissions. An untrusted, null-origin plugin could therefore claim `onRenderEmailBody` and replace the rendered body of any opened email without ever holding `email:render-takeover` — the permission was enforced only by the one-time consent dialog, i.e. it gated what the user was *asked*, not what the host *allowed*. Add HOOK_PERMISSIONS covering the hooks that can read message content, alter outgoing mail, or observe key state: render takeover, the three send-interception hooks, bulk-content hooks, attachment upload, and the four S/MIME hooks. Hooks absent from the map stay unrestricted (UI observation, toasts, navigation), so ordinary plugins are unaffected. Refused hooks fail closed and log the missing permission by name — a silently inert hook is far harder to diagnose than a refused one. Export hasPermission() from host-api rather than reimplementing the rule in the loader, so the hook gate and the RPC gate cannot drift apart. Remaining ~200 hooks are tracked as B-09. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
f0e63de09b
commit
ae19ad888b
@@ -99,7 +99,12 @@ const PERM_PER_METHOD: Record<string, Permission | null> = {
|
|||||||
'sieve.regenerate': 'filters:write',
|
'sieve.regenerate': 'filters:write',
|
||||||
};
|
};
|
||||||
|
|
||||||
function hasPermission(plugin: InstalledPlugin, perm: Permission): boolean {
|
/**
|
||||||
|
* Single source of truth for "may this plugin use `perm`?". Exported so the
|
||||||
|
* loader can gate hook registration with the same rule the RPC layer uses -
|
||||||
|
* two copies of this logic would drift.
|
||||||
|
*/
|
||||||
|
export function hasPermission(plugin: InstalledPlugin, perm: Permission): boolean {
|
||||||
if ((IMPLICIT_PERMISSIONS as readonly string[]).includes(perm)) return true;
|
if ((IMPLICIT_PERMISSIONS as readonly string[]).includes(perm)) return true;
|
||||||
if (!plugin.permissions.includes(perm)) return false;
|
if (!plugin.permissions.includes(perm)) return false;
|
||||||
// Defense-in-depth: even if the manifest declares a permission, the host
|
// Defense-in-depth: even if the manifest declares a permission, the host
|
||||||
|
|||||||
@@ -18,8 +18,9 @@ import { verifyBundle } from './bundle-integrity';
|
|||||||
import { createBackgroundInstance } from './host-bridge';
|
import { createBackgroundInstance } from './host-bridge';
|
||||||
import { resolvePluginTier } from './tier';
|
import { resolvePluginTier } from './tier';
|
||||||
import { register as registerActive, deregister as deregisterActive, all as allActiveEntries } from './registry';
|
import { register as registerActive, deregister as deregisterActive, all as allActiveEntries } from './registry';
|
||||||
import { cancelPluginDialogs } from './host-api';
|
import { cancelPluginDialogs, hasPermission } from './host-api';
|
||||||
import { registerShortcuts } from './shortcuts';
|
import { registerShortcuts } from './shortcuts';
|
||||||
|
import type { Permission } from '../plugin-types';
|
||||||
|
|
||||||
// ─── Hook-bus lookup (one flat map for name → bus) ────────────
|
// ─── Hook-bus lookup (one flat map for name → bus) ────────────
|
||||||
|
|
||||||
@@ -35,6 +36,38 @@ const HOOK_BUSES: Record<string, AnyBus> = Object.assign({},
|
|||||||
messageListTabHooks,
|
messageListTabHooks,
|
||||||
) as Record<string, AnyBus>;
|
) as Record<string, AnyBus>;
|
||||||
|
|
||||||
|
// ─── Permission-gated hooks ───────────────────────────────────
|
||||||
|
//
|
||||||
|
// `info.hooks` is SELF-REPORTED by the sandboxed bundle, so registration must
|
||||||
|
// be checked against granted permissions - otherwise any untrusted plugin could
|
||||||
|
// claim a sensitive hook simply by naming it. Consent-dialog copy is not a
|
||||||
|
// substitute: it gates what the user was *asked*, not what the host *allows*.
|
||||||
|
//
|
||||||
|
// Listed here are the hooks that can read message content, alter outgoing mail,
|
||||||
|
// or observe key state. Hooks absent from this map are unrestricted (UI
|
||||||
|
// observation, navigation, toasts and similar) and register as before.
|
||||||
|
const HOOK_PERMISSIONS: Record<string, Permission> = {
|
||||||
|
// Render takeover - replaces the rendered body the user sees.
|
||||||
|
onRenderEmailBody: 'email:render-takeover',
|
||||||
|
onEmailListItemRender: 'email:read',
|
||||||
|
onEmailContentRender: 'email:read',
|
||||||
|
// Outgoing-mail interception: veto, mutate, or take over the send entirely.
|
||||||
|
onComposeSend: 'email:send',
|
||||||
|
onBeforeEmailSend: 'email:send',
|
||||||
|
onTransformOutgoingEmail: 'email:send',
|
||||||
|
// Bulk message content reaching the plugin.
|
||||||
|
onEmailsFetched: 'email:read',
|
||||||
|
onProvideSearchResults: 'email:read',
|
||||||
|
// Attachment bytes on the way up.
|
||||||
|
onBeforeBlobUpload: 'email:blob-write',
|
||||||
|
onBeforeAttachmentUpload: 'email:blob-write',
|
||||||
|
// S/MIME key + certificate state.
|
||||||
|
onSmimeKeyImport: 'smime:read',
|
||||||
|
onSmimeCertImport: 'smime:read',
|
||||||
|
onSmimeKeyStateChange: 'smime:read',
|
||||||
|
onSmimeDefaultsChange: 'smime:read',
|
||||||
|
};
|
||||||
|
|
||||||
// ─── Store accessor (status updates flow through the existing store) ──
|
// ─── Store accessor (status updates flow through the existing store) ──
|
||||||
|
|
||||||
type StoreAccessor = { setPluginStatus: (id: string, status: InstalledPlugin['status'], error?: string) => void };
|
type StoreAccessor = { setPluginStatus: (id: string, status: InstalledPlugin['status'], error?: string) => void };
|
||||||
@@ -128,6 +161,7 @@ export async function loadSandboxedPlugin(plugin: InstalledPlugin): Promise<void
|
|||||||
// entry whose handler dispatches into the sandbox. `shortcut:<id>` hooks
|
// entry whose handler dispatches into the sandbox. `shortcut:<id>` hooks
|
||||||
// are dispatched by the keyboard module separately and don't have a bus.
|
// are dispatched by the keyboard module separately and don't have a bus.
|
||||||
const hookDisposables: Disposable[] = [];
|
const hookDisposables: Disposable[] = [];
|
||||||
|
const refusedHooks: string[] = [];
|
||||||
for (const hookName of info.hooks) {
|
for (const hookName of info.hooks) {
|
||||||
if (hookName.startsWith('shortcut:')) continue;
|
if (hookName.startsWith('shortcut:')) continue;
|
||||||
const bus = HOOK_BUSES[hookName];
|
const bus = HOOK_BUSES[hookName];
|
||||||
@@ -135,6 +169,17 @@ export async function loadSandboxedPlugin(plugin: InstalledPlugin): Promise<void
|
|||||||
console.warn(`[plugin-sandbox] Plugin "${plugin.id}" registered unknown hook "${hookName}"`);
|
console.warn(`[plugin-sandbox] Plugin "${plugin.id}" registered unknown hook "${hookName}"`);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
// Refuse sensitive hooks the plugin has no permission for. Fail closed and
|
||||||
|
// say so loudly - a silently inert hook is far harder to diagnose than a
|
||||||
|
// refused one.
|
||||||
|
const required = HOOK_PERMISSIONS[hookName];
|
||||||
|
if (required && !hasPermission(plugin, required)) {
|
||||||
|
refusedHooks.push(`${hookName} (needs ${required})`);
|
||||||
|
console.error(
|
||||||
|
`[plugin-sandbox] "${plugin.id}" refused hook "${hookName}": missing permission "${required}"`,
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
const proxy = async (...args: unknown[]) => {
|
const proxy = async (...args: unknown[]) => {
|
||||||
try {
|
try {
|
||||||
return await bg.invokeHook(hookName, args);
|
return await bg.invokeHook(hookName, args);
|
||||||
@@ -160,7 +205,15 @@ export async function loadSandboxedPlugin(plugin: InstalledPlugin): Promise<void
|
|||||||
});
|
});
|
||||||
|
|
||||||
storeAccessor?.setPluginStatus(plugin.id, 'running');
|
storeAccessor?.setPluginStatus(plugin.id, 'running');
|
||||||
console.info(`[plugin-sandbox] "${plugin.id}" activated (hooks=${info.hooks.length}, slots=${info.slots.length})`);
|
console.info(
|
||||||
|
`[plugin-sandbox] "${plugin.id}" activated (hooks=${info.hooks.length}, slots=${info.slots.length}`
|
||||||
|
+ `${refusedHooks.length > 0 ? `, refused=${refusedHooks.length}` : ''})`,
|
||||||
|
);
|
||||||
|
if (refusedHooks.length > 0) {
|
||||||
|
console.warn(
|
||||||
|
`[plugin-sandbox] "${plugin.id}" ran without ${refusedHooks.length} hook(s): ${refusedHooks.join(', ')}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const msg = (err as Error).message ?? String(err);
|
const msg = (err as Error).message ?? String(err);
|
||||||
storeAccessor?.setPluginStatus(plugin.id, 'error', msg);
|
storeAccessor?.setPluginStatus(plugin.id, 'error', msg);
|
||||||
|
|||||||
Reference in New Issue
Block a user