diff --git a/src/tools/args.ts b/src/tools/args.ts index 653f72da..fcef93d0 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -30,7 +30,7 @@ export const CommonArgs = { }; export const AtlasArgs = { - projectId: (): z.ZodString => CommonArgs.objectId("projectId"), + projectId: (): z.ZodString => CommonArgs.objectId("projectId").describe("Atlas project ID"), organizationId: (): z.ZodString => CommonArgs.objectId("organizationId"), @@ -70,6 +70,14 @@ export const AtlasArgs = { z.string().min(1, "Password is required").max(100, "Password must be 100 characters or less"), }; +export const ProjectAndClusterArgs = { + projectId: AtlasArgs.projectId(), + clusterName: AtlasArgs.clusterName().describe("Atlas cluster name"), +}; + +export const ProjectArgs = { + projectId: AtlasArgs.projectId(), +}; function toEJSON(value: T): T { if (!value) { return value; diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 54f3ae8b..d1fae89c 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -20,7 +20,7 @@ function sleep(ms: number): Promise { } export const ConnectClusterArgs = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), + projectId: AtlasArgs.projectId(), clusterName: AtlasArgs.clusterName().describe("Atlas cluster name"), }; diff --git a/src/tools/atlas/create/createAccessList.ts b/src/tools/atlas/create/createAccessList.ts index fe5a862f..cbdf9669 100644 --- a/src/tools/atlas/create/createAccessList.ts +++ b/src/tools/atlas/create/createAccessList.ts @@ -3,10 +3,10 @@ import { type OperationType, type ToolArgs } from "../../tool.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import { makeCurrentIpAccessListEntry, DEFAULT_ACCESS_LIST_COMMENT } from "../../../common/atlas/accessListUtils.js"; -import { AtlasArgs, CommonArgs } from "../../args.js"; +import { AtlasArgs, CommonArgs, ProjectArgs } from "../../args.js"; export const CreateAccessListArgs = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), + ...ProjectArgs, ipAddresses: z.array(AtlasArgs.ipAddress()).describe("IP addresses to allow access from").optional(), cidrBlocks: z.array(AtlasArgs.cidrBlock()).describe("CIDR blocks to allow access from").optional(), currentIpAddress: z.boolean().describe("Add the current IP address").default(false), diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index c8e8ea01..df7d4a28 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -8,7 +8,7 @@ import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUti import { AtlasArgs, CommonArgs } from "../../args.js"; export const CreateDBUserArgs = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), + projectId: AtlasArgs.projectId(), username: AtlasArgs.username().describe("Username for the new user"), // Models will generate overly simplistic passwords like SecurePassword123 or // AtlasPassword123, which are easily guessable and exploitable. We're instructing @@ -26,7 +26,7 @@ export const CreateDBUserArgs = { collectionName: CommonArgs.string().describe("Collection name").optional(), }) ) - .describe("Roles for the new user"), + .describe("Roles for the new database user"), clusters: z .array(AtlasArgs.clusterName()) .describe("Clusters to assign the user to, leave empty for access to all clusters") @@ -35,7 +35,7 @@ export const CreateDBUserArgs = { export class CreateDBUserTool extends AtlasToolBase { public name = "atlas-create-db-user"; - protected description = "Create an MongoDB Atlas database user"; + protected description = "Create a MongoDB Atlas database user"; public operationType: OperationType = "create"; protected argsShape = { ...CreateDBUserArgs, diff --git a/src/tools/atlas/read/inspectAccessList.ts b/src/tools/atlas/read/inspectAccessList.ts index 6c8eaed3..24a142b2 100644 --- a/src/tools/atlas/read/inspectAccessList.ts +++ b/src/tools/atlas/read/inspectAccessList.ts @@ -4,7 +4,7 @@ import { AtlasToolBase } from "../atlasTool.js"; import { AtlasArgs } from "../../args.js"; export const InspectAccessListArgs = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), + projectId: AtlasArgs.projectId(), }; export class InspectAccessListTool extends AtlasToolBase { diff --git a/src/tools/atlas/read/inspectCluster.ts b/src/tools/atlas/read/inspectCluster.ts index 56e1e5a8..66c8e4a7 100644 --- a/src/tools/atlas/read/inspectCluster.ts +++ b/src/tools/atlas/read/inspectCluster.ts @@ -3,19 +3,14 @@ import { type OperationType, type ToolArgs, formatUntrustedData } from "../../to import { AtlasToolBase } from "../atlasTool.js"; import type { Cluster } from "../../../common/atlas/cluster.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; -import { AtlasArgs } from "../../args.js"; - -export const InspectClusterArgs = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), - clusterName: AtlasArgs.clusterName().describe("Atlas cluster name"), -}; +import { ProjectAndClusterArgs } from "../../args.js"; export class InspectClusterTool extends AtlasToolBase { public name = "atlas-inspect-cluster"; protected description = "Inspect MongoDB Atlas cluster"; public operationType: OperationType = "read"; protected argsShape = { - ...InspectClusterArgs, + ...ProjectAndClusterArgs, }; protected async execute({ projectId, clusterName }: ToolArgs): Promise { diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index d62354a8..8084aa16 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -103,8 +103,10 @@ export function setupIntegrationTest( keychain: new Keychain(), }); - // Mock hasValidAccessToken for tests + // Mock API Client for tests if (!userConfig.apiClientId && !userConfig.apiClientSecret) { + userConfig.apiClientId = "test"; + userConfig.apiClientSecret = "test"; const mockFn = vi.fn().mockResolvedValue(true); session.apiClient.validateAccessToken = mockFn; } @@ -242,6 +244,16 @@ export const databaseCollectionParameters: ParameterInfo[] = [ { name: "collection", type: "string", description: "Collection name", required: true }, ]; +export const projectIdParameters: ParameterInfo[] = [ + { name: "projectId", type: "string", description: "Atlas project ID", required: true }, +]; + +export const createClusterParameters: ParameterInfo[] = [ + { name: "name", type: "string", description: "Name of the cluster", required: true }, + { name: "projectId", type: "string", description: "Atlas project ID to create the cluster in", required: true }, + { name: "region", type: "string", description: "Region of the cluster", required: false }, +]; + export const databaseCollectionInvalidArgs = [ {}, { database: "test" }, @@ -252,6 +264,14 @@ export const databaseCollectionInvalidArgs = [ { database: "test", collection: [] }, ]; +export const projectIdInvalidArgs = [ + {}, + { projectId: 123 }, + { projectId: [] }, + { projectId: "!✅invalid" }, + { projectId: "invalid-test-project-id" }, +]; + export const databaseInvalidArgs = [{}, { database: 123 }, { database: [] }]; export function validateToolMetadata( diff --git a/tests/integration/tools/atlas/alerts.test.ts b/tests/integration/tools/atlas/alerts.test.ts index 3bf29658..06307a52 100644 --- a/tests/integration/tools/atlas/alerts.test.ts +++ b/tests/integration/tools/atlas/alerts.test.ts @@ -1,15 +1,24 @@ -import { expectDefined, getResponseElements } from "../../helpers.js"; +import { + getResponseElements, + projectIdInvalidArgs, + validateThrowsForInvalidArguments, + validateToolMetadata, +} from "../../helpers.js"; import { parseTable, describeWithAtlas, withProject } from "./atlasHelpers.js"; -import { expect, it } from "vitest"; +import { expect, it, describe } from "vitest"; describeWithAtlas("atlas-list-alerts", (integration) => { - it("should have correct metadata", async () => { - const { tools } = await integration.mcpClient().listTools(); - const listAlerts = tools.find((tool) => tool.name === "atlas-list-alerts"); - expectDefined(listAlerts); - expect(listAlerts.inputSchema.type).toBe("object"); - expectDefined(listAlerts.inputSchema.properties); - expect(listAlerts.inputSchema.properties).toHaveProperty("projectId"); + describe("should have correct metadata and validate invalid arguments", () => { + validateToolMetadata(integration, "atlas-list-alerts", "List MongoDB Atlas alerts", [ + { + name: "projectId", + type: "string", + description: "Atlas project ID to list alerts for", + required: true, + }, + ]); + + validateThrowsForInvalidArguments(integration, "atlas-list-alerts", projectIdInvalidArgs); }); withProject(integration, ({ getProjectId }) => { diff --git a/tests/integration/tools/atlas/atlasHelpers.ts b/tests/integration/tools/atlas/atlasHelpers.ts index 00ac53fe..760c684c 100644 --- a/tests/integration/tools/atlas/atlasHelpers.ts +++ b/tests/integration/tools/atlas/atlasHelpers.ts @@ -9,11 +9,7 @@ import { afterAll, beforeAll, describe } from "vitest"; export type IntegrationTestFunction = (integration: IntegrationTest) => void; export function describeWithAtlas(name: string, fn: IntegrationTestFunction): void { - const describeFn = - !process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length - ? describe.skip - : describe; - describeFn(name, () => { + describe(name, () => { const integration = setupIntegrationTest( () => ({ ...defaultTestConfig, @@ -34,8 +30,23 @@ interface ProjectTestArgs { type ProjectTestFunction = (args: ProjectTestArgs) => void; +export function withCredentials(integration: IntegrationTest, fn: IntegrationTestFunction): SuiteCollector { + const describeFn = + !process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length + ? describe.skip + : describe; + return describeFn("with credentials", () => { + fn(integration); + }); +} + export function withProject(integration: IntegrationTest, fn: ProjectTestFunction): SuiteCollector { - return describe("with project", () => { + const describeFn = + !process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length + ? describe.skip + : describe; + + return describeFn("with project", () => { let projectId: string = ""; let ipAddress: string = ""; diff --git a/tests/integration/tools/atlas/clusters.test.ts b/tests/integration/tools/atlas/clusters.test.ts index 5c50c570..fdbb8418 100644 --- a/tests/integration/tools/atlas/clusters.test.ts +++ b/tests/integration/tools/atlas/clusters.test.ts @@ -1,5 +1,13 @@ import type { Session } from "../../../../src/common/session.js"; -import { expectDefined, getDataFromUntrustedContent, getResponseElements } from "../../helpers.js"; +import { + expectDefined, + getResponseElements, + getDataFromUntrustedContent, + createClusterParameters, + projectIdInvalidArgs, + validateThrowsForInvalidArguments, + validateToolMetadata, +} from "../../helpers.js"; import { describeWithAtlas, withProject, randomId, parseTable } from "./atlasHelpers.js"; import type { ClusterDescription20240805 } from "../../../../src/common/atlas/openapi.js"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; @@ -57,6 +65,19 @@ async function waitCluster( } describeWithAtlas("clusters", (integration) => { + describe("should have correct metadata and validate invalid arguments", () => { + validateToolMetadata( + integration, + "atlas-create-free-cluster", + "Create a free MongoDB Atlas cluster", + createClusterParameters + ); + + expect(() => { + validateThrowsForInvalidArguments(integration, "atlas-create-free-cluster", projectIdInvalidArgs); + }).not.toThrow(); + }); + withProject(integration, ({ getProjectId, getIpAddress }) => { const clusterName = "ClusterTest-" + randomId; diff --git a/tests/integration/tools/atlas/dbUsers.test.ts b/tests/integration/tools/atlas/dbUsers.test.ts index fee08b42..676f83b6 100644 --- a/tests/integration/tools/atlas/dbUsers.test.ts +++ b/tests/integration/tools/atlas/dbUsers.test.ts @@ -1,10 +1,19 @@ import { describeWithAtlas, withProject, randomId } from "./atlasHelpers.js"; -import { expectDefined, getResponseElements } from "../../helpers.js"; +import { + expectDefined, + getResponseElements, + projectIdInvalidArgs, + validateThrowsForInvalidArguments, +} from "../../helpers.js"; import { ApiClientError } from "../../../../src/common/atlas/apiClientError.js"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { Keychain } from "../../../../src/common/keychain.js"; describeWithAtlas("db users", (integration) => { + describe("should have correct metadata and validate invalid arguments", () => { + validateThrowsForInvalidArguments(integration, "atlas-create-db-user", projectIdInvalidArgs); + }); + withProject(integration, ({ getProjectId }) => { let userName: string; beforeEach(() => { diff --git a/tests/integration/tools/atlas/orgs.test.ts b/tests/integration/tools/atlas/orgs.test.ts index 72e0182b..baa4f96a 100644 --- a/tests/integration/tools/atlas/orgs.test.ts +++ b/tests/integration/tools/atlas/orgs.test.ts @@ -1,23 +1,25 @@ import { expectDefined, getDataFromUntrustedContent, getResponseElements } from "../../helpers.js"; -import { parseTable, describeWithAtlas } from "./atlasHelpers.js"; +import { parseTable, describeWithAtlas, withCredentials } from "./atlasHelpers.js"; import { describe, expect, it } from "vitest"; describeWithAtlas("orgs", (integration) => { - describe("atlas-list-orgs", () => { - it("should have correct metadata", async () => { - const { tools } = await integration.mcpClient().listTools(); - const listOrgs = tools.find((tool) => tool.name === "atlas-list-orgs"); - expectDefined(listOrgs); - }); + withCredentials(integration, () => { + describe("atlas-list-orgs", () => { + it("should have correct metadata", async () => { + const { tools } = await integration.mcpClient().listTools(); + const listOrgs = tools.find((tool) => tool.name === "atlas-list-orgs"); + expectDefined(listOrgs); + }); - it("returns org names", async () => { - const response = await integration.mcpClient().callTool({ name: "atlas-list-orgs", arguments: {} }); - const elements = getResponseElements(response); - expect(elements[0]?.text).toContain("Found 1 organizations"); - expect(elements[1]?.text).toContain(" { + const response = await integration.mcpClient().callTool({ name: "atlas-list-orgs", arguments: {} }); + const elements = getResponseElements(response); + expect(elements[0]?.text).toContain("Found 1 organizations"); + expect(elements[1]?.text).toContain(" { - const projName = "testProj-" + randomId; + withCredentials(integration, () => { + const projName = "testProj-" + randomId; - afterAll(async () => { - const session = integration.mcpServer().session; + afterAll(async () => { + const session = integration.mcpServer().session; - const projects = await session.apiClient.listProjects(); - for (const project of projects?.results || []) { - if (project.name === projName) { - await session.apiClient.deleteProject({ - params: { - path: { - groupId: project.id || "", + const projects = await session.apiClient.listProjects(); + for (const project of projects?.results || []) { + if (project.name === projName) { + await session.apiClient.deleteProject({ + params: { + path: { + groupId: project.id || "", + }, }, - }, - }); - break; + }); + break; + } } - } - }); - - describe("atlas-create-project", () => { - it("should have correct metadata", async () => { - const { tools } = await integration.mcpClient().listTools(); - const createProject = tools.find((tool) => tool.name === "atlas-create-project"); - expectDefined(createProject); - expect(createProject.inputSchema.type).toBe("object"); - expectDefined(createProject.inputSchema.properties); - expect(createProject.inputSchema.properties).toHaveProperty("projectName"); - expect(createProject.inputSchema.properties).toHaveProperty("organizationId"); }); - it("should create a project", async () => { - const response = await integration.mcpClient().callTool({ - name: "atlas-create-project", - arguments: { projectName: projName }, + + describe("atlas-create-project", () => { + it("should have correct metadata", async () => { + const { tools } = await integration.mcpClient().listTools(); + const createProject = tools.find((tool) => tool.name === "atlas-create-project"); + expectDefined(createProject); + expect(createProject.inputSchema.type).toBe("object"); + expectDefined(createProject.inputSchema.properties); + expect(createProject.inputSchema.properties).toHaveProperty("projectName"); + expect(createProject.inputSchema.properties).toHaveProperty("organizationId"); }); + it("should create a project", async () => { + const response = await integration.mcpClient().callTool({ + name: "atlas-create-project", + arguments: { projectName: projName }, + }); - const elements = getResponseElements(response); - expect(elements).toHaveLength(1); - expect(elements[0]?.text).toContain(projName); - }); - }); - describe("atlas-list-projects", () => { - it("should have correct metadata", async () => { - const { tools } = await integration.mcpClient().listTools(); - const listProjects = tools.find((tool) => tool.name === "atlas-list-projects"); - expectDefined(listProjects); - expect(listProjects.inputSchema.type).toBe("object"); - expectDefined(listProjects.inputSchema.properties); - expect(listProjects.inputSchema.properties).toHaveProperty("orgId"); + const elements = getResponseElements(response); + expect(elements).toHaveLength(1); + expect(elements[0]?.text).toContain(projName); + }); }); + describe("atlas-list-projects", () => { + it("should have correct metadata", async () => { + const { tools } = await integration.mcpClient().listTools(); + const listProjects = tools.find((tool) => tool.name === "atlas-list-projects"); + expectDefined(listProjects); + expect(listProjects.inputSchema.type).toBe("object"); + expectDefined(listProjects.inputSchema.properties); + expect(listProjects.inputSchema.properties).toHaveProperty("orgId"); + }); - it("returns project names", async () => { - const response = await integration.mcpClient().callTool({ name: "atlas-list-projects", arguments: {} }); - const elements = getResponseElements(response); - expect(elements).toHaveLength(2); - expect(elements[1]?.text).toContain(" { + const response = await integration.mcpClient().callTool({ name: "atlas-list-projects", arguments: {} }); + const elements = getResponseElements(response); + expect(elements).toHaveLength(2); + expect(elements[1]?.text).toContain("