MCP token parameter contract: audit findings #681

Summary

Audit of workers/mcp/src/ in this repo (137 statically-registered tools, matching the pinned MCP_TOOL_COUNT in tool-count.ts) found two deviations from a uniform token-parameter contract.

The canonical contract is one exact string, used by 122 tools:

token: z.string().optional().describe("PAGS session token. Omit when connected with browser sign-in.")

Findings

1. agent_deploy_status — no token parameter and no ownership check

index.ts:719-727. The whole handler:

this.server.tool(
  "agent_deploy_status",
  "Check the latest GitHub Actions deploy runs for an agent repo.",
  { agent_id: z.string().describe("Agent ID or slug") },
  async ({ agent_id }) => {
    const org = this.env.GITHUB_ORG || "ProAgentStore";
    return text(await deployStatus(this.env, org, repoNameFor(agent_id)));
  },
);

It takes no token and calls no ownsAgent(). It reaches GitHub with the worker's own env.GITHUB_TOKEN (repo-tools.ts:62-95), not the caller's, and repoNameFor only slugifies its input (repo-tools.ts:41) — so the caller chooses any repo name within the fixed org. Returned per run: workflow name, conclusion/status, short head SHA, timestamp, run URL.

It is the sole outlier in the GitHub-backed family:

(scaffold_agent has a token but no ownsAgent() — correct, it creates the agent, so there is nothing to own yet.)

Its declared TOOL_RISK of "read" (tool-metadata.ts:102) is announced but not enforced here — the handler never calls requirePermission, unlike its siblings. The registration wrapper's only gate is suspensionBlock, which returns null when there is no token (suspension.ts:38), so a tool with no token parameter never meets it.

Worth noting the codebase already flags this tool by name: suspension.ts:10 and registration.ts:11 both record that "agent_deploy_status did not even take a token". #273 closed the suspension hole by wrapping the registrar; ownership was orthogonal and was not part of that fix.

Verified: no token param, no ownership check, platform credential, caller-controlled repo name within the org.
Not verified: whether the /mcp endpoint permits an unauthenticated connection at all. If OAuth is mandatory the impact is cross-tenant (any authenticated user reads any creator's CI metadata); if not, it is unauthenticated. I did not trace the OAuth layer far enough to say. The fix is the same either way.

Severity: security / correctness. Not a schema-only fix — the handler needs the credential and ownership check, not just a parameter.

2. Nine tools in storage-tools.ts — token parameter missing .describe()

Lines 28, 43, 69, 93, 119, 249, 268, 301, 324 — list_collections, create_collection, query_records, insert_record, update_record, list_agent_files, upload_agent_file, search_agent_knowledge, agent_activity.

All nine declare token: z.string().optional(), and implement the correct fallback, so the runtime contract is right — this is documentation only. They just omit .describe(), so an MCP client rendering the schema shows a blank description and the model is not told it can omit the token under browser sign-in. Every other tool in the surface carries the canonical string. The divergence is file-wide: storage-tools.ts is internally consistent and inconsistent with everything else.

Severity: cosmetic / documentation. Schema-only fix.

What is correct

131 of 137 tools declare token as z.string().optional() with a working fallback (this.token(provided) in index.ts, tokenFor(token) in the instance-tools/* registrars); 122 of those use the canonical describe string. There are zero tools where token is typed as anything other than z.string().optional().

The remaining six take no token parameter, and five are correct: list_agents, agent_info and chat_with_agent hit /v1/public/* endpoints, and platform_guide and sdk_reference return static text. The sixth is finding 1.

Users are not asked to paste session tokens anywhere in this surface. The regression described in proappstore-online/platform#139 — list_apps and schema_status declaring token REQUIRED with no fallback — is not present here.

Fix recommendation

1. agent_deploy_status — add the token parameter and an ownsAgent() check, matching trigger_agent_deploy (index.ts:729-758), which is the same GitHub-backed shape with the guards already in place. Ship the credential check, not a schema-only change.
2. storage-tools.ts — add .describe("PAGS session token. Omit when connected with browser sign-in.") to the token parameter of all nine tools, matching the canonical string used everywhere else.

Cross-reference

Found during a two-repo audit of MCP token-parameter contracts, alongside proappstore-online/platform#139 (PAS).

The two repos solve the same problem with inverted defaults:

• PAGS (this repo) — identity resolved per handler. A tool can omit the fallback entirely and nothing catches it, which is finding 1.
• PAS — identity resolved per registrar via a closure. An individual tool cannot forget the fallback, but a registrar wired without the getter silently strips it from every tool it registers, which is Orphaned coding sessions never reconciled — D1 shows 6 active, runner has 0 live tmux #139.

The credential-handling regression in #139 (authenticated users asked to paste session tokens) is specific to PAS and does not occur here; conversely finding 1 has no PAS equivalent.