chore(plugin-chassis): tighten gating, drop dead paths, gate forgery setters

Five clean-up items from the independent review of PR #5651. None of
these change behaviour on a normal run; they each fix a concrete way
the current code is misleading or unnecessarily exposed.

1) lib/ProxySQL_Admin.cpp: gate the three plugin-DB-handle getters
   (proxysql_plugin_get_admindb / _configdb / _statsdb) under
   #ifdef PROXYSQL40.

   Previously these were defined unconditionally and emitted symbols
   into v3.0/v3.1 binaries. The chassis is a v4.0 feature; the user
   explicitly required that v3.x builds carry no plugin-aware code.
   Wrap in PROXYSQL40 so they are entirely absent from v3.x linkage.
   ProxySQL_PluginManager.cpp's extern declarations (lines 23-25) are
   already inside the file-wide PROXYSQL40 gate and so resolve only
   when the gate is active.

2) lib/ProxySQL_PluginManager.cpp + include/ProxySQL_PluginManager.h:
   delete the dead `#else /* !PROXYSQL40 */` branches.

   Both files are wrapped in a top-level `#ifdef PROXYSQL40` covering
   the entire body. Inner `#ifdef PROXYSQL40 ... #else ... #endif`
   blocks therefore had unreachable `#else` arms — 30+ lines of
   "pre-chassis two-phase loader" in the .cpp, plus a redundant
   declaration of proxysql_load_configured_plugins in the .h. The
   dead arms read as load-bearing alternative implementations on
   review and that is exactly the wrong signal. Drop them; the
   single PROXYSQL40 path is the only one.

3) lib/Admin_Bootstrap.cpp + include/proxysql_admin.h + src/main.cpp:
   remove ProxySQL_Admin::materialize_plugin_tables().

   `Admin::init()` already merges plugin-declared schemas into
   tables_defs_{admin,config,stats} (~line 944) and runs the DDL via
   check_and_build_standard_tables() (~line 994), all on the same
   first-boot/reload code path the core tables use. The follow-up
   call to GloAdmin->materialize_plugin_tables() in main.cpp ran a
   second name-dedup pass that found everything already present and
   produced an empty new-rows set — i.e. the post-init helper was a
   no-op disguised as load-bearing infrastructure. Delete the helper,
   the header declaration, and the main.cpp call site, and update
   the comments in main.cpp + ProxySQL_PluginManager.cpp to point at
   Admin::init() as the single canonical materialization site. Leave
   a NOTE in Admin_Bootstrap.cpp at the old call site so anyone
   re-adding a similar helper sees why the prior one was removed.

4) plugins/mysqlx/include/mysqlx_session.h +
   plugins/mysqlx/src/mysqlx_session.cpp: gate the two genuine
   forgery vectors behind MYSQLX_TEST_BUILD.

   inject_identity_for_test() bypasses the full auth flow — no
   credential check, no capability negotiation, just sets identity_
   to a caller-supplied MysqlxResolvedIdentity. resolve_backend_target_
   for_test() drives a private routing helper without an authenticated
   identity. Both are necessary for unit tests but should not be
   reachable at all in shipped binaries; an in-process exploit
   reaching the session can call inject_identity_for_test() to forge
   an authenticated identity. Wrap them in #ifdef MYSQLX_TEST_BUILD;
   define -DMYSQLX_TEST_BUILD in the test Makefile only. The remaining
   target_*_for_test() getters are read-only state observers and are
   left unconditional — they cannot mutate the session and a debugger
   could observe the same state regardless.

5) test/tap/tests/unit/Makefile: define -DMYSQLX_TEST_BUILD on every
   unit test compile line via OPT.

   This is the test-only knob that re-enables the gated forgery
   methods so unit tests still compile. plugins/mysqlx/Makefile does
   NOT define this macro, so the production .so does not compile in
   the entry points.

Verified locally:
- plugins/mysqlx/ProxySQL_MySQLX_Plugin.so builds clean with
  PROXYSQL40=1 PROXYSQL31=1 PROXYSQLFFTO=1 PROXYSQLTSDB=1
  PROXYSQLGENAI=1 (no MYSQLX_TEST_BUILD).
- test/tap/tests/unit/mysqlx_robustness_unit-t builds and runs:
  74/74 assertions pass, including the ones that exercise
  inject_identity_for_test (visible only because the test Makefile
  defines MYSQLX_TEST_BUILD).
fix/test-mysqlx-plugin-load-phase-b
Rene Cannao 4 weeks ago
parent 4bd4b462be
commit 04bccec51e

@ -124,7 +124,6 @@ bool proxysql_dispatch_configured_plugin_admin_command(
const std::string& sql,
ProxySQL_PluginCommandResult& result
);
#ifdef PROXYSQL40
bool proxysql_dispatch_configured_plugin_query_hook(
ProxySQL_PluginProtocol proto,
const ProxySQL_PluginQueryHookPayload& payload,
@ -160,14 +159,6 @@ bool proxysql_init_configured_plugins(
ProxySQL_PluginManager* manager,
std::string& err
);
#else /* !PROXYSQL40 */
// Pre-chassis two-phase load: dlopen + init_all in one call.
bool proxysql_load_configured_plugins(
std::unique_ptr<ProxySQL_PluginManager>& manager,
const std::vector<std::string>& plugin_modules,
std::string& err
);
#endif /* PROXYSQL40 */
bool proxysql_start_configured_plugins(
ProxySQL_PluginManager* manager,
std::string& err

@ -635,9 +635,6 @@ class ProxySQL_Admin {
* @return Always true.
*/
bool init(const bootstrap_info_t& bootstrap_info);
#ifdef PROXYSQL40
void materialize_plugin_tables();
#endif /* PROXYSQL40 */
void init_ldap();
/** @brief Initializes the HTTP server. For safety should be called after 'phase3'. */
void init_http_server();

@ -1347,64 +1347,15 @@ bool ProxySQL_Admin::init(const bootstrap_info_t& bootstrap_info) {
return true;
};
#ifdef PROXYSQL40
void ProxySQL_Admin::materialize_plugin_tables() {
ProxySQL_PluginManager* plugin_manager = proxysql_get_plugin_manager();
if (!plugin_manager) return;
// merge_new returns the subset of `defs` that were actually added
// (skipping entries whose table_name already exists in `target`).
// The caller runs DDL for only this subset so a repeat invocation
// would not re-execute CREATE TABLE statements for tables that
// already exist in the SQLite database.
auto merge_new = [this](
std::vector<table_def_t *>* target,
const std::vector<ProxySQL_PluginTableDef>& defs
) -> std::vector<const ProxySQL_PluginTableDef*> {
std::vector<const ProxySQL_PluginTableDef*> added;
added.reserve(defs.size());
for (const auto& def : defs) {
bool exists = false;
for (const auto* existing : *target) {
if (strcasecmp(existing->table_name, def.table_name) == 0) {
exists = true;
break;
}
}
if (!exists) {
insert_into_tables_defs(target, def.table_name, def.table_def);
added.push_back(&def);
}
}
return added;
};
const auto new_admin = merge_new(tables_defs_admin, plugin_manager->tables(ProxySQL_PluginDBKind::admin_db));
const auto new_config = merge_new(tables_defs_config, plugin_manager->tables(ProxySQL_PluginDBKind::config_db));
const auto new_stats = merge_new(tables_defs_stats, plugin_manager->tables(ProxySQL_PluginDBKind::stats_db));
// Plugin-declared DDL failures are startup-fatal: a partially
// materialized schema would only surface later as opaque runtime
// errors from the plugin itself. Log the plugin, table name, and
// SQLite message, then abort.
auto run_ddl = [](SQLite3DB* db, const std::vector<const ProxySQL_PluginTableDef*>& defs, const char* kind) {
for (const auto* def : defs) {
if (!db->execute(def->table_def)) {
proxy_error("Plugin %s-db DDL failed for table %s: %s\n",
kind,
def->table_name ? def->table_name : "(null)",
def->table_def ? def->table_def : "(null)");
exit(EXIT_FAILURE);
}
}
};
run_ddl(admindb, new_admin, "admin");
run_ddl(configdb, new_config, "config");
run_ddl(statsdb, new_stats, "stats");
// admindb<->configdb("disk") and admindb<->statsdb("stats") are
// already attached by init_sqlite3db() above this function in the
// startup flow; duplicate ATTACH errors out in SQLite. Do NOT
// re-attach them here.
}
#endif /* PROXYSQL40 */
// NOTE: materialize_plugin_tables() used to live here. It was removed
// because plugin tables are already merged into tables_defs_{admin,
// config,stats} by ProxySQL_Admin::init() (~line 944) and the DDL is
// already executed by check_and_build_standard_tables() (~line 994) on
// the same path. Calling materialize_plugin_tables() afterwards from
// src/main.cpp would re-run a name-dedup pass that found everything
// already present and produced an empty new-rows set, then run no DDL —
// dead code that read as load-bearing. The merge in Admin::init() is
// the single canonical site for plugin-table materialization. If a
// future change wants Admin to expose plugin-tables for live reload
// (admindb merge after init returns), reintroduce a function with a
// clear contract; do not resurrect the previous post-init no-op.

@ -339,6 +339,12 @@ extern AI_Features_Manager *GloAI;
extern void (*flush_logs_function)();
#ifdef PROXYSQL40
// Plugin DB-handle getters consumed by ProxySQL_PluginManager when it
// builds the Phase-B services struct. Gated under PROXYSQL40 so v3.x
// builds export no plugin-aware symbols at all (the chassis is a v4.0
// feature; in v3.0/v3.1 these would be unused-but-reachable symbols
// that contradict the "chassis fully invisible to v3.x" requirement).
SQLite3DB* proxysql_plugin_get_admindb() {
return GloAdmin ? GloAdmin->admindb : nullptr;
}
@ -350,6 +356,7 @@ SQLite3DB* proxysql_plugin_get_configdb() {
SQLite3DB* proxysql_plugin_get_statsdb() {
return GloAdmin ? GloAdmin->statsdb : nullptr;
}
#endif /* PROXYSQL40 */
extern Web_Interface *GloWebInterface;

@ -609,14 +609,10 @@ bool ProxySQL_PluginManager::dispatch_admin_command(const ProxySQL_PluginCommand
}
proxy_debug(PROXY_DEBUG_ADMIN, 4, "Dispatching plugin command: %s (via %s)\n",
command.sql.c_str(), normalized_sql.c_str());
#ifdef PROXYSQL40
// Pass the CANONICAL form to the callback so plugins can ignore
// which alias the user typed — they match on their own canonical
// strings only.
result = command.cb(ctx, command.sql.c_str());
#else
result = command.cb(ctx, normalized_sql.c_str());
#endif /* PROXYSQL40 */
return true;
}
@ -886,14 +882,14 @@ bool proxysql_load_configured_plugins(
const std::vector<std::string>& plugin_modules,
std::string& err
) {
#ifdef PROXYSQL40
// Phase A + Phase B of the four-phase lifecycle. Executed BEFORE
// ProxySQL_Main_init_Admin_module so that plugin-declared schemas are
// available when merge_plugin_tables materializes the SQLite schema.
// available when Admin::init() merges them into tables_defs_* and
// runs the DDL via check_and_build_standard_tables.
//
// On return, `manager` is populated and installed as the active
// manager — GloAdmin->materialize_plugin_tables() can then query it
// for the tables to CREATE. Phase D (init) runs later, via
// manager — Admin::init() reads it via proxysql_get_plugin_manager()
// to find the tables to merge. Phase D (init) runs later, via
// proxysql_init_configured_plugins, once admin is up.
std::lock_guard<std::mutex> lifecycle_lock(g_plugin_lifecycle_mutex);
err.clear();
@ -917,17 +913,19 @@ bool proxysql_load_configured_plugins(
// Phase B: register_schemas runs here. Plugins declare their
// admin-schema tables into the manager's pending-tables list.
// GloAdmin->materialize_plugin_tables() (called after
// ProxySQL_Admin::init() (called next, via
// ProxySQL_Main_init_Admin_module in src/main.cpp) drains that list
// into SQLite via the merge_plugin_tables code path. Plugins that
// left register_schemas null are no-ops here.
// by merging into tables_defs_{admin,config,stats} and then running
// the DDL via check_and_build_standard_tables — same code path as
// the core tables. Plugins that left register_schemas null are
// no-ops here.
if (!next_manager->invoke_register_schemas_phase(err)) {
return false;
}
// Install as active manager BEFORE admin init, so that
// proxysql_get_plugin_manager() — used by
// ProxySQL_Admin::materialize_plugin_tables — can find the
// proxysql_get_plugin_manager() — used by ProxySQL_Admin::init() to
// merge plugin-declared schemas into tables_defs_* — can find the
// registered tables.
//
// INVARIANT (publish-before-Phase-D): after this point Phase D
@ -946,39 +944,6 @@ bool proxysql_load_configured_plugins(
g_active_plugin_manager.store(manager.get(), std::memory_order_release);
}
return true;
#else /* !PROXYSQL40 */
// Pre-chassis two-phase: load + init_all in one call, installed as
// active manager only on full success.
err.clear();
{
std::lock_guard<std::mutex> lock(g_active_plugin_manager_mutex);
g_active_plugin_manager.store(nullptr, std::memory_order_release);
}
manager.reset();
if (plugin_modules.empty()) {
return true;
}
auto next_manager = std::make_unique<ProxySQL_PluginManager>();
for (const auto& path : plugin_modules) {
if (!next_manager->load(path, err)) {
err = path + ": " + err;
return false;
}
}
if (!next_manager->init_all(err)) {
return false;
}
{
std::lock_guard<std::mutex> lock(g_active_plugin_manager_mutex);
manager = std::move(next_manager);
g_active_plugin_manager.store(manager.get(), std::memory_order_release);
}
return true;
#endif /* PROXYSQL40 */
}
#ifdef PROXYSQL40

@ -119,9 +119,23 @@ public:
// string overload is a convenience wrapper that fetches the identity
// from the thread's configured MysqlxConfigStore, mimicking what the
// auth handler does when a real client connects.
//
// inject_identity_for_test and resolve_backend_target_for_test are
// gated behind MYSQLX_TEST_BUILD because they are forgery vectors:
// inject_identity_for_test bypasses the full auth flow (no credential
// check, no cap negotiation), and resolve_backend_target_for_test
// drives a private routing helper without an authenticated identity.
// The test Makefile defines MYSQLX_TEST_BUILD; the production .so
// build does not, so these methods do not exist in shipped binaries.
// The remaining target_*_for_test getters are read-only state
// observers and are left available unconditionally (they leak no
// state a debugger could not also observe and cannot mutate the
// session).
#ifdef MYSQLX_TEST_BUILD
void inject_identity_for_test(const MysqlxResolvedIdentity& id) { identity_ = id; }
void inject_identity_for_test(const std::string& username);
int resolve_backend_target_for_test() { return resolve_backend_target(); }
#endif /* MYSQLX_TEST_BUILD */
int target_hostgroup_for_test() const { return target_hostgroup_; }
const std::string& target_address_for_test() const { return target_address_; }
int target_port_for_test() const { return target_port_; }

@ -1016,6 +1016,7 @@ int MysqlxSession::resolve_backend_target() {
return 0;
}
#ifdef MYSQLX_TEST_BUILD
// Test-only convenience overload. Mirrors what the auth handler does on a
// real client connection: look up the identity via the thread's config
// store, caching the result in identity_. Silently no-ops if the thread
@ -1028,6 +1029,7 @@ void MysqlxSession::inject_identity_for_test(const std::string& username) {
auto id = cs->resolve_identity(username);
if (id) identity_ = *id;
}
#endif /* MYSQLX_TEST_BUILD */
void MysqlxSession::handler_connecting_server() {
if (!backend_conn_) {

@ -1561,15 +1561,15 @@ void ProxySQL_Main_init_phase2___not_started(const bootstrap_info_t& boostrap_in
// loader at all):
// Phase A+B: dlopen + register_schemas (plugin-declared schemas
// populate the pending-tables list).
// Phase C: admin module init + materialize_plugin_tables (creates
// plugin-owned SQLite tables through the
// merge_plugin_tables path — first-boot == reload).
// Phase C: admin module init merges plugin-declared schemas into
// tables_defs_{admin,config,stats} and runs the DDL via
// check_and_build_standard_tables, all on the same
// first-boot/reload code path as the core tables.
// Phase D: init() with full services (live DB handles pointing at
// a schema that already contains the plugin's own tables).
// Phase E: start() launches the plugin's threads / accept loops.
LoadConfiguredPlugins();
ProxySQL_Main_init_Admin_module(boostrap_info);
GloAdmin->materialize_plugin_tables();
InitConfiguredPlugins();
StartConfiguredPlugins();
#else /* !PROXYSQL40 */

@ -232,13 +232,20 @@ ifneq ($(shell nm $(LIBPROXYSQLAR) 2>/dev/null | grep -cw 'init_debug_struct'),0
PSQLDEBUG := -DDEBUG
endif
# MYSQLX_TEST_BUILD enables the test-only methods (`*_for_test`) on the
# mysqlx plugin's public classes (e.g. MysqlxSession::inject_identity_for_test).
# Defined ONLY here, in the unit-test Makefile — never in plugins/mysqlx/Makefile.
# That way the production .so does not compile in entry points that could be
# misused (most importantly identity-forgery setters), while tests that include
# the plugin headers and link against the plugin sources still see the helpers
# they need.
OPT := $(STDCPP) -O0 -ggdb $(PSQLCH) $(PSQLGA) $(PSQL40) $(PSQL31) $(PSQLFFTO) $(PSQLTSDB) $(PSQLDEBUG) \
-DGITVERSION=\"$(GIT_VERSION)\" $(NOJEM) $(WGCOV) $(WASAN) \
-DGITVERSION=\"$(GIT_VERSION)\" -DMYSQLX_TEST_BUILD $(NOJEM) $(WGCOV) $(WASAN) \
-Wl,--no-as-needed -Wl,-rpath,$(TAP_LDIR)
ifeq ($(UNAME_S),Darwin)
OPT := $(STDCPP) -O0 -ggdb $(PSQLCH) $(PSQLGA) $(PSQL40) $(PSQL31) $(PSQLFFTO) $(PSQLTSDB) $(PSQLDEBUG) \
-DGITVERSION=\"$(GIT_VERSION)\" $(NOJEM) $(WGCOV) $(WASAN)
-DGITVERSION=\"$(GIT_VERSION)\" -DMYSQLX_TEST_BUILD $(NOJEM) $(WGCOV) $(WASAN)
endif

Loading…
Cancel
Save