fix(webhooks): unify signing scheme with timestamp binding and replay-window enforcement

[Cyber-Mitch]

closes #97

Two divergent signing schemes existed for the same conceptual operation. webhookSignature.js (used by webhookDispatcher.js for every pool.* delivery) computed a pure HMAC over the body with no timestamp — so any captured (body, signature) pair was replayable forever. webhook.js (used for price.alert deliveries) did bind a timestamp, but never enforced a max-age, so replay-prevention was merely representable, not enforced.

This unifies both onto one hardened implementation with a timestamp bound into the HMAC input and an enforced replay window.

⚠️ Breaking change — hard cutover, and why

Before: HMAC-SHA256(secret, body) → X-SmartDrop-Signature
After: HMAC-SHA256(secret, "${timestamp}.${body}") → X-SmartDrop-Signature, X-SmartDrop-Timestamp, X-SmartDrop-Signature-Version: 2

The issue raises dual-signature transition vs. hard cutover as a product decision. Hard cutover — because a transition period wouldn't actually protect anyone: the README's own reference verifier (README.md:818-830, the code subscribers would naturally copy) HMACs req.rawBody alone with no version branching. Emitting a v1 signature alongside v2 wouldn't help a correctly-implemented subscriber, because their verification logic never looks at the version header to decide which scheme to apply. Every subscriber fails closed simultaneously either way. A transition period would add two live signing paths and two test surfaces to maintain, for zero real compatibility benefit.

Happy to reverse this if you'd prefer the transition anyway — it's your call, and it's a contained change.

The retry-interaction fix (the non-obvious part)

The issue flags this precisely: webhookDispatcher.js's exponential backoff can push a retry well past a 300s max-age window. If the timestamp were computed once in dispatch(), a legitimately-late retry would sign with an already-stale timestamp and immediately fail its own freshness check on arrival — this fix would have broken the existing, working retry mechanism for any subscriber implementing the newly-recommended max-age check.

Date.now() is therefore computed at the top of attempt(), fresh for every individual delivery attempt. Two tests pin this: a retry pushed past the window produces a currently-valid signature (not a stale one), and two targets in a single dispatch() each receive their own timestamp.

Blueprint drift found during recon

The issue's line references have drifted since filing — three substantive corrections:

1. dispatch() doesn't use Promise.all(targets.map(...)) — it's a batched concurrency loop (DISPATCH_CONCURRENCY, default 10) delegating to processBatch(), either sequential (ORDERED_DELIVERY) or Promise.allSettled. The structural conclusion holds — the fix location is unchanged — but the described shape isn't current.
2. backoffMs is no longer bare exponential — webhookDispatcher.backoffMs() has no jitter — synchronized retries thundering-herd a subscriber's endpoint right as it recovers #128 added equal jitter, so actual delay is [det/2, det), not the deterministic base * factor^(n-1) the issue describes. The retry-freshness test accounts for the jitter range rather than assuming the deterministic figure, which would have made it flaky.
3. buildHeaders takes 5 params and emits 5–6 headers, not 3 — X-Request-Id was added by No request ID tracking across services #250. The issue's "old 3-header assumption" maps to webhook.js:19-26, not the dispatcher.

Design decisions

• Canonical module: webhookSignature.js. It backs the published contract (the dispatcher path is what the README documents), it's dependency-pure (imports only crypto, versus webhook.js which mixes in axios/logger/delivery), it already owns generateSecret()/SIGNATURE_PREFIX, and webhook.js's signPayload/verifySignature have zero runtime callers (verified by grep — test-only), so removing them carries no blast radius.
• signatureHeaders() resolves the timestamp once and uses that same value for both the HMAC and the X-SmartDrop-Timestamp header — making a header/signature mismatch structurally impossible rather than merely tested-against. Both dispatcher and alerts paths spread its output, so there's exactly one place a timestamp gets resolved.
• Strict timestamp parsing (/^\d+$/ + Number.isSafeInteger) rather than bare Number(), which would coerce '', [], true, and null into valid-looking numbers (Number('') === 0, Number([]) === 0).
• Symmetric skew check — Math.abs(now - ts) > maxAge * 1000, rejecting future-dated timestamps too, closing the direction gap the issue notes webhook.js never addressed.

Tests

15 net-new passing tests. Notable ones beyond the basics:

• Boundary: exactly 300s → valid; 300s + 1ms → rejected. Catches an off-by-one that would silently widen or narrow the replay window.
• Timestamp-altered-after-signing → rejected, proving the timestamp is covered by the HMAC, not merely transported alongside it.
• README-snippet self-test — the exact verification code from the README, run against real sign() output, so the documented snippet is proven correct rather than illustrative pseudocode that quietly doesn't work.
• Cross-module agreement — webhook.buildSignatureHeaders (alerts path) verifies under webhookSignature.verify, proving unification; would fail if a divergent scheme survived anywhere.
• Malformed-timestamp matrix — '', 'abc', null, undefined, '-1', '12.5', '1e3', [], true all rejected without throwing.
• Stale assertions updated, not left passing: webhookDispatcher.test.js:59 (old 2-arg sign) and webhookSignature.test.js:64-67 (a hardcoded 2026-06-25 timestamp that must now fail, and does).

Verification

Ran the suite on stashed baseline vs. this branch:

	Baseline	This branch
Suites failed	5	5
Tests failed	21	21
Tests passed	627	642

Identical failure count — zero regressions. The 21 failures are pre-existing on main and unrelated to this work (see below). Note: this repo has no lint or build script; npm test is the only gate.

Found, not fixed — pre-existing failures on main

Confirmed present on an unmodified checkout at db45c1c, none introduced here:

1. test/timeout.test.js — real bug: TIMEOUT isn't in the ERROR_CODES registry (src/errors/AppError.js:20-41), so new AppError('TIMEOUT', …) at src/middleware/timeout.js:14 throws, yielding 500 instead of 504.
2. test/cors.test.js — test-side: the inline handler reads err.status, but AppError sets err.statusCode, falling through to 500.
3. test/health.test.js — test-side: the cache mock omits getCommandQueueLength, which src/index.js:100 calls (the real cache.js:57 does export it).
4. test/priceWebSocket.test.js — WS message timeouts plus an afterAll done() hang.
5. test/auth.test.js / test/webhooks.routes.test.js — pass in isolation (41/41), fail only under parallel load.

Items 1–3 look like merge artifacts from the recently-landed error-code/observability work and are each a small, self-contained fix — happy to open separate issues if useful.

Cross-references

No collision with #96 (SSRF guard lives in validation/schemas.js) or #90 (rate limiter in middleware/rateLimit.js) — both landed, neither touches the signing path. #89's constantTimeEquals consolidation would fit naturally in this now-canonical signing module as a follow-up; not built here to keep this PR scoped.

[Cyber-Mitch]

@ritaifeoluwa @prodbycorne please review