From 515879dcc889b0e739629622915d0a5a965d1589 Mon Sep 17 00:00:00 2001
From: Joe Cooper <swelljoe@rocky-10-dev>
Date: Sat, 5 Sep 2026 17:35:03 -0500
Subject: [PATCH 1/8] Cleanup meta-confg layer

---
 nftables/active.cgi       |   2 +-
 nftables/active_table.cgi |   2 +-
 nftables/apply-boot.pl    |  25 ---
 nftables/bootup.cgi       |   7 +-
 nftables/help/intro.html  |   1 +
 nftables/import_table.cgi |  12 +-
 nftables/index.cgi        |   3 +-
 nftables/install_check.pl |   7 +-
 nftables/lang/en          |  27 +--
 nftables/lang/tr          |   2 -
 nftables/nftables-lib.pl  | 458 +++++++++++++++++++++-----------------
 nftables/postinstall.pl   |  35 +++
 nftables/save_manual.cgi  |   2 -
 nftables/setup.cgi        |   2 +-
 nftables/t/run-tests.t    | 147 +++++++++++-
 15 files changed, 465 insertions(+), 267 deletions(-)
 delete mode 100755 nftables/apply-boot.pl
 create mode 100644 nftables/postinstall.pl

diff --git a/nftables/active.cgi b/nftables/active.cgi
index 606fbff41..c69c59db5 100755
--- a/nftables/active.cgi
+++ b/nftables/active.cgi
@@ -45,7 +45,7 @@ else {
 			my $flags = $t->{'flags'} || "-";
 			my $status_key = active_table_status($t, \@saved_tables);
 			my $status = $text{'active_'.$status_key};
-			my $is_saved = table_is_webmin_managed($t, \@saved_tables);
+			my $is_saved = table_is_saved($t, \@saved_tables);
 			my $table_url =
 			    "active_table.cgi?family=".
 			    urlize($t->{'family'}).
diff --git a/nftables/active_table.cgi b/nftables/active_table.cgi
index 2c93da6c1..d66fcc071 100755
--- a/nftables/active_table.cgi
+++ b/nftables/active_table.cgi
@@ -24,7 +24,7 @@ $table || error($text{'active_table_notable'});
 assert_table_acl($table);
 my @saved_tables = get_nftables_save();
 my $status_key = active_table_status($table, \@saved_tables);
-my $is_saved = table_is_webmin_managed($table, \@saved_tables);
+my $is_saved = table_is_saved($table, \@saved_tables);
 
 ui_print_header(undef, $text{'active_table_title'}, "");
 
diff --git a/nftables/apply-boot.pl b/nftables/apply-boot.pl
deleted file mode 100755
index 5f57fbeeb..000000000
--- a/nftables/apply-boot.pl
+++ /dev/null
@@ -1,25 +0,0 @@
-#!/usr/bin/perl
-# apply-boot.pl
-# Apply Webmin-managed nftables rules from the saved configuration
-
-use strict;
-use warnings;
-no warnings 'redefine';
-no warnings 'uninitialized';
-our ($module_name, $no_acl_check);
-$no_acl_check++;
-if ($0 =~ /^(.*\/)[^\/]+$/) {
-	chdir($1);
-	}
-require './nftables-lib.pl';    ## no critic (Modules::RequireBarewordIncludes)
-if ($module_name ne 'nftables') {
-	print STDERR "Command must be run with full path\n";
-	exit(5);
-	}
-
-my $err = apply_restore();
-if ($err) {
-	print STDERR $err, "\n";
-	exit(1);
-	}
-exit(0);
diff --git a/nftables/bootup.cgi b/nftables/bootup.cgi
index 3c5089b3a..9513f7545 100755
--- a/nftables/bootup.cgi
+++ b/nftables/bootup.cgi
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 # bootup.cgi
-# Enable or disable Webmin-managed nftables rules at boot time
+# Enable or disable the system nftables service at boot time
 
 require './nftables-lib.pl';    ## no critic
 use strict;
@@ -9,12 +9,13 @@ our (%in, %text);
 ReadParse();
 assert_acl('bootup');
 foreign_check("init") || error($text{'bootup_einit'});
+nftables_service_status() || error($text{'bootup_eservice'});
 
 if ($in{'boot'}) {
-	create_nftables_init();
+	enable_nftables_at_boot();
 	}
 else {
-	disable_nftables_init();
+	disable_nftables_at_boot();
 	}
 webmin_log($in{'boot'} ? "bootup" : "bootdown");
 redirect("index.cgi");
diff --git a/nftables/help/intro.html b/nftables/help/intro.html
index c3f054d6f..771c00cd7 100644
--- a/nftables/help/intro.html
+++ b/nftables/help/intro.html
@@ -1,4 +1,5 @@
 <header>Introduction</header>
 <p>nftables stores firewall rules in tables. Each table belongs to a family (such as <tt>inet</tt>, <tt>ip</tt>, or <tt>ip6</tt>) and contains one or more chains. Chains contain rules, and each rule is a sequence of tests (matches) followed by an action like <tt>accept</tt>, <tt>drop</tt>, <tt>jump</tt>, or <tt>log</tt>. Named sets can group addresses or services for reuse in multiple rules.</p>
+<p>Rules are stored in the system's own nftables configuration file &mdash; <tt>/etc/nftables.conf</tt> or <tt>/etc/sysconfig/nftables.conf</tt>, whichever the <tt>nftables</tt> service loads on this system. Anything already in that file, such as comments, <tt>define</tt> variables and <tt>include</tt> lines, is left alone. The <tt>nftables</tt> service loads that file at boot, so rules saved here survive a reboot in the same way as rules written by hand.</p>
 <p>To get started, use the Setup page to create a default ruleset, or create a table and chain manually. Then add rules (and sets) from the table view. When you are ready to activate your changes, click Apply Configuration to load the ruleset into the kernel.</p>
 <footer>nft(8)</footer>
diff --git a/nftables/import_table.cgi b/nftables/import_table.cgi
index 4dffa2351..242a9cddc 100755
--- a/nftables/import_table.cgi
+++ b/nftables/import_table.cgi
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 # import_table.cgi
-# Import an active nftables table as a Webmin-managed saved table
+# Import an active nftables table into the saved configuration
 
 require './nftables-lib.pl';    ## no critic
 use strict;
@@ -25,7 +25,7 @@ $source || error($text{'import_esource'});
 assert_table_acl($source);
 
 my @tables = get_nftables_save();
-if (table_is_webmin_managed($source, \@tables)) {
+if (table_is_saved($source, \@tables)) {
 	error(text('import_emanaged', nft_table_spec($source)));
 	}
 
@@ -52,14 +52,6 @@ if ($in{'import'}) {
 	assert_table_acl($import);
 	push(@tables, $import);
 	write_configuration(@tables);
-	register_managed_table(
-		$import,
-		'source' => 'imported',
-		'imported_from' => nft_table_spec($source),
-		'imported_from_family' => $source->{'family'},
-		'imported_from_name' => $source->{'name'},
-		'imported_at' => time()
-	);
 	webmin_log("import", "table", $source->{'name'},
 		{'family' => $source->{'family'}, 'new' => $name});
 	redirect("index.cgi?table_family=".
diff --git a/nftables/index.cgi b/nftables/index.cgi
index 665e480c3..cee147085 100755
--- a/nftables/index.cgi
+++ b/nftables/index.cgi
@@ -845,7 +845,8 @@ if ($partial) {
 
 print $rules_html;
 
-my $init_support = foreign_check("init") && check_acl('bootup') ? 1 : 0;
+my $init_support = foreign_check("init") && check_acl('bootup') &&
+                   nftables_service_status() ? 1 : 0;
 if (
 	@tables &&
 	(check_acl('active') ||
diff --git a/nftables/install_check.pl b/nftables/install_check.pl
index 26a9c7790..0f4d8d159 100644
--- a/nftables/install_check.pl
+++ b/nftables/install_check.pl
@@ -15,10 +15,5 @@ sub is_installed
 {
 my ($mode) = @_;
 return 0 if (&check_nftables());
-if ($mode) {
-	my $file = nftables_rules_file();
-	return 1 if (!-s $file);
-	return 2;
-	}
-return 1;
+return $mode ? 2 : 1;
 }
diff --git a/nftables/lang/en b/nftables/lang/en
index 731899a7d..da0cd4a08 100644
--- a/nftables/lang/en
+++ b/nftables/lang/en
@@ -52,10 +52,10 @@ index_cmovesel=Move Selected
 index_radd=Add Rule
 index_apply_changes=Apply Changes
 index_ruleset_active=View Active Ruleset
-index_ruleset_activedesc=View active nftables tables and import copies into module's saved configuration.
+index_ruleset_activedesc=View active nftables tables and import copies into the system nftables configuration.
 index_foreign_firewalls=The other configured firewall modules were detected: $1. It is recommended not to use multiple programs to change the active kernel firewall configuration at the same time.
 index_edit_manual=Edit Config Files
-index_edit_manualdesc=Edit saved nftables configuration files manually.
+index_edit_manualdesc=Edit the system nftables configuration files manually.
 index_allowip_go=Allow IP/CIDR
 index_blockip_go=Block IP/CIDR
 index_allowport_go=Add allowed port
@@ -94,8 +94,9 @@ quick_failed=Failed to save and apply quick rule: $1
 index_unapply=Revert Configuration
 index_unapplydesc=Click this button to reset the configuration listed above to the one that is currently active.
 index_bootup=Activate at Boot
-index_bootupdesc=Change whether saved module-managed nftables rules are applied at boot time or not.
+index_bootupdesc=Change whether the system nftables service loads the saved rules at boot time or not.
 bootup_einit=This system does not support boot-time service management from Webmin.
+bootup_eservice=The nftables service does not exist on this system, so it cannot be enabled at boot.
 index_return=rules list
 edit_title=Edit Rule
 edit_header=Rule Details
@@ -105,7 +106,7 @@ save=Save
 delete=Delete
 save_err=Failed to save rule
 apply_err=Failed to apply configuration
-apply_enone=No saved nftables tables were found to apply.
+apply_enone=No nftables tables were found in $1 to apply.
 apply_eexternal=Cannot apply configuration because table $1 is currently marked as externally managed.
 apply_esettype=Set $1 in table $2 has type $3, but chain $4 uses it for $5. Use ipv4_addr or ipv6_addr sets for address fields, and inet_service sets for port fields.
 setup_header=Ruleset profile
@@ -158,7 +159,7 @@ setup_port_usermin=Usermin
 setup_port_passive_ftp=FTP passive range
 setup_create=Create
 setup_invalid_type=Invalid ruleset type selected.
-setup_edup=Table $1 already exists in module's saved nftables configuration.
+setup_edup=Table $1 already exists in the system nftables configuration.
 setup_eservice=Invalid service selected: $1
 setup_failed=Failed to create ruleset profile: <pre>$1</pre>
 index_profile_setup=Create Ruleset Profile
@@ -334,11 +335,11 @@ active_chains=Chains
 active_sets=Sets
 active_rules=Rules
 active_status=Status
-active_webmin=Managed by Webmin
+active_saved=Saved in configuration
 active_external=Externally managed
-active_unclaimed=Unclaimed
+active_unsaved=Not saved
 active_import=Import Copy
-active_importdesc=Import this active table as a separate module-managed table.
+active_importdesc=Import this active table into the system nftables configuration.
 active_clear=Clear
 active_clear_all=Clear All
 active_clear_alldesc=Remove all clearable active nftables tables from the live ruleset.
@@ -349,21 +350,21 @@ active_table_notable=No such active table selected
 active_table_summary=Active table details
 clear_title=Clear active table
 clear_err=Failed to clear active table
-clear_confirm=Clear active table $1 from the live ruleset? This does not change module's saved configuration.
+clear_confirm=Clear active table $1 from the live ruleset? This does not change the saved configuration.
 clear_failed=Failed to clear active table: $1
 clear_all_title=Clear active tables
 clear_all_err=Failed to clear active tables
 clear_all_enone=No clearable active tables were found.
-clear_all_confirm=Clear $1 active table(s) from the live ruleset? This does not change module's saved configuration. Tables managed externally will not be cleared.
+clear_all_confirm=Clear $1 active table(s) from the live ruleset? This does not change the saved configuration. Tables managed externally will not be cleared.
 import_title=Import active table
-import_header=Import table as module-managed copy
+import_header=Import table into the saved configuration
 import_err=Failed to import active table
 import_esource=No such active table selected
 import_eexternal=Cannot import as table $1 because that active table is externally managed.
-import_emanaged=Table $1 is already managed by Webmin.
+import_emanaged=Table $1 is already in the saved configuration.
 import_source=Source table
 import_flags=Source flags
-import_external_note=This active table is marked as externally managed; importing creates a separate module-managed copy and does not change the active source table
+import_external_note=This active table is marked as externally managed; importing creates a separate saved copy and does not change the active source table
 import_new_name=New table name
 import_ok=Import Copy
 manual_editsel=Edit nftables configuration file
diff --git a/nftables/lang/tr b/nftables/lang/tr
index 058400e8c..93c5b623c 100644
--- a/nftables/lang/tr
+++ b/nftables/lang/tr
@@ -334,9 +334,7 @@ active_chains=Zincirler
 active_sets=Kümeler
 active_rules=Kurallar
 active_status=Durum
-active_webmin=Webmin tarafından yönetiliyor
 active_external=Dışarıdan yönetiliyor
-active_unclaimed=Sahipsiz
 active_import=Kopyayı İçe Aktar
 active_importdesc=Bu etkin tabloyu ayrı bir modül tarafından yönetilen tablo olarak içe aktar.
 active_clear=Temizle
diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl
index 2d3b5e18b..d73650dbb 100644
--- a/nftables/nftables-lib.pl
+++ b/nftables/nftables-lib.pl
@@ -5,9 +5,10 @@ BEGIN { push(@INC, ".."); };    ## no critic
 use WebminCore;
 use strict;
 use warnings;
-our (%config, %access, $module_config_directory, $module_var_directory,
-     $module_root_directory);
+our (%config, %access, %gconfig, $module_config_directory,
+     $module_var_directory, $module_root_directory);
 our ($last_config_change_flag, $last_restart_time_flag);
+our ($nftables_rules_file_cache, $nftables_service_status_cache);
 init_config();
 %access = get_module_acl();
 $last_config_change_flag = $module_var_directory."/config-flag";
@@ -193,76 +194,128 @@ return if (get_nft_command());
 return text('index_ecommand', "<tt>nft</tt>");
 }
 
-# nftables_rules_file()
-# Returns the Webmin-managed nftables rules file
-sub nftables_rules_file
+# nftables_service_name()
+# Returns the name of the system nftables service
+sub nftables_service_name
 {
-return "$module_config_directory/rules.conf";
+return "nftables";
 }
 
-# nftables_boot_action()
-# Returns the init action name for applying nftables rules at boot
-sub nftables_boot_action
+# nftables_service_unit_file()
+# Returns the path to the system nftables systemd unit, if there is one
+sub nftables_service_unit_file
+{
+return if (!foreign_check("init"));
+foreign_require("init", "init-lib.pl");
+no warnings 'once';
+return if (($init::init_mode || "") ne "systemd");
+my $unit = init::action_unit(nftables_service_name());
+my $root = init::get_systemd_root($unit);
+return $root && -r "$root/$unit" ? "$root/$unit" : undef;
+}
+
+# nftables_service_rules_file()
+# Returns the ruleset file that the system nftables service loads
+sub nftables_service_rules_file
+{
+my $unit_file = nftables_service_unit_file();
+return if (!$unit_file);
+my $data = read_file_contents($unit_file);
+return if (!$data);
+foreach my $l (split(/\r?\n/, $data)) {
+	next if ($l !~ /^\s*ExecStart\s*=/);
+	next if ($l !~ /\s-f\s+(\S+)/);
+	my $file = $1;
+	$file =~ s/^["']|["']$//g;
+	return $file if ($file =~ /^\//);
+	}
+return;
+}
+
+# nftables_rules_file()
+# Returns the system nftables ruleset file, which is the file the nftables
+# service loads at boot
+sub nftables_rules_file
+{
+return $nftables_rules_file_cache if ($nftables_rules_file_cache);
+my $file = nftables_service_rules_file();
+if (!$file) {
+	# No service to ask, so fall back to the distribution's convention
+	my @conv = ($gconfig{'os_type'} || "") =~ /^(redhat|suse|united)/
+	    ? ("/etc/sysconfig/nftables.conf", "/etc/nftables.conf")
+	    : ("/etc/nftables.conf", "/etc/sysconfig/nftables.conf");
+	($file) = grep { -r $_ } @conv;
+	$file ||= $conv[0];
+	}
+return $nftables_rules_file_cache = $file;
+}
+
+# nftables_service_status()
+# Returns the init status of the system nftables service
+sub nftables_service_status
+{
+return $nftables_service_status_cache
+    if (defined($nftables_service_status_cache));
+return $nftables_service_status_cache = 0 if (!foreign_check("init"));
+foreign_require("init", "init-lib.pl");
+return $nftables_service_status_cache =
+    init::action_status(nftables_service_name());
+}
+
+# nftables_started_at_boot()
+# Returns true if the system nftables service is enabled at boot
+sub nftables_started_at_boot
+{
+return nftables_service_status() == 2 ? 1 : 0;
+}
+
+# enable_nftables_at_boot()
+# Enables the system nftables service at boot
+sub enable_nftables_at_boot
+{
+foreign_require("init", "init-lib.pl");
+init::enable_at_boot(nftables_service_name());
+undef($nftables_service_status_cache);
+}
+
+# disable_nftables_at_boot()
+# Disables the system nftables service at boot
+sub disable_nftables_at_boot
+{
+foreign_require("init", "init-lib.pl");
+init::disable_at_boot(nftables_service_name());
+undef($nftables_service_status_cache);
+}
+
+# legacy_nftables_rules_files()
+# Returns the private rules files that releases before the switch to the
+# system nftables configuration wrote to
+sub legacy_nftables_rules_files
+{
+my @files = ("$module_config_directory/rules.conf",
+	     "$module_config_directory/nftables.conf");
+unshift(@files, $config{'save_file'}) if ($config{'save_file'});
+my %seen;
+return grep { !$seen{$_}++ && -s $_ } @files;
+}
+
+# legacy_nftables_boot_action()
+# Returns the name of the private boot action those releases created
+sub legacy_nftables_boot_action
 {
 return "webmin-nftables";
 }
 
-# nftables_boot_wrapper()
-# Returns the generated wrapper used by the boot action
-sub nftables_boot_wrapper
-{
-return "$module_config_directory/apply-boot.pl";
-}
-
-# nftables_started_at_boot()
-# Returns true if Webmin-managed nftables rules are enabled at boot
-sub nftables_started_at_boot
+# remove_legacy_nftables_init()
+# Removes the private boot action that applied the private rules file, and
+# enables the system nftables service in its place if it was in use
+sub remove_legacy_nftables_init
 {
 return 0 if (!foreign_check("init"));
 foreign_require("init", "init-lib.pl");
-return init::action_status(nftables_boot_action()) == 2 ? 1 : 0;
-}
-
-# create_nftables_init()
-# Creates or enables the boot action for Webmin-managed nftables rules
-sub create_nftables_init
-{
-foreign_require("init", "init-lib.pl");
-chmod(0755, "$module_root_directory/apply-boot.pl");
-create_wrapper(nftables_boot_wrapper(), "nftables", "apply-boot.pl");
-my $action = nftables_boot_action();
-{
-	no warnings 'once';
-	if (($init::init_mode || "") eq "systemd") {
-		my $unit = init::action_unit($action);
-		my $unit_file = init::get_systemd_root($unit)."/".$unit;
-		if (-r $unit_file) {
-			init::disable_at_boot($action);
-			init::delete_systemd_service($unit);
-			}
-		}
-	}
-init::enable_at_boot(
-	$action,
-	"Load Webmin nftables rules",
-	nftables_boot_wrapper(),
-	undef, undef,
-	{
-		'exit' => 1,
-		'opts' => {
-			'after' => 'local-fs.target systemd-modules-load.service',
-			'before' => 'network-pre.target network.target',
-			'wants' => 'network-pre.target',
-		}
-	});
-}
-
-# disable_nftables_init()
-# Disables the boot action for Webmin-managed nftables rules
-sub disable_nftables_init
-{
-foreign_require("init", "init-lib.pl");
-my $action = nftables_boot_action();
+my $action = legacy_nftables_boot_action();
+my $st = init::action_status($action);
+return 0 if (!$st);
 init::disable_at_boot($action);
 {
 	no warnings 'once';
@@ -270,7 +323,47 @@ init::disable_at_boot($action);
 		init::delete_systemd_service(init::action_unit($action));
 		}
 	}
-unlink_file(nftables_boot_wrapper());
+unlink_file("$module_config_directory/apply-boot.pl");
+
+# The rules were loaded at boot before, so keep loading them
+if ($st == 2 && nftables_service_status() == 1) {
+	enable_nftables_at_boot();
+	}
+return 1;
+}
+
+# remove_legacy_managed_metadata()
+# Removes the sidecar that tracked which tables the module considered its
+# own, a distinction that stopped meaning anything once the saved rules
+# became the system's own configuration file
+sub remove_legacy_managed_metadata
+{
+my $file = "$module_config_directory/managed.json";
+return 0 if (!-e $file);
+unlink_file($file);
+return 1;
+}
+
+# migrate_legacy_nftables_config()
+# Moves tables out of the module's own rules file and into the system
+# nftables configuration, so that upgrading does not silently drop rules that
+# used to be applied at boot. Returns the number of tables moved
+sub migrate_legacy_nftables_config
+{
+my ($legacy) = legacy_nftables_rules_files();
+return 0 if (!$legacy);
+my $file = nftables_rules_file();
+return 0 if ($legacy eq $file);
+
+my @old = get_nftables_save($legacy);
+my @new = get_nftables_save($file);
+my %have = map { table_key($_) => 1 } @new;
+my @add = grep { !$have{table_key($_)} } @old;
+if (@add) {
+	write_configuration(@new, @add);
+	}
+rename_file($legacy, $legacy.".migrated");
+return scalar(@add);
 }
 
 # get_nftables_config_files()
@@ -353,6 +446,8 @@ my $set;
 my $set_depth = 0;
 my $set_elem_open = 0;
 my $set_elem_buf = '';
+my $rawblock;
+my $raw_depth = 0;
 my $lnum = 0;
 my $content;
 my $fh;
@@ -376,6 +471,15 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 	$lnum++;
 	$line =~ s/#.*$//;    # Ignore comments for now
 
+	if ($rawblock) {
+		push(@{$rawblock->{'lines'}}, $lines[$i]);
+		my $opens = () = $line =~ /\{/g;
+		my $closes = () = $line =~ /\}/g;
+		$raw_depth += $opens - $closes;
+		$rawblock = undef if ($raw_depth <= 0);
+		next;
+		}
+
 	if ($set) {
 		my $sline = $line;
 		$sline =~ s/^\s+//;
@@ -468,7 +572,8 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 		# Start of a chain
 		if ($table) {
 			$chain = $1;
-			$table->{'chains'}->{$chain} = {};
+			$table->{'chains'}->{$chain} =
+			    {'order' => scalar(keys %{$table->{'chains'}})};
 
 			# Look at next line for chain definition
 			if ($lines[$i + 1] =~
@@ -481,6 +586,20 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 				}
 			}
 		}
+	elsif ($table && $line =~ /^\s*\}\s*$/) {
+		# End of the chain, or of the table itself
+		$chain ? ($chain = undef) : ($table = undef);
+		}
+	elsif ($table && !$chain && $line =~ /\S/) {
+		# Something else in the table that this module does not model,
+		# such as a map, flowtable, named counter or table comment.
+		# Keep it verbatim so that re-writing the file does not drop it
+		$rawblock = {'lines' => [ $lines[$i] ]};
+		push(@{$table->{'raw_blocks'}}, $rawblock);
+		$raw_depth = () = $line =~ /\{/g;
+		$raw_depth -= () = $line =~ /\}/g;
+		$rawblock = undef if ($raw_depth <= 0);
+		}
 	elsif ($line =~ /^\s*(.*?)$/ && $table && $chain && $1 ne "}") {
 		# A rule
 		my $rule_str = $1;
@@ -3321,7 +3440,7 @@ return $table;
 }
 
 # save_profile_ruleset(table-name, profile-id, allowed-service-ids|'*')
-# Saves or replaces a Webmin-managed profile table and returns an error
+# Saves or replaces a profile table and returns an error
 sub save_profile_ruleset
 {
 my ($table_name, $profile_id, $allow_ids) = @_;
@@ -3542,12 +3661,55 @@ foreach my $r (@{$table->{'rules'}}) {
 return;
 }
 
-# nftables_save_header()
-# Returns the generated-file header for saved rules
-sub nftables_save_header
+# get_nftables_extras(file)
+# Returns the content of a ruleset file that is outside any table block, as
+# the text before the first table and the text after the last one. Comments,
+# defines and includes placed there by the system or by hand must survive a
+# re-write of the tables
+sub get_nftables_extras
 {
-return "# This file was auto-generated by the module.\n".
-       "# Manual changes may be overwritten.\n\n";
+my ($file) = @_;
+return ("", "") if (!$file || !-r $file);
+my $data = read_file_contents($file);
+return ("", "") if (!defined($data));
+my (@pre, @cur);
+my ($depth, $seen) = (0, 0);
+foreach my $l (split(/\r?\n/, $data)) {
+	if (!$depth) {
+		if ($l =~ /^\s*table\s+\S+(\s+\S+)?\s*\{/) {
+			# Anything seen since the last table has to stay ahead
+			# of the tables, as defines and includes are only in
+			# scope for what follows them
+			push(@pre, @cur);
+			@cur = ();
+			$seen = 1;
+			$depth = 1;
+			next;
+			}
+		push(@cur, $l);
+		next;
+		}
+	my $opens = () = $l =~ /\{/g;
+	my $closes = () = $l =~ /\}/g;
+	$depth += $opens - $closes;
+	$depth = 0 if ($depth < 0);
+	}
+my @post;
+if ($seen) {
+	@post = @cur;
+	}
+else {
+	push(@pre, @cur);
+	}
+my $pre = join("\n", @pre);
+my $post = join("\n", @post);
+$pre =~ s/\n{3,}/\n\n/g;
+$pre =~ s/\s+$//;
+$post =~ s/^\s+//;
+$post =~ s/\s+$//;
+$pre .= "\n\n" if ($pre ne "");
+$post = "\n".$post."\n" if ($post ne "");
+return ($pre, $post);
 }
 
 # dump_nftables_save(@tables)
@@ -3555,7 +3717,7 @@ return "# This file was auto-generated by the module.\n".
 sub dump_nftables_save
 {
 my (@tables) = @_;
-my $rv = nftables_save_header();
+my $rv = "";
 foreach my $t (@tables) {
 	if ($t->{'family'}) {
 		$rv .= "table $t->{'family'} $t->{'name'} {\n";
@@ -3589,7 +3751,22 @@ foreach my $t (@tables) {
 			}
 		}
 
-	foreach my $c (keys %{$t->{'chains'}}) {
+	if ($t->{'raw_blocks'} && ref($t->{'raw_blocks'}) eq 'ARRAY') {
+		foreach my $b (@{$t->{'raw_blocks'}}) {
+			next if (!$b || ref($b) ne 'HASH');
+			foreach my $l (@{$b->{'lines'}}) {
+				$rv .= $l."\n";
+				}
+			}
+		}
+
+	my $chain_order = sub {
+		my ($n) = @_;
+		my $o = $t->{'chains'}->{$n}->{'order'};
+		return defined($o) ? $o : 0x7fffffff;
+		};
+	foreach my $c (sort { &$chain_order($a) <=> &$chain_order($b) ||
+			      $a cmp $b } keys %{$t->{'chains'}}) {
 		my $chain = $t->{'chains'}->{$c};
 		$rv .= "\tchain $c {\n";
 		if ($chain->{'type'}) {
@@ -3616,13 +3793,13 @@ return $rv;
 sub write_configuration
 {
 my (@tables) = @_;
-my $out = dump_nftables_save(@tables);
 my $file = nftables_rules_file();
+my ($pre, $post) = get_nftables_extras($file);
+my $out = $pre.dump_nftables_save(@tables).$post;
 
 open_lock_tempfile(my $fh, ">$file");
 print_tempfile($fh, $out);
 close_tempfile($fh);
-sync_managed_metadata(@tables);
 update_last_config_change();
 return;
 }
@@ -3677,7 +3854,7 @@ return;
 }
 
 # apply_restore([file])
-# Applies Webmin-managed tables from the save file
+# Applies the saved tables to the live ruleset
 sub apply_restore
 {
 my ($file) = @_;
@@ -3686,7 +3863,7 @@ my $cmd = get_nft_command();
 return text('index_ecommand', "<tt>nft</tt>") if (!$cmd);
 
 my @tables = get_nftables_save($file);
-return text('apply_enone') if (!@tables);
+return text('apply_enone', "<tt>".html_escape($file)."</tt>") if (!@tables);
 
 my ($active, $active_err) = get_active_nftables_save();
 return $active_err if ($active_err);
@@ -3793,9 +3970,9 @@ my %flags =
 return $flags{'owner'} || $flags{'persist'};
 }
 
-# table_is_webmin_managed(&table, [&saved_tables])
-# Returns true if an active table is present in Webmin's saved config
-sub table_is_webmin_managed
+# table_is_saved(&table, [&saved_tables])
+# Returns true if an active table is also in the saved configuration
+sub table_is_saved
 {
 my ($table, $saved_tables) = @_;
 if (!$saved_tables) {
@@ -3809,130 +3986,13 @@ return 0;
 }
 
 # active_table_status(&table, [&saved_tables])
-# Returns webmin, external or unclaimed for an active table
+# Returns saved, external or unsaved for an active table
 sub active_table_status
 {
 my ($table, $saved_tables) = @_;
 return "external" if (table_is_externally_managed($table));
-return "webmin" if (table_is_webmin_managed($table, $saved_tables));
-return "unclaimed";
-}
-
-# managed_metadata_file()
-# Returns the path to Webmin's nftables metadata file
-sub managed_metadata_file
-{
-return "$module_config_directory/managed.json";
-}
-
-# managed_table_key(&table)
-# Returns the key used for managed table metadata
-sub managed_table_key
-{
-my ($table) = @_;
-return nft_table_spec($table);
-}
-
-# read_managed_metadata()
-# Returns metadata about tables managed by this module
-sub read_managed_metadata
-{
-my $file = managed_metadata_file();
-return parse_managed_metadata(undef) if (!-r $file);
-lock_file($file);
-my $json = read_file_contents($file);
-unlock_file($file);
-return parse_managed_metadata($json);
-}
-
-# parse_managed_metadata(json)
-# Parses managed table metadata, returning an empty structure on failure
-sub parse_managed_metadata
-{
-my ($json) = @_;
-my $meta = eval { convert_from_json($json) };
-if (!$meta || ref($meta) ne 'HASH') {
-	$meta = {};
-	}
-if (!$meta->{'tables'} || ref($meta->{'tables'}) ne 'HASH') {
-	$meta->{'tables'} = {};
-	}
-return $meta;
-}
-
-# sync_managed_metadata(@tables)
-# Keeps managed metadata aligned with the saved Webmin config
-sub sync_managed_metadata
-{
-my (@tables) = @_;
-my $file = managed_metadata_file();
-lock_file($file);
-my $meta =
-    -r $file
-    ? parse_managed_metadata(read_file_contents($file))
-    : {'tables' => {}};
-my %old = %{$meta->{'tables'}};
-my %new;
-foreach my $t (@tables) {
-	my $key = managed_table_key($t);
-	my %entry =
-	    $old{$key} && ref($old{$key}) eq 'HASH' ? %{$old{$key}} : ();
-	$entry{'family'} = $t->{'family'};
-	$entry{'name'} = $t->{'name'};
-	$entry{'source'} ||= 'webmin';
-	$entry{'managed_at'} ||= time();
-	$new{$key} = \%entry;
-	}
-$meta->{'tables'} = \%new;
-write_file_contents($file, convert_to_json($meta, 1));
-unlock_file($file);
-return;
-}
-
-# register_managed_table(&table, %info)
-# Adds or updates metadata for a Webmin-managed table
-sub register_managed_table
-{
-my ($table, %info) = @_;
-my $file = managed_metadata_file();
-lock_file($file);
-my $meta =
-    -r $file
-    ? parse_managed_metadata(read_file_contents($file))
-    : {'tables' => {}};
-my $key = managed_table_key($table);
-my %entry = $meta->{'tables'}->{$key} &&
-    ref($meta->{'tables'}->{$key}) eq 'HASH'
-    ? %{$meta->{'tables'}->{$key}}
-    : ();
-foreach my $k (keys %info) {
-	$entry{$k} = $info{$k};
-	}
-$entry{'family'} = $table->{'family'};
-$entry{'name'} = $table->{'name'};
-$entry{'source'} ||= 'webmin';
-$entry{'managed_at'} ||= time();
-$meta->{'tables'}->{$key} = \%entry;
-write_file_contents($file, convert_to_json($meta, 1));
-unlock_file($file);
-return;
-}
-
-# unregister_managed_table(&table)
-# Removes metadata for a table no longer managed by this module
-sub unregister_managed_table
-{
-my ($table) = @_;
-my $file = managed_metadata_file();
-lock_file($file);
-my $meta =
-    -r $file
-    ? parse_managed_metadata(read_file_contents($file))
-    : {'tables' => {}};
-delete($meta->{'tables'}->{managed_table_key($table)});
-write_file_contents($file, convert_to_json($meta, 1));
-unlock_file($file);
-return;
+return "saved" if (table_is_saved($table, $saved_tables));
+return "unsaved";
 }
 
 # describe_rule(&rule)
diff --git a/nftables/postinstall.pl b/nftables/postinstall.pl
new file mode 100644
index 000000000..a649ce958
--- /dev/null
+++ b/nftables/postinstall.pl
@@ -0,0 +1,35 @@
+# postinstall.pl
+use strict;
+use warnings;
+no warnings 'redefine';
+no warnings 'once';
+no warnings 'uninitialized';
+
+do 'nftables-lib.pl';
+
+# module_install()
+# Moves off the module's own rules file and boot action, which earlier
+# releases used in place of the system nftables configuration
+sub module_install
+{
+my ($moved, $removed);
+eval {
+	local $main::error_must_die = 1;
+	$moved = &migrate_legacy_nftables_config();
+	$removed = &remove_legacy_nftables_init();
+	&remove_legacy_managed_metadata();
+	};
+if ($@) {
+	print STDERR "Failed to migrate nftables configuration : $@\n";
+	return;
+	}
+if ($moved) {
+	print STDERR "Moved $moved nftables table(s) into ".
+		     &nftables_rules_file()."\n";
+	}
+if ($removed && !&nftables_started_at_boot()) {
+	print STDERR "The webmin-nftables boot action was removed, but the ".
+		     "nftables service is not enabled at boot. Saved rules ".
+		     "will not be loaded until it is.\n";
+	}
+}
diff --git a/nftables/save_manual.cgi b/nftables/save_manual.cgi
index c63784ce2..85e6e7a3a 100755
--- a/nftables/save_manual.cgi
+++ b/nftables/save_manual.cgi
@@ -23,8 +23,6 @@ open_lock_tempfile(my $fh, ">$file");
 print_tempfile($fh, $in{'data'});
 close_tempfile($fh);
 
-my @tables = get_nftables_save($file);
-sync_managed_metadata(@tables);
 update_last_config_change();
 
 webmin_log("manual", undef, $file);
diff --git a/nftables/setup.cgi b/nftables/setup.cgi
index e7bc13f24..dcab32381 100755
--- a/nftables/setup.cgi
+++ b/nftables/setup.cgi
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 # setup.cgi
-# Create a Webmin-managed nftables profile table
+# Create an nftables profile table in the saved configuration
 
 require './nftables-lib.pl';    ## no critic
 use strict;
diff --git a/nftables/t/run-tests.t b/nftables/t/run-tests.t
index c07ac5cd4..eb796bdcd 100755
--- a/nftables/t/run-tests.t
+++ b/nftables/t/run-tests.t
@@ -231,10 +231,10 @@ ok(table_is_externally_managed($tables_prio[0]),
    'table with owner,persist flags is externally managed');
 is(active_table_status($tables_prio[0], []), 'external',
    'external active table status');
-is(active_table_status({ family => 'inet', name => 'filter' }, [ $t ]), 'webmin',
+is(active_table_status({ family => 'inet', name => 'filter' }, [ $t ]), 'saved',
    'saved active table status');
-is(active_table_status({ family => 'inet', name => 'loose' }, []), 'unclaimed',
-   'unclaimed active table status');
+is(active_table_status({ family => 'inet', name => 'loose' }, []), 'unsaved',
+   'unsaved active table status');
 my $managed_chain = $tables_prio[0]->{chains}->{managed_INPUT};
 ok($managed_chain, 'externally managed priority chain present');
 is($managed_chain->{type}, 'filter', 'externally managed priority chain type');
@@ -493,4 +493,145 @@ ok(scalar(grep { $_ eq '2022' }
           @{$profile_table->{sets}->{profile_hosting_tcp_ports}->{elements}}),
    'profile helper includes dynamic ssh port');
 
+# The saved configuration is now the system's own nftables file, so re-writing
+# it must not discard anything the module does not model
+my $sysfile = write_ruleset($confdir, 'system.nft', <<'EOF');
+#!/usr/sbin/nft -f
+# system firewall
+
+flush ruleset
+
+define lan = 192.168.0.0/24
+
+table inet filter {
+	set trusted {
+		type ipv4_addr
+		flags interval
+		elements = { 10.0.0.0/8 }
+	}
+
+	map porttoip {
+		type inet_service : ipv4_addr
+		elements = { 80 : 10.0.0.1 }
+	}
+
+	counter http_hits {
+	}
+
+	chain input {
+		type filter hook input priority 0; policy drop;
+		tcp dport 22 accept
+	}
+
+	chain output {
+		type filter hook output priority 0; policy accept;
+	}
+}
+
+table ip nat {
+	chain prerouting {
+		type nat hook prerouting priority -100; policy accept;
+	}
+}
+
+include "/etc/nftables.d/*.nft"
+
+define wan = eth0
+
+table inet extra {
+	comment "hand written"
+
+	chain forward {
+		type filter hook forward priority 0; policy drop;
+	}
+}
+
+# trailing note
+include "/etc/nftables.d/late.nft"
+EOF
+
+my @systables = get_nftables_save($sysfile);
+is(scalar(@systables), 3, 'system ruleset table count');
+is(scalar(@{$systables[0]->{raw_blocks} || []}), 2,
+   'unmodelled table objects are captured');
+ok(exists($systables[0]->{chains}->{output}),
+   'chain following an unmodelled object is still parsed');
+is(scalar(@{$systables[0]->{rules}}), 1,
+   'unmodelled objects are not parsed as rules');
+
+my ($pre, $post) = get_nftables_extras($sysfile);
+like($pre, qr/^\#\!\/usr\/sbin\/nft -f/, 'shebang kept ahead of the tables');
+like($pre, qr/flush ruleset/, 'flush ruleset kept ahead of the tables');
+like($pre, qr/define lan = /, 'defines kept ahead of the tables');
+like($pre, qr/include "\/etc\/nftables\.d\/\*\.nft"/,
+     'include between tables kept ahead of them');
+like($post, qr/include "\/etc\/nftables\.d\/late\.nft"/,
+     'trailing include kept after the tables');
+
+my $rewritten = $pre.dump_nftables_save(@systables).$post;
+like($rewritten, qr/map porttoip \{/, 'map survives a re-write');
+like($rewritten, qr/counter http_hits \{/, 'named counter survives a re-write');
+like($rewritten, qr/table ip nat \{/, 'second table survives a re-write');
+like($rewritten, qr/comment "hand written"/, 'table comment survives a re-write');
+is(scalar(() = $rewritten =~ /define wan = eth0/g), 1,
+   'content between tables is kept exactly once');
+like($pre, qr/define wan = eth0/, 'content between tables is kept ahead of them');
+
+is_deeply([ map { $_->[0] }
+            sort { $a->[1] <=> $b->[1] }
+            map { [ $_, $systables[0]->{chains}->{$_}->{order} ] }
+            keys %{$systables[0]->{chains}} ],
+          [ 'input', 'output' ], 'chain order in the file is recorded');
+like($rewritten, qr/chain input \{.*chain output \{/s,
+     'chains are written back in the order they were read');
+
+my $twicefile = write_ruleset($confdir, 'twice.nft', $rewritten);
+my @twice = get_nftables_save($twicefile);
+my ($tpre, $tpost) = get_nftables_extras($twicefile);
+is($tpre.dump_nftables_save(@twice).$tpost, $rewritten,
+   're-writing an already written file changes nothing');
+
+# A file with no tables at all, such as a stock /etc/sysconfig/nftables.conf,
+# keeps its comments and takes new tables at the end
+my $emptyfile = write_ruleset($confdir, 'empty.nft', <<'EOF');
+# Uncomment the include statement here to load the default config sample
+#include "/etc/nftables/main.nft"
+EOF
+my ($epre, $epost) = get_nftables_extras($emptyfile);
+like($epre, qr/Uncomment the include statement/,
+     'comment-only file is kept ahead of new tables');
+is($epost, '', 'comment-only file leaves nothing trailing');
+
+# Upgrades have to move rules out of the module's old private file, or the
+# firewall silently disappears once the private boot action is gone
+our ($module_config_directory, $nftables_rules_file_cache);
+mkdir($module_config_directory) if (!-d $module_config_directory);
+my $legacy = write_ruleset($module_config_directory, 'rules.conf', <<'EOF');
+# This file was auto-generated by the module.
+# Manual changes may be overwritten.
+
+table inet webmin {
+	chain input {
+		type filter hook input priority 0; policy drop;
+		tcp dport 10000 accept
+	}
+}
+EOF
+my $target = write_ruleset($confdir, 'migrate-target.nft', <<'EOF');
+# Uncomment the include statement here to load the default config sample
+#include "/etc/nftables/main.nft"
+EOF
+{
+    local $nftables_rules_file_cache = $target;
+    is(migrate_legacy_nftables_config(), 1, 'legacy table is migrated');
+    ok(!-e $legacy, 'legacy rules file is moved aside');
+    ok(-e $legacy.'.migrated', 'legacy rules file is kept as a backup');
+    my @moved = get_nftables_save($target);
+    is(scalar(@moved), 1, 'migrated table lands in the system file');
+    is($moved[0]->{name}, 'webmin', 'migrated table keeps its name');
+    like(read_file_contents($target), qr/Uncomment the include statement/,
+         'migration keeps the system file comments');
+    is(migrate_legacy_nftables_config(), 0, 'migration only runs once');
+}
+
 done_testing();

From e0b8113348e5256c29b4afddd4dfe57766cd0f0d Mon Sep 17 00:00:00 2001
From: Joe Cooper <swelljoe@rocky-10-dev>
Date: Sat, 5 Sep 2026 18:26:24 -0500
Subject: [PATCH 2/8] Handle included files correctly and labeled as such

---
 nftables/active_table.cgi |   4 +
 nftables/import_table.cgi |   3 +
 nftables/index.cgi        |   9 +++
 nftables/lang/en          |   2 +
 nftables/nftables-lib.pl  | 156 ++++++++++++++++++++++++++++----------
 nftables/save_manual.cgi  |  21 ++++-
 nftables/t/run-tests.t    |  98 +++++++++++++++++++++++-
 7 files changed, 250 insertions(+), 43 deletions(-)

diff --git a/nftables/active_table.cgi b/nftables/active_table.cgi
index d66fcc071..dc7b7f4de 100755
--- a/nftables/active_table.cgi
+++ b/nftables/active_table.cgi
@@ -32,6 +32,10 @@ print ui_table_start($text{'active_table_summary'}, "width=100%", 2);
 print ui_table_row($text{'active_table'}, html_escape(nft_table_spec($table)));
 print ui_table_row($text{'active_flags'}, html_escape($table->{'flags'} || "-"));
 print ui_table_row($text{'active_status'}, $text{'active_'.$status_key});
+my ($saved_copy) = grep { table_key($_) eq table_key($table) } @saved_tables;
+print ui_table_row($text{'active_file'},
+		   "<tt>".html_escape($saved_copy->{'file'})."</tt>")
+    if ($saved_copy && $saved_copy->{'file'});
 print ui_table_end();
 
 if (!$is_saved && check_acl('import')) {
diff --git a/nftables/import_table.cgi b/nftables/import_table.cgi
index 242a9cddc..cbd3ea355 100755
--- a/nftables/import_table.cgi
+++ b/nftables/import_table.cgi
@@ -49,6 +49,9 @@ if ($in{'import'}) {
 	my $import = dclone($source);
 	$import->{'name'} = $name;
 	delete($import->{'flags'});
+
+	# It came from the live ruleset, so it has no file of its own yet
+	delete($import->{'file'});
 	assert_table_acl($import);
 	push(@tables, $import);
 	write_configuration(@tables);
diff --git a/nftables/index.cgi b/nftables/index.cgi
index cee147085..7f7e3be8d 100755
--- a/nftables/index.cgi
+++ b/nftables/index.cgi
@@ -423,6 +423,15 @@ else {
 	# Identify current table
 	my $curr = $tables[$in{'table'}];
 
+	# Say which file holds it, as the ruleset can be spread over the main
+	# configuration file and the files it includes
+	if ($curr && $curr->{'file'} && !$partial) {
+		print ui_tag('div',
+			text('index_table_file',
+			     "<tt>".html_escape($curr->{'file'})."</tt>"),
+			{'class' => 'nftables_table_file'}), "\n";
+		}
+
 	if ($curr) {
 		my ($sets_html, $chains_html);
 
diff --git a/nftables/lang/en b/nftables/lang/en
index da0cd4a08..fae9793d0 100644
--- a/nftables/lang/en
+++ b/nftables/lang/en
@@ -164,6 +164,7 @@ setup_eservice=Invalid service selected: $1
 setup_failed=Failed to create ruleset profile: <pre>$1</pre>
 index_profile_setup=Create Ruleset Profile
 index_profile_setupdesc=Create a managed nftables table from a predefined profile.
+index_table_file=Defined in $1
 index_table_create=Create Table
 index_table_createdesc=Add a new nftables table.
 index_table_delete=Delete Table
@@ -335,6 +336,7 @@ active_chains=Chains
 active_sets=Sets
 active_rules=Rules
 active_status=Status
+active_file=Saved in
 active_saved=Saved in configuration
 active_external=Externally managed
 active_unsaved=Not saved
diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl
index d73650dbb..7edfa4560 100644
--- a/nftables/nftables-lib.pl
+++ b/nftables/nftables-lib.pl
@@ -366,32 +366,73 @@ rename_file($legacy, $legacy.".migrated");
 return scalar(@add);
 }
 
+# nftables_include_files(file)
+# Returns the files that a ruleset file pulls in with include directives, in
+# the order nft reads them. Includes are followed recursively, globs are
+# expanded, and a relative path resolves against the including file's own
+# directory
+sub nftables_include_files
+{
+my ($file, $seen) = @_;
+$seen ||= {};
+return () if (!$file || $file =~ /\|\s*$/);
+return () if ($seen->{simplify_path($file)}++);
+return () if (!-r $file);
+my $data = read_file_contents($file);
+return () if (!defined($data));
+my $dir = $file;
+$dir =~ s/\/[^\/]+$//;
+$dir = "/" if ($dir eq "");
+
+my @rv;
+my $depth = 0;
+foreach my $l (split(/\r?\n/, $data)) {
+	$l =~ s/#.*$//;
+	if ($depth) {
+		my $opens = () = $l =~ /\{/g;
+		my $closes = () = $l =~ /\}/g;
+		$depth += $opens - $closes;
+		$depth = 0 if ($depth < 0);
+		next;
+		}
+	if ($l =~ /^\s*table\s+\S+(\s+\S+)?\s*\{/) {
+		$depth = 1;
+		next;
+		}
+
+	# An include only means anything at the top level
+	next if ($l !~ /^\s*include\s+(\S.*?)\s*;?\s*$/);
+	my $spec = $1;
+	if ($spec =~ /^"([^"]*)"$/ || $spec =~ /^'([^']*)'$/) {
+		$spec = $1;
+		}
+	next if ($spec eq "");
+	$spec = $dir."/".$spec if ($spec !~ /^\//);
+	foreach my $inc (nftables_glob($spec)) {
+		next if (!-f $inc);
+		push(@rv, $inc, nftables_include_files($inc, $seen));
+		}
+	}
+return @rv;
+}
+
+# nftables_glob(pattern)
+# Expands one include pattern, without the word splitting that the built-in
+# glob does on paths containing spaces
+sub nftables_glob
+{
+my ($pattern) = @_;
+require File::Glob;
+return File::Glob::bsd_glob($pattern);
+}
+
 # get_nftables_config_files()
 # Returns files that can be manually edited by this module
 sub get_nftables_config_files
 {
-my @files;
-push(@files, nftables_rules_file());
-
-foreach my $sysfile ("/etc/nftables.conf", "/etc/sysconfig/nftables.conf") {
-	push(@files, $sysfile) if (-f $sysfile);
-	}
-
-if (-d "/etc/nftables") {
-	opendir(my $dir, "/etc/nftables");
-	if ($dir) {
-		foreach my $name (sort readdir($dir)) {
-			next if ($name =~ /^\./);
-			next if ($name !~ /\.(?:nft|conf)$/);
-			my $path = "/etc/nftables/$name";
-			push(@files, $path) if (-f $path);
-			}
-		closedir($dir);
-		}
-	}
-
+my $main = nftables_rules_file();
 my %seen;
-return grep { !$seen{$_}++ } @files;
+return grep { !$seen{$_}++ } ($main, nftables_include_files($main));
 }
 
 # list_foreign_firewall_modules()
@@ -412,19 +453,18 @@ foreach my $mod (@mods) {
 return @rv;
 }
 
-# validate_nftables_text(text)
-# Returns an error if nft rejects the supplied ruleset text
-sub validate_nftables_text
+# validate_nftables_files()
+# Returns an error if nft rejects the saved ruleset as it stands on disk.
+# An included file cannot be checked on its own, as it may well use a define
+# from the file that includes it, so the whole ruleset is checked from the
+# top
+sub validate_nftables_files
 {
-my ($text) = @_;
 my $cmd = get_nft_command();
 return text('index_ecommand', "<tt>nft</tt>") if (!$cmd);
-my $tmp = tempname();
-open_tempfile(my $fh, ">$tmp");
-print_tempfile($fh, $text);
-close_tempfile($fh);
-my $out = backquote_logged("$cmd -c -f $tmp 2>&1");
-unlink_file($tmp);
+my $file = nftables_rules_file();
+return if (!-r $file);
+my $out = backquote_logged("$cmd -c -f ".quotemeta($file)." 2>&1");
 return $? ? "<pre>$out</pre>" : undef;
 }
 
@@ -433,9 +473,21 @@ return $? ? "<pre>$out</pre>" : undef;
 sub get_nftables_save
 {
 my ($file) = @_;
-if (!$file) {
-	$file = nftables_rules_file();
+$file ||= nftables_rules_file();
+return () if (!$file);
+my @rv = parse_nftables_file($file);
+foreach my $inc (nftables_include_files($file)) {
+	push(@rv, parse_nftables_file($inc));
 	}
+return @rv;
+}
+
+# parse_nftables_file(file)
+# Returns the tables defined in one ruleset file, each tagged with the file
+# it came from so that it can be written back to the same place
+sub parse_nftables_file
+{
+my ($file) = @_;
 return () if (!$file);
 return () if ($file !~ /\|\s*$/ && !-r $file);
 
@@ -540,6 +592,7 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 		$table = {
 			'name' => $2,
 			'family' => $1,
+			'file' => $file,
 			'line' => $lnum,
 			'rules' => [ ],
 			'chains' => {},
@@ -3793,13 +3846,38 @@ return $rv;
 sub write_configuration
 {
 my (@tables) = @_;
-my $file = nftables_rules_file();
-my ($pre, $post) = get_nftables_extras($file);
-my $out = $pre.dump_nftables_save(@tables).$post;
+my $main = nftables_rules_file();
+my @known = ($main, nftables_include_files($main));
+my %known = map { $_ => 1 } @known;
 
-open_lock_tempfile(my $fh, ">$file");
-print_tempfile($fh, $out);
-close_tempfile($fh);
+# Each table goes back to the file it was read from, so that a table living
+# in an included file is edited there instead of being copied into the main
+# one. Anything without a home, or pointing somewhere we do not manage, goes
+# to the main file
+my %byfile;
+foreach my $t (@tables) {
+	my $f = $t->{'file'};
+	$f = $main if (!$f || !$known{$f});
+	push(@{$byfile{$f}}, $t);
+	}
+
+# Every file that holds tables has to be re-written even if it ends up with
+# none, or a deleted table would survive in its own file
+foreach my $f (@known) {
+	$byfile{$f} ||= [ ];
+	}
+
+foreach my $f (sort keys %byfile) {
+	# Leave a file alone unless its own tables actually changed. Comparing
+	# through the dumper ignores whatever indenting the file happens to
+	# use, so an untouched file is not re-formatted behind the admin's back
+	my $want = dump_nftables_save(@{$byfile{$f}});
+	next if ($want eq dump_nftables_save(parse_nftables_file($f)));
+	my ($pre, $post) = get_nftables_extras($f);
+	open_lock_tempfile(my $fh, ">$f");
+	print_tempfile($fh, $pre.$want.$post);
+	close_tempfile($fh);
+	}
 update_last_config_change();
 return;
 }
diff --git a/nftables/save_manual.cgi b/nftables/save_manual.cgi
index 85e6e7a3a..151b381f6 100755
--- a/nftables/save_manual.cgi
+++ b/nftables/save_manual.cgi
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 # save_manual.cgi
-# Save the manually edited nftables rules file
+# Save a manually edited nftables configuration file
 
 require './nftables-lib.pl';    ## no critic
 use strict;
@@ -16,13 +16,28 @@ my $file = $in{'file'};
 indexof($file, @files) >= 0 || error($text{'manual_efile'});
 
 $in{'data'} =~ s/\r//g;
-my $err = validate_nftables_text($in{'data'});
-error(text('manual_evalidate', $err)) if ($err);
 
+# Check the saved ruleset as a whole with the new content in place, as an
+# included file on its own may use a define from the file that includes it.
+# Put the old content back if nft rejects the result
+my $old = -r $file ? read_file_contents($file) : undef;
 open_lock_tempfile(my $fh, ">$file");
 print_tempfile($fh, $in{'data'});
 close_tempfile($fh);
 
+my $err = validate_nftables_files();
+if ($err) {
+	if (defined($old)) {
+		open_lock_tempfile(my $rfh, ">$file");
+		print_tempfile($rfh, $old);
+		close_tempfile($rfh);
+		}
+	else {
+		unlink_file($file);
+		}
+	error(text('manual_evalidate', $err));
+	}
+
 update_last_config_change();
 
 webmin_log("manual", undef, $file);
diff --git a/nftables/t/run-tests.t b/nftables/t/run-tests.t
index eb796bdcd..2d017b14d 100755
--- a/nftables/t/run-tests.t
+++ b/nftables/t/run-tests.t
@@ -602,9 +602,105 @@ like($epre, qr/Uncomment the include statement/,
      'comment-only file is kept ahead of new tables');
 is($epost, '', 'comment-only file leaves nothing trailing');
 
+our ($module_config_directory, $nftables_rules_file_cache);
+
+# A ruleset spread over a main file and the files it includes has to be read
+# from, and written back to, the file each table actually lives in
+my $incdir = "$confdir/nftables.d";
+mkdir($incdir);
+my $incmain = write_ruleset($confdir, 'main.nft', <<'EOF');
+#!/usr/sbin/nft -f
+flush ruleset
+
+define lan = 192.168.0.0/24
+
+include "nftables.d/*.nft"
+
+table inet main_table {
+	chain input {
+		type filter hook input priority 0; policy drop;
+	}
+}
+EOF
+write_ruleset($incdir, '10-web.nft', <<'EOF');
+# web rules
+table inet web {
+	chain input {
+		type filter hook input priority 10; policy accept;
+		tcp dport 80 accept
+	}
+}
+EOF
+# Deliberately non-canonical spacing, so that a needless re-write would show
+write_ruleset($incdir, '20-mail.nft', <<'EOF');
+table inet mail {
+      chain input {
+            type filter hook input priority 20; policy accept;
+            tcp dport 25 accept
+      }
+}
+EOF
+
+is_deeply([ nftables_include_files($incmain) ],
+          [ "$incdir/10-web.nft", "$incdir/20-mail.nft" ],
+          'relative include glob is expanded in order');
+
+{
+    local $nftables_rules_file_cache = $incmain;
+    my @inctables = get_nftables_save();
+    is_deeply([ map { $_->{name} } @inctables ],
+              [ 'main_table', 'web', 'mail' ],
+              'tables are read from the main file and its includes');
+    is($inctables[0]->{file}, $incmain, 'main table is tagged with its file');
+    is($inctables[1]->{file}, "$incdir/10-web.nft",
+       'included table is tagged with the file it came from');
+    is_deeply([ get_nftables_config_files() ],
+              [ $incmain, "$incdir/10-web.nft", "$incdir/20-mail.nft" ],
+              'the manual editor offers exactly the files that are loaded');
+
+    # An included table is saved, so it must not be offered for import
+    is(active_table_status({ family => 'inet', name => 'web' }, \@inctables),
+       'saved', 'a table from an included file counts as saved');
+
+    my $main_before = read_file_contents($incmain);
+    my $mail_before = read_file_contents("$incdir/20-mail.nft");
+
+    # Editing a table in an included file writes it back there
+    my ($web) = grep { $_->{name} eq 'web' } @inctables;
+    push(@{$web->{rules}}, {'text' => 'tcp dport 443 accept',
+                            'chain' => 'input', 'index' => 99});
+    write_configuration(@inctables);
+    like(read_file_contents("$incdir/10-web.nft"), qr/tcp dport 443 accept/,
+         'edit lands in the included file');
+    unlike(read_file_contents($incmain), qr/table inet web/,
+           'edit is not copied into the main file');
+    is(read_file_contents($incmain), $main_before,
+       'the main file is left alone');
+    is(read_file_contents("$incdir/20-mail.nft"), $mail_before,
+       'an untouched included file is not re-written');
+
+    # Deleting it empties that file without disturbing the others
+    my @keep = grep { $_->{name} ne 'web' } get_nftables_save();
+    write_configuration(@keep);
+    unlike(read_file_contents("$incdir/10-web.nft"), qr/table\s/,
+           'deleted table is removed from its own file');
+    like(read_file_contents("$incdir/10-web.nft"), qr/# web rules/,
+         'the emptied file keeps its own comments');
+    is(read_file_contents($incmain), $main_before,
+       'deleting from an include leaves the main file alone');
+    is_deeply([ map { $_->{name} } get_nftables_save() ],
+              [ 'main_table', 'mail' ], 'the deleted table is gone');
+}
+
+# An include loop must not send the parser into a spin
+my $loop_a = write_ruleset($confdir, 'loop-a.nft', "include \"loop-b.nft\"\n");
+write_ruleset($confdir, 'loop-b.nft', "include \"loop-a.nft\"\n");
+is_deeply([ nftables_include_files($loop_a) ],
+          [ "$confdir/loop-b.nft", "$confdir/loop-a.nft" ],
+          'an include loop terminates');
+
 # Upgrades have to move rules out of the module's old private file, or the
 # firewall silently disappears once the private boot action is gone
-our ($module_config_directory, $nftables_rules_file_cache);
 mkdir($module_config_directory) if (!-d $module_config_directory);
 my $legacy = write_ruleset($module_config_directory, 'rules.conf', <<'EOF');
 # This file was auto-generated by the module.

From 3e91407f812a23ca5bafcb63d0bf2a2c2b6f2d21 Mon Sep 17 00:00:00 2001
From: Joe Cooper <swelljoe@rocky-10-dev>
Date: Sat, 5 Sep 2026 23:24:33 -0500
Subject: [PATCH 3/8] Fix start/stop buttons

---
 nftables/acl_security.pl |  4 +--
 nftables/defaultacl      |  1 +
 nftables/index.cgi       | 32 ++++++++++++++++++++----
 nftables/lang/en         |  8 ++++++
 nftables/nftables-lib.pl | 53 ++++++++++++++++++++++++++++++++++++++++
 nftables/start.cgi       | 17 +++++++++++++
 nftables/stop.cgi        | 17 +++++++++++++
 7 files changed, 125 insertions(+), 7 deletions(-)
 create mode 100755 nftables/start.cgi
 create mode 100755 nftables/stop.cgi

diff --git a/nftables/acl_security.pl b/nftables/acl_security.pl
index 51b413480..c93ce1394 100644
--- a/nftables/acl_security.pl
+++ b/nftables/acl_security.pl
@@ -36,7 +36,7 @@ print ui_table_row(
 
 foreach my $a (
 	qw(view active create setup chains sets rules raw delete
-	   apply bootup import clear quick quick_ip quick_port
+	   apply bootup service import clear quick quick_ip quick_port
 	   quick_service quick_forward manual)
     )
 {
@@ -61,7 +61,7 @@ else {
 	}
 foreach my $a (
 	qw(view active create setup chains sets rules raw delete
-	   apply bootup import clear quick quick_ip quick_port
+	   apply bootup service import clear quick quick_ip quick_port
 	   quick_service quick_forward manual)
     )
 {
diff --git a/nftables/defaultacl b/nftables/defaultacl
index 721cfe05b..55a7b5416 100644
--- a/nftables/defaultacl
+++ b/nftables/defaultacl
@@ -10,6 +10,7 @@ raw=1
 delete=1
 apply=1
 bootup=1
+service=1
 import=1
 clear=1
 quick=1
diff --git a/nftables/index.cgi b/nftables/index.cgi
index 7f7e3be8d..10e26987a 100755
--- a/nftables/index.cgi
+++ b/nftables/index.cgi
@@ -854,14 +854,11 @@ if ($partial) {
 
 print $rules_html;
 
-my $init_support = foreign_check("init") && check_acl('bootup') &&
-                   nftables_service_status() ? 1 : 0;
 if (
 	@tables &&
 	(check_acl('active') ||
 		check_acl('setup') ||
-		check_manual_acl() ||
-		$init_support)
+		check_manual_acl())
     )
 {
 	print ui_hr();
@@ -881,10 +878,35 @@ if (
 		$text{'index_edit_manual'},
 		$text{'index_edit_manualdesc'}
 	) if (check_manual_acl());
+	print ui_buttons_end();
+	}
+
+# Service controls, shown whether or not any rules are saved yet. Without
+# them there is no way to see that the service is stopped or disabled, which
+# is the normal state on a system that has never had a ruleset
+my $svc = foreign_check("init") ? nftables_service_status() : 0;
+my $can_service = $svc && check_acl('service');
+my $can_bootup = $svc && check_acl('bootup');
+if ($can_service || $can_bootup) {
+	print ui_hr();
+	print ui_buttons_start();
+	if ($can_service) {
+		if (nftables_service_running()) {
+			my $desc = $text{'index_stopdesc'};
+			$desc .= " ".$text{'index_stopflush'}
+			    if (nftables_service_stop_flushes());
+			print ui_buttons_row("stop.cgi", $text{'index_stop'},
+				$desc);
+			}
+		else {
+			print ui_buttons_row("start.cgi", $text{'index_start'},
+				$text{'index_startdesc'});
+			}
+		}
 	print ui_buttons_row("bootup.cgi", $text{'index_bootup'},
 		$text{'index_bootupdesc'},
 		undef, ui_yesno_radio("boot", nftables_started_at_boot()))
-	    if ($init_support);
+	    if ($can_bootup);
 	print ui_buttons_end();
 	}
 
diff --git a/nftables/lang/en b/nftables/lang/en
index fae9793d0..a85020699 100644
--- a/nftables/lang/en
+++ b/nftables/lang/en
@@ -93,6 +93,13 @@ quick_edup=An equivalent quick rule for $1 already exists.
 quick_failed=Failed to save and apply quick rule: $1
 index_unapply=Revert Configuration
 index_unapplydesc=Click this button to reset the configuration listed above to the one that is currently active.
+index_start=Start Firewall
+index_startdesc=Load the saved nftables configuration into the kernel by starting the nftables service.
+index_stop=Stop Firewall
+index_stopdesc=Stop the nftables service.
+index_stopflush=On this system stopping the service flushes the entire kernel ruleset, including tables belonging to other software.
+start_err=Failed to start the nftables service
+stop_err=Failed to stop the nftables service
 index_bootup=Activate at Boot
 index_bootupdesc=Change whether the system nftables service loads the saved rules at boot time or not.
 bootup_einit=This system does not support boot-time service management from Webmin.
@@ -393,6 +400,7 @@ acl_raw=Edit raw rule text
 acl_delete=Delete tables, chains and sets
 acl_apply=Apply saved configuration
 acl_bootup=Enable firewall at boot
+acl_service=Start and stop the firewall service
 acl_import=Import active tables
 acl_clear=Clear active tables
 acl_quick=Use quick controls
diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl
index 7edfa4560..badebe85a 100644
--- a/nftables/nftables-lib.pl
+++ b/nftables/nftables-lib.pl
@@ -287,6 +287,59 @@ init::disable_at_boot(nftables_service_name());
 undef($nftables_service_status_cache);
 }
 
+# nftables_service_running()
+# Returns true if the system nftables service has its ruleset loaded
+sub nftables_service_running
+{
+return 0 if (!nftables_service_status());
+foreign_require("init", "init-lib.pl");
+no warnings 'once';
+if (($init::init_mode || "") eq "systemd") {
+	return init::is_active_systemd(
+	    init::action_unit(nftables_service_name())) ? 1 : 0;
+	}
+my $file = init::action_filename(nftables_service_name());
+return 0 if (!$file || !-x $file);
+return init::action_running($file) == 1 ? 1 : 0;
+}
+
+# nftables_service_stop_flushes()
+# Returns true if stopping the service flushes the whole kernel ruleset. Most
+# distributions do exactly that, which takes out tables belonging to other
+# software as well, so the admin is warned before doing it
+sub nftables_service_stop_flushes
+{
+my $unit_file = nftables_service_unit_file();
+return 0 if (!$unit_file);
+my $data = read_file_contents($unit_file);
+return 0 if (!$data);
+foreach my $l (split(/\r?\n/, $data)) {
+	next if ($l !~ /^\s*ExecStop\s*=/);
+	return 1 if ($l =~ /flush\s+ruleset/);
+	}
+return 0;
+}
+
+# start_nftables_service()
+# Starts the system nftables service, loading the saved ruleset
+sub start_nftables_service
+{
+foreign_require("init", "init-lib.pl");
+my ($ok, $err) = init::start_action(nftables_service_name());
+undef($nftables_service_status_cache);
+return $ok ? undef : $err;
+}
+
+# stop_nftables_service()
+# Stops the system nftables service
+sub stop_nftables_service
+{
+foreign_require("init", "init-lib.pl");
+my ($ok, $err) = init::stop_action(nftables_service_name());
+undef($nftables_service_status_cache);
+return $ok ? undef : $err;
+}
+
 # legacy_nftables_rules_files()
 # Returns the private rules files that releases before the switch to the
 # system nftables configuration wrote to
diff --git a/nftables/start.cgi b/nftables/start.cgi
new file mode 100755
index 000000000..22ce3b0c1
--- /dev/null
+++ b/nftables/start.cgi
@@ -0,0 +1,17 @@
+#!/usr/bin/perl
+# start.cgi
+# Start the system nftables service
+
+require './nftables-lib.pl';    ## no critic
+use strict;
+use warnings;
+our (%text);
+error_setup($text{'start_err'});
+assert_acl('service');
+nftables_service_status() || error($text{'bootup_eservice'});
+
+my $err = start_nftables_service();
+error($err) if ($err);
+
+webmin_log("start");
+redirect("index.cgi");
diff --git a/nftables/stop.cgi b/nftables/stop.cgi
new file mode 100755
index 000000000..49af43379
--- /dev/null
+++ b/nftables/stop.cgi
@@ -0,0 +1,17 @@
+#!/usr/bin/perl
+# stop.cgi
+# Stop the system nftables service
+
+require './nftables-lib.pl';    ## no critic
+use strict;
+use warnings;
+our (%text);
+error_setup($text{'stop_err'});
+assert_acl('service');
+nftables_service_status() || error($text{'bootup_eservice'});
+
+my $err = stop_nftables_service();
+error($err) if ($err);
+
+webmin_log("stop");
+redirect("index.cgi");

From 2aab9aa69fd51a069f5627d831ad3954f578ac17 Mon Sep 17 00:00:00 2001
From: Joe Cooper <swelljoe@rocky-10-dev>
Date: Sun, 6 Sep 2026 00:54:54 -0500
Subject: [PATCH 4/8] Add ACL thing

---
 nftables/negativeacl | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 nftables/negativeacl

diff --git a/nftables/negativeacl b/nftables/negativeacl
new file mode 100644
index 000000000..ead519e8b
--- /dev/null
+++ b/nftables/negativeacl
@@ -0,0 +1 @@
+service=0

From 7f4438702855cb2716e218da8fa8e60fa71ab95f Mon Sep 17 00:00:00 2001
From: Joe Cooper <swelljoe@rocky-10-dev>
Date: Sun, 6 Sep 2026 01:08:47 -0500
Subject: [PATCH 5/8] Cleanup help and comment

---
 nftables/help/intro.html | 2 +-
 nftables/t/run-tests.t   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/nftables/help/intro.html b/nftables/help/intro.html
index 771c00cd7..7b835c55d 100644
--- a/nftables/help/intro.html
+++ b/nftables/help/intro.html
@@ -1,5 +1,5 @@
 <header>Introduction</header>
 <p>nftables stores firewall rules in tables. Each table belongs to a family (such as <tt>inet</tt>, <tt>ip</tt>, or <tt>ip6</tt>) and contains one or more chains. Chains contain rules, and each rule is a sequence of tests (matches) followed by an action like <tt>accept</tt>, <tt>drop</tt>, <tt>jump</tt>, or <tt>log</tt>. Named sets can group addresses or services for reuse in multiple rules.</p>
-<p>Rules are stored in the system's own nftables configuration file &mdash; <tt>/etc/nftables.conf</tt> or <tt>/etc/sysconfig/nftables.conf</tt>, whichever the <tt>nftables</tt> service loads on this system. Anything already in that file, such as comments, <tt>define</tt> variables and <tt>include</tt> lines, is left alone. The <tt>nftables</tt> service loads that file at boot, so rules saved here survive a reboot in the same way as rules written by hand.</p>
+<p>Rules are stored in the system's nftables configuration file &mdash; <tt>/etc/nftables.conf</tt> or <tt>/etc/sysconfig/nftables.conf</tt>, whichever the <tt>nftables</tt> service loads on this system. Anything already in that file, such as comments, <tt>define</tt> variables and <tt>include</tt> lines, is left alone. The <tt>nftables</tt> service loads that file at boot, so rules saved here survive a reboot in the same way as rules written by hand.</p>
 <p>To get started, use the Setup page to create a default ruleset, or create a table and chain manually. Then add rules (and sets) from the table view. When you are ready to activate your changes, click Apply Configuration to load the ruleset into the kernel.</p>
 <footer>nft(8)</footer>
diff --git a/nftables/t/run-tests.t b/nftables/t/run-tests.t
index 2d017b14d..0ee094f9c 100755
--- a/nftables/t/run-tests.t
+++ b/nftables/t/run-tests.t
@@ -493,7 +493,7 @@ ok(scalar(grep { $_ eq '2022' }
           @{$profile_table->{sets}->{profile_hosting_tcp_ports}->{elements}}),
    'profile helper includes dynamic ssh port');
 
-# The saved configuration is now the system's own nftables file, so re-writing
+# The saved configuration is the system's own nftables file, so re-writing
 # it must not discard anything the module does not model
 my $sysfile = write_ruleset($confdir, 'system.nft', <<'EOF');
 #!/usr/sbin/nft -f

From cf9c101e9981292ec8c971a7ca0b81a6902e91a3 Mon Sep 17 00:00:00 2001
From: Joe Cooper <swelljoe@rocky-10-dev>
Date: Sun, 6 Sep 2026 01:45:09 -0500
Subject: [PATCH 6/8] Update note about import

---
 nftables/lang/en | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nftables/lang/en b/nftables/lang/en
index a85020699..3bc611659 100644
--- a/nftables/lang/en
+++ b/nftables/lang/en
@@ -373,7 +373,7 @@ import_eexternal=Cannot import as table $1 because that active table is external
 import_emanaged=Table $1 is already in the saved configuration.
 import_source=Source table
 import_flags=Source flags
-import_external_note=This active table is marked as externally managed; importing creates a separate saved copy and does not change the active source table
+import_external_note=This active table is marked as externally managed; importing creates a separate saved copy and does not change the active source table. Tables that are actively maneged by other services, such as fail2ban, should generally not be imported.
 import_new_name=New table name
 import_ok=Import Copy
 manual_editsel=Edit nftables configuration file

From 3291429123ba22752f1e99aa088301f0f3220b35 Mon Sep 17 00:00:00 2001
From: Ilia Ross <ilia@ross.gdn>
Date: Sat, 12 Sep 2026 22:57:21 +0200
Subject: [PATCH 7/8] Add follow up fixes for nftables config parsing,
 rewriting and apply

---
 nftables/edit_chain.cgi         |   2 +-
 nftables/help/chain_policy.html |   2 +-
 nftables/help/intro.html        |   2 +-
 nftables/lang/en                |   7 +-
 nftables/nftables-lib.pl        | 548 ++++++++++++++++++++++++--------
 nftables/postinstall.pl         |   4 +-
 nftables/save_chain.cgi         |   3 +
 nftables/t/run-tests.t          | 274 ++++++++++++++--
 8 files changed, 675 insertions(+), 167 deletions(-)

diff --git a/nftables/edit_chain.cgi b/nftables/edit_chain.cgi
index 10679991f..c9ead12d6 100755
--- a/nftables/edit_chain.cgi
+++ b/nftables/edit_chain.cgi
@@ -35,7 +35,7 @@ my @hook_opts = (
 );
 my @policy_opts = (
 	["", $text{'chain_policy_none'}],
-	map { [$_, $_] } qw(accept drop reject return queue continue)
+	map { [$_, $_] } qw(accept drop)
 );
 
 print ui_form_start("save_chain.cgi");
diff --git a/nftables/help/chain_policy.html b/nftables/help/chain_policy.html
index c0397985d..cd3a0c735 100644
--- a/nftables/help/chain_policy.html
+++ b/nftables/help/chain_policy.html
@@ -1,3 +1,3 @@
 <header>Policy</header>
-<p>Default action for this base chain, such as <tt>accept</tt>, <tt>drop</tt>, <tt>reject</tt>, <tt>queue</tt>, or <tt>continue</tt>.</p>
+<p>Optional default action for this base chain, such as <tt>accept</tt> or <tt>drop</tt>. If no policy is set, nftables uses <tt>accept</tt>.</p>
 <footer>nft(8)</footer>
diff --git a/nftables/help/intro.html b/nftables/help/intro.html
index 7b835c55d..63d0a2e1f 100644
--- a/nftables/help/intro.html
+++ b/nftables/help/intro.html
@@ -1,5 +1,5 @@
 <header>Introduction</header>
 <p>nftables stores firewall rules in tables. Each table belongs to a family (such as <tt>inet</tt>, <tt>ip</tt>, or <tt>ip6</tt>) and contains one or more chains. Chains contain rules, and each rule is a sequence of tests (matches) followed by an action like <tt>accept</tt>, <tt>drop</tt>, <tt>jump</tt>, or <tt>log</tt>. Named sets can group addresses or services for reuse in multiple rules.</p>
-<p>Rules are stored in the system's nftables configuration file &mdash; <tt>/etc/nftables.conf</tt> or <tt>/etc/sysconfig/nftables.conf</tt>, whichever the <tt>nftables</tt> service loads on this system. Anything already in that file, such as comments, <tt>define</tt> variables and <tt>include</tt> lines, is left alone. The <tt>nftables</tt> service loads that file at boot, so rules saved here survive a reboot in the same way as rules written by hand.</p>
+<p>Rules are stored in the system's nftables configuration file &mdash; <tt>/etc/nftables.conf</tt> or <tt>/etc/sysconfig/nftables.conf</tt>, whichever the <tt>nftables</tt> service loads on this system. Top-level comments, <tt>define</tt> variables and <tt>include</tt> lines remain in their original positions. A table changed through Webmin may be rewritten in a normalized form. The <tt>nftables</tt> service loads that file at boot, so rules saved here survive a reboot in the same way as rules written by hand.</p>
 <p>To get started, use the Setup page to create a default ruleset, or create a table and chain manually. Then add rules (and sets) from the table view. When you are ready to activate your changes, click Apply Configuration to load the ruleset into the kernel.</p>
 <footer>nft(8)</footer>
diff --git a/nftables/lang/en b/nftables/lang/en
index 3bc611659..30d911081 100644
--- a/nftables/lang/en
+++ b/nftables/lang/en
@@ -112,9 +112,11 @@ create=Create
 save=Save
 delete=Delete
 save_err=Failed to save rule
+save_eparse=Cannot safely update $1 because a table in that file has no recognizable closing brace.
 apply_err=Failed to apply configuration
 apply_enone=No nftables tables were found in $1 to apply.
 apply_eexternal=Cannot apply configuration because table $1 is currently marked as externally managed.
+apply_einclude_loop=Cannot apply the configuration because its include files form a loop at $1.
 apply_esettype=Set $1 in table $2 has type $3, but chain $4 uses it for $5. Use ipv4_addr or ipv6_addr sets for address fields, and inet_service sets for port fields.
 setup_header=Ruleset profile
 setup_err=Failed to create ruleset profile
@@ -268,7 +270,8 @@ chain_ename=Chain name is invalid
 chain_edup=A chain with that name already exists
 chain_notable=No such table selected
 chain_nochain=No such chain selected
-chain_ebase=Base chains require type, hook, priority, and policy.
+chain_ebase=Base chains require type, hook, and priority. Policy is optional.
+chain_epolicy=Base chain policy must be accept or drop.
 delete_chain_err=Failed to delete chain
 delete_chain_failed=Failed to delete chain: <pre>$1</pre>
 delete_chain_inuse=Chain $1 is referenced by $2 rule(s) via jump/goto. Remove those rules first.
@@ -373,7 +376,7 @@ import_eexternal=Cannot import as table $1 because that active table is external
 import_emanaged=Table $1 is already in the saved configuration.
 import_source=Source table
 import_flags=Source flags
-import_external_note=This active table is marked as externally managed; importing creates a separate saved copy and does not change the active source table. Tables that are actively maneged by other services, such as fail2ban, should generally not be imported.
+import_external_note=This active table is marked as externally managed; importing creates a separate saved copy and does not change the active source table. Tables that are actively managed by other services, such as fail2ban, should generally not be imported.
 import_new_name=New table name
 import_ok=Import Copy
 manual_editsel=Edit nftables configuration file
diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl
index badebe85a..63549af81 100644
--- a/nftables/nftables-lib.pl
+++ b/nftables/nftables-lib.pl
@@ -8,7 +8,9 @@ use warnings;
 our (%config, %access, %gconfig, $module_config_directory,
      $module_var_directory, $module_root_directory);
 our ($last_config_change_flag, $last_restart_time_flag);
-our ($nftables_rules_file_cache, $nftables_service_status_cache);
+our ($nftables_rules_file_cache, $nftables_service_status_cache,
+     $nftables_include_paths_cache, $nftables_include_cwd_cache,
+     $nftables_include_basedir_cache);
 init_config();
 %access = get_module_acl();
 $last_config_change_flag = $module_var_directory."/config-flag";
@@ -250,6 +252,124 @@ if (!$file) {
 return $nftables_rules_file_cache = $file;
 }
 
+# nftables_include_paths()
+# Returns nft's compiled search path for include directives
+sub nftables_include_paths
+{
+return @{$nftables_include_paths_cache}
+    if (ref($nftables_include_paths_cache) eq 'ARRAY');
+
+my @paths;
+my $cmd = get_nft_command();
+if ($cmd) {
+	my $out = backquote_command(quotemeta($cmd)." --help 2>&1");
+	if ($out =~ /--includepath.*?Default is:\s*([^\s]+)/i) {
+		my $path = $1;
+		$path =~ s/^["']|["']$//g;
+		push(@paths, $path) if ($path =~ /^\//);
+		}
+	}
+push(@paths, "/etc") if (!@paths);
+$nftables_include_paths_cache = \@paths;
+return @paths;
+}
+
+# nftables_include_uses_basedir()
+# Returns true when nft prepends the -f input file's directory to its include
+# path. This behavior was added in nftables 1.1.0
+sub nftables_include_uses_basedir
+{
+return $nftables_include_basedir_cache
+    if (defined($nftables_include_basedir_cache));
+
+my $cmd = get_nft_command();
+return $nftables_include_basedir_cache = 0 if (!$cmd);
+my $out = backquote_command(quotemeta($cmd)." --version 2>&1");
+if ($out =~ /\bv?(\d+)\.(\d+)\.(\d+)\b/) {
+	my ($major, $minor) = ($1, $2);
+	return $nftables_include_basedir_cache =
+	    ($major > 1 || $major == 1 && $minor >= 1) ? 1 : 0;
+	}
+return $nftables_include_basedir_cache = 0;
+}
+
+# nftables_include_cwd()
+# Returns the working directory used for explicitly relative ./ includes.
+# System services start in / unless their unit says otherwise; standard
+# nftables units do not override it
+sub nftables_include_cwd
+{
+return defined($nftables_include_cwd_cache) ? $nftables_include_cwd_cache : "/";
+}
+
+# nftables_code_line(line)
+# Removes an nft comment without treating a # inside a quoted string as one
+sub nftables_code_line
+{
+my ($line) = @_;
+my ($quote, $escaped);
+my $rv = "";
+foreach my $ch (split(//, $line || "")) {
+	if ($quote) {
+		$rv .= $ch;
+		if ($escaped) {
+			$escaped = 0;
+			}
+		elsif ($ch eq "\\") {
+			$escaped = 1;
+			}
+		elsif ($ch eq $quote) {
+			$quote = undef;
+			}
+		}
+	elsif ($ch eq '"') {
+		$quote = $ch;
+		$rv .= $ch;
+		}
+	elsif ($ch eq '#') {
+		last;
+		}
+	else {
+		$rv .= $ch;
+		}
+	}
+return $rv;
+}
+
+# nftables_brace_delta(line)
+# Returns the structural brace change, ignoring comments and quoted strings
+sub nftables_brace_delta
+{
+my ($line) = @_;
+my ($quote, $escaped, $delta) = (undef, 0, 0);
+foreach my $ch (split(//, $line || "")) {
+	if ($quote) {
+		if ($escaped) {
+			$escaped = 0;
+			}
+		elsif ($ch eq "\\") {
+			$escaped = 1;
+			}
+		elsif ($ch eq $quote) {
+			$quote = undef;
+			}
+		}
+	elsif ($ch eq '"') {
+		$quote = $ch;
+		}
+	elsif ($ch eq '#') {
+		last;
+		}
+	elsif ($ch eq '{') {
+		$delta++;
+		}
+	elsif ($ch eq '}') {
+		$delta--;
+		}
+	}
+return $delta;
+}
+
 # nftables_service_status()
 # Returns the init status of the system nftables service
 sub nftables_service_status
@@ -403,68 +523,126 @@ return 1;
 # used to be applied at boot. Returns the number of tables moved
 sub migrate_legacy_nftables_config
 {
-my ($legacy) = legacy_nftables_rules_files();
-return 0 if (!$legacy);
 my $file = nftables_rules_file();
-return 0 if ($legacy eq $file);
+my @legacy = grep { $_ ne $file } legacy_nftables_rules_files();
+return 0 if (!@legacy);
 
-my @old = get_nftables_save($legacy);
 my @new = get_nftables_save($file);
 my %have = map { table_key($_) => 1 } @new;
-my @add = grep { !$have{table_key($_)} } @old;
+
+# Several development releases used different private paths. Merge all of
+# them, taking the newest copy when the same table occurs more than once
+my %order = map { $legacy[$_] => $_ } 0 .. $#legacy;
+@legacy = sort {
+	(stat($b))[9] <=> (stat($a))[9] || $order{$a} <=> $order{$b}
+	} @legacy;
+my @add;
+foreach my $legacy (@legacy) {
+	foreach my $table (get_nftables_save($legacy)) {
+		push(@add, $table) if (!$have{table_key($table)}++);
+		}
+	}
 if (@add) {
 	write_configuration(@new, @add);
 	}
-rename_file($legacy, $legacy.".migrated");
+foreach my $legacy (@legacy) {
+	rename_file($legacy, $legacy.".migrated");
+	}
 return scalar(@add);
 }
 
+# nftables_include_spec(line)
+# Returns the filename or glob from a top-level include directive
+sub nftables_include_spec
+{
+my ($line) = @_;
+return if ($line !~ /^\s*include\s+(\S.*?)\s*;?\s*$/);
+my $spec = $1;
+$spec =~ s/\s*;\s*$//;
+if ($spec =~ /^"([^"]*)"$/ || $spec =~ /^'([^']*)'$/) {
+	$spec = $1;
+	}
+return $spec ne "" ? $spec : undef;
+}
+
+# nftables_resolve_include(pattern, [root-file])
+# Expands an include using nft's path rules. Current nft versions prepend the
+# root input file's directory to the compiled include path, while ./ and ../
+# use nft's working directory
+sub nftables_resolve_include
+{
+my ($pattern, $root) = @_;
+return () if (!defined($pattern) || $pattern eq "");
+my $is_glob = $pattern =~ /[*?\[]/;
+
+my @patterns;
+if ($pattern =~ /^\//) {
+	@patterns = ($pattern);
+	}
+elsif ($pattern =~ /^\.\.?(?:\/|$)/) {
+	@patterns = (nftables_include_cwd()."/".$pattern);
+	}
+else {
+	my @paths = nftables_include_paths();
+	if (nftables_include_uses_basedir() &&
+	    $root && $root !~ /\|\s*$/ && $root =~ /\//) {
+		my $dir = $root;
+		$dir =~ s/\/[^\/]*$//;
+		$dir = "/" if ($dir eq "");
+		unshift(@paths, $dir);
+		}
+	my %seen_path;
+	@patterns = map { $_."/".$pattern }
+		    grep { !$seen_path{simplify_path($_)}++ } @paths;
+
+	# A literal name comes from the first directory that has it, but a
+	# wildcard collects the matches from every directory. nft stacks those
+	# matches and reads the stack from the top, so the files below the last
+	# directory are processed first
+	@patterns = reverse(@patterns) if ($is_glob);
+	}
+
+my (%seen, @rv);
+foreach my $spec (@patterns) {
+	my @matched;
+	foreach my $file (nftables_glob($spec)) {
+		next if (!-f $file);
+		$file = simplify_path($file);
+		push(@matched, $file) if (!$seen{$file}++);
+		}
+	push(@rv, @matched);
+	last if (@matched && !$is_glob);
+	}
+return @rv;
+}
+
 # nftables_include_files(file)
-# Returns the files that a ruleset file pulls in with include directives, in
-# the order nft reads them. Includes are followed recursively, globs are
-# expanded, and a relative path resolves against the including file's own
-# directory
+# Returns recursively included files in the collation order used by nft
 sub nftables_include_files
 {
-my ($file, $seen) = @_;
+my ($file, $seen, $root) = @_;
 $seen ||= {};
+$root ||= $file;
 return () if (!$file || $file =~ /\|\s*$/);
 return () if ($seen->{simplify_path($file)}++);
 return () if (!-r $file);
 my $data = read_file_contents($file);
 return () if (!defined($data));
-my $dir = $file;
-$dir =~ s/\/[^\/]+$//;
-$dir = "/" if ($dir eq "");
 
 my @rv;
 my $depth = 0;
 foreach my $l (split(/\r?\n/, $data)) {
-	$l =~ s/#.*$//;
-	if ($depth) {
-		my $opens = () = $l =~ /\{/g;
-		my $closes = () = $l =~ /\}/g;
-		$depth += $opens - $closes;
-		$depth = 0 if ($depth < 0);
-		next;
-		}
-	if ($l =~ /^\s*table\s+\S+(\s+\S+)?\s*\{/) {
-		$depth = 1;
-		next;
-		}
-
-	# An include only means anything at the top level
-	next if ($l !~ /^\s*include\s+(\S.*?)\s*;?\s*$/);
-	my $spec = $1;
-	if ($spec =~ /^"([^"]*)"$/ || $spec =~ /^'([^']*)'$/) {
-		$spec = $1;
-		}
-	next if ($spec eq "");
-	$spec = $dir."/".$spec if ($spec !~ /^\//);
-	foreach my $inc (nftables_glob($spec)) {
-		next if (!-f $inc);
-		push(@rv, $inc, nftables_include_files($inc, $seen));
+	my $code = nftables_code_line($l);
+	if (!$depth) {
+		my $spec = nftables_include_spec($code);
+		foreach my $inc (nftables_resolve_include($spec, $root)) {
+			next if ($seen->{simplify_path($inc)});
+			push(@rv, $inc,
+			     nftables_include_files($inc, $seen, $root));
+			}
 		}
+	$depth += nftables_brace_delta($l);
+	$depth = 0 if ($depth < 0);
 	}
 return @rv;
 }
@@ -517,7 +695,8 @@ my $cmd = get_nft_command();
 return text('index_ecommand', "<tt>nft</tt>") if (!$cmd);
 my $file = nftables_rules_file();
 return if (!-r $file);
-my $out = backquote_logged("$cmd -c -f ".quotemeta($file)." 2>&1");
+my $cwd = quotemeta(nftables_include_cwd());
+my $out = backquote_logged("cd $cwd && $cmd -c -f ".quotemeta($file)." 2>&1");
 return $? ? "<pre>$out</pre>" : undef;
 }
 
@@ -573,14 +752,12 @@ unlock_file($file) if (!$is_pipe);
 my @lines = split /\r?\n/, $content;
 for (my $i = 0 ; $i < @lines ; $i++) {
 	my $line = $lines[$i];
-	$lnum++;
-	$line =~ s/#.*$//;    # Ignore comments for now
+	$lnum = $i + 1;
+	$line = nftables_code_line($line);    # Ignore actual comments for now
 
 	if ($rawblock) {
 		push(@{$rawblock->{'lines'}}, $lines[$i]);
-		my $opens = () = $line =~ /\{/g;
-		my $closes = () = $line =~ /\}/g;
-		$raw_depth += $opens - $closes;
+		$raw_depth += nftables_brace_delta($line);
 		$rawblock = undef if ($raw_depth <= 0);
 		next;
 		}
@@ -628,9 +805,7 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 				}
 			}
 
-		my $opens = () = $line =~ /\{/g;
-		my $closes = () = $line =~ /\}/g;
-		$set_depth += $opens - $closes;
+		$set_depth += nftables_brace_delta($line);
 		if ($set_depth <= 0) {
 			$set = undef;
 			$set_depth = 0;
@@ -640,7 +815,7 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 		next;
 		}
 
-	if ($line =~ /^table\s+(\S+)\s+(\S+)\s+\{/) {
+	if ($line =~ /^\s*table\s+(\S+)\s+(\S+)\s+\{/) {
 		# Start of a table
 		$table = {
 			'name' => $2,
@@ -653,6 +828,21 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 		};
 		push(@rv, $table);
 		$chain = undef;
+
+		# A balanced table contained on one physical line is already
+		# complete. Without this, following top-level text is mistaken for
+		# part of the table and the file cannot be saved
+		if (nftables_brace_delta($line) == 0 &&
+		    $line =~ /\}\s*;?\s*$/) {
+			my $inner = $line;
+			$inner =~ s/^[^\{]*\{//;
+			$inner =~ s/\}\s*;?\s*$//;
+			if ($inner =~ /\S/) {
+				$table->{'raw_blocks'} = [ {'lines' => [ $inner ]} ];
+				}
+			$table->{'end_line'} = $lnum;
+			$table = undef;
+			}
 		}
 	elsif ($line =~ /^\s*flags\s+(.+?)\s*;?$/ && $table && !$chain) {
 		$table->{'flags'} = $1;
@@ -668,8 +858,7 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 				'raw_lines' => [ ],
 			};
 			$table->{'sets'}->{$setname} = $set;
-			$set_depth = () = $line =~ /\{/g;
-			$set_depth -= () = $line =~ /\}/g;
+			$set_depth = nftables_brace_delta($line);
 			$set_elem_open = 0;
 			$set_elem_buf = '';
 			}
@@ -681,20 +870,30 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 			$table->{'chains'}->{$chain} =
 			    {'order' => scalar(keys %{$table->{'chains'}})};
 
-			# Look at next line for chain definition
-			if ($lines[$i + 1] =~
-			    /^\s*type\s+(\S+)\s+hook\s+(\S+)\s+priority\s+(.+?);\s+policy\s+(\S+);/) {
+			# Look at the next line for a base-chain definition. Policy is
+			# optional in nft and defaults to accept when it is absent
+			my $base = defined($lines[$i + 1])
+			    ? nftables_code_line($lines[$i + 1]) : "";
+			if ($base =~
+			    /^\s*type\s+(\S+)\s+hook\s+(\S+)\s+priority\s+(.+?)\s*;\s*(?:policy\s+(\S+)\s*;\s*)?$/) {
 				$table->{'chains'}->{$chain}->{'type'} = $1;
 				$table->{'chains'}->{$chain}->{'hook'} = $2;
 				$table->{'chains'}->{$chain}->{'priority'} = $3;
-				$table->{'chains'}->{$chain}->{'policy'} = $4;
+				$table->{'chains'}->{$chain}->{'policy'} = $4
+				    if (defined($4));
 				$i++;    # Skip next line
 				}
 			}
 		}
 	elsif ($table && $line =~ /^\s*\}\s*$/) {
 		# End of the chain, or of the table itself
-		$chain ? ($chain = undef) : ($table = undef);
+		if ($chain) {
+			$chain = undef;
+			}
+		else {
+			$table->{'end_line'} = $lnum;
+			$table = undef;
+			}
 		}
 	elsif ($table && !$chain && $line =~ /\S/) {
 		# Something else in the table that this module does not model,
@@ -702,8 +901,7 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 		# Keep it verbatim so that re-writing the file does not drop it
 		$rawblock = {'lines' => [ $lines[$i] ]};
 		push(@{$table->{'raw_blocks'}}, $rawblock);
-		$raw_depth = () = $line =~ /\{/g;
-		$raw_depth -= () = $line =~ /\}/g;
+		$raw_depth = nftables_brace_delta($line);
 		$rawblock = undef if ($raw_depth <= 0);
 		}
 	elsif ($line =~ /^\s*(.*?)$/ && $table && $chain && $1 ne "}") {
@@ -851,14 +1049,15 @@ return "ip";
 }
 
 # validate_chain_base(type, hook, priority, policy)
-# Returns true if a chain has a complete or empty base-chain definition
+# Returns true if a chain has a complete or empty base-chain definition.
+# A base chain's policy is optional and defaults to accept
 sub validate_chain_base
 {
 my ($type, $hook, $priority, $policy) = @_;
 if (defined($type) || defined($hook) ||
     defined($priority) || defined($policy)) {
 	return 0 if (!defined($type) || !defined($hook) ||
-		     !defined($priority) || !defined($policy));
+		     !defined($priority));
 	}
 return 1;
 }
@@ -3767,57 +3966,6 @@ foreach my $r (@{$table->{'rules'}}) {
 return;
 }
 
-# get_nftables_extras(file)
-# Returns the content of a ruleset file that is outside any table block, as
-# the text before the first table and the text after the last one. Comments,
-# defines and includes placed there by the system or by hand must survive a
-# re-write of the tables
-sub get_nftables_extras
-{
-my ($file) = @_;
-return ("", "") if (!$file || !-r $file);
-my $data = read_file_contents($file);
-return ("", "") if (!defined($data));
-my (@pre, @cur);
-my ($depth, $seen) = (0, 0);
-foreach my $l (split(/\r?\n/, $data)) {
-	if (!$depth) {
-		if ($l =~ /^\s*table\s+\S+(\s+\S+)?\s*\{/) {
-			# Anything seen since the last table has to stay ahead
-			# of the tables, as defines and includes are only in
-			# scope for what follows them
-			push(@pre, @cur);
-			@cur = ();
-			$seen = 1;
-			$depth = 1;
-			next;
-			}
-		push(@cur, $l);
-		next;
-		}
-	my $opens = () = $l =~ /\{/g;
-	my $closes = () = $l =~ /\}/g;
-	$depth += $opens - $closes;
-	$depth = 0 if ($depth < 0);
-	}
-my @post;
-if ($seen) {
-	@post = @cur;
-	}
-else {
-	push(@pre, @cur);
-	}
-my $pre = join("\n", @pre);
-my $post = join("\n", @post);
-$pre =~ s/\n{3,}/\n\n/g;
-$pre =~ s/\s+$//;
-$post =~ s/^\s+//;
-$post =~ s/\s+$//;
-$pre .= "\n\n" if ($pre ne "");
-$post = "\n".$post."\n" if ($post ne "");
-return ($pre, $post);
-}
-
 # dump_nftables_save(@tables)
 # Returns a string representation of the firewall rules
 sub dump_nftables_save
@@ -3831,6 +3979,7 @@ foreach my $t (@tables) {
 	else {
 		$rv .= "table $t->{'name'} {\n";
 		}
+	$rv .= "\tflags $t->{'flags'}\n" if ($t->{'flags'});
 
 	if ($t->{'sets'} && ref($t->{'sets'}) eq 'HASH') {
 		foreach my $s (sort keys %{$t->{'sets'}}) {
@@ -3876,8 +4025,12 @@ foreach my $t (@tables) {
 		my $chain = $t->{'chains'}->{$c};
 		$rv .= "\tchain $c {\n";
 		if ($chain->{'type'}) {
-			$rv .=
-			    "\t\ttype $chain->{'type'} hook $chain->{'hook'} priority $chain->{'priority'}; policy $chain->{'policy'};\n";
+			$rv .= "\t\ttype $chain->{'type'} hook " .
+			       "$chain->{'hook'} priority $chain->{'priority'};";
+			$rv .= " policy $chain->{'policy'};"
+			    if (defined($chain->{'policy'}) &&
+				$chain->{'policy'} ne "");
+			$rv .= "\n";
 			}
 
 		# Add rules for this chain
@@ -3894,6 +4047,143 @@ foreach my $t (@tables) {
 return $rv;
 }
 
+# rewrite_nftables_file(file, &tables)
+# Replaces only changed table spans. Text outside tables and byte-for-byte
+# copies of untouched tables stay in their original positions
+sub rewrite_nftables_file
+{
+my ($file, $tables) = @_;
+my @current = parse_nftables_file($file);
+my (%wanted, @order);
+foreach my $table (@$tables) {
+	my $key = table_key($table);
+	push(@order, $key) if (!exists($wanted{$key}));
+	$wanted{$key} = $table;
+	}
+
+my $data = -r $file ? read_file_contents($file) : "";
+$data = "" if (!defined($data));
+my @lines = split(/(?<=\n)/, $data, -1);
+pop(@lines) if (@lines && $lines[-1] eq "");
+foreach my $table (@current) {
+	if (!$table->{'line'} || !$table->{'end_line'} ||
+	    $table->{'end_line'} < $table->{'line'}) {
+		error(text('save_eparse', html_escape($file)));
+		}
+	}
+my %used;
+my $out = "";
+my $next_line = 1;
+
+# Copy the gaps verbatim, and replace, retain or remove each existing table
+# according to the desired table list
+foreach my $old (sort { $a->{'line'} <=> $b->{'line'} } @current) {
+	my ($start, $end) = ($old->{'line'}, $old->{'end_line'});
+	next if (!$start || !$end || $end < $start);
+	if ($start > $next_line) {
+		$out .= join("", @lines[$next_line - 1 .. $start - 2]);
+		}
+	my $key = table_key($old);
+	if (exists($wanted{$key})) {
+		my $new = $wanted{$key};
+		if (dump_nftables_save($old) eq dump_nftables_save($new)) {
+			$out .= join("", @lines[$start - 1 .. $end - 1]);
+			}
+		else {
+			$out .= dump_nftables_save($new);
+			}
+		$used{$key} = 1;
+		}
+	$next_line = $end + 1;
+	}
+
+# New tables go after the last existing table but before trailing directives.
+# If the file had no tables, append them after its header and comments
+my $added = join("", map { dump_nftables_save($wanted{$_}) }
+			  grep { !$used{$_} } @order);
+if (@current) {
+	$out .= "\n" if ($added ne "" && $out ne "" && $out !~ /\n\z/);
+	$out .= $added;
+	$out .= join("", @lines[$next_line - 1 .. $#lines])
+	    if ($next_line - 1 <= $#lines);
+	}
+else {
+	$out = $data;
+	if ($added ne "") {
+		$out .= "\n" if ($out ne "" && $out !~ /\n\z/);
+		$out .= "\n" if ($out ne "" && $out !~ /\n\n\z/);
+		$out .= $added;
+		}
+	}
+return 0 if ($out eq $data);
+
+open_lock_tempfile(my $fh, ">$file");
+print_tempfile($fh, $out);
+close_tempfile($fh);
+return 1;
+}
+
+# nftables_apply_text(file, [&stack], [root-file])
+# Expands includes into an apply-time copy of the saved ruleset. The usual
+# top-level flush is omitted so applying Webmin-visible tables cannot remove
+# active tables owned by fail2ban, firewalld or another service
+sub nftables_apply_text
+{
+my ($file, $stack, $root) = @_;
+$stack ||= {};
+return "" if (!$file || $file =~ /\|\s*$/ || !-r $file);
+$file = simplify_path($file);
+if ($stack->{$file}) {
+	error(text('apply_einclude_loop', html_escape($file)));
+	}
+$root ||= $file;
+$stack->{$file} = 1;
+my $data = read_file_contents($file);
+if (!defined($data)) {
+	delete($stack->{$file});
+	return "";
+	}
+
+my @lines = split(/(?<=\n)/, $data, -1);
+pop(@lines) if (@lines && $lines[-1] eq "");
+my ($depth, $rv) = (0, "");
+foreach my $line (@lines) {
+	my $code = nftables_code_line($line);
+	while (!$depth &&
+	       $code =~ /^\s*flush\s+ruleset(?:\s+(?:ip|ip6|inet|arp|bridge|netdev))?(?:\s*;\s*|\s*$)/) {
+		my $rest = substr($line, $+[0]);
+		if ($rest !~ /\S/) {
+			$line = "";
+			last;
+			}
+		$line = $rest;
+		$code = nftables_code_line($line);
+		}
+	next if ($line eq "");
+	if (!$depth) {
+		my $spec = nftables_include_spec($code);
+		if (defined($spec)) {
+			my @includes = nftables_resolve_include($spec, $root);
+			if (@includes || $spec =~ /[*?\[]/) {
+				foreach my $inc (@includes) {
+					my $included =
+					    nftables_apply_text($inc, $stack, $root);
+					$rv .= $included;
+					$rv .= "\n"
+					    if ($included ne "" && $included !~ /\n\z/);
+					}
+				next;
+				}
+			}
+		}
+	$rv .= $line;
+	$depth += nftables_brace_delta($line);
+	$depth = 0 if ($depth < 0);
+	}
+delete($stack->{$file});
+return $rv;
+}
+
 # write_configuration(@tables)
 # Writes the configuration to the save file
 sub write_configuration
@@ -3920,18 +4210,14 @@ foreach my $f (@known) {
 	$byfile{$f} ||= [ ];
 	}
 
-foreach my $f (sort keys %byfile) {
-	# Leave a file alone unless its own tables actually changed. Comparing
-	# through the dumper ignores whatever indenting the file happens to
-	# use, so an untouched file is not re-formatted behind the admin's back
-	my $want = dump_nftables_save(@{$byfile{$f}});
-	next if ($want eq dump_nftables_save(parse_nftables_file($f)));
-	my ($pre, $post) = get_nftables_extras($f);
-	open_lock_tempfile(my $fh, ">$f");
-	print_tempfile($fh, $pre.$want.$post);
-	close_tempfile($fh);
+my $changed;
+my %done;
+foreach my $f (@known, sort keys %byfile) {
+	next if ($done{$f}++);
+	my $file_changed = rewrite_nftables_file($f, $byfile{$f} || [ ]);
+	$changed ||= $file_changed;
 	}
-update_last_config_change();
+update_last_config_change() if ($changed);
 return;
 }
 
@@ -4018,12 +4304,20 @@ foreach my $t (@tables) {
 	print_tempfile($fh, "delete table ".nft_table_spec($t)."\n")
 	    if ($active{table_key($t)});
 	}
-print_tempfile($fh, dump_nftables_save(@tables));
+
+# Use the actual saved text so top-level variables and hand-written syntax
+# remain available. Includes are expanded before loading the temporary file,
+# while flush ruleset is intentionally left out to protect unrelated tables
+my $rules = $file =~ /\|\s*$/
+    ? dump_nftables_save(@tables)
+    : nftables_apply_text($file);
+print_tempfile($fh, $rules);
 close_tempfile($fh);
 
-my $out = backquote_logged("$cmd -c -f $tmp 2>&1");
+my $cwd = quotemeta(nftables_include_cwd());
+my $out = backquote_logged("cd $cwd && $cmd -c -f $tmp 2>&1");
 if (!$?) {
-	$out = backquote_logged("$cmd -f $tmp 2>&1");
+	$out = backquote_logged("cd $cwd && $cmd -f $tmp 2>&1");
 	}
 unlink_file($tmp);
 if ($?) {
diff --git a/nftables/postinstall.pl b/nftables/postinstall.pl
index a649ce958..a0c5365c6 100644
--- a/nftables/postinstall.pl
+++ b/nftables/postinstall.pl
@@ -5,7 +5,7 @@ no warnings 'redefine';
 no warnings 'once';
 no warnings 'uninitialized';
 
-do 'nftables-lib.pl';
+require 'nftables-lib.pl';    ## no critic
 
 # module_install()
 # Moves off the module's own rules file and boot action, which earlier
@@ -20,7 +20,7 @@ eval {
 	&remove_legacy_managed_metadata();
 	};
 if ($@) {
-	print STDERR "Failed to migrate nftables configuration : $@\n";
+	print STDERR "Failed to migrate nftables configuration: $@\n";
 	return;
 	}
 if ($moved) {
diff --git a/nftables/save_chain.cgi b/nftables/save_chain.cgi
index 986631075..dcd1313bf 100755
--- a/nftables/save_chain.cgi
+++ b/nftables/save_chain.cgi
@@ -92,6 +92,9 @@ $hook = undef if (!defined($hook) || $hook eq '');
 $priority = undef if (!defined($priority) || $priority eq '');
 $policy = undef if (!defined($policy) || $policy eq '');
 
+defined($policy) && $policy !~ /^(accept|drop)$/ &&
+    error($text{'chain_epolicy'});
+
 validate_chain_base($type, $hook, $priority, $policy) ||
     error($text{'chain_ebase'});
 
diff --git a/nftables/t/run-tests.t b/nftables/t/run-tests.t
index 0ee094f9c..45cd8417c 100755
--- a/nftables/t/run-tests.t
+++ b/nftables/t/run-tests.t
@@ -40,7 +40,7 @@ $ENV{'FOREIGN_ROOT_DIRECTORY'} = $rootdir;
 chdir("$bindir/..") or die "chdir: $!";
 
 require "$bindir/../nftables-lib.pl";
-our %access;
+our (%access, %config);
 
 {
     local %access = (quick => 1);
@@ -214,6 +214,29 @@ is($chain->{hook}, 'input', 'chain hook');
 is($chain->{priority}, '0', 'chain priority');
 is($chain->{policy}, 'drop', 'chain policy');
 
+# Policy is optional for nft base chains. This is the form used by the stock
+# nftables configuration on current Debian and Ubuntu systems
+my $policyless_file = write_ruleset($confdir, "policyless.nft", <<'EOF');
+table inet policyless {
+    chain input {
+        type filter hook input priority filter;
+    }
+}
+EOF
+my ($policyless) = get_nftables_save($policyless_file);
+my $policyless_chain = $policyless->{chains}->{input};
+is($policyless_chain->{type}, 'filter', 'policy-less base chain type');
+is($policyless_chain->{hook}, 'input', 'policy-less base chain hook');
+is($policyless_chain->{priority}, 'filter',
+   'policy-less base chain priority');
+ok(!defined($policyless_chain->{policy}),
+   'policy-less base chain keeps its implicit policy');
+is(scalar(@{$policyless->{rules}}), 0,
+   'policy-less base-chain definition is not parsed as a rule');
+like(dump_nftables_save($policyless),
+     qr/type filter hook input priority filter;\n/,
+     'policy-less base chain is serialized without an invalid policy');
+
 my $ruleset_prio = write_ruleset($confdir, "externally-managed-priority.nft", <<'EOF');
 table inet externally_managed {
     flags owner,persist
@@ -295,6 +318,8 @@ like($rset_out, qr/\@web_ports/, 'set rule format preserves port set');
 
 ok(validate_chain_base('filter', 'input', '0', 'accept'),
    'chain base allows zero priority');
+ok(validate_chain_base('filter', 'input', '0', undef),
+   'chain base allows an implicit accept policy');
 ok(!validate_chain_base('filter', 'input', undef, 'accept'),
    'chain base missing priority invalid');
 ok(validate_chain_base(undef, undef, undef, undef),
@@ -559,23 +584,34 @@ ok(exists($systables[0]->{chains}->{output}),
 is(scalar(@{$systables[0]->{rules}}), 1,
    'unmodelled objects are not parsed as rules');
 
-my ($pre, $post) = get_nftables_extras($sysfile);
-like($pre, qr/^\#\!\/usr\/sbin\/nft -f/, 'shebang kept ahead of the tables');
-like($pre, qr/flush ruleset/, 'flush ruleset kept ahead of the tables');
-like($pre, qr/define lan = /, 'defines kept ahead of the tables');
-like($pre, qr/include "\/etc\/nftables\.d\/\*\.nft"/,
-     'include between tables kept ahead of them');
-like($post, qr/include "\/etc\/nftables\.d\/late\.nft"/,
-     'trailing include kept after the tables');
+my $sys_before = read_file_contents($sysfile);
+ok(!rewrite_nftables_file($sysfile, \@systables),
+   'an unchanged system file is not rewritten');
+is(read_file_contents($sysfile), $sys_before,
+   'an unchanged system file stays byte-for-byte identical');
 
-my $rewritten = $pre.dump_nftables_save(@systables).$post;
+# Change only the middle table. The writer must not move any top-level text,
+# because defines and redefines are scoped by where they appear in the file
+push(@{$systables[1]->{rules}}, {
+    text => 'tcp dport 8080 accept', chain => 'prerouting', index => 0 });
+ok(rewrite_nftables_file($sysfile, \@systables),
+   'a changed table rewrites its source file');
+my $rewritten = read_file_contents($sysfile);
+like($rewritten, qr/^\#\!\/usr\/sbin\/nft -f/,
+     'shebang stays ahead of the tables');
+like($rewritten, qr/flush ruleset/, 'flush ruleset is retained on disk');
+like($rewritten, qr/define lan = /, 'leading define is retained');
+like($rewritten,
+     qr/table ip nat \{.*include "\/etc\/nftables\.d\/\*\.nft".*define wan = eth0.*table inet extra \{/s,
+     'content between tables remains in its original position');
+like($rewritten, qr/include "\/etc\/nftables\.d\/late\.nft"\s*\z/,
+     'trailing include remains after the tables');
 like($rewritten, qr/map porttoip \{/, 'map survives a re-write');
 like($rewritten, qr/counter http_hits \{/, 'named counter survives a re-write');
 like($rewritten, qr/table ip nat \{/, 'second table survives a re-write');
 like($rewritten, qr/comment "hand written"/, 'table comment survives a re-write');
 is(scalar(() = $rewritten =~ /define wan = eth0/g), 1,
    'content between tables is kept exactly once');
-like($pre, qr/define wan = eth0/, 'content between tables is kept ahead of them');
 
 is_deeply([ map { $_->[0] }
             sort { $a->[1] <=> $b->[1] }
@@ -587,9 +623,67 @@ like($rewritten, qr/chain input \{.*chain output \{/s,
 
 my $twicefile = write_ruleset($confdir, 'twice.nft', $rewritten);
 my @twice = get_nftables_save($twicefile);
-my ($tpre, $tpost) = get_nftables_extras($twicefile);
-is($tpre.dump_nftables_save(@twice).$tpost, $rewritten,
+ok(!rewrite_nftables_file($twicefile, \@twice),
    're-writing an already written file changes nothing');
+is(read_file_contents($twicefile), $rewritten,
+   'the second rewrite remains byte-for-byte identical');
+
+like(dump_nftables_save($tables_prio[0]), qr/^\s*flags owner,persist$/m,
+     'table ownership flags survive serialization');
+
+# Leading whitespace is valid before a table declaration, and a # inside an
+# nft quoted string is data rather than the start of a source comment
+my $quotedfile = write_ruleset($confdir, 'quoted.nft', <<'EOF');
+  table inet quoted {
+	chain input {
+		type filter hook input priority 0; policy accept;
+		tcp dport 22 accept comment "ticket #123"
+	}
+}
+EOF
+my @quoted = get_nftables_save($quotedfile);
+is(scalar(@quoted), 1, 'indented table declaration is parsed');
+is($quoted[0]->{rules}->[0]->{comment}, 'ticket #123',
+   'hash inside a quoted comment is preserved');
+
+my $brokenfile = write_ruleset($confdir, 'broken.nft', <<'EOF');
+table inet unfinished {
+	chain input {
+	}
+EOF
+my $broken_before = read_file_contents($brokenfile);
+eval {
+    no warnings 'once';
+    local $main::error_must_die = 1;
+    my @broken = get_nftables_save($brokenfile);
+    rewrite_nftables_file($brokenfile, \@broken);
+};
+like($@, qr/no recognizable closing brace/,
+     'an unterminated table is rejected before writing');
+is(read_file_contents($brokenfile), $broken_before,
+   'a rejected malformed file remains unchanged');
+
+# A table may open and close on one line. It must not capture the following
+# top-level text or prevent the rest of the file from being saved
+my $onelinefile = write_ruleset($confdir, 'one-line.nft', <<'EOF');
+table inet first { chain hidden { counter } }
+define next_port = 10000
+table inet second {
+	chain input {
+	}
+}
+EOF
+my @oneline = get_nftables_save($onelinefile);
+is_deeply([ map { $_->{name} } @oneline ], [ 'first', 'second' ],
+          'one-line table does not swallow the following table');
+is($oneline[0]->{end_line}, 1, 'one-line table span ends on its opening line');
+like(dump_nftables_save($oneline[0]), qr/chain hidden \{ counter \}/,
+     'unmodelled content inside a one-line table is retained');
+my $oneline_before = read_file_contents($onelinefile);
+ok(!rewrite_nftables_file($onelinefile, \@oneline),
+   'an unchanged file containing a one-line table can be saved');
+is(read_file_contents($onelinefile), $oneline_before,
+   'one-line table and following top-level text stay byte-identical');
 
 # A file with no tables at all, such as a stock /etc/sysconfig/nftables.conf,
 # keeps its comments and takes new tables at the end
@@ -597,12 +691,16 @@ my $emptyfile = write_ruleset($confdir, 'empty.nft', <<'EOF');
 # Uncomment the include statement here to load the default config sample
 #include "/etc/nftables/main.nft"
 EOF
-my ($epre, $epost) = get_nftables_extras($emptyfile);
-like($epre, qr/Uncomment the include statement/,
-     'comment-only file is kept ahead of new tables');
-is($epost, '', 'comment-only file leaves nothing trailing');
+my $empty_table = create_profile_ruleset('new_table', 'allow_all', '*');
+ok(rewrite_nftables_file($emptyfile, [ $empty_table ]),
+   'a table can be added to a comment-only file');
+like(read_file_contents($emptyfile),
+     qr/Uncomment the include statement.*table inet new_table \{/s,
+     'comment-only header stays ahead of the new table');
 
-our ($module_config_directory, $nftables_rules_file_cache);
+our ($module_config_directory, $nftables_rules_file_cache,
+     $nftables_include_paths_cache, $nftables_include_cwd_cache,
+     $nftables_include_basedir_cache);
 
 # A ruleset spread over a main file and the files it includes has to be read
 # from, and written back to, the file each table actually lives in
@@ -613,6 +711,7 @@ my $incmain = write_ruleset($confdir, 'main.nft', <<'EOF');
 flush ruleset
 
 define lan = 192.168.0.0/24
+define web_port = 80
 
 include "nftables.d/*.nft"
 
@@ -627,7 +726,7 @@ write_ruleset($incdir, '10-web.nft', <<'EOF');
 table inet web {
 	chain input {
 		type filter hook input priority 10; policy accept;
-		tcp dport 80 accept
+		tcp dport $web_port accept
 	}
 }
 EOF
@@ -641,11 +740,50 @@ table inet mail {
 }
 EOF
 
-is_deeply([ nftables_include_files($incmain) ],
-          [ "$incdir/10-web.nft", "$incdir/20-mail.nft" ],
-          'relative include glob is expanded in order');
-
 {
+    # Match nft's root-file and compiled search paths with test directories.
+    local $nftables_include_paths_cache = [ $confdir ];
+    local $nftables_include_cwd_cache = $confdir;
+    local $nftables_include_basedir_cache = 1;
+
+    is_deeply([ nftables_include_files($incmain) ],
+              [ "$incdir/10-web.nft", "$incdir/20-mail.nft" ],
+              'include glob is expanded below nft search path in order');
+    my $dotmain = write_ruleset($confdir, 'dot-main.nft',
+                                "include \"./nftables.d/20-mail.nft\"\n");
+    is_deeply([ nftables_include_files($dotmain) ],
+              [ "$incdir/20-mail.nft" ],
+              'explicitly relative include uses nft working directory');
+
+    # Current nft versions prepend the root input file's directory to the
+    # search path. A literal match there shadows the compiled-path copy
+    my $rootdir = "$confdir/root-path";
+    mkdir($rootdir);
+    my $rootchild = write_ruleset($rootdir, 'shadow.nft',
+                                  "table inet from_root { }\n");
+    write_ruleset($confdir, 'shadow.nft',
+                  "table inet from_compiled_path { }\n");
+    my $rootmain = write_ruleset($rootdir, 'root-main.nft',
+                                 "include \"shadow.nft\"\n");
+    is_deeply([ nftables_include_files($rootmain) ], [ $rootchild ],
+              'root input directory precedes the compiled include path');
+    {
+        local $nftables_include_basedir_cache = 0;
+        is_deeply([ nftables_include_files($rootmain) ],
+                  [ "$confdir/shadow.nft" ],
+                  'older nft versions use only the compiled include path');
+    }
+
+    # A wildcard collects matches from every directory, and nft reads the
+    # compiled path's files ahead of the root input directory's files
+    write_ruleset($rootdir, 'glob-a.nft', "table inet glob_root { }\n");
+    write_ruleset($confdir, 'glob-b.nft', "table inet glob_compiled { }\n");
+    my $wildmain = write_ruleset($rootdir, 'wild-main.nft',
+                                 "include \"glob-*.nft\"\n");
+    is_deeply([ nftables_include_files($wildmain) ],
+              [ "$confdir/glob-b.nft", "$rootdir/glob-a.nft" ],
+              'wildcard includes collect every directory, compiled path first');
+
     local $nftables_rules_file_cache = $incmain;
     my @inctables = get_nftables_save();
     is_deeply([ map { $_->{name} } @inctables ],
@@ -665,6 +803,45 @@ is_deeply([ nftables_include_files($incmain) ],
     my $main_before = read_file_contents($incmain);
     my $mail_before = read_file_contents("$incdir/20-mail.nft");
 
+    # Apply uses the real saved text, so variables stay in scope, while
+    # includes are expanded and the broad flush command is removed
+    my $apply_text = nftables_apply_text($incmain);
+    unlike($apply_text, qr/^\s*flush\s+ruleset/m,
+           'apply text omits flush ruleset');
+    unlike($apply_text, qr/^\s*include\s/m,
+           'apply text expands include directives');
+    like($apply_text,
+         qr/define web_port = 80.*table inet web \{.*\$web_port/s,
+         'apply text keeps a define in scope for an included table');
+    like($apply_text, qr/table inet mail \{.*table inet main_table \{/s,
+         'included tables remain ahead of the following main-file table');
+
+    my $flushfile = write_ruleset($confdir, 'family-flush.nft',
+        "flush ruleset inet; flush ruleset ip; table inet after_flush { }\n");
+    my $flush_text = nftables_apply_text($flushfile);
+    unlike($flush_text, qr/flush\s+ruleset/,
+           'apply text omits family-qualified flush commands');
+    like($flush_text, qr/table inet after_flush \{ \}/,
+         'commands following family-qualified flushes are retained');
+
+    # Repeated includes are meaningful in nft. Application must expand every
+    # occurrence even though the UI lists each included file only once
+    my $repeat_child = write_ruleset($confdir, 'repeat-child.nft',
+        "add rule inet repeated input counter comment \"repeat marker\"\n");
+    my $repeat_main = write_ruleset($confdir, 'repeat-main.nft', <<'EOF');
+table inet repeated {
+	chain input {
+	}
+}
+include "repeat-child.nft"
+include "repeat-child.nft"
+EOF
+    is_deeply([ nftables_include_files($repeat_main) ], [ $repeat_child ],
+              'configuration file list de-duplicates repeated includes');
+    my $repeat_text = nftables_apply_text($repeat_main);
+    is(scalar(() = $repeat_text =~ /repeat marker/g), 2,
+       'apply text expands every repeated include');
+
     # Editing a table in an included file writes it back there
     my ($web) = grep { $_->{name} eq 'web' } @inctables;
     push(@{$web->{rules}}, {'text' => 'tcp dport 443 accept',
@@ -690,14 +867,22 @@ is_deeply([ nftables_include_files($incmain) ],
        'deleting from an include leaves the main file alone');
     is_deeply([ map { $_->{name} } get_nftables_save() ],
               [ 'main_table', 'mail' ], 'the deleted table is gone');
-}
 
-# An include loop must not send the parser into a spin
-my $loop_a = write_ruleset($confdir, 'loop-a.nft', "include \"loop-b.nft\"\n");
-write_ruleset($confdir, 'loop-b.nft', "include \"loop-a.nft\"\n");
-is_deeply([ nftables_include_files($loop_a) ],
-          [ "$confdir/loop-b.nft", "$confdir/loop-a.nft" ],
-          'an include loop terminates');
+    # An include loop must not send the parser into a spin
+    my $loop_a = write_ruleset($confdir, 'loop-a.nft',
+                               "include \"loop-b.nft\"\n");
+    write_ruleset($confdir, 'loop-b.nft', "include \"loop-a.nft\"\n");
+    is_deeply([ nftables_include_files($loop_a) ],
+              [ "$confdir/loop-b.nft" ],
+              'an include loop terminates');
+    eval {
+        no warnings 'once';
+        local $main::error_must_die = 1;
+        nftables_apply_text($loop_a);
+    };
+    like($@, qr/include files form a loop/,
+         'an include loop is rejected before applying');
+}
 
 # Upgrades have to move rules out of the module's old private file, or the
 # firewall silently disappears once the private boot action is gone
@@ -713,18 +898,41 @@ table inet webmin {
 	}
 }
 EOF
+my $custom_legacy = write_ruleset($confdir, 'custom-legacy.nft', <<'EOF');
+table inet webmin {
+	chain input {
+		type filter hook input priority 0; policy drop;
+		tcp dport 9999 accept
+	}
+}
+table inet legacy_extra {
+}
+EOF
+my $now = time();
+utime($now - 120, $now - 120, $custom_legacy);
+utime($now, $now, $legacy);
 my $target = write_ruleset($confdir, 'migrate-target.nft', <<'EOF');
 # Uncomment the include statement here to load the default config sample
 #include "/etc/nftables/main.nft"
 EOF
 {
     local $nftables_rules_file_cache = $target;
-    is(migrate_legacy_nftables_config(), 1, 'legacy table is migrated');
+    local $config{'save_file'} = $custom_legacy;
+    is(migrate_legacy_nftables_config(), 2,
+       'tables from every legacy file are migrated');
     ok(!-e $legacy, 'legacy rules file is moved aside');
     ok(-e $legacy.'.migrated', 'legacy rules file is kept as a backup');
+    ok(!-e $custom_legacy, 'custom legacy rules file is moved aside');
+    ok(-e $custom_legacy.'.migrated',
+       'custom legacy rules file is kept as a backup');
     my @moved = get_nftables_save($target);
-    is(scalar(@moved), 1, 'migrated table lands in the system file');
-    is($moved[0]->{name}, 'webmin', 'migrated table keeps its name');
+    is(scalar(@moved), 2, 'migrated tables land in the system file');
+    is_deeply([ sort map { $_->{name} } @moved ],
+              [ 'legacy_extra', 'webmin' ],
+              'migrated tables keep their names');
+    my ($webmin) = grep { $_->{name} eq 'webmin' } @moved;
+    is($webmin->{rules}->[0]->{dport}, '10000',
+       'the newest duplicate legacy table is migrated');
     like(read_file_contents($target), qr/Uncomment the include statement/,
          'migration keeps the system file comments');
     is(migrate_legacy_nftables_config(), 0, 'migration only runs once');

From a06590c17e56d7f3ba71fdf916d1163c28478961 Mon Sep 17 00:00:00 2001
From: Ilia Ross <ilia@ross.gdn>
Date: Sun, 13 Sep 2026 01:34:46 +0200
Subject: [PATCH 8/8] Fix to change nftables migration to prefix and validate
 tables

---
 nftables/lang/en         |   1 +
 nftables/nftables-lib.pl | 235 ++++++++++++++++++++++++++++++++-------
 nftables/postinstall.pl  |  10 +-
 nftables/t/run-tests.t   | 203 ++++++++++++++++++++++++++++++---
 4 files changed, 388 insertions(+), 61 deletions(-)

diff --git a/nftables/lang/en b/nftables/lang/en
index 30d911081..e41945759 100644
--- a/nftables/lang/en
+++ b/nftables/lang/en
@@ -117,6 +117,7 @@ apply_err=Failed to apply configuration
 apply_enone=No nftables tables were found in $1 to apply.
 apply_eexternal=Cannot apply configuration because table $1 is currently marked as externally managed.
 apply_einclude_loop=Cannot apply the configuration because its include files form a loop at $1.
+migrate_evalidate=Cannot migrate the deprecated Webmin rules because the resulting system nftables configuration is invalid: $1
 apply_esettype=Set $1 in table $2 has type $3, but chain $4 uses it for $5. Use ipv4_addr or ipv6_addr sets for address fields, and inet_service sets for port fields.
 setup_header=Ruleset profile
 setup_err=Failed to create ruleset profile
diff --git a/nftables/nftables-lib.pl b/nftables/nftables-lib.pl
index 63549af81..8986bf2dc 100644
--- a/nftables/nftables-lib.pl
+++ b/nftables/nftables-lib.pl
@@ -5,6 +5,8 @@ BEGIN { push(@INC, ".."); };    ## no critic
 use WebminCore;
 use strict;
 use warnings;
+use Errno qw(EEXIST);
+use Fcntl qw(O_CREAT O_EXCL O_WRONLY);
 our (%config, %access, %gconfig, $module_config_directory,
      $module_var_directory, $module_root_directory);
 our ($last_config_change_flag, $last_restart_time_flag);
@@ -461,8 +463,7 @@ return $ok ? undef : $err;
 }
 
 # legacy_nftables_rules_files()
-# Returns the private rules files that releases before the switch to the
-# system nftables configuration wrote to
+# Returns obsolete private rules files outside the system configuration
 sub legacy_nftables_rules_files
 {
 my @files = ("$module_config_directory/rules.conf",
@@ -473,7 +474,7 @@ return grep { !$seen{$_}++ && -s $_ } @files;
 }
 
 # legacy_nftables_boot_action()
-# Returns the name of the private boot action those releases created
+# Returns the name of the obsolete private boot action
 sub legacy_nftables_boot_action
 {
 return "webmin-nftables";
@@ -518,9 +519,8 @@ return 1;
 }
 
 # migrate_legacy_nftables_config()
-# Moves tables out of the module's own rules file and into the system
-# nftables configuration, so that upgrading does not silently drop rules that
-# used to be applied at boot. Returns the number of tables moved
+# Moves tables from obsolete private files into the system configuration.
+# Returns the number of tables moved
 sub migrate_legacy_nftables_config
 {
 my $file = nftables_rules_file();
@@ -528,29 +528,155 @@ my @legacy = grep { $_ ne $file } legacy_nftables_rules_files();
 return 0 if (!@legacy);
 
 my @new = get_nftables_save($file);
-my %have = map { table_key($_) => 1 } @new;
+my %used = map { table_key($_) => $_ } @new;
 
-# Several development releases used different private paths. Merge all of
-# them, taking the newest copy when the same table occurs more than once
+# Merge every private file, taking the newest copy when a table occurs more
+# than once
 my %order = map { $legacy[$_] => $_ } 0 .. $#legacy;
 @legacy = sort {
 	(stat($b))[9] <=> (stat($a))[9] || $order{$a} <=> $order{$b}
 	} @legacy;
-my @add;
+my (%seen, @add);
 foreach my $legacy (@legacy) {
 	foreach my $table (get_nftables_save($legacy)) {
-		push(@add, $table) if (!$have{table_key($table)}++);
+		my $oldkey = table_key($table);
+		next if ($seen{$oldkey}++);
+
+		# Prefix imported table names. If the name is already used, add a
+		# migration suffix so neither table is lost.
+		my %renamed = %$table;
+		delete($renamed{'file'});
+		my $base = "webmin_".$table->{'name'};
+		my $number = 0;
+		while (1) {
+			my $suffix = !$number ? "" : $number == 1
+			    ? "_migrated" : "_migrated_".$number;
+			$renamed{'name'} = substr($base, 0, 255 - length($suffix)).
+					   $suffix;
+			my $key = table_key(\%renamed);
+			my $existing = $used{$key};
+			if (!$existing) {
+				$used{$key} = \%renamed;
+				push(@add, \%renamed);
+				last;
+				}
+
+			# A previous run may have saved this table before cleanup.
+			# Treat an identical destination as already migrated.
+			last if (dump_nftables_save($existing) eq
+				 dump_nftables_save(\%renamed));
+			$number++;
+			}
 		}
 	}
 if (@add) {
-	write_configuration(@new, @add);
+	write_migrated_nftables_config($file, \@add);
+	}
+else {
+	# Keep the private files unless the system ruleset that replaces them
+	# can be loaded.
+	my $err = validate_nftables_files($file);
+	error(text('migrate_evalidate', $err)) if ($err);
 	}
 foreach my $legacy (@legacy) {
-	rename_file($legacy, $legacy.".migrated");
+	if (!unlink_file($legacy)) {
+		print STDERR "Failed to remove deprecated nftables rules file ".
+			     "$legacy: $!\n";
+		}
 	}
 return scalar(@add);
 }
 
+# write_migrated_nftables_config(file, &tables)
+# Writes migrated tables to a validated candidate, then atomically replaces
+# the system configuration
+sub write_migrated_nftables_config
+{
+my ($file, $add) = @_;
+my $data = "";
+if (-e $file) {
+	$data = read_file_contents($file);
+	error("Failed to read ".html_escape($file).": $!")
+	    if (!defined($data));
+	}
+
+# Keep the candidate beside the real file so nft resolves relative includes
+# from the same directory. The real file is unchanged during validation.
+my $dir = $file;
+$dir =~ s/\/[^\/]+$//;
+$dir = "/" if ($dir eq "");
+my $candidate_base = $dir."/.webmin-nftables-migrate.$$";
+my ($candidate, $fh);
+for (my $suffix = 0; ; $suffix++) {
+	$candidate = $candidate_base.($suffix ? ".".$suffix : "");
+	last if (sysopen($fh, $candidate, O_WRONLY|O_CREAT|O_EXCL, 0600));
+	next if ($! == EEXIST);
+	error("Failed to create migration candidate ".
+	      html_escape($candidate).": $!");
+	}
+binmode($fh);
+push(@main::temporary_files, $candidate);
+if (!(print $fh $data)) {
+	my $err = $!;
+	close($fh);
+	unlink_file($candidate);
+	error("Failed to write migration candidate ".
+	      html_escape($candidate).": $err");
+	}
+if (!close($fh)) {
+	my $err = $!;
+	unlink_file($candidate);
+	error("Failed to close migration candidate ".
+	      html_escape($candidate).": $err");
+	}
+
+# Keep the candidate private while writing, then apply the target file's mode.
+my @st = stat($file);
+my $mode = @st ? $st[2] & 07777 : 0600;
+if (!chmod($mode, $candidate)) {
+	my $err = $!;
+	unlink_file($candidate);
+	error("Failed to set permissions on migration candidate ".
+	      html_escape($candidate).": $err");
+	}
+
+my @root_tables = parse_nftables_file($candidate);
+rewrite_nftables_file($candidate, [ @root_tables, @$add ]);
+my $err = validate_nftables_files($candidate);
+if ($err) {
+	unlink_file($candidate);
+	error(text('migrate_evalidate', $err));
+	}
+
+# Replace an existing system file while preserving its mode, ownership, ACLs
+# and security attributes. A new file can take the validated candidate's place
+# directly, preserving its private mode.
+if (@st) {
+	my $candidate_data = read_file_contents($candidate);
+	error("Failed to read migration candidate ".html_escape($candidate).": $!")
+	    if (!defined($candidate_data));
+	open_lock_tempfile(my $out, ">".$file);
+	print_tempfile($out, $candidate_data);
+	close_tempfile($out);
+	unlink_file($candidate);
+	}
+else {
+	lock_file($file);
+	if (!rename($candidate, $file)) {
+		my $err = $!;
+		unlock_file($file);
+		unlink_file($candidate);
+		error("Failed to install migration candidate as ".
+		      html_escape($file).": $err");
+		}
+	unlock_file($file);
+	@main::temporary_files = grep { $_ ne $candidate }
+				       @main::temporary_files;
+	}
+update_last_config_change();
+return;
+}
+
 # nftables_include_spec(line)
 # Returns the filename or glob from a top-level include directive
 sub nftables_include_spec
@@ -684,16 +810,17 @@ foreach my $mod (@mods) {
 return @rv;
 }
 
-# validate_nftables_files()
+# validate_nftables_files([file])
 # Returns an error if nft rejects the saved ruleset as it stands on disk.
 # An included file cannot be checked on its own, as it may well use a define
 # from the file that includes it, so the whole ruleset is checked from the
 # top
 sub validate_nftables_files
 {
+my ($file) = @_;
 my $cmd = get_nft_command();
 return text('index_ecommand', "<tt>nft</tt>") if (!$cmd);
-my $file = nftables_rules_file();
+$file ||= nftables_rules_file();
 return if (!-r $file);
 my $cwd = quotemeta(nftables_include_cwd());
 my $out = backquote_logged("cd $cwd && $cmd -c -f ".quotemeta($file)." 2>&1");
@@ -850,38 +977,54 @@ for (my $i = 0 ; $i < @lines ; $i++) {
 	elsif ($line =~ /^\s*set\s+(\S+)\s+\{/) {
 		# Start of a set
 		if ($table) {
-			my $setname = $1;
-			$set = {
-				'name' => $setname,
-				'line' => $lnum,
-				'elements' => [ ],
-				'raw_lines' => [ ],
-			};
-			$table->{'sets'}->{$setname} = $set;
-			$set_depth = nftables_brace_delta($line);
-			$set_elem_open = 0;
-			$set_elem_buf = '';
+			if (nftables_brace_delta($line) == 0 &&
+			    $line =~ /\}\s*;?\s*$/) {
+				# Preserve balanced inline syntax as one raw object.
+				push(@{$table->{'raw_blocks'}},
+				     {'lines' => [ $lines[$i] ]});
+				}
+			else {
+				my $setname = $1;
+				$set = {
+					'name' => $setname,
+					'line' => $lnum,
+					'elements' => [ ],
+					'raw_lines' => [ ],
+				};
+				$table->{'sets'}->{$setname} = $set;
+				$set_depth = nftables_brace_delta($line);
+				$set_elem_open = 0;
+				$set_elem_buf = '';
+				}
 			}
 		}
 	elsif ($line =~ /^\s*chain\s+(\S+)\s+\{/) {
 		# Start of a chain
 		if ($table) {
-			$chain = $1;
-			$table->{'chains'}->{$chain} =
-			    {'order' => scalar(keys %{$table->{'chains'}})};
+			if (nftables_brace_delta($line) == 0 &&
+			    $line =~ /\}\s*;?\s*$/) {
+				# Preserve balanced inline syntax as one raw object.
+				push(@{$table->{'raw_blocks'}},
+				     {'lines' => [ $lines[$i] ]});
+				}
+			else {
+				$chain = $1;
+				$table->{'chains'}->{$chain} =
+				    {'order' => scalar(keys %{$table->{'chains'}})};
 
-			# Look at the next line for a base-chain definition. Policy is
-			# optional in nft and defaults to accept when it is absent
-			my $base = defined($lines[$i + 1])
-			    ? nftables_code_line($lines[$i + 1]) : "";
-			if ($base =~
-			    /^\s*type\s+(\S+)\s+hook\s+(\S+)\s+priority\s+(.+?)\s*;\s*(?:policy\s+(\S+)\s*;\s*)?$/) {
-				$table->{'chains'}->{$chain}->{'type'} = $1;
-				$table->{'chains'}->{$chain}->{'hook'} = $2;
-				$table->{'chains'}->{$chain}->{'priority'} = $3;
-				$table->{'chains'}->{$chain}->{'policy'} = $4
-				    if (defined($4));
-				$i++;    # Skip next line
+				# Look at the next line for a base-chain definition. Policy is
+				# optional in nft and defaults to accept when it is absent
+				my $base = defined($lines[$i + 1])
+				    ? nftables_code_line($lines[$i + 1]) : "";
+				if ($base =~
+				    /^\s*type\s+(\S+)\s+hook\s+(\S+)\s+priority\s+(.+?)\s*;\s*(?:policy\s+(\S+)\s*;\s*)?$/) {
+					$table->{'chains'}->{$chain}->{'type'} = $1;
+					$table->{'chains'}->{$chain}->{'hook'} = $2;
+					$table->{'chains'}->{$chain}->{'priority'} = $3;
+					$table->{'chains'}->{$chain}->{'policy'} = $4
+					    if (defined($4));
+					$i++;    # Skip next line
+					}
 				}
 			}
 		}
@@ -3841,6 +3984,14 @@ return $name;
 sub profile_base_table_name
 {
 my ($profile) = @_;
+return "webmin_".profile_base_name($profile);
+}
+
+# profile_base_name(profile-id)
+# Returns the base name used for generated sets and rules
+sub profile_base_name
+{
+my ($profile) = @_;
 my %names = (
 	'allow_all' => 'profile_allow_all',
 	'management' => 'profile_management',
@@ -3859,7 +4010,7 @@ return $names{$profile} || 'profile_custom';
 sub profile_port_set_name
 {
 my ($profile, $proto, $proto_count) = @_;
-my $name = profile_base_table_name($profile);
+my $name = profile_base_name($profile);
 $name .= "_".$proto if ($proto_count && $proto_count > 1);
 $name .= "_ports";
 $name =~ s/[^\w-]/_/g;
diff --git a/nftables/postinstall.pl b/nftables/postinstall.pl
index a0c5365c6..78fd0ac1d 100644
--- a/nftables/postinstall.pl
+++ b/nftables/postinstall.pl
@@ -8,8 +8,8 @@ no warnings 'uninitialized';
 require 'nftables-lib.pl';    ## no critic
 
 # module_install()
-# Moves off the module's own rules file and boot action, which earlier
-# releases used in place of the system nftables configuration
+# Migrates private rules into the system configuration and removes the
+# obsolete boot action
 sub module_install
 {
 my ($moved, $removed);
@@ -20,7 +20,11 @@ eval {
 	&remove_legacy_managed_metadata();
 	};
 if ($@) {
-	print STDERR "Failed to migrate nftables configuration: $@\n";
+	# Remove the browser-only wrapper before writing the package error
+	my $err = $@;
+	$err =~ s/<\/?pre>//g;
+	$err =~ s/\s+$//;
+	print STDERR "Failed to migrate nftables configuration: $err\n";
 	return;
 	}
 if ($moved) {
diff --git a/nftables/t/run-tests.t b/nftables/t/run-tests.t
index 45cd8417c..52a541310 100755
--- a/nftables/t/run-tests.t
+++ b/nftables/t/run-tests.t
@@ -517,6 +517,8 @@ ok(scalar(grep { $_->{text} eq 'ip6 daddr fe80::/64 udp dport 546 accept' }
 ok(scalar(grep { $_ eq '2022' }
           @{$profile_table->{sets}->{profile_hosting_tcp_ports}->{elements}}),
    'profile helper includes dynamic ssh port');
+is(profile_base_table_name('virtualmin'), 'webmin_profile_hosting',
+   'Virtualmin profiles use a Webmin-prefixed table name');
 
 # The saved configuration is the system's own nftables file, so re-writing
 # it must not discard anything the module does not model
@@ -685,6 +687,28 @@ ok(!rewrite_nftables_file($onelinefile, \@oneline),
 is(read_file_contents($onelinefile), $oneline_before,
    'one-line table and following top-level text stay byte-identical');
 
+# Balanced objects inside a multiline table remain raw and cannot hide its end.
+my $inline_object_file = write_ruleset($confdir, 'inline-objects.nft', <<'EOF');
+table inet inline_objects {
+	chain empty { }
+	set ports { type inet_service; elements = { 80, 443 }; }
+}
+table inet after_inline_objects {
+}
+EOF
+my @inline_objects = parse_nftables_file($inline_object_file);
+is(scalar(@inline_objects), 2,
+   'one-line objects do not swallow the following table');
+is($inline_objects[0]->{end_line}, 4,
+   'a table containing one-line objects has the correct span');
+is(scalar(@{$inline_objects[0]->{raw_blocks}}), 2,
+   'one-line chain and set syntax is preserved as raw content');
+my $inline_before = read_file_contents($inline_object_file);
+ok(!rewrite_nftables_file($inline_object_file, \@inline_objects),
+   'an unchanged table containing one-line objects can be saved');
+is(read_file_contents($inline_object_file), $inline_before,
+   'one-line objects remain byte-identical');
+
 # A file with no tables at all, such as a stock /etc/sysconfig/nftables.conf,
 # keeps its comments and takes new tables at the end
 my $emptyfile = write_ruleset($confdir, 'empty.nft', <<'EOF');
@@ -884,22 +908,34 @@ EOF
          'an include loop is rejected before applying');
 }
 
-# Upgrades have to move rules out of the module's old private file, or the
-# firewall silently disappears once the private boot action is gone
+# Migration must preserve rules before removing the private files and boot
+# action
 mkdir($module_config_directory) if (!-d $module_config_directory);
 my $legacy = write_ruleset($module_config_directory, 'rules.conf', <<'EOF');
 # This file was auto-generated by the module.
 # Manual changes may be overwritten.
 
-table inet webmin {
+table inet profile_hosting {
 	chain input {
 		type filter hook input priority 0; policy drop;
 		tcp dport 10000 accept
 	}
 }
+table inet standard_collision {
+	chain deprecated_copy {
+	}
+}
+table inet destination_collision {
+	chain deprecated_destination_copy {
+	}
+}
+table inet numbered_collision {
+	chain deprecated_numbered_copy {
+	}
+}
 EOF
 my $custom_legacy = write_ruleset($confdir, 'custom-legacy.nft', <<'EOF');
-table inet webmin {
+table inet profile_hosting {
 	chain input {
 		type filter hook input priority 0; policy drop;
 		tcp dport 9999 accept
@@ -914,28 +950,163 @@ utime($now, $now, $legacy);
 my $target = write_ruleset($confdir, 'migrate-target.nft', <<'EOF');
 # Uncomment the include statement here to load the default config sample
 #include "/etc/nftables/main.nft"
+
+table inet retained {
+	chain standard_table {
+	}
+}
+table inet standard_collision {
+	chain standard_copy {
+	}
+}
+table inet webmin_destination_collision {
+	chain existing_prefixed_copy {
+	}
+}
+table inet webmin_numbered_collision {
+	chain existing_numbered_copy {
+	}
+}
+table inet webmin_numbered_collision_migrated {
+	chain existing_migrated_copy {
+	}
+}
 EOF
+chmod(0600, $target);
 {
     local $nftables_rules_file_cache = $target;
     local $config{'save_file'} = $custom_legacy;
-    is(migrate_legacy_nftables_config(), 2,
-       'tables from every legacy file are migrated');
-    ok(!-e $legacy, 'legacy rules file is moved aside');
-    ok(-e $legacy.'.migrated', 'legacy rules file is kept as a backup');
-    ok(!-e $custom_legacy, 'custom legacy rules file is moved aside');
-    ok(-e $custom_legacy.'.migrated',
-       'custom legacy rules file is kept as a backup');
+    no warnings 'redefine';
+    my (@validated, @validated_modes);
+    local *validate_nftables_files = sub {
+        my ($candidate) = @_;
+        push(@validated, read_file_contents($candidate));
+        push(@validated_modes, (stat($candidate))[2] & 07777);
+        return undef;
+    };
+    is(migrate_legacy_nftables_config(), 5,
+       'unique tables from every legacy file are migrated');
+    ok(!-e $legacy, 'legacy rules file is deleted after migration');
+    ok(!-e $legacy.'.migrated', 'migration does not retain a backup');
+    ok(!-e $custom_legacy, 'custom legacy rules file is deleted');
+    ok(!-e $custom_legacy.'.migrated',
+       'custom legacy file leaves no backup');
+    is(scalar(@validated), 1, 'the complete candidate is validated once');
+    is(sprintf('%04o', $validated_modes[0]), '0600',
+       'the candidate is no more readable than the system file');
+    is(sprintf('%04o', (stat($target))[2] & 07777), '0600',
+       'migration keeps the system file mode');
     my @moved = get_nftables_save($target);
-    is(scalar(@moved), 2, 'migrated tables land in the system file');
+    is(scalar(@moved), 10, 'migrated tables join the system configuration');
     is_deeply([ sort map { $_->{name} } @moved ],
-              [ 'legacy_extra', 'webmin' ],
-              'migrated tables keep their names');
-    my ($webmin) = grep { $_->{name} eq 'webmin' } @moved;
-    is($webmin->{rules}->[0]->{dport}, '10000',
+              [ 'retained', 'standard_collision',
+                'webmin_destination_collision',
+                'webmin_destination_collision_migrated',
+                'webmin_legacy_extra', 'webmin_numbered_collision',
+                'webmin_numbered_collision_migrated',
+                'webmin_numbered_collision_migrated_2',
+                'webmin_profile_hosting',
+                'webmin_standard_collision' ],
+              'migrated tables receive Webmin-prefixed names');
+    my ($profile) = grep { $_->{name} eq 'webmin_profile_hosting' } @moved;
+    is($profile->{rules}->[0]->{dport}, '10000',
        'the newest duplicate legacy table is migrated');
     like(read_file_contents($target), qr/Uncomment the include statement/,
          'migration keeps the system file comments');
+    like(read_file_contents($target), qr/chain standard_table/,
+         'migration preserves the existing system table');
+    like(read_file_contents($target), qr/chain standard_copy/,
+         'migration keeps a system table whose name matches a legacy table');
+    like(read_file_contents($target), qr/chain deprecated_copy/,
+         'an old-name match is migrated under the Webmin prefix');
+    like(read_file_contents($target), qr/chain existing_prefixed_copy/,
+         'migration preserves an existing prefixed table');
+    like(read_file_contents($target), qr/chain deprecated_destination_copy/,
+         'a prefixed-name collision receives a migration suffix');
+    like(read_file_contents($target), qr/chain deprecated_numbered_copy/,
+         'migration adds a number when the suffix is already used');
+
+    # Re-running cleanup after the system file was saved must not duplicate
+    # a table under the next available suffix.
+    my $before_retry = read_file_contents($target);
+    my ($numbered) = grep {
+        $_->{name} eq 'webmin_numbered_collision_migrated_2'
+    } @moved;
+    my %retry = %$numbered;
+    $retry{name} = 'numbered_collision';
+    delete($retry{file});
+    write_ruleset($module_config_directory, 'rules.conf',
+                  dump_nftables_save(\%retry));
+    is(migrate_legacy_nftables_config(), 0,
+       'an identical migrated table is not duplicated');
+    is(read_file_contents($target), $before_retry,
+       'cleanup retry leaves the system configuration unchanged');
+    ok(!-e $legacy, 'cleanup retry removes the legacy file');
     is(migrate_legacy_nftables_config(), 0, 'migration only runs once');
 }
 
+# A missing system file is installed atomically with a private mode.
+my $new_legacy = write_ruleset($module_config_directory, 'rules.conf', <<'EOF');
+table inet create_target {
+}
+EOF
+my $new_target = "$confdir/new-migrate-target.nft";
+{
+    local $nftables_rules_file_cache = $new_target;
+    no warnings 'redefine';
+    local *validate_nftables_files = sub { return; };
+    is(migrate_legacy_nftables_config(), 1,
+       'migration creates a missing system file');
+    is(sprintf('%04o', (stat($new_target))[2] & 07777), '0600',
+       'a new system file is private');
+    ok(!-e $new_legacy,
+       'creating the system file removes the legacy file');
+}
+
+# A rejected candidate must leave both source configurations byte-identical.
+my $failed_legacy = write_ruleset($module_config_directory, 'rules.conf', <<'EOF');
+table inet retry_me {
+}
+EOF
+my $failed_target = write_ruleset($confdir, 'failed-migrate-target.nft', <<'EOF');
+table inet keep_me {
+}
+EOF
+my $failed_before = read_file_contents($failed_target);
+{
+    local $nftables_rules_file_cache = $failed_target;
+    no warnings 'redefine';
+    local *validate_nftables_files = sub { return 'test rejection'; };
+    local $main::error_must_die = 1;
+    eval { migrate_legacy_nftables_config(); };
+    like($@, qr/test rejection/, 'invalid migration candidate is rejected');
+    is(read_file_contents($failed_target), $failed_before,
+       'failed migration leaves the system configuration unchanged');
+    ok(-e $failed_legacy,
+       'failed migration leaves the deprecated configuration in place');
+}
+
+# An existing destination that cannot be read must never be treated as empty.
+my $unreadable_target = write_ruleset($confdir, 'unreadable-target.nft', <<'EOF');
+table inet keep_unreadable {
+}
+EOF
+my $unreadable_before = read_file_contents($unreadable_target);
+chmod(0000, $unreadable_target);
+SKIP: {
+    if (-r $unreadable_target) {
+        chmod(0600, $unreadable_target);
+        skip('the test user can read mode 0000 files', 3);
+    }
+    local $nftables_rules_file_cache = $unreadable_target;
+    local $main::error_must_die = 1;
+    eval { migrate_legacy_nftables_config(); };
+    like($@, qr/Failed to read/, 'unreadable system configuration is rejected');
+    chmod(0600, $unreadable_target);
+    is(read_file_contents($unreadable_target), $unreadable_before,
+       'unreadable system configuration is not replaced');
+    ok(-e $failed_legacy,
+       'unreadable system configuration leaves deprecated rules in place');
+}
+
 done_testing();