test: add Pest v1 security test infrastructure by somethingwithproof · Pull Request #305 · Cacti/plugin_syslog

somethingwithproof · 2026-04-09T06:57:01Z

Summary

Add Pest v1 test scaffold with Cacti framework stubs
Source-scan tests for prepared statement consistency
PHP 7.4 compatibility verification tests
Plugin setup.php structure validation

Test plan

composer install && vendor/bin/pest passes
Tests verify security patterns match hardening PRs

Add source-scan tests verifying security patterns (prepared statements, output escaping, auth guards, PHP 7.4 compatibility) remain in place across refactors. Tests run with Pest v1 (PHP 7.3+) and stub the Cacti framework so plugins can be tested in isolation. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot

Pull request overview

Adds a Pest v1 test scaffold and lightweight Cacti stubs to enable security/compatibility checks against the syslog plugin codebase.

Changes:

Introduces Pest bootstrap/config and several source-scanning security/compatibility tests.
Adds a composer.json with Pest as a dev dependency and test bootstrap autoloading.
Adds a CodeQL workflow configuration.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`tests/Security/SetupStructureTest.php`	Verifies `setup.php` defines expected plugin entrypoints and version keys.
`tests/Security/PreparedStatementConsistencyTest.php`	Scans key plugin files for raw `db_*` calls to prevent regressions.
`tests/Security/Php74CompatibilityTest.php`	Scans plugin files for PHP 8-only syntax/function usage.
`tests/Pest.php`	Loads the test bootstrap for Pest runs.
`tests/bootstrap.php`	Provides Cacti framework stubs for running tests without full Cacti.
`composer.json`	Adds Pest v1 as a dev dependency and wires test bootstrap autoloading.
`.github/workflows/codeql.yml`	Adds CodeQL workflow definition.

Copilot · 2026-04-09T07:01:04Z

tests/Security/PreparedStatementConsistencyTest.php

