Close a manage-grant laundering path and harden the rollback tooling

Follow-up to the previous two commits, all four issues were introduced by them:

- The legacy `access_all`-group authority was accepted by
  `caller_may_grant_collection_manage`, so a Custom member with manageGroups +
  editAnyCollection could create an access_all group holding itself, use the
  derived authority to persist `collections_groups.manage`, then leave the group
  and keep collection deletion without ever holding deleteAnyCollection. The
  clamp now takes only a real stored grant again, and the legacy exception is
  limited to members holding none of the three collection permissions -- which
  is the shape the migration leaves a group-derived Manager in, and never a
  member who can create such a group in the first place.
- `Collection::to_json_details` reported `manage: false` for a member reaching
  every collection even when a real grant existed, so the client rendered
  controls that disagree with the guards. Both the cached and the uncached path
  now read the effective grant.
- The SQLite rollback script continued after a failed statement, so a second run
  dropped `users_organizations` and committed an empty replacement. It now sets
  `.bail on`, refuses up front unless the permission columns are present, and
  the README documents `-bail` instead of claiming the run is atomic regardless.
- The downgrade acknowledgement was never consumed, so one approval authorized
  every later revert. It is now dropped by the revert it permits and cleared
  again on re-upgrade.

migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/down.sql

@@ -21,3 +21,7 @@ WHERE NOT EXISTS (
     WHERE table_schema = DATABASE() AND table_name = '__vw_allow_custom_role_downgrade'
 );
 DROP TABLE __vw_custom_role_downgrade_guard;
+
+-- Consume the acknowledgement: it authorized *this* revert, not every future one. After a
+-- re-upgrade the next revert has to be acknowledged again.
+DROP TABLE IF EXISTS __vw_allow_custom_role_downgrade;

migrations/mysql/2026-07-24-140000_guard_custom_role_downgrade/up.sql

@@ -5,3 +5,7 @@
 -- (2026-07-23 consumed the marker), so it is not left behind in every database. A single DDL
 -- statement is safe even on MySQL, where DDL commits implicitly -- re-running it is a no-op.
 DROP TABLE IF EXISTS __vw_custom_role_same_run_0716;
+
+-- Also clear a downgrade acknowledgement left over from an earlier revert, so consent is
+-- never inherited across an upgrade.
+DROP TABLE IF EXISTS __vw_allow_custom_role_downgrade;

migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/down.sql

@@ -18,3 +18,7 @@ INSERT INTO __vw_custom_role_downgrade_guard (blocked)
 SELECT 1
 WHERE to_regclass('__vw_allow_custom_role_downgrade') IS NULL;
 DROP TABLE __vw_custom_role_downgrade_guard;
+
+-- Consume the acknowledgement: it authorized *this* revert, not every future one. After a
+-- re-upgrade the next revert has to be acknowledged again.
+DROP TABLE IF EXISTS __vw_allow_custom_role_downgrade;

migrations/postgresql/2026-07-24-140000_guard_custom_role_downgrade/up.sql

@@ -5,3 +5,7 @@
 -- (2026-07-23 consumed the marker), so it is not left behind in every database. A single DDL
 -- statement is safe even on MySQL, where DDL commits implicitly -- re-running it is a no-op.
 DROP TABLE IF EXISTS __vw_custom_role_same_run_0716;
+
+-- Also clear a downgrade acknowledgement left over from an earlier revert, so consent is
+-- never inherited across an upgrade.
+DROP TABLE IF EXISTS __vw_allow_custom_role_downgrade;

migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/down.sql

@@ -21,3 +21,7 @@ WHERE NOT EXISTS (
     WHERE type = 'table' AND name = '__vw_allow_custom_role_downgrade'
 );
 DROP TABLE __vw_custom_role_downgrade_guard;
+
+-- Consume the acknowledgement: it authorized *this* revert, not every future one. After a
+-- re-upgrade the next revert has to be acknowledged again.
+DROP TABLE IF EXISTS __vw_allow_custom_role_downgrade;

migrations/sqlite/2026-07-24-140000_guard_custom_role_downgrade/up.sql

@@ -5,3 +5,7 @@
 -- (2026-07-23 consumed the marker), so it is not left behind in every database. A single DDL
 -- statement is safe even on MySQL, where DDL commits implicitly -- re-running it is a no-op.
 DROP TABLE IF EXISTS __vw_custom_role_same_run_0716;
+
+-- Also clear a downgrade acknowledgement left over from an earlier revert, so consent is
+-- never inherited across an upgrade.
+DROP TABLE IF EXISTS __vw_allow_custom_role_downgrade;

src/api/core/organizations.rs

