no-command-injection-in-tool

CWE: CWE-78 OWASP: A03:2021 – Injection

Detects a tool handler's own parameter reaching exec, spawn, execFile or fork as the thing being run. This rule is part of eslint-plugin-mcp-sdk-security.

💼 This rule is set to error in the strict config.

Quick Summary

Why this matters

A tool handler's parameter is attacker-influenced by construction. It is filled from the model's tool call, and the model can be steered by any content it has read — a web page it fetched, a file it opened, another tool's output. Treating it as trusted is the MCP equivalent of trusting req.body, except the caller is a language model that an attacker may be writing the inputs for.

When that value names the command, whoever steers the model chooses what runs on the host.

Why this is not node-security/no-shell-injection

They split by shape, and the split is deliberate. no-shell-injection says so in its own header:

Does NOT fire on: exec(variable) — indirect; data-flow analysis required, out of scope

It reports exec(`git ${cmd}`) because the concatenation is visible in the expression, and stays silent on exec(cmd) because proving what cmd holds needs data-flow analysis it does not do.

Inside a tool handler that analysis is not needed — the taint source is the handler's own parameter, declared in the same expression:

server.registerTool('run', { inputSchema: { cmd: z.string() } },
  async ({ cmd }) => {
    execSync(cmd);           // ← this rule; nothing else reports it
    execSync(`ls ${cmd}`);   // ← node-security/no-shell-injection reports
  });

So this rule takes exactly the half its sibling declines. The concatenated shape is not reported here, which is what keeps one line from carrying a finding from two plugins.

❌ Incorrect

// ❌ destructured argument names the command
server.registerTool('run', cfg, async ({ cmd }) => { execSync(cmd); });

// ❌ same thing through the whole-args object
server.registerTool('run', cfg, async (args) => { execSync(args.cmd); });

// ❌ spawn picks the binary too — the argv array does not help here
server.registerTool('run', cfg, async ({ bin }) => { spawn(bin, argv); });

✅ Correct

// ✅ the argument selects an operation; the code names the binary
const ALLOWED = { list: 'ls', disk: 'df' } as const;

server.registerTool('run', cfg, async ({ op, target }) => {
  const binary = ALLOWED[op];
  if (!binary) throw new Error('unsupported operation');
  execFile(binary, [target]);   // user data is an argv element, never the command
});

Two things make that safe, and both are needed: the binary comes from a closed set the code owns, and the user-supplied value is passed as an argv array element rather than spliced into a command line. execFile with an array does not involve a shell, so there is no metacharacter to escape.

What this rule deliberately does not report

• The concatenated form. It belongs to node-security/no-shell-injection — see the table above.
• The whole args object. execSync(args) is not a command; only a member of it (args.cmd) is.
• A variable that is not a tool argument. execSync(configuredBinary) may well be unsafe, but this rule cannot show it came from the model, and guessing is what earns a security rule its false-positive reputation.
• A computed member. args[key] is not statically a name.
• A sink outside any tool handler. The handler is the taint boundary; a sink elsewhere in the file is node-security's question.
• A file that never imports @modelcontextprotocol/sdk.

When Not To Use It

There is no configuration in which letting a model-supplied value name the command is correct, so this rule has no options.

Further Reading

• CWE-78: OS Command Injection
• OWASP A03:2021 – Injection
• MCP: Tools
• Node.js: child_process.execFile