fix: tighten Content-Length and chunked-encoding handling on /api/v1/… - #2385
Merged
Baskarayelu merged 1 commit intoJul 29, 2026
Merged
Conversation
…events The ingest guard ran after the application-wide 1MB JSON parser, so oversized payloads were fully buffered before being refused and the 256KB budget was never actually bound to the route. It also read Content-Length before Transfer-Encoding, which masked chunked requests behind a 411, and treated the mere presence of X-Allow-Chunked-Encoding as authorisation. The header guard now runs before any body parsing (the global parser skips this route), chunked framing is evaluated first and only accepted from a proxy listed in EVENT_INGEST_CHUNKED_PROXY_ALLOWLIST, and combining both framing headers is rejected as a smuggling signature. Body-parser failures are re-mapped to constant messages so no rejection echoes payload bytes. Closes QuickLendX#2283 Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Pull Request Template
📝 Description
POST /api/v1/eventshad an ingest guard, but it was mounted at the router level — after the application-wide 1 MBexpress.jsonparser. Oversized payloads were therefore fully buffered and parsed before being refused, and the 256 KB budget was never actually bound to the route. The guard also readContent-LengthbeforeTransfer-Encoding(HTTP stripsContent-Lengthfrom chunked requests, so genuine chunked traffic was masked behind a411), and it treated the mere presence ofX-Allow-Chunked-Encodingas authorisation, which is trivially forgeable.This PR moves the header guard ahead of all body parsing, gives the route its own 256 KB parser, validates the chunked-encoding allowlist against configured proxy identifiers, and re-maps body-parser failures to constant messages so no rejection echoes payload bytes.
🎯 Type of Change
🔧 Changes Made
Files Modified
backend/src/middleware/event-ingest-limits.ts— rewrittenbackend/src/app.tsbackend/src/routes/v1/index.tsbackend/src/tests/events-ingest-limits.test.ts— rewrittenbackend/jest.config.jsbackend/docs/security.mdbackend/docs/limits.mdNew Files Added
event-ingest-limits.tsandevents-ingest-limits.test.tsalready existed from PR Feature/event ingest limits #1465; both were rewritten rather than created.Key Changes
isEventIngestRequest, and the route mounts its ownexpress.jsonparser with a 256 KB limit. An oversized request is refused without buffering its payload.Content-Length. Evaluating chunked first prevents a smuggled request from being masked behind a411.X-Allow-Chunked-Encodingmust carry a value present inEVENT_INGEST_CHUNKED_PROXY_ALLOWLIST. The variable is unset by default, so every chunked request is rejected out of the box.Transfer-Encoding: chunkedcombined withContent-Lengthis rejected asAMBIGUOUS_REQUEST_FRAMING, even for allowlisted proxies. That combination is the canonical request-smuggling signature.Content-Lengthparsing. Must be a single non-negative integer; duplicated headers and non-numeric values are rejected asINVALID_CONTENT_LENGTHinstead of being coerced.application/jsonrequestno longer passes;charsetand other parameters are still accepted.Unexpected token c ... is not valid JSON).Error codes
application/json415INVALID_CONTENT_TYPEContent-Lengthabsent on a non-chunked request411CONTENT_LENGTH_REQUIREDContent-Lengthis not a single non-negative integer400INVALID_CONTENT_LENGTH413BODY_LIMIT_EXCEEDED400CHUNKED_ENCODING_NOT_ALLOWEDContent-Length400AMBIGUOUS_REQUEST_FRAMINGContent-Length400CONTENT_LENGTH_MISMATCH400INVALID_JSON_BODY🧪 Testing
Test Coverage
34 tests, all passing, with 100% coverage (statements, branches, functions, lines) on
src/middleware/event-ingest-limits.ts. A 95% threshold for the file was added tojest.config.jsso the coverage cannot silently regress.Before this PR, 6 of the suite's 8 tests were failing on
main.The suite is organised in five groups:
Content-Length, the budget boundary at exactly 256 KB, chunked with and without an allowlisted proxy, allowlist read from the environment,chunked+Content-Length, and non-chunked transfer encodings such asgzip.CONTENT_LENGTH_MISMATCHmapping and the fall-through to the error handler.isEventIngestRequest— path and method matching, including trailing slash, casing, and an undefined path.Every rejection path asserts that the response does not contain a canary string planted in the request payload, which covers the issue's requirement that rejection messages not echo payload bytes.
Regression check: the full backend suite was run against a stashed baseline of
main. Failures went from 346 to 340, and a diff of failing suites shows the only difference isevents-ingest-limits.test.tsmoving from failing to passing. Zero new failures. The ~340 remaining failures and thetsc --noEmiterrors are pre-existing onmainand unrelated to this change; no error is reported for any file touched here.📋 Contract-Specific Checks
Not applicable — this PR touches only
backend/. No Soroban contract code was modified.Contract Testing Details
None. No contract changes.
📋 Review Checklist
🔍 Code Quality
🚀 Performance & Security
Because the global parser now skips this route,
requestLoggerno longer snapshots the ingest payload intoreq.bodybefore the route runs, so event payloads stop reaching the log sink. Memory pressure also improves: a 900 KB body is refused on its headers instead of being buffered and parsed first.📚 Documentation
backend/docs/security.mdnow documents the full framing policy: the error-code table, why chunked is evaluated beforeContent-Length, the allowlist configuration, and the guarantee that no rejection echoes payload bytes.backend/docs/limits.mdpreviously documented only the global 1 MB budget, so it gained the per-route override and a cross-reference to the security doc.🔗 Related Issues
Closes #2283
📋 Additional Notes
The endpoint sits behind the CSRF middleware, which requires either
x-api-keyorx-csrf-tokenon state-changing requests. The integration tests sendx-api-keybecause the indexer is a machine-to-machine client. Worth confirming that the real indexer does the same — if it does not, it is receiving403 MISSING_CSRF_TOKENtoday, which is a separate pre-existing issue rather than something introduced here.The middleware exposes
createEventIngestLimitsMiddleware(options)andcreateEventIngestBodyParser(parser)factories. These exist so tests can inject a budget, an allowlist, or a failing parser without touching global state, which is how the error-mapping branches reach full coverage.🧪 How to Test
cd backend && npm cinpm test -- events-ingest-limits— expect 34 passing tests.npx jest events-ingest-limits --coverage --collectCoverageFrom="src/middleware/event-ingest-limits.ts"— expect 100% across all four metrics.npm run dev) and exercise the endpoint manually:415 INVALID_CONTENT_TYPEcurl -i -X POST localhost:3000/api/v1/events -H "x-api-key: qlx_dev" -H "Content-Type: text/plain" -d "hello"413 BODY_LIMIT_EXCEEDEDcurl -i -X POST localhost:3000/api/v1/events -H "x-api-key: qlx_dev" -H "Content-Type: application/json" --data-binary @large.json(withlarge.jsonabove 256 KB)400 CHUNKED_ENCODING_NOT_ALLOWEDcurl -i -X POST localhost:3000/api/v1/events -H "x-api-key: qlx_dev" -H "Content-Type: application/json" -H "Transfer-Encoding: chunked" -d '[]'400 INVALID_JSON_BODY, and confirm the response body contains none of the bytes you sent.npx jest --forceExit --coverage=falseand compare against the same command onmain.📸 Screenshots (if applicable)
Not applicable — backend-only change, no UI.
Two behavioural changes affect clients of
POST /api/v1/eventsthat were relying on the previous, ineffective guard:X-Allow-Chunked-Encodingwith any value used to be enough. The value must now match an entry inEVENT_INGEST_CHUNKED_PROXY_ALLOWLIST, which is unset by default.413. They were previously accepted or failed with a400parse error, because the global parser handled them before the guard ran. This is the documented intent of the 256 KB budget, so it is a fix rather than a new restriction, but callers may observe a status change.🔄 Migration Steps (if applicable)
Only needed if an upstream proxy legitimately forwards chunked bodies to this endpoint:
EVENT_INGEST_CHUNKED_PROXY_ALLOWLIST=edge-proxy-1,edge-proxy-2X-Allow-Chunked-Encoding: edge-proxy-1on forwarded requests.X-Allow-Chunked-Encodingheader before forwarding, so callers cannot assert a proxy identity themselves.Transfer-Encoding: chunkedandContent-Length; that combination is rejected regardless of the allowlist.No migration is required if chunked ingest is not in use, which is the default.