From 3add372bc33e62e1adb451612bcafbfee94f5efd Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 13:02:48 +0000 Subject: [PATCH 1/2] docs(#233): add secure HTTP client guidance to AGENTS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a "Secure HTTP clients" subsection under "Go code" documenting the five required security properties for any new http.Client that fetches external URLs: 1. SSRF-safe dialer with DNS pre-resolution and IP validation via netutil.CheckIP 2. HTTPS-only with redirect blocking or scheme validation 3. Transport.Proxy = nil to prevent proxy-based redirection 4. Explicit timeout (30s default) — never http.DefaultClient 5. io.LimitReader for response bodies (10 MB default) References internal/fetch/fetch.go (FetchURL/FetchPolicy), internal/netutil/ip.go (CheckIP), and internal/security/ssrf.go (SSRFValidator) as the canonical implementations. Closes #233 --- AGENTS.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5620b735f..328a07e7f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,6 +46,20 @@ The e2e tests require GitHub credentials. There are three ways to provide them: If only `E2E_GITHUB_USERNAME` and a password source are available, `make e2e-test` will automatically generate a session file before running tests. See `make help` for all available targets. +### Secure HTTP clients + +Any new `http.Client` that fetches external or user-supplied URLs **must** include all of the following protections: + +1. **SSRF-safe dialer.** Use a custom `DialContext` that pre-resolves DNS, validates every resolved IP with `netutil.CheckIP` (`internal/netutil/ip.go`), and dials the validated IP directly (pinning against DNS rebinding). Reject loopback, private (RFC 1918), link-local, and other reserved addresses. Fail closed — if DNS resolution fails or returns no addresses, do not connect. +2. **HTTPS only.** Reject `http://` URLs at parse time. Block redirects entirely (`CheckRedirect` returning `http.ErrUseLastResponse`) or validate that every redirect target also uses HTTPS. +3. **No proxy.** Set `Transport.Proxy = nil` to prevent the client from honoring `HTTP_PROXY`/`HTTPS_PROXY` environment variables, which an attacker could use to redirect traffic. +4. **Explicit timeout.** Set `http.Client.Timeout` (30 s default). Never use `http.DefaultClient` or `http.Get` — both have no timeout. +5. **Response size limit.** Wrap the response body with `io.LimitReader` (10 MB default) to prevent memory exhaustion from malicious or unexpectedly large responses. + +See `internal/fetch/fetch.go` for the canonical SSRF-hardened HTTP client (`FetchURL` / `FetchPolicy`) that implements all five properties. See `internal/netutil/ip.go` for the `CheckIP` implementation and `internal/security/ssrf.go` for the URL-level SSRF validator. + +**When reviewing PRs:** Flag any new `http.Get`, `http.DefaultClient`, or `http.Client{}` without a custom transport and timeout as a medium-severity finding when the client fetches external URLs. + ## Forge abstraction All git forge operations (GitHub API calls, PR comments, issue creation, workflow dispatch, etc.) **must** go through the `forge.Client` interface defined in `internal/forge/forge.go`. This is a fundamental architectural rule — the codebase supports multiple forges (GitHub, GitLab, Forgejo) and direct coupling to any single forge breaks the abstraction. From 26a222a1b62afa87788462483fcf60d3cf9bfaf0 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:50:02 +0000 Subject: [PATCH 2/2] fix(#233): clarify IP validation and SSRF validator roles in secure HTTP guidance - Property 1: mention both netutil.IsInternal (used by fetch.go) and netutil.CheckIP so developers can find the function in the reference impl - Closing paragraph: clarify that ssrf.go handles host/IP/scheme validation while HTTPS enforcement lives in fetch.go Addresses review feedback on #277 --- AGENTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 328a07e7f..3894483ec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,13 +50,13 @@ If only `E2E_GITHUB_USERNAME` and a password source are available, `make e2e-tes Any new `http.Client` that fetches external or user-supplied URLs **must** include all of the following protections: -1. **SSRF-safe dialer.** Use a custom `DialContext` that pre-resolves DNS, validates every resolved IP with `netutil.CheckIP` (`internal/netutil/ip.go`), and dials the validated IP directly (pinning against DNS rebinding). Reject loopback, private (RFC 1918), link-local, and other reserved addresses. Fail closed — if DNS resolution fails or returns no addresses, do not connect. +1. **SSRF-safe dialer.** Use a custom `DialContext` that pre-resolves DNS, validates every resolved IP with `netutil.IsInternal` or `netutil.CheckIP` (`internal/netutil/ip.go`), and dials the validated IP directly (pinning against DNS rebinding). Reject loopback, private (RFC 1918), link-local, and other reserved addresses. Fail closed — if DNS resolution fails or returns no addresses, do not connect. 2. **HTTPS only.** Reject `http://` URLs at parse time. Block redirects entirely (`CheckRedirect` returning `http.ErrUseLastResponse`) or validate that every redirect target also uses HTTPS. 3. **No proxy.** Set `Transport.Proxy = nil` to prevent the client from honoring `HTTP_PROXY`/`HTTPS_PROXY` environment variables, which an attacker could use to redirect traffic. 4. **Explicit timeout.** Set `http.Client.Timeout` (30 s default). Never use `http.DefaultClient` or `http.Get` — both have no timeout. 5. **Response size limit.** Wrap the response body with `io.LimitReader` (10 MB default) to prevent memory exhaustion from malicious or unexpectedly large responses. -See `internal/fetch/fetch.go` for the canonical SSRF-hardened HTTP client (`FetchURL` / `FetchPolicy`) that implements all five properties. See `internal/netutil/ip.go` for the `CheckIP` implementation and `internal/security/ssrf.go` for the URL-level SSRF validator. +See `internal/fetch/fetch.go` for the canonical SSRF-hardened HTTP client (`FetchURL` / `FetchPolicy`) that implements all five properties. See `internal/netutil/ip.go` for the `CheckIP` implementation and `internal/security/ssrf.go` for host/IP/scheme validation (note: HTTPS enforcement is in `fetch.go`, not `ssrf.go`). **When reviewing PRs:** Flag any new `http.Get`, `http.DefaultClient`, or `http.Client{}` without a custom transport and timeout as a medium-severity finding when the client fetches external URLs.