diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/copy-input-items.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/copy-input-items.test.ts index 7f74be9dc46..51dde8b2af7 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/copy-input-items.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/copy-input-items.test.ts @@ -46,4 +46,17 @@ describe('copyInputItems', () => { expect(output[0].a).toEqual(input.a); expect(output[0].a === input.a).toEqual(false); }); + + it.each(['__proto__', 'constructor', 'prototype'])( + 'should isolate items from inherited properties when given "%s"', + (propertyName) => { + const output = copyInputItems( + [{ json: { [propertyName]: 'test_value', safe: 1 } }], + [propertyName, 'safe'], + ); + + expect(output[0]).toHaveProperty('safe', 1); + expect(Object.getOwnPropertyNames(output[0])).toContain(propertyName); + }, + ); }); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/copy-input-items.ts b/packages/core/src/execution-engine/node-execution-context/utils/copy-input-items.ts index c0808a379df..5c652455f65 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/copy-input-items.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/copy-input-items.ts @@ -7,7 +7,7 @@ import { deepCopy } from 'n8n-workflow'; */ export function copyInputItems(items: INodeExecutionData[], properties: string[]): IDataObject[] { return items.map((item) => { - const newItem: IDataObject = {}; + const newItem: IDataObject = Object.create(null) as IDataObject; for (const property of properties) { if (item.json[property] === undefined) { newItem[property] = null; diff --git a/packages/nodes-base/nodes/Aws/DynamoDB/GenericFunctions.ts b/packages/nodes-base/nodes/Aws/DynamoDB/GenericFunctions.ts index 83e8d352027..e7ef624f332 100644 --- a/packages/nodes-base/nodes/Aws/DynamoDB/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Aws/DynamoDB/GenericFunctions.ts @@ -2,16 +2,15 @@ import type { IDataObject, IExecuteFunctions, IHookFunctions, + IHttpRequestMethods, + IHttpRequestOptions, ILoadOptionsFunctions, IWebhookFunctions, - IHttpRequestOptions, - INodeExecutionData, - IHttpRequestMethods, } from 'n8n-workflow'; -import { ApplicationError, deepCopy } from 'n8n-workflow'; +import { ApplicationError } from 'n8n-workflow'; -import type { IRequestBody } from './types'; import { getAwsCredentials } from '../GenericFunctions'; +import type { IRequestBody } from './types'; export async function awsApiRequest( this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions, @@ -94,16 +93,3 @@ export async function awsApiRequestAllItems( return returnData; } - -export function copyInputItem(item: INodeExecutionData, properties: string[]): IDataObject { - // Prepare the data to insert and copy it to be returned - const newItem: IDataObject = {}; - for (const property of properties) { - if (item.json[property] === undefined) { - newItem[property] = null; - } else { - newItem[property] = deepCopy(item.json[property]); - } - } - return newItem; -} diff --git a/packages/nodes-base/nodes/Aws/DynamoDB/utils.ts b/packages/nodes-base/nodes/Aws/DynamoDB/utils.ts index d7b37ece5a1..2f64dcc6867 100644 --- a/packages/nodes-base/nodes/Aws/DynamoDB/utils.ts +++ b/packages/nodes-base/nodes/Aws/DynamoDB/utils.ts @@ -1,5 +1,5 @@ -import type { IDataObject, INodeExecutionData } from 'n8n-workflow'; -import { deepCopy, assert, ApplicationError } from 'n8n-workflow'; +import type { IDataObject } from 'n8n-workflow'; +import { ApplicationError, assert } from 'n8n-workflow'; import type { AdjustedPutItem, @@ -102,19 +102,6 @@ export function validateJSON(input: any): object { } } -export function copyInputItem(item: INodeExecutionData, properties: string[]): IDataObject { - // Prepare the data to insert and copy it to be returned - const newItem: IDataObject = {}; - for (const property of properties) { - if (item.json[property] === undefined) { - newItem[property] = null; - } else { - newItem[property] = deepCopy(item.json[property]); - } - } - return newItem; -} - export function mapToAttributeValues(item: IDataObject): void { for (const key of Object.keys(item)) { if (!key.startsWith(':')) { diff --git a/packages/nodes-base/nodes/Bubble/Bubble.node.ts b/packages/nodes-base/nodes/Bubble/Bubble.node.ts index b88b66f8ff3..ebfb14ea380 100644 --- a/packages/nodes-base/nodes/Bubble/Bubble.node.ts +++ b/packages/nodes-base/nodes/Bubble/Bubble.node.ts @@ -80,7 +80,7 @@ export class Bubble implements INodeType { property: [{ key: string; value: string }]; }; - const body = {} as IDataObject; + const body = Object.create(null) as IDataObject; property.forEach((data) => (body[data.key] = data.value)); @@ -164,7 +164,7 @@ export class Bubble implements INodeType { property: [{ key: string; value: string }]; }; - const body = {} as IDataObject; + const body = Object.create(null) as IDataObject; property.forEach((data) => (body[data.key] = data.value)); responseData = await bubbleApiRequest.call(this, 'PATCH', endpoint, body, {}); diff --git a/packages/nodes-base/nodes/Microsoft/Sql/GenericFunctions.ts b/packages/nodes-base/nodes/Microsoft/Sql/GenericFunctions.ts index 6de40e4df90..76687325174 100644 --- a/packages/nodes-base/nodes/Microsoft/Sql/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Microsoft/Sql/GenericFunctions.ts @@ -16,7 +16,7 @@ import type { ITables, OperationInputData } from './interfaces'; */ export function copyInputItem(item: INodeExecutionData, properties: string[]): IDataObject { // Prepare the data to insert and copy it to be returned - const newItem: IDataObject = {}; + const newItem: IDataObject = Object.create(null); for (const property of properties) { if (item.json[property] === undefined) { newItem[property] = null; @@ -44,20 +44,21 @@ export function createTableStruct( const table = getNodeParam('table', index) as string; const columnString = getNodeParam('columns', index) as string; const columns = columnString.split(',').map((column) => column.trim()); + const itemCopy = copyInputItem(item, columns.concat(additionalProperties)); const keyParam = keyName ? (getNodeParam(keyName, index) as string) : undefined; - if (tables[table] === undefined) { - tables[table] = {}; + if (!Object.hasOwn(tables, table)) { + tables[table] = Object.create(null); } - if (tables[table][columnString] === undefined) { + if (!Object.hasOwn(tables[table], columnString)) { tables[table][columnString] = []; } if (keyName) { itemCopy[keyName] = keyParam; } - tables[table][columnString].push(itemCopy); + (tables[table][columnString] as IDataObject[]).push(itemCopy); return tables; - }, {} as ITables); + }, Object.create(null) as ITables); } /** diff --git a/packages/nodes-base/nodes/Microsoft/Sql/MicrosoftSql.node.ts b/packages/nodes-base/nodes/Microsoft/Sql/MicrosoftSql.node.ts index 36c45769225..456dec0eebc 100644 --- a/packages/nodes-base/nodes/Microsoft/Sql/MicrosoftSql.node.ts +++ b/packages/nodes-base/nodes/Microsoft/Sql/MicrosoftSql.node.ts @@ -384,15 +384,16 @@ export class MicrosoftSql implements INodeType { const tables = items.reduce((acc, item, index) => { const table = this.getNodeParameter('table', index) as string; const deleteKey = this.getNodeParameter('deleteKey', index) as string; - if (acc[table] === undefined) { - acc[table] = {}; + + if (!Object.hasOwn(acc, table)) { + acc[table] = Object.create(null); } - if (acc[table][deleteKey] === undefined) { + if (!Object.hasOwn(acc[table], deleteKey)) { acc[table][deleteKey] = []; } - acc[table][deleteKey].push(item); + (acc[table][deleteKey] as INodeExecutionData[]).push(item); return acc; - }, {} as ITables); + }, Object.create(null) as ITables); responseData = await deleteOperation(tables, pool); } diff --git a/packages/nodes-base/nodes/Microsoft/Sql/test/MicrosoftSql.node.test.ts b/packages/nodes-base/nodes/Microsoft/Sql/test/MicrosoftSql.node.test.ts index b5aab65cf66..4e69408fdbc 100644 --- a/packages/nodes-base/nodes/Microsoft/Sql/test/MicrosoftSql.node.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Sql/test/MicrosoftSql.node.test.ts @@ -178,4 +178,42 @@ describe('MicrosoftSql Node', () => { } }, ); + + describe('delete operation parameter validation', () => { + const buildPoolMock = () => { + const mockRequest = { + query: jest.fn().mockResolvedValue({ rowsAffected: [1] }), + input: jest.fn(), + }; + const mockPool = mock({ + connect: jest.fn().mockResolvedValue(undefined), + close: jest.fn(), + request: jest.fn().mockReturnValue(mockRequest), + }); + return mockPool; + }; + + it('should not alter shared object state when delete parameters contain reserved tokens', async () => { + const protoKeysBefore = Object.getOwnPropertyNames(Object.prototype); + + mockedConnectionPool.mockReturnValue(buildPoolMock()); + const node = new MicrosoftSql(); + + for (const reserved of ['__proto__', 'constructor', 'prototype']) { + const context = getMockedExecuteFunctions({ + getInputData: jest.fn().mockReturnValue([{ json: { id: 1 }, pairedItem: { item: 0 } }]), + getNodeParameter: jest.fn().mockImplementation((paramName: string) => { + if (paramName === 'operation') return 'delete'; + if (paramName === 'table') return reserved; + if (paramName === 'deleteKey') return 'id'; + return undefined; + }), + continueOnFail: jest.fn().mockReturnValue(false), + }); + await node.execute.call(context); + } + + expect(Object.getOwnPropertyNames(Object.prototype)).toEqual(protoKeysBefore); + }); + }); }); diff --git a/packages/nodes-base/nodes/Microsoft/Sql/test/utils.test.ts b/packages/nodes-base/nodes/Microsoft/Sql/test/utils.test.ts index 4cb251ade37..f4f1551de6a 100644 --- a/packages/nodes-base/nodes/Microsoft/Sql/test/utils.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Sql/test/utils.test.ts @@ -1,10 +1,12 @@ import { Request } from 'mssql'; import type { IResult } from 'mssql'; import type mssql from 'mssql'; -import type { IDataObject } from 'n8n-workflow'; +import type { IDataObject, INodeExecutionData } from 'n8n-workflow'; import { configurePool, + copyInputItem, + createTableStruct, deleteOperation, escapeIdentifier, escapeTableName, @@ -429,4 +431,54 @@ describe('MSSQL tests', () => { expect(escapeTableName('[schema].table')).toEqual('[[schema]].table]'); }); }); + + describe('createTableStruct', () => { + const makeItem = (json: IDataObject): INodeExecutionData => ({ + json, + pairedItem: { item: 0 }, + }); + + const makeGetParam = + (table: string, columns: string) => + (param: string, _index: number): string => { + if (param === 'table') return table; + if (param === 'columns') return columns; + return ''; + }; + + it('should build the correct struct for standard inputs', () => { + const items = [makeItem({ id: 1, name: 'Alice' })]; + const result = createTableStruct(makeGetParam('users', 'id, name'), items); + + expect(result).toEqual({ + users: { + 'id, name': [{ id: 1, name: 'Alice' }], + }, + }); + }); + + it('should group multiple items with the same table and columns', () => { + const items = [makeItem({ id: 1 }), makeItem({ id: 2 })]; + const result = createTableStruct(makeGetParam('orders', 'id'), items); + + expect(result.orders['id']).toHaveLength(2); + }); + }); + + describe('copyInputItem', () => { + const makeItem = (json: IDataObject): INodeExecutionData => ({ + json, + pairedItem: { item: 0 }, + }); + + it('should copy specified properties from item json', () => { + const item = makeItem({ id: 1, name: 'Bob', age: 30 }); + expect(copyInputItem(item, ['id', 'name'])).toEqual({ id: 1, name: 'Bob' }); + }); + + it('should set missing properties to null', () => { + const item = makeItem({ id: 1 }); + expect(copyInputItem(item, ['id', 'name'])).toEqual({ id: 1, name: null }); + }); + }); }); diff --git a/packages/nodes-base/nodes/Postgres/v1/genericFunctions.ts b/packages/nodes-base/nodes/Postgres/v1/genericFunctions.ts index 38217f4a3c3..586de9179cf 100644 --- a/packages/nodes-base/nodes/Postgres/v1/genericFunctions.ts +++ b/packages/nodes-base/nodes/Postgres/v1/genericFunctions.ts @@ -21,7 +21,7 @@ export function getItemsCopy( ): IDataObject[] { let newItem: IDataObject; return items.map((item) => { - newItem = {}; + newItem = Object.create(null) as IDataObject; if (guardedColumns) { Object.keys(guardedColumns).forEach((column) => { newItem[column] = item.json[guardedColumns[column]]; @@ -47,7 +47,7 @@ export function getItemCopy( properties: string[], guardedColumns?: { [key: string]: string }, ): IDataObject { - const newItem: IDataObject = {}; + const newItem: IDataObject = Object.create(null) as IDataObject; if (guardedColumns) { Object.keys(guardedColumns).forEach((column) => { newItem[column] = item.json[guardedColumns[column]];