Commit graph

2 commits

Author SHA1 Message Date
ScreenTinker 4b13dadb4d fix(logging): gate CF-Connecting-IP on a Cloudflare peer, not any trusted proxy
getClientIp() decides the value every per-IP control keys on — the auth/pairing rate
limiters, lib/pair-lockout, and activity_log.ip_address — so a caller must never be able
to choose it. It believed CF-Connecting-IP whenever the immediate peer was in the
`trust proxy` list, which includes loopback/linklocal/uniquelocal.

Those entries are correct for X-Forwarded-For: a proxy APPENDS to that header and Express
walks the chain right-to-left, so a client-supplied value cannot become the resolved
address. CF-Connecting-IP has no chain — a local reverse proxy passes through whatever
single value the client sent — so treating a loopback peer as evidence the request came
through Cloudflare means trusting the client.

Gate it on the published Cloudflare ranges alone. This is also the portable behaviour:
most self-hosted installs are not behind Cloudflare, and for them the header is now
simply ignored, with attribution falling back to req.ip under whatever `trust proxy` the
operator configured. Installs that do front with Cloudflare are unaffected — their peer
really is a CF edge.

Documented the distinction at config/cloudflareIps.js so the two lists are not conflated
again. No response shape or DB change; no client impact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-26 10:23:04 -05:00
ScreenTinker 45a6800621 fix: log real client IPs through Cloudflare instead of CF edge
Express's req.ip was resolving to a Cloudflare edge address (e.g.
172.70.x.x) for any request fronted by Cloudflare, because trust proxy
was set to '1' — that trusts the immediate hop, which IS Cloudflare.
All activity_log rows from API paths captured the proxy, not the
client. The WebSocket path was unaffected and recorded the real IP.

Two layers of defense:

1. trust proxy now lists Cloudflare's published v4 + v6 ranges plus
   loopback / linklocal / uniquelocal (config/cloudflareIps.js). With
   this list req.ip resolves to the original client when fronted by
   CF, and X-Forwarded-For from any non-trusted source is ignored —
   so the value can't be spoofed.

2. New getClientIp(req) helper in services/activity.js prefers the
   CF-Connecting-IP header but only honors it when the immediate TCP
   peer is itself a trusted address. Same gate as trust proxy, so a
   visitor who hits the origin directly with a forged header is
   logged at their real address.

Routed all five activity-log call sites (auth login success/failure,
admin password reset, generic activityLogger middleware, and the
in-memory rate-limiter key) through the helper.

Logging-only change. No schema changes. Existing rows are not
modified — fix applies to new entries going forward.

Verified locally:
- Bare loopback hit logs 127.0.0.1 (not a proxy address).
- Helper unit cases including an untrusted peer (203.0.113.7) sending
  a forged CF-Connecting-IP correctly fall back to the real peer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 15:26:37 -05:00