Outbound MCP calls need audit log, redaction, and connection diagnostics #265

Activity

1. serge-ivo

   added a commit that references this issue on Aug 6, 2026

   feat(mcp): dual-era protocol, per-server consent, redacted trace (#260 …

   ...

   4f77f33
2. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Partly delivered in 4f77f33 — leaving this open. Server-side logging + redaction are done;
   the admin filters and the connection-test UI are not.

   Verified against HEAD before building

   • Outbound MCP calls emitted nothing — mcp.ts never called logEvent or logError. ✔
   • Code pointers were accurate: lib/events.ts, lib/redact.ts and
     routes/admin-mcp-audit.ts all exist. One correction: admin-mcp-audit.ts reads the
     inbound MCP server's KV audit (workers/mcp/src/safety.ts → audit()), keyed by MCP
     account subject. It is not a place outbound calls can be written to, so outbound rides the
     unified agent_events trace (migration 0038) instead — which is what makes it visible in
     the existing owner-facing /v1/instances/:id/trace, the MCP agent_trace tool, and
     /v1/admin/trace/:instanceId for free.

   Done

   Every outbound call writes one agent_events row, source: "mcp":

   • mcp.call — endpoint (normalized), method, remote tool, era + negotiated protocol version,
     HTTP status, durationMs, ok, and a failure class: bad_input · no_credential ·
     denied · auth · unsupported_version · header_mismatch · missing_capability ·
     rpc_error · tool_error · unparseable · blocked · network. tool_error (the tool
     refused) is deliberately distinct from rpc_error (the call failed) — different thing to
     debug.
   • mcp.denied — written before dispatch when Outbound MCP consent should be per server and per remote tool, not one global write grant #262 consent refuses, with the reason. A
     denial writes no mcp.call, so the timeline doesn't imply an attempt that never happened.

   Redaction is real, not cosmetic, and is tested by asserting on the serialized log:

   • Argument and result VALUES are never recorded. Only argument key names and a byte
     count. This is a deliberate deviation from "logs redact secrets … by key name and value
     shape": arguments to an arbitrary remote tool are the most PII-dense thing this connector
     touches, and a log that stores them redacted is still a second copy of the user's data
     sitting in a table with 14-day retention. Key names + size answer the actual debugging
     question ("did it send the field?"). If you want values, that should be an explicit opt-in
     with its own retention, not the default.
   • The endpoint's query string is dropped — an MCP endpoint's query is routinely the
     credential.
   • Server error bodies are run through redactText before they reach the transcript or the
     log, so a server that reflects our Authorization header into its error page can't leak it.
   • Everything that is recorded still goes through redactSecrets as a second net.
   • The bearer token never enters the context at all; there's a test asserting it isn't in the
     serialized events.

   Acceptance criteria

   • mcp_list_tools and remote tool calls log endpoint/connection, method, remote tool,
     status, duration, failure class
   • logs redact secrets, bearer headers, tokens, keys (by key name and value shape) — see
     the deviation above on argument values
   • denied calls logged with the consent reason before dispatch
   • user-facing instance history can show recent outbound MCP activity — GET /v1/instances/:id/trace?source=mcp
   • admin diagnostics can filter by user, instance, endpoint, and remote tool — only
     partly. /v1/admin/trace/:instanceId?source=mcp filters by instance/source/level/trace,
     and endpoint + tool are in context, but there is no cross-user view and no
     endpoint/tool filter. Needs either a context LIKE filter on admin-trace or a
     dedicated route; I left it rather than half-adding a filter that scans unindexed JSON.
   • connection test UI exposes actionable diagnostics — not built. This overlaps almost
     entirely with MCP connection setup UI should validate, connect, discover, and test a user-provided endpoint #266 (connection setup UI: validate → connect → discover → test), and the
     server side it needs now exists: era/version negotiation reports a precise outcome (Outbound MCP client needs protocol/version negotiation for current Streamable HTTP #260),
     discoverAuthServer reports the auth model ([connectors] dcr-oauth2 — discovery + dynamic client registration + PKCE for user-named endpoints #180/[connectors] Model unattended credential survivability — warn at wiring time, not via dead letters #181), and every attempt is traced. It
     belongs in MCP connection setup UI should validate, connect, discover, and test a user-provided endpoint #266's UI work rather than as a second connection panel here.
   • tests cover successful call logging, denied call logging, redaction, and remote error
     logging

   Remaining work is the two unchecked boxes.
3. serge-ivo

   added a commit that references this issue on Aug 6, 2026

   feat(mcp): connect an MCP server by testing it, not by guessing (#266, …

   ...

   8de7c56
4. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Completed across two commits.

   4f77f33 — outbound calls logged to the agent_events unified trace as mcp.call / mcp.denied (endpoint, method, tool, era, version, status, duration, failure class), with denials logged before dispatch.

   8de7c56 — the two remaining ACs:

   • Admin filtering by endpoint and remote tool. routes/admin-trace.ts gains a cross-instance listing plus endpoint/tool filters. The endpoint is normalized before matching, so a filter typed with a trailing slash still matches. Deliberately NOT a LIKE over the serialized blob — that would match an endpoint appearing in some unrelated field, and a filter that quietly over-matches in an AUDIT tool is worse than no filter at all.
   • Connection diagnostics — lib/mcp-connection.ts, surfaced through the setup UI (MCP connection setup UI should validate, connect, discover, and test a user-provided endpoint #266).

   One deliberate deviation, restating it here so it is not lost: argument and result VALUES are never logged — only argument key names and byte counts. A redacted copy of arbitrary remote-tool arguments is still a second copy of the user's PII, so value logging should be an explicit opt-in, never the default. If that opt-in is wanted it is a new issue, not a silent change here.

   The original issue body pointed at routes/admin-mcp-audit.ts as the sink. That was wrong: it reads the INBOUND MCP server's KV audit keyed by MCP account subject, so outbound calls cannot be written there. agent_events was used instead, which also gets owner-facing /trace, MCP agent_trace and /v1/admin/trace for free.
5. serge-ivo

   closed this as completedon Aug 6, 2026
6. serge-ivo

   added 2 commits that reference this issue on Aug 6, 2026

   feat(mcp): read the server's resources and prompts — fenced, capped, …

   ...

   4062a07

   feat(mcp): pause a remote call that asks for user input, and resume i…

   ...

   5d8e497