Skip to content

Commit 7fb21ed

Browse files
committed
fix(mcp): freeze the per-call principal copy and teach describe_object only beside data:read
Two hardening items from the contract review of the authoring bridge. The provenance builder stamped the host's principal object by reference, so an injected tool that wrote into its context could taint the next call in the same request and the host's own object. Each call now receives a frozen copy of the principal, with its `scopes` array copied and frozen too, and a frozen `actor`; the context object was already fresh per call. A pin drives four hostile writes through a tool and expects every one refused, the second call to see the host's principal byte for byte, and the host's object untouched. A second pin records what the SDK does with a duplicate tool name (it throws), so a host knows to keep its names clear of the native surface. The served SKILL.md's Build section told the model to call `describe_object` before changing an object even when the grant was metadata:write alone, where the data gate registers no such tool. The sentence now follows the same predicate as the data gate (`data:read`); without it the section points at the authoring tools' own verify step instead and names no native tool. The surface guard gains the case: the Build section rendered for a grant names no native tool that grant does not register. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGufziVp7Y4ARxehhrEJbM
1 parent cf3ec26 commit 7fb21ed

7 files changed

Lines changed: 148 additions & 17 deletions

‎.changeset/26464-mcp-metadata-write-scope.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ Metadata-protocol: none
1111
A developer driving an MCP client (Claude Code, Claude Desktop, …) against a hosted environment could read and run it over `/api/v1/mcp` but never build in it: `handleHttpRequest` served the data and action tools only, and no OAuth scope existed under which a token could even ask for authoring. This change adds the scope and its first reader.
1212

1313
- **`@objectstack/spec` — `MCP_OAUTH_SCOPE_METADATA_WRITE` (`metadata:write`)** joins `data:read`, `data:write` and `actions:execute` in `MCP_OAUTH_SCOPES`, so the authorization server advertises it and the runtime's up-front scope check admits it. It is a tool-surface grant, not a data grant: `scopesToAgentPermissionSets` maps it alone to the no-object-access floor, and beside a data scope it changes nothing about the data ceiling. **`MCP_OAUTH_SCOPE_CONSENT_COPY`** declares the one-sentence consent copy for every scope (the three existing sentences the Console already renders, plus the new one), keyed by `McpOauthScope` so a scope cannot be added without its sentence.
14-
- **`@objectstack/mcp` — `handleHttpRequest(request, { authoring })`** accepts an optional `McpAuthoringInjection`: a host's `McpAuthoringToolRegistry` plus the token principal and the environment. Its tools appear in `tools/list` only when the request's `grantedScopes` names `metadata:write`; without the scope they are not registered, so `tools/call` on one is answered exactly as `create_record` is without `data:write` (the SDK's unknown-tool error, one mechanism). The undefined grant of an API key or a session is served no authoring tool: authoring follows the token's consent. Every call into the registry receives a fresh `McpAuthoringToolContext` — `door: 'mcp'` (`MCP_TOOL_DOOR`), `actor.id`, `principal`, `environmentId` — built from the host's inputs alone, so no request header can claim another door, user or environment. The runtime stays door-agnostic: which tools the host injects is the host's business.
15-
- **`renderSkillMarkdown({ grantedScopes, authoringTools })`** renders a Build section naming the injected tools, with the build loop (create → verify → draft, a person publishes in the UI) and the switched frontmatter description `OBJECTSTACK_SKILL_DESCRIPTION_WITH_AUTHORING`, only under the same grant; every other render is byte-identical to before.
14+
- **`@objectstack/mcp` — `handleHttpRequest(request, { authoring })`** accepts an optional `McpAuthoringInjection`: a host's `McpAuthoringToolRegistry` plus the token principal and the environment. Its tools appear in `tools/list` only when the request's `grantedScopes` names `metadata:write`; without the scope they are not registered, so `tools/call` on one is answered exactly as `create_record` is without `data:write` (the SDK's unknown-tool error, one mechanism). The undefined grant of an API key or a session is served no authoring tool: authoring follows the token's consent. Every call into the registry receives a fresh `McpAuthoringToolContext` — `door: 'mcp'` (`MCP_TOOL_DOOR`), `actor.id`, `principal`, `environmentId` — built from the host's inputs alone, so no request header can claim another door, user or environment; the principal (and its `scopes`) is a frozen per-call copy, so a tool that writes into its context taints neither the next call nor the host's object. Tool names are the host's to keep unique: the SDK throws on a duplicate registration (a native name or a repeat within the registry) and the request fails with it. The runtime stays door-agnostic: which tools the host injects is the host's business.
15+
- **`renderSkillMarkdown({ grantedScopes, authoringTools })`** renders a Build section naming the injected tools, with the build loop (create → verify → draft, a person publishes in the UI) and the switched frontmatter description `OBJECTSTACK_SKILL_DESCRIPTION_WITH_AUTHORING`, only under the same grant; it teaches `describe_object` before a change only when the grant also carries `data:read`, the scope that registers it; every other render is byte-identical to before.
1616
- **Unchanged.** The stdio transport's `bridgeTools` still calls its registry with one argument; the data and action tools register, refuse and answer as before. New exports: `MCP_TOOL_DOOR`, `grantsAuthoringTools`, and the types `McpToolDoor`, `McpTokenPrincipal`, `McpAuthoringToolContext`, `McpAuthoringToolRegistry`, `McpAuthoringInjection`.
1717

1818
What to do: nothing for an existing host. A host that wants the build door passes `authoring` to `handleHttpRequest` and `grantedScopes` / `authoringTools` to `renderSkill`.

‎packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,66 @@ describe('[#26464] authoring tools over HTTP — door provenance in the tool con
411411
expect(PRINCIPAL.userId).toBe('usr_dev_1');
412412
});
413413

414+
it('the principal in the context is a frozen per-call copy: a handler can taint neither the next call nor the host object', async () => {
415+
const attempts: unknown[] = [];
416+
const tainting: McpAuthoringToolRegistry = {
417+
getAll: () => AUTHORING_TOOLS,
418+
async execute(toolCall, context) {
419+
registry.calls.push({ toolCall, context });
420+
// Each write is what a hostile or buggy tool would try; every one must be refused,
421+
// not silently absorbed into state a later call could read.
422+
for (const write of [
423+
() => { (context.principal as any).userId = 'usr_admin_evil'; },
424+
() => { (context.principal as any).organizationId = 'org_victim'; },
425+
() => { (context.principal.scopes as any).push(MCP_OAUTH_SCOPE_DATA_WRITE); },
426+
() => { (context.actor as any).id = 'usr_admin_evil'; },
427+
]) {
428+
try { write(); attempts.push('absorbed'); } catch (e) { attempts.push(e); }
429+
}
430+
return {
431+
type: 'tool-result',
432+
toolCallId: toolCall.toolCallId,
433+
toolName: toolCall.toolName,
434+
output: { type: 'text', value: 'ok' },
435+
} as ToolExecutionResult;
436+
},
437+
};
438+
const o: CallOptions = { grantedScopes: GRANT, registry: tainting }; // the harness injects PRINCIPAL + ENVIRONMENT_ID
439+
await callTool(runtime, bridge, 'verify_build', {}, o);
440+
await callTool(runtime, bridge, 'verify_build', {}, o);
441+
expect(registry.calls).toHaveLength(2);
442+
// Every write was refused (strict-mode TypeError on a frozen object), none absorbed.
443+
expect(attempts).toHaveLength(8);
444+
for (const a of attempts) expect(a).toBeInstanceOf(TypeError);
445+
const [first, second] = registry.calls.map((c) => c.context);
446+
expect(Object.isFrozen(first.principal)).toBe(true);
447+
expect(Object.isFrozen(first.principal.scopes)).toBe(true);
448+
expect(Object.isFrozen(first.actor)).toBe(true);
449+
// A copy, not the host's reference — and a copy per call, not one shared copy.
450+
expect(first.principal).not.toBe(PRINCIPAL);
451+
expect(second.principal).not.toBe(first.principal);
452+
expect(first.principal.scopes).not.toBe(PRINCIPAL.scopes);
453+
// The second call still sees the host's principal, byte for byte …
454+
expect(second.principal).toEqual(PRINCIPAL);
455+
expect(second.actor).toEqual({ id: PRINCIPAL.userId });
456+
// … and the host's own object is untouched.
457+
expect(PRINCIPAL).toEqual({
458+
userId: 'usr_dev_1',
459+
organizationId: 'org_acme',
460+
clientId: 'claude-code',
461+
scopes: [MCP_OAUTH_SCOPE_METADATA_WRITE],
462+
});
463+
});
464+
465+
it('a tool name already on the server is refused at registration (the SDK throws), so a host keeps its names unique', () => {
466+
const server = new McpServer({ name: 'x', version: '0' });
467+
const first = registerAuthoringTools(server, { registry, principal: PRINCIPAL }, GRANT);
468+
expect(first).toEqual(AUTHORING_TOOLS.map((t) => t.name));
469+
// The same names a second time — as a registry that duplicates one of its own, or one
470+
// that reuses a native name, would — is thrown back by the SDK, not merged or shadowed.
471+
expect(() => registerAuthoringTools(server, { registry, principal: PRINCIPAL }, GRANT)).toThrow(/already registered/i);
472+
});
473+
414474
it('an environment the host did not name is absent from the context, not undefined-valued', async () => {
415475
const server = new McpServer({ name: 'x', version: '0' });
416476
const names = registerAuthoringTools(server, { registry, principal: PRINCIPAL }, GRANT);

‎packages/mcp/src/mcp-server-runtime.ts‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { MCP_TOOL_DOOR } from './types.js';
1010
import type {
1111
McpAuthoringInjection,
1212
McpAuthoringToolContext,
13+
McpTokenPrincipal,
1314
ToolRegistry,
1415
ToolExecutionResult,
1516
} from './types.js';
@@ -1130,7 +1131,16 @@ function registerToolFromDefinition(
11301131
* so no header can reach the context — a client sending `x-door`,
11311132
* `x-environment-id` or any other spelling changes nothing a tool sees. The
11321133
* host's access check and tier mapping read the door from the context; that is
1133-
* the whole reason it is stamped on the server side of the transport.
1134+
* the whole reason it is stamped on the server side of the transport. The
1135+
* principal (and its `scopes` array) is copied and frozen per call, so a tool
1136+
* that writes into its context taints neither the next call nor the host's
1137+
* object.
1138+
*
1139+
* Tool names are the host's to keep unique: the SDK refuses a second
1140+
* registration under a name already on the server — a native name such as
1141+
* `describe_object`, or a duplicate within the registry — by throwing, and
1142+
* the request fails with it. A host checks its names against the native
1143+
* surface before injecting.
11341144
*
11351145
* ## What stays the host's
11361146
*
@@ -1154,12 +1164,22 @@ export function registerAuthoringTools(
11541164
if (!grantsAuthoringTools(grantedScopes)) return registered;
11551165

11561166
const { registry, principal, environmentId } = authoring;
1157-
const provenance = (): McpAuthoringToolContext => ({
1158-
door: MCP_TOOL_DOOR,
1159-
actor: { id: principal.userId },
1160-
principal,
1161-
...(environmentId !== undefined ? { environmentId } : {}),
1162-
});
1167+
// Each call gets its own FROZEN copy of the principal (and of its one array,
1168+
// `scopes`): a tool that writes into its context taints neither the next
1169+
// call in this request nor the host's own object. The context object itself
1170+
// is fresh per call for the same reason.
1171+
const provenance = (): McpAuthoringToolContext => {
1172+
const frozenPrincipal: McpTokenPrincipal = Object.freeze({
1173+
...principal,
1174+
...(principal.scopes !== undefined ? { scopes: Object.freeze([...principal.scopes]) } : {}),
1175+
});
1176+
return {
1177+
door: MCP_TOOL_DOOR,
1178+
actor: Object.freeze({ id: frozenPrincipal.userId }),
1179+
principal: frozenPrincipal,
1180+
...(environmentId !== undefined ? { environmentId } : {}),
1181+
};
1182+
};
11631183

11641184
for (const tool of registry.getAll()) {
11651185
registerToolFromDefinition(server, tool, (call) => registry.execute(call, provenance()), logger);

‎packages/mcp/src/skill-md-surface-guard.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,18 @@ describe('SKILL.md ↔ native tool surface drift guard', () => {
164164
expect(mdWithoutScope).not.toContain(t.name);
165165
}
166166
});
167+
168+
it('[#26464] the Build section names no native tool the same grant does not register (metadata:write alone has no data tool)', async () => {
169+
const registry = makeAuthoringRegistry();
170+
const native = await listRegisteredToolNames({ grantedScopes: EVERY_SCOPE });
171+
expect(native.length).toBeGreaterThanOrEqual(9);
172+
const metadataOnly = await listRegisteredToolNames({ grantedScopes: [MCP_OAUTH_SCOPE_METADATA_WRITE], registry });
173+
expect(metadataOnly.sort()).toEqual(AUTHORING_TOOLS.map((t) => t.name).sort());
174+
175+
const md = renderSkillMarkdown({ grantedScopes: [MCP_OAUTH_SCOPE_METADATA_WRITE], authoringTools: registry.getAll() });
176+
const section = md.slice(md.indexOf('## Build — authoring tools'), md.indexOf('## Conventions & gotchas'));
177+
expect(section.length).toBeGreaterThan(0);
178+
const taughtButUnregistered = native.filter((name) => section.includes(name) && !metadataOnly.includes(name));
179+
expect(taughtButUnregistered, `the Build section teaches a tool this grant does not register: ${taughtButUnregistered.join(', ')}`).toEqual([]);
180+
});
167181
});

‎packages/mcp/src/skill-md.authoring.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,25 @@ describe('[#26464] renderSkillMarkdown — the authoring section follows the gra
8585
expect(OBJECTSTACK_SKILL_DESCRIPTION_WITH_AUTHORING).not.toContain('\n');
8686
});
8787

88+
it('metadata:write alone does not teach describe_object in the Build section — that grant registers no data tool', () => {
89+
const md = renderSkillMarkdown({ grantedScopes: [MCP_OAUTH_SCOPE_METADATA_WRITE], authoringTools: TOOLS });
90+
const section = md.slice(md.indexOf(SECTION), md.indexOf('## Conventions & gotchas'));
91+
expect(section).not.toContain('describe_object');
92+
expect(section).not.toContain('list_objects');
93+
expect(section).toContain('no `data:read`');
94+
expect(section).toMatch(/before you change an object/);
95+
});
96+
97+
it('metadata:write beside data:read teaches describe_object before a change — the same grant registers it', () => {
98+
const md = renderSkillMarkdown({
99+
grantedScopes: [MCP_OAUTH_SCOPE_DATA_READ, MCP_OAUTH_SCOPE_METADATA_WRITE],
100+
authoringTools: TOOLS,
101+
});
102+
const section = md.slice(md.indexOf(SECTION), md.indexOf('## Conventions & gotchas'));
103+
expect(section).toContain('`describe_object` on an object');
104+
expect(section).not.toContain('no `data:read`');
105+
});
106+
88107
it('the section is placed after the native tools and before the conventions, with the data content untouched', () => {
89108
const md = renderSkillMarkdown({ grantedScopes: [MCP_OAUTH_SCOPE_METADATA_WRITE], authoringTools: TOOLS });
90109
const tools = md.indexOf('## Tools');

‎packages/mcp/src/skill-md.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
*/
3232

3333
import type { AIToolDefinition } from '@objectstack/spec/contracts';
34-
import { MCP_OAUTH_SCOPE_METADATA_WRITE } from '@objectstack/spec/ai';
34+
import { MCP_OAUTH_SCOPE_DATA_READ, MCP_OAUTH_SCOPE_METADATA_WRITE } from '@objectstack/spec/ai';
3535
import { grantsAuthoringTools } from './mcp-http-tools.js';
3636

3737
/** Skill identity (mirrors the `SKILL.md` YAML frontmatter). */
@@ -108,8 +108,17 @@ function oneLine(text: string): string {
108108
* UI — from the same source as the data loop, so the client model learns both
109109
* the same way. Tool names come from the host; none is hard-coded here.
110110
*/
111-
function renderAuthoringSection(tools: ReadonlyArray<Pick<AIToolDefinition, 'name' | 'description'>>): string {
111+
function renderAuthoringSection(
112+
tools: ReadonlyArray<Pick<AIToolDefinition, 'name' | 'description'>>,
113+
describeObjectRegistered: boolean,
114+
): string {
112115
const bullets = tools.map((t) => `- **${t.name}** — ${oneLine(t.description)}`).join('\n');
116+
// Name `describe_object` only when the same grant registers it (the data
117+
// gate: `data:read`). A metadata:write-only connection has no data tool, so
118+
// teaching one there would be teaching a tool the server refuses.
119+
const discover = describeObjectRegistered
120+
? 'Discover first here too — `describe_object` on an object\nbefore you change it.'
121+
: 'Your grant carries no `data:read`, so the data tools are not on this\nconnection: read the current state through the authoring tools\' own verify\nstep before you change an object.';
113122
return `
114123
## Build — authoring tools (\`${MCP_OAUTH_SCOPE_METADATA_WRITE}\`)
115124
@@ -125,8 +134,7 @@ broken.
125134
The build loop is **create → verify → draft**: create or change the metadata,
126135
verify it (the environment validates what you wrote and reports what it
127136
refuses, so read the result before the next step), and leave it as a draft for
128-
a person to publish. Discover first here too — \`describe_object\` on an object
129-
before you change it.
137+
a person to publish. ${discover}
130138
131139
${bullets}
132140
`;
@@ -150,6 +158,8 @@ export function renderSkillMarkdown(options: RenderSkillOptions = {}): string {
150158
? options.authoringTools
151159
: undefined;
152160
const description = authoringTools ? OBJECTSTACK_SKILL_DESCRIPTION_WITH_AUTHORING : OBJECTSTACK_SKILL_DESCRIPTION;
161+
// Mirrors the native data gate in `wireBridgeTools` (`!scopes || scopes.includes(data:read)`).
162+
const describeObjectRegistered = !options.grantedScopes || options.grantedScopes.includes(MCP_OAUTH_SCOPE_DATA_READ);
153163

154164
return `---
155165
name: ${OBJECTSTACK_SKILL_NAME}
@@ -264,7 +274,7 @@ create/update payload.
264274
screen, which you fill with another \`resume_run\`. Only the user whose call
265275
started the run can resume it, under the same gates as \`run_action\`
266276
(including \`confirm: true\` for an action flagged \`requiresConfirmation\`).
267-
${authoringTools ? renderAuthoringSection(authoringTools) : ''}
277+
${authoringTools ? renderAuthoringSection(authoringTools, describeObjectRegistered) : ''}
268278
## Conventions & gotchas
269279
270280
- **Permissions/RLS apply to every call.** Fewer rows than expected, or a

‎packages/mcp/src/types.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export interface McpAuthoringToolContext extends ToolExecutionContext {
7171
door: McpToolDoor;
7272
/** The token's user, as the actor a `ToolExecutionContext` reader expects (RLS on, as that user). */
7373
actor: { id: string };
74-
/** The token principal, verbatim from the host. */
74+
/** The token principal: a frozen per-call copy of what the host passed, so a tool cannot taint the next call. */
7575
principal: McpTokenPrincipal;
7676
/** The environment the request was dispatched for, verbatim from the host. */
7777
environmentId?: string;
@@ -98,9 +98,17 @@ export interface McpAuthoringToolRegistry {
9898
* provenance of every call into it.
9999
*/
100100
export interface McpAuthoringInjection {
101-
/** The authoring registry — which tools it holds is the host's business. */
101+
/**
102+
* The authoring registry — which tools it holds is the host's business. Its
103+
* names must not collide with the native surface or with each other: the SDK
104+
* throws on a duplicate registration and the request fails with it.
105+
*/
102106
registry: McpAuthoringToolRegistry;
103-
/** The token principal the host resolved for this request. */
107+
/**
108+
* The token principal the host resolved for this request — from the SAME
109+
* token whose scopes the host passes as `toolOptions.grantedScopes`, since
110+
* that list is what admits the registry. Copied and frozen per call.
111+
*/
104112
principal: McpTokenPrincipal;
105113
/** The environment the host dispatched this request for. */
106114
environmentId?: string;

0 commit comments

Comments
 (0)