From 439d2601815a72bf93ca185af1e5b49c520fa9af Mon Sep 17 00:00:00 2001 From: Juuso Tapaninen Date: Fri, 29 May 2026 16:40:03 +0300 Subject: [PATCH] fix(MongoDB Node): Validate update key value type (#31371) --- .../nodes/MongoDb/GenericFunctions.test.ts | 79 +++++++++++- .../nodes/MongoDb/GenericFunctions.ts | 33 ++++- .../nodes-base/nodes/MongoDb/MongoDb.node.ts | 120 +++++++++--------- .../nodes/MongoDb/test/MongoDB.test.ts | 72 +++++++++++ 4 files changed, 238 insertions(+), 66 deletions(-) diff --git a/packages/nodes-base/nodes/MongoDb/GenericFunctions.test.ts b/packages/nodes-base/nodes/MongoDb/GenericFunctions.test.ts index 73bf1dcc6f0..40af929347d 100644 --- a/packages/nodes-base/nodes/MongoDb/GenericFunctions.test.ts +++ b/packages/nodes-base/nodes/MongoDb/GenericFunctions.test.ts @@ -1,12 +1,16 @@ +import type { INode } from 'n8n-workflow'; + import { prepareItems } from './GenericFunctions'; +const mockNode = { name: 'MongoDB', type: 'n8n-nodes-base.mongoDb' } as INode; + describe('MongoDB Node: Generic Functions', () => { describe('prepareItems', () => { it('should select fields', () => { const items = [{ json: { name: 'John', age: 30 } }, { json: { name: 'Jane', age: 25 } }]; const fields = ['name']; - const result = prepareItems({ items, fields }); + const result = prepareItems({ items, fields, node: mockNode }); expect(result).toEqual([{ name: 'John' }, { name: 'Jane' }]); }); @@ -16,7 +20,7 @@ describe('MongoDB Node: Generic Functions', () => { const fields = ['age']; const updateKey = 'name'; - const result = prepareItems({ items, fields, updateKey }); + const result = prepareItems({ items, fields, updateKey, node: mockNode }); expect(result).toEqual([ { name: 'John', age: 30 }, @@ -29,7 +33,13 @@ describe('MongoDB Node: Generic Functions', () => { const fields = ['user.name']; const useDotNotation = true; - const result = prepareItems({ items, fields, updateKey: '', useDotNotation }); + const result = prepareItems({ + items, + fields, + updateKey: '', + useDotNotation, + node: mockNode, + }); expect(result).toEqual([{ user: { name: 'John' } }, { user: { name: 'Jane' } }]); }); @@ -50,6 +60,7 @@ describe('MongoDB Node: Generic Functions', () => { useDotNotation, dateFields, isUpdate, + node: mockNode, }); expect(result).toEqual([ { date: new Date('2023-10-01T00:00:00Z') }, @@ -57,6 +68,67 @@ describe('MongoDB Node: Generic Functions', () => { ]); }); + describe('updateKey value validation', () => { + it('throws when the updateKey value is a plain object', () => { + const items = [{ json: { id: { $regex: '^a' }, value: 'x' } }]; + const args = { items, fields: ['value'], updateKey: 'id', node: mockNode }; + + expect(() => prepareItems(args)).toThrow(/must be a string, number, boolean, or date/); + }); + + it('throws when the updateKey value is an array', () => { + const items = [{ json: { id: ['a', 'b'], value: 'x' } }]; + const args = { items, fields: ['value'], updateKey: 'id', node: mockNode }; + + expect(() => prepareItems(args)).toThrow(); + }); + + it('drops items where useDotNotation would resolve the updateKey to a non-scalar', () => { + // The data-filter step in prepareItems uses bracket access on the dotted key, + // so items whose dot path resolves to an object are excluded before the map loop + // runs. No operator-shaped value reaches the driver. + const items = [{ json: { user: { id: { $gt: 1 } }, value: 'x' } }]; + const args = { + items, + fields: ['value'], + updateKey: 'user.id', + useDotNotation: true, + node: mockNode, + }; + + const result = prepareItems(args); + + expect(result).toEqual([]); + }); + + it('passes a string updateKey value through unchanged even when its content looks like JSON', () => { + const items = [{ json: { id: '{"$regex":"^a"}', value: 'x' } }]; + const args = { items, fields: ['value'], updateKey: 'id', node: mockNode }; + + const result = prepareItems(args); + + expect(result).toEqual([{ id: '{"$regex":"^a"}', value: 'x' }]); + }); + + it('accepts number, boolean, and null updateKey values', () => { + const items = [ + { json: { id: 1, value: 'a' } }, + { json: { id: true, value: 'b' } }, + { json: { id: null, value: 'c' } }, + ]; + const args = { items, fields: ['value'], updateKey: 'id', node: mockNode }; + + expect(() => prepareItems(args)).not.toThrow(); + }); + + it('accepts Date updateKey values', () => { + const items = [{ json: { id: new Date('2024-01-01'), value: 'a' } }]; + const args = { items, fields: ['value'], updateKey: 'id', node: mockNode }; + + expect(() => prepareItems(args)).not.toThrow(); + }); + }); + it('should handle updates', () => { // Should keep dot notation in result to not overwrite the original values const items = [ @@ -73,6 +145,7 @@ describe('MongoDB Node: Generic Functions', () => { useDotNotation, dateFields: [], isUpdate, + node: mockNode, }); expect(result).toEqual([{ 'user.name': 'John' }, { 'user.name': 'Jane' }]); }); diff --git a/packages/nodes-base/nodes/MongoDb/GenericFunctions.ts b/packages/nodes-base/nodes/MongoDb/GenericFunctions.ts index b622dac98d2..1b383890a6e 100644 --- a/packages/nodes-base/nodes/MongoDb/GenericFunctions.ts +++ b/packages/nodes-base/nodes/MongoDb/GenericFunctions.ts @@ -80,6 +80,24 @@ export function validateAndResolveMongoCredentials( } } +function isScalarUpdateKeyValue( + value: unknown, +): value is string | number | boolean | bigint | Date | null { + if (value === null) return true; + const type = typeof value; + if (type === 'string' || type === 'number' || type === 'boolean' || type === 'bigint') { + return true; + } + return value instanceof Date; +} + +function describeUpdateKeyValueType(value: unknown): string { + if (value === null) return 'null'; + if (Array.isArray(value)) return 'array'; + if (value instanceof Date) return 'date'; + return typeof value; +} + export function prepareItems({ items, fields, @@ -87,6 +105,7 @@ export function prepareItems({ useDotNotation = false, dateFields = [], isUpdate = false, + node, }: { items: INodeExecutionData[]; fields: string[]; @@ -94,6 +113,7 @@ export function prepareItems({ useDotNotation?: boolean; dateFields?: string[]; isUpdate?: boolean; + node: INode; }) { let data = items; @@ -104,7 +124,7 @@ export function prepareItems({ data = items.filter((item) => item.json[updateKey] !== undefined); } - const preparedItems = data.map(({ json }) => { + const preparedItems = data.map(({ json }, itemIndex) => { const updateItem: IDataObject = {}; for (const field of fields) { @@ -120,6 +140,17 @@ export function prepareItems({ fieldData = new Date(fieldData as string); } + if (field === updateKey && !isScalarUpdateKeyValue(fieldData)) { + throw new NodeOperationError( + node, + `The value of "${updateKey}" must be a string, number, boolean, or date`, + { + itemIndex, + description: `Got ${describeUpdateKeyValueType(fieldData)} instead. Objects and arrays are not allowed as the match value.`, + }, + ); + } + if (useDotNotation && !isUpdate) { set(updateItem, field, fieldData); } else { diff --git a/packages/nodes-base/nodes/MongoDb/MongoDb.node.ts b/packages/nodes-base/nodes/MongoDb/MongoDb.node.ts index 8d827605409..df294fb3d03 100644 --- a/packages/nodes-base/nodes/MongoDb/MongoDb.node.ts +++ b/packages/nodes-base/nodes/MongoDb/MongoDb.node.ts @@ -254,28 +254,25 @@ export class MongoDb implements INodeType { const updateOptions = (this.getNodeParameter('upsert', i) as boolean) ? { upsert: true } : undefined; - const [item] = prepareItems({ - items: [items[i]], - fields, - updateKey, - useDotNotation, - dateFields, - }); - - if (!item) { - if (this.continueOnFail()) { - returnData.push({ - json: { error: 'Item is missing the updateKey field' }, - pairedItem: { item: i }, - }); - continue; - } - throw new NodeOperationError(this.getNode(), 'Item is missing the updateKey field', { - itemIndex: i, - }); - } try { + const [item] = prepareItems({ + items: [items[i]], + fields, + updateKey, + useDotNotation, + dateFields, + node: this.getNode(), + }); + + if (!item) { + throw new NodeOperationError( + this.getNode(), + 'Item is missing the updateKey field', + { itemIndex: i }, + ); + } + const filter = { [updateKey]: item[updateKey] }; if (updateKey === '_id') { filter[updateKey] = new ObjectId(item[updateKey] as string); @@ -321,6 +318,7 @@ export class MongoDb implements INodeType { updateKey, useDotNotation, dateFields, + node: this.getNode(), }); for (const item of updateItems) { @@ -367,29 +365,26 @@ export class MongoDb implements INodeType { const updateOptions = (this.getNodeParameter('upsert', i) as boolean) ? { upsert: true } : undefined; - const [item] = prepareItems({ - items: [items[i]], - fields, - updateKey, - useDotNotation, - dateFields, - isUpdate: true, - }); - - if (!item) { - if (this.continueOnFail()) { - returnData.push({ - json: { error: 'Item is missing the updateKey field' }, - pairedItem: { item: i }, - }); - continue; - } - throw new NodeOperationError(this.getNode(), 'Item is missing the updateKey field', { - itemIndex: i, - }); - } try { + const [item] = prepareItems({ + items: [items[i]], + fields, + updateKey, + useDotNotation, + dateFields, + isUpdate: true, + node: this.getNode(), + }); + + if (!item) { + throw new NodeOperationError( + this.getNode(), + 'Item is missing the updateKey field', + { itemIndex: i }, + ); + } + const filter = { [updateKey]: item[updateKey] }; if (updateKey === '_id') { filter[updateKey] = new ObjectId(item[updateKey] as string); @@ -436,6 +431,7 @@ export class MongoDb implements INodeType { useDotNotation, dateFields, isUpdate: nodeVersion >= 1.2, + node: this.getNode(), }); for (const item of updateItems) { @@ -488,6 +484,7 @@ export class MongoDb implements INodeType { updateKey: '', useDotNotation, dateFields, + node: this.getNode(), }); if (!insertItem) continue; @@ -561,6 +558,7 @@ export class MongoDb implements INodeType { updateKey: '', useDotNotation, dateFields, + node: this.getNode(), }); const { insertedIds } = await mdb @@ -606,29 +604,26 @@ export class MongoDb implements INodeType { const updateOptions = (this.getNodeParameter('upsert', i) as boolean) ? { upsert: true } : undefined; - const [item] = prepareItems({ - items: [items[i]], - fields, - updateKey, - useDotNotation, - dateFields, - isUpdate: true, - }); - - if (!item) { - if (this.continueOnFail()) { - returnData.push({ - json: { error: 'Item is missing the updateKey field' }, - pairedItem: { item: i }, - }); - continue; - } - throw new NodeOperationError(this.getNode(), 'Item is missing the updateKey field', { - itemIndex: i, - }); - } try { + const [item] = prepareItems({ + items: [items[i]], + fields, + updateKey, + useDotNotation, + dateFields, + isUpdate: true, + node: this.getNode(), + }); + + if (!item) { + throw new NodeOperationError( + this.getNode(), + 'Item is missing the updateKey field', + { itemIndex: i }, + ); + } + const filter = { [updateKey]: item[updateKey] }; if (updateKey === '_id') { filter[updateKey] = new ObjectId(item[updateKey] as string); @@ -675,6 +670,7 @@ export class MongoDb implements INodeType { useDotNotation, dateFields, isUpdate: nodeVersion >= 1.2, + node: this.getNode(), }); for (const item of updateItems) { diff --git a/packages/nodes-base/nodes/MongoDb/test/MongoDB.test.ts b/packages/nodes-base/nodes/MongoDb/test/MongoDB.test.ts index d613bf3bfb8..b82c7f5255a 100644 --- a/packages/nodes-base/nodes/MongoDb/test/MongoDB.test.ts +++ b/packages/nodes-base/nodes/MongoDb/test/MongoDB.test.ts @@ -370,6 +370,78 @@ describe('MongoDB CRUD Node', () => { expect(items[2].pairedItem).toEqual({ item: 2 }); }); + describe.each(['findOneAndReplace', 'findOneAndUpdate', 'update'])( + '%s: non-scalar updateKey value', + (operation) => { + const itemsWithObjectKey = [ + { json: { id: { $regex: '^a' }, value: 'x', collection: 'col1' } }, + ]; + + function mockObjectKey(continueOnFail: boolean) { + const mock = mockExecuteFunctions(1.3, operation); + mock.getInputData.mockReturnValue(itemsWithObjectKey); + mock.continueOnFail.mockReturnValue(continueOnFail); + mock.getNodeParameter.mockImplementation( + (parameterName: string, _itemIndex = 0, fallbackValue?: NodeParameterValueType) => { + switch (parameterName) { + case 'operation': + return operation; + case 'collection': + return 'col1'; + case 'fields': + return 'value'; + case 'updateKey': + return 'id'; + case 'upsert': + return false; + case 'options.useDotNotation': + return false; + case 'options.dateFields': + return ''; + default: + return fallbackValue; + } + }, + ); + return mock; + } + + it('throws NodeOperationError when continueOnFail is off', async () => { + await expect(node.execute.call(mockObjectKey(false))).rejects.toThrow( + /must be a string, number, boolean, or date/, + ); + }); + + it('pushes error item with pairedItem when continueOnFail is on', async () => { + const [items] = await node.execute.call(mockObjectKey(true)); + expect(items).toHaveLength(1); + expect(items[0].json.error).toMatch(/must be a string, number, boolean, or date/); + expect(items[0].pairedItem).toEqual({ item: 0 }); + }); + + it('does not invoke the driver for the affected item', async () => { + const findOneAndReplaceSpy = jest.spyOn(Collection.prototype, 'findOneAndReplace'); + const findOneAndUpdateSpy = jest.spyOn(Collection.prototype, 'findOneAndUpdate'); + const updateOneSpy = jest.spyOn(Collection.prototype, 'updateOne'); + findOneAndReplaceSpy.mockResolvedValue(null); + findOneAndUpdateSpy.mockResolvedValue(null); + updateOneSpy.mockResolvedValue({ + acknowledged: true, + matchedCount: 0, + modifiedCount: 0, + upsertedCount: 0, + upsertedId: null, + }); + + await node.execute.call(mockObjectKey(true)); + + expect(findOneAndReplaceSpy).not.toHaveBeenCalled(); + expect(findOneAndUpdateSpy).not.toHaveBeenCalled(); + expect(updateOneSpy).not.toHaveBeenCalled(); + }); + }, + ); + describe.each(['findOneAndReplace', 'findOneAndUpdate', 'update'])( '%s: item missing the updateKey field', (operation) => {