fix(core): Sign S3 object paths with strict RFC 3986 encoding (#34694)

packages/nodes-base/credentials/common/aws/s3-path-signing.test.ts

@@ -0,0 +1,216 @@
+import type { IHttpRequestMethods, IHttpRequestOptions, IDataObject } from 'n8n-workflow';
+import { createHash, createHmac } from 'node:crypto';
+
+import type { AwsIamCredentialsType, AwsSecurityHeaders } from './types';
+import { awsGetSignInOptionsAndUpdateRequest, signOptions } from './utils';
+
+// End-to-end SigV4 tests with the REAL signers (no aws4 mock): a hand-computed
+// golden vector proves the signed canonical path is S3's strict UriEncode form,
+// and legacy/new parity runs prove the smithy path signs identically to aws4 —
+// the decade-proven oracle for S3's server-side canonicalization.
+
+const credentials: AwsIamCredentialsType = {
+	region: 'us-east-1',
+	customEndpoints: false,
+	accessKeyId: 'AKIDEXAMPLE',
+	secretAccessKey: 'wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY',
+	temporaryCredentials: false,
+};
+
+const securityHeaders: AwsSecurityHeaders = {
+	accessKeyId: credentials.accessKeyId,
+	secretAccessKey: credentials.secretAccessKey,
+	sessionToken: undefined,
+};
+
+const FIXED_TIME = new Date('2026-07-22T08:00:00Z');
+const AMZ_DATE = '20260722T080000Z';
+const DATE_STAMP = '20260722';
+
+const sha256Hex = (data: string | Buffer) => createHash('sha256').update(data).digest('hex');
+const hmac = (key: string | Buffer, data: string) =>
+	createHmac('sha256', key).update(data).digest();
+
+function expectedSignature(canonicalRequest: string, region: string, service: string): string {
+	const stringToSign = [
+		'AWS4-HMAC-SHA256',
+		AMZ_DATE,
+		`${DATE_STAMP}/${region}/${service}/aws4_request`,
+		sha256Hex(canonicalRequest),
+	].join('\n');
+	const kDate = hmac(`AWS4${credentials.secretAccessKey}`, DATE_STAMP);
+	const kRegion = hmac(kDate, region);
+	const kService = hmac(kRegion, service);
+	const kSigning = hmac(kService, 'aws4_request');
+	return createHmac('sha256', kSigning).update(stringToSign).digest('hex');
+}
+
+function getHeader(result: IHttpRequestOptions, name: string): string {
+	const headers = (result.headers ?? {}) as Record<string, string>;
+	const key = Object.keys(headers).find((k) => k.toLowerCase() === name.toLowerCase());
+	if (!key) throw new Error(`missing expected header: ${name}`);
+	return headers[key];
+}
+
+function parseAuthorization(auth: string): { signature: string; signedHeaders: string } {
+	const signature = /Signature=([0-9a-f]{64})/.exec(auth)?.[1];
+	const signedHeaders = /SignedHeaders=([^,\s]+)/.exec(auth)?.[1];
+	if (!signature || !signedHeaders) throw new Error(`unparseable Authorization header: ${auth}`);
+	return { signature, signedHeaders };
+}
+
+async function signS3Request(options: {
+	legacy: boolean;
+	path: string;
+	method: IHttpRequestMethods;
+	body?: string | Buffer;
+	query?: IDataObject;
+}): Promise<{ result: IHttpRequestOptions; url: string }> {
+	// The flag must be set before the path is built: the legacy branch skips the
+	// strict encoding so the rollback lever reproduces pre-migration wire bytes.
+	if (options.legacy) {
+		process.env.N8N_AWS_LEGACY_SIGNER = 'true';
+	} else {
+		delete process.env.N8N_AWS_LEGACY_SIGNER;
+	}
+	const requestOptions = {
+		headers: {},
+		...(options.body !== undefined && { body: options.body }),
+		...(options.query && { qs: { query: options.query } }),
+	} as IHttpRequestOptions;
+	const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+		requestOptions,
+		credentials,
+		options.path,
+		options.method,
+		's3',
+		'us-east-1',
+	);
+	const result = await signOptions(requestOptions, signOpts, securityHeaders, url, options.method);
+	return { result, url };
+}
+
+describe('S3 path signing (real signers)', () => {
+	beforeEach(() => {
+		vi.useFakeTimers();
+		vi.setSystemTime(FIXED_TIME);
+	});
+
+	afterEach(() => {
+		vi.useRealTimers();
+		delete process.env.N8N_AWS_LEGACY_SIGNER;
+	});
+
+	it('signs the strictly UriEncoded object path — hand-computed golden vector', async () => {
+		const { result, url } = await signS3Request({
+			legacy: false,
+			path: '/bucket/report (1)+final.pdf',
+			method: 'GET',
+		});
+
+		const canonicalPath = '/bucket/report%20%281%29%2Bfinal.pdf';
+		expect(url).toBe(`https://s3.us-east-1.amazonaws.com${canonicalPath}`);
+
+		const emptyBodyHash = sha256Hex('');
+		const canonicalRequest = [
+			'GET',
+			canonicalPath,
+			'',
+			'host:s3.us-east-1.amazonaws.com',
+			`x-amz-content-sha256:${emptyBodyHash}`,
+			`x-amz-date:${AMZ_DATE}`,
+			'',
+			'host;x-amz-content-sha256;x-amz-date',
+			emptyBodyHash,
+		].join('\n');
+
+		const auth = getHeader(result, 'authorization');
+		expect(auth).toContain(`Signature=${expectedSignature(canonicalRequest, 'us-east-1', 's3')}`);
+		expect(auth).toContain(
+			`Credential=${credentials.accessKeyId}/${DATE_STAMP}/us-east-1/s3/aws4_request`,
+		);
+	});
+
+	// Keys covering every character class the fix changes, plus already-working ones.
+	const KEY_CORPUS = [
+		'/bucket/plain.txt',
+		'/bucket/my report.pdf',
+		'/bucket/report (1)&v=2!.pdf',
+		"/bucket/quart'ile*star.log",
+		'/bucket/at@10:30,x;y=[z]|w.bin',
+		'/bucket/café 中文.pdf',
+		'/bucket/nested/deep (copy)/key.json',
+		'/bucket/a%2Fb.txt',
+	];
+
+	describe.each(KEY_CORPUS)('key %s', (path) => {
+		it('new signer matches the legacy aws4 signature (GET)', async () => {
+			const legacy = await signS3Request({ legacy: true, path, method: 'GET' });
+			const smithy = await signS3Request({ legacy: false, path, method: 'GET' });
+
+			const legacyAuth = parseAuthorization(getHeader(legacy.result, 'authorization'));
+			const smithyAuth = parseAuthorization(getHeader(smithy.result, 'authorization'));
+
+			// Same header set means the signatures are computed over comparable
+			// canonical requests — any difference is the path.
+			expect(smithyAuth.signedHeaders).toBe(legacyAuth.signedHeaders);
+			expect(smithyAuth.signature).toBe(legacyAuth.signature);
+		});
+
+		it('new signer matches the legacy aws4 signature (PUT with body)', async () => {
+			const body = Buffer.from('object body bytes');
+			const legacy = await signS3Request({ legacy: true, path, method: 'PUT', body });
+			const smithy = await signS3Request({ legacy: false, path, method: 'PUT', body });
+
+			const legacyAuth = parseAuthorization(getHeader(legacy.result, 'authorization'));
+			const smithyAuth = parseAuthorization(getHeader(smithy.result, 'authorization'));
+
+			expect(smithyAuth.signedHeaders).toBe(legacyAuth.signedHeaders);
+			expect(smithyAuth.signature).toBe(legacyAuth.signature);
+		});
+	});
+
+	it('signs multipart request shapes identically to aws4 (query canonicalization)', async () => {
+		const shapes: Array<{ method: IHttpRequestMethods; query: IDataObject; body?: Buffer }> = [
+			{ method: 'POST', query: { uploads: '' } },
+			{
+				method: 'PUT',
+				query: { partNumber: '2', uploadId: 'abc+def/ghi==' },
+				body: Buffer.from('chunk'),
+			},
+		];
+		for (const shape of shapes) {
+			const path = '/bucket/key (1).pdf';
+			const legacy = await signS3Request({ legacy: true, path, ...shape });
+			const smithy = await signS3Request({ legacy: false, path, ...shape });
+
+			const legacyAuth = parseAuthorization(getHeader(legacy.result, 'authorization'));
+			const smithyAuth = parseAuthorization(getHeader(smithy.result, 'authorization'));
+
+			expect(smithyAuth.signedHeaders).toBe(legacyAuth.signedHeaders);
+			expect(smithyAuth.signature).toBe(legacyAuth.signature);
+			expect(smithy.url).toBe(
+				'https://s3.us-east-1.amazonaws.com/bucket/key%20%281%29.pdf?' +
+					new URLSearchParams(shape.query as Record<string, string>).toString(),
+			);
+		}
+	});
+
+	it('keeps pre-migration wire bytes when the legacy signer flag is set (rollback lever)', async () => {
+		const { url } = await signS3Request({
+			legacy: true,
+			path: '/bucket/report (1)+final.pdf',
+			method: 'GET',
+		});
+		// Raw WHATWG form: parens and + untouched, only the space encoded — byte-identical
+		// to what pre-2.30 aws4 deployments sent, so + keys keep addressing 'a b'-style keys.
+		expect(url).toBe('https://s3.us-east-1.amazonaws.com/bucket/report%20(1)+final.pdf');
+	});
+
+	it('deliberately diverges from aws4 for a literal + in the key (kept as %2B, not space)', async () => {
+		const { url } = await signS3Request({ legacy: false, path: '/bucket/a+b.pdf', method: 'GET' });
+		// aws4 signed this as /bucket/a%20b.pdf (plus-as-space), silently storing
+		// 'a b.pdf'. The SDK-correct form preserves the plus; S3 decodes %2B → '+'.
+		expect(url).toBe('https://s3.us-east-1.amazonaws.com/bucket/a%2Bb.pdf');
+	});
+});

