diff --git a/README.md b/README.md index 7f30410b2..b61353bc4 100644 --- a/README.md +++ b/README.md @@ -341,6 +341,7 @@ The MongoDB MCP Server can be configured using multiple methods, with the follow 1. Command-line arguments 2. Environment variables +3. Configuration File ### Configuration Options @@ -562,6 +563,57 @@ For a full list of roles and their privileges, see the [Atlas User Roles documen ### Configuration Methods +#### Configuration File + +Store configuration in a JSON file and load it using the `MDB_MCP_CONFIG` environment variable. + +> **🔒 Security Best Practice:** Prefer using the `MDB_MCP_CONFIG` environment variable for sensitive fields over the configuration file or `--config` CLI argument. Command-line arguments are visible in process listings. + +> **🔒 File Security:** Ensure your configuration file has proper ownership and permissions, limited to the user running the MongoDB MCP server: +> +> **Linux/macOS:** +> +> ```bash +> chmod 600 /path/to/config.json +> chown your-username /path/to/config.json +> ``` +> +> **Windows:** Right-click the file → Properties → Security → Restrict access to your user account only. + +Create a JSON file with your configuration (all keys use camelCase): + +```json +{ + "connectionString": "mongodb://localhost:27017", + "readOnly": true, + "loggers": ["stderr", "mcp"], + "apiClientId": "your-atlas-service-accounts-client-id", + "apiClientSecret": "your-atlas-service-accounts-client-secret", + "maxDocumentsPerQuery": 100 +} +``` + +**Linux/macOS (bash/zsh):** + +```bash +export MDB_MCP_CONFIG="/path/to/config.json" +npx -y mongodb-mcp-server@latest +``` + +**Windows Command Prompt (cmd):** + +```cmd +set "MDB_MCP_CONFIG=C:\path\to\config.json" +npx -y mongodb-mcp-server@latest +``` + +**Windows PowerShell:** + +```powershell +$env:MDB_MCP_CONFIG="C:\path\to\config.json" +npx -y mongodb-mcp-server@latest +``` + #### Environment Variables Set environment variables with the prefix `MDB_MCP_` followed by the option name in uppercase with underscores: diff --git a/src/common/config/createUserConfig.ts b/src/common/config/createUserConfig.ts index 8ed825e71..b04739840 100644 --- a/src/common/config/createUserConfig.ts +++ b/src/common/config/createUserConfig.ts @@ -74,6 +74,10 @@ function parseUserConfigSources(cliArguments: string[]): { args: cliArguments, schema: UserConfigSchema, parserOptions: { + // This is the name of key that yargs-parser will look up in CLI + // arguments (--config) and ENV variables (MDB_MCP_CONFIG) to load an + // initial configuration from. + config: "config", // This helps parse the relevant environment variables. envPrefix: "MDB_MCP_", configuration: { diff --git a/src/common/config/userConfig.ts b/src/common/config/userConfig.ts index 3b672c5c0..045788295 100644 --- a/src/common/config/userConfig.ts +++ b/src/common/config/userConfig.ts @@ -178,7 +178,7 @@ const ServerConfigSchema = z4.object({ .describe( "API key for Voyage AI embeddings service (required for vector search operations with text-to-embedding conversion)." ) - .register(configRegistry, { isSecret: true, overrideBehavior: "override" }), + .register(configRegistry, { isSecret: true, overrideBehavior: "not-allowed" }), embeddingsValidation: z4 .preprocess(parseBoolean, z4.boolean()) .default(true) diff --git a/tests/fixtures/config-with-invalid-value.json b/tests/fixtures/config-with-invalid-value.json new file mode 100644 index 000000000..128b3d181 --- /dev/null +++ b/tests/fixtures/config-with-invalid-value.json @@ -0,0 +1,4 @@ +{ + "connectionString": "mongodb://invalid-value-json-localhost:1000", + "loggers": "stderr,stderr" +} diff --git a/tests/fixtures/valid-config.json b/tests/fixtures/valid-config.json new file mode 100644 index 000000000..e364276c9 --- /dev/null +++ b/tests/fixtures/valid-config.json @@ -0,0 +1,4 @@ +{ + "connectionString": "mongodb://valid-json-localhost:1000", + "loggers": "stderr" +} diff --git a/tests/integration/server.test.ts b/tests/integration/server.test.ts index a1ccba1fb..7a2af0c0b 100644 --- a/tests/integration/server.test.ts +++ b/tests/integration/server.test.ts @@ -1,6 +1,64 @@ +import { MCPConnectionManager } from "../../src/common/connectionManager.js"; +import { ExportsManager } from "../../src/common/exportsManager.js"; +import { CompositeLogger } from "../../src/common/logger.js"; +import { DeviceId } from "../../src/helpers/deviceId.js"; +import { Session } from "../../src/common/session.js"; import { defaultTestConfig, expectDefined } from "./helpers.js"; import { describeWithMongoDB } from "./tools/mongodb/mongodbHelpers.js"; -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; +import { Elicitation, Keychain, Telemetry } from "../../src/lib.js"; +import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; +import { defaultCreateAtlasLocalClient } from "../../src/common/atlasLocal.js"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { Server } from "../../src/server.js"; +import { connectionErrorHandler } from "../../src/common/connectionErrorHandler.js"; +import { type OperationType, ToolBase, type ToolCategory, type ToolClass } from "../../src/tools/tool.js"; +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import type { TelemetryToolMetadata } from "../../src/telemetry/types.js"; +import { InMemoryTransport } from "../../src/transports/inMemoryTransport.js"; +import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; + +class TestToolOne extends ToolBase { + public name = "test-tool-one"; + protected description = "A test tool one for verification tests"; + static category: ToolCategory = "mongodb"; + static operationType: OperationType = "delete"; + protected argsShape = {}; + protected async execute(): Promise { + return Promise.resolve({ + content: [ + { + type: "text", + text: "Test tool one executed successfully", + }, + ], + }); + } + protected resolveTelemetryMetadata(): TelemetryToolMetadata { + return {}; + } +} + +class TestToolTwo extends ToolBase { + public name = "test-tool-two"; + protected description = "A test tool two for verification tests"; + static category: ToolCategory = "mongodb"; + static operationType: OperationType = "delete"; + protected argsShape = {}; + protected async execute(): Promise { + return Promise.resolve({ + content: [ + { + type: "text", + text: "Test tool two executed successfully", + }, + ], + }); + } + protected resolveTelemetryMetadata(): TelemetryToolMetadata { + return {}; + } +} describe("Server integration test", () => { describeWithMongoDB( @@ -84,7 +142,7 @@ describe("Server integration test", () => { // Check that non-read tools are NOT available expect(tools.tools.some((tool) => tool.name === "insert-many")).toBe(false); expect(tools.tools.some((tool) => tool.name === "update-many")).toBe(false); - expect(tools.tools.some((tool) => tool.name === "delete-one")).toBe(false); + expect(tools.tools.some((tool) => tool.name === "delete-many")).toBe(false); expect(tools.tools.some((tool) => tool.name === "drop-collection")).toBe(false); }); }, @@ -97,4 +155,64 @@ describe("Server integration test", () => { }), } ); + + describe("with additional tools", () => { + const initServerWithTools = async (tools: ToolClass[]): Promise<{ server: Server; transport: Transport }> => { + const logger = new CompositeLogger(); + const deviceId = DeviceId.create(logger); + const connectionManager = new MCPConnectionManager(defaultTestConfig, logger, deviceId); + const exportsManager = ExportsManager.init(defaultTestConfig, logger); + + const session = new Session({ + userConfig: defaultTestConfig, + logger, + exportsManager, + connectionManager, + keychain: Keychain.root, + vectorSearchEmbeddingsManager: new VectorSearchEmbeddingsManager(defaultTestConfig, connectionManager), + atlasLocalClient: await defaultCreateAtlasLocalClient(), + }); + + const telemetry = Telemetry.create(session, defaultTestConfig, deviceId); + const mcpServerInstance = new McpServer({ name: "test", version: "1.0" }); + const elicitation = new Elicitation({ server: mcpServerInstance.server }); + + const server = new Server({ + session, + userConfig: defaultTestConfig, + telemetry, + mcpServer: mcpServerInstance, + elicitation, + connectionErrorHandler, + tools: [...tools], + }); + + const transport = new InMemoryTransport(); + + return { transport, server }; + }; + + let server: Server | undefined; + let transport: Transport | undefined; + + afterEach(async () => { + await transport?.close(); + }); + + it("should start server with only the tools provided", async () => { + ({ server, transport } = await initServerWithTools([TestToolOne])); + await server.connect(transport); + expect(server.tools).toHaveLength(1); + }); + + it("should throw error before starting when provided tools have name conflict", async () => { + ({ server, transport } = await initServerWithTools([ + TestToolOne, + class TestToolTwoButOne extends TestToolTwo { + public name = "test-tool-one"; + }, + ])); + await expect(server.connect(transport)).rejects.toThrow(/Tool test-tool-one is already registered/); + }); + }); }); diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index fc6f25324..5e4806458 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -10,6 +10,7 @@ import { import { Keychain } from "../../../src/common/keychain.js"; import type { Secret } from "../../../src/common/keychain.js"; import { createEnvironment } from "../../utils/index.js"; +import path from "path"; // Expected hardcoded values (what we had before) const expectedDefaults = { @@ -50,6 +51,11 @@ const expectedDefaults = { allowRequestOverrides: false, }; +const CONFIG_FIXTURES = { + VALID: path.resolve(import.meta.dirname, "..", "..", "fixtures", "valid-config.json"), + WITH_INVALID_VALUE: path.resolve(import.meta.dirname, "..", "..", "fixtures", "config-with-invalid-value.json"), +}; + describe("config", () => { it("should generate defaults from UserConfigSchema that match expected values", () => { expect(UserConfigSchema.parse({})).toStrictEqual(expectedDefaults); @@ -528,6 +534,53 @@ describe("config", () => { }); }); + describe("loading a config file", () => { + describe("through env variable MDB_MCP_CONFIG", () => { + const { setVariable, clearVariables } = createEnvironment(); + afterEach(() => { + clearVariables(); + }); + + it("should load a valid config file without troubles", () => { + setVariable("MDB_MCP_CONFIG", CONFIG_FIXTURES.VALID); + const { warnings, error, parsed } = createUserConfig({ args: [] }); + expect(warnings).toHaveLength(0); + expect(error).toBeUndefined(); + + expect(parsed?.connectionString).toBe("mongodb://valid-json-localhost:1000"); + expect(parsed?.loggers).toStrictEqual(["stderr"]); + }); + + it("should attempt loading config file with wrong value and exit", () => { + setVariable("MDB_MCP_CONFIG", CONFIG_FIXTURES.WITH_INVALID_VALUE); + const { warnings, error, parsed } = createUserConfig({ args: [] }); + expect(warnings).toHaveLength(0); + expect(error).toEqual(expect.stringContaining("loggers - Duplicate loggers found in config")); + expect(parsed).toBeUndefined(); + }); + }); + + describe("through cli argument --config", () => { + it("should load a valid config file without troubles", () => { + const { warnings, error, parsed } = createUserConfig({ args: ["--config", CONFIG_FIXTURES.VALID] }); + expect(warnings).toHaveLength(0); + expect(error).toBeUndefined(); + + expect(parsed?.connectionString).toBe("mongodb://valid-json-localhost:1000"); + expect(parsed?.loggers).toStrictEqual(["stderr"]); + }); + + it("should attempt loading config file with wrong value and exit", () => { + const { warnings, error, parsed } = createUserConfig({ + args: ["--config", CONFIG_FIXTURES.WITH_INVALID_VALUE], + }); + expect(warnings).toHaveLength(0); + expect(error).toEqual(expect.stringContaining("loggers - Duplicate loggers found in config")); + expect(parsed).toBeUndefined(); + }); + }); + }); + describe("precedence rules", () => { const { setVariable, clearVariables } = createEnvironment(); @@ -538,30 +591,34 @@ describe("config", () => { it("positional argument takes precedence over all", () => { setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://crazyhost1"); const { parsed: actual } = createUserConfig({ - args: ["mongodb://crazyhost2", "--connectionString", "mongodb://localhost"], + args: [ + "mongodb://crazyhost2", + "--config", + CONFIG_FIXTURES.VALID, + "--connectionString", + "mongodb://localhost", + ], }); expect(actual?.connectionString).toBe("mongodb://crazyhost2/?directConnection=true"); }); - it("cli arguments take precedence over env vars", () => { - setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://crazyhost"); - const { parsed: actual } = createUserConfig({ - args: ["--connectionString", "mongodb://localhost"], + it("any cli argument takes precedence over env vars, config and defaults", () => { + setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://dummyhost"); + const { parsed } = createUserConfig({ + args: ["--config", CONFIG_FIXTURES.VALID, "--connectionString", "mongodb://host-from-cli"], }); - expect(actual?.connectionString).toBe("mongodb://localhost"); + expect(parsed?.connectionString).toBe("mongodb://host-from-cli"); }); - it("any cli argument takes precedence over defaults", () => { - const { parsed: actual } = createUserConfig({ - args: ["--connectionString", "mongodb://localhost"], - }); - expect(actual?.connectionString).toBe("mongodb://localhost"); + it("any env var takes precedence over config and defaults", () => { + setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://dummyhost"); + const { parsed } = createUserConfig({ args: ["--config", CONFIG_FIXTURES.VALID] }); + expect(parsed?.connectionString).toBe("mongodb://dummyhost"); }); - it("any env var takes precedence over defaults", () => { - setVariable("MDB_MCP_CONNECTION_STRING", "mongodb://localhost"); - const { parsed: actual } = createUserConfig({ args: [] }); - expect(actual?.connectionString).toBe("mongodb://localhost"); + it("config file takes precedence over defaults", () => { + const { parsed } = createUserConfig({ args: ["--config", CONFIG_FIXTURES.VALID] }); + expect(parsed?.connectionString).toBe("mongodb://valid-json-localhost:1000"); }); }); diff --git a/tests/unit/common/config/configOverrides.test.ts b/tests/unit/common/config/configOverrides.test.ts index 0381f5a2c..14fb26fab 100644 --- a/tests/unit/common/config/configOverrides.test.ts +++ b/tests/unit/common/config/configOverrides.test.ts @@ -230,6 +230,7 @@ describe("configOverrides", () => { "maxDocumentsPerQuery", "exportsPath", "exportCleanupIntervalMs", + "voyageApiKey", "allowRequestOverrides", "dryRun", ]); @@ -252,24 +253,19 @@ describe("configOverrides", () => { }); describe("secret fields", () => { - it("should allow overriding secret fields with headers if they have override behavior", () => { - const request: RequestContext = { - headers: { - "x-mongodb-mcp-voyage-api-key": "test", - }, - }; - const result = applyConfigOverrides({ baseConfig: baseConfig as UserConfig, request }); - expect(result.voyageApiKey).toBe("test"); + const secretFields = Object.keys(UserConfigSchema.shape).filter((configKey) => { + const meta = getConfigMeta(configKey as keyof UserConfig); + return meta?.isSecret; }); - it("should not allow overriding secret fields via query params", () => { + it.each(secretFields)("should not allow overriding secret fields - $0", () => { const request: RequestContext = { - query: { - mongodbMcpVoyageApiKey: "test", + headers: { + "x-mongodb-mcp-voyage-api-key": "test", }, }; expect(() => applyConfigOverrides({ baseConfig: baseConfig as UserConfig, request })).toThrow( - "Config key voyageApiKey can only be overriden with headers" + "Config key voyageApiKey is not allowed to be overridden" ); }); });