feat(selfhost): add a backup restore/validation drill (verify-backup.sh) - #2010
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 7efaf83 | Commit Preview URL Branch Preview URL |
Jul 01 2026, 09:13 AM |
JSONbored
added this to the
M3.5 — Agent Layer Phase 2 (maintainer auto-maintain) milestone
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-01 09:23:46 UTC
⏸️ Suggested Action - Manual Review
Review summary Blockers
Nits — 6 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
loopover-orb
Bot
added
gittensor
gittensor:feature
|
Fixed. Root cause was worse than a missing package: `docker compose run --rm backup sh /verify-backup.sh` replaces `command:`, not `entrypoint:`, and the old entrypoint (`/bin/sh -c`) folded the apk install into `command:` — so an override skipped the install AND, because `-c`'s extra args become the nested shell's $0/$1 rather than a script to run, it collapsed into a bare interactive `sh` that just hangs reading stdin. (Confirmed live with real `docker compose run` — it hung until killed. The pre-existing documented `sh /backup.sh` one-shot had the identical bug.) Fix: the install now lives in the entrypoint, ending in `exec "$@"`, with a placeholder `sh` entrypoint arg so the real payload (default loop or a `run` override) lands in `"$@"` starting at $1 — mirrors the inline entrypoint-script pattern this file already uses for grafana. Verified against real Docker, not just reasoned about:
New commit: 24d76af. |
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
superagent-security
Bot
added
the
pr:flagged
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2010 +/- ##
=======================================
Coverage 95.71% 95.71%
=======================================
Files 222 222
Lines 24620 24620
Branches 8936 8936
=======================================
Hits 23565 23565
Misses 432 432
Partials 623 623 🚀 New features to boost your workflow:
|
The backup profile writes Postgres pg_dump / SQLite backups but nothing checked
they are restorable. Add scripts/verify-backup.sh, run on demand via the backup
service, which verifies the newest backup (or an explicit file) WITHOUT touching
the live database:
- Postgres .dump: `pg_restore --list` must parse the archive and find a
non-empty table of contents.
- SQLite .sqlite.gz: gzip integrity + `PRAGMA integrity_check` on a temp copy.
- Opt-in scratch restore (VERIFY_RESTORE_SCRATCH=1 + a dedicated scratch DB
URL): restores the dump into a throwaway database and sanity-counts tables.
It refuses to run when the scratch URL is empty or equals the live source,
so a destructive restore cannot hit production by accident.
Mounts the script into the backup service and passes the scratch env through;
documents the drill (and what a healthy run looks like) in the backup docs page.
Covered by test/unit/selfhost-verify-backup-script.test.ts (fake pg_restore /
psql / sqlite3, real gzip): validation pass/fail, the scratch guards, the
scratch happy path, explicit-file mode, and the SQLite integrity/gzip failures.
… packages `docker compose run --rm backup sh /verify-backup.sh` REPLACES the service's `command:`, not its `entrypoint:`. The old entrypoint (`/bin/sh -c`) took the package-install as part of `command:`, so an on-demand run skipped it entirely — worse, since `-c`'s extra positional args become the nested shell's $0/$1 rather than a script to execute, the override became a bare interactive `sh` that just hangs reading stdin (confirmed with a live `docker compose run`; the documented `sh /backup.sh` one-shot had the exact same pre-existing bug). Move the package install into the entrypoint and end it with `exec "$@"`, with a placeholder `sh` entrypoint arg so the real payload — the default loop or a `run` override — lands in "$@" starting at $1, not $0. Mirrors the inline entrypoint-script pattern this file already uses for grafana. Verified against real Docker: `docker compose --profile backup up -d backup` still completes its normal backup.sh cycle, and `docker compose run --rm backup sh /verify-backup.sh` now actually executes (previously hung) with pg_restore present and invoked (confirmed via a seeded dummy dump producing pg_restore's own parse error, not "command not found").
…ection URLs `[ "$scratch" = "$PG_DB" ]` compared connection strings byte-for-byte, so a scratch URL that is a DIFFERENTLY-SPELLED equivalent of the live backup source (postgresql:// vs postgres://, a host alias, an explicit vs default port) would pass the guard and let `pg_restore --clean` drop objects in the live database. Replace the string compare with an identity check: ask Postgres itself for `current_database() || '@' || pg_control_system().system_identifier` on both connections and compare THAT. system_identifier is a random 64-bit value fixed for the life of a cluster's data directory, independent of how the connection was dialed, and PUBLIC has EXECUTE on pg_control_system() by default (no privilege escalation needed) — verified against a real non-superuser role. Deliberately NOT network-address-based (e.g. inet_server_addr()): testing against real Postgres showed the same server can report different addresses across connections over different address families (IPv6 vs IPv4 loopback), which would have reopened a false "these differ" negative — the wrong direction for a safety guard. Any failure to fingerprint either side now aborts (fails closed) rather than assuming the databases differ. Validated two ways: - test/unit/selfhost-verify-backup-script.test.ts: a fake psql keyed by connection string simulates two differently-spelled URLs resolving to the SAME database (the exact reported bypass) plus fail-closed cases when either identity query fails. - A real end-to-end run against Postgres 16 (matching the backup image): refuses a byte-identical URL, refuses a differently-spelled equivalent of the same database, and correctly allows + completes a restore into a genuinely different database on the same cluster — with the live database's table confirmed untouched throughout.
24d76af to
7efaf83
Compare
|
Confirmed valid, fixed. `[ "$scratch" = "$PG_DB" ]` compared connection strings byte-for-byte, so a differently-spelled equivalent (`postgresql://` vs `postgres://`, a host alias, explicit vs default port) would slip past the guard. Replaced the string compare with an identity check: ask Postgres itself for `current_database() || '@' || pg_control_system().system_identifier` on both connections and compare that. `system_identifier` is a random 64-bit value fixed for the life of a cluster's data directory — independent of how the connection was dialed — and PUBLIC has EXECUTE on `pg_control_system()` by default (verified against a real non-superuser role, no privilege escalation needed). I deliberately avoided a network-address-based fingerprint (e.g. `inet_server_addr()`): testing against a real Postgres 16 instance showed the SAME server can report different addresses across connections over different address families (IPv6 `::1` vs IPv4 `127.0.0.1` loopback) — a false "these differ" that would have reopened a gap in the wrong direction for a safety guard. Any failure to fingerprint either side now aborts (fails closed). Validated two ways:
New commit: 7efaf83. |
superagent-security
Bot
removed
the
pr:flagged
1 participant
Part of #2011.
Summary
The
backupprofile writes Postgrespg_dump/ SQLite backups, but nothing verified they are actually restorable — a backup you can't restore is not a backup. This addsverify-backup.sh, an on-demand drill that checks the newest backup (or a specific file) without touching the live database, plus an opt-in scratch restore.What changed
scripts/verify-backup.sh(new):.dump→pg_restore --listmust parse the archive and find a non-empty table of contents..sqlite.gz→gzip -tintegrity +PRAGMA integrity_checkon a temp copy.VERIFY_RESTORE_SCRATCH=1+GITTENSORY_VERIFY_SCRATCH_DATABASE_URL): restores the dump into a throwaway database and sanity-counts tables. It refuses to run when the scratch URL is empty or equals the live backup source, so a destructivepg_restore --cleancannot hit production by accident.docker-compose.yml— mounts the script into thebackupservice (sh /verify-backup.sh) and passes the opt-in scratch env through.Why
Goal #1 of the Postgres reliability hardening (#2011): give operators a safe, documented way to prove backups restore, prioritized as the smallest high-value slice.
Validation
test/unit/selfhost-verify-backup-script.test.ts(new, 10 cases; fakepg_restore/psql/sqlite3, realgzip): Postgres validation pass/fail, no-dump, the two scratch guards (missing URL / equals live), the scratch happy path, explicit-file mode, and the SQLite integrity/gzip failures.npm run db:migrations:check,npx tsc --noEmit,npm run ui:lint,npm run ui:typecheck— all clean.UI Evidence
Docs-only content addition to an existing page (
docs.self-hosting-backup-scaling.tsx) — no new component, styling, or layout change; uses the existingCodeBlock/Calloutprimitives, and passesui:lint+ui:typecheck. Rendered copy is the new "Verify a backup is restorable" section shown in the diff. Happy to attach a rendered screenshot if you'd like one.Notes
First slice of #2011, kept deliberately narrow.
pg_restore/psqlare already installed in the backup image (postgresql16-client); no image change needed.