Skip to content

fix(common): prevent path traversal and parameter injection in REST clients #9152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
danieljbruce wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(common): prevent path traversal and parameter injection in REST clients #9152

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions core/common/src/service-object.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,17 +563,25 @@ class ServiceObject<T = any> extends EventEmitter {
const uriComponents = [this.baseUrl, this.id || '', reqOpts.uri];

if (isAbsoluteUrl) {
uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri));
const url = new URL(reqOpts.uri);
const encodedPath = util.encodeURIPath(url.pathname);
url.pathname = encodedPath;
let res = url.toString();
if (!reqOpts.uri.endsWith('/') && res.endsWith('/')) {
res = res.slice(0, -1);
}
reqOpts.uri = res;
} else {
Comment on lines 565 to +574

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

Applying util.encodeURIPath to url.pathname of an absolute URL will double-encode any already percent-encoded characters (e.g., %20 becomes %2520, %2F becomes %252F). Absolute URLs (such as those returned by the Google API backend for resumable uploads or media downloads) are already fully formed and encoded. Re-encoding them will break these requests. We should only apply the encoding and validation logic to non-absolute URLs.

    if (isAbsoluteUrl) {
      const trimSlashesRegex = /^\/*|\/*$/g;
      reqOpts.uri = reqOpts.uri.replace(trimSlashesRegex, '');
    } else {

reqOpts.uri = uriComponents
.filter(x => x!.trim()) // Limit to non-empty strings.
.map(uriComponent => {
const trimSlashesRegex = /^\/*|\/*$/g;
const trimmed = uriComponent!.replace(trimSlashesRegex, '');
return util.encodeURIPath(trimmed);
})
.join('/');
}

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_!,
);
Expand Down
32 changes: 20 additions & 12 deletions core/common/src/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,20 +212,28 @@ export class Service {
uriComponents.push(reqOpts.uri);

if (isAbsoluteUrl) {
uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri));
const url = new URL(reqOpts.uri);
const encodedPath = util.encodeURIPath(url.pathname);
url.pathname = encodedPath;
let res = url.toString();
if (!reqOpts.uri.endsWith('/') && res.endsWith('/')) {
res = res.slice(0, -1);
}
reqOpts.uri = res;
} else {
Comment on lines 214 to +223

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

Applying util.encodeURIPath to url.pathname of an absolute URL will double-encode any already percent-encoded characters (e.g., %20 becomes %2520, %2F becomes %252F). Absolute URLs (such as those returned by the Google API backend for resumable uploads or media downloads) are already fully formed and encoded. Re-encoding them will break these requests. We should only apply the encoding and validation logic to non-absolute URLs.

    if (isAbsoluteUrl) {
      const trimSlashesRegex = /^\/*|\/*$/g;
      reqOpts.uri = reqOpts.uri
        .replace(trimSlashesRegex, '')
        .replace(/\/:/g, ':');
    } else {

reqOpts.uri = uriComponents
.map(uriComponent => {
const trimSlashesRegex = /^\/*|\/*$/g;
const trimmed = uriComponent.replace(trimSlashesRegex, '');
return util.encodeURIPath(trimmed);
})
.join('/')
// 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(
Expand Down
49 changes: 49 additions & 0 deletions core/common/src/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,10 @@ export class Util {
return dup;
}

encodeWithSlashes = encodeWithSlashes;
encodeWithoutSlashes = encodeWithoutSlashes;
encodeURIPath = encodeURIPath;

/**
* Decorate the options about to be made in a request.
*
Expand Down Expand Up @@ -1024,5 +1028,50 @@ class ProgressStream extends Transform {
}
}

export function encodeWithSlashes(str: string, propertyName = 'resource ID field'): string {
const segments = str.split('/');
for (const segment of segments) {
if (segment === '.' || segment === '..') {
throw new Error(
`Value for ${propertyName} must not contain segments that are exactly . or .. .`,
);
}
}
return encodeURIComponent(str)
.replace(/%2F/gi, '/')
.replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase());
}

export function encodeWithoutSlashes(str: string, propertyName = 'resource ID field'): string {
if (str === '.' || str === '..') {
throw new Error(`Invalid value ${str} for ${propertyName}.`);
}
return encodeURIComponent(str)
.replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase());
}

export function encodeURIPath(uri: string): string {
const parts = uri.split('/');
return parts
.map(part => {
if (part === '') {
return '';
}
if (part.includes(':')) {
const subparts = part.split(':');
return subparts
.map(subpart => {
if (subpart === '') {
return '';
}
return encodeWithoutSlashes(subpart, 'path segment');
})
.join(':');
}
return encodeWithoutSlashes(part, 'path segment');
})
.join('/');
}

const util = new Util();
export {util};
52 changes: 52 additions & 0 deletions core/common/test/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1903,6 +1903,58 @@ describe('common/util', () => {
});
});

describe('encodeWithSlashes & encodeWithoutSlashes', () => {
it('encodeWithSlashes should allow valid path segments and encode special characters', () => {
assert.strictEqual(
util.encodeWithSlashes('foo/bar-123_~.baz'),
'foo/bar-123_~.baz',
);
assert.strictEqual(
util.encodeWithSlashes('foo/bar baz'),
'foo/bar%20baz',
);
});

it('encodeWithSlashes should throw if any segment is . or ..', () => {
assert.throws(() => {
util.encodeWithSlashes('foo/./bar', 'testField');
}, /Value for testField must not contain segments that are exactly \. or \.\. \./);

assert.throws(() => {
util.encodeWithSlashes('foo/../bar', 'testField');
}, /Value for testField must not contain segments that are exactly \. or \.\. \./);
});

it('encodeWithoutSlashes should allow valid characters and encode slashes and special characters', () => {
assert.strictEqual(
util.encodeWithoutSlashes('foo-123_~.baz'),
'foo-123_~.baz',
);
assert.strictEqual(
util.encodeWithoutSlashes('foo/bar'),
'foo%2Fbar',
);
assert.strictEqual(
util.encodeWithoutSlashes('photo_😀.png'),
'photo_%F0%9F%98%80.png',
);
assert.strictEqual(
util.encodeWithoutSlashes('test*file!'),
'test%2Afile%21',
);
});

it('encodeWithoutSlashes should throw if the value is . or ..', () => {
assert.throws(() => {
util.encodeWithoutSlashes('.', 'testField');
}, /Invalid value \. for testField\./);

assert.throws(() => {
util.encodeWithoutSlashes('..', 'testField');
}, /Invalid value \.\. for testField\./);
});
});

describe('maybeOptionsOrCallback', () => {
it('should allow passing just a callback', () => {
const optionsOrCallback = () => {};
Expand Down
26 changes: 17 additions & 9 deletions handwritten/storage/src/nodejs-common/service-object.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,17 +562,25 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
const uriComponents = [this.baseUrl, this.id || '', reqOpts.uri];

if (isAbsoluteUrl) {
uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri));
const url = new URL(reqOpts.uri);
const encodedPath = util.encodeURIPath(url.pathname);
url.pathname = encodedPath;
let res = url.toString();
if (!reqOpts.uri.endsWith('/') && res.endsWith('/')) {
res = res.slice(0, -1);
}
reqOpts.uri = res;
} else {
Comment on lines 564 to +573

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

Applying util.encodeURIPath to url.pathname of an absolute URL will double-encode any already percent-encoded characters (e.g., %20 becomes %2520, %2F becomes %252F). Absolute URLs (such as those returned by the Google API backend for resumable uploads or media downloads) are already fully formed and encoded. Re-encoding them will break these requests. We should only apply the encoding and validation logic to non-absolute URLs.

    if (isAbsoluteUrl) {
      const trimSlashesRegex = /^\/*|\/*$/g;
      reqOpts.uri = reqOpts.uri.replace(trimSlashesRegex, '');
    } else {

reqOpts.uri = uriComponents
.filter(x => x!.trim()) // Limit to non-empty strings.
.map(uriComponent => {
const trimSlashesRegex = /^\/*|\/*$/g;
const trimmed = uriComponent!.replace(trimSlashesRegex, '');
return util.encodeURIPath(trimmed);
})
.join('/');
}

reqOpts.uri = uriComponents
.filter(x => x!.trim()) // Limit to non-empty strings.
.map(uriComponent => {
const trimSlashesRegex = /^\/*|\/*$/g;
return uriComponent!.replace(trimSlashesRegex, '');
})
.join('/');

const childInterceptors = Array.isArray(reqOpts.interceptors_)
? reqOpts.interceptors_
: [];
Expand Down
32 changes: 20 additions & 12 deletions handwritten/storage/src/nodejs-common/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,20 +234,28 @@ export class Service {
uriComponents.push(reqOpts.uri);

if (isAbsoluteUrl) {
uriComponents.splice(0, uriComponents.indexOf(reqOpts.uri));
const url = new URL(reqOpts.uri);
const encodedPath = util.encodeURIPath(url.pathname);
url.pathname = encodedPath;
let res = url.toString();
if (!reqOpts.uri.endsWith('/') && res.endsWith('/')) {
res = res.slice(0, -1);
}
reqOpts.uri = res;
} else {
Comment on lines 236 to +245

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

Applying util.encodeURIPath to url.pathname of an absolute URL will double-encode any already percent-encoded characters (e.g., %20 becomes %2520, %2F becomes %252F). Absolute URLs (such as those returned by the Google API backend for resumable uploads or media downloads) are already fully formed and encoded. Re-encoding them will break these requests. We should only apply the encoding and validation logic to non-absolute URLs.

    if (isAbsoluteUrl) {
      const trimSlashesRegex = /^\/*|\/*$/g;
      reqOpts.uri = reqOpts.uri
        .replace(trimSlashesRegex, '')
        .replace(/\/:/g, ':');
    } else {

reqOpts.uri = uriComponents
.map(uriComponent => {
const trimSlashesRegex = /^\/*|\/*$/g;
const trimmed = uriComponent.replace(trimSlashesRegex, '');
return util.encodeURIPath(trimmed);
})
.join('/')
// 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();
const interceptorArray = Array.isArray(reqOpts.interceptors_)
? reqOpts.interceptors_
Expand Down
49 changes: 49 additions & 0 deletions handwritten/storage/src/nodejs-common/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,10 @@ export class Util {
: [optionsOrCallback as T, cb as C];
}

encodeWithSlashes = encodeWithSlashes;
encodeWithoutSlashes = encodeWithoutSlashes;
encodeURIPath = encodeURIPath;

_getDefaultHeaders(gcclGcsCmd?: string) {
const headers = {
'User-Agent': getUserAgentString(),
Expand Down Expand Up @@ -1072,5 +1076,50 @@ class ProgressStream extends Transform {
}
}

export function encodeWithSlashes(str: string, propertyName = 'resource ID field'): string {
const segments = str.split('/');
for (const segment of segments) {
if (segment === '.' || segment === '..') {
throw new Error(
`Value for ${propertyName} must not contain segments that are exactly . or .. .`,
);
}
}
return encodeURIComponent(str)
.replace(/%2F/gi, '/')
.replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase());
}

export function encodeWithoutSlashes(str: string, propertyName = 'resource ID field'): string {
if (str === '.' || str === '..') {
throw new Error(`Invalid value ${str} for ${propertyName}.`);
}
return encodeURIComponent(str)
.replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase());
}

export function encodeURIPath(uri: string): string {
const parts = uri.split('/');
return parts
.map(part => {
if (part === '') {
return '';
}
if (part.includes(':')) {
const subparts = part.split(':');
return subparts
.map(subpart => {
if (subpart === '') {
return '';
}
return encodeWithoutSlashes(subpart, 'path segment');
})
.join(':');
}
return encodeWithoutSlashes(part, 'path segment');
})
.join('/');
}

const util = new Util();
export {util};
22 changes: 22 additions & 0 deletions handwritten/storage/test/bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1878,6 +1878,28 @@ describe('Bucket', () => {
});
});

describe('security - URI encoding and path traversal protection', () => {
it('should throw error when bucket name or object path segment is dot or dot-dot', () => {
assert.throws(() => {
const invalidBucket = new Bucket(STORAGE, '..');
invalidBucket.getMetadata(assert.ifError);
}, /Invalid value \.\. for path segment\./);
});

it('should percent-encode query parameter injection payload in file name', done => {
const maliciousFileName = 'file_name?\\$httpMethod=DELETE#';
const file = bucket.file(maliciousFileName);

bucket.request = (reqOpts: DecorateRequestOptions) => {
assert(reqOpts.uri.includes('file_name%3F%24httpMethod%3DDELETE%23'));
assert(!reqOpts.uri.includes('?$httpMethod=DELETE#'));
done();
};

file.getMetadata(assert.ifError);
});
});

describe('file', () => {
const FILE_NAME = 'remote-file-name.jpg';
let file: FakeFile;
Expand Down
Loading