Skip to content

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

Merged
yizhaodev merged 2 commits intollm-d-incubation: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 intollm-d-incubation:mainfrom
pierDipi:fix/passthrough-header-last-value

Conversation

@pierDipi
Copy link
Copy Markdown
Contributor

@pierDipi pierDipi commented Mar 5, 2026

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
Copy link
Copy Markdown
Contributor Author

pierDipi commented Mar 5, 2026

/cc @yizhaodev @lioraron

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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.

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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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.

Copy link
Copy Markdown
Contributor

@yizhaodev yizhaodev left a comment

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
Copy link
Copy Markdown
Contributor Author

pierDipi commented Mar 6, 2026

@yizhaodev can we merge?

@yizhaodev yizhaodev merged commit b5c2f9b into llm-d-incubation:main Mar 6, 2026
2 checks passed
@yizhaodev
Copy link
Copy Markdown
Contributor

@pierDipi thanks, merged

@vishbhat vishbhat added the bug Something isn't working label Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants