Fix the vaultwarden backup failing silently for nine days

Every nightly run had been failing since 20 Sep, writing the reason only to a log file nobody reads. The last backup on the share was from 19 Sep. Root cause: a sudoers rule that names arguments matches only that exact argument list. The rule allowed sudo /usr/bin/sqlite3 /var/.../db.sqlite3 but the script ran sudo sqlite3 /var/.../db.sqlite3 ".backup '/tmp/.../db'" The second argument was not in the rule, so it did not match, sudo fell through to the password-required bdm ALL=(ALL) ALL, and cron has no terminal to prompt on. Reproduced with -n, so neither call could prompt: sudo -n sqlite3 <db> # 1 arg, matches -> exit 0 sudo -n sqlite3 <db> "SELECT 1" # 2 args, no match -> password required Two further problems were hidden behind that early exit: The cleanup phase called sudo mount/umount/find inline, but sudoers only whitelisted helper scripts that existed and were never invoked. The 60-day retention was dead too. The old cleanup also could not distinguish "share not mounted" from "nothing to delete": find matched nothing and the script exited 0 reporting a healthy run. The helpers that *were* whitelisted lived under /home/bdm and are owned by bdm, so the grant was equivalent to unrestricted root. Changes: Move each privileged step into a root-owned helper in /usr/local/sbin, deployed from the repo as source of truth. This avoids depending on fragile argument globs and lets the rule constrain the one argument that varies. vw-backup-db re-validates the destination path itself rather than trusting the sudoers pattern. Call the helpers from the script and drop the inline sudo. Verify the snapshot with PRAGMA integrity_check before uploading, and abort rather than shipping a copy that does not verify. Check smbclient's exit status directly instead of piping into grep, which would have let SIGPIPE mask a failed upload. Add a systemd staleness watchdog so this cannot hide again. It mounts the share read-only and separates STALE (nothing recent) from CANNOT VERIFY (could not mount), which mean different things. Also chmod 0440 /etc/sudoers.d/bdm, which visudo was warning about. Tested with a cold sudo cache, since timestamp_type=global means an interactive sudo -v otherwise masks the failure. A real cron invocation succeeded end to end: snapshot, integrity check, upload, and prune. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-09-28 11:11:42 -07:00
parent d27e93d220
commit dd26e7c751
9 changed files with 502 additions and 60 deletions
--- a/vaultwarden-backup-helpers/README.md
+++ b/vaultwarden-backup-helpers/README.md
@@ -41,15 +41,111 @@ Use `export-to-json.sh` and `import-from-json.sh` for individual user vaults:
 | `restore-sqlite.sh` | Restore entire instance from SQLite backup | No |
 | `export-to-json.sh` | Export single user vault to JSON | **Yes** |
 | `import-from-json.sh` | Import JSON vault to user account | **Yes** |
-| `mount-smb.sh` | Mount SMB share for cleanup (internal) | No |
-| `umount-smb.sh` | Unmount SMB share (internal) | No |
-| `cleanup-old-backups.sh` | Delete backups older than 60 days (internal) | No |
+| `vw-backup-db` | Take the consistent SQLite snapshot (root helper) | No |
+| `mount-smb.sh` | Mount SMB share for cleanup (root helper) | No |
+| `umount-smb.sh` | Unmount SMB share (root helper) | No |
+| `cleanup-old-backups.sh` | Delete backups older than 60 days (root helper) | No |
+| `vw-backup-check` | Staleness watchdog (root helper, run by systemd) | No |
+
+## Privileged Helpers
+
+The four helpers that need root are kept in this directory as the **source of
+truth** and deployed to `/usr/local/sbin/`, owned by `root:root`, mode 755:
+
+| Source (here) | Deployed to |
+|---|---|
+| `vw-backup-db` | `/usr/local/sbin/vw-backup-db` |
+| `mount-smb.sh` | `/usr/local/sbin/vw-mount-smb` |
+| `umount-smb.sh` | `/usr/local/sbin/vw-umount-smb` |
+| `cleanup-old-backups.sh` | `/usr/local/sbin/vw-cleanup-old` |
+| `vw-backup-check` | `/usr/local/sbin/vw-backup-check` |
+
+They must stay root-owned. sudoers runs a script *as root*, so a NOPASSWD grant
+pointing at a file that `bdm` can rewrite is equivalent to granting unrestricted
+root. The previous sudoers file pointed at scripts under `/home/bdm`, which
+`bdm` owns.
+
+To deploy after editing:
+
+```bash
+cd /home/bdm/scripts/vaultwarden-backup-helpers
+sudo install -o root -g root -m 0755 vw-backup-db          /usr/local/sbin/vw-backup-db
+sudo install -o root -g root -m 0755 mount-smb.sh          /usr/local/sbin/vw-mount-smb
+sudo install -o root -g root -m 0755 umount-smb.sh         /usr/local/sbin/vw-umount-smb
+sudo install -o root -g root -m 0755 cleanup-old-backups.sh /usr/local/sbin/vw-cleanup-old
+sudo install -o root -g root -m 0755 vw-backup-check       /usr/local/sbin/vw-backup-check
+```
+
+## Why the work is done in helpers, not inline sudo
+
+A sudoers rule that names arguments matches **only that exact argument list**.
+
+This was the cause of a silent nine-day backup outage. The rule was:
+
+```
+bdm ALL=(ALL) NOPASSWD: /usr/bin/sqlite3 /var/.../db.sqlite3
+```
+
+but the script ran `sudo sqlite3 /var/.../db.sqlite3 ".backup '/tmp/...db'"` -
+a second argument the rule did not name. The rule did not match, sudo fell
+through to the password-required `bdm ALL=(ALL) ALL`, and cron has no terminal,
+so every nightly run failed with `sudo: a password is required` and wrote the
+failure only to a log nobody read.
+
+Demonstrating it, both with `-n` so neither can prompt:
+
+```bash
+sudo -n /usr/bin/sqlite3 /var/.../db.sqlite3                 # 1 arg, matches  -> works
+sudo -n /usr/bin/sqlite3 /var/.../db.sqlite3 "SELECT 1;"     # 2 args, no match -> password required
+```
+
+Putting each privileged step in a root-owned helper avoids depending on
+fragile argument globs, and lets the sudoers rule constrain the one argument
+that varies. The `vw-backup-db` rule is still argument-constrained, so it cannot
+be used to write files outside the backup directory:
+
+```
+bdm ALL=(ALL) NOPASSWD: /usr/local/sbin/vw-backup-db /tmp/vaultwarden-backup/vaultwarden-*.db
+```
+
+Note the filename must match `vaultwarden-*.db` or sudo will not match the rule
+and will fall back to asking for a password.
+
+**Never replace these helper calls with inline `sudo` in the script.** That is
+what broke it.
+
+## Staleness Watchdog
+
+A backup that fails silently is worse than one that fails loudly, so a systemd
+timer checks the share every day and fails visibly.
+
+- **Timer:** `vw-backup-check.timer`, daily at 10:30 + up to 10 min jitter, `Persistent=true` so a missed run catches up
+- **Service:** `vw-backup-check.service`, runs as root, no sudo involved
+- **Alarm threshold:** 2 days
+
+It mounts the share **read-only** and reports the three states separately, which
+matters because they mean different things:
+
+| Exit | State | Meaning |
+|---|---|---|
+| 0 | `OK` | Newest backup is under 2 days old |
+| 1 | `STALE` | Newest backup is 2+ days old, or there are no backups at all |
+| 2 | `CANNOT VERIFY` | The share could not be mounted, so freshness is unknown |
+
+Check it by hand:
+
+```bash
+systemctl status vw-backup-check.service
+journalctl -t vaultwarden-backup-check -n 20
+sudo systemctl start vw-backup-check.service   # run it now
+```

 ## Backup Location

 - **SMB Share:** `//192.168.123.2/NetBackup/Vault/`
 - **Local Temp:** `/tmp/vaultwarden-backup/` (cleaned up after upload)
 - **Log File:** `/var/log/vaultwarden-backup.log`
+- **Journal:** `journalctl -t vaultwarden-backup` (the log file is append-only and nobody reads it; the journal is the one to check)

 ## Cron Schedule

@@ -67,10 +163,38 @@ To run a manual backup:

 ## Sudo Configuration

