Repository navigation
security: convert cactiReturnTo cancel buttons to the CSP-safe class - #78
Merged
Merged
Conversation
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.
TheWitness
requested review from
bmfmancini,
browniebraun and
xmacan
and
a balanced review from Copilot
October 7, 2026 21:17
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two converted Return buttons contain malformed HTML that can affect rendering.
1 open finding
What changed in this PR
Replaces inline Cancel/Return handlers with CSP-safe class bindings and documents compatibility requirements.
Changes:
- Converts 12 controls to
cactiReturnToclass bindings. - Documents pre-1.2.31 compatibility.
- Updates the changelog.
| File | Description |
|---|---|
maint.php |
Converts inline handlers; two malformed button tags remain. |
README.md |
Adds compatibility instructions. |
CHANGELOG.md |
Records the CSP cleanup. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The schedules Return control (line 447) ended '</button' with no closing '>', and the webseer Return control (line 587) carried a stray double '>>' in its opening tag. Both left malformed markup on the no-selection path; normalize them to a well-formed <button ...>Return</button>.
bmfmancini
approved these changes
Oct 8, 2026
bmfmancini
left a comment
bmfmancini
left a comment
Member
There was a problem hiding this comment.
Reviewed; cactiReturnTo class moves to the Cancel button correctly. Checks pass.
browniebraun
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
Converts the plugin's confirmation-page Cancel buttons from inline
onClick='cactiReturnTo(...)'handlers to the CSP-safecactiReturnToCSS class, so the pages no longer trip Cacti's Content-Security-Policyscript-src-attrdirective. Part of the fleet-wide CSP inline-handler cleanup.Approach (no shim in core or plugin)
onClick='cactiReturnTo(url)'toclass='cactiReturnTo'(plusdata-url='<url>'when a target URL was passed; the no-arg case falls back todocument.referrer, whichcactiReturnTo()already does).class='ui-button ...'attributes are merged, not duplicated.applySkin()snippet for operators on earlier releases. Nothing is baked into core or the plugin.Changes
data-url='maint.php').maint.phpis already in the patch-coverage gate's allowlist; all edits are same-line (no source-line shift), socacti.potis unaffected.