Extract shared HTTP response body read helpers to httputil package

[lpcox]

Summary

Fixes #8320

Deduplicates the HTTP response body read + status check pattern that was independently implemented in three files.

Changes

New helpers (internal/httputil/response.go)

• ReadResponseBody(resp, context) — Reads the full body, closes it, and returns an error if status ≥ 400. Used by proxy and collaborator call sites.
• ReadResponseBodyStrict(resp, expectedStatus, context) — Reads the full body, closes it, and returns an error (including the body text) if the status doesn't match exactly. Used by the OIDC call site which checks != 200 and includes the body in error messages.

Refactored call sites

File	Before	After
internal/proxy/proxy.go	defer Close + ReadAll + >= 400 check	httputil.ReadResponseBody
internal/githubhttp/collaborator.go	defer Close + ReadAll + >= 400 check	httputil.ReadResponseBody
internal/oidc/provider.go	defer Close + ReadAll + != 200 check (body in error)	httputil.ReadResponseBodyStrict

Test updates

• Added comprehensive tests for both new helpers in internal/httputil/response_test.go
• Updated existing test assertions in collaborator_test.go and provider_test.go to match the slightly different error message format from the shared helpers

[Copilot]

Pull request overview

This PR addresses issue #8320 by deduplicating repeated “read HTTP response body + close + status check” logic across the proxy, GitHub collaborator permission client, and OIDC provider, consolidating it into shared helpers under internal/httputil.

Changes:

• Added shared response body helpers: ReadResponseBody (status >= 400) and ReadResponseBodyStrict (exact expected status, includes body in error).
• Refactored proxy, collaborator permission fetching, and OIDC token fetching to use the shared helpers.
• Added/updated unit tests to cover the new helpers and align existing tests with the new error formats.

Show a summary per file

File	Description
internal/httputil/response.go	Introduces shared response-body read + status-check helpers.
internal/httputil/response_test.go	Adds unit coverage for both new helper variants and read-error cases.
internal/proxy/proxy.go	Switches REST enrichment call path to use the shared helper.
internal/githubhttp/collaborator.go	Switches collaborator permission fetch to use the shared helper and updates logging.
internal/githubhttp/collaborator_test.go	Updates assertions for new shared-helper error message formats.
internal/oidc/provider.go	Switches OIDC response reading/status validation to strict shared helper.
internal/oidc/provider_test.go	Updates assertions to match shared-helper error message wording.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 7/7 changed files
• Comments generated: 4
• Review effort level: Low

[lpcox]

@copilot address review feedback

[Copilot]

@copilot address review feedback

The PR was merged while I was processing the feedback. Here's what happened with each review point:

1. Nil guards on ReadResponseBody / ReadResponseBodyStrict — The PR author addressed these directly in commits e976b65 and 05b2f2c before merging. Both helpers now guard against nil resp and nil resp.Body.

2. Import grouping in provider.go — Still broken in the merged code (stdlib imports net/url, strings, sync, time appear after the internal httputil import). Needs a follow-up fix.

3. Diagnostic logging before returning OIDC error — Not included in the merge. The logOIDC.Printf for failed responses is still skipped on non-200 status.

4. Tests for nil-guard cases — Not included in the merge.

Items 2–4 are ready as local commits and can be pushed to a follow-up PR if desired.