-The backup script uses sudo for specific commands. Sudoers rules are in `/etc/sudoers.d/vaultwarden-backup`:
- `sqlite3` backup command
- `chown` for temp files
- SMB mount/umount/cleanup wrapper scripts
+Sudoers rules are in `/etc/sudoers.d/vaultwarden-backup` and grant passwordless
+root for exactly four root-owned helpers in `/usr/local/sbin/`: the SQLite
+snapshot, the SMB mount, the SMB unmount, and the retention prune.
+
+The old rules for a bare `sqlite3` call and for `chown` are gone. The sqlite3
+rule could not match the two-argument call the script actually makes, and
+`chown` is no longer needed because `vw-backup-db` hands the file back to the
+calling user using `SUDO_UID`/`SUDO_GID`.
+
+Validate after any edit - `visudo -c` catches a bad rule before it can lock you
+out, and `/etc/sudoers.d/*.bak` is not read by sudo, so a broken file can simply
+be removed:
+
+```bash
+sudo visudo -c -f /path/to/newfile     # check before installing
+sudo install -o root -g root -m 0440 /path/to/newfile /etc/sudoers.d/vaultwarden-backup
+sudo visudo -c                         # then check the whole config
+```
+
+Two pre-existing quirks in this system's sudo setup are worth knowing about,
+because they explain confusing behaviour:
+
+- `/etc/sudoers.d/010_pi-nopasswd` contains `bdm ALL=(ALL) NOPASSWD: ALL`, which
+  looks like blanket passwordless root. It is **inert**: sudo applies the *last*
+  matching rule, and `bdm ALL=(ALL) ALL` in `sudoers.d/bdm` sorts after it and
+  wins. Left alone deliberately - reordering would actually grant the blanket
+  access that the file only appears to.
+- `Defaults timestamp_type=global` in `010_global-tty` means a `sudo -v` in any
+  terminal is reused by *all* of `bdm`'s sessions, including cron, for 15
+  minutes. So `sudo` can appear to work from a script when it would actually
+  fail on an unattended run. `sudo -k` clears it, which is the only honest way
+  to test a NOPASSWD path.

 ## Disaster Recovery Procedure

@@ -105,6 +229,37 @@ The backup script uses sudo for specific commands. Sudoers rules are in `/etc/su
 - Check volume path: `ls -la /var/lib/docker/volumes/caddy_vaultwarden_data/_data/`
 - Run manually to see errors: `/home/bdm/scripts/vaultwarden-backup.sh`

+### Backup fails with "sudo: a password is required"
+The NOPASSWD rule did not match. Almost always an argument mismatch - see
+[Why the work is done in helpers](#why-the-work-is-done-in-helpers-not-inline-sudo).
+Check what the rule expects against what the script passes:
+
+```bash
+sudo -n -l                                    # effective rules for bdm
+sudo -k                                       # clear the cached credential first
+sudo -n /usr/local/sbin/vw-backup-db /tmp/vaultwarden-backup/vaultwarden-probe.db
+```
+
+If that probe works with a cold cache, the grant is fine and the problem is in
+the script's arguments. If it fails, the sudoers file is wrong or was not
+installed.
+
+### Backup succeeds but nothing is on the share
+Compare the two sides. The helper verifies integrity before uploading, so a
+successful run with no file on the NAS points at the upload rather than the
+snapshot:
+
+```bash
+tail -20 /var/log/vaultwarden-backup.log
+smbclient //192.168.123.2/NetBackup -A "$HOME/.smbcredentials" -c 'cd Vault; ls'
+```
+
+### The watchdog alarms but backups look fine
+Exit 2 (`CANNOT VERIFY`) means the share could not be mounted, so the check
+could not read anything - that is a NAS or credential problem, not a backup
+problem. Exit 1 (`STALE`) means it mounted the share and genuinely found nothing
+recent.
+
 ### SMB upload fails
 - Check network connectivity: `ping 192.168.123.2`
 - Check the SMB credentials file is present and readable: `ls -l ~/.smbcredentials`
@@ -142,4 +297,14 @@ mount -t cifs` calls, which run as root. The two are independent.
 - SMB credentials are kept in `~/.smbcredentials` (mode 600), not in the scripts
 - JSON exports contain plaintext passwords - encrypt them if storing long-term
 - SQLite backups contain encrypted data - safe to store but treat as sensitive
- Master passwords are never stored - only used interactively or via env var
+- Master passwords are never stored - only used interactively or via env var
+- Every NOPASSWD grant points at a root-owned helper in `/usr/local/sbin/`, never
+  at a file the `bdm` account can edit. A sudoers grant to a user-writable script
+  is unrestricted root in practice.
+- `vw-backup-db` re-checks the destination path itself rather than trusting the
+  sudoers argument glob, so the grant stays constrained even if the rule is
+  broadened later
+- The database is in **WAL mode** with a large uncheckpointed WAL. `cp` would
+  capture stale data; only `sqlite3 .backup` produces a consistent snapshot. The
+  backup is also `PRAGMA integrity_check`ed before upload, and a failure there
+  aborts rather than shipping a bad copy.
--- a/vaultwarden-backup-helpers/cleanup-old-backups.sh
+++ b/vaultwarden-backup-helpers/cleanup-old-backups.sh
@@ -1,3 +1,36 @@
 #!/bin/bash
-# Clean up old vaultwarden backups from SMB share
-find "/mnt/netbackup/Vault" -name "vaultwarden-*.db" -mtime +60 -type f -print -delete 2>/dev/null | wc -l
+# Delete vaultwarden backups older than the retention window.
+#
+# Run as root via a NOPASSWD sudoers rule, because the share is mounted as root.
+# Install to /usr/local/sbin/vw-cleanup-old owned by root, mode 755.
+#
+# The retention window is fixed here rather than taken as an argument, so this
+# grant cannot be used to delete anything outside the named pattern.
+
+set -euo pipefail
+
+MNT="/mnt/netbackup"
+KEEP_DAYS=60
+
+# Refuse to delete anything if the share is not mounted. Without this check the
+# path is a plain empty directory, find matches nothing, and the script exits 0
+# reporting "deleted 0" - which is indistinguishable from a healthy run. That
+# is how the retention policy could fail silently.
+if ! mountpoint -q "$MNT"; then
+    echo "vw-cleanup-old: ${MNT} is not mounted, refusing to run" >&2
+    exit 1
+fi
+
+DIR="${MNT}/Vault"
+
+if [ ! -d "$DIR" ]; then
+    echo "vw-cleanup-old: ${DIR} does not exist on the share" >&2
+    exit 1
+fi
+
+# -delete implies -depth, so directory contents are handled before the
+# directory itself. The pattern is pinned to this share and this filename
+# shape; nothing else on the share can match.
+find "$DIR" -maxdepth 1 -type f -name 'vaultwarden-*.db' -mtime "+${KEEP_DAYS}" -print -delete
+
+echo "vw-cleanup-old: retention ${KEEP_DAYS} days enforced"
--- a/vaultwarden-backup-helpers/mount-smb.sh
+++ b/vaultwarden-backup-helpers/mount-smb.sh
@@ -1,7 +1,38 @@
 #!/bin/bash
-# Mount SMB share for vaultwarden backup cleanup
-mkdir -p /mnt/netbackup
-if ! mountpoint -q /mnt/netbackup; then
-    mount -t cifs "//192.168.123.2/NetBackup" /mnt/netbackup \
-        -o credentials=/root/.smbcredentials,iocharset=utf8,vers=3.0,uid=1000,gid=1000,file_mode=0666,dir_mode=0777 2>/dev/null
-fi
+# Mount the NetBackup SMB share so old backups can be pruned.
+#
+# Run as root via a NOPASSWD sudoers rule. The mount has to be done as root
+# because /root/.smbcredentials is root-only.
+#
+# Install to /usr/local/sbin/vw-mount-smb owned by root, mode 755.
+
+set -euo pipefail
+
+MNT="/mnt/netbackup"
+SMB_HOST="192.168.123.2"
+SMB_SHARE="NetBackup"
+CRED="/root/.smbcredentials"
+
+if [ ! -f "$CRED" ]; then
+    echo "vw-mount-smb: credentials missing at ${CRED}" >&2
+    exit 1
+fi
+
+mkdir -p "$MNT"
+
+if mountpoint -q "$MNT"; then
+    echo "vw-mount-smb: already mounted"
+    exit 0
+fi
+
+# Default to the invoking user's ids rather than hardcoding 1000.
+uid="${SUDO_UID:-0}"
+gid="${SUDO_GID:-0}"
+
+mount -t cifs "//${SMB_HOST}/${SMB_SHARE}" "$MNT" \
+    -o "credentials=${CRED},iocharset=utf8,vers=3.0,uid=${uid},gid=${gid},file_mode=0666,dir_mode=0777"
+
+if ! mountpoint -q "$MNT"; then
+    echo "vw-mount-smb: mount did not take effect" >&2
+    exit 1
+fi
--- a/vaultwarden-backup-helpers/umount-smb.sh
+++ b/vaultwarden-backup-helpers/umount-smb.sh
@@ -1,3 +1,16 @@
 #!/bin/bash
-# Unmount SMB share for vaultwarden backup cleanup
-umount /mnt/netbackup 2>/dev/null
+# Unmount the NetBackup SMB share.
+#
+# Run as root via a NOPASSWD sudoers rule. Install to /usr/local/sbin/vw-umount-smb
+# owned by root, mode 755.
+
+set -euo pipefail
+
+MNT="/mnt/netbackup"
+
+if ! mountpoint -q "$MNT"; then
+    echo "vw-umount-smb: not mounted, nothing to do"
+    exit 0
+fi
+
+umount "$MNT"
--- a/vaultwarden-backup-helpers/vw-backup-check
+++ b/vaultwarden-backup-helpers/vw-backup-check
@@ -0,0 +1,65 @@
+#!/bin/bash
+# Fail loudly when the vaultwarden backup has gone stale.
+#
+# The backup job itself is only visible by reading a log file, which is how a
+# nine-day outage went unnoticed. This runs from a systemd timer as root, and
+# exits non-zero when the newest backup on the share is older than the
+# threshold, so the failure shows up in "systemctl status" and the journal.
+#
+# It covers both ways the backup can stop: the job failing, and the job not
+# running at all.
+
+set -uo pipefail
+
+THRESHOLD_DAYS=2
+MNT="/mnt/netbackup-check"
+SMB_HOST="192.168.123.2"
+SMB_SHARE="NetBackup"
+CRED="/root/.smbcredentials"
+TAG="vaultwarden-backup-check"
+
+note() { logger -t "$TAG" -- "$*"; echo "$*"; }
+
+cleanup() {
+    if mountpoint -q "$MNT"; then
+        umount "$MNT" 2>/dev/null || true
+    fi
+    rmdir "$MNT" 2>/dev/null || true
+}
+trap cleanup EXIT
+
+if [ ! -f "$CRED" ]; then
+    note "STALE CHECK FAILED - credentials missing at ${CRED}"
+    exit 2
+fi
+
+mkdir -p "$MNT"
+
+# Read-only, and not a system mount, so an unreachable NAS cannot wedge the
+# system. A failure to mount is reported as "cannot verify", not as "stale",
+# because those mean different things.
+if ! mount -t cifs "//${SMB_HOST}/${SMB_SHARE}" "$MNT" \
+        -o "credentials=${CRED},ro,iocharset=utf8,vers=3.0" 2>/dev/null; then
+    note "CANNOT VERIFY - could not mount //${SMB_HOST}/${SMB_SHARE} to check backup freshness"
+    exit 2
+fi
+
+newest=$(find "$MNT/Vault" -maxdepth 1 -type f -name 'vaultwarden-*.db' -printf '%T@ %p\n' 2>/dev/null \
+         | sort -rn | head -1 | cut -d' ' -f2-)
+
+if [ -z "$newest" ]; then
+    note "STALE - no vaultwarden backups found on the share at all"
+    exit 1
+fi
+
+# Age in days, rounded down.
+age_days=$(( ( $(date +%s) - $(stat -c %Y "$newest") ) / 86400 ))
+newest_name=$(basename "$newest")
+
+if [ "$age_days" -ge "$THRESHOLD_DAYS" ]; then
+    note "STALE - newest backup is ${newest_name}, ${age_days} day(s) old (threshold ${THRESHOLD_DAYS})"
+    exit 1
+fi
+
+note "OK - newest backup is ${newest_name}, ${age_days} day(s) old"
+exit 0
--- a/vaultwarden-backup-helpers/vw-backup-check.service
+++ b/vaultwarden-backup-helpers/vw-backup-check.service
@@ -0,0 +1,17 @@
+[Unit]
+Description=Check that the vaultwarden backup is not stale
+Documentation=file:/home/bdm/scripts/vaultwarden-backup-helpers/README.md
+After=network-online.target
+Wants=network-online.target
+
+[Service]
+Type=oneshot
+# Runs as root so it can mount the share itself; no sudo is involved.
+ExecStart=/usr/local/sbin/vw-backup-check
+# Exit 1 means stale, exit 2 means it could not verify. Both are failures, and
+# both leave a failed unit behind so "systemctl status" shows the problem.
+SuccessExitStatus=0
+TimeoutStartSec=120
+
+[Install]
+WantedBy=multi-user.target
--- a/vaultwarden-backup-helpers/vw-backup-check.timer
+++ b/vaultwarden-backup-helpers/vw-backup-check.timer
@@ -0,0 +1,14 @@
+[Unit]
+Description=Daily vaultwarden backup staleness check
+
+[Timer]
+# Mid-morning, so it runs well after the 03:00 backup and would catch a
+# missed or failed run from the previous night.
+OnCalendar=*-*-* 10:30:00
+# Catch up after the machine has been off, rather than silently skipping a day.
+Persistent=true
+# Spread the check a little so it is not always at the same instant.
+RandomizedDelaySec=10m
+
+[Install]
+WantedBy=timers.target
--- a/vaultwarden-backup-helpers/vw-backup-db
+++ b/vaultwarden-backup-helpers/vw-backup-db
@@ -0,0 +1,63 @@
+#!/bin/bash
+# Create a consistent SQLite backup of the vaultwarden database.
+#
+# Run as root via a NOPASSWD sudoers rule, because the database lives in a
+# root-owned Docker volume. The destination is constrained to the backup
+# directory so this grant cannot be used to write a file anywhere else.
+#
+# Install to /usr/local/sbin/vw-backup-db owned by root, mode 755. The copy
+# under scripts/ is the source of truth; do not sudo this one directly, or the
+# grant becomes a way to run arbitrary code as root.
+
+set -euo pipefail
+
+DB="/var/lib/docker/volumes/caddy_vaultwarden_data/_data/db.sqlite3"
+BACKUP_DIR="/tmp/vaultwarden-backup"
+OUT="${1:-}"
+
+if [ -z "$OUT" ]; then
+    echo "vw-backup-db: no destination given" >&2
+    exit 1
+fi
+
+# Only ever write a backup file into the backup directory. This is the
+# constraint that makes the sudoers grant safe, so it is checked here too and
+# not just in sudoers.
+case "$OUT" in
+    "${BACKUP_DIR}"/vaultwarden-*.db) ;;
+    *)
+        echo "vw-backup-db: refusing to write outside ${BACKUP_DIR}: ${OUT}" >&2
+        exit 1
+        ;;
+esac
+
+if [ ! -f "$DB" ]; then
+    echo "vw-backup-db: database not found at ${DB}" >&2
+    exit 1
+fi
+
+mkdir -p "$BACKUP_DIR"
+# Remove a stale file so a failed backup cannot be mistaken for a good one.
+rm -f "$OUT"
+
+# .backup takes a read lock and produces a consistent snapshot. The database
+# runs in WAL mode with an uncheckpointed WAL, so copying the file with cp
+# would capture stale data; .backup is required.
+if ! /usr/bin/sqlite3 "$DB" ".backup '${OUT}'"; then
+    echo "vw-backup-db: sqlite3 .backup failed" >&2
+    rm -f "$OUT"
+    exit 1
+fi
+
+if [ ! -s "$OUT" ]; then
+    echo "vw-backup-db: backup file is missing or empty" >&2
+    rm -f "$OUT"
+    exit 1
+fi
+
+# Hand the file back to the calling user so it can be uploaded without sudo.
+if [ -n "${SUDO_UID:-}" ] && [ -n "${SUDO_GID:-}" ]; then
+    chown "${SUDO_UID}:${SUDO_GID}" "$OUT"
+fi
+
+echo "vw-backup-db: wrote ${OUT} ($(stat -c %s "$OUT") bytes)"
--- a/vaultwarden-backup.sh
+++ b/vaultwarden-backup.sh
@@ -1,65 +1,106 @@
 #!/bin/bash
