fix(webhook): validate signature templates at startup; build formatters once - #1025
Open
alexluong wants to merge 4 commits into
Open
fix(webhook): validate signature templates at startup; build formatters once#1025alexluong wants to merge 4 commits into
alexluong wants to merge 4 commits into
Conversation
…instead of panicking An invalid signature template passed startup and crashed the delivery worker on the first event, then again on every restart since the event stayed queued. Config validation now builds both formatters and renders each against a synthetic payload, so a template that doesn't parse — or that references a field of the other payload type, which parsing can't catch — fails startup naming the offending template. The dry run is a smoke test, not a proof: value-dependent failures still reach delivery. So the formatters return execution errors instead of panicking, and Publish carries them through the existing NewFormatError path to a failed attempt. Closes #1020 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CreatePublisher built both formatters per destination, so every cached publisher retained its own copy of two sprig function maps — ~44 KB each, ~436 MB at the 10k publisher cache cap. Both templates come from provider config and a parsed template is safe for parallel execution, so they're built in New() and shared. Encoder and algorithm hoist the same way. destwebhookstandard gets the same treatment, with its spec-fixed templates lifted to constants. Closes #1022 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alexluong
changed the title
…yer check
Validation moves to where the templates are used: New dry-runs the
formatters it just built against synthetic payloads, so both a template
that doesn't parse and one that borrows a field from the other payload
type fail provider registration — with no second parse, and no way to
construct an unvalidated provider outside config.Parse.
The config-layer check (validateDestinations) is dropped as redundant.
Only visible consequence: a standalone log service, which never builds
the destination registry, now boots with an invalid template; api and
delivery still fail startup at registration.
The publish-time degradation test keeps its purpose via a
value-dependent template ({{index .Signatures 1}}), which survives the
two-signature dry-run and fails against the single configured secret.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A white-box identity assertion pins that publishers share the provider's formatters, so a regression back to per-destination construction fails a test. The destination mock server gets the same hoist the providers got: formatters built once in NewMockStore instead of per verifySignature call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Validates the webhook signature templates at startup instead of on the first delivery. The provider's
New()parses both templates and dry-runs each against a synthetic payload, so a template that doesn't parse — or that references a field of the other payload type, which parsing can't catch — fails provider registration naming the offending template. Validation lives where the templates are used: there is no separate config-layer check, and no way to construct an unvalidated provider.A synthetic payload can't prove a template though (
{{index .Signatures 2}}is fine with three secrets, and a helper may error only on real input), so the formatters now return execution errors rather than panicking.Publishcarries those through the existingNewFormatErrorpath to a failed attempt, so what slips past validation costs a delivery instead of the process.Second commit fixes #1022 on the same construction path: the formatters are built once in the provider's
New()and shared, instead of per destination where each cached publisher retained its own copy of two sprig function maps (~44 KB per publisher, ~436 MB at the 10k cache cap). Encoder and algorithm hoist alongside them.Retry behaviour is unchanged: format errors are still retryable, so a deterministic template failure burns the whole retry budget. Pre-existing, flagged in #1020 as a separate call.
Behavior change: a deployment with an invalid
DESTINATIONS_WEBHOOK_SIGNATURE_*_TEMPLATEthat previously started (and panicked on the first webhook delivery) now fails to start with an error naming the template. This applies to the api and delivery services, which register the webhook provider; a standalone log service doesn't and is unaffected.Closes #1020
Closes #1022
🤖 Generated with Claude Code