From eec989039d51909492f0bb1c6e590931dd9ea00a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Wed, 26 Aug 2026 08:59:09 +0200 Subject: [PATCH 1/5] fix: prevent path traversal by sanitizing URLs --- src/base/api_client.ts | 16 +++---- src/resource_clients/key_value_store.ts | 10 ++-- src/resource_clients/request_queue.ts | 10 ++-- src/utils.ts | 19 ++++++++ test/url_path_encoding.test.ts | 63 +++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 test/url_path_encoding.test.ts diff --git a/src/base/api_client.ts b/src/base/api_client.ts index 1013f361c..813bfc091 100644 --- a/src/base/api_client.ts +++ b/src/base/api_client.ts @@ -1,6 +1,7 @@ import type { ApifyClient } from '../apify_client'; import type { HttpClient } from '../http_client'; import type { PaginatedResponse, PaginationOptions } from '../utils'; +import { toPath, toPathSegment } from '../utils'; /** @private */ export interface ApiClientOptions { @@ -50,7 +51,7 @@ export abstract class ApiClient { this.baseUrl = baseUrl; this.publicBaseUrl = publicBaseUrl; this.resourcePath = resourcePath; - this.url = id ? `${baseUrl}/${resourcePath}/${this.safeId}` : `${baseUrl}/${resourcePath}`; + this.url = id ? `${baseUrl}/${resourcePath}/${toPathSegment(this.safeId!)}` : `${baseUrl}/${resourcePath}`; this.apifyClient = apifyClient; this.httpClient = httpClient; this.params = params; @@ -67,15 +68,15 @@ export abstract class ApiClient { return { ...baseOptions, ...moreOptions } as BaseOptions & T; } - protected _url(path?: string): string { - return path ? `${this.url}/${path}` : this.url; + protected _url(path?: string | string[]): string { + return path ? `${this.url}/${toPath(path)}` : this.url; } - protected _publicUrl(path?: string): string { + protected _publicUrl(path?: string | string[]): string { const url = this.id - ? `${this.publicBaseUrl}/${this.resourcePath}/${this.safeId}` + ? `${this.publicBaseUrl}/${this.resourcePath}/${toPathSegment(this.safeId!)}` : `${this.publicBaseUrl}/${this.resourcePath}`; - return path ? `${url}/${path}` : url; + return path ? `${url}/${toPath(path)}` : url; } protected _params(endpointParams?: T): Record { @@ -83,8 +84,7 @@ export abstract class ApiClient { } protected _toSafeId(id: string): string { - // The id has the format `username/actor-name`, so we only need to replace the first `/`. - return id.replace('/', '~'); + return id.replaceAll('/', '~'); } /** diff --git a/src/resource_clients/key_value_store.ts b/src/resource_clients/key_value_store.ts index 71e09a138..e32b0f3b9 100644 --- a/src/resource_clients/key_value_store.ts +++ b/src/resource_clients/key_value_store.ts @@ -216,7 +216,7 @@ export class KeyValueStoreClient extends ResourceClient { const store = await this.get(); - const recordPublicUrl = new URL(this._publicUrl(`records/${key}`)); + const recordPublicUrl = new URL(this._publicUrl(['records', key])); if (store?.urlSigningSecretKey) { const signature = await createHmacSignatureAsync(store.urlSigningSecretKey, key); @@ -301,7 +301,7 @@ export class KeyValueStoreClient extends ResourceClient { */ async recordExists(key: string): Promise { const requestOpts: Record = { - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'HEAD', params: this._params(), }; @@ -363,7 +363,7 @@ export class KeyValueStoreClient extends ResourceClient { if (options.signature) queryParams.signature = options.signature; const requestOpts: Record = { - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'GET', params: this._params(queryParams), timeout: DEFAULT_TIMEOUT_MILLIS, @@ -474,7 +474,7 @@ export class KeyValueStoreClient extends ResourceClient { } const uploadOpts: ApifyRequestConfig = { - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'PUT', params: this._params(), data: value, @@ -501,7 +501,7 @@ export class KeyValueStoreClient extends ResourceClient { ow(key, ow.string); await this.httpClient.call({ - url: this._url(`records/${key}`), + url: this._url(['records', key]), method: 'DELETE', params: this._params(), timeout: SMALL_TIMEOUT_MILLIS, diff --git a/src/resource_clients/request_queue.ts b/src/resource_clients/request_queue.ts index f125009cf..66f0bd1a5 100644 --- a/src/resource_clients/request_queue.ts +++ b/src/resource_clients/request_queue.ts @@ -517,7 +517,7 @@ export class RequestQueueClient extends ResourceClient { async getRequest(id: string): Promise { ow(id, ow.string); const requestOpts: ApifyRequestConfig = { - url: this._url(`requests/${id}`), + url: this._url(['requests', id]), method: 'GET', timeout: Math.min(SMALL_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params(), @@ -559,7 +559,7 @@ export class RequestQueueClient extends ResourceClient { ); const response = await this.httpClient.call({ - url: this._url(`requests/${request.id}`), + url: this._url(['requests', request.id]), method: 'PUT', timeout: Math.min(MEDIUM_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), data: request, @@ -581,7 +581,7 @@ export class RequestQueueClient extends ResourceClient { ow(id, ow.string); await this.httpClient.call({ - url: this._url(`requests/${id}`), + url: this._url(['requests', id]), method: 'DELETE', timeout: Math.min(SMALL_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params({ @@ -629,7 +629,7 @@ export class RequestQueueClient extends ResourceClient { ); const response = await this.httpClient.call({ - url: this._url(`requests/${id}/lock`), + url: this._url(['requests', id, 'lock']), method: 'PUT', timeout: Math.min(MEDIUM_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params({ @@ -663,7 +663,7 @@ export class RequestQueueClient extends ResourceClient { ); await this.httpClient.call({ - url: this._url(`requests/${id}/lock`), + url: this._url(['requests', id, 'lock']), method: 'DELETE', timeout: Math.min(SMALL_TIMEOUT_MILLIS, this.timeoutMillis ?? Infinity), params: this._params({ diff --git a/src/utils.ts b/src/utils.ts index 260e2c4a9..272aa6873 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -408,3 +408,22 @@ export const mutuallyExclusive = message: `At most one of the following fields is allowed: ${keys.join(', ')}`, }; }; + +/** + * Percent-encodes a caller-supplied URL path segment so it cannot restructure the request path. + * + * Empty strings and dot segments are rejected instead of encoded, because URL parsers resolve + * dot segments after decoding - `records/%2E%2E` collapses to the parent endpoint just like `records/..`. + */ +export function toPathSegment(value: string): string { + ow(value, 'pathSegment', ow.string.nonEmpty.not.oneOf(['.', '..'])); + return encodeURIComponent(value); +} + +/** + * Builds a URL path from segments. A plain string is used as-is, so literal paths such as + * `requests/batch` keep their separators; an array has each of its segments encoded individually. + */ +export function toPath(path: string | string[]): string { + return Array.isArray(path) ? path.map(toPathSegment).join('/') : path; +} diff --git a/test/url_path_encoding.test.ts b/test/url_path_encoding.test.ts new file mode 100644 index 000000000..8bcdccd1b --- /dev/null +++ b/test/url_path_encoding.test.ts @@ -0,0 +1,63 @@ +import http from 'node:http'; +import type { AddressInfo } from 'node:net'; + +import { ApifyClient } from 'apify-client'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +describe('URL path encoding', () => { + let server: http.Server; + let client: ApifyClient; + let lastPath: string | undefined; + + beforeAll(async () => { + server = http.createServer((req, res) => { + lastPath = req.url; + res.setHeader('content-type', 'application/json'); + res.end(JSON.stringify({ data: {} })); + }); + await new Promise((resolve) => server.listen(0, resolve)); + client = new ApifyClient({ + baseUrl: `http://localhost:${(server.address() as AddressInfo).port}`, + token: 'token', + maxRetries: 0, + }); + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(resolve)); + }); + + it('encodes record keys instead of letting them restructure the path', async () => { + await client.keyValueStore('STORE').getRecord('../../actor-runs/VICTIM/abort'); + expect(lastPath).toBe( + '/v2/key-value-stores/STORE/records/..%2F..%2Factor-runs%2FVICTIM%2Fabort?attachment=true', + ); + + await client.keyValueStore('STORE').getRecord('foo?injected=1'); + expect(lastPath).toBe('/v2/key-value-stores/STORE/records/foo%3Finjected%3D1?attachment=true'); + + await client.keyValueStore('STORE').getRecord('foo#frag'); + expect(lastPath).toBe('/v2/key-value-stores/STORE/records/foo%23frag?attachment=true'); + }); + + it('encodes request ids', async () => { + await client.requestQueue('QUEUE').getRequest('../../../datasets/V/items'); + expect(lastPath).toBe('/v2/request-queues/QUEUE/requests/..%2F..%2F..%2Fdatasets%2FV%2Fitems'); + }); + + it('keeps literal path separators', async () => { + await client.requestQueue('QUEUE').prolongRequestLock('abc', { lockSecs: 1 }); + expect(lastPath).toBe('/v2/request-queues/QUEUE/requests/abc/lock?lockSecs=1'); + }); + + it('replaces every slash in a resource id', async () => { + await client.actor('user/name/extra').get(); + expect(lastPath).toBe('/v2/actors/user~name~extra'); + }); + + it('rejects dot segments and empty values', async () => { + await expect(client.keyValueStore('STORE').getRecord('..')).rejects.toThrow(/not be one of/); + await expect(client.requestQueue('QUEUE').getRequest('.')).rejects.toThrow(/not be one of/); + expect(() => client.dataset('..')).toThrow(/not be one of/); + }); +}); From 1f29cf8acd02b297d43ca894039adf4905e4e032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Wed, 26 Aug 2026 09:32:38 +0200 Subject: [PATCH 2/5] chore: regenerate API report --- docs/public-api/apify-client.api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/public-api/apify-client.api.md b/docs/public-api/apify-client.api.md index 5cbc2ddfe..c9e67222b 100644 --- a/docs/public-api/apify-client.api.md +++ b/docs/public-api/apify-client.api.md @@ -611,7 +611,7 @@ abstract class ApiClient { protected _params(endpointParams?: T): Record; publicBaseUrl: string; // (undocumented) - protected _publicUrl(path?: string): string; + protected _publicUrl(path?: string | string[]): string; // (undocumented) resourcePath: string; // (undocumented) @@ -623,7 +623,7 @@ abstract class ApiClient { // (undocumented) url: string; // (undocumented) - protected _url(path?: string): string; + protected _url(path?: string | string[]): string; } // Not exported by the entry point; reachable only as a referenced type. From c1c02323019b925c9ae9bd4ef9cc8b294cad443b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Thu, 27 Aug 2026 11:13:22 +0200 Subject: [PATCH 3/5] chore: use zod for validation on v3 --- src/utils.ts | 9 +++++++-- test/url_path_encoding.test.ts | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 23c43665e..5cbc2b90c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -473,6 +473,11 @@ export const mutuallyExclusive = (...keys: (keyof T & string)[ `At most one of the following fields is allowed: ${keys.join(', ')}`, ]; +const pathSegmentSchema = z + .string() + .nonempty() + .refine((v) => v !== '.' && v !== '..', { error: 'URL path must not contain dot segments' }); + /** * Percent-encodes a caller-supplied URL path segment so it cannot restructure the request path. * @@ -480,8 +485,8 @@ export const mutuallyExclusive = (...keys: (keyof T & string)[ * dot segments after decoding - `records/%2E%2E` collapses to the parent endpoint just like `records/..`. */ export function toPathSegment(value: string): string { - ow(value, 'pathSegment', ow.string.nonEmpty.not.oneOf(['.', '..'])); - return encodeURIComponent(value); + const valueParsed = parseArgument(value, pathSegmentSchema); + return encodeURIComponent(valueParsed); } /** diff --git a/test/url_path_encoding.test.ts b/test/url_path_encoding.test.ts index 8bcdccd1b..4f1b6bb34 100644 --- a/test/url_path_encoding.test.ts +++ b/test/url_path_encoding.test.ts @@ -56,8 +56,8 @@ describe('URL path encoding', () => { }); it('rejects dot segments and empty values', async () => { - await expect(client.keyValueStore('STORE').getRecord('..')).rejects.toThrow(/not be one of/); - await expect(client.requestQueue('QUEUE').getRequest('.')).rejects.toThrow(/not be one of/); - expect(() => client.dataset('..')).toThrow(/not be one of/); + await expect(client.keyValueStore('STORE').getRecord('..')).rejects.toThrow(/must not contain dot segments/); + await expect(client.requestQueue('QUEUE').getRequest('.')).rejects.toThrow(/must not contain dot segments/); + expect(() => client.dataset('..')).toThrow(/must not contain dot segments/); }); }); From 35dce7ff4a222e2b91a65f9991fcfb137c729387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Thu, 27 Aug 2026 11:24:16 +0200 Subject: [PATCH 4/5] chore: revert public API report --- docs/public-api/apify-client.api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/public-api/apify-client.api.md b/docs/public-api/apify-client.api.md index c9e67222b..5cbc2ddfe 100644 --- a/docs/public-api/apify-client.api.md +++ b/docs/public-api/apify-client.api.md @@ -611,7 +611,7 @@ abstract class ApiClient { protected _params(endpointParams?: T): Record; publicBaseUrl: string; // (undocumented) - protected _publicUrl(path?: string | string[]): string; + protected _publicUrl(path?: string): string; // (undocumented) resourcePath: string; // (undocumented) @@ -623,7 +623,7 @@ abstract class ApiClient { // (undocumented) url: string; // (undocumented) - protected _url(path?: string | string[]): string; + protected _url(path?: string): string; } // Not exported by the entry point; reachable only as a referenced type. From d8c40673677934bfcc8ec8ca513c5d9a145e6305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Thu, 27 Aug 2026 11:32:19 +0200 Subject: [PATCH 5/5] chore: regenerate API report --- docs/public-api/apify-client.api.md | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/public-api/apify-client.api.md b/docs/public-api/apify-client.api.md index 5cbc2ddfe..ce3769c33 100644 --- a/docs/public-api/apify-client.api.md +++ b/docs/public-api/apify-client.api.md @@ -27,6 +27,7 @@ import type { STORAGE_GENERAL_ACCESS } from '@apify/consts'; import { STORAGE_OWNERSHIP_FILTER } from '@apify/consts'; import type { ValueOf } from 'type-fest'; import type { WEBHOOK_EVENT_TYPES } from '@apify/consts'; +import type { z } from 'zod'; // @public export interface AccountAndUsageLimits { @@ -611,7 +612,7 @@ abstract class ApiClient { protected _params(endpointParams?: T): Record; publicBaseUrl: string; // (undocumented) - protected _publicUrl(path?: string): string; + protected _publicUrl(path?: string | string[]): string; // (undocumented) resourcePath: string; // (undocumented) @@ -623,7 +624,7 @@ abstract class ApiClient { // (undocumented) url: string; // (undocumented) - protected _url(path?: string): string; + protected _url(path?: string | string[]): string; } // Not exported by the entry point; reachable only as a referenced type. @@ -743,6 +744,12 @@ interface ApifyResponse extends AxiosResponse { config: ApifyRequestConfig & InternalAxiosRequestConfig; } +// @public +export class ArgumentValidationError extends Error { + constructor(error: z.ZodError, value: unknown, label?: string); + readonly issues: z.ZodError['issues']; +} + // @public (undocumented) export interface BaseActorVersion { // (undocumented) @@ -998,13 +1005,13 @@ export class DatasetClient = Record { // (undocumented) expiresInSecs?: number; } // @public -export interface DatasetClientDownloadItemsOptions extends DatasetClientListItemOptions { +export interface DatasetClientDownloadItemsOptions extends Omit { // (undocumented) attachment?: boolean; // (undocumented) @@ -1256,7 +1263,7 @@ export class InvalidResponseBodyError extends Error { } // @public -export interface KeyValueClientCreateKeysUrlOptions extends KeyValueClientListKeysOptions { +export interface KeyValueClientCreateKeysUrlOptions extends Omit { // (undocumented) expiresInSecs?: number; }