[pull] main from hookdeck:main

build/test/compose.yml

@@ -26,7 +26,7 @@ services:
       - 35672:5672
       - 45672:15672
   test-aws:
-    image: localstack/localstack:latest
+    image: localstack/localstack:3
     environment:
       - SERVICES=s3,sns,sts,sqs,kinesis
     ports:
@@ -60,8 +60,3 @@ services:
       ]
     ports:
       - "38085:8085"
-
-networks:
-  default:
-    name: outpost
-    external: true

cmd/e2e/configs/basic.go

@@ -89,10 +89,6 @@ func Basic(t *testing.T, opts BasicOpts) config.Config {
 	c.Alert.AutoDisableDestination = true
 	c.Alert.ConsecutiveFailureCount = 20
 
-	// Use signature templates with timestamps for mock server compatibility
-	c.Destinations.Webhook.SignatureContentTemplate = "{{.Timestamp.Unix}}.{{.Body}}"
-	c.Destinations.Webhook.SignatureHeaderTemplate = "t={{.Timestamp.Unix}},v0={{.Signatures | join \",\"}}"
-
 	// Setup cleanup
 	t.Cleanup(func() {
 		redisClient, err := redis.New(context.Background(), c.Redis.ToConfig())

internal/apirouter/destination_credentials_test.go

@@ -90,6 +90,42 @@ func TestDestinationCredentials_SecretRotationViaAPI(t *testing.T) {
 		"previous_secret should be the old secret")
 	assert.NotEmpty(t, rotated.Credentials["previous_secret_invalid_at"],
 		"previous_secret_invalid_at should be set")
+
+	// Rotate again with an explicit invalidation window
+	customInvalidAt := time.Now().Add(1 * time.Hour).Format(time.RFC3339)
+	secondRotateReq := h.jsonReq(http.MethodPatch, "/api/v1/tenants/t1/destinations/d1", map[string]any{
+		"credentials": map[string]any{
+			"rotate_secret":              true,
+			"previous_secret_invalid_at": customInvalidAt,
+		},
+	})
+	secondRotateResp := h.do(h.withAPIKey(secondRotateReq))
+	require.Equal(t, http.StatusOK, secondRotateResp.Code)
+
+	var secondRotated destregistry.DestinationDisplay
+	require.NoError(t, json.Unmarshal(secondRotateResp.Body.Bytes(), &secondRotated))
+	assert.Equal(t, customInvalidAt, secondRotated.Credentials["previous_secret_invalid_at"],
+		"explicit previous_secret_invalid_at should be respected")
+
+	// Rotate once more without previous_secret_invalid_at: the 24h default
+	// must be re-applied, not the previously stored window carried forward
+	thirdRotateReq := h.jsonReq(http.MethodPatch, "/api/v1/tenants/t1/destinations/d1", map[string]any{
+		"credentials": map[string]any{
+			"rotate_secret": true,
+		},
+	})
+	thirdRotateResp := h.do(h.withAPIKey(thirdRotateReq))
+	require.Equal(t, http.StatusOK, thirdRotateResp.Code)
+
+	var thirdRotated destregistry.DestinationDisplay
+	require.NoError(t, json.Unmarshal(thirdRotateResp.Body.Bytes(), &thirdRotated))
+	assert.Equal(t, secondRotated.Credentials["secret"], thirdRotated.Credentials["previous_secret"],
+		"previous_secret should be the secret from the previous rotation")
+
+	invalidAt, err := time.Parse(time.RFC3339, thirdRotated.Credentials["previous_secret_invalid_at"])
+	require.NoError(t, err)
+	assert.WithinDuration(t, time.Now().Add(24*time.Hour), invalidAt, time.Minute,
+		"omitting previous_secret_invalid_at should default to now+24h")
 }
 
 func TestDestinationCredentials_TenantCannotSetCustomSecret(t *testing.T) {

internal/apirouter/destination_handlers.go

@@ -121,6 +121,10 @@ func (h *DestinationHandlers) Create(c *gin.Context) {
 	}
 	if err := h.registry.PreprocessDestination(&destination, nil, &destregistry.PreprocessDestinationOpts{
 		Role: mustRoleFromContext(c),
+		Request: destregistry.PreprocessRequest{
+			Config:      destination.Config,
+			Credentials: destination.Credentials,
+		},
 	}); err != nil {
 		AbortWithValidationError(c, err)
 		return
@@ -193,7 +197,7 @@ func (h *DestinationHandlers) Update(c *gin.Context) {
 	}
 
 	// Config (merge-patch)
-	configResult, configChanged, err := applyMergePatchStringMap(originalDestination.Config, input.Config)
+	configResult, configRequest, configChanged, err := applyMergePatchStringMap(originalDestination.Config, input.Config)
 	if err != nil {
 		AbortWithValidationError(c, fmt.Errorf("invalid config: %w", err))
 		return
@@ -204,7 +208,7 @@ func (h *DestinationHandlers) Update(c *gin.Context) {
 	}
 
 	// Credentials (merge-patch)
-	credsResult, credsChanged, err := applyMergePatchStringMap(originalDestination.Credentials, input.Credentials)
+	credsResult, credsRequest, credsChanged, err := applyMergePatchStringMap(originalDestination.Credentials, input.Credentials)
 	if err != nil {
 		AbortWithValidationError(c, fmt.Errorf("invalid credentials: %w", err))
 		return
@@ -229,7 +233,7 @@ func (h *DestinationHandlers) Update(c *gin.Context) {
 	}
 
 	// DeliveryMetadata (merge-patch)
-	dmResult, dmChanged, err := applyMergePatchStringMap(originalDestination.DeliveryMetadata, input.DeliveryMetadata)
+	dmResult, _, dmChanged, err := applyMergePatchStringMap(originalDestination.DeliveryMetadata, input.DeliveryMetadata)
 	if err != nil {
 		AbortWithValidationError(c, fmt.Errorf("invalid delivery_metadata: %w", err))
 		return
@@ -239,7 +243,7 @@ func (h *DestinationHandlers) Update(c *gin.Context) {
 	}
 
 	// Metadata (merge-patch)
-	metaResult, metaChanged, err := applyMergePatchStringMap(originalDestination.Metadata, input.Metadata)
+	metaResult, _, metaChanged, err := applyMergePatchStringMap(originalDestination.Metadata, input.Metadata)
 	if err != nil {
 		AbortWithValidationError(c, fmt.Errorf("invalid metadata: %w", err))
 		return
@@ -279,6 +283,10 @@ func (h *DestinationHandlers) Update(c *gin.Context) {
 	// Always preprocess before updating
 	if err := h.registry.PreprocessDestination(&updatedDestination, originalDestination, &destregistry.PreprocessDestinationOpts{
 		Role: mustRoleFromContext(c),
+		Request: destregistry.PreprocessRequest{
+			Config:      configRequest,
+			Credentials: credsRequest,
+		},
 	}); err != nil {
 		AbortWithValidationError(c, err)
 		return
@@ -507,26 +515,28 @@ func isJSONNull(raw json.RawMessage) bool {
 }
 
 // applyMergePatchStringMap applies RFC 7396 merge-patch semantics for a map[string]string field.
-// Returns (result, changed, error):
+// Returns (result, request, changed, error), where request is the patch as
+// sent by the caller (string-coerced, nulls dropped):
 //   - raw is nil (field omitted): returns original unchanged
 //   - raw is "null": returns nil (clear field)
 //   - raw is "{}": returns original unchanged (empty merge = no-op)
 //   - raw is an object: merge-patch into original
-func applyMergePatchStringMap(original map[string]string, raw json.RawMessage) (map[string]string, bool, error) {
+func applyMergePatchStringMap(original map[string]string, raw json.RawMessage) (map[string]string, map[string]string, bool, error) {
 	if raw == nil {
-		return original, false, nil
+		return original, nil, false, nil
 	}
 	if isJSONNull(raw) {
-		return nil, true, nil
+		return nil, nil, true, nil
 	}
 	var patch map[string]any
 	if err := json.Unmarshal(raw, &patch); err != nil {
-		return nil, false, err
+		return nil, nil, false, err
 	}
 	if len(patch) == 0 {
-		return original, false, nil
+		return original, nil, false, nil
 	}
-	return maputil.MergePatchStringMap(original, patch), true, nil
+	request := maputil.MergePatchStringMap(nil, patch)
+	return maputil.MergePatchStringMap(original, patch), request, true, nil
 }
 
 // tenantSnapshot captures the tenant's derived state before a destination mutation.

internal/destregistry/providers/destwebhook/destwebhook.go

@@ -443,7 +443,7 @@ func (d *WebhookDestination) resolveConfig(ctx context.Context, destination *mod
 }
 
 // rotateSecret handles secret rotation and returns clean credentials
-func (d *WebhookDestination) rotateSecret(newDest, origDest *models.Destination) (map[string]string, error) {
+func (d *WebhookDestination) rotateSecret(origDest *models.Destination, opts *destregistry.PreprocessDestinationOpts) (map[string]string, error) {
 	if origDest == nil {
 		return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{
 			{
@@ -474,9 +474,11 @@ func (d *WebhookDestination) rotateSecret(newDest, origDest *models.Destination)
 	}
 	creds["secret"] = secret
 
-	// Keep custom invalidation time if provided, otherwise set default
-	if newDest.Credentials["previous_secret_invalid_at"] != "" {
-		creds["previous_secret_invalid_at"] = newDest.Credentials["previous_secret_invalid_at"]
+	// Keep custom invalidation time if provided, otherwise set default.
+	// The merged credentials can't tell us whether the caller sent the
+	// field — the raw request credentials can.
+	if invalidAt := opts.Request.Credentials["previous_secret_invalid_at"]; invalidAt != "" {
+		creds["previous_secret_invalid_at"] = invalidAt
 	} else {
 		creds["previous_secret_invalid_at"] = time.Now().Add(24 * time.Hour).Format(time.RFC3339)
 	}
@@ -607,7 +609,7 @@ func (d *WebhookDestination) Preprocess(newDestination *models.Destination, orig
 	var cleanCredentials map[string]string
 	var err error
 	if isTruthy(newDestination.Credentials["rotate_secret"]) {
-		cleanCredentials, err = d.rotateSecret(newDestination, originalDestination)
+		cleanCredentials, err = d.rotateSecret(originalDestination, opts)
 	} else {
 		cleanCredentials, err = d.updateSecret(newDestination, originalDestination, opts)
 		// For new destinations, ensure credentials are initialized if needed

internal/destregistry/providers/destwebhook/destwebhook_validate_test.go

@@ -606,17 +606,65 @@ func TestWebhookDestination_Preprocess(t *testing.T) {
 			}),
 		)
 
-		// Merge both config and credentials to simulate handler behavior
+		// Merge both config and credentials to simulate handler behavior,
+		// passing the raw request credentials via opts as the handler does
+		requestCredentials := newDestination.Credentials
 		newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config)
 		newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials)
 
-		err := webhookDestination.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{})
+		err := webhookDestination.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{
+			Request: destregistry.PreprocessRequest{Credentials: requestCredentials},
+		})
 		require.NoError(t, err)
 
 		// Verify that the custom invalidation time was preserved
 		assert.Equal(t, customInvalidAt, newDestination.Credentials["previous_secret_invalid_at"])
 	})
 
+	t.Run("should apply 24h default on rotation when request omits invalid_at", func(t *testing.T) {
+		t.Parallel()
+		storedInvalidAt := time.Now().Add(1 * time.Hour).Format(time.RFC3339)
+		originalDestination := testutil.DestinationFactory.Any(
+			testutil.DestinationFactory.WithType("webhook"),
+			testutil.DestinationFactory.WithConfig(map[string]string{
+				"url": "https://example.com",
+			}),
+			testutil.DestinationFactory.WithCredentials(map[string]string{
+				"secret":                     "current-secret",
+				"previous_secret":            "older-secret",
+				"previous_secret_invalid_at": storedInvalidAt,
+			}),
+		)
+
+		newDestination := testutil.DestinationFactory.Any(
+			testutil.DestinationFactory.WithType("webhook"),
+			testutil.DestinationFactory.WithConfig(map[string]string{
+				"url": "https://example.com",
+			}),
+			testutil.DestinationFactory.WithCredentials(map[string]string{
+				"rotate_secret": "true",
+			}),
+		)
+
+		// Merge both config and credentials to simulate handler behavior:
+		// the stored previous_secret_invalid_at is merged into newDestination
+		// even though the caller's request didn't contain it.
+		requestCredentials := newDestination.Credentials
+		newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config)
+		newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials)
+
+		err := webhookDestination.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{
+			Request: destregistry.PreprocessRequest{Credentials: requestCredentials},
+		})
+		require.NoError(t, err)
+
+		// The merged-in stored value must not be carried forward; the
+		// default window applies.
+		invalidAt, err := time.Parse(time.RFC3339, newDestination.Credentials["previous_secret_invalid_at"])
+		require.NoError(t, err)
+		assert.WithinDuration(t, time.Now().Add(24*time.Hour), invalidAt, 5*time.Second)
+	})
+
 	t.Run("should set default previous_secret_invalid_at when previous_secret is provided", func(t *testing.T) {
 		t.Parallel()
 		originalDestination := testutil.DestinationFactory.Any(

internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go

@@ -344,7 +344,7 @@ func (d *StandardWebhookDestination) resolveConfig(ctx context.Context, destinat
 }
 
 // rotateSecret handles secret rotation and returns clean credentials
-func (d *StandardWebhookDestination) rotateSecret(newDest, origDest *models.Destination) (map[string]string, error) {
+func (d *StandardWebhookDestination) rotateSecret(origDest *models.Destination, opts *destregistry.PreprocessDestinationOpts) (map[string]string, error) {
 	if origDest == nil {
 		return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{
 			{
@@ -375,9 +375,11 @@ func (d *StandardWebhookDestination) rotateSecret(newDest, origDest *models.Dest
 	}
 	creds["secret"] = secret
 
-	// Keep custom invalidation time if provided, otherwise set default
-	if newDest.Credentials["previous_secret_invalid_at"] != "" {
-		creds["previous_secret_invalid_at"] = newDest.Credentials["previous_secret_invalid_at"]
+	// Keep custom invalidation time if provided, otherwise set default.
+	// The merged credentials can't tell us whether the caller sent the
+	// field — the raw request credentials can.
+	if invalidAt := opts.Request.Credentials["previous_secret_invalid_at"]; invalidAt != "" {
+		creds["previous_secret_invalid_at"] = invalidAt
 	} else {
 		creds["previous_secret_invalid_at"] = time.Now().Add(24 * time.Hour).Format(time.RFC3339)
 	}
@@ -508,7 +510,7 @@ func (d *StandardWebhookDestination) Preprocess(newDestination *models.Destinati
 	var cleanCredentials map[string]string
 	var err error
 	if isTruthy(newDestination.Credentials["rotate_secret"]) {
-		cleanCredentials, err = d.rotateSecret(newDestination, originalDestination)
+		cleanCredentials, err = d.rotateSecret(originalDestination, opts)
 	} else {
 		cleanCredentials, err = d.updateSecret(newDestination, originalDestination, opts)
 		// For new destinations, ensure credentials are initialized if needed

internal/destregistry/providers/destwebhookstandard/destwebhookstandard_validate_test.go

@@ -553,17 +553,65 @@ func TestStandardWebhookDestination_Preprocess(t *testing.T) {
 			}),
 		)
 
-		// Merge both config and credentials to simulate handler behavior
+		// Merge both config and credentials to simulate handler behavior,
+		// passing the raw request credentials via opts as the handler does
+		requestCredentials := newDestination.Credentials
 		newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config)
 		newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials)
 
-		err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{})
+		err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{
+			Request: destregistry.PreprocessRequest{Credentials: requestCredentials},
+		})
 		require.NoError(t, err)
 
 		// Verify that the custom invalidation time was preserved
 		assert.Equal(t, customInvalidAt, newDestination.Credentials["previous_secret_invalid_at"])
 	})
 
+	t.Run("should apply 24h default on rotation when request omits invalid_at", func(t *testing.T) {
+		t.Parallel()
+		storedInvalidAt := time.Now().Add(1 * time.Hour).Format(time.RFC3339)
+		originalDestination := testutil.DestinationFactory.Any(
+			testutil.DestinationFactory.WithType("webhook"),
+			testutil.DestinationFactory.WithConfig(map[string]string{
+				"url": "https://example.com",
+			}),
+			testutil.DestinationFactory.WithCredentials(map[string]string{
+				"secret":                     "whsec_CurrentSecretBase64EncodedString",
+				"previous_secret":            "whsec_OlderSecretBase64EncodedStringgg",
+				"previous_secret_invalid_at": storedInvalidAt,
+			}),
+		)
+
+		newDestination := testutil.DestinationFactory.Any(
+			testutil.DestinationFactory.WithType("webhook"),
+			testutil.DestinationFactory.WithConfig(map[string]string{
+				"url": "https://example.com",
+			}),
+			testutil.DestinationFactory.WithCredentials(map[string]string{
+				"rotate_secret": "true",
+			}),
+		)
+
+		// Merge both config and credentials to simulate handler behavior:
+		// the stored previous_secret_invalid_at is merged into newDestination
+		// even though the caller's request didn't contain it.
+		requestCredentials := newDestination.Credentials
+		newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config)
+		newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials)
+
+		err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{
+			Request: destregistry.PreprocessRequest{Credentials: requestCredentials},
+		})
+		require.NoError(t, err)
+
+		// The merged-in stored value must not be carried forward; the
+		// default window applies.
+		invalidAt, err := time.Parse(time.RFC3339, newDestination.Credentials["previous_secret_invalid_at"])
+		require.NoError(t, err)
+		assert.WithinDuration(t, time.Now().Add(24*time.Hour), invalidAt, 5*time.Second)
+	})
+
 	t.Run("should set default previous_secret_invalid_at when previous_secret is provided", func(t *testing.T) {
 		t.Parallel()
 		originalDestination := testutil.DestinationFactory.Any(

internal/destregistry/registry.go

@@ -19,6 +19,19 @@ import (
 // PreprocessDestinationOpts contains options for preprocessing a destination
 type PreprocessDestinationOpts struct {
 	Role string
+	// Request holds the destination fields exactly as the caller sent them in
+	// the API request. On updates, newDestination carries the result of
+	// merge-patching the request into the stored values, so it cannot answer
+	// "did the caller provide this field" — the request can.
+	Request PreprocessRequest
+}
+
+// PreprocessRequest is the caller's view of the provider-owned destination
+// fields, before any merging with stored state. Maps are nil when the request
+// did not contain the corresponding field.
+type PreprocessRequest struct {
+	Config      map[string]string
+	Credentials map[string]string
 }
 
 // Registry manages providers, their metadata, and publishers