From d5928d48137bb52cfd61b7a363ed7e28defa9dac Mon Sep 17 00:00:00 2001 From: danieljbruce Date: Tue, 18 Aug 2026 14:35:47 -0400 Subject: [PATCH 1/7] =?UTF-8?q?Revert=20"fix:=20validate=20path=20paramete?= =?UTF-8?q?rs=20and=20prevent=20traversal/injection=20in=20REST=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 0152a8ee36592d90f435336fda95993c9cbd227a. --- core/packages/gax/src/transcoding.ts | 79 ++--------- core/packages/gax/test/unit/transcoding.ts | 58 -------- .../test/transcoding_validation.ts | 132 ------------------ 3 files changed, 9 insertions(+), 260 deletions(-) delete mode 100644 packages/google-cloud-dialogflow-cx/test/transcoding_validation.ts diff --git a/core/packages/gax/src/transcoding.ts b/core/packages/gax/src/transcoding.ts index 070612ec117c..3d27a86c4f74 100644 --- a/core/packages/gax/src/transcoding.ts +++ b/core/packages/gax/src/transcoding.ts @@ -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 = '', @@ -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(''); } function escapeRegExp(str: string) { @@ -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); } @@ -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); } @@ -285,7 +225,6 @@ export function match( const appliedPattern = applyPattern( pattern, fieldValue === null ? 'null' : fieldValue!.toString(), - camelCasedField, ); if (appliedPattern === undefined) { return undefined; diff --git a/core/packages/gax/test/unit/transcoding.ts b/core/packages/gax/test/unit/transcoding.ts index 3d1d613adf1e..4c0d40395107 100644 --- a/core/packages/gax/test/unit/transcoding.ts +++ b/core/packages/gax/test/unit/transcoding.ts @@ -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( @@ -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'); @@ -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'}); diff --git a/packages/google-cloud-dialogflow-cx/test/transcoding_validation.ts b/packages/google-cloud-dialogflow-cx/test/transcoding_validation.ts deleted file mode 100644 index 9130c26a8e79..000000000000 --- a/packages/google-cloud-dialogflow-cx/test/transcoding_validation.ts +++ /dev/null @@ -1,132 +0,0 @@ -// 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 -// -// https://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. - -import * as assert from 'assert'; -import { describe, it } from 'mocha'; -import { v3 } from '../src'; - -const sinon = require('sinon'); - -describe('Dialogflow CX Fallback Transcoding and Path Traversal Prevention', () => { - let client: v3.SessionsClient; - let fetchStub: any; - - beforeEach(() => { - client = new v3.SessionsClient({ - fallback: true, - credentials: { client_email: 'bogus@example.com', private_key: 'bogus' }, - projectId: 'bogus', - }); - fetchStub = sinon.stub().resolves({ - ok: true, - status: 200, - arrayBuffer: () => Promise.resolve(Buffer.from('{}')), - }); - client.auth.fetch = fetchStub; - }); - - // Test 1: Single Asterisk Dot Validation on client call - it.skip('1. should throw an error for single-asterisk segment traversal using exactly "." as session ID', async () => { - // TODO: Re-enable this test when the gax version with the new encoding is released. - await client.initialize(); - await assert.rejects( - client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/.', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }), - /Invalid value \. for session/ - ); - }); - - // Test 2: Single Asterisk Dot-Dot Validation on client call - it.skip('2. should throw an error for single-asterisk segment traversal using exactly ".." as session ID', async () => { - // TODO: Re-enable this test when the gax version with the new encoding is released. - await client.initialize(); - await assert.rejects( - client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/..', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }), - /Invalid value \.\. for session/ - ); - }); - - - - // Test 5: Standard Valid Path fallback REST call - it('5. should pass transcoding validation with a valid session path and construct the correct REST URL', async () => { - await client.initialize(); - await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/valid-session-id', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }); - assert.strictEqual(fetchStub.callCount, 1); - const requestUrl = fetchStub.firstCall.args[0]; - assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/valid-session-id:detectIntent')); - }); - - // Test 6: Query Parameter Injection Prevention via percent-encoding - it('6. should protect against query parameter injection by percent-encoding "?" and "$" in the session ID', async () => { - await client.initialize(); - await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/my-session?$httpMethod=DELETE#', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }); - assert.strictEqual(fetchStub.callCount, 1); - const requestUrl = fetchStub.firstCall.args[0]; - // "?" -> %3F, "$" -> %24, "=" -> %3D, "#" -> %23 - assert.ok(requestUrl.includes('my-session%3F%24httpMethod%3DDELETE%23')); - }); - - // Test 7: Combined Path Traversal and Query Parameter Injection - it('7. should protect against path traversal and query injection by percent-encoding combined patterns', async () => { - // This request is permitted because the template uses * instead of ** - // * is supposed to match against exactly . or .. - // This is okay because we still percent encode the ? parameter. - // example: POST https://-dialogflow.googleapis.com/v3/{session=projects/*/locations/*/agents/*/sessions/*}:detectIntent - await client.initialize(); - await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/..?$httpMethod=DELETE#', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }); - assert.strictEqual(fetchStub.callCount, 1); - const requestUrl = fetchStub.firstCall.args[0]; - assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/..%3F%24httpMethod%3DDELETE%23:detectIntent')); - }); - - // Test 8: Combined Path Traversal (.) and Query Parameter Injection - it('8. should protect against path traversal and query injection by percent-encoding combined patterns using dot', async () => { - await client.initialize(); - await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/.?$httpMethod=DELETE#', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }); - assert.strictEqual(fetchStub.callCount, 1); - const requestUrl = fetchStub.firstCall.args[0]; - assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/.%3F%24httpMethod%3DDELETE%23:detectIntent')); - }); - - // Test 9: Percent-encoding all other characters - it.skip('9. should percent-encode all other characters except unreserved ones', async () => { - // TODO: Re-enable this test when the gax version with the new encoding is released. - await client.initialize(); - await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/ !@$&\'()*+,;=:%', - queryInput: { text: { text: 'hello' }, languageCode: 'en' }, - }); - assert.strictEqual(fetchStub.callCount, 1); - const requestUrl = fetchStub.firstCall.args[0]; - assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/%20%21%40%24%26%27%28%29%2A%2B%2C%3B%3D%3A%25:detectIntent')); - }); -}); From 3c595c3b78745003d2696a45feb82bbc1b2163c5 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 18 Aug 2026 15:08:49 -0400 Subject: [PATCH 2/7] reintroduce the transcoding changes for vulnerability --- core/packages/gax/src/transcoding.ts | 85 +++++++++-- core/packages/gax/test/unit/transcoding.ts | 61 ++++++++ .../gax/test/unit/transcoding_validation.ts | 138 ++++++++++++++++++ 3 files changed, 274 insertions(+), 10 deletions(-) create mode 100644 core/packages/gax/test/unit/transcoding_validation.ts diff --git a/core/packages/gax/src/transcoding.ts b/core/packages/gax/src/transcoding.ts index 3d27a86c4f74..f855c8875ac8 100644 --- a/core/packages/gax/src/transcoding.ts +++ b/core/packages/gax/src/transcoding.ts @@ -118,6 +118,32 @@ 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 = '', @@ -140,7 +166,9 @@ export function buildQueryStringComponents( } else { resultList.push( `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( - requestValue === null || requestValue === undefined ? 'null' : requestValue.toString(), + requestValue === null || requestValue === undefined + ? 'null' + : requestValue.toString(), )}`, ); } @@ -148,18 +176,35 @@ 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 str - .split('') - .map(c => (c.match(/[-_.~0-9a-zA-Z]/) ? c : encodeURIComponent(c))) - .join(''); + 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 {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(c => (c.match(/[-_.~0-9a-zA-Z/]/) ? c : encodeURIComponent(c))) - .join(''); + return str.split('/').map(encodeWithSlashes).join('/'); } function escapeRegExp(str: string) { @@ -169,8 +214,10 @@ 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); } @@ -187,10 +234,27 @@ export function applyPattern( '$', ); - if (!fieldValue.match(regex)) { + const match = fieldValue.match(regex); + if (!match) { 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); } @@ -225,6 +289,7 @@ export function match( const appliedPattern = applyPattern( pattern, fieldValue === null ? 'null' : fieldValue!.toString(), + camelCasedField, ); if (appliedPattern === undefined) { return undefined; diff --git a/core/packages/gax/test/unit/transcoding.ts b/core/packages/gax/test/unit/transcoding.ts index 4c0d40395107..5eb901223f2c 100644 --- a/core/packages/gax/test/unit/transcoding.ts +++ b/core/packages/gax/test/unit/transcoding.ts @@ -370,6 +370,24 @@ 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( @@ -384,6 +402,12 @@ 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'); @@ -411,6 +435,43 @@ 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'}); diff --git a/core/packages/gax/test/unit/transcoding_validation.ts b/core/packages/gax/test/unit/transcoding_validation.ts new file mode 100644 index 000000000000..7d4a09e48266 --- /dev/null +++ b/core/packages/gax/test/unit/transcoding_validation.ts @@ -0,0 +1,138 @@ +// 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 +// +// https://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. + +import * as assert from 'assert'; +import { describe, it } from 'mocha'; +import * as path from 'path'; + +const dialogflowPath = path.resolve( + __dirname, + '../../../../../../packages/google-cloud-dialogflow-cx' +); +const { v3 } = require(dialogflowPath); + +const sinon = require('sinon'); + +describe('Dialogflow CX Fallback Transcoding and Path Traversal Prevention', () => { + let client: any; + let fetchStub: any; + + beforeEach(() => { + client = new v3.SessionsClient({ + fallback: true, + credentials: { client_email: 'bogus@example.com', private_key: 'bogus' }, + projectId: 'bogus', + }); + fetchStub = sinon.stub().resolves({ + ok: true, + status: 200, + arrayBuffer: () => Promise.resolve(Buffer.from('{}')), + }); + client.auth.fetch = fetchStub; + }); + + // Test 1: Single Asterisk Dot Validation on client call + it.skip('1. should throw an error for single-asterisk segment traversal using exactly "." as session ID', async () => { + // TODO: Re-enable this test when the gax version with the new encoding is released. + await client.initialize(); + await assert.rejects( + client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/.', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }), + /Invalid value \. for session/ + ); + }); + + // Test 2: Single Asterisk Dot-Dot Validation on client call + it.skip('2. should throw an error for single-asterisk segment traversal using exactly ".." as session ID', async () => { + // TODO: Re-enable this test when the gax version with the new encoding is released. + await client.initialize(); + await assert.rejects( + client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/..', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }), + /Invalid value \.\. for session/ + ); + }); + + + + // Test 5: Standard Valid Path fallback REST call + it('5. should pass transcoding validation with a valid session path and construct the correct REST URL', async () => { + await client.initialize(); + await client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/valid-session-id', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }); + assert.strictEqual(fetchStub.callCount, 1); + const requestUrl = fetchStub.firstCall.args[0]; + assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/valid-session-id:detectIntent')); + }); + + // Test 6: Query Parameter Injection Prevention via percent-encoding + it('6. should protect against query parameter injection by percent-encoding "?" and "$" in the session ID', async () => { + await client.initialize(); + await client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/my-session?$httpMethod=DELETE#', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }); + assert.strictEqual(fetchStub.callCount, 1); + const requestUrl = fetchStub.firstCall.args[0]; + // "?" -> %3F, "$" -> %24, "=" -> %3D, "#" -> %23 + assert.ok(requestUrl.includes('my-session%3F%24httpMethod%3DDELETE%23')); + }); + + // Test 7: Combined Path Traversal and Query Parameter Injection + it('7. should protect against path traversal and query injection by percent-encoding combined patterns', async () => { + // This request is permitted because the template uses * instead of ** + // * is supposed to match against exactly . or .. + // This is okay because we still percent encode the ? parameter. + // example: POST https://-dialogflow.googleapis.com/v3/{session=projects/*/locations/*/agents/*/sessions/*}:detectIntent + await client.initialize(); + await client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/..?$httpMethod=DELETE#', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }); + assert.strictEqual(fetchStub.callCount, 1); + const requestUrl = fetchStub.firstCall.args[0]; + assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/..%3F%24httpMethod%3DDELETE%23:detectIntent')); + }); + + // Test 8: Combined Path Traversal (.) and Query Parameter Injection + it('8. should protect against path traversal and query injection by percent-encoding combined patterns using dot', async () => { + await client.initialize(); + await client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/.?$httpMethod=DELETE#', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }); + assert.strictEqual(fetchStub.callCount, 1); + const requestUrl = fetchStub.firstCall.args[0]; + assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/.%3F%24httpMethod%3DDELETE%23:detectIntent')); + }); + + // Test 9: Percent-encoding all other characters + it.skip('9. should percent-encode all other characters except unreserved ones', async () => { + // TODO: Re-enable this test when the gax version with the new encoding is released. + await client.initialize(); + await client.detectIntent({ + session: 'projects/p/locations/l/agents/a/sessions/ !@$&\'()*+,;=:%', + queryInput: { text: { text: 'hello' }, languageCode: 'en' }, + }); + assert.strictEqual(fetchStub.callCount, 1); + const requestUrl = fetchStub.firstCall.args[0]; + assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/%20%21%40%24%26%27%28%29%2A%2B%2C%3B%3D%3A%25:detectIntent')); + }); +}); From d17aa365c73efdf8586e13bbe82c7ca02dc39deb Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 18 Aug 2026 16:02:21 -0400 Subject: [PATCH 3/7] reintroduce changes verbatim that AI missed --- core/packages/gax/src/transcoding.ts | 66 ++++++++++++++++------ core/packages/gax/test/unit/transcoding.ts | 35 ++++-------- 2 files changed, 60 insertions(+), 41 deletions(-) diff --git a/core/packages/gax/src/transcoding.ts b/core/packages/gax/src/transcoding.ts index 61ab518f48ff..40928240c66a 100644 --- a/core/packages/gax/src/transcoding.ts +++ b/core/packages/gax/src/transcoding.ts @@ -137,9 +137,7 @@ function validateUriPath(propertyName: string, value: string): void { // 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 ..`, - ); + throw new Error(`Value for ${propertyName} must not contain segments that are exactly . or ..`); } } } @@ -150,25 +148,22 @@ export function buildQueryStringComponents( ): string[] { const resultList = []; for (const key in request) { - const requestValue = request[key]; - if (Array.isArray(requestValue)) { - for (const value of requestValue as JSONObject[]) { + if (Array.isArray(request[key])) { + for (const value of request[key] as JSONObject[]) { resultList.push( `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( value.toString(), )}`, ); } - } else if (typeof requestValue === 'object' && requestValue !== null) { + } else if (typeof request[key] === 'object' && request[key] !== null) { resultList.push( - ...buildQueryStringComponents(requestValue as JSONObject, `${key}.`), + ...buildQueryStringComponents(request[key] as JSONObject, `${key}.`), ); } else { resultList.push( `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( - requestValue === null || requestValue === undefined - ? 'null' - : requestValue.toString(), + request[key] === null ? 'null' : request[key]!.toString(), )}`, ); } @@ -176,18 +171,35 @@ 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(), + 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 {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(c => (c.match(/[-_.~0-9a-zA-Z/]/) ? c : encodeURIComponent(c))) - .join(''); + return str.split('/').map(encodeWithSlashes).join('/'); } function escapeRegExp(str: string) { @@ -197,8 +209,10 @@ 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); } @@ -215,10 +229,27 @@ export function applyPattern( '$', ); - if (!fieldValue.match(regex)) { + const match = fieldValue.match(regex); + if (!match) { 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); } @@ -253,6 +284,7 @@ export function match( const appliedPattern = applyPattern( pattern, fieldValue === null ? 'null' : fieldValue!.toString(), + camelCasedField, ); if (appliedPattern === undefined) { return undefined; diff --git a/core/packages/gax/test/unit/transcoding.ts b/core/packages/gax/test/unit/transcoding.ts index 85d4a1f7b4b5..cdb02048a7fa 100644 --- a/core/packages/gax/test/unit/transcoding.ts +++ b/core/packages/gax/test/unit/transcoding.ts @@ -382,7 +382,7 @@ describe('gRPC to HTTP transcoding', () => { assert.strictEqual(encodeWithSlashes(unreserved), unreserved); // Reserved and special characters: should be percent encoded, including !\'()* - const specialChars = "!'()*"; + const specialChars = "!\'()*"; const encoded = encodeWithSlashes(specialChars); // ! -> %21, ' -> %27, ( -> %28, ) -> %29, * -> %2A assert.strictEqual(encoded, '%21%27%28%29%2A'); @@ -402,6 +402,12 @@ 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'); @@ -434,7 +440,7 @@ describe('gRPC to HTTP transcoding', () => { applyPattern( 'projects/*/locations/*/agents/*/sessions/**', 'projects/p/locations/l/agents/a/sessions/agents/../subagent', - 'session', + 'session' ); }, /Value for session must not contain segments that are exactly \. or \.\./); }); @@ -444,7 +450,7 @@ describe('gRPC to HTTP transcoding', () => { applyPattern( 'projects/*/locations/*/agents/*/sessions/**', 'projects/p/locations/l/agents/a/sessions/agents/./subagent', - 'session', + 'session' ); }, /Value for session must not contain segments that are exactly \. or \.\./); }); @@ -453,12 +459,9 @@ describe('gRPC to HTTP transcoding', () => { 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', + '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', () => { @@ -616,22 +619,6 @@ describe('gRPC to HTTP transcoding', () => { ], ); }); - - it('should gracefully handle undefined property values without throwing', () => { - const request = { - definedField: 'value', - undefinedField: undefined, - }; - - // Prior to PR 9150, this threw: TypeError: Cannot read properties of undefined (reading 'toString') - // With PR 9150, it gracefully serializes the undefined property to 'null' - const result = buildQueryStringComponents(request as any); - - assert.deepStrictEqual(result, [ - 'definedField=value', - 'undefinedField=null', - ]); - }); }); describe('override the HTTP rules in protoJson', () => { From 192fa6fa1e57522dfb0c6d7c4629cdbc18dbbf5c Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 18 Aug 2026 16:05:32 -0400 Subject: [PATCH 4/7] Undo the buildQueryStringComponents changes --- core/packages/gax/src/transcoding.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/packages/gax/src/transcoding.ts b/core/packages/gax/src/transcoding.ts index 40928240c66a..070612ec117c 100644 --- a/core/packages/gax/src/transcoding.ts +++ b/core/packages/gax/src/transcoding.ts @@ -148,22 +148,23 @@ export function buildQueryStringComponents( ): string[] { const resultList = []; for (const key in request) { - if (Array.isArray(request[key])) { - for (const value of request[key] as JSONObject[]) { + const requestValue = request[key]; + if (Array.isArray(requestValue)) { + for (const value of requestValue as JSONObject[]) { resultList.push( `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( value.toString(), )}`, ); } - } else if (typeof request[key] === 'object' && request[key] !== null) { + } else if (typeof requestValue === 'object' && requestValue !== null) { resultList.push( - ...buildQueryStringComponents(request[key] as JSONObject, `${key}.`), + ...buildQueryStringComponents(requestValue as JSONObject, `${key}.`), ); } else { resultList.push( `${prefix}${encodeWithoutSlashes(key)}=${encodeWithoutSlashes( - request[key] === null ? 'null' : request[key]!.toString(), + requestValue === null || requestValue === undefined ? 'null' : requestValue.toString(), )}`, ); } From 60586c789e9a5b859103ac1ed052ffdf541fdb15 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 18 Aug 2026 16:13:35 -0400 Subject: [PATCH 5/7] re-add the transcoding test --- core/packages/gax/test/unit/transcoding.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/packages/gax/test/unit/transcoding.ts b/core/packages/gax/test/unit/transcoding.ts index cdb02048a7fa..3d1d613adf1e 100644 --- a/core/packages/gax/test/unit/transcoding.ts +++ b/core/packages/gax/test/unit/transcoding.ts @@ -619,6 +619,22 @@ describe('gRPC to HTTP transcoding', () => { ], ); }); + + it('should gracefully handle undefined property values without throwing', () => { + const request = { + definedField: 'value', + undefinedField: undefined, + }; + + // Prior to PR 9150, this threw: TypeError: Cannot read properties of undefined (reading 'toString') + // With PR 9150, it gracefully serializes the undefined property to 'null' + const result = buildQueryStringComponents(request as any); + + assert.deepStrictEqual(result, [ + 'definedField=value', + 'undefinedField=null', + ]); + }); }); describe('override the HTTP rules in protoJson', () => { From 7dcf95409af3b82e6d0fbd0fea02c9951a6ea30d Mon Sep 17 00:00:00 2001 From: danieljbruce Date: Tue, 18 Aug 2026 16:15:28 -0400 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- core/packages/gax/test/unit/transcoding_validation.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/core/packages/gax/test/unit/transcoding_validation.ts b/core/packages/gax/test/unit/transcoding_validation.ts index 7d4a09e48266..50796e57f396 100644 --- a/core/packages/gax/test/unit/transcoding_validation.ts +++ b/core/packages/gax/test/unit/transcoding_validation.ts @@ -14,13 +14,7 @@ import * as assert from 'assert'; import { describe, it } from 'mocha'; -import * as path from 'path'; - -const dialogflowPath = path.resolve( - __dirname, - '../../../../../../packages/google-cloud-dialogflow-cx' -); -const { v3 } = require(dialogflowPath); +const { v3 } = require('../../../../../../packages/google-cloud-dialogflow-cx'); const sinon = require('sinon'); From 79ce8d562283caceb0436e64ee3d2b651223794f Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 18 Aug 2026 16:33:28 -0400 Subject: [PATCH 7/7] obscure the tests like we did for transcoding test --- .../packages/gax/test/unit/transcoding_validation.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/packages/gax/test/unit/transcoding_validation.ts b/core/packages/gax/test/unit/transcoding_validation.ts index 7d4a09e48266..86e8647eff41 100644 --- a/core/packages/gax/test/unit/transcoding_validation.ts +++ b/core/packages/gax/test/unit/transcoding_validation.ts @@ -86,13 +86,13 @@ describe('Dialogflow CX Fallback Transcoding and Path Traversal Prevention', () it('6. should protect against query parameter injection by percent-encoding "?" and "$" in the session ID', async () => { await client.initialize(); await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/my-session?$httpMethod=DELETE#', + session: 'projects/p/locations/l/agents/a/sessions/my-session?$foo=BAR#', queryInput: { text: { text: 'hello' }, languageCode: 'en' }, }); assert.strictEqual(fetchStub.callCount, 1); const requestUrl = fetchStub.firstCall.args[0]; // "?" -> %3F, "$" -> %24, "=" -> %3D, "#" -> %23 - assert.ok(requestUrl.includes('my-session%3F%24httpMethod%3DDELETE%23')); + assert.ok(requestUrl.includes('my-session%3F%24foo%3DBAR%23')); }); // Test 7: Combined Path Traversal and Query Parameter Injection @@ -103,24 +103,24 @@ describe('Dialogflow CX Fallback Transcoding and Path Traversal Prevention', () // example: POST https://-dialogflow.googleapis.com/v3/{session=projects/*/locations/*/agents/*/sessions/*}:detectIntent await client.initialize(); await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/..?$httpMethod=DELETE#', + session: 'projects/p/locations/l/agents/a/sessions/..?$foo=BAR#', queryInput: { text: { text: 'hello' }, languageCode: 'en' }, }); assert.strictEqual(fetchStub.callCount, 1); const requestUrl = fetchStub.firstCall.args[0]; - assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/..%3F%24httpMethod%3DDELETE%23:detectIntent')); + assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/..%3F%24foo%3DBAR%23:detectIntent')); }); // Test 8: Combined Path Traversal (.) and Query Parameter Injection it('8. should protect against path traversal and query injection by percent-encoding combined patterns using dot', async () => { await client.initialize(); await client.detectIntent({ - session: 'projects/p/locations/l/agents/a/sessions/.?$httpMethod=DELETE#', + session: 'projects/p/locations/l/agents/a/sessions/.?$foo=BAR#', queryInput: { text: { text: 'hello' }, languageCode: 'en' }, }); assert.strictEqual(fetchStub.callCount, 1); const requestUrl = fetchStub.firstCall.args[0]; - assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/.%3F%24httpMethod%3DDELETE%23:detectIntent')); + assert.ok(requestUrl.includes('/v3/projects/p/locations/l/agents/a/sessions/.%3F%24foo%3DBAR%23:detectIntent')); }); // Test 9: Percent-encoding all other characters