internal/httpfetcher had only helper-level unit tests; the paths that carry real risk were untested. This adds tests (no non-test code changed) exercising the full Fetch path:
Redirect SSRF: a 302 to a link-local address is refused with ErrSSRFBlocked and the private target is never dialed, while a redirect to a public target and a two-hop chain still succeed.
Per-host semaphore release on the error path, on close after a full read, and on close after a partial read — each proven by saturating a one-slot host and confirming a later fetch still proceeds.
MaxResponseSize end-to-end: an oversized body yields a bounded read and ErrResponseTooLarge, not a truncated success.
Non-2xx and disallowed content-type rejection.
ssrfSafeDialer blocking private, link-local and loopback targets and allowing a public one.
What the diff does not show: to run the real SSRF checks against a loopback server, the upstream host is a TEST-NET-1 literal (192.0.2.10) that resolves with no DNS and that the private-IP check treats as public; a recording dialer routes it to the test server and records every dial, so the redirect test can assert the private address was never contacted. Full DNS-rebinding simulation (a resolver answering public then private) is out of scope: it would mean mutating the global resolver under parallel race tests. The dial-time re-resolution that closes that window is tested directly instead.
No defect found in the covered code.
Model: opus-4-8
Closes https://git.eeqj.de/sneak/pixa/issues/78.
`internal/httpfetcher` had only helper-level unit tests; the paths that carry real risk were untested. This adds tests (no non-test code changed) exercising the full `Fetch` path:
- Redirect SSRF: a 302 to a link-local address is refused with `ErrSSRFBlocked` and the private target is never dialed, while a redirect to a public target and a two-hop chain still succeed.
- Per-host semaphore release on the error path, on close after a full read, and on close after a partial read — each proven by saturating a one-slot host and confirming a later fetch still proceeds.
- `MaxResponseSize` end-to-end: an oversized body yields a bounded read and `ErrResponseTooLarge`, not a truncated success.
- Non-2xx and disallowed content-type rejection.
- `ssrfSafeDialer` blocking private, link-local and loopback targets and allowing a public one.
What the diff does not show: to run the real SSRF checks against a loopback server, the upstream host is a TEST-NET-1 literal (192.0.2.10) that resolves with no DNS and that the private-IP check treats as public; a recording dialer routes it to the test server and records every dial, so the redirect test can assert the private address was never contacted. Full DNS-rebinding simulation (a resolver answering public then private) is out of scope: it would mean mutating the global resolver under parallel race tests. The dial-time re-resolution that closes that window is tested directly instead.
No defect found in the covered code.
Model: opus-4-8
Adds httptest-server tests for the risk-bearing paths that only had
helper-level coverage: the CheckRedirect validator (a 302 to a
link-local address is refused with ErrSSRFBlocked and never dialed,
while a redirect chain and a redirect to a public target still
succeed), per-host semaphore release on the error, full-read-close and
partial-read-close paths (proven by saturating a one-slot host), the
MaxResponseSize limit through Fetch, non-2xx and disallowed
content-type rejection, and ssrfSafeDialer's block of a private target.
The upstream host is a TEST-NET-1 literal (192.0.2.10) that isPrivateIP
treats as public and that resolves with no DNS, with a recording dialer
routing it to the local server so the real SSRF checks still run. Full
DNS-rebinding simulation is out of scope; the dial-time private-target
block that closes that window is tested directly.
Model: opus-4-8
Reviewed against issue #78 on the head rebased onto current next: the redirect, semaphore-release, response-size, non-2xx and content-type, and dialer tests each fail when the protection they cover is removed, use no sleeps or real network, run clean under -race, touch only test code, and make check is green — PASS.
Model: opus-4-8
Reviewed against issue https://git.eeqj.de/sneak/pixa/issues/78 on the head rebased onto current `next`: the redirect, semaphore-release, response-size, non-2xx and content-type, and dialer tests each fail when the protection they cover is removed, use no sleeps or real network, run clean under `-race`, touch only test code, and `make check` is green — PASS.
Model: opus-4-8
clawbot
merged commit 6f416eac31 into next2026-09-21 19:59:27 +02: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.
Closes #78.
internal/httpfetcherhad only helper-level unit tests; the paths that carry real risk were untested. This adds tests (no non-test code changed) exercising the fullFetchpath:ErrSSRFBlockedand the private target is never dialed, while a redirect to a public target and a two-hop chain still succeed.MaxResponseSizeend-to-end: an oversized body yields a bounded read andErrResponseTooLarge, not a truncated success.ssrfSafeDialerblocking private, link-local and loopback targets and allowing a public one.What the diff does not show: to run the real SSRF checks against a loopback server, the upstream host is a TEST-NET-1 literal (192.0.2.10) that resolves with no DNS and that the private-IP check treats as public; a recording dialer routes it to the test server and records every dial, so the redirect test can assert the private address was never contacted. Full DNS-rebinding simulation (a resolver answering public then private) is out of scope: it would mean mutating the global resolver under parallel race tests. The dial-time re-resolution that closes that window is tested directly instead.
No defect found in the covered code.
Model: opus-4-8
Reviewed against issue #78 on the head rebased onto current
next: the redirect, semaphore-release, response-size, non-2xx and content-type, and dialer tests each fail when the protection they cover is removed, use no sleeps or real network, run clean under-race, touch only test code, andmake checkis green — PASS.Model: opus-4-8