Skip to content

Cap readBody to 64 KB / 4 MB by route (closes D-3)#24

Merged
stevepridemore merged 0 commit into
mainfrom
readbody-size-caps
May 9, 2026
Merged

Cap readBody to 64 KB / 4 MB by route (closes D-3)#24
stevepridemore merged 0 commit into
mainfrom
readbody-size-caps

Conversation

@stevepridemore
Copy link
Copy Markdown
Owner

Summary

  • Extracts readBody to src/mcp-server/read-body.ts and adds two named size caps:
    • READ_BODY_CAP_OAUTH = 64 KB — DCR + token + revoke + authorize
    • READ_BODY_CAP_MCP = 4 MB — batch graph_relate from transcripts
  • Two-layer defense: fast reject via Content-Length before reading any bytes; streaming reject inside the data handler (defends against missing/lying Content-Length).
  • OAuth handlers use the default 64 KB cap; /mcp passes the 4 MB cap explicitly. Caps hit on /mcp are recorded in mcp-access.jsonl.
  • Hardcoded named constants — easy to grep for and adjust if traffic profile changes.

Closes finding D-3 from docs/internal/THREAT-MODEL.md.

Test plan

  • npm run build clean
  • npx vitest run src/mcp-server/read-body.test.ts — 10 new tests passing
  • Existing tests unaffected (Neo4j-dependent failures pre-exist and are unrelated)
  • Deploy and verify curl -X POST -H 'Content-Length: 99999999' --data-binary @<largefile> https://graph-memory.doublec.biz/oauth/register returns 413 {"error":"payload_too_large"}
  • Verify normal claude.ai connector reconnect + a tool call still works (regression check on the happy path)

🤖 Generated with Claude Code

@stevepridemore stevepridemore merged this pull request into main May 9, 2026
2 checks passed
@stevepridemore stevepridemore deleted the readbody-size-caps branch May 9, 2026 19:13
stevepridemore added a commit that referenced this pull request May 10, 2026
Extract the inline body parser into src/mcp-server/read-body.ts with
two named caps:

  READ_BODY_CAP_OAUTH = 64 KB   (DCR + token + revoke + authorize)
  READ_BODY_CAP_MCP   = 4  MB   (batch graph_relate from transcripts)

Defends in two layers:

  1. Fast reject — if Content-Length exceeds the cap, throw
     PayloadTooLargeError before reading any body bytes
  2. Streaming reject — accumulate bytes during the data event;
     if the running total exceeds the cap, req.destroy() and reject
     (defends against missing or lying Content-Length)

OAuth handlers use the default 64 KB cap. The /mcp route's inline
body reader continues to be inline (it streams chunks for the JSON-RPC
transport rather than buffering as a string) but now enforces the
4 MB cap with the same two-layer pattern. Caps hit on /mcp are
recorded in mcp-access.jsonl alongside other route activity.

Caps are hardcoded named constants — easy to grep for and adjust if
profiles change, no env var to document and forget.

Closes finding D-3 from docs/internal/THREAT-MODEL.md.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevepridemore added a commit that referenced this pull request May 10, 2026
…rs (#27)

- New devDep @vitest/coverage-v8; coverage block in vitest.config.ts
  using v8 provider, text + html reporters, sensible exclude list
  (dist, scripts, prompts, *.test.ts, *.config.ts, coverage). No
  thresholds — measure first, decide later.

- Three new npm scripts:
    test:coverage          — vitest run --coverage (assumes NEO4J_*
                             env set; CI uses this)
    test:local             — wrapper-driven full suite, no coverage
    test:coverage:local    — wrapper-driven full suite with coverage

- scripts/test-with-neo4j.sh and .ps1 wrappers spin up a throwaway
  neo4j:5.20-community on :7689 with NEO4J_AUTH=neo4j/test1234,
  wait for readiness via cypher-shell polling, run vitest with
  forwarded args, guarantee teardown via trap/finally even on
  Ctrl-C or test failure. Idempotent — removes any leftover
  container before starting.

- CI test job now runs `npm run test:coverage` against its existing
  service container, so coverage numbers are real (not lowered by
  Neo4j-test failures the way local-without-throwaway would be).

- docs/TESTING.md: new Coverage section explains the three commands,
  reporters (text + html), local Docker requirement, PowerShell
  fallback when bash isn't on PATH, and CI behaviour. Test-suite
  table refreshed to include oauth-events and read-body tests added
  in PRs #17 and #24, with a non-numeric phrasing for the suite
  size so the doc doesn't drift again.

- vitest.config.ts excludes .claude/** at the test level — caught
  during verification when the agent's worktree under .claude/
  caused vitest to discover duplicate test files. Project-level
  fix unrelated to coverage.

Baseline coverage on this run (133 tests, 4 files): 65% statements,
67% lines project-wide. Best covered: oauth-events.ts (100%),
oauth.ts (82%), read-body.ts (80%). Largest gap: neo4j-client.ts
(57%) — fault-injection paths not unit-tested.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant