fix(webhook): enforce non-empty webhook secret

docs/operator-manual/git-webhook.md

@@ -23,7 +23,10 @@ Only `application/json` content type is supported.
 
 ## Configure the webhook secret in credentials
 
-Add the webhook secret to the repository or shared credentials used to authenticate to the repository. The webhook secret is used to validate the authenticity of webhook payloads from your Git provider.
+Add the webhook secret to the repository or shared credentials used to authenticate to the repository. The webhook secret is used to validate the authenticity of webhook payloads from your Git provider and prevents attackers from forging webhook requests to Burrito.
+
+!!! warning "The webhook secret must be non-empty"
+    Burrito requires a **non-empty** `webhookSecret` to verify payloads. Without it, webhook events are rejected. Make sure it matches the secret set on your Git provider's webhook.
 
 ### Using repository-specific credentials
 

internal/repository/repository.go

@@ -40,18 +40,55 @@ func GetAPIProviderFromRepository(store *credentials.CredentialStore, repo *conf
 }
 
 func GetProviderFromCredentials(RepositoryCredentials credentials.Credential) (types.Provider, error) {
+	var provider types.Provider
 	switch RepositoryCredentials.Provider {
 	case "github":
-		return &github.Github{Config: RepositoryCredentials}, nil
+		provider = &github.Github{Config: RepositoryCredentials}
 	case "gitlab":
-		return &gitlab.Gitlab{Config: RepositoryCredentials}, nil
+		provider = &gitlab.Gitlab{Config: RepositoryCredentials}
 	case "standard":
-		return &standard.Standard{Config: RepositoryCredentials}, nil
+		provider = &standard.Standard{Config: RepositoryCredentials}
 	case "mock":
-		return &mock.Mock{}, nil
+		provider = &mock.Mock{}
 	default:
 		return nil, fmt.Errorf("unknown provider: %s", RepositoryCredentials.Provider)
 	}
+	// Wrap every provider so the webhook-secret invariant is enforced centrally,
+	// in the single code path all providers are constructed through, rather than
+	// relying on each provider (present or future) to remember it. See
+	// webhookSecretEnforcer for details.
+	return &webhookSecretEnforcer{Provider: provider, credential: RepositoryCredentials}, nil
+}
+
+// webhookSecretEnforcer wraps a types.Provider to guarantee that a webhook
+// provider is never handed out when the credential has no webhook secret.
+//
+// This exists for security reasons. An empty webhook secret makes the
+// underlying go-playground/webhooks parser skip signature/token verification
+// entirely (`if len(hook.secret) > 0`), which would let anyone POST forged,
+// unsigned events to /api/webhook and drive real Terraform plans under the
+// operator's own credentials. Because GetProviderFromCredentials is the single
+// choke point every provider is built through, enforcing the check here makes
+// it structurally impossible for any provider — including ones added in the
+// future — to serve webhooks without a configured secret. Providers therefore
+// do not (and must not) need to re-implement this guard themselves.
+type webhookSecretEnforcer struct {
+	types.Provider
+	credential credentials.Credential
+}
+
+// GetWebhookProvider delegates to the wrapped provider first (so providers that
+// do not support webhooks keep returning their own error), then refuses to hand
+// back a working webhook provider unless a non-empty webhook secret is set.
+func (w *webhookSecretEnforcer) GetWebhookProvider() (types.WebhookProvider, error) {
+	inner, err := w.Provider.GetWebhookProvider()
+	if err != nil {
+		return nil, err
+	}
+	if w.credential.WebhookSecret == "" {
+		return nil, fmt.Errorf("webhook secret is required but not configured for this repository: refusing to serve webhooks without signature verification")
+	}
+	return inner, nil
 }
 
 func getStandardGitNoAuth(URL string) types.GitProvider {

internal/repository/repository_test.go

@@ -0,0 +1,59 @@
+package repository_test
+
+import (
+	"testing"
+
+	"github.com/padok-team/burrito/internal/repository"
+	"github.com/padok-team/burrito/internal/repository/credentials"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// allProviderKeys lists every provider key handled by
+// repository.GetProviderFromCredentials. That switch statement is the single,
+// mandatory registration point for providers (see internal/repository/AGENTS.md:
+// "Adding a provider = add a case here"), so keeping this list in sync with it
+// guarantees that every provider reachable in production is exercised by the
+// non-regression test below.
+var allProviderKeys = []string{"github", "gitlab", "standard", "mock"}
+
+// TestGetWebhookProvider_RejectsEmptyWebhookSecret is a security non-regression
+// test: no provider must ever hand back a usable webhook provider when the
+// webhook secret is empty. An empty secret makes go-playground/webhooks skip
+// HMAC/token verification entirely, which would let anyone POST forged,
+// unsigned events to /api/webhook. The invariant is enforced centrally in
+// GetProviderFromCredentials, so this test drives every provider through that
+// factory to prove the enforcement is wired in for all of them.
+func TestGetWebhookProvider_RejectsEmptyWebhookSecret(t *testing.T) {
+	for _, key := range allProviderKeys {
+		t.Run(key, func(t *testing.T) {
+			provider, err := repository.GetProviderFromCredentials(credentials.Credential{
+				Provider:      key,
+				WebhookSecret: "",
+			})
+			assert.NoError(t, err, "provider %q should be constructible", key)
+
+			_, err = provider.GetWebhookProvider()
+			assert.Error(t, err, "provider %q must return an error when the webhook secret is empty", key)
+		})
+	}
+}
+
+// TestGetWebhookProvider_AcceptsNonEmptyWebhookSecret guards the legitimate,
+// correctly-configured path: providers that support webhooks must still return
+// a working webhook provider when a secret is configured.
+func TestGetWebhookProvider_AcceptsNonEmptyWebhookSecret(t *testing.T) {
+	for _, key := range []string{"github", "gitlab"} {
+		t.Run(key, func(t *testing.T) {
+			provider, err := repository.GetProviderFromCredentials(credentials.Credential{
+				Provider:      key,
+				WebhookSecret: "a-real-secret",
+			})
+			assert.NoError(t, err, "provider %q should be constructible", key)
+
+			whProvider, err := provider.GetWebhookProvider()
+			assert.NoError(t, err, "provider %q should accept a non-empty webhook secret", key)
+			assert.NotNil(t, whProvider)
+		})
+	}
+}