mirror of
https://github.com/Cacti/cacti.git
synced 2026-11-03 18:47:54 +09:00
fix/graph-wrapper-date-range-develop
64
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 TheWitness
|
5e870a91df |
security: batch 6 develop - path_php_binary escape (GHSA-4p7f), boost-log .log guard (GHSA-m6wx), tree lock/unlock/get_node authz (GHSA-73qf), rrd create strip (GHSA-967c) (#8148)
* security: batch 6 develop - escape path_php_binary (GHSA-4p7f), boost-log .log guard (GHSA-m6wx), tree lock/unlock/get_node authz (GHSA-73qf), confirm rrd create control-char strip (GHSA-967c) * security: cast path_php_binary before escaping in resource_cache_out (GHSA-4p7f) |
||
|
TheWitness
|
9d628b4959 |
security: batch 5 develop - fail closed on No-Auth admin bypass (GHSA-69vw), tag GHSA-3jj2, add 1.2.32 CHANGELOG section (#8146)
* security(GHSA-69vw): fail closed on auth_method None instead of auto-granting an admin session - check_reset_no_authentication() no longer wipes the admin password, flips auth_method, or hands an unauthenticated request an admin session; it fails closed and requires a manual auth_method reset (GHSA-69vw-qxxv-p8w2). - Tag GHSA-3jj2-v5ch-wmq5 in the CHANGELOG: the domains_login_process() realm bypass is already closed by the Login Providers rewrite (realm < 1000 fails closed). - CHANGELOG: add a 1.2.32 section below 1.3.0-dev with the GHSAs also fixed in 1.2.x, and de-duplicate the security entries the #8145 merge doubled. * docs(CHANGELOG): file host_id SQLi fix under 1.2.31 as GHSA-r7hc-hrpv-52gf; drop plain issue#7693 entry |
||
|
github-actions[bot]
|
850778aa5b |
ci: refresh security/SQL-interpolation baselines (#8140)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
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>
|
||
|
github-actions[bot]
|
44a14fb5e8 |
ci: refresh security/SQL-interpolation baselines (#8108)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
TheWitnessandCopilot
|
599c76b369 |
Add Force Uninstall for orphaned plugins whose directory is missing (develop) (#8107)
* Add Force Uninstall for orphaned plugins with a missing directory (#8105) The Plugin Management page showed a directory-missing plugin (status -5) with only an inert info icon, so its stale plugin_config/hooks/realms/db_changes entries could not be cleaned up from the UI. Make that icon a Force Uninstall action guarded by the existing displayDialog() modal confirmation; on confirm it POSTs action=remove, which calls api_plugin_uninstall(plugin, true) - that skips the unavailable uninstall hook and removes the plugin's hooks, realms, plugin_config row, and the tables/columns it created (via plugin_db_changes). * Force Uninstall: POST via dialog form to satisfy CSRF action guard; add cacti.pot strings * Force Uninstall: restrict to truly-orphaned plugins, block if required by others, replicate config to remote pollers * Refresh sink inventory baseline for Force Uninstall header redirects in plugins.php * Force Uninstall: post action=remove via dialog form fields (not a URL) and JSON-inject dialog strings to satisfy CsrfGetDeleteTest and the inline-JS hardening guard --------- Co-authored-by: Copilot <copilot@users.noreply.github.com> |
||
|
TheWitness
|
a52368125e |
feat(auth): rename User Domains to Login Providers, add SAML2 and OpenID Connect (#8066)
* feat(auth): rename User Domains to Login Providers Collapses user_domains/user_domains_ldap into a single login_providers table with a JSON parameters column, migrated in place during the 1.3.0 upgrade. Renames the admin page, menu, breadcrumbs, and help links to match. * feat(auth): rework login providers as Cacti\Auth classes Introduces LoginProviderInterface with credential (LDAP/AD) and redirect (SAML2/OpenID) flavors, replacing the procedural domains_login_process()/domains_ldap_*() functions. Adds blank-field group membership gating shared by every provider type. * feat(auth): add SAML2 and OpenID Connect login providers Adds onelogin/php-saml and firebase/php-jwt, a new cacti_http() SSRF-hardened outbound request helper for every OpenID discovery/token/JWKS call, and login_sso.php to handle the redirect-based SSO flow. Login page now shows a button per enabled SAML2/OpenID provider, and per-provider auth cookie opt-out is enforced on both the password and SSO login paths. * chore(i18n): regenerate cacti.pot * feat(auth): add Local and Basic auth as Cacti\Auth providers LocalAuthLoginProvider (CredentialLoginProviderInterface) and BasicAuthLoginProvider (new PreAuthenticatedLoginProviderInterface, since Basic Auth has no password for Cacti to check) are thin adapters over the existing local_auth_login_process()/basic_auth_login_process() functions, which are left untouched to avoid disturbing their regression-tested lockout/timing/rehash behavior. * feat(auth): move local password verification into LocalAuthLoginProvider secpass_login_process() and local_auth_login_process() are retired; their lockout, constant-time verification, forced-password-change, and rehash-on-login logic now live end to end in the class (verifyCredential/enforcePasswordPolicy/rehashIfNeeded). secpass_check_pass() stays a shared lib/auth.php function since password change/reset/user_admin also call it outside any login flow. Local auth still has no login_providers row: isEnabled() is always true and it cannot be disabled or deleted. * feat(auth): move Login Provider CRUD into Cacti\Auth classes AbstractLoginProvider gains static deleteById()/enableById()/disableById()/makeDefaultById()/persist(); each concrete provider owns its own collectParameters(), dispatched via LoginProviderFactory. login_providers.php's form_save()/form_actions() call these instead of doing raw SQL, and the retired provider_remove()/provider_disable()/provider_enable()/provider_default()/provider_collect_parameters() procedural functions are gone. Local/Basic auth are unaffected: they never extend AbstractLoginProvider, so they structurally cannot expose delete/disable. * fix(auth): address Copilot review findings on SSO/SAML/OIDC security Fixes callback/ACS/SLS/metadata URLs that used the raw provider id instead of the login realm (broke every SSO login), adds InResponseTo correlation for SAML and sub/azp validation for OIDC, requires HTTPS for OIDC discovery/endpoints, pins cacti_http() connections against DNS rebinding via CURLOPT_RESOLVE, exempts login_sso.php from the authenticated-page redirect guard and gates it on auth_method, wraps provider dispatch in a exception boundary, adds a dedicated SAML SLO path instead of routing it through login completion, makes remember-me an explicit opt-in for SSO, fixes disabled/deleted providers not revoking existing auth cookies, restricts the default-realm action to LDAP/AD, and fixes the 1.3.0 migration silently enabling group enforcement for installs that had it off. Also updates the E2E seed script for the login_providers schema. * chore(auth): refresh CI security baselines and document no-op validation Regenerate the sink inventory baseline for the new/renamed lib/Auth/* and login_providers.php files, and add '// no-validation:' comments on the intentionally-unconstrained admin/free-text/secret fields (LDAP DNs/filters, SAML certs/keys, OIDC client credentials, provider display fields) flagged by the validator constraint ratchet. Refreshes the validator baseline to drop the deleted user_domains.php entries. * refactor(ldap): remove dead DB-lookup path from Ldap::__construct The domain_id>0 branch duplicated the JSON-parameter-to-property mapping already owned by LdapLoginProvider::buildLdap(), and nothing constructs Ldap with a real id anymore (all call sites pass 0). Drop the parameter and the branch so Ldap stays a pure connection/protocol class with no DB coupling. Also fixes a latent bug: group_require is a typed bool property, but LdapLoginProvider::authenticate() assigned it an int (1/0) from a strict_types=1 file, which throws a TypeError on every LDAP/AD login attempt. Assign a real bool instead. * fix(auth): fix LDAP login, upgrade ordering, and hardening guard failures LdapLoginProvider compared LdapError::GetErrorDetails()'s int error_num against the string '0' with strict equality, so it never matched and every LDAP/AD login was rejected regardless of valid credentials. Compare against int 0 instead. install/upgrades/1_3_0.php called login_providers_convert_1_3_0() (which reads and then drops user_domains/user_domains_ldap) before the pre-existing legacy ALTER TABLE statements that add the debug/network_timeout/bind_timeout columns those reads depend on, so upgrades failed once the tables were already gone. Moved the call to run after those ALTERs. OpenIdLoginProvider::complete() read state/code from raw \ instead of gnrv(), tripping the hardening pattern guard; switched to the request helper. * style(auth): apply php-cs-fixer formatting and normalize line endings to LF Runs php-cs-fixer across the new lib/Auth/* classes, login_providers.php, login_sso.php, and the other touched files, fixing indentation/alignment/docblock style to match project convention. Also normalizes several files that had picked up CRLF line endings back to the LF the project's .php-cs-fixer.php config (setLineEnding) and the rest of the tracked tree actually use. * fix(auth): address second Copilot review round (9 high/medium findings) - Make login_providers_convert_1_3_0() resumable: CREATE TABLE IF NOT EXISTS and INSERT ... ON DUPLICATE KEY UPDATE so an interrupted upgrade can be re-run instead of leaving a half-migrated, unrecoverable table. - SamlLoginProvider::complete() now fails closed when no saved AuthnRequest ID exists instead of passing null to processResponse(), which skipped InResponseTo validation and accepted unsolicited IdP-initiated assertions. - LoginResult gains an isCredentialFailure flag; only a genuine wrong-password LDAP bind (LdapError::Failure) counts toward the lockout counter now, not connection errors or group-membership denials. - Fixed button_label/debug being read from the JSON parameters column in OpenIdLoginProvider/SamlLoginProvider/LdapLoginProvider when they are actually top-level login_providers columns; AbstractLoginProvider now hydrates and exposes both. - Removed SamlLoginProvider's wantMessagesSigned, which silently required the outer SAML message (not just the assertion) to be signed, rejecting IdPs that only sign the assertion as the UI promises. - Added getTemplateUserId() to CredentialLoginProviderInterface and implemented it on LocalAuthLoginProvider so the contract (not just incidental AbstractLoginProvider inheritance) guarantees the method lib/auth.php calls. - cacti_http()'s CURLOPT_RESOLVE entries now bracket IPv6 literals correctly, and safe-IP validation strips parse_url()'s brackets before checking, fixing IPv6 target pinning. - login_providers.php's type dropdown now swaps the Full Name attribute default (cn/displayName) on an actual user-driven type change, not on page load, so a new Active Directory provider does not silently save the LDAP default. - Extracted auth_apply_group_login_opts() and call it from both auth_login.php and login_sso.php, so SSO logins honor a user's group-derived login_opts the same way password logins already do. * fix(auth): fix PHPStan level 8 errors and pest harness stub drift Test harness fix: the LoginProvidersLoginRealmValidationTest stub LoginResult class was missing the isCredentialFailure property added to the real class, causing 'Undefined property' warnings that broke the harness's JSON output. Added the property and set it on the simulated bad-password failure. PHPStan (48+ errors, all newly introduced by this branch's rewrite, never actually verified until now): narrowed db_fetch_row_prepared()/db_fetch_assoc()'s array|bool (including bare true, which survives a '?: []' short-circuit - fixed with explicit is_array() checks instead) at every call site across auth_login.php, login_sso.php, lib/auth.php, lib/Auth/LocalAuthLoginProvider.php, lib/Auth/LoginProviderFactory.php, and install/upgrades/1_3_0.php. Fixed OpenIdLoginProvider::discover()'s return shape to match its declared docblock instead of returning the raw decoded JSON. Fixed lib/functions.php's cacti_http() to narrow curl_exec()'s string|bool result with is_string() (not ===false, which still leaves 'true' as a residual type) and guard / as non-empty for curl_setopt_array()'s stubs. Fixed a real bug in SamlLoginProvider::processLogout(): processSLO(true) was passing 'true' as \ (the first parameter) instead of \ (the fifth), so it always redirected/exited internally with the default \=false, making Cacti's own logout.php never regain control. Now calls processSLO(keepLocalSession: true, stay: true) and is typed ': never' to match; login_sso.php's now-genuinely-unreachable break statements after processLogout()/initiate() are removed. Fixed login_providers.php passing a expression (not a variable) to inject_form_variables()'s by-reference parameter. Verified: PHPStan analyse clean (0 errors), full Pest Unit/Security suite passes (128 tests), hardening guard/sink inventory/validator ratchet all still clean. * fix(auth): fix ambiguous column and array+bool crash in user_log filter The user filter dropdown query selected a bare 'id' column after joining user_auth and login_providers, which is ambiguous now that login_providers also has an id column (unlike the old user_domains.domain_id) - MySQL rejected the query outright. Qualified as ua.id. Also narrow db_fetch_assoc()'s array|bool result before the \ + \ + \ merge, which was throwing a TypeError whenever the query failed instead of surfacing the real SQL error. * fix(auth): address third Copilot review round (5 findings + tests) - AbstractLoginProvider::persist() now disables the template account for every provider type; SAML2/OpenID also copy it via user_copy() in login_sso_complete() and were left directly usable as a local login. - cacti_http_resolve_safe_ips() now detects IPv4-mapped IPv6 literals (::ffff:0:0/96) via inet_pton() and validates the embedded IPv4 address, closing an SSRF bypass via e.g. ::ffff:169.254.169.254. - login_providers_convert_1_3_0() now tracks db_install_execute()'s status for every row and only drops the legacy user_domains/user_domains_ldap tables once every row (and the final row count) is confirmed migrated. - Ldap::Getcn() now looks up ldap_get_entries()'s always-lowercased attribute keys case-insensitively while still storing the result under the originally-requested key, fixing AD's displayName (and any other mixed-case configured attribute) always resolving to a blank full name. - SamlLoginProvider::processLogout() now redirects to the LogoutResponse URL processSLO() returns for a LogoutRequest (stay:true), instead of discarding it and always redirecting to Cacti's own logout page - federated logout was never actually acknowledged to the IdP. - Added tests/Unit/Security/Auth/OpenIdLoginProviderTest.php (13 tests) covering verifyIdToken() signature/issuer/audience/azp/nonce validation, discover()'s HTTPS enforcement, and complete()'s callback-state and UserInfo-subject-matching checks, against a locally-generated RSA keypair/JWKS. - Regenerated the sink inventory baseline for the new header_redirect call in SamlLoginProvider::processLogout(). Verified: PHPStan clean, full Pest Security/Auth suite passes (830 tests including the 13 new ones), php-cs-fixer/hardening-guard/sink-inventory/validator-ratchet all clean. |
||
|
TheWitnessandcopilot-swe-agent
|
2d516ca0a9 |
security: consolidate additional verified GHSA fixes for develop (batch 2) (#8065)
* security: contain data source paths within the RRA directory (GHSA-9qff-xccr-rrc5)
* security: escape executable path in exec_background on non-win32 (GHSA-cv7j-p6fh-j4q7)
* security: stop disabled accounts authenticating via remember-me cookie and revoke on disable (GHSA-pqrp-v752-mc4f)
* security: prefer base_url over client Host for the force_https redirect (GHSA-3466-fp7g-c9rf)
* changelog: add fragments for GHSA-9qff-xccr-rrc5, GHSA-cv7j-p6fh-j4q7, GHSA-pqrp-v752-mc4f, GHSA-3466-fp7g-c9rf
* security: address review findings on GHSA batch 2 (develop)
- lib/dsstats.php, poller_maintenance.php: route through
get_data_source_path() instead of expanding data_source_path directly,
so the RRA containment check applies to these consumers too (GHSA-9qff)
- lib/functions.php: data_source_path_within_rra() now also validates
the realpath of each existing ancestor segment, closing a symlink
pivot that the purely lexical check missed (GHSA-9qff)
- include/global.php: fail closed instead of falling back to SERVER_NAME
when base_url is unset/invalid, removing the residual open-redirect
path (GHSA-3466)
- lib/html_utility.php: cacti_force_https_host()/
cacti_build_https_redirect_url() now parse a scheme-less base_url
(matching lib/reports.php's own normalization) and preserve a
configured non-default port instead of silently dropping it (GHSA-3466)
- tests: move DataSourcePathContainmentTest.php into
tests/Unit/Security/PathTraversal/, and switch its fixtures from a
fictional CACTI_PATH_RRA path to a real temp directory so the new
realpath()-based checks (including a symlink-pivot regression case)
can actually run
* test: fix CACTI_PATH_RRA constant collision in DataSourcePathContainmentTest
CI showed 2 failures: RrdProxyProtocolTest.php runs earlier in the same
Pest process and already defines CACTI_PATH_RRA to a fictional,
non-existent '/var/lib/cacti/rra', so our own definition guard never
ran and realpath(CACTI_PATH_RRA) returned false for every assertion,
failing the two tests that expect containment to be true.
Now ensure the directory backing CACTI_PATH_RRA actually exists on
disk regardless of who defined the constant first, and gracefully
skip only the fixture-dependent assertions if this process can't
create it (e.g. no permission on a system path).
* test: refresh sink inventory baseline for exec_background rename
|
||
|
github-actions[bot]
|
a1fab6816f |
ci: refresh security/SQL-interpolation baselines (#8059)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
d3d99bb08a |
test(cdef): enforce resolver coverage (#7974)
* test(cdef): enforce resolver coverage Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci(cdef): pin cache action correctly Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: isolate CDEF coverage bootstrap * fix(cdef): fail closed on invalid recursive definitions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): log corrupt resolver records Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): bound and isolate recursive resolution Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): preserve resolver failure state Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): isolate invalid graph expressions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): abort unsafe consumer operations Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): close consumer failure paths Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): make aggregate updates atomic Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): keep repair queries auditable Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): bind bulk deletes and escape item types Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cdef): guard corrupt types and repair reporting Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(aggregate): propagate push-out failures Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(aggregate): document mutable message checks Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
|
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 ( |
||
|
Thomas VincentandTheWitness
|
cb49f31ff9 |
Replace csrf-magic with maintained CSRF protection (#7744)
* build: add symfony/security-csrf Backs the CSRF token core that replaces the vendored csrf-magic fork. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: fix security-csrf package path assertion symfony/security-csrf ships CsrfTokenManager.php at the package root, not under src/. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor: register the composer autoloader before the CSRF include CSRF validation ran before the autoloader existed, which blocks any class-based guard. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(csrf): add CactiCsrfGuard over symfony/security-csrf Issues and validates session-scoped tokens; not yet wired into the request path. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(csrf): accept pre-upgrade csrf-magic tokens during the grace window Legacy tokens carry their own expiry, so the fallback closes itself without a config flag. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(csrf): port the output buffer rewriter to CactiCsrfGuard Keeps the hidden field name and JS globals that third-party plugins rely on. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): use json_encode for the JS script context in rewriteBuffer htmlspecialchars() protects the HTML attribute context but not the <script> string literal, which relied on Symfony's token generator never emitting a quote rather than being safe by construction. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(csrf): move the XHR shim to include/js as Cacti-owned code It has been maintained by the Cacti Group since 2020 and is not an upstream package. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(csrf): retire the vendored csrf-magic fork The procedural csrf_* surface stays as a deprecated shim so plugins keep working. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(csrf): validate requests through CactiCsrfGuard csrf-magic self-invoked from its own file; the guard is called explicitly. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(install): stop gating the permission step on the csrf secret Retiring csrf-magic removed the directory its default path lives in, and setStep() clamps to the first errored step, so no fresh install could advance. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): key the legacy fallback on the raw secret bytes refresh_csrf.php writes PHP source with a trailing newline, and trimming that yields a different HMAC key than csrf-magic used. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): log when the guard degrades to disabled in a web context The installer is the only web path that can reach it, and failing open in silence is not something csrf-magic could do. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: changelog for the CSRF library replacement Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): mint the token before the session write callback closes it csrf_startup() minted lazily from the output buffer callback, which runs after session_write_close()'s shutdown-function registration, so a token minted there on a legacy-fallback POST was silently dropped. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): make refresh_csrf.php a no-op Its default secret path no longer ships and live tokens are session-bound, not HMACs over the secret, so rotation neither worked nor did anything meaningful; running it during the upgrade grace window used to delete the legacy secret and kill validation for pre-upgrade tokens outright. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): fall back to the old default secret path when reading it csrf-magic tried its own directory as a second candidate regardless of $path_csrf_secret; the new reader only tried the configured path, so a site pointing $path_csrf_secret at a file that was never created lost the legacy-token grace window entirely. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(csrf): point the JS header at its BSD license sidecar The header reduces the original BSD terms to prose and the actual text sits in an unreferenced csrf.js.LICENSE, so add a pointer to it. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): restore csrf_check()'s bool return and $fatal contract The shim ignored $fatal and always returned void, so audit/audit.php's `!csrf_check(false)` evaluated `!null` (always true) and denied every request instead of getting a real pass/fail result back. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: reference the pull request number in the changelog entry Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: resolve includes through Cacti path constants Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(csrf): enforce full coverage Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Harden CSRF fallback handling Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(csrf): assert the component stays out of the index Committing it would leave a second copy to drift against the lock. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: align CSRF checks with PHP 8.2 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: normalize the CSRF changelog entry * Fix CHANGELOG entry ordering Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(csrf): support constrained test environments * fix(csrf): narrow nullable guard state * test: place CSRF coverage in security domain Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * security(test): match disabled exec exactly Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: run CSRF coverage on PHP 8.3 Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
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> |
||
|
Thomas Vincent
|
1ae479d4f5 |
feat(cli): add unified command frontend and normalize legacy failures (#7903)
* feat(cli): add unified command frontend and normalize legacy failures Add the Symfony-backed bin/cacti entry point while preserving legacy argument, terminal, signal, and exit-status behavior. Fold in the focused legacy CLI correctness fixes from #7906 without adding its unused parser abstraction. Keep path containment and the PHP 8.3 floor out of this review; the latter remains in #7939. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(cli): enforce PHP floor and deny web dispatch --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
08c02d3322 |
security: resolve open code scanning alerts (#8003)
* security: resolve open code scanning alerts * test(security): constrain SQL helper sanitizer * fix(security): address follow-up review |
||
|
github-actions[bot]
|
dd61dd7a31 |
ci: refresh security/SQL-interpolation baselines (#8001)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas Vincent
|
130f58243b | Fix SNMP uptime selection across polling paths (#7984) | ||
|
Thomas Vincent
|
6e72b00ae5 |
security(cli): keep database credentials off the command line (#7909)
* security(cli): keep database credentials off the command line audit_database.php passed the database password as --password= to the mysql client. Escaping does not help there: an argument is readable from the process list by every local user for as long as the import runs, and with --debug the same string was printed to stdout. The credentials now go in a defaults file created with mode 0600 and removed afterwards. splice_rrd.php named its dump files from the RRD basename and mt_rand() in a world writable directory and created them by shell redirection, and backupRRDFile() copied into the same place. Both now use one private directory created for the run. Modern kernels block the symlink case through fs.protected_symlinks, so this is hardening rather than a fix for an exploitable path, but the names should not be guessable in the first place. float_rrdfiles.php appended its debug log to a fixed /tmp/clearer.log, which any local user could pre-create or read. It now uses the system temporary directory with a name of its own. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh audit database checks * fix: quote credentials written to the MySQL option file The option file format is not byte transparent. An unquoted '#' starts a comment, a backslash escapes, and trailing whitespace is stripped, so any install whose database password contained one of those authenticated with a truncated credential and failed the schema load with no useful diagnostic. Double quote the values and escape the backslash and quote characters, verified against my_print_defaults. Refuse a credential containing a newline rather than write a file the parser would read as further options. Use --defaults-file rather than --defaults-extra-file so a ~/.my.cnf belonging to whoever runs the script cannot override the credentials Cacti supplies, and trim the 'which mysql' fallback, whose trailing newline split the command and stranded the credential argument on the second line. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Align CLI hardening with quality guards Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
ba7b8ef818 |
hardening: predictable import temp name and unsanitized table-filter search (#7866)
* issue#7492: use tempnam for import temp files instead of rand() The import wrote to sys_get_temp_dir()/package_import_<rand()>, a predictable name a local user could pre-create or symlink. tempnam() creates the file atomically with an unpredictable name. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7451: sanitize the CactiTableFilter search term The shared table-filter Search box registered its term with FILTER_DEFAULT (no sanitization), so the raw term reached filter processing. Validate it with sanitize_search_string like the other search filters. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7450: confine the auth-none change-password referer redirect check_reset_no_authentication() placed the raw HTTP_REFERER into the auth_changepassword ref= parameter, an open redirect and header-injection vector. Wrap it in urlencode(validate_redirect_url()), restoring the guard 1.2.x already has (this regressed on develop). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7490: anchor the graphs.php template_id filter regex The template_id filter used an unanchored regex, so any value containing a cg_/dq_/digit substring passed validation and reached explode()/SQL. Anchor it to match the hardened sibling filters in lib/html_graph.php. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * issue#7453: escape template and query names in the package preview get_package_contents() escaped file names with htmle() but emitted graph template, data query, and data-query graph-template names raw, so a package whose template name contains markup produced stored XSS in the import preview. Wrap the names in htmle() like the file rows. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: refresh sink inventory baseline Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
ed5ab24474 |
fix: detect a failed SNMP walk by exit code, not by device data (#7929)
* fix: detect a failed SNMP walk by exit code, not by device data cacti_snmp_walk() discarded the entire walk when its output contained the word Timeout. That output is the walk itself, one line per OID, so a device with an ifAlias of 'Timeout Monitor' or a matching sysDescr emptied the result and data_query.php then dropped every index for the device. The check could not detect a real timeout either. net-snmp writes that to stderr and neither walk command redirects it, so the matched string was always device data. cacti_snmp_get() already reads the exit code for exactly this reason; exec_into_array() exposes it by reference. Also refuse a walk with empty credentials, matching cacti_snmp_get, get_raw and getnext. An unknown v3 protocol previously built a credential-less snmpwalk and failed with no log line. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover the credential guard and refresh the security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: give the exec_into_array stub the real signature The stub dropped the by-reference exit code that lib/functions.php exposes, so any code reading it saw 0 regardless of what the command returned. That is why the walk's failure detection could regress to matching stdout without a test noticing. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: expect the exit-code wording for a failed walk Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
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> |
||
|
Thomas Vincent
|
ead882172e |
Make installer completion fail closed and add test coverage (#7626)
* Make installer completion fail closed Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Add installer unit and Docker integration gates Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Require installer coverage evidence Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Stabilize installer failure scenarios Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Address installer review feedback Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Reject incomplete installer selection payloads Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Match table validation to rendered controls Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Protect long-running installer migrations Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Load installer table metadata in one query Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Optimize installer completion queries Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch installer default settings writes Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch installer color upserts Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Batch installer template and poller lookups Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Refresh SQL interpolation baseline after develop rebase Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: use Cacti Composer test setup Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: resolve includes through Cacti path constants Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: align installer workflows with policy Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines after develop rebase Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: reuse TCP database credentials for installer checks 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> * ci: run installer coverage on PHP 8.2 The 8.1 pin is below the ^8.2 composer requirement, so pest died in platform_check before any test ran. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
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> |
||
|
Thomas Vincent
|
c02e3803a5 | fix(security): resolve automation taint findings (#7848) | ||
|
github-actions[bot]
|
f472548df8 |
ci: refresh security/SQL-interpolation baselines (#7838)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas Vincent
|
5c6e3d21cb | Add DI-friendly CactiFilesystem and adopt atomic CLI writes (#7189) | ||
|
Thomas Vincent
|
21708ab599 |
security: stop trusting Host for HTTPS redirects (#7833)
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
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 |
||
|
Thomas Vincent
|
5a71037953 | ci(security): scope Semgrep SARIF to changed PHP files (#7767) | ||
|
Thomas Vincent
|
ff475bef6d | feat(installer): add installation domain foundation (#7291) | ||
|
Thomas Vincent
|
619b33142e | feat(rendering): add HTML, URL, and inline-data output helpers (#7252) | ||
|
Thomas Vincent
|
d4e255b5d9 |
security: stage nonce CSP migration and gated enforcement (#7662)
* security: add nonce CSP mode for inline JavaScript protection Reworks the content_security_policy_script setting into a three-mode Inline JavaScript Protection dropdown (HTMLX default, Nonce, None) and ports the per-request nonce, strict-dynamic policy, and CSP violation report endpoint from 1.2.x. The stored option key and its default are unchanged, so existing installs keep their current policy byte-for-byte; nonce mode is opt-in. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * security: stage nonce CSP enforcement safely Add report-only migration, gated enforcement, nonce-aware shared emitters, Linux Docker validation, and operator documentation.\n\nRefs #7703 * security: validate CSP header sources Reject malformed alternate-source configurations at the CSP boundary, validate fallback report URIs, and remove the obsolete Psalm header-taint baseline entry.\n\nRefs #7705 * chore: drop the composer.json change from this PR Dependency and manifest changes are a maintainer call and do not belong in this one. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas VincentandTheWitness
|
ae54a2a552 |
security: bind device IDs and escape graph and device output (#7732)
* security: bind device IDs and escape graph and device output Device removal waits for graph cleanup inside a transaction before deleting the host row. Clone and template-sync ID lists go through db_in_clause, SNMP and ping strings are escaped at HTML sinks, and graph page size and column settings use bounded validators. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: skip cloned RRD task item remap when no matching data_template_rrd row exists Guards the outer loop in api_clone_device_template() the same way the inner loop already guards $new_graph_template_id: db_fetch_cell_prepared() returns false when the cloned Data Template has no matching data_template_rrd row, and binding that into task_item_id would corrupt the cloned graph template's wiring instead of leaving it alone. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Number the CHANGELOG entry so the format check passes Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas VincentandTheWitness
|
f2898730e1 |
fix: guard process registration and cleanup against PID reuse (#7425) (#7574)
* fix: guard process registration and cleanup against PID reuse A bare posix_kill($pid, 0) only proves some process owns the pid, so a recycled one blocked new tasks or drew a SIGTERM meant for something else. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: spawn probe children without a wrapping shell proc_open with a string command runs it via sh, so the child's /proc comm was sh and the identity check correctly refused to signal it on Linux. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: append the changelog entry at the end of the issue block Matches .github/copilot-instructions.md, which appends rather than inserting in numeric order. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: limit the process identity check to CLI callers The registry holds CLI tasks, so a web request comparing command names would never match and would report a running task dead. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use the full licence header on the new test files Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: treat CACTI_PATH_LOG as a directory and pass ctor args to the statement class Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: make process identity checks self-contained * fix(batchgapfix): guard forced process termination * fix(processes): preserve live pids on permission errors * Refresh security baselines after develop rebase Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Wait for the spawned child to exec before the identity check Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Serve fetch() from the buffered rows in the process registry fake db_table_exists() probes with SHOW TABLES, and db_fetch_row_prepared() reads a single row with fetch(); the fake drained the cursor on execute() and answered neither, so every registry query came back empty and the tests exercised the no-processes-table path instead of the guard. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(changelog): drop entries duplicated by an earlier rebase Only the issue#7425 line belongs to this branch. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(poller): identify a registered process by its executable, not its name Children are spawned from path_php_binary, so a versioned child and a parent started as plain php reported different command names for one interpreter. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(poller): skip the alias check where both paths share a command name Whether a second path to the interpreter yields a second name is the runner's business; asserting it failed where php is already the plain name. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
|
0a73b61d08 |
ci(security): add Cacti-aware Semgrep taint rules (#7730)
* ci(security): add Cacti-aware Semgrep taint rules Off-the-shelf PHP SAST does not model Cacti's request helpers, escaping wrappers, or db_* API, so it misses request-variable to SQL/shell/RRDtool flows and flags htmle()-escaped output as false positives. These rules teach the scanner Cacti's sources (grv/gnrv), sanitizers (db_qstr, sanitize_sql_column, cacti_escapeshellarg, html_escape, casts) and sinks. The SQL rule independently flags the host_id sink in lib/html_graph.php. The rule self-test is a hard gate; the whole-tree scan is advisory and its findings go to the code-scanning tab (grv() is validated where a filter is registered, which Semgrep cannot see, and taint over large files is not deterministic enough for a pass/fail baseline). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
github-actions[bot]
|
602a5e64cb |
ci: refresh security/SQL-interpolation baselines (#7737)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
a6da63ac89 |
Harden remote graph transport and response validation (#7631)
* Harden remote graph transport boundaries * Tighten remote graph SVG validation * fix: PHPStan Issues with emerging code * Refresh security baselines after develop rebase Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Match transport test to the explicit response-header default Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
github-actions[bot]
|
014d81489a |
ci: refresh security/SQL-interpolation baselines (#7659)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
github-actions[bot]
|
d75530f8ca |
ci: refresh security/SQL-interpolation baselines (#7654)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
github-actions[bot]
|
eaa1f022a9 |
ci: refresh security/SQL-interpolation baselines (#7653)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
edafd489a5 |
fix(poller): launch cactid without a shell (#7603)
* Launch cactid poller without a shell * Handle an unset poller PHP binary * fix(poller): reject null bytes in executable paths --------- Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
github-actions[bot]
|
bf45e21fd1 |
ci: refresh security/SQL-interpolation baselines (#7640)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
a75bf823bb |
feat: consolidate htmx loader and renderer abstraction global.php includes (#7517)
* feat(rendering): add view renderer abstraction Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * isolate renderer traversal test file * Standardize renderer headers and endpoint guard * style(renderer): use standard unit-test header in CactiRendererTest Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(renderer): reject null-byte paths before realpath Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh sink and hotspot baselines for line shifts Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(renderer): guard root base path and template-dir redirect Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(renderer): canonicalize both paths in index.php direct-access guard Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * hardening: normalize renderer base path to one separator Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(renderer): reject filesystem root as template path A /-rooted renderer collapsed the containment prefix to /, which str_starts_with() matches for every absolute path, so any file was includable. Fail closed by refusing a root template path. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * hardening: reject windows drive-root template path Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor: adopt PHP 8.4 idioms in view renderer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover renderer abstraction to full coverage Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * hardening(renderer): reject UNC template names and fix sink baselines Refuse a leading-backslash (UNC) template name the same way absolute POSIX paths are refused, skip the direct-request redirect when SCRIPT_FILENAME carries a null byte, and correct the renderer include and views redirect line numbers in the security baselines. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style: apply php-cs-fixer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(renderer): use PHP 8.1-compatible readonly property in CactiRenderer Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * hardening(rendering): drop template context array before include Unset the internal context array after extract so an included template only sees the extracted variables, not the full context. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines for renderer line shift Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(renderer): simplify containment check and drain nested output buffers The redundant real_path !== template_path comparison in validateTemplateFile() can never be false once is_file()/is_dir() have run, so drop it and keep the str_starts_with() prefix check as the actual containment guard. renderFile() also only unwound a single output buffer, so a template that opens (and leaves) its own buffer could leak an ob level or drop its content; unwind every buffer opened during rendering and reassemble them in the order they were written. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines for renderer buffer/containment fix The dynamic_include line for lib/renderer.php's include shifted again after adding drainOutputBuffers(). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(htmx): vendor htmx 2.0.6 and add opt-in filter loader Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover htmx filter loader Add missing-vendored-file case for htmx_script_tag and mark the pre-bootstrap read_config_option guard as uncoverable. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: harden htmx loader harness and tighten enable check Skip anything but an exact 'on' for htmx_enabled, reject 4xx/5xx fixture responses, fail fast when the fixture base path is unresolved, pin setup-php to a commit SHA, and reindent the e2e specs to tabs. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * style: apply php-cs-fixer to htmx test files Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(htmx): drop duplicate script-tag src assertion The subdir-install test repeated the preceding assertion; CACTI_PATH_URL is fixed at bootstrap so it could not exercise a distinct scenario. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(e2e): drop sqlite3 from docker-php-ext-install php:8.2-apache already ships sqlite3 built in, and its ext/sqlite3 source only carries config0.m4 (not config.m4), so building it standalone always fails with "Cannot find config.m4", breaking the e2e image build before Playwright ever runs. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(htmx): clear stat cache around version-file rename Earlier tests already stat the version file via file_exists()/ file_get_contents(), so PHP's stat cache can report it present right after these tests rename it away, making the assertions flaky. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(htmx): recompute script-tag cache-buster per call The md5 cache-buster was cached in a static, so FPM/script_server processes kept serving a stale ?v=... hash after an htmx.min.js upgrade. Recompute it per call, matching get_md5_include_js() and get_theme_paths() in lib/functions.php, and drop the 'v=' parameter name to match their path + '?' + hash shape. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(e2e): drop obsolete MariaDB options * ci(htmx): run the htmx integration test by path instead of a testsuite filter Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * build: vendor htmx unminified Debian rejects minified sources. The file is the htmx 2.0.6 dist source; its minified sibling hashes identically to what was committed, and the pinned SRI is recomputed over the unminified file. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: address htmx review feedback * test(e2e): resolve Cacti version through PHP * test(htmx): harden portable server cleanup * ci(actions): update checkout action pins --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
github-actions[bot]
|
085bee630d |
ci: refresh security/SQL-interpolation baselines (#7608)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas Vincent
|
906d19ca72 | fix(reports): consolidate image conversion and notification payload hardening (#7515) | ||
|
github-actions[bot]
|
616da95e6d |
ci: refresh security/SQL-interpolation baselines (#7588)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
f2a5ef0d51 |
fix: cast path_php_binary before passing it to string parameters (#7576) (#7577)
* fix: cast path_php_binary before passing it to string parameters The setting has no default, so it is null on a fresh install, and PHP 9 turns the resulting coercion from a deprecation into a TypeError. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: append the changelog entry at the end of the issue block Matches .github/copilot-instructions.md, which appends rather than inserting in numeric order. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: use the full licence header on the new test files Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: cast path_php_binary at every site that escapes it The first pass covered three of them; a repo-wide scan found 21, and the test now fails if a new one appears. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: strengthen PHP binary path guard * fix: cover default-config PHP binary reads --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
github-actions[bot]
|
967da9443d |
ci: refresh security/SQL-interpolation baselines (#7565)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
github-actions[bot]
|
2f1178fccf |
ci: refresh security/SQL-interpolation baselines (#7564)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Thomas VincentandTheWitness
|
7c5e4c102a |
feat(support): consolidate diagnostics, lockout CSRF, process-table hook, and safe-triage (#7514)
* feat(support): add redaction toggle and recent-log panel Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover support safe triage Unit-test the redaction helper, severity log tail and environment table, and add a Docker probe that the redacted report masks the RSA fingerprint and hides the database host and password. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): escape rsa fingerprint and default unknown poller type Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): redact compressed IPv6 addresses Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: skip tests tree in hardening pattern guard Test fixtures assign request superglobals to exercise product code; the guard only protects product code. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): drop redundant isset flagged by phpstan Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(support): guard snmpget path before executing Reject a null byte and require a regular executable file for path_snmpget so a misconfigured path cannot throw a ValueError or match a directory. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines for snmpget path guard Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): redact FQDNs before the node name and stop truncating tool output Masking the node name first could leave a domain suffix unmasked when the name was a prefix of an FQDN, and strip_tags() on installed SNMP output dropped legitimate angle-bracket content instead of just masking it. Also tighten the redact toggle to a strict '1' match, carry it through page-refresh URLs, and correct the diagnostics report comment to describe what actually gets masked. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): guard poller type lookup, log stats, and binary path checks Fall back to an "Unknown" label when poller_type doesn't match a known poller instead of an undefined-index warning, suppress warnings from filesize()/fopen() if the log file disappears between the caller's check and the open, and require is_file() plus a null-byte guard on the required-binaries version check (matches the snmpget check next to it). Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(tests): align triage probe comments with actual assertions The header comments claimed the unredacted report exposes the DB host and password, but the probe only ever asserts the RSA fingerprint; DB credentials are checked for absence when present, not required to appear unmasked. Correct the wording in both files. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): require POST + CSRF token for maintenance lockout, compare-and-set Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): require a valid expected state for lockout compare-and-set Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(security): refresh sink/hotspot baselines for upstream line shifts Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): clamp page offset, quote SQL literals, suppress lockout URL state Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover support lockout csrf guard Exercise the POST-only guard and compare-and-set branches, and add a Docker probe that a GET or tokenless POST cannot change the lockout state. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): guard lockout probe extraction and correct probe scope note Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: skip tests tree in hardening pattern guard Test fixtures assign request superglobals to exercise product code; the guard only protects product code. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): keep lockout e2e health poll alive under pipefail Tolerate grep returning non-zero before the Health field appears so the poll loop waits instead of exiting the script. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): add integration coverage for the lockout compare-and-set The Unit test cannot reach the "currently locked" branch (forced reads return the PHP_TESTING default) and the e2e probe deliberately never locks the system, so the real read/write round trip against cacti_lockout_status has no coverage. Add a fast integration test that drives the lock, a stale-expected rejection, and the unlock against a real database, skipping gracefully when none is reachable. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): match the page-clamp assertion whitespace-tolerantly Whitespace-exact substr_count() breaks on reformatting; the regex enforces the same guard without depending on exact spacing. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): harden the lockout e2e probe's auth and cookie-jar handling Assert a CSRF token was extracted and the session actually authenticated before running the lockout checks, otherwise a failed login still passes. Also handle tempnam() failure and clean up the cookie jar on every exit path via a shutdown handler. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * feat(support): add diagnostics export and environment checks Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover support diagnostics export Unit-test the environment table and report redaction, and add a Docker probe that checks the report never leaks the database host, password or full RSA fingerprint. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): guard null snmp version and copy-status styling Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): keep diagnostics textarea visible on copy failure and report real extension load state Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): verify PHP extensions in both cli and web contexts show_tech_environment() only checked utility_php_verify_extensions()'s web flag, which seeds CLI-only extensions like pcntl as satisfied without checking whether they're actually loaded. Shell out to cli_check.php for the real CLI state and combine it with the web state the same way lib/installer.php does, so status reflects reality in both directions. Also guard the source-extraction helpers in SupportDiagnosticsTest and the e2e probe against a moved/unreadable support.php instead of letting strpos(false, ...)/eval() fail confusingly. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): require POST + CSRF token for maintenance lockout, compare-and-set Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor(support): register process tables via support_process_tables hook Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): guard non-array hook result and empty process-table set Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): cast poller_interval to int in SQL and validate hook entry types Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(security): refresh sink/hotspot baselines for upstream line shifts Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): reuse forced admin lookup and coerce expected state Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover support process-table hook Verify the hook drops malformed plugin definitions and keeps valid ones, and add a Docker probe that the hook returns well-formed tables against a real database. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): guard hook extraction and tolerate empty health probe Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): escape plugin-contributed process-table cells Plugins can now supply the process-table SELECT fragments via the support_process_tables hook, so escape the rendered cell values, and install the e2e cleanup trap before backing up config.php. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * ci: refresh security baselines for support.php line shift Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): use form_selectable_ecell for process rows and cast pid in row id Plugin-contributed process-table values were only escaped via manual html_escape() calls, and the row id was built from an unsanitized pid; switch to the file's existing form_selectable_ecell() helper and cast pid to int before it goes into the id attribute. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): harden hook-call rewrite and narrow validation comment Replace the exact-string str_replace() with a regex plus a match-count assertion so the probe fails loudly instead of silently no-op'ing if support_process_tables() formatting drifts, and correct the header comment's claim that malformed definitions can't corrupt the UNION -- the validation only checks types, not SQL validity. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): make e2e hook extraction regex whitespace-tolerant The extraction pattern required exact spacing around the function signature; match flexible whitespace instead and fail loudly if preg_match doesn't return exactly one match. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): add integration coverage for the process tables SQL path Unit tests eval() support_process_tables() with the plugin hook stubbed and never touch a database; the e2e Docker probe calls the same function directly but never builds or executes the UNION show_cacti_processes() runs. Add a fast, DB-only integration test that executes the real query against a live schema, skipping gracefully when no database is reachable. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(support): harden process table hook contracts * docs: add changelog entry * style: satisfy support hook fixer rules * fix(support): reconcile escaping contract and diagnostics test after merge tech_env_status() now escapes its $text argument (safe-triage's design, tested by SupportSafeTriageTest); strip the now-redundant html_escape() calls at its call sites in show_tech_environment() to avoid double encoding. Update SupportDiagnosticsTest's source-extraction marker and eval() inputs to match the merged, redact-toggle-aware report block from safe-triage. Refresh the sink inventory baseline for the resulting line shift. * fix(support): escape the PHP uname value on both redact paths Hostnames and kernel strings can carry markup, and support_redact() itself substitutes <host>, so redact first and escape the result either way. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(support): locate the diagnostics report block by regex, not literal The $snmp_line assignment became a conditional and the literal start marker stopped matching, aborting the probe before it built a report. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs: append the process tables entry to the feature block Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |