Skip to content

fix(webhook): accept Pocket (heypocketai.com) X-HeyPocket-Signature - #92972

Open
menhguin wants to merge 1 commit into
NousResearch:mainfrom
menhguin:fix/webhook-heypocket-signature
Open

menhguin wants to merge 1 commit into
NousResearch:mainfrom
menhguin:fix/webhook-heypocket-signature

Conversation

@menhguin

Copy link
Copy Markdown
Contributor

Problem

The Hermes webhook adapter's _validate_signature() has no branch for Pocket's X-HeyPocket-Signature header, so Pocket webhook deliveries cannot be authenticated via HMAC. Pocket signs the raw request body with HMAC-SHA256 of {timestamp}.{body} using a per-webhook signing secret and sends the result in X-HeyPocket-Signature, with the millisecond-resolution timestamp in X-HeyPocket-Timestamp.

Because the adapter's _validate_signature() has no branch for that header name, any Pocket route with a real secret configured is rejected as "no recognized signature header" — forcing users to run Pocket webhooks with INSECURE_NO_AUTH (unauthenticated) or not use Pocket webhooks at all.

The algorithm Pocket uses is already implemented in the adapter (generic HMAC-SHA256, hex-encoded); the gap is the missing header-name branch plus the ms-resolution timestamp (the existing generic V2 scheme uses second-resolution).

Fix

Adds a Pocket branch between the GitHub and GitLab branches in _validate_signature():

  • Reads X-HeyPocket-Signature + X-HeyPocket-Timestamp
  • Enforces a 300s replay window on the millisecond timestamp (same window as generic V2)
  • Rejects on missing/malformed/stale timestamp rather than falling through to a less-protected scheme (mirrors the V2 downgrade-guard reasoning)
  • Compares with _hmac_str_equal for timing-safe comparison

Testing

  • All 95 existing webhook adapter tests pass (pytest tests/gateway/test_webhook_adapter.py)
  • Verified live end-to-end with a real Pocket account: signed POST → 202, unsigned/wrong-signature → 401, stale timestamp → 401

References

The Hermes webhook adapter's _validate_signature() has no branch for
Pocket's X-HeyPocket-Signature header, so Pocket webhook deliveries
cannot be authenticated via HMAC. Pocket signs the raw request body
with HMAC-SHA256 of "{timestamp}.{body}" using a per-webhook signing
secret and sends the result in X-HeyPocket-Signature, with the
millisecond-resolution timestamp in X-HeyPocket-Timestamp.

Because the adapter's _validate_signature() has no branch for that
header name, any Pocket route with a real secret configured is rejected
as "no recognized signature header" — forcing users to run Pocket
webhooks with INSECURE_NO_AUTH (unauthenticated) or not use Pocket
webhooks at all.

The algorithm Pocket uses is already implemented in the adapter
(generic HMAC-SHA256, hex-encoded); the gap is the missing header-name
branch plus the ms-resolution timestamp (the existing generic V2
scheme uses second-resolution).

Adds a Pocket branch between the GitHub and GitLab branches:
- Reads X-HeyPocket-Signature + X-HeyPocket-Timestamp
- Enforces a 300s replay window on the millisecond timestamp
  (same window as generic V2)
- Rejects on missing/malformed/stale timestamp rather than falling
  through to a less-protected scheme (mirrors the V2 downgrade guard)
- Compares with _hmac_str_equal for timing-safe comparison

Docs: https://docs.heypocketai.com/docs/api/webhooks

Same pattern as the Linear branch added in NousResearch#87348.

Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 23, 2026
@Enough1122

Enough1122 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Overall: clean, correctly-scoped addition following the existing per-provider dispatch pattern — constant-time compare via _hmac_str_equal, ms replay window matching the generic V2 scheme, and clear failure paths. Points:

  1. No testsgateway/platforms/webhook.py:994-1018: this is the only provider branch added without a corresponding case in the webhook test suite. At minimum cover: (a) valid signature over {timestamp}.{body} accepted, (b) stale/future timestamp beyond 300s rejected, (c) missing X-HeyPocket-Timestamp rejected even with a signature present, (d) tampered body rejected. The signed-content construction (hp_timestamp.encode() + b"." + body) is exactly the kind of detail a regression test should pin against the documented scheme.

  2. Signature prefix tolerancegateway/platforms/webhook.py:1016-1017: the comparison assumes a bare hex digest. If Pocket ever emits sha256=<hex> styling (common in this family of schemes, cf. GitHub), every request fails closed with no diagnostic. Cheap insurance: strip an optional sha256=/v1= prefix before comparing, or log at debug when the shape looks prefixed.

  3. Minor — the replay-window warning logs route_name from match_info; confirm that key exists on all routes that reach this branch, otherwise the log line renders empty for direct-dispatch paths.

The fail-closed behavior on absent/unparseable timestamps is right, and placing the branch before the plain-token GitLab check avoids ambiguity since the header names are provider-specific.

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

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/webhook Webhook / API server sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants