fix: stop trusting a caller-supplied X-Forwarded-For (#210)

[mekarpeles]

Fixes #210. Prod was already patched by hand (172.18.0.0/16); this fixes the defaults every other install inherits, including fresh ones.

The bug

LENNY_FORWARDED_ALLOW_IPS defaulted to *. nginx sets the header with $proxy_add_x_forwarded_for, which appends the address it observed to whatever the client already sent:

client sends:  X-Forwarded-For: 1.2.3.4
app receives:  X-Forwarded-For: 1.2.3.4, <address nginx observed>

With always_trust, uvicorn returns x_forwarded_for_hosts[0] — the leftmost, i.e. the client's own value. So a caller could choose the IP Lenny binds session cookies and OTPs to. Every IP check became advisory.

Bounded, uvicorn instead walks right-to-left and returns the first untrusted hop: the address nginx actually saw.

Three sites, not one

docker/api/Dockerfile CMD	the live path — configs.OPTIONS only reaches uvicorn under python -m lenny.app, so this is what production ran
lenny/configs/__init__.py:76	the python -m lenny.app path
docker/configure.sh	didn't template the variable at all

Why the subnet lookup is not inline

configure.sh now resolves the real Compose subnet when it can, falling back to 172.16.0.0/12. The lookup is deliberately kept out of the ${VAR:-default} expression.

docker/utils/update/020_env_sync.sh back-fills new keys into existing installs by grepping this file for the literal KEY="${KEY:-<default>}" form, and explicitly resolves a $(...) default to the empty string (020_env_sync.sh:163-170). Inline, it would have written LENNY_FORWARDED_ALLOW_IPS= into every existing .env on the next make update.

Empty is worse than *. uvicorn then trusts no proxy at all, so every patron collapses onto the nginx address — one identity, one shared OTP rate-limit bucket (OL allows 3/60s per ip), and a session binding that matches from anywhere. That's the outage-class bug #201 existed to fix.

Verified by running env_sync's own extraction pipeline against the new file:

$ grep -E '^[[:space:]]*LENNY_FORWARDED_ALLOW_IPS="\$\{LENNY_FORWARDED_ALLOW_IPS:-[^}]*\}"' docker/configure.sh \
    | sed 's/.*:-\(.*\)}".*/resolved default = [\1]/'
resolved default = [172.16.0.0/12]

And the shell guard against every failure mode:

normal lookup                      -> 172.18.0.0/16
lookup returns empty               -> 172.16.0.0/12
lookup errors / garbage            -> 172.16.0.0/12
whitespace only                    -> 172.16.0.0/12
operator override wins             -> 10.9.0.0/24

Making it loud

Both failure modes are silent — no exception, no status change, no log line. That is why this survived from the day proxy headers were enabled. Added:

• a startup warning when the range is *;
• a once-per-worker request warning when the resolved client.host is not the last hop of the chain. One check catches both directions: matching the first hop means too broad (spoofable), matching neither means too narrow (everyone collapsed).

Testing

• 14 new tests driving uvicorn's real ProxyHeadersMiddleware, not asserting on our config string — so they fail for the same reason production would.
• Covers: a spoofed chain loses to the observed address; multi-hop spoofing loses; the nginx $proxy_add_x_forwarded_for shape still yields the real patron IP (the fix: surface OTP failures instead of swallowing them #201 regression, previously untested); a blank env var falls back rather than disabling proxy trust; an operator override is honoured; neither shipped entrypoint carries a wildcard; configure.sh templates the key.
• One test characterizes the old * behaviour so the regression stays legible.
• Mutation-checked: restoring the * default fails 7 of them.
• Full suite 307 passed, 13 skipped; flake8 clean; bash -n clean.

Coordination

Found and scoped with olsystem-3b, who filed #210 and fixed prod. Their #211 (draft) also touches docker/configure.sh's .env heredoc — my addition is contiguous at the end and should rebase either direction; they've said they'll take the conflict and move #211 to 0.2.19 if this lands first. I've left nginx.conf, Makefile, models.py, and everything under oauth2 untouched.

Not verified against a running Lenny — the middleware behaviour is exercised directly, and prod already carries an equivalent value set by hand.