mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
Renzo Beux
937e32924d
Found in review of the branch. Each one is a case where the host told the model or the admin something it did not enforce. Advertised schemas are now enforced. buildToolInputSchema built a loose object that checked only which keys were present, so a tool advertising an integer, an enum and additionalProperties: false accepted a string, an unlisted value and an extra property. .meta() still advertises the declaration verbatim, and the schema underneath it is now built from that same declaration, so the two cannot disagree. Anything the builder cannot enforce (oneOf, unknown formats, non-primitive enum members) is refused by parseManifest rather than advertised, and the failure lands at install time where an admin sees it. Writing the test for that turned up a live bug: z.custom() cannot be represented in JSON Schema, so building enums that way would have thrown inside tools/list for the whole session. Enums and consts are real literal types now. readOnlyHint classification fails closed. The side-effect list spelled out db:write: and missed db:create:trips entirely, letting a plugin that can create trips advertise a tool as read-only and idempotent, which is what clients gate auto-approval on. It is an allowlist of read-only grants now, and mcp-tool-annotations.test.ts walks the whole catalog so a new permission fails a test rather than producing a wrong hint. The result cap covers the whole result. It bounded each text block, so a hundred blocks cost a hundred times the limit: 6.5M characters in a probe, now 65K. Large objects are serialised with a budget instead of stringified whole and then sliced, and the block count is capped. The admin tool list was dead code. The registry preview carries only widget and tripPage, so the section added for it never rendered, and the UI hid the description behind the title when both existed. The preview carries sanitised tools now (previewManifest is exported and tested, which is how the gap survived), and both fields render, description always. plugins:use is out of the DCR default. Adding it to ALL_SCOPES meant a registration that names no scopes got it and had it pre-selected at consent. Defaults come from DEFAULT_CLIENT_SCOPES now; asking for it by name still works. The public ManifestCapabilities gained mcpTools, so a typed manifest can express what the runtime already validates. MCPANN-001..005, MCPENF-001..010, MCPPREV-001..005, MCPTOOLS-020..022, SDKP-015b/c.
66 lines
2.7 KiB
TypeScript
66 lines
2.7 KiB
TypeScript
/**
|
|
* Grant classification behind readOnlyHint — MCPANN-001 to MCPANN-005.
|
|
*
|
|
* readOnlyHint is what MCP clients gate auto-approval on, so getting this wrong
|
|
* means a tool that writes runs without asking. The classification therefore
|
|
* fails closed, and this file pins every permission in the catalog so a new one
|
|
* shows up here rather than as a wrong hint in production.
|
|
*
|
|
* An earlier denylist spelled out `db:write:` and missed `db:create:trips`.
|
|
*/
|
|
import { KNOWN_PERMISSIONS } from '../../../src/nest/plugins/protocol/envelope';
|
|
import { clampToolAnnotations } from '../../../src/nest/plugins/mcp-tool-schema';
|
|
|
|
import { describe, expect, it } from 'vitest';
|
|
|
|
/** Holding only this grant, may a tool still call itself read-only? */
|
|
const readOnly = (grant: string): boolean =>
|
|
clampToolAnnotations({ readOnlyHint: true }, new Set([grant])).readOnlyHint;
|
|
|
|
/**
|
|
* The permissions that leave a tool read-only. Everything else in the catalog
|
|
* must cost the hint. Spelled out rather than derived, so this file disagrees
|
|
* out loud with the implementation when either one moves.
|
|
*/
|
|
const EXPECTED_READ_ONLY = new Set<string>([
|
|
'db:own',
|
|
'events:subscribe',
|
|
'geolocation:read',
|
|
'rates:read',
|
|
'weather:read',
|
|
...KNOWN_PERMISSIONS.filter((p) => p.startsWith('db:read:')),
|
|
...KNOWN_PERMISSIONS.filter((p) => p.startsWith('hook:')),
|
|
]);
|
|
|
|
describe('grant classification', () => {
|
|
it('MCPANN-001: every permission in the catalog is classified deliberately', () => {
|
|
const wrong = KNOWN_PERMISSIONS.filter((p) => readOnly(p) !== EXPECTED_READ_ONLY.has(p));
|
|
expect(wrong).toEqual([]);
|
|
});
|
|
|
|
it('MCPANN-002: db:create:trips costs the read-only hint', () => {
|
|
// The specific miss that motivated the flip to an allowlist.
|
|
expect(KNOWN_PERMISSIONS).toContain('db:create:trips');
|
|
expect(readOnly('db:create:trips')).toBe(false);
|
|
});
|
|
|
|
it('MCPANN-003: every write and create grant costs the hint', () => {
|
|
const mutating = KNOWN_PERMISSIONS.filter((p) => p.startsWith('db:write:') || p.startsWith('db:create:'));
|
|
expect(mutating.length).toBeGreaterThan(0);
|
|
for (const p of mutating) expect(readOnly(p), p).toBe(false);
|
|
});
|
|
|
|
it('MCPANN-004: an unknown permission family fails closed', () => {
|
|
// The point of the allowlist: something nobody classified must not be
|
|
// silently treated as harmless.
|
|
expect(readOnly('db:destroy:everything')).toBe(false);
|
|
expect(readOnly('some:future:grant')).toBe(false);
|
|
});
|
|
|
|
it('MCPANN-005: a per-host egress grant is a side effect and forces openWorldHint', () => {
|
|
const out = clampToolAnnotations({ readOnlyHint: true, openWorldHint: false }, new Set(['http:outbound:api.example.com']));
|
|
expect(out.readOnlyHint).toBe(false);
|
|
expect(out.openWorldHint).toBe(true);
|
|
});
|
|
});
|