* 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.
6.0 KiB
Settings Validation
Why this exists
Cacti settings are written from several places: the web UI (settings.php),
CLI utilities under cli/, and (planned) JSON-RPC endpoints. Each of those
paths previously carried its own ad-hoc checks for the same setting, and the
checks drifted apart as code was added. The intent of CactiSettings::validate()
is to put the constraint definition next to the setting itself so every write
path uses the same rules.
A setting in include/global_settings.php declares its constraints alongside
the existing keys (method, default, max_length, ...). The validator runs
the entire posted set in one pass and returns a {name => message} map. An
empty map means the input is valid.
Why constraints are declared as closures
include/global_settings.php is loaded from include/global.php before
include/vendor/autoload.php. Eagerly instantiating new Assert\NotBlank()
inside the array literal would fatal at file-load time because the Symfony
Validator classes are not yet loadable. Wrapping each constraint in an arrow
function (fn() => new Assert\NotBlank()) defers the instantiation until
CactiSettings::validate() runs the closures. The use Symfony\Component\Validator\Constraints as Assert;
import at the top of the file is a compile-time alias and does not trigger
autoloading.
How CactiSettings::validate is used in settings.php
save_settings() builds a snapshot of posted values via Cacti's request-var
helpers (gnrv) and calls the validator before writing any rows:
require_once(CACTI_PATH_LIBRARY . '/CactiSettings.php');
$snapshot = [];
foreach ($settings[grv('tab')] as $field_name => $field_array) {
$snapshot[$field_name] = gnrv($field_name);
}
$violations = CactiSettings::validate($snapshot, $settings);
if (cacti_sizeof($violations) > 0) {
foreach ($violations as $name => $message) {
$_SESSION['sess_error_fields'][$name] = $name;
$_SESSION['sess_field_values'][$name] = $snapshot[$name] ?? '';
raise_message('cacti_settings_' . $name, ..., MESSAGE_LEVEL_ERROR);
}
header('Location: settings.php?tab=...');
exit;
}
When violations are present the request is rejected before any REPLACE INTO settings
runs. The user is redirected back to the same tab with the error highlighted.
How to add constraints to a setting
- Open
include/global_settings.phpand find the setting definition. - Add a
'constraints'key holding an array of closures, each returning a Symfony constraint instance. - The
use Symfony\Component\Validator\Constraints as Assert;alias is already imported at the top of the file, so constraints read asfn() => new Assert\Range(...). - Wrap any custom
message:argument in__()so the message participates in Cacti i18n.
Example:
'snmp_timeout' => [
'friendly_name' => __('Timeout'),
'method' => 'textbox',
'default' => '500',
'max_length' => '10',
'constraints' => [
fn() => new Assert\Regex(pattern: '/^\d+$/', message: __('must be a positive integer (milliseconds).')),
fn() => new Assert\Range(min: 1, max: 600000),
],
],
When a Choice list duplicates a canonical array from global_arrays.php,
derive the choices from that array inside the closure rather than restating
the values:
'constraints' => [
fn() => new Assert\Choice(choices: array_merge(
array_keys($GLOBALS['poller_intervals']),
array_map('strval', array_keys($GLOBALS['poller_intervals']))
)),
],
Both string and integer forms of the keys are accepted because $_POST
values arrive as strings while the canonical keys are integers.
Constraint types used in the pilot
See the Symfony Validator reference for the full catalog. The pilot uses:
Assert\NotBlank-- value is present and non-empty.Assert\Length-- string length withinmin/max.Assert\Range-- numeric value withinmin/max.Assert\Choice-- value is one of an enumerated list. Use this for any setting that is rendered as adrop_array.Assert\Regex-- value matches a pattern. Useful for "is this an integer string" since$_POSTvalues arrive as strings.Assert\Positive-- value is strictly greater than zero.
Migration template for the next batch
For each setting you want to constrain:
- Identify the setting's existing implicit rule (e.g.
methodisdrop_arraywith a fixed key set, ormax_lengthis'255'). - Translate that rule to a constraint object.
drop_arraybecomesAssert\Choiceover the array keys.max_lengthbecomesAssert\Length. Numeric textboxes typically needAssert\RegexplusAssert\Range. - Add a unit test in
tests/Unit/Security/InputValidation/CactiSettingsTest.phpthat exercises the new constraint with a synthetic definition (do not depend oninclude/global_settings.phpfrom the test). - Run
composer testand confirm the new case passes.
Translator wiring (known limitation)
CactiSettings::validator() builds the validator with
Validation::createValidator() and does not pass a TranslatorInterface.
Symfony's default English messages render regardless of the active Cacti
locale. Custom messages declared in global_settings.php should be wrapped
in __() so the operator-facing text is translatable through Cacti's own
i18n stack. Wiring a TranslatorInterface so that the framework's built-in
constraint messages also translate is a planned follow-up.
Defense in depth
The constraint layer supplements existing checks rather than replacing them.
- The form-render method (
drop_array,dirpath,filepath,textbox_password) still enforces its implicit rules insave_settings(). For example,dirpathcontinues to verify the directory exists before storing. - Any post-save handler (cache-clear, poller restart, log rotation) keeps whatever validation it already performs.
- The constraint check runs first and short-circuits the write, so downstream handlers see only values that already cleared the declared rules.