From 205610d2005b18f0bcca49fa9c11444053410361 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:33:24 +0200 Subject: [PATCH] feat(mcp): distribution-channel client marker in MCP telemetry (#706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mcp): distribution-channel client marker in MCP telemetry Record an optional client marker on mcp.tool_called, mcp.tools_list_called and mcp.resource_read events so per-channel adoption (e.g. the OpenClaw skill) is measurable in event_log (180-day TTL). - Server reads X-Gnubok-Client header, falling back to a ?client= query param on the endpoint URL. Sanitized ([A-Za-z0-9._-]{1,64}, lowercased), telemetry-only — same trust level as Mcp-Session-Id, never auth. - The query param works with the already-published gnubok-mcp 1.0.1 via GNUBOK_URL, so no npm release is required to start measuring. - Bridge 1.1.0 additionally forwards GNUBOK_CLIENT as X-Gnubok-Client. OAuth-path attribution via DCR client_name is a possible follow-up — DCR is stateless today, so client_name isn't recoverable at token time. Co-Authored-By: Claude Fable 5 * fix(mcp): address PR #706 compliance findings - ropa.yaml: declare the distribution-channel marker in the mcp.telemetry processing activity (GDPR Art. 30 — RoPA was drifting from actual flow) - bridge: mirror the server's allow-list on GNUBOK_CLIENT so an invalid value degrades to no header instead of fetch() rejecting every request - lib/events/types.ts: annotate client as client-supplied/telemetry-only - test: pin that the allow-list runs on the percent-decoded ?client= value Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .compliance/ropa.yaml | 15 +-- .../mcp-server/__tests__/telemetry.test.ts | 100 +++++++++++++++++- extensions/general/mcp-server/server.ts | 17 +++ lib/events/types.ts | 4 + packages/gnubok-mcp/README.md | 1 + packages/gnubok-mcp/index.mjs | 10 ++ packages/gnubok-mcp/package.json | 2 +- 7 files changed, 139 insertions(+), 10 deletions(-) diff --git a/.compliance/ropa.yaml b/.compliance/ropa.yaml index a43ef819..6f2b8c45 100644 --- a/.compliance/ropa.yaml +++ b/.compliance/ropa.yaml @@ -282,12 +282,15 @@ processing_activities: name: MCP/agent-telemetri i event_log purpose: >- Varje MCP-verktygsanrop loggar metadata (verktygsnamn, felkod, - felmeddelande max 500 tecken, latens, aktör, session) och varje - skill-laddning loggar slug/tier till event_log. Syftet är - tillförlitlighetsanalys av agentgränssnittet: felfrekvens per verktyg, - korrelation mellan laddade skills och efterföljande fel, samt agenters - egenrapporterade feedback (agent.feedback). Inga verktygsargument - eller verktygsresultat persisteras. + felmeddelande max 500 tecken, latens, aktör, session, frivillig + distributionskanal-markör — t.ex. 'openclaw' via X-Gnubok-Client-header + eller ?client=-parameter, sanerad mot allow-list och aldrig använd för + auktorisation) och varje skill-laddning loggar slug/tier till event_log. + Syftet är tillförlitlighetsanalys av agentgränssnittet: felfrekvens per + verktyg, adoption per distributionskanal, korrelation mellan laddade + skills och efterföljande fel, samt agenters egenrapporterade feedback + (agent.feedback). Inga verktygsargument eller verktygsresultat + persisteras. lawful_basis: art_6_1_f # legitimate interest (service reliability/improvement) special_category_basis: null controller: gnubok-tenant diff --git a/extensions/general/mcp-server/__tests__/telemetry.test.ts b/extensions/general/mcp-server/__tests__/telemetry.test.ts index 0c2d16d4..c2ed4aea 100644 --- a/extensions/general/mcp-server/__tests__/telemetry.test.ts +++ b/extensions/general/mcp-server/__tests__/telemetry.test.ts @@ -78,10 +78,15 @@ vi.mock('@/lib/auth/api-keys', async (importOriginal) => { import { handleMcpRequest } from '../server' -function mcpRequest(method: string, params?: Record, id: number | string = 1): Request { - return new Request('http://localhost:3000/api/extensions/ext/mcp-server/mcp', { +function mcpRequest( + method: string, + params?: Record, + id: number | string = 1, + opts: { url?: string; headers?: Record } = {} +): Request { + return new Request(opts.url ?? 'http://localhost:3000/api/extensions/ext/mcp-server/mcp', { method: 'POST', - headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token' }, + headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token', ...opts.headers }, body: JSON.stringify({ jsonrpc: '2.0', id, method, params }), }) } @@ -101,6 +106,7 @@ interface ToolCalledPayload { requestId: string | number | null userId: string companyId: string + client: string | null } interface ToolsListCalledPayload { @@ -112,6 +118,7 @@ interface ToolsListCalledPayload { requestId: string | number | null userId: string companyId: string + client: string | null } interface ResourceReadPayload { @@ -126,6 +133,7 @@ interface ResourceReadPayload { requestId: string | number | null userId: string companyId: string + client: string | null } async function captureNextToolCalledEvent(): Promise { @@ -280,6 +288,92 @@ describe('mcp.tool_called telemetry', () => { }) }) +describe('client marker telemetry', () => { + beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + }) + + it('records a lowercased X-Gnubok-Client header on mcp.tool_called', async () => { + const eventPromise = captureNextToolCalledEvent() + + await handleMcpRequest( + mcpRequest('tools/call', { name: 'gnubok_list_skills', arguments: {} }, 1, { + headers: { 'X-Gnubok-Client': 'OpenClaw' }, + }) + ) + + const event = await eventPromise + expect(event.client).toBe('openclaw') + }) + + it('falls back to the ?client= query param when no header is present', async () => { + const eventPromise = captureNextToolsListEvent() + + await handleMcpRequest( + mcpRequest('tools/list', undefined, 1, { + url: 'http://localhost:3000/api/extensions/ext/mcp-server/mcp?client=openclaw', + }) + ) + + const event = await eventPromise + expect(event.client).toBe('openclaw') + }) + + it('prefers the header over the query param when both are present', async () => { + const eventPromise = captureNextToolCalledEvent() + + await handleMcpRequest( + mcpRequest('tools/call', { name: 'gnubok_list_skills', arguments: {} }, 1, { + url: 'http://localhost:3000/api/extensions/ext/mcp-server/mcp?client=other', + headers: { 'X-Gnubok-Client': 'openclaw' }, + }) + ) + + const event = await eventPromise + expect(event.client).toBe('openclaw') + }) + + it('runs the allow-list on the percent-decoded query param value', async () => { + const eventPromise = captureNextToolCalledEvent() + + // URLSearchParams.get() percent-decodes before our regex runs, so encoded + // payloads can't smuggle disallowed characters past the allow-list. + await handleMcpRequest( + mcpRequest('tools/call', { name: 'gnubok_list_skills', arguments: {} }, 1, { + url: 'http://localhost:3000/api/extensions/ext/mcp-server/mcp?client=open%63law', + }) + ) + + const event = await eventPromise + expect(event.client).toBe('openclaw') + }) + + it('drops markers that fail the charset/length sanitation and reports null', async () => { + const eventPromise = captureNextToolCalledEvent() + + await handleMcpRequest( + mcpRequest('tools/call', { name: 'gnubok_list_skills', arguments: {} }, 1, { + headers: { 'X-Gnubok-Client': 'bad client!