refactor: Upgrade mongodb from 4.10.0 to 5.3.0 #8560
Conversation
Snyk has created this PR to upgrade mongodb from 4.10.0 to 5.3.0. See this package in npm: https://www.npmjs.com/package/mongodb See this project in Snyk: https://app.snyk.io/org/acinader/project/21343059-02d9-4182-87d7-718a44b181ef?utm_source=github&utm_medium=referral&page=upgrade-pr
Thanks for opening this pull request! |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## alpha #8560 +/- ##
==========================================
- Coverage 94.44% 94.42% -0.02%
==========================================
Files 183 183
Lines 14594 14593 -1
==========================================
- Hits 13783 13780 -3
- Misses 811 813 +2
☔ View full report in Codecov by Sentry. |
| @@ -457,6 +457,15 @@ const parseObjectKeyValueToMongoObjectKeyValue = (restKey, restValue, schema) => | |||
| ); | |||
| } | |||
| value = mapValues(restValue, transformInteriorValue); | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
The updated mongodb seems to require a bson version when writing bson directly to the database
There was a problem hiding this comment.
Do you have a docs reference? We may need to expose this in the DB adapter options if it should be user-configurable.
There was a problem hiding this comment.
The code that fails is:
const params = {
headers: headers,
method: 'POST',
url: 'http://localhost:8378/1/classes/RCE',
body: JSON.stringify({
obj: {
_bsontype: 'Code',
code: 'delete Object.prototype.evalFunctions',
},
}),
};
const response = await request(params).catch(e => e);With the error:
Unsupported BSON version, bson types must be from bson 5.
As obj[Symbol.for('@@mdb.bson.version')] is undefined
There was a problem hiding this comment.
So we are changing the product code only because of the tests in vulnerabilities.spec.js? I know we have some code parts where we do something like this, but this seems rather hacky, as if it could break if the MongoDB Node.js adapter changed internally, for example renames its internal symbols. Is there a way to add this in the tests?
There was a problem hiding this comment.
I've updated the code so that if _bsontype is specified, it is converted to a bson code. this makes the tests pass, but it will mean that support for sending other bson types will need to be added in time (as we don't support nested $)
There was a problem hiding this comment.
Not sure what the implication of this is. Could you give an example of what kind of query currently works but would not work anymore after this PR has been merged? We don't seem to have anything like that in our tests, since they pass, but the MongoDB adapter is versatile and I think there are vast areas we do not cover in tests but are legitimate and possibly frequent uses; for example BSON types can be used in aggregation pipeline.
There was a problem hiding this comment.
How do you think we should approach this then? I don't think it's too big of a risk as custom _bsontypes have been the cause of a few vulnerabilities here
There was a problem hiding this comment.
So you mean it's still possible to use bson types in the aggregation pipeline? Could you give an example of what would not work anymore? Then we can add this at least to the changelog as a note.
|
👋 it would be awesome to get this merged. We've been having some issues with the older Mongo driver and think the updated mongo driver would fix it. What else is left to do before we can merge this? |
|
We are working on it, see #8761 (comment) |
|
Closing via #8761 |
Pull Request
Issue
Closes: #8550
Approach
Tasks