refactor(sanity): sync ranges and add validation coverage

.github/tests/unit/sanity.t

@@ -0,0 +1,237 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use FindBin qw($Bin);
+use File::Spec;
+use File::Temp qw(tempdir);
+use Test::More;
+use lib "$Bin/../lib";
+
+use TestBootstrap ();
+require ConfigServer::Sanity;
+
+{
+    package Local::SanityConfig;
+
+    sub new {
+        my ($class, $config) = @_;
+        return bless { config => $config }, $class;
+    }
+
+    sub config {
+        my ($self) = @_;
+        return %{ $self->{config} };
+    }
+}
+
+sub write_test_sanity_file {
+    my $dir = tempdir(CLEANUP => 1);
+    my $path = File::Spec->catfile($dir, 'sanity.txt');
+
+    open(my $fh, '>', $path) or die "Unable to create test sanity file $path: $!";
+    print {$fh} <<'EOF';
+AT_INTERVAL=10-3600=60
+DROP=DROP|TARPIT|REJECT=DROP
+CT_LIMIT=0|10-1000=0
+DENY_IP_LIMIT=10-1000=200
+EOF
+    close($fh);
+
+    return ($dir, $path);
+}
+
+sub reset_sanity_state {
+    %ConfigServer::Sanity::sanity = ();
+    %ConfigServer::Sanity::sanitydefault = ();
+    $ConfigServer::Sanity::loaded = 0;
+    return;
+}
+
+sub with_mock_config {
+    my ($config, $code) = @_;
+
+    no warnings qw(redefine once);
+    local *ConfigServer::Config::loadconfig = sub {
+        return Local::SanityConfig->new($config);
+    };
+
+    return $code->();
+}
+
+subtest 'sanity data is loaded lazily on first use' => sub {
+    my (undef, $path) = write_test_sanity_file();
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 0 }, sub {
+        is($ConfigServer::Sanity::loaded, 0, 'sanity rules are not loaded at import time');
+        is(scalar keys %ConfigServer::Sanity::sanity, 0, 'sanity hash is empty before first call');
+        is(scalar keys %ConfigServer::Sanity::sanitydefault, 0, 'default hash is empty before first call');
+
+        my ($insane, $range, $default) = ConfigServer::Sanity::sanity('AT_INTERVAL', '60');
+
+        is($insane, 0, 'first call validates successfully');
+        is($range, '10-3600', 'range comes from the loaded sanity file');
+        is($default, '60', 'default comes from the loaded sanity file');
+        is($ConfigServer::Sanity::loaded, 1, 'first call loads sanity rules');
+    });
+};
+
+subtest 'range, discrete, and mixed rules are validated correctly' => sub {
+    my (undef, $path) = write_test_sanity_file();
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 0 }, sub {
+        my ($insane, $range, $default);
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('AT_INTERVAL', '10');
+        is($insane, 0, 'range rule accepts the lower boundary');
+        is($range, '10-3600', 'range rule is reported as stored');
+        is($default, '60', 'range rule default is returned');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('AT_INTERVAL', '3601');
+        is($insane, 1, 'range rule rejects values above the upper boundary');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('DROP', 'TARPIT');
+        is($insane, 0, 'discrete rule accepts an allowed token');
+        is($range, 'DROP or TARPIT or REJECT', 'discrete rule is formatted for display');
+        is($default, 'DROP', 'discrete rule default is returned');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('DROP', 'QUEUE');
+        is($insane, 1, 'discrete rule rejects an unsupported token');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('CT_LIMIT', '0');
+        is($insane, 0, 'mixed rule accepts its exact zero value');
+        is($range, '0 or 10-1000', 'mixed rule keeps both exact and ranged choices');
+        is($default, '0', 'mixed rule default is returned');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('CT_LIMIT', '500');
+        is($insane, 0, 'mixed rule accepts values in its numeric range');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('CT_LIMIT', '5');
+        is($insane, 1, 'mixed rule rejects values outside every allowed branch');
+    });
+};
+
+subtest 'undef values return early without loading sanity rules' => sub {
+    my (undef, $path) = write_test_sanity_file();
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 0 }, sub {
+        my @result = ConfigServer::Sanity::sanity('AT_INTERVAL', undef);
+
+        is_deeply(\@result, [0], 'undef values return the early 0 result');
+        is($ConfigServer::Sanity::loaded, 0, 'undef values do not trigger lazy loading');
+        is(scalar keys %ConfigServer::Sanity::sanity, 0, 'undef values leave the sanity cache empty');
+    });
+};
+
+subtest 'display formatting does not mutate cached rules' => sub {
+    my (undef, $path) = write_test_sanity_file();
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 0 }, sub {
+        my ($insane, $range, $default) = ConfigServer::Sanity::sanity('DROP', 'TARPIT');
+        is($insane, 0, 'first lookup validates an allowed token');
+        is($range, 'DROP or TARPIT or REJECT', 'display output is formatted for humans');
+        is($default, 'DROP', 'default value is preserved');
+        is($ConfigServer::Sanity::sanity{DROP}, 'DROP|TARPIT|REJECT', 'cached rule keeps raw separators after formatting');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('DROP', 'REJECT');
+        is($insane, 0, 'later lookups still validate against the cached rule');
+        is($range, 'DROP or TARPIT or REJECT', 'later lookups still report the formatted display value');
+        is($ConfigServer::Sanity::sanity{DROP}, 'DROP|TARPIT|REJECT', 'cached rule remains unchanged across calls');
+    });
+};
+
+subtest 'cached sanity data is reused after first load' => sub {
+    my (undef, $first_path) = write_test_sanity_file();
+    my $second_dir = tempdir(CLEANUP => 1);
+    my $second_path = File::Spec->catfile($second_dir, 'sanity.txt');
+
+    open(my $fh, '>', $second_path) or die "Unable to create second sanity file $second_path: $!";
+    print {$fh} <<'EOF';
+AT_INTERVAL=100-200=150
+DROP=DROP|REJECT=DROP
+CT_LIMIT=1-5=2
+DENY_IP_LIMIT=500-600=550
+EOF
+    close($fh);
+
+    reset_sanity_state();
+
+    with_mock_config({ IPSET => 0 }, sub {
+        local $ConfigServer::Sanity::sanityfile = $first_path;
+        my ($insane, $range, $default) = ConfigServer::Sanity::sanity('AT_INTERVAL', '60');
+
+        is($insane, 0, 'first file is used for initial load');
+        is($range, '10-3600', 'initial range is from the first file');
+        is($default, '60', 'initial default is from the first file');
+
+        local $ConfigServer::Sanity::sanityfile = $second_path;
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('AT_INTERVAL', '60');
+
+        is($insane, 0, 'cached data remains active after file path changes');
+        is($range, '10-3600', 'cached range is unchanged after first load');
+        is($default, '60', 'cached default is unchanged after first load');
+    });
+};
+
+subtest 'whitespace is ignored and unknown keys stay non-fatal' => sub {
+    my (undef, $path) = write_test_sanity_file();
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 0 }, sub {
+        my ($insane, $range, $default) = ConfigServer::Sanity::sanity(' DROP ', ' TARPIT ');
+        is($insane, 0, 'leading and trailing whitespace is ignored');
+        is($range, 'DROP or TARPIT or REJECT', 'whitespace does not change the displayed rule');
+        is($default, 'DROP', 'whitespace does not change the default');
+
+        ($insane, $range, $default) = ConfigServer::Sanity::sanity('UNKNOWN_ITEM', '999');
+        is($insane, 0, 'unknown keys are treated as non-insane');
+        is($range, undef, 'unknown keys have no reported range');
+        is($default, undef, 'unknown keys have no reported default');
+    });
+};
+
+subtest 'DENY_IP_LIMIT is skipped when IPSET is enabled' => sub {
+    my (undef, $path) = write_test_sanity_file();
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 1 }, sub {
+        my ($insane, $range, $default) = ConfigServer::Sanity::sanity('DENY_IP_LIMIT', '5');
+
+        is($insane, 0, 'DENY_IP_LIMIT is not validated when IPSET is enabled');
+        is($range, undef, 'no range is reported when DENY_IP_LIMIT is skipped');
+        is($default, undef, 'no default is reported when DENY_IP_LIMIT is skipped');
+    });
+};
+
+subtest 'missing sanity file fails with a clear error' => sub {
+    my $dir = tempdir(CLEANUP => 1);
+    my $path = File::Spec->catfile($dir, 'missing-sanity.txt');
+
+    reset_sanity_state();
+    local $ConfigServer::Sanity::sanityfile = $path;
+
+    with_mock_config({ IPSET => 0 }, sub {
+        my $ok = eval { ConfigServer::Sanity::sanity('AT_INTERVAL', '60'); 1 };
+        ok(!$ok, 'sanity() dies when the sanity file is missing');
+        like($@, qr/^Cannot open \Q$path\E:/, 'error message includes the missing file path');
+    });
+};
+
+done_testing();

ConfigServer/Sanity.pm

@@ -17,7 +17,6 @@
 # this program; if not, see <https://www.gnu.org/licenses>.
 ###############################################################################
 ## no critic (RequireUseWarnings, ProhibitExplicitReturnUndef, ProhibitMixedBooleanOperators, RequireBriefOpen)
-# start main
 package ConfigServer::Sanity;
 
 use strict;
@@ -27,60 +26,95 @@ use Carp;
 use ConfigServer::Config;
 
 use Exporter qw(import);
-our $VERSION     = 1.02;
-our @ISA         = qw(Exporter);
-our @EXPORT_OK   = qw(sanity);
-
-my %sanity;
-my %sanitydefault;
-my $sanityfile = "/usr/local/csf/lib/sanity.txt";
-
-open (my $IN, "<", $sanityfile);
-flock ($IN, LOCK_SH);
-my @data = <$IN>;
-close ($IN);
-chomp @data;
-foreach my $line (@data) {
-	my ($name,$value,$def) = split(/\=/,$line);
-	$sanity{$name} = $value;
-	$sanitydefault{$name} = $def;
-}
-
-my $config = ConfigServer::Config->loadconfig();
-my %config = $config->config();
+our $VERSION   = 1.02;
+our @ISA       = qw(Exporter);
+our @EXPORT_OK = qw(sanity);
 
-if ($config{IPSET}) {
-	delete $sanity{"DENY_IP_LIMIT"};
-	delete $sanitydefault{"DENY_IP_LIMIT"};
-}
+# Cached validation metadata loaded from sanity.txt.
+#
+# Each file entry is expected to be in the form:
+#   NAME=ALLOWED=DEFAULT
+#
+# Examples:
+#   AT_INTERVAL=10-3600=60
+#   DROP=DROP|TARPIT|REJECT=DROP
+#   CT_LIMIT=0|10-1000=0
+#
+# %sanity stores the allowed-value expression for each setting and
+# %sanitydefault stores the recommended default value.
+our %sanity;
+our %sanitydefault;
+our $loaded     = 0;
+our $sanityfile = "/usr/local/csf/lib/sanity.txt";
 
-# end main
-###############################################################################
-# start sanity
 sub sanity {
-	my $sanity_item = shift;
+	my $sanity_item  = shift;
 	my $sanity_value = shift;
-	my $insane = 0;
+	my $insane       = 0;
+
+	# Preserve historical behaviour for undefined input and avoid loading the
+	# validation table when there is no value to validate.
+	return 0 unless defined $sanity_value;
+
+	# Load the validation table on first use and keep it cached for the rest of
+	# the process lifetime. This keeps module load cheap and makes the runtime
+	# behaviour deterministic after the first lookup.
+	if (!$loaded) {
+		open my $IN, '<', $sanityfile or croak "Cannot open $sanityfile: $!";
+		flock $IN, LOCK_SH;
+		chomp(my @data = <$IN>);
+		close $IN;
+
+		%sanity        = ();
+		%sanitydefault = ();
+
+		foreach my $line (@data) {
+			my ($name, $value, $def) = split(/\=/, $line, 3);
+			$sanity{$name}        = $value;
+			$sanitydefault{$name} = $def;
+		}
+
+		my $config_obj = ConfigServer::Config->loadconfig();
+		my %config_values = $config_obj->config();
+
+		# When IPSET is enabled, DENY_IP_LIMIT no longer applies, so remove its
+		# rule from the cached table before any validations are performed.
+		if ($config_values{IPSET}) {
+			delete $sanity{DENY_IP_LIMIT};
+			delete $sanitydefault{DENY_IP_LIMIT};
+		}
+
+		$loaded = 1;
+	}
 
+	$sanity_item = '' unless defined $sanity_item;
 	$sanity_item =~ s/\s//g;
 	$sanity_value =~ s/\s//g;
 
+	# Rules support both numeric ranges (10-3600) and exact tokens
+	# (DROP|TARPIT|REJECT). A setting is marked insane until one branch matches.
 	if (defined $sanity{$sanity_item}) {
 		$insane = 1;
-		foreach my $check (split(/\|/,$sanity{$sanity_item})) {
+		foreach my $check (split(/\|/, $sanity{$sanity_item})) {
 			if ($check =~ /-/) {
-				my ($from,$to) = split(/\-/,$check);
-				if (($sanity_value >= $from) and ($sanity_value <= $to)) {$insane = 0}
-
-			} else {
-				if ($sanity_value eq $check) {$insane = 0}
+				my ($from, $to) = split(/\-/, $check);
+				if (($sanity_value >= $from) and ($sanity_value <= $to)) { $insane = 0 }
+			}
+			else {
+				if ($sanity_value eq $check) { $insane = 0 }
 			}
 		}
-		$sanity{$sanity_item} =~ s/\|/ or /g;
 	}
-	return ($insane,$sanity{$sanity_item},$sanitydefault{$sanity_item});
+
+	# Keep cached rule text untouched and only format a display copy for callers.
+	my $acceptable_display = defined $sanity{$sanity_item} ? $sanity{$sanity_item} : undef;
+	$acceptable_display =~ s/\|/ or /g if defined $acceptable_display;
+
+	# Return value tuple:
+	#   0/1  => sane or insane
+	#   text => acceptable values for display
+	#   text => recommended default value
+	return ($insane, $acceptable_display, $sanitydefault{$sanity_item});
 }
-# end sanity
-###############################################################################
 
-1;
+1;