Two gaps in the check that keeps pixa from fetching internal addresses, from #299:
64:ff9b:1::/48, the local-use NAT64 range, is now one of the built-in blocked ranges, next to 64:ff9b::/96.
isBlockedIP refuses an address in the IPv4-compatible form (::/96) when blocked_networks contains the address as written or the IPv4 address it holds. [::203.0.113.5] now answers 403 with 203.0.113.0/24 listed, as [::ffff:203.0.113.5] already did, and still answers 403 with ::/96 listed.
The mapped form (::ffff:0:0/96) is matched only as the IPv4 address it holds, as on next: Go keeps a plain IPv4 address in that form, so matching it as written would let an IPv6 entry such as ::/0 refuse every IPv4 address. The blocked_networks entry in README.md now says so.
The ::/96 reading that #298 put in isPrivateIP moved into ipv4CompatibleToIPv4, which isPrivateIP and isBlockedIP call; isPrivateIP behaves as before.
What the diff does not show:
blocked_networks is checked only when the fetcher connects. The route tests therefore use the fetcher pixad builds from the config (new helper startImageProxyWithBlockedNetworks), not the test fetcher that sends every address to a local server.
In the two cases expected not to be refused, [::203.0.113.5] with blocked_networks empty and 203.0.113.5 with ::/0 listed, the fetcher really tries to connect. Nothing answers there, so they check only that the answer is not 403, and the 2-second fetch timeout bounds them.
Judgement call: those two cases reach the network, because no other setup runs the connection check.
Model: opus-5-5
Two gaps in the check that keeps pixa from fetching internal addresses, from https://git.eeqj.de/sneak/pixa/issues/299:
- `64:ff9b:1::/48`, the local-use NAT64 range, is now one of the built-in blocked ranges, next to `64:ff9b::/96`.
- `isBlockedIP` refuses an address in the IPv4-compatible form (`::/96`) when `blocked_networks` contains the address as written or the IPv4 address it holds. `[::203.0.113.5]` now answers 403 with `203.0.113.0/24` listed, as `[::ffff:203.0.113.5]` already did, and still answers 403 with `::/96` listed.
The mapped form (`::ffff:0:0/96`) is matched only as the IPv4 address it holds, as on `next`: Go keeps a plain IPv4 address in that form, so matching it as written would let an IPv6 entry such as `::/0` refuse every IPv4 address. The `blocked_networks` entry in `README.md` now says so.
The `::/96` reading that https://git.eeqj.de/sneak/pixa/pulls/298 put in `isPrivateIP` moved into `ipv4CompatibleToIPv4`, which `isPrivateIP` and `isBlockedIP` call; `isPrivateIP` behaves as before.
What the diff does not show:
- `blocked_networks` is checked only when the fetcher connects. The route tests therefore use the fetcher pixad builds from the config (new helper `startImageProxyWithBlockedNetworks`), not the test fetcher that sends every address to a local server.
- In the two cases expected not to be refused, `[::203.0.113.5]` with `blocked_networks` empty and `203.0.113.5` with `::/0` listed, the fetcher really tries to connect. Nothing answers there, so they check only that the answer is not 403, and the 2-second fetch timeout bounds them.
Judgement call: those two cases reach the network, because no other setup runs the connection check.
Model: opus-5-5
Both image routes, with the fetcher pixad builds from the config:
[64:ff9b:1::a00:1] must answer 403, and [::203.0.113.5] must answer 403
when blocked_networks lists 203.0.113.0/24 and must not be refused when
the list is empty. The first two fail on next: both addresses are
fetched.
Model: opus-5-5
64:ff9b:1::/48 joins the built-in blocked ranges next to 64:ff9b::/96.
isBlockedIP now reads an address in ::/96 as the IPv4 address in its
last four bytes before matching blocked_networks, as it already unmapped
the ::ffff: form. The reading isPrivateIP gained for ::/96 moved into
ipv4CompatibleToIPv4, which both call.
Model: opus-5-5
clawbot
self-assigned this 2026-10-16 05:44:25 +02:00
internal/httpfetcher/httpfetcher.go, isBlockedIP: an address in ::/96 is now matched against blocked_networks only by the IPv4 address it holds, and no longer as the IPv6 address it is. An IPv6 entry that covers such an address therefore stops refusing it: with blocked_networks listing ::/96 or ::/0, [::203.0.113.5] answers 403 on next, but with this change it is fetched, through both image routes. This makes the check weaker where the issue asks for it to be stronger, and the README's description of blocked_networks as ranges never fetched from is no longer true for those entries. Acceptable: refuse the address when blocked_networks contains either the address as written or the IPv4 address it holds, with a new test that [::203.0.113.5] answers 403 when blocked_networks lists ::/96.
Judgement call: the route test that lets the fetcher try to connect to ::203.0.113.5 with blocked_networks empty is not a finding; it cannot answer 403, and the 2-second fetch timeout keeps it short.
Model: opus-5-5
Review failed: needs-rework.
1. `internal/httpfetcher/httpfetcher.go`, `isBlockedIP`: an address in `::/96` is now matched against `blocked_networks` only by the IPv4 address it holds, and no longer as the IPv6 address it is. An IPv6 entry that covers such an address therefore stops refusing it: with `blocked_networks` listing `::/96` or `::/0`, `[::203.0.113.5]` answers 403 on `next`, but with this change it is fetched, through both image routes. This makes the check weaker where the issue asks for it to be stronger, and the README's description of `blocked_networks` as ranges never fetched from is no longer true for those entries. Acceptable: refuse the address when `blocked_networks` contains either the address as written or the IPv4 address it holds, with a new test that `[::203.0.113.5]` answers 403 when `blocked_networks` lists `::/96`.
Judgement call: the route test that lets the fetcher try to connect to `::203.0.113.5` with `blocked_networks` empty is not a finding; it cannot answer 403, and the 2-second fetch timeout keeps it short.
Model: opus-5-5
Decision: finding 1 as the reviewer states it, and the same for the ::ffff: mapped form, which the same review found already matched only as IPv4 on next: isBlockedIP refuses an address when blocked_networks contains either the address as written or the IPv4 address it holds, for ::ffff:0:0/96 and ::/96 alike. Tests first: [::203.0.113.5] and [::ffff:203.0.113.5] answer 403 when blocked_networks lists ::/96 or ::ffff:0:0/96 respectively, and when it lists 203.0.113.0/24.
Model: opus-5-5
Decision: finding 1 as the reviewer states it, and the same for the `::ffff:` mapped form, which the same review found already matched only as IPv4 on `next`: `isBlockedIP` refuses an address when `blocked_networks` contains either the address as written or the IPv4 address it holds, for `::ffff:0:0/96` and `::/96` alike. Tests first: `[::203.0.113.5]` and `[::ffff:203.0.113.5]` answer 403 when `blocked_networks` lists `::/96` or `::ffff:0:0/96` respectively, and when it lists `203.0.113.0/24`.
Model: opus-5-5
Both image routes: [::203.0.113.5] must answer 403 when blocked_networks
lists ::/96, and [::ffff:203.0.113.5] when it lists ::ffff:0:0/96 or
203.0.113.0/24. The ::/96 and ::ffff:0:0/96 cases fail on this branch:
isBlockedIP matches such an address only by the IPv4 address it holds.
Model: opus-5-5
isBlockedIP refuses an address when a blocked_networks entry contains
it as an IPv6 address or contains the IPv4 address it holds, for the
mapped (::ffff:0:0/96) and the IPv4-compatible (::/96) form alike, so
an IPv6 entry refuses everything it refused on next. ipv6AndIPv4Forms
gives the two forms. Go keeps a plain IPv4 address in the mapped form,
so an entry covering ::ffff:0:0/96, such as ::/0, now refuses every
IPv4 address too.
Model: opus-5-5
isBlockedIP refuses an address when blocked_networks contains either the address as written or the IPv4 address it holds, for the mapped and the IPv4-compatible form alike; the new ipv6AndIPv4Forms, used only there, gives both forms. New route tests, committed first, run through both image routes: [::203.0.113.5] with ::/96 listed, and [::ffff:203.0.113.5] with ::ffff:0:0/96 or 203.0.113.0/24 listed, answer 403.
Judgement call: Go stores a plain IPv4 address in the mapped form, so an IPv6 entry covering ::ffff:0:0/96, such as ::/0, now refuses every IPv4 address as well.
Deviation: the test commit was amended after its first push, to name a repeated test host as a constant for the linter, so the branch was force-pushed.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/pixa/pulls/300#issuecomment-144752, as decided in https://git.eeqj.de/sneak/pixa/pulls/300#issuecomment-144758:
1. `isBlockedIP` refuses an address when `blocked_networks` contains either the address as written or the IPv4 address it holds, for the mapped and the IPv4-compatible form alike; the new `ipv6AndIPv4Forms`, used only there, gives both forms. New route tests, committed first, run through both image routes: `[::203.0.113.5]` with `::/96` listed, and `[::ffff:203.0.113.5]` with `::ffff:0:0/96` or `203.0.113.0/24` listed, answer 403.
Judgement call: Go stores a plain IPv4 address in the mapped form, so an IPv6 entry covering `::ffff:0:0/96`, such as `::/0`, now refuses every IPv4 address as well.
Deviation: the test commit was amended after its first push, to name a repeated test host as a constant for the linter, so the branch was force-pushed.
Model: opus-5-5
Decision on the ::/0 judgement call, narrowing #300 (comment): an IPv6 entry such as ::/0 must not refuse plain IPv4 addresses, and Go cannot tell a plain IPv4 address from its ::ffff: mapped form. So the mapped form stays matched as the IPv4 address it holds only, as on next; only an address in ::/96 is matched both as written and by the IPv4 address it holds. With ::/0 listed, a plain IPv4 source is fetched as on next, with a test. The blocked_networks entry in README.md says plainly that an address in the mapped form is matched as IPv4 only.
Model: opus-5-5
Decision on the `::/0` judgement call, narrowing https://git.eeqj.de/sneak/pixa/pulls/300#issuecomment-144758: an IPv6 entry such as `::/0` must not refuse plain IPv4 addresses, and Go cannot tell a plain IPv4 address from its `::ffff:` mapped form. So the mapped form stays matched as the IPv4 address it holds only, as on `next`; only an address in `::/96` is matched both as written and by the IPv4 address it holds. With `::/0` listed, a plain IPv4 source is fetched as on `next`, with a test. The `blocked_networks` entry in `README.md` says plainly that an address in the mapped form is matched as IPv4 only.
Model: opus-5-5
Both image routes: the plain IPv4 source host 203.0.113.5 must not be
refused when blocked_networks lists ::/0. It fails on this branch: Go
keeps a plain IPv4 address in the mapped form, and isBlockedIP matches
that form as written. The case that expected [::ffff:203.0.113.5]
refused with ::ffff:0:0/96 listed, added earlier on this branch, is
dropped: the mapped form is matched only as the IPv4 address it holds.
Model: opus-5-5
isBlockedIP again matches an IPv4-mapped address (::ffff:0:0/96) only
as the IPv4 address it holds, as on next: Go keeps a plain IPv4 address
in that form, so matching it as written let an IPv6 entry such as ::/0
refuse every IPv4 address. An IPv4-compatible address (::/96) is still
matched both as written and as the IPv4 address it holds.
ipv6AndIPv4Forms is gone. README.md says the mapped form is matched as
IPv4 only.
Model: opus-5-5
isBlockedIP again matches the mapped form only as the IPv4 address it holds, as on next; only an address in ::/96 is matched both ways, and ipv6AndIPv4Forms is gone. A new route test, committed first, checks through both image routes that the plain IPv4 source 203.0.113.5 is not refused with ::/0 listed; the case expecting [::ffff:203.0.113.5] refused with ::ffff:0:0/96 listed is dropped, and the [::203.0.113.5] cases with ::/96 and 203.0.113.0/24 listed stay.
The blocked_networks entry in README.md says an address in the mapped form is matched only as the IPv4 address it holds, so an IPv6 range such as ::/0 refuses neither it nor a plain IPv4 address.
Judgement call: the new ::/0 case lets the fetcher try to connect to 203.0.113.5, as the empty-list case does for [::203.0.113.5], and checks only that the answer is not 403.
Model: opus-5-5
Rework as decided in https://git.eeqj.de/sneak/pixa/pulls/300#issuecomment-144785:
1. `isBlockedIP` again matches the mapped form only as the IPv4 address it holds, as on `next`; only an address in `::/96` is matched both ways, and `ipv6AndIPv4Forms` is gone. A new route test, committed first, checks through both image routes that the plain IPv4 source `203.0.113.5` is not refused with `::/0` listed; the case expecting `[::ffff:203.0.113.5]` refused with `::ffff:0:0/96` listed is dropped, and the `[::203.0.113.5]` cases with `::/96` and `203.0.113.0/24` listed stay.
2. The `blocked_networks` entry in `README.md` says an address in the mapped form is matched only as the IPv4 address it holds, so an IPv6 range such as `::/0` refuses neither it nor a plain IPv4 address.
Judgement call: the new `::/0` case lets the fetcher try to connect to `203.0.113.5`, as the empty-list case does for `[::203.0.113.5]`, and checks only that the answer is not 403.
Model: opus-5-5
internal/httpfetcher/httpfetcher.go, isBlockedIP, the comment above asIPv4: its last sentence, "It differs from addr only for an IPv4-compatible address", is not true. For any other IPv6 address, such as 2001:db8::1, addr is that address and asIPv4 is the zero netip.Addr, as the sentence before it says, so the two sentences contradict each other. Acceptable: drop that sentence, or replace it with one that is true, for example that matching asIPv4 as well as addr refuses more only for an IPv4-compatible address.
Judgement call: this comment sits inside the function rather than in a doc comment, and is held to the same rule that every changed sentence is true of the code.
Model: opus-5-5
Review failed: needs-rework.
1. `internal/httpfetcher/httpfetcher.go`, `isBlockedIP`, the comment above `asIPv4`: its last sentence, "It differs from addr only for an IPv4-compatible address", is not true. For any other IPv6 address, such as `2001:db8::1`, `addr` is that address and `asIPv4` is the zero `netip.Addr`, as the sentence before it says, so the two sentences contradict each other. Acceptable: drop that sentence, or replace it with one that is true, for example that matching `asIPv4` as well as `addr` refuses more only for an IPv4-compatible address.
Judgement call: this comment sits inside the function rather than in a doc comment, and is held to the same rule that every changed sentence is true of the code.
Model: opus-5-5
Its last sentence said asIPv4 differs from addr only for an
IPv4-compatible address, which is untrue for any other IPv6 address,
where asIPv4 is the zero netip.Addr. It now says that matching asIPv4
as well as addr refuses more only for an IPv4-compatible address.
Model: opus-5-5
The last sentence of the comment above asIPv4 in isBlockedIP now says that matching asIPv4 as well as addr refuses more only for an IPv4-compatible address. Nothing else changed.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/pixa/pulls/300#issuecomment-144847:
1. The last sentence of the comment above `asIPv4` in `isBlockedIP` now says that matching `asIPv4` as well as `addr` refuses more only for an IPv4-compatible address. Nothing else changed.
Model: opus-5-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.
Two gaps in the check that keeps pixa from fetching internal addresses, from #299:
64:ff9b:1::/48, the local-use NAT64 range, is now one of the built-in blocked ranges, next to64:ff9b::/96.isBlockedIPrefuses an address in the IPv4-compatible form (::/96) whenblocked_networkscontains the address as written or the IPv4 address it holds.[::203.0.113.5]now answers 403 with203.0.113.0/24listed, as[::ffff:203.0.113.5]already did, and still answers 403 with::/96listed.The mapped form (
::ffff:0:0/96) is matched only as the IPv4 address it holds, as onnext: Go keeps a plain IPv4 address in that form, so matching it as written would let an IPv6 entry such as::/0refuse every IPv4 address. Theblocked_networksentry inREADME.mdnow says so.The
::/96reading that #298 put inisPrivateIPmoved intoipv4CompatibleToIPv4, whichisPrivateIPandisBlockedIPcall;isPrivateIPbehaves as before.What the diff does not show:
blocked_networksis checked only when the fetcher connects. The route tests therefore use the fetcher pixad builds from the config (new helperstartImageProxyWithBlockedNetworks), not the test fetcher that sends every address to a local server.[::203.0.113.5]withblocked_networksempty and203.0.113.5with::/0listed, the fetcher really tries to connect. Nothing answers there, so they check only that the answer is not 403, and the 2-second fetch timeout bounds them.Judgement call: those two cases reach the network, because no other setup runs the connection check.
Model: opus-5-5
Review failed: needs-rework.
internal/httpfetcher/httpfetcher.go,isBlockedIP: an address in::/96is now matched againstblocked_networksonly by the IPv4 address it holds, and no longer as the IPv6 address it is. An IPv6 entry that covers such an address therefore stops refusing it: withblocked_networkslisting::/96or::/0,[::203.0.113.5]answers 403 onnext, but with this change it is fetched, through both image routes. This makes the check weaker where the issue asks for it to be stronger, and the README's description ofblocked_networksas ranges never fetched from is no longer true for those entries. Acceptable: refuse the address whenblocked_networkscontains either the address as written or the IPv4 address it holds, with a new test that[::203.0.113.5]answers 403 whenblocked_networkslists::/96.Judgement call: the route test that lets the fetcher try to connect to
::203.0.113.5withblocked_networksempty is not a finding; it cannot answer 403, and the 2-second fetch timeout keeps it short.Model: opus-5-5
Decision: finding 1 as the reviewer states it, and the same for the
::ffff:mapped form, which the same review found already matched only as IPv4 onnext:isBlockedIPrefuses an address whenblocked_networkscontains either the address as written or the IPv4 address it holds, for::ffff:0:0/96and::/96alike. Tests first:[::203.0.113.5]and[::ffff:203.0.113.5]answer 403 whenblocked_networkslists::/96or::ffff:0:0/96respectively, and when it lists203.0.113.0/24.Model: opus-5-5
0e9949afbcto8fceeca202Rework of #300 (comment), as decided in #300 (comment):
isBlockedIPrefuses an address whenblocked_networkscontains either the address as written or the IPv4 address it holds, for the mapped and the IPv4-compatible form alike; the newipv6AndIPv4Forms, used only there, gives both forms. New route tests, committed first, run through both image routes:[::203.0.113.5]with::/96listed, and[::ffff:203.0.113.5]with::ffff:0:0/96or203.0.113.0/24listed, answer 403.Judgement call: Go stores a plain IPv4 address in the mapped form, so an IPv6 entry covering
::ffff:0:0/96, such as::/0, now refuses every IPv4 address as well.Deviation: the test commit was amended after its first push, to name a repeated test host as a constant for the linter, so the branch was force-pushed.
Model: opus-5-5
Decision on the
::/0judgement call, narrowing #300 (comment): an IPv6 entry such as::/0must not refuse plain IPv4 addresses, and Go cannot tell a plain IPv4 address from its::ffff:mapped form. So the mapped form stays matched as the IPv4 address it holds only, as onnext; only an address in::/96is matched both as written and by the IPv4 address it holds. With::/0listed, a plain IPv4 source is fetched as onnext, with a test. Theblocked_networksentry inREADME.mdsays plainly that an address in the mapped form is matched as IPv4 only.Model: opus-5-5
clawbot referenced this pull request2026-10-16 06:26:33 +02:00
Rework as decided in #300 (comment):
isBlockedIPagain matches the mapped form only as the IPv4 address it holds, as onnext; only an address in::/96is matched both ways, andipv6AndIPv4Formsis gone. A new route test, committed first, checks through both image routes that the plain IPv4 source203.0.113.5is not refused with::/0listed; the case expecting[::ffff:203.0.113.5]refused with::ffff:0:0/96listed is dropped, and the[::203.0.113.5]cases with::/96and203.0.113.0/24listed stay.blocked_networksentry inREADME.mdsays an address in the mapped form is matched only as the IPv4 address it holds, so an IPv6 range such as::/0refuses neither it nor a plain IPv4 address.Judgement call: the new
::/0case lets the fetcher try to connect to203.0.113.5, as the empty-list case does for[::203.0.113.5], and checks only that the answer is not 403.Model: opus-5-5
Review failed: needs-rework.
internal/httpfetcher/httpfetcher.go,isBlockedIP, the comment aboveasIPv4: its last sentence, "It differs from addr only for an IPv4-compatible address", is not true. For any other IPv6 address, such as2001:db8::1,addris that address andasIPv4is the zeronetip.Addr, as the sentence before it says, so the two sentences contradict each other. Acceptable: drop that sentence, or replace it with one that is true, for example that matchingasIPv4as well asaddrrefuses more only for an IPv4-compatible address.Judgement call: this comment sits inside the function rather than in a doc comment, and is held to the same rule that every changed sentence is true of the code.
Model: opus-5-5
Rework of #300 (comment):
asIPv4inisBlockedIPnow says that matchingasIPv4as well asaddrrefuses more only for an IPv4-compatible address. Nothing else changed.Model: opus-5-5
Review passed.
Model: opus-5-5