bito-code-review[bot] commented on PR #44910:
URL: https://github.com/apache/superset/pull/44910#issuecomment-5962052605
<!-- Bito Reply -->
The suggestion is valid. The current test
`test_require_mysql_tls_connects_with_verified_tls` only asserts that the
connection is encrypted (`assert cipher[1]`), which confirms TLS is in use but
does not verify that the client is actually validating the server's
certificate. A connection using unverified TLS (e.g., `ssl_mode=REQUIRED`
without a valid CA) would also result in an encrypted connection, potentially
masking a failure to verify the certificate.
To resolve this, you should add an assertion that verifies the connection
fails when an invalid or missing CA is provided, or explicitly check that the
connection parameters include the expected verification settings.
Would you like me to fetch all other comments on this PR to validate them
and implement a comprehensive fix?
**tests/testcontainers/db_engine_specs/test_mysql.py**
```
with tls_engine.connect() as conn:
cipher = conn.execute(text("SHOW STATUS LIKE
'Ssl_cipher'")).fetchone()
assert cipher is not None
assert cipher[1], "connection succeeded but is not actually using
TLS"
# Add verification check here, e.g.:
# assert conn.connection.ssl_verify_cert()
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
