docs(#376): add secure HTTP client guidance to AGENTS.md

[fullsend-ai-coder]

Add a "Secure HTTP clients" subsection under "Go code" documenting required security properties for outbound HTTP clients. Points agents to the SSRF-hardened fetch package (internal/fetch/) and lists the mandatory protections when a custom client is needed: HTTPS-only, DNS pre-resolution with IP validation via netutil, IP-pinned DialContext, redirect blocking, explicit timeout, and response size limiting. Prohibits http.Get and http.DefaultClient for external URLs from configuration or user input.

---

Closes #376

Post-script verification

• Branch is not main/master (agent/376-secure-http-guidance)
• Secret scan passed (gitleaks — aa6585ddb3dff17dccfd4dba8b6c5c306275156a..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:01 PM UTC · Completed 12:08 PM UTC
Commit: aa6585d · View workflow run →

[fullsend-ai-review]

Review — Approve

PR: docs(#376): add secure HTTP client guidance to AGENTS.md
Scope: 1 file, 19 additions — documentation only

Summary

This PR adds a well-structured "Secure HTTP clients" subsection under "Go code" in AGENTS.md, documenting SSRF hardening requirements for outbound HTTP clients. The change directly traces to issue #376 (retro from PR fullsend-ai#3002's iterative security fixes) and stays within its authorized scope.

Verification

All technical claims in the new documentation were verified against the current codebase:

Claim	Verified	Source
FetchURL exists	✅	internal/fetch/fetch.go:93
CheckIP / IsInternal exist	✅	internal/netutil/ip.go:39,70
HTTPS-only enforcement	✅	fetch.go:104-106
DNS pre-resolution + IP validation	✅	fetch.go:120-135 via netutil.IsInternal
IP-pinned DialContext	✅	fetch.go:156-169
Redirect blocking	✅	fetch.go:178-180 (http.ErrUseLastResponse)
30s default timeout	✅	fetch.go:76 (DefaultPolicy.Timeout)
10 MB response body limit	✅	fetch.go:74 (DefaultPolicy.MaxSizeBytes)
Domain allowlisting	✅	fetch.go:114-117
Proxy disabled (via zero-value)	✅	fetch.go:156 — &http.Transport{} zero-value Proxy is nil

Dimensions reviewed

• Correctness: All referenced symbols, behaviors, and default values are technically accurate. No contradictions.
• Security: Clean diff — no secrets, injection patterns, Unicode steganography, or permission changes. The guidance itself is security-positive.
• Intent & coherence: Traces to [ARCHIVED] Benchmark issue 376 #376, scope matches, ### heading level correct under ## Go code, placement appropriate.
• Style/conventions: Follows established AGENTS.md patterns (bold-label paragraphs, bullet lists, canonical implementation references, prohibited patterns).
• Docs currency: All code references accurate. One pre-existing staleness noted below.
• Cross-repo contracts: Skipped — no exported interfaces modified.

Actionable follow-up

Pre-existing doc staleness (low): docs/plans/universal-harness-access-phase1.md (lines 51, 57) still references internal/security/ssrf.go:checkIP() as a future extraction target, but the extraction to internal/netutil/ has already been completed. Not caused by this PR, but worth updating since this PR establishes netutil as the canonical reference.

---

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

• AGENTS.md

Previous run

Review — Approve

Clean documentation PR that adds SSRF-hardened HTTP client guidance to AGENTS.md. All six referenced identifiers (FetchURL, CheckIP, IsInternal, internal/fetch/, internal/netutil/, fetch.go) verified against source — every technical claim is accurate. No secrets, injection patterns, or permission changes.

Verified

• internal/fetch/fetch.go exports FetchURL and implements all documented properties (HTTPS-only, DNS pre-resolution, IP-pinned DialContext, redirect blocking, 30s timeout, 10MB limit, domain allowlisting)
• internal/netutil/ip.go exports CheckIP and IsInternal, rejecting loopback, private, link-local, and reserved addresses as described
• Placement under ### Secure HTTP clients within ## Go code fits the document hierarchy
• Scope matches issue [ARCHIVED] Benchmark issue 376 #376's authorization; canonical references correctly point to the refactored internal/fetch/ package rather than the now-absent internal/repos/manifest.go

Low-severity notes

1. Transport.Proxy omission (AGENTS.md) — Issue [ARCHIVED] Benchmark issue 376 #376 listed Transport.Proxy = nil as a required property, but neither the canonical implementation nor this guidance includes it. The IP-pinned DialContext neutralizes proxy routing in practice, so the omission is internally consistent but worth noting for completeness.

2. Formatting style (AGENTS.md:51,53) — Bold standalone imperative sentences (e.g., **Use the \fetch` package.**) differ from the label-colon pattern used elsewhere in the document (e.g., Prohibited outside `internal/forge/github/`:`). Minor consistency nit — existing patterns use bold-label-colon to introduce content blocks.

None of these block merge.

---

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

• AGENTS.md

Previous run (2)

Review — approve

Summary: This PR adds well-written, technically accurate security guidance for outbound HTTP clients to AGENTS.md. Every claim in the documentation was verified against the actual source code in internal/fetch/fetch.go and internal/netutil/ip.go — function names, package paths, the seven listed security properties, the 30 s timeout default, and the 10 MB body limit all match. The section is correctly placed under "Go code," uses consistent heading levels (###) and formatting conventions, and correctly points developers to the hardened FetchURL implementation as the primary path.

The PR deviates from issue #376 in two ways that are improvements: (1) it references internal/fetch/fetch.go as the canonical implementation instead of internal/repos/manifest.go (which no longer exists in the repo — the hardened code has been properly extracted into a reusable package), and (2) it corrects the response body limit from "1 MB" (issue text) to "10 MB" to match the actual DefaultPolicy.MaxSizeBytes.

Three low-severity gaps in the custom-client checklist are noted below as follow-up improvements.

Findings

#	Severity	Category	Description
1	low	completeness	Proxy handling omission. Issue #376 listed Transport.Proxy = nil as a required property. The PR omits it from the custom-client checklist. The canonical FetchURL is safe — it creates a new &http.Transport{} where Proxy defaults to nil (no proxy) — but a developer who starts from http.DefaultTransport.Clone() would inherit http.ProxyFromEnvironment, potentially allowing proxy-based SSRF bypass. Consider adding a bullet: "Disable proxy — set Transport.Proxy to nil (or http.ProxyFunc(nil)) to prevent requests from being routed through HTTP_PROXY/HTTPS_PROXY environment variables."
2	low	completeness	Port restriction omission. The canonical FetchURL enforces a port allowlist (defaults to 443 only via AllowedPorts). The custom-client checklist does not mention port validation. A developer building a custom client could accept arbitrary ports, widening the attack surface.
3	low	completeness	Double-encoding rejection omission. FetchURL rejects URLs containing %25 (double-encoded percent signs) as an anti-bypass measure. The custom-client checklist does not mention this.

None of these gaps affect the accuracy of the existing guidance — they are additive improvements that would make the checklist more comprehensive. The PR is safe to merge as-is; these can be addressed in a follow-up.

---

🤖 Reviewed with fullsend

---

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

• AGENTS.md

---

Labels: PR adds security-focused developer guidance documentation to AGENTS.md

[guyoron1]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:18 PM UTC · Completed 1:25 PM UTC
Commit: aa6585d · View workflow run →

[guyoron1]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 1:51 PM UTC · Completed 2:00 PM UTC
Commit: aa6585d · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed both low-severity review findings: added Transport.Proxy bullet to custom-client checklist and fixed bold formatting to use consistent label-colon pattern.

Fixed (2):

1. Transport.Proxy omission in custom-client checklist (AGENTS.md): Added 'Disable proxy' bullet to the custom-client requirements list, advising Transport.Proxy = nil to prevent HTTP_PROXY/HTTPS_PROXY-based SSRF bypass when starting from http.DefaultTransport.Clone().
2. Formatting style inconsistency (bold-period vs label-colon) (AGENTS.md): Changed bold formatting on 'Use the fetch package' and 'If you must build a custom client' from standalone imperative (.) / comma (,) to the label-colon pattern (**:) used throughout the rest of the document.

Tests: passed

_{Updated by fullsend fix agent}

[guyoron1]

/fs-review

[guyoron1]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:39 AM UTC · Completed 6:47 AM UTC
Commit: e49ecd5 · View workflow run →