From 185fae2d78e058cb77aad8e3f68488f4634caef3 Mon Sep 17 00:00:00 2001 From: Daliys Date: Wed, 8 Jul 2026 01:46:58 +0200 Subject: [PATCH] Harden CLI installer command arguments --- CHANGELOG.md | 1 + Editor/MCPCliInstaller.ClaudeCode.cs | 6 +- Editor/MCPCliInstaller.Codex.cs | 8 +-- Editor/MCPCliInstaller.Gemini.cs | 6 +- Editor/MCPCliInstaller.cs | 74 +++++++++++++++++++++-- Tests~/Editor/MCPCliInstallerTests.cs | 87 +++++++++++++++++++++++++++ 6 files changed, 164 insertions(+), 18 deletions(-) create mode 100644 Tests~/Editor/MCPCliInstallerTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 13b9764..521b301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ All notable public changes to Nexus Unity are documented here. - `get_test_results` now reads messages only from scoped NUnit result nodes and falls back to the test result instead of unrelated nested messages. - `unity_scene_manager` schema aliases now preserve action-specific required parameters. - Fast-path health JSON-RPC methods now use cached editor state instead of direct Unity API reads on the listener thread. +- MCP CLI installers now pass executable paths and project paths as process arguments instead of shell command strings. ## [1.4.2] - 2026-06-13 diff --git a/Editor/MCPCliInstaller.ClaudeCode.cs b/Editor/MCPCliInstaller.ClaudeCode.cs index eab7a03..76bf702 100644 --- a/Editor/MCPCliInstaller.ClaudeCode.cs +++ b/Editor/MCPCliInstaller.ClaudeCode.cs @@ -36,12 +36,10 @@ private static void ExecuteClaudeCodeLinkSequence(string scriptPath, string pyth if (!string.IsNullOrEmpty(claudePath) && claudePath != "claude") { // 1. Remove any stale registration so re-running is idempotent (silent). - string removeCommand = "\"" + claudePath + "\" mcp remove nexus-unity -s project"; - RunInstallerProcess(CreateProcessStartInfo(removeCommand), claudePath, false, "Claude Code"); + RunInstallerProcess(CreateProcessStartInfo(claudePath, "mcp", "remove", "nexus-unity", "-s", "project"), claudePath, false, "Claude Code"); // 2. Add the server at project scope (silent, because we have a file fallback). - string addCommand = "\"" + claudePath + "\" mcp add nexus-unity -s project -e " + MCPServer.AuthTokenEnvironmentVariable + "=" + MCPServer.AuthToken + " -- \"" + pythonPath + "\" \"" + scriptPath + "\""; - if (RunInstallerProcess(CreateProcessStartInfo(addCommand), claudePath, false, "Claude Code")) + if (RunInstallerProcess(CreateProcessStartInfo(claudePath, "mcp", "add", "nexus-unity", "-s", "project", "-e", MCPServer.AuthTokenEnvironmentVariable + "=" + MCPServer.AuthToken, "--", pythonPath, scriptPath), claudePath, false, "Claude Code")) { NexusEditorLog.Log(NexusLogCategory.Integrations, "[MCP] Successfully linked Nexus Unity to Claude Code via '" + claudePath + "'.", true); EditorUtility.DisplayDialog("MCP Success", "Successfully linked Nexus Unity to Claude Code.\n\nRun /mcp inside Claude Code (or restart it) to load the server.", "OK"); diff --git a/Editor/MCPCliInstaller.Codex.cs b/Editor/MCPCliInstaller.Codex.cs index 64ab839..709bdbd 100644 --- a/Editor/MCPCliInstaller.Codex.cs +++ b/Editor/MCPCliInstaller.Codex.cs @@ -1,7 +1,6 @@ using UnityEditor; using UnityEngine; using System.IO; -using System.Diagnostics; using System.Linq; using System; @@ -33,14 +32,11 @@ private static void ExecuteCodexLinkSequence(string scriptPath, string pythonPat if (!string.IsNullOrEmpty(codexPath) && codexPath != "codex") { // 1. Remove existing to ensure clean slate (silent) - string removeCommand = "\"" + codexPath + "\" mcp remove nexus-unity"; - RunInstallerProcess(CreateProcessStartInfo(removeCommand), codexPath, false, "Codex"); + RunInstallerProcess(CreateProcessStartInfo(codexPath, "mcp", "remove", "nexus-unity"), codexPath, false, "Codex"); // 2. Add new with command/args (silent, because we have a fallback) - string addCommand = "\"" + codexPath + "\" mcp add nexus-unity --env " + MCPServer.AuthTokenEnvironmentVariable + "=" + MCPServer.AuthToken + " -- \"" + pythonPath + "\" \"" + scriptPath + "\""; - // If it succeeds, we are done - if (RunInstallerProcess(CreateProcessStartInfo(addCommand), codexPath, false, "Codex")) + if (RunInstallerProcess(CreateProcessStartInfo(codexPath, "mcp", "add", "nexus-unity", "--env", MCPServer.AuthTokenEnvironmentVariable + "=" + MCPServer.AuthToken, "--", pythonPath, scriptPath), codexPath, false, "Codex")) { NexusEditorLog.Log(NexusLogCategory.Integrations, "[MCP] Successfully linked Nexus Unity to Codex CLI via '" + codexPath + "'", true); EditorUtility.DisplayDialog("MCP Success", "Successfully linked Nexus Unity to your system Codex CLI!", "OK"); diff --git a/Editor/MCPCliInstaller.Gemini.cs b/Editor/MCPCliInstaller.Gemini.cs index 735e4c5..97f5048 100644 --- a/Editor/MCPCliInstaller.Gemini.cs +++ b/Editor/MCPCliInstaller.Gemini.cs @@ -30,12 +30,10 @@ private static void ExecuteGeminiLinkSequence(string scriptPath) string pythonPath = ResolvePythonPath(); // 1. Ensure clean slate by removing existing registration - string removeCommand = "\"" + geminiPath + "\" mcp remove nexus-unity"; - RunInstallerProcess(CreateProcessStartInfo(removeCommand), geminiPath, false, "Gemini"); + RunInstallerProcess(CreateProcessStartInfo(geminiPath, "mcp", "remove", "nexus-unity"), geminiPath, false, "Gemini"); // 2. Add new registration with stable path - string addCommand = "\"" + geminiPath + "\" mcp add nexus-unity --trust -e " + MCPServer.AuthTokenEnvironmentVariable + "=" + MCPServer.AuthToken + " \"" + pythonPath + "\" \"" + scriptPath + "\""; - RunInstallerProcess(CreateProcessStartInfo(addCommand), geminiPath, true, "Gemini"); + RunInstallerProcess(CreateProcessStartInfo(geminiPath, "mcp", "add", "nexus-unity", "--trust", "-e", MCPServer.AuthTokenEnvironmentVariable + "=" + MCPServer.AuthToken, pythonPath, scriptPath), geminiPath, true, "Gemini"); } } } diff --git a/Editor/MCPCliInstaller.cs b/Editor/MCPCliInstaller.cs index a248bc4..2b1133f 100644 --- a/Editor/MCPCliInstaller.cs +++ b/Editor/MCPCliInstaller.cs @@ -2,8 +2,8 @@ using UnityEngine; using System.IO; using System.Diagnostics; -using System.Collections.Generic; using System; +using System.Text; namespace UnityMCP.Editor { @@ -306,13 +306,15 @@ private static string ResolvePythonPath() return "python3"; } - private static ProcessStartInfo CreateProcessStartInfo(string command) + private static ProcessStartInfo CreateProcessStartInfo(string executable, params string[] arguments) { bool isWindows = Application.platform == RuntimePlatform.WindowsEditor; + string extension = Path.GetExtension(executable); + bool useCmdShim = isWindows && (string.Equals(extension, ".cmd", StringComparison.OrdinalIgnoreCase) || string.Equals(extension, ".bat", StringComparison.OrdinalIgnoreCase)); ProcessStartInfo psi = new ProcessStartInfo { - FileName = isWindows ? "cmd.exe" : "/bin/bash", - Arguments = isWindows ? ("/c \"" + command + "\"") : ("-c \"" + command + "\""), + FileName = useCmdShim ? "cmd.exe" : executable, + Arguments = useCmdShim ? BuildWindowsBatchArguments(executable, arguments) : BuildProcessArguments(arguments), RedirectStandardError = true, UseShellExecute = false, CreateNoWindow = true @@ -329,6 +331,70 @@ private static ProcessStartInfo CreateProcessStartInfo(string command) return psi; } + private static string BuildProcessArguments(string[] arguments) + { + return string.Join(" ", Array.ConvertAll(arguments, QuoteWindowsArgument)); + } + + private static string BuildWindowsBatchArguments(string executable, string[] arguments) + { + string command = QuoteCmdArgument(executable); + foreach (string argument in arguments) + { + command += " " + QuoteCmdArgument(argument); + } + return "/d /v:off /s /c " + QuoteWindowsArgument(command); + } + + private static string QuoteCmdArgument(string argument) + { + return QuoteWindowsArgument(EscapeCmdMetacharacters(argument)); + } + + private static string EscapeCmdMetacharacters(string value) + { + if (string.IsNullOrEmpty(value)) return value; + return value + .Replace("^", "^^") + .Replace("&", "^&") + .Replace("|", "^|") + .Replace("<", "^<") + .Replace(">", "^>") + .Replace("%", "^%"); + } + + private static string QuoteWindowsArgument(string argument) + { + if (string.IsNullOrEmpty(argument)) return "\"\""; + if (argument.IndexOfAny(new[] { ' ', '\t', '\n', '\r', '"' }) < 0) return argument; + + StringBuilder result = new StringBuilder(); + result.Append('"'); + int backslashes = 0; + foreach (char c in argument) + { + if (c == '\\') + { + backslashes++; + continue; + } + + if (c == '"') + { + result.Append('\\', backslashes * 2 + 1); + result.Append('"'); + backslashes = 0; + continue; + } + result.Append('\\', backslashes); + result.Append(c); + backslashes = 0; + } + result.Append('\\', backslashes * 2); + result.Append('"'); + return result.ToString(); + } + private static bool RunInstallerProcess(ProcessStartInfo psi, string cliPath, bool showSuccessDialog, string cliName) { try diff --git a/Tests~/Editor/MCPCliInstallerTests.cs b/Tests~/Editor/MCPCliInstallerTests.cs new file mode 100644 index 0000000..56cd509 --- /dev/null +++ b/Tests~/Editor/MCPCliInstallerTests.cs @@ -0,0 +1,87 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Reflection; +using NUnit.Framework; + +namespace UnityMCP.Editor.Tests +{ + public class MCPCliInstallerTests + { + [Test] + public void CreateProcessStartInfoPreservesShellMetacharactersAsArguments() + { + var method = typeof(MCPCliInstaller).GetMethod( + "CreateProcessStartInfo", + BindingFlags.NonPublic | BindingFlags.Static, + null, + new[] { typeof(string), typeof(string[]) }, + null); + + var args = new[] { "mcp", "add", "nexus-unity", "--env", "TOKEN=a;$(touch bad)", "--", "/tmp/py thon", "/tmp/proj\";echo bad/nexus_unity_bridge.py" }; + var psi = (ProcessStartInfo)method.Invoke(null, new object[] { "/tmp/cli;echo bad", args }); + + Assert.AreEqual("/tmp/cli;echo bad", psi.FileName); + Assert.IsTrue(psi.Arguments.Contains("TOKEN=a;$(touch bad)")); + Assert.IsTrue(psi.Arguments.Contains("/tmp/proj\";echo bad/nexus_unity_bridge.py")); + Assert.IsFalse(psi.Arguments.Contains(" -c ")); + Assert.IsFalse(psi.UseShellExecute); + } + + [Test] + public void WindowsBatchArgumentsEscapeCmdMetacharacters() + { + var method = typeof(MCPCliInstaller).GetMethod( + "BuildWindowsBatchArguments", + BindingFlags.NonPublic | BindingFlags.Static, + null, + new[] { typeof(string), typeof(string[]) }, + null); + + var args = new[] { "mcp", "add", "nexus-unity", "a&b|c%PATH%" }; + string command = (string)method.Invoke(null, new object[] { @"C:\Tools\gemini.cmd", args }); + + Assert.IsTrue(command.Contains(@"C:\Tools\gemini.cmd")); + Assert.IsTrue(command.Contains("a^&b^|c^%PATH^%")); + Assert.IsFalse(command.Contains(" a&b|c%PATH%")); + } + + [Test] + public void CreateProcessStartInfoPassesMetacharactersAsLiteralArgv() + { + if (!File.Exists("/bin/sh")) Assert.Ignore("Requires a Unix shell for the argv smoke test."); + + string root = Path.Combine(Path.GetTempPath(), "NexusCliInstallerTests_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + string script = Path.Combine(root, "argv.sh"); + string output = Path.Combine(root, "argv.txt"); + + try + { + File.WriteAllText(script, "#!/bin/sh\nout=\"$1\"\nshift\nprintf '%s\\n' \"$@\" > \"$out\"\n"); + var method = typeof(MCPCliInstaller).GetMethod( + "CreateProcessStartInfo", + BindingFlags.NonPublic | BindingFlags.Static, + null, + new[] { typeof(string), typeof(string[]) }, + null); + string[] args = { script, output, "TOKEN=a;$(touch bad)", "/tmp/py thon", "/tmp/proj\";echo bad" }; + var psi = (ProcessStartInfo)method.Invoke(null, new object[] { "/bin/sh", args }); + + using (Process process = Process.Start(psi)) + { + string error = process.StandardError.ReadToEnd(); + process.WaitForExit(); + Assert.AreEqual(0, process.ExitCode, error); + } + + CollectionAssert.AreEqual(new[] { "TOKEN=a;$(touch bad)", "/tmp/py thon", "/tmp/proj\";echo bad" }, File.ReadAllLines(output)); + Assert.IsFalse(File.Exists(Path.Combine(root, "bad"))); + } + finally + { + if (Directory.Exists(root)) Directory.Delete(root, true); + } + } + } +}