fix(mcp): expose new tools under the server approval default (#1959)

feeds/skills/.system/files/netclaw-operations/SKILL.md

@@ -3,7 +3,7 @@ name: netclaw-operations
 description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance."
 metadata:
   author: netclaw
-  version: "2.56.0"
+  version: "2.58.0"
 ---
 
 # Netclaw Operations

feeds/skills/.system/files/netclaw-operations/references/tools.md

@@ -56,15 +56,14 @@ additionally cannot discover or load skills, subagents, memory tools,
 scheduling tools, or the `web_search` / `web_fetch` tools regardless of
 feature flags.
 
-### Adding MCP servers (fail-closed by default)
+### Add MCP servers
 
-`netclaw mcp add` writes new MCP servers with **zero granted tools** and
-per-audience approval defaults so freshly added servers are never silently
-exposed:
+`netclaw mcp add` writes an approval default for each audience.
+The command writes closed tool grants for the Team and Public audiences.
 
 | Audience | Grants | Approval default |
 |----------|--------|------------------|
-| Personal | `[]` (empty list — all tools denied until the operator opts in) | `Approval` |
+| Personal | Not used (`All` MCP server mode) | `Auto` |
 | Team     | `[]` | `Approval` |
 | Public   | `[]` | `Deny` |
 
@@ -74,14 +73,14 @@ per-server approval mode. Bare `netclaw mcp tools` is a read-only CLI view
 of the same state; both commands surface a discoverability hint toward the
 TUI.
 
-Escape hatch: `netclaw mcp add --grant-all` keeps the legacy "null grants
-= all tools pass" behavior for CI. Even with `--grant-all`, the per-audience
-approval defaults (Personal/Team=Approval, Public=Deny) are still written —
-you cannot turn off the approval prompts at `mcp add` time.
+The `--grant-all` option skips the closed grants for Team and Public.
+The option does not change the approval defaults.
 
 Inside the TUI (`netclaw mcp permissions`):
 
-- `Enter` toggles the highlighted tool's grant
+- `Enter` toggles the highlighted tool. In the `All` MCP server mode, the toggle sets
+  `Deny` (disabled) or clears it (inherit the server default). In `Allowlist`
+  mode, it adds or removes the tool from the grant list.
 - `A` toggles all tools on/off for the current audience
 - `E` enables/disables the whole server for the current audience
 - `M` cycles the **server default** approval mode (`Auto → Approval → Deny → Auto`)
@@ -93,19 +92,30 @@ Approval-mode resolution precedence (for MCP tools):
 
 1. Exact `ToolOverrides["{server}/{tool}"]` override
 2. `McpServerDefaults[{server}]` default
-3. Fail-closed fallback (Personal audience, shell/file-edit matcher family)
-4. Audience `DefaultMode`
+3. Audience `DefaultMode`
 
-Newly discovered tools on an existing server automatically inherit the
-server default; you do not need to re-run `permissions` after the server
-learns a new tool.
+A tool with an effective `Deny` mode does not appear in the model tool list.
 
-### Migrating existing MCP servers
+The MCP server mode controls tool grants:
+
+- `All` does not use `McpServerToolGrants`.
+- A new tool inherits the server approval default in the `All` mode.
+- `Allowlist` applies `McpServerToolGrants` when the server has an entry.
+- An absent server entry adds no per-tool filter.
+- Within the approval policy, an exact tool override wins over the server default.
+
+Use `--revoke` to write a `Deny` override in the `All` mode.
+Use `--grant` to remove a `Deny` override or enable a tool above a `Deny` default.
+
+### MCP servers in old configurations
 
 Servers added to `netclaw.json` before this behavior shipped stay untouched —
 their tool grants, `ApprovalPolicy.McpServerDefaults`, and `ToolOverrides`
 entries are not rewritten during an upgrade.
 
+An existing grant snapshot has no effect when its audience uses the `All` mode.
+The update does not remove or replace any exact tool override.
+
 `netclaw doctor` will emit a warning for each enabled MCP server that
 Personal can reach (`McpServersMode = All`) but has no
 `ApprovalPolicy.McpServerDefaults[server]` entry and no `notion/*`-style

src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs

@@ -377,8 +377,79 @@ public void McpServerToolGrants_DeserializesFromJson()
         Assert.False(policy.IsToolExposed(CreateMcpTool("memorizer", "store"), TeamContext()));
     }
 
+    [Theory]
+    [InlineData(ToolApprovalMode.Auto, false)]
+    [InlineData(ToolApprovalMode.Approval, true)]
+    public void AllMcpServersMode_NewTool_InheritsServerDefault(
+        ToolApprovalMode serverDefault,
+        bool needsApproval)
+    {
+        var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed };
+        config.AudienceProfiles.Personal.McpServerToolGrants = new Dictionary<string, List<string>>
+        {
+            ["dropbox"] = ["copy"]
+        };
+        config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig
+        {
+            McpServerDefaults = new Dictionary<string, ToolApprovalMode>(StringComparer.Ordinal)
+            {
+                ["dropbox"] = serverDefault
+            }
+        };
+        var policy = new ToolAccessPolicy(config, Defaults, new ShellCommandPolicy(), new ToolPathPolicy([]));
+        var newTool = CreateMcpTool("dropbox", "get_upload_url");
+
+        Assert.True(policy.IsToolExposed(newTool, PersonalContext()));
+        Assert.Equal(
+            needsApproval,
+            policy.AuthorizeInvocation(newTool, CreateExecutionContext(TrustAudience.Personal)).NeedsApproval);
+    }
+
+    [Fact]
+    public void FilterExposedTools_HidesDenyOverrideAndKeepsNewTool()
+    {
+        var registry = new ToolRegistry();
+        var deniedTool = CreateMcpTool("dropbox", "delete");
+        var newTool = CreateMcpTool("dropbox", "get_upload_url");
+        registry.Register(deniedTool);
+        registry.Register(newTool);
+
+        var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed };
+        config.AudienceProfiles.Personal.McpServerToolGrants = new Dictionary<string, List<string>>
+        {
+            ["dropbox"] = ["delete"]
+        };
+        config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig
+        {
+            ToolOverrides = new Dictionary<string, ToolApprovalMode>(StringComparer.Ordinal)
+            {
+                ["dropbox/delete"] = ToolApprovalMode.Deny
+            }
+        };
+        var policy = new ToolAccessPolicy(config, Defaults, new ShellCommandPolicy(), new ToolPathPolicy([]));
+
+        var filtered = policy.FilterExposedTools(
+            [deniedTool.ToAITool(), newTool.ToAITool()],
+            registry,
+            PersonalTrustContext());
+
+        var exposed = Assert.Single(filtered);
+        Assert.Equal("dropbox__get_upload_url", ((AIFunction)exposed).Name);
+    }
+
     // ── Helpers ──
 
+    private static EffectiveTrustContext PersonalTrustContext() => new(
+        DeploymentPosture.Personal,
+        TrustAudience.Personal,
+        TrustAudience.Personal,
+        TrustAudience.Personal,
+        TrustBoundary.TrustedInstance,
+        PrincipalClassification.TrustedInternal,
+        TransportAuthenticity.Verified,
+        PayloadTaint.Trusted,
+        null, null, false, false, null);
+
     private static McpToolAdapter CreateMcpTool(string serverName, string toolName, string? description = null)
     {
         var func = AIFunctionFactory.Create(() => "result", toolName, description ?? toolName);

src/Netclaw.Actors/Tools/ToolAccessPolicy.cs

@@ -142,7 +142,12 @@ internal bool IsToolExposed(INetclawTool tool, TrustAudience audience)
 
         if (tool is McpToolAdapter mcp)
             return _profileResolver.IsMcpServerAllowed(new McpServerName(mcp.ServerName), audience)
-                && _profileResolver.IsMcpToolAllowed(new McpServerName(mcp.ServerName), new ToolName(mcp.BareToolName), audience);
+                && _profileResolver.IsMcpToolAllowed(
+                    new McpServerName(mcp.ServerName),
+                    new ToolName(mcp.BareToolName),
+                    audience)
+                && _profileResolver.ResolveProfile(audience).ApprovalPolicy?.GetEffectiveMode(mcp.Name)
+                    != ToolApprovalMode.Deny;
 
         if (!_profileResolver.IsToolAllowed(new ToolName(tool.Name), audience))
             return false;

src/Netclaw.Actors/Tools/ToolAudienceProfileResolver.cs

@@ -102,11 +102,8 @@ public bool IsMcpServerAllowed(McpServerName serverName, TrustAudience audience)
     }
 
     /// <summary>
-    /// Checks whether a specific tool from an MCP server is allowed for the given audience.
-    /// Returns true if:
-    /// - The profile has no <see cref="ToolAudienceProfile.McpServerToolGrants"/> (null), or
-    /// - The server has no entry in the grants dictionary, or
-    /// - The tool name appears in the server's grant list.
+    /// Checks whether an MCP tool passes the audience profile.
+    /// Per-tool grants apply only when the profile uses <see cref="ToolProfileMode.Allowlist"/>.
     /// </summary>
     public bool IsMcpToolAllowed(McpServerName serverName, ToolName toolName, TrustAudience audience)
     {
@@ -174,6 +171,9 @@ private static bool IsMcpServerAllowed(McpServerName serverName, ToolAudiencePro
 
     private static bool IsMcpToolAllowed(McpServerName serverName, ToolName toolName, ToolAudienceProfile profile)
     {
+        if (profile.McpServersMode == ToolProfileMode.All)
+            return true;
+
         if (profile.McpServerToolGrants is not { } grants)
             return true;
 

src/Netclaw.Cli.Tests/Mcp/McpCommandTests.cs

@@ -663,6 +663,155 @@ public async Task ReadMcpError_MalformedBodyFallsBackToHttpStatusAndReason()
         Assert.Equal("HTTP 502 Bad Gateway", message);
     }
 
+    [Fact]
+    public async Task Tools_Revoke_AllMcpServersMode_WritesDenyOverrideNotGrantAllowlist()
+    {
+        File.WriteAllText(_paths.NetclawConfigPath, """
+        { "configVersion": 1, "Tools": { "AudienceProfiles": { "Personal": { "McpServersMode": "All" } } } }
+        """);
+        var daemonApi = ToolsDaemonApi("dropbox", "copy", "delete");
+
+        var exitCode = await McpCommand.RunAsync(
+            ["mcp", "tools", "dropbox", "--revoke", "delete", "--audience", "personal"],
+            _paths, daemonApi, _output);
+
+        Assert.Equal(0, exitCode);
+        using var doc = ReadConfigFile(_paths.NetclawConfigPath);
+        var personal = doc.RootElement.GetProperty("Tools").GetProperty("AudienceProfiles").GetProperty("Personal");
+        Assert.Equal(
+            "Deny",
+            personal.GetProperty("ApprovalPolicy").GetProperty("ToolOverrides").GetProperty("dropbox/delete").GetString());
+        Assert.False(personal.TryGetProperty("McpServerToolGrants", out _));
+    }
+
+    [Fact]
+    public async Task Tools_Grant_AllMcpServersMode_ClearsDenyOverride()
+    {
+        File.WriteAllText(_paths.NetclawConfigPath, """
+        {
+          "configVersion": 1,
+          "Tools": { "AudienceProfiles": { "Personal": {
+            "McpServersMode": "All",
+            "ApprovalPolicy": { "ToolOverrides": { "dropbox/delete": "Deny" } }
+          } } }
+        }
+        """);
+        var daemonApi = ToolsDaemonApi("dropbox", "copy", "delete");
+
+        var exitCode = await McpCommand.RunAsync(
+            ["mcp", "tools", "dropbox", "--grant", "delete", "--audience", "personal"],
+            _paths, daemonApi, _output);
+
+        Assert.Equal(0, exitCode);
+        using var doc = ReadConfigFile(_paths.NetclawConfigPath);
+        var overrides = doc.RootElement.GetProperty("Tools").GetProperty("AudienceProfiles")
+            .GetProperty("Personal").GetProperty("ApprovalPolicy").GetProperty("ToolOverrides");
+        Assert.False(overrides.TryGetProperty("dropbox/delete", out _));
+    }
+
+    [Fact]
+    public async Task Tools_Grant_AllMcpServersMode_ClearsAliasDenyOverride()
+    {
+        File.WriteAllText(_paths.NetclawConfigPath, """
+        {
+          "configVersion": 1,
+          "Tools": { "AudienceProfiles": { "Personal": {
+            "McpServersMode": "All",
+            "ApprovalPolicy": { "ToolOverrides": { "dropbox__delete": "Deny" } }
+          } } }
+        }
+        """);
+        var daemonApi = ToolsDaemonApi("dropbox", "copy", "delete");
+
+        var exitCode = await McpCommand.RunAsync(
+            ["mcp", "tools", "dropbox", "--grant", "delete", "--audience", "personal"],
+            _paths, daemonApi, _output);
+
+        Assert.Equal(0, exitCode);
+        using var doc = ReadConfigFile(_paths.NetclawConfigPath);
+        var overrides = doc.RootElement.GetProperty("Tools").GetProperty("AudienceProfiles")
+            .GetProperty("Personal").GetProperty("ApprovalPolicy").GetProperty("ToolOverrides");
+        Assert.False(overrides.TryGetProperty("dropbox/delete", out _));
+        Assert.False(overrides.TryGetProperty("dropbox__delete", out _));
+    }
+
+    [Fact]
+    public async Task Tools_Grant_AllMcpServersMode_PreservesApprovalOverride()
+    {
+        File.WriteAllText(_paths.NetclawConfigPath, """
+        {
+          "configVersion": 1,
+          "Tools": { "AudienceProfiles": { "Personal": {
+            "McpServersMode": "All",
+            "ApprovalPolicy": { "ToolOverrides": { "dropbox/copy": "Approval" } }
+          } } }
+        }
+        """);
+        var daemonApi = ToolsDaemonApi("dropbox", "copy", "delete");
+
+        var exitCode = await McpCommand.RunAsync(
+            ["mcp", "tools", "dropbox", "--grant", "copy", "--audience", "personal"],
+            _paths, daemonApi, _output);
+
+        Assert.Equal(0, exitCode);
+        using var doc = ReadConfigFile(_paths.NetclawConfigPath);
+        var overrides = doc.RootElement.GetProperty("Tools").GetProperty("AudienceProfiles")
+            .GetProperty("Personal").GetProperty("ApprovalPolicy").GetProperty("ToolOverrides");
+        Assert.Equal("Approval", overrides.GetProperty("dropbox/copy").GetString());
+    }
+
+    [Fact]
+    public async Task Tools_Snapshot_AllMcpServersMode_IsRejected()
+    {
+        File.WriteAllText(_paths.NetclawConfigPath, """
+        { "configVersion": 1, "Tools": { "AudienceProfiles": { "Personal": { "McpServersMode": "All" } } } }
+        """);
+        var daemonApi = ToolsDaemonApi("dropbox", "copy", "delete");
+
+        var exitCode = await McpCommand.RunAsync(
+            ["mcp", "tools", "dropbox", "--snapshot", "--audience", "personal"],
+            _paths, daemonApi, _output);
+
+        Assert.Equal(1, exitCode);
+        Assert.Contains("All MCP server mode", _output.ToString(), StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public async Task Tools_Grant_AllMcpServersMode_OverServerDefaultDeny_WritesApprovalOverride()
+    {
+        File.WriteAllText(_paths.NetclawConfigPath, """
+        {
+          "configVersion": 1,
+          "Tools": { "AudienceProfiles": { "Personal": {
+            "McpServersMode": "All",
+            "ApprovalPolicy": { "McpServerDefaults": { "dropbox": "Deny" } }
+          } } }
+        }
+        """);
+        var daemonApi = ToolsDaemonApi("dropbox", "copy", "delete");
+
+        var exitCode = await McpCommand.RunAsync(
+            ["mcp", "tools", "dropbox", "--grant", "copy", "--audience", "personal"],
+            _paths, daemonApi, _output);
+
+        Assert.Equal(0, exitCode);
+        using var doc = ReadConfigFile(_paths.NetclawConfigPath);
+        var overrides = doc.RootElement.GetProperty("Tools").GetProperty("AudienceProfiles")
+            .GetProperty("Personal").GetProperty("ApprovalPolicy").GetProperty("ToolOverrides");
+        Assert.Equal("Approval", overrides.GetProperty("dropbox/copy").GetString());
+    }
+
+    private static DaemonApi ToolsDaemonApi(string serverName, params string[] tools)
+    {
+        var body = JsonSerializer.Serialize(tools);
+        return CreateDaemonApi(request => request.RequestUri!.AbsolutePath == $"/api/mcp/tools/{serverName}"
+            ? new HttpResponseMessage(HttpStatusCode.OK)
+            {
+                Content = new StringContent(body, Encoding.UTF8, "application/json"),
+            }
+            : new HttpResponseMessage(HttpStatusCode.NotFound));
+    }
+
     private static JsonDocument ReadConfigFile(string path)
     {
         return JsonDocument.Parse(File.ReadAllText(path));