Merge latest origin/main into feat/read-receipts

Preserve read-receipt partial updates while incorporating webhook hardening and repository-wide Go vet fixes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: bf00a0ac-e11f-4015-b295-3cdd9b491229

Author: AchoArnold

.github/workflows/api.yml

@@ -74,6 +74,12 @@ jobs:
           docker compose wait seed || true
           sleep 2
 
+      - name: Run Handler Integration Tests
+        working-directory: ./api
+        env:
+          USER_API_KEY: test-user-api-key
+        run: go test -tags integration -v -timeout 60s ./pkg/handlers
+
       - name: Run Integration Tests
         working-directory: ./tests
         run: go test -v -timeout 300s ./...

api/pkg/cache/memory_cache.go

@@ -2,7 +2,6 @@ package cache
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	"github.com/NdoleStudio/httpsms/pkg/telemetry"
@@ -31,7 +30,7 @@ func (cache *memoryCache) Get(ctx context.Context, key string) (value string, er
 
 	response, ok := cache.store.Get(key)
 	if !ok {
-		return "", stacktrace.NewError(fmt.Sprintf("no item found in cache with key [%s]", key))
+		return "", stacktrace.NewError("no item found in cache with key [%s]", key)
 	}
 
 	return response.(string), nil

api/pkg/cache/redis_cache.go

@@ -3,7 +3,6 @@ package cache
 import (
 	"context"
 	"errors"
-	"fmt"
 	"time"
 
 	"github.com/NdoleStudio/httpsms/pkg/telemetry"
@@ -32,10 +31,10 @@ func (cache *redisCache) Get(ctx context.Context, key string) (value string, err
 
 	response, err := cache.client.Get(ctx, key).Result()
 	if errors.Is(err, redis.Nil) {
-		return "", stacktrace.Propagate(err, fmt.Sprintf("no item found in redis with key [%s]", key))
+		return "", stacktrace.Propagate(err, "no item found in redis with key [%s]", key)
 	}
 	if err != nil {
-		return "", stacktrace.Propagate(err, fmt.Sprintf("cannot get item in redis with key [%s]", key))
+		return "", stacktrace.Propagate(err, "cannot get item in redis with key [%s]", key)
 	}
 	return response, nil
 }

api/pkg/di/container.go

@@ -279,11 +279,11 @@ func (container *Container) DedicatedDB() (db *gorm.DB) {
 
 	container.logger.Debug(fmt.Sprintf("Running migrations for dedicated [%T]", db))
 	if err = db.AutoMigrate(&entities.Heartbeat{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.Heartbeat{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.Heartbeat{}))
 	}
 
 	if err = db.AutoMigrate(&entities.HeartbeatMonitor{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.HeartbeatMonitor{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.HeartbeatMonitor{}))
 	}
 
 	return container.dedicatedDB
@@ -371,47 +371,47 @@ ALTER TABLE discords ADD CONSTRAINT IF NOT EXISTS uni_discords_server_id CHECK (
 	}
 
 	if err = db.AutoMigrate(&entities.Message{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.Message{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.Message{}))
 	}
 
 	if err = db.AutoMigrate(&entities.MessageThread{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.MessageThread{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.MessageThread{}))
 	}
 
 	if err = db.AutoMigrate(&entities.User{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.User{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.User{}))
 	}
 
 	if err = db.AutoMigrate(&entities.MessageSendSchedule{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.MessageSendSchedule{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.MessageSendSchedule{}))
 	}
 
 	if err = db.AutoMigrate(&entities.Phone{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.Phone{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.Phone{}))
 	}
 
 	if err = db.AutoMigrate(&entities.PhoneNotification{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.PhoneNotification{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.PhoneNotification{}))
 	}
 
 	if err = db.AutoMigrate(&entities.BillingUsage{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.BillingUsage{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.BillingUsage{}))
 	}
 
 	if err = db.AutoMigrate(&entities.Webhook{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.Webhook{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.Webhook{}))
 	}
 
 	if err = db.AutoMigrate(&entities.Discord{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.Discord{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.Discord{}))
 	}
 
 	if err = db.AutoMigrate(&entities.Integration3CX{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.Integration3CX{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.Integration3CX{}))
 	}
 
 	if err = db.AutoMigrate(&entities.PhoneAPIKey{}); err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot migrate %T", &entities.PhoneAPIKey{})))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot migrate %T", &entities.PhoneAPIKey{}))
 	}
 
 	return container.db
