Stop the config backups granting INTERACTIVE read, and pin the exact identity set (#1769)

[erikdarlingdata]

Closes #1769, which has been retitled to what actually ships: "Config BACKUPS were interactive-readable; darling.json's INTERACTIVE grant is load-bearing by design (Viewer)".

The original ask — drop the INTERACTIVE ACE from darling.json itself — is not being made, because it would break the shipped Viewer. That design question, with all three options recorded verbatim, now lives in #1792 and does not close with this PR.

The issue's fix shape does not survive contact, and this is why

#1769 asks for the INTERACTIVE ACE to be dropped from darling.json's hardening so the emitted DACL is exactly SYSTEM + Administrators + the service identity. That would break the shipped Viewer, and the chain is not subtle:

1. The Viewer reads the config file directly — ViewerSettings.TryLoad does File.ReadAllText(path) at Darling/PerformanceMonitor.Darling.Viewer/ViewerSettings.cs:122.
2. It runs non-elevated — <requestedExecutionLevel level="asInvoker" /> at Darling/PerformanceMonitor.Darling.Viewer/app.manifest:20, wired via <ApplicationManifest> in the Viewer csproj.
3. Under UAC, an interactive administrator's default token carries BUILTIN\Administrators as SE_GROUP_USE_FOR_DENY_ONLY, so an Allow ACE for Administrators grants that token nothing. A three-identity DACL therefore denies the Viewer.
4. The failure is total, not degraded: MainWindow.OnLoaded catches and shows "darling.json could not be read: …" (MainWindow.xaml.cs:174-178). No connection, no data.
5. It hits the default shipped layout — the release zip puts the Viewer in viewer\ under the service root, and it probes the parent for the service's darling.json (ViewerSettings.cs:92-104).

The existing code says as much at DarlingWorker.cs:540-542, and install-darling.ps1 mirrors the grant deliberately. So the current behavior is intentional; the issue and the design disagree, and the issue did not account for the Viewer.

The security rationale is still sound, to be clear. I checked the obvious rebuttal: the Viewer already gets INTERACTIVE read on pg-admin-credential.dpapi by design, but the admin role's SELECT carves out the secret columns, so darling.json really is the only interactive-readable path to the monitored servers' SQL passwords and the MCP/web tokens. The tightening is worth doing — it just cannot be done by deleting the ACE alone, and choosing how to give the Viewer a non-secret path to what it needs (it only wants the postgres section) is a design decision, not a small fix.

What this PR does instead

The config BACKUPS have no such constraint. A darling.json.bak-* is a byte-for-byte copy of the same secrets, and nothing reads one: the only references to the .bak- name in non-test code are the two lines that CONSTRUCT it (DarlingCliCommands.cs:1333,1336), and restoring one is a hand operation that already requires elevation, because writing darling.json does — INTERACTIVE only ever had Read here, never Write.

So the grant bought nothing and left a second readable copy of every encrypted password and access token, exposed to any interactively-logged-on user. HardenConfigBackup now passes allowInteractiveRead: false, and install-darling.ps1 gets the same split so the installer and the service cannot disagree — its summary line now states which file gets which posture rather than claiming one rule for both.

This is correct under all three ways the headline question could be decided, which is why it is worth landing on its own.

Pins

Two, because the two failure modes are different.

• HardenFile_NonInteractive_EmitsExactlyTheThreeIntendedIdentities — the exactly-three-identities assertion the issue asked for, valid on the non-interactive posture (superuser credential, transient init pwfile, and now backups). Presence/absence assertions pass happily while a fourth principal rides along, which is exactly the reported defect's shape, so this one names the intruder.
• The end-to-end assertion added to WriteWithBackup_HardensTheBackup_… runs through the real WriteWithBackupAsync path, because the thing being fixed is a call site passing the wrong argument — a test of the ACL shape alone cannot see that.

Mutation table

#	Mutation	Result
M5	Make HardenFile add INTERACTIVE unconditionally	RED (2): the new pin, reporting granted principals beyond SYSTEM, Administrators and the service identity: NT AUTHORITY\INTERACTIVE, plus the existing superuser test
M6	Revert HardenConfigBackup to allowInteractiveRead: true	RED (1): WriteWithBackup_HardensTheBackup_BecauseFileCopyDoesNotCarryTheDacl

M6 is the one that matters for this change: it proves the call site is pinned, not merely the shape.

Verification

• Darling.Tests full suite: 3485 passed, 0 failed, 171 skipped (the live-Postgres tests skip without DARLING_TEST_PG; unrelated to this change).
• DarlingFileSecurityTests: 12/12.
• dotnet build … Darling.Tests -c Debug -t:Rebuild and … Darling.Service -c Debug -t:Rebuild — both 0 Warning(s), 0 Error(s).
• install-darling.ps1 parses clean (Parser::ParseFile, 0 errors).

Issue hygiene done alongside this

#1769 carries a comment with the full six-link chain at file:line (verified at 50012266) and an explicit decision block; it is retitled to the shipping scope; and #1792 was opened to hold Erik's three-way decision on darling.json so it survives this issue closing.

Note for the prompt maintainer

The client checklist's step-14 note currently says INTERACTIVE:(R) "will be removed in a future build". That is now false and wants rewording to by-design-pending-#1792 — this PR does not change darling.json's own DACL, so the observation stays expected on a field box. It also gains a new falsifiable check: post-this-build, any darling.json.bak-* shows three identities only.

🤖 Generated with Claude Code