fix: do-common v2.0.0 - capture TLS chain from handshake#270
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the do-common PowerShell module’s certificate tooling to capture and return the server-presented TLS certificate chain from the handshake (proxy/broken-chain friendly), aligns the OpenSSL and native APIs around a new -PresentedChain switch, and improves macOS root-certificate discovery.
Changes:
- Replaced
Get-Certificate -BuildChain/-IgnoreValidationwith-PresentedChain, capturing the presented chain via the TLS validation callback and always accepting the certificate for inspection scenarios. - Added macOS support to
Get-RootCertificatesviasecurity find-certificate. - Bumped module version to
2.0.0and updated docs/examples; added a design lesson to prevent doc drift on API renames.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/do-common/Functions/python.ps1 | Updates certifi-fix flow to use -PresentedChain and native trust gating. |
| modules/do-common/Functions/certs.ps1 | Implements presented-chain capture, removes validation/chain-build switches, adds macOS roots, renames OpenSSL switch. |
| modules/do-common/do-common.psd1 | Bumps module version to 2.0.0 for the breaking API change. |
| docs/do-common/certs.md | Updates examples to use -PresentedChain. |
| design/lessons.md | Adds a lesson to enforce doc updates when renaming/removing public API surface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
szymonos
force-pushed
the
fix/do-common
branch
2 times, most recently
from
July 17, 2026 12:17
59bfbdc to
c14268a
Compare
Capture the endpoint-presented TLS chain from the handshake callback instead of rebuilding it locally, which failed behind an inspecting proxy (PartialChain + unreachable revocation) and crashed Show-CertificateChain. - Get-Certificate: replace -BuildChain/.Build() with -PresentedChain (callback capture, deep-copied leaf-first, null-chain guard + dedup, leaf fallback when the callback captures nothing); read the leaf via GetRawCertData(); drop -IgnoreValidation (always accept so invalid certs can be inspected) - Get-CertificateOpenSSL/Show-Certificate/Show-CertificateChain: -BuildChain -> -PresentedChain; remove WarningPreference=Stop that turned the chain warning into a terminating error - Get-RootCertificates: add macOS keychain branch (was null on macOS) - Invoke-CertifiFixFromChain: OS-trust-only gate (no revocation, no AIA downloads; X509Chain disposed via finally), empty-chain guard, guard pip fallback, reset per-iteration state, and skip (warn, don't abort) cacert paths that aren't writable BREAKING CHANGE: -BuildChain and -IgnoreValidation removed; use -PresentedChain. Co-Authored-By: Claude <noreply@anthropic.com>
Align the published Show-Certificate examples with the renamed module API and record the module-API/doc-sync lesson. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Repairs certificate-chain retrieval in
do-common, which was broken behind a TLS-inspecting proxy and crashedShow-CertificateChain. Breaking (MAJOR, 1.8.3 -> 2.0.0):-BuildChainand-IgnoreValidationare removed; use-PresentedChain.Root cause.
Get-Certificate -BuildChainreturned only the leaf (SslStream.RemoteCertificate) and then rebuilt the chain locally viaX509Chain.Build(). Behind an inspecting proxy that fails on nix pwsh/macOS -PartialChain(the proxy intermediate isn't locally resolvable and has no working AIA) plusRevocationStatusUnknown(defaultRevocationMode=Online, internal CRL unreachable) - so it fell back to the bare leaf and emitted a warning thatShow-Certificate'sWarningPreference=Stopturned into a terminating error.Fix. Capture the full server-presented chain from the TLS handshake callback (
RemoteCertificateValidationCallback's$chain) - the .NET equivalent ofopenssl s_client -showcerts- so it works even for broken/untrusted/proxied chains, independent of the local trust store and revocation reachability.Changes
Get-Certificate:-BuildChain/.Build()->-PresentedChain(handshake-callback capture, deep-copied leaf-first so certs survive stream disposal; null-$chainguard, per-call dedup, and leaf fallback when the callback captures nothing). Leaf read viaGetRawCertData()(baseX509Certificatemethod).-IgnoreValidationremoved - the handshake now always accepts the presented cert so expired/self-signed/untrusted certs can be inspected without throwing; only connect/transport failures raise.Get-CertificateOpenSSL/Show-Certificate/Show-CertificateChain:-BuildChain->-PresentedChain(symmetry); removedWarningPreference=Stopthat caused the crash.Get-RootCertificates: added a macOS keychain branch (security find-certificate); it returned$nullon macOS before.Invoke-CertifiFixFromChain: native cross-platformX509Chaintrust gate (resolves against the OS store incl. macOS keychain; disposed viafinally), empty-chain guard,pipfallback guarded withGet-Command, per-iteration state reset, and per-path write hardening (an unwritable cacert - e.g. a system-owned pip_vendorbundle - is warned-and-skipped, not aborted). Fixes a prior silent no-op on macOS.Show-Certificateexamples updated to-PresentedChain.Test plan
make lint-diff- all pre-commit hooks green (incl.build mkdocs)/second-opinion(gpt-5.3-codex) + Copilot PR review - all findings addressed (chain capture null-guard/dedup/leaf-fallback,GetRawCertData, empty-chain guard,X509Chaindispose, filter simplification)Show-CertificateChain google.comreturns the full 3-cert chain (leaf -> PG HTTPS Proxy -> PG Root CA), no crashbadssl.commatrix (self-signed, expired, untrusted-root, wrong-host, incomplete-chain) - all return a cert, none throw, never emptyInvoke-CertifiFixFromChainend-to-end - appends presented CAs to certifi, idempotent on re-run; read-only cacert warned-and-skipped without aborting (was a no-op on macOS before)-BuildChain/-IgnoreValidation(repo-internal, ad-hoc use only per author)🤖 Generated with Claude Code