fix: correct webhook validation logic, unsafe type assertion, and non-idiomatic loops

Agent-Logs-Url: https://github.com/bzimmer/activity/sessions/d5f3dece-c858-4a4b-99da-c3fe36d74b01

Co-authored-by: bzimmer <12852+bzimmer@users.noreply.github.com>

Author: Copilot

cyclinganalytics/rides.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"math"
 	"mime/multipart"
 	"net/http"
 	"net/url"
@@ -57,8 +56,7 @@ func WithRideOptions(r RideOptions) APIOption {
 func (s *RidesService) Ride(ctx context.Context, rideID int64, opts ...APIOption) (*Ride, error) {
 	uri := fmt.Sprintf("ride/%d", rideID)
 	v := url.Values{}
-	for i := range opts {
-		f := opts[i]
+	for _, f := range opts {
 		if f != nil {
 			if err := f(v); err != nil {
 				return nil, err
@@ -95,9 +93,8 @@ func (s *RidesService) Rides(ctx context.Context, userID UserID, spec activity.P
 	if err != nil {
 		return nil, err
 	}
-	if spec.Total > 0 {
-		n := math.Min(float64(len(res.Rides)), float64(spec.Total))
-		res.Rides = res.Rides[:int(n)]
+	if spec.Total > 0 && len(res.Rides) > spec.Total {
+		res.Rides = res.Rides[:spec.Total]
 	}
 	return res.Rides, nil
 }
@@ -179,10 +176,9 @@ func (s *RidesService) StreamSets() map[string]string {
 
 func validateStreams(streams []string) error {
 	x := streamsets()
-	for i := range streams {
-		_, ok := x[streams[i]]
-		if !ok {
-			return fmt.Errorf("invalid stream '%s'", streams[i])
+	for _, stream := range streams {
+		if _, ok := x[stream]; !ok {
+			return fmt.Errorf("invalid stream '%s'", stream)
 		}
 	}
 	return nil

strava/activity.go

@@ -267,10 +267,9 @@ func (s *ActivityService) StreamSets() map[string]string {
 
 func (s *ActivityService) validateStreams(streams []string) error {
 	x := streamsets()
-	for i := range streams {
-		_, ok := x[streams[i]]
-		if !ok {
-			return fmt.Errorf("invalid stream '%s'", streams[i])
+	for _, stream := range streams {
+		if _, ok := x[stream]; !ok {
+			return fmt.Errorf("invalid stream '%s'", stream)
 		}
 	}
 	return nil

strava/strava_with.go

@@ -98,12 +98,12 @@ func webhookSubscriptionHandler(subscriber WebhookSubscriber) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		q := r.URL.Query()
 		verify, ok := q["hub.verify_token"]
-		if !ok && len(verify) == 1 {
+		if !ok || len(verify) != 1 {
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 		challenge, ok := q["hub.challenge"]
-		if !ok && len(challenge) == 1 {
+		if !ok || len(challenge) != 1 {
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
@@ -112,6 +112,7 @@ func webhookSubscriptionHandler(subscriber WebhookSubscriber) http.HandlerFunc {
 			err := subscriber.SubscriptionRequest(challenge[0], verify[0])
 			if err != nil {
 				w.WriteHeader(http.StatusInternalServerError)
+				return
 			}
 		}
 		w.Header().Set("Content-Type", "application/json")

strava/webhook.go

@@ -232,3 +232,36 @@ func TestWebhookSubscriptionHandler(t *testing.T) {
 
 	a.Equal(500, w.Code)
 }
+
+func TestWebhookSubscriptionHandlerMissingParams(t *testing.T) {
+	t.Parallel()
+	a := assert.New(t)
+
+	tests := []struct {
+		name string
+		url  string
+	}{
+		{
+			name: "missing verify_token",
+			url:  "/webhook?hub.challenge=baz",
+		},
+		{
+			name: "missing challenge",
+			url:  "/webhook?hub.verify_token=bar",
+		},
+		{
+			name: "missing both params",
+			url:  "/webhook",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			_, router := setupTestRouter()
+			w := httptest.NewRecorder()
+			req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, tt.url, nil)
+			router.ServeHTTP(w, req)
+			a.Equal(500, w.Code)
+		})
+	}
+}