fix(blocknode): keep a hand-created host firewall across block node reconfigure

[alex-au]

Description

A host firewall created with the standalone network firewall create verb was silently deleted by the next block node reconfigure — table, both on-disk artifacts, and the management allowlist — while the command reported success.

$ sudo solo-provisioner network firewall create --from-file /root/rules.yaml --force
INF inet weaver-host-firewall firewall is in the desired state

$ sudo solo-provisioner block node reconfigure -p local --non-interactive
INF Removing host firewall (inet weaver-host-firewall)   step_id=network-firewall-delete
INF Host firewall removed                                status=success
INF Successfully reconfigured Hedera Block Node

$ sudo nft list table inet weaver-host-firewall
Error: No such file or directory

Two sources of truth that never synced. reconfigure seeds its enable/disable choice from MachineState.Firewall (cmd/cli/commands/block/node/reconfigure.go:61), which only the block-node workflow ever wrote — patchMachineFirewallFromConfig runs inside the intent handlers. The standalone verbs stopped at the manager, so a hand-created firewall left the record nil, which is indistinguishable from "explicitly disabled". The no-flag seed resolved to disabled and NetworkPlaneSteps took the teardown branch.

On a remote host with no console, tearing down a management allowlist is a truck roll — so this PR makes the two paths agree, and additionally makes removal of an active firewall require saying so.

What changed

The standalone verbs record the decision. network firewall create and delete --all now write machineState.firewall.disabled, the same field the block-node workflow writes. Best-effort by design: network firewall create is node-agnostic and may run on a host with no state file, and an nft ruleset already applied to the kernel must not be reported as a failure because a bookkeeping write missed. The delete side matters as much as create — a record still saying "enabled" would make the next reconfigure re-create a table the operator just removed.

The seed consults kernel ground truth. common.ResolveFirewallSeed falls back to Manager.IsActive, which that method's own doc comment already claimed was wired up (internal/network/firewall/manager.go:194-201) but which had no caller outside the rollback guard. A live table always seeds enabled, whatever state records, so removing an active host firewall is only reachable through an explicit --firewall-enabled=false or an interactive decline. The converse is untouched: with no live table the recorded decision stands, so a reconfigure still re-asserts a firewall state says should be there.

The live firewall becomes a content precedence tier. Content resolution is now flag > --config > live firewall > MachineState.Firewall > default. /etc/solo-provisioner/network-weaver-host-firewall.yaml is always at least as fresh as machine state — every path that writes machine state also re-renders the firewall, but the standalone verbs write only the firewall. Without this tier a reconfigure's force re-render would revert an urgent network firewall add --name mgmt --cidr … back to the allowlist captured at install time.

Why the decision is mirrored but the content is not

models.HostConfig is flag-shaped and strictly narrower than a firewall.Table: its CIDR fields are IPv4-only and its port fields are []int, where a Rule holds port specs that may be inclusive ranges (2379-2380). Mirroring reserved-block content into machine state would therefore have to drop exactly the kind of operator data this issue is about. Reading the firewall's own config file instead is lossless for the path that matters, and hostConfigFromTable leaves out what HostConfig cannot hold — with a warning naming the skipped values, rather than silently, or in the IPv6 case failing HostConfig.Validate and aborting the whole reconfigure.

Base

Targets main. This was originally opened against 00996-collapse-icmp-path-health-accepts (#997), because the declarative YAML config and named allow rules it builds on landed via #999, which had merged into 00996 rather than main. Now that #997 is in main (7facfc1), the branch has been rebased with --onto so only this PR's own commit is replayed — a plain rebase would have re-applied commits already folded into that squash merge.

Files changed

File	Change
cmd/cli/commands/network/firewall/state.go	new — recordHostFirewallDecision, the best-effort decision write
cmd/cli/commands/network/firewall/state_test.go	new — decision-write tests over an in-memory fake state.Manager
cmd/cli/commands/network/firewall/create.go	record disabled: false after a successful Create, no-op included
cmd/cli/commands/network/firewall/delete.go	record disabled: true after a successful --all teardown
cmd/cli/commands/network/firewall/firewall_test.go	stub state inside stubManager so no unit test can touch the host's real state
cmd/cli/commands/common/host_firewall.go	ResolveFirewallSeed, mergeLiveHostFirewall, and the Table → HostConfig projection
cmd/cli/commands/common/host_firewall_test.go	seed truth table, four-tier precedence, lossy-projection guard
cmd/cli/commands/block/node/reconfigure.go	seed the firewall gate through ResolveFirewallSeed
internal/network/firewall/manager.go	IsActive doc comment now names its real callers
docs/quickstart.md	reconfigure seeding note + a network firewall note on the recorded decision
docs/dev/traffic-shaper.md	the decision/content split between machine state and the firewall's config

Review guide

Code-review checklist

• ResolveFirewallSeed (cmd/cli/commands/common/host_firewall.go) — verify the direction of the override is one-way: a live table forces enabled, but an absent table never forces disabled over a recorded disabled: false. The probe error path must fall back to the recorded decision (pre-existing behaviour), not to a guess.
• hostConfigFromTable — every field it fills must survive HostConfig.Validate(). This is asserted directly in TestHostConfigFromTable_SkipsWhatHostConfigCannotHold; an IPv6 mgmt CIDR reaching HostConfig would turn a working reconfigure into a hard error, which is worse than not seeding the field.
• Ordering in ResolveHostFirewallConfig — mergeLiveHostFirewall must run before mergeHostFirewallFromState; both only fill empty fields, so the order is what encodes the precedence.
• recordHostFirewallDecision — confirm it can never return an error to the verb. The nft ruleset is already live at that point.
• NetworkPlaneSteps (internal/workflows/network_setup.go:85) is deliberately unchanged: once the seed is ground-truth-backed, Host.Disabled == true with a live table can only come from an explicit flag or prompt, so threading an "explicit opt-out" boolean through NetworkPlaneOptions would buy nothing.

Test commands

task test:unit
go test ./cmd/cli/commands/network/firewall/... ./cmd/cli/commands/common/...
go test -run 'TestResolveFirewallSeed|TestMergeLiveHostFirewall|TestHostConfigFromTable' ./cmd/cli/commands/common/...

These packages transitively import the Linux-only internal/mount, so they do not build on macOS — run them in the VM (task vm:test:unit) or a Linux container.

Manual UAT (not yet run — Debian 13 VM, the issue's exact reproducer)

# 1. install with NO --firewall-enabled, so machineState.firewall is absent
sudo solo-provisioner block node install -p local --non-interactive

# 2. operator sets up the host firewall by hand
sudo solo-provisioner network firewall create --from-file /root/rules.yaml --force
sudo nft list table inet weaver-host-firewall | grep -c 'saddr @'

# 3. an unrelated reconfigure
sudo solo-provisioner block node reconfigure -p local --non-interactive

# expected: table, artifacts, mgmt set and every named allow rule all survive
sudo nft list tables | grep weaver-host-firewall
ls /etc/solo-provisioner/network-weaver-host-firewall.{nft,yaml}

# 4. the explicit opt-out still tears it down
sudo solo-provisioner block node reconfigure -p local --non-interactive --firewall-enabled=false
sudo nft list tables | grep weaver-host-firewall   # gone

# 5. the membership verbs are no longer reverted by a reconfigure
sudo solo-provisioner network firewall add --name mgmt --cidr 203.0.113.0/24
sudo solo-provisioner block node reconfigure -p local --non-interactive
sudo nft list table inet weaver-host-firewall | grep 203.0.113.0/24

The host firewall's input chain is policy drop, so any UAT that actually loads the ruleset needs mgmt.cidrs covering the tester's own SSH source, or it locks the box.

Risks / rollback

• A live table now pins the seed to enabled. A host left with a stale table after a failed disable keeps the firewall across a bare reconfigure instead of converging to removed. That is the deliberate trade the issue asks for; the explicit opt-out still converges.
• New state-write path from the network command tree, on a host that may have no state file. Mitigated by making the write best-effort and never failing the verb.
• Rollback is a plain revert: the change is additive at the call sites and writes no new on-disk shape — a machineState.firewall block written by a standalone verb is exactly the shape the block-node path already writes.

Related Issues

• Closes fix(blocknode): block node reconfigure silently deletes a host firewall created with network firewall create #1003

[swirlds-automation]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[Copilot]

Pull request overview

This PR fixes a block-node operational footgun where a host firewall created via the standalone solo-provisioner network firewall create command could be silently torn down by a later block node reconfigure due to missing/unsynced persisted “enablement decision” state. It aligns the standalone firewall verbs with the block-node workflow by recording the enable/disable decision, and it makes reconfigure seeding consult live kernel ground truth so an active firewall is never removed by default.

Changes:

• Persist the host-firewall enable/disable decision from network firewall create and network firewall delete --all into runtime state.
• Seed block node reconfigure’s firewall gate from a new ResolveFirewallSeed that prefers “live table exists ⇒ enabled” over an absent/disabled recorded decision.
• Add a “live firewall config” precedence tier so urgent operator edits in /etc/solo-provisioner/network-weaver-host-firewall.yaml aren’t reverted by reconfigure.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
cmd/cli/commands/network/firewall/state.go	New best-effort runtime-state decision writer for standalone firewall verbs.
cmd/cli/commands/network/firewall/state_test.go	Tests for decision persistence behavior using an in-memory state.Manager.
cmd/cli/commands/network/firewall/create.go	Record disabled=false after successful create (including no-op create).
cmd/cli/commands/network/firewall/delete.go	Record disabled=true after successful --all teardown.
cmd/cli/commands/network/firewall/firewall_test.go	Ensure unit tests stub state so tests never touch host runtime state.
cmd/cli/commands/common/host_firewall.go	Add ResolveFirewallSeed, live firewall merge tier, and table→HostConfig projection.
cmd/cli/commands/common/host_firewall_test.go	Seed truth table tests + precedence tests + lossy-projection guard.
cmd/cli/commands/block/node/reconfigure.go	Use ResolveFirewallSeed instead of only persisted state for firewall seeding.
internal/network/firewall/manager.go	Update IsActive doc comment to reflect real callers.
docs/quickstart.md	Document new seeding/precedence behavior and decision recording.
docs/dev/traffic-shaper.md	Document decision/content split between machine state and firewall config file.

Suppressed comments (1)

cmd/cli/commands/common/host_firewall.go:439

• This warning recommends network firewall set --ports, but set requires --name when using --cidrs/--ports. Including --name <rule> makes the remediation actionable.

		logx.As().Warn().Strs("ports", skipped).Msg(
			"the live host firewall's " + desc + " block holds port ranges, which the flag-shaped config cannot " +
				"express; they are not seeded and a re-render would drop them — re-apply them afterwards with " +
				"`network firewall set --ports`")

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[brunodam]

LGTM