test: add Pest v1 security test infrastructure by somethingwithproof · Pull Request #93 · Cacti/plugin_mikrotik

somethingwithproof · 2026-04-09T06:55:58Z

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-based security testing scaffold for the Cacti MikroTik plugin to validate setup structure, discourage raw DB helper usage, and enforce PHP 7.4 compatibility constraints.

Changes:

Introduces Pest bootstrap/configuration and a dev Composer setup to run the test suite.
Adds security-oriented source-scan tests for setup.php structure and prepared-statement helper usage.
Adds source-scan checks to prevent use of select PHP 8-only APIs/syntax in key plugin files.

Reviewed changes

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

Show a summary per file

File	Description
composer.json	Adds Pest v1 as a dev dependency and wires test bootstrap via autoload-dev.
tests/Pest.php	Pest entry/config that loads the test bootstrap.
tests/bootstrap.php	Provides Cacti framework stubs/constants so tests can run in isolation.
tests/Security/SetupStructureTest.php	Verifies expected plugin hook functions and version array keys exist in `setup.php`.
tests/Security/PreparedStatementConsistencyTest.php	Scans key plugin files to forbid raw `db_execute`/`db_fetch_*` usage.
tests/Security/Php74CompatibilityTest.php	Scans for select PHP 8-only APIs/syntax (e.g., `str_contains`, nullsafe operator).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T06:59:59Z

+	it('uses prepared DB helpers in all plugin files', function () {
+		$targetFiles = array(
+		'mikrotik.php',
+		'mikrotik_users.php',
+		'poller_graphs.php',
+		'poller_mikrotik.php',
+		'setup.php',
+		);


This test currently enforces that the listed plugin files contain zero raw db_execute/db_fetch_* calls, but the repository still has many such calls (e.g., poller_mikrotik.php contains multiple db_execute() usages). As written, the new test suite will fail immediately; either migrate these files to *_prepared in the same PR, or narrow/phase the assertion (e.g., only enforce on already-migrated files or mark the check as skipped until the hardening changes land).

Copilot · 2026-04-09T06:59:59Z

+			$lines = explode("\n", $contents);
+			$rawCallsOutsideComments = 0;
+
+			foreach ($lines as $line) {
+				$trimmed = ltrim($line);
+
+				if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0 || strpos($trimmed, '#') === 0) {
+					continue;
+				}
+
+				if (preg_match($rawPattern, $line) && !preg_match($preparedPattern, $line)) {
+					$rawCallsOutsideComments++;
+				}
+			}
+


The scan skips only lines that start with //, * or #, but it does not treat lines starting with "/" (or inline comments / block-comment bodies) as comments. That can lead to false positives/negatives. Consider using token_get_all() to ignore comments/strings robustly, or at least expand the skip logic to handle "/" and block-comment state.

Suggested change

$lines = explode("\n", $contents);

$rawCallsOutsideComments = 0;

foreach ($lines as $line) {

$trimmed = ltrim($line);

if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0 || strpos($trimmed, '#') === 0) {

continue;

}

if (preg_match($rawPattern, $line) && !preg_match($preparedPattern, $line)) {

$rawCallsOutsideComments++;

}

}

$code = '';

foreach (token_get_all($contents) as $token) {

if (is_array($token)) {

$tokenId = $token[0];

$tokenText = $token[1];

if (

$tokenId === T_COMMENT ||

$tokenId === T_DOC_COMMENT ||

$tokenId === T_CONSTANT_ENCAPSED_STRING ||

$tokenId === T_ENCAPSED_AND_WHITESPACE

) {

$code .= str_repeat(' ', strlen($tokenText));

continue;

}

$code .= $tokenText;

continue;

}

$code .= $token;

}

preg_match_all($rawPattern, $code, $rawMatches);

$rawCallsOutsideComments = count($rawMatches[0]);

Copilot · 2026-04-09T06:59:59Z

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


If realpath() fails (or setup.php is unreadable), file_get_contents() will emit a warning and $source will be false, which can make the failure mode noisy/unclear. Consider asserting the resolved path exists and that file_get_contents() returned a string before running the structural expectations, so the test fails with a clear message.

Suggested change

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

$setupFile = __DIR__ . '/../../setup.php';

$setupPath = realpath($setupFile);

if ($setupPath === false || !is_file($setupPath) || !is_readable($setupPath)) {

throw new RuntimeException("Unable to resolve readable setup.php at {$setupFile}");

}

$source = file_get_contents($setupPath);

if (!is_string($source)) {

throw new RuntimeException("Unable to read setup.php contents from {$setupPath}");

}

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

+	it('does not use str_contains (PHP 8.0)', function () use ($files) {
+		foreach ($files as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+
+			if ($path === false) {
+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;
+			}
+
+			expect(preg_match('/\bstr_contains\s*\(/', $contents))->toBe(0,
+				"{$relativeFile} uses str_contains() which requires PHP 8.0"
+			);
+		}
+	});
+
+	it('does not use str_starts_with (PHP 8.0)', function () use ($files) {
+		foreach ($files as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+
+			if ($path === false) {
+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;
+			}
+
+			expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0,
+				"{$relativeFile} uses str_starts_with() which requires PHP 8.0"
+			);
+		}
+	});
+
+	it('does not use str_ends_with (PHP 8.0)', function () use ($files) {
+		foreach ($files as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+
+			if ($path === false) {
+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;
+			}
+
+			expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0,
+				"{$relativeFile} uses str_ends_with() which requires PHP 8.0"
+			);
+		}
+	});
+
+	it('does not use nullsafe operator (PHP 8.0)', function () use ($files) {
+		foreach ($files as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+
+			if ($path === false) {
+				continue;
+			}
+
+			$contents = file_get_contents($path);
+
+			if ($contents === false) {
+				continue;
+			}
+


These checks silently continue when a target file can’t be resolved/read. That can make the suite pass while skipping the intended verification (e.g., wrong relative paths or missing files). Consider failing the test when a listed file is missing/unreadable so the compatibility guarantee is enforced.

Suggested change

it('does not use str_contains (PHP 8.0)', function () use ($files) {

foreach ($files as $relativeFile) {

$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

}

expect(preg_match('/\bstr_contains\s*\(/', $contents))->toBe(0,

"{$relativeFile} uses str_contains() which requires PHP 8.0"

);

}

});

it('does not use str_starts_with (PHP 8.0)', function () use ($files) {

foreach ($files as $relativeFile) {

$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

}

expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0,

"{$relativeFile} uses str_starts_with() which requires PHP 8.0"

);

}

});

it('does not use str_ends_with (PHP 8.0)', function () use ($files) {

foreach ($files as $relativeFile) {

$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

}

expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0,

"{$relativeFile} uses str_ends_with() which requires PHP 8.0"

);

}

});

