fix: prevent path traversal by sanitizing URLs

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<T>(endpointParams?: T): Record<string, unknown>;
     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<T = any> extends AxiosResponse<T> {
     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<SourceType extends ActorSourceType> {
     // (undocumented)
@@ -998,13 +1005,13 @@ export class DatasetClient<Data extends Record<string | number, any> = Record<st
 }
 
 // @public
-export interface DatasetClientCreateItemsUrlOptions extends DatasetClientListItemOptions {
+export interface DatasetClientCreateItemsUrlOptions extends Omit<DatasetClientListItemOptions, 'chunkSize' | 'signature'> {
     // (undocumented)
     expiresInSecs?: number;
 }
 
 // @public
-export interface DatasetClientDownloadItemsOptions extends DatasetClientListItemOptions {
+export interface DatasetClientDownloadItemsOptions extends Omit<DatasetClientListItemOptions, 'chunkSize'> {
     // (undocumented)
     attachment?: boolean;
     // (undocumented)
@@ -1256,7 +1263,7 @@ export class InvalidResponseBodyError extends Error {
 }
 
 // @public
-export interface KeyValueClientCreateKeysUrlOptions extends KeyValueClientListKeysOptions {
+export interface KeyValueClientCreateKeysUrlOptions extends Omit<KeyValueClientListKeysOptions, 'signature'> {
     // (undocumented)
     expiresInSecs?: number;
 }

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,24 +68,23 @@ 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<T>(endpointParams?: T): Record<string, unknown> {
         return { ...this.params, ...endpointParams };
     }
 
     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('/', '~');
     }
 
     /**

src/resource_clients/key_value_store.ts

@@ -254,7 +254,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);
@@ -330,7 +330,7 @@ export class KeyValueStoreClient extends ResourceClient {
      */
     async recordExists(key: string): Promise<boolean> {
         const requestOpts: Record<string, unknown> = {
-            url: this._url(`records/${key}`),
+            url: this._url(['records', key]),
             method: 'HEAD',
             params: this._params(),
         };
@@ -384,7 +384,7 @@ export class KeyValueStoreClient extends ResourceClient {
         if (parsed.signature) queryParams.signature = parsed.signature;
 
         const requestOpts: Record<string, unknown> = {
-            url: this._url(`records/${key}`),
+            url: this._url(['records', key]),
             method: 'GET',
             params: this._params(queryParams),
             timeout: DEFAULT_TIMEOUT_MILLIS,
@@ -481,7 +481,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,
@@ -508,7 +508,7 @@ export class KeyValueStoreClient extends ResourceClient {
         parseArgument(key, keySchema);
 
         await this.httpClient.call({
-            url: this._url(`records/${key}`),
+            url: this._url(['records', key]),
             method: 'DELETE',
             params: this._params(),
             timeout: SMALL_TIMEOUT_MILLIS,

src/resource_clients/request_queue.ts

@@ -518,7 +518,7 @@ export class RequestQueueClient extends ResourceClient {
     async getRequest(id: string): Promise<RequestQueueClientGetRequestResult | undefined> {
         parseArgument(id, requestIdSchema);
         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(),
@@ -549,7 +549,7 @@ export class RequestQueueClient extends ResourceClient {
         const parsed = parseArgument(options, forefrontOptionsSchema, 'RequestQueueClientAddRequestOptions');
 
         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,
@@ -571,7 +571,7 @@ export class RequestQueueClient extends ResourceClient {
         parseArgument(id, requestIdSchema);
 
         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({
@@ -617,7 +617,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({
@@ -646,7 +646,7 @@ export class RequestQueueClient extends ResourceClient {
         const parsed = parseArgument(options, forefrontOptionsSchema, 'RequestQueueClientDeleteRequestLockOptions');
 
         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({

src/utils.ts

@@ -472,3 +472,27 @@ export const mutuallyExclusive = <T extends object>(...keys: (keyof T & string)[
     (value) => keys.filter((key) => typeof value[key] !== 'undefined').length <= 1,
     `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.
+ *
+ * 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 {
+    const valueParsed = parseArgument(value, pathSegmentSchema);
+    return encodeURIComponent(valueParsed);
+}
+
+/**
+ * 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;
+}

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<void>((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(/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/);
+    });
+});