From e65c0bd5432199544dbbf6452853b10ae0509626 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Thu, 7 Aug 2025 15:29:42 +0200 Subject: [PATCH 01/15] chore: add arg-parser and put the config under test This commit is biggish because it adds a big chunk of tests that weren't there before. While it would be arguably that testing every flag might be too much, because I'm changing how arguments are parsed, I want to have this as a safeline. --- package-lock.json | 45 ++++ package.json | 1 + src/common/config.ts | 186 ++++++++++++-- tests/unit/common/config.test.ts | 415 +++++++++++++++++++++++++++++++ 4 files changed, 633 insertions(+), 14 deletions(-) create mode 100644 tests/unit/common/config.test.ts diff --git a/package-lock.json b/package-lock.json index d6e7d7dd..87cc8dc5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "@mongodb-js/device-id": "^0.3.1", "@mongodb-js/devtools-connect": "^3.9.2", "@mongodb-js/devtools-proxy-support": "^0.5.1", + "@mongosh/arg-parser": "^3.14.0", "@mongosh/service-provider-node-driver": "^3.10.2", "@vitest/eslint-plugin": "^1.3.4", "bson": "^6.10.4", @@ -2274,6 +2275,29 @@ "node": ">=0.10.0" } }, + "node_modules/@mongosh/arg-parser": { + "version": "3.14.0", + "resolved": "https://registry.npmjs.org/@mongosh/arg-parser/-/arg-parser-3.14.0.tgz", + "integrity": "sha512-ue7FtuO9rgmjrh2lFZdgtrVMrGXeqBG8mWDDTb/CZ2GZNtULhAdk9d+gdnjupQNH66fGWoKdLhuur466HuJrtw==", + "license": "Apache-2.0", + "dependencies": { + "@mongosh/errors": "2.4.2", + "@mongosh/i18n": "^2.15.2", + "mongodb-connection-string-url": "^3.0.1" + }, + "engines": { + "node": ">=14.15.1" + } + }, + "node_modules/@mongosh/arg-parser/node_modules/@mongosh/errors": { + "version": "2.4.2", + "resolved": "https://registry.npmjs.org/@mongosh/errors/-/errors-2.4.2.tgz", + "integrity": "sha512-p+LOHVj/VIt6cpJY1AvDmG/QLP7WZQ7q+32paU6qxjXaoC0kMqmemaqK5cUj8JWod1VEv9/Ol4T6OfWTwPG20A==", + "license": "Apache-2.0", + "engines": { + "node": ">=14.15.1" + } + }, "node_modules/@mongosh/errors": { "version": "2.4.0", "resolved": "https://registry.npmjs.org/@mongosh/errors/-/errors-2.4.0.tgz", @@ -2283,6 +2307,27 @@ "node": ">=14.15.1" } }, + "node_modules/@mongosh/i18n": { + "version": "2.15.2", + "resolved": "https://registry.npmjs.org/@mongosh/i18n/-/i18n-2.15.2.tgz", + "integrity": "sha512-E286cGq9/Cgg1TjkOvuKG5ymmIZI/gUpXatK83Ulp9EbQ+tqSwDsv+A3Z+unFyRHYvFuTLVlBCXFyHRDBn8Nww==", + "license": "Apache-2.0", + "dependencies": { + "@mongosh/errors": "2.4.2" + }, + "engines": { + "node": ">=14.15.1" + } + }, + "node_modules/@mongosh/i18n/node_modules/@mongosh/errors": { + "version": "2.4.2", + "resolved": "https://registry.npmjs.org/@mongosh/errors/-/errors-2.4.2.tgz", + "integrity": "sha512-p+LOHVj/VIt6cpJY1AvDmG/QLP7WZQ7q+32paU6qxjXaoC0kMqmemaqK5cUj8JWod1VEv9/Ol4T6OfWTwPG20A==", + "license": "Apache-2.0", + "engines": { + "node": ">=14.15.1" + } + }, "node_modules/@mongosh/service-provider-core": { "version": "3.3.3", "resolved": "https://registry.npmjs.org/@mongosh/service-provider-core/-/service-provider-core-3.3.3.tgz", diff --git a/package.json b/package.json index 7bcd5a1e..7bba9bf6 100644 --- a/package.json +++ b/package.json @@ -94,6 +94,7 @@ "@mongodb-js/device-id": "^0.3.1", "@mongodb-js/devtools-connect": "^3.9.2", "@mongodb-js/devtools-proxy-support": "^0.5.1", + "@mongosh/arg-parser": "^3.14.0", "@mongosh/service-provider-node-driver": "^3.10.2", "@vitest/eslint-plugin": "^1.3.4", "bson": "^6.10.4", diff --git a/src/common/config.ts b/src/common/config.ts index 2367a3ad..4ea2c3f9 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -1,9 +1,104 @@ import path from "path"; import os from "os"; import argv from "yargs-parser"; - +import type { CliOptions } from "@mongosh/arg-parser"; import { ReadConcernLevel, ReadPreferenceMode, W } from "mongodb"; +// From: https://github.com/mongodb-js/mongosh/blob/main/packages/cli-repl/src/arg-parser.ts +const OPTIONS = { + string: [ + "apiBaseUrl", + "apiClientId", + "apiClientSecret", + "connectionString", + "httpHost", + "httpPort", + "idleTimeoutMs", + "logPath", + "notificationTimeoutMs", + "telemetry", + "transport", + "apiVersion", + "authenticationDatabase", + "authenticationMechanism", + "browser", + "db", + "gssapiHostName", + "gssapiServiceName", + "host", + "oidcFlows", + "oidcRedirectUri", + "password", + "port", + "sslCAFile", + "sslCRLFile", + "sslCertificateSelector", + "sslDisabledProtocols", + "sslPEMKeyFile", + "sslPEMKeyPassword", + "sspiHostnameCanonicalization", + "sspiRealmOverride", + "tlsCAFile", + "tlsCRLFile", + "tlsCertificateKeyFile", + "tlsCertificateKeyFilePassword", + "tlsCertificateSelector", + "tlsDisabledProtocols", + "username", + ], + boolean: [ + "apiDeprecationErrors", + "apiStrict", + "help", + "indexCheck", + "ipv6", + "nodb", + "oidcIdTokenAsAccessToken", + "oidcNoNonce", + "oidcTrustedEndpoint", + "readOnly", + "retryWrites", + "ssl", + "sslAllowInvalidCertificates", + "sslAllowInvalidHostnames", + "sslFIPSMode", + "tls", + "tlsAllowInvalidCertificates", + "tlsAllowInvalidHostnames", + "tlsFIPSMode", + "tlsUseSystemCA", + "version", + ], + array: ["disabledTools", "loggers"], + alias: { + h: "help", + p: "password", + u: "username", + "build-info": "buildInfo", + browser: "browser", + oidcDumpTokens: "oidcDumpTokens", + oidcRedirectUrl: "oidcRedirectUri", + oidcIDTokenAsAccessToken: "oidcIdTokenAsAccessToken", + }, + configuration: { + "camel-case-expansion": false, + "unknown-options-as-args": true, + "parse-positional-numbers": false, + "parse-numbers": false, + "greedy-arrays": true, + "short-option-groups": false, + }, +}; + +function isConnectionSpecifier(arg: string | undefined): boolean { + return ( + arg !== undefined && + (arg.startsWith("mongodb://") || + arg.startsWith("mongodb+srv://") || + !(arg.endsWith(".js") || arg.endsWith(".mongodb"))) + ); +} + export interface ConnectOptions { readConcern: ReadConcernLevel; readPreference: ReadPreferenceMode; @@ -13,7 +108,7 @@ export interface ConnectOptions { // If we decide to support non-string config options, we'll need to extend the mechanism for parsing // env variables. -export interface UserConfig { +export interface UserConfig extends CliOptions { apiBaseUrl: string; apiClientId?: string; apiClientSecret?: string; @@ -59,11 +154,11 @@ const defaults: UserConfig = { notificationTimeoutMs: 540000, // 9 minutes }; -export const config = { - ...defaults, - ...getEnvConfig(), - ...getCliConfig(), -}; +export const config = setupUserConfig({ + defaults, + cli: process.argv, + env: process.env, +}); function getLocalDataPath(): string { return process.platform === "win32" @@ -83,7 +178,7 @@ function getExportsPath(): string { // Gets the config supplied by the user as environment variables. The variable names // are prefixed with `MDB_MCP_` and the keys match the UserConfig keys, but are converted // to SNAKE_UPPER_CASE. -function getEnvConfig(): Partial { +function parseEnvConfig(env: Record): Partial { function setValue(obj: Record, path: string[], value: string): void { const currentField = path.shift(); if (!currentField) { @@ -120,7 +215,7 @@ function getEnvConfig(): Partial { } const result: Record = {}; - const mcpVariables = Object.entries(process.env).filter( + const mcpVariables = Object.entries(env).filter( ([key, value]) => value !== undefined && key.startsWith("MDB_MCP_") ) as [string, string][]; for (const [key, value] of mcpVariables) { @@ -139,9 +234,72 @@ function SNAKE_CASE_toCamelCase(str: string): string { return str.toLowerCase().replace(/([-_][a-z])/g, (group) => group.toUpperCase().replace("_", "")); } -// Reads the cli args and parses them into a UserConfig object. -function getCliConfig(): Partial { - return argv(process.argv.slice(2), { - array: ["disabledTools", "loggers"], - }) as unknown as Partial; +// Right now we have arguments that are not compatible with the format used in mongosh. +// An example is using --connectionString and positional arguments. +// We will consolidate them in a way where the mongosh format takes precedence. +// We will warn users that previous configuration is deprecated in favour of +// whatever is in mongosh. +function parseCliConfig(args: string[]): CliOptions { + const programArgs = args.slice(2); + const parsed = argv(programArgs, OPTIONS) as unknown as CliOptions & + UserConfig & { + _?: string[]; + }; + + const positionalArguments = parsed._ ?? []; + if (!parsed.nodb && isConnectionSpecifier(positionalArguments[0])) { + parsed.connectionSpecifier = positionalArguments.shift(); + } + + delete parsed._; + return parsed; +} + +function commaSeparatedToArray(str: string | string[] | undefined): T { + if (str === undefined) { + return [] as unknown as T; + } + + if (!Array.isArray(str)) { + return [str] as T; + } + + if (str.length === 0) { + return str as T; + } + + if (str.length === 1) { + if (str[0]?.indexOf(",") === -1) { + return str as T; + } else { + return str[0]?.split(",").map((e) => e.trim()) as T; + } + } + + return str as T; +} + +export function setupUserConfig({ + cli, + env, + defaults, +}: { + cli: string[]; + env: Record; + defaults: Partial; +}): UserConfig { + const userConfig: UserConfig = { + ...defaults, + ...parseEnvConfig(env), + ...parseCliConfig(cli), + } as UserConfig; + + userConfig.disabledTools = commaSeparatedToArray(userConfig.disabledTools); + userConfig.loggers = commaSeparatedToArray(userConfig.loggers); + + if (userConfig.connectionString && userConfig.connectionSpecifier) { + delete userConfig.connectionString; + } + + return userConfig; } diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts new file mode 100644 index 00000000..b9a27cea --- /dev/null +++ b/tests/unit/common/config.test.ts @@ -0,0 +1,415 @@ +import { describe, it, expect } from "vitest"; +import { setupUserConfig, UserConfig } from "../../../src/common/config.js"; + +describe.only("config", () => { + describe("env var parsing", () => { + describe("string cases", () => { + const testCases = { + MDB_MCP_API_BASE_URL: "apiBaseUrl", + MDB_MCP_API_CLIENT_ID: "apiClientId", + MDB_MCP_API_CLIENT_SECRET: "apiClientSecret", + MDB_MCP_TELEMETRY: "telemetry", + MDB_MCP_LOG_PATH: "logPath", + MDB_MCP_CONNECTION_STRING: "connectionString", + MDB_MCP_READ_ONLY: "readOnly", + MDB_MCP_INDEX_CHECK: "indexCheck", + MDB_MCP_TRANSPORT: "transport", + MDB_MCP_HTTP_PORT: "httpPort", + MDB_MCP_HTTP_HOST: "httpHost", + MDB_MCP_IDLE_TIMEOUT_MS: "idleTimeoutMs", + MDB_MCP_NOTIFICATION_TIMEOUT_MS: "notificationTimeoutMs", + } as const; + + for (const [envVar, config] of Object.entries(testCases)) { + it(`should map ${envVar} to ${config}`, () => { + const randomValue = "value=" + Math.random(); + + const actual = setupUserConfig({ + cli: [], + env: { + [envVar]: randomValue, + }, + defaults: {}, + }); + + expect(actual[config]).toBe(randomValue); + }); + } + }); + + describe("array cases", () => { + const testCases = { + MDB_MCP_DISABLED_TOOLS: "disabledTools", + MDB_MCP_LOGGERS: "loggers", + } as const; + + for (const [envVar, config] of Object.entries(testCases)) { + it(`should map ${envVar} to ${config}`, () => { + const randomValue = "value=" + Math.random(); + + const actual = setupUserConfig({ + cli: [], + env: { + [envVar]: randomValue, + }, + defaults: {}, + }); + + expect(actual[config]).toEqual([randomValue]); + }); + } + }); + }); + + describe("cli parsing", () => { + describe("string use cases", () => { + const testCases = [ + { + cli: ["--apiBaseUrl", "http://some-url.com"], + expected: { apiBaseUrl: "http://some-url.com" }, + }, + { + cli: ["--apiClientId", "OmgSoIdYeah"], + expected: { apiClientId: "OmgSoIdYeah" }, + }, + { + cli: ["--apiClientSecret", "OmgSoSecretYeah"], + expected: { apiClientSecret: "OmgSoSecretYeah" }, + }, + { + cli: ["--connectionString", "mongodb://localhost"], + expected: { connectionString: "mongodb://localhost" }, + }, + { + cli: ["--httpHost", "mongodb://localhost"], + expected: { httpHost: "mongodb://localhost" }, + }, + { + cli: ["--httpPort", "8080"], + expected: { httpPort: "8080" }, + }, + { + cli: ["--idleTimeoutMs", "42"], + expected: { idleTimeoutMs: "42" }, + }, + { + cli: ["--logPath", "/var/"], + expected: { logPath: "/var/" }, + }, + { + cli: ["--notificationTimeoutMs", "42"], + expected: { notificationTimeoutMs: "42" }, + }, + { + cli: ["--telemetry", "obviously"], + expected: { telemetry: "obviously" }, + }, + { + cli: ["--transport", "stdio"], + expected: { transport: "stdio" }, + }, + { + cli: ["--apiVersion", "1"], + expected: { apiVersion: "1" }, + }, + { + cli: ["--authenticationDatabase", "admin"], + expected: { authenticationDatabase: "admin" }, + }, + { + cli: ["--authenticationMechanism", "PLAIN"], + expected: { authenticationMechanism: "PLAIN" }, + }, + { + cli: ["--browser", "firefox"], + expected: { browser: "firefox" }, + }, + { + cli: ["--db", "test"], + expected: { db: "test" }, + }, + { + cli: ["--gssapiHostName", "localhost"], + expected: { gssapiHostName: "localhost" }, + }, + { + cli: ["--gssapiServiceName", "SERVICE"], + expected: { gssapiServiceName: "SERVICE" }, + }, + { + cli: ["--host", "localhost"], + expected: { host: "localhost" }, + }, + { + cli: ["--oidcFlows", "device"], + expected: { oidcFlows: "device" }, + }, + { + cli: ["--oidcRedirectUri", "https://oidc"], + expected: { oidcRedirectUri: "https://oidc" }, + }, + { + cli: ["--password", "123456"], + expected: { password: "123456" }, + }, + { + cli: ["--port", "27017"], + expected: { port: "27017" }, + }, + { + cli: ["--sslCAFile", "/var/file"], + expected: { sslCAFile: "/var/file" }, + }, + { + cli: ["--sslCRLFile", "/var/file"], + expected: { sslCRLFile: "/var/file" }, + }, + { + cli: ["--sslCertificateSelector", "pem=pom"], + expected: { sslCertificateSelector: "pem=pom" }, + }, + { + cli: ["--sslDisabledProtocols", "tls1"], + expected: { sslDisabledProtocols: "tls1" }, + }, + { + cli: ["--sslPEMKeyFile", "/var/pem"], + expected: { sslPEMKeyFile: "/var/pem" }, + }, + { + cli: ["--sslPEMKeyPassword", "654321"], + expected: { sslPEMKeyPassword: "654321" }, + }, + { + cli: ["--sspiHostnameCanonicalization", "true"], + expected: { sspiHostnameCanonicalization: "true" }, + }, + { + cli: ["--sspiRealmOverride", "OVER9000!"], + expected: { sspiRealmOverride: "OVER9000!" }, + }, + { + cli: ["--tlsCAFile", "/var/file"], + expected: { tlsCAFile: "/var/file" }, + }, + { + cli: ["--tlsCRLFile", "/var/file"], + expected: { tlsCRLFile: "/var/file" }, + }, + { + cli: ["--tlsCertificateKeyFile", "/var/file"], + expected: { tlsCertificateKeyFile: "/var/file" }, + }, + { + cli: ["--tlsCertificateKeyFilePassword", "4242"], + expected: { tlsCertificateKeyFilePassword: "4242" }, + }, + { + cli: ["--tlsCertificateSelector", "pom=pum"], + expected: { tlsCertificateSelector: "pom=pum" }, + }, + { + cli: ["--tlsDisabledProtocols", "tls1"], + expected: { tlsDisabledProtocols: "tls1" }, + }, + { + cli: ["--username", "admin"], + expected: { username: "admin" }, + }, + ] as { cli: string[]; expected: Partial }[]; + + for (const { cli, expected } of testCases) { + it(`should parse '${cli.join(" ")}' to ${JSON.stringify(expected)}`, () => { + const actual = setupUserConfig({ + cli: ["myself", "--", ...cli], + env: {}, + defaults: {}, + }); + + for (const [key, value] of Object.entries(expected)) { + expect(actual[key as keyof UserConfig]).toBe(value as unknown); + } + }); + } + }); + + describe("boolean use cases", () => { + const testCases = [ + { + cli: ["--apiDeprecationErrors"], + expected: { apiDeprecationErrors: true }, + }, + { + cli: ["--apiStrict"], + expected: { apiStrict: true }, + }, + { + cli: ["--help"], + expected: { help: true }, + }, + { + cli: ["--indexCheck"], + expected: { indexCheck: true }, + }, + { + cli: ["--ipv6"], + expected: { ipv6: true }, + }, + { + cli: ["--nodb"], + expected: { nodb: true }, + }, + { + cli: ["--oidcIdTokenAsAccessToken"], + expected: { oidcIdTokenAsAccessToken: true }, + }, + { + cli: ["--oidcNoNonce"], + expected: { oidcNoNonce: true }, + }, + { + cli: ["--oidcTrustedEndpoint"], + expected: { oidcTrustedEndpoint: true }, + }, + { + cli: ["--readOnly"], + expected: { readOnly: true }, + }, + { + cli: ["--retryWrites"], + expected: { retryWrites: true }, + }, + { + cli: ["--ssl"], + expected: { ssl: true }, + }, + { + cli: ["--sslAllowInvalidCertificates"], + expected: { sslAllowInvalidCertificates: true }, + }, + { + cli: ["--sslAllowInvalidHostnames"], + expected: { sslAllowInvalidHostnames: true }, + }, + { + cli: ["--sslFIPSMode"], + expected: { sslFIPSMode: true }, + }, + { + cli: ["--tls"], + expected: { tls: true }, + }, + { + cli: ["--tlsAllowInvalidCertificates"], + expected: { tlsAllowInvalidCertificates: true }, + }, + { + cli: ["--tlsAllowInvalidHostnames"], + expected: { tlsAllowInvalidHostnames: true }, + }, + { + cli: ["--tlsFIPSMode"], + expected: { tlsFIPSMode: true }, + }, + { + cli: ["--tlsUseSystemCA"], + expected: { tlsUseSystemCA: true }, + }, + { + cli: ["--version"], + expected: { version: true }, + }, + ] as { cli: string[]; expected: Partial }[]; + + for (const { cli, expected } of testCases) { + it(`should parse '${cli.join(" ")}' to ${JSON.stringify(expected)}`, () => { + const actual = setupUserConfig({ + cli: ["myself", "--", ...cli], + env: {}, + defaults: {}, + }); + + for (const [key, value] of Object.entries(expected)) { + expect(actual[key as keyof UserConfig]).toBe(value as unknown); + } + }); + } + }); + + describe("array use cases", () => { + const testCases = [ + { + cli: ["--disabledTools", "some,tool"], + expected: { disabledTools: ["some", "tool"] }, + }, + { + cli: ["--loggers", "canada,file"], + expected: { loggers: ["canada", "file"] }, + }, + ] as { cli: string[]; expected: Partial }[]; + + for (const { cli, expected } of testCases) { + it(`should parse '${cli.join(" ")}' to ${JSON.stringify(expected)}`, () => { + const actual = setupUserConfig({ + cli: ["myself", "--", ...cli], + env: {}, + defaults: {}, + }); + + for (const [key, value] of Object.entries(expected)) { + expect(actual[key as keyof UserConfig]).toEqual(value as unknown); + } + }); + } + }); + }); + + describe("precende rules", () => { + it("cli arguments take precedence over env vars", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--connectionString", "mongodb://localhost"], + env: { MDB_MCP_CONNECTION_STRING: "mongodb://crazyhost" }, + defaults: {}, + }); + + expect(actual.connectionString).toBe("mongodb://localhost"); + }); + + it("any cli argument takes precedence over defaults", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--connectionString", "mongodb://localhost"], + env: {}, + defaults: { + connectionString: "mongodb://crazyhost", + }, + }); + + expect(actual.connectionString).toBe("mongodb://localhost"); + }); + + it("any env var takes precedence over defaults", () => { + const actual = setupUserConfig({ + cli: [], + env: { MDB_MCP_CONNECTION_STRING: "mongodb://localhost" }, + defaults: { + connectionString: "mongodb://crazyhost", + }, + }); + + expect(actual.connectionString).toBe("mongodb://localhost"); + }); + }); + + describe("consolidation", () => { + it("positional argument for url has precedence over --connectionString", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "mongodb://localhost", "--connectionString", "toRemove"], + env: {}, + defaults: {}, + }); + + expect(actual.connectionString).toBe(undefined); + expect(actual.connectionSpecifier).toBe("mongodb://localhost"); + }); + }); +}); From 0c989c9168f43da0333a1c34437a4f7e890180f1 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 11:10:10 +0200 Subject: [PATCH 02/15] chore: load the systemCA and check for FIPS, help and version For help we are refering to the README.md, if we want something more fancy we porobably want to have better documentation generation so we don't maintain two files (here and the README.md) --- src/common/config.ts | 1 - src/index.ts | 46 ++++++++++++++++++++++++++++++++ tests/unit/common/config.test.ts | 12 +++------ 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index 4ea2c3f9..b857a30c 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -66,7 +66,6 @@ const OPTIONS = { "tlsAllowInvalidCertificates", "tlsAllowInvalidHostnames", "tlsFIPSMode", - "tlsUseSystemCA", "version", ], array: ["disabledTools", "loggers"], diff --git a/src/index.ts b/src/index.ts index 3fb2a744..5897ade9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,10 +2,19 @@ import { ConsoleLogger, LogId } from "./common/logger.js"; import { config } from "./common/config.js"; +import crypto from "crypto"; +import { packageInfo } from "./common/packageInfo.js"; import { StdioRunner } from "./transports/stdio.js"; import { StreamableHttpRunner } from "./transports/streamableHttp.js"; +import { systemCA } from "@mongodb-js/devtools-proxy-support"; async function main(): Promise { + systemCA().catch(() => undefined); // load system CA asynchronously as in mongosh + + assertFIPSMode(); + assertHelpMode(); + assertVersionMode(); + const transportRunner = config.transport === "stdio" ? new StdioRunner(config) : new StreamableHttpRunner(config); const shutdown = (): void => { @@ -78,3 +87,40 @@ main().catch((error: unknown) => { }); process.exit(1); }); + +function assertFIPSMode() { + let fipsError: Error | undefined = undefined; + if (config.tlsFIPSMode) { + if (!fipsError && !crypto.getFips()) { + fipsError = new Error("FIPS mode not enabled despite requested."); + } + } + + if (fipsError) { + if (process.config.variables.node_shared_openssl) { + console.error( + "Could not enable FIPS mode. Please ensure that your system OpenSSL installation supports FIPS." + ); + } else { + console.error("Could not enable FIPS mode. This installation does not appear to support FIPS."); + } + console.error("Error details:"); + console.error(fipsError); + process.exit(1); + } +} + +function assertHelpMode() { + if (!!config.help) { + console.log("For usage information refer to the README.md:"); + console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start"); + process.exit(0); + } +} + +function assertVersionMode() { + if (!!config.version) { + console.log(packageInfo.version); + process.exit(0); + } +} diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index b9a27cea..51403003 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from "vitest"; import { setupUserConfig, UserConfig } from "../../../src/common/config.js"; -describe.only("config", () => { +describe("config", () => { describe("env var parsing", () => { describe("string cases", () => { const testCases = { @@ -227,7 +227,7 @@ describe.only("config", () => { }); for (const [key, value] of Object.entries(expected)) { - expect(actual[key as keyof UserConfig]).toBe(value as unknown); + expect(actual[key as keyof UserConfig]).toBe(value); } }); } @@ -311,10 +311,6 @@ describe.only("config", () => { cli: ["--tlsFIPSMode"], expected: { tlsFIPSMode: true }, }, - { - cli: ["--tlsUseSystemCA"], - expected: { tlsUseSystemCA: true }, - }, { cli: ["--version"], expected: { version: true }, @@ -330,7 +326,7 @@ describe.only("config", () => { }); for (const [key, value] of Object.entries(expected)) { - expect(actual[key as keyof UserConfig]).toBe(value as unknown); + expect(actual[key as keyof UserConfig]).toBe(value); } }); } @@ -357,7 +353,7 @@ describe.only("config", () => { }); for (const [key, value] of Object.entries(expected)) { - expect(actual[key as keyof UserConfig]).toEqual(value as unknown); + expect(actual[key as keyof UserConfig]).toEqual(value); } }); } From 32ede21d014e37d3b580d807aa6f4d6c6406efad Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 11:34:19 +0200 Subject: [PATCH 03/15] chore: Inherit always preconfigured default driver options The driver options can not be sent by the agent, so we are going to use the ones specified in the CLI arguments / ENV Vars. This is relevant because new connections should use any inherited configuration like TLS settings or FIPS by default. --- src/common/config.ts | 42 ++++++++++++------- src/common/connectionManager.ts | 16 ++----- src/index.ts | 4 +- src/resources/common/config.ts | 3 +- src/server.ts | 1 - src/tools/atlas/connect/connectCluster.ts | 2 +- src/tools/mongodb/mongodbTool.ts | 2 +- .../common/connectionManager.test.ts | 5 --- .../tools/mongodb/connect/connect.test.ts | 1 - tests/unit/common/session.test.ts | 4 +- 10 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index b857a30c..c75aeb71 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -2,7 +2,7 @@ import path from "path"; import os from "os"; import argv from "yargs-parser"; import type { CliOptions } from "@mongosh/arg-parser"; -import { ReadConcernLevel, ReadPreferenceMode, W } from "mongodb"; +import type { ConnectionInfo } from "@mongosh/arg-parser"; // From: https://github.com/mongodb-js/mongosh/blob/main/packages/cli-repl/src/arg-parser.ts const OPTIONS = { @@ -98,13 +98,6 @@ function isConnectionSpecifier(arg: string | undefined): boolean { ); } -export interface ConnectOptions { - readConcern: ReadConcernLevel; - readPreference: ReadPreferenceMode; - writeConcern: W; - timeoutMS: number; -} - // If we decide to support non-string config options, we'll need to extend the mechanism for parsing // env variables. export interface UserConfig extends CliOptions { @@ -117,7 +110,6 @@ export interface UserConfig extends CliOptions { exportTimeoutMs: number; exportCleanupIntervalMs: number; connectionString?: string; - connectOptions: ConnectOptions; disabledTools: Array; readOnly?: boolean; indexCheck?: boolean; @@ -135,12 +127,6 @@ const defaults: UserConfig = { exportsPath: getExportsPath(), exportTimeoutMs: 300000, // 5 minutes exportCleanupIntervalMs: 120000, // 2 minutes - connectOptions: { - readConcern: "local", - readPreference: "secondaryPreferred", - writeConcern: "majority", - timeoutMS: 30_000, - }, disabledTools: [], telemetry: "enabled", readOnly: false, @@ -165,6 +151,19 @@ function getLocalDataPath(): string { : path.join(os.homedir(), ".mongodb"); } +export const defaultDriverOptions: ConnectionInfo["driverOptions"] = { + readConcern: { + level: "local", + }, + readPreference: "secondaryPreferred", + writeConcern: { + w: "majority", + }, + timeoutMS: 30_000, + proxy: { useEnvironmentVariableProxies: true }, + applyProxyToOIDC: true, +}; + function getLogPath(): string { const logPath = path.join(getLocalDataPath(), "mongodb-mcp", ".app-logs"); return logPath; @@ -302,3 +301,16 @@ export function setupUserConfig({ return userConfig; } + +/** + readConcern: { + level: settings.readConcern, + }, + readPreference: settings.readPreference, + writeConcern: { + w: settings.writeConcern, + }, + timeoutMS: settings.timeoutMS, + proxy: { useEnvironmentVariableProxies: true }, + applyProxyToOIDC: true, +**/ diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index db33b21b..5e154de2 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -1,4 +1,4 @@ -import { ConnectOptions } from "./config.js"; +import { defaultDriverOptions } from "./config.js"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import EventEmitter from "events"; import { setAppNameParamIfMissing } from "../helpers/connectionOptions.js"; @@ -14,7 +14,7 @@ export interface AtlasClusterConnectionInfo { expiryDate: Date; } -export interface ConnectionSettings extends ConnectOptions { +export interface ConnectionSettings { connectionString: string; atlas?: AtlasClusterConnectionInfo; } @@ -70,6 +70,7 @@ export class ConnectionManager extends EventEmitter { constructor() { super(); + this.state = { tag: "disconnected" }; } @@ -91,16 +92,7 @@ export class ConnectionManager extends EventEmitter { serviceProvider = await NodeDriverServiceProvider.connect(settings.connectionString, { productDocsLink: "https://github.com/mongodb-js/mongodb-mcp-server/", productName: "MongoDB MCP", - readConcern: { - level: settings.readConcern, - }, - readPreference: settings.readPreference, - writeConcern: { - w: settings.writeConcern, - }, - timeoutMS: settings.timeoutMS, - proxy: { useEnvironmentVariableProxies: true }, - applyProxyToOIDC: true, + ...defaultDriverOptions, }); } catch (error: unknown) { const errorReason = error instanceof Error ? error.message : `${error as string}`; diff --git a/src/index.ts b/src/index.ts index 5897ade9..e8e3bdfc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -111,7 +111,7 @@ function assertFIPSMode() { } function assertHelpMode() { - if (!!config.help) { + if (config.help) { console.log("For usage information refer to the README.md:"); console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start"); process.exit(0); @@ -119,7 +119,7 @@ function assertHelpMode() { } function assertVersionMode() { - if (!!config.version) { + if (config.version) { console.log(packageInfo.version); process.exit(0); } diff --git a/src/resources/common/config.ts b/src/resources/common/config.ts index 5a0570d4..d67b0400 100644 --- a/src/resources/common/config.ts +++ b/src/resources/common/config.ts @@ -1,4 +1,5 @@ import { ReactiveResource } from "../resource.js"; +import { defaultDriverOptions } from "../../common/config.js"; import type { UserConfig } from "../../common/config.js"; import type { Telemetry } from "../../telemetry/telemetry.js"; import type { Session } from "../../lib.js"; @@ -37,7 +38,7 @@ export class ConfigResource extends ReactiveResource { connectionString: this.current.connectionString ? "set; access to MongoDB tools are currently available to use" : "not set; before using any MongoDB tool, you need to configure a connection string, alternatively you can setup MongoDB Atlas access, more info at 'https://github.com/mongodb-js/mongodb-mcp-server'.", - connectOptions: this.current.connectOptions, + connectOptions: defaultDriverOptions, atlas: this.current.apiClientId && this.current.apiClientSecret ? "set; MongoDB Atlas tools are currently available to use" diff --git a/src/server.ts b/src/server.ts index cc8d30dd..d9d5d702 100644 --- a/src/server.ts +++ b/src/server.ts @@ -221,7 +221,6 @@ export class Server { try { await this.session.connectToMongoDB({ connectionString: this.userConfig.connectionString, - ...this.userConfig.connectOptions, }); } catch (error) { console.error( diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts index 2df76ae9..1653c3f6 100644 --- a/src/tools/atlas/connect/connectCluster.ts +++ b/src/tools/atlas/connect/connectCluster.ts @@ -139,7 +139,7 @@ export class ConnectClusterTool extends AtlasToolBase { try { lastError = undefined; - await this.session.connectToMongoDB({ connectionString, ...this.config.connectOptions, atlas }); + await this.session.connectToMongoDB({ connectionString, atlas }); break; } catch (err: unknown) { const error = err instanceof Error ? err : new Error(String(err)); diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index ca4a1349..2cd1a060 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -118,7 +118,7 @@ export abstract class MongoDBToolBase extends ToolBase { } protected connectToMongoDB(connectionString: string): Promise { - return this.session.connectToMongoDB({ connectionString, ...this.config.connectOptions }); + return this.session.connectToMongoDB({ connectionString }); } protected resolveTelemetryMetadata( diff --git a/tests/integration/common/connectionManager.test.ts b/tests/integration/common/connectionManager.test.ts index 25c88d9e..4536361a 100644 --- a/tests/integration/common/connectionManager.test.ts +++ b/tests/integration/common/connectionManager.test.ts @@ -6,7 +6,6 @@ import { } from "../../../src/common/connectionManager.js"; import { describeWithMongoDB } from "../tools/mongodb/mongodbHelpers.js"; import { describe, beforeEach, expect, it, vi, afterEach } from "vitest"; -import { config } from "../../../src/common/config.js"; describeWithMongoDB("Connection Manager", (integration) => { function connectionManager(): ConnectionManager { @@ -48,7 +47,6 @@ describeWithMongoDB("Connection Manager", (integration) => { await connectionManager().connect({ connectionString: integration.connectionString(), - ...integration.mcpServer().userConfig.connectOptions, }); }); @@ -88,7 +86,6 @@ describeWithMongoDB("Connection Manager", (integration) => { beforeEach(async () => { await connectionManager().connect({ connectionString: integration.connectionString(), - ...integration.mcpServer().userConfig.connectOptions, }); }); @@ -110,7 +107,6 @@ describeWithMongoDB("Connection Manager", (integration) => { try { await connectionManager().connect({ connectionString: "mongodb://localhost:xxxxx", - ...integration.mcpServer().userConfig.connectOptions, }); } catch (_error: unknown) { void _error; @@ -158,7 +154,6 @@ describe("Connection Manager connection type inference", () => { it(`infers ${connectionType} from ${connectionString}`, () => { const actualConnectionType = ConnectionManager.inferConnectionTypeFromSettings({ connectionString, - ...config.connectOptions, }); expect(actualConnectionType).toBe(connectionType); diff --git a/tests/integration/tools/mongodb/connect/connect.test.ts b/tests/integration/tools/mongodb/connect/connect.test.ts index 8e9d20f3..7dd275d3 100644 --- a/tests/integration/tools/mongodb/connect/connect.test.ts +++ b/tests/integration/tools/mongodb/connect/connect.test.ts @@ -15,7 +15,6 @@ describeWithMongoDB( beforeEach(async () => { await integration.mcpServer().session.connectToMongoDB({ connectionString: integration.connectionString(), - ...config.connectOptions, }); }); diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts index 1d26d8d8..991fa4e0 100644 --- a/tests/unit/common/session.test.ts +++ b/tests/unit/common/session.test.ts @@ -1,7 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { Session } from "../../../src/common/session.js"; -import { config } from "../../../src/common/config.js"; import { CompositeLogger } from "../../../src/common/logger.js"; import { ConnectionManager } from "../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../src/common/exportsManager.js"; @@ -52,7 +51,6 @@ describe("Session", () => { it(`should update connection string for ${testCase.name}`, async () => { await session.connectToMongoDB({ connectionString: testCase.connectionString, - ...config.connectOptions, }); expect(session.serviceProvider).toBeDefined(); @@ -68,7 +66,7 @@ describe("Session", () => { } it("should configure the proxy to use environment variables", async () => { - await session.connectToMongoDB({ connectionString: "mongodb://localhost", ...config.connectOptions }); + await session.connectToMongoDB({ connectionString: "mongodb://localhost" }); expect(session.serviceProvider).toBeDefined(); const connectMock = MockNodeDriverServiceProvider.connect; From 7b3e06376b514d7ed049643500881776d5f3d5a6 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 11:50:17 +0200 Subject: [PATCH 04/15] chore: generate the connection string properly on first connection This is necessary when connecting for the first time. --- src/common/config.ts | 20 ++++---------------- src/lib.ts | 2 +- tests/unit/common/config.test.ts | 5 ++++- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index c75aeb71..084bb9ae 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -1,8 +1,8 @@ import path from "path"; import os from "os"; import argv from "yargs-parser"; -import type { CliOptions } from "@mongosh/arg-parser"; -import type { ConnectionInfo } from "@mongosh/arg-parser"; +import type { CliOptions, ConnectionInfo } from "@mongosh/arg-parser"; +import { generateConnectionInfoFromCliArgs } from "@mongosh/arg-parser"; // From: https://github.com/mongodb-js/mongosh/blob/main/packages/cli-repl/src/arg-parser.ts const OPTIONS = { @@ -296,21 +296,9 @@ export function setupUserConfig({ userConfig.loggers = commaSeparatedToArray(userConfig.loggers); if (userConfig.connectionString && userConfig.connectionSpecifier) { - delete userConfig.connectionString; + const connectionInfo = generateConnectionInfoFromCliArgs(userConfig); + userConfig.connectionString = connectionInfo.connectionString; } return userConfig; } - -/** - readConcern: { - level: settings.readConcern, - }, - readPreference: settings.readPreference, - writeConcern: { - w: settings.writeConcern, - }, - timeoutMS: settings.timeoutMS, - proxy: { useEnvironmentVariableProxies: true }, - applyProxyToOIDC: true, -**/ diff --git a/src/lib.ts b/src/lib.ts index 773933ff..7843a9cd 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -1,4 +1,4 @@ export { Server, type ServerOptions } from "./server.js"; export { Telemetry } from "./telemetry/telemetry.js"; export { Session, type SessionOptions } from "./common/session.js"; -export type { UserConfig, ConnectOptions } from "./common/config.js"; +export type { UserConfig } from "./common/config.js"; diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index 51403003..088c4e73 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -404,7 +404,10 @@ describe("config", () => { defaults: {}, }); - expect(actual.connectionString).toBe(undefined); + // the shell specifies directConnection=true and serverSelectionTimeoutMS=2000 by default + expect(actual.connectionString).toBe( + "mongodb://localhost/?directConnection=true&serverSelectionTimeoutMS=2000" + ); expect(actual.connectionSpecifier).toBe("mongodb://localhost"); }); }); From 6bf0ec5dc16070ae2d30c2a328547b69e40c54e6 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 12:14:38 +0200 Subject: [PATCH 05/15] chore: some minor refactors and linter checks --- src/common/config.ts | 19 +++++++++++++++++++ src/common/connectionManager.ts | 4 ++-- src/index.ts | 6 +++--- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index 084bb9ae..56b3045e 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -164,6 +164,11 @@ export const defaultDriverOptions: ConnectionInfo["driverOptions"] = { applyProxyToOIDC: true, }; +export const driverOptions = setupDriverConfig({ + config, + defaults: defaultDriverOptions, +}); + function getLogPath(): string { const logPath = path.join(getLocalDataPath(), "mongodb-mcp", ".app-logs"); return logPath; @@ -302,3 +307,17 @@ export function setupUserConfig({ return userConfig; } + +export function setupDriverConfig({ + config, + defaults, +}: { + config: UserConfig; + defaults: ConnectionInfo["driverOptions"]; +}): ConnectionInfo["driverOptions"] { + const { driverOptions } = generateConnectionInfoFromCliArgs(config); + return { + ...defaults, + ...driverOptions, + }; +} diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index 5e154de2..6c2cb277 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -1,4 +1,4 @@ -import { defaultDriverOptions } from "./config.js"; +import { driverOptions } from "./config.js"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import EventEmitter from "events"; import { setAppNameParamIfMissing } from "../helpers/connectionOptions.js"; @@ -92,7 +92,7 @@ export class ConnectionManager extends EventEmitter { serviceProvider = await NodeDriverServiceProvider.connect(settings.connectionString, { productDocsLink: "https://github.com/mongodb-js/mongodb-mcp-server/", productName: "MongoDB MCP", - ...defaultDriverOptions, + ...driverOptions, }); } catch (error: unknown) { const errorReason = error instanceof Error ? error.message : `${error as string}`; diff --git a/src/index.ts b/src/index.ts index e8e3bdfc..72be8812 100644 --- a/src/index.ts +++ b/src/index.ts @@ -88,7 +88,7 @@ main().catch((error: unknown) => { process.exit(1); }); -function assertFIPSMode() { +function assertFIPSMode(): void | never { let fipsError: Error | undefined = undefined; if (config.tlsFIPSMode) { if (!fipsError && !crypto.getFips()) { @@ -110,7 +110,7 @@ function assertFIPSMode() { } } -function assertHelpMode() { +function assertHelpMode(): void | never { if (config.help) { console.log("For usage information refer to the README.md:"); console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start"); @@ -118,7 +118,7 @@ function assertHelpMode() { } } -function assertVersionMode() { +function assertVersionMode(): void | never { if (config.version) { console.log(packageInfo.version); process.exit(0); From bfc6a94d07b0410cbebf2d791f73d3fbbdd660d3 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 12:18:05 +0200 Subject: [PATCH 06/15] Update tests/unit/common/config.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/unit/common/config.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index 088c4e73..682b41ec 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -360,7 +360,7 @@ describe("config", () => { }); }); - describe("precende rules", () => { + describe("precedence rules", () => { it("cli arguments take precedence over env vars", () => { const actual = setupUserConfig({ cli: ["myself", "--", "--connectionString", "mongodb://localhost"], From d8f75dbdb6e6131842708a193e41bc8891dac6e9 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 12:59:24 +0200 Subject: [PATCH 07/15] chore: enable FIPS before doing anything else --- src/index.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 72be8812..df795479 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,22 @@ #!/usr/bin/env node +let fipsError: Error | undefined; +function enableFipsIfRequested(): void { + if (process.argv.includes("--tlsFIPSMode")) { + // FIPS mode should be enabled before we run any other code, including any dependencies. + // We still wrap this into a function so we can also call it immediately after + // entering the snapshot main function. + try { + // eslint-disable-next-line + require("crypto").setFips(1); + } catch (err: unknown) { + fipsError ??= err as Error; + } + } +} + +enableFipsIfRequested(); + import { ConsoleLogger, LogId } from "./common/logger.js"; import { config } from "./common/config.js"; import crypto from "crypto"; @@ -89,7 +106,6 @@ main().catch((error: unknown) => { }); function assertFIPSMode(): void | never { - let fipsError: Error | undefined = undefined; if (config.tlsFIPSMode) { if (!fipsError && !crypto.getFips()) { fipsError = new Error("FIPS mode not enabled despite requested."); From 3611cdf33589031ccf0f5d855f0e96c9c2e8aa23 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 14:34:45 +0200 Subject: [PATCH 08/15] chore: move config validation to the actual config --- src/common/config.ts | 33 ++++++++++++++- src/server.ts | 29 ------------- tests/unit/common/config.test.ts | 72 ++++++++++++++++---------------- 3 files changed, 66 insertions(+), 68 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index 56b3045e..33568a57 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -121,7 +121,7 @@ export interface UserConfig extends CliOptions { notificationTimeoutMs: number; } -const defaults: UserConfig = { +export const defaultUserConfig: UserConfig = { apiBaseUrl: "https://cloud.mongodb.com/", logPath: getLogPath(), exportsPath: getExportsPath(), @@ -140,7 +140,7 @@ const defaults: UserConfig = { }; export const config = setupUserConfig({ - defaults, + defaults: defaultUserConfig, cli: process.argv, env: process.env, }); @@ -305,6 +305,35 @@ export function setupUserConfig({ userConfig.connectionString = connectionInfo.connectionString; } + const transport = userConfig.transport as string; + if (transport !== "http" && transport !== "stdio") { + throw new Error(`Invalid transport: ${transport}`); + } + + const telemetry = userConfig.telemetry as string; + if (telemetry !== "enabled" && telemetry !== "disabled") { + throw new Error(`Invalid telemetry: ${telemetry}`); + } + + if (userConfig.httpPort < 1 || userConfig.httpPort > 65535) { + throw new Error(`Invalid httpPort: ${userConfig.httpPort}`); + } + + if (userConfig.loggers.length === 0) { + throw new Error("No loggers found in config"); + } + + const loggerTypes = new Set(userConfig.loggers); + if (loggerTypes.size !== userConfig.loggers.length) { + throw new Error("Duplicate loggers found in config"); + } + + for (const loggerType of userConfig.loggers as string[]) { + if (loggerType !== "mcp" && loggerType !== "disk" && loggerType !== "stderr") { + throw new Error(`Invalid logger: ${loggerType}`); + } + } + return userConfig; } diff --git a/src/server.ts b/src/server.ts index d9d5d702..bf41b26d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -188,35 +188,6 @@ export class Server { } private async validateConfig(): Promise { - const transport = this.userConfig.transport as string; - if (transport !== "http" && transport !== "stdio") { - throw new Error(`Invalid transport: ${transport}`); - } - - const telemetry = this.userConfig.telemetry as string; - if (telemetry !== "enabled" && telemetry !== "disabled") { - throw new Error(`Invalid telemetry: ${telemetry}`); - } - - if (this.userConfig.httpPort < 1 || this.userConfig.httpPort > 65535) { - throw new Error(`Invalid httpPort: ${this.userConfig.httpPort}`); - } - - if (this.userConfig.loggers.length === 0) { - throw new Error("No loggers found in config"); - } - - const loggerTypes = new Set(this.userConfig.loggers); - if (loggerTypes.size !== this.userConfig.loggers.length) { - throw new Error("Duplicate loggers found in config"); - } - - for (const loggerType of this.userConfig.loggers as string[]) { - if (loggerType !== "mcp" && loggerType !== "disk" && loggerType !== "stderr") { - throw new Error(`Invalid logger: ${loggerType}`); - } - } - if (this.userConfig.connectionString) { try { await this.session.connectToMongoDB({ diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index 682b41ec..e4977716 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -1,38 +1,36 @@ import { describe, it, expect } from "vitest"; -import { setupUserConfig, UserConfig } from "../../../src/common/config.js"; +import { setupUserConfig, UserConfig, defaultUserConfig } from "../../../src/common/config.js"; describe("config", () => { describe("env var parsing", () => { describe("string cases", () => { - const testCases = { - MDB_MCP_API_BASE_URL: "apiBaseUrl", - MDB_MCP_API_CLIENT_ID: "apiClientId", - MDB_MCP_API_CLIENT_SECRET: "apiClientSecret", - MDB_MCP_TELEMETRY: "telemetry", - MDB_MCP_LOG_PATH: "logPath", - MDB_MCP_CONNECTION_STRING: "connectionString", - MDB_MCP_READ_ONLY: "readOnly", - MDB_MCP_INDEX_CHECK: "indexCheck", - MDB_MCP_TRANSPORT: "transport", - MDB_MCP_HTTP_PORT: "httpPort", - MDB_MCP_HTTP_HOST: "httpHost", - MDB_MCP_IDLE_TIMEOUT_MS: "idleTimeoutMs", - MDB_MCP_NOTIFICATION_TIMEOUT_MS: "notificationTimeoutMs", - } as const; - - for (const [envVar, config] of Object.entries(testCases)) { - it(`should map ${envVar} to ${config}`, () => { - const randomValue = "value=" + Math.random(); + const testCases = [ + { envVar: "MDB_MCP_API_BASE_URL", property: "apiBaseUrl", value: "http://test.com" }, + { envVar: "MDB_MCP_API_CLIENT_ID", property: "apiClientId", value: "ClientIdLol" }, + { envVar: "MDB_MCP_API_CLIENT_SECRET", property: "apiClientSecret", value: "SuperClientSecret" }, + { envVar: "MDB_MCP_TELEMETRY", property: "telemetry", value: "enabled" }, + { envVar: "MDB_MCP_LOG_PATH", property: "logPath", value: "/var/log" }, + { envVar: "MDB_MCP_CONNECTION_STRING", property: "connectionString", value: "mongodb://localhost" }, + { envVar: "MDB_MCP_READ_ONLY", property: "readOnly", value: true }, + { envVar: "MDB_MCP_INDEX_CHECK", property: "indexCheck", value: true }, + { envVar: "MDB_MCP_TRANSPORT", property: "transport", value: "http" }, + { envVar: "MDB_MCP_HTTP_PORT", property: "httpPort", value: 8080 }, + { envVar: "MDB_MCP_HTTP_HOST", property: "httpHost", value: "localhost" }, + { envVar: "MDB_MCP_IDLE_TIMEOUT_MS", property: "idleTimeoutMs", value: 5000 }, + { envVar: "MDB_MCP_NOTIFICATION_TIMEOUT_MS", property: "notificationTimeoutMs", value: 5000 }, + ] as const; + for (const { envVar, property, value } of testCases) { + it(`should map ${envVar} to ${property} with value "${value}"`, () => { const actual = setupUserConfig({ cli: [], env: { - [envVar]: randomValue, + [envVar]: String(value), }, - defaults: {}, + defaults: defaultUserConfig, }); - expect(actual[config]).toBe(randomValue); + expect(actual[property]).toBe(value); }); } }); @@ -45,17 +43,15 @@ describe("config", () => { for (const [envVar, config] of Object.entries(testCases)) { it(`should map ${envVar} to ${config}`, () => { - const randomValue = "value=" + Math.random(); - const actual = setupUserConfig({ cli: [], env: { - [envVar]: randomValue, + [envVar]: "disk,mcp", }, - defaults: {}, + defaults: defaultUserConfig, }); - expect(actual[config]).toEqual([randomValue]); + expect(actual[config]).toEqual(["disk", "mcp"]); }); } }); @@ -101,8 +97,8 @@ describe("config", () => { expected: { notificationTimeoutMs: "42" }, }, { - cli: ["--telemetry", "obviously"], - expected: { telemetry: "obviously" }, + cli: ["--telemetry", "enabled"], + expected: { telemetry: "enabled" }, }, { cli: ["--transport", "stdio"], @@ -223,7 +219,7 @@ describe("config", () => { const actual = setupUserConfig({ cli: ["myself", "--", ...cli], env: {}, - defaults: {}, + defaults: defaultUserConfig, }); for (const [key, value] of Object.entries(expected)) { @@ -322,7 +318,7 @@ describe("config", () => { const actual = setupUserConfig({ cli: ["myself", "--", ...cli], env: {}, - defaults: {}, + defaults: defaultUserConfig, }); for (const [key, value] of Object.entries(expected)) { @@ -339,8 +335,8 @@ describe("config", () => { expected: { disabledTools: ["some", "tool"] }, }, { - cli: ["--loggers", "canada,file"], - expected: { loggers: ["canada", "file"] }, + cli: ["--loggers", "disk,mcp"], + expected: { loggers: ["disk", "mcp"] }, }, ] as { cli: string[]; expected: Partial }[]; @@ -349,7 +345,7 @@ describe("config", () => { const actual = setupUserConfig({ cli: ["myself", "--", ...cli], env: {}, - defaults: {}, + defaults: defaultUserConfig, }); for (const [key, value] of Object.entries(expected)) { @@ -365,7 +361,7 @@ describe("config", () => { const actual = setupUserConfig({ cli: ["myself", "--", "--connectionString", "mongodb://localhost"], env: { MDB_MCP_CONNECTION_STRING: "mongodb://crazyhost" }, - defaults: {}, + defaults: defaultUserConfig, }); expect(actual.connectionString).toBe("mongodb://localhost"); @@ -376,6 +372,7 @@ describe("config", () => { cli: ["myself", "--", "--connectionString", "mongodb://localhost"], env: {}, defaults: { + ...defaultUserConfig, connectionString: "mongodb://crazyhost", }, }); @@ -388,6 +385,7 @@ describe("config", () => { cli: [], env: { MDB_MCP_CONNECTION_STRING: "mongodb://localhost" }, defaults: { + ...defaultUserConfig, connectionString: "mongodb://crazyhost", }, }); @@ -401,7 +399,7 @@ describe("config", () => { const actual = setupUserConfig({ cli: ["myself", "--", "mongodb://localhost", "--connectionString", "toRemove"], env: {}, - defaults: {}, + defaults: defaultUserConfig, }); // the shell specifies directConnection=true and serverSelectionTimeoutMS=2000 by default From f0d17ca75d3b25a3e64330246653f278ffdbdaa8 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 14:50:53 +0200 Subject: [PATCH 09/15] chore: add tests for the moved validation --- src/common/config.ts | 10 +- tests/unit/common/config.test.ts | 195 +++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 3 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index 33568a57..3ac592f8 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -273,9 +273,12 @@ function commaSeparatedToArray(str: string | string[] | unde if (str.length === 1) { if (str[0]?.indexOf(",") === -1) { - return str as T; + return str.filter((e) => e.trim().length > 0) as T; } else { - return str[0]?.split(",").map((e) => e.trim()) as T; + return str[0] + ?.split(",") + .map((e) => e.trim()) + .filter((e) => e.length > 0) as T; } } @@ -315,7 +318,8 @@ export function setupUserConfig({ throw new Error(`Invalid telemetry: ${telemetry}`); } - if (userConfig.httpPort < 1 || userConfig.httpPort > 65535) { + const httpPort = +userConfig.httpPort; + if (httpPort < 1 || httpPort > 65535 || isNaN(httpPort)) { throw new Error(`Invalid httpPort: ${userConfig.httpPort}`); } diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index e4977716..d4dbbf83 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; import { setupUserConfig, UserConfig, defaultUserConfig } from "../../../src/common/config.js"; +import assert from "assert"; describe("config", () => { describe("env var parsing", () => { @@ -409,4 +410,198 @@ describe("config", () => { expect(actual.connectionSpecifier).toBe("mongodb://localhost"); }); }); + + describe("validation", () => { + describe("transport", () => { + it("should support http", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--transport", "http"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.transport).toEqual("http"); + }); + + it("should support stdio", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--transport", "stdio"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.transport).toEqual("stdio"); + }); + + it("should not support sse", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--transport", "sse"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Invalid transport: sse"); + }); + + it("should not support arbitrary values", () => { + const value = Math.random() + "transport"; + + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--transport", value], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError(`Invalid transport: ${value}`); + }); + }); + + describe("telemetry", () => { + it("can be enabled", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--telemetry", "enabled"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.telemetry).toEqual("enabled"); + }); + + it("can be disabled", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--telemetry", "disabled"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.telemetry).toEqual("disabled"); + }); + + it("should not support the boolean true value", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--telemetry", "true"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Invalid telemetry: true"); + }); + + it("should not support the boolean false value", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--telemetry", "false"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Invalid telemetry: false"); + }); + + it("should not support arbitrary values", () => { + const value = Math.random() + "telemetry"; + + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--telemetry", value], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError(`Invalid telemetry: ${value}`); + }); + }); + + describe("httpPort", () => { + it("must be above 1", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--httpPort", "0"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Invalid httpPort: 0"); + }); + + it("must be below 65535 (OS limit)", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--httpPort", "89527345"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Invalid httpPort: 89527345"); + }); + + it("should not support non numeric values", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--httpPort", "portAventura"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Invalid httpPort: portAventura"); + }); + + it("should support numeric values", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--httpPort", "8888"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.httpPort).toEqual("8888"); + }); + }); + + describe("loggers", () => { + it("must not be empty", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--loggers", ""], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("No loggers found in config"); + }); + + it("must not allow duplicates", () => { + expect(() => + setupUserConfig({ + cli: ["myself", "--", "--loggers", "disk,disk,disk"], + env: {}, + defaults: defaultUserConfig, + }) + ).toThrowError("Duplicate loggers found in config"); + }); + + it("allows mcp logger", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--loggers", "mcp"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.loggers).toEqual(["mcp"]); + }); + + it("allows disk logger", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--loggers", "disk"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.loggers).toEqual(["disk"]); + }); + + it("allows stderr logger", () => { + const actual = setupUserConfig({ + cli: ["myself", "--", "--loggers", "stderr"], + env: {}, + defaults: defaultUserConfig, + }); + + expect(actual.loggers).toEqual(["stderr"]); + }); + }); + }); }); From b8e00f64790bf2a4e3bbe75596b486489bfb5f77 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 14:53:21 +0200 Subject: [PATCH 10/15] chore: add a comment to explain the shift --- src/common/config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/common/config.ts b/src/common/config.ts index 3ac592f8..cc650c98 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -250,6 +250,9 @@ function parseCliConfig(args: string[]): CliOptions { }; const positionalArguments = parsed._ ?? []; + // if we have a positional argument that matches a connection string + // store it as the connection specifier and remove it from the argument + // list, so it doesn't get misunderstood by the mongosh args-parser if (!parsed.nodb && isConnectionSpecifier(positionalArguments[0])) { parsed.connectionSpecifier = positionalArguments.shift(); } From 69e6c731b5c26d4a9238187d2ea0b8aa7126168a Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Fri, 8 Aug 2025 14:56:50 +0200 Subject: [PATCH 11/15] chore: make eslint happy for a happy weekend --- tests/unit/common/config.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/common/config.test.ts b/tests/unit/common/config.test.ts index d4dbbf83..c2b47f69 100644 --- a/tests/unit/common/config.test.ts +++ b/tests/unit/common/config.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect } from "vitest"; import { setupUserConfig, UserConfig, defaultUserConfig } from "../../../src/common/config.js"; -import assert from "assert"; describe("config", () => { describe("env var parsing", () => { From d15aa893c602ae33246fa407c885ca8c2ff58ad0 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Mon, 11 Aug 2025 13:01:52 +0200 Subject: [PATCH 12/15] chore: oopsie in the merge, add config reference --- tests/unit/common/session.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts index 991fa4e0..add1cac5 100644 --- a/tests/unit/common/session.test.ts +++ b/tests/unit/common/session.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { Session } from "../../../src/common/session.js"; +import { config } from "../../../src/common/config.js"; import { CompositeLogger } from "../../../src/common/logger.js"; import { ConnectionManager } from "../../../src/common/connectionManager.js"; import { ExportsManager } from "../../../src/common/exportsManager.js"; From 1826c1916c717cf903376faf5087e4cdbbe34d55 Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Mon, 11 Aug 2025 15:01:21 +0200 Subject: [PATCH 13/15] chore: merge fips initialisation and error handling This diverges a bit on how mongosh does it, but it should be safe enough. --- src/index.ts | 50 +++++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/src/index.ts b/src/index.ts index df795479..f391a9a7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,11 +1,10 @@ #!/usr/bin/env node -let fipsError: Error | undefined; function enableFipsIfRequested(): void { - if (process.argv.includes("--tlsFIPSMode")) { - // FIPS mode should be enabled before we run any other code, including any dependencies. - // We still wrap this into a function so we can also call it immediately after - // entering the snapshot main function. + let fipsError: Error | undefined; + const tlsFIPSMode = process.argv.includes("--tlsFIPSMode"); + + if (tlsFIPSMode) { try { // eslint-disable-next-line require("crypto").setFips(1); @@ -13,6 +12,25 @@ function enableFipsIfRequested(): void { fipsError ??= err as Error; } } + + if (tlsFIPSMode) { + if (!fipsError && !crypto.getFips()) { + fipsError = new Error("FIPS mode not enabled despite requested due to unknown error."); + } + } + + if (fipsError) { + if (process.config.variables.node_shared_openssl) { + console.error( + "Could not enable FIPS mode. Please ensure that your system OpenSSL installation supports FIPS." + ); + } else { + console.error("Could not enable FIPS mode. This installation does not appear to support FIPS."); + } + console.error("Error details:"); + console.error(fipsError); + process.exit(1); + } } enableFipsIfRequested(); @@ -28,7 +46,6 @@ import { systemCA } from "@mongodb-js/devtools-proxy-support"; async function main(): Promise { systemCA().catch(() => undefined); // load system CA asynchronously as in mongosh - assertFIPSMode(); assertHelpMode(); assertVersionMode(); @@ -105,27 +122,6 @@ main().catch((error: unknown) => { process.exit(1); }); -function assertFIPSMode(): void | never { - if (config.tlsFIPSMode) { - if (!fipsError && !crypto.getFips()) { - fipsError = new Error("FIPS mode not enabled despite requested."); - } - } - - if (fipsError) { - if (process.config.variables.node_shared_openssl) { - console.error( - "Could not enable FIPS mode. Please ensure that your system OpenSSL installation supports FIPS." - ); - } else { - console.error("Could not enable FIPS mode. This installation does not appear to support FIPS."); - } - console.error("Error details:"); - console.error(fipsError); - process.exit(1); - } -} - function assertHelpMode(): void | never { if (config.help) { console.log("For usage information refer to the README.md:"); From bd030cf26fedcfc37688e69798d75b234bbbb91b Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 12 Aug 2025 11:54:29 +0200 Subject: [PATCH 14/15] chore: remove duplicated branch --- src/common/config.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/common/config.ts b/src/common/config.ts index cc650c98..f5c6a079 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -275,14 +275,10 @@ function commaSeparatedToArray(str: string | string[] | unde } if (str.length === 1) { - if (str[0]?.indexOf(",") === -1) { - return str.filter((e) => e.trim().length > 0) as T; - } else { - return str[0] - ?.split(",") - .map((e) => e.trim()) - .filter((e) => e.length > 0) as T; - } + return str[0] + ?.split(",") + .map((e) => e.trim()) + .filter((e) => e.length > 0) as T; } return str as T; From bec33709ad927b80c51819ab66b5bcb419a8633b Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 12 Aug 2025 12:01:49 +0200 Subject: [PATCH 15/15] chore(ci): seems that 0.16.3 (the latest) is broken --- .github/workflows/check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 7cd91fbb..205f7a2a 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -55,4 +55,4 @@ jobs: rm -rf node_modules npm pkg set scripts.prepare="exit 0" npm install --omit=dev - - run: npx -y @modelcontextprotocol/inspector --cli --method tools/list -- node dist/esm/index.js + - run: npx -y @modelcontextprotocol/inspector@0.16.2 --cli --method tools/list -- node dist/esm/index.js