Skip to content

fix: reject invalid DMARC_BUILDER policy and tag values - #5013

Merged
TomOnTime merged 1 commit into
DNSControl:mainfrom
spicysharkco:fix/dmarc-builder-validation
Oct 18, 2026
Merged

TomOnTime merged 1 commit into
DNSControl:mainfrom
spicysharkco:fix/dmarc-builder-validation

Conversation

@spicysharkco

Copy link
Copy Markdown
Contributor

DMARC_BUILDER has checks for policy, subdomainPolicy, nonexistentSubdomainPolicy, publicSuffixDomain and testMode, but none of them ever fires. A typo in one of those values goes into the TXT record unchanged.

Config:

var REG_NONE = NewRegistrar("none");
var DNS_NONE = NewDnsProvider("none");

D("example.com", REG_NONE, DnsProvider(DNS_NONE),
    DMARC_BUILDER({
        policy: "quarentine",
        rua: ["mailto:dmarc@example.com"],
    })
);

dnscontrol check prints No errors., and dnscontrol print-ir shows the record _dmarc with the TXT value v=DMARC1; p=quarentine; rua=mailto:dmarc@example.com. preview would create that record. After the fix dnscontrol check stops with executing dnsconfig.js: Invalid DMARC policy. The other four options behave the same way. For example testMode: "yes" is written as t=yes. The documented values are the ones in DMARC_BUILDER.md.

Cause: each check is written as !value.x === "lit", which parses as (!value.x) === "lit". A boolean never equals a string, so the condition is always false and the throw is dead code. It is the same in all five places: policy (L1866-L1872), subdomainPolicy (L1879-L1885), nonexistentSubdomainPolicy (L1891-L1897), publicSuffixDomain (L1994-L2000) and testMode (L2008-L2010).

Fix: compare the value itself and join the alternatives with &&: throw when the value is none of the allowed ones. policy already defaults to "none" above its check. subdomainPolicy and nonexistentSubdomainPolicy are optional, so their checks start with value.x &&. The other two sit inside an if (value.x) already. Configs that carry a value outside the documented list now fail check instead of publishing it. Matching is case-sensitive, like the documented values and the alignmentSPF and alignmentDKIM options, so policy: "Reject" now fails too. This is a behaviour change for those configs; I can mark the title with ! if you prefer.

The new TestDMARCBuilderValues in pkg/js/js_test.go has two rows that must pass (only policy, and every option with a valid value) and one row per option with a bad value. It is not a TestErrors row: that table passes the script text to ExecuteJavaScript, which expects a file name, so the helper is never reached there. The test calls ExecuteJavascriptString.

go test -count=1 -run '^TestDMARCBuilderValues$' ./pkg/js/
go test -p 1 -count=1 ./pkg/js/

Before the fix the five bad-value rows fail and the two valid rows pass. After the fix all 7 pass, and the package passes. Putting back any one of the five conditions fails its own bad-value row. Dropping the value.x && guard from subdomainPolicy or nonexistentSubdomainPolicy makes an unset option throw, and the rows that do not set it fail. gofmt -l, go vet ./pkg/js/ and git diff --check are clean, and prettier 3.9.9 leaves helpers.js unchanged. No generated file changes.

Overlap checked: searched open and closed issues and PRs for DMARC_BUILDER, DMARC policy validation and DMARC invalid policy, and open PRs for DMARC. #4823 and #4794 changed this helper, #4855 fixed its TypeScript types, and #4378 and #4370 are issues about it. None touches these checks.

The checks for policy, subdomainPolicy, nonexistentSubdomainPolicy,
publicSuffixDomain and testMode were written as `!value.x === "lit"`,
which parses as `(!value.x) === "lit"` and is never true. A typo such
as policy: "quarentine" was written into the TXT record unchanged.

Compare the value itself, and for the optional fields only when it is
set. Add TestDMARCBuilderValues, which runs the script text with
ExecuteJavascriptString.
@TomOnTime
TomOnTime merged commit ea88bee into DNSControl:main Oct 18, 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

Development

Successfully merging this pull request may close these issues.

2 participants