From 89ad172377ea85b1c58848a42089b66f71697c2b Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sun, 1 Nov 2026 17:49:59 +0000 Subject: [PATCH 1/4] fix(mcp-guard): guard sessions opened by components (RFC 0001 Phase 4, Q4) A component's "local" command-service session that impersonates an MCP account (as component_mcp will open, and component_mysql_rest_service opens) raises no connection event, so the guard never marked it as an MCP session: on 9.7.2 such a session could write, read mysql.user and run without the time limit. - The guard classifies an unclassified session from its privileges at its first statement (query or table access event); a "not MCP" marker keeps it to one grant check per session. An MCP session that cannot be marked is refused. - mcp.sessions_connected is decremented when the THD ends (the slot's free callback), covering sessions that raise no disconnect event. - These sessions also start unaudited (Srv_session: m_audited = false): a component must emit MYSQL_AUDIT_CONNECTION_CONNECT / _DISCONNECT for them. Documented; component_mcp will. - src/mcp/guard/test/component_mcp_session_probe.cc: test-only component that runs SQL as component_mcp will (own thread, impersonated local session, connection audit events); built with MYVECTOR_BUILD_MCP=ON, not packaged. - test-mcp-guard.sh: local-session checks (120 checks on 8.4.11, 9.7.2, 26.10.0); the unfixed guard fails them. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 + CMakeLists.txt | 12 ++ docs/MCP_COMPONENTS.md | 7 +- scripts/build-component-26.10-docker.sh | 4 +- scripts/build-component-26.7-docker.sh | 4 +- scripts/build-component-8.4-docker.sh | 4 +- scripts/build-component-9.7-docker.sh | 4 +- scripts/test-mcp-guard.sh | 32 ++++ src/mcp/guard/component_mcp_guard.cc | 75 +++++++- .../guard/test/component_mcp_session_probe.cc | 172 ++++++++++++++++++ 10 files changed, 305 insertions(+), 14 deletions(-) create mode 100644 src/mcp/guard/test/component_mcp_session_probe.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 183e1fd4..9602670a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 and rebuild the index. See [Declaring a Vector Column](docs/usage.md#declaring-a-vector-column). ### Fixed +- **`component_mcp_guard`: sessions opened by components were never guarded** (RFC 0001 Phase 4, open question 4). A component's `"local"` command-service session that impersonates an MCP account raises no connection event, so the guard never marked it as an MCP session. On 9.7.2 such a session could write, read `mysql.user` and run without the time limit. + - The guard now classifies an unclassified session from its privileges at its first statement. A "not MCP" marker keeps the cost at one grant check per session. + - `mcp.sessions_connected` is decremented when the session's THD ends, which also covers sessions without a disconnect event. + - These sessions also start **unaudited** (no events at all), so a component must emit connection audit events for them. This is documented in `docs/MCP_COMPONENTS.md`, and `component_mcp` will do it. + - `scripts/test-mcp-guard.sh`: 120 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. - **`component_mcp_guard`: table reads the schema rules could not see** (found reviewing the code behind #247). The server raises table access events only for `SELECT`, data changes, `ANALYZE` and `HANDLER READ`. An MCP session could therefore read any table, `mysql.user` included, past `mcp.allowed_schemas` and `mcp.deny_system_schemas`: - directly, with a subquery in `SET` or `DO` (`SET @x = (SELECT … FROM mysql.user); SELECT @x`), also through `PREPARE`/`EXECUTE`; - as a yes/no oracle, with a subquery in `SHOW … WHERE` or in a `CALL` argument; diff --git a/CMakeLists.txt b/CMakeLists.txt index de84126f..767e9ed3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -176,6 +176,18 @@ if(MYVECTOR_BUILD_MCP) install(FILES src/mcp/guard/mcp_guard.json DESTINATION share/mysql/component ) + + # Test only, not installed: runs SQL in impersonated command-service sessions, as + # component_mcp does, for scripts/test-mcp-guard.sh. + find_package(Threads REQUIRED) + add_library(component_mcp_session_probe SHARED src/mcp/guard/test/component_mcp_session_probe.cc) + target_include_directories(component_mcp_session_probe PRIVATE ${MYVECTOR_INCLUDE_DIRS}) + target_link_libraries(component_mcp_session_probe PRIVATE Threads::Threads) + set_target_properties(component_mcp_session_probe PROPERTIES PREFIX "") + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set_property(TARGET component_mcp_session_probe APPEND_STRING PROPERTY LINK_FLAGS + " -static-libstdc++ -static-libgcc") + endif() endif() # Install: shared library and component manifest diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index 8d9feb42..b1e1aacb 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -52,6 +52,11 @@ A session is an **MCP session** when its account holds `MCP_USER` and not `MCP_A - `MCP_ADMIN` holders are never guarded. After the component is installed, `GRANT ALL ON *.*` includes both privileges, so an administrator granted that way is not put under the guard. - **Privilege changes take effect at the next connection.** Sessions already connected when the component is installed, or when `MCP_USER` / `MCP_ADMIN` is granted or revoked, keep their previous status. Disconnect them (`KILL CONNECTION`) or have the client reconnect after such a change. - Grant `MCP_USER` to the account itself; a role activated later with `SET ROLE` is not considered. +- **Sessions opened by components.** A component can run SQL as an account through the command services: a `"local"` session that impersonates the account, as `component_mcp` (RFC 0001 Phase 4) and `component_mysql_rest_service` do. Such a session raises no connection event, so the guard classifies it from its privileges at its first statement. + - The server starts these sessions **unaudited**: no statement or connection event reaches any audit consumer, the guard included. + - A component that runs SQL for MCP accounts must therefore emit `MYSQL_AUDIT_CONNECTION_CONNECT` for the session's THD after connecting (`mysql_audit_api_connection`), and `..._DISCONNECT` before closing. The CONNECT event turns auditing on. + - `component_mcp` does this. A component that doesn't runs its SQL unseen by the guard and by audit log plugins. + - `scripts/test-mcp-guard.sh` checks both cases with a test-only component (`src/mcp/guard/test/component_mcp_session_probe.cc`). ### Settings @@ -174,6 +179,6 @@ The option is `mcp.read_only`, `mcp.deny_system_schemas`, `mcp.allowed_schemas` ### Tests -`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events and the parity table below against the official image (110 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. +`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (120 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. **Parity with mysql-mcp-server** (RFC 0001 §5.2 item 7): [`sql/mcp/guard-parity.tsv`](https://github.com/askdba/myvector/blob/main/sql/mcp/guard-parity.tsv) lists statements a `run_query` call might send, with the verdict of mysql-mcp-server's validator (`go`) and of the guard under the default settings (`guard`). Every row where they differ says why. The test checks the guard column, sending each statement as one packet. mysql-mcp-server vendors the file and checks the go column. Both reject every write and locking read. The guard allows a few things the validator does not (session `SET`, `information_schema`, `CALL mysql_mcp.*`) and catches what text checks cannot (a write inside a function called from `SELECT`). diff --git a/scripts/build-component-26.10-docker.sh b/scripts/build-component-26.10-docker.sh index f271ffe7..a87e1fab 100755 --- a/scripts/build-component-26.10-docker.sh +++ b/scripts/build-component-26.10-docker.sh @@ -161,13 +161,15 @@ docker run --rm \ cp src/component_src/myvector.json "/workspace/$OUTPUT_DIR/" # MCP guard component (RFC 0001), built when MYVECTOR_BUILD_MCP=ON. Clear any # copy from an earlier build first, so an OFF build never ships a stale guard. - rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" + rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" "/workspace/$OUTPUT_DIR/component_mcp_session_probe.so" if [ "$MYVECTOR_BUILD_MCP" = ON ]; then if [ ! -f "$COMPONENT_BUILD/component_mcp_guard.so" ]; then echo "ERROR: MYVECTOR_BUILD_MCP=ON but component_mcp_guard.so was not built" >&2 exit 1 fi cp "$COMPONENT_BUILD/component_mcp_guard.so" src/mcp/guard/mcp_guard.json "/workspace/$OUTPUT_DIR/" + # Test-only component for scripts/test-mcp-guard.sh (not packaged). + cp "$COMPONENT_BUILD/component_mcp_session_probe.so" "/workspace/$OUTPUT_DIR/" echo "==> Built: /workspace/$OUTPUT_DIR/component_mcp_guard.so" fi echo "==> Built: /workspace/$OUTPUT_DIR/libmyvector_component.so" diff --git a/scripts/build-component-26.7-docker.sh b/scripts/build-component-26.7-docker.sh index f402b3ad..70ce5695 100755 --- a/scripts/build-component-26.7-docker.sh +++ b/scripts/build-component-26.7-docker.sh @@ -160,13 +160,15 @@ docker run --rm \ cp src/component_src/myvector.json "/workspace/$OUTPUT_DIR/" # MCP guard component (RFC 0001), built when MYVECTOR_BUILD_MCP=ON. Clear any # copy from an earlier build first, so an OFF build never ships a stale guard. - rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" + rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" "/workspace/$OUTPUT_DIR/component_mcp_session_probe.so" if [ "$MYVECTOR_BUILD_MCP" = ON ]; then if [ ! -f "$COMPONENT_BUILD/component_mcp_guard.so" ]; then echo "ERROR: MYVECTOR_BUILD_MCP=ON but component_mcp_guard.so was not built" >&2 exit 1 fi cp "$COMPONENT_BUILD/component_mcp_guard.so" src/mcp/guard/mcp_guard.json "/workspace/$OUTPUT_DIR/" + # Test-only component for scripts/test-mcp-guard.sh (not packaged). + cp "$COMPONENT_BUILD/component_mcp_session_probe.so" "/workspace/$OUTPUT_DIR/" echo "==> Built: /workspace/$OUTPUT_DIR/component_mcp_guard.so" fi echo "==> Built: /workspace/$OUTPUT_DIR/libmyvector_component.so" diff --git a/scripts/build-component-8.4-docker.sh b/scripts/build-component-8.4-docker.sh index 97115fba..473a25f7 100755 --- a/scripts/build-component-8.4-docker.sh +++ b/scripts/build-component-8.4-docker.sh @@ -161,13 +161,15 @@ docker run --rm \ cp src/component_src/myvector.json "/workspace/$OUTPUT_DIR/" # MCP guard component (RFC 0001), built when MYVECTOR_BUILD_MCP=ON. Clear any # copy from an earlier build first, so an OFF build never ships a stale guard. - rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" + rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" "/workspace/$OUTPUT_DIR/component_mcp_session_probe.so" if [ "$MYVECTOR_BUILD_MCP" = ON ]; then if [ ! -f "$COMPONENT_BUILD/component_mcp_guard.so" ]; then echo "ERROR: MYVECTOR_BUILD_MCP=ON but component_mcp_guard.so was not built" >&2 exit 1 fi cp "$COMPONENT_BUILD/component_mcp_guard.so" src/mcp/guard/mcp_guard.json "/workspace/$OUTPUT_DIR/" + # Test-only component for scripts/test-mcp-guard.sh (not packaged). + cp "$COMPONENT_BUILD/component_mcp_session_probe.so" "/workspace/$OUTPUT_DIR/" echo "==> Built: /workspace/$OUTPUT_DIR/component_mcp_guard.so" fi echo "==> Built: /workspace/$OUTPUT_DIR/libmyvector_component.so" diff --git a/scripts/build-component-9.7-docker.sh b/scripts/build-component-9.7-docker.sh index a3ac677b..40644e40 100755 --- a/scripts/build-component-9.7-docker.sh +++ b/scripts/build-component-9.7-docker.sh @@ -163,13 +163,15 @@ docker run --rm \ cp src/component_src/myvector.json "/workspace/$OUTPUT_DIR/" # MCP guard component (RFC 0001), built when MYVECTOR_BUILD_MCP=ON. Clear any # copy from an earlier build first, so an OFF build never ships a stale guard. - rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" + rm -f "/workspace/$OUTPUT_DIR/component_mcp_guard.so" "/workspace/$OUTPUT_DIR/mcp_guard.json" "/workspace/$OUTPUT_DIR/component_mcp_session_probe.so" if [ "$MYVECTOR_BUILD_MCP" = ON ]; then if [ ! -f "$COMPONENT_BUILD/component_mcp_guard.so" ]; then echo "ERROR: MYVECTOR_BUILD_MCP=ON but component_mcp_guard.so was not built" >&2 exit 1 fi cp "$COMPONENT_BUILD/component_mcp_guard.so" src/mcp/guard/mcp_guard.json "/workspace/$OUTPUT_DIR/" + # Test-only component for scripts/test-mcp-guard.sh (not packaged). + cp "$COMPONENT_BUILD/component_mcp_session_probe.so" "/workspace/$OUTPUT_DIR/" echo "==> Built: /workspace/$OUTPUT_DIR/component_mcp_guard.so" fi echo "==> Built: /workspace/$OUTPUT_DIR/libmyvector_component.so" diff --git a/scripts/test-mcp-guard.sh b/scripts/test-mcp-guard.sh index 94448783..39c95ffe 100755 --- a/scripts/test-mcp-guard.sh +++ b/scripts/test-mcp-guard.sh @@ -5,6 +5,8 @@ # # Usage: scripts/test-mcp-guard.sh # e.g. scripts/test-mcp-guard.sh 9.7 dist/mcp-guard-9.7.2 +# The artifact dir holds component_mcp_guard.so and the test-only component_mcp_session_probe.so +# (both built with MYVECTOR_BUILD_MCP=ON). # Requires Docker. bash 3.2+. set -u @@ -12,6 +14,9 @@ TAG=${1:?mysql image tag, e.g. 9.7} ART=${2:?artifact dir containing component_mcp_guard.so} SO=$ART/component_mcp_guard.so [ -f "$SO" ] || { echo "missing $SO"; exit 1; } +# Test-only component that opens command-service sessions as component_mcp does. +PROBE=$ART/component_mcp_session_probe.so +[ -f "$PROBE" ] || { echo "missing $PROBE"; exit 1; } REPO=$(cd "$(dirname "$0")/.." && pwd) CNAME=mcp-guard-test-$(printf '%s' "$TAG" | tr -c 'a-zA-Z0-9.-' '-')-$$ PASS=0 @@ -47,6 +52,8 @@ echo " server: $(sql 'select version()')" PLUGIN_DIR=$(sql 'select @@plugin_dir') docker cp "$SO" "$CNAME:$PLUGIN_DIR/component_mcp_guard.so" && docker exec -u 0 "$CNAME" chmod 0644 "$PLUGIN_DIR/component_mcp_guard.so" +docker cp "$PROBE" "$CNAME:$PLUGIN_DIR/component_mcp_session_probe.so" && + docker exec -u 0 "$CNAME" chmod 0644 "$PLUGIN_DIR/component_mcp_session_probe.so" out=$(docker exec -i "$CNAME" mysql -h127.0.0.1 -uroot -ptestpass <"$REPO/sql/mcp/mysql_mcp.sql" 2>&1 | grep -v "Using a password") [ -z "$out" ] && [ "$(sql "CALL mysql_mcp.version()")" = "1.0.0" ] || { echo "tool pack import failed: $out"; exit 1; } @@ -194,6 +201,31 @@ d1_state() { sql "SELECT CONCAT_WS(',', (SELECT GROUP_CONCAT(i ORDER BY i) FROM (SELECT GROUP_CONCAT(TABLE_NAME ORDER BY TABLE_NAME) FROM information_schema.TABLES WHERE TABLE_SCHEMA = 'd1'))"; } expect "parity table: nothing was written" "$(d1_state)" "^1,1,t,v_d2,v_nested,v_sys$" +# --- sessions opened by components (command services), as component_mcp opens them ----------- +# A component's "local" session that impersonates an MCP account raises no connection event; +# the guard classifies it at its first statement. The component must emit connection audit +# events for it (Srv_session starts unaudited), as component_mcp will. +expect "probe component installed" "$(sql "INSTALL COMPONENT 'file://component_mcp_session_probe'")" "^$" +local_session() { sql "SELECT mcp_test_local_session($1)"; } +# Refused by the guard: its message, with errno 1290 (8.4 reports errno 0 through the +# command service for an error raised by an event consumer; the message is the same). +denied_local() { expect "$1" "$2" "^err:query:(1290|0):The MySQL server is running with the mcp\."; } +denied_local "local MCP session: write refused" "$(local_session "'mcp', '%', 'INSERT INTO d1.t VALUES (2)'")" +denied_local "local MCP session: mysql schema refused" "$(local_session "'mcp', '%', 'SELECT COUNT(*) FROM mysql.user'")" +denied_local "local MCP session: SET subquery refused" "$(local_session "'mcp', '%', 'SET @x = (SELECT COUNT(*) FROM mysql.user)'")" +expect "local MCP session: reads allowed" "$(local_session "'mcp', '%', 'SELECT COUNT(*) FROM d1.t'")" "^ok:1$" +sql "SET GLOBAL mcp.max_execution_time_ms = 1500" >/dev/null +expect "local MCP session: time limit applied" "$(local_session "'mcp', '%', 'SELECT @@max_execution_time'")" "^ok:1500$" +sql "SET GLOBAL mcp.max_execution_time_ms = 0" >/dev/null +expect "local session of another account: not restricted" "$(local_session "'app', '%', 'INSERT INTO d1.t VALUES (9)'")" "^ok:no-result$" +sql "DELETE FROM d1.t WHERE i = 9" >/dev/null +# Without the connection event the session is unaudited: no event reaches the guard. This is +# why component_mcp must emit it (checked here so that a server change is noticed). +expect "REQUIREMENT: an unaudited local session is not seen (components must emit connection events)" "$(local_session "'mcp', '%', 'INSERT INTO d1.t VALUES (8)', 'unaudited'")" "^ok:no-result$" +sql "DELETE FROM d1.t WHERE i = 8" >/dev/null +expect "local sessions: nothing written, no MCP session left" "$(sql "SELECT GROUP_CONCAT(i ORDER BY i) FROM d1.t; SHOW GLOBAL STATUS LIKE 'mcp.sessions_connected'" | tr '\n' ' ')" "^1 mcp.sessions_connected 0 $" +expect "probe component uninstalled" "$(sql "UNINSTALL COMPONENT 'file://component_mcp_session_probe'")" "^$" + sql "SET GLOBAL mcp.enabled = OFF" >/dev/null denied "mcp.enabled OFF refuses MCP connections" "$(mcp "SELECT 1")" "mcp.enabled" expect "mcp.enabled OFF leaves other accounts alone" "$(as app apppass1 "SELECT 1")" "^1$" diff --git a/src/mcp/guard/component_mcp_guard.cc b/src/mcp/guard/component_mcp_guard.cc index cee1c911..a37de25f 100644 --- a/src/mcp/guard/component_mcp_guard.cc +++ b/src/mcp/guard/component_mcp_guard.cc @@ -363,24 +363,50 @@ struct SessionState { mysql_thd_store_slot session_slot = nullptr; -// Called by the server when the THD ends with a state still attached. +// The slot holds an MCP session's state, kNotMcp for a session classified as not MCP, or +// null for a session not classified yet. Sessions opened without connection events (the +// "local" sessions of the command services, which components such as component_mcp open +// with an impersonated account) are classified at their first statement. +SessionState not_mcp_marker; +SessionState *const kNotMcp = ¬_mcp_marker; + +// Frees an MCP session's state (the only allocated kind) and uncounts it. +void drop_state(SessionState *state) { + if (state == nullptr || state == kNotMcp) return; + delete state; + --sessions_connected; +} + +// Called by the server when the THD ends with a state still attached; command-service +// sessions end this way, without a disconnect event. int free_session_slot(void *state) { - delete static_cast(state); + drop_state(static_cast(state)); return 0; } -SessionState *session_state(MYSQL_THD thd) { +SessionState *raw_session_state(MYSQL_THD thd) { if (thd == nullptr || session_slot == nullptr) return nullptr; return static_cast(mysql_service_mysql_thd_store->get(thd, session_slot)); } +// The MCP session state of thd, or null if it is not an MCP session (or not classified). +SessionState *session_state(MYSQL_THD thd) { + SessionState *state = raw_session_state(thd); + return state == kNotMcp ? nullptr : state; +} + // Returns true on failure. An MCP session that cannot be marked would not be guarded, so // the caller refuses it. bool mark_session(MYSQL_THD thd, bool mcp) { if (session_slot == nullptr) return mcp; SessionState *state = session_state(thd); - if ((state != nullptr) == mcp) return false; + if ((state != nullptr) == mcp) { + // Already right; an unclassified non-MCP session still gets the marker. + if (!mcp && raw_session_state(thd) == nullptr) + mysql_service_mysql_thd_store->set(thd, session_slot, kNotMcp); + return false; + } if (mcp) { state = new (std::nothrow) SessionState(); if (state == nullptr) return true; @@ -388,11 +414,11 @@ bool mark_session(MYSQL_THD thd, bool mcp) { delete state; return true; } + ++sessions_connected; } else { - mysql_service_mysql_thd_store->set(thd, session_slot, nullptr); - delete state; + mysql_service_mysql_thd_store->set(thd, session_slot, kNotMcp); + drop_state(state); } - sessions_connected += mcp ? 1 : -1; return false; } @@ -743,6 +769,29 @@ void apply_time_limit(MYSQL_THD thd, SessionState *state) { thd, "SESSION", nullptr, max_execution_time_name, limit); } +// The MCP session state of thd, classifying a session that has not been classified yet +// (no connection event: a command-service "local" session, possibly impersonating an +// account) from its grants. failed is set if an MCP session could not be marked; the +// caller then refuses the statement, so it is never run unguarded. +SessionState *classify(MYSQL_THD thd, bool *failed) { + *failed = false; + if (thd == nullptr || session_slot == nullptr) return nullptr; + SessionState *state = raw_session_state(thd); + if (state == kNotMcp) return nullptr; + if (state != nullptr) return state; + const bool mcp = thd_has_grant(thd, kMcpUser) && !thd_has_grant(thd, kMcpAdmin); + if (mark_session(thd, mcp)) { + *failed = mcp; + return nullptr; + } + state = session_state(thd); + if (state != nullptr) { + state->account = session_account(thd); + apply_time_limit(thd, state); + } + return state; +} + } // namespace // ----- Event tracking ----------------------------------------------------------------- @@ -839,7 +888,11 @@ mysql_event_tracking_query_subclass_t Event_tracking_query_implementation::filte bool Event_tracking_query_implementation::callback(const mysql_event_tracking_query_data *data) { MYSQL_THD thd = current_thd_or_null(); - SessionState *state = session_state(thd); + bool failed = false; + SessionState *state = classify(thd, &failed); + if (failed) + return deny("mcp_guard", statements_denied, data->connection_id, + "statement refused: session state unavailable"); if (state == nullptr) return false; if (data->event_subclass == EVENT_TRACKING_QUERY_STATUS_END) { track_statement_end(state, data->status); // performance_schema.mcp_tool_calls @@ -885,7 +938,11 @@ mysql_event_tracking_table_access_subclass_t bool Event_tracking_table_access_implementation::callback( const mysql_event_tracking_table_access_data *data) { MYSQL_THD thd = current_thd_or_null(); - SessionState *state = session_state(thd); + bool failed = false; + SessionState *state = classify(thd, &failed); + if (failed) + return deny("mcp_guard", table_access_denied, data->connection_id, + "table access refused: session state unavailable"); if (state == nullptr) return false; const std::string_view db(data->table_database.str ? data->table_database.str : "", data->table_database.str ? data->table_database.length : 0); diff --git a/src/mcp/guard/test/component_mcp_session_probe.cc b/src/mcp/guard/test/component_mcp_session_probe.cc new file mode 100644 index 00000000..abb16747 --- /dev/null +++ b/src/mcp/guard/test/component_mcp_session_probe.cc @@ -0,0 +1,172 @@ +/* + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License, version 2.0, + as published by the Free Software Foundation. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License, version 2.0, for more details. +*/ + +// TEST ONLY, never installed or packaged. A component that runs SQL the way component_mcp +// (RFC 0001 Phase 4) does: on a thread of its own, in a "local" command-service session +// that impersonates an account, announced with connection audit events. The guard test +// (scripts/test-mcp-guard.sh) uses it to check that component_mcp_guard enforces its +// policy on such sessions. +// +// SELECT mcp_test_local_session('mcp', '%', 'INSERT INTO d1.t VALUES (1)'); +// SELECT mcp_test_local_session('mcp', '%', '...', 'unaudited'); -- no connection events +// -> "ok:" | "ok:no-result" | "err:::" +// +// Sessions of the command services start unaudited (Srv_session sets m_audited = false): +// no connection or statement events reach audit consumers until the component emits +// MYSQL_AUDIT_CONNECTION_CONNECT for the session's THD, which turns auditing on. The THD +// is read from the handle, as component_mysql_rest_service does, because it is current +// only while a statement runs. A component that skips the event runs its SQL unseen by +// component_mcp_guard (and by audit log plugins). +#include +#include +#include +#include +#include +#include +// In-tree only (as component_mysql_rest_service): the session THD behind a handle. +#include "sql/server_component/mysql_command_services_imp.h" +#include +#include + +REQUIRES_SERVICE_PLACEHOLDER_AS(udf_registration, udf_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_factory, cmd_factory_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_options, cmd_options_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_query, cmd_query_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_query_result, cmd_query_result_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_field_info, cmd_field_info_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_error_info, cmd_error_info_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_command_thread, cmd_thread_srv); +REQUIRES_SERVICE_PLACEHOLDER_AS(mysql_audit_api_connection, audit_conn_srv); + +static MYSQL_THD session_thd(MYSQL_H h) { + auto *handle = reinterpret_cast(h); + if (handle == nullptr || handle->mysql == nullptr) return nullptr; + return static_cast(handle->mysql->thd); // set by connect, as MRS reads it +} + +static std::string run(const std::string &user, const std::string &host, const std::string &sql, bool audit = false) { + MYSQL_H h = nullptr; + std::string out; + auto err = [&](const char *where) { + unsigned int no = 0; + // sql_error copies into the caller's buffer before 26.10 (strcpy into *errmsg), so it + // is always given one; MYSQL_ERRMSG_SIZE is 512. + char buf[512] = ""; + char *msg = buf; + if (h != nullptr) { + cmd_error_info_srv->sql_errno(h, &no); + cmd_error_info_srv->sql_error(h, &msg); + } + return std::string("err:") + where + ":" + std::to_string(no) + ":" + (msg ? msg : ""); + }; + if (cmd_factory_srv->init(&h) || h == nullptr) return "err:init"; + if (cmd_options_srv->set(h, MYSQL_COMMAND_PROTOCOL, "local") || + cmd_options_srv->set(h, MYSQL_COMMAND_USER_NAME, user.c_str()) || + cmd_options_srv->set(h, MYSQL_COMMAND_HOST_NAME, host.c_str())) { + out = err("options"); + } else if (cmd_factory_srv->connect(h)) { + out = err("connect"); + } else if (audit && [&] { + // As component_mcp will: on a thread of its own, emit CONNECT for the session + // so audit consumers (component_mcp_guard) see it. + MYSQL_THD thd = session_thd(h); + if (thd == nullptr) { + out = "[no session THD] "; + return true; + } + return audit_conn_srv->emit(thd, MYSQL_AUDIT_CONNECTION_CONNECT) != 0; + }()) { + out += err("audit-connect"); + } else if (cmd_query_srv->query(h, sql.data(), sql.size())) { + out += err("query"); + } else { + MYSQL_RES_H res = nullptr; + if (cmd_query_result_srv->store_result(h, &res) || res == nullptr) { + out += "ok:no-result"; + } else { + MYSQL_ROW_H row = nullptr; + if (!cmd_query_result_srv->fetch_row(res, &row) && row != nullptr && row[0] != nullptr) + out += std::string("ok:") + row[0]; + else + out += "ok:empty"; + cmd_query_result_srv->free_result(res); + } + } + if (audit) { + MYSQL_THD thd = session_thd(h); + if (thd != nullptr) audit_conn_srv->emit(thd, MYSQL_AUDIT_CONNECTION_DISCONNECT); + } + cmd_factory_srv->close(h); + return out; +} + +static char *probe_udf(UDF_INIT *, UDF_ARGS *args, char *result, unsigned long *length, + unsigned char *is_null, unsigned char *error) { + *is_null = 0; + *error = 0; + for (unsigned i = 0; i < args->arg_count; ++i) + if (args->args[i] == nullptr) { + std::memcpy(result, "err:null-argument", 17); + *length = 17; + return result; + } + const std::string user(args->args[0], args->lengths[0]), host(args->args[1], args->lengths[1]), + sql(args->args[2], args->lengths[2]); + const bool audited = !(args->arg_count == 4 && args->args[3] != nullptr && + std::string(args->args[3], args->lengths[3]) == "unaudited"); + std::string r; + // A thread of its own, as component_mcp's HTTP workers. + std::thread t([&] { + if (cmd_thread_srv->init()) { + r = "err:thread-init"; + return; + } + r = run(user, host, sql, audited); + cmd_thread_srv->end(); + }); + t.join(); + const size_t n = r.size() < 250 ? r.size() : 250; + std::memcpy(result, r.data(), n); + *length = n; + return result; +} + +static bool probe_init(UDF_INIT *, UDF_ARGS *args, char *msg) { + if (args->arg_count != 3 && args->arg_count != 4) { + std::strcpy(msg, "mcp_test_local_session(user, host, sql[, 'unaudited'])"); + return true; + } + for (unsigned i = 0; i < args->arg_count; ++i) args->arg_type[i] = STRING_RESULT; + return false; +} + +static mysql_service_status_t init() { + return udf_srv->udf_register("mcp_test_local_session", STRING_RESULT, reinterpret_cast(probe_udf), + probe_init, nullptr); +} +static mysql_service_status_t deinit() { + int was_present = 0; + return udf_srv->udf_unregister("mcp_test_local_session", &was_present); +} + +BEGIN_COMPONENT_PROVIDES(mcp_session_probe) END_COMPONENT_PROVIDES(); +BEGIN_COMPONENT_REQUIRES(mcp_session_probe) +REQUIRES_SERVICE_AS(udf_registration, udf_srv), REQUIRES_SERVICE_AS(mysql_command_factory, cmd_factory_srv), + REQUIRES_SERVICE_AS(mysql_command_options, cmd_options_srv), + REQUIRES_SERVICE_AS(mysql_command_query, cmd_query_srv), + REQUIRES_SERVICE_AS(mysql_command_query_result, cmd_query_result_srv), + REQUIRES_SERVICE_AS(mysql_command_field_info, cmd_field_info_srv), + REQUIRES_SERVICE_AS(mysql_command_error_info, cmd_error_info_srv), + REQUIRES_SERVICE_AS(mysql_command_thread, cmd_thread_srv), + REQUIRES_SERVICE_AS(mysql_audit_api_connection, audit_conn_srv), END_COMPONENT_REQUIRES(); +BEGIN_COMPONENT_METADATA(mcp_session_probe) END_COMPONENT_METADATA(); +DECLARE_COMPONENT(mcp_session_probe, "mysql:mcp_session_probe") init, deinit END_DECLARE_COMPONENT(); +DECLARE_LIBRARY_COMPONENTS &COMPONENT_REF(mcp_session_probe) END_DECLARE_LIBRARY_COMPONENTS From 45b2bda7b12d7b086fd57762b38253ed8ec528a4 Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sun, 1 Nov 2026 18:02:25 +0000 Subject: [PATCH 2/4] docs(mcp-guard): status is decided once; sessions connected before install are classified Review of #252 (CodeRabbit): with first-statement classification, a session connected before INSTALL COMPONENT becomes an MCP session at its next statement; grant changes still take effect at the next connection (the status is kept once decided). Docs say so, and test-mcp-guard.sh checks it: a session that connects while the guard is uninstalled has its write refused after reinstall (122 checks on 8.4.11, 9.7.2, 26.10.0). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- docs/MCP_COMPONENTS.md | 4 ++-- scripts/test-mcp-guard.sh | 9 +++++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9602670a..ba8932bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,7 +139,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The guard now classifies an unclassified session from its privileges at its first statement. A "not MCP" marker keeps the cost at one grant check per session. - `mcp.sessions_connected` is decremented when the session's THD ends, which also covers sessions without a disconnect event. - These sessions also start **unaudited** (no events at all), so a component must emit connection audit events for them. This is documented in `docs/MCP_COMPONENTS.md`, and `component_mcp` will do it. - - `scripts/test-mcp-guard.sh`: 120 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. + - `scripts/test-mcp-guard.sh`: 122 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. - **`component_mcp_guard`: table reads the schema rules could not see** (found reviewing the code behind #247). The server raises table access events only for `SELECT`, data changes, `ANALYZE` and `HANDLER READ`. An MCP session could therefore read any table, `mysql.user` included, past `mcp.allowed_schemas` and `mcp.deny_system_schemas`: - directly, with a subquery in `SET` or `DO` (`SET @x = (SELECT … FROM mysql.user); SELECT @x`), also through `PREPARE`/`EXECUTE`; - as a yes/no oracle, with a subquery in `SHOW … WHERE` or in a `CALL` argument; diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index b1e1aacb..40464d15 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -50,7 +50,7 @@ The images are built and published with MyVector releases (`docker-publish.yml`) A session is an **MCP session** when its account holds `MCP_USER` and not `MCP_ADMIN`. This is decided at connect and at `COM_CHANGE_USER`, from privileges only; connection attributes and statement comments cannot change it. - `MCP_ADMIN` holders are never guarded. After the component is installed, `GRANT ALL ON *.*` includes both privileges, so an administrator granted that way is not put under the guard. -- **Privilege changes take effect at the next connection.** Sessions already connected when the component is installed, or when `MCP_USER` / `MCP_ADMIN` is granted or revoked, keep their previous status. Disconnect them (`KILL CONNECTION`) or have the client reconnect after such a change. +- **Privilege changes take effect at the next connection.** A session's status is decided once (at connect, at `COM_CHANGE_USER`, or at its first statement if neither was seen) and kept. Sessions connected when `MCP_USER` / `MCP_ADMIN` is granted or revoked keep their previous status; disconnect them (`KILL CONNECTION`) or have the client reconnect after such a change. Sessions already connected when the component is installed are classified at their next statement. - Grant `MCP_USER` to the account itself; a role activated later with `SET ROLE` is not considered. - **Sessions opened by components.** A component can run SQL as an account through the command services: a `"local"` session that impersonates the account, as `component_mcp` (RFC 0001 Phase 4) and `component_mysql_rest_service` do. Such a session raises no connection event, so the guard classifies it from its privileges at its first statement. - The server starts these sessions **unaudited**: no statement or connection event reaches any audit consumer, the guard included. @@ -179,6 +179,6 @@ The option is `mcp.read_only`, `mcp.deny_system_schemas`, `mcp.allowed_schemas` ### Tests -`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (120 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. +`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (122 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. **Parity with mysql-mcp-server** (RFC 0001 §5.2 item 7): [`sql/mcp/guard-parity.tsv`](https://github.com/askdba/myvector/blob/main/sql/mcp/guard-parity.tsv) lists statements a `run_query` call might send, with the verdict of mysql-mcp-server's validator (`go`) and of the guard under the default settings (`guard`). Every row where they differ says why. The test checks the guard column, sending each statement as one packet. mysql-mcp-server vendors the file and checks the go column. Both reject every write and locking read. The guard allows a few things the validator does not (session `SET`, `information_schema`, `CALL mysql_mcp.*`) and catches what text checks cannot (a write inside a function called from `SELECT`). diff --git a/scripts/test-mcp-guard.sh b/scripts/test-mcp-guard.sh index 39c95ffe..1db20011 100755 --- a/scripts/test-mcp-guard.sh +++ b/scripts/test-mcp-guard.sh @@ -316,6 +316,15 @@ expect "MCP_ADMIN unknown after uninstall" "$(sql "GRANT MCP_ADMIN ON *.* TO app expect "mcp.* variables gone after uninstall" "$(sql "SHOW VARIABLES LIKE 'mcp.%'")" "^$" expect "performance_schema.mcp_tool_calls gone after uninstall" "$(sql "SELECT COUNT(*) FROM performance_schema.mcp_tool_calls")" "ERROR 1146" expect "no enforcement after uninstall" "$(mcp "INSERT INTO d1.t VALUES (5); SELECT COUNT(*) FROM d1.t")" "^2$" +# A session connected before INSTALL COMPONENT is classified at its next statement. +early_out=$(mktemp) +docker exec -i "$CNAME" mysql -h127.0.0.1 -umcp -pmcppass1 -N -e "SELECT SLEEP(4); INSERT INTO d1.t VALUES (6)" >"$early_out" 2>&1 & +early=$! +sleep 1 +expect "reinstalled while an MCP session is connected" "$(sql "INSTALL COMPONENT 'file://component_mcp_guard'")" "^$" +wait "$early" +denied "a session connected before INSTALL is guarded at its next statement" "$(grep -v "Using a password" "$early_out")" "mcp.read_only" +rm -f "$early_out" # --- lower_case_table_names = 1: allowed_schemas compares case-insensitively --------------------- docker run -d --name "$CNAME_LCTN" -e MYSQL_ROOT_PASSWORD=testpass "mysql:$TAG" --lower-case-table-names=1 >/dev/null From c7439c9095b9c62b7dc42344e633b417a052e2b8 Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sun, 1 Nov 2026 19:02:37 +0000 Subject: [PATCH 3/4] test(mcp-guard): synchronize the connected-before-install check; precise docs Review of #252 (CodeRabbit on 45b2bda): - The check now confirms the MCP session is connected and in SLEEP before INSTALL COMPONENT, and still in SLEEP after it, so its INSERT runs only once the guard is installed (124 checks on 8.4.11, 9.7.2, 26.10.0). - Docs: an unaudited session sends no events to the guard until the component emits CONNECT (turning auditing on); the guard then classifies it at its first statement (the CONNECT callback runs on the component's thread, where the session is not current). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- docs/MCP_COMPONENTS.md | 4 ++-- scripts/test-mcp-guard.sh | 11 ++++++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba8932bd..8a696f55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,7 +139,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The guard now classifies an unclassified session from its privileges at its first statement. A "not MCP" marker keeps the cost at one grant check per session. - `mcp.sessions_connected` is decremented when the session's THD ends, which also covers sessions without a disconnect event. - These sessions also start **unaudited** (no events at all), so a component must emit connection audit events for them. This is documented in `docs/MCP_COMPONENTS.md`, and `component_mcp` will do it. - - `scripts/test-mcp-guard.sh`: 122 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. + - `scripts/test-mcp-guard.sh`: 124 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. - **`component_mcp_guard`: table reads the schema rules could not see** (found reviewing the code behind #247). The server raises table access events only for `SELECT`, data changes, `ANALYZE` and `HANDLER READ`. An MCP session could therefore read any table, `mysql.user` included, past `mcp.allowed_schemas` and `mcp.deny_system_schemas`: - directly, with a subquery in `SET` or `DO` (`SET @x = (SELECT … FROM mysql.user); SELECT @x`), also through `PREPARE`/`EXECUTE`; - as a yes/no oracle, with a subquery in `SHOW … WHERE` or in a `CALL` argument; diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index 40464d15..40a09acb 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -52,7 +52,7 @@ A session is an **MCP session** when its account holds `MCP_USER` and not `MCP_A - `MCP_ADMIN` holders are never guarded. After the component is installed, `GRANT ALL ON *.*` includes both privileges, so an administrator granted that way is not put under the guard. - **Privilege changes take effect at the next connection.** A session's status is decided once (at connect, at `COM_CHANGE_USER`, or at its first statement if neither was seen) and kept. Sessions connected when `MCP_USER` / `MCP_ADMIN` is granted or revoked keep their previous status; disconnect them (`KILL CONNECTION`) or have the client reconnect after such a change. Sessions already connected when the component is installed are classified at their next statement. - Grant `MCP_USER` to the account itself; a role activated later with `SET ROLE` is not considered. -- **Sessions opened by components.** A component can run SQL as an account through the command services: a `"local"` session that impersonates the account, as `component_mcp` (RFC 0001 Phase 4) and `component_mysql_rest_service` do. Such a session raises no connection event, so the guard classifies it from its privileges at its first statement. +- **Sessions opened by components.** A component can run SQL as an account through the command services: a `"local"` session that impersonates the account, as `component_mcp` (RFC 0001 Phase 4) and `component_mysql_rest_service` do. Such a session raises no connection event the guard can use, and it starts **unaudited**: the guard receives no events for it until the component emits `MYSQL_AUDIT_CONNECTION_CONNECT` for the session's THD, which turns auditing on. The guard then classifies the session from its privileges at its first statement. (The CONNECT callback runs on the component's thread, where the session is not current, so the guard does not classify it there.) - The server starts these sessions **unaudited**: no statement or connection event reaches any audit consumer, the guard included. - A component that runs SQL for MCP accounts must therefore emit `MYSQL_AUDIT_CONNECTION_CONNECT` for the session's THD after connecting (`mysql_audit_api_connection`), and `..._DISCONNECT` before closing. The CONNECT event turns auditing on. - `component_mcp` does this. A component that doesn't runs its SQL unseen by the guard and by audit log plugins. @@ -179,6 +179,6 @@ The option is `mcp.read_only`, `mcp.deny_system_schemas`, `mcp.allowed_schemas` ### Tests -`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (122 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. +`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (124 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. **Parity with mysql-mcp-server** (RFC 0001 §5.2 item 7): [`sql/mcp/guard-parity.tsv`](https://github.com/askdba/myvector/blob/main/sql/mcp/guard-parity.tsv) lists statements a `run_query` call might send, with the verdict of mysql-mcp-server's validator (`go`) and of the guard under the default settings (`guard`). Every row where they differ says why. The test checks the guard column, sending each statement as one packet. mysql-mcp-server vendors the file and checks the go column. Both reject every write and locking read. The guard allows a few things the validator does not (session `SET`, `information_schema`, `CALL mysql_mcp.*`) and catches what text checks cannot (a write inside a function called from `SELECT`). diff --git a/scripts/test-mcp-guard.sh b/scripts/test-mcp-guard.sh index 1db20011..1424431a 100755 --- a/scripts/test-mcp-guard.sh +++ b/scripts/test-mcp-guard.sh @@ -316,12 +316,17 @@ expect "MCP_ADMIN unknown after uninstall" "$(sql "GRANT MCP_ADMIN ON *.* TO app expect "mcp.* variables gone after uninstall" "$(sql "SHOW VARIABLES LIKE 'mcp.%'")" "^$" expect "performance_schema.mcp_tool_calls gone after uninstall" "$(sql "SELECT COUNT(*) FROM performance_schema.mcp_tool_calls")" "ERROR 1146" expect "no enforcement after uninstall" "$(mcp "INSERT INTO d1.t VALUES (5); SELECT COUNT(*) FROM d1.t")" "^2$" -# A session connected before INSTALL COMPONENT is classified at its next statement. +# A session connected before INSTALL COMPONENT is classified at its next statement. The +# session is confirmed connected and idle (in SLEEP) before the install, and still in SLEEP +# after it, so its INSERT runs only once the guard is installed. early_out=$(mktemp) -docker exec -i "$CNAME" mysql -h127.0.0.1 -umcp -pmcppass1 -N -e "SELECT SLEEP(4); INSERT INTO d1.t VALUES (6)" >"$early_out" 2>&1 & +docker exec -i "$CNAME" mysql -h127.0.0.1 -umcp -pmcppass1 -N -e "SELECT SLEEP(8); INSERT INTO d1.t VALUES (6)" >"$early_out" 2>&1 & early=$! -sleep 1 +in_sleep() { sql "SELECT COUNT(*) FROM performance_schema.processlist WHERE USER = 'mcp' AND INFO LIKE 'SELECT SLEEP(8)%'"; } +for _ in $(seq 1 20); do [ "$(in_sleep)" = 1 ] && break; sleep 0.25; done +expect "the MCP session is connected before the install" "$(in_sleep)" "^1$" expect "reinstalled while an MCP session is connected" "$(sql "INSTALL COMPONENT 'file://component_mcp_guard'")" "^$" +expect "the MCP session is still waiting after the install" "$(in_sleep)" "^1$" wait "$early" denied "a session connected before INSTALL is guarded at its next statement" "$(grep -v "Using a password" "$early_out")" "mcp.read_only" rm -f "$early_out" From b08611ea15e771cf7937767df645ea1c90200a0a Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sun, 1 Nov 2026 20:02:53 +0000 Subject: [PATCH 4/4] test(mcp-guard): keep the session idle through the install (review of #252) CodeRabbit: with SLEEP running during INSTALL, the guard could classify the session at that statement's end. The client now stays connected and idle (fed through a FIFO) through the install, and its INSERT is sent on the same connection afterwards, so the check exercises classification at the first statement after the install (123 checks on 8.4.11, 9.7.2, 26.10.0). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- docs/MCP_COMPONENTS.md | 2 +- scripts/test-mcp-guard.sh | 25 ++++++++++++++----------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a696f55..437d459c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,7 +139,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The guard now classifies an unclassified session from its privileges at its first statement. A "not MCP" marker keeps the cost at one grant check per session. - `mcp.sessions_connected` is decremented when the session's THD ends, which also covers sessions without a disconnect event. - These sessions also start **unaudited** (no events at all), so a component must emit connection audit events for them. This is documented in `docs/MCP_COMPONENTS.md`, and `component_mcp` will do it. - - `scripts/test-mcp-guard.sh`: 124 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. + - `scripts/test-mcp-guard.sh`: 123 checks on 8.4.11, 9.7.2 and 26.10.0, with the test-only `component_mcp_session_probe`. The unfixed guard fails the local-session checks. - **`component_mcp_guard`: table reads the schema rules could not see** (found reviewing the code behind #247). The server raises table access events only for `SELECT`, data changes, `ANALYZE` and `HANDLER READ`. An MCP session could therefore read any table, `mysql.user` included, past `mcp.allowed_schemas` and `mcp.deny_system_schemas`: - directly, with a subquery in `SET` or `DO` (`SET @x = (SELECT … FROM mysql.user); SELECT @x`), also through `PREPARE`/`EXECUTE`; - as a yes/no oracle, with a subquery in `SHOW … WHERE` or in a `CALL` argument; diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index 40a09acb..bc61fdd3 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -179,6 +179,6 @@ The option is `mcp.read_only`, `mcp.deny_system_schemas`, `mcp.allowed_schemas` ### Tests -`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (124 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. +`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases, the tool-call table and its labels, views, multi-statement packets, statements without table access events, sessions opened by components and the parity table below against the official image (123 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. **Parity with mysql-mcp-server** (RFC 0001 §5.2 item 7): [`sql/mcp/guard-parity.tsv`](https://github.com/askdba/myvector/blob/main/sql/mcp/guard-parity.tsv) lists statements a `run_query` call might send, with the verdict of mysql-mcp-server's validator (`go`) and of the guard under the default settings (`guard`). Every row where they differ says why. The test checks the guard column, sending each statement as one packet. mysql-mcp-server vendors the file and checks the go column. Both reject every write and locking read. The guard allows a few things the validator does not (session `SET`, `information_schema`, `CALL mysql_mcp.*`) and catches what text checks cannot (a write inside a function called from `SELECT`). diff --git a/scripts/test-mcp-guard.sh b/scripts/test-mcp-guard.sh index 1424431a..61dedc0d 100755 --- a/scripts/test-mcp-guard.sh +++ b/scripts/test-mcp-guard.sh @@ -316,20 +316,23 @@ expect "MCP_ADMIN unknown after uninstall" "$(sql "GRANT MCP_ADMIN ON *.* TO app expect "mcp.* variables gone after uninstall" "$(sql "SHOW VARIABLES LIKE 'mcp.%'")" "^$" expect "performance_schema.mcp_tool_calls gone after uninstall" "$(sql "SELECT COUNT(*) FROM performance_schema.mcp_tool_calls")" "ERROR 1146" expect "no enforcement after uninstall" "$(mcp "INSERT INTO d1.t VALUES (5); SELECT COUNT(*) FROM d1.t")" "^2$" -# A session connected before INSTALL COMPONENT is classified at its next statement. The -# session is confirmed connected and idle (in SLEEP) before the install, and still in SLEEP -# after it, so its INSERT runs only once the guard is installed. -early_out=$(mktemp) -docker exec -i "$CNAME" mysql -h127.0.0.1 -umcp -pmcppass1 -N -e "SELECT SLEEP(8); INSERT INTO d1.t VALUES (6)" >"$early_out" 2>&1 & +# A session connected before INSTALL COMPONENT is classified at its first statement after it. +# The client is connected and idle (no statement running) through the install; its INSERT is +# then sent over the same connection, through a FIFO. +early_dir=$(mktemp -d) +mkfifo "$early_dir/in" +docker exec -i "$CNAME" mysql -h127.0.0.1 -umcp -pmcppass1 -N <"$early_dir/in" >"$early_dir/out" 2>&1 & early=$! -in_sleep() { sql "SELECT COUNT(*) FROM performance_schema.processlist WHERE USER = 'mcp' AND INFO LIKE 'SELECT SLEEP(8)%'"; } -for _ in $(seq 1 20); do [ "$(in_sleep)" = 1 ] && break; sleep 0.25; done -expect "the MCP session is connected before the install" "$(in_sleep)" "^1$" +exec 3>"$early_dir/in" +idle_mcp() { sql "SELECT COUNT(*) FROM performance_schema.processlist WHERE USER = 'mcp' AND COMMAND = 'Sleep'"; } +for _ in $(seq 1 40); do [ "$(idle_mcp)" = 1 ] && break; sleep 0.25; done +expect "the MCP session is connected and idle before the install" "$(idle_mcp)" "^1$" expect "reinstalled while an MCP session is connected" "$(sql "INSTALL COMPONENT 'file://component_mcp_guard'")" "^$" -expect "the MCP session is still waiting after the install" "$(in_sleep)" "^1$" +printf 'INSERT INTO d1.t VALUES (6);\n' >&3 +exec 3>&- wait "$early" -denied "a session connected before INSTALL is guarded at its next statement" "$(grep -v "Using a password" "$early_out")" "mcp.read_only" -rm -f "$early_out" +denied "a session connected before INSTALL is guarded at its first statement after it" "$(grep -v "Using a password" "$early_dir/out")" "mcp.read_only" +rm -rf "$early_dir" # --- lower_case_table_names = 1: allowed_schemas compares case-insensitively --------------------- docker run -d --name "$CNAME_LCTN" -e MYSQL_ROOT_PASSWORD=testpass "mysql:$TAG" --lower-case-table-names=1 >/dev/null