From 2c69b2b027cd29ec54ad1fc845dd6831af16b804 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Mon, 14 Sep 2026 22:53:30 +0200 Subject: [PATCH 01/10] fix(mcp-server): isolate wallet config per runtime A later programmatic main() could overwrite the process-wide config object while an HTTP listener from an earlier start was still serving, so new sessions built signing tools from the mutated singleton. Co-authored-by: Cursor --- .../fix-mcp-server-instance-scoped-wallet.md | 9 ++ REVIEW.md | 9 +- packages/mcp-server/README.md | 2 +- packages/mcp-server/src/core/config.ts | 30 ++++- packages/mcp-server/src/core/wallet/index.ts | 30 ++++- .../src/core/wallet/providers/private-key.ts | 4 +- packages/mcp-server/src/index.ts | 11 +- packages/mcp-server/src/server/args.ts | 7 +- packages/mcp-server/src/server/server.ts | 37 +++--- .../src/server/transport/factory.ts | 8 +- .../src/server/transport/http-sse.ts | 10 +- .../src/server/transport/streamable-http.ts | 10 +- .../mcp-server/src/server/transport/types.ts | 3 +- .../mcp-server/src/tests/core/config.test.ts | 42 ++++++- .../src/tests/core/wallet/index.test.ts | 11 +- .../core/wallet/providers/private-key.test.ts | 9 ++ packages/mcp-server/src/tests/index.test.ts | 6 + .../mcp-server/src/tests/server/args.test.ts | 17 +++ .../tests/server/transport/factory.test.ts | 28 +++++ .../server/transport/wallet-isolation.test.ts | 116 ++++++++++++++++++ 20 files changed, 354 insertions(+), 45 deletions(-) create mode 100644 .changeset/fix-mcp-server-instance-scoped-wallet.md create mode 100644 packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts diff --git a/.changeset/fix-mcp-server-instance-scoped-wallet.md b/.changeset/fix-mcp-server-instance-scoped-wallet.md new file mode 100644 index 00000000..fad973a7 --- /dev/null +++ b/.changeset/fix-mcp-server-instance-scoped-wallet.md @@ -0,0 +1,9 @@ +--- +'@sei-js/mcp-server': patch +--- + +Keep each MCP runtime on the wallet configuration that passed its security check. + +A later programmatic `main()` could overwrite the process-wide config object while an HTTP listener started earlier was still serving requests. New sessions on that listener then built their tool list from the updated singleton, so a wallet-disabled HTTP server could expose signing tools after a trusted stdio start in the same process. No shipped CLI or host spawn does that, but the public lifecycle returned independent runtimes without isolating their keys. + +`parseArgs()` now returns a frozen `AppConfig` snapshot. HTTP transports close their `serverFactory` and request/session handling over that snapshot, `stop()` drops the process-wide wallet provider memo, and both HTTP transports keep wallet tools hidden after a subsequent private-key `initializeConfig()`. Direct wallet-on-HTTP startup still exits 1. diff --git a/REVIEW.md b/REVIEW.md index 4f5a58ef..af8c1a8f 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -11,7 +11,7 @@ like bugs in isolation but are deliberate. ## 1. `packages/mcp-server` is the security surface This package hands blockchain capabilities to an LLM client, so it is the one -place in the repo where a subtle regression can cost users funds. Two +place in the repo where a subtle regression can cost users funds. Three invariants are load-bearing and are enforced in code rather than by convention: - **Wallet mode is stdio-only.** `validateSecurityConfig()` in @@ -20,6 +20,13 @@ invariants are load-bearing and are enforced in code rather than by convention: transports are reachable cross-origin, so a signing key behind one is a drain-the-wallet primitive. Any change that narrows this check, makes it non-fatal, or adds a transport that bypasses it is a finding. +- **HTTP tool and signing policy is instance-scoped.** Each HTTP transport + closes over the `AppConfig` snapshot that passed `validateSecurityConfig`. + New sessions and requests must not consult a later `initializeConfig()` / + `main()` mutation of the process singleton. Treat a default `serverFactory` + that calls unbound `getServer()`, or request handling that reads wallet + state outside `runWithAppConfig`, as a regression. Keep the isolation tests + in `src/tests/server/transport/wallet-isolation.test.ts` meaningful. - **SSE messages are bound to their session.** `src/server/transport/http-sse.ts` keys `connections` by `transport.sessionId` and requires a matching `?sessionId=` on diff --git a/packages/mcp-server/README.md b/packages/mcp-server/README.md index 88270b28..022fdf11 100644 --- a/packages/mcp-server/README.md +++ b/packages/mcp-server/README.md @@ -104,7 +104,7 @@ Legacy SSE accepts at most 100 concurrent sessions by default. Set `SSE_MAX_SESS Streamable HTTP accepts at most 100 active requests by default. Set `STREAMABLE_HTTP_MAX_REQUESTS` to a positive integer to tune the limit; excess requests receive HTTP 503. -HTTP transports create and close an isolated MCP server for each session or request; startup does not allocate an unused bootstrap server. Programmatic callers can use `main()`, which reports and rethrows ordinary startup failures. The packaged executable uses `runCli()` to convert those failures into exit code 1. The wallet-on-HTTP guard is stricter: it terminates immediately with exit code 1 before any listener or signing surface is created. +HTTP transports create and close an isolated MCP server for each session or request; startup does not allocate an unused bootstrap server. Programmatic callers can use `main()`, which reports and rethrows ordinary startup failures. The packaged executable uses `runCli()` to convert those failures into exit code 1. The wallet-on-HTTP guard is stricter: it terminates immediately with exit code 1 before any listener or signing surface is created. Each `main()` call snapshots wallet configuration for that runtime, so a later start cannot expose signing tools on an already-running HTTP listener. ## Configuration diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index 5a719aaf..977a0b59 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -1,3 +1,4 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; import { config as loadDotenv } from 'dotenv'; import type { Hex } from 'viem'; import { z } from 'zod'; @@ -47,13 +48,36 @@ export function initializeConfig(environment: Record = process. return config; } +const runtimeConfig = new AsyncLocalStorage(); + +/** + * Copy of the process singleton as it existed at a given start. + * HTTP transports close over this object so a later initializeConfig() cannot + * change an already-running listener's tool policy or signer. + */ +export function snapshotConfig(source: AppConfig = config): AppConfig { + return Object.freeze({ + privateKey: source.privateKey, + walletMode: source.walletMode, + walletApiKey: source.walletApiKey + }); +} + +export function runWithAppConfig(appConfig: AppConfig, fn: () => T): T { + return runtimeConfig.run(appConfig, fn); +} + +export function getRuntimeConfig(): AppConfig { + return runtimeConfig.getStore() ?? config; +} + /** * Get the private key from environment variable as a Hex type for viem. * Returns undefined if the PRIVATE_KEY environment variable is not set. * @returns Private key from environment variable as Hex or undefined */ export function getPrivateKeyAsHex(): Hex | undefined { - return config.privateKey as Hex | undefined; + return getRuntimeConfig().privateKey as Hex | undefined; } /** @@ -61,7 +85,7 @@ export function getPrivateKeyAsHex(): Hex | undefined { * @returns True if wallet functionality should be available */ export function isWalletEnabled(): boolean { - return config.walletMode !== 'disabled'; + return getRuntimeConfig().walletMode !== 'disabled'; } /** @@ -69,5 +93,5 @@ export function isWalletEnabled(): boolean { * @returns The configured wallet mode */ export function getWalletMode(): WalletMode { - return config.walletMode; + return getRuntimeConfig().walletMode; } diff --git a/packages/mcp-server/src/core/wallet/index.ts b/packages/mcp-server/src/core/wallet/index.ts index 6743e29d..b5f4164d 100644 --- a/packages/mcp-server/src/core/wallet/index.ts +++ b/packages/mcp-server/src/core/wallet/index.ts @@ -1,15 +1,41 @@ -import { getWalletMode } from '../config.js'; +import { type AppConfig, getRuntimeConfig, getWalletMode, config as processConfig } from '../config.js'; import { DisabledWalletProvider } from './providers/disabled.js'; import { PrivateKeyWalletProvider } from './providers/private-key.js'; import type { WalletProvider } from './types.js'; -// Cache wallet provider instance +const providersByConfig = new WeakMap(); + +// Cache for the process-global singleton only. Instance snapshots use the WeakMap. let walletProviderInstance: WalletProvider | null = null; +function createWalletProvider(appConfig: AppConfig): WalletProvider { + switch (appConfig.walletMode) { + case 'private-key': + return new PrivateKeyWalletProvider(appConfig.privateKey); + case 'disabled': + return new DisabledWalletProvider(); + default: + throw new Error(`Unknown wallet mode: ${appConfig.walletMode}`); + } +} + +function providerForSnapshot(appConfig: AppConfig): WalletProvider { + const cached = providersByConfig.get(appConfig); + if (cached) return cached; + const provider = createWalletProvider(appConfig); + providersByConfig.set(appConfig, provider); + return provider; +} + /** * Get the wallet provider instance based on configuration */ export function getWalletProvider(): WalletProvider { + const runtime = getRuntimeConfig(); + if (runtime !== processConfig) { + return providerForSnapshot(runtime); + } + if (walletProviderInstance) { return walletProviderInstance; } diff --git a/packages/mcp-server/src/core/wallet/providers/private-key.ts b/packages/mcp-server/src/core/wallet/providers/private-key.ts index d1df9ef1..3f4299d1 100644 --- a/packages/mcp-server/src/core/wallet/providers/private-key.ts +++ b/packages/mcp-server/src/core/wallet/providers/private-key.ts @@ -13,8 +13,8 @@ import { WalletProviderError } from '../types.js'; export class PrivateKeyWalletProvider implements WalletProvider { private privateKey: string | undefined; - constructor() { - this.privateKey = getPrivateKeyAsHex(); + constructor(...key: [] | [string | undefined]) { + this.privateKey = key.length === 0 ? getPrivateKeyAsHex() : key[0]; } isAvailable(): boolean { diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index abd0ea1a..cc54057b 100755 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -2,6 +2,7 @@ import { realpathSync } from 'node:fs'; import { pathToFileURL } from 'node:url'; import { isWalletEnabled } from './core/config.js'; import { sanitizeError } from './core/errors.js'; +import { resetWalletProvider } from './core/wallet/index.js'; import { parseArgs } from './server/args.js'; import { getServer } from './server/server.js'; import { createTransport } from './server/transport/index.js'; @@ -38,7 +39,7 @@ export function registerShutdownHandlers(stop: () => Promise): () => void export async function startMcpServer(): Promise { const config = parseArgs(); - const server = config.mode === 'stdio' ? await getServer() : undefined; + const server = config.mode === 'stdio' ? await getServer(config.appConfig) : undefined; let transport: McpTransport | undefined; try { @@ -60,8 +61,12 @@ export async function startMcpServer(): Promise { if (!stopPromise) { removeShutdownHandlers(); stopPromise = (async () => { - const errors = await collectOperationErrors([() => transport.stop(), () => server?.close()]); - throwCollectedErrors(errors, 'Failed to stop all MCP server resources.'); + try { + const errors = await collectOperationErrors([() => transport.stop(), () => server?.close()]); + throwCollectedErrors(errors, 'Failed to stop all MCP server resources.'); + } finally { + resetWalletProvider(); + } })(); } return stopPromise; diff --git a/packages/mcp-server/src/server/args.ts b/packages/mcp-server/src/server/args.ts index a7ed8fef..3b8d7be3 100644 --- a/packages/mcp-server/src/server/args.ts +++ b/packages/mcp-server/src/server/args.ts @@ -1,6 +1,6 @@ import { Command } from 'commander'; import { config as dotenvConfig } from 'dotenv'; -import { initializeConfig } from '../core/config.js'; +import { initializeConfig, snapshotConfig } from '../core/config.js'; import { validatePrivateKeyConfiguration } from '../core/private-key.js'; import { getPackageInfo } from './package-info.js'; import type { TransportConfig, TransportMode } from './transport/types.js'; @@ -147,14 +147,15 @@ Security Note: const config = loadConfig(); validateConfig(config); - initializeConfig(process.env); + const appConfig = snapshotConfig(initializeConfig(process.env)); return { mode: config.server.transport, port: Number.isNaN(config.server.port) ? DEFAULT_CONFIG.server.port : config.server.port, host: config.server.host, path: config.server.path, - walletMode: config.wallet.mode, + walletMode: appConfig.walletMode, + appConfig, maxSseSessions: Number.isInteger(config.server.sseMaxSessions) && config.server.sseMaxSessions > 0 ? config.server.sseMaxSessions : DEFAULT_CONFIG.server.sseMaxSessions, maxStreamableRequests: diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index 41d8ca7a..ab432aa2 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -1,5 +1,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { getSupportedNetworks } from '../core/chains.js'; +import { type AppConfig, config as processConfig, runWithAppConfig } from '../core/config.js'; import { sanitizeError } from '../core/errors.js'; import { registerEVMPrompts } from '../core/prompts.js'; import { registerEVMResources } from '../core/resources.js'; @@ -7,24 +8,26 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; -export const getServer = async () => { - try { - const packageInfo = getPackageInfo(); - const server = new McpServer({ - name: packageInfo.name, - version: packageInfo.version - }); +export const getServer = async (appConfig: AppConfig = processConfig) => { + return runWithAppConfig(appConfig, async () => { + try { + const packageInfo = getPackageInfo(); + const server = new McpServer({ + name: packageInfo.name, + version: packageInfo.version + }); - registerEVMResources(server); - registerEVMTools(server); - registerEVMPrompts(server); - createDocsSearchTool(server, packageInfo); + registerEVMResources(server); + registerEVMTools(server); + registerEVMPrompts(server); + createDocsSearchTool(server, packageInfo); - console.error('Supported networks:', getSupportedNetworks().join(', ')); + console.error('Supported networks:', getSupportedNetworks().join(', ')); - return server; - } catch (error) { - console.error('Failed to initialize server:', sanitizeError(error)); - throw error; - } + return server; + } catch (error) { + console.error('Failed to initialize server:', sanitizeError(error)); + throw error; + } + }); }; diff --git a/packages/mcp-server/src/server/transport/factory.ts b/packages/mcp-server/src/server/transport/factory.ts index 687abb2f..5fe85bb7 100644 --- a/packages/mcp-server/src/server/transport/factory.ts +++ b/packages/mcp-server/src/server/transport/factory.ts @@ -4,6 +4,8 @@ import { StreamableHttpTransport } from './streamable-http.js'; import type { McpTransport, TransportConfig } from './types.js'; export const createTransport = (config: TransportConfig): McpTransport => { + const appConfig = config.appConfig ? { appConfig: config.appConfig } : {}; + switch (config.mode) { case 'stdio': return new StdioTransport(); @@ -14,7 +16,8 @@ export const createTransport = (config: TransportConfig): McpTransport => { host: config.host, path: config.path, walletMode: config.walletMode, - maxActiveRequests: config.maxStreamableRequests + maxActiveRequests: config.maxStreamableRequests, + ...appConfig }); case 'http-sse': @@ -23,7 +26,8 @@ export const createTransport = (config: TransportConfig): McpTransport => { host: config.host, path: config.path, walletMode: config.walletMode, - maxSessions: config.maxSseSessions + maxSessions: config.maxSseSessions, + ...appConfig }); default: diff --git a/packages/mcp-server/src/server/transport/http-sse.ts b/packages/mcp-server/src/server/transport/http-sse.ts index a5c26dda..8114181d 100644 --- a/packages/mcp-server/src/server/transport/http-sse.ts +++ b/packages/mcp-server/src/server/transport/http-sse.ts @@ -3,6 +3,7 @@ import type { Socket } from 'node:net'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js'; import express, { type Request, type Response } from 'express'; +import { type AppConfig, runWithAppConfig, snapshotConfig } from '../../core/config.js'; import { sanitizeError } from '../../core/errors.js'; import { getServer } from '../server.js'; import { closeHttpServer, collectOperationErrors, runAllOperations, throwCollectedErrors } from './lifecycle.js'; @@ -20,6 +21,7 @@ export interface HttpSseTransportOptions { host: string; path: string; walletMode?: WalletMode; + appConfig?: AppConfig; maxSessions?: number; } @@ -54,6 +56,7 @@ export class HttpSseTransport implements McpTransport { private readonly host: string; private readonly path: string; private readonly walletMode: WalletMode; + private readonly appConfig: AppConfig; private readonly serverFactory: McpServerFactory; private readonly transportFactory: SseServerTransportFactory; private readonly listenFactory: SseListenFactory; @@ -64,8 +67,9 @@ export class HttpSseTransport implements McpTransport { this.host = options.host; this.path = options.path; this.walletMode = options.walletMode ?? 'disabled'; + this.appConfig = options.appConfig ?? snapshotConfig(); this.maxSessions = options.maxSessions ?? DEFAULT_MAX_SSE_SESSIONS; - this.serverFactory = dependencies.serverFactory ?? getServer; + this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = dependencies.transportFactory ?? ((endpoint, response) => new SSEServerTransport(endpoint, response)); this.listenFactory = dependencies.listenFactory ?? ((app, port, host) => app.listen(port, host)); this.app = express(); @@ -140,7 +144,7 @@ export class HttpSseTransport implements McpTransport { req.socket.on('error', onDisconnect); try { - const server = await this.serverFactory(); + const server = await runWithAppConfig(this.appConfig, () => this.serverFactory()); session = { server, transport, releaseSlot }; if (this.state !== 'running' || disconnected) { @@ -188,7 +192,7 @@ export class HttpSseTransport implements McpTransport { } try { - await session.transport.handlePostMessage(req, res, req.body); + await runWithAppConfig(this.appConfig, () => session.transport.handlePostMessage(req, res, req.body)); } catch (error) { console.error('Error handling SSE message:', sanitizeError(error)); if (!res.headersSent) { diff --git a/packages/mcp-server/src/server/transport/streamable-http.ts b/packages/mcp-server/src/server/transport/streamable-http.ts index 5b369a8c..bd892f1d 100644 --- a/packages/mcp-server/src/server/transport/streamable-http.ts +++ b/packages/mcp-server/src/server/transport/streamable-http.ts @@ -3,6 +3,7 @@ import type { Socket } from 'node:net'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import express, { type Request, type Response } from 'express'; +import { type AppConfig, runWithAppConfig, snapshotConfig } from '../../core/config.js'; import { sanitizeError } from '../../core/errors.js'; import { getServer } from '../server.js'; import { closeHttpServer, collectOperationErrors, runAllOperations, throwCollectedErrors } from './lifecycle.js'; @@ -20,6 +21,7 @@ export interface StreamableHttpTransportOptions { host?: string; path?: string; walletMode?: WalletMode; + appConfig?: AppConfig; maxActiveRequests?: number; } @@ -54,6 +56,7 @@ export class StreamableHttpTransport implements McpTransport { private readonly host: string; private readonly path: string; private readonly walletMode: WalletMode; + private readonly appConfig: AppConfig; private readonly serverFactory: StreamableServerFactory; private readonly transportFactory: StreamableTransportFactory; private readonly listenFactory: StreamableListenFactory; @@ -64,8 +67,9 @@ export class StreamableHttpTransport implements McpTransport { this.host = options.host ?? 'localhost'; this.path = options.path ?? '/mcp'; this.walletMode = options.walletMode ?? 'disabled'; + this.appConfig = options.appConfig ?? snapshotConfig(); this.maxActiveRequests = options.maxActiveRequests ?? DEFAULT_MAX_STREAMABLE_REQUESTS; - this.serverFactory = dependencies.serverFactory ?? getServer; + this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = dependencies.transportFactory ?? (() => @@ -142,7 +146,7 @@ export class StreamableHttpTransport implements McpTransport { let activeRequest: ActiveRequest | undefined; let requestServer: McpServer | undefined; try { - const server = await this.serverFactory(); + const server = await runWithAppConfig(this.appConfig, () => this.serverFactory()); requestServer = server; const transport = this.transportFactory(); activeRequest = { server, transport, releaseSlot }; @@ -187,7 +191,7 @@ export class StreamableHttpTransport implements McpTransport { } await server.connect(transport); - await transport.handleRequest(req, res, req.body); + await runWithAppConfig(this.appConfig, () => transport.handleRequest(req, res, req.body)); } catch (error) { console.error('Error handling MCP request:', sanitizeError(error)); if (!res.headersSent) { diff --git a/packages/mcp-server/src/server/transport/types.ts b/packages/mcp-server/src/server/transport/types.ts index bffc8764..aace4f96 100644 --- a/packages/mcp-server/src/server/transport/types.ts +++ b/packages/mcp-server/src/server/transport/types.ts @@ -1,5 +1,5 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; -import type { WalletMode } from '../../core/config.js'; +import type { AppConfig, WalletMode } from '../../core/config.js'; export type TransportMode = 'stdio' | 'streamable-http' | 'http-sse'; @@ -12,6 +12,7 @@ export interface McpTransport { export interface TransportConfig { mode: TransportMode; walletMode: WalletMode; + appConfig?: AppConfig; port: number; // Required for HTTP-based transports host: string; // Required for HTTP-based transports path: string; // Required for HTTP-based transports diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index 5581e091..0b28a7b3 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -3,11 +3,14 @@ import { config, formatPrivateKey, getPrivateKeyAsHex, + getRuntimeConfig, getWalletMode, initializeConfig, isValidPrivateKey, isWalletEnabled, - loadConfig + loadConfig, + runWithAppConfig, + snapshotConfig } from '../../core/config.js'; describe('Config Module - Actual Implementation', () => { @@ -85,6 +88,43 @@ describe('Config Module - Actual Implementation', () => { walletApiKey: undefined }); }); + + test('snapshots freeze a copy that later initializeConfig mutations cannot change', () => { + const snapshot = snapshotConfig(); + expect(snapshot).toEqual({ + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }); + expect(snapshot).not.toBe(config); + expect(Object.isFrozen(snapshot)).toBe(true); + + const privateKey = '3'.repeat(64); + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: privateKey }); + expect(snapshot.walletMode).toBe('disabled'); + expect(snapshot.privateKey).toBeUndefined(); + expect(config.walletMode).toBe('private-key'); + }); + + test('runWithAppConfig makes getters read the snapshot instead of the process singleton', () => { + config.walletMode = 'private-key'; + config.privateKey = '0xabcdef'; + const snapshot = snapshotConfig({ + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }); + + expect(isWalletEnabled()).toBe(true); + runWithAppConfig(snapshot, () => { + expect(isWalletEnabled()).toBe(false); + expect(getWalletMode()).toBe('disabled'); + expect(getPrivateKeyAsHex()).toBeUndefined(); + expect(getRuntimeConfig()).toBe(snapshot); + }); + expect(isWalletEnabled()).toBe(true); + expect(getPrivateKeyAsHex()).toBe('0xabcdef'); + }); }); describe('getPrivateKeyAsHex', () => { diff --git a/packages/mcp-server/src/tests/core/wallet/index.test.ts b/packages/mcp-server/src/tests/core/wallet/index.test.ts index 42308bab..e31a3653 100644 --- a/packages/mcp-server/src/tests/core/wallet/index.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/index.test.ts @@ -5,9 +5,14 @@ import { PrivateKeyWalletProvider } from '../../../core/wallet/providers/private import type { WalletProvider } from '../../../core/wallet/types.js'; // Mock dependencies -jest.mock('../../../core/config.js', () => ({ - getWalletMode: jest.fn() -})); +jest.mock('../../../core/config.js', () => { + const config = { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }; + return { + getWalletMode: jest.fn(), + getRuntimeConfig: jest.fn(() => config), + config + }; +}); jest.mock('../../../core/wallet/providers/private-key.js', () => ({ PrivateKeyWalletProvider: jest.fn() diff --git a/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts b/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts index 247646f4..852e41d0 100644 --- a/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts @@ -63,6 +63,15 @@ describe('PrivateKeyWalletProvider', () => { expect(provider.isAvailable()).toBe(false); }); + + test('does not read the process singleton when constructed with an explicit key slot', () => { + (getPrivateKeyAsHex as jest.Mock).mockReturnValue(mockPrivateKey); + + const provider = new PrivateKeyWalletProvider(undefined); + + expect(provider.isAvailable()).toBe(false); + expect(getPrivateKeyAsHex).not.toHaveBeenCalled(); + }); }); describe('getName', () => { diff --git a/packages/mcp-server/src/tests/index.test.ts b/packages/mcp-server/src/tests/index.test.ts index 4b759e6f..edd83bc6 100644 --- a/packages/mcp-server/src/tests/index.test.ts +++ b/packages/mcp-server/src/tests/index.test.ts @@ -13,6 +13,10 @@ jest.mock('../core/config.js', () => ({ isWalletEnabled: jest.fn() })); +jest.mock('../core/wallet/index.js', () => ({ + resetWalletProvider: jest.fn() +})); + jest.mock('../server/args.js', () => ({ parseArgs: jest.fn() })); @@ -176,6 +180,8 @@ describe('index', () => { await expect(runtime?.stop()).rejects.toBeInstanceOf(AggregateError); expect(mockTransport.stop).toHaveBeenCalledTimes(1); expect(mockServer.close).toHaveBeenCalledTimes(1); + const walletModule = await import('../core/wallet/index.js'); + expect(walletModule.resetWalletProvider).toHaveBeenCalled(); }); it('runs graceful shutdown once and allows signal handlers to be removed', async () => { diff --git a/packages/mcp-server/src/tests/server/args.test.ts b/packages/mcp-server/src/tests/server/args.test.ts index 75c17170..8bdcb5ba 100644 --- a/packages/mcp-server/src/tests/server/args.test.ts +++ b/packages/mcp-server/src/tests/server/args.test.ts @@ -113,9 +113,15 @@ describe('Args Module', () => { host: 'localhost', path: '/mcp', walletMode: 'disabled', + appConfig: { + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }, maxSseSessions: 100, maxStreamableRequests: 100 }); + expect(Object.isFrozen(result.appConfig)).toBe(true); }); it('should load configuration from environment variables', () => { @@ -134,6 +140,11 @@ describe('Args Module', () => { host: '0.0.0.0', path: '/api/mcp', walletMode: 'private-key', + appConfig: { + privateKey: `0x${'1'.repeat(64)}`, + walletMode: 'private-key', + walletApiKey: undefined + }, maxSseSessions: 100, maxStreamableRequests: 100 }); @@ -358,6 +369,7 @@ describe('Args Module', () => { expect(result).toHaveProperty('host'); expect(result).toHaveProperty('path'); expect(result).toHaveProperty('walletMode'); + expect(result).toHaveProperty('appConfig'); // Verify types expect(typeof result.mode).toBe('string'); @@ -399,6 +411,11 @@ describe('Args Module', () => { host: 'localhost', path: '/mcp', walletMode: 'private-key', + appConfig: { + privateKey: `0x${'1'.repeat(64)}`, + walletMode: 'private-key', + walletApiKey: undefined + }, maxSseSessions: 100, maxStreamableRequests: 100 }); diff --git a/packages/mcp-server/src/tests/server/transport/factory.test.ts b/packages/mcp-server/src/tests/server/transport/factory.test.ts index 229dbe5a..0a384ddd 100644 --- a/packages/mcp-server/src/tests/server/transport/factory.test.ts +++ b/packages/mcp-server/src/tests/server/transport/factory.test.ts @@ -108,6 +108,34 @@ describe('Transport Factory', () => { expect(transport).toBe(mockSseInstance); }); + it('forwards a frozen appConfig snapshot to HTTP transports', () => { + const appConfig = Object.freeze({ + privateKey: undefined, + walletMode: 'disabled' as const, + walletApiKey: undefined + }); + const config: TransportConfig = { + mode: 'streamable-http', + walletMode: 'disabled', + appConfig, + port: 8080, + host: 'localhost', + path: '/mcp' + }; + StreamableHttpTransport.mockImplementation(() => ({ mode: 'streamable-http' })); + + createTransport(config); + + expect(StreamableHttpTransport).toHaveBeenCalledWith({ + port: 8080, + host: 'localhost', + path: '/mcp', + walletMode: 'disabled', + maxActiveRequests: undefined, + appConfig + }); + }); + it('should throw error for unsupported transport mode', () => { const config = { mode: 'unsupported-mode', diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts new file mode 100644 index 00000000..9fbc88f0 --- /dev/null +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -0,0 +1,116 @@ +import { afterEach, describe, expect, it, jest } from 'bun:test'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { config, initializeConfig, isWalletEnabled, snapshotConfig } from '../../../core/config.js'; +import { resetWalletProvider } from '../../../core/wallet/index.js'; +import { HttpSseTransport } from '../../../server/transport/http-sse.js'; +import { StreamableHttpTransport } from '../../../server/transport/streamable-http.js'; + +const HOST = '127.0.0.1'; +const PATH = '/mcp'; +const PRIVATE_KEY = '1'.repeat(64); +const WALLET_TOOLS = new Set([ + 'approve_token_spending', + 'deploy_contract', + 'get_address_from_private_key', + 'transfer_erc1155', + 'transfer_erc20', + 'transfer_nft', + 'transfer_sei', + 'transfer_token', + 'write_contract' +]); + +type HttpMode = 'http-sse' | 'streamable-http'; + +async function listWalletTools(mode: HttpMode, url: URL): Promise { + const client = new Client({ name: `wallet-isolation-${mode}`, version: '1.0.0' }); + const transport = mode === 'http-sse' ? new SSEClientTransport(url) : new StreamableHTTPClientTransport(url); + await client.connect(transport); + try { + const listed = await client.listTools(); + return listed.tools + .map((tool) => tool.name) + .filter((name) => WALLET_TOOLS.has(name)) + .sort(); + } finally { + await client.close(); + } +} + +describe('HTTP wallet isolation across later starts', () => { + const transports: Array = []; + let consoleErrorSpy: jest.SpiedFunction; + const originalConfig = { ...config }; + + afterEach(async () => { + await Promise.allSettled(transports.splice(0).map((transport) => transport.stop())); + consoleErrorSpy?.mockRestore(); + Object.assign(config, originalConfig); + resetWalletProvider(); + delete process.env.WALLET_MODE; + delete process.env.PRIVATE_KEY; + }); + + it.each(['http-sse', 'streamable-http'] as const)('keeps %s sessions read-only after a later private-key initializeConfig', async (mode) => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const appConfig = snapshotConfig({ + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }); + const transport = + mode === 'http-sse' + ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled', appConfig }) + : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled', appConfig }); + transports.push(transport); + await transport.start(); + + const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); + expect(await listWalletTools(mode, url)).toEqual([]); + + process.env.WALLET_MODE = 'private-key'; + process.env.PRIVATE_KEY = PRIVATE_KEY; + expect(await listWalletTools(mode, url)).toEqual([]); + expect(isWalletEnabled()).toBe(false); + + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: PRIVATE_KEY }); + process.env.WALLET_MODE = 'disabled'; + delete process.env.PRIVATE_KEY; + expect(isWalletEnabled()).toBe(true); + expect(await listWalletTools(mode, url)).toEqual([]); + }); + + it.each(['http-sse', 'streamable-http'] as const)('snapshots %s at construction when appConfig is omitted', async (mode) => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + Object.assign(config, { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); + const transport = + mode === 'http-sse' + ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled' }) + : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled' }); + transports.push(transport); + await transport.start(); + + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: PRIVATE_KEY }); + const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); + expect(isWalletEnabled()).toBe(true); + expect(await listWalletTools(mode, url)).toEqual([]); + }); + + it('still terminates a direct private-key HTTP start before listen', async () => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const processExit = jest.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`process.exit called with code ${code}`); + }); + const listenFactory = jest.fn(); + const streamable = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key' }, { listenFactory }); + const sse = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key' }, { listenFactory }); + + await expect(streamable.start()).rejects.toThrow('process.exit called with code 1'); + await expect(sse.start()).rejects.toThrow('process.exit called with code 1'); + expect(processExit).toHaveBeenCalledWith(1); + expect(listenFactory).not.toHaveBeenCalled(); + processExit.mockRestore(); + }); +}); From f14e8c6d087e4bc1021e629ceecbbb38dbaa4c84 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 00:27:28 +0200 Subject: [PATCH 02/10] fix(mcp-server): keep stdio signer on its snapshot after stop Stdio request handling never entered runWithAppConfig, so resetWalletProvider() on another runtime's stop() made a live stdio server recreate its signer from the latest process singleton. Co-authored-by: Cursor --- .../fix-mcp-server-instance-scoped-wallet.md | 2 +- REVIEW.md | 17 +++--- packages/mcp-server/README.md | 2 +- packages/mcp-server/src/core/config.ts | 8 +++ packages/mcp-server/src/server/server.ts | 26 ++++++++- .../src/server/transport/factory.ts | 2 +- .../mcp-server/src/server/transport/stdio.ts | 14 ++++- .../mcp-server/src/tests/core/config.test.ts | 19 ++++++- .../src/tests/core/wallet/index.test.ts | 24 ++++++++- .../src/tests/server/server.test.ts | 3 +- .../tests/server/transport/factory.test.ts | 23 +++++++- .../src/tests/server/transport/stdio.test.ts | 33 ++++++++++++ .../server/transport/wallet-isolation.test.ts | 54 +++++++++++++++++++ 13 files changed, 209 insertions(+), 18 deletions(-) diff --git a/.changeset/fix-mcp-server-instance-scoped-wallet.md b/.changeset/fix-mcp-server-instance-scoped-wallet.md index fad973a7..62328278 100644 --- a/.changeset/fix-mcp-server-instance-scoped-wallet.md +++ b/.changeset/fix-mcp-server-instance-scoped-wallet.md @@ -6,4 +6,4 @@ Keep each MCP runtime on the wallet configuration that passed its security check A later programmatic `main()` could overwrite the process-wide config object while an HTTP listener started earlier was still serving requests. New sessions on that listener then built their tool list from the updated singleton, so a wallet-disabled HTTP server could expose signing tools after a trusted stdio start in the same process. No shipped CLI or host spawn does that, but the public lifecycle returned independent runtimes without isolating their keys. -`parseArgs()` now returns a frozen `AppConfig` snapshot. HTTP transports close their `serverFactory` and request/session handling over that snapshot, `stop()` drops the process-wide wallet provider memo, and both HTTP transports keep wallet tools hidden after a subsequent private-key `initializeConfig()`. Direct wallet-on-HTTP startup still exits 1. +`parseArgs()` now returns a frozen `AppConfig` snapshot. HTTP transports close their `serverFactory` and request/session handling over that snapshot, stdio `connect` / `onmessage` stay on the same snapshot after another runtime's `stop()` drops the process-wide wallet provider memo, and both HTTP transports keep wallet tools hidden after a subsequent private-key `initializeConfig()`. Direct wallet-on-HTTP startup still exits 1. diff --git a/REVIEW.md b/REVIEW.md index af8c1a8f..e454c478 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -20,13 +20,16 @@ invariants are load-bearing and are enforced in code rather than by convention: transports are reachable cross-origin, so a signing key behind one is a drain-the-wallet primitive. Any change that narrows this check, makes it non-fatal, or adds a transport that bypasses it is a finding. -- **HTTP tool and signing policy is instance-scoped.** Each HTTP transport - closes over the `AppConfig` snapshot that passed `validateSecurityConfig`. - New sessions and requests must not consult a later `initializeConfig()` / - `main()` mutation of the process singleton. Treat a default `serverFactory` - that calls unbound `getServer()`, or request handling that reads wallet - state outside `runWithAppConfig`, as a regression. Keep the isolation tests - in `src/tests/server/transport/wallet-isolation.test.ts` meaningful. +- **Tool and signing policy is instance-scoped.** Each runtime closes over + the `AppConfig` snapshot that passed `validateSecurityConfig`. HTTP + `serverFactory` / request handling and stdio `connect` / `onmessage` must + not consult a later `initializeConfig()` / `main()` mutation of the process + singleton. `stop()` may drop the process-wide wallet provider memo; a + still-running stdio server must keep signing from its snapshot after that + reset. Treat a default `serverFactory` that calls unbound `getServer()`, + or request handling that reads wallet state outside `runWithAppConfig`, as + a regression. Keep the isolation tests in + `src/tests/server/transport/wallet-isolation.test.ts` meaningful. - **SSE messages are bound to their session.** `src/server/transport/http-sse.ts` keys `connections` by `transport.sessionId` and requires a matching `?sessionId=` on diff --git a/packages/mcp-server/README.md b/packages/mcp-server/README.md index 022fdf11..60690d1e 100644 --- a/packages/mcp-server/README.md +++ b/packages/mcp-server/README.md @@ -104,7 +104,7 @@ Legacy SSE accepts at most 100 concurrent sessions by default. Set `SSE_MAX_SESS Streamable HTTP accepts at most 100 active requests by default. Set `STREAMABLE_HTTP_MAX_REQUESTS` to a positive integer to tune the limit; excess requests receive HTTP 503. -HTTP transports create and close an isolated MCP server for each session or request; startup does not allocate an unused bootstrap server. Programmatic callers can use `main()`, which reports and rethrows ordinary startup failures. The packaged executable uses `runCli()` to convert those failures into exit code 1. The wallet-on-HTTP guard is stricter: it terminates immediately with exit code 1 before any listener or signing surface is created. Each `main()` call snapshots wallet configuration for that runtime, so a later start cannot expose signing tools on an already-running HTTP listener. +HTTP transports create and close an isolated MCP server for each session or request; startup does not allocate an unused bootstrap server. Programmatic callers can use `main()`, which reports and rethrows ordinary startup failures. The packaged executable uses `runCli()` to convert those failures into exit code 1. The wallet-on-HTTP guard is stricter: it terminates immediately with exit code 1 before any listener or signing surface is created. Each `main()` call snapshots wallet configuration for that runtime, so a later start cannot expose signing tools on an already-running HTTP listener or rebind a still-running stdio server's signer. ## Configuration diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index 977a0b59..dee77b1a 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -67,6 +67,14 @@ export function runWithAppConfig(appConfig: AppConfig, fn: () => T): T { return runtimeConfig.run(appConfig, fn); } +/** + * Bind a callback to an AppConfig snapshot so later initializeConfig() + * mutations cannot change wallet policy mid-request. + */ +export function wrapWithAppConfig(appConfig: AppConfig, fn: (...args: Args) => Result): (...args: Args) => Result { + return (...args: Args): Result => runWithAppConfig(appConfig, () => fn(...args)); +} + export function getRuntimeConfig(): AppConfig { return runtimeConfig.getStore() ?? config; } diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index ab432aa2..05a7bcb1 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -1,6 +1,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; import { getSupportedNetworks } from '../core/chains.js'; -import { type AppConfig, config as processConfig, runWithAppConfig } from '../core/config.js'; +import { type AppConfig, config as processConfig, runWithAppConfig, wrapWithAppConfig } from '../core/config.js'; import { sanitizeError } from '../core/errors.js'; import { registerEVMPrompts } from '../core/prompts.js'; import { registerEVMResources } from '../core/resources.js'; @@ -8,6 +9,27 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; +function bindTransportToAppConfig(transport: Transport, appConfig: AppConfig): void { + const start = transport.start.bind(transport); + let bound = false; + transport.start = async () => { + if (!bound && transport.onmessage) { + bound = true; + transport.onmessage = wrapWithAppConfig(appConfig, transport.onmessage); + } + return start(); + }; +} + +function bindServerToAppConfig(server: McpServer, appConfig: AppConfig): McpServer { + const connect = server.connect.bind(server); + server.connect = (async (transport: Transport) => { + bindTransportToAppConfig(transport, appConfig); + return connect(transport); + }) as typeof server.connect; + return server; +} + export const getServer = async (appConfig: AppConfig = processConfig) => { return runWithAppConfig(appConfig, async () => { try { @@ -24,7 +46,7 @@ export const getServer = async (appConfig: AppConfig = processConfig) => { console.error('Supported networks:', getSupportedNetworks().join(', ')); - return server; + return bindServerToAppConfig(server, appConfig); } catch (error) { console.error('Failed to initialize server:', sanitizeError(error)); throw error; diff --git a/packages/mcp-server/src/server/transport/factory.ts b/packages/mcp-server/src/server/transport/factory.ts index 5fe85bb7..71eec655 100644 --- a/packages/mcp-server/src/server/transport/factory.ts +++ b/packages/mcp-server/src/server/transport/factory.ts @@ -8,7 +8,7 @@ export const createTransport = (config: TransportConfig): McpTransport => { switch (config.mode) { case 'stdio': - return new StdioTransport(); + return new StdioTransport(config.appConfig); case 'streamable-http': return new StreamableHttpTransport({ diff --git a/packages/mcp-server/src/server/transport/stdio.ts b/packages/mcp-server/src/server/transport/stdio.ts index 8fdcb340..3ebbbd3f 100644 --- a/packages/mcp-server/src/server/transport/stdio.ts +++ b/packages/mcp-server/src/server/transport/stdio.ts @@ -1,15 +1,25 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; +import { type AppConfig, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../../core/config.js'; import type { McpTransport, TransportMode } from './types.js'; export class StdioTransport implements McpTransport { public readonly mode: TransportMode = 'stdio'; + private readonly appConfig: AppConfig; private transport?: StdioServerTransport; + constructor(appConfig?: AppConfig) { + this.appConfig = appConfig ?? snapshotConfig(); + } + async start(server?: McpServer): Promise { if (!server) throw new Error('STDIO transport requires an MCP server.'); - this.transport = new StdioServerTransport(); - await server.connect(this.transport); + const transport = new StdioServerTransport(); + this.transport = transport; + await runWithAppConfig(this.appConfig, () => server.connect(transport)); + if (transport.onmessage) { + transport.onmessage = wrapWithAppConfig(this.appConfig, transport.onmessage); + } console.error('MCP Server ready (stdio transport)'); } diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index 0b28a7b3..d946684a 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -10,7 +10,8 @@ import { isWalletEnabled, loadConfig, runWithAppConfig, - snapshotConfig + snapshotConfig, + wrapWithAppConfig } from '../../core/config.js'; describe('Config Module - Actual Implementation', () => { @@ -125,6 +126,22 @@ describe('Config Module - Actual Implementation', () => { expect(isWalletEnabled()).toBe(true); expect(getPrivateKeyAsHex()).toBe('0xabcdef'); }); + + test('wrapWithAppConfig keeps later callbacks on the snapshot', () => { + config.walletMode = 'private-key'; + const snapshot = snapshotConfig({ + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }); + const readMode = wrapWithAppConfig(snapshot, () => getWalletMode()); + + expect(getWalletMode()).toBe('private-key'); + expect(readMode()).toBe('disabled'); + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '4'.repeat(64) }); + expect(getWalletMode()).toBe('private-key'); + expect(readMode()).toBe('disabled'); + }); }); describe('getPrivateKeyAsHex', () => { diff --git a/packages/mcp-server/src/tests/core/wallet/index.test.ts b/packages/mcp-server/src/tests/core/wallet/index.test.ts index e31a3653..8c6f98dc 100644 --- a/packages/mcp-server/src/tests/core/wallet/index.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/index.test.ts @@ -22,7 +22,7 @@ jest.mock('../../../core/wallet/providers/disabled.js', () => ({ DisabledWalletProvider: jest.fn() })); -import { getWalletMode } from '../../../core/config.js'; +import { getRuntimeConfig, getWalletMode, config as processConfig } from '../../../core/config.js'; describe('Wallet Provider', () => { const mockPrivateKeyProvider: WalletProvider = { @@ -48,6 +48,8 @@ describe('Wallet Provider', () => { // Reset all mocks jest.resetAllMocks(); + (getRuntimeConfig as jest.Mock).mockImplementation(() => processConfig); + // Setup default mock implementations (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); (DisabledWalletProvider as unknown as jest.Mock).mockImplementation(() => mockDisabledProvider); @@ -96,6 +98,26 @@ describe('Wallet Provider', () => { expect(() => getWalletProvider()).toThrow('Unknown wallet mode: unknown-mode'); }); + + test('keeps a snapshot provider after resetWalletProvider clears the process memo', () => { + const snapshot = { privateKey: '0xabc', walletMode: 'private-key' as const, walletApiKey: undefined }; + (getRuntimeConfig as jest.Mock).mockReturnValue(snapshot); + + const provider1 = getWalletProvider(); + expect(PrivateKeyWalletProvider).toHaveBeenCalledWith('0xabc'); + expect(getWalletMode).not.toHaveBeenCalled(); + expect(provider1).toBe(mockPrivateKeyProvider); + + resetWalletProvider(); + jest.clearAllMocks(); + (getRuntimeConfig as jest.Mock).mockReturnValue(snapshot); + (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); + + const provider2 = getWalletProvider(); + expect(provider2).toBe(provider1); + expect(PrivateKeyWalletProvider).not.toHaveBeenCalled(); + expect(getWalletMode).not.toHaveBeenCalled(); + }); }); describe('resetWalletProvider', () => { diff --git a/packages/mcp-server/src/tests/server/server.test.ts b/packages/mcp-server/src/tests/server/server.test.ts index 8dfaecaf..5da4e263 100644 --- a/packages/mcp-server/src/tests/server/server.test.ts +++ b/packages/mcp-server/src/tests/server/server.test.ts @@ -52,7 +52,8 @@ describe('Server Module', () => { // Create mock server instance mockServerInstance = { name: '@sei-js/mcp-server', - version: '1.0.0' + version: '1.0.0', + connect: jest.fn().mockResolvedValue(undefined) }; // Import mocked functions first diff --git a/packages/mcp-server/src/tests/server/transport/factory.test.ts b/packages/mcp-server/src/tests/server/transport/factory.test.ts index 0a384ddd..9ce62658 100644 --- a/packages/mcp-server/src/tests/server/transport/factory.test.ts +++ b/packages/mcp-server/src/tests/server/transport/factory.test.ts @@ -55,7 +55,7 @@ describe('Transport Factory', () => { const transport = createTransport(config); - expect(StdioTransport).toHaveBeenCalledWith(); + expect(StdioTransport).toHaveBeenCalledWith(undefined); expect(transport).toBe(mockStdioInstance); }); @@ -108,6 +108,27 @@ describe('Transport Factory', () => { expect(transport).toBe(mockSseInstance); }); + it('forwards a frozen appConfig snapshot to stdio transport', () => { + const appConfig = Object.freeze({ + privateKey: `0x${'1'.repeat(64)}`, + walletMode: 'private-key' as const, + walletApiKey: undefined + }); + const config: TransportConfig = { + mode: 'stdio', + walletMode: 'private-key', + appConfig, + port: 3000, + host: 'localhost', + path: '/mcp' + }; + StdioTransport.mockImplementation(() => ({ mode: 'stdio' })); + + createTransport(config); + + expect(StdioTransport).toHaveBeenCalledWith(appConfig); + }); + it('forwards a frozen appConfig snapshot to HTTP transports', () => { const appConfig = Object.freeze({ privateKey: undefined, diff --git a/packages/mcp-server/src/tests/server/transport/stdio.test.ts b/packages/mcp-server/src/tests/server/transport/stdio.test.ts index 0469603c..e2a6dbc5 100644 --- a/packages/mcp-server/src/tests/server/transport/stdio.test.ts +++ b/packages/mcp-server/src/tests/server/transport/stdio.test.ts @@ -48,6 +48,39 @@ describe('StdioTransport', () => { }); describe('start', () => { + it('scopes request handling to the construction snapshot after connect', async () => { + const configModule = await import('../../../core/config.js'); + const { config, getWalletMode, initializeConfig, snapshotConfig } = configModule; + const originalConfig = { ...config }; + const seenModes: string[] = []; + const mockTransportInstance = { + onmessage: undefined as ((message: unknown) => void) | undefined + }; + mockStdioServerTransport.mockImplementation(() => mockTransportInstance); + mockServer.connect.mockImplementation(async (transport: { onmessage?: (message: unknown) => void }) => { + transport.onmessage = () => { + seenModes.push(getWalletMode()); + }; + }); + + try { + const appConfig = snapshotConfig({ + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }); + const transport = new StdioTransport(appConfig); + await transport.start(mockServer); + + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '1'.repeat(64) }); + expect(getWalletMode()).toBe('private-key'); + mockTransportInstance.onmessage?.({ jsonrpc: '2.0', method: 'tools/list', id: 1 }); + expect(seenModes).toEqual(['disabled']); + } finally { + Object.assign(config, originalConfig); + } + }); + it('should create StdioServerTransport and connect server', async () => { const mockTransportInstance = { connect: jest.fn() diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index 9fbc88f0..2066c009 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -2,8 +2,11 @@ import { afterEach, describe, expect, it, jest } from 'bun:test'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { privateKeyToAccount } from 'viem/accounts'; import { config, initializeConfig, isWalletEnabled, snapshotConfig } from '../../../core/config.js'; import { resetWalletProvider } from '../../../core/wallet/index.js'; +import { getServer } from '../../../server/server.js'; import { HttpSseTransport } from '../../../server/transport/http-sse.js'; import { StreamableHttpTransport } from '../../../server/transport/streamable-http.js'; @@ -39,6 +42,14 @@ async function listWalletTools(mode: HttpMode, url: URL): Promise { } } +async function deriveAddressFromPrivateKey(client: Client): Promise { + const result = await client.callTool({ name: 'get_address_from_private_key', arguments: {} }); + expect(result.isError).toBeFalsy(); + const text = result.content.find((block) => block.type === 'text'); + expect(text).toEqual(expect.objectContaining({ type: 'text' })); + return JSON.parse((text as { text: string }).text).address; +} + describe('HTTP wallet isolation across later starts', () => { const transports: Array = []; let consoleErrorSpy: jest.SpiedFunction; @@ -114,3 +125,46 @@ describe('HTTP wallet isolation across later starts', () => { processExit.mockRestore(); }); }); + +describe('stdio wallet isolation after another runtime stop', () => { + let consoleErrorSpy: jest.SpiedFunction; + const originalConfig = { ...config }; + + afterEach(() => { + consoleErrorSpy?.mockRestore(); + Object.assign(config, originalConfig); + resetWalletProvider(); + delete process.env.WALLET_MODE; + delete process.env.PRIVATE_KEY; + }); + + it('keeps signing with the stdio snapshot after resetWalletProvider and a later initializeConfig', async () => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const keyA = '1'.repeat(64); + const keyB = '2'.repeat(64); + const expectedAddress = privateKeyToAccount(`0x${keyA}`).address; + const otherAddress = privateKeyToAccount(`0x${keyB}`).address; + + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: keyA }); + const appConfig = snapshotConfig(); + const server = await getServer(appConfig); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'stdio-wallet-isolation', version: '1.0.0' }); + await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); + + try { + expect(await deriveAddressFromPrivateKey(client)).toBe(expectedAddress); + + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: keyB }); + resetWalletProvider(); + expect(isWalletEnabled()).toBe(true); + expect(config.privateKey).toBe(`0x${keyB}`); + + expect(await deriveAddressFromPrivateKey(client)).toBe(expectedAddress); + expect(otherAddress).not.toBe(expectedAddress); + } finally { + await client.close(); + await server.close(); + } + }); +}); From 66a554e62b1d28a8667fa48a31ebaeafa5c2021e Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 00:37:43 +0200 Subject: [PATCH 03/10] fix(mcp-server): typecheck wallet isolation tool results callTool() can return a union where content is unknown, so the isolation helper now narrows to a text payload before parsing the derived address. Co-authored-by: Cursor --- .../src/tests/server/transport/wallet-isolation.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index 2066c009..d629578b 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -45,9 +45,10 @@ async function listWalletTools(mode: HttpMode, url: URL): Promise { async function deriveAddressFromPrivateKey(client: Client): Promise { const result = await client.callTool({ name: 'get_address_from_private_key', arguments: {} }); expect(result.isError).toBeFalsy(); - const text = result.content.find((block) => block.type === 'text'); - expect(text).toEqual(expect.objectContaining({ type: 'text' })); - return JSON.parse((text as { text: string }).text).address; + if (!('content' in result)) throw new Error('Expected an immediate tool result'); + const content = (result as { content: Array<{ type: string; text?: string }> }).content[0]; + if (content?.type !== 'text' || content.text === undefined) throw new Error('Expected a text tool result'); + return JSON.parse(content.text).address; } describe('HTTP wallet isolation across later starts', () => { From e9082c1344a5e122bacd2def64f7b4642f6024ea Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 00:57:01 +0200 Subject: [PATCH 04/10] fix(mcp-server): fail closed on runtime wallet config Co-authored-by: Cursor --- .../fix-mcp-server-instance-scoped-wallet.md | 2 +- REVIEW.md | 13 +++---- packages/mcp-server/src/core/config.ts | 28 ++++++++++----- packages/mcp-server/src/core/wallet/index.ts | 24 +++++++------ .../src/core/wallet/providers/private-key.ts | 8 +++-- packages/mcp-server/src/index.ts | 5 ++- packages/mcp-server/src/server/server.ts | 18 +++++----- .../src/server/transport/factory.ts | 6 ++-- .../src/server/transport/http-sse.ts | 10 +++--- .../src/server/transport/security.ts | 4 +-- .../mcp-server/src/server/transport/stdio.ts | 8 ++--- .../src/server/transport/streamable-http.ts | 10 +++--- .../mcp-server/src/server/transport/types.ts | 4 +-- .../mcp-server/src/tests/core/config.test.ts | 17 +++++++++ .../src/tests/core/wallet/index.test.ts | 26 +++++++++----- .../core/wallet/providers/private-key.test.ts | 4 +-- packages/mcp-server/src/tests/index.test.ts | 18 ++++------ .../src/tests/server/server.test.ts | 14 +++++++- .../tests/server/transport/factory.test.ts | 15 +++++--- .../tests/server/transport/security.test.ts | 6 ++++ .../server/transport/wallet-isolation.test.ts | 35 +++++++++++++++++-- 21 files changed, 181 insertions(+), 94 deletions(-) diff --git a/.changeset/fix-mcp-server-instance-scoped-wallet.md b/.changeset/fix-mcp-server-instance-scoped-wallet.md index 62328278..2ec99aac 100644 --- a/.changeset/fix-mcp-server-instance-scoped-wallet.md +++ b/.changeset/fix-mcp-server-instance-scoped-wallet.md @@ -6,4 +6,4 @@ Keep each MCP runtime on the wallet configuration that passed its security check A later programmatic `main()` could overwrite the process-wide config object while an HTTP listener started earlier was still serving requests. New sessions on that listener then built their tool list from the updated singleton, so a wallet-disabled HTTP server could expose signing tools after a trusted stdio start in the same process. No shipped CLI or host spawn does that, but the public lifecycle returned independent runtimes without isolating their keys. -`parseArgs()` now returns a frozen `AppConfig` snapshot. HTTP transports close their `serverFactory` and request/session handling over that snapshot, stdio `connect` / `onmessage` stay on the same snapshot after another runtime's `stop()` drops the process-wide wallet provider memo, and both HTTP transports keep wallet tools hidden after a subsequent private-key `initializeConfig()`. Direct wallet-on-HTTP startup still exits 1. +`parseArgs()` now returns a frozen `AppConfig` snapshot, and every transport handles requests against that snapshot. Stopping one runtime releases only its provider, while other runtimes keep their original signer. HTTP startup also checks both its compatibility option and the snapshot that controls signing, so conflicting programmatic options fail closed before listening. diff --git a/REVIEW.md b/REVIEW.md index e454c478..b1b8f898 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -20,15 +20,10 @@ invariants are load-bearing and are enforced in code rather than by convention: transports are reachable cross-origin, so a signing key behind one is a drain-the-wallet primitive. Any change that narrows this check, makes it non-fatal, or adds a transport that bypasses it is a finding. -- **Tool and signing policy is instance-scoped.** Each runtime closes over - the `AppConfig` snapshot that passed `validateSecurityConfig`. HTTP - `serverFactory` / request handling and stdio `connect` / `onmessage` must - not consult a later `initializeConfig()` / `main()` mutation of the process - singleton. `stop()` may drop the process-wide wallet provider memo; a - still-running stdio server must keep signing from its snapshot after that - reset. Treat a default `serverFactory` that calls unbound `getServer()`, - or request handling that reads wallet state outside `runWithAppConfig`, as - a regression. Keep the isolation tests in +- **Tool and signing policy is instance-scoped.** A running transport's tool + list and signer must remain fixed to the `AppConfig` snapshot that passed + its security check. Later process configuration changes, starts, or stops + must not alter that runtime. Keep the cross-runtime coverage in `src/tests/server/transport/wallet-isolation.test.ts` meaningful. - **SSE messages are bound to their session.** `src/server/transport/http-sse.ts` keys `connections` by diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index dee77b1a..5b769b9e 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -26,6 +26,8 @@ export interface AppConfig { walletApiKey: string | undefined; } +export type AppConfigSnapshot = Readonly; + export const loadConfig = (environment: Record = process.env): AppConfig => { const env = envSchema.parse(environment); const privateKey = formatPrivateKey(env.PRIVATE_KEY); @@ -48,14 +50,15 @@ export function initializeConfig(environment: Record = process. return config; } -const runtimeConfig = new AsyncLocalStorage(); +const runtimeConfig = new AsyncLocalStorage(); /** * Copy of the process singleton as it existed at a given start. - * HTTP transports close over this object so a later initializeConfig() cannot - * change an already-running listener's tool policy or signer. + * Transports close over this object so a later initializeConfig() cannot + * change an already-running runtime's tool policy or signer. */ -export function snapshotConfig(source: AppConfig = config): AppConfig { +export function snapshotConfig(source: Readonly = config): AppConfigSnapshot { + if (Object.isFrozen(source)) return source; return Object.freeze({ privateKey: source.privateKey, walletMode: source.walletMode, @@ -63,7 +66,8 @@ export function snapshotConfig(source: AppConfig = config): AppConfig { }); } -export function runWithAppConfig(appConfig: AppConfig, fn: () => T): T { +export function runWithAppConfig(appConfig: AppConfigSnapshot, fn: () => T): T { + if (!Object.isFrozen(appConfig)) throw new TypeError('runWithAppConfig requires a frozen AppConfig snapshot.'); return runtimeConfig.run(appConfig, fn); } @@ -71,12 +75,20 @@ export function runWithAppConfig(appConfig: AppConfig, fn: () => T): T { * Bind a callback to an AppConfig snapshot so later initializeConfig() * mutations cannot change wallet policy mid-request. */ -export function wrapWithAppConfig(appConfig: AppConfig, fn: (...args: Args) => Result): (...args: Args) => Result { +export function wrapWithAppConfig(appConfig: AppConfigSnapshot, fn: (...args: Args) => Result): (...args: Args) => Result { return (...args: Args): Result => runWithAppConfig(appConfig, () => fn(...args)); } -export function getRuntimeConfig(): AppConfig { - return runtimeConfig.getStore() ?? config; +export function getScopedAppConfig(): AppConfigSnapshot | undefined { + return runtimeConfig.getStore(); +} + +/** + * Runtime request paths must establish an AsyncLocalStorage scope. The mutable + * process config remains the fallback for direct configuration helpers. + */ +export function getRuntimeConfig(): Readonly { + return getScopedAppConfig() ?? config; } /** diff --git a/packages/mcp-server/src/core/wallet/index.ts b/packages/mcp-server/src/core/wallet/index.ts index b5f4164d..e600c565 100644 --- a/packages/mcp-server/src/core/wallet/index.ts +++ b/packages/mcp-server/src/core/wallet/index.ts @@ -1,17 +1,17 @@ -import { type AppConfig, getRuntimeConfig, getWalletMode, config as processConfig } from '../config.js'; +import { type AppConfigSnapshot, getScopedAppConfig, getWalletMode } from '../config.js'; import { DisabledWalletProvider } from './providers/disabled.js'; import { PrivateKeyWalletProvider } from './providers/private-key.js'; import type { WalletProvider } from './types.js'; -const providersByConfig = new WeakMap(); +const providersByConfig = new WeakMap(); // Cache for the process-global singleton only. Instance snapshots use the WeakMap. let walletProviderInstance: WalletProvider | null = null; -function createWalletProvider(appConfig: AppConfig): WalletProvider { +function createWalletProvider(appConfig: AppConfigSnapshot): WalletProvider { switch (appConfig.walletMode) { case 'private-key': - return new PrivateKeyWalletProvider(appConfig.privateKey); + return new PrivateKeyWalletProvider({ privateKey: appConfig.privateKey }); case 'disabled': return new DisabledWalletProvider(); default: @@ -19,7 +19,7 @@ function createWalletProvider(appConfig: AppConfig): WalletProvider { } } -function providerForSnapshot(appConfig: AppConfig): WalletProvider { +function providerForSnapshot(appConfig: AppConfigSnapshot): WalletProvider { const cached = providersByConfig.get(appConfig); if (cached) return cached; const provider = createWalletProvider(appConfig); @@ -31,8 +31,8 @@ function providerForSnapshot(appConfig: AppConfig): WalletProvider { * Get the wallet provider instance based on configuration */ export function getWalletProvider(): WalletProvider { - const runtime = getRuntimeConfig(); - if (runtime !== processConfig) { + const runtime = getScopedAppConfig(); + if (runtime) { return providerForSnapshot(runtime); } @@ -57,10 +57,14 @@ export function getWalletProvider(): WalletProvider { } /** - * Reset the wallet provider instance (useful for testing) + * Release one runtime's provider, or reset the process fallback for tests. */ -export function resetWalletProvider(): void { - walletProviderInstance = null; +export function resetWalletProvider(appConfig?: AppConfigSnapshot): void { + if (appConfig) { + providersByConfig.delete(appConfig); + } else { + walletProviderInstance = null; + } } export { DisabledWalletProvider } from './providers/disabled.js'; diff --git a/packages/mcp-server/src/core/wallet/providers/private-key.ts b/packages/mcp-server/src/core/wallet/providers/private-key.ts index 3f4299d1..7ba9b43c 100644 --- a/packages/mcp-server/src/core/wallet/providers/private-key.ts +++ b/packages/mcp-server/src/core/wallet/providers/private-key.ts @@ -6,6 +6,10 @@ import { getPrivateKeyAsHex } from '../../config.js'; import type { TransactionRequest, WalletProvider } from '../types.js'; import { WalletProviderError } from '../types.js'; +export interface PrivateKeyWalletProviderOptions { + privateKey: string | undefined; +} + /** * Private Key Wallet Provider * Uses a private key from environment variables @@ -13,8 +17,8 @@ import { WalletProviderError } from '../types.js'; export class PrivateKeyWalletProvider implements WalletProvider { private privateKey: string | undefined; - constructor(...key: [] | [string | undefined]) { - this.privateKey = key.length === 0 ? getPrivateKeyAsHex() : key[0]; + constructor(options?: PrivateKeyWalletProviderOptions) { + this.privateKey = options ? options.privateKey : getPrivateKeyAsHex(); } isAvailable(): boolean { diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index cc54057b..9a7d6294 100755 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -1,6 +1,5 @@ import { realpathSync } from 'node:fs'; import { pathToFileURL } from 'node:url'; -import { isWalletEnabled } from './core/config.js'; import { sanitizeError } from './core/errors.js'; import { resetWalletProvider } from './core/wallet/index.js'; import { parseArgs } from './server/args.js'; @@ -53,7 +52,7 @@ export async function startMcpServer(): Promise { throw error; } - if (!isWalletEnabled()) console.error('Wallet functionality is disabled. Signing and broadcasting tools are not available.'); + if (config.walletMode === 'disabled') console.error('Wallet functionality is disabled. Signing and broadcasting tools are not available.'); let stopPromise: Promise | undefined; let removeShutdownHandlers = () => {}; @@ -65,7 +64,7 @@ export async function startMcpServer(): Promise { const errors = await collectOperationErrors([() => transport.stop(), () => server?.close()]); throwCollectedErrors(errors, 'Failed to stop all MCP server resources.'); } finally { - resetWalletProvider(); + if (config.appConfig) resetWalletProvider(config.appConfig); } })(); } diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index 05a7bcb1..9352ceec 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -1,7 +1,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; import { getSupportedNetworks } from '../core/chains.js'; -import { type AppConfig, config as processConfig, runWithAppConfig, wrapWithAppConfig } from '../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../core/config.js'; import { sanitizeError } from '../core/errors.js'; import { registerEVMPrompts } from '../core/prompts.js'; import { registerEVMResources } from '../core/resources.js'; @@ -9,7 +9,7 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; -function bindTransportToAppConfig(transport: Transport, appConfig: AppConfig): void { +function bindTransportToAppConfig(transport: Transport, appConfig: AppConfigSnapshot): void { const start = transport.start.bind(transport); let bound = false; transport.start = async () => { @@ -21,17 +21,19 @@ function bindTransportToAppConfig(transport: Transport, appConfig: AppConfig): v }; } -function bindServerToAppConfig(server: McpServer, appConfig: AppConfig): McpServer { +function bindServerToAppConfig(server: McpServer, appConfig: AppConfigSnapshot): McpServer { const connect = server.connect.bind(server); - server.connect = (async (transport: Transport) => { + server.connect = (async (...args: Parameters) => { + const [transport] = args; bindTransportToAppConfig(transport, appConfig); - return connect(transport); + return connect(...args); }) as typeof server.connect; return server; } -export const getServer = async (appConfig: AppConfig = processConfig) => { - return runWithAppConfig(appConfig, async () => { +export const getServer = async (appConfig: AppConfigSnapshot = snapshotConfig()) => { + const config = snapshotConfig(appConfig); + return runWithAppConfig(config, async () => { try { const packageInfo = getPackageInfo(); const server = new McpServer({ @@ -46,7 +48,7 @@ export const getServer = async (appConfig: AppConfig = processConfig) => { console.error('Supported networks:', getSupportedNetworks().join(', ')); - return bindServerToAppConfig(server, appConfig); + return bindServerToAppConfig(server, config); } catch (error) { console.error('Failed to initialize server:', sanitizeError(error)); throw error; diff --git a/packages/mcp-server/src/server/transport/factory.ts b/packages/mcp-server/src/server/transport/factory.ts index 71eec655..3a1d85af 100644 --- a/packages/mcp-server/src/server/transport/factory.ts +++ b/packages/mcp-server/src/server/transport/factory.ts @@ -4,8 +4,6 @@ import { StreamableHttpTransport } from './streamable-http.js'; import type { McpTransport, TransportConfig } from './types.js'; export const createTransport = (config: TransportConfig): McpTransport => { - const appConfig = config.appConfig ? { appConfig: config.appConfig } : {}; - switch (config.mode) { case 'stdio': return new StdioTransport(config.appConfig); @@ -17,7 +15,7 @@ export const createTransport = (config: TransportConfig): McpTransport => { path: config.path, walletMode: config.walletMode, maxActiveRequests: config.maxStreamableRequests, - ...appConfig + appConfig: config.appConfig }); case 'http-sse': @@ -27,7 +25,7 @@ export const createTransport = (config: TransportConfig): McpTransport => { path: config.path, walletMode: config.walletMode, maxSessions: config.maxSseSessions, - ...appConfig + appConfig: config.appConfig }); default: diff --git a/packages/mcp-server/src/server/transport/http-sse.ts b/packages/mcp-server/src/server/transport/http-sse.ts index 8114181d..8a54bbb2 100644 --- a/packages/mcp-server/src/server/transport/http-sse.ts +++ b/packages/mcp-server/src/server/transport/http-sse.ts @@ -3,7 +3,7 @@ import type { Socket } from 'node:net'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js'; import express, { type Request, type Response } from 'express'; -import { type AppConfig, runWithAppConfig, snapshotConfig } from '../../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig } from '../../core/config.js'; import { sanitizeError } from '../../core/errors.js'; import { getServer } from '../server.js'; import { closeHttpServer, collectOperationErrors, runAllOperations, throwCollectedErrors } from './lifecycle.js'; @@ -21,7 +21,7 @@ export interface HttpSseTransportOptions { host: string; path: string; walletMode?: WalletMode; - appConfig?: AppConfig; + appConfig?: AppConfigSnapshot; maxSessions?: number; } @@ -56,7 +56,7 @@ export class HttpSseTransport implements McpTransport { private readonly host: string; private readonly path: string; private readonly walletMode: WalletMode; - private readonly appConfig: AppConfig; + private readonly appConfig: AppConfigSnapshot; private readonly serverFactory: McpServerFactory; private readonly transportFactory: SseServerTransportFactory; private readonly listenFactory: SseListenFactory; @@ -67,7 +67,7 @@ export class HttpSseTransport implements McpTransport { this.host = options.host; this.path = options.path; this.walletMode = options.walletMode ?? 'disabled'; - this.appConfig = options.appConfig ?? snapshotConfig(); + this.appConfig = snapshotConfig(options.appConfig); this.maxSessions = options.maxSessions ?? DEFAULT_MAX_SSE_SESSIONS; this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = dependencies.transportFactory ?? ((endpoint, response) => new SSEServerTransport(endpoint, response)); @@ -225,7 +225,7 @@ export class HttpSseTransport implements McpTransport { } async start(_server?: McpServer): Promise { - validateSecurityConfig(this.mode, this.walletMode); + validateSecurityConfig(this.mode, this.walletMode, this.appConfig.walletMode); if (this.host.trim().length === 0) { throw new Error('SERVER_HOST must not be empty.'); } diff --git a/packages/mcp-server/src/server/transport/security.ts b/packages/mcp-server/src/server/transport/security.ts index 231024c0..3c3bafcd 100644 --- a/packages/mcp-server/src/server/transport/security.ts +++ b/packages/mcp-server/src/server/transport/security.ts @@ -19,9 +19,9 @@ export function createCorsMiddleware(): RequestHandler { * Validates that wallet mode is not used with HTTP transports * Exits the process if unsafe configuration detected */ -export function validateSecurityConfig(transportMode: TransportMode, walletMode: WalletMode): void { +export function validateSecurityConfig(transportMode: TransportMode, walletMode: WalletMode, ...additionalWalletModes: WalletMode[]): void { const isHttpTransport = transportMode === 'streamable-http' || transportMode === 'http-sse'; - const isWalletEnabled = walletMode !== 'disabled'; + const isWalletEnabled = [walletMode, ...additionalWalletModes].some((mode) => mode !== 'disabled'); if (isHttpTransport && isWalletEnabled) { console.error(''); diff --git a/packages/mcp-server/src/server/transport/stdio.ts b/packages/mcp-server/src/server/transport/stdio.ts index 3ebbbd3f..660810fd 100644 --- a/packages/mcp-server/src/server/transport/stdio.ts +++ b/packages/mcp-server/src/server/transport/stdio.ts @@ -1,15 +1,15 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; -import { type AppConfig, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../../core/config.js'; import type { McpTransport, TransportMode } from './types.js'; export class StdioTransport implements McpTransport { public readonly mode: TransportMode = 'stdio'; - private readonly appConfig: AppConfig; + private readonly appConfig: AppConfigSnapshot; private transport?: StdioServerTransport; - constructor(appConfig?: AppConfig) { - this.appConfig = appConfig ?? snapshotConfig(); + constructor(appConfig?: AppConfigSnapshot) { + this.appConfig = snapshotConfig(appConfig); } async start(server?: McpServer): Promise { diff --git a/packages/mcp-server/src/server/transport/streamable-http.ts b/packages/mcp-server/src/server/transport/streamable-http.ts index bd892f1d..5ba0e343 100644 --- a/packages/mcp-server/src/server/transport/streamable-http.ts +++ b/packages/mcp-server/src/server/transport/streamable-http.ts @@ -3,7 +3,7 @@ import type { Socket } from 'node:net'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import express, { type Request, type Response } from 'express'; -import { type AppConfig, runWithAppConfig, snapshotConfig } from '../../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig } from '../../core/config.js'; import { sanitizeError } from '../../core/errors.js'; import { getServer } from '../server.js'; import { closeHttpServer, collectOperationErrors, runAllOperations, throwCollectedErrors } from './lifecycle.js'; @@ -21,7 +21,7 @@ export interface StreamableHttpTransportOptions { host?: string; path?: string; walletMode?: WalletMode; - appConfig?: AppConfig; + appConfig?: AppConfigSnapshot; maxActiveRequests?: number; } @@ -56,7 +56,7 @@ export class StreamableHttpTransport implements McpTransport { private readonly host: string; private readonly path: string; private readonly walletMode: WalletMode; - private readonly appConfig: AppConfig; + private readonly appConfig: AppConfigSnapshot; private readonly serverFactory: StreamableServerFactory; private readonly transportFactory: StreamableTransportFactory; private readonly listenFactory: StreamableListenFactory; @@ -67,7 +67,7 @@ export class StreamableHttpTransport implements McpTransport { this.host = options.host ?? 'localhost'; this.path = options.path ?? '/mcp'; this.walletMode = options.walletMode ?? 'disabled'; - this.appConfig = options.appConfig ?? snapshotConfig(); + this.appConfig = snapshotConfig(options.appConfig); this.maxActiveRequests = options.maxActiveRequests ?? DEFAULT_MAX_STREAMABLE_REQUESTS; this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = @@ -98,7 +98,7 @@ export class StreamableHttpTransport implements McpTransport { } async start(_server?: McpServer): Promise { - validateSecurityConfig(this.mode, this.walletMode); + validateSecurityConfig(this.mode, this.walletMode, this.appConfig.walletMode); if (this.host.trim().length === 0) { throw new Error('SERVER_HOST must not be empty.'); } diff --git a/packages/mcp-server/src/server/transport/types.ts b/packages/mcp-server/src/server/transport/types.ts index aace4f96..f9bda376 100644 --- a/packages/mcp-server/src/server/transport/types.ts +++ b/packages/mcp-server/src/server/transport/types.ts @@ -1,5 +1,5 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; -import type { AppConfig, WalletMode } from '../../core/config.js'; +import type { AppConfigSnapshot, WalletMode } from '../../core/config.js'; export type TransportMode = 'stdio' | 'streamable-http' | 'http-sse'; @@ -12,7 +12,7 @@ export interface McpTransport { export interface TransportConfig { mode: TransportMode; walletMode: WalletMode; - appConfig?: AppConfig; + appConfig?: AppConfigSnapshot; port: number; // Required for HTTP-based transports host: string; // Required for HTTP-based transports path: string; // Required for HTTP-based transports diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index d946684a..041dd93a 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -4,6 +4,7 @@ import { formatPrivateKey, getPrivateKeyAsHex, getRuntimeConfig, + getScopedAppConfig, getWalletMode, initializeConfig, isValidPrivateKey, @@ -99,6 +100,7 @@ describe('Config Module - Actual Implementation', () => { }); expect(snapshot).not.toBe(config); expect(Object.isFrozen(snapshot)).toBe(true); + expect(snapshotConfig(snapshot)).toBe(snapshot); const privateKey = '3'.repeat(64); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: privateKey }); @@ -107,6 +109,19 @@ describe('Config Module - Actual Implementation', () => { expect(config.walletMode).toBe('private-key'); }); + test('runWithAppConfig rejects mutable configuration objects', () => { + expect(() => + runWithAppConfig( + { + privateKey: undefined, + walletMode: 'disabled', + walletApiKey: undefined + }, + () => undefined + ) + ).toThrow('runWithAppConfig requires a frozen AppConfig snapshot.'); + }); + test('runWithAppConfig makes getters read the snapshot instead of the process singleton', () => { config.walletMode = 'private-key'; config.privateKey = '0xabcdef'; @@ -122,9 +137,11 @@ describe('Config Module - Actual Implementation', () => { expect(getWalletMode()).toBe('disabled'); expect(getPrivateKeyAsHex()).toBeUndefined(); expect(getRuntimeConfig()).toBe(snapshot); + expect(getScopedAppConfig()).toBe(snapshot); }); expect(isWalletEnabled()).toBe(true); expect(getPrivateKeyAsHex()).toBe('0xabcdef'); + expect(getScopedAppConfig()).toBeUndefined(); }); test('wrapWithAppConfig keeps later callbacks on the snapshot', () => { diff --git a/packages/mcp-server/src/tests/core/wallet/index.test.ts b/packages/mcp-server/src/tests/core/wallet/index.test.ts index 8c6f98dc..13967cfd 100644 --- a/packages/mcp-server/src/tests/core/wallet/index.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/index.test.ts @@ -6,11 +6,9 @@ import type { WalletProvider } from '../../../core/wallet/types.js'; // Mock dependencies jest.mock('../../../core/config.js', () => { - const config = { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }; return { getWalletMode: jest.fn(), - getRuntimeConfig: jest.fn(() => config), - config + getScopedAppConfig: jest.fn() }; }); @@ -22,7 +20,7 @@ jest.mock('../../../core/wallet/providers/disabled.js', () => ({ DisabledWalletProvider: jest.fn() })); -import { getRuntimeConfig, getWalletMode, config as processConfig } from '../../../core/config.js'; +import { getScopedAppConfig, getWalletMode } from '../../../core/config.js'; describe('Wallet Provider', () => { const mockPrivateKeyProvider: WalletProvider = { @@ -48,7 +46,7 @@ describe('Wallet Provider', () => { // Reset all mocks jest.resetAllMocks(); - (getRuntimeConfig as jest.Mock).mockImplementation(() => processConfig); + (getScopedAppConfig as jest.Mock).mockReturnValue(undefined); // Setup default mock implementations (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); @@ -101,16 +99,16 @@ describe('Wallet Provider', () => { test('keeps a snapshot provider after resetWalletProvider clears the process memo', () => { const snapshot = { privateKey: '0xabc', walletMode: 'private-key' as const, walletApiKey: undefined }; - (getRuntimeConfig as jest.Mock).mockReturnValue(snapshot); + (getScopedAppConfig as jest.Mock).mockReturnValue(snapshot); const provider1 = getWalletProvider(); - expect(PrivateKeyWalletProvider).toHaveBeenCalledWith('0xabc'); + expect(PrivateKeyWalletProvider).toHaveBeenCalledWith({ privateKey: '0xabc' }); expect(getWalletMode).not.toHaveBeenCalled(); expect(provider1).toBe(mockPrivateKeyProvider); resetWalletProvider(); jest.clearAllMocks(); - (getRuntimeConfig as jest.Mock).mockReturnValue(snapshot); + (getScopedAppConfig as jest.Mock).mockReturnValue(snapshot); (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); const provider2 = getWalletProvider(); @@ -118,6 +116,18 @@ describe('Wallet Provider', () => { expect(PrivateKeyWalletProvider).not.toHaveBeenCalled(); expect(getWalletMode).not.toHaveBeenCalled(); }); + + test('recreates only the provider for a reset snapshot', () => { + const snapshot = { privateKey: '0xabc', walletMode: 'private-key' as const, walletApiKey: undefined }; + const providerAfterReset = { ...mockPrivateKeyProvider }; + (getScopedAppConfig as jest.Mock).mockReturnValue(snapshot); + (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementationOnce(() => mockPrivateKeyProvider).mockImplementationOnce(() => providerAfterReset); + + expect(getWalletProvider()).toBe(mockPrivateKeyProvider); + resetWalletProvider(snapshot); + expect(getWalletProvider()).toBe(providerAfterReset); + expect(PrivateKeyWalletProvider).toHaveBeenCalledTimes(2); + }); }); describe('resetWalletProvider', () => { diff --git a/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts b/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts index 852e41d0..5b1def58 100644 --- a/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/providers/private-key.test.ts @@ -64,10 +64,10 @@ describe('PrivateKeyWalletProvider', () => { expect(provider.isAvailable()).toBe(false); }); - test('does not read the process singleton when constructed with an explicit key slot', () => { + test('does not read the process singleton when constructed with explicit options', () => { (getPrivateKeyAsHex as jest.Mock).mockReturnValue(mockPrivateKey); - const provider = new PrivateKeyWalletProvider(undefined); + const provider = new PrivateKeyWalletProvider({ privateKey: undefined }); expect(provider.isAvailable()).toBe(false); expect(getPrivateKeyAsHex).not.toHaveBeenCalled(); diff --git a/packages/mcp-server/src/tests/index.test.ts b/packages/mcp-server/src/tests/index.test.ts index edd83bc6..ae8aa089 100644 --- a/packages/mcp-server/src/tests/index.test.ts +++ b/packages/mcp-server/src/tests/index.test.ts @@ -9,10 +9,6 @@ jest.mock('../server/transport/index.js', () => ({ createTransport: jest.fn() })); -jest.mock('../core/config.js', () => ({ - isWalletEnabled: jest.fn() -})); - jest.mock('../core/wallet/index.js', () => ({ resetWalletProvider: jest.fn() })); @@ -24,10 +20,10 @@ jest.mock('../server/args.js', () => ({ describe('index', () => { let mockGetServer: jest.MockedFunction<() => Promise>; let mockCreateTransport: jest.MockedFunction<(config: unknown) => unknown>; - let mockIsWalletEnabled: jest.MockedFunction<() => boolean>; let mockParseArgs: jest.MockedFunction<() => unknown>; let mockTransport: { start: jest.Mock; stop: jest.Mock }; let mockServer: { close: jest.Mock }; + let mockAppConfig: Readonly<{ privateKey: string | undefined; walletMode: 'private-key' | 'disabled'; walletApiKey: string | undefined }>; let consoleErrorSpy: jest.SpyInstance; let processExitSpy: jest.SpyInstance; let originalExitCode: number | string | null | undefined; @@ -39,12 +35,10 @@ describe('index', () => { // Import mocked modules const serverModule = await import('../server/server.js'); const transportModule = await import('../server/transport/index.js'); - const configModule = await import('../core/config.js'); const argsModule = await import('../server/args.js'); mockGetServer = serverModule.getServer as jest.MockedFunction<() => Promise>; mockCreateTransport = transportModule.createTransport as jest.MockedFunction<(config: unknown) => unknown>; - mockIsWalletEnabled = configModule.isWalletEnabled as jest.MockedFunction<() => boolean>; mockParseArgs = argsModule.parseArgs as jest.MockedFunction<() => unknown>; // Setup mock objects @@ -57,10 +51,10 @@ describe('index', () => { }; // Setup default mock implementations - mockParseArgs.mockReturnValue({ mode: 'stdio' }); + mockAppConfig = Object.freeze({ privateKey: '0xabc', walletMode: 'private-key', walletApiKey: undefined }); + mockParseArgs.mockReturnValue({ mode: 'stdio', walletMode: 'private-key', appConfig: mockAppConfig }); mockGetServer.mockResolvedValue(mockServer); mockCreateTransport.mockReturnValue(mockTransport); - mockIsWalletEnabled.mockReturnValue(true); // Spy on console and process consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); @@ -89,13 +83,13 @@ describe('index', () => { expect(mockGetServer).toHaveBeenCalled(); expect(mockCreateTransport).toHaveBeenCalled(); expect(mockTransport.start).toHaveBeenCalledWith(mockServer); - expect(mockIsWalletEnabled).toHaveBeenCalled(); expect(consoleErrorSpy).not.toHaveBeenCalled(); await runtime?.stop(); }); it('should log warning when wallet is disabled', async () => { - mockIsWalletEnabled.mockReturnValue(false); + const appConfig = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); + mockParseArgs.mockReturnValue({ mode: 'stdio', walletMode: 'disabled', appConfig }); const indexModule = await import('../index.js'); const runtime = await indexModule.main(); @@ -181,7 +175,7 @@ describe('index', () => { expect(mockTransport.stop).toHaveBeenCalledTimes(1); expect(mockServer.close).toHaveBeenCalledTimes(1); const walletModule = await import('../core/wallet/index.js'); - expect(walletModule.resetWalletProvider).toHaveBeenCalled(); + expect(walletModule.resetWalletProvider).toHaveBeenCalledWith(mockAppConfig); }); it('runs graceful shutdown once and allows signal handlers to be removed', async () => { diff --git a/packages/mcp-server/src/tests/server/server.test.ts b/packages/mcp-server/src/tests/server/server.test.ts index 5da4e263..9585d8f8 100644 --- a/packages/mcp-server/src/tests/server/server.test.ts +++ b/packages/mcp-server/src/tests/server/server.test.ts @@ -45,15 +45,17 @@ describe('Server Module', () => { let consoleErrorSpy: jest.SpiedFunction; let processExitSpy: jest.SpiedFunction; let mockServerInstance: any; + let mockConnect: jest.Mock; beforeEach(async () => { jest.clearAllMocks(); // Create mock server instance + mockConnect = jest.fn().mockResolvedValue(undefined); mockServerInstance = { name: '@sei-js/mcp-server', version: '1.0.0', - connect: jest.fn().mockResolvedValue(undefined) + connect: mockConnect }; // Import mocked functions first @@ -122,6 +124,16 @@ describe('Server Module', () => { expect(consoleErrorSpy).toHaveBeenCalledWith('Supported networks:', 'sei, sei-testnet'); }); + it('forwards all connect arguments after binding runtime context', async () => { + const server = await getServer(); + const transport = { start: jest.fn() }; + const options = { testOption: true }; + + await (server.connect as unknown as (...args: unknown[]) => Promise)(transport, options); + + expect(mockConnect).toHaveBeenCalledWith(transport, options); + }); + it('should sanitize and propagate server initialization errors', async () => { const testError = new Error('Initialization failed'); mockGetPackageInfo.mockImplementation(() => { diff --git a/packages/mcp-server/src/tests/server/transport/factory.test.ts b/packages/mcp-server/src/tests/server/transport/factory.test.ts index 9ce62658..75b65ee0 100644 --- a/packages/mcp-server/src/tests/server/transport/factory.test.ts +++ b/packages/mcp-server/src/tests/server/transport/factory.test.ts @@ -78,7 +78,8 @@ describe('Transport Factory', () => { host: '0.0.0.0', path: '/api/mcp', walletMode: 'private-key', - maxActiveRequests: undefined + maxActiveRequests: undefined, + appConfig: undefined }); expect(transport).toBe(mockStreamableInstance); }); @@ -103,7 +104,8 @@ describe('Transport Factory', () => { host: '127.0.0.1', path: '/sse', walletMode: 'disabled', - maxSessions: 25 + maxSessions: 25, + appConfig: undefined }); expect(transport).toBe(mockSseInstance); }); @@ -191,7 +193,8 @@ describe('Transport Factory', () => { expect(StreamableHttpTransport).toHaveBeenCalledWith({ ...params, walletMode: 'disabled', - maxActiveRequests: undefined + maxActiveRequests: undefined, + appConfig: undefined }); expect(transport).toBe(mockInstance); @@ -219,7 +222,8 @@ describe('Transport Factory', () => { host: '::1', path: '/', walletMode: 'private-key', - maxSessions: undefined + maxSessions: undefined, + appConfig: undefined }); expect(transport1).toBe(mockInstance1); @@ -244,7 +248,8 @@ describe('Transport Factory', () => { host: '0.0.0.0', path: '/very/long/path/to/test/edge/cases', walletMode: 'disabled', - maxActiveRequests: undefined + maxActiveRequests: undefined, + appConfig: undefined }); expect(transport2).toBe(mockInstance2); }); diff --git a/packages/mcp-server/src/tests/server/transport/security.test.ts b/packages/mcp-server/src/tests/server/transport/security.test.ts index da2817ef..7b14d501 100644 --- a/packages/mcp-server/src/tests/server/transport/security.test.ts +++ b/packages/mcp-server/src/tests/server/transport/security.test.ts @@ -124,6 +124,12 @@ describe('Security Module', () => { expect(processExitSpy).toHaveBeenCalledWith(1); }); + it('should reject HTTP when any configuration source enables the wallet', () => { + expect(() => validateSecurityConfig('streamable-http', 'disabled', 'private-key')).toThrow('process.exit called with code 1'); + expect(() => validateSecurityConfig('http-sse', 'disabled', 'private-key')).toThrow('process.exit called with code 1'); + expect(processExitSpy).toHaveBeenCalledTimes(2); + }); + it('should terminate directly with a safe diagnostic for unsafe config', () => { expect(() => validateSecurityConfig('streamable-http', 'private-key')).toThrow('process.exit called with code 1'); expect(consoleErrorSpy).toHaveBeenCalledWith('â•‘ Wallet mode cannot be used with HTTP transports! â•‘'); diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index d629578b..4fb588bf 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -4,8 +4,8 @@ import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { privateKeyToAccount } from 'viem/accounts'; -import { config, initializeConfig, isWalletEnabled, snapshotConfig } from '../../../core/config.js'; -import { resetWalletProvider } from '../../../core/wallet/index.js'; +import { config, initializeConfig, isWalletEnabled, runWithAppConfig, snapshotConfig } from '../../../core/config.js'; +import { getWalletProvider, resetWalletProvider } from '../../../core/wallet/index.js'; import { getServer } from '../../../server/server.js'; import { HttpSseTransport } from '../../../server/transport/http-sse.js'; import { StreamableHttpTransport } from '../../../server/transport/streamable-http.js'; @@ -125,6 +125,31 @@ describe('HTTP wallet isolation across later starts', () => { expect(listenFactory).not.toHaveBeenCalled(); processExit.mockRestore(); }); + + it.each([ + ['http-sse', undefined], + ['http-sse', 'disabled'], + ['streamable-http', undefined], + ['streamable-http', 'disabled'] + ] as const)('rejects %s when appConfig enables the wallet and walletMode is %s', async (mode, walletMode) => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const processExit = jest.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`process.exit called with code ${code}`); + }); + const listenFactory = jest.fn(); + const appConfig = snapshotConfig({ + privateKey: `0x${PRIVATE_KEY}`, + walletMode: 'private-key', + walletApiKey: undefined + }); + const options = { port: 8080, host: HOST, path: PATH, walletMode, appConfig }; + const transport = mode === 'http-sse' ? new HttpSseTransport(options, { listenFactory }) : new StreamableHttpTransport(options, { listenFactory }); + + await expect(transport.start()).rejects.toThrow('process.exit called with code 1'); + expect(processExit).toHaveBeenCalledWith(1); + expect(listenFactory).not.toHaveBeenCalled(); + processExit.mockRestore(); + }); }); describe('stdio wallet isolation after another runtime stop', () => { @@ -154,10 +179,14 @@ describe('stdio wallet isolation after another runtime stop', () => { await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); try { + const listed = await client.listTools(); + expect(listed.tools.filter((tool) => WALLET_TOOLS.has(tool.name))).not.toEqual([]); expect(await deriveAddressFromPrivateKey(client)).toBe(expectedAddress); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: keyB }); - resetWalletProvider(); + const otherAppConfig = snapshotConfig(); + runWithAppConfig(otherAppConfig, () => getWalletProvider()); + resetWalletProvider(otherAppConfig); expect(isWalletEnabled()).toBe(true); expect(config.privateKey).toBe(`0x${keyB}`); From c06418b0dd81cc1da2279be091c407adad9d6872 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 10:00:56 +0200 Subject: [PATCH 05/10] fix(mcp-server): tighten runtime config boundaries Co-authored-by: Cursor --- .../fix-mcp-server-instance-scoped-wallet.md | 2 +- REVIEW.md | 7 +-- packages/mcp-server/src/core/config.ts | 11 ++++- packages/mcp-server/src/index.ts | 2 +- packages/mcp-server/src/server/server.ts | 42 +++++++++++++---- .../src/server/transport/factory.ts | 2 - .../src/server/transport/http-sse.ts | 11 +++-- .../src/server/transport/security.ts | 4 +- .../mcp-server/src/server/transport/stdio.ts | 7 ++- .../src/server/transport/streamable-http.ts | 13 +++-- .../mcp-server/src/server/transport/types.ts | 2 +- .../mcp-server/src/tests/core/config.test.ts | 18 ++++++- packages/mcp-server/src/tests/index.test.ts | 3 +- .../src/tests/server/server.test.ts | 15 ++++++ .../tests/server/transport/factory.test.ts | 38 +++++++++------ .../tests/server/transport/http-sse.test.ts | 31 +++++++----- .../tests/server/transport/security.test.ts | 6 --- .../server/transport/streamable-http.test.ts | 35 +++++++++----- .../server/transport/wallet-isolation.test.ts | 47 +++++++++++++------ 19 files changed, 197 insertions(+), 99 deletions(-) diff --git a/.changeset/fix-mcp-server-instance-scoped-wallet.md b/.changeset/fix-mcp-server-instance-scoped-wallet.md index 2ec99aac..c8d67198 100644 --- a/.changeset/fix-mcp-server-instance-scoped-wallet.md +++ b/.changeset/fix-mcp-server-instance-scoped-wallet.md @@ -6,4 +6,4 @@ Keep each MCP runtime on the wallet configuration that passed its security check A later programmatic `main()` could overwrite the process-wide config object while an HTTP listener started earlier was still serving requests. New sessions on that listener then built their tool list from the updated singleton, so a wallet-disabled HTTP server could expose signing tools after a trusted stdio start in the same process. No shipped CLI or host spawn does that, but the public lifecycle returned independent runtimes without isolating their keys. -`parseArgs()` now returns a frozen `AppConfig` snapshot, and every transport handles requests against that snapshot. Stopping one runtime releases only its provider, while other runtimes keep their original signer. HTTP startup also checks both its compatibility option and the snapshot that controls signing, so conflicting programmatic options fail closed before listening. +`parseArgs()` now returns a frozen `AppConfig` snapshot, and every transport handles requests against that snapshot. Stopping one runtime releases only its provider, while other runtimes keep their original signer. HTTP transports require that snapshot, derive signing policy from it, and reject conflicting legacy `walletMode` options. diff --git a/REVIEW.md b/REVIEW.md index b1b8f898..4f5a58ef 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -11,7 +11,7 @@ like bugs in isolation but are deliberate. ## 1. `packages/mcp-server` is the security surface This package hands blockchain capabilities to an LLM client, so it is the one -place in the repo where a subtle regression can cost users funds. Three +place in the repo where a subtle regression can cost users funds. Two invariants are load-bearing and are enforced in code rather than by convention: - **Wallet mode is stdio-only.** `validateSecurityConfig()` in @@ -20,11 +20,6 @@ invariants are load-bearing and are enforced in code rather than by convention: transports are reachable cross-origin, so a signing key behind one is a drain-the-wallet primitive. Any change that narrows this check, makes it non-fatal, or adds a transport that bypasses it is a finding. -- **Tool and signing policy is instance-scoped.** A running transport's tool - list and signer must remain fixed to the `AppConfig` snapshot that passed - its security check. Later process configuration changes, starts, or stops - must not alter that runtime. Keep the cross-runtime coverage in - `src/tests/server/transport/wallet-isolation.test.ts` meaningful. - **SSE messages are bound to their session.** `src/server/transport/http-sse.ts` keys `connections` by `transport.sessionId` and requires a matching `?sessionId=` on diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index 5b769b9e..b76c9836 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -45,8 +45,11 @@ export const config: AppConfig = { walletApiKey: undefined }; +let warnedAboutUnscopedWalletRead = false; + export function initializeConfig(environment: Record = process.env): AppConfig { Object.assign(config, loadConfig(environment)); + warnedAboutUnscopedWalletRead = false; return config; } @@ -88,7 +91,13 @@ export function getScopedAppConfig(): AppConfigSnapshot | undefined { * process config remains the fallback for direct configuration helpers. */ export function getRuntimeConfig(): Readonly { - return getScopedAppConfig() ?? config; + const scopedConfig = getScopedAppConfig(); + if (scopedConfig) return scopedConfig; + if (config.walletMode !== 'disabled' && !warnedAboutUnscopedWalletRead) { + warnedAboutUnscopedWalletRead = true; + console.error('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); + } + return config; } /** diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index 9a7d6294..15287a02 100755 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -64,7 +64,7 @@ export async function startMcpServer(): Promise { const errors = await collectOperationErrors([() => transport.stop(), () => server?.close()]); throwCollectedErrors(errors, 'Failed to stop all MCP server resources.'); } finally { - if (config.appConfig) resetWalletProvider(config.appConfig); + resetWalletProvider(config.appConfig); } })(); } diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index 9352ceec..9d371e89 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -9,16 +9,40 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; -function bindTransportToAppConfig(transport: Transport, appConfig: AppConfigSnapshot): void { - const start = transport.start.bind(transport); - let bound = false; - transport.start = async () => { - if (!bound && transport.onmessage) { - bound = true; - transport.onmessage = wrapWithAppConfig(appConfig, transport.onmessage); +const transportConfigs = new WeakMap(); + +function findPropertyDescriptor(target: object, property: PropertyKey): PropertyDescriptor | undefined { + let current: object | null = target; + while (current) { + const descriptor = Object.getOwnPropertyDescriptor(current, property); + if (descriptor) return descriptor; + current = Object.getPrototypeOf(current); + } + return undefined; +} + +export function bindTransportToAppConfig(transport: Transport, appConfig: AppConfigSnapshot): void { + const existingConfig = transportConfigs.get(transport); + if (existingConfig) { + if (existingConfig !== appConfig) throw new Error('MCP transport is already bound to a different AppConfig snapshot.'); + return; + } + + const initialHandler = transport.onmessage; + const descriptor = findPropertyDescriptor(transport, 'onmessage'); + let boundHandler: Transport['onmessage']; + Object.defineProperty(transport, 'onmessage', { + configurable: true, + enumerable: true, + get: () => (descriptor?.get ? descriptor.get.call(transport) : boundHandler), + set: (handler: Transport['onmessage']) => { + const wrappedHandler = handler ? wrapWithAppConfig(appConfig, handler) : undefined; + if (descriptor?.set) descriptor.set.call(transport, wrappedHandler); + else boundHandler = wrappedHandler; } - return start(); - }; + }); + transportConfigs.set(transport, appConfig); + if (initialHandler) transport.onmessage = initialHandler; } function bindServerToAppConfig(server: McpServer, appConfig: AppConfigSnapshot): McpServer { diff --git a/packages/mcp-server/src/server/transport/factory.ts b/packages/mcp-server/src/server/transport/factory.ts index 3a1d85af..fc4d156f 100644 --- a/packages/mcp-server/src/server/transport/factory.ts +++ b/packages/mcp-server/src/server/transport/factory.ts @@ -13,7 +13,6 @@ export const createTransport = (config: TransportConfig): McpTransport => { port: config.port, host: config.host, path: config.path, - walletMode: config.walletMode, maxActiveRequests: config.maxStreamableRequests, appConfig: config.appConfig }); @@ -23,7 +22,6 @@ export const createTransport = (config: TransportConfig): McpTransport => { port: config.port, host: config.host, path: config.path, - walletMode: config.walletMode, maxSessions: config.maxSseSessions, appConfig: config.appConfig }); diff --git a/packages/mcp-server/src/server/transport/http-sse.ts b/packages/mcp-server/src/server/transport/http-sse.ts index 8a54bbb2..f1d06f2e 100644 --- a/packages/mcp-server/src/server/transport/http-sse.ts +++ b/packages/mcp-server/src/server/transport/http-sse.ts @@ -20,8 +20,9 @@ export interface HttpSseTransportOptions { port: number; host: string; path: string; + appConfig: AppConfigSnapshot; + /** @deprecated Wallet mode is derived from appConfig. */ walletMode?: WalletMode; - appConfig?: AppConfigSnapshot; maxSessions?: number; } @@ -55,7 +56,6 @@ export class HttpSseTransport implements McpTransport { private readonly port: number; private readonly host: string; private readonly path: string; - private readonly walletMode: WalletMode; private readonly appConfig: AppConfigSnapshot; private readonly serverFactory: McpServerFactory; private readonly transportFactory: SseServerTransportFactory; @@ -66,8 +66,11 @@ export class HttpSseTransport implements McpTransport { this.port = options.port; this.host = options.host; this.path = options.path; - this.walletMode = options.walletMode ?? 'disabled'; + if (!options.appConfig) throw new Error('appConfig is required.'); this.appConfig = snapshotConfig(options.appConfig); + if (options.walletMode !== undefined && options.walletMode !== this.appConfig.walletMode) { + throw new Error('walletMode must match appConfig.walletMode.'); + } this.maxSessions = options.maxSessions ?? DEFAULT_MAX_SSE_SESSIONS; this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = dependencies.transportFactory ?? ((endpoint, response) => new SSEServerTransport(endpoint, response)); @@ -225,7 +228,7 @@ export class HttpSseTransport implements McpTransport { } async start(_server?: McpServer): Promise { - validateSecurityConfig(this.mode, this.walletMode, this.appConfig.walletMode); + validateSecurityConfig(this.mode, this.appConfig.walletMode); if (this.host.trim().length === 0) { throw new Error('SERVER_HOST must not be empty.'); } diff --git a/packages/mcp-server/src/server/transport/security.ts b/packages/mcp-server/src/server/transport/security.ts index 3c3bafcd..231024c0 100644 --- a/packages/mcp-server/src/server/transport/security.ts +++ b/packages/mcp-server/src/server/transport/security.ts @@ -19,9 +19,9 @@ export function createCorsMiddleware(): RequestHandler { * Validates that wallet mode is not used with HTTP transports * Exits the process if unsafe configuration detected */ -export function validateSecurityConfig(transportMode: TransportMode, walletMode: WalletMode, ...additionalWalletModes: WalletMode[]): void { +export function validateSecurityConfig(transportMode: TransportMode, walletMode: WalletMode): void { const isHttpTransport = transportMode === 'streamable-http' || transportMode === 'http-sse'; - const isWalletEnabled = [walletMode, ...additionalWalletModes].some((mode) => mode !== 'disabled'); + const isWalletEnabled = walletMode !== 'disabled'; if (isHttpTransport && isWalletEnabled) { console.error(''); diff --git a/packages/mcp-server/src/server/transport/stdio.ts b/packages/mcp-server/src/server/transport/stdio.ts index 660810fd..3a95f262 100644 --- a/packages/mcp-server/src/server/transport/stdio.ts +++ b/packages/mcp-server/src/server/transport/stdio.ts @@ -1,6 +1,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; -import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig } from '../../core/config.js'; +import { bindTransportToAppConfig } from '../server.js'; import type { McpTransport, TransportMode } from './types.js'; export class StdioTransport implements McpTransport { @@ -16,10 +17,8 @@ export class StdioTransport implements McpTransport { if (!server) throw new Error('STDIO transport requires an MCP server.'); const transport = new StdioServerTransport(); this.transport = transport; + bindTransportToAppConfig(transport, this.appConfig); await runWithAppConfig(this.appConfig, () => server.connect(transport)); - if (transport.onmessage) { - transport.onmessage = wrapWithAppConfig(this.appConfig, transport.onmessage); - } console.error('MCP Server ready (stdio transport)'); } diff --git a/packages/mcp-server/src/server/transport/streamable-http.ts b/packages/mcp-server/src/server/transport/streamable-http.ts index 5ba0e343..23f6c8b7 100644 --- a/packages/mcp-server/src/server/transport/streamable-http.ts +++ b/packages/mcp-server/src/server/transport/streamable-http.ts @@ -20,8 +20,9 @@ export interface StreamableHttpTransportOptions { port?: number; host?: string; path?: string; + appConfig: AppConfigSnapshot; + /** @deprecated Wallet mode is derived from appConfig. */ walletMode?: WalletMode; - appConfig?: AppConfigSnapshot; maxActiveRequests?: number; } @@ -55,19 +56,21 @@ export class StreamableHttpTransport implements McpTransport { private readonly port: number; private readonly host: string; private readonly path: string; - private readonly walletMode: WalletMode; private readonly appConfig: AppConfigSnapshot; private readonly serverFactory: StreamableServerFactory; private readonly transportFactory: StreamableTransportFactory; private readonly listenFactory: StreamableListenFactory; private readonly maxActiveRequests: number; - constructor(options: StreamableHttpTransportOptions = {}, dependencies: StreamableHttpTransportDependencies = {}) { + constructor(options: StreamableHttpTransportOptions, dependencies: StreamableHttpTransportDependencies = {}) { this.port = options.port ?? 8080; this.host = options.host ?? 'localhost'; this.path = options.path ?? '/mcp'; - this.walletMode = options.walletMode ?? 'disabled'; + if (!options.appConfig) throw new Error('appConfig is required.'); this.appConfig = snapshotConfig(options.appConfig); + if (options.walletMode !== undefined && options.walletMode !== this.appConfig.walletMode) { + throw new Error('walletMode must match appConfig.walletMode.'); + } this.maxActiveRequests = options.maxActiveRequests ?? DEFAULT_MAX_STREAMABLE_REQUESTS; this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = @@ -98,7 +101,7 @@ export class StreamableHttpTransport implements McpTransport { } async start(_server?: McpServer): Promise { - validateSecurityConfig(this.mode, this.walletMode, this.appConfig.walletMode); + validateSecurityConfig(this.mode, this.appConfig.walletMode); if (this.host.trim().length === 0) { throw new Error('SERVER_HOST must not be empty.'); } diff --git a/packages/mcp-server/src/server/transport/types.ts b/packages/mcp-server/src/server/transport/types.ts index f9bda376..12e2222d 100644 --- a/packages/mcp-server/src/server/transport/types.ts +++ b/packages/mcp-server/src/server/transport/types.ts @@ -12,7 +12,7 @@ export interface McpTransport { export interface TransportConfig { mode: TransportMode; walletMode: WalletMode; - appConfig?: AppConfigSnapshot; + appConfig: AppConfigSnapshot; port: number; // Required for HTTP-based transports host: string; // Required for HTTP-based transports path: string; // Required for HTTP-based transports diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index 041dd93a..1fbc9650 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, jest, test } from 'bun:test'; import { config, formatPrivateKey, @@ -123,6 +123,7 @@ describe('Config Module - Actual Implementation', () => { }); test('runWithAppConfig makes getters read the snapshot instead of the process singleton', () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); config.walletMode = 'private-key'; config.privateKey = '0xabcdef'; const snapshot = snapshotConfig({ @@ -142,9 +143,23 @@ describe('Config Module - Actual Implementation', () => { expect(isWalletEnabled()).toBe(true); expect(getPrivateKeyAsHex()).toBe('0xabcdef'); expect(getScopedAppConfig()).toBeUndefined(); + consoleErrorSpy.mockRestore(); + }); + + test('warns once when an enabled wallet is read outside a runtime scope', () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '5'.repeat(64) }); + + expect(getWalletMode()).toBe('private-key'); + expect(isWalletEnabled()).toBe(true); + + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); + consoleErrorSpy.mockRestore(); }); test('wrapWithAppConfig keeps later callbacks on the snapshot', () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); config.walletMode = 'private-key'; const snapshot = snapshotConfig({ privateKey: undefined, @@ -158,6 +173,7 @@ describe('Config Module - Actual Implementation', () => { initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '4'.repeat(64) }); expect(getWalletMode()).toBe('private-key'); expect(readMode()).toBe('disabled'); + consoleErrorSpy.mockRestore(); }); }); diff --git a/packages/mcp-server/src/tests/index.test.ts b/packages/mcp-server/src/tests/index.test.ts index ae8aa089..8e56003d 100644 --- a/packages/mcp-server/src/tests/index.test.ts +++ b/packages/mcp-server/src/tests/index.test.ts @@ -142,7 +142,8 @@ describe('index', () => { }); it('does not construct an unused bootstrap server for HTTP transports', async () => { - mockParseArgs.mockReturnValue({ mode: 'http-sse' }); + const appConfig = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); + mockParseArgs.mockReturnValue({ mode: 'http-sse', walletMode: 'disabled', appConfig }); const indexModule = await import('../index.js'); const runtime = await indexModule.main(); diff --git a/packages/mcp-server/src/tests/server/server.test.ts b/packages/mcp-server/src/tests/server/server.test.ts index 9585d8f8..27064b9e 100644 --- a/packages/mcp-server/src/tests/server/server.test.ts +++ b/packages/mcp-server/src/tests/server/server.test.ts @@ -134,6 +134,21 @@ describe('Server Module', () => { expect(mockConnect).toHaveBeenCalledWith(transport, options); }); + it('binds an onmessage handler assigned after connect', async () => { + const { getScopedAppConfig } = await import('../../core/config.js'); + const server = await getServer(); + const transport = { start: jest.fn(), onmessage: undefined as ((message: unknown) => void) | undefined }; + let sawRuntimeScope = false; + + await server.connect(transport as never); + transport.onmessage = () => { + sawRuntimeScope = getScopedAppConfig() !== undefined; + }; + transport.onmessage({}); + + expect(sawRuntimeScope).toBe(true); + }); + it('should sanitize and propagate server initialization errors', async () => { const testError = new Error('Initialization failed'); mockGetPackageInfo.mockImplementation(() => { diff --git a/packages/mcp-server/src/tests/server/transport/factory.test.ts b/packages/mcp-server/src/tests/server/transport/factory.test.ts index 75b65ee0..73f01625 100644 --- a/packages/mcp-server/src/tests/server/transport/factory.test.ts +++ b/packages/mcp-server/src/tests/server/transport/factory.test.ts @@ -14,6 +14,9 @@ jest.mock('../../../server/transport/http-sse.js', () => ({ HttpSseTransport: jest.fn() })); +const DISABLED_APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); +const PRIVATE_KEY_APP_CONFIG = Object.freeze({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined }); + describe('Transport Factory', () => { let createTransport: any; let StdioTransport: jest.MockedClass; @@ -45,6 +48,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'stdio', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' @@ -55,7 +59,7 @@ describe('Transport Factory', () => { const transport = createTransport(config); - expect(StdioTransport).toHaveBeenCalledWith(undefined); + expect(StdioTransport).toHaveBeenCalledWith(DISABLED_APP_CONFIG); expect(transport).toBe(mockStdioInstance); }); @@ -63,6 +67,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'streamable-http', walletMode: 'private-key', + appConfig: PRIVATE_KEY_APP_CONFIG, port: 8080, host: '0.0.0.0', path: '/api/mcp' @@ -77,9 +82,8 @@ describe('Transport Factory', () => { port: 8080, host: '0.0.0.0', path: '/api/mcp', - walletMode: 'private-key', maxActiveRequests: undefined, - appConfig: undefined + appConfig: PRIVATE_KEY_APP_CONFIG }); expect(transport).toBe(mockStreamableInstance); }); @@ -88,6 +92,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'http-sse', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 9000, host: '127.0.0.1', path: '/sse', @@ -103,9 +108,8 @@ describe('Transport Factory', () => { port: 9000, host: '127.0.0.1', path: '/sse', - walletMode: 'disabled', maxSessions: 25, - appConfig: undefined + appConfig: DISABLED_APP_CONFIG }); expect(transport).toBe(mockSseInstance); }); @@ -153,7 +157,6 @@ describe('Transport Factory', () => { port: 8080, host: 'localhost', path: '/mcp', - walletMode: 'disabled', maxActiveRequests: undefined, appConfig }); @@ -182,6 +185,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'streamable-http', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, ...params }; @@ -192,9 +196,8 @@ describe('Transport Factory', () => { expect(StreamableHttpTransport).toHaveBeenCalledWith({ ...params, - walletMode: 'disabled', maxActiveRequests: undefined, - appConfig: undefined + appConfig: DISABLED_APP_CONFIG }); expect(transport).toBe(mockInstance); @@ -207,6 +210,7 @@ describe('Transport Factory', () => { const config1: TransportConfig = { mode: 'http-sse', walletMode: 'private-key', + appConfig: PRIVATE_KEY_APP_CONFIG, port: 1, host: '::1', // IPv6 localhost path: '/' @@ -221,9 +225,8 @@ describe('Transport Factory', () => { port: 1, host: '::1', path: '/', - walletMode: 'private-key', maxSessions: undefined, - appConfig: undefined + appConfig: PRIVATE_KEY_APP_CONFIG }); expect(transport1).toBe(mockInstance1); @@ -233,6 +236,7 @@ describe('Transport Factory', () => { const config2: TransportConfig = { mode: 'streamable-http', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 65535, host: '0.0.0.0', path: '/very/long/path/to/test/edge/cases' @@ -247,9 +251,8 @@ describe('Transport Factory', () => { port: 65535, host: '0.0.0.0', path: '/very/long/path/to/test/edge/cases', - walletMode: 'disabled', maxActiveRequests: undefined, - appConfig: undefined + appConfig: DISABLED_APP_CONFIG }); expect(transport2).toBe(mockInstance2); }); @@ -261,6 +264,7 @@ describe('Transport Factory', () => { const validConfig: TransportConfig = { mode: 'stdio', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' @@ -273,6 +277,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'stdio', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' @@ -299,6 +304,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'stdio', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' @@ -319,6 +325,7 @@ describe('Transport Factory', () => { it('should handle config with missing mode', () => { const invalidConfig = { walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' @@ -331,9 +338,9 @@ describe('Transport Factory', () => { describe('integration scenarios', () => { it('should create different transport types in sequence', () => { const configs: TransportConfig[] = [ - { mode: 'stdio', walletMode: 'disabled', port: 3000, host: 'localhost', path: '/mcp' }, - { mode: 'streamable-http', walletMode: 'private-key', port: 8080, host: '0.0.0.0', path: '/api' }, - { mode: 'http-sse', walletMode: 'disabled', port: 9000, host: '127.0.0.1', path: '/sse' } + { mode: 'stdio', walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' }, + { mode: 'streamable-http', walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG, port: 8080, host: '0.0.0.0', path: '/api' }, + { mode: 'http-sse', walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 9000, host: '127.0.0.1', path: '/sse' } ]; const mockInstances = [{ mode: 'stdio' }, { mode: 'streamable-http' }, { mode: 'http-sse' }]; @@ -358,6 +365,7 @@ describe('Transport Factory', () => { const config: TransportConfig = { mode: 'streamable-http', walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' diff --git a/packages/mcp-server/src/tests/server/transport/http-sse.test.ts b/packages/mcp-server/src/tests/server/transport/http-sse.test.ts index cb353317..e9444630 100644 --- a/packages/mcp-server/src/tests/server/transport/http-sse.test.ts +++ b/packages/mcp-server/src/tests/server/transport/http-sse.test.ts @@ -10,6 +10,8 @@ import { HttpSseTransport } from '../../../server/transport/http-sse.js'; const HOST = '127.0.0.1'; const PATH = '/mcp'; +const DISABLED_APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); +const PRIVATE_KEY_APP_CONFIG = Object.freeze({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined }); function textOf(result: Awaited>): string { if (!('content' in result)) throw new Error('Expected an immediate tool result'); @@ -60,7 +62,10 @@ describe('HttpSseTransport', () => { throw new Error(`process.exit called with code ${code}`); }); const listenFactory = jest.fn(); - const transport = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key' }, { listenFactory }); + const transport = new HttpSseTransport( + { port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG }, + { listenFactory } + ); await expect(transport.start()).rejects.toThrow('process.exit called with code 1'); expect(processExit).toHaveBeenCalledWith(1); @@ -75,7 +80,7 @@ describe('HttpSseTransport', () => { let nextSession = 0; const transport = new HttpSseTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => { const sessionIndex = nextSession++; @@ -149,7 +154,7 @@ describe('HttpSseTransport', () => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); let serverCount = 0; const transport = new HttpSseTransport( - { port: 0, host: HOST, path: PATH, maxSessions: 1 }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG, maxSessions: 1 }, { serverFactory: async () => { serverCount++; @@ -189,7 +194,7 @@ describe('HttpSseTransport', () => { let serverCloseCount = 0; let transportCloseCount = 0; const transport = new HttpSseTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => { const server = new McpServer({ name: 'disconnect-session', version: '1.0.0' }); @@ -246,7 +251,7 @@ describe('HttpSseTransport', () => { } as unknown as McpServer; const sessionTransportClose = jest.fn().mockRejectedValue(new Error('simulated transport cleanup failure')); const transport = new HttpSseTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => sessionServer, transportFactory: (endpoint, sessionResponse) => { @@ -291,7 +296,7 @@ describe('HttpSseTransport', () => { }) as typeof delayedServer.close; let listenCount = 0; const transport = new HttpSseTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => new McpServer({ name: 'session', version: '1.0.0' }), transportFactory: (endpoint, sessionResponse) => new SSEServerTransport(endpoint, sessionResponse), @@ -323,13 +328,13 @@ describe('HttpSseTransport', () => { const address = occupied.address(); if (!address || typeof address === 'string') throw new Error('Expected an occupied TCP port'); - const blocked = new HttpSseTransport({ port: address.port, host: HOST, path: PATH }); + const blocked = new HttpSseTransport({ port: address.port, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }); const bootstrap = new McpServer({ name: 'bootstrap', version: '1.0.0' }); bootstrapServers.push(bootstrap); await expect(blocked.start(bootstrap)).rejects.toMatchObject({ code: 'EADDRINUSE' }); await new Promise((resolve, reject) => occupied.close((error) => (error ? reject(error) : resolve()))); - const transport = new HttpSseTransport({ port: 0, host: HOST, path: PATH }); + const transport = new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }); transports.push(transport); await transport.start(bootstrap); const health = await fetch(`http://${HOST}:${transport.getListeningPort()}/health`); @@ -342,7 +347,7 @@ describe('HttpSseTransport', () => { const transportFactory = jest.fn(() => { throw new Error('simulated SSE constructor failure'); }); - const transport = new HttpSseTransport({ port: 0, host: HOST, path: PATH, maxSessions: 1 }, { transportFactory }); + const transport = new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG, maxSessions: 1 }, { transportFactory }); transports.push(transport); const bootstrap = new McpServer({ name: 'bootstrap', version: '1.0.0' }); bootstrapServers.push(bootstrap); @@ -363,7 +368,7 @@ describe('HttpSseTransport', () => { } as unknown as McpServer; const sessionTransportClose = jest.fn().mockRejectedValue(new Error('transport close failed')); const transport = new HttpSseTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => sessionServer, transportFactory: (endpoint, response) => { @@ -402,14 +407,14 @@ describe('HttpSseTransport', () => { it('rejects messages without a valid session and validates a nonempty host', async () => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - const invalidHost = new HttpSseTransport({ port: 8080, host: ' ', path: PATH }); + const invalidHost = new HttpSseTransport({ port: 8080, host: ' ', path: PATH, appConfig: DISABLED_APP_CONFIG }); const unused = new McpServer({ name: 'unused', version: '1.0.0' }); bootstrapServers.push(unused); await expect(invalidHost.start(unused)).rejects.toThrow('SERVER_HOST must not be empty'); - const invalidLimit = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, maxSessions: 0 }); + const invalidLimit = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG, maxSessions: 0 }); await expect(invalidLimit.start(unused)).rejects.toThrow('SSE_MAX_SESSIONS must be a positive integer'); - const transport = new HttpSseTransport({ port: 0, host: HOST, path: PATH }); + const transport = new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }); transports.push(transport); const bootstrap = new McpServer({ name: 'bootstrap', version: '1.0.0' }); bootstrapServers.push(bootstrap); diff --git a/packages/mcp-server/src/tests/server/transport/security.test.ts b/packages/mcp-server/src/tests/server/transport/security.test.ts index 7b14d501..da2817ef 100644 --- a/packages/mcp-server/src/tests/server/transport/security.test.ts +++ b/packages/mcp-server/src/tests/server/transport/security.test.ts @@ -124,12 +124,6 @@ describe('Security Module', () => { expect(processExitSpy).toHaveBeenCalledWith(1); }); - it('should reject HTTP when any configuration source enables the wallet', () => { - expect(() => validateSecurityConfig('streamable-http', 'disabled', 'private-key')).toThrow('process.exit called with code 1'); - expect(() => validateSecurityConfig('http-sse', 'disabled', 'private-key')).toThrow('process.exit called with code 1'); - expect(processExitSpy).toHaveBeenCalledTimes(2); - }); - it('should terminate directly with a safe diagnostic for unsafe config', () => { expect(() => validateSecurityConfig('streamable-http', 'private-key')).toThrow('process.exit called with code 1'); expect(consoleErrorSpy).toHaveBeenCalledWith('â•‘ Wallet mode cannot be used with HTTP transports! â•‘'); diff --git a/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts b/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts index 06e8d293..5901ac8f 100644 --- a/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts +++ b/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts @@ -9,6 +9,8 @@ import { StreamableHttpTransport } from '../../../server/transport/streamable-ht const HOST = '127.0.0.1'; const PATH = '/mcp'; +const DISABLED_APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); +const PRIVATE_KEY_APP_CONFIG = Object.freeze({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined }); async function listenOnRandomPort(server: Server): Promise { await new Promise((resolve, reject) => { @@ -60,7 +62,10 @@ describe('StreamableHttpTransport', () => { throw new Error(`process.exit called with code ${code}`); }); const listenFactory = jest.fn(); - const transport = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key' }, { listenFactory }); + const transport = new StreamableHttpTransport( + { port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG }, + { listenFactory } + ); await expect(transport.start()).rejects.toThrow('process.exit called with code 1'); expect(processExit).toHaveBeenCalledWith(1); @@ -71,7 +76,7 @@ describe('StreamableHttpTransport', () => { it('resolves start only after listening and rejects an empty host', async () => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); const bootstrap = { close: jest.fn() } as unknown as McpServer; - const transport = new StreamableHttpTransport({ port: 0, host: HOST, path: PATH }); + const transport = new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }); transports.push(transport); await transport.start(bootstrap); @@ -81,16 +86,16 @@ describe('StreamableHttpTransport', () => { expect(health.status).toBe(200); expect(await health.json()).toMatchObject({ status: 'ok' }); - const invalidHost = new StreamableHttpTransport({ port: 8080, host: ' ', path: PATH }); + const invalidHost = new StreamableHttpTransport({ port: 8080, host: ' ', path: PATH, appConfig: DISABLED_APP_CONFIG }); await expect(invalidHost.start(bootstrap)).rejects.toThrow('SERVER_HOST must not be empty'); - const invalidLimit = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, maxActiveRequests: 0 }); + const invalidLimit = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG, maxActiveRequests: 0 }); await expect(invalidLimit.start(bootstrap)).rejects.toThrow('STREAMABLE_HTTP_MAX_REQUESTS must be a positive integer'); }); it('serves a real local MCP client request', async () => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); const transport = new StreamableHttpTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => { const server = new McpServer({ name: 'request-server', version: '1.0.0' }); @@ -141,7 +146,10 @@ describe('StreamableHttpTransport', () => { }) }) as unknown as StreamableHTTPServerTransport ); - const transport = new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, maxActiveRequests: 1 }, { serverFactory, transportFactory }); + const transport = new StreamableHttpTransport( + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG, maxActiveRequests: 1 }, + { serverFactory, transportFactory } + ); transports.push(transport); await transport.start(); const url = `http://${HOST}:${transport.getListeningPort()}${PATH}`; @@ -171,7 +179,7 @@ describe('StreamableHttpTransport', () => { const occupied = createServer(); occupiedServers.push(occupied); const port = await listenOnRandomPort(occupied); - const transport = new StreamableHttpTransport({ port, host: HOST, path: PATH }); + const transport = new StreamableHttpTransport({ port, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }); transports.push(transport); try { @@ -190,7 +198,10 @@ describe('StreamableHttpTransport', () => { const transportFactory = jest.fn(() => { throw new Error('simulated request transport constructor failure'); }); - const transport = new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, maxActiveRequests: 1 }, { serverFactory, transportFactory }); + const transport = new StreamableHttpTransport( + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG, maxActiveRequests: 1 }, + { serverFactory, transportFactory } + ); transports.push(transport); await transport.start(); const url = `http://${HOST}:${transport.getListeningPort()}${PATH}`; @@ -231,7 +242,7 @@ describe('StreamableHttpTransport', () => { } as unknown as StreamableHTTPServerTransport; const transport = new StreamableHttpTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => requestServer, transportFactory: () => requestTransport @@ -280,7 +291,7 @@ describe('StreamableHttpTransport', () => { }) } as unknown as StreamableHTTPServerTransport; const transport = new StreamableHttpTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => requestServer, transportFactory: () => requestTransport @@ -323,7 +334,7 @@ describe('StreamableHttpTransport', () => { }) as typeof delayedServer.close; let listenCount = 0; const transport = new StreamableHttpTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => new McpServer({ name: 'request', version: '1.0.0' }), transportFactory: () => ({ close: jest.fn() }) as unknown as StreamableHTTPServerTransport, @@ -363,7 +374,7 @@ describe('StreamableHttpTransport', () => { }) } as unknown as StreamableHTTPServerTransport; const transport = new StreamableHttpTransport( - { port: 0, host: HOST, path: PATH }, + { port: 0, host: HOST, path: PATH, appConfig: DISABLED_APP_CONFIG }, { serverFactory: async () => requestServer, transportFactory: () => requestTransport diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index 4fb588bf..7a497bd6 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -94,19 +94,19 @@ describe('HTTP wallet isolation across later starts', () => { expect(await listWalletTools(mode, url)).toEqual([]); }); - it.each(['http-sse', 'streamable-http'] as const)('snapshots %s at construction when appConfig is omitted', async (mode) => { + it.each(['http-sse', 'streamable-http'] as const)('snapshots a mutable %s appConfig at construction', async (mode) => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - Object.assign(config, { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); + const appConfig = { privateKey: undefined as string | undefined, walletMode: 'disabled' as 'disabled' | 'private-key', walletApiKey: undefined }; const transport = mode === 'http-sse' - ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled' }) - : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled' }); + ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig }) + : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, appConfig }); transports.push(transport); await transport.start(); - initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: PRIVATE_KEY }); + appConfig.walletMode = 'private-key'; + appConfig.privateKey = `0x${PRIVATE_KEY}`; const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); - expect(isWalletEnabled()).toBe(true); expect(await listWalletTools(mode, url)).toEqual([]); }); @@ -116,8 +116,9 @@ describe('HTTP wallet isolation across later starts', () => { throw new Error(`process.exit called with code ${code}`); }); const listenFactory = jest.fn(); - const streamable = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key' }, { listenFactory }); - const sse = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key' }, { listenFactory }); + const appConfig = snapshotConfig({ privateKey: `0x${PRIVATE_KEY}`, walletMode: 'private-key', walletApiKey: undefined }); + const streamable = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig }, { listenFactory }); + const sse = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig }, { listenFactory }); await expect(streamable.start()).rejects.toThrow('process.exit called with code 1'); await expect(sse.start()).rejects.toThrow('process.exit called with code 1'); @@ -126,12 +127,7 @@ describe('HTTP wallet isolation across later starts', () => { processExit.mockRestore(); }); - it.each([ - ['http-sse', undefined], - ['http-sse', 'disabled'], - ['streamable-http', undefined], - ['streamable-http', 'disabled'] - ] as const)('rejects %s when appConfig enables the wallet and walletMode is %s', async (mode, walletMode) => { + it.each(['http-sse', 'streamable-http'] as const)('rejects %s when appConfig enables the wallet and walletMode is omitted', async (mode) => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); const processExit = jest.spyOn(process, 'exit').mockImplementation((code) => { throw new Error(`process.exit called with code ${code}`); @@ -142,7 +138,7 @@ describe('HTTP wallet isolation across later starts', () => { walletMode: 'private-key', walletApiKey: undefined }); - const options = { port: 8080, host: HOST, path: PATH, walletMode, appConfig }; + const options = { port: 8080, host: HOST, path: PATH, appConfig }; const transport = mode === 'http-sse' ? new HttpSseTransport(options, { listenFactory }) : new StreamableHttpTransport(options, { listenFactory }); await expect(transport.start()).rejects.toThrow('process.exit called with code 1'); @@ -150,6 +146,27 @@ describe('HTTP wallet isolation across later starts', () => { expect(listenFactory).not.toHaveBeenCalled(); processExit.mockRestore(); }); + + it.each(['http-sse', 'streamable-http'] as const)('rejects conflicting compatibility walletMode for %s at construction', (mode) => { + const appConfig = snapshotConfig({ + privateKey: `0x${PRIVATE_KEY}`, + walletMode: 'private-key', + walletApiKey: undefined + }); + const options = { port: 8080, host: HOST, path: PATH, walletMode: 'disabled' as const, appConfig }; + + expect(() => (mode === 'http-sse' ? new HttpSseTransport(options) : new StreamableHttpTransport(options))).toThrow( + 'walletMode must match appConfig.walletMode.' + ); + }); + + it.each(['http-sse', 'streamable-http'] as const)('requires an explicit appConfig for %s', (mode) => { + const options = { port: 8080, host: HOST, path: PATH }; + + expect(() => (mode === 'http-sse' ? new HttpSseTransport(options as never) : new StreamableHttpTransport(options as never))).toThrow( + 'appConfig is required.' + ); + }); }); describe('stdio wallet isolation after another runtime stop', () => { From 86b098af642a5cebe456729717e5d609f421a1ca Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 10:12:33 +0200 Subject: [PATCH 06/10] fix(mcp-server): bind wallet context at tool handlers Co-authored-by: Cursor --- packages/mcp-server/src/core/config.ts | 6 +- packages/mcp-server/src/core/tools.ts | 8 ++- packages/mcp-server/src/index.ts | 2 +- packages/mcp-server/src/server/args.ts | 1 - packages/mcp-server/src/server/server.ts | 53 +--------------- .../mcp-server/src/server/transport/stdio.ts | 5 +- .../mcp-server/src/server/transport/types.ts | 1 - .../mcp-server/src/tests/core/config.test.ts | 4 +- packages/mcp-server/src/tests/index.test.ts | 6 +- .../mcp-server/src/tests/server/args.test.ts | 6 +- .../src/tests/server/server.test.ts | 39 +++--------- .../tests/server/transport/factory.test.ts | 21 ++----- .../src/tests/server/transport/stdio.test.ts | 63 ++++++------------- .../server/transport/wallet-isolation.test.ts | 2 + 14 files changed, 50 insertions(+), 167 deletions(-) diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index b76c9836..ae80c7d3 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -45,11 +45,8 @@ export const config: AppConfig = { walletApiKey: undefined }; -let warnedAboutUnscopedWalletRead = false; - export function initializeConfig(environment: Record = process.env): AppConfig { Object.assign(config, loadConfig(environment)); - warnedAboutUnscopedWalletRead = false; return config; } @@ -93,8 +90,7 @@ export function getScopedAppConfig(): AppConfigSnapshot | undefined { export function getRuntimeConfig(): Readonly { const scopedConfig = getScopedAppConfig(); if (scopedConfig) return scopedConfig; - if (config.walletMode !== 'disabled' && !warnedAboutUnscopedWalletRead) { - warnedAboutUnscopedWalletRead = true; + if (config.walletMode !== 'disabled') { console.error('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); } return config; diff --git a/packages/mcp-server/src/core/tools.ts b/packages/mcp-server/src/core/tools.ts index f58a83c0..98ac0e1a 100644 --- a/packages/mcp-server/src/core/tools.ts +++ b/packages/mcp-server/src/core/tools.ts @@ -2,7 +2,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { Address, Hash, Hex } from 'viem'; import { z } from 'zod'; import { DEFAULT_NETWORK, getSupportedNetworks, networkSchema } from './chains.js'; -import { isWalletEnabled } from './config.js'; +import { getScopedAppConfig, isWalletEnabled, wrapWithAppConfig } from './config.js'; import { sanitizeError } from './errors.js'; import * as services from './services/index.js'; import { getWalletProvider } from './wallet/index.js'; @@ -30,6 +30,9 @@ const READ_ONLY_TOOL_NAMES = new Set([ ]); export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boolean): McpServer { + // Tool callbacks run after server construction, potentially after another + // runtime updates process config. Capture this server's validated snapshot. + const appConfig = getScopedAppConfig(); const registerTool = server.tool.bind(server) as unknown as ( name: string, description: string, @@ -45,7 +48,8 @@ export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boo console.error(`Tool registration suppressed by wallet policy: ${name}`); return undefined; } - return registerTool(name, description, 'network' in schema ? { ...schema, network: networkSchema.optional() } : schema, handler); + const runtimeHandler = appConfig ? wrapWithAppConfig(appConfig, handler) : handler; + return registerTool(name, description, 'network' in schema ? { ...schema, network: networkSchema.optional() } : schema, runtimeHandler); }; } diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index 15287a02..5d2f69ce 100755 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -52,7 +52,7 @@ export async function startMcpServer(): Promise { throw error; } - if (config.walletMode === 'disabled') console.error('Wallet functionality is disabled. Signing and broadcasting tools are not available.'); + if (config.appConfig.walletMode === 'disabled') console.error('Wallet functionality is disabled. Signing and broadcasting tools are not available.'); let stopPromise: Promise | undefined; let removeShutdownHandlers = () => {}; diff --git a/packages/mcp-server/src/server/args.ts b/packages/mcp-server/src/server/args.ts index 3b8d7be3..699f4f05 100644 --- a/packages/mcp-server/src/server/args.ts +++ b/packages/mcp-server/src/server/args.ts @@ -154,7 +154,6 @@ Security Note: port: Number.isNaN(config.server.port) ? DEFAULT_CONFIG.server.port : config.server.port, host: config.server.host, path: config.server.path, - walletMode: appConfig.walletMode, appConfig, maxSseSessions: Number.isInteger(config.server.sseMaxSessions) && config.server.sseMaxSessions > 0 ? config.server.sseMaxSessions : DEFAULT_CONFIG.server.sseMaxSessions, diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index 9d371e89..f78f3968 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -1,7 +1,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; -import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; import { getSupportedNetworks } from '../core/chains.js'; -import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig } from '../core/config.js'; import { sanitizeError } from '../core/errors.js'; import { registerEVMPrompts } from '../core/prompts.js'; import { registerEVMResources } from '../core/resources.js'; @@ -9,53 +8,7 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; -const transportConfigs = new WeakMap(); - -function findPropertyDescriptor(target: object, property: PropertyKey): PropertyDescriptor | undefined { - let current: object | null = target; - while (current) { - const descriptor = Object.getOwnPropertyDescriptor(current, property); - if (descriptor) return descriptor; - current = Object.getPrototypeOf(current); - } - return undefined; -} - -export function bindTransportToAppConfig(transport: Transport, appConfig: AppConfigSnapshot): void { - const existingConfig = transportConfigs.get(transport); - if (existingConfig) { - if (existingConfig !== appConfig) throw new Error('MCP transport is already bound to a different AppConfig snapshot.'); - return; - } - - const initialHandler = transport.onmessage; - const descriptor = findPropertyDescriptor(transport, 'onmessage'); - let boundHandler: Transport['onmessage']; - Object.defineProperty(transport, 'onmessage', { - configurable: true, - enumerable: true, - get: () => (descriptor?.get ? descriptor.get.call(transport) : boundHandler), - set: (handler: Transport['onmessage']) => { - const wrappedHandler = handler ? wrapWithAppConfig(appConfig, handler) : undefined; - if (descriptor?.set) descriptor.set.call(transport, wrappedHandler); - else boundHandler = wrappedHandler; - } - }); - transportConfigs.set(transport, appConfig); - if (initialHandler) transport.onmessage = initialHandler; -} - -function bindServerToAppConfig(server: McpServer, appConfig: AppConfigSnapshot): McpServer { - const connect = server.connect.bind(server); - server.connect = (async (...args: Parameters) => { - const [transport] = args; - bindTransportToAppConfig(transport, appConfig); - return connect(...args); - }) as typeof server.connect; - return server; -} - -export const getServer = async (appConfig: AppConfigSnapshot = snapshotConfig()) => { +export const getServer = async (appConfig: AppConfigSnapshot) => { const config = snapshotConfig(appConfig); return runWithAppConfig(config, async () => { try { @@ -72,7 +25,7 @@ export const getServer = async (appConfig: AppConfigSnapshot = snapshotConfig()) console.error('Supported networks:', getSupportedNetworks().join(', ')); - return bindServerToAppConfig(server, config); + return server; } catch (error) { console.error('Failed to initialize server:', sanitizeError(error)); throw error; diff --git a/packages/mcp-server/src/server/transport/stdio.ts b/packages/mcp-server/src/server/transport/stdio.ts index 3a95f262..ca495e20 100644 --- a/packages/mcp-server/src/server/transport/stdio.ts +++ b/packages/mcp-server/src/server/transport/stdio.ts @@ -1,7 +1,6 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig } from '../../core/config.js'; -import { bindTransportToAppConfig } from '../server.js'; import type { McpTransport, TransportMode } from './types.js'; export class StdioTransport implements McpTransport { @@ -9,7 +8,8 @@ export class StdioTransport implements McpTransport { private readonly appConfig: AppConfigSnapshot; private transport?: StdioServerTransport; - constructor(appConfig?: AppConfigSnapshot) { + constructor(appConfig: AppConfigSnapshot) { + if (!appConfig) throw new Error('appConfig is required.'); this.appConfig = snapshotConfig(appConfig); } @@ -17,7 +17,6 @@ export class StdioTransport implements McpTransport { if (!server) throw new Error('STDIO transport requires an MCP server.'); const transport = new StdioServerTransport(); this.transport = transport; - bindTransportToAppConfig(transport, this.appConfig); await runWithAppConfig(this.appConfig, () => server.connect(transport)); console.error('MCP Server ready (stdio transport)'); } diff --git a/packages/mcp-server/src/server/transport/types.ts b/packages/mcp-server/src/server/transport/types.ts index 12e2222d..acc9b27d 100644 --- a/packages/mcp-server/src/server/transport/types.ts +++ b/packages/mcp-server/src/server/transport/types.ts @@ -11,7 +11,6 @@ export interface McpTransport { export interface TransportConfig { mode: TransportMode; - walletMode: WalletMode; appConfig: AppConfigSnapshot; port: number; // Required for HTTP-based transports host: string; // Required for HTTP-based transports diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index 1fbc9650..edf294f1 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -146,14 +146,14 @@ describe('Config Module - Actual Implementation', () => { consoleErrorSpy.mockRestore(); }); - test('warns once when an enabled wallet is read outside a runtime scope', () => { + test('warns on every enabled-wallet read outside a runtime scope', () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '5'.repeat(64) }); expect(getWalletMode()).toBe('private-key'); expect(isWalletEnabled()).toBe(true); - expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledTimes(2); expect(consoleErrorSpy).toHaveBeenCalledWith('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); consoleErrorSpy.mockRestore(); }); diff --git a/packages/mcp-server/src/tests/index.test.ts b/packages/mcp-server/src/tests/index.test.ts index 8e56003d..238309a5 100644 --- a/packages/mcp-server/src/tests/index.test.ts +++ b/packages/mcp-server/src/tests/index.test.ts @@ -52,7 +52,7 @@ describe('index', () => { // Setup default mock implementations mockAppConfig = Object.freeze({ privateKey: '0xabc', walletMode: 'private-key', walletApiKey: undefined }); - mockParseArgs.mockReturnValue({ mode: 'stdio', walletMode: 'private-key', appConfig: mockAppConfig }); + mockParseArgs.mockReturnValue({ mode: 'stdio', appConfig: mockAppConfig }); mockGetServer.mockResolvedValue(mockServer); mockCreateTransport.mockReturnValue(mockTransport); @@ -89,7 +89,7 @@ describe('index', () => { it('should log warning when wallet is disabled', async () => { const appConfig = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); - mockParseArgs.mockReturnValue({ mode: 'stdio', walletMode: 'disabled', appConfig }); + mockParseArgs.mockReturnValue({ mode: 'stdio', appConfig }); const indexModule = await import('../index.js'); const runtime = await indexModule.main(); @@ -143,7 +143,7 @@ describe('index', () => { it('does not construct an unused bootstrap server for HTTP transports', async () => { const appConfig = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); - mockParseArgs.mockReturnValue({ mode: 'http-sse', walletMode: 'disabled', appConfig }); + mockParseArgs.mockReturnValue({ mode: 'http-sse', appConfig }); const indexModule = await import('../index.js'); const runtime = await indexModule.main(); diff --git a/packages/mcp-server/src/tests/server/args.test.ts b/packages/mcp-server/src/tests/server/args.test.ts index 8bdcb5ba..a3cf4f22 100644 --- a/packages/mcp-server/src/tests/server/args.test.ts +++ b/packages/mcp-server/src/tests/server/args.test.ts @@ -112,7 +112,6 @@ describe('Args Module', () => { port: 8080, host: 'localhost', path: '/mcp', - walletMode: 'disabled', appConfig: { privateKey: undefined, walletMode: 'disabled', @@ -139,7 +138,6 @@ describe('Args Module', () => { port: 3001, host: '0.0.0.0', path: '/api/mcp', - walletMode: 'private-key', appConfig: { privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key', @@ -368,7 +366,6 @@ describe('Args Module', () => { expect(result).toHaveProperty('port'); expect(result).toHaveProperty('host'); expect(result).toHaveProperty('path'); - expect(result).toHaveProperty('walletMode'); expect(result).toHaveProperty('appConfig'); // Verify types @@ -376,7 +373,7 @@ describe('Args Module', () => { expect(typeof result.port).toBe('number'); expect(typeof result.host).toBe('string'); expect(typeof result.path).toBe('string'); - expect(typeof result.walletMode).toBe('string'); + expect(typeof result.appConfig.walletMode).toBe('string'); }); it('should handle getPackageInfo errors gracefully', () => { @@ -410,7 +407,6 @@ describe('Args Module', () => { port: 9000, host: 'localhost', path: '/mcp', - walletMode: 'private-key', appConfig: { privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key', diff --git a/packages/mcp-server/src/tests/server/server.test.ts b/packages/mcp-server/src/tests/server/server.test.ts index 27064b9e..0a87c6b3 100644 --- a/packages/mcp-server/src/tests/server/server.test.ts +++ b/packages/mcp-server/src/tests/server/server.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, jest, test } from 'bun:test'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { AppConfigSnapshot } from '../../core/config.js'; // Mock all dependencies jest.mock('@modelcontextprotocol/sdk/server/mcp.js', () => ({ @@ -31,7 +32,8 @@ jest.mock('../../core/chains.js', () => ({ rpcUrlMap: {} })); -type GetServerFunction = () => Promise; +type GetServerFunction = (appConfig: AppConfigSnapshot) => Promise; +const APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); describe('Server Module', () => { let getServer: GetServerFunction; @@ -45,17 +47,15 @@ describe('Server Module', () => { let consoleErrorSpy: jest.SpiedFunction; let processExitSpy: jest.SpiedFunction; let mockServerInstance: any; - let mockConnect: jest.Mock; beforeEach(async () => { jest.clearAllMocks(); // Create mock server instance - mockConnect = jest.fn().mockResolvedValue(undefined); mockServerInstance = { name: '@sei-js/mcp-server', version: '1.0.0', - connect: mockConnect + connect: jest.fn().mockResolvedValue(undefined) }; // Import mocked functions first @@ -103,7 +103,7 @@ describe('Server Module', () => { describe('getServer', () => { it('should call all initialization functions', async () => { - await getServer(); + await getServer(APP_CONFIG); expect(mockGetPackageInfo).toHaveBeenCalled(); expect(mockRegisterEVMResources).toHaveBeenCalled(); @@ -119,43 +119,18 @@ describe('Server Module', () => { it('should log supported networks', async () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); - await getServer(); + await getServer(APP_CONFIG); expect(consoleErrorSpy).toHaveBeenCalledWith('Supported networks:', 'sei, sei-testnet'); }); - it('forwards all connect arguments after binding runtime context', async () => { - const server = await getServer(); - const transport = { start: jest.fn() }; - const options = { testOption: true }; - - await (server.connect as unknown as (...args: unknown[]) => Promise)(transport, options); - - expect(mockConnect).toHaveBeenCalledWith(transport, options); - }); - - it('binds an onmessage handler assigned after connect', async () => { - const { getScopedAppConfig } = await import('../../core/config.js'); - const server = await getServer(); - const transport = { start: jest.fn(), onmessage: undefined as ((message: unknown) => void) | undefined }; - let sawRuntimeScope = false; - - await server.connect(transport as never); - transport.onmessage = () => { - sawRuntimeScope = getScopedAppConfig() !== undefined; - }; - transport.onmessage({}); - - expect(sawRuntimeScope).toBe(true); - }); - it('should sanitize and propagate server initialization errors', async () => { const testError = new Error('Initialization failed'); mockGetPackageInfo.mockImplementation(() => { throw testError; }); - await expect(getServer()).rejects.toThrow('Initialization failed'); + await expect(getServer(APP_CONFIG)).rejects.toThrow('Initialization failed'); expect(consoleErrorSpy).toHaveBeenCalledWith('Failed to initialize server:', 'Initialization failed'); expect(processExitSpy).not.toHaveBeenCalled(); diff --git a/packages/mcp-server/src/tests/server/transport/factory.test.ts b/packages/mcp-server/src/tests/server/transport/factory.test.ts index 73f01625..49a59351 100644 --- a/packages/mcp-server/src/tests/server/transport/factory.test.ts +++ b/packages/mcp-server/src/tests/server/transport/factory.test.ts @@ -47,7 +47,6 @@ describe('Transport Factory', () => { it('should create StdioTransport for stdio mode', () => { const config: TransportConfig = { mode: 'stdio', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', @@ -66,7 +65,6 @@ describe('Transport Factory', () => { it('should create StreamableHttpTransport for streamable-http mode', () => { const config: TransportConfig = { mode: 'streamable-http', - walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG, port: 8080, host: '0.0.0.0', @@ -91,7 +89,6 @@ describe('Transport Factory', () => { it('should create HttpSseTransport for http-sse mode', () => { const config: TransportConfig = { mode: 'http-sse', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 9000, host: '127.0.0.1', @@ -122,7 +119,6 @@ describe('Transport Factory', () => { }); const config: TransportConfig = { mode: 'stdio', - walletMode: 'private-key', appConfig, port: 3000, host: 'localhost', @@ -143,7 +139,6 @@ describe('Transport Factory', () => { }); const config: TransportConfig = { mode: 'streamable-http', - walletMode: 'disabled', appConfig, port: 8080, host: 'localhost', @@ -165,7 +160,7 @@ describe('Transport Factory', () => { it('should throw error for unsupported transport mode', () => { const config = { mode: 'unsupported-mode', - walletMode: 'disabled', + appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' @@ -184,7 +179,6 @@ describe('Transport Factory', () => { configs.forEach((params, index) => { const config: TransportConfig = { mode: 'streamable-http', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, ...params }; @@ -209,7 +203,6 @@ describe('Transport Factory', () => { // Test with minimal path const config1: TransportConfig = { mode: 'http-sse', - walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG, port: 1, host: '::1', // IPv6 localhost @@ -235,7 +228,6 @@ describe('Transport Factory', () => { // Test with maximum port number const config2: TransportConfig = { mode: 'streamable-http', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 65535, host: '0.0.0.0', @@ -263,7 +255,6 @@ describe('Transport Factory', () => { // This test ensures TypeScript compilation catches invalid configs const validConfig: TransportConfig = { mode: 'stdio', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', @@ -276,7 +267,6 @@ describe('Transport Factory', () => { it('should return McpTransport interface', () => { const config: TransportConfig = { mode: 'stdio', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', @@ -303,7 +293,6 @@ describe('Transport Factory', () => { it('should handle transport constructor errors', () => { const config: TransportConfig = { mode: 'stdio', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', @@ -324,7 +313,6 @@ describe('Transport Factory', () => { it('should handle config with missing mode', () => { const invalidConfig = { - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', @@ -338,9 +326,9 @@ describe('Transport Factory', () => { describe('integration scenarios', () => { it('should create different transport types in sequence', () => { const configs: TransportConfig[] = [ - { mode: 'stdio', walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' }, - { mode: 'streamable-http', walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG, port: 8080, host: '0.0.0.0', path: '/api' }, - { mode: 'http-sse', walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 9000, host: '127.0.0.1', path: '/sse' } + { mode: 'stdio', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', path: '/mcp' }, + { mode: 'streamable-http', appConfig: PRIVATE_KEY_APP_CONFIG, port: 8080, host: '0.0.0.0', path: '/api' }, + { mode: 'http-sse', appConfig: DISABLED_APP_CONFIG, port: 9000, host: '127.0.0.1', path: '/sse' } ]; const mockInstances = [{ mode: 'stdio' }, { mode: 'streamable-http' }, { mode: 'http-sse' }]; @@ -364,7 +352,6 @@ describe('Transport Factory', () => { it('should handle repeated creation of same transport type', () => { const config: TransportConfig = { mode: 'streamable-http', - walletMode: 'disabled', appConfig: DISABLED_APP_CONFIG, port: 3000, host: 'localhost', diff --git a/packages/mcp-server/src/tests/server/transport/stdio.test.ts b/packages/mcp-server/src/tests/server/transport/stdio.test.ts index e2a6dbc5..932255cf 100644 --- a/packages/mcp-server/src/tests/server/transport/stdio.test.ts +++ b/packages/mcp-server/src/tests/server/transport/stdio.test.ts @@ -5,6 +5,8 @@ jest.mock('@modelcontextprotocol/sdk/server/stdio.js', () => ({ StdioServerTransport: jest.fn() })); +const APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); + describe('StdioTransport', () => { let StdioTransport: any; let mockStdioServerTransport: jest.MockedClass; @@ -37,57 +39,28 @@ describe('StdioTransport', () => { describe('Constructor', () => { it('should initialize with stdio mode', () => { - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); expect(transport.mode).toBe('stdio'); }); it('should not have transport initially', () => { - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); expect((transport as any).transport).toBeUndefined(); }); - }); - - describe('start', () => { - it('scopes request handling to the construction snapshot after connect', async () => { - const configModule = await import('../../../core/config.js'); - const { config, getWalletMode, initializeConfig, snapshotConfig } = configModule; - const originalConfig = { ...config }; - const seenModes: string[] = []; - const mockTransportInstance = { - onmessage: undefined as ((message: unknown) => void) | undefined - }; - mockStdioServerTransport.mockImplementation(() => mockTransportInstance); - mockServer.connect.mockImplementation(async (transport: { onmessage?: (message: unknown) => void }) => { - transport.onmessage = () => { - seenModes.push(getWalletMode()); - }; - }); - try { - const appConfig = snapshotConfig({ - privateKey: undefined, - walletMode: 'disabled', - walletApiKey: undefined - }); - const transport = new StdioTransport(appConfig); - await transport.start(mockServer); - - initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '1'.repeat(64) }); - expect(getWalletMode()).toBe('private-key'); - mockTransportInstance.onmessage?.({ jsonrpc: '2.0', method: 'tools/list', id: 1 }); - expect(seenModes).toEqual(['disabled']); - } finally { - Object.assign(config, originalConfig); - } + it('requires an appConfig snapshot', () => { + expect(() => new StdioTransport()).toThrow('appConfig is required.'); }); + }); + describe('start', () => { it('should create StdioServerTransport and connect server', async () => { const mockTransportInstance = { connect: jest.fn() }; mockStdioServerTransport.mockImplementation(() => mockTransportInstance); - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); await transport.start(mockServer); expect(mockStdioServerTransport).toHaveBeenCalledWith(); @@ -99,7 +72,7 @@ describe('StdioTransport', () => { const mockTransportInstance = {}; mockStdioServerTransport.mockImplementation(() => mockTransportInstance); - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); await transport.start(mockServer); expect(consoleErrorSpy).toHaveBeenCalledWith('MCP Server ready (stdio transport)'); @@ -112,7 +85,7 @@ describe('StdioTransport', () => { const connectionError = new Error('Connection failed'); mockServer.connect.mockRejectedValue(connectionError); - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); await expect(transport.start(mockServer)).rejects.toThrow('Connection failed'); expect(mockStdioServerTransport).toHaveBeenCalledWith(); @@ -125,7 +98,7 @@ describe('StdioTransport', () => { throw constructorError; }); - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); await expect(transport.start(mockServer)).rejects.toThrow('Transport creation failed'); expect(mockServer.connect).not.toHaveBeenCalled(); @@ -134,7 +107,7 @@ describe('StdioTransport', () => { describe('stop', () => { it('should set transport to undefined', async () => { - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); // Set up transport first const mockTransportInstance = {}; @@ -151,7 +124,7 @@ describe('StdioTransport', () => { }); it('should resolve immediately if no transport exists', async () => { - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); // Don't start transport, just stop await expect(transport.stop()).resolves.toBeUndefined(); @@ -159,7 +132,7 @@ describe('StdioTransport', () => { }); it('should not throw errors during stop', async () => { - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); // Start and then stop multiple times const mockTransportInstance = {}; @@ -174,7 +147,7 @@ describe('StdioTransport', () => { describe('mode property', () => { it('should always return stdio', () => { - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); expect(transport.mode).toBe('stdio'); // Verify it's readonly - TypeScript prevents assignment but doesn't throw at runtime @@ -188,7 +161,7 @@ describe('StdioTransport', () => { const mockTransportInstance = {}; mockStdioServerTransport.mockImplementation(() => mockTransportInstance); - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); // Start await transport.start(mockServer); @@ -211,7 +184,7 @@ describe('StdioTransport', () => { return callCount === 1 ? mockTransportInstance1 : mockTransportInstance2; }); - const transport = new StdioTransport(); + const transport = new StdioTransport(APP_CONFIG); // First start await transport.start(mockServer); diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index 7a497bd6..035c14de 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -191,6 +191,8 @@ describe('stdio wallet isolation after another runtime stop', () => { initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: keyA }); const appConfig = snapshotConfig(); const server = await getServer(appConfig); + // The real SDK transport verifies that registered handlers carry the + // snapshot without relying on transport.onmessage interception. const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); const client = new Client({ name: 'stdio-wallet-isolation', version: '1.0.0' }); await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]); From e1ce4f6f81d326decac8962ee18cf9c586c5bf33 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 10:24:14 +0200 Subject: [PATCH 07/10] refactor(mcp-server): simplify scoped wallet policy Co-authored-by: Cursor --- .../fix-mcp-server-instance-scoped-wallet.md | 2 +- packages/mcp-server/src/core/config.ts | 8 ++- packages/mcp-server/src/core/tools.ts | 8 ++- packages/mcp-server/src/core/wallet/index.ts | 18 ++----- packages/mcp-server/src/server/server.ts | 2 + .../src/server/transport/http-sse.ts | 7 +-- .../src/server/transport/streamable-http.ts | 7 +-- .../mcp-server/src/tests/core/config.test.ts | 4 +- .../mcp-server/src/tests/core/tools.test.ts | 16 +++++- .../src/tests/core/wallet/index.test.ts | 28 +++++------ .../tests/server/transport/http-sse.test.ts | 5 +- .../server/transport/streamable-http.test.ts | 5 +- .../server/transport/wallet-isolation.test.ts | 49 +++++-------------- 13 files changed, 66 insertions(+), 93 deletions(-) diff --git a/.changeset/fix-mcp-server-instance-scoped-wallet.md b/.changeset/fix-mcp-server-instance-scoped-wallet.md index c8d67198..43305e90 100644 --- a/.changeset/fix-mcp-server-instance-scoped-wallet.md +++ b/.changeset/fix-mcp-server-instance-scoped-wallet.md @@ -6,4 +6,4 @@ Keep each MCP runtime on the wallet configuration that passed its security check A later programmatic `main()` could overwrite the process-wide config object while an HTTP listener started earlier was still serving requests. New sessions on that listener then built their tool list from the updated singleton, so a wallet-disabled HTTP server could expose signing tools after a trusted stdio start in the same process. No shipped CLI or host spawn does that, but the public lifecycle returned independent runtimes without isolating their keys. -`parseArgs()` now returns a frozen `AppConfig` snapshot, and every transport handles requests against that snapshot. Stopping one runtime releases only its provider, while other runtimes keep their original signer. HTTP transports require that snapshot, derive signing policy from it, and reject conflicting legacy `walletMode` options. +`parseArgs()` now returns a frozen `AppConfig` snapshot, and every transport handles requests against that snapshot. Stopping one runtime evicts only its provider cache entry, while other runtimes keep their original signer. HTTP transports require that snapshot and derive signing policy from it. diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index ae80c7d3..d01ed444 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -45,8 +45,11 @@ export const config: AppConfig = { walletApiKey: undefined }; +let warnedAboutUnscopedWalletRead = false; + export function initializeConfig(environment: Record = process.env): AppConfig { Object.assign(config, loadConfig(environment)); + warnedAboutUnscopedWalletRead = false; return config; } @@ -90,7 +93,10 @@ export function getScopedAppConfig(): AppConfigSnapshot | undefined { export function getRuntimeConfig(): Readonly { const scopedConfig = getScopedAppConfig(); if (scopedConfig) return scopedConfig; - if (config.walletMode !== 'disabled') { + // Direct helpers intentionally retain the process fallback. Warn once per + // initialization so a lost runtime scope is visible without flooding stderr. + if (config.walletMode !== 'disabled' && !warnedAboutUnscopedWalletRead) { + warnedAboutUnscopedWalletRead = true; console.error('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); } return config; diff --git a/packages/mcp-server/src/core/tools.ts b/packages/mcp-server/src/core/tools.ts index 98ac0e1a..11245da4 100644 --- a/packages/mcp-server/src/core/tools.ts +++ b/packages/mcp-server/src/core/tools.ts @@ -7,7 +7,7 @@ import { sanitizeError } from './errors.js'; import * as services from './services/index.js'; import { getWalletProvider } from './wallet/index.js'; -const READ_ONLY_TOOL_NAMES = new Set([ +export const READ_ONLY_TOOL_NAMES: ReadonlySet = new Set([ 'check_nft_ownership', 'estimate_gas', 'get_balance', @@ -26,13 +26,17 @@ const READ_ONLY_TOOL_NAMES = new Set([ 'get_transaction', 'get_transaction_receipt', 'is_contract', - 'read_contract' + 'read_contract', + 'search_docs' ]); export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boolean): McpServer { // Tool callbacks run after server construction, potentially after another // runtime updates process config. Capture this server's validated snapshot. const appConfig = getScopedAppConfig(); + if (walletEnabled && !appConfig) { + throw new Error('Wallet-enabled tool registration requires an active AppConfig scope.'); + } const registerTool = server.tool.bind(server) as unknown as ( name: string, description: string, diff --git a/packages/mcp-server/src/core/wallet/index.ts b/packages/mcp-server/src/core/wallet/index.ts index e600c565..d794fff2 100644 --- a/packages/mcp-server/src/core/wallet/index.ts +++ b/packages/mcp-server/src/core/wallet/index.ts @@ -1,4 +1,4 @@ -import { type AppConfigSnapshot, getScopedAppConfig, getWalletMode } from '../config.js'; +import { type AppConfigSnapshot, getScopedAppConfig, config as processConfig, snapshotConfig } from '../config.js'; import { DisabledWalletProvider } from './providers/disabled.js'; import { PrivateKeyWalletProvider } from './providers/private-key.js'; import type { WalletProvider } from './types.js'; @@ -40,24 +40,12 @@ export function getWalletProvider(): WalletProvider { return walletProviderInstance; } - const mode = getWalletMode(); - - switch (mode) { - case 'private-key': - walletProviderInstance = new PrivateKeyWalletProvider(); - break; - case 'disabled': - walletProviderInstance = new DisabledWalletProvider(); - break; - default: - throw new Error(`Unknown wallet mode: ${mode}`); - } - + walletProviderInstance = createWalletProvider(snapshotConfig(processConfig)); return walletProviderInstance; } /** - * Release one runtime's provider, or reset the process fallback for tests. + * Evict one runtime's cached provider, or reset the process fallback for tests. */ export function resetWalletProvider(appConfig?: AppConfigSnapshot): void { if (appConfig) { diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index f78f3968..b4515cfc 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -18,6 +18,8 @@ export const getServer = async (appConfig: AppConfigSnapshot) => { version: packageInfo.version }); + // Resources and prompts are wallet-independent. Any future + // wallet-sensitive callback must use the scoped tool policy below. registerEVMResources(server); registerEVMTools(server); registerEVMPrompts(server); diff --git a/packages/mcp-server/src/server/transport/http-sse.ts b/packages/mcp-server/src/server/transport/http-sse.ts index f1d06f2e..786d9282 100644 --- a/packages/mcp-server/src/server/transport/http-sse.ts +++ b/packages/mcp-server/src/server/transport/http-sse.ts @@ -8,7 +8,7 @@ import { sanitizeError } from '../../core/errors.js'; import { getServer } from '../server.js'; import { closeHttpServer, collectOperationErrors, runAllOperations, throwCollectedErrors } from './lifecycle.js'; import { createCorsMiddleware, validateSecurityConfig } from './security.js'; -import type { McpTransport, WalletMode } from './types.js'; +import type { McpTransport } from './types.js'; export type McpServerFactory = () => Promise; export type SseServerTransportFactory = (endpoint: string, response: Response) => SSEServerTransport; @@ -21,8 +21,6 @@ export interface HttpSseTransportOptions { host: string; path: string; appConfig: AppConfigSnapshot; - /** @deprecated Wallet mode is derived from appConfig. */ - walletMode?: WalletMode; maxSessions?: number; } @@ -68,9 +66,6 @@ export class HttpSseTransport implements McpTransport { this.path = options.path; if (!options.appConfig) throw new Error('appConfig is required.'); this.appConfig = snapshotConfig(options.appConfig); - if (options.walletMode !== undefined && options.walletMode !== this.appConfig.walletMode) { - throw new Error('walletMode must match appConfig.walletMode.'); - } this.maxSessions = options.maxSessions ?? DEFAULT_MAX_SSE_SESSIONS; this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = dependencies.transportFactory ?? ((endpoint, response) => new SSEServerTransport(endpoint, response)); diff --git a/packages/mcp-server/src/server/transport/streamable-http.ts b/packages/mcp-server/src/server/transport/streamable-http.ts index 23f6c8b7..e7ef916b 100644 --- a/packages/mcp-server/src/server/transport/streamable-http.ts +++ b/packages/mcp-server/src/server/transport/streamable-http.ts @@ -8,7 +8,7 @@ import { sanitizeError } from '../../core/errors.js'; import { getServer } from '../server.js'; import { closeHttpServer, collectOperationErrors, runAllOperations, throwCollectedErrors } from './lifecycle.js'; import { createCorsMiddleware, validateSecurityConfig } from './security.js'; -import type { McpTransport, TransportMode, WalletMode } from './types.js'; +import type { McpTransport, TransportMode } from './types.js'; export type StreamableServerFactory = () => Promise; export type StreamableTransportFactory = () => StreamableHTTPServerTransport; @@ -21,8 +21,6 @@ export interface StreamableHttpTransportOptions { host?: string; path?: string; appConfig: AppConfigSnapshot; - /** @deprecated Wallet mode is derived from appConfig. */ - walletMode?: WalletMode; maxActiveRequests?: number; } @@ -68,9 +66,6 @@ export class StreamableHttpTransport implements McpTransport { this.path = options.path ?? '/mcp'; if (!options.appConfig) throw new Error('appConfig is required.'); this.appConfig = snapshotConfig(options.appConfig); - if (options.walletMode !== undefined && options.walletMode !== this.appConfig.walletMode) { - throw new Error('walletMode must match appConfig.walletMode.'); - } this.maxActiveRequests = options.maxActiveRequests ?? DEFAULT_MAX_STREAMABLE_REQUESTS; this.serverFactory = dependencies.serverFactory ?? (() => getServer(this.appConfig)); this.transportFactory = diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index edf294f1..af26d1d1 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -146,14 +146,14 @@ describe('Config Module - Actual Implementation', () => { consoleErrorSpy.mockRestore(); }); - test('warns on every enabled-wallet read outside a runtime scope', () => { + test('warns once per initialization for enabled-wallet reads outside a runtime scope', () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '5'.repeat(64) }); expect(getWalletMode()).toBe('private-key'); expect(isWalletEnabled()).toBe(true); - expect(consoleErrorSpy).toHaveBeenCalledTimes(2); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); expect(consoleErrorSpy).toHaveBeenCalledWith('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); consoleErrorSpy.mockRestore(); }); diff --git a/packages/mcp-server/src/tests/core/tools.test.ts b/packages/mcp-server/src/tests/core/tools.test.ts index c0e9b970..a6d80e4a 100644 --- a/packages/mcp-server/src/tests/core/tools.test.ts +++ b/packages/mcp-server/src/tests/core/tools.test.ts @@ -56,7 +56,7 @@ const resetSpiedFunctions = (mod: object) => { }; const { getSupportedNetworks } = chains; -const { getPrivateKeyAsHex, getWalletMode, isWalletEnabled } = config; +const { getPrivateKeyAsHex, getScopedAppConfig, getWalletMode, isWalletEnabled, wrapWithAppConfig } = config; const { getWalletProvider } = wallet; describe('EVM Tools', () => { @@ -91,6 +91,14 @@ describe('EVM Tools', () => { (getPrivateKeyAsHex as jest.Mock).mockReturnValue('0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890'); (isWalletEnabled as jest.Mock).mockReturnValue(true); // Enable wallet for testing (getWalletMode as jest.Mock).mockReturnValue('private-key'); // Set wallet mode + (getScopedAppConfig as jest.Mock).mockReturnValue( + Object.freeze({ + privateKey: '0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890', + walletMode: 'private-key', + walletApiKey: undefined + }) + ); + (wrapWithAppConfig as jest.Mock).mockImplementation((_appConfig, handler) => handler); // Mock wallet provider const mockWalletProvider = { @@ -990,6 +998,12 @@ describe('EVM Tools', () => { ); }); + test('rejects wallet-enabled registration without an active runtime scope', () => { + (getScopedAppConfig as jest.Mock).mockReturnValue(undefined); + + expect(() => withToolRegistrationPolicy(server, true)).toThrow('Wallet-enabled tool registration requires an active AppConfig scope.'); + }); + test('hides unknown future tools by default and forwards non-tool methods', () => { const tool = jest.fn(); const diagnostic = jest.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/packages/mcp-server/src/tests/core/wallet/index.test.ts b/packages/mcp-server/src/tests/core/wallet/index.test.ts index 13967cfd..9f47f87d 100644 --- a/packages/mcp-server/src/tests/core/wallet/index.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/index.test.ts @@ -6,9 +6,11 @@ import type { WalletProvider } from '../../../core/wallet/types.js'; // Mock dependencies jest.mock('../../../core/config.js', () => { + const config = { privateKey: undefined as string | undefined, walletMode: 'disabled', walletApiKey: undefined }; return { - getWalletMode: jest.fn(), - getScopedAppConfig: jest.fn() + config, + getScopedAppConfig: jest.fn(), + snapshotConfig: jest.fn(() => Object.freeze({ ...config })) }; }); @@ -20,7 +22,7 @@ jest.mock('../../../core/wallet/providers/disabled.js', () => ({ DisabledWalletProvider: jest.fn() })); -import { getScopedAppConfig, getWalletMode } from '../../../core/config.js'; +import { getScopedAppConfig, config as processConfig, snapshotConfig } from '../../../core/config.js'; describe('Wallet Provider', () => { const mockPrivateKeyProvider: WalletProvider = { @@ -46,7 +48,9 @@ describe('Wallet Provider', () => { // Reset all mocks jest.resetAllMocks(); + Object.assign(processConfig, { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); (getScopedAppConfig as jest.Mock).mockReturnValue(undefined); + (snapshotConfig as jest.Mock).mockImplementation(() => Object.freeze({ ...processConfig })); // Setup default mock implementations (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); @@ -55,27 +59,23 @@ describe('Wallet Provider', () => { describe('getWalletProvider', () => { test('should create and return PrivateKeyWalletProvider for private-key mode', () => { - (getWalletMode as jest.Mock).mockReturnValue('private-key'); + Object.assign(processConfig, { privateKey: '0xabc', walletMode: 'private-key' }); const provider = getWalletProvider(); - expect(getWalletMode).toHaveBeenCalled(); - expect(PrivateKeyWalletProvider).toHaveBeenCalled(); + expect(PrivateKeyWalletProvider).toHaveBeenCalledWith({ privateKey: '0xabc' }); expect(provider).toBe(mockPrivateKeyProvider); }); test('should create and return DisabledWalletProvider for disabled mode', () => { - (getWalletMode as jest.Mock).mockReturnValue('disabled'); - const provider = getWalletProvider(); - expect(getWalletMode).toHaveBeenCalled(); expect(DisabledWalletProvider).toHaveBeenCalled(); expect(provider).toBe(mockDisabledProvider); }); test('should return cached provider on subsequent calls', () => { - (getWalletMode as jest.Mock).mockReturnValue('private-key'); + Object.assign(processConfig, { privateKey: '0xabc', walletMode: 'private-key' }); // First call const provider1 = getWalletProvider(); @@ -86,13 +86,13 @@ describe('Wallet Provider', () => { // Second call should return cached provider const provider2 = getWalletProvider(); - expect(getWalletMode).not.toHaveBeenCalled(); + expect(snapshotConfig).not.toHaveBeenCalled(); expect(PrivateKeyWalletProvider).not.toHaveBeenCalled(); expect(provider2).toBe(provider1); }); test('should throw error for unknown wallet mode', () => { - (getWalletMode as jest.Mock).mockReturnValue('unknown-mode'); + processConfig.walletMode = 'unknown-mode' as never; expect(() => getWalletProvider()).toThrow('Unknown wallet mode: unknown-mode'); }); @@ -103,7 +103,6 @@ describe('Wallet Provider', () => { const provider1 = getWalletProvider(); expect(PrivateKeyWalletProvider).toHaveBeenCalledWith({ privateKey: '0xabc' }); - expect(getWalletMode).not.toHaveBeenCalled(); expect(provider1).toBe(mockPrivateKeyProvider); resetWalletProvider(); @@ -114,7 +113,6 @@ describe('Wallet Provider', () => { const provider2 = getWalletProvider(); expect(provider2).toBe(provider1); expect(PrivateKeyWalletProvider).not.toHaveBeenCalled(); - expect(getWalletMode).not.toHaveBeenCalled(); }); test('recreates only the provider for a reset snapshot', () => { @@ -132,7 +130,7 @@ describe('Wallet Provider', () => { describe('resetWalletProvider', () => { test('should reset the wallet provider instance', () => { - (getWalletMode as jest.Mock).mockReturnValue('private-key'); + Object.assign(processConfig, { privateKey: '0xabc', walletMode: 'private-key' }); // Create separate mock instances for each call const mockProvider1: WalletProvider = { diff --git a/packages/mcp-server/src/tests/server/transport/http-sse.test.ts b/packages/mcp-server/src/tests/server/transport/http-sse.test.ts index e9444630..211d438d 100644 --- a/packages/mcp-server/src/tests/server/transport/http-sse.test.ts +++ b/packages/mcp-server/src/tests/server/transport/http-sse.test.ts @@ -62,10 +62,7 @@ describe('HttpSseTransport', () => { throw new Error(`process.exit called with code ${code}`); }); const listenFactory = jest.fn(); - const transport = new HttpSseTransport( - { port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG }, - { listenFactory } - ); + const transport = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, appConfig: PRIVATE_KEY_APP_CONFIG }, { listenFactory }); await expect(transport.start()).rejects.toThrow('process.exit called with code 1'); expect(processExit).toHaveBeenCalledWith(1); diff --git a/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts b/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts index 5901ac8f..e282aa3c 100644 --- a/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts +++ b/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts @@ -62,10 +62,7 @@ describe('StreamableHttpTransport', () => { throw new Error(`process.exit called with code ${code}`); }); const listenFactory = jest.fn(); - const transport = new StreamableHttpTransport( - { port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig: PRIVATE_KEY_APP_CONFIG }, - { listenFactory } - ); + const transport = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, appConfig: PRIVATE_KEY_APP_CONFIG }, { listenFactory }); await expect(transport.start()).rejects.toThrow('process.exit called with code 1'); expect(processExit).toHaveBeenCalledWith(1); diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index 035c14de..a7a7bb13 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -5,6 +5,7 @@ import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/ import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { privateKeyToAccount } from 'viem/accounts'; import { config, initializeConfig, isWalletEnabled, runWithAppConfig, snapshotConfig } from '../../../core/config.js'; +import { READ_ONLY_TOOL_NAMES } from '../../../core/tools.js'; import { getWalletProvider, resetWalletProvider } from '../../../core/wallet/index.js'; import { getServer } from '../../../server/server.js'; import { HttpSseTransport } from '../../../server/transport/http-sse.js'; @@ -13,21 +14,10 @@ import { StreamableHttpTransport } from '../../../server/transport/streamable-ht const HOST = '127.0.0.1'; const PATH = '/mcp'; const PRIVATE_KEY = '1'.repeat(64); -const WALLET_TOOLS = new Set([ - 'approve_token_spending', - 'deploy_contract', - 'get_address_from_private_key', - 'transfer_erc1155', - 'transfer_erc20', - 'transfer_nft', - 'transfer_sei', - 'transfer_token', - 'write_contract' -]); type HttpMode = 'http-sse' | 'streamable-http'; -async function listWalletTools(mode: HttpMode, url: URL): Promise { +async function listUnsafeTools(mode: HttpMode, url: URL): Promise { const client = new Client({ name: `wallet-isolation-${mode}`, version: '1.0.0' }); const transport = mode === 'http-sse' ? new SSEClientTransport(url) : new StreamableHTTPClientTransport(url); await client.connect(transport); @@ -35,7 +25,7 @@ async function listWalletTools(mode: HttpMode, url: URL): Promise { const listed = await client.listTools(); return listed.tools .map((tool) => tool.name) - .filter((name) => WALLET_TOOLS.has(name)) + .filter((name) => !READ_ONLY_TOOL_NAMES.has(name)) .sort(); } finally { await client.close(); @@ -74,24 +64,24 @@ describe('HTTP wallet isolation across later starts', () => { }); const transport = mode === 'http-sse' - ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled', appConfig }) - : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, walletMode: 'disabled', appConfig }); + ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig }) + : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, appConfig }); transports.push(transport); await transport.start(); const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); - expect(await listWalletTools(mode, url)).toEqual([]); + expect(await listUnsafeTools(mode, url)).toEqual([]); process.env.WALLET_MODE = 'private-key'; process.env.PRIVATE_KEY = PRIVATE_KEY; - expect(await listWalletTools(mode, url)).toEqual([]); + expect(await listUnsafeTools(mode, url)).toEqual([]); expect(isWalletEnabled()).toBe(false); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: PRIVATE_KEY }); process.env.WALLET_MODE = 'disabled'; delete process.env.PRIVATE_KEY; expect(isWalletEnabled()).toBe(true); - expect(await listWalletTools(mode, url)).toEqual([]); + expect(await listUnsafeTools(mode, url)).toEqual([]); }); it.each(['http-sse', 'streamable-http'] as const)('snapshots a mutable %s appConfig at construction', async (mode) => { @@ -107,7 +97,7 @@ describe('HTTP wallet isolation across later starts', () => { appConfig.walletMode = 'private-key'; appConfig.privateKey = `0x${PRIVATE_KEY}`; const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); - expect(await listWalletTools(mode, url)).toEqual([]); + expect(await listUnsafeTools(mode, url)).toEqual([]); }); it('still terminates a direct private-key HTTP start before listen', async () => { @@ -117,8 +107,8 @@ describe('HTTP wallet isolation across later starts', () => { }); const listenFactory = jest.fn(); const appConfig = snapshotConfig({ privateKey: `0x${PRIVATE_KEY}`, walletMode: 'private-key', walletApiKey: undefined }); - const streamable = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig }, { listenFactory }); - const sse = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, walletMode: 'private-key', appConfig }, { listenFactory }); + const streamable = new StreamableHttpTransport({ port: 8080, host: HOST, path: PATH, appConfig }, { listenFactory }); + const sse = new HttpSseTransport({ port: 8080, host: HOST, path: PATH, appConfig }, { listenFactory }); await expect(streamable.start()).rejects.toThrow('process.exit called with code 1'); await expect(sse.start()).rejects.toThrow('process.exit called with code 1'); @@ -127,7 +117,7 @@ describe('HTTP wallet isolation across later starts', () => { processExit.mockRestore(); }); - it.each(['http-sse', 'streamable-http'] as const)('rejects %s when appConfig enables the wallet and walletMode is omitted', async (mode) => { + it.each(['http-sse', 'streamable-http'] as const)('rejects a wallet-enabled %s appConfig before listen', async (mode) => { consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); const processExit = jest.spyOn(process, 'exit').mockImplementation((code) => { throw new Error(`process.exit called with code ${code}`); @@ -147,19 +137,6 @@ describe('HTTP wallet isolation across later starts', () => { processExit.mockRestore(); }); - it.each(['http-sse', 'streamable-http'] as const)('rejects conflicting compatibility walletMode for %s at construction', (mode) => { - const appConfig = snapshotConfig({ - privateKey: `0x${PRIVATE_KEY}`, - walletMode: 'private-key', - walletApiKey: undefined - }); - const options = { port: 8080, host: HOST, path: PATH, walletMode: 'disabled' as const, appConfig }; - - expect(() => (mode === 'http-sse' ? new HttpSseTransport(options) : new StreamableHttpTransport(options))).toThrow( - 'walletMode must match appConfig.walletMode.' - ); - }); - it.each(['http-sse', 'streamable-http'] as const)('requires an explicit appConfig for %s', (mode) => { const options = { port: 8080, host: HOST, path: PATH }; @@ -199,7 +176,7 @@ describe('stdio wallet isolation after another runtime stop', () => { try { const listed = await client.listTools(); - expect(listed.tools.filter((tool) => WALLET_TOOLS.has(tool.name))).not.toEqual([]); + expect(listed.tools.filter((tool) => !READ_ONLY_TOOL_NAMES.has(tool.name))).not.toEqual([]); expect(await deriveAddressFromPrivateKey(client)).toBe(expectedAddress); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: keyB }); From 40523dd7a75bfa91f61414c49c8c64824c685348 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 10:33:17 +0200 Subject: [PATCH 08/10] test(mcp-server): harden wallet isolation coverage Co-authored-by: Cursor --- packages/mcp-server/src/core/config.ts | 6 +-- packages/mcp-server/src/core/tools.ts | 11 ++--- packages/mcp-server/src/core/wallet/index.ts | 3 +- packages/mcp-server/src/server/server.ts | 5 ++- .../mcp-server/src/tests/core/tools.test.ts | 5 ++- .../server/transport/wallet-isolation.test.ts | 40 ++++++++++++++----- 6 files changed, 42 insertions(+), 28 deletions(-) diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index d01ed444..70e19b73 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -62,11 +62,7 @@ const runtimeConfig = new AsyncLocalStorage(); */ export function snapshotConfig(source: Readonly = config): AppConfigSnapshot { if (Object.isFrozen(source)) return source; - return Object.freeze({ - privateKey: source.privateKey, - walletMode: source.walletMode, - walletApiKey: source.walletApiKey - }); + return Object.freeze({ ...source }); } export function runWithAppConfig(appConfig: AppConfigSnapshot, fn: () => T): T { diff --git a/packages/mcp-server/src/core/tools.ts b/packages/mcp-server/src/core/tools.ts index 11245da4..a9cf0ae7 100644 --- a/packages/mcp-server/src/core/tools.ts +++ b/packages/mcp-server/src/core/tools.ts @@ -7,7 +7,7 @@ import { sanitizeError } from './errors.js'; import * as services from './services/index.js'; import { getWalletProvider } from './wallet/index.js'; -export const READ_ONLY_TOOL_NAMES: ReadonlySet = new Set([ +const READ_ONLY_TOOL_NAMES = new Set([ 'check_nft_ownership', 'estimate_gas', 'get_balance', @@ -26,17 +26,14 @@ export const READ_ONLY_TOOL_NAMES: ReadonlySet = new Set([ 'get_transaction', 'get_transaction_receipt', 'is_contract', - 'read_contract', - 'search_docs' + 'read_contract' ]); export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boolean): McpServer { // Tool callbacks run after server construction, potentially after another // runtime updates process config. Capture this server's validated snapshot. const appConfig = getScopedAppConfig(); - if (walletEnabled && !appConfig) { - throw new Error('Wallet-enabled tool registration requires an active AppConfig scope.'); - } + if (!appConfig) throw new Error('Tool registration requires an active AppConfig scope.'); const registerTool = server.tool.bind(server) as unknown as ( name: string, description: string, @@ -52,7 +49,7 @@ export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boo console.error(`Tool registration suppressed by wallet policy: ${name}`); return undefined; } - const runtimeHandler = appConfig ? wrapWithAppConfig(appConfig, handler) : handler; + const runtimeHandler = wrapWithAppConfig(appConfig, handler); return registerTool(name, description, 'network' in schema ? { ...schema, network: networkSchema.optional() } : schema, runtimeHandler); }; } diff --git a/packages/mcp-server/src/core/wallet/index.ts b/packages/mcp-server/src/core/wallet/index.ts index d794fff2..d9327421 100644 --- a/packages/mcp-server/src/core/wallet/index.ts +++ b/packages/mcp-server/src/core/wallet/index.ts @@ -5,7 +5,8 @@ import type { WalletProvider } from './types.js'; const providersByConfig = new WeakMap(); -// Cache for the process-global singleton only. Instance snapshots use the WeakMap. +// Deliberate process-lifetime fallback for direct, unscoped consumers. +// Runtime snapshots use the WeakMap and are evicted by their own stop(). let walletProviderInstance: WalletProvider | null = null; function createWalletProvider(appConfig: AppConfigSnapshot): WalletProvider { diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index b4515cfc..7fc17754 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -18,8 +18,9 @@ export const getServer = async (appConfig: AppConfigSnapshot) => { version: packageInfo.version }); - // Resources and prompts are wallet-independent. Any future - // wallet-sensitive callback must use the scoped tool policy below. + // Registration stays inside this scope so wallet-gated prompts and + // tool policy read this snapshot. Future wallet-sensitive callbacks + // must also be bound through the scoped tool policy. registerEVMResources(server); registerEVMTools(server); registerEVMPrompts(server); diff --git a/packages/mcp-server/src/tests/core/tools.test.ts b/packages/mcp-server/src/tests/core/tools.test.ts index a6d80e4a..9118b6d1 100644 --- a/packages/mcp-server/src/tests/core/tools.test.ts +++ b/packages/mcp-server/src/tests/core/tools.test.ts @@ -998,10 +998,11 @@ describe('EVM Tools', () => { ); }); - test('rejects wallet-enabled registration without an active runtime scope', () => { + test('rejects tool registration without an active runtime scope', () => { (getScopedAppConfig as jest.Mock).mockReturnValue(undefined); - expect(() => withToolRegistrationPolicy(server, true)).toThrow('Wallet-enabled tool registration requires an active AppConfig scope.'); + expect(() => withToolRegistrationPolicy(server, true)).toThrow('Tool registration requires an active AppConfig scope.'); + expect(() => withToolRegistrationPolicy(server, false)).toThrow('Tool registration requires an active AppConfig scope.'); }); test('hides unknown future tools by default and forwards non-tool methods', () => { diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index a7a7bb13..c81628d3 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -5,7 +5,6 @@ import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/ import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { privateKeyToAccount } from 'viem/accounts'; import { config, initializeConfig, isWalletEnabled, runWithAppConfig, snapshotConfig } from '../../../core/config.js'; -import { READ_ONLY_TOOL_NAMES } from '../../../core/tools.js'; import { getWalletProvider, resetWalletProvider } from '../../../core/wallet/index.js'; import { getServer } from '../../../server/server.js'; import { HttpSseTransport } from '../../../server/transport/http-sse.js'; @@ -14,19 +13,38 @@ import { StreamableHttpTransport } from '../../../server/transport/streamable-ht const HOST = '127.0.0.1'; const PATH = '/mcp'; const PRIVATE_KEY = '1'.repeat(64); +const EXPECTED_HTTP_TOOL_NAMES = [ + 'check_nft_ownership', + 'estimate_gas', + 'get_balance', + 'get_block_by_number', + 'get_chain_info', + 'get_erc1155_balance', + 'get_erc1155_token_uri', + 'get_erc20_balance', + 'get_latest_block', + 'get_nft_balance', + 'get_nft_info', + 'get_supported_networks', + 'get_token_balance', + 'get_token_balance_erc20', + 'get_token_info', + 'get_transaction', + 'get_transaction_receipt', + 'is_contract', + 'read_contract', + 'search_docs' +].sort(); type HttpMode = 'http-sse' | 'streamable-http'; -async function listUnsafeTools(mode: HttpMode, url: URL): Promise { +async function listTools(mode: HttpMode, url: URL): Promise { const client = new Client({ name: `wallet-isolation-${mode}`, version: '1.0.0' }); const transport = mode === 'http-sse' ? new SSEClientTransport(url) : new StreamableHTTPClientTransport(url); await client.connect(transport); try { const listed = await client.listTools(); - return listed.tools - .map((tool) => tool.name) - .filter((name) => !READ_ONLY_TOOL_NAMES.has(name)) - .sort(); + return listed.tools.map((tool) => tool.name).sort(); } finally { await client.close(); } @@ -70,18 +88,18 @@ describe('HTTP wallet isolation across later starts', () => { await transport.start(); const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); - expect(await listUnsafeTools(mode, url)).toEqual([]); + expect(await listTools(mode, url)).toEqual(EXPECTED_HTTP_TOOL_NAMES); process.env.WALLET_MODE = 'private-key'; process.env.PRIVATE_KEY = PRIVATE_KEY; - expect(await listUnsafeTools(mode, url)).toEqual([]); + expect(await listTools(mode, url)).toEqual(EXPECTED_HTTP_TOOL_NAMES); expect(isWalletEnabled()).toBe(false); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: PRIVATE_KEY }); process.env.WALLET_MODE = 'disabled'; delete process.env.PRIVATE_KEY; expect(isWalletEnabled()).toBe(true); - expect(await listUnsafeTools(mode, url)).toEqual([]); + expect(await listTools(mode, url)).toEqual(EXPECTED_HTTP_TOOL_NAMES); }); it.each(['http-sse', 'streamable-http'] as const)('snapshots a mutable %s appConfig at construction', async (mode) => { @@ -97,7 +115,7 @@ describe('HTTP wallet isolation across later starts', () => { appConfig.walletMode = 'private-key'; appConfig.privateKey = `0x${PRIVATE_KEY}`; const url = new URL(`http://${HOST}:${transport.getListeningPort()}${PATH}`); - expect(await listUnsafeTools(mode, url)).toEqual([]); + expect(await listTools(mode, url)).toEqual(EXPECTED_HTTP_TOOL_NAMES); }); it('still terminates a direct private-key HTTP start before listen', async () => { @@ -176,7 +194,7 @@ describe('stdio wallet isolation after another runtime stop', () => { try { const listed = await client.listTools(); - expect(listed.tools.filter((tool) => !READ_ONLY_TOOL_NAMES.has(tool.name))).not.toEqual([]); + expect(listed.tools.map((tool) => tool.name)).toContain('get_address_from_private_key'); expect(await deriveAddressFromPrivateKey(client)).toBe(expectedAddress); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: keyB }); From f7364c90b121590f7ef542bab3797fa92c95c835 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 10:44:47 +0200 Subject: [PATCH 09/10] refactor(mcp-server): bind all runtime callbacks Co-authored-by: Cursor --- packages/mcp-server/src/core/config.ts | 7 +- packages/mcp-server/src/core/tools.ts | 9 +-- packages/mcp-server/src/core/wallet/index.ts | 23 ++---- packages/mcp-server/src/server/server.ts | 36 +++++++-- .../mcp-server/src/tests/core/config.test.ts | 5 +- .../mcp-server/src/tests/core/tools.test.ts | 17 +--- .../src/tests/core/wallet/index.test.ts | 77 ++++--------------- .../src/tests/server/server.test.ts | 23 +++++- .../tests/server/transport/factory.test.ts | 9 ++- .../tests/server/transport/http-sse.test.ts | 5 +- .../src/tests/server/transport/stdio.test.ts | 3 +- .../server/transport/streamable-http.test.ts | 5 +- .../server/transport/wallet-isolation.test.ts | 10 +-- 13 files changed, 97 insertions(+), 132 deletions(-) diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index 70e19b73..d99aa6f5 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -26,7 +26,8 @@ export interface AppConfig { walletApiKey: string | undefined; } -export type AppConfigSnapshot = Readonly; +declare const snapshotBrand: unique symbol; +export type AppConfigSnapshot = Readonly & { readonly [snapshotBrand]: true }; export const loadConfig = (environment: Record = process.env): AppConfig => { const env = envSchema.parse(environment); @@ -61,8 +62,8 @@ const runtimeConfig = new AsyncLocalStorage(); * change an already-running runtime's tool policy or signer. */ export function snapshotConfig(source: Readonly = config): AppConfigSnapshot { - if (Object.isFrozen(source)) return source; - return Object.freeze({ ...source }); + if (Object.isFrozen(source)) return source as AppConfigSnapshot; + return Object.freeze({ ...source }) as AppConfigSnapshot; } export function runWithAppConfig(appConfig: AppConfigSnapshot, fn: () => T): T { diff --git a/packages/mcp-server/src/core/tools.ts b/packages/mcp-server/src/core/tools.ts index a9cf0ae7..f58a83c0 100644 --- a/packages/mcp-server/src/core/tools.ts +++ b/packages/mcp-server/src/core/tools.ts @@ -2,7 +2,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { Address, Hash, Hex } from 'viem'; import { z } from 'zod'; import { DEFAULT_NETWORK, getSupportedNetworks, networkSchema } from './chains.js'; -import { getScopedAppConfig, isWalletEnabled, wrapWithAppConfig } from './config.js'; +import { isWalletEnabled } from './config.js'; import { sanitizeError } from './errors.js'; import * as services from './services/index.js'; import { getWalletProvider } from './wallet/index.js'; @@ -30,10 +30,6 @@ const READ_ONLY_TOOL_NAMES = new Set([ ]); export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boolean): McpServer { - // Tool callbacks run after server construction, potentially after another - // runtime updates process config. Capture this server's validated snapshot. - const appConfig = getScopedAppConfig(); - if (!appConfig) throw new Error('Tool registration requires an active AppConfig scope.'); const registerTool = server.tool.bind(server) as unknown as ( name: string, description: string, @@ -49,8 +45,7 @@ export function withToolRegistrationPolicy(server: McpServer, walletEnabled: boo console.error(`Tool registration suppressed by wallet policy: ${name}`); return undefined; } - const runtimeHandler = wrapWithAppConfig(appConfig, handler); - return registerTool(name, description, 'network' in schema ? { ...schema, network: networkSchema.optional() } : schema, runtimeHandler); + return registerTool(name, description, 'network' in schema ? { ...schema, network: networkSchema.optional() } : schema, handler); }; } diff --git a/packages/mcp-server/src/core/wallet/index.ts b/packages/mcp-server/src/core/wallet/index.ts index d9327421..622f2808 100644 --- a/packages/mcp-server/src/core/wallet/index.ts +++ b/packages/mcp-server/src/core/wallet/index.ts @@ -1,14 +1,10 @@ -import { type AppConfigSnapshot, getScopedAppConfig, config as processConfig, snapshotConfig } from '../config.js'; +import { type AppConfigSnapshot, getRuntimeConfig, getScopedAppConfig, snapshotConfig } from '../config.js'; import { DisabledWalletProvider } from './providers/disabled.js'; import { PrivateKeyWalletProvider } from './providers/private-key.js'; import type { WalletProvider } from './types.js'; const providersByConfig = new WeakMap(); -// Deliberate process-lifetime fallback for direct, unscoped consumers. -// Runtime snapshots use the WeakMap and are evicted by their own stop(). -let walletProviderInstance: WalletProvider | null = null; - function createWalletProvider(appConfig: AppConfigSnapshot): WalletProvider { switch (appConfig.walletMode) { case 'private-key': @@ -37,23 +33,14 @@ export function getWalletProvider(): WalletProvider { return providerForSnapshot(runtime); } - if (walletProviderInstance) { - return walletProviderInstance; - } - - walletProviderInstance = createWalletProvider(snapshotConfig(processConfig)); - return walletProviderInstance; + return createWalletProvider(snapshotConfig(getRuntimeConfig())); } /** - * Evict one runtime's cached provider, or reset the process fallback for tests. + * Evict one runtime's cached provider. */ -export function resetWalletProvider(appConfig?: AppConfigSnapshot): void { - if (appConfig) { - providersByConfig.delete(appConfig); - } else { - walletProviderInstance = null; - } +export function resetWalletProvider(appConfig: AppConfigSnapshot): void { + providersByConfig.delete(appConfig); } export { DisabledWalletProvider } from './providers/disabled.js'; diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index 7fc17754..ddc2b378 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -1,6 +1,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { getSupportedNetworks } from '../core/chains.js'; -import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig } from '../core/config.js'; +import { type AppConfigSnapshot, runWithAppConfig, snapshotConfig, wrapWithAppConfig } from '../core/config.js'; import { sanitizeError } from '../core/errors.js'; import { registerEVMPrompts } from '../core/prompts.js'; import { registerEVMResources } from '../core/resources.js'; @@ -8,6 +8,26 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; +const SCOPED_REGISTRATION_METHODS = new Set(['tool', 'resource', 'prompt']); + +function withScopedCallbacks(server: McpServer, appConfig: AppConfigSnapshot): McpServer { + return new Proxy(server, { + get(target, property) { + const value = Reflect.get(target, property, target); + if (typeof value !== 'function') return value; + if (!SCOPED_REGISTRATION_METHODS.has(property)) return value.bind(target); + + return (...args: unknown[]) => { + let callbackIndex = args.length - 1; + while (callbackIndex >= 0 && typeof args[callbackIndex] !== 'function') callbackIndex--; + if (callbackIndex < 0) throw new Error(`MCP ${String(property)} registration requires a callback.`); + args[callbackIndex] = wrapWithAppConfig(appConfig, args[callbackIndex] as (...callbackArgs: unknown[]) => unknown); + return Reflect.apply(value, target, args); + }; + } + }); +} + export const getServer = async (appConfig: AppConfigSnapshot) => { const config = snapshotConfig(appConfig); return runWithAppConfig(config, async () => { @@ -17,14 +37,14 @@ export const getServer = async (appConfig: AppConfigSnapshot) => { name: packageInfo.name, version: packageInfo.version }); + const scopedServer = withScopedCallbacks(server, config); - // Registration stays inside this scope so wallet-gated prompts and - // tool policy read this snapshot. Future wallet-sensitive callbacks - // must also be bound through the scoped tool policy. - registerEVMResources(server); - registerEVMTools(server); - registerEVMPrompts(server); - createDocsSearchTool(server, packageInfo); + // Registration stays inside this scope for wallet-gated policy, and + // every request callback is permanently bound to the same snapshot. + registerEVMResources(scopedServer); + registerEVMTools(scopedServer); + registerEVMPrompts(scopedServer); + createDocsSearchTool(scopedServer, packageInfo); console.error('Supported networks:', getSupportedNetworks().join(', ')); diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index af26d1d1..31de0968 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, jest, test } from 'bun:test'; +import type { AppConfigSnapshot } from '../../core/config.js'; import { config, formatPrivateKey, @@ -93,7 +94,7 @@ describe('Config Module - Actual Implementation', () => { test('snapshots freeze a copy that later initializeConfig mutations cannot change', () => { const snapshot = snapshotConfig(); - expect(snapshot).toEqual({ + expect(snapshot).toMatchObject({ privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined @@ -116,7 +117,7 @@ describe('Config Module - Actual Implementation', () => { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined - }, + } as unknown as AppConfigSnapshot, () => undefined ) ).toThrow('runWithAppConfig requires a frozen AppConfig snapshot.'); diff --git a/packages/mcp-server/src/tests/core/tools.test.ts b/packages/mcp-server/src/tests/core/tools.test.ts index 9118b6d1..c0e9b970 100644 --- a/packages/mcp-server/src/tests/core/tools.test.ts +++ b/packages/mcp-server/src/tests/core/tools.test.ts @@ -56,7 +56,7 @@ const resetSpiedFunctions = (mod: object) => { }; const { getSupportedNetworks } = chains; -const { getPrivateKeyAsHex, getScopedAppConfig, getWalletMode, isWalletEnabled, wrapWithAppConfig } = config; +const { getPrivateKeyAsHex, getWalletMode, isWalletEnabled } = config; const { getWalletProvider } = wallet; describe('EVM Tools', () => { @@ -91,14 +91,6 @@ describe('EVM Tools', () => { (getPrivateKeyAsHex as jest.Mock).mockReturnValue('0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890'); (isWalletEnabled as jest.Mock).mockReturnValue(true); // Enable wallet for testing (getWalletMode as jest.Mock).mockReturnValue('private-key'); // Set wallet mode - (getScopedAppConfig as jest.Mock).mockReturnValue( - Object.freeze({ - privateKey: '0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890', - walletMode: 'private-key', - walletApiKey: undefined - }) - ); - (wrapWithAppConfig as jest.Mock).mockImplementation((_appConfig, handler) => handler); // Mock wallet provider const mockWalletProvider = { @@ -998,13 +990,6 @@ describe('EVM Tools', () => { ); }); - test('rejects tool registration without an active runtime scope', () => { - (getScopedAppConfig as jest.Mock).mockReturnValue(undefined); - - expect(() => withToolRegistrationPolicy(server, true)).toThrow('Tool registration requires an active AppConfig scope.'); - expect(() => withToolRegistrationPolicy(server, false)).toThrow('Tool registration requires an active AppConfig scope.'); - }); - test('hides unknown future tools by default and forwards non-tool methods', () => { const tool = jest.fn(); const diagnostic = jest.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/packages/mcp-server/src/tests/core/wallet/index.test.ts b/packages/mcp-server/src/tests/core/wallet/index.test.ts index 9f47f87d..8ef39063 100644 --- a/packages/mcp-server/src/tests/core/wallet/index.test.ts +++ b/packages/mcp-server/src/tests/core/wallet/index.test.ts @@ -9,8 +9,9 @@ jest.mock('../../../core/config.js', () => { const config = { privateKey: undefined as string | undefined, walletMode: 'disabled', walletApiKey: undefined }; return { config, + getRuntimeConfig: jest.fn(() => config), getScopedAppConfig: jest.fn(), - snapshotConfig: jest.fn(() => Object.freeze({ ...config })) + snapshotConfig: jest.fn((source = config) => Object.freeze({ ...source })) }; }); @@ -22,7 +23,7 @@ jest.mock('../../../core/wallet/providers/disabled.js', () => ({ DisabledWalletProvider: jest.fn() })); -import { getScopedAppConfig, config as processConfig, snapshotConfig } from '../../../core/config.js'; +import { getRuntimeConfig, getScopedAppConfig, config as processConfig, snapshotConfig } from '../../../core/config.js'; describe('Wallet Provider', () => { const mockPrivateKeyProvider: WalletProvider = { @@ -42,15 +43,13 @@ describe('Wallet Provider', () => { }; beforeEach(() => { - // Reset wallet provider instance before each test - resetWalletProvider(); - // Reset all mocks jest.resetAllMocks(); Object.assign(processConfig, { privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); + (getRuntimeConfig as jest.Mock).mockReturnValue(processConfig); (getScopedAppConfig as jest.Mock).mockReturnValue(undefined); - (snapshotConfig as jest.Mock).mockImplementation(() => Object.freeze({ ...processConfig })); + (snapshotConfig as jest.Mock).mockImplementation((source = processConfig) => Object.freeze({ ...source })); // Setup default mock implementations (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); @@ -74,21 +73,17 @@ describe('Wallet Provider', () => { expect(provider).toBe(mockDisabledProvider); }); - test('should return cached provider on subsequent calls', () => { + test('should not memoize providers outside a runtime scope', () => { Object.assign(processConfig, { privateKey: '0xabc', walletMode: 'private-key' }); + const provider2: WalletProvider = { ...mockPrivateKeyProvider }; + (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementationOnce(() => mockPrivateKeyProvider).mockImplementationOnce(() => provider2); - // First call const provider1 = getWalletProvider(); + const nextProvider = getWalletProvider(); - // Reset mocks to verify they aren't called again - jest.clearAllMocks(); - - // Second call should return cached provider - const provider2 = getWalletProvider(); - - expect(snapshotConfig).not.toHaveBeenCalled(); - expect(PrivateKeyWalletProvider).not.toHaveBeenCalled(); - expect(provider2).toBe(provider1); + expect(PrivateKeyWalletProvider).toHaveBeenCalledTimes(2); + expect(nextProvider).toBe(provider2); + expect(nextProvider).not.toBe(provider1); }); test('should throw error for unknown wallet mode', () => { @@ -97,18 +92,16 @@ describe('Wallet Provider', () => { expect(() => getWalletProvider()).toThrow('Unknown wallet mode: unknown-mode'); }); - test('keeps a snapshot provider after resetWalletProvider clears the process memo', () => { - const snapshot = { privateKey: '0xabc', walletMode: 'private-key' as const, walletApiKey: undefined }; + test('returns the cached provider for a runtime snapshot', () => { + const snapshot = snapshotConfig({ privateKey: '0xabc', walletMode: 'private-key', walletApiKey: undefined }); (getScopedAppConfig as jest.Mock).mockReturnValue(snapshot); const provider1 = getWalletProvider(); expect(PrivateKeyWalletProvider).toHaveBeenCalledWith({ privateKey: '0xabc' }); expect(provider1).toBe(mockPrivateKeyProvider); - resetWalletProvider(); jest.clearAllMocks(); (getScopedAppConfig as jest.Mock).mockReturnValue(snapshot); - (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementation(() => mockPrivateKeyProvider); const provider2 = getWalletProvider(); expect(provider2).toBe(provider1); @@ -116,7 +109,7 @@ describe('Wallet Provider', () => { }); test('recreates only the provider for a reset snapshot', () => { - const snapshot = { privateKey: '0xabc', walletMode: 'private-key' as const, walletApiKey: undefined }; + const snapshot = snapshotConfig({ privateKey: '0xabc', walletMode: 'private-key', walletApiKey: undefined }); const providerAfterReset = { ...mockPrivateKeyProvider }; (getScopedAppConfig as jest.Mock).mockReturnValue(snapshot); (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementationOnce(() => mockPrivateKeyProvider).mockImplementationOnce(() => providerAfterReset); @@ -127,44 +120,4 @@ describe('Wallet Provider', () => { expect(PrivateKeyWalletProvider).toHaveBeenCalledTimes(2); }); }); - - describe('resetWalletProvider', () => { - test('should reset the wallet provider instance', () => { - Object.assign(processConfig, { privateKey: '0xabc', walletMode: 'private-key' }); - - // Create separate mock instances for each call - const mockProvider1: WalletProvider = { - getName: () => 'private-key', - isAvailable: () => true, - getAddress: jest.fn(), - signTransaction: jest.fn(), - getWalletClient: jest.fn() - }; - const mockProvider2: WalletProvider = { - getName: () => 'private-key', - isAvailable: () => true, - getAddress: jest.fn(), - signTransaction: jest.fn(), - getWalletClient: jest.fn() - }; - - // Mock constructor to return different instances on each call - (PrivateKeyWalletProvider as unknown as jest.Mock).mockImplementationOnce(() => mockProvider1).mockImplementationOnce(() => mockProvider2); - - // Create provider instance - const provider1 = getWalletProvider(); - expect(provider1).toBe(mockProvider1); - - // Reset the provider - resetWalletProvider(); - - // Create new provider instance - const provider2 = getWalletProvider(); - expect(provider2).toBe(mockProvider2); - - // Should have created a new instance - expect(PrivateKeyWalletProvider).toHaveBeenCalledTimes(2); - expect(provider2).not.toBe(provider1); - }); - }); }); diff --git a/packages/mcp-server/src/tests/server/server.test.ts b/packages/mcp-server/src/tests/server/server.test.ts index 0a87c6b3..0ef1c113 100644 --- a/packages/mcp-server/src/tests/server/server.test.ts +++ b/packages/mcp-server/src/tests/server/server.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, jest, test } from 'bun:test'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; -import type { AppConfigSnapshot } from '../../core/config.js'; +import { type AppConfigSnapshot, getScopedAppConfig, snapshotConfig } from '../../core/config.js'; // Mock all dependencies jest.mock('@modelcontextprotocol/sdk/server/mcp.js', () => ({ @@ -33,7 +33,7 @@ jest.mock('../../core/chains.js', () => ({ })); type GetServerFunction = (appConfig: AppConfigSnapshot) => Promise; -const APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); +const APP_CONFIG = snapshotConfig({ privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); describe('Server Module', () => { let getServer: GetServerFunction; @@ -124,6 +124,25 @@ describe('Server Module', () => { expect(consoleErrorSpy).toHaveBeenCalledWith('Supported networks:', 'sei, sei-testnet'); }); + it('binds tool, resource, and prompt callbacks to the runtime snapshot', async () => { + await getServer(APP_CONFIG); + const scopedServer = mockRegisterEVMTools.mock.calls[0][0] as McpServer; + const seenConfigs: unknown[] = []; + + for (const method of ['tool', 'resource', 'prompt'] as const) { + let registeredCallback: (() => void) | undefined; + mockServerInstance[method] = jest.fn((...args: unknown[]) => { + registeredCallback = args.at(-1) as () => void; + }); + (scopedServer[method] as (...args: unknown[]) => unknown)('name', 'description', {}, () => { + seenConfigs.push(getScopedAppConfig()); + }); + registeredCallback?.(); + } + + expect(seenConfigs).toEqual([APP_CONFIG, APP_CONFIG, APP_CONFIG]); + }); + it('should sanitize and propagate server initialization errors', async () => { const testError = new Error('Initialization failed'); mockGetPackageInfo.mockImplementation(() => { diff --git a/packages/mcp-server/src/tests/server/transport/factory.test.ts b/packages/mcp-server/src/tests/server/transport/factory.test.ts index 49a59351..b40866e5 100644 --- a/packages/mcp-server/src/tests/server/transport/factory.test.ts +++ b/packages/mcp-server/src/tests/server/transport/factory.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, jest, test } from 'bun:test'; +import { snapshotConfig } from '../../../core/config.js'; import type { TransportConfig } from '../../../server/transport/types.js'; // Mock transport classes @@ -14,8 +15,8 @@ jest.mock('../../../server/transport/http-sse.js', () => ({ HttpSseTransport: jest.fn() })); -const DISABLED_APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); -const PRIVATE_KEY_APP_CONFIG = Object.freeze({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined }); +const DISABLED_APP_CONFIG = snapshotConfig({ privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); +const PRIVATE_KEY_APP_CONFIG = snapshotConfig({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key', walletApiKey: undefined }); describe('Transport Factory', () => { let createTransport: any; @@ -112,7 +113,7 @@ describe('Transport Factory', () => { }); it('forwards a frozen appConfig snapshot to stdio transport', () => { - const appConfig = Object.freeze({ + const appConfig = snapshotConfig({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined @@ -132,7 +133,7 @@ describe('Transport Factory', () => { }); it('forwards a frozen appConfig snapshot to HTTP transports', () => { - const appConfig = Object.freeze({ + const appConfig = snapshotConfig({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined diff --git a/packages/mcp-server/src/tests/server/transport/http-sse.test.ts b/packages/mcp-server/src/tests/server/transport/http-sse.test.ts index 211d438d..9c979bc0 100644 --- a/packages/mcp-server/src/tests/server/transport/http-sse.test.ts +++ b/packages/mcp-server/src/tests/server/transport/http-sse.test.ts @@ -6,12 +6,13 @@ import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js'; import type { Response } from 'express'; +import { snapshotConfig } from '../../../core/config.js'; import { HttpSseTransport } from '../../../server/transport/http-sse.js'; const HOST = '127.0.0.1'; const PATH = '/mcp'; -const DISABLED_APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); -const PRIVATE_KEY_APP_CONFIG = Object.freeze({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined }); +const DISABLED_APP_CONFIG = snapshotConfig({ privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); +const PRIVATE_KEY_APP_CONFIG = snapshotConfig({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key', walletApiKey: undefined }); function textOf(result: Awaited>): string { if (!('content' in result)) throw new Error('Expected an immediate tool result'); diff --git a/packages/mcp-server/src/tests/server/transport/stdio.test.ts b/packages/mcp-server/src/tests/server/transport/stdio.test.ts index 932255cf..6608b266 100644 --- a/packages/mcp-server/src/tests/server/transport/stdio.test.ts +++ b/packages/mcp-server/src/tests/server/transport/stdio.test.ts @@ -1,11 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, jest, test } from 'bun:test'; +import { snapshotConfig } from '../../../core/config.js'; // Mock dependencies jest.mock('@modelcontextprotocol/sdk/server/stdio.js', () => ({ StdioServerTransport: jest.fn() })); -const APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); +const APP_CONFIG = snapshotConfig({ privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); describe('StdioTransport', () => { let StdioTransport: any; diff --git a/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts b/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts index e282aa3c..aed28f17 100644 --- a/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts +++ b/packages/mcp-server/src/tests/server/transport/streamable-http.test.ts @@ -5,12 +5,13 @@ import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import type { Request, Response } from 'express'; +import { snapshotConfig } from '../../../core/config.js'; import { StreamableHttpTransport } from '../../../server/transport/streamable-http.js'; const HOST = '127.0.0.1'; const PATH = '/mcp'; -const DISABLED_APP_CONFIG = Object.freeze({ privateKey: undefined, walletMode: 'disabled' as const, walletApiKey: undefined }); -const PRIVATE_KEY_APP_CONFIG = Object.freeze({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key' as const, walletApiKey: undefined }); +const DISABLED_APP_CONFIG = snapshotConfig({ privateKey: undefined, walletMode: 'disabled', walletApiKey: undefined }); +const PRIVATE_KEY_APP_CONFIG = snapshotConfig({ privateKey: `0x${'1'.repeat(64)}`, walletMode: 'private-key', walletApiKey: undefined }); async function listenOnRandomPort(server: Server): Promise { await new Promise((resolve, reject) => { diff --git a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts index c81628d3..bd7dbf49 100644 --- a/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts +++ b/packages/mcp-server/src/tests/server/transport/wallet-isolation.test.ts @@ -4,7 +4,7 @@ import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { privateKeyToAccount } from 'viem/accounts'; -import { config, initializeConfig, isWalletEnabled, runWithAppConfig, snapshotConfig } from '../../../core/config.js'; +import { type AppConfigSnapshot, config, initializeConfig, isWalletEnabled, runWithAppConfig, snapshotConfig } from '../../../core/config.js'; import { getWalletProvider, resetWalletProvider } from '../../../core/wallet/index.js'; import { getServer } from '../../../server/server.js'; import { HttpSseTransport } from '../../../server/transport/http-sse.js'; @@ -13,6 +13,8 @@ import { StreamableHttpTransport } from '../../../server/transport/streamable-ht const HOST = '127.0.0.1'; const PATH = '/mcp'; const PRIVATE_KEY = '1'.repeat(64); +// Intentionally mirrors the EVM read-only policy plus search_docs without +// importing its implementation, so additions require explicit review here. const EXPECTED_HTTP_TOOL_NAMES = [ 'check_nft_ownership', 'estimate_gas', @@ -68,7 +70,6 @@ describe('HTTP wallet isolation across later starts', () => { await Promise.allSettled(transports.splice(0).map((transport) => transport.stop())); consoleErrorSpy?.mockRestore(); Object.assign(config, originalConfig); - resetWalletProvider(); delete process.env.WALLET_MODE; delete process.env.PRIVATE_KEY; }); @@ -107,8 +108,8 @@ describe('HTTP wallet isolation across later starts', () => { const appConfig = { privateKey: undefined as string | undefined, walletMode: 'disabled' as 'disabled' | 'private-key', walletApiKey: undefined }; const transport = mode === 'http-sse' - ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig }) - : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, appConfig }); + ? new HttpSseTransport({ port: 0, host: HOST, path: PATH, appConfig: appConfig as unknown as AppConfigSnapshot }) + : new StreamableHttpTransport({ port: 0, host: HOST, path: PATH, appConfig: appConfig as unknown as AppConfigSnapshot }); transports.push(transport); await transport.start(); @@ -171,7 +172,6 @@ describe('stdio wallet isolation after another runtime stop', () => { afterEach(() => { consoleErrorSpy?.mockRestore(); Object.assign(config, originalConfig); - resetWalletProvider(); delete process.env.WALLET_MODE; delete process.env.PRIVATE_KEY; }); From 1cb693bc04722669ff8ed00c48c54f1269c7bffc Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Tue, 15 Sep 2026 10:52:30 +0200 Subject: [PATCH 10/10] refactor(mcp-server): close remaining scope fallbacks Co-authored-by: Cursor --- packages/mcp-server/src/core/config.ts | 5 +++-- packages/mcp-server/src/core/wallet/index.ts | 2 ++ packages/mcp-server/src/server/server.ts | 2 +- packages/mcp-server/src/server/transport/http-sse.ts | 2 +- .../mcp-server/src/server/transport/streamable-http.ts | 2 +- packages/mcp-server/src/tests/core/config.test.ts | 8 ++++++-- packages/mcp-server/src/tests/server/server.test.ts | 6 +++--- 7 files changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/mcp-server/src/core/config.ts b/packages/mcp-server/src/core/config.ts index d99aa6f5..5796f0df 100644 --- a/packages/mcp-server/src/core/config.ts +++ b/packages/mcp-server/src/core/config.ts @@ -91,8 +91,9 @@ export function getRuntimeConfig(): Readonly { const scopedConfig = getScopedAppConfig(); if (scopedConfig) return scopedConfig; // Direct helpers intentionally retain the process fallback. Warn once per - // initialization so a lost runtime scope is visible without flooding stderr. - if (config.walletMode !== 'disabled' && !warnedAboutUnscopedWalletRead) { + // initialization in either wallet direction so a lost runtime scope is + // visible without flooding stderr. + if (!warnedAboutUnscopedWalletRead) { warnedAboutUnscopedWalletRead = true; console.error('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); } diff --git a/packages/mcp-server/src/core/wallet/index.ts b/packages/mcp-server/src/core/wallet/index.ts index 622f2808..d59b7282 100644 --- a/packages/mcp-server/src/core/wallet/index.ts +++ b/packages/mcp-server/src/core/wallet/index.ts @@ -33,6 +33,8 @@ export function getWalletProvider(): WalletProvider { return providerForSnapshot(runtime); } + // Unscoped callers get a fresh provider so mutable process config cannot + // leave a stale signer memoized across initializeConfig() calls. return createWalletProvider(snapshotConfig(getRuntimeConfig())); } diff --git a/packages/mcp-server/src/server/server.ts b/packages/mcp-server/src/server/server.ts index ddc2b378..2f42e26e 100644 --- a/packages/mcp-server/src/server/server.ts +++ b/packages/mcp-server/src/server/server.ts @@ -8,7 +8,7 @@ import { registerEVMTools } from '../core/tools.js'; import { createDocsSearchTool } from '../docs/index.js'; import { getPackageInfo } from './package-info.js'; -const SCOPED_REGISTRATION_METHODS = new Set(['tool', 'resource', 'prompt']); +const SCOPED_REGISTRATION_METHODS = new Set(['tool', 'resource', 'prompt', 'registerTool', 'registerResource', 'registerPrompt']); function withScopedCallbacks(server: McpServer, appConfig: AppConfigSnapshot): McpServer { return new Proxy(server, { diff --git a/packages/mcp-server/src/server/transport/http-sse.ts b/packages/mcp-server/src/server/transport/http-sse.ts index 786d9282..343f38ba 100644 --- a/packages/mcp-server/src/server/transport/http-sse.ts +++ b/packages/mcp-server/src/server/transport/http-sse.ts @@ -151,7 +151,7 @@ export class HttpSseTransport implements McpTransport { } this.connections.set(sessionId, session); - await server.connect(transport); + await runWithAppConfig(this.appConfig, () => server.connect(transport)); if (this.state !== 'running' || disconnected) { await this.closeSession(sessionId); diff --git a/packages/mcp-server/src/server/transport/streamable-http.ts b/packages/mcp-server/src/server/transport/streamable-http.ts index e7ef916b..bb012a1f 100644 --- a/packages/mcp-server/src/server/transport/streamable-http.ts +++ b/packages/mcp-server/src/server/transport/streamable-http.ts @@ -188,7 +188,7 @@ export class StreamableHttpTransport implements McpTransport { return; } - await server.connect(transport); + await runWithAppConfig(this.appConfig, () => server.connect(transport)); await runWithAppConfig(this.appConfig, () => transport.handleRequest(req, res, req.body)); } catch (error) { console.error('Error handling MCP request:', sanitizeError(error)); diff --git a/packages/mcp-server/src/tests/core/config.test.ts b/packages/mcp-server/src/tests/core/config.test.ts index 31de0968..1579fdb4 100644 --- a/packages/mcp-server/src/tests/core/config.test.ts +++ b/packages/mcp-server/src/tests/core/config.test.ts @@ -147,14 +147,18 @@ describe('Config Module - Actual Implementation', () => { consoleErrorSpy.mockRestore(); }); - test('warns once per initialization for enabled-wallet reads outside a runtime scope', () => { + test('warns once per initialization for reads outside a runtime scope', () => { const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); initializeConfig({ WALLET_MODE: 'private-key', PRIVATE_KEY: '5'.repeat(64) }); expect(getWalletMode()).toBe('private-key'); expect(isWalletEnabled()).toBe(true); - expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + + initializeConfig({ WALLET_MODE: 'disabled' }); + expect(getWalletMode()).toBe('disabled'); + expect(isWalletEnabled()).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledTimes(2); expect(consoleErrorSpy).toHaveBeenCalledWith('Wallet configuration was read outside an MCP runtime scope; using the mutable process configuration.'); consoleErrorSpy.mockRestore(); }); diff --git a/packages/mcp-server/src/tests/server/server.test.ts b/packages/mcp-server/src/tests/server/server.test.ts index 0ef1c113..e8fb2653 100644 --- a/packages/mcp-server/src/tests/server/server.test.ts +++ b/packages/mcp-server/src/tests/server/server.test.ts @@ -124,12 +124,12 @@ describe('Server Module', () => { expect(consoleErrorSpy).toHaveBeenCalledWith('Supported networks:', 'sei, sei-testnet'); }); - it('binds tool, resource, and prompt callbacks to the runtime snapshot', async () => { + it('binds legacy and current registration callbacks to the runtime snapshot', async () => { await getServer(APP_CONFIG); const scopedServer = mockRegisterEVMTools.mock.calls[0][0] as McpServer; const seenConfigs: unknown[] = []; - for (const method of ['tool', 'resource', 'prompt'] as const) { + for (const method of ['tool', 'resource', 'prompt', 'registerTool', 'registerResource', 'registerPrompt'] as const) { let registeredCallback: (() => void) | undefined; mockServerInstance[method] = jest.fn((...args: unknown[]) => { registeredCallback = args.at(-1) as () => void; @@ -140,7 +140,7 @@ describe('Server Module', () => { registeredCallback?.(); } - expect(seenConfigs).toEqual([APP_CONFIG, APP_CONFIG, APP_CONFIG]); + expect(seenConfigs).toEqual([APP_CONFIG, APP_CONFIG, APP_CONFIG, APP_CONFIG, APP_CONFIG, APP_CONFIG]); }); it('should sanitize and propagate server initialization errors', async () => {