Skip to content

hardening: run the test suite, and add CodeQL - #229

Open
somethingwithproof wants to merge 12 commits into
Cacti:developfrom
somethingwithproof:hardening/testing-and-workflows
Open

hardening: run the test suite, and add CodeQL#229
somethingwithproof wants to merge 12 commits into
Cacti:developfrom
somethingwithproof:hardening/testing-and-workflows

Conversation

@somethingwithproof

Copy link
Copy Markdown
Member

Stacked on #225, which supplies the workflow modernisation, the SHA pinning and the permissions: block. This adds only what is missing from it.

The gap this closes

The plugin shipped three test files that nothing ran. The workflow contained no reference to tests/ at all, so a suite covering request-variable output escaping sat unexecuted since it was written.

Testing

Adopts the pattern the other Cacti plugins already use: Pest through composer, as in plugin_thold and plugin_mactrack, with a tests/bootstrap.php that stubs the Cacti framework so plugin code loads without a full install.

Two of the three existing files were meaningful grep guards against raw request reuse and are kept as Pest datasets. The third only asserted that htmlspecialchars() works and is dropped.

Pest 2 rather than mactrack's Pest 1, which no longer runs on current PHP: it fails on 8.4 with implicit-nullable deprecations.

There is no config.platform pin. It would hold the dev tooling to the plugin's PHP 8.0 runtime floor, which Pest 2 cannot meet. The floor is enforced by the new syntax-floor job instead, which is where it belongs. composer.lock is not committed, matching thold.

What this adds to #225

  • unit-test, running composer test; ci: pin integration tests to Cacti 1.2.31 #225 has no test job
  • syntax-floor, linting every file at PHP 8.0, the floor compat = 1.2.15 implies. The existing matrix starts at 8.1 and cannot catch syntax the declared minimum rejects
  • codeql.yml, matching plugin_audit and plugin_syslog. It analyses javascript-typescript only, which is correct rather than an oversight: CodeQL has no PHP analyser, so the PHP is covered by the syntax and integration jobs
  • composer install --no-plugins --no-scripts, since the tree carries no lockfile

Dependabot is deliberately absent: #226 covers it.

What these tests do not do

The escaping tests match source text rather than running the pages. They detect an html_escape call being deleted; they prove nothing about runtime behaviour. The pages chdir() to the Cacti root and include auth.php, so exercising them needs a live install. That limitation is stated in the file rather than left for a reader to find.

Verification

$ composer test
Tests:    15 passed (16 assertions)

$ find . -path ./vendor -prune -o -name '*.php' -print | xargs -n1 php -l
(no errors at the PHP 8.0 floor)

PHPStan reports 209 errors on the two files this touches, both before and after; no new findings are introduced.

somethingwithproof and others added 12 commits July 12, 2026 13:54
Pin actions/checkout (v5), shivammathur/setup-php (v2) and actions/upload-artifact (v4) to full commit SHAs with a trailing # vX comment. Mutable tags let a compromised or force-pushed tag run arbitrary code in CI; pinning to an immutable SHA is the supply-chain-hardening best practice and matches the SHA-pinned checkout used in the core security workflow.
Keep 1.2.31 pin while covering mid-stream PHP releases.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The plugin shipped three test files that nothing ran: the workflow had no
reference to tests/ at all, so a suite covering request-variable output
escaping sat unexecuted.

Adopts the Pest-through-composer pattern from thold and mactrack, with a
bootstrap that stubs Cacti so plugin code loads without a full install. Two of
the three existing files were meaningful grep guards against raw request reuse
and are kept as Pest datasets; the third only asserted that htmlspecialchars()
works.

Pest 2 rather than mactrack's Pest 1, which no longer runs on current PHP. The
PHP 8.0 runtime floor is enforced by the syntax-floor job rather than a
composer platform pin, since the dev tooling needs 8.1.

Stacked on Cacti#225, so the SHA pinning and permissions block are inherited rather
than duplicated. Dependabot is left to Cacti#226.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copilot AI lite review requested due to automatic review settings August 17, 2026 05:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the plugin’s CI posture by introducing an executable PHP test suite (via Pest + a lightweight Cacti bootstrap stub), adding a PHP 8.0 syntax-floor job to enforce the declared minimum runtime compatibility, and enabling CodeQL scanning for the plugin’s JavaScript/TypeScript.

Changes:

  • Replace the previously standalone/CLI-style test scripts under tests/ with a Pest-based suite and a tests/bootstrap.php Cacti stub so plugin code can load without a full Cacti install.
  • Extend the existing CI workflow to run composer test and add a separate PHP 8.0 syntax lint job.
  • Add a CodeQL workflow (JS/TS only) for scheduled and PR scanning.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unit/test_request_output_escaping.php Removes the old standalone escaping check script (superseded by Pest coverage).
tests/Pest.php Adds global Pest hooks to reset request/settings/sql test globals before each test.
tests/Integration/test_monitor_request_output_wiring.php Removes old grep-guard style script (replaced by Pest datasets).
tests/Integration/OutputEscapingTest.php Adds Pest datasets asserting request output escaping and preventing raw request concatenation regressions.
tests/e2e/test_monitor_no_raw_request_reuse.php Removes old standalone “raw request reuse” script (replaced by Pest datasets).
tests/bootstrap.php Adds Cacti framework stubs for test execution without a full Cacti install.
phpunit.xml.dist Adds PHPUnit/Pest bootstrap configuration targeting the Integration test directory.
composer.json Introduces Composer metadata and Pest as a dev dependency with composer test scripts.
.gitignore Ignores composer artifacts/caches related to the new test tooling.
.github/workflows/plugin-ci-workflow.yml Adds a unit-test job and a PHP 8.0 syntax-floor job; refactors parts of integration job setup.
.github/workflows/codeql.yml Adds CodeQL analysis workflow for JavaScript/TypeScript.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +45 to +53
permissions:
contents: read

concurrency:
group: monitor-ci-${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read
Comment on lines 109 to 112
services:
mariadb:
image: mariadb:10.6
image: mariadb:10.11
env:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants