Skip to content

Post-execution exceptions leave a completed tool call with no audit outcome #571

Description

@Yatsuiii

Summary

CMCPProxy.call_tool writes a terminal audit entry for an unexpected exception raised before the upstream tool call, and writes nothing for one raised after it. Since the tool has already run by then, an upstream can execute a tool call and leave no record of it in the audit chain.

The invariant is not missing from this codebase. It is implemented on one side of the upstream request only.

POLICY-003 (tests/unit/test_mcp_proxy.py:556) asserts that a Cedar backend exception appends a fault entry before re-raising. That is the pre-execution case, and it works. There is no equivalent boundary on the response side, and everything between _forward_to_upstream returning and the audit append at proxy.py:1221 is fallible.

Verified end to end against main at a2893da. The reproduction below exits 0 on confirmation and includes POLICY-003 as a live control in the same run.

Reproduction result

--- A  nested text inside a valid envelope
    crash path:
        cmcp_runtime/mcp/proxy.py:1209  call_tool()
        cmcp_runtime/mcp/proxy.py:139  _extract_external_execution_evidence()
    upstream invoked    : ['test.echo']
    outcome             : RecursionError propagated out of call_tool
    audit entries       : 1 -> 1  ['session_start']
    chain tip changed   : False

--- B  nested envelope, before the size guard runs
    crash path:
        cmcp_runtime/mcp/proxy.py:975  call_tool()
        cmcp_runtime/mcp/proxy.py:525  _forward_to_upstream()
        cmcp_runtime/mcp/streamable_http.py:113  parse_response()
    upstream invoked    : ['test.echo']
    outcome             : RecursionError propagated out of call_tool
    audit entries       : 1 -> 1  ['session_start']
    chain tip changed   : False

--- C  control: pre-execution exception (POLICY-003)
    crash path:
        cmcp_runtime/mcp/proxy.py:861  call_tool()
    upstream invoked    : []
    outcome             : RuntimeError propagated out of call_tool
    audit entries       : 1 -> 2  ['session_start', 'fault']
    chain tip changed   : True

Same class of unexpected exception. One side of the upstream request records it, the other does not.

The concrete trigger

json.loads raises RecursionError on deeply nested input. RecursionError subclasses RuntimeError, not ValueError, so it is caught by neither of the two guards standing between the upstream call and the audit write:

  • proxy.py:140 catches json.JSONDecodeError only.
  • proxy.py:538 catches ValueError and maps it to UpstreamUnavailable.

There are two independent paths, which is the part that matters for how this gets fixed.

Path A, nested text inside a valid envelope. The envelope is small, shallow, and entirely valid, so parse_response handles it without complaint. The hostile shape is the result.content[].text string, which _extract_external_execution_evidence parses again at proxy.py:139. The crash lands at the call site on proxy.py:1209, twelve lines above the audit append on proxy.py:1221.

Path B, nested envelope. No inner-text trick needed. parse_response calls response.json() at streamable_http.py:113 and dies there, inside _forward_to_upstream. This one is worth separating out because it fires before the max_response_size_bytes guard at proxy.py:1016 has run at all, so that guard cannot bound it at any configured value.

Patching either guard on its own leaves the other. That is what makes this look like an ordering problem rather than a missing exception type.

Cost to trigger

Bisected on the harness below, the decoder gives out between depth 9,987 and 10,218, so roughly 60KB, which is under 3% of the 2MB response size guard. The exact threshold depends on how much C stack remains at the point of the parse, so it will differ in a real deployment. It is not a large or expensive payload in any case.

Why this is an audit problem and not only a 500

MCPServer._handle_tool_call wraps call_tool in except Exception and returns a 500 TEE_FAULT, so the process survives. What does not survive is the record.

docs/spec/threat-model.md is specific about why that matters:

  • Line 31 enumerates A3, malicious or compromised MCP server, whose stated capability is "Controls the tool's response payload" and whose stated limit is "Cannot modify runtime policy or audit entries."
  • Line 66 names the mitigation for the corresponding repudiation threat: "Tool server denies a call was made", mitigated by "Audit entry records call, tool server identity, and response hash."

A3 cannot rewrite an audit entry. It can cause one not to exist, using exactly the capability A3 is stipulated to have. The response payload is the input that decides whether the call gets recorded.

I want to be careful about the size of that claim. This is not remote code execution and it is not policy bypass. Ingress policy still ran and still decided. What is lost is the evidence that the call happened, on a gateway whose stated purpose is producing that evidence.

A buggy upstream reaches the same end state without any adversary at all.

Suggested fix

Two parts, and I think only the second one actually closes it.

1. The narrow patch. Widen the two guards. At proxy.py:140, except (json.JSONDecodeError, RecursionError) matches how that helper already treats unparseable optional evidence. At proxy.py:538, adding RecursionError alongside ValueError maps it to the existing UpstreamUnavailable path.

That stops today's reproduction. It does not establish the invariant, and the next exception class that shows up between the upstream call and the audit write reopens the same hole.

2. The finalization boundary. One exception boundary immediately after _forward_to_upstream returns successfully. If any later stage raises before a terminal outcome has been recorded, append exactly one fault entry carrying call_id, tool name, request hash, the failed stage, and the exception type, then re-raise so the server keeps its current 500 behaviour.

One boundary rather than a try around each stage. POLICY-003 already established both the entry type and the append-then-re-raise shape, so this would be applying an existing decision to the other side of the request rather than introducing a new concept.

The stages currently exposed between the upstream response and the audit append, for sizing:

Location Stage
streamable_http.py:113 envelope parse
proxy.py:561 result join and serialize
proxy.py:1016 response size check
proxy.py:1054 AGT response interception
proxy.py:1130 session mutation
proxy.py:1139 response encode
proxy.py:1142 egress authorization, catches PolicyDeny only
proxy.py:1209 execution evidence extraction

I have only proven streamable_http.py:113 and proxy.py:1209 reachable. The rest are listed as the surface the boundary would cover, not as claimed bugs.

3. Bounding the input is worth doing too, but separately. docs/spec/proxy-security.md specs MAX_JSON_NESTING_DEPTH = 64 and its fuzz target 4 is "Tool response processor. Input: arbitrary JSON as tool response. Output: processed response or error. Must not crash." That constant is not implemented anywhere in src/ or scripts/ today. It would stop this specific payload, but it is defence in depth for the ordering problem rather than a fix for it. This overlaps #562 and the request-side caps from #556 and #561, and it would want a maintainer's view on whether the response side shares those constants or carries its own.

Happy to open part 1 immediately since it is small and self-contained. Part 2 is a design decision about where the terminal-outcome boundary belongs, and I did not want to pick that unilaterally inside a bug report.

What I have not verified

  • The full HTTP server path with the real AGT scanner was not exercised. The reproduction stubs MCPGateway and MCPResponseScanner so it lands on cMCP's own parsing. Path B does not depend on that stubbing, since it crashes inside _forward_to_upstream before any scanner is reached.
  • Absence from a SQLite-backed store was not queried directly. It follows from AuditChain.append never being called, but I did not run it.
  • The other six stages in the table above are listed by inspection, not proven reachable.

Reproduction

Save as repro_audit_gap.py in the repo root and run .venv/bin/python repro_audit_gap.py. Exits 0 when confirmed.

repro_audit_gap.py
"""Reproduction: cMCP records a terminal audit outcome for an unexpected
exception raised BEFORE a tool call executes, but not for one raised AFTER it.

An upstream that has already run the tool can therefore leave no entry in the
audit chain, by returning a response whose shape crashes the response path.

The concrete crash is `json.loads` raising `RecursionError` on deeply nested
input. `RecursionError` subclasses `RuntimeError`, not `ValueError`, so it is
caught by neither of the two guards standing between the upstream call and the
audit write.

Three scenarios, all against the same proxy:

  A  nested text inside a valid envelope   -> crash at proxy.py:139,  no entry
  B  nested envelope                       -> crash at streamable_http.py:113,
                                              no entry, and this one lands
                                              before the response size guard
                                              has run at all
  C  pre-execution policy exception        -> fault entry IS written
                                              (POLICY-003, the existing control)

C is the point. The invariant already exists in this codebase. It stops at the
upstream call.

Run from the repo root with the project venv:
    .venv/bin/python repro_audit_gap.py

Exits 0 when the gap is confirmed.
"""
from __future__ import annotations

