Skip to content

Commit 5c7229a

Browse files
alexboucharddalexbouchardd
committed
fix: enforce validation for empty custom_headers in webhook configurations
- Updated tests to ensure that an error is returned when custom_headers is empty or invalid. - Modified the WebhookDestination and StandardWebhookDestination to return validation errors for empty custom_headers. - Adjusted frontend logic to handle empty custom_headers appropriately, ensuring backward compatibility while enforcing validation rules.
1 parent d0fa4ce commit 5c7229a

File tree

7 files changed

+51
-44
lines changed

7 files changed

+51
-44
lines changed

‎internal/destregistry/providers/destwebhook/destwebhook.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,12 @@ func (d *WebhookDestination) resolveConfig(ctx context.Context, destination *mod
322322
Type: "invalid",
323323
}})
324324
}
325+
if len(config.CustomHeaders) == 0 {
326+
return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{
327+
Field: "config.custom_headers",
328+
Type: "invalid",
329+
}})
330+
}
325331
if err := ValidateCustomHeaders(config.CustomHeaders); err != nil {
326332
return nil, nil, err
327333
}

‎internal/destregistry/providers/destwebhook/destwebhook_config_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestWebhookDestination_CustomHeadersConfig(t *testing.T) {
105105
assert.NoError(t, err)
106106
})
107107

108-
t.Run("should parse config with empty custom_headers", func(t *testing.T) {
108+
t.Run("should fail on empty custom_headers object", func(t *testing.T) {
109109
t.Parallel()
110110
destination := testutil.DestinationFactory.Any(
111111
testutil.DestinationFactory.WithType("webhook"),
@@ -119,7 +119,11 @@ func TestWebhookDestination_CustomHeadersConfig(t *testing.T) {
119119
)
120120

121121
err := webhookDestination.Validate(context.Background(), &destination)
122-
assert.NoError(t, err)
122+
assert.Error(t, err)
123+
var validationErr *destregistry.ErrDestinationValidation
124+
assert.ErrorAs(t, err, &validationErr)
125+
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
126+
assert.Equal(t, "invalid", validationErr.Errors[0].Type)
123127
})
124128

125129
t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) {

‎internal/destregistry/providers/destwebhook/destwebhook_publish_test.go‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212
"time"
1313

14+
"github.com/hookdeck/outpost/internal/destregistry"
1415
"github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook"
1516
testsuite "github.com/hookdeck/outpost/internal/destregistry/testing"
1617
"github.com/hookdeck/outpost/internal/models"
@@ -566,7 +567,7 @@ func TestWebhookPublisher_CustomHeaders(t *testing.T) {
566567
assert.Equal(t, "delivery-metadata-value", req.Header.Get("x-outpost-source"))
567568
})
568569

569-
t.Run("should work with empty custom_headers", func(t *testing.T) {
570+
t.Run("should fail CreatePublisher when custom_headers is empty object", func(t *testing.T) {
570571
t.Parallel()
571572

572573
provider, err := destwebhook.New(testutil.Registry.MetadataLoader(), nil)
@@ -583,19 +584,12 @@ func TestWebhookPublisher_CustomHeaders(t *testing.T) {
583584
}),
584585
)
585586

586-
publisher, err := provider.CreatePublisher(context.Background(), &destination)
587-
require.NoError(t, err)
588-
589-
event := testutil.EventFactory.Any(
590-
testutil.EventFactory.WithData(map[string]interface{}{"key": "value"}),
591-
)
592-
593-
req, err := publisher.(*destwebhook.WebhookPublisher).Format(context.Background(), &event)
594-
require.NoError(t, err)
595-
596-
// Should still have standard headers
597-
assert.NotEmpty(t, req.Header.Get("x-outpost-event-id"))
598-
assert.NotEmpty(t, req.Header.Get("x-outpost-timestamp"))
587+
_, err = provider.CreatePublisher(context.Background(), &destination)
588+
require.Error(t, err)
589+
var validationErr *destregistry.ErrDestinationValidation
590+
assert.ErrorAs(t, err, &validationErr)
591+
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
592+
assert.Equal(t, "invalid", validationErr.Errors[0].Type)
599593
})
600594

601595
t.Run("should work without custom_headers field", func(t *testing.T) {

‎internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,12 @@ func (d *StandardWebhookDestination) resolveConfig(ctx context.Context, destinat
250250
Type: "invalid",
251251
}})
252252
}
253+
if len(config.CustomHeaders) == 0 {
254+
return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{
255+
Field: "config.custom_headers",
256+
Type: "invalid",
257+
}})
258+
}
253259
if err := destwebhook.ValidateCustomHeaders(config.CustomHeaders); err != nil {
254260
return nil, nil, err
255261
}

‎internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) {
3434
assert.NoError(t, err)
3535
})
3636

37-
t.Run("should parse config with empty custom_headers", func(t *testing.T) {
37+
t.Run("should fail on empty custom_headers object", func(t *testing.T) {
3838
t.Parallel()
3939
destination := testutil.DestinationFactory.Any(
4040
testutil.DestinationFactory.WithType("webhook"),
@@ -48,7 +48,11 @@ func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) {
4848
)
4949

5050
err := provider.Validate(context.Background(), &destination)
51-
assert.NoError(t, err)
51+
assert.Error(t, err)
52+
var validationErr *destregistry.ErrDestinationValidation
53+
assert.ErrorAs(t, err, &validationErr)
54+
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
55+
assert.Equal(t, "required", validationErr.Errors[0].Type)
5256
})
5357

5458
t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) {

‎internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212
"time"
1313

14+
"github.com/hookdeck/outpost/internal/destregistry"
1415
"github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard"
1516
testsuite "github.com/hookdeck/outpost/internal/destregistry/testing"
1617
"github.com/hookdeck/outpost/internal/models"
@@ -521,46 +522,29 @@ func TestStandardWebhookPublisher_CustomHeaders(t *testing.T) {
521522
}
522523
})
523524

524-
t.Run("should work with empty custom_headers", func(t *testing.T) {
525+
t.Run("should fail CreatePublisher when custom_headers is empty object", func(t *testing.T) {
525526
t.Parallel()
526527

527-
consumer := NewStandardWebhookConsumer()
528-
defer consumer.Close()
529-
530528
provider, err := destwebhookstandard.New(testutil.Registry.MetadataLoader(), nil)
531529
require.NoError(t, err)
532530

533531
dest := testutil.DestinationFactory.Any(
534532
testutil.DestinationFactory.WithType("webhook"),
535533
testutil.DestinationFactory.WithConfig(map[string]string{
536-
"url": consumer.server.URL + "/webhook",
534+
"url": "http://example.com/webhook",
537535
"custom_headers": `{}`,
538536
}),
539537
testutil.DestinationFactory.WithCredentials(map[string]string{
540538
"secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw",
541539
}),
542540
)
543541

544-
publisher, err := provider.CreatePublisher(context.Background(), &dest)
545-
require.NoError(t, err)
546-
defer publisher.Close()
547-
548-
event := testutil.EventFactory.Any(
549-
testutil.EventFactory.WithData(map[string]interface{}{"key": "value"}),
550-
)
551-
552-
_, err = publisher.Publish(context.Background(), &event)
553-
require.NoError(t, err)
554-
555-
select {
556-
case msg := <-consumer.Consume():
557-
req := msg.Raw.(*http.Request)
558-
// Should still have standard headers
559-
assert.NotEmpty(t, req.Header.Get("webhook-id"))
560-
assert.NotEmpty(t, req.Header.Get("webhook-timestamp"))
561-
case <-time.After(5 * time.Second):
562-
t.Fatal("timeout waiting for message")
563-
}
542+
_, err = provider.CreatePublisher(context.Background(), &dest)
543+
require.Error(t, err)
544+
var validationErr *destregistry.ErrDestinationValidation
545+
assert.ErrorAs(t, err, &validationErr)
546+
assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field)
547+
assert.Equal(t, "required", validationErr.Errors[0].Type)
564548
})
565549

566550
t.Run("should work without custom_headers field", func(t *testing.T) {

‎internal/portal/src/scenes/Destination/DestinationSettings/DestinationSettings.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,16 @@ const DestinationSettings = ({
120120
if (type.credential_fields.some((field) => field.key === key)) {
121121
credentials[key] = String(value);
122122
} else {
123-
config[key] = String(value);
123+
let configValue = String(value);
124+
// Webhook custom_headers must have at least one header or be null/empty
125+
if (
126+
type.type === "webhook" &&
127+
key === "custom_headers" &&
128+
(configValue === "{}" || configValue === "")
129+
) {
130+
configValue = "";
131+
}
132+
config[key] = configValue;
124133
}
125134
});
126135

0 commit comments

Comments
 (0)