Skip to content

fix(rate-limit): rate-limit /internal/webhooks/replay and fix pre-existing keyGen bug - #150

Open
Whiznificent wants to merge 2 commits into
Vero-protocol:mainfrom
Whiznificent:fix/internal-replay-rate-limit
Open

fix(rate-limit): rate-limit /internal/webhooks/replay and fix pre-existing keyGen bug#150
Whiznificent wants to merge 2 commits into
Vero-protocol:mainfrom
Whiznificent:fix/internal-replay-rate-limit

Conversation

@Whiznificent

Copy link
Copy Markdown
Contributor

closes #130

Summary

  • Add ingestRateLimiter to the /internal/webhooks/replay middleware chain so a compromised or leaked internal JWT cannot replay-flood the queue.
  • Fix a pre-existing bug in src/middleware/rateLimit.js clientIp(req) that was silently disabling every rate limiter in the codebase (it was passing the request object to ipKeyGenerator, which caused the helper to return an object literal — and the default store keys off object references, so every request landed in its own bucket and hits never accumulated).
  • Add an end-to-end integration test that proves the new endpoint returns 429 once the configured limit is exceeded.

Problem

POST /internal/webhooks/replay requires a valid JWT (verifyJwtBearer) but unlike /github-webhook had no rate limiter in its middleware chain. A leaked or compromised internal token could replay-flood the queue/on-chain submission path with no throttle.

Changes

New files

  • test/rateLimit-internal-replay.test.js — integration test that pins RATE_LIMIT_AUTH_MAX=2 before any require, drives createApp with mocked fetchRawEvent / enqueueEventJob, sends three authenticated supertest requests from the same X-Forwarded-For IP, and asserts the first two are 202 and the third is 429 with code: "RATE_LIMIT_EXCEEDED". Restores env vars in after() so sibling test files are unaffected.

Modified files

  • index.js — append ingestRateLimiter before verifyJwtBearer on the /internal/webhooks/replay route so the limiter throttles even before JWT verification runs.
  • src/middleware/rateLimit.js — fix clientIp(req) to call ipKeyGenerator(req.ip) (the IP string), matching what express-rate-limit v8+ expects. The previous ipKeyGenerator(req) call returned an object like { ip: "..." }, which silently broke Map-backed hit tracking and made every rate limit a no-op. IPv6-subnet keying still works as documented by the upstream helper.

Verification

node --test test/rateLimit-internal-replay.test.js   # 1 pass
node --test test/rateLimit.test.js                  # 17 pass (was 16/17)

As a side effect, the previously-failing test ingestRateLimiter respects RATE_LIMIT_PUBLIC_MAX env when reloaded (can trigger 429) now passes because the underlying clientIp bug is fixed.

Security notes

  • The rate limit runs before JWT verification by design: the threat model is a valid-but-compromised token, where cheap rate limiting is the only line of defence before signature verification completes. Placing it after JWT verification would be strictly weaker.
  • Authenticated tier (1000 / 15 min, configurable via RATE_LIMIT_AUTH_MAX) applies because every request to this endpoint carries an Authorization: Bearer … header.

Notes

  • No changes to src/middleware/rateLimit.js API surface, configuration, or exports — purely a correctness fix to clientIp.
  • This PR does not bump the existing limits; defenders can tighten RATE_LIMIT_AUTH_MAX independently if desired.

Whiznificent and others added 2 commits July 27, 2026 16:10
…sting keyGen bug

Fixes Vero-protocol#130

Problem
  - POST /internal/webhooks/replay had JWT verification but no rate limit,
    so a compromised/leaked internal token could replay-flood the queue
    path unbounded.

Changes
  - index.js: place ingestRateLimiter before verifyJwtBearer on
    /internal/webhooks/replay so the limiter throttles even before JWT
    verification runs.
  - src/middleware/rateLimit.js: fix clientIp() to call
    ipKeyGenerator(req.ip) (string) rather than ipKeyGenerator(req)
    (request object). The previous version returned { ip: "..." }  -
    an object - which silently broke hit-tracking: express-rate-limit's
    default Map-backed store keys on object references, so every request was
    its own bucket and the rate limit never engaged. Verifying IPv6
    subnet-keying still works as documented by the upstream helper.
  - test/rateLimit-internal-replay.test.js: new integration test that
    pins RATE_LIMIT_AUTH_MAX=2 before any require, drives createApp with
    mocked fetchRawEvent/enqueueEventJob, sends three authenticated
    requests from the same X-Forwarded-For IP, asserts first two are
    202 and third is 429 with code RATE_LIMIT_EXCEEDED. Restores env vars
    in after() so sibling tests are unaffected.

Side effects
  - The pre-existing failing test "ingestRateLimiter respects
    RATE_LIMIT_PUBLIC_MAX env when reloaded (can trigger 429)" now passes
    too, because the underlying keyGenerator bug is fixed.

Verification
  - node --test test/rateLimit-internal-replay.test.js  -> 1 pass
  - node --test test/rateLimit.test.js                 -> 17 pass
  - node --test test/rateLimit.test.js test/rateLimit-internal-replay.test.js test/webhook.test.js
                                                      -> 22 pass, 1 unrelated failure
                                                       (github-webhook enqueue test was already
                                                        failing on this branch due to missing
                                                        REDIS_HOST infrastructure; unrelated)
Comment thread index.js
app.post('/internal/webhooks/replay', verifyJwtBearer, async (req, res) => {
// Internal replay endpoint — rate-limited before JWT verification so a
// compromised/leaked internal token cannot replay-flood the queue path.
app.post('/internal/webhooks/replay', ingestRateLimiter, verifyJwtBearer, async (req, res) => {
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.

fix: /internal/webhooks/replay has no rate limiting

3 participants