diff --git a/server/routes/mcp/index.test.ts b/server/routes/mcp/index.test.ts index b8793575be..f75becd668 100644 --- a/server/routes/mcp/index.test.ts +++ b/server/routes/mcp/index.test.ts @@ -1,5 +1,6 @@ import { Scope, TeamPreference } from "@shared/types"; import type { ProsemirrorData } from "@shared/types"; +import { Attachment } from "@server/models"; import { buildUser, buildAdmin, @@ -823,6 +824,92 @@ describe("POST /mcp/", () => { }); }); + describe("attachment tools", () => { + it("create_attachment returns absolute uploadUrl and proxied attachment url", async () => { + const { accessToken } = await buildOAuthUser(); + const res = await callMcpTool(server, accessToken, "create_attachment", { + contentType: "image/png", + name: "test.png", + size: 1000, + }); + + expect(res?.result?.isError).toBeFalsy(); + const data = JSON.parse(res?.result?.content?.[0]?.text ?? "{}"); + + expect(data.uploadUrl).toMatch(/^https?:\/\//); + expect(data.attachment.url).toMatch(/^https?:\/\//); + expect(data.attachment.url).toContain("/api/attachments.redirect?id="); + expect(data.curlCommand).toContain(data.uploadUrl); + }); + + it("create_attachment persists attachment record", async () => { + const { user, accessToken } = await buildOAuthUser(); + const res = await callMcpTool(server, accessToken, "create_attachment", { + contentType: "image/png", + name: "test.png", + size: 1000, + }); + + const data = JSON.parse(res?.result?.content?.[0]?.text ?? "{}"); + const attachment = await Attachment.findByPk(data.attachment.id, { + rejectOnEmpty: true, + }); + expect(Number(attachment.size)).toEqual(1000); + expect(attachment.contentType).toEqual("image/png"); + expect(attachment.userId).toEqual(user.id); + expect(attachment.teamId).toEqual(user.teamId); + }); + + it("create_attachment rejects size larger than max", async () => { + const { accessToken } = await buildOAuthUser(); + const res = await callMcpTool(server, accessToken, "create_attachment", { + contentType: "image/png", + name: "huge.png", + size: 10_000_000_000, + }); + expect(res?.result?.isError).toBe(true); + }); + + it("create_attachment rejects negative size", async () => { + const { accessToken } = await buildOAuthUser(); + const res = await callMcpTool(server, accessToken, "create_attachment", { + contentType: "image/png", + name: "neg.png", + size: -1, + }); + expect(res?.error ?? res?.result?.isError).toBeTruthy(); + }); + + it("create_attachment rejects fractional size", async () => { + const { accessToken } = await buildOAuthUser(); + const res = await callMcpTool(server, accessToken, "create_attachment", { + contentType: "image/png", + name: "frac.png", + size: 1.5, + }); + expect(res?.error ?? res?.result?.isError).toBeTruthy(); + }); + + it("read-only token does not have create_attachment tool", async () => { + const user = await buildUser(); + const auth = await buildOAuthAuthentication({ + user, + scope: [Scope.Read], + }); + const res = await callMcpTool( + server, + auth.accessToken!, + "create_attachment", + { + contentType: "image/png", + name: "test.png", + size: 1000, + } + ); + expect(res?.result?.isError).toBe(true); + }); + }); + describe("scope enforcement", () => { async function buildScopedOAuthUser(scope: Scope[]) { const user = await buildUser(); diff --git a/server/tools/attachments.ts b/server/tools/attachments.ts index c146d125a5..ab23a802dd 100644 --- a/server/tools/attachments.ts +++ b/server/tools/attachments.ts @@ -1,6 +1,7 @@ import { randomUUID } from "crypto"; import { z } from "zod"; import { type McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { ValidationError } from "@server/errors"; import { Attachment, Team } from "@server/models"; import AttachmentHelper from "@server/models/helpers/AttachmentHelper"; import { authorize } from "@server/policies"; @@ -8,7 +9,14 @@ import presentAttachment from "@server/presenters/attachment"; import FileStorage from "@server/storage/files"; import AuthenticationHelper from "@shared/helpers/AuthenticationHelper"; import { AttachmentPreset } from "@shared/types"; -import { error, success, buildAPIContext, withTracing } from "./util"; +import { bytesToHumanReadable } from "@shared/utils/files"; +import { + error, + success, + buildAPIContext, + pathToUrl, + withTracing, +} from "./util"; /** * Registers attachment-related MCP tools on the given server, filtered by @@ -36,7 +44,12 @@ export function attachmentTools(server: McpServer, scopes: string[]) { name: z .string() .describe("The filename including extension, e.g. screenshot.png."), - size: z.coerce.number().describe("The file size in bytes."), + size: z.coerce + .number() + .int() + .nonnegative() + .finite() + .describe("The file size in bytes."), }, }, withTracing( @@ -53,6 +66,15 @@ export function attachmentTools(server: McpServer, scopes: string[]) { const preset = AttachmentPreset.DocumentAttachment; const maxUploadSize = AttachmentHelper.presetToMaxUploadSize(preset); + + if (size > maxUploadSize) { + throw ValidationError( + `Sorry, this file is too large – the maximum size is ${bytesToHumanReadable( + maxUploadSize + )}` + ); + } + const id = randomUUID(); const acl = AttachmentHelper.presetToAcl(preset); const key = AttachmentHelper.getKey({ @@ -79,7 +101,8 @@ export function attachmentTools(server: McpServer, scopes: string[]) { contentType ); - const uploadUrl = FileStorage.getUploadUrl(); + const uploadUrl = new URL(FileStorage.getUploadUrl(), team.url) + .href; const form = { "Cache-Control": "max-age=31557600", "Content-Type": contentType, @@ -97,10 +120,10 @@ export function attachmentTools(server: McpServer, scopes: string[]) { form, maxUploadSize, curlCommand, - attachment: { + attachment: pathToUrl(team, { ...presentAttachment(attachment), url: attachment.redirectUrl, - }, + }), }); } catch (message) { return error(message); diff --git a/server/tools/util.ts b/server/tools/util.ts index a18cc7f6af..5350068550 100644 --- a/server/tools/util.ts +++ b/server/tools/util.ts @@ -42,6 +42,7 @@ export function buildAPIContext(context: McpContext) { return { state: { auth }, context: { auth, ip }, + cookies: { get: () => undefined, set: () => undefined }, } as unknown as APIContext; }