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..aa6ec8812163 100644 --- a/core/common/src/util.ts +++ b/core/common/src/util.ts @@ -1024,5 +1024,149 @@ 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). + * + * This method is a replica of the method found in Google GAX (google-gax). + * + * @param {string} propertyName - The name of the property being validated. + * @param {string} value - The segment value to validate. + */ +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). + * + * This method is a replica of the method found in Google GAX (google-gax). + * + * @param {string} propertyName - The name of the property being validated. + * @param {string} value - The path value to validate. + */ +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. + * + * This method is a replica of the method found in Google GAX (google-gax). + * + * @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. + * + * This method is a replica of the method found in Google GAX (google-gax). + * + * @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/traversal.ts b/core/common/test/traversal.ts new file mode 100644 index 000000000000..1fd7816ee730 --- /dev/null +++ b/core/common/test/traversal.ts @@ -0,0 +1,167 @@ +// 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. + +// TODO: Delete this test suite after the traversal tests in +// handwritten/bigquery/system-test/traversal.ts are unskipped. + +import * as assert from 'assert'; +import {describe, it} from 'mocha'; +import * as r from 'teeny-request'; + +import {Service, ServiceObject} from '../src'; +import { + ApiError, + BodyResponseCallback, + DecorateRequestOptions, + MakeAuthenticatedRequest, +} from '../src/util'; + +describe('URI path handling and traversal (ServiceObject & Service)', () => { + 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/, + }, + { + description: + 'should handle colon-separated segments without double decoding', + datasetId: 'domain.com:custom_dataset%20name', + expectedError: + /Not found: Dataset.*datasets\/domain\.com:custom_dataset%20name/, + }, + { + description: + 'should preserve literal percent-encoded sequences in colon-separated segments without decoding twice', + datasetId: 'project:item%2520name', + expectedError: + /Not found: Dataset.*datasets\/project:item%2520name/, + }, + ]; + + for (const {description, datasetId, expectedError} of testCases) { + it(description, async () => { + // Test ServiceObject path handling + const fakeParent = { + interceptors: [], + getRequestInterceptors: () => [], + requestStream: () => ({} as r.Request), + request: ( + reqOpts: DecorateRequestOptions, + callback: BodyResponseCallback, + ) => { + const notFoundError = new ApiError({ + message: `Not found: Dataset ${reqOpts.uri}`, + code: 404, + response: {} as r.Response, + }); + callback(notFoundError, null, {} as r.Response); + }, + }; + + const serviceObject = new ServiceObject({ + parent: fakeParent as unknown as Service, + baseUrl: 'datasets', + id: datasetId, + }); + + await assert.rejects(async () => { + await serviceObject.getMetadata(); + }, expectedError); + + // Test Service path handling + const service = new Service({ + scopes: [], + baseUrl: 'datasets', + projectIdRequired: false, + apiEndpoint: 'datasets', + packageJson: {name: 'test', version: '1.0.0'}, + }); + service.makeAuthenticatedRequest = (( + reqOpts: DecorateRequestOptions, + callback?: BodyResponseCallback, + ) => { + const notFoundError = new ApiError({ + message: `Not found: Dataset ${reqOpts.uri}`, + code: 404, + response: {} as r.Response, + }); + if (typeof callback === 'function') { + callback(notFoundError, null, {} as r.Response); + } + }) as unknown as MakeAuthenticatedRequest; + + await assert.rejects(async () => { + await new Promise((resolve, reject) => { + service.request({uri: datasetId}, (err, resp) => { + if (err) return reject(err); + resolve(resp); + }); + }); + }, expectedError); + }); + } +}); diff --git a/handwritten/bigquery/system-test/traversal.ts b/handwritten/bigquery/system-test/traversal.ts new file mode 100644 index 000000000000..b8a882d3cb94 --- /dev/null +++ b/handwritten/bigquery/system-test/traversal.ts @@ -0,0 +1,134 @@ +// 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} from 'mocha'; +import {GoogleAuth} from 'google-auth-library'; +import {ApiError} from '@google-cloud/common'; +import {BigQuery} from '../src'; + +describe.skip('BigQuery URI path handling and traversal', () => { + + 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/, + }, + { + description: + 'should handle colon-separated segments without double decoding', + datasetId: 'domain.com:custom_dataset%20name', + expectedError: + /Not found: Dataset.*datasets\/domain\.com:custom_dataset%20name/, + }, + { + description: + 'should preserve literal percent-encoded sequences in colon-separated segments without decoding twice', + datasetId: 'project:item%2520name', + expectedError: + /Not found: Dataset.*datasets\/project:item%2520name/, + }, + ]; + + for (const {description, datasetId, expectedError} of testCases) { + it(description, async () => { + const bigquery = new BigQuery({ + projectId: 'test-project', + authClient: fakeAuthClient as any, + }); + + // Mock makeAuthenticatedRequest to return a 404 error containing the requested URI + (bigquery as any).makeAuthenticatedRequest = ( + reqOpts: any, + callback?: any, + ) => { + const notFoundError = new ApiError({ + message: `Not found: Dataset ${reqOpts.uri}`, + code: 404, + response: {} as any, + }); + if (typeof callback === 'function') { + callback(notFoundError, null, null); + } + return undefined; + }; + + const dataset = bigquery.dataset(datasetId); + + await assert.rejects(async () => { + await dataset.getMetadata(); + }, expectedError); + }); + } +});