fix(core): Restore mutating array methods on $json data in expressions (#37172)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

packages/@n8n/expression-runtime/src/extensions/array-extensions.ts

@@ -34,6 +34,37 @@ function reverse(value: unknown[]): unknown[] {
 	return [...value].reverse();
 }
 
+// TODO(CAT-4266): reconsider this approach — copy-on-write proxy traps would
+// make these shims unnecessary.
+// Copy-first shims for native in-place mutators, like reverse(): the VM engine's
+// data proxies are read-only (the natives would throw), and mutations from
+// expressions must not leak into workflow data.
+function sort(value: unknown[], extraArgs: unknown[]): unknown[] {
+	const [comparator] = extraArgs as [((a: unknown, b: unknown) => number)?];
+	return value.slice().sort(comparator);
+}
+
+function splice(value: unknown[], extraArgs: unknown[]): unknown[] {
+	const copy = value.slice();
+	return copy.splice(...(extraArgs as [number, number, ...unknown[]]));
+}
+
+function fill(value: unknown[], extraArgs: unknown[]): unknown[] {
+	return value.slice().fill(...(extraArgs as [unknown, number?, number?]));
+}
+
+function copyWithin(value: unknown[], extraArgs: unknown[]): unknown[] {
+	return value.slice().copyWithin(...(extraArgs as [number, number, number?]));
+}
+
+function shift(value: unknown[]): unknown {
+	return value.slice().shift();
+}
+
+function unshift(value: unknown[], extraArgs: unknown[]): number {
+	return value.slice().unshift(...extraArgs);
+}
+
 function pluck(value: unknown[], extraArgs: unknown[]): unknown[] {
 	if (!Array.isArray(extraArgs)) {
 		throw new ExpressionExtensionError('arguments must be passed to pluck');
@@ -738,6 +769,12 @@ export const arrayExtensions: ExtensionMap = {
 		first,
 		last,
 		reverse,
+		sort,
+		splice,
+		fill,
+		copyWithin,
+		shift,
+		unshift,
 		pluck,
 		randomItem,
 		sum,

packages/workflow/src/extensions/array-extensions.ts

@@ -34,6 +34,37 @@ function reverse(value: unknown[]): unknown[] {
 	return [...value].reverse();
 }
 
+// TODO(CAT-4266): reconsider this approach — copy-on-write proxy traps would
+// make these shims unnecessary.
+// Copy-first shims for native in-place mutators, like reverse(): the VM engine's
+// data proxies are read-only (the natives would throw), and mutations from
+// expressions must not leak into workflow data.
+function sort(value: unknown[], extraArgs: unknown[]): unknown[] {
+	const [comparator] = extraArgs as [((a: unknown, b: unknown) => number)?];
+	return value.slice().sort(comparator);
+}
+
+function splice(value: unknown[], extraArgs: unknown[]): unknown[] {
+	const copy = value.slice();
+	return copy.splice(...(extraArgs as [number, number, ...unknown[]]));
+}
+
+function fill(value: unknown[], extraArgs: unknown[]): unknown[] {
+	return value.slice().fill(...(extraArgs as [unknown, number?, number?]));
+}
+
+function copyWithin(value: unknown[], extraArgs: unknown[]): unknown[] {
+	return value.slice().copyWithin(...(extraArgs as [number, number, number?]));
+}
+
+function shift(value: unknown[]): unknown {
+	return value.slice().shift();
+}
+
+function unshift(value: unknown[], extraArgs: unknown[]): number {
+	return value.slice().unshift(...extraArgs);
+}
+
 function pluck(value: unknown[], extraArgs: unknown[]): unknown[] {
 	if (!Array.isArray(extraArgs)) {
 		throw new ExpressionError('arguments must be passed to pluck');
@@ -740,6 +771,12 @@ export const arrayExtensions: ExtensionMap = {
 		first,
 		last,
 		reverse,
+		sort,
+		splice,
+		fill,
+		copyWithin,
+		shift,
+		unshift,
 		pluck,
 		randomItem,
 		sum,

packages/workflow/test/expression-array-proxy-semantics.test.ts

@@ -78,4 +78,63 @@ describe('Expression — array proxy semantics (engine parity)', () => {
 			[3, 2, 1],
 		]);
 	});
+
+	describe('mutating array methods return a mutated copy', () => {
+		it('sort() returns the sorted array', () => {
+			const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
+			expect(evaluate('={{ $json.arr.sort() }}', json)).toEqual([
+				'Apple',
+				'Kiwi',
+				'Mango',
+				'Orange',
+			]);
+			expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
+		});
+
+		it('sort() forwards the comparator', () => {
+			const json = { arr: [3, 1, 10, 2] };
+			expect(evaluate('={{ $json.arr.sort((a, b) => b - a) }}', json)).toEqual([10, 3, 2, 1]);
+			expect(json.arr).toEqual([3, 1, 10, 2]);
+		});
+
+		it('splice() returns the removed elements', () => {
+			const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
+			expect(evaluate('={{ $json.arr.splice(0, 2) }}', json)).toEqual(['Mango', 'Apple']);
+			expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
+		});
+
+		it('fill() returns the filled array', () => {
+			const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
+			expect(evaluate('={{ $json.arr.fill("X", 0, 2) }}', json)).toEqual([
+				'X',
+				'X',
+				'Kiwi',
+				'Orange',
+			]);
+			expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
+		});
+
+		it('shift() returns the removed first element', () => {
+			const json = { arr: ['Mango', 'Apple', 'Kiwi'] };
+			expect(evaluate('={{ $json.arr.shift() }}', json)).toBe('Mango');
+			expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi']);
+		});
+
+		it('unshift() returns the new length', () => {
+			const json = { arr: ['Mango', 'Apple', 'Kiwi'] };
+			expect(evaluate('={{ $json.arr.unshift("Peach", "Grape") }}', json)).toBe(5);
+			expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi']);
+		});
+
+		it('copyWithin() returns the copied-within array', () => {
+			const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
+			expect(evaluate('={{ $json.arr.copyWithin(0, 2, 4) }}', json)).toEqual([
+				'Kiwi',
+				'Orange',
+				'Kiwi',
+				'Orange',
+			]);
+			expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
+		});
+	});
 });