Skip to content

fix: tighten Content-Length and chunked-encoding handling on /api/v1/… - #2385

Merged
Baskarayelu merged 1 commit into
QuickLendX:mainfrom
D240021:feature/events-ingest-limits
Jul 29, 2026
Merged

fix: tighten Content-Length and chunked-encoding handling on /api/v1/…#2385
Baskarayelu merged 1 commit into
QuickLendX:mainfrom
D240021:feature/events-ingest-limits

Conversation

@D240021

@D240021 D240021 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Pull Request Template

📝 Description

POST /api/v1/events had an ingest guard, but it was mounted at the router level — after the application-wide 1 MB express.json parser. 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 read Content-Length before Transfer-Encoding (HTTP strips Content-Length from chunked requests, so genuine chunked traffic was masked behind a 411), and it treated the mere presence of X-Allow-Chunked-Encoding as 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

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Performance improvement
  • Security enhancement
  • Other (please describe):

🔧 Changes Made

Files Modified

  • backend/src/middleware/event-ingest-limits.ts — rewritten
  • backend/src/app.ts
  • backend/src/routes/v1/index.ts
  • backend/src/tests/events-ingest-limits.test.ts — rewritten
  • backend/jest.config.js
  • backend/docs/security.md
  • backend/docs/limits.md

New Files Added

Key Changes

  • The 256 KB budget is now bound to the route. The global 1 MB parser skips this path via isEventIngestRequest, and the route mounts its own express.json parser with a 256 KB limit. An oversized request is refused without buffering its payload.
  • Check order fixed. Content type → chunked framing → Content-Length. Evaluating chunked first prevents a smuggled request from being masked behind a 411.
  • The chunked allowlist actually validates. X-Allow-Chunked-Encoding must carry a value present in EVENT_INGEST_CHUNKED_PROXY_ALLOWLIST. The variable is unset by default, so every chunked request is rejected out of the box.
  • Transfer-Encoding: chunked combined with Content-Length is rejected as AMBIGUOUS_REQUEST_FRAMING, even for allowlisted proxies. That combination is the canonical request-smuggling signature.
  • Stricter Content-Length parsing. Must be a single non-negative integer; duplicated headers and non-numeric values are rejected as INVALID_CONTENT_LENGTH instead of being coerced.
  • Stricter content-type matching. Exact media-type comparison, so application/jsonrequest no longer passes; charset and other parameters are still accepted.
  • No payload echo. Body-parser errors are re-mapped to constant messages, because its native messages quote the offending bytes (for example Unexpected token c ... is not valid JSON).

Error codes

Condition Status Error code
Content type is not exactly application/json 415 INVALID_CONTENT_TYPE
Content-Length absent on a non-chunked request 411 CONTENT_LENGTH_REQUIRED
Content-Length is not a single non-negative integer 400 INVALID_CONTENT_LENGTH
Declared or actual body above 256 KB 413 BODY_LIMIT_EXCEEDED
Chunked encoding without an allowlisted proxy 400 CHUNKED_ENCODING_NOT_ALLOWED
Chunked encoding combined with Content-Length 400 AMBIGUOUS_REQUEST_FRAMING
Body bytes do not match the declared Content-Length 400 CONTENT_LENGTH_MISMATCH
Body is not parseable JSON 400 INVALID_JSON_BODY

🧪 Testing

  • Unit tests pass
  • Integration tests pass
  • Manual testing completed
  • No breaking changes introduced
  • Cross-platform compatibility verified
  • Edge cases tested

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 to jest.config.js so 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:

  • Header guard (unit) — content type variants, missing/non-numeric/duplicated 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 as gzip.
  • Body parser — oversized body, malformed JSON, valid body with raw-byte capture, plus injected parser failures covering the CONTENT_LENGTH_MISMATCH mapping and the fall-through to the error handler.
  • Guard chain — the two handlers mounted together, verifying a valid request reaches the handler with the parsed body and that an oversized one is refused before parsing.
  • isEventIngestRequest — path and method matching, including trailing slash, casing, and an undefined path.
  • Integration against the real app — wrong content type, oversized body, a body sized between the route budget and the global 1 MB budget, malformed JSON, and a valid batch.

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 is events-ingest-limits.test.ts moving from failing to passing. Zero new failures. The ~340 remaining failures and the tsc --noEmit errors are pre-existing on main and 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.

  • Soroban contract builds successfully — N/A
  • WASM compilation works — N/A
  • Gas usage optimized — N/A
  • Security considerations reviewed
  • Events properly emitted — N/A
  • Contract functions tested — N/A
  • Error handling implemented
  • Access control verified — N/A

