Reject unrecognised DATABASE_URL instead of silent SQLite fallback#7061
Open
mfw78 wants to merge 3 commits intodani-garcia:mainfrom
Open
Reject unrecognised DATABASE_URL instead of silent SQLite fallback#7061mfw78 wants to merge 3 commits intodani-garcia:mainfrom
mfw78 wants to merge 3 commits intodani-garcia:mainfrom
Conversation
Contributor
|
May I ask why a panic and not a proper error message and clean exit with a specific error code? |
Author
|
Good catch, and fair point. The panic was a bit short-sighted on my part; this issue had actually bitten me personally so I was perhaps a touch too eager to make it loud. I've since had a proper look through the codebase and can see the established convention is to use I've pushed a follow-up commit that swaps the |
BlackDex
requested changes
Apr 9, 2026
Collaborator
BlackDex
left a comment
There was a problem hiding this comment.
Just a small text change. But i think that is all, the rest LGTM.
d095438 to
a86865d
Compare
BlackDex
approved these changes
Apr 10, 2026
a86865d to
5da1d40
Compare
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 dani-garcia#2835, dani-garcia#1910, dani-garcia#860.
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).
Per review feedback, 'scheme' is the more accurate term for the sqlite:// portion of the URL.
5da1d40 to
ac5d1f6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
DATABASE_URLdoes not matchmysql:orpostgresql:, vaultwarden silently treats it as a SQLite file path. Any misconfiguration (typo, quoting issue, garbage value) quietly creates an empty SQLite database instead of erroring. In containerised environments this means data loss on restart, which has bitten me personally and others (#2835, #1910, #860).The partial fix in #2873 catches some cases by checking the parent directory, but does not help when the path resolves within an existing directory.
The core issue is that SQLite selection is implicit. Database selection for a secrets manager should be explicit, not a fallback. An operator who misconfigures their database URL should get a clear error, not a silent downgrade to a different database engine entirely.
Changes
DbConnType::from_urlnow recognises an explicitsqlite:scheme. URLs without any recognised scheme are still accepted as bare-path SQLite for backwards compatibility, but only if the file already exists. Otherwise the process panics with a clear message.DATABASE_URLchanged tosqlite://{data_folder}/db.sqlite3so new deployments use the explicit scheme.backup_sqliteandvalidate_configupdated to handle thesqlite://prefix..env.templateupdated accordingly.Test plan
DATABASE_URL=data/db.sqlite3(file exists) still worksfoobar) panics with a helpful messagesqlite://prefixFixes #7062