fix(security): throttle failed logins; add security headers to the API - #135
Merged
Merged
Conversation
Login throttling: the local and LDAP credential logins had no limit (50 wrong passwords against admin took under 5 seconds, and the right one still worked straight after). A new in-memory throttle counts failed sign-ins in a 15-minute window and answers 429 with Retry-After: - 5 failures per client IP + username (case-insensitive), and - 50 failures per client IP across usernames. There is deliberately no per-username lock, which would let anyone lock an account (e.g. admin) out by failing to sign in as it. A successful sign-in clears that client's count. The limits and window are configurable with ANGLES_LOGIN_MAX_FAILURES, ANGLES_LOGIN_MAX_FAILURES_PER_IP and ANGLES_LOGIN_LOCKOUT_MINUTES. Behind a reverse proxy, TRUST_PROXY=true is needed for the client's real address to be used. Headers: every API response now carries X-Content-Type-Options: nosniff, X-Frame-Options: DENY, Referrer-Policy: no-referrer and Content-Security-Policy "default-src 'none'; frame-ancestors 'none'", and X-Powered-By is no longer sent. The Swagger UI keeps its scripts (CSP frame-ancestors only). The HTML build report gets its own policy: inline styles, data: images, and only its one script, by a per-request nonce. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KgQXSUjuLVXmLWxobnMfSf
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
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
Fixes two findings from the security review: no login rate limiting, and no clickjacking or security headers. The UI half of the headers fix is in AnglesHQ/angles-ui (same branch name).
Login throttling (
app/utils/login-throttle.js)Before: the local and LDAP credential logins had no limit. 50 wrong passwords against
admintook under 5 seconds, and the correct password still worked straight after.Now: failed sign-ins are counted within a 15-minute window. Once over a limit, the login answers 429 with a
Retry-Afterheader.ANGLES_LOGIN_MAX_FAILURES,ANGLES_LOGIN_MAX_FAILURES_PER_IPandANGLES_LOGIN_LOCKOUT_MINUTES.TRUST_PROXY=trueso the client's real address is used. Otherwise every request comes from the proxy and shares the per-IP limit.Security headers (
app/utils/security-headers.js)Every API response now sends:
X-Content-Type-Options: nosniffX-Frame-Options: DENYReferrer-Policy: no-referrerContent-Security-Policy: default-src 'none'; frame-ancestors 'none'. The API answers with JSON and files, so a response that ever gets rendered as a page can't load or run anything.X-Powered-Byis no longer sent.Swagger UI (
/api-docs) getsframe-ancestors 'none'only, so its own scripts and styles keep working.HTML build report gets its own policy: inline styles,
data:images, and only its single inline script, allowed by a per-request nonce.Attachment files keep the sandbox policy they already set.
Testing
test/security-hardening.tests.js(11 tests):npx eslint app server.jsis clean. The existing suite's deliberate failed logins don't trip the limits.Notes
server.js, where they edit different places.🤖 Generated with Claude Code
https://claude.ai/code/session_01KgQXSUjuLVXmLWxobnMfSf
Generated by Claude Code