fix: instruct models not to generate passwords for createDBUser (#177)

Author: nirinchev

src/common/atlas/generatePassword.ts

@@ -0,0 +1,10 @@
+import { randomBytes } from "crypto";
+import { promisify } from "util";
+
+const randomBytesAsync = promisify(randomBytes);
+
+export async function generateSecurePassword(): Promise<string> {
+    const buf = await randomBytesAsync(16);
+    const pass = buf.toString("base64url");
+    return pass;
+}

src/tools/atlas/create/createDBUser.ts

@@ -3,6 +3,7 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
 import { AtlasToolBase } from "../atlasTool.js";
 import { ToolArgs, OperationType } from "../../tool.js";
 import { CloudDatabaseUser, DatabaseUserRole } from "../../../common/atlas/openapi.js";
+import { generateSecurePassword } from "../../../common/atlas/generatePassword.js";
 
 export class CreateDBUserTool extends AtlasToolBase {
     protected name = "atlas-create-db-user";
@@ -11,7 +12,16 @@ export class CreateDBUserTool extends AtlasToolBase {
     protected argsShape = {
         projectId: z.string().describe("Atlas project ID"),
         username: z.string().describe("Username for the new user"),
-        password: z.string().describe("Password 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. 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({
@@ -34,6 +44,11 @@ export class CreateDBUserTool extends AtlasToolBase {
         roles,
         clusters,
     }: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> {
+        const shouldGeneratePassword = !password;
+        if (shouldGeneratePassword) {
+            password = await generateSecurePassword();
+        }
+
         const input = {
             groupId: projectId,
             awsIAMType: "NONE",
@@ -62,7 +77,12 @@ export class CreateDBUserTool extends AtlasToolBase {
         });
 
         return {
-            content: [{ type: "text", text: `User "${username}" created sucessfully.` }],
+            content: [
+                {
+                    type: "text",
+                    text: `User "${username}" created successfully${shouldGeneratePassword ? ` with password: \`${password}\`` : ""}.`,
+                },
+            ],
         };
     }
 }

src/tools/atlas/metadata/connectCluster.ts

@@ -2,24 +2,14 @@ import { z } from "zod";
 import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
 import { AtlasToolBase } from "../atlasTool.js";
 import { ToolArgs, OperationType } from "../../tool.js";
-import { randomBytes } from "crypto";
-import { promisify } from "util";
+import { generateSecurePassword } from "../../../common/atlas/generatePassword.js";
 import logger, { LogId } from "../../../logger.js";
 
 const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours
 
-const randomBytesAsync = promisify(randomBytes);
-
-async function generateSecurePassword(): Promise<string> {
-    const buf = await randomBytesAsync(16);
-    const pass = buf.toString("base64url");
-    return pass;
-}
-
 function sleep(ms: number): Promise<void> {
     return new Promise((resolve) => setTimeout(resolve, ms));
 }
-
 export class ConnectClusterTool extends AtlasToolBase {
     protected name = "atlas-connect-cluster";
     protected description = "Connect to MongoDB Atlas cluster";

tests/integration/helpers.ts

@@ -117,7 +117,7 @@ export function getResponseElements(content: unknown | { content: unknown }): {
         content = (content as { content: unknown }).content;
     }
 
-    expect(Array.isArray(content)).toBe(true);
+    expect(content).toBeArray();
 
     const response = content as { type: string; text: string }[];
     for (const item of response) {

tests/integration/tools/atlas/dbUsers.test.ts

@@ -1,24 +1,49 @@
 import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
-import { Session } from "../../../../src/session.js";
 import { describeWithAtlas, withProject, randomId } from "./atlasHelpers.js";
-import { expectDefined } from "../../helpers.js";
+import { expectDefined, getResponseElements } from "../../helpers.js";
+import { ApiClientError } from "../../../../src/common/atlas/apiClientError.js";
 
 describeWithAtlas("db users", (integration) => {
-    const userName = "testuser-" + randomId;
     withProject(integration, ({ getProjectId }) => {
-        afterAll(async () => {
-            const projectId = getProjectId();
+        let userName: string;
+        beforeEach(() => {
+            userName = "testuser-" + randomId;
+        });
 
-            const session: Session = integration.mcpServer().session;
-            await session.apiClient.deleteDatabaseUser({
-                params: {
-                    path: {
-                        groupId: projectId,
-                        username: userName,
-                        databaseName: "admin",
-                    },
+        const createUserWithMCP = async (password?: string): Promise<unknown> => {
+            return await integration.mcpClient().callTool({
+                name: "atlas-create-db-user",
+                arguments: {
+                    projectId: getProjectId(),
+                    username: userName,
+                    password,
+                    roles: [
+                        {
+                            roleName: "readWrite",
+                            databaseName: "admin",
+                        },
+                    ],
                 },
             });
+        };
+
+        afterEach(async () => {
+            try {
+                await integration.mcpServer().session.apiClient.deleteDatabaseUser({
+                    params: {
+                        path: {
+                            groupId: getProjectId(),
+                            username: userName,
+                            databaseName: "admin",
+                        },
+                    },
+                });
+            } catch (error) {
+                // Ignore 404 errors when deleting the user
+                if (!(error instanceof ApiClientError) || error.response?.status !== 404) {
+                    throw error;
+                }
+            }
         });
 
         describe("atlas-create-db-user", () => {
@@ -34,26 +59,24 @@ describeWithAtlas("db users", (integration) => {
                 expect(createDbUser.inputSchema.properties).toHaveProperty("roles");
                 expect(createDbUser.inputSchema.properties).toHaveProperty("clusters");
             });
-            it("should create a database user", async () => {
-                const projectId = getProjectId();
 
-                const response = (await integration.mcpClient().callTool({
-                    name: "atlas-create-db-user",
-                    arguments: {
-                        projectId,
-                        username: userName,
-                        password: "testpassword",
-                        roles: [
-                            {
-                                roleName: "readWrite",
-                                databaseName: "admin",
-                            },
-                        ],
-                    },
-                })) as CallToolResult;
-                expect(response.content).toBeArray();
-                expect(response.content).toHaveLength(1);
-                expect(response.content[0].text).toContain("created sucessfully");
+            it("should create a database user with supplied password", async () => {
+                const response = await createUserWithMCP("testpassword");
+
+                const elements = getResponseElements(response);
+                expect(elements).toHaveLength(1);
+                expect(elements[0].text).toContain("created successfully");
+                expect(elements[0].text).toContain(userName);
+                expect(elements[0].text).not.toContain("testpassword");
+            });
+
+            it("should create a database user with generated password", async () => {
+                const response = await createUserWithMCP();
+                const elements = getResponseElements(response);
+                expect(elements).toHaveLength(1);
+                expect(elements[0].text).toContain("created successfully");
+                expect(elements[0].text).toContain(userName);
+                expect(elements[0].text).toContain("with password: `");
             });
         });
         describe("atlas-list-db-users", () => {
@@ -68,6 +91,8 @@ describeWithAtlas("db users", (integration) => {
             it("returns database users by project", async () => {
                 const projectId = getProjectId();
 
+                await createUserWithMCP();
+
                 const response = (await integration
                     .mcpClient()
                     .callTool({ name: "atlas-list-db-users", arguments: { projectId } })) as CallToolResult;