Reach the delivery cooldown through the control plane as delivery.cooldown_minutes

Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs

@@ -361,6 +361,24 @@ await ExecAsync(mcp,
             await ExecAsync(owner, "INSERT INTO config.config_alert_settings (id) VALUES (1) ON CONFLICT (id) DO NOTHING", ct);
             await ExecAsync(mcp, "UPDATE config.config_alert_settings SET enabled = enabled WHERE id = 1", ct);
 
+            /* #3314: the delivery cooldown, on config_notification rather than config_alert_settings, so
+               update_alert_settings spans two tables. This is the one live check that separates the two ways
+               the write can 42501 — a missing column SELECT and a missing column UPDATE raise the IDENTICAL
+               "permission denied for table config_notification", so a single passing write attempt cannot
+               attribute itself. The read half is asserted by the secret-column loop above (which SELECTs the
+               whole non-secret set, email_cooldown_minutes included); this is the write half, and the
+               sibling-column denials below are what prove the grant is really one column wide rather than
+               the table-wide write it would be easiest to reach for. */
+            await ExecAsync(owner, "INSERT INTO config.config_notification (id) VALUES (1) ON CONFLICT (id) DO NOTHING", ct);
+            await ExecAsync(mcp, "UPDATE config.config_notification SET email_cooldown_minutes = email_cooldown_minutes WHERE id = 1", ct);
+
+            foreach (var sibling in new[] { "smtp_encrypted_password = 'x'", "slack_url = 'x'", "smtp_host = 'x'" })
+            {
+                var siblingDenied = await Assert.ThrowsAsync<PostgresException>(async () =>
+                    await ExecAsync(mcp, $"UPDATE config.config_notification SET {sibling} WHERE id = 1", ct));
+                Assert.Equal("42501", siblingDenied.SqlState);
+            }
+
             /* But mcp is STILL denied a write to a config table it was NOT granted (42501) — config_command is
                the service-credential pivot; the alert-tuning grants did not widen into a schema-wide config write. */
             var stillDenied = await Assert.ThrowsAsync<PostgresException>(async () =>
@@ -546,6 +564,10 @@ IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = '{McpRole}') THEN
 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};
+-- #3314: the DELIVERY cooldown, the one alert knob stored on config_notification. COLUMN-level, because that
+-- table holds the SMTP password blob and the Teams/Slack/generic webhook URLs and the PagerDuty routing key.
+-- Mirrors DarlingManagedRoles section 8.
+GRANT UPDATE (email_cooldown_minutes) ON config.config_notification 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).

Darling/Darling.Tests/McpConfigReadAvoidsSecretColumnsTests.cs

@@ -112,6 +112,49 @@ public void TheNotificationReadStillNamesCarvedSecretColumns()
         Assert.NotEmpty(namedSecrets);
     }
 
+    /// <summary>
+    /// #3314 put a <c>config_notification</c> read back on the MCP surface — the delivery cooldown, the one
+    /// alert-engine knob stored on that table — so the carve now has to hold for a read that DOES ship,
+    /// rather than by the whole read having been removed.
+    ///
+    /// <para>The mirror image of <see cref="TheNotificationReadStillNamesCarvedSecretColumns"/>, and it
+    /// fails in the costly direction: that one asserts the PRIVILEGED read still names secrets (so the skip
+    /// stays justified), this one asserts the MCP read names NONE. Column-level denial answers for the whole
+    /// TABLE, so a single carved column added to this SELECT does not degrade the read — it 42501s the entire
+    /// call, and that is the #2293 failure, where skipping one denied row simply moved the error to the next.
+    /// Every non-secret column is derived from the ACL rather than listed, so a column reclassified as secret
+    /// makes THIS fail on the day of the reclassification instead of on the next deployment.</para>
+    ///
+    /// <para>The read is also asserted to stay narrow: the value the tool needs is one column, and
+    /// <c>SELECT *</c> — or a convenience widening to "the non-secret columns" — is denied outright by the
+    /// carve for the star and is a pointless secret-adjacent read for the rest.</para>
+    /// </summary>
+    [Fact]
+    public void TheMcpDeliveryCooldownReadNamesNoCarvedSecretColumn()
+    {
+        var sql = PerformanceMonitor.Darling.Service.Mcp.DarlingAlertReader.DeliveryCooldownSelectSql;
+        var acl = DarlingManagedRoles.ViewerRestrictedConfigTables
+            .Single(t => string.Equals(t.Table, "config_notification", StringComparison.Ordinal));
+
+        Assert.Contains("FROM config_notification", sql, StringComparison.Ordinal);
+        Assert.DoesNotContain("*", sql, StringComparison.Ordinal);
+
+        var named = acl.SecretColumns.Where(c => sql.Contains(c, StringComparison.Ordinal)).ToArray();
+        Assert.Empty(named);
+
+        /* And it really does name the one non-secret column it needs -- without this the assertions above
+           are satisfied by a SELECT that reads nothing from the table at all. */
+        var selected = sql[(sql.IndexOf("SELECT", StringComparison.Ordinal) + 6)..
+                            sql.IndexOf("FROM config_notification", StringComparison.Ordinal)]
+            .Split(',', StringSplitOptions.RemoveEmptyEntries)
+            .Select(c => c.Trim())
+            .Where(c => c.Length > 0)
+            .ToArray();
+
+        Assert.Equal(new[] { "email_cooldown_minutes" }, selected);
+        Assert.Contains("email_cooldown_minutes", acl.NonSecretColumns);
+    }
+
     private static string RepoRoot([CallerFilePath] string thisFile = "")
     {
         var dir = Path.GetDirectoryName(thisFile)!;

Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs

@@ -41,7 +41,8 @@ namespace PerformanceMonitor.Darling.Service;
 /// INSERT on <c>collect.analysis_findings</c> and <c>config.analysis_muted</c> (what <c>analyze_server</c>
 /// persists + the <c>mute</c> tool need), INSERT/UPDATE/DELETE on <c>config.custom_views</c> (the
 /// custom-view tools, #1599), the alert-tuning writes (INSERT/UPDATE/DELETE on
-/// <c>config.config_mute_rules</c> + UPDATE on the singleton <c>config.config_alert_settings</c>, plus the
+/// <c>config.config_mute_rules</c> + UPDATE on the singleton <c>config.config_alert_settings</c> + UPDATE on
+/// the single non-secret <c>email_cooldown_minutes</c> column of <c>config.config_notification</c>, plus the
 /// two beacon columns of <c>config.config_service</c> so the settings write's self-bump trigger can fire),
 /// and the server-onboarding writes (INSERT/UPDATE/DELETE on <c>config.config_monitored_servers</c> for the
 /// <c>add_servers</c>/<c>remove_server</c> tools — a single non-secret-KEY table; the credential column stays
@@ -243,7 +244,7 @@ public static async Task<int> 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) + onboard servers (config_monitored_servers)) — 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_notification.email_cooldown_minutes, config_service reload beacon) + onboard servers (config_monitored_servers)) — the Viewer and MCP host no longer connect as the superuser");
 
         /* CLAMPED, not raw: the batch above wrote the clamped form, so returning the raw read would hand the
            caller a baseline that differs from what the roles actually carry (a stored 0 provisions '15s').
@@ -704,8 +705,9 @@ ALTER DEFAULT PRIVILEGES FOR ROLE {owner} IN SCHEMA {config}
 --    / create_mute_rule / delete_mute_rule let a token-holder tune the SAME alert engine the Viewer's Settings
 --    window drives: INSERT/UPDATE/DELETE on config_mute_rules (the mute rules the delivery paths honor) and UPDATE
 --    on the SINGLETON config_alert_settings row (id=1 -- UPDATE only, never INSERT/DELETE: the row is a fixed
---    singleton the service seeds). Still NARROW -- never the config_command service-credential pivot, the
---    monitored-servers/notification secret tables, or a schema-wide config write.
+--    singleton the service seeds). Still NARROW -- never the config_command service-credential pivot, a
+--    schema-wide config write, or any SECRET column: the one config_notification write is a single column
+--    (see #3314 below), and the monitored-servers credential column stays SELECT-carved.
 --    The beacon caveat: a config_alert_settings write fires the existing statement-level bump trigger
 --    (trg_bump_alert_settings -> config_bump_version), which UPDATEs config_service.config_version AS THE CURRENT
 --    ROLE (the trigger function is SECURITY INVOKER). So mcp ALSO needs UPDATE on JUST the two beacon columns of
@@ -722,6 +724,20 @@ 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};
+-- #3314: the DELIVERY cooldown -- the sole throttle on a Slack/Teams/PagerDuty/webhook post -- is the one
+-- alert-engine knob stored on config_notification rather than config_alert_settings, so update_alert_settings
+-- spans two tables and needs a write here. This DOES widen mcp into a table holding bearer secrets (the SMTP
+-- password blob, the Teams/Slack/generic webhook URLs, the PagerDuty routing key), so the grant is
+-- COLUMN-level on exactly that one column -- the same shape as the config_service beacon grant above and for
+-- the same reason. MEASURED, not assumed: as mcp, the baseline UPDATE raises 42501; with this grant it
+-- succeeds; with the column SELECT revoked and this grant kept it STILL succeeds (so UPDATE is the privilege
+-- doing the work, not an ambient SELECT); and a write to smtp_encrypted_password, slack_url or even the
+-- non-secret sibling smtp_host stays 42501. A missing SELECT and a missing UPDATE both raise the identical
+-- 42501 permission-denied-for-table-config_notification message, so only isolating the grants separates them.
+-- The READ side needs nothing: email_cooldown_minutes is already in the section-6 non-secret column carve.
+-- The BEACON is already covered: config_notification carries trg_bump_notification -> config_bump_version
+-- (SECURITY INVOKER), which UPDATEs config_service.config_version AS mcp, and the column grant above serves it.
+GRANT UPDATE (email_cooldown_minutes) ON {config}.config_notification 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 /

Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingAlertReader.cs

@@ -16,8 +16,9 @@ namespace PerformanceMonitor.Darling.Service.Mcp;
 
 /// <summary>
 /// Service-side reads for the alerts MCP tools (<see cref="DarlingMcpAlertTools"/>) — the alert-history log
-/// (<c>config_alert_log</c>) and the single global alert-settings row (<c>config_alert_settings</c>), both
-/// STORED reads (no live monitored-server hit). Each SQL is reproduced from the viewer's proven read
+/// (<c>config_alert_log</c>), the single global alert-settings row (<c>config_alert_settings</c>), and the
+/// delivery cooldown that lives on <c>config_notification</c> instead, all STORED reads (no live
+/// monitored-server hit). Each SQL is reproduced from the viewer's proven read
 /// (<c>ViewerDataService.AlertHistory.cs</c> / <c>.AlertSettings.cs</c>) rather than referenced — the MCP
 /// host is in the Service assembly and cannot reference the WPF Viewer, the same reason
 /// <see cref="DarlingConfigHistoryReader"/> reproduces the viewer's config SQL. The reads live in public
@@ -29,6 +30,10 @@ namespace PerformanceMonitor.Darling.Service.Mcp;
 /// running <c>DarlingAlertSettings</c>, so it reports the alert engine + analysis config the service is
 /// actually using (matching the viewer's Settings-window prefill), or null when the store has not seeded it
 /// yet.</para>
+///
+/// <para>The delivery cooldown is the one alert-control-plane value on a DIFFERENT table, which is why it
+/// gets its own read rather than another column on the settings SELECT — see
+/// <see cref="DeliveryCooldownSelectSql"/> for why the SELECT list is deliberately one column wide.</para>
 /// </summary>
 internal static class DarlingAlertReader
 {
@@ -187,11 +192,13 @@ FROM config_alert_settings
 WHERE id = 1";
 
     /// <summary>Reads the single global alert-settings row, or null when the store has not seeded it yet
-    /// (a pre-control-plane store, or the service has not started).</summary>
-    public static async Task<AlertSettingsReadRow?> GetAlertSettingsAsync(
-        NpgsqlDataSource postgres, CancellationToken cancellationToken = default)
+    /// (a pre-control-plane store, or the service has not started). Takes the caller's connection and
+    /// transaction rather than the data source, so it cannot be invoked outside the shared snapshot
+    /// <see cref="GetAlertConfigurationAsync"/> establishes — see there for why that matters.</summary>
+    private static async Task<AlertSettingsReadRow?> ReadAlertSettingsAsync(
+        NpgsqlConnection connection, NpgsqlTransaction transaction, CancellationToken cancellationToken = default)
     {
-        await using var command = postgres.CreateCommand(AlertSettingsSelectSql);
+        await using var command = new NpgsqlCommand(AlertSettingsSelectSql, connection) { Transaction = transaction };
         command.CommandTimeout = McpCommandDeadlines.ReadSeconds;
         await using var reader = await command.ExecuteReaderAsync(cancellationToken);
         if (!await reader.ReadAsync(cancellationToken))
@@ -225,4 +232,82 @@ FROM config_alert_settings
             /* #2391: V79 file-growth knobs at 54–57. */
             reader.GetBoolean(54), reader.GetInt32(55), reader.GetInt32(56), reader.GetInt32(57));
     }
+
+    /* ─────────────────────── delivery cooldown (a SECOND config table) ─────────────────────── */
+
+    /// <summary>The per-fingerprint DELIVERY cooldown the shared notification paths throttle on
+    /// (<c>WebhookAlertService</c> and <c>EmailSendCore</c> both pass it to <c>IncidentCooldown</c>), stored
+    /// as <c>config_notification.email_cooldown_minutes</c>. Reported and accepted under the channel-neutral
+    /// name <c>delivery.cooldown_minutes</c>: the column predates the webhook channels and one number now
+    /// governs Slack, Teams, PagerDuty, the generic webhook AND email, so a headless deployment with no SMTP
+    /// at all is still throttled by it.
+    ///
+    /// <para>A SEPARATE read because this is the ONE column of the alert control plane that does not live on
+    /// <c>config_alert_settings</c> — which is also why it reached 3.5.0 reachable only from the WPF Settings
+    /// window. The SELECT list is exactly one non-secret column, deliberately: <c>config_notification</c>
+    /// holds the SMTP password and the Teams/Slack/generic/PagerDuty bearer URLs, and the section-6 ACL
+    /// (<c>DarlingManagedRoles.ViewerRestrictedConfigTables</c>) SELECT-carves every one of them from
+    /// <c>mcp</c> — while column-level denial answers for the whole TABLE, so naming a single carved column
+    /// here would 42501 the entire read. That is the #2293/#2298 failure exactly, and it is why the host's
+    /// own whole-row <c>LoadViewAsync</c> was removed rather than made to skip rows; a tool-time read of
+    /// columns the carve GRANTS is the shape that survives. <c>McpConfigReadAvoidsSecretColumnsTests</c>
+    /// pins that this SELECT names no carved column.</para></summary>
+    public const string DeliveryCooldownSelectSql = @"
+SELECT email_cooldown_minutes
+FROM config_notification
+WHERE id = 1";
+
+    /// <summary>Reads the delivery cooldown, or null when the store has no notification row. Null is a
+    /// distinct answer rather than the shipped 15: the service seeds this row and
+    /// <c>config_alert_settings</c> in ONE pass, so "settings present, notification absent" is not a state
+    /// the product produces, and reporting a number nobody wrote would claim a reading never taken.</summary>
+    private static async Task<int?> ReadDeliveryCooldownAsync(
+        NpgsqlConnection connection, NpgsqlTransaction transaction, CancellationToken cancellationToken = default)
+    {
+        await using var command = new NpgsqlCommand(DeliveryCooldownSelectSql, connection) { Transaction = transaction };
+        command.CommandTimeout = McpCommandDeadlines.ReadSeconds;
+        await using var reader = await command.ExecuteReaderAsync(cancellationToken);
+        if (!await reader.ReadAsync(cancellationToken) || reader.IsDBNull(0))
+        {
+            return null;
+        }
+
+        return reader.GetInt32(0);
+    }
+
+    /// <summary>Both halves of the alert configuration, read in ONE snapshot: the settings row and the
+    /// delivery cooldown that lives on the other table. Either may be null when the store has not seeded
+    /// that singleton yet.</summary>
+    public sealed record AlertConfigurationRead(AlertSettingsReadRow? Settings, int? DeliveryCooldownMinutes);
+
+    /// <summary>
+    /// The alert configuration across BOTH config tables, under one snapshot.
+    ///
+    /// <para>Two independent reads would let an <c>update_alert_settings</c> commit land between them and
+    /// hand the caller a payload mixing pre- and post-update state across the two tables — a stale
+    /// <c>cooldown_minutes</c> beside a fresh <c>delivery.cooldown_minutes</c>, or the reverse. The write
+    /// path already refuses to leave a half-landed state; a read that can REPORT one puts the asymmetry
+    /// back, and the post-write re-read is described to the caller as the authoritative merged state, which
+    /// it would not be.</para>
+    ///
+    /// <para><b>REPEATABLE READ, and the level is the whole mechanism.</b> PostgreSQL takes a FRESH snapshot
+    /// per statement under READ COMMITTED, so wrapping these two SELECTs in a default transaction reads
+    /// exactly like a fix and changes nothing at all. The two single-table reads are private and take this
+    /// method's connection and transaction, so the split cannot be reintroduced by calling one of them
+    /// alone — a stronger guarantee than a test, since it does not compile.</para>
+    ///
+    /// <para>Read-only, so the transaction is disposed rather than committed; nothing here writes.</para>
+    /// </summary>
+    public static async Task<AlertConfigurationRead> GetAlertConfigurationAsync(
+        NpgsqlDataSource postgres, CancellationToken cancellationToken = default)
+    {
+        await using var connection = await postgres.OpenConnectionAsync(cancellationToken);
+        await using var transaction = await connection.BeginTransactionAsync(
+            System.Data.IsolationLevel.RepeatableRead, cancellationToken);
+
+        var settings = await ReadAlertSettingsAsync(connection, transaction, cancellationToken);
+        var deliveryCooldownMinutes = await ReadDeliveryCooldownAsync(connection, transaction, cancellationToken);
+
+        return new AlertConfigurationRead(settings, deliveryCooldownMinutes);
+    }
 }