From 6179712145bf85e6b4a5bff2e9577c363f393c39 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:34:15 -0400 Subject: [PATCH] Darling MCP: bulk add/remove monitored servers (add_servers, remove_server) Add the server-onboarding MCP write tools so an MCP client can stand up or tear down FLEET monitoring conversationally - the service-side twin of the Viewer's Add / Manage Servers dialogs, and the sibling of the #1600 Custom Views and #1608 alert-tuning MCP write tools. - add_servers (BULK): a JSON array of server objects, processed sequentially - validate -> case-folded dedupe (shared ServerIdHelper.BuildStorageName gate, #1549) -> in-process DarlingServerConnector.ProbeAsync (the service holds the network path + credentials, so no test_connect command plane) -> DarlingSecrets DPAPI-encrypt (round-trips at collection) -> INSERT mirroring SeedMonitoredServersAsync. Per-server added/duplicate/connection_failed/invalid; a connection failure does not abort the batch; Entra/MFA rejected; encrypt_mode + trust_server_certificate exposed. Returns {added, skipped, failed, results}. - remove_server: resolve via the same resolver the read tools use, DELETE the config_monitored_servers row. - Grant: mcp role gets INSERT/UPDATE/DELETE on config.config_monitored_servers (provisioning, not migration); the encrypted_password column stays SELECT-carved (write a credential, never read one back). The #1608 config_service beacon column-grant already covers the monitored-servers bump trigger. - MCP instructions (85 -> 87 tools), cross-app ratchet (Darling-only), /api/read write-exclusion, and the README MCP blast-radius (credential-on-the-wire note). Verified: Darling service + Darling.Tests build clean; Darling.Tests 2847 passed / 0 failed / 146 gated-live skipped (DARLING_TEST_PG cleared); Lite.Tests CrossAppMcpToolInventoryPinTests 2 passed. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 3 + .../Darling.Tests/DarlingManagedRolesTests.cs | 23 + .../DarlingMcpServerAdminToolsTests.cs | 383 ++++++++++++ .../DarlingSecuritySplitLiveTests.cs | 18 +- .../Darling.Tests/DarlingWebEndpointsTests.cs | 13 +- .../DarlingManagedRoles.cs | 21 +- .../DarlingWebEndpoints.cs | 5 +- .../Mcp/DarlingMcpHostService.cs | 13 +- .../Mcp/DarlingMcpInstructions.cs | 19 +- .../Mcp/DarlingMcpServerAdminTools.cs | 564 ++++++++++++++++++ Darling/README.md | 12 +- .../CrossAppMcpToolInventoryPinTests.cs | 9 + 12 files changed, 1063 insertions(+), 20 deletions(-) create mode 100644 Darling/Darling.Tests/DarlingMcpServerAdminToolsTests.cs create mode 100644 Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpServerAdminTools.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 38f133165..5a9ff5d0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Darling MCP: bulk add/remove monitored servers — an MCP client can now stand up FLEET monitoring conversationally** ([#1609]) - the Darling MCP server exposed read tools plus the [#1600] Custom Views and [#1608] alert-tuning writes, but no way for an MCP client (or Claude, over MCP) to add or remove the MONITORED SERVERS themselves - onboarding was WPF-Viewer-only (the Add / Manage Servers dialogs). Two new tools close that gap, the direct sibling of the alert-tuning slice: `add_servers` (BULK) takes a JSON ARRAY of server objects and, IN ORDER (sequential, mirroring [#1549]'s bulk-probe, to avoid a probe storm), validates each, connection-tests it IN the service, and saves the new+reachable ones - so "monitor these twenty servers with this login" stands up fleet monitoring in one call; `remove_server` removes one by name. **No divergent second implementation:** the probe is `DarlingServerConnector.ProbeAsync` run IN-PROCESS (the MCP host lives inside the service, which holds the network path + credentials - unlike the Viewer's dialogs, which enqueue a `test_connect` command for the service to run), the case-folded dedupe gate is the shared `ServerIdHelper.BuildStorageName` identity in a `HashSet(OrdinalIgnoreCase)` exactly as the [#1549] bulk dialog uses, the SQL password is DPAPI-encrypted through the SAME `DarlingSecrets.Protect` the service decrypts with at collection time (so it round-trips), and the INSERT mirrors `StoreConfigProvider.SeedMonitoredServersAsync`'s exact column set + `server_id` identity (so a tool-written row JOINs the collected data and the service's reconcile matches it, picking it up within one sweep - no restart). Per server, `add_servers` returns `status` `added` / `duplicate` (a case-variant or exact dup of an existing or earlier-in-batch server, skipped WITHOUT a probe) / `connection_failed` (recorded, the batch CONTINUES) / `invalid` (a bad field, or Entra/MFA/Service-Principal/Managed-Identity auth - interactive MFA is nonsensical headless, the same belt [#1549] applies), and the whole call returns `{added, skipped, failed, results:[...]}`; per Erik, the TLS options `encrypt_mode` (Optional/Mandatory/Strict) and `trust_server_certificate` are EXPOSED so a headless caller sets the connection posture explicitly. **Security (deliberate, scoped):** the tools connect as the least-privilege `mcp` role, now granted - via role PROVISIONING, not a migration - INSERT/UPDATE/DELETE on `config.config_monitored_servers` (mirroring how [#1600]/[#1608] granted their single tables), a single non-secret-KEY table: the `encrypted_password` column stays in the fail-closed secret carve, so `mcp` can WRITE a credential blob (onboarding) but can never READ one back, and it still cannot reach the `config_command` service-credential pivot or a schema-wide config write. **The #1608 beacon grant already covers this:** a `config_monitored_servers` write fires the existing `trg_bump_monitored_servers → config_bump_version` trigger (SECURITY INVOKER, UPDATEs `config_service.config_version` AS `mcp`), and [#1608] already granted `mcp` UPDATE on the two `config_service` beacon columns - verified, no new `config_service` grant is added. **Credential on the wire:** the SQL password travels to the MCP endpoint inside `add_servers`' request JSON (DPAPI-encrypted at rest, never returned by any read tool), which is one more reason a LAN deployment should front the endpoint with the documented TLS reverse proxy - the README's MCP blast-radius section now states this and recommends Windows/integrated auth for onboarded servers where possible. The MCP server instructions (now eighty-seven tools), the cross-app tool-inventory ratchet (Lite is a single-instance app with no central monitored-server store, so these are Darling-only), the `/api/read` write-exclusion set, and the Darling README are updated. Verified: the Darling service + `Darling.Tests` build clean (0 errors, no new warnings in the changed source), full `Darling.Tests` **2847 passed / 0 failed / 146 gated-live skipped** (with `DARLING_TEST_PG` cleared), and `Lite.Tests` `CrossAppMcpToolInventoryPinTests` **2 passed** - the ungated pins cover the two-tool surface, the Gemini-clean schema + required-params, validate-before-write (a malformed payload or a bad/MFA entry returns `invalid` WITHOUT probing or opening a connection), and the pure case-folded dedupe partition; one gated-live test (`DARLING_TEST_PG`, own-scoped + cleaned up, the SQL probe stubbed to success - no live SQL Server touched in CI) proves the store INSERT (the SQL secret is DPAPI-encrypted at rest and round-trips, the Windows-auth server is secret-free), the duplicate skip, the `config_version` self-bump, and `remove_server` (removed, then not_found). The REAL end-to-end probe is what the human dogfoods (remove sql2016, re-add via MCP). + - **Darling MCP: alert-tuning write tools — an MCP client can now tune thresholds and manage mute rules conversationally** ([#1608]) - the Darling MCP server exposed alert READS (`get_alert_history` / `get_alert_settings` / `get_mute_rules`) but no way for an MCP client (or Claude, over MCP) to CHANGE the alerting - thresholds and mute rules could only be edited in the WPF Viewer's Settings window. Three new tools close that gap, the direct sibling of the [#1600] Custom Views MCP write tools: `update_alert_settings` (a PARTIAL update of the single global alert-settings row - the agent reads via `get_alert_settings`, changes fields, and sends only those back in the SAME nested shape, e.g. `{"cpu":{"threshold_percent":90},"cooldown_minutes":10}`), and `create_mute_rule` / `delete_mute_rule` (add/remove the mute rules the delivery paths honor). **No divergent second implementation:** `update_alert_settings` validates EVERY provided field against the SAME ranges/enums the Viewer's Settings window enforces (`SettingsWindow.BuildAlertRowFromControls` - thresholds in range, `cpu.mode` `sql`/`total`, `delivery.mode` `Summary`/`PerEvent`, counts within bounds) BEFORE any write - an out-of-range value or an unknown field (top-level or nested) returns `{status:"invalid"}` and writes nothing - then applies ONLY the provided columns via a targeted parameterized `UPDATE ... WHERE id = 1` and re-reads the merged state; `create_mute_rule` / `delete_mute_rule` reuse the SAME `PgMuteRuleStore` `get_mute_rules` reads through, with the same GUID id-generation the Viewer's mute-create path uses. SMTP/webhook delivery credentials are out of scope (the `mcp` role cannot read or write the secret columns). A `config_alert_settings` write self-bumps `config_version` via the existing config-table trigger, so the running service HOT-RELOADS the change within one collection sweep (the tool never writes `config_version` itself). **Security (deliberate, scoped):** the tools connect as the least-privilege `mcp` role, now granted - via role PROVISIONING, not a migration - INSERT/UPDATE/DELETE on `config.config_mute_rules` and UPDATE on the singleton `config.config_alert_settings` (mirroring how [#1600] granted `config.custom_views`), so a token-holder can tune alerting but still cannot reach the `config_command` service-credential pivot or the carved secret columns. **One non-obvious grant, called out for review:** the `config_alert_settings` bump trigger (`config_bump_version`) is SECURITY INVOKER and UPDATEs `config_service` AS the writing role, so the `mcp` role ALSO needs a COLUMN-level UPDATE on just the two `config_service` beacon columns (`config_version`, `updated_at`) or every `update_alert_settings` write would fail 42501 in production - and the superuser-run gated-live tests would never catch it. The column grant lets `mcp` bump the reload beacon but NOT flip `paused` / `capture_plans` / `mcp_enabled` / `mcp_port`; the live security test now proves this end-to-end as the real `mcp` role (a `config_alert_settings` UPDATE succeeds and fires the beacon; a `paused` UPDATE still 42501s). The MCP server instructions, the cross-app tool-inventory ratchet (Lite has no central alert store, so these are Darling-only), and the Darling README's MCP blast-radius section are updated. Verified: the Darling service + `Darling.Tests` build clean (0 errors, 0 warnings in the changed source), full `Darling.Tests` **2821 passed / 0 failed / 145 gated-live skipped** (with `DARLING_TEST_PG` cleared), and `Lite.Tests` `CrossAppMcpToolInventoryPinTests` **2 passed** - the ungated pins cover the six-tool surface, the Gemini-clean schema + required-params, and validate-before-write (a bad/unknown partial update returns `invalid` WITHOUT opening a connection); one gated-live test (`DARLING_TEST_PG`, own-scoped + restored) proves `update_alert_settings` flips a threshold AND self-bumps `config_version`, and `create_mute_rule`→`get_mute_rules`→`delete_mute_rule` round-trips, and it skips in the normal unit run like the others. - **Darling: headless `--enable-mcp` / `--disable-mcp` / `--enable-web` / `--disable-web` CLI verbs — bring an endpoint up (store + firewall) without the Viewer** ([#1601]) - a headless Darling box had no supported way to (a) turn the MCP or web-dashboard endpoint on/off or (b) open its firewall. Two structural reasons: `mcp.enabled`/`web.enabled` in `darling.json` are only a FIRST-RUN seed - after the first run the store (`config.config_service.mcp_enabled`/`web_enabled`) is authoritative and is normally toggled only by the WPF Viewer's Settings, which a headless deployment does not have; and the service runs as a virtual service account (`NT SERVICE\PerformanceMonitor Darling`) that CANNOT modify Windows Firewall, so its best-effort self-reconcile silently fails. Each verb closes both gaps in one elevated action. **(store)** a TARGETED `UPDATE config.config_service SET = ..., updated_by = 'cli' WHERE id = 1` flips ONLY that endpoint's flag; the existing BEFORE-UPDATE self-bump trigger increments `config_version`, so the worker HOT-RELOADS within one collection sweep - no restart - and the write deliberately never touches `config_version` itself, `paused`, or the other endpoint's flag (0 rows affected ⇒ the store isn't seeded yet, reported as such; the owner credential missing ⇒ the service has never initialized the store, reported as such). **(firewall)** only when the endpoint's `darling.json` network block opts into LAN exposure (a non-loopback `listen`, decided via the shared `DarlingNetwork.IsExposedListenAddress`): run ELEVATED, it opens/removes the SAME scoped, idempotent-by-DisplayName rule the host self-reconciles (the two `McpFirewallRuleName`/`WebFirewallRuleName` builders are now `internal` so the CLI and host act on the EXACT same rule, through the shared `BuildFirewallEnableCommand`/`BuildFirewallDisableCommand` builders); run NON-elevated, the store toggle still succeeds and the exact elevated command is printed as a HANDOFF (never a failure); a loopback-only endpoint gets a note pointing at `--configure-network` and takes no firewall action. A firewall failure is non-fatal. Managed-mode only (BYO governs its own `config_service` + exposure) and Windows-only (DPAPI credential decrypt + `WindowsPrincipal` + firewall), the same guard shape as `--print-viewer-connection`; wired into `IsKnownVerb`, `UsageText`, and the `Program` dispatch (allow-list and dispatch kept in sync per the existing anti-drift comments). The output states the store change, that the running service applies it live within one sweep, the firewall outcome/handoff, and a reminder that `darling.json`'s `enabled` is only the seed - the store is the live switch. No store schema change. Verified: `Darling.Tests` **2789 passed / 0 failed / 144 gated-live skipped** (with `DARLING_TEST_PG` cleared) and 0 new build warnings - the pure tests pin the four store-write SQL strings (right flag, `updated_by='cli'`, `WHERE id=1`, never `config_version`/`paused`/the other endpoint's flag), the verb recognition + classify wiring, the pure firewall-step classifier (exposed × elevated), and the shared rule names; one gated-live test (`DARLING_TEST_PG`, transaction-rolled-back) proves enable then disable flip the flag AND self-bump `config_version` against a throwaway Postgres, and it skips in the normal unit run exactly like the other `*_AgainstDevPostgres` tests (CI's `darling-pg` job runs it live). @@ -490,6 +492,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#1598]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1598 [#1599]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1599 [#1608]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1608 +[#1609]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1609 [#1601]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1601 [#1604]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1604 [#1602]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1602 diff --git a/Darling/Darling.Tests/DarlingManagedRolesTests.cs b/Darling/Darling.Tests/DarlingManagedRolesTests.cs index b736c8f31..f043c1716 100644 --- a/Darling/Darling.Tests/DarlingManagedRolesTests.cs +++ b/Darling/Darling.Tests/DarlingManagedRolesTests.cs @@ -303,6 +303,29 @@ public void BuildProvisioningSql_McpRole_GrantsAlertTuningWrites_NarrowlyWithBea } } + [Fact] + public void BuildProvisioningSql_McpRole_GrantsMonitoredServersWrite_Narrowly() + { + var sql = DarlingManagedRoles.BuildProvisioningSql("AdminPassword01", "ViewerPassword02", "McpPassword03"); + + /* The MCP server-onboarding write tools (add_servers / remove_server): full CRUD on the single + config_monitored_servers table — an EXPLICIT single-table statement, its own 'TO mcp' line. */ + Assert.Contains("GRANT INSERT, UPDATE, DELETE ON config.config_monitored_servers TO mcp;", sql, StringComparison.Ordinal); + + /* Still NARROW: no schema-wide config write for mcp, and NO ALTER DEFAULT PRIVILEGES names mcp (either + would broaden it to all of config). No NEW config_service grant — section 8's beacon column-grant + already covers the monitored-servers bump trigger. */ + Assert.DoesNotContain("INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA config TO mcp", sql, StringComparison.Ordinal); + foreach (var adpLine in sql.Split('\n').Where(l => l.Contains("ALTER DEFAULT PRIVILEGES", StringComparison.Ordinal))) + { + Assert.DoesNotContain("mcp", adpLine, StringComparison.Ordinal); + } + + /* The credential column stays SELECT-carved from mcp (section 6) — mcp WRITEs a password blob but never + READs one back — so config_monitored_servers still appears in the mcp REVOKE/GRANT-column carve. */ + Assert.Contains("REVOKE SELECT ON config.config_monitored_servers FROM mcp;", sql, StringComparison.Ordinal); + } + [Fact] public void BuildProvisioningSql_McpRole_CarvesSecretColumns_LikeViewer() { diff --git a/Darling/Darling.Tests/DarlingMcpServerAdminToolsTests.cs b/Darling/Darling.Tests/DarlingMcpServerAdminToolsTests.cs new file mode 100644 index 000000000..822a6382d --- /dev/null +++ b/Darling/Darling.Tests/DarlingMcpServerAdminToolsTests.cs @@ -0,0 +1,383 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.ComponentModel; +using System.Linq; +using System.Reflection; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Server; +using Npgsql; +using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Service; +using PerformanceMonitor.Darling.Service.Mcp; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/// +/// Ungated (no-live-store) contract for the server-onboarding MCP tools (add_servers / remove_server): the tool +/// surface is EXACTLY the two names (both static, on a [McpServerToolType] class, returning Task<string>), +/// the advertised tools/list schema is Gemini-clean (#1074) with the expected required-param set, and — the +/// load-bearing safety property — a structurally-bad request (malformed JSON, empty array, an entry with a bad +/// field or an unsupported Entra/MFA auth) is REJECTED as invalid WITHOUT ever probing a server or opening a store +/// connection (proved against a dead data source + a probe seam that throws if reached). The pure dedupe partition +/// (case-folded, first-occurrence-wins) is unit-tested directly. The live store INSERT/DELETE + config_version +/// bump round-trip (probe stubbed to success) is gated below. +/// +public sealed class DarlingMcpServerAdminToolsSurfaceTests +{ + /// A dead data source (unroutable port) — proves the validate-before-write path bails on a bad request + /// WITHOUT ever opening a connection (the call returns before touching the store). + private const string DeadStore = "Host=127.0.0.1;Port=1;Username=none;Password=none;Database=none;Timeout=1"; + + /// A probe seam that FAILS the test if the pure-validation path ever reaches it — so a bad request + /// that returns before probing is proved to have skipped the network hit entirely. + private static readonly DarlingMcpServerAdminTools.ServerProbe ThrowingProbe = + (_, _) => throw new InvalidOperationException("the probe must not run for a structurally-invalid request"); + + private static readonly string[] ExpectedToolSurface = + { + "add_servers", + "remove_server", + }; + + private static MethodInfo[] ToolMethods() => typeof(DarlingMcpServerAdminTools) + .GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance) + .Where(m => m.GetCustomAttribute() is not null) + .ToArray(); + + [Fact] + public void ToolSurface_IsExactlyTheTwoServerAdminTools() + { + var toolMethods = ToolMethods(); + var names = toolMethods + .Select(m => m.GetCustomAttribute()!.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToArray(); + + Assert.Equal(ExpectedToolSurface, names); + Assert.NotNull(typeof(DarlingMcpServerAdminTools).GetCustomAttribute()); + Assert.All(toolMethods, m => Assert.True(m.IsStatic, $"{m.Name} must be static")); + Assert.All(toolMethods, m => Assert.True(m.ReturnType == typeof(Task), $"{m.Name} must return Task")); + } + + private static (string Name, bool Optional)[] McpParams(string toolName) + { + var method = ToolMethods().Single(m => m.GetCustomAttribute()!.Name == toolName); + return method.GetParameters() + .Where(p => p.GetCustomAttribute() is not null) + .Select(p => (p.Name!, p.HasDefaultValue)) + .ToArray(); + } + + [Theory] + [InlineData("add_servers", "servers_json")] + [InlineData("remove_server", "server_name")] + public void ParamContract_MatchesContract(string toolName, string expectedCsv) + { + Assert.Equal(expectedCsv.Split(','), McpParams(toolName).Select(p => p.Name).ToArray()); + } + + [Theory] + [InlineData("add_servers", "servers_json")] + [InlineData("remove_server", "server_name")] + public void ParamContract_BothTools_RequireTheirTarget(string toolName, string requiredCsv) + { + var required = McpParams(toolName).Where(p => !p.Optional).Select(p => p.Name) + .OrderBy(n => n, StringComparer.Ordinal).ToArray(); + Assert.Equal(requiredCsv.Split(',').OrderBy(n => n, StringComparer.Ordinal).ToArray(), required); + } + + private static System.Collections.Generic.Dictionary BuildToolSchemas() + { + var services = new ServiceCollection(); + services.AddSingleton(typeof(NpgsqlDataSource), _ => null!); + services.AddMcpServer().WithGeminiCompatibleTools(); + using var provider = services.BuildServiceProvider(); + return provider.GetServices().ToDictionary(t => t.ProtocolTool.Name, t => t.ProtocolTool); + } + + [Fact] + public void AdvertisedSchema_IsGeminiClean_ForBothTools() + { + var tools = BuildToolSchemas(); + Assert.Equal(2, tools.Count); + var violations = tools.Values.SelectMany(t => DarlingMcpSchemaAssert.Violations(t.Name, t.InputSchema)).ToList(); + Assert.True(violations.Count == 0, "Gemini-incompatible schema keywords leaked:\n" + string.Join("\n", violations)); + } + + [Theory] + [InlineData("add_servers", "servers_json")] + [InlineData("remove_server", "server_name")] + public void AdvertisedSchema_RequiredParams_MatchTheContract(string toolName, string expectedCsv) + { + var expected = expectedCsv.Length == 0 ? Array.Empty() : expectedCsv.Split(','); + var required = DarlingMcpSchemaAssert.RequiredOf(BuildToolSchemas()[toolName].InputSchema) + .OrderBy(n => n, StringComparer.Ordinal).ToArray(); + Assert.Equal(expected.OrderBy(n => n, StringComparer.Ordinal).ToArray(), required); + } + + /* ---------------- validate BEFORE write (no connection opened, no probe) ---------------- */ + + [Theory] + [InlineData("not json")] // not valid JSON + [InlineData("{\"host\":\"x\"}")] // a JSON object, not an array + [InlineData("[]")] // empty array + public async Task AddServers_UnusablePayload_ReturnsInvalid_WithoutTouchingStoreOrProbe(string json) + { + await using var dead = NpgsqlDataSource.Create(DeadStore); + var result = await DarlingMcpServerAdminTools.AddServersAsync(dead, json, ThrowingProbe, CancellationToken.None); + Assert.Equal("invalid", DarlingMcpTestData.StatusOf(result)); + } + + [Theory] + [InlineData("[{\"database\":\"x\"}]")] // missing host + [InlineData("[{\"host\":\"x\",\"auth\":\"Entra\"}]")] // Entra/MFA rejected + [InlineData("[{\"host\":\"x\",\"auth\":\"ManagedIdentity\"}]")] // Managed Identity rejected + [InlineData("[{\"host\":\"x\",\"auth\":\"SQL\"}]")] // SQL without username/password + [InlineData("[{\"host\":\"x\",\"auth\":\"SQL\",\"username\":\"u\"}]")] // SQL without password + [InlineData("[{\"host\":\"x\",\"encrypt_mode\":\"nope\"}]")] // bad encrypt_mode enum + [InlineData("[{\"host\":\"x\",\"trust_server_certificate\":\"yes\"}]")] // bool field, wrong type + public async Task AddServers_AllEntriesInvalid_ReturnsPerEntryInvalid_WithoutTouchingStoreOrProbe(string json) + { + /* Every entry is structurally invalid, so there is no candidate to dedupe / probe / insert — the store is + never opened (the dead store would throw) and the throwing probe is never reached. */ + await using var dead = NpgsqlDataSource.Create(DeadStore); + var result = await DarlingMcpServerAdminTools.AddServersAsync(dead, json, ThrowingProbe, CancellationToken.None); + + using var doc = JsonDocument.Parse(result); + Assert.Equal(0, doc.RootElement.GetProperty("added").GetInt32()); + Assert.Equal(0, doc.RootElement.GetProperty("skipped").GetInt32()); + Assert.Equal(1, doc.RootElement.GetProperty("failed").GetInt32()); + Assert.Equal("invalid", doc.RootElement.GetProperty("results")[0].GetProperty("status").GetString()); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public async Task RemoveServer_BlankName_ReturnsInvalid_WithoutTouchingStore(string name) + { + await using var dead = NpgsqlDataSource.Create(DeadStore); + var result = await DarlingMcpServerAdminTools.RemoveServer(dead, name); + Assert.Equal("invalid", DarlingMcpTestData.StatusOf(result)); + } + + /* ---------------- pure parse + dedupe ---------------- */ + + [Fact] + public void ParseRequest_ValidWindowsEntry_BuildsIntegratedProbeConfig_NoSecret() + { + var (entries, invalid, wholeError) = DarlingMcpServerAdminTools.ParseRequest("[{\"host\":\"sql01\"}]"); + + Assert.Null(wholeError); + Assert.Empty(invalid); + var entry = Assert.Single(entries); + Assert.Equal("sql01", entry.DisplayName); + Assert.Equal("integrated", entry.ProbeConfig.Auth); + Assert.Null(entry.PlaintextPassword); + /* Fail-closed TLS defaults are the MonitoredServer defaults. */ + Assert.Equal("Mandatory", entry.ProbeConfig.EncryptMode); + Assert.False(entry.ProbeConfig.TrustServerCertificate); + } + + [Fact] + public void ParseRequest_SqlEntry_CarriesPlaintextForProbe_AndAllExposedOptions() + { + var (entries, invalid, wholeError) = DarlingMcpServerAdminTools.ParseRequest( + "[{\"host\":\"sql02\",\"display_name\":\"Prod\",\"database\":\"AppDb\",\"auth\":\"SQL\",\"username\":\"monitor\"," + + "\"password\":\"p@ss\",\"encrypt_mode\":\"Strict\",\"trust_server_certificate\":true,\"read_only_intent\":true}]"); + + Assert.Null(wholeError); + Assert.Empty(invalid); + var entry = Assert.Single(entries); + Assert.Equal("Prod", entry.DisplayName); + Assert.Equal("sql", entry.ProbeConfig.Auth); + Assert.Equal("monitor", entry.ProbeConfig.Username); + Assert.Equal("p@ss", entry.PlaintextPassword); + Assert.Equal("AppDb", entry.ProbeConfig.Database); + Assert.Equal("Strict", entry.ProbeConfig.EncryptMode); + Assert.True(entry.ProbeConfig.TrustServerCertificate); + Assert.True(entry.ProbeConfig.ReadOnlyIntent); + /* read_only_intent flows into the storage identity key (host:RO), matching the shared identity rule. */ + Assert.Equal(ServerIdHelper.BuildStorageName("sql02", "AppDb", true), entry.StorageKey); + } + + [Fact] + public void ParseRequest_MixedBatch_SplitsValidFromInvalid_PreservingOrder() + { + var (entries, invalid, wholeError) = DarlingMcpServerAdminTools.ParseRequest( + "[{\"host\":\"good1\"},{\"auth\":\"SQL\"},{\"host\":\"good2\"}]"); + + Assert.Null(wholeError); + Assert.Equal(2, entries.Count); + var bad = Assert.Single(invalid); + Assert.Equal("invalid", bad.Status); + Assert.Equal(1, bad.Order); // the second element (index 1) + } + + [Fact] + public void PartitionDuplicates_CaseVariantEntries_CollapseToOneAdded_OneDuplicate() + { + /* "SQL2016" and "sql2016" build the SAME storage identity under OrdinalIgnoreCase (BuildStorageName is + case-preserving; the gate is case-folded), so the second is a duplicate of the first in-batch. */ + var (entries, _, _) = DarlingMcpServerAdminTools.ParseRequest("[{\"host\":\"SQL2016\"},{\"host\":\"sql2016\"}]"); + Assert.Equal(2, entries.Count); + Assert.Equal(entries[0].StorageKey, entries[1].StorageKey, StringComparer.OrdinalIgnoreCase); + + var (ready, duplicates) = DarlingMcpServerAdminTools.PartitionDuplicates(entries, Array.Empty()); + + Assert.Single(ready); + var dup = Assert.Single(duplicates); + Assert.Equal("duplicate", dup.Status); + Assert.Equal(1, dup.Order); // the second entry is the duplicate + } + + [Fact] + public void PartitionDuplicates_EntryMatchingAnExistingServer_IsSkipped() + { + var (entries, _, _) = DarlingMcpServerAdminTools.ParseRequest("[{\"host\":\"sql2019\"}]"); + + /* Seed the gate with the SAME server (a case variant of the existing store key) → no ready, one duplicate. */ + var (ready, duplicates) = DarlingMcpServerAdminTools.PartitionDuplicates(entries, new[] { "SQL2019" }); + + Assert.Empty(ready); + Assert.Single(duplicates); + } +} + +/// +/// Gated (DARLING_TEST_PG) live round-trip for the server-onboarding MCP tools against a real PostgreSQL — the +/// probe is STUBBED to success (no live SQL Server), so this exercises the STORE side: add_servers INSERTs the +/// config_monitored_servers rows (SQL-auth password DPAPI-encrypted at rest, Windows-auth server secret-free), +/// the case-folded duplicate is skipped, the write self-bumps config_version (the reload beacon) via the existing +/// trigger, and remove_server resolves + DELETEs. Own-scoped per the shared-store doctrine (GUID-suffixed hosts + +/// a finally cleanup). No live SQL Server connection is made in CI — the real end-to-end probe is what the human +/// dogfoods (remove sql2016, re-add via MCP). +/// +[Collection("live-postgres")] +public sealed class DarlingMcpServerAdminToolsLivePostgresTests +{ + private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + + /// The stubbed probe — a healthy on-prem Enterprise box; no live SQL Server is touched. + private static readonly DarlingMcpServerAdminTools.ServerProbe SuccessProbe = + (_, _) => Task.FromResult(new ConnectionProbeResult( + Success: true, MajorVersion: 15, EngineEdition: 3, EngineEditionDescription: "Enterprise", + IsAzureSqlDb: false, IsAzureManagedInstance: false, IsAwsRds: false, HasMsdbAccess: true, Error: null)); + + [Fact] + public async Task AddServers_InsertsEncryptsDedupesBumpsVersion_ThenRemoveServer_AgainstDevPostgres() + { + var cs = ConnectionString; + Assert.SkipWhen(string.IsNullOrEmpty(cs), + "Set DARLING_TEST_PG to a Postgres connection string (owner/superuser) to run the server-admin MCP tools live test."); + + var ct = TestContext.Current.CancellationToken; + using var connection = new NpgsqlConnection(cs); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + + /* Explicit search_path so the tools' bare config_monitored_servers / servers resolve regardless of the + store's database default (mirrors the other live tool tests). */ + var dataSourceConnectionString = new NpgsqlConnectionStringBuilder(cs) + { + SearchPath = "collect,config,public", + }.ConnectionString; + await using var postgres = NpgsqlDataSource.Create(dataSourceConnectionString); + + var suffix = Guid.NewGuid().ToString("N")[..12]; + var sqlHost = "mcp-add-sql-" + suffix; + var winHost = "mcp-add-win-" + suffix; + var sqlId = ServerIdHelper.GetDeterministicHashCode(ServerIdHelper.BuildStorageName(sqlHost, null, false)); + var winId = ServerIdHelper.GetDeterministicHashCode(ServerIdHelper.BuildStorageName(winHost, null, false)); + var password = "P@ss-" + Guid.NewGuid().ToString("N"); + + await CleanupAsync(connection, ct, sqlId, winId); + await DarlingMcpTestData.ExecAsync(connection, ct, "INSERT INTO config_service (id) VALUES (1) ON CONFLICT (id) DO NOTHING"); + + try + { + var versionBefore = Convert.ToInt64(await ScalarAsync(connection, ct, "SELECT config_version FROM config_service WHERE id = 1")); + + /* SQL-auth server + Windows-auth server + a case-variant DUPLICATE of the Windows host. */ + var json = + $"[{{\"host\":\"{sqlHost}\",\"auth\":\"SQL\",\"username\":\"monitor\",\"password\":\"{password}\"," + + $"\"encrypt_mode\":\"Strict\",\"trust_server_certificate\":true}}," + + $"{{\"host\":\"{winHost}\"}}," + + $"{{\"host\":\"{winHost.ToUpperInvariant()}\"}}]"; + var added = await DarlingMcpServerAdminTools.AddServersAsync(postgres, json, SuccessProbe, ct); + using (var doc = JsonDocument.Parse(added)) + { + Assert.Equal(2, doc.RootElement.GetProperty("added").GetInt32()); + Assert.Equal(1, doc.RootElement.GetProperty("skipped").GetInt32()); + Assert.Equal(0, doc.RootElement.GetProperty("failed").GetInt32()); + } + + /* The config_monitored_servers write self-bumped config_version (the service's reload beacon) via the + existing trg_bump_monitored_servers trigger — proving the mcp beacon column-grant covers this write. */ + var versionAfter = Convert.ToInt64(await ScalarAsync(connection, ct, "SELECT config_version FROM config_service WHERE id = 1")); + Assert.True(versionAfter > versionBefore, "config_version should self-bump on a config_monitored_servers write"); + + /* SQL server: the password is DPAPI-ENCRYPTED at rest (not plaintext) and round-trips; the exposed TLS + options + auth landed as sent. (Darling.Tests is net10.0-windows, so DPAPI is available here.) */ + var storedSecret = await ScalarAsync(connection, ct, $"SELECT encrypted_password FROM config_monitored_servers WHERE server_id = {sqlId}") as string; + Assert.False(string.IsNullOrEmpty(storedSecret)); + Assert.NotEqual(password, storedSecret); + Assert.Equal(password, DarlingSecrets.Unprotect(storedSecret!)); + Assert.Equal("sql", await ScalarAsync(connection, ct, $"SELECT auth FROM config_monitored_servers WHERE server_id = {sqlId}") as string); + Assert.Equal("Strict", await ScalarAsync(connection, ct, $"SELECT encrypt_mode FROM config_monitored_servers WHERE server_id = {sqlId}") as string); + Assert.True((bool)(await ScalarAsync(connection, ct, $"SELECT trust_server_certificate FROM config_monitored_servers WHERE server_id = {sqlId}"))!); + + /* Windows server: integrated auth, NO stored secret. */ + Assert.Equal("integrated", await ScalarAsync(connection, ct, $"SELECT auth FROM config_monitored_servers WHERE server_id = {winId}") as string); + Assert.True(await ScalarAsync(connection, ct, $"SELECT encrypted_password FROM config_monitored_servers WHERE server_id = {winId}") is null or DBNull); + + /* Re-adding both existing servers → all duplicates, nothing added. */ + var again = await DarlingMcpServerAdminTools.AddServersAsync( + postgres, $"[{{\"host\":\"{sqlHost}\"}},{{\"host\":\"{winHost}\"}}]", SuccessProbe, ct); + using (var doc = JsonDocument.Parse(again)) + { + Assert.Equal(0, doc.RootElement.GetProperty("added").GetInt32()); + Assert.Equal(2, doc.RootElement.GetProperty("skipped").GetInt32()); + } + + /* remove_server resolves against the servers registry, so register the SQL host there (same server_id + config_monitored_servers keys on), then remove — the config row is deleted. */ + await DarlingMcpTestData.RegisterServerAsync(connection, sqlId, sqlHost, ct); + Assert.Equal("removed", DarlingMcpTestData.StatusOf(await DarlingMcpServerAdminTools.RemoveServer(postgres, sqlHost))); + Assert.Equal(0L, Convert.ToInt64(await ScalarAsync(connection, ct, $"SELECT count(*) FROM config_monitored_servers WHERE server_id = {sqlId}"))); + + /* Remove again: the servers-registry row still resolves, but the config row is gone → not_found. */ + Assert.Equal("not_found", DarlingMcpTestData.StatusOf(await DarlingMcpServerAdminTools.RemoveServer(postgres, sqlHost))); + + /* A name that does not resolve at all → not_found. */ + Assert.Equal("not_found", DarlingMcpTestData.StatusOf(await DarlingMcpServerAdminTools.RemoveServer(postgres, "no-such-server-" + suffix))); + } + finally + { + await CleanupAsync(connection, ct, sqlId, winId); + } + } + + private static async Task ScalarAsync(NpgsqlConnection connection, CancellationToken ct, string sql) + { + using var command = new NpgsqlCommand(sql, connection); + return await command.ExecuteScalarAsync(ct); + } + + private static async Task CleanupAsync(NpgsqlConnection connection, CancellationToken ct, int sqlId, int winId) + { + await DarlingMcpTestData.ExecAsync(connection, ct, $"DELETE FROM config_monitored_servers WHERE server_id IN ({sqlId}, {winId})"); + await DarlingMcpTestData.ExecAsync(connection, ct, $"DELETE FROM servers WHERE server_id IN ({sqlId}, {winId})"); + } +} diff --git a/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs b/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs index 2118069b3..defbf7b4e 100644 --- a/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs +++ b/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs @@ -323,6 +323,17 @@ await ExecAsync(mcp, await ExecAsync(mcp, "INSERT INTO config.config_mute_rules (id, enabled, created_at_utc) VALUES ('sec-mcp-mute', true, now())", ct); await ExecAsync(mcp, "DELETE FROM config.config_mute_rules WHERE id = 'sec-mcp-mute'", ct); + /* The mcp server-onboarding writes (add_servers / remove_server): full CRUD on config_monitored_servers, + proven by a real INSERT/UPDATE/DELETE round-trip as the mcp role (own-scoped sentinel server_id). The + INSERT fires trg_bump_monitored_servers -> config_bump_version AS mcp, so it doubles as proof the + section-8 config_service beacon column-grant covers THIS write too (no extra config_service grant). */ + Assert.True(await HasPrivAsync(mcp, "config.config_monitored_servers", "INSERT", ct)); + Assert.True(await HasPrivAsync(mcp, "config.config_monitored_servers", "UPDATE", ct)); + Assert.True(await HasPrivAsync(mcp, "config.config_monitored_servers", "DELETE", ct)); + await ExecAsync(mcp, "INSERT INTO config.config_monitored_servers (server_id, name, host) VALUES (-424242, 'sec-mcp-onboard', 'sec-mcp-onboard-host')", ct); + await ExecAsync(mcp, "UPDATE config.config_monitored_servers SET is_enabled = FALSE WHERE server_id = -424242", ct); + await ExecAsync(mcp, "DELETE FROM config.config_monitored_servers WHERE server_id = -424242", ct); + /* The load-bearing beacon proof: an UPDATE on config_alert_settings as the mcp role fires the statement-level bump trigger, which UPDATEs config_service.config_version AS the mcp role (the trigger function is SECURITY INVOKER). Without the column-level config_service beacon grant this @@ -349,6 +360,7 @@ await ExecAsync(mcp, /* Belt-and-suspenders cleanup (the DELETEs above already removed these on the happy path). */ await ExecAsync(owner, $"DELETE FROM config.custom_views WHERE name = '{viewName}'", ct); await ExecAsync(owner, "DELETE FROM config.config_mute_rules WHERE id = 'sec-mcp-mute'", ct); + await ExecAsync(owner, "DELETE FROM config.config_monitored_servers WHERE server_id = -424242", ct); await DropTestRolesAsync(owner, ct); } } @@ -495,7 +507,11 @@ IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = '{McpRole}') THEN -- config_service.config_version AS the mcp role). Mirrors DarlingManagedRoles section 8. GRANT INSERT, UPDATE, DELETE ON config.config_mute_rules TO {McpRole}; GRANT UPDATE ON config.config_alert_settings TO {McpRole}; -GRANT UPDATE (config_version, updated_at) ON config.config_service TO {McpRole};"; +GRANT UPDATE (config_version, updated_at) ON config.config_service TO {McpRole}; +-- The mcp server-onboarding writes (add_servers / remove_server): CRUD on the single config_monitored_servers +-- table. Mirrors DarlingManagedRoles section 9. The beacon is already covered by the config_service column grant +-- above (a config_monitored_servers write fires the same SECURITY-INVOKER bump trigger). +GRANT INSERT, UPDATE, DELETE ON config.config_monitored_servers TO {McpRole};"; await ExecAsync(owner, ddl, ct); } diff --git a/Darling/Darling.Tests/DarlingWebEndpointsTests.cs b/Darling/Darling.Tests/DarlingWebEndpointsTests.cs index 1dce4d380..4680a49bb 100644 --- a/Darling/Darling.Tests/DarlingWebEndpointsTests.cs +++ b/Darling/Darling.Tests/DarlingWebEndpointsTests.cs @@ -84,15 +84,16 @@ public void ExcludedToolNames_AreTheNonReadSurfaceTools() { /* The six original non-read tools (analyze_server, the mute write, the four analyze_*_plan), the eight Custom Views tools (#1599 + describe_custom_view_catalog) served by /api/views + /api/compose/run + - /api/catalog, and the three alert-tuning WRITE tools (no read endpoint, like mute_analysis_finding) — - none is a /api/read/{tool} 1:1 mirror. */ + /api/catalog, the three alert-tuning WRITE tools, and the two server-onboarding WRITE tools + (add_servers / remove_server) — all writes with no read endpoint, like mute_analysis_finding; none is a + /api/read/{tool} 1:1 mirror. */ Assert.Equal( new[] { - "analyze_plan_xml", "analyze_procedure_plan", "analyze_query_plan", "analyze_query_store_plan", "analyze_server", - "create_custom_view", "create_mute_rule", "delete_custom_view", "delete_mute_rule", "describe_custom_view_catalog", - "get_custom_view", "list_custom_views", "mute_analysis_finding", "run_custom_view_panel", "update_alert_settings", - "update_custom_view", "validate_custom_view", + "add_servers", "analyze_plan_xml", "analyze_procedure_plan", "analyze_query_plan", "analyze_query_store_plan", + "analyze_server", "create_custom_view", "create_mute_rule", "delete_custom_view", "delete_mute_rule", + "describe_custom_view_catalog", "get_custom_view", "list_custom_views", "mute_analysis_finding", "remove_server", + "run_custom_view_panel", "update_alert_settings", "update_custom_view", "validate_custom_view", }, DarlingWebEndpoints.ExcludedToolNames.OrderBy(n => n, StringComparer.Ordinal).ToArray()); } diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs index 624b162f3..9e6259bff 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs @@ -40,9 +40,12 @@ namespace PerformanceMonitor.Darling.Service; /// collect + config-minus-the-secret-columns) PLUS a NARROW, enumerated set of writes — /// INSERT on collect.analysis_findings and config.analysis_muted (what analyze_server /// persists + the mute tool need), INSERT/UPDATE/DELETE on config.custom_views (the -/// custom-view tools, #1599), and the alert-tuning writes (INSERT/UPDATE/DELETE on +/// custom-view tools, #1599), the alert-tuning writes (INSERT/UPDATE/DELETE on /// config.config_mute_rules + UPDATE on the singleton config.config_alert_settings, plus the -/// two beacon columns of config.config_service so the settings write's self-bump trigger can fire). +/// two beacon columns of config.config_service so the settings write's self-bump trigger can fire), +/// and the server-onboarding writes (INSERT/UPDATE/DELETE on config.config_monitored_servers for the +/// add_servers/remove_server tools — a single non-secret-KEY table; the credential column stays +/// SELECT-carved, so mcp can WRITE a password blob but never READ one back). /// Deliberately NOT admin: a token-holder reachable over the network must never get the /// config_command service-credential pivot or the secret columns. Every write grant is an EXPLICIT /// single-table (or single-column) statement with NO ALTER DEFAULT PRIVILEGES (ADP has no per-table @@ -195,7 +198,7 @@ public static async Task EnsureProvisionedAsync( await command.ExecuteNonQueryAsync(cancellationToken); logger.LogInformation( - "Least-privilege roles ready (admin: read both schemas + write config; viewer: read-only + write config.custom_views; mcp: viewer's reads + INSERT on analysis_findings/analysis_muted + write config.custom_views + tune alerting (config_mute_rules, config_alert_settings, config_service reload beacon)) — the Viewer and MCP host no longer connect as the superuser"); + "Least-privilege roles ready (admin: read both schemas + write config; viewer: read-only + write config.custom_views; mcp: viewer's reads + INSERT on analysis_findings/analysis_muted + write config.custom_views + tune alerting (config_mute_rules, config_alert_settings, config_service reload beacon) + onboard servers (config_monitored_servers)) — the Viewer and MCP host no longer connect as the superuser"); } /// @@ -447,6 +450,18 @@ ALTER DEFAULT PRIVILEGES FOR ROLE {owner} IN SCHEMA {config} GRANT INSERT, UPDATE, DELETE ON {config}.config_mute_rules TO {mcp}; GRANT UPDATE ON {config}.config_alert_settings TO {mcp}; GRANT UPDATE (config_version, updated_at) ON {config}.config_service TO {mcp}; + +-- 9. Server onboarding (the MCP server-admin write tools): the mcp role's monitored-server writes, mirroring +-- sections 7/8's model (an EXPLICIT single-table statement, NO ALTER DEFAULT PRIVILEGES). add_servers / +-- remove_server let a token-holder add or remove monitored servers in the SAME central store the Viewer's +-- Add / Manage-Servers dialogs write: INSERT/UPDATE/DELETE on config_monitored_servers. Still NARROW -- a +-- single non-secret-KEY table (the encrypted_password column is SELECT-carved from mcp by the section-6 +-- secret-column ACL above, so mcp can WRITE a credential blob but never READ one back), never the +-- config_command service-credential pivot or a schema-wide config write. The BEACON is already covered: a +-- config_monitored_servers write fires trg_bump_monitored_servers -> config_bump_version (SECURITY INVOKER), +-- which UPDATEs config_service.config_version AS mcp, and section 8 already granted mcp +-- UPDATE (config_version, updated_at) ON config_service -- so no additional config_service grant is needed here. +GRANT INSERT, UPDATE, DELETE ON {config}.config_monitored_servers TO {mcp}; "; } diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs index 38012a8d9..ab326c5eb 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs @@ -57,7 +57,8 @@ public static class DarlingWebEndpoints /// richer web endpoints (/api/views CRUD + /api/compose/run + the /api/catalog compose /// vocabulary that describe_custom_view_catalog mirrors), not a /api/read/{tool} query-string mirror; /// and the alert-tuning tools (update_alert_settings / create_mute_rule / delete_mute_rule) - /// WRITE the alert config, so — like mute_analysis_finding — they have no read endpoint. + /// WRITE the alert config, and the server-onboarding tools (add_servers / remove_server) WRITE the + /// monitored-server registry, so — like mute_analysis_finding — they have no read endpoint. public static readonly IReadOnlySet ExcludedToolNames = new HashSet(StringComparer.Ordinal) { "analyze_server", @@ -77,6 +78,8 @@ public static class DarlingWebEndpoints "update_alert_settings", "create_mute_rule", "delete_mute_rule", + "add_servers", + "remove_server", }; /// The window (hours) the fleet card blocking / deadlock counts default to — the WPF Overview's window. diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs index b886b4941..3a279943f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHostService.cs @@ -487,7 +487,18 @@ views in config.custom_views through the SAME CustomViewStore + ValidateDefiniti runner the web viewer's editor uses (no divergent second impl), and run back a composed panel's data for a self-test loop. The mcp role carries the narrow INSERT/UPDATE/DELETE grant on ONLY config.custom_views (mirroring viewer's) — never the config pivot or the secret columns. */ - .WithGeminiCompatibleTools(); + .WithGeminiCompatibleTools() + /* The server-onboarding WRITE tools — add_servers (BULK) / remove_server: an MCP client can stand up + or tear down FLEET monitoring conversationally. The service-side twin of the Viewer's Add / Add- + Multiple dialogs: add_servers validates each entry, probes the connection IN-PROCESS (the service + holds the network path + credentials, so no test_connect command plane is needed), skips + case-folded duplicates via the shared ServerIdHelper identity, DPAPI-encrypts the SQL password + (the service identity, so it round-trips at collection time), and INSERTs config.config_monitored_ + servers mirroring StoreConfigProvider.SeedMonitoredServersAsync; remove_server DELETEs by the same + resolver the read tools use. The mcp role carries the narrow INSERT/UPDATE/DELETE grant on ONLY + config.config_monitored_servers (the encrypted_password column stays SELECT-carved) — never the + config pivot or a schema-wide write. */ + .WithGeminiCompatibleTools(); _app = builder.Build(); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs index b401790a3..6bc3a232a 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpInstructions.cs @@ -10,9 +10,9 @@ namespace PerformanceMonitor.Darling.Service.Mcp; /// /// Server instructions sent to MCP clients during initialization — Lite's McpInstructions -/// framing (read-only posture, collection-freshness notes, tool reference) scoped to the ~85 -/// analysis + plan-analysis + data-read tools (plus the Custom Views + alert-tuning write surfaces) -/// this headless service exposes (the body enumerates them). +/// framing (read-only posture, collection-freshness notes, tool reference) scoped to the ~87 +/// analysis + plan-analysis + data-read tools (plus the Custom Views + alert-tuning + server-onboarding +/// write surfaces) this headless service exposes (the body enumerates them). /// internal static class DarlingMcpInstructions { @@ -41,7 +41,7 @@ internal static class DarlingMcpInstructions ## Tool Reference - This server exposes eighty-five tools. Seventy-four are the same names Performance Monitor Lite and the Dashboard expose: six diagnostic-analysis tools, five plan-analysis tools, fifteen core data-read tools, twenty-one diagnostic-depth data-read tools, eight resource-contention + jobs data-read tools, five trend data-read tools, eight system-health parse-on-read tools, five alert + health-overview tools, and one Default Trace tool. The remaining eleven are unique to Darling's central store: eight are the Custom Views tools (seven manage the saved views — the one view-authoring write surface — and `describe_custom_view_catalog` returns the read-only compose vocabulary those authoring tools draw from), and three are alert-tuning write tools (`update_alert_settings` tunes the alert engine's thresholds; `create_mute_rule` / `delete_mute_rule` manage the mute rules) that write only the shared alert configuration in the monitoring store. Every data-read tool reads the data the collectors already captured into the store — a stored read, never a live query against the monitored server. + This server exposes eighty-seven tools. Seventy-four are the same names Performance Monitor Lite and the Dashboard expose: six diagnostic-analysis tools, five plan-analysis tools, fifteen core data-read tools, twenty-one diagnostic-depth data-read tools, eight resource-contention + jobs data-read tools, five trend data-read tools, eight system-health parse-on-read tools, five alert + health-overview tools, and one Default Trace tool. The remaining thirteen are unique to Darling's central store: eight are the Custom Views tools (seven manage the saved views — the one view-authoring write surface — and `describe_custom_view_catalog` returns the read-only compose vocabulary those authoring tools draw from), three are alert-tuning write tools (`update_alert_settings` tunes the alert engine's thresholds; `create_mute_rule` / `delete_mute_rule` manage the mute rules) that write only the shared alert configuration in the monitoring store, and two are server-onboarding write tools (`add_servers` bulk-adds monitored servers; `remove_server` removes one) that add or remove rows in the monitoring store's monitored-server registry. Every data-read tool reads the data the collectors already captured into the store — a stored read, never a live query against the monitored server. ### Diagnostic-analysis tools @@ -203,6 +203,17 @@ internal static class DarlingMcpInstructions | `delete_custom_view` | Deletes a view by id (permanent) | `view_id` (required) | | `run_custom_view_panel` | Compiles + runs a single composed panel and returns `{sql, rows, annotations}` — the composer's live preview, for checking a panel's data before saving | `spec` (required — a JSON object `{panel, variables?, values?, server?, hours?}`) | + ### Server onboarding (add & remove) + + Two write tools stand up or tear down FLEET monitoring conversationally — the service-side twin of the WPF viewer's Add / Manage Servers dialogs. They write ONLY the monitoring store's monitored-server registry (`config.config_monitored_servers`); neither runs anything on a monitored SQL Server beyond a one-time connection probe, and neither touches the collected performance data. A change is picked up by the running service within one collection sweep (no restart). + + | Tool | Purpose | Key Parameters | + |------|---------|----------------| + | `add_servers` | BULK-adds monitored servers: pass a JSON ARRAY of server objects and each is validated, connection-tested IN the service, and (if new and reachable) saved. Per object: `host` (required), `display_name`, `database` (one DB only, e.g. an Azure SQL Database), `auth` (`Windows`/`SQL`, default `Windows`), `username`+`password` (required for `SQL`), `encrypt_mode` (`Optional`/`Mandatory`/`Strict`, default `Mandatory`), `trust_server_certificate` (default false), `read_only_intent`, `multi_subnet_failover`. Servers are processed in order; a duplicate (case-folded, vs existing or an earlier entry) is `duplicate`, an unreachable server is `connection_failed` (the batch continues), Entra/MFA/Service-Principal/Managed-Identity auth is `invalid` (Windows/SQL only). Returns `{added, skipped, failed, results:[{server, status, detail}]}` | `servers_json` (required — a JSON array of server objects) | + | `remove_server` | Removes a monitored server by name (resolved like every `server_name`). Returns `{status:"removed", server}` or `{status:"not_found"}`. Already-collected history is NOT deleted | `server_name` (required) | + + A SQL password is encrypted at rest (DPAPI, the service identity) and is NEVER returned by a read tool. It DOES travel to this endpoint inside `add_servers`' request JSON, so on a LAN deployment reach the endpoint only through the documented TLS reverse proxy. + The three config-change tools diff the store's config snapshots. This edition captures configuration WHEN THE SERVICE CONNECTS to a server (not on a fixed schedule), so a change is detected between two connect snapshots and at least two are needed — a stable, always-connected deployment may show no changes until the next connect. They emit only the values the collectors capture; the Dashboard's `requires_restart` / setting `description` / `setting_type` / generated change-narrative enrichment is not collected here and is omitted. `get_blocking_deadlock_stats` (the Dashboard's blocking/deadlock aggregate) is NOT hosted: this edition has no blocking/deadlock rollup table — use `get_blocking` / `get_deadlocks` for the raw events. Note on `next_tools`: analyze_server findings include `next_tools` recommendations. Most are hosted on this server — the plan-analysis tools (`analyze_query_plan`, `analyze_query_store_plan`) and the data-read tools listed above (`get_wait_stats`, `get_top_queries_by_cpu`, `get_cpu_utilization`, `get_memory_stats`, `get_file_io_stats`, `get_tempdb_trend`, `get_blocking`, `get_deadlocks`, `get_waiting_tasks`, `get_active_queries`, ...) — so follow those here. `get_top_queries_by_cpu` / `get_top_procedures_by_cpu` / `get_query_store_top` are where the `query_hash` / `sql_handle` / `query_id` + `plan_id` keys for the plan-analysis tools come from. The resource-contention + jobs tools (`get_latch_stats`, `get_spinlock_stats`, `get_resource_semaphore`, `get_memory_grants`, `get_plan_cache_bloat`, `get_cpu_scheduler_pressure`, `get_running_jobs`), the trend siblings (`get_memory_trend`, `get_perfmon_trend`, `get_file_io_trend`, `get_query_trend`, `get_query_duration_trend`), the `get_health_parser_*` system-health family, and the blocking/deadlock trend + memory-pressure reads (`get_blocking_trend`, `get_deadlock_trend`, `get_memory_pressure_events`) are all hosted here too — follow those `next_tools` on this server. Two `next_tools` names differ from what this edition hosts: `get_blocked_process_reports` (a Lite name) is served here as `get_blocked_process_xml` (with `get_blocking` for a quick overview), and `get_blocking_deadlock_stats` (the Dashboard's blocking/deadlock rollup) is not hosted at all — use `get_blocking` / `get_deadlocks` instead. diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpServerAdminTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpServerAdminTools.cs new file mode 100644 index 000000000..2f4e14801 --- /dev/null +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpServerAdminTools.cs @@ -0,0 +1,564 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using ModelContextProtocol.Server; +using Npgsql; +using NpgsqlTypes; +using PerformanceMonitor.Common; + +#pragma warning disable CA1707 // MCP tools use snake_case naming convention + +namespace PerformanceMonitor.Darling.Service.Mcp; + +/// +/// The server-onboarding MCP tools — add_servers (BULK) and remove_server — the Darling-only WRITE +/// surface that lets an MCP client (an LLM assistant) stand up or tear down FLEET monitoring conversationally: +/// "monitor these twenty servers with this login" writes twenty config.config_monitored_servers rows the +/// running service reconciles into its collection set on the next reload beacon. This is the service-side twin of +/// the Viewer's Add / Add-Multiple dialogs (AddServerDialog / AddMultipleServersDialog), and the +/// direct sibling of the #1600 Custom Views and #1608 alert-tuning MCP write tools. +/// +/// No divergent second implementation. Every authority is REUSED, never re-invented: the connection +/// probe is run IN-PROCESS (the MCP host lives inside the service, +/// which holds the network path + credentials — unlike the Viewer, whose dialogs enqueue a test_connect +/// command for the service to run); the case-folded dedupe gate is the shared +/// identity in a HashSet<string>(OrdinalIgnoreCase) exactly as the bulk dialog (#1549) uses; the SQL +/// password is DPAPI-encrypted through (the SAME service identity that +/// decrypts it during collection, so it round-trips) and NEVER stored, logged, or echoed in plaintext; and the +/// INSERT mirrors StoreConfigProvider.SeedMonitoredServersAsync's exact column set + server_id = +/// ServerIdHelper.GetDeterministicHashCode(StorageName) identity, so a tool-written row JOINs the collected +/// data and the service's reconcile matches it. +/// +/// add_servers processes its JSON array SEQUENTIALLY (mirroring #1549's Darling bulk probe, which is +/// serial to avoid a probe storm): for each server it validates the fields, skips an exact/case-variant DUPLICATE +/// of an existing or earlier-in-batch server (status:"duplicate"), probes the connection (a failure is +/// status:"connection_failed" and does NOT abort the batch), DPAPI-encrypts the SQL password, and INSERTs +/// the row (status:"added"). Windows/integrated auth stores no secret; Entra/MFA/Service-Principal/Managed- +/// Identity auth is rejected (status:"invalid") — interactive MFA is nonsensical headless, the same belt the +/// bulk dialog applies. The whole call returns {added, skipped, failed, results:[...]}. remove_server +/// resolves a name through the SAME the read tools use and DELETEs the +/// config.config_monitored_servers row. +/// +/// Security. These tools connect (like every MCP tool) as the least-privilege mcp role, granted +/// (see ) INSERT/UPDATE/DELETE on config.config_monitored_servers — a single +/// non-secret-key table (the DPAPI password blob is written but the encrypted_password column is SELECT- +/// carved from mcp, so a token-holder can WRITE a credential but never READ one back) — and nothing else, so +/// it still cannot reach the config_command service-credential pivot or the carved secret columns. The +/// config_monitored_servers write fires the existing trg_bump_monitored_servers → config_bump_version +/// trigger (SECURITY INVOKER), which the #1608 config_service beacon column-grant already covers, so the +/// running service hot-reloads the monitored set within one sweep. The SQL password transits the MCP endpoint in +/// the request JSON; on a LAN deployment front the endpoint with the documented TLS reverse proxy. +/// +[McpServerToolType] +public sealed class DarlingMcpServerAdminTools +{ + /// The connect-and-probe seam — in production + /// ( in-process), a stub in the unit tests so the store-write / + /// dedupe / encrypt / result-aggregation logic can be exercised WITHOUT a real SQL Server. + internal delegate Task ServerProbe(MonitoredServer server, CancellationToken cancellationToken); + + private static Task DefaultProbeAsync(MonitoredServer server, CancellationToken cancellationToken) => + DarlingServerConnector.ProbeAsync(server, null, cancellationToken); + + [McpServerTool(Name = "add_servers"), Description( + "Adds one or more SQL Servers to the fleet the Darling service monitors — BULK onboarding: pass a JSON " + + "ARRAY of server objects and each is validated, connection-tested, and (if new and reachable) saved to the " + + "central monitoring store, which the running service picks up within one collection sweep (no restart). " + + "Each object: host (REQUIRED); display_name (optional, defaults to host); database (optional — set it only " + + "to monitor a single database, e.g. one Azure SQL Database); auth (\"Windows\" for integrated security or " + + "\"SQL\" for a SQL login — default \"Windows\"); username + password (REQUIRED for \"SQL\" auth, ignored for " + + "\"Windows\"); encrypt_mode (\"Optional\"|\"Mandatory\"|\"Strict\", default \"Mandatory\"); " + + "trust_server_certificate (bool, default false — set true to accept a self-signed server cert); " + + "read_only_intent (bool, default false); multi_subnet_failover (bool, default false). Servers are processed " + + "IN ORDER, one at a time. A case-variant or exact duplicate of an already-monitored server (or an earlier " + + "entry in the same array) is skipped as status \"duplicate\". A server that fails to connect is recorded as " + + "status \"connection_failed\" and does NOT stop the rest of the batch. Microsoft Entra / MFA / Service " + + "Principal / Managed Identity auth is rejected (status \"invalid\") — the service connects with Windows or " + + "SQL authentication only. A SQL password is encrypted at rest (DPAPI, the service identity) and is never " + + "returned. Returns {added:N, skipped:N, failed:N, results:[{server, status:\"added\"|\"duplicate\"|" + + "\"connection_failed\"|\"invalid\", detail}]}. NOTE: the password travels to this endpoint in the request; " + + "on a LAN use the documented TLS reverse proxy.")] + public static Task AddServers( + NpgsqlDataSource postgres, + [Description("A JSON ARRAY of server objects to add (see the tool description for the per-object fields), e.g. [{\"host\":\"sql01\",\"auth\":\"SQL\",\"username\":\"monitor\",\"password\":\"...\",\"encrypt_mode\":\"Mandatory\",\"trust_server_certificate\":true},{\"host\":\"sql02\"}].")] string servers_json) => + AddServersAsync(postgres, servers_json, DefaultProbeAsync, CancellationToken.None); + + /// The testable core of add_servers: validates + dedupes + probes (through the injected + /// seam) + encrypts + INSERTs, aggregating a per-server result. Structural validation + /// runs BEFORE any store access, and when NO structurally-valid candidate remains the store is never opened — + /// so a call whose entries are all invalid (bad field, MFA auth) returns without a connection or a probe. + internal static async Task AddServersAsync( + NpgsqlDataSource postgres, string servers_json, ServerProbe probe, CancellationToken cancellationToken) + { + try + { + var (entries, invalidResults, wholeError) = ParseRequest(servers_json); + if (wholeError != null) + { + return Outcome("invalid", wholeError); + } + + var results = new List(invalidResults); + + /* No structurally-valid candidate → never open the store (validate-before-write); the aggregate below + reports only the per-entry invalids. */ + if (entries.Count == 0) + { + return Aggregate(results); + } + + /* Seed the case-folded dedupe gate from the authoritative store rows FIRST, then partition the batch — + a duplicate (of an existing server OR an earlier entry in this batch, first occurrence wins) is + skipped WITHOUT a probe, exactly as the bulk dialog (#1549) does. */ + var existingKeys = await LoadExistingStorageKeysAsync(postgres, cancellationToken); + var (ready, duplicates) = PartitionDuplicates(entries, existingKeys); + results.AddRange(duplicates); + + foreach (var entry in ready) + { + /* Validate the connection IN-PROCESS (the service holds the network path + credentials). A failure + is recorded and the batch CONTINUES — one unreachable server never aborts the rest. */ + var probeResult = await probe(entry.ProbeConfig, cancellationToken); + if (!probeResult.Success) + { + results.Add(new ServerResult(entry.Order, entry.DisplayName, "connection_failed", + string.IsNullOrWhiteSpace(probeResult.Error) + ? "Could not connect to the server." + : $"Could not connect: {probeResult.Error}")); + continue; + } + + /* DPAPI-encrypt the SQL password for storage (the service identity encrypts here and decrypts it + during collection, so it round-trips); Windows-auth servers store no secret. The plaintext never + leaves this method — it is not logged, not echoed in a result. */ + var encryptedPassword = ProtectPasswordForStorage(entry.PlaintextPassword); + await InsertServerAsync(postgres, entry, encryptedPassword, cancellationToken); + results.Add(new ServerResult(entry.Order, entry.DisplayName, "added", DescribeProbe(probeResult))); + } + + return Aggregate(results); + } + catch (Exception ex) + { + return McpHelpers.FormatError("add_servers", ex); + } + } + + [McpServerTool(Name = "remove_server"), Description( + "Removes a monitored SQL Server from the fleet by name (the display name or address, resolved the same way " + + "the read tools resolve server_name — exact match first, then partial, against the storage name and the " + + "display name). Deletes the server's definition from the central monitoring store; the running service " + + "drops it from its collection set within one sweep. Already-collected historical data is NOT deleted. " + + "Returns {status:\"removed\", server} on success, or {status:\"not_found\", ...} when no monitored server " + + "matches the name.")] + public static async Task RemoveServer( + NpgsqlDataSource postgres, + [Description("The name of the monitored server to remove — its display name or address (as list_servers / get_alert_history report it).")] string server_name) + { + try + { + if (string.IsNullOrWhiteSpace(server_name)) + { + return Outcome("invalid", "server_name is required."); + } + + /* Resolve through the SAME resolver the read tools use, against the servers registry (the id it returns + is the shared-identity server_id that keys config_monitored_servers). A miss returns the resolver's + available-servers listing — surfaced as not_found here. */ + var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); + if (error != null) + { + return Outcome("not_found", error); + } + + await using var command = postgres.CreateCommand("DELETE FROM config_monitored_servers WHERE server_id = $1"); + command.Parameters.Add(new NpgsqlParameter { TypedValue = resolved!.Value.ServerId }); + var affected = await command.ExecuteNonQueryAsync(); + + return affected > 0 + ? JsonSerializer.Serialize(new { status = "removed", server = resolved.Value.ServerName }, McpHelpers.JsonOptions) + : Outcome("not_found", + $"'{resolved.Value.ServerName}' is registered but has no monitored-server definition to remove (it may have been added only via darling.json, or already removed)."); + } + catch (Exception ex) + { + return McpHelpers.FormatError("remove_server", ex); + } + } + + /* ─────────────────────────────── pure parse + validate (no I/O) ─────────────────────────────── */ + + /// One structurally-valid server ready for dedupe → probe → insert. is the + /// shared case-folded identity (); is the + /// the in-process probe connects with (carrying the plaintext password for the + /// connect); is retained ONLY until the post-probe DPAPI encrypt, never stored + /// or echoed. is the entry's index in the input array, so the aggregated results echo input + /// order. + internal sealed record ParsedServerEntry( + int Order, string DisplayName, string StorageKey, MonitoredServer ProbeConfig, string? PlaintextPassword); + + /// One per-server outcome (added / duplicate / connection_failed / invalid) + /// with a human-readable ; restores input order in the aggregate. + internal sealed record ServerResult(int Order, string Server, string Status, string Detail); + + /// + /// PURE structural validation of the servers_json request — no store, no probe, no crypto — so the + /// validate-before-write behavior is unit-testable without a live SQL Server. Returns the structurally-valid + /// Entries (with their dedupe keys + probe configs built), the per-entry Invalid results (a bad + /// field or an unsupported auth mode — recorded, the batch continues past them), and a non-null + /// WholeError when the whole payload is unusable (not JSON, not an array, or empty), for which the caller + /// returns a single {status:"invalid"} without opening the store. + /// + internal static (List Entries, List Invalid, string? WholeError) ParseRequest(string servers_json) + { + var entries = new List(); + var invalid = new List(); + + JsonNode? root; + try + { + root = JsonNode.Parse(servers_json); + } + catch (JsonException ex) + { + return (entries, invalid, $"servers_json is not valid JSON: {ex.Message}"); + } + + if (root is not JsonArray array) + { + return (entries, invalid, "servers_json must be a JSON array of server objects (e.g. [{\"host\":\"sql01\"}])."); + } + + if (array.Count == 0) + { + return (entries, invalid, "servers_json must be a non-empty JSON array — provide at least one server object."); + } + + for (var i = 0; i < array.Count; i++) + { + var (entry, result) = ParseEntry(i, array[i]); + if (entry != null) + { + entries.Add(entry); + } + else + { + invalid.Add(result!); + } + } + + return (entries, invalid, null); + } + + /// Parses + validates ONE array element into a ready entry, or an invalid result naming the + /// problem. Windows/SQL are the only auth modes the service can honor; every other value (including the + /// Entra/MFA/Service-Principal/Managed-Identity modes) is rejected — interactive MFA is nonsensical headless. + private static (ParsedServerEntry? Entry, ServerResult? Result) ParseEntry(int index, JsonNode? node) + { + if (node is not JsonObject obj) + { + return (null, new ServerResult(index, $"(entry {index + 1})", "invalid", "Each entry must be a JSON object.")); + } + + var host = TryGetString(obj, "host"); + var label = string.IsNullOrWhiteSpace(host) ? $"(entry {index + 1})" : host!.Trim(); + + ServerResult Invalid(string message) => new(index, label, "invalid", message); + + if (string.IsNullOrWhiteSpace(host)) + { + return (null, Invalid("host is required.")); + } + + host = host!.Trim(); + var displayName = TryGetString(obj, "display_name") is { Length: > 0 } dn ? dn.Trim() : host; + var databaseRaw = TryGetString(obj, "database"); + var database = string.IsNullOrWhiteSpace(databaseRaw) ? null : databaseRaw!.Trim(); + + /* Auth: Windows (integrated) or SQL only. Absent defaults to Windows (the MonitoredServer default). Any + other value — Entra / MFA / ServicePrincipal / ManagedIdentity, or a typo — is refused with the SAME + message, so a headless caller learns the service's two supported modes. */ + var authRaw = TryGetString(obj, "auth"); + string storeAuth; + if (string.IsNullOrWhiteSpace(authRaw) || authRaw.Trim().Equals("Windows", StringComparison.OrdinalIgnoreCase)) + { + storeAuth = ServerStoreAuth.Integrated; + } + else if (authRaw.Trim().Equals("SQL", StringComparison.OrdinalIgnoreCase)) + { + storeAuth = ServerStoreAuth.Sql; + } + else + { + return (null, Invalid( + "auth must be \"Windows\" or \"SQL\". Microsoft Entra / MFA / Service Principal / Managed Identity " + + "are not supported for headless onboarding — the Darling service connects with Windows (integrated) " + + "or SQL authentication only.")); + } + + string? username = null; + string? plaintextPassword = null; + if (storeAuth == ServerStoreAuth.Sql) + { + username = TryGetString(obj, "username"); + if (string.IsNullOrWhiteSpace(username)) + { + return (null, Invalid("username is required for SQL authentication.")); + } + + username = username!.Trim(); + plaintextPassword = TryGetString(obj, "password"); + if (string.IsNullOrEmpty(plaintextPassword)) + { + return (null, Invalid("password is required for SQL authentication.")); + } + } + + /* encrypt_mode + trust_server_certificate are deliberately EXPOSED (per Erik) — a headless caller sets the + TLS posture explicitly. Optional with the fail-closed MonitoredServer defaults (Mandatory / no trust). */ + var (encryptMode, encryptError) = ResolveEncryptMode(TryGetString(obj, "encrypt_mode")); + if (encryptError != null) + { + return (null, Invalid(encryptError)); + } + + var (trustCert, trustError) = ResolveBool(obj, "trust_server_certificate", false); + if (trustError != null) + { + return (null, Invalid(trustError)); + } + + var (readOnlyIntent, roError) = ResolveBool(obj, "read_only_intent", false); + if (roError != null) + { + return (null, Invalid(roError)); + } + + var (multiSubnet, msError) = ResolveBool(obj, "multi_subnet_failover", false); + if (msError != null) + { + return (null, Invalid(msError)); + } + + var probeConfig = new MonitoredServer + { + Name = displayName, + Host = host, + Database = database, + Auth = storeAuth, + Username = username, + /* Plaintext for the probe's connect (ResolvePassword's dev-plaintext fallback); the stored blob is the + DPAPI encryption of this, produced only AFTER a successful probe. */ + Password = plaintextPassword, + EncryptMode = encryptMode, + TrustServerCertificate = trustCert, + ReadOnlyIntent = readOnlyIntent, + MultiSubnetFailover = multiSubnet, + }; + + var storageKey = ServerIdHelper.BuildStorageName(host, database, readOnlyIntent); + return (new ParsedServerEntry(index, displayName, storageKey, probeConfig, plaintextPassword), null); + } + + /// + /// PURE dedupe partition — the case-folded gate seeded with the + /// existing store keys, first-occurrence-wins within the batch (the #1549 idiom). Returns the Ready + /// entries to probe + insert and the Duplicates as ready-to-report results. Unit-testable without a + /// store or probe. + /// + internal static (List Ready, List Duplicates) PartitionDuplicates( + IReadOnlyList entries, IEnumerable existingKeys) + { + var seen = new HashSet(existingKeys, StringComparer.OrdinalIgnoreCase); + var ready = new List(); + var duplicates = new List(); + + foreach (var entry in entries) + { + if (seen.Add(entry.StorageKey)) + { + ready.Add(entry); + } + else + { + duplicates.Add(new ServerResult(entry.Order, entry.DisplayName, "duplicate", + "Already monitored (or a duplicate of an earlier entry in this batch); skipped.")); + } + } + + return (ready, duplicates); + } + + /* ─────────────────────────────── store I/O ─────────────────────────────── */ + + /// Reads the identity fields of every existing monitored server so the dedupe gate can be seeded from + /// the authoritative set (mirrors the bulk dialog's LoadExistingKeysAsync). Non-secret columns only. + public const string ExistingServersSql = "SELECT host, database, read_only_intent FROM config_monitored_servers"; + + /// The INSERT — column set + shape mirrored from StoreConfigProvider.SeedMonitoredServersAsync + /// (the seed authority), so a tool-added row is byte-identical to a seeded one. capture_plans and + /// alert_delivery_mode_override default to NULL (inherit the globals), monthly_cost_usd/ + /// excluded_databases are the neutral defaults, and is_enabled is TRUE (collection starts at + /// once). ON CONFLICT DO NOTHING guards a race with a concurrent writer — the dedupe gate is the primary + /// guard. + public const string InsertServerSql = @" +INSERT INTO config_monitored_servers ( + server_id, name, host, database, auth, username, encrypted_password, encrypt_mode, + trust_server_certificate, read_only_intent, multi_subnet_failover, excluded_databases, + monthly_cost_usd, capture_plans, alert_delivery_mode_override, is_enabled, created_at, modified_at) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, NULL, NULL, TRUE, $14, $14) +ON CONFLICT (server_id) DO NOTHING"; + + private static async Task> LoadExistingStorageKeysAsync(NpgsqlDataSource postgres, CancellationToken cancellationToken) + { + var keys = new List(); + await using var command = postgres.CreateCommand(ExistingServersSql); + await using var reader = await command.ExecuteReaderAsync(cancellationToken); + while (await reader.ReadAsync(cancellationToken)) + { + var host = reader.GetString(0); + var database = reader.IsDBNull(1) ? null : reader.GetString(1); + var readOnlyIntent = !reader.IsDBNull(2) && reader.GetBoolean(2); + keys.Add(ServerIdHelper.BuildStorageName(host, database, readOnlyIntent)); + } + + return keys; + } + + private static async Task InsertServerAsync( + NpgsqlDataSource postgres, ParsedServerEntry entry, string? encryptedPassword, CancellationToken cancellationToken) + { + var config = entry.ProbeConfig; + var now = DateTime.SpecifyKind(DateTime.UtcNow, DateTimeKind.Unspecified); + + await using var command = postgres.CreateCommand(InsertServerSql); + command.Parameters.Add(new NpgsqlParameter { TypedValue = ServerIdHelper.GetDeterministicHashCode(entry.StorageKey) }); // $1 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.Name }); // $2 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.Host }); // $3 + AddNullableText(command, config.Database); // $4 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.Auth }); // $5 + AddNullableText(command, config.Username); // $6 + AddNullableText(command, encryptedPassword); // $7 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.EncryptMode }); // $8 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.TrustServerCertificate }); // $9 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.ReadOnlyIntent }); // $10 + command.Parameters.Add(new NpgsqlParameter { TypedValue = config.MultiSubnetFailover }); // $11 + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Array | NpgsqlDbType.Text, Value = Array.Empty() }); // $12 + command.Parameters.Add(new NpgsqlParameter { TypedValue = 0m }); // $13 + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Timestamp, Value = now }); // $14 + await command.ExecuteNonQueryAsync(cancellationToken); + } + + /* ─────────────────────────────── helpers ─────────────────────────────── */ + + /// DPAPI-encrypts a SQL password for storage, or null for Windows auth (no secret). The + /// OperatingSystem.IsWindows() guard keeps CA1416 honest and gives a clear failure on a non-Windows BYO + /// host — the same platform posture as every other DPAPI surface here; managed mode (where onboarding runs) is + /// Windows-only. The plaintext is not logged and never leaves this method. + private static string? ProtectPasswordForStorage(string? plaintextPassword) + { + if (string.IsNullOrEmpty(plaintextPassword)) + { + return null; + } + + if (!OperatingSystem.IsWindows()) + { + throw new PlatformNotSupportedException("Storing a SQL-auth password requires Windows (DPAPI)."); + } + + return DarlingSecrets.Protect(plaintextPassword); + } + + /// Builds the {added, skipped, failed, results} envelope, results in input order. + private static string Aggregate(List results) + { + var ordered = results.OrderBy(r => r.Order).ToList(); + return JsonSerializer.Serialize(new + { + added = ordered.Count(r => r.Status == "added"), + skipped = ordered.Count(r => r.Status == "duplicate"), + failed = ordered.Count(r => r.Status is "connection_failed" or "invalid"), + results = ordered.Select(r => new { server = r.Server, status = r.Status, detail = r.Detail }), + }, McpHelpers.JsonOptions); + } + + /// The probed facts for an added server — edition / major version / msdb access, mirroring the + /// --test-connection CLI line (DarlingCliCommands.FormatProbeLine). + private static string DescribeProbe(ConnectionProbeResult probe) + { + var edition = string.IsNullOrEmpty(probe.EngineEditionDescription) + ? DarlingServerConnector.DescribeEngineEdition(probe.EngineEdition) + : probe.EngineEditionDescription; + var msdb = probe.HasMsdbAccess ? "msdb access: yes" : "msdb access: NO (SQL Agent job data unavailable)"; + return $"Connected — SQL major version {probe.MajorVersion}, {edition}, {msdb}."; + } + + /// Validates the optional encrypt_mode ("Optional"/"Mandatory"/"Strict"); absent → the + /// fail-closed "Mandatory" default (matching + the connection builder). + private static (string Mode, string? Error) ResolveEncryptMode(string? raw) + { + if (string.IsNullOrWhiteSpace(raw)) + { + return ("Mandatory", null); + } + + return raw.Trim().ToUpperInvariant() switch + { + "OPTIONAL" => ("Optional", null), + "MANDATORY" => ("Mandatory", null), + "STRICT" => ("Strict", null), + _ => ("Mandatory", "encrypt_mode must be \"Optional\", \"Mandatory\", or \"Strict\"."), + }; + } + + /// Reads an optional boolean field; absent → , a non-boolean value → error. + private static (bool Value, string? Error) ResolveBool(JsonObject obj, string field, bool fallback) + { + if (obj[field] is not JsonNode node) + { + return (fallback, null); + } + + if (node is JsonValue value && value.TryGetValue(out var b)) + { + return (b, null); + } + + return (fallback, $"{field} must be true or false."); + } + + private static string? TryGetString(JsonObject obj, string key) => + obj[key] is JsonValue value && value.TryGetValue(out var s) ? s : null; + + private static void AddNullableText(NpgsqlCommand command, string? value) => + command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Text, Value = (object?)value ?? DBNull.Value }); + + /// A small {status, message} envelope for a non-data outcome (invalid / not_found) — the same + /// shape / use, so an MCP client can + /// branch on the outcome kind. A successful add-batch / removal returns its own data-bearing shape, not this. + private static string Outcome(string status, string message) => + JsonSerializer.Serialize(new { status, message }, McpHelpers.JsonOptions); + + /// The two config.config_monitored_servers.auth values the Darling service connect path honors + /// — the service-side twin of the Viewer's ServerStoreCredential constants (the Viewer project is not + /// referenced here, so the values are restated; they are pinned equal to the store's DDL default 'integrated'). + private static class ServerStoreAuth + { + public const string Integrated = "integrated"; + public const string Sql = "sql"; + } +} diff --git a/Darling/README.md b/Darling/README.md index 0fd9bc3dc..238b67be0 100644 --- a/Darling/README.md +++ b/Darling/README.md @@ -283,7 +283,7 @@ A channel is enabled by a non-empty URL. ### mcp -The embedded MCP server, over Streamable HTTP bound to `localhost` by default (see [Opt-in Network Endpoints (LAN)](#opt-in-network-endpoints-lan) to reach it — and the store — from the LAN). It exposes the same tool names Lite and the Dashboard expose, plus small Darling-only WRITE surfaces — Custom Views management and alert tuning (see the last two bullets): +The embedded MCP server, over Streamable HTTP bound to `localhost` by default (see [Opt-in Network Endpoints (LAN)](#opt-in-network-endpoints-lan) to reach it — and the store — from the LAN). It exposes the same tool names Lite and the Dashboard expose, plus small Darling-only WRITE surfaces — Custom Views management, alert tuning, and server onboarding (see the last three bullets): - **Six diagnostic-analysis tools** — `analyze_server`, `get_analysis_facts`, `compare_analysis`, `audit_config`, `get_analysis_findings`, `mute_analysis_finding`. - **Five plan-analysis tools** — `analyze_query_plan` (by `query_hash`), `analyze_procedure_plan` (by `sql_handle`), `analyze_query_store_plan` (by `database_name` + `query_id`), `analyze_plan_xml` (raw showplan XML, no fetch), and `get_plan_xml` (raw stored plan XML by `query_hash`). These run the shared execution-plan analyzer over the plan XML the collectors already captured into the store — a stored-plan read, never a live query against the monitored server. `analyze_query_plan`/`get_plan_xml` accept an optional `database_name`, and `analyze_query_store_plan` an optional `plan_id`, to pin the exact stored plan when the caller knows it. @@ -333,6 +333,8 @@ The embedded MCP server, over Streamable HTTP bound to `localhost` by default (s - **Three alert-tuning write tools (Darling-only)** — `update_alert_settings`, `create_mute_rule`, and `delete_mute_rule` let an MCP client TUNE the alert engine the fleet shares — the SAME config `get_alert_settings` / `get_mute_rules` read and the Viewer's Settings window writes. `update_alert_settings` is a PARTIAL update of the single global settings row: read via `get_alert_settings`, change fields, and send only those back in the same nested shape; every field is validated against the SAME ranges/enums the Settings window enforces BEFORE any write, an out-of-range or unknown field returns `{status:"invalid"}` and writes nothing, and the write self-bumps `config_version` so the running service hot-reloads within one collection sweep. `create_mute_rule` / `delete_mute_rule` reuse the SAME `PgMuteRuleStore` `get_mute_rules` reads through (and the same GUID id-generation the Viewer's mute-create path uses). None touches a monitored SQL Server or the collected data — only the shared alert configuration; SMTP/webhook delivery credentials are out of scope (the `mcp` role cannot read or write the secret columns). This widens what an MCP token can do — see [Blast radius](#opt-in-network-endpoints-lan) below. +- **Two server-onboarding write tools (Darling-only)** — `add_servers` (BULK) and `remove_server` let an MCP client stand up or tear down FLEET monitoring conversationally ("monitor these twenty servers with this login"), the service-side twin of the Viewer's Add / Manage Servers dialogs. `add_servers` takes a JSON **array** of server objects (`host` required; optional `display_name` / `database` / `read_only_intent` / `multi_subnet_failover`; `auth` `Windows`/`SQL` with `username`+`password` for SQL; and the exposed TLS options `encrypt_mode` `Optional`/`Mandatory`/`Strict` + `trust_server_certificate`) and processes them **in order**: it validates each entry, PROBES the connection in-process (reusing the same `DarlingServerConnector.ProbeAsync` the `--test-connection` verb runs — the service holds the network path + credentials, so no `test_connect` command plane is needed), skips a case-folded duplicate (`duplicate`) of an already-monitored server or an earlier entry, DPAPI-encrypts the SQL password (the service identity, so it round-trips at collection time), and INSERTs the row mirroring the service's own seed shape. A server that fails to connect is `connection_failed` and the batch continues; Entra/MFA/Service-Principal/Managed-Identity auth is `invalid` (the service connects with Windows or SQL only). `remove_server` DELETEs a monitored server by name (resolved the same way every `server_name` is) — already-collected history is kept. Both write only the monitoring store's `config.config_monitored_servers` registry; neither runs anything on a monitored server beyond the one-time probe. **The SQL password travels to the endpoint inside `add_servers`' request** and is DPAPI-encrypted at rest (never returned) — this widens what an MCP token can do and puts a credential on the wire; see [Blast radius](#opt-in-network-endpoints-lan) below. + | Key | Default | Notes | |---|---|---| | `enabled` | `false` | **Off by default** — a headless service does not open a local port unless you ask | @@ -571,9 +573,9 @@ Table names are unchanged — only their schema moved — and the shared SQL kee | `darling` | superuser / owner | the service (collection, migration, provisioning) | | `admin` | SELECT on both schemas + INSERT/UPDATE/DELETE on `config` only | the Viewer, by default (`connectAs: "admin"`) | | `viewer` | SELECT on both schemas + INSERT/UPDATE/DELETE on `config.custom_views` only (the web composer's saved views) | a locked-down Viewer (`connectAs: "viewer"`) | -| `mcp` | `viewer`'s exact read surface + INSERT on `collect.analysis_findings` / `config.analysis_muted` + INSERT/UPDATE/DELETE on `config.custom_views` (the custom-view tools) + the alert-tuning writes (INSERT/UPDATE/DELETE on `config.config_mute_rules`, UPDATE on `config.config_alert_settings`, and the `config_service` reload-beacon columns) | the store identity the opt-in MCP **network** endpoint connects as (managed only); dormant until MCP is exposed on the LAN | +| `mcp` | `viewer`'s exact read surface + INSERT on `collect.analysis_findings` / `config.analysis_muted` + INSERT/UPDATE/DELETE on `config.custom_views` (the custom-view tools) + the alert-tuning writes (INSERT/UPDATE/DELETE on `config.config_mute_rules`, UPDATE on `config.config_alert_settings`, and the `config_service` reload-beacon columns) + the server-onboarding writes (INSERT/UPDATE/DELETE on `config.config_monitored_servers` — the credential column stays SELECT-carved, so it can WRITE a password blob but never READ one back) | the store identity the opt-in MCP **network** endpoint connects as (managed only); dormant until MCP is exposed on the LAN | -`admin` cannot `DROP`, alter schema, touch `collect` data, or create objects — it can only do what the Viewer's mute-rule / alert-dismiss surfaces need. The `mcp` role is narrower still: it reads exactly what `viewer` reads (the secret config columns are carved out identically) and its writes are a small, enumerated set — the two analysis-table INSERTs (`analyze_server` + `mute_analysis_finding`), the single-table `config.custom_views` CRUD (the custom-view tools), and the alert-tuning writes (`config.config_mute_rules` CRUD + a single-row `config.config_alert_settings` UPDATE, plus the two `config_service` beacon columns so a settings write's self-bump trigger can fire) — so a token-holder on the network MCP endpoint can never reach the `config`-table service-credential pivot, the secret columns, or a service flag like `paused`. `ALTER DEFAULT PRIVILEGES` means new collector tables auto-inherit SELECT for `admin`/`viewer`, so the model never drifts as collectors are added (every `mcp` write is an explicit single-table/single-column grant, deliberately not schema-wide). +`admin` cannot `DROP`, alter schema, touch `collect` data, or create objects — it can only do what the Viewer's mute-rule / alert-dismiss surfaces need. The `mcp` role is narrower still: it reads exactly what `viewer` reads (the secret config columns are carved out identically) and its writes are a small, enumerated set — the two analysis-table INSERTs (`analyze_server` + `mute_analysis_finding`), the single-table `config.custom_views` CRUD (the custom-view tools), the alert-tuning writes (`config.config_mute_rules` CRUD + a single-row `config.config_alert_settings` UPDATE, plus the two `config_service` beacon columns so a settings write's self-bump trigger can fire), and the server-onboarding writes (`config.config_monitored_servers` CRUD for `add_servers` / `remove_server` — its `config_monitored_servers` write fires the SAME `config_service` beacon trigger, already covered by that column grant) — so a token-holder on the network MCP endpoint can never reach the `config`-table service-credential pivot, the secret columns, or a service flag like `paused`. Even on `config_monitored_servers`, which it may write, the `encrypted_password` column stays in the fail-closed secret carve, so `mcp` can WRITE a credential blob (onboarding) but can never READ one back. `ALTER DEFAULT PRIVILEGES` means new collector tables auto-inherit SELECT for `admin`/`viewer`, so the model never drifts as collectors are added (every `mcp` write is an explicit single-table/single-column grant, deliberately not schema-wide). **Managed mode** provisions all of this automatically on every start (idempotent and self-healing), generating a per-role DPAPI-LocalMachine credential — `pg-admin-credential.dpapi`, `pg-viewer-credential.dpapi`, and `pg-mcp-credential.dpapi` beside the data directory, same posture as the owner's `pg-credential.dpapi`. Nothing to configure beyond `connectAs`. @@ -693,7 +695,9 @@ When `listen` is a network address **and** a token is present **and** `allowFrom New-NetFirewallRule -DisplayName "Darling MCP" -Direction Inbound -Action Allow -Protocol TCP -LocalPort 5152 -RemoteAddress 192.168.1.0/24 ``` -**Blast radius, stated honestly.** The token gates the entire read surface, `analyze_server` (which opens **live outbound connections to your monitored SQL Servers** — the plan-fetcher), the Custom Views management tools, which **create / modify / delete** the saved dashboards and notebooks in `config.custom_views`, *and* the alert-tuning tools (`update_alert_settings` / `create_mute_rule` / `delete_mute_rule`), which change the shared alert configuration the service delivers on. Treat the token as a high-value secret. The store-side identity is still the least-privilege `mcp` role: read, the two analysis-table INSERTs, INSERT/UPDATE/DELETE on the single `config.custom_views` table (the same narrow write the web composer's `viewer` role has), and the narrow alert-config writes (`config.config_mute_rules` CRUD + a single-row `config.config_alert_settings` UPDATE, plus the `config_service` reload-beacon columns). So a token-holder can read everything collected, trigger analysis, author custom views, and tune alerting — but can never reach the `config_command` service-credential pivot, the carved secret columns (SMTP/webhook credentials included), or a service flag like `paused`, and a stored view is structurally incapable of reading a config control-plane table (a composed query names only `collect.*` collector tables). Custom-view JSON and alert config carry no secrets. +**Blast radius, stated honestly.** The token gates the entire read surface, `analyze_server` (which opens **live outbound connections to your monitored SQL Servers** — the plan-fetcher), the Custom Views management tools, which **create / modify / delete** the saved dashboards and notebooks in `config.custom_views`, the alert-tuning tools (`update_alert_settings` / `create_mute_rule` / `delete_mute_rule`), which change the shared alert configuration the service delivers on, *and* the server-onboarding tools (`add_servers` / `remove_server`), which **add or remove the monitored servers themselves** in `config.config_monitored_servers` — including storing a SQL-auth **credential** for a server it adds. Treat the token as a high-value secret. The store-side identity is still the least-privilege `mcp` role: read, the two analysis-table INSERTs, INSERT/UPDATE/DELETE on the single `config.custom_views` table (the same narrow write the web composer's `viewer` role has), the narrow alert-config writes (`config.config_mute_rules` CRUD + a single-row `config.config_alert_settings` UPDATE, plus the `config_service` reload-beacon columns), and the single-table `config.config_monitored_servers` CRUD. So a token-holder can read everything collected, trigger analysis, author custom views, tune alerting, and onboard/offboard servers — but can never reach the `config_command` service-credential pivot, the carved secret columns (SMTP/webhook credentials, and the monitored-server `encrypted_password` blob it can WRITE during onboarding but never READ back, all included), or a service flag like `paused`, and a stored view is structurally incapable of reading a config control-plane table (a composed query names only `collect.*` collector tables). Custom-view JSON and alert config carry no secrets. + +**A monitored-server credential rides `add_servers` — mind the wire.** `add_servers` accepts a SQL-auth `password` in its request JSON; the service DPAPI-encrypts it at rest (never returns it), but it travels to the MCP endpoint in the clear on the same plaintext HTTP the token does. That is one more reason to **front the MCP port with the TLS reverse proxy below** on any segment you do not fully trust — the password, like the token, is captured by an on-path attacker otherwise. Prefer Windows/integrated auth for onboarded servers where you can (no per-server secret crosses the wire at all). **MCP has no TLS — the MITM control is a TLS reverse proxy.** A self-signed cert breaks real MCP clients, so the MCP endpoint is plain HTTP and the bearer token travels **cleartext on the segment**; an active on-path attacker (ARP spoof, rogue DHCP, compromised switch) could capture and replay it. The in-app CIDR bounds *who can route to* the port; it does **not** protect the wire. If your segment is not fully trusted, put a **TLS-terminating reverse proxy** in front of the MCP port and point clients at that — the named MITM control for this endpoint. (The store endpoint needs no such proxy: it has verify-full TLS built in.) diff --git a/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs b/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs index b800df220..f1ee3ee58 100644 --- a/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs +++ b/Lite.Tests/CrossAppMcpToolInventoryPinTests.cs @@ -87,6 +87,15 @@ the compose vocabulary those authoring tools draw from so an MCP client composes "update_alert_settings", "create_mute_rule", "delete_mute_rule", + + /* Darling MCP server-onboarding write tools — add/remove the monitored servers in the CENTRAL store the + whole fleet shares (config.config_monitored_servers). add_servers bulk-onboards (validate + in-process + probe + case-folded dedupe + DPAPI-encrypt + INSERT); remove_server DELETEs by the shared resolver. + Darling-ONLY by architecture, not "not ported yet": Lite is a single-instance WPF app that monitors + servers from its own local config + DuckDB, with no central service-honored monitored-server store, so + there is no Lite twin to port (same reasoning as the Custom Views + alert-tuning tools above). */ + "add_servers", + "remove_server", }; [Fact]