fix(settings): expose plugin ep_* config blocks to CJS require (#8110)

CHANGELOG.md

@@ -1,3 +1,9 @@
+# 3.3.6
+
+### Notable fixes
+
+- **Plugins — `settings.ep_<plugin>` config blocks are reachable again from `require()` (#8109, #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 @mathewcsims and @tris-ots; an equivalent fix was also proposed by @AkprasadoP in #8113.
+
 # 3.3.5
 
 3.3.5 is a bug-fix follow-up to 3.3.4. It fixes a startup crash on fresh installs when pnpm 12 (now pnpm's default release) is installed, and makes the built-in updater work on Windows.

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 = {

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,46 @@ describe(__filename, function () {
         cjs.title = original;
       }
     });
+
+    // Regression test for ether/etherpad#8109 / #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');
+        assert.ok(Object.keys(settingsMod).includes('ep_regression_8110'),
+            'plugin ep_* key must be enumerable on the CJS module export');
+        if (settingsMod.default != null) {
+          assert.strictEqual(settingsMod.ep_regression_8110, settingsMod.default.ep_regression_8110,
+              'CJS accessor must return the live settings object, not a copy');
+        }
+      } 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.