diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index 4e17a59fc8..38e1d2649e 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -74,12 +74,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 } diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index 0e96c8ee15..70a81e64af 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -431,34 +431,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) { @@ -475,8 +481,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) } }) }