feat(plugins): persist nodeExecution consent per plugin (#8512) (#8600)

* feat(plugins): persist nodeExecution consent per plugin (#8512)

Phase 2 of #8512: remember an uploaded plugin's nodeExecution consent so
it is asked once, not every app session, while keeping the trust decision
local to the device.

- Main-owned, local-only store (electron/plugin-node-consent-store.ts,
  wrapping simple-store under key 'pluginNodeExecutionConsent'); never
  pfapi-synced, so a grant on one device never auto-grants on another.
- Ask-once is scoped to UPLOADED plugins; built-in plugins (sync-md) keep
  the per-session verified prompt unchanged (regression-safe).
- Consent is written only after a native Allow in main; the renderer has a
  delete-only clearConsent IPC (fail-safe) and no way to self-grant.
- Cleared on disable / uninstall / re-upload (never in generic teardown),
  so revoke = the existing disable toggle and changed code re-consents.
- No code hash: re-ask-on-change is structural (re-upload clears consent);
  a renderer hash would be forgeable and security-worthless. version:1 is
  the migration anchor if main-owned hashing is ever added.

Tests: electron 154 pass (consent-store + executor ask-once/deny/clear/
built-in-never-persisted); 474 plugin specs pass incl. the sync-exclusion
guard. Docs updated.

* fix(plugins): harden persisted consent against prototype-pollution ids

Multi-review (security) found a CRITICAL in the Phase 2 consent store: the
consents map was a plain object keyed on an attacker-controlled pluginId, so
an uploaded plugin with id 'constructor' / 'toString' / 'valueOf' /
'hasOwnProperty' resolved consents[id] to the inherited Object.prototype
member (a truthy function). The executor's ask-once check treated that as a
prior grant and minted a nodeExecution token with NO consent dialog on a fresh
install — full code execution with zero user approval. ('__proto__' was already
blocked by the id allowlist; these names pass it.) Unit tests missed it because
the executor test stub used a Map, which is immune to the footgun.

Fix:
- Store: null-prototype consents map (Object.create(null)) + own-property
  (hasOwnProperty) guarded reads + reject non-object entries. Closes the class
  for any prototype-member id, including across a disk round-trip.
- Executor: reject __proto__/prototype/constructor in assertSafePluginId as
  defense-in-depth at the boundary.
- Regression tests: consent store returns null for prototype-member ids (fresh,
  after a real set, after clear); grant request for these ids is rejected with
  no dialog and no mint.

Also from review: ask-once path now re-checks the sender URL after the consent
read (parity with the dialog path); clarified why the consent mutation queue is
not redundant with simple-store's save queue.

Electron suite 156/156 pass.

* refactor(plugins): log consent persist-failure via electron-log

Multi-review follow-ups (non-blocking):
- Route the best-effort consent persist-failure to electron-log/main (the
  user-exportable host log) instead of console; console in the executor is
  otherwise the sandboxed plugin's own output. Only the validated id is logged.
- Clarify the disable-path comment: clearing revokes the live session grant
  always, and the persisted consent only for uploaded plugins (built-ins have
  none).

* fix(plugins): clear persisted consent on cache-clear and disclose persistence in dialog

Two gaps found in multi-agent review of the Phase 2 persisted-consent feature:

- clearUploadedPluginsFromMemory() (the 'Clear plugin cache' button) wiped the
  plugin code from IndexedDB but left the main-owned persisted nodeExecution
  consent behind. A later re-upload of the same id has no existingState, so the
  re-upload consent-clear in loadPluginFromZip never fired and the (possibly
  different) code was silently granted node execution with no prompt — defeating
  the 'replacing code under an id always re-asks' invariant. Now clears consent
  for every evicted uploaded id, mirroring removeUploadedPlugin.

- The uploaded-plugin native consent dialog still said access was valid 'for this
  app session', but Allow is now persisted across sessions. The prompt now
  discloses that the choice is remembered on the device until disable / remove /
  re-upload, so the user consents to the actual scope.

Regression tests added on both sides.

* refactor(plugins): key persisted consent on a Map, not a null-prototype object

Multi-review simplification. The consent store keyed an attacker-controlled
pluginId into a plain object, defended against `Object.prototype` member names
(constructor/toString/…) with a null-prototype object + hasOwn guards + a
typeof-object read check. A `Map` makes that safety structural and self-evident —
an unstored key is simply `undefined` — and matches the sibling `grants` Map in
the executor. The on-disk format is unchanged (a plain {version, consents} object);
the Map is serialized via Object.fromEntries (define-semantics, no prototype write)
and rebuilt via Object.entries with the well-formedness guard moved to load time.

Also corrects the stale 'never downgrade-corrupt it' comment with the accurate
downgrade behavior, and adds a round-trip test proving a hand-edited on-disk
__proto__ data key loads inertly without polluting Object.prototype.

* refactor(plugins): funnel disable through PluginService.disablePlugin and de-dup dialog display

Two more multi-review items, now that we own the PR:

- 'Disabling a node plugin revokes its consent' previously lived only in the
  plugin-management UI handler, so a future programmatic disable path could unload
  the plugin yet leave persisted consent behind — re-enabling would then silently
  re-grant node execution. Added PluginService.disablePlugin(setEnabled=false +
  unload + clearNodeExecutionConsent) and routed the UI through it, making the
  revoke a structural invariant. The consent clear is a safe no-op for non-node
  plugins, so the previous requiresNodeExecution gate is dropped.

- The uploaded-plugin name/version were sanitized once for the dialog and again for
  persistence (same lengths/fallbacks, duplicated). Extracted sanitizedUploadedDisplay
  as the single source of truth so the persisted record always matches what the user
  saw in the prompt.

Tests added for the disablePlugin invariant.

* fix(plugins): harden persisted nodeExecution consent (multi-review)

Follow-ups from a multi-agent review of #8600:

- Re-ask structurally on every upload: clear consent unconditionally in
  loadPluginFromZip (outside the `existingState` branch) so a same-id
  re-upload always re-prompts even if consent was orphaned (crash
  mid-uninstall, IndexedDB eviction, external/partial wipe).
- Fail closed on upload: clearNodeExecutionConsent reports a persist failure
  via its return value; loadPluginFromZip aborts the upload if the prior
  consent could not be revoked, so replacement code can't inherit a stale
  grant. Lifecycle edges (disable/uninstall/cache-clear) ignore the result so
  a rare disk failure can't abort their bookkeeping.
- Mint the grant before the best-effort consent persist in the executor so a
  navigation/destroy during the write drops it via cleanup and a persist
  failure can't lose an approved grant.
- Validate the full consent record shape on load so a corrupt {}/array entry
  can't read as a grant.
- Log only the validated id + error code on persist failure (no userData path).
- Fix a stale comment (the store keys consent in a Map, not null-proto objects).

Adds regression tests: mint-before-persist ordering, best-effort persist,
malformed-entry rejection, and the consent-clear fail-closed return contract.

* test(plugins): add clearNodeExecutionConsent to PluginBridgeService spy

loadPluginFromZip now clears prior persisted nodeExecution consent
before loading replacement code (#8512 Phase 2). The spy in this spec
lacked the method, so the call threw, was caught, returned false, and
aborted the upload, failing both load-from-zip tests.

electron/plugin-node-consent-store.test.cjs

@@ -0,0 +1,233 @@
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const os = require('node:os');
+const path = require('node:path');
+const { promises: fs } = require('node:fs');
+const Module = require('node:module');
+
+require('ts-node/register/transpile-only');
+
+const originalModuleLoad = Module._load;
+const consentStoreModulePath = path.resolve(__dirname, 'plugin-node-consent-store.ts');
+const simpleStoreModulePath = path.resolve(__dirname, 'simple-store.ts');
+
+let userDataDir;
+const getStorePath = () => path.join(userDataDir, 'simpleSettings');
+const readStoreFile = async () => JSON.parse(await fs.readFile(getStorePath(), 'utf8'));
+
+const installMocks = () => {
+  Module._load = function patchedLoad(request, parent, isMain) {
+    if (request === 'electron') {
+      return { app: { getPath: () => userDataDir } };
+    }
+    if (request === 'electron-log/main') {
+      return { log: () => {}, error: () => {} };
+    }
+    return originalModuleLoad.call(this, request, parent, isMain);
+  };
+};
+
+// Both the consent store and the simple store it wraps hold module-level state (save /
+// mutation queues), so reset both caches to get a clean store bound to the new temp dir.
+const resetModules = () => {
+  delete require.cache[consentStoreModulePath];
+  delete require.cache[simpleStoreModulePath];
+};
+
+const loadConsentStore = () => {
+  resetModules();
+  return require(consentStoreModulePath);
+};
+
+test.beforeEach(async () => {
+  userDataDir = await fs.mkdtemp(path.join(os.tmpdir(), 'sp-node-consent-'));
+  installMocks();
+});
+
+test.afterEach(async () => {
+  Module._load = originalModuleLoad;
+  resetModules();
+  await fs.rm(userDataDir, { recursive: true, force: true });
+});
+
+test('set then get round-trips consent and pins the v1 format on disk', async () => {
+  const store = loadConsentStore();
+  await store.setNodeExecutionConsent('community.plugin', {
+    name: 'Community Plugin',
+    version: '1.2.3',
+    grantedAt: 1000,
+  });
+
+  assert.deepEqual(await store.getNodeExecutionConsent('community.plugin'), {
+    name: 'Community Plugin',
+    version: '1.2.3',
+    grantedAt: 1000,
+  });
+
+  // On-disk shape: a top-level version field (the migration anchor) keyed under the
+  // dedicated simpleSettings key — never in any pfapi-synced model.
+  const onDisk = await readStoreFile();
+  assert.equal(onDisk.pluginNodeExecutionConsent.version, 1);
+  assert.ok(onDisk.pluginNodeExecutionConsent.consents['community.plugin']);
+});
+
+test('get returns null for an unknown plugin id', async () => {
+  const store = loadConsentStore();
+  assert.equal(await store.getNodeExecutionConsent('never-granted'), null);
+});
+
+test('clear removes a persisted consent', async () => {
+  const store = loadConsentStore();
+  await store.setNodeExecutionConsent('p', { name: 'P', version: '1', grantedAt: 1 });
+  await store.clearNodeExecutionConsent('p');
+  assert.equal(await store.getNodeExecutionConsent('p'), null);
+});
+
+test('clear is a no-op (no throw) when nothing is persisted', async () => {
+  const store = loadConsentStore();
+  await assert.doesNotReject(() => store.clearNodeExecutionConsent('absent'));
+});
+
+test('ignores an on-disk blob with an unknown future version (forward-safe)', async () => {
+  // A future client may write version 2+; an older client must re-prompt (treat as
+  // empty) rather than mis-read a format it does not understand into a spurious grant.
+  await fs.writeFile(
+    getStorePath(),
+    JSON.stringify({
+      pluginNodeExecutionConsent: {
+        version: 999,
+        consents: { x: { name: 'X', version: '1', grantedAt: 1 } },
+      },
+    }),
+    'utf8',
+  );
+
+  const store = loadConsentStore();
+  assert.equal(await store.getNodeExecutionConsent('x'), null);
+});
+
+test('never treats a prototype-member id as a stored consent', async () => {
+  // SECURITY regression: the consents map is keyed on an attacker-controlled pluginId.
+  // A plain object would resolve e.g. consents['constructor'] to Object.prototype.constructor
+  // (a truthy function), which the executor would mistake for a prior grant and mint a
+  // token with NO dialog. A `Map` returns undefined for any unstored key, so reads must
+  // return null for every prototype-member id.
+  const store = loadConsentStore();
+  const protoIds = ['constructor', 'toString', 'valueOf', 'hasOwnProperty', '__proto__'];
+
+  for (const id of protoIds) {
+    assert.equal(await store.getNodeExecutionConsent(id), null, `fresh store: ${id}`);
+  }
+
+  // Persisting a real plugin must not make prototype-member ids start resolving either.
+  await store.setNodeExecutionConsent('real-plugin', {
+    name: 'Real',
+    version: '1',
+    grantedAt: 1,
+  });
+  for (const id of protoIds) {
+    assert.equal(
+      await store.getNodeExecutionConsent(id),
+      null,
+      `after a real set: ${id}`,
+    );
+  }
+
+  // Clearing a prototype-member id is a safe no-op that does not corrupt real entries.
+  await store.clearNodeExecutionConsent('constructor');
+  assert.deepEqual(await store.getNodeExecutionConsent('real-plugin'), {
+    name: 'Real',
+    version: '1',
+    grantedAt: 1,
+  });
+});
+
+test('a hand-edited on-disk `__proto__` data key round-trips inertly without polluting Object.prototype', async () => {
+  // The store serializes the consents Map via Object.fromEntries and rebuilds it via
+  // Object.entries. A maliciously hand-edited file naming a prototype member as a data key
+  // must load as an ordinary, ignorable entry — never a prototype write, never a grant.
+  await fs.writeFile(
+    getStorePath(),
+    '{"pluginNodeExecutionConsent":{"version":1,"consents":' +
+      '{"__proto__":{"name":"X","version":"1","grantedAt":1},' +
+      '"real-plugin":{"name":"Real","version":"1","grantedAt":2}}}}',
+    'utf8',
+  );
+
+  const store = loadConsentStore();
+  // A real (non-prototype) plugin still loads; loading did not pollute Object.prototype.
+  assert.deepEqual(await store.getNodeExecutionConsent('real-plugin'), {
+    name: 'Real',
+    version: '1',
+    grantedAt: 2,
+  });
+  assert.equal({}.name, undefined, 'Object.prototype must not be polluted on load');
+
+  // A subsequent write re-serializes via Object.fromEntries without polluting either.
+  await store.setNodeExecutionConsent('another', {
+    name: 'A',
+    version: '1',
+    grantedAt: 3,
+  });
+  assert.equal({}.name, undefined, 'Object.prototype must not be polluted on save');
+  assert.deepEqual(await store.getNodeExecutionConsent('another'), {
+    name: 'A',
+    version: '1',
+    grantedAt: 3,
+  });
+});
+
+test('keeps other consents when one is cleared', async () => {
+  const store = loadConsentStore();
+  await store.setNodeExecutionConsent('a', { name: 'A', version: '1', grantedAt: 1 });
+  await store.setNodeExecutionConsent('b', { name: 'B', version: '1', grantedAt: 2 });
+  await store.clearNodeExecutionConsent('a');
+
+  assert.equal(await store.getNodeExecutionConsent('a'), null);
+  assert.deepEqual(await store.getNodeExecutionConsent('b'), {
+    name: 'B',
+    version: '1',
+    grantedAt: 2,
+  });
+});
+
+test('ignores a malformed on-disk consent entry (presence alone must not authorize)', async () => {
+  // SECURITY: mere presence of an entry authorizes execution, so a corrupt/tampered value
+  // (empty object, array, or a record missing name/version/grantedAt) must be dropped — the
+  // user re-prompts — rather than read as a grant. Only a fully well-formed record loads.
+  await fs.writeFile(
+    getStorePath(),
+    JSON.stringify({
+      pluginNodeExecutionConsent: {
+        version: 1,
+        consents: {
+          'empty-object': {},
+          'array-entry': [],
+          'missing-version': { name: 'P', grantedAt: 1 },
+          'non-number-grantedAt': { name: 'P', version: '1', grantedAt: 'soon' },
+          'good-plugin': { name: 'Good', version: '1', grantedAt: 5 },
+        },
+      },
+    }),
+    'utf8',
+  );
+
+  const store = loadConsentStore();
+  for (const id of [
+    'empty-object',
+    'array-entry',
+    'missing-version',
+    'non-number-grantedAt',
+  ]) {
+    assert.equal(
+      await store.getNodeExecutionConsent(id),
+      null,
+      `malformed entry ${id} must be ignored`,
+    );
+  }
+  assert.deepEqual(await store.getNodeExecutionConsent('good-plugin'), {
+    name: 'Good',
+    version: '1',
+    grantedAt: 5,
+  });
+});

electron/plugin-node-consent-store.ts

@@ -0,0 +1,144 @@
+import { loadSimpleStoreAll, saveSimpleStore } from './simple-store';
+import { SimpleStoreKey } from './shared-with-frontend/simple-store.const';
+
+/**
+ * Persisted, per-plugin nodeExecution consent (issue #8512 Phase 2).
+ *
+ * SECURITY / TRUST MODEL
+ * - This store lives in the main-owned `simpleSettings` file under the OS userData
+ *   dir. It is NEVER part of any pfapi-synced model, so a granted consent on one
+ *   device does not auto-grant on another (a node call there triggers a fresh native
+ *   prompt). There is no renderer IPC that can *write* a consent entry — only the
+ *   native Allow dialog in `plugin-node-executor.ts` calls `setNodeExecutionConsent`.
+ *   The renderer can only ask to *clear* consent (fail-safe: clearing forces a
+ *   re-prompt, never an auto-grant).
+ * - Consent is keyed on `pluginId` only. The "re-ask when the plugin's code changes"
+ *   property is achieved structurally, not by a stored code hash: the only legitimate
+ *   way an uploaded plugin's code changes is a re-upload, and the renderer clears this
+ *   consent on disable, uninstall, and re-upload. A renderer-computed hash would be a
+ *   forgeable TOCTOU tripwire with no security value (a granted plugin already has full
+ *   machine access via `executeScript`), so it is deliberately omitted. The top-level
+ *   `version` field below is the migration anchor if a main-owned hash is ever added.
+ * - `name`/`version` are the self-declared display strings shown at grant time. They
+ *   are stored for diagnostics/UX only and are NEVER used for authorization — the
+ *   non-spoofable trust anchor is the `pluginId`.
+ */
+
+export const NODE_EXECUTION_CONSENT_STORE_VERSION = 1 as const;
+
+export interface PersistedNodeExecutionConsent {
+  /** Self-declared display name at grant time (unverified for uploaded plugins). */
+  name: string;
+  /** Self-declared version at grant time (unverified for uploaded plugins). */
+  version: string;
+  /** ms epoch when the user allowed it. */
+  grantedAt: number;
+}
+
+interface NodeExecutionConsentBlob {
+  version: number;
+  // SECURITY: keyed on an attacker-controlled pluginId. A `Map` (not a plain object) is
+  // used so an id that names an `Object.prototype` member (`constructor`, `toString`,
+  // `valueOf`, `hasOwnProperty`, …) is just an ordinary key that returns `undefined` when
+  // absent — it can never resolve to an inherited function the executor would mistake for
+  // a stored grant. Mirrors the sibling `grants` Map in plugin-node-executor.ts.
+  consents: Map<string, PersistedNodeExecutionConsent>;
+}
+
+const emptyBlob = (): NodeExecutionConsentBlob => ({
+  version: NODE_EXECUTION_CONSENT_STORE_VERSION,
+  consents: new Map(),
+});
+
+const loadBlob = async (): Promise<NodeExecutionConsentBlob> => {
+  const all = await loadSimpleStoreAll();
+  const raw = all[SimpleStoreKey.PLUGIN_NODE_EXECUTION_CONSENT];
+  if (!raw || typeof raw !== 'object') {
+    return emptyBlob();
+  }
+  const blob = raw as { version?: unknown; consents?: unknown };
+  // Forward-safe: a future on-disk format we don't understand is ignored (the user is
+  // re-prompted) rather than mis-read into a spurious grant.
+  if (
+    blob.version !== NODE_EXECUTION_CONSENT_STORE_VERSION ||
+    !blob.consents ||
+    typeof blob.consents !== 'object'
+  ) {
+    return emptyBlob();
+  }
+  // Build the Map from the persisted plain object. Only a fully well-formed entry counts:
+  // mere presence authorizes execution, so a corrupt/tampered value (a primitive, an array,
+  // an empty `{}`, or a partial record) is dropped — the user re-prompts — rather than
+  // mis-read as a grant. A literal `__proto__`/`constructor` key from a hand-edited file is
+  // just an ordinary Map key here, never a prototype write.
+  const consents = new Map<string, PersistedNodeExecutionConsent>();
+  for (const [pluginId, entry] of Object.entries(
+    blob.consents as Record<string, unknown>,
+  )) {
+    if (entry === null || typeof entry !== 'object' || Array.isArray(entry)) {
+      continue;
+    }
+    const { name, version, grantedAt } = entry as Record<string, unknown>;
+    if (
+      typeof name === 'string' &&
+      typeof version === 'string' &&
+      typeof grantedAt === 'number' &&
+      Number.isFinite(grantedAt)
+    ) {
+      consents.set(pluginId, { name, version, grantedAt });
+    }
+  }
+  return { version: NODE_EXECUTION_CONSENT_STORE_VERSION, consents };
+};
+
+// Serialize read-modify-write mutations so two concurrent grants/clears can't clobber
+// each other (load-load-write-write). This is NOT redundant with simple-store's own save
+// queue: `loadBlob()` happens *outside* `saveSimpleStore`, so without this lock two
+// interleaved mutations could both read the same blob before either writes. Reads
+// (getNodeExecutionConsent) are point-in-time and need no lock.
+let _mutationQueue: Promise<unknown> = Promise.resolve();
+
+const mutate = (apply: (blob: NodeExecutionConsentBlob) => boolean): Promise<void> => {
+  const run = async (): Promise<void> => {
+    const blob = await loadBlob();
+    if (apply(blob)) {
+      // Serialize the Map to a plain object for JSON persistence. `Object.fromEntries`
+      // uses define-semantics, so even a literal `__proto__` key becomes an own data
+      // property — it cannot pollute a prototype. Downgrade note: an older client that
+      // finds a newer on-disk `version` reads it as empty (loadBlob) and this write then
+      // replaces it with a v1 blob, discarding a future client's consents — worst case a
+      // re-prompt, never a spurious grant.
+      await saveSimpleStore(SimpleStoreKey.PLUGIN_NODE_EXECUTION_CONSENT, {
+        version: blob.version,
+        consents: Object.fromEntries(blob.consents),
+      });
+    }
+  };
+  _mutationQueue = _mutationQueue.then(run, run);
+  return _mutationQueue as Promise<void>;
+};
+
+export const getNodeExecutionConsent = async (
+  pluginId: string,
+): Promise<PersistedNodeExecutionConsent | null> => {
+  const blob = await loadBlob();
+  return blob.consents.get(pluginId) ?? null;
+};
+
+export const setNodeExecutionConsent = async (
+  pluginId: string,
+  consent: PersistedNodeExecutionConsent,
+): Promise<void> =>
+  mutate((blob) => {
+    blob.consents.set(pluginId, {
+      name: consent.name,
+      version: consent.version,
+      grantedAt: consent.grantedAt,
+    });
+    return true;
+  });
+
+// `Map.delete` returns true only when an entry existed, which is exactly the
+// "write only if changed" signal `mutate` wants — a clear of an absent id is a no-op.
+export const clearNodeExecutionConsent = async (pluginId: string): Promise<void> =>
+  mutate((blob) => blob.consents.delete(pluginId));

electron/plugin-node-executor.test.cjs

@@ -27,6 +27,9 @@ let ipcHandlers;
 let dialogCalls;
 let nextDialogResult;
 let nextDialogPromise;
+// In-memory stand-in for the main-owned persisted consent store (plugin-node-consent-store.ts),
+// so the executor's Phase 2 ask-once logic can be tested without touching the filesystem.
+let consentStore;
 
 class FakeWebContents extends EventEmitter {
   constructor(id, url = 'app://index.html') {
@@ -96,6 +99,20 @@ const installMocks = () => {
       };
     }
 
+    // Stub the persisted-consent store so Phase 2 ask-once logic is exercised in-memory.
+    if (
+      request === './plugin-node-consent-store' &&
+      parent &&
+      typeof parent.filename === 'string' &&
+      parent.filename.endsWith('plugin-node-executor.ts')
+    ) {
+      return consentStore;
+    }
+
+    if (request === 'electron-log/main') {
+      return { log: () => {}, error: () => {} };
+    }
+
     if (request === 'electron') {
       return {
         app: {
@@ -141,6 +158,17 @@ test.beforeEach(() => {
   dialogCalls = [];
   nextDialogResult = { response: 0 };
   nextDialogPromise = undefined;
+  const records = new Map();
+  consentStore = {
+    __records: records,
+    getNodeExecutionConsent: async (pluginId) => records.get(pluginId) ?? null,
+    setNodeExecutionConsent: async (pluginId, consent) => {
+      records.set(pluginId, consent);
+    },
+    clearNodeExecutionConsent: async (pluginId) => {
+      records.delete(pluginId);
+    },
+  };
   installMocks();
 });
 
@@ -274,6 +302,10 @@ test('issues a grant to an uploaded (non-bundled) plugin and labels it unverifie
   // and defaults to Deny.
   assert.match(opts.detail, /Plugin ID: uploaded-node-plugin/);
   assert.match(opts.detail, /self-declared, unverified/);
+  // The Allow is persisted (ask-once), so the prompt must disclose that the choice is
+  // remembered rather than claiming it is only valid "for this app session".
+  assert.match(opts.detail, /remembered on this device/);
+  assert.equal(opts.detail.includes('for this app session'), false);
   assert.equal(opts.defaultId, 1);
 });
 
@@ -478,3 +510,182 @@ test('never upgrades trust: an on-disk match that does not cleanly verify uses t
     BUILT_IN_PLUGIN_MANIFEST.permissions = originalPermissions;
   }
 });
+
+// --- Phase 2: persisted, per-plugin consent (#8512) ---
+
+test('uploaded plugin with persisted consent is granted without a dialog (ask-once)', async () => {
+  loadModule();
+  // Simulate a grant remembered from a previous app session.
+  consentStore.__records.set('uploaded-node-plugin', {
+    name: 'Uploaded',
+    version: '1.0.0',
+    grantedAt: 1,
+  });
+  const webContents = new FakeWebContents(20);
+
+  const grant = await callIpc(
+    'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+    webContents,
+    'uploaded-node-plugin',
+    { name: 'Uploaded', version: '1.0.0' },
+  );
+
+  assert.equal(typeof grant.token, 'string');
+  assert.equal(dialogCalls.length, 0);
+});
+
+test('granting an uploaded plugin persists consent for the next session', async () => {
+  loadModule();
+  const webContents = new FakeWebContents(21);
+
+  await callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', webContents, 'persist-me', {
+    name: 'Persist Me',
+    version: '2.0.0',
+  });
+
+  assert.equal(dialogCalls.length, 1);
+  const persisted = consentStore.__records.get('persist-me');
+  assert.equal(persisted.name, 'Persist Me');
+  assert.equal(persisted.version, '2.0.0');
+  assert.equal(typeof persisted.grantedAt, 'number');
+});
+
+test('denying an uploaded plugin does not persist consent', async () => {
+  loadModule();
+  nextDialogResult = { response: 1 };
+  const webContents = new FakeWebContents(22);
+
+  const denied = await callIpc(
+    'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+    webContents,
+    'deny-me',
+    { name: 'Deny', version: '1.0.0' },
+  );
+
+  assert.equal(denied, null);
+  assert.equal(consentStore.__records.has('deny-me'), false);
+});
+
+test('clearConsent drops the grant + persisted consent so the next request re-prompts', async () => {
+  loadModule();
+  const wc1 = new FakeWebContents(23);
+  const grant = await callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', wc1, 'clear-me', {
+    name: 'Clear',
+    version: '1.0.0',
+  });
+  assert.equal(dialogCalls.length, 1);
+  assert.ok(consentStore.__records.has('clear-me'));
+
+  await callIpc('PLUGIN_CLEAR_NODE_EXECUTION_CONSENT', wc1, 'clear-me');
+  assert.equal(consentStore.__records.has('clear-me'), false);
+
+  // The live session grant is gone too: exec is now unauthorized.
+  await assert.rejects(
+    () =>
+      callIpc('PLUGIN_EXEC_NODE_SCRIPT', wc1, 'clear-me', grant.token, {
+        script: 'return true;',
+      }),
+    /not authorized/,
+  );
+
+  // A fresh request re-prompts because no persisted consent remains.
+  const wc2 = new FakeWebContents(24);
+  await callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', wc2, 'clear-me', {
+    name: 'Clear',
+    version: '1.0.0',
+  });
+  assert.equal(dialogCalls.length, 2);
+});
+
+test('rejects prototype-pollution ids before any consent lookup or mint', async () => {
+  // SECURITY regression: 'constructor'/'prototype' pass the id allowlist regex and would,
+  // against a plain-object consent map, resolve to an Object.prototype member and mint a
+  // grant with no dialog. They must be rejected outright. ('__proto__' fails the regex.)
+  loadModule();
+  const webContents = new FakeWebContents(27);
+
+  for (const badId of ['constructor', 'prototype', '__proto__']) {
+    await assert.rejects(
+      () =>
+        callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', webContents, badId, {
+          name: 'x',
+          version: '1',
+        }),
+      /Invalid pluginId/,
+      `expected ${badId} to be rejected`,
+    );
+  }
+  assert.equal(dialogCalls.length, 0);
+  assert.equal(consentStore.__records.has('constructor'), false);
+});
+
+test('built-in plugin consent is never persisted (stays per-session, regression)', async () => {
+  loadModule();
+  const wc1 = new FakeWebContents(25);
+  await callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', wc1, 'sync-md');
+  assert.equal(dialogCalls.length, 1);
+  // Built-in plugins keep the verified per-session prompt — nothing is written to the
+  // persisted store, so a new session prompts again.
+  assert.equal(consentStore.__records.has('sync-md'), false);
+
+  const wc2 = new FakeWebContents(26);
+  await callIpc('PLUGIN_REQUEST_NODE_EXECUTION_GRANT', wc2, 'sync-md');
+  assert.equal(dialogCalls.length, 2);
+});
+
+test('mints the grant before persisting consent (persist is best-effort, never gates the grant)', async () => {
+  // SECURITY ordering: the consent persist is a new `await` after the post-dialog
+  // sender-validity check. Minting BEFORE that write keeps the grant in the live table
+  // during the write, so a navigation/destroy cleanup that fires mid-write drops it (in
+  // real Electron), and a persist failure can never lose an approved grant. Asserting the
+  // grant already exists at the moment of the persist write pins that ordering.
+  let grantsSizeAtPersist = -1;
+  let mod;
+  consentStore.setNodeExecutionConsent = async (pluginId, consent) => {
+    grantsSizeAtPersist = mod.pluginNodeExecutor.grants.size;
+    consentStore.__records.set(pluginId, consent);
+  };
+  mod = loadModule();
+  const webContents = new FakeWebContents(30);
+
+  const grant = await callIpc(
+    'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+    webContents,
+    'uploaded-order',
+    { name: 'Uploaded', version: '1.0.0' },
+  );
+
+  assert.equal(typeof grant.token, 'string');
+  // The grant for this request was already live when the persist ran (mint-before-persist).
+  assert.equal(grantsSizeAtPersist, 1);
+});
+
+test('a consent persist failure still grants the approved session (best-effort)', async () => {
+  // Persisting is best-effort: a write failure only costs a re-prompt next session, never
+  // the grant the user just approved. Minting before the persist guarantees this.
+  let mod;
+  consentStore.setNodeExecutionConsent = async () => {
+    throw Object.assign(new Error('disk full'), { code: 'ENOSPC' });
+  };
+  mod = loadModule();
+  assert.ok(mod);
+  const webContents = new FakeWebContents(31);
+
+  const grant = await callIpc(
+    'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
+    webContents,
+    'persist-fails',
+    { name: 'Uploaded', version: '1.0.0' },
+  );
+
+  assert.equal(typeof grant.token, 'string');
+  const result = await callIpc(
+    'PLUGIN_EXEC_NODE_SCRIPT',
+    webContents,
+    'persist-fails',
+    grant.token,
+    { script: 'return 1 + 1;' },
+  );
+  assert.equal(result.success, true);
+  assert.equal(result.result, 2);
+});

electron/plugin-node-executor.ts

@@ -1,5 +1,6 @@
 import { app, BrowserWindow, dialog, ipcMain } from 'electron';
 import type { WebContents } from 'electron';
+import { error as logError } from 'electron-log/main';
 import { spawn } from 'child_process';
 import { randomBytes } from 'crypto';
 import { existsSync, readFileSync } from 'fs';
@@ -11,6 +12,11 @@ import {
   PluginNodeScriptResult,
   PluginManifest,
 } from '../packages/plugin-api/src/types';
+import {
+  clearNodeExecutionConsent,
+  getNodeExecutionConsent,
+  setNodeExecutionConsent,
+} from './plugin-node-consent-store';
 
 const DEFAULT_TIMEOUT = 30000; // 30 seconds
 const MAX_TIMEOUT = 300000; // 5 minutes
@@ -27,6 +33,12 @@ const BUILT_IN_PLUGIN_ID_RE = /^[a-z0-9][a-z0-9-]*$/;
 // 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._-]*$/;
+// The id is used as a key into the persisted-consent store, which keys consent in a `Map`
+// (returns `undefined` for any unstored key) so an `Object.prototype` member name can't
+// masquerade as a stored grant. Reject the classic pollution keys at this boundary too as
+// defense in depth (`__proto__` is already excluded by the leading-char allowlist; the
+// others pass it).
+const FORBIDDEN_PLUGIN_IDS = new Set(['__proto__', 'prototype', 'constructor']);
 // 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)
