fix: socket reconnection loop and trust proxy validation error#4604
Merged
Conversation
Fix infinite connect/disconnect loop when running behind a reverse proxy (e.g. Home Assistant ingress) by addressing multiple contributing factors: - Add connect_error handler to detect auth failures and redirect to login instead of silently retrying with a stale token forever - Check JWT expiry client-side before attempting socket connection - Suppress ERR_ERL_UNEXPECTED_X_FORWARDED_FOR errors from express-rate-limit when TRUST_PROXY is not explicitly set - Auto-detect X-Forwarded-For header and enable trust proxy automatically for environments where users cannot set env vars (e.g. HA Green) Closes #4564 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Coverage Report for CI Build 25097969982Coverage decreased (-0.02%) to 18.285%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
…rror A single connect_error with "Authentication error" can happen transiently (server restart, temporary network issues). Only force logout after 3 consecutive auth failures. Reset counter on successful connect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use proxyDetected flag to short-circuit middleware after first detection - Rename _req to req since the parameter is used (read headers) - Add security note about X-Forwarded-For spoofing risk - Add comment referencing server auth middleware for error string match Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Has this made it into the release yet? |
Member
Author
|
@DenverOps Nope, I have to do some tests before merging but didn't have time yet |
|
Thank you sir. No pressure was just trying to understand if I should test on my end on this next release and I didn't see it in the release notes si figured I'd ask. |
Drops the X-Forwarded-For sniffing middleware that flipped `trust proxy` to `true` on the first proxied request: any client can spoof XFF, which would let attackers bypass the login rate-limiter. Also drops the blanket `xForwardedForHeader: false` validator suppression — when trust proxy is correctly configured, the warning no longer fires on its own. Replaces both with: - `trustProxy?: string` on `GatewayConfig` (hop count, IP/CIDR list, or `loopback`/`linklocal`/`uniquelocal`). - A `Trust Proxy` text field in the General settings panel. - `configureTrustProxy()` in `startServer`: env var wins, falls back to settings; coerces `"true"`/`"false"`/numeric strings properly (the prior env-var path forwarded `"2"` as a literal IP). - `gateway.trustProxy` reported in `managedExternally` when the env var is set. Refs #4535 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the new General settings entry and explains the accepted formats (hop count, IP/CIDR list, presets). Calls out that `true`/empty triggers the express-rate-limit `ERR_ERL_PERMISSIVE_TRUST_PROXY` warning, and that the env var overrides the UI value. Refs #4535 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Home Assistant addon defaults TRUST_PROXY=1 (home-assistant/addons#4597), which covers the largest user group affected by #4535. A redundant UI field would just create two ways to configure the same thing. Reverts: - trustProxy field on GatewayConfig - gateway.trustProxy in managedExternally - Trust Proxy text field in Settings.vue General panel - Trust Proxy entry in docs/usage/setup.md - Settings fallback in configureTrustProxy() Kept (still needed): - parseTrustProxy() coercion — required for TRUST_PROXY=1 to be applied as a hop count rather than the IP literal "1" (the prior env-var path forwarded the raw string to app.set, which Express treats as an IP, leaving trust proxy effectively off). - Removal of the X-Forwarded-For auto-detect middleware (spoofable). - Removal of the blanket xForwardedForHeader validator suppression. Refs #4535 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
1 task
1 task
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 issues affecting users behind reverse proxies (notably the Home Assistant addon).
Socket reconnection loop (Fixes #4564)
When the server's auth middleware rejected a socket connection (e.g. expired JWT), the client silently looped on reconnection forever. Socket.IO v4 emits
connect_erroron rejected handshakes, but the client only listened forerror.src/App.vue— JWT expiry is checked client-side before attempting to connect; an expired token triggerslogout()instead of a connect attempt.src/App.vue— added aconnect_errorhandler. Three consecutive auth failures trigger logout (single failures are tolerated to ride out transient issues); other errors update status to Reconnecting.ERR_ERL_UNEXPECTED_X_FORWARDED_FORvalidation error (Fixes #4535)express-rate-limitemitted a noisyValidationErroron every restart for users running behind a reverse proxy. The pre-existingTRUST_PROXYenv-var path looked correct but was effectively broken:TRUST_PROXY=1was forwarded toapp.set('trust proxy', '1')as a string, which Express treats as the IP literal"1"— leaving trust proxy effectively off and the validation error firing.api/app.ts—parseTrustProxy()coerces the raw env value:"true"/"false"→ boolean, numeric strings → hop count integer, anything else → passed through verbatim (IP/CIDR list or preset name).api/app.ts— movedapp.set('trust proxy', ...)intostartServer()viaconfigureTrustProxy(), mirroring how HTTPS is handled. Logs the resolved value at startup.docs/guide/env-vars.md— expanded theTRUST_PROXYdescription with accepted formats and a note aboutERR_ERL_PERMISSIVE_TRUST_PROXY(don't set it totrue; prefer an explicit hop count or IP list).This pairs with home-assistant/addons#4597, which sets
TRUST_PROXY=1by default in the HA addon — that PR only actually resolves #4535 once this coercion is in place.An earlier revision of this PR added an X-Forwarded-For auto-detect middleware and a UI toggle for trust proxy; both have been dropped. Auto-detect was spoofable (any client could trip it via a forged XFF header and bypass the login rate-limiter), and the UI field became redundant once the HA addon set the env var by default.
Test plan
ERR_ERL_UNEXPECTED_X_FORWARDED_FORerrors in logs when behind proxy withTRUST_PROXY=1TRUST_PROXY=1is applied as a hop count (integer), not as the IP literal"1"TRUST_PROXYaccepting an IP/CIDR list still worksnpm run lintpasses