mcp_json_rest_bridge: reject path traversal in path-template variables#46067
Open
davidvo-lyft wants to merge 1 commit into
Open
mcp_json_rest_bridge: reject path traversal in path-template variables#46067davidvo-lyft wants to merge 1 commit into
davidvo-lyft wants to merge 1 commit into
Conversation
The constructed upstream :path is installed verbatim (not re-normalized by
Envoy), so an unauthenticated client could inject '.'/'..' segments through
a path-template variable value and reach unintended upstream endpoints.
Reject '.'/'..' traversal segments in every template variable value, and
additionally percent-encode '/' in simple variables ({id}) so they cannot
span segments.
Known follow-ups (worth maintainer/security input): wildcard variable
values ({name=projects/*}) are not yet validated against their declared
pattern, so extra path segments (no traversal) can still be injected; and
backslash segments are only mitigated by percent-encoding.
Fixes envoyproxy#45931
Signed-off-by: David Vo <davidvo@lyft.com>
davidvo-lyft
requested review from
botengyao,
tyxia and
yanavlasov
as code owners
davidvo-lyft
requested a deployment
to
external-contributors
GitHub Actions
Waiting
|
Hi @davidvo-lyft, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
guoyilin42
approved these changes
Jul 9, 2026
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
3 participants
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.
The constructed upstream
:pathis installed verbatim (Envoy does not re-normalize it), so an unauthenticated client could inject./..segments through a path-template variable value and reach unintended upstream endpoints. This rejects./..traversal segments in every template variable value, and additionally percent-encodes/in simple variables ({id}) so they cannot span segments.Scope is deliberately limited to traversal. Two related items I'd like maintainer/security input on before extending (kept out of this PR to keep it reviewable):
{name=projects/*}) are not validated against their declared pattern, so extra path segments (no..) can still be injected.Risk Level: Low (extension is alpha; change contained to http_request_builder)
Testing: http_request_builder_test — new red->green cases covering traversal rejection and slash encoding.
Docs Changes: n/a
Release Notes: changelog added
Fixes #45931