feat: add /fetch endpoint to proxy a downstream request by mmatur · Pull Request #3 · traefik/test-whoami

mmatur · 2026-09-07T12:52:36Z

Adds a GET /fetch?url= handler that fetches the given URL and streams
the response back to the caller — useful for checking what a downstream
service sees when whoami calls out to it.

New fetchHandler in fetch.go, registered on /fetch
Basic test covering the happy path
README updated with the new endpoint

Adds a GET /fetch?url= handler that fetches the given URL and streams the response back, for checking what a downstream service sees when whoami calls out to it.

github-actions

Blocking

fetch.go:18 — SSRF: the /fetch?url= endpoint fetches an arbitrary, user-supplied URL server-side with no validation (scheme, private/loopback/link-local ranges, cloud metadata IPs, or redirect targets). This lets any caller use the service as an open internal-network proxy. Needs a human decision on whether/how this endpoint should be scoped (allowlist? disable in prod? restrict to non-private destinations?) before merge.

Should-fix

fetch.go:22 — resp.Body is never closed (connection/fd leak).
fetch.go:26 — no timeout on the outbound request and no size limit on the copied response body; combined with the SSRF issue this enables trivial resource-exhaustion/hang. The io.Copy error is also silently discarded.

Nits

None beyond what's folded into the should-fix items above.

No missing-test concerns beyond the security gap itself — the added test only covers the happy path, not any of the unsafe-target cases that should exist once validation is added.

Generated by Pull Request Reviewer for #3 · claude · agent · 39 AIC · ⌖ 7.82 AIC · ⊞ 5.1K

github-actions · 2026-09-07T12:59:06Z

+		return
+	}
+
+	resp, err := http.Get(target)


Blocking — SSRF. target comes directly from the url query parameter and is passed to http.Get with no validation. Any caller can make the server issue requests to internal/private addresses, loopback, or cloud metadata endpoints (e.g. 169.254.169.254), and can follow redirects to the same. Since this is deployed as a network service, this endpoint effectively turns whoami into an open internal-network proxy.

Even for a debug/test utility, this needs at minimum: restrict to http/https schemes, resolve and block private/loopback/link-local/metadata IP ranges (including after redirects), and consider disabling redirect-following or capping redirect count.

github-actions · 2026-09-07T12:59:06Z

+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadGateway)
+		return
+	}


Should-fix — resource leak. resp.Body is never closed. Add defer resp.Body.Close() right after the error check, otherwise every call to this handler leaks a connection/file descriptor.

github-actions · 2026-09-07T12:59:06Z

+
+	w.Header().Set("Content-Type", resp.Header.Get("Content-Type"))
+	w.WriteHeader(resp.StatusCode)
+	io.Copy(w, resp.Body)


Should-fix. http.Get uses no timeout, and this io.Copy has no size or deadline limit — a slow or huge upstream response (attacker-controlled via the SSRF above) can hang the handler or exhaust memory/bandwidth. Consider an http.Client with a timeout and wrapping resp.Body in an io.LimitReader. Also, the io.Copy error is silently discarded — at least log it (nit-level on its own, but worth fixing alongside the above).

github-actions

Blocking

fetch.go:18 — SSRF: the url query param is passed straight to http.Get with no allowlist/denylist for private/internal/metadata addresses. Already flagged in a prior review comment on this line; still unaddressed.

Should fix

fetch.go:22 — resp.Body is never closed (resource/connection leak). Already flagged in a prior review comment.
fetch.go:26 — no timeout on http.Get and unbounded io.Copy; a slow/huge upstream response can hang the handler or exhaust memory. Already flagged in a prior review comment.
fetch.go:18/26 — http.Get follows redirects by default (up to 10). Even if an allowlist is added for the blocking SSRF issue above, an externally-allowed URL can 302 to an internal address and bypass it — worth handling together with the SSRF fix (e.g. custom CheckRedirect or disabling redirects).
fetch_test.go — only the happy path is covered. No test for the missing-url 400 branch or the upstream-error 502 branch.

Needs a human decision

This endpoint is effectively "fetch any URL on behalf of the server," which is SSRF-as-a-feature. The PR body frames it as a debugging convenience ("useful for checking what a downstream service sees"), but as implemented it has no restriction to intentionally-testable targets. A maintainer should decide whether this needs an allowlist/opt-in flag before merging, or whether the risk is accepted given this project's purpose (whoami is itself a debug/test tool typically run in trusted environments) — I can't verify that intent from the diff alone.

Coverage: Reviewed 4 files, skimmed 0, skipped 0 of 4 (skipped: none). Prior review comments read: 3.

Generated by Pull Request Reviewer for #3 · claude · agent · 42.8 AIC · ⌖ 10.3 AIC · ⊞ 6.9K

….sha Live-tested the previous version on #3: the agent correctly called get_file_contents for .claude/skills/review/SKILL.md, but at github.event.pull_request.base.sha — a payload snapshot from whenever the PR was last synced, not the actual current tip of main. Since main moved (this very SKILL.md was added) after the PR was opened, the call 404'd and the skill was silently skipped (the agent's own fallback for a missing file, working as designed, just against a stale ref). github.event.pull_request.base.ref isn't in gh-aw's allowed-expressions list for the prompt body (only base.sha is), so switch to a small pre-agent-steps shell fetch instead: a plain `gh api .../contents/...` call with no `ref` defaults to the repo's default branch, which can't go stale the same way. Confirmed pre-agent-steps from our own frontmatter and from the imported shared/pr-diff-data-fetch.md both land in the compiled job (concatenated, not overridden). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…locks Live-tested run on #3 posted zero inline comments: the agent read the dedup-against-prior-comments instruction too broadly and folded every finding (including one genuinely new one) into the review body as prose instead of posting create_pull_request_review_comment for any of them. Per feedback: detailed, line-anchored comments are the point of this review, not one consolidated summary. - Narrow the dedup rule: prior comments only suppress an exact repeat at the same path/line, never a reason to skip inline comments altogether. - Require a GitHub suggestion block (```suggestion fenced code) on inline comments whenever the fix is a small, mechanical, safe patch (defer Close(), add a timeout, a nil check) — skip it only when the real fix needs a design decision. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

github-actions

Blocking

fetch.go:18 — SSRF: target is taken verbatim from the url query parameter and passed straight to http.Get with no scheme/host allowlist or private-range blocking. Any caller can make the server reach internal services or cloud metadata endpoints (e.g. 169.254.169.254). Already flagged inline by a prior review comment on this PR; restating because it blocks merge. The PR body's framing ("useful for checking what a downstream service sees when whoami calls out to it") doesn't address who can call this endpoint or from where — author's claim, not verified.

Should fix

fetch.go:22 — resp.Body is never closed, leaking a connection per request. Already flagged inline by a prior comment; add defer resp.Body.Close() right after the error check.
fetch.go:26 — http.Get has no timeout and io.Copy has no size/deadline bound, so a slow or huge upstream response (attacker-controlled given the SSRF issue above) can hang the handler or exhaust memory. Already flagged inline by a prior comment.
fetch_test.go:9 — only the happy path is tested; no coverage for the missing-url 400 case, the upstream-error 502 case, or (once fixed) SSRF rejection. New inline comment added.

Needs a human decision

The concrete SSRF mitigation (allowlist of schemes/hosts, blocking RFC1918/link-local ranges, or requiring the endpoint to be admin-only/behind auth) is a policy call, not a mechanical patch — needs a maintainer decision on the intended trust model for this endpoint before a fix is merged.

Coverage: Reviewed 4 files, skimmed 0, skipped 0 of 4 (skipped: none). Prior review comments read: 3.

