diff --git a/internal/destregistry/providers/destwebhook/destwebhook.go b/internal/destregistry/providers/destwebhook/destwebhook.go index 96e4d067..c0a00508 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook.go +++ b/internal/destregistry/providers/destwebhook/destwebhook.go @@ -322,6 +322,12 @@ func (d *WebhookDestination) resolveConfig(ctx context.Context, destination *mod Type: "invalid", }}) } + if len(config.CustomHeaders) == 0 { + return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ + Field: "config.custom_headers", + Type: "invalid", + }}) + } if err := ValidateCustomHeaders(config.CustomHeaders); err != nil { return nil, nil, err } diff --git a/internal/destregistry/providers/destwebhook/destwebhook_config_test.go b/internal/destregistry/providers/destwebhook/destwebhook_config_test.go index cd44a118..d9f14ace 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook_config_test.go +++ b/internal/destregistry/providers/destwebhook/destwebhook_config_test.go @@ -105,7 +105,7 @@ func TestWebhookDestination_CustomHeadersConfig(t *testing.T) { assert.NoError(t, err) }) - t.Run("should parse config with empty custom_headers", func(t *testing.T) { + t.Run("should fail on empty custom_headers object", func(t *testing.T) { t.Parallel() destination := testutil.DestinationFactory.Any( testutil.DestinationFactory.WithType("webhook"), @@ -119,7 +119,11 @@ func TestWebhookDestination_CustomHeadersConfig(t *testing.T) { ) err := webhookDestination.Validate(context.Background(), &destination) - assert.NoError(t, err) + assert.Error(t, err) + var validationErr *destregistry.ErrDestinationValidation + assert.ErrorAs(t, err, &validationErr) + assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field) + assert.Equal(t, "invalid", validationErr.Errors[0].Type) }) t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) { diff --git a/internal/destregistry/providers/destwebhook/destwebhook_publish_test.go b/internal/destregistry/providers/destwebhook/destwebhook_publish_test.go index 2b755ca6..db05b84f 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook_publish_test.go +++ b/internal/destregistry/providers/destwebhook/destwebhook_publish_test.go @@ -11,6 +11,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" @@ -566,7 +567,7 @@ func TestWebhookPublisher_CustomHeaders(t *testing.T) { assert.Equal(t, "delivery-metadata-value", req.Header.Get("x-outpost-source")) }) - t.Run("should work with empty custom_headers", func(t *testing.T) { + t.Run("should fail CreatePublisher when custom_headers is empty object", func(t *testing.T) { t.Parallel() provider, err := destwebhook.New(testutil.Registry.MetadataLoader(), nil) @@ -583,19 +584,12 @@ func TestWebhookPublisher_CustomHeaders(t *testing.T) { }), ) - publisher, err := provider.CreatePublisher(context.Background(), &destination) - require.NoError(t, err) - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithData(map[string]interface{}{"key": "value"}), - ) - - req, err := publisher.(*destwebhook.WebhookPublisher).Format(context.Background(), &event) - require.NoError(t, err) - - // Should still have standard headers - assert.NotEmpty(t, req.Header.Get("x-outpost-event-id")) - assert.NotEmpty(t, req.Header.Get("x-outpost-timestamp")) + _, err = provider.CreatePublisher(context.Background(), &destination) + require.Error(t, err) + var validationErr *destregistry.ErrDestinationValidation + assert.ErrorAs(t, err, &validationErr) + assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field) + assert.Equal(t, "invalid", validationErr.Errors[0].Type) }) t.Run("should work without custom_headers field", func(t *testing.T) { diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go index 2899044c..890ae9f5 100644 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go +++ b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go @@ -250,6 +250,12 @@ func (d *StandardWebhookDestination) resolveConfig(ctx context.Context, destinat Type: "invalid", }}) } + if len(config.CustomHeaders) == 0 { + return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ + Field: "config.custom_headers", + Type: "invalid", + }}) + } if err := destwebhook.ValidateCustomHeaders(config.CustomHeaders); err != nil { return nil, nil, err } diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go index 21494a8b..bd5593d2 100644 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go +++ b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go @@ -34,7 +34,7 @@ func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) { assert.NoError(t, err) }) - t.Run("should parse config with empty custom_headers", func(t *testing.T) { + t.Run("should fail on empty custom_headers object", func(t *testing.T) { t.Parallel() destination := testutil.DestinationFactory.Any( testutil.DestinationFactory.WithType("webhook"), @@ -48,7 +48,11 @@ func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) { ) err := provider.Validate(context.Background(), &destination) - assert.NoError(t, err) + assert.Error(t, err) + var validationErr *destregistry.ErrDestinationValidation + assert.ErrorAs(t, err, &validationErr) + assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field) + assert.Equal(t, "invalid", validationErr.Errors[0].Type) }) t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) { diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go index ec648b83..c94a6849 100644 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go +++ b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/hookdeck/outpost/internal/destregistry" "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard" testsuite "github.com/hookdeck/outpost/internal/destregistry/testing" "github.com/hookdeck/outpost/internal/models" @@ -521,19 +522,16 @@ func TestStandardWebhookPublisher_CustomHeaders(t *testing.T) { } }) - t.Run("should work with empty custom_headers", func(t *testing.T) { + t.Run("should fail CreatePublisher when custom_headers is empty object", func(t *testing.T) { t.Parallel() - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - provider, err := destwebhookstandard.New(testutil.Registry.MetadataLoader(), nil) require.NoError(t, err) dest := testutil.DestinationFactory.Any( testutil.DestinationFactory.WithType("webhook"), testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", + "url": "http://example.com/webhook", "custom_headers": `{}`, }), testutil.DestinationFactory.WithCredentials(map[string]string{ @@ -541,26 +539,12 @@ func TestStandardWebhookPublisher_CustomHeaders(t *testing.T) { }), ) - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithData(map[string]interface{}{"key": "value"}), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - // Should still have standard headers - assert.NotEmpty(t, req.Header.Get("webhook-id")) - assert.NotEmpty(t, req.Header.Get("webhook-timestamp")) - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } + _, err = provider.CreatePublisher(context.Background(), &dest) + require.Error(t, err) + var validationErr *destregistry.ErrDestinationValidation + assert.ErrorAs(t, err, &validationErr) + assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field) + assert.Equal(t, "invalid", validationErr.Errors[0].Type) }) t.Run("should work without custom_headers field", func(t *testing.T) { diff --git a/internal/portal/src/scenes/Destination/DestinationSettings/DestinationSettings.tsx b/internal/portal/src/scenes/Destination/DestinationSettings/DestinationSettings.tsx index f9c9f07b..0a4730ce 100644 --- a/internal/portal/src/scenes/Destination/DestinationSettings/DestinationSettings.tsx +++ b/internal/portal/src/scenes/Destination/DestinationSettings/DestinationSettings.tsx @@ -120,7 +120,16 @@ const DestinationSettings = ({ if (type.credential_fields.some((field) => field.key === key)) { credentials[key] = String(value); } else { - config[key] = String(value); + let configValue = String(value); + // Webhook custom_headers must have at least one header or be null/empty + if ( + type.type === "webhook" && + key === "custom_headers" && + (configValue === "{}" || configValue === "") + ) { + configValue = ""; + } + config[key] = configValue; } });