Skip to content

Resolve IPv6 sender addresses and exercise an alert in CI (stacked on #422) - #424

Merged
bmfmancini merged 2 commits into
fix/review-hardeningfrom
fix/ipv6-hostname-and-ci-data
Oct 25, 2026
Merged

bmfmancini merged 2 commits into
fix/review-hardeningfrom
fix/ipv6-hostname-and-ci-data

Conversation

@somethingwithproof

Copy link
Copy Markdown
Member

Description

Stacked on #422 (base fix/review-hardening). Merge #422 first; this PR must merge after it and should be retargeted to develop once #422 lands.

Hostname resolution now handles IPv6, and the CI test data now contains an alert rule that fires.

  • syslog_resolve_incoming_hosts() used IPv4-only gethostbyname(), so a sender known only by an AAAA record, or by an IPv6 address, was marked unresolved-. Names are now checked with dns_get_record() for A and AAAA, falling back to gethostbyname(). IPv4 and IPv6 literals need no lookup and do not use the lookup budget from Harden alert commands, upgrades and dashboards, and fix review findings #422. IPv6 text is normalised with inet_pton()/inet_ntop() (compressed, lower case; brackets removed, zone suffix kept) before it is compared with Cacti devices and stored. Harden alert commands, upgrades and dashboards, and fix review findings #422's lookup budget, its "left as received" handling when the budget is spent, and syslog_no_dns are unchanged. The function gains an optional last argument, $dns_records, so tests can supply DNS answers.
  • tests/bin/populate_syslog_incoming.sh inserted its alert rule without suppression_schedule (text NOT NULL, no default), and with mismatched column and value counts, so the insert failed under strict mode and CI never had a rule to evaluate. The insert is fixed, the script uses set -euo pipefail, and the removal rule uses REPLACE INTO.
  • The "force Syslog Plugin Poller" step now asserts that the alert fired (log line and a syslog_logs row for host bob). "Check Cacti Logs for Errors" still fails on any FATAL:, ERROR: or WARNING: line except that one alert line, matched literally.
  • HostnameLookupBudgetTest used the literal 192.0.2.1 as a name that fails to resolve. Literals now count as resolved, so it uses no-such-sender.invalid.

Related Issue

None.

Motivation and Context

IPv6-only senders were reported as unresolved. The CI gap meant alert evaluation was never exercised end to end.

How Has This Been Tested?

Executed, on Cacti 1.2.x and on develop, on top of #422 head f93418a:

  • Pest: 589 passed on PHP 8.4 on both cores. New tests in tests/Unit/HostAddressResolutionTest.php cover IPv4 literal; IPv6 compressed, expanded, mixed case, bracketed, mapped and zoned; A only, AAAA only, both, none and a failed query; hosts-file fallback; syslog_no_dns on; and the budget: AAAA-only names resolve within the budget, are left as received with one budget log line when it is exhausted, literals skip the budget, and no lookup is made when syslog_no_dns is on, with either budget.
  • php tests/bin/patch-coverage.php coverage/clover.xml f93418a 100: 100.00% (31/31 lines).
  • php tests/bin/check-i18n-pot.php f93418a: OK.
  • CI-order replay against MariaDB on both cores: the five database regression scripts and cli_schema_upgrade_test.php pass (including review_fixes_paths_database_test.php, 124 checks), populate and processor give one alert-fired line and one syslog_logs row for bob, live_pipeline_test.php passes (39 checks) as www-data, and nothing remains after the log-check filter.

Not run: a real AAAA lookup against a live resolver (stubbed in tests); the GitHub workflow itself; the optional Xdebug dump and coverage merge from #422 (no Xdebug in the replay containers).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation repository accordingly.

README gains a short note on IPv6 senders.

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

Copy link
Copy Markdown

@bmfmancini
bmfmancini merged commit 3e16932 into fix/review-hardening Oct 25, 2026
2 checks passed
TheWitness pushed a commit that referenced this pull request Nov 4, 2026
…gs (#422)

* fix: substitute alert command tokens in a single pass

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

* fix: keep rule tables when upgrade truncate is chosen

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

* fix: keep asterisks in host-level alert rule expressions

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

* fix: count alert rules one by one when the combined query fails

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

* fix: apply the report row limit to threshold alerts

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

* fix: validate imported alert rules and ignore imported row ids

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

* fix: group the combined dashboard queries per log table

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

* fix: filter staging rows before joining reference tables in archive rules

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

* fix: reprocess only the selected removal rule

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

* fix: widen rule owner columns to the Cacti username length

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

* fix: bound hostname lookup time per processing run

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

* fix: validate imported saved search templates

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

* test: cover alert rules without a query in the count fallback

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

* test: add database regression checks for the review fixes

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

* test: extract page functions through temporary files, not eval

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

* test: repair the E2E harness for the current schema and Playwright image

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

* test: add E2E checks for the review fixes

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

* test: give the schema CLI fixture the owner column helpers

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

* test: build the stub hash from random bytes

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

* test: use bound values and fixed names in the database regression script

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

* test: build the E2E base address from the environment

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

* fix: reject an imported alert that has no query to run

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

* test: share the database setup between the regression scripts

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

* test: add database path checks for install choices, dashboards, removal, alerts and imports

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

* test: run the processor end to end against an installed Cacti in CI

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

* test: add an optional coverage dump for the real entry points

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

* test: check the upgrade advisor text

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

* test: extend the E2E run with import, reprocess, dashboard and owner paths

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

* test: set the DNS option in one assignment

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

* Resolve IPv6 sender addresses and exercise an alert in CI (stacked on #422) (#424)

* fix: look up A and AAAA records and accept IPv6 sender addresses

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

* test: populate a valid alert rule in CI and assert that it fires

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

---------

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

* fix: use standard schema upgrades for rule owners

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

* fix: use SHA-256 for threshold settings fingerprints

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

* test: exercise owner schema upgrades through the database fixture

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

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants