Feat(webhook): route-configurable signature and event type headers - #68791
Feat(webhook): route-configurable signature and event type headers#68791Naroh091 wants to merge 5 commits into
Conversation
Providers like Gitea (X-Gitea-Signature) and Asana (X-Hook-Signature) use the standard HMAC-SHA256-over-body scheme but under their own header name, so their deliveries failed validation unless the adapter hardcoded each one (see NousResearch#66895, NousResearch#54697). Instead of growing a per-provider list, routes can now declare the header to validate: - signature_header: header name carrying the signature/token - signature_scheme: hmac-sha256 (default) or token (plain constant-time compare, GitLab-style) - signature_prefix: optional prefix (e.g. "sha256=") required and stripped before comparison When signature_header is set it is exclusive and fail-closed: built-in GitHub/GitLab/Svix/generic detection is skipped and requests missing the header are rejected, so a route pinned to one provider cannot be authenticated through a different scheme. Unknown schemes reject rather than fall back; connect() validates the config at startup so typos fail early with an actionable error. The CLI gains matching support: hermes webhook subscribe accepts --signature-header / --signature-scheme / --signature-prefix, list shows the pinned header, and hermes webhook test signs its test POST with the route's configured header. Docs updated (route properties table + "Custom signature headers" section) and 13 tests added covering validation, exclusivity, prefix handling, token scheme, fail-closed unknown schemes, hostile non-ASCII values, and startup validation errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Some providers still sign webhooks with a weaker HMAC digest and offer nothing stronger. Extend signature_scheme with hmac-sha1 and hmac-md5 (hex digest of the raw body, same as hmac-sha256). HMAC remains a sound authenticator with these digests — collision attacks on the bare hash do not transfer to HMAC — but hmac-sha256 stays the default and the docs steer users toward it whenever the provider supports it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The closing tip always said "HMAC-SHA256" even when the subscription was just created with --signature-scheme hmac-md5 or token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Providers with their own event header (e.g. X-Gitea-Event) previously
resolved every delivery to the payload fields or "unknown", making the
route-level `events` filter useless for them. A route can now declare
`event_header`; it is consulted before the built-in X-GitHub-Event /
X-GitLab-Event headers and payload fallbacks, and the resolved value
drives `events` filtering, `filters` on event, and the {event_type}
template token the agent receives.
`hermes webhook subscribe` gains --event-header, and `hermes webhook
test` sends the route's configured event header in its test POST.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Reviewed against #68768 with a focus on the signature-validation security boundary — this is a clean, well-tested implementation that satisfies the issue's fail-closed / exclusive requirements. Ran the suites locally: Security properties I verified in
This is exactly the config-driven approach the issue asked for, and it makes the per-provider PRs (#66895 Gitea, #54697 Asana) one line of config each. One minor, non-blocking note: the HMAC path compares LGTM — thorough tests on the exact security-critical paths, fail-closed by construction, and docs updated. |
|
Perfect! This would solve ClickUp too. ClickUp webhooks send HMAC-SHA256 (raw hex of the body) in the X-Signature header. With this PR's signature_header: "X-Signature" + signature_scheme: hmac-sha256, ClickUp would work out of the box. Would love to see this merged ASAP :) |
|
Thanks for taking the route-configured approach rather than adding provider-specific branches. The premise remains valid on current main: Problems
Suggested changes
This is an automated hermes-sweeper review. |
|
@teknium1 I've added the CLI tests. |
SummaryOne PR addresses #68768. #68791 implements provider-independent, route-configurable signature and event headers across gateway validation, dynamic-subscription CLI behavior, tests, and documentation, directly replacing the fixed-header limitation described by the issue. Related pull requests
Suggested consolidationKeep #68791 open with a salvage path: retain its provider-independent route configuration, fail-closed validation, event resolution, CLI integration, documentation, and regression tests, and request maintainer re-review now that the automated keep_open review’s concrete CLI-test gap is addressed by the current diff. There are no competing PRs in this complex to close as duplicates. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I68768(["issue #68768 (open)"])
P68791["PR #68791 (open)"]
P68791 -->|best fix| I68768
class I68768 open
class P68791 open
class P68791 best
class P68791 target
click I68768 "https://github.com/NousResearch/hermes-agent/issues/68768"
click P68791 "https://github.com/NousResearch/hermes-agent/pull/68791"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 41 kB of PR diffs, 10 kB of issue/PR text, 3 kB of discussion (3 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
Adds route-level options to the webhook adapter so any provider's headers can be handled through configuration instead of code, making Hermes a truly configurable webhook receiver:
Why
In my case, I was trying to integrate Patreon, which uses
X-Patreon-EventandX-Patreon-Signature, so the previous PRs wouldn't help as they are platform specific.Related Issue
Fixes #68768
Type of Change
Changes Made
Providers that use the standard HMAC-over-body scheme under their own header name (Gitea's
X-Gitea-Signature, Asana'sX-Hook-Signature, …) currently fail validation, and their event headers aren't recognized, making the route-leveleventsfilter unusable. Instead of hardcoding providers one at a time (#66895, #54697 — this PR supersedes both), the header names become route configuration:gateway/platforms/webhook.py— new per-route options:signature_header: name of the header carrying the signature/token. Exclusive and fail-closed: when set, built-in GitHub/GitLab/Svix/generic detection is skipped and requests missing the header are rejected with401, so a route pinned to one provider can't be authenticated through a different (possibly weaker) scheme.signature_scheme:hmac-sha256(default, hex HMAC digest of the raw body),hmac-sha1/hmac-md5(same, for providers that offer nothing stronger — HMAC remains a sound authenticator with these digests, but docs steer users to sha256), ortoken(plain constant-time compare against the secret, GitLab-style). Unknown schemes reject rather than fall back.signature_prefix: optional prefix (e.g.sha256=) required and stripped before comparison.event_header: consulted before the built-inX-GitHub-Event/X-GitLab-Eventheaders and payload fallbacks; driveseventsfiltering,filtersonevent, and the{event_type}template token. Falls back to built-in resolution when absent, so mixed senders keep working.connect()validates the config at startup (typo'd scheme, or scheme/prefix withoutsignature_header) matching the existing fail-early pattern; comparisons use the existing hardened constant-time helper.hermes_cli/subcommands/webhook.py,hermes_cli/webhook.py—hermes webhook subscribeaccepts--signature-header/--signature-scheme/--signature-prefix/--event-header;listshows the pinned headers;testsigns its POST with the route's configured signature header and sends the configured event header;subscribeoutput reflects the actual scheme instead of always saying HMAC-SHA256.website/docs/user-guide/messaging/webhooks.md— new route properties rows + "Custom signature headers" section with examples and the replay-protection caveat (body-only HMAC, same as generic V1; generic V2 recommended when you control the sender).tests/gateway/test_webhook_adapter.py— 20 new tests: valid/invalid custom-header HMAC across sha256/sha1/md5, exclusivity, digest cross-acceptance rejected, prefix required and stripped,tokenscheme, unknown scheme fails closed, case-insensitive lookup, hostile non-ASCII values, both startupValueErrors, and event-header resolution (match/ignore/priority/fallback).Out of scope: composite schemes that bind a timestamp (Stripe/Svix-style) still need native support, as Svix already has.
How to Test
pytest tests/gateway/test_webhook_adapter.py— 115 tests pass (20 new); all 5 webhook suites pass (149 total).hermes webhook subscribe demo --signature-header X-Provider-Signature --signature-scheme hmac-md5 --event-header X-Provider-Eventthen
hermes webhook test demowith the gateway running →202 accepted, with the event type resolved from the custom header.X-Hub-Signature-256) → all401.signature_scheme: hmac-sha512(or a scheme/prefix withoutsignature_header) on a static route → gateway refuses to start with an actionable error.401.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings)cli-config.yaml.exampleif I added/changed config keysCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Test webhook from Patreon showing a 202:
Result: