Skip to content

fix: ship Net_DNS2 with the plugin again - #345

Open
somethingwithproof wants to merge 18 commits into
Cacti:developfrom
somethingwithproof:fix/restore-bundled-dns-dependency
Open

fix: ship Net_DNS2 with the plugin again#345
somethingwithproof wants to merge 18 commits into
Cacti:developfrom
somethingwithproof:fix/restore-bundled-dns-dependency

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Sep 3, 2026

Copy link
Copy Markdown
Member

Three regressions on current develop, all introduced by #336.

1. Missing Composer autoloader. Cacti plugins install by unpacking a tarball, so a runtime Composer dependency is never present on a user's system. vendor/ was not committed, and mactrack_resolver.php:45 required its autoloader unconditionally. require_once on a missing file is fatal, so the friendly "run composer install" message two lines below was unreachable and the resolver died on every install that had not run Composer by hand. setup.php guarded its own load correctly; the resolver did not. Reported on #342, where Cacti caught the fatal and disabled the plugin.

2. db_qstr_rlike() does not exist before Cacti 1.2.31. mactrack_view_interfaces.php:48 calls it. Checked every tag: absent in release/1.2.14 through release/1.2.30, present only in release/1.2.31, and absent on Cacti develop. INFO declares compat = 1.2.14, so the Interfaces view is an undefined-function fatal on 17 of the 18 supported 1.2 releases and on 1.3. Filed as #347.

3. The same call strips the Ports to Ignore regex. db_qstr_rlike() removes |, { and }, so the shipped default (Vlan|Loopback|Null) becomes (VlanLoopbackNull). Verified against MariaDB: 'Vlan10' NOT RLIKE '(Vlan|Loopback|Null)' is 0, 'Vlan10' NOT RLIKE '(VlanLoopbackNull)' is 1. Every Vlan, Loopback and Null interface leaks into the Issues views and "Ignored Interfaces" returns nothing. Filed as #347.

4. The plugin never activates. plugin_mactrack_check_config() gates enablement on loading Net_DNS2, whose autoloader uses relative includes and cannot resolve from Cacti's working directory, so a clean install parks at status 2, "needs configuration". The gate did not exist before #336 and flowview does not have one. Filed as #348.

Net_DNS2 now ships under Net/ again, at 1.5.5, the version composer.lock already pinned. The library code is therefore unchanged rather than reverting to the 1.5.0 the old bundle carried. plugin_flowview ships the same library the same way.

What the branch does:

  1. Ship Net_DNS2 with the plugin. Bundles 1.5.5, drops composer.json and composer.lock, points both load sites at Net/DNS2.php, which self-registers its autoloader. Three tests asserted on the Composer wiring and were updated with it.

  2. Quote the Ports to Ignore pattern with db_qstr() instead of db_qstr_rlike(). That keeps the injection fix from fix: harden Mactrack release boundaries #336, leaves the regex intact, and drops the dependency on a helper that only exists in 1.2.31. An earlier commit on this branch added a function_exists shim for that helper; it is removed again, since not calling it at all is the better answer.

    To be explicit about what this gives up: db_qstr_rlike() does two things, escaping and stripping |, {, } to bound regex backtracking. This keeps the escaping and drops the stripping. That is deliberate. mt_ignorePorts is an admin-only setting whose whole purpose is to hold a regular expression, so mangling its metacharacters does not harden it, it disables it. The pre-fix: harden Mactrack release boundaries #336 code interpolated the value with no quoting at all, which was the actual injection; db_qstr() closes that. Binding the value as a query parameter would keep both properties, but $sql_where here is a concatenated string consumed by db_fetch_assoc() in three places, so that is a separate refactor rather than something to fold into a regression fix.

  3. Stop gating enablement on the DNS library. plugin_mactrack_check_config() goes back to its pre-fix: harden Mactrack release boundaries #336 body. The resolver adds the plugin directory to include_path so the bundled autoloader no longer depends on chdir(), and guards its require_once with is_file() so the diagnostic below it is reachable.

  4. Check coding standards against Cacti's ruleset. Adds .php-cs-fixer.php using core's rules plus a CI job. The plugin already matched them without carrying the config; only the new code needed reformatting.

  5. Stop calling functions that do not exist. The ARP export called format_mac_address(), which is defined nowhere in the plugin or core; the identical line in mactrack_view_macs.php:461 calls mactrack_format_mac(). The ajax graph-settings action lost its handler in 75b4d2b in 2016 and nothing requests it.

Verification:

Disposable Cacti 1.2.31 + MariaDB 10.11 install, driven by cli/install_cacti.php
and cli/plugin_manage.php --install --enable:

plugin_config.status                          1 (Active), was 2 before this branch
mactrack_view_devices.php                     HTTP 200
mactrack_view_interfaces.php?issues=-1        HTTP 200
mactrack_resolver.php run from /tmp           starts, prints usage
tests/Unit, tests/Integration, tests/e2e      5 of 5 pass
php-cs-fixer                                  0 of 50 files can be fixed

The RLIKE change checked against the running MariaDB:

'Vlan10'  NOT RLIKE '(Vlan|Loopback|Null)'  -> 0   suppressed, correct
'Vlan10'  NOT RLIKE '(VlanLoopbackNull)'    -> 1   what develop does today
'Gi1/0/1' NOT RLIKE either                  -> 1   real ports unaffected

Also, for the record on the PHP floor: all 67 runtime files parse on 7.4, a
tokenizer sweep of all 428 call sites finds no PHP 8 only builtin, and the
bundled Net_DNS2 1.5.5 instantiates on 7.4. Net_DNS2 aside, nothing here needs
more than 7.4, whatever floor the project settles on.

One note for review. 86 of the 105 changed files are the vendored library. Reviewing only what was written by hand is 19 files and 326 added lines:

git diff develop...HEAD -- . ':(exclude)Net/'

Net/README.md returns with a provenance record: upstream repository, tag v1.5.5, commit ea39ef5a97d5c2b9893a8c35af7b5fd5b0e40bc9, and what changed between it and the 1.5.0 the old bundle carried. README.md also drops the composer install step and corrects its stated PHP floor from 8.2 to 8.1, which is what Cacti 1.2 requires and what this plugin's CI already tests.

Closes #346, #347, #348.

Closes #347
Closes #348

Cacti plugins install by unpacking a tarball, so a runtime Composer
dependency is never present on a user's system. vendor/ was not committed and
mactrack_resolver.php required its autoloader unconditionally, which made the
resolver fatal on every install that had not run composer by hand.

Net_DNS2 1.5.5 is bundled under Net/, the version composer.lock already
pinned, matching how plugin_flowview ships the same library.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The function came from a 1.2.x security backport and has no equivalent on
develop, so mactrack_view_interfaces.php is fatal there. Removing the call
would put the RLIKE injection back, so the fallback mirrors core's body.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The plugin already matched core's php-cs-fixer rules without carrying the
config, so nothing but the new fallback needed reformatting.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The ARP export called format_mac_address(), which is defined nowhere; the
identical line in mactrack_view_macs.php calls mactrack_format_mac(). The ajax
graph-settings action lost its handler in 75b4d2b back in 2016 and nothing
requests it.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Every branch of mactrack_format_mac() returned early on a known format and
the function fell off the end otherwise, so an unset setting rendered null
in place of each address.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The resolver CLI entrypoint still require_once()s the bundled Net_DNS2 file without an is_file() guard, so a missing file would fatal before the intended diagnostic can run.

Pull request overview

This PR restores a tarball-friendly dependency model by bundling Net_DNS2 back into the plugin (removing the runtime Composer requirement) and adds compatibility shims/fixes to prevent fatals on newer Cacti core variants.

Changes:

  • Bundle Net_DNS2 v1.5.5 under Net/ and update resolver/setup/tests/docs to load Net/DNS2.php instead of vendor/autoload.php.
  • Add a guarded fallback implementation of db_qstr_rlike() for cores that don’t provide it, preserving safe interface filtering.
  • Remove calls to non-existent/dead handlers (ARP export MAC formatting, AJAX graph-settings action) and add a php-cs-fixer ruleset plus CI job.
File summaries
File Description
.github/workflows/code-quality.yml Add php-cs-fixer standards check job and keep lint/phpstan flows consistent with bundled dependency model
.gitignore Ignore php-cs-fixer cache file
.php-cs-fixer.php Add Cacti-core-aligned php-cs-fixer configuration (excluding Net/, tests/, etc.)
CHANGELOG.md Document new fixes/compatibility changes
README.md Remove Composer install step and adjust documented PHP floor
composer.json Remove Composer manifest (no runtime Composer dependency)
lib/mactrack_functions.php Add db_qstr_rlike() fallback and ensure MAC formatting fallback returns the raw MAC
mactrack_ajax.php Remove dead save_graph_settings action dispatch
mactrack_resolver.php Load bundled Net_DNS2 via Net/DNS2.php
mactrack_view_arp.php Use mactrack_format_mac() instead of non-existent format_mac_address()
setup.php Guard and load bundled Net_DNS2 during plugin enablement checks
tests/Unit/test_device_type_sql_safety.php Update assertions to validate bundled Net_DNS2 wiring
tests/e2e/bootstrap-mactrack.sh Update install check to require bundled Net_DNS2 entrypoint
tests/e2e/mactrack_smoke.php Update smoke test to load bundled Net_DNS2 entrypoint
Net/LICENSE Add bundled Net_DNS2 license text
Net/DNS2.php Bundled Net_DNS2 v1.5.5 entrypoint (autoload registration)
Net/DNS2/BitMap.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Cache.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Cache/File.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Cache/Shm.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Exception.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Header.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Lookups.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Names.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Notifier.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Packet.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Packet/Request.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Packet/Response.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/PrivateKey.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Question.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/RR.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/RR/A.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/AAAA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/AFSDB.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/AMTRELAY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/ANY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/APL.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/ATMA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/AVC.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/CAA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/CDNSKEY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/CDS.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/CERT.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/CNAME.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/CSYNC.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/DHCID.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/DLV.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/DNAME.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/DNSKEY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/DS.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/EID.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/EUI48.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/EUI64.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/HINFO.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/HIP.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/IPSECKEY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/ISDN.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/KEY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/KX.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/L32.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/L64.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/LOC.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/LP.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/MX.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NAPTR.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NID.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NIMLOC.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NS.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NSAP.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NSEC.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NSEC3.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/NSEC3PARAM.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/OPENPGPKEY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/OPT.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/PTR.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/PX.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/RP.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/RRSIG.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/RT.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/SIG.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/SMIMEA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/SOA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/SPF.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/SRV.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/SSHFP.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TALINK.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TKEY.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TLSA.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TSIG.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TXT.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/TYPE65534.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/URI.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/WKS.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/X25.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/RR/ZONEMD.php Bundled Net_DNS2 v1.5.5 RR implementation
Net/DNS2/Resolver.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Socket.php Bundled Net_DNS2 v1.5.5 library file
Net/DNS2/Updater.php Bundled Net_DNS2 v1.5.5 library file
Review details
  • Files reviewed: 90/100 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Copy link
Copy Markdown
Member Author

Fixed in 84197f0 — the resolver now guards the load with is_file() and adds the plugin directory to include_path, so the diagnostic below it is reachable.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are unresolved security/documentation issues (notably unsafe unserialize() usage in vendored cache paths and a PHP version floor mismatch in README) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

README.md:33

  • README prerequisites state PHP 8.1+, but the repo has a PHP 7.4 compatibility contract (tests/Security/Php74CompatibilityTest.php) and the PR description asserts runtime files parse on 7.4. This mismatch can mislead users about the supported runtime floor.

Net/DNS2/Cache/File.php:176

  • Unserialize is used to reload and merge the cache metadata array without restricting classes. Use allowed_classes => false here as well to prevent object instantiation from a tampered cache file.
    Net/DNS2/Cache/Shm.php:217
  • Unserialize is used to reload and merge the cache metadata array without restricting classes. Use allowed_classes => false to prevent object instantiation if the shared segment is tampered with.
  • Files reviewed: 92/105 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread Net/DNS2/Cache.php
Comment thread Net/DNS2/Cache/File.php
Comment thread Net/DNS2/Cache/Shm.php Outdated
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It introduces a large vendored dependency tree plus multiple runtime/CI behavior changes, so a final human pass is warranted even though the targeted regressions appear addressed.

Review details
  • Files reviewed: 93/107 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread README.md Outdated
@somethingwithproof somethingwithproof added bug QA Bug found in QA dependencies Dependabot dependencies updates labels Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug dependencies Dependabot dependencies updates QA Bug found in QA

Projects

None yet

2 participants