280 Commits
Author SHA1 Message Date
dependabot[bot]andThomas Vincent 2be8a7f5fe deps: update Composer tooling and resolve PHPStan 2.3 diagnostics (#8357)
* deps(deps-dev): bump the composer group with 2 updates

Bumps the composer group with 2 updates: [phpstan/phpstan](https://github.com/phpstan/phpstan-phar-composer-source) and [rector/rector](https://github.com/rectorphp/rector).


Updates `phpstan/phpstan` from 2.2.17 to 2.3.0
- [Commits](https://github.com/phpstan/phpstan-phar-composer-source/commits)

Updates `rector/rector` from 2.6.7 to 2.7.0
- [Release notes](https://github.com/rectorphp/rector/releases)
- [Commits](https://github.com/rectorphp/rector/compare/2.6.7...2.7.0)

---
updated-dependencies:
- dependency-name: phpstan/phpstan
  dependency-version: 2.3.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: composer
- dependency-name: rector/rector
  dependency-version: 2.7.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: composer
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix: resolve PHPStan 2.3 dependency upgrade diagnostics

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-16 08:49:23 -04:00
Thomas Vincent 2b6e96b23a style(php): [develop] wrap CLI and poller calls without changing execution (#8303)
* style(php): wrap CLI and poller calls without changing execution

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(ci): review security inventories after poller wrapping

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(boost): compare handoff source contracts by PHP tokens

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style(php): wrap poller constant strings without changing values

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style(php): wrap poller concatenations with token-preserving source contracts

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(cli): assert CDEF repair queries after constant folding

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: verify PHP formatting preserves parsed behavior

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-14 00:01:26 -07:00
TheWitness f79dc15c08 Port #8166 SNMP credential cache + procedural SNMPv3 (with #8171/#8172 follow-ups) to develop (#8174)
* Port #8166 SNMP credential cache + procedural SNMPv3 to develop

Ports the shared SNMPv3 credential cache and procedural SNMPv3 routing from the
1.2.x feature (merged as #8166) onto develop, adapted to develop's refactored
SNMP layer. Parts of the 1.2.x change that do not apply to develop are omitted:
the cacti_exec() idle-timeout rewrite (develop has no cacti_exec()), the PHP
floor bump and composer/doc changes (develop is already 8.3+), and the cmd.php
max_oids batching (develop's cmd.php already collects SNMP in-process through
persistent sessions, so the per-item spawn the batching removed does not occur).

- lib/cache.php: new cross-process \Cacti\Cache\SharedCache (file + POSIX
  shared-memory + process-local tiers, optional AES-256-GCM sealing). Loaded via
  lib/snmp.php (the Cacti\Cache\ namespace is not PSR-4 autoloaded). The crypto
  helpers it seals with already live in develop's lib/auth.php, loaded by
  global.php, so no duplicate was added.
- lib/snmp.php: snmp_get_method() now routes SNMPv3 get/getnext to the
  procedural snmp3_*() calls when the running PHP supports the configured
  auth/privacy tokens and no context/engine id is set (SHA-224/384 and
  AES-192/256[C] stay on the binary before PHP 8.6); the three single-get
  functions gain the procedural v3 path with \Throwable handling; adds the
  credential-cache subsystem, protocol-option/MD5-DES helpers,
  snmp_native_protocol(), snmp_format_agent() and cacti_snmp_get_multi(); the
  binary v3 auth builder consults the cache when enabled.
- poller.php / script_server.php: prime / load the shared cache.
- poller_maintenance.php: daily out-of-band cache rebuild on every collector
  (per-poller lastrun marker).
- lib/api_device.php / lib/utility.php: bump snmp_cred_version only on v3
  credential add/change (and propagate the token to the affected remote poller).
- include/global.php: resolve and create the private cache/ directory.
- include/global_settings.php: add 'Enable Credential Cache' and 'Enable MD5 and
  DES for SNMPv3' settings (both off by default); default new-device v3
  auth/privacy to SHA/AES instead of legacy MD5/DES.
- host.php / automation_snmp.php: drop MD5/DES from the pickers when disabled and
  default new records to SHA/AES on upgraded installs.
- include/config.php.dist: document $config['cache_dir'].
- tests/Unit/Core/Cache/SharedCacheTest.php: SharedCache behaviour coverage.
- tests/security/baselines: baseline the two new lib/cache.php sinks.

* Incorporate #8171: binary fallback for unsupported SNMPv3 protocols

Layers PR #8171 onto the develop port so an SNMPv3 device whose auth/privacy
protocols the running PHP's ext-snmp cannot service natively (e.g. AES-256
before PHP 8.6) no longer fatals the poller or device-edit screen.

- lib/snmp.php: add CactiSnmpBinarySession, a session stand-in exposing the
  get/getnext/walk/close interface the cacti_snmp_session_*() helpers use and
  delegating each call to cacti_snmp_*() -> the Net-SNMP binary.
  cacti_snmp_session() returns it when php-snmp is absent or the v3 protocol is
  not natively serviceable, so cmd.php/host.php collect these devices via the
  binary on pre-8.6 and switch to the in-process path on 8.6 with no caller
  change. v1/v2 snmpget/snmp2_get now go through snmp_format_agent(). The
  ERRNO_TIMEOUT check tolerates php-snmp being absent. Drop the vendored
  classSNMP.php compatibility shim and its include (and the Yandex attribution).
- cmd.php: open_snmp_session() returns the down sentinel when a host is already
  marked down this run, instead of returning an unset (null) []
  that cacti_snmp_session_get() then dereferenced (TypeError).
- lib/ping.php: ping_snmp() runs the availability check through the procedural
  cacti_snmp_get()/getnext() path so an unsupported protocol falls back to the
  binary instead of raising a ValueError in SNMP::setSecurity().

* Incorporate #8172: retire include/vendor/phpsnmp, relocate into lib

Follow-up to #8171 on the develop port. After classSNMP.php was removed, the
include/vendor/phpsnmp/ directory held only Cacti-owned files; this retires it.

- lib/snmp.php: the ext-snmp subclass that carried Cacti's bulk_walk_size /
  value_output_format (previously vendor/phpsnmp/extension.php) is now declared
  directly as CactiSnmpNativeSession, guarded by CACTI_PHP_SNMP; the include and
  the 'use phpsnmp\SNMP' alias are gone, and new SNMP(...) / SNMP::VERSION_*
  now reference the native \SNMP and the new subclass.
- lib/mib_parser.php: moved from include/vendor/phpsnmp/mib_parser.php;
  lib/mib_cache.php includes it via __DIR__.
- Remove include/vendor/phpsnmp/extension.php and index.php (directory retired).
- tests: SnmpCoverageTest uses \SNMP::ERRNO_TIMEOUT now that phpsnmp\SNMP is
  gone; drop the stale include/vendor/phpsnmp/ entry from the vendor-policy
  allowlist.

develop's sink baseline tracks constant-based includes differently than 1.2.x,
so no baseline refresh was needed here.

* Address PR #8174 review: snmp_get_method params, v3 walk, getnext agent, cache bump, CHANGELOG

- lib/snmp.php: restore $php_snmp as the 6th snmp_get_method() parameter and move
  the new $auth_proto/$priv_proto after it (callers pass them by name), so the
  positional $php_snmp used by SnmpCoverageTest and others keeps working.
- lib/snmp.php: keep every SNMPv3 operation except get/getnext on the Net-SNMP
  binary - the native walk branch implements only v1/v2, so a v3 walk reaching
  the procedural gate would have silently returned empty when path_snmpbulkwalk
  is missing.
- lib/snmp.php: cacti_snmp_getnext() v1/v2 branches now use snmp_format_agent()
  like get/get_raw, so an IPv6 literal / non-default port is bracketed.
- lib/api_device.php: bump snmp_cred_version only when the host's v3 membership
  or one of the cached credential fields actually changed, instead of on every
  save of a v3 device.
- CHANGELOG: move the #8171 entry into the issue block and the #8166 entries into
  the feature block, matching the established section ordering.

* Fix CI: unbracket IPv4/DNS SNMP targets, inline auth-cache SQL columns, apply phpcsfixer standard

* Refresh sink inventory baseline after short-array normalization

* Restore lib/snmp.php 100% coverage after aligning v3 routing to 1.2.x

- update the three SnmpCoverageTest assertions that still encoded develop's
  pre-port behavior: v3 snmp_get_method now returns PHP, an unsupported-
  protocol v3 session returns the binary-delegating session object, and the
  v3 cacti_snmp_get path runs natively
- add a v3 noAuthNoPriv user to the snmp-coverage snmpd fixture so native
  snmp3_get is exercised deterministically
- add tests covering the ported subsystems to 100%: shared auth cache,
  binary-delegating session, protocol pickers, v3 credential args,
  cacti_snmp_get_multi, and the cacti_get_snmpv3_auth cache path
- snmp_auth_cache_load uses a resettable process guard so its enabled and
  disabled branches are both coverable in a single process

* Resolve phpstan errors in the relocated SNMP MIB parser and ported snmp.php

- lib/mib_parser.php: type the MibParser properties, add parameter/return
  types, guard file_get_contents()/preg_split() against false, correct the
  parse_bracket_token delimiter params (string, not integer), and drop the
  unreachable return after trigger_error(E_USER_ERROR) - now phpstan clean
- lib/mib_cache.php: drop the redundant isset($mp->oids) guard (the typed
  property is always initialized; $mp->mib is the real check)
- lib/snmp.php: type the CactiSnmpBinarySession and CactiSnmpNativeSession
  properties/methods and pass bool to snmp_set_quick_print()
- .phpstan.neon: ignore greaterOrEqual.alwaysFalse alongside the existing
  greater.alwaysFalse (the PHP_VERSION_ID >= 80600 forward-compat gate)
- regenerate phpstan-baseline.neon for the irreducible ext-snmp stub
  strictness (snmp3_get literal-union protocols, int<-1,max> timeouts) in
  the ported procedural v3 path, the new lib/cache.php shmop stubs, and the
  $config array-shape offsets in include/global.php

* Make the v3 getter catch coverage net-snmp-build-independent

The DES-rejection trick that drove the procedural snmp3_get()/snmp3_getnext()
try/catch blocks only works on net-snmp builds compiled without DES; CI's
Debian net-snmp accepts DES, so those catches went uncovered (98.3%).

- extend the existing native_get injection seam (already on cacti_snmp_get)
  to cacti_snmp_get_raw, cacti_snmp_getnext, and cacti_snmp_get_multi, and
  drive the catch blocks with a throwing native getter in the test
- mark the seam-less cacti_snmp_session() setSecurity catch with
  @codeCoverageIgnore (it only throws for a protocol the running net-snmp
  rejects, which is not portably reproducible in the fixture)

* Exclude ext-snmp/PHP-8.6-gated defensive branches from snmp coverage

Two branches in lib/snmp.php are unreachable on the PHP 8.3 + ext-snmp
coverage runner and were only "covered" locally via opcache dead-code
elimination:

- snmp_get_method()'s !function_exists('snmp3_get') fallback (ext-snmp
  always provides snmp3_get)
- snmp_php_v3_protocols_supported()'s PHP_VERSION_ID >= 80600 block
  (SHA-224/384 and AES-192/256 tokens are only accepted on PHP 8.6+)

Mark both with @codeCoverageIgnore so the 100% gate is deterministic
across runners regardless of opcache settings.

* Account for the #8166 SNMPv3 credential-cache gate query in the batching test

poller_update_poller_cache_from_buffer() now runs one SELECT per flush to
decide whether the flush touched SNMPv3 poller items before bumping the shared
credential-cache version (matching 1.2.x). UpdateDataQueryCacheBatchingIntegrationTest
counted 4 statements per flush; update it to 5 (and 3 per-row flushes to 15,
1 batched flush to 5). The guard's intent - batching issues fewer statements
than per-row flushes - still holds.

* Port #8175: group SNMP options under an SNMP Behavior section

Mirror the 1.2.x settings reorganization (#8175) on develop. develop already
keeps the SNMP options in a dedicated SNMP tab, so add an "SNMP Behavior"
spacer there, order the behavior options as md5/des, credential cache, max
get size, bulkwalk fetch size, and move "Disable increasing OID Check" out of
the Poller tab to sit last in that group. Display-only; no settings added or
removed and no behavior change. Adds the "SNMP Behavior" heading to cacti.pot.

* fix: complete dependabot htmx 2.0.10 to 2.0.11 bump

Dependabot bumped package.json and package-lock.json to htmx.org 2.0.11
(PR 8179) but left the vendored include/js/htmx.js, its version marker,
and the pinned SRI constant at 2.0.10. The htmx-filters CI workflow runs
npm ci, which regenerates include/js/htmx.js from the 2.0.11 package, so
HtmxTest's runtime sha384 no longer matches the HTMX_2_0_10_SRI constant
and the integrity assertion fails.

Complete the bump: regenerate the vendored include/js/htmx.js to the
2.0.11 build, bump include/js/htmx.js.version to 2.0.11, rename
HTMX_2_0_10_SRI to HTMX_2_0_11_SRI with the recomputed sha384 hash, and
update the version/integrity assertions in tests/Unit/Ui/Html/HtmxTest.php
and tests/HandOff/HtmxLoaderHandOffTest.php. package.json and
package-lock.json match the values dependabot already landed on develop.

* settings: move SNMP Agent Support under the SNMP Behavior spacer

Relocate the enable_snmp_agent setting (SNMP Agent Support Enabled) so it
is the first item under the SNMP Behavior spacer instead of sitting in the
Poller General group, grouping it with the other SNMP behavior options.

* Address PR #8174 review: keep SHA-224/384 on the binary, honor port on transport targets

Resolves the two still-valid Copilot review findings on lib/snmp.php that were
left unresolved (only marked outdated after later commits shifted their lines).

- snmp_php_v3_protocols_supported(): drop SHA224/SHA384 from the native auth
  allowlist. ext-snmp's SNMP::setSecurity()/snmp3_*() only map MD5/SHA/SHA256/
  SHA512 - there is no SHA-224/SHA-384 mapping on any build, including PHP 8.6
  (verified against php-src ext/snmp/snmp.c), so routing those tokens to the
  procedural path raised a ValueError instead of using the binary. They now
  always fall back to the Net-SNMP binary. The AES-192/256 privacy tokens that
  ext-snmp does add on 8.6 stay gated behind PHP_VERSION_ID >= 80600.
- snmp_format_agent(): a udp/tcp transport-qualified target that omits a port
  now has the configured non-default port appended (udp6:[2001:db8::1] + 1161 ->
  udp6:[2001:db8::1]:1161), matching the Net-SNMP binary path. Targets that
  already carry a port, and unix: sockets (no port), are left untouched.
- SnmpCoverageTest: cover the new transport-target branches; lib/snmp.php stays
  at 100%.
2026-10-05 10:18:41 -04:00
TheWitnessandTheWitness 3ab4c69fb7 PHPStan Level 8 - Phase 1 Conversion (#8135)
* PHPStan Level 8 - Phase 1 Conversion

Reduces the real (no-baseline) PHPStan level 8 error count from ~2,724 to 425
and regenerates phpstan-baseline.neon accordingly (8,941 -> 1,921 lines).

Core type-safety changes
- lib/database.php: narrow db_fetch_row()/db_fetch_row_prepared() to return
  array (was bool|array) and db_fetch_assoc()/db_fetch_assoc_prepared() to
  return array (was mixed); the _prepared workers coerce the connection-failure
  path to an empty array so the declared type is honest. empty()/cacti_sizeof()
  checks are unaffected.
- lib/database.php: narrow $db_conn guards from is_object() to instanceof PDO in
  the transaction/quote/insert-id helpers so the PDO calls resolve.
- lib/mib_cache.php: the fluent MibCache methods mib()/object()/table()/row()
  now return self instead of a bare object so chained calls type-check.

Type-safety casts and guards (mechanical, behaviour-preserving)
- Cast string|false/|null builtin results feeding string parameters
  (json_decode, base64_encode, trim, htmle, preg_replace, explode, etc.).
- Cast int|string / int|false values feeding int id/pid parameters.
- Guard resource|false fopen()/popen() handles before fwrite/fclose/fread, and
  wrap foreach over array|false/list|false results with `?: []`.

Genuine bugs found and fixed
- cli/add_tree.php / lib/api_tree.php: site tree nodes could not be created via
  the CLI - the --site-id option was never parsed, $nodeTypes had no 'site'
  entry (passing null to api_tree_item_save(int $type) -> TypeError), and
  api_tree_item_save() never persisted site_id. All three fixed.
- tree.php: leaves_exist() is typed : int but returned db_fetch_assoc_prepared()
  (an array of rows) for a SELECT COUNT(*); now uses db_fetch_cell_prepared()
  and returns the scalar count.
- lib/mib_cache.php: $entries_per_object was a float (division / ceil) used as
  an array key (PHP 8.1 implicit float-to-int key deprecation); now cast to int.

Config
- .phpstan.neon: add notIdentical.alwaysTrue and booleanAnd.alwaysFalse to the
  existing always-true/false ignore family (same "PHPStan gets confused"
  dynamic-constant cause already covered by identical.alwaysTrue,
  notEqual.alwaysTrue, booleanAnd.leftAlwaysTrue, etc.).
- phpstan-baseline.neon: regenerated; the remaining 425 entries are the
  untouched long tail for Phase 2.

* Remove redundant @phpstan-ignore comments

Follow-up cleanup for the level 8 Phase 1 work. Removes 14 inline
@phpstan-ignore comments that no longer suppress any reported error:

- cli/repair_database.php: `@phpstan-ignore booleanAnd.alwaysFalse` is now
  covered by the global booleanAnd.alwaysFalse ignore added to .phpstan.neon.
- 13 others (host.php, include/global.php, include/session.php,
  lib/api_automation.php x2, lib/functions.php x2, lib/html_utility.php x2,
  lib/installer.php, lib/package.php, lib/reports.php, lib/rrd.php) no longer
  match any reported error after the type-safety fixes (e.g. the
  db_fetch_row_prepared() return-type narrowing removed the offset-assign
  errors the api_automation ignores were suppressing).

Verified with reportUnmatchedIgnoredErrors enabled: 0 unmatched ignores
remain and the full level 8 run still reports 0 errors.

* ci: satisfy validator ratchet and refresh security sink baselines

- lib/api_tree.php: give the new tree-item site_id a real numeric
  constraint ('^[-0-9]+$', matching html_reports.php) instead of an
  empty regex, so the validator constraint ratchet passes without
  bumping its baseline.
- tests/security/baselines: refresh the three sink/hotspot signatures
  whose code text changed from the (string) casts added in this branch
  (lib/poller.php setsid, cli/rrdresize.php json_encode, lib/import.php
  simplexml_load_string).

* ci(js): allow DOMPurify 3.4.16 and update sync_js pin test

The dependabot bump (#8134) moved package.json/package-lock.json to
DOMPurify 3.4.16 on develop but left tests/js/sync_js.test.mjs pinned to
3.4.15, so the PR merge commit fails the 'DOMPurify is pinned' contract.
Bring package.json/package-lock.json to 3.4.16 (matching develop) and
update the pin test to expect 3.4.16.

* test: update sql.php null-handling assertion for the (string) cast

This branch changed scripts/sql.php from trim(\ ?? '') to
trim((string) \) (the phpstan-safe form that coerces both null and
false from shell_exec()). Update SqlScriptsTest's null-return assertion
to match the new cast instead of the removed '?? ''.

* fix: address Copilot review - ownership IDs, fopen guards, mib_cache bound

- chown()/chgrp(): pass integer UID/GID from fileowner()/filegroup()
  (guarding the false lookup) instead of string-casting them, which made
  them resolve account/group names like '1001' and fail. Covers
  lib/api_device.php, lib/boost.php and poller_maintenance.php (x2).
- fopen() failure now short-circuits instead of reporting success:
  lib/package.php returns false, lib/clog_webapi.php and utilities.php
  render an error, and snmpagent_mibcachechild.php skips the cache write
  and sentinel unlink so a concurrent writer's lock is preserved.
- lib/mib_cache.php: keep the float quotient as the loop bound and cast
  only the array offset, so a non-even entries/columns split no longer
  truncates the loop and drops a cached row.

* revert: keep db_fetch_* returning false on error; baseline Level 8 instead

Per review, restoring the intentional fail-safe: db_fetch_row[_prepared]()
return bool|array and db_fetch_assoc[_prepared]() return mixed again, so a
genuine SQL error (not just a missing connection) is signalled as false and
the === false checks in cdef_is_in_use(), boost_process_* and aggregate.php
work as before. Dropped the is_array() ? : [] coercion.

Level 8 stays green by regenerating phpstan-baseline.neon to capture the
re-introduced bool|array/mixed call-site errors (the ~1,850 the narrowing
had eliminated). Tightening those callers to drop them from the baseline is
Phase 2 work.

---------

Co-authored-by: TheWitness <copilot@users.noreply.github.com>
2026-10-02 10:08:56 -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
Thomas VincentandTheWitness 3c20f4ce7e Harden Boost data handoffs and graph cache writes (#7729)
* fix(boost): harden data handoffs and cache writes

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Define $updates_ok when there is nothing to flush

PHPStan level 6 also flags the poller ownership check, which is the POLLER_ID
constant case the file already annotates above.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Use a line comment to match the fixer

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Record the boost process columns in the audit schema

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: resolve includes through Cacti path constants

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Fix Boost handoff review concerns

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: seed settings in Boost database fixture

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: refresh SQL interpolation baseline

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: record Boost handoff hardening

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: stop boost discarding a cycle and wedging a shard

Three defects in the handoff hardening.

A rejected ownership handoff returned false, which the poller reads as boost
having consumed the data, so it skipped the RRD write. The poller_output rows
were already deleted by then, so a stale poller_item copy on a remote poller
silently lost the whole cycle. Return true instead and let the poller write
directly, matching the database failure path.

Any non-OK RRDtool response retained the shard, and the retry then replayed rows
already written, which RRDtool rejects as an illegal update time. That failed
again on every pass, so the archive tables accumulated and boost never drained.
Treat an at-or-behind timestamp as applied, since the sample is in the RRD;
genuine errors still retain the shard.

The two new poller_output_boost_processes columns passed 'null' where
db_install_update_table reads 'NULL', so NOT NULL was never emitted and upgraded
installs diverged from cacti.sql.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: spell the NULL column key so upgrades emit NOT NULL

db_add_column() reads $column['NULL'] and PHP array keys are case sensitive, so
the thirteen definitions in the 1.3.0 upgrade that passed a lowercase 'null' key
had the flag discarded and created nullable columns. Fresh installs built from
cacti.sql got NOT NULL, upgraded installs did not, and Cacti's own schema audit
reports the difference.

The same file already spells the key in upper case 144 times, and the wrapper's
own docblock example uses the upper case form.

Guard the class with a test over every upgrade file rather than only the one
that was wrong.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: assert the NULL key plainly

toContain() takes needles rather than a failure message.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): close remaining handoff review gaps

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): guard nullable graph output

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): keep in-flight completion rows through the 1.3.0 upgrade

The truncate only existed so the run_child UNIQUE key could build over rows
that predate run_id; deleting just those rows leaves a running boost its
completion records.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(boost): drop the poller ownership gate

Its boost_poller_on_demand() call sits behind the boost_redirect early return
that poller.php forces on for every multi-collector install, and its recovery
call turns a device deletion on main into a recovery that never brings the
collector back online.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): bound atomic cache write allocations

Guard upgrade source reads so failures report clearly, write graph cache data in fixed-size chunks, and exercise a multi-chunk payload.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): support Windows cache replacement

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(boost): expand handoff failure coverage

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(boost): support ANSI_QUOTES upgrade checks

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): stop staging after chunk failure

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(recovery): use numeric poller status

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(sql): support ANSI_QUOTES in recovery paths

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): preserve cache on replacement failure

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): retain readable cache permissions

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): repair completion schema at runtime

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(recovery): bound timestamp batch fetches

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): satisfy runtime schema analysis

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(recovery): restore status after transfer failure

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(boost): rely on unit bootstrap helpers

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): refresh generated cache secret

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): normalize cache path separators

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(boost): fail clearly on unreadable fixtures

* fix(boost): name ignored duplicate sample keys

The staging warning only said rows were dropped, so a retry could not be told from a colliding writer.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Lack of pre-commit hook execution allowed lint failure

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-09-12 20:17:35 -04:00
Thomas Vincent 130f58243b Fix SNMP uptime selection across polling paths (#7984) 2026-09-05 18:25:43 -07:00
Thomas Vincent ac0b2c9515 Fix confirmed poller, database, automation, and UI defects (#7861)
* issue#7024: do not delete still-running poller_time rows on cycle start

The overrun path deleted every poller_time row for the poller, including the
in-flight ones it had just counted, so overrunning processes lost their
tracking. Restrict the delete to completed rows.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7028: restore identifier and connection safety in the db layer

_db_replace interpolated caller-supplied column keys as backtick identifiers
without escaping, and db_commit_transaction checked @@in_transaction on the
default connection rather than the one passed in. Both were correct on 1.2.x
and regressed on develop.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7341: size the CSV export maxrows to the requested span

The export hardcoded --maxrows=10000, truncating wide date ranges regardless
of the data source profile step.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7421: offset automation_get_next_host through the 32-bit integer

The hand-rolled octet walk subtracted the wrong magnitude per octet and
emitted out-of-range octets for ranges wider than a /16.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7030: skip the poller_output ENGINE=MEMORY conversion when already applied

The output-table refresh ran ALTER TABLE poller_output ENGINE=MEMORY every
cycle even though the swapped-in copy already inherits the engine, adding a
metadata-locking DDL to the hot path. Probe the current engine and convert
only when needed.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7342: prefer snmpEngineTime only when it covers system uptime

update_system_mibs preferred snmpEngineTime (x100) over sysUpTime whenever it
was present, so an SNMP engine that restarts independently of the system (for
example OpenBSD snmpd) made the stored uptime drop, which reads as a reboot
and forces a perpetual reindex with mass 'U' results. Capture both values and
use snmpEngineTime only when it is at least the system uptime, which still
covers the 32-bit sysUpTime wrap.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7403: scale lmsensors readings in the get path like the query path

The get path returned the raw milli-unit SNMP counter while the query path
divided voltage and temperature by 1000 (and corrected negative voltages), so
a get gave values 1000x too large. Apply the same scaling before returning.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7029: propagate the reconnected handle back to the caller

db_check_reconnect() closed the dead connection, opened a new one, and
returned true without giving the new handle back, so the db_execute_prepared
retry ran its query against the connection that was just closed. Take $db_conn
by reference and assign the fresh handle; the one caller that passed a literal
false now passes a variable.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: refresh sink inventory baseline

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: refresh architectural hotspots baseline

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7474: return valid after correcting the theme to selected_theme

is_valid_theme() corrected an invalid theme to selected_theme but left $valid
false, so callers saw the theme as invalid even though it had been fixed.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7462: allow SELECT aliases as sortable automation columns

site_name and host_template_name are aliases (s.name AS site_name), valid in
ORDER BY but not real columns, so api_automation_column_exists() rejected them
and the site-name sort silently failed.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7456: bind the OS value in the automation OS filter

The OS filter built 'os = ?' but bound the network value, so filtering by OS
matched on the wrong input.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7491: filter host_template_id against its own column

html_make_device_where() compared h.location to the host_template_id value
instead of h.host_template_id.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7479: apply the hostname and description filters in get_hosts

The REST get_hosts() honored host_id/poller/site/template/location but ignored
the hostname and description filters its callers pass; add them as bound LIKE.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* issue#7475: authorize a direct client-IP poller match before the hostname guard

A poller configured by IP with no hostname-based pollers was rejected because
the empty-hostname guard ran before the direct-match check, even though the
client address exactly matched a poller's configured address.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Guard two divisors that PHP 8 makes fatal

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: consolidate functional fix changelog

* test: cover consolidated functional fixes

* docs: identify consolidated functional fixes

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-01 21:01:28 -07:00
Thomas VincentandTheWitness db39178735 fix: harden the poller_item cache and Boost RRD staging subsystems (#7538)
* fix(boost): forward still-open-round archive rows instead of dropping them

boost_process_poller_output() correctly excluded rows from an
in-progress poll round when building the RRD write, but the
archive-table cleanup right after deleted every row for the
local_data_id anyway, with no time filter. Those rows now get
INSERT IGNORE'd into the live poller_output_boost table before the
archive table is cleared, so a still-open round's data survives.

Also fixes the duplicate-record guard in the same function: $last_item
was initialized once before the results loop and never reassigned, so
the comparison against it could never be true. It is now updated after
each processed row.

Fixes #7519
Fixes #7523

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): gate archive-table drop on shard completeness

poller_boost.php decided whether to drop a run's archive tables using
SELECT SUM(status) FROM poller_output_boost_processes, an aggregate
over whatever completion rows landed. If one child crashed before
writing its completion row, the sum from the surviving shards was
still positive and every archive table for the run got dropped,
including the crashed shard's unprocessed data.

The drain loop already computes boost_completed_children() vs
$expected_children to detect a crashed child; the drop now uses that
same completeness check instead, and leaves the archive tables in
place with a warning when it is not met.

Fixes #7520

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(dsstats,rrdcheck): apply Boost's startup barrier to launch loops

dsstats_launch_children()/rrdcheck_launch_children() fork children via
exec_background() (non-blocking) and the callers then wait with a bare
while ($running = ..._processes_running($type)) { sleep(2); }. If the
parent evaluates that before any child has registered in the processes
table, $running reads 0 and the loop body never runs, treating "not
started yet" as "already done" -- the same race Boost's
boost_all_children_registered() barrier was added to close.

dsstats_launch_children()/rrdcheck_launch_children() now return the
launched count, and dsstats_wait_for_children()/
rrdcheck_wait_for_children() wait for the running count to reach it
(bounded by a deadline) before trusting the drain loop's running-count
poll.

Fixes #7526

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(poller): consolidate poller_item deletes behind a shared helper

Extract poller_item_delete_for_data_source() and poller_item_delete_for_host()
in lib/poller.php and migrate the existing local+remote delete pairs in
lib/api_device.php and lib/api_data_source.php onto them, so the
local-delete-plus-remote-mirror pattern lives in one place instead of eight.

Refs #7535

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): mirror poller_item deletes to the remote poller on handoff

api_data_source_change_host(), data_query_remove_disabled_items(), and
data_source_to_data_template() each moved or removed a data source's
poller_item row locally but never on the remote Data Collector that
still owned it, so that Collector kept polling an item it no longer
should. All three now resolve the owning remote poller before the
local change and purge the row there too via the shared helper.

Fixes #7521
Fixes #7524

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): correct poller_item join semantics and cron reset in cmd.php

poller_item and host share ~13 identically-named snmp_*/hostname columns;
SELECT * across the join let PDO FETCH_ASSOC silently collapse them to
host's live values instead of poller_item's cached copies. Select pi.*
plus explicitly aliased host/site disabled columns instead. The now
already-fetched $poller_items also makes the separate script-server-call
COUNT(*) re-join redundant, replaced with an in-PHP count. Also apply the
same cron_interval-aware rrd_next_step reset used by the ranged branch to
the --allhost branch, which previously ignored it.

Fixes #7522, #7525, #7528.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): thread known poller_id through update_poller_cache() callers

update_poller_cache() re-derived poller_id via a host/data_local JOIN on
every call, even when the caller already had it: repopulate_poller_cache()
and push_out_data_input_method() both already select h.poller_id and
discarded it, push_out_host() has the host row cached per data source, and
api_device_enable_devices()/update_poller_cache_from_query() compute it
once outside their loops. Add an optional $poller_id argument (and honor
an already-present 'poller_id' array key) so these callers skip the
redundant query.

Fixes #7527.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): scope global aggregate scans and make process-leveling sargable

Three poller.php queries (total_snmp_ports and the mutually-exclusive
active_profiles pair) had no poller_id filter, unlike the rest of the
file's central-only blocks, so every remote poller redundantly recomputed
and overwrote the same global config options; the snmp_port scan is also
an unindexed full table scan. Gate them behind poller_id == 1, with remote
pollers reading back the value the central poller already stored.

Also rewrite the process-leveling $sql_where's rrd_next_step predicate
from the non-sargable `rrd_next_step - $poller_interval <= 0` to the
algebraically equivalent `rrd_next_step <= $poller_interval`, restoring
use of the poller_id_rrd_next_step index.

Fixes #7529, #7530.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): batch update_data_query_cache()'s poller cache writes

Each changed data source called update_poller_cache($data_source, true),
which flushes immediately via poller_update_poller_cache_from_buffer() --
its own present=0 UPDATE, INSERT ON DUPLICATE KEY UPDATE, and stale-row
DELETE -- on every iteration. Re-indexing a host where every port changed
issued up to 3x as many statements as ports instead of one batched flush.

Accumulate changed rows across the loop and call update_poller_cache()
with commit=false, then flush once via poller_update_poller_cache_from_buffer()
after the loop, matching the batched pattern already used by
push_out_data_input_method().

Fixes #7531.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): add USING BTREE to poller_output_boost_local_data_ids indexes

MEMORY-engine indexes default to HASH unless USING BTREE is specified,
unlike the sibling poller_output/poller_output_boost tables in the same
schema file. boost_output_rrd_data()'s ORDER BY/LIMIT query and
boost_process_local_data_ids()'s ranged DELETE both need to scan
local_data_id in order, which a HASH index can't serve, forcing a
filesort on every boost worker pass.

Verified against a real MariaDB 11 instance: EXPLAIN on the ORDER BY
query shows "Using filesort" with the old HASH schema and no filesort
with USING BTREE, using the primary key for ordering instead.

Also aligns boost_prepare_process_table()'s dead CREATE TABLE IF NOT
EXISTS fallback with the corrected schema, and adds an upgrade step
that rebuilds both indexes as BTREE on existing installs (checked via
information_schema.STATISTICS, since db_install_add_key() only compares
index columns, not algorithm, and would otherwise skip an existing HASH
index with the right column list).

Closes #7532

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): skip the UNION ALL wrapper for the single-archive-table case

boost_process_local_data_ids() always wrapped its flush query in
"SELECT * FROM (... UNION ALL ...) t ORDER BY ...", even when there was
only one archive table -- the steady-state case, since old archive
tables are dropped at the end of each cycle. UNION ALL results wrapped
in a derived table are one of the documented cases MySQL/MariaDB's
derived-table merging doesn't apply to, so every pass paid for a
filesort over an indexless derived table regardless of how well-ordered
the underlying archive table's rows already were.

Extracts the per-table SELECT into boost_archive_select_sql() and uses
it directly, without the wrapper, when there's exactly one archive
table. The UNION ALL/derived-table shape is kept as the fallback for
the genuinely rare case where a prior crashed run left more than one
archive table behind.

