From 58200ce5a4f8e916ef1df93dacc99f1d26b1c526 Mon Sep 17 00:00:00 2001 From: Loc Nguyen <113083148+thailoc-dev@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:16:28 +0700 Subject: [PATCH] fix(mongodb): reject unsupported find() chain methods instead of dropping them --- .../src/lib/mongo/mongoShellCommand.ts | 74 ++++++++++++++++++- packages/app-tests/mongoShellCommand.test.ts | 43 +++++++++++ 2 files changed, 115 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/lib/mongo/mongoShellCommand.ts b/apps/desktop/src/lib/mongo/mongoShellCommand.ts index 85f89dd56d..7d94a9795b 100644 --- a/apps/desktop/src/lib/mongo/mongoShellCommand.ts +++ b/apps/desktop/src/lib/mongo/mongoShellCommand.ts @@ -160,6 +160,13 @@ function diagnoseMongoCommand(source: string): string | null { } const tail = source.slice(closeIndex + 1).trim(); + if (tail && method === "find") { + const chainProblem = describeUnsupportedFindChain(tail); + if (chainProblem) return chainProblem; + } + if (tail && (method === "findOne" || method === "countDocuments" || method === "count" || method === "distinct")) { + return `${method}() returns a result, not a cursor, so nothing can be chained after it: "${tail.length > 40 ? `${tail.slice(0, 40)}…` : tail}".`; + } if (tail) return `Unexpected text after ${method}(...): "${tail.length > 40 ? `${tail.slice(0, 40)}…` : tail}".`; if (method === "bulkWrite" && args[0]) { const operations = normalizeJsonArgument(args[0]); @@ -355,8 +362,11 @@ export function parseMongoFindCommand(input: string): MongoFindCommand | null { } const chain = source.slice(findCloseIndex + 1).trim(); - if (chain && !chain.startsWith(".")) return null; - if (findChainedMethodCallIndex(chain, "count") >= 0) return null; + // Every chained call must be one the editor executes; dropping an unknown one + // (`.hint()`, `.forEach()`, `.explain()`) would run a different query than written. + const calls = listChainedCalls(chain); + if (!calls || calls.some((call) => !FIND_CHAIN_METHODS.has(call.name) || (isNoopCursorMethod(call.name) && call.args.trim()))) return null; + if (calls.some((call) => call.name === "count")) return null; const sortArg = readChainedCallArgument(chain, "sort"); let sort: string | undefined; @@ -1554,6 +1564,66 @@ function readChainedCallArgument(source: string, name: string): string | undefin return undefined; } +/** Cursor methods the editor executes after `find()`. `toArray` / `pretty` change nothing and are dropped. */ +const FIND_CHAIN_METHODS = new Set(["sort", "skip", "limit", "collation", "count", "toArray", "pretty"]); + +function isNoopCursorMethod(name: string): boolean { + return name === "toArray" || name === "pretty"; +} + +interface ChainedCall { + name: string; + /** Raw text between the parentheses. */ + args: string; + /** Offset of the `.` within the chain text. */ + index: number; +} + +/** + * Tokenise `.name(args).name(args)…`. Returns null when the chain is not a sequence + * of calls, e.g. a property access such as `.count` without parentheses or trailing text. + */ +export function listChainedCalls(chain: string): ChainedCall[] | null { + const calls: ChainedCall[] = []; + let index = 0; + while (index < chain.length) { + const rest = chain.slice(index); + if (!rest.trim()) break; + const match = /^\s*\.\s*([A-Za-z_$][\w$]*)\s*\(/.exec(rest); + if (!match) return null; + const openIndex = index + match[0].length - 1; + const closeIndex = findMatchingParen(chain, openIndex); + if (closeIndex < 0) return null; + calls.push({ name: match[1]!, args: chain.slice(openIndex + 1, closeIndex), index: index + match[0].indexOf(".") }); + index = closeIndex + 1; + } + return calls; +} + +/** Why a `find()` chain was rejected, for the parse-failure diagnostics. */ +function describeUnsupportedFindChain(chain: string): string | null { + const calls = listChainedCalls(chain); + if (!calls) return null; + const offending = calls.find((call) => !FIND_CHAIN_METHODS.has(call.name) || (isNoopCursorMethod(call.name) && call.args.trim())); + if (!offending) return null; + const supported = "Supported after find(): sort, skip, limit, collation, count, toArray, pretty."; + switch (offending.name) { + case "explain": + return `find().explain() is not supported in the editor yet; aggregate([...], { explain: true }) is. ${supported}`; + case "itcount": + case "size": + return `find().${offending.name}() is not supported; use find().count() or countDocuments() to count results. ${supported}`; + case "forEach": + case "map": + return `find().${offending.name}() runs JavaScript, which the editor does not execute; run the find and read the results instead.`; + case "toArray": + case "pretty": + return `find().${offending.name}() takes no arguments.`; + default: + return `find().${offending.name}() is not supported yet. ${supported}`; + } +} + function hasSingleEmptyChainedCall(source: string, name: string): boolean { const trimmed = source.trim(); const match = chainedMethodCallPattern(name).exec(trimmed); diff --git a/packages/app-tests/mongoShellCommand.test.ts b/packages/app-tests/mongoShellCommand.test.ts index d3fec0f49e..cf054bb7ef 100644 --- a/packages/app-tests/mongoShellCommand.test.ts +++ b/packages/app-tests/mongoShellCommand.test.ts @@ -5,6 +5,7 @@ import { evaluateMongoAggregateSafety, evaluateMongoWriteSafety, mongoAggregateWriteStage, + listChainedCalls, mongoBulkWriteToQueryResult, mongoCollectionStatsToQueryResult, mongoCountToQueryResult, @@ -447,6 +448,48 @@ test("parseMongoFindCommand validates UUID strings at parse time", () => { assert.deepEqual(JSON.parse(command.filter), { u: { $uuid: "3B241101-E2BB-4255-8CAF-4136C566A962" } }); }); +test("parseMongoFindCommand rejects chained methods it would otherwise silently drop", () => { + // Each of these used to parse as a plain find and run a different query than written. + for (const [source, expected] of [ + ["db.c.find({}).hint({a: 1})", /find\(\)\.hint\(\) is not supported yet\. Supported after find\(\): sort, skip, limit, collation, count, toArray, pretty\./], + ["db.c.find({}).batchSize(10)", /batchSize\(\) is not supported yet/], + ["db.c.find({}).sort({a: 1}).maxTimeMS(100)", /maxTimeMS\(\) is not supported yet/], + ["db.c.find({}).explain('executionStats')", /find\(\)\.explain\(\) is not supported in the editor yet; aggregate\(\[\.\.\.\], \{ explain: true \}\) is/], + ["db.c.find({}).itcount()", /itcount\(\) is not supported; use find\(\)\.count\(\) or countDocuments\(\)/], + ["db.c.find({}).size()", /size\(\) is not supported; use find\(\)\.count\(\)/], + ["db.c.find({}).forEach(d => print(d))", /forEach\(\) runs JavaScript, which the editor does not execute/], + ["db.c.find({}).map(d => d.a)", /map\(\) runs JavaScript/], + ["db.c.find({}).toArray(1)", /toArray\(\) takes no arguments/], + ["db.c.find({}).count", /Unexpected text after find\(\.\.\.\): "\.count"/], + ] as const) { + assert.equal(parseMongoFindCommand(source), null, source); + assert.equal(parseMongoCommand(source), null, source); + assert.match(describeMongoCommandParseFailure(source), expected, source); + } + + // Non-cursor methods explain why nothing can follow them. + assert.match(describeMongoCommandParseFailure("db.c.findOne({}).limit(1)"), /findOne\(\) returns a result, not a cursor/); + assert.match(describeMongoCommandParseFailure("db.c.countDocuments({}).limit(1)"), /countDocuments\(\) returns a result, not a cursor/); +}); + +test("parseMongoFindCommand still accepts the chain methods it executes", () => { + assert.equal(parseMongoFindCommand("db.c.find({a: 1}).sort({b: -1}).skip(2).limit(5)")?.limit, 5); + assert.ok(parseMongoFindCommand("db.c.find({}).collation({locale: 'en'}).limit(1)")); + assert.ok(parseMongoFindCommand("db.c.find({a: 1}).sort({b: 1}).toArray()")); + assert.ok(parseMongoFindCommand("db.c.find({a: 1}).pretty()")); + assert.ok(parseMongoFindCommand("db.c.find({a: 1})\n .sort({b: 1})\n .limit(10)")); + assert.equal(parseMongoCommand("db.c.find({a: 1}).count()")?.command.kind, "countDocuments"); + assert.deepEqual( + listChainedCalls(".sort({b: 1}) . limit( 10 ).toArray()")?.map((call) => [call.name, call.args.trim()]), + [ + ["sort", "{b: 1}"], + ["limit", "10"], + ["toArray", ""], + ], + ); + assert.equal(listChainedCalls(".count"), null); +}); + test("parseMongoFindCommand accepts single-quoted string values and unquoted sort keys", () => { const command = parseMongoFindCommand("db.products.find({category: 'Electronics'}).sort({price: -1}).limit(2)"); assert.ok(command);