Skip server certificate verification for MariaDB connections#334
Conversation
📝 WalkthroughWalkthroughMariaDB connections now skip server certificate verification by default unless explicitly overridden. MySQL option handling accepts both SSL verification flags, and a MySQL/MariaDB feature scenario verifies forwarding of the skip option. ChangesMariaDB SSL verification
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
MariaDB 11.4 enables TLS server certificate verification by default. WP-CLI connects the same way WordPress does, which does not verify the server certificate. Against MariaDB's auto-generated self-signed certificate, verification either prints a warning to STDERR that it disabled itself for the passwordless login or fails the connection outright. The warning fails the Behat tests on MariaDB and is noise for users running `wp db` commands. Explicitly pass `--skip-ssl-verify-server-cert` on MariaDB to keep the previous (pre-11.4) behaviour, unless the user opted into verification. Enabling verification instead would force certificate checks that break self-signed or untrusted certificates and older clients, so it is not the right default. Both `ssl-verify-server-cert` and its `skip-` variant are added to the allowed MySQL client options so the behaviour can be overridden per invocation.
d7d3a95 to
e0783e6
Compare
schlessera
changed the title
|
The remaining failures are due to #336, but to test that one, this one needs to be merged first. Merging this now as is with the remaining (unrelated) failures. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
features/db-query.feature (1)
122-139: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the default and positive-override paths.
This scenario always supplies
--skip-ssl-verify-server-cert; it never verifies the new no-flag MariaDB default,--ssl-verify-server-cert, or the newget_mysql_args()path used by import and query helpers. Add focused cases for those paths, keeping MariaDB-only default assertions separate from the MySQL-or-MariaDB scenario.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/db-query.feature` around lines 122 - 139, The existing scenario only covers the explicit skip flag; add focused scenarios for the no-flag MariaDB default and the positive --ssl-verify-server-cert override, keeping the default assertion MariaDB-only and the override assertion under MySQL-or-MariaDB. Also add coverage for the get_mysql_args() path used by import and query helpers, while preserving the current forwarded-flag debug assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/db-query.feature`:
- Around line 135-139: Update the scenario around the
`--skip-ssl-verify-server-cert` assertion to verify the spawned MySQL client’s
final argv rather than relying on `--debug` output from parsed arguments. Reuse
the existing test mechanism for inspecting client arguments and assert that the
SSL-skip option is present in the arguments passed to MySQL.
In `@src/DB_Command.php`:
- Around line 1991-2000: The helper queries get_current_sql_modes() and
get_posts_table_charset() currently pass only execute to self::run(), dropping
explicit SSL verification flags. Forward the caller’s SSL-related entries from
$assoc_args when invoking self::run() so both helper connections preserve
--ssl-verify-server-cert and --skip-ssl-verify-server-cert choices.
---
Nitpick comments:
In `@features/db-query.feature`:
- Around line 122-139: The existing scenario only covers the explicit skip flag;
add focused scenarios for the no-flag MariaDB default and the positive
--ssl-verify-server-cert override, keeping the default assertion MariaDB-only
and the override assertion under MySQL-or-MariaDB. Also add coverage for the
get_mysql_args() path used by import and query helpers, while preserving the
current forwarded-flag debug assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8698c509-6529-45c6-abc3-066db69f0760
📒 Files selected for processing (2)
features/db-query.featuresrc/DB_Command.php
| When I try `wp db query "SELECT 1" --skip-ssl-verify-server-cert --debug` | ||
| Then STDERR should contain: | ||
| """ | ||
| skip-ssl-verify-server-cert | ||
| """ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## features/db-query.feature (around lines 120-150)\n'
sed -n '120,150p' features/db-query.feature | cat -n
printf '\n## src/DB_Command.php around query and mysql args\n'
rg -n "function query|get_mysql_args|skip-ssl-verify-server-cert|run\(" src/DB_Command.php
sed -n '560,700p' src/DB_Command.php | cat -nRepository: wp-cli/db-command
Length of output: 8621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('src/DB_Command.php')
lines = path.read_text().splitlines()
for start,end in [(600,690)]:
print(f"\n## {path} lines {start}-{end}")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PY
printf '\n## search for debug/logging related to assoc args or command printing\n'
rg -n "assoc_args|debug|STDERR|run\(|mysql args|get_mysql_args|skip-ssl-verify-server-cert" src/DB_Command.php src -g '!vendor'Repository: wp-cli/db-command
Length of output: 38664
🌐 Web query:
WP-CLI DB_Command skip-ssl-verify-server-cert query assoc_args self::run get_mysql_args debug STDERR
💡 Result:
In WP-CLI's DB_Command implementation, the ssl-verify-server-cert flag and its associated handling are primarily used to manage SSL/TLS connection requirements and suppress specific warnings during database operations, particularly when using MariaDB [1][2]. Key details regarding the components you mentioned: 1. ssl-verify-server-cert: This flag is included in WP-CLI's internal allow-list for MySQL/MariaDB connection arguments [1][3]. When using MariaDB, WP-CLI may automatically inject --ssl-verify-server-cert into the database command to prevent "insecure passwordless login" warnings from cluttering STDERR during automated processes like Behat tests [1][2]. 2. get_mysql_args(): This method in DB_Command is responsible for filtering user-provided arguments against an allow-list of valid MySQL client options [4]. It ensures that flags like ssl-verify-server-cert, ssl, and skip-ssl are correctly passed to the underlying MySQL or MariaDB client binary [4]. 3. self::run() and debug: WP-CLI executes database commands by constructing a shell command and invoking it via self::run() [5][6]. When the --debug global flag is used, WP-CLI outputs the final, constructed MySQL command string and the associative arguments being passed, which is useful for verifying that flags like --ssl-verify-server-cert are being correctly applied [7][6][4]. 4. STDERR: In many environments, especially with MariaDB, SSL warnings are output to STDERR [1][2]. If these warnings occur during SQL mode discovery or connection establishment, they can sometimes cause WP-CLI commands to fail or produce unexpected output [7][2]. Technical Context and Known Issues: The handling of SSL and default configurations has been a point of development within WP-CLI [8][3]. Specifically, WP-CLI often uses a --no-defaults flag by default to avoid issues with misconfigured local environments, but this can prevent the MySQL client from respecting SSL configuration files [9][10][2]. Users encountering SSL connection failures (e.g., certificate verification errors) often need to use the --defaults flag or explicitly pass additional SSL-related arguments [9][3][10]. Recent PRs have focused on improving how wp db commands handle these arguments to ensure better compatibility with modern SSL/TLS-secured database environments [3][4].
Citations:
- 1: Add --ssl-verify-server-cert flag for MariaDB to suppress passwordless login warnings #310
- 2: Improve MariaDB tests #309
- 3: Try refactor
wp db query --defaultshandling #285 - 4: Support the --ssl and --skip-ssl flags in wp db import #332
- 5: https://github.com/wp-cli/db-command/blob/main/src/DB_Command.php
- 6: https://github.com/wp-cli/db-command/blob/master/src/DB_Command.php
- 7: Detect MariaDB vs MySQL wp-cli#6072
- 8: query with "--default"-flag fails when discovering the SQL mode #237
- 9: get_current_sql_modes() fails if TLS is being used. #278
- 10: https://core.trac.wordpress.org/ticket/63876
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('src/DB_Command.php')
lines = path.read_text().splitlines()
for start, end, title in [
(1969, 2025, 'run()'),
(2176, 2290, 'get_mysql_args()'),
]:
print(f"\n## {title} ({path} lines {start}-{end})")
for i in range(start - 1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: wp-cli/db-command
Length of output: 5999
Assert the spawned mysql argv
features/db-query.feature:16-19 — --debug can print the parsed assoc args before self::run(), so this check can pass without proving the option reached the MySQL client. Assert the final client arguments instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/db-query.feature` around lines 135 - 139, Update the scenario around
the `--skip-ssl-verify-server-cert` assertion to verify the spawned MySQL
client’s final argv rather than relying on `--debug` output from parsed
arguments. Reuse the existing test mechanism for inspecting client arguments and
assert that the SSL-skip option is present in the arguments passed to MySQL.
| // MariaDB 11.4 enables TLS server certificate verification by default. WP-CLI | ||
| // connects the same way WordPress does, which does not verify the certificate, | ||
| // and against MariaDB's auto-generated self-signed certificate verification | ||
| // either just warns on STDERR that it disabled itself or fails outright. Opt out | ||
| // explicitly to keep the previous behaviour, unless the user asked to verify. | ||
| if ( 'mariadb' === Utils\get_db_type() | ||
| && ! isset( $assoc_args['ssl-verify-server-cert'] ) | ||
| && ! isset( $assoc_args['skip-ssl-verify-server-cert'] ) ) { | ||
| $required['skip-ssl-verify-server-cert'] = true; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1960,2025p;835,860p;2360,2400p' src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 4415
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('src/DB_Command.php')
text = p.read_text()
for start,end in [(1960,2025),(835,860),(2360,2400)]:
print(f'--- {start}-{end} ---')
for i,line in enumerate(text.splitlines(),1):
if start <= i <= end:
print(f'{i}:{line}')
PYRepository: wp-cli/db-command
Length of output: 5106
🏁 Script executed:
rg -n "ssl-verify-server-cert|skip-ssl-verify-server-cert|get_current_sql_modes|get_posts_table_charset|function run\(" src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 859
🌐 Web query:
"src/DB_Command.php get_current_sql_modes get_posts_table_charset WP-CLI MariaDB skip-ssl-verify-server-cert"
💡 Result:
The components you identified are part of the wp-cli/db-command package, which manages database interactions for WP-CLI [1][2][3]. get_current_sql_modes This is a internal function used by WP-CLI to retrieve the active SQL modes of the MySQL/MariaDB session (via SELECT @@SESSION.sql_mode) to ensure compatibility with WordPress database operations [4][5]. It has been a source of various issues, particularly when connecting to databases using SSL/TLS, as the command internally forces specific flags (like --no-defaults) that may override necessary user configurations [1][4][6]. get_posts_table_charset This typically refers to logic or methods used to identify the character set of the WordPress posts table [2]. It is important for ensuring proper data handling and query execution, and fixes have been implemented in the past to correctly handle how these character sets are retrieved and processed to avoid command failures [2]. skip-ssl-verify-server-cert This relates to handling SSL/TLS certificate verification in MariaDB and MySQL connections [7]. Recent MariaDB versions (11.4+) enable SSL and --ssl-verify-server-cert by default [6][8]. This has caused significant friction in WP-CLI environments, leading to warnings or errors (such as TLS certificate verification failures or passwordless login warnings) [1][9][7][10]. To address this, updates have been made to the WP-CLI db-command package to: - Conditionally add --ssl-verify-server-cert for MariaDB connections to suppress insecure login warnings [7]. - Add ssl-verify-server-cert to the list of allowed MySQL options [3][7]. - Attempt to better preserve user-supplied connection arguments (like --defaults) which were previously stripped by the tool's internal commands, causing connection failures [3][6]. If you are encountering issues, ensure you are using a recent version of the wp-cli/db-command package, as these connection-related bugs are actively being addressed [3][7].
Citations:
- 1: get_current_sql_modes() fails if TLS is being used. #278
- 2: Fix retrieval of get_post_table_characterset #231
- 3: Try refactor
wp db query --defaultshandling #285 - 4: Unnecessary check of sql modes in query command #311
- 5: https://wp-kama.com/handbook/wp-cli/function/get_sql_modes
- 6: https://core.trac.wordpress.org/ticket/63876
- 7: Add --ssl-verify-server-cert flag for MariaDB to suppress passwordless login warnings #310
- 8: https://mariadb.com/docs/server/security/encryption/data-in-transit-encryption/zero-configuration-ssl
- 9: Detect MariaDB vs MySQL wp-cli#6072
- 10: Improve MariaDB tests #309
🏁 Script executed:
sed -n '900,1040p;1180,1260p;2200,2265p' src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 8498
🏁 Script executed:
rg -n "function get_defaults_flag_string|get_defaults_flag_string\(" src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 828
🏁 Script executed:
sed -n '520,700p;1040,1185p;2200,2265p' src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 13003
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
text = Path('src/DB_Command.php').read_text().splitlines()
for start,end in [(520,700),(1040,1185),(2200,2265)]:
print(f'--- {start}-{end} ---')
for i,line in enumerate(text,1):
if start <= i <= end:
print(f'{i}:{line}')
PYRepository: wp-cli/db-command
Length of output: 14839
🏁 Script executed:
rg -n "function get_defaults_flag_string|get_defaults_flag_string\(" src/DB_Command.php && sed -n '1260,1385p' src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 3946
🏁 Script executed:
sed -n '2280,2335p' src/DB_Command.phpRepository: wp-cli/db-command
Length of output: 1842
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
text = Path('src/DB_Command.php').read_text().splitlines()
for start,end in [(2280,2335)]:
print(f'--- {start}-{end} ---')
for i,line in enumerate(text,1):
if start <= i <= end:
print(f'{i}:{line}')
PYRepository: wp-cli/db-command
Length of output: 2140
Forward SSL flags into internal DB queries.
get_current_sql_modes() and get_posts_table_charset() build the command string with $assoc_args but pass only execute into self::run(), so an explicit --ssl-verify-server-cert is still dropped and MariaDB re-injects skip-ssl-verify-server-cert for those helper connections. Pass the caller’s SSL-related args through.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/DB_Command.php` around lines 1991 - 2000, The helper queries
get_current_sql_modes() and get_posts_table_charset() currently pass only
execute to self::run(), dropping explicit SSL verification flags. Forward the
caller’s SSL-related entries from $assoc_args when invoking self::run() so both
helper connections preserve --ssl-verify-server-cert and
--skip-ssl-verify-server-cert choices.
1 participant
Summary
MariaDB 11.4 enables TLS server certificate verification by default. WP-CLI
connects the same way WordPress does, which does not verify the server
certificate. Against MariaDB's auto-generated self-signed certificate the client
either prints a warning to STDERR:
or fails the connection outright. The warning fails the Behat tests on MariaDB
(#309) and is noise for anyone running
wp dbcommands against MariaDB.Approach
run()now passes--skip-ssl-verify-server-certon MariaDB, keeping thepre-11.4 behaviour where the server certificate is not verified (the same as
WordPress itself). The warning already reports that MariaDB disabled
verification for these connections, so making the opt-out explicit does not
change the effective security posture, it just silences the warning.
This is an alternative to #310, which added
--ssl-verify-server-cert(enablingverification). Enabling it turns on certificate checks that fail against
self-signed or untrusted certificates, and on clients older than 11.4 where
verification was off by default, so it can break real connections rather than
just silencing the warning. Users who do want verification can still pass
--ssl-verify-server-certthemselves, since both that flag and itsskip-variant are now allowed as MySQL client options.
Addresses #309. The remaining MariaDB failure (
wp db repair --verbose,a pre-existing MySQL-vs-MariaDB verbose-output difference) is tracked in #335.
Summary by CodeRabbit
wp db querycompatibility with MariaDB connections by handling SSL certificate verification consistently.