-# Vaultwarden daily backup to SMB share via smbclient
+# Vaultwarden daily backup to the NetBackup SMB share.
+#
+# Run daily from bdm's crontab at 03:00. Logging goes to
+# /var/log/vaultwarden-backup.log, and a failure is also reported to the
+# journal so it is visible without reading the log file.
+#
+# The privileged steps are four root-owned helpers in /usr/local/sbin, each
+# granted to bdm NOPASSWD in /etc/sudoers.d/vaultwarden-backup. Do not replace
+# those calls with inline sudo: a sudoers rule that names arguments matches only
+# that exact argument list, so anything that varies the arguments falls through
+# to a password-required rule and fails from cron, where there is no terminal.
+#
+# Source of truth for the helpers is scripts/vaultwarden-backup-helpers/.
+
+set -uo pipefail

-VAULT_DATA="/var/lib/docker/volumes/caddy_vaultwarden_data/_data"
 BACKUP_DIR="/tmp/vaultwarden-backup"
 TIMESTAMP=$(date +%Y-%m-%d_%H-%M-%S)
 BACKUP_FILE="${BACKUP_DIR}/vaultwarden-${TIMESTAMP}.db"
 SMB_HOST="192.168.123.2"
 SMB_SHARE="NetBackup"
 SMB_DIR="Vault"
-KEEP_DAYS=60
+BACKUP_NAME="vaultwarden-${TIMESTAMP}.db"

-# SMB credentials are stored in an external file (mode 600) rather than inline
-# here, so the password never lands in version control or in the process list.
-# See vaultwarden-backup-helpers/README.md for setup.
+# SMB credentials live in an external file (mode 600) rather than inline here,
+# so the password never lands in version control or in the process list.
 SMB_CRED_FILE="${SMB_CRED_FILE:-$HOME/.smbcredentials}"

+# Report to the journal as well as stdout, so a failure is not invisible.
+log() {
+    echo "$(date): $*"
+    logger -t vaultwarden-backup -- "$*"
+}
+
+die() {
+    log "FATAL - $*"
+    exit 1
+}
+
 if [ ! -r "$SMB_CRED_FILE" ]; then
-    echo "$(date): ERROR - SMB credentials file not readable: ${SMB_CRED_FILE}"
-    echo "$(date): Create it with: printf 'username = %s\\npassword = %s\\n' <user> <pass> | install -m 600 /dev/stdin ${SMB_CRED_FILE}"
-    exit 1
+    die "SMB credentials file not readable: ${SMB_CRED_FILE}"
 fi

-mkdir -p "$BACKUP_DIR"
+mkdir -p "$BACKUP_DIR" || die "could not create ${BACKUP_DIR}"

-echo "$(date): Starting vaultwarden backup..."
-sudo sqlite3 "${VAULT_DATA}/db.sqlite3" ".backup '${BACKUP_FILE}'"
-sudo chown "$(id -u):$(id -g)" "$BACKUP_FILE"
+log "Starting vaultwarden backup..."

-if [ -f "$BACKUP_FILE" ] && [ -s "$BACKUP_FILE" ]; then
-    SIZE=$(du -h "$BACKUP_FILE" | cut -f1)
-    echo "$(date): Backup created ($SIZE), uploading to SMB..."
+# Consistent snapshot. The database runs in WAL mode with an uncheckpointed
+# WAL, so this must be sqlite3 .backup and not a file copy. The helper also
+# chowns the result back to this user.
+if ! sudo -n /usr/local/sbin/vw-backup-db "$BACKUP_FILE"; then
+    die "backup step failed"
+fi

-    smbclient "//${SMB_HOST}/${SMB_SHARE}" -A "$SMB_CRED_FILE" \
-        -c "cd ${SMB_DIR}; put ${BACKUP_FILE} vaultwarden-${TIMESTAMP}.db"
+if [ ! -s "$BACKUP_FILE" ]; then
+    die "backup file missing or empty: ${BACKUP_FILE}"
+fi

-    if [ $? -eq 0 ]; then
-        echo "$(date): Backup finished successfully."
+SIZE=$(du -h "$BACKUP_FILE" | cut -f1)
+log "Backup created (${SIZE}), uploading to SMB..."
+
+# The backup is worthless if it cannot be read back, so verify it before
+# spending bandwidth and before deleting anything.
+INTEGRITY=$(sqlite3 "$BACKUP_FILE" "PRAGMA integrity_check;" 2>&1)
+if [ "$INTEGRITY" != "ok" ]; then
+    log "ERROR - backup failed integrity check: ${INTEGRITY}"
+    rm -f "$BACKUP_FILE"
+    die "refusing to upload a backup that does not verify"
+fi
+log "Backup passed integrity check."
+
+# Capture the output and the exit status separately. Piping smbclient into
+# "grep -q" would not work: grep exits at the first match, smbclient takes
+# SIGPIPE, and the pipeline status would describe grep rather than the upload.
+upload_out=$(smbclient "//${SMB_HOST}/${SMB_SHARE}" -A "$SMB_CRED_FILE" \
+    -c "cd ${SMB_DIR}; put ${BACKUP_FILE} ${BACKUP_NAME}" 2>&1)
+upload_rc=$?
+printf '%s\n' "$upload_out"
+
+if [ "$upload_rc" -ne 0 ] || ! printf '%s' "$upload_out" | grep -q 'putting file'; then
+    rm -f "$BACKUP_FILE"
+    die "SMB upload failed (smbclient exit ${upload_rc})"
+fi
+
+rm -f "$BACKUP_FILE"
+log "Backup uploaded successfully to //${SMB_HOST}/${SMB_SHARE}/${SMB_DIR}/${BACKUP_NAME}"
+
+# Retention pruning. This is best-effort: a failure here must not be reported
+# as a failed backup, because the backup itself already succeeded. The helpers
+# fail loudly if the share is not mounted, which is the case worth knowing
+# about, so the warning is logged rather than swallowed.
+log "Pruning backups older than 60 days..."
+if sudo -n /usr/local/sbin/vw-mount-smb; then
+    if sudo -n /usr/local/sbin/vw-cleanup-old; then
+        log "Retention prune complete."
    else
-        echo "$(date): ERROR - SMB upload failed!"
-        exit 1
+        log "WARNING - pruning failed; retention may not be enforced."
    fi
-
-    rm -f "$BACKUP_FILE"
+    sudo -n /usr/local/sbin/vw-umount-smb || log "WARNING - could not unmount ${BACKUP_DIR}"
 else
-    echo "$(date): ERROR - sqlite3 backup failed or file is empty!"
-    rm -f "$BACKUP_FILE"
-    exit 1
+    log "WARNING - could not mount SMB share, skipping retention prune."
 fi

-# Clean up old backups (older than KEEP_DAYS)
-echo "$(date): Cleaning up backups older than ${KEEP_DAYS} days..."
-sudo mkdir -p /mnt/netbackup
-if ! mountpoint -q /mnt/netbackup; then
-    sudo mount -t cifs "//${SMB_HOST}/${SMB_SHARE}" /mnt/netbackup \
-        -o credentials=/root/.smbcredentials,iocharset=utf8,vers=3.0,uid=$(id -u),gid=$(id -g),file_mode=0666,dir_mode=0777 2>/dev/null
-fi
-
-if mountpoint -q /mnt/netbackup; then
-    DELETED=$(find "/mnt/netbackup/${SMB_DIR}" -name "vaultwarden-*.db" -mtime +${KEEP_DAYS} -type f -print -delete 2>/dev/null | wc -l)
-    sudo umount /mnt/netbackup 2>/dev/null
-    echo "$(date): Deleted ${DELETED} old backup(s)."
-else
-    echo "$(date): WARNING - Could not mount SMB for cleanup, skipping."
-fi
+log "Backup finished successfully."
+exit 0