From c0b924510291c1db7d829f7a17cd8daabcb7b690 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 18:35:21 -0400 Subject: [PATCH 01/16] fix(update): snapshot the live SQLite DB consistently (online backup API) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of a real-world failed update: backup_current, deploy_source and deploy_release all plain-cp'd the LIVE panel database. On a busy panel that races the backend's own writes and the copy can capture half a transaction — 'database disk image is malformed' — which then sinks the migration in the new slot and silently poisons the pre-upgrade backup that was supposed to be the safety net. - New copy_sqlite_db() uses the SQLite online backup API via python (guaranteed by pre-flight) and refuses to fall back to a racy cp. - backup_current halts rather than proceed without a valid safety net. - deploy_source/deploy_release halt before any switch if the live DB cannot be snapshotted consistently. - backup_docker_db prefers a consistent snapshot via docker exec python, with the plain docker cp as a fallback. - locate_python rejects candidates whose version probe returns empty (broken shims pass 'command -v' but can't execute; an empty version string used to pass the sort-based range check). Tests: T10d exercises a real snapshot (integrity ok, all rows); a contract test bans plain-cp of the live DB; T10d fixture python detection validates the interpreter actually runs. --- scripts/test/test_update.sh | 55 ++++++++++++++++++++++++++ scripts/update.sh | 78 +++++++++++++++++++++++++++++++------ 2 files changed, 122 insertions(+), 11 deletions(-) diff --git a/scripts/test/test_update.sh b/scripts/test/test_update.sh index 4260db5d..d584dc60 100644 --- a/scripts/test/test_update.sh +++ b/scripts/test/test_update.sh @@ -324,6 +324,61 @@ else bad "migrate_database probe-unavailable: rc=$mig_rc, expected rc 0 (warn + proceed)" fi rm -f "$STUB_BIN/flask" + +# -------------------------------------------------------------------------- +# T10d — copy_sqlite_db snapshots a LIVE database consistently via the SQLite +# online backup API. A plain cp of a hot DB can capture half a transaction +# and come out "database disk image is malformed" — the exact failure that +# sank the migration on a real 1.7.35 -> 1.7.65 update. +# -------------------------------------------------------------------------- +t="$WORK/t10d"; mkdir -p "$t" +PY="" +for c in python3.12 python3.11 python3; do + if command -v "$c" >/dev/null 2>&1 && "$c" -c 'import sqlite3' 2>/dev/null; then + PY="$c"; break + fi +done +if [ -z "$PY" ]; then + skip "copy_sqlite_db snapshots a live DB consistently (no working python3 on this host)" +else + "$PY" - "$t/live.db" <<'PYEOF' +import sqlite3, sys +c = sqlite3.connect(sys.argv[1]) +c.execute('CREATE TABLE t(id INTEGER PRIMARY KEY, v TEXT)') +c.executemany('INSERT INTO t(v) VALUES (?)', [('row%d' % i,) for i in range(1000)]) +c.commit(); c.close() +PYEOF + snap_rc=0 + ( + set -Eeuo pipefail + DRY_RUN=0 + copy_sqlite_db "$t/live.db" "$t/snap.db" + ) >/dev/null 2>&1 || snap_rc=$? + snap_check="$("$PY" - "$t/snap.db" <<'PYEOF' +import sqlite3, sys +c = sqlite3.connect(sys.argv[1]) +print(c.execute('PRAGMA integrity_check').fetchone()[0], + c.execute('SELECT COUNT(*) FROM t').fetchone()[0]) +PYEOF + )" + if [ "$snap_rc" -eq 0 ] && [ "$snap_check" = "ok 1000" ]; then + ok "copy_sqlite_db snapshots a live DB consistently (integrity ok, all rows)" + else + bad "copy_sqlite_db: rc=$snap_rc, snapshot check=[$snap_check], expected 'ok 1000'" + fi +fi + +# Contract: no racy plain-cp of the LIVE database anywhere in update.sh — +# the pre-upgrade backup and both deploy paths must go through copy_sqlite_db. +if ! grep -qF 'cp "$db_file" "$backup_file"' "$UPDATE_SH" \ + && ! grep -qF 'cp "$INSTALL_DIR/backend/instance/serverkit.db"' "$UPDATE_SH"; then + ok "live SQLite DB is never plain-cp'd (consistent snapshot only)" +else + bad "a racy plain-cp of the live serverkit.db is still in update.sh" +fi + +# -------------------------------------------------------------------------- +# T11 — zero-downtime regression: reload_nginx_graceful must RELOAD a running # nginx and must NEVER stop it. Host nginx fronts every managed app, so a stop # during a panel update used to black out unrelated sites. A recording systemctl # stub (PATH-prepended ahead of the global stub) captures every invocation. diff --git a/scripts/update.sh b/scripts/update.sh index 20687ca7..333af46d 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -600,6 +600,10 @@ locate_python() { for c in python3.12 python3.11 python3; do if command -v "$c" &>/dev/null; then v="$($c -c 'import sys;print(".".join(map(str,sys.version_info[:2])))' 2>/dev/null || true)" + # An EMPTY version means the binary/shim exists but can't actually + # execute Python — and an empty string would pass the sort checks + # below ("" sorts before everything). Reject it and move on. + [ -n "$v" ] || continue if printf '%s\n%s' "3.11" "$v" | sort -C -V && \ printf '%s\n%s' "$v" "3.12" | sort -C -V; then printf '%s' "$c" @@ -814,6 +818,29 @@ atomic_switch() { good "Switched active install to $target" } +# --------------------------------------------------------------------------- +# Consistent SQLite copy. A plain `cp` of the LIVE panel database races the +# backend's own writes: the copy can capture half a transaction and come out +# "database disk image is malformed" — which then sinks the migration in the +# new slot, and silently poisons the pre-upgrade backup that was supposed to +# be the safety net. The SQLite online backup API snapshots a consistent DB +# even while the panel is writing. Python 3.11/3.12 is guaranteed by +# pre-flight; without it we fail rather than fall back to a racy cp. +# --------------------------------------------------------------------------- +copy_sqlite_db() { + local src="$1" dest="$2" py + py="$(locate_python)" || { warn "No Python 3.11/3.12 available for a consistent DB snapshot"; return 1; } + "$py" - "$src" "$dest" <<'PYEOF' +import sqlite3, sys +src, dest = sys.argv[1], sys.argv[2] +s = sqlite3.connect(src) +d = sqlite3.connect(dest) +s.backup(d) +d.close() +s.close() +PYEOF +} + # --------------------------------------------------------------------------- # Backup # --------------------------------------------------------------------------- @@ -827,7 +854,10 @@ backup_current() { if [ -f "$db_file" ]; then backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).db" - run_or_dry cp "$db_file" "$backup_file" + # A torn backup is worse than none: it looks like a safety net but + # restores a malformed DB. Snapshot consistently or refuse to proceed. + run_or_dry copy_sqlite_db "$db_file" "$backup_file" \ + || halt "Database backup failed — refusing to update without a safety net" good "Database backed up to $backup_file" else warn "No SQLite database at $db_file — skipping DB backup" @@ -1019,20 +1049,28 @@ deploy_source() { return 0 fi - # Preserve .env and database across rewrites. - local tmp_env tmp_db + # Preserve .env and database across rewrites. The DB snapshot must be + # consistent (online backup API) — a racy cp of the live DB can come out + # malformed and sink the migration in the new slot. + local tmp_env tmp_db live_db tmp_env="$(mktemp)" tmp_db="$(mktemp)" cp "$INSTALL_DIR/.env" "$tmp_env" 2>/dev/null || true - cp "$INSTALL_DIR/backend/instance/serverkit.db" "$tmp_db" 2>/dev/null || true + live_db="$INSTALL_DIR/backend/instance/serverkit.db" + if [ -f "$live_db" ]; then + copy_sqlite_db "$live_db" "$tmp_db" \ + || halt "Could not snapshot the live database consistently — aborting before any switch" + fi rm -rf "$target" git clone --depth 1 --branch "$branch" "https://github.com/${GITHUB_REPO}.git" "$target" \ || halt "Failed to clone ${GITHUB_REPO}:$branch" cp "$tmp_env" "$target/.env" 2>/dev/null || true - mkdir -p "$target/backend/instance" - cp "$tmp_db" "$target/backend/instance/serverkit.db" 2>/dev/null || true + if [ -s "$tmp_db" ]; then + mkdir -p "$target/backend/instance" + cp "$tmp_db" "$target/backend/instance/serverkit.db" 2>/dev/null || true + fi rm -f "$tmp_env" "$tmp_db" preserve_installed_plugins "$INSTALL_DIR" "$target" @@ -1072,10 +1110,15 @@ deploy_release() { fi [ -d "$unpacked" ] || halt "Release tarball layout is unrecognized" - # Preserve live state. + # Preserve live state. Same consistent-snapshot rule as deploy_source: + # never plain-cp the live DB into the new slot. cp "$INSTALL_DIR/.env" "$unpacked/.env" 2>/dev/null || true - mkdir -p "$unpacked/backend/instance" - cp "$INSTALL_DIR/backend/instance/serverkit.db" "$unpacked/backend/instance/serverkit.db" 2>/dev/null || true + local live_db="$INSTALL_DIR/backend/instance/serverkit.db" + if [ -f "$live_db" ]; then + mkdir -p "$unpacked/backend/instance" + copy_sqlite_db "$live_db" "$unpacked/backend/instance/serverkit.db" \ + || halt "Could not snapshot the live database consistently — aborting before any switch" + fi preserve_installed_plugins "$INSTALL_DIR" "$unpacked" rm -rf "$target" @@ -1464,9 +1507,22 @@ backup_docker_db() { local backup_file backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).db" # The live DB lives in the serverkit-data volume at /app/instance inside - # the container, not in the source tree. - if docker cp serverkit-backend:/app/instance/serverkit.db "$backup_file" 2>/dev/null; then + # the container, not in the source tree. Prefer a consistent snapshot via + # the container's Python (SQLite online backup API) — a plain docker cp + # of the live file races the backend's writes and can come out malformed. + if docker exec -i serverkit-backend python - /app/instance/serverkit.db /tmp/.sk-snap.db <<'PYEOF' 2>/dev/null \ + && docker cp serverkit-backend:/tmp/.sk-snap.db "$backup_file" 2>/dev/null; then +import sqlite3, sys +s = sqlite3.connect(sys.argv[1]) +d = sqlite3.connect(sys.argv[2]) +s.backup(d) +d.close() +s.close() +PYEOF + docker exec serverkit-backend rm -f /tmp/.sk-snap.db 2>/dev/null || true good "Database backed up to $backup_file" + elif docker cp serverkit-backend:/app/instance/serverkit.db "$backup_file" 2>/dev/null; then + good "Database backed up to $backup_file (plain copy)" else warn "Could not copy DB from container — continuing (backend self-backs-up before migrating)" fi From 9edb5deb81e3642972ea13a31157ee61a9016478 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 18:42:10 -0400 Subject: [PATCH 02/16] fix(update): detect unquoted VIRTUAL_ENV binding in prebuilt venvs The venv binding check only matched the quoted VIRTUAL_ENV="/path" form; venvs created on some platforms write 'export VIRTUAL_ENV=/path' unquoted, so the binding read as 'unknown path' and always triggered a rebuild. Now matches both forms (and skips cygpath-style indirection), so fresh installs keep the fast prebuilt-venv path. --- scripts/update.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/update.sh b/scripts/update.sh index 333af46d..2e6b7a51 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -649,7 +649,9 @@ require_venv() { # dependencies (and run the OLD flask binary for migrations), so a # venv bound to any other path must be rebuilt in place. local bound_path resolved_bound resolved_target - bound_path="$(sed -n 's/^VIRTUAL_ENV="\(.*\)"$/\1/p' "$target_dir/bin/activate" | head -1)" + # Matches both VIRTUAL_ENV="/path" (classic venv) and the unquoted + # export VIRTUAL_ENV=/path form; skips cygpath-style indirection. + bound_path="$(grep -m1 -oE 'VIRTUAL_ENV="?/[^")]+"?' "$target_dir/bin/activate" | cut -d= -f2- | tr -d '"')" # Compare RESOLVED paths: on a fresh install the symlink already points # at the only slot, so the baked path resolves to the target and the # prebuilt venv is genuinely usable (fast path). During an update it From a61eab1509a160e5913f52b94141389c01c74b63 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 18:56:50 -0400 Subject: [PATCH 03/16] fix(update): pg_dump pre-upgrade backup for PostgreSQL installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Postgres panels updated with zero safety net: backup_current found no SQLite file and just warned, while the migration runs IN PLACE against the shared external DB — a failed migration left no way back. - backup_current now pg_dumps (custom format) when .env points at PostgreSQL, sanitizing SQLAlchemy driver suffixes (postgresql+psycopg2://) that libpq can't parse. - Same rule as SQLite: no valid backup, no update — halt when pg_dump is missing (install postgresql-client) or the dump fails. Tests: T10e covers the dump invocation + URL sanitization and the halt-on-failed-dump path. --- scripts/test/test_update.sh | 45 +++++++++++++++++++++++++++++++++++++ scripts/update.sh | 16 +++++++++++++ 2 files changed, 61 insertions(+) diff --git a/scripts/test/test_update.sh b/scripts/test/test_update.sh index d584dc60..edeeca58 100644 --- a/scripts/test/test_update.sh +++ b/scripts/test/test_update.sh @@ -377,6 +377,51 @@ else bad "a racy plain-cp of the live serverkit.db is still in update.sh" fi +# -------------------------------------------------------------------------- +# T10e — PostgreSQL installs get a real pre-upgrade safety net: backup_current +# must pg_dump the external DB (sanitizing SQLAlchemy driver suffixes libpq +# can't parse) and must HALT when the dump fails — updating a shared, +# migrate-in-place database with no dump is flying without a parachute. +# -------------------------------------------------------------------------- +t="$WORK/t10e"; mkdir -p "$t/serverkit" "$t/backups" +printf 'DATABASE_URL=postgresql+psycopg2://user:secret@db.host:5432/serverkit\n' > "$t/serverkit/.env" +cat > "$STUB_BIN/pg_dump" < "$WORK/t10e/url.txt" +out=""; prev="" +for a in "\$@"; do [ "\$prev" = "-f" ] && out="\$a"; prev="\$a"; done +: > "\$out" +exit \${PG_DUMP_RC:-0} +EOF +chmod +x "$STUB_BIN/pg_dump" +bk_rc=0 +( + set -Eeuo pipefail + DRY_RUN=0 INSTALL_DIR="$t/serverkit" BACKUP_DIR="$t/backups" + backup_current +) >/dev/null 2>&1 || bk_rc=$? +dump_file="$(ls "$t/backups"/serverkit-pre-upgrade-*.dump 2>/dev/null | head -1)" +if [ "$bk_rc" -eq 0 ] && [ -n "$dump_file" ] \ + && [ "$(cat "$WORK/t10e/url.txt" 2>/dev/null)" = "postgresql://user:secret@db.host:5432/serverkit" ]; then + ok "backup_current pg_dumps PostgreSQL installs (driver suffix sanitized)" +else + bad "backup_current postgres: rc=$bk_rc, dump=[$dump_file], url=[$(cat "$WORK/t10e/url.txt" 2>/dev/null)]" +fi +rm -rf "$t/backups"; mkdir -p "$t/backups" +bk_rc=0 +( + set -Eeuo pipefail + DRY_RUN=0 INSTALL_DIR="$t/serverkit" BACKUP_DIR="$t/backups" + export PG_DUMP_RC=1 + backup_current +) >/dev/null 2>&1 || bk_rc=$? +if [ "$bk_rc" -ne 0 ]; then + ok "backup_current halts when the PostgreSQL dump fails (no safety net, no update)" +else + bad "backup_current proceeded despite a failed pg_dump (rc=0)" +fi +rm -f "$STUB_BIN/pg_dump" + # -------------------------------------------------------------------------- # T11 — zero-downtime regression: reload_nginx_graceful must RELOAD a running # nginx and must NEVER stop it. Host nginx fronts every managed app, so a stop diff --git a/scripts/update.sh b/scripts/update.sh index 2e6b7a51..6cedbf90 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -861,6 +861,22 @@ backup_current() { run_or_dry copy_sqlite_db "$db_file" "$backup_file" \ || halt "Database backup failed — refusing to update without a safety net" good "Database backed up to $backup_file" + elif grep -qE '^DATABASE_URL=postgres' "$active/.env" 2>/dev/null; then + # PostgreSQL: the DB is external and SHARED between slots — there is + # no per-slot file to snapshot, and the migration runs in place. A + # pre-upgrade dump is the only safety net AND the only way back: + # without it, a rollback hands the old code a newer schema. + backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).dump" + command -v pg_dump >/dev/null 2>&1 \ + || halt "PostgreSQL install but pg_dump is unavailable — install postgresql-client so the update has a safety net" + # Strip SQLAlchemy driver suffixes (postgresql+psycopg2://) that + # libpq can't parse, and any wrapping quotes from the .env value. + local pg_url + pg_url="$(grep -E '^DATABASE_URL=' "$active/.env" | head -1 | cut -d= -f2- \ + | tr -d "\"'" | sed -E 's|^postgres(ql)?\+[a-z0-9]+://|postgresql://|')" + run_or_dry pg_dump "$pg_url" -Fc -f "$backup_file" \ + || halt "PostgreSQL backup failed — refusing to update without a safety net" + good "Database backed up to $backup_file" else warn "No SQLite database at $db_file — skipping DB backup" fi From b48256fba26c4a4d4526900f37f3dea787377356 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 19:07:17 -0400 Subject: [PATCH 04/16] =?UTF-8?q?fix(update):=20dump-proof=20the=20backup?= =?UTF-8?q?=20layer=20=E2=80=94=20verify,=20auto-restore,=20cap=20retentio?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A backup that was never verified is a hope, not a safety net. Closes the remaining ways the pre-upgrade safety net could silently not exist: - Every backup is verified immediately after creation: SQLite snapshots via PRAGMA integrity_check, PostgreSQL dumps via size + pg_restore TOC listing (when available). Invalid artifacts are deleted and the update halts — a malformed backup is never mistaken for a good one. - Partial artifacts from a failed backup are removed (rm on failure), so a half-written file can never be 'restored' later. - A failed IN-PLACE PostgreSQL migration now automatically restores the verified pre-upgrade dump (pg_restore --clean --if-exists) before halting; without it the half-migrated schema would stay live against the old code. If the restore itself fails, the halt message carries the exact manual pg_restore command. - backup_docker_db tries both python and python3 in the container, and now verifies the result too — the plain docker cp fallback can no longer pass off a torn copy as a backup. - Backup retention capped: default 5 (SERVERKIT_BACKUP_RETENTION overrides) across trees, .db and .dump files. Full-size DB copies pile up fast — a real 25G droplet carried 6.3G of stale backups. Tests: T10f (verification helpers accept/reject correctly), T10g (failed in-place PG migration auto-restores with the exact pg_restore args), T10h (retention prunes all three backup kinds to the configured cap). --- scripts/test/test_update.sh | 110 +++++++++++++++++++++++++++++++++++- scripts/update.sh | 101 +++++++++++++++++++++++++++------ 2 files changed, 194 insertions(+), 17 deletions(-) diff --git a/scripts/test/test_update.sh b/scripts/test/test_update.sh index edeeca58..9a73ef3f 100644 --- a/scripts/test/test_update.sh +++ b/scripts/test/test_update.sh @@ -390,7 +390,7 @@ cat > "$STUB_BIN/pg_dump" < "$WORK/t10e/url.txt" out=""; prev="" for a in "\$@"; do [ "\$prev" = "-f" ] && out="\$a"; prev="\$a"; done -: > "\$out" +printf 'PGDMP fake contents\n' > "\$out" exit \${PG_DUMP_RC:-0} EOF chmod +x "$STUB_BIN/pg_dump" @@ -422,6 +422,114 @@ else fi rm -f "$STUB_BIN/pg_dump" +# -------------------------------------------------------------------------- +# T10f — backup verification helpers: a backup is only a safety net when it +# is PROVEN valid. verify_sqlite_db accepts a real DB and rejects garbage; +# verify_pg_dump rejects empty/truncated dumps. +# -------------------------------------------------------------------------- +t="$WORK/t10f"; mkdir -p "$t" +PY="" +for c in python3.12 python3.11 python3; do + if command -v "$c" >/dev/null 2>&1 && "$c" -c 'import sqlite3' 2>/dev/null; then + PY="$c"; break + fi +done +if [ -z "$PY" ]; then + skip "verify_sqlite_db accepts valid DBs and rejects garbage (no working python3)" +else + "$PY" - "$t/good.db" <<'PYEOF' +import sqlite3, sys +c = sqlite3.connect(sys.argv[1]) +c.execute('CREATE TABLE t(a)') +c.commit(); c.close() +PYEOF + printf 'garbage not a database' > "$t/bad.db" + v_rc=0 + ( + set -Eeuo pipefail + verify_sqlite_db "$t/good.db" && ! verify_sqlite_db "$t/bad.db" + ) >/dev/null 2>&1 || v_rc=$? + if [ "$v_rc" -eq 0 ]; then + ok "verify_sqlite_db accepts a valid DB and rejects a malformed one" + else + bad "verify_sqlite_db misclassified a fixture (rc=$v_rc)" + fi +fi +: > "$t/empty.dump"; printf 'PGDMP\nstuff\n' > "$t/full.dump" +v_rc=0 +( + set -Eeuo pipefail + verify_pg_dump "$t/full.dump" && ! verify_pg_dump "$t/empty.dump" && ! verify_pg_dump "$t/missing.dump" +) >/dev/null 2>&1 || v_rc=$? +if [ "$v_rc" -eq 0 ]; then + ok "verify_pg_dump rejects empty/missing dumps (TOC check when pg_restore exists)" +else + bad "verify_pg_dump misclassified a fixture (rc=$v_rc)" +fi + +# -------------------------------------------------------------------------- +# T10g — a failed IN-PLACE PostgreSQL migration must trigger an automatic +# restore from the verified pre-upgrade dump: the DB is shared, so without a +# restore the half-migrated schema stays live against the old code. +# -------------------------------------------------------------------------- +t="$WORK/t10g/serverkit-b" +mkdir -p "$t/venv/bin" "$t/backend/instance" +: > "$t/venv/bin/activate" +printf 'DATABASE_URL=postgresql://user:secret@db.host:5432/serverkit\n' > "$t/.env" +printf 'PGDMP fake contents\n' > "$WORK/t10g.dump" +cat > "$STUB_BIN/flask" <<'EOF' +#!/usr/bin/env bash +exit 1 # migration blows up mid-flight +EOF +cat > "$STUB_BIN/pg_restore" < "$WORK/t10g/restore-args.txt" +exit 0 +EOF +chmod +x "$STUB_BIN/flask" "$STUB_BIN/pg_restore" +mig_rc=0 +( + set -Eeuo pipefail + DRY_RUN=0 + PRE_UPGRADE_DUMP="$WORK/t10g.dump" + migrate_database "$t" +) >/dev/null 2>&1 || mig_rc=$? +if [ "$mig_rc" -ne 0 ] && grep -q -- "--clean --if-exists -d postgresql://user:secret@db.host:5432/serverkit $WORK/t10g.dump" "$WORK/t10g/restore-args.txt" 2>/dev/null; then + ok "failed in-place PostgreSQL migration auto-restores the pre-upgrade dump" +else + bad "pg auto-restore: rc=$mig_rc, args=[$(cat "$WORK/t10g/restore-args.txt" 2>/dev/null)]" +fi +rm -f "$STUB_BIN/flask" "$STUB_BIN/pg_restore" + +# -------------------------------------------------------------------------- +# T10h — backup retention is capped (default 5, SERVERKIT_BACKUP_RETENTION +# overrides) across ALL backup kinds: full-size DB copies pile up fast on +# small VPSes (a real box carried 6.3G of stale backups on a 25G disk). +# -------------------------------------------------------------------------- +t="$WORK/t10h"; mkdir -p "$t/backups" +for i in 1 2 3 4 5 6; do + mkdir -p "$t/backups/serverkit-tree-2026010$i" + : > "$t/backups/serverkit-pre-upgrade-2026010$i-00000$i.db" + : > "$t/backups/serverkit-pre-upgrade-2026010$i-00000$i.dump" + touch -d "2026-01-0$i 00:00" "$t/backups/serverkit-tree-2026010$i" \ + "$t/backups/serverkit-pre-upgrade-2026010$i-00000$i.db" \ + "$t/backups/serverkit-pre-upgrade-2026010$i-00000$i.dump" 2>/dev/null || true +done +ret_rc=0 +( + set -Eeuo pipefail + DRY_RUN=0 BACKUP_DIR="$t/backups" SERVERKIT_BACKUP_RETENTION=3 + cleanup +) >/dev/null 2>&1 || ret_rc=$? +left_trees=$(find "$t/backups" -mindepth 1 -maxdepth 1 -name 'serverkit-tree-*' | wc -l) +left_dbs=$(find "$t/backups" -mindepth 1 -maxdepth 1 -name '*.db' | wc -l) +left_dumps=$(find "$t/backups" -mindepth 1 -maxdepth 1 -name '*.dump' | wc -l) +if [ "$ret_rc" -eq 0 ] && [ "$left_trees" -eq 3 ] && [ "$left_dbs" -eq 3 ] && [ "$left_dumps" -eq 3 ]; then + ok "cleanup prunes all backup kinds to SERVERKIT_BACKUP_RETENTION (3 kept)" +else + bad "retention: rc=$ret_rc, left trees=$left_trees db=$left_dbs dump=$left_dumps (want 3/3/3)" +fi + # -------------------------------------------------------------------------- # T11 — zero-downtime regression: reload_nginx_graceful must RELOAD a running # nginx and must NEVER stop it. Host nginx fronts every managed app, so a stop diff --git a/scripts/update.sh b/scripts/update.sh index 6cedbf90..ffd8e2db 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -730,6 +730,20 @@ migrate_database() { FLASK_ENV=production SERVERKIT_SKIP_BACKGROUND=1 flask db upgrade fi ); then + # PostgreSQL (and other external DBs): the migration ran IN PLACE + # against the SHARED database — there is no untouched slot copy to + # fall back on. Put the database back the way we found it using the + # verified pre-upgrade dump before reporting the failure. + if [ "$use_slot_db" != "1" ] && [ -n "${PRE_UPGRADE_DUMP:-}" ] && [ -f "${PRE_UPGRADE_DUMP:-}" ]; then + warn "Migration failed — restoring the pre-upgrade dump into the shared database..." + local pg_url + pg_url="$(pg_url_from_env "$work_dir")" + if command -v pg_restore >/dev/null 2>&1 \ + && pg_restore --clean --if-exists -d "$pg_url" "$PRE_UPGRADE_DUMP"; then + halt "Database migration failed; the pre-upgrade dump was restored, so the database is back to its pre-update state. The previous installation is still active." + fi + halt "Database migration failed AND the automatic restore failed. Restore MANUALLY with: pg_restore --clean --if-exists -d '$pg_url' '$PRE_UPGRADE_DUMP' — the previous installation is still active." + fi halt "Database migration failed. The previous installation is still active." fi good "Database migrated" @@ -843,6 +857,31 @@ s.close() PYEOF } +# A backup that was never verified is a hope, not a safety net. Verify every +# artifact BEFORE it is allowed to justify proceeding with the update. +verify_sqlite_db() { + local db="$1" py + [ -s "$db" ] || return 1 + py="$(locate_python)" || return 1 + [ "$("$py" -c "import sqlite3,sys;print(sqlite3.connect(sys.argv[1]).execute('PRAGMA integrity_check').fetchone()[0])" "$db" 2>/dev/null || true)" = "ok" ] +} +verify_pg_dump() { + local dump="$1" + [ -s "$dump" ] || return 1 + # Structural validation when pg_restore is available: list the dump's TOC. + if command -v pg_restore >/dev/null 2>&1; then + pg_restore -l "$dump" >/dev/null 2>&1 || return 1 + fi + return 0 +} + +# Extract the DATABASE_URL from a slot's .env, normalized for libpq: strip +# SQLAlchemy driver suffixes (postgresql+psycopg2://) and wrapping quotes. +pg_url_from_env() { + grep -E '^DATABASE_URL=' "$1/.env" 2>/dev/null | head -1 | cut -d= -f2- \ + | tr -d "\"'" | sed -E 's|^postgres(ql)?\+[a-z0-9]+://|postgresql://|' +} + # --------------------------------------------------------------------------- # Backup # --------------------------------------------------------------------------- @@ -857,9 +896,17 @@ backup_current() { if [ -f "$db_file" ]; then backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).db" # A torn backup is worse than none: it looks like a safety net but - # restores a malformed DB. Snapshot consistently or refuse to proceed. - run_or_dry copy_sqlite_db "$db_file" "$backup_file" \ - || halt "Database backup failed — refusing to update without a safety net" + # restores a malformed DB. Snapshot consistently, VERIFY the result, + # and refuse to proceed without a proven-valid net. Failed or invalid + # artifacts are removed so they are never mistaken for a good backup. + if ! run_or_dry copy_sqlite_db "$db_file" "$backup_file"; then + rm -f "$backup_file" + halt "Database backup failed — refusing to update without a safety net" + fi + if [ "$DRY_RUN" != "1" ] && ! verify_sqlite_db "$backup_file"; then + rm -f "$backup_file" + halt "Database backup verification failed — refusing to update without a VALID safety net" + fi good "Database backed up to $backup_file" elif grep -qE '^DATABASE_URL=postgres' "$active/.env" 2>/dev/null; then # PostgreSQL: the DB is external and SHARED between slots — there is @@ -869,13 +916,19 @@ backup_current() { backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).dump" command -v pg_dump >/dev/null 2>&1 \ || halt "PostgreSQL install but pg_dump is unavailable — install postgresql-client so the update has a safety net" - # Strip SQLAlchemy driver suffixes (postgresql+psycopg2://) that - # libpq can't parse, and any wrapping quotes from the .env value. local pg_url - pg_url="$(grep -E '^DATABASE_URL=' "$active/.env" | head -1 | cut -d= -f2- \ - | tr -d "\"'" | sed -E 's|^postgres(ql)?\+[a-z0-9]+://|postgresql://|')" - run_or_dry pg_dump "$pg_url" -Fc -f "$backup_file" \ - || halt "PostgreSQL backup failed — refusing to update without a safety net" + pg_url="$(pg_url_from_env "$active")" + if ! run_or_dry pg_dump "$pg_url" -Fc -f "$backup_file"; then + rm -f "$backup_file" + halt "PostgreSQL backup failed — refusing to update without a safety net" + fi + if [ "$DRY_RUN" != "1" ] && ! verify_pg_dump "$backup_file"; then + rm -f "$backup_file" + halt "PostgreSQL backup verification failed — refusing to update without a VALID safety net" + fi + # Remember the dump path so migrate_database can offer/perform an + # automatic restore if the in-place migration fails. + PRE_UPGRADE_DUMP="$backup_file" good "Database backed up to $backup_file" else warn "No SQLite database at $db_file — skipping DB backup" @@ -1426,8 +1479,14 @@ cleanup() { return 0 fi - prune_old_backups 'serverkit-tree-*' 10 - prune_old_backups 'serverkit-pre-upgrade-*.db' 10 + # Retention is capped (default 5, SERVERKIT_BACKUP_RETENTION overrides): + # pre-upgrade DB backups are full-size copies — on a small VPS, keeping + # 10 of them is itself a disk-filling time bomb (seen in the wild: 6.3G + # of stale backups on a 25G droplet). + local keep="${SERVERKIT_BACKUP_RETENTION:-5}" + prune_old_backups 'serverkit-tree-*' "$keep" + prune_old_backups 'serverkit-pre-upgrade-*.db' "$keep" + prune_old_backups 'serverkit-pre-upgrade-*.dump' "$keep" local new_version new_version="$(cat "$INSTALL_DIR/VERSION" 2>/dev/null | tr -d '\n\r ' || echo "unknown")" @@ -1528,8 +1587,9 @@ backup_docker_db() { # the container, not in the source tree. Prefer a consistent snapshot via # the container's Python (SQLite online backup API) — a plain docker cp # of the live file races the backend's writes and can come out malformed. - if docker exec -i serverkit-backend python - /app/instance/serverkit.db /tmp/.sk-snap.db <<'PYEOF' 2>/dev/null \ - && docker cp serverkit-backend:/tmp/.sk-snap.db "$backup_file" 2>/dev/null; then + local py snap_ok=0 + for py in python python3; do + if docker exec -i serverkit-backend "$py" - /app/instance/serverkit.db /tmp/.sk-snap.db <<'PYEOF' 2>/dev/null; then import sqlite3, sys s = sqlite3.connect(sys.argv[1]) d = sqlite3.connect(sys.argv[2]) @@ -1537,12 +1597,21 @@ s.backup(d) d.close() s.close() PYEOF + snap_ok=1 + break + fi + done + if [ "$snap_ok" = "1" ] && docker cp serverkit-backend:/tmp/.sk-snap.db "$backup_file" 2>/dev/null; then docker exec serverkit-backend rm -f /tmp/.sk-snap.db 2>/dev/null || true + else + docker cp serverkit-backend:/app/instance/serverkit.db "$backup_file" 2>/dev/null || true + fi + # Same rule as the host path: an unverified backup is not a safety net. + if [ -f "$backup_file" ] && verify_sqlite_db "$backup_file"; then good "Database backed up to $backup_file" - elif docker cp serverkit-backend:/app/instance/serverkit.db "$backup_file" 2>/dev/null; then - good "Database backed up to $backup_file (plain copy)" else - warn "Could not copy DB from container — continuing (backend self-backs-up before migrating)" + rm -f "$backup_file" + halt "Could not obtain a VALID database backup from the container — refusing to update without a safety net" fi } From c52927e31fe394dbe07fdcce2b3d716e1ecba792 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 23:03:46 -0400 Subject: [PATCH 05/16] ci: post merged PRs to Discord #dev-updates --- .github/workflows/discord-notify.yml | 85 ++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 .github/workflows/discord-notify.yml diff --git a/.github/workflows/discord-notify.yml b/.github/workflows/discord-notify.yml new file mode 100644 index 00000000..2053f13b --- /dev/null +++ b/.github/workflows/discord-notify.yml @@ -0,0 +1,85 @@ +name: Discord PR Notify + +on: + pull_request: + types: [closed] # merged only (filtered in the job `if:`) + +jobs: + notify: + # Only when a PR is actually merged; closed-unmerged skips silently. + if: github.event.pull_request.merged == true + runs-on: ubuntu-latest + steps: + - name: Send merged PR to Discord + env: + DISCORD_WEBHOOK_URL: ${{ secrets.DISCORD_WEBHOOK_URL }} + run: | + set -euo pipefail + + # Read everything from the event payload via jq: PR text is + # attacker-controlled input, so it must never be interpolated + # into the script (backticks/$() would execute). + TITLE=$(jq -r '.pull_request.title' "$GITHUB_EVENT_PATH") + URL=$(jq -r '.pull_request.html_url' "$GITHUB_EVENT_PATH") + AUTHOR=$(jq -r '.pull_request.user.login' "$GITHUB_EVENT_PATH") + BASE=$(jq -r '.pull_request.base.ref' "$GITHUB_EVENT_PATH") + BODY=$(jq -r '.pull_request.body // ""' "$GITHUB_EVENT_PATH") + NUMBER=$(jq -r '.pull_request.number' "$GITHUB_EVENT_PATH") + MERGED_AT=$(jq -r '.pull_request.merged_at' "$GITHUB_EVENT_PATH") + + echo "title_len=${#TITLE} body_len=${#BODY}" + + # NOTE: use [ \t], never [[:space:]] — the runner's awk is mawk, + # which silently mis-parses POSIX character classes. + # Extract bullets from the "### Highlights" section (top 5). + # Stops at the next heading or the
block. + HIGHLIGHTS=$(printf '%s\n' "$BODY" | awk ' + /^#+[ \t]*Highlights/ { f=1; next } + f && (/^#/ || /^ payload.json + + curl -sS -f -X POST -H "Content-Type: application/json" \ + -d @payload.json "$DISCORD_WEBHOOK_URL" From 4cb986ecb13d8053e22e6db9436b0ee2e2043edb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 03:10:47 +0000 Subject: [PATCH 06/16] chore: bump version to 1.7.69 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 07a8425e..3fffb541 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.68 +1.7.69 From e97b5dee3bd63b8cc35d012bf4fcb8fafc643329 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 23:18:21 -0400 Subject: [PATCH 07/16] fix(test): stub pg_restore so the pg-dump assertions stop reading host state verify_pg_dump only runs its structural `pg_restore -l` TOC check when the binary is on PATH. The harness stubs every other external command it touches (systemctl, nginx, npm, curl, docker, flask, pg_dump) but never pg_restore, so two assertions were decided by the host: * no postgresql-client -> branch skipped, check degrades to "non-empty", and the synthetic `PGDMP fake contents` fixture passes * postgresql-client present -> the real binary parses the version bytes after the PGDMP magic, rejects the fixture, verify_pg_dump returns 1 That is the CI failure. T10f failed directly; T10e failed downstream because backup_current correctly halts on an unverifiable backup. update.sh is right -- the tests leaked. Add a make_stub_pg_restore factory mimicking the real -l semantics (rejects missing/empty, rejects anything without the PGDMP magic, else prints a TOC) and install it in T10e/T10f. Give T10f a non-empty non-archive fixture too: that is the case separating the two branches inside verify_pg_dump -- the size check passes it, only the TOC listing catches it -- so the path the test was named for is now actually exercised. Verified both with and without a pg_restore on PATH: 59 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/test/stubs.sh | 28 ++++++++++++++++++++++++++++ scripts/test/test_update.sh | 14 +++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/scripts/test/stubs.sh b/scripts/test/stubs.sh index 7bca7d20..e48e1a66 100644 --- a/scripts/test/stubs.sh +++ b/scripts/test/stubs.sh @@ -111,6 +111,34 @@ make_stub_sleep_noop() { make_stub_exit "$1" 0 sleep } +# pg_restore in TOC-listing mode (`pg_restore -l`), the archive check +# verify_pg_dump degrades to when the binary is present. MUST be stubbed: +# otherwise the assertion depends on the HOST. A runner with postgresql-client +# installed runs the real binary, which parses the version bytes after the +# PGDMP magic and rejects the suite's synthetic dumps; a runner without it +# skips the check entirely and accepts anything non-empty. Same fixtures, two +# verdicts. Here a dump is valid iff it carries the PGDMP magic. +make_stub_pg_restore() { + cat > "$1/pg_restore" <<'PGR_EOF' +#!/usr/bin/env bash +dump="" +for arg in "$@"; do + case "$arg" in -*) ;; *) dump="$arg" ;; esac +done +if [ -z "$dump" ] || [ ! -s "$dump" ]; then + echo "pg_restore: error: could not open input file" >&2 + exit 1 +fi +if [ "$(head -c 5 "$dump" 2>/dev/null)" != "PGDMP" ]; then + echo "pg_restore: error: did not find magic string in file header" >&2 + exit 1 +fi +printf ';\n; Archive created at 2026-01-01 00:00:00 UTC\n;\n' +exit 0 +PGR_EOF + chmod +x "$1/pg_restore" +} + # make_fresh_box_fixture — lay out the degenerate fresh-box world: # /bin PATH-prepend stubs: systemctl exit 5, # docker with zero containers, curl diff --git a/scripts/test/test_update.sh b/scripts/test/test_update.sh index 9a73ef3f..bf956633 100644 --- a/scripts/test/test_update.sh +++ b/scripts/test/test_update.sh @@ -394,6 +394,7 @@ printf 'PGDMP fake contents\n' > "\$out" exit \${PG_DUMP_RC:-0} EOF chmod +x "$STUB_BIN/pg_dump" +make_stub_pg_restore "$STUB_BIN" bk_rc=0 ( set -Eeuo pipefail @@ -420,7 +421,7 @@ if [ "$bk_rc" -ne 0 ]; then else bad "backup_current proceeded despite a failed pg_dump (rc=0)" fi -rm -f "$STUB_BIN/pg_dump" +rm -f "$STUB_BIN/pg_dump" "$STUB_BIN/pg_restore" # -------------------------------------------------------------------------- # T10f — backup verification helpers: a backup is only a safety net when it @@ -456,16 +457,23 @@ PYEOF fi fi : > "$t/empty.dump"; printf 'PGDMP\nstuff\n' > "$t/full.dump" +# A non-archive that is merely NON-EMPTY is the fixture that separates the two +# verify_pg_dump branches: the size check passes it, only the TOC listing +# catches it. Requires the stub — see make_stub_pg_restore. +printf 'ERROR: connection refused\n' > "$t/garbage.dump" +make_stub_pg_restore "$STUB_BIN" v_rc=0 ( set -Eeuo pipefail - verify_pg_dump "$t/full.dump" && ! verify_pg_dump "$t/empty.dump" && ! verify_pg_dump "$t/missing.dump" + verify_pg_dump "$t/full.dump" && ! verify_pg_dump "$t/empty.dump" \ + && ! verify_pg_dump "$t/missing.dump" && ! verify_pg_dump "$t/garbage.dump" ) >/dev/null 2>&1 || v_rc=$? if [ "$v_rc" -eq 0 ]; then - ok "verify_pg_dump rejects empty/missing dumps (TOC check when pg_restore exists)" + ok "verify_pg_dump rejects empty/missing/non-archive dumps (TOC check via pg_restore)" else bad "verify_pg_dump misclassified a fixture (rc=$v_rc)" fi +rm -f "$STUB_BIN/pg_restore" # -------------------------------------------------------------------------- # T10g — a failed IN-PLACE PostgreSQL migration must trigger an automatic From 69b6ffb94a558f624ee3ecd9c82fe686fc4fd381 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 03:19:39 +0000 Subject: [PATCH 08/16] chore: bump version to 1.7.70 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 3fffb541..994c3610 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.69 +1.7.70 From 664a5e62bd9e22c5595ad864b97f6f8597946398 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sun, 26 Jul 2026 23:33:51 -0400 Subject: [PATCH 09/16] fix(update): don't gate SQLite backup/verify on a 3.11/3.12 interpreter copy_sqlite_db and verify_sqlite_db reused locate_python, which enforces 3.11 <= v <= 3.12. That bound is correct for rebuild_virtualenv and preflight_check, but snapshotting or integrity-checking a SQLite file needs nothing beyond the stdlib sqlite3 module. Reusing it coupled the DATA-safety paths to the venv-builder constraint and blocked updates for a reason unrelated to the data: * an all-Docker install returns before preflight_check ever runs and does every real step in the container -- yet backup_docker_db snapshots with the CONTAINER's python and then verified the extracted file with the HOST's. On a stock EL9 (python3.9) or Ubuntu 22.04 (python3.10) host that halted with "Could not obtain a VALID database backup", blaming the backup for a missing interpreter. * backup_current halts when it cannot snapshot, so those hosts could never update at all. Split out locate_sqlite_python: any candidate that can `import sqlite3` wins, and the install's own venv interpreter is preferred -- on a real box it is guaranteed present, guaranteed to carry the module, and independent of PATH. locate_python keeps the 3.11/3.12 bound where it belongs. Surfaced by the scripts-ci unit matrix: almalinux:9 and rockylinux:9 ship python3.9, so the suite's own probe accepted an interpreter locate_python rejected, and copy_sqlite_db/verify_sqlite_db failed there while passing on python3.12 runners. T10i pins the contract: on a box where every candidate reports 3.9 (locate_python fails), the helpers still resolve an interpreter, prefer the slot's venv, and round-trip 100 rows. Fails on the parent commit, passes here. Verified on the full 7-image CI matrix (ubuntu 22.04/24.04, debian:12, rockylinux:9, almalinux:9, fedora:40, opensuse/leap:15.5), all six shell suites: 0 failed. test_update is 63 passed / 0 failed on every image carrying a python3. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/test/test_update.sh | 58 +++++++++++++++++++++++++++++++++++++ scripts/update.sh | 45 +++++++++++++++++++++++++--- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/scripts/test/test_update.sh b/scripts/test/test_update.sh index bf956633..31e4d388 100644 --- a/scripts/test/test_update.sh +++ b/scripts/test/test_update.sh @@ -475,6 +475,64 @@ else fi rm -f "$STUB_BIN/pg_restore" +# -------------------------------------------------------------------------- +# T10i — the SQLite helpers must NOT inherit the venv-builder's 3.11/3.12 +# constraint. Snapshotting and integrity-checking a DB needs only an +# interpreter carrying the stdlib sqlite3 module; demanding 3.11/3.12 blocked +# updates on stock EL9 (python3.9) and Ubuntu 22.04 (python3.10) boxes for a +# reason unrelated to the data — including all-Docker installs, which return +# before preflight_check and do every real step inside the container, yet +# verified the extracted DB with the HOST's interpreter. +# -------------------------------------------------------------------------- +t="$WORK/t10i"; mkdir -p "$t/bin" "$t/slot/backend/instance" "$t/slot/venv/bin" +if [ -z "$PY" ]; then + skip "SQLite helpers work without a 3.11/3.12 interpreter (no working python3)" +else + REAL_PY="$(command -v "$PY")" + # An interpreter that answers a version probe with 3.9 but is otherwise a + # real Python: makes locate_python fail exactly as it does on EL9, while + # `import sqlite3` keeps working. Shadowing all four candidate names keeps + # the precondition true regardless of what the host actually has. + for n in python3.12 python3.11 python3 python; do + cat > "$t/bin/$n" </dev/null 2>&1 || el9_rc=$? + rows="$("$PY" -c "import sqlite3,sys;print(sqlite3.connect(sys.argv[1]).execute('SELECT COUNT(*) FROM t').fetchone()[0])" "$t/snap.db" 2>/dev/null || true)" + if [ "$el9_rc" -eq 0 ] && [ "$rows" = "100" ]; then + ok "SQLite snapshot/verify need only sqlite3, not Python 3.11/3.12 (EL9/22.04 boxes)" + else + bad "sqlite helpers still gated on locate_python: rc=$el9_rc, rows=[$rows], expected 100" + fi +fi + # -------------------------------------------------------------------------- # T10g — a failed IN-PLACE PostgreSQL migration must trigger an automatic # restore from the verified pre-upgrade dump: the DB is shared, so without a diff --git a/scripts/update.sh b/scripts/update.sh index ffd8e2db..e715ee4c 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -843,9 +843,40 @@ atomic_switch() { # even while the panel is writing. Python 3.11/3.12 is guaranteed by # pre-flight; without it we fail rather than fall back to a racy cp. # --------------------------------------------------------------------------- +# Snapshotting or integrity-checking a SQLite file needs nothing more than an +# interpreter carrying the stdlib `sqlite3` module — NOT the 3.11/3.12 that +# rebuild_virtualenv and preflight_check legitimately demand. Reusing +# locate_python here silently coupled the DATA-safety paths to the venv-builder +# constraint, and blocked updates for a reason unrelated to the data: +# * an all-Docker install returns before preflight_check ever runs and does +# all its work in the container, yet backup_docker_db verified the extracted +# DB on the HOST — so a stock EL9 (python3.9) or Ubuntu 22.04 (python3.10) +# host halted with "Could not obtain a VALID database backup", blaming the +# backup for a missing interpreter. +# * backup_current halts when it cannot snapshot, so the same boxes could +# never update at all. +# Prefer the install's own venv interpreter: on a real box it is guaranteed +# present, guaranteed to have the module, and independent of what is on PATH. +locate_sqlite_python() { + local c + for c in "$@" python3.12 python3.11 python3 python; do + [ -n "$c" ] || continue + command -v "$c" >/dev/null 2>&1 || continue + "$c" -c 'import sqlite3' >/dev/null 2>&1 || continue + printf '%s' "$c" + return 0 + done + return 1 +} + copy_sqlite_db() { - local src="$1" dest="$2" py - py="$(locate_python)" || { warn "No Python 3.11/3.12 available for a consistent DB snapshot"; return 1; } + local src="$1" dest="$2" py slot + # The DB lives at /backend/instance/serverkit.db — derive that slot's + # interpreter as the preferred candidate. A src outside a slot leaves the + # path unchanged, yielding a non-executable candidate that is simply skipped. + slot="${src%/backend/instance/*}" + py="$(locate_sqlite_python "$slot/venv/bin/python")" \ + || { warn "No Python with the sqlite3 module available for a consistent DB snapshot"; return 1; } "$py" - "$src" "$dest" <<'PYEOF' import sqlite3, sys src, dest = sys.argv[1], sys.argv[2] @@ -860,9 +891,15 @@ PYEOF # A backup that was never verified is a hope, not a safety net. Verify every # artifact BEFORE it is allowed to justify proceeding with the update. verify_sqlite_db() { - local db="$1" py + local db="$1" py active hint="" [ -s "$db" ] || return 1 - py="$(locate_python)" || return 1 + # Backups live in BACKUP_DIR, outside any slot, so hint at the active + # install's interpreter instead of deriving one from the file's path. + active="$(active_real_dir 2>/dev/null || true)" + if [ -n "$active" ]; then + hint="$active/venv/bin/python" + fi + py="$(locate_sqlite_python "$hint")" || return 1 [ "$("$py" -c "import sqlite3,sys;print(sqlite3.connect(sys.argv[1]).execute('PRAGMA integrity_check').fetchone()[0])" "$db" 2>/dev/null || true)" = "ok" ] } verify_pg_dump() { From e73e6fc6b4cff6ccb5bdf2b909b91a04a7435c48 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 03:40:09 +0000 Subject: [PATCH 10/16] chore: bump version to 1.7.71 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 994c3610..a63bf004 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.70 +1.7.71 From 5bf5e50f624bbf0122f459f4691b3e735093f41a Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Mon, 27 Jul 2026 00:06:12 -0400 Subject: [PATCH 11/16] fix(update): pick the backup engine by DATABASE_URL, not by which files exist backup_current tested `[ -f serverkit.db ]` FIRST and only fell through to the PostgreSQL branch when no file was there. An install migrated SQLite -> PostgreSQL keeps its old serverkit.db, and both deploy paths copy it forward on file-existence alone (deploy_source, deploy_release), so the stale file is carried into every future slot and never ages out. Such a box backed up the abandoned file, skipped pg_dump entirely, and left PRE_UPGRADE_DUMP unset -- which migrate_database reads as "${PRE_UPGRADE_DUMP:-}", so its auto-restore silently did nothing. PostgreSQL is the one engine that migrates IN PLACE and cannot roll back by switching slots, so that dump is its only way home: the exact safety net a61eab1/b48256f added was disabled for it. migrate_database has always keyed off DATABASE_URL (its sqlite check). Test the declared engine first so the two functions agree. Deliberately NOT `grep DATABASE_URL=sqlite && [ -f db ]` as the reviewer suggested: backend/config.py defaults DATABASE_URL when absent, so any install whose .env lacks the line would fall to the else branch and update with NO backup at all -- trading a mismatched backup for none. An explicit `postgres` URL is a positive signal; the absence of a `sqlite` one is not. Also: * name the engine when it is one we cannot dump. "No SQLite database ..." read like a benign nothing-to-do on a MySQL install that in fact had no safety net. Prints the scheme only -- a DATABASE_URL carries a password and this text lands in the update log. * drop the stale "Python 3.11/3.12 is guaranteed by pre-flight" comment, contradicted by locate_sqlite_python two lines below it (664a5e6). * discord-notify: skip cleanly when DISCORD_WEBHOOK_URL is absent. Secrets are not exposed to `pull_request` runs from a fork, so a merged fork PR failed the job on an empty curl URL. Not switching to pull_request_target for this -- handing a write-scoped token and the webhook to an outsider-triggered run is a poor trade for a chat notification. Tests: a PostgreSQL install carrying a leftover serverkit.db must still dump and set PRE_UPGRADE_DUMP (fails on the parent commit, rc=1); an undumpable engine must be named without leaking the password. Neither needs python3, so both run on all 7 images. Workflow verified end-to-end against a synthetic event payload: fork PR exits 0 without calling curl, normal PR still builds the same embed. Verified on the full CI matrix (ubuntu 22.04/24.04, debian:12, rockylinux:9, almalinux:9, fedora:40, opensuse/leap:15.5), all six suites: 0 failed. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/discord-notify.yml | 11 ++++++ scripts/test/test_update.sh | 48 ++++++++++++++++++++++ scripts/update.sh | 59 +++++++++++++++++++--------- 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/.github/workflows/discord-notify.yml b/.github/workflows/discord-notify.yml index 2053f13b..76e77969 100644 --- a/.github/workflows/discord-notify.yml +++ b/.github/workflows/discord-notify.yml @@ -16,6 +16,17 @@ jobs: run: | set -euo pipefail + # Repository secrets are NOT exposed to `pull_request` runs triggered + # from a fork, so a merged fork PR would reach curl with an empty URL + # and fail the job for no useful reason. Skip cleanly instead. The + # fix is deliberately NOT `pull_request_target`: that would hand a + # write-scoped token and this webhook to an outsider-triggered run, + # which is a poor trade for a chat notification. + if [ -z "${DISCORD_WEBHOOK_URL:-}" ]; then + echo "DISCORD_WEBHOOK_URL unavailable (fork PR?) — skipping notification." + exit 0 + fi + # Read everything from the event payload via jq: PR text is # attacker-controlled input, so it must never be interpolated # into the script (backticks/$() would execute). diff --git a/scripts/test/test_update.sh b/scripts/test/test_update.sh index 31e4d388..e9472265 100644 --- a/scripts/test/test_update.sh +++ b/scripts/test/test_update.sh @@ -421,6 +421,54 @@ if [ "$bk_rc" -ne 0 ]; then else bad "backup_current proceeded despite a failed pg_dump (rc=0)" fi + +# The engine is decided by DATABASE_URL, never by which files happen to exist. +# An install migrated SQLite -> PostgreSQL keeps its old serverkit.db (every +# deploy copies it forward on file-existence alone), and backing THAT up +# instead of dumping left PRE_UPGRADE_DUMP unset — quietly disabling +# migrate_database's auto-restore, which reads it as "${PRE_UPGRADE_DUMP:-}", +# on the one engine that migrates in place and cannot roll back by switching +# slots. Needs no python3: the PostgreSQL path is pg_dump + pg_restore only. +t="$WORK/t10e2"; mkdir -p "$t/serverkit/backend/instance" "$t/backups" +printf 'DATABASE_URL=postgresql://user:secret@db.host:5432/serverkit\n' > "$t/serverkit/.env" +printf 'stale sqlite file left over from before the postgres migration\n' \ + > "$t/serverkit/backend/instance/serverkit.db" +lo_rc=0 +( + set -Eeuo pipefail + DRY_RUN=0 INSTALL_DIR="$t/serverkit" BACKUP_DIR="$t/backups" + backup_current + printf '%s\n' "${PRE_UPGRADE_DUMP:-}" > "$t/dump-var.txt" +) >/dev/null 2>&1 || lo_rc=$? +lo_dump="$(ls "$t/backups"/serverkit-pre-upgrade-*.dump 2>/dev/null | head -1)" +lo_db="$(ls "$t/backups"/serverkit-pre-upgrade-*.db 2>/dev/null | head -1)" +if [ "$lo_rc" -eq 0 ] && [ -n "$lo_dump" ] && [ -z "$lo_db" ] \ + && [ "$(cat "$t/dump-var.txt" 2>/dev/null)" = "$lo_dump" ]; then + ok "a leftover serverkit.db never diverts a PostgreSQL install away from pg_dump" +else + bad "pg install w/ stale sqlite file: rc=$lo_rc, dump=[$lo_dump], db=[$lo_db], PRE_UPGRADE_DUMP=[$(cat "$t/dump-var.txt" 2>/dev/null)]" +fi + +# An engine this updater cannot dump (MySQL) must say so. "No SQLite database" +# reads like a benign nothing-to-do while the update proceeds with no safety +# net at all. The warning names the SCHEME only — a DATABASE_URL carries a +# password, and this text lands in the update log. +t="$WORK/t10e3"; mkdir -p "$t/serverkit" "$t/backups" +printf 'DATABASE_URL=mysql://user:hunter2@db.host:3306/serverkit\n' > "$t/serverkit/.env" +my_rc=0 +my_out="$( + set -Eeuo pipefail + DRY_RUN=0 INSTALL_DIR="$t/serverkit" BACKUP_DIR="$t/backups" + backup_current 2>&1 +)" || my_rc=$? +if [ "$my_rc" -eq 0 ] \ + && printf '%s' "$my_out" | grep -q "mysql" \ + && printf '%s' "$my_out" | grep -q "NO database backup" \ + && ! printf '%s' "$my_out" | grep -q "hunter2"; then + ok "an undumpable engine is named in the warning, without leaking the password" +else + bad "mysql warning: rc=$my_rc, out=[$my_out]" +fi rm -f "$STUB_BIN/pg_dump" "$STUB_BIN/pg_restore" # -------------------------------------------------------------------------- diff --git a/scripts/update.sh b/scripts/update.sh index e715ee4c..a0a19bdb 100644 --- a/scripts/update.sh +++ b/scripts/update.sh @@ -840,8 +840,8 @@ atomic_switch() { # "database disk image is malformed" — which then sinks the migration in the # new slot, and silently poisons the pre-upgrade backup that was supposed to # be the safety net. The SQLite online backup API snapshots a consistent DB -# even while the panel is writing. Python 3.11/3.12 is guaranteed by -# pre-flight; without it we fail rather than fall back to a racy cp. +# even while the panel is writing, using any interpreter that can import +# sqlite3; with none available we fail rather than fall back to a racy cp. # --------------------------------------------------------------------------- # Snapshotting or integrity-checking a SQLite file needs nothing more than an # interpreter carrying the stdlib `sqlite3` module — NOT the 3.11/3.12 that @@ -930,22 +930,17 @@ backup_current() { active="$(active_real_dir)" db_file="$active/backend/instance/serverkit.db" - if [ -f "$db_file" ]; then - backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).db" - # A torn backup is worse than none: it looks like a safety net but - # restores a malformed DB. Snapshot consistently, VERIFY the result, - # and refuse to proceed without a proven-valid net. Failed or invalid - # artifacts are removed so they are never mistaken for a good backup. - if ! run_or_dry copy_sqlite_db "$db_file" "$backup_file"; then - rm -f "$backup_file" - halt "Database backup failed — refusing to update without a safety net" - fi - if [ "$DRY_RUN" != "1" ] && ! verify_sqlite_db "$backup_file"; then - rm -f "$backup_file" - halt "Database backup verification failed — refusing to update without a VALID safety net" - fi - good "Database backed up to $backup_file" - elif grep -qE '^DATABASE_URL=postgres' "$active/.env" 2>/dev/null; then + # Branch on the DECLARED engine, not on which files happen to exist. An + # install migrated SQLite -> PostgreSQL keeps its old serverkit.db on disk + # (both deploy paths copy it forward on file-existence alone, so it is + # carried into every future slot), and a file-first test then backed up + # that abandoned file, skipped the dump, and left PRE_UPGRADE_DUMP unset — + # silently disabling the auto-restore in migrate_database, which reads it + # as "${PRE_UPGRADE_DUMP:-}" and just does nothing. PostgreSQL is the ONE + # install type that migrates in place and cannot roll back by switching + # slots, so that dump is its only way home. migrate_database has always + # keyed off DATABASE_URL; this keeps the two functions in agreement. + if grep -qE '^DATABASE_URL=postgres' "$active/.env" 2>/dev/null; then # PostgreSQL: the DB is external and SHARED between slots — there is # no per-slot file to snapshot, and the migration runs in place. A # pre-upgrade dump is the only safety net AND the only way back: @@ -967,8 +962,34 @@ backup_current() { # automatic restore if the in-place migration fails. PRE_UPGRADE_DUMP="$backup_file" good "Database backed up to $backup_file" + elif [ -f "$db_file" ]; then + backup_file="$BACKUP_DIR/serverkit-pre-upgrade-$(date +%Y%m%d-%H%M%S).db" + # A torn backup is worse than none: it looks like a safety net but + # restores a malformed DB. Snapshot consistently, VERIFY the result, + # and refuse to proceed without a proven-valid net. Failed or invalid + # artifacts are removed so they are never mistaken for a good backup. + if ! run_or_dry copy_sqlite_db "$db_file" "$backup_file"; then + rm -f "$backup_file" + halt "Database backup failed — refusing to update without a safety net" + fi + if [ "$DRY_RUN" != "1" ] && ! verify_sqlite_db "$backup_file"; then + rm -f "$backup_file" + halt "Database backup verification failed — refusing to update without a VALID safety net" + fi + good "Database backed up to $backup_file" else - warn "No SQLite database at $db_file — skipping DB backup" + # Neither engine matched. Say which one, and say plainly that the + # update is proceeding unprotected — "No SQLite database" reads like a + # benign nothing-to-do on, say, a MySQL install that in fact has no + # safety net at all. + local db_scheme + db_scheme="$(grep -E '^DATABASE_URL=' "$active/.env" 2>/dev/null | head -1 \ + | cut -d= -f2- | tr -d "\"'" | cut -d: -f1)" + if [ -n "$db_scheme" ] && [ "$db_scheme" != "sqlite" ]; then + warn "DATABASE_URL uses '$db_scheme', which this updater cannot dump — proceeding with NO database backup" + else + warn "No SQLite database at $db_file — skipping DB backup" + fi fi local tree_backup From 45ebb8e878e2769b5817b19b384a0f3ce334c039 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 04:07:05 +0000 Subject: [PATCH 12/16] chore: bump version to 1.7.72 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index a63bf004..9eacb6a0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.71 +1.7.72 From 1f2381caa8603616d58ee43e4c6350fb2472404f Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Mon, 27 Jul 2026 00:11:14 -0400 Subject: [PATCH 13/16] ci: retry the distro-image pull instead of failing the job on a Hub hiccup `docker run` pulls implicitly, so a registry timeout ("context deadline exceeded" reaching registry-1.docker.io) killed the whole matrix leg with exit 125 before a single test ran, and cost a manual re-run. Seven images per matrix run makes that a recurring tax rather than a one-off. Pull up front with backoff (10/20/30s) and only then run the suites. ONLY the pull is retried. Wrapping `docker run` would re-run the suites on failure too, which would quietly turn a flaky TEST green -- the opposite of what this job is for. Left test-system-utils.yml alone: it uses a job-level `container:`, which the runner pulls itself, so there is nothing to wrap. Verified against the step text extracted from the YAML, under `bash -e` (the shell GitHub actually uses for `run:`): registry down -> 3 retry warnings then one error annotation and exit 1; flaky -> recovers and exits 0; healthy -> exits 0 immediately with no retry noise. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/scripts-ci.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.github/workflows/scripts-ci.yml b/.github/workflows/scripts-ci.yml index 44e557c8..e9d4cab8 100644 --- a/.github/workflows/scripts-ci.yml +++ b/.github/workflows/scripts-ci.yml @@ -110,6 +110,28 @@ jobs: - 'opensuse/leap:15.5' steps: - uses: actions/checkout@v4 + + - name: Pull ${{ matrix.image }} (retried — Docker Hub is flaky) + run: | + # `docker run` pulls implicitly, so a registry hiccup ("context + # deadline exceeded" reaching registry-1.docker.io) failed the whole + # job with exit 125 and cost a manual re-run. Seven images per matrix + # run makes that a recurring tax, so pull up front and retry. + # + # ONLY the pull is retried. Wrapping `docker run` would re-run the + # suites on failure too, quietly turning a flaky TEST green. + for attempt in 1 2 3 4; do + if docker pull --quiet "${{ matrix.image }}"; then + exit 0 + fi + if [ "$attempt" -lt 4 ]; then + echo "::warning::pull of ${{ matrix.image }} failed (attempt $attempt/4) — retrying" + sleep $((attempt * 10)) + fi + done + echo "::error::could not pull ${{ matrix.image }} after 4 attempts — Docker Hub unreachable" + exit 1 + - name: bash -n + unit suites on ${{ matrix.image }} run: | docker run --rm -v "$PWD:/src" -w /src "${{ matrix.image }}" sh -c ' From 67cbb10c05ac96494dcffa1581d0ded39095f795 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 04:11:56 +0000 Subject: [PATCH 14/16] chore: bump version to 1.7.73 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 9eacb6a0..2edd9ec6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.72 +1.7.73 From 42905c357292f25982c59f7e3dd2b5a563e4bbb5 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Mon, 27 Jul 2026 00:29:15 -0400 Subject: [PATCH 15/16] ci: fall back to a Hub mirror when Docker Hub is unreachable The retry added in 1f2381c behaved exactly as designed and still failed the job: Hub was unreachable for the full two minutes ("Client.Timeout exceeded while awaiting headers" on every attempt), not flaky. Retrying a single registry harder cannot survive that registry being down -- it only delays the failure. Two consecutive runs were blocked this way. Fall back to Google's public Hub mirror, a separate failure domain, and re-tag to the original name so the run step needs no changes (`docker run` resolves the local image; default pull policy is "missing"). Hub stays primary. Ref construction differs by image kind -- `mirror.gcr.io/library/` for official images, `mirror.gcr.io//` for namespaced ones -- and all seven matrix images were confirmed to resolve on the mirror via `docker manifest inspect`, opensuse/leap included. ONLY the pull falls back. Wrapping `docker run` would re-run the suites on failure too, quietly turning a flaky TEST green. Verified locally end-to-end against the step text extracted from this YAML, under `bash -e`, with a docker shim reproducing the runner's exact Hub error: * Hub down, mirror up -> 3 warnings, REAL pull from mirror.gcr.io, re-tag, and `docker run ` then starts a container with the repo mounted while Hub is still blocked * both down -> 6 warnings, one error annotation, exit 1 (fails honestly) * Hub healthy -> exits 0 on the first pull, no mirror attempt, no noise Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/scripts-ci.yml | 56 +++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/.github/workflows/scripts-ci.yml b/.github/workflows/scripts-ci.yml index e9d4cab8..24df61c2 100644 --- a/.github/workflows/scripts-ci.yml +++ b/.github/workflows/scripts-ci.yml @@ -111,25 +111,51 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Pull ${{ matrix.image }} (retried — Docker Hub is flaky) + - name: Pull ${{ matrix.image }} (Docker Hub, then mirror) + env: + IMAGE: ${{ matrix.image }} run: | - # `docker run` pulls implicitly, so a registry hiccup ("context - # deadline exceeded" reaching registry-1.docker.io) failed the whole - # job with exit 125 and cost a manual re-run. Seven images per matrix - # run makes that a recurring tax, so pull up front and retry. + # `docker run` pulls implicitly, so an unreachable registry kills the + # matrix leg with exit 125 before a single test runs. Docker Hub did + # exactly that on two consecutive runs — a sustained egress blackout + # from the runner ("Client.Timeout exceeded while awaiting headers"), + # not a rate limit, so retrying Hub alone rides it out for two + # minutes and then fails anyway. Fall back to Google's public Hub + # mirror, a separate failure domain, and re-tag to the original name + # so the run step below needs no changes. # # ONLY the pull is retried. Wrapping `docker run` would re-run the # suites on failure too, quietly turning a flaky TEST green. - for attempt in 1 2 3 4; do - if docker pull --quiet "${{ matrix.image }}"; then - exit 0 - fi - if [ "$attempt" -lt 4 ]; then - echo "::warning::pull of ${{ matrix.image }} failed (attempt $attempt/4) — retrying" - sleep $((attempt * 10)) - fi - done - echo "::error::could not pull ${{ matrix.image }} after 4 attempts — Docker Hub unreachable" + case "$IMAGE" in + */*) MIRROR="mirror.gcr.io/$IMAGE" ;; # namespaced: opensuse/leap + *) MIRROR="mirror.gcr.io/library/$IMAGE" ;; # official image + esac + + try_pull() { + ref="$1"; tries="$2" + for i in $(seq 1 "$tries"); do + if docker pull --quiet "$ref"; then + return 0 + fi + echo "::warning::pull of $ref failed (attempt $i/$tries)" + if [ "$i" -lt "$tries" ]; then + sleep $((i * 5)) + fi + done + return 1 + } + + if try_pull "$IMAGE" 3; then + exit 0 + fi + echo "::warning::Docker Hub unreachable — falling back to $MIRROR" + if try_pull "$MIRROR" 3; then + # Re-tag so `docker run "$IMAGE"` resolves locally and never + # reaches for Hub again (default pull policy is "missing"). + docker tag "$MIRROR" "$IMAGE" + exit 0 + fi + echo "::error::could not pull $IMAGE from Docker Hub or $MIRROR" exit 1 - name: bash -n + unit suites on ${{ matrix.image }} From 63b835c355eca245312c4454c43a8ab1d39929f5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Jul 2026 04:34:40 +0000 Subject: [PATCH 16/16] chore: bump version to 1.7.74 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 2edd9ec6..8d47170e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.73 +1.7.74