feat(admin): give Admin/stats credentials their own credential scope (PROXYSQL31) - #5993
Conversation
…assword
MySQL_Protocol::PPHR_5passwordFalse_0() is the only code path that
authenticates the 'mysql-monitor_username' / 'mysql-monitor_password'
credential: that credential is not stored in GloMyAuth, so
PPHR_verify_password() reaches it through its 'vars1.password == NULL' branch
for ADMIN / STATS / SQLITE sessions.
It was hardcoded to the mysql_native_password scramble -- a SHA1
'proxy_scramble' compared over SHA_DIGEST_LENGTH bytes. Under
caching_sha2_password the client's fast-auth response is a 32-byte
SHA256-derived value, so the comparison could never succeed and the credential
was rejected outright. Kubernetes liveness/readiness probes and metrics
exporters connecting to the Admin interface failed with 'Access denied'. TLS was
not a workaround either: the function returned false without ever sending
'perform full authentication', so the transport was irrelevant.
Dispatch on auth_plugin_id instead. The caching_sha2 fast-auth verification
already existed in PPHR_6auth2(); it is extracted into
caching_sha2_fast_auth_verify() so there is exactly one implementation of the
algorithm, and PPHR_6auth2() now calls it. A successful caching_sha2 fast auth
also needs the 'fast_auth_success' (0x03) marker before the OK packet, mirroring
what the PPHR_6auth2() call site does.
Neither comparison is length-gated, and both now carry a comment saying why.
'vars1.pass_len' is NOT the amount of valid data in 'vars1.pass': PPHR_2 strips
a trailing NUL byte ("remove the extra 0 if present"), so a legitimate 32-byte
caching_sha2 response ending in 0x00 -- about 1 in 256 -- arrives with
pass_len == 31 while all 32 bytes are present. An earlier revision of this
change gated on pass_len and caused intermittent 'Access denied' on the
frontend; test_auth_methods-t reproduced it at 20 spurious denials across ~6520
connections.
Also stop reporting the caching_sha2 'request_public_key' packet (a 1-byte 0x02
at switching_auth_stage 5) as "client is disconnecting during switch auth". It
is a request for an RSA public key ProxySQL does not serve; the log line now
names that. Client-visible messaging needs the session-level error path and is
left to the RSA work.
Verification:
- test/repro/reg_test_5363_admin_monitor_caching_sha2.bash goes from exit 1
(2 assertions tagged [BUG #5363]) to exit 0, 10/10, on a cold instance
- new reg_test_5363_admin_monitor_caching_sha2-t passes 12/12, and was
confirmed to FAIL exactly assertions 7 and 8 with errno 1045 when the fix is
reverted and rebuilt
- test_auth_methods-t (40194 assertions) passes; reg_test_4935-caching_sha2-t
and the full no-infra-g1 group pass
Fixes #5363
…(PROXYSQL31)
admin-admin_credentials and admin-stats_credentials shared USERNAME_FRONTEND
with mysql_users -- one flat map keyed by username -- so an Admin credential
and a row of the same name overwrote each other. That is why the docs state
those users cannot also appear in mysql_users. The restriction exists only
because of the shared map: the two are used on different ports, for different
session types, and are never interchangeable.
Add a third credential scope, USERNAME_ADMIN, selected via ADMIN_CRED_SCOPE:
#ifdef PROXYSQL31
#define ADMIN_CRED_SCOPE USERNAME_ADMIN
#else
#define ADMIN_CRED_SCOPE USERNAME_FRONTEND
#endif
This is an INCOMPATIBLE change -- a colliding name currently resolves to one
entry and afterwards the two are independent -- so it is gated to the
Innovative tier. On the stable tier ADMIN_CRED_SCOPE is USERNAME_FRONTEND,
every call site passes exactly what it passed before, and behaviour is
unchanged.
Both collision directions are covered, including the one not in the original
report: SET admin-admin_credentials calls delete_credentials(), which issued an
unconditional GloMyAuth->del() by username and so removed a same-named
mysql_users row from the runtime auth map until the next
LOAD MYSQL USERS TO RUNTIME. With separate scopes that cannot happen.
The nine `usertype==USERNAME_BACKEND ? creds_backends : creds_frontends`
ternaries are replaced by MySQL_Authentication::creds_for(). creds_admins is
never walked by dump_all_users(), so runtime_mysql_users and the cluster
checksum are untouched -- admin credentials were already excluded there by the
`default_hostgroup >= 0` filter.
Verified on both tiers, same scenario (admin credential 'dual:adminpass' plus a
mysql_users row 'dual' with a different password):
PROXYSQL31 : :6032 with the admin password -> ADMIN_OK; with the row's
password -> denied; the row keeps its own identity in
runtime_mysql_users; SET admin-admin_credentials no longer
deletes it
stable : :6032 with the admin password -> Access denied, i.e. the
documented collision behaviour is preserved unchanged
Refs #5987
📝 WalkthroughWalkthroughThe PR adds separate Admin credential scopes for MySQL and PostgreSQL, routes Admin and Stats sessions to those scopes, expands monitor authentication support, and adds regression tests for Admin ChangesAdmin authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp (1)
71-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
case_tto a PascalCase type name.Use a name such as
TestCase. Update the vector declaration and loop references.As per coding guidelines, class names must use
PascalCase.🤖 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 `@test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp` around lines 71 - 77, Rename the case_t struct to the PascalCase name TestCase, then update the associated vector declaration and loop references to use TestCase consistently.Source: Coding guidelines
lib/MySQL_Protocol.cpp (1)
2344-2348: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse a constant-time comparison for the authentication tag.
memcmpcan return early on a differing byte, so the runtime leaks how many leading bytes of the client response match the server recomputed tag. Replace this comparison with a constant-time HMAC/tag comparison API.🤖 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 `@lib/MySQL_Protocol.cpp` around lines 2344 - 2348, Replace the memcmp call in the authentication verification flow with the project’s constant-time HMAC or tag comparison API, comparing e against client_response for SHA256_DIGEST_LENGTH bytes. Preserve the existing boolean success result and tag computation loop.
🤖 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 `@lib/MySQL_Authentication.cpp`:
- Around line 60-76: The reset flow in MySQL_Authentication::reset must also
release Admin credentials before MySQL_Authentication::~MySQL_Authentication
deletes creds_admins.cred_array. Under the PROXYSQL31 configuration, invoke
_reset(USERNAME_ADMIN) (or an equivalent scoped Admin reset) alongside the
existing backend and frontend resets, ensuring Admin account details are freed
without changing other cleanup behavior.
In `@lib/MySQL_Protocol.cpp`:
- Around line 2133-2166: Prevent unbounded auth-response reads by making the
length requirement part of caching_sha2_fast_auth_verify’s contract: update its
declaration, definition, and all callers to accept and validate the response
length before reading SHA256_DIGEST_LENGTH bytes. In lib/MySQL_Protocol.cpp
lines 2133-2166, require vars1.pass to be non-null and use vars1.pass_len + 1 as
the supplied length, preserving the one-byte PPHR_2 shortfall allowance; also
gate the native-password memcmp on vars1.pass != NULL and vars1.pass_len + 1 >=
SHA_DIGEST_LENGTH. In lib/MySQL_Protocol.cpp lines 2357-2361, pass the available
response length through PPHR_6auth2 to caching_sha2_fast_auth_verify; no direct
fixed-size read should occur without the bound.
- Line 1261: Update the set_SHA1 write-back calls in the session authentication
flow and verify_user_pass to pass cred_scope_for_session(session_type) instead
of USERNAME_FRONTEND. Keep the write-back scope aligned with the corresponding
credential lookup for ADMIN, STATS, and frontend sessions, including all
referenced set_SHA1 call sites.
In `@test/repro/README.md`:
- Around line 39-42: Update the fenced code block in the README to specify the
text language identifier, preserving its documented environment-variable
examples unchanged.
In `@test/repro/reg_test_5363_admin_monitor_caching_sha2.bash`:
- Around line 155-157: Update the assertion using adm in the runtime_mysql_users
check to query only rows where username is 'monitor', matching the TAP
counterpart. Preserve the existing ok/nok messages and ensure unrelated users no
longer cause the test to fail or return exit code 2.
- Around line 146-151: Update restore() in
test/repro/reg_test_5363_admin_monitor_caching_sha2.bash (lines 146-151) to save
and restore the original mysql-monitor_username, mysql-monitor_password, and
mysql-default_authentication_plugin values, without overwriting unchanged
admin-admin_credentials. In
test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp (lines 110-117),
escape dynamic SQL literals before restoring configuration. In the same file
(lines 198-220), include mysql-monitor_username in the saved/restored variables
and assert that it is restored.
In `@test/repro/reg_test_5985_admin_caching_sha2_full_auth.bash`:
- Around line 138-143: Update the test’s setup and restore flow around restore()
to capture the original admin-admin_credentials and
mysql-default_authentication_plugin values before modification, then restore
those exact saved values in the exit handler instead of BASE_CREDS and
mysql_native_password. Ensure restoration failure causes the run to fail, while
preserving the existing runtime variable reloads and handling COLD_START=0
without altering the instance’s original configuration.
In `@test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp`:
- Around line 174-175: Remove plaintext credential values from the diagnostic
output around the existing diag calls: update the saved mysql-monitor_password
logging and the admin-admin_credentials logging to report only whether
credentials are present or use a redacted value, while retaining the
mysql-default_authentication_plugin value.
---
Nitpick comments:
In `@lib/MySQL_Protocol.cpp`:
- Around line 2344-2348: Replace the memcmp call in the authentication
verification flow with the project’s constant-time HMAC or tag comparison API,
comparing e against client_response for SHA256_DIGEST_LENGTH bytes. Preserve the
existing boolean success result and tag computation loop.
In `@test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp`:
- Around line 71-77: Rename the case_t struct to the PascalCase name TestCase,
then update the associated vector declaration and loop references to use
TestCase consistently.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ac931073-7daa-4a07-8b7d-f6b20357a43c
📒 Files selected for processing (10)
include/MySQL_Authentication.hppinclude/proxysql_structs.hlib/MySQL_Authentication.cpplib/MySQL_Protocol.cpplib/ProxySQL_Admin.cpptest/repro/README.mdtest/repro/reg_test_5363_admin_monitor_caching_sha2.bashtest/repro/reg_test_5985_admin_caching_sha2_full_auth.bashtest/tap/groups/groups.jsontest/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: run / trigger
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (4)
include/**/*.h
📄 CodeRabbit inference engine (CLAUDE.md)
Header include guards use the
#ifndef __CLASS_*_Hconvention.
Files:
include/proxysql_structs.h
**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{cpp,h,hpp}: Class names must usePascalCasewith protocol prefixes such asMySQL_,PgSQL_, andProxySQL_.
Member variables must usesnake_case.
Constants and macros must useUPPER_SNAKE_CASE.
Use C++17, and gate conditional code with#ifdef PROXYSQL31,#ifdef PROXYSQL40,#ifdef PROXYSQLFFTO,#ifdef PROXYSQLTSDB, and#ifdef PROXYSQLCLICKHOUSE;PROXYSQLGENAImust not guard core code outsideplugins/genai/.
Consider performance implications when changing hot paths or other performance-critical code.
Use RAII for resource management and jemalloc for allocation.
Use pthread mutexes for synchronization andstd::atomic<>for counters.
Files:
include/proxysql_structs.hlib/ProxySQL_Admin.cppinclude/MySQL_Authentication.hpptest/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpplib/MySQL_Authentication.cpplib/MySQL_Protocol.cpp
include/**/*.hpp
📄 CodeRabbit inference engine (CLAUDE.md)
Header include guards use the
#ifndef __CLASS_*_Hconvention.
Files:
include/MySQL_Authentication.hpp
test/tap/tests/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
test/tap/tests/**/*.cpp: Test files intest/tap/tests/must follow the naming patterntest_*.cppor*-t.cpp.
To add a new TAP test, add the<testname>-t.cppfile and register it intest/tap/tests/Makefile/groups.json; no special Makefile target is needed becausemake <testname>-tis generated by pattern rule.
Files:
test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp
🧠 Learnings (3)
📚 Learning: 2026-04-11T13:17:55.508Z
Learnt from: renecannao
Repo: sysown/proxysql PR: 5607
File: doc/GH-Actions/README.md:13-18
Timestamp: 2026-04-11T13:17:55.508Z
Learning: When using GitHub-flavored Markdown headings, be aware that an em-dash surrounded by spaces (written as ` — `) affects the generated anchor/slug: GitHub replaces spaces with hyphens and removes non-alphanumeric punctuation, which can produce double hyphens (e.g., `## Foo — bar` → anchor `#foo--bar`, not `#foo-bar`). If you reference these anchors (e.g., internal links), ensure the expected slug matches this behavior.
Applied to files:
test/repro/README.md
📚 Learning: 2026-04-11T13:17:55.509Z
Learnt from: renecannao
Repo: sysown/proxysql PR: 5607
File: doc/GH-Actions/README.md:13-18
Timestamp: 2026-04-11T13:17:55.509Z
Learning: When reviewing GitHub-flavored Markdown links/anchors, remember that heading-to-anchor slug generation treats spaces as hyphens and removes punctuation. If a heading contains an em-dash surrounded by spaces (e.g. ` — `), the slugs can legitimately include a double hyphen where the two surrounding space-runs become `-` on either side of the removed em-dash (e.g. `...vocabulary--read...`). Do not flag double-hyphens in anchor links for em-dash-containing headings as errors; they reflect GitHub’s correct slug behavior.
Applied to files:
test/repro/README.md
📚 Learning: 2026-01-20T09:34:19.124Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48
Timestamp: 2026-01-20T09:34:19.124Z
Learning: In ProxySQL's TAP test suite, resource leaks (e.g., not calling mysql_close() on early return paths) are commonly tolerated because test processes are short-lived and OS frees resources on exit. This pattern applies to all C++ test files under test/tap/tests. When reviewing, recognize this as a project-wide test convention and focus on test correctness and isolation rather than insisting on fixing such leaks in these test files.
Applied to files:
test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp
🪛 Cppcheck (2.21.0)
lib/MySQL_Authentication.cpp
[warning] 138-138: If memory allocation fails, then there is a possible null pointer dereference
(nullPointerOutOfMemory)
lib/MySQL_Protocol.cpp
[warning] 2348-2348: Uninitialized variable
(uninitvar)
🪛 markdownlint-cli2 (0.23.2)
test/repro/README.md
[warning] 39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
test/repro/reg_test_5363_admin_monitor_caching_sha2.bash
[warning] 109-109: This assignment is only seen by the forked process.
(SC2097)
[warning] 110-110: This expansion will not see the mentioned assignment.
(SC2098)
[warning] 113-113: This assignment is only seen by the forked process.
(SC2097)
[warning] 114-114: This expansion will not see the mentioned assignment.
(SC2098)
[warning] 118-118: This assignment is only seen by the forked process.
(SC2097)
[warning] 119-119: This expansion will not see the mentioned assignment.
(SC2098)
[info] 146-150: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
[info] 156-156: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 159-159: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 166-166: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 173-173: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 176-176: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 183-183: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 187-187: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 190-190: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 198-198: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 201-201: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
test/repro/reg_test_5985_admin_caching_sha2_full_auth.bash
[warning] 92-92: This assignment is only seen by the forked process.
(SC2097)
[warning] 93-93: This expansion will not see the mentioned assignment.
(SC2098)
[warning] 98-98: This assignment is only seen by the forked process.
(SC2097)
[warning] 99-99: This expansion will not see the mentioned assignment.
(SC2098)
[warning] 103-103: This assignment is only seen by the forked process.
(SC2097)
[warning] 104-104: This expansion will not see the mentioned assignment.
(SC2098)
[info] 138-143: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
[info] 148-148: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 151-151: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 164-164: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 193-193: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 197-197: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 203-203: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 208-208: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 213-213: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 217-217: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
[info] 221-221: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
🔇 Additional comments (12)
test/tap/groups/groups.json (1)
276-276: LGTM!include/proxysql_structs.h (1)
50-54: LGTM!include/MySQL_Authentication.hpp (1)
68-89: LGTM!Also applies to: 99-109
lib/MySQL_Authentication.cpp (2)
124-148: LGTM!
102-102: LGTM!Also applies to: 507-507, 550-550, 587-587, 655-655, 712-712, 804-804
lib/ProxySQL_Admin.cpp (2)
3970-3978: LGTM!
3935-3940: 🔒 Security & PrivacyNo change needed.
add_admin_users()re-adds the currentadmin_credentialsandstats_credentialsduring__refresh_users, and the same credential updates inset_variable()delete the old values before adding the new ones.lib/MySQL_Protocol.cpp (5)
63-81: LGTM!
1483-1483: LGTM!Also applies to: 3237-3237
1718-1740: LGTM!
2095-2100: LGTM!
2168-2202: LGTM!
| #endif /* PROXYSQLCLICKHOUSE */ | ||
| } else { | ||
| account_details = GloMyAuth->lookup((char*)userinfo->username, USERNAME_FRONTEND, dup_details); | ||
| account_details = GloMyAuth->lookup((char*)userinfo->username, cred_scope_for_session(session_type), dup_details); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The credential lookup scope and the set_SHA1 write-back scope now diverge.
This lookup resolves against cred_scope_for_session(session_type), so an ADMIN or STATS session reads from USERNAME_ADMIN when PROXYSQL31 is defined. The matching write-back at line 1282 still passes USERNAME_FRONTEND:
GloMyAuth->set_SHA1((char *)userinfo->username, USERNAME_FRONTEND, reply);
The same mismatch exists in verify_user_pass, which is reached from the changed lookup at line 1483. Line 1358 admits PROXYSQL_SESSION_ADMIN and PROXYSQL_SESSION_STATS, and line 1369 then writes with USERNAME_FRONTEND.
Two outcomes follow under PROXYSQL31:
- If no
mysql_usersrow shares the name,set_SHA1finds no entry, returns false, and the Admin credential never caches its SHA1. The derivation repeats on every connection. - If a
mysql_usersrow does share the name, the Admin session writes a SHA1 derived from the Admin password onto that unrelated frontend account. That is the cross-scope contamination this PR removes elsewhere.
Pass the session scope to these set_SHA1 calls, in the same way the lookups now do.
🔒️ Proposed fix at line 1282
if (account_details.sha1_pass==NULL) {
// currently proxysql doesn't know any sha1_pass for that specific user, let's set it!
- GloMyAuth->set_SHA1((char *)userinfo->username, USERNAME_FRONTEND,reply);
+ GloMyAuth->set_SHA1((char *)userinfo->username, cred_scope_for_session(session_type), reply);
}Apply the same change at lines 1351 and 1369 in verify_user_pass, which already receives session_type.
🤖 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 `@lib/MySQL_Protocol.cpp` at line 1261, Update the set_SHA1 write-back calls in
the session authentication flow and verify_user_pass to pass
cred_scope_for_session(session_type) instead of USERNAME_FRONTEND. Keep the
write-back scope aligned with the corresponding credential lookup for ADMIN,
STATS, and frontend sessions, including all referenced set_SHA1 call sites.
| bool verified = false; | ||
|
|
||
| switch (auth_plugin_id) { | ||
| case AUTH_MYSQL_NATIVE_PASSWORD: | ||
| proxy_scramble(reply, (*myds)->myconn->scramble_buff, mysql_thread___monitor_password); | ||
| // NOTE: do NOT gate this on 'vars1.pass_len == SHA_DIGEST_LENGTH'. | ||
| // 'pass_len' is not the amount of valid data in 'vars1.pass': PPHR_2 | ||
| // strips a trailing NUL byte from the client's response | ||
| // ("remove the extra 0 if present"), so a legitimate 20-byte native | ||
| // response whose last byte is 0x00 -- about 1 in 256 -- arrives with | ||
| // pass_len == 19 while all 20 bytes are present in the buffer. | ||
| verified = (memcmp(reply, vars1.pass, SHA_DIGEST_LENGTH) == 0); | ||
| break; | ||
|
|
||
| case AUTH_MYSQL_CACHING_SHA2_PASSWORD: | ||
| if ((*myds)->switching_auth_stage == 5) { | ||
| // A full-auth round trip was driven by another path (e.g. pass-through | ||
| // auth), so 'vars1.pass' already holds the cleartext. | ||
| verified = | ||
| (vars1.pass != NULL) && | ||
| (strcmp(mysql_thread___monitor_password, (const char *)vars1.pass) == 0); | ||
| } else { | ||
| verified = caching_sha2_fast_auth_verify( | ||
| mysql_thread___monitor_password, (*myds)->myconn->scramble_buff, | ||
| vars1.pass | ||
| ); | ||
| } | ||
| break; | ||
|
|
||
| case AUTH_MYSQL_CLEAR_PASSWORD: | ||
| verified = | ||
| (vars1.pass != NULL) && | ||
| (strcmp(mysql_thread___monitor_password, (const char *)vars1.pass) == 0); | ||
| break; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Unbounded fixed-size reads of the client auth response in lib/MySQL_Protocol.cpp. vars1.pass is a heap buffer of pass_len+1 bytes and the client controls pass_len. Both call sites read a fixed 20 or 32 bytes from it, and neither establishes a lower bound, so a short client response causes an out-of-bounds heap read. The shared root cause is one missing length gate before the fixed-size comparisons.
lib/MySQL_Protocol.cpp#L2133-L2166: gate theAUTH_MYSQL_NATIVE_PASSWORDmemcmponvars1.pass != NULL && vars1.pass_len + 1 >= SHA_DIGEST_LENGTH, and gate thecaching_sha2_fast_auth_verifycall onvars1.pass_len + 1 >= SHA256_DIGEST_LENGTH. Allow the one-byte shortfall that PPHR_2 introduces when it strips a trailing NUL.lib/MySQL_Protocol.cpp#L2357-L2361: apply the sameSHA256_DIGEST_LENGTHlower bound beforePPHR_6auth2passesvars1.passtocaching_sha2_fast_auth_verify, or add aresponse_lenparameter to the helper so every caller must supply the bound.
Adding the length parameter to caching_sha2_fast_auth_verify is the more durable fix. It makes the bound part of the contract instead of a rule each caller must remember.
📍 Affects 1 file
lib/MySQL_Protocol.cpp#L2133-L2166(this comment)lib/MySQL_Protocol.cpp#L2357-L2361
🤖 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 `@lib/MySQL_Protocol.cpp` around lines 2133 - 2166, Prevent unbounded
auth-response reads by making the length requirement part of
caching_sha2_fast_auth_verify’s contract: update its declaration, definition,
and all callers to accept and validate the response length before reading
SHA256_DIGEST_LENGTH bytes. In lib/MySQL_Protocol.cpp lines 2133-2166, require
vars1.pass to be non-null and use vars1.pass_len + 1 as the supplied length,
preserving the one-byte PPHR_2 shortfall allowance; also gate the
native-password memcmp on vars1.pass != NULL and vars1.pass_len + 1 >=
SHA_DIGEST_LENGTH. In lib/MySQL_Protocol.cpp lines 2357-2361, pass the available
response length through PPHR_6auth2 to caching_sha2_fast_auth_verify; no direct
fixed-size read should occur without the bound.
Source: Linters/SAST tools
| ``` | ||
| COLD_START=0 (default) use the existing ProxySQL instance | ||
| COLD_START=1 destroy and recreate it first | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
markdownlint reports MD040 for this block. Use text because the block documents environment-variable values.
Proposed fix
-```
+```text
COLD_START=0 (default) use the existing ProxySQL instance
COLD_START=1 destroy and recreate it first📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| COLD_START=0 (default) use the existing ProxySQL instance | |
| COLD_START=1 destroy and recreate it first | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@test/repro/README.md` around lines 39 - 42, Update the fenced code block in
the README to specify the text language identifier, preserving its documented
environment-variable examples unchanged.
Source: Linters/SAST tools
| restore() { | ||
| setvar 'admin-admin_credentials' "$BASE_CREDS" 'LOAD ADMIN VARIABLES TO RUNTIME;' | ||
| setvar 'mysql-default_authentication_plugin' 'mysql_native_password' 'LOAD MYSQL VARIABLES TO RUNTIME;' | ||
| setvar 'mysql-monitor_password' 'monitor' 'LOAD MYSQL VARIABLES TO RUNTIME;' | ||
| } | ||
| trap restore EXIT |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restore the exact shared runtime state after each test.
Both tests modify Admin global variables. Their restoration paths can leave the shared ProxySQL instance in a changed state.
test/repro/reg_test_5363_admin_monitor_caching_sha2.bash#L146-L151: save and restore the original monitor username, monitor password, and authentication plugin. Do not overwrite unmodified Admin credentials.test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp#L110-L117: escape dynamic SQL literal values before restoring configuration.test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp#L198-L220: save and restoremysql-monitor_usernamewith the other modified variables, then assert restoration.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 146-150: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
📍 Affects 2 files
test/repro/reg_test_5363_admin_monitor_caching_sha2.bash#L146-L151(this comment)test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp#L110-L117test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp#L198-L220
🤖 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 `@test/repro/reg_test_5363_admin_monitor_caching_sha2.bash` around lines 146 -
151, Update restore() in
test/repro/reg_test_5363_admin_monitor_caching_sha2.bash (lines 146-151) to save
and restore the original mysql-monitor_username, mysql-monitor_password, and
mysql-default_authentication_plugin values, without overwriting unchanged
admin-admin_credentials. In
test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp (lines 110-117),
escape dynamic SQL literals before restoring configuration. In the same file
(lines 198-220), include mysql-monitor_username in the saved/restored variables
and assert that it is restored.
| [ "$(adm 'SELECT count(*) FROM runtime_mysql_users;')" = "0" ] \ | ||
| && ok "no mysql_users rows (nothing can shadow the monitor credential)" \ | ||
| || nok "mysql_users is not empty -- result would be confounded" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check only for the conflicting username.
Rows for users other than monitor do not affect this test path. The empty-table check rejects valid shared instances and changes the result to exit code 2.
Query runtime_mysql_users with WHERE username='monitor', as the TAP counterpart does.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 156-156: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
🤖 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 `@test/repro/reg_test_5363_admin_monitor_caching_sha2.bash` around lines 155 -
157, Update the assertion using adm in the runtime_mysql_users check to query
only rows where username is 'monitor', matching the TAP counterpart. Preserve
the existing ok/nok messages and ensure unrelated users no longer cause the test
to fail or return exit code 2.
| restore() { | ||
| adm "UPDATE global_variables SET variable_value='${BASE_CREDS}' WHERE variable_name='admin-admin_credentials'; | ||
| LOAD ADMIN VARIABLES TO RUNTIME; | ||
| UPDATE global_variables SET variable_value='mysql_native_password' WHERE variable_name='mysql-default_authentication_plugin'; | ||
| LOAD MYSQL VARIABLES TO RUNTIME;" >/dev/null | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restore the original Admin configuration.
Line 139 always replaces admin-admin_credentials with BASE_CREDS. Line 141 always replaces the prior authentication plugin with mysql_native_password.
When COLD_START=0, this script modifies the existing instance after completion. It can replace its Admin access-control configuration with known test credentials and change later authentication behavior. Read and save both original values before the test. Restore those exact values in the exit handler. Also fail the run if restoration fails. The related TAP test already preserves its original values.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 138-143: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
🤖 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 `@test/repro/reg_test_5985_admin_caching_sha2_full_auth.bash` around lines 138
- 143, Update the test’s setup and restore flow around restore() to capture the
original admin-admin_credentials and mysql-default_authentication_plugin values
before modification, then restore those exact saved values in the exit handler
instead of BASE_CREDS and mysql_native_password. Ensure restoration failure
causes the run to fail, while preserving the existing runtime variable reloads
and handling COLD_START=0 without altering the instance’s original
configuration.
| diag("Saved mysql-default_authentication_plugin='%s', mysql-monitor_password='%s'", | ||
| orig_plugin.c_str(), orig_mon_pass.c_str()); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not write credentials to TAP output.
Line 174 logs the original mysql-monitor_password. Lines 194-195 log the complete admin-admin_credentials value. TAP logs can be retained as CI artifacts.
Log only the plugin, the credential-presence result, or a redacted value.
Also applies to: 193-195
🤖 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 `@test/tap/tests/reg_test_5363_admin_monitor_caching_sha2-t.cpp` around lines
174 - 175, Remove plaintext credential values from the diagnostic output around
the existing diag calls: update the saved mysql-monitor_password logging and the
admin-admin_credentials logging to report only whether credentials are present
or use a redacted value, while retaining the mysql-default_authentication_plugin
value.
…licy Mirrors the MySQL change for GloPgAuth: ProxySQL_Admin::add_credentials() / delete_credentials() have a live SERVER_TYPE_PGSQL arm, and add_admin_users<SERVER_TYPE_PGSQL>() is called from __refresh_pgsql_users() with the same add-admin-then-add-users ordering, so admin credentials landed in GloPgAuth's frontend scope and collided with pgsql_users rows exactly as they did on the MySQL side. Leaving one protocol converted would have left the two asymmetric. PgSQL_Authentication gains creds_admins and creds_for(), replacing its eight copies of the backend/frontend ternary, and the GloPgAuth->lookup() in PgSQL_Protocol.cpp now resolves through the shared scope helper. ADMIN_CRED_SCOPE and cred_scope_for_session() move from MySQL_Authentication.hpp / a file-static in MySQL_Protocol.cpp into proxysql_structs.h, which is where both cred_username_type and proxysql_session_type already live and which both protocol implementations already include. The policy now exists once rather than being duplicated per protocol. Both tiers clean-build: `make clean && PROXYSQL31=1 make debug` and `make clean && make debug`. Refs #5987
|
Update: PgSQL arm implemented, and the scope policy is now shared rather than duplicated. You were right that it belongs here. Confirmed in code before implementing: What changed since the first commit:
Verified on
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@lib/PgSQL_Authentication.cpp`:
- Around line 35-44: Update PgSQL_Authentication::reset() to call
_reset(USERNAME_ADMIN) alongside the existing credential-group resets, before
the credential arrays are deleted, so admin records are released both during
reset() and destruction.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e37de018-70d0-4caa-ab71-123697c5c5a6
📒 Files selected for processing (7)
include/MySQL_Authentication.hppinclude/PgSQL_Authentication.hinclude/proxysql_structs.hlib/MySQL_Protocol.cpplib/PgSQL_Authentication.cpplib/PgSQL_Protocol.cpplib/ProxySQL_Admin.cpp
💤 Files with no reviewable changes (2)
- include/MySQL_Authentication.hpp
- lib/MySQL_Protocol.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ProxySQL_Admin.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: CI-builds / builds (debian12,-dbg)
- GitHub Check: run / trigger
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{cpp,h,hpp}: Class names must usePascalCasewith protocol prefixes such asMySQL_,PgSQL_, andProxySQL_.
Member variables must usesnake_case.
Constants and macros must useUPPER_SNAKE_CASE.
Use C++17, and gate conditional code with#ifdef PROXYSQL31,#ifdef PROXYSQL40,#ifdef PROXYSQLFFTO,#ifdef PROXYSQLTSDB, and#ifdef PROXYSQLCLICKHOUSE;PROXYSQLGENAImust not guard core code outsideplugins/genai/.
Consider performance implications when changing hot paths or other performance-critical code.
Use RAII for resource management and jemalloc for allocation.
Use pthread mutexes for synchronization andstd::atomic<>for counters.
Files:
lib/PgSQL_Protocol.cppinclude/proxysql_structs.hlib/PgSQL_Authentication.cppinclude/PgSQL_Authentication.h
include/**/*.h
📄 CodeRabbit inference engine (CLAUDE.md)
Header include guards use the
#ifndef __CLASS_*_Hconvention.
Files:
include/proxysql_structs.hinclude/PgSQL_Authentication.h
🔇 Additional comments (4)
include/proxysql_structs.h (1)
778-812: LGTM!include/PgSQL_Authentication.h (1)
72-81: LGTM!lib/PgSQL_Authentication.cpp (1)
29-33: LGTM!Also applies to: 51-52, 71-71, 93-117, 432-432, 472-472, 525-525, 560-560, 641-641
lib/PgSQL_Protocol.cpp (1)
907-910: LGTM!
| creds_backends.cred_array = new PtrArray(); | ||
| creds_frontends.cred_array = new PtrArray(); | ||
| creds_admins.cred_array = new PtrArray(); | ||
| }; | ||
|
|
||
| PgSQL_Authentication::~PgSQL_Authentication() { | ||
| reset(); | ||
| delete creds_backends.cred_array; | ||
| delete creds_frontends.cred_array; | ||
| delete creds_admins.cred_array; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reset the Admin credential group.
PgSQL_Authentication::~PgSQL_Authentication() calls reset(), but reset() does not call _reset(USERNAME_ADMIN). The new creds_admins records remain allocated when this object is destroyed. They also remain available if another caller uses reset() to clear credentials.
Add _reset(USERNAME_ADMIN) to PgSQL_Authentication::reset() before the credential arrays are deleted.
Proposed fix
bool PgSQL_Authentication::reset() {
_reset(USERNAME_BACKEND);
_reset(USERNAME_FRONTEND);
+ _reset(USERNAME_ADMIN);
return true;
}As per coding guidelines, use RAII for resource management.
🤖 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 `@lib/PgSQL_Authentication.cpp` around lines 35 - 44, Update
PgSQL_Authentication::reset() to call _reset(USERNAME_ADMIN) alongside the
existing credential-group resets, before the credential arrays are deleted, so
admin records are released both during reset() and destruction.
Source: Coding guidelines
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3.0 #5993 +/- ##
==========================================
+ Coverage 52.42% 52.44% +0.02%
==========================================
Files 472 472
Lines 143105 143173 +68
Branches 36164 36165 +1
==========================================
+ Hits 75022 75091 +69
- Misses 51237 51245 +8
+ Partials 16846 16837 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI regression — this PR breaks two TAP groups.
Both jobs ran to completion (25m49s / 20m17s), so it is test failures rather than an infra abort. I have not diagnosed it yet — the job logs aren't retrievable via Leading hypothesis to check first: something authenticates to Staying in draft until this is understood and fixed. Not merging on the assumption it's unrelated. For the record, the other failures on this PR are shared with all my other open PRs and are not from this change: |
…rite-back Two defects from the previous commits, both found by CodeRabbit on #5993. 1. reset() released only USERNAME_BACKEND and USERNAME_FRONTEND, while the destructor deletes creds_admins.cred_array. Under PROXYSQL31 the Admin accounts were handed to setAllInactive() without ever being released, leaking their account_details strings. Add _reset(USERNAME_ADMIN); on the stable tier the scope is empty so it is a no-op. 2. The lookups were rescoped via cred_scope_for_session() but the matching set_SHA1() write-backs still passed USERNAME_FRONTEND. Under PROXYSQL31 that diverges: an ADMIN/STATS session reads from USERNAME_ADMIN and writes to USERNAME_FRONTEND, so either the SHA1 is never cached (no frontend entry, the derivation repeats on every connection) or -- worse -- it is written onto an unrelated same-named mysql_users account, which is exactly the cross-scope contamination this work removes elsewhere. Five call sites now use cred_scope_for_session(). The sixth, in PPHR_5passwordFalse_auth2(), deliberately keeps USERNAME_FRONTEND: it is the LDAP path, which only ever backs frontend users and which an ADMIN/STATS session never reaches. Both tiers clean-build. Refs #5987
…l scope
test_simple_embedded_HTTP_server-t failed under PROXYSQL31 with HTTP 401 on
every request (CI-legacy-g9, CI-mysql84-g9):
not ok 1 - Response code: 401 for https://proxysql:6080
not ok 2 - Response code: 401 for .../stats?metric=system
not ok 3 - Response code: 401 for .../stats?metric=mysql
not ok 4 - Response code: 401 for .../stats?metric=cache
The embedded HTTP server authenticates the 'stats' account, which
add_credentials() populates from 'admin-stats_credentials' into
ADMIN_CRED_SCOPE. The handler still resolved it with USERNAME_FRONTEND, so
once ADMIN_CRED_SCOPE became USERNAME_ADMIN the lookup returned no password,
ad.password was NULL, and every request was rejected before the digest check.
On the stable tier ADMIN_CRED_SCOPE is USERNAME_FRONTEND, so this is a no-op
there; the failure was PROXYSQL31-only, which is why it showed up solely in
the genai-gcov jobs.
This was the last read site still on the old scope: every other credential
lookup already routes through cred_scope_for_session(), and SQLite3_Server's
admin/stats credentials are a separate sqliteserver-* subsystem.
Verified locally on a PROXYSQL31 debug build: mysql84-g9 now passes 31/31
(was 30/31).
|
Brings the branch up to date with v3.0 (14 commits, including the #5993 credential-scope work that already carried this branch's ca03c8d), and re-triggers a full CI run after the CI-mysql84-g9 job on 9421d44 wedged for 6 hours in test_ssl_fast_forward-3_libmariadb-t. That stall was not reproducible: the same commit ran mysql84-g9 locally in 26 minutes and the test passed 5/5 in isolation, four other g9 runs completed successfully inside the same window, and no GitHub Actions incident overlapped it. Cause still unknown; investigation paused. Merge verified: lib/MySQL_Protocol.cpp auto-merged keeping both v3.0's cred_scope_for_session() call sites and this branch's Session/DS argument fix; the repro scripts and TAP test keep their restore-exact-state versions. Builds clean under PROXYSQL31.
1 participant
Implements #5987. Draft — the MySQL side is complete and verified; the PgSQL arm and the TAP tests are not written yet. See "Outstanding" below.
Stacked on #5991 (branched from
fix/5363-monitor-caching-sha2, since both touchPPHR_verify_password). Rebase ontov3.0once that merges.What and why
admin-admin_credentials/admin-stats_credentialssharedUSERNAME_FRONTENDwithmysql_users— one flat map keyed by username — so an Admin credential and a row of the same name overwrote each other. That is the sole reason the documentation says those users cannot also appear inmysql_users. The two are used on different ports, for different session types, and are never interchangeable; the restriction is an artefact of the shared map, not a design constraint.This adds a third scope,
USERNAME_ADMIN, selected through one macro:Gated to the Innovative tier because it is an incompatible change: a colliding name currently resolves to a single entry, afterwards the two are independent. On the stable tier the macro is
USERNAME_FRONTEND, every call site passes exactly what it passed before, and behaviour is unchanged by construction.Both collision directions are fixed, including the one not in the original report:
SET admin-admin_credentialscallsdelete_credentials(), which issued an unconditionalGloMyAuth->del()by username and so removed a same-namedmysql_usersrow from the runtime auth map until the nextLOAD MYSQL USERS TO RUNTIME.Shape of the change
cred_username_typegainsUSERNAME_ADMIN(compiled unconditionally, used only underPROXYSQL31).MySQL_Authenticationgainscreds_adminspluscreds_for(), replacing nine copies ofusertype==USERNAME_BACKEND ? creds_backends : creds_frontends.ProxySQL_Admin::add_credentials()/delete_credentials()targetADMIN_CRED_SCOPE.GloMyAuth->lookup(...)sites inMySQL_Protocol.cppgo throughcred_scope_for_session(), which returnsADMIN_CRED_SCOPEforADMIN/STATSsessions andUSERNAME_FRONTENDotherwise.creds_adminsis never walked bydump_all_users(), soruntime_mysql_usersand the cluster checksum are untouched — admin credentials were already excluded there by thedefault_hostgroup >= 0filter atProxySQL_Admin.cpp:6852.Verified — both tiers, same scenario
Admin credential
dual:adminpass, plus amysql_usersrowdualwith a different passwordfrontpass:PROXYSQL31=1:6032withadminpass, no row:6032withadminpass, row presentAccess denied(documented collision, unchanged):6032withfrontpassruntime_mysql_usersSET admin-admin_credentialsdeletes the rowThe stable-tier row is the compatibility guarantee, and it was run, not inferred from the macro — the identity argument is obvious enough that I'd normally trust it, and obvious-looking reasoning about this exact code has already cost one regression today (see #5991).
Both tiers compile:
PROXYSQL31=1 make debugand plainmake debug, each aftermake clean.Outstanding — why this is a draft
add_credentials/delete_credentialsare templated overSERVER_TYPEand theSERVER_TYPE_PGSQLarm still writes toGloPgAuth's frontend scope. Give Admin/stats credentials their own scope, lifting the "cannot also be in mysql_users" restriction #5987 explicitly says not to skip it; leaving the two asymmetric is a future bug.@proxysql_min_version:3.1. Without both, gating silently drops coverage on one side.The behaviour above is real and reproducible, but I'd rather have it reviewed as a draft than have the missing PgSQL arm and tests read as complete.
Summary by CodeRabbit
New Features
caching_sha2_passwordmethods.Bug Fixes
CHANGE_USERauthentication flows.Documentation & Tests