chore: adds eslint rule to restrict import from config file MCP-130

eslint-rules/no-config-imports.js

@@ -0,0 +1,71 @@
+"use strict";
+import path from "path";
+
+// The file from which we wish to discourage importing values
+const configFilePath = path.resolve(import.meta.dirname, "../src/common/config.js");
+
+// Files that are allowed to import value exports from config.ts
+const allowedConfigValueImportFiles = [
+    // Main entry point that injects the config
+    "src/index.ts",
+    // Config resource definition that works with the some config values
+    "src/resources/common/config.ts",
+];
+
+// Ref: https://eslint.org/docs/latest/extend/custom-rules
+export default {
+    meta: {
+        type: "problem",
+        docs: {
+            description:
+                "Disallows value imports from config.ts, with a few exceptions, to enforce dependency injection of the config.",
+            recommended: true,
+        },
+        fixable: null,
+        messages: {
+            noConfigImports:
+                "Value imports from config.ts are not allowed. Use dependency injection instead. Only type imports are permitted.",
+        },
+    },
+    create(context) {
+        const currentFilePath = path.resolve(context.getFilename());
+
+        const isCurrentFileAllowedToImport = allowedConfigValueImportFiles.some((allowedFile) => {
+            const resolvedAllowedFile = path.resolve(allowedFile);
+            return currentFilePath === resolvedAllowedFile;
+        });
+
+        if (isCurrentFileAllowedToImport) {
+            return {};
+        }
+
+        return {
+            ImportDeclaration(node) {
+                const importPath = node.source.value;
+
+                // If the path is not relative, very likely its targeting a
+                // node_module so we skip it. And also if the entire import is
+                // marked with a type keyword.
+                if (typeof importPath !== "string" || !importPath.startsWith(".") || node.importKind === "type") {
+                    return;
+                }
+
+                const currentDir = path.dirname(currentFilePath);
+                const resolvedImportPath = path.resolve(currentDir, importPath);
+
+                if (resolvedImportPath === configFilePath) {
+                    const hasValueImportFromConfig = node.specifiers.some((specifier) => {
+                        return specifier.importKind !== "type";
+                    });
+
+                    if (hasValueImportFromConfig) {
+                        context.report({
+                            node,
+                            messageId: "noConfigImports",
+                        });
+                    }
+                }
+            },
+        };
+    },
+};

eslint-rules/no-config-imports.test.js

@@ -0,0 +1,69 @@
+import path from "path";
+import { RuleTester } from "eslint";
+import { describe, it } from "vitest";
+import tsParser from "@typescript-eslint/parser";
+import rule from "./no-config-imports.js";
+
+const ROOT = process.cwd();
+const resolve = (p) => path.resolve(ROOT, p);
+
+const ruleTester = new RuleTester({
+    languageOptions: {
+        parser: tsParser,
+        parserOptions: { ecmaVersion: 2022, sourceType: "module" },
+    },
+});
+
+describe("no-config-imports", () => {
+    it("should not report any violations", () => {
+        ruleTester.run("no-config-imports", rule, {
+            valid: [
+                {
+                    filename: resolve("src/some/module.ts"),
+                    code: 'import type { UserConfig } from "../common/config.js";\n',
+                },
+                {
+                    filename: resolve("src/some/module.ts"),
+                    code: 'import { something } from "../common/logger.js";\n',
+                },
+                {
+                    filename: resolve("src/some/module.ts"),
+                    code: 'import type * as Cfg from "../common/config.js";\n',
+                },
+                {
+                    filename: resolve("src/index.ts"),
+                    code: 'import { driverOptions } from "../common/config.js";\n',
+                },
+            ],
+            invalid: [],
+        });
+    });
+
+    it("should report rule violations", () => {
+        ruleTester.run("no-config-imports", rule, {
+            valid: [],
+            invalid: [
+                {
+                    filename: resolve("src/another/module.ts"),
+                    code: 'import { driverOptions } from "../common/config.js";\n',
+                    errors: [{ messageId: "noConfigImports" }],
+                },
+                {
+                    filename: resolve("src/another/module.ts"),
+                    code: 'import configDefault from "../common/config.js";\n',
+                    errors: [{ messageId: "noConfigImports" }],
+                },
+                {
+                    filename: resolve("src/another/module.ts"),
+                    code: 'import * as cfg from "../common/config.js";\n',
+                    errors: [{ messageId: "noConfigImports" }],
+                },
+                {
+                    filename: resolve("src/another/module.ts"),
+                    code: 'import { type UserConfig, driverOptions } from "../common/config.js";\n',
+                    errors: [{ messageId: "noConfigImports" }],
+                },
+            ],
+        });
+    });
+});

eslint.config.js

@@ -1,9 +1,11 @@
+import path from "path";
 import { defineConfig, globalIgnores } from "eslint/config";
 import js from "@eslint/js";
 import globals from "globals";
 import tseslint from "typescript-eslint";
 import eslintPluginPrettierRecommended from "eslint-plugin-prettier/recommended";
 import vitestPlugin from "@vitest/eslint-plugin";
+import noConfigImports from "./eslint-rules/no-config-imports.js";
 
 const testFiles = ["tests/**/*.test.ts", "tests/**/*.ts"];
 
@@ -62,6 +64,19 @@ export default defineConfig([
             "@typescript-eslint/explicit-function-return-type": "error",
         },
     },
+    {
+        files: ["src/**/*.ts"],
+        plugins: {
+            "no-config-imports": {
+                rules: {
+                    "no-config-imports": noConfigImports,
+                },
+            },
+        },
+        rules: {
+            "no-config-imports/no-config-imports": "error",
+        },
+    },
     globalIgnores([
         "node_modules",
         "dist",
@@ -72,6 +87,7 @@ export default defineConfig([
         "vitest.config.ts",
         "src/types/*.d.ts",
         "tests/integration/fixtures/",
+        "eslint-rules",
     ]),
     eslintPluginPrettierRecommended,
 ]);

package.json

@@ -52,7 +52,7 @@
     "fix:lint": "eslint . --fix",
     "reformat": "prettier --write .",
     "generate": "./scripts/generate.sh",
-    "test": "vitest --project unit-and-integration --coverage",
+    "test": "vitest --project eslint-rules --project unit-and-integration --coverage",
     "pretest:accuracy": "npm run build",
     "test:accuracy": "sh ./scripts/accuracy/runAccuracyTests.sh"
   },

src/index.ts

@@ -36,7 +36,7 @@ function enableFipsIfRequested(): void {
 enableFipsIfRequested();
 
 import { ConsoleLogger, LogId } from "./common/logger.js";
-import { config } from "./common/config.js";
+import { config, driverOptions } from "./common/config.js";
 import crypto from "crypto";
 import { packageInfo } from "./common/packageInfo.js";
 import { StdioRunner } from "./transports/stdio.js";
@@ -49,7 +49,10 @@ async function main(): Promise<void> {
     assertHelpMode();
     assertVersionMode();
 
-    const transportRunner = config.transport === "stdio" ? new StdioRunner(config) : new StreamableHttpRunner(config);
+    const transportRunner =
+        config.transport === "stdio"
+            ? new StdioRunner(config, driverOptions)
+            : new StreamableHttpRunner(config, driverOptions);
     const shutdown = (): void => {
         transportRunner.logger.info({
             id: LogId.serverCloseRequested,

src/transports/base.ts

@@ -1,5 +1,4 @@
-import type { UserConfig } from "../common/config.js";
-import { driverOptions } from "../common/config.js";
+import type { DriverOptions, UserConfig } from "../common/config.js";
 import { packageInfo } from "../common/packageInfo.js";
 import { Server } from "../server.js";
 import { Session } from "../common/session.js";
@@ -15,7 +14,10 @@ export abstract class TransportRunnerBase {
     public logger: LoggerBase;
     public deviceId: DeviceId;
 
-    protected constructor(protected readonly userConfig: UserConfig) {
+    protected constructor(
+        protected readonly userConfig: UserConfig,
+        private readonly driverOptions: DriverOptions
+    ) {
         const loggers: LoggerBase[] = [];
         if (this.userConfig.loggers.includes("stderr")) {
             loggers.push(new ConsoleLogger());
@@ -35,37 +37,37 @@ export abstract class TransportRunnerBase {
         this.deviceId = DeviceId.create(this.logger);
     }
 
-    protected setupServer(userConfig: UserConfig): Server {
+    protected setupServer(): Server {
         const mcpServer = new McpServer({
             name: packageInfo.mcpServerName,
             version: packageInfo.version,
         });
 
         const loggers = [this.logger];
-        if (userConfig.loggers.includes("mcp")) {
+        if (this.userConfig.loggers.includes("mcp")) {
             loggers.push(new McpLogger(mcpServer));
         }
 
         const logger = new CompositeLogger(...loggers);
-        const exportsManager = ExportsManager.init(userConfig, logger);
-        const connectionManager = new ConnectionManager(userConfig, driverOptions, logger, this.deviceId);
+        const exportsManager = ExportsManager.init(this.userConfig, logger);
+        const connectionManager = new ConnectionManager(this.userConfig, this.driverOptions, logger, this.deviceId);
 
         const session = new Session({
-            apiBaseUrl: userConfig.apiBaseUrl,
-            apiClientId: userConfig.apiClientId,
-            apiClientSecret: userConfig.apiClientSecret,
+            apiBaseUrl: this.userConfig.apiBaseUrl,
+            apiClientId: this.userConfig.apiClientId,
+            apiClientSecret: this.userConfig.apiClientSecret,
             logger,
             exportsManager,
             connectionManager,
         });
 
-        const telemetry = Telemetry.create(session, userConfig, this.deviceId);
+        const telemetry = Telemetry.create(session, this.userConfig, this.deviceId);
 
         return new Server({
             mcpServer,
             session,
             telemetry,
-            userConfig,
+            userConfig: this.userConfig,
         });
     }
 

src/transports/stdio.ts

@@ -5,7 +5,7 @@ import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js";
 import { JSONRPCMessageSchema } from "@modelcontextprotocol/sdk/types.js";
 import { EJSON } from "bson";
 import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
-import type { UserConfig } from "../common/config.js";
+import type { DriverOptions, UserConfig } from "../common/config.js";
 
 // This is almost a copy of ReadBuffer from @modelcontextprotocol/sdk
 // but it uses EJSON.parse instead of JSON.parse to handle BSON types
@@ -54,13 +54,13 @@ export function createStdioTransport(): StdioServerTransport {
 export class StdioRunner extends TransportRunnerBase {
     private server: Server | undefined;
 
-    constructor(userConfig: UserConfig) {
-        super(userConfig);
+    constructor(userConfig: UserConfig, driverOptions: DriverOptions) {
+        super(userConfig, driverOptions);
     }
 
     async start(): Promise<void> {
         try {
-            this.server = this.setupServer(this.userConfig);
+            this.server = this.setupServer();
 
             const transport = createStdioTransport();
 

src/transports/streamableHttp.ts

@@ -3,7 +3,7 @@ import type http from "http";
 import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js";
 import { isInitializeRequest } from "@modelcontextprotocol/sdk/types.js";
 import { TransportRunnerBase } from "./base.js";
-import type { UserConfig } from "../common/config.js";
+import type { DriverOptions, UserConfig } from "../common/config.js";
 import { LogId } from "../common/logger.js";
 import { randomUUID } from "crypto";
 import { SessionStore } from "../common/sessionStore.js";
@@ -18,8 +18,8 @@ export class StreamableHttpRunner extends TransportRunnerBase {
     private httpServer: http.Server | undefined;
     private sessionStore!: SessionStore;
 
-    constructor(userConfig: UserConfig) {
-        super(userConfig);
+    constructor(userConfig: UserConfig, driverOptions: DriverOptions) {
+        super(userConfig, driverOptions);
     }
 
     async start(): Promise<void> {
@@ -89,7 +89,7 @@ export class StreamableHttpRunner extends TransportRunnerBase {
                     return;
                 }
 
-                const server = this.setupServer(this.userConfig);
+                const server = this.setupServer();
                 const transport = new StreamableHTTPServerTransport({
                     sessionIdGenerator: (): string => randomUUID().toString(),
                     onsessioninitialized: (sessionId): void => {

tests/integration/transports/streamableHttp.test.ts

@@ -2,7 +2,7 @@ import { StreamableHttpRunner } from "../../../src/transports/streamableHttp.js"
 import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
 import { describe, expect, it, beforeAll, afterAll } from "vitest";
-import { config } from "../../../src/common/config.js";
+import { config, driverOptions } from "../../../src/common/config.js";
 
 describe("StreamableHttpRunner", () => {
     let runner: StreamableHttpRunner;
@@ -14,7 +14,7 @@ describe("StreamableHttpRunner", () => {
         oldLoggers = config.loggers;
         config.telemetry = "disabled";
         config.loggers = ["stderr"];
-        runner = new StreamableHttpRunner(config);
+        runner = new StreamableHttpRunner(config, driverOptions);
         await runner.start();
     });
 

vitest.config.ts

@@ -36,6 +36,13 @@ export default defineConfig({
                     include: ["**/accuracy/*.test.ts"],
                 },
             },
+            {
+                extends: true,
+                test: {
+                    name: "eslint-rules",
+                    include: ["eslint-rules/*.test.js"],
+                },
+            },
         ],
     },
 });