fix: secure URL transcoding path traversal vulnerability in REST fallback - #9146
fix: secure URL transcoding path traversal vulnerability in REST fallback#9146danieljbruce wants to merge 22 commits into
Conversation
…back
To prevent directory traversal and reserved-character injection attacks in fallback HTTP/REST transport, this commit implements the strict transcoding validation guidelines:
1. Rejects single-asterisk (*) matches resolving exactly to '.' or '..' with error: 'Invalid value {value} for {propertyName}'.
2. Rejects double-asterisk (**) matches containing segments exactly '.' or '..' with error: 'Value for {propertyName} must not contain segments that are exactly . or ..'.
3. Percent-encodes all variable segments except unreserved [-__.~/0-9a-zA-Z], ensuring characters like '!', ''', '(', ')', and '*' are encoded.
4. Filters out null and undefined values in buildQueryStringComponents array serialization to prevent TypeErrors.
5. Guards optional unmatched groups in applyPattern.
Includes a rigorous suite of 10 tests using the google-cloud-dialogflow-cx library to verify these mitigations in action.
Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces path traversal validation and stricter percent-encoding (specifically for characters like !'()*) to the transcoding module in google-gax. It also updates query string building to handle null/undefined array elements safely and adds a test suite in the Dialogflow CX package. The reviewer feedback highlights that using a local link: dependency in package.json will break the package when published to npm, and warns against using fragile relative paths and dynamic require() statements in the test file. It is highly recommended to move these transcoding tests directly into the google-gax package to maintain clean decoupling.
| const transcodingPath = path.resolve(__dirname, '../../node_modules/google-gax/build/src/transcoding.js'); | ||
| const transcoding = require(transcodingPath); |
There was a problem hiding this comment.
This code violates two general rules:
- It hardcodes a relative path to
node_modules(../../node_modules/google-gax/...), which is fragile and can break across different environments or directory structures. - It uses
require()with a dynamically computed path variable (transcodingPath), which is incompatible with bundlers and static analysis tools.
To resolve this, please move these tests to the google-gax package (core/packages/gax/test/) where the transcoding module can be imported directly and cleanly without hacky path resolution.
References
- Avoid hardcoding relative paths to dependencies in node_modules. Instead, resolve the path dynamically using require.resolve and path.join to ensure robustness across different environments and directory structures.
- Do not use require() with a dynamically computed path variable. The path argument in require() must be a constant string literal to ensure compatibility with bundlers and runtime environments.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…870442' of https://github.com/googleapis/google-cloud-node into jules-transcoding-path-traversal-vuln-fix-6401005213542870442
…ath" This reverts commit 94095ab.
This reverts commit c898db0.
…path" This reverts commit d33163b.
…rse the path"" This reverts commit ca4f260.
This reverts commit ea99463.
1 participant
This PR resolves a directory path traversal and parameter injection vulnerability in the google-gax REST/Fallback transcoding layer. It implements strict validation of '.' and '..' segments for single and double asterisk wildcards and enforces RFC-3986 compliant percent encoding. It also includes 10 rigorous, high-quality tests on the @google-cloud/dialogflow-cx package to demonstrate these security mitigations and type safety fixes in action, without adding any lock files.
PR created automatically by Jules for task 6401005213542870442 started by @danieljbruce