Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/__tests__/commands/install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ describe("handleInstall — happy path (GREEN trust score)", () => {
expect(adapter.addServer).toHaveBeenCalledWith(
"/fake/claude-desktop/config.json",
"io.github.test/my-server",
expect.objectContaining({ command: "npx" })
expect.objectContaining({ command: "npx" }),
expect.objectContaining({})
);
});

Expand Down Expand Up @@ -506,7 +507,8 @@ describe("handleInstall — env var prompting", () => {
expect(adapter.addServer).toHaveBeenCalledWith(
expect.any(String),
expect.any(String),
expect.objectContaining({ env: { API_KEY: "secret-value" } })
expect.objectContaining({ env: { API_KEY: "secret-value" } }),
expect.objectContaining({})
);
});

Expand Down Expand Up @@ -1343,7 +1345,8 @@ describe("handleInstall — no env vars on server", () => {
expect(adapter.addServer).toHaveBeenCalledWith(
expect.any(String),
expect.any(String),
expect.not.objectContaining({ env: expect.anything() })
expect.not.objectContaining({ env: expect.anything() }),
expect.objectContaining({})
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/commands/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ export async function handleInstall(
: {}),
};

await adapter.addServer(configPath, name, entry);
await adapter.addServer(configPath, name, entry, { force: options.force });
installedClients.push(clientId);
}

Expand Down
5 changes: 3 additions & 2 deletions src/config/adapters/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,16 @@ export abstract class BaseAdapter implements ConfigAdapter {
async addServer(
configPath: string,
name: string,
entry: McpServerEntry
entry: McpServerEntry,
options?: { force?: boolean }
): Promise<void> {
// readRaw returns {} for ENOENT and re-throws all other errors,
// so we can call it directly here.
const raw = await this.readRaw(configPath);

const existing = (raw[this.rootKey] ?? {}) as Record<string, McpServerEntry>;

if (Object.prototype.hasOwnProperty.call(existing, name)) {
if (Object.prototype.hasOwnProperty.call(existing, name) && !options?.force) {
throw new Error(
`Server "${name}" already exists in ${this.clientId} config. Use --force to overwrite.`
);
Expand Down
6 changes: 4 additions & 2 deletions src/config/adapters/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ export interface ConfigAdapter {
read(configPath: string): Promise<Record<string, McpServerEntry>>;

/**
* Add a new server entry. Throws if the server name already exists.
* Add a new server entry. Throws if the server name already exists
* (unless options.force is true, in which case it overwrites).
* Creates the file (with correct structure) if it does not exist.
*/
addServer(
configPath: string,
name: string,
entry: McpServerEntry
entry: McpServerEntry,
options?: { force?: boolean }
): Promise<void>;

/** Remove a server entry. Throws if the server name is not found. */
Expand Down
84 changes: 83 additions & 1 deletion src/scanner/health-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,88 @@
import { spawn } from "node:child_process";
import type { McpServerEntry } from "../config/adapters/index.js";

// ---------------------------------------------------------------------------
// Environment sanitization for health checks
// ---------------------------------------------------------------------------

/**
* Environment variables that should NOT be passed to untrusted MCP servers
* during health checks. The health check only needs the server to start and
* respond to tools/list — it does not need access to the user's secrets.
*
* Pattern: known secret variable names from major cloud providers, AI APIs,
* CI systems, and databases. Uses exact matches (case-insensitive compare).
*/
const SENSITIVE_ENV_PREFIXES: readonly string[] = [
"AWS_SECRET",
"AWS_SESSION",
"AZURE_CLIENT_SECRET",
"AZURE_TENANT",
"GCP_SERVICE_ACCOUNT",
"GOOGLE_APPLICATION_CREDENTIALS",
];

const SENSITIVE_ENV_NAMES: ReadonlySet<string> = new Set([
// Cloud provider secrets
"AWS_SECRET_ACCESS_KEY",
"AWS_SESSION_TOKEN",
// AI API keys
"ANTHROPIC_API_KEY",
"OPENAI_API_KEY",
"GOOGLE_API_KEY",
// VCS tokens
"GITHUB_TOKEN",
"GH_TOKEN",
"GITLAB_TOKEN",
"BITBUCKET_TOKEN",
// Package registry tokens
"NPM_TOKEN",
"NODE_AUTH_TOKEN",
"PYPI_TOKEN",
// Database
"DATABASE_URL",
"DB_PASSWORD",
"PGPASSWORD",
"MYSQL_PWD",
"REDIS_PASSWORD",
// CI/CD
"CI_JOB_TOKEN",
"CIRCLE_TOKEN",
"TRAVIS_TOKEN",
// Generic
"SECRET_KEY",
"PRIVATE_KEY",
"ENCRYPTION_KEY",
"API_SECRET",
"AUTH_TOKEN",
]);

/**
* Build a sanitized environment for health check subprocesses.
* Strips known sensitive variables from process.env before merging
* the server's declared env vars (which may legitimately need API keys
* the user provided during install).
*/
function buildHealthCheckEnv(
serverEnv?: Record<string, string>,
extraEnv?: Record<string, string>
): Record<string, string> {
const base: Record<string, string> = {};

for (const [key, value] of Object.entries(process.env)) {
if (value === undefined) continue;
const upper = key.toUpperCase();
if (SENSITIVE_ENV_NAMES.has(upper)) continue;
if (SENSITIVE_ENV_PREFIXES.some((p) => upper.startsWith(p))) continue;
base[key] = value;
}

// Merge caller-provided env and server-declared env on top.
// Server env vars (e.g. API keys the user typed during install) are
// intentionally NOT filtered — the user explicitly provided them.
return { ...base, ...(extraEnv ?? {}), ...(serverEnv ?? {}) };
}

export interface HealthCheckResult {
tier: 1 | 2 | 3;
passed: boolean;
Expand Down Expand Up @@ -86,7 +168,7 @@ export async function runHealthCheck(

const input = `${initRequest}\n${initializedNotification}\n${toolsRequest}\n`;

const mergedEnv = { ...process.env, ...(env ?? {}), ...(entry.env ?? {}) };
const mergedEnv = buildHealthCheckEnv(entry.env, env);

return new Promise<HealthCheckResult>((resolve) => {
const child = spawn(command, [...args], {
Expand Down
4 changes: 2 additions & 2 deletions src/server/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ describe("handleRemove", () => {
};
const deps = makeDeps({ getAdapter: vi.fn().mockReturnValue(adapter) });

const result = await handleRemove({ name: "my-server" }, deps);
const result = await handleRemove({ name: "io.github.test/my-server" }, deps);
const r = result as { removed: boolean; clients: string[] };
expect(r.removed).toBe(true);
expect(r.clients).toContain("cursor");
Expand All @@ -210,7 +210,7 @@ describe("handleRemove", () => {
};
const deps = makeDeps({ getAdapter: vi.fn().mockReturnValue(adapter) });

await expect(handleRemove({ name: "nonexistent" }, deps)).rejects.toThrow(/not found/i);
await expect(handleRemove({ name: "io.github.test/nonexistent" }, deps)).rejects.toThrow(/not found/i);
});
});

Expand Down
53 changes: 52 additions & 1 deletion src/server/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,36 @@ import { extractRegistryMeta } from "../utils/format-trust.js";
import { formatMcpEntryCommand } from "../utils/format-entry.js";
import { resolveInstallEntry } from "../commands/install.js";

// ---------------------------------------------------------------------------
// Input validation for MCP server tool arguments
// ---------------------------------------------------------------------------

/**
* Server name pattern for MCP registry names.
* Format: "namespace/server-name" — alphanumeric with dots, hyphens, underscores.
* Max length 256 to prevent abuse. Must not contain shell metacharacters,
* path traversal sequences, or control characters.
*/
const SERVER_NAME_RE =
/^[a-zA-Z0-9][a-zA-Z0-9._-]{0,126}\/[a-zA-Z0-9][a-zA-Z0-9._-]{0,126}$/;

/**
* Validate a server name received from an MCP tool call.
* This is the trust boundary — AI agents provide these strings, and they
* could be influenced by prompt injection or adversarial inputs.
*/
function validateMcpServerName(name: string): void {
if (typeof name !== "string" || name.length === 0 || name.length > 256) {
throw new Error(`Invalid server name: must be a non-empty string under 256 characters.`);
}
if (!SERVER_NAME_RE.test(name)) {
throw new Error(
`Invalid server name format: "${name}". Expected format: "namespace/server-name" ` +
`(alphanumeric, dots, hyphens, underscores only).`
);
}
}

const execFileAsync = promisify(execFile);

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -87,13 +117,32 @@ export async function handleSearch(
return { servers };
}

/** Default minimum trust score for MCP server tool installs (no human in the loop). */
const DEFAULT_MIN_TRUST_SCORE = 50;

export async function handleInstall(
args: { name: string; client?: string },
args: { name: string; client?: string; minTrustScore?: number },
deps: ServerDeps,
preResolved?: { entry: ServerEntry; trust: TrustScore }
): Promise<object> {
if (!preResolved) validateMcpServerName(args.name);
const entry = preResolved?.entry ?? await deps.registryGetServer(args.name);
const trust = preResolved?.trust ?? computeTrust(entry, deps);

// Security gate: reject servers below the minimum trust score.
// Unlike the CLI path which has a human confirmation prompt, the MCP server
// path is driven by AI agents with no human in the loop. A malicious prompt
// could trick an agent into installing a dangerous server, so we enforce a
// hard trust floor here.
const minScore = args.minTrustScore ?? DEFAULT_MIN_TRUST_SCORE;
if (trust.score < minScore) {
throw new Error(
`Server "${args.name}" has trust score ${trust.score}/${trust.maxPossible} ` +
`(level: ${trust.level}), which is below the minimum threshold of ${minScore}. ` +
`Install rejected for safety. Use mcpm CLI with --yes to override after manual review.`
);
}

const clients = await resolveClients(args.client, deps);

const installedClients: ClientId[] = [];
Expand Down Expand Up @@ -125,6 +174,7 @@ export async function handleInfo(
args: { name: string },
deps: ServerDeps
): Promise<object> {
validateMcpServerName(args.name);
const entry = await deps.registryGetServer(args.name);
const trust = computeTrust(entry, deps);
return {
Expand Down Expand Up @@ -164,6 +214,7 @@ export async function handleRemove(
args: { name: string; client?: string },
deps: ServerDeps
): Promise<object> {
validateMcpServerName(args.name);
const clients = await resolveClients(args.client, deps);
const removedClients: ClientId[] = [];

Expand Down
4 changes: 3 additions & 1 deletion src/server/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ export const TOOL_DEFINITIONS = [
},
{
name: "mcpm_install",
description: "Install an MCP server from the registry into detected AI client configs. Runs trust assessment automatically.",
description: "Install an MCP server from the registry into detected AI client configs. Runs trust assessment automatically. Rejects servers below the minimum trust score (default 50).",
inputSchema: {
type: "object" as const,
properties: {
name: { type: "string", description: "Server name (e.g. io.github.domdomegg/filesystem-mcp)" },
client: { type: "string", description: "Install to specific client only (claude-desktop, cursor, vscode, windsurf)" },
minTrustScore: { type: "number", description: "Minimum trust score to allow install (default 50, range 0-100)" },
},
required: ["name"],
},
Expand Down Expand Up @@ -107,6 +108,7 @@ export const SearchInput = z.object({
export const InstallInput = z.object({
name: z.string(),
client: z.string().optional(),
minTrustScore: z.number().min(0).max(100).optional().default(50),
});

export const InfoInput = z.object({
Expand Down
Loading