Skip to content

fix: always set original-path internal headers in extproc#2398

Open
kanurag94 wants to merge 3 commits into
envoyproxy:mainfrom
kanurag94:fix/extproc-unconditional-original-path
Open

fix: always set original-path internal headers in extproc#2398
kanurag94 wants to merge 3 commits into
envoyproxy:mainfrom
kanurag94:fix/extproc-unconditional-original-path

Conversation

@kanurag94

@kanurag94 kanurag94 commented Jul 20, 2026

Copy link
Copy Markdown
Member

Description

The extproc router filter writes two "original path" internal headers from the incoming request path: x-ai-eg-original-path and x-envoy-original-path. The first is set unconditionally, but the second was only set when it was empty. Because of that, a client-supplied or otherwise pre-existing x-envoy-original-path survives and shadows the gateway's own value, and downstream logic that keys off the original path (for example the upstream-filter processor lookup on retry) then operates on the wrong path.

This change makes extproc the authoritative writer of the internal headers it owns. x-envoy-original-path is now set unconditionally, matching the sibling x-ai-eg-original-path. The header is already treated as gateway-owned elsewhere in extproc (the header mutator ignores it so users can't change it through the header mutation API), so writing it unconditionally simply aligns the two headers' behavior.

A unit test is added asserting that pre-existing values on both original-path headers are overwritten with the request path.

Related Issues/PRs (if applicable)

Special notes for reviewers (if applicable)

The MCP proxy sets both original-path headers with set-if-missing semantics when forwarding to its own upstream; that is a separate hop and is not affected by this change. Since the router filter already wrote x-ai-eg-original-path unconditionally, this only brings x-envoy-original-path in line with existing behavior rather than introducing new behavior.

The x-envoy-original-path header was only set when unset, so a
client-supplied or pre-existing value could survive and shadow the
gateway's own value, causing downstream logic (e.g. processor lookup on
retry) to key off the wrong path. Set it unconditionally, matching the
already-unconditional x-ai-eg-original-path, so extproc is the
authoritative writer of the internal headers it owns.

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.85%. Comparing base (ea1c886) to head (b46bb7a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2398      +/-   ##
==========================================
- Coverage   84.85%   84.85%   -0.01%     
==========================================
  Files         151      151              
  Lines       22086    22085       -1     
==========================================
- Hits        18742    18740       -2     
- Misses       2213     2214       +1     
  Partials     1131     1131              

☔ 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.

@kanurag94
kanurag94 marked this pull request as ready for review July 21, 2026 05:18
@kanurag94
kanurag94 requested a review from a team as a code owner July 21, 2026 05:18
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 21, 2026
@missBerg missBerg added bug Something isn't working area/extproc ExtProc data-plane architecture, body limits, performance labels Jul 21, 2026
@kanurag94

Copy link
Copy Markdown
Member Author

@gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request ensures that original-path headers are set unconditionally in extproc to prevent client-supplied values from shadowing the gateway's own values, and adds a corresponding unit test. The review feedback correctly points out that because HeaderValueOption defaults to appending values, the implementation should explicitly set AppendAction to OVERWRITE_IF_EXISTS_OR_ADD to guarantee that pre-existing headers are completely overwritten, and the unit tests should be updated to assert this behavior.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread internal/extproc/processor_impl.go
Comment thread internal/extproc/processor_impl_test.go
… extproc

Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/extproc ExtProc data-plane architecture, body limits, performance bug Something isn't working size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants