Harden existing darling.json.bak-* files at service start (#1816) - #1818
Merged
Conversation
Field-verified exposure: backups created before #1786 fixed the creation path kept inherited BUILTIN\Users read - full DPAPI recoverability for every stored credential and token by any local account. The installer's check flags them but only prints the fix. The config-hardening sweep now also hardens every darling.json.bak-* sibling (allowInteractiveRead: false, matching the creation path), with the live file's best-effort + runnable-remediation contract and the still-readable CRITICAL backstop. One service start closes the exposure fleet-wide with no human in the loop. ACL tests against real files pin: both backups stripped, live file left to its own hardening, lookalike backups untouched, no-backup no-op; watched red by disabling the hardening call. Full fast suite 3572 green; zero warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nfig-backups # Conflicts: # CHANGELOG.md
This was referenced Jul 29, 2026
MisterZeus
pushed a commit
to MisterZeus/PerformanceMonitor
that referenced
this pull request
Aug 3, 2026
…stops overclaiming erikdarlingdata#1957: the SECURITY WARNING was honest. After applying the hardened ACL the installer set the service account as owner by handing Set-Acl a freshly constructed FileSecurity carrying nothing but that owner. Set-Acl applies the whole descriptor it is given, so that call also wrote an empty, unprotected DACL - re-enabling inheritance and handing BUILTIN\Users read back on an install under C:\, one statement after the hardening. Confirmed on a scratch layout under C:\ before changing anything: immediately after the owner step the file read protected=False with BUILTIN\Users present, all four inherited ACEs back and every hardened ACE gone. erikdarlingdata#1818's startup sweep re-hardened everything seconds later at the first service start, which is why the operator's own before/after captures both looked clean and only the installer disagreed. The owner now goes onto the file's CURRENT descriptor, so the hardened DACL rides along and the verified state is the final state. Proven by running the SHIPPING hardening block against the scratch layout: fresh install and a second install over the already-hardened layout both report 3 hardened / 0 warned with protected=True, no BUILTIN\Users, owner = the service SID; and a control where the harden genuinely fails still names the files, which are world-readable on disk at that moment. The per-file verification predicate is untouched - it was correct throughout. The install summary now states that the service re-verifies these ACLs at every start. erikdarlingdata#1958: the retention summary read "(raw 4 days, hourly CAGGs 90 days; daily CAGGs kept indefinitely)" - a universal claim with three counterexamples in timescaledb_information.jobs, where the docs send an operator to check it. query_store_stats_interval_hourly carries 7 days by design (erikdarlingdata#1849's dedup tier, sized to outlive raw), its daily twin carries 10, and the nine baseline aggregates keep 35. The line now names every tier that has a policy, with every number interpolated from its constant per erikdarlingdata#1942. Darling/README.md gains the rollup-tier ladder, which was not documented anywhere user-facing - its Retention section described only the collector horizons, so the three rolled-up raw tables read as 30 days when a 4-day policy is what actually drops them. Both fixes are newly pinned; neither line had a test before. The summary pin derives its expectation from RetentionPolicies itself, so a tier added on a new horizon fails until the summary mentions it. Watched red: the summary pin fails naming IntervalRetentionInterval against the old wording, and the installer pin fails both against the bare descriptor and against a second bare descriptor left after a correct re-read. Also fixes two pre-existing xUnit2013 warnings on dev in ProcedureStatsCollectorDefinitionTests, restoring the zero-warning build. Closes erikdarlingdata#1957 Closes erikdarlingdata#1958 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1816 — the security half of the round-5 field findings, same day.
The exposure, field-verified
darling.json.bak-*files created before #1786 fixed the backup-CREATION path kept whatever ACL the install folder handed them — on the reporting box, inheritedBUILTIN\Users:(I)(RX). The backups carry the same machine-scoped DPAPI blobs as the live config, so read access = any local account can recover every monitored server's SQL credential plus the MCP and web bearer tokens. The service's hardening sweep covered only the live file; the installer's security check flags the backups but only prints the remediation.The fix
TryHardenConfigBackupsruns beside the live file's hardening at every service start: everydarling.json.bak-*sibling getsHardenFile(allowInteractiveRead: false)— matching the creation path, since backups are rollback artifacts nothing interactive reads — under the live file's exact contract: best-effort, a failure logs the runnable icacls remediation naming the file, and a backup still readable by ordinary Users after the attempt logs CRITICAL. One service start closes the exposure on every affected install with no human in the loop; the installer's check remains the independent witness.Test plan
unrelated.json.bak-*untouched; a no-backup start a clean no-opField note: the affected box was remediated manually with the installer's printed icacls lines; this makes that permanent and universal.
🤖 Generated with Claude Code