From c2c274acf5190155ac3c584113b48eac6c9f6279 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 1 May 2026 14:39:45 +0100 Subject: [PATCH 1/2] fix(plugins): updatePlugins.sh actually updates installed plugins (#6670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bin/updatePlugins.sh detected outdated plugins by running `pnpm --filter ep_etherpad-lite outdated --depth=0`, but installed plugins are not registered in src/package.json — bin/plugins.ts adds them via linkInstaller.installPlugin which writes to src/plugin_packages/.versions/@/ and tracks the result in var/installed_plugins.json. pnpm has no view of them, so `outdated` returns empty and the script always reported "All plugins are up-to-date" even when newer versions existed on the registry. PR #7468 fixed npm→pnpm and install→update but kept the same broken detection mechanism, which is why the issue stayed open after that PR landed. Read the plugin list from var/installed_plugins.json instead, then re-invoke linkInstaller.installPlugin(name) for each entry. Calling the installer without a version pin resolves the registry-latest and overwrites the existing pinned copy, so an outdated plugin is brought to head while plugins already at latest are no-ops apart from the pnpm cache hit. Add an `update`/`up` action to bin/plugins.ts so users can also run `pnpm run plugins update` directly, mirroring the existing install/remove/list actions. updatePlugins.sh becomes a one-line wrapper for backwards compatibility. Reproduction (verified): pnpm run install-plugins ep_markdown@11.0.5 # latest is 11.0.18 ./bin/updatePlugins.sh # → 11.0.18 Edge cases tested: no plugins installed, missing installed_plugins.json, already-at-latest re-run. Closes #6670. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/plugins.ts | 43 ++++++++++++++++++++++++++++++++++++++++++- bin/updatePlugins.sh | 10 +--------- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/bin/plugins.ts b/bin/plugins.ts index 9acd2af5326..5384fd599a6 100644 --- a/bin/plugins.ts +++ b/bin/plugins.ts @@ -19,7 +19,9 @@ const possibleActions = [ "rm", "remove", "ls", - "list" + "list", + "up", + "update" ] const install = ()=> { @@ -76,6 +78,39 @@ const list = ()=>{ })(); } +// Re-install every plugin in installed_plugins.json without a version pin so +// the registry-latest gets resolved and overwrites the existing pinned copy +// in src/plugin_packages/. ep_etherpad-lite is excluded because the core is +// vendored, not plugin-installed. +const update = ()=> { + (async () => { + const path = settings.root+"/var/installed_plugins.json"; + let plugins: {name: string}[]; + try { + plugins = JSON.parse(fs.readFileSync(path, "utf-8")).plugins || []; + } catch (err: any) { + if (err.code === 'ENOENT') { + console.log("No installed_plugins.json found — nothing to update"); + return; + } + throw err; + } + const names = plugins + .map((p) => p.name) + .filter((n) => n && n !== 'ep_etherpad-lite'); + if (names.length === 0) { + console.log("No plugins installed — nothing to update"); + return; + } + console.log(`Updating plugins to latest from registry: ${names.join(', ')}`); + await checkForMigration(); + for (const name of names) { + await linkInstaller.installPlugin(name); + } + await persistInstalledPlugins(); + })(); +} + const remove = (plugins: string[])=>{ const walk = async () => { for (const plugin of plugins) { @@ -112,6 +147,12 @@ switch (action) { case "remove": remove(args.slice(1)); break; + case "up": + update(); + break; + case "update": + update(); + break; default: console.error('Expected at least one argument!'); process.exit(1); diff --git a/bin/updatePlugins.sh b/bin/updatePlugins.sh index a2b30ca3d34..d5ca0732805 100755 --- a/bin/updatePlugins.sh +++ b/bin/updatePlugins.sh @@ -2,12 +2,4 @@ set -e mydir=$(cd "${0%/*}" && pwd -P) || exit 1 cd "${mydir}"/.. -outdated_raw=$(pnpm --filter ep_etherpad-lite outdated --depth=0 2>&1) || true -OUTDATED=$(printf '%s\n' "$outdated_raw" | awk '{print $1}' | grep '^ep_' | grep -v '^ep_etherpad-lite$') || true -if [ -z "$OUTDATED" ]; then - echo "All plugins are up-to-date" - exit 0 -fi -set -- ${OUTDATED} -echo "Updating plugins: $*" -exec pnpm --filter ep_etherpad-lite update "$@" +exec pnpm --filter bin run plugins update From 750430aaf0258bfec7e80b3ef03597c92b07cc96 Mon Sep 17 00:00:00 2001 From: John McLear Date: Fri, 1 May 2026 16:43:53 +0100 Subject: [PATCH 2/2] fix(plugins): validate ep_ prefix and dedupe + add regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo flagged two issues on the original update() addition: 1. Security — update() trusted every name in var/installed_plugins.json, so a corrupted or hand-edited manifest could coerce the script into installing arbitrary npm packages. pluginfw/plugins.getPackages already gates on the ep_ prefix; mirror that gate here. 2. Reliability — no automated regression test, so a future refactor could silently bring back the broken behaviour. Extract the safe-name filter to filterUpdatablePluginNames in bin/commonPlugins.ts (pure, side-effect-free, prefix configurable, also de-duplicates repeats so a duplicated entry installs once). Use it from plugins.ts update(). Add src/tests/backend/specs/filterUpdatablePluginNames.ts covering: keep prefixed names, drop ep_etherpad-lite, reject non-prefixed entries, de-dupe repeats, tolerate missing/null/non-string name fields, empty input, custom prefix. Manually verified end-to-end on a live install: an installed_plugins.json containing ep_markdown@11.0.5, a duplicate ep_markdown, and a "malicious-package" entry runs `Updating plugins to latest from registry: ep_markdown` (only) and ep_markdown ends up at 11.0.18 — the bad entries are silently filtered out. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/commonPlugins.ts | 20 +++++ bin/plugins.ts | 17 +++-- .../specs/filterUpdatablePluginNames.ts | 75 +++++++++++++++++++ 3 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 src/tests/backend/specs/filterUpdatablePluginNames.ts diff --git a/bin/commonPlugins.ts b/bin/commonPlugins.ts index ecc6a67c83f..54d07cc3a7a 100644 --- a/bin/commonPlugins.ts +++ b/bin/commonPlugins.ts @@ -3,6 +3,26 @@ import {writeFileSync} from "fs"; import {installedPluginsPath} from "ep_etherpad-lite/static/js/pluginfw/installer"; const pluginsModule = require('ep_etherpad-lite/static/js/pluginfw/plugins'); +// Pure helper used by `pnpm run plugins update` to whittle the contents of +// var/installed_plugins.json down to names safe to re-install. Mirrors the +// gate inside pluginfw/plugins.getPackages: only entries that start with the +// plugin prefix (ep_) are real Etherpad plugins; ep_etherpad-lite is the +// vendored core, never installed via the plugin path. De-duplicates so a +// corrupted manifest with repeated entries triggers one install per name. +// Exported and kept side-effect-free so backend tests can exercise it. +export const filterUpdatablePluginNames = ( + entries: ReadonlyArray<{name?: unknown} | null | undefined>, + prefix: string = pluginsModule.prefix as string, +): string[] => { + const names = entries + .map((e) => (e == null ? undefined : e.name)) + .filter( + (n): n is string => + typeof n === 'string' && n.startsWith(prefix) && n !== 'ep_etherpad-lite', + ); + return Array.from(new Set(names)); +}; + export const persistInstalledPlugins = async () => { const plugins:PackageData[] = [] const installedPlugins = {plugins: plugins}; diff --git a/bin/plugins.ts b/bin/plugins.ts index 5384fd599a6..a1f18233bc1 100644 --- a/bin/plugins.ts +++ b/bin/plugins.ts @@ -1,7 +1,7 @@ 'use strict'; import {linkInstaller, checkForMigration} from "ep_etherpad-lite/static/js/pluginfw/installer"; -import {persistInstalledPlugins} from "./commonPlugins"; +import {persistInstalledPlugins, filterUpdatablePluginNames} from "./commonPlugins"; import fs from "node:fs"; const settings = require('ep_etherpad-lite/node/utils/Settings'); @@ -80,14 +80,17 @@ const list = ()=>{ // Re-install every plugin in installed_plugins.json without a version pin so // the registry-latest gets resolved and overwrites the existing pinned copy -// in src/plugin_packages/. ep_etherpad-lite is excluded because the core is -// vendored, not plugin-installed. +// in src/plugin_packages/. ep_etherpad-lite is the vendored core, never +// installed via the plugin path. filterUpdatablePluginNames also enforces +// the ep_ prefix so a corrupted manifest cannot coerce us into installing +// arbitrary npm packages, and de-duplicates repeats. const update = ()=> { (async () => { const path = settings.root+"/var/installed_plugins.json"; - let plugins: {name: string}[]; + let entries: Array<{name?: unknown}>; try { - plugins = JSON.parse(fs.readFileSync(path, "utf-8")).plugins || []; + const parsed = JSON.parse(fs.readFileSync(path, "utf-8")); + entries = Array.isArray(parsed?.plugins) ? parsed.plugins : []; } catch (err: any) { if (err.code === 'ENOENT') { console.log("No installed_plugins.json found — nothing to update"); @@ -95,9 +98,7 @@ const update = ()=> { } throw err; } - const names = plugins - .map((p) => p.name) - .filter((n) => n && n !== 'ep_etherpad-lite'); + const names = filterUpdatablePluginNames(entries); if (names.length === 0) { console.log("No plugins installed — nothing to update"); return; diff --git a/src/tests/backend/specs/filterUpdatablePluginNames.ts b/src/tests/backend/specs/filterUpdatablePluginNames.ts new file mode 100644 index 00000000000..925e51078e6 --- /dev/null +++ b/src/tests/backend/specs/filterUpdatablePluginNames.ts @@ -0,0 +1,75 @@ +'use strict'; + +import {strict as assert} from 'assert'; +import {filterUpdatablePluginNames} from '../../../../bin/commonPlugins'; + +// Regression test for #6670: the bug fix in `pnpm run plugins update` reads +// var/installed_plugins.json and re-invokes the installer per entry. The +// filter that picks safe-to-install names lives in bin/commonPlugins so that +// it is testable in isolation — the script itself has top-level side +// effects. If the filter ever regresses (skipping ep_ validation, allowing +// ep_etherpad-lite back in, dropping de-dup, choking on bad shapes) these +// assertions fail before the broken behaviour can ship. +describe(__filename, function () { + it('keeps every ep_-prefixed plugin name verbatim', function () { + const out = filterUpdatablePluginNames([ + {name: 'ep_align'}, + {name: 'ep_markdown'}, + {name: 'ep_headings2'}, + ]); + assert.deepEqual(out, ['ep_align', 'ep_markdown', 'ep_headings2']); + }); + + it('drops ep_etherpad-lite because the core is vendored, not plugin-installed', function () { + const out = filterUpdatablePluginNames([ + {name: 'ep_etherpad-lite'}, + {name: 'ep_align'}, + ]); + assert.deepEqual(out, ['ep_align']); + }); + + it('rejects names without the ep_ prefix so a corrupted manifest cannot install arbitrary packages', function () { + const out = filterUpdatablePluginNames([ + {name: 'ep_align'}, + {name: 'malicious-package'}, + {name: 'lodash'}, + {name: '../../../etc/passwd'}, + ]); + assert.deepEqual(out, ['ep_align']); + }); + + it('de-duplicates repeated names so each plugin is installed at most once', function () { + const out = filterUpdatablePluginNames([ + {name: 'ep_align'}, + {name: 'ep_align'}, + {name: 'ep_markdown'}, + {name: 'ep_align'}, + ]); + assert.deepEqual(out, ['ep_align', 'ep_markdown']); + }); + + it('tolerates missing, null, or non-string name fields', function () { + const out = filterUpdatablePluginNames([ + {name: 'ep_align'}, + {}, + null, + undefined, + {name: null as unknown as string}, + {name: 42 as unknown as string}, + {name: 'ep_markdown'}, + ]); + assert.deepEqual(out, ['ep_align', 'ep_markdown']); + }); + + it('returns an empty array for an empty manifest', function () { + assert.deepEqual(filterUpdatablePluginNames([]), []); + }); + + it('honours a custom prefix when one is supplied (defends against hard-coding)', function () { + const out = filterUpdatablePluginNames( + [{name: 'foo_one'}, {name: 'ep_align'}, {name: 'foo_two'}], + 'foo_', + ); + assert.deepEqual(out, ['foo_one', 'foo_two']); + }); +});