-
Notifications
You must be signed in to change notification settings - Fork 713
fix: validate path parameters and prevent traversal/injection in apiary request encodings #9181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
f2031d9
Do . and .. check and percent encode
danieljbruce 2cef1c5
clean the tsconfig file
danieljbruce 133114f
Add a comment about aliases
danieljbruce 2e93597
Add some comments to the encoding
danieljbruce 0f2977c
Eliminate the url tostring change
danieljbruce cf8edea
Remove the transcoding import
danieljbruce d6ce21f
Separate the two methods
danieljbruce 0be0c31
Get rid of the backwards compatibility helper
danieljbruce a324113
Merge branch 'main' of https://github.com/googleapis/google-cloud-nod…
danieljbruce 05f3b8e
Adopt changes from other vulnerability PR
danieljbruce 7c622cd
Add dialogflow tests
danieljbruce ff9fdaa
Introduce proper method names
danieljbruce 39b911d
Update the dialogflow tests
danieljbruce 899a822
Reduce test size for demonstration purposes
danieljbruce ff7bc4f
move comment to bottom
danieljbruce ddb0991
Simplify validateAndEncode
danieljbruce 0ac0aa5
Undo unnecessary changes
danieljbruce bc35c8f
Consolidate the code into multi and single path
danieljbruce 8f11100
inline urlTemplateString
danieljbruce 4a31513
reduce api surface for code change
danieljbruce 6c76cd3
Eliminate the while loop
danieljbruce 567330d
Do the scan by wildcards instead
danieljbruce c60f22d
Merge branch 'main' of https://github.com/googleapis/google-cloud-nod…
danieljbruce 7d6bfb2
Add the code snippet verbatim
danieljbruce c206f2d
Add comments about refactor
danieljbruce 188385e
consolidate all the code into applyPattern
danieljbruce 8ce8ad1
Add JS documentation
danieljbruce 9f98c3a
Simplify use of apply pattern
danieljbruce b0ea0dd
Add an input/output example
danieljbruce 5e6f379
Change the variable name to parameterValue
danieljbruce 91860e5
Add a single wildcard test
danieljbruce 9b02fe8
chore: remove normalizePathParams from request pipeline
danieljbruce b564f35
refactor with comments so the * and ** distinction is clear
danieljbruce 4345676
Remove the extra line
danieljbruce e30270d
Merge branch 'main' into apiary-path-traversal-vulnerability
danieljbruce 5d64cb9
simplify expression
danieljbruce 2d5e51f
Eliminate applyPattern
danieljbruce 45be523
Add a comment explaining the rationale of the code
danieljbruce a96ae8e
Replace the templating function
danieljbruce File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
181 changes: 181 additions & 0 deletions
181
core/packages/nodejs-googleapis-common/src/transcoding.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| // Copyright 2026 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| /** | ||
| * Validates a single path segment matched by a single wildcard (*) or {param}. | ||
| * Checks that the segment is not exactly '.' or '..' (directory traversal indicators). | ||
| * | ||
| * @param propertyName Name of the parameter being validated | ||
| * @param value Value of the path segment | ||
| */ | ||
| function validateUriPathSegment( | ||
| propertyName: string, | ||
| value: string, | ||
| ): void { | ||
| if (value === '.' || value === '..') { | ||
| throw new Error(`Invalid value ${value} for ${propertyName}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validates a multi-segment path matched by a double wildcard (**) or {+param}. | ||
| * Splitting by slash, it checks that no individual segment is exactly '.' or '..'. | ||
| * This segment-by-segment check prevents directory traversal while allowing | ||
| * legitimate resource names containing dots (e.g., domain-scoped project IDs). | ||
| * | ||
| * @param propertyName Name of the parameter being validated | ||
| * @param value Value of the multi-segment path | ||
| */ | ||
| function validateUriPath( | ||
| propertyName: string, | ||
| value: string, | ||
| ): void { | ||
| if (value) { | ||
| const segments = value.split('/'); | ||
| if (segments.some(segment => segment === '.' || segment === '..')) { | ||
| throw new Error( | ||
| `Value for ${propertyName} must not contain segments that are exactly . or ..`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Percent-encodes a string according to RFC 3986, preserving only unreserved | ||
| * characters (alpha-numeric, '-', '_', '.', and '~'). All other characters, | ||
| * including slashes ('/'), are percent-encoded. | ||
| * | ||
| * This is necessary because encodeURIComponent natively encodes URL-unsafe | ||
| * characters like ?, #, $, &, +, etc., but preserves !, ', (, ), and *. | ||
| * To ensure strict compliance, we manually encode those preserved characters. | ||
| * | ||
| * @param str The input string to encode | ||
| * @returns The percent-encoded string | ||
| */ | ||
| function encodeWithSlashes(str: string): string { | ||
| return encodeURIComponent(str).replace( | ||
| /[!'()*]/g, // Characters preserved by encodeURIComponent | ||
| character => '%' + character.charCodeAt(0).toString(16).toUpperCase(), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Percent-encodes a string according to RFC 3986, preserving unreserved | ||
| * characters (alpha-numeric, '-', '_', '.', and '~') and slashes ('/'). All other | ||
| * characters are percent-encoded. | ||
| * | ||
| * @param str The input string to encode | ||
| * @returns The percent-encoded string with slashes preserved | ||
| */ | ||
| function encodeWithoutSlashes(str: string): string { | ||
| return str.split('/').map(encodeWithSlashes).join('/'); | ||
| } | ||
|
|
||
| /** | ||
| * Extracts template parameters and their corresponding wildcard types ('*' or '**'). | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * // Input: | ||
| * 'https://example.com/v1/{+parent}/databases/{databaseId}/documents/{+documentPath}' | ||
| * | ||
| * // Output: | ||
| * [ | ||
| * { param: 'parent', wildcard: '**' }, | ||
| * { param: 'databaseId', wildcard: '*' }, | ||
| * { param: 'documentPath', wildcard: '**' } | ||
| * ] | ||
| * ``` | ||
| * | ||
| * @param urlTemplate The RFC 6570 URI template string | ||
| * @returns Array of parameter names and their associated wildcard pattern | ||
| */ | ||
| function extractTemplateParams(urlTemplate: string): Array<{ | ||
| param: string; | ||
| wildcard: '*' | '**'; | ||
| }> { | ||
| const paramMap = new Map<string, '*' | '**'>(); | ||
|
|
||
| // Natively skips {}, {#}, {?}, and {,} by demanding valid variable characters | ||
| const matches = urlTemplate.matchAll(/\{(\+?)([a-zA-Z0-9_$-]+)\}/g); | ||
|
|
||
| for (const match of matches) { | ||
| const wildcard = match[1] === '+' ? '**' : '*'; | ||
| const paramName = match[2]; | ||
|
|
||
| if (wildcard === '**' || !paramMap.has(paramName)) { | ||
| paramMap.set(paramName, wildcard); | ||
| } | ||
| } | ||
|
|
||
| return Array.from(paramMap.entries()).map(([param, wildcard]) => ({ | ||
| param, | ||
| wildcard, | ||
| })); | ||
| } | ||
|
|
||
| /** | ||
| * Validates path parameters against traversal attacks ('.' and '..') and encodes | ||
| * multi-segment parameters in params so that reserved characters (query params, fragments, etc.) | ||
| * cannot be injected into the path. Modifies params in-place. | ||
| * | ||
| * @param urlTemplate URL template associated with the request (e.g. url, mediaUrl) | ||
| * @param params Request parameters dictionary (modified in-place) | ||
| */ | ||
| export function validateAndEncodeParams( | ||
| urlTemplate: string | undefined, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| params: Record<string, any>, | ||
| ): void { | ||
| // Early return if params is undefined, null, or not an object, or if urlTemplate is missing | ||
| if (!params || typeof params !== 'object' || !urlTemplate) { | ||
| return; | ||
| } | ||
|
|
||
| // Identify the parameters and wildcards in the URL template | ||
| const templateParams = extractTemplateParams(urlTemplate); | ||
|
|
||
| for (const {param, wildcard} of templateParams) { | ||
| const parameterValue = params[param]; | ||
| if (parameterValue === undefined || parameterValue === null) { | ||
| continue; | ||
| } | ||
| if (wildcard === '**') { | ||
| // This block applies the core logic of google-gax's applyPattern method, | ||
| // but is greatly simplified because the wildcard type ('**') is already known. | ||
| // As a result, we do not need to convert arbitrary patterns into regular | ||
| // expressions, match against field values, extract capture groups, or | ||
| // scan and dispatch validation for variable wildcard types. We can directly | ||
| // validate against traversal segments and encode with slashes preserved. | ||
| const encodeParam = (val: string) => { | ||
| validateUriPath(param, val); | ||
| return encodeWithoutSlashes(val); | ||
| }; | ||
| params[param] = Array.isArray(parameterValue) | ||
| ? parameterValue.map(item => encodeParam(String(item))) | ||
| : encodeParam(String(parameterValue)); | ||
| } else { | ||
| // For single-segment parameters (*), only validation against path traversal (. and ..) | ||
| // is needed here. Character percent-encoding is handled automatically by url-template later | ||
| // when urlTemplate.parse(url).expand(params) is called in createAPIRequestAsync. | ||
| if (Array.isArray(parameterValue)) { | ||
| parameterValue.forEach(item => | ||
| validateUriPathSegment(param, String(item)), | ||
| ); | ||
| } else { | ||
| validateUriPathSegment(param, String(parameterValue)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this covers some advanced templating that I don't think we use in Apiary.
Consider:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I've applied this code to simplify the method.