[Silthus]

Problem

• A customer with a DMARC policy who clicks "Configure automatically" for an email sender at Cloudflare ends up with two DMARC records.
• Receivers then ignore DMARC for the domain, so the customer's existing policy stops working.
• Cloudflare ignores Domain Connect TXT conflict matching (docs), so the template's _dmarc record is added next to the existing one.

Changes

• The apply URL includes the template's dmarc group only when DNS confirms that _dmarc.<sender domain> has no DMARC record.
• An existing record or a failed lookup sends groupId=verification,dkim,spf,mailfrom, so the provider applies every other record.
• Failing closed is deliberate: a missing DMARC record is visible and fixable, a duplicate silently breaks the policy.
• groupId sits inside the signed query string, as the Domain Connect spec requires.
• Nothing changes for the reverse proxy flow or for domains without DMARC.

Note

Cloudflare's docs do not list groupId among the parameters it ignores, but a live Cloudflare apply was not run. If Cloudflare turns out to ignore it, the fallback is to hide auto-configure when _dmarc already exists.

How did you test this code?

Test rationale: two existing tests in posthog/test/test_domain_connect.py were extended rather than adding new ones.

• test_email_resolver_variables_match_template gains four parameterized DNS outcomes: no record, no answer, an existing record, a timeout. It catches the duplicate coming back, a lookup failure that fails open, and a template group rename that groupId would no longer match.
• test_url_with_signing now verifies the signature over a query that carries groupId. It catches groupId being appended after signing, which the provider would reject.
• Breaking each guarded line on purpose made the matching case fail.
• Not run: a live Domain Connect apply against Cloudflare or Vercel.

Release status

• No feature flag controls this change
• This change is behind a feature flag and is not available to users
• This change makes a previously flagged feature available to everyone

Automatic notifications

• Publish to changelog?

Docs update

None. No doc under docs/ covers email domain setup.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (1M context)

• Part of a redesign of first-time email domain setup in Workflows. This fix ships first and on its own because it is live and independent of the wizard.
• Skills invoked: /writing-tests, /writing-code-comments, /reviewing-with-coderabbit, /writing-pr-descriptions.
• A Codex adversarial review found that the first version read DMARC presence from the verify result, which reports a DNS timeout the same as no record. The fix now does its own lookup and fails closed.
• CodeRabbit CLI pass skipped: the CLI was signed out and nobody was available to sign in. The bot review on this PR covers it.
• The diff-aware devex and security semgrep rules ran locally with no findings.
• Duplicate search (dmarc domain connect, groupId domain_connect) found no open PR.

🤖 Generated with Claude Code

[trunk-io]

😎 Merged successfully - details.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (6)

.agents/security.md — configured
.agents/skills/sending-notifications/SKILL.md — configured
.agents/skills/writing-tests/SKILL.md — configured
docs/internal/person-data-access.md — configured
.claude/commands/conventions.md — configured
.agents/skills/writing-code-comments/SKILL.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 54043d56-d5d9-46ac-b01f-4b58304a754b

📥 Commits

Reviewing files that changed from the base of the PR and between d161996 and 3652b28.

📒 Files selected for processing (2)

• posthog/domain_connect.py
• posthog/test/test_domain_connect.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

---

📝 Walkthrough

Walkthrough

Email context resolution checks the sender domain for a DMARC TXT record and selects template groups based on the lookup result. It stores the selected group IDs in the context and passes them to apply URL generation. When group IDs are present, generated URLs include a comma-separated groupId parameter. TXT value normalization is shared with endpoint discovery. Tests cover DMARC lookup outcomes, URL encoding, and signature verification.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3652b

Confirmed DMARC absence still allows the DMARC template, while existing records and lookup failures exclude it without invalid TXT data blocking URL generation. No concrete current merge risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3652b

The change conservatively excludes DMARC when an existing record is found or its lookup fails, and protects the selection when request signing is enabled. No introduced security regression was established. Actual provider enforcement and behavior when DNS changes between URL generation and application remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The supported affected asset is the configured sender domain’s DNS and effective email authentication policy. Integration selection remains team-scoped; the reviewed change does not grant additional signing authority or establish broader tenant or service exposure.

