User account key rotation is not atomic

[0x484558]

• I have searched the existing Closed AND Open Issues AND Discussions
• I have searched and read the documentation

(most of bug report form is irrelevant)

POST /accounts/key-management/rotate-user-account-keys currently performs a long sequence of database operations without a DB transaction and returns early on error. This allows partial commits across account data categories which can leave account state inconsistent. The endpoint is intended as all-or-nothing key rotation operation, and partial success means some encrypted payloads/key metadata are updated while others are not.

If my understanding of the code is correct, it is possible to run into an exact failure like this:

1. Have an account with at least one folder, one emergency-access entry / org recovery key, and at least one send;
2. Trigger key rotation;
3. Keep folder/emergency/recovery updates value, but set one send.deletionDate to an invalid (bear with me here) far-future value (e.g. now + 31 days);
4. Submit the request.

Rotate request will fail with 400, folder update will be committed anyway but user key material will not get updated, and send will remain unchanged.

This exact example relies on an invalid request as a precondition, which of course not something clients would normally do, but it is just one way to trigger all sorts of unpredictable consequences from user account keys rotation code path. Transient database errors and process interruptions also happen. The real issue here is lack of defensive programming.

I'm submitting this as an issue because unfortunately, it appears architecturally to be slightly non-trivial to fix.

[BlackDex]

I'm not sure if we need to keep this as an issue, we already know about this as there is a ToDo line regarding this exact item

vaultwarden/src/api/core/accounts.rs

Line 810 in 235cf88

// TODO: Ideally we'd do everything after this point in a single transaction.

It would more be an enhancement. And this call site isn't the only location which could benefit from running within a transaction, like purging the vault, import, and probably a few more.