Skip to content

feat: implement memory-bounded bucket eviction for rate limiter and a… - #232

Merged
Jagadeeshftw merged 1 commit into
AnchorNet-Org:mainfrom
GiftedGiftB:The-rate-limiter-keeps-buckets-in-a-process-local
Aug 29, 2026
Merged

feat: implement memory-bounded bucket eviction for rate limiter and a…#232
Jagadeeshftw merged 1 commit into
AnchorNet-Org:mainfrom
GiftedGiftB:The-rate-limiter-keeps-buckets-in-a-process-local

Conversation

@GiftedGiftB

Copy link
Copy Markdown
Contributor

Closed #223

Description

This PR addresses the high-priority vulnerability regarding the rateLimiter middleware, specifically mitigating the unbounded memory growth (memory-pressure vector) while formalizing the operational behavior for multi-instance deployments.

1. Store Decision (Explicit Deferral)

After evaluation, I have deliberately deferred the introduction of a shared distributed store (like Redis) for rate limiting. This service currently has no external storage dependencies and no persistence layer. Introducing one strictly for rate limiting would prematurely bloat the operational footprint of the service. This explicit deferral is now documented in README.md, and it will be revisited when the broader persistence layer issue is resolved.

2. Fail-Open / Fail-Closed Reasoning

Because the shared store decision has been deferred and rate limiting remains completely in-memory, network failure policies regarding a cache store are not applicable at this stage. The limiter operates entirely locally within the Node.js process and does not suffer from external store outages.

3. Read-Path Conclusion

We are continuing to leave standard GET reads unlimited. The only computationally expensive "read" operation (POST /api/v1/quote) is already independently bounded by its own stricter rate limiter instance in src/app.ts. Standard stateless GET requests are extremely fast and do not mutate state, so leaving them unlimited is acceptable for now.

4. Memory Bounds Implementation (Fixing Unbounded Growth)

To defend against the memory-pressure attack vector, a hard capacity limit (MAX_BUCKETS = 5000) has been added to the local Map state. If an attacker cycles distinct IPs, the limiter will:

  • Lazily prune expired buckets when capacity is reached.
  • Safely evict the oldest entry if the capacity remains full.

Verification

The following tests were successfully added and pass with no external dependencies:

  • Bypass Test: Explicitly demonstrates that multi-instance deployments without a shared store grant clients a full quota per instance.
  • Bounded-Growth Test: Proves that the bucket state remains strictly bounded (capping at 5000) under a flood of distinct IP keys by evicting the oldest entries.
  • The standard npm run lint, npm run build, and npm test suites pass fully with 0 regressions, preserving default limits and windows.

@Jagadeeshftw
Jagadeeshftw merged commit 9e18fd2 into AnchorNet-Org:main Aug 29, 2026
Jagadeeshftw added a commit that referenced this pull request Aug 29, 2026
… (#231)

* feat(config): fail-fast validation + standalone typecheck script (#230)

* fix(#225): replace float arithmetic with bigint for exact monetary precision (#236)

* fix(#225): replace float arithmetic with bigint for exact monetary precision

* fix(#225): add all bigint migration files missing from previous commit

* feat(metrics): protect metrics reads and bound history [Issue #228] (#235)

Aggregate metrics (participant counts, liquidity totals, settlement
volume and fees over time) describe the network's operational state.
Exposing that publicly should be deliberate, not a side effect of the
write-only auth middleware, whose MUTATING_METHODS set left every GET
unauthenticated and unlimited.

- Auth: new metricsAuth guards GET /api/v1/metrics and /history. When
  API_KEY or the new read-only METRICS_API_KEY is set, reads require a
  matching x-api-key (401 otherwise); when neither is set they stay
  open, matching the existing write-auth model. METRICS_API_KEY unlocks
  metrics only, so a scraper never needs the write key.
- Rate limiting: opt-in limitReads flag on rateLimiter (default off, so
  global behaviour is unchanged) enabled only on the metrics mount via
  METRICS_RATE_LIMIT_MAX (default 120/min), so the history endpoint is
  not an unlimited load generator. Global read limiting and the shared
  store remain owned by the separate rate-limiter issue.
- Retention: history stays bounded at MAX_HISTORY = 50, now pinned by
  route-level tests (eviction of the oldest entry).
- openapi.ts declares an ApiKeyAuth scheme and marks both metrics
  operations as protected; README/CHANGELOG document the scraper path.

npm run lint, npm run build and npm test (43 suites, 509 tests) pass.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(idempotency): share a bounded in-process replay store (#234)

Close the per-middleware Map hole that let the same key execute twice
across mounts, add a hard entry cap with soonest-expiry eviction, and
coalesce concurrent same-key requests onto one in-flight handler.
Replay semantics stay response-body based; headers are never cached.
Cross-replica sharing waits on the separate persistence issue.

* fix: make audit log an explicit operator convenience buffer (#233)

* feat: implement memory-bounded bucket eviction for rate limiter and add operational documentation (#232)

* Add fail-fast config validation and standalone typecheck (#230) (#237)

- validateConfig() enforces required values before the server binds:
  API_KEY required in production, PORT valid 1-65535, non-negative
  rate-limit/idempotency values; warns loudly (non-prod) on open access.
- Wire validateConfig into createApp/getConfig in app.ts.
- Add typecheck script (tsc --noEmit) and a distinct CI Typecheck step.
- Extend config.test.ts with a validateConfig suite.

closes #230

Co-authored-by: ChainBid Developer <developer@chain-bid.io>

---------

Co-authored-by: Paranoa-dev <paranoa-dev@users.noreply.github.com>
Co-authored-by: Mauricio Gil | GramSeo Studio <gramseostudio@gmail.com>
Co-authored-by: jahswillb-dev <tech.jahswillb@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: kaleel <128490484+nyuiela@users.noreply.github.com>
Co-authored-by: martinshub-tech <jambemail2003@gmail.com>
Co-authored-by: Oyakhilome Gift A <asekhamegift@gmail.com>
Co-authored-by: ChainBid Developer <developer@chain-bid.io>
Co-authored-by: Jagadeeshftw <92681651+Jagadeeshftw@users.noreply.github.com>
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.

The rate limiter keeps buckets in a process-local Map, so limits multiply by replica count and reset on every deploy

2 participants