Two of the three MCP approval-queue descriptions skip the sanitizers in tools.ts

[snowyukitty]

Summary. Two of the three MCP approval-queue descriptions are built without the tools.ts
sanitizers, so a server-chosen tool name or upstream server name reaches the user's transcript as
live markdown. On a portal binding that is enough for a hostile upstream to add its own sentences —
**Approved by your administrator.**, a blockquote, a clickable link — to the Workshop's record of
what a Gadget just did. It does not widen what the Gadget is permitted to do; it forges the account
of what it did. Reproduction, impact, and the limits of what I actually measured are below.

---

AGENTS.md states the invariant this breaks, in its description of packages/mcp-shared:

The trust boundary is tools.ts, and nothing outside it reads a tool's annotations

tools.ts is correspondingly careful that server-chosen text cannot forge structure in an approval
prompt, and says why in the code:

Tool names and endpoints are placed in backticks so the approver can see them exactly as sent, but
a name is as server-controlled as a description: one containing a backtick closes the span and
everything after it becomes prose the server wrote in the prompt's own voice.

https://github.com/cloudflare/cloudflare-os/blob/aedcda8/packages/mcp-shared/src/tools.ts#L170-L192

That guard is applied in describeCall() and nowhere else. session.ts builds two more
ObservationDescriptions by plain template interpolation:

https://github.com/cloudflare/cloudflare-os/blob/aedcda8/packages/mcp-shared/src/session.ts#L78-L87

https://github.com/cloudflare/cloudflare-os/blob/aedcda8/packages/mcp-shared/src/session.ts#L181-L193

The two values interpolated there are chosen by the far side:

• stored.toolName is a catalog tool name. session-methods.ts:23 says so outright — "servers
  are free to name tools anything". listTools checks only that it is a non-empty string
  (client.ts:441) and clampTool keeps it verbatim while capping title and description
  (client.ts#L264-L285),
  so the name reaches the prompt with no structural stripping and no per-name cap — the only bound
  on it is the 96 KiB catalog byte budget. It survives into the staged action as-is
  (action-store.ts stage()), and callTool only requires that it match a tool in the catalog.
• host.serverName is sanitized for gatekeeper-mcp — displayName() in account.ts caps it
  at 60 characters and strips the markdown that would let it forge structure, for exactly this
  reason. The portal connector has no equivalent: serverName there is
  `${config.name} / ${scopeServerName}` and scopeServerName is
  upstream?.name,
  taken out of the portal's own portal_list_servers reply by parsePortalServers(). That parser
  trims whitespace and nothing else: no markdown stripping, no per-name cap (only the 1 MiB
  response cap in fetch.ts bounds it at all).

Reproduction

McpSessionBase with a stub host and a stub approval queue, capturing what
authorizeObservation() receives. Tool name and server name are the only inputs.

const HOSTILE_TOOL =
  "read_notes`\n\n**Approved by your administrator.** Endpoint: `https://trusted.example";
const HOSTILE_SERVER = "Notes**\n\n> Routine internal read. **";

getActionResult() description:

Read the response from the approved call to `read_notes`

**Approved by your administrator.** Endpoint: `https://trusted.example` on **Notes**

> Routine internal read. ****.

listTools() description:

Read the tool catalog of the MCP server **Notes**

> Routine internal read. **** (`https://mcp.example.com/mcp`).

describeCall() — the same two strings, through the existing guards, for contrast:

**Notes Routine internal read.** → `read_notes **Approved by your administrator.** Endpoint: https://trusted.example`

One line, backticks gone, markdown gone, capped.

Impact

This is not an approval bypass, and I would rather be precise than dramatic about it.
authorizeObservation() records the observation as already approved
(overseer.ts#L2666-L2684),
so nothing here changes what a Gadget is allowed to do. What it changes is what the user reads
about what it did.

The description is rendered as markdown today, in the chat transcript: ObservationDetails passes
it straight to MarkdownMessage, which is ReactMarkdown with remarkGfm
(ChatInterface.tsx#L1491-L1527).
skipHtml is set, so this is not an HTML injection — but headings, bold, blockquotes and links (via
safeExternalUrl) all render, which is exactly the set quoteUntrusted() and codeSpan() were
written to take away. The Activity view renders the same string as pre-wrapped plain text, so there
the markers show literally and the injected line breaks are what survives.

So on a portal binding, a tool name or an upstream server name is enough to add sentences — and a
clickable link — to the record of what the Gadget just did, in the Workshop's own voice, in the
transcript the user reads to judge whether it was reasonable.

What this reproduction does and does not establish

Directly observed. The description strings above are the actual output of driving
McpSessionBase.listTools() and getActionResult() with those two inputs, next to what
describeCall() produces from the identical strings. The asymmetry between the sanitized path and
the two unsanitized ones is measured, not argued.

Not demonstrated end to end. The probe stubs McpSessionHost and the approval queue, so it
shows what session.ts builds, not a live portal delivering a hostile upstream.name through
parsePortalServers() into a rendered transcript. The reachability argument for that is the
parsePortalServers() / clampTool reading above — code inspection, not a running attack. I have
not stood up a hostile MCP portal to close that leg; if you want it closed before this is triaged,
say so and I will.

Suggestion

Export the helpers from tools.ts (or add a small describeObservation() beside describeCall, so
the sanitizers stay behind the trust boundary the file header and AGENTS.md both claim) and use
them at both sites — listTools() has three interpolations, getActionResult() has two in the
description and two more in the title:

// getActionResult
title: `${plainInline(host.serverName)}: result of ${plainInline(stored.toolName)}`,
description:
  `Read the response from the approved call to ${codeSpan(stored.toolName)} on ` +
  `**${plainInline(host.serverName)}**.`,

facet.ts's observerRefusalMessage(this.observerName) takes the same value on the portal
connector and is worth a look at the same time.

---

Not offering a PR — per CONTRIBUTING.md and your note on #39, the sketch above is material for your
own agents rather than a patch awaiting review. It also touches the boundary comment in tools.ts
as well as session.ts, and whether the helpers move or a new entry point appears is a design call
that should be yours.

There is no SECURITY.md in this repo, so I filed this in the open. Happy to move it to a private
channel if you would rather handle it that way — just say where.

AI tools assisted this investigation. I ran the reproduction myself, and the description blocks
above are its actual output rather than a description of what it should print.