super-productivity/electron/bundled-plugin-ids.test.cjs
Johannes Millan a67323b2b5
feat(plugins): allow uploaded nodeExecution behind consent gate (#8576)
* 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).
2026-06-25 16:45:54 +02:00

149 lines
6 KiB
JavaScript

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/<dir>/manifest.json
* packages/plugin-dev/<dir>/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<string>([', ']'),
);
// 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 ')}`,
);
});