[propr-dev]

AI Implementation Summary

Closes #3304

Branch: 3304/claude-opus-5-5-remediate-mcp-runtime-sdk-20261021-0122-syy
Commits: ✅ Changes committed (8007027)

---

AI Processing Completed

Execution Details:

• Issue: Remediate MCP runtime SDK advisories and enforce bounded HTTP tool inputs #3304
• Repository: integry/propr
• Status: Success
• Execution Time: 22m 28s
• Tokens used: 20.75M tokens [20.67M input + 76.2K output]
• API cost: $6.79
• Timestamp: Oct 21, 2026, 1:45 AM UTC
• LLM Model: Claude Opus 5.5
• Subscription usage: Session +1%

Summary:
The MCP server dependency is now on a patched release, and both audit gates pass on the regenerated lock without an exception. After a fresh npm ci, the full test:mcp suite passed 426/426 with nothing skipped. The required CI has not run yet; it runs on the pushed head.

Dependencies

• @modelcontextprotocol/server goes from 2.0.0 to 2.3.1, the latest patched release. No vulnerable copy is left anywhere in the tree.
• @modelcontextprotocol/node also had to go from 2.0.0 to 2.1.1. Version 2.0.0 kept server 2.0.0 installed at the root as its peer; 2.1.1 requires server 2.3.0 or later.
• I left the dev-only @modelcontextprotocol/client at 2.2.0, so there is no overlap with Dependabot PR chore(deps-dev): bump @modelcontextprotocol/client from 2.2.0 to 2.3.0 in the npm_and_yarn group across 1 directory #3303. The tests drive the patched server with that client and pass.
• The lock was regenerated with npm 11, and npm 10 (the version CI's npm ci uses) re-resolves it byte-for-byte.
• One addition you didn't ask for: the packaging audit was already failing on the untouched base lock. Newly published advisories hit vite 7.3.5/7.3.6, so I moved vite to 7.3.7, which is inside the existing ^7.3.5 range. Only the lockfile changed, no package.json.

Shared-server advisory (GHSA-jm63)

• serveMcpRequest still builds a new handler, transport and McpServer for every request, with the same cancellation and cleanup. I only added a comment explaining why.
• New tests in mcpHttpIsolation.test.ts go through the real /api/mcp route with real tokens. Two users with different scopes send concurrent requests, all using the same JSON-RPC id. They also cover a long-running call held open while the other user's requests run, a client disconnect that cancels it, and two SDK clients whose ids collide naturally.
• I checked that the tests catch the bug: sharing one handler across requests made all three fail, with one user receiving the other's results.

Input limits (GHSA-46jr)

• Tool arguments: maxToolInputElements is set to 10,000. The largest bounded tool input is 532 elements (update_execution_settings). Only the synthetic-agent composition inputs have no limit of their own, and a 30-model × 20-member composition still fits under 10,000.
  • mcpToolInputBudget.test.ts computes these maximums from the real tool schemas and fails if a new unbounded schema appears.
• Request body: the root JSON parser now skips /api/mcp. The MCP route has its own 1 MiB parser that runs before authentication, which is the same limit MCP requests already had, so no existing workflow changes. The same limit is passed to the SDK for any body it reads itself.
• Tests:
  • A body one byte over the limit gets 413 with no GitHub lookup and no access-log row; a body of exactly 1 MiB is still accepted.
  • A ~13,000-element create_plan call is refused by the element check, with no plan, operation or goal written, and the next ordinary request succeeds.
  • I added a check to extensionServerMount.test.ts, which starts the real server.ts, to confirm the limit holds there too.
• docs/mcp.md now states both limits.

Wrong-audience advisory (GHSA-rvq5)
ProPR doesn't use the affected SDK bearer helpers (requireBearerAuth/verifyBearerToken), so I kept McpPolicy.authenticate as it is.

• Local tokens are opaque digests looked up in this instance's store, and each grant is only accepted for this instance and this exact MCP resource.
• Connect tokens are checked for this instance's audience, its instance id, the Connect resource, and the resource the client says it connected to.

mcpAudience.test.ts tests each wrong resource, instance and audience against a valid token. Removing either the local or the Connect resource check makes those tests fail.

Other checks

• API typecheck and API lint (zero warnings, including tests) are clean.
• The OpenAPI, route-authorization, CORS, rate-limit and MCP browser/access-log tests pass (69/69).
• I didn't run test:mcp:connect, because it needs the external routing bundle.
• This change isn't visible in the UI, so there are no previews.

Detailed Logs:

• Conversation: 278 messages
• Session: 7937997d-368f-4b65-8cf1-baa9c73786f9

Log files stored at:

• conversation: /usr/src/app/logs/executions/issue-3304-2026-10-21T01-45-00-682Z-conversation.json
• output: /usr/src/app/logs/executions/issue-3304-2026-10-21T01-45-00-682Z-output.txt

Latest Conversation Messages

ASSISTANT: [content unavailable]

ASSISTANT: The MCP server dependency is now on a patched release, and both audit gates pass on the regenerated lock without an exception. After a fresh `npm ci`, the full `test:mcp` suite passed 426/426 with not...

---

This PR was created automatically by ProPR after processing issue #3304.

---

💡 Need changes?

Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!

[integry]

/review codex-astra

Independently review the full release-blocking remediation for issue #3304 at exact head 8007027. Assess correctness and the original acceptance criteria, not just whether the version bump clears an audit. Operator fresh-install, runtime/packaging audits, real-SDK HTTP checks, MCP regressions and UI artifact continuity checks are starting; do not claim their results or final Linux/mobile/hardware qualification from static review.

Check all of the following:

• Patched production MCP server and compatible node/core dependencies; no nested vulnerable server. The dev-only client remains unchanged and PR chore(deps-dev): bump @modelcontextprotocol/client from 2.2.0 to 2.3.0 in the npm_and_yarn group across 1 directory #3303 is separate. The PR also updates Vite within its existing range because the author reports the previously untouched packaging audit now fails; verify necessity and scope, including any UI/build implications.
• A fresh handler, transport and server for each HTTP request/principal; colliding JSON-RPC IDs, differently scoped users, long reads, cancellation, cleanup, normal subsequent requests, and unchanged authorization/attribution/error-envelope contracts.
• A finite 10,000-element tool-input budget before schema/tool execution, legitimate bounded tool schemas and synthetic-agent compositions, useful refusal without plan/operation/goal side effects, and healthy service afterward.
• The 1 MiB body limit through the real API middleware ordering, endpoint parser and SDK; case variants/trailing slash, exact boundary versus oversized input, authentication ordering, malformed bodies and continued normal API/extension upload behavior. Avoid an audit-only fix that leaves actual endpoint behavior unbounded or breaks normal requests.
• Existing custom McpPolicy/OAuth/Connect audience, instance, resource and scope checks. ProPR already avoids the vulnerable SDK bearer helpers; evaluate the new negative/valid controls without broadening permissions, copying credentials or substituting a new auth implementation.
• Meaningful real-installed-SDK HTTP tests, not version-string/source-text assertions alone. Inspect all new fixtures/tests, schema-budget enumeration and existing test modifications; temporary debugging probes must be absent or converted into proper regressions. Preserve Linux/mobile Search fixes from [3301 by Claude Opus 5.5] Make existing global search accessible by touch from the ProPR mobile menu #3302 and all existing release gates.

Primary advisories are linked in issue #3304. Root's bounded baseline against 1fa3154 independently observed correct per-request isolation for two clients using ID0, acceptance/invocation of a 20,044-byte /10,016-value synthetic read tool call with no element budget, and a successful following ordinary request. This used the real production dispatcher but synthetic principals and in-memory data in a network-isolated container; it is not a memory-exhaustion demonstration or real authorization/provider/desktop acceptance. A corrected harness explicitly closes both source/compiled core SQLite pools and exits0; earlier import setup failures and a cleanup timeout were preserved separately. Verify the new implementation on its merits.

[propr-dev]

✅ AI Code Review Complete requested by @integry

Posted 1 review:

• GPT-6 Astra

View Task Details

[propr-dev]

🔍 AI Code Review — codex-astra

Overall Evaluation

The remediation is focused and well supported by regression tests. No merge-blocking defect is demonstrated by the supplied diff, but I cannot declare the release remediation fully verified from this static review.

✅ Explicit input bounds — buildMcpServer configures a 10,000-element budget, while the endpoint parser and handler receive the same 1 MiB body limit. The global parser exemption matches Express’s case-insensitive endpoint and optional trailing slash without changing ordinary API or archive limits.

✅ Request-local isolation preserved — Handler creation remains inside serveMcpRequest, with a principal-bound server factory, disconnect cancellation, listener removal and handler.close() in finally. New HTTP tests exercise colliding IDs, differing scopes, blocking reads, disconnects and subsequent requests.

✅ Existing authentication preserved — The unchanged OAuth grant lookup binds local tokens to instance and resource; Connect verification checks audience and additional bindings before online validation. New negative tests include valid controls without replacing authentication or widening permissions.

The lock diff aligns server/core at 2.3.1 and node at 2.1.1, whose declared server peer range is compatible. It shows no added nested server and leaves the dev client unchanged. Vite moves to 7.3.7 within the existing range and removes the UI’s nested copy, so UI builds now resolve the shared updated version. Its claimed audit necessity and resulting artifact continuity are not independently established by the supplied code.

No commands or tests were run. Installed SDK implementation details, fresh-install dependency resolution, audit results and platform qualification remain unverified. The original issue text explicitly contains a truncation notice; this review covers its visible requirements and the detailed review request, without inferring omitted acceptance criteria. The supplied current status reports no failures and eight pending checks.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Exercise malformed HTTP bodies

The new tests cover exact and oversized JSON bodies but do not explicitly exercise malformed JSON through the real API composition. An optional regression could assert the existing 400/INVALID_REQUEST response, no bearer lookup or dispatch, and successful service afterward. The parser and endpoint error handler already provide the expected static path; this is additional coverage, not a demonstrated defect.

S2: 🟢 Make schema enumeration fail closed

In packages/api/test/mcpToolInputBudget.test.ts, maxElements does not recognize additionalProperties: true as unbounded and returns zero for unrecognized schema shapes. No supplied current schema demonstrates an incorrect result, but future schemas could receive misleading budget assurance. Explicitly handling open objects and rejecting unsupported constructs would strengthen this optional test helper.

Score

The implementation has no demonstrated code blocker and adds meaningful HTTP regressions. A material verification gap remains around actual installed SDK behavior, dependency audit results and the Vite build implications, so full merge readiness is not established by this review.

Score: 7/10

---

🤖 Review Details

• Model: GPT-6 Astra
• Time: 45s
• Tokens: 66,069 (65,224 in / 845 out)
• Cost: $0.56

View Task

---

💡 Next step: Comment /fix to address every F# merge blocker, /fix all for every pending blocker and suggestion, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them or request /fix all, and they never relax a merge blocker.

Copy and edit to choose records from this review:

/fix S1 S2

_{🤖 Review by ProPR}