Harden AR parsing and alarm on sender-auth rejects

The SES-stamped Authentication-Results value echoes attacker-controlled
SMTP-session tokens (envelope-from, helo, header.from) as their own
semicolon-delimited property clauses. A naive split(";") tore an RFC 5321
quoted-local-part MAIL FROM apart and manufactured a forged dkim=pass
clause, so a fully spoofed email was accepted on the genuinely
SES-stamped topmost header. Tokenise comment- and quoted-string-aware
(RFC 8601 / RFC 5322): strip CFWS comments, split clauses only on
semicolons outside a quoted-string, and fail closed on unbalanced
quotes/comments so a ';' inside a quoted pvalue can never start a clause.

Rejected mail returns normally (no error, no retry, no DLQ message), so a
signing-domain drift or a wrong allowlist would silently discard 100% of
legitimate mail while every alarm stayed green. Add a CloudWatch Logs
metric filter + alarm on the sender_auth_rejected warning to both stacks
so a false-reject storm pages instead of vanishing. This is also the
safety net for the WO seahaven.com allowlist assumption, which must be
validated against a live SES-stamped header (a plain Gmail auto-forward
re-signs under the sending Workspace domain, not seahaven.com).

Refs: INFRA-107

README.md

@@ -47,7 +47,7 @@ Amazon APM work order emails (from Hexagon EAM / HxGN SmartCloud) are received a
 2. Gmail filter forwards APM emails to `apm@int.seahaven.com` (SES).
 3. SES drops the raw MIME into `s3://workorder-ingest-emails-{AccountId}/inbound/`.
 4. S3 triggers the `workorder-email-processor` Lambda.
-5. Fail-closed sender authentication (INFRA-107): the SES-stamped `Authentication-Results` header must show `dkim=pass` for `seahaven.com` (the Google Groups forward re-signs the mail; see [Sender authentication](#sender-authentication-infra-107)); otherwise the email is logged and dropped.
+5. Fail-closed sender authentication (INFRA-107): the SES-stamped `Authentication-Results` header must show `dkim=pass` for the domain that re-signs the forward (currently allowlisted as `seahaven.com` — see the validation caveat under [Sender authentication](#sender-authentication-infra-107)); otherwise the email is logged and dropped.
 6. Claude extracts structured JSON (work order ID, site code, severity, priority, dates, assigned technician).
 7. Work order upserted to `WorkOrders`, event/comment appended to `WorkOrderComments`.
 
@@ -85,10 +85,12 @@ The allowlist is the `ALLOWED_DKIM_DOMAINS` Lambda environment variable (comma-s
 
 | Pipeline | `ALLOWED_DKIM_DOMAINS` | Why |
 |---|---|---|
-| Work orders | `seahaven.com` | APM mail arrives via the `apm@` Google Groups forward, which re-signs as `seahaven.com` (the original `hxgnsmartcloud.com` signature does not survive the forward) |
+| Work orders | `seahaven.com` | APM mail reaches `apm@int.seahaven.com` via a forward off `amazon@seahavenind.com`; the allowlist trusts the domain that **re-signs** DKIM on that forward (the original `hxgnsmartcloud.com` signature does not survive it). **Must be validated against a live SES-stamped header** — a plain Gmail auto-forward re-signs under the *sending Workspace* domain (`seahavenind.com` / a `*.gappssmtp.com` key), **not** `seahaven.com`; only a Google Group (or Workspace routing) with "sign as `seahaven.com`" produces `dkim=pass header.i=@seahaven.com`. If the observed re-signing domain differs, update this value (do **not** widen it to a shared key like `*.gappssmtp.com`, which any Google customer's mail would pass). The `workorder-email-processor-sender-auth-rejected` alarm pages if this assumption is wrong instead of silently dropping every work order. |
 | Purchase orders | `amazon.coupahost.com` | Coupa signs as `amazon.coupahost.com`. `amazonses.com` also passes but is deliberately not allowlisted — every SES customer's mail passes for it |
 
-On any failure (env var unset, header missing/unparseable, verdict fail, unaligned domain) the processor logs a structured `sender_auth_rejected` warning with the reason and S3 key, skips the email, and returns normally — rejected mail never triggers Lambda retries or DLQ messages. Unit tests live in `tests/test_ses_auth.py`.
+> **Clause-injection hardening:** SES echoes attacker-controlled SMTP-session tokens (`envelope-from`, `helo`, `header.from`) into its own `Authentication-Results` value, and an RFC 5321 quoted-local-part MAIL FROM may legally contain `;` and spaces. The parser therefore tokenises comment- and quoted-string-aware (RFC 8601 / RFC 5322): CFWS comments `(...)` are stripped and clauses are split only on semicolons **outside** a quoted string, so a `;` inside a quoted `envelope-from=` value can never be torn into a forged `dkim=pass` clause.
+
+On any failure (env var unset, header missing/unparseable, verdict fail, unaligned domain) the processor logs a structured `sender_auth_rejected` warning with the reason and S3 key, skips the email, and returns normally — rejected mail never triggers Lambda retries or DLQ messages, but the `<fn>-sender-auth-rejected` CloudWatch alarm (see [CloudWatch alarms](#cloudwatch-alarms)) pages on a rejection spike so a drift-induced outage is not silent. Unit tests live in `tests/test_ses_auth.py`.
 
 **Failure handling (INFRA-41):** Each email-processor is async-invoked (S3 → Lambda). Both have a CDK-managed SQS dead-letter queue (`dead_letter_queue=`, 14-day retention, SSL-enforced) so a failed parse is captured rather than silently dropped after Lambda's retries.
 
@@ -103,6 +105,9 @@ Every alarm is **ALARM-only** (no OK action), sends to the shared `site-alerts`
 | `<fn>-errors` | `po-email-processor`, `po-ingest-site-extractor`, `workorder-email-processor` | `Errors` Sum, 5 min, `> 0`, eval 1 |
 | `<fn>-throttles` | `po-email-processor`, `po-ingest-site-extractor`, `po-web-ui`, `workorder-email-processor` | `Throttles` Sum, 5 min, `> 0`, eval 1 |
 | `<fn>-duration` | `po-email-processor`, `po-ingest-site-extractor`, `po-web-ui` (p99); `workorder-email-processor` (p95) | `Duration` percentile, 5 min, `>= 45000` ms (75% of the 60s timeout), eval 3 / datapoints 2 |
+| `<fn>-sender-auth-rejected` | `po-email-processor`, `workorder-email-processor` | Log-metric-filter count (namespace `Seahaven/ProcurementIngest`, `default_value=0`) on `sender_auth_rejected` warnings, `Sum` 15 min, `>= 3`, eval 1 |
+
+The `<fn>-sender-auth-rejected` alarm closes the silent-drop gap in INFRA-107: a rejected email returns normally (no error, no retry, no DLQ message), so without a log-metric filter a signing-domain drift or a wrong allowlist would discard 100% of legitimate mail while every other alarm stayed green. `>= 3` rejections in 15 minutes tolerates the occasional spoofed probe to the internal ingest address but pages quickly on a genuine false-reject storm; the threshold is easy to tune in the CDK helper.
 
 The `<fn>-duration` and `<fn>-throttles` alarms for `po-email-processor` and `workorder-email-processor` supersede the orphaned, CLI-created `Lambda-Duration-*` / `Lambda-Throttles-*` alarms (deleted post-deploy).
 

cdk/po_stack.py

@@ -77,6 +77,67 @@ def _add_ddb_alarms(scope, id_prefix, table, alarm_name_prefix, alarm_topic):
     ).add_alarm_action(cw_actions.SnsAction(alarm_topic))
 
 
+# CloudWatch namespace for the log-derived sender-authentication metrics.
+_SENDER_AUTH_METRIC_NAMESPACE = "Seahaven/ProcurementIngest"
+
+
+def _add_sender_auth_rejected_alarm(scope, id_prefix, function_name, alarm_topic):
+    """Metric-filter + alarm on ``sender_auth_rejected`` warnings (INFRA-107).
+
+    A rejected inbound email is skipped without erroring the invocation, so it
+    is invisible to the Errors/Throttles/DLQ alarms. This turns the structured
+    warning log into a CloudWatch metric and pages when rejections spike --
+    catching a silent false-reject storm (allowlist wrong, signing-domain
+    drift, SES header-format change) that would otherwise discard legitimate
+    mail while the pipeline reports healthy.
+
+    ALARM-only SnsAction to site-alerts; no OK action. The metric filter reads
+    the function's own log group (imported by the deterministic
+    ``/aws/lambda/<fn>`` name, created by the function's log_retention). A plain
+    substring pattern is used because Lambda prefixes each line with its own
+    level/timestamp/request-id, so the JSON payload is not a standalone JSON
+    log event a `{$.event=...}` pattern could match.
+    """
+    metric_name = f"{function_name}-sender-auth-rejected"
+    logs.MetricFilter(
+        scope,
+        f"{id_prefix}SenderAuthRejectedFilter",
+        log_group=logs.LogGroup.from_log_group_name(
+            scope,
+            f"{id_prefix}LogGroup",
+            f"/aws/lambda/{function_name}",
+        ),
+        filter_pattern=logs.FilterPattern.literal('"sender_auth_rejected"'),
+        metric_namespace=_SENDER_AUTH_METRIC_NAMESPACE,
+        metric_name=metric_name,
+        metric_value="1",
+        default_value=0,
+    )
+
+    # >=3 rejections in a 15-min window pages: a lone spoofed probe to the
+    # (obscure, internal) ingest address is tolerated, but a genuine
+    # false-reject storm -- where legitimate senders are being dropped -- trips
+    # quickly. Threshold is intentionally conservative and easy to tune.
+    cloudwatch.Metric(
+        namespace=_SENDER_AUTH_METRIC_NAMESPACE,
+        metric_name=metric_name,
+        period=Duration.minutes(15),
+        statistic="Sum",
+    ).create_alarm(
+        scope,
+        f"{id_prefix}SenderAuthRejectedAlarm",
+        alarm_name=f"{function_name}-sender-auth-rejected",
+        alarm_description=(
+            f"{function_name} rejected inbound mail on sender authentication "
+            "(possible allowlist/DKIM-domain drift silently dropping real mail)"
+        ),
+        threshold=3,
+        evaluation_periods=1,
+        comparison_operator=cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
+        treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
+    ).add_alarm_action(cw_actions.SnsAction(alarm_topic))
+
+
 class PoIngestStack(Stack):
     def __init__(self, scope: Construct, construct_id: str, **kwargs):
         super().__init__(scope, construct_id, **kwargs)
@@ -218,6 +279,20 @@ class PoIngestStack(Stack):
             treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
         ).add_alarm_action(cw_actions.SnsAction(alarm_topic))
 
+        # --- Sender-auth rejection alarm (INFRA-107) ---
+        # A rejected email (bad/unaligned DKIM verdict) returns normally, so it
+        # produces NO Lambda error, NO DLQ message and NO retry -- only a
+        # `sender_auth_rejected` warning log. Without this metric filter + alarm a
+        # domain drift (Coupa rotates its signing subdomain, SES changes its
+        # Authentication-Results format, the allowlist is wrong) would silently
+        # discard 100% of legitimate PO mail while every other alarm stays green.
+        # A CloudWatch Logs metric filter turns those warnings into a metric so a
+        # false-reject storm pages instead of vanishing. default_value=0 keeps the
+        # series populated (alarm stays OK, never INSUFFICIENT_DATA) between events.
+        _add_sender_auth_rejected_alarm(
+            self, "EmailProcessor", "po-email-processor", alarm_topic
+        )
+
         # --- Throttles alarm: po-email-processor ---
         # Any throttled invocation (concurrency cap hit) in a 5-min window pages.
         # ALARM-only to site-alerts; no OK action; NOT_BREACHING when no data.

cdk/wo_stack.py

@@ -76,6 +76,70 @@ def _add_ddb_alarms(scope, id_prefix, table, alarm_name_prefix, alarm_topic):
     ).add_alarm_action(cw_actions.SnsAction(alarm_topic))
 
 
+# CloudWatch namespace for the log-derived sender-authentication metrics.
+_SENDER_AUTH_METRIC_NAMESPACE = "Seahaven/ProcurementIngest"
+
+
+def _add_sender_auth_rejected_alarm(scope, id_prefix, function_name, alarm_topic):
+    """Metric-filter + alarm on ``sender_auth_rejected`` warnings (INFRA-107).
+
+    A rejected inbound email is skipped without erroring the invocation, so it
+    is invisible to the Errors/Throttles/DLQ alarms. This turns the structured
+    warning log into a CloudWatch metric and pages when rejections spike --
+    catching a silent false-reject storm (allowlist wrong, signing-domain
+    drift, SES header-format change) that would otherwise discard legitimate
+    mail while the pipeline reports healthy. This is the safety net for the WO
+    allowlist domain assumption (seahaven.com) -- if the real Gmail-forward
+    re-signing domain differs, this alarm surfaces it instead of a silent
+    work-order outage.
+
+    ALARM-only SnsAction to site-alerts; no OK action. The metric filter reads
+    the function's own log group (imported by the deterministic
+    ``/aws/lambda/<fn>`` name, created by the function's log_retention). A plain
+    substring pattern is used because Lambda prefixes each line with its own
+    level/timestamp/request-id, so the JSON payload is not a standalone JSON
+    log event a `{$.event=...}` pattern could match.
+    """
+    metric_name = f"{function_name}-sender-auth-rejected"
+    logs.MetricFilter(
+        scope,
+        f"{id_prefix}SenderAuthRejectedFilter",
+        log_group=logs.LogGroup.from_log_group_name(
+            scope,
+            f"{id_prefix}LogGroup",
+            f"/aws/lambda/{function_name}",
+        ),
+        filter_pattern=logs.FilterPattern.literal('"sender_auth_rejected"'),
+        metric_namespace=_SENDER_AUTH_METRIC_NAMESPACE,
+        metric_name=metric_name,
+        metric_value="1",
+        default_value=0,
+    )
+
+    # >=3 rejections in a 15-min window pages: a lone spoofed probe to the
+    # (obscure, internal) ingest address is tolerated, but a genuine
+    # false-reject storm -- where legitimate senders are being dropped -- trips
+    # quickly. Threshold is intentionally conservative and easy to tune.
+    cloudwatch.Metric(
+        namespace=_SENDER_AUTH_METRIC_NAMESPACE,
+        metric_name=metric_name,
+        period=Duration.minutes(15),
+        statistic="Sum",
+    ).create_alarm(
+        scope,
+        f"{id_prefix}SenderAuthRejectedAlarm",
+        alarm_name=f"{function_name}-sender-auth-rejected",
+        alarm_description=(
+            f"{function_name} rejected inbound mail on sender authentication "
+            "(possible allowlist/DKIM-domain drift silently dropping real mail)"
+        ),
+        threshold=3,
+        evaluation_periods=1,
+        comparison_operator=cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
+        treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
+    ).add_alarm_action(cw_actions.SnsAction(alarm_topic))
+
+
 class WorkorderIngestStack(Stack):
     def __init__(self, scope: Construct, construct_id: str, **kwargs):
         super().__init__(scope, construct_id, **kwargs)
@@ -236,6 +300,19 @@ class WorkorderIngestStack(Stack):
             treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
         ).add_alarm_action(cw_actions.SnsAction(alarm_topic))
 
+        # --- Sender-auth rejection alarm (INFRA-107) ---
+        # A rejected email (bad/unaligned DKIM verdict) returns normally, so it
+        # produces NO Lambda error, NO DLQ message and NO retry -- only a
+        # `sender_auth_rejected` warning log. The WO allowlist trusts dkim=pass
+        # for seahaven.com on the assumption the apm@ forward re-signs there; if
+        # that assumption is wrong (e.g. a Gmail auto-forward re-signs under a
+        # different domain), 100% of legitimate work-order mail is silently
+        # dropped. This metric filter + alarm turns those warnings into a paging
+        # signal so a false-reject storm surfaces instead of a silent outage.
+        _add_sender_auth_rejected_alarm(
+            self, "EmailProcessor", "workorder-email-processor", alarm_topic
+        )
+
         # --- Throttles alarm: workorder-email-processor ---
         # Any throttled invocation (concurrency cap hit) in a 5-min window pages.
         # ALARM-only to site-alerts; no OK action; NOT_BREACHING when no data.

lambdas/po/email_processor/ses_auth.py

@@ -27,6 +27,25 @@ Observed SES format (2026-07-15, both ingest buckets)::
 
 Note SES reports the passing DKIM identity as ``header.i=@<domain>``
 (RFC 6376 AUID), not ``header.d=``; the parser accepts both.
+
+**Clause injection defence (INFRA-107 hardening).** SES echoes several
+attacker-controlled SMTP-session tokens into its own Authentication-Results
+value as their own semicolon-delimited property clauses -- notably
+``envelope-from=<MAIL FROM>``, ``helo=<HELO>`` and ``header.from=<From
+domain>``. An RFC 5321 quoted-local-part MAIL FROM may legally contain
+spaces and semicolons, e.g.::
+
+    MAIL FROM:<"x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com>
+
+which SES renders verbatim as ``envelope-from="x; dkim=pass
+header.i=@amazon.coupahost.com"@attacker.com;``. A naive ``split(";")``
+would tear the quoted string apart and manufacture a synthetic
+``dkim=pass header.i=@amazon.coupahost.com`` clause out of attacker input.
+The parser therefore tokenises per RFC 8601 / RFC 5322 structure: CFWS
+comments ``(...)`` are stripped first, and the value is split into clauses
+only on semicolons that sit *outside* a quoted-string. A ``;`` inside a
+quoted ``pvalue`` stays part of that one property clause and can never be
+read as the start of a ``dkim=`` methodspec.
 """
 
 import email.parser
@@ -44,9 +63,11 @@ SES_AUTHSERV_ID = "amazonses.com"
 # One resinfo clause of an Authentication-Results value, e.g.
 # "dkim=pass header.i=@seahaven.com". The clause must START with the
 # method=result pair; header.d= / header.i= may appear anywhere after it.
-# The result token must be terminated by end-of-clause, whitespace, or a
+# The result token must be terminated by end-of-clause or whitespace so
-# comment so "dkim=pass-anything" can never be read as "pass".
+# "dkim=pass-anything" can never be read as "pass". (Comments are stripped
-_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|[\s(])", re.IGNORECASE)
+# before matching, so the pre-strip "dkim=pass(comment)" form arrives here
+# as "dkim=pass ..." and still terminates on whitespace.)
+_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|\s)", re.IGNORECASE)
 _HEADER_D_RE = re.compile(r"header\.d\s*=\s*\"?([^\s\";]+)", re.IGNORECASE)
 _HEADER_I_RE = re.compile(r"header\.i\s*=\s*\"?([^\s\";]+)", re.IGNORECASE)
 
@@ -64,15 +85,120 @@ def _unfold(value: str) -> str:
     return re.sub(r"[\r\n\t ]+", " ", value).strip()
 
 
+def _strip_comments(value: str) -> tuple:
+    """Remove RFC 5322 CFWS comments ``(...)`` from an unfolded header value.
+
+    Comments may nest and may contain quoted pairs (``\\)``). A ``(`` that
+    appears *inside* a quoted-string is literal text, not a comment start,
+    so quoted-strings (which carry attacker-controlled ``pvalue`` content
+    such as a quoted MAIL FROM local part) are passed through untouched.
+    Dropping comments first means comment-embedded ``header.i=`` / ``;``
+    fakes can never influence clause splitting or domain extraction.
+
+    Returns ``(stripped_text, well_formed)``. ``well_formed`` is False when
+    the value ends inside an unterminated comment or quoted-string, i.e. the
+    parens/quotes are unbalanced. Callers reject on ``not well_formed`` so a
+    malformed header (which could otherwise be mis-tokenised) fails closed
+    rather than being partially parsed.
+    """
+    out = []
+    depth = 0  # comment nesting depth
+    in_quote = False  # inside a quoted-string (only tracked at depth 0)
+    i = 0
+    n = len(value)
+    while i < n:
+        c = value[i]
+        if depth > 0:
+            # Inside a comment: only quoted-pairs and nested parens matter.
+            if c == "\\":
+                i += 2
+                continue
+            if c == "(":
+                depth += 1
+            elif c == ")":
+                depth -= 1
+            i += 1
+            continue
+        if in_quote:
+            out.append(c)
+            if c == "\\" and i + 1 < n:
+                out.append(value[i + 1])
+                i += 2
+                continue
+            if c == '"':
+                in_quote = False
+            i += 1
+            continue
+        # Normal context (outside any comment or quoted-string).
+        if c == "(":
+            depth += 1
+            i += 1
+            continue
+        if c == '"':
+            in_quote = True
+        out.append(c)
+        i += 1
+    well_formed = depth == 0 and not in_quote
+    return "".join(out), well_formed
+
+
+def _split_clauses(value: str) -> list:
+    """Split a comment-free header value into clauses on top-level ``;``.
+
+    A semicolon inside a quoted-string is preserved as part of the clause,
+    so an attacker-controlled quoted ``pvalue`` (e.g. a quoted MAIL FROM
+    echoed into ``envelope-from=``) cannot smuggle in a fake ``dkim=pass``
+    clause. Callers must run :func:`_strip_comments` first.
+    """
+    clauses = []
+    buf = []
+    in_quote = False
+    i = 0
+    n = len(value)
+    while i < n:
+        c = value[i]
+        if in_quote:
+            buf.append(c)
+            if c == "\\" and i + 1 < n:
+                buf.append(value[i + 1])
+                i += 2
+                continue
+            if c == '"':
+                in_quote = False
+            i += 1
+            continue
+        if c == '"':
+            in_quote = True
+            buf.append(c)
+            i += 1
+            continue
+        if c == ";":
+            clauses.append("".join(buf))
+            buf = []
+            i += 1
+            continue
+        buf.append(c)
+        i += 1
+    clauses.append("".join(buf))
+    return clauses
+
+
 def parse_authentication_results(value: str) -> tuple:
     """Parse one Authentication-Results header value.
 
     Returns ``(authserv_id, passing_dkim_domains)`` where the domains are
     the lowercased d=/i= domains of every ``dkim=pass`` clause. Malformed
     input yields ``("", frozenset())``, which callers treat as a rejection.
+
+    Tokenisation is comment- and quoted-string-aware (RFC 8601 / RFC 5322)
+    so attacker-controlled tokens SES echoes into its header (envelope-from,
+    helo, header.from) cannot be split into a forged ``dkim=pass`` clause.
     """
-    text = _unfold(value)
+    text, well_formed = _strip_comments(_unfold(value))
-    clauses = [c.strip() for c in text.split(";")]
+    if not well_formed:
+        # Unbalanced quotes/comments: refuse to guess how to tokenise it.
+        return "", frozenset()
+    clauses = [c.strip() for c in _split_clauses(text)]
     if not clauses or not clauses[0]:
         return "", frozenset()
 

lambdas/wo/email_processor/ses_auth.py

@@ -27,6 +27,25 @@ Observed SES format (2026-07-15, both ingest buckets)::
 
 Note SES reports the passing DKIM identity as ``header.i=@<domain>``
 (RFC 6376 AUID), not ``header.d=``; the parser accepts both.
+
+**Clause injection defence (INFRA-107 hardening).** SES echoes several
+attacker-controlled SMTP-session tokens into its own Authentication-Results
+value as their own semicolon-delimited property clauses -- notably
+``envelope-from=<MAIL FROM>``, ``helo=<HELO>`` and ``header.from=<From
+domain>``. An RFC 5321 quoted-local-part MAIL FROM may legally contain
+spaces and semicolons, e.g.::
+
+    MAIL FROM:<"x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com>
+
+which SES renders verbatim as ``envelope-from="x; dkim=pass
+header.i=@amazon.coupahost.com"@attacker.com;``. A naive ``split(";")``
+would tear the quoted string apart and manufacture a synthetic
+``dkim=pass header.i=@amazon.coupahost.com`` clause out of attacker input.
+The parser therefore tokenises per RFC 8601 / RFC 5322 structure: CFWS
+comments ``(...)`` are stripped first, and the value is split into clauses
+only on semicolons that sit *outside* a quoted-string. A ``;`` inside a
+quoted ``pvalue`` stays part of that one property clause and can never be
+read as the start of a ``dkim=`` methodspec.
 """
 
 import email.parser
@@ -44,9 +63,11 @@ SES_AUTHSERV_ID = "amazonses.com"
 # One resinfo clause of an Authentication-Results value, e.g.
 # "dkim=pass header.i=@seahaven.com". The clause must START with the
 # method=result pair; header.d= / header.i= may appear anywhere after it.
-# The result token must be terminated by end-of-clause, whitespace, or a
+# The result token must be terminated by end-of-clause or whitespace so
-# comment so "dkim=pass-anything" can never be read as "pass".
+# "dkim=pass-anything" can never be read as "pass". (Comments are stripped
-_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|[\s(])", re.IGNORECASE)
+# before matching, so the pre-strip "dkim=pass(comment)" form arrives here
+# as "dkim=pass ..." and still terminates on whitespace.)
+_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|\s)", re.IGNORECASE)
 _HEADER_D_RE = re.compile(r"header\.d\s*=\s*\"?([^\s\";]+)", re.IGNORECASE)
 _HEADER_I_RE = re.compile(r"header\.i\s*=\s*\"?([^\s\";]+)", re.IGNORECASE)
 
@@ -64,15 +85,120 @@ def _unfold(value: str) -> str:
     return re.sub(r"[\r\n\t ]+", " ", value).strip()
 
 
+def _strip_comments(value: str) -> tuple:
+    """Remove RFC 5322 CFWS comments ``(...)`` from an unfolded header value.
+
+    Comments may nest and may contain quoted pairs (``\\)``). A ``(`` that
+    appears *inside* a quoted-string is literal text, not a comment start,
+    so quoted-strings (which carry attacker-controlled ``pvalue`` content
+    such as a quoted MAIL FROM local part) are passed through untouched.
+    Dropping comments first means comment-embedded ``header.i=`` / ``;``
+    fakes can never influence clause splitting or domain extraction.
+
+    Returns ``(stripped_text, well_formed)``. ``well_formed`` is False when
+    the value ends inside an unterminated comment or quoted-string, i.e. the
+    parens/quotes are unbalanced. Callers reject on ``not well_formed`` so a
+    malformed header (which could otherwise be mis-tokenised) fails closed
+    rather than being partially parsed.
+    """
+    out = []
+    depth = 0  # comment nesting depth
+    in_quote = False  # inside a quoted-string (only tracked at depth 0)
+    i = 0
+    n = len(value)
+    while i < n:
+        c = value[i]
+        if depth > 0:
+            # Inside a comment: only quoted-pairs and nested parens matter.
+            if c == "\\":
+                i += 2
+                continue
+            if c == "(":
+                depth += 1
+            elif c == ")":
+                depth -= 1
+            i += 1
+            continue
+        if in_quote:
+            out.append(c)
+            if c == "\\" and i + 1 < n:
+                out.append(value[i + 1])
+                i += 2
+                continue
+            if c == '"':
+                in_quote = False
+            i += 1
+            continue
+        # Normal context (outside any comment or quoted-string).
+        if c == "(":
+            depth += 1
+            i += 1
+            continue
+        if c == '"':
+            in_quote = True
+        out.append(c)
+        i += 1
+    well_formed = depth == 0 and not in_quote
+    return "".join(out), well_formed
+
+
+def _split_clauses(value: str) -> list:
+    """Split a comment-free header value into clauses on top-level ``;``.
+
+    A semicolon inside a quoted-string is preserved as part of the clause,
+    so an attacker-controlled quoted ``pvalue`` (e.g. a quoted MAIL FROM
+    echoed into ``envelope-from=``) cannot smuggle in a fake ``dkim=pass``
+    clause. Callers must run :func:`_strip_comments` first.
+    """
+    clauses = []
+    buf = []
+    in_quote = False
+    i = 0
+    n = len(value)
+    while i < n:
+        c = value[i]
+        if in_quote:
+            buf.append(c)
+            if c == "\\" and i + 1 < n:
+                buf.append(value[i + 1])
+                i += 2
+                continue
+            if c == '"':
+                in_quote = False
+            i += 1
+            continue
+        if c == '"':
+            in_quote = True
+            buf.append(c)
+            i += 1
+            continue
+        if c == ";":
+            clauses.append("".join(buf))
+            buf = []
+            i += 1
+            continue
+        buf.append(c)
+        i += 1
+    clauses.append("".join(buf))
+    return clauses
+
+
 def parse_authentication_results(value: str) -> tuple:
     """Parse one Authentication-Results header value.
 
     Returns ``(authserv_id, passing_dkim_domains)`` where the domains are
     the lowercased d=/i= domains of every ``dkim=pass`` clause. Malformed
     input yields ``("", frozenset())``, which callers treat as a rejection.
+
+    Tokenisation is comment- and quoted-string-aware (RFC 8601 / RFC 5322)
+    so attacker-controlled tokens SES echoes into its header (envelope-from,
+    helo, header.from) cannot be split into a forged ``dkim=pass`` clause.
     """
-    text = _unfold(value)
+    text, well_formed = _strip_comments(_unfold(value))
-    clauses = [c.strip() for c in text.split(";")]
+    if not well_formed:
+        # Unbalanced quotes/comments: refuse to guess how to tokenise it.
+        return "", frozenset()
+    clauses = [c.strip() for c in _split_clauses(text)]
     if not clauses or not clauses[0]:
         return "", frozenset()
 

tests/test_ses_auth.py

@@ -108,6 +108,73 @@ class TestParseAuthenticationResults:
         )
         assert passing == frozenset({"seahaven.com"})
 
+    def test_quoted_semicolon_in_envelope_from_is_not_split(self, ses_auth):
+        # RFC 5321 quoted-local-part MAIL FROM can carry ';' and spaces; SES
+        # echoes it into envelope-from=. The ';' inside the quotes must stay
+        # part of the one envelope-from clause, never a synthetic dkim clause.
+        value = (
+            "amazonses.com; spf=pass client-ip=1.2.3.4;"
+            ' envelope-from="x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com;'
+            " helo=mail.attacker.com; dkim=fail header.i=@attacker.com;"
+        )
+        _, passing = ses_auth.parse_authentication_results(value)
+        assert passing == frozenset()
+
+    def test_quoted_pair_in_envelope_from_is_not_split(self, ses_auth):
+        # A quoted-pair (\") inside the quoted local part must not prematurely
+        # end the quoted-string and re-expose the smuggled tokens.
+        value = (
+            'amazonses.com; envelope-from="a\\"; dkim=pass header.d=seahaven.com"@evil.com;'
+            " dkim=fail header.i=@evil.com;"
+        )
+        _, passing = ses_auth.parse_authentication_results(value)
+        assert passing == frozenset()
+
+    def test_helo_injection_is_not_split(self, ses_auth):
+        # helo= is attacker-influenced too; a crafted value quoting a fake
+        # methodspec must not manufacture a passing clause.
+        value = (
+            'amazonses.com; helo="h; dkim=pass header.i=@seahaven.com";'
+            " dkim=fail header.i=@attacker.com;"
+        )
+        _, passing = ses_auth.parse_authentication_results(value)
+        assert passing == frozenset()
+
+    def test_comment_embedded_header_i_is_ignored(self, ses_auth):
+        # A CFWS comment carrying a fake header.i must be stripped before
+        # domain extraction, so only the real (failing) result is considered.
+        _, passing = ses_auth.parse_authentication_results(
+            "amazonses.com; dkim=fail (header.i=@seahaven.com) header.i=@attacker.com"
+        )
+        assert passing == frozenset()
+
+    def test_comment_hiding_semicolon_does_not_split(self, ses_auth):
+        # A ';' inside a comment is not a clause separator either.
+        _, passing = ses_auth.parse_authentication_results(
+            "amazonses.com; spf=pass (note: a; b) client-ip=1.2.3.4;"
+            " dkim=pass header.i=@seahaven.com"
+        )
+        assert passing == frozenset({"seahaven.com"})
+
+    def test_unbalanced_quote_fails_closed(self, ses_auth):
+        # An unterminated quoted-string is malformed; refuse to parse it so a
+        # dkim=pass clause "swallowed" by the runaway quote can't be salvaged
+        # (and, conversely, a runaway quote can't be used to mis-tokenise).
+        authserv_id, passing = ses_auth.parse_authentication_results(
+            'amazonses.com; envelope-from="oops@attacker.com;'
+            " dkim=pass header.i=@seahaven.com"
+        )
+        assert authserv_id == ""
+        assert passing == frozenset()
+
+    def test_unbalanced_comment_fails_closed(self, ses_auth):
+        # An unterminated comment is malformed and must fail closed.
+        authserv_id, passing = ses_auth.parse_authentication_results(
+            "amazonses.com; dkim=pass header.i=@seahaven.com (runaway comment"
+        )
+        assert authserv_id == ""
+        assert passing == frozenset()
+
 
 class TestEvaluateSenderAuthentication:
     def test_ses_stamped_pass_accepted(self, ses_auth):
@@ -206,6 +273,26 @@ class TestEvaluateSenderAuthentication:
         assert not accepted
         assert reason == "allowlist_not_configured"
 
+    def test_envelope_from_clause_injection_rejected(self, ses_auth):
+        # Full forged email: attacker's quoted MAIL FROM is echoed by SES into
+        # its own (topmost, genuinely SES-stamped) Authentication-Results as
+        # envelope-from=, trying to smuggle a dkim=pass for an allowlisted
+        # domain. The real DKIM verdict is fail. Must fail closed.
+        injected = (
+            "Authentication-Results: amazonses.com;\r\n"
+            " spf=pass (spfCheck: domain of attacker.com designates 1.2.3.4 as"
+            " permitted sender) client-ip=1.2.3.4;\r\n"
+            ' envelope-from="x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com;'
+            " helo=mail.attacker.com;\r\n"
+            " dkim=fail header.i=@attacker.com;\r\n"
+            " dmarc=fail header.from=attacker.com;\r\n"
+        )
+        accepted, reason, _ = ses_auth.evaluate_sender_authentication(
+            raw(injected), {"amazon.coupahost.com", "seahaven.com"}
+        )
+        assert not accepted
+        assert reason == "no_passing_dkim_signature"
+
 
 class TestAuthenticateInboundEmail:
     S3_KEY = "s3://bucket/inbound/abc123"