it('does not use nullsafe operator (PHP 8.0)', function () use ($files) {

foreach ($files as $relativeFile) {

$path = realpath(__DIR__ . '/../../' . $relativeFile);

if ($path === false) {

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

}

$readFileContents = function (string $relativeFile): string {

$path = realpath(__DIR__ . '/../../' . $relativeFile);

expect($path)->not->toBeFalse(

"Unable to resolve compatibility test target file: {$relativeFile}"

);

$contents = file_get_contents($path);

expect($contents)->not->toBeFalse(

"Unable to read compatibility test target file: {$relativeFile}"

);

return $contents;

};

it('does not use str_contains (PHP 8.0)', function () use ($files, $readFileContents) {

foreach ($files as $relativeFile) {

$contents = $readFileContents($relativeFile);

expect(preg_match('/\bstr_contains\s*\(/', $contents))->toBe(0,

"{$relativeFile} uses str_contains() which requires PHP 8.0"

);

}

});

it('does not use str_starts_with (PHP 8.0)', function () use ($files, $readFileContents) {

foreach ($files as $relativeFile) {

$contents = $readFileContents($relativeFile);

expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0,

"{$relativeFile} uses str_starts_with() which requires PHP 8.0"

);

}

});

it('does not use str_ends_with (PHP 8.0)', function () use ($files, $readFileContents) {

foreach ($files as $relativeFile) {

$contents = $readFileContents($relativeFile);

expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0,

"{$relativeFile} uses str_ends_with() which requires PHP 8.0"

);

}

});

it('does not use nullsafe operator (PHP 8.0)', function () use ($files, $readFileContents) {

foreach ($files as $relativeFile) {

$contents = $readFileContents($relativeFile);

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

+}
+
+if (!defined('CACTI_PATH_BASE')) {
+	define('CACTI_PATH_BASE', '/var/www/html/cacti');


CACTI_PATH_BASE is stubbed to a hard-coded filesystem path (/var/www/html/cacti). If any tests later load plugin code that uses CACTI_PATH_BASE for includes/paths, this will break on most dev/CI environments. Prefer defining it relative to the repository root (e.g., realpath(DIR . '/..')).

Suggested change

define('CACTI_PATH_BASE', '/var/www/html/cacti');

$cacti_path_base = realpath(__DIR__ . '/..');

define('CACTI_PATH_BASE', $cacti_path_base !== false ? $cacti_path_base : dirname(__DIR__));

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

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


This test continues when a file path can’t be resolved or read, which can let the suite pass without checking anything (e.g., if paths change). Consider making missing/unreadable target files a hard failure so the security regression check remains meaningful.

Suggested change

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

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

}

$contents = file_get_contents($path);

if ($contents === false) {

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

…dabot - Throw RuntimeException when realpath/file_get_contents fails (previously silent continue hid unscanned files) - Fix Dependabot ecosystem from npm to composer - Remove committed .omc session artifacts, add .omc/ to .gitignore Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof · 2026-04-11T00:10:07Z

Converted to draft to serialize the stack in this repo. Blocked by #90; will un-draft after that merges to avoid cross-PR merge conflicts.

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

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

Copilot AI reviewed Apr 9, 2026

View reviewed changes

somethingwithproof marked this pull request as draft April 11, 2026 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add Pest v1 security test infrastructure#93

test: add Pest v1 security test infrastructure#93
somethingwithproof wants to merge 2 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!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-			$lines = explode("\n", $contents);
-			$rawCallsOutsideComments = 0;
-			foreach ($lines as $line) {
-				$trimmed = ltrim($line);
-				if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0 || strpos($trimmed, '#') === 0) {
-					continue;
-				}
-				if (preg_match($rawPattern, $line) && !preg_match($preparedPattern, $line)) {
-					$rawCallsOutsideComments++;
-				}
-			}
+			$code = '';
+			foreach (token_get_all($contents) as $token) {
+				if (is_array($token)) {
+					$tokenId = $token[0];
+					$tokenText = $token[1];
+					if (
+						$tokenId === T_COMMENT ||
+						$tokenId === T_DOC_COMMENT ||
+						$tokenId === T_CONSTANT_ENCAPSED_STRING ||
+						$tokenId === T_ENCAPSED_AND_WHITESPACE
+					) {
+						$code .= str_repeat(' ', strlen($tokenText));
+						continue;
+					}
+					$code .= $tokenText;
+					continue;
+				}
+				$code .= $token;
+			}
+			preg_match_all($rawPattern, $code, $rawMatches);
+			$rawCallsOutsideComments = count($rawMatches[0]);

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

-	$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
+	$setupFile = __DIR__ . '/../../setup.php';
+	$setupPath = realpath($setupFile);
+	if ($setupPath === false || !is_file($setupPath) || !is_readable($setupPath)) {
+		throw new RuntimeException("Unable to resolve readable setup.php at {$setupFile}");
+	}
+	$source = file_get_contents($setupPath);
+	if (!is_string($source)) {
+		throw new RuntimeException("Unable to read setup.php contents from {$setupPath}");
+	}

	define('CACTI_PATH_BASE', '/var/www/html/cacti');
	$cacti_path_base = realpath(__DIR__ . '/..');
	define('CACTI_PATH_BASE', $cacti_path_base !== false ? $cacti_path_base : dirname(__DIR__));

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!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants