Skip to content

Commit 4495a46

Browse files
JohnMcLearclaude
andcommitted
fix(settings): expose plugin ep_* config blocks to CJS require (#8110)
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_<name>` 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WTrZxkTJhiH1p7RuAx3NJ
1 parent d4d7a61 commit 4495a46

3 files changed

Lines changed: 60 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
### Notable fixes
44

5+
- **Plugins — `settings.ep_<plugin>` 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.
56
- **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:<id>`, `:revs:N` and `:chat:N` records — never `pad:<id>: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.
67

78
# 3.3.3

src/node/utils/Settings.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -911,19 +911,27 @@ export const getPublicPrivacyBanner = () => ({
911911
export default settings;
912912
// CJS compatibility: plugins use require('ep_etherpad-lite/node/utils/Settings')
913913
// and expect settings properties directly on the module object, not under .default
914-
if (typeof module !== 'undefined' && module.exports) {
914+
//
915+
// Must be re-run after every settings load: keys that only exist because the
916+
// operator put them in settings.json — notably the top-level `ep_*` blocks that
917+
// plugins read their own configuration from (ep_hash_auth, ep_ldapauth, …) —
918+
// are not present on `settings` while this module is still evaluating, so a
919+
// one-shot pass at module scope would leave them permanently invisible to
920+
// require() consumers (ether/etherpad#8110).
921+
export const syncCjsExports = () => {
922+
if (typeof module === 'undefined' || !module.exports) return;
915923
const currentExports = module.exports;
916924
for (const key of Object.keys(settings)) {
917-
if (!(key in currentExports)) {
918-
Object.defineProperty(currentExports, key, {
919-
get: () => (settings as any)[key],
920-
set: (v: any) => { (settings as any)[key] = v; },
921-
enumerable: true,
922-
configurable: true,
923-
});
924-
}
925+
if (key in currentExports) continue;
926+
Object.defineProperty(currentExports, key, {
927+
get: () => (settings as any)[key],
928+
set: (v: any) => { (settings as any)[key] = v; },
929+
enumerable: true,
930+
configurable: true,
931+
});
925932
}
926-
}
933+
};
934+
syncCjsExports();
927935

928936
/**
929937
* This setting is passed with dbType to ueberDB to set up the database
@@ -1454,6 +1462,11 @@ export const reloadSettings = () => {
14541462
.slice(0, 8);
14551463
}
14561464
logger.info(`String used for versioning assets: ${settings.randomVersionString}`);
1465+
1466+
// Expose any newly-seen top-level keys (plugin `ep_*` blocks, …) on
1467+
// module.exports so `require('ep_etherpad-lite/node/utils/Settings')`
1468+
// sees them. See syncCjsExports() above.
1469+
syncCjsExports();
14571470
};
14581471

14591472
export const exportedForTestingOnly = {

src/tests/backend/specs/settings.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ const assert = require('assert').strict;
44
import {exportedForTestingOnly} from '../../../node/utils/Settings'
55
import path from 'path';
66
import process from 'process';
7+
import fs from 'fs';
8+
import os from 'os';
79

810
describe(__filename, function () {
911
describe('parseSettings', function () {
@@ -146,6 +148,40 @@ describe(__filename, function () {
146148
cjs.title = original;
147149
}
148150
});
151+
152+
// Regression test for ether/etherpad#8110.
153+
// Plugin configuration lives in top-level `ep_*` blocks in settings.json
154+
// (ep_hash_auth.hash_dir, ep_ldapauth.url, …). Those keys don't exist on
155+
// the settings object while Settings.ts is still evaluating, so a shim
156+
// that only ran once at module scope never defined accessors for them and
157+
// every plugin silently fell back to its built-in defaults — for
158+
// ep_hash_auth that meant reading hashes from /var/etherpad/users and
159+
// rejecting every admin login with a 401.
160+
it('exposes plugin ep_* blocks added by a later reloadSettings()', function () {
161+
const settingsMod = require('../../../node/utils/Settings');
162+
const savedSettingsFile = settingsMod.settingsFilename;
163+
const savedCredsFile = settingsMod.credentialsFilename;
164+
const tmpFile = path.join(os.tmpdir(), `ep-8110-settings-${process.pid}.json`);
165+
fs.writeFileSync(tmpFile, JSON.stringify({
166+
ep_regression_8110: {hash_dir: '/srv/etherpad/users'},
167+
}));
168+
settingsMod.settingsFilename = tmpFile;
169+
settingsMod.credentialsFilename = path.join(os.tmpdir(), 'ep-8110-no-credentials.json');
170+
try {
171+
settingsMod.reloadSettings();
172+
assert.deepEqual(settingsMod.ep_regression_8110, {hash_dir: '/srv/etherpad/users'},
173+
'plugin ep_* settings must be reachable via CJS require, not just via .default');
174+
} finally {
175+
// Drop the key from the shared settings object as well as the accessor
176+
// the shim installed on module.exports, so later specs see a clean slate.
177+
delete (settingsMod.default || settingsMod).ep_regression_8110;
178+
delete settingsMod.ep_regression_8110;
179+
settingsMod.settingsFilename = savedSettingsFile;
180+
settingsMod.credentialsFilename = savedCredsFile;
181+
fs.rmSync(tmpFile, {force: true});
182+
settingsMod.reloadSettings();
183+
}
184+
});
149185
});
150186

151187
// Regression test for https://github.com/ether/etherpad/issues/7213.

0 commit comments

Comments
 (0)