Skip to content

fix(security): defense-in-depth hardening for plugin_routerconfigs#151

Draft
somethingwithproof wants to merge 3 commits into
Cacti:developfrom
somethingwithproof:fix/defense-in-depth
Draft

fix(security): defense-in-depth hardening for plugin_routerconfigs#151
somethingwithproof wants to merge 3 commits into
Cacti:developfrom
somethingwithproof:fix/defense-in-depth

Conversation

@somethingwithproof
Copy link
Copy Markdown

Summary

Defense-in-depth hardening addressing 9 security audit findings.

  • XSS: Escape request variables in HTML output with html_escape_request_var()
  • SQLi: Convert string-concat queries to prepared statements
  • Deserialization: Add allowed_classes => false to unserialize()

All changes PHP 7.0+ compatible.

Test plan

  • PHP lint clean
  • Pre-push review PASS (Claude + Grok + Copilot)
  • Verify plugin functionality

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copilot AI review requested due to automatic review settings April 9, 2026 06:16
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

This PR hardens the routerconfigs Cacti plugin against PHP object-injection risks by restricting deserialization in the shared credential decode helper.

Changes:

  • Update plugin_routerconfigs_decode() to call unserialize() with allowed_classes => false to prevent object instantiation during deserialization.

- Change Dependabot ecosystem from npm to composer (PHP-only repo)
- Remove PHP from CodeQL paths-ignore so security PRs get analysis
- Remove committed .omc session artifacts, add .omc/ to .gitignore

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof marked this pull request as draft April 11, 2026 00:10
@somethingwithproof
Copy link
Copy Markdown
Author

Converted to draft to serialize the stack in this repo. Blocked by #153; will un-draft after that merges to avoid cross-PR merge conflicts.

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