+	it('uses prepared DB helpers in all plugin files', function () {
+		$targetFiles = array(
+		'database.php',
+		'functions.php',
+		'setup.php',
+		'syslog.php',
+		'syslog_alerts.php',
+		'syslog_process.php',
+		'syslog_removal.php',
+		'syslog_reports.php',
+		);


This test will flag legitimate db_* calls that must exist in this repo, so it’s likely to fail immediately (e.g., database.php contains return db_execute(...) inside the syslog_db_* wrappers, and setup.php has db_execute(...) calls for plugin metadata cleanup). Consider excluding database.php (and/or known-safe sections) from scanning, or tightening the check to only prohibit raw db_* usage outside the syslog DB wrapper layer (e.g., enforce syslog_db_* usage in non-wrapper files).

Copilot · 2026-04-09T07:01:04Z

tests/Security/PreparedStatementConsistencyTest.php

+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;


Silently continue-ing when a target file is missing or unreadable makes this test pass without actually validating anything. Prefer failing the test with a clear message when realpath() or file_get_contents() fails so CI can catch moved/renamed files or unexpected read errors.

Suggested change

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

throw new RuntimeException("Unable to resolve target file for prepared statement consistency test: {$relativeFile}");

}

$contents = file_get_contents($path);

if ($contents === false) {

throw new RuntimeException("Unable to read target file for prepared statement consistency test: {$path}");

Copilot · 2026-04-09T07:01:04Z

tests/bootstrap.php

+	function db_execute($sql) {
+		$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array());


The db_execute stub signature doesn’t match how the plugin calls it (the syslog DB wrappers call db_execute($sql, $log, $syslog_cnn)), which can cause ArgumentCountError if any test loads plugin code. Update the stub to accept the additional optional parameters (log flag / connection) to mirror Cacti’s function signature.

Suggested change

function db_execute($sql) {

$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array());

function db_execute($sql, $log = true, $cnn = false) {

$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array(), 'log' => $log, 'cnn' => $cnn);

Copilot · 2026-04-09T07:01:05Z

tests/bootstrap.php

+}
+
+if (!function_exists('db_execute_prepared')) {
+	function db_execute_prepared($sql, $params = array()) {


Similarly, db_execute_prepared is called by the plugin with additional parameters (e.g. $log, $syslog_cnn). The stub should accept those optional arguments to avoid runtime errors when plugin code is executed during tests.

Suggested change

function db_execute_prepared($sql, $params = array()) {

function db_execute_prepared($sql, $params = array(), $log = true, $cnn = null) {

Copilot · 2026-04-09T07:01:05Z

tests/bootstrap.php

+if (!function_exists('db_fetch_assoc')) {
+	function db_fetch_assoc($sql) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_assoc_prepared')) {
+	function db_fetch_assoc_prepared($sql, $params = array()) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_row')) {
+	function db_fetch_row($sql) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_row_prepared')) {
+	function db_fetch_row_prepared($sql, $params = array()) {
+		return array();
+	}
+}
+
+if (!function_exists('db_fetch_cell')) {
+	function db_fetch_cell($sql) {
+		return '';
+	}
+}
+
+if (!function_exists('db_fetch_cell_prepared')) {
+	function db_fetch_cell_prepared($sql, $params = array()) {
+		return '';
+	}
+}


The db_fetch_* stubs accept fewer parameters than the plugin’s wrappers pass through (e.g. db_fetch_cell($sql, $col_name, $log, $cnn) and db_fetch_assoc($sql, $log, $cnn)). If any test includes code paths that invoke these wrappers, this will fatally error; consider updating these stubs to accept the optional $log/$cnn parameters (and $col_name where applicable).

Copilot · 2026-04-09T07:01:05Z

tests/Security/SetupStructureTest.php

+	$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
+


If setup.php can’t be resolved/read (e.g., path changes), $source becomes false and subsequent string expectations will behave unpredictably and/or emit warnings. Consider asserting that realpath() and file_get_contents() succeeded (fail fast with a clear message) before running toContain / toMatch expectations.

Suggested change

$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

$setupPath = realpath(__DIR__ . '/../../setup.php');

expect($setupPath)

->not->toBeFalse('Failed to resolve setup.php path from ' . __DIR__);

$source = file_get_contents($setupPath);

expect($source)

->not->toBeFalse('Failed to read setup.php at path: ' . $setupPath);

Copilot · 2026-04-09T07:01:05Z

tests/Security/Php74CompatibilityTest.php

+		foreach ($files as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+
+			if ($path === false) {
+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;
+			}
+


These tests silently skip files when realpath() or file_get_contents() fails, which can lead to false-green results. Prefer failing with an explicit assertion/message when a target file is missing/unreadable so the compatibility gate remains meaningful over time.

Copilot · 2026-04-09T07:01:06Z

.github/workflows/codeql.yml

+  push:
+    branches: [main, master, develop, regression-audit]
+    paths-ignore:
+      - "**/*.php"
+      - "**/*.md"
+  pull_request:
+    branches: [main, master, develop, regression-audit]
+    paths-ignore:
+      - "**/*.php"
+      - "**/*.md"


paths-ignore excludes all **/*.php, so CodeQL won’t run on the vast majority of changes in this PHP plugin. If the intent is to security-scan the plugin, remove the PHP ignore (and consider ignoring only vendor/ or generated paths instead).

Copilot · 2026-04-09T07:01:06Z

.github/workflows/codeql.yml

+    strategy:
+      fail-fast: false
+      matrix:
+        language: ["javascript-typescript", "python", "ruby"]
+    steps:


The CodeQL language matrix doesn’t include php, so even when the workflow runs it won’t analyze the primary language in this repository. Add PHP to the matrix (or switch to a single-language config) so CodeQL produces useful results for this codebase.

TheWitness · 2026-04-10T01:06:58Z

Closing this one. We can discuss on Sunday.

somethingwithproof added 3 commits March 15, 2026 19:06

ci: add CodeQL for javascript-typescript, python, ruby

9603b08

fix(ci): use specific runner labels, fix codeql concurrency

743ba74

Copilot AI review requested due to automatic review settings April 9, 2026 06:57

Copilot started reviewing on behalf of somethingwithproof April 9, 2026 06:57 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

TheWitness closed this Apr 10, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add Pest v1 security test infrastructure#305

test: add Pest v1 security test infrastructure#305
somethingwithproof wants to merge 3 commits intoCacti:developfrom
somethingwithproof:test/add-security-test-infrastructure

somethingwithproof commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

TheWitness commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		function db_execute($sql) {
		$GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute', 'sql' => $sql, 'params' => array());

	function db_execute_prepared($sql, $params = array()) {
	function db_execute_prepared($sql, $params = array(), $log = true, $cnn = null) {

		$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

-	$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
+	$setupPath = realpath(__DIR__ . '/../../setup.php');
+	expect($setupPath)
+		->not->toBeFalse('Failed to resolve setup.php path from ' . __DIR__);
+	$source = file_get_contents($setupPath);
+	expect($source)
+		->not->toBeFalse('Failed to read setup.php at path: ' . $setupPath);

Conversation

somethingwithproof commented Apr 9, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

TheWitness commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants