SQLite files are created 0644 with plaintext credentials, and the documented Docker deployment supplies the parent directory 0755 #255
Reference in New Issue
Block a user
Delete Branch "%!s()"
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?
Verified during the deployability audit. On a data directory the app created itself: directory
0750, lock file0600, butwebhooker.dbis0644.That file holds credentials in plaintext —
targets.config(bearer tokens, API keys, Slack webhook URLs) andentrypoints.signature_secret. Confirmed bygrep -aon the file and bysqlite3:{"url":"...","headers":{"Authorization":"Bearer CANARY-BEARER-TOKEN-AAA","X-Api-Key":"..."}}andsignature_secret = GITLAB-TOKEN-SUPER-SECRET-9x.The
0750directory is therefore the only barrier — and the standard Docker-vbind-mount deployment the README documents supplies that directory at0755, removing it. Every local user on the host can then read every stored credential.The per-webhook event databases were checked and are clean (zero credential canaries) since #206.
Why this rather than encryption at rest
#212 proposes encrypting the config column. That is the wrong control for this deployment and remains deferred. An unattended process needs a key it can read without a human, so the key lands in an env var or a sibling file on the same host; an attacker who can read
webhooker.dbcan read that too. Encryption at rest would buy protection only against OFFLINE disclosure — a backup, a snapshot, a decommissioned disk — which is better addressed by encrypting the backup and the volume, and it carries a key-rotation and re-wrap story that 1.0 should not take on.What is wrong today is narrow, concrete and cheap: the file mode.
Definition of done
webhooker.db, every per-webhook event database, and every archive database are created0600. Existing files are tightened to0600on open, so an upgrade fixes a deployment already on disk rather than only new ones.0750when the app creates it.DATA_DIRownership and permission requirement. Note the audit separately found that a bind-mounted directory Docker creates as0:0makes the container fail to start at all (it runs as UID 1000), and the requiredchown -R 1000:1000currently appears only under Restore — fix both in the same pass.Verification
make checkgreen.statoutput for each database file on a freshly created data directory, and on a data directory created by an older build then opened by the new one.Plan.
The fix goes in
OpenSQLite(internal/database/sqlite_open.go), the single open path all three tiers share since #256 — main, per-webhook event, and archive are covered at once, with no change ininternal/delivery.0644comes from SQLite itself:robust_opensubstitutesSQLITE_DEFAULT_FILE_PERMISSIONS(0644) whenever the caller passes mode 0, andfindCreateFileModereturns 0 for a main database opened by URI without amodeofparameter. Chmod-after-open would race the window in which the file exists at0644, so insteadOpenSQLitecreates the file itself at0600(os.OpenFile,O_CREATE) before handing the path to the driver, and chmods it to0600when it already exists.That also settles the sidecars, which is the part that could silently not work. Reading modernc.org/sqlite v1.28.0:
-waltakes its mode fromfindCreateFileMode, which stats the main database file with the-walsuffix stripped;-shmis opened inunixOpenSharedMemorywithst_mode & 0777from anfstatof the open main-database descriptor. Both therefore inherit0600from the main file rather than needing their own call — and any stale-wal/-shmleft at0644by an earlier build gets chmodded alongside the main file. I willstatall of them rather than trust that reading.Data directory stays
0750(dataDirPerm), untouched.Tests: a table over all three tiers asserting
0600on the database and on each sidecar that exists, plus the directory still0750. Plus end-to-end evidence in the PR body — receive, deliver, restart, reopen.README: the Docker section gets the
DATA_DIRownership and permission requirement stated where the bind-mount is documented, consistent with the bind-address material from #268 and with thechown -R 1000:1000under Restore.Not doing: encryption at rest (#212 stays deferred), and no migration code — the chmod-on-open is the one-liner that covers a directory an older build left behind.
Built in #287 (base
next, branchissue-255-sqlite-file-mode).OpenSQLitenow creates each database file itself at0600before the driver sees the path, and chmods one that already exists. That covers all three tiers from one place. The-waland-shmsidecars inherit the mode from the main file — SQLite derives both from it — so they come out0600without a call of their own.Definition of done, item by item:
webhooker.db, everyevents-*.dband everyarchive-*.dbcreated0600, sidecars included.statfor all nine files plus the lock is in the PR body.webhooker.db,-waland-shmall644; the new build opened that same directory and all three came back600.0750, asserted in a test.DATA_DIRownership and permission requirement. Theroot:rootbind-mount startup failure is reproduced in the actual container — it exits non-zero onwebhooker.lockbefore opening any database — and thechown -R 1000:1000, previously only under Restore, is now stated where the bind mount is documented.internal/database/sqlite_mode_test.goassert the mode of each database and both its sidecars across all three tiers. Negative control: with the fix stashed they fail on the main file and on both sidecars.Verified beyond the tests: an event received and delivered, the service stopped and restarted against the same
DATA_DIR, a second event delivered, and every file back at0600after the reopen. In a container on a0755bind mount, the files are0600— the parent directory is no longer the barrier.make checkgreen withGOFLAGS=-count=1; lint ran in Docker,0 issues.No encryption at rest; #212 is untouched.