diff --git a/docs/plugin-development.md b/docs/plugin-development.md
index 47621514b8..85e42e4685 100644
--- a/docs/plugin-development.md
+++ b/docs/plugin-development.md
@@ -552,10 +552,24 @@ console.log(data); // '{ count: 42 }'
Plugins with `"permissions": ["nodeExecution"]` can run Node.js scripts in the Electron
desktop app after the user allows the desktop permission prompt.
-The desktop grant is currently issued only for packaged built-in plugins whose
-manifest can be verified by the main process. Uploaded plugins that request
-`nodeExecution` are rejected until uploaded plugin installation is moved to a
-main-process-owned verification path.
+Both built-in and uploaded (community) plugins may request `nodeExecution`. The grant is
+issued by the Electron **main** process after a native consent dialog, and is bound to
+the plugin id for the current app session (it is never persisted or synced). For uploaded
+plugins the app cannot verify the manifest, so the dialog flags the plugin as unverified
+third-party code with full machine access that Super Productivity cannot sandbox, and
+defaults to **Deny** — only allow plugins whose source you trust. If the user denies,
+the plugin stays enabled but its node calls fail until it is re-enabled or the app is
+restarted (consent is re-requested once per session).
+
+> **Plugin id constraints (for `nodeExecution`):** the consent grant keys on your
+> manifest `id`, so it must be a single safe token — no whitespace, control/bidi
+> characters, `:`, path separators (`/`, `\`), and at most 100 characters. Lowercase
+> kebab-case is recommended; dots and uppercase are accepted.
+
+> **Security note:** a granted `nodeExecution` plugin can run any program with full
+> access to your files and system. The file/IPC channel a plugin uses to talk to a
+> companion process is an open local channel — treat any data it reads as untrusted
+> input (never `eval`/`require` its contents).
```javascript
const result = await plugin.executeNodeScript({
diff --git a/electron/bundled-plugin-ids.test.cjs b/electron/bundled-plugin-ids.test.cjs
new file mode 100644
index 0000000000..5e4be6b686
--- /dev/null
+++ b/electron/bundled-plugin-ids.test.cjs
@@ -0,0 +1,149 @@
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+// SECURITY INVARIANT (cross-layer regression guard)
+// --------------------------------------------------
+// `src/app/plugins/plugin.service.ts` declares two top-of-file lists:
+// - BUNDLED_PLUGIN_PATHS: the on-disk asset dirs of the plugins we ship.
+// - BUNDLED_PLUGIN_IDS: the reserved set of *manifest ids* that an uploaded
+// plugin is forbidden from claiming.
+// The renderer rejects an uploaded plugin whose manifest id is in
+// BUNDLED_PLUGIN_IDS so unverified code cannot impersonate a built-in. With
+// nodeExecution now openable to uploaded plugins, an unguarded id would also
+// let an upload borrow a bundled dir's "verified built-in" consent dialog in
+// the main process. PATHS are keyed by on-disk dir name; IDS by manifest id;
+// the dir->id mapping is NOT identity (e.g. dir `yesterday-tasks-plugin` has
+// manifest id `yesterday-tasks`). The two lists already drifted once (gitea /
+// linear / trello / azure issue-providers were in PATHS but missing from IDS),
+// which silently opened the impersonation gap for those ids.
+//
+// This test enforces PATHS ⊆ IDS: every bundled plugin's real manifest id MUST
+// be reserved. It deliberately does NOT require equality — IDS may reserve ids
+// for plugins not currently shipped via PATHS (e.g. `ai-productivity-prompts`),
+// which only widens the reserved set and is harmless.
+//
+// A Karma/browser unit test cannot read the filesystem, and importing
+// plugin.service.ts would drag in the whole Angular DI graph. So we parse the
+// file as text here, in the filesystem-capable `node --test` (electron) suite,
+// and read manifests straight off disk.
+
+const REPO_ROOT = path.resolve(__dirname, '..');
+const PLUGIN_SERVICE_PATH = path.join(
+ REPO_ROOT,
+ 'src/app/plugins/plugin.service.ts',
+);
+const PLUGIN_DEV_DIR = path.join(REPO_ROOT, 'packages/plugin-dev');
+
+/**
+ * Extract a named array/Set literal's quoted string entries from the source
+ * text. We don't evaluate the file (no Angular import) — we slice the literal
+ * between its opening token and the matching close bracket, then pull every
+ * single/double-quoted string out of that slice. This stays robust to
+ * formatting (line breaks, trailing commas, `as const`) without executing code.
+ *
+ * @param {string} source full plugin.service.ts text
+ * @param {string} declStart the literal's opening, e.g. `BUNDLED_PLUGIN_PATHS = [`
+ * @param {string} closeChar the matching close bracket, `]` or `)`
+ * @returns {string[]} the quoted entries, in source order
+ */
+const extractStringLiteralList = (source, declStart, closeChar) => {
+ const startIdx = source.indexOf(declStart);
+ assert.notEqual(
+ startIdx,
+ -1,
+ `Could not find "${declStart}" in plugin.service.ts — the const may have been renamed; update this regression test.`,
+ );
+ const contentStart = startIdx + declStart.length;
+ const closeIdx = source.indexOf(closeChar, contentStart);
+ assert.notEqual(
+ closeIdx,
+ -1,
+ `Could not find closing "${closeChar}" for "${declStart}" in plugin.service.ts.`,
+ );
+ const slice = source.slice(contentStart, closeIdx);
+ const matches = slice.match(/['"]([^'"]+)['"]/g) || [];
+ return matches.map((m) => m.slice(1, -1));
+};
+
+/**
+ * Locate a bundled plugin's manifest. Both layouts exist in the repo:
+ * packages/plugin-dev/
/manifest.json
+ * packages/plugin-dev//src/manifest.json
+ * Returns the first that exists, or null if neither does.
+ */
+const findManifestPath = (dirName) => {
+ const candidates = [
+ path.join(PLUGIN_DEV_DIR, dirName, 'manifest.json'),
+ path.join(PLUGIN_DEV_DIR, dirName, 'src', 'manifest.json'),
+ ];
+ return candidates.find((p) => fs.existsSync(p)) || null;
+};
+
+test('every BUNDLED_PLUGIN_PATHS plugin has its manifest id reserved in BUNDLED_PLUGIN_IDS', () => {
+ const source = fs.readFileSync(PLUGIN_SERVICE_PATH, 'utf8');
+
+ const bundledPaths = extractStringLiteralList(
+ source,
+ 'BUNDLED_PLUGIN_PATHS = [',
+ ']',
+ );
+ const bundledIds = new Set(
+ extractStringLiteralList(source, 'BUNDLED_PLUGIN_IDS = new Set([', ']'),
+ );
+
+ // Sanity: if either list parsed empty, the source format changed and the
+ // guard is silently inert — fail loudly rather than pass vacuously.
+ assert.ok(
+ bundledPaths.length > 0,
+ 'Parsed zero entries from BUNDLED_PLUGIN_PATHS — the source format likely changed; update this test.',
+ );
+ assert.ok(
+ bundledIds.size > 0,
+ 'Parsed zero entries from BUNDLED_PLUGIN_IDS — the source format likely changed; update this test.',
+ );
+
+ const missingIds = [];
+ const missingManifests = [];
+
+ for (const assetPath of bundledPaths) {
+ const dirName = assetPath.split('/').pop();
+ const manifestPath = findManifestPath(dirName);
+
+ if (!manifestPath) {
+ missingManifests.push(dirName);
+ continue;
+ }
+
+ const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8'));
+ const id = manifest.id;
+ assert.ok(
+ typeof id === 'string' && id.length > 0,
+ `Manifest for "${dirName}" has no string "id" field (${manifestPath}).`,
+ );
+
+ if (!bundledIds.has(id)) {
+ missingIds.push(`${dirName} -> "${id}"`);
+ }
+ }
+
+ // A missing manifest for a listed dir is itself drift/misconfig.
+ assert.equal(
+ missingManifests.length,
+ 0,
+ `No manifest.json (top-level or src/) found under packages/plugin-dev for bundled plugin dir(s): ${missingManifests.join(
+ ', ',
+ )}. Each BUNDLED_PLUGIN_PATHS entry must have a manifest so its id can be verified.`,
+ );
+
+ // The core invariant. List ALL offenders, not just the first.
+ assert.equal(
+ missingIds.length,
+ 0,
+ `SECURITY: the following bundled plugins' manifest ids are NOT reserved in ` +
+ `BUNDLED_PLUGIN_IDS (src/app/plugins/plugin.service.ts). An uploaded plugin ` +
+ `could claim these ids and impersonate a built-in. Add each missing id to ` +
+ `BUNDLED_PLUGIN_IDS:\n ${missingIds.join('\n ')}`,
+ );
+});
diff --git a/electron/electronAPI.d.ts b/electron/electronAPI.d.ts
index 5053798c19..10c9a28932 100644
--- a/electron/electronAPI.d.ts
+++ b/electron/electronAPI.d.ts
@@ -10,26 +10,13 @@ import { AppDataCompleteLegacy } from '../src/app/imex/sync/sync.model';
import { Task } from '../src/app/features/tasks/task.model';
import { LocalBackupMeta } from '../src/app/imex/local-backup/local-backup.model';
import { AppDataComplete } from '../src/app/op-log/model/model-config';
-import {
- PluginNodeScriptRequest,
- PluginNodeScriptResult,
-} from '../packages/plugin-api/src/types';
+import { PluginNodeExecutionElectronApi } from './shared-with-frontend/plugin-node-execution.model';
import {
LocalRestApiRequestPayload,
LocalRestApiResponsePayload,
} from './shared-with-frontend/local-rest-api.model';
import { ElectronDistChannel } from './shared-with-frontend/get-dist-channel';
-export interface PluginNodeExecutionElectronApi {
- requestGrant(pluginId: string): Promise<{ token: string } | null>;
- executeScript(
- pluginId: string,
- grantToken: string,
- request: PluginNodeScriptRequest,
- ): Promise;
- revokeGrant(pluginId: string, grantToken: string): Promise;
-}
-
export interface ElectronAPI {
on(
channel: string,
diff --git a/electron/plugin-node-executor.test.cjs b/electron/plugin-node-executor.test.cjs
index 883aafdba5..0d31ba351b 100644
--- a/electron/plugin-node-executor.test.cjs
+++ b/electron/plugin-node-executor.test.cjs
@@ -221,7 +221,7 @@ test('does not mint a token when the sender navigates while consent is pending',
assert.equal(await grantPromise, null);
});
-test('revoke requires the issuing webContents and token', async () => {
+test('revoke is scoped to the issuing webContents', async () => {
loadModule();
const webContents = new FakeWebContents(6);
const otherWebContents = new FakeWebContents(7);
@@ -255,3 +255,226 @@ test('revoke requires the issuing webContents and token', async () => {
/not authorized/,
);
});
+
+test('issues a grant to an uploaded (non-bundled) plugin and labels it unverified', async () => {
+ loadModule();
+ const webContents = new FakeWebContents(10);
+
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ webContents,
+ 'uploaded-node-plugin',
+ { name: 'Uploaded Node Plugin', version: '1.2.3' },
+ );
+
+ assert.equal(typeof grant.token, 'string');
+ assert.equal(dialogCalls.length, 1);
+ const opts = dialogCalls[0][1];
+ // Dialog anchors on the validated id and flags the self-declared name as unverified,
+ // and defaults to Deny.
+ assert.match(opts.detail, /Plugin ID: uploaded-node-plugin/);
+ assert.match(opts.detail, /self-declared, unverified/);
+ assert.equal(opts.defaultId, 1);
+});
+
+test('uploaded plugin executes after grant; a denied request leaves exec unauthorized', async () => {
+ loadModule();
+ const allowWc = new FakeWebContents(11);
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ allowWc,
+ 'uploaded-node-plugin',
+ { name: 'Uploaded', version: '1.0.0' },
+ );
+ const okResult = await callIpc(
+ 'PLUGIN_EXEC_NODE_SCRIPT',
+ allowWc,
+ 'uploaded-node-plugin',
+ grant.token,
+ { script: 'return args[0] + 1;', args: [4] },
+ );
+ assert.equal(okResult.success, true);
+ assert.equal(okResult.result, 5);
+
+ // A denied request mints no token, so exec stays unauthorized.
+ nextDialogResult = { response: 1 };
+ const denyWc = new FakeWebContents(12);
+ const denied = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ denyWc,
+ 'denied-plugin',
+ { name: 'Denied', version: '1.0.0' },
+ );
+ assert.equal(denied, null);
+ await assert.rejects(
+ () =>
+ callIpc('PLUGIN_EXEC_NODE_SCRIPT', denyWc, 'denied-plugin', 'made-up-token', {
+ script: 'return true;',
+ }),
+ /not authorized/,
+ );
+});
+
+test('accepts a non-kebab uploaded id (dots/uppercase) the built-in rule would reject', async () => {
+ loadModule();
+ const webContents = new FakeWebContents(13);
+
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ webContents,
+ 'Community.Plugin-2',
+ { name: 'Community Plugin', version: '2.0.0' },
+ );
+
+ assert.equal(typeof grant.token, 'string');
+});
+
+test('rejects unsafe ids and sanitizes self-declared display strings', async () => {
+ loadModule();
+ const webContents = new FakeWebContents(14);
+
+ // A newline in the id is rejected outright (it is a grant Map key + dialog text).
+ await assert.rejects(
+ () =>
+ callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', webContents, 'evil\nid', {
+ name: 'x',
+ version: '1',
+ }),
+ /Invalid pluginId/,
+ );
+
+ // Path separators / dot-segments are rejected (the id is used as a path component in
+ // the bundled-manifest existsSync probe).
+ for (const badId of ['../../etc', '..', '.', 'a/b', 'a\\b']) {
+ await assert.rejects(
+ () =>
+ callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', webContents, badId, {
+ name: 'x',
+ version: '1',
+ }),
+ /Invalid pluginId/,
+ );
+ }
+
+ // A crafted name cannot inject an extra dialog line and is length-capped.
+ const craftedName = `${'A'.repeat(500)}\nVerified by Super Productivity`;
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ webContents,
+ 'crafted-name-plugin',
+ { name: craftedName, version: '1.0.0' },
+ );
+ assert.equal(typeof grant.token, 'string');
+ const detail = dialogCalls[dialogCalls.length - 1][1].detail;
+ assert.equal(detail.includes('Verified by Super Productivity'), false);
+ assert.ok(detail.includes('…'));
+});
+
+test('revoke from the issuing webContents drops the grant even without the token', async () => {
+ loadModule();
+ const webContents = new FakeWebContents(15);
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ webContents,
+ 'uploaded-node-plugin',
+ { name: 'Uploaded', version: '1.0.0' },
+ );
+
+ // Teardown/re-upload revokes by id without resupplying the token, so a re-uploaded
+ // plugin reusing the id cannot inherit this live grant.
+ await callIpc(
+ 'PLUGIN_REVOKE_NODE_EXECUTION_GRANT',
+ webContents,
+ 'uploaded-node-plugin',
+ '',
+ );
+ await assert.rejects(
+ () =>
+ callIpc(
+ 'PLUGIN_EXEC_NODE_SCRIPT',
+ webContents,
+ 'uploaded-node-plugin',
+ grant.token,
+ {
+ script: 'return true;',
+ },
+ ),
+ /not authorized/,
+ );
+});
+
+test('rejects bidi/zero-width/homoglyph and leading-dot ids the allowlist must exclude', async () => {
+ loadModule();
+ const webContents = new FakeWebContents(16);
+
+ // These are exactly the dialog-anchor spoofing vectors a denylist of explicit Unicode
+ // ranges tends to miss; the allowlist rejects every non-[A-Za-z0-9._-] id by construction.
+ const badIds = [
+ `sync${String.fromCodePoint(0x061c)}md`, // U+061C ARABIC LETTER MARK (bidi)
+ `sync${String.fromCodePoint(0x2060)}md`, // U+2060 WORD JOINER (zero-width)
+ `sync${String.fromCodePoint(0x3164)}md`, // U+3164 HANGUL FILLER (invisible)
+ `${String.fromCodePoint(0xff53)}ync-md`, // U+FF53 fullwidth 's' (homoglyph)
+ '.hidden', // leading dot-segment
+ '-leading-dash',
+ 'a b', // whitespace
+ ];
+
+ for (const badId of badIds) {
+ await assert.rejects(
+ () =>
+ callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', webContents, badId, {
+ name: 'x',
+ version: '1',
+ }),
+ /Invalid pluginId/,
+ `expected ${JSON.stringify(badId)} to be rejected`,
+ );
+ }
+});
+
+test('strips bidi/zero-width chars from self-declared display strings', async () => {
+ loadModule();
+ const webContents = new FakeWebContents(17);
+
+ const name = `Tru${String.fromCodePoint(0x061c)}sted${String.fromCodePoint(0x2060)} Plugin`;
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ webContents,
+ 'display-sanitize-plugin',
+ { name, version: `1.0${String.fromCodePoint(0xfeff)}.0` },
+ );
+
+ assert.equal(typeof grant.token, 'string');
+ const detail = dialogCalls[dialogCalls.length - 1][1].detail;
+ // The control/format chars are gone; the visible text survives.
+ assert.equal(detail.includes(String.fromCodePoint(0x061c)), false);
+ assert.equal(detail.includes(String.fromCodePoint(0x2060)), false);
+ assert.equal(detail.includes(String.fromCodePoint(0xfeff)), false);
+ assert.match(detail, /Trusted Plugin/);
+});
+
+test('never upgrades trust: an on-disk match that does not cleanly verify uses the unverified dialog', async () => {
+ // Simulate a bundled dir whose manifest exists but is not a grantable nodeExecution
+ // built-in (here: missing the permission). The verified-built-in branch must return
+ // null and fall back to the unverified-uploaded dialog rather than throw or upgrade.
+ const originalPermissions = BUILT_IN_PLUGIN_MANIFEST.permissions;
+ BUILT_IN_PLUGIN_MANIFEST.permissions = [];
+ try {
+ loadModule();
+ const webContents = new FakeWebContents(18);
+ const grant = await callIpc(
+ 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+ webContents,
+ BUILT_IN_PLUGIN_MANIFEST.id,
+ { name: 'Impersonator', version: '9.9.9' },
+ );
+
+ assert.equal(typeof grant.token, 'string');
+ const opts = dialogCalls[dialogCalls.length - 1][1];
+ assert.match(opts.title, /run code on your machine/);
+ assert.match(opts.detail, /self-declared, unverified/);
+ assert.equal(opts.defaultId, 1);
+ } finally {
+ BUILT_IN_PLUGIN_MANIFEST.permissions = originalPermissions;
+ }
+});
diff --git a/electron/plugin-node-executor.ts b/electron/plugin-node-executor.ts
index 0c1502af83..a623b6d98e 100644
--- a/electron/plugin-node-executor.ts
+++ b/electron/plugin-node-executor.ts
@@ -16,11 +16,72 @@ const DEFAULT_TIMEOUT = 30000; // 30 seconds
const MAX_TIMEOUT = 300000; // 5 minutes
const BUILT_IN_PLUGIN_ID_RE = /^[a-z0-9][a-z0-9-]*$/;
+// An uploaded (community) plugin id is attacker-controlled and used both as a grant Map
+// key and as the consent dialog's trust anchor ("Plugin ID: ..."), and as a path segment
+// in getBuiltInManifestPath(). It is NOT held to the strict built-in kebab rule —
+// community ids may use dots/uppercase, e.g. `super-productivity-mcp` — but it must be a
+// single safe ASCII token. We use an allowlist rather than a denylist on purpose: the
+// allowlist rejects control/zero-width/bidi/homoglyph characters that could spoof the
+// dialog, whitespace that could inject extra dialog lines, the ':' persistence delimiter,
+// and path separators / leading-dot segments ('.', '..', '/', '\\') — all by construction,
+// with no Unicode range to keep updated as new code points are assigned.
+const MAX_UPLOADED_PLUGIN_ID_LENGTH = 100;
+const SAFE_UPLOADED_PLUGIN_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/;
+// Self-declared name/version are display-only. Strip every Unicode control (Cc) and
+// format (Cf) character — this covers C0/C1 controls, all zero-width characters, the BOM,
+// and every bidi control (incl. U+061C ALM, the word-joiner range, and the isolate marks)
+// without enumerating ranges — then collapse whitespace so a crafted value cannot inject
+// extra dialog lines. (Global flag is for replace, not test, so no lastIndex statefulness.)
+const UNSAFE_DISPLAY_CHARS_RE = /[\p{Cc}\p{Cf}]/gu;
+
+const assertSafePluginId = (pluginId: unknown): string => {
+ if (typeof pluginId !== 'string' || pluginId.length === 0) {
+ throw new Error('Invalid pluginId');
+ }
+ if (pluginId.length > MAX_UPLOADED_PLUGIN_ID_LENGTH) {
+ throw new Error('Invalid pluginId');
+ }
+ // Allowlist match also rejects path separators ('/'/'\\'), leading-dot segments
+ // ('.', '..'), ':', whitespace and all non-ASCII (bidi/zero-width/homoglyph), so the id
+ // can neither escape the bundled-plugins dir in getBuiltInManifestPath() nor spoof the
+ // consent dialog's trust anchor.
+ if (!SAFE_UPLOADED_PLUGIN_ID_RE.test(pluginId)) {
+ throw new Error('Invalid pluginId');
+ }
+ return pluginId;
+};
+
+const sanitizeDialogString = (value: unknown, maxLength: number): string => {
+ if (typeof value !== 'string') {
+ return '';
+ }
+ const cleaned = value.replace(UNSAFE_DISPLAY_CHARS_RE, '').replace(/\s+/g, ' ').trim();
+ return cleaned.length > maxLength ? `${cleaned.slice(0, maxLength)}…` : cleaned;
+};
+
+// Shared shell for both nodeExecution consent dialogs: a warning with Allow/Deny where
+// Deny is the default + cancel action, so a reflexive Enter/Escape denies.
+const NODE_CONSENT_DIALOG_BASE: Pick<
+ Electron.MessageBoxOptions,
+ 'type' | 'buttons' | 'defaultId' | 'cancelId'
+> = {
+ type: 'warning',
+ buttons: ['Allow', 'Deny'],
+ defaultId: 1,
+ cancelId: 1,
+};
+
interface NodeExecutionGrant {
token: string;
webContentsId: number;
}
+/** Self-declared, unverified display metadata supplied by the renderer for uploaded plugins. */
+interface NodeExecutionGrantDisplayInfo {
+ name?: string;
+ version?: string;
+}
+
interface WebContentsGrantCleanup {
webContents: WebContents;
cleanup: () => void;
@@ -48,42 +109,42 @@ class PluginNodeExecutor {
private setupIpcHandler(): void {
ipcMain.handle(
IPC.PLUGIN_REQUEST_NODE_EXECUTION_GRANT,
- async (event, pluginId: string) => {
+ async (event, pluginId: string, displayInfo?: NodeExecutionGrantDisplayInfo) => {
const window = BrowserWindow.fromWebContents(event.sender);
if (!window) {
throw new Error('No window found for event sender');
}
+ // Sanitize first: the id is used as a grant Map key AND shown in the consent
+ // dialog, and for uploaded plugins it is attacker-controlled.
+ const safeId = assertSafePluginId(pluginId);
+
const webContentsId = event.sender.id;
- const existingGrant = this.grants.get(pluginId);
+ const existingGrant = this.grants.get(safeId);
if (existingGrant) {
if (existingGrant.webContentsId === webContentsId) {
return { token: existingGrant.token };
}
- this.grants.delete(pluginId);
+ this.grants.delete(safeId);
this.releaseGrantCleanupIfUnused(existingGrant.webContentsId);
}
- const manifest = this.getVerifiedBuiltInNodeExecutionManifest(pluginId);
+ // Bundled vs uploaded is decided by the main-owned filesystem, never by a
+ // renderer-supplied flag, and only an id that resolves to a cleanly-verified
+ // on-disk manifest gets the trusted built-in dialog. A partial or colliding match
+ // (id mismatch, missing nodeExecution permission, unreadable manifest) returns
+ // null and falls back to the unverified dialog, so uploaded code can never borrow
+ // a built-in plugin's trusted name even if its id collides with a bundled dir.
+ const dialogOptions =
+ this.describeVerifiedBuiltInDialog(safeId) ??
+ this.describeUnverifiedUploadedDialog(safeId, displayInfo);
+
const requestUrl = event.sender.getURL();
this.registerGrantCleanup(event.sender);
- let result;
+ let result: Electron.MessageBoxReturnValue;
try {
- result = await dialog.showMessageBox(window, {
- type: 'warning',
- buttons: ['Allow', 'Deny'],
- defaultId: 1,
- cancelId: 1,
- title: 'Allow plugin Node.js execution?',
- message: `Allow "${manifest.name}" to run Node.js scripts?`,
- detail: [
- `Plugin ID: ${pluginId}`,
- `Version: ${manifest.version}`,
- '',
- 'This permission is valid for the current app session. Node.js execution can access local files and desktop APIs. Only allow plugins you trust.',
- ].join('\n'),
- });
+ result = await dialog.showMessageBox(window, dialogOptions);
} catch (error) {
this.releaseGrantCleanupIfUnused(webContentsId);
throw error;
@@ -94,19 +155,19 @@ class PluginNodeExecutor {
!this.grantCleanupByWebContents.has(webContentsId) ||
event.sender.getURL() !== requestUrl
) {
- this.grants.delete(pluginId);
+ this.grants.delete(safeId);
this.releaseGrantCleanupIfUnused(webContentsId);
return null;
}
if (result.response !== 0) {
- this.grants.delete(pluginId);
+ this.grants.delete(safeId);
this.releaseGrantCleanupIfUnused(webContentsId);
return null;
}
const token = randomBytes(32).toString('base64url');
- this.grants.set(pluginId, {
+ this.grants.set(safeId, {
token,
webContentsId,
});
@@ -116,10 +177,25 @@ class PluginNodeExecutor {
ipcMain.handle(
IPC.PLUGIN_REVOKE_NODE_EXECUTION_GRANT,
- (event, pluginId: string, grantToken: string) => {
- const grant = this.grants.get(pluginId);
- if (grant?.token === grantToken && grant.webContentsId === event.sender.id) {
- this.grants.delete(pluginId);
+ // grantToken is accepted for signature compatibility but intentionally not
+ // required: revoking only removes a capability, and the issuing window must be
+ // able to drop its own grant during teardown even if it no longer holds the
+ // token (e.g. on re-upload) — otherwise a re-uploaded plugin reusing the id
+ // could inherit a live session grant. The webContents binding still prevents
+ // another window from revoking this one's grant.
+ (event, pluginId: string, _grantToken?: string) => {
+ // Key the lookup through the same validator the request handler uses, so the
+ // "always revoke by id on teardown/re-upload" guarantee holds even if the id
+ // canonicalisation ever changes (an unsafe id can never hold a grant anyway).
+ let safeId: string;
+ try {
+ safeId = assertSafePluginId(pluginId);
+ } catch {
+ return;
+ }
+ const grant = this.grants.get(safeId);
+ if (grant && grant.webContentsId === event.sender.id) {
+ this.grants.delete(safeId);
}
},
);
@@ -137,7 +213,15 @@ class PluginNodeExecutor {
throw new Error('No window found for event sender');
}
- const grant = this.grants.get(pluginId);
+ // Validate the id the same way the grant handler does so the Map keys match.
+ // An unsafe id can never hold a grant, so treat it as unauthorized.
+ let safeId: string;
+ try {
+ safeId = assertSafePluginId(pluginId);
+ } catch {
+ throw new Error('Plugin is not authorized for nodeExecution');
+ }
+ const grant = this.grants.get(safeId);
if (
!grant ||
grant.token !== grantToken ||
@@ -146,7 +230,7 @@ class PluginNodeExecutor {
throw new Error('Plugin is not authorized for nodeExecution');
}
- return await this.executeScript(pluginId, request);
+ return await this.executeScript(safeId, request);
},
);
}
@@ -213,6 +297,64 @@ class PluginNodeExecutor {
this.unregisterGrantCleanup(webContentsId);
}
+ /**
+ * Consent dialog for a verified built-in plugin (name/version read from disk).
+ * Returns null when the id does not resolve to a cleanly-verified built-in
+ * nodeExecution manifest (no on-disk match, id mismatch, missing permission, or
+ * unreadable/invalid manifest), so the caller falls back to the unverified-uploaded
+ * dialog — a partial or colliding match must never *upgrade* trust to the built-in
+ * dialog.
+ */
+ private describeVerifiedBuiltInDialog(
+ pluginId: string,
+ ): Electron.MessageBoxOptions | null {
+ let manifest: PluginManifest;
+ try {
+ manifest = this.getVerifiedBuiltInNodeExecutionManifest(pluginId);
+ } catch {
+ return null;
+ }
+ return {
+ ...NODE_CONSENT_DIALOG_BASE,
+ title: 'Allow plugin Node.js execution?',
+ message: `Allow "${manifest.name}" to run Node.js scripts?`,
+ detail: [
+ `Plugin ID: ${pluginId}`,
+ `Version: ${manifest.version}`,
+ '',
+ 'This permission is valid for the current app session. Node.js execution can access local files and desktop APIs. Only allow plugins you trust.',
+ ].join('\n'),
+ };
+ }
+
+ /**
+ * Consent dialog for an uploaded (community) plugin. The app cannot verify an
+ * uploaded plugin's identity, so the dialog anchors on the validated id and marks
+ * the renderer-supplied name/version as self-declared/unverified. Default = Deny.
+ */
+ private describeUnverifiedUploadedDialog(
+ pluginId: string,
+ displayInfo?: NodeExecutionGrantDisplayInfo,
+ ): Electron.MessageBoxOptions {
+ const name = sanitizeDialogString(displayInfo?.name, 80) || '(unnamed)';
+ const version = sanitizeDialogString(displayInfo?.version, 32) || '(unknown)';
+ return {
+ ...NODE_CONSENT_DIALOG_BASE,
+ title: 'Allow this plugin to run code on your machine?',
+ message: `Plugin "${pluginId}" wants to run Node.js code`,
+ detail: [
+ `Plugin ID: ${pluginId}`,
+ `Name (self-declared, unverified): ${name}`,
+ `Version (self-declared): ${version}`,
+ '',
+ 'This is a third-party plugin. Super Productivity cannot verify its identity and cannot sandbox it.',
+ 'If you allow it, the plugin can run any program with full access to your files and system for this app session.',
+ '',
+ 'Only allow this if you trust the source of this plugin.',
+ ].join('\n'),
+ };
+ }
+
private getVerifiedBuiltInNodeExecutionManifest(pluginId: string): PluginManifest {
if (typeof pluginId !== 'string' || !pluginId) {
throw new Error('Invalid pluginId');
diff --git a/electron/preload.ts b/electron/preload.ts
index b034803142..0c9cfa6c30 100644
--- a/electron/preload.ts
+++ b/electron/preload.ts
@@ -245,8 +245,11 @@ const ea: ElectronAPI = {
}
pluginNodeExecutionApiConsumed = true;
return {
- requestGrant: (pluginId: string) =>
- _invoke('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', pluginId) as Promise<{
+ requestGrant: (
+ pluginId: string,
+ displayInfo?: { name?: string; version?: string },
+ ) =>
+ _invoke('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', pluginId, displayInfo) as Promise<{
token: string;
} | null>,
executeScript: (
diff --git a/electron/shared-with-frontend/plugin-node-execution.model.ts b/electron/shared-with-frontend/plugin-node-execution.model.ts
new file mode 100644
index 0000000000..c3a9d91558
--- /dev/null
+++ b/electron/shared-with-frontend/plugin-node-execution.model.ts
@@ -0,0 +1,24 @@
+import {
+ PluginNodeScriptRequest,
+ PluginNodeScriptResult,
+} from '../../packages/plugin-api/src/types';
+
+/**
+ * Shape of the Electron main-process bridge the renderer uses to grant, run, and
+ * revoke Node script execution for a plugin. This is a host-internal IPC contract
+ * (not part of the public plugin API), shared between the renderer
+ * (`plugin-bridge.service.ts`) and the Electron API typing
+ * (`electron/electronAPI.d.ts`) so the two cannot drift.
+ */
+export interface PluginNodeExecutionElectronApi {
+ requestGrant(
+ pluginId: string,
+ displayInfo?: { name?: string; version?: string },
+ ): Promise<{ token: string } | null>;
+ executeScript(
+ pluginId: string,
+ grantToken: string,
+ request: PluginNodeScriptRequest,
+ ): Promise;
+ revokeGrant(pluginId: string, grantToken: string): Promise;
+}
diff --git a/src/app/plugins/plugin-bridge.service.ts b/src/app/plugins/plugin-bridge.service.ts
index ade1b59bbf..fe69d1e421 100644
--- a/src/app/plugins/plugin-bridge.service.ts
+++ b/src/app/plugins/plugin-bridge.service.ts
@@ -117,16 +117,7 @@ import {
} from '../features/simple-counter/store/simple-counter.actions';
import { getDbDateStr } from '../util/get-db-date-str';
import { DataInitService } from '../core/data-init/data-init.service';
-
-interface PluginNodeExecutionElectronApi {
- requestGrant(pluginId: string): Promise<{ token: string } | null>;
- executeScript(
- pluginId: string,
- grantToken: string,
- request: PluginNodeScriptRequest,
- ): Promise;
- revokeGrant(pluginId: string, grantToken: string): Promise;
-}
+import { PluginNodeExecutionElectronApi } from '../../../electron/shared-with-frontend/plugin-node-execution.model';
type PluginDateFormat = 'short' | 'medium' | 'long' | 'time' | 'datetime';
@@ -1716,8 +1707,11 @@ export class PluginBridgeService implements OnDestroy {
return this.#nodeExecutionGrantTokens.get(pluginId);
}
- async requestNodeExecutionGrant(pluginId: string): Promise<{ token: string } | null> {
- return (await this.#nodeExecutionApi?.requestGrant(pluginId)) ?? null;
+ async requestNodeExecutionGrant(
+ pluginId: string,
+ displayInfo?: { name?: string; version?: string },
+ ): Promise<{ token: string } | null> {
+ return (await this.#nodeExecutionApi?.requestGrant(pluginId, displayInfo)) ?? null;
}
revokeNodeExecutionGrantToken(pluginId: string): string | undefined {
diff --git a/src/app/plugins/plugin.service.load-from-zip.spec.ts b/src/app/plugins/plugin.service.load-from-zip.spec.ts
index 5a6afef541..15ee80fb0e 100644
--- a/src/app/plugins/plugin.service.load-from-zip.spec.ts
+++ b/src/app/plugins/plugin.service.load-from-zip.spec.ts
@@ -220,7 +220,7 @@ describe('PluginService loadPluginFromZip iframe-only plugins', () => {
expect(pluginCache.storePlugin).not.toHaveBeenCalled();
});
- it('rejects uploaded plugins that declare nodeExecution before storing or loading code', async () => {
+ it('accepts uploaded plugins that declare nodeExecution (gated later by main-process consent)', async () => {
const nodeExecutionManifest: PluginManifest = {
...iframeManifest,
id: 'uploaded-node-plugin',
@@ -229,13 +229,17 @@ describe('PluginService loadPluginFromZip iframe-only plugins', () => {
};
const files: Record = {};
files['manifest.json'] = JSON.stringify(nodeExecutionManifest);
- files['plugin.js'] = 'PluginAPI.log.log("should not run")';
+ files['plugin.js'] = 'PluginAPI.log.log("node plugin")';
const file = createZipFile(files);
- await expectAsync(service.loadPluginFromZip(file)).toBeRejectedWithError(
- T.PLUGINS.NODE_EXECUTION_BUILT_IN_ONLY,
- );
- expect(pluginCache.storePlugin).not.toHaveBeenCalled();
- expect(pluginRunner.loadPlugin).not.toHaveBeenCalled();
+ // Resolving (instead of throwing NODE_EXECUTION_BUILT_IN_ONLY) is the behaviour
+ // change: uploaded node plugins are no longer rejected at upload time. The
+ // nodeExecution capability is gated by the main-process consent dialog at grant
+ // time instead.
+ const instance = await service.loadPluginFromZip(file);
+
+ expect(instance).toBeTruthy();
+ expect(instance.manifest.id).toBe('uploaded-node-plugin');
+ expect(pluginCache.storePlugin).toHaveBeenCalled();
});
});
diff --git a/src/app/plugins/plugin.service.spec.ts b/src/app/plugins/plugin.service.spec.ts
index ad3c191275..ad088e7cb9 100644
--- a/src/app/plugins/plugin.service.spec.ts
+++ b/src/app/plugins/plugin.service.spec.ts
@@ -255,7 +255,10 @@ describe('PluginService', () => {
const result = await service.enableAndActivatePlugin(manifest.id);
expect(result).toBeNull();
- expect(pluginBridge.requestNodeExecutionGrant).toHaveBeenCalledOnceWith(manifest.id);
+ expect(pluginBridge.requestNodeExecutionGrant).toHaveBeenCalledOnceWith(manifest.id, {
+ name: manifest.name,
+ version: manifest.version,
+ });
expect(pluginMetaPersistenceService.setPluginEnabled).not.toHaveBeenCalled();
expect(pluginLoader.loadPluginAssets).not.toHaveBeenCalled();
expect(service.getAllPluginStates().get(manifest.id)).toEqual(
@@ -266,6 +269,31 @@ describe('PluginService', () => {
);
});
+ it('does not re-prompt for nodeExecution after a denial within the same session', async () => {
+ const runtime = service as unknown as { _isElectronRuntime: () => boolean };
+ spyOn(runtime, '_isElectronRuntime').and.returnValue(true);
+ const manifest: PluginManifest = {
+ ...mockManifest,
+ id: 'node-plugin',
+ name: 'Node Plugin',
+ permissions: ['nodeExecution'],
+ };
+ const ensureGrant = (
+ service as unknown as {
+ _ensureNodeExecutionGrant: (m: PluginManifest) => Promise;
+ }
+ )._ensureNodeExecutionGrant.bind(service);
+
+ // First interactive attempt prompts and is denied (requestNodeExecutionGrant -> null).
+ await expectAsync(service.checkNodeExecutionPermission(manifest)).toBeResolvedTo(
+ false,
+ );
+ // A later non-interactive grant attempt this session (e.g. startup re-entry via
+ // _fireOnReady) must NOT re-open the native prompt.
+ await expectAsync(ensureGrant(manifest)).toBeResolvedTo(false);
+ expect(pluginBridge.requestNodeExecutionGrant).toHaveBeenCalledTimes(1);
+ });
+
it('stores main-issued nodeExecution grants for Electron plugins', async () => {
pluginBridge.requestNodeExecutionGrant.and.resolveTo({ token: 'token-1' });
const runtime = service as unknown as { _isElectronRuntime: () => boolean };
@@ -281,7 +309,10 @@ describe('PluginService', () => {
true,
);
- expect(pluginBridge.requestNodeExecutionGrant).toHaveBeenCalledOnceWith(manifest.id);
+ expect(pluginBridge.requestNodeExecutionGrant).toHaveBeenCalledOnceWith(manifest.id, {
+ name: manifest.name,
+ version: manifest.version,
+ });
expect(pluginBridge.setNodeExecutionGrantToken).toHaveBeenCalledOnceWith(
manifest.id,
'token-1',
diff --git a/src/app/plugins/plugin.service.ts b/src/app/plugins/plugin.service.ts
index 4283aeac31..8791c00c3a 100644
--- a/src/app/plugins/plugin.service.ts
+++ b/src/app/plugins/plugin.service.ts
@@ -62,18 +62,29 @@ const BUNDLED_PLUGIN_PATHS = [
'assets/bundled-plugins/doc-mode',
] as const;
+// Reserved ids: an uploaded plugin may not reuse a bundled plugin's manifest id (it would
+// let unverified code impersonate a built-in — and, with nodeExecution now openable to
+// uploaded plugins, claim a bundled dir's "verified built-in" consent dialog in the main
+// process, which decides bundled-vs-uploaded by on-disk dir). This set MUST contain the
+// manifest id of every entry in BUNDLED_PLUGIN_PATHS; the invariant is guarded by
+// electron/bundled-plugin-ids.test.cjs (a filesystem-reading node test, since a browser
+// Karma spec cannot read the manifests) so the two lists cannot silently drift again.
const BUNDLED_PLUGIN_IDS = new Set([
'ai-productivity-prompts',
'api-test-plugin',
'automations',
+ 'azure-devops-issue-provider',
'brain-dump',
'caldav-calendar-provider',
'clickup-issue-provider',
'doc-mode',
+ 'gitea-issue-provider',
'github-issue-provider',
'google-calendar-provider',
+ 'linear-issue-provider',
'procrastination-buster',
'sync-md',
+ 'trello-issue-provider',
'voice-reminder',
'yesterday-tasks',
]);
@@ -109,6 +120,13 @@ export class PluginService implements OnDestroy {
private _pluginIcons: Map = new Map(); // Store plugin ID -> SVG icon content
private _pluginIframeGenerations: Map = new Map();
private _pluginIconsSignal = signal