Skip to content

ci: run the test suite, and add Dependabot and CodeQL - #228

Closed
somethingwithproof wants to merge 1 commit into
Cacti:developfrom
somethingwithproof:ci/testing-and-workflows
Closed

ci: run the test suite, and add Dependabot and CodeQL#228
somethingwithproof wants to merge 1 commit into
Cacti:developfrom
somethingwithproof:ci/testing-and-workflows

Conversation

@somethingwithproof

Copy link
Copy Markdown
Member

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.

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.

CI

  • unit-test runs composer test
  • syntax-floor lints 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
  • permissions: contents: read at workflow scope, which was absent
  • composer install --no-plugins --no-scripts, since the tree has no lockfile
  • every action pinned to a commit SHA resolved from the API rather than a tag

Adds dependabot.yml and codeql.yml to match the sibling plugins. CodeQL analyses javascript-typescript only, as in plugin_audit and plugin_syslog: it has no PHP analyser, so for a Cacti plugin it covers the JavaScript and the PHP is covered by the syntax and integration jobs.

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 discover.

$ composer test
Tests:    15 passed (16 assertions)

The plugin shipped three test files that nothing ran: the workflow contained no
reference to tests/ at all. Two were meaningful grep guards against raw request
reuse and are kept as Pest datasets; the third only asserted that
htmlspecialchars() works.

Adopts the thold and mactrack pattern of Pest via composer, with a bootstrap
that stubs Cacti so plugin code loads without a full install.

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

Also adds Dependabot and CodeQL to match the sibling plugins, and pins every
action to a commit SHA.

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

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

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 updates the plugin’s automation and security posture by introducing a Composer/Pest-based test suite (with a lightweight Cacti stub bootstrap) and expanding GitHub-side automation (CI job coverage, Dependabot, and CodeQL) so the shipped escaping-related tests are actually executed in CI.

Changes:

  • Adds a Pest test suite under tests/ with a tests/bootstrap.php stub to load plugin code without a full Cacti install, and migrates the prior grep-guard tests into Pest datasets.
  • Extends CI to run the new test suite (composer test) and adds a PHP 8.0 “syntax floor” lint job to enforce the declared compatibility minimum.
  • Adds GitHub-native automation: CodeQL scanning (JS/TS) and Dependabot updates for GitHub Actions.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/unit/test_request_output_escaping.php Removes the prior standalone CLI escaping check (replaced by Pest).
tests/Integration/test_monitor_request_output_wiring.php Removes standalone integration grep-guard (replaced by Pest datasets).
tests/e2e/test_monitor_no_raw_request_reuse.php Removes standalone e2e grep-guard (replaced by Pest datasets).
tests/Pest.php Adds global Pest hooks to reset stubbed globals and request state per test.
tests/Integration/OutputEscapingTest.php Adds Pest datasets/assertions to guard request output escaping and prevent raw request reuse in markup.
tests/bootstrap.php Adds Cacti framework stubs for test execution without a live install.
phpunit.xml.dist Configures PHPUnit/Pest bootstrap and test discovery for the Integration suite.
composer.json Introduces Composer scripts and Pest dependency to run the test suite in CI.
.gitignore Ignores Composer lockfile and PHPUnit cache artifacts (and continues ignoring vendor/).
.github/workflows/plugin-ci-workflow.yml Adds unit-test and syntax-floor jobs; pins actions by SHA; adds workflow permissions.
.github/workflows/codeql.yml Adds CodeQL workflow configured for JS/TS analysis with pinned actions and minimal permissions.
.github/dependabot.yml Adds Dependabot configuration for GitHub Actions updates.
Suppressed comments (1)

tests/Integration/OutputEscapingTest.php:42

  • Same as above: guard against file_get_contents() returning false so failures are reported as assertions instead of type errors.
it('never concatenates a raw request value into markup', function (string $file, string $pattern) use ($root) {
	expect(file_get_contents($root . '/' . $file))->not->toContain($pattern);
})->with('raw_reuse');

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

Comment thread .github/dependabot.yml
Comment on lines +9 to +10
# No composer entry: the plugin ships no composer.json, and its tests run from
# a plain PHP CLI by design.
Comment thread composer.json
Comment on lines +6 to +11
"require": {
"php": ">=8.0"
},
"require-dev": {
"pestphp/pest": "^2"
},
Comment on lines +27 to +29
it('escapes request values before printing them', function (string $file, string $pattern) use ($root) {
expect(file_get_contents($root . '/' . $file))->toContain($pattern);
})->with('escaped_outputs');
@somethingwithproof

Copy link
Copy Markdown
Member Author

Reopening against the renamed branch: the work is now stacked on #225 rather than duplicating its SHA pinning and permissions block, and Dependabot is left to #226.

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.

3 participants