Repository navigation
security: move filter inline onChange handlers into ready() for CSP - #24
Merged
Merged
Conversation
The tags filter's Target, Type and Rows selects carried inline onChange='applyFilter()' attributes. Under Cacti's Content-Security-Policy these trip the script-src-attr directive (inline event handlers). Remove them and fix the ready block to bind those selects via change (they were bound via click, which does not reliably fire on a select). Allowlist tags.php in the patch-coverage gate (web UI entry point, not loaded into the isolated unit process). Regenerated locales/po/cacti.pot for the shifted source line references. The cactiReturnTo() cancel buttons are left inline (shared Cacti-core pattern, tracked separately).
TheWitness
requested review from
bmfmancini,
browniebraun and
xmacan
and
a balanced review from Copilot
October 7, 2026 20:12
There was a problem hiding this comment.
🟢 Approval recommended
The narrowly scoped changes preserve filter behavior, with no blocking issues identified.
0 open findings
What changed in this PR
Updates tag filters to avoid inline event handlers blocked by Cacti’s Content Security Policy.
Changes:
- Binds filter selects through
changeevents in the ready block. - Exempts the web entry point from unmeasured patch coverage.
- Documents the change and refreshes the translation template timestamp.
| File | Description |
|---|---|
| tests/bin/patch-coverage.php | Allowlists tags.php as an unmeasured web entry point. |
| tags.php | Replaces inline handlers and click bindings with change bindings. |
| locales/po/cacti.pot | Updates the template creation timestamp. |
| CHANGELOG.md | Records the filter event-handler change. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The confirmation pages' Cancel buttons used an inline onClick='cactiReturnTo()' handler, which trips Cacti's Content-Security-Policy script-src-attr directive. Switch them to the CSP-safe cactiReturnTo CSS class (class='cactiReturnTo', plus an optional data-url when a target is passed). Cacti 1.2.31 and later bind this class automatically. The README documents a one-time include/layout.js applySkin() snippet for operators still on an earlier release - no shim is baked into the core or the plugin.
bmfmancini
approved these changes
Oct 8, 2026
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
Removes the inline
onChangeevent handlers from the tags filter so the page no longer violates Cacti's Content-Security-Policyscript-src-attrdirective (continuing the CSP cleanup across the plugin fleet).Changes
#target), Type (#type) and Rows (#rows) selects had inlineonChange='applyFilter()'. Removed them and corrected the ready block to bind those selects via.change()(they were previously bound via.click(), which does not reliably fire on a<select>— the inlineonChangewas what actually drove them).tags.php(web UI entry point not loaded into the isolated unit process).locales/po/cacti.potfor the shifted source line references (msgid set unchanged).The
cactiReturnTo()cancel buttons on confirmation pages are left inline — shared Cacti-core pattern, handled separately.