Replace organization_uuid unwrap with proper error handling

[xjohnyknox]

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

[BlackDex]

I'm not sure if this is really needed. It doesn't add any useful from my point of view, except maybe for some more nice messages. But since this is the case for more function besides only these, I'm leaning towards closing this as it will not be that beneficial in any way. But, if @dani-garcia finds this useful it can be added of course.

[tessus]

I think not crashing the vw service is quite useful. While I don't know how likely or unlikely a panic is in this case, unwrap should never be used in production without proper handling (or extensive comments). Just my 2 cents.

P.S.: I have just recently used unwrap, but tracked down that it cannot panic unless being run on an embedded system without home dirs and/or file systems. Thus I added a comment: cannot panic (with a link to a gh issue).

[BlackDex]

Well in this case it can't panic, as organization items require a collection, and there is already a check done to validate the user has access to the collections.

While I'm for trying to minimize unwrap, or even deny them at all. This pattern is located at several places which also need to change. This could be a start, but maybe more can be done.

[tessus]

Thanks for the explanation. In this case nothing further is required as you pointed out earlier. But please consider adding a comment in the source. Something like: "Cannot panic, checks done previously".

An unwrap (without proper handling) is a red flag, which can be easily dismissed with a proper comment.
I also use expect instead of unwrap. e.g. if the comment of the panic is "cannot panic - checks already done previously", then it is clear in case of a panic that the previous checks were not complete, in which case a panic might not even be a bad idea. (Although I would still prefer an error rather than the service to die.)

[dani-garcia]

Seems reasonable enough when the changes are so minimal, thanks!