Skip to content

security: move inline event handlers to CSP-safe bindings #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheWitness merged 4 commits into from
Oct 8, 2026
Merged

security: move inline event handlers to CSP-safe bindings #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afbbaac
security: move inline event handlers to CSP-safe bindings
Oct 7, 2026
4c55984
test: define REPORTIT_BASE_PATH fallback in ReportitCalcSyntaxTest
TheWitness Oct 7, 2026
fa88f4d
style: expand empty debug() stub body to satisfy php-cs-fixer
TheWitness Oct 7, 2026
64bf16d
fix: dedupe reportitAddCalc handler and clarify Cancel-button compat
TheWitness Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

--- develop ---

* security: Move the plugin's remaining inline event handlers to CSP-safe bindings so the pages no longer trip Cacti's Content-Security-Policy script-src-attr directive: the confirmation-page Cancel buttons use the `cactiReturnTo` class, the filter rows selects are bound via change() in their ready blocks, and the template calc-syntax links use a delegated `reportitAddCalc` handler
* 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
* security: Add a version-safe CSP nonce (`plugin_reportit_csp_nonce()`) to every inline `<script>` tag so pages stay compatible with Cacti's Content-Security-Policy nonce enforcement, while falling back cleanly on older Cacti releases that lack the `CactiSecureHeaders` class
Expand Down
18 changes: 18 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ extensions are required

* Fast report generation: PHP extension 'php_rrdtool'

## Cacti compatibility

The Cancel buttons on ReportIt's forms rely on Cacti core binding the
`cactiReturnTo` class in `include/layout.js`. The 1.2.x line (1.2.31 and later)
ships this binding, but not every release does — notably the current
1.3.0/`develop` line, whose refactored `include/layout.js` no longer binds the
`.cactiReturnTo` class. If your Cacti release's `include/layout.js` has no
`.cactiReturnTo` click handler, add the binding below to its `applySkin()`
function so the Cancel buttons work:

```js
$(document).off('click.cactiReturnTo', '.cactiReturnTo')
.on('click.cactiReturnTo', '.cactiReturnTo', function(event) {
event.preventDefault();
cactiReturnTo($(this).attr('data-url'));
});
```

## Installation

The Installation of ReportIt is similar to other plugins:
Expand Down
2 changes: 1 addition & 1 deletion lib/funct_html.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function html_calc_syntax($measurand_id, $template_id): string {
. __('Examples: %s', $properties['examples'], 'reportit') . '</div>';
}

$output .= '<a id="' . $name . '" class="linkOverDark1 reportItHover" data-title="' . base64_encode($title) . '" onClick=add_to_calc("' . $name . '") style="cursor:pointer;">' . $name . '&nbsp;&nbsp;</a>';
$output .= '<a id="' . $name . '" class="linkOverDark1 reportItHover reportitAddCalc" data-title="' . base64_encode($title) . '" data-calc-name="' . $name . '" style="cursor:pointer;">' . $name . '&nbsp;&nbsp;</a>';
}