Security Findings and Attack Paths

• inferred — DNS answers influence group selection, so stale or inaccurate absence results can still permit DMARC application. Provider disregard of groupId can also defeat preservation. Neither is established as newly introduced or worsened: base requested all groups regardless of DNS state. No verified attack path introduced by this PR was established.

Trust Boundaries and Controls

• observed — The caller selects an integration and optional provider, but the server obtains the sender domain and group IDs from resolved context rather than accepting group IDs from the request. Provider allowlisting and rejection of missing signing keys for providers requiring signing remain in place. The returned URL is intended for user approval at the DNS provider.

Resilience and Maintainability Implications

• inferred — Lookup uncertainty fails conservatively before URL generation, but the selected groups are only an in-memory snapshot. DNS can change before external application, and an already returned URL retains its earlier selection. Provider-side replay protection, application atomicity, partial-failure recovery, and inherited DMARC policy behavior remain unverified; these limitations must not be treated as a complete preservation guarantee.

Hardening Proposals

• proposed — Validate group filtering and repeated application with each supported provider before presenting this mechanism as guaranteed DMARC preservation. If a provider ignores filtering, use a conservative alternative that avoids offering an apply operation known to duplicate an existing policy.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is complete and stand-alone. It covers the problem, user-visible changes, fallback behavior, testing rationale and limitations, release status, documentation, and agent context. It doe…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Handle non-UTF-8 TXT values conservatively. · domain_connect.py:384

posthog/domain_connect.py:384
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle non-UTF-8 TXT values conservatively.

When _txt_value evaluates a non-UTF-8 TXT segment, it raises UnicodeDecodeError outside _dmarc_absence_confirmed's error handling. The endpoint converts this to a validation error, so the process does not crash, but email apply URL generation stops instead of using the conservative non-DMARC fallback.

Move the normalization into the try block so decoding failures return False, and add a regression test with a non-UTF-8 TXT value.

Suggested fix

     try:
         answers = dns.resolver.resolve(f"_dmarc.{domain}", "TXT", lifetime=5)
+        return not any(_txt_value(rdata).lower().startswith("v=dmarc1") for rdata in answers)
     except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer):
         return True
     except Exception:
         logger.warning("DMARC lookup failed for %s", domain, exc_info=True)
         return False
-    return not any(_txt_value(rdata).lower().startswith("v=dmarc1") for rdata in answers)

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 51683b47-a7f2-4b9c-8ff5-76779f4ed518

📥 Commits

Reviewing files that changed from the base of the PR and between 135e64a and d161996.

📒 Files selected for processing (1)

• posthog/domain_connect.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[Silthus]

@coderabbitai Fixed in 3652b28: TXT parsing now runs inside the try, so a non-UTF-8 _dmarc value counts as a present record and the dmarc group is left out. Added a non-UTF-8 case to the parameterized resolver test.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
ast-grep outline posthog/domain_connect.py --match 'dmarc|txt|resolve' --view expanded
ast-grep outline posthog/test/test_domain_connect.py --match 'test_email_resolver_variables_match_template' --view expanded
rg -n -C 20 'def _.*dmarc|def .*txt|_dmarc|UnicodeDecodeError' posthog/domain_connect.py
rg -n -C 35 'non.utf|\\xff|test_email_resolver_variables_match_template' posthog/test/test_domain_connect.py

Length of output: 7733

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '434,470p' posthog/test/test_domain_connect.py

Length of output: 1578

---

@Silthus No further change is needed for this finding. Source inspection confirms the fix and the regression assertion. Tests were not run.

You are interacting with an AI system.

[mayteio]

Nice! LGTM man, thanks 😄

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-10-01 18:16 UTC	Run
prod-us	✅ Deployed	2026-10-01 18:28 UTC	Run
prod-eu	✅ Deployed	2026-10-01 18:29 UTC	Run