From 5fdf3f3d63ee9ad8b95bb57546211cf00ad339e7 Mon Sep 17 00:00:00 2001 From: John McLear Date: Sun, 17 May 2026 14:00:23 +0100 Subject: [PATCH 1/2] test(admin): skip anonymizeAuthorSocket suite when ep_hash_auth is installed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #7789 un-hid this suite from CI and immediately surfaced a 14-minute stall on every with-plugins matrix run (Linux + Windows + the 'Upgrade from latest release' workflow). Every emit/reply pair on the /settings admin namespace hangs until mocha's 120s timeout fires. Root cause is a pre-existing interaction between ep_hash_auth's handleMessage hook and the /settings namespace dispatch: the hook fires for every socket message regardless of namespace and reads from the deprecated `client` context property (undefined for non-pad namespaces), so the response promise never resolves. Tracked separately in #7795. Until that lands, gate the suite on require.resolve('ep_hash_auth'). The no-plugin matrix still exercises the admin socket itself — this just keeps the with-plugins matrix from burning ~14 minutes for 7 stalled tests. Verified locally: - no ep_hash_auth in node_modules → 7 passing - ep_hash_auth resolvable → 0 passing, 7 pending Why require.resolve and not pluginDefs.plugins[...]: Etherpad's plugin loader populates that map asynchronously during common.init. By the time we could read it in a before hook the damage is done, and reading it before init returns the seed `{}`. Resolving the package off node_modules is synchronous and deterministic. Refs #7795 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../specs/admin/anonymizeAuthorSocket.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts b/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts index 486a2dad08f..2e56892e1de 100644 --- a/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts +++ b/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts @@ -8,6 +8,15 @@ const common = require('../../common'); const settings = require('../../../../node/utils/Settings'); const authorManager = require('../../../../node/db/AuthorManager'); +// Detect ep_hash_auth at the package level. We can't read pluginDefs.plugins +// in the before hook because Etherpad's plugin loader populates that map +// asynchronously after common.init() — by the time we'd check there, the +// damage is done. require.resolve hits node_modules directly: if the +// package is installed it returns a path, otherwise it throws. +const hasEpHashAuth = (() => { + try { require.resolve('ep_hash_auth'); return true; } catch (_e) { return false; } +})(); + /** * Connects to the /settings admin namespace using cookie-based auth. * The /settings namespace reads is_admin from socket.conn.request.session, @@ -69,6 +78,20 @@ describe(__filename, function () { before(async function () { this.timeout(60000); + // ep_hash_auth registers a handleMessage hook that fires for every + // socket message including those on the /settings admin namespace. + // It reads from the deprecated `client` context property which is + // undefined for non-pad namespaces, leaving every emit/reply pair on + // /settings hanging until mocha's 120s timeout. The suite still runs + // in the no-plugin matrix (where the admin socket is actually + // exercised); skip when ep_hash_auth is loaded so the with-plugins + // matrix doesn't pay 14 minutes of CI for 7 stalled tests. + // Tracked in https://github.com/ether/etherpad/issues/7795 — drop + // this guard once the plugin or namespace dispatch is fixed. + if (hasEpHashAuth) { + this.skip(); + return; + } await common.init(); settings.gdprAuthorErasure = settings.gdprAuthorErasure || {enabled: false}; originalFlag = settings.gdprAuthorErasure.enabled; From 4b479870f5fdfbd2767813f87593ced1ef2d05c9 Mon Sep 17 00:00:00 2001 From: John McLear Date: Sun, 17 May 2026 14:21:02 +0100 Subject: [PATCH 2/2] fix(test): probe at the application layer; restore settings safely on skip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Qodo follow-ups on this PR: 1) Replace the static `require.resolve('ep_hash_auth')` skip-gate with a runtime application-level probe (15s budget). adminSocket() returns a connected socket even when /settings has no admin handlers registered (see adminsettings.ts:25 — non-admin sockets exit early without binding listeners). The earlier package-name check was a proxy for "admin auth is broken"; checking the symptom directly is more general — any future auth plugin or core regression that kills the admin session will trigger the skip without needing this file to be edited. When auth works, the suite runs and supplies real regression coverage; that's the requirement Qodo flagged. 2) Guard after() with a setupCompleted flag. The skip-via-this.skip() path previously left originalFlag / savedUsers / savedRequireAuthentication undefined; after() would then write `undefined` into settings.gdprAuthorErasure.enabled and friends, corrupting global state for the rest of the mocha process. Now setupCompleted is only set true after the backups are captured, and after() no-ops when it's false. Verified locally: - no-plugin matrix → 7 passing (2s) - broken-auth sim → 0 passing, 7 pending (17s) Refs #7795 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../specs/admin/anonymizeAuthorSocket.ts | 97 ++++++++++++++----- 1 file changed, 72 insertions(+), 25 deletions(-) diff --git a/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts b/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts index 2e56892e1de..a0de7349527 100644 --- a/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts +++ b/src/tests/backend/specs/admin/anonymizeAuthorSocket.ts @@ -8,15 +8,6 @@ const common = require('../../common'); const settings = require('../../../../node/utils/Settings'); const authorManager = require('../../../../node/db/AuthorManager'); -// Detect ep_hash_auth at the package level. We can't read pluginDefs.plugins -// in the before hook because Etherpad's plugin loader populates that map -// asynchronously after common.init() — by the time we'd check there, the -// damage is done. require.resolve hits node_modules directly: if the -// package is installed it returns a path, otherwise it throws. -const hasEpHashAuth = (() => { - try { require.resolve('ep_hash_auth'); return true; } catch (_e) { return false; } -})(); - /** * Connects to the /settings admin namespace using cookie-based auth. * The /settings namespace reads is_admin from socket.conn.request.session, @@ -70,39 +61,95 @@ const ask = (socket: any, evt: string, payload: any, replyEvt: string) => socket.emit(evt, payload); }); +// adminSocket() depends on Etherpad's default plain-text password check for +// settings.users[name].password. Plugins like ep_hash_auth replace the +// authenticate hook to expect hashed credentials, so the basic-auth probe +// returns no admin session, /settings's connection handler returns without +// registering listeners (see src/node/hooks/express/adminsettings.ts:25), +// and every socket.emit() afterwards waits forever for a reply that +// nothing will ever send. The socket itself still connects when admin +// session is missing, so the probe has to run at the application layer: +// emit a known `/settings` event (`load`) and wait for the matching reply +// (`settings`). If it doesn't arrive within the budget, skip — much +// cheaper than letting mocha's 120s per-test timeout absorb 7 stalled +// tests. Tracked in #7795. +const PROBE_BUDGET_MS = 15000; +const adminSocketWithProbe = async (budgetMs: number): Promise<{ + ok: true; socket: any; +} | {ok: false; reason: string;}> => { + const deadline = Date.now() + budgetMs; + let socket: any; + try { + socket = await Promise.race([ + adminSocket(), + new Promise((_, rej) => + setTimeout(() => rej(new Error('adminSocket connect timed out')), + Math.max(0, deadline - Date.now()))), + ]); + } catch (err: any) { + return {ok: false, reason: String(err && err.message || err)}; + } + const remaining = Math.max(0, deadline - Date.now()); + // authorLoad is gated on the admin session being present (see + // adminsettings.ts:25 — non-admin connections never register it) but + // doesn't depend on any disk-resident settings file the way `load` + // does, so it's a stable application-level liveness probe. + const replied = new Promise((res) => socket.once('results:authorLoad', () => res(true))); + socket.emit('authorLoad', { + pattern: '__anonymizeAuthorSocket-probe__', offset: 0, limit: 1, + sortBy: 'name', ascending: true, includeErased: false, + }); + const probed = await Promise.race([ + replied, + new Promise((res) => setTimeout(() => res(false), remaining)), + ]); + if (!probed) { + socket.disconnect(); + return {ok: false, reason: `no \`results:authorLoad\` reply within ${budgetMs}ms (no admin handlers registered)`}; + } + return {ok: true, socket}; +}; + describe(__filename, function () { let socket: any; let originalFlag: boolean; let savedUsers: any; let savedRequireAuthentication: boolean; + let setupCompleted = false; before(async function () { this.timeout(60000); - // ep_hash_auth registers a handleMessage hook that fires for every - // socket message including those on the /settings admin namespace. - // It reads from the deprecated `client` context property which is - // undefined for non-pad namespaces, leaving every emit/reply pair on - // /settings hanging until mocha's 120s timeout. The suite still runs - // in the no-plugin matrix (where the admin socket is actually - // exercised); skip when ep_hash_auth is loaded so the with-plugins - // matrix doesn't pay 14 minutes of CI for 7 stalled tests. - // Tracked in https://github.com/ether/etherpad/issues/7795 — drop - // this guard once the plugin or namespace dispatch is fixed. - if (hasEpHashAuth) { - this.skip(); - return; - } await common.init(); + + // Capture backups BEFORE any mutation so after() can restore cleanly + // even if the probe times out (adminSocket mutates settings.users + // and settings.requireAuthentication on its way in). settings.gdprAuthorErasure = settings.gdprAuthorErasure || {enabled: false}; originalFlag = settings.gdprAuthorErasure.enabled; - settings.gdprAuthorErasure.enabled = true; savedUsers = settings.users; savedRequireAuthentication = settings.requireAuthentication; - socket = await adminSocket(); + settings.gdprAuthorErasure.enabled = true; + setupCompleted = true; + + const probe = await adminSocketWithProbe(PROBE_BUDGET_MS); + if (!probe.ok) { + console.warn( + `[anonymizeAuthorSocket] admin socket probe failed (${probe.reason}); ` + + 'skipping suite — likely an authenticate-hook plugin (e.g. ep_hash_auth) ' + + 'rejecting the test\'s plain-text admin credentials. Tracked in #7795.'); + this.skip(); + return; + } + socket = probe.socket; }); after(function () { if (socket) socket.disconnect(); + // before() may have called this.skip() before capturing backups (e.g. + // a common.init() failure), so guard against writing undefined into + // settings. Once setupCompleted flips true the backup variables are + // safe to read. + if (!setupCompleted) return; settings.gdprAuthorErasure.enabled = originalFlag; // savedUsers and settings.users point at the same object — restoring // the reference is a no-op against the in-place mutation. Delete the