PR #2849 CodeRabbit Follow-up: Trusted-Sender Authentication Gap (CWE-290)
Finding: CodeRabbit review on PR #2849, flagged as genuine pre-existing code, correctly out of that PR’s diff.
Confirmed 2026-08-04. _trusted_sender_rule (apps/temporal-worker/activities/gemini.py) matches the raw From header’s domain/pattern against erp_connections.email_config.trusted_sender_domains / trusted_sender_patterns. A match short-circuits straight to decision="process", confidence=0.99, skipping the LLM classifier — but the From header is written by the sender and was never checked against anything the receiving mail system verified. A spoofed From matching a trusted domain bypassed triage entirely. The webapp’s isTrustedSender (apps/webapp/src/pages/api/email/triage.ts) has the identical shape.
Fixed in this pass
Section titled “Fixed in this pass”| Path | Action |
|---|---|
Gmail pull path (activities/gmail.py) |
Fixed. _fetch_message_content already fetched every header via format=full and just discarded Authentication-Results — no new scope. Now surfaces it as email["auth_results"], using the existing _header_value (first-occurrence, not _parse_headers’ last-wins dict — see “Why first, not last” below). |
Microsoft Graph pull path (activities/microsoft_graph.py) |
Fixed. Added internetMessageHeaders to the existing $select on fetch_microsoft_message — a plain property of the message resource, no new Graph permission beyond the already-granted Mail.Read[.Shared]. New _graph_header_value mirrors gmail.py’s first-occurrence lookup; surfaced as email["auth_results"]. |
Trusted-sender gate (activities/gemini.py) |
Fixed, shadow-mode. New _sender_auth_aligned(email) requires dmarc=pass plus a provider-specific marker in that header before a From-header match is allowed to skip the classifier (see “Why a provider marker, not just dmarc=pass” below). Gated by new flag email_triage_trusted_sender_auth_required_v1 (default off, per AGENTS.md Rule #4 — see “Rollout” below). |
| Unit tests | Added. test_triage_email.py (gate on/off/aligned, the provider-marker requirement, the domain-vs-pattern distinction, the fail-closed-on-exception property), test_gmail_attachments.py / test_microsoft_graph.py (_header_value/_graph_header_value first-occurrence semantics — the actual security property). 895 passed, 1 skipped; ruff check + ruff format clean. |
Deferred — webapp / Cloudflare Email Routing path
Section titled “Deferred — webapp / Cloudflare Email Routing path”Not fixed. Needs a human decision before any change ships here.
apps/webapp/src/pages/api/email/triage.ts’s isTrustedSender has the same gap, called from apps/email-worker (Cloudflare Email Worker) via webapp-client.ts::callTriage. Unlike Gmail/Graph, it’s unclear whether trustworthy authentication data is even available on this path, and the two things I could check from the repo disagree:
apps/email-worker/src/worker.ts’s own header comment says it “Receives raw MIME email for*@ordermatic.covia Cloudflare Email Routing.”- A live MX lookup (2026-08-04) shows
ordermatic.co’s MX is Google Workspace (aspmx.l.google.comand alternates) — not Cloudflare.erpunlocked.com’s MX is Cloudflare (*.route*.mx.cloudflare.net), perdocs/security/dns-hardening-runbook.md’s 2026-06-25 probe, which this lookup reconfirms.
That discrepancy matters because it changes which fix is even possible:
- If Google Workspace is genuinely the first hop (receives the SMTP session from the internet, then a Workspace routing rule relays a copy to Cloudflare Email Routing / the Worker) — Google’s own MTA stamps
Authentication-Resultson original receipt, before any relay. In that casemessage.rawas the Worker sees it already carries Google’s verdict, and the exact same “first occurrence, trust the receiving-mailbox’s own header” pattern used for the Gmail/Graph fix above applies directly — no new scope, small diff (thread it throughemail-parser.ts→webapp-client.ts→triage.ts, gateisTrustedSenderthe same way). - If Cloudflare Email Routing is genuinely the first hop for whatever address pattern this actually runs on — it is not confirmed (and I could not confirm from Cloudflare’s public docs at my knowledge cutoff) that Cloudflare Email Routing stamps its own
Authentication-Resultsbefore invoking a Worker. If it doesn’t, anyAuthentication-Resultsheader found inmessage.rawin that case was written by the sender’s own outbound infrastructure — the sender controls the entire raw byte stream Cloudflare hands to the Worker, so trusting it would be no safer than trustingFrom(worse: it would look fixed while still being spoofable). A real fix would then need the Worker to independently run SPF + DKIM + DMARC verification itself (e.g. themailauthnpm package, operating on the raw MIME and the connecting IPmessage.raw/envelope data Cloudflare provides) — a genuine new dependency and a bigger design/review lift, not a small diff.
Recommendation: resolve which of the two is actually true (dig further into the Cloudflare Email Routing dashboard rules for both zones, and/or confirm with Google Workspace admin routing config whether *@ordermatic.co or an alias of it relays anywhere) before writing any code here. I did not guess and did not change triage.ts / email-worker in this pass — no scope creep beyond gemini.py’s own gap, and no false sense of security shipped on a path I can’t verify.
Why “first occurrence”, not last
Section titled “Why “first occurrence”, not last”Authentication-Results can legitimately appear once per relay hop. The receiving mailbox’s own MTA (Gmail: mx.google.com; Microsoft 365: EOP) prepends its verdict at the top of the header block on final delivery — physically ahead of any copy of that header name the original sender could have forged into their own message before it was ever relayed. Gmail’s existing _parse_headers collapses repeats into a dict where the last value wins, which would have picked the attacker-controlled copy over Gmail’s own. Both new helpers (gmail.py’s existing _header_value, reused as-is; microsoft_graph.py’s new _graph_header_value) scan the raw header list and keep the first match instead. Pinned by test_prefers_first_occurrence_over_last in both test files.
Why a provider marker, not just dmarc=pass
Section titled “Why a provider marker, not just dmarc=pass”The ship review’s security specialist caught a real gap in the first version of this fix: “first occurrence wins” is a positional trust anchor, and it only holds if the provider preserves raw-MIME header order. That’s well-documented Gmail behavior — Google always names itself mx.google.com as the authserv-id on delivery, which is itself now a second, independent signal, not just a header-order assumption. Microsoft Graph’s internetMessageHeaders docs make no such ordering promise. My original code comments stated the “receiving MTA stamps the first one” guarantee with equal confidence for both providers, which overstated what’s actually verified for Graph.
Fix: _sender_auth_aligned now requires dmarc=pass and a provider-specific marker that a forged look-alike header would be unlikely to include, keyed on email["provider"]:
gmail→ the authserv-id itself must containmx.google.com.microsoft→ the header value must containcompauth=pass— not just the tag’s presence. Microsoft’s own proprietary composite-authentication verdict, stamped on effectively every inbound M365 message (Microsoft Learn: message headers in EOP/MDO);compauthis EOP’s own anti-spoofing heuristic and can readfail/none/softpasseven when SPF/DKIM/DMARC formally pass, so a message Microsoft’s own intelligence flagged must not be treated as aligned just because the tag exists. (Ship review caught this exact bug: an earlier version matched barecompauth=presence, which a genuinecompauth=failverdict would have satisfied — fixed before merge, with a regression test.)- No/unknown
provider→ never aligned ondmarc=passalone; there’s no marker to check.
This is defense-in-depth, not proof — a sufficiently format-aware attacker who knows to fake compauth=pass too could still get through on the Graph path if header order really is attacker-controllable there. What it defeats is the realistic case: a forged header shaped like every public reference, missing the undocumented field a real EOP stamp always carries. I considered instead validating an authserv-id/hostname for Microsoft the same way as Gmail, but real EOP-stamped headers often skip the RFC 8601 authserv-id token entirely (many start directly with spf=..., no hostname prefix) — hostname-matching there risks marking all genuine M365 mail as unaligned, i.e. a guaranteed regression once enforcement is enabled, in exchange for closing an unconfirmed gap. compauth=pass is the safer bet: low risk of false-negative on real traffic, grounded in documented Microsoft behavior rather than a guess about their exact format.
Adversarial review pass: two more real gaps closed
Section titled “Adversarial review pass: two more real gaps closed”Ran a Claude adversarial subagent and a Codex structured review before merge. Both found real issues in what looked like a stable diff after four specialist rounds:
1. Flagsmith-unreachable fallback silently reopened the exact bypass this gate exists to close (Claude adversarial). get_feature_flag’s default param fires both when the flag hasn’t been created yet in Flagsmith AND when Flagsmith is transiently unreachable (self-hosted instance down, network blip) — those two cases are indistinguishable to the caller. _trusted_sender_auth_required_enabled used default=False like every sibling flag getter, which is correct for “not yet decided” but wrong for “can’t tell right now”: an operator who already enabled enforcement for a connection would have that decision silently undone for the duration of any Flagsmith hiccup, with the warning log actively misreporting it as normal shadow-mode operation. Fixed: this one flag’s fallback now defaults to True. The two failure directions aren’t symmetric — a stray True before anyone opts in just costs a few extra classifier calls (already this file’s documented safe fallback for every unaligned/unverifiable case); a stray False after someone opts in reinstates confidence=0.99 on an unauthenticated sender.
2. Pattern-based trust can’t be tied to a verified domain at all (Codex structured review, [P2]). _sender_auth_aligned’s dmarc=pass check provably ties back to the same visible From: domain a trusted_sender_domains match already compared — but trusted_sender_patterns matches arbitrary text (e.g. a display name like ^Hyster-Yale PO <), with no domain to tie back to at all. Before this fix, "Hyster-Yale PO <po@totally-unrelated-domain.com>" would satisfy the pattern AND come back dmarc=pass + a real provider marker — the sender is genuinely authenticated, just not the one the pattern was written to represent — defeating the entire auth gate for any connection using patterns instead of domains. Fixed: alignment can now only save a trusted_domain:-prefixed match; a trusted_pattern: match always takes the unaligned path (shadow-mode logs a warning; enforcement falls through to the classifier). This also sharpens why the codebase already recommends domains over patterns for portability reasons — patterns are now explicitly documented as un-hardenable by this gate.
3. Open question, not fixed (Claude adversarial, INVESTIGATE). Gmail’s from_raw — the value both the domain/pattern match AND (transitively) the auth check’s target sender ride on — is built via _parse_headers’ last-wins dict (gmail.py), not the first-occurrence lookup this fix deliberately uses for Authentication-Results. If Gmail ever accepts and delivers a message with two From: headers (RFC 5322 permits exactly one, but real-world MTA leniency on ingest varies, and this needs a live test I can’t run from static review — send a crafted dual-From message to a connected Gmail account and inspect the delivered Authentication-Results order versus which From: value Gmail’s own DMARC engine aligned against), an attacker could potentially pass DMARC on one From: value while the last-wins extraction picks a second, unauthenticated one inserted to match trusted_sender_domains — reproducing this PR’s own threat model through a header-duplication side door. Does not apply to Microsoft Graph (from there is Graph’s own structured, pre-normalized object, not a raw duplicable header list). Flagging for human follow-up rather than guessing at a fix — changing from_raw’s extraction touches other callers and needs a live Gmail test to confirm the premise before touching it.
Rollout — email_triage_trusted_sender_auth_required_v1
Section titled “Rollout — email_triage_trusted_sender_auth_required_v1”Registered in infrastructure/flagsmith/configs/flags.json, default off, no segment override added — this is an operational decision, not a code one.
While off (today’s deploy state): behavior is byte-for-byte unchanged, but every trusted-sender match that lacks an aligned Authentication-Results (dmarc=pass + provider marker) now logs a warning (activity.logger.warning in triage_email). Before enabling this for any connection, sample that log (or email_events.provider_metadata/traces once auth_results starts flowing) for the connection’s actual trusted_sender_domains traffic and confirm they show a clean pass in practice — for an M365-connected customer specifically, confirm real sampled traffic actually carries compauth=pass (not just dmarc=pass); if it doesn’t, the microsoft branch needs its own follow-up before that connection can safely enable enforcement. email_triage_trusted_senders_v1 is already live for conn_vms_e4 (Hyster-Yale, a Gmail-side connection) per infrastructure/flagsmith/configs/segment-overrides.json — that’s the first connection to check once this deploys, since it’s the one currently exposed to the original gap in production.
While on for a connection: an unaligned or unverifiable match falls through to the normal LLM classifier instead of a hard skip. It never drops the email — only routes it through the slower, content-based path, exactly like every other classifier-bypass fallback in this file.