From d5928d48137bb52cfd61b7a363ed7e28defa9dac Mon Sep 17 00:00:00 2001 From: danieljbruce Date: Tue, 18 Aug 2026 14:35:47 -0400 Subject: [PATCH] =?UTF-8?q?Revert=20"fix:=20validate=20path=20parameters?= =?UTF-8?q?=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')); - }); -});