Skip to content

Rotated webhook secret is not used while the destination's publisher is cached #1084

Description

@alexluong

What happens

After rotating a webhook destination's secret (PATCH /destinations/:id with credentials.rotate_secret: true), deliveries keep being signed with the pre-rotation secret only. This lasts as long as the destination receives traffic; the new secret (and previous_secret) only take effect once the destination has been idle for 60s.

Reproduction

  1. Create a webhook destination with secret A.
  2. Publish an event every 10s; verify signatures against A.
  3. PATCH the destination with credentials.rotate_secret: true. Response shows secret: B, previous_secret: A.
  4. Keep publishing. Signature header still verifies only against A, never B.
  5. Stop publishing for 60s, publish again. Signature now verifies against B (and A until previous_secret_invalid_at).

Cause

internal/destregistry/registry.go:

  • MakePublisherKey hashes dest.ID, dest.Type and dest.Config, but not dest.Credentials.
  • ResolvePublisher caches the publisher in an LRU (internal/lru) with a sliding TTL (defaultPublisherTTL = 1 minute; Get refreshes expiresAt).

internal/destregistry/providers/destwebhook/destwebhook.go CreatePublisher builds the signing secrets list from creds.Secret / creds.PreviousSecret at construction time, so a cached publisher never sees the rotated credentials.

Same shape for other providers whose credentials are captured at construction (AWS Kinesis/SQS/S3 static credentials, and any other provider with a credential-bearing client): a credentials-only PATCH is ignored while the publisher is cached.

Suggested fix

Include dest.Credentials in MakePublisherKey. Simplest and covers every provider; cost is a new publisher per credential change, which is rare.

Alternative is evicting on destination update, but the API and delivery services are separate processes and nothing in the update path (internal/apirouter/destination_handlers.go) currently reaches the registry cache, so that needs a cross-process signal.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions