From 6c0f47acad18e93c367ee0f208aedcdbca5925d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 29 Jul 2026 13:38:41 +0200 Subject: [PATCH] test(01-01): pin MCP read scope boundaries - cover health read and wildcard positive capability paths - reject unrelated scopes before server registration and database reads --- src/app/mcp/__tests__/route.test.ts | 63 +++++++++- src/lib/mcp/__tests__/auth.test.ts | 41 +++++-- src/lib/mcp/__tests__/scopes.test.ts | 22 ++++ .../bearer-scope-enforcement.test.ts | 111 ++++++++++++++++++ 4 files changed, 229 insertions(+), 8 deletions(-) diff --git a/src/app/mcp/__tests__/route.test.ts b/src/app/mcp/__tests__/route.test.ts index 738817f6a..d3659601d 100644 --- a/src/app/mcp/__tests__/route.test.ts +++ b/src/app/mcp/__tests__/route.test.ts @@ -21,6 +21,11 @@ */ import { describe, it, expect, vi, beforeEach } from "vitest"; +const { createMcpServerSpy, executeCoachTool } = vi.hoisted(() => ({ + createMcpServerSpy: vi.fn(), + executeCoachTool: vi.fn(async () => ({ present: false, reason: "no_data" })), +})); + // The surface fails closed without a pinned origin (M1); pin it for the suite. process.env.APP_URL = "http://localhost"; @@ -43,10 +48,21 @@ vi.mock("@/lib/rate-limit", () => ({ // The factory pulls the Coach retrieval layer through `tools.ts`; stub the // heavy executor + inventory so importing the route never reaches a DB. The // list/reject paths never execute a tool, so the stubs are never called. -vi.mock("@/lib/ai/coach/tools/executor", () => ({ executeCoachTool: vi.fn() })); +vi.mock("@/lib/ai/coach/tools/executor", () => ({ executeCoachTool })); vi.mock("@/lib/ai/coach/tools/inventory", () => ({ buildCoachDataInventory: vi.fn(), })); +vi.mock("@/lib/mcp/server", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + createMcpServer: (...args: Parameters) => { + createMcpServerSpy(...args); + return actual.createMcpServer(...args); + }, + }; +}); // A cookie path must NEVER be reached from the MCP wire. Mock both cookie // sources so the test can assert they are never consulted. const getSession = vi.fn(); @@ -123,6 +139,12 @@ function postRpc(body: unknown, headers: Record = {}): Request { } const TOOLS_LIST = { jsonrpc: "2.0", id: 1, method: "tools/list" }; +const GET_LABS = { + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { name: "get_labs", arguments: {} }, +}; beforeEach(() => { vi.clearAllMocks(); @@ -136,6 +158,7 @@ beforeEach(() => { remaining: 119, resetAt: Date.now() + 60_000, }); + executeCoachTool.mockResolvedValue({ present: false, reason: "no_data" }); }); describe("/mcp — happy path", () => { @@ -177,6 +200,22 @@ describe("/mcp — happy path", () => { // Tools + resources are advertised; no admin capability exists. expect(body.result.capabilities.tools).toBeDefined(); }); + + it.each([ + ["health:read", ["health:read"]], + ["wildcard", ["*"]], + ])("lets a %s token reach a real health reader", async (_label, scopes) => { + validToken(scopes); + + const res = await POST(postRpc(GET_LABS)); + + expect(res.status).toBe(200); + expect(executeCoachTool).toHaveBeenCalledWith({ + userId: "user-1", + name: "get_labs", + rawArguments: "{}", + }); + }); }); describe("/mcp — Origin gate (DNS-rebinding defense, spec PR #1439)", () => { @@ -269,7 +308,29 @@ describe("/mcp — authentication", () => { expect(JSON.stringify(body)).not.toMatch(/revoked|expired|hlk_/); // A rejected token never reaches the module gate or the transport. expect(isModuleEnabled).not.toHaveBeenCalled(); + expect(createMcpServerSpy).not.toHaveBeenCalled(); }); + + it.each([ + ["medication ingest", ["medication:ingest"]], + ["notifications", ["notifications:send"]], + ["FHIR", ["fhir:read"]], + ["write-only", ["health:write"]], + ["unrelated", ["profile:read"]], + ])( + "denies a valid %s token before server registration or health reads", + async (_label, scopes) => { + validToken(scopes); + + const res = await POST(postRpc(GET_LABS)); + + expect([401, 403]).toContain(res.status); + expect(createMcpServerSpy).not.toHaveBeenCalled(); + expect(executeCoachTool).not.toHaveBeenCalled(); + expect(checkMcpRateLimit).not.toHaveBeenCalled(); + expect(isModuleEnabled).not.toHaveBeenCalled(); + }, + ); }); describe("/mcp — module gate (off by default)", () => { diff --git a/src/lib/mcp/__tests__/auth.test.ts b/src/lib/mcp/__tests__/auth.test.ts index a1164f24f..6088dfacd 100644 --- a/src/lib/mcp/__tests__/auth.test.ts +++ b/src/lib/mcp/__tests__/auth.test.ts @@ -14,7 +14,7 @@ beforeEach(() => { }); describe("resolveMcpAuthContext", () => { - it("binds the session to : (REQ-SEC-11) via the canonical Bearer path", async () => { + it("binds a health:read session to : via the canonical Bearer path", async () => { vi.mocked(resolveBearerToken).mockResolvedValue({ user: FAKE_USER, tokenId: "token-9", @@ -24,11 +24,6 @@ describe("resolveMcpAuthContext", () => { const ctx = await resolveMcpAuthContext(" hlk_abc "); - // Trimmed token resolved through the shared validator (no requiredPermission for reads). - // `any-valid-token` is the one deliberate fail-open posture in the tree: - // `/mcp` authenticates here and authorises downstream (audience binding + - // `tokenAllowsWrite`). Asserted explicitly so a silent switch to another - // posture — in either direction — shows up as a failing test. expect(resolveBearerToken).toHaveBeenCalledWith("hlk_abc", { kind: "any-valid-token", }); @@ -39,7 +34,19 @@ describe("resolveMcpAuthContext", () => { expect(ctx.canWrite).toBe(false); }); - it("grants write capability only when the token carries health:write", async () => { + it("admits wildcard read capability and preserves wildcard write capability", async () => { + vi.mocked(resolveBearerToken).mockResolvedValue({ + user: FAKE_USER, + tokenId: "token-wild", + permissions: ["*"], + expiresAt: new Date(), + }); + const ctx = await resolveMcpAuthContext("hlk_wild"); + expect(ctx.canRead).toBe(true); + expect(ctx.canWrite).toBe(true); + }); + + it("does not infer read capability from health:write", async () => { vi.mocked(resolveBearerToken).mockResolvedValue({ user: FAKE_USER, tokenId: "token-w", @@ -47,9 +54,29 @@ describe("resolveMcpAuthContext", () => { expiresAt: new Date(), }); const ctx = await resolveMcpAuthContext("hlk_xyz"); + expect(ctx.canRead).toBe(false); expect(ctx.canWrite).toBe(true); }); + it.each([ + ["medication ingest", ["medication:ingest"]], + ["notification delivery", ["notifications:send"]], + ["FHIR read", ["fhir:read"]], + ["unrelated", ["profile:read"]], + ["empty", []], + ])("does not grant MCP reads to a valid %s token", async (_label, scopes) => { + vi.mocked(resolveBearerToken).mockResolvedValue({ + user: FAKE_USER, + tokenId: "token-narrow", + permissions: scopes, + expiresAt: new Date(), + }); + + const ctx = await resolveMcpAuthContext("hlk_narrow"); + + expect(ctx.canRead).toBe(false); + }); + it("rejects an empty token without calling the validator", async () => { await expect(resolveMcpAuthContext(" ")).rejects.toThrow(); expect(resolveBearerToken).not.toHaveBeenCalled(); diff --git a/src/lib/mcp/__tests__/scopes.test.ts b/src/lib/mcp/__tests__/scopes.test.ts index 0a10753c0..8e53d2791 100644 --- a/src/lib/mcp/__tests__/scopes.test.ts +++ b/src/lib/mcp/__tests__/scopes.test.ts @@ -1,12 +1,34 @@ import { describe, it, expect } from "vitest"; import { + tokenAllowsRead, tokenAllowsWrite, SCOPE_HEALTH_READ, SCOPE_HEALTH_WRITE, SCOPE_WILDCARD, } from "../scopes"; +describe("tokenAllowsRead", () => { + it.each([ + ["health:read", [SCOPE_HEALTH_READ]], + ["wildcard", [SCOPE_WILDCARD]], + ["read plus write", [SCOPE_HEALTH_READ, SCOPE_HEALTH_WRITE]], + ])("admits an explicit %s grant", (_label, permissions) => { + expect(tokenAllowsRead(permissions)).toBe(true); + }); + + it.each([ + ["write-only", [SCOPE_HEALTH_WRITE]], + ["medication ingest", ["medication:ingest"]], + ["notifications", ["notifications:send"]], + ["FHIR", ["fhir:read"]], + ["unrelated", ["profile:read"]], + ["empty", []], + ])("refuses a %s scope set", (_label, permissions) => { + expect(tokenAllowsRead(permissions)).toBe(false); + }); +}); + describe("tokenAllowsWrite", () => { it("is false for a read-only token", () => { expect(tokenAllowsWrite([SCOPE_HEALTH_READ])).toBe(false); diff --git a/tests/integration/bearer-scope-enforcement.test.ts b/tests/integration/bearer-scope-enforcement.test.ts index 2b158e1e2..251701285 100644 --- a/tests/integration/bearer-scope-enforcement.test.ts +++ b/tests/integration/bearer-scope-enforcement.test.ts @@ -27,6 +27,8 @@ process.env.API_TOKEN_HMAC_KEY ??= const { hashToken } = await import("@/lib/auth/hmac"); const USER_ID = "user-bearer-scope-test"; +const MCP_RESOURCE = "https://health.example/mcp"; +process.env.APP_URL = "https://health.example"; vi.mock("next/headers", async () => { const { cookieJar, headerJar } = await import("./mock-next-headers"); @@ -63,6 +65,7 @@ beforeEach(async () => { username: "bearer-scope", email: "bearer-scope@example.test", timezone: "UTC", + modulePreferencesJson: { mcp: true }, }, }); }); @@ -82,6 +85,40 @@ async function useToken(permissions: string[], label = "t"): Promise { return raw; } +async function useExpiringToken( + permissions: string[], + label: string, + state: { expiresAt?: Date; revoked?: boolean } = {}, +): Promise { + const raw = await useToken(permissions, label); + await getPrismaClient().apiToken.update({ + where: { tokenHash: hashToken(raw) }, + data: state, + }); + return raw; +} + +function mcpReq( + rawToken: string, + method: "initialize" | "tools/list" | "tools/call", + params?: Record, +): Request { + return new Request(MCP_RESOURCE, { + method: "POST", + headers: { + authorization: `Bearer ${rawToken}`, + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method, + ...(params ? { params } : {}), + }), + }); +} + /** Arm a real cookie session instead of a Bearer token. */ async function useCookieSession(): Promise { const session = await getPrismaClient().session.create({ @@ -274,6 +311,80 @@ describe("B5 — an MCP token is audience-bound to /mcp", () => { }); }); +describe("B5b — MCP health reads require an explicit read grant", () => { + async function postMcp( + rawToken: string, + method: "initialize" | "tools/list" | "tools/call", + params?: Record, + ): Promise { + const { POST } = await import("@/app/mcp/route"); + return POST(mcpReq(rawToken, method, params)); + } + + it.each([ + ["health:read", ["health:read"]], + ["wildcard", ["*"]], + ])("admits %s through capability registration", async (_label, scopes) => { + const raw = await useToken(scopes, `mcp-positive-${_label}`); + + const res = await postMcp(raw, "tools/list"); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { tools: Array<{ name: string }> }; + }; + expect(body.result.tools.map((tool) => tool.name)).toContain("get_labs"); + }); + + it.each([ + ["medication ingest", ["medication:ingest"]], + ["notifications", ["notifications:send"]], + ["FHIR", ["fhir:read"]], + ["write-only", ["health:write"]], + ["unrelated", ["profile:read"]], + ])( + "denies a same-user %s token before a real lab read", + async (_label, scopes) => { + const raw = await useToken(scopes, `mcp-negative-${_label}`); + await getPrismaClient().labResult.create({ + data: { + userId: USER_ID, + analyte: "LDL", + value: 177, + unit: "mg/dL", + takenAt: new Date("2026-07-20T08:00:00.000Z"), + }, + }); + + const res = await postMcp(raw, "tools/call", { + name: "get_labs", + arguments: {}, + }); + + expect([401, 403]).toContain(res.status); + expect(await res.text()).not.toContain("177"); + }, + ); + + it.each([ + [ + "expired", + { expiresAt: new Date("2020-01-01T00:00:00.000Z") }, + ], + ["revoked", { revoked: true }], + ])("denies an explicit but %s health:read credential", async (_label, state) => { + const raw = await useExpiringToken( + ["health:read"], + `mcp-${_label}`, + state, + ); + + const res = await postMcp(raw, "tools/list"); + + expect([401, 403]).toContain(res.status); + }); +}); + describe("B6 — a declared scope grants only what it names", () => { it("admits ['fhir:read'] on the FHIR face", async () => { await useToken(["fhir:read"], "fhir");