Calling 'updateMany' without 'update' param leads to entire collection updated by the 'filter' param's data

[eyalk-af]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Mongoose version

6.11.3

Node.js version

15.14

MongoDB server version

4.4.10

Typescript version (if applicable)

4.3.5

Description

A severe, undocumented behavior: Model.updateMany is switching between filter and update params if update is missing.

The result is unpredicted and very destructive as mongoose updates ALL records of the collection with the filter parameter.

The bug is clearly stated (yet undocumented) in file query.js (line 4038)

    doc = conditions;
    conditions = undefined;

Important to note that MongoDB rejects calls to updateMany without an update parameter .

This is only a mongoose undocumented feature, thus a (severe) bug as it updates the database without discretion.

Steps to Reproduce

For a model named PersonModel, which represents a collection (Person) containing fields like color (a string)

Call this method:

PersonModel.updateMany({ color : 'red' }); // should not compile + throw runtime error

Expected Result:
A runtime error should be thrown stating that 'update' must have a value (exactly as MongoDB rejects this issue)

Actual Result:
All records in collection Person are updated with their color set to "red"

Expected Behavior

Mongoose should

1. Throw an error if this updateMany is called an undefined update paramter
2. Remove the optional mark (?) from parameter update in updateMany
3. document this behavior
4. It should never ever update the entire collection with the filter's data (!!)

[vkarpov15]

Hi, I'm very sorry for the trouble! It looks like this behavior was better documented in older versions of Mongoose, but we removed the examples that indicated this behavior when we removed support for callbacks.

You're right that this behavior is risky. However, I don't think we can change this behavior throughout without a breaking change because changing the equivalent code in updateOne() causes test failures. AFAIK the original intent of this syntax was to support chaining like Person.find({ name: 'John' }).updateMany({ color: 'red' }), see #3221 for an example. However, we should likely not support this behavior in Model.updateMany(), which would mean we can still support chaining syntax without the potential updateMany(update) footgun.