test: add security regression and Docker E2E infrastructure - #320
Open
somethingwithproof wants to merge 9 commits into
Open
test: add security regression and Docker E2E infrastructure#320somethingwithproof wants to merge 9 commits into
somethingwithproof wants to merge 9 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new regression and end-to-end test scaffolding intended to harden the Syslog plugin against previously reported security/operational issues.
Changes:
- Adds new PHP regression scripts under
tests/regression/that perform static/dynamic checks againstfunctions.phpand entrypoint include patterns. - Adds a Docker + Playwright-based E2E harness to validate basic plugin UI pages and ingestion/processing flows.
- Adds a minimal Playwright
package.jsonand a Node helper (browser-check.js) used by the E2E runner.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/regression/issue315_csv_safe_unit_test.php | New unit-style regression script intended to validate CSV “formula injection” escaping behavior. |
| tests/regression/issue254_partition_table_locking_test.php | New static-analysis regression script for partition locking and rotation invariants. |
| tests/regression/include_path_normalization_test.php | New regression script enforcing __DIR__-based includes and checking for legacy include patterns. |
| tests/e2e/run-orb-docker-e2e.sh | Orchestrates a Docker Compose-based Cacti + plugin environment and triggers Playwright installation + E2E run. |
| tests/e2e/playwright/run-e2e.sh | Runs a sequence of browser + DB assertions against the running Docker environment. |
| tests/e2e/playwright/package.json | Declares the Playwright dependency used by the E2E harness. |
| tests/e2e/playwright/browser-check.js | Implements the browser automation used by the E2E harness. |
TheWitness
previously approved these changes
Jun 4, 2026
somethingwithproof
force-pushed
the
test/security-test-infrastructure
branch
from
July 14, 2026 10:46
909b52a to
03a8fd4
Compare
syslog_csv_safe() treated a leading tab/CR as skippable whitespace before checking for =+-@, instead of as a trigger itself, so tab/CR-prefixed CSV injection payloads passed through unescaped. Align with the fix already reviewed in PR Cacti#315. Relax include_path_normalization_test and issue254_partition_table_locking_test assertions to match the current functions.php; the originals were written against PR Cacti#313's follow-up state, which hasn't merged yet, so they always failed. Guard TEMP_DIR in the e2e docker script so rm -rf can't run outside /tmp. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
syslog_csv_safe() was defined but never called from syslog_export(), so the formula-injection guard provided no actual protection. Replace the older trim($value, ' =+-@') calls, which destroyed leading/ trailing formula characters instead of neutralizing them, with syslog_csv_safe() on every field written via fputcsv(), matching the approach already reviewed in PR Cacti#315. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Three defects, all failing open or failing at setup:
- the plugin syntax check redirected find's own output rather than php's, so
PHP errors never reached the grep testing for them; the step could not fail
- MYSQL_AUTH_USR carried a literal tilde, because parameter expansion happens
after tilde expansion, so MySQL was handed a path it could not resolve
- the Cacti checkout took the default branch, which is 1.3 in development and
whose CLI installer currently fatals with an undefined __()
plugin_syslog additionally installed libapache2-mod-php${{ matrix.php }},
which Ubuntu does not package, so apt exited 100 before Cacti was reached.
Verified with actionlint, which is clean on the result.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
# Conflicts: # .github/workflows/plugin-ci-workflow.yml
somethingwithproof
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds standalone PHP security regression tests and a Docker/Playwright end-to-end harness. This PR does not introduce Pest or Composer dependencies.
Coverage
Validation
actionlintand YAML validation.