fix(security): harden auth surfaces (WS credential, MCP default, non-REST audit)#601
Merged
Conversation
…REST audit) - Stop accepting the WebSocket API key from the query string — it leaks the credential into proxy/ access logs. Only the Socket.IO handshake auth field and the X-API-Key header are accepted now. (Behavior change: clients using ?apiKey= must switch to auth.apiKey or the header.) - Default the MCP server to READ-ONLY: write tools are exposed only on an explicit MCP_READONLY=false. Previously an unset MCP_READONLY defaulted to read-write, silently exposing state-changing tools the moment MCP_ENABLED was on. (Behavior change: set MCP_READONLY=false to keep write tools.) - Audit rejected WebSocket auth attempts (missing + invalid key) with the same API_KEY_AUTH_FAILED event the REST guard emits, so credential probing over the WS surface leaves a forensic trail. - Add a server-side role-coverage test locking that the ADMIN-only integration surfaces are guarded server-side (the dashboard role is cosmetic UX only). Docs updated for both behavior changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Auth-surface hardening across the WebSocket, MCP, and audit paths. Contains two behavior changes (⚠️ below) — targeted for a minor release.
Changes
?apiKey=handshake fallback leaked the credential into proxy/access logs. Only the Socket.IO handshakeauth.apiKeyfield and theX-API-Keyheader are accepted now. Clients using?apiKey=must switch (docs updated).MCP_READONLY=false. Previously an unsetMCP_READONLYdefaulted to read-write, silently exposing state-changing tools (send messages, group ops) the momentMCP_ENABLEDwas on. SetMCP_READONLY=falseto keep the old behavior (docs updated).API_KEY_AUTH_FAILEDevent the REST guard emits, so probing over the WS surface leaves a forensic trail.Verification
npm run build✓ ·npm run lint✓ ·npm test✓ (1894/1894, +8). New tests: WS ignores a query-string key, WS audits a rejected attempt, MCP read-only secure default resolution, and admin controllers require the ADMIN role.Follow-up
Auditing MCP and Bull-Board auth failures (raw-Express surfaces) is left as a separate change.