diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 792b7717..39bb7a04 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -12687,9 +12687,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 @@ -12710,3 +12729,38 @@ 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. + +## 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. 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_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_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_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) 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}" + )