fix(dicom): an AE-title list is not a peer control on its own (BACKLOG #252)

[wshallwshall]

Implements option (a) of the three recorded on BACKLOG #252, chosen by the owner: pair, do not remove.

The defect

The fail-closed peer-control gate refuses a non-loopback DICOM C-STORE SCP that has no peer control, and it accepted any one of three — calling_ae_allowlist, source_ip_allowlist, or mTLS:

if self._host not in _LOOPBACK_HOSTS and not (
    self._calling_ae_allowlist or self._source_ip_allowlist or mtls_on
):

It counted the controls rather than weighing them. Two of the three are real: an IP allowlist constrains who can reach you; mTLS proves the peer holds a private key. The third is not.

A Calling AE Title is a string the caller asserts about itself in the association request — no key, no signature, nothing to verify. It is a username with no password. And AE Titles are not secret: they are published in DICOM conformance statements, written into integration runbooks, and visible in any packet capture.

So an SCP whose only peer control was an AE-title list was reachable by anyone who could route to it and knew one string — while passing a check named "fail-closed peer controls".

Server TLS does not close this. Without tls_ca_file there is no client certificate, so the cleartext bind guard (confidentiality) and this gate (authentication) are orthogonal; satisfying one says nothing about the other.

What changed

Off-loopback, the gate now requires a verifiable control — source_ip_allowlist or mTLS. calling_ae_allowlist is kept and still enforced at association time: it catches a misrouted sender and pins intent, which is real value. It simply cannot stand alone.

Measured (executed, not reasoned about):

config, off-loopback	before	after
nothing	REFUSED	REFUSED
calling_ae_allowlist only	STARTS	REFUSED
calling_ae_allowlist + source_ip_allowlist	STARTS	STARTS
source_ip_allowlist only	STARTS	STARTS
mTLS only	STARTS	STARTS
any loopback bind	STARTS	STARTS

The dev/single-box loopback case does not regress.

The refusal detects the unpaired case specifically and says why an AE Title cannot be verified — and tells the operator to keep it and add a control. Without that, the message reads as "but I already set an allowlist" and the obvious wrong move is to delete the AE list.

⚠️ BREAKING — for release notes

A site whose non-loopback SCP relies on calling_ae_allowlist alone starts today and will refuse after upgrading. The fix is one line — add source_ip_allowlist=[...] to the inbound(...) call, which for a DICOM SCP is the only authoring surface — and the refusal names it. Keep the AE list; it is still doing work.

Unlike the other loosening-style gates in this codebase, option (a) ships without an audited opt-out. That was deliberate: an IP allowlist is available to essentially every real deployment (you know your modalities' addresses), so an escape hatch here would mostly serve to preserve the weak configuration. Options (b) opt-out and (c) document-only were declined.

A correction worth reading

The code and both documents credited this rule to "ADR 0025 §9". That citation is dangling:

• ADR 0025 has no §9 — its decision sections run 1–8. The §9 references in that file point at CLAUDE.md §9 (the file uses CLAUDE.md §1/§2/§4/§6/§8/§9 elsewhere).
• calling_ae_allowlist appears nowhere in ADR 0025.

The normative statement actually lives in docs/CONNECTIONS.md and the docs/SECURITY.md decision table — both amended here. No ADR amendment was possible, because there is no ADR text to amend. The new code, tests and docs cite BACKLOG #252, which is real, rather than repeating a pointer that sends a reader somewhere they cannot go. That is the same defect class this whole series has been fixing, so it did not seem right to propagate it.

Verification

• Unfiltered pytest: 9518 passed, 817 skipped, 0 failed (13:16). No -k filter.
• Tests now pin the inversion (AE-title alone → refused, asserting the message explains why and says to keep it) and the pairing case.
• ruff check / ruff format clean.
• BACKLOG test(sandbox): a static AST guard pins the codec and worker import boundary (BACKLOG #346) #252 flipped to ✅ SHIPPED with the measured matrix; backlog invariant holds (242 items).

🤖 Generated with Claude Code