[BUG] MCP host still cannot read the config store — encrypted_password is SELECT-carved from mcp but ReadMonitoredServersAsync selects it (#2293 fixed only the notification half)

What happens

The MCP host cannot read the config store at all, on a fully-provisioned box. From prod-sql-use2-monitor-01 tonight, ~2 seconds after the service came up:

18:52:52.384 [INFO] [DarlingWorker] Least-privilege roles ready (... mcp: viewer's reads + ...
             + onboard servers (config_monitored_servers)) — the Viewer and MCP host no longer connect as the superuser
18:52:54.761 [WARN] [DarlingMcpHostService] Could not read the config store — keeping the current live config:
             42501: permission denied for table config_monitored_servers

The consequence is the one the product's own message names: live plan fetch falls back to darling.json, so any server added through add_servers or the Viewer has no connection. On a seeded box the store is authoritative for servers (#2254), so that fallback is exactly the set of servers the file does not know about.

Why — and it is not a missing grant

I checked the ACLs before assuming, and the grants are correct as designed. Column-level SELECT on config.config_monitored_servers (in pg_attribute.attacl, not pg_class.relacl — the table-level ACL shows only awd for mcp, which is what makes this look like a missing grant at first glance):

every column                 mcp_select=true
encrypted_password           mcp_select=false   attacl=-

That denial is deliberate and documented in DarlingManagedRoles.cs:479 — "the encrypted_password column is SELECT-carved from mcp by the section-6 secret-column ACL, so mcp can WRITE a credential blob but never READ one back."

The collision is on the read side. StoreConfigProvider.ReadMonitoredServersAsync (StoreConfigProvider.cs:801) selects it:

SELECT name, host, database, auth, username, encrypted_password, encrypt_mode, ...
FROM config_monitored_servers WHERE is_enabled = TRUE

So LoadViewAsync throws 42501 for the mcp role and the entire config view read is abandoned — alert settings, schedules and servers included, not just the credential.

This is the second half of #2293, and I should have caught it there

#2293 fixed the same defect on config_notification by having the MCP host pass includeNotification: false. It did not fix the MCP host's config read; it fixed the first of at least two blockers, and the failure simply moved to the next denied column. The timeline says so plainly: #2293 merged 13:57 UTC, the build running before tonight was cut at 12:31 UTC (so it failed on config_notification), and the 18:52 build — the first with #2293 — fails on config_monitored_servers instead. I verified that change by its code path and its tests rather than by watching the MCP host actually read the store afterwards, which is the miss.

Options, since the fix is a decision rather than a one-liner

A credential-free server read for the MCP host — the sibling of Stop MCP losing the server registry over a password it never reads #2293's flag (includeCredentials: false), returning the registry without encrypted_password. Needs an answer to what the MCP host does with a MonitoredServer carrying no password: if it only needs identity (names, ids, host, engine) then this is clean; if it builds live connections itself, that work has to move to the service's config_command pivot, which already exists for exactly this reason.
Grant mcp SELECT on encrypted_password — rejected, and worth stating so it is not re-proposed: it deletes the property the carve exists for (write a credential, never read one back), for a convenience the pivot already covers.
Do nothing and make the message honest — the weakest option, but strictly better than today: the WARN currently reads as a transient store problem ("keeping the current live config") when it is a permanent, by-design denial that will recur on every start.

I would take 1, and I have not started it because it needs the answer to what the MCP host legitimately needs from that row — that is a design call on the security seam, not a bug fix.

Two things this also suggests

The fail-closed carve has no test that the readers stay inside it. Both halves of this bug are a query selecting a column its role is denied, which compiles, passes every text assertion, and fails only against a provisioned store as a non-superuser role. A live-postgres test that runs each StoreConfigProvider read as the mcp and viewer roles would have caught both, and would catch the next column added to a secret table.
LoadViewAsync fails whole rather than partially. One denied column costing the alert settings and schedules as well is worth a separate look — the notification arm already degrades gracefully, so the pattern exists.

Found while dogfooding the #2150 flip on the use2 box; not a regression in that work.

Correcting my own option 1 above, and it changes the answer rather than refining it.

I wrote "if it only needs identity (names, ids, host, engine) then this is clean." It does not only need identity. The MCP host builds a PgPlanFetcher whose resolver hands back a full connection string per server (Mcp/DarlingMcpHostService.cs:407):

var serversById = new Dictionary<int, MonitoredServer>();
foreach (var server in registryServers) { serversById.TryAdd(server.ServerId, server); }

var planFetcher = new PgPlanFetcher(
    serverId => serversById.TryGetValue(serverId, out var server)
        ? DarlingServerConnector.ResolveConnectionString(server, _logger)
        : null,
    _logger);

So a credential-free read would leave ResolveConnectionString with nothing to resolve, and live plan fetch would break for every server rather than only for store-registered ones — strictly worse than today's fallback. Option 1 as filed is wrong.

The option that does work, and why the security property survives

The MCP host is a BackgroundService in the same process as the collector (Program.cs:356 DarlingWorker, :360 DarlingMcpHostService), and the worker already loads the same view over its own privileged connection (DarlingWorker.cs:979 LoadViewAsync, refreshed at :2183) — it must, or it could not connect to anything to collect. So the process already holds the credentials the MCP host needs; the MCP host is re-reading the same rows over a second, deliberately-restricted connection and failing.

The fix is to stop doing that: source the MCP host's server map from the process's existing privileged load rather than from a fresh read as the mcp role.

That keeps the carve's actual security property intact, which is worth being precise about because it is easy to read this as "so grant mcp the credential after all." The property is that a token-holder talking to the MCP server cannot obtain a stored credential — no MCP tool returns encrypted_password, and the DPAPI blob is machine-bound anyway. The mcp ROLE's inability to SELECT that column is one mechanism for that property, not the property itself, and it is the wrong mechanism for an in-process consumer that is already trusted with the credential by virtue of running inside the service. Granting the role read access would weaken it (anything holding the mcp credentials could then read blobs directly); sourcing from the in-process load does not, because it crosses no new boundary.

The role split still earns its keep for the surface it was built for: the Viewer connects over the network as viewer, and that is where table- and column-level denial is doing real work.

What is still a decision, and what is not

Not a decision: today's behaviour is wrong regardless of which fix lands. The read fails on every start, the whole view is abandoned for one denied column, and the warning reads as transient when it is permanent.

Still Erik's: whether the MCP host should share the worker's loaded view (needs a small seam — the worker refreshes on a config-version bump, so "whose copy is authoritative and when" has to be answered rather than assumed), or whether PgPlanFetcher should route through the service's config_command pivot so the MCP host never holds a credential at all. The second is a bigger change and the better end state if live plan fetch is meant to be service-mediated; the first is small and fixes the observed break.

Two smaller things I would fix either way, neither of which needs that answer:

LoadViewAsync fails whole for one denied column. Alert settings and collector schedules are lost with it. The notification arm already degrades gracefully (Stop MCP losing the server registry over a password it never reads #2293), so the pattern exists.
No test keeps the readers inside the carve. Both halves of this bug are a query selecting a column its role is denied — compiles, passes every text assertion, fails only against a provisioned store as a non-superuser. A live-postgres test that runs each StoreConfigProvider read as mcp and as viewer would have caught both, and would catch the next column added to a secret table. That one I can write without any design call, and it is the piece that stops this recurring a third time.

Bring the class doc forward to the #2298 fix it now pins (review nit)

Source the MCP host's server map from the worker's privileged load (#…

Fixed by #2304, squash-merged to dev as 40319e7: the MCP host no longer performs any config-view read of its own — the plan-fetch resolver serves the worker-published MonitoredServerRegistryState, so the mcp role's secret-column carve (encrypted_password, the notification secrets) can never cost it the registry again. Reaches the box with the next nightly + service upgrade.

Verified live on the dogfood box. Upgraded prod-sql-use2-monitor-01 to 3.4.0-nightly.20260817 (carrying 40319e7); on startup the MCP host bound cleanly with no "could not read the monitored-server registry" line and no 42501 — the registry is served from the worker's privileged load as designed. The use1 twin's binaries were refreshed to the same nightly in the same pass (service still uninstalled pending its darling_monitor credentials).

Navigation Menu

Repository navigation

[BUG] MCP host still cannot read the config store — encrypted_password is SELECT-carved from mcp but ReadMonitoredServersAsync selects it (#2293 fixed only the notification half) #2298

What happens

Why — and it is not a missing grant

This is the second half of #2293, and I should have caught it there

Options, since the fix is a decision rather than a one-liner

Two things this also suggests

The option that does work, and why the security property survives

What is still a decision, and what is not

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

[BUG] MCP host still cannot read the config store — encrypted_password is SELECT-carved from mcp but ReadMonitoredServersAsync selects it (#2293 fixed only the notification half) #2298

Description

What happens

Why — and it is not a missing grant

This is the second half of #2293, and I should have caught it there

Options, since the fix is a decision rather than a one-liner

Two things this also suggests

Activity

erikdarlingdata commented on Aug 16, 2026

The option that does work, and why the security property survives

What is still a decision, and what is not

erikdarlingdata commented on Aug 17, 2026

erikdarlingdata commented on Aug 17, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions