Create every SQLite file 0600 (closes #255) #287
Reference in New Issue
Block a user
Delete Branch "issue-255-sqlite-file-mode"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #255.
webhooker.dbwas created0644while holding target configuration inplaintext — bearer tokens, API keys, Slack webhook URLs — plus the
session encryption key. The
0750data directory was the only barrier,and a Docker bind mount supplies that directory at
0755.The change
One place:
OpenSQLiteininternal/database/sqlite_open.go, the singleopen path all three tiers have shared since
#256. Main, per-webhook event
and archive databases are all covered without touching
internal/delivery.0644comes from SQLite itself:robust_opensubstitutesSQLITE_DEFAULT_FILE_PERMISSIONSwhenever it is handed mode 0, andfindCreateFileModeyields 0 for a main database opened by URI with nomodeofparameter. A chmod after opening would leave a window in whichthe credentials are on disk world-readable, so
OpenSQLitecreates thefile itself at
0600before the driver sees the path, and chmods onethat already exists.
The sidecars are the part that could have silently not worked. SQLite
derives both from the main database file —
-walthroughfindCreateFileMode, which stats the path with the suffix stripped, and-shminunixOpenSharedMemoryfrom anfstatof the open databasedescriptor — so a main file at
0600produces sidecars at0600. Thatis what the driver source says; the
statoutput below is what actuallyhappened.
Existing files are chmodded on open, so a directory an earlier build left
0644is fixed rather than staying exposed until it is recreated. Nomigration code.
Not encryption at rest —
#212 stays deferred.
The data directory stays
0750.Verification
make checkgreen,GOFLAGS=-count=1, on the branch rebased ontob9f7db6. Exit 0; 21 packagesok, zero(cached)lines; lint ran inDocker for 50s and reported
0 issues.staton a freshly created data directoryThe real binary, fresh
DATA_DIR, one webhook with alogtarget and adatabasetarget, one event received and delivered — all three tiers andevery sidecar present with handles open:
Directory still
0750.Control: the same run on a pre-change build
Same binary minus the
sqlite_open.gochange, fresh directory:Then the fixed build opened that same directory:
Note the control confirms the trap was real: the pre-change build left
the
-waland-shmworld-readable too, so fixing only the main filewould have fixed nothing.
End to end, and restart
POST /webhook/{entrypoint}answered 200 and the event was delivered:Stopped, restarted against the same
DATA_DIR: health checkok, asecond event received and delivered, and every file — including the
sidecars SQLite recreated on reopen — back at
0600. The archive tierreopened with the rest.
In the actual container
Built the image and reproduced the deployment from the README, which is
where the
0755parent directory comes from:A world-listable parent, and the credentials are still unreadable. Every
container was run
--rmanddocker ps -ais clean.Tests
internal/database/sqlite_mode_test.go— the mode of the database andboth sidecars for each of the three tiers, the tighten-on-open case, the
reopen-after-restart case, that
SQLiteModeExistingstill does notmaterialize a file, and that the data directory is not world-accessible.
Negative control: with the
sqlite_open.gochange stashed, theassertions fail on the main file and on both sidecars.
The directory assertion is a property, not an exact mode
The data-directory check was originally
assert.Equal(fs.FileMode(0o750), info.Mode().Perm()).MkdirAllapplies the ambient umask, so that pinned
make checkto the umask ofwhoever ran it. Measured: under
umask 077it fails withexpected: 0x1e8, actual: 0x1c0— 0750 against 0700.It now asserts the property that carries the security meaning:
The group bits are deliberately left unasserted, since deployments may
rely on them. Run both ways with
GOFLAGS=-count=1:make testexits 0under
umask 022and underumask 077, andTestMainDatabaseFilesAreOwnerOnlypasses in both. The file-modeassertions — the point of this PR — were already umask-independent and
are unchanged.
README
The Docker section now carries the
DATA_DIRownership requirement wherethe bind mount is documented. Verified in a container rather than
asserted: a
-vsource path Docker creates isroot:root, the processruns as UID 1000, and it exits non-zero on the lock file before opening
any database —
The
chown -R 1000:1000that fixes this previously appeared only underRestore. Also verified and stated: a named volume does not have the
problem, because the image creates
/var/lib/webhookerowned bywebhookerand Docker copies that onto a volume it initializes. The newtext sits after the bind-address material from
#268 and points at Restore
rather than repeating it.
Disclosures
killagainst the outputof a bare
pgrep -x webhooker, which on this shared host matched 81PIDs belonging to other sessions. All 81 were
<defunct>zombies, onwhich
killis a no-op; I confirmed afterwards that the only livewebhookerprocess was another session's, started after my command.Nothing was disrupted, but the command was wrong and I am flagging it.
make dockertagswebhooker:latest, a name shared with every othersession on this host. I retagged to
impl-255-webhooker:testimmediately and removed that tag afterwards; the shared
webhooker:latesttag now points at my build. Not re-run since.PASS.
Mechanism verified against
modernc.org/sqlite v1.28.0as pinned and it is exactly as described:robust_opensubstitutesSQLITE_DEFAULT_FILE_PERMISSIONSfor mode 0;findCreateFileModeyields 0 for a URI-opened main DB with nomodeof, and forSQLITE_OPEN_WAL|MAIN_JOURNALstats the path with the suffix stripped;unixOpenSharedMemorypassesst_mode & 0777from anfstatof the open DB descriptor intorobust_open. The sidecars are therefore covered structurally, not by a chmod that could miss a later file — androbust_open's trailingfchmodon a zero-length new file makes them umask-proof too. Reproduced0644on unmodifiednextacross all three tiers and both sidecars; confirmed0600on the head for every file underumask 000and inside a0755bind mount in the container; a busy-loop watcher over first boot, first write and lazy archive creation recorded zero instants wider than0600. Negative control is live: five of the six new tests fail at base, each on the main file and on-waland-shm.Two non-blocking notes:
internal/database/sqlite_mode_test.go:100asserts the data directory is0750.MkdirAllapplies the umask, somake testnow fails underumask 077(measured: actual0700). Pre-existingdataDirPermbehaviour, newly pinned by a test; the file-mode assertions themselves are umask-independent.perm & 0o007 == 0, or chmodding the directory, would remove the dependence.archive-{uuid}.dbleft0644by an earlier build is tightened only on the next archive delivery. The idle sweep opens withSQLiteModeExisting, and with the defaultexpiry=neverit skips opening the file at all, so a dormant webhook's archive stays world-readable indefinitely (measured; it goes to0600the moment an event is delivered). Moot with no installed base;webhooker.dbandevents-*.dbare both tightened at startup.README checked against current
nextrather than as a diff: the new Docker text agrees with Restore step 4 and with the bind-address material from #268, both anchors resolve, and the quoted lock-file failure was reproduced verbatim in a container on a Docker-createdroot:rootmount.Disclosure: the negative control and a
create-always-true probe were run withgo testin throwaway copies outside the reviewed tree, never on it. During teardown I used apkill -fscoped to my own unique scratch path; it matched only my own processes, including my own shell.1230ae990etoa5bc29f433