fix data race caused by webpush-go mutating msg []byte

johnstcn · johnstcn · commit 204ab4a2aefc · 2025-03-26T16:36:59.000Z
diff --git a/coderd/notifications/push/push.go b/coderd/notifications/push/push.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"io"
 	"net/http"
+	"slices"
 	"sync"
 
 	"github.com/SherClockHolmes/webpush-go"
@@ -93,8 +94,8 @@ func (n *Notifier) Dispatch(ctx context.Context, userID uuid.UUID, notification
 		subscription := subscription
 		eg.Go(func() error {
 			n.log.Debug(ctx, "dispatching via push", slog.F("subscription", subscription.Endpoint))
-
-			resp, err := webpush.SendNotificationWithContext(ctx, notificationJSON, &webpush.Subscription{
+			cpy := slices.Clone(notificationJSON) // Need to copy as webpush.SendNotificationWithContext modifies the slice.
+			resp, err := webpush.SendNotificationWithContext(ctx, cpy, &webpush.Subscription{
 				Endpoint: subscription.Endpoint,
 				Keys: webpush.Keys{
 					Auth:   subscription.EndpointAuthKey,
diff --git a/coderd/notifications/push/push_test.go b/coderd/notifications/push/push_test.go
@@ -179,7 +179,7 @@ func TestPush(t *testing.T) {
 			},
 		}
 
-		err = manager.Dispatch(context.Background(), user.ID, notification)
+		err = manager.Dispatch(ctx, user.ID, notification)
 		require.NoError(t, err)
 		assert.True(t, okEndpointCalled, "The valid endpoint should be called")
 		assert.True(t, goneEndpointCalled, "The expired endpoint should be called")

Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@ func TestPush(t *testing.T) {`
`179`	`179`	`},`
`180`	`180`	`}`
`181`	`181`
`182`		`- err = manager.Dispatch(context.Background(), user.ID, notification)`
	`182`	`+ err = manager.Dispatch(ctx, user.ID, notification)`
`183`	`183`	`require.NoError(t, err)`
`184`	`184`	`assert.True(t, okEndpointCalled, "The valid endpoint should be called")`
`185`	`185`	`assert.True(t, goneEndpointCalled, "The expired endpoint should be called")`