7 Commits
Author SHA1 Message Date
Thomas Vincent 2761fdc226 fix(rrd): [develop] retain source identity after CF fallback (#8339)
* fix(rrd): [develop] retain source identity after CF fallback

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

* test: measure native consolidation fallback coverage

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-23 10:37:54 -06:00
1997691de0 Render RRDtool Tune Info as balanced div boxes and POST the Data Source repair actions (#8412)
* data_sources: render RRDtool Tune Info as balanced div boxes

rrdtool_tune() emitted an unbalanced mix of html_header() and
html_end_box() calls wrapped around raw table rows inside the single
box opened by data_sources.php.  The premature and extra closing tags
corrupted the DOM, which (depending on theme) clipped the Save button
at the bottom of the Data Source edit page while in Data Source Info
mode.

- Separate the execute, CLI and HTML code paths in rrdtool_tune() and,
  for HTML, render each section as a self contained, balanced div box.
- Extract and rename the helper print_leaves() to the standalone
  rrdtool_tune_print_leaves(), matching the rrdtool_tune_* naming
  convention, and emit div rows instead of table rows.
- Switch the data_sources.php wrapper box to div mode to match.

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

* data_debug: require POST for the Data Source repair actions

Applying RRDfile repairs and rerunning the debugger are state changing
operations, so they should not be reachable with a GET.  Driving them
through a GET also meant the follow up redirect was pulled into the
content area as a full page, losing the theme and navigation on
rerender.

Register data_debug.php run_repair and run_debug in the
cactiIsPostActionUrl() allowlist so loadUrl() upgrades them to a
token-bearing POST, and add the matching csrf_require_post() guard so
the browser transport and server side guard stay in sync.

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

* rrd: split rrdtool_tune into testable render and execute helpers

Break rrdtool_tune() into small single-purpose helpers so each path can
be exercised directly by unit tests (the web display path is otherwise
unreachable under CACTI_CLI):

- rrdtool_tune_print_leaves() takes an explicit $html flag instead of
  branching on CACTI_CLI, so both the div and plain-text branches are
  testable.
- rrdtool_tune_execute() applies the tune/resize commands for a repair.
- rrdtool_tune_print_cli() prints the source commands as plain text.
- rrdtool_tune_print_html() renders the balanced div boxes.
- rrdtool_tune() becomes a thin dispatcher over the three.

Add tests/Unit/RrdTuneRenderTest.php covering the leaf printer, the HTML
and CLI printers, the execute path and the dispatcher, asserting the
output is balanced div markup with no table rows.

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

* review: post Data Source Troubleshooter actions via cactiPostAction

Address PR review feedback:

- Switch the Apply Suggested Fixes and Rerun Analysis links to the
  existing cactiPostAction/data-url idiom (as data_sources.php already
  does for ds_repair) instead of upgrading a loadUrl() GET through the
  cactiIsPostActionUrl allowlist. The allowlist path replayed the
  query-free mutation URL into browser history, so a refresh reopened
  the list instead of the selected Data Source; a full cactiPostAction
  submission follows the server redirect to the read-only view URL and
  records that in history. The run_repair / run_debug csrf_require_post
  guards are retained, and the data_debug.php allowlist entry (and its
  now redundant click handlers) are removed.

- Add a callable $executor test seam to rrdtool_tune_execute() (matching
  rrdtool_dump_document) so the unit tests assert the tune and resize
  commands are actually executed, with resize.rrd produced only when the
  resize command runs, rather than passing on the rename alone.

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

* review: move RrdTuneRenderTest into the Unit/Core/Rrd category folder

Per AGENTS.md, new unit tests belong in a category subfolder rather than
directly under tests/Unit. Move RrdTuneRenderTest there and update the
UnitStubs include depth accordingly.

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

---------

Co-authored-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-10-16 11:23:07 -04:00
32467ddcab perf: reuse graph option arrays and document validation results (#8361)
* perf: reuse bootstrapped graph option arrays and document validation

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

* test: record the installed RRDtool version in profiles

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

* docs: record final validation scope and profiling runtime limits

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

* docs: clarify the scope of changed-code coverage

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

* ci: repair relocated PHPUnit configs and the deletion-only coverage gate

- Point the five relocated configs that still carried a contributor's local
  autoload path (#8376) at ../include/vendor/autoload.php.
- Pass --configuration=tests/phpunit.xml to the CSV window workflow and the
  README example now that phpunit.xml no longer lives in the repository root.
- Let the changed-lines gate pass deletion-only PHP changes. Without added
  lines the generated config has no source filter, PHPUnit writes no clover
  report, and the gate failed on the missing file instead of reporting zero
  executable changed lines.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: TheWitness <thewitness@cacti.net>

* ci: keep the missing request-report check for deletion-only changes

Only short-circuit the changed-lines gate when there are no added PHP lines
and PHPUnit wrote no clover report, so an empty request-coverage directory
is still rejected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: TheWitness <thewitness@cacti.net>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: TheWitness <thewitness@cacti.net>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-10-14 22:45:42 -04:00
Thomas Vincent 4d11070ced feat: expose RRDtool 1.11 graph axis controls (#8356)
* feat: expose RRDtool 1.11 graph axis controls

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

* fix: constrain axis template inheritance checkboxes

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

* test(rrd): exercise native aggregate validation and enforce coverage

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

* ci: share changed-code coverage contracts and enforce shell policy

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

* fix(ci): run request coverage with portable path matching

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

* ci: retain bootstrap coverage and validate request dispatch

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

* test: synchronize native coverage fixture dispatch

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

* test: measure coverage at the exact PR revision

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-10-14 12:10:28 -04:00
Thomas VincentandTheWitness 182b21dc53 fix(csv): align export and graph sample windows (#7987)
* fix(csv): align export and graph sample windows

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

* ci(csv): satisfy release policy checks

* test(csv): align legacy capacity contract

* test(csv): add standard license headers

* fix(rrd): infer step from trailing fetch row

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

* ci(csv): trigger on resolution query coverage

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

* test(rrd): mirror fetch helper arguments

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

* test(rrd): isolate percentile globals

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

* test(rrd): annotate intentional eval

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

* test(rrd): restore integration config

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

* test(rrd): enable strict types

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

* test(rrd): restore font config

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

* ci: align CSV coverage with supported PHP versions

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-09-12 20:18:00 -04:00
Thomas Vincent 964c1fb1f4 Harden RRDtool execution and data handoff (#7615)
* fix(rrdtool): quote RRDfile paths passed to the command line

__rrd_execute() hands the assembled line to shell_exec(), and its docblock
already required callers to quote each variable argument.

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

* fix(tests): widen bare RRD path sweep

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

* fix(rrdtool): quote binary path as one argument

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

* fix(rrd): preserve caller logging context

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

* Make RRD XML mutations transactional

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

* Harden RRDtool proxy transport handling

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

* fix(rrdproxy): reject invalid command argument lists

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

* fix(rrdproxy): preserve binary graph payloads

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

* Keep RRDtool proxy sessions encrypted

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

* Harden local RRDtool command and output handoff

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

* Eliminate graph rendering metadata lookups

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

* Batch DSStats metadata and last-value reads

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

* Batch interface speed metadata reads

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

* Update architectural hotspot baseline for RRD execution

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

* Batch RRD resolution profile lookups

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

* Batch DSStats summary aggregates

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

* Narrow phpseclib key types for the proxy encrypt and decrypt calls

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

* test(rrd): bound the forked proxy peers so a stall cannot hang the suite

The children accepted and read with no deadline while the parent used a 2s
socket timeout, so any mismatch left the child blocked in read and the parent
blocked in a bare pcntl_waitpid(). Note that accept() ignores SO_RCVTIMEO, so
readiness has to be waited on with socket_select() rather than a socket option.
Also corrects the array-command expectation, which still assumed develop's
escape-everything behaviour rather than rrdtool_build_command() leaving the
operation unquoted.

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

* test(rrd): seed proxy settings where the real read_config_option looks

UnitStubs only declares its stub when lib/functions.php has not been loaded
yet, so during a full run the real database-backed function is the one in
effect and $unit_config_options was being ignored. The parent then dialled an
address the child was not listening on, which is what stalled the handshake.
Seeding the option cache keeps the settings visible to either implementation.

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

* test(rrd): assert the shell-free RRDtool launch instead of the old quoting

__rrd_init now hands proc_open an argv array with bypass_shell, so escaping the
binary path would put the quotes inside the filename. The assertion still
expected the pre-argv line and had been failing unnoticed behind the suite hang.

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

* test(dsstats): derive the expected stamps instead of assuming UTC

dsstats_poller_output() formats with date(), so the hardcoded UTC strings only
matched on a UTC runner. The failure was masked by the suite hang.

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

* fix(rrd): keep RRDtool control codes intact in stdin commands

Escaping the backslash when converting a shell-quoted argument to an RRDtool
token left legends such as "  IO Wait\:" ending in a dangling control code, so
RRDtool refused every graph carrying an escaped colon. Verified against RRDtool
1.7.2 in pipe mode; values with no unambiguous quoted form are now refused
rather than mangled.

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

* test: use Cacti test bootstrap and config

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

* test: resolve includes through Cacti path constants

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

* test: match the escaping helpers by token, not by text

lib/rrd.php documents its escaping behaviour in a comment, and the regex
over raw source counted that as a call site.

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

* ci: refresh sink inventory after develop rebase

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

* test(rrd): prove argv isolation and concurrent pipe draining

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

* test(security): refresh sink inventory baseline after rebase onto develop

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-31 16:49:44 -04: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