Skip to content

fix(webhook): reject generic V2 signature missing timestamp instead o… - #58740

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
MorAlekss:fix/webhook-generic-hmac-replay-protection
Jul 5, 2026
Merged

fix(webhook): reject generic V2 signature missing timestamp instead o…#58740
teknium1 merged 1 commit into
NousResearch:mainfrom
MorAlekss:fix/webhook-generic-hmac-replay-protection

Conversation

@MorAlekss

@MorAlekss MorAlekss commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up on #58461, which added a timestamp-bound V2 signature scheme
(X-Webhook-Signature-V2 + X-Webhook-Timestamp) for the generic
webhook route to fix a replay vulnerability in the legacy body-only V1
scheme. While #58461 was open, @teknium1 opened #58508 as a salvage of
it and merged that instead. A downgrade issue in the V2 logic was found
in review and fixed with a follow-up commit on the #58461 branch, but
that commit landed after the salvage snapshot was taken, so it did not
make it into the merged #58508. This PR carries just that fix, rebased
onto current main.


Root cause

The V2 check in _validate_signature() was if v2_sig and v2_timestamp:.
A sender migrating from V1 to V2 typically sends both signatures
together during the transition. If a captured mixed request has its
X-Webhook-Timestamp header stripped before replay, the condition
becomes false and validation falls through to the legacy V1 check,
which still validates against the unprotected, non-expiring V1
signature present in the same captured request — silently downgrading
a V2-protected request back into the exact replay hole V2 exists to
close.


Behavioral change

Before: a captured mixed V1+V2 request with the timestamp header
removed validated successfully via the fallback to V1.

After: the presence of X-Webhook-Signature-V2 alone commits to V2
validation. A missing, malformed, or expired timestamp now rejects the
request instead of falling back to V1.


What changed

gateway/platforms/webhook.py: changed the V2 guard from
if v2_sig and v2_timestamp: to if v2_sig:, with an explicit reject
(and warning) when the timestamp is absent, before the legacy V1
branch.

tests/gateway/test_webhook_adapter.py: added a regression test
reproducing the exact stripped-timestamp downgrade scenario.


What is NOT changed

Infographic

PR #58740 infographic

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists needs-repro Bug needs reproduction steps sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 5, 2026
@teknium1
teknium1 merged commit d577408 into NousResearch:main Jul 5, 2026
31 checks passed
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 needs-repro Bug needs reproduction steps P2 Medium — degraded but workaround exists platform/webhook Webhook / API server sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants