Replace organization_uuid unwrap with proper error handling #64

New Issue

MrUnknownDE · 2026-04-05T20:29:28+02:00

MrUnknownDE commented

2026-04-05 20:29:28 +02:00

Originally created by @xjohnyknox on 3/13/2026

Summary

The collection update endpoints (post_collections_update and post_collections_admin) call .unwrap() on cipher.organization_uuid in four places. If a cipher without an organization UUID reaches these code paths, the server panics instead of returning an error.

Root cause

cipher.organization_uuid is Option<OrganizationId>. While the earlier is_in_editable_collection_by_user guard should prevent user-owned ciphers from reaching this code, the .unwrap() calls create a hard dependency on that guard — any future changes to the guard logic could silently introduce a panic path.

Fix

Extract organization_uuid early in each function using let Some(ref org_uuid) = ... else { err!(...) }, then reuse the binding throughout. This replaces four .unwrap() calls with a single, clear error return per function.

Test plan

Assign a cipher to a collection as a normal org user — should work as before
Update collections via admin endpoint — should work as before
Attempt to update collections on a cipher with no organization (if reachable) — should return error instead of panicking

*Originally created by @xjohnyknox on 3/13/2026* ## Summary The collection update endpoints (`post_collections_update` and `post_collections_admin`) call `.unwrap()` on `cipher.organization_uuid` in four places. If a cipher without an organization UUID reaches these code paths, the server panics instead of returning an error. ### Root cause `cipher.organization_uuid` is `Option<OrganizationId>`. While the earlier `is_in_editable_collection_by_user` guard should prevent user-owned ciphers from reaching this code, the `.unwrap()` calls create a hard dependency on that guard — any future changes to the guard logic could silently introduce a panic path. ### Fix Extract `organization_uuid` early in each function using `let Some(ref org_uuid) = ... else { err!(...) }`, then reuse the binding throughout. This replaces four `.unwrap()` calls with a single, clear error return per function. ## Test plan - [ ] Assign a cipher to a collection as a normal org user — should work as before - [ ] Update collections via admin endpoint — should work as before - [ ] Attempt to update collections on a cipher with no organization (if reachable) — should return error instead of panicking

Sign in to join this conversation.

No Label

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github/vaultwarden#64