My test suite came back green. 119 passed, 3 skipped, under a second.
At that exact moment, my tool was telling people to do the one thing a test in that suite promised it never told anyone to do. The test was named after the promise. It ran. It passed.
That gap between what a test is named and what it actually checks is the most expensive thing I have found in this project. I do not think it is rare, and the shape of it transfers to almost any codebase.
The 30-second version of what the tool does
I built a scanner that checks a company's email authentication setup from public DNS records, then generates a plain-English page telling them what to publish to fix it. Small accounting firms, mostly. They take that page to whoever runs their DNS and paste in the records.
That last part is the whole risk. The output is not a report someone reads and nods at. It is a set of instructions someone follows. If the instruction is wrong, a real business loses real mail.
The relevant feature is a staged rollout, a ladder. Each rung is one DNS record to publish, in order, with a stage label attached.
The advice it got wrong
One DMARC setting matters here: a tag called t.
A domain can publish "reject anything that fails authentication" and simultaneously publish t=y, which means "but don't actually do that yet, I'm still testing." Think of t=y as a parking brake. The policy is set. The brake is on.
Removing that brake is safe once your mail is properly signed. Removing it before that is how a company starts bouncing its own invoices.
So the tool has a rule. If a domain has the brake on and no working mail signing, do not tell them to take the brake off. Tell them to leave it alone and go fix signing first. That rung exists, it works, and it is the first thing on the page:
stage : Not yet - keep the t=y tag until DKIM is signing
value : v=DMARC1; p=quarantine; t=y; rua=mailto:r@example.com
Correct. Brake intact.
Here is the second rung on that same page, for that same unsigned domain:
stage : Next - after 2 weeks of clean reports, and only once DKIM is signing
value : v=DMARC1; p=reject; sp=reject; rua=mailto:dmarc@x.test; fo=1;
No t tag anywhere in it. Step one says keep the brake on. Step two hands over a record with the brake gone and the policy escalated, on the same page, to the same client.
The test that was written to prevent this
This is the part that should bother you, because it is the part I would have sworn was covered:
def test_no_rung_offered_to_an_unsigned_domain_removes_its_test_mode_brake(
label: str, record: str, weights: Weights
) -> None:
before = assess(record, weights, dkim=False)
if not before.dmarc.policy_test_mode:
pytest.skip("not in test mode; no brake to remove")
brake_removed = strip_test_mode_tag(record)
for rung in dmarc_rollout("x.test", before):
assert rung.value != brake_removed
Read the name. Then read the assertion.
The name claims: no rung removes the brake.
The assertion checks: no rung equals this one specific string. That string is the input record with the tag deleted and every other byte left identical.
Those are not the same claim, and the distance between them is exactly where four bad outputs were living. The offending rung was v=DMARC1; p=reject; sp=reject; rua=mailto:dmarc@x.test; fo=1;. It has no brake. It is also not the input-minus-the-tag, because the policy changed and the other tags were regenerated. Different string. != holds. Test passes. Bug ships.
The test was not missing. It was not skipped. It ran on exactly the right input, in exactly the right scenario, and reported success.
Three more guards, all blind, all reasonable
This is the part I want any engineer reading to sit with. It was not one weak test. It was four, and each one was defensible on its own.
Guard 1. The string comparison above. It checked one exact value out of an unbounded set of wrong values.
Guard 2. "No suggested record drops the tag." Same technique, same blind spot. Two tests, one assumption, zero independence. Two guards that share an assumption are one guard wearing a disguise.
Guard 3. Publish the advice, then re-scan. Good design, this one: take the record the page tells you to publish, publish it in a simulated zone, run the whole assessment again, confirm the domain is still braked. It passed because it read only the first rung. The first rung was the correct one. It never looked at rung two.
Guard 4. "Following any step must not introduce a critical finding." The finding it watches for is triggered by the policy value alone. On a domain already set to quarantine, that finding had already fired. You cannot introduce something that is already present. The guard was structurally incapable of failing on this input.
Four layers. One live path straight through all of them.
How I actually found it
Not by reading the code. I had read that function. I had an opinion about it. My opinion identified one faulty code path.
What I did instead was enumerate. I wrote a throwaway script that took every relevant input state, ran it through the ladder, and printed every single rung it produced into a table. One column ran each generated record back through the real parser and asked, plainly, "does this still have the brake on?"
Eighteen rungs. Four with the brake removed.
| input | stage | brake in output? |
|---|---|---|
p=quarantine; t=y; rua=… |
Next - ...only once DKIM is signing |
gone |
p=quarantine; t=y; adkim=s; ruf=… |
Next - ...only once DKIM is signing |
gone |
p=reject; t=y (no reporting) |
Repair |
gone |
p=reject; t=y; pct=20; rua=… |
Repair |
gone |
Three separate code paths, not one. My reasoning had found a third of the problem and felt finished.
Now look at the bottom two rows. Those are labeled Repair. Not "only once DKIM is signing." Just Repair. Flat, unqualified, no warning attached anywhere. The safety caveat had been added to the escalation steps and never to the repair steps, because when that caveat was written nobody was thinking about repairs. A client reads a step called "Repair," assumes repairs are safe, and pastes it in.
The root cause was mundane, by the way. A branch appended its rung and did not return. Execution fell straight through into the three branches below it.
What I take from this
A test's name is documentation, and documentation drifts. The name was written when the intent was fresh. The assertion was written to the example in front of me that day. Nobody reconciled them afterward, because the test was green and green means done.
Assert on the property, not on a value. != "one_known_bad_string" is not a guarantee. Parsing the output and asking "does this have the property I care about?" is. Same test, ten more lines, and it would have caught all four.
Count your independent guards, not your total guards. I had four. Two shared a technique, one read only the first element, and one was watching a condition that was already true. That is not four layers of defense. That is one, and it was the weak one.
Enumerate the output space. Every real error in this project, mine and the AI assistant's I use on it, came from reasoning about what code would do instead of running it and printing what it did. Reasoning gave me one code path. A table gave me three. The table took fifteen minutes.
Mutation-test in both directions. A guard that should fire must be shown to fire. A guard that should stay quiet must be shown to stay quiet. An assertion no one has ever seen fail is a hypothesis, not a test.
Where it stands
I caught this in a working tree, mid-change, before the feature was finished. Not from a client call. That is the good outcome, and I only got it because I stopped trusting a green checkmark long enough to enumerate what the thing actually emits.
The fix is not obvious, which is why it is still open. Carrying the brake tag forward onto an escalation step means handing someone a record that says "reject everything, but not yet," which is confusing on its own terms. Suppressing those rungs entirely means the page goes quiet about a real next step. Both are defensible. That is a product decision, not a test decision, and I would rather sit with it a day than patch it because a table embarrassed me.
The test is getting rewritten either way. Right now its name is a promise its assertion does not keep, and that is worse than having no test at all. A missing test is an obvious gap. A lying test is a gap you have already convinced yourself is covered.
Go look at your suite. Find a test whose name makes a broad claim. Read what it actually asserts.
I would bet you find at least one.
For the curious
The sweep. The whole thing was about forty lines. Every input state, through the ladder, one row per rung, and one column that did the actual work:
def parses_as_test_mode(value: str) -> bool:
"""Run a candidate record back through the real parser."""
res = dmarc_check(FakeResolver({("_dmarc.probe.test", "TXT"): [value]}), "probe.test")
return bool(res.policy_test_mode)
That is the difference between the guard that missed and the one that would have caught it. The failing guard asked "is this string equal to that string." This asks "does the thing I am about to hand a client still have the property I promised." Same effort. Different question.
Why the parking brake is only half a brake. RFC 9989 is the current DMARC standard, and it defines t in section 4.7 with a default of n. It obsoletes RFC 7489. That transition is the part worth knowing: a receiver still running the older spec does not recognise t at all. It ignores the tag and applies the published policy anyway. So the brake works on some receivers and not others, which is why the tool never tells anyone the tag is protecting their mail. It only ever says the tag is why some receivers are holding off.
What the test names look like now. The suite reads as sentences, which is how I noticed the problem at all. test_no_rung_offered_to_an_unsigned_domain_removes_its_test_mode_brake is a claim you can check against its own body. A test called test_rollout_3 is not. Naming tests as assertions is worth doing on its own merits, and it comes with a cost nobody mentions: a name that specific will eventually promise more than the assertion delivers, and you will believe the name.
The code. The scanner is at Mailauth, bug and all. Passive DNS only, no probing, MIT licensed.
Top comments (0)