fix: harden header sanitization and handling logic#2025
fix: harden header sanitization and handling logic#2025k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Conversation
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @LukeAVanDrie. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
needs-ok-to-test
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, LukeAVanDrie The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
This change improves the robustness of header manipulation in the ext_proc server. It implements strict sanitization for "system-owned" headers (such as Content-Length and internal routing metadata) in both the request and response paths. Previously, these headers were passed through transparently from the input. This change ensures the extension maintains authoritative control over protocol and routing headers, preventing potential ambiguity in downstream processing.
LukeAVanDrie
force-pushed
the
fix/headers-1624
branch
from
a20b48c to
95d9f1b
Compare
k8s-ci-robot
removed
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
| var ( | ||
| // InputControlHeaders are sent by the Gateway/User to control EPP behavior. | ||
| // We must extract these, then strip them so they don't leak to the backend. | ||
| InputControlHeaders = map[string]bool{ |
There was a problem hiding this comment.
nit: there’s no meaning for the “true” here, and this map acts as a hash set.
could be nice to use “set” from k8s utils instead of this map, and use the set.Has function (it’s backed by a map to struct{}).
it reads clearer IMO.
(if you want to push in a follow up)
There was a problem hiding this comment.
example here:
There was a problem hiding this comment.
I considered this but figured map[string]bool or map[string]struct{} was idiomatic enough. The set utility reads nicer though, especially now that we have generics. Will send out a small followup!
) This change improves the robustness of header manipulation in the ext_proc server. It implements strict sanitization for "system-owned" headers (such as Content-Length and internal routing metadata) in both the request and response paths. Previously, these headers were passed through transparently from the input. This change ensures the extension maintains authoritative control over protocol and routing headers, preventing potential ambiguity in downstream processing.
…ateway-api-inference-extension#2025) This change improves the robustness of header manipulation in the ext_proc server. It implements strict sanitization for "system-owned" headers (such as Content-Length and internal routing metadata) in both the request and response paths. Previously, these headers were passed through transparently from the input. This change ensures the extension maintains authoritative control over protocol and routing headers, preventing potential ambiguity in downstream processing.
4 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR hardens the HTTP header handling logic within the extension to ensure strict ownership of protocol and system metadata headers.
Previously, the header generation logic in
generateHeaders(Request) andgenerateResponseHeaders(Response) acted as a transparent passthrough, appending user-supplied headers to the outgoing message.Changes:
requtil.IsSystemOwnedHeaderto define a specific set of headers (Protocol, Routing, and Control) that must be managed exclusively by the extension.Reasoning:
These changes are necessary to prevent ambiguity in downstream proxies (Envoy) and backends. For example, internal routing headers should not be influenceable by external input.
Which issue(s) this PR fixes:
Fixes #1624
Does this PR introduce a user-facing change?: