-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(query): make sanitizeProjection prevent projecting in paths deselected in the schema #14691
Conversation
…lected in the schema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hasezoey what do you think about this PR, is this a reasonable feature and do you think it's reasonable to ship in 8.5?
i think it is reasonable to ship it with 8.5.0.
i personally have not used this yet, but wouldnt this (according to the test cases) completely disable find().select("+field -field")
, or is this just for fields which have a schema select: false
?
as a side note, i had tried to search for option sanitizeProjection
in the documentation, and the only mention of this is in Query.prototype.setOptions
as The following options are only for find(), findOne(), findById(), findOneAndUpdate(), findOneAndReplace(), findOneAndDelete(), and findByIdAndUpdate():
, no documentation about what this option does.
Summary
sanitizeProjection option currently exists to prevent cases like
select({ name: '$password' })
, which would cause thename
property to contain the value of thepassword
property in newer versions of MongoDB.While that is helpful,
sanitizeProjection
can do a bit more to prevent inclusion of sensitive data when the projection is potentially untrusted. With this PR, ifsanitizeProjection
is enabled, there is no way to project in a field that's deselected withselect: false
in the schema definition. Ifpassword
has{ type: String, select: false }
andsanitizeProjection
is set, thenselect('+password')
,select('password')
, etc. will be ignored.@hasezoey what do you think about this PR, is this a reasonable feature and do you think it's reasonable to ship in 8.5?
Examples