Skip to content

Trim and merge audit:dns:* keys like audit:accept:* - #9953

Merged
jasonlyc merged 1 commit into
firewalla:masterfrom
cristianchiriac:fix-audit-dns-retention
Oct 16, 2026
Merged

jasonlyc merged 1 commit into
firewalla:masterfrom
cristianchiriac:fix-audit-dns-retention

Conversation

@cristianchiriac

Copy link
Copy Markdown
Contributor

Fixes #9946.

With dnsmasq_log_allow_redis on, allowed DNS records are written to audit:dns:<mac> and audit:dns:system. Two things were missing for these keys:

  • OldDataCleanSensor.registerFilterFunctions: no filter matched audit:dns:, so regularClean never trimmed them. The auditAccept filter now also matches audit:dns:, so these keys use the same retention (expire, count and the DNS-flow platform multipliers) as audit:accept:*. This follows option 1 from the issue, so no new config entry is needed.
  • ACLAuditLogPlugin: audit:dns:system was missing from the default touchedKeys, in the constructor and in the reset in mergeLogs. Only per-mac keys were added on write, so mergeLogs never compacted the system key. It's now in both defaults.

Test: test/test_audit_dns_retention.js loads both sensors through proxyquire. Every module they require is replaced by a stub, except Constants.js and CommonKeys.js.

  • OldDataCleanSensor: audit:dns:system and audit:dns:<mac> match exactly one filter, auditAccept. audit:accept:* and audit:drop:* keep their filters.
  • ACLAuditLogPlugin.mergeLogs: with a stub rclient, two merges in a row both read audit:dns:system. The second merge checks the reset touchedKeys.

Both tests fail without the fix and pass with it. I ran them with mocha 11 on Node 26, as in #9909.

With dnsmasq_log_allow_redis on, allowed DNS records go to audit:dns:<mac>
and audit:dns:system. OldDataCleanSensor had no filter for these keys, so
they were never trimmed, and mergeLogs never compacted audit:dns:system.

Fixes #9946
@jasonlyc
jasonlyc merged commit 9c95e24 into firewalla:master Oct 16, 2026
1 check passed
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.

audit:dns:* zsets are never trimmed or merged when dnsmasq_log_allow_redis is on

2 participants