mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
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
This commit is contained in:
1 parent
fb533147d3
commit
6c0f47acad
4 files changed
+229
-8
No files matched your search
@@ -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<typeof import("@/lib/mcp/server")>();
|
||||
return {
|
||||
...actual,
|
||||
createMcpServer: (...args: Parameters<typeof actual.createMcpServer>) => {
|
||||
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<string, string> = {}): 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)", () => {
|
||||
|
||||
@@ -14,7 +14,7 @@ beforeEach(() => {
|
||||
});
|
||||
|
||||
describe("resolveMcpAuthContext", () => {
|
||||
it("binds the session to <userId>:<tokenId> (REQ-SEC-11) via the canonical Bearer path", async () => {
|
||||
it("binds a health:read session to <userId>:<tokenId> 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();
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user