test: add Pest v1 security test infrastructure by somethingwithproof · Pull Request #154 · Cacti/plugin_routerconfigs

somethingwithproof · 2026-04-09T06:56:41Z

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 test scaffold to the routerconfigs Cacti plugin repo, focused on “security hardening” style source scans and basic structure checks for setup.php.

Changes:

Add Pest bootstrap/configuration plus Cacti framework stubs for isolated test execution.
Add security-oriented source-scan tests for prepared-statement usage and PHP 7.4 compatibility patterns.
Add a minimal composer.json with Pest as a dev dependency.

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`	Introduces Pest v1 as a dev dependency and configures dev autoload for the test bootstrap.
`tests/Pest.php`	Pest entry/config file that loads the test bootstrap.
`tests/bootstrap.php`	Provides stub implementations of common Cacti functions/constants for test isolation.
`tests/Security/SetupStructureTest.php`	Adds a setup.php “structure” scan test (functions + version metadata presence).
`tests/Security/PreparedStatementConsistencyTest.php`	Adds a scan test intended to enforce use of prepared DB helper calls.
`tests/Security/Php74CompatibilityTest.php`	Adds a scan test intended to flag obvious PHP 8+ usage in selected files.

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

+	it('uses prepared DB helpers in all plugin files', function () {
+		$targetFiles = array(
+		'router-accounts.php',
+		'router-backups.php',
+		'router-compare.php',
+		'router-devices.php',
+		'routerconfigs.php',
+		'setup.php',
+		);


This test will fail against the current codebase because several of the listed target files contain raw db_execute/db_fetch_* calls (e.g., router-accounts.php uses db_execute(...) and db_fetch_cell(...), setup.php uses db_execute(...) for upgrades). Either update the plugin sources to meet this requirement, or narrow the assertion (e.g., only enforce prepared calls in specific query helpers / user-input paths, or exclude setup.php/DDL and other legacy files until migrated).

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

+			$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)) {


The scan logic for “outside comments” is too weak: it skips lines starting with "" but not block comment starts like "/" (or inline block comments), so commented-out code can be incorrectly counted as raw DB usage. Consider using token_get_all() to ignore T_COMMENT/T_DOC_COMMENT (and strings), or at least skip lines beginning with "/*" and track multi-line 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)) {

$tokens = token_get_all($contents);

$rawCallsOutsideComments = 0;

foreach ($tokens as $token) {

if (is_array($token)) {

if ($token[0] === T_COMMENT || $token[0] === T_DOC_COMMENT || $token[0] === T_CONSTANT_ENCAPSED_STRING) {

continue;

}

$text = $token[1];

} else {

$text = $token;

}

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

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

+	it('returns version array with name key', function () use ($source) {
+		expect($source)->toMatch('/[\'\""]name[\'\""]\s*=>/');
+	});
+
+	it('returns version array with version key', function () use ($source) {
+		expect($source)->toMatch('/[\'\""]version[\'\""]\s*=>/');
+	});


These assertions don’t actually validate the plugin version info: setup.php contains many occurrences of "'name' =>" and "'version' =>" in DB table definition arrays, so the regexes will pass even if plugin_routerconfigs_version() stops returning name/version metadata. If the intent is to validate version info, consider checking the INFO file directly, or loading setup.php and asserting plugin_routerconfigs_version() returns an array containing required keys (with a minimal $config stub).

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

+	$files = array(
+		'router-accounts.php',
+		'router-backups.php',
+		'router-compare.php',
+		'router-devices.php',
+		'routerconfigs.php',
+		'setup.php',
+	);
+


The test description claims the plugin must be PHP 7.4-compatible, but this suite only scans a small subset of files and misses PHP 8-only syntax already present elsewhere in the plugin (e.g., #[AllowDynamicProperties] in classes/PHPConnection.php). Either expand the scan to include classes/ and include/ (or auto-discover PHP files), or adjust the test name/docs so it doesn’t imply full PHP 7.4 compatibility coverage.

Suggested change

$files = array(

'router-accounts.php',

'router-backups.php',

'router-compare.php',

'router-devices.php',

'routerconfigs.php',

'setup.php',

);

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

$files = array();

if ($root !== false) {

$iterator = new RegexIterator(

new RecursiveIteratorIterator(

new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS)

),

'/^.+\.php$/i',

RegexIterator::GET_MATCH

);

foreach ($iterator as $matches) {

$path = $matches[0];

$relativeFile = str_replace('\\', '/', substr($path, strlen($root) + 1));

if (preg_match('#^tests/#', $relativeFile)) {

continue;

}

$files[] = $relativeFile;

}

sort($files);

}

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

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


The test silently skips files when realpath()/file_get_contents() fails. This can mask coverage gaps (e.g., the list includes "routerconfigs.php", which doesn’t exist in the repo and will be skipped). Consider failing the test when a listed file can’t be read, or auto-discovering PHP files under the plugin root instead of maintaining a hard-coded list.

Suggested change

continue;

}

$contents = file_get_contents($path);

if ($contents === false) {

continue;

throw new RuntimeException("Unable to resolve listed plugin file: {$relativeFile}");

}

$contents = file_get_contents($path);

if ($contents === false) {

throw new RuntimeException("Unable to read listed plugin file: {$relativeFile}");

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

+	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;
+			}
+


Like the prepared-statement scan, this test silently skips files if realpath()/file_get_contents() fails, which can hide missing/renamed files and reduce coverage. If the goal is compatibility enforcement, consider failing when a listed file can’t be read, or auto-discovering all PHP files under the plugin (e.g., include/, classes/, router-*.php, setup.php).

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;

}

$readFile = function ($relativeFile) {

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

if ($path === false) {

throw new RuntimeException("Unable to resolve plugin source file: {$relativeFile}");

}

$contents = file_get_contents($path);

if ($contents === false) {

throw new RuntimeException("Unable to read plugin source file: {$relativeFile}");

}

return $contents;

};

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

foreach ($files as $relativeFile) {

$contents = $readFile($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, $readFile) {

foreach ($files as $relativeFile) {

$contents = $readFile($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, $readFile) {

foreach ($files as $relativeFile) {

$contents = $readFile($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, $readFile) {

foreach ($files as $relativeFile) {

$contents = $readFile($relativeFile);

…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>

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

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

Converted to draft to serialize the stack in this repo. Blocked by #153; 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 AI reviewed Apr 9, 2026

View reviewed changes

somethingwithproof added 2 commits April 9, 2026 23:01

fix(security): pass allowed_classes to unserialize

cd0699f

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

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#154

test: add Pest v1 security test infrastructure#154
somethingwithproof wants to merge 3 commits into
Cacti: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

-	$files = array(
-		'router-accounts.php',
-		'router-backups.php',
-		'router-compare.php',
-		'router-devices.php',
-		'routerconfigs.php',
-		'setup.php',
-	);
+	$root = realpath(__DIR__ . '/../../');
+	$files = array();
+	if ($root !== false) {
+		$iterator = new RegexIterator(
+			new RecursiveIteratorIterator(
+				new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS)
+			),
+			'/^.+\.php$/i',
+			RegexIterator::GET_MATCH
+		);
+		foreach ($iterator as $matches) {
+			$path = $matches[0];
+			$relativeFile = str_replace('\\', '/', substr($path, strlen($root) + 1));
+			if (preg_match('#^tests/#', $relativeFile)) {
+				continue;
+			}
+			$files[] = $relativeFile;
+		}
+		sort($files);
+	}

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