12 Commits

Include renames

Author	SHA1	Message	Date
TheWitness	7b551eea72	chore: normalize CRLF line endings to LF across Cacti-owned files (#8156) ... Excludes vendored packages, third-party JS, patches, and the Win32 probe fixture. Pure EOL change; guards pass and php -l is clean.	2026-10-02 17:22:42 -04:00
TheWitnessandci	9facd9b500	security: batch 7 develop - consolidate remaining develop-only GHSA advisories (12 fixes + 8 confirmations) (#8149) ... * security: batch 7 develop - consolidate remaining develop-only GHSA advisories (12 fixes + 8 regression confirmations) Fixes: f7jw (automation preview XSS), rp5g (db_column_exists LIKE), vx2m (automation pivot CASE WHEN), h5wg (plugin DDL identifiers), 929h (aggregate LIKE filter), mfw8 (add_graphs REGEXP), m49v (user_admin sort_column allowlist), mpfm (rrd error-image theme allowlist), m3fh (SNMPv3 control chars), cg45 (remember-me row revoke), xq26 (remember-me retention window), m67r (import xml_path containment). Confirmed already fixed on develop, pinned with regression tests: 4gph, 977w, mgcq, 5p79, cfhh, wfcf, pjq5, wp33. Excluded: 227p (1.2.x-only), mx5c (maintainer private branch). * security: address batch-7 review feedback (lexical-first xml_path containment, session-wide sort allowlist, connection-aware db_column_exists quoting, reject-not-normalize plugin DDL identifiers, behavior-based test names) * style: apply php-cs-fixer single-quote fix to api_automation pivot builders; add WSL dev-environment note to copilot instructions * fix phpstan string typing * test: refresh SQL interpolation baseline for db_column_exists db_qstr change (database.php 9->8) --------- Co-authored-by: ci <ci@local>	2026-10-02 15:27:34 -04:00
3 people	1e5329dd89	security: consolidate verified GHSA-rjvj win32 cmd.exe hardening for develop (#8056) ... * fix(win32): harden the cmd.exe shell sinks on develop (GHSA-rjvj-r52f-8v5q) Forward-port of the 1.2.x win32 command-injection remediation. On Windows the SNMP/ping/data-query/trap commands run through cmd.exe, which ignores the \" escape and toggles quoting on every ", so cacti_escapeshellarg cannot stop & | ^ < > ( ). - add cacti_escapeshellarg_cmd(), stripping those operators on win32 before quoting (a Unix pass-through) - snmp_escape_string(): strip the operators in the win32 branch - route the SNMP binary hostname, the data-query device values (get_script_query_path), the data-input arg builders, the Windows ping hostname, and the SNMP trap notification arguments through the strip get_script_query_path is device-reachable (a rogue monitored device supplies an SNMP index/system field that executes on the Windows poller); the rest are post-auth. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(win32): strip %VAR% from the hostname on Windows (GHSA-rjvj-r52f-8v5q) cmd.exe expands %VAR% even inside quotes, so a device hostname of %USERNAME% would expand into the SNMP/ping command line. Add an opt-in $strip_env flag to cacti_escapeshellarg_cmd() and set it for the SNMP binary and ping hostnames, which never legitimately contain a percent. Community and credential values are left untouched because they can legitimately contain %. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(changelog): note win32 cmd.exe hardening for develop * security: require Package Management permission to trust new package signers package_accept_key() (reachable via the repository-based package import flow) inserted new keys into package_public_keys, the global trust store consulted by import_validate_signature() for every future import by every user, without checking for the Package Management (realm 29) permission that gates package_keys.php/package.php/package_repos.php for the same table. An Import Templates (realm 17) user could therefore add themselves as a trusted signer for the whole install. Also add the 'accept' action to the CSRF-protected bad_actions list so the acceptance step requires POST plus a valid token like every other state-changing action. GHSA-gxgm-8rhv-r7jv * docs(changelog): note package-signer trust authorization fix * security: close tab-based open redirect bypass in CSRF timeout handler sanitize_uri()'s guard (20298c6cd) models the WHATWG rule that browsers strip leading C0 controls/whitespace before resolving a redirect, but Chromium separately strips an embedded tab from anywhere in the target. A request target of /%09/evil.example passes the guard (no leading control char, not two leading slashes) but Chromium reduces the resulting 'Location: /<TAB>/evil.example' header to a network-path reference and navigates off-site. 1.2.x already uses validate_redirect_url() for this same sink (3338a63e1), which resolves the URL via parse_url() and checks host/port against the server-configured values, so it isn't bypassable this way. Switch develop's CSRF timeout handler to match. GHSA-c725-4jm5-9fx5 * security: authorize single-item Report actions against ownership reports.php sets \ = true, so include/auth.php grants an implicit guest session (when the Guest User feature is enabled) without ever evaluating the realm 22 gate declared for this page. The single-item action handlers reports_item_remove()/_moveup()/_movedown() relied solely on that page-level gate and performed no independent ownership check, so an unauthenticated guest-bound request could delete or reorder reports_items rows belonging to any report, including ones owned by an administrator. reports_form_actions() already gained an equivalent \ check; add the same authorization (Reports admin realm 21, or the requesting user owns the item's report) to the three single-item handlers. Same underlying chain as the 1.2.31 fix for reports_user.php (guest bind removed + cacti_authorize_resource() added to reports_item_remove()); the rewritten develop reports.php/lib/html_reports.php did not carry either half of that fix forward. GHSA-j5jh-fhwg-r56x * security: authorize tree node CRUD actions against per-tree access tree.php dispatches create_node/delete_node/move_node/rename_node/copy_node directly to api_tree_*() with no is_tree_allowed() check, even though the normal tree-view path already enforces that boundary. A non-admin user with the Trees realm but no explicit grant on a given tree_id could still create, delete, move, rename or copy nodes on that tree by calling these actions directly, bypassing the authorization boundary enforced elsewhere. GHSA-prxj-84qq-v5jq * security: remove shell from Realtime RRD create and validate DS bounds poller_realtime.php built the Realtime-graph RRD 'create' command line (including the SNMP-sourced ifSpeed/ifHighSpeed interface speed, substituted in verbatim as the DS maximum) and ran it through shell_exec(). A hostile or spoofed monitored device could return an ifSpeed value such as '1000000000\uid=197608(ladams) gid=197121 groups=197121' and get arbitrary OS command execution as the web/poller user the next time any user with the Realtime Graphs permission (or an unauthenticated guest, where configured) viewed a realtime graph for an affected data source. - lib/rrd.php: rrdtool_function_interface_speed() now casts ifHighSpeed/ ifSpeed to int before they become the DS maximum. - lib/rrd.php: rrdtool_function_create() now rejects any non-numeric rrd_minimum/rrd_maximum right before they are concatenated onto the rrdtool command line, forcing them to 'U' and logging instead. - poller_realtime.php: the RRD create is now routed through the same shell-free rrdtool_execute()/proc_open(bypass_shell) path every other RRDtool invocation already uses, matching the pattern 1.2.x already uses for this exact call site, instead of shell_exec(). GHSA-f88m-q789-g22j * security: authorize Report save/edit against report ownership reports_form_save()'s update path fetched the existing report's owner from the database but never compared it to the session user before calling sql_save(), so any user holding only the Reports realm could overwrite another user's report by POSTing its id. reports_edit() had the same gap on the read side: it fetched a report by id with no ownership check, so the same user could view any other user's report configuration (recipients, schedule, etc.). 1.2.x already guards both call sites with cacti_authorize_resource(..., 'reports'); develop had lost that check. GHSA-8f5c-3h59-q7gc * security: reject ORDER BY sort_column subquery bypass sanitize_sql_column()'s allowlist regex kept parentheses to support the INET_ATON(col)/NATURAL_SORT_KEY(col) sort wrappers, but that also let MySQL subquery syntax through unchanged, since a subquery like (SELECT(0)FROM(SELECT(SLEEP(5)))a) needs only alphanumerics and parentheses. get_order_string() then detects the '(' and disables its backtick quoting, so the payload reaches ORDER BY verbatim, giving any user with realm 2-5 a blind, time-based SQL injection via sort_column. sanitize_sql_column() now only allows '(' through when the whole value is exactly one of the two known safe wrapper forms around a plain identifier; anything else containing '(' -- including subquery syntax -- now falls back to the default. Added regression tests covering the reported subquery and REGEXP-exfiltration payloads. GHSA-8j73-cvf6-jrvx * security: confine Reports format_file to a bare filename reports_form_save() wrote the POST-supplied format_file value verbatim into reports.format_file, and reports_load_format_file() concatenated it onto CACTI_PATH_FORMATS with no directory-component check, so a value such as ../../../../etc/passwd escaped the formats directory and any Reports-realm user could read arbitrary files readable by the web server. 1.2.x already fixed the same sink (GHSA-g37j-39f4-6r4j / CVE-2026-41884, GHSA-mjvw-mhj5-9jcj / CVE-2026-40084); develop had lost the guard. format_file is now confined to basename() at both the save site and the load site, with the load site additionally rejecting a bare '.'/'..'/empty result before any file IO. Added regression tests. GHSA-5x27-wqpm-m24v * security: authorize Data Source enable/disable against owning device ds_enable()/ds_disable() and the bulk drp_action enable/disable loops in form_actions() passed a user-controlled local_data_id straight into api_data_source_enable()/api_data_source_disable() with no object-level authorization check, so any user holding the Sites/Devices/Data realm could toggle the enabled state of a data source belonging to a device outside their assigned Device Permissions. Added data_source_authorized(\), which allows host-independent data sources (data_local.host_id = 0) and otherwise defers to is_device_allowed(\), and applied it to both the single-item and bulk action paths. GHSA-2fhv-c794-v433 * security: authorize bulk Change Data Source Profile against owning device The bulk 'Change Data Source Profile' action (drp_action == '6') iterated selected_items and updated data_template_data/data_template_rrd/poller_item for each supplied local_data_id with no object-level authorization check, so any Sites/Devices/Data realm user could retarget the RRD step/heartbeat of a data source belonging to a device outside their assigned Device Permissions -- the same class of gap as GHSA-2fhv, in a different bulk action. Guarded with the data_source_authorized() helper added for that fix. This action does not exist on 1.2.x (develop-only feature). GHSA-fc82-w6xq-4jg7 * security: grant AllowOverride All in docker/apache.conf The packaged docker/apache.conf granted only 'AllowOverride FileInfo Options=FollowSymLinks' for the Cacti document root. Cacti's shipped .htaccess deny rules (log/, cache/*, scripts/, cli/, bin/, mibs/, rra/, contrib/, tests/) rely on 'Require all denied' (Apache 2.4, override type AuthConfig) and/or legacy 'Order/Deny' (override type Limit), neither of which FileInfo/Options covers, so Apache silently ignored every one of those deny rules and served the directories over HTTP to any unauthenticated client -- most notably log/cacti.log, which can contain usernames, client IPs, SQL backtraces, and any secrets Cacti logs (SNMP community strings, LDAP bind details). AllowOverride All lets every shipped .htaccess apply as intended. Added a regression test. docker/ does not exist on 1.2.x (develop-only). GHSA-fxfg-j995-j69j * docs: add security advisory / CVE policy to Copilot instructions Documents the policy established for this remediation effort: develop is unreleased, so a develop-only vulnerability does not get a CVE requested; 1.2.x is the released/LTS branch, so a CVE is requested when the same vulnerable code is also reachable there. Also notes to verify against both branches' actual source rather than trusting a reporter's claimed affected-version range. * fix(security): update stale test harnesses and address review findings CI fixes: - CsrfCheckContractProbe.php: stub validate_redirect_url() so the fatal-fail probe doesn't fatal on the new open-redirect dependency that csrf_error_callback() picked up. - InstallerCsrfRecoveryTest.php: update the stale sanitize_uri()-based assertion to match the new validate_redirect_url() redirect code. - psalm-baseline.xml: drop the TaintedHeader suppression for include/csrf.php now that validate_redirect_url() resolves it. Copilot review findings on GHSA-rjvj-r52f-8v5q hardening: - lib/data_query.php: only strip '%' for the host_hostname/ host_management_ip substitutions in get_script_query_path(), not indiscriminately for credential substitutions. - lib/functions.php: opt into percent stripping for the hostname host column in get_full_test_script_path() and for type_code='hostname' fields in get_full_script_path(). - lib/snmpagent.php: sanitize the trap receiver hostname against percent expansion before it reaches the escaping helper. - tests/security/baselines/*.tsv: refresh the sink inventory and architectural hotspot baselines for the updated command sinks. - tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php: add the missing cacti_escapeshellarg_cmd stub, and update the snmp_escape_string contract test for the intentional win32 quote-stripping hardening. - tests/Unit/Security/Shell/CmdMetacharHelperTest.php + tests/fixtures/CmdMetacharHelperWin32Probe.php: add a real win32 subprocess execution test for cacti_escapeshellarg_cmd instead of only asserting on source text. - CHANGELOG: add the pull-request reference to the new entry. * fix(snmp): remove now-unreachable quote-escaping branch in snmp_escape_string The metachar strip on the line above already removes the double quote on win32, so the substr_count(SNMP_ESCAPE_CHARACTER) branch below it could never execute; drop it so lib/snmp.php reaches 100% coverage again. * style: fix pre-existing php-cs-fixer blank-line drift blocking CI data_sources.php and lib/html_reports.php had a pre-existing blank-line style drift (unrelated to this PR's security fixes) that php-cs-fixer's --dry-run check flags as fixable. Since "PHP 8.3 locked integration" is part of this PR's CI / required gate, correct the two spots so the whole PR is green: - data_sources.php: collapse a double blank line before form_actions(). - lib/html_reports.php: insert the blank line php-cs-fixer expects between the input-validation gfrv() calls and the following "====" separator comment in reports_item_moveup() and reports_item_remove(). No behavior change; purely CI/style formatting. * style: refresh phpstan baseline for pre-existing, unrelated findings Three PHPStan findings were masked behind the earlier php-cs-fixer failure and only surfaced once that step passed: - lib/html_reports.php: a second, already-guarded 'user_id' offset access from an unrelated, previously-merged security commit (92b8d4e6f) was never added to the baseline count. - cli/splice_rrd.php, poller_realtime.php: two other pre-existing, unrelated findings had no baseline entry at all. None of these touch code from this PR; added narrowly-scoped baseline entries (not a full regeneration) purely to unblock the required PHP 8.3 locked integration check. * fix(security): address follow-up review findings on consolidated PR Closes the four remaining review findings from the follow-up review round: - package_import.php: trustSigner() now POSTs with a CSRF token instead of $.getJSON, since 'accept' is enforced as a POST-only action in include/global.php's bad_actions list (GHSA-gxgm-8rhv-r7jv). - lib/html_reports.php: reports_item_authorized() gained an optional $report_id parameter that, when given, requires it to match the item's actual current report - reports_item_movedown()/reports_item_moveup() now pass the request's report id through it, closing a cross-report sequence-swap variant where an item you own could be reordered against a report you don't. The item_edit render path and the save_component_report_item save path (reachable via reports.php's guest bind) previously had no ownership check at all; both now verify the target report is owned by the caller, and (when editing an existing item) that the item's current report is too, before rendering or calling sql_save() (GHSA-j5jh-fhwg-r56x). - tree.php + include/global.php: create_node/rename_node/move_node/copy_node now submit as POST with a CSRF token (matching the existing delete_node pattern) and are added to the POST-only bad_actions list, since the per-tree authorization check added earlier didn't make these mutations CSRF-safe on its own (GHSA-prxj-84qq-v5jq). Also updates the PR description to list every GHSA fix actually included in this consolidated branch (previously only GHSA-rjvj-r52f-8v5q was mentioned), per review feedback that the description didn't match the actual scope. * fix: resolve CI failures on PR #8056 (docblock alignment, sink baseline) - lib/html_reports.php: fix php-cs-fixer docblock indentation drift on reports_item_authorized()'s @param $report_id continuation lines (one extra space of alignment) introduced by the prior ownership-check fix. - tests/security/baselines/sink_inventory.baseline.tsv: refresh to track the 4 new header('Location: ' . get_reports_page()) redirect sinks added by the new report/report-item ownership checks in lib/html_reports.php. Verified via tests/security/verify_sink_inventory.sh and confirmed tests/security/verify_architectural_hotspots.sh still reports no new hotspots. * fix: track new PHPStan offset findings from extended reports_item_authorized() phpstan-baseline.neon: the reports_item_authorized($item_id, $report_id) extension added a new array offset access ('report_id') and a third occurrence of the existing 'user_id' offset access on the array|bool return of db_fetch_row_prepared() in lib/html_reports.php. Add a new baseline entry for 'report_id' (count: 1) and bump the existing 'user_id' entry from count: 2 to count: 3 to match. Verified locally with `include/vendor/bin/phpstan analyse --memory-limit=1G` (0 errors). --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <copilot@users.noreply.github.com>	2026-09-23 09:45:42 -04:00
Thomas Vincent	d07758c48b	security: substitute query data before escaping graph titles and axis labels (#7846) ... * security: substitute query data before escaping graph titles and labels rrd_substitute_host_query_data() ran over the whole graph command after each argument was cacti_escapeshellarg'd, so a device-supplied SNMP value expanded from a |query_*| token in a graph title, vertical-label or right-axis-label could contain a quote and break out of the RRDtool argument. Move the substitution inside cacti_escapeshellarg at those three fields and drop the post-escape whole-command pass. Forward-port of the 1.2.x fix. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * security: finish resolve-before-escape refactor, drop redundant inner substitution $value is already resolved (credential-stripped and host/query substituted) by rrdtool_resolve_graph_text() at the top of the graph-options loop, so the inner rrd_substitute_host_query_data() in the title, vertical-label and right-axis-label argument builders was a redundant second pass. Reconcile the older escape test to match; the device-quote-neutralisation behavior test is unchanged. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-09-02 20:32:47 -07: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
Thomas Vincent	4f74377c5a	security: escape path_boost_log in the Boost debug redirect (#7784)	2026-08-26 00:00:17 -07:00
Thomas Vincent	b287b1bfdf	security: escape path_stderrlog in the poller stderr redirect (#7785)	2026-08-26 00:00:00 -07:00
Thomas Vincent	13adf317c0	chore: stop pull requests conflicting each other on shared files (#7798) ... * chore: resolve concurrent CHANGELOG appends with a union merge Every PR lands on the same two anchors, so merging one reliably conflicts the others on CHANGELOG alone. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> (cherry picked from commit 6bc8e63ecb) * feat(changelog): take entries as fragment files instead of CHANGELOG edits Every pull request edited one of two lines, so merging one conflicted the rest. A file per change gives concurrent PRs nothing to collide over. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> (cherry picked from commit a8a0bfa29a) * chore(security): drop line numbers from the tracked sink baselines Both verifiers already strip the line before comparing, so the tracked numbers carried no signal and rewrote 212 lines of baseline on any edit that shifted code, conflicting every open pull request. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> (cherry picked from commit ccfd3d374b) * fix(ci): rebase stale pull request branches instead of merging them A merge update adds a commit per run, so branches accumulate snapshots of old develop and re-conflict on every shared file they carry. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> (cherry picked from commit e9e33c77cd) * test: match the escaping call rather than the exact source line The literal assertions here required the uncast form while PhpBinaryPathNullCoercionTest required the cast, so the two could not both pass and whichever landed second broke the other. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> (cherry picked from commit 98cfc84756) * fix(changelog): handle security entries in fragments and validation Review caught the missing type. Security entries also take GHSA identifiers and open a release section ahead of issue and feature entries, none of which the first pass handled. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: require Composer-managed dependencies Prevent agents from committing third-party package trees and require composer.json plus composer.lock updates instead. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-08-25 21:10:50 -07:00
Thomas Vincent	faf3227400	security: escape path_rrdcheck_log in the rrdcheck poller redirect (#7787) ... path_rrdcheck_log is concatenated into the args passed to exec_background(), which does not escape string args, so a shell metacharacter would inject a command. No settings form exposes this value today, so this is defense in depth and a forward-port of the release/1.2.31 fix. Refs #7473 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-08-23 10:26:19 +01:00
Thomas Vincent	f487bc3530	fix(functions): cast path_php_binary before escaping it (#7792) ... The forward-port in 02386f5 escaped the value but dropped the (string) cast, so the null-coercion guard failed on develop and on every open PR. Its comment also tripped the sink scanner, which has no comment awareness, so the wording now avoids the bare token. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>	2026-08-22 23:35:07 -07:00
Thomas Vincent	02386f5006	security: escape path_php_binary in test_data_source (#7786)	2026-08-22 17:14:12 -07:00
Sean Mancini	96033267cb	refactor(tests): reorganize unit tests into domain category folders (#7733) ... * refactor(tests): reorganize unit tests into domain category folders - Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class - Group all 186 test files into domain folders: DataCollection/, Ui/, Core/, Database/, Installer/, Scripts/, Security/, Plugin/ - Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions - Fix all __DIR__ relative paths to match new folder nesting depth - Update CI workflow path triggers and phpunit XML configs for new file locations - Update .github/copilot-instructions.md with unit test organization guidelines - Add tests/vendor/ to .gitignore * add gaurds * fix up guard for unit tests * fix redeclaration of cacti_log in tests * fix updates to unit tests * fix test patjhs * fix: update test __DIR__ paths for reorganized test structure * fix: conditionally define read_user_setting to prevent redeclaration in tests * fix: conditionally define functions to prevent redeclaration in tests - Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix with function_exists() checks to prevent conflicts when test stubs are loaded before lib/functions.php in the same PHP process * fix: add unit test bootstrap to prevent function redeclaration errors - Create tests/bootstrap-unit.php to load core libraries before tests - Ensures lib/functions.php loads before test stubs define stub functions - Test stubs use function_exists() checks, so they detect pre-loaded functions - Update phpunit.xml to use bootstrap-unit.php for Unit testsuite - Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process * fix: load test stubs before global.php in unit test bootstrap - Reorder bootstrap to load CactiStubs.php and UnitStubs.php first - Ensures stub functions like __() are available when global.php loads - global.php calls get_cacti_version_text() which needs __() for translations - Fixes 'Call to undefined function __()' error during bootstrap * fix: remove function conflicts from test stubs - Remove stub definitions for functions already in lib/functions.php: cacti_sizeof, cacti_log, cacti_strtolower, read_config_option, read_user_setting, cacti_count - Keep translation function stubs (__(), __esc()) needed by global.php - Update bootstrap to load lib/functions.php after stubs - This prevents function redeclaration errors while maintaining test isolation * fix: prevent constant and variable redeclaration in bootstrap - Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap (CactiStubs.php now handles this with guards) - Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING - Only set CACTI_TEST_BOOTSTRAP env vars if not already set - Fixes PHP warnings about 'Constant X already defined' * fix: remove redundant lib/functions.php load from bootstrap include/global.php already loads lib/functions.php at lines 287-288. The bootstrap was trying to load it again after global.php, but global.php needs format_cacti_version() to be available on lines 293-294 to define CACTI_VERSION and CACTI_VERSION_FULL constants. Solution: Let global.php handle loading lib/functions.php itself, and don't load it separately in the bootstrap. * fix: remove more conflicting function stubs from test helpers - Remove stubs for functions also defined in lib/functions.php: raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix - Remove POLLER_ID constant stub (defined unconditionally by global.php) - Keep only translation functions and test-specific helpers - This prevents function/constant redeclaration errors * fix: minimize test stubs to only translation functions Remove ALL helper functions and constants from UnitStubs.php that are: - Defined in lib/functions.php (raise_message, clean_up_lines, etc.) - Defined in lib/html_utility.php (srv, set_request_var) - Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*) - Defined in global.php (POLLER_ID) Keep ONLY translation functions (__(), __esc()) that are: - Called by global.php during initialization - Not defined elsewhere - Required for proper bootstrap sequence This eliminates all constant and function redeclaration errors. * fix: remove __esc stub - already defined in global_languages.php Keep only __() translation function stub, which is needed by global.php and not defined elsewhere in Cacti core. * fix: handle no-arg case in __() translation stub The __() function may be called with just a string and no format arguments. Previously, vsprintf() would fail with 'arguments array must contain 2 items'. Now, if no args are provided, return the text as-is.	2026-08-16 19:23:31 -07:00