diff --git a/Assets/Tests/Editor/McpEditorWindowRefreshPolicyTests.cs b/Assets/Tests/Editor/McpEditorWindowRefreshPolicyTests.cs index ac2fae119..f8f8c6726 100644 --- a/Assets/Tests/Editor/McpEditorWindowRefreshPolicyTests.cs +++ b/Assets/Tests/Editor/McpEditorWindowRefreshPolicyTests.cs @@ -148,7 +148,6 @@ private static ToolSettingsSectionData CreateToolSettingsData( { return new ToolSettingsSectionData( showToolSettings, - allowThirdPartyTools: false, DynamicCodeSecurityLevel.Restricted, System.Array.Empty(), System.Array.Empty(), diff --git a/Assets/Tests/Editor/ToolSettingsSectionTests.cs b/Assets/Tests/Editor/ToolSettingsSectionTests.cs index f07ba08d8..4b694af96 100644 --- a/Assets/Tests/Editor/ToolSettingsSectionTests.cs +++ b/Assets/Tests/Editor/ToolSettingsSectionTests.cs @@ -83,7 +83,6 @@ public void Update_HeaderOnlyRefreshAfterLoad_PreservesLoadedRows() ToolSettingsSectionData headerOnlyData = CreateData( compileEnabled: false, includeGetLogs: false, - allowThirdPartyTools: false, includeThirdPartyTool: true, hasToolListData: false); @@ -177,14 +176,6 @@ private static VisualElement CreateRootElement() { name = "cli-reference-link" }; - Toggle allowThirdPartyToggle = new Toggle - { - name = "allow-third-party-toggle" - }; - Label allowThirdPartyLabel = new Label - { - name = "allow-third-party-label" - }; Button securityLevelRestrictedButton = new Button { name = "security-level-restricted-button" @@ -202,8 +193,6 @@ private static VisualElement CreateRootElement() name = "tool-settings-info-container" }; - foldout.Add(allowThirdPartyToggle); - foldout.Add(allowThirdPartyLabel); foldout.Add(securityLevelRestrictedButton); foldout.Add(securityLevelFullAccessButton); foldout.Add(securityLevelDescription); @@ -218,7 +207,6 @@ private static ToolSettingsSectionData CreateData( bool compileEnabled, bool includeGetLogs, bool showToolSettings = true, - bool allowThirdPartyTools = true, bool includeThirdPartyTool = false, bool hasToolListData = true, DynamicCodeSecurityLevel dynamicCodeSecurityLevel = DynamicCodeSecurityLevel.Restricted) @@ -256,7 +244,6 @@ private static ToolSettingsSectionData CreateData( return new ToolSettingsSectionData( showToolSettings: showToolSettings, - allowThirdPartyTools: allowThirdPartyTools, dynamicCodeSecurityLevel: dynamicCodeSecurityLevel, builtInTools: builtInTools, thirdPartyTools: thirdPartyTools, diff --git a/Assets/Tests/Editor/ULoopSettingsTests.cs b/Assets/Tests/Editor/ULoopSettingsTests.cs index 5b92fab9b..7e9a6c44e 100644 --- a/Assets/Tests/Editor/ULoopSettingsTests.cs +++ b/Assets/Tests/Editor/ULoopSettingsTests.cs @@ -108,9 +108,10 @@ public void GetSettings_WhenNewFileAbsentAndLegacyExists_ShouldMigrateRemainingF ULoopSettingsData result = ULoopSettings.GetSettings(); - Assert.IsTrue(result.allowThirdPartyTools); Assert.AreEqual((int)DynamicCodeSecurityLevel.Restricted, result.dynamicCodeSecurityLevel); Assert.IsTrue(File.Exists(SettingsFilePath), $"{SettingsFilePath} should be created by migration"); + string updatedJson = File.ReadAllText(SettingsFilePath); + StringAssert.DoesNotContain("\"allowThirdPartyTools\"", updatedJson); } [Test] @@ -118,7 +119,6 @@ public void GetSettings_WhenNewFileExists_ShouldIgnoreLegacy() { ULoopSettingsData newSettings = new ULoopSettingsData { - allowThirdPartyTools = true, dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.FullAccess }; File.WriteAllText(SettingsFilePath, JsonUtility.ToJson(newSettings, true)); @@ -133,7 +133,6 @@ public void GetSettings_WhenNewFileExists_ShouldIgnoreLegacy() ULoopSettingsData result = ULoopSettings.GetSettings(); - Assert.IsTrue(result.allowThirdPartyTools); Assert.AreEqual((int)DynamicCodeSecurityLevel.FullAccess, result.dynamicCodeSecurityLevel); } @@ -142,7 +141,6 @@ public void SetAndGet_RoundTrip_ShouldPreserveRemainingValues() { ULoopSettingsData written = new ULoopSettingsData { - allowThirdPartyTools = true, dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.Restricted }; ULoopSettings.SaveSettings(written); @@ -150,7 +148,6 @@ public void SetAndGet_RoundTrip_ShouldPreserveRemainingValues() ULoopSettingsData readBack = ULoopSettings.GetSettings(); - Assert.AreEqual(written.allowThirdPartyTools, readBack.allowThirdPartyTools); Assert.AreEqual(written.dynamicCodeSecurityLevel, readBack.dynamicCodeSecurityLevel); } @@ -189,7 +186,6 @@ public void GetSettings_WhenBothFilesAbsent_ShouldReturnDefaults() ULoopSettingsData result = ULoopSettings.GetSettings(); - Assert.IsFalse(result.allowThirdPartyTools); Assert.AreEqual((int)DynamicCodeSecurityLevel.Restricted, result.dynamicCodeSecurityLevel); Assert.IsFalse(File.Exists(LegacySettingsFilePath), "Legacy file should not be created when both files are absent"); @@ -200,7 +196,7 @@ public void GetSettings_WhenPrimaryMissingAndBackupExists_ShouldRecoverFromBacku { DeleteIfExists(SettingsFilePath); - ULoopSettingsData backupData = new ULoopSettingsData + SettingsFileFixture backupData = new SettingsFileFixture { allowThirdPartyTools = false, dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.FullAccess @@ -211,9 +207,10 @@ public void GetSettings_WhenPrimaryMissingAndBackupExists_ShouldRecoverFromBacku ULoopSettingsData result = ULoopSettings.GetSettings(); Assert.IsTrue(File.Exists(SettingsFilePath), $"{SettingsFilePath} should be recovered from .bak"); - Assert.IsFalse(result.allowThirdPartyTools); Assert.AreEqual((int)DynamicCodeSecurityLevel.FullAccess, result.dynamicCodeSecurityLevel); Assert.IsFalse(File.Exists(SettingsBackupPath), ".bak should be consumed by recovery"); + string updatedJson = File.ReadAllText(SettingsFilePath); + StringAssert.DoesNotContain("\"allowThirdPartyTools\"", updatedJson); } [Test] @@ -222,7 +219,7 @@ public void GetSettings_WhenOldSecurityJsonExists_ShouldRenameToPermissions() DeleteIfExists(SettingsFilePath); DeleteIfExists(LegacySettingsFilePath); - ULoopSettingsData oldData = new ULoopSettingsData + SettingsFileFixture oldData = new SettingsFileFixture { allowThirdPartyTools = false, dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.Restricted @@ -234,8 +231,30 @@ public void GetSettings_WhenOldSecurityJsonExists_ShouldRenameToPermissions() Assert.IsTrue(File.Exists(SettingsFilePath), "New settings file should exist after rename"); Assert.IsFalse(File.Exists(OldSettingsFilePath), "Old settings file should be removed after rename"); - Assert.IsFalse(result.allowThirdPartyTools); Assert.AreEqual((int)DynamicCodeSecurityLevel.Restricted, result.dynamicCodeSecurityLevel); + string updatedJson = File.ReadAllText(SettingsFilePath); + StringAssert.DoesNotContain("\"allowThirdPartyTools\"", updatedJson); + } + + [Test] + public void GetSettings_WhenOldSecurityJsonExistsAndLegacyDisablesRunTests_ShouldPreferLegacyToolToggle() + { + DeleteIfExists(SettingsFilePath); + DeleteIfExists(ToolSettingsFilePath); + + ULoopSettingsData oldData = new ULoopSettingsData + { + dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.Restricted + }; + File.WriteAllText(OldSettingsFilePath, JsonUtility.ToJson(oldData, true)); + File.WriteAllText(LegacySettingsFilePath, "{ \"enableTestsExecution\": false, \"showDeveloperTools\": true }"); + InvalidateBothCaches(); + + ULoopSettingsData result = ULoopSettings.GetSettings(); + + Assert.AreEqual((int)DynamicCodeSecurityLevel.Restricted, result.dynamicCodeSecurityLevel); + Assert.IsFalse(ToolSettings.IsToolEnabled(McpConstants.TOOL_NAME_RUN_TESTS)); + Assert.IsFalse(File.Exists(OldSettingsFilePath), "Old settings file should be removed after legacy migration"); } [Test] @@ -253,8 +272,9 @@ public void GetSettings_WhenJsonContainsRemovedFields_ShouldIgnoreThem() ULoopSettingsData result = ULoopSettings.GetSettings(); - Assert.IsTrue(result.allowThirdPartyTools); Assert.AreEqual((int)DynamicCodeSecurityLevel.FullAccess, result.dynamicCodeSecurityLevel); + string updatedJson = File.ReadAllText(SettingsFilePath); + StringAssert.DoesNotContain("\"allowThirdPartyTools\"", updatedJson); } [Test] @@ -344,7 +364,7 @@ public void SaveSettings_WhenJsonContainsRemovedFields_ShouldRewriteWithoutThem( StringAssert.DoesNotContain("\"enableTestsExecution\"", updatedJson); StringAssert.DoesNotContain("\"allowMenuItemExecution\"", updatedJson); - StringAssert.Contains("\"allowThirdPartyTools\"", updatedJson); + StringAssert.DoesNotContain("\"allowThirdPartyTools\"", updatedJson); StringAssert.Contains("\"dynamicCodeSecurityLevel\"", updatedJson); } diff --git a/Packages/src/Editor/Config/ULoopSettings.cs b/Packages/src/Editor/Config/ULoopSettings.cs index 055d70d97..672addf5c 100644 --- a/Packages/src/Editor/Config/ULoopSettings.cs +++ b/Packages/src/Editor/Config/ULoopSettings.cs @@ -16,6 +16,8 @@ namespace io.github.hatayama.uLoopMCP /// public static class ULoopSettings { + private const string LEGACY_ALLOW_THIRD_PARTY_TOOLS_FIELD = "allowThirdPartyTools"; + private static string SettingsFilePath => Path.Combine(McpConstants.ULOOP_DIR, McpConstants.ULOOP_SETTINGS_FILE_NAME); @@ -63,18 +65,6 @@ public static void UpdateSettings(Func tra SaveSettings(updated); } - public static bool GetAllowThirdPartyTools() - { - return GetSettings().allowThirdPartyTools; - } - - public static void SetAllowThirdPartyTools(bool value) - { - ULoopSettingsData settings = GetSettings(); - ULoopSettingsData updated = settings with { allowThirdPartyTools = value }; - SaveSettings(updated); - } - public static DynamicCodeSecurityLevel GetDynamicCodeSecurityLevel() { ULoopSettingsData settings = GetSettings(); @@ -159,7 +149,8 @@ private static void LoadSettings() _cachedSettings = JsonUtility.FromJson(json); bool migratedToolToggles = ApplyLegacyToolToggleMigrations(json); bool normalizedDynamicCode = NormalizeLegacyDisabledDynamicCode(); - if (migratedToolToggles || normalizedDynamicCode) + bool removedThirdPartyToolsField = json.Contains($"\"{LEGACY_ALLOW_THIRD_PARTY_TOOLS_FIELD}\""); + if (migratedToolToggles || normalizedDynamicCode || removedThirdPartyToolsField) { SaveSettings(_cachedSettings); } @@ -178,7 +169,9 @@ private static bool LegacyFileHasSecurityFields() } string json = File.ReadAllText(LegacySettingsFilePath); - return json.Contains($"\"{nameof(LegacySecuritySettingsProbe.allowThirdPartyTools)}\"") + return json.Contains($"\"{LEGACY_ALLOW_THIRD_PARTY_TOOLS_FIELD}\"") + || json.Contains($"\"{nameof(LegacySecuritySettingsProbe.enableTestsExecution)}\"") + || json.Contains($"\"{nameof(LegacySecuritySettingsProbe.allowMenuItemExecution)}\"") || json.Contains($"\"{nameof(LegacySecuritySettingsProbe.dynamicCodeSecurityLevel)}\""); } @@ -199,7 +192,6 @@ private class LegacySecuritySettingsProbe { public bool enableTestsExecution = true; public bool allowMenuItemExecution = true; - public bool allowThirdPartyTools = false; public int dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.Restricted; } @@ -227,7 +219,6 @@ private static void MigrateFromLegacySettings() _cachedSettings = new ULoopSettingsData { - allowThirdPartyTools = probe.allowThirdPartyTools, dynamicCodeSecurityLevel = probe.dynamicCodeSecurityLevel }; diff --git a/Packages/src/Editor/Config/ULoopSettingsData.cs b/Packages/src/Editor/Config/ULoopSettingsData.cs index de03d32a3..9f82b37b6 100644 --- a/Packages/src/Editor/Config/ULoopSettingsData.cs +++ b/Packages/src/Editor/Config/ULoopSettingsData.cs @@ -9,7 +9,6 @@ namespace io.github.hatayama.uLoopMCP [Serializable] public record ULoopSettingsData { - public bool allowThirdPartyTools = false; public int dynamicCodeSecurityLevel = (int)DynamicCodeSecurityLevel.Restricted; } } diff --git a/Packages/src/Editor/Security/McpSecurityChecker.cs b/Packages/src/Editor/Security/McpSecurityChecker.cs index d543a2e46..3518faa55 100644 --- a/Packages/src/Editor/Security/McpSecurityChecker.cs +++ b/Packages/src/Editor/Security/McpSecurityChecker.cs @@ -92,7 +92,7 @@ private static bool IsToolAllowedByAttribute(ToolAttributeInfo toolInfo) switch (toolInfo.RequiredSecuritySetting) { case SecuritySettings.None: - return IsThirdPartyToolAllowed(toolInfo.ToolName); + return true; default: return false; // Unknown setting - block by default } @@ -148,18 +148,7 @@ public static string GetBlockReason(string toolName) return $"Tool '{toolName}' is not allowed by security policy."; } - switch (toolInfo.Value.RequiredSecuritySetting) - { - case SecuritySettings.None: - if (IsThirdPartyTool(toolName)) - { - return "Third party tools execution is disabled. Enable 'Allow Third Party Tools' in uLoopMCP Security Settings."; - } - return $"Tool '{toolName}' is not allowed by security policy."; - - default: - return $"Tool '{toolName}' is not allowed by security policy."; - } + return $"Tool '{toolName}' is not allowed by security policy."; } /// @@ -175,58 +164,6 @@ public static ToolSecurityInfo GetToolSecurityInfo(string toolName) return new ToolSecurityInfo(toolName, isAllowed, reason); } - /// - /// Checks if third party tools execution is allowed - /// - /// True if third party tools execution is allowed - private static bool IsThirdPartyToolsAllowed() - { - return ULoopSettings.GetAllowThirdPartyTools(); - } - - /// - /// Checks if a specific tool is allowed (considers both explicit setting and third party status) - /// - /// The name of the tool to check - /// True if tool is allowed - private static bool IsThirdPartyToolAllowed(string toolName) - { - // If it's not a third party tool (i.e., official tool), allow it - if (!IsThirdPartyTool(toolName)) - { - return true; - } - - // If it's a third party tool, check the setting - return IsThirdPartyToolsAllowed(); - } - - /// - /// Checks if a tool is a third party tool based on its assembly - /// - /// The name of the tool to check - /// True if tool is from a third party assembly - private static bool IsThirdPartyTool(string toolName) - { - // Get tool type from registry - var registry = CustomToolManager.GetRegistry(); - if (registry == null) - { - return true; // Default to third party if registry unavailable - } - - var toolType = registry.GetToolType(toolName); - if (toolType == null) - { - return true; // Default to third party if tool type not found - } - - // Check assembly name - string assemblyName = toolType.Assembly.GetName().Name; - - // Official uLoopMCP assembly - return assemblyName != "uLoopMCP.Editor"; - } } /// diff --git a/Packages/src/Editor/UI/McpEditorModel.cs b/Packages/src/Editor/UI/McpEditorModel.cs index 8e051cb43..874223619 100644 --- a/Packages/src/Editor/UI/McpEditorModel.cs +++ b/Packages/src/Editor/UI/McpEditorModel.cs @@ -209,14 +209,6 @@ public void UpdateToolEnabled(string toolName, bool enabled) ToolSettings.SetToolEnabled(toolName, enabled); } - /// - /// Update AllowThirdPartyTools setting with persistence - /// - public void UpdateAllowThirdPartyTools(bool allow) - { - ULoopSettings.SetAllowThirdPartyTools(allow); - } - public void UpdateShowConfiguration(bool show) { UpdateUIState(ui => new UIState( diff --git a/Packages/src/Editor/UI/McpEditorWindow.cs b/Packages/src/Editor/UI/McpEditorWindow.cs index 83fb2ed0a..46249ae69 100644 --- a/Packages/src/Editor/UI/McpEditorWindow.cs +++ b/Packages/src/Editor/UI/McpEditorWindow.cs @@ -101,7 +101,6 @@ private void SetupViewCallbacks() _view.OnConnectedToolsFoldoutChanged += UpdateShowConnectedTools; _view.OnToolSettingsFoldoutChanged += UpdateShowToolSettings; _view.OnToolToggled += HandleToolToggled; - _view.OnAllowThirdPartyChanged += UpdateAllowThirdPartyTools; _view.OnSecurityLevelChanged += UpdateDynamicCodeSecurityLevel; } @@ -382,7 +381,6 @@ private ToolSettingsSectionData CreateToolSettingsHeaderData() { return new ToolSettingsSectionData( _model.UI.ShowToolSettings, - ULoopSettings.GetAllowThirdPartyTools(), ULoopSettings.GetDynamicCodeSecurityLevel(), System.Array.Empty(), System.Array.Empty(), @@ -397,7 +395,6 @@ private ToolSettingsSectionData CreateToolSettingsData() { return new ToolSettingsSectionData( _model.UI.ShowToolSettings, - ULoopSettings.GetAllowThirdPartyTools(), ULoopSettings.GetDynamicCodeSecurityLevel(), System.Array.Empty(), System.Array.Empty(), @@ -437,7 +434,6 @@ private ToolSettingsSectionData CreateToolSettingsData() return new ToolSettingsSectionData( _model.UI.ShowToolSettings, - ULoopSettings.GetAllowThirdPartyTools(), ULoopSettings.GetDynamicCodeSecurityLevel(), builtIn.ToArray(), thirdParty.ToArray(), @@ -561,12 +557,6 @@ private void UpdateShowConfiguration(bool show) _model.UpdateShowConfiguration(show); } - private void UpdateAllowThirdPartyTools(bool allow) - { - _model.UpdateAllowThirdPartyTools(allow); - RefreshToolSettingsHeader(); - } - private void UpdateDynamicCodeSecurityLevel(DynamicCodeSecurityLevel level) { ULoopSettings.SetDynamicCodeSecurityLevel(level); diff --git a/Packages/src/Editor/UI/McpEditorWindowViewData.cs b/Packages/src/Editor/UI/McpEditorWindowViewData.cs index 12df8f037..aa76c0f49 100644 --- a/Packages/src/Editor/UI/McpEditorWindowViewData.cs +++ b/Packages/src/Editor/UI/McpEditorWindowViewData.cs @@ -69,7 +69,6 @@ public ToolToggleItem(string toolName, string description, bool isEnabled, bool public record ToolSettingsSectionData { public readonly bool ShowToolSettings; - public readonly bool AllowThirdPartyTools; public readonly DynamicCodeSecurityLevel DynamicCodeSecurityLevel; public readonly ToolToggleItem[] BuiltInTools; public readonly ToolToggleItem[] ThirdPartyTools; @@ -78,7 +77,6 @@ public record ToolSettingsSectionData public ToolSettingsSectionData( bool showToolSettings, - bool allowThirdPartyTools, DynamicCodeSecurityLevel dynamicCodeSecurityLevel, ToolToggleItem[] builtInTools, ToolToggleItem[] thirdPartyTools, @@ -86,7 +84,6 @@ public ToolSettingsSectionData( bool hasToolListData = true) { ShowToolSettings = showToolSettings; - AllowThirdPartyTools = allowThirdPartyTools; DynamicCodeSecurityLevel = dynamicCodeSecurityLevel; BuiltInTools = builtInTools; ThirdPartyTools = thirdPartyTools; diff --git a/Packages/src/Editor/UI/UIToolkit/Components/CliSetupSection.cs b/Packages/src/Editor/UI/UIToolkit/Components/CliSetupSection.cs index f16a0f8a5..24aa05354 100644 --- a/Packages/src/Editor/UI/UIToolkit/Components/CliSetupSection.cs +++ b/Packages/src/Editor/UI/UIToolkit/Components/CliSetupSection.cs @@ -82,7 +82,7 @@ private void UpdateCliStatus(CliSetupData data) { ViewDataBinder.ToggleClass(_cliStatusIcon, "mcp-cli-status-icon--installed", false); ViewDataBinder.ToggleClass(_cliStatusIcon, "mcp-cli-status-icon--not-installed", false); - _cliStatusLabel.text = "uloop-cli: Checking..."; + _cliStatusLabel.text = "CLI: Checking..."; return; } @@ -91,11 +91,11 @@ private void UpdateCliStatus(CliSetupData data) if (data.IsCliInstalled && data.CliVersion != null) { - _cliStatusLabel.text = $"uloop-cli: v{data.CliVersion}"; + _cliStatusLabel.text = $"CLI: v{data.CliVersion}"; return; } - _cliStatusLabel.text = "uloop-cli: Not installed"; + _cliStatusLabel.text = "CLI: Not installed"; } private void UpdateRefreshButton(CliSetupData data) diff --git a/Packages/src/Editor/UI/UIToolkit/Components/ToolSettingsSection.cs b/Packages/src/Editor/UI/UIToolkit/Components/ToolSettingsSection.cs index f6daae226..227a421d8 100644 --- a/Packages/src/Editor/UI/UIToolkit/Components/ToolSettingsSection.cs +++ b/Packages/src/Editor/UI/UIToolkit/Components/ToolSettingsSection.cs @@ -16,8 +16,6 @@ public class ToolSettingsSection private const int InlineToolRowLimit = 40; private readonly Foldout _foldout; - private readonly Toggle _allowThirdPartyToggle; - private readonly Label _allowThirdPartyLabel; private readonly Button _securityLevelRestrictedButton; private readonly Button _securityLevelFullAccessButton; private readonly Label _securityLevelDescription; @@ -27,7 +25,6 @@ public class ToolSettingsSection private readonly ListView _toolListView; private readonly List _toolListRows = new(); private readonly Dictionary _togglesByToolName = new(); - private bool _allowThirdPartyTools = true; private bool _isRegistryAvailable; private bool _isUnavailableStateShown; private bool _isLoadingStateShown; @@ -35,14 +32,11 @@ public class ToolSettingsSection public event Action OnFoldoutChanged; public event Action OnToolToggled; - public event Action OnAllowThirdPartyChanged; public event Action OnSecurityLevelChanged; public ToolSettingsSection(VisualElement root) { _foldout = root.Q("tool-settings-foldout"); - _allowThirdPartyToggle = root.Q("allow-third-party-toggle"); - _allowThirdPartyLabel = root.Q