fix(backup): sanitize failure message + classify reason + meter (SLA observability)

go.mod

@@ -2,7 +2,7 @@ module instant.dev/worker
 
 go 1.25.0
 
-toolchain go1.25.10
+toolchain go1.25.11
 
 require (
 	github.com/DATA-DOG/go-sqlmock v1.5.2

internal/jobs/customer_backup_keepn_test.go

@@ -0,0 +1,149 @@
+package jobs
+
+// customer_backup_keepn_test.go — count-based retention ("keep last N healthy
+// backups per resource", 2026-06-03 operator request). Covers runKeepLastNSweep:
+// every status='ok' backup beyond the newest keepHealthyBackupsPerResource for
+// its resource is retired (S3 object deleted + row soft-flagged via s3_key=NULL).
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	sqlmock "github.com/DATA-DOG/go-sqlmock"
+)
+
+// deleteErrStore wraps the in-memory store but always errors on DeleteObject,
+// exercising the per-victim S3-failure (skip) branch.
+type deleteErrStore struct{ *fakeBackupStore }
+
+func (d deleteErrStore) DeleteObject(_ context.Context, _, _ string) error {
+	return errors.New("s3 unavailable")
+}
+
+func TestRunKeepLastNSweep_RetiresBeyondCap(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+
+	// Window query returns the over-cap victims (rn > keep). Two here.
+	mock.ExpectQuery(`row_number\(\) OVER`).
+		WithArgs(keepHealthyBackupsPerResource).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "s3_key"}).
+			AddRow("b6", "backups/r/b6.dump.gz").
+			AddRow("b7", "backups/r/b7.dump.gz"))
+	mock.ExpectExec(`UPDATE resource_backups`).
+		WithArgs("b6").WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(`UPDATE resource_backups`).
+		WithArgs("b7").WillReturnResult(sqlmock.NewResult(0, 1))
+
+	store := newFakeBackupStore()
+	w := &CustomerBackupRunnerWorker{db: db, store: store, bucket: "instant-shared"}
+	w.runKeepLastNSweep(context.Background())
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sqlmock expectations: %v", err)
+	}
+	if len(store.deletes) != 2 {
+		t.Fatalf("expected 2 S3 objects retired, got %d (%v)", len(store.deletes), store.deletes)
+	}
+}
+
+func TestRunKeepLastNSweep_NoVictimsWithinCap(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+
+	// Resource has <= keep backups → window query returns nothing → no deletes.
+	mock.ExpectQuery(`row_number\(\) OVER`).
+		WithArgs(keepHealthyBackupsPerResource).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "s3_key"}))
+
+	store := newFakeBackupStore()
+	w := &CustomerBackupRunnerWorker{db: db, store: store, bucket: "instant-shared"}
+	w.runKeepLastNSweep(context.Background())
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sqlmock expectations: %v", err)
+	}
+	if len(store.deletes) != 0 {
+		t.Fatalf("expected 0 retired when within cap, got %d", len(store.deletes))
+	}
+}
+
+func TestRunKeepLastNSweep_QueryError_FailsSoft(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+	mock.ExpectQuery(`row_number\(\) OVER`).
+		WithArgs(keepHealthyBackupsPerResource).
+		WillReturnError(errors.New("db blip"))
+	w := &CustomerBackupRunnerWorker{db: db, store: newFakeBackupStore(), bucket: "b"}
+	w.runKeepLastNSweep(context.Background()) // must not panic; fails soft
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+func TestRunKeepLastNSweep_ScanError_SkipsRow(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+	// NULL s3_key cannot scan into a string → scan error → row skipped.
+	mock.ExpectQuery(`row_number\(\) OVER`).
+		WithArgs(keepHealthyBackupsPerResource).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "s3_key"}).AddRow("b6", nil))
+	store := newFakeBackupStore()
+	w := &CustomerBackupRunnerWorker{db: db, store: store, bucket: "b"}
+	w.runKeepLastNSweep(context.Background())
+	if len(store.deletes) != 0 {
+		t.Fatalf("scan-failed row must not be retired, got %d deletes", len(store.deletes))
+	}
+}
+
+func TestRunKeepLastNSweep_DeleteError_SkipsUpdate(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+	mock.ExpectQuery(`row_number\(\) OVER`).
+		WithArgs(keepHealthyBackupsPerResource).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "s3_key"}).AddRow("b6", "backups/r/b6.dump.gz"))
+	// No ExpectExec(UPDATE): an S3 delete failure must skip the row's DB update.
+	w := &CustomerBackupRunnerWorker{db: db, store: deleteErrStore{newFakeBackupStore()}, bucket: "b"}
+	w.runKeepLastNSweep(context.Background())
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sqlmock expectations (UPDATE must NOT run on delete error): %v", err)
+	}
+}
+
+func TestRunKeepLastNSweep_DBUpdateError_FailsSoft(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer db.Close()
+	mock.ExpectQuery(`row_number\(\) OVER`).
+		WithArgs(keepHealthyBackupsPerResource).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "s3_key"}).AddRow("b6", "backups/r/b6.dump.gz"))
+	mock.ExpectExec(`UPDATE resource_backups`).
+		WithArgs("b6").WillReturnError(errors.New("update blip"))
+	store := newFakeBackupStore()
+	w := &CustomerBackupRunnerWorker{db: db, store: store, bucket: "b"}
+	w.runKeepLastNSweep(context.Background()) // soft-fails on the update error
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatalf("unmet sqlmock expectations: %v", err)
+	}
+	if len(store.deletes) != 1 {
+		t.Fatalf("object should still be deleted before the failed DB update, got %d", len(store.deletes))
+	}
+}

internal/jobs/customer_backup_reason_test.go

@@ -0,0 +1,78 @@
+package jobs
+
+// customer_backup_reason_test.go — unit tests for the backup-failure reason
+// classifier and the customer-safe message sanitizer (2026-06-03 backup
+// observability fix). These pin two invariants:
+//   1. A credential/auth failure is classified "auth" (SLA-relevant, paged)
+//      and distinguished from a transient "dump"/"upload" failure.
+//   2. The customer-facing summary NEVER leaks internal detail (host/IP,
+//      per-tenant role name, raw pg_dump stderr) — the incident that
+//      triggered this fix forwarded exactly that to a user.
+
+import (
+	"errors"
+	"strings"
+	"testing"
+)
+
+func TestBackupFailReason(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+		want string
+	}{
+		// The exact prod stderr that triggered this fix.
+		{"prod password auth", errors.New(`pg_dump: error: connection to server at "pg.instanode.dev" (152.42.154.144), port 5432 failed: FATAL: password authentication failed for user "usr_96edf9eed8ed42929036b63298ec5b2b"`), "auth"},
+		{"generic auth failed", errors.New("authentication failed"), "auth"},
+		{"no password supplied", errors.New("pg_dump: error: no password supplied"), "auth"},
+		{"role does not exist", errors.New(`FATAL: role "usr_abc" does not exist`), "auth"},
+		{"permission denied", errors.New("permission denied for table users"), "auth"},
+		{"server unavailable is transient", errors.New("pg_dump: server unavailable"), "dump"},
+		{"connection refused is transient", errors.New("connection refused"), "dump"},
+		{"nil err defaults to dump", nil, "dump"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := backupFailReason(c.err); got != c.want {
+				t.Fatalf("backupFailReason(%q) = %q, want %q", c.err, got, c.want)
+			}
+		})
+	}
+}
+
+func TestSanitizedBackupFailure_NeverLeaksInternals(t *testing.T) {
+	// Tokens from the real leaked error that must NEVER appear in a
+	// customer-facing summary, regardless of reason.
+	leaks := []string{
+		"pg.instanode.dev", "152.42.154.144", "5432",
+		"usr_96edf9eed8ed42929036b63298ec5b2b", "pg_dump", "password", "FATAL",
+	}
+	for _, reason := range []string{"auth", "decrypt", "config", "dump", "upload", "other", ""} {
+		msg := sanitizedBackupFailure(reason)
+		if strings.TrimSpace(msg) == "" {
+			t.Fatalf("sanitizedBackupFailure(%q) is empty", reason)
+		}
+		low := strings.ToLower(msg)
+		for _, leak := range leaks {
+			if strings.Contains(low, strings.ToLower(leak)) {
+				t.Errorf("sanitizedBackupFailure(%q) leaks %q: %s", reason, leak, msg)
+			}
+		}
+	}
+}
+
+func TestSanitizedBackupFailure_PerReasonCopy(t *testing.T) {
+	// auth → reassuring + "no action needed"; transient → "try again".
+	if !strings.Contains(strings.ToLower(sanitizedBackupFailure("auth")), "no action") {
+		t.Error("auth message should reassure the user no action is needed")
+	}
+	for _, r := range []string{"dump", "upload"} {
+		if !strings.Contains(strings.ToLower(sanitizedBackupFailure(r)), "try again") {
+			t.Errorf("%q message should say we'll retry", r)
+		}
+	}
+	// decrypt/config are internal config issues — surfaced as such, no leak.
+	if !strings.Contains(strings.ToLower(sanitizedBackupFailure("config")), "internal configuration") {
+		t.Error("config message should name an internal configuration issue")
+	}
+}