Skip to content

security: convert cactiReturnTo cancel buttons to the CSP-safe class - #93

Merged
TheWitness merged 2 commits into
developfrom
fix/csp-cactireturnto-class
Oct 8, 2026
Merged

TheWitness merged 2 commits into
developfrom
fix/csp-cactireturnto-class

Conversation

@TheWitness

Copy link
Copy Markdown
Member

Summary

Pilots the cactiReturnTo CSS-class approach for the plugin fleet's confirmation-page Cancel buttons, replacing the inline onClick='cactiReturnTo()' handlers that trip Cacti's Content-Security-Policy script-src-attr directive.

Approach (no shim in core or plugin)

  • Markup switches from onClick='cactiReturnTo(url)' to class='cactiReturnTo' (plus data-url='<url>' only when a target URL is passed; the no-arg case falls back to document.referrer, which cactiReturnTo() already does).
  • Cacti 1.2.31+ binds this class automatically.
  • To preserve the plugin's 1.2.29 compatibility floor, the README documents a one-time include/layout.js snippet that operators on 1.2.29–1.2.30 add before upgrading. Nothing is baked into core or the plugin.

Changes

  • webseer.php, webseer_servers.php, webseer_proxies.php — 8 Cancel buttons converted to class='cactiReturnTo'.
  • README.md — new "Cacti 1.2.29 - 1.2.30 compatibility" section with the snippet.
  • CHANGELOG.md — entry.

All three PHP files are already in the patch-coverage gate's allowlist; the edits are same-line (no source-line shift), so cacti.pot is unaffected.

Depends on the core delegated .cactiReturnTo handler landing in 1.2.31 (tracked in the core CSP issue). Until a site is on such a core, the README snippet is the bridge.

This is the pilot — once the pattern/README wording is approved, I'll roll it across the rest of the fleet.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Regression coverage is needed because the changed entry points are exempt from patch-coverage enforcement.

1 open finding
What changed in this PR

Updates confirmation-page Cancel buttons to use Cacti’s CSP-safe delegated event handler while documenting compatibility for older Cacti releases.

Changes:

  • Replaces eight inline handlers with cactiReturnTo classes.
  • Documents the Cacti 1.2.29–1.2.30 compatibility bridge.
  • Records the security change.
File Description
webseer.php Converts URL action Cancel buttons.
webseer_servers.php Converts server action Cancel buttons.
webseer_proxies.php Converts the proxy action Cancel button.
README.md Documents compatibility setup.
CHANGELOG.md Records the CSP improvement.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread webseer.php
The confirmation-page Cancel buttons on webseer.php, webseer_servers.php and
webseer_proxies.php 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. To keep the plugin's
1.2.29 compatibility floor, the README documents a one-time include/layout.js
snippet for operators still on 1.2.29-1.2.30 - no shim is baked into the core
or the plugin.
@TheWitness
TheWitness force-pushed the fix/csp-cactireturnto-class branch from 1267fd7 to c955b75 Compare October 7, 2026 21:00
Assert the three confirmation-page entry points carry no inline cactiReturnTo onClick handler and expose all eight Cancel buttons via the CSP-safe cactiReturnTo class. These files are exempt from the patch-coverage gate, so this source-level test is what catches a reintroduced inline handler.
@TheWitness
TheWitness merged commit 611910d into develop Oct 8, 2026
3 checks passed
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.

4 participants