The custom-role migration converts every legacy Manager (atype 3) into a
Custom member (atype 4) with all collection flags false. collection_delete_access
only granted the per-collection (ManagedOnly) path to atype == Manager, so those
migrated members silently lost the ability to delete collections they still
manage via an explicit users_collections.manage / collections_groups.manage grant
(audit finding F-1).
Grant ManagedOnly to confirmed Custom members as well, but only when they hold
neither edit_any_collection nor access_all, so Edit any collection (which is
mirrored onto access_all) can never become a blanket delete. The downstream
is_coll_manageable_by_user check still requires a real per-collection Manage
grant, so a flagless Custom member without any assignment gains nothing; a
blanket "delete any collection" still requires the explicit delete_any_collection
permission handled by the Any branch.
Adds regression test migrated_legacy_manager_retains_managed_collection_delete.
Addresses the confirmed High/Medium findings of the granular-collection-
permissions security audit:
- H-1: bind direct (non-sync) cipher access to a confirmed membership in the
cipher's organization, and harden the user/group collection access-flag
queries to require org consistency plus confirmed status. Revoked/invited
members can no longer reach ciphers via stale assignment rows.
- H-2: validate group ids against the org before mutating group membership in
edit_member, and reject cross-org group<->membership links in GroupUser::save
and Group::is_in_full_access_group.
- H-3: fully pre-validate collections/groups/users in bulk-access and collection
create before any mutation, and reject cross-org collection<->group links in
CollectionGroup::save.
- M-1: require Manage Users for the full member list and Manage Users/Groups for
group details (new ManageUsersOrGroupsHeaders guard).
- M-2: validate the whole bulk-access request before the destructive
delete/replace so an invalid element can't leave partial state behind.
- M-3: bounds-validate import collection relationships before writing and
propagate cipher-save errors instead of discarding them.
A Custom member with the manage_users permission reaches edit_member via
ManageUsersHeaders. Every mutated field there (access_all, the manage_* flags,
collection and group assignments) is gated behind an Admin/collection-management
check -- except the role type itself, which was written unconditionally aside
from the existing Admin/Owner-elevation guard.
Because collection "manage" rights are role-derived (`atype >= Manager` grants
manage on any collection the member can write), a manage_users caller with no
collection access could promote a plain User to Manager/Custom to grant them
collection administration (rename/delete/re-share), or demote to revoke it --
a separation-of-duties break between the user-management and data planes.
Add a may_change_member_type() guard so callers below Admin may only submit an
unchanged role (keeping the regular edit dialog working), and cover it with a
regression unit test.
The manage flag in collection JSON was gated on atype == Manager, a raw
comparison that now excludes Custom (4). A Custom member holding an
explicit per-collection manage assignment (or full read/write access)
would have lost the manage capability in /sync and the collection
details endpoints - before this PR they were stored as Manager and
matched. Compare by access level (>= Manager, which Manager and Custom
share) instead, restoring the exact pre-PR behavior for migrated
members. Admins/Owners are unaffected: they are caught by the earlier
has_full_access / >= Admin arms.
The admin panel only knew types 0/1/2 plus the masqueraded "4":
Manager mapping. A Custom member would have been shown as "Manager"
with the Manager radio (value 3) preselected in the role dialog, so
saving it silently converted the member to Manager and wiped their
manage_* flags. Any remaining legacy Manager member (type 3) would
have thrown a TypeError and broken the dialog entirely.
Add a proper Custom (4) entry and radio button, and keep a Manager (3)
entry for members created through older clients.
Before this PR every member created with the Custom role was stored as
Manager (3) and masqueraded as Custom (4) in API responses. With the
masquerade removed, those members would suddenly surface as type 3,
which current clients no longer support and the admin panel cannot
render. Convert them to the now-persisted Custom type; access_all is
preserved and the new manage_* flags stay false, matching the exact
capabilities and appearance these members had before.
The down migration converts Custom members back to Manager so older
server versions (which cannot load type 4) keep working after a
rollback.
* Misc updates and fixes
- Updated Rust to v1.96.1
- Updated all the crates
- Updated GitHub Actions
- Updated the web-vault to v2026.6.2
- Updated Alpine to v3.24
- Fixed several clippy lints
- The `send` UUID wrappers didn't need the special namespace anymore since an updated crate, so removed this extra mod.
Signed-off-by: BlackDex <black.dex@gmail.com>
* Update MSRV to v1.94.1
Signed-off-by: BlackDex <black.dex@gmail.com>
---------
Signed-off-by: BlackDex <black.dex@gmail.com>
* ci: remove dead BASE_TAGS reference in release bake step
steps.determine-version doesn't exist in docker-build; the expression
resolves to empty string. The HCL default (testing) would have
applied, but it's moot - the bake uses push-by-digest=true so tags are
only set in merge-manifests. Dead code.
* ci: replace unsecured curl hadolint download with an official action
hadolint/hadolint-action uses a Docker-based runner with hadolint
pre-bundled in ghcr.io/hadolint/hadolint:v2.14.0-debian,so no binary
downloaded at runtime. Pinning the action to a commit SHA covers the
Dockerfile that specifies the image version, closing the supply-chain
gap from the previous unverified curl | sudo install.
Split {debian,alpine}: the action takes a single dockerfile argument,
so debian and alpine are linted separately.
* ci: pin ubuntu-latest to ubuntu-24.04 in merge-manifests and zizmor
ubuntu-latest is a moving target that can silently change the runner OS
on the next GitHub-side update. All other jobs in this repo already pin
to ubuntu-24.04; this makes merge-manifests and zizmor consistent.
* ci: return BASE_TAGS - it's needed for bake step
A Custom member with only the manage_groups permission could revoke other
members' collection access via two endpoints that were missing the
collection-access check enforced elsewhere (put_group_members, edit_member):
- POST /organizations/<org>/groups/<group>/delete-user/<member>
(post_delete_group_member) removed a member from any group, including
collection-bearing ones.
- DELETE /organizations/<org>/groups/<group> and its bulk variant
(delete_group_impl / bulk_delete_groups) deleted collection-bearing groups
outright, revoking access for all their members.
Neither path can grant access, so confidentiality was never at risk, but both
let a manage_groups-only user tamper with other members' collection access,
contradicting the permission's invariant. Both now require Admin/Owner or full
collection access before touching a group that confers collection access (via
access_all or assigned collections).
send_invite computed access_all from the invite permissions but never
wrote the manage_users / manage_groups / manage_policies flags onto the
new membership, so a Custom member invited with a management permission
checked was created without it (the web-vault sends everything in a
single invite POST, with no follow-up edit). Mirror the handling in
edit_member and persist the flags at invite time.
Only Owners can invite Custom members, so the caller is always
authorized to grant these; the flags are gated on the Custom type and
forced false for every other type.
A Custom member holding manage_users + manage_groups (but without collection
access) could add a member -- including themselves via edit_member, or an
attacker-controlled invitee via send_invite -- to an access_all or
collection-bearing group, and thereby indirectly gain read access to those
collections' cipher contents. This bypassed the collection-access boundary
already enforced in put_group_members and add_update_group.
Gate both member-centric group-assignment paths on collection-management
rights via a shared `may_change_group_membership` predicate: callers who
cannot manage collections may only add/remove membership of groups that
confer no collection access, and collection-bearing memberships are left
untouched (neither granted nor revoked). Admins/Owners and full-access
callers keep the previous full-replace behaviour.
Add a regression unit test for the predicate.
Tighten two custom-role read paths that were broader than intended:
- get_group, get_group_details and get_group_members only required
ManagerHeadersLoose, exposing group metadata, collection mappings and
membership to any confirmed Manager/Custom member. Require
ManageGroupsHeaders (Admin/Owner or manage_groups) instead.
- list_policies returned the full policy configuration to any manage_*
member. Keep the endpoint reachable so the Admin Console still loads,
but return an empty list to callers without manage_policies.
Security fixes and hardening following a review of the custom-role
permissions feature:
- Gate the manage_* flags on the Custom membership type via new
Membership::has_manage_* helpers. Previously a stale flag (e.g. left
over after changing a member's type through the admin panel, which
does not go through edit_member) would keep granting management
permissions to a member of any type.
- Clear the manage_* flags in admin::update_membership_type when a
member is changed away from the Custom type.
- edit_member: reject any *change* to the manage_* flags by non-admin
callers. This keeps the previous protection against granting flags,
and additionally prevents a manage_users member from silently
stripping flags an Admin/Owner granted to fellow Custom members.
Unchanged flags still pass, so the regular edit dialog keeps working.
- list_policies: restrict the full policy list to Admins/Owners and
Custom members holding at least one management permission. The
previous loosening to ManagerHeadersLoose also exposed all policies
(including their configuration data) to plain Managers.
- Collection list metadata (get_org_collections{,_details}) is now only
readable with manage_users or manage_groups; manage_policies does not
need the collection list.
- find_confirmed_and_manage_all_by_org: include Custom members with
access_all, matching the pre-existing behaviour for Managers.
- Fix the partial_cmp_MembershipType unit test (Custom is no longer
converted to Manager) and document that Manager and Custom share an
access level in Ord while remaining distinct roles under PartialEq.
- Replace the remaining stale "HACK" comments and the raw "4" type
comparison in send_invite; drop unrelated .gitignore entries.
The /api/sync response serialized a Send hide_email field directly from
Option<bool>, so a NULL value in the sends table (the column is
Nullable<Bool> with no default) produced "hideEmail": null.
The Bitwarden Android client deserializes SyncResponseJson.Send.hideEmail
as a non-null Kotlin Boolean and aborts the entire sync with a
JsonDecodingException when it encounters null. Web, desktop and CLI
clients coerce null to false, so only accounts with at least one Send
are affected and only on Android.
Default None to false at the serialization boundary, matching the
official Bitwarden server where hideEmail is non-nullable. This needs no
database migration and fixes both legacy NULL rows and any future NULLs.
The hide_email field stays Option<bool> internally.
A Custom member with only manage_users could set the "manage all
collections" child permissions (createNewCollections / editAnyCollection /
deleteAnyCollection) on any member, including themselves, via
POST /organizations/<org_id>/users/<member_id>. edit_member wrote
member_to_edit.access_all unconditionally, so the resulting access_all=true
granted full access to every collection's contents - defeating the
"manage users without collection access" guarantee.
Gate the access_all change on caller_can_manage_collections (Admins/Owners
or full-access members), mirroring how put_group preserves a group's
access_all for callers without collection rights. For everyone else the
member's existing access_all is left untouched. The collection- and
group-assignment paths were already gated; this closes the remaining
direct path.
- Updated Rust to v1.96.0
- Updated all the crates, and adjusted code where needed
- Fixed some nightly reported clippy lints
- Updated all the GitHub actions
Signed-off-by: BlackDex <black.dex@gmail.com>
* Update to Rust 2024 Edition
Updated to the Rust 2024 Edition and added and fixed several lint checks.
This is a large change which, because of the extra lints, added some possible fixes for issues.
Signed-off-by: BlackDex <black.dex@gmail.com>
* Reorder and merge imports
Signed-off-by: BlackDex <black.dex@gmail.com>
* Remove "db_run!" macro calls where possible
Signed-off-by: BlackDex <black.dex@gmail.com>
---------
Signed-off-by: BlackDex <black.dex@gmail.com>
* Update crates and gha
Updated all the crates
Updated GitHub Actions
Signed-off-by: BlackDex <black.dex@gmail.com>
* Fix restoring revoked user
A new endpoint is used to restore a revoked user.
This commit fixes that.
Fixes#7224
Signed-off-by: BlackDex <black.dex@gmail.com>
* Update datatables
Signed-off-by: BlackDex <black.dex@gmail.com>
---------
Signed-off-by: BlackDex <black.dex@gmail.com>
* Panic on unrecognised DATABASE_URL instead of silent SQLite fallback
Previously, any DATABASE_URL that did not match the mysql: or postgresql:
prefix was silently treated as a SQLite file path. This caused data loss
in containerised environments when the URL was misconfigured (typos,
quoting issues), as vaultwarden would create an ephemeral SQLite database
that was wiped on restart.
Now, an explicit sqlite:// prefix is supported and used as the default.
Bare paths without a recognised scheme are still accepted for backwards
compatibility, but only if the database file already exists. If not, the
process panics with a clear error message.
Relates to #2835, #1910, #860.
* Use err!() instead of panic!() for unrecognised DATABASE_URL
Follow the established codebase convention where configuration
validation errors use err!() to propagate gracefully, rather than
panic!(). The error propagates through from_config() and is caught
by create_db_pool() which logs and calls exit(1).
* Use 'scheme' instead of 'prefix' in DATABASE_URL messages
Per review feedback, 'scheme' is the more accurate term for the
sqlite:// portion of the URL.
* deps: upgrade the reqwest stack to 0.13
The reqwest 0.13 rustls feature selects the aws-lc provider. Use
rustls-no-provider instead, add rustls 0.23 with the ring provider, and
install that provider at process startup. This keeps Vaultwarden on the
existing ring crypto provider while giving reqwest, OpenDAL and lettre a
process-wide rustls provider.
Disable openidconnect default features and provide a small
AsyncHttpClient wrapper around Vaultwarden's shared reqwest client
builder. This preserves custom DNS, request blocking, timeouts and the
no-redirect OIDC behavior without openidconnect enabling its own reqwest
stack.
Upgrade yubico_ng to 0.15.0 and OpenDAL to 0.56.0. OpenDAL 0.56 also
moves S3 signing to reqsign 3, so switch the optional S3 dependencies
from reqsign/anyhow to reqsign-core and reqsign-aws-v4 and adapt the AWS
SDK credential bridge to the new ProvideCredential API.
Adjust the local OpenDAL call sites for the 0.56 API: use the FS_SCHEME
constant for filesystem checks and replace deprecated remove_all() with
delete_with(...).recursive(true) for Send file cleanup.
* storage: add OpenDAL S3 URI options
OpenDAL S3 storage accepts bucket and root path data today, but
serverless deployments also need URI query parameters to describe provider
behavior in one DATA_FOLDER value.
Update OpenDAL to 0.56.0 and build S3 operators with
S3Config::from_uri(). Keep Vaultwarden's AWS SDK credential chain by
installing a reqsign provider when the URI does not explicitly request
OpenDAL-native credential handling.
Move path handling and operator construction into storage.rs so S3-specific
parsing, credential setup, and URI path manipulation stay out of
configuration handling. Local filesystem behavior is unchanged, and S3
child paths are derived before query strings.
- Update crates including fixing a regression of Diesel
- Update web-vault to v2026.4.1
- Adjusted the README to address the secure context and needing HTTPS
Fixes#7132Closes#7137
Signed-off-by: BlackDex <black.dex@gmail.com>
Keeping the default behaviour of SQLite being built statically,
so as not to break anyone's workflow, but allowing for downstream
packagers to link dynamically against SQLite (where it's fine because
that's the point of package managers).
Note that SQLite is still *not* enabled by default, thanks to the `?` operator.
Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>
`Cipher::to_json()` returns `Result<Value, Error>` but its match arm for
unknown `atype` values called `panic!("Wrong type")` instead of
propagating an error. This means if a cipher with an invalid/unknown type
ends up in the database (via direct DB edits, data migration issues, or
future type additions in the upstream Bitwarden protocol), the entire
server process would crash on the next sync request.
Replace the `panic!` with `err!()` so callers receive a proper `Err` and
can handle or log it gracefully without taking down the server.
Co-authored-by: easonysliu <easonysliu@tencent.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>
The collection update endpoints (post_collections_update and
post_collections_admin) call .unwrap() on cipher.organization_uuid
in four places. If a user-owned cipher without an organization
somehow reaches these code paths, the server would panic.
Extract the organization UUID early with a descriptive error message
instead of relying on .unwrap(), preventing potential panics and
providing a clear API error response.
Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>
tom27052006
Mathijs van Veluw
Denis Pisarev
pilotstew
Timshel
kvdb
user71424q
Daniel
mfw78
Stefan Melmuk
Chase Douglas
Eldred Habert
eason
Johny Jiménez
Daniel García
Shocker