Skip to content

extproc: always redact credential header mutations in debug log - #2435

Merged
nacx merged 3 commits into
envoyproxy:mainfrom
Aias00:fix/extproc-redact-credential-mutation
Jul 31, 2026
Merged

extproc: always redact credential header mutations in debug log#2435
nacx merged 3 commits into
envoyproxy:mainfrom
Aias00:fix/extproc-redact-credential-mutation

Conversation

@Aias00

@Aias00 Aias00 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

The "request headers processed" debug log only ran header-mutation redaction when Server.enableRedaction was on. With debug logging on but redaction off (the aigw run default), the upstream filter's injected Authorization (the backend API key) was logged in cleartext, even though the incoming authorization header was already redacted by filterSensitiveHeadersForLogging.

This was inconsistent with the RequestBody phase, whose redactRequestBodyResponse always redacts header mutations and only gates body-content redaction on the flag. This change aligns the RequestHeaders phase to the same pattern: redact header mutations whenever debug logging is on, independent of enableRedaction, and gate only body content on the flag via a new redactBody parameter on redactProcessingResponseRequestHeaders.

A nil-response guard is added matching redactRequestBodyResponse.

Special notes for reviewers (if applicable)

  • No behavior change to the response sent to Envoy; redaction only touches the logContent copy used for the debug log. Process-level tests assert on the returned resp, not log content, so they are unaffected.
  • Verified live: with AIGW_DEBUG=true and enableRedaction off, the upstream Authorization mutation now logs as [REDACTED] while the actual response sent upstream still carries the real key.
  • Out of scope: Envoy's own [http] and [ext_proc] debug logs still emit the raw Authorization header / proto and are outside extproc's control; those require Envoy log-level tuning, not addressed here.

Related Issues/PRs (if applicable)

Adjacent to but distinct from #2428 (which covers request body content redaction for the 7 passthrough endpoints). This PR covers a different leak surface: the backend credential injected into the request-headers header mutation by the extproc upstream filter.

Co-Authored-By: Claude noreply@anthropic.com

Partially addresses #2436 (extproc Go side; Envoy-side leak points remain).

The "request headers processed" debug log only ran header-mutation
redaction when Server.enableRedaction was on. With debug logging on
but redaction off (the aigw run default), the upstream filter's
injected Authorization (the backend API key) was logged in cleartext
even though the incoming authorization header was already redacted by
filterSensitiveHeadersForLogging.

This was inconsistent with the RequestBody phase, whose
redactRequestBodyResponse always redacts header mutations and only
gates body-content redaction on the flag. Align the RequestHeaders
phase to the same pattern: redact header mutations whenever debug
logging is on, independent of enableRedaction; gate only body content
on the flag via a new redactBody parameter on
redactProcessingResponseRequestHeaders.

A nil-response guard is added matching redactRequestBodyResponse.

Note: Envoy's own [http] and [ext_proc] debug logs still emit the
raw Authorization header/proto and are outside extproc's control;
those require Envoy log-level tuning, not addressed here.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>
@Aias00
Aias00 requested a review from a team as a code owner July 28, 2026 03:19
Copilot AI review requested due to automatic review settings July 28, 2026 03:19
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jul 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.92%. Comparing base (f9a03db) to head (1a8c172).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2435      +/-   ##
==========================================
+ Coverage   84.86%   84.92%   +0.06%     
==========================================
  Files         154      154              
  Lines       22419    22423       +4     
==========================================
+ Hits        19026    19043      +17     
+ Misses       2237     2224      -13     
  Partials     1156     1156              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nacx
nacx enabled auto-merge (squash) July 30, 2026 10:59
@nacx
nacx merged commit e989693 into envoyproxy:main Jul 31, 2026
34 checks passed
@dosubot

dosubot Bot commented Jul 31, 2026

Copy link
Copy Markdown

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-08-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.


Leave Feedback Ask Dosu about ai-gateway Add Dosu to your team

sivanantha321 pushed a commit to sivanantha321/ai-gateway that referenced this pull request Jul 31, 2026
…yproxy#2435)

**Description**

The "request headers processed" debug log only ran header-mutation
redaction when Server.enableRedaction was on. With debug logging on but
redaction off (the aigw run default), the upstream filter's injected
Authorization (the backend API key) was logged in cleartext, even though
the incoming authorization header was already redacted by
filterSensitiveHeadersForLogging.

This was inconsistent with the RequestBody phase, whose
redactRequestBodyResponse always redacts header mutations and only gates
body-content redaction on the flag. This change aligns the
RequestHeaders phase to the same pattern: redact header mutations
whenever debug logging is on, independent of enableRedaction, and gate
only body content on the flag via a new redactBody parameter on
redactProcessingResponseRequestHeaders.

A nil-response guard is added matching redactRequestBodyResponse.

**Special notes for reviewers (if applicable)**

- No behavior change to the response sent to Envoy; redaction only
touches the logContent copy used for the debug log. Process-level tests
assert on the returned resp, not log content, so they are unaffected.
- Verified live: with AIGW_DEBUG=true and enableRedaction off, the
upstream Authorization mutation now logs as [REDACTED] while the actual
response sent upstream still carries the real key.
- Out of scope: Envoy's own [http] and [ext_proc] debug logs still emit
the raw Authorization header / proto and are outside extproc's control;
those require Envoy log-level tuning, not addressed here.

**Related Issues/PRs (if applicable)**

Adjacent to but distinct from envoyproxy#2428 (which covers request body content
redaction for the 7 passthrough endpoints). This PR covers a different
leak surface: the backend credential injected into the request-headers
header mutation by the extproc upstream filter.

Co-Authored-By: Claude <noreply@anthropic.com>


Partially addresses envoyproxy#2436 (extproc Go side; Envoy-side leak points
remain).

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Ignasi Barrera <ignasi@tetrate.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants