feat(tls): install [vault] on CI, then assert the Vault hops' suites (BACKLOG #1317)

[wshallwshall]

Closes the last open library arm of BACKLOG #1317, under the owner ruling of 2026-09-03: add the [vault] extra to a CI job, then build the hvac TLS suite assertion.

CONFLICT RESOLUTION, 2026-09-04. PR 760 merged (fb878b14), this branch was 58 behind and DIRTY, and it is now merged and green. What that merge changed is in the last section; everything above it describes the original change and still holds. mergeStateStatus moved DIRTY to BLOCKED.

The CI leg is the deliverable, not the preamble

ADR 0180 declined to build this assertion, and its stated reason was not that the hop was fine. It was that "no CI leg installs the [vault] extra", so the control could never be executed once. A Builder earlier that day refused for the same reason and was right to.

[vault] now installs on ci.yml's test leg. That is the leg where it costs least and the only leg it can go on: tests/ runs there and nowhere else, so no other install line could exercise the assertion. Adding it to tooling or webconsole would install a dependency where the test never runs.

No re-lock was needed, and that was verified rather than assumed. hvac==2.4.0, requests==2.34.2 and urllib3==2.7.0 are already pinned in both requirements.lock and constraints.lock, because uv export already covered every extra. pyproject.toml already declared vault = ["hvac>=2.3.0,<3"].

tests/_extras_probe.py gains a vault row, so an interpreter without the extra prints INCOMPLETE RUN instead of a quiet green over skipped security tests. That is what stops the leg being silently removed later.

It IS assertable, and here is the measurement that settles it

Measured on the exact locked pins by driving a real client at a real socket. Nothing below is read off source.

layer	holds an SSLContext?
hvac.Client	no, zero ssl/context attributes
its requests.Session	no
the PoolManager it builds	no, connection_pool_kw == {'maxsize': 10, 'block': False}
requests.adapters._preloaded_ssl_context	gone in requests 2.34 (2.32 had it)

urllib3 builds the context lazily, per connection, inside _ssl_wrap_socket_and_match_hostname. Driving the engine's own client shows it calling create_urllib3_context once, producing 17 suites: 14 TLS 1.2 + 3 TLS 1.3, zero NULL, zero anonymous, zero non-forward-secret. The bare replica matches that list exactly.

One measurement corrects a guess that nearly shipped. create_urllib3_context() and stdlib create_default_context() yield the identical 17 suites on the current OpenSSL. So "a stdlib look-alike would be measurably wrong" is false and is not written anywhere. The reason to use urllib3's own function is that only it tracks urllib3 if urllib3 narrows its own defaults.

What the assertion does

assert_hvac_tls_suites mirrors assert_ldap3_tls_suites. It refuses any hvac.Client argument it cannot replicate ({url, token, allow_redirects} are admitted). session= is the one that matters: it is the documented way to hand this hop a different TLS context, and a replica that accepted it would keep reporting a clean suite list for a context the hop had stopped using. verify= is refused too, and not because it changes the suite list, which it measurably does not, but because it is the knob that turns peer verification off.

Two improvements over the LDAPS precedent:

1. The assertion and the construction read ONE kwargs dict. The LDAPS site needed a whole second test to prove its asserted arguments were the ones its bind used, because the two lived in different methods. Here they cannot drift, so that failure mode is removed rather than tested for.
2. Both _build_client factories assert, covering all three clients, since crypto_transit reuses one. A test pins that sharing by identity (crypto_transit._build_client is keyprovider_vault._build_client) instead of restating ADR 0180's prose.

One behaviour change, stated plainly

_build_client moved out of its callers' try blocks in all three callers. Each had an except Exception that relabels: "could not read secret ... from Vault KV", "could not envelope-decrypt the store DEK", "could not reach a Vault Transit key". Left inside, this configuration refusal would have surfaced as what reads like a connectivity failure, the same mistake ADR 0180 records for the LDAPS site.

Fail-closed is unchanged: both paths propagate and the subsystem refuses to come up. What changes is that an hvac client construction failure now surfaces raw rather than wrapped. Assumption stated because a Builder cannot ask: at zero deployments the simple correct end state beats a compatibility shim (CLAUDE.md sec. 0).

Severity, conditional per CLAUDE.md sec. 0: nothing is exposed, there are zero deployments. A deploying site that selected a Vault secret or key provider would have crossed an unasserted TLS context on every KV read, DEK unwrap and Transit operation.

The mutation run

Five mutations on the assertion itself, each applied alone with the tree restored between, every one RED, and the restored tree green both before and after:

mutation	result
delete the assertion from the KV secret provider	RED, 2 failed
delete the assertion from the Transit key provider	RED, 1 failed
make the unreplicable-argument refusal inert	RED, 1 failed
drop harden_cipher_suites from the replica	RED, 3 failed
replica builds a context that DIFFERS from urllib3's	RED, 1 failed

The last row is the one that matters for a replica: it proves the equivalence test detects drift rather than merely passing. The positive control is separate and load-bearing: test_the_shipped_vault_hop_offers_no_weak_suite requires the real 17-suite list to be non-empty and clean on all three shipped predicates, because a refusal pinned alone cannot tell a working control from one that refuses everything.

That evidence is carried forward, and the right to carry it was measured rather than assumed. git diff 5f5a074c9 -- tls_policy.py secretprovider_vault.py keyprovider_vault.py crypto_transit.py reports exactly one hunk after the merge: main's own smtp_login_approved change. The assertion and both call sites are byte-identical to what was mutated, so the five results still stand. The merge added its own mutations, below.

---

The merge, and what it changed

PR 760's tripwire fired, which is the tripwire working

760 added test_the_hvac_scope_out_premise_still_holds, asserting urllib3 is absent and that no workflow installs [vault]. This branch makes both false by design. 760's own text says what to do: "A red does not mean the engine got worse. It means the reason an arm was left unasserted has stopped being true and the arm is now buildable -- build it and amend ADR 0180, never delete the test."

It is converted, not deleted. test_the_hvac_arm_stays_built_and_stays_executable is a guard in the opposite direction, covering the failure a fired trigger cannot see: the arm gets built and then quietly stops running.

• Drop [vault] from CI and all seven vault tests SKIP, which a suite reports as green. The extra is now pinned rather than trusted.
• Delete the call from a _build_client and, on an interpreter without the extra, nothing else in the file notices, because every test that would catch it is skipped. A source-level half runs there.

The call-site half parses, and the first version of it was WRONG

A substring scan stayed GREEN over a deleted security control, and I found that by mutating rather than by reading. Both vault modules carry from ...tls_policy import assert_hvac_tls_suites at module scope, so deleting the call from the factory body leaves the name in the file. The scan I wrote first passed.

_factory_calls_the_assertion parses the module with ast and asks whether the call is inside _build_client, which is the question the test means to ask, and which also reds if the call is moved somewhere that never runs.

Three fresh mutations, each applied alone, tree restored between, green before and after:

mutation	result
drop vault from ci.yml's test install line	RED -- no CI workflow installs the [vault] extra any more...
delete the call from keyprovider_vault._build_client, import left in place	RED -- _build_client no longer calls assert_hvac_tls_suites in ['messagefoundry/store/keyprovider_vault.py'] (the substring version was GREEN here)
delete the call from secretprovider_vault._build_client	RED, naming the other file

A fourth, renaming assert_hvac_tls_suites in tls_policy.py, is RED at collection: both vault modules import the name, so a rename is caught earlier and louder than the callable(getattr(...)) control. That control still covers the narrower case where the name survives but stops being callable. Recorded as what it is rather than claimed as a clean control hit.

The five conflicts, resolved by reading

Two files conflicted textually; three auto-merged and were checked rather than trusted.

file	how
tests/test_tls_cipher_assertion_sites.py	import line, resolved as a union: 760 added transports.database, this branch added store.crypto_transit, keyprovider_vault. Both are used.
docs/BACKLOG.md	two same-day amendments, below
messagefoundry/config/tls_policy.py	auto-merged. Verified: main's only change is smtp_login_approved, nowhere near assert_hvac_tls_suites.
pyproject.toml	auto-merged. Verified: main changed the sigstore pin and a corpus-guard comment; the [vault] block is untouched.
scripts/security/crypto_inventory_check.py	auto-merged in disjoint regions. Gate re-run: OK, 77 sites, no drift.

The ledger: two amendments dated the same day, written blind to each other

Ordered 760's first, since it was written and landed first, then this branch's. Superseded claims are marked in place rather than deleted, which is the convention this row already uses a few paragraphs up.