packages/nodes-base/credentials/common/aws/utils.test.ts

@@ -9,6 +9,7 @@ import {
 	AWS_REGION_SHAPE_PATTERN,
 	awsGetSignInOptionsAndUpdateRequest,
 	parseAwsUrl,
+	uriEncodeS3Pathname,
 	validateBedrockEndpointOverride,
 } from './utils';
 
@@ -1645,3 +1646,238 @@ describe('validateBedrockEndpointOverride', () => {
 		).toThrow(UserError);
 	});
 });
+
+describe('uriEncodeS3Pathname', () => {
+	// Characters WHATWG URL leaves raw in a pathname but S3's SigV4 canonicalization
+	// (AWS UriEncode) percent-encodes. Inputs are built through `new URL` so each
+	// case sees exactly what awsGetSignInOptionsAndUpdateRequest sees.
+	it.each([
+		['+', '%2B'],
+		['!', '%21'],
+		["'", '%27'],
+		['(', '%28'],
+		[')', '%29'],
+		['*', '%2A'],
+		['$', '%24'],
+		['&', '%26'],
+		[',', '%2C'],
+		[';', '%3B'],
+		['=', '%3D'],
+		[':', '%3A'],
+		['@', '%40'],
+		['[', '%5B'],
+		[']', '%5D'],
+		['|', '%7C'],
+	])('percent-encodes "%s" as "%s"', (char, encoded) => {
+		const { pathname } = new URL(`https://s3.us-east-1.amazonaws.com/bucket/file${char}name.pdf`);
+		expect(uriEncodeS3Pathname(pathname)).toBe(`/bucket/file${encoded}name.pdf`);
+	});
+
+	it('leaves unreserved characters and slash separators untouched', () => {
+		expect(uriEncodeS3Pathname('/bucket/Report_Final-1.2~ok/file.pdf')).toBe(
+			'/bucket/Report_Final-1.2~ok/file.pdf',
+		);
+	});
+
+	it('keeps sequences WHATWG already encoded stable (space stays %20)', () => {
+		const { pathname } = new URL('https://s3.us-east-1.amazonaws.com/bucket/my report.pdf');
+		expect(pathname).toBe('/bucket/my%20report.pdf');
+		expect(uriEncodeS3Pathname(pathname)).toBe('/bucket/my%20report.pdf');
+	});
+
+	it('is a no-op on WHATWG-encoded pathnames without S3-delta characters (no regression for working keys)', () => {
+		for (const key of ['plain.txt', 'my report.pdf', 'résumé 中文.pdf', 'nested/deep/key.json']) {
+			const { pathname } = new URL(`https://s3.us-east-1.amazonaws.com/bucket/${key}`);
+			expect(uriEncodeS3Pathname(pathname)).toBe(pathname);
+		}
+	});
+
+	it('percent-encodes raw non-ASCII input as UTF-8', () => {
+		expect(uriEncodeS3Pathname('/bucket/résumé.pdf')).toBe('/bucket/r%C3%A9sum%C3%A9.pdf');
+	});
+
+	it('collapses an encoded slash to / — S3 keys are flat, %2F and / address the same key', () => {
+		expect(uriEncodeS3Pathname('/bucket/a%2Fb.txt')).toBe('/bucket/a/b.txt');
+		// A double-encoded slash stays an encoded literal, exactly like aws4.
+		expect(uriEncodeS3Pathname('/bucket/a%252Fb.txt')).toBe('/bucket/a%252Fb.txt');
+	});
+
+	it('treats a stray % as literal text without poisoning valid escapes around it', () => {
+		expect(uriEncodeS3Pathname('/bucket/100%')).toBe('/bucket/100%25');
+		// '100% legit' after WHATWG: stray % stays raw, space became %20
+		expect(uriEncodeS3Pathname('/bucket/100%%20legit')).toBe('/bucket/100%25%20legit');
+	});
+
+	it('keeps a malformed UTF-8 escape run as literal text', () => {
+		expect(uriEncodeS3Pathname('/bucket/%C3.pdf')).toBe('/bucket/%25C3.pdf');
+	});
+
+	it('preserves empty segments (S3 paths are not normalized)', () => {
+		expect(uriEncodeS3Pathname('//bucket//key.txt')).toBe('//bucket//key.txt');
+		expect(uriEncodeS3Pathname('/')).toBe('/');
+	});
+
+	it('is idempotent over a corpus of hostile keys', () => {
+		const keys = [
+			'/bucket/report (1)+final&v=2!.pdf',
+			"/bucket/quart'ile*star.log",
+			'/bucket/at@10:30,x;y=[z]|w.bin',
+			'/bucket/café 中文.pdf',
+			'/bucket/100%%20legit',
+			'/bucket/a%2Fb%2520c.txt',
+		];
+		for (const key of keys) {
+			const once = uriEncodeS3Pathname(key);
+			expect(uriEncodeS3Pathname(once)).toBe(once);
+		}
+	});
+
+	it('matches the legacy aws4 canonical encoding, except + which is preserved as %2B', () => {
+		// aws4's S3 canonicalization: decode each segment (+ read as space), re-encode
+		// everything but unreserved characters, then collapse %2F back to /.
+		const aws4Canonical = (pathname: string) =>
+			pathname
+				.split('/')
+				.map((piece) =>
+					encodeURIComponent(decodeURIComponent(piece.replace(/\+/g, ' '))).replace(
+						/[!'()*]/g,
+						(c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`,
+					),
+				)
+				.join('/')
+				.replace(/%2F/g, '/');
+		const plusFreeKeys = [
+			'/bucket/report (1)&v=2!.pdf',
+			"/bucket/quart'ile*star.log",
+			'/bucket/at@10:30,x;y=[z]|w.bin',
+			'/bucket/café 中文.pdf',
+			'/bucket/a%2Fb.txt',
+		];
+		for (const key of plusFreeKeys) {
+			const { pathname } = new URL(`https://s3.us-east-1.amazonaws.com${key}`);
+			expect(uriEncodeS3Pathname(pathname)).toBe(aws4Canonical(pathname));
+		}
+		// Deliberate divergence: aws4 read a path + as a space (key 'a+b' was stored
+		// as 'a b'); we keep the literal plus, matching the AWS SDK.
+		expect(uriEncodeS3Pathname('/bucket/a+b.pdf')).toBe('/bucket/a%2Bb.pdf');
+		expect(aws4Canonical('/bucket/a+b.pdf')).toBe('/bucket/a%20b.pdf');
+	});
+});
+
+describe('awsGetSignInOptionsAndUpdateRequest — S3 object-path canonicalization', () => {
+	const credentials: AwsIamCredentialsType = {
+		region: 'us-east-1',
+		customEndpoints: false,
+		accessKeyId: 'AKIA-test',
+		secretAccessKey: 'secret-test',
+		temporaryCredentials: false,
+	};
+
+	it('signs and sends the strictly encoded path for the default S3 endpoint', () => {
+		const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+			{ headers: {} } as any,
+			credentials,
+			'/bucket/report (1)+final&v=2!.pdf',
+			'PUT',
+			's3',
+			'us-east-1',
+		);
+
+		expect(signOpts.path).toBe('/bucket/report%20%281%29%2Bfinal%26v%3D2%21.pdf');
+		// The wire URL carries the exact bytes that were signed.
+		expect(url).toBe('https://s3.us-east-1.amazonaws.com' + signOpts.path);
+	});
+
+	it('canonicalizes virtual-hosted-style requests (bucket.s3 signs under s3)', () => {
+		const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+			{ headers: {} } as any,
+			credentials,
+			'/report:2026-07-22T10:00.pdf',
+			'PUT',
+			'mybucket.s3',
+			'eu-central-1',
+		);
+
+		expect(signOpts.service).toBe('s3');
+		expect(signOpts.path).toBe('/report%3A2026-07-22T10%3A00.pdf');
+		expect(url).toBe('https://mybucket.s3.eu-central-1.amazonaws.com' + signOpts.path);
+	});
+
+	it('encodes the path but leaves query parameters to the signer (multipart shapes)', () => {
+		const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+			{ headers: {}, qs: { query: { uploads: '' } } } as any,
+			credentials,
+			'/bucket/key (1).pdf',
+			'POST',
+			's3',
+			'us-east-1',
+		);
+
+		expect(signOpts.path).toBe('/bucket/key%20%281%29.pdf?uploads=');
+		expect(url).toBe('https://s3.us-east-1.amazonaws.com' + signOpts.path);
+	});
+
+	it('canonicalizes S3 requests arriving through the uri branch (HTTP Request node)', () => {
+		const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+			{
+				uri: 'https://mybucket.s3.eu-west-1.amazonaws.com/exports/a+b (copy).csv',
+				headers: {},
+			} as any,
+			credentials,
+			'',
+			'GET',
+			'',
+			'us-east-1',
+		);
+
+		expect(signOpts.service).toBe('s3');
+		expect(signOpts.path).toBe('/exports/a%2Bb%20%28copy%29.csv');
+		expect(url).toBe('https://mybucket.s3.eu-west-1.amazonaws.com' + signOpts.path);
+	});
+
+	it('canonicalizes paths for a custom s3Endpoint (S3-compatible providers)', () => {
+		const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+			{ headers: {} } as any,
+			{ ...credentials, s3Endpoint: 'https://minio.internal:9000' },
+			'/bucket/a(1).pdf',
+			'PUT',
+			's3',
+			'us-east-1',
+		);
+
+		expect(signOpts.path).toBe('/bucket/a%281%29.pdf');
+		expect(url).toBe('https://minio.internal:9000' + signOpts.path);
+	});
+
+	it('leaves non-S3 service paths untouched (Lambda ARN colons stay raw)', () => {
+		const path =
+			'/2015-03-31/functions/arn:aws:lambda:us-east-1:123456789012:function:fn/invocations';
+		const { signOpts, url } = awsGetSignInOptionsAndUpdateRequest(
+			{ headers: {} } as any,
+			credentials,
+			path,
+			'POST',
+			'lambda',
+			'us-east-1',
+		);
+
+		expect(signOpts.path).toBe(path);
+		expect(url).toBe('https://lambda.us-east-1.amazonaws.com' + path);
+	});
+
+	it('does not change the signed path for keys that already worked (spaces, unicode)', () => {
+		const { signOpts } = awsGetSignInOptionsAndUpdateRequest(
+			{ headers: {} } as any,
+			credentials,
+			'/bucket/my réport 中.pdf',
+			'GET',
+			's3',
+			'us-east-1',
+		);
+
+		// Identical to the WHATWG pathname this function produced before the fix.
+		expect(signOpts.path).toBe(
+			new URL('https://s3.us-east-1.amazonaws.com/bucket/my réport 中.pdf').pathname,
+		);
+	});
+});

