Skip to content

Revert "fix: validate path parameters and prevent traversal/injection in REST transcoder" #9165

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.

Sign up for GitHub

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
danieljbruce merged 1 commit into from
Aug 18, 2026
+9 −260
Merged

Revert "fix: validate path parameters and prevent traversal/injection in REST transcoder" #9165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 9 additions & 70 deletions core/packages/gax/src/transcoding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,30 +118,6 @@ export function deleteField(request: JSONObject, field: string): void {
delete request[part];
}

// Validates a single path segment matched by a single wildcard (*).
// Checks that the segment is not exactly '.' or '..' (directory traversal indicators).
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 (**).
// 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).
function validateUriPath(propertyName: string, value: string): void {
if (value) {
// Split by slash and check for exact segment matches of '.' or '..' rather
// than using a simple string.includes('.') check. This avoids rejecting
// valid domain-scoped resource segments (e.g. projects/example.com:project-id).
const segments = value.split('/');
if (segments.some(segment => segment === '.' || segment === '..')) {
throw new Error(`Value for ${propertyName} must not contain segments that are exactly . or ..`);
}
}
}

export function buildQueryStringComponents(
request: JSONObject,
prefix = '',
Expand Down Expand Up @@ -172,35 +148,18 @@ export function buildQueryStringComponents(
return resultList;
}

/**
* 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 {string} str - The input string to encode.
* @returns {string} The percent-encoded string.
*/
export function encodeWithSlashes(str: string): string {
return encodeURIComponent(str).replace(
/[!'()*]/g, // Characters preserved by encodeURIComponent
character => '%' + character.charCodeAt(0).toString(16).toUpperCase()
);
return str
.split('')
.map(c => (c.match(/[-_.~0-9a-zA-Z]/) ? c : encodeURIComponent(c)))
.join('');
}

/**
* Percent-encodes a string according to RFC 3986, preserving unreserved
* characters (alpha-numeric, '-', '_', '.', and '~') and slashes ('/'). All other
* characters are percent-encoded.
*
* @param {string} str - The input string to encode.
* @returns {string} The percent-encoded string with slashes preserved.
*/
export function encodeWithoutSlashes(str: string): string {
return str.split('/').map(encodeWithSlashes).join('/');
return str
.split('')
.map(c => (c.match(/[-_.~0-9a-zA-Z/]/) ? c : encodeURIComponent(c)))
.join('');
}
Comment on lines 151 to 163

@gemini-code-assist gemini-code-assist Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Reverting these functions to use .split('') and mapping over individual characters with encodeURIComponent re-introduces a critical bug. When a string contains Unicode surrogate pairs (such as emojis or non-BMP characters), .split('') splits them into unpaired surrogate halves. Passing an unpaired surrogate to encodeURIComponent throws a URIError: URI malformed at runtime.

We should keep the robust, standard-compliant implementation that uses encodeURIComponent on the whole string and then replaces the preserved characters, which safely handles surrogate pairs.

export function encodeWithSlashes(str: string): string {
  return encodeURIComponent(str).replace(
    /[!'()*]/g,
    character => '%' + character.charCodeAt(0).toString(16).toUpperCase()
  );
}

export function encodeWithoutSlashes(str: string): string {
  return str.split('/').map(encodeWithSlashes).join('/');
}

@danieljbruce danieljbruce Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. We are going to reintroduce these changes again to solve the vulnerability.


function escapeRegExp(str: string) {
Expand All @@ -210,10 +169,8 @@ function escapeRegExp(str: string) {
export function applyPattern(
pattern: string,
fieldValue: string,
propertyName = 'resource', // Used to provide precise error messages when path validation fails
): string | undefined {
if (!pattern || pattern === '*') {
validateUriPathSegment(propertyName, fieldValue);
return encodeWithSlashes(fieldValue);
}

Expand All @@ -230,27 +187,10 @@ export function applyPattern(
'$',
);

const match = fieldValue.match(regex);
if (!match) {
if (!fieldValue.match(regex)) {
return undefined;
}

// Identify the segments and wildcards in pattern to perform validation in order of appearance
const wildcards: string[] = pattern.match(/\*\*|\*/g) || [];

// Check the captured group values
for (let i = 1; i < match.length; i++) {
const groupVal = match[i];
if (groupVal !== undefined && groupVal !== null) {
const wildcardType = wildcards[i - 1];
if (wildcardType === '*') {
validateUriPathSegment(propertyName, groupVal);
} else if (wildcardType === '**') {
validateUriPath(propertyName, groupVal);
}
}
}

return encodeWithoutSlashes(fieldValue);
}

Expand Down Expand Up @@ -285,7 +225,6 @@ export function match(
const appliedPattern = applyPattern(
pattern,
fieldValue === null ? 'null' : fieldValue!.toString(),
camelCasedField,
);
if (appliedPattern === undefined) {
return undefined;
Expand Down
58 changes: 0 additions & 58 deletions core/packages/gax/test/unit/transcoding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,24 +370,6 @@ describe('gRPC to HTTP transcoding', () => {
);
});

it('should correctly handle Unicode surrogate pairs in encodeWithSlashes', () => {
// Emojis (like 😊) are surrogate pairs.
// They should be encoded successfully instead of throwing a URIError.
assert.strictEqual(encodeWithSlashes('😊'), '%F0%9F%98%8A');
});

it('should preserve unreserved characters while strictly percent-encoding all other characters in encodeWithSlashes', () => {
// Standard RFC unreserved characters: [-_.~0-9a-zA-Z]
const unreserved = 'abc-123_.~';
assert.strictEqual(encodeWithSlashes(unreserved), unreserved);

// Reserved and special characters: should be percent encoded, including !\'()*
const specialChars = "!\'()*";
const encoded = encodeWithSlashes(specialChars);
// ! -> %21, ' -> %27, ( -> %28, ) -> %29, * -> %2A
assert.strictEqual(encoded, '%21%27%28%29%2A');
});

it('encodeWithoutSlashes', () => {
assert.strictEqual(encodeWithoutSlashes('abcd'), 'abcd');
assert.strictEqual(
Expand All @@ -402,12 +384,6 @@ describe('gRPC to HTTP transcoding', () => {
);
});

it('should correctly handle Unicode surrogate pairs in encodeWithoutSlashes', () => {
// Emojis (like 😊) are surrogate pairs.
// They should be encoded successfully instead of throwing a URIError.
assert.strictEqual(encodeWithoutSlashes('😊'), '%F0%9F%98%8A');
});

it('applyPattern', () => {
assert.strictEqual(applyPattern('*', 'test'), 'test');
assert.strictEqual(applyPattern('test', 'test'), 'test');
Expand Down Expand Up @@ -435,40 +411,6 @@ describe('gRPC to HTTP transcoding', () => {
);
});

it('applyPattern should throw an error for double-asterisk segment traversal containing segments that are exactly ".."', () => {
assert.throws(() => {
applyPattern(
'projects/*/locations/*/agents/*/sessions/**',
'projects/p/locations/l/agents/a/sessions/agents/../subagent',
'session'
);
}, /Value for session must not contain segments that are exactly \. or \.\./);
});

it('applyPattern should throw an error for double-asterisk segment traversal containing segments that are exactly "."', () => {
assert.throws(() => {
applyPattern(
'projects/*/locations/*/agents/*/sessions/**',
'projects/p/locations/l/agents/a/sessions/agents/./subagent',
'session'
);
}, /Value for session must not contain segments that are exactly \. or \.\./);
});

it('applyPattern should percent-encode query injection attempt on double-asterisk without throwing traversal error', () => {
const res = applyPattern(
'projects/*/locations/*/agents/*/sessions/**',
'projects/p/locations/l/agents/a/sessions/..?$foo=BAR#',
'session'
);
assert.strictEqual(res, 'projects/p/locations/l/agents/a/sessions/..%3F%24foo%3DBAR%23');
});

it('applyPattern should handle optional unmatched groups gracefully without throwing TypeErrors', () => {
const res = applyPattern('projects/*', 'projects/p', 'session');
assert.strictEqual(res, 'projects/p');
});

it('flattenObject', () => {
assert.deepStrictEqual(flattenObject({}), {});
assert.deepStrictEqual(flattenObject({field: 'value'}), {field: 'value'});
Expand Down
132 changes: 0 additions & 132 deletions packages/google-cloud-dialogflow-cx/test/transcoding_validation.ts
View file
Open in desktop

This file was deleted.

Loading