fix(security): skip with clear diagnostic on TLS cert verify failure#210
Merged
ian-flores merged 6 commits intomainfrom Apr 22, 2026
Merged
fix(security): skip with clear diagnostic on TLS cert verify failure#210ian-flores merged 6 commits intomainfrom
ian-flores merged 6 commits intomainfrom
Conversation
inspect_headers now distinguishes ssl.SSLCertVerificationError (wrapped by httpx.ConnectError) from plain connection refusals. A cert-verify failure is a trust-bundle issue on the test runner (common when the server is behind an AWS ALB with an ACM certificate), not a server security finding — skip with guidance pointing to SSL_CERT_FILE and common CA bundle paths, mirroring the pattern applied to test_ssl.py in #198. Fixes #175
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns inspect_headers in the security HTTPS tests with the TLS handshake classification added in #198, so certificate verification failures behind SSL-terminating proxies are skipped with actionable CA-bundle guidance instead of being reported as “connection refused”.
Changes:
- Add
sslimport and classifyhttpx.ConnectErrorcases caused by TLS certificate verification failures. - Skip (with a clear diagnostic pointing to
SSL_CERT_FILEand common CA bundle locations) when cert verification fails; otherwise keep failing with the existing “connection refused” guidance.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+80
to
+82
| # guidance rather than failing as "connection refused". See | ||
| # test_ssl.py (commit c057b80 / PR #198) for the same | ||
| # classification applied to the TLS-version test. |
Comment on lines
+88
to
+92
| f"Could not verify TLS certificate for {product} at {pc.url}: {exc}. " | ||
| "This is a certificate-trust issue on the test runner, not a " | ||
| "server security finding. If the server uses a valid public " | ||
| "certificate (e.g. behind an AWS ALB with an ACM cert), set " | ||
| "SSL_CERT_FILE to a CA bundle that includes public roots: " |
Remove commit hash/PR number from inline comment and fix double spaces in the TLS skip message for cleaner, more durable source text.
Move import re to module scope, and improve the inline comment to use the full path to test_ssl.py and explain why that test raises while this one skips.
Document why the CERTIFICATE_VERIFY_FAILED string fallback is needed alongside the isinstance check, clarifying the transport-specific case where httpx does not populate __cause__.
4 tasks
|
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
Issue #175 reported that
test_tls_12_or_higher_is_enforced_for_productfailed with an opaqueSSL: CERTIFICATE_VERIFY_FAILEDerror behind an SSL-terminating proxy. That specific test lives insrc/vip_tests/cross_product/test_ssl.pyand was already fixed by #198 (commitc057b80), which classifies handshake outcomes and surfaces a trust-bundle hint when verification fails.This PR applies the same classification to the sibling
inspect_headersstep insrc/vip_tests/security/test_https.py, which was catchinghttpx.ConnectErrorand reporting "connection refused" for all failure modes — including cert-verify failures thathttpxwraps insideConnectError. The test now detectsssl.SSLCertVerificationError(via__cause__and aCERTIFICATE_VERIFY_FAILEDsubstring fallback) and skips with a clear message pointing toSSL_CERT_FILEand common CA bundle paths (Debian/Ubuntu, RHEL, andpython -m certifi).Test plan
just checkpassesuv run pytest selftests/passes (303/303 locally)SSL_CERT_FILEpointed at a self-signed CA that doesn't trust the ACM cert,test_product_does_not_expose_sensitive_headers[Connect]skips with the new diagnostic: "Could not verify TLS certificate for Connect at ...: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate ... This is a certificate-trust issue on the test runner, not a server security finding. If the server uses a valid public certificate (e.g. behind an AWS ALB with an ACM cert), set SSL_CERT_FILE to a CA bundle that includes public roots: /etc/ssl/certs/ca-certificates.crt on Debian/Ubuntu, /etc/pki/tls/certs/ca-bundle.crt on RHEL, or the path produced bypython -m certifi."SSL_CERT_FILEunset (valid default CA bundle), the same test PASSES against Connect — the cert-verify branch does not swallow legitimate test runsFixes #175