packages/nodes-base/credentials/common/aws/utils.ts

@@ -410,6 +410,40 @@ function resolveServiceAndRegion(
 	return { service: resolvedService, region: resolvedRegion };
 }
 
+/**
+ * Applies AWS's S3 `UriEncode` canonicalization to a URL pathname: each segment is
+ * decoded, then percent-encoded so that only RFC 3986 unreserved characters and the
+ * `/` separators stay literal — the form S3 computes server-side when verifying
+ * SigV4 signatures, and the form the AWS SDK sends on the wire. A segment that is
+ * not valid percent-encoding (a stray `%`) is treated as raw text, so the key still
+ * round-trips unchanged. An encoded slash collapses to `/` (S3 keys are flat, so
+ * `%2F` and `/` address the same key — aws4 did the same). A literal `+` becomes
+ * `%2B` (AWS SDK behavior); the legacy aws4 signer read a path `+` as a space.
+ */
+export function uriEncodeS3Pathname(pathname: string): string {
+	return pathname
+		.split('/')
+		.map((segment) => {
+			// Decode runs of percent-escapes rather than the whole segment: a stray `%`
+			// (not valid encoding) then stays literal text and is encoded below, instead
+			// of poisoning the valid escapes around it.
+			const decoded = segment.replace(/(?:%[0-9A-Fa-f]{2})+/g, (run) => {
+				try {
+					return decodeURIComponent(run);
+				} catch {
+					// e.g. malformed UTF-8 byte sequences — keep the run as literal text
+					return run;
+				}
+			});
+			return encodeURIComponent(decoded).replace(
+				/[!'()*]/g,
+				(char) => `%${char.charCodeAt(0).toString(16).toUpperCase()}`,
+			);
+		})
+		.join('/')
+		.replace(/%2F/g, '/');
+}
+
 /**
  * Prepares AWS request options for signing by constructing the proper endpoint URL,
  * handling query parameters, and setting up the request body for AWS4 signature.
@@ -524,7 +558,17 @@ export function awsGetSignInOptionsAndUpdateRequest(
 		body = '';
 	}
 
-	path = endpoint.pathname + endpoint.search;
+	const signingService = getAwsSigningService(service);
+
+	// S3 verifies the signature against the strictly encoded object path, and
+	// uriEscapePath is off for S3 so smithy signs this string verbatim. WHATWG URL
+	// leaves characters like ( ) + & = : @ raw in the pathname, so encode it once
+	// here — the same string becomes both the signed path and the wire URL below.
+	// The legacy signer must keep the raw path: it canonicalizes internally, and
+	// the rollback flag has to reproduce pre-migration wire bytes exactly.
+	const encodeS3Path = signingService === 's3' && process.env.N8N_AWS_LEGACY_SIGNER !== 'true';
+	path =
+		(encodeS3Path ? uriEncodeS3Pathname(endpoint.pathname) : endpoint.pathname) + endpoint.search;
 
 	// ! aws4.sign *must* have the body to sign, but we might have .form instead of .body
 	const requestWithForm = requestOptions as unknown as { form?: Record<string, string> };
@@ -544,7 +588,6 @@ export function awsGetSignInOptionsAndUpdateRequest(
 		contentTypeHeader = 'application/x-www-form-urlencoded';
 	}
 
-	const signingService = getAwsSigningService(service);
 	const signOpts = {
 		...requestOptions,
 		headers: {