fix: harden v2 custom widgets and Workshop (#6480)
This commit is contained in:
1 parent
e8a2c87c2d
commit
557ebe0a2f
138 files changed
+5151
-1976
No files matched your search
@@ -4,6 +4,9 @@ import { beforeEach, describe, expect, test, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
authenticate: vi.fn(),
|
||||
ipAddress: vi.fn(() => "127.0.0.1"),
|
||||
toolProcedure: vi.fn(),
|
||||
loggerWarn: vi.fn(),
|
||||
buildPrompt: vi.fn((request?: string, documentationUrl?: string) =>
|
||||
[`Request: ${request ?? ""}`, `Documentation: ${documentationUrl ?? ""}`].join("\n"),
|
||||
),
|
||||
@@ -13,16 +16,16 @@ const mocks = vi.hoisted(() => ({
|
||||
vi.mock("next/server", () => ({ userAgent: () => ({ ua: "MCP route test" }) }));
|
||||
vi.mock("@homarr/api/mcp", () => ({
|
||||
createTRPCContext: vi.fn(() => ({})),
|
||||
mcpRouter: { createCaller: vi.fn(() => ({})) },
|
||||
mcpRouter: { createCaller: vi.fn(() => ({ board: { getAllBoards: mocks.toolProcedure } })) },
|
||||
}));
|
||||
vi.mock("@homarr/auth/api-key", () => ({
|
||||
API_KEY_HEADER_NAME: "ApiKey",
|
||||
getSessionFromApiKeyAsync: mocks.authenticate,
|
||||
}));
|
||||
vi.mock("@homarr/common", () => ({ extractBaseUrlFromHeaders: () => "http://localhost" }));
|
||||
vi.mock("@homarr/common/server", () => ({ ipAddressFromHeaders: () => "127.0.0.1" }));
|
||||
vi.mock("@homarr/common/server", () => ({ ipAddressFromHeaders: mocks.ipAddress }));
|
||||
vi.mock("@homarr/core/infrastructure/logs", () => ({
|
||||
createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
|
||||
createLogger: () => ({ info: vi.fn(), warn: mocks.loggerWarn, error: vi.fn() }),
|
||||
}));
|
||||
vi.mock("@homarr/custom-widgets/authoring-prompt", () => ({ buildCustomWidgetMcpPrompt: mocks.buildPrompt }));
|
||||
vi.mock("@homarr/custom-widgets/authoring-resources", () => ({
|
||||
@@ -36,7 +39,28 @@ vi.mock("@homarr/custom-widgets/core", () => ({
|
||||
}));
|
||||
vi.mock("@homarr/db", () => ({ db: {} }));
|
||||
vi.mock("~/versions/package-reader", () => ({ getPackageVersion: () => "test-version" }));
|
||||
vi.mock("../_extract-tools", () => ({ extractMcpTools: () => [] }));
|
||||
vi.mock("../_extract-tools", () => ({
|
||||
extractMcpTools: () => [
|
||||
{
|
||||
name: "board_getAllBoards",
|
||||
description: "List boards",
|
||||
pathInRouter: ["board", "getAllBoards"],
|
||||
inputSchema: { type: "object", properties: {} },
|
||||
},
|
||||
{
|
||||
name: "customWidget_list",
|
||||
description: "List Custom Widgets",
|
||||
pathInRouter: ["customWidget", "list"],
|
||||
inputSchema: { type: "object", properties: {} },
|
||||
},
|
||||
{
|
||||
name: "customWidget_workshopSearch",
|
||||
description: "Search Workshop",
|
||||
pathInRouter: ["customWidget", "workshopSearch"],
|
||||
inputSchema: { type: "object", properties: {} },
|
||||
},
|
||||
],
|
||||
}));
|
||||
|
||||
import { POST } from "./route";
|
||||
|
||||
@@ -48,9 +72,14 @@ interface JsonRpcResponse {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.authenticate.mockResolvedValue({ user: { id: "user-1" } });
|
||||
mocks.authenticate.mockResolvedValue({ user: { id: "user-1", permissions: ["admin"] } });
|
||||
mocks.buildPrompt.mockClear();
|
||||
mocks.getComponent.mockClear();
|
||||
mocks.ipAddress.mockClear();
|
||||
mocks.ipAddress.mockReturnValue("127.0.0.1");
|
||||
mocks.toolProcedure.mockReset();
|
||||
mocks.toolProcedure.mockResolvedValue([]);
|
||||
mocks.loggerWarn.mockClear();
|
||||
});
|
||||
|
||||
async function callMcp(method: string, params: Record<string, unknown> = {}, id = 1) {
|
||||
@@ -60,6 +89,8 @@ async function callMcp(method: string, params: Record<string, unknown> = {}, id
|
||||
Accept: "application/json, text/event-stream",
|
||||
ApiKey: "key.secret",
|
||||
"Content-Type": "application/json",
|
||||
"X-Forwarded-For": "198.51.100.10",
|
||||
"X-Real-IP": "127.0.0.1",
|
||||
},
|
||||
body: JSON.stringify({ jsonrpc: "2.0", id, method, params }),
|
||||
});
|
||||
@@ -91,6 +122,7 @@ describe("authenticated MCP prompt protocol", () => {
|
||||
serverInfo: { name: "homarr", version: "test-version" },
|
||||
});
|
||||
expect(mocks.authenticate).toHaveBeenCalledWith({}, "key.secret", "127.0.0.1", "MCP route test");
|
||||
expect(mocks.ipAddress).toHaveBeenCalledWith(expect.any(Headers));
|
||||
});
|
||||
|
||||
test("lists and interpolates the custom-widget authoring prompt", async () => {
|
||||
@@ -178,6 +210,34 @@ describe("authenticated MCP resource protocol", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("non-admin MCP discovery", () => {
|
||||
test("does not list or expose Custom Widget tools, prompts or resources", async () => {
|
||||
mocks.authenticate.mockResolvedValue({ user: { id: "user-1", permissions: ["board-view-all"] } });
|
||||
|
||||
const tools = await callMcp("tools/list");
|
||||
expect(((tools.body.result?.tools ?? []) as Array<{ name: string }>).map(({ name }) => name)).toEqual([
|
||||
"board_getAllBoards",
|
||||
]);
|
||||
|
||||
const prompts = await callMcp("prompts/list");
|
||||
expect(prompts.body.result).toEqual({ prompts: [] });
|
||||
|
||||
const resources = await callMcp("resources/list");
|
||||
expect(resources.body.result).toEqual({ resources: [] });
|
||||
|
||||
const templates = await callMcp("resources/templates/list");
|
||||
expect(templates.body.result).toEqual({ resourceTemplates: [] });
|
||||
|
||||
const prompt = await callMcp("prompts/get", { name: "homarr-custom-widget-author" });
|
||||
expect(prompt.body.result).toBeUndefined();
|
||||
expect(prompt.body.error).toBeDefined();
|
||||
|
||||
const resource = await callMcp("resources/read", { uri: "homarr://custom-widgets/schema" });
|
||||
expect(resource.body.result).toBeUndefined();
|
||||
expect(resource.body.error).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCP protocol errors", () => {
|
||||
test.each([
|
||||
["prompts/get", { name: "unknown" }],
|
||||
@@ -193,4 +253,23 @@ describe("MCP protocol errors", () => {
|
||||
expect(body.result).toBeUndefined();
|
||||
expect(body.error).toEqual(expect.objectContaining({ code: -32603, message: expect.any(String) }));
|
||||
});
|
||||
|
||||
test("does not log sensitive tool error messages", async () => {
|
||||
const failure = Object.assign(new Error("request to https://user:secret@example.test/private failed"), {
|
||||
code: "BAD_GATEWAY",
|
||||
});
|
||||
mocks.toolProcedure.mockRejectedValue(failure);
|
||||
|
||||
const { body } = await callMcp("tools/call", { name: "board_getAllBoards", arguments: {} });
|
||||
|
||||
expect(JSON.stringify(body)).not.toContain("user:secret");
|
||||
expect(mocks.loggerWarn).toHaveBeenCalledWith("MCP tool execution failed", {
|
||||
tool: "board_getAllBoards",
|
||||
errorName: "Error",
|
||||
errorCode: "BAD_GATEWAY",
|
||||
});
|
||||
const metadata = mocks.loggerWarn.mock.calls.at(-1)?.[1] as Record<string, unknown>;
|
||||
expect(metadata).not.toHaveProperty("error");
|
||||
expect(metadata).not.toHaveProperty("message");
|
||||
});
|
||||
});
|
||||
@@ -85,7 +85,12 @@ function clearAuthFailures(ip: string | null) {
|
||||
authFailures.delete(ip ?? "unknown");
|
||||
}
|
||||
|
||||
const callerStorage = new AsyncLocalStorage<ReturnType<typeof mcpRouter.createCaller>>();
|
||||
interface McpCallerContext {
|
||||
caller: ReturnType<typeof mcpRouter.createCaller>;
|
||||
isAdmin: boolean;
|
||||
}
|
||||
|
||||
const callerStorage = new AsyncLocalStorage<McpCallerContext>();
|
||||
|
||||
const emptyObjectSchema = {
|
||||
type: "object" as const,
|
||||
@@ -113,6 +118,20 @@ function getTools() {
|
||||
return toolsCache;
|
||||
}
|
||||
|
||||
function getVisibleTools() {
|
||||
const tools = getTools();
|
||||
if (!isCustomWidgetMcpEnabled()) {
|
||||
return tools.filter((tool) => !tool.name.startsWith("customWidget_"));
|
||||
}
|
||||
return tools;
|
||||
}
|
||||
|
||||
function requireCustomWidgetMcpAdmin() {
|
||||
if (!isCustomWidgetMcpEnabled()) throw new Error("Resource not found");
|
||||
}
|
||||
|
||||
const isCustomWidgetMcpEnabled = () => Boolean(callerStorage.getStore()?.isAdmin);
|
||||
|
||||
const SAFE_ERROR_MESSAGES: Record<string, string> = {
|
||||
UNAUTHORIZED: "You do not have permission to perform this action",
|
||||
FORBIDDEN: "Access denied",
|
||||
@@ -184,19 +203,22 @@ If \`hasUseAccess\` is false, the API key owner lacks permission for that integr
|
||||
const mcpHandler = createMcpHandler(
|
||||
(server) => {
|
||||
server.server.setRequestHandler(ListPromptsRequestSchema, () => ({
|
||||
prompts: [
|
||||
{
|
||||
name: "homarr-custom-widget-author",
|
||||
description: "Author and iterate on one Homarr Custom JSX v2 widget.",
|
||||
arguments: [
|
||||
{ name: "request", description: "The widget the user wants.", required: false },
|
||||
{ name: "documentationUrl", description: "External API documentation URL.", required: false },
|
||||
],
|
||||
},
|
||||
],
|
||||
prompts: isCustomWidgetMcpEnabled()
|
||||
? [
|
||||
{
|
||||
name: "homarr-custom-widget-author",
|
||||
description: "Author and iterate on one Homarr Custom JSX v2 widget.",
|
||||
arguments: [
|
||||
{ name: "request", description: "The widget the user wants.", required: false },
|
||||
{ name: "documentationUrl", description: "External API documentation URL.", required: false },
|
||||
],
|
||||
},
|
||||
]
|
||||
: [],
|
||||
}));
|
||||
|
||||
server.server.setRequestHandler(GetPromptRequestSchema, (request) => {
|
||||
requireCustomWidgetMcpAdmin();
|
||||
if (request.params.name !== "homarr-custom-widget-author") throw new Error("Prompt not found");
|
||||
return {
|
||||
description: "Current Homarr Custom Widget authoring workflow.",
|
||||
@@ -216,36 +238,49 @@ const mcpHandler = createMcpHandler(
|
||||
});
|
||||
|
||||
server.server.setRequestHandler(ListResourcesRequestSchema, () => ({
|
||||
resources: [
|
||||
{ uri: "homarr://custom-widgets/schema", name: "Custom Widget schema", mimeType: "application/schema+json" },
|
||||
{ uri: "homarr://custom-widgets/components", name: "Custom Widget components", mimeType: "application/json" },
|
||||
{ uri: "homarr://custom-widgets/skill", name: "Custom Widget skill", mimeType: "text/markdown" },
|
||||
...customJsxExamples.map((example) => ({
|
||||
uri: `homarr://custom-widgets/examples/${example.id}`,
|
||||
name: `Custom Widget example: ${example.title}`,
|
||||
mimeType: "application/json",
|
||||
})),
|
||||
],
|
||||
resources: isCustomWidgetMcpEnabled()
|
||||
? [
|
||||
{
|
||||
uri: "homarr://custom-widgets/schema",
|
||||
name: "Custom Widget schema",
|
||||
mimeType: "application/schema+json",
|
||||
},
|
||||
{
|
||||
uri: "homarr://custom-widgets/components",
|
||||
name: "Custom Widget components",
|
||||
mimeType: "application/json",
|
||||
},
|
||||
{ uri: "homarr://custom-widgets/skill", name: "Custom Widget skill", mimeType: "text/markdown" },
|
||||
...customJsxExamples.map((example) => ({
|
||||
uri: `homarr://custom-widgets/examples/${example.id}`,
|
||||
name: `Custom Widget example: ${example.title}`,
|
||||
mimeType: "application/json",
|
||||
})),
|
||||
]
|
||||
: [],
|
||||
}));
|
||||
|
||||
server.server.setRequestHandler(ListResourceTemplatesRequestSchema, () => ({
|
||||
resourceTemplates: [
|
||||
{
|
||||
uriTemplate: "homarr://custom-widgets/components/{name}",
|
||||
name: "Custom Widget component",
|
||||
description: "Metadata for one installed Custom Widget component.",
|
||||
mimeType: "application/json",
|
||||
},
|
||||
{
|
||||
uriTemplate: "homarr://custom-widgets/examples/{name}",
|
||||
name: "Custom Widget example",
|
||||
description: "One canonical Custom Widget example.",
|
||||
mimeType: "application/json",
|
||||
},
|
||||
],
|
||||
resourceTemplates: isCustomWidgetMcpEnabled()
|
||||
? [
|
||||
{
|
||||
uriTemplate: "homarr://custom-widgets/components/{name}",
|
||||
name: "Custom Widget component",
|
||||
description: "Metadata for one installed Custom Widget component.",
|
||||
mimeType: "application/json",
|
||||
},
|
||||
{
|
||||
uriTemplate: "homarr://custom-widgets/examples/{name}",
|
||||
name: "Custom Widget example",
|
||||
description: "One canonical Custom Widget example.",
|
||||
mimeType: "application/json",
|
||||
},
|
||||
]
|
||||
: [],
|
||||
}));
|
||||
|
||||
server.server.setRequestHandler(ReadResourceRequestSchema, (request) => {
|
||||
requireCustomWidgetMcpAdmin();
|
||||
const { uri } = request.params;
|
||||
let text: string;
|
||||
let mimeType = "application/json";
|
||||
@@ -271,12 +306,12 @@ const mcpHandler = createMcpHandler(
|
||||
});
|
||||
|
||||
server.server.setRequestHandler(ListToolsRequestSchema, () => ({
|
||||
tools: getTools(),
|
||||
tools: getVisibleTools(),
|
||||
}));
|
||||
|
||||
server.server.setRequestHandler(CallToolRequestSchema, async (request) => {
|
||||
const { name, arguments: args } = request.params;
|
||||
const tools = getTools();
|
||||
const tools = getVisibleTools();
|
||||
const tool = tools.find((t) => t.name === name);
|
||||
|
||||
if (!tool) {
|
||||
@@ -286,8 +321,8 @@ const mcpHandler = createMcpHandler(
|
||||
};
|
||||
}
|
||||
|
||||
const caller = callerStorage.getStore();
|
||||
if (!caller) {
|
||||
const callerContext = callerStorage.getStore();
|
||||
if (!callerContext) {
|
||||
return {
|
||||
content: [{ type: "text" as const, text: "Authentication context not available" }],
|
||||
isError: true,
|
||||
@@ -295,7 +330,7 @@ const mcpHandler = createMcpHandler(
|
||||
}
|
||||
|
||||
try {
|
||||
const procedure = tool.pathInRouter.reduce<any>((acc, part) => acc?.[part], caller);
|
||||
const procedure = tool.pathInRouter.reduce<any>((acc, part) => acc?.[part], callerContext.caller);
|
||||
if (typeof procedure !== "function") {
|
||||
return {
|
||||
content: [{ type: "text" as const, text: `Tool "${name}" is not callable` }],
|
||||
@@ -311,7 +346,9 @@ const mcpHandler = createMcpHandler(
|
||||
const message = sanitizeErrorMessage(error);
|
||||
logger.warn("MCP tool execution failed", {
|
||||
tool: name,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
errorName: error instanceof Error ? error.name : "UnknownError",
|
||||
errorCode:
|
||||
error instanceof Error && "code" in error && typeof error.code === "string" ? error.code : undefined,
|
||||
});
|
||||
return {
|
||||
content: [{ type: "text" as const, text: JSON.stringify({ error: message }) }],
|
||||
@@ -398,7 +435,9 @@ const handler = async (req: NextRequest) => {
|
||||
const ctx = createTRPCContext({ session, headers: req.headers });
|
||||
const caller = mcpRouter.createCaller(ctx);
|
||||
|
||||
return callerStorage.run(caller, () => mcpHandler(req as unknown as Request));
|
||||
return callerStorage.run({ caller, isAdmin: session.user.permissions.includes("admin") }, () =>
|
||||
mcpHandler(req as unknown as Request),
|
||||
);
|
||||
};
|
||||
|
||||
export { handler as GET, handler as POST, handler as DELETE };
|
||||
Reference in new issue
Block a user