From a5671ab3beaf353ed5a645a9a8186dfdc96d9421 Mon Sep 17 00:00:00 2001 From: null Date: Fri, 3 Jul 2026 10:32:39 -0500 Subject: [PATCH] =?UTF-8?q?fix(qa):=20harden=20DB=20file=20permissions=20?= =?UTF-8?q?=E2=80=94=20was=20world-readable=20644=20(QA-B16-02)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docker-entrypoint chmod 700'd the data dir but never the DB file; SQLite created bills.db/-wal/-shm at umask 644 (world-readable), holding financial data + encrypted SimpleFIN token/sessions/secrets. Add `umask 077` (files 600, dirs 700) + explicit chmod 600 of any pre-existing DB files on upgrade. Found on the live nebula deploy (BillTracker.db was 644). Co-Authored-By: Claude Opus 4.8 --- HISTORY.md | 1 + docker-entrypoint.sh | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index d400c7b..be05cb9 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ ### 🐛 QA Fixes +- **[Security] SQLite DB was created world-readable (644)** — `docker-entrypoint.sh` locked the data *directory* (`chmod 700`) but never the DB *file*, and SQLite created `bills.db`/`-wal`/`-shm` under the default umask (644). On a real deploy the DB (financial data + encrypted SimpleFIN token, sessions, SMTP/OIDC secrets) was world-readable, shielded only by the parent dir's 700. Added `umask 077` to the entrypoint (DB, WAL/SHM, backups, exports now created 600/700) plus an explicit `chmod 600` for pre-existing files on upgrade. (QA-B16-02) - **[Privacy] The version check is now opt-out-able** — the privacy policy described the external version check as "optional", but there was no way to disable it (it hit a hardcoded upstream host whenever the About/Status/version page loaded). Added an **admin toggle**: an `update_check_enabled` setting gates the request in `services/updateCheckService.js` (default on — when off, **no external request is made**), exposed via `GET`/`PUT /api/about-admin/update-check-setting` and a switch on the admin **System Status** page. Privacy policy updated to state an admin can disable it. Test: `tests/updateCheckOptOut.test.js`. (was QA-B16-01) - **[Security] Bill name could inject HTML into reminder emails** — `buildEmailHtml` (`services/notificationService.js`) escaped the bill name in the detail table but interpolated it **raw** into the reminder message line (`${bill.name} is due…`), so a bill named `` landed unescaped in the email HTML. Self-XSS (reminder emails go to the bill's owner), but a clear inconsistent-escaping bug — now escaped everywhere. Covered by `tests/notificationDelivery.test.js`. (was QA-B14-04) - **[Notifications] "Send test push" was completely broken** — `services/notificationService.js` attached its `_push` export (the ntfy/Gotify/Discord/Telegram helpers) *before* the final `module.exports = {…}`, which clobbered it, so `require('…/notificationService')._push` was `undefined`. `routes/notifications.js` (`const { sendTestPush } = require(…)._push || {}`) therefore always hit `throw 'Push service not initialised'` → **`POST /api/notifications/test-push` always returned 500** for every user testing their push channel. Scheduled reminders were unaffected (they call `sendPushToUser` in-scope). Moved the `_push` assignment after the reassignment. Covered by `tests/notificationDelivery.test.js` (per-channel payloads, dispatch, error handling, and a check that the auth token never leaks into the message body). (was QA-B10-01) diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 840e99b..5dbf37d 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -1,6 +1,12 @@ #!/bin/sh set -eu +# Files this app writes (the SQLite DB + WAL/SHM, backups, exports) hold financial +# data and encrypted secrets (SimpleFIN token, sessions, SMTP/OIDC). Create them +# owner-only (600 files / 700 dirs) — not world-readable. Inherited by the exec'd +# node process so SQLite's -wal/-shm are locked too. (QA-B16-02) +umask 077 + APP_USER="${APP_USER:-bill}" APP_GROUP="${APP_GROUP:-bill}" DATA_DIR="${DATA_DIR:-/data}" @@ -13,6 +19,9 @@ mkdir -p "$DATA_DIR" "$DB_DIR" "$BACKUP_DIR" /app/backups if [ "$(id -u)" = "0" ]; then chown -R "$APP_USER:$APP_GROUP" "$DATA_DIR" /app/backups chmod 700 "$DB_DIR" "$BACKUP_DIR" /app/backups + # Lock any pre-existing DB files that were created world-readable (644) before + # this umask fix — otherwise they keep their old mode across an upgrade. + chmod 600 "$DB_FILE" "$DB_FILE"-wal "$DB_FILE"-shm 2>/dev/null || true if [ "${RUN_DB_MIGRATIONS:-true}" = "true" ]; then su-exec "$APP_USER:$APP_GROUP" node scripts/migrate-db.js fi