Failing update operations with encrypted subdocument array on Mongoose 6

[yelworc]

I'm getting errors like this one when trying to update encrypted documents after upgrading to Mongoose 6:

MongoServerError: Updating the path 'subdocs' would create a conflict at 'subdocs'

The schema/model in question has encryption enabled for an entire field that contains an array of subdocuments, i.e. schema.plugin(encrypt, { /* keys… */, encryptedFields: ['subdocs'] }); This may not have been a good idea, but here we are 😄 (it's been in production like that for a long time).

I created a unit test that reproduces the problem. With Mongoose 5, the update operation results in this query:

Mongoose: parents.updateOne(
  { _id: ObjectId("6197884a403d8c659a0c1437") },
  {
    '$set': { _ac: 'BinData(0, "…")', _ct: 'BinData(0, "…")', _id: ObjectId("6197884a403d8c659a0c1437") },
    '$unset': { children: 1 }
  },
  { session: null }
)

whereas with Mongoose 6, it's:

Mongoose: parents.updateOne(
  { _id: new ObjectId("6197883f877393d38902437d"), __v: 0 },
  {
    '$unset': { 'children.0.text': 1, children: 1 },
    '$set': { _id: new ObjectId("6197883f877393d38902437d"), _ct: 'BinData(0, "…")', _ac: 'BinData(0, "…")' }},
  { session: null }
)

Most likely, the children.0.text in the $unset part of the query is the issue. That's about as far as I got – not sure where to go from here…

[spencersteers]

So I was able to get around this by calling unmarkModified on each modified path and then calling save:

doc.children[0].text = 'New unencrypted text';
doc.directModifiedPaths().forEach((path) => doc.unmarkModified(path));
doc.save();

I believe this is because mongoose v6 now uses proxies for arrays and uses them to track modifications.

[yelworc]

@spencersteers cool, thanks! That workaround seems to help in my case, too. Ideally, mongoose-encryption would take care of this internally… but I'm not sure whether that's even possible.

[joegoldbeck]

Thanks @yelworc for reporting this and for the excellent unit test! I agree that this should be taken care of within the plugin.

The unmarkModified trick seems a little risky to me in that there may be other side-effects – presumably mongoose (or other mongoose plugins?) use the modified path markers to do something..., right? At minimum (for any casual readers - you may have already done this), I'd suggest being specific about which fields to unmarkModified rather than simple doing it for all fields.

As to fixing it in the plugin, I do agree that should be done. I might not get to it for a bit since I'll first need to get up to date on whatever Mongoose is doing with its subdocuments these days and am not using it day-to-day anymore.

Definitely welcome any PRs if folks with a bit more familiarity want to take a stab. If you branch from @yelworc's excellent unit test then the CI pipeline will take care of testing across both Mongoose version 5 & 6, and also crediting appropriately.

Ideally there can be a simple fix, that just updates the handling of subdocs to match whatever Mongoose now expects, but maintains backward compatibility with Mongoose v5.

It does seem like unmarking modified could help, but again I'm not sure the repercussions, and importantly don't want to break scenarios in which a user might be using this alongside another mongoose plugin, or their own middleware, which certainly could take advantage of that. Maybe if we did it as the very last step before encrypting that would be safe...?

Welcome any thoughts on this as well!

[offaxis]

Any progress for this thread ? MongoDb Atlas will update to MongoDB 5 (witch is only compatible with Mongoose v6 => https://mongoosejs.com/docs/compatibility.html) on shared cluster next week...

[yelworc]

Tried my luck at a fix (see #119). Thanks for your feedback, @joegoldbeck – I share your concerns around unmarkModified, but since these fields should not be present in the persisted document to begin with, and we're setting them to undefined in the encrypt method anyway, I think we should be good… let me know what you think :)

I think this is the PR that introduced the new array change tracking in Mongoose; as far as I can see, there is no option to disable it (which might have been another solution to this problem).

[yelworc]

FWIW, I finally upgraded our production system to Mongoose 6 some 4 weeks ago, using the workaround/fix in #119 – no ugly surprises so far :smile:

EDIT: Moved to Mongoose 7 for a short bit, and now finally on Mongoose 8. Everything still seems to be fine.