From 7dbcd6db84b9a0a13d037281c84881d0d6a63654 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:22:11 -0700 Subject: [PATCH 1/6] fix(mcp): an unparseable upstream result envelope is response_rejected, not delivery_uncertain A tools/call response the upstream sent but whose envelope the MCP SDK cannot parse (content a string, isError not a bool, ...) ended as delivery_uncertain (504 / -32005, counted by the circuit breaker) through a real SDK session, though _call_tool_once intends response_rejected. The SDK's anyio task group rewraps anything raised inside the session block in an ExceptionGroup, so the adapter's ValidationError branch never matched and the generic handler classified the call by dispatch_started alone. Tests that fake the session do not enter the task group, which is why they passed. _call_tool_once now records the rejection at the call_tool await, where the SDK raises ValidationError only after the upstream's JSON-RPC result has arrived, before the session unwinds. The call ends as response_rejected with code upstream_response_invalid (502 / -32006): debit kept, never redispatched, replay returns the same receipt. Precedence is unchanged: claim contention, guard rejection, a known received result, this rejection, then delivery_uncertain only when no response was obtained. A call with no response (timeout, dropped connection, JSON-RPC error answering tools/call) and a malformed initialize answer keep their outcomes. The ValidationError branch also lacked the "a result is already in hand" guard its two sibling handlers have, so a ValidationError or JSONDecodeError raised only while the session closes could turn a received result into response_rejected or replace a known rejection's code. It has the guard now. Behavior change: a malformed envelope no longer counts toward the optional circuit breaker, like the other response_rejected causes. Tests drive a real SDK session behind the real adapter (in memory and over the production Streamable HTTP client), including a fixture guard that the envelope reaches the client and fails there, the no-response boundary, a secret-echo check, and a governed-router receipt test (one debit, one dispatch, a response_rejected receipt, no redispatch on replay, breaker state). Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 43 ++- app/services/upstream_mcp.py | 30 ++- docs/failure-semantics.md | 18 +- docs/mcp-security-posture.md | 54 ++-- tests/test_mcp_upstream_governed.py | 218 ++++++++++++++- tests/test_upstream_mcp.py | 395 +++++++++++++++++++++++++++- 6 files changed, 724 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11b9fb05..10c65a7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,10 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 error answering `tools/call`) is still `delivery_uncertain`; a tool-level `isError` result is still a refunded `returned_error`; oversized or bearer-echoing results are still `response_rejected`. No new outcome, no - schema or API change. **Not fixed here (follow-up):** a response whose - envelope the SDK cannot parse still ends as `delivery_uncertain` through a - real SDK session, not `response_rejected` (see - [`docs/mcp-security-posture.md`](docs/mcp-security-posture.md) §3.2). + schema or API change. - The bypass uses a private SDK attribute. If an `mcp` release drops it, the call fails closed before the dispatch claim (generic pre-dispatch failure, logged as `upstream_mcp_sdk_result_validation_not_disabled`) instead of @@ -51,6 +48,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [`docs/mcp-security-posture.md`](docs/mcp-security-posture.md) §3.2 and [`docs/failure-semantics.md`](docs/failure-semantics.md). +### Fixed — an upstream response the SDK cannot parse is `response_rejected` + +- A `tools/call` response the upstream sent but whose envelope the MCP SDK + cannot parse (for example `content` that is a string, or a non-boolean + `isError`) ended as `delivery_uncertain` (504 / `-32005`) through a real SDK + session, although `_call_tool_once` intended `response_rejected`. The SDK's + anyio task group rewraps anything raised inside the session block in an + `ExceptionGroup`, so the adapter's `ValidationError` branch never matched and + the generic handler classified the call by `dispatch_started` alone. Tests + that fake the session do not enter the task group, which is why they passed. +- `_call_tool_once` now records the rejection where the `ValidationError` is + raised (the `call_tool` await, which the SDK reaches only after the upstream's + JSON-RPC result has arrived), before the session unwinds. The call ends as + `response_rejected` with code `upstream_response_invalid` (502 / `-32006`): + the debit stays, the call is never redispatched, and a replay returns the same + receipt. Existing precedence is unchanged: claim contention, then a transport + guard rejection, then a known received result, then this rejection, then + `delivery_uncertain` only when no response was obtained. +- The `ValidationError` branch also lacked the "a result is already in hand" + guard the other handlers have, so a `ValidationError` or `JSONDecodeError` + raised only while the session closes could turn a received result into + `response_rejected`, or replace a known rejection's code. It now has the guard. +- **Behavior change:** a malformed envelope is no longer counted toward the + optional circuit breaker. The `response_rejected` path never fed it (an + oversized or bearer-echoing result did not either); `delivery_uncertain` does. +- Unchanged: a call with no response (connection dropped, timeout, JSON-RPC + error answering `tools/call`) is still `delivery_uncertain`; a malformed + `initialize` answer is still a pre-dispatch failure. Not covered: an upstream + body that is not a JSON-RPC message at all is dropped inside the SDK + transport, so that call times out and ends as `delivery_uncertain`. +- New tests drive a real SDK session behind the real adapter, in memory and over + the production Streamable HTTP client, with four envelope shapes, a fixture + guard (the envelope must reach the client and fail there), the no-response + boundary, a secret-echo check, and a governed-router receipt test (one debit, + one dispatch, a `response_rejected` receipt, no redispatch on replay). See + [`docs/mcp-security-posture.md`](docs/mcp-security-posture.md) §3.2 and + [`docs/failure-semantics.md`](docs/failure-semantics.md). + ### Added — per-upstream concurrency limit and circuit breaker (default off) - `UPSTREAM_MAX_CONCURRENCY_PER_UPSTREAM` (default `0`, unlimited) caps diff --git a/app/services/upstream_mcp.py b/app/services/upstream_mcp.py index 90f04729..89f21742 100644 --- a/app/services/upstream_mcp.py +++ b/app/services/upstream_mcp.py @@ -1025,7 +1025,10 @@ async def _call_tool_once( classified as delivery-uncertain. A well-formed result that was received is not: SDK-side output-schema validation is disabled for the call (see ``_bypass_sdk_result_validation``), so only this adapter's own shape, - size, and secret checks can reject it. + size, and secret checks can reject it. A response the SDK itself cannot + parse is rejected too, not uncertain: the upstream did answer. That + failure is recorded at the ``call_tool`` await because the SDK's task + group rewraps anything that leaves the session block. """ if self._discovered_tool is None: raise UpstreamMcpPreDispatchError("upstream_tool_not_initialized") @@ -1081,6 +1084,16 @@ async def _call_tool_once( canonical_result = self._canonicalize_result(result) except UpstreamMcpResponseRejectedError as exc: response_rejected = exc + except ValidationError: + # The SDK validates the result envelope after the upstream + # has answered; timeouts, dropped connections and JSON-RPC + # errors raise other types. So this is a response received + # but unparseable, which is not delivery uncertainty. Record + # it here: leaving the session wraps it in an + # ExceptionGroup that no handler below can match. + response_rejected = UpstreamMcpResponseRejectedError( + "upstream_response_invalid" + ) except UpstreamMcpError: if dispatch_claim_unavailable is not None: # A context-manager cleanup failure can itself use one of our @@ -1119,11 +1132,22 @@ async def _call_tool_once( raise UpstreamMcpPreDispatchError( response_guard.rejection_code ) from None - if dispatch_started: + if canonical_result is not None or response_rejected is not None: + # As below: a close-time parse failure cannot change a + # response that is already known. + logger.warning( + "upstream_mcp_session_cleanup_failed", + extra={ + "public_tool_id": self.configuration.public_tool_id, + "upstream_origin": self.configuration.origin, + }, + ) + elif dispatch_started: raise UpstreamMcpResponseRejectedError( "upstream_response_invalid" ) from None - raise UpstreamMcpPreDispatchError() from None + else: + raise UpstreamMcpPreDispatchError() from None except Exception: if dispatch_claim_unavailable is not None: # Session cleanup must never erase expected claim contention: diff --git a/docs/failure-semantics.md b/docs/failure-semantics.md index 0307038e..0d85718f 100644 --- a/docs/failure-semantics.md +++ b/docs/failure-semantics.md @@ -76,7 +76,7 @@ isolation regressions are in `tests/test_permit_effective_lifecycle.py`. | `insufficient_funds` | The charge itself was refused (balance, child-wallet cap, daily limit) | Never debited; reservation released | No gateway dispatch | 402 / `-32004` | `test_insufficient_funds_returns_receipt_and_replays_without_charge` | | `failed_refunded` | A **confirmed** failure: local tool raised; upstream returned `isError: true`; DNS/TLS/`initialize` failed before the remote dispatch claim; charge refused on the upstream path; or a crash before dispatch, finalized by the reconciler | Debit refunded (or never taken). The receipt signs `credits_charged = 0`; the ledger separately proves the correlated refund | An `isError` response may have reached the gateway; pre-claim cases were not dispatched; no downstream effect is independently proven | 500 or 502 / `-32006` (402/403 for refused charges) | Pre-claim failure, guarded completion, and reconciliation tests in `tests/test_mcp_upstream_governed.py`, `tests/test_mcp_dispatch_router_claim.py`, and `tests/test_mcp_dispatch_reconciliation.py` | | `delivery_uncertain` | The one-shot remote dispatch claim was committed, then a timeout, transport failure, or process death left the outcome unknowable | **Stays charged.** Never redispatched | **Unknown** — that is the definition | 504 / `-32005` | Stale-claim reconciliation and PostgreSQL process-kill cases | -| `response_rejected` | The upstream **did** respond, but the response is unusable: invalid shape, reflected bearer token, non-serializable, or over the byte cap (wire-level or at persistence). A result that merely disagrees with the upstream's advertised output schema is **not** unusable: it is a `success` | **Stays charged** | A response reached the gateway; the downstream effect is not independently proven | 502 / `-32006` | `test_confirmed_result_rejected_by_persistence_is_terminal_and_replayable`, `test_terminal_response_rejected_retains_charge_and_is_replayable` | +| `response_rejected` | The upstream **did** respond, but the response is unusable: invalid shape (including an envelope the MCP SDK cannot parse), reflected bearer token, non-serializable, or over the byte cap (wire-level or at persistence). A result that merely disagrees with the upstream's advertised output schema is **not** unusable: it is a `success` | **Stays charged** | A response reached the gateway; the downstream effect is not independently proven | 502 / `-32006` | `test_confirmed_result_rejected_by_persistence_is_terminal_and_replayable`, `test_terminal_response_rejected_retains_charge_and_is_replayable`, `test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch` | | `failed_unrefunded` | A refund was owed and the refund **itself** failed | Charged; a durable operator work item is created atomically with the receipt | Depends on the underlying failure | 500 / `-32603` | `test_governed_refund_failure_keeps_permit_budget_reserved`, `test_refund_reconciliation_retries_exactly_once_and_preserves_agent_replay` | A well-formed tool result the gateway **received** is never `delivery_uncertain`. The @@ -86,13 +86,15 @@ a hostile `$ref` cannot turn a completed call into an ambiguous one; only the adapter's own shape, size, and secret checks can reject a received result ([`mcp-security-posture.md`](mcp-security-posture.md) §3.2). Conversely, `delivery_uncertain` is for calls where no result was obtained: the connection -dropped, the request timed out, or the process died after the claim. Known gap, -not changed here: a response whose envelope the SDK itself cannot parse still -ends as `delivery_uncertain` through a real SDK session instead of -`response_rejected` -([`mcp-security-posture.md`](mcp-security-posture.md) §3.2, "Still open"). -The drifted-schema case is proven end to end by -`test_completed_call_with_drifted_live_schema_is_a_charged_success_end_to_end`. +dropped, the request timed out, or the process died after the claim. A response +the gateway received but the SDK cannot parse (for example `content` that is not +a list) is `response_rejected`, not `delivery_uncertain`: the upstream answered, +so the outcome is known and unusable +([`mcp-security-posture.md`](mcp-security-posture.md) §3.2). The drifted-schema +case is proven end to end by +`test_completed_call_with_drifted_live_schema_is_a_charged_success_end_to_end`, +and the unparseable-envelope case, next to its `delivery_uncertain` boundary, by +`test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch`. Requests rejected before a valid permit and an executable tool are established produce no receipt — there is no authority to bill against. The README states diff --git a/docs/mcp-security-posture.md b/docs/mcp-security-posture.md index 0a608043..d6aa9a15 100644 --- a/docs/mcp-security-posture.md +++ b/docs/mcp-security-posture.md @@ -302,21 +302,45 @@ a replay that sends nothing). With the one call that applies the fix disabled, every one of these fails except the two boundary tests, which hold either way, and the end-to-end test returns `-32005 delivery_uncertain`. -**Still open, a neighbouring defect found while verifying this fix (not -changed).** A tool response whose *envelope* the SDK cannot parse (for example -`content` that is a string, or a non-boolean `isError`) is meant to end as -`response_rejected`: `_call_tool_once` has a `ValidationError` branch for it. -Through a real SDK session it ends as `delivery_uncertain` instead, because the -SDK's task group rewraps the `ValidationError` in an `ExceptionGroup` before -that branch can match. Probed 2026-10-22 against an in-memory SDK server, with -and without the fix above, with identical results. The charge stays either -way; what differs is the receipt outcome (504 / `-32005`, counted by the -circuit breaker, versus a terminal 502 / `-32006`). A fix has to unwrap the -group in `_call_tool_once`'s handlers and is its own change with its own tests. -A second, narrower gap in the same function: its `ValidationError` branch lacks -the "a result is already in hand" guard that the two sibling handlers have, so a -`ValidationError` raised only while the session closes would turn a received -result into `response_rejected`. +**A neighbouring defect, fixed in the same series: a response the SDK cannot +parse.** A `tools/call` response whose *envelope* the SDK cannot parse (for +example `content` that is a string, or a non-boolean `isError`) is +`response_rejected` (`upstream_response_invalid`, 502 / `-32006`): charged, +terminal, replay-stable. It used to end as `delivery_uncertain` (504 / `-32005`, +counted by the circuit breaker). The SDK parses the envelope inside +`send_request`, and the anyio task group around the session rewraps that +`ValidationError` in an `ExceptionGroup` as the session block exits, so the +`ValidationError` branch of `_call_tool_once` never matched and the generic +handler classified it by `dispatch_started` alone. The adapter now records the +rejection where the `ValidationError` is raised, at the `call_tool` await, +before the session unwinds. That point is reached only after the upstream's +JSON-RPC result has arrived (a timeout, a dropped connection and a JSON-RPC +error answering `tools/call` raise other types), so a failure with no response +is still `delivery_uncertain`, and a malformed answer to `initialize` is still a +pre-dispatch failure. The same change gives the `ValidationError` branch the +"a result is already in hand" guard its two sibling handlers have, so a +`ValidationError` or `JSONDecodeError` raised only while the session closes can +no longer replace a response the adapter already holds. One consequence: a +malformed envelope is no longer counted by the circuit breaker, because the +`response_rejected` path does not feed it (an oversized or bearer-echoing result +never did either). + +Evidence, real SDK session behind the real adapter (in memory, and over the +production Streamable HTTP client against a scripted transport): +`test_unparseable_result_envelope_is_response_rejected_not_delivery_uncertain` +and `test_unparseable_result_envelope_over_streamable_http_is_response_rejected` +(four envelope shapes each); `test_the_sdk_client_itself_rejects_each_envelope_fixture` +(the fixtures reach the client and fail there, not on a crashed server); +`test_unparseable_envelope_error_does_not_echo_upstream_content`; the boundary +pins `test_no_response_to_tools_call_stays_delivery_uncertain_in_memory`, +`test_transport_failure_over_streamable_http_stays_delivery_uncertain` and +`test_unparseable_initialize_response_stays_pre_dispatch`; and, through the +governed router, `test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch` +(one debit, one dispatch, a `response_rejected` receipt, a replay that sends +nothing; the JSON-RPC-error case stays `delivery_uncertain`). Residual case not +covered: an upstream body that is not JSON-RPC at all (not parseable as a +message) is dropped by the SDK transport without reaching the caller, so that +call times out and ends as `delivery_uncertain`. **Still open, drift detection.** The gateway checks results against no output schema. If a customer needs drift detection, it belongs in the adapter, against the diff --git a/tests/test_mcp_upstream_governed.py b/tests/test_mcp_upstream_governed.py index 66ae7321..18f9c171 100644 --- a/tests/test_mcp_upstream_governed.py +++ b/tests/test_mcp_upstream_governed.py @@ -19,7 +19,7 @@ from sqlalchemy import func, select from app.routers import mcp as mcp_router -from app.core.config import Settings +from app.core.config import Settings, get_settings from app.core.time import utc_now from app.db.database import get_session_factory from app.db.models import ( @@ -54,6 +54,7 @@ from app.services.receipts import ReceiptService, get_receipt_service from app.services.service_registry import get_service_registry from app.services.signing_keys import sha256_hex +from app.services.upstream_admission import clear_upstream_admission_state from app.services.upstream_mcp import ( DiscoveredUpstreamTool, UpstreamMcpAdapter, @@ -748,6 +749,221 @@ async def session_context(**_kwargs: Any) -> AsyncIterator[Any]: get_service_registry().unregister_local(tool_name) +def _adapter_over_real_sdk_server(server: Server, tool_name: str) -> UpstreamMcpAdapter: + """The real adapter over a real in-memory SDK session, for ``tool_name``.""" + adapter = UpstreamMcpAdapter( + UpstreamMcpConfiguration.from_settings( + Settings( + _env_file=None, + ENVIRONMENT="local", + MCP_UPSTREAM_ENABLED=True, + MCP_UPSTREAM_URL="http://localhost:9000/mcp", + MCP_UPSTREAM_TOOL_NAME="partner.echo", + MCP_UPSTREAM_PUBLIC_TOOL_ID=tool_name, + MCP_UPSTREAM_BEARER_TOKEN="secret-partner-token", + MCP_UPSTREAM_CREDITS_PER_CALL="2", + ) + ) + ) + adapter._discovered_tool = DiscoveredUpstreamTool( + name="partner.echo", + description="Controlled remote MCP test tool", + input_schema={"type": "object"}, + output_schema=None, + ) + + @asynccontextmanager + async def session_context(**_kwargs: Any) -> AsyncIterator[Any]: + async with create_connected_server_and_client_session(server) as session: + yield session + + adapter._session = session_context # type: ignore[method-assign] + return adapter + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ( + "upstream_behavior", + "expected_state", + "expected_code", + "expected_attempt_error_code", + "expect_breaker_open", + ), + [ + pytest.param( + "unparseable_envelope", + "response_rejected", + -32006, + "upstream_response_invalid", + False, + id="upstream-answered-with-an-unparseable-envelope", + ), + pytest.param( + "jsonrpc_error", + "delivery_uncertain", + -32005, + "delivery_uncertain", + True, + id="upstream-answered-with-a-jsonrpc-error", + ), + ], +) +async def test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch( + client: AsyncClient, + clean_database: None, + monkeypatch: pytest.MonkeyPatch, + upstream_behavior: str, + expected_state: str, + expected_code: int, + expected_attempt_error_code: str, + expect_breaker_open: bool, +) -> None: + """Receipt-level proof of the unparseable-envelope classification. + + The real adapter runs over a real in-memory SDK upstream. When that + upstream answers ``tools/call`` with an envelope the SDK cannot parse, the + call finalizes as ``response_rejected`` (502 / ``-32006``): one debit kept, + one dispatch, a signed receipt, and a replay that sends nothing. Before the + fix the SDK's task group rewrapped the parse error and the router signed + ``delivery_uncertain`` (504 / ``-32005``) instead. + + The second case is the boundary: an upstream that answers with a JSON-RPC + error gives the adapter no response to judge, so that stays + ``delivery_uncertain``. Both outcomes are charged and replay-stable. With + a threshold-1 circuit breaker, only ``delivery_uncertain`` opens it: the + next call is refused before any dispatch, whereas after a + ``response_rejected`` the next call reaches the upstream. + """ + settings = get_settings() + monkeypatch.setattr(settings, "UPSTREAM_CIRCUIT_BREAKER_ENABLED", True) + monkeypatch.setattr(settings, "UPSTREAM_CIRCUIT_BREAKER_FAILURE_THRESHOLD", 1) + monkeypatch.setattr(settings, "UPSTREAM_CIRCUIT_BREAKER_RECOVERY_SECONDS", 30.0) + clear_upstream_admission_state() + provisioned = await provision_agent_wallet(client) + tool_name = f"partner-upstream-real-sdk-{upstream_behavior.replace('_', '-')}" + idempotency_key = f"{tool_name}-1" + upstream_state = {"calls": 0, "list_calls": 0} + + server = Server("misbehaving-upstream") + + async def list_tools(_req: mcp_types.ListToolsRequest) -> mcp_types.ServerResult: + upstream_state["list_calls"] += 1 + return mcp_types.ServerResult(mcp_types.ListToolsResult(tools=[])) + + async def call_tool(_req: mcp_types.CallToolRequest) -> mcp_types.ServerResult: + upstream_state["calls"] += 1 + if upstream_behavior == "jsonrpc_error": + raise RuntimeError("upstream failed mid-call") + # ``model_construct`` skips validation, so this server serializes an + # envelope that only the CLIENT side fails to parse. + return mcp_types.ServerResult.model_construct( + root=mcp_types.CallToolResult.model_construct( + content="not-a-list", isError=False + ) + ) + + server.request_handlers[mcp_types.ListToolsRequest] = list_tools + server.request_handlers[mcp_types.CallToolRequest] = call_tool + _register_upstream( + tool_name, + _adapter_over_real_sdk_server(server, tool_name), # type: ignore[arg-type] + ) + outcomes_before = get_upstream_mcp_metrics_snapshot()["outcomes"] + try: + permit = await create_tool_permit( + client, + wallet_id=provisioned["agent_wallet_id"], + key_id=provisioned["key_id"], + tool_name=tool_name, + idem_key=f"{tool_name}-permit", + ) + body = _call_body( + tool_name=tool_name, + wallet_id=provisioned["agent_wallet_id"], + permit_id=permit["permit_id"], + idempotency_key=idempotency_key, + ) + + first = await client.post( + "/mcp/messages", json=body, headers=provisioned["agent_headers"] + ) + replay = await client.post( + "/mcp/messages", json=body, headers=provisioned["agent_headers"] + ) + + first_error = first.json()["error"] + assert first_error["code"] == expected_code + assert first_error["message"] == expected_state + assert replay.json()["error"] == first_error + receipt = first_error["data"]["receipt"] + assert receipt["outcome"] == expected_state + assert Decimal(receipt["credits_charged"]) == Decimal("2") + # One dispatch for two requests; no tools/list is ever sent. + assert upstream_state == {"calls": 1, "list_calls": 0} + assert await _wallet_debit_and_canonical_counts( + wallet_id=provisioned["agent_wallet_id"], + idempotency_key=idempotency_key, + ) == (1, 1) + + persisted = await _load_persisted_invocation( + wallet_id=provisioned["agent_wallet_id"], + permit_id=permit["permit_id"], + idempotency_key=idempotency_key, + ) + _assert_linked_terminal_state( + persisted, + state=expected_state, + outcome=expected_state, + charged=Decimal("2"), + refunded=False, + ) + assert persisted.attempt.dispatched_at is not None + assert persisted.attempt.error_code == expected_attempt_error_code + assert first_error["data"]["dispatch"] == { + "attempt_id": persisted.attempt.attempt_id, + "state": expected_state, + } + valid, reason, _ = await get_receipt_service().verify_receipt( + persisted.receipt.receipt_id + ) + assert (valid, reason) == (True, None) + evidence = await client.get( + f"/v1/receipts/{persisted.receipt.receipt_id}/evidence", + headers=provisioned["agent_headers"], + ) + assert evidence.status_code == 200 + assert evidence.json()["valid"] is True + outcomes_after = get_upstream_mcp_metrics_snapshot()["outcomes"] + for outcome in ("response_rejected", "delivery_uncertain"): + assert outcomes_after[outcome] == outcomes_before[outcome] + ( + 1 if outcome == expected_state else 0 + ) + + # A different key, so this is a new call and not a replay. + following = await client.post( + "/mcp/messages", + json=_call_body( + tool_name=tool_name, + wallet_id=provisioned["agent_wallet_id"], + permit_id=permit["permit_id"], + idempotency_key=f"{idempotency_key}-following", + ), + headers=provisioned["agent_headers"], + ) + following_error = following.json()["error"] + if expect_breaker_open: + assert following_error["message"] == "upstream_circuit_open" + assert "receipt" not in following_error["data"] + assert upstream_state["calls"] == 1 + else: + assert following_error["message"] == expected_state + assert upstream_state["calls"] == 2 + finally: + clear_upstream_admission_state() + get_service_registry().unregister_local(tool_name) + + @pytest.mark.anyio async def test_approval_required_upstream_crash_reconciliation_keeps_approval( client: AsyncClient, diff --git a/tests/test_upstream_mcp.py b/tests/test_upstream_mcp.py index 14645d51..66e908db 100644 --- a/tests/test_upstream_mcp.py +++ b/tests/test_upstream_mcp.py @@ -19,8 +19,17 @@ from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.x509.oid import ExtendedKeyUsageOID, NameOID from mcp.server.fastmcp import Context, FastMCP +from mcp.server.lowlevel import Server from mcp.server.transport_security import TransportSecuritySettings -from mcp.types import CallToolResult, ListToolsResult, TextContent, Tool +from mcp.shared.memory import create_connected_server_and_client_session +from mcp.types import ( + CallToolRequest, + CallToolResult, + ListToolsResult, + ServerResult, + TextContent, + Tool, +) from pydantic import ValidationError from app.core.config import Settings @@ -1259,8 +1268,15 @@ async def before_dispatch() -> None: [ UpstreamMcpPreDispatchError("secret-partner-token"), UpstreamMcpDeliveryUncertainError(), + _cleanup_validation_error(), + json.JSONDecodeError("malformed cleanup payload", "{", 1), + ], + ids=[ + "typed-pre-dispatch", + "typed-delivery-uncertain", + "validation-error", + "json-decode-error", ], - ids=["typed-pre-dispatch", "typed-delivery-uncertain"], ) async def test_typed_cleanup_failure_does_not_replace_known_response_rejection( cleanup_error: Exception, @@ -1300,8 +1316,16 @@ async def before_dispatch() -> None: RuntimeError("secret-partner-token"), UpstreamMcpPreDispatchError("secret-partner-token"), UpstreamMcpDeliveryUncertainError(), + _cleanup_validation_error(), + json.JSONDecodeError("malformed cleanup payload", "{", 1), + ], + ids=[ + "untyped", + "typed-pre-dispatch", + "typed-delivery-uncertain", + "validation-error", + "json-decode-error", ], - ids=["untyped", "typed-pre-dispatch", "typed-delivery-uncertain"], ) async def test_cleanup_failure_does_not_replace_confirmed_response( cleanup_error: Exception, @@ -1604,3 +1628,368 @@ async def test_disabled_startup_helper_unregisters_previous_upstream() -> None: ) assert result is None assert registry.get_local("partner.notes.write") is None + + +# --------------------------------------------------------------------------- +# Error classification through a real SDK session +# +# ``FakeSession`` never enters the MCP SDK's anyio task group, so it cannot show +# what the adapter sees when something fails inside ``async with self._session()``: +# the SDK leaves that block as an ExceptionGroup, never as the bare exception. +# The tests below use a real ``ClientSession``, either in memory or over the +# production Streamable HTTP client against a scripted transport. +# --------------------------------------------------------------------------- + +# Each of these is well-formed JSON-RPC that the SDK's ``CallToolResult`` model +# refuses to parse, so the upstream demonstrably answered. +UNPARSEABLE_ENVELOPES = [ + pytest.param({"content": "not-a-list", "isError": False}, id="content-is-a-string"), + pytest.param({"content": [], "isError": {"x": 1}}, id="is-error-is-not-a-bool"), + pytest.param( + {"content": [{"type": "text"}], "isError": False}, + id="content-block-missing-its-text", + ), + pytest.param( + {"content": [], "structuredContent": ["not", "an", "object"], "isError": False}, + id="structured-content-is-not-an-object", + ), +] + + +def _envelope_server(envelope: dict[str, Any], served: list[str]) -> Server: + """A real SDK server whose ``tools/call`` answer the CLIENT cannot parse. + + ``model_construct`` skips validation, so the server serializes the invalid + envelope and puts it on the wire. A payload the server cannot serialize at + all crashes the server instead, so no response is ever sent; that is a + different case, and ``delivery_uncertain`` is right for it. + """ + server = Server("unparseable-envelope") + + async def call_tool(_req: CallToolRequest) -> ServerResult: + served.append("tools/call") + return ServerResult.model_construct( + root=CallToolResult.model_construct(**envelope) + ) + + server.request_handlers[CallToolRequest] = call_tool + return server + + +def _adapter_over_in_memory_sdk(server: Server, **settings: Any) -> UpstreamMcpAdapter: + adapter = UpstreamMcpAdapter(_configuration(**settings)) + _mark_discovered(adapter) + + @asynccontextmanager + async def session_context(**_kwargs: Any) -> AsyncIterator[Any]: + async with create_connected_server_and_client_session(server) as session: + yield session + + adapter._session = session_context # type: ignore[method-assign] + return adapter + + +async def _call_recording_claims( + adapter: UpstreamMcpAdapter, claims: list[str] +) -> None: + """Run one call; ``claims`` records each time ``before_dispatch`` ran.""" + + async def before_dispatch() -> None: + claims.append("claimed") + + await adapter.call_tool( + {}, + invocation_id="inv", + idempotency_key="idem", + before_dispatch=before_dispatch, + ) + + +@pytest.mark.anyio +@pytest.mark.parametrize("envelope", UNPARSEABLE_ENVELOPES) +async def test_the_sdk_client_itself_rejects_each_envelope_fixture( + envelope: dict[str, Any], +) -> None: + """Guards the fixtures: the bytes must reach the client and fail THERE. + + Without this a fixture that crashed the server while serializing would pass + every classification test below for the wrong reason, because "nothing was + sent" also ends in ``delivery_uncertain``. + """ + served: list[str] = [] + async with create_connected_server_and_client_session( + _envelope_server(envelope, served) + ) as session: + with pytest.raises(ValidationError): + await session.call_tool("partner.write", {}) + + assert served == ["tools/call"] + + +@pytest.mark.anyio +@pytest.mark.parametrize("envelope", UNPARSEABLE_ENVELOPES) +async def test_unparseable_result_envelope_is_response_rejected_not_delivery_uncertain( + envelope: dict[str, Any], +) -> None: + """The upstream answered but the SDK cannot parse the answer. + + The call was dispatched and the response is known to be unusable. That is + ``response_rejected`` (terminal, charged, replayable), not + ``delivery_uncertain``, which is reserved for "no response was obtained". + """ + served: list[str] = [] + adapter = _adapter_over_in_memory_sdk(_envelope_server(envelope, served)) + metrics_before = get_upstream_mcp_metrics_snapshot()["outcomes"] + + claims: list[str] = [] + with pytest.raises(UpstreamMcpResponseRejectedError) as exc_info: + await _call_recording_claims(adapter, claims) + + assert exc_info.value.code == "upstream_response_invalid" + assert exc_info.value.dispatch_started is True + assert claims == ["claimed"] + assert served == ["tools/call"] + metrics_after = get_upstream_mcp_metrics_snapshot()["outcomes"] + assert metrics_after["response_rejected"] == metrics_before["response_rejected"] + 1 + assert metrics_after["delivery_uncertain"] == metrics_before["delivery_uncertain"] + + +@pytest.mark.anyio +async def test_unparseable_envelope_error_does_not_echo_upstream_content( + caplog: pytest.LogCaptureFixture, +) -> None: + """Pydantic's message quotes the offending value, so it must not escape. + + A hostile upstream can put the bearer token (or anything else) into the + field that fails to parse. The raised error carries only a fixed code and + no chained exception. + """ + served: list[str] = [] + adapter = _adapter_over_in_memory_sdk( + _envelope_server({"content": "secret-partner-token", "isError": False}, served) + ) + + claims: list[str] = [] + with pytest.raises(UpstreamMcpResponseRejectedError) as exc_info: + await _call_recording_claims(adapter, claims) + + assert served == ["tools/call"] + assert "secret-partner-token" not in str(exc_info.value) + assert "secret-partner-token" not in repr(exc_info.value) + assert exc_info.value.__cause__ is None + assert exc_info.value.__context__ is None + assert "secret-partner-token" not in caplog.text + + +@pytest.mark.anyio +@pytest.mark.parametrize("failure", ["jsonrpc-error", "no-answer"]) +async def test_no_response_to_tools_call_stays_delivery_uncertain_in_memory( + failure: str, +) -> None: + """The boundary of the fix: no response obtained means no known outcome. + + A JSON-RPC error answering ``tools/call`` and an upstream that never + answers within the call timeout both leave the adapter unable to say + whether the tool ran. Neither is a parse failure, so neither may become + ``response_rejected``. + """ + served: list[str] = [] + server = Server("no-usable-response") + + async def call_tool(_req: CallToolRequest) -> ServerResult: + served.append("tools/call") + if failure == "jsonrpc-error": + raise RuntimeError("upstream failed mid-call") + await asyncio.Event().wait() + raise AssertionError("unreachable") + + server.request_handlers[CallToolRequest] = call_tool + adapter = _adapter_over_in_memory_sdk( + server, MCP_UPSTREAM_CALL_TIMEOUT_SECONDS="0.3" + ) + metrics_before = get_upstream_mcp_metrics_snapshot()["outcomes"] + + claims: list[str] = [] + with pytest.raises(UpstreamMcpDeliveryUncertainError) as exc_info: + await _call_recording_claims(adapter, claims) + + assert exc_info.value.dispatch_started is True + assert claims == ["claimed"] + assert served == ["tools/call"] + metrics_after = get_upstream_mcp_metrics_snapshot()["outcomes"] + assert metrics_after["delivery_uncertain"] == ( + metrics_before["delivery_uncertain"] + 1 + ) + assert metrics_after["response_rejected"] == metrics_before["response_rejected"] + + +def _scripted_mcp_client( + *, + on_call: Any = None, + initialize_result: dict[str, Any] | None = None, +) -> httpx.AsyncClient: + """A scripted Streamable HTTP upstream for the production ``_session``. + + ``on_call`` receives the JSON-RPC request id of ``tools/call`` and returns + the ``result`` dict to answer with, returns a ready ``httpx.Response``, or + raises (to simulate a transport failure). + """ + + async def handler(request: httpx.Request) -> httpx.Response: + if request.method != "POST": + return httpx.Response(405) + payload = json.loads((await request.aread()).decode("utf-8")) + request_id = payload.get("id") + method = payload["method"] + if request_id is None: + return httpx.Response(202) + if method == "initialize": + result: Any = initialize_result or { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "serverInfo": {"name": "partner", "version": "1.0"}, + } + elif method == "tools/call": + result = on_call(request_id) + if isinstance(result, httpx.Response): + return result + else: # pragma: no cover - the call lifecycle is deterministic + raise AssertionError(f"unexpected MCP method: {method}") + return httpx.Response( + 200, + headers={"Content-Type": "application/json"}, + content=json.dumps( + {"jsonrpc": "2.0", "id": request_id, "result": result} + ).encode("utf-8"), + ) + + return httpx.AsyncClient( + transport=httpx.MockTransport(handler), + headers={"Authorization": "Bearer secret-partner-token"}, + follow_redirects=False, + ) + + +async def _call_over_scripted_http( + client: httpx.AsyncClient, claims: list[str], **settings: Any +) -> None: + adapter = UpstreamMcpAdapter( + _configuration(MCP_UPSTREAM_URL="https://partner.example/mcp", **settings), + http_client=client, + resolver=_global_resolver, + ) + _mark_discovered(adapter) + try: + await _call_recording_claims(adapter, claims) + finally: + await client.aclose() + + +@pytest.mark.anyio +@pytest.mark.parametrize("envelope", UNPARSEABLE_ENVELOPES) +async def test_unparseable_result_envelope_over_streamable_http_is_response_rejected( + envelope: dict[str, Any], +) -> None: + """Same classification through the production ``_session`` wiring.""" + client = _scripted_mcp_client(on_call=lambda _request_id: envelope) + + claims: list[str] = [] + with pytest.raises(UpstreamMcpResponseRejectedError) as exc_info: + await _call_over_scripted_http(client, claims) + + assert exc_info.value.code == "upstream_response_invalid" + assert exc_info.value.dispatch_started is True + assert claims == ["claimed"] + + +def _json_rpc_error(request_id: Any) -> httpx.Response: + return httpx.Response( + 200, + headers={"Content-Type": "application/json"}, + content=json.dumps( + { + "jsonrpc": "2.0", + "id": request_id, + "error": {"code": -32000, "message": "upstream failed"}, + } + ).encode("utf-8"), + ) + + +def _connection_dropped(_request_id: Any) -> Any: + raise httpx.ReadError("connection reset by peer") + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "on_call", + [_json_rpc_error, _connection_dropped], + ids=["jsonrpc-error-answering-tools-call", "connection-dropped"], +) +async def test_transport_failure_over_streamable_http_stays_delivery_uncertain( + on_call: Any, +) -> None: + client = _scripted_mcp_client(on_call=on_call) + + claims: list[str] = [] + with pytest.raises(UpstreamMcpDeliveryUncertainError) as exc_info: + await _call_over_scripted_http( + client, claims, MCP_UPSTREAM_CALL_TIMEOUT_SECONDS="1" + ) + + assert exc_info.value.dispatch_started is True + assert claims == ["claimed"] + + +@pytest.mark.anyio +async def test_unparseable_initialize_response_stays_pre_dispatch() -> None: + """The new rejection applies only to the ``tools/call`` response. + + A malformed ``initialize`` answer is raised before the dispatch checkpoint, + so nothing is claimed and nothing is sent. + """ + client = _scripted_mcp_client( + on_call=lambda _request_id: {}, initialize_result={"protocolVersion": 5} + ) + + claims: list[str] = [] + with pytest.raises(UpstreamMcpPreDispatchError) as exc_info: + await _call_over_scripted_http(client, claims) + + assert exc_info.value.dispatch_started is False + assert claims == [] + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "cleanup_error", + [ + RuntimeError("secret-partner-token"), + UpstreamMcpPreDispatchError("secret-partner-token"), + UpstreamMcpDeliveryUncertainError(), + ], + ids=["untyped", "typed-pre-dispatch", "typed-delivery-uncertain"], +) +async def test_cleanup_failure_does_not_replace_unparseable_envelope_rejection( + cleanup_error: Exception, +) -> None: + """A close-time failure must not turn a known rejection into uncertainty. + + Closing the session after an unparseable answer can fail too. The rejection + is already known, so it keeps its outcome and its code. + """ + adapter = UpstreamMcpAdapter(_configuration()) + _mark_discovered(adapter) + _bind_session( + adapter, + FakeSession(call_error=_cleanup_validation_error()), + cleanup_error=cleanup_error, + ) + + claims: list[str] = [] + with pytest.raises(UpstreamMcpResponseRejectedError) as exc_info: + await _call_recording_claims(adapter, claims) + + assert exc_info.value.code == "upstream_response_invalid" + assert exc_info.value.dispatch_started is True + assert "secret-partner-token" not in str(exc_info.value) From 0698168fc4a550b92b35a88e98a0e5395083f594 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:57:28 -0700 Subject: [PATCH 2/6] fix(mcp): /mcp tells the model not to resend a rejected response; reject only unparseable results Two review findings on 7dbcd6db. 1. /mcp (app/routers/mcp_standard.py) turned only delivery_uncertain into a tool result carrying the do-not-resend text. response_rejected, which is also charged (the call was dispatched and the debit stands), became a bare -32006, the code that refunded, retry-safe failures (upstream_returned_error, upstream_pre_dispatch_failed) share. A model that sees only the message could resend a call whose charge stands and whose tool probably ran. response_rejected now returns the same guided result: isError true, text saying the upstream answered but the answer was unusable and the effect is unknown, the signed receipt, and _meta["io.agentmiddleware/outcome"] with status "unknown" and reason "response_rejected". The gateway outcome recorded for insights stays "failed", as it was on the JSON-RPC channel. Refunded failures stay bare errors; /mcp/messages and REST are unchanged. 2. The inner except ValidationError in _call_tool_once also caught a pydantic error raised while the SDK builds the request, before anything is sent (arguments with a non-string key: the claim is committed, no tools/call reaches the upstream, and the call was labeled response_rejected, which asserts the upstream answered). It now counts only a ValidationError for CallToolResult; any other falls through to the generic handler as before (delivery_uncertain, conservative since the checkpoint was claimed). Tests: a /mcp end-to-end test with a real SDK upstream answering an unparseable envelope (readable tool result, receipt, one debit, no redispatch on replay), the existing test that upstream_returned_error stays a bare error, and a real-SDK test that an unsendable request is not a rejected response. The fake-session test now raises a genuine CallToolResult validation error. Co-Authored-By: Claude Sonnet 5.5 --- app/routers/mcp_standard.py | 96 +++++++++++++++++--------- app/services/upstream_mcp.py | 15 ++-- tests/test_standard_mcp_endpoint.py | 103 +++++++++++++++++++++++++++- tests/test_upstream_mcp.py | 54 ++++++++++++++- 4 files changed, 228 insertions(+), 40 deletions(-) diff --git a/app/routers/mcp_standard.py b/app/routers/mcp_standard.py index 132a66ba..c7af78bb 100644 --- a/app/routers/mcp_standard.py +++ b/app/routers/mcp_standard.py @@ -32,16 +32,20 @@ (fund the wallet, ask a human via ``POST /v1/permit-requests``, retry the same call once an approval is decided). -One governed outcome is not a denial and does not use the JSON-RPC error -channel: ``delivery_uncertain``. The upstream call may have been delivered -and taken effect, and the charge stands, so the model itself must read that -it should not call the tool again. Clients commonly hand the model only a -JSON-RPC error's message, and ``-32005`` is this surface's retryable code, -so the outcome is returned as a tool result instead: ``isError: true``, a -text explanation carrying the do-not-resend instruction, the receipt, and +Two governed outcomes are not denials and do not use the JSON-RPC error +channel: ``delivery_uncertain`` and ``response_rejected``. In both the upstream +call may have been delivered and taken effect, and the charge stands, so the +model itself must read that it should not call the tool again. Clients +commonly hand the model only a JSON-RPC error's message, ``-32005`` is this +surface's retryable code, and ``-32006`` is also the code of refunded, +retry-safe failures (``upstream_returned_error``, +``upstream_pre_dispatch_failed``). Both outcomes are therefore returned as a +tool result instead: ``isError: true``, a text explanation carrying the +do-not-resend instruction, the receipt, and ``_meta["io.agentmiddleware/outcome"]`` with ``status: "unknown"``. The -governed endpoints (``/mcp/messages`` and REST) keep their ``-32005`` -contract. +refunded failures stay JSON-RPC errors, because retrying them is safe. The +governed endpoints (``/mcp/messages`` and REST) keep their ``-32005`` and +``-32006`` contract. Replay safety: a client-supplied ``Idempotency-Key`` header (or the ``io.agentmiddleware/idempotency_key`` entry in ``params._meta``) is honored @@ -143,6 +147,7 @@ _RECEIPT_META_KEY = "io.agentmiddleware/receipt" _OUTCOME_META_KEY = "io.agentmiddleware/outcome" _DELIVERY_UNCERTAIN = "delivery_uncertain" +_RESPONSE_REJECTED = "response_rejected" _META_KEY_SOURCE = 'params._meta["io.agentmiddleware/idempotency_key"]' # A legacy-transport client may send its key in params.mcpContext; the # source label names that location in the conflict/refusal payload. @@ -394,21 +399,42 @@ async def _mint_auto_permit( return permit.permit_id -def _delivery_uncertain_tool_result( - error: GovernedToolError, *, client_key_supplied: bool +# What each charged, effect-unknown outcome tells the model. Only the first two +# sentences differ: the instruction not to resend is the same for both. +_UNKNOWN_OUTCOME_LEAD = { + _DELIVERY_UNCERTAIN: ( + "delivery_uncertain: outcome unknown.", + "This call may have reached the upstream tool, but no trustworthy " + "response came back, so the action may or may not have happened.", + ), + _RESPONSE_REJECTED: ( + "response_rejected: outcome unknown.", + "This call reached the upstream tool and it answered, but the answer " + "was unusable and is not returned, so the gateway cannot tell whether " + "the action took effect.", + ), +} + + +def _unknown_outcome_tool_result( + error: GovernedToolError, *, reason: str, client_key_supplied: bool ) -> dict[str, Any]: - """Return ``delivery_uncertain`` as a tool result the model can read. - - Every other governed refusal on this surface stays a JSON-RPC error. This - outcome is different: the upstream call may already have been delivered - and taken effect, and the charge stands. A JSON-RPC error commonly - reaches the model as its message alone, and ``-32005`` is this surface's - retryable code, so a model told only "delivery_uncertain" tends to call - the tool again. Without a client Idempotency-Key that is a new, charged - dispatch: the duplicate this gateway exists to prevent. The MCP spec - routes errors the model must see into the result (``isError: true``). - ``isError`` stays true because ``false`` tells clients the call - succeeded, which is not known here. + """Return a charged, effect-unknown outcome as a tool result the model can read. + + ``reason`` is ``delivery_uncertain`` (no trustworthy response came back) or + ``response_rejected`` (the upstream answered but the answer was unusable: + an envelope the SDK cannot parse, a bad shape, oversize, or a reflected + credential). Every other governed refusal on this surface stays a JSON-RPC + error. These outcomes are different: the upstream call may already have + been delivered and taken effect, and the charge stands. A JSON-RPC error + commonly reaches the model as its message alone, ``-32005`` is this + surface's retryable code, and ``-32006`` is also what refunded, retry-safe + failures use, so a model told only the reason tends to call the tool + again. Without a client Idempotency-Key that is a new, charged dispatch: + the duplicate this gateway exists to prevent. The MCP spec routes errors + the model must see into the result (``isError: true``). ``isError`` stays + true because ``false`` tells clients the call succeeded, which is not known + here. The evidence is unchanged: the same signed receipt rides ``receipt`` and ``_meta["io.agentmiddleware/receipt"]`` as on a successful call, and the @@ -441,9 +467,7 @@ def _delivery_uncertain_tool_result( "attempt is a new dispatch and a new charge." ) sentences = [ - "delivery_uncertain: outcome unknown.", - "This call may have reached the upstream tool, but no trustworthy " - "response came back, so the action may or may not have happened.", + *_UNKNOWN_OUTCOME_LEAD[reason], "The gateway will not resend it.", "Do not call this tool again for the same action until you have checked " "whether it took effect in the system the tool acts on.", @@ -453,7 +477,7 @@ def _delivery_uncertain_tool_result( sentences.append(f"Receipt: {receipt_id}.") outcome: dict[str, Any] = { "status": "unknown", - "reason": _DELIVERY_UNCERTAIN, + "reason": reason, "redispatched": False, "idempotency_key_supplied": client_key_supplied, "remediation": { @@ -653,16 +677,22 @@ async def _governed_tools_call( denial_data["details"] = e.details raise _mcp_error(-32003, str(e), denial_data or None) from e except GovernedToolError as e: - if str(e) == _DELIVERY_UNCERTAIN: + reason = str(e) + if reason in _UNKNOWN_OUTCOME_LEAD: # First call and same-key replay both arrive here (replay rebuilds # the error from the stored terminal record), so both get the same - # result shape. See _delivery_uncertain_tool_result for why this - # one outcome leaves the JSON-RPC error channel. + # result shape. See _unknown_outcome_tool_result for why these two + # outcomes leave the JSON-RPC error channel. The gateway outcome + # keeps what each had on that channel: a rejected response is + # "failed" (a fault), an undelivered one is "unknown". mark_current_request( - gateway_outcome="unknown", reason_code="delivery_uncertain" + gateway_outcome=( + "unknown" if reason == _DELIVERY_UNCERTAIN else "failed" + ), + reason_code=reason, ) - return _delivery_uncertain_tool_result( - e, client_key_supplied=bool(client_idempotency_key) + return _unknown_outcome_tool_result( + e, reason=reason, client_key_supplied=bool(client_idempotency_key) ) mark_current_request( reason_code=bounded_governed_reason( diff --git a/app/services/upstream_mcp.py b/app/services/upstream_mcp.py index 89f21742..de813a30 100644 --- a/app/services/upstream_mcp.py +++ b/app/services/upstream_mcp.py @@ -1084,13 +1084,18 @@ async def _call_tool_once( canonical_result = self._canonicalize_result(result) except UpstreamMcpResponseRejectedError as exc: response_rejected = exc - except ValidationError: + except ValidationError as exc: # The SDK validates the result envelope after the upstream # has answered; timeouts, dropped connections and JSON-RPC - # errors raise other types. So this is a response received - # but unparseable, which is not delivery uncertainty. Record - # it here: leaving the session wraps it in an - # ExceptionGroup that no handler below can match. + # errors raise other types. So a CallToolResult that fails + # to parse is a response received but unusable, which is + # not delivery uncertainty. Record it here: leaving the + # session wraps it in an ExceptionGroup that no handler + # below can match. Any other validation error, such as one + # building the request before it is sent, says nothing + # about a response and is classified as before. + if exc.title != CallToolResult.__name__: + raise response_rejected = UpstreamMcpResponseRejectedError( "upstream_response_invalid" ) diff --git a/tests/test_standard_mcp_endpoint.py b/tests/test_standard_mcp_endpoint.py index b52d2bf6..f630e4db 100644 --- a/tests/test_standard_mcp_endpoint.py +++ b/tests/test_standard_mcp_endpoint.py @@ -10,8 +10,10 @@ import hashlib +import mcp.types as mcp_types import pytest from httpx import ASGITransport, AsyncClient +from mcp.server.lowlevel import Server from mcp.types import LATEST_PROTOCOL_VERSION from app.core.config import get_settings @@ -26,6 +28,7 @@ from app.services.mcp_generator import McpGenerator from app.services.service_registry import get_service_registry from tests.test_delivery_uncertain_replay import AmbiguousExecutor +from tests.test_mcp_upstream_governed import _adapter_over_real_sdk_server from tests.test_trust_helpers import BOOTSTRAP_HEADERS, provision_agent_wallet # The SDK's streamable HTTP transport requires an explicit Accept header. @@ -589,6 +592,99 @@ async def test_tools_call_delivery_uncertain_without_key_warns_a_retry_is_a_new_ assert executor.dispatch_count == 1 +@pytest.mark.anyio +async def test_tools_call_unparseable_upstream_envelope_is_a_tool_result_the_model_can_read( + client, standard_mcp_enabled, clean_database +): + """``response_rejected`` is charged, like ``delivery_uncertain``, so it too + must not reach the model as a bare ``-32006``, the code refunded and + retry-safe failures use. A real SDK upstream answers with an envelope the + SDK cannot parse; the call has run, the charge stands, and the model is told + not to send it again. + """ + provisioned = await provision_agent_wallet(client) + tool_name = "standard-unparseable-envelope-upstream" + served: list[str] = [] + server = Server("unparseable-envelope-upstream") + + async def call_tool(_req: mcp_types.CallToolRequest) -> mcp_types.ServerResult: + served.append("tools/call") + # model_construct skips validation, so the SERVER serializes an envelope + # that only the CLIENT side fails to parse. + return mcp_types.ServerResult.model_construct( + root=mcp_types.CallToolResult.model_construct( + content="not-a-list", isError=False + ) + ) + + server.request_handlers[mcp_types.CallToolRequest] = call_tool + get_service_registry().register_upstream( + service_id=tool_name, + name="Standard Unparseable Envelope Upstream", + description="Upstream tool whose response envelope cannot be parsed", + category=ServiceCategory.AGENT_COMMS, + executor=_adapter_over_real_sdk_server(server, tool_name), + input_schema={"type": "object", "properties": {"test": {"type": "string"}}}, + output_schema=None, + credits_per_unit=2.0, + upstream_tool_name="partner.echo", + upstream_origin="https://test.example.com", + ) + call = _rpc( + "tools/call", params={"name": tool_name, "arguments": {"test": "value"}} + ) + headers = { + **provisioned["agent_headers"], + **MCP_HEADERS, + "Idempotency-Key": "standard-unparseable-1", + } + try: + first = await client.post("/mcp", json=call, headers=headers) + assert first.status_code == 200 + body = first.json() + assert "error" not in body + result = body["result"] + assert result["isError"] is True + text = result["content"][0]["text"] + assert text.startswith("response_rejected: outcome unknown.") + assert "answered, but the answer was unusable" in text + assert "The gateway will not resend it." in text + assert "Do not call this tool again for the same action" in text + assert "same Idempotency-Key returns this same result" in text + # Nothing the upstream sent is relayed, only the fixed explanation. + assert "not-a-list" not in text + + receipt = result["receipt"] + assert receipt["outcome"] == "response_rejected" + assert text.endswith(f"Receipt: {receipt['receipt_id']}.") + assert result["_meta"]["io.agentmiddleware/receipt"] == receipt + outcome = result["_meta"]["io.agentmiddleware/outcome"] + assert outcome["status"] == "unknown" + assert outcome["reason"] == "response_rejected" + assert outcome["redispatched"] is False + assert outcome["idempotency_key_supplied"] is True + assert outcome["remediation"]["type"] == "verify_before_new_attempt" + assert outcome["dispatch"]["state"] == "response_rejected" + assert served == ["tools/call"] + + # Same key: the identical result, no second dispatch, no second debit. + replay = await client.post("/mcp", json=call, headers=headers) + assert replay.status_code == 200 + assert replay.json() == body + assert served == ["tools/call"] + + ledger = await client.get( + f"/v1/billing/ledger/{provisioned['agent_wallet_id']}", + headers=provisioned["agent_headers"], + ) + assert ledger.status_code == 200 + entries = ledger.json()["entries"] + assert len([e for e in entries if e["action"] == "debit"]) == 1 + assert [e for e in entries if e["action"] == "refund"] == [] + finally: + get_service_registry().unregister_local(tool_name) + + @pytest.mark.anyio async def test_tools_call_other_governed_errors_stay_jsonrpc_errors( client, @@ -596,7 +692,12 @@ async def test_tools_call_other_governed_errors_stay_jsonrpc_errors( clean_database, monkeypatch: pytest.MonkeyPatch, ): - """Only delivery_uncertain leaves the error channel; the rest keep codes.""" + """Refunded failures keep their JSON-RPC error; only charged, unknown ones leave. + + ``upstream_returned_error`` shares ``-32006`` with ``response_rejected`` but is + refunded and safe to retry, so it must stay a bare error (see the tests above + and below for the two outcomes that become a readable tool result). + """ provisioned = await provision_agent_wallet(client) tool_name = "standard-governed-upstream-error" diff --git a/tests/test_upstream_mcp.py b/tests/test_upstream_mcp.py index 66e908db..631c614c 100644 --- a/tests/test_upstream_mcp.py +++ b/tests/test_upstream_mcp.py @@ -86,6 +86,15 @@ def _cleanup_validation_error() -> ValidationError: raise AssertionError("invalid timeout unexpectedly passed validation") +def _unparseable_result_error() -> ValidationError: + """The error the SDK raises when a tools/call result fails to parse.""" + try: + CallToolResult.model_validate({"content": "not-a-list"}) + except ValidationError as exc: + return exc + raise AssertionError("invalid CallToolResult unexpectedly passed validation") + + class FakeSession: def __init__( self, @@ -1823,6 +1832,49 @@ async def call_tool(_req: CallToolRequest) -> ServerResult: assert metrics_after["response_rejected"] == metrics_before["response_rejected"] +@pytest.mark.anyio +async def test_validation_error_before_the_request_is_sent_is_not_a_rejected_response() -> ( + None +): + """Only a CallToolResult that fails to parse proves the upstream answered. + + ``arguments`` with a non-string key makes the SDK's own request model raise + a ``ValidationError`` before anything is written. The dispatch checkpoint + has already been claimed, so the call is conservatively + ``delivery_uncertain`` as before; it must not be ``response_rejected``, + which says the upstream answered when no request ever reached it. The + upstream here would answer correctly, so a request that was wrongly sent + would show up as a success or a served call. + """ + served: list[str] = [] + server = Server("healthy-upstream") + + async def call_tool(_req: CallToolRequest) -> ServerResult: + served.append("tools/call") + return ServerResult( + CallToolResult(content=[TextContent(type="text", text="ok")]) + ) + + server.request_handlers[CallToolRequest] = call_tool + adapter = _adapter_over_in_memory_sdk(server) + claims: list[str] = [] + + async def before_dispatch() -> None: + claims.append("claimed") + + with pytest.raises(UpstreamMcpDeliveryUncertainError) as exc_info: + await adapter.call_tool( + {1: "x"}, # type: ignore[dict-item] + invocation_id="inv", + idempotency_key="idem", + before_dispatch=before_dispatch, + ) + + assert exc_info.value.dispatch_started is True + assert claims == ["claimed"] + assert served == [] + + def _scripted_mcp_client( *, on_call: Any = None, @@ -1982,7 +2034,7 @@ async def test_cleanup_failure_does_not_replace_unparseable_envelope_rejection( _mark_discovered(adapter) _bind_session( adapter, - FakeSession(call_error=_cleanup_validation_error()), + FakeSession(call_error=_unparseable_result_error()), cleanup_error=cleanup_error, ) From 663867960a2ea871c3deba2e9881c914e22a46c3 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:10:19 -0700 Subject: [PATCH 3/6] docs(mcp): record the endpoint decision, re-pin stale citations, re-measure gateway overhead Follow-up to 0698168f. Endpoint decision. POST /mcp/messages and the REST invoke route are the live governed path in the supported pilot. POST /mcp ships off (ENABLE_STANDARD_MCP_ENDPOINT defaults to false) and the production runbook keeps it off, because it mints a short-lived single-tool permit and generates an idempotency key on the caller's behalf. README.md and SECURITY_LIMITATIONS.md no longer call it the supported or preferred path today. The posture doc records the decision instead of describing a contradiction, and the overhead page no longer calls /mcp/messages "not the supported endpoint". docs/deploy-railway.md and docs/mcp-registry-submission.md were already correct and are untouched. Citations. The file:line citations in docs/mcp-security-posture.md and docs/gateway-overhead.md were written against a base more than a thousand commits older. Of 17, 15 had moved, one SDK range was a line short and mcp.py:15 was still right. Each is re-pinned and the claim beside it re-read. Corrected: idempotency_key_required applies to every governed call (including any call in strict trust mode), not only permit-bound ones; a signed denied receipt is issued only when the permit names the caller's wallet; the SDK validation range is 412-448; exit-and-export.md said no OpenTelemetry instrumentation exists, but optional tracing (OTEL_TRACING_ENABLED, default off) does; the poisoning-test docstring named one call site of register_configured_upstream_mcp where scripts/failure_lab.py is a second. Response classification docs. failure-semantics and the posture page now state the caveat that a body that is not a JSON-RPC message at all (non-JSON text, jsonrpc 1.0, neither result nor error, unknown request id) still ends as delivery_uncertain after the call timeout, each observed against a scripted transport; that with the reconcile worker enabled response_rejected is terminal and never probed or refunded; and that /mcp now returns response_rejected as a readable tool result. The CHANGELOG no longer says precedence is unchanged for the outer ValidationError branch, which gained the guard, and describes the transport behaviour accurately. TROUBLESHOOTING gains a response_rejected section and authority-required-flow names the renamed helper. Gateway overhead re-measured twice on this base (commit 0698168f, git_dirty false, load average recorded): first call p50 29.5-30.9 ms, replay p50 5.4-5.6 ms (earlier runs 29.5-31.9 and 4.4-4.7). Results in docs/research/gateway-overhead-2026-10-23/. The public site's figure is not touched. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 91 +++++++++++--- README.md | 16 ++- SECURITY_LIMITATIONS.md | 14 ++- TROUBLESHOOTING.md | 5 + docs/authority-required-flow.md | 6 +- docs/exit-and-export.md | 4 +- docs/failure-semantics.md | 28 ++++- docs/gateway-overhead.md | 59 +++++---- docs/mcp-security-posture.md | 114 ++++++++++++------ .../gateway-overhead-2026-10-23/run1.json | 50 ++++++++ .../gateway-overhead-2026-10-23/run2.json | 50 ++++++++ tests/test_mcp_upstream_tool_poisoning.py | 5 +- 12 files changed, 343 insertions(+), 99 deletions(-) create mode 100644 docs/research/gateway-overhead-2026-10-23/run1.json create mode 100644 docs/research/gateway-overhead-2026-10-23/run2.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 10c65a7a..2854d9ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,34 +58,91 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `ExceptionGroup`, so the adapter's `ValidationError` branch never matched and the generic handler classified the call by `dispatch_started` alone. Tests that fake the session do not enter the task group, which is why they passed. -- `_call_tool_once` now records the rejection where the `ValidationError` is - raised (the `call_tool` await, which the SDK reaches only after the upstream's - JSON-RPC result has arrived), before the session unwinds. The call ends as +- `_call_tool_once` now records the rejection where the error is raised (the + `call_tool` await), before the session unwinds, and only for a + `ValidationError` of the `CallToolResult` model, which the SDK raises only + after the upstream's JSON-RPC result has arrived. The call ends as `response_rejected` with code `upstream_response_invalid` (502 / `-32006`): the debit stays, the call is never redispatched, and a replay returns the same - receipt. Existing precedence is unchanged: claim contention, then a transport - guard rejection, then a known received result, then this rejection, then - `delivery_uncertain` only when no response was obtained. -- The `ValidationError` branch also lacked the "a result is already in hand" - guard the other handlers have, so a `ValidationError` or `JSONDecodeError` - raised only while the session closes could turn a received result into - `response_rejected`, or replace a known rejection's code. It now has the guard. + receipt. A validation error for any other model, such as the request the SDK + builds before sending (a non-string key in `arguments`: the claim is + committed and nothing reaches the upstream), says nothing about a response + and is classified as before, as `delivery_uncertain`. +- Outcome precedence is unchanged where a handler already applied it: claim + contention, then a transport guard rejection, then a known received result, + then this rejection, then `delivery_uncertain` only when no response was + obtained. The outer `ValidationError` / `JSONDecodeError` branch did not apply + the "a result is already in hand" rule, so such an error raised only while the + session closes could turn a received result into `response_rejected`, or + replace a known rejection's code. It now applies it. +- **Behavior change, `POST /mcp` (where enabled):** `response_rejected` is + charged like `delivery_uncertain`, so it is now returned like it: a + `tools/call` result with `isError: true`, text saying the upstream answered but + the answer was unusable and the effect is unknown, the do-not-resend + instruction, the receipt, and `_meta["io.agentmiddleware/outcome"]` with + `status: "unknown"` and `reason: "response_rejected"`. It was a bare `-32006`, + the code that refunded, retry-safe failures (`upstream_returned_error`, + `upstream_pre_dispatch_failed`) also use, so a model shown only the message + could resend a call whose charge stands and whose tool probably ran. Those + refunded failures stay JSON-RPC errors. `/mcp/messages` and REST are + unchanged. The helper is now `_unknown_outcome_tool_result` + (was `_delivery_uncertain_tool_result`). - **Behavior change:** a malformed envelope is no longer counted toward the optional circuit breaker. The `response_rejected` path never fed it (an oversized or bearer-echoing result did not either); `delivery_uncertain` does. +- Documented, not changed: with the optional reconcile worker enabled, + `response_rejected` is terminal and is never probed or refunded; only + `delivery_uncertain` is leased for a provider status probe. - Unchanged: a call with no response (connection dropped, timeout, JSON-RPC error answering `tools/call`) is still `delivery_uncertain`; a malformed - `initialize` answer is still a pre-dispatch failure. Not covered: an upstream - body that is not a JSON-RPC message at all is dropped inside the SDK - transport, so that call times out and ends as `delivery_uncertain`. + `initialize` answer is still a pre-dispatch failure. **Not covered:** a body + that is not a JSON-RPC message at all (non-JSON text, `"jsonrpc": "1.0"`, + neither `result` nor `error`, or an unknown request id). The SDK transport + logs the parse error and forwards it into the session, whose default message + handler discards it, so the request is never answered and the call ends as + `delivery_uncertain` after the call timeout. The SDK's log line quotes the + start of the body; that is unchanged. - New tests drive a real SDK session behind the real adapter, in memory and over - the production Streamable HTTP client, with four envelope shapes, a fixture - guard (the envelope must reach the client and fail there), the no-response - boundary, a secret-echo check, and a governed-router receipt test (one debit, - one dispatch, a `response_rejected` receipt, no redispatch on replay). See + the production Streamable HTTP client: four envelope shapes, a fixture guard + (the envelope must reach the client and fail there), the no-response + boundary, a request that cannot be built stays `delivery_uncertain`, a + malformed `initialize` stays pre-dispatch, a secret-echo check, and + governed-router tests on `/mcp/messages` (one debit, one dispatch, a + `response_rejected` receipt, no redispatch on replay, circuit breaker closed, + against a JSON-RPC error that opens it) and on `/mcp`. See [`docs/mcp-security-posture.md`](docs/mcp-security-posture.md) §3.2 and [`docs/failure-semantics.md`](docs/failure-semantics.md). +### Changed — documentation re-pinned to the current code + +- **Supported endpoint decided.** `POST /mcp/messages` and the REST invoke route + are the live governed path in the supported pilot. `POST /mcp` ships off + (`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false) and the production runbook + keeps it off, because it mints a permit and generates an idempotency key on the + caller's behalf. `README.md` and `SECURITY_LIMITATIONS.md` no longer call it the + supported or preferred path today; `docs/mcp-security-posture.md` records the + decision instead of describing a contradiction; `docs/gateway-overhead.md` no + longer calls `/mcp/messages` "not the supported endpoint". +- `file:line` citations in `docs/mcp-security-posture.md` and + `docs/gateway-overhead.md` were written against a base more than a thousand + commits older. Of 17, 15 had moved (by 1 to about 970 lines), one SDK range + was a line short, and one (`mcp.py:15`) was still right. Each is re-pinned and + the claim beside it re-read. Claims that no longer held, corrected: the `idempotency_key_required` + refusal applies to every governed call, including any call in strict trust + mode, not only permit-bound ones; a signed `denied` receipt is issued only when + the permit names the caller's wallet; the SDK validation range is + `mcp/client/session.py:412-448`; `docs/exit-and-export.md` said no + OpenTelemetry instrumentation exists, but optional tracing + (`OTEL_TRACING_ENABLED`, default off) does; and the poisoning-test docstring + named one call site of `register_configured_upstream_mcp` where + `scripts/failure_lab.py` is a second. +- `docs/gateway-overhead.md` is re-measured twice on this base, with the load + average recorded: first call p50 29.5–30.9 ms and replay p50 5.4–5.6 ms + (earlier runs: 29.5–31.9 ms and 4.4–4.7 ms), results in + `docs/research/gateway-overhead-2026-10-23/`. The public site's figure is not + touched. +- `TROUBLESHOOTING.md` gains a `response_rejected` receipt section. + ### Added — per-upstream concurrency limit and circuit breaker (default off) - `UPSTREAM_MAX_CONCURRENCY_PER_UPSTREAM` (default `0`, unlimited) caps diff --git a/README.md b/README.md index b30e6e99..f34d7822 100644 --- a/README.md +++ b/README.md @@ -286,12 +286,16 @@ curl -sS -X POST "$API_URL/mcp/messages" \ }" ``` -`POST /mcp/messages` is the legacy JSON-RPC transport — deprecated in the -OpenAPI contract, fully supported, and the only JSON-RPC surface in a default -local run. `POST /mcp` is the standards-compliant Streamable HTTP endpoint on -the official MCP SDK; it is opt-in today (`ENABLE_STANDARD_MCP_ENDPOINT=true`, -default off) and new integrations should prefer it. Both run the same governed -permit → meter → receipt path. +`POST /mcp/messages` is the JSON-RPC transport the supported pilot runs on — +marked deprecated in the OpenAPI contract, fully supported, and the only +JSON-RPC surface in a default local run. `POST /mcp` is the standards-compliant +Streamable HTTP endpoint on the official MCP SDK. It ships off +(`ENABLE_STANDARD_MCP_ENDPOINT`, default false) and the production runbook +([docs/deploy-railway.md](docs/deploy-railway.md)) requires it to stay off, +because it mints a short-lived, single-tool permit on the caller's behalf and +generates an idempotency key when the client sends none. It becomes the +preferred path for new integrations once an operator enables it. Both run the +same governed permit → meter → receipt path. An agent holding no authority can ask a human for it with `POST /v1/permit-requests`. Full HTTP sequence: diff --git a/SECURITY_LIMITATIONS.md b/SECURITY_LIMITATIONS.md index 74374a29..5b156447 100644 --- a/SECURITY_LIMITATIONS.md +++ b/SECURITY_LIMITATIONS.md @@ -364,11 +364,15 @@ production-like boots refuse): - `/v1/dev-keys/self-provision` stays runtime-gated by its own flag and is advertised in the schema only when that flag is on (never in production). -The legacy invoke entry points `POST /mcp/messages` and -`POST /mcp/tools/{id}/invoke` remain mounted for existing clients and the -local proof scripts but are marked `deprecated` in the spec; the standard -MCP Streamable HTTP endpoint at `POST /mcp` is the supported path. All entry -points run the same governed permit→meter→receipt path. +`POST /mcp/messages` and `POST /mcp/tools/{id}/invoke` are the live governed +path in the supported pilot, and are marked `deprecated` in the spec. The +standard MCP Streamable HTTP endpoint at `POST /mcp` ships off +(`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false) and the production runbook +([`docs/deploy-railway.md`](docs/deploy-railway.md)) requires it to stay off, +because it mints a short-lived, single-tool permit on the caller's behalf and +generates an idempotency key when the client sends none. It becomes the +preferred path for new integrations once an operator enables it. Every entry +point that is enabled runs the same governed permit→meter→receipt path. ## Public Health Reporting diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index 8da2de95..832c2f5d 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -104,6 +104,11 @@ The gateway claimed a send but has no trustworthy terminal result. Delivery and On the standard `/mcp` endpoint this outcome arrives as a `tools/call` result with `isError: true` and `_meta["io.agentmiddleware/outcome"].status` set to `"unknown"`, with the do-not-resend instruction in its text, not as a JSON-RPC `-32005` error. `/mcp/messages` and the REST surface keep `-32005`. +### `response_rejected` receipt +The upstream answered, but the answer was unusable: its envelope could not be parsed, its shape was invalid, it was over the byte cap, or it reflected the upstream bearer token. The call was dispatched and the charge stands, so the action may have taken effect; the gateway does not redispatch, and replaying the same key returns the same receipt (`502` / `-32006`). Do not retry automatically; check the downstream system first. Unlike `delivery_uncertain`, it is never probed or refunded by the optional reconcile worker. `-32006` is also the code of refunded, retry-safe failures (`upstream_returned_error`, `upstream_pre_dispatch_failed`), so tell them apart by `error.message` and the receipt `outcome`, not by the code. + +On the standard `/mcp` endpoint this outcome also arrives as a `tools/call` result with `isError: true`, `_meta["io.agentmiddleware/outcome"].status` set to `"unknown"` (reason `response_rejected`) and the do-not-resend instruction in its text. The refunded failures stay JSON-RPC errors there. + --- ## Examples fail diff --git a/docs/authority-required-flow.md b/docs/authority-required-flow.md index 486c0c79..20a46d89 100644 --- a/docs/authority-required-flow.md +++ b/docs/authority-required-flow.md @@ -226,8 +226,10 @@ resume) — opposite instructions under one code. On `/mcp` the worst of this is solved by moving `delivery_uncertain` out of the error channel: it returns a tool result with `isError: true`, explicit do-not-resend text, the receipt, and `_meta["io.agentmiddleware/outcome"].status == "unknown"` -(`_delivery_uncertain_tool_result` in `app/routers/mcp_standard.py`), because -a JSON-RPC error commonly reaches the model as its message alone. +(`_unknown_outcome_tool_result` in `app/routers/mcp_standard.py`), because +a JSON-RPC error commonly reaches the model as its message alone. The other +charged, effect-unknown outcome, `response_rejected` (`-32006`, a code refunded +retry-safe failures also use), takes the same route there for the same reason. `human_approval_pending` still carries `data.error = "authority_required"`. `/mcp/messages` and REST keep `delivery_uncertain` on `-32005`, where its data carries `receipt` and `dispatch` and never an `error` key. The remaining diff --git a/docs/exit-and-export.md b/docs/exit-and-export.md index 6c549ff7..4281693f 100644 --- a/docs/exit-and-export.md +++ b/docs/exit-and-export.md @@ -5,7 +5,7 @@ they adopt. This page lists the artifacts a customer can take, with each one's export route, format, and whether the thing that checks it afterwards depends on us. It is not an exhaustive data inventory; §3 lists what has no export. -Last updated: 2026-10-22. Routes are from the routers under `app/routers/`, +Last updated: 2026-10-23. Routes are from the routers under `app/routers/`, except `/openapi.json`, which FastAPI serves itself. Verifiers are from `b2a_sdk/`. "Independent" means the verifier imports none of this application and needs no access to the issuer at verification time. "First-party" means @@ -36,7 +36,7 @@ per key, so a large export is slow: 10,000 receipts at two calls each is about | **Tool arguments and upstream results** | **No export route** | Stored (`mcp_dispatch_attempts.result_json`, `idempotency_records.response_json`); exported as hashes only (`request_hash`, `response_hash`) | | The evidence bundle deliberately keeps payloads private. They exist only in a database copy. | | **API keys** | Not exportable by design | Only the SHA-256 hash is stored for database keys | | Wallet keys are revoked per wallet at `/v1/api-keys`. Administrator keys are the `VALID_API_KEYS` variable, not rows; they are removed by whoever controls the deployment's variables. | | **Tool catalog** | `GET /mcp/tools.json`, `GET /v1/discover`, `GET /openapi.json` | MCP tools JSON; OpenAPI 3.1 | | Yes, as data. | -| **Traces / spans** | **None exist.** | | | No OpenTelemetry instrumentation and no OTLP export. Runtime logging is `structlog` JSON written to **stderr** from `app/main.py` (the container entrypoint does not merge it into stdout); other modules log through the standard library. Unclassified failures carry a `correlation_id`. Per-call evidence lives in receipts and evidence bundles. | +| **Traces / spans** | **No export route.** Optional OpenTelemetry tracing exists (`app/core/tracing.py`) and is off by default. | OTLP/HTTP to a collector the operator runs | | Nothing here a customer can take. With `OTEL_TRACING_ENABLED` false (the default) or the otel extra absent, no spans are made and no exporter is configured. When on, six spans on the governed path (`http.request`, `mcp.invoke`, `mcp.reserve`, `mcp.dispatch_claim`, `mcp.upstream_dispatch`, `mcp.receipt`) carry only an allowlist of attributes (tool name, outcome state, a hash of the wallet id, the attempt id), so they are not a record of calls you could rebuild from. Runtime logging is `structlog` JSON written to **stderr** from `app/main.py` (the container entrypoint does not merge it into stdout); other modules log through the standard library. Unclassified failures carry a `correlation_id`. Per-call evidence lives in receipts and evidence bundles. | | **Agent definitions, memory stores, eval datasets** | **Not applicable on the supported deployment.** | | | The core trust plane does not define agents, hold memory, or own model evaluation. Two frozen proof surfaces do. `comms` keeps an agent registry (`GET /v1/comms/agents`) and `ai` keeps per-agent memory (`POST /v1/ai/memory/recall`, newest 1,000 entries per agent); both are in durable state, mount only with `ENABLE_PROOF_SURFACES=true`, which production refuses at boot, and have no purpose-built export. The repository's own proofs and tests travel with the code. | ## 2. What is open and what is source-available diff --git a/docs/failure-semantics.md b/docs/failure-semantics.md index 0d85718f..8d96b77a 100644 --- a/docs/failure-semantics.md +++ b/docs/failure-semantics.md @@ -86,16 +86,32 @@ a hostile `$ref` cannot turn a completed call into an ambiguous one; only the adapter's own shape, size, and secret checks can reject a received result ([`mcp-security-posture.md`](mcp-security-posture.md) §3.2). Conversely, `delivery_uncertain` is for calls where no result was obtained: the connection -dropped, the request timed out, or the process died after the claim. A response -the gateway received but the SDK cannot parse (for example `content` that is not -a list) is `response_rejected`, not `delivery_uncertain`: the upstream answered, -so the outcome is known and unusable -([`mcp-security-posture.md`](mcp-security-posture.md) §3.2). The drifted-schema -case is proven end to end by +dropped, the request timed out, or the process died after the claim. A JSON-RPC +result the gateway received but whose envelope the SDK cannot parse (for example +`content` that is not a list) is `response_rejected`, not `delivery_uncertain`: +the upstream answered, so the outcome is known and unusable +([`mcp-security-posture.md`](mcp-security-posture.md) §3.2). The caveat: a body +that is not a JSON-RPC message at all (non-JSON text, `"jsonrpc": "1.0"`, +neither `result` nor `error`, or an unknown request id) is dropped by the SDK +transport and never reaches the adapter as a response, so it still ends as +`delivery_uncertain` after the call timeout (§3.2, "Not covered"). The +drifted-schema case is proven end to end by `test_completed_call_with_drifted_live_schema_is_a_charged_success_end_to_end`, and the unparseable-envelope case, next to its `delivery_uncertain` boundary, by `test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch`. +Two further differences between the two charged outcomes. With the optional +reconcile worker enabled (`UPSTREAM_RECONCILE_WORKER_ENABLED`), only +`delivery_uncertain` is leased for a provider status probe and can be refunded +on an attested `not_executed` answer; `response_rejected` is terminal and is +never probed or refunded. And on `POST /mcp`, where enabled, both arrive as a +`tools/call` result with `isError: true`, the do-not-resend text, the receipt and +`_meta["io.agentmiddleware/outcome"].status` `"unknown"`, not as a JSON-RPC error: +`-32006` is also the code of refunded, retry-safe failures +(`upstream_returned_error`, `upstream_pre_dispatch_failed`), which stay errors. +`/mcp/messages` and REST keep `-32005` and `-32006` +(`test_tools_call_unparseable_upstream_envelope_is_a_tool_result_the_model_can_read`). + Requests rejected before a valid permit and an executable tool are established produce no receipt — there is no authority to bill against. The README states this limit. What they leave behind differs by reason: diff --git a/docs/gateway-overhead.md b/docs/gateway-overhead.md index b48d785b..cf37f1c5 100644 --- a/docs/gateway-overhead.md +++ b/docs/gateway-overhead.md @@ -9,25 +9,30 @@ reproduced or disputed. **It is the floor of the gateway's own handler work, not the cost of the hop in production.** §3 lists what is excluded; read it before quoting the number. -Measured 2026-10-22 on an Apple M4 Pro (14 cores), **under load** (see §4). +Measured 2026-10-23 on an Apple M4 Pro (14 cores), **under load** (see §4). Raw results and provenance are committed next to this page in -[`docs/research/gateway-overhead-2026-10-22/`](research/gateway-overhead-2026-10-22/). +[`docs/research/gateway-overhead-2026-10-23/`](research/gateway-overhead-2026-10-23/). +An earlier pair of runs from 2026-10-22, on a much older base, is kept in +[`docs/research/gateway-overhead-2026-10-22/`](research/gateway-overhead-2026-10-22/) +and compared in §4. ## 1. The numbers -Per governed `tools/call` on `POST /mcp/messages` (the legacy endpoint, with a -pre-minted permit), strict trust mode, zero-latency in-process upstream: +Per governed `tools/call` on `POST /mcp/messages` (the live governed path in the +supported pilot, marked deprecated in the OpenAPI contract; with a pre-minted +permit), strict trust mode, zero-latency in-process upstream: | Path | p50 | p95 | n | |---|---|---|---| -| **First call** (full pipeline: one debit, one receipt, one audit event) | **30–32 ms** | 36–49 ms | 2 × 1,000 | -| **Replay** (same key, stored receipt returned, nothing re-executed) | **4.4–4.7 ms** | 5.0–5.5 ms | 2 × 1,000 | +| **First call** (full pipeline: one debit, one receipt, one audit event) | **29.5–30.9 ms** | 38–77 ms | 2 × 1,000 | +| **Replay** (same key, stored receipt returned, nothing re-executed) | **5.4–5.6 ms** | 6.6–10.0 ms | 2 × 1,000 | Two runs back to back, 20 warm-up calls discarded. Ranges are the spread between the two runs. The p99 and max values are in the raw files but are not -quoted here: p99 varied from 71 to 76 ms and max from 76 to 237 ms between -runs, which is scheduler noise on a loaded machine, not a property of the -gateway. +quoted here: p99 varied from 77 to 167 ms and max from 97 to 263 ms between +runs, and the first-call p95 from 38 to 77 ms, which is scheduler noise on a +loaded machine (the second run started at a load average of 34), not a property +of the gateway. What the first call pays for, in pipeline order ([`POLICY_ENFORCEMENT.md`](POLICY_ENFORCEMENT.md) §2): API-key hash lookup and @@ -36,11 +41,11 @@ creation; permit validation including the Ed25519 signature check; wallet policy evaluation; the atomic budget reservation and prepared dispatch attempt (one transaction); the debit; the executor call, inside which the one-shot dispatch claim commits immediately before the send (the router hands -the claim to the executor as a callback, `mcp.py:2706-2710`); result +the claim to the executor as a callback: `claim_dispatch` at `mcp.py:3674`, passed as `before_dispatch` at `:3735`); result canonicalization; receipt signing (Ed25519) and the hash-chained, signed audit event; idempotency record completion. All of these commits go to the ORM database, a SQLite file here, in WAL mode with `synchronous=NORMAL` -(`app/db/database.py:141-146`): a commit is a WAL append, and `fsync` happens +(`app/db/database.py:175-180`): a commit is a WAL append, and `fsync` happens at checkpoints, not on every commit. The replay number is the cost of refusing to do any of that again: key @@ -98,9 +103,10 @@ stderr. the upstream's own time, none of them measured here. The live `/health/dependencies` payload reports aggregate `call_metrics` (count, total and max latency) for the real path, without percentiles. -- **Not the supported endpoint.** The script measures `POST /mcp/messages`, - which is marked deprecated in the OpenAPI contract. `POST /mcp`, when - enabled, mints a permit per call (policy lookup, idempotency begin and +- **Not the standard endpoint.** The script measures `POST /mcp/messages`, the + live governed path in the supported pilot. `POST /mcp` ships off and the + production runbook keeps it off ([`mcp-security-posture.md`](mcp-security-posture.md)). + When enabled it mints a permit per call (policy lookup, idempotency begin and complete for the mint, a signed permit) and is expected to cost more. That was not measured. - **Not concurrent.** Calls are sequential. Under concurrency calls on one @@ -117,12 +123,19 @@ stderr. | | | |---|---| -| Machine | Apple M4 Pro, 14 cores, macOS 26.7 (Darwin 25.6.0), Python 3.12.13 | -| Load | **Not idle.** Load average (1/5/15 min) was 23.5/23.9/30.2 at the start of run 1 and 17.6/22.3/29.3 at run 2, on a machine running many concurrent sessions. Both runs are therefore pessimistic, and their agreement within a couple of milliseconds should not be read as proof of stability. | -| Code | commit `b8f8806c` plus uncommitted changes (`git_dirty` is true in both result files); the measuring script is the version committed with this page | -| Packages | sqlalchemy 2.0.52, aiosqlite 0.22.1, cryptography 50.0.1, httpx 0.28.1, mcp 1.30.0 | +| Machine | Apple M4 Pro, 14 cores, macOS 26.7.2 (Darwin 25.6.0), Python 3.13.14 | +| Load | **Not idle.** Load average (1/5/15 min) was 22.8/16.0/21.9 at the start of run 1 and 33.8/19.8/23.0 at run 2, on a machine running many concurrent sessions. Both runs are therefore pessimistic, and run 2 (the heavier) shows it in its tail: first-call p95 77 ms against 38 ms. Their agreement on the p50 should not be read as proof of stability. | +| Code | commit `0698168f`, with `git_dirty` false in both result files (the script counts tracked files only). The documentation commit that follows changes no code. The measuring script is the version committed with this page | +| Packages | sqlalchemy 2.0.54, aiosqlite 0.22.1, cryptography 50.0.2, httpx 0.28.1, mcp 1.30.0 | | Posture | strict trust mode; `ENVIRONMENT=local`; `RATE_LIMIT_PER_MINUTE=1000000` with the in-memory limiter; no policy bundles; no per-tool call cap; no approval gate; one key-bound permit; `STATE_BACKEND=memory` | +**Against the earlier runs.** The 2026-10-22 pair (commit `b8f8806c` plus +uncommitted changes, Python 3.12.13, load averages of 18–24 at the 1-minute +mark) gave a first-call p50 of 29.5–31.9 ms and a replay p50 of 4.4–4.7 ms. The +first-call p50 did not move; the replay p50 is about 1 ms higher on this base. +That difference is not attributed: the base is more than a thousand commits +later, the interpreter differs, and the load differs. + `STATE_BACKEND=memory` affects only the separate key/value state store, which the governed call path does not touch. The idempotency records, permit reservation, dispatch attempt, ledger, receipt, and audit rows all live in the @@ -139,7 +152,7 @@ These do not agree with this page, and they have not been reconciled: |---|---|---| | `site/proof/transcript.json`, `docs/demo-trust-plane-output.md` | p50 67.3 ms, p95 83.7 ms | `POST /mcp/messages`, 100 samples, local tool `trust-plane-echo`, in-process, local SQLite, recorded 2026-09-08; machine and load not recorded | | `docs/failure-lab.md` | about 90 ms added per call at the median | through the real upstream adapter against an in-process MCP server, so it includes connection setup and the SDK's calls | -| This page | p50 30–32 ms first call, 4.4–4.7 ms replay | stub executor, 1,000 samples, loaded M4 Pro | +| This page | p50 29.5–30.9 ms first call, 5.4–5.6 ms replay | stub executor, 1,000 samples, loaded M4 Pro | The first row is the same endpoint measured with a different tool, a smaller sample, an earlier commit, and an unrecorded machine, so the gap cannot be @@ -149,7 +162,9 @@ provenance fields would make them comparable. ## 6. Raw results -[`run1.json`](research/gateway-overhead-2026-10-22/run1.json) and -[`run2.json`](research/gateway-overhead-2026-10-22/run2.json). Both runs: +[`run1.json`](research/gateway-overhead-2026-10-23/run1.json) and +[`run2.json`](research/gateway-overhead-2026-10-23/run2.json). Both runs: `"dispatches": 1020` (20 warm-up plus 1,000 measured); replays dispatched -nothing. +nothing. The earlier pair is +[`run1.json`](research/gateway-overhead-2026-10-22/run1.json) and +[`run2.json`](research/gateway-overhead-2026-10-22/run2.json). diff --git a/docs/mcp-security-posture.md b/docs/mcp-security-posture.md index d6aa9a15..0e8d4753 100644 --- a/docs/mcp-security-posture.md +++ b/docs/mcp-security-posture.md @@ -6,7 +6,7 @@ notes, OWASP Agentic Top 10). For the product boundary itself read [`SECURITY_LIMITATIONS.md`](../SECURITY_LIMITATIONS.md) first; for the OWASP mapping read [`owasp-agentic-top10-mapping.md`](owasp-agentic-top10-mapping.md). -Last updated: 2026-10-22. Claims name their evidence where it exists. Labels: +Last updated: 2026-10-23. Claims name their evidence where it exists. Labels: **verified** means read in code or in a test body for this revision; **verified by name** means the test exists but its body was not read; **documented** means stated elsewhere in the repo and not re-read here; @@ -32,13 +32,18 @@ operator-configured upstream server over Streamable HTTP, and three MCP The REST route `POST /mcp/tools/{id}/invoke` is a deprecated entry into the same governed pipeline. -**Which endpoints the supported pilot exposes is currently contradictory in the -repo.** `docs/deploy-railway.md` (the `ENABLE_STANDARD_MCP_ENDPOINT` row and -the post-deploy check) says that flag must be false or unset on a production -deployment ("Auto-minted permits on `POST /mcp`. Do not turn this on."), while -`README.md` and `SECURITY_LIMITATIONS.md` call `POST /mcp` the supported path -for new integrations. Until that is resolved, read every `POST /mcp` statement -below as applying only when that flag is on. +**Which endpoints the supported pilot exposes (decision recorded 2026-10-23).** +`POST /mcp/messages` and the REST route `POST /mcp/tools/{id}/invoke` are the +live governed path in the supported pilot. `POST /mcp` is the intended +standards-compliant path once an operator enables it, but it ships off +(`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false in `app/core/config.py`) and +the production runbook requires it to stay off (`docs/deploy-railway.md`, the +`ENABLE_STANDARD_MCP_ENDPOINT` row and the post-deploy check: "Auto-minted +permits on `POST /mcp`. Do not turn this on."), because it mints a short-lived, +single-tool permit sized to one call on the caller's behalf and generates an +idempotency key when the client sends none. `README.md` and +`SECURITY_LIMITATIONS.md` say the same. Every `POST /mcp` statement below +applies only when that flag is on. The agent never connects to the upstream. Every governed `tools/call` passes the pipeline in [`POLICY_ENFORCEMENT.md`](POLICY_ENFORCEMENT.md) before the @@ -62,15 +67,15 @@ defending against them, and it leaves gaps that §3 states plainly. | **Rug pull** (definition changes after startup) | Pinned for the agent, not detected | Discovery runs once at startup (`register_configured_upstream_mcp`, called from the `app/main.py` lifespan; that call-site fact is by code reading, not asserted). The agent-facing definition is the startup registry record and is never refreshed. The gateway does not notice a change, and it checks a result against no output schema, pinned or live. **The call session no longer consults the live upstream.** The SDK used to list tools after each call and validate the result against the live `outputSchema`, so a drifting or hostile schema could turn a completed call into a signed `delivery_uncertain`; the adapter now switches that validation off, so the live schema cannot change a call's outcome (§3.2). | `test_definition_rewritten_after_startup_does_not_reach_the_agent` (verified, agent-facing only); `test_live_output_schema_drift_must_not_turn_a_completed_call_uncertain`, `test_completed_call_with_drifted_live_schema_is_a_charged_success_end_to_end` (verified, §3.2) | | **Confused deputy** | Enforced at the agent boundary; textbook gap at the upstream boundary (§3.1) | Application-layer wallet checks run before the idempotency store is touched; a key cannot use another wallet's permit; a key-bound permit refuses another key. No row-level security. | `scripts/invariant_attacks/attack6_key_misuse.py` (HELD, run against the quickstart's local tool); `tests/test_trust_negative_security.py` (verified by name) | | **Token passthrough** | Structurally absent | The upstream credential is operator configuration (`MCP_UPSTREAM_BEARER_TOKEN`), loaded at startup and never read from a request. The agent's `X-API-Key` is never forwarded. A discovery page or tool result that contains the bearer string **verbatim** is rejected; encoded or partial reflections are not detected, and a rejected result is a charged call (`response_rejected` keeps the debit). | `_contains_exact_secret`, `upstream_mcp.py`; `test_discovery_rejects_bearer_token_echo_without_exposing_it`; `test_call_rejects_bearer_token_echo_without_returning_or_logging_it` (verified by name) | -| **Session hijack / event injection** | Structurally absent | `POST /mcp` is stateless: `mcp_session_id=None` (`mcp_standard.py:752`), `stateless=True` (`:768`), authenticated per request by `Depends(get_auth_context)` (`:788`); GET and DELETE return 405. `/mcp/messages` has no SSE transport (module docstring, `mcp.py:15`). The upstream client opens a fresh session per call. | `mcp_standard.py` (verified); `tests/test_standard_mcp_endpoint.py` (documented) | -| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a permit-bound call without an idempotency key is refused (`idempotency_key_required`, `mcp.py:1265-1278`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:400`) and the call runs, so an un-keyed retry is a new charged operation; the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | +| **Session hijack / event injection** | Structurally absent | `POST /mcp` is stateless: `mcp_session_id=None` (`mcp_standard.py:984`), `stateless=True` (`:1000`), authenticated per request by `Depends(get_auth_context)` (`:1020`); GET and DELETE return 405 (`standard_mcp_no_stream`, `:1034`). `/mcp/messages` has no SSE transport (module docstring, `mcp.py:15`). The upstream client opens a fresh session per call. | `mcp_standard.py` (verified); `tests/test_standard_mcp_endpoint.py` (documented) | +| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a governed call (permit-bound, or any call in strict trust mode) without an idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation; the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | | **Excessive scope / permission creep** | Enforced on `/mcp/messages` and REST; wallet-level only on `/mcp` | Permits are Ed25519-signed capabilities naming the tool, scopes, spend cap, use count, expiry, optional key binding. Wallet policy bundles add standing caps on every path. | `app/services/permits.py`, `app/services/policies.py`; `make red-team-trust-plane` (documented) | -| **Prompt injection, direct or indirect** | Contained, not prevented | The gateway does not see prompts. A hijacked agent holding a permit cannot act outside it, and an out-of-permit call against an existing permit produces a signed `denied` receipt. Refusals that happen before permit lookup (bad key, no permit, no idempotency key, wallet access) are audit events or HTTP errors, not receipts. Tool results are bounded and canonicalized but their content is passed through unlabeled (§3.3). | ASI01 row in the OWASP mapping (documented); `mcp.py:1355` (verified) | +| **Prompt injection, direct or indirect** | Contained, not prevented | The gateway does not see prompts. A hijacked agent holding a permit cannot act outside it, and an out-of-permit call against an existing permit produces a signed `denied` receipt (when the permit names the caller's wallet; a permit naming another wallet is not the caller's evidence and gets no receipt). Refusals that happen before permit lookup (bad key, no permit, no idempotency key, wallet access) are audit events or HTTP errors, not receipts. Tool results are bounded and canonicalized but their content is passed through unlabeled (§3.3). | ASI01 row in the OWASP mapping (documented); `mcp.py:2071-2072` (verified) | | **Local MCP server compromise** (client launches a process from configuration) | Not applicable | The gateway launches no process from configuration (§2). | search, §2 (verified) | | **SSRF via upstream URL** | Enforced | The URL is shape-validated, resolved, checked against private and reserved ranges, and pinned for the connection while preserving Host and SNI; redirects are refused. Validation runs on every connection (`_http_client` calls `validate_upstream_url` for each session), not once. Production requires public HTTPS. The upstream certificate is verified, not pinned. | `validate_upstream_url`, `_PinnedAsyncTransport`; `test_url_guard_*`, `test_http_transport_pins_validated_address_and_blocks_endpoint_drift` (verified by name) | -| **Argument injection into the tool call** | Enforced for named keys only | A permit's `forbidden_fields` match dict keys anywhere in the argument tree, exactly and case-sensitively (`_find_forbidden_field`, `permits.py:180`, enforced at `:892`); values are not scanned. The gateway does **not** validate call arguments against the published `inputSchema`; the discovery-time check only requires that schema to have type `object` or no type. Conformance is the upstream's job. | `permits.py` (verified); `test_discovery_requires_object_input_schema_or_absent_type` (verified) | +| **Argument injection into the tool call** | Enforced for named keys only | A permit's `forbidden_fields` match dict keys anywhere in the argument tree, exactly and case-sensitively (`_find_forbidden_field`, `permits.py:195`, enforced at `:1160`); values are not scanned. The gateway does **not** validate call arguments against the published `inputSchema`; the discovery-time check only requires that schema to have type `object` or no type. Conformance is the upstream's job. | `permits.py` (verified); `test_discovery_requires_object_input_schema_or_absent_type` (verified) | | **Upstream abusing client capabilities** (sampling, elicitation, roots, server `instructions`, prompts, resources) | Refused or ignored by SDK defaults; not tested here | The adapter builds `ClientSession(read_stream, write_stream)` with no callbacks, so the SDK answers server-initiated requests with its defaults, and the gateway never relays server `instructions`, prompts, or resources to the agent. | `upstream_mcp.py` `_session` (verified); behavior of the SDK defaults not tested | -| **Oversized or malformed payloads** | Enforced | Inbound bodies are capped on every route by the app-wide `RequestBodyLimitMiddleware` (`app/middleware/request_body_limit.py`, registered at `app/main.py:560`, `MAX_REQUEST_BODY_BYTES`, 1 MiB default) before the rate limiter; upstream responses are capped on the identity-encoded stream before buffering; discovery metadata is capped, paged, and tool-count-limited. | `SECURITY_LIMITATIONS.md`; `test_discovery_bounds_*`, `test_transport_*` (documented) | +| **Oversized or malformed payloads** | Enforced | Inbound bodies are capped on every route by the app-wide `RequestBodyLimitMiddleware` (`app/middleware/request_body_limit.py`, registered at `app/main.py:677`, `MAX_REQUEST_BODY_BYTES`, 1 MiB default) before the rate limiter; upstream responses are capped on the identity-encoded stream before buffering; discovery metadata is capped, paged, and tool-count-limited. | `SECURITY_LIMITATIONS.md`; `test_discovery_bounds_*`, `test_transport_*` (documented) | Two anti-patterns the MCP authorization guidance names deserve their own sentence each, because evaluators ask for them by name: @@ -138,9 +143,9 @@ launches a configured command over STDIO. This repository has none - The code that does spawn subprocesses is not an MCP transport and launches a fixed executable, never a configured command. The behavioral sandbox runs **caller-supplied code by design**: `docker run python -I -S -c ` - (`app/services/behavioral_sandbox.py:411`, image from operator settings) or, + (`app/services/behavioral_sandbox.py:412`, image from operator settings) or, behind `ALLOW_UNSAFE_HOST_PYTHON_SANDBOX`, `sys.executable -I -S -c ` - (`:471`). It is a proof surface, unmounted in production, and not a + (`:477`). It is a proof surface, unmounted in production, and not a production isolation boundary (`SECURITY_LIMITATIONS.md`); production refuses the host backend and the flag at boot. - The three code files that mention `stdio` are all server-side: @@ -180,12 +185,12 @@ version floor before this change, `mcp>=1.29.0`. | Advisory | Topic | Affected `mcp` (1.x) | Fixed | Posture here | |---|---|---|---|---| | GHSA-5h93-6whr-6q8j | Streamable HTTP client followed cross-origin redirects, forwarding custom headers and 307/308 bodies | `>=1.8.0,<1.30.0` | 1.30.0 | Also mitigated structurally: the upstream client sets `follow_redirects=False` (asserted by test) and the pinned transport refuses endpoint drift. | -| GHSA-fmmv-w9g8-j3gc (CVE-2026-109527) | Server transports and OAuth routes read request bodies with no size limit | `<1.29.1` | 1.29.1 | The SDK's own guard (`RequestBodyLimitMiddleware` inside the SDK transport wrapper, `mcp_standard.py:662`) is wired only for `/mcp/public` (256 KiB); the standard endpoint passes no limit to it. `/mcp` and `/mcp/messages` are bounded by the app-wide middleware (1 MiB default, `app/main.py:560`) that runs before the rate limiter. Excluded by the floor in any case. | +| GHSA-fmmv-w9g8-j3gc (CVE-2026-109527) | Server transports and OAuth routes read request bodies with no size limit | `<1.29.1` | 1.29.1 | The SDK's own guard (`RequestBodyLimitMiddleware` inside the SDK transport wrapper, `mcp_standard.py:894`) is wired only for `/mcp/public` (256 KiB); the standard endpoint passes no limit to it. `/mcp` and `/mcp/messages` are bounded by the app-wide middleware (1 MiB default, `app/main.py:677`) that runs before the rate limiter. Excluded by the floor in any case. | | GHSA-84m7-p3x7-pcfv (CVE-2026-59951) | Stateful Streamable HTTP sessions never reclaimed | `>=1.8.0,<1.30.0` | 1.30.0 | Not reachable: `/mcp` and `/mcp/public` are stateless with a fresh transport per POST; `/mcp/messages` does not use the SDK. | | GHSA-jpw9-pfvf-9f58 (CVE-2026-52869) | Session requests routed by session id without checking the principal | `<=1.27.1` | 1.27.2 | Not reachable (no sessions), and excluded by the floor. | | GHSA-w4fh-qvv9-3v23 | Server bearer-token auth had no audience check | `>=1.10.0,<1.30.0` | 1.30.0, plus opt-in `validate_token_resource=True` | Not used. `/mcp` and `/mcp/messages` authenticate through the gateway's own `get_auth_context` (`X-API-Key`; on deployments that enable it, a gateway-issued JWT or an enterprise bearer alongside a key); `/mcp/public` is unauthenticated and read-only by design. None imports `mcp.server.auth`. The upstream bearer is static operator configuration. | | GHSA-qx49-fqc8-xw99 | OAuth client could send credentials to a server-chosen token endpoint | `>=1.9.1,<1.30.0` | 1.30.0, plus `issuer=` on the provider | Not used: the upstream bearer is static operator configuration; there is no OAuth client. | -| GHSA-rwrf-2pqf-9j8j | Client fetched server-chosen `$ref` URLs while validating tool results | `>=1.10.0,<1.30.0` | 1.30.0 | **Closed on the call path by the adapter, in addition to the pin.** The SDK's `ClientSession.call_tool` used to fetch the live output schema after the call was dispatched and validate `structuredContent` against it (`mcp/client/session.py:412-447`, 1.30.0); that is where a hostile upstream's live `$ref` was reachable on any image below 1.30.0. The adapter now seeds the SDK's per-session schema cache before the dispatch checkpoint (`_bypass_sdk_result_validation`), so no `tools/list` follows the call and no result schema is compiled or resolved. `test_hostile_external_ref_in_live_output_schema_is_never_resolved` makes outbound connections fail and asserts none are attempted; `test_production_session_sends_no_tools_list_after_dispatch` observes the production session's HTTP traffic. The floor stays: the other advisories above still depend on it. The bypass uses a private SDK attribute; if a release drops it, calls fail closed before dispatch (§3.2). Result size is bounded by the response guard. | +| GHSA-rwrf-2pqf-9j8j | Client fetched server-chosen `$ref` URLs while validating tool results | `>=1.10.0,<1.30.0` | 1.30.0 | **Closed on the call path by the adapter, in addition to the pin.** The SDK's `ClientSession.call_tool` used to fetch the live output schema after the call was dispatched and validate `structuredContent` against it (`mcp/client/session.py:412-448`, 1.30.0); that is where a hostile upstream's live `$ref` was reachable on any image below 1.30.0. The adapter now seeds the SDK's per-session schema cache before the dispatch checkpoint (`_bypass_sdk_result_validation`), so no `tools/list` follows the call and no result schema is compiled or resolved. `test_hostile_external_ref_in_live_output_schema_is_never_resolved` makes outbound connections fail and asserts none are attempted; `test_production_session_sends_no_tools_list_after_dispatch` observes the production session's HTTP traffic. The floor stays: the other advisories above still depend on it. The bypass uses a private SDK attribute; if a release drops it, calls fail closed before dispatch (§3.2). Result size is bounded by the response guard. | | GHSA-vj7q-gjh5-988w (CVE-2026-59950) | Deprecated WebSocket server transport lacked Host/Origin validation | `<1.28.1` | 1.28.1 | Not used, and excluded by the floor. | | GHSA-hvrp-rf83-w775 (CVE-2026-52870) | Experimental task handlers allowed cross-client task access | `>=1.23.0,<=1.27.1` | 1.27.2 | Not used, and excluded by the floor. | | GHSA-9h52-p55h-vw2f (CVE-2025-66416) | DNS-rebinding protection off by default for localhost FastMCP servers | `<1.23.0` | 1.23.0 | Fixed in 1.23.0; excluded by the floor. | @@ -312,18 +317,50 @@ counted by the circuit breaker). The SDK parses the envelope inside `ValidationError` in an `ExceptionGroup` as the session block exits, so the `ValidationError` branch of `_call_tool_once` never matched and the generic handler classified it by `dispatch_started` alone. The adapter now records the -rejection where the `ValidationError` is raised, at the `call_tool` await, -before the session unwinds. That point is reached only after the upstream's -JSON-RPC result has arrived (a timeout, a dropped connection and a JSON-RPC -error answering `tools/call` raise other types), so a failure with no response -is still `delivery_uncertain`, and a malformed answer to `initialize` is still a -pre-dispatch failure. The same change gives the `ValidationError` branch the -"a result is already in hand" guard its two sibling handlers have, so a -`ValidationError` or `JSONDecodeError` raised only while the session closes can -no longer replace a response the adapter already holds. One consequence: a -malformed envelope is no longer counted by the circuit breaker, because the -`response_rejected` path does not feed it (an oversized or bearer-echoing result -never did either). +rejection where the error is raised, at the `call_tool` await, before the +session unwinds, and only for a `ValidationError` of the `CallToolResult` +model, which the SDK raises only after the upstream's JSON-RPC result has +arrived. A validation error for any other model, such as the request the SDK +builds before sending (reproduced with a non-string key in `arguments`: the +claim is committed and nothing reaches the upstream), says nothing about a +response and stays `delivery_uncertain`, as do a timeout, a dropped connection +and a JSON-RPC error answering `tools/call`. A malformed answer to `initialize` +is still a pre-dispatch failure. The same change gives the outer +`ValidationError` / `JSONDecodeError` branch the "a result is already in hand" +guard its two sibling handlers have, so such an error raised only while the +session closes can no longer replace a response the adapter already holds. + +Two consequences. A malformed envelope no longer counts toward the circuit +breaker, because the `response_rejected` path does not feed it (an oversized or +bearer-echoing result never did either). And with the optional reconcile worker +enabled (`UPSTREAM_RECONCILE_WORKER_ENABLED`, default off), `response_rejected` +stays terminal: the worker leases and probes only `delivery_uncertain` +attempts, so a rejected response is never probed and never refunded, whereas +an attested `not_executed` answer can refund a `delivery_uncertain` call when +the operator has also enabled `UPSTREAM_RECONCILE_REFUND_ON_ATTESTED_NOT_EXECUTED`. + +**`POST /mcp`, where enabled, tells the model not to resend.** `response_rejected` +is charged like `delivery_uncertain`, so it is returned the same way: a +`tools/call` result with `isError: true`, text saying the upstream answered but +the answer was unusable and the effect is unknown, the signed receipt, and +`_meta["io.agentmiddleware/outcome"]` with `status: "unknown"` and +`reason: "response_rejected"`. It had been a bare `-32006`, the code that +refunded, retry-safe failures (`upstream_returned_error`, +`upstream_pre_dispatch_failed`) also use, so a model shown only the message +could resend a call whose charge stands and whose tool probably ran. Those +refunded failures stay JSON-RPC errors, and `/mcp/messages` and REST are +unchanged (`_unknown_outcome_tool_result`, `mcp_standard.py`). + +**Not covered: a body that is not a JSON-RPC message at all.** Non-JSON text, +`"jsonrpc": "1.0"`, a body with neither `result` nor `error`, and a response +carrying an unknown request id all still end as `delivery_uncertain` after the +call timeout (each observed on 2026-10-23 against a scripted transport). The SDK +transport logs "Error parsing JSON response" and forwards the error into the +session, whose default message handler discards it, so the pending request is +never answered. Closing that would need a check in the transport guard +(`_PinnedAsyncTransport`) and is not built. The log line is the SDK's own and +quotes the start of the offending body, which is upstream-authored text; the +gateway does not change it. Evidence, real SDK session behind the real adapter (in memory, and over the production Streamable HTTP client against a scripted transport): @@ -333,14 +370,17 @@ and `test_unparseable_result_envelope_over_streamable_http_is_response_rejected` (the fixtures reach the client and fail there, not on a crashed server); `test_unparseable_envelope_error_does_not_echo_upstream_content`; the boundary pins `test_no_response_to_tools_call_stays_delivery_uncertain_in_memory`, -`test_transport_failure_over_streamable_http_stays_delivery_uncertain` and -`test_unparseable_initialize_response_stays_pre_dispatch`; and, through the -governed router, `test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch` -(one debit, one dispatch, a `response_rejected` receipt, a replay that sends -nothing; the JSON-RPC-error case stays `delivery_uncertain`). Residual case not -covered: an upstream body that is not JSON-RPC at all (not parseable as a -message) is dropped by the SDK transport without reaching the caller, so that -call times out and ends as `delivery_uncertain`. +`test_transport_failure_over_streamable_http_stays_delivery_uncertain`, +`test_unparseable_initialize_response_stays_pre_dispatch` and +`test_validation_error_before_the_request_is_sent_is_not_a_rejected_response`; +and, through the governed routers, +`test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch` +(`/mcp/messages`: one debit, one dispatch, a `response_rejected` receipt, a +replay that sends nothing, and the breaker closed; the JSON-RPC-error case stays +`delivery_uncertain` and opens it) and +`test_tools_call_unparseable_upstream_envelope_is_a_tool_result_the_model_can_read` +(`/mcp`), with `test_tools_call_other_governed_errors_stay_jsonrpc_errors` as the +refunded counterpart. **Still open, drift detection.** The gateway checks results against no output schema. If a customer needs drift detection, it belongs in the adapter, against the diff --git a/docs/research/gateway-overhead-2026-10-23/run1.json b/docs/research/gateway-overhead-2026-10-23/run1.json new file mode 100644 index 00000000..e4d95f99 --- /dev/null +++ b/docs/research/gateway-overhead-2026-10-23/run1.json @@ -0,0 +1,50 @@ +{ + "measured_at": "2026-10-23T10:01:50+00:00", + "machine": "arm64 / Darwin 25.6.0", + "python": "3.13.14", + "endpoint": "POST /mcp/messages (legacy, pre-minted permit)", + "calls": 1000, + "warmup": 20, + "provenance": { + "git_head": "0698168fc4a550b92b35a88e98a0e5395083f594", + "git_dirty": false, + "cpu": "Apple M4 Pro", + "cpu_count": 14, + "load_average_1_5_15": [ + 22.83, + 15.96, + 21.88 + ], + "packages": { + "sqlalchemy": "2.0.54", + "aiosqlite": "0.22.1", + "cryptography": "50.0.2", + "httpx": "0.28.1", + "mcp": "1.30.0" + } + }, + "posture": { + "trust_mode": "strict", + "database": "sqlite (temp file)", + "state_backend": "memory", + "transport": "in-process ASGI, no network", + "upstream_latency": "zero (in-process executor)" + }, + "first_call": { + "n": 1000, + "p50_ms": 29.52, + "p95_ms": 38.46, + "p99_ms": 77.19, + "max_ms": 97.5, + "mean_ms": 30.85 + }, + "replay": { + "n": 1000, + "p50_ms": 5.36, + "p95_ms": 6.58, + "p99_ms": 7.9, + "max_ms": 24.87, + "mean_ms": 5.51 + }, + "dispatches": 1020 +} diff --git a/docs/research/gateway-overhead-2026-10-23/run2.json b/docs/research/gateway-overhead-2026-10-23/run2.json new file mode 100644 index 00000000..284aa327 --- /dev/null +++ b/docs/research/gateway-overhead-2026-10-23/run2.json @@ -0,0 +1,50 @@ +{ + "measured_at": "2026-10-23T10:02:37+00:00", + "machine": "arm64 / Darwin 25.6.0", + "python": "3.13.14", + "endpoint": "POST /mcp/messages (legacy, pre-minted permit)", + "calls": 1000, + "warmup": 20, + "provenance": { + "git_head": "0698168fc4a550b92b35a88e98a0e5395083f594", + "git_dirty": false, + "cpu": "Apple M4 Pro", + "cpu_count": 14, + "load_average_1_5_15": [ + 33.83, + 19.83, + 23.0 + ], + "packages": { + "sqlalchemy": "2.0.54", + "aiosqlite": "0.22.1", + "cryptography": "50.0.2", + "httpx": "0.28.1", + "mcp": "1.30.0" + } + }, + "posture": { + "trust_mode": "strict", + "database": "sqlite (temp file)", + "state_backend": "memory", + "transport": "in-process ASGI, no network", + "upstream_latency": "zero (in-process executor)" + }, + "first_call": { + "n": 1000, + "p50_ms": 30.9, + "p95_ms": 76.62, + "p99_ms": 167.01, + "max_ms": 263.42, + "mean_ms": 38.15 + }, + "replay": { + "n": 1000, + "p50_ms": 5.64, + "p95_ms": 10.0, + "p99_ms": 26.16, + "max_ms": 97.21, + "mean_ms": 6.67 + }, + "dispatches": 1020 +} diff --git a/tests/test_mcp_upstream_tool_poisoning.py b/tests/test_mcp_upstream_tool_poisoning.py index b57edb00..cec6ff49 100644 --- a/tests/test_mcp_upstream_tool_poisoning.py +++ b/tests/test_mcp_upstream_tool_poisoning.py @@ -20,8 +20,9 @@ reaches the agent. One test records that residual channel instead of hiding it. * Tool *results* are relayed to the agent unlabeled; nothing here tests that. * "Discovery happens once at startup" is true by code reading (the only call - site of ``register_configured_upstream_mcp`` is the ``app/main.py`` lifespan), - not by any assertion here. + site of ``register_configured_upstream_mcp`` under ``app/`` is the + ``app/main.py`` lifespan; ``scripts/failure_lab.py`` also calls it, to run a + harness), not by any assertion here. * Most tests replace the adapter's MCP session with a fake, so they observe what the gateway's code does, not SDK wire traffic. The real SDK validates a result against the upstream's live schema after the call (mcp 1.30.0), so the tests From 67d56b3153a8ef66e1ac99e3fff40a439db7b511 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:55:02 -0700 Subject: [PATCH 4/6] fix(mcp): one parse-failure rule for both handlers; correct posture claims; finish the endpoint wording Second review of 0698168f and 66386796. Code. The outer ValidationError / JSONDecodeError branch of _call_tool_once still answered response_rejected for any such error after dispatch, so a session that does not wrap exceptions would again label the request-build error response_rejected. Both handlers now use one rule, _is_unparseable_response: after dispatch a CallToolResult validation error or a JSON decode error is response_rejected, and any other validation error is delivery_uncertain. Tests. A non-wrapping session double covers both directions. The /mcp guided-result test now covers an oversized result and a bearer echo as well as an unparseable envelope, and asserts nothing the upstream wrote is relayed. A spy test pins the gateway outcome recorded on /mcp (failed for a rejected response, unknown for an undelivered one); a mutation that always marked "unknown" used to pass every test. A new test pins that POST /mcp honours a permit the caller names, enforcing forbidden_fields with a signed denied receipt, and mints an unconstrained permit only when the reference is omitted. The keyless test asserts the generated key is readable in the receipt as the forwarded key, and the comment in mcp_standard.py that said it is never returned is corrected. Docs, claims that did not hold. The posture page said operator permit constraints apply only on /mcp/messages and REST (not so when the caller names a permit), that there is no KMS and no row-level security (both exist as opt-ins, default off), and listed governed_call causes incompletely. It now also notes that the refunded upstream_returned_error relays the upstream's result payload in error.data on all three surfaces. exit-and-export.md no longer says traces are not produced and says the only mounted DELETE route that deletes anything is for API keys. gateway-overhead.md quotes only the first-call p99 and max and records the kernel release rather than a macOS version the result files do not contain. The poisoning-test docstring lists all three call sites. Endpoint wording, leftovers. The REST invoke route's OpenAPI description no longer tells every integration to call POST /mcp; docs/openapi.json is regenerated with scripts/export_openapi.py and differs by that one line. The Cursor plugin README and skill say the standard endpoint ships off. README names the REST invoke route as supported; authority-required-flow and two test docstrings no longer present /mcp as the surface integrations are told to use. The plugin's mcp.json, which targets /mcp on the first-party host, is deliberately unchanged: that is a product decision. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 51 +++- README.md | 7 +- app/routers/mcp.py | 11 +- app/routers/mcp_standard.py | 6 +- app/services/upstream_mcp.py | 28 +- docs/authority-required-flow.md | 8 +- docs/exit-and-export.md | 8 +- docs/failure-semantics.md | 2 +- docs/gateway-overhead.md | 12 +- docs/mcp-security-posture.md | 58 ++-- docs/openapi.json | 2 +- .../cursor-plugin-agent-middleware/README.md | 2 +- .../skills/amw-permit-receipt/SKILL.md | 2 +- tests/test_audit_chain_contention_surface.py | 4 +- tests/test_mcp_upstream_governed.py | 34 ++- tests/test_mcp_upstream_tool_poisoning.py | 5 +- .../test_receipt_write_contention_surface.py | 4 +- tests/test_standard_mcp_endpoint.py | 270 ++++++++++++++++-- tests/test_upstream_mcp.py | 61 ++++ 19 files changed, 479 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2854d9ca..2f9e5cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 obtained. The outer `ValidationError` / `JSONDecodeError` branch did not apply the "a result is already in hand" rule, so such an error raised only while the session closes could turn a received result into `response_rejected`, or - replace a known rejection's code. It now applies it. + replace a known rejection's code. It now applies it, and applies the same + parse-failure rule as the `call_tool` await (`_is_unparseable_response`): after + dispatch, a `CallToolResult` validation error or a JSON decode error is + `response_rejected`, and any other validation error is `delivery_uncertain`. + That branch is reached only by a session that does not wrap exceptions, where + the request-build error above would otherwise have been labeled a rejection + again. - **Behavior change, `POST /mcp` (where enabled):** `response_rejected` is charged like `delivery_uncertain`, so it is now returned like it: a `tools/call` result with `isError: true`, text saying the upstream answered but @@ -83,9 +89,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `status: "unknown"` and `reason: "response_rejected"`. It was a bare `-32006`, the code that refunded, retry-safe failures (`upstream_returned_error`, `upstream_pre_dispatch_failed`) also use, so a model shown only the message - could resend a call whose charge stands and whose tool probably ran. Those - refunded failures stay JSON-RPC errors. `/mcp/messages` and REST are - unchanged. The helper is now `_unknown_outcome_tool_result` + could resend a call whose charge stands and whose tool probably ran. This + covers every cause of `response_rejected`: an unparseable envelope, an + oversized result, and a result echoing the bearer token. Those refunded + failures stay JSON-RPC errors. `/mcp/messages` and REST are unchanged. The helper is now `_unknown_outcome_tool_result` (was `_delivery_uncertain_tool_result`). - **Behavior change:** a malformed envelope is no longer counted toward the optional circuit breaker. The `response_rejected` path never fed it (an @@ -105,8 +112,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - New tests drive a real SDK session behind the real adapter, in memory and over the production Streamable HTTP client: four envelope shapes, a fixture guard (the envelope must reach the client and fail there), the no-response - boundary, a request that cannot be built stays `delivery_uncertain`, a - malformed `initialize` stays pre-dispatch, a secret-echo check, and + boundary, a request that cannot be built stays `delivery_uncertain` (also with + a session that does not wrap exceptions), a malformed `initialize` stays + pre-dispatch, a secret-echo check, the gateway outcome recorded on `/mcp` + (`failed` for a rejected response, `unknown` for an undelivered one), and governed-router tests on `/mcp/messages` (one debit, one dispatch, a `response_rejected` receipt, no redispatch on replay, circuit breaker closed, against a JSON-RPC error that opens it) and on `/mcp`. See @@ -142,6 +151,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `docs/research/gateway-overhead-2026-10-23/`. The public site's figure is not touched. - `TROUBLESHOOTING.md` gains a `response_rejected` receipt section. +- Further claims corrected after a second review. The posture page said + operator permit constraints apply only on `/mcp/messages` and REST; `POST /mcp` + honours a permit the caller names (`_meta["io.agentmiddleware/permit_id"]` or + `mcpContext.permit_id`), enforcing `forbidden_fields` with a signed `denied` + receipt, refuses action-bound tools without one, and mints an unconstrained + permit only when the reference is omitted (now pinned by a test). It said there + is no KMS and no row-level security; an AWS KMS signing backend + (`TRUST_SIGNING_BACKEND=aws_kms`) and PostgreSQL row-level security + (`TENANT_RLS_MODE=enforce`) exist as opt-ins, default off. The idempotency-key + refusal also covers action-bound calls and tools that require a permit. The + posture page now notes that the refunded `upstream_returned_error` relays the + upstream's result payload in `error.data.upstream_result` on all three + surfaces, and that a keyless `/mcp` call's generated key is readable in the + signed receipt as the forwarded key. `docs/exit-and-export.md` no longer says + traces are not produced, and says the only mounted `DELETE` route that deletes + anything is for API keys (the `/mcp` and `/mcp/public` `DELETE` handlers are + stubs). `docs/gateway-overhead.md` quotes only the first-call p99 and max, and + records the kernel release rather than a macOS version the result files do not + contain. A comment in `mcp_standard.py` that said a keyless call's key is + "never returned" is corrected. +- Endpoint wording, leftovers. The REST invoke route's OpenAPI description no + longer tells every integration to call `POST /mcp`; it points at + `/.well-known/agent.json` as the `/mcp/messages` description does, and + `docs/openapi.json` is regenerated. The Cursor plugin README and skill say the + standard endpoint ships off and the plugin works only against a deployment that + enables it. `README.md` names the REST invoke route as supported; + `docs/authority-required-flow.md` and two test docstrings no longer present + `/mcp` as the surface integrations are told to use. The plugin's + `mcp.json`, which targets `/mcp` on the first-party host, is deliberately left + as is: that is a product decision. ### Added — per-upstream concurrency limit and circuit breaker (default off) diff --git a/README.md b/README.md index f34d7822..ec9e4ff2 100644 --- a/README.md +++ b/README.md @@ -286,9 +286,10 @@ curl -sS -X POST "$API_URL/mcp/messages" \ }" ``` -`POST /mcp/messages` is the JSON-RPC transport the supported pilot runs on — -marked deprecated in the OpenAPI contract, fully supported, and the only -JSON-RPC surface in a default local run. `POST /mcp` is the standards-compliant +`POST /mcp/messages` and the REST invoke route `POST /mcp/tools/{id}/invoke` +are what the supported pilot runs on — both marked deprecated in the OpenAPI +contract, both fully supported, and `/mcp/messages` is the only JSON-RPC +surface in a default local run. `POST /mcp` is the standards-compliant Streamable HTTP endpoint on the official MCP SDK. It ships off (`ENABLE_STANDARD_MCP_ENDPOINT`, default false) and the production runbook ([docs/deploy-railway.md](docs/deploy-railway.md)) requires it to stay off, diff --git a/app/routers/mcp.py b/app/routers/mcp.py index 205aced9..9641b097 100644 --- a/app/routers/mcp.py +++ b/app/routers/mcp.py @@ -5145,11 +5145,12 @@ async def _handle_tools_call( summary="Invoke a registered MCP tool", deprecated=True, description=( - "Legacy REST-shaped invoke, kept for existing clients. New " - "integrations should call tools through the standard MCP Streamable " - "HTTP endpoint at POST /mcp (or the legacy JSON-RPC POST " - "/mcp/messages); every entry point runs the same governed " - "permit→meter→receipt path." + "Legacy REST-shaped invoke, kept for existing clients. Check " + "`/.well-known/agent.json` before assuming the standard MCP " + "Streamable HTTP endpoint at POST /mcp is available; it ships off. " + "Where it is enabled, new integrations can call tools through it; " + "otherwise use POST /mcp/messages or this route. Every enabled entry " + "point runs the same governed permit→meter→receipt path." ), ) async def invoke_tool( diff --git a/app/routers/mcp_standard.py b/app/routers/mcp_standard.py index c7af78bb..fc030093 100644 --- a/app/routers/mcp_standard.py +++ b/app/routers/mcp_standard.py @@ -443,9 +443,9 @@ def _unknown_outcome_tool_result( """ receipt = error.receipt or {} receipt_id = receipt.get("receipt_id") - # A keyless call's key was generated here and never returned, so the - # caller cannot replay it: both the text and the structured remediation - # must say a new attempt is a new charge rather than promise a replay. + # A keyless call's key was generated here and is not returned as a field + # (an upstream call's receipt carries it as the forwarded key, by design). + # So the text and remediation say a new attempt is a new charge, no replay. if client_key_supplied: retry_guidance = ( "Repeating this request with the same Idempotency-Key returns this " diff --git a/app/services/upstream_mcp.py b/app/services/upstream_mcp.py index de813a30..e2a25606 100644 --- a/app/services/upstream_mcp.py +++ b/app/services/upstream_mcp.py @@ -655,6 +655,19 @@ async def _await_guarded_response( await asyncio.gather(operation_task, rejection_task, return_exceptions=True) +def _is_unparseable_response(exc: Exception) -> bool: + """Whether ``exc`` means a response was received but could not be parsed. + + A JSON decode failure, or a validation error for the ``CallToolResult`` + model, which the SDK raises only after the upstream's JSON-RPC result has + arrived. A validation error for any other model, such as the request the + SDK builds before sending, says nothing about a response. + """ + if isinstance(exc, json.JSONDecodeError): + return True + return isinstance(exc, ValidationError) and exc.title == CallToolResult.__name__ + + def _contains_exact_secret(value: Any, secret: str) -> bool: """Return whether an untrusted JSON-like value reflects the bearer token.""" if isinstance(value, str): @@ -1094,7 +1107,7 @@ async def _call_tool_once( # below can match. Any other validation error, such as one # building the request before it is sent, says nothing # about a response and is classified as before. - if exc.title != CallToolResult.__name__: + if not _is_unparseable_response(exc): raise response_rejected = UpstreamMcpResponseRejectedError( "upstream_response_invalid" @@ -1126,7 +1139,7 @@ async def _call_tool_once( ) else: raise - except (ValidationError, json.JSONDecodeError): + except (ValidationError, json.JSONDecodeError) as exc: if dispatch_claim_unavailable is not None: raise dispatch_claim_unavailable if response_guard.rejection_code is not None: @@ -1148,9 +1161,14 @@ async def _call_tool_once( }, ) elif dispatch_started: - raise UpstreamMcpResponseRejectedError( - "upstream_response_invalid" - ) from None + # The same rule as at the call_tool await. Only a failure to + # parse a response is a rejection; any other validation error + # after dispatch leaves the outcome unknown. + if _is_unparseable_response(exc): + raise UpstreamMcpResponseRejectedError( + "upstream_response_invalid" + ) from None + raise UpstreamMcpDeliveryUncertainError() from None else: raise UpstreamMcpPreDispatchError() from None except Exception: diff --git a/docs/authority-required-flow.md b/docs/authority-required-flow.md index 20a46d89..117681f1 100644 --- a/docs/authority-required-flow.md +++ b/docs/authority-required-flow.md @@ -132,10 +132,10 @@ caller lacks authority for it right now" returns: } ``` -Scope: the standard `/mcp` surface. `POST /mcp/messages` is deprecated in -favor of `/mcp` and stays frozen — its clients hand-orchestrate permits by -definition, and rewriting a deprecated surface widens the change for no -partner value. +Scope: the standard `/mcp` surface, which ships off. `POST /mcp/messages` is +deprecated in favor of `/mcp` once an operator enables it, and stays frozen — +its clients hand-orchestrate permits by definition, and rewriting a deprecated +surface widens the change for no partner value. ### D2. Resume semantics: same key while undecided, new key after a decided denial diff --git a/docs/exit-and-export.md b/docs/exit-and-export.md index 4281693f..acebf040 100644 --- a/docs/exit-and-export.md +++ b/docs/exit-and-export.md @@ -76,7 +76,9 @@ a database copy and a signing key (§4, step 7). ## 3. What does not leave, and what is not promised -- No traces, because none are produced. +- No exportable traces. Optional OpenTelemetry tracing exists but is off by + default (`OTEL_TRACING_ENABLED`) and exports only an allowlist of attributes + to an operator-run collector (see the Traces row in §1). - No independent audit-chain verifier yet; the check is first-party. - **The audit chain is tamper-evident, not immutable.** Whoever administers the database can delete rows, and whoever holds the signing key can rewrite @@ -90,7 +92,9 @@ a database copy and a signing key (§4, step 7). - **No deletion controls.** `docs/deploy-railway.md` lists retention, deletion, and legal-hold controls among things not yet built. No route deletes receipts, ledger rows, audit events, or stored results; the - only core `DELETE` route is for API keys. Removal means the operator + only mounted `DELETE` route that deletes anything is for API keys (`DELETE + /mcp` and `DELETE /mcp/public` are stubs that refuse and delete nothing). + Removal means the operator deleting the customer's deployment project. No timeline is documented (not verified), and provider backups are not addressed. - No documented procedure, timeline, or commitment for handing over a diff --git a/docs/failure-semantics.md b/docs/failure-semantics.md index 8d96b77a..e7f2b7ed 100644 --- a/docs/failure-semantics.md +++ b/docs/failure-semantics.md @@ -110,7 +110,7 @@ never probed or refunded. And on `POST /mcp`, where enabled, both arrive as a `-32006` is also the code of refunded, retry-safe failures (`upstream_returned_error`, `upstream_pre_dispatch_failed`), which stay errors. `/mcp/messages` and REST keep `-32005` and `-32006` -(`test_tools_call_unparseable_upstream_envelope_is_a_tool_result_the_model_can_read`). +(`test_tools_call_rejected_upstream_response_is_a_tool_result_the_model_can_read`). Requests rejected before a valid permit and an executable tool are established produce no receipt — there is no authority to bill against. The README states diff --git a/docs/gateway-overhead.md b/docs/gateway-overhead.md index cf37f1c5..5e3281ec 100644 --- a/docs/gateway-overhead.md +++ b/docs/gateway-overhead.md @@ -28,11 +28,11 @@ permit), strict trust mode, zero-latency in-process upstream: | **Replay** (same key, stored receipt returned, nothing re-executed) | **5.4–5.6 ms** | 6.6–10.0 ms | 2 × 1,000 | Two runs back to back, 20 warm-up calls discarded. Ranges are the spread -between the two runs. The p99 and max values are in the raw files but are not -quoted here: p99 varied from 77 to 167 ms and max from 97 to 263 ms between -runs, and the first-call p95 from 38 to 77 ms, which is scheduler noise on a -loaded machine (the second run started at a load average of 34), not a property -of the gateway. +between the two runs. The p99 and max values are in the raw files, and only the +first-call ones are quoted here: first-call p99 varied from 77 to 167 ms and max +from 97 to 263 ms between runs, and the first-call p95 from 38 to 77 ms, which +is scheduler noise on a loaded machine (the second run started at a load average +of 34), not a property of the gateway. What the first call pays for, in pipeline order ([`POLICY_ENFORCEMENT.md`](POLICY_ENFORCEMENT.md) §2): API-key hash lookup and @@ -123,7 +123,7 @@ stderr. | | | |---|---| -| Machine | Apple M4 Pro, 14 cores, macOS 26.7.2 (Darwin 25.6.0), Python 3.13.14 | +| Machine | Apple M4 Pro, 14 cores, Darwin 25.6.0 (the result files record the kernel release, not the macOS marketing version), Python 3.13.14 | | Load | **Not idle.** Load average (1/5/15 min) was 22.8/16.0/21.9 at the start of run 1 and 33.8/19.8/23.0 at run 2, on a machine running many concurrent sessions. Both runs are therefore pessimistic, and run 2 (the heavier) shows it in its tail: first-call p95 77 ms against 38 ms. Their agreement on the p50 should not be read as proof of stability. | | Code | commit `0698168f`, with `git_dirty` false in both result files (the script counts tracked files only). The documentation commit that follows changes no code. The measuring script is the version committed with this page | | Packages | sqlalchemy 2.0.54, aiosqlite 0.22.1, cryptography 50.0.2, httpx 0.28.1, mcp 1.30.0 | diff --git a/docs/mcp-security-posture.md b/docs/mcp-security-posture.md index 0e8d4753..e0be0400 100644 --- a/docs/mcp-security-posture.md +++ b/docs/mcp-security-posture.md @@ -14,7 +14,9 @@ Last updated: 2026-10-23. Claims name their evidence where it exists. Labels: **Deployment boundary.** The supported deployment is one vendor-managed project per customer, restricted to synthetic or redacted, low-sensitivity -data. Wallet checks are application-layer isolation: there is no row-level +data. Wallet checks are application-layer isolation. PostgreSQL row-level +security exists only as an opt-in defense in depth (`TENANT_RLS_MODE=enforce`, +migration `048_tenant_rls`, default `off`); while it is off there is no row-level security and no tenant-isolation guarantee (`SECURITY_LIMITATIONS.md`). ## What this gateway is, in MCP terms @@ -47,12 +49,19 @@ applies only when that flag is on. The agent never connects to the upstream. Every governed `tools/call` passes the pipeline in [`POLICY_ENFORCEMENT.md`](POLICY_ENFORCEMENT.md) before the -gateway's client sends anything. One difference matters for the rows below: on -`POST /mcp` the gateway **mints a single-tool, single-call permit itself** for -whoever holds a wallet key (`_mint_auto_permit`, `mcp_standard.py`), so -operator-set permit constraints (scopes, `forbidden_fields`, use counts) apply -only on `/mcp/messages` and REST invoke. On `/mcp` the limits are the wallet -balance and wallet policy bundles. +gateway's client sends anything. One difference matters for the rows below. +`POST /mcp` honours a permit the caller names (`params._meta +["io.agentmiddleware/permit_id"]` or `mcpContext.permit_id`, +`_action_permit_reference`, `mcp_standard.py:258`) and then applies the +operator's permit constraints (scopes, `forbidden_fields`, use counts) as +`/mcp/messages` does: a violation is a signed `denied` receipt, and an +action-bound tool refuses without a named permit (`action_permit_required`). But +a caller who omits the reference, on a tool that is not action-bound, gets a +single-tool permit the gateway **mints itself**, sized to one call +(`_mint_auto_permit`, `mcp_standard.py`), which carries none of those +constraints. For that caller the limits are the wallet balance and wallet +policy bundles. The gap is the omitted reference, not the endpoint +(`test_tools_call_honours_a_named_permit_but_mints_an_unconstrained_one_otherwise`). That shape removes several MCP attack classes structurally rather than defending against them, and it leaves gaps that §3 states plainly. @@ -65,11 +74,11 @@ defending against them, and it leaves gaps that §3 states plainly. | **Tool poisoning, input schema** | **Residual channel** | `inputSchema` is published verbatim, so any string inside it (property descriptions, titles, defaults, enum values, property names) is upstream-authored text that reaches the agent. It is bounded by `MCP_UPSTREAM_MAX_RESPONSE_BYTES` (default 1 MiB, summed over the whole `tools/list`), which limits memory, not influence, and rejected if it echoes the bearer token. Not rewritten. `outputSchema` is kept in the registry record only; the catalog advertises a `hasOutputSchema` flag. | `test_hostile_text_inside_input_schema_reaches_the_catalog_verbatim` (verified) | | **Tool shadowing / name collision** | Refused at startup | The configured tool name must appear exactly once in `tools/list`; two entries, or zero, abort registration. The startup call is unguarded in the lifespan, so the deployment does not boot. | `test_duplicated_tool_name_aborts_registration_before_any_dispatch`; `test_discovery_fails_closed_when_exact_tool_is_absent` (verified) | | **Rug pull** (definition changes after startup) | Pinned for the agent, not detected | Discovery runs once at startup (`register_configured_upstream_mcp`, called from the `app/main.py` lifespan; that call-site fact is by code reading, not asserted). The agent-facing definition is the startup registry record and is never refreshed. The gateway does not notice a change, and it checks a result against no output schema, pinned or live. **The call session no longer consults the live upstream.** The SDK used to list tools after each call and validate the result against the live `outputSchema`, so a drifting or hostile schema could turn a completed call into a signed `delivery_uncertain`; the adapter now switches that validation off, so the live schema cannot change a call's outcome (§3.2). | `test_definition_rewritten_after_startup_does_not_reach_the_agent` (verified, agent-facing only); `test_live_output_schema_drift_must_not_turn_a_completed_call_uncertain`, `test_completed_call_with_drifted_live_schema_is_a_charged_success_end_to_end` (verified, §3.2) | -| **Confused deputy** | Enforced at the agent boundary; textbook gap at the upstream boundary (§3.1) | Application-layer wallet checks run before the idempotency store is touched; a key cannot use another wallet's permit; a key-bound permit refuses another key. No row-level security. | `scripts/invariant_attacks/attack6_key_misuse.py` (HELD, run against the quickstart's local tool); `tests/test_trust_negative_security.py` (verified by name) | +| **Confused deputy** | Enforced at the agent boundary; textbook gap at the upstream boundary (§3.1) | Application-layer wallet checks run before the idempotency store is touched; a key cannot use another wallet's permit; a key-bound permit refuses another key. Row-level security is opt-in (`TENANT_RLS_MODE`, default `off`) and is not relied on here. | `scripts/invariant_attacks/attack6_key_misuse.py` (HELD, run against the quickstart's local tool); `tests/test_trust_negative_security.py` (verified by name) | | **Token passthrough** | Structurally absent | The upstream credential is operator configuration (`MCP_UPSTREAM_BEARER_TOKEN`), loaded at startup and never read from a request. The agent's `X-API-Key` is never forwarded. A discovery page or tool result that contains the bearer string **verbatim** is rejected; encoded or partial reflections are not detected, and a rejected result is a charged call (`response_rejected` keeps the debit). | `_contains_exact_secret`, `upstream_mcp.py`; `test_discovery_rejects_bearer_token_echo_without_exposing_it`; `test_call_rejects_bearer_token_echo_without_returning_or_logging_it` (verified by name) | | **Session hijack / event injection** | Structurally absent | `POST /mcp` is stateless: `mcp_session_id=None` (`mcp_standard.py:984`), `stateless=True` (`:1000`), authenticated per request by `Depends(get_auth_context)` (`:1020`); GET and DELETE return 405 (`standard_mcp_no_stream`, `:1034`). `/mcp/messages` has no SSE transport (module docstring, `mcp.py:15`). The upstream client opens a fresh session per call. | `mcp_standard.py` (verified); `tests/test_standard_mcp_endpoint.py` (documented) | -| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a governed call (permit-bound, or any call in strict trust mode) without an idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation; the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | -| **Excessive scope / permission creep** | Enforced on `/mcp/messages` and REST; wallet-level only on `/mcp` | Permits are Ed25519-signed capabilities naming the tool, scopes, spend cap, use count, expiry, optional key binding. Wallet policy bundles add standing caps on every path. | `app/services/permits.py`, `app/services/policies.py`; `make red-team-trust-plane` (documented) | +| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a governed call (permit-bound, action-bound, a tool that requires a permit, or any call in strict trust mode) without an idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation (for an upstream call the generated key is readable in the signed receipt, as `constraints_evaluated.upstream_dispatch.forwarded_idempotency_key`, by design, as the provider lookup handle); the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | +| **Excessive scope / permission creep** | Enforced on `/mcp/messages` and REST, and on `/mcp` when the caller names a permit; wallet-level on `/mcp` when it does not | Permits are Ed25519-signed capabilities naming the tool, scopes, spend cap, use count, expiry, optional key binding. Wallet policy bundles add standing caps on every path. | `app/services/permits.py`, `app/services/policies.py`; `make red-team-trust-plane` (documented) | | **Prompt injection, direct or indirect** | Contained, not prevented | The gateway does not see prompts. A hijacked agent holding a permit cannot act outside it, and an out-of-permit call against an existing permit produces a signed `denied` receipt (when the permit names the caller's wallet; a permit naming another wallet is not the caller's evidence and gets no receipt). Refusals that happen before permit lookup (bad key, no permit, no idempotency key, wallet access) are audit events or HTTP errors, not receipts. Tool results are bounded and canonicalized but their content is passed through unlabeled (§3.3). | ASI01 row in the OWASP mapping (documented); `mcp.py:2071-2072` (verified) | | **Local MCP server compromise** (client launches a process from configuration) | Not applicable | The gateway launches no process from configuration (§2). | search, §2 (verified) | | **SSRF via upstream URL** | Enforced | The URL is shape-validated, resolved, checked against private and reserved ranges, and pinned for the connection while preserving Host and SNI; redirects are refused. Validation runs on every connection (`_http_client` calls `validate_upstream_url` for each session), not once. Production requires public HTTPS. The upstream certificate is verified, not pinned. | `validate_upstream_url`, `_PinnedAsyncTransport`; `test_url_guard_*`, `test_http_transport_pins_validated_address_and_blocks_endpoint_drift` (verified by name) | @@ -371,16 +380,19 @@ and `test_unparseable_result_envelope_over_streamable_http_is_response_rejected` `test_unparseable_envelope_error_does_not_echo_upstream_content`; the boundary pins `test_no_response_to_tools_call_stays_delivery_uncertain_in_memory`, `test_transport_failure_over_streamable_http_stays_delivery_uncertain`, -`test_unparseable_initialize_response_stays_pre_dispatch` and -`test_validation_error_before_the_request_is_sent_is_not_a_rejected_response`; +`test_unparseable_initialize_response_stays_pre_dispatch`, +`test_validation_error_before_the_request_is_sent_is_not_a_rejected_response` and +`test_a_session_that_does_not_wrap_errors_is_classified_by_the_same_rule`; and, through the governed routers, `test_real_sdk_upstream_failures_get_the_right_charged_receipt_and_never_redispatch` (`/mcp/messages`: one debit, one dispatch, a `response_rejected` receipt, a replay that sends nothing, and the breaker closed; the JSON-RPC-error case stays `delivery_uncertain` and opens it) and -`test_tools_call_unparseable_upstream_envelope_is_a_tool_result_the_model_can_read` -(`/mcp`), with `test_tools_call_other_governed_errors_stay_jsonrpc_errors` as the -refunded counterpart. +`test_tools_call_rejected_upstream_response_is_a_tool_result_the_model_can_read` +(`/mcp`; unparseable, oversized and bearer-echo causes) and +`test_tools_call_marks_the_gateway_outcome_of_each_charged_unknown_outcome`, with +`test_tools_call_other_governed_errors_stay_jsonrpc_errors` as the refunded +counterpart. **Still open, drift detection.** The gateway checks results against no output schema. If a customer needs drift detection, it belongs in the adapter, against the @@ -393,14 +405,22 @@ capability and is held under the named-partner rule in `AGENTS.md`. The gateway caps and canonicalizes every tool result and checks it for the exact upstream bearer string, but returns its content to the agent unmarked. Any other credential in a result passes; so does an encoded or split copy of -the bearer. Labeling results as untrusted for the agent's context assembly is +the bearer. A tool-level error result is relayed too: the refunded +`upstream_returned_error` carries the upstream's result payload, which is +upstream-authored text, in `error.data.upstream_result` on `/mcp/messages`, REST +and `/mcp`. Labeling results as untrusted for the agent's context assembly is a runtime concern above this boundary. ### 3.4 Custody and network -`TRUST_SIGNING_PRIVATE_KEY_B64` is an environment variable and there is no -KMS (`SECURITY_LIMITATIONS.md`), so a host compromise can forge permits and -receipts. `MCP_UPSTREAM_BEARER_TOKEN` is a deployment variable read at +By default (`TRUST_SIGNING_BACKEND=env`) the signing key is the environment +variable `TRUST_SIGNING_PRIVATE_KEY_B64`, so a host compromise can forge permits +and receipts. An AWS KMS backend exists as an opt-in +(`TRUST_SIGNING_BACKEND=aws_kms`, `app/services/kms_signer.py`): the private key +then never enters the process, at the cost of a blocking network call per +signature (`SECURITY_LIMITATIONS.md`, which also lists what is not implemented: +other KMS providers and Vault Transit). A deployment that has not opted in has +no KMS. `MCP_UPSTREAM_BEARER_TOKEN` is a deployment variable read at startup, so rotating it needs a restart. An egress allowlist for the single upstream origin is the operator's job. The upstream certificate is verified but not pinned. diff --git a/docs/openapi.json b/docs/openapi.json index 1c52268a..0faab8cd 100644 --- a/docs/openapi.json +++ b/docs/openapi.json @@ -3377,7 +3377,7 @@ "MCP" ], "summary": "Invoke a registered MCP tool", - "description": "Legacy REST-shaped invoke, kept for existing clients. New integrations should call tools through the standard MCP Streamable HTTP endpoint at POST /mcp (or the legacy JSON-RPC POST /mcp/messages); every entry point runs the same governed permit→meter→receipt path.", + "description": "Legacy REST-shaped invoke, kept for existing clients. Check `/.well-known/agent.json` before assuming the standard MCP Streamable HTTP endpoint at POST /mcp is available; it ships off. Where it is enabled, new integrations can call tools through it; otherwise use POST /mcp/messages or this route. Every enabled entry point runs the same governed permit→meter→receipt path.", "operationId": "Invoke_MCP_Tool_mcp_tools__service_id__invoke_post", "deprecated": true, "security": [ diff --git a/integrations/cursor-plugin-agent-middleware/README.md b/integrations/cursor-plugin-agent-middleware/README.md index 8ca28db1..ec09907b 100644 --- a/integrations/cursor-plugin-agent-middleware/README.md +++ b/integrations/cursor-plugin-agent-middleware/README.md @@ -6,7 +6,7 @@ Public-ready scaffold, not yet submitted to any marketplace. Layout mirrors the The `agent-middleware` plugin connects Cursor to the Agent Middleware API so that side-effecting tool calls follow one guardrail: permit before action, receipt after. -- `plugins/agent-middleware/mcp.json` points at the AMW API base `https://api.thisisatest.tech`, standard MCP endpoint `POST /mcp` (Streamable HTTP, the preferred endpoint in this repo; legacy JSON-RPC `POST /mcp/messages` also exists). The API key is the `${AMW_API_KEY}` environment placeholder. No real key is stored anywhere in this tree. +- `plugins/agent-middleware/mcp.json` points at the AMW API base `https://api.thisisatest.tech`, standard MCP endpoint `POST /mcp` (Streamable HTTP). That endpoint ships off (`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false) and the production runbook keeps it off, so the plugin works only against a deployment whose operator has enabled it; the live governed path otherwise is legacy JSON-RPC `POST /mcp/messages`, which the plugin does not use. The API key is the `${AMW_API_KEY}` environment placeholder. No real key is stored anywhere in this tree. - `plugins/agent-middleware/skills/amw-permit-receipt/SKILL.md` tells the agent to request a permit (`POST /v1/permits`) before any side-effecting tool call and to record the signed receipt (`GET /v1/receipts/{receipt_id}`) after the action. Note on transport: the endpoint path (`/mcp`) and the `X-API-Key` header shape were checked against this repo (standard MCP router, repo `.mcp.json`, quickstart docs). Cursor's exact remote-server key names may vary by version, so confirm the `mcp.json` fields against your Cursor version when installing. diff --git a/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/skills/amw-permit-receipt/SKILL.md b/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/skills/amw-permit-receipt/SKILL.md index 64195a79..ade810ca 100644 --- a/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/skills/amw-permit-receipt/SKILL.md +++ b/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/skills/amw-permit-receipt/SKILL.md @@ -16,7 +16,7 @@ Govern side-effecting tool calls through the Agent Middleware API: permit before ## Instructions 1. Before the side-effecting call, request a permit from the Agent Middleware API (`POST /v1/permits`). The permit names the wallet, the allowed tool, and the spend cap. Do not proceed without one. -2. Pass the issued `permit_id` with the governed invocation (standard MCP endpoint `POST /mcp`, legacy JSON-RPC `POST /mcp/messages`). +2. Pass the issued `permit_id` with the governed invocation (legacy JSON-RPC `POST /mcp/messages`, or the standard MCP endpoint `POST /mcp` on a deployment whose operator has enabled it; it ships off). 3. After the action, record the receipt: keep the returned receipt id and payload with the permit id that authorized it, so the action can be audited later (`GET /v1/receipts/{receipt_id}`, or `GET /v1/permits/{permit_id}/receipts`). 4. If the call is denied (for example the spend cap is reached), keep the denial response with the same permit reference instead of retrying with a different permit. 5. Never store API keys in files, prompts, or receipts. The key always comes from the `AMW_API_KEY` environment placeholder. diff --git a/tests/test_audit_chain_contention_surface.py b/tests/test_audit_chain_contention_surface.py index d7719620..e492d501 100644 --- a/tests/test_audit_chain_contention_surface.py +++ b/tests/test_audit_chain_contention_surface.py @@ -636,8 +636,8 @@ async def test_the_standard_surface_answers_the_same_named_outcome( """``/mcp`` and ``/mcp/messages`` agree, and the answer is actionable. The two transports classify in separate handlers, so a name added to one is - not a name on the other -- and ``/mcp`` is the surface new integrations are - told to use. It also pins the body: a caller that is refused a retry has to + not a name on the other -- and ``/mcp`` is the standards-compliant surface, + where an operator enables it. It also pins the body: a caller that is refused a retry has to be told what to do instead, which here is "do not mint a new key, reconcile out of band". A bare code would leave the correct action to guesswork, and the wrong guess is a second charged execution. diff --git a/tests/test_mcp_upstream_governed.py b/tests/test_mcp_upstream_governed.py index 18f9c171..3b12e2ea 100644 --- a/tests/test_mcp_upstream_governed.py +++ b/tests/test_mcp_upstream_governed.py @@ -749,21 +749,27 @@ async def session_context(**_kwargs: Any) -> AsyncIterator[Any]: get_service_registry().unregister_local(tool_name) -def _adapter_over_real_sdk_server(server: Server, tool_name: str) -> UpstreamMcpAdapter: - """The real adapter over a real in-memory SDK session, for ``tool_name``.""" +def _adapter_over_real_sdk_server( + server: Server, tool_name: str, **settings: Any +) -> UpstreamMcpAdapter: + """The real adapter over a real in-memory SDK session, for ``tool_name``. + + ``settings`` overrides the adapter's configuration (for example a small + ``MCP_UPSTREAM_MAX_RESPONSE_BYTES``). The bearer token is + ``secret-partner-token``. + """ + values: dict[str, Any] = { + "ENVIRONMENT": "local", + "MCP_UPSTREAM_ENABLED": True, + "MCP_UPSTREAM_URL": "http://localhost:9000/mcp", + "MCP_UPSTREAM_TOOL_NAME": "partner.echo", + "MCP_UPSTREAM_PUBLIC_TOOL_ID": tool_name, + "MCP_UPSTREAM_BEARER_TOKEN": "secret-partner-token", + "MCP_UPSTREAM_CREDITS_PER_CALL": "2", + } + values.update(settings) adapter = UpstreamMcpAdapter( - UpstreamMcpConfiguration.from_settings( - Settings( - _env_file=None, - ENVIRONMENT="local", - MCP_UPSTREAM_ENABLED=True, - MCP_UPSTREAM_URL="http://localhost:9000/mcp", - MCP_UPSTREAM_TOOL_NAME="partner.echo", - MCP_UPSTREAM_PUBLIC_TOOL_ID=tool_name, - MCP_UPSTREAM_BEARER_TOKEN="secret-partner-token", - MCP_UPSTREAM_CREDITS_PER_CALL="2", - ) - ) + UpstreamMcpConfiguration.from_settings(Settings(_env_file=None, **values)) ) adapter._discovered_tool = DiscoveredUpstreamTool( name="partner.echo", diff --git a/tests/test_mcp_upstream_tool_poisoning.py b/tests/test_mcp_upstream_tool_poisoning.py index cec6ff49..2ec36709 100644 --- a/tests/test_mcp_upstream_tool_poisoning.py +++ b/tests/test_mcp_upstream_tool_poisoning.py @@ -21,8 +21,9 @@ * Tool *results* are relayed to the agent unlabeled; nothing here tests that. * "Discovery happens once at startup" is true by code reading (the only call site of ``register_configured_upstream_mcp`` under ``app/`` is the - ``app/main.py`` lifespan; ``scripts/failure_lab.py`` also calls it, to run a - harness), not by any assertion here. + ``app/main.py`` lifespan; ``failure_lab/gateway.py`` and + ``scripts/failure_lab.py`` also call it, to run harnesses), not by any + assertion here. * Most tests replace the adapter's MCP session with a fake, so they observe what the gateway's code does, not SDK wire traffic. The real SDK validates a result against the upstream's live schema after the call (mcp 1.30.0), so the tests diff --git a/tests/test_receipt_write_contention_surface.py b/tests/test_receipt_write_contention_surface.py index 64262c5c..ae865a6f 100644 --- a/tests/test_receipt_write_contention_surface.py +++ b/tests/test_receipt_write_contention_surface.py @@ -555,8 +555,8 @@ async def test_the_standard_surface_answers_the_same_named_outcome( The two transports do not share an exception ladder: ``/mcp/messages`` matches types itself and ``/mcp`` maps them into ``McpError``. A name on one - is not a name on the other, and ``/mcp`` is the surface new integrations are - pointed at. + is not a name on the other, and ``/mcp`` is the standards-compliant surface, + where an operator enables it. It also pins the body. Refusing a retry without saying what to do instead leaves the caller to guess, and the wrong guess -- a fresh idempotency key -- diff --git a/tests/test_standard_mcp_endpoint.py b/tests/test_standard_mcp_endpoint.py index f630e4db..a4396622 100644 --- a/tests/test_standard_mcp_endpoint.py +++ b/tests/test_standard_mcp_endpoint.py @@ -9,6 +9,7 @@ from __future__ import annotations import hashlib +from datetime import datetime, timedelta, timezone import mcp.types as mcp_types import pytest @@ -27,6 +28,7 @@ ) from app.services.mcp_generator import McpGenerator from app.services.service_registry import get_service_registry +from app.services.upstream_mcp import UpstreamMcpResponseRejectedError from tests.test_delivery_uncertain_replay import AmbiguousExecutor from tests.test_mcp_upstream_governed import _adapter_over_real_sdk_server from tests.test_trust_helpers import BOOTSTRAP_HEADERS, provision_agent_wallet @@ -590,40 +592,98 @@ async def test_tools_call_delivery_uncertain_without_key_warns_a_retry_is_a_new_ assert "same-key replay" not in detail assert "any new attempt is a new dispatch and a new charge" in detail assert executor.dispatch_count == 1 + # By design the signed receipt carries the forwarded key as the provider + # lookup handle, so a keyless call's generated key is readable there (it is + # not returned as a field of its own). + forwarded = result["receipt"]["constraints_evaluated"]["upstream_dispatch"][ + "forwarded_idempotency_key" + ] + assert forwarded.startswith("mcp-auto-") + + +def _unparseable_envelope() -> mcp_types.ServerResult: + # model_construct skips validation, so the SERVER serializes an envelope + # that only the CLIENT side fails to parse. + return mcp_types.ServerResult.model_construct( + root=mcp_types.CallToolResult.model_construct( + content="not-a-list-marker", isError=False + ) + ) + + +def _oversized_result() -> mcp_types.ServerResult: + return mcp_types.ServerResult( + mcp_types.CallToolResult( + content=[mcp_types.TextContent(type="text", text="oversized-marker" * 80)], + isError=False, + ) + ) + + +def _bearer_echo_result() -> mcp_types.ServerResult: + return mcp_types.ServerResult( + mcp_types.CallToolResult( + content=[ + mcp_types.TextContent( + type="text", text="echo-marker secret-partner-token" + ) + ], + isError=False, + ) + ) @pytest.mark.anyio -async def test_tools_call_unparseable_upstream_envelope_is_a_tool_result_the_model_can_read( - client, standard_mcp_enabled, clean_database +@pytest.mark.parametrize( + ("cause", "make_result", "settings", "leaked_marker"), + [ + pytest.param( + "unparseable", _unparseable_envelope, {}, "not-a-list-marker", id="envelope" + ), + pytest.param( + "oversized", + _oversized_result, + {"MCP_UPSTREAM_MAX_RESPONSE_BYTES": 256}, + "oversized-marker", + id="oversized", + ), + pytest.param( + "bearer-echo", _bearer_echo_result, {}, "secret-partner-token", id="bearer" + ), + ], +) +async def test_tools_call_rejected_upstream_response_is_a_tool_result_the_model_can_read( + client, + standard_mcp_enabled, + clean_database, + cause, + make_result, + settings, + leaked_marker, ): """``response_rejected`` is charged, like ``delivery_uncertain``, so it too must not reach the model as a bare ``-32006``, the code refunded and retry-safe failures use. A real SDK upstream answers with an envelope the - SDK cannot parse; the call has run, the charge stands, and the model is told - not to send it again. + SDK cannot parse, a result over the byte cap, or a result echoing the + bearer token; the call has run, the charge stands, and the model is told not + to send it again, without anything the upstream wrote. """ provisioned = await provision_agent_wallet(client) - tool_name = "standard-unparseable-envelope-upstream" + tool_name = f"standard-rejected-response-upstream-{cause}" served: list[str] = [] - server = Server("unparseable-envelope-upstream") + server = Server("rejected-response-upstream") async def call_tool(_req: mcp_types.CallToolRequest) -> mcp_types.ServerResult: served.append("tools/call") - # model_construct skips validation, so the SERVER serializes an envelope - # that only the CLIENT side fails to parse. - return mcp_types.ServerResult.model_construct( - root=mcp_types.CallToolResult.model_construct( - content="not-a-list", isError=False - ) - ) + return make_result() server.request_handlers[mcp_types.CallToolRequest] = call_tool get_service_registry().register_upstream( service_id=tool_name, - name="Standard Unparseable Envelope Upstream", - description="Upstream tool whose response envelope cannot be parsed", + name="Standard Rejected Response Upstream", + description="Upstream tool whose response the gateway rejects", category=ServiceCategory.AGENT_COMMS, - executor=_adapter_over_real_sdk_server(server, tool_name), + executor=_adapter_over_real_sdk_server(server, tool_name, **settings), input_schema={"type": "object", "properties": {"test": {"type": "string"}}}, output_schema=None, credits_per_unit=2.0, @@ -636,7 +696,7 @@ async def call_tool(_req: mcp_types.CallToolRequest) -> mcp_types.ServerResult: headers = { **provisioned["agent_headers"], **MCP_HEADERS, - "Idempotency-Key": "standard-unparseable-1", + "Idempotency-Key": f"standard-rejected-{cause}-1", } try: first = await client.post("/mcp", json=call, headers=headers) @@ -651,8 +711,8 @@ async def call_tool(_req: mcp_types.CallToolRequest) -> mcp_types.ServerResult: assert "The gateway will not resend it." in text assert "Do not call this tool again for the same action" in text assert "same Idempotency-Key returns this same result" in text - # Nothing the upstream sent is relayed, only the fixed explanation. - assert "not-a-list" not in text + # Nothing the upstream wrote is relayed, only the fixed explanation. + assert leaked_marker not in first.text receipt = result["receipt"] assert receipt["outcome"] == "response_rejected" @@ -685,6 +745,178 @@ async def call_tool(_req: mcp_types.CallToolRequest) -> mcp_types.ServerResult: get_service_registry().unregister_local(tool_name) +class _RejectedExecutor: + """Dispatches, then reports a response the gateway rejected.""" + + def __init__(self) -> None: + self.dispatch_count = 0 + + async def call_tool( + self, arguments, *, invocation_id, idempotency_key, before_dispatch + ): + await before_dispatch() + self.dispatch_count += 1 + raise UpstreamMcpResponseRejectedError("upstream_response_invalid") + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ("outcome", "make_executor", "expected_gateway_outcome"), + [ + pytest.param( + "delivery_uncertain", AmbiguousExecutor, "unknown", id="uncertain" + ), + pytest.param("response_rejected", _RejectedExecutor, "failed", id="rejected"), + ], +) +async def test_tools_call_marks_the_gateway_outcome_of_each_charged_unknown_outcome( + client, + standard_mcp_enabled, + clean_database, + monkeypatch, + outcome, + make_executor, + expected_gateway_outcome, +): + """The request record keeps what each outcome had on the JSON-RPC channel. + + An undelivered call is ``unknown``; a rejected response is ``failed`` (a + fault), as ``/mcp/messages`` and REST mark ``-32005`` and ``-32006``. + """ + marks: list[dict] = [] + real_mark = standard_mcp_router.mark_current_request + + def spy(**kwargs): + marks.append(kwargs) + return real_mark(**kwargs) + + monkeypatch.setattr(standard_mcp_router, "mark_current_request", spy) + provisioned = await provision_agent_wallet(client) + tool_name = f"standard-marked-outcome-{outcome}" + get_service_registry().register_upstream( + service_id=tool_name, + name="Standard Marked Outcome", + description="Upstream tool with a charged, effect-unknown outcome", + category=ServiceCategory.AGENT_COMMS, + executor=make_executor(), + input_schema={"type": "object", "properties": {"test": {"type": "string"}}}, + output_schema=None, + credits_per_unit=2.0, + upstream_tool_name=tool_name, + upstream_origin="https://test.example.com", + ) + try: + response = await client.post( + "/mcp", + json=_rpc( + "tools/call", + params={"name": tool_name, "arguments": {"test": "value"}}, + ), + headers={ + **provisioned["agent_headers"], + **MCP_HEADERS, + "Idempotency-Key": f"standard-marked-{outcome}-1", + }, + ) + finally: + get_service_registry().unregister_local(tool_name) + + result = response.json()["result"] + assert result["receipt"]["outcome"] == outcome + gateway_marks = [m for m in marks if "gateway_outcome" in m] + assert gateway_marks[-1] == { + "gateway_outcome": expected_gateway_outcome, + "reason_code": outcome, + } + + +@pytest.mark.anyio +async def test_tools_call_honours_a_named_permit_but_mints_an_unconstrained_one_otherwise( + client, standard_mcp_enabled, clean_database +): + """The gap on ``/mcp`` is an omitted permit reference, not the endpoint. + + A caller who names a permit (``_meta["io.agentmiddleware/permit_id"]``) gets + the operator's constraints, here ``forbidden_fields``, with a signed + ``denied`` receipt. A caller who omits the reference, on a tool that is not + action-bound, gets a permit the gateway mints itself, which carries none of + them, so the same arguments run. + """ + provisioned = await provision_agent_wallet(client) + wallet_id = provisioned["agent_wallet_id"] + tool_name = "standard-named-permit-tool" + ran: list[str] = [] + + def tool(secret: str = "") -> dict: + ran.append(secret) + return {"ok": True} + + registry = get_service_registry() + registry.register_local( + service_id=tool_name, + name="Standard Named Permit Tool", + description="Standard MCP named-permit contract test", + category=ServiceCategory.AGENT_COMMS, + func=tool, + credits_per_unit=2.0, + unit_name="call", + ) + try: + created = await client.post( + "/v1/permits", + json={ + "issuer_wallet_id": wallet_id, + "subject_wallet_id": wallet_id, + "subject_key_id": provisioned["key_id"], + "allowed_tools": [tool_name], + "scopes": [f"tool:{tool_name}:invoke", "billing:charge"], + "max_credits": 50, + "forbidden_fields": ["secret"], + "expires_at": ( + datetime.now(timezone.utc) + timedelta(minutes=30) + ).isoformat(), + }, + headers={**BOOTSTRAP_HEADERS, "Idempotency-Key": "standard-named-permit"}, + ) + assert created.status_code == 201 + permit_id = created.json()["permit_id"] + headers = {**provisioned["agent_headers"], **MCP_HEADERS} + + named = await client.post( + "/mcp", + json=_rpc( + "tools/call", + params={ + "name": tool_name, + "arguments": {"secret": "x"}, + "_meta": {"io.agentmiddleware/permit_id": permit_id}, + }, + ), + headers={**headers, "Idempotency-Key": "standard-named-1"}, + ) + error = named.json()["error"] + assert error["code"] == -32003 + assert error["message"].startswith("permit_forbidden_field") + assert error["data"]["receipt"]["outcome"] == "denied" + assert error["data"]["receipt"]["permit_id"] == permit_id + assert ran == [] + + omitted = await client.post( + "/mcp", + json=_rpc( + "tools/call", + params={"name": tool_name, "arguments": {"secret": "x"}}, + ), + headers={**headers, "Idempotency-Key": "standard-omitted-1"}, + ) + result = omitted.json()["result"] + assert result["isError"] is False + assert result["receipt"]["permit_id"] != permit_id + assert ran == ["x"] + finally: + registry.unregister_local(tool_name) + + @pytest.mark.anyio async def test_tools_call_other_governed_errors_stay_jsonrpc_errors( client, diff --git a/tests/test_upstream_mcp.py b/tests/test_upstream_mcp.py index 631c614c..3f84de88 100644 --- a/tests/test_upstream_mcp.py +++ b/tests/test_upstream_mcp.py @@ -24,6 +24,7 @@ from mcp.shared.memory import create_connected_server_and_client_session from mcp.types import ( CallToolRequest, + CallToolRequestParams, CallToolResult, ListToolsResult, ServerResult, @@ -95,6 +96,15 @@ def _unparseable_result_error() -> ValidationError: raise AssertionError("invalid CallToolResult unexpectedly passed validation") +def _request_validation_error() -> ValidationError: + """The error the SDK raises while building a request, before it is sent.""" + try: + CallToolRequestParams(name="partner.write", arguments={1: "x"}) # type: ignore[dict-item] + except ValidationError as exc: + return exc + raise AssertionError("invalid request params unexpectedly passed validation") + + class FakeSession: def __init__( self, @@ -1875,6 +1885,57 @@ async def before_dispatch() -> None: assert served == [] +@pytest.mark.anyio +@pytest.mark.parametrize( + ("call_error", "expected"), + [ + pytest.param( + _unparseable_result_error(), + UpstreamMcpResponseRejectedError, + id="result-model-validation-error", + ), + pytest.param( + json.JSONDecodeError("not json", "{", 1), + UpstreamMcpResponseRejectedError, + id="json-decode-error", + ), + pytest.param( + _request_validation_error(), + UpstreamMcpDeliveryUncertainError, + id="request-model-validation-error", + ), + pytest.param( + _cleanup_validation_error(), + UpstreamMcpDeliveryUncertainError, + id="validation-error-for-another-model", + ), + ], +) +async def test_a_session_that_does_not_wrap_errors_is_classified_by_the_same_rule( + call_error: Exception, expected: type[Exception] +) -> None: + """The handlers must not depend on the SDK's task group wrapping the error. + + ``FakeSession`` raises the bare exception, as a session without the anyio + task group would. After dispatch, only a failure to parse a response (a + ``CallToolResult`` validation error or a JSON decode error) is + ``response_rejected``. Any other validation error, such as the one building + a request that was never sent, says nothing about a response and stays + ``delivery_uncertain``. + """ + adapter = UpstreamMcpAdapter(_configuration()) + _mark_discovered(adapter) + _bind_session(adapter, FakeSession(call_error=call_error)) + claims: list[str] = [] + + with pytest.raises(expected) as exc_info: + await _call_recording_claims(adapter, claims) + + assert type(exc_info.value) is expected + assert exc_info.value.dispatch_started is True + assert claims == ["claimed"] + + def _scripted_mcp_client( *, on_call: Any = None, From 93aee1ab1a5525c4f218cc13c7b64461952436c3 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:24:53 -0700 Subject: [PATCH 5/6] docs(mcp): correct the idempotency-key and delete claims; say /mcp/messages is the live path Third review of 67d56b31. Text and served text only; no behavior change. Refuted claims. The posture page and CHANGELOG said an action-bound call without an idempotency key is refused with idempotency_key_required on /mcp/messages and REST. It is not: mcp.py replaces the key with a server-derived act1- key (action_permits.py) before that check, and a call that names no permit is refused earlier as permit_required. The refusal is for a call that names a permit and sends no key. exit-and-export.md and the CHANGELOG said the API-key DELETE route is the one that deletes something; it only revokes (status, revoked_at, revoke_reason) and keeps the row, so in the default configuration no mounted DELETE route deletes anything. Served text. The /mcp/messages OpenAPI description called it a legacy endpoint "kept for existing clients and the local proof scripts" and named only /mcp, which ships off, as an alternative. It now says it is the live governed path in the supported pilot and why it is still marked deprecated (a project transport that does not implement the standard MCP initialization lifecycle, to be replaced by /mcp once an operator enables it). The REST invoke description and site/docs-src/http-api.md get the same fix. The description keeps its line count so the cited mcp.py lines do not move. docs/openapi.json is regenerated under requirements.lock and differs by those two descriptions. The machine-read discovery labels (legacy_project_transport, legacy_project_json_rpc) are deliberately unchanged. Smaller fixes. The _call_tool_once docstring now says only a CallToolResult parse failure is rejected, while a body that is not a JSON-RPC message is logged by the SDK transport, never raised, and ends delivery_uncertain at the timeout. On REST the relayed upstream text is detail.upstream_result in the 502 body. The CHANGELOG lists the other causes of response_rejected, which /mcp matches on by reason. Two comments that contradicted corrected claims are fixed (mcpContext.permit_id is read; the keyless key is readable in the receipt). failure-semantics cites a test that exercises /mcp/messages and says the REST status is untested. The Cursor plugin READMEs name REST invoke as a live path and carry the ships-off caveat; mcp.json is untouched. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 28 +++++++++--------- app/routers/mcp.py | 29 ++++++++++--------- app/routers/mcp_standard.py | 8 ++--- app/services/upstream_mcp.py | 11 ++++--- docs/exit-and-export.md | 10 ++++--- docs/failure-semantics.md | 8 +++-- docs/mcp-security-posture.md | 6 ++-- docs/openapi.json | 4 +-- .../cursor-plugin-agent-middleware/README.md | 2 +- .../plugins/agent-middleware/README.md | 2 +- site/docs-src/http-api.md | 2 +- tests/test_standard_mcp_endpoint.py | 4 +-- 12 files changed, 63 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f9e5cd1..0f976c4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,10 +90,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the code that refunded, retry-safe failures (`upstream_returned_error`, `upstream_pre_dispatch_failed`) also use, so a model shown only the message could resend a call whose charge stands and whose tool probably ran. This - covers every cause of `response_rejected`: an unparseable envelope, an - oversized result, and a result echoing the bearer token. Those refunded - failures stay JSON-RPC errors. `/mcp/messages` and REST are unchanged. The helper is now `_unknown_outcome_tool_result` - (was `_delivery_uncertain_tool_result`). + applies to any `response_rejected` outcome, because `mcp_standard.py` matches + on the reason, not the cause: for example an unparseable envelope, an + oversized result, a result echoing the bearer token, a non-identity + `Content-Encoding` or malformed `Content-Length` after dispatch, a result that + cannot be canonicalized, and a result the persistence layer rejects. Those + refunded failures stay JSON-RPC errors. `/mcp/messages` and REST are + unchanged. The helper is now `_unknown_outcome_tool_result` (was + `_delivery_uncertain_tool_result`). - **Behavior change:** a malformed envelope is no longer counted toward the optional circuit breaker. The `response_rejected` path never fed it (an oversized or bearer-echoing result did not either); `delivery_uncertain` does. @@ -159,15 +163,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 permit only when the reference is omitted (now pinned by a test). It said there is no KMS and no row-level security; an AWS KMS signing backend (`TRUST_SIGNING_BACKEND=aws_kms`) and PostgreSQL row-level security - (`TENANT_RLS_MODE=enforce`) exist as opt-ins, default off. The idempotency-key - refusal also covers action-bound calls and tools that require a permit. The - posture page now notes that the refunded `upstream_returned_error` relays the - upstream's result payload in `error.data.upstream_result` on all three - surfaces, and that a keyless `/mcp` call's generated key is readable in the - signed receipt as the forwarded key. `docs/exit-and-export.md` no longer says - traces are not produced, and says the only mounted `DELETE` route that deletes - anything is for API keys (the `/mcp` and `/mcp/public` `DELETE` handlers are - stubs). `docs/gateway-overhead.md` quotes only the first-call p99 and max, and + (`TENANT_RLS_MODE=enforce`) exist as opt-ins, default off. The posture page now + notes that the refunded `upstream_returned_error` relays the upstream's result + payload (`error.data.upstream_result` on `/mcp/messages` and `/mcp`, + `detail.upstream_result` in the HTTP 502 body on REST), and that a keyless + `/mcp` call's generated key is readable in the signed receipt as the forwarded + key. `docs/exit-and-export.md` no longer says traces are not produced. + `docs/gateway-overhead.md` quotes only the first-call p99 and max, and records the kernel release rather than a macOS version the result files do not contain. A comment in `mcp_standard.py` that said a keyless call's key is "never returned" is corrected. diff --git a/app/routers/mcp.py b/app/routers/mcp.py index 9641b097..a0709ce0 100644 --- a/app/routers/mcp.py +++ b/app/routers/mcp.py @@ -556,14 +556,14 @@ async def get_tools_json( name="MCP JSON-RPC Messages", deprecated=True, description=( - "Legacy project JSON-RPC endpoint, kept for existing clients and the " - "local proof scripts. It does not implement the standard MCP " - "initialization lifecycle. Check `/.well-known/agent.json` before " - "assuming the standard MCP Streamable HTTP endpoint at POST /mcp is " - "available; both transports run the same governed permit→meter→receipt " - "path when enabled. tools/call uses params.mcpContext (mcp_context is " - "accepted as an alias on every route). Preserve one " - "Idempotency-Key across retries." + "The live governed JSON-RPC path in the supported pilot. It is marked " + "deprecated because it is a project transport that does not implement " + "the standard MCP initialization lifecycle; the standard Streamable " + "HTTP endpoint at POST /mcp is meant to replace it once an operator " + "enables that endpoint (it ships off; check `/.well-known/agent.json`)." + " Both run the same governed permit→meter→receipt path when enabled. " + "tools/call uses params.mcpContext (mcp_context is accepted as an " + "alias on every route). Preserve one Idempotency-Key across retries." ), openapi_extra={ "requestBody": { @@ -5145,12 +5145,13 @@ async def _handle_tools_call( summary="Invoke a registered MCP tool", deprecated=True, description=( - "Legacy REST-shaped invoke, kept for existing clients. Check " - "`/.well-known/agent.json` before assuming the standard MCP " - "Streamable HTTP endpoint at POST /mcp is available; it ships off. " - "Where it is enabled, new integrations can call tools through it; " - "otherwise use POST /mcp/messages or this route. Every enabled entry " - "point runs the same governed permit→meter→receipt path." + "The live governed REST-shaped invoke in the supported pilot, next to " + "POST /mcp/messages. It is marked deprecated because it is not the " + "standard MCP transport; the standard Streamable HTTP endpoint at " + "POST /mcp is meant to replace both once an operator enables it. " + "Check `/.well-known/agent.json` before assuming that endpoint is " + "available; it ships off. Every enabled entry point runs the same " + "governed permit→meter→receipt path." ), ) async def invoke_tool( diff --git a/app/routers/mcp_standard.py b/app/routers/mcp_standard.py index fc030093..ab9a92e5 100644 --- a/app/routers/mcp_standard.py +++ b/app/routers/mcp_standard.py @@ -287,10 +287,10 @@ def _client_idempotency_key( if meta_source is not None: sources.append(meta_source) # A client written against the legacy transport may send its key in - # params.mcpContext. This endpoint mints its own permit and ignores - # the rest of that object, but a key the caller explicitly sent must - # not be silently dropped: it is one more source, validated and - # conflict-checked like the others. JSON null reads as absent, as + # params.mcpContext. Apart from permit_id (_action_permit_reference) + # this endpoint ignores that object, but a key the caller explicitly + # sent must not be silently dropped: it is one more source, validated + # and conflict-checked like the others. JSON null reads as absent, as # everywhere else. legacy_context = _legacy_context(params) if isinstance(legacy_context, dict): diff --git a/app/services/upstream_mcp.py b/app/services/upstream_mcp.py index e2a25606..05017f80 100644 --- a/app/services/upstream_mcp.py +++ b/app/services/upstream_mcp.py @@ -1038,10 +1038,13 @@ async def _call_tool_once( classified as delivery-uncertain. A well-formed result that was received is not: SDK-side output-schema validation is disabled for the call (see ``_bypass_sdk_result_validation``), so only this adapter's own shape, - size, and secret checks can reject it. A response the SDK itself cannot - parse is rejected too, not uncertain: the upstream did answer. That - failure is recorded at the ``call_tool`` await because the SDK's task - group rewraps anything that leaves the session block. + size, and secret checks can reject it. A JSON-RPC result whose envelope the + SDK cannot parse into a CallToolResult is rejected too, not uncertain: + the upstream did answer. That failure is recorded at the ``call_tool`` + await because the SDK's task group rewraps anything that leaves the + session block. A body that is not a JSON-RPC message at all is parsed + by the SDK transport instead, which logs the error and never raises it, + so that call ends delivery-uncertain at the timeout. """ if self._discovered_tool is None: raise UpstreamMcpPreDispatchError("upstream_tool_not_initialized") diff --git a/docs/exit-and-export.md b/docs/exit-and-export.md index acebf040..82e96fb9 100644 --- a/docs/exit-and-export.md +++ b/docs/exit-and-export.md @@ -91,10 +91,12 @@ a database copy and a signing key (§4, step 7). for keys needs out-of-band pinning, frozen until a named partner requires it. - **No deletion controls.** `docs/deploy-railway.md` lists retention, deletion, and legal-hold controls among things not yet built. No - route deletes receipts, ledger rows, audit events, or stored results; the - only mounted `DELETE` route that deletes anything is for API keys (`DELETE - /mcp` and `DELETE /mcp/public` are stubs that refuse and delete nothing). - Removal means the operator + route deletes receipts, ledger rows, audit events, or stored results; in + the default configuration no mounted `DELETE` route deletes anything. The one + mounted `DELETE` route, for API keys, only revokes (it sets `status`, + `revoked_at` and `revoke_reason` and keeps the row), and `DELETE /mcp` and + `DELETE /mcp/public` are stubs that refuse. Proof surfaces, which production + refuses at boot, mount further `DELETE` routes. Removal means the operator deleting the customer's deployment project. No timeline is documented (not verified), and provider backups are not addressed. - No documented procedure, timeline, or commitment for handing over a diff --git a/docs/failure-semantics.md b/docs/failure-semantics.md index e7f2b7ed..4bd65a0f 100644 --- a/docs/failure-semantics.md +++ b/docs/failure-semantics.md @@ -109,8 +109,12 @@ never probed or refunded. And on `POST /mcp`, where enabled, both arrive as a `_meta["io.agentmiddleware/outcome"].status` `"unknown"`, not as a JSON-RPC error: `-32006` is also the code of refunded, retry-safe failures (`upstream_returned_error`, `upstream_pre_dispatch_failed`), which stay errors. -`/mcp/messages` and REST keep `-32005` and `-32006` -(`test_tools_call_rejected_upstream_response_is_a_tool_result_the_model_can_read`). +`/mcp/messages` keeps `-32005` and `-32006` +(`test_ambiguous_or_rejected_upstream_response_stays_charged_and_replays`), and +REST keeps HTTP 504 and 502 (by code reading, `invoke_tool` raises +`HTTPException(status_code=exc.status_code)`; no test asserts the REST status). +The `/mcp` shape is pinned by +`test_tools_call_rejected_upstream_response_is_a_tool_result_the_model_can_read`. Requests rejected before a valid permit and an executable tool are established produce no receipt — there is no authority to bill against. The README states diff --git a/docs/mcp-security-posture.md b/docs/mcp-security-posture.md index e0be0400..2fa93d10 100644 --- a/docs/mcp-security-posture.md +++ b/docs/mcp-security-posture.md @@ -77,7 +77,7 @@ defending against them, and it leaves gaps that §3 states plainly. | **Confused deputy** | Enforced at the agent boundary; textbook gap at the upstream boundary (§3.1) | Application-layer wallet checks run before the idempotency store is touched; a key cannot use another wallet's permit; a key-bound permit refuses another key. Row-level security is opt-in (`TENANT_RLS_MODE`, default `off`) and is not relied on here. | `scripts/invariant_attacks/attack6_key_misuse.py` (HELD, run against the quickstart's local tool); `tests/test_trust_negative_security.py` (verified by name) | | **Token passthrough** | Structurally absent | The upstream credential is operator configuration (`MCP_UPSTREAM_BEARER_TOKEN`), loaded at startup and never read from a request. The agent's `X-API-Key` is never forwarded. A discovery page or tool result that contains the bearer string **verbatim** is rejected; encoded or partial reflections are not detected, and a rejected result is a charged call (`response_rejected` keeps the debit). | `_contains_exact_secret`, `upstream_mcp.py`; `test_discovery_rejects_bearer_token_echo_without_exposing_it`; `test_call_rejects_bearer_token_echo_without_returning_or_logging_it` (verified by name) | | **Session hijack / event injection** | Structurally absent | `POST /mcp` is stateless: `mcp_session_id=None` (`mcp_standard.py:984`), `stateless=True` (`:1000`), authenticated per request by `Depends(get_auth_context)` (`:1020`); GET and DELETE return 405 (`standard_mcp_no_stream`, `:1034`). `/mcp/messages` has no SSE transport (module docstring, `mcp.py:15`). The upstream client opens a fresh session per call. | `mcp_standard.py` (verified); `tests/test_standard_mcp_endpoint.py` (documented) | -| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a governed call (permit-bound, action-bound, a tool that requires a permit, or any call in strict trust mode) without an idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation (for an upstream call the generated key is readable in the signed receipt, as `constraints_evaluated.upstream_dispatch.forwarded_idempotency_key`, by design, as the provider lookup handle); the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | +| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a call that names a permit but sends no idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`); a governed call that names no permit (any call in strict trust mode, or a tool that requires one) is refused earlier as `permit_required` (`mcp.py:1867-1888`); and an action-bound call is never refused for lack of a key, because the server derives an `act1-` key from the permit and payload (`mcp.py:1691`, `action_permits.py:467`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation (for an upstream call the generated key is readable in the signed receipt, as `constraints_evaluated.upstream_dispatch.forwarded_idempotency_key`, by design, as the provider lookup handle); the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | | **Excessive scope / permission creep** | Enforced on `/mcp/messages` and REST, and on `/mcp` when the caller names a permit; wallet-level on `/mcp` when it does not | Permits are Ed25519-signed capabilities naming the tool, scopes, spend cap, use count, expiry, optional key binding. Wallet policy bundles add standing caps on every path. | `app/services/permits.py`, `app/services/policies.py`; `make red-team-trust-plane` (documented) | | **Prompt injection, direct or indirect** | Contained, not prevented | The gateway does not see prompts. A hijacked agent holding a permit cannot act outside it, and an out-of-permit call against an existing permit produces a signed `denied` receipt (when the permit names the caller's wallet; a permit naming another wallet is not the caller's evidence and gets no receipt). Refusals that happen before permit lookup (bad key, no permit, no idempotency key, wallet access) are audit events or HTTP errors, not receipts. Tool results are bounded and canonicalized but their content is passed through unlabeled (§3.3). | ASI01 row in the OWASP mapping (documented); `mcp.py:2071-2072` (verified) | | **Local MCP server compromise** (client launches a process from configuration) | Not applicable | The gateway launches no process from configuration (§2). | search, §2 (verified) | @@ -407,8 +407,8 @@ exact upstream bearer string, but returns its content to the agent unmarked. Any other credential in a result passes; so does an encoded or split copy of the bearer. A tool-level error result is relayed too: the refunded `upstream_returned_error` carries the upstream's result payload, which is -upstream-authored text, in `error.data.upstream_result` on `/mcp/messages`, REST -and `/mcp`. Labeling results as untrusted for the agent's context assembly is +upstream-authored text, in `error.data.upstream_result` on `/mcp/messages` and +`/mcp`, and in `detail.upstream_result` of the HTTP 502 body on REST. Labeling results as untrusted for the agent's context assembly is a runtime concern above this boundary. ### 3.4 Custody and network diff --git a/docs/openapi.json b/docs/openapi.json index 0faab8cd..79062a25 100644 --- a/docs/openapi.json +++ b/docs/openapi.json @@ -3211,7 +3211,7 @@ "MCP" ], "summary": "Mcp Json-Rpc Messages", - "description": "Legacy project JSON-RPC endpoint, kept for existing clients and the local proof scripts. It does not implement the standard MCP initialization lifecycle. Check `/.well-known/agent.json` before assuming the standard MCP Streamable HTTP endpoint at POST /mcp is available; both transports run the same governed permit→meter→receipt path when enabled. tools/call uses params.mcpContext (mcp_context is accepted as an alias on every route). Preserve one Idempotency-Key across retries.", + "description": "The live governed JSON-RPC path in the supported pilot. It is marked deprecated because it is a project transport that does not implement the standard MCP initialization lifecycle; the standard Streamable HTTP endpoint at POST /mcp is meant to replace it once an operator enables that endpoint (it ships off; check `/.well-known/agent.json`). Both run the same governed permit→meter→receipt path when enabled. tools/call uses params.mcpContext (mcp_context is accepted as an alias on every route). Preserve one Idempotency-Key across retries.", "operationId": "MCP_JSON_RPC_Messages_mcp_messages_post", "deprecated": true, "security": [ @@ -3377,7 +3377,7 @@ "MCP" ], "summary": "Invoke a registered MCP tool", - "description": "Legacy REST-shaped invoke, kept for existing clients. Check `/.well-known/agent.json` before assuming the standard MCP Streamable HTTP endpoint at POST /mcp is available; it ships off. Where it is enabled, new integrations can call tools through it; otherwise use POST /mcp/messages or this route. Every enabled entry point runs the same governed permit→meter→receipt path.", + "description": "The live governed REST-shaped invoke in the supported pilot, next to POST /mcp/messages. It is marked deprecated because it is not the standard MCP transport; the standard Streamable HTTP endpoint at POST /mcp is meant to replace both once an operator enables it. Check `/.well-known/agent.json` before assuming that endpoint is available; it ships off. Every enabled entry point runs the same governed permit→meter→receipt path.", "operationId": "Invoke_MCP_Tool_mcp_tools__service_id__invoke_post", "deprecated": true, "security": [ diff --git a/integrations/cursor-plugin-agent-middleware/README.md b/integrations/cursor-plugin-agent-middleware/README.md index ec09907b..f4d081d0 100644 --- a/integrations/cursor-plugin-agent-middleware/README.md +++ b/integrations/cursor-plugin-agent-middleware/README.md @@ -6,7 +6,7 @@ Public-ready scaffold, not yet submitted to any marketplace. Layout mirrors the The `agent-middleware` plugin connects Cursor to the Agent Middleware API so that side-effecting tool calls follow one guardrail: permit before action, receipt after. -- `plugins/agent-middleware/mcp.json` points at the AMW API base `https://api.thisisatest.tech`, standard MCP endpoint `POST /mcp` (Streamable HTTP). That endpoint ships off (`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false) and the production runbook keeps it off, so the plugin works only against a deployment whose operator has enabled it; the live governed path otherwise is legacy JSON-RPC `POST /mcp/messages`, which the plugin does not use. The API key is the `${AMW_API_KEY}` environment placeholder. No real key is stored anywhere in this tree. +- `plugins/agent-middleware/mcp.json` points at the AMW API base `https://api.thisisatest.tech`, standard MCP endpoint `POST /mcp` (Streamable HTTP). That endpoint ships off (`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false) and the production runbook keeps it off, so the plugin works only against a deployment whose operator has enabled it; the live governed paths otherwise are JSON-RPC `POST /mcp/messages` and REST `POST /mcp/tools/{id}/invoke`, which the plugin does not use. The API key is the `${AMW_API_KEY}` environment placeholder. No real key is stored anywhere in this tree. - `plugins/agent-middleware/skills/amw-permit-receipt/SKILL.md` tells the agent to request a permit (`POST /v1/permits`) before any side-effecting tool call and to record the signed receipt (`GET /v1/receipts/{receipt_id}`) after the action. Note on transport: the endpoint path (`/mcp`) and the `X-API-Key` header shape were checked against this repo (standard MCP router, repo `.mcp.json`, quickstart docs). Cursor's exact remote-server key names may vary by version, so confirm the `mcp.json` fields against your Cursor version when installing. diff --git a/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/README.md b/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/README.md index 084571d0..3b248ce0 100644 --- a/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/README.md +++ b/integrations/cursor-plugin-agent-middleware/plugins/agent-middleware/README.md @@ -4,5 +4,5 @@ Permit before action, receipt after, for side-effecting tool calls made from Cur ## Included -- `mcp.json`: MCP server config pointing at the Agent Middleware API (`https://api.thisisatest.tech/mcp`), with the API key as the `${AMW_API_KEY}` environment placeholder. +- `mcp.json`: MCP server config pointing at the Agent Middleware API (`https://api.thisisatest.tech/mcp`), with the API key as the `${AMW_API_KEY}` environment placeholder. The standard `/mcp` endpoint ships off (`ENABLE_STANDARD_MCP_ENDPOINT` defaults to false) and the production runbook keeps it off, so this config works only against a deployment whose operator has enabled it. - `skills/amw-permit-receipt/`: the governance skill. Request an AMW permit before any side-effecting tool call, record the receipt after the action. diff --git a/site/docs-src/http-api.md b/site/docs-src/http-api.md index 1a4bc790..b6c61dc4 100644 --- a/site/docs-src/http-api.md +++ b/site/docs-src/http-api.md @@ -7,7 +7,7 @@ description: Find the live API contract and invoke a governed MCP tool. # HTTP API -Read `GET /openapi.json` on your gateway for its current HTTP contract. Use `GET /v1/discover/tools` to find a tool and its argument schema. Create a Permit with `POST /v1/permits`. The standard MCP path invokes at `POST /mcp` when enabled. The example below uses `POST /mcp/messages`, a deprecated JSON-RPC route retained for existing clients and local proof scripts. +Read `GET /openapi.json` on your gateway for its current HTTP contract. Use `GET /v1/discover/tools` to find a tool and its argument schema. Create a Permit with `POST /v1/permits`. The standard MCP path invokes at `POST /mcp` when an operator enables it; it ships off. The example below uses `POST /mcp/messages`, the live governed JSON-RPC route in the supported pilot. It is marked deprecated because it is not the standard MCP transport, which is meant to replace it once enabled. Send your wallet key in `X-API-Key`. Permit creation requires an `Idempotency-Key` header. A Call includes its Permit id, wallet id, and Request key in `mcpContext`; the same key can also be sent in the header. Replace the tool name and arguments below with values from discovery. diff --git a/tests/test_standard_mcp_endpoint.py b/tests/test_standard_mcp_endpoint.py index a4396622..deec75f4 100644 --- a/tests/test_standard_mcp_endpoint.py +++ b/tests/test_standard_mcp_endpoint.py @@ -585,8 +585,8 @@ async def test_tools_call_delivery_uncertain_without_key_warns_a_retry_is_a_new_ assert "same Idempotency-Key returns" not in text outcome = result["_meta"]["io.agentmiddleware/outcome"] assert outcome["status"] == "unknown" - # The generated key was never returned, so the structured remediation - # must not promise a replay the caller cannot make. + # The generated key is not returned as a field of its own (the receipt + # carries it as the forwarded key), so the remediation promises no replay. assert outcome["idempotency_key_supplied"] is False detail = outcome["remediation"]["detail"] assert "same-key replay" not in detail From 8c2b4428869385c83a760066194b11dfba0aae52 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:46:20 -0700 Subject: [PATCH 6/6] docs(mcp): scope the act1- key and idempotency_key_required claims precisely Fourth review of 93aee1ab. Text only; no .py file differs from that commit. The posture page said the server derives an act1- key "from the permit and payload". The digest (action_permits.py) covers only the action permit's identity: contract version, permit id, subject wallet, public tool id and upstream binding hash. The permit's action_payload_hash goes into the request payload, not the key. It now says so. The same row said a governed call that names no permit is refused earlier as permit_required. Two checks answer first: an action-bound tool called without an action permit is action_permit_required (mcp.py:1672-1676), and an unknown tool is "Tool not found" (mcp.py:1740-1750). Both are named, with the lines. An older CHANGELOG entry and failure-semantics.md still said idempotency_key_required applies to every governed call, including any call in strict trust mode. Both now scope it to a call that names a permit and sends no key, matching the posture row. exit-and-export.md called the API-key route "the one mounted DELETE route" in a sentence that also names two DELETE stubs; it now says the API-key route only revokes and the stubs refuse, so no default mounted DELETE route deletes anything. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 7 +++++-- docs/exit-and-export.md | 8 ++++---- docs/failure-semantics.md | 6 ++++-- docs/mcp-security-posture.md | 2 +- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f976c4d..90607f6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,8 +141,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 commits older. Of 17, 15 had moved (by 1 to about 970 lines), one SDK range was a line short, and one (`mcp.py:15`) was still right. Each is re-pinned and the claim beside it re-read. Claims that no longer held, corrected: the `idempotency_key_required` - refusal applies to every governed call, including any call in strict trust - mode, not only permit-bound ones; a signed `denied` receipt is issued only when + refusal applies to a call that names a permit and sends no key (a governed + call that names no permit is refused sooner, as `permit_required`, + `action_permit_required` or `Tool not found`, and an action-bound call gets a + server-derived `act1-` key and is never refused for lack of one); a signed + `denied` receipt is issued only when the permit names the caller's wallet; the SDK validation range is `mcp/client/session.py:412-448`; `docs/exit-and-export.md` said no OpenTelemetry instrumentation exists, but optional tracing diff --git a/docs/exit-and-export.md b/docs/exit-and-export.md index 82e96fb9..21d93fca 100644 --- a/docs/exit-and-export.md +++ b/docs/exit-and-export.md @@ -92,10 +92,10 @@ a database copy and a signing key (§4, step 7). - **No deletion controls.** `docs/deploy-railway.md` lists retention, deletion, and legal-hold controls among things not yet built. No route deletes receipts, ledger rows, audit events, or stored results; in - the default configuration no mounted `DELETE` route deletes anything. The one - mounted `DELETE` route, for API keys, only revokes (it sets `status`, - `revoked_at` and `revoke_reason` and keeps the row), and `DELETE /mcp` and - `DELETE /mcp/public` are stubs that refuse. Proof surfaces, which production + the default configuration no mounted `DELETE` route deletes anything. The + API-key route only revokes (it sets `status`, `revoked_at` and `revoke_reason` + and keeps the row), and `DELETE /mcp` and `DELETE /mcp/public` are stubs that + refuse. Proof surfaces, which production refuses at boot, mount further `DELETE` routes. Removal means the operator deleting the customer's deployment project. No timeline is documented (not verified), and provider backups are not addressed. diff --git a/docs/failure-semantics.md b/docs/failure-semantics.md index 4bd65a0f..ae66e98f 100644 --- a/docs/failure-semantics.md +++ b/docs/failure-semantics.md @@ -319,8 +319,10 @@ Only a genuinely **absent** key — no header, no body entry, or a JSON `null` entry — is treated as an un-keyed call. On the standard MCP endpoint that means a generated key, and every un-keyed call, retries included, is a new charged operation. Clients that retry must always send their own persistent key; a -governed `/mcp/messages` call without one is refused with -`idempotency_key_required`. +`/mcp/messages` call that names a permit but sends none is refused with +`idempotency_key_required` (an action-bound call is given a server-derived key +instead, and a call that names no permit is refused sooner, as +`permit_required`). The governed AWI HTTP routes — `POST /v1/awi/execute`, `/v1/awi/passkey/challenge`, `/v1/awi/passkey/verify`, `/v1/awi/dom/sync`, diff --git a/docs/mcp-security-posture.md b/docs/mcp-security-posture.md index 2fa93d10..9ad766be 100644 --- a/docs/mcp-security-posture.md +++ b/docs/mcp-security-posture.md @@ -77,7 +77,7 @@ defending against them, and it leaves gaps that §3 states plainly. | **Confused deputy** | Enforced at the agent boundary; textbook gap at the upstream boundary (§3.1) | Application-layer wallet checks run before the idempotency store is touched; a key cannot use another wallet's permit; a key-bound permit refuses another key. Row-level security is opt-in (`TENANT_RLS_MODE`, default `off`) and is not relied on here. | `scripts/invariant_attacks/attack6_key_misuse.py` (HELD, run against the quickstart's local tool); `tests/test_trust_negative_security.py` (verified by name) | | **Token passthrough** | Structurally absent | The upstream credential is operator configuration (`MCP_UPSTREAM_BEARER_TOKEN`), loaded at startup and never read from a request. The agent's `X-API-Key` is never forwarded. A discovery page or tool result that contains the bearer string **verbatim** is rejected; encoded or partial reflections are not detected, and a rejected result is a charged call (`response_rejected` keeps the debit). | `_contains_exact_secret`, `upstream_mcp.py`; `test_discovery_rejects_bearer_token_echo_without_exposing_it`; `test_call_rejects_bearer_token_echo_without_returning_or_logging_it` (verified by name) | | **Session hijack / event injection** | Structurally absent | `POST /mcp` is stateless: `mcp_session_id=None` (`mcp_standard.py:984`), `stateless=True` (`:1000`), authenticated per request by `Depends(get_auth_context)` (`:1020`); GET and DELETE return 405 (`standard_mcp_no_stream`, `:1034`). `/mcp/messages` has no SSE transport (module docstring, `mcp.py:15`). The upstream client opens a fresh session per call. | `mcp_standard.py` (verified); `tests/test_standard_mcp_endpoint.py` (documented) | -| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a call that names a permit but sends no idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`); a governed call that names no permit (any call in strict trust mode, or a tool that requires one) is refused earlier as `permit_required` (`mcp.py:1867-1888`); and an action-bound call is never refused for lack of a key, because the server derives an `act1-` key from the permit and payload (`mcp.py:1691`, `action_permits.py:467`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation (for an upstream call the generated key is readable in the signed receipt, as `constraints_evaluated.upstream_dispatch.forwarded_idempotency_key`, by design, as the provider lookup handle); the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | +| **Replay / duplicate side effect** | Enforced for keyed calls; **not** for un-keyed calls on `POST /mcp` | On `/mcp/messages` and REST invoke, a call that names a permit but sends no idempotency key is refused (`idempotency_key_required`, `mcp.py:1889-1902`); a governed call that names no permit (any call in strict trust mode, or a tool that requires one) is refused before that check as `permit_required` (`mcp.py:1867-1888`), or sooner still as `action_permit_required` (an action-bound tool called without an action permit, `mcp.py:1672-1676`) or `Tool not found` (an unknown tool, `mcp.py:1740-1750`); and an action-bound call that does name its permit is never refused for lack of a key, because the server derives an `act1-` key from the action permit's identity (permit id, wallet, tool and upstream binding), not from the call's payload (`mcp.py:1691`, `action_permits.py:467`). On `POST /mcp` an absent key is replaced by a server-generated `mcp-auto-` (`mcp_standard.py:566`) and the call runs, so an un-keyed retry is a new charged operation (for an upstream call the generated key is readable in the signed receipt, as `constraints_evaluated.upstream_dispatch.forwarded_idempotency_key`, by design, as the provider lookup handle); the key is mandatory there only when wallet policy requires human approval. With a client key: same key replays the stored receipt, a changed payload fails closed, at most one **gateway** dispatch and debit. The upstream's effect is once only if it honors the forwarded `io.agentmiddleware/idempotency_key`. A timeout after the dispatch claim is signed `delivery_uncertain`, keeps the debit, and is never redispatched. | [`failure-semantics.md`](failure-semantics.md); `tests/test_adversarial_five_claims.py` (documented) | | **Excessive scope / permission creep** | Enforced on `/mcp/messages` and REST, and on `/mcp` when the caller names a permit; wallet-level on `/mcp` when it does not | Permits are Ed25519-signed capabilities naming the tool, scopes, spend cap, use count, expiry, optional key binding. Wallet policy bundles add standing caps on every path. | `app/services/permits.py`, `app/services/policies.py`; `make red-team-trust-plane` (documented) | | **Prompt injection, direct or indirect** | Contained, not prevented | The gateway does not see prompts. A hijacked agent holding a permit cannot act outside it, and an out-of-permit call against an existing permit produces a signed `denied` receipt (when the permit names the caller's wallet; a permit naming another wallet is not the caller's evidence and gets no receipt). Refusals that happen before permit lookup (bad key, no permit, no idempotency key, wallet access) are audit events or HTTP errors, not receipts. Tool results are bounded and canonicalized but their content is passed through unlabeled (§3.3). | ASI01 row in the OWASP mapping (documented); `mcp.py:2071-2072` (verified) | | **Local MCP server compromise** (client launches a process from configuration) | Not applicable | The gateway launches no process from configuration (§2). | search, §2 (verified) |