• Move loadTest warning from per-message to startup initialization
• Eliminate ~4% CPU overhead in SecurityManager.checkAccess logging
• Simplify authentication check logic by inverting condition
• Add startup warning in Settings.ts alongside other config warnings

Remove per-message loadTest warning from checkAccess

• Removed per-message console.warn call that fired on every checkAccess invocation
• Simplified authentication check by inverting condition logic (if !settings.loadTest &&
 settings.requireAuthentication)
• Added detailed comment explaining the performance issue and rationale for the change
• Preserved loadTest short-circuit behavior without logging overhead

src/node/db/SecurityManager.ts

Add startup warning for loadTest configuration

• Added one-time startup warning when settings.loadTest is enabled
• Warning placed in reloadSettings initialization alongside other config-time warnings
• Includes production safety notice about authentication/authorization bypass
• Eliminates need for per-request logging

src/node/utils/Settings.ts

Description

New code in Settings.ts uses 4+ space indentation rather than the required 2-space indentation.
This introduces formatting inconsistency and violates the no-tabs/2-space indentation standard for
modified lines.

Code

src/node/utils/Settings.ts[R1196-1200]

+ 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.');
+ }

Evidence

PR Compliance ID 3 requires spaces-only with 2-space indentation for all modified/added lines. The
added if (settings.loadTest) { block and its wrapped logger.warn(...) lines are indented with
more than 2 spaces in the modified region cited below.

src/node/utils/Settings.ts[1196-1200]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Modified lines in `src/node/utils/Settings.ts` use indentation that is not 2-space (e.g., the added `if (settings.loadTest)` block is indented with 4+ spaces). The compliance checklist requires spaces-only and 2-space indentation for all changed/added code.
## Issue Context
This PR adds a startup `logger.warn(...)` when `settings.loadTest` is enabled. The logic is fine, but the indentation of the newly added lines does not match the 2-space requirement.
## Fix Focus Areas
- src/node/utils/Settings.ts[1196-1200]

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new Settings.reloadSettings() warning says only socket.io authn/authz is bypassed, but
settings.loadTest short-circuits SecurityManager.checkAccess() which is used by both socket.io
and HTTP handlers. This warning therefore understates the security-impact surface area of enabling
loadTest.

Code

src/node/utils/Settings.ts[R1196-1200]

+ 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.');
+ }

Evidence

The warning text is socket.io-specific, but settings.loadTest gates enforcement in
SecurityManager.checkAccess(), and that function is called from both socket.io handling
(PadMessageHandler) and HTTP endpoints (padaccess, importexport).

src/node/utils/Settings.ts[1196-1200]
src/node/db/SecurityManager.ts[60-101]
src/node/handler/PadMessageHandler.ts[496-508]
src/node/padaccess.ts[6-14]
src/node/hooks/express/importexport.ts[77-92]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The warning message for `settings.loadTest` currently claims that only socket.io authentication/authorization checks are bypassed. In reality, `settings.loadTest` affects `SecurityManager.checkAccess()`, which is used by both socket.io message handling and HTTP routes (e.g., pad access and import/export).
### Issue Context
Accurate operator-facing warnings matter for a security-bypass setting like `loadTest`. The current wording can mislead an operator into thinking the bypass is limited to socket.io.
### Fix Focus Areas
- src/node/utils/Settings.ts[1196-1200]
- src/node/db/SecurityManager.ts[60-101]
- src/node/handler/PadMessageHandler.ts[496-508]
- src/node/padaccess.ts[6-14]
- src/node/hooks/express/importexport.ts[77-92]
### Suggested fix
Adjust the warning string to reflect the real scope, e.g.:
- "settings.loadTest is true: authentication/authorization checks in SecurityManager.checkAccess() will be bypassed (affects both HTTP and socket.io). Do NOT enable this in production."
Optionally mention that it applies to pad access checks generally, not a single transport.

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

SecurityManager.checkAccess() now states the loadTest warning "fires once at startup", but the
warning is emitted from reloadSettings(), which is also called on admin restart and after plugin
install. This inaccurate comment can mislead future maintainers about when the warning will appear.

Code

src/node/db/SecurityManager.ts[R78-80]

+ // 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

Evidence

The comment explicitly claims the warning fires only at startup, but the warning is implemented
inside reloadSettings(), and reloadSettings() is invoked from runtime code paths (admin restart
and plugin install completion).

src/node/db/SecurityManager.ts[77-85]
src/node/utils/Settings.ts[1187-1201]
src/node/hooks/express/adminsettings.ts[395-401]
src/static/js/pluginfw/installer.ts[29-35]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
A comment in `SecurityManager.checkAccess()` claims the `settings.loadTest` warning "fires once at startup", but the warning is logged from `reloadSettings()`, which can be called multiple times during runtime (admin restart, plugin operations). This makes the comment factually incorrect.
### Issue Context
The PR moved the warning from the per-message hot path to settings reload time, which is good. However, the comment now over-specifies behavior that isn’t true.
### Fix Focus Areas
- src/node/db/SecurityManager.ts[77-85]
- src/node/utils/Settings.ts[1192-1201]
- src/node/hooks/express/adminsettings.ts[395-401]
- src/static/js/pluginfw/installer.ts[29-35]
### Suggested fix
Update the comment to something like: "the warning is logged during settings load/reload (see Settings.ts), not per request" (or remove the timing claim entirely).

i Copy this prompt and use it to remediate the issue with your preferred AI generation tools