fix(webhook): validate signature templates at startup; build formatters once

internal/config/validation_test.go

@@ -418,3 +418,4 @@ func TestOpenTelemetry(t *testing.T) {
 		})
 	}
 }
+

internal/destinationmockserver/model.go

@@ -36,12 +36,28 @@ type mockStore struct {
 	mu           sync.RWMutex
 	destinations map[string]models.Destination
 	events       map[string][]Event
+
+	// Built once at construction — shared by every verifySignature call.
+	sigFormatter    destwebhook.SignatureFormatter
+	headerFormatter destwebhook.HeaderFormatter
 }
 
 func NewMockStore() MockStore {
+	// The default templates are package constants, so construction cannot
+	// fail at runtime — a panic here means the constants themselves are broken.
+	sigFormatter, err := destwebhook.NewSignatureFormatter(destwebhook.DefaultSignatureContentTmpl)
+	if err != nil {
+		panic(fmt.Sprintf("destinationmockserver: invalid default signature content template: %v", err))
+	}
+	headerFormatter, err := destwebhook.NewHeaderFormatter(destwebhook.DefaultSignatureHeaderTmpl)
+	if err != nil {
+		panic(fmt.Sprintf("destinationmockserver: invalid default signature header template: %v", err))
+	}
 	return &mockStore{
-		destinations: make(map[string]models.Destination),
-		events:       make(map[string][]Event),
+		destinations:    make(map[string]models.Destination),
+		events:          make(map[string][]Event),
+		sigFormatter:    sigFormatter,
+		headerFormatter: headerFormatter,
 	}
 }
 
@@ -120,7 +136,7 @@ func (s *mockStore) ReceiveEvent(ctx context.Context, destinationID string, rawB
 	if signature := metadata["signature"]; signature != "" {
 		// Try current secret
 		if secret := destination.Credentials["secret"]; secret != "" {
-			event.Verified = verifySignature(
+			event.Verified = s.verifySignature(
 				secret,
 				rawBody,
 				signature,
@@ -136,7 +152,7 @@ func (s *mockStore) ReceiveEvent(ctx context.Context, destinationID string, rawB
 				if invalidAtStr := destination.Credentials["previous_secret_invalid_at"]; invalidAtStr != "" {
 					if invalidAt, err := time.Parse(time.RFC3339, invalidAtStr); err == nil {
 						if time.Now().Before(invalidAt) {
-							event.Verified = verifySignature(
+							event.Verified = s.verifySignature(
 								prevSecret,
 								rawBody,
 								signature,
@@ -155,7 +171,7 @@ func (s *mockStore) ReceiveEvent(ctx context.Context, destinationID string, rawB
 }
 
 // verifySignature verifies the signature using the provided secret and algorithm
-func verifySignature(secret string, payload []byte, signature string, algorithm string, encoding string) bool {
+func (s *mockStore) verifySignature(secret string, payload []byte, signature string, algorithm string, encoding string) bool {
 	log.Println("verifySignature", secret, payload, signature, algorithm, encoding)
 	if signature == "" {
 		return false
@@ -190,8 +206,8 @@ func verifySignature(secret string, payload []byte, signature string, algorithm
 		secrets,
 		destwebhook.WithEncoder(destwebhook.GetEncoder(encoding)),
 		destwebhook.WithAlgorithm(destwebhook.GetAlgorithm(algorithm)),
-		destwebhook.WithSignatureFormatter(destwebhook.NewSignatureFormatter(destwebhook.DefaultSignatureContentTmpl)),
-		destwebhook.WithHeaderFormatter(destwebhook.NewHeaderFormatter(destwebhook.DefaultSignatureHeaderTmpl)),
+		destwebhook.WithSignatureFormatter(s.sigFormatter),
+		destwebhook.WithHeaderFormatter(s.headerFormatter),
 	)
 
 	for _, sig := range signatures {

internal/destregistry/providers/default_test.go

@@ -0,0 +1,46 @@
+package destregistrydefault_test
+
+import (
+	"testing"
+
+	"github.com/hookdeck/outpost/internal/destregistry"
+	destregistrydefault "github.com/hookdeck/outpost/internal/destregistry/providers"
+	"github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook"
+	"github.com/hookdeck/outpost/internal/util/testutil"
+	"github.com/stretchr/testify/assert"
+)
+
+// Signature template validation lives in destwebhook.New, so registration is
+// where a bad template fails startup — and only in default mode. In 'standard'
+// mode the templates are fixed by the Standard Webhooks spec, the configured
+// ones are never parsed, and registration must succeed regardless of their
+// contents.
+func TestRegisterDefault_WebhookSignatureTemplates(t *testing.T) {
+	webhookConfig := func(mode string) *destregistrydefault.DestWebhookConfig {
+		return &destregistrydefault.DestWebhookConfig{
+			Mode:                     mode,
+			HeaderPrefix:             destwebhook.DefaultHeaderPrefix,
+			SignatureContentTemplate: destwebhook.DefaultSignatureContentTmpl,
+			SignatureHeaderTemplate:  "v0={{.Body}}", // header templates have no .Body — invalid at render
+			SignatureEncoding:        destwebhook.DefaultEncoding,
+			SignatureAlgorithm:       destwebhook.DefaultAlgorithm,
+			SigningSecretTemplate:    destwebhook.DefaultSigningSecretTmpl,
+		}
+	}
+
+	t.Run("default mode rejects an invalid template", func(t *testing.T) {
+		registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t))
+		err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{
+			Webhook: webhookConfig(""),
+		})
+		assert.ErrorContains(t, err, "can't evaluate field Body")
+	})
+
+	t.Run("standard mode ignores the configured templates", func(t *testing.T) {
+		registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t))
+		err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{
+			Webhook: webhookConfig("standard"),
+		})
+		assert.NoError(t, err)
+	})
+}

internal/destregistry/providers/destwebhook/destwebhook.go

@@ -104,6 +104,15 @@ type WebhookDestination struct {
 	topicHeader              headerConfig
 	encoding                 string
 	algorithm                string
+	// Built once in New() and shared by every publisher: both are derived
+	// solely from provider config, and a parsed template is safe for parallel
+	// execution. Building them per destination gave each cached publisher its
+	// own copy of two sprig function maps (~44 KB retained per publisher).
+	signatureFormatter SignatureFormatter
+	headerFormatter    HeaderFormatter
+	encoder            SignatureEncoder
+	signingAlgorithm   SigningAlgorithm
+
 	rawSigningSecretTemplate string
 	signingSecretTemplate    *template.Template
 	maxResponseBodyBytes     int
@@ -300,6 +309,25 @@ func New(loader metadata.MetadataLoader, basePublisherOpts []destregistry.BasePu
 		seenNames[effective] = h.label
 	}
 
+	// Build the signature formatters once — shared by every publisher
+	destination.signatureFormatter, err = NewSignatureFormatter(destination.signatureContentTemplate)
+	if err != nil {
+		return nil, err
+	}
+	destination.headerFormatter, err = NewHeaderFormatter(destination.signatureHeaderTemplate)
+	if err != nil {
+		return nil, err
+	}
+	// Dry-run render both, so a template that parses but references a field
+	// its payload type doesn't have fails construction even when the provider
+	// is built outside config validation.
+	if err := dryRunFormatters(destination.signatureFormatter, destination.headerFormatter,
+		destination.signatureContentTemplate, destination.signatureHeaderTemplate); err != nil {
+		return nil, err
+	}
+	destination.encoder = GetEncoder(destination.encoding)
+	destination.signingAlgorithm = GetAlgorithm(destination.algorithm)
+
 	// Parse signing secret template — fail on invalid syntax
 	tmpl, err := template.New("signing_secret").Funcs(sprig.TxtFuncMap()).Parse(destination.rawSigningSecretTemplate)
 	if err != nil {
@@ -393,10 +421,10 @@ func (d *WebhookDestination) CreatePublisher(ctx context.Context, destination *m
 
 	sm := NewSignatureManager(
 		secrets,
-		WithSignatureFormatter(NewSignatureFormatter(d.signatureContentTemplate)),
-		WithHeaderFormatter(NewHeaderFormatter(d.signatureHeaderTemplate)),
-		WithEncoder(GetEncoder(d.encoding)),
-		WithAlgorithm(GetAlgorithm(d.algorithm)),
+		WithSignatureFormatter(d.signatureFormatter),
+		WithHeaderFormatter(d.headerFormatter),
+		WithEncoder(d.encoder),
+		WithAlgorithm(d.signingAlgorithm),
 	)
 
 	var proxyURL *string
@@ -762,12 +790,15 @@ func (p *WebhookPublisher) Format(ctx context.Context, event *models.Event) (*ht
 
 	// Add signature header unless disabled
 	if !p.signatureHeader.disabled {
-		signatureHeader := p.sm.GenerateSignatureHeader(SignaturePayload{
+		signatureHeader, err := p.sm.GenerateSignatureHeader(SignaturePayload{
 			EventID:   event.ID,
 			Topic:     event.Topic,
 			Timestamp: now,
 			Body:      string(rawBody),
 		})
+		if err != nil {
+			return nil, err
+		}
 		if signatureHeader != "" {
 			req.Header.Set(resolveHeaderName(p.signatureHeader, p.headerPrefix, "signature"), signatureHeader)
 		}

internal/destregistry/providers/destwebhook/destwebhook_config_test.go

@@ -229,3 +229,40 @@ func TestWebhookDestination_SignatureOptions(t *testing.T) {
 		})
 	}
 }
+
+// Formatters are built in New() now, so a template that doesn't parse fails
+// provider construction rather than the first CreatePublisher call.
+func TestWebhookDestination_InvalidSignatureTemplate(t *testing.T) {
+	baseOpts := []destwebhook.Option{
+		destwebhook.WithHeaderPrefix(destwebhook.DefaultHeaderPrefix),
+		destwebhook.WithSignatureContentTemplate(destwebhook.DefaultSignatureContentTmpl),
+		destwebhook.WithSignatureHeaderTemplate(destwebhook.DefaultSignatureHeaderTmpl),
+		destwebhook.WithSignatureEncoding(destwebhook.DefaultEncoding),
+		destwebhook.WithSignatureAlgorithm(destwebhook.DefaultAlgorithm),
+		destwebhook.WithSigningSecretTemplate(destwebhook.DefaultSigningSecretTmpl),
+	}
+
+	tests := []struct {
+		name    string
+		opt     destwebhook.Option
+		wantErr string
+	}{
+		{
+			name:    "content template",
+			opt:     destwebhook.WithSignatureContentTemplate("{{.Timestamp.{{.Body}}"),
+			wantErr: "invalid signature content template",
+		},
+		{
+			name:    "header template",
+			opt:     destwebhook.WithSignatureHeaderTemplate("t={{.Timestamp},v0={{.Signatures}"),
+			wantErr: "invalid signature header template",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := destwebhook.New(testutil.Registry.MetadataLoader(), nil, append(baseOpts, tt.opt)...)
+			assert.ErrorContains(t, err, tt.wantErr)
+		})
+	}
+}

internal/destregistry/providers/destwebhook/destwebhook_internal_test.go

@@ -0,0 +1,62 @@
+package destwebhook
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/hookdeck/outpost/internal/destregistry/metadata"
+	"github.com/hookdeck/outpost/internal/models"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Formatters are built once in New() and shared by every publisher. This pins
+// that sharing: a regression back to per-destination construction (rebuilding
+// formatters inside CreatePublisher) makes the identity assertions fail.
+//
+// White-box on purpose — the formatter fields are unexported, and testutil
+// can't be imported here without a cycle, so the destination is built inline.
+func TestWebhookDestination_PublishersShareFormatters(t *testing.T) {
+	t.Parallel()
+
+	provider, err := New(metadata.NewMetadataLoader(""), nil,
+		WithHeaderPrefix(DefaultHeaderPrefix),
+		WithSignatureContentTemplate(DefaultSignatureContentTmpl),
+		WithSignatureHeaderTemplate(DefaultSignatureHeaderTmpl),
+		WithSignatureEncoding(DefaultEncoding),
+		WithSignatureAlgorithm(DefaultAlgorithm),
+		WithSigningSecretTemplate(DefaultSigningSecretTmpl),
+	)
+	require.NoError(t, err)
+
+	now := time.Now()
+	dest := models.Destination{
+		ID:          "dest-formatter-sharing",
+		TenantID:    "test-tenant",
+		Type:        "webhook",
+		Topics:      []string{"*"},
+		Config:      map[string]string{"url": "http://localhost:8080/webhook"},
+		Credentials: map[string]string{"secret": "test-secret"},
+		CreatedAt:   now,
+		UpdatedAt:   now,
+	}
+
+	pub1, err := provider.CreatePublisher(context.Background(), &dest)
+	require.NoError(t, err)
+	defer pub1.Close()
+	pub2, err := provider.CreatePublisher(context.Background(), &dest)
+	require.NoError(t, err)
+	defer pub2.Close()
+
+	wp1, ok := pub1.(*WebhookPublisher)
+	require.True(t, ok)
+	wp2, ok := pub2.(*WebhookPublisher)
+	require.True(t, ok)
+
+	// Both publishers hold the provider's formatter instances, not copies.
+	assert.Same(t, provider.signatureFormatter, wp1.sm.sigFormatter)
+	assert.Same(t, provider.headerFormatter, wp1.sm.headerFormatter)
+	assert.Same(t, wp1.sm.sigFormatter, wp2.sm.sigFormatter)
+	assert.Same(t, wp1.sm.headerFormatter, wp2.sm.headerFormatter)
+}

internal/destregistry/providers/destwebhook/destwebhook_publish_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hookdeck/outpost/internal/destregistry"
 	"github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook"
 	testsuite "github.com/hookdeck/outpost/internal/destregistry/testing"
 	"github.com/hookdeck/outpost/internal/models"
@@ -1047,6 +1048,59 @@ func TestWebhookPublisher_PreservesKeyOrder(t *testing.T) {
 	assert.Equal(t, `{"z":1,"a":2,"m":3}`, string(body))
 }
 
+// A template that parses but references a field of the other payload type only
+// fails when it renders. That must fail the delivery, not the process.
+func TestWebhookPublisher_SignatureTemplateRenderFailure(t *testing.T) {
+	dest := testutil.DestinationFactory.Any(
+		testutil.DestinationFactory.WithType("webhook"),
+		testutil.DestinationFactory.WithConfig(map[string]string{
+			"url": "http://example.com",
+		}),
+		testutil.DestinationFactory.WithCredentials(map[string]string{
+			"secret": "test-secret",
+		}),
+	)
+
+	// Wrong-field templates now fail provider construction (see
+	// TestNew_ValidatesSignatureTemplates), so this uses a value-dependent
+	// template: it survives the construction dry-run, which renders against
+	// two synthetic signatures, and fails here where the single configured
+	// secret yields only one.
+	tests := []struct {
+		name string
+		opt  destwebhook.Option
+	}{
+		{
+			name: "header template indexes a signature that doesn't exist",
+			opt:  destwebhook.WithSignatureHeaderTemplate("v0={{index .Signatures 1}}"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			provider := NewTestProvider(t, tt.opt)
+
+			publisher, err := provider.CreatePublisher(context.Background(), &dest)
+			require.NoError(t, err)
+
+			event := testutil.EventFactory.Any(
+				testutil.EventFactory.WithDataMap(map[string]interface{}{"hello": "world"}),
+			)
+
+			assert.NotPanics(t, func() {
+				delivery, err := publisher.Publish(context.Background(), &event)
+				require.Error(t, err)
+				require.NotNil(t, delivery)
+				assert.Equal(t, "failed", delivery.Status)
+
+				var publishErr *destregistry.ErrDestinationPublishAttempt
+				require.ErrorAs(t, err, &publishErr)
+				assert.Equal(t, "format_failed", publishErr.Data["error"])
+			})
+		})
+	}
+}
+
 func TestWebhookPublisher_SignatureTemplates(t *testing.T) {
 	dest := testutil.DestinationFactory.Any(
 		testutil.DestinationFactory.WithType("webhook"),
@@ -1158,10 +1212,14 @@ func TestWebhookPublisher_SignatureTemplates(t *testing.T) {
 			if verifyHeaderTemplate == "" {
 				verifyHeaderTemplate = destwebhook.DefaultSignatureHeaderTmpl
 			}
+			sigFormatter, err := destwebhook.NewSignatureFormatter(verifyContentTemplate)
+			require.NoError(t, err)
+			headerFormatter, err := destwebhook.NewHeaderFormatter(verifyHeaderTemplate)
+			require.NoError(t, err)
 			sm := destwebhook.NewSignatureManager(
 				secrets,
-				destwebhook.WithSignatureFormatter(destwebhook.NewSignatureFormatter(verifyContentTemplate)),
-				destwebhook.WithHeaderFormatter(destwebhook.NewHeaderFormatter(verifyHeaderTemplate)),
+				destwebhook.WithSignatureFormatter(sigFormatter),
+				destwebhook.WithHeaderFormatter(headerFormatter),
 			)
 
 			// Verify signature matches expected content