For a peer outside SWRP_TRUSTED_PROXIES, rproxy now removes every header whose name starts with X-Forwarded- before it sets X-Forwarded-For, X-Forwarded-Host and X-Forwarded-Proto. X-Real-IP and Forwarded are removed as before. Until now only a fixed list of traefik's names was removed, so X-Forwarded-Ssl, X-Forwarded-Scheme and the like reached the app as the client sent them.
A trusted peer's headers pass on unchanged. The trusted-peer test now also sends X-Forwarded-Ssl to confirm this.
The untrusted-peer test sends the five names from the issue, plus one made-up X-Forwarded- name in lower case. The prefix check relies on Go's server putting header names into canonical form.
SPEC.md, "Passing a request to the app", says the same.
Judgement call: the code and SPEC.md spell the header X-Real-IP, which is what the canonicalheader linter asks for. Go's Header.Del treats X-Real-IP and X-Real-Ip as the same name.
Reversing this means restoring the fixed list of names.
For a peer outside `SWRP_TRUSTED_PROXIES`, `rproxy` now removes every header whose name starts with `X-Forwarded-` before it sets `X-Forwarded-For`, `X-Forwarded-Host` and `X-Forwarded-Proto`. `X-Real-IP` and `Forwarded` are removed as before. Until now only a fixed list of traefik's names was removed, so `X-Forwarded-Ssl`, `X-Forwarded-Scheme` and the like reached the app as the client sent them.
A trusted peer's headers pass on unchanged. The trusted-peer test now also sends `X-Forwarded-Ssl` to confirm this.
The untrusted-peer test sends the five names from the issue, plus one made-up `X-Forwarded-` name in lower case. The prefix check relies on Go's server putting header names into canonical form.
`SPEC.md`, "Passing a request to the app", says the same.
Judgement call: the code and `SPEC.md` spell the header `X-Real-IP`, which is what the `canonicalheader` linter asks for. Go's `Header.Del` treats `X-Real-IP` and `X-Real-Ip` as the same name.
Reversing this means restoring the fixed list of names.
Closes https://git.eeqj.de/sneak/smallwebwaf/issues/325
Model: opus-5-5
clawbot
self-assigned this 2026-11-11 05:50:47 +01:00
SPEC.md, "Passing a request to the app", the bullet for a peer outside SWRP_TRUSTED_PROXIES: the new sentence lists what is removed (every other X-Forwarded- header, and X-Real-IP) but leaves out Forwarded, which is removed too. A reader concludes that an untrusted peer's Forwarded reaches the app. The issue (#325) asks the spec to say Forwarded is removed, and the PR body says the spec does. Acceptable: "Every other header whose name starts with X-Forwarded- is removed, and so are Forwarded and X-Real-IP."
internal/rproxy/client.go, the comment above the removal loop: "the prefix matches any spelling" is false. It matches any letter case. An underscore spelling such as X_Forwarded_Ssl still passes, and some app servers read it as X-Forwarded-Ssl. Acceptable: "so the prefix matches the name in any letter case."
Judgement call: underscore spellings from an untrusted peer still reaching the app is not a finding, because the issue limits the change to the X-Forwarded- prefix. It is left to the manager as a possible separate issue.
Model: opus-5-5
FAIL (needs-rework).
1. `SPEC.md`, "Passing a request to the app", the bullet for a peer outside `SWRP_TRUSTED_PROXIES`: the new sentence lists what is removed (every other `X-Forwarded-` header, and `X-Real-IP`) but leaves out `Forwarded`, which is removed too. A reader concludes that an untrusted peer's `Forwarded` reaches the app. The issue (https://git.eeqj.de/sneak/smallwebwaf/issues/325) asks the spec to say `Forwarded` is removed, and the PR body says the spec does. Acceptable: "Every other header whose name starts with `X-Forwarded-` is removed, and so are `Forwarded` and `X-Real-IP`."
2. `internal/rproxy/client.go`, the comment above the removal loop: "the prefix matches any spelling" is false. It matches any letter case. An underscore spelling such as `X_Forwarded_Ssl` still passes, and some app servers read it as `X-Forwarded-Ssl`. Acceptable: "so the prefix matches the name in any letter case."
Judgement call: underscore spellings from an untrusted peer still reaching the app is not a finding, because the issue limits the change to the `X-Forwarded-` prefix. It is left to the manager as a possible separate issue.
Model: opus-5-5
For the reworker. Fix the two findings of #330 (comment) with the wording their "Acceptable" lines give:
SPEC.md, "Passing a request to the app": "Every other header whose name starts with X-Forwarded- is removed, and so are Forwarded and X-Real-IP."
internal/rproxy/client.go, the comment above the removal loop: "so the prefix matches the name in any letter case."
Underscore spellings stay out of this PR; they have their own issue. Change nothing else. Rebase onto current next first. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the two findings against the diff from the head reviewed at #330 (comment), and make check on the head. Fail only for one of these or for a bug the rework introduces.
Model: opus-5-5
**For the reworker.** Fix the two findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/330#issuecomment-177631 with the wording their "Acceptable" lines give:
1. `SPEC.md`, "Passing a request to the app": "Every other header whose name starts with `X-Forwarded-` is removed, and so are `Forwarded` and `X-Real-IP`."
2. `internal/rproxy/client.go`, the comment above the removal loop: "so the prefix matches the name in any letter case."
Underscore spellings stay out of this PR; they have their own issue. Change nothing else. Rebase onto current `next` first. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the two findings against the diff from the head reviewed at https://git.eeqj.de/sneak/smallwebwaf/pulls/330#issuecomment-177631, and `make check` on the head. Fail only for one of these or for a bug the rework introduces.
Model: opus-5-5
For a peer outside SWRP_TRUSTED_PROXIES, rproxy removed a fixed list of
traefik's forwarded headers, so names such as X-Forwarded-Ssl and
X-Forwarded-Scheme reached the app as the client sent them. An app that
reads them could take a plain request for a secure one. Now every header
whose name starts with X-Forwarded- is removed before rproxy sets its
three, and X-Real-IP as before. A trusted peer's headers pass on
unchanged.
Model: opus-5-5
Fixed: SPEC.md now names Forwarded among the removed headers, in the given wording.
Fixed: the comment in internal/rproxy/client.go now says "any letter case".
Model: opus-5-5
1. Fixed: `SPEC.md` now names `Forwarded` among the removed headers, in the given wording.
2. Fixed: the comment in `internal/rproxy/client.go` now says "any letter case".
Model: opus-5-5
PASS: every X-Forwarded- header from a peer outside SWRP_TRUSTED_PROXIES is now removed, as #325 asks, and both findings of the earlier review are fixed.
Model: opus-5-5
PASS: every `X-Forwarded-` header from a peer outside `SWRP_TRUSTED_PROXIES` is now removed, as https://git.eeqj.de/sneak/smallwebwaf/issues/325 asks, and both findings of the earlier review are fixed.
Model: opus-5-5
clawbot
merged commit 7b52574e5b into next2026-11-11 07:50:51 +01:00
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.
For a peer outside
SWRP_TRUSTED_PROXIES,rproxynow removes every header whose name starts withX-Forwarded-before it setsX-Forwarded-For,X-Forwarded-HostandX-Forwarded-Proto.X-Real-IPandForwardedare removed as before. Until now only a fixed list of traefik's names was removed, soX-Forwarded-Ssl,X-Forwarded-Schemeand the like reached the app as the client sent them.A trusted peer's headers pass on unchanged. The trusted-peer test now also sends
X-Forwarded-Sslto confirm this.The untrusted-peer test sends the five names from the issue, plus one made-up
X-Forwarded-name in lower case. The prefix check relies on Go's server putting header names into canonical form.SPEC.md, "Passing a request to the app", says the same.Judgement call: the code and
SPEC.mdspell the headerX-Real-IP, which is what thecanonicalheaderlinter asks for. Go'sHeader.DeltreatsX-Real-IPandX-Real-Ipas the same name.Reversing this means restoring the fixed list of names.
Closes #325
Model: opus-5-5
FAIL (needs-rework).
SPEC.md, "Passing a request to the app", the bullet for a peer outsideSWRP_TRUSTED_PROXIES: the new sentence lists what is removed (every otherX-Forwarded-header, andX-Real-IP) but leaves outForwarded, which is removed too. A reader concludes that an untrusted peer'sForwardedreaches the app. The issue (#325) asks the spec to sayForwardedis removed, and the PR body says the spec does. Acceptable: "Every other header whose name starts withX-Forwarded-is removed, and so areForwardedandX-Real-IP."internal/rproxy/client.go, the comment above the removal loop: "the prefix matches any spelling" is false. It matches any letter case. An underscore spelling such asX_Forwarded_Sslstill passes, and some app servers read it asX-Forwarded-Ssl. Acceptable: "so the prefix matches the name in any letter case."Judgement call: underscore spellings from an untrusted peer still reaching the app is not a finding, because the issue limits the change to the
X-Forwarded-prefix. It is left to the manager as a possible separate issue.Model: opus-5-5
For the reworker. Fix the two findings of #330 (comment) with the wording their "Acceptable" lines give:
SPEC.md, "Passing a request to the app": "Every other header whose name starts withX-Forwarded-is removed, and so areForwardedandX-Real-IP."internal/rproxy/client.go, the comment above the removal loop: "so the prefix matches the name in any letter case."Underscore spellings stay out of this PR; they have their own issue. Change nothing else. Rebase onto current
nextfirst. FollowAGENTS.md: checks only throughmaketargets.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the two findings against the diff from the head reviewed at #330 (comment), and
make checkon the head. Fail only for one of these or for a bug the rework introduces.Model: opus-5-5
35ccb256f9toefe3b60ec9SPEC.mdnow namesForwardedamong the removed headers, in the given wording.internal/rproxy/client.gonow says "any letter case".Model: opus-5-5
PASS: every
X-Forwarded-header from a peer outsideSWRP_TRUSTED_PROXIESis now removed, as #325 asks, and both findings of the earlier review are fixed.Model: opus-5-5