From b89637fe39aa5c6ed6575349daa2fa3bf71e5500 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 10 Sep 2025 17:49:30 +0100 Subject: [PATCH 01/15] fix: add argument validation --- src/tools/atlas/create/createFreeCluster.ts | 8 ++--- src/tools/atlas/validators.ts | 38 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 src/tools/atlas/validators.ts diff --git a/src/tools/atlas/create/createFreeCluster.ts b/src/tools/atlas/create/createFreeCluster.ts index 5a110d95d..3739fafde 100644 --- a/src/tools/atlas/create/createFreeCluster.ts +++ b/src/tools/atlas/create/createFreeCluster.ts @@ -1,18 +1,18 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import type { ToolArgs, OperationType } from "../../tool.js"; import type { ClusterDescription20240805 } from "../../../common/atlas/openapi.js"; import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; +import { AtlasArgs } from "../validators.js"; export class CreateFreeClusterTool extends AtlasToolBase { public name = "atlas-create-free-cluster"; protected description = "Create a free MongoDB Atlas cluster"; public operationType: OperationType = "create"; protected argsShape = { - projectId: z.string().describe("Atlas project ID to create the cluster in"), - name: z.string().describe("Name of the cluster"), - region: z.string().describe("Region of the cluster").default("US_EAST_1"), + projectId: AtlasArgs.projectId().describe("Atlas project ID to create the cluster in"), + name: AtlasArgs.clusterName().describe("Name of the cluster"), + region: AtlasArgs.region().describe("Region of the cluster").default("US_EAST_1"), }; protected async execute({ projectId, name, region }: ToolArgs): Promise { diff --git a/src/tools/atlas/validators.ts b/src/tools/atlas/validators.ts new file mode 100644 index 000000000..4d1691f6a --- /dev/null +++ b/src/tools/atlas/validators.ts @@ -0,0 +1,38 @@ +import { z } from "zod"; +/** + * Common Zod validators for Atlas tools + * These can be reused across different Atlas tools for consistent validation + */ +export const AtlasArgs = { + objectId: (fieldName: string): z.ZodString => + z + .string() + .min(1, `${fieldName} is required`) + .regex(/^[0-9a-fA-F]{24}$/, `${fieldName} must be a valid 24-character hexadecimal string`), + + projectId: (): z.ZodString => AtlasArgs.objectId("Project ID"), + + organizationId: (): z.ZodString => AtlasArgs.objectId("Organization ID"), + + clusterName: (): z.ZodString => + z + .string() + .min(1, "Cluster name is required") + .max(64, "Cluster name must be 64 characters or less") + .regex(/^[a-zA-Z0-9_-]+$/, "Cluster name can only contain letters, numbers, hyphens, and underscores"), + + username: (): z.ZodString => + z + .string() + .min(1, "Username is required") + .max(100, "Username must be 100 characters or less") + .regex(/^[a-zA-Z0-9._-]+$/, "Username can only contain letters, numbers, dots, hyphens, and underscores"), + + ipAddress: (): z.ZodString => z.string().ip({ version: "v4" }), + + cidrBlock: (): z.ZodString => + z.string().regex(/^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$/, "Must be a valid CIDR block (e.g., 192.168.1.0/24)"), + + region: (): z.ZodString => + z.string().regex(/^[a-zA-Z0-9_-]+$/, "Region can only contain letters, numbers, hyphens, and underscores"), +}; From 7682f5c7484487cf58e7f58f23ad823e2103497e Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 10 Sep 2025 18:40:49 +0100 Subject: [PATCH 02/15] update read tools --- src/tools/atlas/read/inspectAccessList.ts | 4 +-- src/tools/atlas/read/inspectCluster.ts | 3 ++- src/tools/atlas/read/listAlerts.ts | 4 +-- src/tools/atlas/read/listClusters.ts | 4 +-- src/tools/atlas/read/listDBUsers.ts | 4 +-- src/tools/atlas/read/listProjects.ts | 4 +-- src/tools/atlas/validators.ts | 33 ++++++++++++++--------- 7 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/tools/atlas/read/inspectAccessList.ts b/src/tools/atlas/read/inspectAccessList.ts index 7eedf6ed7..87d60929b 100644 --- a/src/tools/atlas/read/inspectAccessList.ts +++ b/src/tools/atlas/read/inspectAccessList.ts @@ -1,15 +1,15 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import type { ToolArgs, OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; +import { AtlasArgs } from "../validators.js"; export class InspectAccessListTool extends AtlasToolBase { public name = "atlas-inspect-access-list"; protected description = "Inspect Ip/CIDR ranges with access to your MongoDB Atlas clusters."; public operationType: OperationType = "read"; protected argsShape = { - projectId: z.string().describe("Atlas project ID"), + projectId: AtlasArgs.projectId().describe("Atlas project ID"), }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/inspectCluster.ts b/src/tools/atlas/read/inspectCluster.ts index feb5f5ac2..432649ca9 100644 --- a/src/tools/atlas/read/inspectCluster.ts +++ b/src/tools/atlas/read/inspectCluster.ts @@ -5,13 +5,14 @@ import type { ToolArgs, OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; import type { Cluster } from "../../../common/atlas/cluster.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; +import { AtlasArgs } from "../validators.js"; export class InspectClusterTool extends AtlasToolBase { public name = "atlas-inspect-cluster"; protected description = "Inspect MongoDB Atlas cluster"; public operationType: OperationType = "read"; protected argsShape = { - projectId: z.string().describe("Atlas project ID"), + projectId: AtlasArgs.projectId().describe("Atlas project ID"), clusterName: z.string().describe("Atlas cluster name"), }; diff --git a/src/tools/atlas/read/listAlerts.ts b/src/tools/atlas/read/listAlerts.ts index 8ab4666c7..9e7fea73a 100644 --- a/src/tools/atlas/read/listAlerts.ts +++ b/src/tools/atlas/read/listAlerts.ts @@ -1,15 +1,15 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import type { ToolArgs, OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; +import { AtlasArgs } from "../validators.js"; export class ListAlertsTool extends AtlasToolBase { public name = "atlas-list-alerts"; protected description = "List MongoDB Atlas alerts"; public operationType: OperationType = "read"; protected argsShape = { - projectId: z.string().describe("Atlas project ID to list alerts for"), + projectId: AtlasArgs.projectId().describe("Atlas project ID to list alerts for"), }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listClusters.ts b/src/tools/atlas/read/listClusters.ts index e3894b3f6..425364c05 100644 --- a/src/tools/atlas/read/listClusters.ts +++ b/src/tools/atlas/read/listClusters.ts @@ -1,4 +1,3 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import type { ToolArgs, OperationType } from "../../tool.js"; @@ -10,13 +9,14 @@ import type { PaginatedFlexClusters20241113, } from "../../../common/atlas/openapi.js"; import { formatCluster, formatFlexCluster } from "../../../common/atlas/cluster.js"; +import { AtlasArgs } from "../validators.js"; export class ListClustersTool extends AtlasToolBase { public name = "atlas-list-clusters"; protected description = "List MongoDB Atlas clusters"; public operationType: OperationType = "read"; protected argsShape = { - projectId: z.string().describe("Atlas project ID to filter clusters").optional(), + projectId: AtlasArgs.projectId().describe("Atlas project ID to filter clusters").optional(), }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listDBUsers.ts b/src/tools/atlas/read/listDBUsers.ts index 26bb28b93..56d7d79fb 100644 --- a/src/tools/atlas/read/listDBUsers.ts +++ b/src/tools/atlas/read/listDBUsers.ts @@ -1,16 +1,16 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import type { ToolArgs, OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; import type { DatabaseUserRole, UserScope } from "../../../common/atlas/openapi.js"; +import { AtlasArgs } from "../validators.js"; export class ListDBUsersTool extends AtlasToolBase { public name = "atlas-list-db-users"; protected description = "List MongoDB Atlas database users"; public operationType: OperationType = "read"; protected argsShape = { - projectId: z.string().describe("Atlas project ID to filter DB users"), + projectId: AtlasArgs.projectId().describe("Atlas project ID to filter DB users"), }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listProjects.ts b/src/tools/atlas/read/listProjects.ts index d5d3931b0..cb5f1e886 100644 --- a/src/tools/atlas/read/listProjects.ts +++ b/src/tools/atlas/read/listProjects.ts @@ -2,15 +2,15 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; import type { OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; -import { z } from "zod"; import type { ToolArgs } from "../../tool.js"; +import { AtlasArgs } from "../validators.js"; export class ListProjectsTool extends AtlasToolBase { public name = "atlas-list-projects"; protected description = "List MongoDB Atlas projects"; public operationType: OperationType = "read"; protected argsShape = { - orgId: z.string().describe("Atlas organization ID to filter projects").optional(), + orgId: AtlasArgs.organizationId().describe("Atlas organization ID to filter projects").optional(), }; protected async execute({ orgId }: ToolArgs): Promise { diff --git a/src/tools/atlas/validators.ts b/src/tools/atlas/validators.ts index 4d1691f6a..4d2442212 100644 --- a/src/tools/atlas/validators.ts +++ b/src/tools/atlas/validators.ts @@ -1,38 +1,47 @@ -import { z } from "zod"; +import { z, type ZodString } from "zod"; + +export const ToolArgs = { + string: (): ZodString => + z.string().regex(/^[\x20-\x7E]*$/, "String cannot contain special characters or Unicode symbols"), +}; + /** * Common Zod validators for Atlas tools * These can be reused across different Atlas tools for consistent validation */ export const AtlasArgs = { objectId: (fieldName: string): z.ZodString => - z - .string() + ToolArgs.string() .min(1, `${fieldName} is required`) .regex(/^[0-9a-fA-F]{24}$/, `${fieldName} must be a valid 24-character hexadecimal string`), - projectId: (): z.ZodString => AtlasArgs.objectId("Project ID"), + projectId: (): z.ZodString => AtlasArgs.objectId("projectId"), - organizationId: (): z.ZodString => AtlasArgs.objectId("Organization ID"), + organizationId: (): z.ZodString => AtlasArgs.objectId("organizationId"), clusterName: (): z.ZodString => - z - .string() + ToolArgs.string() .min(1, "Cluster name is required") .max(64, "Cluster name must be 64 characters or less") .regex(/^[a-zA-Z0-9_-]+$/, "Cluster name can only contain letters, numbers, hyphens, and underscores"), username: (): z.ZodString => - z - .string() + ToolArgs.string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") .regex(/^[a-zA-Z0-9._-]+$/, "Username can only contain letters, numbers, dots, hyphens, and underscores"), - ipAddress: (): z.ZodString => z.string().ip({ version: "v4" }), + ipAddress: (): z.ZodString => ToolArgs.string().ip({ version: "v4" }), cidrBlock: (): z.ZodString => - z.string().regex(/^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$/, "Must be a valid CIDR block (e.g., 192.168.1.0/24)"), + ToolArgs.string().regex( + /^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$/, + "Must be a valid CIDR block (e.g., 192.168.1.0/24)" + ), region: (): z.ZodString => - z.string().regex(/^[a-zA-Z0-9_-]+$/, "Region can only contain letters, numbers, hyphens, and underscores"), + ToolArgs.string().regex( + /^[a-zA-Z0-9_-]+$/, + "Region can only contain letters, numbers, hyphens, and underscores" + ), }; From dd003e6686d310746ef2379c318bb637eba748aa Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 10 Sep 2025 21:34:22 +0100 Subject: [PATCH 03/15] update all args --- src/tools/{atlas/validators.ts => args.ts} | 31 +- src/tools/atlas/atlasTool.ts | 3 +- src/tools/atlas/connect/connectCluster.ts | 12 +- src/tools/atlas/create/createAccessList.ts | 27 +- src/tools/atlas/create/createDBUser.ts | 57 ++-- src/tools/atlas/create/createFreeCluster.ts | 4 +- src/tools/atlas/create/createProject.ts | 12 +- src/tools/atlas/read/inspectAccessList.ts | 11 +- src/tools/atlas/read/inspectCluster.ts | 14 +- src/tools/atlas/read/listAlerts.ts | 11 +- src/tools/atlas/read/listClusters.ts | 8 +- src/tools/atlas/read/listDBUsers.ts | 8 +- src/tools/atlas/read/listProjects.ts | 8 +- tests/unit/args.test.ts | 333 ++++++++++++++++++++ 14 files changed, 453 insertions(+), 86 deletions(-) rename src/tools/{atlas/validators.ts => args.ts} (62%) create mode 100644 tests/unit/args.test.ts diff --git a/src/tools/atlas/validators.ts b/src/tools/args.ts similarity index 62% rename from src/tools/atlas/validators.ts rename to src/tools/args.ts index 4d2442212..a92feeffd 100644 --- a/src/tools/atlas/validators.ts +++ b/src/tools/args.ts @@ -1,17 +1,13 @@ import { z, type ZodString } from "zod"; -export const ToolArgs = { +export const CommonArgs = { string: (): ZodString => z.string().regex(/^[\x20-\x7E]*$/, "String cannot contain special characters or Unicode symbols"), }; -/** - * Common Zod validators for Atlas tools - * These can be reused across different Atlas tools for consistent validation - */ export const AtlasArgs = { objectId: (fieldName: string): z.ZodString => - ToolArgs.string() + CommonArgs.string() .min(1, `${fieldName} is required`) .regex(/^[0-9a-fA-F]{24}$/, `${fieldName} must be a valid 24-character hexadecimal string`), @@ -20,27 +16,32 @@ export const AtlasArgs = { organizationId: (): z.ZodString => AtlasArgs.objectId("organizationId"), clusterName: (): z.ZodString => - ToolArgs.string() + CommonArgs.string() .min(1, "Cluster name is required") .max(64, "Cluster name must be 64 characters or less") + .regex(/^[^/]*$/, "String cannot contain '/'") .regex(/^[a-zA-Z0-9_-]+$/, "Cluster name can only contain letters, numbers, hyphens, and underscores"), + projectName: (): z.ZodString => + CommonArgs.string() + .min(1, "Project name is required") + .max(64, "Project name must be 64 characters or less") + .regex(/^[^/]*$/, "String cannot contain '/'") + .regex(/^[a-zA-Z0-9_-]+$/, "Project name can only contain letters, numbers, hyphens, and underscores"), + username: (): z.ZodString => - ToolArgs.string() + CommonArgs.string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") + .regex(/^[^/]*$/, "String cannot contain '/'") .regex(/^[a-zA-Z0-9._-]+$/, "Username can only contain letters, numbers, dots, hyphens, and underscores"), - ipAddress: (): z.ZodString => ToolArgs.string().ip({ version: "v4" }), + ipAddress: (): z.ZodString => CommonArgs.string().ip({ version: "v4" }), - cidrBlock: (): z.ZodString => - ToolArgs.string().regex( - /^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$/, - "Must be a valid CIDR block (e.g., 192.168.1.0/24)" - ), + cidrBlock: (): z.ZodString => CommonArgs.string().cidr(), region: (): z.ZodString => - ToolArgs.string().regex( + CommonArgs.string().regex( /^[a-zA-Z0-9_-]+$/, "Region can only contain letters, numbers, hyphens, and underscores" ), diff --git a/src/tools/atlas/atlasTool.ts b/src/tools/atlas/atlasTool.ts index 452f2e794..b68eeafde 100644 --- a/src/tools/atlas/atlasTool.ts +++ b/src/tools/atlas/atlasTool.ts @@ -1,6 +1,5 @@ -import type { ToolCategory, TelemetryToolMetadata, ToolArgs } from "../tool.js"; -import { ToolBase } from "../tool.js"; import type { ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { ToolBase, type ToolArgs, type ToolCategory, type TelemetryToolMetadata } from "../tool.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { LogId } from "../../common/logger.js"; import { z } from "zod"; diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 618f5483b..1baf0c6f7 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -1,13 +1,13 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type OperationType, type ToolArgs } from "../../tool.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; import { generateSecurePassword } from "../../../helpers/generatePassword.js"; import { LogId } from "../../../common/logger.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; import type { AtlasClusterConnectionInfo } from "../../../common/connectionManager.js"; import { getDefaultRoleFromConfig } from "../../../common/atlas/roles.js"; +import { AtlasArgs } from "../../args.js"; const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours const addedIpAccessListMessage = @@ -20,13 +20,17 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +export const ConnectClusterArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID"), + clusterName: AtlasArgs.clusterName().describe("Atlas cluster name"), +}; + export class ConnectClusterTool extends AtlasToolBase { public name = "atlas-connect-cluster"; protected description = "Connect to MongoDB Atlas cluster"; public operationType: OperationType = "connect"; protected argsShape = { - projectId: z.string().describe("Atlas project ID"), - clusterName: z.string().describe("Atlas cluster name"), + ...ConnectClusterArgs, }; private queryConnection( diff --git a/src/tools/atlas/create/createAccessList.ts b/src/tools/atlas/create/createAccessList.ts index c7f5d43d8..0cf3b808e 100644 --- a/src/tools/atlas/create/createAccessList.ts +++ b/src/tools/atlas/create/createAccessList.ts @@ -1,26 +1,27 @@ import { z } from "zod"; +import { type OperationType, type ToolArgs } from "../../tool.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; import { makeCurrentIpAccessListEntry, DEFAULT_ACCESS_LIST_COMMENT } from "../../../common/atlas/accessListUtils.js"; +import { AtlasArgs, CommonArgs } from "../../args.js"; + +export const CreateAccessListArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID"), + 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), + comment: CommonArgs.string() + .describe("Comment for the access list entries") + .default(DEFAULT_ACCESS_LIST_COMMENT) + .optional(), +}; export class CreateAccessListTool extends AtlasToolBase { public name = "atlas-create-access-list"; protected description = "Allow Ip/CIDR ranges to access your MongoDB Atlas clusters."; public operationType: OperationType = "create"; protected argsShape = { - projectId: z.string().describe("Atlas project ID"), - ipAddresses: z - .array(z.string().ip({ version: "v4" })) - .describe("IP addresses to allow access from") - .optional(), - cidrBlocks: z.array(z.string().cidr()).describe("CIDR blocks to allow access from").optional(), - currentIpAddress: z.boolean().describe("Add the current IP address").default(false), - comment: z - .string() - .describe("Comment for the access list entries") - .default(DEFAULT_ACCESS_LIST_COMMENT) - .optional(), + ...CreateAccessListArgs, }; protected async execute({ diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index a807c44f5..04fb7f164 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -1,41 +1,46 @@ import { z } from "zod"; +import type { ToolArgs, OperationType } from "../../tool.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; import type { CloudDatabaseUser, DatabaseUserRole } from "../../../common/atlas/openapi.js"; import { generateSecurePassword } from "../../../helpers/generatePassword.js"; import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; +import { AtlasArgs, CommonArgs } from "../../args.js"; + +export const CreateDBUserArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID"), + 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 + // the model not to try and generate anything and instead leave the field unset. + password: z + .string() + .optional() + .nullable() + .describe( + "Password for the new user. IMPORTANT: If the user hasn't supplied an explicit password, leave it unset and under no circumstances try to generate a random one. A secure password will be generated by the MCP server if necessary." + ), + roles: z + .array( + z.object({ + roleName: CommonArgs.string().describe("Role name"), + databaseName: CommonArgs.string().describe("Database name").default("admin"), + collectionName: CommonArgs.string().describe("Collection name").optional(), + }) + ) + .describe("Roles for the new user"), + clusters: z + .array(CommonArgs.string()) + .describe("Clusters to assign the user to, leave empty for access to all clusters") + .optional(), +}; export class CreateDBUserTool extends AtlasToolBase { public name = "atlas-create-db-user"; protected description = "Create an MongoDB Atlas database user"; public operationType: OperationType = "create"; protected argsShape = { - projectId: z.string().describe("Atlas project ID"), - username: z.string().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 - // the model not to try and generate anything and instead leave the field unset. - password: z - .string() - .optional() - .nullable() - .describe( - "Password for the new user. IMPORTANT: If the user hasn't supplied an explicit password, leave it unset and under no circumstances try to generate a random one. A secure password will be generated by the MCP server if necessary." - ), - roles: z - .array( - z.object({ - roleName: z.string().describe("Role name"), - databaseName: z.string().describe("Database name").default("admin"), - collectionName: z.string().describe("Collection name").optional(), - }) - ) - .describe("Roles for the new user"), - clusters: z - .array(z.string()) - .describe("Clusters to assign the user to, leave empty for access to all clusters") - .optional(), + ...CreateDBUserArgs, }; protected async execute({ diff --git a/src/tools/atlas/create/createFreeCluster.ts b/src/tools/atlas/create/createFreeCluster.ts index 3739fafde..6b1ac98eb 100644 --- a/src/tools/atlas/create/createFreeCluster.ts +++ b/src/tools/atlas/create/createFreeCluster.ts @@ -1,9 +1,9 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type ToolArgs, type OperationType } from "../../tool.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; import type { ClusterDescription20240805 } from "../../../common/atlas/openapi.js"; import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; export class CreateFreeClusterTool extends AtlasToolBase { public name = "atlas-create-free-cluster"; diff --git a/src/tools/atlas/create/createProject.ts b/src/tools/atlas/create/createProject.ts index 60753b6b0..b981fd8e8 100644 --- a/src/tools/atlas/create/createProject.ts +++ b/src/tools/atlas/create/createProject.ts @@ -1,16 +1,20 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type OperationType, type ToolArgs } from "../../tool.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; import type { Group } from "../../../common/atlas/openapi.js"; +import { AtlasArgs } from "../../args.js"; + +export const CreateProjectArgs = { + projectName: AtlasArgs.projectName().optional().describe("Name for the new project"), + organizationId: AtlasArgs.organizationId().optional().describe("Organization ID for the new project"), +}; export class CreateProjectTool extends AtlasToolBase { public name = "atlas-create-project"; protected description = "Create a MongoDB Atlas project"; public operationType: OperationType = "create"; protected argsShape = { - projectName: z.string().optional().describe("Name for the new project"), - organizationId: z.string().optional().describe("Organization ID for the new project"), + ...CreateProjectArgs, }; protected async execute({ projectName, organizationId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/inspectAccessList.ts b/src/tools/atlas/read/inspectAccessList.ts index 87d60929b..6c8eaed30 100644 --- a/src/tools/atlas/read/inspectAccessList.ts +++ b/src/tools/atlas/read/inspectAccessList.ts @@ -1,15 +1,18 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type OperationType, type ToolArgs, formatUntrustedData } from "../../tool.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; -import { formatUntrustedData } from "../../tool.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; + +export const InspectAccessListArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID"), +}; export class InspectAccessListTool extends AtlasToolBase { public name = "atlas-inspect-access-list"; protected description = "Inspect Ip/CIDR ranges with access to your MongoDB Atlas clusters."; public operationType: OperationType = "read"; protected argsShape = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), + ...InspectAccessListArgs, }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/inspectCluster.ts b/src/tools/atlas/read/inspectCluster.ts index 432649ca9..56e1e5a8b 100644 --- a/src/tools/atlas/read/inspectCluster.ts +++ b/src/tools/atlas/read/inspectCluster.ts @@ -1,19 +1,21 @@ -import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type OperationType, type ToolArgs, formatUntrustedData } from "../../tool.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; -import { formatUntrustedData } from "../../tool.js"; import type { Cluster } from "../../../common/atlas/cluster.js"; import { inspectCluster } from "../../../common/atlas/cluster.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; + +export const InspectClusterArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID"), + clusterName: AtlasArgs.clusterName().describe("Atlas cluster name"), +}; export class InspectClusterTool extends AtlasToolBase { public name = "atlas-inspect-cluster"; protected description = "Inspect MongoDB Atlas cluster"; public operationType: OperationType = "read"; protected argsShape = { - projectId: AtlasArgs.projectId().describe("Atlas project ID"), - clusterName: z.string().describe("Atlas cluster name"), + ...InspectClusterArgs, }; protected async execute({ projectId, clusterName }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listAlerts.ts b/src/tools/atlas/read/listAlerts.ts index 9e7fea73a..d55a917f8 100644 --- a/src/tools/atlas/read/listAlerts.ts +++ b/src/tools/atlas/read/listAlerts.ts @@ -1,15 +1,18 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type OperationType, type ToolArgs, formatUntrustedData } from "../../tool.js"; import { AtlasToolBase } from "../atlasTool.js"; -import type { ToolArgs, OperationType } from "../../tool.js"; -import { formatUntrustedData } from "../../tool.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; + +export const ListAlertsArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID to list alerts for"), +}; export class ListAlertsTool extends AtlasToolBase { public name = "atlas-list-alerts"; protected description = "List MongoDB Atlas alerts"; public operationType: OperationType = "read"; protected argsShape = { - projectId: AtlasArgs.projectId().describe("Atlas project ID to list alerts for"), + ...ListAlertsArgs, }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listClusters.ts b/src/tools/atlas/read/listClusters.ts index 425364c05..5c8e72a30 100644 --- a/src/tools/atlas/read/listClusters.ts +++ b/src/tools/atlas/read/listClusters.ts @@ -9,14 +9,18 @@ import type { PaginatedFlexClusters20241113, } from "../../../common/atlas/openapi.js"; import { formatCluster, formatFlexCluster } from "../../../common/atlas/cluster.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; + +export const ListClustersArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID to filter clusters").optional(), +}; export class ListClustersTool extends AtlasToolBase { public name = "atlas-list-clusters"; protected description = "List MongoDB Atlas clusters"; public operationType: OperationType = "read"; protected argsShape = { - projectId: AtlasArgs.projectId().describe("Atlas project ID to filter clusters").optional(), + ...ListClustersArgs, }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listDBUsers.ts b/src/tools/atlas/read/listDBUsers.ts index 56d7d79fb..5ab23250c 100644 --- a/src/tools/atlas/read/listDBUsers.ts +++ b/src/tools/atlas/read/listDBUsers.ts @@ -3,14 +3,18 @@ import { AtlasToolBase } from "../atlasTool.js"; import type { ToolArgs, OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; import type { DatabaseUserRole, UserScope } from "../../../common/atlas/openapi.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; + +export const ListDBUsersArgs = { + projectId: AtlasArgs.projectId().describe("Atlas project ID to filter DB users"), +}; export class ListDBUsersTool extends AtlasToolBase { public name = "atlas-list-db-users"; protected description = "List MongoDB Atlas database users"; public operationType: OperationType = "read"; protected argsShape = { - projectId: AtlasArgs.projectId().describe("Atlas project ID to filter DB users"), + ...ListDBUsersArgs, }; protected async execute({ projectId }: ToolArgs): Promise { diff --git a/src/tools/atlas/read/listProjects.ts b/src/tools/atlas/read/listProjects.ts index cb5f1e886..3b7d24939 100644 --- a/src/tools/atlas/read/listProjects.ts +++ b/src/tools/atlas/read/listProjects.ts @@ -3,14 +3,18 @@ import { AtlasToolBase } from "../atlasTool.js"; import type { OperationType } from "../../tool.js"; import { formatUntrustedData } from "../../tool.js"; import type { ToolArgs } from "../../tool.js"; -import { AtlasArgs } from "../validators.js"; +import { AtlasArgs } from "../../args.js"; + +export const ListProjectsArgs = { + orgId: AtlasArgs.organizationId().describe("Atlas organization ID to filter projects").optional(), +}; export class ListProjectsTool extends AtlasToolBase { public name = "atlas-list-projects"; protected description = "List MongoDB Atlas projects"; public operationType: OperationType = "read"; protected argsShape = { - orgId: AtlasArgs.organizationId().describe("Atlas organization ID to filter projects").optional(), + ...ListProjectsArgs, }; protected async execute({ orgId }: ToolArgs): Promise { diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts new file mode 100644 index 000000000..1a3fbfdba --- /dev/null +++ b/tests/unit/args.test.ts @@ -0,0 +1,333 @@ +import { describe, expect, it } from "vitest"; +import { AtlasArgs, CommonArgs } from "../../src/tools/args.js"; + +describe("Atlas Validators", () => { + describe("CommonArgs", () => { + describe("string", () => { + it("should return a ZodString schema", () => { + const schema = CommonArgs.string(); + expect(schema).toBeDefined(); + expect(schema.parse("test")).toBe("test"); + }); + + it("should accept any string value", () => { + const schema = CommonArgs.string(); + expect(schema.parse("hello")).toBe("hello"); + expect(schema.parse("123")).toBe("123"); + expect(schema.parse("test@#$%")).toBe("test@#$%"); + }); + + it("should not allow special characters and unicode symbols", () => { + const schema = CommonArgs.string(); + expect(() => schema.parse("héllo")).toThrow(); + expect(() => schema.parse("测试")).toThrow(); + expect(() => schema.parse("🚀")).toThrow(); + }); + + it("should reject non-string values", () => { + const schema = CommonArgs.string(); + expect(() => schema.parse(123)).toThrow(); + expect(() => schema.parse(null)).toThrow(); + expect(() => schema.parse(undefined)).toThrow(); + expect(() => schema.parse({})).toThrow(); + }); + }); + }); + + describe("AtlasArgs", () => { + describe("objectId", () => { + it("should validate 24-character hexadecimal strings", () => { + const schema = AtlasArgs.objectId("Test ID"); + const validId = "507f1f77bcf86cd799439011"; + expect(schema.parse(validId)).toBe(validId); + }); + + it("should reject invalid ObjectId formats", () => { + const schema = AtlasArgs.objectId("Test ID"); + + // Too short + expect(() => schema.parse("507f1f77bcf86cd79943901")).toThrow(); + + // Too long + expect(() => schema.parse("507f1f77bcf86cd7994390111")).toThrow(); + + // Invalid characters + expect(() => schema.parse("507f1f77bcf86cd79943901g")).toThrow(); + expect(() => schema.parse("507f1f77bcf86cd79943901!")).toThrow(); + + // Empty string + expect(() => schema.parse("")).toThrow(); + }); + + it("should provide custom field name in error messages", () => { + const schema = AtlasArgs.objectId("Custom Field"); + expect(() => schema.parse("invalid")).toThrow( + "Custom Field must be a valid 24-character hexadecimal string" + ); + }); + + it("should not faile if the value is optional", () => { + const schema = AtlasArgs.objectId("Custom Field").optional(); + expect(schema.parse(undefined)).toBeUndefined(); + }); + + it("should not fail if the value is empty", () => { + const schema = AtlasArgs.objectId("Custom Field"); + expect(() => schema.parse(undefined)).toThrow("Required"); + }); + }); + + describe("projectId", () => { + it("should validate project IDs", () => { + const schema = AtlasArgs.projectId(); + const validId = "507f1f77bcf86cd799439011"; + expect(schema.parse(validId)).toBe(validId); + }); + + it("should reject invalid project IDs", () => { + const schema = AtlasArgs.projectId(); + expect(() => schema.parse("invalid")).toThrow( + "projectId must be a valid 24-character hexadecimal string" + ); + }); + }); + + describe("organizationId", () => { + it("should validate organization IDs", () => { + const schema = AtlasArgs.organizationId(); + const validId = "507f1f77bcf86cd799439011"; + expect(schema.parse(validId)).toBe(validId); + }); + + it("should reject invalid organization IDs", () => { + const schema = AtlasArgs.organizationId(); + expect(() => schema.parse("invalid")).toThrow( + "organizationId must be a valid 24-character hexadecimal string" + ); + }); + }); + + describe("clusterName", () => { + it("should validate valid cluster names", () => { + const schema = AtlasArgs.clusterName(); + const validNames = ["my-cluster", "cluster_1", "Cluster123", "test-cluster-2", "my_cluster_name"]; + + validNames.forEach((name) => { + expect(schema.parse(name)).toBe(name); + }); + }); + + it("should reject invalid cluster names", () => { + const schema = AtlasArgs.clusterName(); + + // Empty string + expect(() => schema.parse("")).toThrow("Cluster name is required"); + + // Too long (over 64 characters) + const longName = "a".repeat(65); + expect(() => schema.parse(longName)).toThrow("Cluster name must be 64 characters or less"); + + // Invalid characters + expect(() => schema.parse("cluster@name")).toThrow( + "Cluster name can only contain letters, numbers, hyphens, and underscores" + ); + expect(() => schema.parse("cluster name")).toThrow( + "Cluster name can only contain letters, numbers, hyphens, and underscores" + ); + expect(() => schema.parse("cluster.name")).toThrow( + "Cluster name can only contain letters, numbers, hyphens, and underscores" + ); + }); + + it("should accept exactly 64 characters", () => { + const schema = AtlasArgs.clusterName(); + const maxLengthName = "a".repeat(64); + expect(schema.parse(maxLengthName)).toBe(maxLengthName); + }); + }); + + describe("username", () => { + it("should validate valid usernames", () => { + const schema = AtlasArgs.username(); + const validUsernames = ["user123", "user_name", "user.name", "user-name", "User123", "test.user_name"]; + + validUsernames.forEach((username) => { + expect(schema.parse(username)).toBe(username); + }); + }); + + it("should reject invalid usernames", () => { + const schema = AtlasArgs.username(); + + // Empty string + expect(() => schema.parse("")).toThrow("Username is required"); + + // Too long (over 100 characters) + const longUsername = "a".repeat(101); + expect(() => schema.parse(longUsername)).toThrow("Username must be 100 characters or less"); + + // Invalid characters + expect(() => schema.parse("user@name")).toThrow( + "Username can only contain letters, numbers, dots, hyphens, and underscores" + ); + expect(() => schema.parse("user name")).toThrow( + "Username can only contain letters, numbers, dots, hyphens, and underscores" + ); + }); + + it("should accept exactly 100 characters", () => { + const schema = AtlasArgs.username(); + const maxLengthUsername = "a".repeat(100); + expect(schema.parse(maxLengthUsername)).toBe(maxLengthUsername); + }); + }); + + describe("ipAddress", () => { + it("should validate valid IPv4 addresses", () => { + const schema = AtlasArgs.ipAddress(); + const validIPs = ["192.168.1.1", "10.0.0.1", "172.16.0.1", "127.0.0.1", "0.0.0.0", "255.255.255.255"]; + + validIPs.forEach((ip) => { + expect(schema.parse(ip)).toBe(ip); + }); + }); + + it("should reject invalid IP addresses", () => { + const schema = AtlasArgs.ipAddress(); + + // Invalid formats + expect(() => schema.parse("192.168.1")).toThrow(); + expect(() => schema.parse("192.168.1.1.1")).toThrow(); + expect(() => schema.parse("192.168.1.256")).toThrow(); + expect(() => schema.parse("192.168.1.-1")).toThrow(); + expect(() => schema.parse("not-an-ip")).toThrow(); + + // IPv6 (should be rejected since we only support IPv4) + expect(() => schema.parse("2001:0db8:85a3:0000:0000:8a2e:0370:7334")).toThrow(); + }); + }); + + describe("cidrBlock", () => { + it("should validate valid CIDR blocks", () => { + const schema = AtlasArgs.cidrBlock(); + const validCIDRs = ["192.168.1.0/24", "10.0.0.0/8", "172.16.0.0/12", "0.0.0.0/0", "192.168.1.1/32"]; + + validCIDRs.forEach((cidr) => { + expect(schema.parse(cidr)).toBe(cidr); + }); + }); + + it("should reject invalid CIDR blocks", () => { + const schema = AtlasArgs.cidrBlock(); + + // Invalid formats + expect(() => schema.parse("192.168.1.0")).toThrow("Invalid cidr"); + expect(() => schema.parse("192.168.1.0/")).toThrow("Invalid cidr"); + expect(() => schema.parse("192.168.1.0/33")).toThrow("Invalid cidr"); + expect(() => schema.parse("192.168.1.256/24")).toThrow("Invalid cidr"); + expect(() => schema.parse("not-a-cidr")).toThrow("Invalid cidr"); + }); + }); + + describe("region", () => { + it("should validate valid region names", () => { + const schema = AtlasArgs.region(); + const validRegions = [ + "US_EAST_1", + "us-west-2", + "eu-central-1", + "ap-southeast-1", + "region_123", + "test-region", + ]; + + validRegions.forEach((region) => { + expect(schema.parse(region)).toBe(region); + }); + }); + + it("should reject invalid region names", () => { + const schema = AtlasArgs.region(); + + // Invalid characters + expect(() => schema.parse("US EAST 1")).toThrow( + "Region can only contain letters, numbers, hyphens, and underscores" + ); + expect(() => schema.parse("US.EAST.1")).toThrow( + "Region can only contain letters, numbers, hyphens, and underscores" + ); + expect(() => schema.parse("US@EAST#1")).toThrow( + "Region can only contain letters, numbers, hyphens, and underscores" + ); + }); + }); + + describe("projectName", () => { + it("should validate valid project names", () => { + const schema = AtlasArgs.projectName(); + const validNames = ["my-project", "project_1", "Project123", "test-project-2", "my_project_name"]; + + validNames.forEach((name) => { + expect(schema.parse(name)).toBe(name); + }); + }); + + it("should reject invalid project names", () => { + const schema = AtlasArgs.projectName(); + expect(() => schema.parse("")).toThrow("Project name is required"); + expect(() => schema.parse("a".repeat(65))).toThrow("Project name must be 64 characters or less"); + expect(() => schema.parse("invalid@name")).toThrow( + "Project name can only contain letters, numbers, hyphens, and underscores" + ); + }); + }); + }); + + describe("Edge Cases and Security", () => { + it("should handle empty strings appropriately", () => { + const schema = CommonArgs.string(); + expect(schema.parse("")).toBe(""); + + // But AtlasArgs validators should reject empty strings + expect(() => AtlasArgs.clusterName().parse("")).toThrow(); + expect(() => AtlasArgs.username().parse("")).toThrow(); + }); + + it("should handle very long strings", () => { + const schema = CommonArgs.string(); + const longString = "a".repeat(10000); + expect(schema.parse(longString)).toBe(longString); + + // But AtlasArgs validators should enforce length limits + expect(() => AtlasArgs.clusterName().parse("a".repeat(65))).toThrow(); + expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow(); + }); + + it("should handle null and undefined values", () => { + const schema = CommonArgs.string(); + expect(() => schema.parse(null)).toThrow(); + expect(() => schema.parse(undefined)).toThrow(); + }); + }); + + describe("Error Messages", () => { + it("should provide clear error messages for validation failures", () => { + // Test specific error messages + expect(() => AtlasArgs.clusterName().parse("")).toThrow("Cluster name is required"); + expect(() => AtlasArgs.clusterName().parse("a".repeat(65))).toThrow( + "Cluster name must be 64 characters or less" + ); + expect(() => AtlasArgs.clusterName().parse("invalid@name")).toThrow( + "Cluster name can only contain letters, numbers, hyphens, and underscores" + ); + + expect(() => AtlasArgs.username().parse("")).toThrow("Username is required"); + expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow( + "Username must be 100 characters or less" + ); + expect(() => AtlasArgs.username().parse("invalid name")).toThrow( + "Username can only contain letters, numbers, dots, hyphens, and underscores" + ); + }); + }); +}); From d4fec262ca7601dbf8b70f5175f318e8bac9bb2c Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 10 Sep 2025 21:43:41 +0100 Subject: [PATCH 04/15] add more limits --- src/tools/args.ts | 8 ++++---- tests/unit/args.test.ts | 30 +++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index a92feeffd..7d8ed1d7b 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -41,8 +41,8 @@ export const AtlasArgs = { cidrBlock: (): z.ZodString => CommonArgs.string().cidr(), region: (): z.ZodString => - CommonArgs.string().regex( - /^[a-zA-Z0-9_-]+$/, - "Region can only contain letters, numbers, hyphens, and underscores" - ), + CommonArgs.string() + .min(1, "Region is required") + .max(50, "Region must be 50 characters or less") + .regex(/^[a-zA-Z0-9_-]+$/, "Region can only contain letters, numbers, hyphens, and underscores"), }; diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 1a3fbfdba..30b6478dc 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { AtlasArgs, CommonArgs } from "../../src/tools/args.js"; -describe("Atlas Validators", () => { +describe("Tool args", () => { describe("CommonArgs", () => { describe("string", () => { it("should return a ZodString schema", () => { @@ -19,9 +19,24 @@ describe("Atlas Validators", () => { it("should not allow special characters and unicode symbols", () => { const schema = CommonArgs.string(); + + // Unicode characters expect(() => schema.parse("héllo")).toThrow(); expect(() => schema.parse("测试")).toThrow(); + expect(() => schema.parse("café")).toThrow(); + + // Emojis expect(() => schema.parse("🚀")).toThrow(); + expect(() => schema.parse("hello😀")).toThrow(); + + // Control characters (below ASCII 32) + expect(() => schema.parse("hello\nworld")).toThrow(); + expect(() => schema.parse("hello\tworld")).toThrow(); + expect(() => schema.parse("hello\0world")).toThrow(); + + // Extended ASCII characters (above ASCII 126) + expect(() => schema.parse("hello\x80")).toThrow(); + expect(() => schema.parse("hello\xFF")).toThrow(); }); it("should reject non-string values", () => { @@ -246,9 +261,22 @@ describe("Atlas Validators", () => { }); }); + it("should accept exactly 50 characters", () => { + const schema = AtlasArgs.region(); + const maxLengthRegion = "a".repeat(50); + expect(schema.parse(maxLengthRegion)).toBe(maxLengthRegion); + }); + it("should reject invalid region names", () => { const schema = AtlasArgs.region(); + // Empty string + expect(() => schema.parse("")).toThrow("Region is required"); + + // Too long (over 50 characters) + const longRegion = "a".repeat(51); + expect(() => schema.parse(longRegion)).toThrow("Region must be 50 characters or less"); + // Invalid characters expect(() => schema.parse("US EAST 1")).toThrow( "Region can only contain letters, numbers, hyphens, and underscores" From db75e4f171df4771d83042ad0d2c359d4c46d718 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 09:18:02 +0100 Subject: [PATCH 05/15] update passwords --- src/tools/args.ts | 7 +++++++ src/tools/atlas/create/createDBUser.ts | 3 +-- tests/unit/args.test.ts | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index 7d8ed1d7b..a37aa38c2 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -45,4 +45,11 @@ export const AtlasArgs = { .min(1, "Region is required") .max(50, "Region must be 50 characters or less") .regex(/^[a-zA-Z0-9_-]+$/, "Region can only contain letters, numbers, hyphens, and underscores"), + + password: (): z.ZodString => + CommonArgs.string() + .min(1, "Password is required") + .max(100, "Password must be 100 characters or less") + .regex(/^[^/]*$/, "String cannot contain '/'") + .regex(/^[a-zA-Z0-9._-]+$/, "Password can only contain letters, numbers, dots, hyphens, and underscores"), }; diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index 04fb7f164..b8c71c5c7 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -13,8 +13,7 @@ export const CreateDBUserArgs = { // Models will generate overly simplistic passwords like SecurePassword123 or // AtlasPassword123, which are easily guessable and exploitable. We're instructing // the model not to try and generate anything and instead leave the field unset. - password: z - .string() + password: AtlasArgs.password() .optional() .nullable() .describe( diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 30b6478dc..ff7e31e4e 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -309,6 +309,26 @@ describe("Tool args", () => { ); }); }); + + describe("password", () => { + it("should validate valid passwords", () => { + const schema = AtlasArgs.password().optional(); + const validPasswords = ["password123", "password_123", "Password123", "test-password-123"]; + validPasswords.forEach((password) => { + expect(schema.parse(password)).toBe(password); + }); + expect(schema.parse(undefined)).toBeUndefined(); + }); + + it("should reject invalid passwords", () => { + const schema = AtlasArgs.password(); + expect(() => schema.parse("")).toThrow("Password is required"); + expect(() => schema.parse("a".repeat(101))).toThrow("Password must be 100 characters or less"); + expect(() => schema.parse("invalid password")).toThrow( + "Password can only contain letters, numbers, dots, hyphens, and underscores" + ); + }); + }); }); describe("Edge Cases and Security", () => { From 2f99c5a36db01c7ca5302968ff5614b6a422bcee Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 11:56:11 +0100 Subject: [PATCH 06/15] address copilot comments --- src/tools/args.ts | 12 ++++++++---- tests/unit/args.test.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index a37aa38c2..448503f84 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -1,5 +1,9 @@ import { z, type ZodString } from "zod"; +// Shared validation constants +const NO_SLASH_REGEX = /^[^/]*$/; +const NO_SLASH_ERROR = "String cannot contain '/'"; + export const CommonArgs = { string: (): ZodString => z.string().regex(/^[\x20-\x7E]*$/, "String cannot contain special characters or Unicode symbols"), @@ -19,21 +23,21 @@ export const AtlasArgs = { CommonArgs.string() .min(1, "Cluster name is required") .max(64, "Cluster name must be 64 characters or less") - .regex(/^[^/]*$/, "String cannot contain '/'") + .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) .regex(/^[a-zA-Z0-9_-]+$/, "Cluster name can only contain letters, numbers, hyphens, and underscores"), projectName: (): z.ZodString => CommonArgs.string() .min(1, "Project name is required") .max(64, "Project name must be 64 characters or less") - .regex(/^[^/]*$/, "String cannot contain '/'") + .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) .regex(/^[a-zA-Z0-9_-]+$/, "Project name can only contain letters, numbers, hyphens, and underscores"), username: (): z.ZodString => CommonArgs.string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") - .regex(/^[^/]*$/, "String cannot contain '/'") + .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) .regex(/^[a-zA-Z0-9._-]+$/, "Username can only contain letters, numbers, dots, hyphens, and underscores"), ipAddress: (): z.ZodString => CommonArgs.string().ip({ version: "v4" }), @@ -50,6 +54,6 @@ export const AtlasArgs = { CommonArgs.string() .min(1, "Password is required") .max(100, "Password must be 100 characters or less") - .regex(/^[^/]*$/, "String cannot contain '/'") + .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) .regex(/^[a-zA-Z0-9._-]+$/, "Password can only contain letters, numbers, dots, hyphens, and underscores"), }; diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index ff7e31e4e..a266c1ecc 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -81,7 +81,7 @@ describe("Tool args", () => { ); }); - it("should not faile if the value is optional", () => { + it("should not fail if the value is optional", () => { const schema = AtlasArgs.objectId("Custom Field").optional(); expect(schema.parse(undefined)).toBeUndefined(); }); From 9374570ee0e8809f1608245e64ed07d475af7568 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 12:08:20 +0100 Subject: [PATCH 07/15] update clusters args with clusterName --- src/tools/atlas/create/createDBUser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/atlas/create/createDBUser.ts b/src/tools/atlas/create/createDBUser.ts index b8c71c5c7..18d22d358 100644 --- a/src/tools/atlas/create/createDBUser.ts +++ b/src/tools/atlas/create/createDBUser.ts @@ -29,7 +29,7 @@ export const CreateDBUserArgs = { ) .describe("Roles for the new user"), clusters: z - .array(CommonArgs.string()) + .array(AtlasArgs.clusterName()) .describe("Clusters to assign the user to, leave empty for access to all clusters") .optional(), }; From 4db1b1f25e9e03eb172d109145ed1f324162aa99 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 14:44:54 +0100 Subject: [PATCH 08/15] unify regex and update rules --- src/tools/args.ts | 57 ++++++++++++++----------- tests/unit/args.test.ts | 92 +++++++++++++++++++++++++++-------------- 2 files changed, 95 insertions(+), 54 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index 448503f84..fd0903394 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -1,17 +1,29 @@ import { z, type ZodString } from "zod"; -// Shared validation constants -const NO_SLASH_REGEX = /^[^/]*$/; -const NO_SLASH_ERROR = "String cannot contain '/'"; +const NO_UNICODE_REGEX = /^[\x20-\x7E]*$/; +const NO_UNICODE_ERROR = "String cannot contain special characters or Unicode symbols"; +const ALLOWED_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9._-]+$/; +export const ALLOWED_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; + +const ALLLOWED_REGION_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; +export const ALLOWED_REGION_CHARACTERS_ERROR = "Region can only contain letters, numbers, hyphens, and underscores"; + +const ALLOWED_CLUSTER_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; +export const ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR = + "Cluster names can only contain ASCII letters, numbers, and hyphens."; + +const ALLOWED_PROJECT_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9\s()@&+:._',-]+$/; +export const ALLOWED_PROJECT_NAME_CHARACTERS_ERROR = + "Project names can't be longer than 64 characters and can only contain letters, numbers, spaces, and the following symbols: ( ) @ & + : . _ - ' ,"; export const CommonArgs = { - string: (): ZodString => - z.string().regex(/^[\x20-\x7E]*$/, "String cannot contain special characters or Unicode symbols"), + string: (): ZodString => z.string().regex(NO_UNICODE_REGEX, NO_UNICODE_ERROR), }; export const AtlasArgs = { objectId: (fieldName: string): z.ZodString => - CommonArgs.string() + z + .string() .min(1, `${fieldName} is required`) .regex(/^[0-9a-fA-F]{24}$/, `${fieldName} must be a valid 24-character hexadecimal string`), @@ -20,40 +32,37 @@ export const AtlasArgs = { organizationId: (): z.ZodString => AtlasArgs.objectId("organizationId"), clusterName: (): z.ZodString => - CommonArgs.string() + z + .string() .min(1, "Cluster name is required") .max(64, "Cluster name must be 64 characters or less") - .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) - .regex(/^[a-zA-Z0-9_-]+$/, "Cluster name can only contain letters, numbers, hyphens, and underscores"), + .regex(ALLOWED_CLUSTER_NAME_CHARACTERS_REGEX, "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR), projectName: (): z.ZodString => - CommonArgs.string() + z + .string() .min(1, "Project name is required") .max(64, "Project name must be 64 characters or less") - .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) - .regex(/^[a-zA-Z0-9_-]+$/, "Project name can only contain letters, numbers, hyphens, and underscores"), + .regex(ALLOWED_PROJECT_NAME_CHARACTERS_REGEX, ALLOWED_PROJECT_NAME_CHARACTERS_ERROR), username: (): z.ZodString => - CommonArgs.string() + z + .string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") - .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) - .regex(/^[a-zA-Z0-9._-]+$/, "Username can only contain letters, numbers, dots, hyphens, and underscores"), + .regex(ALLOWED_NAME_CHARACTERS_REGEX, "Username " + ALLOWED_CHARACTERS_ERROR), - ipAddress: (): z.ZodString => CommonArgs.string().ip({ version: "v4" }), + ipAddress: (): z.ZodString => z.string().ip({ version: "v4" }), - cidrBlock: (): z.ZodString => CommonArgs.string().cidr(), + cidrBlock: (): z.ZodString => z.string().cidr(), region: (): z.ZodString => - CommonArgs.string() + z + .string() .min(1, "Region is required") .max(50, "Region must be 50 characters or less") - .regex(/^[a-zA-Z0-9_-]+$/, "Region can only contain letters, numbers, hyphens, and underscores"), + .regex(ALLLOWED_REGION_CHARACTERS_REGEX, ALLOWED_REGION_CHARACTERS_ERROR), password: (): z.ZodString => - CommonArgs.string() - .min(1, "Password is required") - .max(100, "Password must be 100 characters or less") - .regex(NO_SLASH_REGEX, NO_SLASH_ERROR) - .regex(/^[a-zA-Z0-9._-]+$/, "Password can only contain letters, numbers, dots, hyphens, and underscores"), + z.string().min(1, "Password is required").max(100, "Password must be 100 characters or less"), }; diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index a266c1ecc..677c57630 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -1,5 +1,12 @@ import { describe, expect, it } from "vitest"; -import { AtlasArgs, CommonArgs } from "../../src/tools/args.js"; +import { + AtlasArgs, + CommonArgs, + ALLOWED_PROJECT_NAME_CHARACTERS_ERROR, + ALLOWED_CHARACTERS_ERROR, + ALLOWED_REGION_CHARACTERS_ERROR, + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR, +} from "../../src/tools/args.js"; describe("Tool args", () => { describe("CommonArgs", () => { @@ -104,6 +111,13 @@ describe("Tool args", () => { expect(() => schema.parse("invalid")).toThrow( "projectId must be a valid 24-character hexadecimal string" ); + expect(() => schema.parse("507f1f77bc*86cd79943901")).toThrow( + "projectId must be a valid 24-character hexadecimal string" + ); + expect(() => schema.parse("")).toThrow("projectId is required"); + expect(() => schema.parse("507f1f77/bcf86cd799439011")).toThrow( + "projectId must be a valid 24-character hexadecimal string" + ); }); }); @@ -144,13 +158,17 @@ describe("Tool args", () => { // Invalid characters expect(() => schema.parse("cluster@name")).toThrow( - "Cluster name can only contain letters, numbers, hyphens, and underscores" + "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR ); expect(() => schema.parse("cluster name")).toThrow( - "Cluster name can only contain letters, numbers, hyphens, and underscores" + "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR ); expect(() => schema.parse("cluster.name")).toThrow( - "Cluster name can only contain letters, numbers, hyphens, and underscores" + "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR + ); + + expect(() => schema.parse("cluster/name")).toThrow( + "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR ); }); @@ -182,12 +200,8 @@ describe("Tool args", () => { expect(() => schema.parse(longUsername)).toThrow("Username must be 100 characters or less"); // Invalid characters - expect(() => schema.parse("user@name")).toThrow( - "Username can only contain letters, numbers, dots, hyphens, and underscores" - ); - expect(() => schema.parse("user name")).toThrow( - "Username can only contain letters, numbers, dots, hyphens, and underscores" - ); + expect(() => schema.parse("user@name")).toThrow("Username " + ALLOWED_CHARACTERS_ERROR); + expect(() => schema.parse("user name")).toThrow("Username " + ALLOWED_CHARACTERS_ERROR); }); it("should accept exactly 100 characters", () => { @@ -278,22 +292,32 @@ describe("Tool args", () => { expect(() => schema.parse(longRegion)).toThrow("Region must be 50 characters or less"); // Invalid characters - expect(() => schema.parse("US EAST 1")).toThrow( - "Region can only contain letters, numbers, hyphens, and underscores" - ); - expect(() => schema.parse("US.EAST.1")).toThrow( - "Region can only contain letters, numbers, hyphens, and underscores" - ); - expect(() => schema.parse("US@EAST#1")).toThrow( - "Region can only contain letters, numbers, hyphens, and underscores" - ); + expect(() => schema.parse("US EAST 1")).toThrow(ALLOWED_REGION_CHARACTERS_ERROR); + expect(() => schema.parse("US.EAST.1")).toThrow(ALLOWED_REGION_CHARACTERS_ERROR); + expect(() => schema.parse("US@EAST#1")).toThrow(ALLOWED_REGION_CHARACTERS_ERROR); }); }); describe("projectName", () => { it("should validate valid project names", () => { const schema = AtlasArgs.projectName(); - const validNames = ["my-project", "project_1", "Project123", "test-project-2", "my_project_name"]; + const validNames = [ + "my-project", + "project_1", + "Project123", + "test-project-2", + "my_project_name", + "project with spaces", + "project(with)parentheses", + "project@with@at", + "project&with&ersand", + "project+with+plus", + "project:with:colon", + "project.with.dots", + "project'with'apostrophe", + "project,with,comma", + "complex project (with) @all &symbols+here:test.name'value,", + ]; validNames.forEach((name) => { expect(schema.parse(name)).toBe(name); @@ -302,11 +326,24 @@ describe("Tool args", () => { it("should reject invalid project names", () => { const schema = AtlasArgs.projectName(); + + // Empty string expect(() => schema.parse("")).toThrow("Project name is required"); + + // Too long (over 64 characters) expect(() => schema.parse("a".repeat(65))).toThrow("Project name must be 64 characters or less"); - expect(() => schema.parse("invalid@name")).toThrow( - "Project name can only contain letters, numbers, hyphens, and underscores" - ); + + // Invalid characters not in the allowed set + expect(() => schema.parse("project#with#hash")).toThrow(ALLOWED_PROJECT_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("project$with$dollar")).toThrow(ALLOWED_PROJECT_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("project!with!exclamation")).toThrow(ALLOWED_PROJECT_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("project[with]brackets")).toThrow(ALLOWED_PROJECT_NAME_CHARACTERS_ERROR); + }); + + it("should accept exactly 64 characters", () => { + const schema = AtlasArgs.projectName(); + const maxLengthName = "a".repeat(64); + expect(schema.parse(maxLengthName)).toBe(maxLengthName); }); }); @@ -324,9 +361,6 @@ describe("Tool args", () => { const schema = AtlasArgs.password(); expect(() => schema.parse("")).toThrow("Password is required"); expect(() => schema.parse("a".repeat(101))).toThrow("Password must be 100 characters or less"); - expect(() => schema.parse("invalid password")).toThrow( - "Password can only contain letters, numbers, dots, hyphens, and underscores" - ); }); }); }); @@ -366,16 +400,14 @@ describe("Tool args", () => { "Cluster name must be 64 characters or less" ); expect(() => AtlasArgs.clusterName().parse("invalid@name")).toThrow( - "Cluster name can only contain letters, numbers, hyphens, and underscores" + "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR ); expect(() => AtlasArgs.username().parse("")).toThrow("Username is required"); expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow( "Username must be 100 characters or less" ); - expect(() => AtlasArgs.username().parse("invalid name")).toThrow( - "Username can only contain letters, numbers, dots, hyphens, and underscores" - ); + expect(() => AtlasArgs.username().parse("invalid name")).toThrow("Username " + ALLOWED_CHARACTERS_ERROR); }); }); }); From 9526a4de5c6803c5570a9fffe035036132c45157 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 15:23:54 +0100 Subject: [PATCH 09/15] address comment: use length in objectId --- src/tools/args.ts | 3 ++- tests/unit/args.test.ts | 16 +++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index fd0903394..d0aae1419 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -25,7 +25,8 @@ export const AtlasArgs = { z .string() .min(1, `${fieldName} is required`) - .regex(/^[0-9a-fA-F]{24}$/, `${fieldName} must be a valid 24-character hexadecimal string`), + .length(24, `${fieldName} must be exactly 24 characters`) + .regex(/^[0-9a-fA-F]+$/, `${fieldName} must contain only hexadecimal characters`), projectId: (): z.ZodString => AtlasArgs.objectId("projectId"), diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 677c57630..6af827940 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -83,9 +83,7 @@ describe("Tool args", () => { it("should provide custom field name in error messages", () => { const schema = AtlasArgs.objectId("Custom Field"); - expect(() => schema.parse("invalid")).toThrow( - "Custom Field must be a valid 24-character hexadecimal string" - ); + expect(() => schema.parse("invalid")).toThrow("Custom Field must be exactly 24 characters"); }); it("should not fail if the value is optional", () => { @@ -108,15 +106,13 @@ describe("Tool args", () => { it("should reject invalid project IDs", () => { const schema = AtlasArgs.projectId(); - expect(() => schema.parse("invalid")).toThrow( - "projectId must be a valid 24-character hexadecimal string" - ); + expect(() => schema.parse("invalid")).toThrow("projectId must be exactly 24 characters"); expect(() => schema.parse("507f1f77bc*86cd79943901")).toThrow( - "projectId must be a valid 24-character hexadecimal string" + "projectId must contain only hexadecimal characters" ); expect(() => schema.parse("")).toThrow("projectId is required"); expect(() => schema.parse("507f1f77/bcf86cd799439011")).toThrow( - "projectId must be a valid 24-character hexadecimal string" + "projectId must contain only hexadecimal characters" ); }); }); @@ -130,9 +126,7 @@ describe("Tool args", () => { it("should reject invalid organization IDs", () => { const schema = AtlasArgs.organizationId(); - expect(() => schema.parse("invalid")).toThrow( - "organizationId must be a valid 24-character hexadecimal string" - ); + expect(() => schema.parse("invalid")).toThrow("organizationId must be exactly 24 characters"); }); }); From 8a90fd6411f09989a4a4403cc7c4a6679ea9af2e Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 15:32:05 +0100 Subject: [PATCH 10/15] address comment: move objectId to common args --- src/tools/args.ts | 8 ++++---- tests/unit/args.test.ts | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index d0aae1419..9618d4b00 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -18,19 +18,19 @@ export const ALLOWED_PROJECT_NAME_CHARACTERS_ERROR = "Project names can't be longer than 64 characters and can only contain letters, numbers, spaces, and the following symbols: ( ) @ & + : . _ - ' ,"; export const CommonArgs = { string: (): ZodString => z.string().regex(NO_UNICODE_REGEX, NO_UNICODE_ERROR), -}; -export const AtlasArgs = { objectId: (fieldName: string): z.ZodString => z .string() .min(1, `${fieldName} is required`) .length(24, `${fieldName} must be exactly 24 characters`) .regex(/^[0-9a-fA-F]+$/, `${fieldName} must contain only hexadecimal characters`), +}; - projectId: (): z.ZodString => AtlasArgs.objectId("projectId"), +export const AtlasArgs = { + projectId: (): z.ZodString => CommonArgs.objectId("projectId"), - organizationId: (): z.ZodString => AtlasArgs.objectId("organizationId"), + organizationId: (): z.ZodString => CommonArgs.objectId("organizationId"), clusterName: (): z.ZodString => z diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 6af827940..b950799e9 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -54,18 +54,16 @@ describe("Tool args", () => { expect(() => schema.parse({})).toThrow(); }); }); - }); - describe("AtlasArgs", () => { describe("objectId", () => { it("should validate 24-character hexadecimal strings", () => { - const schema = AtlasArgs.objectId("Test ID"); + const schema = CommonArgs.objectId("Test ID"); const validId = "507f1f77bcf86cd799439011"; expect(schema.parse(validId)).toBe(validId); }); it("should reject invalid ObjectId formats", () => { - const schema = AtlasArgs.objectId("Test ID"); + const schema = CommonArgs.objectId("Test ID"); // Too short expect(() => schema.parse("507f1f77bcf86cd79943901")).toThrow(); @@ -82,21 +80,23 @@ describe("Tool args", () => { }); it("should provide custom field name in error messages", () => { - const schema = AtlasArgs.objectId("Custom Field"); + const schema = CommonArgs.objectId("Custom Field"); expect(() => schema.parse("invalid")).toThrow("Custom Field must be exactly 24 characters"); }); it("should not fail if the value is optional", () => { - const schema = AtlasArgs.objectId("Custom Field").optional(); + const schema = CommonArgs.objectId("Custom Field").optional(); expect(schema.parse(undefined)).toBeUndefined(); }); it("should not fail if the value is empty", () => { - const schema = AtlasArgs.objectId("Custom Field"); + const schema = CommonArgs.objectId("Custom Field"); expect(() => schema.parse(undefined)).toThrow("Required"); }); }); + }); + describe("AtlasArgs", () => { describe("projectId", () => { it("should validate project IDs", () => { const schema = AtlasArgs.projectId(); From b988118cffe65cb16f4bf3ce076504b5f3f5a7cc Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 16:20:37 +0100 Subject: [PATCH 11/15] address comment: fix typo and add unicode error validation --- src/tools/args.ts | 6 +++--- tests/unit/args.test.ts | 21 +++++++++++---------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index 9618d4b00..92a5cb616 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -1,12 +1,12 @@ import { z, type ZodString } from "zod"; const NO_UNICODE_REGEX = /^[\x20-\x7E]*$/; -const NO_UNICODE_ERROR = "String cannot contain special characters or Unicode symbols"; +export const NO_UNICODE_ERROR = "String cannot contain special characters or Unicode symbols"; const ALLOWED_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9._-]+$/; export const ALLOWED_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; -const ALLLOWED_REGION_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; +const ALLOWED_REGION_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; export const ALLOWED_REGION_CHARACTERS_ERROR = "Region can only contain letters, numbers, hyphens, and underscores"; const ALLOWED_CLUSTER_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; @@ -62,7 +62,7 @@ export const AtlasArgs = { .string() .min(1, "Region is required") .max(50, "Region must be 50 characters or less") - .regex(ALLLOWED_REGION_CHARACTERS_REGEX, ALLOWED_REGION_CHARACTERS_ERROR), + .regex(ALLOWED_REGION_CHARACTERS_REGEX, ALLOWED_REGION_CHARACTERS_ERROR), password: (): z.ZodString => z.string().min(1, "Password is required").max(100, "Password must be 100 characters or less"), diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index b950799e9..2dcaf13ed 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -6,6 +6,7 @@ import { ALLOWED_CHARACTERS_ERROR, ALLOWED_REGION_CHARACTERS_ERROR, ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR, + NO_UNICODE_ERROR, } from "../../src/tools/args.js"; describe("Tool args", () => { @@ -28,22 +29,22 @@ describe("Tool args", () => { const schema = CommonArgs.string(); // Unicode characters - expect(() => schema.parse("héllo")).toThrow(); - expect(() => schema.parse("测试")).toThrow(); - expect(() => schema.parse("café")).toThrow(); + expect(() => schema.parse("héllo")).toThrow(NO_UNICODE_ERROR); + expect(() => schema.parse("测试")).toThrow(NO_UNICODE_ERROR); + expect(() => schema.parse("café")).toThrow(NO_UNICODE_ERROR); // Emojis - expect(() => schema.parse("🚀")).toThrow(); - expect(() => schema.parse("hello😀")).toThrow(); + expect(() => schema.parse("🚀")).toThrow(NO_UNICODE_ERROR); + expect(() => schema.parse("hello😀")).toThrow(NO_UNICODE_ERROR); // Control characters (below ASCII 32) - expect(() => schema.parse("hello\nworld")).toThrow(); - expect(() => schema.parse("hello\tworld")).toThrow(); - expect(() => schema.parse("hello\0world")).toThrow(); + expect(() => schema.parse("hello\nworld")).toThrow(NO_UNICODE_ERROR); + expect(() => schema.parse("hello\tworld")).toThrow(NO_UNICODE_ERROR); + expect(() => schema.parse("hello\0world")).toThrow(NO_UNICODE_ERROR); // Extended ASCII characters (above ASCII 126) - expect(() => schema.parse("hello\x80")).toThrow(); - expect(() => schema.parse("hello\xFF")).toThrow(); + expect(() => schema.parse("hello\x80")).toThrow(NO_UNICODE_ERROR); + expect(() => schema.parse("hello\xFF")).toThrow(NO_UNICODE_ERROR); }); it("should reject non-string values", () => { From 0b9c32f7cec2b6c4cbb7b8203488cc2e90ec0920 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 16:22:10 +0100 Subject: [PATCH 12/15] address comment: rename allowed name chars --- src/tools/args.ts | 4 ++-- tests/unit/args.test.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index 92a5cb616..f7e07e019 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -4,7 +4,7 @@ const NO_UNICODE_REGEX = /^[\x20-\x7E]*$/; export const NO_UNICODE_ERROR = "String cannot contain special characters or Unicode symbols"; const ALLOWED_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9._-]+$/; -export const ALLOWED_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; +export const ALLOWED_NAME_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; const ALLOWED_REGION_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; export const ALLOWED_REGION_CHARACTERS_ERROR = "Region can only contain letters, numbers, hyphens, and underscores"; @@ -51,7 +51,7 @@ export const AtlasArgs = { .string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") - .regex(ALLOWED_NAME_CHARACTERS_REGEX, "Username " + ALLOWED_CHARACTERS_ERROR), + .regex(ALLOWED_NAME_CHARACTERS_REGEX, "Username " + ALLOWED_NAME_CHARACTERS_ERROR), ipAddress: (): z.ZodString => z.string().ip({ version: "v4" }), diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 2dcaf13ed..aa8249c21 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -3,7 +3,7 @@ import { AtlasArgs, CommonArgs, ALLOWED_PROJECT_NAME_CHARACTERS_ERROR, - ALLOWED_CHARACTERS_ERROR, + ALLOWED_NAME_CHARACTERS_ERROR, ALLOWED_REGION_CHARACTERS_ERROR, ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR, NO_UNICODE_ERROR, @@ -195,8 +195,8 @@ describe("Tool args", () => { expect(() => schema.parse(longUsername)).toThrow("Username must be 100 characters or less"); // Invalid characters - expect(() => schema.parse("user@name")).toThrow("Username " + ALLOWED_CHARACTERS_ERROR); - expect(() => schema.parse("user name")).toThrow("Username " + ALLOWED_CHARACTERS_ERROR); + expect(() => schema.parse("user@name")).toThrow("Username " + ALLOWED_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("user name")).toThrow("Username " + ALLOWED_NAME_CHARACTERS_ERROR); }); it("should accept exactly 100 characters", () => { @@ -402,7 +402,7 @@ describe("Tool args", () => { expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow( "Username must be 100 characters or less" ); - expect(() => AtlasArgs.username().parse("invalid name")).toThrow("Username " + ALLOWED_CHARACTERS_ERROR); + expect(() => AtlasArgs.username().parse("invalid name")).toThrow("Username " + ALLOWED_NAME_CHARACTERS_ERROR); }); }); }); From 91b1caab36f11bea6e96afa1ed5566fec1071e39 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 16:24:04 +0100 Subject: [PATCH 13/15] address comment: update to allowed username --- src/tools/args.ts | 6 +++--- tests/unit/args.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index f7e07e019..ac48dc96e 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -3,8 +3,8 @@ import { z, type ZodString } from "zod"; const NO_UNICODE_REGEX = /^[\x20-\x7E]*$/; export const NO_UNICODE_ERROR = "String cannot contain special characters or Unicode symbols"; -const ALLOWED_NAME_CHARACTERS_REGEX = /^[a-zA-Z0-9._-]+$/; -export const ALLOWED_NAME_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; +const ALLOWED_USERNAME_CHARACTERS_REGEX = /^[a-zA-Z0-9._-]+$/; +export const ALLOWED_USERNAME_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; const ALLOWED_REGION_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; export const ALLOWED_REGION_CHARACTERS_ERROR = "Region can only contain letters, numbers, hyphens, and underscores"; @@ -51,7 +51,7 @@ export const AtlasArgs = { .string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") - .regex(ALLOWED_NAME_CHARACTERS_REGEX, "Username " + ALLOWED_NAME_CHARACTERS_ERROR), + .regex(ALLOWED_USERNAME_CHARACTERS_REGEX, "Username " + ALLOWED_USERNAME_CHARACTERS_ERROR), ipAddress: (): z.ZodString => z.string().ip({ version: "v4" }), diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index aa8249c21..8f32d44ad 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -3,7 +3,7 @@ import { AtlasArgs, CommonArgs, ALLOWED_PROJECT_NAME_CHARACTERS_ERROR, - ALLOWED_NAME_CHARACTERS_ERROR, + ALLOWED_USERNAME_CHARACTERS_ERROR, ALLOWED_REGION_CHARACTERS_ERROR, ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR, NO_UNICODE_ERROR, @@ -195,8 +195,8 @@ describe("Tool args", () => { expect(() => schema.parse(longUsername)).toThrow("Username must be 100 characters or less"); // Invalid characters - expect(() => schema.parse("user@name")).toThrow("Username " + ALLOWED_NAME_CHARACTERS_ERROR); - expect(() => schema.parse("user name")).toThrow("Username " + ALLOWED_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("user@name")).toThrow("Username " + ALLOWED_USERNAME_CHARACTERS_ERROR); + expect(() => schema.parse("user name")).toThrow("Username " + ALLOWED_USERNAME_CHARACTERS_ERROR); }); it("should accept exactly 100 characters", () => { @@ -402,7 +402,7 @@ describe("Tool args", () => { expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow( "Username must be 100 characters or less" ); - expect(() => AtlasArgs.username().parse("invalid name")).toThrow("Username " + ALLOWED_NAME_CHARACTERS_ERROR); + expect(() => AtlasArgs.username().parse("invalid name")).toThrow("Username " + ALLOWED_USERNAME_CHARACTERS_ERROR); }); }); }); From e24937aeb55f3b5da1cf0b5e4e74f8c0f0788316 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 16:31:37 +0100 Subject: [PATCH 14/15] address comment: update error messages --- src/tools/args.ts | 7 ++++--- tests/unit/args.test.ts | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tools/args.ts b/src/tools/args.ts index ac48dc96e..165f3da0d 100644 --- a/src/tools/args.ts +++ b/src/tools/args.ts @@ -4,7 +4,8 @@ const NO_UNICODE_REGEX = /^[\x20-\x7E]*$/; export const NO_UNICODE_ERROR = "String cannot contain special characters or Unicode symbols"; const ALLOWED_USERNAME_CHARACTERS_REGEX = /^[a-zA-Z0-9._-]+$/; -export const ALLOWED_USERNAME_CHARACTERS_ERROR = " can only contain letters, numbers, dots, hyphens, and underscores"; +export const ALLOWED_USERNAME_CHARACTERS_ERROR = + "Username can only contain letters, numbers, dots, hyphens, and underscores"; const ALLOWED_REGION_CHARACTERS_REGEX = /^[a-zA-Z0-9_-]+$/; export const ALLOWED_REGION_CHARACTERS_ERROR = "Region can only contain letters, numbers, hyphens, and underscores"; @@ -37,7 +38,7 @@ export const AtlasArgs = { .string() .min(1, "Cluster name is required") .max(64, "Cluster name must be 64 characters or less") - .regex(ALLOWED_CLUSTER_NAME_CHARACTERS_REGEX, "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR), + .regex(ALLOWED_CLUSTER_NAME_CHARACTERS_REGEX, ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR), projectName: (): z.ZodString => z @@ -51,7 +52,7 @@ export const AtlasArgs = { .string() .min(1, "Username is required") .max(100, "Username must be 100 characters or less") - .regex(ALLOWED_USERNAME_CHARACTERS_REGEX, "Username " + ALLOWED_USERNAME_CHARACTERS_ERROR), + .regex(ALLOWED_USERNAME_CHARACTERS_REGEX, ALLOWED_USERNAME_CHARACTERS_ERROR), ipAddress: (): z.ZodString => z.string().ip({ version: "v4" }), diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 8f32d44ad..6680daa35 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -402,7 +402,9 @@ describe("Tool args", () => { expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow( "Username must be 100 characters or less" ); - expect(() => AtlasArgs.username().parse("invalid name")).toThrow("Username " + ALLOWED_USERNAME_CHARACTERS_ERROR); + expect(() => AtlasArgs.username().parse("invalid name")).toThrow( + "Username " + ALLOWED_USERNAME_CHARACTERS_ERROR + ); }); }); }); From 8dc0359e79eb5aa887161308f3671cef696931df Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Thu, 11 Sep 2025 16:37:07 +0100 Subject: [PATCH 15/15] fix tests --- tests/unit/args.test.ts | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/tests/unit/args.test.ts b/tests/unit/args.test.ts index 6680daa35..d7a5a1eb4 100644 --- a/tests/unit/args.test.ts +++ b/tests/unit/args.test.ts @@ -152,19 +152,10 @@ describe("Tool args", () => { expect(() => schema.parse(longName)).toThrow("Cluster name must be 64 characters or less"); // Invalid characters - expect(() => schema.parse("cluster@name")).toThrow( - "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR - ); - expect(() => schema.parse("cluster name")).toThrow( - "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR - ); - expect(() => schema.parse("cluster.name")).toThrow( - "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR - ); - - expect(() => schema.parse("cluster/name")).toThrow( - "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR - ); + expect(() => schema.parse("cluster@name")).toThrow(ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("cluster name")).toThrow(ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("cluster.name")).toThrow(ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR); + expect(() => schema.parse("cluster/name")).toThrow(ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR); }); it("should accept exactly 64 characters", () => { @@ -195,8 +186,8 @@ describe("Tool args", () => { expect(() => schema.parse(longUsername)).toThrow("Username must be 100 characters or less"); // Invalid characters - expect(() => schema.parse("user@name")).toThrow("Username " + ALLOWED_USERNAME_CHARACTERS_ERROR); - expect(() => schema.parse("user name")).toThrow("Username " + ALLOWED_USERNAME_CHARACTERS_ERROR); + expect(() => schema.parse("user@name")).toThrow(ALLOWED_USERNAME_CHARACTERS_ERROR); + expect(() => schema.parse("user name")).toThrow(ALLOWED_USERNAME_CHARACTERS_ERROR); }); it("should accept exactly 100 characters", () => { @@ -394,17 +385,13 @@ describe("Tool args", () => { expect(() => AtlasArgs.clusterName().parse("a".repeat(65))).toThrow( "Cluster name must be 64 characters or less" ); - expect(() => AtlasArgs.clusterName().parse("invalid@name")).toThrow( - "Cluster name " + ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR - ); + expect(() => AtlasArgs.clusterName().parse("invalid@name")).toThrow(ALLOWED_CLUSTER_NAME_CHARACTERS_ERROR); expect(() => AtlasArgs.username().parse("")).toThrow("Username is required"); expect(() => AtlasArgs.username().parse("a".repeat(101))).toThrow( "Username must be 100 characters or less" ); - expect(() => AtlasArgs.username().parse("invalid name")).toThrow( - "Username " + ALLOWED_USERNAME_CHARACTERS_ERROR - ); + expect(() => AtlasArgs.username().parse("invalid name")).toThrow(ALLOWED_USERNAME_CHARACTERS_ERROR); }); }); });