fix(security): stop four gates re-seeding the doc drift they caused

docs/ASVS-L2-PHASE0-CHANGES.md

@@ -374,7 +374,8 @@ tables in [`CONNECTIONS.md`](CONNECTIONS.md) §"Resource management & limits" (A
 > HTTP listeners) are **listeners**: no egress arm applies to them at any setting, because
 > `check_egress_allowed` gates destinations and `check_source_allowed` gates only the DATABASE
 > poll source and the REMOTEFILE source — the two source kinds that dial out. A listener is
-> gated by `calling_ae_allowlist` / `[inbound].source_ip_allowlist` instead. DICOMWEB rides
+> gated by `calling_ae_allowlist` / the per-connection `source_ip_allowlist` (an `inbound(...)`
+> keyword, not an `[inbound]` service key) instead. DICOMWEB rides
 > `allowed_http` like REST/SOAP/FHIR; DIRECT has its **own** `allowed_direct` list, deliberately
 > separate from `allowed_smtp`; and the **AI broker rides neither** — it is gated only by the
 > dedicated fail-closed `[ai].allowed_endpoints`.

docs/BACKLOG.md

@@ -7131,3 +7131,27 @@ The existing test is too weak to catch any of it: `test_insert_comment_reads_bac
 **Related:** #250 (the ops API this would sit on), #238, #26 (the declined visual-authoring line — why a declarative artifact interpreted by a second execution path is out).
 
 **Source:** Windmill/Kestra evaluation (2026-07-30), "Anvil Ops Tasks" design. Recorded here because the design memo holding the conditions has been deleted.
+## 252. DICOM SCP peer-control gate counts a spoofable AE-title list as sufficient
+
+> 🔢 **Filed 2026-07-30.** P2. Surfaced while fixing the refusal message that named a non-existent settings key (that half is fixed; this half is a contract change and was deliberately not ridden in on it).
+
+**Type:** security hardening — authentication strength of a fail-closed gate.
+
+**What:** `transports/dicom.py` refuses a non-loopback C-STORE SCP that has *no* peer control, but the check **counts controls rather than weighing them**:
+
+```python
+mtls_on = bool(s.get("tls")) and bool(s.get("tls_ca_file"))
+if self._host not in _LOOPBACK_HOSTS and not (
+    self._calling_ae_allowlist or self._source_ip_allowlist or mtls_on
+):
+```
+
+So `calling_ae_allowlist` **alone** satisfies a gate whose stated purpose is to fail closed. A DICOM Calling AE Title is a **caller-asserted string with no cryptographic binding** — any peer that guesses or learns one AE Title is admitted. Empirically confirmed: an SCP on `0.0.0.0` with only `calling_ae_allowlist=["MOD1"]` constructs without error.
+
+**Why it is filed rather than fixed:** the three-control set is a documented contract — [ADR 0025](adr/0025-dicom-codec-store-connectors.md) §9 and the `docs/SECURITY.md` decision-table row both enumerate `calling_ae_allowlist` / `source_ip_allowlist` / mTLS as co-equal satisfiers. Demoting one is an ADR amendment and a breaking change for any site relying on it, so it needs its own decision rather than being a rider on a message correction.
+
+**Options:** (a) keep three satisfiers but require AE-title to be paired with an IP allowlist or mTLS off-loopback; (b) demote AE-title to *not* satisfying the gate alone, with an explicit audited opt-out mirroring the other loosenings; (c) accept as-is and document the weakness at the gate (the refusal message now warns about it in-band).
+
+**Related:** ADR 0025 §9, `docs/SECURITY.md` DICOM peer-control row, `tests/test_dicom_scp_security.py::test_nonloopback_scp_with_calling_ae_allowlist_ok` (pins the current behaviour, so it changes with the decision).
+
+**Source:** adversarial documentation security review (2026-07-30); the message/doc half shipped alongside this filing.

docs/SECURITY.md

@@ -1040,9 +1040,13 @@ access decision — every one consumed at this release, on the control plane **a
 **Scope frame.** There are **two independent source-address allow-lists** with the same syntax and the
 same matcher, but deliberately different carve-outs. `[security].allowed_client_networks` restricts the
 **operator surface** (JSON API + `/ui` + `/ws/stats`) and **never** restricts an ingest listener;
-loopback is always allowed there. Per-connection `[inbound].source_ip_allowlist` restricts **one ingest
+loopback is always allowed there. The per-connection `source_ip_allowlist` restricts **one ingest
 listener** and deliberately does **not** inherit the loopback carve-out — an allow-list naming a partner
-must not also admit anything running on the local box.
+must not also admit anything running on the local box. ⚠️ That one is an **`inbound(...)` keyword** (or
+the top-level key in a `connections.toml` `[[inbound]]` table); there is **no**
+`[inbound].source_ip_allowlist` service setting. `[inbound]` carries only `bind_host`, `ack_after` and
+`stream_inflight_budget_bytes`, and every section model ignores unknown keys — so that spelling in
+`messagefoundry.toml` is accepted and **silently discarded**, leaving the listener ungated.
 
 **Action vocabulary (closed set).** Every row below **opens its Action cell** with exactly one of:
 **ALLOW** (pass through), **DENY** (403 / 401 / 400 / 409 / refused connection / DIMSE status /
@@ -1093,7 +1097,7 @@ one-to-one — that is why the bind/exposure posture occupies two rows and the A
 
 #### Table B — data plane (ingest listeners)
 
-`[inbound].source_ip_allowlist` is enforced on **five** listener types — at **accept** on the four
+The per-connection `source_ip_allowlist` is enforced on **five** listener types — at **accept** on the four
 stream listeners (MLLP, TCP, X12, HTTP), and for **DICOM inside the C-STORE handler**, i.e. after the
 association has been negotiated and accepted and after the object has been received, but before any
 durable commit. A non-allowlisted DICOM peer can therefore still establish an association, consume a
@@ -1114,7 +1118,7 @@ listen source. The refusal action differs materially per listener, so each has i
 | **MLLP / HTTP / DICOM** — peer client certificate | the TLS peer certificate presented at handshake | `tls = true` **and** `tls_ca_file` set → `ssl.CERT_REQUIRED` plus strict RFC 5280 verify flags; no client certificate, or one not issued by that CA | **DENY** — the TLS handshake fails and the connection **never reaches the accept path**, so there is **no** connection event and no allow-list evaluation. `tls_ca_file` unset → server-only TLS and no peer-certificate decision. TCP and X12 have no inbound TLS at this release |
 | **DICOM** — calling AE | the requesting AE's Calling AE Title, at **association negotiation** | `calling_ae_allowlist` set and the title is not in it | **DENY** — the association is rejected by pynetdicom before any C-STORE callback runs (`ae.require_calling_aet`). `None` = any AE the peer-IP allow-list admits |
 | **DICOM** — called AE | the AE Title the peer addressed the association to | not this engine's own `ae_title` | **DENY** at negotiation (`ae.require_called_aet`); **default `require_called_ae_title = true`** |
-| **DICOM** — peer-control construction gate | the SCP's bind host × the presence of any peer control | non-loopback bind with **none** of `calling_ae_allowlist`, `[inbound].source_ip_allowlist`, or mTLS (`tls` + `tls_ca_file` → `CERT_REQUIRED`) | **DENY at construction** (ValueError). The connection degrades per ADR 0031 startup fault isolation and the fault surfaces under `messagefoundry check` / dry-run. Loopback hosts are exempt |
+| **DICOM** — peer-control construction gate | the SCP's bind host × the presence of any peer control | non-loopback bind with **none** of `calling_ae_allowlist`, `source_ip_allowlist` (an `inbound(...)` keyword — for a DICOM SCP the ONLY surface, since `DICOM()` is not authorable in `connections.toml`), or mTLS (`tls` + `tls_ca_file` → `CERT_REQUIRED`) | **DENY at construction** (ValueError). The connection degrades per ADR 0031 startup fault isolation and the fault surfaces under `messagefoundry check` / dry-run. Loopback hosts are exempt |
 
 > **Telemetry honesty.** The `peer_not_allowlisted` connection event is durable when the connection's
 > `capture_connection_errors` is `true`, **or is unset (`None`, the default) and the

messagefoundry/__main__.py

@@ -980,7 +980,7 @@
 _DEFAULT_SERVICE_TOML = "messagefoundry.toml"
 
 
 def _serve(args: argparse.Namespace) -> int:
     import uvicorn
     from pydantic import ValidationError
 
@@ -1132,10 +1132,13 @@
     )
 
     # Delegated-identity precondition (#203, ASVS 13.2.1/13.3.2): when the operator declares
-    # [store].require_managed_identity, refuse (production) / warn (non-production) if the store
-    # authenticates with a static credential rather than a managed/delegated identity (Windows
-    # Integrated / Entra). Off by default → byte-identical. Admin device posture and AD/SMTP managed
-    # identity stay deployment-delegated (documented in docs/SECURITY.md), not engine-checked here.
+    # [store].require_managed_identity, refuse to start if the store authenticates with a static
+    # credential rather than a managed/delegated identity (Windows Integrated / Entra). The refuse/warn
+    # split is [security].enforcement, NOT the deployment tier — the branch below reads `enforcing`, and
+    # `enforce` is the shipped default on dev and staging as much as on prod, so a staging box that
+    # turns this on and leaves a static credential is REFUSED, not warned. It downgrades to a warning
+    # only under enforcement = warn. Off by default → byte-identical. Admin device posture and AD/SMTP
+    # managed identity stay deployment-delegated (documented in docs/SECURITY.md), not engine-checked.
     mi_reason = settings.store.managed_identity_precondition()
     if mi_reason is not None:
         if enforcing:
@@ -1235,28 +1238,53 @@
     #
     # Open-egress posture (Q5b): on a PHI-carrying instance, outbound egress that is fully
     # unrestricted — no [egress] allowlist AND deny_by_default off — lets a transform send PHI to any
-    # destination. On a PRODUCTION instance this fails closed (refuse to start, the prod analogue of
-    # the keyless-store refusal above); on a non-production PHI instance (e.g. staging) it is an
-    # advisory warning. A synthetic instance stays quiet. Lock it down with [security].block_unlisted_outbound
-    # or per-transport [egress].allowed_* lists.
+    # destination. The refuse/warn split is [security].enforcement, NOT the deployment tier: the branch
+    # below reads `enforcing`, and `enforce` is the shipped default on dev and staging as much as on
+    # prod, so all three REFUSE on stock defaults. It downgrades to an advisory warning only under
+    # enforcement = warn. A synthetic instance carries no PHI and stays quiet. Lock it down with
+    # [security].block_unlisted_outbound or per-transport [egress].allowed_* lists.
+    #
+    # [egress] declares EIGHT allowed_* lists and every one is enforced downstream by _allowlist_for
+    # (pipeline/wiring_runner.py). Counting only six here meant a mail-only or Direct-only instance
+    # could enumerate every destination it actually uses and still be refused as "UNRESTRICTED", with
+    # nothing in the refusal naming the two lists that did not count. allowed_smtp/allowed_direct are
+    # counted only when [security].block_unlisted_outbound was NOT set explicitly — precisely the state
+    # the flip below turns deny-by-default ON for, so such an instance still starts fail-closed. An
+    # instance that explicitly opted OUT of deny-by-default is deliberately unchanged: it cannot
+    # satisfy this gate on smtp/direct alone, because there the other six transports stay allow-any.
     if data_class is DataClass.PHI:
         eg = settings.egress
-        egress_open = not eg.deny_by_default and not (
+        listed = (
             eg.allowed_mllp
             or eg.allowed_tcp
             or eg.allowed_http
             or eg.allowed_db
             or eg.allowed_remote
             or eg.allowed_file_dirs
         )
+        if "deny_by_default" not in eg.model_fields_set:
+            listed = listed or eg.allowed_smtp or eg.allowed_direct
+        egress_open = not eg.deny_by_default and not listed
         if egress_open:
+            # Reaching here WITH allowed_smtp/allowed_direct declared is only possible when
+            # [security].block_unlisted_outbound was set explicitly (otherwise those two count above), so
+            # name that override rather than leaving the operator to wonder why a declared allowlist did
+            # not satisfy the gate.
+            mail_only_note = (
+                " You have declared [egress].allowed_smtp/allowed_direct, but those satisfy this gate "
+                "only when [security].block_unlisted_outbound is left unset — setting it false opts out "
+                "of the deny-by-default flip, which would leave every OTHER transport allow-any. Remove "
+                "that override (or set it true) and a mail-only/Direct-only allowlist is accepted."
+                if (eg.allowed_smtp or eg.allowed_direct)
+                else ""
+            )
             if enforcing:
                 print(
                     f"error: outbound egress is UNRESTRICTED on a "
                     f"{'production ' if production else ''}PHI instance "
                     f"({env_name!r}); refusing to start — a transform could send PHI to any "
                     "destination. Set [security].block_unlisted_outbound=true, or declare the permitted "
-                    "destinations with per-transport [egress].allowed_* allowlists.",
+                    f"destinations with per-transport [egress].allowed_* allowlists.{mail_only_note}",
                     file=sys.stderr,
                 )
                 return 2
@@ -2078,9 +2106,12 @@
     # The per-user security-event push (lockout, password/email/roles change, new-IP admin action)
     # rides the [alerts] SMTP transport AND the [auth].notify_security_events kill-switch — api/app.py
     # builds the notifier only when BOTH are on, so with either off it is silently absent (which the
-    # defaults and the off-loopback runbook never set). Mirror the retention posture: a PRODUCTION PHI
-    # instance with no effective channel REFUSES to start; a non-production PHI instance WARNS;
-    # synthetic/dev is byte-identical. The explicit, audited opt-out is
+    # defaults and the off-loopback runbook never set). A PHI instance with no effective channel REFUSES
+    # to start; synthetic/dev is byte-identical. The refuse/warn split is [security].enforcement, NOT the
+    # deployment tier — the branch below reads `enforcing`, and `enforce` is the shipped default on dev
+    # and staging as much as on prod, so `serve --env staging` on stock defaults with no [alerts] SMTP is
+    # REFUSED, not warned. It downgrades to a warning only under enforcement = warn. This gate is why
+    # [alerts] is not optional on a stock instance. The explicit, audited opt-out is
     # [alerts].security_notifications_required=false (accept the pull-only /me/security-events feed in
     # writing). "Effective channel" == notify_security_events on + SMTP host + sender (parity with the
     # app.py notifier wiring). Skipped when auth is disabled (no accounts to notify — a non-loopback
@@ -2100,7 +2131,9 @@
                         "start — account-security events (lockout, password/roles change, new-IP admin "
                         "action) would have no push channel, only the pull-only /me/security-events feed "
                         "(ASVS 6.3.5/6.3.7). Configure the [alerts] SMTP transport (email_smtp_host + "
-                        "email_from) and keep [auth].notify_security_events on; or, to rely on the "
+                        'email_from; add email_to as well if any [[alerts.rules]] routes to "email" — '
+                        "the alert email transport requires all three) and keep "
+                        "[auth].notify_security_events on; or, to rely on the "
                         "pull-only feed, set [alerts].security_notifications_required=false (audited).",
                         file=sys.stderr,
                     )
@@ -4184,7 +4217,7 @@
     return 0
 
 
 def _alert(args: argparse.Namespace) -> int:
     """Manage the operator-authored ``[[alerts.rules]]`` in the service-settings TOML (ADR 0014):
     ``list`` to populate the VS Code editor, ``add``/``remove`` to save (a developer can also hand-
     edit the file). ``add``/``remove`` re-load the whole settings file BEFORE persisting and roll
@@ -4194,6 +4227,7 @@
 
     from messagefoundry.config import alerts_edit
     from messagefoundry.config.settings import AlertRule, load_settings
+    from messagefoundry.pipeline.alert_sinks import configured_alert_transport_names
 
     path = args.service_config
 
@@ -4215,9 +4249,38 @@
             raw = args.data if args.data is not None else sys.stdin.read()
             obj = json.loads(raw)
             try:
-                AlertRule.model_validate(obj)  # precise per-field error before we touch the file
+                new_rule = AlertRule.model_validate(obj)
             except ValidationError as exc:
                 return _emit_error(f"invalid alert rule: {exc}", as_json=args.json)
+            # Routing cross-check at AUTHORING time. notifier_from_settings refuses a rule that routes to
+            # an unconfigured transport, but that fires at the next START — so without this the editor
+            # happily writes a rule that bricks the following boot with no diagnostic here. Scoped to the
+            # rule being ADDED (not every rule in the file) so a file that already contains a bad rule can
+            # still be repaired with `alert remove` instead of being wedged shut.
+            routed = set(new_rule.transports or [])
+            for step in new_rule.escalate:
+                routed |= set(step.transports or [])
+            if routed:
+                # Only a rule that NAMES a transport can be refused, so only that case needs the settings
+                # file — which keeps `alert add` working against a file that does not exist yet (the
+                # from-scratch create path, where a rule routing nowhere is still perfectly valid).
+                try:
+                    configured = configured_alert_transport_names(
+                        load_settings(config_path=path).alerts
+                    )
+                except (OSError, ValueError):
+                    configured = (
+                        set()
+                    )  # unreadable/absent settings file → nothing is configured yet
+                unknown = sorted(routed - configured)
+                if unknown:
+                    return _emit_error(
+                        f"alert rule routes to unconfigured transport(s) {unknown}; this instance "
+                        f"configures {sorted(configured) or 'none'}. Configure [alerts].webhook_url, or "
+                        "email_smtp_host + email_from + email_to (all three), before adding the rule — "
+                        "otherwise the engine refuses to start.",
+                        as_json=args.json,
+                    )
             result = alerts_edit.add_rule(path, obj, validate=validate)
         else:  # remove
             if args.index is None:

messagefoundry/config/settings.py

@@ -3496,7 +3496,8 @@ class SecuritySettings(_Section):
     # host address ("10.20.4.7" -> /32); IPv4 and IPv6, mixed freely. Same syntax as an inbound
     # connection's source_ip_allowlist, and the same matcher (messagefoundry.netaddr).
     # SCOPE: the OPERATOR surface only. It does NOT restrict the MLLP/TCP/X12/DICOM/HTTP ingest
-    # listeners — those have their own per-connection [inbound].source_ip_allowlist.
+    # listeners — those have their own per-connection source_ip_allowlist (an inbound(...) keyword,
+    # NOT an [inbound] service key: that spelling is accepted and silently discarded).
     # LOOPBACK IS ALWAYS ALLOWED, unconditionally and with no knob: the credential-less on-box clients
     # (the tray's tokenless /health poll (ADR 0113), a browser opening /ui on the engine host,
     # `messagefoundry check`, the harness/apiclient, a container HEALTHCHECK) cannot be allow-listed, so