Skip to content

feat(webhook): support custom signature header name#992

Open
ambroziepaval wants to merge 1 commit into
hookdeck:mainfrom
ambroziepaval:feat/webhook-signature-header-name
Open

feat(webhook): support custom signature header name#992
ambroziepaval wants to merge 1 commit into
hookdeck:mainfrom
ambroziepaval:feat/webhook-signature-header-name

Conversation

@ambroziepaval

Copy link
Copy Markdown

Add the global DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME config option. When set, its exact value becomes the signature header name, bypassing the "signature" construction; when unset, behavior is unchanged, so the change is fully backwards compatible.

Scoped to default mode only: standard-webhooks mode fixes the header as "webhook-signature" per spec, and renaming it would break spec-compliant verifiers. This mirrors every other signature-customization field and the existing SignatureHeaderTemplate plumbing. The name is validated at construction against the header-name regex and reserved-header set.

Issue #982.

Add the global DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME config option.
When set, its exact value becomes the signature header name, bypassing the
"<prefix>signature" construction; when unset, behavior is unchanged, so the
change is fully backwards compatible.

Scoped to default mode only: standard-webhooks mode fixes the header as
"webhook-signature" per spec, and renaming it would break spec-compliant
verifiers. This mirrors every other signature-customization field and the
existing SignatureHeaderTemplate plumbing. The name is validated at
construction against the header-name regex and reserved-header set.

Tracks hookdeck#982.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alexbouchardd

Copy link
Copy Markdown
Contributor

As mentioned in #982 (comment), I do not believe that's the right approach, since we now have 2 configs that implement essentially the same thing. The approach should also be header-agnostic and cover every header value.

We also need to account for backward compatibility of existing deployments, possibly by keeping the DISABLE_* config but marking it as deprecated and printing a warning.

@ambroziepaval

Copy link
Copy Markdown
Author

Thanks @alexbouchardd , this makes sense and I want to confirm I've got your point right. The signature header already has two configs that overlap: DISABLE_DEFAULT_SIGNATURE_HEADER (emit or not) and SIGNATURE_HEADER_TEMPLATE (the value). "Disable" and "empty value" are the same outcome, and my new SIGNATURE_HEADER_NAME piles a third knob onto the same header.

Concrete step I'd take now: keep the two configs that do distinct things: SIGNATURE_HEADER_NAME (the header name) and SIGNATURE_HEADER_TEMPLATE (the value), and treat an empty template (SIGNATURE_HEADER_TEMPLATE='') as "no signature emitted". So we go from three signature knobs down to two, with no overlap. DISABLE_DEFAULT_SIGNATURE_HEADER stays working but deprecated with a warning for existing deployments. That directly removes the config creep you flagged.

One scope question: Should this stay signature-only, or give the same treatment to event-id, timestamp, and topic too (deprecating their DISABLE_DEFAULT_*_HEADER flags the same way)?

@alexluong

Copy link
Copy Markdown
Collaborator

Hi @ambroziepaval, thanks for the PR and sorry for the miscommunication/confusion. It was caused by me misunderstanding what @alexbouchardd meant. We connected internally and agreed on the design here. Can you take a look and update the PR accordingly? We can chat more in the issue thread if there's anything you want to discuss further.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants