feat(ops): ship backups off-host and cover the audio volume (#414) #472

Merged

claude-bot merged 2 commits from feat/414-offhost-backups into main 2026-08-31 22:42:28 +00:00

Conversation 0 Commits 2 Files changed 11 +1066 -20

claude-bot commented 2026-08-31 22:09:49 +00:00

Contributor

Copy link

Closes the code half of #414. See the note at the bottom on criterion 2, which asks for something that does not exist.

The problem

The only backup mechanism in the product wrote a pg_dump to a directory on the same host and disk it was backing up. Losing that host — a failed disk, a deprovisioned server, ransomware — took the database, the recordings, and every backup of the database at the same moment, for every group on the instance.

Off-host shipping, via rclone

A new Off-host remote setting takes an rclone destination; each finished artefact is copied there after the dump.

rclone rather than an S3 SDK because one binary covers S3, B2, SFTP, WebDAV and the rest — a self-hoster with a NAS is served as well as a hosted instance on object storage — docs/OPERATIONS.md already recommended it for the manual version of this job, and rclone keeps its own credentials, so Quest Board never stores another secret it would have to encrypt, rotate and leak.

Three decisions worth stating, because each is a place this could have gone quietly wrong:

A backup that cannot be shipped is recorded as failed, even though pg_dump succeeded. A dump on the disk it protects is the exact state this issue exists to stop reporting as a working backup. The local file is kept regardless.

rclone copy, never rclone sync. sync mirrors, so a run whose local backup directory was empty — a fresh volume, a bad mount, the disk failure this feature is for — would delete the entire off-host set at the moment it became the only copy. Same shape as reconcile_audio_directories' empty-table guard from #406: an empty source is evidence of a problem, never an instruction to delete. The cost is that we never prune the remote; that is the right trade (lifecycle rules and rclone delete --min-age both exist) and it is documented.

The remote is treated as a secret. rclone accepts inline connection strings — :s3,access_key_id=…,secret_access_key=…:bucket — which are easy to paste into an admin field. Unredacted they would land in backup_logs.destination, the admin audit log, and rclone's own stderr on failure. The whole parameter block is replaced rather than known key names matched: a redactor that only knows today's secret names leaks tomorrow's. This also retires the # No secrets in backup config comment in admin.py, which was true right up until this commit.

Audio volume

An opt-in tar of audio_temp ships alongside the dump.

Off by default, unlike scheduled backups themselves. Enabling dumps by default costs megabytes; enabling audio snapshots by default could multiply an existing install's storage on the next upgrade — and a full volume breaks the database backups too, turning a durability feature into the outage it was meant to prevent. It also buys nothing on its own: a tar written to the same host survives no failure the volume itself doesn't. It is worth its cost alongside a remote, which is a deliberate act of configuration either way.

The docs were actively wrong, and that is the most dangerous part of this issue

docs/OPERATIONS.md described the audio volume as:

deliberately excluded. It is transient scratch space … Nothing here needs to survive a restore.

That has not been true for some time:

• process_audio never deletes a session directory — it waits for a GM to approve the transcript, which can be days or weeks
• retain_indefinitely is a supported per-campaign audio retention mode
• trashed audio is restorable through a minimum 7-day grace (#427)
• a session whose transcription failed has no transcript at all, so its audio is the only record it happened (#402)

An operator following that advice would lose every recording on the instance during a restore. Rewritten to describe what the volume actually holds, with a restore procedure that deliberately extracts over the volume rather than clearing it first — a recording that arrived after the snapshot is real data nothing else has a copy of, and wiping to make the restore "clean" would destroy exactly the sessions most likely to matter.

On criterion 2

Off-host backups are enabled by default for the hosted deployment profile.

There is no deployment-profile concept in the codebase — no DEPLOYMENT_PROFILE, no hosted/self-hosted switch. Rather than invent one as a side effect of a backup change, off-host shipping activates when a remote is configured. Inventing that abstraction is a product decision worth making deliberately, not incidentally.

Also worth noting: parts of #414 had already landed and the issue body does not reflect it. Scheduled backups are already enabled by default (get_backup_config's docstring cites #414), the backups bind mount already cites #414, and #429 has since added a dump/restore version check. The issue's line references have drifted too (3179–3200 → 3942–4079).

Verification

25 new tests, every one run against unfixed code first and confirmed to fail. 13 mutations, all caught:

• copy → sync (the remote-deleting one)
• inline connection string unredacted in the stored destination
• rclone's stderr unredacted on failure
• shipping failure swallowed and recorded as success
• rclone timeout swallowed
• silently skipping when rclone is absent
• include_audio defaulting to on
• the audio snapshot never shipped
• both remote-validation checks removed
• pre-existing config keys not read back

Two mutations initially reported as "caught" were actually collection errors — my mutation hadn't compiled — so I redid them with AST-validated replacements rather than accept a misleading green.

Also: full backend suite 1539 passed; frontend 465 passed, lint 0 errors, build succeeds, audit gate clean; ruff check + ruff format --check clean; backend-prod image builds and rclone version runs inside it.

Not covered here

Criterion 4's "periodically exercised restore" is an ops practice, not code. The runbook it needs already exists in docs/OPERATIONS.md (and already says "A dump you have never restored is an untested backup"), now extended to audio.

🤖 Generated with Claude Code

Closes the code half of #414. See the note at the bottom on criterion 2, which asks for something that does not exist. ## The problem The only backup mechanism in the product wrote a `pg_dump` to a directory on the **same host and disk** it was backing up. Losing that host — a failed disk, a deprovisioned server, ransomware — took the database, the recordings, and every backup of the database at the same moment, for every group on the instance. ## Off-host shipping, via rclone A new **Off-host remote** setting takes an rclone destination; each finished artefact is copied there after the dump. rclone rather than an S3 SDK because one binary covers S3, B2, SFTP, WebDAV and the rest — a self-hoster with a NAS is served as well as a hosted instance on object storage — `docs/OPERATIONS.md` already recommended it for the manual version of this job, and rclone keeps its own credentials, so Quest Board never stores another secret it would have to encrypt, rotate and leak. Three decisions worth stating, because each is a place this could have gone quietly wrong: **A backup that cannot be shipped is recorded as `failed`,** even though `pg_dump` succeeded. A dump on the disk it protects is the exact state this issue exists to stop reporting as a working backup. The local file is kept regardless. **`rclone copy`, never `rclone sync`.** `sync` mirrors, so a run whose local backup directory was empty — a fresh volume, a bad mount, the disk failure this feature is *for* — would delete the entire off-host set at the moment it became the only copy. Same shape as `reconcile_audio_directories`' empty-table guard from #406: an empty source is evidence of a problem, never an instruction to delete. The cost is that we never prune the remote; that is the right trade (lifecycle rules and `rclone delete --min-age` both exist) and it is documented. **The remote is treated as a secret.** rclone accepts inline connection strings — `:s3,access_key_id=…,secret_access_key=…:bucket` — which are easy to paste into an admin field. Unredacted they would land in `backup_logs.destination`, the admin audit log, *and* rclone's own stderr on failure. The whole parameter block is replaced rather than known key names matched: a redactor that only knows today's secret names leaks tomorrow's. This also retires the `# No secrets in backup config` comment in `admin.py`, which was true right up until this commit. ## Audio volume An opt-in tar of `audio_temp` ships alongside the dump. **Off by default, unlike scheduled backups themselves.** Enabling dumps by default costs megabytes; enabling audio snapshots by default could multiply an existing install's storage on the next upgrade — and a full volume breaks the database backups too, turning a durability feature into the outage it was meant to prevent. It also buys nothing on its own: a tar written to the same host survives no failure the volume itself doesn't. It is worth its cost alongside a remote, which is a deliberate act of configuration either way. ## The docs were actively wrong, and that is the most dangerous part of this issue `docs/OPERATIONS.md` described the audio volume as: > deliberately **excluded**. It is transient scratch space … Nothing here needs to survive a restore. That has not been true for some time: - `process_audio` **never** deletes a session directory — it waits for a GM to approve the transcript, which can be days or weeks - `retain_indefinitely` is a supported per-campaign audio retention mode - trashed audio is restorable through a minimum 7-day grace (#427) - a session whose transcription **failed** has no transcript at all, so its audio is the only record it happened (#402) An operator following that advice would lose every recording on the instance during a restore. Rewritten to describe what the volume actually holds, with a restore procedure that deliberately extracts *over* the volume rather than clearing it first — a recording that arrived after the snapshot is real data nothing else has a copy of, and wiping to make the restore "clean" would destroy exactly the sessions most likely to matter. ## On criterion 2 > Off-host backups are enabled by default for the hosted deployment profile. There is **no deployment-profile concept** in the codebase — no `DEPLOYMENT_PROFILE`, no hosted/self-hosted switch. Rather than invent one as a side effect of a backup change, off-host shipping activates when a remote is configured. Inventing that abstraction is a product decision worth making deliberately, not incidentally. Also worth noting: parts of #414 had already landed and the issue body does not reflect it. Scheduled backups are already enabled by default (`get_backup_config`'s docstring cites #414), the `backups` bind mount already cites #414, and #429 has since added a dump/restore version check. The issue's line references have drifted too (3179–3200 → 3942–4079). ## Verification **25 new tests, every one run against unfixed code first and confirmed to fail.** 13 mutations, all caught: - `copy` → `sync` (the remote-deleting one) - inline connection string unredacted in the stored destination - rclone's stderr unredacted on failure - shipping failure swallowed and recorded as success - rclone timeout swallowed - silently skipping when rclone is absent - `include_audio` defaulting to on - the audio snapshot never shipped - both remote-validation checks removed - pre-existing config keys not read back Two mutations initially reported as "caught" were actually *collection errors* — my mutation hadn't compiled — so I redid them with AST-validated replacements rather than accept a misleading green. Also: full backend suite **1539 passed**; frontend **465 passed**, lint 0 errors, build succeeds, audit gate clean; `ruff check` + `ruff format --check` clean; `backend-prod` image builds and `rclone version` runs inside it. ## Not covered here Criterion 4's "periodically **exercised** restore" is an ops practice, not code. The runbook it needs already exists in `docs/OPERATIONS.md` (and already says "A dump you have never restored is an untested backup"), now extended to audio. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-08-31 22:09:49 +00:00

feat(ops): ship backups off-host and cover the audio volume (#414) ... 262e211144

The only backup mechanism in the product wrote a pg_dump to a directory on the
same host and disk it was backing up. Losing that host took the database, the
recordings, and every backup of the database at the same moment, for every
group on the instance.

Off-host shipping, via rclone
-----------------------------
A new "Off-host remote" setting takes an rclone destination; each finished
artefact is copied there after the dump. rclone rather than an S3 SDK because
one binary covers S3, B2, SFTP, WebDAV and the rest — a self-hoster with a NAS
is served as well as a hosted instance on object storage — docs/OPERATIONS.md
already recommended it for the manual version of this job, and rclone keeps its
own credentials, so Quest Board never stores another secret it would have to
encrypt, rotate and leak.

Three decisions worth stating, because each is a place this could have gone
quietly wrong:

- **A backup that cannot be shipped is recorded as failed**, even though
  pg_dump succeeded. A dump on the disk it protects is the exact state #414
  exists to stop reporting as a working backup. The local file is kept.
- **rclone copy, never rclone sync.** sync mirrors, so a run whose local backup
  directory was empty — a fresh volume, a bad mount, the disk failure this
  feature is for — would delete the entire off-host set at the moment it became
  the only copy. Same shape as reconcile_audio_directories' empty-table guard:
  an empty source is evidence of a problem, never an instruction to delete. The
  cost is that we never prune the remote, which is the right trade and is
  documented.
- **The remote is treated as a secret.** rclone accepts inline connection
  strings (":s3,access_key_id=...,secret_access_key=...:bucket") which are easy
  to paste into an admin field; unredacted they would land in
  backup_logs.destination, the admin audit log, and rclone's own stderr on
  failure. The whole parameter block is replaced rather than known key names
  matched — a redactor that only knows today's secrets leaks tomorrow's. This
  also retires the "No secrets in backup config" comment in admin.py, which was
  true until this commit.

Audio volume
------------
An opt-in tar of audio_temp ships alongside the dump. Off by default, unlike
scheduled backups themselves: enabling dumps by default costs megabytes, while
enabling audio snapshots by default could multiply an existing install's
storage on upgrade — and a full volume breaks the database backups too, turning
a durability feature into the outage it was meant to prevent. It also buys
nothing without a remote, since a tar written to the same host survives no
failure the volume doesn't.

The docs were actively wrong here
---------------------------------
docs/OPERATIONS.md described audio_temp as "transient scratch space ... nothing
here needs to survive a restore". That has not been true for some time:
process_audio never deletes a session directory (it waits for GM approval),
retain_indefinitely is a supported per-campaign mode, trashed audio is
restorable through a 7-day minimum grace, and a session whose transcription
FAILED has its audio as the only record it happened. An operator following that
advice lost every recording on the instance during a restore. Rewritten, with a
restore procedure that deliberately extracts over the volume rather than
clearing it first — a recording that arrived after the snapshot is real data
nothing else has a copy of.

Criterion 2 of the issue asks for this to be "enabled by default for the hosted
deployment profile". No deployment-profile concept exists in the codebase, so
off-host shipping activates when a remote is configured instead. Noted on the
issue rather than inventing the profile.

Verification: 25 new tests, each run against unfixed code first and confirmed to
fail — 13 mutations, all caught, including copy→sync, an unredacted connection
string in both the stored destination and rclone's stderr, a swallowed shipping
failure recorded as success, a swallowed timeout, silently skipping when rclone
is absent, and include_audio defaulting on. Full backend suite 1539 passed;
frontend 465 passed and builds; ruff clean; backend-prod image builds and rclone
runs in it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rbrooks added 1 commit 2026-08-31 22:21:29 +00:00

fix(security): stop a backup remote being read as rclone flags (#414) ... 7d1fd5c12f

Flagged by automated security review on the previous commit. Verified rather
than taken on trust, and the honest answer is that it was not exploitable —
but the reason it was not is a property of rclone's parser, not of our input
handling, and that is the wrong thing to be relying on.

`validate_remote`'s character class `[A-Za-z0-9_-]` included a dash, so
`--dry-run:`, `--config:x`, `--dump:headers` and `-P:x` all passed validation
and were handed to `subprocess.run` as argv. Probing the rclone in our own
backend image, every one comes back as "unknown flag": the check also demands a
"," or ":" straight after the name, and pflag wants "=" or a separate argv
element for a flag's value, so no accepted string is a usable flag today.

That argument would have to be re-derived on every rclone upgrade, so:

- `validate_remote` now rejects a leading "-" outright, and the name pattern
  no longer admits one either. The two overlap deliberately — the pattern is
  what still holds if the explicit check is ever removed as redundant.
- `ship_to_remote` terminates flag parsing with "--" before the operands, so
  neither the local path nor the operator's remote can be read as a flag
  however it is spelled. This is the layer that does not depend on rclone's
  grammar. Confirmed against the image's binary: after "--", a destination of
  "--dry-run:" is treated as a remote ("didn't find section in config file")
  rather than parsed as a flag.

Only an admin can set this field, so the exposure was never anonymous. But
"admin of the app" and "can pass arbitrary flags to a subprocess" are different
privileges, and --config or --dump would have bridged them.

The tests index the source path off the "--" separator rather than a fixed
position, so they cannot quietly stop asserting anything if the layout changes
again.

Mutation-checked: 4 of 5 caught. Removing the leading-dash check is caught by
its specific error message (asserting only "ValueError" could not tell the two
overlapping guards apart, and would have stayed green while one was removed);
removing both together, dropping "--", and moving "--" after the operands where
it does nothing are all caught. The remaining mutation — reverting only the
name pattern — is genuinely unobservable while the explicit check runs first,
which is what redundancy means; noted in the code rather than papered over with
a contrived test.

Backend suite 1547 passed; ruff clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot merged commit 0a9055bd2c into main 2026-08-31 22:42:28 +00:00

claude-bot deleted branch feat/414-offhost-backups 2026-08-31 22:42:29 +00:00

claude-bot referenced this pull request from a commit 2026-08-31 22:42:30 +00:00

Merge PR #472: ship backups off-host and cover the audio volume (#414)

claude-bot referenced this pull request 2026-08-31 22:43:03 +00:00

[Ops] Move backups off-host and cover the audio volume #414

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!472

No description provided.