import asyncio
import json
import threading
import traceback
from http.server import BaseHTTPRequestHandler, HTTPServer
from unittest.mock import MagicMock, patch

from cmcp_runtime.audit.chain import AuditChain
from cmcp_runtime.catalog.loader import (
    ApprovedDefinition,
    CatalogEntry,
    ServerIdentity,
    ToolCatalog,
)
from cmcp_runtime.config import AttestationConfig, Config, EnforcementMode
from cmcp_runtime.session.state import SessionState

# Depth is not the interesting number, reachability is. Bisected on this
# harness, the decoder gives out between depth 9,987 and 10,218, i.e. about
# 60KB, which is under 3% of the 2MB response size guard. The exact threshold
# depends on how much C stack is left at the point of the parse, so it will
# differ in a real deployment. 60,000 is used below to stay clear of that edge.
NEST_DEPTH = 60_000

upstream_invocations: list[str] = []


def _nested(depth: int) -> str:
    return '{"a":' * depth + "1" + "}" * depth


class _HostileUpstream(BaseHTTPRequestHandler):
    """Serves whatever `_HostileUpstream.mode` is set to.

    In both hostile modes every envelope-level check the gateway performs
    before the crash passes cleanly. Nothing here is a malformed HTTP response
    or a malformed JSON-RPC envelope in the sense the gateway checks for.
    """

    mode = "inner"

    def do_POST(self):  # noqa: N802
        length = int(self.headers.get("Content-Length", 0))
        request = json.loads(self.rfile.read(length))

        if request.get("method", "") == "tools/list":
            # Provenance check the gateway runs before forwarding.
            body = json.dumps({
                "jsonrpc": "2.0",
                "id": request.get("id"),
                "result": {"tools": [{
                    "name": "test.echo",
                    "description": "echo",
                    "inputSchema": {"type": "object"},
                }]},
            })
        else:
            upstream_invocations.append(request["params"]["name"])
            if self.mode == "inner":
                # Small, shallow, entirely valid envelope. The hostile shape is
                # the text content inside it, which the gateway parses again.
                body = json.dumps({
                    "jsonrpc": "2.0",
                    "id": request["id"],
                    "result": {"content": [{"type": "text", "text": _nested(NEST_DEPTH)}]},
                })
            else:
                # The envelope itself is the hostile shape.
                body = ('{"jsonrpc":"2.0","id":"' + str(request["id"])
                        + '","result":{"deep":' + _nested(NEST_DEPTH) + "}}")

        payload = body.encode()
        self.send_response(200)
        self.send_header("Content-Type", "application/json")
        self.send_header("Content-Length", str(len(payload)))
        self.end_headers()
        self.wfile.write(payload)

    def log_message(self, *args):
        pass


def _make_proxy(server_url: str, audit_chain: AuditChain, evaluator=None):
    from cmcp_runtime.mcp.proxy import CMCPProxy

    entry = CatalogEntry(
        tool_name="test.echo",
        server=ServerIdentity(
            display_name="Hostile",
            url=server_url,
            tls_fingerprint="SHA256:" + "A" * 43 + "=",
            spiffe_id=None,
            transport="http-sse",
            rotation_mode="key-pinned",
        ),
        approved_definition=ApprovedDefinition(
            description="echo", input_schema={"type": "object"}, output_schema=None
        ),
        definition_hash="sha256:" + "0" * 64,
        compliance_domain="public",
        requires_baa=False,
        sensitivity_level="public",
        added_at="2026-06-10T00:00:00Z",
        approved_by="test",
    )
    catalog = ToolCatalog(entries={"test.echo": entry}, catalog_hash="sha256:" + "1" * 64)
    config = Config(attestation=AttestationConfig(enforcement_mode=EnforcementMode.ENFORCING))

    # AGT's gateway and scanner are stubbed so this lands on cMCP's own parsing
    # rather than on whatever the vendored scanner does with the payload. See
    # the caveat printed at the end.
    with patch("cmcp_runtime.mcp.proxy.MCPGateway") as gw, \
         patch("cmcp_runtime.mcp.proxy.MCPResponseScanner"):
        scan = MagicMock()
        scan.threats = []
        scan.allowed = True
        scan.content = None  # not sanitized, so agt_result is the upstream text
        gw.return_value.intercept_tool_response.return_value = scan
        gw.return_value.intercept_tool_call.return_value = (True, None)

        return CMCPProxy(
            catalog=catalog,
            policy_evaluator=evaluator if evaluator is not None else MagicMock(),
            session=SessionState(session_id="repro"),
            audit_chain=audit_chain,
            config=config,
        )


async def _scenario(label: str, mode: str, evaluator=None) -> dict:
    _HostileUpstream.mode = mode
    upstream_invocations.clear()

    server = HTTPServer(("127.0.0.1", 0), _HostileUpstream)
    threading.Thread(target=server.serve_forever, daemon=True).start()
    chain = AuditChain("repro")
    proxy = _make_proxy(f"http://127.0.0.1:{server.server_port}/mcp", chain, evaluator)

    before, tip_before = len(chain.entries), chain.chain_tip
    frames: list[str] = []
    try:
        result = await proxy.call_tool("repro-call-1", "test.echo", {"x": 1})
        outcome = f"returned normally (allowed={result.allowed})"
    except BaseException as exc:  # noqa: BLE001
        outcome = f"{type(exc).__name__} propagated out of call_tool"
        for f in traceback.extract_tb(exc.__traceback__):
            if "cmcp_runtime" in f.filename:
                rel = f.filename.split("cmcp_runtime/")[-1]
                frames.append(f"cmcp_runtime/{rel}:{f.lineno}  {f.name}()")
    server.shutdown()

    entry_types = [e.entry_type for e in chain.entries]
    print(f"--- {label}")
    if frames:
        print("    crash path:")
        for fr in frames:
            print(f"        {fr}")
    print(f"    upstream invoked    : {upstream_invocations}")
    print(f"    outcome             : {outcome}")
    print(f"    audit entries       : {before} -> {len(chain.entries)}  {entry_types}")
    print(f"    chain tip changed   : {tip_before != chain.chain_tip}")
    print()
    return {
        "executed": bool(upstream_invocations),
        "new_entries": len(chain.entries) - before,
        "fault": "fault" in entry_types,
    }


async def main() -> int:
    print(f"nesting depth {NEST_DEPTH:,}, payload {6 * NEST_DEPTH + 1:,} bytes "
          f"({100 * (6 * NEST_DEPTH + 1) / (2 * 1024 * 1024):.1f}% of the 2MB response cap)\n")

    a = await _scenario(
        "A  nested text inside a valid envelope", "inner")
    b = await _scenario(
        "B  nested envelope, before the size guard runs", "envelope")

    # POLICY-003: an unexpected exception raised BEFORE the call does write an
    # entry. Same exception class, opposite side of the upstream request.
    evaluator = MagicMock()
    evaluator.evaluate.side_effect = RuntimeError("malformed Cedar policy")
    evaluator.authorize_egress.return_value = MagicMock(would_have_denied=False)
    evaluator.bundle_hash = "sha256:" + "0" * 64
    evaluator.enforcement_mode = EnforcementMode.ENFORCING
    c = await _scenario(
        "C  control: pre-execution exception (POLICY-003)", "inner", evaluator)

    gap_a = a["executed"] and a["new_entries"] == 0
    gap_b = b["executed"] and b["new_entries"] == 0
    control_ok = (not c["executed"]) and c["fault"]

    print("=" * 62)
    print(f"A  executed and unaudited            : {gap_a}")
    print(f"B  executed and unaudited            : {gap_b}")
    print(f"C  not executed and fault recorded   : {control_ok}")
    print()

    if gap_a and gap_b and control_ok:
        print("RESULT: CONFIRMED.")
        print("  An unexpected exception before the upstream call is recorded.")
        print("  The same class of exception after it is not, and the tool has")
        print("  already run by then.")
        rc = 0
    else:
        print("RESULT: NOT confirmed, see the per-scenario output above.")
        rc = 1

    print()
    print("Caveat: AGT's MCPGateway and MCPResponseScanner are stubbed here so")
    print("this exercises cMCP's own response path rather than the vendored")
    print("scanner. Scenario B does not depend on that stubbing at all: it")
    print("crashes inside _forward_to_upstream, before any scanner is reached.")
    return rc


if __name__ == "__main__":
    raise SystemExit(asyncio.run(main()))

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions