fix(security): rate limit /api/stellar/submit and /api/stellar/sign-and-submit (closes #365) - #540
Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Conversation
/api/stellar/submit and /api/stellar/sign-and-submit had no rate limit at all: an authenticated session could spray unlimited Horizon round-trips and DB writes, and unlimited server-side signing attempts. Both now use the existing rateLimit() helper, keyed per user, with sign-and-submit held to a tighter budget because it accepts a raw secret key. 429 responses now carry Retry-After (send included). Closes Northgate-Systems#365
|
Someone is attempting to deploy a commit to the codex723's projects Team on Vercel. A member of the Team first needs to authorize it. |
6 tasks
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
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.
Closes #365
The actual gap
/api/stellar/sendalready hadrateLimit(\send:${user.id}`, 20, 60_000)`. The two endpoints that actually spend a transaction did not:/api/stellar/sendRetry-After)/api/stellar/submit/api/stellar/sign-and-submitSo an authenticated session could loop
submitwithout limit — each call is a Horizon round-trip plus two DB writes — and could spraysign-and-submit, which accepts a raw Stellar secret key and signs server-side. That one gets the tighter budget for exactly that reason: a stolen session cookie shouldn't buy unlimited signing attempts.Changes
src/app/api/stellar/submit/route.ts,src/app/api/stellar/sign-and-submit/route.ts—rateLimit()from@/lib/securitykeyed per user, placed after the auth check and before any body read, DB query or Horizon call, so a throttled request costs nothing. Each emits the existinglogSecurityEvent("rate_limited", …)event, same as the routes that already had limits.src/lib/api-response.ts—errorResponse()takes an optional thirdheadersargument (defaults to previous behaviour exactly, no existing call site changes shape). Used to attachRetry-Afterto the three 429s insrc/app/api/stellar/. A 429 with noRetry-Aftergives a client nothing to back off on.I deliberately left the
auth/*andanalyticslimiters alone — they already have limits, and consolidating all of them is #366's job, not this PR's.Tests
14 new tests, all mocked, no network:
src/app/api/stellar/submit/__tests__/route.test.ts(7) — first 20 allowed, 21st is 429 in the standard{success:false,error}shape,Retry-Afterpresent and within the window, a throttled call reaches neither Horizon nor the DB, budgets are per-user (a noisy account can't lock out another), anonymous callers still get 401, and a request that fails schema validation still burns budget (otherwise malformed bodies are a free unlimited channel).src/app/api/stellar/sign-and-submit/__tests__/route.test.ts(7) — the same matrix at the 10/min budget, plus: it is strictly tighter thansubmit, and a rejected secret key still burns budget, so key-guessing is throttled rather than free.Negative control: reverting only the two route files and re-running these tests turns 10 of the 14 red; restoring them puts it back to 14/14. The tests fail without the fix.
npx eslint src/app/api/stellar src/lib/api-response.ts→ clean, 0 warnings.npx tsc --noEmit→ 0 errors in any touched file.Two disclosures
@/lib/validationsis currently broken onmain—ReferenceError: isValidStellarPublicKey is not definedatsrc/lib/validations.ts:40, introduced by fix(send): validate the Stellar address checksum client-side (closes #424) #527 (the.refine()was added without importing the helper). It makesnpm run buildfail and 4 test files fail to collect; verified it reproduces on a cleanorigin/mainviagit stash. The fix is waiting in the still-open fix(validations): restore broken isValidStellarPublicKey import; feat: add /api/stellar/fee-estimate #529. Becausesubmit/route.tsimports that module, the submit test mocks@/lib/validationswith a byte-identical copy ofstellarSubmitSchema(that schema is not affected by the bug) — commented in the file, and the mock can be deleted once fix(validations): restore broken isValidStellarPublicKey import; feat: add /api/stellar/fee-estimate #529 lands.request.json()toreadBodyWithLimit(). Different lines (limiter goes above the body read), so they should merge in either order; happy to rebase whichever lands second.