mirror of
https://github.com/outline/outline.git
synced 2026-06-13 03:14:59 +03:00
fix: Relative path returned from MCP (#12255)
* fix: relative path returned from MCP * fix: MCP create_attachment uploadUrl and size validation Make uploadUrl absolute against team.url so MCP clients can resolve it without a base, tighten the size schema to match the REST endpoint (int, nonnegative, finite), and stub cookies on the MCP API context so LocalStorage's CSRF-aware getPresignedPost works for Bearer-authed MCP requests. Adds tests covering the success path, persistence, size limits, schema rejections, and read-only scope enforcement. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -42,6 +42,7 @@ export function buildAPIContext(context: McpContext) {
|
||||
return {
|
||||
state: { auth },
|
||||
context: { auth, ip },
|
||||
cookies: { get: () => undefined, set: () => undefined },
|
||||
} as unknown as APIContext;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user