Skip to content

fix: use last header value for pass-through headers to prevent spoofing - #92

Merged
yizhaodev merged 2 commits into
llm-d:mainfrom
pierDipi:fix/passthrough-header-last-value
Mar 6, 2026
Merged

fix: use last header value for pass-through headers to prevent spoofing#92
yizhaodev merged 2 commits into
llm-d:mainfrom
pierDipi:fix/passthrough-header-last-value

Conversation

@pierDipi

@pierDipi pierDipi commented Mar 5, 2026

Copy link
Copy Markdown
Member

Switch from Header.Get (first value) to Header.Values with last-entry selection so that Envoy ext_authz-injected headers take precedence over client-supplied ones. Add tests covering single value, multi-value last-wins, absent header, and multiple configured headers.

Similar to #87

Copilot AI review requested due to automatic review settings March 5, 2026 08:53
@pierDipi

pierDipi commented Mar 5, 2026

Copy link
Copy Markdown
Member Author

/cc @yizhaodev @lioraron

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.

Pull request overview

This PR hardens pass-through header handling in the batch API by ensuring that when a header has multiple values (e.g., a spoofed client value plus an Envoy ext_authz-injected value), the handler stores the last value so the trusted auth-injected value takes precedence.

Changes:

  • Update batch creation to read configured pass-through headers via r.Header.Values() and select the last value.
  • Add unit tests covering single-value, multi-value last-wins, missing header, and multiple configured headers.
  • Refactor test setup helper to allow injecting a custom ServerConfig per test.

Reviewed changes

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

File Description
internal/apiserver/batch/batch_handler.go Switch pass-through header extraction from first-value semantics to last-value semantics to prevent spoofing.
internal/apiserver/batch/batch_handler_test.go Add targeted tests validating correct tag persistence for pass-through headers, including multi-value last-wins behavior.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread internal/apiserver/batch/batch_handler.go Outdated
Switch from Header.Get (first value) to Header.Values with last-entry
selection so that Envoy ext_authz-injected headers take precedence over
client-supplied ones. Add tests covering single value, multi-value
last-wins, absent header, and multiple configured headers.

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@pierDipi
pierDipi force-pushed the fix/passthrough-header-last-value branch from 5e992b7 to f23b590 Compare March 5, 2026 08:59
@pierDipi
pierDipi requested a review from Copilot March 5, 2026 08:59

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.

Pull request overview

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


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

You can also share your feedback on Copilot code review. Take the survey.

@yizhaodev yizhaodev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, Thanks!

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>
@pierDipi

pierDipi commented Mar 6, 2026

Copy link
Copy Markdown
Member Author

@yizhaodev can we merge?

@yizhaodev
yizhaodev merged commit b5c2f9b into llm-d:main Mar 6, 2026
2 checks passed
@yizhaodev

Copy link
Copy Markdown
Collaborator

@pierDipi thanks, merged

@vishbhat vishbhat added the bug Fixes incorrect behavior label Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants