Merge pull request #2294 from pi-hole/tweak/validate_envs

Also validate env variables
pi-hole · Feb 28, 2025 · ff1a528 · ff1a528
2 parents 0c9a3cd + efc51ed
commit ff1a528
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/src/config/env.c b/src/config/env.c
@@ -122,10 +122,10 @@ void printFTLenv(void)
 			else
 			{
 				if(item->error != NULL)
-					log_err("  %s %s %s",
+					log_err("  %s %s %s, using default instead",
 						cli_cross(), item->key, item->error);
 				else
-					log_err("  %s %s is invalid",
+					log_err("  %s %s is invalid, using default instead",
 					        cli_cross(), item->key);
 
 				if(item->error_allocated)
@@ -613,6 +613,20 @@ bool __attribute__((nonnull(1,2,3))) readEnvValue(struct conf_item *conf_item, s
 		}
 	}
 
+	// Validate enum value if it is valid and validation function is defined
+	char errbuf[VALIDATOR_ERRBUF_LEN] = { 0 };
+	if(conf_item->c != NULL && !conf_item->c(&conf_item->v, conf_item->k, errbuf))
+	{
+		log_debug(DEBUG_CONFIG, "Config item validation failed for %s: %s", conf_item->k, errbuf);
+		item->error = strdup(errbuf);
+		item->error_allocated = true;
+		item->valid = false;
+		return false;
+	}
+
+
+	log_debug(DEBUG_CONFIG, "Env var %s passed validation successfully", conf_item->k);
+
 	return true;
 }
 

diff --git a/test/run.sh b/test/run.sh
@@ -78,6 +78,7 @@ export FTLCONF_misc_nice="-11"
 export FTLCONF_dns_upstrrr="-11"
 export FTLCONF_debug_api="not_a_bool"
 export FTLCONF_MISC_CHECK_SHMEM=91
+export FTLCONF_files_pcap='*123#./test/pcap'
 
 # Prepare gdb session
 echo "handle SIGHUP nostop SIGPIPE nostop SIGTERM nostop SIG32 nostop SIG33 nostop SIG34 nostop SIG35 nostop SIG41 nostop" > /root/.gdbinit

diff --git a/test/test_suite.bats b/test/test_suite.bats
@@ -756,7 +756,7 @@
 @test "No ERROR messages in FTL.log (besides known/intended error)" {
   run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log'
   printf "%s\n" "${lines[@]}"
-  run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is not a boolean)|(Failed to set|adjust time during NTP sync: Insufficient permissions)"'
+  run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is not a boolean)|(FTLCONF_files_pcap files.pcap: not a valid file path)|(Failed to set|adjust time during NTP sync: Insufficient permissions)"'
   printf "count: %s\n" "${lines[@]}"
   [[ ${lines[0]} == "0" ]]
 }
@@ -1630,24 +1630,40 @@
 }
 
 @test "Correct number of environmental variables is logged" {
-  run bash -c 'grep -q "4 FTLCONF environment variables found (2 used, 1 invalid, 1 ignored)" /var/log/pihole/FTL.log'
+  grep "FTLCONF environment variables" /var/log/pihole/FTL.log
+  printf "%s\n" "${lines[@]}"
+  run bash -c 'grep -q "5 FTLCONF environment variables found (2 used, 2 invalid, 1 ignored)" /var/log/pihole/FTL.log'
   printf "%s\n" "${lines[@]}"
   [[ $status == 0 ]]
 }
 
 @test "Correct environmental variable is logged" {
+  grep "FTLCONF_misc_nice" /var/log/pihole/FTL.log
+  printf "%s\n" "${lines[@]}"
   run bash -c 'grep -q "FTLCONF_misc_nice is used" /var/log/pihole/FTL.log'
   printf "%s\n" "${lines[@]}"
   [[ $status == 0 ]]
 }
 
-@test "Invalid environmental variable is logged" {
-  run bash -c 'grep -q "FTLCONF_debug_api is not a boolean" /var/log/pihole/FTL.log'
+@test "Invalid environmental variable is logged (type mismatch)" {
+  grep "FTLCONF_debug_api" /var/log/pihole/FTL.log
+  printf "%s\n" "${lines[@]}"
+  run bash -c 'grep -q "FTLCONF_debug_api is not a boolean, using default instead" /var/log/pihole/FTL.log'
+  printf "%s\n" "${lines[@]}"
+  [[ $status == 0 ]]
+}
+
+@test "Invalid environmental variable is logged (validation failed)" {
+  grep "FTLCONF_files_pcap" /var/log/pihole/FTL.log
+  printf "%s\n" "${lines[@]}"
+  run bash -c 'grep -q "FTLCONF_files_pcap files.pcap: not a valid file path (\"\*123#./test/pcap\"), using default instead" /var/log/pihole/FTL.log'
   printf "%s\n" "${lines[@]}"
   [[ $status == 0 ]]
 }
 
 @test "Unknown environmental variable is logged, a useful alternative is suggested" {
+  grep "FTLCONF_dns_upstrrr" /var/log/pihole/FTL.log
+  printf "%s\n" "${lines[@]}"
   run bash -c 'grep -A1 "FTLCONF_dns_upstrrr is unknown" /var/log/pihole/FTL.log'
   printf "%s\n" "${lines[@]}"
   [[ ${lines[0]} == *"WARNING: [?] FTLCONF_dns_upstrrr is unknown, did you mean any of these?" ]]