fix(smtp): the EMAIL and DIRECT TLS hops were encrypted but unauthenticated (#323, layers 1-2)

docs/ASVS-L2-PHASE0-CHANGES.md

@@ -314,8 +314,8 @@ tables in [`CONNECTIONS.md`](CONNECTIONS.md) §"Resource management & limits" (A
 | DICOMweb STOW-RS destination (`dicomweb`) | outbound | HTTPS; port from the base URL (ADR 0025 Phase 2) | `verify_tls` default true (false needs `MEFOR_ALLOW_INSECURE_TLS`) | static bearer or HTTP Basic | **yes** — `url` per Connection | `DICOMweb(url, study_uid, timeout_seconds, verify_tls)`, `[egress].allowed_http` |
 | DICOM C-STORE SCP (`dimse`) | inbound | DICOM upper layer over TCP, **default port 104**, bound to `[inbound].bind_host` | opt-in TLS with opt-in mTLS via `tls_ca_file`; a **non-loopback cleartext SCP is refused at start** unless `serve --allow-insecure-bind` | `calling_ae_allowlist` (AE-Title gate) + `require_called_ae_title` (default true) + the `[inbound].source_ip_allowlist` IP gate | bind host is `[inbound].bind_host`; the modality dials in | `DICOM(ae_title, port, calling_ae_allowlist, require_called_ae_title, max_associations, max_pdu_size, max_object_bytes, timeout_seconds)` |
 | DICOM C-STORE SCU + C-ECHO destination (`dimse`) | outbound | DICOM upper layer over TCP, **default port 104** | opt-in TLS (`tls`, `tls_allow_expired`) | the association's calling / `called_ae_title` AE Titles | **yes** — `host`/`port`/`called_ae_title` per Connection | `DICOM(host, port, called_ae_title, timeout_seconds, connect_timeout)`, `[egress].allowed_tcp` |
-| SMTP email destination (`email`) | outbound | SMTP, **default port 587** (STARTTLS submission); port 465 selects implicit TLS (ADR 0029) | `use_tls` default true (STARTTLS); false needs `MEFOR_ALLOW_INSECURE_TLS` | optional SMTP AUTH `username` / `password` (`env()`) | **yes** — `host`/`port` per Connection | `Email(host, port, sender, recipients, use_tls, username, password, timeout_seconds)`, `[egress].allowed_smtp` |
-| Direct-Project S/MIME destination (`direct`) | outbound | SMTP to a HISP relay, **default port 587** (465 = implicit TLS), ADR 0085 | STARTTLS on by default; the sign-then-encrypt S/MIME body protects PHI **independently of** session TLS | the sender's S/MIME `signing_key` + `signing_cert` (+ optional `signing_key_password`), the partner `recipient_cert` chaining to `trust_anchor`, plus optional SMTP AUTH | **yes** — `host` / `recipient_cert` / `trust_anchor` per Connection | `Direct(host, port, signing_key, signing_cert, recipient_cert, trust_anchor, use_tls, timeout_seconds)`, `[egress].allowed_direct` |
+| SMTP email destination (`email`) | outbound | SMTP, **default port 587** (STARTTLS submission); port 465 selects implicit TLS (ADR 0029) | `use_tls` default true (STARTTLS); false needs `MEFOR_ALLOW_INSECURE_TLS`. The server certificate **is verified** (`tls_verify` default true, #323) — chain + hostname + strict RFC 5280, anchored to the OS roots, a per-connection `tls_ca_file`, or `[tls].internal_ca_file`; `tls_verify=false` needs the **clamped** escape and also refuses SMTP AUTH | optional SMTP AUTH `username` / `password` (`env()`) | **yes** — `host`/`port` per Connection | `Email(host, port, sender, recipients, use_tls, username, password, timeout_seconds)`, `[egress].allowed_smtp` |
+| Direct-Project S/MIME destination (`direct`) | outbound | SMTP to a HISP relay, **default port 587** (465 = implicit TLS), ADR 0085 | STARTTLS on by default **and the relay certificate is verified** (`tls_verify` default true, #323; `tls_ca_file` is the TLS-hop CA, distinct from `trust_anchor`, which is the partner's S/MIME CA); the sign-then-encrypt S/MIME body protects PHI **independently of** session TLS | the sender's S/MIME `signing_key` + `signing_cert` (+ optional `signing_key_password`), the partner `recipient_cert` chaining to `trust_anchor`, plus optional SMTP AUTH | **yes** — `host` / `recipient_cert` / `trust_anchor` per Connection | `Direct(host, port, signing_key, signing_cert, recipient_cert, trust_anchor, use_tls, timeout_seconds)`, `[egress].allowed_direct` |
 | DATABASE destination, poll source, and `db_lookup` read connection (`database`) | outbound (the poll source also dials out, for inbound data) | ODBC — TDS for the `sqlserver` dialect, **default port 1433** | the `sqlserver` dialect enforces Driver-18 TLS (`encrypt` default true, `trust_server_certificate` default false); the `generic` dialect delegates TLS to the operator's `odbc_params` | SQL / Integrated / Entra; statements are parameterized | **yes** — `server` / `database` / statement per Connection | `Database(...)`, `DatabasePoll(...)`, `DatabaseLookup(...)` with `connect_timeout`, `pool_max`, `acquire_timeout`; `[egress].allowed_db` |
 | Reference-set sync dial-out — `DatabaseRef` (`database`) | outbound (periodic) | ODBC, **default port 1433** (ADR 0006) | as above | as above | **yes** — `server` / `statement` per reference set | `DatabaseRef(...)` + `Reference(name, refresh_seconds)`; `[egress].allowed_db` |
 | Internal sources that **open no socket**: `timer`, `loopback`, `passthrough` | inbound (internal only) | none — clock-driven (ADR 0011), re-ingress-only (ADR 0013), and Handler-fed pass-through respectively | n/a | n/a — they reach no external system | no | `Timer(...)`, `Loopback()`, `PassThrough()` |

docs/BACKLOG.md

@@ -5261,7 +5261,9 @@ sourced — **#1 (SQL Server concurrency)** and **#2 (console off-thread)** —
 
 **Why:** Real gap. The alert SMTP sink (EmailTransport in pipeline/alert_sinks.py) calls starttls() with the default SSL context and exposes only email_use_tls plus the global MEFOR_ALLOW_INSECURE_TLS cleartext escape, so there is no per-mail-server option to keep TLS on yet unconditionally trust a self-signed/mismatched certificate.
 
-**Why it is an anti-feature:** unconditionally trusting an SMTP server's certificate defeats TLS. The engine's `EmailAlertSink` uses STARTTLS with a verifying context by design; the only escape (`MEFOR_ALLOW_INSECURE_TLS`) is global and deliberately loud. Recorded for parity completeness, not as a want.
+**Why it is an anti-feature:** unconditionally trusting an SMTP server's certificate defeats TLS. The only escape (`MEFOR_ALLOW_INSECURE_TLS`) is global and deliberately loud. Recorded for parity completeness, not as a want.
+
+> ⚠️ **CORRECTED 2026-08-01 — this item previously asserted "The engine's `EmailAlertSink` uses STARTTLS with a verifying context by design." That was FALSE**, and it is the shape [`CLAUDE.md`](../CLAUDE.md) §11 names as worst: *a compensating control resting on a false premise*. `smtplib.starttls()` with no context falls back to `ssl._create_stdlib_context`, which **is** `ssl._create_unverified_context` (`CERT_NONE`, `check_hostname=False`) — so the alert sink was encrypting without authenticating, and a reader of this item would have concluded alert email was TLS-verified when it was not. #323 fixed the two **connectors** (`EmailDestination` / `DirectDestination`); it did **not** fix the alert sink, so the **Why:** paragraph above remains accurate and this item's premise stays false until #323's alerts-cell residual lands. Do not re-assert verification here before then.
 
 **Nearest existing mechanism:** EmailTransport / send_plain_email in pipeline/alert_sinks.py (SMTP alert sink, built by notifier_from_settings from AlertsSettings in config/settings.py); its only TLS knob is email_use_tls (STARTTLS on/off) plus the global MEFOR_ALLOW_INSECURE_TLS / insecure_tls_allowed() escape, which permits CLEARTEXT SMTP — not a keep-TLS-but-trust-any-cert override.
 
@@ -7397,7 +7399,15 @@ MEFOR_FORBIDDEN_TOKENS=scripts/security/scan-tokens.local.txt.example \
 
 ## 323. SMTP TLS is unverified on all three send paths
 
-> 🔢 **Filed 2026-08-01 — not started.** Value **8/10** · Difficulty **4/10** · _fill-in_. All three SMTP send paths call `starttls()` / `SMTP_SSL()` with **no** SSL context, so Python 3.14's stdlib default applies (`CERT_NONE`, `check_hostname=False`) — the EMAIL destination puts Handler PHI *and* the SMTP AUTH password over an encrypted-but-unauthenticated hop, while the #201 revocation guard, the cleartext-credential rule and `DEPLOYMENT.md` all describe that same hop as **verified**.
+> 🚧 **Status 2026-08-01 — PARTIALLY SHIPPED, 2 of 3 cells** (PR #132). The two **connectors** verify: `EmailDestination` and `DirectDestination` build an explicit verifying context via the new `tls_policy.build_smtp_tls_context()` and pass it on both `smtplib` arms, with per-connection `tls_verify` / `tls_ca_file` / `tls_check_hostname` on the `Email()` and `Direct()` factories. Proven by **negative control**: with the code change stashed and the tests kept, all eight new assertions in `tests/test_email_destination.py` go **red** — the pre-existing "STARTTLS was issued" assertions stayed green throughout the insecure period and could never have caught this. ⚠️ **The alerts cell is NOT fixed**: `pipeline/alert_sinks.py:384` still calls `smtp.starttls()` bare, so alert + security-notify email remains unverified. That is the residual below, and it is why [#139](#139)'s premise is still false.
+>
+> 🔢 **Filed 2026-08-01.** Value **8/10** · Difficulty **4/10** · _fill-in_. All SMTP send paths called `starttls()` / `SMTP_SSL()` with **no** SSL context, so Python 3.14's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` — `CERT_NONE`, `check_hostname=False`) — the EMAIL destination put Handler PHI *and* the SMTP AUTH password over an encrypted-but-unauthenticated hop, while the #201 revocation guard, the cleartext-credential rule and `DEPLOYMENT.md` all described that same hop as **verified**.
+
+**Residual — the alerts cell (~1 layer).** `pipeline/alert_sinks.py` `send_plain_email` and `pipeline/security_notify.py` need the same context, plumbed from new `[alerts].email_tls_verify` / `email_tls_ca_file`, plus a `[security].allow_unverified_alert_smtp_tls` acknowledgment switch at the serve gate. It needs an **acknowledgment switch rather than the clamp** because the contextvar hop posture is never stamped for that cell — which is why it was deferred rather than folded in. The shared `refuse_unverified_smtp_tls()` helper belongs in `config/settings.py` at that point; the connectors currently **inline** the refusal, matching the `mllp.py` / `remotefile.py` house style. Then: register the deviation in `security_loosenings()` (see [#333](#333)), add a `checks.py` advisory, and correct `docs/PHI.md` and [#139](#139).
+
+**Also landed, called out rather than folded in silently.** `transports/direct.py`'s cleartext arm read the **unclamped** `insecure_tls_allowed()` while its sibling arm one branch away read the clamped `weakened_tls_escape_permitted_here()`. Two different escapes in one connector is how the next bug gets written, so it now reads the clamped one. Strictly **adds** refusals (ADR 0092 decision 5). Partially closes the [#329](#329) concern.
+
+**Correction to this item's own text.** The "Migration risk, stated plainly" framing filed with this item presumed existing deployments. The owner confirmed on 2026-08-01 that **there are none**, so secure-by-default was simply correct and no phased rollout, CHANGELOG breaking entry or migration guide was warranted. Do not resurrect that framing from this item's history.
 
 **Cluster:** Security & Compliance. **Priority:** P1. **Verdict:** build. **Severity:** high.
 
@@ -7759,6 +7769,12 @@ What it *is*: the realistic failure is a dev/CI environment variable riding into
 
 The AI-broker cell has an additional argument: `transports/smart.py:126-149` moved the *same* question — a credential on a cleartext token endpoint — off the raw escape and onto `refuse_cleartext_credential_hop` in commit `a3015196`, with a comment describing exactly this defect (*"It used to read the raw, UNCLAMPED `MEFOR_ALLOW_INSECURE_TLS`"*). `ai_broker.py:140` is the un-migrated twin of a cell fixed days ago.
 
+**Additionally — converting all five is what makes the property *checkable*, not just true.** While these five remain, "no unclamped escape survives on an enforcing PHI posture" is five separate per-site facts, each verifiable only by opening the site and reading it, and each silently falsified by a sixth cell added later. Convert them all and it collapses into **one repo-wide invariant**: the raw `insecure_tls_allowed()` becomes unreachable outside `config/settings.py`'s own clamp, so the absence of the raw predicate is checkable everywhere at once, with `weakened_tls_escape_permitted_here` as the thing that must still be present. Today the property is a convention enforced by review; afterwards it is an invariant enforced by a grep — and a *new* unclamped cell fails immediately instead of waiting for the next audit to enumerate it.
+
+That distinction matters concretely for the ASVS record. The scorecard's absence-claim mechanism runs regexes over the whole `*.py` corpus and **cannot scope a grep to one file**, so a per-connector claim ("`direct.py`'s escape is clamped") is not expressible and has to be carried as stated-but-unchecked prose. A repo-wide claim is expressible and machine-verified on every commit. So this item is not only five leaks to plug: it is the difference between a security property that must be re-audited by hand and one that a gate can hold. *(Framing contributed by the ADR 0156 ASVS-sweep session, 2026-08-02.)*
+
+**Scope note, because the count is moving and two censuses will disagree.** #323 routes `transports/direct.py` and `transports/email.py` through the clamp, taking the remaining set to four once it lands — so a census taken on that branch disagrees with one taken on `main`, and neither is wrong. Measured at `main` by counting **`ast.Call` nodes**, not matching lines: six real call sites outside `config/settings.py` — `auth/ldap.py`, `pipeline/alert_sinks.py`, `transports/ai_broker.py`, `transports/database.py`, `transports/direct.py`, `transports/remotefile.py`. `transports/database.py` is the documented unstamped fallback, excluded above; `transports/mllp.py` matches a naive grep for the raw name but its occurrence is **prose inside a docstring, not a call at all**. A line-based census reports it as a further site; an AST-based one does not — which is the instrument distinction, not a detail about this item.
+
 **Proposed:** convert all five, but note that a blanket swap to `weakened_tls_escape_permitted_here()` would silently fix only two of them.
 
 1. **In-gate cells — a one-line swap each.** `remotefile.py:375` and `direct.py:170` are built inside `build_check_registry`/`wiring_runner`'s `active_hop_posture` scope (`config/tls_policy.py:587-603`; the stamping sites are all in `pipeline/wiring_runner.py`), so `weakened_tls_escape_permitted_here()` reads a real posture there — byte-identical to how `remotefile.py:176`/`:577` and `email.py:134` already behave.
@@ -8178,7 +8194,9 @@ ADR 0144:193-195 records the decorated-scope trade, but justifies it with an **`
 
 **The third gap the audit named is narrower than described.** The non-recursive `base.glob("*.py")` at `checks.py:893`/`:898` (ADR 0144:196) is **not** an unscanned execution path. `load_config` globs `directory.glob("*.py")` non-recursively too (`config/wiring.py:3969`, and `:4392` for `validate_config`), and `_SiblingHelperFinder.find_spec` returns `None` for any dotted name and serves only `_`-prefixed top-level helpers from the config dir (`wiring.py:3902`, `:3908-3912`). A `.py` in a config subdirectory is therefore neither executed by the loader nor importable by a sibling — and `_assert_safe_config_source` is non-recursive for the same reason (`wiring.py:4194`, `:4324`). The lint's file set already equals the executable set. A recursive walk here would make the lint report on files the safe-source ownership gate never vets — an asymmetry in the other direction. #226 (`docs/BACKLOG.md:6917`) already parks recursion as a *loader* question; it belongs there, not here.
 
-**Why:** Bounded, and bounded hard. The lint is advisory by default (`checks.py:953`, `ok=not strict, required=strict`), so a finding blocks nobody unless an adopter opts into `--strict-handler-security` on their own CI. It governs code the adopter's own administrator authors, inside a directory whose write access is already the trust boundary (`_assert_safe_config_source`, `wiring.py:4194`/`:4324`) — anyone who can drop a `.py` there already has arbitrary in-process execution under the engine account, so this is **not** a privilege boundary and evading it buys an attacker nothing they did not already have. ADR 0144:171-174 and the `_check_handler_security` docstring (`checks.py:878`) both say so: "a filter, not a fix." There is no PHI-exposure path and no runtime behaviour change of any kind.
+**Why:** Bounded, and bounded hard. The lint is advisory by default (`checks.py:953`, `ok=not strict, required=strict`), so a finding blocks nobody unless an adopter opts into `--strict-handler-security` on their own CI. It governs code the adopter's own administrator authors, inside a directory whose write access is already the trust boundary (`_assert_safe_config_source`, `wiring.py:4194`/`:4324`) — anyone who can drop a `.py` there already has arbitrary in-process execution under the engine account, so this is **not** a privilege boundary and evading it buys an attacker nothing they did not already have.
+
+> ⚠️ **Rationale amended 2026-08-01 (ADR 0087 sandbox session) — the severity is right, the reason was not.** "The author already has in-process execution" is true at the **default** `[sandbox].mode=off`, and **false** under `mode=subprocess`, where the entire premise is that the author is *not* trusted with it. A severity floor resting on a posture-specific claim reads as settled and misleads the next reader. The rationale that holds in **both** postures: the lint is advisory and pre-deployment; under `mode=off` the author already has in-process execution, and under `mode=subprocess` an evasion still only reaches **host** actions the sandbox does not confine — `DEFAULT_FORBIDDEN_MODULES` (`pipeline/sandbox.py:84-95`) blocks `socket`, `ssl`, `asyncio`, `multiprocessing`, the I/O-bearing `messagefoundry.*` subpackages and `cryptography`, but **not `os` or `subprocess`** (verified at HEAD). ADR 0087 confines the **address space** (the child cannot reach the parent's DEK, audit chain or sockets), not the **host**; OS-level default-deny is ADR 0147, *Proposed with no code*. So an evasion reaches neither the DEK nor the audit chain in either posture. **Re-score upward when ADR 0147 lands**, at which point the lint becomes load-bearing for exactly the class OS confinement is meant to close. ADR 0144:171-174 and the `_check_handler_security` docstring (`checks.py:878`) both say so: "a filter, not a fix." There is no PHI-exposure path and no runtime behaviour change of any kind.
 
 What it *is*: an adopter who turns on the strict gate gets a **green build** on a Handler containing `getattr(os, "system")`, and gets a green build on a transforms helper logging `msg.raw` at INFO. Gap (2) is the one that actually costs something, because the miss is not a malicious bypass — it is the ordinary fallible-author case ADR 0144 exists for, landing in the exact file the project's own layout guidance created. Gap (1) is mostly a claim-hygiene problem: the ADR asserts the false negative in prose and no test proves it, so nobody notices if a future change silently widens or narrows it.