$output .= '</div>';
Expand Down
22 changes: 15 additions & 7 deletions reportit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function report_wizard(): void {
header('Location: reportit.php');
exit;
} else {
$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"reportit.php\")'>&nbsp;<input type='submit' value='" . __esc('Continue', 'reportit') . "' title='" . __esc('Create a new report', 'reportit') . "'>";
$save_html = "<input class='cactiReturnTo' data-url='reportit.php' type='button' value='" . __esc('Cancel', 'reportit') . "'>&nbsp;<input type='submit' value='" . __esc('Continue', 'reportit') . "' title='" . __esc('Create a new report', 'reportit') . "'>";

foreach ($templates_list as $tmp) {
$templates[$tmp['id']] = $tmp['name'];
Expand Down Expand Up @@ -181,7 +181,7 @@ function report_filter(): void {
<?php print __('Reports', 'reportit'); ?>
</td>
<td>
<select id='rows' onChange='applyFilter()'>
<select id='rows'>
<option value='-1'<?php print (get_request_var('rows') == '-1' ? ' selected>' : '>') . __('Default', 'reportit'); ?></option>
<?php
if (cacti_sizeof($item_rows)) {
Expand Down Expand Up @@ -221,6 +221,10 @@ function clearFilter() {
}

$(function() {
$('#rows').change(function() {
applyFilter();
});

$('#refresh').click(function() {
applyFilter();
});
Expand Down Expand Up @@ -1445,7 +1449,7 @@ function report_edit(): void {
<?php print __('RRDs', 'reportit'); ?>
</td>
<td>
<select id='rows' onChange='applyFilter()'>
<select id='rows'>
<option value='-1'<?php print (get_request_var('rows') == '-1' ? ' selected>' : '>') . __('Default', 'reportit'); ?></option>
<?php
if (cacti_sizeof($item_rows)) {
Expand Down Expand Up @@ -1495,6 +1499,10 @@ function clearFilter() {
}

$(function() {
$('#rows').change(function() {
applyFilter();
});

$('#refresh').click(function() {
applyFilter();
});
Expand Down Expand Up @@ -2033,7 +2041,7 @@ function form_actions(): void {

if ($reports === false || empty($reports)) {
print "<tr><td class='textArea'><span class='textError'>" . __('You must select at least one unlocked, not running, report.', 'reportit') . "</span>$reports_sql</td></tr>";
$save_html = "<input type='button' value='" . __('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"reportit.php\")'>";
$save_html = "<input class='cactiReturnTo' data-url='reportit.php' type='button' value='" . __('Cancel', 'reportit') . "'>";
} else {
print "<tr><td class='textArea'>$section</td></tr><tr><td>";
print '<div class="itemlist"><ul>';
Expand All @@ -2045,7 +2053,7 @@ function form_actions(): void {

print '</ul></div>';
print '</td></tr>';
$save_html = "<input type='button' value='" . __('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"reportit.php\")'>&nbsp;<input type='submit' value='" . __('Continue', 'reportit') . "'>";
$save_html = "<input class='cactiReturnTo' data-url='reportit.php' type='button' value='" . __('Cancel', 'reportit') . "'>&nbsp;<input type='submit' value='" . __('Continue', 'reportit') . "'>";
}

print "<tr>
Expand Down Expand Up @@ -2149,9 +2157,9 @@ function form_actions(): void {
if (empty($ds_list)) {
print "<tr><td class='odd''><span class='textError'>" . __('You must select at least one Report.', 'reportit') . '</span></td></tr>';

$save_html = "<input type='button' value='" . __('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"reportit.php\")'>";
$save_html = "<input class='cactiReturnTo' data-url='reportit.php' type='button' value='" . __('Cancel', 'reportit') . "'>";
} else {
$save_html = "<input type='button' value='" . __('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"reportit.php\")'>&nbsp;
$save_html = "<input class='cactiReturnTo' data-url='reportit.php' type='button' value='" . __('Cancel', 'reportit') . "'>&nbsp;
<input type='submit' value='" . __('Continue', 'reportit') . "'>";
}

Expand Down
32 changes: 22 additions & 10 deletions templates.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ function template_wizard(string $action): void {
<span class='textError'>" . __('There are no Data Templates in use.', 'reportit') . '</span>
</td>
</tr>';
$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"templates.php\")'>";
$save_html = "<input class='cactiReturnTo' data-url='templates.php' type='button' value='" . __esc('Cancel', 'reportit') . "'>";
} else {
$save_html = '<input type="button" value="' . __esc('Cancel', 'reportit') . '" onClick="cactiReturnTo(\'templates.php\')">&nbsp;<input type="submit" value="' . __esc('Continue', 'reportit') . '" title="' . __esc('Create a new Report Template', 'reportit') . '">';
$save_html = '<input type="button" class="cactiReturnTo" data-url="templates.php" value="' . __esc('Cancel', 'reportit') . '">&nbsp;<input type="submit" value="' . __esc('Continue', 'reportit') . '" title="' . __esc('Create a new Report Template', 'reportit') . '">';
print "<tr class='textArea'>
<td>
<p>" . __('Choose a Data Template this Report Template should depend on. Unused Data Templates are hidden.', 'reportit') . '</p><p>';
Expand Down Expand Up @@ -363,8 +363,8 @@ function template_wizard(string $action): void {
}

$save_html = ($compatible)
? "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo()'>&nbsp;<input type='submit' value='" . __esc('Import', 'reportit') . "' title='" . __esc('Import Report Template', 'reportit') . "'>"
: "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo()'>";
? "<input class='cactiReturnTo' type='button' value='" . __esc('Cancel', 'reportit') . "'>&nbsp;<input type='submit' value='" . __esc('Import', 'reportit') . "' title='" . __esc('Import Report Template', 'reportit') . "'>"
: "<input class='cactiReturnTo' type='button' value='" . __esc('Cancel', 'reportit') . "'>";

print "<tr>
<td class='saveRow' colspan='6'>
Expand Down Expand Up @@ -491,7 +491,7 @@ function template_filter(): void {
<?php print __('Templates', 'reportit'); ?>
</td>
<td>
<select id='rows' onChange='applyFilter()'>
<select id='rows'>
<option value='-1'<?php print (get_request_var('rows') == '-1' ? ' selected>' : '>') . __('Default', 'reportit'); ?></option>
<?php
if (cacti_sizeof($item_rows)) {
Expand Down Expand Up @@ -529,6 +529,10 @@ function clearFilter() {
}

$(function() {
$('#rows').change(function() {
applyFilter();
});

$('#refresh').click(function() {
applyFilter();
});
Expand Down Expand Up @@ -1569,7 +1573,7 @@ function DownloadStart(url) {
}

$save_focus = ' class="ui-button ui-corner-all ui-widget ui-state-active"';
$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo()'>";
$save_html = "<input class='cactiReturnTo' type='button' value='" . __esc('Cancel', 'reportit') . "'>";

if (empty($ds_list)) {
print "<tr>
Expand Down Expand Up @@ -1697,9 +1701,9 @@ function DownloadStart(url) {
print "<tr><td class='odd'><span class='textError'>" . __('You must select at least one variable.', 'reportit') . '</span></td></tr>';
}

$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"templates.php?tab=variables&id=" . get_request_var('id') . "\")'>";
$save_html = "<input type='button' class='cactiReturnTo' data-url='templates.php?tab=variables&id=" . get_request_var('id') . "' value='" . __esc('Cancel', 'reportit') . "'>";
} else {
$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo(\"templates.php?tab=variables&id=" . get_request_var('id') . "\")'>&nbsp;<input type='submit' value='" . __esc('Continue', 'reportit') . "' title='" . __esc('Delete Template Variables', 'reportit') . "'>";
$save_html = "<input type='button' class='cactiReturnTo' data-url='templates.php?tab=variables&id=" . get_request_var('id') . "' value='" . __esc('Cancel', 'reportit') . "'>&nbsp;<input type='submit' value='" . __esc('Continue', 'reportit') . "' title='" . __esc('Delete Template Variables', 'reportit') . "'>";
}
}

Expand Down Expand Up @@ -1796,9 +1800,9 @@ function DownloadStart(url) {
</td>
</tr>';

$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo()'>";
$save_html = "<input class='cactiReturnTo' type='button' value='" . __esc('Cancel', 'reportit') . "'>";
} else {
$save_html = "<input type='button' value='" . __esc('Cancel', 'reportit') . "' onClick='cactiReturnTo()'>&nbsp;<input type='submit' value='" . __esc('Continue', 'reportit') . "' title='" . __esc('Delete Template Metrics', 'reportit') . "'>";
$save_html = "<input class='cactiReturnTo' type='button' value='" . __esc('Cancel', 'reportit') . "'>&nbsp;<input type='submit' value='" . __esc('Continue', 'reportit') . "' title='" . __esc('Delete Template Metrics', 'reportit') . "'>";
}
}

Expand Down Expand Up @@ -2345,6 +2349,14 @@ function add_to_calc(name) {
}

$(function(){
/* delegated + namespaced-off-before-on: this ready block re-runs on every
* AJAX page load (applySkin), and document persists across those loads, so
* without the off() the handler would stack and append the token repeatedly */
$(document).off('click.reportitAddCalc', '.reportitAddCalc')
.on('click.reportitAddCalc', '.reportitAddCalc', function() {
add_to_calc($(this).data('calc-name'));
});

$('#data_type').change(function() {
change_data_type();
});
Expand Down
44 changes: 44 additions & 0 deletions tests/Unit/ReportitCalcSyntaxTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
*/

/*
* Unit coverage for html_calc_syntax() in lib/funct_html.php: the calc-syntax
* helper renders one anchor per usable variable token. The anchors use a
* CSP-safe `reportitAddCalc` class and `data-calc-name` attribute (bound via a
* delegated handler) rather than an inline onClick.
*/

beforeAll(function () {
require_once __DIR__ . '/../../setup.php';

// setup.php only defines REPORTIT_BASE_PATH at runtime inside
// reportit_define_constants() (realm/DB work); define the same value here.
if (!defined('REPORTIT_BASE_PATH')) {
define('REPORTIT_BASE_PATH', dirname(__DIR__, 2));
}

require_once REPORTIT_BASE_PATH . '/lib/funct_shared.php';
require_once REPORTIT_BASE_PATH . '/lib/funct_html.php';

if (!function_exists('debug')) {
function debug($data, $label = '') {
}
}
});

it('renders calc-syntax links with CSP-safe markup and no inline onClick', function () {
global $calc_var_names, $rubrics;

$calc_var_names = ['maxValue', 'calcVarA', 'calcVarB'];
$rubrics = [];

$output = html_calc_syntax(0, 1);

expect($output)->toContain('reportitAddCalc');
expect($output)->toContain('data-calc-name=');
expect($output)->not->toContain('onClick');
});
3 changes: 3 additions & 0 deletions tests/bin/patch-coverage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ function changed_lines($base_ref) {
* Empty by default; add entries per repository as the need arises.
*/
$unmeasured_allowlist = [
'reportit.php', // web UI entry point (chdir + require auth.php); not loadable in the isolated unit process
'templates.php', // web UI entry point (chdir + require auth.php); not loadable in the isolated unit process
'view.php', // web UI entry point (chdir + require auth.php); not loadable in the isolated unit process
];
$unmeasured = array_values(array_diff(array_keys($changed), array_keys($measured)));
$unexpected_unmeasured = array_values(array_diff($unmeasured, $unmeasured_allowlist));
Expand Down
2 changes: 1 addition & 1 deletion view.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ function standard(): void {
<?php print __('Reports', 'reportit'); ?>
</td>
<td>
<select id='rows' onChange='applyFilter()'>
<select id='rows'>
<option value='-1'<?php print (get_request_var('rows') == '-1' ? ' selected>' : '>') . __('Default', 'reportit'); ?></option>
<?php
if (cacti_sizeof($item_rows)) {
Expand Down
Loading