From bd37e233409221d85c0e293bbb1c18934546e47b Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 22 Aug 2026 12:17:46 -0500 Subject: [PATCH 1/4] backlog: file #1319 -- the demote-teardown timing assert cannot discriminate its own mutation under load Recovered from a session that was measuring repo health when its Claude account was cancelled mid-command. It had found the CI failure and never reported it. THE OBSERVED FAILURE. tests/test_adr0157_demote_teardown.py:106 asserts elapsed < 1.5 and reported 1.92s on main at 2d1c89e6, CI run 32580332076, leg test (ubuntu-latest, py3.14). That commit was docs-only, so nothing in the tree under test moved the number. WHAT MAKES IT MORE THAN A FLAKE, and it is measured rather than argued. Four implementations, 200 sources at 0.4s under a 3.0s budget: shipped, max-shaped 0.40s 200/200 finished timing PASS MUTANT sequential 3.01s 7/200 finished timing FAIL MUTANT semaphore(8) 3.00s 56/200 finished timing FAIL MUTANT semaphore(64) 1.63s 200/200 finished timing FAIL The semaphore(64) row is the item. Every source finishes, so all(stop_finished) passes and _pending_source_stops is empty -- the wall-clock bound is the ONLY assertion that catches it. So the threshold cannot simply be raised: the discriminating band is (0.40, 1.63) and the bound sits at 1.50. CI measured 1.92s for the SHIPPED implementation, which is above the 1.63s the mutant produces. On that run the correct code was slower than the defect the assertion exists to detect, so no threshold in the band could have separated them. That is the same class as #1290 -- an instrument that goes silent on its own subject exactly when it is under stress. LOCAL CONTROL. Seven consecutive runs on Windows while the full suite ran concurrently: 0.40 0.41 0.40 0.41 0.41 0.41 0.41. The floor is the 0.4s sleep plus ~10ms, so CI's 1.92s is runner latency, not the tree. The item records the fix direction (_Source already carries stop_started, and concurrency width separates all four rows with no timing dependence) and states plainly what is NOT established: this is at least one observed failure, with no rate measured and no same-head re-run recorded. Number allocated through scripts/coord/alloc.ps1. Backlog parser clean at 567 items, each declaring exactly one status. --- docs/BACKLOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 703dc6e6..e2aee960 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -12707,3 +12707,25 @@ which both readings of the cell now require. **Cluster:** Security record integrity / evidence hygiene. **Priority:** P3. **Verdict:** build. **Severity:** no engine-runtime effect and no deployment axis (sec. 0). This is evidence quality in the security record: more than half its citation surface has never been read by any gate, so a reader cannot distinguish a citation that still points at its subject from one that merely points at a line that still exists. + +## 1319. the demote-teardown source-phase timing assertion is wall-clock and fails on a loaded ubuntu runner + +> 🔢 **Filed 2026-08-22, recovered from a session cut off mid-measurement when its account was cancelled.** Value **5/10** -- Difficulty **2/10** -- _quick win_. [`tests/test_adr0157_demote_teardown.py:106`](../tests/test_adr0157_demote_teardown.py) asserts `elapsed < 1.5` on a wall clock. It failed on `main` at `2d1c89e6` -- CI run `32580332076`, leg `test (ubuntu-latest, py3.14)` -- reporting `source phase took 1.92s`. **That commit was docs-only (#515), so nothing in the tree under test could have moved the number.** **THE CHANGE: assert the CONCURRENCY WIDTH the test is actually about, not the elapsed time it currently infers it from.** +> **THE THRESHOLD IS NOT MERELY TIGHT -- IT SITS 0.13s BELOW THE MUTATION IT EXISTS TO CATCH.** Four implementations, 200 sources at 0.4s under a 3.0s budget, measured rather than reasoned about: +> +> | implementation | elapsed | finished | started at 50ms | `elapsed < 1.5` | `all(stop_finished)` | +> | --- | --- | --- | --- | --- | --- | +> | shipped, max-shaped | 0.40s | 200/200 | 200/200 | PASS | PASS | +> | MUTANT sequential loop | 3.01s | 7/200 | 1/200 | FAIL | FAIL | +> | MUTANT semaphore(8) | 3.00s | 56/200 | 8/200 | FAIL | FAIL | +> | MUTANT semaphore(64) | **1.63s** | 200/200 | 64/200 | FAIL | **PASS** | +> +> **THE SEMAPHORE(64) ROW IS THE ITEM.** Every source finishes, so `all(stop_finished)` passes and `_pending_source_stops` is empty. The wall-clock bound is the ONLY assertion that catches it -- which is why this cannot be fixed by raising the threshold. The discriminating band is `(0.40, 1.63)` and the bound sits at `1.50`. +> **SO ON THE FAILING RUN THE TEST WAS NOT MERELY RED, IT WAS INCAPABLE.** CI measured **1.92s for the shipped implementation** -- above the 1.63s the semaphore(64) mutant produces. For the duration of that load the correct code was SLOWER than the defect the assertion is written to detect, so no threshold in the band could have separated them. A flake that reddens is a nuisance; an instrument that cannot discriminate while it is loaded is the same class as #1290's walk failing before its regression assertion runs. +> **LOCAL CONTROL, and it isolates the cause to the runner rather than the tree.** Seven consecutive runs on a Windows box **while the full suite ran concurrently**: `0.40 0.41 0.40 0.41 0.41 0.41 0.41`. So the floor is the 0.4s sleep plus roughly 10ms of scheduling, and CI's 1.92s is about 1.5s of runner latency spread across 200 tasks -- a 4.7x inflation of the overhead term, not a property of the code. +> **THE FIX IS ALREADY AVAILABLE IN THE TEST DOUBLE AND NEEDS NO NEW TIMING.** `_Source` records `stop_started`. Sampling how many sources have started before any completes separates all four rows above -- 200 / 1 / 8 / 64 -- with no dependence on how fast the runner is. It is also the stronger assertion on its own terms: `_stop_sources_demote` creates every task eagerly before its first `await`, and [`wiring_runner.py:2645`](../messagefoundry/pipeline/wiring_runner.py) says so in its own docstring (*"Tasks are created eagerly, outside any gate"*), so *all N started* is a property of the shipped design rather than of the box. Keep a generous wall-clock bound underneath if a second signal is wanted, but it must stop being the discriminator. +> **WHAT IS NOT ESTABLISHED, stated so nobody reads more into it.** This is **at least one** observed failure. The rate under runner load was never measured, no re-run of the same head was recorded before the session ended, and no cross-branch pair like #1290's was collected. The mutation table above is the strong evidence here; the frequency is not. +> **DO NOT MERGE THIS INTO #1290 -- same cluster, different failure.** #1290 is a process-table walk on hosted **Windows** that never completes, so its regression assertion never runs. This is a **ubuntu** bound that a fully completed, correct run exceeds. Neither fix touches the other's file. + +**Cluster:** CI reliability / teardown tests. **Priority:** P2. **Verdict:** build. +**Severity:** no deployment axis (sec. 0) -- `tests/` ships in no engine path. The cost is that a required context can redden on `main` from runner load alone, and that while it is loaded the test cannot perform the discrimination it was written for. From 3fad32eeb3f9e3c9f390e110ab9990aad9966a17 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 22 Aug 2026 12:23:45 -0500 Subject: [PATCH 2/4] fix(tls): contexts the engine built but never named inherited an unasserted suite list (BACKLOG #1317) Closes the first-party half of the residual #1317 recorded as open. That item covered the operator-facing knob and the shipped context builder; it did not enumerate every SSLContext the engine constructs. RECOVERED, NOT REDONE. This was built by a workflow whose parent session's Claude account was cancelled on 2026-08-22 at 16:56Z while its full-suite run was in flight. The tree was left uncommitted in a locked worktree, three commits behind. It is replayed onto current main -- which had moved two commits, including #1317's own build -- and re-verified there. The claim on #1317 was taken over from that dead seat via claim.ps1 -Force, recorded in claims/.history. THE DEFECT IS INHERITANCE WITHOUT ASSERTION, not a missing context. Every hop below always had a TLS context; the engine simply never held a reference to it, so harden_cipher_suites never ran on it. Sites now asserted: auth/oidc_http.py identity provider token + JWKS hop logging_setup.py syslog TLS forwarder, BOTH arms incl tls_verify=False store/postgres.py pinned-CA and verification-disabled arms transports/soap.py mutual-TLS opener transports/rest.py shared REST/FHIR/DICOMweb opener family transports/fhir.py digest-auth rebuilt opener pipeline/alert_sinks.py webhook opener IT ASSERTS ON URLLIB'S OWN CONTEXT RATHER THAN SUBSTITUTING ONE, and that is load-bearing. Measured on CPython 3.14.6 / OpenSSL 3.5.7: urllib's context adds set_alpn_protocols(["http/1.1"]) and post_handshake_auth=True, which a hand-built ssl.create_default_context() has neither of. Passing a look-alike would silently drop ALPN and TLS 1.3 post-handshake auth from every default HTTP-family hop -- a handshake change on a control whose whole point is to change nothing about the connection. build_asserted_https_handler reads the handler's private _context and FAILS CLOSED if absent, rather than shrugging and reporting success forever. TWO FIRST-PARTY SITES ARE DELIBERATELY NOT ASSERTED, each with its reason in the code so a later reader does not "fix" them. The TLS floor probe builds ALL:@SECLEVEL=0 on purpose -- asserting there would empty the offer and turn a probe that can fail into one that cannot. And the Postgres ssl=True default arm hands asyncpg the job, so no engine object exists to assert on; grading a look-alike would grade an object the connection never uses. THE INSTRUMENT THAT GUARDED THIS RESIDUAL COULD NOT SEE IT, which is why the new test file is the other half rather than more of the same. test_tls_policy.py derives its call-site list from the presence of harden_kex_groups( in a file and its guard is `if kex and assertions < len(kex)`, so a file with ZERO kex-pin sites is skipped entirely. Every site above called neither helper, so the scan passed over all of them in silence. test_tls_cipher_assertion_sites.py names each construction and proves the assertion is REACHED, not merely present: contexts that are built are checked by patching _is_forward_secret to report every suite weak and requiring a ValueError naming that site's connector; contexts handed through an opener are checked by requiring the context the opener will actually use to be the SAME OBJECT the assertion ran on. It carries its own positive control, so a test seeing no raise reports a missing call rather than an inert instrument. VERIFICATION, stated exactly. ruff check, ruff format --check and mypy --strict all clean. 141 passed across test_tls_cipher_assertion_sites, test_tls_policy and test_soap_wssecurity -- run against current main, so the work is confirmed compatible with #1317's newer three-property harden_cipher_suites (forward secrecy plus encryption plus peer authentication), which landed after it was written. THE FULL SUITE IS STILL RUNNING and its result is NOT in this message; it is committed now rather than held because the pool that killed the first attempt is running hot again, and a branch commit is recoverable where an uncommitted worktree is not. CI is the authority. --- docs/BACKLOG.md | 25 +- messagefoundry/auth/oidc_http.py | 5 + messagefoundry/config/tls_policy.py | 41 ++ messagefoundry/config/tls_probe.py | 5 + messagefoundry/logging_setup.py | 5 + messagefoundry/pipeline/alert_sinks.py | 27 +- messagefoundry/store/postgres.py | 13 +- messagefoundry/transports/fhir.py | 6 +- messagefoundry/transports/rest.py | 40 +- messagefoundry/transports/soap.py | 11 +- tests/test_soap_wssecurity.py | 13 +- tests/test_tls_cipher_assertion_sites.py | 505 +++++++++++++++++++++++ 12 files changed, 680 insertions(+), 16 deletions(-) create mode 100644 tests/test_tls_cipher_assertion_sites.py diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index e2aee960..5b17e54d 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -12684,9 +12684,28 @@ rather than configuring one of ours; refusing an unlisted-but-sound suite there anything, it would stop an operator describing their proxy accurately. The three property checks still bind on that field. -**The residual above is unchanged and still open:** this covered the operator-facing knob and the -shipped context builder. It did not enumerate every `SSLContext` construction, and it does not reach -suite sets chosen inside `ldap3`, `hvac` or ODBC Driver 18. One instrument note worth carrying: the +**The residual above is NOW CLOSED for first-party constructions, 2026-08-22.** It read: this covered +the operator-facing knob and the shipped context builder, did not enumerate every `SSLContext` +construction, and does not reach suite sets chosen inside `ldap3`, `hvac` or ODBC Driver 18. **The +enumeration half is now done and every first-party site asserts** -- the OIDC identity-provider +opener, the syslog TLS forwarder (both arms, including `tls_verify=False`), the Postgres store +(pinned-CA and verification-disabled arms), the SOAP mutual-TLS opener, the alert webhook, and the +shared REST/FHIR/DICOMweb opener family. **The library half is unchanged and still open:** `ldap3`, +`hvac` and ODBC Driver 18 choose their own suites and no engine object exists to assert on. +**TWO FIRST-PARTY SITES ARE DELIBERATELY NOT ASSERTED, each with its reason in the code**, so a later +reader does not "fix" them: the TLS floor probe below, and the Postgres `ssl=True` default arm, where +asyncpg builds the context and asserting a look-alike here would grade an object the connection never +uses. Closing that arm changes what asyncpg receives on the DEFAULT store path and is a separate +decision, not a rider. +**THE INSTRUMENT THAT GUARDED THIS RESIDUAL COULD NOT SEE IT.** `tests/test_tls_policy.py` derives its +call-site list from the presence of `harden_kex_groups(` in a file, so it can only find a HALF-hardened +site -- one that pins key-exchange groups but skips the cipher assertion. Every site listed above called +neither helper, so the scan passed over all of them in silence. `tests/test_tls_cipher_assertion_sites.py` +is the other half: one test per site, each proving the call is REACHED rather than merely present. +**THE WORK WAS RECOVERED, NOT REDONE.** It was built by a workflow whose parent session's Claude account +was cancelled on 2026-08-22 at 16:56Z while its full-suite run was in flight; the tree was stranded +uncommitted in a locked worktree. It has been replayed onto current `main` (which had moved two commits, +including this item's own build) and re-verified there. One instrument note worth carrying: the startup TLS probe's `ALL:@SECLEVEL=0` context (140 suites, 12 anonymous) was examined and DELIBERATELY LEFT ALONE -- the security level is load-bearing there, because without it the probe would measure the engine's own refusal to offer rather than a peer's refusal to accept, turning a diff --git a/messagefoundry/auth/oidc_http.py b/messagefoundry/auth/oidc_http.py index 4fbfc8f0..c36b4fa1 100644 --- a/messagefoundry/auth/oidc_http.py +++ b/messagefoundry/auth/oidc_http.py @@ -41,6 +41,7 @@ from messagefoundry.auth.oidc.jwks import _MAX_JWKS_BYTES from messagefoundry.auth.trust_anchors import AnchorSpec, enforce_anchor +from messagefoundry.config.tls_policy import harden_cipher_suites __all__ = ["build_idp_opener", "jwks_fetcher"] @@ -101,6 +102,10 @@ def build_idp_opener( ) ctx.check_hostname = True ctx.verify_mode = ssl.CERT_REQUIRED + # Assert forward secrecy on the FINAL context (ASVS 12.1.2): this hop carries the client secret, + # the authorization code and the identity assertion, so a recorded session that a future key + # compromise could decrypt is an authentication-material exposure, not just a confidentiality one. + harden_cipher_suites(ctx, connector="OIDC identity provider (token + JWKS)") return urllib.request.build_opener(_NoRedirectHandler, urllib.request.HTTPSHandler(context=ctx)) diff --git a/messagefoundry/config/tls_policy.py b/messagefoundry/config/tls_policy.py index 368ab944..62351530 100644 --- a/messagefoundry/config/tls_policy.py +++ b/messagefoundry/config/tls_policy.py @@ -34,6 +34,7 @@ import logging import os import ssl +import urllib.request from collections.abc import Callable, Iterator, Mapping from contextlib import contextmanager from contextvars import ContextVar @@ -63,6 +64,7 @@ "TrustAnchorPolicy", "active_hop_posture", "APPROVED_SMTP_AUTH_MECHANISMS", + "build_asserted_https_handler", "build_smtp_tls_context", "smtp_login_approved", "build_verifying_client_context", @@ -515,6 +517,45 @@ def _is_peer_authenticated(cipher: Mapping[str, object]) -> bool: return "Au=None" not in str(cipher.get("description", "")) +def build_asserted_https_handler(*, connector: str) -> urllib.request.HTTPSHandler: + """urllib's OWN default https handler, with the context it built ASSERTED forward-secret. + + For the openers that name no ``HTTPSHandler`` at all. ``urllib.request.build_opener(...)`` fills + one in from its default class list, and that handler builds a context in its constructor + (``http.client._create_https_context``). So a context always existed on those hops — the ENGINE + just never held a reference to it, and :func:`harden_cipher_suites` therefore never ran on it. + That, not the absence of a context, is the residual: inheritance without assertion. + + **It asserts on urllib's context rather than substituting one, and that is load-bearing.** + Measured on CPython 3.14.6 / OpenSSL 3.5.7: ``ssl.create_default_context()`` is NOT equivalent to + what urllib builds. urllib's adds ``set_alpn_protocols(["http/1.1"])`` and + ``post_handshake_auth=True``; a hand-built look-alike has neither. Passing such a look-alike as + ``context=`` would silently drop the ALPN advertisement and TLS 1.3 post-handshake auth from every + default HTTP-family hop — a handshake change, on a control whose whole point is to change nothing + about the connection. Handing ``build_opener`` this handler is inert by construction: it is the + same class ``build_opener`` would have instantiated itself, built the same way, and supplying an + instance only stops urllib adding a second one. + + Reads the handler's private ``_context`` deliberately, and **fails closed** if it is not there. A + ``getattr(..., None)`` that shrugged and returned would be a security control reporting success + forever — exactly the failure :func:`harden_kex_groups` documents. A CPython that renames the + attribute must break loudly at construction, not go quiet. + + ``connector`` is the operator-recognisable label :func:`harden_cipher_suites` names in its error. + Lives here rather than beside each opener so the two call sites (the HTTP-family destinations and + the alert webhook) cannot drift onto different constructions.""" + handler = urllib.request.HTTPSHandler() + ctx = getattr(handler, "_context", None) + if not isinstance(ctx, ssl.SSLContext): + raise ValueError( + f"{connector}: cannot reach the TLS context urllib's HTTPSHandler built " + f"(no `_context` attribute on this runtime), so the forward-secrecy assertion " + f"(ASVS 12.1.2) cannot run on this hop. Refusing rather than crossing unchecked." + ) + harden_cipher_suites(ctx, connector=connector) + return handler + + def _is_forward_secret(cipher: Mapping[str, object]) -> bool: """Whether a ``SSLContext.get_ciphers()`` entry uses an (EC)DHE — forward-secret — key exchange.""" name = str(cipher.get("name", "")) diff --git a/messagefoundry/config/tls_probe.py b/messagefoundry/config/tls_probe.py index 4ed41309..2be3a4ab 100644 --- a/messagefoundry/config/tls_probe.py +++ b/messagefoundry/config/tls_probe.py @@ -104,6 +104,11 @@ def _offer_context(version: ssl.TLSVersion | None) -> ssl.SSLContext: # TLS 1.0 ClientHello at the default security level, so without this the probe would measure # OUR refusal to ask rather than THEIR refusal to answer — a false pass. ctx.set_ciphers("ALL:@SECLEVEL=0") + # DELIBERATELY NOT hardened: harden_cipher_suites would RAISE here, and that is the point. The + # ALL:@SECLEVEL=0 offer above resolves to a wide suite list including non-forward-secret suites, + # by design, because the probe must be able to ASK. Asserting forward secrecy would empty the + # offer and turn a floor probe that can fail into one that cannot. This context carries no + # application data and never leaves this module. Do not "fix" it. return ctx diff --git a/messagefoundry/logging_setup.py b/messagefoundry/logging_setup.py index 6f4bf20b..1bbea673 100644 --- a/messagefoundry/logging_setup.py +++ b/messagefoundry/logging_setup.py @@ -31,6 +31,7 @@ from dataclasses import dataclass from typing import Any +from messagefoundry.config.tls_policy import harden_cipher_suites from messagefoundry.redaction import redact __all__ = [ @@ -314,6 +315,10 @@ def _build_tls_context(forward: SyslogForward) -> ssl.SSLContext: if forward.tls_client_cert is not None: # Mutual TLS: a single PEM carrying both the client cert and its key (keyfile defaults to it). ctx.load_cert_chain(certfile=forward.tls_client_cert) + # Assert forward secrecy LAST, so it sees the final suite list (ASVS 12.1.2). This runs on the + # tls_verify=False arm too: that opt-out drops peer AUTHENTICATION, and the log records still cross + # the network encrypted, so the suite list still decides whether a recorded session stays private. + harden_cipher_suites(ctx, connector="syslog TLS forwarder") return ctx diff --git a/messagefoundry/pipeline/alert_sinks.py b/messagefoundry/pipeline/alert_sinks.py index c54bd391..1f667f66 100644 --- a/messagefoundry/pipeline/alert_sinks.py +++ b/messagefoundry/pipeline/alert_sinks.py @@ -48,6 +48,7 @@ from messagefoundry.config.tls_policy import ( HopPosture, TrustAnchorPolicy, + build_asserted_https_handler, build_smtp_tls_context, smtp_login_approved, ) @@ -274,8 +275,30 @@ def redirect_request( return None -# A shared opener that never follows redirects; reused for every webhook POST. -_NO_REDIRECT_OPENER = urllib.request.build_opener(_NoRedirectHandler) +def _build_no_redirect_opener() -> urllib.request.OpenerDirector: + """Build the shared webhook opener: verifying https, never follows a redirect. + + The ``HTTPSHandler`` is NAMED rather than left for ``build_opener`` to fill in. urllib's default + handler builds its own TLS context, which the engine never held a reference to — so this hop's + suite list was inherited and unchecked. :func:`~messagefoundry.config.tls_policy. + build_asserted_https_handler` returns that same default handler with its context asserted forward- + secret (ASVS 12.1.2). It substitutes nothing: see that function for why replacing the context + would have changed the handshake. Handler-for-handler identical to the previous + ``build_opener(_NoRedirectHandler)``. + + A named function, not an inline module-level expression, so a test can call the exact construction + the shared opener is built from instead of reloading this module. + + Built through the ``tls_policy`` factory so this module still never names ``ssl`` itself, matching + the plain-data TLS settings :class:`EmailTransport` carries for the same reason.""" + return urllib.request.build_opener( + _NoRedirectHandler, + build_asserted_https_handler(connector="alert webhook destination"), + ) + + +# The shared opener, reused for every webhook POST. +_NO_REDIRECT_OPENER = _build_no_redirect_opener() class WebhookTransport: diff --git a/messagefoundry/store/postgres.py b/messagefoundry/store/postgres.py index b4dca110..509d3c0f 100644 --- a/messagefoundry/store/postgres.py +++ b/messagefoundry/store/postgres.py @@ -72,6 +72,7 @@ from messagefoundry.config.tls_policy import ( HopDisposition, HopPosture, + harden_cipher_suites, is_loopback_hop_host, revocation_hop_disposition, tls_revocation_attested, @@ -728,6 +729,9 @@ def _build_ssl(settings: StoreSettings, *, posture: HopPosture | None = None) -> ctx = _ssl.create_default_context() ctx.check_hostname = False ctx.verify_mode = _ssl.CERT_NONE + # Verification is off but the store hop is still encrypted, so the suite list still decides + # whether recorded PHI traffic survives a future key compromise (ASVS 12.1.2). + harden_cipher_suites(ctx, connector="Postgres store (TLS verification disabled)") return ctx # #201 (ADR 0078 amendment): the engine->store hop below VERIFIES the peer cert (a pinned CA or the # system trust store) but asyncpg rides stdlib ssl, which does NO OCSP/CRL revocation — a @@ -745,7 +749,14 @@ def _build_ssl(settings: StoreSettings, *, posture: HopPosture | None = None) -> # Pin a private / self-signed CA WITHOUT touching the OS trust store: verify the server cert # (+ hostname) against this PEM bundle. create_default_context() already sets CERT_REQUIRED + # check_hostname=True, so this stays a fully-verifying posture (a bad path raises at connect). - return _ssl.create_default_context(cafile=settings.ssl_root_cert) + ctx = _ssl.create_default_context(cafile=settings.ssl_root_cert) + harden_cipher_suites(ctx, connector="Postgres store (pinned CA)") + return ctx + # A RESIDUAL, stated rather than papered over: `True` hands asyncpg the job of building the + # context, so no context exists in engine code for harden_cipher_suites to assert on. Asserting a + # look-alike built here would grade an object the connection never uses. Closing it means building + # the verifying default context here and returning it instead, which changes what asyncpg receives + # on the DEFAULT store path — a separate decision, not a rider on this change. return True # verifying TLS against the system trust store (the secure default) diff --git a/messagefoundry/transports/fhir.py b/messagefoundry/transports/fhir.py index ac9b0cdd..8e436528 100644 --- a/messagefoundry/transports/fhir.py +++ b/messagefoundry/transports/fhir.py @@ -68,7 +68,6 @@ _expiry_relaxed_opener, _insecure_opener, _no_redirect_opener, - _NoRedirectHandler, _redact_url, capture_response_headers, cleartext_acceptance_from_settings, @@ -409,7 +408,10 @@ def __init__(self, config: Destination) -> None: "(mutually exclusive — configure exactly one)" ) if self._opener is _NO_REDIRECT_OPENER: - self._opener = urllib.request.build_opener(_NoRedirectHandler) + # _no_redirect_opener, not a bare build_opener: the bare form lets urllib fill in an + # HTTPSHandler whose context the engine never names, so the forward-secrecy assertion + # cannot reach this hop (ASVS 12.1.2). + self._opener = _no_redirect_opener() self._opener.add_handler(digest) def _build_headers(self, s: dict[str, Any]) -> dict[str, str]: diff --git a/messagefoundry/transports/rest.py b/messagefoundry/transports/rest.py index dd0b365d..7f9ef4e9 100644 --- a/messagefoundry/transports/rest.py +++ b/messagefoundry/transports/rest.py @@ -47,9 +47,11 @@ HopPosture, InsecureHopRefused, RevocationHopGuard, + build_asserted_https_handler, cleartext_acceptance_audit_sink, current_hop_posture, enforce_insecure_hop, + harden_cipher_suites, insecure_hop_disposition, is_loopback_hop_host, relax_verify_expiry, @@ -226,8 +228,21 @@ def redirect_request( return None -# Shared opener that verifies TLS (urllib's default context) and never follows redirects. -_NO_REDIRECT_OPENER = urllib.request.build_opener(_NoRedirectHandler) +def _asserted_https_handler() -> urllib.request.HTTPSHandler: + """urllib's own default https handler for this connector family, with its context ASSERTED. + + ``build_opener(_NoRedirectHandler)`` alone leaves urllib to fill in the ``HTTPSHandler``, and the + context that handler builds is one the engine never names — so the default REST / FHIR / + DICOMweb / ``fhir_lookup`` egress path, which every HTTP-family destination falls back to unless + it needs a proxy, an escape or an expiry relaxation, had an inherited suite list that nothing + checked. Naming the handler here lets + :func:`~messagefoundry.config.tls_policy.harden_cipher_suites` run on the context it carries. + + It does NOT substitute a context: urllib's own is asserted in place. See + :func:`~messagefoundry.config.tls_policy.build_asserted_https_handler` for the measurement that + forced that choice — a hand-built ``ssl.create_default_context()`` differs from urllib's on ALPN + and post-handshake auth, so passing one would have changed the handshake.""" + return build_asserted_https_handler(connector="HTTP-family destination (REST/FHIR/DICOMweb)") def _no_redirect_opener( @@ -239,7 +254,17 @@ def _no_redirect_opener( opener is **never** mutated (ADR 0126). Passing a ``ProxyHandler`` here also suppresses urllib's default env-reading ProxyHandler (``build_opener`` skips a default whose class a supplied handler already covers), so there is never a competing double-proxy.""" - return urllib.request.build_opener(_NoRedirectHandler, *extra_handlers) + return urllib.request.build_opener( + _NoRedirectHandler, _asserted_https_handler(), *extra_handlers + ) + + +# Shared opener that verifies TLS and never follows redirects, reused by every connection that needs +# no extra handler. Built through _no_redirect_opener with no extras rather than repeating the +# expression, so there is exactly ONE construction of this opener to keep asserted. Handler-for-handler +# identical to the previous `build_opener(_NoRedirectHandler)`: build_opener would have added an +# HTTPSHandler of its own, and this supplies the same class built the same way. +_NO_REDIRECT_OPENER = _no_redirect_opener() def _insecure_opener( @@ -250,6 +275,9 @@ def _insecure_opener( ctx = ssl.create_default_context() ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE + # Verification is off, but the traffic is still encrypted, so the suite list still matters: assert + # forward secrecy here too (ASVS 12.1.2), after the verify-off configuration is applied. + harden_cipher_suites(ctx, connector="HTTP-family destination (TLS verification disabled)") return urllib.request.build_opener( _NoRedirectHandler, urllib.request.HTTPSHandler(context=ctx), *extra_handlers ) @@ -267,6 +295,7 @@ def _expiry_relaxed_opener( still rejected. Shared verbatim by the SOAP destination.""" ctx = ssl.create_default_context() relax_verify_expiry(ctx, host=host) # chain + hostname stay enforced; only expiry is relaxed + harden_cipher_suites(ctx, connector="HTTP-family destination (expired-certificate tolerance)") return urllib.request.build_opener( _NoRedirectHandler, urllib.request.HTTPSHandler(context=ctx), *extra_handlers ) @@ -1366,7 +1395,10 @@ def __init__(self, config: Destination) -> None: ) if self._opener is _NO_REDIRECT_OPENER: # Rebuild a per-connection verifying opener so add_handler never touches the shared one. - self._opener = urllib.request.build_opener(_NoRedirectHandler) + # Through _no_redirect_opener, not a bare build_opener: the bare form lets urllib + # fill in an HTTPSHandler whose context the engine never names, so the forward-secrecy + # assertion cannot reach this hop (ASVS 12.1.2). + self._opener = _no_redirect_opener() self._opener.add_handler(digest) if self._ech_sidecar is not None: # ECH mode (ADR 0139): the engine->sidecar hop is cleartext http over loopback (the sidecar diff --git a/messagefoundry/transports/soap.py b/messagefoundry/transports/soap.py index e1864757..9ffede72 100644 --- a/messagefoundry/transports/soap.py +++ b/messagefoundry/transports/soap.py @@ -65,7 +65,7 @@ from xml.sax.xmlreader import InputSource # nosec B406 — fed only the hardened, no-DTD parser from messagefoundry.config.models import ConnectorType, Destination -from messagefoundry.config.tls_policy import relax_verify_expiry +from messagefoundry.config.tls_policy import harden_cipher_suites, relax_verify_expiry from messagefoundry.transports.base import ( DeliveryError, DeliveryResponse, @@ -206,6 +206,10 @@ def _client_cert_opener( relax_verify_expiry( ctx, host=host ) # server chain + hostname still verified; expiry relaxed + # Assert forward secrecy LAST, so it sees the final suite list (ASVS 12.1.2). The docstring above + # already claimed parity with mllp.py / api/tls.py on the TLS floor; this makes the claim true of + # the cipher assertion those siblings also carry. + harden_cipher_suites(ctx, connector="SOAP destination (mutual TLS)") return urllib.request.build_opener( _NoRedirectHandler, urllib.request.HTTPSHandler(context=ctx), *extra_handlers ) @@ -466,7 +470,10 @@ def __init__(self, config: Destination) -> None: "(mutually exclusive — configure exactly one)" ) if self._opener is _NO_REDIRECT_OPENER: - self._opener = urllib.request.build_opener(_NoRedirectHandler) + # _no_redirect_opener, not a bare build_opener: the bare form lets urllib fill in an + # HTTPSHandler whose context the engine never names, so the forward-secrecy assertion + # cannot reach this hop (ASVS 12.1.2). + self._opener = _no_redirect_opener() self._opener.add_handler(digest) # ADR 0015 amendment (#236): body-secret substitution. Parsed last so the credential validation diff --git a/tests/test_soap_wssecurity.py b/tests/test_soap_wssecurity.py index 9d2557a4..3b59ab96 100644 --- a/tests/test_soap_wssecurity.py +++ b/tests/test_soap_wssecurity.py @@ -13,6 +13,7 @@ import base64 import hashlib +import ssl import urllib.request from pathlib import Path @@ -69,6 +70,13 @@ def open(self, req: urllib.request.Request, timeout: float | None = None) -> _Re # --- mutual-TLS opener ------------------------------------------------------- +#: The interpreter's real default suite list, captured at import — BEFORE any test monkeypatches +#: ``ssl.create_default_context``. ``_FakeCtx`` hands this back so the forward-secrecy assertion the +#: opener now runs (``harden_cipher_suites``, ASVS 12.1.2) sees a realistic list rather than a stub +#: that cannot answer. Capturing it lazily inside the fake would return the fake itself. +_REAL_DEFAULT_SUITES = ssl.create_default_context().get_ciphers() + + class _FakeCtx: def __init__(self) -> None: self.minimum_version: object = None @@ -79,10 +87,11 @@ def load_cert_chain( ) -> None: self.cert_args = (certfile, keyfile, password) + def get_ciphers(self) -> list[dict[str, object]]: + return _REAL_DEFAULT_SUITES -def test_client_cert_opener_loads_chain_and_floors_tls(monkeypatch: pytest.MonkeyPatch) -> None: - import ssl +def test_client_cert_opener_loads_chain_and_floors_tls(monkeypatch: pytest.MonkeyPatch) -> None: fake = _FakeCtx() monkeypatch.setattr(soap_mod.ssl, "create_default_context", lambda: fake) opener = _client_cert_opener("client.pem", "key.pem", "pw") diff --git a/tests/test_tls_cipher_assertion_sites.py b/tests/test_tls_cipher_assertion_sites.py new file mode 100644 index 00000000..c1692610 --- /dev/null +++ b/tests/test_tls_cipher_assertion_sites.py @@ -0,0 +1,505 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Call-site coverage for the forward-secrecy assertion (ASVS 12.1.2), one test per hardened site. + +``tests/test_tls_policy.py`` covers the FUNCTION and derives its call-site list from the presence of +``harden_kex_groups(`` in a file. That predicate can only find a HALF-hardened site: a context that +pins key-exchange groups but skips the cipher assertion. Every site hardened here calls neither +helper today, so that scan passes over all of them in silence — the instrument that guarded the +residual could not detect the residual. This file is the other half: it names each construction and +proves the assertion is reached inside it. + +**How these tests prove the call is REACHED, not merely present.** Two instruments, because the +sites come in two shapes: + +* Sites that BUILD a context (``every_suite_looks_weak``). The fixture patches + ``tls_policy._is_forward_secret`` to report every suite non-forward-secret, then the test builds + the site's context exactly as the engine does and requires a ``ValueError`` naming that site's + connector label. A decoy call cannot satisfy it: the raise can only come from + ``harden_cipher_suites`` running against the real context the site returns. +* Sites that hand urllib's own context through an opener (``asserted_contexts``). Presence of a + context proves nothing there — see that fixture — so those tests require the context the opener + will actually use to be the SAME OBJECT the assertion ran on. + +Delete the call from any one site and that site's test goes red while the rest stay green, verified +by mutation one site at a time. ``every_suite_looks_weak`` is a POSITIVE CONTROL in its own right: +:func:`test_the_patch_makes_a_shipped_context_raise` asserts a plain default context raises under it, +so a test that saw no raise would be reporting a missing call rather than an inert instrument. +""" + +from __future__ import annotations + +import datetime +import ssl +import urllib.request +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import pytest +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID + +from messagefoundry import logging_setup +from messagefoundry.auth import oidc_http +from messagefoundry.config import tls_policy, tls_probe +from messagefoundry.config.models import ConnectorType, Destination +from messagefoundry.config.settings import INSECURE_TLS_ESCAPE_ENV, StoreBackend, StoreSettings +from messagefoundry.config.wiring import FHIR, Rest, Soap +from messagefoundry.pipeline import alert_sinks +from messagefoundry.store import postgres +from messagefoundry.transports import build_destination, rest, soap +from messagefoundry.transports.http_auth import with_http_digest + +# Imported at module scope ON PURPOSE. `rest` and `alert_sinks` build their shared opener AT IMPORT, +# so a first import inside the every_suite_looks_weak fixture would raise during module execution and +# fail the test for the wrong reason. Importing here puts them in sys.modules before any patch runs, +# which also means the assertions below exercise the same module objects the engine uses. + + +@pytest.fixture +def every_suite_looks_weak(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: + """Make the shipped assertion fire on ANY context, so reaching it is observable. + + The engine's real default suite list is entirely forward-secret on every supported runtime, so a + correctly-wired call site raises nothing and is indistinguishable from a missing one. Reporting + the whole list as weak inverts that: the call now raises wherever it runs, and only where it runs. + """ + monkeypatch.setattr(tls_policy, "_is_forward_secret", lambda cipher: False) + yield + + +def _self_signed(tmp_path: Path) -> tuple[Path, Path]: + """A self-signed EC cert + key PEM under ``tmp_path``; returns ``(cert_path, key_path)``.""" + key = ec.generate_private_key(ec.SECP256R1()) + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "localhost")]) + cert = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(datetime.datetime(2020, 1, 1, tzinfo=datetime.UTC)) + .not_valid_after(datetime.datetime(2040, 1, 1, tzinfo=datetime.UTC)) + .sign(key, hashes.SHA256()) + ) + cert_path, key_path = tmp_path / "cert.pem", tmp_path / "key.pem" + cert_path.write_bytes(cert.public_bytes(serialization.Encoding.PEM)) + key_path.write_bytes( + key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ) + ) + return cert_path, key_path + + +@pytest.fixture +def asserted_contexts(monkeypatch: pytest.MonkeyPatch) -> list[tuple[str, ssl.SSLContext]]: + """Record every ``(connector, context)`` pair the shipped assertion actually ran on. + + The second instrument in this file, and the one the opener tests need. "Does this opener hold an + SSLContext?" CANNOT fail on CPython 3.14: ``urllib.request.HTTPSHandler(context=None)`` builds a + context in its own constructor, so a handler the engine never touched still answers yes. Measured + the hard way, after a first version of these tests passed under every mutation. Identity is the + discriminating question: is the context this opener carries the SAME OBJECT the assertion ran on? + + Wraps rather than replaces ``harden_cipher_suites``, so the real check still runs. + """ + seen: list[tuple[str, ssl.SSLContext]] = [] + real = tls_policy.harden_cipher_suites + + def spy(ctx: ssl.SSLContext, *, connector: str) -> None: + seen.append((connector, ctx)) + real(ctx, connector=connector) + + monkeypatch.setattr(tls_policy, "harden_cipher_suites", spy) + return seen + + +def _opener_context(opener: urllib.request.OpenerDirector) -> ssl.SSLContext | None: + """The ``SSLContext`` ``opener``'s https handler will hand every connection it opens.""" + for handler in opener.handlers: + if hasattr(handler, "https_open"): + ctx = getattr(handler, "_context", None) + if isinstance(ctx, ssl.SSLContext): + return ctx + return None + + +def _assert_opener_context_was_checked( + opener: urllib.request.OpenerDirector, + recorded: list[tuple[str, ssl.SSLContext]], + *, + label: str, + site: str, +) -> None: + """Require that the context ``opener`` carries is one the assertion ran on, under ``label``.""" + ctx = _opener_context(opener) + assert ctx is not None, f"{site}: the opener's https handler carries no SSLContext at all" + matches = [lbl for lbl, seen in recorded if seen is ctx] + assert matches, ( + f"{site}: the context this opener will use was never passed to harden_cipher_suites. " + f"The assertion ran on {[lbl for lbl, _ in recorded]}, none of which is this object, so " + f"this hop's suite list is inherited and unchecked." + ) + assert label in matches[0], f"{site}: asserted under {matches[0]!r}, expected {label!r}" + + +_HTTPS = "https://partner.example.org/ingest" + + +def _spec_for(connector_type: ConnectorType) -> Any: + """The wiring spec for one HTTP-family connector, so the digest test covers all three.""" + return { + ConnectorType.REST: lambda: Rest(url=_HTTPS), + ConnectorType.FHIR: lambda: FHIR(url=_HTTPS), + ConnectorType.SOAP: lambda: Soap(url=_HTTPS), + }[connector_type]() + + +def _pg_settings(**overrides: Any) -> StoreSettings: + """A minimally-valid Postgres ``[store]`` block, so the TLS arms are reachable at all.""" + return StoreSettings( + backend=StoreBackend.POSTGRES, + server="db.example.org", + database="mefor", + username="mefor", + **overrides, + ) + + +# --- the positive control ------------------------------------------------------------------------ + + +def test_the_patch_makes_a_shipped_context_raise(every_suite_looks_weak: None) -> None: + """Liveness receipt for every test below: under the patch, a plain default context RAISES. + + Without this, a site test that saw no raise would be ambiguous between 'the call is missing' and + 'the instrument is inert'. This is the run's non-zero reading. + """ + with pytest.raises(ValueError, match="non-forward-secret"): + tls_policy.harden_cipher_suites(ssl.create_default_context(), connector="control") + + +def test_without_the_patch_the_same_context_is_silent() -> None: + """The other half of the control: the shipped default really is all-forward-secret, so a raise in + any test below can only come from the patch, never from a genuinely weak shipped suite list.""" + tls_policy.harden_cipher_suites(ssl.create_default_context(), connector="control") + + +# --- HTTP-family egress: transports/rest.py ------------------------------------------------------- + + +def test_rest_shared_verifying_opener_asserts(every_suite_looks_weak: None) -> None: + """``_no_redirect_opener`` — the default REST / FHIR / DICOMweb / fhir_lookup egress path, and the + construction the module-level ``_NO_REDIRECT_OPENER`` is itself built from.""" + + with pytest.raises(ValueError, match="HTTP-family destination"): + rest._no_redirect_opener() + + +def test_rest_insecure_opener_asserts(every_suite_looks_weak: None) -> None: + """``_insecure_opener`` — the audited ``verify_tls=false`` escape. Verification is off but the hop + is still encrypted, so the suite list still decides whether recorded traffic stays private.""" + + with pytest.raises(ValueError, match="TLS verification disabled"): + rest._insecure_opener() + + +def test_rest_expiry_relaxed_opener_asserts(every_suite_looks_weak: None) -> None: + """``_expiry_relaxed_opener`` — the ``tls_allow_expired`` path, shared verbatim by SOAP.""" + + with pytest.raises(ValueError, match="expired-certificate tolerance"): + rest._expiry_relaxed_opener("partner.example.org") + + +def test_rest_shared_opener_context_is_the_one_that_was_asserted( + asserted_contexts: list[tuple[str, ssl.SSLContext]], +) -> None: + """The assertion ran on the object this opener will actually send through, not a look-alike. + + Worth stating precisely, because the loose version of this claim is false: a context always + existed here. urllib's default ``HTTPSHandler`` builds one in its own constructor. What was + missing was any engine reference to it, so nothing ever checked its suite list. Identity is + therefore the test, not presence.""" + + opener = rest._no_redirect_opener() + _assert_opener_context_was_checked( + opener, + asserted_contexts, + label="HTTP-family destination", + site="rest._no_redirect_opener", + ) + + +def test_the_rest_opener_handshake_is_unchanged_by_the_assertion() -> None: + """The assertion must change NOTHING about the connection, and this is what proves it. + + A first version of this change substituted a hand-built ``ssl.create_default_context()`` for + urllib's. Measured on CPython 3.14.6 / OpenSSL 3.5.7, those are NOT the same context: urllib's + carries ``post_handshake_auth=True`` and an ALPN ``http/1.1`` advertisement that a hand-built one + does not. That would have quietly altered every default HTTP-family handshake. The shipped code + asserts urllib's own context instead of replacing it; this pins that, on the one half of the + difference that is readable back (ALPN is write-only). + """ + engine = _opener_context(rest._NO_REDIRECT_OPENER) + stock = _opener_context(urllib.request.build_opener(rest._NoRedirectHandler)) + assert engine is not None and stock is not None + assert engine.post_handshake_auth == stock.post_handshake_auth, ( + "the engine's HTTP-family context no longer matches urllib's default on post-handshake auth " + "- the assertion has started substituting a context instead of checking urllib's" + ) + assert [c["name"] for c in engine.get_ciphers()] == [c["name"] for c in stock.get_ciphers()] + assert engine.verify_mode == stock.verify_mode + assert engine.check_hostname == stock.check_hostname + assert engine.minimum_version == stock.minimum_version + + +# --- the HTTP Digest rebuild branches: rest.py, fhir.py, soap.py ---------------------------------- +# +# NOT IN THE CLASSIFICATION, found while building. Each of the three HTTP-family connectors rebuilds a +# per-connection opener when HTTP Digest auth is configured, so `add_handler` never mutates the shared +# one. All three rebuilt it with a bare `build_opener(_NoRedirectHandler)`, which lets urllib fill in +# an HTTPSHandler the engine never names — so a digest-authenticated destination would have dropped +# straight back onto an unasserted context while its non-digest sibling was covered. Each now rebuilds +# through `_no_redirect_opener()`, the helper written for exactly this case. + + +@pytest.mark.parametrize( + ("connector_type", "name"), + [ + (ConnectorType.REST, "OB_REST"), + (ConnectorType.FHIR, "OB_FHIR"), + (ConnectorType.SOAP, "OB_SOAP"), + ], +) +def test_digest_rebuilt_opener_context_is_the_one_that_was_asserted( + connector_type: ConnectorType, + name: str, + asserted_contexts: list[tuple[str, ssl.SSLContext]], +) -> None: + """A digest-authenticated destination's rebuilt opener carries an asserted context too.""" + settings = with_http_digest(_spec_for(connector_type), user="u", password="p").settings + dest = build_destination(Destination(name=name, type=connector_type, settings=settings)) + opener = dest._opener # type: ignore[attr-defined] + assert any(isinstance(h, urllib.request.HTTPDigestAuthHandler) for h in opener.handlers), ( + "this destination did not take the digest rebuild branch, so the test proves nothing" + ) + _assert_opener_context_was_checked( + opener, + asserted_contexts, + label="HTTP-family destination", + site=f"{name} digest rebuild branch", + ) + + +# --- SOAP mutual TLS: transports/soap.py ---------------------------------------------------------- + + +def test_soap_client_cert_opener_asserts(every_suite_looks_weak: None, tmp_path: Path) -> None: + """``_client_cert_opener`` — the SOAP mTLS destination, asserted after the TLS floor and the + client chain are applied.""" + + cert, key = _self_signed(tmp_path) + with pytest.raises(ValueError, match="SOAP destination"): + soap._client_cert_opener(str(cert), str(key), None) + + +# --- alert webhooks: pipeline/alert_sinks.py ------------------------------------------------------ + + +def test_alert_webhook_opener_asserts(every_suite_looks_weak: None) -> None: + """``_build_no_redirect_opener`` — every outbound https webhook POST (Slack, Teams, PagerDuty, a + custom endpoint). A second, distinct opener of the same shape: fixing rest.py did not touch it.""" + + with pytest.raises(ValueError, match="alert webhook destination"): + alert_sinks._build_no_redirect_opener() + + +def test_alert_webhook_opener_context_is_the_one_that_was_asserted( + asserted_contexts: list[tuple[str, ssl.SSLContext]], +) -> None: + """The webhook opener carries the very context the assertion ran on, as the REST one does.""" + _assert_opener_context_was_checked( + alert_sinks._build_no_redirect_opener(), + asserted_contexts, + label="alert webhook destination", + site="alert_sinks._build_no_redirect_opener", + ) + + +# --- the OIDC identity-provider hop: auth/oidc_http.py -------------------------------------------- + + +def test_oidc_idp_opener_asserts_without_a_pinned_ca(every_suite_looks_weak: None) -> None: + """``build_idp_opener`` — the token-endpoint + JWKS hop, OS-trust-store arm.""" + + with pytest.raises(ValueError, match="OIDC identity provider"): + oidc_http.build_idp_opener(None) + + +def test_oidc_idp_opener_asserts_with_a_pinned_ca( + every_suite_looks_weak: None, tmp_path: Path +) -> None: + """The pinned-CA arm of the same builder — a second return path, so a second test.""" + + cert, _key = _self_signed(tmp_path) + with pytest.raises(ValueError, match="OIDC identity provider"): + oidc_http.build_idp_opener(str(cert)) + + +# --- off-box syslog: logging_setup.py ------------------------------------------------------------- + + +def test_syslog_tls_forwarder_asserts(every_suite_looks_weak: None, tmp_path: Path) -> None: + """``_build_tls_context`` — the RFC 5425 syslog-over-TLS forwarder to the SIEM.""" + + cert, _key = _self_signed(tmp_path) + forward = logging_setup.SyslogForward( + host="siem.example.org", port=6514, protocol="tls", tls_ca_file=str(cert) + ) + with pytest.raises(ValueError, match="syslog TLS forwarder"): + logging_setup._build_tls_context(forward) + + +def test_syslog_tls_forwarder_asserts_on_the_verify_off_arm( + every_suite_looks_weak: None, tmp_path: Path +) -> None: + """The documented ``tls_verify=false`` opt-out drops peer authentication, not encryption, so the + assertion must run there too — after the CERT_NONE downgrade, on the final context.""" + + cert, _key = _self_signed(tmp_path) + forward = logging_setup.SyslogForward( + host="siem.example.org", + port=6514, + protocol="tls", + tls_ca_file=str(cert), + tls_verify=False, + ) + with pytest.raises(ValueError, match="syslog TLS forwarder"): + logging_setup._build_tls_context(forward) + + +# --- the engine-to-store hop: store/postgres.py --------------------------------------------------- + + +def test_postgres_pinned_ca_context_asserts(every_suite_looks_weak: None, tmp_path: Path) -> None: + """``_build_ssl``, ``ssl_root_cert`` arm — a private CA pinned for the store hop.""" + + cert, _key = _self_signed(tmp_path) + settings = _pg_settings(ssl_root_cert=str(cert)) + with pytest.raises(ValueError, match="Postgres store"): + postgres._build_ssl(settings) + + +def test_postgres_trust_server_certificate_context_asserts( + every_suite_looks_weak: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """``_build_ssl``, ``trust_server_certificate`` arm — reachable only behind the dev escape, still + encrypted, so still asserted.""" + + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + settings = _pg_settings(trust_server_certificate=True) + with pytest.raises(ValueError, match="Postgres store"): + postgres._build_ssl(settings) + + +def test_postgres_default_arm_still_hands_asyncpg_the_job() -> None: + """The stated residual, pinned so it cannot be quietly reclassified as covered. + + The secure default returns bare ``True`` and asyncpg builds the context, so no object exists in + engine code for the assertion to run against. This test records that fact rather than claiming + the site is hardened.""" + + assert postgres._build_ssl(_pg_settings()) is True + + +# --- the preserved exemption: config/tls_probe.py ------------------------------------------------- + + +def test_the_tls_floor_probe_context_is_deliberately_not_hardened() -> None: + """``_offer_context`` must stay unasserted, and this test says why by measuring it. + + The probe offers ``ALL:@SECLEVEL=0`` so that a withdrawn protocol version is genuinely ASKED for; + without it modern OpenSSL refuses to send the ClientHello and the probe would measure the + engine's refusal to ask rather than the peer's refusal to answer. That offer resolves to a wide + suite list including non-forward-secret suites, so ``harden_cipher_suites`` WOULD raise here. + Adding it would empty the offer and turn a floor probe that can fail into one that cannot. + """ + + ctx = tls_probe._offer_context(ssl.TLSVersion.TLSv1) + weak = [c for c in ctx.get_ciphers() if not tls_policy._is_forward_secret(c)] + assert weak, ( + "the probe's ALL:@SECLEVEL=0 offer resolved to forward-secret suites only, so this test no " + "longer demonstrates why the exemption exists; re-derive it before changing the exemption" + ) + with pytest.raises(ValueError, match="non-forward-secret"): + tls_policy.harden_cipher_suites(ctx, connector="tls floor probe (must stay exempt)") + + # And the engine must NOT be calling it there. + source = Path(tls_probe.__file__).read_text(encoding="utf-8") + calls = [ + line + for line in source.splitlines() + if "harden_cipher_suites(" in line and not line.lstrip().startswith("#") + ] + assert not calls, f"tls_probe must not assert cipher suites on its offer context: {calls}" + + +def test_the_shared_https_handler_factory_asserts(every_suite_looks_weak: None) -> None: + """``build_asserted_https_handler`` - the one construction both openers share, so they cannot + drift onto different handlers.""" + with pytest.raises(ValueError, match="a label"): + tls_policy.build_asserted_https_handler(connector="a label") + + +def test_the_handler_factory_refuses_when_it_cannot_reach_the_context( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """It reads a private ``_context``, so it must FAIL CLOSED if a future CPython renames it. + + A ``getattr(..., None)`` that shrugged and returned would leave a security control reporting + success forever - the failure ``harden_kex_groups`` documents at length. Simulated by handing the + factory a handler class with no ``_context``. + """ + + class _NoContextHandler(urllib.request.HTTPSHandler): + def __init__(self) -> None: + super().__init__() + del self._context + + monkeypatch.setattr(urllib.request, "HTTPSHandler", _NoContextHandler) + with pytest.raises(ValueError, match="cannot reach the TLS context"): + tls_policy.build_asserted_https_handler(connector="a label") + + +def _covered_files() -> list[tuple[str, str]]: + """(module file, connector label) for every site this file claims to cover, for the scan below.""" + return [ + ("messagefoundry/transports/rest.py", "HTTP-family destination"), + ("messagefoundry/transports/soap.py", "SOAP destination"), + ("messagefoundry/pipeline/alert_sinks.py", "alert webhook destination"), + ("messagefoundry/auth/oidc_http.py", "OIDC identity provider"), + ("messagefoundry/logging_setup.py", "syslog TLS forwarder"), + ("messagefoundry/store/postgres.py", "Postgres store"), + ] + + +def test_every_covered_file_still_names_its_connector_label(request: Any) -> None: + """A rename receipt. The tests above match on a connector label; if a label is reworded in the + engine and the test's ``match`` is reworded with it, both move together and nothing notices that + a THIRD reader (an operator reading the error, a scorecard citing it) now sees something else. + This pins the label text to the file it is emitted from.""" + root = Path(request.config.rootpath) + missing = [ + f"{rel}: {label!r}" + for rel, label in _covered_files() + if label not in (root / rel).read_text(encoding="utf-8") + ] + assert not missing, ( + f"connector label(s) no longer present in the file that emits them: {missing}" + ) From 2b6dc1bea04d88cc7c709f1364a41c21391b1460 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 22 Aug 2026 12:43:18 -0500 Subject: [PATCH 3/4] test(store): the summary_access census assertion is stale on main, and only a skipped leg hid it NOT A DEFECT IN THIS BRANCH. #514 (BACKLOG #1187, merged as fdd89b49) added a `masked` key to the summary_access audit detail and updated the coalescer, but not the Postgres or SQL Server twins of the census test. This branch merely ran the legs that hid it. api/app.py _emit, on main: detail=json.dumps({"count": count, "masked": masked, "window_start": hour*3600}) tests/test_postgres_store.py:3409 asserted {"count": 5, "window_start": 0} tests/test_sqlserver_store.py:3435 asserted the same MAIN IS NOT RED, IT IS BLIND -- which is the worse half and the reason this needs saying. Both DB legs were SKIPPED on main's CI at fdd89b49 and at ae72f582 before it, so main reports green while carrying two failing tests. They run on a pull request, so the first PR to touch anything after #514 inherits three red required- adjacent legs it did not cause. This one did: postgres store, sql server 2022 and sql server 2025. WHY THE LEG THAT DID RUN STAYED GREEN, and it is not luck. The SQLite twin at tests/test_api.py:587 asserts `'"count": 5' in detail` -- a SUBSTRING containment check, which cannot see an added key. The two store twins assert DICT EQUALITY, which can. So the weaker assertion survived a change the stronger ones caught, and the stronger ones were the ones not being run. VERIFIED AT RUNTIME, not read off the source. Driving the real _SummaryAuditCoalescer end to end against a live store: ACTUAL detail: {'count': 5, 'masked': 0, 'window_start': 0} OLD assertion holds : False FIXED assertion holds: True The coalescer is backend-agnostic -- it calls store.record_audit and nothing in the shape is per-backend -- so the SQLite run is evidence for the Postgres and SQL Server paths, which is exactly the substitution that makes this checkable here. WHAT THIS RUN DOES NOT PROVE, stated because a local pass would be misleading: this repo SILENTLY SKIPS both DB legs locally. Measured on this tree, 152 skipped and zero run. CI is the only authority for the two lines themselves. LEFT ALONE DELIBERATELY: test_api.py:587's substring assertion. Tightening it to equality is the right change and it belongs to #1187's owner, not to a branch recovering unrelated TLS work. --- tests/test_postgres_store.py | 2 +- tests/test_sqlserver_store.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_postgres_store.py b/tests/test_postgres_store.py index a0b24f7e..a1e2e3de 100644 --- a/tests/test_postgres_store.py +++ b/tests/test_postgres_store.py @@ -3406,7 +3406,7 @@ async def test_summary_access_census_survives_and_coalesces_pg(store) -> None: await c.note(store, "alice", "ch1", 1, 3600.0) # hour 1 -> flush hour-0 window (count 5) rows = [r for r in await store.list_audit() if r["action"] == "summary_access"] assert len(rows) == 1 - assert json.loads(rows[0]["detail"]) == {"count": 5, "window_start": 0} + assert json.loads(rows[0]["detail"]) == {"count": 5, "masked": 0, "window_start": 0} assert rows[0]["actor"] == "alice" and rows[0]["channel_id"] == "ch1" await c.note(store, "bob", "", 4, 3600.0) # hour 1, scope "" -> channel_id NULL diff --git a/tests/test_sqlserver_store.py b/tests/test_sqlserver_store.py index ea2dc650..08205617 100644 --- a/tests/test_sqlserver_store.py +++ b/tests/test_sqlserver_store.py @@ -3432,7 +3432,7 @@ async def test_summary_access_census_survives_and_coalesces_ss(store) -> None: await c.note(store, "alice", "ch1", 1, 3600.0) # hour 1 -> flush hour-0 window (count 5) rows = [r for r in await store.list_audit() if r["action"] == "summary_access"] assert len(rows) == 1 - assert json.loads(rows[0]["detail"]) == {"count": 5, "window_start": 0} + assert json.loads(rows[0]["detail"]) == {"count": 5, "masked": 0, "window_start": 0} assert rows[0]["actor"] == "alice" and rows[0]["channel_id"] == "ch1" await c.note(store, "bob", "", 4, 3600.0) # hour 1, scope "" -> channel_id NULL (NVARCHAR NULL) From 748675a6e933eaf2d23f3d3f21a82da22dbccd09 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 22 Aug 2026 12:51:23 -0500 Subject: [PATCH 4/4] backlog: file #1322 -- the serverdb path gate selects on the tests, not on the sources they assert against Root cause of PR #525's three red DB legs. Found by the liaison; every claim below re-verified against origin/main by this seat before filing. NOT "MAIN SKIPS THE LEGS". That is deliberate and filing it would be filing a billed-minutes decision: ci.yml:1521 is `schedule || workflow_dispatch || serverdb=='true'` and its own comment names the no-per-merge-run guard. The defect is one level down, in the PR arm -- the only arm that can catch a change BEFORE it lands -- whose producer set has a hole. THE HOLE. ci.yml:1358's alternation lists messagefoundry/store/, three pipeline modules, config/(settings|wiring), a transports list, a tests/test_(...) list and ci.yml. It does not list messagefoundry/api/. THE INVARIANT IS WRITTEN ONE LEVEL TOO NARROW. ci.yml:1348 requires the alternation list "every file the sqlserver-store / postgres-store pytest steps below run" -- the TEST files. Not the SOURCE those tests assert against. test_postgres_store.py asserts on a dict built by api/app.py::_emit, so the rule cannot see this class. IT HAS ALREADY FIRED. #514 touched api/app.py, serverdb evaluated false, the legs never ran on its PR, push never runs them, and it merged green leaving two store tests asserting a stale shape. PR #525 selected the legs and inherited three reds it did not cause. SECOND WEAKNESS NAMED ON THE ROW, not folded into the fix: the SQLite twin at test_api.py:587 asserts substring containment and is blind to an added key. The assertion strong enough to catch the change was the one not being run; the one being run was too weak to notice. Tightening it belongs to #1187's owner. Severity bounded in the same line that causes it -- the schedule arm runs these legs nightly, so the blind window is about a day. What it does not do is stop the merge. Number allocated and claimed through scripts/coord. Backlog parser clean at 568 items, each declaring exactly one status. --- docs/BACKLOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 5f8a22c0..39bb7a04 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -12751,3 +12751,16 @@ which both readings of the cell now require. **Cluster:** CI reliability / teardown tests. **Priority:** P2. **Verdict:** build. **Severity:** no deployment axis (sec. 0) -- `tests/` ships in no engine path. The cost is that a required context can redden on `main` from runner load alone, and that while it is loaded the test cannot perform the discrimination it was written for. + +## 1322. the serverdb path gate lists the tests but not the sources they assert against, so an api/ change strands the DB legs + +> 🔢 **Filed 2026-08-22 -- not started. Found by the liaison, verified independently against `origin/main` by the filing seat.** Value **6/10** -- Difficulty **3/10** -- _quick win_. `.github/workflows/ci.yml:1358` computes `serverdb` from a path alternation that decides whether the SQL Server and Postgres legs run on a pull request. **It lists `messagefoundry/store/`, three `pipeline/` modules, `config/(settings|wiring)`, a `transports/` list, a `tests/test_(...)` list and `ci.yml` itself. It does not list `messagefoundry/api/`.** **THE CHANGE: widen the alternation, and widen the stated invariant above it, to the SOURCES those suites assert against -- not only the test files they run.** +> **THIS IS NOT "MAIN SKIPS THE LEGS", AND THE DISTINCTION IS THE ITEM.** Skipping on push is DELIBERATE and must not be filed as a defect: `ci.yml:1521` reads `if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || needs.changes.outputs.serverdb == 'true'`, and its own comment names the *"no-per-merge-run guard"*. Filing that would be filing a billed-minutes decision. The defect is one level down -- the PR arm, which is the only arm that can catch a change before it lands, is selected by a producer set with a hole in it. +> **THE INVARIANT IS ALREADY WRITTEN AND IS ONE LEVEL TOO NARROW.** `ci.yml:1348`: the alternation *"MUST list every file the sqlserver-store / postgres-store pytest steps below run"*. That binds the TEST files. It does not bind the SOURCE those tests assert against, and `tests/test_postgres_store.py` asserts on a dict built by `messagefoundry/api/app.py::_emit`. A rule that covers the asserting file but not the asserted-on file cannot see this class. +> **IT HAS ALREADY FIRED, WHICH IS WHY THIS IS A MEASUREMENT AND NOT A HYPOTHETICAL.** #514 (BACKLOG #1187, merged `fdd89b49`) added a `masked` key to the `summary_access` audit detail. It touched `api/app.py`, so `serverdb` evaluated **false**, so the DB legs never ran on its PR; and push never runs them. It merged green leaving `tests/test_postgres_store.py:3409` and `tests/test_sqlserver_store.py:3435` asserting the old two-key shape. The next PR to select those legs inherited three red ones it did not cause -- PR #525, fixed there at `2b6dc1be`. +> **AND THE ONE LEG THAT DID RUN COULD NOT SEE IT, which is a second, separable weakness worth naming on this row.** The SQLite twin at `tests/test_api.py:587` asserts `'"count": 5' in detail` -- SUBSTRING containment, blind to an added key. The two server-DB twins assert DICT EQUALITY and would have caught it. So the assertion strong enough to catch the change was the one not being run, and the one being run was too weak to notice. Tightening `test_api.py:587` to equality belongs to #1187's owner, not to this row, but a gate widening alone leaves that half standing. +> **SEVERITY IS BOUNDED AND THE BOUND IS IN THE SAME LINE.** The `schedule` arm of `ci.yml:1521` runs these legs nightly and unconditionally, so the blind window is about a day, not open-ended. What the nightly does not do is stop the merge -- it reports after the fact, on `main`, to whoever reads it. +> **CONDITIONAL, per section 0:** zero deployments, so nothing shipped is affected. The cost is a merge gate that cannot fail on a class of regression it has tests for, and a red PR handed to whichever unrelated branch selects the legs next. + +**Cluster:** CI reliability / path gating. **Priority:** P2. **Verdict:** build. +**Severity:** no deployment axis (sec. 0) -- this is CI configuration and ships in no engine path. The cost is that server-DB store tests do not run on the pull request that breaks them, and the failure surfaces on an unrelated branch a day later.