From 7cc896424dd1feb606fd361acba767614d4f71dd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 12 Feb 2026 01:34:50 +0000 Subject: [PATCH] refactor(33-data-model-integrity): harden shutdown lifecycle in pkg/audit --- pkg/audit/sqlite_logger.go | 26 ++++++++++++++++---------- pkg/audit/sqlite_logger_test.go | 20 ++++++++++++++++++++ pkg/audit/webhook.go | 9 ++++++--- pkg/audit/webhook_delivery_test.go | 8 ++++++++ 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/pkg/audit/sqlite_logger.go b/pkg/audit/sqlite_logger.go index 85e44fb00..a5a2532ae 100644 --- a/pkg/audit/sqlite_logger.go +++ b/pkg/audit/sqlite_logger.go @@ -31,6 +31,8 @@ type SQLiteLogger struct { retentionDays int stopChan chan struct{} wg sync.WaitGroup + closeOnce sync.Once + closeErr error } // NewSQLiteLogger creates a new SQLite-backed audit logger. @@ -387,20 +389,24 @@ func (l *SQLiteLogger) VerifySignature(event Event) bool { // Close gracefully shuts down the logger. func (l *SQLiteLogger) Close() error { - close(l.stopChan) + l.closeOnce.Do(func() { + close(l.stopChan) - if l.webhookDelivery != nil { - l.webhookDelivery.Stop() - } + if l.webhookDelivery != nil { + l.webhookDelivery.Stop() + } - l.wg.Wait() + l.wg.Wait() - if err := l.db.Close(); err != nil { - return fmt.Errorf("failed to close audit database: %w", err) - } + if err := l.db.Close(); err != nil { + l.closeErr = fmt.Errorf("failed to close audit database: %w", err) + return + } - log.Info().Msg("SQLite audit logger closed") - return nil + log.Info().Msg("SQLite audit logger closed") + }) + + return l.closeErr } // loadWebhookURLs loads webhook URLs from the config table. diff --git a/pkg/audit/sqlite_logger_test.go b/pkg/audit/sqlite_logger_test.go index 02ce93707..81584a0b1 100644 --- a/pkg/audit/sqlite_logger_test.go +++ b/pkg/audit/sqlite_logger_test.go @@ -502,3 +502,23 @@ func TestSQLiteLoggerConcurrentAccess(t *testing.T) { t.Errorf("Expected 100 events, got %d", count) } } + +func TestSQLiteLoggerCloseIdempotent(t *testing.T) { + tempDir := t.TempDir() + + logger, err := NewSQLiteLogger(SQLiteLoggerConfig{ + DataDir: tempDir, + CryptoMgr: newMockCryptoManager(), + RetentionDays: 30, + }) + if err != nil { + t.Fatalf("NewSQLiteLogger failed: %v", err) + } + + if err := logger.Close(); err != nil { + t.Fatalf("first Close failed: %v", err) + } + if err := logger.Close(); err != nil { + t.Fatalf("second Close failed: %v", err) + } +} diff --git a/pkg/audit/webhook.go b/pkg/audit/webhook.go index 35b254029..6a513e6eb 100644 --- a/pkg/audit/webhook.go +++ b/pkg/audit/webhook.go @@ -32,6 +32,7 @@ type WebhookDelivery struct { client *http.Client queue chan Event stopChan chan struct{} + stopOnce sync.Once wg sync.WaitGroup } @@ -72,9 +73,11 @@ func (w *WebhookDelivery) Start() { // Stop gracefully stops the delivery workers. func (w *WebhookDelivery) Stop() { - close(w.stopChan) - w.wg.Wait() - log.Debug().Msg("Audit webhook delivery stopped") + w.stopOnce.Do(func() { + close(w.stopChan) + w.wg.Wait() + log.Debug().Msg("Audit webhook delivery stopped") + }) } // Enqueue adds an event to the delivery queue. diff --git a/pkg/audit/webhook_delivery_test.go b/pkg/audit/webhook_delivery_test.go index 470c876d4..c88d4afa5 100644 --- a/pkg/audit/webhook_delivery_test.go +++ b/pkg/audit/webhook_delivery_test.go @@ -158,3 +158,11 @@ func TestWebhookDeliveryDeliverInvalidURL(t *testing.T) { t.Fatalf("expected URL blocked error, got %v", err) } } + +func TestWebhookDeliveryStopIdempotent(t *testing.T) { + delivery := NewWebhookDelivery([]string{}) + delivery.Start() + + delivery.Stop() + delivery.Stop() +}