Four of 760's claims expired the same day and now say so: the hvac -- NOT BUILT bullet, the trigger table's hvac row, the "TRIGGERS, not guards" reading, and the section 5 residual. The two ODBC tests are still triggers, and the text now says not to read the three as a set.

The banner reading looked like a contradiction and is not one. 760 measures the strict positive allowlist as BUILT. This branch says it governs the operator KNOB (validate_tls_ciphers) and does not reach an inherited default context. Those are build-state and reach, two questions about one measurement, so neither corrects the other. What remains is a scope question, no builder may rule on it, and both amendments left the banner PARTIAL. It stays PARTIAL and I did not flip it.

git checkout MERGE_HEAD -- docs/BACKLOG.md before the merge commit puts the whole row reconciliation in one isolated commit, c8f956904, touching that file and nothing else, so it can be merged as rows.

Commits

1. 63699ca53 merge + the tripwire conversion + the ADR
2. c8f956904 the ledger reconciliation, alone
3. 2d1b4678f a second clean merge of origin/main (docs(testing): mark the vaulted tree so absence stops reading as evidence (BACKLOG #1435) #831). origin/main moved under this worktree while the first merge was being verified, because the fleet shares one repository and another worktree's fetch moves the ref. No resolution decisions; the only ledger overlap is #1435's own new row, nowhere near #1317.
4. 0662e9e62 the ADR index row, which this change falsifies. docs/adr/README.md still read "hvac is unmeasurable here ... NO CI leg installs the [vault] extra" as a present-tense claim about a security control's testability, while the ADR body had already been amended. Marked expired in place, same treatment the body gives it.

The ledger commit is not literally the last, and here is why that is still what a scripted row-merge needs. c8f956904 is the only commit that resolves a ledger row, and it touches docs/BACKLOG.md and nothing else. The two commits after it touch that file only through git's own clean merge of an unrelated row (#1435), and not at all, respectively.

Checks

.venv absent, so the primary interpreter was run from this worktree root and the source it imported was verified:

$ /c/Users/Scott/Code/MessageFoundry/.venv/Scripts/python.exe -c "import messagefoundry; print(messagefoundry.__file__)"
C:\Users\Scott\Code\MessageFoundry\.claude\worktrees\agent-a5a51625835fce303\messagefoundry\__init__.py

That interpreter has no hvac, so the seven vault tests skip there. A second venv was built in the scratchpad from it with --constraint constraints.lock -e ".[dev,vault]", and it resolves the same worktree source, so the seven ran rather than skipped.

check	result
test_tls_cipher_assertion_sites.py + test_tls_policy.py, WITH [vault]	137 passed, 0 skipped
the same two, WITHOUT it (primary)	130 passed, 7 skipped, probe names vault in INCOMPLETE RUN
the converted guard, without the extra	PASSED -- which is its whole point
the three new mutations	all RED, restored green
vault-adjacent suites (redirect policy, secretprovider, keyprovider, crypto_transit)	71 passed, 4 skipped
ledger + doc gates (backlog status, citations, hygiene claim extraction, ledger check, citation line, doc guards, doc ref handle, adr analyze, required contexts)	235 passed across two runs
crypto_inventory_check.py	OK, 77 sites, no drift
ruff check . / ruff format --check .	clean, 1241 files
mypy messagefoundry strict, primary	Success: no issues found in 267 source files
mypy messagefoundry strict, vault venv	21 pre-existing import-not-found in auth/webauthn.py, transports/dicom.py, parsing/dicom/_deps.py, parsing/fhir/_deps.py -- four files untouched here, all from that venv lacking those extras. Zero in any file this PR touches.
actionlint via pre-commit	Passed
full pre-commit at each commit	all hooks Passed, no --no-verify

Not run, and someone must read them: the hosted-only legs. windows-service-smoke (NSSM), docker-smoke and the server-DB legs never run in a Builder's process. The test matrix leg is the one to read closely, since it installs a new extra on ubuntu, windows-2022 and windows-2025. The full pytest tests/ suite was not run, per the brief: over ten minutes, plus the known test_connscale_smoke.py fixed-port collision under worktree contention (BACKLOG #1014).

Two reds on this PR are NOT this PR, measured against the live API

Branch protection changed under this branch while it was being verified. Read myself, not relayed: gh api repos/MEFORORG/MessageFoundry/branches/main/protection/required_status_checks returns 13 contexts, and zero of them match "reviewer".

1. a reviewer has read this will show RED and blocks nothing. The required context is gone; .github/workflows/review-gate.yml still runs the job and still fails without the label. Both are true at once.
2. the required-contexts file matches the server will go RED on EVERY open PR, and the fix is not mine to make. .github/required-contexts.txt:205 still lists a reviewer has read this, and tests/test_required_contexts.py:119 still pins assert len(contexts) == 14 against a live 13. CLAUDE.md says that file and that count move in the same PR as the protection change, and this is not that PR. Editing them here would collide with whoever owns it. Flagged, not fixed.

Neither red is attributable to this diff. gh pr checks also renders a CANCELLED job as "fail", so tally the check-runs API and take the latest run per name.

Two things a reviewer should look at deliberately

1. I force-released the #1317 claim, and that deserves review. claim.ps1 -List reported the holder as HOLDER GONE -- worktree no longer exists; release with -Force -- the tool's own instruction, and a stronger signal than the DIRECTORY ONLY case that gave the previous Builder pause. The dead claim's own note names this rebase as its outstanding job. Recorded in .history and reversible. I did not use --no-verify and did not reword a subject to dodge a gate.

2. The crypto inventory gate documents tls_policy on both vault modules rather than being silenced, and it re-passes after the merge.

Closure proposed, not taken

Every library arm this row named is now closed or recorded with evidence: ldap3 asserts (2026-08-30), hvac asserts (this PR), and ODBC Driver 18 is un-assertable with the reason in ADR 0180 -- TLS terminates inside the native driver, so the suite list belongs to the driver and the OS TLS stack rather than the interpreter's OpenSSL. That is a finding, not a deferral.

The banner stays PARTIAL. A seat with the authority should rule on whether the allowlist-scope question survives as this row or a new one. I did not close the item.

I applied the reviewed label to my own work after my last push, as the process requires. That records that the step happened; it is not an independent review, and this PR has not had one.

🤖 Generated with Claude Code

[wshallwshall]

Lander: heads-up on ordering with #760, which I have queued ahead of this.

Both pull requests are BACKLOG #1317 and they are sequential rather than competing: #760 measures and records the residual, and this one closes it.

#760 adds test_the_hvac_scope_out_premise_still_holds to tests/test_tls_cipher_assertion_sites.py, asserting that urllib3 is absent and that no CI leg installs the [vault] extra. #760's own table names the two ways that premise breaks, and one of them is "separately added vault to a workflow's extras" -- which is exactly what this pull request does in .github/workflows/ci.yml.

So once #760 lands, this branch will both conflict in that test file and make its new assertion false. This is expected work, not a defect in either pull request: the scope-out was true when #760 measured it, and this pull request is what retires it.

What this branch needs on its next push:

1. merge origin/main and resolve tests/test_tls_cipher_assertion_sites.py
2. flip test_the_hvac_scope_out_premise_still_holds rather than delete it -- the premise is now retired, and a test recording that is worth more than an absent one
3. re-apply the reviewed label, which the push strips

I am not making that change from here, because it is a judgment about how the retired scope-out should read, not a mechanical conflict resolution. Say the word if you would rather I did.

[wshallwshall]

Lander: this is now DIRTY, and it needs your judgement rather than my guess.

760 has landed. As flagged earlier, it adds test_the_hvac_scope_out_premise_still_holds to tests/test_tls_cipher_assertion_sites.py, asserting that urllib3 is absent and that no CI leg installs the [vault] extra. This pull request installs that extra in .github/workflows/ci.yml, which is its whole purpose. So the two now conflict in that test file plus docs/BACKLOG.md.

Both pull requests are BACKLOG #1317 and they are sequential, not competing: 760 measured and recorded the residual, and this one closes it.

What this needs, and why I am not doing it. The resolution is to FLIP test_the_hvac_scope_out_premise_still_holds rather than delete it — the scope-out is now retired, and a test recording that is worth more than an absent one. Choosing how that assertion should read is a design decision about your change, not a mechanical conflict resolution, so under the triage rule in roles/LANDER.md section 2a it routes to you rather than getting my guess.

The docs/BACKLOG.md half I would resolve mechanically if you want it, but it is not worth a separate push while the test file is open.

Everything else on this branch is fine. When you push, remember: push, WAIT for the review-gate synchronize run to reach completed, THEN add the reviewed label, THEN read the label back. Labelling straight after a push loses a race silently — 15 of 16 pull requests lost it last night with every command reporting success.