release(v0.45.0): remote --upstream-url connector + in-repo SEP-2787 vectors + Streamable HTTP conformance#172
Merged
Merged
Conversation
Lets the MCP proxy sit in front of remote MCP servers, not just local stdio subprocesses. Extracts an UpstreamClient interface over the request/notify/close surface and adds HttpUpstreamClient, a stdlib-only (urllib) Streamable HTTP transport: POST JSON-RPC, read application/json or text/event-stream replies, capture and echo Mcp-Session-Id, send MCP-Protocol-Version once negotiated, and run a standing GET SSE channel for server-initiated notifications with Last-Event-ID resume. CLI: --upstream-url NAME=URL (bare URL lands under default) and --upstream-header NAME=HEADER for static auth. A slot is stdio or remote, never both; collisions and stray headers are rejected at startup. Zero new runtime dependencies. The deprecated 2024-11-05 two-endpoint HTTP+SSE transport and interactive OAuth are out of scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror the execution-receipt vector layout for the attestation envelope. Pinned HS256/ES256/RS256 keys, six cases spanning signature, TTL, and argument-commitment verdicts, a stdlib-only walker that verifies them without importing Vaara, and a pytest guard that re-checks every verdict through the library. Marks the conformance doc in-repo.
Close three Streamable HTTP conformance gaps on /mcp. Mcp-Session-Id must be visible ASCII (0x21 to 0x7E) on POST and GET next to the length cap. A present MCP-Protocol-Version must be a revision the transport speaks (2025-03-26 or 2025-06-18); absent is assumed 2025-03-26. POST Accept must allow both application/json and text/event-stream, wildcard-aware so wildcard and absent headers still pass.
…vectors + Streamable HTTP conformance Three changes ship in v0.45.0. --upstream-url lets the proxy front a remote MCP server over Streamable HTTP rather than only a local stdio subprocess. Standard-library urllib only, so the zero-dependency core holds: session-id echo, protocol-version negotiation, and a standing GET SSE channel with Last-Event-ID resume. Static-header auth via --upstream-header. The deprecated 2024-11-05 transport and interactive OAuth are out of scope. In-repo SEP-2787 attestation conformance vectors land under tests/vectors/sep2787_attestation_v0/, with a standard-library-only independent checker and a pytest cross-check. docs/sep2787-conformance.md now points at the in-repo vectors. Three Streamable HTTP conformance fixes in the proxy: visible-ASCII session-id validation on POST and GET, MCP-Protocol-Version validation against the supported set, and a wildcard-aware POST Accept check that leaves existing clients unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 44 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion 0.45.0 introduces HTTP/SSE upstream MCP support for the proxy and SEP-2787 attestation conformance vector fixtures. The release spans upstream protocol abstraction, HTTP transport implementation, proxy integration with new CLI options and validation rules, and a complete test vector infrastructure with independent and library-based verification. ChangesVersion 0.45.0 release with HTTP upstream and attestation vectors
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vaara/integrations/_mcp_upstream_http.py`:
- Around line 104-110: When handling POST responses, only accept non-SSE replies
whose Content-Type is exactly application/json (ignoring parameters like
charset); after resp = self._post(...) and before calling _reply_from_json,
parse resp.headers.get("Content-Type", "") splitting on ';' and lower()-strip
the media type and raise/return an error if it is not "application/json". Apply
the same Content-Type validation in the other POST-reply handling block
referenced around lines 197-220 so both paths validate media types before
calling _reply_from_json; keep the existing _is_event_stream and _reply_from_sse
logic unchanged for text/event-stream.
In `@src/vaara/integrations/mcp_proxy.py`:
- Around line 574-596: Summary: Don't strip Mcp-Session-Id before validation;
validate the raw header value verbatim. Replace the .strip() usage so
session_value = (mcp_session_id or "") (no .strip()) and then perform the length
check against _MCP_SESSION_ID_MAX_LEN and the visible-ASCII check via
_session_id_is_visible_ascii(session_value); ensure you raise the same
HTTPException details when invalid. Apply the identical change to the other
validation block that uses
session_value/_MCP_SESSION_ID_MAX_LEN/_session_id_is_visible_ascii later in the
file so both header validations treat leading/trailing spaces as invalid
distinct values.
- Around line 152-171: The _accept_satisfies function currently ignores Accept
parameters like q=0; update it to parse each token's parameters and treat any
token with q=0 as unacceptable. In _accept_satisfies (params accept, media_type)
change the tokens comprehension to parse each token's params (split on ";" then
parse a q value if present), skip tokens whose q parsed as 0 (or 0.0), and only
include type tokens with q>0; then continue matching "*/*", "{main_type}/*", or
the exact media_type as before, handling invalid/missing q as q=1 and using
lowercasing for comparisons.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca17eacb-6c89-4dc8-a6d3-452df84d2de9
⛔ Files ignored due to path filters (5)
tests/vectors/sep2787_attestation_v0/keys/es256_private.pemis excluded by!**/*.pemtests/vectors/sep2787_attestation_v0/keys/es256_public.pemis excluded by!**/*.pemtests/vectors/sep2787_attestation_v0/keys/hs256_secret.binis excluded by!**/*.bintests/vectors/sep2787_attestation_v0/keys/rs256_private.pemis excluded by!**/*.pemtests/vectors/sep2787_attestation_v0/keys/rs256_public.pemis excluded by!**/*.pem
📒 Files selected for processing (34)
.claude-plugin/marketplace.jsonCHANGELOG.mdclients/ts/package.jsondocs/sep2787-conformance.mdpyproject.tomlscripts/generate_sep2787_attestation_vectors.pyserver-vaara-server.jsonserver.jsonsrc/vaara/__init__.pysrc/vaara/integrations/_mcp_upstream.pysrc/vaara/integrations/_mcp_upstream_http.pysrc/vaara/integrations/mcp_proxy.pytests/test_attestation_vectors.pytests/test_mcp_proxy_conformance.pytests/test_mcp_upstream_http.pytests/vectors/sep2787_attestation_v0/README.mdtests/vectors/sep2787_attestation_v0/_check_independent.pytests/vectors/sep2787_attestation_v0/normative/es256_projection_identity/attestation.jsontests/vectors/sep2787_attestation_v0/normative/es256_projection_identity/expected.jsontests/vectors/sep2787_attestation_v0/normative/es256_projection_identity/runtime_args.jsontests/vectors/sep2787_attestation_v0/normative/hs256_digest_identity/attestation.jsontests/vectors/sep2787_attestation_v0/normative/hs256_digest_identity/expected.jsontests/vectors/sep2787_attestation_v0/normative/hs256_digest_identity/runtime_args.jsontests/vectors/sep2787_attestation_v0/normative/neg_args_mismatch/attestation.jsontests/vectors/sep2787_attestation_v0/normative/neg_args_mismatch/expected.jsontests/vectors/sep2787_attestation_v0/normative/neg_args_mismatch/runtime_args.jsontests/vectors/sep2787_attestation_v0/normative/neg_bad_signature/attestation.jsontests/vectors/sep2787_attestation_v0/normative/neg_bad_signature/expected.jsontests/vectors/sep2787_attestation_v0/normative/neg_bad_signature/runtime_args.jsontests/vectors/sep2787_attestation_v0/normative/neg_expired/attestation.jsontests/vectors/sep2787_attestation_v0/normative/neg_expired/expected.jsontests/vectors/sep2787_attestation_v0/normative/neg_expired/runtime_args.jsontests/vectors/sep2787_attestation_v0/normative/rs256_signature_ttl_only/attestation.jsontests/vectors/sep2787_attestation_v0/normative/rs256_signature_ttl_only/expected.json
Comment on lines
+104
to
+110
| resp = self._post(payload, timeout=timeout) | ||
| try: | ||
| self._capture_session(resp) | ||
| if self._is_event_stream(resp): | ||
| response = self._reply_from_sse(resp, payload["id"]) | ||
| else: | ||
| response = self._reply_from_json(resp) |
There was a problem hiding this comment.
Reject unsupported POST reply media types.
The non-SSE branch currently treats every other 2xx response as JSON-RPC, so a text/plain or application/problem+json body will be accepted as long as it parses. This transport only advertises application/json or text/event-stream, so other Content-Types should fail fast instead of silently bypassing conformance checks.
Suggested fix
try:
self._capture_session(resp)
if self._is_event_stream(resp):
response = self._reply_from_sse(resp, payload["id"])
+ elif self._is_json_response(resp):
+ response = self._reply_from_json(resp)
else:
- response = self._reply_from_json(resp)
+ raise ProxyError(
+ "Upstream MCP server replied with unsupported Content-Type "
+ f"{resp.headers.get('Content-Type')!r}"
+ )
finally:
resp.close()
@@
`@staticmethod`
def _is_event_stream(resp: Any) -> bool:
return "text/event-stream" in (resp.headers.get("Content-Type") or "").lower()
+
+ `@staticmethod`
+ def _is_json_response(resp: Any) -> bool:
+ return "application/json" in (resp.headers.get("Content-Type") or "").lower()Also applies to: 197-220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vaara/integrations/_mcp_upstream_http.py` around lines 104 - 110, When
handling POST responses, only accept non-SSE replies whose Content-Type is
exactly application/json (ignoring parameters like charset); after resp =
self._post(...) and before calling _reply_from_json, parse
resp.headers.get("Content-Type", "") splitting on ';' and lower()-strip the
media type and raise/return an error if it is not "application/json". Apply the
same Content-Type validation in the other POST-reply handling block referenced
around lines 197-220 so both paths validate media types before calling
_reply_from_json; keep the existing _is_event_stream and _reply_from_sse logic
unchanged for text/event-stream.
Comment on lines
+152
to
+171
| def _accept_satisfies(accept: Optional[str], media_type: str) -> bool: | ||
| """True iff an Accept header value can receive ``media_type``. | ||
|
|
||
| A missing or blank header states no preference and is accepted. A | ||
| present header satisfies ``media_type`` when it lists ``*/*``, the | ||
| matching type wildcard (e.g. ``application/*``), or the exact type. | ||
| Wildcard-aware where a literal substring check would not be. | ||
| """ | ||
| if not accept or not accept.strip(): | ||
| return True | ||
| main_type = media_type.split("/", 1)[0] | ||
| tokens = { | ||
| token.strip().split(";", 1)[0].strip().lower() | ||
| for token in accept.split(",") | ||
| } | ||
| return ( | ||
| "*/*" in tokens | ||
| or f"{main_type}/*" in tokens | ||
| or media_type.lower() in tokens | ||
| ) |
There was a problem hiding this comment.
Honor q=0 in the Accept check.
Line 164 drops parameters entirely, so application/json;q=0 or */*;q=0 still count as acceptable. That lets POST /mcp proceed even when the client explicitly rejected one of the two required response types.
Suggested fix
def _accept_satisfies(accept: Optional[str], media_type: str) -> bool:
@@
if not accept or not accept.strip():
return True
main_type = media_type.split("/", 1)[0]
- tokens = {
- token.strip().split(";", 1)[0].strip().lower()
- for token in accept.split(",")
- }
- return (
- "*/*" in tokens
- or f"{main_type}/*" in tokens
- or media_type.lower() in tokens
- )
+ wanted = media_type.lower()
+ for token in accept.split(","):
+ parts = [part.strip() for part in token.split(";")]
+ candidate = parts[0].lower()
+ q = 1.0
+ for param in parts[1:]:
+ if param.lower().startswith("q="):
+ try:
+ q = float(param.split("=", 1)[1])
+ except ValueError:
+ q = 0.0
+ break
+ if q <= 0:
+ continue
+ if candidate in {"*/*", f"{main_type}/*", wanted}:
+ return True
+ return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vaara/integrations/mcp_proxy.py` around lines 152 - 171, The
_accept_satisfies function currently ignores Accept parameters like q=0; update
it to parse each token's parameters and treat any token with q=0 as
unacceptable. In _accept_satisfies (params accept, media_type) change the tokens
comprehension to parse each token's params (split on ";" then parse a q value if
present), skip tokens whose q parsed as 0 (or 0.0), and only include type tokens
with q>0; then continue matching "*/*", "{main_type}/*", or the exact media_type
as before, handling invalid/missing q as q=1 and using lowercasing for
comparisons.
Comment on lines
574
to
596
| session_value = (mcp_session_id or "").strip() | ||
| # Cap session id length to keep the inflight-progress and HttpRouter | ||
| # session-map keys bounded against a malicious client that submits | ||
| # an absurdly long header. 128 chars is comfortably wider than any | ||
| # realistic cryptographically-random session id. | ||
| if len(session_value) > 128: | ||
| if len(session_value) > _MCP_SESSION_ID_MAX_LEN: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={"error": { | ||
| "code": "session_id_too_long", | ||
| "message": "Mcp-Session-Id must be 128 characters or fewer", | ||
| "message": ( | ||
| f"Mcp-Session-Id must be {_MCP_SESSION_ID_MAX_LEN} " | ||
| "characters or fewer" | ||
| ), | ||
| }}, | ||
| ) | ||
| if not _session_id_is_visible_ascii(session_value): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={"error": { | ||
| "code": "session_id_invalid", | ||
| "message": ( | ||
| "Mcp-Session-Id must contain only visible ASCII " | ||
| "characters (0x21-0x7E)" | ||
| ), | ||
| }}, | ||
| ) |
There was a problem hiding this comment.
Validate Mcp-Session-Id verbatim instead of trimming it.
The current .strip() turns " sess-1" and "sess-1 " into "sess-1" instead of rejecting them. That bypasses the visible-ASCII rule and can collapse two distinct wire values onto the same SSE session/router entry.
Suggested fix
- session_value = (mcp_session_id or "").strip()
+ session_value = mcp_session_id or ""
@@
- session_value = (mcp_session_id or "").strip()
+ session_value = mcp_session_id or ""Also applies to: 637-667
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vaara/integrations/mcp_proxy.py` around lines 574 - 596, Summary: Don't
strip Mcp-Session-Id before validation; validate the raw header value verbatim.
Replace the .strip() usage so session_value = (mcp_session_id or "") (no
.strip()) and then perform the length check against _MCP_SESSION_ID_MAX_LEN and
the visible-ASCII check via _session_id_is_visible_ascii(session_value); ensure
you raise the same HTTPException details when invalid. Apply the identical
change to the other validation block that uses
session_value/_MCP_SESSION_ID_MAX_LEN/_session_id_is_visible_ascii later in the
file so both header validations treat leading/trailing spaces as invalid
distinct values.
CodeQL security-and-quality flagged the three `...` bodies in the UpstreamClient Protocol as ineffectual statements (py/ineffectual-statement); each method already carries a docstring, so the ellipsis was redundant. Also documents the best-effort resp.close() in the SSE listener's finally block (py/empty-except) so the swallow is intentional and explained. Behaviour-neutral: Protocol bodies are never executed, and the close path already ignored errors. Full suite 1032 passed, ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
2 participants
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.
v0.45.0
Three changes, all on
feat/v045-upstream-url.Remote
--upstream-urlconnectorThe proxy can now front a remote MCP server over Streamable HTTP, not only a local stdio subprocess.
--upstream-url NAME=URL(a bare URL lands underdefault), with static-header auth via--upstream-header NAME=HEADER. It speaks the 2025-03-26 and 2025-06-18 revisions: POST JSON-RPC,application/jsonortext/event-streamreplies, session-id capture and echo, protocol-version negotiation, and a standing GET SSE channel withLast-Event-IDresume and bounded reconnect. Standard-libraryurllibonly, so the zero-dependency core is untouched (httpx is not pulled in; fastapi and uvicorn stay behind theserverextra). The deprecated 2024-11-05 two-endpoint transport and interactive OAuth are out of scope.A concurrency bug surfaced and was fixed along the way: the first
close()could close the in-flight GET response from the main thread while the listener thread was still blocked reading it, deadlocking on the BufferedReader lock. The fix never closes the response across threads and bounds the listener read with a socket timeout, so a flag-onlyclose()is observed within the window.In-repo SEP-2787 attestation vectors
tests/vectors/sep2787_attestation_v0/now carries the attestation conformance vectors that previously lived only on the fork PR: pinned HS256, ES256, and RS256 keys, six cases across the signature, TTL, and args-commitment dimensions, a standard-library-only checker that imports no Vaara code, a generator, and a pytest cross-check against the library verifier.docs/sep2787-conformance.mdflips from a planned follow-up to the in-repo vectors.Streamable HTTP conformance
Three gaps in the HTTP transport closed: the session-id is validated as visible ASCII on POST and GET,
MCP-Protocol-Versionis validated against the supported set (absent assumed 2025-03-26, unsupported returns 400), and the POSTAcceptcheck is wildcard-aware and requires both JSON and SSE, so*/*and an absent header still pass, existing clients are unaffected, and violations return 406.Ship gate
Full suite green, ruff clean. Version bumped to 0.45.0 across
pyproject.toml,src/vaara/__init__.py,clients/ts/package.json, the marketplace ref, and bothserver.jsonmanifests, including the two MCP manifests that were missed in the v0.44 release and needed a follow-up PR.Summary by CodeRabbit
New Features
--upstream-urland--upstream-headerCLI flags to connect to remote MCP servers over HTTP with server-initiated notification support.Bug Fixes
Tests
Chores