Conversation
… transcoder Implement path parameter validation in GrpcTranscoder#bind_uri_values! to protect GAPIC REST clients from path traversal and parameter injection exploits. Specifically, this change: - Rejects query (?) and fragment (#) characters in path parameter values. - Rejects slashes (/) and dots (. or ..) in standard single-wildcard (*) variables. - Validates path traversals in double-wildcard (**) variables using the segment-traversal validation algorithm. - Extracts the wildcard segment using the __wildcard__ named capture group, with on-the-fly regex patching fallback for legacy stubs. - Validates prefix segments to prevent prefix buffering bypasses. Note: If the canonical specification is updated to favor the simpler alternative, this implementation can be easily simplified to "Fail always if dots are found in a double-wildcard value" by replacing the segment-traversal logic with a simple check for any "." or ".." substring.
quartzmo
force-pushed
the
gapic-common-validation
branch
from
August 3, 2026 17:35
fda77f0 to
25e80c6
Compare
quartzmo
marked this pull request as ready for review
August 4, 2026 23:30
danieljbruce
added a commit
to googleapis/google-cloud-node
that referenced
this pull request
Aug 18, 2026
… transcoder (#9151) ## Description [Protect GAPIC REST clients from path traversal](https://b.corp.google.com/issues/506021899/dependencies) and parameter injection exploits with two mechanisms: 1. Percent encodes special characters to ensure intended operations like reads can't become writes etc. 2. Throws an error when .. or . characters are used for path traversal. This differs depending on whether the * or ** wildcard is used. For * throws an error when exact matches are made with . and .. . For ** throws an error when the placeholder includes . or .. between slashes. ## Impact Improves the security of our clients by preventing exploits. ## Testing It should be noted that some new tests in packages/google-cloud-dialogflow-cx/test/transcoding_validation.ts were skipped because they fail in the CI pipeline. This is because they pass with these changes, but these changes are to gax and google-cloud-dialogflow-cx does not get this version of gax yet until it is published. To test locally: cd packages/google-cloud-dialogflow-cx pnpm link ../../core/packages/gax npm run compile && npx mocha build/test/transcoding_validation.js Transcoding tests are also added which provide support for ** wildcard which the dialogflow-cx tests can't cover. ## Additional Info I identified a refactor in https://github.com/googleapis/google-cloud-node/pull/9150/changes which should be worth considering to improve readability. It should also be noted that googleapis/ruby-core-libraries#67 was done for Ruby which has differences that may vary from language to language. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
torreypayne
approved these changes
Aug 19, 2026
| "for field #{field_binding.field_path.inspect}" | ||
| end | ||
|
|
||
| validate_path_binding! field_binding, field_value |
Member
There was a problem hiding this comment.
super nit: we could just call these methods independently on L121 and L122 instead of wrapping validate_path_binding!.
Contributor
|
@quartzmo LGTM overall! One question: earlier we merged the generator change to add the |
Member
Author
@aandreassa, good catch. You're right that this PR no longer makse use of the capture group. The final design (updated after merging the capture group PR) changed the solution to simply reject the request if any segment in the parameter value is |
quartzmo
force-pushed
the
gapic-common-validation
branch
from
August 27, 2026 22:07
69e9172 to
d0210db
Compare
efevans
approved these changes
Aug 28, 2026
efevans
left a comment
efevans
left a comment
There was a problem hiding this comment.
Can't speak on the ruby-ness but domain-wise looks good
| next unless segment == "." || segment == ".." | ||
| if field_binding.preserve_slashes | ||
| raise ::Gapic::Common::Error, | ||
| "Value for #{field_binding.field_path} must not contain segments that are exactly . or .." |
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
5 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.
Implement path parameter validation in GrpcTranscoder#bind_uri_values! to protect
GAPIC REST clients from path traversal and parameter injection exploits.
Specifically, this change: