Skip to content

CI: the SQL Server certificate pin test runs on the 2025 leg against a strict server, with the certificate untrusted and trusted (BACKLOG #3714 items 6, 8) - #2579

Merged
wshallwshall merged 4 commits into
mainfrom
b342-3714b
Oct 28, 2026
Merged

wshallwshall merged 4 commits into
mainfrom
b342-3714b

Conversation

@wshallwshall

@wshallwshall wshallwshall commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Merge danger

Reversible: yes -- a revert of this change on main undoes all of it.

Blast radius: CI only, plus one test. In .github/workflows/ci.yml, job sqlserver-store, five new steps, all gated if: matrix.label == '2025', after the privilege-preflight step: start a second SQL Server container (mssql-strict, same ${{ matrix.image }} digest, port 14330) with a generated test certificate and network.forcestrict = 1; wait for a TLS-first (TDS 8.0) handshake and a strict login (bounded at 180 s inside a 5-minute cap); run the pin test untrusted; trust the certificate with update-ca-certificates; run the pin test trusted. No job id, step or name: is renamed; services:, the matrix, the 2022 leg, .github/required-contexts.txt, the CI gate roll-up and changes are untouched; there is no continue-on-error, and every new step fails closed. The leg is not a required context by name, but the required CI gate rolls it up, so a red here blocks a merge. tests/test_sqlserver_store.py: the pin test gains a strict_chain arm (strict, no pin) and names the arm in its warning line. ADR 0173: one "No CI job sets it" line corrected with a marker. scripts/ci/retry-native-crash.sh: one comment.

Evidence

  • Before: on PR 2533's run 40381630197, both sql server (store + connector) 2022 and ... 2025 SKIPPED test_server_certificate_pin_is_enforced_under_strict_encryption; it has never run in CI, so a green SQL Server leg said nothing about the pin.
  • After: NOT YET MEASURED. This PR's own CI run is the measurement. Read, in job sql server (store + connector) 2025: the wait step's BACKLOG-3714-STRICT-READY after <n>s: ...; strict login CONNECTED (or its ::error:: naming the stage it never passed); BACKLOG-2223-PIN arm=untrusted strict_right=... strict_wrong=... strict_chain=... yes_right=... yes_wrong=... (also a ::notice); the trust step's Verify return code: 0 (ok); BACKLOG-2223-PIN arm=trusted .... Item 6 closes on the trusted line showing strict_chain=CONNECTED and strict_wrong=REFUSED; item 8 on either line printing. The 2022 leg should show none of these steps. Each pin step fails if no BACKLOG-2223-PIN line prints, because a skip would read like a pass.

BACKLOG #3714 slice B (items 6, 8) -- batch 342

Source branch b342-3714b, head 92fdc73da71d3ae5827294a2f59c0a412e9c60d9, base origin/main. Commits 956afd7 (build), 09bc22c (review round 1 repairs), 92fdc73 (fix after run 40524945884: the test's guard 'ServerCertificate' not in unpinned_dsn matched inside TrustServerCertificate=no;; the test now reads DSN keys with _odbc_pairs/_odbc_value/_odbc_with, plus a server-free test of that reader). The head is the reviewed head: the fix round's single review read it, and nothing was committed after.

First reading (run 40524945884): BACKLOG-3714-STRICT-READY after 4s: New, TLSv1.3, Cipher is TLS_AES_256_GCM_SHA384 ALPN protocol: tds/8.0; strict login CONNECTED. Linux SQL Server 2025 serves TDS 8.0 strict in hosted CI; the pin test then failed on its own guard before measuring.

Whose decision

Decided 2026-10-27 by the batch 342 Manager under the owner's delegation of 2026-10-27 (decision d20261027T133707051-2171e6). The question, as asked through AskUserQuestion: "#3714 items 6 and 8: the SQL Server pin test has never run in CI (both SQL Server legs skip it). Making it run is a CI workflow change. Adversarial review SPLIT: Linux SQL Server 2022 cannot force strict encryption (only 2025+ has network.forcestrict), the job's services: container would have to become a docker run step, and a flaky strict leg could block the merge queue. What should happen?" The owner's answer: "/driver". Decided: measure, then make the 2025 leg strict only.

One departure from the brief: the brief said to replace the 2025 service container. The Builder kept it and added a separate strict container, because forcestrict refuses every non-strict login and every other suite on the leg connects with Encrypt=yes. That keeps all the constraints above.

Checks (Builder's report)

  • Ran: YAML parse; bash -n on all five new run bodies (with a broken-body control that exits 2); the certificate recipe on local OpenSSL 3.5 (a trusted self-signed leaf verifies; a wrong-hostname control is refused; Ubuntu's 3.0 not tried); ruff, mypy on the test file (linux); pre-commit including "Lint GitHub Actions workflow files"; pytest over every test reading the workflows, the retry wrapper or the ADRs, plus test_sqlserver_store.py: 6020 passed, 279 skipped, 71 xfailed (test_workflow_pipefail_screen caught one unguarded docker logs | tail in the first draft, fixed); /simplify.
  • Skipped: hosted CI (it cannot run it); a local container probe (Docker Desktop not running; pulling the image is a download); the full suite.

Known open findings (shipped open; two rounds, cap reached)

  1. Forcestrict discards the Encrypt=yes readings and confines the test to 2025. Both rounds say a server with only tlscert/tlskey would likely accept strict logins too, keeping the yes comparison with #3009 and possibly running on 2022. Kept because the decision names forcestrict and 2025; worth revisiting once this run's lines are read.
  2. The readiness login pins the right certificate, which is the strict_right control itself; a real right-pin failure surfaces as "not ready within 180 s" naming the stage and the driver's text, with no PIN line.
  3. Two unguarded command substitutions under the wait step's bash -e (handshake=, logs="$(docker logs ...)") could end the step without its diagnostic.
  4. The two pin steps keep the retry wrapper's default crash cause (release: complete the 0.4.0 notes before the tag (BACKLOG #1904, #1905) #1459/audit-verify exits 4, not checked, for a keyed chain verified without its key (vault BACKLOG #2725) #2049) though the test binds no parameters; they should probably set RETRY_NATIVE_CRASH_CAUSE: "". The "every database-leg call site" comment is a completeness claim (and tests/test_ci_retry_native_crash.py:128 still says "Nine call sites").
  5. The untrusted arm now asserts strict_chain REFUSED, so a local run with the strict variable against a trusted certificate fails unless it sets MEFOR_TEST_SQLSERVER_PIN_TRUSTED=1.
  6. The strict steps run only if every earlier suite passed; gating them on !cancelled() would separate a flaky earlier suite from the pin lines.
  7. The strict container binds the port on all interfaces; 127.0.0.1 would do.
  8. ADR 0173's next sentence, "The paragraph above is kept as written", now follows an edited paragraph.
  9. The two pin steps duplicate their run body and env.
  10. The strict container boots cold at the end of the leg.
  11. The ::notice text is not escaped for %, CR or LF.
  12. Nothing removes the container or the trusted .crt at job end (matters only on a reused runner).
  13. The asserts accept any refusal of strict_wrong, not only a certificate mismatch; the driver's message is in the line.

From the fix round's review (new): 14. the new reader test's control "ServerCertificate" in unpinned also passes on the password text alone; 15. the server-free reader test sits in the gated module, so the main test job never runs it; 16. the trust and trusted-arm steps run only if every earlier step passed, so a failed untrusted arm skips the one reading that separates a pin refusal from a chain refusal (!cancelled() plus the wait step's outcome suggested); 17. no step checks that forcestrict took effect; 18. the second server starts beside the uncapped service container (memory pressure); 19. a pytest-timeout kill reports as "did not run".

Proposed ledger banner (for the Lander)

#3714 items 6 and 8 BUILT, NOT YET MEASURED: the 2025 sqlserver-store leg runs the pin test against a forcestrict server in two arms; read both BACKLOG-2223-PIN lines on the PR run before closing. (The batch 342 Manager will post the readings here.)

Landing order

None required. Clean against main and against the open PRs that touch ci.yml (2550, 2554, 2561, 2565, 2570), each merged onto current main first.

wshallwshall added 2 commits October 27, 2026 09:07
…rver, trusted and untrusted

BACKLOG #3714 slice B, items 6 and 8.

The 2025 sqlserver-store leg now starts a second container from the same
digest-pinned image on 127.0.0.1:14330, after the other suites. It mounts a
generated test certificate (serverAuth EKU, CN and SAN localhost) and an
mssql.conf with forceencryption and forcestrict. The service container stays:
forcestrict refuses every non-strict login, which every other suite makes.
A wait step needs the server's ready line and a TLS-first (TDS 8.0)
handshake. The pin test then runs twice: certificate untrusted, then in the
runner's OpenSSL store. Each step fails if the test did not run.

The test gains strict_chain (strict, no pin). The trusted arm asserts it
connected, so its refused wrong pin is told apart from a chain refusal.
The 2022 leg is unchanged.

Proposed PR title: CI: the SQL Server certificate pin test runs on the 2025 leg against a strict server, with the certificate untrusted and trusted (BACKLOG #3714 items 6, 8)

Proposed banner: #3714 items 6 and 8 BUILT, NOT YET MEASURED: the 2025 sqlserver-store leg runs the pin test against a forcestrict server in two arms; read both BACKLOG-2223-PIN lines on the PR run before closing.
…sserts both arms' chain

BACKLOG #3714 slice B, items 6 and 8: code-review round 1 fixes.

- The test certificate is CA:FALSE, made from a config file.
- The container publishes 14330 on all interfaces, the probes use localhost.
- The wait is bounded at 180 s inside its cap, and ends with a strict login.
- The second server's memory is capped at 2048 MB.
- The pin steps' cap covers the retry wrapper's three attempts.
- The untrusted arm asserts strict_chain REFUSED; the right pin is asserted
  first, and the trusted arm's message carries the driver's text.
- ADR 0173 and the retry wrapper's header no longer state stale facts.

Proposed PR title: CI: the SQL Server certificate pin test runs on the 2025 leg against a strict server, with the certificate untrusted and trusted (BACKLOG #3714 items 6, 8)

Proposed banner: #3714 items 6 and 8 BUILT, NOT YET MEASURED: the 2025 sqlserver-store leg runs the pin test against a forcestrict server in two arms; read both BACKLOG-2223-PIN lines on the PR run before closing.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings (forks a0c0f941d57b63f0c round-a1df1d1ad8, a9db3a51ebd077505 round-a6632f8e87).
Rounds: 2. Findings: round 1 15 -- 11 confirmed and fixed, 1 rejected (duplicate run bodies: already byte-identical after /simplify; env repetition follows the job's style), 3 open (forcestrict loses yes_*, 2022 scope, strict_wrong refusal cause); round 2 13, all open (no commit after the last review).

@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 27, 2026
greptile-apps[bot]
greptile-apps Bot previously approved these changes Oct 27, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager (batch 342): do not merge yet. First reading from run 40524945884: the strict container works (BACKLOG-3714-STRICT-READY after 4s: New, TLSv1.3, Cipher is TLS_AES_256_GCM_SHA384 ALPN protocol: tds/8.0; strict login CONNECTED), so Linux SQL Server 2025 does serve TDS 8.0 strict in CI. The pin test then failed before measuring, on its own guard: 'ServerCertificate' not in unpinned_dsn matches inside TrustServerCertificate=no;. A Builder is fixing that assertion; I will post the new reviewed head and the BACKLOG-2223-PIN readings here.

…ficate no longer reads as a pin

Run 40524945884, job 122262048822 (sql server 2025): the strict server came
up and a strict login connected, then the untrusted arm failed before it
measured. Its guard, "ServerCertificate" not in unpinned_dsn, is also true
of TrustServerCertificate=no;, which every verified string carries. The
DSN was right; the guard was wrong.

The test now reads the string as keyword=value pairs, brace-aware, and
sets Encrypt by key instead of a substring replace. A new server-free
test checks the reader on the builder's own strings, with a password
holding the text a substring check trips on. It runs on both SQL Server
legs ahead of the strict step.

PR title: CI: the SQL Server certificate pin test runs on the 2025 leg
against a strict server, with the certificate untrusted and trusted
(BACKLOG #3714 items 6, 8)

BACKLOG #3714
@greptile-apps
greptile-apps Bot dismissed their stale review October 27, 2026 15:52

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager (batch 342): new reviewed head 92fdc73, one test-only commit fixing the substring guard that stopped run 40524945884 (details and six new findings in the body; the workflow is unchanged). Builder's QA line, verbatim:

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (fork a4e1f554887f14f53 round-71bca6c440; ascii spelling).
Rounds: 1. Findings: 0 confirmed and fixed (the brief barred commits after review), 0 rejected, 15 open.

My hold stays until I post this run's two BACKLOG-2223-PIN readings.

@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Oct 27, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager (batch 342): the readings, from run 40535242435, job sql server (store + connector) 2025 (job 122303000020), head 92fdc73. The leg passed.

  • BACKLOG-3714-STRICT-READY after 4s: New, TLSv1.3, Cipher is TLS_AES_256_GCM_SHA384 ALPN protocol: tds/8.0; strict login CONNECTED
  • Untrusted arm: strict_right=CONNECTED, strict_wrong=REFUSED ([08001] ... SSL Provider: ... Provided certificate doesn't match with server/peer certificate), strict_chain=REFUSED ([08001] ... certificate verify failed:self-signed certificate).
  • Trust step: Verify return code: 0 (ok).
  • Trusted arm: strict_right=CONNECTED, strict_wrong=REFUSED ([08001] ... Provided certificate doesn't match with server/peer certificate), strict_chain=CONNECTED; yes_right=REFUSED (forcestrict refuses every non-strict login, as expected).

What it shows: with the certificate trusted through the chain, a strict login with no pin connects, and a strict login pinning a different certificate is still refused with the driver's own pin-mismatch message. So on Linux ODBC Driver 18 against SQL Server 2025, the ServerCertificate pin is enforced on its own and is not a chain pass. This closes #3714 item 6 (the CI leg trusts the presented certificate through the chain and still shows a wrong pin refused) and item 8 (the live test has run on the SQL Server CI leg, read from its BACKLOG-2223-PIN line). The Encrypt=yes comparison with #3009 is not measured here (open finding 1).

Proposed banner, replacing the one in the body: #3714 items 6 and 8 BUILT AND MEASURED on PR 2579's run 40535242435: under Encrypt=strict on Linux ODBC Driver 18 / SQL Server 2025, a wrong ServerCertificate pin is refused with the certificate trusted through the chain (strict_chain=CONNECTED, strict_wrong=REFUSED).

My hold is lifted once CI gate is green on 92fdc73.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager (batch 342): the head moved to fa5364b (Merge branch 'main' into b342-3714b, after PR 2583 landed). Checked: a fresh git merge-tree of the reviewed head 92fdc73 with main cf441c8 gives a tree identical to fa5364b's, so the new head adds only a clean merge of main. My readings comment stands; the hold lifts when the required checks are green on fa5364b.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager (batch 342): re-ran the failed job once on head fa5364b. The red leg was test (windows-2025, py3.14): 1 failed of 43583, tests/test_durability_hook_push_bound.py::test_the_BOUND_does_not_break_a_push_that_finishes (a git-push timing test of the coordination hooks; this PR touches none). Failed log: https://github.com/MEFORORG/MessageFoundry/actions/runs/40586457377/job/122479237781. A second red on the same leg is a finding.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander (session 8a04c9f3), 2026-10-28. Enqueuing at pinned head fa5364b7200fc87c913353545554c406626ed321.

Review proof: the Builder's QA line (xhigh, two rounds) covers 92fdc73da7; the Manager's merge-tree check shows fa5364b720 adds only a clean merge of main. All nine required contexts are completed and successful on fa5364b720.

Open findings, each filed on vault main (vault PR 2734, merged 9ba0487fc), none changing product behaviour or weakening a control:

  • Finding 1: #4663
  • Findings 2, 3, 6, 11, 16, 17, 19: #4664
  • Findings 5, 13, 14, 15: #4665
  • Findings 4, 7, 8, 9, 10, 12, 18: #4666

@wshallwshall
wshallwshall added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit dad1bea Oct 28, 2026
80 of 81 checks passed
@wshallwshall
wshallwshall deleted the b342-3714b branch October 28, 2026 03:35
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. qa Builder QA record posted; not a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant