Rewrites the internal/api comment blocks to rule 32: task, milestone and
rule ids gone, the stories behind them restated as the constraint they
left, stale references corrected (the stream secret is wired from config,
the timezone handler already refreshes the scheduler, the Android app is
the native client, the design docs cited are not in the repo). Also
rewraps comment lines over 80 columns left by the earlier batches.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
buildResetURL used r.Host and r.TLS, so a forgot-password request with a
forged Host emailed the victim a real reset token on a link to the
attacker's server. Links now come only from network_settings.public_url
(migration 0062), and no reset email is sent while it is empty; the response
stays the same opaque 200 and the log says why.
The address is set on a new "Public address" card under Admin → Integrations,
which offers the page's own origin and warns while unset. PUT
/api/admin/network-settings takes either field alone, so the proxy card and
this one can't overwrite each other.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes the defect the operator spotted in #370 immediately after it shipped:
auth.ClientIP ignored X-Forwarded-For whenever RemoteAddr was public, so a
proxy on a public address — a separate host, or a CDN, i.e. anyone running
this publicly, since public means TLS means a proxy — recorded the PROXY for
every session. created_ip and last_ip were then always equal and the
"Address changed" signal could never fire. The feature looked like it worked
and reported nothing.
Replaced with the standard trusted-hop model (Rails, Caddy, Traefik, nginx).
XFF grows left-to-right as each proxy appends the peer it received from, so
for client -> CDN -> own-proxy -> app the app sees [client, CDN] with
RemoteAddr = own-proxy, and the client sits at XFF[len - hops]:
0 RemoteAddr, XFF ignored — no proxy
1 the address your own proxy observed
2 through a CDN in front of your proxy
Default 1, per the operator: publicly reachable means a TLS terminator in
front.
The cost is real and stated rather than hidden. hops >= 1 DECLARES that a
proxy exists; set it with no proxy, or deeper than the actual chain, and the
index reaches attacker-supplied entries, letting a visitor choose which
address their own session shows — defeating exactly the detection #370 is
for. That's inherent to the model, which is why 0 is a first-class value and
the admin card says "count your proxies, don't guess high" instead of just
exposing a number. Both mis-set shapes are pinned by tests so they stay known
consequences rather than surprises.
Migration 0053 + internal/netsettings, cached under an RWMutex. That's not an
optimisation: ClientIP runs in RequireUser for every authenticated request, so
a per-request query would put the database on the critical path of the whole
API. New() always returns a usable service so a boot-time DB hiccup degrades
to the default instead of breaking that path (rule #131), and Hops() is
nil-safe because test routers construct middleware without it.
RequireUser now takes a func() int rather than an int — the value is
operator-editable at runtime while the middleware is built once at boot, and
reading it per request is what makes a save take effect with no restart
(rule #25).
The admin card is verifiable, not just configurable: it reports the address
the CURRENT setting resolves THIS request to, the raw forwarded chain, and the
socket peer — so you set the number, save, and confirm the address matches the
machine you're on. It also counts the arriving chain and says how many proxies
that implies. GET/PUT both return that payload, PUT recomputed under the new
value, so the effect is visible without a reload.
Also fixes styling in the #370 card that CI could not catch: text-destructive
and bg-destructive don't exist in this Tailwind config — the palette is
colors.action.destructive — so the "Address changed" warning and the
sign-out-others button were rendering unstyled. Both now use
text-action-destructive / bg-action-destructive / text-action-fg.
Not done here: requestlog.go still logs raw RemoteAddr and will disagree with
the sessions UI about who connected. Left for its own change.
bvandeusenandClaude Opus 5.5