Skip to content

test(tls): re-check the two library scope-outs ADR 0180 left as prose (BACKLOG #1317) - #760

Merged
wshallwshall merged 1 commit into
mainfrom
claude/backlog-1317-library-arm-scope-outs
Sep 4, 2026
Merged

test(tls): re-check the two library scope-outs ADR 0180 left as prose (BACKLOG #1317)#760
wshallwshall merged 1 commit into
mainfrom
claude/backlog-1317-library-arm-scope-outs

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

What this is

BACKLOG #1317's library half, re-measured. The named defect (forward secrecy was the only property checked) has been fixed on main since PR 519; this PR does not rebuild it.

Read against main at c2237d78.

Test file and BACKLOG row only. No engine source is touched.

The headline: I did not build the hvac assertion, and here is why

The brief asked me to build the missing assertions for hvac and ODBC Driver 18. Both were already ruled on by ADR 0180, accepted 2026-08-28 — three days before the brief was written, and the #1317 row never recorded it. My own measurements reproduce the ADR's reasoning independently, before I found it.

Building the hvac assertion would need the [vault] extra installed on a CI leg — ADR 0180's own named precondition. I cannot install hvac/requests/urllib3 here (that is an ad-hoc install into another checkout's environment), so I could not have run a single line of it. Overriding an accepted ADR with a control no test in my turn can execute is the silent-control shape CLAUDE.md section 11 forbids. I recorded the residual instead.

What I measured, by running the shipped code

Controls firing both ways on every claim. None of this is a source re-read.

Arm 1 — ldap3: BUILT

assert_ldap3_tls_suites exists, auth/ldap.py calls it from LdapAuthenticator.__init__, and it refuses both ways: a sound config passes; a ciphers= argument it cannot replicate raises; a fixture reporting every suite non-encrypting makes it raise on the good config too. The row's "the library half is unchanged" sentence is stale, and is corrected.

Arm 2 — ODBC Driver 18: genuinely un-assertable

probe result
pyodbc 5.3.0 module + Connection names containing ssl/tls/cipher/cert 0
aioodbc same scan 0
control: ssl module, same scan 75 names
store/sqlserver.py / transports/database.py build an SSLContext no — neither imports ssl

TLS there is connection-string keywords terminated inside the native driver, so the suite list belongs to the driver and the OS TLS stack.

ADR 0180 measured store/sqlserver.py alone and generalised to "ODBC Driver 18". The ODBC transport — the DATABASE destination and the ADR 0010 db_lookup hop, where message content crosses — was never named in it. This PR measures both, closing that hole in the ADR's evidence.

Arm 3 — hvac: unmeasurable here, as the ADR records

hvac delegates to requests, which delegates to urllib3, which builds and owns the context per connection. hvac, requests and urllib3 are all absent from this interpreter, and no CI leg installs [vault] (workflows install dev,harness,fhir,dicom,x12,xml,webauthn plus sqlserver/postgres).

What actually shipped

ADR 0180 ruled two arms out in prose, on measurements taken once, re-checked by nothing. A scope-out is a compensating control like any other, so SDS-3.7 binds it: it must not rest on a premise nothing re-checks. Three tests make both premises checked facts.

test premise mutation that reds it
test_the_odbc_scope_out_premise_still_holds neither ODBC module reaches for an ssl context appended ssl.create_default_context() to transports/database.py
(its control) the scan finds contexts where they exist pointed the control at a module with none
test_the_sqlserver_hop_asserts_no_suites_and_pins_what_it_can_control the DSN path asserts nothing; Encrypt=yes / TrustServerCertificate=no pinned changed the emitted Encrypt value
test_the_hvac_scope_out_premise_still_holds urllib3 absent AND no leg installs [vault] made urllib3 resolvable; separately added vault to a workflow's extras
(its control) the workflow scan reads real install lines removed webauthn from both workflows that install it

All five mutations were run. Each turned its own test red and nothing else. Every mutated file was restored and git status confirmed clean before commit. These are triggers, not guards: a red means the reason an arm was left unasserted has stopped being true, so build the assertion and amend the ADR — never delete the test.

Two claims the row carried as open that are false at HEAD

1. The strict positive allowlist IS built. The 2026-08-30 amendment says it is not. _APPROVED_TLS_SUITES landed 2026-08-22 in ae72f5828 (PR 519) — eight days earlier, and the row's own BUILT paragraph says so. Measured:

input tls_ciphers knob proxy_tls_ciphers
ECDHE-RSA-AES256-GCM-SHA384 accepted (positive control) accepted
ECDHE-RSA-AES256-SHA384 REFUSED (approved list) accepted
ECDHE-RSA-NULL-SHA REFUSED (must ENCRYPT) REFUSED
ADH-AES256-GCM-SHA384 REFUSED (must AUTHENTICATE) REFUSED
RC4-MD5 REFUSED (parse) REFUSED

Row two is the discriminating one: forward-secret, encrypting AND authenticated, refused anyway because it is unnamed. That is an allowlist, not a pile of properties. The same suite is accepted on proxy_tls_ciphers, so the refusal is the allowlist specifically. tests/test_tls_policy.py already pins it, so nothing to build. I left the false paragraph standing with a correction beside it, rather than deleting someone's dated record.

2. The ldap3 arm. Corrected in place, pointing at the new amendment.

Residual, and proposed closure

One residual: install the [vault] extra on a CI leg, then urllib3's own create_urllib3_context() becomes executable by a test and the assertion belongs in both _build_client factories (store/keyprovider_vault.py and config/secretprovider_vault.py, which between them cover all three clients — crypto_transit.py reuses the first). Named by subject, not by number: none is allocated. test_the_hvac_scope_out_premise_still_holds is what will report the day that premise changes.

Closure is proposed, not taken. Every open claim is now built, permanently out with evidence, or that one named residual. A Builder must not flip a banner while a residual stands, so the banner stays PARTIAL for the owner to rule on.

Checks

sys.path verified first, per the coordinator's instruction — the pass is about my worktree, not the primary checkout:

$ python -c "import messagefoundry; print(messagefoundry.__file__)"
C:\Users\Scott\Code\MessageFoundry\.claude\worktrees\agent-a27fa413caef20509\messagefoundry\__init__.py
check result
ruff check . All checks passed
ruff format --check . 1220 files already formatted
mypy messagefoundry (strict) Success, no issues in 267 source files
pytest tests/test_tls_cipher_assertion_sites.py tests/test_tls_policy.py 129 passed (was 126; +3 new)
pytest -k "backlog or ledger or citation or link_resolution or adr_index or docs_" 647 passed, 8 skipped
scripts/docs/backlog_status_check.py OK, 657 items, one status each

Not run, and they must be read on the runner: the full suite, and every hosted-only leg (windows-service-smoke, the SQL Server and Postgres store legs, CodeQL). Note the SQL Server leg is selected by the serverdb path filter; this PR touches tests/test_tls_cipher_assertion_sites.py, which is not in that alternation, so those legs likely will not run on it.

mypy tests/test_tls_cipher_assertion_sites.py reports 2 errors at _opener_context and _NoContextHandler. Both are pre-existing lines I did not touch, and CI type-checks messagefoundry messagefoundry_webconsole only, not tests/.

Collisions, both flagged by the coordination gate

  1. transports/database.py — a peer Builder is working BACKLOG #1178 in that file. My tests read it and change nothing in it. Two mutations temporarily edited it and both were reverted with git checkout --; the committed diff touches only tests/ and docs/BACKLOG.md. One thing for whoever lands second: test_the_odbc_scope_out_premise_still_holds reds if that file gains an ssl import or an SSLContext. That is the tripwire working, not a conflict — it means the ODBC scope-out's premise changed and ADR 0180 needs re-deriving.
  2. docs/BACKLOG.md — session b953c526 committed b653bb8c7 (scoring 73 unranked items) on claude/backlog-rankings-review-62db29. It is purely additive, 259 insertions and no deletions, and touches no line matching 1317. No overlap; my edit is confined to the #1317 body.

Assumptions I made, since a Builder cannot ask

  • An accepted ADR outranks a brief's premise. The brief said build the hvac and ODBC assertions; ADR 0180 had already ruled both out with reasons the brief did not cite. I followed the ADR and recorded the disagreement here rather than silently doing either one.
  • I did not add [vault] to the CI extras. It is ADR 0180's own trigger and would make the hvac assertion buildable, but I cannot run any of it in this environment, and pushing an unverified equivalence test plus a fail-closed runtime assertion I have never executed is a worse trade than recording the residual. If the owner wants that, it is a clean follow-up: flip the extra, watch test_the_hvac_scope_out_premise_still_holds go red, then build the assertion it points at.

🤖 Generated with Claude Code

… (BACKLOG #1317)

Read against main at c2237d7.

ADR 0180 asserted the AD LDAPS bind and ruled the other two library arms
#1317 names out of scope: hvac unmeasured, ODBC Driver 18 out permanently.
Both rulings were written down once and re-checked by nothing. A scope-out
is a compensating control like any other, so SDS-3.7 binds it.

Three tests re-check the premises, each with a live control and each proved
non-vacuous by mutation:

  test_the_odbc_scope_out_premise_still_holds -- neither store/sqlserver.py
  nor transports/database.py reaches for an ssl context. The ADR measured
  the first alone and generalised; the ODBC transport, where message content
  crosses, was never named. Control: the same scan finds the contexts in
  store/postgres.py.

  test_the_sqlserver_hop_asserts_no_suites_and_pins_what_it_can_control --
  the DSN path reaches harden_cipher_suites zero times, and the posture the
  engine CAN express is pinned (Encrypt=yes, TrustServerCertificate=no).

  test_the_hvac_scope_out_premise_still_holds -- urllib3 stays absent and no
  workflow installs the [vault] extra. Control: the workflow scan sees the
  extras CI really installs.

These are triggers, not guards. A red means the reason an arm was left
unasserted has stopped being true, so build the assertion and amend the ADR.

Also corrects two claims #1317 carried as open and are false at HEAD: the
strict positive allowlist (built 2026-08-22 in ae72f58, eight days before
the amendment saying it was not), and the ldap3 arm.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@wshallwshall wshallwshall added the reviewed A reviewer has read this. Removed automatically when new commits arrive. label Sep 3, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

LANDER RECEIPT. The owner directed this session to land the open pull requests and told it not to use the Reviewer seat, so this reviewed label is SELF-APPLIED and records a lander read, not an independent one. Saying so here because a self-applied label is otherwise indistinguishable from a reviewed one -- on PR 752 a withheld refusal was invisible to this gate and four findings shipped.

Head read: c978b80

Covered: three tests that re-measure the premises ADR 0180 used to scope ldap3, hvac and ODBC Driver 18 out in prose. They are triggers, not guards, and the docstrings say so -- a red means the reason an arm was left unasserted stopped being true. The substantive catch is real: the ADR measured store/sqlserver.py alone and generalised to "ODBC Driver 18", while transports/database.py carries the same keywords on a hop that moves message content and was never named. The scan carries a positive control (store/postgres.py, which does build a context), so a clean result on the subjects is a reading rather than a dead regex.

Not covered: tests only, no engine change.

@wshallwshall
wshallwshall added this pull request to the merge queue Sep 3, 2026
@wshallwshall
wshallwshall removed this pull request from the merge queue due to a manual request Sep 3, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 3, 2026
@wshallwshall
wshallwshall removed this pull request from the merge queue due to a manual request Sep 3, 2026
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Sep 4, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 4, 2026
@wshallwshall
wshallwshall removed this pull request from the merge queue due to a manual request Sep 4, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 4, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit fb878b1 Sep 4, 2026
41 of 42 checks passed
@wshallwshall
wshallwshall deleted the claude/backlog-1317-library-arm-scope-outs branch September 4, 2026 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. reviewed A reviewer has read this. Removed automatically when new commits arrive.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant