fix(backup): sanitize failure message + classify reason + meter (SLA observability) - #79
Merged
mastermanas805 merged 3 commits intoJun 3, 2026
Conversation
Incident: a customer received a "Backup failed" notification containing raw
pg_dump stderr — the internal host/IP (pg.instanode.dev / 152.42.154.144:5432)
and the per-tenant role name (usr_…). The message was also non-actionable: the
backup credential is platform-managed, so there's nothing the user can do. And
because markFailed emitted no metric, a clean failure slipped past both
existing backup alerts (backup-stale fires only after 36h; no-followup only on
STUCK rows) — a paid-tier durability/SLA breach with no prompt page.
Root cause of *that* failure: crypto.Decrypt fails CLOSED, and the error was
"password authentication failed" (not a decrypt error) — so the credential
decrypted fine but the live DB role rejected it (stored connection_url drifted
from the role's password). That per-resource drift needs an ops fix; this
change makes the *class* of failure safe, classified, and observable:
- markFailed now persists a SANITIZED, customer-safe summary to
resource_backups.error_summary + the backup.failed audit (what the email and
backup-health surface read). Raw stderr stays in the worker log only.
- Classify failures by reason (auth|decrypt|config|dump|upload) via
backupFailReason(); auth = credential drift (won't self-heal, SLA-relevant,
paged) vs transient dump/upload (retried next run).
- New metrics instant_customer_backup_failed_total{reason} +
instant_customer_backup_succeeded_total for an alert + success-ratio tile
(rule 25 — NR alert + Prom rule + dashboard tile land in the infra PR).
Retry semantics already correct: Work() returns nil after markFailed, so auth
failures aren't hammered.
Tests: table tests for backupFailReason (incl. the exact prod stderr) and an
anti-leak guard asserting the sanitized summary never contains the host/IP/
role/pg_dump/password tokens. make gate green; new funcs 100% covered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
enabled auto-merge (squash)
June 3, 2026 05:14
Operator request (2026-06-03): cap retained successful backups at 5 per resource. Adds runKeepLastNSweep — retires every status='ok' backup that is NOT among the keepHealthyBackupsPerResource (=5) most-recent for its resource, even if still within the tier's time-based retention window. Reuses runRetentionSweep's retire mechanism (delete S3 object, soft-flag the row via s3_key=NULL; error_summary='retained:count-cap' distinguishes it from the time-based 'retained:expired'). Runs at the end of each backup tick alongside the existing retention sweep; fail-soft per victim so one S3/DB blip never blocks the rest. Tests: retire-beyond-cap, within-cap no-op, query-error, scan-error, delete-error (skips DB update), db-update-error (fails soft). New method 100% covered. make gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
govulncheck flagged GO-2026-5039 (net/textproto arbitrary input in errors) and GO-2026-5038 (mime WordDecoder quadratic complexity), both fixed in go1.25.11. The version-file-based CI jobs (govulncheck/coverage/codeql/lint) read the go.mod toolchain, so they were pinning the vulnerable 1.25.10. Bumping the toolchain patches the stdlib and unblocks this PR (and all worker PRs). Pre-existing — not introduced by the backup changes in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incident: a customer's 'Backup failed' notification leaked raw pg_dump stderr — internal host/IP (
152.42.154.144:5432) + per-tenant role (usr_…) — and was non-actionable (platform-managed credential). markFailed emitted no metric, so the failure slipped past both existing backup alerts (stale=36h, no-followup=stuck-only).Root cause of the instance:
crypto.Decryptfails closed; the error waspassword authentication failed, so decrypt succeeded and the live DB role rejected the stored credential (connection_url drifted from the role password). Per-resource drift = ops fix; this PR makes the class safe/observable.Changes: sanitized customer-safe
error_summary(raw stderr → worker log only); reason classification (auth|decrypt|config|dump|upload);instant_customer_backup_failed_total{reason}+_succeeded_total. NR alert + Prom rule + dashboard tile follow in the infra PR (rule 25). Table tests incl. the exact prod stderr + an anti-leak guard. make gate green; new funcs 100% covered.🤖 Generated with Claude Code