Contract Testing Details

None. No contract changes.

📋 Review Checklist

  • Code follows project style guidelines
  • Documentation updated if needed
  • No sensitive data exposed
  • Error handling implemented
  • Edge cases considered
  • Code is self-documenting
  • No hardcoded values
  • Proper logging implemented

🔍 Code Quality

  • Clippy warnings addressed — N/A, no Rust changes
  • Code formatting follows rustfmt standards — N/A, no Rust changes
  • No unused imports or variables
  • Functions are properly documented
  • Complex logic is commented

🚀 Performance & Security

  • Gas optimization reviewed — N/A
  • No potential security vulnerabilities
  • Input validation implemented
  • Access controls properly configured
  • No sensitive information in logs

Because the global parser now skips this route, requestLogger no longer snapshots the ingest payload into req.body before 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

  • README updated if needed
  • Code comments added for complex logic
  • API documentation updated
  • Changelog updated (if applicable)

backend/docs/security.md now documents the full framing policy: the error-code table, why chunked is evaluated before Content-Length, the allowlist configuration, and the guarantee that no rejection echoes payload bytes. backend/docs/limits.md previously 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-key or x-csrf-token on state-changing requests. The integration tests send x-api-key because the indexer is a machine-to-machine client. Worth confirming that the real indexer does the same — if it does not, it is receiving 403 MISSING_CSRF_TOKEN today, which is a separate pre-existing issue rather than something introduced here.

The middleware exposes createEventIngestLimitsMiddleware(options) and createEventIngestBodyParser(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

  1. Install dependencies: cd backend && npm ci
  2. Run the suite: npm test -- events-ingest-limits — expect 34 passing tests.
  3. Verify coverage: npx jest events-ingest-limits --coverage --collectCoverageFrom="src/middleware/event-ingest-limits.ts" — expect 100% across all four metrics.
  4. Start the backend (npm run dev) and exercise the endpoint manually:
    • Wrong content type → 415 INVALID_CONTENT_TYPE
      curl -i -X POST localhost:3000/api/v1/events -H "x-api-key: qlx_dev" -H "Content-Type: text/plain" -d "hello"
    • Oversized body → 413 BODY_LIMIT_EXCEEDED
      curl -i -X POST localhost:3000/api/v1/events -H "x-api-key: qlx_dev" -H "Content-Type: application/json" --data-binary @large.json (with large.json above 256 KB)
    • Chunked without allowlist → 400 CHUNKED_ENCODING_NOT_ALLOWED
      curl -i -X POST localhost:3000/api/v1/events -H "x-api-key: qlx_dev" -H "Content-Type: application/json" -H "Transfer-Encoding: chunked" -d '[]'
    • Malformed JSON → 400 INVALID_JSON_BODY, and confirm the response body contains none of the bytes you sent.
  5. Confirm no regressions elsewhere: npx jest --forceExit --coverage=false and compare against the same command on main.

📸 Screenshots (if applicable)

Not applicable — backend-only change, no UI.

⚠️ Breaking Changes

Two behavioural changes affect clients of POST /api/v1/events that were relying on the previous, ineffective guard:

  • Chunked requests now need explicit configuration. Sending X-Allow-Chunked-Encoding with any value used to be enough. The value must now match an entry in EVENT_INGEST_CHUNKED_PROXY_ALLOWLIST, which is unset by default.
  • Bodies between 256 KB and 1 MB are now rejected with 413. They were previously accepted or failed with a 400 parse 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:

  1. Set the allowlist on the backend, using one identifier per proxy:
    EVENT_INGEST_CHUNKED_PROXY_ALLOWLIST=edge-proxy-1,edge-proxy-2
  2. Configure that proxy to send X-Allow-Chunked-Encoding: edge-proxy-1 on forwarded requests.
  3. Configure the proxy to strip any client-supplied X-Allow-Chunked-Encoding header before forwarding, so callers cannot assert a proxy identity themselves.
  4. Ensure the proxy does not forward both Transfer-Encoding: chunked and Content-Length; that combination is rejected regardless of the allowlist.

No migration is required if chunked ingest is not in use, which is the default.

…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>
@Baskarayelu
Baskarayelu merged commit 6fa9e8a into QuickLendX:main Jul 29, 2026
4 of 5 checks passed
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.

Backend: Validate Content-Length and reject chunked-encoding tricks on /api/v1/events

2 participants