diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e54bf0..614b7fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ --- 3.0 --- * security: Require POST for the save, remove, mute and card-order actions so csrf-magic covers them, and only let a dashboard's owner save over it +* security: Bind the plugin version lookup and the device_remove cleanup deletes as prepared statements instead of building them from the device list * bug: Restore alert audio for down and triggered devices; getMonitorSound() looked for sounds in includes/sounds/ after the controller moved, so the audio element was never rendered * feature: Replace the single Status column/filter with a multi-select Events filter (additive) and an Events column that shows a short pill per active event (Down, Recovering, Triggered, Down/Triggered M/A, Warn/Alert Ping, Failed Svc), 'None' when clean; the Status scope dropdown now only selects All Monitored / All Devices / Not Monitored / Without Thresholds * bug: Centre the tile hostname under the glyph and keep it clickable in the Tiles & Time view diff --git a/setup.php b/setup.php index 3a01467..7088d3b 100644 --- a/setup.php +++ b/setup.php @@ -328,7 +328,7 @@ function monitor_check_upgrade() { $info = plugin_monitor_version(); $current = $info['version']; - $old = db_fetch_cell('SELECT version FROM plugin_config WHERE directory = "monitor"'); + $old = db_fetch_cell_prepared('SELECT version FROM plugin_config WHERE directory = ?', array('monitor')); if ($current != $old) { monitor_setup_table(); @@ -484,9 +484,15 @@ function monitor_device_action_execute($action) { * @return array The unmodified $devices array. */ function monitor_device_remove($devices) { - db_execute('DELETE FROM plugin_monitor_notify_history WHERE host_id IN(' . implode(',', $devices) . ')'); - db_execute('DELETE FROM plugin_monitor_reboot_history WHERE host_id IN(' . implode(',', $devices) . ')'); - db_execute('DELETE FROM plugin_monitor_uptime WHERE host_id IN(' . implode(',', $devices) . ')'); + $ids = array_map('intval', array_values($devices)); + + if (cacti_sizeof($ids)) { + $in = implode(',', array_fill(0, count($ids), '?')); + + db_execute_prepared("DELETE FROM plugin_monitor_notify_history WHERE host_id IN ($in)", $ids); + db_execute_prepared("DELETE FROM plugin_monitor_reboot_history WHERE host_id IN ($in)", $ids); + db_execute_prepared("DELETE FROM plugin_monitor_uptime WHERE host_id IN ($in)", $ids); + } return $devices; } diff --git a/tests/Unit/MonitorDeviceHooksTest.php b/tests/Unit/MonitorDeviceHooksTest.php index 376f187..f6fb4dc 100644 --- a/tests/Unit/MonitorDeviceHooksTest.php +++ b/tests/Unit/MonitorDeviceHooksTest.php @@ -59,10 +59,28 @@ expect($result)->toBe(array(3, 4)); foreach ($GLOBALS['__test_db_calls'] as $call) { - expect($call['sql'])->toContain('IN(3,4)'); + expect($call['fn'])->toBe('db_execute_prepared'); + expect($call['sql'])->toContain('IN (?,?)'); + expect($call['params'])->toBe(array(3, 4)); } }); +it('binds device ids as integers on device_remove', function () { + monitor_device_remove(array('5', '6) OR 1=1 --')); + + foreach ($GLOBALS['__test_db_calls'] as $call) { + expect($call['sql'])->not->toContain('OR 1=1'); + expect($call['params'])->toBe(array(5, 6)); + } +}); + +it('issues no delete on device_remove when no devices were removed', function () { + $result = monitor_device_remove(array()); + + expect($GLOBALS['__test_db_calls'])->toBeEmpty(); + expect($result)->toBe(array()); +}); + it('adds the monitor breadcrumb entry without disturbing existing ones', function () { $nav = monitor_draw_navigation_text(array('other.php:' => array('title' => 'Other'))); diff --git a/tests/Unit/MonitorLifecycleTest.php b/tests/Unit/MonitorLifecycleTest.php index 73f1f53..71e1ad2 100644 --- a/tests/Unit/MonitorLifecycleTest.php +++ b/tests/Unit/MonitorLifecycleTest.php @@ -97,7 +97,7 @@ }); it('updates the stored plugin_config version when it drifts', function () { - monitor_test_mock_db('db_fetch_cell', 'plugin_config', '0.0.0'); + monitor_test_mock_db('db_fetch_cell_prepared', 'plugin_config', '0.0.0'); monitor_check_upgrade();