From 4495a46d81cf03c8dbfd9220b2ca62c3c62dd275 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 24 Aug 2026 13:38:07 +0100 Subject: [PATCH] fix(settings): expose plugin ep_* config blocks to CJS require (#8110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugins read their own configuration from a top-level `ep_*` block in settings.json via `require('ep_etherpad-lite/node/utils/Settings')`. The CJS-compatibility shim added in #7421 installs accessor properties on `module.exports` for `Object.keys(settings)` — but it ran exactly once, at module-evaluation time, and `ep_*` blocks are only merged onto the settings object later, by the `reloadSettings()` call at the bottom of that same module. No accessor was ever defined for them, so every plugin config block was invisible to the require() path; the value was reachable only under `.default`. Consequence: every plugin that reads `settings.ep_` silently ran on its built-in defaults. The reported symptom is ep_hash_auth, whose `hash_dir` reverted to `/var/etherpad/users`, so every hash lookup failed, the `authenticate` hook returned false, core's basic-auth fallback found no `password` on the settings.json user, and admin login answered 401. Note the `authenticate` hook itself was never the problem — /admin-auth/ is handled by webaccess.checkAccess like any other path and does call the hook. Extract the shim into `syncCjsExports()` and re-run it at the end of `reloadSettings()`, so keys that only exist because the operator put them in settings.json get accessors as soon as they are loaded. Reported by @tris-ots. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013WTrZxkTJhiH1p7RuAx3NJ --- CHANGELOG.md | 1 + src/node/utils/Settings.ts | 33 ++++++++++++++++++-------- src/tests/backend/specs/settings.ts | 36 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df5624d9ba9..5250965c23d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Notable fixes +- **Plugins — `settings.ep_` config blocks are reachable again from `require()` (#8110).** Plugins read their own configuration out of a top-level `ep_*` block in `settings.json` via `require('ep_etherpad-lite/node/utils/Settings')`. The CJS-compatibility shim in `Settings.ts` installed accessor properties on `module.exports` for the keys present on the settings object *while that module was still evaluating* — but `ep_*` blocks are only merged in later, by the `reloadSettings()` call at the bottom of the same module. Every plugin config block was therefore invisible to the `require()` path (the value was reachable only under `.default`), so plugins silently fell back to their built-in defaults. For `ep_hash_auth` that meant `hash_dir` reverted to `/var/etherpad/users`, every hash lookup failed, and admin login returned 401 with no usable diagnostic — the symptom that surfaced this. The shim is now re-run after each settings load. Reported by @tris-ots. - **API — `movePad` now carries the pad's deletion token to the new id (#7995).** `movePad` is implemented as `copy()` + `remove()`, but `Pad.copy()` only copies the `pad:`, `:revs:N` and `:chat:N` records — never `pad::deletionToken` — and `remove()` then deleted the source pad's token. The renamed pad therefore had no token at all: the token the creator had been told to save no longer deleted anything, and because the copy keeps the same revision-0 author, their next visit tripped `createDeletionTokenIfAbsent()` and popped a second "save your pad deletion token" modal. The token record is now handed over to the destination as part of the move, so the saved token keeps working and the modal does not reappear. `force`-overwriting an existing destination discards that pad's own token along with its content. `copyPad` is deliberately unchanged — two pads sharing one secret would let a token saved for one delete the other. # 3.3.3 diff --git a/src/node/utils/Settings.ts b/src/node/utils/Settings.ts index f1ed4e4e085..611a4f35a60 100644 --- a/src/node/utils/Settings.ts +++ b/src/node/utils/Settings.ts @@ -911,19 +911,27 @@ export const getPublicPrivacyBanner = () => ({ export default settings; // CJS compatibility: plugins use require('ep_etherpad-lite/node/utils/Settings') // and expect settings properties directly on the module object, not under .default -if (typeof module !== 'undefined' && module.exports) { +// +// Must be re-run after every settings load: keys that only exist because the +// operator put them in settings.json — notably the top-level `ep_*` blocks that +// plugins read their own configuration from (ep_hash_auth, ep_ldapauth, …) — +// are not present on `settings` while this module is still evaluating, so a +// one-shot pass at module scope would leave them permanently invisible to +// require() consumers (ether/etherpad#8110). +export const syncCjsExports = () => { + if (typeof module === 'undefined' || !module.exports) return; const currentExports = module.exports; for (const key of Object.keys(settings)) { - if (!(key in currentExports)) { - Object.defineProperty(currentExports, key, { - get: () => (settings as any)[key], - set: (v: any) => { (settings as any)[key] = v; }, - enumerable: true, - configurable: true, - }); - } + if (key in currentExports) continue; + Object.defineProperty(currentExports, key, { + get: () => (settings as any)[key], + set: (v: any) => { (settings as any)[key] = v; }, + enumerable: true, + configurable: true, + }); } -} +}; +syncCjsExports(); /** * This setting is passed with dbType to ueberDB to set up the database @@ -1454,6 +1462,11 @@ export const reloadSettings = () => { .slice(0, 8); } logger.info(`String used for versioning assets: ${settings.randomVersionString}`); + + // Expose any newly-seen top-level keys (plugin `ep_*` blocks, …) on + // module.exports so `require('ep_etherpad-lite/node/utils/Settings')` + // sees them. See syncCjsExports() above. + syncCjsExports(); }; export const exportedForTestingOnly = { diff --git a/src/tests/backend/specs/settings.ts b/src/tests/backend/specs/settings.ts index 4409d0910b4..be3a9bf9102 100644 --- a/src/tests/backend/specs/settings.ts +++ b/src/tests/backend/specs/settings.ts @@ -4,6 +4,8 @@ const assert = require('assert').strict; import {exportedForTestingOnly} from '../../../node/utils/Settings' import path from 'path'; import process from 'process'; +import fs from 'fs'; +import os from 'os'; describe(__filename, function () { describe('parseSettings', function () { @@ -146,6 +148,40 @@ describe(__filename, function () { cjs.title = original; } }); + + // Regression test for ether/etherpad#8110. + // Plugin configuration lives in top-level `ep_*` blocks in settings.json + // (ep_hash_auth.hash_dir, ep_ldapauth.url, …). Those keys don't exist on + // the settings object while Settings.ts is still evaluating, so a shim + // that only ran once at module scope never defined accessors for them and + // every plugin silently fell back to its built-in defaults — for + // ep_hash_auth that meant reading hashes from /var/etherpad/users and + // rejecting every admin login with a 401. + it('exposes plugin ep_* blocks added by a later reloadSettings()', function () { + const settingsMod = require('../../../node/utils/Settings'); + const savedSettingsFile = settingsMod.settingsFilename; + const savedCredsFile = settingsMod.credentialsFilename; + const tmpFile = path.join(os.tmpdir(), `ep-8110-settings-${process.pid}.json`); + fs.writeFileSync(tmpFile, JSON.stringify({ + ep_regression_8110: {hash_dir: '/srv/etherpad/users'}, + })); + settingsMod.settingsFilename = tmpFile; + settingsMod.credentialsFilename = path.join(os.tmpdir(), 'ep-8110-no-credentials.json'); + try { + settingsMod.reloadSettings(); + assert.deepEqual(settingsMod.ep_regression_8110, {hash_dir: '/srv/etherpad/users'}, + 'plugin ep_* settings must be reachable via CJS require, not just via .default'); + } finally { + // Drop the key from the shared settings object as well as the accessor + // the shim installed on module.exports, so later specs see a clean slate. + delete (settingsMod.default || settingsMod).ep_regression_8110; + delete settingsMod.ep_regression_8110; + settingsMod.settingsFilename = savedSettingsFile; + settingsMod.credentialsFilename = savedCredsFile; + fs.rmSync(tmpFile, {force: true}); + settingsMod.reloadSettings(); + } + }); }); // Regression test for https://github.com/ether/etherpad/issues/7213.