Repository navigation
review remediation: framework-level trusted-proxy setting for X-Forwarded-* (isSecure, maintenance IP list, reload rate-limit keying) #2953
Description
Problem
Three findings share one root cause: there is no framework-level trusted-proxy setting, so client-controllable X-Forwarded-* headers (and a URL-sourced IP list) drive security decisions. Wave 1 (#2902) solved this pattern for debug-IP allowlisting; the same trust model should be extended consistently rather than patched per call-site. Line numbers verified against origin/develop @ 8971094.
1. isSecure() unconditionally trusts X-Forwarded-Proto (C9 / SEC-13)
vendor/wheels/controller/miscellaneous.cfc:364 — request.cgi.http_x_forwarded_proto == "https" is trusted with no trusted-proxy gating, and $cgiScope copies the header verbatim (Global.cfc ~2220-2230). A direct-HTTP client can send the header and make isSecure() return true, spoofing secure-cookie/login/CSRF decisions and HTTPS-redirect filters.
2. Maintenance mode reads its IP exception list from the URL and trusts spoofable XFF (DC9 / SEC-14)
vendor/wheels/events/EventMethods.cfc:226, :236-239 — url.except is written into application.wheels.ipExceptions (under a readOnly lock that does not serialize writes), then matched against cgi.http_x_forwarded_for. Any anonymous client can disable maintenance gating (?except=<ip>) persistently for everyone, or spoof XFF to bypass it. (vendor/wheels/events/EventMethods.cfc was occupied by PR #2900, since merged.)
3. Reload/env-switch brute-force rate limit keys on raw REMOTE_ADDR with no XFF handling (SEC-12)
vendor/wheels/events/onapplicationstart.cfc:149 keys the brute-force rate limit on raw REMOTE_ADDR (contrast consoleeval.cfm:36-50). Behind a proxy all clients share one bucket (lockout DoS); connecting directly, an attacker rotates IPs to reset the limit.
Impact
isSecure()can be spoofed by any direct-HTTP client, undermining secure-cookie, HTTPS-redirect, and CSRF decisions that depend on it.- Maintenance mode can be disabled for all users by an anonymous request, and the write-to-application-scope-from-request-data pattern is itself unsafe.
- Reload/env-switch brute-force protection is both DoS-able (shared bucket behind a proxy) and bypassable (IP rotation direct).
Suggested approach
Introduce one framework-level trustProxyHeaders (or reuse/extend the #2902 trusted-proxy mechanism) and route all three through it:
- Gate the
X-Forwarded-Protocheck behind the setting; document that the proxy must overwrite (not append) the header. - Source the maintenance exception list from config only; match against
cgi.remote_addr; honorX-Forwarded-Foronly behind a trusted proxy; never write request data to application scope. - Derive the brute-force key from a trusted client identity (remote_addr, or the leftmost untrusted XFF hop behind a trusted proxy) and add a global/backoff cap.
Acceptance criteria
- A single trusted-proxy setting controls whether
X-Forwarded-*headers are honored; default is "do not trust". - With trust off,
isSecure()ignoresX-Forwarded-Proto;?except=cannot alter maintenance gating; brute-force key uses remote_addr. - With trust on, the documented proxy-overwrite contract is honored for all three.
- No request data is written into application scope for maintenance exceptions.
- Specs cover spoofed-header and direct-connection cases.
Source
Internal multi-agent framework review 2026-06-09, wave 2 (issues phase). Findings: controller C9, security-pass SEC-13/SEC-14/SEC-12, dispatch-core DC9. (EventMethods.cfc was held behind PR #2900, since merged.)
Activity
wheels-bot commentedon Jun 10, 2026
Wheels Bot — Triage
Classified as bug.
Suspected layer
controller— see.ai/wheels/security/https-detection.mdfor the canonicalisSecure()/ proxy-header trust patterns. The fix is cross-cutting: the central call site lives in the controller layer (vendor/wheels/controller/miscellaneous.cfc:363-365), but two of the three findings sit in dispatch/events (vendor/wheels/events/EventMethods.cfc:224-243for maintenance mode,vendor/wheels/events/onapplicationstart.cfc:148-158for reload rate-limit keying).Fix sketch
Introduce one framework-level trusted-proxy setting and route all three call sites through it — the natural move is to generalize PR #2902's
debugAccessTrustProxyinto a broadertrustProxyHeaders(or add a sibling setting) defaulting tofalse. Then:isSecure()(vendor/wheels/controller/miscellaneous.cfc:364): gate theX-Forwarded-Protocheck behind the new setting; when trust is off, rely solely onrequest.cgi.server_port_secure.vendor/wheels/events/EventMethods.cfc:224-243): stop writingurl.exceptintoapplication.wheels.ipExceptions; source exceptions only from config; match againstcgi.remote_addrby default; honorX-Forwarded-Foronly when the trust setting is on (and use the rightmost hop, matching PR fix(dispatch): stop trusting X-Forwarded-For for debug-access IP allowlisting #2902's resolution rule).vendor/wheels/events/onapplicationstart.cfc:148): derive the key fromcgi.remote_addr(or the trusted-proxy-aware client IP); add a global/backoff cap as a secondary mitigation against IP rotation.Specs should cover both the spoofed-header case (trust off → header ignored) and the trusted-proxy case (trust on → documented overwrite contract honored), mirroring the regression-scan idiom from
vendor/wheels/tests/specs/environment/ipbasedaccessSpec.cfc.Confidence:
mediumAuto-downgrade applied: the fix touches security/auth flows (
isSecure()drives secure-cookie, CSRF, and HTTPS-redirect decisions). There is also a real design trade-off — extend PR #2902'sdebugAccessTrustProxysemantics or introduce a separate, broadertrustProxyHeaderssetting — and the change spans three files across two layers, so propose-fix will need to make a judgment call.<!-- wheels-bot:triage:2953 -->
<!-- wheels-bot:triage-class:bug -->
<!-- wheels-bot:triage-confidence:medium -->
wheels-bot commentedon Jun 10, 2026
Wheels Bot — Fix on hold for human review
The proposed fix touches a sensitive area (framework-level trusted-proxy setting feeding
isSecure(), maintenance-mode IP gating, and reload brute-force rate-limit keying — i.e. auth/security flows) and the bot's safety net requires a human in the loop before any code is written.Specifically, all three of the following are present and individually warrant a hold:
isSecure()(vendor/wheels/controller/miscellaneous.cfc:364) drives secure-cookie, CSRF, and HTTPS-redirect decisions — auth-adjacent code.debugAccessTrustProxyvs. introducing a siblingtrustProxyHeaderssetting (the triage flagged this explicitly as a trade-off).controller/miscellaneous.cfc,events/EventMethods.cfc,events/onapplicationstart.cfc) and changes the trust contract forX-Forwarded-*headers framework-wide, which needs a human to ratify the default and the proxy-overwrite documentation before specs are written against it.bpamiri commentedon Jun 10, 2026
Coverage-audit addition (2026-06-10): add a docs acceptance criterion —
allowIPBasedDebugAccess,debugAccessIPs, and the newdebugAccessTrustProxyopt-in (shipped in #2902) are undocumented in the v4-0-0 guide tree. Whatever shape the framework-level trusted-proxy setting takes here should land together with the guide page documenting all three.