[GH-ISSUE #7009] User account key rotation is not atomic #30082

New Issue

GiteaMirror · 2026-06-17T09:48:23-05:00

GiteaMirror commented

2026-06-17 09:48:23 -05:00

Originally created by @0x484558 on GitHub (Mar 24, 2026).
Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/7009

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:

Have an account with at least one folder, one emergency-access entry / org recovery key, and at least one send;
Trigger key rotation;
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);
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.

Originally created by @0x484558 on GitHub (Mar 24, 2026). Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/7009 - [x] I have searched the existing **Closed _AND_ Open** [Issues](https://github.com/dani-garcia/vaultwarden/issues?q=is%3Aissue%20) **_AND_** [Discussions](https://github.com/dani-garcia/vaultwarden/discussions?discussions_q=) - [x] I have searched and read the [documentation](https://github.com/dani-garcia/vaultwarden/wiki/) (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.

GiteaMirror added the enhancement label 2026-06-17 09:48:23 -05:00

GiteaMirror closed this issue

2026-06-17 09:48:24 -05:00

GiteaMirror commented

2026-06-17 09:48:24 -05:00

@BlackDex commented on GitHub (Mar 24, 2026):

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
235cf88231/src/api/core/accounts.rs (L810)

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.

@BlackDex commented on GitHub (Mar 24, 2026): 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 https://github.com/dani-garcia/vaultwarden/blob/235cf88231edff38a0afac9ce75835a8592f101a/src/api/core/accounts.rs#L810 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.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/vaultwarden#30082