[erikdarlingdata]

Closes #1771.

The defect

A production field instance logged both halves of the firewall self-heal failing on every service start, for both endpoints:

[DarlingMcpHostService] Could not remove the firewall rule automatically (exit 1: Remove-NetFirewallRule : Access is denied...)
[DarlingWebHostService] Could not configure the firewall automatically (exit 1: New-NetFirewallRule : Access is denied...)

Harmless there only because the rules already existed from an earlier manual setup. The real defect is what happens without that accident: on a fresh networked install nothing ever creates the rule, so remote MCP and dashboard clients are firewalled off with only a log line to say why - and networked is the normal deployment mode for this product.

The cause is structural, not a bug in the reconcile. The service runs as NT SERVICE\PerformanceMonitor Darling, which cannot modify Windows Firewall by design. Granting it that privilege is the wrong fix and is rejected in the issue.

The elevated path (verified, not assumed)

Darling/tools/install-darling.ps1 - it gates on elevation at install-darling.ps1:65-68 and registers the service with sc.exe create ... obj= "NT SERVICE\$serviceName" at install-darling.ps1:127. Uninstall is Darling/tools/uninstall-darling.ps1:39 (sc.exe delete). Neither touched the firewall before this change.

What changed

Rules are created by the installer. New --configure-firewall verb (DarlingCliCommands.ConfigureFirewallAsync) reconciles all three scoped rules - store, MCP, web - against darling.json in one pass. install-darling.ps1 runs it as step 4c (before the first start) and again after the -Network wizard, which rewrites the very exposure the first pass acted on. uninstall-darling.ps1 removes them by their shared DisplayName prefix, so a rule left over from a previous port is cleared too.

The verb reads only darling.json - no store connection, no credentials - which is what lets it run at install time before the store has ever booted, unlike --enable-mcp/--enable-web, which write the control-plane store. It is idempotent, so an upgrade re-running it is a no-op.

Exposure is delegated, not re-derived. PlanFirewallRules asks the same resolvers the running service fail-closes on (ResolveNetworkExposure, ResolveMcpBind, ResolveWebBind). Re-reading listen would be worse than duplicated logic: the installer would open a port for an endpoint the service binds loopback-only, and then the service's own start-up check would flag - every start - a rule the installer had just deliberately created. The CIDR that reaches the rule is the parser's canonical output, never the operator's raw string (IPNetwork.TryParse masks host bits rather than rejecting them, so 192.168.1.77/24 opens 192.168.1.0/24).

The runtime write degrades to a read. DarlingFirewallCheck probes with Get-NetFirewallRule and reports; it never writes. Reporting is gated on a state change, because the MCP and web hosts retry a failed start on a 30-second backoff - a check wired where the reconcile sat would have traded access-denied spam for warning spam.

The bundled store had the identical bug on its own port and is fixed the same way, though the issue named only 5152/5153. Two behaviors deliberately end: a service stop no longer removes the rule (it could never succeed; the rule is install-managed now, scoped to a port nothing is listening on, and the next start reports it as stale), and _runningNetworkMode, which existed only to drive that removal, is gone.

A port change strands the old rule, so the verb sweeps per surface. The port is part of the rule name, so changing a port does not update a rule — it makes a different one and leaves the old behind as an inbound allow rule on a port nothing serves. Reconciling by exact name could never reach that, so each surface gets a ... (port *) wildcard sweep before its current rule is ensured. This is the "update port if the rule exists with a different port" half of the brief, which my first pass missed. The wildcard is derived from the rule name, never assembled from an operator string, and a name with no port suffix comes back UNCHANGED rather than gaining a * — a wildcard built from a malformed name could match, and therefore delete, rules this product does not own. The sweep runs as its own step rather than concatenated ahead of the open, because it ends in exit 0 and would otherwise terminate the shell before the rule was created while still reporting success; I wrote that bug, caught it, and pinned it with a call-shape test.

Known divergence, ruled: the verb reads the FILE port, the runtime checks the LIVE port

mcp.port / web.port are control-plane after the first run: the Viewer's Settings writes config_service, and the host binds published?.Port ?? config.Mcp.Port (DarlingMcpHostService.cs:143, feeding TryStartServerAsync at :148/:163). The runtime check therefore probes the live port. --configure-firewall reads config.Mcp.Port — the file port (DarlingCliCommands.cs:1874-1875, :1884-1885). So after a port change made from the Viewer's Settings on an exposed endpoint, the verb would open the file port while the service listens on the control-plane one.

Disposition: documented, not changed. Not a regression — the old code could never create a rule for the changed port either, because every write was access-denied. And the degraded outcome is already self-healing in the right direction: the runtime check probes the LIVE port and prints the exact New-NetFirewallRule for the CORRECT one, so the operator has a working one-liner, and the per-surface sweep above guarantees the stale file-port rule dies rather than lingering as an open inbound allow. What remains imperfect is only that --configure-firewall's one-step fix re-creates the file-port rule in that specific scenario.

