Fix: Add strictFilter option to findOneAndUpdate (#14913)

[muazahmed-dev]

Summary

This PR addresses Issue #14913 by adding a strictFilter option to findOneAndUpdate. When enabled (strictFilter: true), it throws an error if the filter is empty ({}), preventing unintended updates to the first document in the collection. The default behavior (strictFilter: false or unset) remains unchanged to avoid breaking existing applications.

Motivation: In applications where data integrity is critical (e.g., financial or user data systems), an empty filter in findOneAndUpdate can accidentally update the wrong document. The strictFilter option enhances safety by requiring an explicit filter, reducing the risk of such errors. This change aligns with Mongoose’s goal of providing robust tools for MongoDB interactions.

Changes:

• Added strictFilter option to findOneAndUpdate in lib/query.js with updated JSDoc.
• Added 4 test cases in test/query.test.js covering empty filter, non-empty filter, undefined filter, and default behavior.
• Verified with MongoDB 8.0.4; behavior consistent with MongoDB 6.8.0 as reported.
• All tests passing (277 total, including 4 new tests).
• Confirmed behavior with a reproduction script (reproduce_error.js) using local Mongoose changes.

Examples

The following example demonstrates the strictFilter behavior:

const mongoose = require('mongoose');
mongoose.connect('mongodb://localhost:27017/testdb');

const PersonSchema = new mongoose.Schema({ name: String, email: String });
const Person = mongoose.model('Person', PersonSchema);

async function run() {
  await Person.deleteMany({});
  await Person.create([
    { name: 'Alice', email: '[email protected]' },
    { name: 'Bob', email: '[email protected]' }
  ]);

  try {
    // This will throw an error with strictFilter: true
    await Person.findOneAndUpdate(
      {}, // Empty filter
      { name: 'Updated' },
      { strictFilter: true }
    );
  } catch (err) {
    console.log('Error:', err.message);
    // Output: Error: Empty filter not allowed in findOneAndUpdate with strictFilter enabled
  }

  // Default behavior (no error, updates first document)
  const updated = await Person.findOneAndUpdate(
    {},
    { name: 'Updated' },
    { new: true }
  );
  console.log('Updated:', updated.name); // Output: Updated: Updated

  await mongoose.disconnect();
}

run();

[Copilot]

Pull Request Overview

This PR introduces a new strictFilter option to findOneAndUpdate to prevent accidental updates when an empty filter is provided. Key changes include:

• Adding strictFilter logic and JSDoc updates in lib/query.js.
• Introducing four new test cases in test/query.test.js to validate the behavior with empty, non-empty, undefined filters, and the default case.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
lib/query.js	Added strictFilter check that rejects empty filters and updated the JSDoc.
test/query.test.js	Added test cases covering strictFilter behavior.

Comments suppressed due to low confidence (1)

test/query.test.js:4501

• [nitpick] Consider adding additional tests for cases where the filter is null or a non-object value, to ensure that the strictFilter option behaves as intended.

it('handles undefined filter with strictFilter true', async function() {

[Copilot]

Consider verifying that 'filter' is a plain object before checking its keys (e.g. using typeof filter === 'object' and filter !== null) to ensure that non-object inputs do not bypass the strictFilter check.

Suggested change

	if (options && options.strictFilter && filter && Object.keys(filter).length === 0) {
	if (options && options.strictFilter && typeof filter === 'object' && filter !== null && !Array.isArray(filter) && Object.keys(filter).length === 0) {

[vkarpov15]

A couple of suggestions. Also, this option should ideally behave similarly for other update and delete functions:

• findOneAndDelete
• findOneAndReplace
• updateOne
• updateMany
• deleteOne
• deleteMany

Overall I like the idea of this option, throwing an error if the final filter is empty is useful. But the implementation needs some work.

[vkarpov15]

I don't think this is the right place for this check because of query chaining. For example, await TestModel.findOneAndUpdate({}, { isPublic: true }).setQuery({ isPublic: false }) will execute with a non-empty filter even though the original findOneAndUpdate() call had an empty filter.

[vkarpov15]

I don't think strictFilter is the right name for this option. strictFilter could misleadingly imply this option has similar semantics to strict or strictQuery, which are about schema-enforced structure, not whether filter exists.

I'd recommend renaming to requireFilter.

[muazahmed-dev]

@vkarpov15 Thank you for the detailed feedback and for supporting the requireFilter feature! I appreciate the suggestions to improve the implementation and ensure consistency across Mongoose’s update and delete methods. I also value the Copilot review’s input on handling edge cases.

I’ll address the following in the next 1-2 days:

• Rename strictFilter to requireFilter to avoid confusion with strict or strictQuery.
• Move the filter check to internal methods (e.g., _findOneAndUpdate) to support query chaining, as you pointed out.
• Extend requireFilter to findOneAndDelete, findOneAndReplace, updateOne, updateMany, deleteOne, and deleteMany.
• Add validation for null, non-object, and array filters in lib/query.js, as suggested by Copilot.
• Add test cases for edge cases (e.g., null, non-object filters) in test/query.test.js for all affected methods.
• Update JSDoc and the PR description to reflect these changes.

I’ll verify the updates with tests and my reproduce_error.js script, ensuring compatibility with MongoDB 8.0.4 and 6.8.0. I’ll push the changes soon and notify you for further review. Please let me know if there’s anything else to prioritize in the meantime. Thanks again!

[muazahmed-dev]

@vkarpov15 The requireFilter implementation is complete and ready for review:

• Extended requireFilter to findOneAndUpdate, findOneAndReplace, replaceOne, findOneAndDelete, updateOne, updateMany, deleteOne, and deleteMany.
• Moved checks to internal methods (e.g., _findOneAndUpdate, _updateThunk) to support query chaining.
• Renamed from strictFilter to requireFilter.
• Validated null, non-object, and array filters.
• Added 35 unit tests in test/query.test.js covering all cases.
• Verified with MongoDB 8.0.4; compatible with MongoDB 6.8.0.
• All 308 tests passing.

Please let me know if any further changes are needed. Thanks for the guidance!

[vkarpov15]

I have a couple of minor comments but looking good overall

[vkarpov15]

I don't think filter can be an array, so I would get rid of this Array.isArray(filter) check.

Also, it would likely be worthwhile to check for empty objects in $and, $or, and $nor as well. For example, deleteOne({ $and: [{}] }) and deleteOne({ $or: [{}] }) is equivalent to deleteOne({}). That would make requireFilter more valuable.

[vkarpov15]

Use Person = db.model() instead of mongoose.model() in these tests please, that's how the rest of our test cases work

[vkarpov15]

Don't call mongoose.connect() in tests. We use db.model() in tests throughout.

[muazahmed-dev]

@vkarpov15 Thanks for the feedback! I’ve addressed both points:

• Removed the Array.isArray(filter) check from checkRequireFilter in lib/query.js, as filters cannot be arrays.
• Added validation for empty objects in $and, $or, and $nor operators (e.g., { $and: [{}] }) in checkRequireFilter using a recursive isEmptyFilter function.
• Added 21 new unit tests (3 per method) in test/query.test.js to cover $and, $or, $nor with empty objects for all methods (findOneAndUpdate, findOneAndReplace, findOneAndDelete, updateOne, updateMany, deleteOne, deleteMany).
• Verified with reproduce_error.js and MongoDB 8.0.4; all 329 tests passing.

Please let me know if there’s anything else to address or refine. Thanks for the guidance!