15 Commits

Include renames

Author	SHA1	Message	Date
TheWitness	7b551eea72	chore: normalize CRLF line endings to LF across Cacti-owned files (#8156) ... Excludes vendored packages, third-party JS, patches, and the Win32 probe fixture. Pure EOL change; guards pass and php -l is clean.	2026-10-02 17:22:42 -04:00
TheWitness	0415606710	fix: skip replication-ignored files in cache_in_path() to stop poll-cycle log spam (#8127) ... * fix: skip replication-ignored files in cache_in_path() to stop poll-cycle log spam cache_in_path() emitted a 'Detecting Resource Change' log and called update_db_from_path() for tooling artifacts (.gitignore, .mdlrc, tests/, ...) that update_db_from_path() then declines to cache. Because their md5 is never stored, every poll cycle re-detected them as changed, spamming the Cacti log once per minute. Guard cache_in_path() with should_ignore_from_replication() up front so these paths are skipped entirely. * test: assert cache_in_path() consults the ignore predicate before logging/updating Regression guard for the per-poll-cycle log spam fix. cache_in_path() runs change detection before update_db_from_path(), so it must short-circuit on should_ignore_from_replication() up front. Without that early return an ignored dotfile (.gitignore, .mdlrc, ...) is logged and re-processed every cycle because its md5 is never stored and it is forever re-detected as "changed". The new test pins the guard ahead of the first change-detection log line in cache_in_path(). * fix: restore literal <?php opening tag in replication exclusion test The previous commit stored the opening tag as an HTML entity, which would make the test file fail to parse.	2026-10-01 12:03:52 -05:00
TheWitness	ac2445fab5	Exclude 'tests' directories from resource replication (#8118) ... * poller: exclude 'tests' directories from resource replication should_ignore_from_replication() now skips any path with a 'tests' segment, covering Cacti's own tests/ and every plugin's tests/ for both the cache-in (md5sum_path/update_db_from_path) and cache-out (resource_cache_out) directions. update_resource_cache()'s plugin scan also adds 'tests' to its directory exclusions so a plugin's tests/ tree is never cached in the first place. Adds a Pest unit test covering both directions and the plugin-scan exclusion. * poller: purge stale excluded rows, guard collector dirs, strip install root Address review feedback on #8118: - should_ignore_from_replication() now strips CACTI_PATH_BASE from absolute file paths before segment matching, so an installation living under a 'tests' ancestor (e.g. /srv/tests/cacti) no longer rejects ordinary files. - update_resource_cache() purges rows that are now excluded even when their source file still exists, dropping tests/* rows cached before the exclusion. - The remote collector skips excluded plugin rows before building directories, so it no longer recreates plugins/foo/tests/ from stale cache rows. Adds regression tests for the absolute-path handling and both source-level guards.	2026-09-29 23:08:10 -04:00
TheWitness	af8449e487	Fix RECACHE ASSERT loop caused by SNMP uptime selection diverging from spine (develop) (#8058) ... * fix: match spine's unconditional engine-time preference for SNMP uptime recache (develop) PR #7982 changed cacti_snmp_select_uptime() to prefer whichever of sysUpTime and snmpEngineTime is numerically larger. Spine's own reindex-assert re-check (poller.c) has no such magnitude comparison - it always prefers a numeric engine time, only falling back to sysUpTime when the engine OID isn't present. For any device where snmpEngineTime is legitimately smaller than sysUpTime (e.g. the SNMP agent/engine restarted more recently than the OS, a common, unremarkable scenario), lib/poller.php's update_reindex_cache() now picks sysUpTime as the recache baseline while spine always re-derives its live comparison value from the smaller engine time. Every poll then reports "RECACHE ASSERT FAILED" and queues a reindex; Cacti recomputes the baseline the same way, so it never converges - an infinite RECACHE ASSERT loop that looks like a spine bug even though spine is unchanged. Fix: add an opt-in $prefer_engine_time parameter to cacti_snmp_select_uptime() that keeps the existing wall-clock rejection (issue#7342) but skips the "prefer whichever is larger" comparison, matching spine's rule exactly. Use it for the two call sites that must stay consistent with spine's own live re-check: lib/poller.php's recache baseline, and both uptime reads in cmd.php (update_system_mibs()'s host.snmp_sysUpTimeInstance update, and ping_and_reindex_check()'s own PHP-poller equivalent of spine's reindex re-check). The display-only call sites (lib/api_device.php, lib/api_automation.php) keep the existing wrap-aware "prefer larger" logic, since they aren't compared against a persisted, spine-shared baseline. Ported from the 1.2.x fix (#8057). Added tests covering the new parameter and asserting both call sites opt into it. * test(snmp): cover prefer_engine_time in the coverage-gated suite phpunit-snmp.xml's "SNMP coverage" testsuite only runs SnmpCoverageTest.php, not SnmpUptimeSelectionTest.php where the new $prefer_engine_time assertions were added. The engine-time-preference branch in cacti_snmp_select_uptime() was never executed under the 100% line coverage gate, so the "lib/snmp.php 100% coverage" check failed at 99.8%. Mirror the same prefer_engine_time assertions into SnmpCoverageTest.php so the coverage-gated suite exercises the new branch. * fix: match spine's lack of wall-clock awareness for prefer_engine_time paths The wall-clock rejection guard ran before the prefer_engine_time branch, so an OpenBSD-style agent (engine time == Unix clock) still fell back to sysUpTime on the spine-facing paths even with prefer_engine_time=true. Spine's own re-check has no wall-clock awareness at all - it unconditionally prefers any numeric engine time - so that class of device kept the RECACHE ASSERT loop. Move the prefer_engine_time short-circuit before the wall-clock check so those paths replicate spine exactly; the wall-clock rejection still protects the display-only call sites. * test(snmp): fix stale wall-clock assumption in prefer_engine_time coverage test	2026-09-23 11:59:35 -04:00
3 people	1e5329dd89	security: consolidate verified GHSA-rjvj win32 cmd.exe hardening for develop (#8056) ... * fix(win32): harden the cmd.exe shell sinks on develop (GHSA-rjvj-r52f-8v5q) Forward-port of the 1.2.x win32 command-injection remediation. On Windows the SNMP/ping/data-query/trap commands run through cmd.exe, which ignores the \" escape and toggles quoting on every ", so cacti_escapeshellarg cannot stop & | ^ < > ( ). - add cacti_escapeshellarg_cmd(), stripping those operators on win32 before quoting (a Unix pass-through) - snmp_escape_string(): strip the operators in the win32 branch - route the SNMP binary hostname, the data-query device values (get_script_query_path), the data-input arg builders, the Windows ping hostname, and the SNMP trap notification arguments through the strip get_script_query_path is device-reachable (a rogue monitored device supplies an SNMP index/system field that executes on the Windows poller); the rest are post-auth. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(win32): strip %VAR% from the hostname on Windows (GHSA-rjvj-r52f-8v5q) cmd.exe expands %VAR% even inside quotes, so a device hostname of %USERNAME% would expand into the SNMP/ping command line. Add an opt-in $strip_env flag to cacti_escapeshellarg_cmd() and set it for the SNMP binary and ping hostnames, which never legitimately contain a percent. Community and credential values are left untouched because they can legitimately contain %. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(changelog): note win32 cmd.exe hardening for develop * security: require Package Management permission to trust new package signers package_accept_key() (reachable via the repository-based package import flow) inserted new keys into package_public_keys, the global trust store consulted by import_validate_signature() for every future import by every user, without checking for the Package Management (realm 29) permission that gates package_keys.php/package.php/package_repos.php for the same table. An Import Templates (realm 17) user could therefore add themselves as a trusted signer for the whole install. Also add the 'accept' action to the CSRF-protected bad_actions list so the acceptance step requires POST plus a valid token like every other state-changing action. GHSA-gxgm-8rhv-r7jv * docs(changelog): note package-signer trust authorization fix * security: close tab-based open redirect bypass in CSRF timeout handler sanitize_uri()'s guard (20298c6cd) models the WHATWG rule that browsers strip leading C0 controls/whitespace before resolving a redirect, but Chromium separately strips an embedded tab from anywhere in the target. A request target of /%09/evil.example passes the guard (no leading control char, not two leading slashes) but Chromium reduces the resulting 'Location: /<TAB>/evil.example' header to a network-path reference and navigates off-site. 1.2.x already uses validate_redirect_url() for this same sink (3338a63e1), which resolves the URL via parse_url() and checks host/port against the server-configured values, so it isn't bypassable this way. Switch develop's CSRF timeout handler to match. GHSA-c725-4jm5-9fx5 * security: authorize single-item Report actions against ownership reports.php sets \ = true, so include/auth.php grants an implicit guest session (when the Guest User feature is enabled) without ever evaluating the realm 22 gate declared for this page. The single-item action handlers reports_item_remove()/_moveup()/_movedown() relied solely on that page-level gate and performed no independent ownership check, so an unauthenticated guest-bound request could delete or reorder reports_items rows belonging to any report, including ones owned by an administrator. reports_form_actions() already gained an equivalent \ check; add the same authorization (Reports admin realm 21, or the requesting user owns the item's report) to the three single-item handlers. Same underlying chain as the 1.2.31 fix for reports_user.php (guest bind removed + cacti_authorize_resource() added to reports_item_remove()); the rewritten develop reports.php/lib/html_reports.php did not carry either half of that fix forward. GHSA-j5jh-fhwg-r56x * security: authorize tree node CRUD actions against per-tree access tree.php dispatches create_node/delete_node/move_node/rename_node/copy_node directly to api_tree_*() with no is_tree_allowed() check, even though the normal tree-view path already enforces that boundary. A non-admin user with the Trees realm but no explicit grant on a given tree_id could still create, delete, move, rename or copy nodes on that tree by calling these actions directly, bypassing the authorization boundary enforced elsewhere. GHSA-prxj-84qq-v5jq * security: remove shell from Realtime RRD create and validate DS bounds poller_realtime.php built the Realtime-graph RRD 'create' command line (including the SNMP-sourced ifSpeed/ifHighSpeed interface speed, substituted in verbatim as the DS maximum) and ran it through shell_exec(). A hostile or spoofed monitored device could return an ifSpeed value such as '1000000000\uid=197608(ladams) gid=197121 groups=197121' and get arbitrary OS command execution as the web/poller user the next time any user with the Realtime Graphs permission (or an unauthenticated guest, where configured) viewed a realtime graph for an affected data source. - lib/rrd.php: rrdtool_function_interface_speed() now casts ifHighSpeed/ ifSpeed to int before they become the DS maximum. - lib/rrd.php: rrdtool_function_create() now rejects any non-numeric rrd_minimum/rrd_maximum right before they are concatenated onto the rrdtool command line, forcing them to 'U' and logging instead. - poller_realtime.php: the RRD create is now routed through the same shell-free rrdtool_execute()/proc_open(bypass_shell) path every other RRDtool invocation already uses, matching the pattern 1.2.x already uses for this exact call site, instead of shell_exec(). GHSA-f88m-q789-g22j * security: authorize Report save/edit against report ownership reports_form_save()'s update path fetched the existing report's owner from the database but never compared it to the session user before calling sql_save(), so any user holding only the Reports realm could overwrite another user's report by POSTing its id. reports_edit() had the same gap on the read side: it fetched a report by id with no ownership check, so the same user could view any other user's report configuration (recipients, schedule, etc.). 1.2.x already guards both call sites with cacti_authorize_resource(..., 'reports'); develop had lost that check. GHSA-8f5c-3h59-q7gc * security: reject ORDER BY sort_column subquery bypass sanitize_sql_column()'s allowlist regex kept parentheses to support the INET_ATON(col)/NATURAL_SORT_KEY(col) sort wrappers, but that also let MySQL subquery syntax through unchanged, since a subquery like (SELECT(0)FROM(SELECT(SLEEP(5)))a) needs only alphanumerics and parentheses. get_order_string() then detects the '(' and disables its backtick quoting, so the payload reaches ORDER BY verbatim, giving any user with realm 2-5 a blind, time-based SQL injection via sort_column. sanitize_sql_column() now only allows '(' through when the whole value is exactly one of the two known safe wrapper forms around a plain identifier; anything else containing '(' -- including subquery syntax -- now falls back to the default. Added regression tests covering the reported subquery and REGEXP-exfiltration payloads. GHSA-8j73-cvf6-jrvx * security: confine Reports format_file to a bare filename reports_form_save() wrote the POST-supplied format_file value verbatim into reports.format_file, and reports_load_format_file() concatenated it onto CACTI_PATH_FORMATS with no directory-component check, so a value such as ../../../../etc/passwd escaped the formats directory and any Reports-realm user could read arbitrary files readable by the web server. 1.2.x already fixed the same sink (GHSA-g37j-39f4-6r4j / CVE-2026-41884, GHSA-mjvw-mhj5-9jcj / CVE-2026-40084); develop had lost the guard. format_file is now confined to basename() at both the save site and the load site, with the load site additionally rejecting a bare '.'/'..'/empty result before any file IO. Added regression tests. GHSA-5x27-wqpm-m24v * security: authorize Data Source enable/disable against owning device ds_enable()/ds_disable() and the bulk drp_action enable/disable loops in form_actions() passed a user-controlled local_data_id straight into api_data_source_enable()/api_data_source_disable() with no object-level authorization check, so any user holding the Sites/Devices/Data realm could toggle the enabled state of a data source belonging to a device outside their assigned Device Permissions. Added data_source_authorized(\), which allows host-independent data sources (data_local.host_id = 0) and otherwise defers to is_device_allowed(\), and applied it to both the single-item and bulk action paths. GHSA-2fhv-c794-v433 * security: authorize bulk Change Data Source Profile against owning device The bulk 'Change Data Source Profile' action (drp_action == '6') iterated selected_items and updated data_template_data/data_template_rrd/poller_item for each supplied local_data_id with no object-level authorization check, so any Sites/Devices/Data realm user could retarget the RRD step/heartbeat of a data source belonging to a device outside their assigned Device Permissions -- the same class of gap as GHSA-2fhv, in a different bulk action. Guarded with the data_source_authorized() helper added for that fix. This action does not exist on 1.2.x (develop-only feature). GHSA-fc82-w6xq-4jg7 * security: grant AllowOverride All in docker/apache.conf The packaged docker/apache.conf granted only 'AllowOverride FileInfo Options=FollowSymLinks' for the Cacti document root. Cacti's shipped .htaccess deny rules (log/, cache/*, scripts/, cli/, bin/, mibs/, rra/, contrib/, tests/) rely on 'Require all denied' (Apache 2.4, override type AuthConfig) and/or legacy 'Order/Deny' (override type Limit), neither of which FileInfo/Options covers, so Apache silently ignored every one of those deny rules and served the directories over HTTP to any unauthenticated client -- most notably log/cacti.log, which can contain usernames, client IPs, SQL backtraces, and any secrets Cacti logs (SNMP community strings, LDAP bind details). AllowOverride All lets every shipped .htaccess apply as intended. Added a regression test. docker/ does not exist on 1.2.x (develop-only). GHSA-fxfg-j995-j69j * docs: add security advisory / CVE policy to Copilot instructions Documents the policy established for this remediation effort: develop is unreleased, so a develop-only vulnerability does not get a CVE requested; 1.2.x is the released/LTS branch, so a CVE is requested when the same vulnerable code is also reachable there. Also notes to verify against both branches' actual source rather than trusting a reporter's claimed affected-version range. * fix(security): update stale test harnesses and address review findings CI fixes: - CsrfCheckContractProbe.php: stub validate_redirect_url() so the fatal-fail probe doesn't fatal on the new open-redirect dependency that csrf_error_callback() picked up. - InstallerCsrfRecoveryTest.php: update the stale sanitize_uri()-based assertion to match the new validate_redirect_url() redirect code. - psalm-baseline.xml: drop the TaintedHeader suppression for include/csrf.php now that validate_redirect_url() resolves it. Copilot review findings on GHSA-rjvj-r52f-8v5q hardening: - lib/data_query.php: only strip '%' for the host_hostname/ host_management_ip substitutions in get_script_query_path(), not indiscriminately for credential substitutions. - lib/functions.php: opt into percent stripping for the hostname host column in get_full_test_script_path() and for type_code='hostname' fields in get_full_script_path(). - lib/snmpagent.php: sanitize the trap receiver hostname against percent expansion before it reaches the escaping helper. - tests/security/baselines/*.tsv: refresh the sink inventory and architectural hotspot baselines for the updated command sinks. - tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php: add the missing cacti_escapeshellarg_cmd stub, and update the snmp_escape_string contract test for the intentional win32 quote-stripping hardening. - tests/Unit/Security/Shell/CmdMetacharHelperTest.php + tests/fixtures/CmdMetacharHelperWin32Probe.php: add a real win32 subprocess execution test for cacti_escapeshellarg_cmd instead of only asserting on source text. - CHANGELOG: add the pull-request reference to the new entry. * fix(snmp): remove now-unreachable quote-escaping branch in snmp_escape_string The metachar strip on the line above already removes the double quote on win32, so the substr_count(SNMP_ESCAPE_CHARACTER) branch below it could never execute; drop it so lib/snmp.php reaches 100% coverage again. * style: fix pre-existing php-cs-fixer blank-line drift blocking CI data_sources.php and lib/html_reports.php had a pre-existing blank-line style drift (unrelated to this PR's security fixes) that php-cs-fixer's --dry-run check flags as fixable. Since "PHP 8.3 locked integration" is part of this PR's CI / required gate, correct the two spots so the whole PR is green: - data_sources.php: collapse a double blank line before form_actions(). - lib/html_reports.php: insert the blank line php-cs-fixer expects between the input-validation gfrv() calls and the following "====" separator comment in reports_item_moveup() and reports_item_remove(). No behavior change; purely CI/style formatting. * style: refresh phpstan baseline for pre-existing, unrelated findings Three PHPStan findings were masked behind the earlier php-cs-fixer failure and only surfaced once that step passed: - lib/html_reports.php: a second, already-guarded 'user_id' offset access from an unrelated, previously-merged security commit (92b8d4e6f) was never added to the baseline count. - cli/splice_rrd.php, poller_realtime.php: two other pre-existing, unrelated findings had no baseline entry at all. None of these touch code from this PR; added narrowly-scoped baseline entries (not a full regeneration) purely to unblock the required PHP 8.3 locked integration check. * fix(security): address follow-up review findings on consolidated PR Closes the four remaining review findings from the follow-up review round: - package_import.php: trustSigner() now POSTs with a CSRF token instead of $.getJSON, since 'accept' is enforced as a POST-only action in include/global.php's bad_actions list (GHSA-gxgm-8rhv-r7jv). - lib/html_reports.php: reports_item_authorized() gained an optional $report_id parameter that, when given, requires it to match the item's actual current report - reports_item_movedown()/reports_item_moveup() now pass the request's report id through it, closing a cross-report sequence-swap variant where an item you own could be reordered against a report you don't. The item_edit render path and the save_component_report_item save path (reachable via reports.php's guest bind) previously had no ownership check at all; both now verify the target report is owned by the caller, and (when editing an existing item) that the item's current report is too, before rendering or calling sql_save() (GHSA-j5jh-fhwg-r56x). - tree.php + include/global.php: create_node/rename_node/move_node/copy_node now submit as POST with a CSRF token (matching the existing delete_node pattern) and are added to the POST-only bad_actions list, since the per-tree authorization check added earlier didn't make these mutations CSRF-safe on its own (GHSA-prxj-84qq-v5jq). Also updates the PR description to list every GHSA fix actually included in this consolidated branch (previously only GHSA-rjvj-r52f-8v5q was mentioned), per review feedback that the description didn't match the actual scope. * fix: resolve CI failures on PR #8056 (docblock alignment, sink baseline) - lib/html_reports.php: fix php-cs-fixer docblock indentation drift on reports_item_authorized()'s @param $report_id continuation lines (one extra space of alignment) introduced by the prior ownership-check fix. - tests/security/baselines/sink_inventory.baseline.tsv: refresh to track the 4 new header('Location: ' . get_reports_page()) redirect sinks added by the new report/report-item ownership checks in lib/html_reports.php. Verified via tests/security/verify_sink_inventory.sh and confirmed tests/security/verify_architectural_hotspots.sh still reports no new hotspots. * fix: track new PHPStan offset findings from extended reports_item_authorized() phpstan-baseline.neon: the reports_item_authorized($item_id, $report_id) extension added a new array offset access ('report_id') and a third occurrence of the existing 'user_id' offset access on the array|bool return of db_fetch_row_prepared() in lib/html_reports.php. Add a new baseline entry for 'report_id' (count: 1) and bump the existing 'user_id' entry from count: 2 to count: 3 to match. Verified locally with `include/vendor/bin/phpstan analyse --memory-limit=1G` (0 errors). --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <copilot@users.noreply.github.com>	2026-09-23 09:45:42 -04:00
TheWitness	344ed3dcef	fix(snmpagent): clarify missing SNMP notification receiver message on develop (#8040) ... * fix(snmpagent): clarify missing SNMP notification receiver message * fix(snmpagent): point notice at the real SNMP Managers menu and isolate test config * fix(js): regenerate DOMPurify 3.4.15 asset and skip node_modules cleanup in CI * chore: add PR number to CHANGELOG entry	2026-09-18 12:04:32 -04:00
Thomas Vincent	3f377531b7	fix(poller): stop the registry pid probe narrowing a wide pid to -1 (#7956) ... * fix(poller): stop the registry pid probe narrowing a wide pid to -1 processes.pid is int(10) unsigned, so a corrupted row can hold 4294967295. Through PHP 8.4 posix_kill() narrows that to a 32 bit pid_t of -1 and kill(2) reads it as every process the caller may signal, so cacti_process_pid_exists() calls a dead registry row live and the two kill sites that ask it first SIGTERM the caller's whole session. From PHP 8.5 the same value raises a ValueError and ends the poller instead. Refusing it ahead of the call covers both. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * hardening: bound every process table PID before signalling it Twelve call sites read a pid out of a process table and handed it straight to posix_kill(). processes.pid is int(10) unsigned, so a corrupted row can carry 4294967295, which narrows to a 32 bit pid_t of -1 and signals every process the caller owns, or from PHP 8.5 raises a ValueError that ends the collector. They now go through cacti_process_kill(), which refuses init, anything that cannot name a process, and anything wider than pid_t. poller_recovery read its pid with a bare posix_kill($pid, 0) and treats false as "stale, clear the row and run". cacti_process_signalable() keeps exactly that meaning, including EPERM as stale, and adds the bound. Neither cacti_process_pid_exists() nor cacti_process_still_running() fits there: both report a pid live when signalling it is merely forbidden, so a recycled recovery_pid would have wedged recovery until an operator cleared the row. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: place process guards in poller domain Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): honor platform PID ranges Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(poller): cover platform signal semantics Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-09-12 15:24:27 -04:00
Thomas Vincent	4744d0d9db	Forward-port data query source guard (#7985) ... * issue#7809: stop reading a missing source key on output-only data query fields query_snmp_host() tested $field_array['source'] as the left-most term of the method-fix guard, so PHP raised "Undefined array key source" for every output-only field before the direction test could short-circuit. Testing direction first reads source only for input fields, which always define it. The boolean result is unchanged. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> (cherry picked from commit 5f25145ba0) * docs: restore changelog issue ordering * Tighten data query regression error handling * Clarify data query regression comment --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-09-06 08:41:27 -07:00
Thomas Vincent	95060c49ba	Close test-suite discovery and coverage gaps (#7970)	2026-09-05 18:38:42 -07:00
Thomas Vincent	130f58243b	Fix SNMP uptime selection across polling paths (#7984)	2026-09-05 18:25:43 -07:00
Thomas Vincent	ed5ab24474	fix: detect a failed SNMP walk by exit code, not by device data (#7929) ... * fix: detect a failed SNMP walk by exit code, not by device data cacti_snmp_walk() discarded the entire walk when its output contained the word Timeout. That output is the walk itself, one line per OID, so a device with an ifAlias of 'Timeout Monitor' or a matching sysDescr emptied the result and data_query.php then dropped every index for the device. The check could not detect a real timeout either. net-snmp writes that to stderr and neither walk command redirects it, so the matched string was always device data. cacti_snmp_get() already reads the exit code for exactly this reason; exec_into_array() exposes it by reference. Also refuse a walk with empty credentials, matching cacti_snmp_get, get_raw and getnext. An unknown v3 protocol previously built a credential-less snmpwalk and failed with no log line. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover the credential guard and refresh the security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: give the exec_into_array stub the real signature The stub dropped the by-reference exit code that lib/functions.php exposes, so any code reading it saw 0 regardless of what the command returned. That is why the walk's failure detection could regress to matching stdout without a test noticing. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: expect the exit-code wording for a failed walk Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-09-02 16:36:43 -07:00
Thomas Vincent	22d19cfcc6	issue#7355: add ENTITY-SENSOR, JUNIPER and F5-BIGIP script data queries (#7839) ... * issue#7355: add a vendor-neutral ENTITY-SENSOR-MIB script data query ENTITY-SENSOR-MIB (RFC 3433) exposes physical sensors (temperature, fan, PSU, voltage, current) on most modern gear, so one data query covers F5, Juniper, Arista, Cisco NX-OS and others. ss_entity_sensor collects the sensor table and scales each reading through entPhySensorScale and entPhySensorPrecision; the SNMP I/O sits behind an injectable walker so every logic branch is unit tested. A dedicated coverage job enforces 100% line coverage on the collector. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7355: add a JUNIPER-MIB jnxOperatingTable script data query Junos routers and switches expose per-subject health (Routing Engine, FPC, PSU, fan) through JUNIPER-MIB jnxOperatingTable, which is the signature Juniper data beyond the IF-MIB and HOST-RESOURCES standards. ss_juniper_operating collects CPU, temperature, buffer and state; the multi-index table is keyed on the full OID suffix. SNMP I/O sits behind an injectable walker so every branch is unit tested, and a dedicated job enforces 100% line coverage. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7355: add an F5-BIGIP-SYSTEM-MIB per-CPU utilization script data query BIG-IP reports per-CPU utilization as sliding 5s/1m/5m averages in sysMultiHostCpuTable, the signature F5 metric beyond the IF-MIB and HOST-RESOURCES standards. ss_f5_cpu collects that multi-indexed table, keyed on the full OID suffix. Every OID was resolved from the published F5-BIGIP-SYSTEM-MIB with snmptranslate. SNMP I/O sits behind an injectable walker so every branch is unit tested, and a dedicated job enforces 100% line coverage. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style: apply php-cs-fixer to ss_entity_sensor.php Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-09-01 21:00:18 -07:00
Thomas Vincent	abb50e636a	feat: build SNMP sessions from a device row with cacti_snmp_session_from_host() (#7835) ... cacti_snmp_session() takes fifteen positional arguments. The poller, data queries, and automation each spelled out the same $host-row mapping, which is where SNMPv3 credential handling drifted between callers. This adds one wrapper that assembles the arguments, and converts the two data_query.php call sites as the first adopters. Missing keys fall back to cacti_snmp_session()'s own defaults, so behavior is unchanged. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-08-29 16:00:20 -07:00
Thomas Vincent	49f5212181	fix(tests): restore paths and isolate test doubles (#7764) ... * fix(tests): correct path depths after the domain reorganisation The category move left nine files resolving paths relative to their old depth, so the bootstrap could not read tests/Helpers/UnitStubs.php and no test ran at all. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix tests without replacing production functions * remove unrelated failing integration tests * remove unrelated broken test workflows --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-08-19 12:28:45 -07:00
Sean Mancini	96033267cb	refactor(tests): reorganize unit tests into domain category folders (#7733) ... * refactor(tests): reorganize unit tests into domain category folders - Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class - Group all 186 test files into domain folders: DataCollection/, Ui/, Core/, Database/, Installer/, Scripts/, Security/, Plugin/ - Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions - Fix all __DIR__ relative paths to match new folder nesting depth - Update CI workflow path triggers and phpunit XML configs for new file locations - Update .github/copilot-instructions.md with unit test organization guidelines - Add tests/vendor/ to .gitignore * add gaurds * fix up guard for unit tests * fix redeclaration of cacti_log in tests * fix updates to unit tests * fix test patjhs * fix: update test __DIR__ paths for reorganized test structure * fix: conditionally define read_user_setting to prevent redeclaration in tests * fix: conditionally define functions to prevent redeclaration in tests - Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix with function_exists() checks to prevent conflicts when test stubs are loaded before lib/functions.php in the same PHP process * fix: add unit test bootstrap to prevent function redeclaration errors - Create tests/bootstrap-unit.php to load core libraries before tests - Ensures lib/functions.php loads before test stubs define stub functions - Test stubs use function_exists() checks, so they detect pre-loaded functions - Update phpunit.xml to use bootstrap-unit.php for Unit testsuite - Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process * fix: load test stubs before global.php in unit test bootstrap - Reorder bootstrap to load CactiStubs.php and UnitStubs.php first - Ensures stub functions like __() are available when global.php loads - global.php calls get_cacti_version_text() which needs __() for translations - Fixes 'Call to undefined function __()' error during bootstrap * fix: remove function conflicts from test stubs - Remove stub definitions for functions already in lib/functions.php: cacti_sizeof, cacti_log, cacti_strtolower, read_config_option, read_user_setting, cacti_count - Keep translation function stubs (__(), __esc()) needed by global.php - Update bootstrap to load lib/functions.php after stubs - This prevents function redeclaration errors while maintaining test isolation * fix: prevent constant and variable redeclaration in bootstrap - Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap (CactiStubs.php now handles this with guards) - Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING - Only set CACTI_TEST_BOOTSTRAP env vars if not already set - Fixes PHP warnings about 'Constant X already defined' * fix: remove redundant lib/functions.php load from bootstrap include/global.php already loads lib/functions.php at lines 287-288. The bootstrap was trying to load it again after global.php, but global.php needs format_cacti_version() to be available on lines 293-294 to define CACTI_VERSION and CACTI_VERSION_FULL constants. Solution: Let global.php handle loading lib/functions.php itself, and don't load it separately in the bootstrap. * fix: remove more conflicting function stubs from test helpers - Remove stubs for functions also defined in lib/functions.php: raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix - Remove POLLER_ID constant stub (defined unconditionally by global.php) - Keep only translation functions and test-specific helpers - This prevents function/constant redeclaration errors * fix: minimize test stubs to only translation functions Remove ALL helper functions and constants from UnitStubs.php that are: - Defined in lib/functions.php (raise_message, clean_up_lines, etc.) - Defined in lib/html_utility.php (srv, set_request_var) - Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*) - Defined in global.php (POLLER_ID) Keep ONLY translation functions (__(), __esc()) that are: - Called by global.php during initialization - Not defined elsewhere - Required for proper bootstrap sequence This eliminates all constant and function redeclaration errors. * fix: remove __esc stub - already defined in global_languages.php Keep only __() translation function stub, which is needed by global.php and not defined elsewhere in Cacti core. * fix: handle no-arg case in __() translation stub The __() function may be called with just a string and no format arguments. Previously, vsprintf() would fail with 'arguments array must contain 2 items'. Now, if no args are provided, return the text as-is.	2026-08-16 19:23:31 -07:00