fix: Enforce mandatory webhook secret for GitLab validation

Enforced strict validation to require both the X-Gitlab-Token header
and a configured webhook secret. This prevented unauthenticated
requests that were previously accepted when both values were empty.

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

Author: chmouel

pkg/provider/gitlab/gitlab.go

@@ -156,12 +156,16 @@ func (v *Provider) SetLogger(logger *zap.SugaredLogger) {
 
 func (v *Provider) Validate(_ context.Context, _ *params.Run, event *info.Event) error {
 	token := event.Request.Header.Get("X-Gitlab-Token")
-	if event.Provider.WebhookSecret == "" && token != "" {
-		return fmt.Errorf("gitlab failed validation: failed to find webhook secret")
+	if token == "" {
+		return fmt.Errorf("no X-Gitlab-Token header detected: webhook validation requires a token for security")
+	}
+
+	if event.Provider.WebhookSecret == "" {
+		return fmt.Errorf("no webhook secret configured: set webhook secret in repository CR or secret")
 	}
 
 	if subtle.ConstantTimeCompare([]byte(event.Provider.WebhookSecret), []byte(token)) == 0 {
-		return fmt.Errorf("gitlab failed validation: event's secret doesn't match with webhook secret")
+		return fmt.Errorf("gitlab webhook validation failed: token does not match configured secret")
 	}
 	return nil
 }

pkg/provider/gitlab/gitlab_test.go

@@ -1168,34 +1168,40 @@ func TestGetFileInsideRepo(t *testing.T) {
 func TestValidate(t *testing.T) {
 	tests := []struct {
 		name        string
-		wantErr     bool
+		wantErr     string
 		secretToken string
 		eventToken  string
 	}{
 		{
-			name:        "valid event",
-			wantErr:     false,
+			name:        "valid event with matching tokens",
+			wantErr:     "",
 			secretToken: "test",
 			eventToken:  "test",
 		},
 		{
-			name:        "fail validation, no secret defined",
-			wantErr:     true,
+			name:        "invalid when webhook secret not configured",
+			wantErr:     "no webhook secret configured",
 			secretToken: "",
 			eventToken:  "test",
 		},
 		{
-			name:        "fail validation",
-			wantErr:     true,
+			name:        "invalid when tokens do not match",
+			wantErr:     "token does not match configured secret",
 			secretToken: "secret",
 			eventToken:  "test",
 		},
 		{
-			name:        "fail validation, missing event token",
-			wantErr:     true,
+			name:        "invalid when X-Gitlab-Token header missing",
+			wantErr:     "no X-Gitlab-Token header detected",
 			secretToken: "secret",
 			eventToken:  "",
 		},
+		{
+			name:        "invalid when both token and secret are empty (security fix)",
+			wantErr:     "no X-Gitlab-Token header detected",
+			secretToken: "",
+			eventToken:  "",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1212,8 +1218,12 @@ func TestValidate(t *testing.T) {
 				WebhookSecret: tt.secretToken,
 			}
 
-			if err := v.Validate(context.TODO(), nil, event); (err != nil) != tt.wantErr {
-				t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr)
+			err := v.Validate(context.TODO(), nil, event)
+			if tt.wantErr != "" {
+				assert.Assert(t, err != nil)
+				assert.ErrorContains(t, err, tt.wantErr)
+			} else {
+				assert.NilError(t, err)
 			}
 		})
 	}