parththakkar106andClaude Opus 5
041f9e25f3
A hosted demo raises a question a local app never does: is anyone using it, and do they reach the part that matters? `/analytics` answers it — visitors, pages, referrers, countries, devices, which shared scenarios get played, turns and demo-key spend, API and turn errors, and a funnel from visited to played a turn to signed up. Not a third-party script, for reasons specific to this one. The CSP allows `script-src 'self'`, so a tracker means loosening it; adblockers eat the popular ones, which silently biases exactly the technical audience this project gets shown to; and none of them can see the measurement that actually matters here, which is a turn, not a pageview. **A visit is a write and never a read.** After the 189x egress fix it would be perverse to add a feature that reads rows per request, so counts accumulate in a process-local dict and flush every 60s as UPSERTs. Storage is a generic `(day, metric, label) -> hits` counter, so measuring something new later costs a constant rather than a migration, plus one row per visitor per day for the funnel flags. Every dashboard query is a GROUP BY returning tens of rows however much traffic sits behind it; a month reads back in a few kilobytes. The buffer's cost is that a hard restart can lose up to a minute — the flusher also runs on shutdown, and a tier that sleeps when idle sleeps on an empty buffer anyway. **The counters are anonymous; the access log beside them is not, on purpose.** A visitor is `HMAC(secret, "visitor:<user id>")` truncated to 32 chars — one-way, so `analytics_daily` and `analytics_visitor_days` cannot be joined back to `users`, and keyed, so no client can compute one. Story content never reaches that module, and the only content it ever names is a seeded public scenario's title; a player's own titles are theirs. `accesslog.py` is the identifying half and is a separate module writing a separate table so that separation is a property of the code rather than a convention: `access_events` records sessions, sign-ins, registrations and failed attempts with address, email and device, read on a second tab of the same page behind the same gate. Both halves are gated on `AIDND_ANALYTICS_EMAILS`, not `POWER_USERS`. An unmetered tester is not automatically someone who should see the traffic. The route 404s and the nav link is absent for everyone else, the same treatment AI Chat gets; unset in a hosted deploy means nobody sees it, including me. Three things came out of building it that a test would not have suggested. **A failed turn is an HTTP 200 with a bad ending.** The status-code middleware cannot see one, so a demo whose model had started refusing every request would look perfectly healthy from outside. All five SSE error paths in `_generate_turn` now go through a `turn_error()` helper that counts on the way out. Error buckets elsewhere are labelled by the matched route template rather than the requested path — one bucket per endpoint instead of one per adventure id, and, the reason it isn't merely tidier, an unmatched path is entirely attacker-chosen, so labelling by it would let anyone mint rows. **The funnel counts people, not clicks.** A player who starts six adventures is one person who started an adventure. That is the whole reason the per-visitor-day table exists; its flags only ever turn on, and `is_new` is settled by the first write of a visitor's first day. **The tests run on SQLite and production is Neon.** A flush that raises is caught and logged, so a dialect mistake in the UPSERTs would have stayed invisible until the dashboard quietly never filled. `test_the_upserts_compile_for_postgres` compiles both statements against the Postgres dialect without connecting to one. Two things this leans on elsewhere. `limits._client_ip` is now public `client_ip`: the access log needs the same answer, and two functions both deciding which hop is the caller's is how one of them ends up trusting a header it shouldn't. And the cleanup sweeper now starts if *either* job has work — a deployment can keep every guest forever and still want its visitor-day rows aged out. No migration. Both tables are new and `bootstrap()` calls `create_all` on existing databases too, the route `branches` took in Phase 14, so `LATEST_VERSION` is still 64. 497 tests green, frontend lint and build clean, driven by hand against a synthetic 90-day fixture at 1568px. The narrow-screen layout follows the existing 720px block but is unverified: `resize_window` is ignored on a maximized Chrome and `frame-ancestors 'none'` rules out checking it in a sized iframe. Also repaired here: a rename in test_ratelimit_hardening.py had run through the test names themselves, leaving `testclient_ip_*` — still collected by pytest, which is why it passed unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfMCsN1KBLsTqMkj5hSgrY
109 lines
4.0 KiB
Python
109 lines
4.0 KiB
Python
"""Regression tests for the X-Forwarded-For rate-limit bypass and the
|
|
per-account login throttle added to close it.
|
|
|
|
Background: uvicorn's --forwarded-allow-ips "*" trusted the LEFTMOST
|
|
X-Forwarded-For entry, which the client controls, so rotating the header
|
|
handed out a fresh rate-limit bucket per request. client_ip now reads the
|
|
hop the trusted edge appends (rightmost), and login has an email-keyed throttle
|
|
that no IP trick can dilute.
|
|
|
|
python -m pytest tests/test_ratelimit_hardening.py -v
|
|
"""
|
|
import os
|
|
import tempfile
|
|
|
|
_tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False)
|
|
_tmp.close()
|
|
os.environ["AIDND_DB_PATH"] = _tmp.name
|
|
os.environ.pop("AIDND_DATABASE_URL", None)
|
|
os.environ.pop("DATABASE_URL", None)
|
|
|
|
import pytest
|
|
|
|
from app import auth, limits
|
|
|
|
|
|
class _Req:
|
|
"""Minimal stand-in for starlette's Request: a header lookup and a peer."""
|
|
|
|
def __init__(self, xff: str | None, peer: str | None = "10.0.0.1"):
|
|
self.headers = {} if xff is None else {"x-forwarded-for": xff}
|
|
self.client = None if peer is None else type("C", (), {"host": peer})()
|
|
|
|
|
|
# ---------- client_ip: the spoof-resistant hop ----------
|
|
|
|
def test_client_ip_takes_appended_rightmost_hop(monkeypatch):
|
|
monkeypatch.setattr(limits, "TRUSTED_PROXY_HOPS", 1)
|
|
# Attacker prepends a fake IP; the edge appends the real one on the right.
|
|
req = _Req("203.0.113.9, 198.51.100.77")
|
|
assert limits.client_ip(req) == "198.51.100.77"
|
|
|
|
|
|
def test_client_ip_ignores_spoofed_leftmost(monkeypatch):
|
|
monkeypatch.setattr(limits, "TRUSTED_PROXY_HOPS", 1)
|
|
# Whatever the client stuffs to the left, the keyed IP stays the real hop —
|
|
# so rotating it no longer mints a new bucket.
|
|
a = limits.client_ip(_Req("1.1.1.1, 198.51.100.77"))
|
|
b = limits.client_ip(_Req("2.2.2.2, 198.51.100.77"))
|
|
c = limits.client_ip(_Req("evil, junk, 198.51.100.77"))
|
|
assert a == b == c == "198.51.100.77"
|
|
|
|
|
|
def test_client_ip_honours_extra_trusted_hops(monkeypatch):
|
|
monkeypatch.setattr(limits, "TRUSTED_PROXY_HOPS", 2)
|
|
# Two trusted hops: real client is second from the right.
|
|
req = _Req("9.9.9.9, 203.0.113.5, 198.51.100.77")
|
|
assert limits.client_ip(req) == "203.0.113.5"
|
|
|
|
|
|
def test_client_ip_falls_back_to_socket_peer():
|
|
assert limits.client_ip(_Req(None, peer="172.16.0.4")) == "172.16.0.4"
|
|
assert limits.client_ip(_Req(None, peer=None)) == "unknown"
|
|
|
|
|
|
# ---------- per-account login throttle ----------
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _multi_user(monkeypatch):
|
|
monkeypatch.setattr(auth, "MULTI_USER", True)
|
|
# Isolate the module-level failure map for each test.
|
|
from collections import defaultdict, deque
|
|
monkeypatch.setattr(limits, "_login_fails", defaultdict(deque))
|
|
|
|
|
|
def test_login_throttle_blocks_after_limit():
|
|
email = "victim@example.com"
|
|
# Up to the limit: allowed, each a recorded failure.
|
|
for _ in range(limits.LOGIN_FAIL_LIMIT):
|
|
limits.check_login_allowed(email) # does not raise
|
|
limits.note_login_failure(email)
|
|
# One more crosses the line.
|
|
with pytest.raises(limits.HTTPException) as exc:
|
|
limits.check_login_allowed(email)
|
|
assert exc.value.status_code == 429
|
|
|
|
|
|
def test_login_throttle_is_per_account():
|
|
for _ in range(limits.LOGIN_FAIL_LIMIT):
|
|
limits.note_login_failure("a@example.com")
|
|
with pytest.raises(limits.HTTPException):
|
|
limits.check_login_allowed("a@example.com")
|
|
# A different account is unaffected — this is not an IP bucket.
|
|
limits.check_login_allowed("b@example.com") # must not raise
|
|
|
|
|
|
def test_successful_login_clears_the_streak():
|
|
email = "typo@example.com"
|
|
for _ in range(limits.LOGIN_FAIL_LIMIT):
|
|
limits.note_login_failure(email)
|
|
limits.note_login_success(email)
|
|
limits.check_login_allowed(email) # streak wiped — must not raise
|
|
|
|
|
|
def test_throttle_is_noop_in_local_mode(monkeypatch):
|
|
monkeypatch.setattr(auth, "MULTI_USER", False)
|
|
for _ in range(limits.LOGIN_FAIL_LIMIT * 3):
|
|
limits.note_login_failure("solo@example.com")
|
|
limits.check_login_allowed("solo@example.com") # never throttled locally
|