The two alternatives were considered and rejected on their own costs. Reading the control plane would be correct in every case, but it would give the install-time verb a store dependency — a credential decrypt plus a connection — and "no store, no credentials" is exactly the property that lets install step 4c run before the store has ever booted; reintroducing that dependency defeats the architecture the verb exists to fit. Making the port file-only for firewall purposes would change product behavior on a niche path, a bigger blast radius than the problem it solves. If control-plane port changes ever turn out to be a common flow, that third option is the revisit — noted, not built.

Loopback-vs-networked gating

The warning is gated on the resolved bind, so it fires only where network exposure is real:

config state	verdict	log
loopback-only, no rule	LoopbackNoRule	nothing - the healthy default
loopback-only, rule still open	LoopbackStaleRule	one WARN naming the removal
exposed, rule present	ExposedRulePresent	one INFO
exposed, rule missing	ExposedRuleMissing	one WARN naming the exact New-NetFirewallRule
probe unusable	ProbeFailed	one WARN saying it could not tell

A default loopback install therefore logs nothing at all about the firewall, which is the constraint from the issue. A ProbeFailed is its own verdict rather than collapsing into "absent", which would manufacture a missing-rule warning out of a broken probe.

Field-verification signature (for the client re-run)

On the next upgrade of the field instance, the access-denied lines disappear entirely. Neither Could not configure the firewall automatically nor Could not remove the firewall rule automatically is emitted anywhere any more - the first string is gone from the codebase outright, and the second survives only inside a code comment describing the historical trap, never as a log call. The service issues no firewall write at all, so there is nothing left to be denied.

That box has pre-existing rules for 5152 and 5153, so the expected replacement is one INFO line per exposed endpoint, once per start:

Firewall rule 'PerformanceMonitor Darling MCP (port 5152)' is present. The service verifies this rule but never modifies it — firewall rules are created by the elevated installer (install-darling.ps1).
Firewall rule 'PerformanceMonitor Darling Web (port 5153)' is present. The service verifies this rule but never modifies it — firewall rules are created by the elevated installer (install-darling.ps1).

Compare NAMES, not just presence

The product owns these exact DisplayName templates, and the configured port is part of the name:

PerformanceMonitor Darling store (port <postgres.port>)
PerformanceMonitor Darling MCP (port <mcp.port>)
PerformanceMonitor Darling Web (port <web.port>)

At default ports that is exactly:

PerformanceMonitor Darling store (port 5641)
PerformanceMonitor Darling MCP (port 5152)
PerformanceMonitor Darling Web (port 5153)

To see what a box actually has (the read needs no elevation):

Get-NetFirewallRule -DisplayName 'PerformanceMonitor Darling *' | Select-Object DisplayName, Enabled, Action

If a WARN naming a MISSING rule appears while a hand-made rule for that port plainly exists, that is a real finding rather than a regression: the manual rule's DisplayName does not match the scoped name the product uses, so the product cannot see it and does not manage it. The port in the expected name must match the port that endpoint is actually configured for. --configure-firewall elevated is the one-step fix, and it also removes the superseded rules it does own.

Live verification

Get-NetFirewallRule needing no elevation was verified on this OS (Windows 11 Pro 26200), not assumed: under a runas /trustlevel:0x20000 restricted token, enumerate-all and exact-name reads succeed while New-NetFirewallRule returns PermissionDenied: Access is denied - the exact field signature, same token. Two shape facts that drove the probe design were verified the same way: an exact -DisplayName matching nothing raises ObjectNotFound (so "absent" must not look like a failure), and a wildcard matching nothing does not (so the uninstall sweep is safe).

The verb itself was run against a real firewall:

• exposed MCP config -> created exactly one rule, TCP 5152, inbound, RemoteAddress 192.168.1.0/255.255.255.0, other two absent
• fail-closed config (allowFrom: "not-a-cidr") -> opened nothing, printed why, removed the stale rule
• loopback config -> removed all three; machine left with zero Darling rules
• port change: with a leftover PerformanceMonitor Darling MCP (port 5152) rule present and mcp.port moved to 5199, one run left exactly one rule, on 5199 — the stale allow rule that the exact-name reconcile would have left open is gone
• the exact probe command, run non-elevated, returned 1 for a present rule and 0 for an absent one, clean exit state both times

The runtime check's log lines themselves are the one behavior with no automated test (they need a live service plus a live firewall); the probe command they run, the verdict they map, and the remedy they print are all pinned purely, and the probe was exercised live as above.

Mutation table

Every mutation watched red. M8 came back GREEN on the first run and exposed a weak test - recorded here rather than quietly fixed.

#	mutation	pin	result
M1	restore the runtime New-NetFirewallRule write (MCP host)	RuntimeHosts_VerifyTheFirewall_AndNeverWriteIt	RED
M2	plan exposure from listen alone, not the resolver	PlanFirewallRules_NeverOpensAPortTheServiceWillNotExpose	RED (4 cases)
M3	drop the probe's explicit exit 0	BuildProbeCommand_TreatsAnAbsentRuleAsAnAnswer_NotAFailure	RED
M4	unparseable probe means "absent" instead of ProbeFailed	TryParseProbeOutput_IsNullWhenItCannotTell	RED
M5	report every tick instead of on state change	ShouldReport_OnlyWhenTheStateChanges	RED
M6	emit an open command with no CIDR (world-open rule)	BuildRemedyCommand_RefusesToBuildAnOpenCommandWithoutACidr	RED
M7	rename the shared rule prefix (orphans rules on uninstall)	UninstallScript_SweepsByTheSharedPrefix	RED
M8	installer's verb invocation replaced	InstallScript_ReconcilesTheFirewall	GREEN - pin too weak
M8a	(after strengthening) invocation replaced	InstallScript_ActuallyInvokesTheFirewallVerb	RED
M8b	wrapper still defined, every call site deleted	InstallScript_ActuallyInvokesTheFirewallVerb	RED
M9	verb dropped from the dispatch allow-list	ConfigureFirewallVerb_IsDispatchable	RED
M10	store bootstrap reverts to writing its own rule	TheStoreBootstrap_VerifiesTheFirewall_AndNeverWritesItAtRuntime	RED
M11	sweep never widens to a wildcard (a port change strands the old rule)	SurfaceRuleWildcard_CoversEveryPortOfOneSurface	RED (3 cases)
M12	sweep invents a wildcard from an unrecognized name	SurfaceRuleWildcard_NeverInventsAWildcardFromAnUnrecognizedName	RED (3 cases)
M13a	concatenate the sweep ahead of the open — sweep + "; " + enable (exit 0 kills it; rule never created, still reports success)	ConfigureFirewall_RunsTheSweepAsItsOwnStep_NeverConcatenatedAheadOfTheOpen	RED
M13b	same bug, no spaces around the + — the retired pin missed this	same	RED
M13c	same bug via an intermediate variable	same	RED
M13d	same bug via string.Concat — the retired pin missed this, and so would a trailing-comma-only pin	same	RED
M14	sweep uses SilentlyContinue (access denied would report exit 1: with an EMPTY message)	BuildFirewallSweepCommand_IsStandalone_AndReportsRealFailuresLoudly	RED
M15	delete the POST-WIZARD reconcile (install-darling.ps1:269)	InstallScript_ReReconcilesInsideTheNetworkWizardBranch	RED
M16	move the pre-start reconcile to AFTER Start-Service	InstallScript_ReconcilesTheFirewallBeforeTheFirstStart	RED

A third pin upgraded, from tripwire to proof. The delta review flagged M13 as the weakest guard in the change: it blacklisted two literal spellings of the concatenation bug, so it caught the regression I happened to write rather than the class. It now asserts POSITIVELY that the sweep sits in TryRunFirewallStepAsync's command position, over whitespace-normalized source so reformatting cannot break it. Measured against four spellings of the same silent regression — all four go red, and two of them (no spaces around the +, and string.Concat) the retired pin would have missed. string.Concat would also have defeated the trailing-comma-only assertion the reviewer suggested, which is why the pin proves the argument POSITION rather than the punctuation around it.

One claim strengthened rather than added. The reviewer observed something about SurfaceRuleWildcard that I had not claimed and should have: on the no-port-suffix path it returns the name unchanged, so the sweep degrades to exactly the exact-name removal the caller already performed. The change is therefore strictly additive in blast radius — it can only ever widen a delete to other ports of the SAME surface, never to anything else. That is provable from two lines, and it is the claim worth making about a delete path, so it now says so in the code rather than merely "safe".

A second vacuous pin, caught in review and replaced the same way. The strengthened M8 pin also asserted Invoke-FirewallReconcile occurs at least twice — but the design carries a definition plus TWO calls, so deleting the post-wizard call left two occurrences and the pin stayed green. Measured rather than assumed: with that call removed the string still occurs 2 times, so the count pin would have passed. Both calls are now pinned by what they are FOR — one inside the brace-matched if ($Network) branch, one before Start-Service — and M15/M16 are the mutations that hold them. That second call is not decorative: -Network runs the wizard AFTER the pre-start reconcile and rewrites the very exposure it acted on, so losing it silently reproduces a subset of #1771 on every networked install.

M8 survived because Contains("--configure-firewall") stayed green with the real call deleted - the script also names the verb in its help block and in its "re-run this by hand" error message. The pin now asserts the exact invocation form and proves the wrapper is called somewhere other than its own definition, and goes red both when the call is replaced (M8a) and when every call site is removed (M8b).

Tests and build

• 3470 passed / 0 failed / 169 gated-live skipped (full Darling.Tests), 53 of them new in DarlingFirewallCheckTests
• Projects built, both -t:Rebuild: PerformanceMonitor.Darling.Service and Darling.Tests - 0 Warning(s), 0 Error(s) each
• The repo's own DocCommentHygieneTests caught a doc block I stranded while reordering fields; fixed by moving the block back onto its member, not by deleting it

No schema change, no new NuGet packages, no changes outside Darling.

Auto-merge deliberately NOT enabled - an adversarial review gates this PR.

🤖 Generated with Claude Code