* 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.
* feat(plugins): allow uploaded nodeExecution behind consent gate
Re-open the nodeExecution permission for uploaded/community plugins
(previously built-in only, #8205) behind the existing main-process
consent dialog. Phase 1 of #8512; unblocks the Super Productivity MCP
plugin (discussion #8385).
- Main process sanitizes the attacker-controlled plugin id and the
self-declared name/version before they reach the consent dialog or
the grant map (control/bidi/whitespace rejected, length-capped).
- Bundled vs uploaded is decided by the on-disk manifest, never a
renderer-supplied flag, so uploaded code can't borrow a built-in
plugin's verified name.
- Uploaded-plugin dialog anchors on the validated id, flags the plugin
as unverified third-party with full machine access / no sandbox, and
defaults to Deny.
- Revoke is main-authoritative by (pluginId, webContents) so a re-upload
reusing an id can't inherit a live session grant.
- Consent stays session-scoped: an in-memory, never-synced denied set
prevents re-prompt storms; deny keeps the plugin enabled but fails
node calls closed until re-enable or restart.
Refs #8512#8385
* fix(plugins): reject path-segment ids in nodeExecution consent gate
Multi-agent review of the Phase 1 gate found the uploaded plugin id —
used as a path component in the bundled-manifest existsSync probe — was
not rejecting path separators or dot-segments. Impact was bounded (the
verified-builtin branch re-validates with the strict kebab regex before
any read, so no code exec / file read / dialog spoof), but `..` / `/`
left a filesystem-existence oracle and rendered misleadingly as the
dialog "Plugin ID". assertSafePluginId now rejects `/`, `\`, `.`, `..`.
Also from review: factor the shared Allow/Deny dialog shell, and correct
the denied-cache comment (the existing token short-circuit handles the
multi-call-site case; the cache only makes a denial sticky across a later
non-interactive grant re-entry). Documents the uploaded-id constraints.
Refs #8512
* fix(plugins): harden uploaded nodeExecution consent gate (review follow-ups)
Addresses multi-agent review findings on the uploaded-plugin nodeExecution
consent gate:
- id validation: use an allowlist (/^[A-Za-z0-9][A-Za-z0-9._-]*$/) instead of a
Unicode denylist, closing bidi/zero-width/homoglyph dialog-anchor spoofing the
range list missed (U+061C, U+2060, U+3164, fullwidth chars); strip all Unicode
control+format chars from the self-declared display name/version.
- never upgrade trust: describeVerifiedBuiltInDialog returns null on any imperfect
on-disk verification (id mismatch, missing permission, unreadable manifest) and
the grant handler falls back to the unverified dialog, so a colliding uploaded id
can never borrow a built-in's verified dialog.
- reserve the gitea/linear/trello/azure-devops issue-provider bundled ids (they had
drifted out of BUNDLED_PLUGIN_IDS, leaving an impersonation gap) and guard the
BUNDLED_PLUGIN_PATHS subset-of BUNDLED_PLUGIN_IDS invariant with a node test.
- key the revoke and exec IPC handlers through the same assertSafePluginId as the
grant handler so the "revoke by id on teardown/re-upload" guarantee can't drift.
- clear the session nodeExecution denial when a plugin is uninstalled, so a fresh
re-upload of the same id is prompted again rather than silently failing closed.
- de-duplicate the PluginNodeExecutionElectronApi interface into a single
electron/shared-with-frontend model (was copied byte-identically in two files).
* fix(plugins): harden node execution grants
* fix(plugins): harden iframe bridge boundaries (#8208)
* fix(plugins): harden iframe bridge boundaries
* fix(plugins): tighten iframe bridge follow-up
* test(plugins): make node-executor electron test hermetic
The new plugin-node-executor test read the real built-in plugin manifest
(src/assets/bundled-plugins/sync-md/manifest.json), which is a build
artifact absent when 'npm run test:electron' runs in CI (before the
frontend/plugin build). Stub the manifest read, scoped to the executor
module via Module._load, so the grant/token/webContents assertions no
longer depend on built plugin assets.
* fix(plugins): allow iframe formatDate & getCurrentLanguage i18n methods
The iframe API allow-list gate added in #8208 only listed a subset of
the i18n methods that master's #8146 exposes to iframe plugins. Without
this, plugin calls to formatDate/getCurrentLanguage (and translate) are
rejected with 'Unknown API method'. Add all three so the merged gate
matches the methods createBoundMethods/createPluginApiScript expose.
* docs(plugins): document node-exec handoff bootstrap-ordering invariant
The one-shot consumePluginNodeExecutionApi() handoff is defended by
construction ordering, not structural isolation: PluginBridgeService must
consume it before any plugin 'new Function' code runs (both share window.ea
in one renderer realm). Document the invariant at the consumption site so a
future lazy-service/early-plugin-load refactor can't silently regress it.
Surfaced by the post-merge security review (latent finding; not currently
exploitable).
* fix(plugins): use static iframe sandbox attribute to avoid NG0910
Binding a security-sensitive iframe attribute (sandbox) via [attr.sandbox]
makes Angular throw RuntimeError NG0910 and tear down the iframe, crashing
the plugin view to the global error screen. This broke the plugin-iframe,
plugin-loading and plugin-lifecycle e2e tests.
Restore the static sandbox attribute (still without allow-same-origin, so
the opaque-origin isolation from #8208 is preserved) and drop the now-unused
iframeSandbox binding/import.
* fix(ui): prevent stored XSS in enlarge-img directive
The enlarged-image element was built by interpolating the image URL into an innerHTML string, so a crafted synced/imported note.imgUrl could break out of the src attribute and inject an event handler (stored DOM-XSS). Build the <img> with createElement and property assignment instead, so the URL is never parsed as HTML. Adds a regression spec.
Refs: GHSA-78rv-m663-4fph
* fix(plugins): authorize nodeExecution from main-process state
The Electron main process authorized Node execution from the manifest the renderer passes on each IPC call, and window.ea.pluginExecNodeScript is callable by any renderer code — so injected renderer JS could forge {permissions:['nodeExecution']} and run arbitrary Node (CWE-501).
The main process now keeps its own grantedPlugins set, populated via a dedicated PLUGIN_SET_NODE_CONSENT channel. The renderer registers a grant in _fireOnReady (before the first node call, covering every load path including zip upload) and revokes it on teardown. The executor requires set membership.
Defense in depth, not a hard boundary: the registration channel is itself renderer-reachable, so a fully-compromised renderer could register a grant itself. It blocks the disclosed PoC (forged manifest for a never-loaded plugin) and removes per-call manifest trust. A hard boundary needs a main-process consent dialog.
Refs: GHSA-78rv-m663-4fph
* fix(security): add object-src 'none' and document CSP constraints
Adds object-src 'none' (the app embeds no <object>/<embed>) and replaces the stale TODO with an accurate note on why 'unsafe-eval'/'unsafe-inline' cannot be dropped yet: the plugin runtime and the inline bootstrap script use new Function, and the packaged app loads the renderer over file:// (opaque origin), so tightening script-src to 'self' is unreliable.
Refs: GHSA-78rv-m663-4fph
Enable plugins to access the Node.js 'os' module for system information
gathering alongside the existing fs and path modules.
Changes:
- Updated canExecuteDirectly() regex pattern to allow 'os' module imports
- Added os module import to executeDirectly() sandbox environment
- Extended sandbox require() function to provide access to os module
This allows plugins to access system information like platform, architecture,
memory usage, CPU info, and network interfaces through the standard Node.js
os module while maintaining the existing security restrictions.
The os module is considered safe as it provides read-only system information
and doesn't allow file system modifications or process execution.
- Add @super-productivity/plugin-api package with TypeScript definitions
- Define core plugin interfaces, types, and manifest structure
- Add plugin hooks system for event-driven architecture
- Create plugin API type definitions and constants
- Add documentation and development guidelines