@@ -3420,13 +3420,14 @@ async fn caller_may_grant_collection_manage(caller: &Membership, col_id: &Collec
     match caller_manage_grant_role_check(caller) {
         // Role alone decides it (Admin/Owner or delete_any -> yes; User/unknown/unconfirmed -> no).
         Some(decision) => decision,
-        // Custom without delete_any: the answer is per-collection and must mirror
-        // `collection_delete_access` exactly, so it can never hand out a right the caller lacks —
-        // a real users_collections.manage / collections_groups.manage grant, or the legacy
-        // organization-local `access_all` group that also confers deletion. Edit any collection
-        // deliberately does not count here.
+        // Custom without delete_any: the answer is per-collection and must reflect a *real* stored
+        // manage grant. Edit any collection deliberately does not count here, and neither does the
+        // legacy `access_all`-group authority: that one is derived from a group membership that can
+        // be taken away again, while a `manage` row written here outlives it. Accepting it would let
+        // temporary authority be laundered into a permanent grant — and with it collection deletion
+        // — which is exactly the escalation this clamp exists to prevent.
         None => match MembershipType::from_i32(caller.atype) {
-            Some(MembershipType::Custom) => caller.has_collection_manage_authority(col_id, conn).await,
+            Some(MembershipType::Custom) => caller.has_explicit_collection_manage_access(col_id, conn).await,
             _ => false,
         },
     }

src/db/models/collection.rs

@@ -131,37 +131,52 @@ impl Collection {
     ) -> Value {
         let (read_only, hide_passwords, manage) = if let Some(cipher_sync_data) = cipher_sync_data {
             match cipher_sync_data.members.get(&self.org_uuid) {
-                // Full collection visibility is not collection-management authority. Admins and
-                // Owners manage implicitly; Custom members still need an explicit stored grant.
-                Some(m) if m.has_full_access() => (false, false, assignment_manage_for_member(m.atype, false)),
                 Some(m) => {
+                    // What the client is told here has to match what the collection guards actually
+                    // allow, or it renders the wrong controls. A stored grant therefore counts even
+                    // for a member who already reaches every collection: full visibility is not
+                    // management authority, but it does not cancel out a real grant either.
+                    //
                     // A legacy organization-local `access_all` group confers collection management
                     // on its Custom members (see `has_legacy_group_collection_manage_access`), and
                     // reaches every collection without a `collections_groups` row that could carry
                     // the `manage` bit — so it has to be answered from the membership side.
                     let legacy_group_manage = m.has_type(MembershipType::Custom)
+                        && !m.has_create_new_collections()
+                        && !m.has_edit_any_collection()
+                        && !m.has_delete_any_collection()
                         && cipher_sync_data.user_group_full_access_for_organizations.contains(&self.org_uuid);
-                    if let Some(cu) = cipher_sync_data.user_collections.get(&self.uuid) {
-                        (
-                            cu.read_only,
-                            cu.hide_passwords,
-                            legacy_group_manage || assignment_manage_for_member(m.atype, cu.manage),
-                        )
-                    } else if let Some(cg) = cipher_sync_data.user_collections_groups.get(&self.uuid) {
-                        (
-                            cg.read_only,
-                            cg.hide_passwords,
-                            legacy_group_manage || assignment_manage_for_member(m.atype, cg.manage),
-                        )
-                    } else {
-                        (false, false, legacy_group_manage)
+                    let assignment = cipher_sync_data
+                        .user_collections
+                        .get(&self.uuid)
+                        .map(|cu| (cu.read_only, cu.hide_passwords, cu.manage))
+                        .or_else(|| {
+                            cipher_sync_data
+                                .user_collections_groups
+                                .get(&self.uuid)
+                                .map(|cg| (cg.read_only, cg.hide_passwords, cg.manage))
+                        });
+                    let stored_manage = assignment.is_some_and(|(_, _, manage)| manage);
+                    let manage = legacy_group_manage || assignment_manage_for_member(m.atype, stored_manage);
+                    match assignment {
+                        Some((read_only, hide_passwords, _)) if !m.has_full_access() => {
+                            (read_only, hide_passwords, manage)
+                        }
+                        // Reaching every collection means nothing is read-only or hidden here.
+                        _ => (false, false, manage),
                     }
                 }
                 _ => (true, true, false),
             }
         } else {
             match Membership::find_confirmed_by_user_and_org(user_uuid, &self.org_uuid, conn).await {
-                Some(m) if m.has_full_access() => (false, false, assignment_manage_for_member(m.atype, false)),
+                // Same rule as the cached branch above: a member who reaches every collection still
+                // reports a real stored grant, so the serialized value matches the guards.
+                Some(m) if m.has_full_access() => (
+                    false,
+                    false,
+                    assignment_manage_for_member(m.atype, m.has_collection_manage_authority(&self.uuid, conn).await),
+                ),
                 Some(m)
                     if m.atype >= MembershipType::Custom
                         && m.has_collection_manage_authority(&self.uuid, conn).await =>

src/db/models/organization.rs

@@ -976,11 +976,22 @@ impl Membership {
     ///
     /// Deliberately not collection *creation*: that historically required membership-level
     /// `access_all` and is now the independent `create_new_collections` permission.
+    ///
+    /// Security: the exception is limited to members holding *none* of the three collection
+    /// permissions, which is exactly the shape the migration leaves a group-derived legacy Manager
+    /// in. Without that limit it would also cover a Custom member holding `edit_any_collection` —
+    /// and since `edit_any_collection` is what lets a caller create an `access_all` group in the
+    /// first place, such a member could grant themselves this authority and use it to persist a
+    /// real `collections_groups.manage` row, keeping collection deletion after leaving the group.
     pub async fn has_legacy_group_collection_manage_access(
         &self,
         collection_uuid: &CollectionId,
         conn: &DbConn,
     ) -> bool {
+        if self.create_new_collections || self.edit_any_collection || self.delete_any_collection {
+            return false;
+        }
+
         let membership_uuid = self.uuid.clone();
         let user_uuid = self.user_uuid.clone();
         let org_uuid = self.org_uuid.clone();

tools/custom_role_rollback/README.md

@@ -47,7 +47,7 @@ Stop every Vaultwarden instance and take a backup first. Then:
 
 ```bash
 # SQLite
-sqlite3 /path/to/data/db.sqlite3 < tools/custom_role_rollback/sqlite.sql
+sqlite3 -bail /path/to/data/db.sqlite3 < tools/custom_role_rollback/sqlite.sql
 
 # MySQL / MariaDB
 mysql -u <user> -p <database> < tools/custom_role_rollback/mysql.sql
@@ -56,10 +56,18 @@ mysql -u <user> -p <database> < tools/custom_role_rollback/mysql.sql
 psql -U <user> -d <database> -v ON_ERROR_STOP=1 -f tools/custom_role_rollback/postgresql.sql
 ```
 
-Run it exactly once — a second run fails because the permission columns are already gone. On
-MySQL/MariaDB the statements cannot be wrapped in a transaction (DDL commits implicitly there); if
-the script is interrupted, restore the backup and start over. SQLite and PostgreSQL apply the whole
-script atomically.
+Each script stops on its own if the database is not in the state it converts from, so running one
+twice is refused rather than half-applied.
+
+**Do not drop the `-bail` / `ON_ERROR_STOP=1` flags, and do not run these through a client that
+keeps going after a failed statement.** The sqlite3 shell continues after errors by default; the
+script sets `.bail on` itself, but that is a shell command a different runner will ignore. A runner
+that carries on past the failing statement would reach the `DROP TABLE` and commit an empty
+`users_organizations`.
+
+SQLite and PostgreSQL apply the script in a single transaction, so an aborted run leaves the
+database untouched. On MySQL/MariaDB the statements cannot be wrapped in a transaction (DDL commits
+implicitly there); if the script is interrupted, restore the backup and start over.
 
 Afterwards start the older Vaultwarden version. Upgrading again later re-applies the seven
 migrations from a clean state.

tools/custom_role_rollback/sqlite.sql

@@ -6,10 +6,30 @@
 -- script has to work on the same older system SQLite the forward migrations support. Rebuilding the
 -- table also recreates `access_all` and drops all nine permission columns in one step.
 
+-- Stop at the first error. Without this the sqlite3 shell keeps going after a failed statement,
+-- and a second run -- where the SELECT below can no longer see the permission columns -- would
+-- still reach DROP TABLE and commit an empty users_organizations. `.bail on` is a shell command;
+-- a runner that is not the sqlite3 CLI has to abort on the first error and roll back by itself.
+.bail on
+
 PRAGMA foreign_keys = OFF;
 
 BEGIN;
 
+-- Refuse to start at all unless the database is in the state this script converts *from*. A repeat
+-- run would otherwise only fail somewhere in the middle. The failing CHECK names the reason.
+CREATE TEMPORARY TABLE __vw_rollback_precondition (
+    ok INTEGER NOT NULL CONSTRAINT
+        this_database_has_no_custom_role_permission_columns_to_roll_back CHECK (ok = 1)
+);
+INSERT INTO __vw_rollback_precondition (ok)
+SELECT CASE
+    WHEN EXISTS (SELECT 1 FROM pragma_table_info('users_organizations') WHERE name = 'create_new_collections')
+    THEN 1
+    ELSE 0
+END;
+DROP TABLE __vw_rollback_precondition;
+
 CREATE TABLE users_organizations_rollback (
   uuid       TEXT    NOT NULL PRIMARY KEY,
   user_uuid  TEXT    NOT NULL REFERENCES users (uuid),