Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions bin/commonPlugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
46 changes: 44 additions & 2 deletions bin/plugins.ts
Original file line number Diff line number Diff line change
@@ -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');

Expand All @@ -19,7 +19,9 @@ const possibleActions = [
"rm",
"remove",
"ls",
"list"
"list",
"up",
"update"
]

const install = ()=> {
Expand Down Expand Up @@ -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();
})();
}
Comment on lines +81 to +113
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. update() lacks regression test 📘 Rule violation ☼ Reliability

This bug fix adds new plugin update behavior but does not include an automated regression test that
would fail if the fix were reverted. This increases the risk of the updatePlugins regression
reappearing unnoticed.
Agent Prompt
## Issue description
A bug fix was made to `bin/updatePlugins.sh`/`bin/plugins.ts` to correctly update installed plugins, but there is no automated regression test to ensure this behavior remains correct.

## Issue Context
The regression test should fail if the update action returns to the prior broken behavior (for example, not reading `var/installed_plugins.json` or not invoking the installer for installed plugins).

## Fix Focus Areas
- bin/plugins.ts[81-112]
- bin/updatePlugins.sh[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 750430a: added src/tests/backend/specs/filterUpdatablePluginNames.ts covering the seven cases that would distinguish the fixed behaviour from the prior broken one — keep prefixed names, drop ep_etherpad-lite, reject non-prefixed entries (the security gate), de-duplicate repeats, tolerate missing/null/non-string name fields, empty input, and honour a custom prefix. The script-level entry point still has top-level side effects so the helper is what's exported and tested; if the filter ever regresses (drops the ep_ gate, lets ep_etherpad-lite back in, etc.) the existing Backend tests workflow will fail before the broken behaviour can ship.


const remove = (plugins: string[])=>{
const walk = async () => {
for (const plugin of plugins) {
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 1 addition & 9 deletions bin/updatePlugins.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
75 changes: 75 additions & 0 deletions src/tests/backend/specs/filterUpdatablePluginNames.ts
Original file line number Diff line number Diff line change
@@ -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']);
});
});
Loading