Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 7845035

Browse files
chore: adds eslint rule to restrict import from config file MCP-130 (#473)
1 parent 55d9304 commit 7845035

File tree

10 files changed

+193
-25
lines changed

10 files changed

+193
-25
lines changed

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
"use strict";
2+
import path from "path";
3+
4+
// The file from which we wish to discourage importing values
5+
const configFilePath = path.resolve(import.meta.dirname, "../src/common/config.js");
6+
7+
// Files that are allowed to import value exports from config.ts
8+
const allowedConfigValueImportFiles = [
9+
// Main entry point that injects the config
10+
"src/index.ts",
11+
// Config resource definition that works with the some config values
12+
"src/resources/common/config.ts",
13+
];
14+
15+
// Ref: https://eslint.org/docs/latest/extend/custom-rules
16+
export default {
17+
meta: {
18+
type: "problem",
19+
docs: {
20+
description:
21+
"Disallows value imports from config.ts, with a few exceptions, to enforce dependency injection of the config.",
22+
recommended: true,
23+
},
24+
fixable: null,
25+
messages: {
26+
noConfigImports:
27+
"Value imports from config.ts are not allowed. Use dependency injection instead. Only type imports are permitted.",
28+
},
29+
},
30+
create(context) {
31+
const currentFilePath = path.resolve(context.getFilename());
32+
33+
const isCurrentFileAllowedToImport = allowedConfigValueImportFiles.some((allowedFile) => {
34+
const resolvedAllowedFile = path.resolve(allowedFile);
35+
return currentFilePath === resolvedAllowedFile;
36+
});
37+
38+
if (isCurrentFileAllowedToImport) {
39+
return {};
40+
}
41+
42+
return {
43+
ImportDeclaration(node) {
44+
const importPath = node.source.value;
45+
46+
// If the path is not relative, very likely its targeting a
47+
// node_module so we skip it. And also if the entire import is
48+
// marked with a type keyword.
49+
if (typeof importPath !== "string" || !importPath.startsWith(".") || node.importKind === "type") {
50+
return;
51+
}
52+
53+
const currentDir = path.dirname(currentFilePath);
54+
const resolvedImportPath = path.resolve(currentDir, importPath);
55+
56+
if (resolvedImportPath === configFilePath) {
57+
const hasValueImportFromConfig = node.specifiers.some((specifier) => {
58+
return specifier.importKind !== "type";
59+
});
60+
61+
if (hasValueImportFromConfig) {
62+
context.report({
63+
node,
64+
messageId: "noConfigImports",
65+
});
66+
}
67+
}
68+
},
69+
};
70+
},
71+
};
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import path from "path";
2+
import { RuleTester } from "eslint";
3+
import { describe, it } from "vitest";
4+
import tsParser from "@typescript-eslint/parser";
5+
import rule from "./no-config-imports.js";
6+
7+
const ROOT = process.cwd();
8+
const resolve = (p) => path.resolve(ROOT, p);
9+
10+
const ruleTester = new RuleTester({
11+
languageOptions: {
12+
parser: tsParser,
13+
parserOptions: { ecmaVersion: 2022, sourceType: "module" },
14+
},
15+
});
16+
17+
describe("no-config-imports", () => {
18+
it("should not report any violations", () => {
19+
ruleTester.run("no-config-imports", rule, {
20+
valid: [
21+
{
22+
filename: resolve("src/some/module.ts"),
23+
code: 'import type { UserConfig } from "../common/config.js";\n',
24+
},
25+
{
26+
filename: resolve("src/some/module.ts"),
27+
code: 'import { something } from "../common/logger.js";\n',
28+
},
29+
{
30+
filename: resolve("src/some/module.ts"),
31+
code: 'import type * as Cfg from "../common/config.js";\n',
32+
},
33+
{
34+
filename: resolve("src/index.ts"),
35+
code: 'import { driverOptions } from "../common/config.js";\n',
36+
},
37+
],
38+
invalid: [],
39+
});
40+
});
41+
42+
it("should report rule violations", () => {
43+
ruleTester.run("no-config-imports", rule, {
44+
valid: [],
45+
invalid: [
46+
{
47+
filename: resolve("src/another/module.ts"),
48+
code: 'import { driverOptions } from "../common/config.js";\n',
49+
errors: [{ messageId: "noConfigImports" }],
50+
},
51+
{
52+
filename: resolve("src/another/module.ts"),
53+
code: 'import configDefault from "../common/config.js";\n',
54+
errors: [{ messageId: "noConfigImports" }],
55+
},
56+
{
57+
filename: resolve("src/another/module.ts"),
58+
code: 'import * as cfg from "../common/config.js";\n',
59+
errors: [{ messageId: "noConfigImports" }],
60+
},
61+
{
62+
filename: resolve("src/another/module.ts"),
63+
code: 'import { type UserConfig, driverOptions } from "../common/config.js";\n',
64+
errors: [{ messageId: "noConfigImports" }],
65+
},
66+
],
67+
});
68+
});
69+
});

‎eslint.config.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
import path from "path";
12
import { defineConfig, globalIgnores } from "eslint/config";
23
import js from "@eslint/js";
34
import globals from "globals";
45
import tseslint from "typescript-eslint";
56
import eslintPluginPrettierRecommended from "eslint-plugin-prettier/recommended";
67
import vitestPlugin from "@vitest/eslint-plugin";
8+
import noConfigImports from "./eslint-rules/no-config-imports.js";
79

810
const testFiles = ["tests/**/*.test.ts", "tests/**/*.ts"];
911

@@ -62,6 +64,19 @@ export default defineConfig([
6264
"@typescript-eslint/explicit-function-return-type": "error",
6365
},
6466
},
67+
{
68+
files: ["src/**/*.ts"],
69+
plugins: {
70+
"no-config-imports": {
71+
rules: {
72+
"no-config-imports": noConfigImports,
73+
},
74+
},
75+
},
76+
rules: {
77+
"no-config-imports/no-config-imports": "error",
78+
},
79+
},
6580
globalIgnores([
6681
"node_modules",
6782
"dist",
@@ -72,6 +87,7 @@ export default defineConfig([
7287
"vitest.config.ts",
7388
"src/types/*.d.ts",
7489
"tests/integration/fixtures/",
90+
"eslint-rules",
7591
]),
7692
eslintPluginPrettierRecommended,
7793
]);

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"fix:lint": "eslint . --fix",
5353
"reformat": "prettier --write .",
5454
"generate": "./scripts/generate.sh",
55-
"test": "vitest --project unit-and-integration --coverage",
55+
"test": "vitest --project eslint-rules --project unit-and-integration --coverage",
5656
"pretest:accuracy": "npm run build",
5757
"test:accuracy": "sh ./scripts/accuracy/runAccuracyTests.sh"
5858
},

‎src/index.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function enableFipsIfRequested(): void {
3636
enableFipsIfRequested();
3737

3838
import { ConsoleLogger, LogId } from "./common/logger.js";
39-
import { config } from "./common/config.js";
39+
import { config,driverOptions } from "./common/config.js";
4040
import crypto from "crypto";
4141
import { packageInfo } from "./common/packageInfo.js";
4242
import { StdioRunner } from "./transports/stdio.js";
@@ -49,7 +49,10 @@ async function main(): Promise<void> {
4949
assertHelpMode();
5050
assertVersionMode();
5151

52-
const transportRunner = config.transport === "stdio" ? new StdioRunner(config) : new StreamableHttpRunner(config);
52+
const transportRunner =
53+
config.transport === "stdio"
54+
? new StdioRunner(config, driverOptions)
55+
: new StreamableHttpRunner(config, driverOptions);
5356
const shutdown = (): void => {
5457
transportRunner.logger.info({
5558
id: LogId.serverCloseRequested,

‎src/transports/base.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import type { UserConfig } from "../common/config.js";
2-
import { driverOptions } from "../common/config.js";
1+
import type { DriverOptions, UserConfig } from "../common/config.js";
32
import { packageInfo } from "../common/packageInfo.js";
43
import { Server } from "../server.js";
54
import { Session } from "../common/session.js";
@@ -15,7 +14,10 @@ export abstract class TransportRunnerBase {
1514
public logger: LoggerBase;
1615
public deviceId: DeviceId;
1716

18-
protected constructor(protected readonly userConfig: UserConfig) {
17+
protected constructor(
18+
protected readonly userConfig: UserConfig,
19+
private readonly driverOptions: DriverOptions
20+
) {
1921
const loggers: LoggerBase[] = [];
2022
if (this.userConfig.loggers.includes("stderr")) {
2123
loggers.push(new ConsoleLogger());
@@ -35,37 +37,37 @@ export abstract class TransportRunnerBase {
3537
this.deviceId = DeviceId.create(this.logger);
3638
}
3739

38-
protected setupServer(userConfig: UserConfig): Server {
40+
protected setupServer(): Server {
3941
const mcpServer = new McpServer({
4042
name: packageInfo.mcpServerName,
4143
version: packageInfo.version,
4244
});
4345

4446
const loggers = [this.logger];
45-
if (userConfig.loggers.includes("mcp")) {
47+
if (this.userConfig.loggers.includes("mcp")) {
4648
loggers.push(new McpLogger(mcpServer));
4749
}
4850

4951
const logger = new CompositeLogger(...loggers);
50-
const exportsManager = ExportsManager.init(userConfig, logger);
51-
const connectionManager = new ConnectionManager(userConfig, driverOptions, logger, this.deviceId);
52+
const exportsManager = ExportsManager.init(this.userConfig, logger);
53+
const connectionManager = new ConnectionManager(this.userConfig, this.driverOptions, logger, this.deviceId);
5254

5355
const session = new Session({
54-
apiBaseUrl: userConfig.apiBaseUrl,
55-
apiClientId: userConfig.apiClientId,
56-
apiClientSecret: userConfig.apiClientSecret,
56+
apiBaseUrl: this.userConfig.apiBaseUrl,
57+
apiClientId: this.userConfig.apiClientId,
58+
apiClientSecret: this.userConfig.apiClientSecret,
5759
logger,
5860
exportsManager,
5961
connectionManager,
6062
});
6163

62-
const telemetry = Telemetry.create(session, userConfig, this.deviceId);
64+
const telemetry = Telemetry.create(session, this.userConfig, this.deviceId);
6365

6466
return new Server({
6567
mcpServer,
6668
session,
6769
telemetry,
68-
userConfig,
70+
userConfig: this.userConfig,
6971
});
7072
}
7173

‎src/transports/stdio.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js";
55
import { JSONRPCMessageSchema } from "@modelcontextprotocol/sdk/types.js";
66
import { EJSON } from "bson";
77
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
8-
import type { UserConfig } from "../common/config.js";
8+
import type { DriverOptions,UserConfig } from "../common/config.js";
99

1010
// This is almost a copy of ReadBuffer from @modelcontextprotocol/sdk
1111
// but it uses EJSON.parse instead of JSON.parse to handle BSON types
@@ -54,13 +54,13 @@ export function createStdioTransport(): StdioServerTransport {
5454
export class StdioRunner extends TransportRunnerBase {
5555
private server: Server | undefined;
5656

57-
constructor(userConfig: UserConfig) {
58-
super(userConfig);
57+
constructor(userConfig: UserConfig,driverOptions: DriverOptions) {
58+
super(userConfig,driverOptions);
5959
}
6060

6161
async start(): Promise<void> {
6262
try {
63-
this.server = this.setupServer(this.userConfig);
63+
this.server = this.setupServer();
6464

6565
const transport = createStdioTransport();
6666

‎src/transports/streamableHttp.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type http from "http";
33
import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js";
44
import { isInitializeRequest } from "@modelcontextprotocol/sdk/types.js";
55
import { TransportRunnerBase } from "./base.js";
6-
import type { UserConfig } from "../common/config.js";
6+
import type { DriverOptions,UserConfig } from "../common/config.js";
77
import { LogId } from "../common/logger.js";
88
import { randomUUID } from "crypto";
99
import { SessionStore } from "../common/sessionStore.js";
@@ -18,8 +18,8 @@ export class StreamableHttpRunner extends TransportRunnerBase {
1818
private httpServer: http.Server | undefined;
1919
private sessionStore!: SessionStore;
2020

21-
constructor(userConfig: UserConfig) {
22-
super(userConfig);
21+
constructor(userConfig: UserConfig,driverOptions: DriverOptions) {
22+
super(userConfig,driverOptions);
2323
}
2424

2525
async start(): Promise<void> {
@@ -89,7 +89,7 @@ export class StreamableHttpRunner extends TransportRunnerBase {
8989
return;
9090
}
9191

92-
const server = this.setupServer(this.userConfig);
92+
const server = this.setupServer();
9393
const transport = new StreamableHTTPServerTransport({
9494
sessionIdGenerator: (): string => randomUUID().toString(),
9595
onsessioninitialized: (sessionId): void => {

‎tests/integration/transports/streamableHttp.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { StreamableHttpRunner } from "../../../src/transports/streamableHttp.js"
22
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
33
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
44
import { describe, expect, it, beforeAll, afterAll } from "vitest";
5-
import { config } from "../../../src/common/config.js";
5+
import { config,driverOptions } from "../../../src/common/config.js";
66

77
describe("StreamableHttpRunner", () => {
88
let runner: StreamableHttpRunner;
@@ -14,7 +14,7 @@ describe("StreamableHttpRunner", () => {
1414
oldLoggers = config.loggers;
1515
config.telemetry = "disabled";
1616
config.loggers = ["stderr"];
17-
runner = new StreamableHttpRunner(config);
17+
runner = new StreamableHttpRunner(config,driverOptions);
1818
await runner.start();
1919
});
2020

‎vitest.config.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export default defineConfig({
3636
include: ["**/accuracy/*.test.ts"],
3737
},
3838
},
39+
{
40+
extends: true,
41+
test: {
42+
name: "eslint-rules",
43+
include: ["eslint-rules/*.test.js"],
44+
},
45+
},
3946
],
4047
},
4148
});

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /