Skip to content

test: add Pest v1 security test infrastructure#328

Open
somethingwithproof wants to merge 1 commit intoCacti:developfrom
somethingwithproof:test/security-test-infrastructure
Open

test: add Pest v1 security test infrastructure#328
somethingwithproof wants to merge 1 commit intoCacti:developfrom
somethingwithproof:test/security-test-infrastructure

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

Separates the Pest security test infrastructure into its own PR as requested.

Copilot AI review requested due to automatic review settings April 11, 2026 23:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a minimal Pest-based security test scaffold for the mactrack plugin, aiming to enforce baseline structural checks, prepared-statement usage consistency, and PHP 7.4 compatibility.

Changes:

  • Introduces Pest bootstrap/config files under tests/ to run plugin-focused checks without loading full Cacti.
  • Adds a setup.php structure test to confirm required plugin hook functions and version keys exist.
  • Adds “security gate” tests for prepared-statement-only DB helper usage and for avoiding select PHP 8.0+ syntax.

Reviewed changes

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

Show a summary per file
File Description
tests/Security/SetupStructureTest.php Scans setup.php source to assert required functions/keys exist.
tests/Security/PreparedStatementConsistencyTest.php Scans selected plugin files to ensure raw db_* calls are not used.
tests/Security/Php74CompatibilityTest.php Scans selected plugin files to forbid a few PHP 8.0+ constructs.
tests/Pest.php Pest entry/config file requiring the test bootstrap.
tests/bootstrap.php Provides stub implementations/constants to support isolated test execution.

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

Comment on lines +15 to +16
$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));

Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

realpath() can return false; passing that into file_get_contents() will raise a warning and leave $source as false, which then makes the expectations behave unpredictably. Consider asserting the file exists/is readable and failing the test with a clear message if it cannot be loaded.

Suggested change
$source = file_get_contents(realpath(__DIR__ . '/../../setup.php'));
$setupFile = __DIR__ . '/../../setup.php';
$setupPath = realpath($setupFile);
expect($setupPath)
->not->toBeFalse("Failed to resolve setup.php path: {$setupFile}");
expect(is_readable($setupPath))
->toBeTrue("setup.php is not readable: {$setupPath}");
$source = file_get_contents($setupPath);
expect($source)
->not->toBeFalse("Failed to read setup.php contents: {$setupPath}");

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +44
if ($path === false) {
continue;
}

$contents = file_get_contents($path);

if ($contents === false) {
continue;
}
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test currently continues when a target file path cannot be resolved (or later when file_get_contents() fails), which means the test can pass while silently skipping files. For a consistency/security gate, it should fail when a listed file is missing or unreadable so regressions can’t be masked.

Suggested change
if ($path === false) {
continue;
}
$contents = file_get_contents($path);
if ($contents === false) {
continue;
}
expect($path)->not->toBeFalse(
"Expected target file {$relativeFile} to exist and resolve to a real path"
);
$contents = file_get_contents($path);
expect($contents)->not->toBeFalse(
"Expected target file {$relativeFile} to be readable"
);

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +28
$targetFiles = array(
'mactrack_devices.php',
'mactrack_device_types.php',
'mactrack_interfaces.php',
'mactrack_sites.php',
'mactrack_snmp.php',
'mactrack_utilities.php',
'mactrack_view_arp.php',
'mactrack_view_macs.php',
'mactrack_view_sites.php',
'setup.php',
);
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, this will fail on the current codebase: several of the listed files contain raw db_execute/db_fetch_* calls (e.g., mactrack_devices.php uses db_execute( and db_fetch_row(, and setup.php uses db_execute(). Either migrate those files before enabling this assertion, or narrow the target list / temporarily skip the test until the migration PR lands.

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +102
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;
}

Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks continue when a file is missing or unreadable, which allows the compatibility tests to pass while skipping files. It would be more reliable to fail with an explicit message when a file in the list can’t be loaded.

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;
}
$loadFileContents = function ($relativeFile) {
$path = realpath(__DIR__ . '/../../' . $relativeFile);
expect($path)->not->toBeFalse("Failed to resolve required file: {$relativeFile}");
$contents = file_get_contents($path);
expect($contents)->not->toBeFalse("Failed to read required file: {$relativeFile}");
return $contents;
};
it('does not use str_contains (PHP 8.0)', function () use ($files, $loadFileContents) {
foreach ($files as $relativeFile) {
$contents = $loadFileContents($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, $loadFileContents) {
foreach ($files as $relativeFile) {
$contents = $loadFileContents($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, $loadFileContents) {
foreach ($files as $relativeFile) {
$contents = $loadFileContents($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, $loadFileContents) {
foreach ($files as $relativeFile) {
$contents = $loadFileContents($relativeFile);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants