fix: Add parameter binding for mongodb sort and projection (#37335)

This commit is contained in:
Dimitri Lavrenük
2026-08-31 14:23:28 +00:00
committed by GitHub
parent 2586e21ebe
commit 292f442afa
6 changed files with 335 additions and 101 deletions
@@ -1,8 +1,6 @@
import { Binary, ObjectId } from 'mongodb';
import type { INode, IExecuteFunctions } from 'n8n-workflow';
import { parseAndResolveQueryParameters } from '@utils/query-parameters';
import {
buildParameterizedConnString,
prepareItems,
@@ -117,58 +115,6 @@ describe('MongoDB Node: Generic Functions', () => {
});
});
describe('parseAndResolveQueryParameters', () => {
it('replaces placeholders with scalars and scalar arrays', () => {
const query = JSON.stringify({
name: '$1',
age: { $gte: '$2' },
tags: { $in: '$3' },
});
const result = parseAndResolveQueryParameters(
query,
'["Alice", 30, ["active", "admin"]]',
mockNode,
0,
);
expect(result).toEqual({
name: 'Alice',
age: { $gte: 30 },
tags: { $in: ['active', 'admin'] },
});
});
it('only replaces complete values, not keys or parts of strings', () => {
const query = JSON.stringify({ $1: 'key', exact: '$1', partial: 'user-$1' });
const result = parseAndResolveQueryParameters(query, ['Alice'], mockNode, 0);
expect(result).toEqual({ $1: 'key', exact: 'Alice', partial: 'user-$1' });
});
it('does not replace placeholders when parameters are empty', () => {
const result = parseAndResolveQueryParameters('{ "name": "$1" }', [], mockNode, 0);
expect(result).toEqual({ name: '$1' });
});
it.each([{ parameters: [{ name: 'Alice' }] }, { parameters: [[['nested']]] }])(
'throws for unsupported parameter value $parameters',
({ parameters }) => {
expect(() =>
parseAndResolveQueryParameters('{ "name": "$1" }', parameters, mockNode, 0),
).toThrow(/must be a scalar or an array of scalars/);
},
);
it('throws when a parameter is not used', () => {
expect(() =>
parseAndResolveQueryParameters('{ "name": "$1" }', ['Alice', 30], mockNode, 0),
).toThrow('Query parameter 2 is not used');
});
});
describe('prepareItems', () => {
it('should select fields', () => {
const items = [{ json: { name: 'John', age: 30 } }, { json: { name: 'Jane', age: 25 } }];
@@ -14,6 +14,7 @@ import type {
ICredentialsDecrypted,
ICredentialTestFunctions,
IDataObject,
INode,
INodeCredentialTestResult,
INodeExecutionData,
INodeType,
@@ -37,6 +38,20 @@ import type { IMongoParametricCredentials } from './mongoDb.types';
import { nodeProperties } from './MongoDbProperties';
import { generatePairedItemData } from '../../utils/utilities';
function resolveIndexDefinition(
ctx: IExecuteFunctions,
node: INode,
itemIndex: number,
): Record<string, unknown> {
return parseAndResolveQueryParameters(
ctx.getNodeParameter('indexDefinition', itemIndex) as string,
ctx.getNodeParameter('indexDefinitionParameters', itemIndex, []),
node,
itemIndex,
'Index Definition',
) as Record<string, unknown>;
}
interface BulkUpdateEntry {
op: AnyBulkWriteOperation;
item: IDataObject;
@@ -259,7 +274,7 @@ export class MongoDb implements INodeType {
try {
const queryParameter = parseAndResolveQueryParameters(
this.getNodeParameter('query', i) as string,
this.getNodeParameter('queryParameters', i, '[]'),
this.getNodeParameter('queryParameters', i, []),
node,
i,
) as IDataObject;
@@ -293,7 +308,7 @@ export class MongoDb implements INodeType {
try {
const queryParameter = parseAndResolveQueryParameters(
this.getNodeParameter('query', i) as string,
this.getNodeParameter('queryParameters', i, '[]'),
this.getNodeParameter('queryParameters', i, []),
node,
i,
) as Document;
@@ -323,7 +338,7 @@ export class MongoDb implements INodeType {
try {
const queryParameter = parseAndResolveQueryParameters(
this.getNodeParameter('query', i) as string,
this.getNodeParameter('queryParameters', i, '[]'),
this.getNodeParameter('queryParameters', i, []),
node,
i,
) as IDataObject;
@@ -340,8 +355,23 @@ export class MongoDb implements INodeType {
const limit = options.limit as number;
const skip = options.skip as number;
const projection =
options.projection && (JSON.parse(options.projection as string) as Document);
const sort = options.sort && (JSON.parse(options.sort as string) as Sort);
options.projection &&
(parseAndResolveQueryParameters(
options.projection as string,
options.projectionParameters ?? [],
node,
i,
'Projection',
) as Document);
const sort =
options.sort &&
(parseAndResolveQueryParameters(
options.sort as string,
options.sortParameters ?? [],
node,
i,
'Sort',
) as Sort);
if (skip > 0) {
query = query.skip(skip);
@@ -914,9 +944,7 @@ export class MongoDb implements INodeType {
const collection = this.getNodeParameter('collection', i) as string;
const indexName = this.getNodeParameter('indexNameRequired', i) as string;
const indexType = this.getNodeParameter('indexType', i) as string;
const definition = JSON.parse(
this.getNodeParameter('indexDefinition', i) as string,
) as Record<string, unknown>;
const definition = resolveIndexDefinition(this, node, i);
await mdb.collection(collection).createSearchIndex({
name: indexName,
@@ -946,9 +974,7 @@ export class MongoDb implements INodeType {
try {
const collection = this.getNodeParameter('collection', i) as string;
const indexName = this.getNodeParameter('indexNameRequired', i) as string;
const definition = JSON.parse(
this.getNodeParameter('indexDefinition', i) as string,
) as Record<string, unknown>;
const definition = resolveIndexDefinition(this, node, i);
await mdb.collection(collection).updateSearchIndex(indexName, definition);
@@ -1,5 +1,8 @@
import type { INodeProperties } from 'n8n-workflow';
const parametersDescription =
'JSON array of values to use for $1, $2, and so on, in order. Values can be strings, numbers, booleans, null, or arrays of these values. You can also use an expression that returns an array.';
export const nodeProperties: INodeProperties[] = [
{
displayName: 'Resource',
@@ -209,8 +212,23 @@ export const nodeProperties: INodeProperties[] = [
rows: 2,
},
default: '{}',
placeholder: '{ "field": -1 }',
description: 'A JSON that defines the sort order of the result set',
placeholder: '{ "field1": 1, "$1": -1 }',
hint: 'Use sort parameters to bind dynamic field names and values',
description:
'A JSON that defines the sort order of the result set. Use $1, $2, and so on as complete field names or values to reference Sort Parameters.',
},
{
displayName: 'Sort Parameters',
name: 'sortParameters',
type: 'json',
typeOptions: {
rows: 2,
},
default: '=[]',
placeholder: '["fieldName"]',
validateType: 'array',
description: parametersDescription,
hint: 'For example, ["name"] replaces $1 with "name", sorting descending by that field',
},
{
displayName: 'Projection (JSON Format)',
@@ -220,9 +238,23 @@ export const nodeProperties: INodeProperties[] = [
rows: 4,
},
default: '{}',
placeholder: '{ "_id": 0, "field": 1 }',
placeholder: '{ "_id": 0, "$1": 1 }',
hint: 'Use projection parameters to bind dynamic field names and values',
description:
'A JSON that defines a selection of fields to retrieve or exclude from the result set',
'A JSON that defines a selection of fields to retrieve or exclude from the result set. Use $1, $2, and so on as complete field names or values to reference Projection Parameters.',
},
{
displayName: 'Projection Parameters',
name: 'projectionParameters',
type: 'json',
typeOptions: {
rows: 2,
},
default: '=[]',
placeholder: '["fieldName"]',
validateType: 'array',
description: parametersDescription,
hint: 'For example, ["name"] replaces $1 with "name", returning only that field',
},
],
},
@@ -259,11 +291,10 @@ export const nodeProperties: INodeProperties[] = [
resource: ['document'],
},
},
default: '[]',
default: '=[]',
placeholder: '["value1", 123, true, null]',
validateType: 'array',
description:
'JSON array of values to use for $1, $2, and so on, in order. Values can be strings, numbers, booleans, null, or arrays of these values. You can also use an expression that returns an array.',
description: parametersDescription,
hint: 'For example, ["Alice", 30] replaces $1 with "Alice" and $2 with 30',
},
@@ -404,7 +435,27 @@ export const nodeProperties: INodeProperties[] = [
hint: 'Learn more about search index definitions <a href="https://www.mongodb.com/docs/atlas/atlas-search/index-definitions/">here</a>',
default: '{}',
required: true,
description: 'The search index definition',
description:
'The search index definition. Use $1, $2, and so on as complete field names or values to reference Index Definition Parameters below.',
},
{
displayName: 'Index Definition Parameters',
name: 'indexDefinitionParameters',
type: 'json',
typeOptions: {
rows: 2,
},
displayOptions: {
show: {
operation: ['createSearchIndex', 'updateSearchIndex'],
resource: ['searchIndexes'],
},
},
default: '=[]',
placeholder: '["value1", 123, true, null]',
validateType: 'array',
description: parametersDescription,
hint: 'For example, ["embedding", 1536] replaces $1 with "embedding" and $2 with 1536',
},
{
displayName: 'Index Type',
@@ -3,6 +3,7 @@ import { mockDeep } from 'vitest-mock-extended';
import { Collection, Db, MongoBulkWriteError, MongoClient, ObjectId } from 'mongodb';
import { constructExecutionMetaData, returnJsonArray } from 'n8n-core';
import type {
IDataObject,
IExecuteFunctions,
INode,
INodeParameters,
@@ -130,7 +131,7 @@ function mockExecuteFunctions(typeVersion: number, operation: string) {
return executeFunctions;
}
function mockQueryOperation(operation: 'aggregate' | 'delete' | 'find') {
function mockQueryOperation(operation: 'aggregate' | 'delete' | 'find', options: IDataObject = {}) {
const executeFunctions = mockExecuteFunctions(1.3, operation);
executeFunctions.getInputData.mockReturnValue([inputItems[0]]);
executeFunctions.getNodeParameter.mockImplementation(
@@ -147,7 +148,7 @@ function mockQueryOperation(operation: 'aggregate' | 'delete' | 'find') {
case 'queryParameters':
return ['Alice', 30];
case 'options':
return {};
return options;
default:
return fallbackValue;
}
@@ -157,6 +158,26 @@ function mockQueryOperation(operation: 'aggregate' | 'delete' | 'find') {
return executeFunctions;
}
function mockFindCursor() {
const applied: { sort?: unknown; project?: unknown } = {};
const cursor = {
skip: () => cursor,
limit: () => cursor,
sort: (value: unknown) => {
applied.sort = value;
return cursor;
},
project: (value: unknown) => {
applied.project = value;
return cursor;
},
toArray: async () => [],
};
vi.spyOn(Collection.prototype, 'find').mockReturnValue(cursor as never);
return applied;
}
function collectionNames(collectionSpy: MockInstance): string[] {
return collectionSpy.mock.calls.reduce<string[]>((names, call) => {
const [collectionName] = call as unknown[];
@@ -487,6 +508,85 @@ describe('MongoDB CRUD Node', () => {
expect(aggregateSpy).toHaveBeenCalledWith([{ $match: expectedQuery }]);
});
it('resolves sort parameters into the sort field name', async () => {
const applied = mockFindCursor();
await node.execute.call(
mockQueryOperation('find', { sort: '{ "$1": -1 }', sortParameters: ['name'] }),
);
expect(applied.sort).toEqual({ name: -1 });
});
it('resolves projection parameters into the projected field name', async () => {
const applied = mockFindCursor();
await node.execute.call(
mockQueryOperation('find', {
projection: '{ "_id": 0, "$1": 1 }',
projectionParameters: ['name'],
}),
);
expect(applied.project).toEqual({ _id: 0, name: 1 });
});
it.each([
{
field: 'sort',
options: { sort: '{ "$1": -1 }', sortParameters: ['name":-1,"_id'] },
expected: { 'name":-1,"_id': -1 },
},
{
field: 'projection',
options: {
projection: '{ "_id": 0, "$1": 1 }',
projectionParameters: ['name":1,"password_hash'],
},
expected: { _id: 0, 'name":1,"password_hash': 1 },
},
])('keeps a $field parameter to a single field', async ({ field, options, expected }) => {
const applied = mockFindCursor();
await node.execute.call(mockQueryOperation('find', options));
expect(field === 'sort' ? applied.sort : applied.project).toEqual(expected);
});
it.each(['sort', 'projection'])(
'rejects a %s parameter that collides with a configured field',
async (field) => {
mockFindCursor();
await expect(
node.execute.call(
mockQueryOperation('find', {
[field]: '{ "_id": 0, "$1": 1 }',
[`${field}Parameters`]: ['_id'],
}),
),
).rejects.toThrow('"_id" is used more than once');
},
);
it.each([
{ field: 'sort', parameter: '$where' },
{ field: 'sort', parameter: 'constructor' },
{ field: 'projection', parameter: '$where' },
{ field: 'projection', parameter: 'constructor' },
])('rejects $parameter bound to a $field field name', async ({ field, parameter }) => {
mockFindCursor();
await expect(
node.execute.call(
mockQueryOperation('find', {
[field]: '{ "$1": 1 }',
[`${field}Parameters`]: [parameter],
}),
),
).rejects.toThrow('is not a valid field name');
});
});
it('groups insert items by collection and uses insertMany per group', async () => {
@@ -26,12 +26,68 @@ describe('parseAndResolveQueryParameters', () => {
});
});
it('only replaces complete values, not keys or parts of strings', () => {
const query = JSON.stringify({ $1: 'key', exact: '$1', partial: 'user-$1' });
it('only replaces complete values, not parts of strings', () => {
const query = JSON.stringify({ exact: '$1', partial: 'user-$1' });
const result = parseAndResolveQueryParameters(query, ['Alice'], mockNode, 0);
expect(result).toEqual({ $1: 'key', exact: 'Alice', partial: 'user-$1' });
expect(result).toEqual({ exact: 'Alice', partial: 'user-$1' });
});
it('replaces placeholders that make up a complete key', () => {
const query = JSON.stringify({ _id: 0, $1: 1, 'nested.$1': 1 });
const result = parseAndResolveQueryParameters(query, ['name'], mockNode, 0);
expect(result).toEqual({ _id: 0, name: 1, 'nested.$1': 1 });
});
it('keeps a parameter bound to a key as a single field name', () => {
const query = JSON.stringify({ _id: 0, $1: 1 });
const result = parseAndResolveQueryParameters(query, ['name":1,"password_hash'], mockNode, 0);
expect(result).toEqual({ _id: 0, 'name":1,"password_hash': 1 });
});
it.each(['$where', '', 30, 'constructor', '__proto__', 'prototype'])(
'throws when %p is bound to a key',
(parameter) => {
expect(() => parseAndResolveQueryParameters('{ "$1": 1 }', [parameter], mockNode, 0)).toThrow(
'Query placeholder $1 is not a valid field name',
);
},
);
it('throws when a bound field name collides with a field the author wrote', () => {
const query = JSON.stringify({ tenantId: 'acme', $1: '$2' });
expect(() => parseAndResolveQueryParameters(query, ['tenantId', 'beta'], mockNode, 0)).toThrow(
'Query field name "tenantId" is used more than once',
);
});
it('throws when two bound field names collide', () => {
expect(() =>
parseAndResolveQueryParameters('{ "$1": 1, "$2": 2 }', ['name', 'name'], mockNode, 0),
).toThrow('Query field name "name" is used more than once');
});
it('allows the same field name at different nesting levels', () => {
const query = JSON.stringify({ $and: [{ $1: 'a' }, { $2: 'b' }] });
const result = parseAndResolveQueryParameters(query, ['name', 'name'], mockNode, 0);
expect(result).toEqual({ $and: [{ name: 'a' }, { name: 'b' }] });
});
it('uses the given label in error messages', () => {
expect(() => parseAndResolveQueryParameters('{', [], mockNode, 0, 'Sort')).toThrow(
"Invalid JSON in 'Sort'",
);
expect(() => parseAndResolveQueryParameters('{}', '{}', mockNode, 0, 'Sort')).toThrow(
'Sort Parameters must be a JSON array',
);
});
it('treats a parameter containing JSON as a plain string value', () => {
+78 -23
View File
@@ -1,9 +1,11 @@
import { jsonParse, NodeOperationError } from 'n8n-workflow';
import { isSafeObjectProperty, jsonParse, NodeOperationError } from 'n8n-workflow';
import type { INode } from 'n8n-workflow';
type QueryParameterScalar = string | number | boolean | bigint | Date | null;
type QueryParameter = QueryParameterScalar | QueryParameterScalar[];
const PLACEHOLDER = /^\$(\d+)$/;
export function isScalarValue(value: unknown): value is QueryParameterScalar {
return (
value === null ||
@@ -19,6 +21,7 @@ function parseQueryParameters(
rawParameters: unknown,
node: INode,
itemIndex: number,
label: string,
): QueryParameter[] {
let parameters: unknown = rawParameters;
@@ -28,14 +31,14 @@ function parseQueryParameters(
} catch (error) {
throw new NodeOperationError(node, error as Error, {
itemIndex,
message: 'Query Parameters must be valid JSON',
message: `${label} Parameters must be valid JSON`,
description: 'Enter the parameters as a JSON array',
});
}
}
if (!Array.isArray(parameters)) {
throw new NodeOperationError(node, 'Query Parameters must be a JSON array', {
throw new NodeOperationError(node, `${label} Parameters must be a JSON array`, {
itemIndex,
description: 'Enter the parameters as a JSON array',
});
@@ -48,7 +51,7 @@ function parseQueryParameters(
throw new NodeOperationError(
node,
`Query parameter ${index + 1} must be a scalar or an array of scalars`,
`${label} parameter ${index + 1} must be a scalar or an array of scalars`,
{
itemIndex,
description: 'Objects and nested arrays are not supported',
@@ -60,46 +63,98 @@ function parseQueryParameters(
/**
* Parses a JSON query and substitutes `$1`, `$2`, ... placeholders with the given parameters.
*
* Placeholders are only substituted when they make up a complete string value, so a parameter
* can never contribute structure (keys, operators, extra clauses) to the resulting query.
* Placeholders are only substituted when they make up a complete string value or a complete
* object key, so a parameter can never contribute structure (extra keys, operators, extra
* clauses) to the resulting query. A parameter bound to a key must be a plain, non-`$` string, so
* it can neither turn into an operator nor shadow a reserved object property such as `constructor`,
* and it must not collide with another field name in the same object, so it cannot replace a clause
* the author wrote.
*/
export function parseAndResolveQueryParameters(
query: string,
rawParameters: unknown,
node: INode,
itemIndex: number,
label = 'Query',
): unknown {
const parsedQuery = jsonParse<unknown>(query, {
errorMessage: "Invalid JSON in 'Query'",
errorMessage: `Invalid JSON in '${label}'`,
});
const parameters = parseQueryParameters(rawParameters, node, itemIndex);
const parameters = parseQueryParameters(rawParameters, node, itemIndex, label);
if (parameters.length === 0) return parsedQuery;
const usedParameters = new Set<number>();
const takeParameter = (placeholder: string, parameterIndex: number): QueryParameter => {
if (parameterIndex < 0 || parameterIndex >= parameters.length) {
throw new NodeOperationError(
node,
`${label} placeholder ${placeholder} has no matching value`,
{
itemIndex,
description: `Add a value for ${placeholder} to ${label} Parameters`,
},
);
}
usedParameters.add(parameterIndex);
return parameters[parameterIndex];
};
const resolveKey = (key: string): string => {
const match = PLACEHOLDER.exec(key);
if (!match) return key;
const value = takeParameter(key, Number(match[1]) - 1);
if (
typeof value !== 'string' ||
value.length === 0 ||
value.startsWith('$') ||
!isSafeObjectProperty(value)
) {
throw new NodeOperationError(node, `${label} placeholder ${key} is not a valid field name`, {
itemIndex,
description:
'A placeholder used as a field name must resolve to a non-empty string that does not start with "$" and does not name a reserved object property',
});
}
return value;
};
const resolveValue = (value: unknown): unknown => {
if (typeof value === 'string') {
const match = /^\$(\d+)$/.exec(value);
if (!match) return value;
const parameterIndex = Number(match[1]) - 1;
if (parameterIndex < 0 || parameterIndex >= parameters.length) {
throw new NodeOperationError(node, `Query placeholder ${value} has no matching value`, {
itemIndex,
description: `Add a value for ${value} to Query Parameters`,
});
}
usedParameters.add(parameterIndex);
return parameters[parameterIndex];
const match = PLACEHOLDER.exec(value);
return match ? takeParameter(value, Number(match[1]) - 1) : value;
}
if (Array.isArray(value)) return value.map(resolveValue);
if (value !== null && typeof value === 'object') {
const seenKeys = new Set<string>();
// Object.fromEntries would let a later key win silently, so a bound field name could
// replace a clause the author wrote. Reject the collision instead.
return Object.fromEntries(
Object.entries(value).map(([key, entry]) => [key, resolveValue(entry)]),
Object.entries(value).map(([key, entry]) => {
const resolvedKey = resolveKey(key);
if (seenKeys.has(resolvedKey)) {
throw new NodeOperationError(
node,
`${label} field name "${resolvedKey}" is used more than once`,
{
itemIndex,
description:
'A parameter bound to a field name must not collide with another field name in the same object, because one clause would silently replace the other',
},
);
}
seenKeys.add(resolvedKey);
return [resolvedKey, resolveValue(entry)];
}),
);
}
@@ -110,7 +165,7 @@ export function parseAndResolveQueryParameters(
const unusedParameter = parameters.findIndex((_, index) => !usedParameters.has(index));
if (unusedParameter !== -1) {
throw new NodeOperationError(node, `Query parameter ${unusedParameter + 1} is not used`, {
throw new NodeOperationError(node, `${label} parameter ${unusedParameter + 1} is not used`, {
itemIndex,
description: `Add $${unusedParameter + 1} to the query or remove the unused parameter`,
});