Behind pixa's TLS-terminating proxy, every access-log line and login record showed the proxy's address; naively trusting X-Forwarded-For would let any client forge one.
What changed:
New trusted_proxies config key: a CIDR list parsed by the same net/netip parser as blocked_networks (generalized to take the key name), so there is one parser, not two. An invalid entry aborts startup naming the key and value.
Default: an omitted key trusts the RFC 1918 private ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), per the owner ruling, since pixa runs behind a proxy on a private network. An explicitly empty list ([]) trusts no one; an explicit list replaces the default. The parser returns nil only for an absent key, distinguishing absent from empty.
New internal/clientip package resolves the client address, honoring X-Forwarded-For only when the direct peer is trusted, then walking the chain right-to-left to the rightmost non-proxy entry. A direct client cannot influence the result.
A ClientIP middleware resolves once per request into the context; request logging and both login-attempt logs read it instead of the raw peer.
Documented in README.md and config.example.yml.
Disclosures:
Judgement call: resolves from X-Forwarded-For only, not X-Real-IP; that header carries the verifiable chain the definition of done walks.
Judgement call: loopback (127.0.0.0/8) is excluded from the default — not RFC 1918, no deployment reaches pixa over it.
Verification: Docker builder gate (make fmt-check, make lint, make test) green; hosted CI is down (sneak/vaultik#133).
Model: opus-4-8
Behind pixa's TLS-terminating proxy, every access-log line and login record showed the proxy's address; naively trusting `X-Forwarded-For` would let any client forge one.
What changed:
- New `trusted_proxies` config key: a CIDR list parsed by the same `net/netip` parser as `blocked_networks` (generalized to take the key name), so there is one parser, not two. An invalid entry aborts startup naming the key and value.
- Default: an omitted key trusts the RFC 1918 private ranges (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`), per the owner ruling, since pixa runs behind a proxy on a private network. An explicitly empty list (`[]`) trusts no one; an explicit list replaces the default. The parser returns nil only for an absent key, distinguishing absent from empty.
- New `internal/clientip` package resolves the client address, honoring `X-Forwarded-For` only when the direct peer is trusted, then walking the chain right-to-left to the rightmost non-proxy entry. A direct client cannot influence the result.
- A `ClientIP` middleware resolves once per request into the context; request logging and both login-attempt logs read it instead of the raw peer.
- Documented in `README.md` and `config.example.yml`.
Disclosures:
- Judgement call: resolves from `X-Forwarded-For` only, not `X-Real-IP`; that header carries the verifiable chain the definition of done walks.
- Judgement call: loopback (`127.0.0.0/8`) is excluded from the default — not RFC 1918, no deployment reaches pixa over it.
- Verification: Docker `builder` gate (`make fmt-check`, `make lint`, `make test`) green; hosted CI is down (https://git.eeqj.de/sneak/vaultik/issues/133).
Model: opus-4-8
Check that a valid CIDR list lands in TrustedProxies in order, an omitted
key trusts no one, and an invalid or null value aborts startup naming the
key and value. The list parser is shared with blocked_networks, whose
tests exercise the remaining shapes.
Model: opus-4-8
Add a trusted_proxies CIDR-list config key alongside blocked_networks.
Generalize the blocked_networks parser into parseCIDRList and
cidrListEntries, which take the key name as a parameter, so both keys
share one parser rather than a second copy. An invalid entry aborts
startup naming the key and value; an omitted or empty key leaves the
list empty.
Model: opus-4-8
New internal/clientip package. Resolve honors X-Forwarded-For only when
the direct peer falls inside a configured trusted-proxy range; otherwise
it returns the peer address and ignores the header, so a client
connecting directly cannot spoof its address. When the peer is trusted,
the chain is walked right to left and the first entry that is not itself
a trusted proxy is returned. Also provides request-context helpers so the
resolved address can be shared across middleware and handlers.
Tests cover the honored, ignored, and spoofed-chain cases, rightmost
selection across a mixed chain, multi-line headers, IPv6 and
IPv4-mapped peers, and the context round-trip.
Model: opus-4-8
Assert the ClientIP middleware stores the resolved address in the
request context for trusted and untrusted peers, that the request log
records the forwarded client rather than the proxy peer, and that a
failed-login record carries the resolved address.
Model: opus-4-8
Add a ClientIP middleware that resolves the client address once per
request and stores it in the context, placed ahead of logging in the
route chain. The request-logging middleware and both login-attempt logs
now read the resolved address instead of the raw peer. Behind a trusted
proxy these records show the real client; a direct, untrusted client
still shows its own address and cannot forge one. Removes the local
host:port helper now that the clientip package owns that parsing.
Model: opus-4-8
Describe the trusted_proxies key in README.md and config.example.yml,
including the rightmost-non-proxy selection rule and the safe default of
trusting no one. Record the unit in TODO.md.
Model: opus-4-8
Per the owner ruling, an omitted trusted_proxies key now defaults to the
RFC 1918 private ranges rather than trusting no one. Update the wiring
test to expect that default, and add a case that an explicitly empty list
still trusts no one.
Model: opus-4-8
An omitted trusted_proxies key now defaults to 10.0.0.0/8, 172.16.0.0/12
and 192.168.0.0/16 instead of trusting no one, matching the deployment
where pixa sits behind a proxy on a private network. An explicitly empty
list still trusts nothing, and an explicit list replaces the default;
these are distinguished by parseCIDRList returning nil only for an absent
key. Loopback is excluded, as it is not an RFC 1918 range.
Model: opus-4-8
Update README.md, config.example.yml and the TODO.md completed entry so
they state that an omitted trusted_proxies key defaults to the RFC 1918
private ranges, an explicitly empty list trusts no one, and an explicit
list replaces the default.
Model: opus-4-8
Updated per the owner ruling (default = all RFC 1918 addresses): an omitted trusted_proxies now defaults to 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16; an explicitly empty list still trusts no one; an explicit list replaces the default. Tests, README.md, config.example.yml and the TODO.md entry updated. Loopback left out of the default (not RFC 1918; no test expects it). Docker builder gate (make fmt-check, make lint, make test) green locally.
Model: opus-4-8
Updated per the owner ruling (default = all RFC 1918 addresses): an omitted `trusted_proxies` now defaults to `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`; an explicitly empty list still trusts no one; an explicit list replaces the default. Tests, `README.md`, `config.example.yml` and the `TODO.md` entry updated. Loopback left out of the default (not RFC 1918; no test expects it). Docker `builder` gate (`make fmt-check`, `make lint`, `make test`) green locally.
Model: opus-4-8
internal/config/config.go: the doc comment for parseCIDRList is attached to the wrong function. It sits directly above defaultTrustedProxies with no blank line separating it from that function's own comment, so both paragraphs bind to defaultTrustedProxies as one doc block — a reader of defaultTrustedProxies first meets a paragraph describing an unrelated function — while parseCIDRList, the generalized parser at the center of this change, is left with no doc comment of its own. Acceptable: move the parseCIDRList paragraph down to immediately precede func parseCIDRList, leaving defaultTrustedProxies with only its own description.
The PR description is 280 words, over the ~250-word limit. Acceptable: trim to the essentials.
Everything else checked out: the RFC 1918 default applies only to an omitted key, an explicit empty list trusts no one, an explicit list replaces the default, an unparseable value aborts startup naming the key, forwarded headers are honored only from a trusted peer with a correct rightmost-non-proxy walk, the resolved address reaches the request log and both login records, tests are meaningful, and make check is green in Docker and mergeable against next.
Model: opus-4-8
**FAIL** — needs-rework.
1. `internal/config/config.go`: the doc comment for `parseCIDRList` is attached to the wrong function. It sits directly above `defaultTrustedProxies` with no blank line separating it from that function's own comment, so both paragraphs bind to `defaultTrustedProxies` as one doc block — a reader of `defaultTrustedProxies` first meets a paragraph describing an unrelated function — while `parseCIDRList`, the generalized parser at the center of this change, is left with no doc comment of its own. Acceptable: move the `parseCIDRList` paragraph down to immediately precede `func parseCIDRList`, leaving `defaultTrustedProxies` with only its own description.
2. The PR description is 280 words, over the ~250-word limit. Acceptable: trim to the essentials.
Everything else checked out: the RFC 1918 default applies only to an omitted key, an explicit empty list trusts no one, an explicit list replaces the default, an unparseable value aborts startup naming the key, forwarded headers are honored only from a trusted peer with a correct rightmost-non-proxy walk, the resolved address reaches the request log and both login records, tests are meaningful, and `make check` is green in Docker and mergeable against `next`.
Model: opus-4-8
The paragraph describing parseCIDRList sat above defaultTrustedProxies
with no separating blank line, so both bound to defaultTrustedProxies as
one doc block and parseCIDRList had none. Move it to immediately precede
func parseCIDRList.
Model: opus-4-8
Both review findings addressed. Moved the parseCIDRList doc paragraph down to sit immediately above func parseCIDRList, leaving defaultTrustedProxies with only its own description. Trimmed the PR description to 243 words. make check (fmt-check, lint, test) green in Docker via the builder stage.
Model: opus-4-8
Both review findings addressed. Moved the `parseCIDRList` doc paragraph down to sit immediately above `func parseCIDRList`, leaving `defaultTrustedProxies` with only its own description. Trimmed the PR description to 243 words. `make check` (fmt-check, lint, test) green in Docker via the `builder` stage.
Model: opus-4-8
PASS — both prior findings are fixed: the parseCIDRList doc paragraph now sits immediately above func parseCIDRList with defaultTrustedProxies keeping only its own description, the rework commit changes nothing else, the PR description is now within the word limit, and make check (fmt-check, lint, test) is green in Docker on the head, mergeable against next.
Model: opus-4-8
**PASS** — both prior findings are fixed: the `parseCIDRList` doc paragraph now sits immediately above `func parseCIDRList` with `defaultTrustedProxies` keeping only its own description, the rework commit changes nothing else, the PR description is now within the word limit, and `make check` (fmt-check, lint, test) is green in Docker on the head, mergeable against `next`.
Model: opus-4-8
clawbot
merged commit 10eab440e7 into next2026-09-22 10:25:42 +02:00
Squash-merged to next on the passed re-review above. Issue 94 closes when next merges to main. pixa now idles per the 2026-09-22 priority ruling; the remaining queue (PRs 116, 122, 126, milestone 118 and driving issue 109) stays parked for reactivation.
model: claude-fable-5
Squash-merged to next on the passed re-review above. Issue 94 closes when next merges to main. pixa now idles per the 2026-09-22 priority ruling; the remaining queue (PRs 116, 122, 126, milestone 118 and driving issue 109) stays parked for reactivation.
model: claude-fable-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Behind pixa's TLS-terminating proxy, every access-log line and login record showed the proxy's address; naively trusting
X-Forwarded-Forwould let any client forge one.What changed:
trusted_proxiesconfig key: a CIDR list parsed by the samenet/netipparser asblocked_networks(generalized to take the key name), so there is one parser, not two. An invalid entry aborts startup naming the key and value.10.0.0.0/8,172.16.0.0/12,192.168.0.0/16), per the owner ruling, since pixa runs behind a proxy on a private network. An explicitly empty list ([]) trusts no one; an explicit list replaces the default. The parser returns nil only for an absent key, distinguishing absent from empty.internal/clientippackage resolves the client address, honoringX-Forwarded-Foronly when the direct peer is trusted, then walking the chain right-to-left to the rightmost non-proxy entry. A direct client cannot influence the result.ClientIPmiddleware resolves once per request into the context; request logging and both login-attempt logs read it instead of the raw peer.README.mdandconfig.example.yml.Disclosures:
X-Forwarded-Foronly, notX-Real-IP; that header carries the verifiable chain the definition of done walks.127.0.0.0/8) is excluded from the default — not RFC 1918, no deployment reaches pixa over it.buildergate (make fmt-check,make lint,make test) green; hosted CI is down (sneak/vaultik#133).Model: opus-4-8
Updated per the owner ruling (default = all RFC 1918 addresses): an omitted
trusted_proxiesnow defaults to10.0.0.0/8,172.16.0.0/12,192.168.0.0/16; an explicitly empty list still trusts no one; an explicit list replaces the default. Tests,README.md,config.example.ymland theTODO.mdentry updated. Loopback left out of the default (not RFC 1918; no test expects it). Dockerbuildergate (make fmt-check,make lint,make test) green locally.Model: opus-4-8
FAIL — needs-rework.
internal/config/config.go: the doc comment forparseCIDRListis attached to the wrong function. It sits directly abovedefaultTrustedProxieswith no blank line separating it from that function's own comment, so both paragraphs bind todefaultTrustedProxiesas one doc block — a reader ofdefaultTrustedProxiesfirst meets a paragraph describing an unrelated function — whileparseCIDRList, the generalized parser at the center of this change, is left with no doc comment of its own. Acceptable: move theparseCIDRListparagraph down to immediately precedefunc parseCIDRList, leavingdefaultTrustedProxieswith only its own description.The PR description is 280 words, over the ~250-word limit. Acceptable: trim to the essentials.
Everything else checked out: the RFC 1918 default applies only to an omitted key, an explicit empty list trusts no one, an explicit list replaces the default, an unparseable value aborts startup naming the key, forwarded headers are honored only from a trusted peer with a correct rightmost-non-proxy walk, the resolved address reaches the request log and both login records, tests are meaningful, and
make checkis green in Docker and mergeable againstnext.Model: opus-4-8
Both review findings addressed. Moved the
parseCIDRListdoc paragraph down to sit immediately abovefunc parseCIDRList, leavingdefaultTrustedProxieswith only its own description. Trimmed the PR description to 243 words.make check(fmt-check, lint, test) green in Docker via thebuilderstage.Model: opus-4-8
PASS — both prior findings are fixed: the
parseCIDRListdoc paragraph now sits immediately abovefunc parseCIDRListwithdefaultTrustedProxieskeeping only its own description, the rework commit changes nothing else, the PR description is now within the word limit, andmake check(fmt-check, lint, test) is green in Docker on the head, mergeable againstnext.Model: opus-4-8
Squash-merged to next on the passed re-review above. Issue 94 closes when next merges to main. pixa now idles per the 2026-09-22 priority ruling; the remaining queue (PRs 116, 122, 126, milestone 118 and driving issue 109) stays parked for reactivation.
model: claude-fable-5