fix: redact sensitive response headers in debug logs

[jbeckwith-oai]

Summary

• Apply the existing sensitive-header redaction policy to cloned HTTP responses before debug logging.
• Redact redirect Location values while preserving original response headers, response identity, unread bodies, and existing request logging behavior.
• Add unit and real-HTTP regression coverage for repeated cookies, authorization values, built-in/custom API-key headers, redirects, and response immutability.

Validation

• go test ./internal/debugmiddleware -count=1 -cover (95.1% statement coverage)
• go test -race ./internal/debugmiddleware -count=5
• go vet ./...
• ./scripts/lint
• Full repository integration suite and Windows test cross-compilation passed using a local test-harness overlay that reuses the freshly built CLI.
• Full repository race suite passed with the same overlay.
• One unrelated existing macOS FIFO cancellation test was excluded after reproducing the same failure against the untouched base commit.

Only synthetic credentials and sanitized fixtures are used.

[Copilot]

Pull request overview

This PR updates the internal/debugmiddleware request/response debug logger so that response headers are redacted before being dumped to logs, aligning response logging with the existing request redaction policy while preserving the original *http.Response returned to callers.

Changes:

• Apply sensitive-header redaction to a cloned http.Response before calling httputil.DumpResponse.
• Refactor header redaction into a shared redactHeaders helper and reuse it for request redaction.
• Add unit + integration-style HTTP tests to validate response header redaction and immutability (including repeated headers, redirects, and ensuring bodies are not consumed).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/debugmiddleware/debug_middleware.go	Redacts sensitive response headers by dumping a redacted response clone; refactors request redaction to reuse shared header redaction logic.
internal/debugmiddleware/debug_middleware_test.go	Adds coverage for response-header redaction (including redirects), verifies the original response object/headers are not mutated, and confirms bodies are not consumed.

Suppressed comments (1)

internal/debugmiddleware/debug_middleware.go:69

• Typo in the redactRequest comment: "original is request" should be "the original request".

// original is request is returned unchanged if no redaction is necessary.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/debugmiddleware/debug_middleware.go:69

• The redactRequest doc comment currently reads awkwardly because of a duplicated word across the line wrap ("... As a small optimization, the" / "the original request..."). Since this comment was touched, please fix the grammar for clarity.

// redactRequest redacts sensitive information from the request for logging
// purposes. If redaction is necessary, the request is cloned before mutating
// the original and that clone is returned. As a small optimization, the
// the original request is returned unchanged if no redaction is necessary.

[jbeckwith-oai]

Addressed Copilot submitted review 4975555643 in 2f0684d: removed the duplicated the across the wrapped redactRequest comment so it reads “As a small optimization, the original request is returned unchanged.” Verified with go test ./internal/debugmiddleware -count=1 -cover (95.1%), go test -race ./internal/debugmiddleware -count=5, go vet ./..., and ./scripts/lint. Documentation only; no behavior changed.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/debugmiddleware/debug_middleware.go:60

• redactResponse dereferences resp unconditionally; if a MiddlewareNext ever returns (nil, nil), this will panic during debug logging. Adding a resp != nil guard keeps the middleware robust (and still preserves current behavior for normal http.Client.Do usage).

		if respBytes, err := httputil.DumpResponse(m.redactResponse(resp), false); err == nil {
			m.logger.Printf("Response Content:\n%s\n", respBytes)
		}

[jbeckwith-oai]

Addressed Copilot submitted review 4975655882 in da569ef. Added a fail-first regression proving (nil, nil) previously panicked, then extended the existing middleware early return to if err != nil || resp == nil; nil responses now pass through unchanged without logging a response, while ordinary response redaction/logging remains covered. Verified: focused middleware tests (97.6% coverage), 10 race repetitions, full integration suite plus Windows cross-compilation, repository race coverage, go vet ./..., and ./scripts/lint. Broad race runs excluded only independently reproduced pre-existing FIFO and requestflag-test issues.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[HAYDEN-OAI]

Two independently reproduced response-logging credential disclosures remain: case-variant sensitive headers and populated chunked-response trailers.

[github-actions]

Castiron custom code

✅ No new custom-code files detected.

6 mixed files remain; 0 existing customizations changed.

Compared b3e0946d9ae8 → 6fe9708764ef. Generated baselines verified.

6 existing customizations unchanged

• pkg/cmd/adminorganizationcertificate.go
• pkg/cmd/adminorganizationcertificate_test.go
• pkg/cmd/cmd.go
• scripts/castiron/README.md
• scripts/castiron/custom_code_report.py
• scripts/castiron/test_custom_code_report.py

A changed generated baseline means this report cannot reliably identify which handwritten lines changed.

Inspect the custom-code diff

Download the exact patch produced by this run (requires repository access):

gh run download 32310210044 --repo openai/openai-cli \
  --name castiron-custom-code-32310210044-1 --dir /tmp/castiron-custom-code-32310210044-1
git apply --stat /tmp/castiron-custom-code-32310210044-1/custom-code.patch
cat /tmp/castiron-custom-code-32310210044-1/custom-code.patch

Or reproduce it from an SDK checkout containing the vendored reporter:

git fetch --no-tags origin b3e0946d9ae83ea5e31b24d728e5f7b80dd1b472 6fe9708764efe1e82bd5ddc85ed42b3c1edfabb9
python3 scripts/castiron/custom_code_report.py report \
  --base b3e0946d9ae83ea5e31b24d728e5f7b80dd1b472 \
  --head 6fe9708764efe1e82bd5ddc85ed42b3c1edfabb9 --fetch --require-head-hash --public \
  --out /tmp/castiron-custom-code-6fe9708764ef
cat /tmp/castiron-custom-code-6fe9708764ef/custom-code.patch

This is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR.

Full report and patch

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[HAYDEN-OAI]

Reviewed exact head 6fe9708. Confirmed the prior mixed-case response-header and populated-trailer credential disclosures are fixed: every case-variant authorization, API-key, cookie, and Location entry is independently redacted, while original response headers/trailers and unread streaming bodies remain unchanged. Verified custom transports, redirects, and nil/error paths; no substantive correctness or security findings.