Skip to content

hardening: prepared statements, PHP 7.4 idioms, and security fixes - #49

Open
somethingwithproof wants to merge 5 commits into
Cacti:developfrom
somethingwithproof:hardening/comprehensive
Open

hardening: prepared statements, PHP 7.4 idioms, and security fixes#49
somethingwithproof wants to merge 5 commits into
Cacti:developfrom
somethingwithproof:hardening/comprehensive

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Apr 9, 2026

Copy link
Copy Markdown

Consolidated hardening PR:

  • Migrate isset() ternary to null coalescing (??) operator in maint.php (behaviorally identical, isset() already excludes null)
  • Guard against zero/negative minterval producing an infinite DateInterval loop in plugin_maint_check_schedule() (functions.php); logs a warning and disables the recurring calc instead of hanging (FIND-004)
  • Add Pest security test suite: auth guard presence, output escaping, prepared-statement consistency, redirect safety, PHP 7.4 compatibility
  • CI: drop the unavailable versioned libapache2-mod-php package from the workflow

PHP 7.4+ compatible. The minterval guard and the CI package fix are behavioral changes, not purely mechanical; everything else in this PR is a mechanical transform.

Copilot AI review requested due to automatic review settings April 9, 2026 06:04

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 maint plugin’s UI and runtime schedule evaluation by modernizing a few PHP idioms, escaping user-controlled output in the schedules list, and adding a guard to prevent pathological recurring-schedule recalculation.

Changes:

  • Replace isset(...) ? ... : ... with ?? in the schedule edit form field values.
  • Escape schedule names passed into form_checkbox_cell() in the schedules list.
  • Prevent an infinite loop in recurring schedule rollover when minterval <= 0 by returning early.

Reviewed changes

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

File Description
maint.php Uses ?? for form values and escapes schedule name in the schedules table checkbox label.
functions.php Adds a minterval guard to prevent infinite looping when rolling recurring schedules forward.

Comment thread functions.php
Comment thread functions.php
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Add targeted tests for prepared statement migration, output escaping,
auth guard presence, CSRF token validation, redirect safety, and
PHP 7.4 compatibility. Tests use source-scan patterns that verify
security invariants without requiring the Cacti database.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
somethingwithproof force-pushed the hardening/comprehensive branch from c71982b to 6cd5c54 Compare July 14, 2026 10:47
@somethingwithproof

Copy link
Copy Markdown
Author

Fixed the CI failure: unescaped quotes in tests/Security/RedirectSafetyTest.php:27 and AuthGuardTest.php:52 (single-quoted PHP string containing an unescaped ') broke php -l on all four PHP versions. Also added a warning log when a schedule has minterval <= 0, per review comment. Updated the PR description to match the actual diff (it was stale from an earlier, smaller version of this PR). No rebase needed, branch was already even with develop.

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