diff --git a/.github/workflows/backup-restore-drill.yml b/.github/workflows/backup-restore-drill.yml index 1fa41c3b..ac9ac83c 100644 --- a/.github/workflows/backup-restore-drill.yml +++ b/.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 diff --git a/ROADMAP.md b/ROADMAP.md index 1b15d1f5..23009220 100644 --- a/ROADMAP.md +++ b/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. diff --git a/backend/tests/integration/sessions.test.mts b/backend/tests/integration/sessions.test.mts index fc232fb3..475e4262 100644 --- a/backend/tests/integration/sessions.test.mts +++ b/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`; diff --git a/scripts/backup-postgres.sh b/scripts/backup-postgres.sh index 86a0ccd5..a08818f3 100644 --- a/scripts/backup-postgres.sh +++ b/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,18 +35,18 @@ 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 @@ -51,31 +54,25 @@ cleanup() { 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" diff --git a/scripts/restore-drill.sh b/scripts/restore-drill.sh index 8a525e16..284749f5 100644 --- a/scripts/restore-drill.sh +++ b/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,9 +52,11 @@ 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) ;; @@ -51,8 +64,7 @@ case "$target_db" in 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"