fix(backups): make the backup restore drill actually run; unique sessions test port

.github/workflows/backup-restore-drill.yml

@@ -5,6 +5,9 @@ on:
     - cron: '0 3 * * *'  # daily at 03:00 UTC
   workflow_dispatch:
 
+permissions:
+  contents: read
+
 env:
   PGCLIENT_VERSION: '16'
 
@@ -17,38 +20,42 @@ jobs:
       postgres:
         image: postgres:16
         env:
+          POSTGRES_USER: drill
           POSTGRES_PASSWORD: drill
           POSTGRES_DB: app
         ports:
           - 5432:5432
-        options: >
-          --health-cmd = pg_isready -U drill -d app
-          --health-interval = 1s
-          --health-timeout = 5s
-          --health-retries = 10
+        options: >-
+          --health-cmd "pg_isready -U drill -d app"
+          --health-interval 1s
+          --health-timeout 5s
+          --health-retries 10
     env:
       DATABASE_URL: postgres://drill:drill@127.0.0.1:5432/app
       RESTORE_TARGET_URL: postgres://drill:drill@127.0.0.1:5432/app_drill
-      BACKUP_ENCRYPTION_KEY: $>
-        {{ secrets.BACKUP_ENCRYPTION_KEY || 'drill-passphrase-not-for-prod' }}
-      BACKUP_DIR: ${{ runner.temp }}/backups
+      BACKUP_ENCRYPTION_KEY: ${{ secrets.BACKUP_ENCRYPTION_KEY || 'drill-passphrase-not-for-prod' }}
       BACKUP_RETENTION_DAYS: '14'
       RESTORE_CONFIRM: '1'
     steps:
       - name: Checkout
         uses: actions/checkout@v4
 
+      # `runner` isn't available in job-level env, so set the backup
+      # directory from the step environment instead.
+      - name: Set backup directory
+        run: echo "BACKUP_DIR=$RUNNER_TEMP/backups" >> "$GITHUB_ENV"
+
       - name: Install Postgres clients
         run: |
           sudo apt-get update
           sudo apt-get install -y postgresql-client
           pg_dump --version
-          ps_sql --version
+          psql --version
 
       - name: Wait for Postgres
         run: |
           for i in $(seq 1 30); do
-            pg_isready -U drill -d app && break
+            pg_isready -h 127.0.0.1 -U drill -d app && break
             sleep 1
           done
 

ROADMAP.md

@@ -137,7 +137,7 @@ roadmap. See each phase section below for status and issue links.
 - [x] Real CI pipeline (replaced `.github/workflows/ci.yml` placeholder — was `pull_request`-only, `echo "TODO"`, never once run). Now runs on push to `main` and on PRs: `npm audit --omit=dev` at high severity (`audit:prod`, Phase 8 issue #47) → `prisma:generate` → `typecheck` → unit tests → **integration tests (the self-contained embedded-Postgres double-booking proof)** → `build`. Verified locally end-to-end before pushing (14 unit + 1 integration test passing) and confirmed green in Actions. No lint script exists yet in `backend/package.json` — not added; `tsc --noEmit` is the closest static check currently available. Lint now tracked as [issue #28](https://github.com/DocForum/docforum-core/issues/28) (Trivial, 100 pts). Closes [issue #21](https://github.com/DocForum/docforum-core/issues/21) (Medium, 150 pts).
 - [ ] Staging environment. Blocked on issue #22's decision landing first — not yet scoped into its own issue.
 - [ ] Production environment + secrets management. Blocked on issue #22's decision landing first — not yet scoped into its own issue.
-- [x] **Automated Postgres backups with a tested restore drill** — scheduled encrypted logical backups (`pg_dump` → `age`-encrypted artifacts, retention policy, checksummed manifest) via `infra/backup/` + `.github/workflows/postgres-backup.yml`; a restore script (`scripts/restore-drill.mjs`) that restores the latest backup into a scratch database with explicit confirmation + name guards (refuses any target whose name doesn't match the scratch pattern, refuses to run against `DATABASE_URL`/`STAGING_DATABASE_URL`/`PRODUCTION_DATABASE_URL`), then runs integrity checks (row counts, `_prisma_migrations` version, a sample booking query). The drill runs on a schedule in CI against staging backups and alerts on failure. RPO/RTO targets and the measured restore time are documented in `docs/backups.md`. Tracked as [issue #30](https://github.com/DocForum/docforum-core/issues/30) (High, 200 pts) — **Done**.
+- [x] **Automated Postgres backups with a tested restore drill** — `scripts/backup-postgres.sh` takes an encrypted logical backup (`pg_dump --format=custom` piped through `gpg --symmetric`, so the plaintext dump never touches disk) with a retention window; `scripts/restore-drill.sh` restores the latest backup into a scratch database with explicit confirmation and name guards (the target must end in `_drill`/`_scratch` and must not be the source database), then runs integrity checks (row counts, `_prisma_migrations`, a sample booking query). `.github/workflows/backup-restore-drill.yml` runs the round-trip daily (and on demand) against a seeded throwaway Postgres service. It does not touch the Render preview database yet, because there's no production target (see the deployment decision above). Design and RPO/RTO notes are in `docs/adr/0007-postgres-backups-and-restore-drill.md` and `infra/backup/README.md`. Tracked as [issue #107](https://github.com/DocForum/docforum-core/issues/107) — **Done**.
 - [x] Docs site deployed — `docs/site/` (VitePress) via `.github/workflows/deploy-docs.yml`, live at https://docforum.github.io/docforum-core/. Unrelated to the production-deploy decision above; this is a static site with no backend of its own.
 - [x] **PR conflict notifier** — `.github/workflows/pr-conflict-notifier.yml` + `.github/scripts/pr-conflict-notifier.cjs`. On every push to `main` (and every 3h) it labels open PRs that conflict with `main` as `needs-rebase` and @mentions the author once per push with rebase steps; the label clears itself when the conflict is resolved. Runs from `main` only and never checks out PR code.
 
@@ -496,4 +496,5 @@ roadmap. See each phase section below for status and issue links.
   asserting the audit rows (the accept's two events, the decline's one)
   against the real `AuditEvent` table.
 - 2026-10-20 — Docs site: `docs/site/scripts/sync-docs.mjs` now rewrites repo-relative links in docforum-core's own synced files (ROADMAP, ARCHITECTURE, PRD, ADRs) to GitHub URLs, as it already did for the other repos. #153 added a relative link to `docs/adr/0006-multiple-referrals-per-consultation.md` in ROADMAP.md, which VitePress rejected as a dead link and which broke the docs deploy on `main`.
-- 2026-10-21 — Phase 9: automated Postgres backups with a tested restore drill (issue #30). Encrypted logical backups (`pg_dump` piped through `age`, checksummed manifest, retention policy) scheduled in `.github/workflows/postgres-backup.yml`; a restore script (`scripts/restore-drill.mjs`) that restores the latest backup into a scratch database and runs integrity checks (row counts, migration version, a sample booking query), with hard safety guards — explicit confirmation flag, scratch-name pattern check, and refusal to target any live database by name or by matching `DATABASE_URL`/`STAGING_DATABASE_URL`/`PRODUCTION_DATABASE_URL`. The drill runs on a schedule in CI against staging backups and alerts on failure. RPO/RTO targets and the measured restore time are documented in `docs/backups.md`. Backups contain PHI, so encryption at rest and access control are mandatory and enforced in the workflow (artifact encryption key is a repo secret; artifacts are never uploaded unencrypted). No schema or endpoint changes, so `ARCHITECTURE.md` §4 and `docs/api/README.md` are untouched.
+- 2026-10-21 — Phase 9: automated Postgres backups with a tested restore drill ([issue #107](https://github.com/DocForum/docforum-core/issues/107)): `scripts/backup-postgres.sh` (gpg-encrypted `pg_dump`, retention), `scripts/restore-drill.sh` (guarded restore into a `_drill` database plus integrity checks), and a daily `.github/workflows/backup-restore-drill.yml`. ADR 0007. No schema or endpoint changes, so `ARCHITECTURE.md` §4 and `docs/api/README.md` are untouched.
+- 2026-10-20 — Fixed the backup drill shipped above, which had never run. `backup-restore-drill.yml` was invalid YAML, so it failed on every push to `main` (broken `BACKUP_ENCRYPTION_KEY` expression and service `--health-cmd` options, missing `POSTGRES_USER`). Both scripts had shell syntax errors (`psal`, `-symmetric`, malformed heredocs, a broken `find -printf`, mangled parameter expansions). Also: gpg now reads the passphrase non-interactively (`--pinentry-mode loopback`); the restore drops and recreates the scratch database through the `postgres` maintenance database and only accepts plain identifiers; the backup log no longer prints the connection string (it contains the password). Corrected the roadmap entry, which described files that don't exist (`postgres-backup.yml`, `restore-drill.mjs`, `age`, `docs/backups.md`). Separately, `tests/integration/sessions.test.mts` moved its embedded Postgres from port 5438 to 5451, because 5438 is already used by `access-control.test.mts` and vitest runs integration files in parallel.

backend/tests/integration/sessions.test.mts

@@ -4,7 +4,7 @@
 // revocation is enforced at refresh time, and ownership checks hold.
 //
 // Self-contained like the other integration suites: throwaway Postgres on
-// port 5438 (one port per file — vitest runs files in parallel).
+// port 5451 (one port per file — vitest runs files in parallel).
 import { execFileSync } from 'node:child_process';
 import fs from 'node:fs';
 import os from 'node:os';
@@ -15,7 +15,7 @@ import request from 'supertest';
 import EmbeddedPostgres from 'embedded-postgres';
 
 const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');
-const PORT = 5438;
+const PORT = 5451;
 const runId = Date.now();
 
 process.env.DATABASE_URL = `postgresql://docforum:docforum@localhost:${PORT}/docforum_test`;

scripts/backup-postgres.sh

@@ -6,21 +6,24 @@ set -euo pipefail
 # Takes an encrypted logical backup of the database and writes it to
 # ${BACKUP_DIR} with a timestamped filename. Encryption is mandatory
 # (backups contain PHI): the dump is piped through `gpg --symmetric` with
-# a passphrase supplied via BCKUP_ENCRYPTION_KEY. The plaintext dump
+# a passphrase supplied via BACKUP_ENCRYPTION_KEY. The plaintext dump
 # never touches disk.
 #
 # Required env:
 #   DATABASE_URL                -- Postgres connection string to back up
 #   BACKUP_ENCRYPTION_KEY       -- symmetric passphrase for gpg
 #   BACKUP_DIR                  -- destination directory (default: ./backups)
-#   BACKUP_RETENTION_DAYS        -- retention window in days (default: 14)
-#   PG_DUMP_FORMAT              -- "custom" (default) or "plain"
+#   BACKUP_RETENTION_DAYS       -- retention window in days (default: 14)
+#
+# The dump is always pg_dump's custom format, which is what
+# scripts/restore-drill.sh restores with pg_restore.
 
 DB_URL="${DATABASE_URL:-}"
 KEY="${BACKUP_ENCRYPTION_KEY:-}"
 BACKUP_DIR="${BACKUP_DIR:-./backups}"
 RETENTION_DAYS="${BACKUP_RETENTION_DAYS:-14}"
-FORMAT="${PG_DUMP_FORMAT:-custom}"
+
+ts() { date -u +%Y-%m-%dT%H:%M:%SZ; }
 
 if [ -z "$DB_URL" ]; then
   echo "error: DATABASE_URL is not set" >&2
@@ -32,50 +35,44 @@ if [ -z "$KEY" ]; then
   exit 2
 fi
 
-if [ "$FORMAT" != "custom" ] && [ "$FORMAT" != "plain" ]; then
-  echo "error: PG_DUMP_FORMAT must be custom or plain" >&2
-  exit 2
-fi
+case "$RETENTION_DAYS" in
+  ''|*[!0-9]*) echo "error: BACKUP_RETENTION_DAYS must be a whole number of days" >&2; exit 2 ;;
+esac
 
 mkdir -p "$BACKUP_DIR"
 
-stamp="$(date -u +%Ym%d_H%M%S)"
+stamp="$(date -u +%Y%m%d_%H%M%S)"
 outfile="$BACKUP_DIR/postgres-${stamp}.dump.gpg"
 tmpfile="${outfile}.part"
 
-echo "[$(stamp)] backing up $DATABASE_URL -> $outfile"
+# Never log the connection string itself: it carries the password.
+echo "[$(ts)] backing up ${DB_URL%%://*}://… -> $outfile"
 
 cleanup() {
   rm -f "${tmpfile}" 2>/dev/null || true
 }
 trap cleanup EXIT
 
 # Pipe the dump straight into gpg so the plaintext dump never exists
-# on disk. --batch --yes makes the command non-interactive.
-if [ "$FORMAT" = "custom" ]; then
-  pg_dump \
-    --db="$DB_URL" \
-    --format=custom \
-    --no-owner \
-    --no-privileges \
-    --blobs \
-    | gpg --batch --yes --symmetric --cipher-algo AES256 \
-        --passphrase-fd 3>"$tmpfile" 3<< <"$KEY"
-        > "$tmpfile" 3<< <"$KEY"
-else
-  pg_dump "$DB_URL" --no-owner --no-privileges --blobs \
-    | gpg --batch --yes -symmetric --cipher-algo AES256 \
-        --passphrase-fd 3>"$tmpfile" 3<< <"$KEY"
-        > "${tmpfile}" 3<< <"$KEY"
-fi
+# on disk. The passphrase goes in on fd 3 so it never appears in argv;
+# --pinentry-mode loopback lets gpg 2.x read it non-interactively.
+pg_dump \
+  --dbname="$DB_URL" \
+  --format=custom \
+  --no-owner \
+  --no-privileges \
+  | gpg --batch --yes --pinentry-mode loopback \
+      --symmetric --cipher-algo AES256 \
+      --passphrase-fd 3 \
+      --output "$tmpfile" 3<<<"$KEY"
 
 mv "$tmpfile" "$outfile"
 
 size=$(wc -c < "$outfile" | tr -d ' ')
-echo "[$(stamp)] wrote $outfile (${size} bytes)"
+echo "[$(ts)] wrote $outfile (${size} bytes)"
 
 # Retention: delete backups older than RETENTION_DAYS.
-echo "[$(stamp)] applying retention (${RETENTION_DAYS} days)"
+echo "[$(ts)] applying retention (${RETENTION_DAYS} days)"
 find "$BACKUP_DIR" -type f -name 'postgres-*.dump.gpg' -mtime +"$RETENTION_DAYS" -print -delete
 
-echo "[$(stamp)] backup complete"
+echo "[$(ts)] backup complete"

scripts/restore-drill.sh

@@ -17,10 +17,14 @@ set -euo pipefail
 #
 # Required env:
 #   DATABASE_URL             -- source connection string (for safety checks)
-#   RESTORE_TARGET_URL        -- scratch database connection string
-#   BACKUP_ENCRYPTION_KEY     -- symmetric passphrase for gpg
+#   RESTORE_TARGET_URL       -- scratch database connection string
+#   BACKUP_ENCRYPTION_KEY    -- symmetric passphrase for gpg
 #   BACKUP_DIR               -- directory containing backups
-#   RESTORE_CONFIRM           -- must equal 1
+#   RESTORE_CONFIRM          -- must equal 1
+#
+# The scratch database is dropped and recreated through the server's
+# `postgres` maintenance database, so the role in RESTORE_TARGET_URL needs
+# CREATEDB.
 
 DB_URL="${DATABASE_URL:-}"
 TARGET_URL="${RESTORE_TARGET_URL:-}"
@@ -33,6 +37,13 @@ fail() {
   exit 2
 }
 
+# Database name from a connection string: the path after the last /,
+# without any ?query.
+db_name() {
+  local name="${1##*/}"
+  echo "${name%%\?*}"
+}
+
 # --------------------------------------------------------------------------
 # Safety guards
 # --------------------------------------------------------------------------
@@ -41,18 +52,19 @@ fail() {
 [ -n "$TARGET_URL" ] || fail "RESTORE_TARGET_URL is not set"
 [ -n "$KEY" ] || fail "BACKUP_ENCRYPTION_KEY is not set"
 
-# Extract the database name from the target URL (path after the last /)
-target_db="${TARGET_URL\%}*/}"
-target_db="${target_db\%?*}"
+target_db="$(db_name "$TARGET_URL")"
+
+# The name is interpolated into DROP/CREATE DATABASE below, so only allow
+# plain identifiers.
+[[ "$target_db" =~ ^[A-Za-z0-9_]+$ ]] || fail "target database name '$target_db' must be letters, digits and underscores only"
 
 case "$target_db" in
   *_drill|*_scratch) ;;
   *) fail "refusing to restore into '$target_db': target database name must end in _drill or _scratch" ;;
 esac
 
 if [ -n "$DB_URL" ]; then
-  source_db="${DB_URL\%}*}"
-  source_db="${source_db\%?*}"
+  source_db="$(db_name "$DB_URL")"
   if [ "$source_db" = "$target_db" ]; then
     fail "refusing to restore into the source database '$target_db'"
   fi
@@ -62,7 +74,7 @@ fi
 # Find the latest backup
 # --------------------------------------------------------------------------
 
-backup="$(find "$BACKUP_DIR" -type f -name 'postgres-*.dump.gpg' -printf '%T%@\n' | sort -r | head -n1 | cut -f'\'\n' -d2)"
+backup="$(find "$BACKUP_DIR" -type f -name 'postgres-*.dump.gpg' -printf '%T@ %p\n' 2>/dev/null | sort -nr | head -n1 | cut -d' ' -f2-)"
 
 [ -n "$backup" ] || fail "no backups found in $BACKUP_DIR"
 
@@ -73,15 +85,18 @@ echo "[restore-drill] target database: $target_db"
 # Restore
 # --------------------------------------------------------------------------
 
-start_ts=$(date +%sT')
+admin_url="${TARGET_URL%/*}/postgres"
+start_ts=$(date +%s)
 echo "[restore-drill] dropping and recreating $target_db"
-psal "$TARGET_URL" -c "DROP DATABASE IF EXISTS;" -c "CREATE DATABASE $target_db;" > /dev/null
+psql "$admin_url" -v ON_ERROR_STOP=1 -q \
+  -c "DROP DATABASE IF EXISTS \"$target_db\";" \
+  -c "CREATE DATABASE \"$target_db\";"
 
-echo "[restore-drill] restoring :..."
-gpg --batch --yes --decrypt --passphrase-fd 3 << <"$KEY" 3<"$backup" \
-  | pg_restore --db="$TARGET_URL" --no-owner --no-privileges --exit-on-error
+echo "[restore-drill] restoring ..."
+gpg --batch --yes --pinentry-mode loopback --passphrase-fd 3 --decrypt "$backup" 3<<<"$KEY" \
+  | pg_restore --dbname="$TARGET_URL" --no-owner --no-privileges --exit-on-error
 
-end_ts=$(date +s%s)
+end_ts=$(date +%s)
 elapsed=$((end_ts - start_ts))
 echo "[restore-drill] restore completed in ${elapsed}s"
 
@@ -96,7 +111,7 @@ check_count() {
   local sql="$2"
   local min="$3"
   local count
-  count="$(psql "$TARGET_URL" -tAt -c "$sql")"
+  count="$(psql "$TARGET_URL" -tAX -c "$sql")"
   if [ "$count" -lt "$min" ]; then
     echo "[restore-drill] FAIL $label: got $count, expected >= $min" >&2
     failed=1
@@ -109,13 +124,13 @@ check_count "migrations applied" "SELECT COUNT(*) FROM _prisma_migrations" 1
 check_count "users" "SELECT COUNT(*) FROM \"users\"" 1
 check_count "bookings" "SELECT COUNT(*) FROM \"bookings\"" 1
 
-# Sample booking query: join a booking to its user and order to prove
-# the relationships survived the round-trip.
-sample="$psql "$TARGET_URL" -tAt -c "\
-  SELECT b.id, b.status, u.email FROM \"bookings\" b \
-  JOIN \"users\" u ON u.id = b.\"userId\" LIMIT 1"")"
+# Sample booking query: join a booking to its user to prove the
+# relationships survived the round-trip.
+sample="$(psql "$TARGET_URL" -tAX -c "
+  SELECT b.id, b.status, u.email FROM \"bookings\" b
+  JOIN \"users\" u ON u.id = b.\"userId\" LIMIT 1")"
 if [ -z "$sample" ]; then
-  echo "[restore-drill] FAIL  sample booking query returned no rows" >&2
+  echo "[restore-drill] FAIL sample booking query returned no rows" >&2
   failed=1
 else
   echo "[restore-drill] ok   sample booking query: $sample"