fix(plugins): updatePlugins.sh actually updates installed plugins (closes #6670) (#7644)

* fix(plugins): updatePlugins.sh actually updates installed plugins (#6670)

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/<name>@<version>/ 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) <noreply@anthropic.com>

* fix(plugins): validate ep_ prefix and dedupe + add regression test

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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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};

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');
 
@@ -19,7 +19,9 @@ const possibleActions = [
   "rm",
   "remove",
   "ls",
-  "list"
+  "list",
+  "up",
+  "update"
 ]
 
 const install = ()=> {
@@ -76,6 +78,40 @@ 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 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 entries: Array<{name?: unknown}>;
+    try {
+      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");
+        return;
+      }
+      throw err;
+    }
+    const names = filterUpdatablePluginNames(entries);
+    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 +148,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);

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

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']);
+  });
+});