@@ -423,8 +423,7 @@ func (container *Container) FirebaseApp() (app *firebase.App) {
 
 	app, err := firebase.NewApp(context.Background(), nil, option.WithCredentialsJSON(container.FirebaseCredentials()))
 	if err != nil {
-		msg := "cannot initialize firebase application"
-		container.logger.Fatal(stacktrace.Propagate(err, msg))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot initialize firebase application"))
 	}
 	return app
 }
@@ -445,7 +444,7 @@ func (container *Container) Cache() cache.Cache {
 	container.logger.Debug("creating cache.Cache")
 	opt, err := redis.ParseURL(os.Getenv("REDIS_URL"))
 	if err != nil {
-		container.logger.Fatal(stacktrace.Propagate(err, fmt.Sprintf("cannot parse redis url [%s]", os.Getenv("REDIS_URL"))))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot parse redis url [%s]", os.Getenv("REDIS_URL")))
 	}
 	if strings.HasPrefix(os.Getenv("REDIS_URL"), "rediss://") {
 		opt.TLSConfig = &tls.Config{
@@ -473,8 +472,7 @@ func (container *Container) FirebaseAuthClient() (client *auth.Client) {
 	container.logger.Debug(fmt.Sprintf("creating %T", client))
 	authClient, err := container.FirebaseApp().Auth(context.Background())
 	if err != nil {
-		msg := "cannot initialize firebase auth client"
-		container.logger.Fatal(stacktrace.Propagate(err, msg))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot initialize firebase auth client"))
 	}
 	return authClient
 }
@@ -553,8 +551,7 @@ func (container *Container) FCMClient() services.FCMClient {
 	container.logger.Debug("creating FirebaseFCMClient")
 	messagingClient, err := container.FirebaseApp().Messaging(context.Background())
 	if err != nil {
-		msg := "cannot initialize firebase messaging client"
-		container.logger.Fatal(stacktrace.Propagate(err, msg))
+		container.logger.Fatal(stacktrace.Propagate(err, "cannot initialize firebase messaging client"))
 	}
 	return services.NewFirebaseFCMClient(messagingClient)
 }

api/pkg/emails/event_payload_formatter.go

@@ -23,7 +23,7 @@ func formatEventPayload(payload string) (string, template.HTML) {
 		content = highlightEventPayloadJSON(formattedPayload)
 	}
 
-	// Every payload token is escaped before this trusted wrapper is constructed.
+	// #nosec G203 -- every dynamic payload token is escaped before the static wrapper is added.
 	richPayload := template.HTML(`<pre style="` + eventPayloadCodeBlockStyle + `">` + content + `</pre>`)
 	return formattedPayload, richPayload
 }

api/pkg/emails/hermes_notification_email_factory.go

@@ -123,12 +123,14 @@ func (factory *hermesNotificationEmailFactory) WebhookSendFailed(user *entities.
 		return nil, stacktrace.Propagate(err, "cannot generate html email")
 	}
 
-	text, err := factory.generator.GeneratePlainText(email)
+	eventPayloadEntryIndex := len(email.Body.Dictionary) - 1
+	text, err := generateWebhookSendFailedPlainText(func(value string) (string, error) {
+		email.Body.Dictionary[eventPayloadEntryIndex].Value = value
+		return factory.generator.GeneratePlainText(email)
+	}, formattedPayload)
 	if err != nil {
 		return nil, stacktrace.Propagate(err, "cannot generate text email")
 	}
-	// Hermes/html2text collapses dictionary whitespace, so restore the payload after plain-text generation.
-	text = replaceWebhookSendFailedEventPayloadPlaceholder(text, formattedPayload)
 
 	return &Email{
 		ToEmail: user.Email,
@@ -138,6 +140,21 @@ func (factory *hermesNotificationEmailFactory) WebhookSendFailed(user *entities.
 	}, nil
 }
 
+func generateWebhookSendFailedPlainText(generate func(string) (string, error), formattedPayload string) (string, error) {
+	text, err := generate(webhookSendFailedEventPayloadPlaceholder)
+	if err != nil {
+		return "", err
+	}
+
+	if strings.Contains(text, webhookSendFailedEventPayloadPlaceholder) {
+		// Hermes/html2text collapses dictionary whitespace, so restore the payload after plain-text generation.
+		return replaceWebhookSendFailedEventPayloadPlaceholder(text, formattedPayload), nil
+	}
+
+	// Preserve the real payload if a Hermes change stops carrying the placeholder through unchanged.
+	return generate(formattedPayload)
+}
+
 func replaceWebhookSendFailedEventPayloadPlaceholder(text string, formattedPayload string) string {
 	before, after, found := strings.Cut(text, webhookSendFailedEventPayloadPlaceholder)
 	if !found {

api/pkg/emails/hermes_notification_email_factory_test.go

@@ -50,6 +50,7 @@ func TestWebhookSendFailedFormatsOnlyEventPayload(t *testing.T) {
 	assert.Contains(t, email.Text, `"retry": false`)
 	assert.NotContains(t, email.Text, "<pre")
 	assert.NotContains(t, email.Text, webhookSendFailedEventPayloadPlaceholder)
+	assert.NotContains(t, email.HTML, webhookSendFailedEventPayloadPlaceholder)
 }
 
 func TestWebhookSendFailedPreservesNonJSONEventPayload(t *testing.T) {
@@ -76,6 +77,24 @@ func TestWebhookSendFailedPreservesNonJSONEventPayload(t *testing.T) {
 	assert.Contains(t, email.Text, "line one\n  line two")
 }
 
+func TestGenerateWebhookSendFailedPlainTextFallsBackToFormattedPayload(t *testing.T) {
+	formattedPayload := "{\n  \"message\": \"hello\"\n}"
+	values := make([]string, 0, 2)
+	generate := func(value string) (string, error) {
+		values = append(values, value)
+		if len(values) == 1 {
+			return "plain text without the placeholder", nil
+		}
+		return value, nil
+	}
+
+	text, err := generateWebhookSendFailedPlainText(generate, formattedPayload)
+
+	require.NoError(t, err)
+	assert.Equal(t, formattedPayload, text)
+	assert.Equal(t, []string{webhookSendFailedEventPayloadPlaceholder, formattedPayload}, values)
+}
+
 func TestReplaceWebhookSendFailedEventPayloadPlaceholder(t *testing.T) {
 	tests := []struct {
 		name             string

api/pkg/handlers/attachment_handler.go

@@ -66,8 +66,7 @@ func (h *AttachmentHandler) GetAttachment(c fiber.Ctx) error {
 
 	data, err := h.storage.Download(ctx, path)
 	if err != nil {
-		msg := fmt.Sprintf("cannot download attachment from path [%s]", path)
-		ctxLogger.Warn(stacktrace.Propagate(err, msg))
+		ctxLogger.Warn(stacktrace.Propagate(err, "cannot download attachment from path [%s]", path))
 		if stacktrace.GetCode(err) == repositories.ErrCodeNotFound {
 			return h.responseNotFound(c, "attachment not found")
 		}

api/pkg/handlers/billing_handler.go

@@ -65,21 +65,18 @@ func (h *BillingHandler) UsageHistory(c fiber.Ctx) error {
 
 	var request requests.BillingUsageHistory
 	if err := c.Bind().Query(&request); err != nil {
-		msg := fmt.Sprintf("cannot marshall params [%s] into %T", c.Body(), request)
-		ctxLogger.Warn(stacktrace.Propagate(err, msg))
+		ctxLogger.Warn(stacktrace.Propagate(err, "cannot marshall params [%s] into %T", c.Body(), request))
 		return h.responseBadRequest(c, err)
 	}
 
 	if errors := h.validator.ValidateHistory(ctx, request.Sanitize()); len(errors) != 0 {
-		msg := fmt.Sprintf("validation errors [%s], while fetching heartbeats [%+#v]", spew.Sdump(errors), request)
-		ctxLogger.Warn(stacktrace.NewError(msg))
+		ctxLogger.Warn(stacktrace.NewError("validation errors [%s], while fetching heartbeats [%+#v]", spew.Sdump(errors), request))
 		return h.responseUnprocessableEntity(c, errors, "validation errors while fetching usage history")
 	}
 
 	heartbeats, err := h.service.GetUsageHistory(ctx, h.userIDFomContext(c), request.ToIndexParams())
 	if err != nil {
-		msg := fmt.Sprintf("cannot get billing usage history with params [%+#v]", request)
-		ctxLogger.Error(stacktrace.Propagate(err, msg))
+		ctxLogger.Error(stacktrace.Propagate(err, "cannot get billing usage history with params [%+#v]", request))
 		return h.responseInternalServerError(c)
 	}
 
@@ -107,8 +104,7 @@ func (h *BillingHandler) Usage(c fiber.Ctx) error {
 
 	billingUsage, err := h.service.GetCurrentUsage(ctx, h.userIDFomContext(c))
 	if err != nil {
-		msg := fmt.Sprintf("cannot get current usage record for user [%s]", h.userFromContext(c))
-		ctxLogger.Error(stacktrace.Propagate(err, msg))
+		ctxLogger.Error(stacktrace.Propagate(err, "cannot get current usage record for user [%s]", h.userFromContext(c)))
 		return h.responseInternalServerError(c)
 	}
 

api/pkg/handlers/bulk_message_handler.go

@@ -67,8 +67,7 @@ func (h *BulkMessageHandler) Index(c fiber.Ctx) error {
 
 	orders, err := h.messageService.GetBulkMessages(ctx, h.userIDFomContext(c))
 	if err != nil {
-		msg := fmt.Sprintf("cannot fetch bulk messages for user [%s]", h.userIDFomContext(c))
-		ctxLogger.Error(stacktrace.Propagate(err, msg))
+		ctxLogger.Error(stacktrace.Propagate(err, "cannot fetch bulk messages for user [%s]", h.userIDFomContext(c)))
 		return h.responseInternalServerError(c)
 	}
 
@@ -95,20 +94,18 @@ func (h *BulkMessageHandler) Store(c fiber.Ctx) error {
 
 	file, err := c.FormFile("document")
 	if err != nil {
-		msg := fmt.Sprintf("cannot fetch file with name [%s] from request", "document")
-		ctxLogger.Warn(stacktrace.Propagate(err, msg))
+		ctxLogger.Warn(stacktrace.Propagate(err, "cannot fetch file with name [%s] from request", "document"))
 		return h.responseBadRequest(c, err)
 	}
 
 	messages, userLocation, validationErrors := h.validator.ValidateStore(ctx, h.userIDFomContext(c), file)
 	if len(validationErrors) != 0 {
-		msg := fmt.Sprintf("validation errors [%s], while sending bulk sms from CSV file [%s] for [%s]", spew.Sdump(validationErrors), file.Filename, h.userIDFomContext(c))
-		ctxLogger.Warn(stacktrace.NewError(msg))
+		ctxLogger.Warn(stacktrace.NewError("validation errors [%s], while sending bulk sms from CSV file [%s] for [%s]", spew.Sdump(validationErrors), file.Filename, h.userIDFomContext(c)))
 		return h.responseUnprocessableEntity(c, validationErrors, "validation errors while sending bulk SMS")
 	}
 
 	if msg := h.billingService.IsEntitledWithCount(ctx, h.userIDFomContext(c), uint(len(messages))); msg != nil {
-		ctxLogger.Warn(stacktrace.NewError(fmt.Sprintf("user with ID [%s] is not entitled to send [%d] messages", h.userIDFomContext(c), len(messages))))
+		ctxLogger.Warn(stacktrace.NewError("user with ID [%s] is not entitled to send [%d] messages", h.userIDFomContext(c), len(messages)))
 		return h.responsePaymentRequired(c, *msg)
 	}
 
@@ -135,8 +132,8 @@ func (h *BulkMessageHandler) Store(c fiber.Ctx) error {
 			)
 			if err != nil {
 				count.Add(-1)
-				msg := fmt.Sprintf("cannot send message with payload [%s] at index [%d]", spew.Sdump(message), index)
-				ctxLogger.Error(stacktrace.Propagate(err, msg))
+
+				ctxLogger.Error(stacktrace.Propagate(err, "cannot send message with payload [%s] at index [%d]", spew.Sdump(message), index))
 			}
 			wg.Done()
 		}(message, perPhoneIndex)