Add --ssl-verify-server-cert flag for MariaDB to suppress passwordless login warnings#310
Add --ssl-verify-server-cert flag for MariaDB to suppress passwordless login warnings#310
Conversation
…s login warning Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively resolves an issue with MariaDB test failures caused by SSL warnings. The changes are well-targeted and correctly implemented. By conditionally adding the --ssl-verify-server-cert flag for MariaDB connections, you've addressed the root cause of the warnings while preserving the ability for users to override this behavior. The corresponding addition of ssl-verify-server-cert to the allowed options in get_mysql_args() is also correct. I have one minor suggestion regarding documenting the potential impact of this change for users in specific configurations.
| if ( 'mariadb' === Utils\get_db_type() && ! isset( $assoc_args['ssl-verify-server-cert'] ) ) { | ||
| $required['ssl-verify-server-cert'] = true; | ||
| } |
There was a problem hiding this comment.
This change correctly suppresses the MariaDB warnings and improves the default security posture. However, it's worth noting that this introduces a potential breaking change for users on MariaDB with passwordless logins and an invalid or untrusted server SSL certificate. Previously, their commands would succeed with a warning; now, they will fail due to the enforced SSL verification.
While users can opt-out by using --no-ssl-verify-server-cert, this change in default behavior could be surprising. I'd recommend adding a note about this to the relevant command documentation or the project's release notes to ensure users are aware of the change and how to manage it.
MariaDB tests were failing due to SSL warnings cluttering STDERR:
WARNING: option --ssl-verify-server-cert is disabled, because of an insecure passwordless login. These warnings trigger Behat test failures even when commands succeed.Changes
DB_Command::run(): Auto-add--ssl-verify-server-certflag when MariaDB is detected viaUtils\get_db_type()get_mysql_args(): Addssl-verify-server-certto allowed MySQL optionsThe flag is only added if not already present in user arguments, preserving override capability. MySQL installations are unaffected.
Mirrors the fix from wp-cli/wp-cli-tests#297, extending it to all db commands (export, import, query, etc.).
Original prompt
This section details on the original issue you should resolve
<issue_title>Improve MariaDB tests</issue_title>
<issue_description>They are still allowed to fail but would be nice to get them to green. There are currently quite a few failing tests, almost all because of
--ssl-verify-server-certwarnings cluttering STDERR.Perhaps we need to pass it manually like was done in wp-cli/wp-cli-tests#297?
Examples: