From 162e420a1f63cd8ecce437f49ada02c37fe9debb Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 21 Jul 2026 23:03:34 +0200 Subject: [PATCH] fix: stop HELO spf=none from downgrading a MAIL FROM spf=pass #650 --- lib/__tests__/email-headers.test.ts | 29 +++++++++++++++++++++++++++++ lib/email-headers.ts | 27 ++++++++++++++------------- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/lib/__tests__/email-headers.test.ts b/lib/__tests__/email-headers.test.ts index 34625699..874f5e6b 100644 --- a/lib/__tests__/email-headers.test.ts +++ b/lib/__tests__/email-headers.test.ts @@ -83,6 +83,35 @@ describe('parseAuthenticationResults', () => { const result = parseAuthenticationResults(header); expect(result.spf?.domain).toBe('example.com'); }); + + it('does not let a HELO `none` downgrade a MAIL FROM `pass` (#650)', () => { + const header = + 'mail.haxalot.com; spf=none (mail.haxalot.com: no SPF records found for postmaster@out-23.smtp.github.com) smtp.helo=out-23.smtp.github.com; spf=pass (mail.haxalot.com: domain of noreply@github.com designates 192.30.252.206 as permitted sender) smtp.mailfrom=noreply@github.com'; + const result = parseAuthenticationResults(header); + expect(result.spf?.result).toBe('pass'); + expect(result.spf?.domain).toBe('noreply@github.com'); + expect(result.spf?.all).toHaveLength(2); + }); + + it('does not let a HELO `neutral` downgrade a MAIL FROM `pass`', () => { + const header = 'spf=neutral smtp.helo=mail.example.com; spf=pass smtp.mailfrom=example.com'; + const result = parseAuthenticationResults(header); + expect(result.spf?.result).toBe('pass'); + }); + + it('still escalates a HELO hard fail over a MAIL FROM pass', () => { + const header = 'spf=fail smtp.helo=mail.spoof.com; spf=pass smtp.mailfrom=example.com'; + const result = parseAuthenticationResults(header); + expect(result.spf?.result).toBe('fail'); + expect(result.spf?.domain).toBe('mail.spoof.com'); + }); + + it('keeps MAIL FROM `none` as the headline even when HELO passes', () => { + const header = 'spf=pass smtp.helo=mail.example.com; spf=none smtp.mailfrom=example.com'; + const result = parseAuthenticationResults(header); + expect(result.spf?.result).toBe('none'); + expect(result.spf?.domain).toBe('example.com'); + }); }); describe('isAuthenticationSpoofed', () => { diff --git a/lib/email-headers.ts b/lib/email-headers.ts index 53a79345..505046ff 100644 --- a/lib/email-headers.ts +++ b/lib/email-headers.ts @@ -49,8 +49,8 @@ export function parseAuthenticationResults(header: string): AuthenticationResult // Parse SPF. A single Authentication-Results header can carry more than one // SPF result when the server evaluates multiple identities (HELO and MAIL - // FROM). Collect them all and surface the most severe as the headline so a - // hard MAIL FROM `fail` isn't softened to a HELO `temperror`. + // FROM). Collect them all so a hard fail on any identity isn't softened to + // an ambiguous state recorded for another one. const spfRegex = /spf=(\w+)(?:\s+\([^)]*\))?(?:\s+smtp\.(mailfrom|helo)=([^\s;]+))?/g; const spfResults: SpfEntry[] = []; let spfM: RegExpExecArray | null; @@ -63,17 +63,18 @@ export function parseAuthenticationResults(header: string): AuthenticationResult } if (spfResults.length > 0) { const severity = (r: string) => SPF_SEVERITY[r as SpfResult] ?? -1; - // Most severe wins; on a tie prefer the MAIL FROM identity (more meaningful - // than HELO) and otherwise keep the first occurrence. - const primary = spfResults.reduce((best, cur) => { - if (severity(cur.result) > severity(best.result)) return cur; - if ( - severity(cur.result) === severity(best.result) && - best.identity !== 'mailfrom' && - cur.identity === 'mailfrom' - ) return cur; - return best; - }); + // MAIL FROM is the primary SPF identity. Another identity (HELO) may only + // escalate the headline to a genuine failure state — a HELO `none` or + // `neutral` must not downgrade a MAIL FROM `pass`, since most senders + // publish no SPF record for their EHLO hostname. + const isFailure = (r: string) => severity(r) >= SPF_SEVERITY.temperror; + let primary = + spfResults.find((e) => e.identity === 'mailfrom') ?? spfResults[0]; + for (const cur of spfResults) { + if (isFailure(cur.result) && severity(cur.result) > severity(primary.result)) { + primary = cur; + } + } results.spf = { result: primary.result, domain: primary.domain,