fix(rate-limit): extend Redis rate-limit guard to Horizon-facing endpoints - #256
Open
teslims2 wants to merge 2 commits into
Open
fix(rate-limit): extend Redis rate-limit guard to Horizon-facing endpoints#256teslims2 wants to merge 2 commits into
teslims2 wants to merge 2 commits into
Conversation
…oints Wraps outbound Stellar Horizon / anchor calls with the existing Redis sliding-window rate-limit infrastructure so a single caller can't exhaust Horizon's shared-IP rate limit for everyone. - Register RateLimitModule in AppModule so the RateLimitGuardRedis global APP_GUARD is actually active (it existed but was never wired up, so no @ratelimit()-decorated route, including the pre-existing transactions:send limit, was enforced). - Add a per-user @ratelimit() to GET /wallet/balances (Horizon balance fetch), configurable via WALLET_BALANCES_RATE_LIMIT_MAX / WALLET_BALANCES_RATE_LIMIT_WINDOW_MS. - Add a per-user @ratelimit() to GET /anchor/fx-rate, configurable via ANCHOR_FX_RATE_RATE_LIMIT_MAX / ANCHOR_FX_RATE_RATE_LIMIT_WINDOW_MS. - Add a per-IP @ratelimit() to the unauthenticated SEP-10 endpoints GET /anchor/auth/challenge and POST /anchor/auth/token, configurable via ANCHOR_AUTH_RATE_LIMIT_MAX / ANCHOR_AUTH_RATE_LIMIT_WINDOW_MS. - Document all new/existing rate-limit env vars in docs/environment-variables.md. - Extend rate-limit.guard.spec.ts with coverage for the new keyPrefixes: per-user vs per-IP keying, 429 + Retry-After on exceeding the limit, and env-var overrides. Also fixes a pre-existing gap in the spec's MockRedis double (missing quit()) that made the suite fail to tear down. Closes Afro-Pay#204 Note: WalletService is currently missing findByUserId, findByPublicKey, and getBalances (and wallet.controller.ts calls enableMultiSignature, which doesn't exist either), and AnchorModule / AnchorAuthController aren't registered in AppModule yet. These are pre-existing gaps from other in-flight work, out of scope here; the rate-limit decorators are added to the routes now so limiting is already in place once those are wired up.
|
@teslims2 is attempting to deploy a commit to the milah's projects Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #204.
AnchorService.getFxRate()and the wallet balance-fetch path proxy to Stellar Horizon / the anchor with no per-user or per-IP rate limiting at the API level, so a single caller could exhaust Horizon's shared-IP rate limit for everyone. This wraps those routes in the existing Redis sliding-window@RateLimit()guard (apps/api/src/rate-limit/rate-limit.guard.redis.ts), and — critically — registersRateLimitModuleinAppModule, since it was defined but never imported, so no@RateLimit()-decorated route (including the pre-existingtransactions:sendlimit) was actually being enforced.Changes
app.module.ts: importRateLimitModulesoRateLimitGuardRedisis active as a globalAPP_GUARD.GET /wallet/balances: per-user limit (WALLET_BALANCES_RATE_LIMIT_MAX/WALLET_BALANCES_RATE_LIMIT_WINDOW_MS, default 10/min).GET /anchor/fx-rate: per-user limit (ANCHOR_FX_RATE_RATE_LIMIT_MAX/ANCHOR_FX_RATE_RATE_LIMIT_WINDOW_MS, default 10/min).GET /anchor/auth/challengeandPOST /anchor/auth/token(unauthenticated SEP-10 endpoints, no JWT so the guard naturally keys on IP): per-IP limit (ANCHOR_AUTH_RATE_LIMIT_MAX/ANCHOR_AUTH_RATE_LIMIT_WINDOW_MS, default 20/min).Retry-Afterheader (existing guard behavior).docs/environment-variables.md.rate-limit.guard.spec.tswith coverage for the new key prefixes: per-user vs. per-IP keying, 429 +Retry-Afteron exceeding the limit, and env-var override behavior. Also fixed a pre-existing gap in the spec'sMockRedistest double (missingquit()) that made the suite fail to tear down.Acceptance criteria
Retry-Afterheaderrate-limit.guard.spec.tsextended with tests for the new endpointsdocs/environment-variables.mdTesting
All rate-limit and anchor-service suites pass.
tsc --noEmitshows no new errors introduced by this change (verified by diffing before/after).Out of scope (pre-existing, unrelated)
While touching these files I found
WalletServiceis currently missingfindByUserId,findByPublicKey, andgetBalances(andwallet.controller.tscallsenableMultiSignature, which doesn't exist either — it'senableMultisig), andAnchorModule/AnchorAuthControlleraren't registered inAppModuleyet. These predate this change and look like fallout from other in-flight work; left untouched here to keep this PR focused on rate limiting. The rate-limit decorators are already in place on those routes so they'll be enforced once that wiring lands.🤖 Generated with Claude Code