Repository navigation
fix: create SQLite database files 0600 and set wider ones to 0600 when opened - #691
Open
LeeroyHannigan wants to merge 14 commits into
Open
LeeroyHannigan wants to merge 14 commits into
LeeroyHannigan wants to merge 14 commits into
Conversation
…QLite opens it `extenddb init --backend sqlite` left the database file and its -wal and -shm files at mode 0644. SQLite's unix VFS creates a database file with mode 0644 less the umask and gives each new sidecar the mode of the database file, and the bootstrapper pool that init and migrate use never changed either mode. The engine set the modes after connecting, on the configured string with a suffix appended, and ignored failures. Every pool opener on a file database (the bootstrapper, the engine and the catalog pool) now creates a missing database file with mode 0600 before it connects, when the connection would create the file, and sets the file and any -wal, -shm and -journal file present to 0600 before and after it connects. The sidecar names come from the resolved path, because SQLite follows symlinks before naming them. A mode that cannot be set is an error, as it is for the lock file. Other platforms are unchanged.
…fter init, serve and a write Unit tests for file_mode: prepare creates a missing database at 0600 and sets an existing database and its sidecars to 0600; a sqlite: URL without mode=rwc, or with mode=ro, creates nothing; a WAL pool opened after prepare gets 0600 sidecars from SQLite; a dangling symlink has its target created. An integration test binary sets a umask of 022 in its own process and runs the bootstrapper sequence init uses, the server components factory, a management write, CreateTable and TagResource, a clean stop and a second start, checking that every file in the deployment directory is 0600 with the WAL sidecars present while a server is up. It also covers a dev-mode serve on a missing path, and serve and migrate over files left at 0644. A control test shows a connection opened outside the crate's openers leaves 0644 files under that umask.
…ly connect The bootstrapper pool, the serve engine and the catalog pool now open their pools through file_mode::connect. It runs prepare before the pool's first connection and restrict after it, so an opener cannot run one step without the other. The new unit tests read the modes of the database file and its -wal and -shm files inside the connection setup, before restrict runs. A database left at 0644 with no sidecars gets 0600 sidecars only because prepare set it to 0600 before SQLite opened it.
…ons prepare leaves to SQLite A pool whose connection setup sets the database file and its sidecars to 0644 finds them at 0600 once connect returns. prepare creates nothing for a file: URI with its own query or for a VFS outside the unix family.
…ation A configured sqlite: URL names the same database file and sidecars as the path inside it. The engine's old post-connect chmod appended the suffixes to the configured string, which for a URL named files that do not exist.
The tests move unchanged from file_mode.rs to file_mode/tests.rs, so the module stays under 600 lines as the tests grow.
…gh a handle that follows no link The mode was set by path on whatever the database name and each sidecar name resolved to. A database path naming a directory set the directory to 0600, a symlink planted at a sidecar name set its target to 0600, and a read-only mount failed the read-only commands although every file was already 0600. Each name is now opened with O_PATH | O_NOFOLLOW on Linux, which opens no file for reading or writing: a symlink yields a handle on the link itself and a FIFO or a device is not opened, so nothing waits. The type and mode are read from that handle, and only a regular file with a permission bit outside 0600 is changed, by chmod of /proc/self/fd/<n>, so the check and the change act on the same file. Closing an O_PATH handle keeps the POSIX locks the process's SQLite connections hold on the file, which closing a descriptor opened for reading would release. On other Unix systems the type and mode come from lstat and the change is fchmodat with AT_SYMLINK_NOFOLLOW, which opens nothing. A regular file wider than 0600 that cannot be narrowed is still an error. The calls that change a mode can be replaced in tests, which use that to check the error without changing any file the test did not create.
…he locks of its open connection A WAL connection holds a read lock on the database file's shared range and on the DMS byte of the -shm file. The test narrows the files of a database whose connection is open and asks, with an open file description lock, whether a write lock on either range would still conflict.
…se file is created with The integration tests opened every pool in sequence and checked the end state, so a later opener's narrowing hid an opener that skipped its own. Three tests now open one pool each on a missing path and read the modes before any other pool opens the file: the bootstrapper's pool, the engine's pool and the catalog pool through the settings store factory. A unit test reads the mode of the database file prepare creates as the file is created, before prepare narrows it, and compares it with a file the test creates with mode 0600 under the same umask.
…ad them before creating the database prepare decided whether to create the database file, and which file, from a reading of the location that differed from sqlx's and SQLite's in four ways. With several vfs values it required all of them to be in the unix family, where sqlx uses the last. It left every file: URI with its own query to SQLite, which created the file at 0644 when that query did not narrow the open. database_file did not decode %HH in a file: URI path, so prepare created a stray file and the database SQLite opened stayed 0644; it matched mode=memory with a raw split, so an encoded key such as mod%65 made prepare create a file SQLite never used; and a file: name combined with vfs or immutable, which sqlx wraps in a second file: URI so that SQLite opens a relative path spelled file:..., got a stray file at the path the URI names. A new module, sqlite_uri, reads a file: URI as the bundled SQLite does: the authority, the path and query split at raw separators, %HH decoded, %00 ending a part, the last vfs, and each mode applied in order with the rule that a mode may not ask for more access than the one before it. prepare creates the file only when SQLite would create it on disk. database_file reads the path through sqlite_uri and mode through form_urlencoded, and file_mode leaves a file: name with vfs or immutable alone. The lock file moves only where the decoding changes the path: for a file: URI with %HH in its path it is now next to the file SQLite opens, and an encoded mode=memory takes no lock, as every other in-memory form.
…hen one cannot be set The security model left file permissions to the operator and did not mention the SQLite database files, and the troubleshooting guide described settings and verify as commands that only read. With the SQLite backend, init, migrate, settings, verify and serve now create the database file with mode 0600 and set it and its -wal, -shm and -journal files to 0600 when they are wider, and they fail when a mode cannot be set. The security model now names those files. The lock entry in the troubleshooting guide says that settings and verify set these modes although they take no lock. A new entry covers the errors "cannot set the mode of SQLite file" and "cannot set the mode of SQLite database" and their causes: a read-only mount holding files wider than 0600, and database files that belong to another user.
creates_file and the SQLITE_OPEN_* flags it reads are used only by the Unix file mode code, so a Windows build reported seven dead-code warnings for them. They are now compiled on Unix only, and the options field they read allows dead code on other platforms.
The SQLite file message also appears when the database file or one of its sidecars cannot be looked up, for example when the user cannot search the directory that holds the database. The SQLite database message comes from a filesystem that refuses to set the mode of the file the command created. The troubleshooting entry now says so, and lists the directory case with its fix.
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 31, 2026 12:01
5 of 8 tasks
Cargo.lock: main added tracing-subscriber to the dependencies of extenddb-storage-sqlite, and this branch added url. The merged file is main's, with url added back by cargo. crates/storage-sqlite/Cargo.toml: main added libsqlite3-sys and this branch added url. The merged file keeps both. crates/storage-sqlite/src/bootstrapper.rs: main replaced the pool's pragmas with set_connection_pragmas, and this branch opens the pool through file_mode::connect. The merged pool runs main's pragmas and opens through file_mode::connect. crates/storage-sqlite/src/lib.rs: main moved catalog_pool onto set_connection_pragmas, and this branch opens it through file_mode::connect and returns its error as a String. The merged catalog_pool keeps main's pragmas and this branch's opener. crates/storage-sqlite/src/store.rs: main moved the engine's pragmas onto set_connection_pragmas and dropped the CATALOG_VERSION import. This branch opens the engine's pool through file_mode::connect and removes the chmod loop and the sqlite_url import. The merged engine keeps main's pragmas and imports and this branch's opener, without the chmod loop. docs/troubleshooting.md: main names migrate --yes as the command that takes the lock and adds migrate without --yes to the read-only commands. This branch added a sentence on the modes settings and verify set. The merged entry keeps main's text with this branch's sentence at the end. Main moved migrate --yes, and the first open of a dev-mode serve, onto the migration runner's own connection, which this branch's openers do not cover. SqliteMigrations::open_with now calls file_mode::prepare and file_mode::restrict around that connection when it opens the database for writing, so migrate --yes sets files left at 0644 to 0600 as this branch describes. The read-only status connection is unchanged. file_mode::prepare and file_mode::restrict are pub(crate) for this. tests/file_modes.rs called bootstrapper methods that main removed. Its init step now follows main's init (the migrations under the lock, then the rows a server needs), its migrate step takes the lock and calls run_locked, and the bootstrapper pool test opens the pool through list_table_names. The new troubleshooting entry and the security model name migrate --yes where they named migrate, and say that a command that creates a missing database file creates it with mode 0600. On main, migrate and verify create no missing database file.
LeeroyHannigan
requested review from
eliseong,
hitesh96db and
navareamol
as code owners
November 5, 2026 01:49
This branch has not been deployed
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.
What
crates/storage-sqlite/src/file_mode.rsaddsconnect(location, SqlitePoolOptions), and the bootstrapper pool,catalog_poolandSqliteEngine::newnow open their pools through it. It runspreparebefore the pool's first connection andrestrictafter it. The migration runner opens its own connection when it writes: formigrate --yes, and for the first open of a dev-modeserve. It callsprepareandrestrictaround that connection. Its read-only connection for the catalog status changes no mode.preparecreates the database file with mode 0600 when the connection would create it, then narrows the file and any-wal,-shmor-journalfile left by an earlier run, so the sidecars SQLite creates on this connection take 0600 from the database file. The file is created withcreate_new, which refuses any existing entry, a symlink included. For a dangling symlink at the database name, the link's target is created.restrictnarrows the database file and each sidecar present again once the pool has connected.Which files are changed
O_NOFOLLOW, so if it needs that sidecar, its own open fails and the command reports SQLite's error.O_PATH | O_NOFOLLOW, which opens no file for reading or writing, so nothing waits on a FIFO or a device. The type and mode are read from that handle and the mode is changed withchmodof/proc/self/fd/<n>, so the check and the change act on the same file. Closing anO_PATHhandle keeps the POSIX locks the process's SQLite connections hold on the file. Closing a descriptor opened for reading releases them, which section 2.2 of How To Corrupt An SQLite Database File lists as a cause of corruption.serveopens the catalog pool while the engine's pool is connected, andconnectnarrows the files while its own pool is connected, so the narrowing runs while connections hold locks on the file.lstatand the mode is changed withfchmodat(AT_SYMLINK_NOFOLLOW), which opens nothing.cannot set the mode of SQLite file <path>: <error>. A database filepreparecreated whose mode cannot be set givescannot set the mode of SQLite database <path>: <error>. The.lockfile already follows this policy (fix(sqlite): one extenddb process per database file #391).Which file the connection opens
prepareandrestrictfind the file the connection opens, andpreparedecides whether the connection creates it, by reading the location as sqlx 0.8.6 and the bundled SQLite 3.46.0 read it. sqlx reads the connection string's query as form-urlencoded:mode=rwccreates unless amode=rois also given, and the lastvfsand the lastimmutablewin. Afile:URI's own query is read by a new module,crates/storage-sqlite/src/sqlite_uri.rs, which follows SQLite'ssqlite3ParseUri:%HHis decoded,?,#,&and=separate only when they appear raw, the lastvfswins, and eachmodeis applied in order. As in SQLite, amodethat asks for more access than the one before it fails the open, so no file is created. Afile:name combined withvfsorimmutableis left to the connection, because sqlx then wraps it so that SQLite opens a relative path spelledfile:....serve_lock::database_filereads afile:path throughsqlite_uriand findsmode=memorythrough the same form-urlencoded parse. For afile:URI with%HHin its path, the lock file is now next to the file SQLite opens, and an encodedmode=memory(for examplemod%65=memory) now takes no lock, like the other in-memory forms. For other locations the lock path stays where it was, and the existing lock path tests pass without changes.The chmod loop in
SqliteEngine::newis removed.catalog_poolreturns its error as aString,serve_lock::resolve_dangling_symlinksispub(crate), and the crate takes the workspaceurldependency, whichCargo.lockalready had through sqlx.docs/troubleshooting.mdgains an entry for the two errors and says thatsettingsandverifyset these modes, anddocs/manuals/10-security-model.mdnames the database files.Why
extenddb init --backend sqliteleaves the database file and its-waland-shmfiles at mode 0644, while the config file and the.lockfile are 0600. The database holds the encryption key next to the access-key secrets and password hashes it protects, so any local user can read them until the firstservenarrows the files.Measured on main (
1c0523c9) on 30 October 2026 with a binary built with--no-default-features --features sqlite, under umask 022, and for the first three items under umask 002 as well:init:extenddb.toml600,x.sqlite644,x.sqlite-wal644,x.sqlite-shm644,x.sqlite.lock600. Umask 002 gives 644 as well, because SQLite creates a database at 0644 less the umask.servestart all five files are 600. The serve engine set the modes after its pool connected, on the files present at that moment.migrateover a copy of the database at 0644 with no sidecars creates-waland-shmat 644, and over a copy at 0600 it creates them at 600. SQLite gives a new sidecar the mode of the database file.link.sqlite -> real.sqliteit usesreal.sqlite-walandreal.sqlite-shm.initwith asqlite:URL as the path also leaves the files at 644.The bootstrapper pool (
init,migrate,destroy) and the catalog pool (settings,verify, and the management and diagnostics stores) open the file without setting a mode. The serve engine set modes only after connecting. It appended-waland-shmto the configured string, which for asqlite:URL names files that do not exist, and it ignored errors.Closes #657
Testing done
Measured with a binary built from this branch on 31 October 2026, under umask 022 and 002: after
initall five files are 600, and they stay 600 through the firstserve, management writes, CreateTable, PutItem, a clean stop, a secondserveandmigrate.migrateover copies at 0600 and 0644 with no sidecars leaves the database and its new sidecars at 600.Unit and integration tests
Unit tests in
crates/storage-sqlite/src/file_mode/tests.rs, independent of the test process's umask:sqlite:URL. Insideafter_connect, beforerestrictruns, the database and the sidecars SQLite created are 0600, for a missing database and for an existing 0644 one. Files set to 0644 insideafter_connectare 0600 onceconnectreturns./. A symlink to a 0644 file at-journal, a FIFO at-waland a socket node at-shmkeep theirs,prepareandrestricteach return within ten seconds, and the link's target stays 0644.EROFS,prepareandrestrictsucceed on files at 0600 and 0400 without the call being made, and a 0644 file gives an error that names the file and stays at 0644. The test puts the failing call in place of the system call, so it changes no file outside its own temporary directory, also when it runs as root.restrict. Afterwards anF_OFD_GETLKquery still finds the connection's read lock on the database file's shared range and on the-shmfile's DMS byte.sqlite:///pathwithout a query,mode=ro,mode=rwc&mode=ro,vfs=memdb,:memory:, an encodedmod%65=memory, afile:name withvfsorimmutable, andfile:URIs whose own query does not create (mode=ro,mode=rw,mode=memory,vfs=memdb, andmode=rofollowed bymode=rwc). A repeatedvfsthat ends withunix, andfile:URIs whose own query (written with%3F) iscache=sharedormode=rwc, create the database at 0600 with its sidecars.file:<dir>/a%2520b.sqlitereachesa b.sqliteand leaves no straya%20b.sqlite.sqlite_uri.rshas unit tests for the decoding, the separators,%00, empty option names, the authority, andcreates_fileover 13 queries.serve_lock.rshas new tests for the lock path of a decodedfile:path, an encodedmodekey, and afile:name withvfs(unchanged).Integration test binary
crates/storage-sqlite/tests/file_modes.rs, which sets umask 022 in its own process:init(the bootstrapper sequence the CLI runs),servethrough the server components factory, a first write (create-account, CreateTable, TagResource), a stop and a secondserve: after each step each regular file in the deployment directory is 0600.servethat bootstraps a missing database creates 0600 files.initare 0600 after aserveand aftermigrate --yes.Negative controls
Each mutation was made on a clean committed tree and reverted afterwards. The first two ran on this branch before the commits that change how files are narrowed and how locations are read; the others ran on those commits.
connectwithoutprepareafter_connect(sidecars at 0644); the integration tests pass, becauserestrictrepairs the end stateconnectwithoutrestrictafter_connectO_PATHwithoutO_NOFOLLOW-journallink followed)O_PATHO_NOFOLLOW | O_NONBLOCKENXIOon the socket node)catalog_poolwithoutfile_modeSqliteEngine::newwithoutfile_modevfswinsvfs=memdb&vfs=unixfile:URI with its own query left to SQLite%3Fcache=shared%HHdecoding%2520tests for the file and the lock path, and twosqlite_uritestsmode=memoryfound with a raw splitmodekey tests for the file and the lock pathfile:name withvfsorimmutablenot left aloneGate
cargo +1.97.0 fmt --all -- --check: no changes.cargo +1.97.0 clippy --locked -p extenddb-storage-sqlite --all-targets -- -D warnings: no warnings.cargo test --locked -p extenddb-storage-sqliteon stable (1.99.0) and on 1.88.0: the crate's 185 unit tests and 7 integration tests pass, and 1 test is ignored, as on main.cargo check --locked --workspace --all-targets: no warnings.sqlite_uri.rscompiled withrustc +1.97.0 --target x86_64-pc-windows-msvcbeside a caller that usesparseandpath, asserve_lockdoes on Windows: no warnings, with and without--test. Before the last two commits it gave seven dead-code warnings.These tests ran on Linux. The macOS version of the narrowing has not been run. The npm build workflow on this pull request compiled it for both macOS targets, and both builds passed.
Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-diskformat, or public CLI surface, an RFC has been accepted or is linked
below. Otherwise, an ADR captures the decision (link below).
CI ran the workspace tests on Rust 1.88.0 and on stable, and the integration suites on all four backends. All of them passed. Locally I ran the SQLite crate's tests as listed under Gate. Clippy ran with
-D warnings, without the pedantic group. The change sets file modes and adds no command, flag or file format.ADR / RFC: n/a
Breaking changes
init,migrate --yes,settings,verifyandserve. On main the serve engine ignored a failed chmod and the other paths set no mode.destroycarries on when its catalog read fails, as on main, and then removes the files while holding the lock file.settings,verify,migrate --yesorservetheir owner runs.settingsandverify, and byserveandmigrate --yeswhen no lock file is present. On main the lock file already refused that user forserve,migrateanddestroywhenever it existed.settingsandverifywork when the files are 0600 and are refused when the files are wider. One command run by the owner on a writable mount, orchmod 600, sets the files so that the read-only mount opens.settingsandverifynow fail withcannot set the mode of SQLite file <path>: Permission denied (os error 13), where main printed SQLite'sunable to open database file. Both exit 1 and leave the files' modes unchanged.Measured on 31 October 2026 with a main binary (
1c0523c9) and a binary built from this branch. Each command ran in anamazonlinux:2023container on a bind mount of a deployment directory the owner created withinit. The read-only rows ran as the owner. The group member is uid 65534 with the owner's group, and the other user is uid 65532.-waland-shmpresentsettings list,settings get,verifysettings list,verifyRead-only file system (os error 30)verifyon a writable mountsettings list,verifychmod 600settings list-walor-shmsettings listunable to open database filesettings list,verifyOperation not permitted (os error 1)migrate --yes,serve,destroy --yescannot set the mode of lock filemigrate --yescannot set the mode of SQLite fileservecannot set the mode of SQLite filesettings list,verifyOperation not permitted (os error 1)For a read-only mount, the new troubleshooting entry has the owner run one command on a writable mount or
chmod 600the files. For files that belong to another user, it has extenddb run as that user.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.