Skip to content

Commit 938ee1f

Browse files
committed
feat(mcp): native-app redirect URIs, and every tool spells out its hints
Registration accepts a private-use scheme in scheme://... authority form (RFC 8252 §7.1), which is what a desktop MCP client registers; PKCE stays mandatory, so a scheme squatter can catch a code and still not exchange it. Read tools now carry destructiveHint and idempotentHint explicitly rather than leaning on the spec's defaults.
1 parent 481d635 commit 938ee1f

4 files changed

Lines changed: 137 additions & 17 deletions

File tree

‎apps/api/src/http/mcp.ts‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ export interface McpToolAnnotations {
9191

9292
export interface McpToolDefinition {
9393
readonly name: string
94+
/**
95+
* Human-readable display name (MCP spec `2025-06-18`, `Tool.title`).
96+
* Directory reviews (Anthropic's connector directory, ChatGPT's app review)
97+
* require every tool to carry one; clients show it in consent prompts where
98+
* `create_site` reads worse than "Create Site".
99+
*/
100+
readonly title: string
94101
readonly description: string
95102
/**
96103
* The `/v1` path this tool dispatches into. A read tool names a `/read/*`
@@ -138,8 +145,20 @@ export function isReadOnlyTool(tool: McpToolDefinition): boolean {
138145
return tool.annotations?.readOnlyHint !== false
139146
}
140147

141-
/** Read-only annotations, the default every existing tool carries. */
142-
const READ_ONLY: McpToolAnnotations = { readOnlyHint: true }
148+
/**
149+
* Read-only annotations, the default every existing tool carries.
150+
*
151+
* All three hints are spelled out rather than left to a client's defaults.
152+
* The spec says `destructiveHint` is only consulted when `readOnlyHint` is
153+
* false, but ChatGPT's app review scans for the annotation itself and marks a
154+
* tool that omits it as unannotated (2026-08-29), and a reviewer reading
155+
* hints should never have to infer one from the absence of another.
156+
*/
157+
const READ_ONLY: McpToolAnnotations = {
158+
readOnlyHint: true,
159+
destructiveHint: false,
160+
idempotentHint: true,
161+
}
143162

144163
const RANGE_PARAMS = [
145164
{ name: 'from', description: 'Range start, ISO-8601 UTC (inclusive).', required: true },
@@ -303,6 +322,7 @@ const EVENT_DEFINITION_CONTENT_PROPERTIES = {
303322
const mcpTools: McpToolDefinition[] = [
304323
{
305324
name: 'list_sites',
325+
title: 'List Sites',
306326
description:
307327
'List the analytics sites this credential may read, with their slug, name, status and the caller’s role. Call this first: every other tool needs a site id from here.',
308328
path: '/read/sites',
@@ -312,6 +332,7 @@ const mcpTools: McpToolDefinition[] = [
312332
},
313333
{
314334
name: 'site_overview',
335+
title: 'Site Overview',
315336
description:
316337
'Totals for one site over a date range — events, pageviews, billable events, visitors — with data-freshness metadata.',
317338
path: '/read/analytics/overview',
@@ -321,6 +342,7 @@ const mcpTools: McpToolDefinition[] = [
321342
},
322343
{
323344
name: 'site_timeseries',
345+
title: 'Site Timeseries',
324346
description: 'The same totals bucketed over time, for trends and comparisons.',
325347
path: '/read/analytics/timeseries',
326348
params: [...RANGE_PARAMS],
@@ -329,6 +351,7 @@ const mcpTools: McpToolDefinition[] = [
329351
},
330352
{
331353
name: 'top_pages',
354+
title: 'Top Pages',
332355
description: 'The most-viewed pages on one site over a range.',
333356
path: '/read/analytics/pages',
334357
params: [...RANGE_PARAMS, LIMIT_PARAM],
@@ -337,6 +360,7 @@ const mcpTools: McpToolDefinition[] = [
337360
},
338361
{
339362
name: 'top_sources',
363+
title: 'Top Sources',
340364
description: 'Where a site’s traffic came from — referrers and campaigns.',
341365
path: '/read/analytics/sources',
342366
params: [...RANGE_PARAMS, LIMIT_PARAM],
@@ -345,6 +369,7 @@ const mcpTools: McpToolDefinition[] = [
345369
},
346370
{
347371
name: 'geography',
372+
title: 'Geography',
348373
description: 'Visitors by country and city.',
349374
path: '/read/analytics/geography',
350375
params: [...RANGE_PARAMS, LIMIT_PARAM],
@@ -353,6 +378,7 @@ const mcpTools: McpToolDefinition[] = [
353378
},
354379
{
355380
name: 'devices',
381+
title: 'Devices',
356382
description: 'Visitors by device, browser and operating system.',
357383
path: '/read/analytics/devices',
358384
params: [...RANGE_PARAMS, LIMIT_PARAM],
@@ -361,6 +387,7 @@ const mcpTools: McpToolDefinition[] = [
361387
},
362388
{
363389
name: 'sessions',
390+
title: 'Sessions',
364391
description:
365392
'Session counts and durations, with the finalizer watermark that says how much of the range can still be revised.',
366393
path: '/read/analytics/sessions',
@@ -370,6 +397,7 @@ const mcpTools: McpToolDefinition[] = [
370397
},
371398
{
372399
name: 'revenue_summary',
400+
title: 'Revenue Summary',
373401
description:
374402
'Revenue totals for one site over a date range: gross charges, refunds, withdrawn and reinstated disputes, provider fees and net, plus charge, refund and dispute counts. Every amount is an integer in the minor unit of the site’s reporting currency (`totals.currency`) — 91000 in USD is $910.00, never divide before you have said which currency. Owner-only: any other role is refused with FORBIDDEN, which you must report rather than work around. Read `meta.revenue.connection_status` before calling a zero "no sales": `not_connected` means no payment provider was ever connected.',
375403
path: '/read/revenue/summary',
@@ -379,6 +407,7 @@ const mcpTools: McpToolDefinition[] = [
379407
},
380408
{
381409
name: 'revenue_timeseries',
410+
title: 'Revenue Timeseries',
382411
description:
383412
'The same revenue measures bucketed over time, one point per hour or per day, for trends. Same currency, minor-unit and owner-only rules as revenue_summary, and the same `meta.revenue.connection_status` caveat. It returns no customer, order or transaction identifiers; individual transactions are not readable from any tool.',
384413
path: '/read/revenue/timeseries',
@@ -392,6 +421,7 @@ const mcpTools: McpToolDefinition[] = [
392421
// path token, not the `X-OA-Site` header.
393422
{
394423
name: 'site_install',
424+
title: 'Install Snippet',
395425
description:
396426
'The tracking snippet and site key for one site — the public token that ships in page source. Read it to drop the tracker into a site’s HTML so it starts reporting. It is public by design; the private read key is never returned here.',
397427
path: '/read/site',
@@ -401,6 +431,7 @@ const mcpTools: McpToolDefinition[] = [
401431
},
402432
{
403433
name: 'team_members',
434+
title: 'Team Members',
404435
description:
405436
'The members of one site’s team — user id, role and email address. Needs the team:read scope, which is named on the consent screen because it carries emails.',
406437
path: '/sites/{site_id}/members',
@@ -410,6 +441,7 @@ const mcpTools: McpToolDefinition[] = [
410441
},
411442
{
412443
name: 'list_funnels',
444+
title: 'List Funnels',
413445
description:
414446
'The funnels defined on one site, for reading and analyzing their steps. Include archived ones with include_archived=true.',
415447
path: '/sites/{site_id}/funnels',
@@ -425,6 +457,7 @@ const mcpTools: McpToolDefinition[] = [
425457
},
426458
{
427459
name: 'list_event_definitions',
460+
title: 'List Event Definitions',
428461
description:
429462
'The dashboard-defined custom events on one site — each with its name, status and currently published version number. Read this before writing a definition: the version number is what publish and rollback need to state which change they expect to replace. Include archived ones with include_archived=true.',
430463
path: '/sites/{site_id}/event-definitions',
@@ -441,6 +474,7 @@ const mcpTools: McpToolDefinition[] = [
441474
},
442475
{
443476
name: 'list_widgets',
477+
title: 'List Widgets',
444478
description: 'The embeddable widgets configured on one site.',
445479
path: '/sites/{site_id}/widgets',
446480
params: [],
@@ -449,6 +483,7 @@ const mcpTools: McpToolDefinition[] = [
449483
},
450484
{
451485
name: 'share_settings',
486+
title: 'Share Settings',
452487
description:
453488
'One site’s public-dashboard settings: which surfaces are shared and the current public link.',
454489
path: '/sites/{site_id}/public-dashboard',
@@ -461,6 +496,7 @@ const mcpTools: McpToolDefinition[] = [
461496
// not read-only prompts the human for confirmation before each of these.
462497
{
463498
name: 'create_site',
499+
title: 'Create Site',
464500
description:
465501
'Create a new analytics site. Returns its id and its tracking key, so the snippet can be installed in the same flow. There is deliberately no domain field: a new site accepts events from any origin until its owner sets an origin allowlist, and that allowlist is a dashboard-only setting, never writable over MCP. Rename or restyle later with update_site.',
466502
path: '/sites',
@@ -481,6 +517,7 @@ const mcpTools: McpToolDefinition[] = [
481517
},
482518
{
483519
name: 'update_site',
520+
title: 'Update Site',
484521
description:
485522
'Change a site’s name or reporting timezone. This tool cannot change the domain allowlist or any other setting — those are not writable over MCP.',
486523
path: '/sites/{site_id}',
@@ -500,6 +537,7 @@ const mcpTools: McpToolDefinition[] = [
500537
},
501538
{
502539
name: 'create_funnel',
540+
title: 'Create Funnel',
503541
description:
504542
'Create a funnel on one site from an ordered list of page-path or event-name steps.',
505543
path: '/sites/{site_id}/funnels',
@@ -526,6 +564,7 @@ const mcpTools: McpToolDefinition[] = [
526564
},
527565
{
528566
name: 'update_funnel',
567+
title: 'Update Funnel',
529568
description: 'Edit a funnel’s name, steps, scope or window.',
530569
path: '/sites/{site_id}/funnels/{funnel_id}',
531570
method: 'PATCH',
@@ -546,6 +585,7 @@ const mcpTools: McpToolDefinition[] = [
546585
},
547586
{
548587
name: 'archive_funnel',
588+
title: 'Archive Funnel',
549589
description: 'Archive a funnel. Reversible — an archived funnel is hidden, not deleted.',
550590
path: '/sites/{site_id}/funnels/{funnel_id}',
551591
method: 'DELETE',
@@ -560,6 +600,7 @@ const mcpTools: McpToolDefinition[] = [
560600
// definition stops its rules being served site-wide.
561601
{
562602
name: 'create_event_definition',
603+
title: 'Create Event Definition',
563604
description:
564605
'Define a new custom event on one site, with its first version’s content. The definition starts unpublished: nothing reaches a visitor’s browser until publish_event_definition names a version. The event name is permanent — it is the key every chart and funnel will refer to.',
565606
path: '/sites/{site_id}/event-definitions',
@@ -584,6 +625,7 @@ const mcpTools: McpToolDefinition[] = [
584625
},
585626
{
586627
name: 'draft_event_version',
628+
title: 'Draft Event Version',
587629
description:
588630
'Save a new draft version of an existing event definition. The whole content is restated, not patched — a version is immutable and complete. A draft is in nobody’s browser until it is published, so this is the safe half of editing an event.',
589631
path: '/sites/{site_id}/event-definitions/{definition_id}/versions',
@@ -601,6 +643,7 @@ const mcpTools: McpToolDefinition[] = [
601643
},
602644
{
603645
name: 'publish_event_definition',
646+
title: 'Publish Event Definition',
604647
description:
605648
'Make one version of an event definition live — from this call on, visitors’ browsers evaluate its rules. Reversible with rollback_event_definition, which is why publishing is on this surface at all. `expected_published_version` is not optional bookkeeping: state the version you read from list_event_definitions (or null if nothing is published yet), and a concurrent publish by someone else comes back as a conflict naming what is actually live instead of silently overwriting it.',
606649
path: '/sites/{site_id}/event-definitions/{definition_id}/publish',
@@ -625,6 +668,7 @@ const mcpTools: McpToolDefinition[] = [
625668
},
626669
{
627670
name: 'rollback_event_definition',
671+
title: 'Rollback Event Definition',
628672
description:
629673
'Republish an older version’s content as a new version — the undo for publish_event_definition. It rewinds nothing: calling it twice produces two versions, and the response says which one is now live. Same `expected_published_version` rule as publishing.',
630674
path: '/sites/{site_id}/event-definitions/{definition_id}/rollback',
@@ -649,6 +693,7 @@ const mcpTools: McpToolDefinition[] = [
649693

650694
{
651695
name: 'create_widget',
696+
title: 'Create Widget',
652697
description: 'Create an embeddable widget on one site.',
653698
path: '/sites/{site_id}/widgets',
654699
method: 'POST',
@@ -665,6 +710,7 @@ const mcpTools: McpToolDefinition[] = [
665710
},
666711
{
667712
name: 'update_widget',
713+
title: 'Update Widget',
668714
description:
669715
'Edit a widget, or enable/disable it. Disabling is the reversible way to take a widget down — there is no delete over MCP, because a widget id is never reissued.',
670716
path: '/sites/{site_id}/widgets/{widget_id}',
@@ -681,6 +727,7 @@ const mcpTools: McpToolDefinition[] = [
681727
},
682728
{
683729
name: 'update_share_settings',
730+
title: 'Update Share Settings',
684731
description:
685732
'Change which surfaces a site’s public dashboard shows, and optionally rotate its public link. Rotating the link revokes the old one — the safe direction.',
686733
path: '/sites/{site_id}/public-dashboard',
@@ -998,6 +1045,18 @@ export function createMcpRoutes(deps: McpRoutesDeps): Hono<Env> {
9981045
},
9991046
)
10001047

1048+
// The transport is POST-only (no SSE stream, no sessions), which streamable
1049+
// HTTP permits; what it does not permit is a `404` for the other verbs on a
1050+
// URL that plainly exists. `405` plus `Allow` tells a probing client "right
1051+
// door, wrong verb" instead of "no such door". `GET` is the optional
1052+
// server-to-client stream, `DELETE` the optional session teardown; refusing
1053+
// both is within spec.
1054+
app.on(
1055+
['GET', 'DELETE'],
1056+
'/mcp',
1057+
() => new Response(null, { status: 405, headers: { Allow: 'POST' } }),
1058+
)
1059+
10011060
app.post('/mcp', async (c) => {
10021061
const authorization = c.req.header('authorization')
10031062
if (!authorization) return unauthorized()
@@ -1043,12 +1102,16 @@ export function createMcpRoutes(deps: McpRoutesDeps): Hono<Env> {
10431102
return reply({
10441103
tools: MCP_TOOLS.map((tool) => ({
10451104
name: tool.name,
1105+
title: tool.title,
10461106
description: tool.description,
10471107
inputSchema: inputSchema(tool),
10481108
// A client reads the hints to decide whether to prompt for
10491109
// confirmation (ADR-0048 D3). Read tools carry no `annotations`
1050-
// field and default to read-only here.
1051-
annotations: tool.annotations ?? READ_ONLY,
1110+
// field and default to read-only here. `openWorldHint` is stamped
1111+
// in one place because it is uniformly false: every tool
1112+
// dispatches into this same api, a closed system, and no tool
1113+
// reaches the open internet.
1114+
annotations: { openWorldHint: false, ...(tool.annotations ?? READ_ONLY) },
10521115
})),
10531116
})
10541117

‎packages/domain/src/oauth-registration.ts‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,17 @@
77
* this file is what it enforces.
88
*
99
* The frame is the F-309 settlement (2026-08-08): public clients only, PKCE
10-
* required (the provider's job, not this file's), redirect URIs https or
11-
* loopback with no wildcards, and the scope ceiling stays whatever the
12-
* authorization server supports — registration introduces a client, it never
13-
* widens scopes.
10+
* required (the provider's job, not this file's), redirect URIs https,
11+
* loopback, or a native app's private-use scheme with no wildcards, and the
12+
* scope ceiling stays whatever the authorization server supports:
13+
* registration introduces a client, it never widens scopes.
14+
*
15+
* Private-use schemes are the one widening ADR-0047 D3 said would be made
16+
* deliberately when a real client needed it. Cursor did (2026-08-28): its MCP
17+
* client registers `cursor://anysphere.cursor-mcp/oauth/callback`, the RFC
18+
* 8252 §7.1 shape for a desktop app with no web origin. PKCE being mandatory
19+
* is what keeps this safe: another app claiming the scheme can catch the
20+
* code but cannot finish the exchange without the verifier.
1421
*/
1522

1623
/** RFC 7591 §3.2.2's refusal vocabulary — the subset this server uses. */
@@ -37,6 +44,15 @@ export const MAX_REDIRECT_URIS = 10
3744
/** The hosts RFC 8252 §7.3 lets a native client listen on over plain http. */
3845
const LOOPBACK_HOSTS = new Set(['127.0.0.1', '[::1]', 'localhost'])
3946

47+
/**
48+
* Schemes a redirect must never carry: the authorize step answers with a
49+
* `Location` header, and any of these in it executes or reads in the
50+
* authorize page's own context instead of launching an app. The structural
51+
* authority-form check below also excludes them; the list is the named
52+
* second lock.
53+
*/
54+
const FORBIDDEN_SCHEMES = new Set(['javascript:', 'data:', 'blob:', 'file:', 'about:', 'vbscript:'])
55+
4056
export interface ValidatedClientRegistration {
4157
readonly redirectUris: readonly string[]
4258
/** Registration-supplied, for the consent screen; absent falls back to the
@@ -67,11 +83,17 @@ function refuse(error: ClientRegistrationError, description: string): ClientRegi
6783
/**
6884
* One redirect URI against ADR-0047 D3, or the sentence that refuses it.
6985
*
70-
* `https:` for any host; `http:` only for the loopback hosts, any port; no
71-
* fragment (RFC 6749 §3.1.2); no `*` anywhere in the string — a wildcard is
72-
* refused at registration so exact-match comparison at authorize time has
73-
* nothing to misjudge. Custom schemes are a policy widening to make
74-
* deliberately, not a default (ADR-0047 D3).
86+
* `https:` for any host; `http:` only for the loopback hosts, any port; a
87+
* private-use scheme (RFC 8252 §7.1) in `scheme://...` authority form for
88+
* native clients; no fragment (RFC 6749 §3.1.2); no `*` anywhere in the
89+
* string, since a wildcard refused at registration leaves exact-match
90+
* comparison at authorize time nothing to misjudge.
91+
*
92+
* The authority-form requirement on custom schemes is structural safety, not
93+
* pedantry: `javascript:alert(1)` and `data:text/html,...` are one-part
94+
* URIs, so demanding `//` after the scheme shuts out the whole family of
95+
* browser-executable payloads even before the denylist names the usual
96+
* suspects.
7597
*/
7698
function redirectUriRefusal(uri: string): string | null {
7799
if (uri.includes('*')) return `redirect_uri must not contain a wildcard: ${uri}`
@@ -87,7 +109,13 @@ function redirectUriRefusal(uri: string): string | null {
87109
if (LOOPBACK_HOSTS.has(parsed.hostname === '::1' ? '[::1]' : parsed.hostname)) return null
88110
return `http redirect_uri is only accepted on a loopback host: ${uri}`
89111
}
90-
return `redirect_uri must be https or a loopback http URL: ${uri}`
112+
if (FORBIDDEN_SCHEMES.has(parsed.protocol)) {
113+
return `redirect_uri scheme is not allowed: ${uri}`
114+
}
115+
if (!uri.slice(parsed.protocol.length).startsWith('//')) {
116+
return `custom-scheme redirect_uri must use the scheme://... authority form: ${uri}`
117+
}
118+
return null
91119
}
92120

93121
/**

0 commit comments

Comments
 (0)