Verified against a real MariaDB 11 instance and via a SQLite-backed
integration test that both shapes return correctly filtered and sorted
rows.

Closes #7533

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): batch-prefetch RRD field name lookups instead of per-row queries

Two N+1 patterns in the Boost hot paths:

boost_process_local_data_ids() (poller_boost.php) re-ran the
unused_data_source_names/nt_rrd_field_names lookups on every
local_data_id boundary in the sorted flush result set.

process_poller_output() (lib/poller.php) had the equivalent lookup, but
without even a boundary check -- it re-ran per matching row whenever
the output contained a ':', including consecutive rows for the same
data source.

Adds poller_prefetch_rrd_field_names() (lib/poller.php, shared by both
callers since poller_boost.php already requires lib/poller.php) to
batch-fetch these mappings once per invocation via IN()-batched queries
keyed by the full set of local_data_ids in the batch, chunked at 1000
ids to match the array_chunk(..., 1000) convention used elsewhere
(lib/api_data_source.php, lib/api_graph.php). Both call sites now look
up the prefetched maps in-memory inside their loops.

While consolidating the lookups, fixes a latent bug the old
process_poller_output() code had in its non-templated fallback branch:
a query referencing gti.task_item_id without joining
graph_templates_item, which throws an unknown-column error whenever
that path is hit. The batched query always joins correctly.

Integration test proves both correctness (against a real SQLite-backed
schema) and the query-count reduction: 5 local_data_ids that would
previously cost up to 10 queries for these two lookups now cost exactly
3, regardless of how many ids are in the batch.

Closes #7534

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(boost): consolidate poller_output_boost inserts into one helper

The INSERT INTO poller_output_boost (...) VALUES ... statement was
hand-written four times: three in cmd.php (behind the same
boost_redirect == 'on' && boost_rrd_update_enable == 'on' guard) using
INSERT IGNORE, and once in boost_poller_on_demand() (lib/boost.php)
using ON DUPLICATE KEY UPDATE. Beyond the duplication, that was a real
data-integrity inconsistency between two paths writing the same shape
of data: silently-drop vs last-write-wins on the same key.

Since boost_redirect gates cmd.php's writes and boost_poller_on_demand()
returns early when boost_redirect is on, the two paths never run
concurrently for the same install; they're alternate write paths
selected by that one setting, not competing writers. A genuine
collision on (local_data_id, rrd_name, time) in either path represents
a retry of an already-collected sample -- a batch resent after a
partial failure, or two writers racing on the same rounded second --
not two independently meaningful readings, since RRD only stores one
value per timestamp regardless. Standardizes on INSERT IGNORE
(first write wins), matching the sibling poller_output insert this
table shadows, which has always used INSERT IGNORE, and 3 of the 4
existing call sites.

Extracts boost_flush_output_batch() (lib/boost.php) as the one shared
helper, handling both the batched INSERT and the max_allowed_packet-safe
chunking cmd.php's three sites previously didn't do at all (they only
chunked by item count).

Integration test exercises a genuine duplicate-key collision against a
real (SQLite-backed) table and asserts the chosen first-write-wins
behavior.

Closes #7536

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(tests): resolve cross-branch test interactions found by full-suite verification

Two test-only bugs surfaced only when all four fix branches for the
poller_item/Boost hardening pass ran together, not when each ran in
isolation:

- BoostArchiveHandoffIntegrationTest extracted the archive-forward INSERT
  IGNORE by searching lib/boost.php for the first occurrence of "INSERT
  IGNORE INTO poller_output_boost". The Boost SQL-optimization branch adds
  a second, earlier occurrence (the new boost_flush_output_batch() helper's
  single-line prefix), so the naive search now grabbed the wrong snippet.
  Anchor on the multi-line "...poller_output_boost\n\tSELECT *" shape that
  only the archive-forward statement has.

- PollerItemRemotePurgeHandOffTest calls push_out_host() (via
  api_data_source_change_host()), which needs CACTI_PATH_LIBRARY. The test
  never requires include/global.php (which normally defines it), and was
  relying on some other file in a full multi-thousand-file suite run having
  defined it first as a process-wide side effect. Define it directly
  instead of depending on unguaranteed load order.

No production code changed.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* style(poller): satisfy php-cs-fixer strict_param in the script-server count

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): order the single-archive-table flush by the table's PK columns

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(boost): key the max_allowed_packet cache per connection

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(tests): restore the default db_* session after swapping it for a fake handle

Left swapped, the fake handle answered every later read_config_option() in the
run; on PHP 8.4 the extra deprecation traffic routed through CactiErrorHandler
turned that into an uncaught PDOException and killed the pest process.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-08-05 14:07:21 -07:00
Thomas VincentandTheWitness 7a1e9bbf92 harden CSV encoding and process execution handling (#7196)
* fix: harden CSV encoding and process execution handling

Route CSV export cells through a shared cacti_csv_cell() helper so values with quotes, commas, or leading formula characters round-trip correctly, and add defensive guards on shell construction, unchecked proc_open handles, discarded exec exit codes, and gap values in nth-percentile fetch math.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: address review feedback on CSV and process hardening

- escape poller_id in cmd_realtime proc_open command to match remote_agent

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: keep numeric CSV cells unquoted and guard snmptrap and rrdtool process failures

- exempt numeric values from the CSV formula-injection prefix and preserve empty snmptrap positions while bailing out when the RRDtool process fails to spawn

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: ignore phpstan function.alreadyNarrowedType identifier

Clears the level 6 alreadyNarrowedType error on lib/data_query.php:2591 that
develop currently emits, matching the sibling narrowing identifiers already ignored.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-06-24 09:43:16 -04:00
Thomas VincentandTheWitness fcbcbdd1d7 refactor: replace strpos() boolean checks with str_contains/str_starts_with (#6645)
* refactor: replace strpos() boolean checks with str_contains/str_starts_with

Completes the PHP 8.x string function migration started in lib/.
Converts remaining legacy patterns across 81 files:

- strpos($x, $y) !== false → str_contains($x, $y)
- strpos($x, $y) === false → !str_contains($x, $y)
- strpos($x, $y) != false  → str_contains($x, $y)
- bare if (strpos($x, $y)) → if (str_contains($x, $y))
- substr($x, 0, strlen($y)) == $y → str_starts_with($x, $y)

Intentionally preserves strpos() calls that use the return value as
an integer offset:
- user_admin.php / user_group_admin.php (4 calls, ternary for position)
- auth_profile.php (2 calls, offset for substr); guards fixed from
  bare if ($pos) to if ($pos !== false)

Behavior fix: ss_esxi_vhosts.php:55 bare strpos() missed position-0
matches; str_contains() now correctly detects 'not installed' at any
position. Adjacent line already used str_contains().

Ref: https://github.com/Cacti/cacti/issues/6641

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: correct dead-code elseif in plugin_display_compat() version comparison

The if/elseif chain in plugin_display_compat() checked >=, >=, >, <
— the second >= was unreachable dead code. Corrected to <= to complete
the operator set (>=, <=, >, <).

Without this fix, <= constraints (e.g. "<=1.2") fell through to the <
branch and were rendered as "< =1.2" instead of "<= 1.2".

This pre-existing bug was exposed when converting strpos() to
str_contains(), which made PHPStan detect the always-false condition.

Ref: https://github.com/Cacti/cacti/issues/6641

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: add limit to explode('=') in CLI argument parsers

17 CLI scripts used explode('=', $parameter) without a limit of 2,
silently truncating values containing = (e.g. --note=a=b parsed as
$value='a' instead of 'a=b'). The other 40 CLI scripts already had
the correct limit.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-02-25 11:38:59 -05:00
TheWitness 27919c50d7 Updating Copyrights to 2026 (#6578) 2026-01-28 16:11:10 -05:00
TheWitnessandCopilot b993471579 Fix #6539 - Allow Standard Cacti Log Files to be Rotated on Demand,PHPdoc, and Typing (#6540)
* Fix #6539 - Allow Standard Cacti Log Files to be Rotated on Demand from the Cacti UI

* QA: php-cs-fixer updates and documentation for lib/database.php

* QA: Update .gitignore for composer files

* QA: Update .gitignore for composer files

* QA: Back off .gitignore change its duplicated

* QA: Fix typing of db_has_permissions

* QA: Fixing typing issue in db_format_index_create

* Update lib/clog_webapi.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* QA: Fix additional code issues

* QA: Applying more defaults to add device

* QA: Fix at the database layer

* QA: Fixes based upon automated testing

* QA: Fix at the database layer

* QA: After new SQL checking

* QA: Skip MacTrack oui database download warnings

* QA: Fixing type issue

* QA: Recommendation from Copilot

* QA: Recommendation from Copilot

* QA: Recommendation from Copilot

* QA: Recommendation from Copilot

* QA: Some minor changes to avoid errors in testing

* QA: Unused variable

* QA: Recommendation from Copilot

* QA: Recommendation from Copilot

* QA: Recommendation from Copilot

* QA: Minor introduced code defect

* QA: Adding some unifying space around concatination

* QA: Consistent comments and new function alias'

* QA: Multi-line comment corrections

* QA: Fix SQL Style

* QA: Missed one function alias

* QA: PHPStan Level 2 and PHPDoc fixes

* QA: PHPStan Level 2 and PHPDoc Fixes

* QA: PHPStan Level 6 Fixes

* QA: Fix lib/xml.php to PHPStan Level 6

* QA: Wrong grep -v

* PHPStan Level 6 for lib/vdef.php

* PHPStan Level 8 for lib/time.php

* QA: Raise PHPStan to Level 6 for lib/database.php

* QA: Remove @deprecated tag from grv()

* QA: Switch to htmle() alias over html_escape()

* QA: More addressing PHPStan issues at Level 6

* QA: Change recommended by Copilot for Readability

* QA: More PHPStan fixes

* QA: More PHPStan fixes

* QA: More PHPStan fixes for lib/utility.php

* QA: More PHPStan fixes at Level 6

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: More PHPStan fixes at Level 6

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: More PHPStan fixes at Level 6

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: More PHPStan fixes at Level 6

* QA: Raise PHPStan to Level 6 More Functions

* QA: Raise PHPStan to Level 6 More Functions

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: Raise PHPStan to Level 6 More Functions

* QA: More PHPStan fixes at Level 6

* QA: Some phpseclib3 fixups to update to latest

* QA: More fixups for graphing for magic strings

* QA: More PHPStan fixes at Level 6

* QA: Fixing order string type on get_allowed_graphs()

* QA: Fixing improper use of print

* QA: Optimization of color_templates

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Copilot recommendation

* QA: Raise PHPStan to Level 6 More Functions

* QA: Issues insalling Cacti

* QA: Issues insalling Cacti

* QA: Issues insalling Cacti

* QA: Issues insalling Cacti

* QA: Issues insalling Cacti

* QA: Issues testing Cacti

* QA: Copilot recommendation

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: More PHPStan fixes at Level 6

* QA: Installer failing

* Undo ownership changes to remove tar errors

* QA: Updates after php-cs-fixer

* QA: Adding code checks to syntax checking

* QA Change syntax flow language

* QA: Reduce the number of cures

* QA: Use sequentional option

* Change runner to self-hosted for check job

* Fix typo in OS version in workflow file

* Add Git installation step in workflow

Added step to install Git if not already installed.

* Add '-y' flag to apt-get install and update commands

Updated apt-get commands to include '-y' for automatic confirmation.

* Change workflow OS version to 'ubuntu-latest'

Updated the workflow to use 'ubuntu-latest' instead of 'ubuntu:latest' and modified the 'runs-on' configuration.

* Remove sequential option from phpcsfixer script

* Remove PHPCS fixer step from workflow

Removed PHP coding standards check from workflow.

* Move code checks to before plugins

Moving these core checks to before the plugins are installed as we really want to check only the base Cacti here.

* Add Composer validation and caching to start of workflow

* QA: Addressing PHPStan additional

* QA: Restore composer lock file

* QA: Addressing PHPStan additional

* QA: Addressing PHPStan additional

* QA: Complete LDAP Project

* QA: php-cs-fixer doing it's thing

* QA: Syntax is already happening in PHPStan

* QA: The md5 function no longer exists

* QA: Update after ldap changes

* QA: Undo that last password hack

* QA: Attempting to switch to MariaDB 11.8

* QA: Attempting to switch to MariaDB 11.8

* QA: Resolving some issues with PHP8.4

* QA: Resolving some issues with PHP8.4

* QA: Add PHP8.5 to the matrix

* QA: Removing PHP8.5 due to dependency issues

* QA: Add the DB version to the testing output

* QA: Switch back to MySQL for the moment

* QA: More fidelity in reporting

* QA: The pipe must be mixed

* QA: Moving audit_schema.php changes from 1.2.x

* QA: Moving audit_schema.php changes from 1.2.x

* QA: Restore mysql monitoring

* QA: php-cs-fixer issues

* QA: Attempt to roll back to MariaDB 10.6

* QA: Attempt to fix check_all_pages.sh

* QA: Make installer functional again

* QA: Fix installer php-cs-fixer issues

* QA: Type to a string

* QA: Get the database version correctly

* QA: Fix issue with reports in check_all_pages.sh

* QA: Check the server version in the right place

* Feature: Support SHA256 as a new hash for passwords

* QA: Function definition and return typing audit

* QA: Refinement of form types

* QA: Refinement of form types

* QA: Consistency of comments

* QA: Issues with php-cs-fixer and more comments

* QA: More typing issues

* QA: php-cs-fixer on merge conflict resolution

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-01-25 10:13:56 -05:00
TheWitness b9334a15d9 QA: Updates for files for php-cs-fixer and some php-doc changes 2025-11-18 21:01:15 -05:00
TheWitness e5c4015e0e QA: Remove most references to $config
This removes most entries to $config in the base code.  A few new constants were defined to achieve this:

* CACTI_CONNECTION - The status of the current connection
* CACTI_CSRF_SECRET - The path to the csrf secret file
* CACTI_PHP_SNMP - True if php-snmp is installed
* CACTI_WHITELIST - The path to the Cacti Whitelist file if defined

I also moved several $config['poller_id'] to the existing POLLER_ID and move a number of $config['is_web'] to the existing CACTI_WEB constants.

There remain a few artifacts in the code.  They will be handled in a separate commit.
2025-03-22 16:48:59 -04:00
TheWitness b7e7da6077 Clear warnings during upgrade 2025-03-09 18:47:11 -04:00
TheWitness 2c779273f9 Copyright Updates and PHP-CS-Fixer updates 2025-03-09 14:34:19 -04:00
TheWitness cefbd2cfc8 Fix: #6001 - Extend SYSTEM STATS
* Provides more visibility into Device collection errors.
2024-12-27 21:02:04 -05:00
TheWitness 22b85e1622 Fix: #5966 - Add snmp retries at the device level
* This covers all the built in scripts.  Several packages will need rebuild
2024-12-06 23:07:32 -05:00
Dmitiry Vinogradov 17228a4051 Fix unused variable (#5764)
Variable $width_errors is never user anywhere.
2024-06-20 08:44:53 -04:00
Petr Macek b38e922d20 fix incorrect variable in ping (#5671) 2024-03-18 10:02:20 -04:00
Petr MacekandTheWitness 79033cc1fe cmd poller - add availability test for devices without poller items (#5641)
* add device status test for devices without poll items

* add device up/down status test for devices without poll items

* update

* Update cmd.php

Minor PSR and language changes

---------

Co-authored-by: TheWitness <thewitness@cacti.net>
2024-02-19 11:03:51 -05:00
TheWitness 1aa72d24c9 QA: Update Copyright to 2024 2024-02-03 14:18:57 -05:00
netniV 98bd9c0995 SNMP Uptime is not always ignored for spikekills (#5355)
Closes #5355
2023-06-12 03:06:05 +01:00
TheWitness ca7a352286 QA: Porting 1.2.24 Changes back to the Develop Branch (#5246) 2023-02-24 03:48:31 +00:00
Mark Brugnoli-Vinten 32ba2f19f4 Update copyright year for 2023 2023-01-03 20:34:26 +00:00
Mark Brugnoli-Vinten 8b29d12cc9 Define cacti system paths as constants for better code checking (#5120)
This also provides a full run of src-fixer against all files to
ensure all code is now in alignment.

Closes #5120
2023-01-01 01:03:15 +00:00
netniV d6e5b764c4 QA: device debug does run (#5063) 2022-12-10 11:30:31 +00:00
TheWitness 0c13987abf QA: Respond with correct --version 2022-12-03 11:16:11 -05:00
Mark Brugnoli-Vinten fae4fe9d2f QA: Minor corrections 2022-11-27 23:05:57 +00:00
TheWitness 05d2923542 QA: Missed commit on cmd.php 2022-11-27 15:59:54 -05:00
TheWitness 04a4c9f147 QA: Resolve SQL Where errors 2022-11-27 14:00:55 -05:00
Mark Brugnoli-Vinten 8b9c9f54cc Fix up incorrect active profile check 2022-11-27 16:14:58 +00:00
TheWitness 29773ef605 Getting rusty, reversed the order 2022-11-27 02:56:13 +00:00
TheWitness c305c3bdf4 Fix broken upgrade 2022-11-27 02:41:29 +00:00
netniV 77bb4ddc21 Ability to disable a site (#3740)
Closes #3740
2022-11-27 01:46:10 +00:00
netniV 371f8b5253 Fix issues with backtracing and input validation (#3559, #3560)
- When a developer uses backtracing, HTML output can be seen from command line bug confirmed
- When using `input_validate_xxx`, calls to `die_html_input_error()` do not pass variable name bug confirmed

Closes #3559
Closes #3560
2022-11-26 18:28:29 +00:00
netniV 21a9ebf84a More version handling changes 2022-11-25 02:40:27 +00:00
TheWitness 02a92633e7 QA on #4899 and #4901. --version and --help broken 2022-08-21 12:31:01 -04:00
TheWitness c626fcb549 QA on #4899, #4901
Some function calls have to come later.
2022-08-21 11:38:36 -04:00
TheWitness a7aa8cb810 Fixing #4899 and #4901 - Script Server and Poller Offline Mode
* feature#4899: Pass poller status to script_server to avoid connecting to the main database when offline or in recovery
* feature#4901: Make the script server accept arguments in the standard way
* force error reporting off to prevent issues with output processing
2022-08-21 09:05:40 -04:00
TheWitness 122782ff0d Fixing #4863 - Non Device Data Sources
Non device based Data Sources not being polled
2022-07-22 09:33:50 -04:00
TheWitness e567b089a7 Fixing Issue #4808 - Poller Output Issues
- WARNING: Poller Output Table not Empty
- This is a summer solution.  The real solution is a bit more complicated
2022-06-10 22:10:42 -04:00
TheWitness f6306f427b Fixing Issue #4687 - Poller Output Table Not Empty
Poller output not empty all the time
2022-04-13 21:09:56 -04:00
TheWitness 705a814760 Fixing Issue #4624 - Uptime Instance SQL Errors
Error Unknown column 'snmp_sysUpTimeInstanceAlt' in 'field list'
2022-03-15 21:06:27 -04:00
TheWitness d79faad274 Attempt to work around issue where PHP server ends and cmd.php attempts to stop it anyway. 2022-02-13 11:44:29 -05:00
TheWitness 82b229db9e Updating Copyright to 2022. 2022-02-06 17:16:20 -05:00
netniV e17ef5604a When using CMD for polling, pipes are not properly referenced for PHP scripts now (#4525)
Closes #4525
2022-01-08 03:01:52 +00:00
TheWitness dd8ce81f60 Fixing #4519 cmd.php errors
Tracebacks and errors when remote host poll fails
2021-12-31 10:03:34 -05:00
TheWitness c81a39e347 Fixing #4428 - Additional GUI changes for alt uptime 2021-10-19 18:54:08 -04:00