diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 2a8386d6e..18bd0d40c 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -16,6 +16,11 @@ import { SCOPE_HEALTH_READ, SCOPE_HEALTH_WRITE } from "./mcp/oauth/config"; /** * HTTP methods a read-only credential may use on the REST surface. A request * with any other method (POST / PUT / PATCH / DELETE) is a write. + * + * The MCP-audience guard below assumes these methods are side-effect-free: + * an MCP-bound token is admitted on them. A future side-effecting GET (or HEAD) + * would silently widen that token's reach over REST — do NOT add one without + * revisiting the MCP audience binding here. */ const READ_HTTP_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); diff --git a/src/lib/mcp/__tests__/write-tools.test.ts b/src/lib/mcp/__tests__/write-tools.test.ts index ac6fa49b4..d677f3ba6 100644 --- a/src/lib/mcp/__tests__/write-tools.test.ts +++ b/src/lib/mcp/__tests__/write-tools.test.ts @@ -3,10 +3,14 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; const logMcpMeasurement = vi.fn(); const logMcpMood = vi.fn(); const logMcpBloodPressure = vi.fn(); +const checkMcpMeasurement = vi.fn(); +const checkMcpBloodPressure = vi.fn(); vi.mock("../writes", () => ({ logMcpMeasurement: (...a: unknown[]) => logMcpMeasurement(...a), logMcpMood: (...a: unknown[]) => logMcpMood(...a), logMcpBloodPressure: (...a: unknown[]) => logMcpBloodPressure(...a), + checkMcpMeasurement: (...a: unknown[]) => checkMcpMeasurement(...a), + checkMcpBloodPressure: (...a: unknown[]) => checkMcpBloodPressure(...a), })); const checkMcpWriteRateLimit = vi.fn(); @@ -41,6 +45,10 @@ beforeEach(() => { remaining: 59, resetAt: Date.now() + 1000, }); + // Default: the pre-write validation passes, so a preview echoes a clean + // would-be record. Individual tests override to assert wouldFail. + checkMcpMeasurement.mockReturnValue({ ok: true }); + checkMcpBloodPressure.mockReturnValue({ ok: true }); }); describe("write-tool surface", () => { @@ -167,6 +175,27 @@ describe("confirm gate — log_measurement", () => { expect(result.written).toBe(false); expect(logMcpMeasurement).not.toHaveBeenCalled(); }); + + it("a preview that would be refused on commit carries wouldFail + reason", async () => { + checkMcpMeasurement.mockReturnValue({ + ok: false, + error: "out_of_range", + reason: "Timestamp must not be in the future", + }); + const result = (await tool("log_measurement").run(CTX, { + type: "WEIGHT", + value: 80, + measuredAt: "9999-01-01T00:00:00Z", + idempotencyKey: "k-1", + })) as Record; + // Still a preview — nothing written — but the commit verdict is surfaced. + expect(result.requiresConfirmation).toBe(true); + expect(result.written).toBe(false); + expect(result.wouldFail).toBe(true); + expect(result.error).toBe("out_of_range"); + expect(result.reason).toBe("Timestamp must not be in the future"); + expect(logMcpMeasurement).not.toHaveBeenCalled(); + }); }); describe("confirm gate — log_mood", () => { diff --git a/src/lib/mcp/write-tools.ts b/src/lib/mcp/write-tools.ts index feba0f78c..508e027b1 100644 --- a/src/lib/mcp/write-tools.ts +++ b/src/lib/mcp/write-tools.ts @@ -35,7 +35,13 @@ import type { MeasurementType } from "@/generated/prisma/client"; import { annotate } from "@/lib/logging/context"; import { checkMcpWriteRateLimit } from "@/lib/rate-limit"; import type { McpToolDefinition } from "./tools"; -import { logMcpMeasurement, logMcpMood, logMcpBloodPressure } from "./writes"; +import { + logMcpMeasurement, + logMcpMood, + logMcpBloodPressure, + checkMcpMeasurement, + checkMcpBloodPressure, +} from "./writes"; /** * Write-tool annotations (MCP 2025-11-25). NOT read-only and NOT destructive @@ -66,6 +72,10 @@ const writeOutput: z.ZodRawShape = { instruction: z.string().optional(), preview: z.unknown().optional(), record: z.unknown().optional(), + // Set on a preview that would be REFUSED on commit (type / range / instant), + // with `error` + `reason` carrying the same verdict the commit would return — + // so a preview and its commit never disagree. + wouldFail: z.boolean().optional(), }; /** Shared confirm-gate inputs every write tool carries. */ @@ -156,6 +166,9 @@ export const MCP_WRITE_TOOLS: McpToolDefinition[] = [ } if (!confirm) { + // Run the SAME validation the commit core runs so the preview cannot + // show a value that would then be refused on commit. + const check = checkMcpMeasurement(type, value, measuredAt); annotate({ action: { name: "mcp.tool.write" }, meta: { tool: "log_measurement", status: "preview" }, @@ -171,6 +184,9 @@ export const MCP_WRITE_TOOLS: McpToolDefinition[] = [ source: "MCP", }, instruction: CONFIRM_INSTRUCTION, + ...(check.ok + ? {} + : { wouldFail: true, error: check.error, reason: check.reason }), }; } @@ -310,6 +326,8 @@ export const MCP_WRITE_TOOLS: McpToolDefinition[] = [ } if (!confirm) { + // Same validation the commit core runs — preview == commit verdict. + const check = checkMcpBloodPressure(systolic, diastolic, measuredAt); annotate({ action: { name: "mcp.tool.write" }, meta: { tool: "log_blood_pressure", status: "preview" }, @@ -325,6 +343,9 @@ export const MCP_WRITE_TOOLS: McpToolDefinition[] = [ source: "MCP", }, instruction: CONFIRM_INSTRUCTION, + ...(check.ok + ? {} + : { wouldFail: true, error: check.error, reason: check.reason }), }; }