Skip to content

fix request body limits for streamed requests - #33

Merged
laceyp99 merged 4 commits into
mainfrom
fix/streamed-request-body-limits
Jul 23, 2026
Merged

laceyp99 merged 4 commits into
mainfrom
fix/streamed-request-body-limits

Conversation

@laceyp99

Copy link
Copy Markdown
Owner

Summary Changes

  • Enforce MEMORIES_REQUEST_BODY_MAX_BYTES against the bytes actually received for REST and MCP POST/PATCH requests, including chunked requests without Content-Length.
  • Preserve accepted request bodies so FastAPI can parse and validate them normally after the size check.
  • Reject disallowed MCP browser origins and rate-limited clients before reading request bodies.
  • Count oversized requests against rate limits so misbehaving clients and AI agents cannot avoid rate-limit consequences.
  • Add contract coverage for request-body replay, disallowed-origin short-circuiting, rate-limit ordering, chunked oversized MCP requests, and bodies exactly at the configured limit.
  • Update the README to describe declared and streamed request-body enforcement.

Validation

  • .venv\Scripts\python.exe -m ruff format --check . - passed
  • .venv\Scripts\python.exe -m ruff check . - passed
  • .venv\Scripts\python.exe -m pytest - passed, 134 tests
  • .venv\Scripts\python.exe -m pre_commit run --all-files - passed

Additional Notes

@laceyp99
laceyp99 marked this pull request as ready for review July 23, 2026 05:03

@laceyp99 laceyp99 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified that it orders the reject origins and rate limits before the body limits. Validated myself formatting, linting, and tests pass too. Looks good to me!

@github-actions

Copy link
Copy Markdown

Agent PR review

Validation

Validation passed: ruff format --check ., ruff check ., and pytest all completed successfully.

Summary

PR #33 changes request-safety middleware so POST/PATCH body limits are enforced against streamed bytes, not only Content-Length, and replays accepted bodies for downstream FastAPI/MCP parsing. It also reorders MCP origin checks before rate limiting/body reads and expands contract tests around streamed bodies and limit boundaries.

Issues and risks

No blocking correctness issues found.

  • severity: low

  • file or area: app/request_limits.py

  • why it matters: body replay depends on Starlette’s private request._body cache behavior. It works with the pinned stack inspected locally, but it is brittle across FastAPI/Starlette upgrades.

  • suggested fix: either add a short comment explaining the Starlette cache dependency and keep this covered by contract tests, or consider an explicit ASGI receive-wrapper middleware if this code needs to survive framework upgrades with less coupling.

  • severity: low

  • file or area: tests/contract/test_mcp_http_transport.py

  • why it matters: the exact-limit MCP test only asserts status_code != 413, so it would pass on unrelated failures such as 500.

  • suggested fix: assert the expected MCP transport behavior more tightly, or at least assert the response is a known non-error/known validation status for {}.

Test coverage

The PR adds focused contract coverage for oversized declared bodies, chunked bodies without Content-Length, accepted exact-limit chunked bodies, replay into FastAPI validation, MCP origin short-circuiting, and rate-limit ordering. The review context reports ruff format --check ., ruff check ., pytest with 134 tests, and pre-commit run --all-files passed.

I did not rerun the full suite in this read-only review environment. One useful additional edge test would be a mismatched declared length under the limit with actual streamed bytes over the limit, to prove the streamed check catches malicious or incorrect Content-Length.

Recommendation

Looks safe to continue review.


This is an automated review. Please treat it as advisory; human review is still required.

Copy link
Copy Markdown
Owner Author

Review follow-up

Pushed 350fadf test(mcp): assert exact-limit redirect behavior.

This strengthens the exact-limit MCP regression test. It now expects the normal 307 redirect from /mcp to /mcp/ and verifies its Location header, rather than only checking that the response is not 413. That prevents unrelated failures such as 500 from passing the test.

Finding 2: intentionally deferred

The request._body replay mechanism depends on Starlette's private body-cache behavior, so the upgrade concern is valid. We are not replacing it with an ASGI receive-wrapper in this PR because the current FastAPI/Starlette behavior is covered by contract tests and works in the full test suite. A receive-wrapper would add substantial, error-prone transport complexity for this local API.

The contract tests should remain in place as an upgrade guard. We should revisit an explicit receive-wrapper if a FastAPI/Starlette upgrade breaks them, the API becomes externally exposed or higher traffic, or removing private-framework coupling becomes a specific project goal.

@laceyp99
laceyp99 merged commit b7709f2 into main Jul 23, 2026
4 checks passed
@laceyp99
laceyp99 deleted the fix/streamed-request-body-limits branch July 23, 2026 05:44
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.

Enforce request body size when Content-Length is absent

2 participants