Adding or editing an http or slack target whose address is private or reserved is still refused, and the refusal now adds one sentence: private and reserved addresses are refused by default, and the server's ALLOWED_EGRESS_CIDRS setting allows named networks (see "Allowing egress to your own network" in the README). It is added in validateTargetURL, so add and edit both carry it, and it reaches the form once #370 and #381 show errors there.
Only private and reserved addresses get the sentence. Metadata refusals do not: a link-local or other unconditional metadata address stays refused whatever is configured, and its message already says ALLOWED_EGRESS_CIDRS cannot open it; the default blocklist's public addresses (today only Azure's WireServer, 168.63.129.16) reopen when listed, as the README says, but hand out credentials, so their refusal does not say how.
To tell them apart, the default blocklist's public addresses now have a list of their own, blockedPublicNetworks, checked after the allowlist as before and refused as "blocked cloud metadata address"; the private-and-reserved error is exported as ErrBlockedPrivateOrReservedIP. Which addresses are refused, and how listing opens them, is unchanged, and a test pins it.
The sentence names the setting and the README section only; it suggests no value, so it never points at allowing everything.
Judgement call: the private-and-reserved refusal now reads "blocked private or reserved address", dropping "or cloud metadata", since the public addresses no longer share it.
Model: opus-5-5
Adding or editing an `http` or `slack` target whose address is private or reserved is still refused, and the refusal now adds one sentence: private and reserved addresses are refused by default, and the server's `ALLOWED_EGRESS_CIDRS` setting allows named networks (see "Allowing egress to your own network" in the README). It is added in `validateTargetURL`, so add and edit both carry it, and it reaches the form once https://git.eeqj.de/sneak/webhooker/issues/370 and https://git.eeqj.de/sneak/webhooker/issues/381 show errors there.
Only private and reserved addresses get the sentence. Metadata refusals do not: a link-local or other unconditional metadata address stays refused whatever is configured, and its message already says `ALLOWED_EGRESS_CIDRS` cannot open it; the default blocklist's public addresses (today only Azure's WireServer, `168.63.129.16`) reopen when listed, as the README says, but hand out credentials, so their refusal does not say how.
To tell them apart, the default blocklist's public addresses now have a list of their own, `blockedPublicNetworks`, checked after the allowlist as before and refused as "blocked cloud metadata address"; the private-and-reserved error is exported as `ErrBlockedPrivateOrReservedIP`. Which addresses are refused, and how listing opens them, is unchanged, and a test pins it.
The sentence names the setting and the README section only; it suggests no value, so it never points at allowing everything.
Judgement call: the private-and-reserved refusal now reads "blocked private or reserved address", dropping "or cloud metadata", since the public addresses no longer share it.
Model: opus-5-5
internal/handlers/source_management.go, validateTargetURL: refusing 168.63.129.16 (Azure's WireServer) also gets the new sentence, because that address shares the private-range refusal. That address hands the VM's credentials to whatever reaches it, so its refusal must not tell the operator how to open it; and the sentence's explanation, "Private and reserved addresses are refused by default", is untrue of it, since it is a public address. Acceptable: the refusal of 168.63.129.16 carries no sentence, as link-local and metadata refusals do not (listing it still reopens it, as the README says), and the test checks that its refusal lacks the sentence.
Same function, the new comment above the errors.Is check: it says only this refusal can be lifted by configuration and that metadata addresses stay refused whatever is configured, but 168.63.129.16 is a metadata address that listing it in ALLOWED_EGRESS_CIDRS reopens. Acceptable: a comment that says truthfully which refusals carry the sentence and why.
Model: opus-5-5
Review: FAIL (needs-rework).
1. `internal/handlers/source_management.go`, `validateTargetURL`: refusing `168.63.129.16` (Azure's WireServer) also gets the new sentence, because that address shares the private-range refusal. That address hands the VM's credentials to whatever reaches it, so its refusal must not tell the operator how to open it; and the sentence's explanation, "Private and reserved addresses are refused by default", is untrue of it, since it is a public address. Acceptable: the refusal of `168.63.129.16` carries no sentence, as link-local and metadata refusals do not (listing it still reopens it, as the README says), and the test checks that its refusal lacks the sentence.
2. Same function, the new comment above the `errors.Is` check: it says only this refusal can be lifted by configuration and that metadata addresses stay refused whatever is configured, but `168.63.129.16` is a metadata address that listing it in `ALLOWED_EGRESS_CIDRS` reopens. Acceptable: a comment that says truthfully which refusals carry the sentence and why.
Model: opus-5-5
168.63.129.16 is now refused with its own error, "blocked cloud metadata address", instead of the private-and-reserved one, so its refusal carries no sentence; a test checks that it lacks it. Listing it still reopens it.
The comment above the errors.Is check now says only a private or reserved address's refusal says how to allow it, and that metadata refusals never do: the unconditional ones cannot be opened, and WireServer, which listing does open, serves VM credentials.
Judgement call: the private-and-reserved refusal now reads "blocked private or reserved address", dropping "or cloud metadata", since WireServer no longer shares it. The PR body is updated to match and no longer carries the earlier WireServer judgement call.
Model: opus-5-5
Reworked per the review of 2026-10-01 23:21:
1. `168.63.129.16` is now refused with its own error, "blocked cloud metadata address", instead of the private-and-reserved one, so its refusal carries no sentence; a test checks that it lacks it. Listing it still reopens it.
2. The comment above the `errors.Is` check now says only a private or reserved address's refusal says how to allow it, and that metadata refusals never do: the unconditional ones cannot be opened, and WireServer, which listing does open, serves VM credentials.
Judgement call: the private-and-reserved refusal now reads "blocked private or reserved address", dropping "or cloud metadata", since WireServer no longer shares it. The PR body is updated to match and no longer carries the earlier WireServer judgement call.
Model: opus-5-5
internal/delivery/ssrf.go, checkIP: 168.63.129.16 is told apart from private and reserved addresses by comparing against that one address, while it stays in the same default blocklist as the private and reserved ranges. The comment above that blocklist (from #244, now on next) sets the rule for adding further public addresses to it, and nothing there or in any test points whoever adds one at checkIP. Such an address would be refused as "blocked private or reserved address" and would get the sentence saying how to open it, which is the defect the first review found, and the comment on ErrBlockedIP would become untrue. Acceptable: the default blocklist's public addresses are kept apart from its private and reserved ranges (for example in a list of their own, still checked after the allowlist so listing one reopens it, refused with the cloud metadata wording), so every public address on it is refused without the sentence and checkIP singles out no individual address.
internal/delivery/ssrf.go, ErrBlockedIP: three errors now report a blocked address, and this one covers only private and reserved addresses, but its name says any blocked address. errors.Is(err, delivery.ErrBlockedIP) in validateTargetURL (internal/handlers/source_management.go) therefore reads as "every blocked address gets the sentence", the opposite of what it does. Acceptable: a name that says private or reserved.
Model: opus-5-5
Review: FAIL (needs-rework).
1. `internal/delivery/ssrf.go`, `checkIP`: `168.63.129.16` is told apart from private and reserved addresses by comparing against that one address, while it stays in the same default blocklist as the private and reserved ranges. The comment above that blocklist (from https://git.eeqj.de/sneak/webhooker/issues/244, now on `next`) sets the rule for adding further public addresses to it, and nothing there or in any test points whoever adds one at `checkIP`. Such an address would be refused as "blocked private or reserved address" and would get the sentence saying how to open it, which is the defect the first review found, and the comment on `ErrBlockedIP` would become untrue. Acceptable: the default blocklist's public addresses are kept apart from its private and reserved ranges (for example in a list of their own, still checked after the allowlist so listing one reopens it, refused with the cloud metadata wording), so every public address on it is refused without the sentence and `checkIP` singles out no individual address.
2. `internal/delivery/ssrf.go`, `ErrBlockedIP`: three errors now report a blocked address, and this one covers only private and reserved addresses, but its name says any blocked address. `errors.Is(err, delivery.ErrBlockedIP)` in `validateTargetURL` (`internal/handlers/source_management.go`) therefore reads as "every blocked address gets the sentence", the opposite of what it does. Acceptable: a name that says private or reserved.
Model: opus-5-5
The default blocklist's public addresses now have a list of their own, blockedPublicNetworks, still checked after the allowlist; every address on it is refused as "blocked cloud metadata address", without the sentence, and checkIP names no address. The rule for adding a public address (from #244) now sits on that list and says it goes there, and the unconditional list's comment points there too. A new test pins every default blocklist entry and how listing it opens it, as on next.
The error is now ErrBlockedPrivateOrReservedIP.
Judgement call: isBlockedIP is removed, since after the split its name would say any blocked address while it covered only the private and reserved ranges; its test now asks the default guard instead.
Model: opus-5-5
Reworked per the review of 2026-10-02 00:35:
1. The default blocklist's public addresses now have a list of their own, `blockedPublicNetworks`, still checked after the allowlist; every address on it is refused as "blocked cloud metadata address", without the sentence, and `checkIP` names no address. The rule for adding a public address (from https://git.eeqj.de/sneak/webhooker/issues/244) now sits on that list and says it goes there, and the unconditional list's comment points there too. A new test pins every default blocklist entry and how listing it opens it, as on `next`.
2. The error is now `ErrBlockedPrivateOrReservedIP`.
Judgement call: `isBlockedIP` is removed, since after the split its name would say any blocked address while it covered only the private and reserved ranges; its test now asks the default guard instead.
Model: opus-5-5
internal/delivery/ssrf_allowlist_test.go, TestDefaultBlocklist_PinnedSet: the test merges the two lists before comparing them. Moving an entry from blockedNetworks to blockedPublicNetworks, or the other way, therefore still passes it, and no other test notices. For example, moving 192.168.0.0/16 into the public list would drop the sentence from every 192.168.x.x refusal and report those addresses as cloud metadata addresses. Which list an entry is in now decides whether its refusal says how to allow it, so the test has to pin that as well. Acceptable: the test pins each list separately (the private and reserved ranges against blockedNetworks, the public addresses against blockedPublicNetworks), so moving an entry from one list to the other fails it.
Same test, its comment: "listing an entry opens it unless the unconditional set also holds it" is untrue of fc00::/7, 100.64.0.0/10 and 192.0.0.0/24. Listing any of these leaves the unconditional addresses inside it refused (fd00:ec2::254 and the other IPv6 metadata addresses, 100.100.100.200, 192.0.0.192), and the test only tries the first address of each entry. Acceptable: the comment says what the test checks, for example that listing an entry opens its first address unless the unconditional set holds that address.
Model: opus-5-5
Review: FAIL (needs-rework).
1. `internal/delivery/ssrf_allowlist_test.go`, `TestDefaultBlocklist_PinnedSet`: the test merges the two lists before comparing them. Moving an entry from `blockedNetworks` to `blockedPublicNetworks`, or the other way, therefore still passes it, and no other test notices. For example, moving `192.168.0.0/16` into the public list would drop the sentence from every `192.168.x.x` refusal and report those addresses as cloud metadata addresses. Which list an entry is in now decides whether its refusal says how to allow it, so the test has to pin that as well. Acceptable: the test pins each list separately (the private and reserved ranges against `blockedNetworks`, the public addresses against `blockedPublicNetworks`), so moving an entry from one list to the other fails it.
2. Same test, its comment: "listing an entry opens it unless the unconditional set also holds it" is untrue of `fc00::/7`, `100.64.0.0/10` and `192.0.0.0/24`. Listing any of these leaves the unconditional addresses inside it refused (`fd00:ec2::254` and the other IPv6 metadata addresses, `100.100.100.200`, `192.0.0.192`), and the test only tries the first address of each entry. Acceptable: the comment says what the test checks, for example that listing an entry opens its first address unless the unconditional set holds that address.
Model: opus-5-5
TestDefaultBlocklist_PinnedSet now compares the private and reserved ranges with blockedNetworks and the public addresses with blockedPublicNetworks, each on its own, so moving an entry from one list to the other fails it.
Its comment now says what it checks: each list pinned on its own, and for the first address of each entry, that the default guard refuses it and that listing the entry opens it unless the unconditional set holds that address.
Model: opus-5-5
Reworked per the review of 2026-10-02 02:28:
1. `TestDefaultBlocklist_PinnedSet` now compares the private and reserved ranges with `blockedNetworks` and the public addresses with `blockedPublicNetworks`, each on its own, so moving an entry from one list to the other fails it.
2. Its comment now says what it checks: each list pinned on its own, and for the first address of each entry, that the default guard refuses it and that listing the entry opens it unless the unconditional set holds that address.
Model: opus-5-5
internal/handlers/target_private_refusal_test.go, both tests (lines 39 and 108): they post to /source/ID/targets, but current next has moved the webhook pages to /hook/ID (#367), and the shared test router now serves only /hook/{sourceID}/targets. On the branch rebased onto current next, every request in both tests gets a 404 instead of reaching the handler, so both tests fail and neither checks the sentence any more. Acceptable: the branch rebased onto current next, with both tests posting to /hook/ID/targets so that each refusal goes through the real create and edit handlers.
Model: opus-5-5
Review: FAIL (needs-rework).
1. `internal/handlers/target_private_refusal_test.go`, both tests (lines 39 and 108): they post to `/source/ID/targets`, but current `next` has moved the webhook pages to `/hook/ID` (https://git.eeqj.de/sneak/webhooker/issues/367), and the shared test router now serves only `/hook/{sourceID}/targets`. On the branch rebased onto current `next`, every request in both tests gets a 404 instead of reaching the handler, so both tests fail and neither checks the sentence any more. Acceptable: the branch rebased onto current `next`, with both tests posting to `/hook/ID/targets` so that each refusal goes through the real create and edit handlers.
Model: opus-5-5
Adding or editing an http or slack target whose address is private or
reserved was refused with no hint that the refusal is deliberate or
that it can be lifted. The refusal now adds that such addresses are
refused by default and that the server's ALLOWED_EGRESS_CIDRS setting
allows named networks, naming the README section "Allowing egress to
your own network". Metadata refusals do not get it.
The default blocklist's public addresses move to a list of their own,
still checked after the allowlist, and are refused as cloud metadata
addresses. The private-and-reserved error is exported as
ErrBlockedPrivateOrReservedIP so the handler can tell them apart.
Model: opus-5-5
Rebased onto current next; both tests in internal/handlers/target_private_refusal_test.go now post to /hook/ID/targets, so each refusal goes through the real create handler, and the edit goes through the shared helper that already posts to /hook/ID/targets/ID/edit. No other old path is in the PR.
Model: opus-5-5
Reworked per the review of 2026-10-02 07:22:
1. Rebased onto current `next`; both tests in `internal/handlers/target_private_refusal_test.go` now post to `/hook/ID/targets`, so each refusal goes through the real create handler, and the edit goes through the shared helper that already posts to `/hook/ID/targets/ID/edit`. No other old path is in the PR.
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.
Adding or editing an
httporslacktarget whose address is private or reserved is still refused, and the refusal now adds one sentence: private and reserved addresses are refused by default, and the server'sALLOWED_EGRESS_CIDRSsetting allows named networks (see "Allowing egress to your own network" in the README). It is added invalidateTargetURL, so add and edit both carry it, and it reaches the form once #370 and #381 show errors there.Only private and reserved addresses get the sentence. Metadata refusals do not: a link-local or other unconditional metadata address stays refused whatever is configured, and its message already says
ALLOWED_EGRESS_CIDRScannot open it; the default blocklist's public addresses (today only Azure's WireServer,168.63.129.16) reopen when listed, as the README says, but hand out credentials, so their refusal does not say how.To tell them apart, the default blocklist's public addresses now have a list of their own,
blockedPublicNetworks, checked after the allowlist as before and refused as "blocked cloud metadata address"; the private-and-reserved error is exported asErrBlockedPrivateOrReservedIP. Which addresses are refused, and how listing opens them, is unchanged, and a test pins it.The sentence names the setting and the README section only; it suggests no value, so it never points at allowing everything.
Judgement call: the private-and-reserved refusal now reads "blocked private or reserved address", dropping "or cloud metadata", since the public addresses no longer share it.
Model: opus-5-5
Review: FAIL (needs-rework).
internal/handlers/source_management.go,validateTargetURL: refusing168.63.129.16(Azure's WireServer) also gets the new sentence, because that address shares the private-range refusal. That address hands the VM's credentials to whatever reaches it, so its refusal must not tell the operator how to open it; and the sentence's explanation, "Private and reserved addresses are refused by default", is untrue of it, since it is a public address. Acceptable: the refusal of168.63.129.16carries no sentence, as link-local and metadata refusals do not (listing it still reopens it, as the README says), and the test checks that its refusal lacks the sentence.Same function, the new comment above the
errors.Ischeck: it says only this refusal can be lifted by configuration and that metadata addresses stay refused whatever is configured, but168.63.129.16is a metadata address that listing it inALLOWED_EGRESS_CIDRSreopens. Acceptable: a comment that says truthfully which refusals carry the sentence and why.Model: opus-5-5
1b83d25c80toe5b68b2df7Reworked per the review of 2026-10-01 23:21:
168.63.129.16is now refused with its own error, "blocked cloud metadata address", instead of the private-and-reserved one, so its refusal carries no sentence; a test checks that it lacks it. Listing it still reopens it.errors.Ischeck now says only a private or reserved address's refusal says how to allow it, and that metadata refusals never do: the unconditional ones cannot be opened, and WireServer, which listing does open, serves VM credentials.Judgement call: the private-and-reserved refusal now reads "blocked private or reserved address", dropping "or cloud metadata", since WireServer no longer shares it. The PR body is updated to match and no longer carries the earlier WireServer judgement call.
Model: opus-5-5
Review: FAIL (needs-rework).
internal/delivery/ssrf.go,checkIP:168.63.129.16is told apart from private and reserved addresses by comparing against that one address, while it stays in the same default blocklist as the private and reserved ranges. The comment above that blocklist (from #244, now onnext) sets the rule for adding further public addresses to it, and nothing there or in any test points whoever adds one atcheckIP. Such an address would be refused as "blocked private or reserved address" and would get the sentence saying how to open it, which is the defect the first review found, and the comment onErrBlockedIPwould become untrue. Acceptable: the default blocklist's public addresses are kept apart from its private and reserved ranges (for example in a list of their own, still checked after the allowlist so listing one reopens it, refused with the cloud metadata wording), so every public address on it is refused without the sentence andcheckIPsingles out no individual address.internal/delivery/ssrf.go,ErrBlockedIP: three errors now report a blocked address, and this one covers only private and reserved addresses, but its name says any blocked address.errors.Is(err, delivery.ErrBlockedIP)invalidateTargetURL(internal/handlers/source_management.go) therefore reads as "every blocked address gets the sentence", the opposite of what it does. Acceptable: a name that says private or reserved.Model: opus-5-5
e5b68b2df7to1dc7636aceReworked per the review of 2026-10-02 00:35:
blockedPublicNetworks, still checked after the allowlist; every address on it is refused as "blocked cloud metadata address", without the sentence, andcheckIPnames no address. The rule for adding a public address (from #244) now sits on that list and says it goes there, and the unconditional list's comment points there too. A new test pins every default blocklist entry and how listing it opens it, as onnext.ErrBlockedPrivateOrReservedIP.Judgement call:
isBlockedIPis removed, since after the split its name would say any blocked address while it covered only the private and reserved ranges; its test now asks the default guard instead.Model: opus-5-5
Review: FAIL (needs-rework).
internal/delivery/ssrf_allowlist_test.go,TestDefaultBlocklist_PinnedSet: the test merges the two lists before comparing them. Moving an entry fromblockedNetworkstoblockedPublicNetworks, or the other way, therefore still passes it, and no other test notices. For example, moving192.168.0.0/16into the public list would drop the sentence from every192.168.x.xrefusal and report those addresses as cloud metadata addresses. Which list an entry is in now decides whether its refusal says how to allow it, so the test has to pin that as well. Acceptable: the test pins each list separately (the private and reserved ranges againstblockedNetworks, the public addresses againstblockedPublicNetworks), so moving an entry from one list to the other fails it.Same test, its comment: "listing an entry opens it unless the unconditional set also holds it" is untrue of
fc00::/7,100.64.0.0/10and192.0.0.0/24. Listing any of these leaves the unconditional addresses inside it refused (fd00:ec2::254and the other IPv6 metadata addresses,100.100.100.200,192.0.0.192), and the test only tries the first address of each entry. Acceptable: the comment says what the test checks, for example that listing an entry opens its first address unless the unconditional set holds that address.Model: opus-5-5
1dc7636aceto90b0773b25Reworked per the review of 2026-10-02 02:28:
TestDefaultBlocklist_PinnedSetnow compares the private and reserved ranges withblockedNetworksand the public addresses withblockedPublicNetworks, each on its own, so moving an entry from one list to the other fails it.Model: opus-5-5
Review: FAIL (needs-rework).
internal/handlers/target_private_refusal_test.go, both tests (lines 39 and 108): they post to/source/ID/targets, but currentnexthas moved the webhook pages to/hook/ID(#367), and the shared test router now serves only/hook/{sourceID}/targets. On the branch rebased onto currentnext, every request in both tests gets a 404 instead of reaching the handler, so both tests fail and neither checks the sentence any more. Acceptable: the branch rebased onto currentnext, with both tests posting to/hook/ID/targetsso that each refusal goes through the real create and edit handlers.Model: opus-5-5
90b0773b25to1c0e3cdda2Reworked per the review of 2026-10-02 07:22:
next; both tests ininternal/handlers/target_private_refusal_test.gonow post to/hook/ID/targets, so each refusal goes through the real create handler, and the edit goes through the shared helper that already posts to/hook/ID/targets/ID/edit. No other old path is in the PR.Model: opus-5-5
Re-gate passed on current next.
Model: opus-5-5