fix(node): stamp verified X-Peer-Id on mesh service ingress - #338
Merged
Conversation
After AuthZ succeeds, the ingress handler now sets X-Peer-Id to the transport-verified caller peer id on requests forwarded to backend services, overwriting any inbound value. Previously a client-supplied X-Peer-Id was forwarded untouched across the mesh, so any backend trusting that header (as the inference service does on loopback) was spoofable. Adds api.HeaderPeerID and adopts it at the existing literal call sites. Fixes google#323
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request standardizes the propagation of the caller's verified libp2p peer ID to backend services using a new HeaderPeerID constant (X-Peer-Id), ensuring that any inbound spoofed values are overwritten at the ingress server. The changes are reflected across the inference service, OpenAI facade, and test suites. The reviewer recommends also explicitly deleting the X-Sam-No-Trailing-Slash header at the ingress handler to prevent potential spoofing of path semantics.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #323 — and it turned out to be a fix, not just a feature: the ingress handler forwarded a client-supplied
X-Peer-Idheader untouched to backend services, so any service trusting that header for caller attribution (asinference_service.godoes for loopback requests) was spoofable by any authorized mesh caller.Changes:
X-Peer-Idwith the transport-verified caller peer id, usingSetso an inbound value is always overwritten — same semantics the inference facade already uses.api.HeaderPeerIDconstant; adopted at the two existing string-literal call sites (no behavior change there).Tests (both verified red against the old code):
TestDatapathHeadersAndRoutingTable): new case asserting an inbound spoofedX-Peer-Idis replaced with the verified caller id.TestIntegrationHTTPDatapath): the cross-binary datapath test's backend now captures received headers and asserts the verified peer id arrives, a spoofed inbound value is overwritten, andX-Sam-Biscuitnever leaks to the backend. Red run against the old code fails withgot "spoofed-peer", want verified caller "12D3Koo…". Runs in ~4.3s, within the integration budget; no e2e addition per the push-down rule.Fixes #323