From 186daabe220d584bcf1480df768e1f85d19e3260 Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 20 Aug 2026 04:17:50 +0000 Subject: [PATCH] Stop target credentials leaking into event databases (closes #206) A Delivery carries its Event and Target structs in memory for the delivery engine, so GORM's automatic association save upserted the whole target row -- config included, which holds destination URLs and bearer credentials -- into the per-webhook event database with an empty webhook_id. Event databases are the files most likely to be backed up or handed to someone else, so they shipped the credentials with them. Register a create and update callback on every per-webhook connection that omits associations, rather than fixing the one call site: it covers writes inside a transaction and write paths added later. Sweep any rows already written, before the migration on each open, so it is idempotent and a no-op on a database with no targets table. Encryption of target config at rest in webhooker.db is deliberately not part of this: it is tracked separately. --- README.md | 30 +- internal/database/event_db_isolation.go | 159 +++++++ internal/database/event_db_isolation_test.go | 438 +++++++++++++++++++ internal/database/webhook_db_manager.go | 19 + internal/delivery/event_db_isolation_test.go | 157 +++++++ 5 files changed, 795 insertions(+), 8 deletions(-) create mode 100644 internal/database/event_db_isolation.go create mode 100644 internal/database/event_db_isolation_test.go create mode 100644 internal/delivery/event_db_isolation_test.go diff --git a/README.md b/README.md index d4e02f0..d2d291e 100644 --- a/README.md +++ b/README.md @@ -506,14 +506,28 @@ backups at rest and restrict who can read them. - `events-{uuid}.db` and `archive-{uuid}.db` hold the **full payload body and headers** of every event as received, including whatever the sending service put in them — tokens, signatures, personal data. -- Until - [issue #206](https://git.eeqj.de/sneak/webhooker/issues/206) is fixed, - the event databases **also contain target credentials**: a GORM - association upsert on the delivery and retry write path copies - `targets` rows, `config` included, into the per-webhook database. For - a Slack target the `webhookUrl` *is* the bearer credential, and an - `http` target's URL can embed userinfo. Handing someone an - `events-*.db` today hands them live delivery destinations. +- Event databases written before + [issue #206](https://git.eeqj.de/sneak/webhooker/issues/206) was fixed + **also contain target credentials**: a GORM association upsert on the + delivery and retry write path copied `targets` rows, `config` + included, into the per-webhook database. For a Slack target the + `webhookUrl` *is* the bearer credential, and an `http` target's URL + can embed userinfo. This version never writes those rows; the first + time it opens such a file it deletes them and vacuums the file, which + removes the credential bytes rather than only unlinking the rows. + Deleting alone would not: the bytes stay readable in the file's free + pages until it is rewritten. The sweep is recorded in the file's + `user_version` only once the vacuum returns, so a sweep that fails or + is interrupted fails the open and is retried on the next one, and a + file this version has opened without error holds no leaked rows and + no recoverable bytes from them. On upgrade this rewrites each + existing `events-{uuid}.db` once, on its first open. Two cases still + hand over live delivery destinations: a backup taken from an older + build, and a backup of a file this version has not yet opened + successfully. Copies already made stay affected — the sweep only + rewrites the file it opens, and freed blocks may persist in + filesystem snapshots and on the underlying storage. Rotate any target + credential that was in a backup you cannot account for. - `webhooker.db` stores target config **unencrypted**, tracked at [issue #212](https://git.eeqj.de/sneak/webhooker/issues/212), next to the session encryption key and the Argon2id password hashes. diff --git a/internal/database/event_db_isolation.go b/internal/database/event_db_isolation.go new file mode 100644 index 0000000..bd1abc7 --- /dev/null +++ b/internal/database/event_db_isolation.go @@ -0,0 +1,159 @@ +package database + +import ( + "fmt" + "log/slog" + + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +// omitAssociationsCallback is the name the association guard is +// registered under on a per-webhook database's create and update +// callback chains. +const omitAssociationsCallback = "webhooker:omit_associations" + +// omitAssociations makes every create and update issued against a +// per-webhook database skip GORM's automatic association save. +// +// A per-webhook database holds the event tier only, but Delivery +// declares belongs-to Event and Target and the delivery engine fills +// both in memory before writing. Without this guard GORM upserts +// those parent rows here on the delivery and retry write paths, +// copying targets.config, which holds destination URLs and bearer +// credentials, into the file most likely to be backed up or handed +// to someone else. Registering the guard on the connection covers +// every write path, including writes inside a transaction and write +// paths added later. Every event-tier row this file holds is written +// explicitly, so nothing depends on the automatic save. +func omitAssociations(db *gorm.DB) error { + omit := func(tx *gorm.DB) { + tx.Statement.Omits = append( + tx.Statement.Omits, clause.Associations, + ) + } + + err := db.Callback().Create(). + Before("gorm:save_before_associations"). + Register(omitAssociationsCallback, omit) + if err != nil { + return fmt.Errorf( + "registering create association guard: %w", err, + ) + } + + err = db.Callback().Update(). + Before("gorm:save_before_associations"). + Register(omitAssociationsCallback, omit) + if err != nil { + return fmt.Errorf( + "registering update association guard: %w", err, + ) + } + + return nil +} + +// eventDBSweptVersion is the PRAGMA user_version purgeTargetRows +// stamps into a per-webhook database once it has removed any leaked +// target rows *and* the VACUUM that removes their bytes has returned. +// Nothing else in the tree uses user_version, so 0 means "not swept +// by this build". +// +// The stamp, not the DELETE, is what records that a file is done. A +// DELETE commits on its own, so a sweep that is interrupted or whose +// VACUUM fails leaves a file whose rows are gone but whose credential +// bytes are still in the free pages -- indistinguishable, by row +// count, from a file that never leaked. Both leave the stamp unset, +// so the next open sweeps again. +const eventDBSweptVersion = 1 + +// purgeTargetRows deletes target rows that an earlier build's +// association upsert wrote into a per-webhook database, and rewrites +// the file so their bytes are gone with them. AutoMigrate creates a +// targets table in every one of these files because Delivery declares +// a belongs-to Target, but nothing in the event tier may put rows in +// it. The rows it did put there are junk, not history: they carry an +// empty webhook_id, and delivery rows resolve their target against +// the main database, so nothing here refers to them. +// +// The DELETE only unlinks the rows: modernc.org/sqlite leaves +// secure_delete at SQLite's default of off, so the credential bytes +// stay readable in the file's free pages and a backup of a swept file +// would still hand them over. VACUUM rewrites the file without them. +// +// This runs before every migration and is gated on +// eventDBSweptVersion, so a file pays for the rewrite once, on the +// first open that finds it unstamped, and every open after that is a +// PRAGMA read. A file this build created is stamped before its +// targets table exists, so it never vacuums at all. A failure here +// fails the open with the stamp left unset, so the sweep is retried +// rather than skipped -- a webhook whose file cannot be swept stays +// unusable instead of quietly serving from a file that still holds +// recoverable credentials. +func purgeTargetRows( + db *gorm.DB, log *slog.Logger, webhookID string, +) error { + var version int + + // Row().Scan, not (*gorm.DB).Scan: see internal/gormlog. + err := db.Raw("PRAGMA user_version").Row().Scan(&version) + if err != nil { + return fmt.Errorf( + "reading sweep marker of webhook database %s: %w", + webhookID, err, + ) + } + + if version >= eventDBSweptVersion { + return nil + } + + var purged int64 + + if db.Migrator().HasTable("targets") { + res := db.Exec("DELETE FROM targets") + if res.Error != nil { + return fmt.Errorf( + "purging target rows from webhook database %s: %w", + webhookID, res.Error, + ) + } + + purged = res.RowsAffected + + // Unconditional: a zero row count here does not mean there is + // nothing to remove, only that no *live* row is left. See + // eventDBSweptVersion. + err = db.Exec("VACUUM").Error + if err != nil { + return fmt.Errorf( + "purged %d leaked target rows from webhook database "+ + "%s but vacuuming it failed, so the deleted "+ + "target credentials are still recoverable from "+ + "the file; it stays marked unswept and the next "+ + "open retries: %w", + purged, webhookID, err, + ) + } + } + + err = db.Exec(fmt.Sprintf( + "PRAGMA user_version = %d", eventDBSweptVersion, + )).Error + if err != nil { + return fmt.Errorf( + "marking webhook database %s swept: %w", webhookID, err, + ) + } + + if purged > 0 { + log.Warn( + "purged leaked target rows from per-webhook database", + "webhook_id", webhookID, + "rows", purged, + ) + } + + return nil +} diff --git a/internal/database/event_db_isolation_test.go b/internal/database/event_db_isolation_test.go new file mode 100644 index 0000000..66c28e0 --- /dev/null +++ b/internal/database/event_db_isolation_test.go @@ -0,0 +1,438 @@ +package database_test + +import ( + "bytes" + "database/sql" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + _ "modernc.org/sqlite" + "sneak.berlin/go/webhooker/internal/database" +) + +// testDataDirPerm is the mode the test data directory is created +// with. +const testDataDirPerm = 0o750 + +// eventDBDataDir returns a data directory that a WebhookDBManager +// can be pointed at. +func eventDBDataDir(t *testing.T) string { + t.Helper() + + dir := filepath.Join(t.TempDir(), "events") + require.NoError(t, os.MkdirAll(dir, testDataDirPerm)) + + return dir +} + +// openRawEventDB opens the per-webhook database file directly, +// without the manager, so a test can put a file on disk in a state +// the manager has to cope with, or inspect one afterwards. +func openRawEventDB( + t *testing.T, dataDir, webhookID string, +) *sql.DB { + t.Helper() + + path := filepath.Join( + dataDir, fmt.Sprintf("events-%s.db", webhookID), + ) + + sqlDB, err := sql.Open( + "sqlite", + fmt.Sprintf("file:%s?mode=rwc", path), + ) + require.NoError(t, err) + + t.Cleanup(func() { _ = sqlDB.Close() }) + + return sqlDB +} + +// eventDBFileBytes reads a per-webhook database file off disk, so a +// test can assert on what the file itself still holds rather than on +// what a query returns. +func eventDBFileBytes(t *testing.T, dataDir, webhookID string) []byte { + t.Helper() + + //nolint:gosec // reads a file the test just created under t.TempDir() + raw, err := os.ReadFile(filepath.Join( + dataDir, fmt.Sprintf("events-%s.db", webhookID), + )) + require.NoError(t, err) + + return raw +} + +// eventDBUserVersion returns the PRAGMA user_version of a per-webhook +// database file, which is the marker purgeTargetRows stamps once it +// has swept and vacuumed. +func eventDBUserVersion(t *testing.T, sqlDB *sql.DB) int { + t.Helper() + + var version int + + require.NoError(t, sqlDB.QueryRowContext( + t.Context(), "PRAGMA user_version", + ).Scan(&version)) + + return version +} + +// clearEventDBSweptMarker resets the sweep marker to 0, which is what +// a file written by a build without the sweep looks like. Tests that +// seed a leaked row have to create the file through the manager to +// get the real targets table shape, and that stamps it. +func clearEventDBSweptMarker(t *testing.T, sqlDB *sql.DB) { + t.Helper() + + _, err := sqlDB.ExecContext(t.Context(), "PRAGMA user_version = 0") + require.NoError(t, err) +} + +// countTargetRows returns the number of rows in the targets table of +// a per-webhook database file, or -1 if the table does not exist. +func countTargetRows(t *testing.T, sqlDB *sql.DB) int { + t.Helper() + + var tables int + + require.NoError(t, sqlDB.QueryRowContext( + t.Context(), + "SELECT count(*) FROM sqlite_master "+ + "WHERE type = 'table' AND name = 'targets'", + ).Scan(&tables)) + + if tables == 0 { + return -1 + } + + var rows int + + require.NoError(t, sqlDB.QueryRowContext( + t.Context(), "SELECT count(*) FROM targets", + ).Scan(&rows)) + + return rows +} + +// TestOpenPurgesLeakedTargetRows covers the sweep for event +// databases written by a build that let GORM upsert target rows +// into them: opening the database clears them, and opening it again +// is a no-op. +func TestOpenPurgesLeakedTargetRows(t *testing.T) { + t.Parallel() + + dataDir := eventDBDataDir(t) + webhookID := uuid.New().String() + + // Create the file the way the application does, so the targets + // table has exactly the shape AutoMigrate gives it, then write + // a leaked row into it the way the association upsert did. + initial := database.NewTestWebhookDBManager(dataDir) + + _, err := initial.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, initial.CloseAll()) + + seed := openRawEventDB(t, dataDir, webhookID) + + _, err = seed.ExecContext( + t.Context(), + "INSERT INTO targets "+ + "(id, webhook_id, name, type, config) "+ + "VALUES (?, '', ?, ?, ?)", + uuid.New().String(), + "leaked-target", + "slack", + `{"webhookUrl":"https://hooks.example/T000/B000/secret"}`, + ) + require.NoError(t, err) + require.Equal(t, 1, countTargetRows(t, seed)) + clearEventDBSweptMarker(t, seed) + require.NoError(t, seed.Close()) + + mgr := database.NewTestWebhookDBManager(dataDir) + + _, err = mgr.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, mgr.CloseAll()) + + check := openRawEventDB(t, dataDir, webhookID) + assert.Zero(t, countTargetRows(t, check)) + assert.Equal( + t, 1, eventDBUserVersion(t, check), + "a completed sweep must mark the file so later opens skip it", + ) + require.NoError(t, check.Close()) + + // Idempotent: a second open leaves it at zero and does not + // error. + again := database.NewTestWebhookDBManager(dataDir) + + _, err = again.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, again.CloseAll()) + + recheck := openRawEventDB(t, dataDir, webhookID) + assert.Zero(t, countTargetRows(t, recheck)) +} + +// TestOpenPurgeRemovesCredentialBytes covers the sweep at the level +// that matters for a backup handed to someone else: the leaked +// credential must be gone from the raw bytes of the file, not merely +// unreachable by query. A bare DELETE unlinks the row and leaves the +// bytes readable in the free pages, so this fails without the VACUUM +// in purgeTargetRows. +func TestOpenPurgeRemovesCredentialBytes(t *testing.T) { + t.Parallel() + + dataDir := eventDBDataDir(t) + webhookID := uuid.New().String() + credential := "T00000000/B00000000/" + uuid.New().String() + + initial := database.NewTestWebhookDBManager(dataDir) + + _, err := initial.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, initial.CloseAll()) + + seed := openRawEventDB(t, dataDir, webhookID) + + _, err = seed.ExecContext( + t.Context(), + "INSERT INTO targets "+ + "(id, webhook_id, name, type, config) "+ + "VALUES (?, '', ?, ?, ?)", + uuid.New().String(), + "leaked-target", + "slack", + fmt.Sprintf( + `{"webhookUrl":"https://hooks.example/%s"}`, credential, + ), + ) + require.NoError(t, err) + clearEventDBSweptMarker(t, seed) + require.NoError(t, seed.Close()) + + // The seed has to be in the file for its absence later to mean + // anything. + require.True( + t, + bytes.Contains( + eventDBFileBytes(t, dataDir, webhookID), + []byte(credential), + ), + "seeded credential is not in the file, so this test proves nothing", + ) + + mgr := database.NewTestWebhookDBManager(dataDir) + + _, err = mgr.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, mgr.CloseAll()) + + assert.NotContains( + t, + string(eventDBFileBytes(t, dataDir, webhookID)), + credential, + "leaked credential is still recoverable from the raw file", + ) +} + +// TestOpenRevacuumsAfterIncompleteSweep covers the case a row count +// cannot see: the rows are already deleted but the file was never +// vacuumed, because an earlier sweep died between the two or its +// VACUUM failed. The credential bytes are still recoverable, and the +// unset marker is the only thing that says so, so the next open must +// vacuum rather than conclude from the empty table that there is +// nothing to do. +func TestOpenRevacuumsAfterIncompleteSweep(t *testing.T) { + t.Parallel() + + dataDir := eventDBDataDir(t) + webhookID := uuid.New().String() + credential := "T00000000/B00000000/" + uuid.New().String() + + initial := database.NewTestWebhookDBManager(dataDir) + + _, err := initial.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, initial.CloseAll()) + + seed := openRawEventDB(t, dataDir, webhookID) + + _, err = seed.ExecContext( + t.Context(), + "INSERT INTO targets "+ + "(id, webhook_id, name, type, config) "+ + "VALUES (?, '', ?, ?, ?)", + uuid.New().String(), + "leaked-target", + "slack", + fmt.Sprintf( + `{"webhookUrl":"https://hooks.example/%s"}`, credential, + ), + ) + require.NoError(t, err) + + // Exactly the state an interrupted sweep leaves: rows gone, + // marker unset, bytes still in the free pages. + _, err = seed.ExecContext(t.Context(), "DELETE FROM targets") + require.NoError(t, err) + require.Zero(t, countTargetRows(t, seed)) + clearEventDBSweptMarker(t, seed) + require.NoError(t, seed.Close()) + + require.True( + t, + bytes.Contains( + eventDBFileBytes(t, dataDir, webhookID), + []byte(credential), + ), + "the deleted row's bytes must still be in the file, or this "+ + "test proves nothing", + ) + + mgr := database.NewTestWebhookDBManager(dataDir) + + _, err = mgr.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, mgr.CloseAll()) + + assert.NotContains( + t, + string(eventDBFileBytes(t, dataDir, webhookID)), + credential, + "an interrupted sweep was not retried, so the credential is "+ + "still recoverable from the raw file", + ) + + check := openRawEventDB(t, dataDir, webhookID) + assert.Equal(t, 1, eventDBUserVersion(t, check)) +} + +// TestOpenSkipsSweptDatabase covers the other half of the marker: a +// file this build created is marked without ever being vacuumed, and +// a marked file is not swept again. +func TestOpenSkipsSweptDatabase(t *testing.T) { + t.Parallel() + + dataDir := eventDBDataDir(t) + webhookID := uuid.New().String() + + mgr := database.NewTestWebhookDBManager(dataDir) + + _, err := mgr.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, mgr.CloseAll()) + + marked := openRawEventDB(t, dataDir, webhookID) + assert.Equal(t, 1, eventDBUserVersion(t, marked)) + + // A marked file is left alone, so a row written into it survives + // a reopen. Nothing writes target rows any more; this stands in + // for the sweep having run. + _, err = marked.ExecContext( + t.Context(), + "INSERT INTO targets "+ + "(id, webhook_id, name, type, config) "+ + "VALUES (?, '', ?, ?, ?)", + uuid.New().String(), "sentinel", "slack", `{}`, + ) + require.NoError(t, err) + require.NoError(t, marked.Close()) + + again := database.NewTestWebhookDBManager(dataDir) + + _, err = again.GetDB(webhookID) + require.NoError(t, err) + require.NoError(t, again.CloseAll()) + + check := openRawEventDB(t, dataDir, webhookID) + assert.Equal( + t, 1, countTargetRows(t, check), + "a marked file must not be swept again", + ) +} + +// TestOpenSucceedsWithoutTargetsTable covers an existing event +// database that never grew a targets table. The sweep must not fail +// startup on it. +func TestOpenSucceedsWithoutTargetsTable(t *testing.T) { + t.Parallel() + + dataDir := eventDBDataDir(t) + webhookID := uuid.New().String() + + seed := openRawEventDB(t, dataDir, webhookID) + + _, err := seed.ExecContext( + t.Context(), + "CREATE TABLE events (id text PRIMARY KEY)", + ) + require.NoError(t, err) + require.NoError(t, seed.Close()) + + mgr := database.NewTestWebhookDBManager(dataDir) + + db, err := mgr.GetDB(webhookID) + require.NoError(t, err) + assert.NotNil(t, db) + require.NoError(t, mgr.CloseAll()) +} + +// TestEventDBCreateOmitsAssociations covers the connection-level +// guard directly: a Delivery carrying its Event and Target in +// memory, written through the manager's handle, must store only the +// delivery row. +func TestEventDBCreateOmitsAssociations(t *testing.T) { + t.Parallel() + + dataDir := eventDBDataDir(t) + webhookID := uuid.New().String() + + mgr := database.NewTestWebhookDBManager(dataDir) + + db, err := mgr.GetDB(webhookID) + require.NoError(t, err) + + target := database.Target{ + WebhookID: webhookID, + Name: "leaky-target", + Type: database.TargetTypeSlack, + Config: `{"webhookUrl":"https://hooks.example/secret"}`, + } + target.ID = uuid.New().String() + + event := database.Event{ + WebhookID: webhookID, + EntrypointID: uuid.New().String(), + Method: "POST", + Headers: `{}`, + Body: `{}`, + } + event.ID = uuid.New().String() + + d := &database.Delivery{ + EventID: event.ID, + TargetID: target.ID, + Status: database.DeliveryStatusPending, + Event: event, + Target: target, + } + d.ID = uuid.New().String() + + require.NoError(t, db.Create(d).Error) + require.NoError(t, db.Model(d). + Update("status", database.DeliveryStatusDelivered). + Error) + require.NoError(t, mgr.CloseAll()) + + check := openRawEventDB(t, dataDir, webhookID) + assert.Zero(t, countTargetRows(t, check)) +} diff --git a/internal/database/webhook_db_manager.go b/internal/database/webhook_db_manager.go index 0e89be1..a888ad5 100644 --- a/internal/database/webhook_db_manager.go +++ b/internal/database/webhook_db_manager.go @@ -262,6 +262,25 @@ func (m *WebhookDBManager) openDB( ) } + // Keep main-database rows out of this file. See + // event_db_isolation.go. + err = omitAssociations(db) + if err != nil { + _ = sqlDB.Close() + + return nil, fmt.Errorf( + "guarding webhook database %s: %w", + webhookID, err, + ) + } + + err = purgeTargetRows(db, m.log, webhookID) + if err != nil { + _ = sqlDB.Close() + + return nil, err + } + // Run migrations for event-tier models only err = db.AutoMigrate( &Event{}, &Delivery{}, &DeliveryResult{}, diff --git a/internal/delivery/event_db_isolation_test.go b/internal/delivery/event_db_isolation_test.go new file mode 100644 index 0000000..b865d5e --- /dev/null +++ b/internal/delivery/event_db_isolation_test.go @@ -0,0 +1,157 @@ +package delivery_test + +import ( + "context" + "database/sql" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + _ "modernc.org/sqlite" + "sneak.berlin/go/webhooker/internal/database" +) + +// assertNoTargetRows opens the per-webhook database file directly, +// outside GORM, and fails if its targets table holds any rows. +// Target config is the credential for slack and http targets, and +// event databases are the files that get backed up and handed +// around. +func assertNoTargetRows(t *testing.T, dbPath string) { + t.Helper() + + sqlDB, err := sql.Open( + "sqlite", fmt.Sprintf("file:%s?mode=ro", dbPath), + ) + require.NoError(t, err) + + defer func() { _ = sqlDB.Close() }() + + var tables int + + require.NoError(t, sqlDB.QueryRowContext( + t.Context(), + "SELECT count(*) FROM sqlite_master "+ + "WHERE type = 'table' AND name = 'targets'", + ).Scan(&tables)) + + if tables == 0 { + return + } + + var rows int + + require.NoError(t, sqlDB.QueryRowContext( + t.Context(), "SELECT count(*) FROM targets", + ).Scan(&rows)) + + assert.Zero( + t, rows, + "per-webhook event database must hold no target rows", + ) +} + +// TestEventDBHoldsNoTargetRows drives a delivery and then a retry +// through the real engine write paths and asserts neither leaves a +// target row behind in events-*.db. +func TestEventDBHoldsNoTargetRows(t *testing.T) { + t.Parallel() + + s := newISetup(t) + + ts := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }, + )) + defer ts.Close() + + cfg := iHTTPConfig(ts.URL) + targetID := uuid.New().String() + dbPath := s.DBMgr.DBPath(s.WebhookID) + + event := iSeedEvent( + t, s.WebhookDB, s.WebhookID, `{"leak":"none"}`, + ) + body := event.Body + + // A new delivery. + d := iSeedDelivery( + t, s.WebhookDB, event.ID, targetID, + database.DeliveryStatusPending, + ) + task := iTask( + d, event, s.WebhookID, targetID, + "leaky-target", cfg, 5, 1, &body, + ) + + s.Engine.ExportProcessNewTask(context.TODO(), &task) + + iAssertStatus( + t, s.WebhookDB, d.ID, + database.DeliveryStatusDelivered, + ) + assertNoTargetRows(t, dbPath) + + // A retry. + rd := iSeedDelivery( + t, s.WebhookDB, event.ID, targetID, + database.DeliveryStatusRetrying, + ) + rTask := iTask( + rd, event, s.WebhookID, targetID, + "leaky-target", cfg, 5, 2, &body, + ) + + s.Engine.ExportProcessRetryTask(context.TODO(), &rTask) + + iAssertStatus( + t, s.WebhookDB, rd.ID, + database.DeliveryStatusDelivered, + ) + assertNoTargetRows(t, dbPath) +} + +// TestEventDBHoldsNoTargetRowsOnFailedDelivery covers the failure +// write path, which updates the delivery to failed and records a +// result, rather than the success path above. +func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) { + t.Parallel() + + s := newISetup(t) + + ts := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }, + )) + defer ts.Close() + + cfg := iHTTPConfig(ts.URL) + targetID := uuid.New().String() + + event := iSeedEvent( + t, s.WebhookDB, s.WebhookID, `{"leak":"none"}`, + ) + body := event.Body + + d := iSeedDelivery( + t, s.WebhookDB, event.ID, targetID, + database.DeliveryStatusPending, + ) + task := iTask( + d, event, s.WebhookID, targetID, + "leaky-target", cfg, 0, 1, &body, + ) + + s.Engine.ExportProcessNewTask(context.TODO(), &task) + + iAssertStatus( + t, s.WebhookDB, d.ID, + database.DeliveryStatusFailed, + ) + assertNoTargetRows(t, s.DBMgr.DBPath(s.WebhookID)) +} -- 2.49.1