@@ -48,6 +60,9 @@ const assertSafePluginId = (pluginId: unknown): string => {
   if (!SAFE_UPLOADED_PLUGIN_ID_RE.test(pluginId)) {
     throw new Error('Invalid pluginId');
   }
+  if (FORBIDDEN_PLUGIN_IDS.has(pluginId)) {
+    throw new Error('Invalid pluginId');
+  }
   return pluginId;
 };
 
@@ -120,6 +135,9 @@ class PluginNodeExecutor {
         const safeId = assertSafePluginId(pluginId);
 
         const webContentsId = event.sender.id;
+        // Captured before any await so both the ask-once and dialog paths can detect a
+        // navigation that happened while we were resolving consent.
+        const requestUrl = event.sender.getURL();
         const existingGrant = this.grants.get(safeId);
         if (existingGrant) {
           if (existingGrant.webContentsId === webContentsId) {
@@ -135,11 +153,31 @@ class PluginNodeExecutor {
         // (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 builtInManifest = this.tryGetVerifiedBuiltInManifest(safeId);
+
+        // Phase 2 (issue #8512): persisted, ask-once consent is scoped to UPLOADED
+        // (community) plugins only. Built-in plugins keep the per-session verified prompt
+        // — no behaviour change, so `sync-md` etc. are regression-safe. Only a prior
+        // native Allow can have written this entry (no renderer write path), and the
+        // renderer clears it on disable/uninstall/re-upload, so a re-uploaded (changed)
+        // plugin reusing the id never silently inherits it.
+        if (!builtInManifest) {
+          const persistedConsent = await getNodeExecutionConsent(safeId);
+          if (persistedConsent) {
+            // Don't mint for a sender that was destroyed or navigated away while the
+            // consent was being read (parity with the post-dialog checks below).
+            if (event.sender.isDestroyed() || event.sender.getURL() !== requestUrl) {
+              return null;
+            }
+            this.registerGrantCleanup(event.sender);
+            return this.mintGrant(safeId, webContentsId);
+          }
+        }
+
+        const dialogOptions = builtInManifest
+          ? this.buildVerifiedBuiltInDialog(safeId, builtInManifest)
+          : this.describeUnverifiedUploadedDialog(safeId, displayInfo);
 
-        const requestUrl = event.sender.getURL();
         this.registerGrantCleanup(event.sender);
 
         let result: Electron.MessageBoxReturnValue;
@@ -166,12 +204,57 @@ class PluginNodeExecutor {
           return null;
         }
 
-        const token = randomBytes(32).toString('base64url');
-        this.grants.set(safeId, {
-          token,
-          webContentsId,
-        });
-        return { token };
+        // Allow → mint the grant FIRST, synchronously, right after the sender-validity
+        // check above. The persist below is a new `await`; minting before it keeps the
+        // "never mint for a navigated/destroyed sender" invariant tight — if the sender
+        // navigates or is destroyed during the write, the cleanup listener registered
+        // before the dialog drops this just-minted grant, so no untracked grant survives
+        // for a stale webContents.
+        const grant = this.mintGrant(safeId, webContentsId);
+
+        // For uploaded plugins, remember the decision so later sessions don't re-prompt
+        // (ask-once). Persisting is best-effort: a write failure only costs a re-prompt
+        // next session, never a grant the user just approved.
+        if (!builtInManifest) {
+          try {
+            // Persist exactly the name/version the user saw in the dialog.
+            await setNodeExecutionConsent(safeId, {
+              ...this.sanitizedUploadedDisplay(displayInfo),
+              grantedAt: Date.now(),
+            });
+          } catch (error) {
+            // Host diagnostic → exportable log (electron-log), distinct from the sandboxed
+            // plugin's own console output. Log only the validated id and the error code —
+            // never the raw error, whose message can embed the userData absolute path (and
+            // thus the OS username) for an fs failure.
+            const code = (error as NodeJS.ErrnoException)?.code ?? 'unknown';
+            logError(`Failed to persist nodeExecution consent for ${safeId} (${code})`);
+          }
+        }
+        return grant;
+      },
+    );
+
+    ipcMain.handle(
+      IPC.PLUGIN_CLEAR_NODE_EXECUTION_CONSENT,
+      // Explicit revoke from the trusted renderer that owns the plugin lifecycle
+      // (disable / uninstall / re-upload). Drops the live session grant for this id AND
+      // the persisted consent, so the next node call re-prompts. A delete is fail-safe:
+      // the worst a hostile renderer can do by calling this is force a prompt, never a
+      // silent grant.
+      async (_event, pluginId: string) => {
+        let safeId: string;
+        try {
+          safeId = assertSafePluginId(pluginId);
+        } catch {
+          return;
+        }
+        const grant = this.grants.get(safeId);
+        if (grant) {
+          this.grants.delete(safeId);
+          this.releaseGrantCleanupIfUnused(grant.webContentsId);
+        }
+        await clearNodeExecutionConsent(safeId);
       },
     );
 
@@ -297,23 +380,36 @@ class PluginNodeExecutor {
     this.unregisterGrantCleanup(webContentsId);
   }
 
+  private mintGrant(pluginId: string, webContentsId: number): { token: string } {
+    const token = randomBytes(32).toString('base64url');
+    this.grants.set(pluginId, { token, webContentsId });
+    return { token };
+  }
+
   /**
-   * 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.
+   * Resolve the cleanly-verified built-in nodeExecution manifest for an id, or null
+   * when the id does not resolve to one (no on-disk match, id mismatch, missing
+   * permission, or unreadable/invalid manifest). A partial or colliding match must
+   * never *upgrade* trust to the built-in dialog, so callers fall back to the
+   * unverified-uploaded path on null.
    */
-  private describeVerifiedBuiltInDialog(
-    pluginId: string,
-  ): Electron.MessageBoxOptions | null {
-    let manifest: PluginManifest;
+  private tryGetVerifiedBuiltInManifest(pluginId: string): PluginManifest | null {
     try {
-      manifest = this.getVerifiedBuiltInNodeExecutionManifest(pluginId);
+      return this.getVerifiedBuiltInNodeExecutionManifest(pluginId);
     } catch {
       return null;
     }
+  }
+
+  /**
+   * Consent dialog for a verified built-in plugin (name/version read from disk).
+   * Built-in plugins are NOT persisted (Phase 2 ask-once is uploaded-only), so the copy
+   * keeps the per-session wording.
+   */
+  private buildVerifiedBuiltInDialog(
+    pluginId: string,
+    manifest: PluginManifest,
+  ): Electron.MessageBoxOptions {
     return {
       ...NODE_CONSENT_DIALOG_BASE,
       title: 'Allow plugin Node.js execution?',
@@ -327,6 +423,21 @@ class PluginNodeExecutor {
     };
   }
 
+  /**
+   * The self-declared name/version exactly as shown in the uploaded-plugin consent dialog.
+   * Single source of truth for the sanitize lengths and fallbacks, so the persisted record
+   * (setNodeExecutionConsent) always matches what the user actually saw and approved.
+   */
+  private sanitizedUploadedDisplay(displayInfo?: NodeExecutionGrantDisplayInfo): {
+    name: string;
+    version: string;
+  } {
+    return {
+      name: sanitizeDialogString(displayInfo?.name, 80) || '(unnamed)',
+      version: sanitizeDialogString(displayInfo?.version, 32) || '(unknown)',
+    };
+  }
+
   /**
    * 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
@@ -336,8 +447,7 @@ class PluginNodeExecutor {
     pluginId: string,
     displayInfo?: NodeExecutionGrantDisplayInfo,
   ): Electron.MessageBoxOptions {
-    const name = sanitizeDialogString(displayInfo?.name, 80) || '(unnamed)';
-    const version = sanitizeDialogString(displayInfo?.version, 32) || '(unknown)';
+    const { name, version } = this.sanitizedUploadedDisplay(displayInfo);
     return {
       ...NODE_CONSENT_DIALOG_BASE,
       title: 'Allow this plugin to run code on your machine?',
@@ -348,7 +458,8 @@ class PluginNodeExecutor {
         `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.',
+        'If you allow it, the plugin can run any program with full access to your files and system.',
+        'Your choice is remembered on this device until you disable, remove, or re-upload this plugin.',
         '',
         'Only allow this if you trust the source of this plugin.',
       ].join('\n'),

electron/preload.ts

@@ -269,6 +269,8 @@ const ea: ElectronAPI = {
           pluginId,
           grantToken,
         ) as Promise<void>,
+      clearConsent: (pluginId: string) =>
+        _invoke('PLUGIN_CLEAR_NODE_EXECUTION_CONSENT', pluginId) as Promise<void>,
     };
   },
 

electron/shared-with-frontend/ipc-events.const.ts

@@ -86,6 +86,7 @@ export enum IPC {
   PLUGIN_REQUEST_NODE_EXECUTION_GRANT = 'PLUGIN_REQUEST_NODE_EXECUTION_GRANT',
   PLUGIN_EXEC_NODE_SCRIPT = 'PLUGIN_EXEC_NODE_SCRIPT',
   PLUGIN_REVOKE_NODE_EXECUTION_GRANT = 'PLUGIN_REVOKE_NODE_EXECUTION_GRANT',
+  PLUGIN_CLEAR_NODE_EXECUTION_CONSENT = 'PLUGIN_CLEAR_NODE_EXECUTION_CONSENT',
 
   // Plugin OAuth
   PLUGIN_OAUTH_PREPARE = 'PLUGIN_OAUTH_PREPARE',

electron/shared-with-frontend/plugin-node-execution.model.ts

@@ -21,4 +21,11 @@ export interface PluginNodeExecutionElectronApi {
     request: PluginNodeScriptRequest,
   ): Promise<PluginNodeScriptResult>;
   revokeGrant(pluginId: string, grantToken: string): Promise<void>;
+  /**
+   * Drop the live session grant AND the main-owned persisted consent for this plugin so
+   * the next node call re-prompts. Called by the renderer on disable / uninstall /
+   * re-upload. There is deliberately no `setConsent` counterpart — only a native Allow
+   * dialog in main can write consent, so the renderer can never self-grant.
+   */
+  clearConsent(pluginId: string): Promise<void>;
 }

electron/shared-with-frontend/simple-store.const.ts

@@ -3,6 +3,10 @@ export enum SimpleStoreKey {
   ALLOWED_COMMANDS = 'allowedCommands',
   // Main-owned sync folder path (issue #8228); the renderer no longer holds it.
   SYNC_FOLDER_PATH = 'syncFolderPath',
+  // Main-owned, never-synced persisted nodeExecution consent (issue #8512 Phase 2).
+  // The renderer has no IPC write path that can grant consent into this key — only
+  // a native Allow dialog in main writes it — so XSS/un-granted code cannot self-grant.
+  PLUGIN_NODE_EXECUTION_CONSENT = 'pluginNodeExecutionConsent',
   // Legacy key kept for backwards compatibility when reading persisted settings
   LEGACY_IS_USE_OBSIDIAN_STYLE_HEADER = 'isUseObsidianStyleHeader',
 }