On Linux a connection to [::] reaches the host's own loopback, and the SSRF guard let it through with no allowlist set.
What I found on this host (Linux 6.8): a connection to [::] reaches a listener on ::1, and one to 0.0.0.0 reaches a listener on 127.0.0.1. Other addresses in 0.0.0.0/8, such as 0.0.0.1, reach nothing local.
What changed:
0.0.0.0/32 and ::/128 are in alwaysBlockedNetworks, so no ALLOWED_EGRESS_CIDRS value opens them: an allowlist reaches loopback only through an entry that covers a loopback address, never through one that covers only 0.0.0.0 or ::. The rest of 0.0.0.0/8 stays reopenable. ::/128 also joins blockedNetworks, beside 0.0.0.0/8, as the rule above alwaysBlockedNetworks requires of its entries.
ff00::/8 (IPv6 multicast) and 2001:db8::/32 (IPv6 documentation) are refused by default, and an allowlist can reopen them.
Every blockedNetworks entry has a one-line comment.
The rules above alwaysBlockedNetworks and checkIP, the unconditional refusal's message, the README's blocklist description and table, and the matching comments in config.go and the target handler name the unspecified addresses.
In TestDefaultBlocklist_PinnedSet the 0.0.0.0/8 row is now reopenable: false: that test checks each block's first address, and 0.0.0.0 is now unconditional.
Judgement call: the unspecified addresses disclose nothing, so the rule above the unconditional list now gives two reasons to be on it: the two-part test, which governs metadata endpoints, and a separately stated reason for the unspecified addresses.
Judgement call: every existing blockedNetworks entry got a comment, not only the new ones, reading the definition of done literally.
Model: opus-5-5
On Linux a connection to `[::]` reaches the host's own loopback, and the SSRF guard let it through with no allowlist set.
What I found on this host (Linux 6.8): a connection to `[::]` reaches a listener on `::1`, and one to `0.0.0.0` reaches a listener on `127.0.0.1`. Other addresses in `0.0.0.0/8`, such as `0.0.0.1`, reach nothing local.
What changed:
- `0.0.0.0/32` and `::/128` are in `alwaysBlockedNetworks`, so no `ALLOWED_EGRESS_CIDRS` value opens them: an allowlist reaches loopback only through an entry that covers a loopback address, never through one that covers only `0.0.0.0` or `::`. The rest of `0.0.0.0/8` stays reopenable. `::/128` also joins `blockedNetworks`, beside `0.0.0.0/8`, as the rule above `alwaysBlockedNetworks` requires of its entries.
- `ff00::/8` (IPv6 multicast) and `2001:db8::/32` (IPv6 documentation) are refused by default, and an allowlist can reopen them.
- Every `blockedNetworks` entry has a one-line comment.
- The rules above `alwaysBlockedNetworks` and `checkIP`, the unconditional refusal's message, the README's blocklist description and table, and the matching comments in `config.go` and the target handler name the unspecified addresses.
In `TestDefaultBlocklist_PinnedSet` the `0.0.0.0/8` row is now `reopenable: false`: that test checks each block's first address, and `0.0.0.0` is now unconditional.
- Judgement call: the unspecified addresses disclose nothing, so the rule above the unconditional list now gives two reasons to be on it: the two-part test, which governs metadata endpoints, and a separately stated reason for the unspecified addresses.
- Judgement call: every existing `blockedNetworks` entry got a comment, not only the new ones, reading the definition of done literally.
Model: opus-5-5
README.md lines 251–252, the reasoning above alwaysBlockedNetworks in internal/delivery/ssrf.go (line 120), and the commit message say an allowlist opens loopback only by naming it (127.0.0.0/8, ::1). That is untrue: any entry that covers a loopback address opens it, including 0.0.0.0/0 and ::/0, which the same README section warns opens this host's loopback services. Acceptable: state what the change actually guarantees, that an allowlist reaches loopback only through an entry covering a loopback address, never through one that covers only 0.0.0.0 or ::.
internal/delivery/ssrf_allowlist_test.go line 221: the comment on metadataAlwaysRefusedCases says it lists every unconditionally refused address together with an allowlist entry that would otherwise reach it, but the two new unconditional entries, 0.0.0.0/32 and ::/128, are not among its cases. The comment is now untrue, and the change's main promise, that no allowlist reopens the unspecified addresses, is never checked at target creation and at delivery. Acceptable: a case each for 0.0.0.0 and :: under a covering allowlist entry (for example 0.0.0.0/0 and ::/0), refused on both paths with the unconditional refusal's wording, with the test's metadata-only description widened to include them.
Model: opus-5-5
Review failed: two findings.
1. `README.md` lines 251–252, the reasoning above `alwaysBlockedNetworks` in `internal/delivery/ssrf.go` (line 120), and the commit message say an allowlist opens loopback only by naming it (`127.0.0.0/8`, `::1`). That is untrue: any entry that covers a loopback address opens it, including `0.0.0.0/0` and `::/0`, which the same README section warns opens this host's loopback services. Acceptable: state what the change actually guarantees, that an allowlist reaches loopback only through an entry covering a loopback address, never through one that covers only `0.0.0.0` or `::`.
2. `internal/delivery/ssrf_allowlist_test.go` line 221: the comment on `metadataAlwaysRefusedCases` says it lists every unconditionally refused address together with an allowlist entry that would otherwise reach it, but the two new unconditional entries, `0.0.0.0/32` and `::/128`, are not among its cases. The comment is now untrue, and the change's main promise, that no allowlist reopens the unspecified addresses, is never checked at target creation and at delivery. Acceptable: a case each for `0.0.0.0` and `::` under a covering allowlist entry (for example `0.0.0.0/0` and `::/0`), refused on both paths with the unconditional refusal's wording, with the test's metadata-only description widened to include them.
Model: opus-5-5
The README, the reasoning above alwaysBlockedNetworks, the commit message and the PR body now say an allowlist reaches loopback only through an entry that covers a loopback address (such as 127.0.0.0/8, ::1 or 0.0.0.0/0), never through one that covers only 0.0.0.0 or :: (such as 0.0.0.0/8).
metadataAlwaysRefusedCases gains 0.0.0.0 under 0.0.0.0/0 and :: under ::/0, each refused at target creation and at delivery with the unconditional refusal's wording. Its description and the test's now name the unspecified addresses.
Model: opus-5-5
Rework, rebased onto `next`:
1. The README, the reasoning above `alwaysBlockedNetworks`, the commit message and the PR body now say an allowlist reaches loopback only through an entry that covers a loopback address (such as `127.0.0.0/8`, `::1` or `0.0.0.0/0`), never through one that covers only `0.0.0.0` or `::` (such as `0.0.0.0/8`).
2. `metadataAlwaysRefusedCases` gains `0.0.0.0` under `0.0.0.0/0` and `::` under `::/0`, each refused at target creation and at delivery with the unconditional refusal's wording. Its description and the test's now name the unspecified addresses.
Model: opus-5-5
internal/delivery/ssrf.go lines 82-95 and 117-124: the rule above alwaysBlockedNetworks still says an address belongs on the list only if both halves of the criterion hold, and tells maintainers to use the two halves to refuse a candidate, yet the same comment admits 0.0.0.0/32 and ::/128 fail the second half and lists them anyway. A rule the list breaks is not true of the tree, and applied as written it refuses the two entries this change adds. internal/config/config.go line 205 repeats it ("the criterion it is built from"). Acceptable: state the rule so every entry meets it, for example the two-part test governs metadata endpoints (as the README already words it: "A metadata address is on the list below when ...") and the unspecified addresses are a second, separately stated reason to be on the list.
internal/handlers/source_management.go line 1564: "link-local, the unspecified addresses and the other unconditional metadata addresses" calls the unspecified addresses metadata addresses, which they are not (the README says they hand out nothing). Acceptable: name them without classing them as metadata addresses, for example by dropping "other".
Model: opus-5-5
Review failed: two findings.
1. `internal/delivery/ssrf.go` lines 82-95 and 117-124: the rule above `alwaysBlockedNetworks` still says an address belongs on the list only if both halves of the criterion hold, and tells maintainers to use the two halves to refuse a candidate, yet the same comment admits `0.0.0.0/32` and `::/128` fail the second half and lists them anyway. A rule the list breaks is not true of the tree, and applied as written it refuses the two entries this change adds. `internal/config/config.go` line 205 repeats it ("the criterion it is built from"). Acceptable: state the rule so every entry meets it, for example the two-part test governs metadata endpoints (as the README already words it: "A metadata address is on the list below when ...") and the unspecified addresses are a second, separately stated reason to be on the list.
2. `internal/handlers/source_management.go` line 1564: "link-local, the unspecified addresses and the other unconditional metadata addresses" calls the unspecified addresses metadata addresses, which they are not (the README says they hand out nothing). Acceptable: name them without classing them as metadata addresses, for example by dropping "other".
Model: opus-5-5
On Linux a connection to the unspecified address [::] or 0.0.0.0
reaches the host's own loopback, and the SSRF guard let [::] through.
Both unspecified addresses now sit in alwaysBlockedNetworks, so an
allowlist reaches loopback only through an entry that covers a
loopback address, never through one that covers only 0.0.0.0 or ::;
::/128 joins the default blocklist beside 0.0.0.0/8. IPv6 multicast
(ff00::/8) and documentation space (2001:db8::/32) are refused by
default. Every default blocklist entry gets a one-line comment, and the
README, the rules above each list, the two pinning tests and the
allowlist refusal test follow.
Model: opus-5-5
The rule above alwaysBlockedNetworks now gives two reasons to be on the list: the two-part test, which governs metadata endpoints (the link-local blocks and the cloud metadata endpoints outside them), and a separately stated reason for the unspecified addresses. Its comment in internal/config/config.go now points to the list and why each entry is on it, not to a single criterion.
The comment in internal/handlers/source_management.go reads "link-local, the unspecified addresses and the unconditional metadata addresses", no longer classing the unspecified addresses as metadata addresses.
The PR body's first judgement-call line is reworded to match.
Model: opus-5-5
Rework, rebased onto `next`:
1. The rule above `alwaysBlockedNetworks` now gives two reasons to be on the list: the two-part test, which governs metadata endpoints (the link-local blocks and the cloud metadata endpoints outside them), and a separately stated reason for the unspecified addresses. Its comment in `internal/config/config.go` now points to the list and why each entry is on it, not to a single criterion.
2. The comment in `internal/handlers/source_management.go` reads "link-local, the unspecified addresses and the unconditional metadata addresses", no longer classing the unspecified addresses as metadata addresses.
The PR body's first judgement-call line is reworded to match.
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.
On Linux a connection to
[::]reaches the host's own loopback, and the SSRF guard let it through with no allowlist set.What I found on this host (Linux 6.8): a connection to
[::]reaches a listener on::1, and one to0.0.0.0reaches a listener on127.0.0.1. Other addresses in0.0.0.0/8, such as0.0.0.1, reach nothing local.What changed:
0.0.0.0/32and::/128are inalwaysBlockedNetworks, so noALLOWED_EGRESS_CIDRSvalue opens them: an allowlist reaches loopback only through an entry that covers a loopback address, never through one that covers only0.0.0.0or::. The rest of0.0.0.0/8stays reopenable.::/128also joinsblockedNetworks, beside0.0.0.0/8, as the rule abovealwaysBlockedNetworksrequires of its entries.ff00::/8(IPv6 multicast) and2001:db8::/32(IPv6 documentation) are refused by default, and an allowlist can reopen them.blockedNetworksentry has a one-line comment.alwaysBlockedNetworksandcheckIP, the unconditional refusal's message, the README's blocklist description and table, and the matching comments inconfig.goand the target handler name the unspecified addresses.In
TestDefaultBlocklist_PinnedSetthe0.0.0.0/8row is nowreopenable: false: that test checks each block's first address, and0.0.0.0is now unconditional.blockedNetworksentry got a comment, not only the new ones, reading the definition of done literally.Model: opus-5-5
Review failed: two findings.
README.mdlines 251–252, the reasoning abovealwaysBlockedNetworksininternal/delivery/ssrf.go(line 120), and the commit message say an allowlist opens loopback only by naming it (127.0.0.0/8,::1). That is untrue: any entry that covers a loopback address opens it, including0.0.0.0/0and::/0, which the same README section warns opens this host's loopback services. Acceptable: state what the change actually guarantees, that an allowlist reaches loopback only through an entry covering a loopback address, never through one that covers only0.0.0.0or::.internal/delivery/ssrf_allowlist_test.goline 221: the comment onmetadataAlwaysRefusedCasessays it lists every unconditionally refused address together with an allowlist entry that would otherwise reach it, but the two new unconditional entries,0.0.0.0/32and::/128, are not among its cases. The comment is now untrue, and the change's main promise, that no allowlist reopens the unspecified addresses, is never checked at target creation and at delivery. Acceptable: a case each for0.0.0.0and::under a covering allowlist entry (for example0.0.0.0/0and::/0), refused on both paths with the unconditional refusal's wording, with the test's metadata-only description widened to include them.Model: opus-5-5
aadbab8cf0to3699f37b76Rework, rebased onto
next:alwaysBlockedNetworks, the commit message and the PR body now say an allowlist reaches loopback only through an entry that covers a loopback address (such as127.0.0.0/8,::1or0.0.0.0/0), never through one that covers only0.0.0.0or::(such as0.0.0.0/8).metadataAlwaysRefusedCasesgains0.0.0.0under0.0.0.0/0and::under::/0, each refused at target creation and at delivery with the unconditional refusal's wording. Its description and the test's now name the unspecified addresses.Model: opus-5-5
Review failed: two findings.
internal/delivery/ssrf.golines 82-95 and 117-124: the rule abovealwaysBlockedNetworksstill says an address belongs on the list only if both halves of the criterion hold, and tells maintainers to use the two halves to refuse a candidate, yet the same comment admits0.0.0.0/32and::/128fail the second half and lists them anyway. A rule the list breaks is not true of the tree, and applied as written it refuses the two entries this change adds.internal/config/config.goline 205 repeats it ("the criterion it is built from"). Acceptable: state the rule so every entry meets it, for example the two-part test governs metadata endpoints (as the README already words it: "A metadata address is on the list below when ...") and the unspecified addresses are a second, separately stated reason to be on the list.internal/handlers/source_management.goline 1564: "link-local, the unspecified addresses and the other unconditional metadata addresses" calls the unspecified addresses metadata addresses, which they are not (the README says they hand out nothing). Acceptable: name them without classing them as metadata addresses, for example by dropping "other".Model: opus-5-5
3699f37b76to57140156d7Rework, rebased onto
next:alwaysBlockedNetworksnow gives two reasons to be on the list: the two-part test, which governs metadata endpoints (the link-local blocks and the cloud metadata endpoints outside them), and a separately stated reason for the unspecified addresses. Its comment ininternal/config/config.gonow points to the list and why each entry is on it, not to a single criterion.internal/handlers/source_management.goreads "link-local, the unspecified addresses and the unconditional metadata addresses", no longer classing the unspecified addresses as metadata addresses.The PR body's first judgement-call line is reworded to match.
Model: opus-5-5
Review passed.
Model: opus-5-5