[security] fix(dashboard-auth): ignore spoofed XFF for login limits

[Hinotoi-agent]

Summary

This PR hardens the dashboard password-login rate-limit boundary so direct clients cannot choose their own rate-limit bucket with a spoofed X-Forwarded-For header.

• Fixes the password-login client-IP helper to key audit/rate-limit decisions from the direct socket peer exposed by the ASGI server.
• Leaves trusted proxy normalization to server/proxy configuration instead of trusting arbitrary request headers in application code.
• Adds regression coverage proving rotated X-Forwarded-For values do not reset the password-login limiter.

Security issues covered

Issue	Impact	Severity
Spoofable X-Forwarded-For bypass for dashboard password-login rate limiting	A remote client that can reach POST /auth/password-login could rotate X-Forwarded-For values to avoid the per-client online-guessing throttle.	Medium

Before this PR

• _client_ip(request) preferred the first value in the client-supplied X-Forwarded-For header.
• /auth/password-login used that helper as the rate-limit key for failed password-login attempts.
• A direct client could send a new X-Forwarded-For value after exhausting one bucket and continue attempts in a fresh bucket.
• Existing tests covered ordinary per-client limiting but did not cover spoofed forwarded-header rotation.

After this PR

• _client_ip(request) returns only the direct peer address from request.client.host.
• Direct clients cannot reset the password-login rate-limit bucket by changing X-Forwarded-For.
• Deployments that intentionally trust a reverse proxy can still rely on server/proxy-level trusted-forwarded-header handling before the request reaches application logic.
• A regression test locks the expected behavior: rotating X-Forwarded-For still returns 429 once the direct client is limited.

Why this matters

Password-login throttling is an online guessing control. If the limiter key is derived from an attacker-controlled header, the control can be bypassed without changing the real network peer. That is especially risky for dashboard deployments where password authentication is used to protect a non-local bind or a reverse-proxy-exposed dashboard.

The patch keeps the trust boundary simple: untrusted HTTP headers do not decide the application's login-throttle identity.

How this differs from related issue/PR

• feat(dashboard-auth): pluggable username/password login (Option B) #38819 added the pluggable username/password login flow and the current password-login route surface. This PR is a narrow hardening fix for that merged route, specifically the client-IP helper used by the password-login limiter.
• feat: add OpenClaw-style dashboard authentication #19959 is an older dashboard-auth feature PR with a broader auth design. This PR targets the currently merged dashboard_auth/routes.py implementation and adds focused regression coverage for the spoofed-forwarded-header limiter bypass.

Attack flow

client can reach POST /auth/password-login
    -> sends failed password-login attempts
        -> application keys the limiter from client-controlled X-Forwarded-For
            -> client rotates X-Forwarded-For after one bucket is exhausted
                -> more online password guesses are admitted instead of returning 429

Affected code

Issue	Files
Spoofable forwarded-header rate-limit key	hermes_cli/dashboard_auth/routes.py, tests/hermes_cli/test_dashboard_auth_password_login.py

Root cause

Issue: spoofable X-Forwarded-For bypass for password-login rate limiting

• _client_ip(request) treated X-Forwarded-For as trustworthy for all callers.
• /auth/password-login used _client_ip(request) for rate-limit and audit decisions.
• X-Forwarded-For is only trustworthy when it has been normalized by a trusted proxy/server boundary; direct dashboard clients can set it arbitrarily.

CVSS assessment

Issue	CVSS v3.1	Vector
Spoofable X-Forwarded-For bypass for dashboard password-login rate limiting	6.5 Medium	CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:N

Rationale:

• The attacker condition is network reachability to a password-login-enabled dashboard endpoint.
• The direct bug is a bypass of an online guessing control rather than an immediate credential disclosure.
• Confidentiality/integrity impact is rated low because successful exploitation increases feasible password guessing against dashboard authentication, but still requires a valid credential guess.

Safe reproduction steps

1. Start from vulnerable code where _client_ip(request) trusts X-Forwarded-For.
2. Exercise POST /auth/password-login locally with a fake provider and fake password so no real credentials or external providers are touched.
3. Send 11 failed attempts without X-Forwarded-For and observe the 11th response is 429.
4. Reset the limiter, send 11 failed attempts with a stable spoofed X-Forwarded-For, and observe the 11th response is 429.
5. Send another attempt with a different spoofed X-Forwarded-For and observe vulnerable code admits it instead of returning 429.
6. Rotate spoofed X-Forwarded-For values and observe vulnerable code keeps admitting attempts in new buckets.
7. Apply this PR and repeat the same proof; rotated X-Forwarded-For values remain blocked with 429 after the direct client is limited.

Expected vulnerable behavior

Local vulnerable-code proof output showed:

same_client_no_xff_statuses= [404, 404, 404, 404, 404, 404, 404, 404, 404, 404, 429]
same_spoofed_xff_statuses= [404, 404, 404, 404, 404, 404, 404, 404, 404, 404, 429]
after_fixed_xff_limited_then_new_xff_status= 404
rotated_xff_statuses_after_limit_window_not_waited= [404, 404, 404, 404, 404, 404, 404, 404, 404, 404, 404]

In this local harness, 404 means the fake provider lookup was reached and the request was not rate-limited; 429 means the login limiter blocked before provider lookup.

After this PR, the same proof showed:

same_client_no_xff_statuses= [404, 404, 404, 404, 404, 404, 404, 404, 404, 404, 429]
same_spoofed_xff_statuses= [404, 404, 404, 404, 404, 404, 404, 404, 404, 404, 429]
after_fixed_xff_limited_then_new_xff_status= 429
rotated_xff_statuses_after_limit_window_not_waited= [429, 429, 429, 429, 429, 429, 429, 429, 429, 429, 429]

Changes in this PR

• Removes direct X-Forwarded-For parsing from _client_ip(request).
• Documents why password-login rate-limit/audit decisions must use the direct peer unless a trusted proxy/server layer has already normalized the client address.
• Adds a regression test that rotates X-Forwarded-For values and verifies the rate limit is not reset.

Files changed

Category	Files	What changed
Dashboard auth hardening	hermes_cli/dashboard_auth/routes.py	_client_ip() now ignores untrusted X-Forwarded-For and uses the direct peer address.
Regression coverage	tests/hermes_cli/test_dashboard_auth_password_login.py	Adds coverage proving spoofed X-Forwarded-For rotation cannot bypass the password-login limiter.

Maintainer impact

• Narrow behavior change for application-level password-login rate-limit/audit keying.
• No frontend, provider, session, token, or unrelated dashboard route behavior is changed.
• Direct dashboard clients can no longer influence their limiter identity with a forwarded-header value.
• Trusted reverse-proxy deployments should continue to handle forwarded-header trust at the server/proxy layer before application code consumes request.client.

Fix rationale

The durable boundary is to avoid trusting forwarded headers in application code for security decisions unless they have already been validated by a trusted proxy boundary. Keying the login limiter from request.client.host preserves a safe default for direct dashboard exposure and avoids duplicating proxy-trust policy inside the route handler.

The regression test is focused on the exact bypass: one direct client rotates X-Forwarded-For values after exhausting the limiter and must still receive 429.

Type of change

• Security fix
• Tests
• Documentation update
• Refactor with no behavior change

Test plan

• uv run --extra dev python -m pytest tests/hermes_cli/test_dashboard_auth_password_login.py -q
  • 20 passed, 1 warning
• uv run --extra dev ruff check hermes_cli/dashboard_auth/routes.py tests/hermes_cli/test_dashboard_auth_password_login.py
  • All checks passed!
• python3 -m py_compile hermes_cli/dashboard_auth/routes.py
• git diff --check
• Safe local proof harness against vulnerable origin/main and patched code, using only fake provider/password values.

Disclosure notes

• This PR is bounded to the dashboard password-login rate-limit keying issue.
• No real credentials, production services, or external providers were used in local proof runs.
• This PR does not claim to audit every dashboard auth trust boundary; it only fixes the forwarded-header bypass for password-login rate limiting.
• No unrelated files are changed.

[liuhao1024]

Verified — this is a real rate-limit bypass and the fix is correct.

Vulnerability: _client_ip() trusts client-supplied X-Forwarded-For as the rate-limit key for /auth/password-login. A direct client can rotate the header value to get fresh rate-limit buckets after exhausting one, effectively unbounded password guessing.

Fix correctness: Removing X-Forwarded-For parsing from _client_ip() and using request.client.host is the correct approach. Deployments behind trusted reverse proxies already handle forwarded-header normalization at the server/proxy layer (Uvicorn's --proxy-headers or similar), so the application doesn't need to duplicate that trust.

Edge cases verified:

• The test test_spoofed_x_forwarded_for_does_not_reset_rate_limit directly covers the bypass: 10 failed attempts exhaust the bucket, then a rotated XFF still returns 429.
• The docstring correctly documents that request.client.host may already be rewritten by Uvicorn's trusted proxy handling — so this doesn't break reverse-proxy deployments.
• No other callers of _client_ip() are affected by the change (it's only used in the auth rate-limit path).

No issues found. Clean security fix with proper regression coverage.

[alpindiay]

LGTM. Important security fix — _client_ip() in the dashboard auth routes now uses request.client.host (the socket peer) instead of the X-Forwarded-For header for rate-limiting IP identification.

Why this matters: X-Forwarded-For is trivially spoofable when clients connect directly to the dashboard. An attacker could rotate the header value to get fresh rate-limit buckets per request, completely bypassing the password-login brute-force protection. The fix correctly notes that uvicorn's trusted-proxy handling can rewrite request.client for reverse-proxy deployments — that's the proper way to handle forwarded IPs.

The new test test_spoofed_x_forwarded_for_does_not_reset_rate_limit confirms that rotating X-Forwarded-For across 11 attempts does not reset the rate limit counter (all attempts count toward the same bucket, 11th returns 429). Well done.

[teknium1]

Thanks for the focused hardening patch. The current-main path is vulnerable: hermes_cli/dashboard_auth/routes.py:109-112 accepts X-Forwarded-For, and routes.py:482-483 uses that result as the password-login limiter key. The added regression test covers the relevant header-rotation behavior.

Problems

• hermes_cli/dashboard_auth/routes.py:419-421 still says a trusted proxy needs X-Forwarded-For to avoid proxy-IP bucketing. After this change, the route no longer reads that header; trusted proxy handling must instead normalize request.client, as the new _client_ip docstring describes.

Suggested changes

• Update that limiter comment to describe trusted server/proxy normalization of request.client.

Automated hermes-sweeper review.

[Hinotoi-agent]

Updated the stale limiter comment per the latest review. It now explains that trusted server/proxy handling must normalize the address exposed through request.client; otherwise requests share the proxy IP bucket. It no longer suggests that this route reads X-Forwarded-For directly.

Validation:

• uv run --extra dev python -m pytest tests/hermes_cli/test_dashboard_auth_password_login.py -q — 20 passed
• uv run --extra dev ruff check hermes_cli/dashboard_auth/routes.py tests/hermes_cli/test_dashboard_auth_password_login.py — passed
• python3 -m py_compile hermes_cli/dashboard_auth/routes.py — passed
• git diff --check — passed

Commit: 2823abcaf

[egilewski]

looks mergeable

Security evidence:

• trust boundary: Direct clients can spoof X-Forwarded-For; the limiter now uses the direct peer address, with trusted proxy normalization handled upstream.
• source/sink/invariant: Client-controlled forwarded headers no longer select password-login rate-limit buckets.
• current-main reproduction: Rotating forwarded-header values after exhausting the limiter admitted another valid login.
• PR-head or patch-replay validation: The same rotated-header sequence remains rate-limited with HTTP 429.
• positive/negative cases: Rotated, stable, and absent-header cases behave correctly; focused authentication tests pass.
• residual bypass search: No remaining forwarded-header use selects the password-login limiter bucket; other uses are audit-only.
• reviewer validation: Focused dashboard-auth tests and static validation pass.

Not checked:

• Full test suite
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex