diff --git a/MCPForUnity/Editor/Tools/ExecuteCode.cs b/MCPForUnity/Editor/Tools/ExecuteCode.cs index bef08d49a..b8e2cd2a8 100644 --- a/MCPForUnity/Editor/Tools/ExecuteCode.cs +++ b/MCPForUnity/Editor/Tools/ExecuteCode.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Reflection; using System.Text; +using System.Text.RegularExpressions; using MCPForUnity.Editor.Helpers; using MCPForUnity.Runtime.Helpers; using Microsoft.CSharp; @@ -19,7 +20,6 @@ public static class ExecuteCode private const int MaxCodeLength = 50000; private const int MaxHistoryEntries = 50; private const int MaxHistoryCodePreview = 500; - internal const int WrapperLineOffset = 10; private const string WrapperClassName = "MCPDynamicCode"; private const string WrapperMethodName = "Execute"; @@ -201,7 +201,7 @@ private static object HandleReplay(JObject @params) private static object CompileAndExecute(string code, string compiler) { - string wrappedSource = WrapUserCode(code); + string wrappedSource = WrapUserCode(code, out int lineOffset); string cacheKey = compiler + "\n" + wrappedSource; if (_compiledCache.TryGetValue(cacheKey, out CompiledSnippet cached)) @@ -217,14 +217,14 @@ private static object CompileAndExecute(string code, string compiler) case "roslyn": if (!RoslynCompiler.IsAvailable) return new ErrorResponse("Roslyn (Microsoft.CodeAnalysis) is not available. Install it via NuGet or use compiler='codedom'."); - compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, out var roslynErrors); + compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, lineOffset, out var roslynErrors); if (compiled == null) return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(roslynErrors), compiler = "roslyn" }); usedCompiler = "roslyn"; break; case "codedom": - compiled = CodeDomCompile(wrappedSource, assemblyPaths, out var codedomErrors); + compiled = CodeDomCompile(wrappedSource, assemblyPaths, lineOffset, out var codedomErrors); if (compiled == null) return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(codedomErrors), compiler = "codedom" }); usedCompiler = "codedom"; @@ -233,14 +233,14 @@ private static object CompileAndExecute(string code, string compiler) default: // "auto" if (RoslynCompiler.IsAvailable) { - compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, out var autoErrors); + compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, lineOffset, out var autoErrors); if (compiled == null) return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(autoErrors), compiler = "roslyn" }); usedCompiler = "roslyn"; } else { - compiled = CodeDomCompile(wrappedSource, assemblyPaths, out var autoFallbackErrors); + compiled = CodeDomCompile(wrappedSource, assemblyPaths, lineOffset, out var autoFallbackErrors); if (compiled == null) return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(autoFallbackErrors), compiler = "codedom" }); usedCompiler = "codedom"; @@ -302,7 +302,7 @@ private static List OffsetErrors(List errors) // ──────────────────── CodeDom compiler ──────────────────── - private static Assembly CodeDomCompile(string source, string[] assemblyPaths, out List errors) + private static Assembly CodeDomCompile(string source, string[] assemblyPaths, int lineOffset, out List errors) { errors = new List(); @@ -360,7 +360,7 @@ private static Assembly CodeDomCompile(string source, string[] assemblyPaths, ou continue; hasRealErrors = true; - int userLine = Math.Max(1, error.Line - WrapperLineOffset); + int userLine = Math.Max(1, error.Line - lineOffset); errors.Add($"Line {userLine}: {error.ErrorText}"); } @@ -508,22 +508,80 @@ public CodeDomAssemblyCandidate(string path, AssemblyName assemblyName) // ──────────────────── Shared helpers ──────────────────── - private static string WrapUserCode(string code) + private static readonly string[] BuiltinUsings = + { + "using System;", + "using System.Collections.Generic;", + "using System.Linq;", + "using System.Reflection;", + "using System.Text;", + "using UnityEngine;", + "using UnityEditor;", + }; + + private static readonly Regex UsingDirectiveLine = new Regex( + @"^\s*using\s+(?:static\s+)?[A-Za-z_]\w*(?:\s*\.\s*[A-Za-z_]\w*)*\s*;\s*$", RegexOptions.Compiled); + + // Must not match the C# 8 `using var x = ...` declaration, which has two tokens before the = + private static readonly Regex UsingAliasLine = new Regex( + @"^\s*using\s+[A-Za-z_]\w*\s*=\s*[A-Za-z_][\w.]*\s*(?:<[^;]*>)?\s*(?:\[\s*\])*\s*;\s*$", RegexOptions.Compiled); + + private static bool IsUsingDirective(string line) + => UsingDirectiveLine.IsMatch(line) || UsingAliasLine.IsMatch(line); + + // A directive can carry comments, but the regexes above must see a bare `using ...;` + private static string StripComments(string line, ref bool inBlock) { var sb = new StringBuilder(); - sb.AppendLine("using System;"); - sb.AppendLine("using System.Collections.Generic;"); - sb.AppendLine("using System.Linq;"); - sb.AppendLine("using System.Reflection;"); - sb.AppendLine("using UnityEngine;"); - sb.AppendLine("using UnityEditor;"); + for (int i = 0; i < line.Length; i++) + { + bool pair = i + 1 < line.Length; + if (inBlock) + { + if (pair && line[i] == '*' && line[i + 1] == '/') { inBlock = false; i++; } + continue; + } + if (pair && line[i] == '/' && line[i + 1] == '*') { inBlock = true; i++; continue; } + if (pair && line[i] == '/' && line[i + 1] == '/') break; + sb.Append(line[i]); + } + return sb.ToString(); + } + + // Blanked, not removed, so compiler line numbers still map to the caller's + private static List HoistLeadingUsings(string[] body) + { + var hoisted = new List(); + bool inBlock = false; + for (int i = 0; i < body.Length; i++) + { + string directive = StripComments(body[i], ref inBlock).Trim(); + if (directive.Length == 0) continue; + if (!IsUsingDirective(directive)) break; + if (!BuiltinUsings.Contains(directive) && !hoisted.Contains(directive)) hoisted.Add(directive); + body[i] = string.Empty; + } + return hoisted; + } + + // A using directive is illegal in a method body, so the parser reads it as a using-statement and wants a '(' + private static string WrapUserCode(string code, out int lineOffset) + { + var body = code.Replace("\r\n", "\n").Split('\n'); + var hoisted = HoistLeadingUsings(body); + + var sb = new StringBuilder(); + foreach (string u in BuiltinUsings) sb.AppendLine(u); + foreach (string u in hoisted) sb.AppendLine(u); sb.AppendLine($"public static class {WrapperClassName}"); sb.AppendLine("{"); sb.AppendLine($" public static object {WrapperMethodName}()"); sb.AppendLine(" {"); - sb.AppendLine(code); + sb.AppendLine(string.Join("\n", body)); sb.AppendLine(" }"); sb.AppendLine("}"); + // 4 = the class line, its brace, the method line, its brace + lineOffset = BuiltinUsings.Length + hoisted.Count + 4; return sb.ToString(); } @@ -559,9 +617,50 @@ private static string[] ResolveAssemblyPaths() return result; } + // A hoisted `using System.IO;` puts File.Delete in scope, which no fully-qualified + // pattern in _blockedPatterns matches. Derive the short forms the directives create. + private static IEnumerable ShortFormsFromUsings(IEnumerable directives) + { + foreach (string directive in directives) + { + string target = directive.Substring("using".Length).Trim().TrimEnd(';').Trim(); + bool isStatic = target.StartsWith("static ", StringComparison.Ordinal); + if (isStatic) target = target.Substring("static ".Length).Trim(); + + string alias = null; + int eq = target.IndexOf('='); + if (eq >= 0) + { + alias = target.Substring(0, eq).Trim(); + target = target.Substring(eq + 1).Trim(); + } + + int lastDot = target.LastIndexOf('.'); + string leaf = lastDot >= 0 ? target.Substring(lastDot + 1) : target; + + foreach (string pattern in _blockedPatterns) + { + if (pattern.StartsWith(target + ".", StringComparison.OrdinalIgnoreCase)) + { + string tail = pattern.Substring(target.Length + 1); + yield return alias == null ? tail : alias + "." + tail; + } + // A blocked pattern already written short, like AssetDatabase.DeleteAsset, + // needs the leaf instead: `using static ...Process;` makes Process.Start bare Start + else if (pattern.StartsWith(leaf + ".", StringComparison.OrdinalIgnoreCase)) + { + string tail = pattern.Substring(leaf.Length + 1); + if (isStatic) yield return tail; + else if (alias != null) yield return alias + "." + tail; + } + } + } + } + private static string CheckBlockedPatterns(string code) { - foreach (var pattern in _blockedPatterns) + var directives = HoistLeadingUsings(code.Replace("\r\n", "\n").Split('\n')); + foreach (var pattern in _blockedPatterns.Concat(ShortFormsFromUsings(directives))) { if (code.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) >= 0) return $"Code contains blocked pattern: '{pattern}'. Disable safety checks with safety_checks=false if this is intentional."; @@ -663,6 +762,7 @@ public static void ResetCache() _isAvailable = null; } + private static bool Initialize() { try @@ -752,7 +852,7 @@ private static bool Initialize() } } - public static Assembly Compile(string source, string[] assemblyPaths, out List errors) + public static Assembly Compile(string source, string[] assemblyPaths, int lineOffset, out List errors) { errors = new List(); @@ -837,7 +937,7 @@ public static Assembly Compile(string source, string[] assemblyPaths, out List("success"), result.ToString()); + Assert.AreEqual("b.txt", result["data"]["result"].Value()); + } + + [Test] + public void Execute_UsingStaticDirective_IsHoisted() + { + var result = Execute("using static System.Math;\nreturn Max(3, 4);"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual(4, result["data"]["result"].Value()); + } + + [Test] + public void Execute_UsingAliasWithGenericTarget_IsHoisted() + { + var result = Execute("using L = System.Collections.Generic.List;\nreturn new L { 1, 2 }.Count;"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual(2, result["data"]["result"].Value()); + } + + [Test] + public void Execute_UsingAfterComment_IsStillHoisted() + { + var result = Execute("// leading comment\nusing System.IO;\nreturn Path.GetFileName(\"/x/y.txt\");"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("y.txt", result["data"]["result"].Value()); + } + + + [Test] + public void Execute_RedundantBuiltinUsing_DoesNotDuplicate() + { + var result = Execute("using System;\nreturn String.Concat(\"a\", \"b\");"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("ab", result["data"]["result"].Value()); + } + + [Test] + public void Execute_ErrorLineNumber_CountsFromCallerCodeAfterHoisting() + { + var result = Execute("using System.IO;\nint ok = 1;\nnope();\nreturn ok;"); + + Assert.IsFalse(result.Value("success"), result.ToString()); + var errors = string.Join("\n", (result["data"]["errors"] as JArray).Select(e => e.Value())); + StringAssert.Contains("Line 3", errors); + } + + [Test] + public void Execute_UsingWithTrailingComment_IsHoisted() + { + var result = Execute("using System.IO; // needed for Path\nreturn Path.GetFileName(\"/a/b.txt\");"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("b.txt", result["data"]["result"].Value()); + } + + [Test] + public void Execute_UsingAfterBlockComment_IsStillHoisted() + { + var result = Execute("/* lead\n in */\nusing System.IO;\nreturn Path.GetFileName(\"/a/c.txt\");"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("c.txt", result["data"]["result"].Value()); + } + + [Test] + public void Execute_UsingBehindInlineBlockComment_IsHoisted() + { + var result = Execute("/* why */ using System.IO;\nreturn Path.GetFileName(\"/a/d.txt\");"); + + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("d.txt", result["data"]["result"].Value()); + } + + // ──────────────────── Execute: hoisting must not widen safety_checks ──────────────────── + + [Test] + public void Execute_HoistedNamespace_DoesNotBypassSafetyChecks() + { + var result = Execute("using System.IO;\nFile.Delete(\"/tmp/mcp-does-not-exist\");\nreturn 1;"); + + Assert.IsFalse(result.Value("success"), result.ToString()); + StringAssert.Contains("Blocked pattern", result.Value("error")); + } + + [Test] + public void Execute_AliasedType_DoesNotBypassSafetyChecks() + { + var result = Execute("using F = System.IO.File;\nF.Delete(\"/tmp/mcp-does-not-exist\");\nreturn 1;"); + + Assert.IsFalse(result.Value("success"), result.ToString()); + StringAssert.Contains("Blocked pattern", result.Value("error")); + } + + [Test] + public void Execute_UsingStatic_DoesNotBypassSafetyChecks() + { + var result = Execute("using static System.Diagnostics.Process;\nStart(\"ls\");\nreturn 1;"); + + Assert.IsFalse(result.Value("success"), result.ToString()); + StringAssert.Contains("Blocked pattern", result.Value("error")); + } + private static JObject Execute(string code) { return ToJObject(ExecuteCode.HandleCommand(new JObject