diff --git a/core/common/src/service-object.ts b/core/common/src/service-object.ts index 798193813f76..853e0bd96540 100644 --- a/core/common/src/service-object.ts +++ b/core/common/src/service-object.ts @@ -28,6 +28,8 @@ import { BodyResponseCallback, DecorateRequestOptions, ResponseBody, + encodeAbsoluteURI, + joinURIComponents, util, } from './util'; @@ -563,17 +565,18 @@ class ServiceObject extends EventEmitter { const uriComponents = [this.baseUrl, this.id || '', reqOpts.uri]; if (isAbsoluteUrl) { - uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri)); + // Encode only the pathname to preserve protocol, host, and query params. + // We cannot pass uriComponents through encodeURIPath after splicing + // because it will percent-encode parts we do not want to encode. + reqOpts.uri = encodeAbsoluteURI(reqOpts.uri); + } else { + // Relative path components contain only path segments (no protocol or host), + // so we encode each segment directly and join them with '/'. + reqOpts.uri = joinURIComponents( + uriComponents.filter(x => x!.trim()) as string[], + ); } - reqOpts.uri = uriComponents - .filter(x => x!.trim()) // Limit to non-empty strings. - .map(uriComponent => { - const trimSlashesRegex = /^\/*|\/*$/g; - return uriComponent!.replace(trimSlashesRegex, ''); - }) - .join('/'); - const childInterceptors = (arrify as unknown as (arg1: any) => [])( reqOpts.interceptors_!, ); diff --git a/core/common/src/service.ts b/core/common/src/service.ts index d0a179242467..3305d0374bdc 100644 --- a/core/common/src/service.ts +++ b/core/common/src/service.ts @@ -27,6 +27,8 @@ import { DecorateRequestOptions, MakeAuthenticatedRequest, PackageJson, + encodeAbsoluteURI, + joinURIComponents, util, } from './util'; @@ -212,20 +214,20 @@ export class Service { uriComponents.push(reqOpts.uri); if (isAbsoluteUrl) { - uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri)); + // Encode only the pathname to preserve protocol, host, and query params. + // We cannot pass uriComponents through encodeURIPath after splicing + // because it will percent-encode parts we do not want to encode. + reqOpts.uri = encodeAbsoluteURI(reqOpts.uri); + } else { + // Relative path components contain only path segments (no protocol or host), + // so we encode each segment directly and join them with '/'. + reqOpts.uri = joinURIComponents(uriComponents) + // Some URIs have colon separators. + // Bad: https://.../projects/:list + // Good: https://.../projects:list + .replace(/\/:/g, ':'); } - reqOpts.uri = uriComponents - .map(uriComponent => { - const trimSlashesRegex = /^\/*|\/*$/g; - return uriComponent.replace(trimSlashesRegex, ''); - }) - .join('/') - // Some URIs have colon separators. - // Bad: https://.../projects/:list - // Good: https://.../projects:list - .replace(/\/:/g, ':'); - const requestInterceptors = this.getRequestInterceptors(); (arrify as unknown as (arg1: any) => any[])(reqOpts.interceptors_!).forEach( diff --git a/core/common/src/util.ts b/core/common/src/util.ts index 322e6cfee37a..a9410f3b5495 100644 --- a/core/common/src/util.ts +++ b/core/common/src/util.ts @@ -1024,5 +1024,131 @@ class ProgressStream extends Transform { } } +// Validates a single path segment matched by a single wildcard (*). +// Checks that the segment is not exactly '.' or '..' (directory traversal indicators). +export 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). +export 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 ..`, + ); + } + } +} + +/** + * 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(), + ); +} + +/** + * 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('/'); +} + +/** + * Encodes each path segment in a URI string while preserving slash (`/`) and + * colon (`:`) delimiters, and validates that no path segment is `.` or `..` to + * prevent path traversal. + * + * @param {string} uri - The URI path to encode. + * @return {string} The encoded URI path. + */ +export function encodeURIPath(uri: string): string { + const processSegment = (segment: string): string => { + if (segment === '') { + return ''; + } + let decoded = segment; + try { + decoded = decodeURIComponent(segment); + } catch { + // Fallback to raw segment if decoding fails (e.g. malformed '%') + } + validateUriPathSegment('path segment', decoded); + return encodeWithSlashes(decoded); + }; + + const parts = uri.split('/'); + return parts + .map(part => { + if (part.includes(':')) { + return part.split(':').map(processSegment).join(':'); + } + return processSegment(part); + }) + .join('/'); +} + +/** + * Encodes the pathname of an absolute URI string using `encodeURIPath`, + * preserving any query parameters, hash, or trailing slash formatting. + * + * @param {string} uri - The absolute URI string to encode. + * @return {string} The formatted and encoded absolute URI string. + */ +export function encodeAbsoluteURI(uri: string): string { + const url = new URL(uri); // Isolate pathname from protocol, host, and query. + const encodedPath = encodeURIPath(url.pathname); + url.pathname = encodedPath; + let res = url.toString(); + if (!uri.endsWith('/') && res.endsWith('/')) { + res = res.slice(0, -1); + } + return res; +} + +/** + * Trims slashes, encodes path segments to prevent path traversal, and joins + * URI components into a single relative path. + * + * @param {string[]} components - URI components to encode and join. + * @return {string} The formatted and joined URI path. + */ +export function joinURIComponents(components: string[]): string { + return components + .map(uriComponent => { + const trimSlashesRegex = /^\/*|\/*$/g; + const trimmed = uriComponent.replace(trimSlashesRegex, ''); + return encodeURIPath(trimmed); // Encode and prevent path traversal. + }) + .join('/'); +} + const util = new Util(); export {util}; diff --git a/core/common/test/service-object.ts b/core/common/test/service-object.ts index 1d3ed35a5fee..edba3e8e31ec 100644 --- a/core/common/test/service-object.ts +++ b/core/common/test/service-object.ts @@ -1096,6 +1096,26 @@ describe('ServiceObject', () => { }); }); + it('should throw error when id or uri contains path traversal segments', () => { + serviceObject.id = '..'; + assert.throws(() => { + asInternal(serviceObject).request_(reqOpts, () => {}); + }, /Invalid value \.\. for path segment/); + }); + + it('should percent-encode query parameter injection payloads in path components', done => { + const maliciousId = 'table_name?param=value#tag'; + serviceObject.id = maliciousId; + serviceObject.parent.request = (reqOpts_, callback) => { + assert.strictEqual( + reqOpts_.uri, + `${serviceObject.baseUrl}/table_name%3Fparam%3Dvalue%23tag/${reqOpts.uri}`, + ); + callback(null, null, {} as r.Response); + }; + asInternal(serviceObject).request_(reqOpts, () => done()); + }); + it('should extend interceptors from child ServiceObjects', async () => { const parent = new ServiceObject(CONFIG) as FakeServiceObject; parent.interceptors.push({ @@ -1235,4 +1255,57 @@ describe('ServiceObject', () => { serviceObject.requestStream(fakeOptions); }); }); + + // Temporary test suite pulling in BigQuery to verify end-to-end path traversal + // protection and URI encoding with the local @google-cloud/common implementation. + // Note: We will delete these tests after the corresponding tests in BigQuery + // (handwritten/bigquery/test/dataset.ts) are unskipped upon the release of @google-cloud/common. + describe('BigQuery Dataset integration (security - URI encoding and path traversal protection)', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let BigQueryDataset: any; + + before(() => { + BigQueryDataset = proxyquire( + '../../../../handwritten/bigquery/build/src/dataset', + { + '@google-cloud/common': { + ServiceObject, + util, + }, + }, + ).Dataset; + }); + + it('should throw error when dataset id or path segment is dot or dot-dot', () => { + const bigqueryMock = { + projectId: 'my-project', + request: util.noop, + }; + + assert.throws(() => { + const invalidDataset = new BigQueryDataset(bigqueryMock, '..'); + invalidDataset.getMetadata(assert.ifError); + }, /Invalid value \.\. for path segment/); + }); + + it('should percent-encode query parameter injection payload in table name', done => { + const bigqueryMock = { + projectId: 'my-project', + request: util.noop, + }; + const ds = new BigQueryDataset(bigqueryMock, 'kittens'); + const maliciousTableId = 'table_name?param=value#tag'; + const table = ds.table(maliciousTableId); + + ds.request = (reqOpts: DecorateRequestOptions) => { + assert.strictEqual( + reqOpts.uri, + 'tables/table_name%3Fparam%3Dvalue%23tag', + ); + done(); + }; + + table.getMetadata(assert.ifError); + }); + }); }); diff --git a/core/common/test/util.ts b/core/common/test/util.ts index 6c018afd4d3d..1a9468fa3de8 100644 --- a/core/common/test/util.ts +++ b/core/common/test/util.ts @@ -46,6 +46,13 @@ import { ParsedHttpRespMessage, ParsedHttpResponseBody, Util, + validateUriPathSegment, + validateUriPath, + encodeWithSlashes, + encodeWithoutSlashes, + encodeURIPath, + encodeAbsoluteURI, + joinURIComponents, } from '../src/util'; import {DEFAULT_PROJECT_ID_TOKEN} from '../src/service'; @@ -1903,6 +1910,119 @@ describe('common/util', () => { }); }); + describe('validateUriPathSegment & validateUriPath', () => { + it('validateUriPathSegment should throw if the value is . or ..', () => { + assert.throws(() => { + validateUriPathSegment('testField', '.'); + }, /Invalid value \. for testField/); + + assert.throws(() => { + validateUriPathSegment('testField', '..'); + }, /Invalid value \.\. for testField/); + }); + + it('validateUriPath should throw if any segment is . or ..', () => { + assert.throws(() => { + validateUriPath('testField', 'foo/./bar'); + }, /Value for testField must not contain segments that are exactly \. or \.\./); + + assert.throws(() => { + validateUriPath('testField', 'foo/../bar'); + }, /Value for testField must not contain segments that are exactly \. or \.\./); + }); + }); + + describe('encodeWithSlashes & encodeWithoutSlashes', () => { + it('encodeWithSlashes should percent-encode special characters and slashes', () => { + assert.strictEqual( + encodeWithSlashes('foo/bar'), + 'foo%2Fbar', + ); + assert.strictEqual( + encodeWithSlashes('abc-123_.~'), + 'abc-123_.~', + ); + assert.strictEqual( + encodeWithSlashes("!'()*"), + '%21%27%28%29%2A', + ); + assert.strictEqual( + encodeWithSlashes('photo_😀.png'), + 'photo_%F0%9F%98%80.png', + ); + }); + + it('encodeWithoutSlashes should preserve slashes and encode special characters', () => { + assert.strictEqual( + encodeWithoutSlashes('foo-123_~.baz'), + 'foo-123_~.baz', + ); + assert.strictEqual( + encodeWithoutSlashes('foo/bar'), + 'foo/bar', + ); + assert.strictEqual( + encodeWithoutSlashes('foo/bar baz'), + 'foo/bar%20baz', + ); + assert.strictEqual( + encodeWithoutSlashes('photo_😀.png'), + 'photo_%F0%9F%98%80.png', + ); + assert.strictEqual( + encodeWithoutSlashes('test*file!'), + 'test%2Afile%21', + ); + }); + }); + + describe('encodeAbsoluteURI', () => { + it('should handle absolute URLs with and without trailing slash', () => { + assert.strictEqual( + encodeAbsoluteURI('https://example.com/foo/bar'), + 'https://example.com/foo/bar', + ); + assert.strictEqual( + encodeAbsoluteURI('https://example.com/foo/bar/'), + 'https://example.com/foo/bar/', + ); + assert.strictEqual( + encodeAbsoluteURI('http://www.google.com'), + 'http://www.google.com', + ); + }); + + it('should encode path segments and handle colons in absolute URLs', () => { + assert.strictEqual( + encodeAbsoluteURI('https://example.com/projects:list'), + 'https://example.com/projects:list', + ); + assert.strictEqual( + encodeAbsoluteURI('https://example.com/foo/bar-123_~.baz'), + 'https://example.com/foo/bar-123_~.baz', + ); + }); + }); + + describe('joinURIComponents', () => { + it('should trim slashes and join components', () => { + assert.strictEqual( + joinURIComponents(['/base/', '/id/', '/path/']), + 'base/id/path', + ); + }); + + it('should encode special characters and prevent path traversal in components', () => { + assert.strictEqual( + joinURIComponents(['datasets', 'my dataset', 'tables']), + 'datasets/my%20dataset/tables', + ); + assert.throws(() => { + joinURIComponents(['datasets', '..', 'tables']); + }); + }); + }); + describe('maybeOptionsOrCallback', () => { it('should allow passing just a callback', () => { const optionsOrCallback = () => {}; diff --git a/handwritten/bigquery/system-test/traversal.ts b/handwritten/bigquery/system-test/traversal.ts new file mode 100644 index 000000000000..6aec81758591 --- /dev/null +++ b/handwritten/bigquery/system-test/traversal.ts @@ -0,0 +1,146 @@ +// 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. + +import * as assert from 'assert'; +import {describe, it, before} from 'mocha'; +import * as proxyquire from 'proxyquire'; +import {GoogleAuth} from 'google-auth-library'; + +// TODO: Remove proxyquire and the local @google-cloud/common injection below +// after the new version of @google-cloud/common is released to npm and bumped in package.json. +// Once released, standard `import {BigQuery} from '../src'` can be used directly. + +// Load the local build of @google-cloud/common from this branch +// eslint-disable-next-line @typescript-eslint/no-var-requires +const common = require('../../../../core/common/build/src'); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +let BigQuery: any; + +describe('BigQuery URI path handling and traversal', () => { + before(() => { + // Inject the local common build prototypes and utilities + // eslint-disable-next-line @typescript-eslint/no-var-requires + const oldCommon = require('@google-cloud/common'); + // eslint-disable-next-line @typescript-eslint/no-var-requires + const oldUtil = require('@google-cloud/common/build/src/util'); + Object.assign(oldCommon.ServiceObject.prototype, common.ServiceObject.prototype); + Object.assign(oldCommon.Service.prototype, common.Service.prototype); + Object.assign(oldCommon.util, common.util); + Object.assign(oldUtil, common.util); + + BigQuery = proxyquire('../src', { + '@google-cloud/common': common, + }).BigQuery; + }); + + const fakeAuthClient = Object.assign(new GoogleAuth(), { + getCredentials: async () => ({}), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + authorizeRequest: async (reqOpts: any) => reqOpts, + getProjectId: async () => 'test-project', + }); + + const testCases = [ + { + description: 'should reject dot segment (.)', + datasetId: '.', + expectedError: /Invalid value \. for path segment/, + }, + { + description: 'should reject dot-dot segment (..)', + datasetId: '..', + expectedError: /Invalid value \.\. for path segment/, + }, + { + description: + 'should reject percent-encoded dot (period . encoded as %2e)', + datasetId: '%2e', + expectedError: /Invalid value \. for path segment/, + }, + { + description: + 'should reject uppercase percent-encoded dot (period . encoded as %2E)', + datasetId: '%2E', + expectedError: /Invalid value \. for path segment/, + }, + { + description: + 'should reject percent-encoded dot-dot (.. encoded as %2e%2e)', + datasetId: '%2e%2e', + expectedError: /Invalid value \.\. for path segment/, + }, + { + description: + 'should reject uppercase percent-encoded dot-dot (.. encoded as %2E%2E)', + datasetId: '%2E%2E', + expectedError: /Invalid value \.\. for path segment/, + }, + { + description: 'should reject paths containing dot-dot segment (foo/../bar)', + datasetId: 'foo/../bar', + expectedError: /Invalid value \.\. for path segment/, + }, + { + description: + 'should attempt request and encode query parameter (?) and fragment (#) characters', + datasetId: 'dataset_name?param=value#tag', + expectedError: + /Not found: Dataset.*datasets\/dataset_name%3Fparam%3Dvalue%23tag/, + }, + { + description: + 'should attempt request and preserve pre-encoded space (space encoded as %20) without double encoding', + datasetId: 'my%20dataset', + expectedError: /Not found: Dataset.*datasets\/my%20dataset/, + }, + { + description: 'should attempt request for standard dataset name', + datasetId: 'valid_dataset_123', + expectedError: /Not found: Dataset.*datasets\/valid_dataset_123/, + }, + ]; + + for (const {description, datasetId, expectedError} of testCases) { + it(description, async () => { + const bigquery = new BigQuery({ + projectId: 'test-project', + authClient: fakeAuthClient, + }); + + // Mock makeAuthenticatedRequest to return a 404 error containing the requested URI + bigquery.makeAuthenticatedRequest = ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + reqOpts: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + callback?: any, + ) => { + const notFoundError = new common.ApiError({ + message: `Not found: Dataset ${reqOpts.uri}`, + code: 404, + }); + if (typeof callback === 'function') { + callback(notFoundError, null, null); + } + return undefined; + }; + + const dataset = bigquery.dataset(datasetId); + + await assert.rejects(async () => { + await dataset.getMetadata(); + }, expectedError); + }); + } +});