Generated by Pull Request Reviewer for #3 · claude · agent · 43.8 AIC · ⌖ 8.57 AIC · ⊞ 7.3K

github-actions · 2026-09-08T13:39:17Z

+	"testing"
+)
+
+func TestFetchHandler(t *testing.T) {


Should-fix — missing tests. Only the happy path is covered. There's no test for the missing-url case (expect 400) or for an unreachable/erroring upstream (expect 502), and no test asserting the handler rejects or restricts non-http(s) targets / internal addresses once the SSRF fix (flagged elsewhere on fetch.go) lands — without that, a future refactor could silently reopen the hole.

github-actions

Blocking

fetch.go:18 — target from the url query parameter is passed unvalidated to http.Get, letting any caller make the server issue requests to internal/private addresses (classic SSRF). Already flagged by an existing inline comment at this exact location; not duplicating it.

Should fix

fetch.go:22 — resp.Body is never closed (missing defer resp.Body.Close()), leaking a connection per request. Already flagged inline.
fetch.go:26 — http.Get has no timeout and io.Copy has no size/deadline limit, so a slow or huge (attacker-controlled, given the SSRF above) upstream response can hang the handler or exhaust memory. Already flagged inline.
fetch_test.go:9 — only the happy path is tested; the missing-url (400) and upstream-error (502) branches in fetch.go have no coverage. Already flagged inline.

All three code-level findings above and the test-coverage gap already have inline review comments at the same file/line from a prior pass, so no new inline comments were added — reposting identical findings at the same anchors would just be noise.

Needs a human decision

Design-level: this handler makes test-whoami act as an open HTTP fetch/proxy primitive (arbitrary URL in, response streamed out). Fixing the leak/timeout/validation issues makes it safer but doesn't change that the feature itself is an SSRF-by-design capability. Maintainers should decide whether this belongs in the tool at all, or whether it needs an allowlist / opt-in flag rather than being on by default — that's a product decision, not a mechanical fix.
The README text ("Useful for checking what a downstream service sees when whoami calls out to it") frames this as an intentional debugging feature — noted as the author's stated intent, not verified as sufficient justification for the missing hardening above.

Coverage
Reviewed 2 files, skimmed 2, skipped 0 of 4 (skipped: none). Prior review comments read: 4.

Generated by 🔍 AI code review for #3 · claude · agent · 40.9 AIC · ⌖ 10.2 AIC · ⊞ 7.4K

_{Add this agentic workflow to your repo}

To install this agentic workflow, run

gh aw add traefik/ai/workflows/test-whoami.ai-review.md@main

feat: add /fetch endpoint to proxy a downstream request

05868ff

Adds a GET /fetch?url= handler that fetches the given URL and streams the response back, for checking what a downstream service sees when whoami calls out to it.

mmatur added the ai/review label Sep 7, 2026

github-actions Bot reviewed Sep 7, 2026

View reviewed changes

mmatur added ai/review and removed ai/review labels Sep 8, 2026

github-actions Bot reviewed Sep 8, 2026

View reviewed changes

mmatur added ai/review and removed ai/review labels Sep 8, 2026

github-actions Bot reviewed Sep 8, 2026

View reviewed changes

mmatur added ai/review and removed ai/review labels Sep 11, 2026

github-actions Bot reviewed Sep 11, 2026

View reviewed changes

youkoulayley mentioned this pull request Sep 15, 2026

add a new path param #13

Closed

Navigation Menu

feat: add /fetch endpoint to proxy a downstream request - #3

feat: add /fetch endpoint to proxy a downstream request#3
mmatur wants to merge 1 commit into
traefik:mainfrom
mmatur:feature/fetch-endpoint

mmatur commented Sep 7, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Sep 7, 2026

Uh oh!

github-actions Bot Sep 7, 2026

Uh oh!

github-actions Bot Sep 7, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Sep 8, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mmatur commented Sep 7, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant