Mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2026-07-27 12:03:49 +00:00
Code Issues Projects Releases Wiki Activity

fixup: address Qodo review on #7776

Browse source 
Three issues flagged:

1. Indentation: outdented the continuation lines inside the new
   `if (settings.loadTest)` block from 10 spaces to 8 (one level
   from `logger.warn(`), matching 2-space indent rule for added
   code.

2. Warning scope: the original wording said only socket.io
   authn/authz is bypassed, but settings.loadTest short-circuits
   SecurityManager.checkAccess() which is called from both HTTP
   (padaccess, importexport) and socket.io (PadMessageHandler)
   paths. Reword to "SecurityManager.checkAccess() will bypass
   authentication and authorization for both HTTP and socket.io
   requests".

3. Misleading "fires once at startup" comment in
   SecurityManager.ts: the warning is logged from Settings.ts
   reloadSettings(), which is also called on admin restart and
   plugin install. Rephrase to "logged from Settings.ts during
   settings load/reload, not on every request".

All three issues are accurate. No behaviour change for the fix
itself; only comment + warning text.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-16 12:14:26 +01:00
parent 620c8b20cd
commit afb3a4a928
2 changed files with 8 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

10
src/node/db/SecurityManager.ts
View file

@ -76,11 +76,11 @@ exports.checkAccess = async (padID:string, sessionCookie:string, token:string, u
// Authentication and authorization checks.
// settings.loadTest just short-circuits authn/authz; the user-facing
// warning about this configuration choice fires once at startup, not
// per request (see Settings.ts). Re-logging it here was costing
// ~4% of process CPU in the 100-400 author dive sweep (#7756): the
// routed-console-warn went through log4js's clustering dispatch on
// every message.
// warning about this configuration choice is logged from Settings.ts
// during settings load/reload, not on every request. Re-logging it
// here was costing ~4% of process CPU in the 100-400 author dive
// sweep (#7756): the routed-console-warn went through log4js's
// clustering dispatch on every message.
if (!settings.loadTest && settings.requireAuthentication) {
if (userSettings == null) {
authLogger.debug('access denied: authentication is required');

5
src/node/utils/Settings.ts
View file

@ -1195,8 +1195,9 @@ export const reloadSettings = () => {
if (settings.loadTest) {
logger.warn(
'settings.loadTest is true: socket.io authentication and authorization checks ' +
'will be bypassed for every connection. Do NOT enable this in production.');
'settings.loadTest is true: SecurityManager.checkAccess() will bypass ' +
'authentication and authorization for both HTTP and socket.io requests. ' +
'Do NOT enable this in production.');
}
if (!settings.skinName) {