ci: enforce regression tests and harden import loading

.github/workflows/plugin-ci-workflow.yml

@@ -40,6 +40,7 @@ jobs:
       uses: actions/checkout@v4
       with:
         repository: Cacti/cacti
+        ref: release/1.2.31
         path: cacti
 
     - name: Checkout Syslog Plugin
@@ -57,11 +58,18 @@ jobs:
     - name: Check PHP Syntax (Lint)
       run: |
         cd cacti/plugins/syslog
-        if find . -name '*.php' -not -path './vendor/*' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then
+        if find . -name '*.php' -not -path './vendor/*' -exec php -l {} \; 2>&1 | grep -iv 'no syntax errors detected'; then
           echo "Syntax errors found!"
           exit 1
         fi
 
+    - name: Run Regression Tests
+      run: |
+        cd cacti/plugins/syslog
+        for test in tests/regression/*.php; do
+          php "$test"
+        done
+
     - name: Run PHP CS Fixer (Dry Run)
       run: |
         cd cacti/plugins/syslog
@@ -122,6 +130,7 @@ jobs:
       uses: actions/checkout@v4
       with:
         repository: Cacti/cacti
+        ref: release/1.2.31
         path: cacti
 
     - name: Checkout Syslog Plugin
@@ -143,7 +152,7 @@ jobs:
       run: sudo apt-get update
 
     - name: Install System Dependencies
-      run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }}
+      run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php
 
     - name: Start SNMPD Agent and Test
       run: |
@@ -163,16 +172,15 @@ jobs:
         echo -e "[client]\nuser = root\npassword = cactiroot\nhost = 127.0.0.1\n" > ~/.my.cnf
 
     - name: Initialize Cacti Database
-      env:
-        MYSQL_AUTH_USR: '--defaults-file=~/.my.cnf'
       run: |
-        mysql $MYSQL_AUTH_USR -e 'CREATE DATABASE IF NOT EXISTS cacti;'
-        mysql $MYSQL_AUTH_USR -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';"
-        mysql $MYSQL_AUTH_USR -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';"
-        mysql $MYSQL_AUTH_USR -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';"
-        mysql $MYSQL_AUTH_USR -e "FLUSH PRIVILEGES;"
-        mysql $MYSQL_AUTH_USR cacti < ${{ github.workspace }}/cacti/cacti.sql
-        mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti
+        MYSQL_AUTH_USR="--defaults-file=$HOME/.my.cnf"
+        mysql "$MYSQL_AUTH_USR" -e 'CREATE DATABASE IF NOT EXISTS cacti;'
+        mysql "$MYSQL_AUTH_USR" -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';"
+        mysql "$MYSQL_AUTH_USR" -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';"
+        mysql "$MYSQL_AUTH_USR" -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';"
+        mysql "$MYSQL_AUTH_USR" -e "FLUSH PRIVILEGES;"
+        mysql "$MYSQL_AUTH_USR" cacti < ${{ github.workspace }}/cacti/cacti.sql
+        mysql "$MYSQL_AUTH_USR" -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti
 
     - name: Validate composer files
       run: |

CHANGELOG.md

@@ -12,6 +12,7 @@
 * issue#278: Extract alert command execution into shared helper in functions.php; command tokenization now uses preg_split (handles tabs and consecutive spaces); /bin/sh fallback for non-executable command templates removed (use absolute paths with execute bit set)
 * issue#298: syslog poller: lock timeout, signal handler, and earlier partition rotation
 * issue#300: Syslog table drop with no apparent reason
+* issue: Repair plugin integration CI, execute regression tests, and reject empty import files safely
 * issue: Making changes to support Cacti 1.3
 * issue: Don't use MyISAM for non-analytical tables
 * issue: The install advisor for Syslog was broken in current Cacti releases

functions.php

@@ -145,9 +145,11 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') {
 }
 
 function syslog_get_import_xml_payload($redirect_url) {
-	if (trim(get_nfilter_request_var('import_text')) != '') {
+	$import_text = (string) get_nfilter_request_var('import_text');
+
+	if (trim($import_text) !== '') {
 		// textbox input
-		return get_nfilter_request_var('import_text');
+		return $import_text;
 	}
 
 	if (isset($_FILES['import_file']['tmp_name']) &&
@@ -166,19 +168,10 @@ function syslog_get_import_xml_payload($redirect_url) {
 			exit;
 		}
 
-		$fp = fopen($tmp_name, 'rb');
-
-		if ($fp === false) {
-			cacti_log('SYSLOG ERROR: Failed to open uploaded import file', false, 'SYSTEM');
-			header('Location: ' . $redirect_url);
-			exit;
-		}
-
-		$xml_data = fread($fp, filesize($tmp_name));
-		fclose($fp);
+		$xml_data = syslog_read_import_file($tmp_name);
 
 		if ($xml_data === false) {
-			cacti_log('SYSLOG ERROR: Failed to read uploaded import file', false, 'SYSTEM');
+			cacti_log('SYSLOG ERROR: Uploaded import file is empty or unreadable', false, 'SYSTEM');
 			header('Location: ' . $redirect_url);
 			exit;
 		}
@@ -190,6 +183,26 @@ function syslog_get_import_xml_payload($redirect_url) {
 	exit;
 }
 
+function syslog_read_import_file(string $filename): string|false {
+	$size = filesize($filename);
+
+	if ($size === false || $size < 1) {
+		return false;
+	}
+
+	$handle = fopen($filename, 'rb');
+
+	if ($handle === false) {
+		return false;
+	}
+
+	try {
+		return fread($handle, $size);
+	} finally {
+		fclose($handle);
+	}
+}
+
 function syslog_is_partitioned() {
 	global $syslogdb_default;
 

tests/regression/issue253_alert_sql_placeholder_test.php

@@ -30,14 +30,14 @@ function issue253_assert($condition, $message) {
 $hostSql = syslog_get_alert_sql($hostAlert, 55);
 $progSql = syslog_get_alert_sql($programAlert, 66);
 
-issue253_assert(strpos($hostSql['sql'], 'AND `status` = ?') !== false, 'Host alert SQL must keep status as a placeholder.');
-issue253_assert(strpos($hostSql['sql'], '?55') === false, 'Host alert SQL must not concatenate uniqueID into SQL text.');
+issue253_assert(strpos($hostSql['sql'], 'AND `status` = 1') !== false, 'Host alert SQL must select processed incoming rows.');
+issue253_assert(strpos($hostSql['sql'], 'AND `seq` <= ?') !== false, 'Host alert SQL must bound rows by sequence.');
 issue253_assert(count($hostSql['params']) === 2, 'Host alert SQL must pass two prepared parameters.');
-issue253_assert($hostSql['params'][1] === 55, 'Host alert status param should be the uniqueID.');
+issue253_assert($hostSql['params'][1] === 55, 'Host alert sequence parameter should be the processing boundary.');
 
-issue253_assert(strpos($progSql['sql'], 'AND `status` = ?') !== false, 'Program alert SQL must keep status as a placeholder.');
-issue253_assert(strpos($progSql['sql'], '?66') === false, 'Program alert SQL must not concatenate uniqueID into SQL text.');
+issue253_assert(strpos($progSql['sql'], 'AND `status` = 1') !== false, 'Program alert SQL must select processed incoming rows.');
+issue253_assert(strpos($progSql['sql'], 'AND `seq` <= ?') !== false, 'Program alert SQL must bound rows by sequence.');
 issue253_assert(count($progSql['params']) === 2, 'Program alert SQL must pass two prepared parameters.');
-issue253_assert($progSql['params'][1] === 66, 'Program alert status param should be the uniqueID.');
+issue253_assert($progSql['params'][1] === 66, 'Program alert sequence parameter should be the processing boundary.');
 
 print "issue253_alert_sql_placeholder_test passed\n";

tests/regression/issue269_import_text_branch_logic_test.php

@@ -3,11 +3,8 @@
 /*
  * Regression test for issue #269 -- branch-logic invariants.
  *
- * These assertions verify the structural properties that make whitespace-only
- * input fall through to the file-upload branch instead of the textbox branch,
- * and that a non-empty payload is assigned to $xml_data without further
- * modification.  Pure source inspection: the functions themselves cannot be
- * called in isolation because they depend on the Cacti runtime.
+ * Import parsing now lives in syslog_get_import_xml_payload().  Each route must
+ * delegate to that helper instead of maintaining a divergent text/file branch.
  */
 
 $root    = dirname(__DIR__, 2);
@@ -25,42 +22,14 @@
 		exit(1);
 	}
 
-	/*
-	 * 1. The request variable must be captured into a local first.
-	 *    Whitespace-only input falls through only because trim() is applied
-	 *    to the local; if the assignment were missing the condition would
-	 *    be wrong.
-	 */
-	if (!preg_match('/\$import_text\s*=\s*get_nfilter_request_var\s*\(\s*\'import_text\'\s*\)/', $content)) {
-		fwrite(STDERR, "$func: \$import_text assignment via get_nfilter_request_var missing in $target\n");
+	if (substr_count($content, 'syslog_get_import_xml_payload(') !== 1) {
+		fwrite(STDERR, "$func: import route must call the shared payload helper exactly once in $target\n");
 		exit(1);
 	}
 
-	/*
-	 * 2. The branch condition must trim the local variable, not the raw
-	 *    request call.  This is what makes whitespace-only values fall
-	 *    through to the file-upload branch.
-	 */
-	if (!preg_match('/trim\s*\(\s*\$import_text\s*\)\s*!=\s*\'\'/', $content)) {
-		fwrite(STDERR, "$func: trim(\$import_text) != '' condition missing in $target\n");
-		exit(1);
-	}
-
-	/*
-	 * 3. Inside the textbox branch, $xml_data must be assigned the
-	 *    untrimmed local.  A non-empty payload is preserved as-is.
-	 */
-	if (!preg_match('/\$xml_data\s*=\s*\$import_text\s*;/', $content)) {
-		fwrite(STDERR, "$func: \$xml_data = \$import_text assignment missing in $target\n");
-		exit(1);
-	}
-
-	/*
-	 * 4. The file-upload branch must still exist (elseif on $_FILES).
-	 *    Ensures the fallback path was not accidentally removed.
-	 */
-	if (!preg_match('/elseif\s*\(\s*\(\s*\$_FILES\s*\[/', $content)) {
-		fwrite(STDERR, "$func: \$_FILES elseif branch missing in $target\n");
+	if (str_contains($content, "get_nfilter_request_var('import_text')") ||
+		str_contains($content, "\$_FILES['import_file']")) {
+		fwrite(STDERR, "$func: route duplicates shared import payload parsing in $target\n");
 		exit(1);
 	}
 }

tests/regression/issue269_import_text_trim_check_test.php

@@ -1,6 +1,7 @@
 <?php
 
 $root    = dirname(__DIR__, 2);
+$helper  = file_get_contents($root . '/functions.php');
 $targets = [
 	$root . '/syslog_alerts.php',
 	$root . '/syslog_reports.php',
@@ -9,6 +10,32 @@
 
 $legacy = "trim(get_nfilter_request_var('import_text') != '')";
 
+if ($helper === false) {
+	fwrite(STDERR, "Unable to read the shared import helper\n");
+	exit(1);
+}
+
+if (substr_count($helper, 'function syslog_get_import_xml_payload(') !== 1 ||
+	preg_match('/^function syslog_get_import_xml_payload\([^)]*\)\s*\{.*?^\}/ms', $helper, $matches) !== 1) {
+	fwrite(STDERR, "Unable to isolate one shared import helper\n");
+	exit(1);
+}
+
+$helperBody = $matches[0];
+$usesLocal  = str_contains($helperBody, '$import_text = (string) get_nfilter_request_var(\'import_text\')') &&
+	str_contains($helperBody, "trim(\$import_text) !== ''");
+$usesDirect = str_contains($helperBody, "trim(get_nfilter_request_var('import_text')) != ''");
+
+if (!$usesLocal && !$usesDirect) {
+	fwrite(STDERR, "Shared import helper does not preserve the issue #269 trim semantics\n");
+	exit(1);
+}
+
+if (str_contains($helperBody, $legacy)) {
+	fwrite(STDERR, "Legacy import_text trim/comparison bug remains in the shared import helper\n");
+	exit(1);
+}
+
 foreach ($targets as $target) {
 	$content = file_get_contents($target);
 
@@ -22,27 +49,10 @@
 		exit(1);
 	}
 
-	$fixedPattern = '/trim\s*\(\s*\$import_text\s*\)\s*!=\s*\'\'/';
-
-	if (!preg_match($fixedPattern, $content)) {
-		fwrite(STDERR, "Fixed import_text trim/comparison check missing in $target\n");
+	if (substr_count($content, 'syslog_get_import_xml_payload(') !== 1) {
+		fwrite(STDERR, "Shared import payload helper call missing in $target\n");
 		exit(1);
 	}
-
-	/* After the local $import_text assignment, there must be no second
-	   get_nfilter_request_var('import_text') call.  A duplicate call
-	   would bypass the cached local variable. */
-	$needle    = "\$import_text = get_nfilter_request_var('import_text')";
-	$assignPos = strpos($content, $needle);
-
-	if ($assignPos !== false) {
-		$afterAssign = substr($content, $assignPos + strlen($needle));
-
-		if (preg_match('/get_nfilter_request_var\s*\(\s*\'import_text\'\s*\)/', $afterAssign)) {
-			fwrite(STDERR, "Redundant get_nfilter_request_var('import_text') call after local assignment in $target\n");
-			exit(1);
-		}
-	}
 }
 
 print "issue269_import_text_trim_check_test passed\n";

tests/regression/issue277_import_payload_loader_test.php

@@ -31,16 +31,73 @@
 	exit(1);
 }
 
-if (strpos($functions, 'function syslog_get_import_xml_payload(') === false) {
-	fwrite(STDERR, "Shared import payload loader helper is missing.\n");
+if (substr_count($functions, 'function syslog_get_import_xml_payload(') !== 1 ||
+	preg_match('/^function syslog_get_import_xml_payload\([^)]*\)\s*\{.*?^\}/ms', $functions, $matches) !== 1) {
+	fwrite(STDERR, "Unable to isolate one shared import payload loader helper.\n");
 	exit(1);
 }
 
-if (strpos($functions, "trim(get_nfilter_request_var('import_text')) != ''") === false) {
+$helperBody = $matches[0];
+
+if (strpos($helperBody, '$import_text = (string) get_nfilter_request_var(\'import_text\')') === false ||
+	strpos($helperBody, "trim(\$import_text) !== ''") === false) {
 	fwrite(STDERR, "Shared import payload loader is missing trimmed text handling.\n");
 	exit(1);
 }
 
+if (strpos($helperBody, 'return $import_text;') === false) {
+	fwrite(STDERR, "Shared import payload loader must return non-empty textbox input without trimming it.\n");
+	exit(1);
+}
+
+$uploadGuards = [
+	"\$_FILES['import_file']['tmp_name']",
+	"\$_FILES['import_file']['error'] !== UPLOAD_ERR_OK",
+	'is_uploaded_file($tmp_name)',
+	'syslog_read_import_file($tmp_name)',
+];
+
+foreach ($uploadGuards as $guard) {
+	if (strpos($helperBody, $guard) === false) {
+		fwrite(STDERR, "Shared import payload loader is missing upload guard: $guard\n");
+		exit(1);
+	}
+}
+
+$validationPosition = strpos($helperBody, 'is_uploaded_file($tmp_name)');
+$readPosition       = strpos($helperBody, 'syslog_read_import_file($tmp_name)');
+
+if ($validationPosition === false || $readPosition === false || $validationPosition > $readPosition) {
+	fwrite(STDERR, "Shared import payload loader must validate an upload before opening it.\n");
+	exit(1);
+}
+
+require_once $root . '/functions.php';
+
+$emptyFixture   = tempnam(sys_get_temp_dir(), 'syslog-empty-import-');
+$payloadFixture = tempnam(sys_get_temp_dir(), 'syslog-import-');
+
+if ($emptyFixture === false || $payloadFixture === false ||
+	file_put_contents($payloadFixture, '<xml>fixture</xml>') === false) {
+	fwrite(STDERR, "Unable to create import payload fixtures.\n");
+	exit(1);
+}
+
+try {
+	if (syslog_read_import_file($emptyFixture) !== false) {
+		fwrite(STDERR, "A zero-byte import must fail without calling fread() with a zero length.\n");
+		exit(1);
+	}
+
+	if (syslog_read_import_file($payloadFixture) !== '<xml>fixture</xml>') {
+		fwrite(STDERR, "A non-empty import payload must round trip without data loss.\n");
+		exit(1);
+	}
+} finally {
+	unlink($emptyFixture);
+	unlink($payloadFixture);
+}
+
 $targets = [
 	$root . '/syslog_alerts.php',
 	$root . '/syslog_reports.php',