Skip to content

test(webhook-signature): add regression coverage for WEBHOOK_SIGNATURE_HEADER failure handling #1051 - #1170

Open
Chulexino wants to merge 1 commit into
RevoraOrg:masterfrom
Chulexino:fix/1051-webhook-signature-regression
Open

Chulexino wants to merge 1 commit into
RevoraOrg:masterfrom
Chulexino:fix/1051-webhook-signature-regression

Conversation

@Chulexino

Copy link
Copy Markdown

Overview

This PR adds a focused regression suite for the WEBHOOK_SIGNATURE_HEADER failure and empty-result paths in src/lib/webhookSignature.ts (issue #1051), so a silent behaviour change is caught in CI instead of reaching webhook receivers. Test-only change — the implementation file is byte-identical; the existing public contract is pinned, not modified.

Related Issue

Closes #1051

Changes

🧪 Webhook Signature Regression Suite

  • [ADD] src/lib/webhookSignature.regression.test.ts

    • Pins the empty-result exit at webhookSignature.ts:156 — extractSignatureFromHeaders returns undefined for absent, unset, empty-array, null, non-string and mixed-case header keys — plus the neighbouring normal path (candidate priority order, every supported header name, empty string returned verbatim).
    • Pins the parseExpiryTimestamp undefined branches at :214 (unset), :218 (invalid Date), :221 (NaN) and :234 (out-of-contract runtime types), together with the boundary rules: the 1e11 seconds/milliseconds cut-off (inclusive on the ms side), 0 kept distinct from "unset", and the anchored ^\d+$ numeric-string branch.
    • Pins the missing-signature failure contract: deterministic, non-leaking MISSING_SIGNATURE for empty/unset/empty-array/empty-string/custom-name-absent headers, ordered after the payload-size check and before the timestamp/replay check, and thrown by assertValidWebhookSignature.
    • Pins the verifyWebhookPayloadDualKey expiry coupling: unparseable / NaN nextSecretExpiry fails open (documented and deliberate), a passed deadline flips nextKeyExpired, Infinity never expires, and no nextSecret keeps the current key only.
    • Pins verifyWebhookPayload malformed signature-container boundaries (fail closed), including an equal-character-length multi-byte value that makes crypto.timingSafeEqual throw — the defensive catch absorbs it, so there is no unhandled RangeError.
    • Header-smuggling defence: only the first array entry is considered, so a later duplicate can never rescue an unusable first entry.
  • [ADD] docs/webhook-signature-header-regression.md

    • Security assumptions and abuse/failure paths, the deliberately pinned behaviours (fail-open expiry, Infinity, negative deadlines), the exercised cases with measured results, and residual risk for reviewers.
  • [MODIFY] package.json

    • Adds test:coverage:webhook-signature — the focused + surrounding suites with a --coverageThreshold of 95% on statements/lines/functions/branches for src/lib/webhookSignature.ts.

Verification Results

npx jest src/lib/webhookSignature.regression.test.ts
✅ 50/50 passed

npm run test:coverage:webhook-signature
✅ 190/190 passed (3 suites)
✅ src/lib/webhookSignature.ts — 100% statements, 97.29% branches, 100% functions, 100% lines (95% gate met, exit 0)

npx jest src/lib/webhookSignature.test.ts src/lib/webhookSignature.regression.test.ts \
  src/middleware/webhookAuth.test.ts src/services/webhookService.test.ts \
  src/services/__tests__/outboxDispatcher.test.ts src/services/__tests__/outboxHmacRotationService.test.ts --coverage=false
✅ 334/334 passed (6 suites)

npx eslint src/lib/webhookSignature.regression.test.ts
✅ 0 problems

npx tsc --noEmit
✅ Type-error count unchanged by this PR (250 before and after; none in the webhookSignature files)

npm run validate:alert-mappings
✅ OK: All 22 known alerts have mapping entries.
Acceptance Criteria Status
Cover the named behavior with focused automated tests, including the relevant success and failure paths ✅ 50 tests across the :156, :214, :218, :221, :234 evidence lines and their neighbouring normal paths
Preserve the existing public contract unless the change includes an explicit compatibility plan ✅ Test-only — src/lib/webhookSignature.ts is untouched (+686/-0 across 3 files)
Make error and boundary behavior observable and deterministic ✅ Exact MISSING_SIGNATURE codes/messages, fixed failure ordering, explicit boundary tables (1e11, 0 vs unset, Infinity)
Run the focused test file and the surrounding suite ✅ 50/50 focused, 334/334 surrounding (6 suites)
Run the repository's configured lint, type, build, or contract checks ✅ eslint clean on the new file, tsc count unchanged, alert mappings OK — the pre-existing repo-wide build/lint/audit failures are documented in docs/webhook-signature-header-regression.md §5
Include the exercised cases and results in the pull request description ✅ This section, plus the per-evidence-line results table in the docs

Security notes

  • A stripped signature header never means "skip verification" — MISSING_SIGNATURE is returned, never valid: true, and the failure payload carries no key material (asserted).
  • An oversized body is rejected before signature handling, and a missing timestamp cannot be used to dodge replay protection because the signature check still runs first.
  • Mis-wired callers that build header maps with mixed-case keys fail closed and deterministically.
  • Pinned, deliberately unchanged (follow-ups in the docs): an unparseable nextSecretExpiry fails open, Infinity means never-expiring, and negative numeric deadlines take the seconds branch.

@drips-wave

drips-wave Bot commented Sep 27, 2026

Copy link
Copy Markdown

@Chulexino Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add regression coverage for WEBHOOK_SIGNATURE_HEADER failure handling

1 participant