Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
180790c
Call Cacti controller methods on an instance in the host import
somethingwithproof Oct 24, 2026
54d4a95
Quote the arguments of the host import command
somethingwithproof Oct 24, 2026
65cb48b
Store the host import address cache as JSON in the Cacti cache directory
somethingwithproof Oct 24, 2026
e6797ef
Require device and data input realms for the host import and data inp…
somethingwithproof Oct 24, 2026
3f462f7
Use the plain object id when reading perfdata for a new data input me…
somethingwithproof Oct 24, 2026
12a6cff
fix: accept only known tab names in the NPC frame script
somethingwithproof Oct 24, 2026
67434d1
Call the Nagios controller on an instance when deleting all comments
somethingwithproof Oct 24, 2026
880e6d7
fix: check the command realm before the command file settings
somethingwithproof Oct 24, 2026
b0a6f0c
fix: load the base controller class in npc.php
somethingwithproof Oct 24, 2026
c06cc29
fix: accept state-changing NPC actions only as POST requests
somethingwithproof Oct 24, 2026
bcf41bd
fix: copy only known request parameters onto controllers and bound st…
somethingwithproof Oct 24, 2026
3a7de47
fix: pass searchFields and data to controllers as JSON
somethingwithproof Oct 24, 2026
6e75bd1
fix: drop dispatcher actions that cannot run with a single parameter …
somethingwithproof Oct 24, 2026
720300f
test: cover the hostgroup selection of the host import
somethingwithproof Oct 24, 2026
936ebdb
fix: name the failing class and method in the controller error response
somethingwithproof Oct 24, 2026
b3f31e9
fix: read only the newest host check when fetching host perfdata
somethingwithproof Oct 24, 2026
457911c
fix: declare the request properties the controllers read
somethingwithproof Oct 24, 2026
a2f5107
fix: list each service once in the service grid
somethingwithproof Oct 24, 2026
bb600ed
fix: skip non-numeric perfdata entries in the data input scripts
somethingwithproof Oct 24, 2026
ab820f1
fix: correct the contacts table upgrade and the deleted user settings…
somethingwithproof Oct 24, 2026
28b07b7
docs: describe the NPC Global Commands realm
somethingwithproof Oct 24, 2026
c0aa522
fix: read the grid and summary responses in the front end
somethingwithproof Oct 24, 2026
fa6fc05
fix: escape quotes in attribute values written by the front end
somethingwithproof Oct 24, 2026
aae0e60
fix: send the required values from the Nagios command dialogs
somethingwithproof Oct 24, 2026
8b19f56
fix: write the LIMIT values of the listing queries as integers
somethingwithproof Oct 24, 2026
3a954bd
test: run the schema and queries against MariaDB
somethingwithproof Oct 24, 2026
ada16c5
chore: refresh the translation template
somethingwithproof Oct 24, 2026
7f8cdd0
test: give the Cacti function stubs their real signatures
somethingwithproof Oct 24, 2026
06b0e99
test: load js/npc.js through require instead of a vm context
somethingwithproof Oct 24, 2026
d18a101
refactor: share one perfdata parser between the data input scripts
somethingwithproof Oct 24, 2026
8af7219
test: let the subprocess tests record line coverage
somethingwithproof Oct 24, 2026
5cdb545
fix: cast unsaved settings to strings in the NPC frame
somethingwithproof Oct 24, 2026
69d91a3
test: run the plugin under a real web server with real logins
somethingwithproof Oct 24, 2026
2f9dfa1
test: use fixed statements for fixtures and add real-database paths
somethingwithproof Oct 24, 2026
5547d61
fix: log a fixed message for a host selection that is not JSON
somethingwithproof Oct 24, 2026
28adb8a
fix: return an empty definition for an unknown Nagios command
somethingwithproof Oct 24, 2026
10bade0
fix: treat a missing searchFields value as empty
somethingwithproof Oct 24, 2026
93c77f8
test: cover denied, empty and hostile input for the changed actions
somethingwithproof Oct 24, 2026
72058d2
test: cover the detail dialogs, comment deletion, filters and routing…
somethingwithproof Oct 24, 2026
a0ff044
fix: drop the device mapping column on uninstall only when it exists
somethingwithproof Oct 24, 2026
a919095
test: share the import fixture and check that the test helpers stay i…
somethingwithproof Oct 24, 2026
59eb27d
Merge branch 'develop' into fix/review-hardening
somethingwithproof Oct 25, 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
71 changes: 71 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,77 @@

--- develop ---

* issue: Drop the device mapping column on uninstall only when it exists; a repeated
uninstall logged a failed ALTER TABLE
* issue: Treat a missing searchFields value as empty in the grid search; a search
string without it raised the PHP 8.1 null deprecation
* issue: Return an empty definition for an unknown Nagios command instead of raising
undefined index and foreach warnings before the not a valid command message
* issue: Log a fixed message when the host import selection is not a JSON object;
the old message converted an array to a string
* issue: Cast the date format, time format and Nagios URL settings to strings in the
NPC frame; unsaved settings raised the PHP 8.1 null deprecation on every page
* dev: Run the schema and the listing, perfdata and upgrade queries against MariaDB
in the Pest suite, and run npc.php, perfdata.php and cli.php as subprocesses
(tests/E2E); the JavaScript front end is tested with Node's test runner
* issue: Write the LIMIT values of the host, service, comment, downtime, event log,
notification and state history listings into the statement as integers;
Cacti binds values as strings and the databases reject a quoted LIMIT, so these
listings failed with an SQL syntax error
* issue: Send the values Acknowledge, Add Comment, Schedule Downtime and Schedule
Check need from the host and service command dialogs; they were rejected for
missing parameters. Sticky, notify and persistent are options in the dialog and
a downtime takes a duration in minutes
* security: Escape quotes in values the front end places inside HTML attributes
(host and service names, user name)
* issue: Show the host, service, comment and event log grids and the summary and
process tables; the front end read a response shape the controllers do not
send, so every table rendered empty
* issue: Document the NPC Global Commands realm that gates remote commands
* issue: Add the minimum_importance column to npc_contacts on upgrade (the upgrade
named a table that does not exist) and remove the settings of deleted users
(the cleanup read a column the query does not return)
* issue: Skip perfdata entries without a numeric value (label=U) in perfdata.php and
cli.php instead of printing PHP warnings into the data input output; both
scripts now share one parser in the base controller
* issue: Declare the hostgroup and unhandled request properties on the base
controller; assigning them raised the PHP 8.2 dynamic property deprecation
* issue: List each service once; the service grid joined the hostgroup members
unconditionally, so a host in several hostgroups repeated its services and
inflated the row count
* issue: Read only the newest host check when fetching host perfdata; the query
returned every stored check and the oldest one was used
* issue: Name the failing class and method in the generic error returned by the
controller logger; the message was built from an invalid property read
* issue: Remove dispatcher actions that cannot work with the single parameter array
they are called with (getSetting, isMapped, mapHost, getSettings, checkHostExists)
* issue: Pass the searchFields and data request values to the controllers as
JSON; the search string sanitizer removed their quotes, so grid searches and the
host import selection never applied; the selection lookup also called a
non-static method statically
* security: Copy only the known request parameters onto the NPC controllers and keep
`start` and `limit` within bounds (at most 1000 rows per request)
* security: Accept the state-changing NPC actions (Nagios commands, comment
deletion, data input creation, host import, graph mapping, settings save) only
as POST requests so Cacti's CSRF token check applies to them
* issue: Load the base controller class in npc.php; every NPC page failed with
a missing Controller class before
* security: Check the NPC command realm before the command file settings so users
without the realm no longer see the configured command file path
* issue: Call the Nagios controller on an instance when deleting all host or service
comments; the static call raised an Error on PHP 8
* security: Accept only known tab names for the `tab` request variable of the NPC
frame; the raw value was written into an inline script block
* issue: Look up the stored perfdata with the plain object ID when creating a
perfdata data input method; the shell-quoted value never matched, so no output
fields were created
* security: Require the Devices realm for the Nagios host import and the Data Input
Methods realm for creating a perfdata data input method; both were open to any
user with NPC access
* security: Keep the host import address cache as JSON in the Cacti cache directory
instead of a PHP file in the system temp directory that was included on each call
* security: Quote the description, address and template arguments of the Nagios host
import command and reject non-numeric template IDs
* dev: Relocate the schema provisioning function `npc_setup_tables()` from
`setup.php` into `includes/database.php` (loaded via `require_once`) to align
with the fleet schema/includes convention; behavior is unchanged
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ Next, there are a few NPC settings to add/update. Goto Console > Configuration >
Settings > NPC.

* **Remote Commands** - Check the checkbox to enable remote commands. At the
moment this is system wide so all users that can access NPC will be able to
execute commands.
moment this is system wide. A user also needs the "NPC Global Commands" realm
(granted in the user's Realm Permissions) to execute commands; the realm is
checked by name, so no page of that name exists in the plugin directory.

* **Nagios Command File Path** - Add the path to the Nagios command file. It
will be something like /usr/local/nagios/var/rw/nagios.cmd
Expand Down
16 changes: 1 addition & 15 deletions cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,21 +137,7 @@ function getPerfData() {
$obj = new $class;
$results = $obj->getPerfData($objectId, $hostname, $serviceName);

$perfParts = explode(' ', $results[0]['perfdata']);

$output = '';

foreach ($perfParts as $perf) {
if (preg_match('/=/', $perf)) {
preg_match('/(\S+)=([-+]?[0-9]*\.?[0-9]+)/', $perf, $matches);
if (preg_match('/^iso./', $matches[1])) {
$matches[1] = 'output';
}
$output .= $matches[1] . ':' . $matches[2] . ' ';
}
}

print $output . "\n";
print $obj->perfdataOutput(isset($results[0]['perfdata']) ? $results[0]['perfdata'] : '') . "\n";
}

function listHosts() {
Expand Down
10 changes: 7 additions & 3 deletions controllers/cacti.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ function getHostnames() {
* @return string json encoded results
*/
function addDataInputMethod($params) {
if (!api_user_realm_auth('data_input.php')) {
return(json_encode(array('success' => false, 'msg' => __('You do not have permission to create data input methods.', 'npc'))));
}

$name = 'NPC - Perfdata - ' . $params['host'];

if (isset($params['service'])) {
Expand All @@ -87,9 +91,9 @@ function addDataInputMethod($params) {
$class = 'NpcHostsController';
}

$object_id = cacti_escapeshellarg($params['object_id']);
$object_id = $params['object_id'];

$input = "php <path_cacti>/plugins/npc/perfdata.php --type=$type --id=$object_id 2> /dev/null";
$input = "php <path_cacti>/plugins/npc/perfdata.php --type=$type --id=" . cacti_escapeshellarg($object_id) . " 2> /dev/null";

$save = array();
$save['hash'] = $this->generateHash();
Expand All @@ -102,7 +106,7 @@ function addDataInputMethod($params) {
$obj = new $class;
$results = $obj->getPerfData($object_id);

$perfParts = explode(';', $results[0]['perfdata']);
$perfParts = explode(';', isset($results[0]['perfdata']) ? $results[0]['perfdata'] : '');

foreach ($perfParts as $perf) {
if (preg_match('/=/', $perf)) {
Expand Down
11 changes: 6 additions & 5 deletions controllers/comments.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ function deleteAllHostComments() {
require_once("plugins/npc/controllers/nagios.php");

$seen = array();
$nagios = new NpcNagiosController();

$results = $this->flattenArray($this->comments(null, 'o.objecttype_id = 1'));

Expand All @@ -128,7 +129,7 @@ function deleteAllHostComments() {
'command' => 'DEL_ALL_HOST_COMMENTS',
'host_name' => $host
);
NpcNagiosController::command($params);
$nagios->command($params);
$seen[$host] = 1;
}
}
Expand All @@ -146,6 +147,7 @@ function deleteAllServiceComments() {
require_once("plugins/npc/controllers/nagios.php");

$seen = array();
$nagios = new NpcNagiosController();

$results = $this->flattenArray($this->comments(null, 'o.objecttype_id = 2'));

Expand All @@ -158,7 +160,7 @@ function deleteAllServiceComments() {
'host_name' => $host,
'service_description' => $service,
);
NpcNagiosController::command($params);
$nagios->command($params);
$seen[$host][$service] = 1;
}
}
Expand Down Expand Up @@ -241,7 +243,6 @@ function comments($id = null, $where = '') {
' . $whereClause,
$params);

$offset = ($this->currentPage - 1) * $this->limit;

$results = db_fetch_assoc_prepared('SELECT i.instance_name,
o.name1 AS host_name,
Expand All @@ -258,8 +259,8 @@ function comments($id = null, $where = '') {
LEFT JOIN npc_hosts h ON c.object_id = h.host_object_id
' . $whereClause . '
ORDER BY ' . $orderBy . '
LIMIT ?, ?',
array_merge($params, array($offset, $this->limit)));
' . $this->limitClause(),
$params);

return($results);
}
Expand Down
62 changes: 60 additions & 2 deletions controllers/controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,22 @@ class Controller {
*/
var $searchString = null;

/**
* Restricts a listing to the hostgroup with this alias
*
* @var string
* @access public
*/
var $hostgroup = null;

/**
* When set, a service listing leaves out acknowledged problems
*
* @var string
* @access public
*/
var $unhandled = null;

/**
* json encoded list of fields to search
*
Expand Down Expand Up @@ -253,6 +269,48 @@ function __construct() {
$this->config_type = isset($config_type) ? $config_type : 1;
}

/**
* limitClause
*
* The LIMIT clause for the current page. The values are written into the
* statement because Cacti's database layer sends bound values as strings,
* and MariaDB and MySQL reject a quoted LIMIT value.
*
* @return string for example " LIMIT 0, 25"
*/
function limitClause() {
$limit = max(1, (int) $this->limit);
$offset = max(0, ((int) $this->currentPage - 1) * $limit);

return ' LIMIT ' . $offset . ', ' . $limit;
}

/**
* perfdataOutput
*
* Reduces Nagios perfdata to the "label:value" pairs a Cacti data input
* method reads. Entries without a numeric value, such as label=U, are left
* out.
*
* @param string $perfdata The perfdata of a check
* @return string for example "load1:0.52 load5:0.4 "
*/
function perfdataOutput($perfdata) {
$output = '';

foreach (explode(' ', (string) $perfdata) as $perf) {
if (preg_match('/(\S+)=([-+]?[0-9]*\.?[0-9]+)/', $perf, $matches)) {
if (preg_match('/^iso./', $matches[1])) {
$matches[1] = 'output';
}

$output .= $matches[1] . ':' . $matches[2] . ' ';
}
}

return $output;
}

function jsonOutput($results=array()) {
if (!$this->numRecords) {
$this->numRecords = count($results);
Expand Down Expand Up @@ -322,7 +380,7 @@ function flattenArray($array=array()) {
* @return string The appended where clause
*/
function searchClause($where, $fieldMap, &$params = array()) {
$fields = json_decode(stripslashes($this->searchFields));
$fields = json_decode(stripslashes((string) $this->searchFields));
if (!is_array($fields)) {
return $where;
}
Expand Down Expand Up @@ -416,7 +474,7 @@ function logger($level, $class, $method, $message) {

// If this was an error send a generic response to the client
if ($level == 'error') {
return(json_encode(array('success' => false, 'msg' => __('An error occurred in %s. See error logs for detail.', $class -> $method, 'npc'))));
return(json_encode(array('success' => false, 'msg' => sprintf(__('An error occurred in %s. See error logs for detail.', 'npc'), $class . '::' . $method))));
}
}

Expand Down
5 changes: 2 additions & 3 deletions controllers/downtime.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ function downtimeHistory($id = null, $where = '') {
' . $whereClause,
$params);

$offset = ($this->currentPage - 1) * $this->limit;

$results = db_fetch_assoc_prepared('SELECT i.instance_name,
o.name1 AS host_name,
Expand All @@ -192,8 +191,8 @@ function downtimeHistory($id = null, $where = '') {
LEFT JOIN npc_instances i ON d.instance_id = i.instance_id
' . $whereClause . '
ORDER BY d.scheduled_start_time DESC, d.actual_start_time DESC, d.actual_start_time_usec DESC
LIMIT ?, ?',
array_merge($params, array($offset, $this->limit)));
' . $this->limitClause(),
$params);

return($results);
}
Expand Down
9 changes: 5 additions & 4 deletions controllers/hosts.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ function summary() {
function getPerfData($id) {
return db_fetch_assoc_prepared('SELECT perfdata
FROM npc_hostchecks
WHERE host_object_id = ?',
WHERE host_object_id = ?
ORDER BY hostcheck_id DESC
LIMIT 1',
array($id));
}

Expand Down Expand Up @@ -156,7 +158,6 @@ function hosts() {
$params);

/* Paginated results */
$offset = ($this->currentPage - 1) * $this->limit;

$hosts = db_fetch_assoc_prepared(
'SELECT i.instance_name,
Expand All @@ -177,8 +178,8 @@ function hosts() {
LEFT JOIN npc_host_graphs hg ON hs.host_object_id = hg.host_object_id
WHERE ' . $where . '
ORDER BY ' . $orderBy . '
LIMIT ?, ?',
array_merge($params, array($offset, $this->limit)));
' . $this->limitClause(),
$params);

return $hosts;
}
Expand Down
21 changes: 13 additions & 8 deletions controllers/layout.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@ function drawFrame($params) {

$npc_base = $config['url_path'] . 'plugins/npc/';

$current_tab = 'dashboard';
if (isset_request_var('tab')) {
$current_tab = get_request_var('tab');
}

$tabs = array(
'dashboard' => __('Dashboard', 'npc'),
'hosts' => __('Hosts', 'npc'),
Expand All @@ -25,6 +20,16 @@ function drawFrame($params) {
'process' => __('Process Info', 'npc'),
);

/* only a known tab name is ever written into the page */
$current_tab = 'dashboard';
if (isset_request_var('tab')) {
$requested_tab = get_request_var('tab');

if (is_string($requested_tab) && isset($tabs[$requested_tab])) {
$current_tab = $requested_tab;
}
}

/* draw the tabs */
print "<div class='tabs' style='margin-bottom:0px;'><nav><ul role='tablist'>\n";

Expand All @@ -44,9 +49,9 @@ function drawFrame($params) {
baseUrl: '<?php echo $npc_base; ?>',
npcUrl: '<?php echo $npc_base; ?>npc.php',
portletRefresh: <?php echo (int) read_config_option('npc_portlet_refresh'); ?>,
dateFormat: '<?php echo addslashes(read_config_option('npc_date_format')); ?>',
timeFormat: '<?php echo addslashes(read_config_option('npc_time_format')); ?>',
nagiosUrl: '<?php echo addslashes(read_config_option('npc_nagios_url')); ?>',
dateFormat: '<?php echo addslashes((string) read_config_option('npc_date_format')); ?>',
timeFormat: '<?php echo addslashes((string) read_config_option('npc_time_format')); ?>',
nagiosUrl: '<?php echo addslashes((string) read_config_option('npc_nagios_url')); ?>',
userName: <?php echo json_encode(db_fetch_cell_prepared('SELECT username FROM user_auth WHERE id = ?', array($_SESSION['sess_user_id']))); ?>,
hostIcons: '<?php echo read_config_option('npc_host_icons'); ?>',
serviceIcons: '<?php echo read_config_option('npc_service_icons'); ?>',
Expand Down
5 changes: 2 additions & 3 deletions controllers/logentries.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,14 @@ function getLogs() {
WHERE ' . $where,
$params);

$offset = ($this->currentPage - 1) * $this->limit;

$results = db_fetch_assoc_prepared('SELECT i.instance_name, l.*
FROM npc_logentries l
LEFT JOIN npc_instances i ON l.instance_id = i.instance_id
WHERE ' . $where . '
ORDER BY l.entry_time DESC, l.entry_time_usec DESC
LIMIT ?, ?',
array_merge($params, array($offset, $this->limit)));
' . $this->limitClause(),
$params);

$results = $this->flattenArray($results);

Expand Down
Loading
Loading