From d19542ff6613112f067c23654d417179c791b774 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Wed, 7 Oct 2026 17:26:03 -0400 Subject: [PATCH 1/2] security: move inline event handlers to CSP-safe bindings Cacti's Content-Security-Policy script-src-attr directive blocks inline event-handler attributes. This converts the plugin's remaining inline handlers: - The graph "Add to QuickTree" icon (graph_buttons hook in setup.php) moves its onClick='addQuickTree(...)' to a delegated jQuery binding in js/quicktree.js, carrying the ids via a quicktreeAddLink class and data-local-graph-id / data-rra-id attributes. - The two confirmation-page Cancel buttons in quicktree.php switch from onClick='cactiReturnTo()' to the CSP-safe cactiReturnTo class. The graph_buttons unit test is updated to assert the new data-attribute markup. Cacti 1.2.31+ binds the cactiReturnTo class automatically; the README documents a one-time applySkin() snippet for earlier releases. --- CHANGELOG.md | 1 + README.md | 14 ++++++++++++++ js/quicktree.js | 8 ++++++++ quicktree.php | 4 ++-- setup.php | 2 +- tests/Unit/QuicktreeGraphButtonsTest.php | 3 ++- 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89ad6ab..b5f37a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* security: Move the graph 'Add to QuickTree' icon's inline `onClick='addQuickTree()'` handler into a delegated jQuery binding in `js/quicktree.js` (via a `quicktreeAddLink` class and `data-` attributes) and replace the confirmation pages' inline `onClick='cactiReturnTo()'` Cancel buttons with the CSP-safe `cactiReturnTo` class, so the pages no longer trip Cacti's Content-Security-Policy script-src-attr directive * dev: Replace the translation-template CI check that regenerated locales/po/cacti.pot and compared it with a diff-based gate (tests/bin/check-i18n-pot.php) requiring cacti.pot to be updated only when a pull request adds, removes, or modifies an i18n function call * dev: Measure CI coverage with xdebug instead of pcov so the plugin's own sources are instrumented (pcov auto-scopes to the Composer root and skipped cacti/plugins/, leaving the patch-coverage gate with nothing to measure) * dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step diff --git a/README.md b/README.md index dfef1b9..f48151b 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,20 @@ Known Issue: the way Cacti caches Graph Trees means that you might need to switch to another graph tree before the newly-created one appears in your list. This is true for Cacti-created trees too, so it seems to just be how it is. +## Cacti compatibility + +If you are running a version of Cacti below 1.2.31, please add the function +below to the `applySkin()` function in `include/layout.js` to enable the Cancel +buttons on forms to work: + +```js +$(document).off('click.cactiReturnTo', '.cactiReturnTo') + .on('click.cactiReturnTo', '.cactiReturnTo', function(event) { + event.preventDefault(); + cactiReturnTo($(this).attr('data-url')); + }); +``` + ## History See CHANGELOG.md diff --git a/js/quicktree.js b/js/quicktree.js index e42ece8..70dafd0 100644 --- a/js/quicktree.js +++ b/js/quicktree.js @@ -41,3 +41,11 @@ function addQuickTree(local_graph_id, rra_id) { displayMessages(); }); } + +$(function() { + $(document).off('click.quicktreeAdd', '.quicktreeAddLink') + .on('click.quicktreeAdd', '.quicktreeAddLink', function(event) { + event.preventDefault(); + addQuickTree($(this).data('local-graph-id'), $(this).data('rra-id')); + }); +}); \ No newline at end of file diff --git a/quicktree.php b/quicktree.php index 7c33a6d..e9b40a0 100644 --- a/quicktree.php +++ b/quicktree.php @@ -131,7 +131,7 @@ print ' -   +   '; quicktree_action_form_end(); @@ -183,7 +183,7 @@ print ' - + '; quicktree_action_form_end(); diff --git a/setup.php b/setup.php index b6269aa..d43b47f 100644 --- a/setup.php +++ b/setup.php @@ -216,7 +216,7 @@ function quicktree_graph_buttons($data): void { $local_graph_id = $data[1]['local_graph_id']; $rra_id = $data[1]['rra']; - print "
"; + print "
"; } } diff --git a/tests/Unit/QuicktreeGraphButtonsTest.php b/tests/Unit/QuicktreeGraphButtonsTest.php index d74a8f7..4a6ec1b 100644 --- a/tests/Unit/QuicktreeGraphButtonsTest.php +++ b/tests/Unit/QuicktreeGraphButtonsTest.php @@ -26,7 +26,8 @@ expect($output)->toContain('quicktreeAdd'); expect($output)->toContain('fa-plus-circle'); - expect($output)->toContain('addQuickTree(42, 7)'); + expect($output)->toContain("data-local-graph-id='42'"); + expect($output)->toContain("data-rra-id='7'"); }); it('prints nothing when the user is not authorized for the quicktree realm', function () { From ec1e0cbb3aa3195538e9f69411719c4f6bbe0758 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Wed, 7 Oct 2026 18:23:14 -0400 Subject: [PATCH 2/2] test: assert quicktreeAddLink binding class and reject inline handlers The graph-buttons test only checked the data- payload, so it would still pass if the quicktreeAddLink class (which the delegated click in js/quicktree.js keys off) were dropped, or if an inline onClick regressed. Assert the binding class is present and that no on*= attribute is emitted. --- tests/Unit/QuicktreeGraphButtonsTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/Unit/QuicktreeGraphButtonsTest.php b/tests/Unit/QuicktreeGraphButtonsTest.php index 4a6ec1b..6a78ee7 100644 --- a/tests/Unit/QuicktreeGraphButtonsTest.php +++ b/tests/Unit/QuicktreeGraphButtonsTest.php @@ -28,6 +28,14 @@ expect($output)->toContain('fa-plus-circle'); expect($output)->toContain("data-local-graph-id='42'"); expect($output)->toContain("data-rra-id='7'"); + + // The delegated click binding in js/quicktree.js keys off this class, so + // its presence is what actually wires the glyph up once the inline + // handler is gone. + expect($output)->toContain('quicktreeAddLink'); + + // Guard the CSP objective: no inline event-handler attribute may return. + expect($output)->not->toMatch('/on[a-z]+\s*=/i'); }); it('prints nothing when the user is not authorized for the quicktree realm', function () {