Skip to content

fix(webhook): support Svix signatures#20518

Closed
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/webhook-svix-signature-20478
Closed

fix(webhook): support Svix signatures#20518
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/webhook-svix-signature-20478

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Summary

  • add Svix-compatible svix-id / svix-timestamp / svix-signature verification to the webhook adapter
  • support whsec_ signing secrets and multiple space-delimited v1,<base64> signatures
  • cover valid, invalid, multi-signature, and partial-header rejection cases

Closes #20478

Tests

  • scripts/run_tests.sh tests/gateway/test_webhook_adapter.py -k 'svix or ValidateSignature'\n- scripts/run_tests.sh tests/gateway/test_webhook_adapter.py\n- git diff --check

@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server P3 Low — cosmetic, nice to have labels May 6, 2026
@twidtwid

Copy link
Copy Markdown

Thanks for picking this up. I ran into this with AgentMail webhooks as well, and the core HMAC calculation here matches the Svix/AgentMail format.

A couple of things look worth tightening before merge:

  1. Replay protection: Svix verification normally rejects timestamps more than five minutes away from the receiver's clock. This implementation verifies svix-timestamp as signed content, but it does not enforce a freshness window, so a captured valid webhook could be replayed indefinitely within Hermes' own one-hour idempotency TTL, and again after that TTL expires. AgentMail's verification docs call out the five-minute default tolerance: https://docs.agentmail.to/webhook-verification

  2. Idempotency for retries: _handle_webhook() still builds delivery_id from X-GitHub-Delivery, then X-Request-ID, then a timestamp fallback. AgentMail/Svix retries preserve svix-id; if Hermes does not use that as the delivery id, retries can trigger duplicate agent runs. I think this fallback order should include svix-id/Webhook-Id, e.g. X-GitHub-Deliverysvix-idWebhook-IdX-Request-ID → timestamp.

  3. Docs: Since the motivating issue is AgentMail, it would help to add a short docs note/example under the webhook guide explaining that Svix-compatible providers such as AgentMail use svix-id, svix-timestamp, svix-signature, and the route secret should be the provider's whsec_... signing secret.

The implementation otherwise looks like it should unblock AgentMail's signature format without adding a new runtime dependency.

@teknium1

Copy link
Copy Markdown
Contributor

This looks implemented on current main by the merged Svix/AgentMail webhook work in #30200, so I'm marking this overlapping PR as stale to close. This is an automated hermes-sweeper review.

Evidence:

  • fix(gateway): validate Svix webhook signatures #30200 was merged as bbf02c322443b7e833d730bbfa1dbd5d246e71e2 and explicitly closed the same motivating issue, [Feature]: Support for more webhook signatures #20478.
  • gateway/platforms/webhook.py:654 now recognizes svix-id, svix-timestamp, and svix-signature and dispatches to Svix validation.
  • gateway/platforms/webhook.py:699 implements Svix validation with required headers, a 300-second replay window, whsec_ decoding, id.timestamp.body signed content, and multiple v1,<base64> signatures.
  • gateway/platforms/webhook.py:496 includes svix-id in the delivery-id fallback for retry deduplication.
  • tests/gateway/test_webhook_adapter.py:186 and tests/gateway/test_webhook_adapter.py:614 cover the Svix signature and svix-id retry behavior.

Thanks for picking this up originally; the merged path appears to have incorporated the useful review feedback from the discussion here.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/webhook Webhook / API server sweeper:implemented-on-main Sweeper: behavior already present on current main type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Support for more webhook signatures

4 participants