From 2ba70c50f6be89c797ef0ab763bd8d6bc8d9be8a Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sat, 31 Oct 2026 22:28:15 +0000 Subject: [PATCH 1/4] fix: post-merge review findings (#249) for the mysql-mcp image and local build CodeRabbit's review of #244/#245/#246/#248 as merged (#249): - mysql-mcp image: an entrypoint wrapper checks the MYSQL_MCP_* settings before the official entrypoint initializes the data directory; MYSQL_MCP_USER equal to MYSQL_USER is refused (CREATE USER failed in first-start setup, and a restart then skipped the setup). - Health check: read port= from option files (my_print_defaults, honoring --defaults-file) when the command line sets no port. - build-docker-local.sh: GCC 11 for 9.7 (9.7.4 rejects GCC < 11), GCC 10 for 8.0/8.4, as in docker-publish.yml. - test-mysql-mcp-image.sh: option-file port, MYSQL_USER clash (25 checks). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ docker/mysql-mcp/Dockerfile | 5 +++++ docker/mysql-mcp/mysql-mcp-entrypoint.sh | 24 +++++++++++++++++++++++ docker/mysql-mcp/mysql-mcp-healthcheck.sh | 11 +++++++++-- docs/MCP_COMPONENTS.md | 2 +- scripts/build-docker-local.sh | 15 +++++++------- scripts/test-mysql-mcp-image.sh | 21 ++++++++++++++++++-- 7 files changed, 70 insertions(+), 12 deletions(-) create mode 100755 docker/mysql-mcp/mysql-mcp-entrypoint.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index ce5fc1b..3c82aec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,10 @@ 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 +- **Post-merge review of #244, #245, #246, #248** (#249; these were merged after rate-limited, not completed, CodeRabbit runs). Fixes: + - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` is refused. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check also reads `port=` from option files when the command line sets none. + - **`scripts/build-docker-local.sh`:** builds 9.7 with GCC 11 (9.7.4 needs 11+), 8.0/8.4 with GCC 10, as `docker-publish.yml` does. + - **`scripts/test-mysql-mcp-image.sh`:** option-file port and `MYSQL_USER` clash cases (25 checks). - **Plugin images pinned to their build release.** The plugin images were built for 8.0.45, 8.4.8 and 9.7.0 but based on the floating `mysql:8.0`/`mysql:8.4`/`mysql:9.7` (now 8.0.46, 8.4.13, 9.7.4), and a library built for one MySQL release can crash another. `docker-publish.yml` now builds them for **8.0.46, 8.4.13 and 9.7.4**, bases each image on `mysql:` and also tags it with the release (`mysql8.0.46`, `mysql8.4.13`, `mysql9.7.4`). The plugin job also runs on a `dry_run` dispatch, building and testing without pushing. `Dockerfile.oraclelinux8`/`9` now require `MYSQL_VERSION`. `scripts/build-docker-local.sh` builds 8.0, 8.4 or 9.7 on the same releases and rejects other versions (it used to map 9.0 to 9.0.0). - **Component images pinned to their build release.** A component works only on the exact MySQL release it was built against: in a dry run, a `component_mcp_guard` built for 8.4.8 crashed `mysql:8.4` (8.4.11). The images were built for 8.4.8 and 9.7.0 but ran on the floating `mysql:8.4`/`mysql:9.7`. `docker-publish.yml` now builds the component images for **8.4.13, 9.7.4 and 26.10.0** (the releases the floating tags now point to) and bases each one on `mysql:`. It also tags each image with its release (`mysql8.4.13-component`, `mysql9.7.4-component`, `mysql26.10.0`), and the artifact smoke test runs on the same release. `Dockerfile.component` now requires `MYSQL_VERSION` (no default). - **Component build scripts reconfigure the server tree after a failed configure.** A `.configure-ok` marker is written only after `cmake` succeeds; before, the scripts trusted `CMakeCache.txt`, which appears before configuration finishes, so a failed configure was skipped on the next run. diff --git a/docker/mysql-mcp/Dockerfile b/docker/mysql-mcp/Dockerfile index e258417..06cc6e6 100644 --- a/docker/mysql-mcp/Dockerfile +++ b/docker/mysql-mcp/Dockerfile @@ -21,8 +21,13 @@ COPY component_mcp_guard-${TARGETARCH}.so /usr/lib64/mysql/plugin/component_mcp_ COPY sql/mcp/mysql_mcp.sql sql/mcp/uninstall.sql sql/mcp/LICENSE /usr/share/mysql-mcp/ COPY docker/mysql-mcp/mysql-mcp-init.sh /docker-entrypoint-initdb.d/50-mysql-mcp.sh COPY docker/mysql-mcp/mysql-mcp-healthcheck.sh /usr/local/bin/mysql-mcp-healthcheck +COPY docker/mysql-mcp/mysql-mcp-entrypoint.sh /usr/local/bin/mysql-mcp-entrypoint # checkov:skip=CKV_DOCKER_3:MySQL entrypoint needs root for initialization # Healthy only after first-start setup, on the server's port, without touching root's # login (see the script). HEALTHCHECK --interval=30s --timeout=5s --retries=5 CMD ["/usr/local/bin/mysql-mcp-healthcheck"] + +# Checks the MYSQL_MCP_* settings before the official entrypoint initializes anything. +ENTRYPOINT ["mysql-mcp-entrypoint"] +CMD ["mysqld"] diff --git a/docker/mysql-mcp/mysql-mcp-entrypoint.sh b/docker/mysql-mcp/mysql-mcp-entrypoint.sh new file mode 100755 index 0000000..511bc0f --- /dev/null +++ b/docker/mysql-mcp/mysql-mcp-entrypoint.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# Entrypoint of the mysql-mcp image: checks the MYSQL_MCP_* settings, then runs the official +# entrypoint. The checks run before the data directory is initialized, because a first-start +# script that fails leaves an initialized data directory whose setup a restart skips. +set -eo pipefail + +mcp_error() { + printf '%s [ERROR] [Entrypoint]: mysql-mcp: %s\n' "$(date --rfc-3339=seconds)" "$*" >&2 + exit 1 +} + +if [ -n "${MYSQL_MCP_USER:-}" ]; then + [ -n "${MYSQL_MCP_PASSWORD:-}" ] || mcp_error "MYSQL_MCP_USER is set but MYSQL_MCP_PASSWORD is not" + [ "$MYSQL_MCP_USER" != root ] || mcp_error "MYSQL_MCP_USER cannot be root" + # The official entrypoint creates MYSQL_USER with ALL on MYSQL_DATABASE; the MCP account + # must be a separate, read-only one. + [ "$MYSQL_MCP_USER" != "${MYSQL_USER:-}" ] || + mcp_error "MYSQL_MCP_USER must differ from MYSQL_USER (that account gets all privileges on MYSQL_DATABASE)" +fi +case "${MYSQL_MCP_MAX_EXECUTION_TIME_MS:-}" in +*[!0-9]*) mcp_error "MYSQL_MCP_MAX_EXECUTION_TIME_MS must be a number of milliseconds" ;; +esac + +exec docker-entrypoint.sh "$@" diff --git a/docker/mysql-mcp/mysql-mcp-healthcheck.sh b/docker/mysql-mcp/mysql-mcp-healthcheck.sh index 4865e75..35fca84 100755 --- a/docker/mysql-mcp/mysql-mcp-healthcheck.sh +++ b/docker/mysql-mcp/mysql-mcp-healthcheck.sh @@ -3,14 +3,21 @@ # # - TCP: the entrypoint's temporary setup server does not listen on TCP, so the check # passes only once the real server runs. -# - The port mysqld was started with (--port=N or --port N on its command line; else 3306). +# - The port mysqld uses: --port=N or --port N on its command line, else port= in its option +# files (read with my_print_defaults, honoring --defaults-file / --defaults-extra-file), +# else 3306. # - As a user that does not exist, with no password: ping reports a running server even # when access is denied, and a nonexistent account cannot be locked by failed logins # (probing as root could lock out administrators with FAILED_LOGIN_ATTEMPTS). -port=$(tr '\0' '\n' /dev/null | awk ' +args=$(tr '\0' '\n' /dev/null) +port=$(printf '%s\n' "$args" | awk ' prev == "--port" || prev == "-P" { p = $0 } /^--port=/ { p = substr($0, 8) } { prev = $0 } END { print p }') +if [ -z "$port" ]; then + defaults=$(printf '%s\n' "$args" | grep -E '^--defaults-(extra-)?file=' | head -n 1) + port=$(my_print_defaults $defaults mysqld 2>/dev/null | sed -n 's/^--port=//p' | tail -n 1) +fi exec mysqladmin ping --protocol=TCP -h127.0.0.1 -P"${port:-3306}" \ -umysql-mcp-healthcheck --connect-timeout=3 --silent diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index b7ef66f..cb9bb58 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -35,7 +35,7 @@ GRANT EXECUTE ON mysql_mcp.* TO 'mcp'@'%'; -- if the SQL tool pack is install | `MYSQL_MCP_ALLOWED_SCHEMAS` | Persisted as `mcp.allowed_schemas`. | | `MYSQL_MCP_MAX_EXECUTION_TIME_MS` | Persisted as `mcp.max_execution_time_ms`. | -The other settings keep their defaults (read-only on). The official image's own variables (`MYSQL_ROOT_PASSWORD`, `MYSQL_DATABASE`, ...) work as usual. +The other settings keep their defaults (read-only on). The settings are checked before the data directory is initialized, and a container with an invalid combination does not start. For example, `MYSQL_MCP_USER` must differ from `MYSQL_USER`, because the official entrypoint gives `MYSQL_USER` all privileges on `MYSQL_DATABASE`. The health check pings the port mysqld uses (`--port`, or `port=` in a mounted option file) without logging in as root. The official image's own variables (`MYSQL_ROOT_PASSWORD`, `MYSQL_DATABASE`, ...) work as usual. ```sh docker run -d -p 3306:3306 -e MYSQL_ROOT_PASSWORD=secret -e MYSQL_DATABASE=shop \ diff --git a/scripts/build-docker-local.sh b/scripts/build-docker-local.sh index 643d927..fcd0d44 100755 --- a/scripts/build-docker-local.sh +++ b/scripts/build-docker-local.sh @@ -12,10 +12,11 @@ REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" cd "$REPO_ROOT" MYSQL_VERSION="${1:-8.4}" +# GCC_MAJOR: the GCC the release builds with, as in docker-publish.yml (9.7 needs 11+). case "$MYSQL_VERSION" in -8.0) MYSQL_TAG="mysql-8.0.46" ;; -8.4) MYSQL_TAG="mysql-8.4.13" ;; -9.7) MYSQL_TAG="mysql-9.7.4" ;; +8.0) MYSQL_TAG="mysql-8.0.46" GCC_MAJOR=10 ;; +8.4) MYSQL_TAG="mysql-8.4.13" GCC_MAJOR=10 ;; +9.7) MYSQL_TAG="mysql-9.7.4" GCC_MAJOR=11 ;; *) echo "unsupported MySQL version: $MYSQL_VERSION (use 8.0, 8.4 or 9.7)" >&2 exit 1 @@ -56,9 +57,9 @@ echo "Using make -j$MAKE_JOBS (set MYVECTOR_BUILD_JOBS to override)" echo "" # Apple Silicon host: GCC ICEs for both arm64 and amd64 (QEMU). Use Clang for both. -# x86_64 host: use gcc-10. Allow manual override via MYVECTOR_CC/MYVECTOR_CXX. -CC="${MYVECTOR_CC:-gcc-10}" -CXX="${MYVECTOR_CXX:-g++-10}" +# Linux: gcc-$GCC_MAJOR for the release. Allow manual override via MYVECTOR_CC/MYVECTOR_CXX. +CC="${MYVECTOR_CC:-gcc-$GCC_MAJOR}" +CXX="${MYVECTOR_CXX:-g++-$GCC_MAJOR}" CMAKE_EXTRA="" [ "$HOST_ARM64" -eq 1 ] && [ -z "${MYVECTOR_CC:-}" ] && [ -z "${MYVECTOR_CXX:-}" ] && CC=clang CXX=clang++ CMAKE_EXTRA="" @@ -75,7 +76,7 @@ docker run --rm --privileged --platform "$PLATFORM" \ ubuntu:22.04 bash -c " set -e apt-get update -qq - apt-get install -y -qq cmake clang g++-10 gcc-10 git libssl-dev libncurses5-dev \ + apt-get install -y -qq cmake clang g++-$GCC_MAJOR gcc-$GCC_MAJOR git libssl-dev libncurses5-dev \ pkg-config bison libtirpc-dev libldap2-dev libsasl2-dev libudev-dev \ libre2-dev libcurl4-openssl-dev libprotobuf-dev protobuf-compiler linux-libc-dev rm -rf /tmp/mysql-server diff --git a/scripts/test-mysql-mcp-image.sh b/scripts/test-mysql-mcp-image.sh index 9929fd9..b72178a 100755 --- a/scripts/test-mysql-mcp-image.sh +++ b/scripts/test-mysql-mcp-image.sh @@ -68,9 +68,16 @@ health "$CNAME" >/dev/null; health "$CNAME" >/dev/null; health "$CNAME" >/dev/nu expect "health probes do not lock root" "$(root "SELECT 1")" "^1$" root "ALTER USER root@'%' FAILED_LOGIN_ATTEMPTS 0 PASSWORD_LOCK_TIME 0" >/dev/null -# Without MYSQL_MCP_* settings: component and tool pack only. -docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass "$IMAGE" >/dev/null +# Without MYSQL_MCP_* settings: component and tool pack only. This server's port comes from +# an option file (as with a mounted my.cnf), which the health check must read. +PORT=3308 +docker create --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass "$IMAGE" >/dev/null +printf '[mysqld]\nport=3308\n' >"/tmp/$CNAME-port.cnf" +docker cp "/tmp/$CNAME-port.cnf" "$CNAME_PLAIN:/etc/mysql/conf.d/port.cnf" >/dev/null +rm -f "/tmp/$CNAME-port.cnf" +docker start "$CNAME_PLAIN" >/dev/null if wait_ready "$CNAME_PLAIN" testpass; then + expect "health check reads the port from an option file" "$(health "$CNAME_PLAIN")" "rc=0" expect "plain: component and tool pack" "$(as "$CNAME_PLAIN" root testpass "SELECT COUNT(*) FROM mysql.component WHERE component_urn LIKE '%mcp_guard'; CALL mysql_mcp.version()" | tr '\n' ' ')" "^1 [0-9]+\.[0-9]+\.[0-9]+ $" expect "plain: default settings" "$(as "$CNAME_PLAIN" root testpass "SELECT CONCAT_WS(',', @@mcp.enabled, @@mcp.read_only, IFNULL(@@mcp.allowed_schemas, ''))")" "^1,1,$" expect "plain: no MCP account" "$(as "$CNAME_PLAIN" root testpass "SELECT COUNT(*) FROM mysql.user WHERE user = 'mcp'")" "^0$" @@ -78,6 +85,16 @@ else bad "plain container" "did not start" fi docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 +unset PORT + +# MYSQL_MCP_USER = MYSQL_USER is refused before the data directory is initialized (the +# official entrypoint would create that account with all privileges on MYSQL_DATABASE). +docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e MYSQL_DATABASE=app \ + -e MYSQL_USER=app -e MYSQL_PASSWORD=apppass -e MYSQL_MCP_USER=app -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null +rc=$(timeout 60 docker wait "$CNAME_PLAIN" 2>/dev/null || echo timeout) +expect "MYSQL_MCP_USER = MYSQL_USER is refused at start" "rc=$rc $(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'MYSQL_MCP_USER must differ from MYSQL_USER')" "^rc=[1-9][0-9]* 1$" +expect "nothing initialized when refused" "$(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'Initializing database files')" "^0$" +docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 # NO_BACKSLASH_ESCAPES: the password with a backslash still works as configured. This # server also runs on another port, which the health check must follow. From f633ec45b19a75876373d86468c55b4701a74e66 Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sat, 31 Oct 2026 23:34:10 +0000 Subject: [PATCH 2/4] =?UTF-8?q?fix(docker):=20review=20of=20#250=20?= =?UTF-8?q?=E2=80=94=20mysqld=20resolves=20the=20health-check=20port;=20MY?= =?UTF-8?q?SQL=5FUSER=5FFILE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on 2ba70c5: - Health check: ask mysqld for its port (mysqld --verbose --help), so option groups ([server], [mysqld-]), --defaults-file (spaces too) and --no-defaults apply exactly as for the server; not ready while PID 1 is still the entrypoint (first-start setup). - Entrypoint: compare MYSQL_MCP_USER with the user in MYSQL_USER_FILE too. - test-mysql-mcp-image.sh: mktemp instead of a predictable /tmp path; a portable bounded wait instead of timeout(1); [server] group port and MYSQL_USER_FILE clash cases (27 checks). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 +-- docker/mysql-mcp/mysql-mcp-entrypoint.sh | 9 +++-- docker/mysql-mcp/mysql-mcp-healthcheck.sh | 25 +++++-------- docs/MCP_COMPONENTS.md | 2 +- scripts/test-mysql-mcp-image.sh | 44 +++++++++++++++++------ 5 files changed, 52 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c82aec..98d70ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -136,9 +136,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **Post-merge review of #244, #245, #246, #248** (#249; these were merged after rate-limited, not completed, CodeRabbit runs). Fixes: - - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` is refused. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check also reads `port=` from option files when the command line sets none. + - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` (or the user in `MYSQL_USER_FILE`) is refused. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check now asks mysqld for its port (`mysqld --verbose --help`), so option files, option groups, `--defaults-file` and `--no-defaults` apply exactly as for the server. It reports not ready while first-start setup runs. - **`scripts/build-docker-local.sh`:** builds 9.7 with GCC 11 (9.7.4 needs 11+), 8.0/8.4 with GCC 10, as `docker-publish.yml` does. - - **`scripts/test-mysql-mcp-image.sh`:** option-file port and `MYSQL_USER` clash cases (25 checks). + - **`scripts/test-mysql-mcp-image.sh`:** option-file port (`[server]` group), `MYSQL_USER` and `MYSQL_USER_FILE` clash cases (27 checks). It uses `mktemp` and a portable wait. - **Plugin images pinned to their build release.** The plugin images were built for 8.0.45, 8.4.8 and 9.7.0 but based on the floating `mysql:8.0`/`mysql:8.4`/`mysql:9.7` (now 8.0.46, 8.4.13, 9.7.4), and a library built for one MySQL release can crash another. `docker-publish.yml` now builds them for **8.0.46, 8.4.13 and 9.7.4**, bases each image on `mysql:` and also tags it with the release (`mysql8.0.46`, `mysql8.4.13`, `mysql9.7.4`). The plugin job also runs on a `dry_run` dispatch, building and testing without pushing. `Dockerfile.oraclelinux8`/`9` now require `MYSQL_VERSION`. `scripts/build-docker-local.sh` builds 8.0, 8.4 or 9.7 on the same releases and rejects other versions (it used to map 9.0 to 9.0.0). - **Component images pinned to their build release.** A component works only on the exact MySQL release it was built against: in a dry run, a `component_mcp_guard` built for 8.4.8 crashed `mysql:8.4` (8.4.11). The images were built for 8.4.8 and 9.7.0 but ran on the floating `mysql:8.4`/`mysql:9.7`. `docker-publish.yml` now builds the component images for **8.4.13, 9.7.4 and 26.10.0** (the releases the floating tags now point to) and bases each one on `mysql:`. It also tags each image with its release (`mysql8.4.13-component`, `mysql9.7.4-component`, `mysql26.10.0`), and the artifact smoke test runs on the same release. `Dockerfile.component` now requires `MYSQL_VERSION` (no default). - **Component build scripts reconfigure the server tree after a failed configure.** A `.configure-ok` marker is written only after `cmake` succeeds; before, the scripts trusted `CMakeCache.txt`, which appears before configuration finishes, so a failed configure was skipped on the next run. diff --git a/docker/mysql-mcp/mysql-mcp-entrypoint.sh b/docker/mysql-mcp/mysql-mcp-entrypoint.sh index 511bc0f..25b3b4e 100755 --- a/docker/mysql-mcp/mysql-mcp-entrypoint.sh +++ b/docker/mysql-mcp/mysql-mcp-entrypoint.sh @@ -13,8 +13,13 @@ if [ -n "${MYSQL_MCP_USER:-}" ]; then [ -n "${MYSQL_MCP_PASSWORD:-}" ] || mcp_error "MYSQL_MCP_USER is set but MYSQL_MCP_PASSWORD is not" [ "$MYSQL_MCP_USER" != root ] || mcp_error "MYSQL_MCP_USER cannot be root" # The official entrypoint creates MYSQL_USER with ALL on MYSQL_DATABASE; the MCP account - # must be a separate, read-only one. - [ "$MYSQL_MCP_USER" != "${MYSQL_USER:-}" ] || + # must be a separate, read-only one. MYSQL_USER may come from MYSQL_USER_FILE (when both + # are set, the official entrypoint refuses them itself). + app_user=${MYSQL_USER:-} + if [ -z "$app_user" ] && [ -n "${MYSQL_USER_FILE:-}" ] && [ -r "$MYSQL_USER_FILE" ]; then + app_user=$(<"$MYSQL_USER_FILE") + fi + [ "$MYSQL_MCP_USER" != "$app_user" ] || mcp_error "MYSQL_MCP_USER must differ from MYSQL_USER (that account gets all privileges on MYSQL_DATABASE)" fi case "${MYSQL_MCP_MAX_EXECUTION_TIME_MS:-}" in diff --git a/docker/mysql-mcp/mysql-mcp-healthcheck.sh b/docker/mysql-mcp/mysql-mcp-healthcheck.sh index 35fca84..44a457c 100755 --- a/docker/mysql-mcp/mysql-mcp-healthcheck.sh +++ b/docker/mysql-mcp/mysql-mcp-healthcheck.sh @@ -1,23 +1,16 @@ -#!/bin/sh +#!/bin/bash # Health check of the mysql-mcp image: is mysqld up and done with first-start setup? # -# - TCP: the entrypoint's temporary setup server does not listen on TCP, so the check -# passes only once the real server runs. -# - The port mysqld uses: --port=N or --port N on its command line, else port= in its option -# files (read with my_print_defaults, honoring --defaults-file / --defaults-extra-file), -# else 3306. +# - Not ready while PID 1 is still the entrypoint (first-start setup runs a temporary +# server, which does not listen on TCP either). +# - The port: mysqld itself resolves it from its own arguments and option files +# (`mysqld --verbose --help`), so command line, option groups ([mysqld], [server], +# [mysqld-]), --defaults-file and --no-defaults apply exactly as for the server. # - As a user that does not exist, with no password: ping reports a running server even # when access is denied, and a nonexistent account cannot be locked by failed logins # (probing as root could lock out administrators with FAILED_LOGIN_ATTEMPTS). -args=$(tr '\0' '\n' /dev/null) -port=$(printf '%s\n' "$args" | awk ' - prev == "--port" || prev == "-P" { p = $0 } - /^--port=/ { p = substr($0, 8) } - { prev = $0 } - END { print p }') -if [ -z "$port" ]; then - defaults=$(printf '%s\n' "$args" | grep -E '^--defaults-(extra-)?file=' | head -n 1) - port=$(my_print_defaults $defaults mysqld 2>/dev/null | sed -n 's/^--port=//p' | tail -n 1) -fi +mapfile -d '' args /dev/null || exit 1 +[ "${args[0]##*/}" = mysqld ] || exit 1 +port=$("${args[0]}" "${args[@]:1}" --verbose --help 2>/dev/null | awk '$1 == "port" { print $2; exit }') exec mysqladmin ping --protocol=TCP -h127.0.0.1 -P"${port:-3306}" \ -umysql-mcp-healthcheck --connect-timeout=3 --silent diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index cb9bb58..5a396c5 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -35,7 +35,7 @@ GRANT EXECUTE ON mysql_mcp.* TO 'mcp'@'%'; -- if the SQL tool pack is install | `MYSQL_MCP_ALLOWED_SCHEMAS` | Persisted as `mcp.allowed_schemas`. | | `MYSQL_MCP_MAX_EXECUTION_TIME_MS` | Persisted as `mcp.max_execution_time_ms`. | -The other settings keep their defaults (read-only on). The settings are checked before the data directory is initialized, and a container with an invalid combination does not start. For example, `MYSQL_MCP_USER` must differ from `MYSQL_USER`, because the official entrypoint gives `MYSQL_USER` all privileges on `MYSQL_DATABASE`. The health check pings the port mysqld uses (`--port`, or `port=` in a mounted option file) without logging in as root. The official image's own variables (`MYSQL_ROOT_PASSWORD`, `MYSQL_DATABASE`, ...) work as usual. +The other settings keep their defaults (read-only on). The settings are checked before the data directory is initialized, and a container with an invalid combination does not start. For example, `MYSQL_MCP_USER` must differ from `MYSQL_USER` (or the user in `MYSQL_USER_FILE`), because the official entrypoint gives that account all privileges on `MYSQL_DATABASE`. The health check asks mysqld for the port it uses (command line and option files, as mysqld resolves them) and pings it without logging in as root. The official image's own variables (`MYSQL_ROOT_PASSWORD`, `MYSQL_DATABASE`, ...) work as usual. ```sh docker run -d -p 3306:3306 -e MYSQL_ROOT_PASSWORD=secret -e MYSQL_DATABASE=shop \ diff --git a/scripts/test-mysql-mcp-image.sh b/scripts/test-mysql-mcp-image.sh index b72178a..5817bff 100755 --- a/scripts/test-mysql-mcp-image.sh +++ b/scripts/test-mysql-mcp-image.sh @@ -15,7 +15,9 @@ PASS=0 FAIL=0 ok() { PASS=$((PASS + 1)); printf ' PASS %s\n' "$1"; } bad() { FAIL=$((FAIL + 1)); printf ' FAIL %s -- %s\n' "$1" "$2"; } -cleanup() { docker rm -f -v "$CNAME" "$CNAME_PLAIN" >/dev/null 2>&1; } +TMPF=$(mktemp) +chmod 644 "$TMPF" # copied into containers, where mysqld (user mysql) must read it +cleanup() { docker rm -f -v "$CNAME" "$CNAME_PLAIN" >/dev/null 2>&1; rm -f "$TMPF"; } trap cleanup EXIT expect() { if printf '%s\n' "$2" | grep -Eq -- "$3"; then ok "$1"; else bad "$1" "got: $(printf '%s' "$2" | head -c 300)"; fi @@ -69,12 +71,12 @@ expect "health probes do not lock root" "$(root "SELECT 1")" "^1$" root "ALTER USER root@'%' FAILED_LOGIN_ATTEMPTS 0 PASSWORD_LOCK_TIME 0" >/dev/null # Without MYSQL_MCP_* settings: component and tool pack only. This server's port comes from -# an option file (as with a mounted my.cnf), which the health check must read. +# the [server] group of an option file (as with a mounted my.cnf), which the health check +# must apply as mysqld does. PORT=3308 docker create --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass "$IMAGE" >/dev/null -printf '[mysqld]\nport=3308\n' >"/tmp/$CNAME-port.cnf" -docker cp "/tmp/$CNAME-port.cnf" "$CNAME_PLAIN:/etc/mysql/conf.d/port.cnf" >/dev/null -rm -f "/tmp/$CNAME-port.cnf" +printf '[server]\nport=3308\n' >"$TMPF" +docker cp "$TMPF" "$CNAME_PLAIN:/etc/mysql/conf.d/port.cnf" >/dev/null docker start "$CNAME_PLAIN" >/dev/null if wait_ready "$CNAME_PLAIN" testpass; then expect "health check reads the port from an option file" "$(health "$CNAME_PLAIN")" "rc=0" @@ -87,14 +89,34 @@ fi docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 unset PORT -# MYSQL_MCP_USER = MYSQL_USER is refused before the data directory is initialized (the -# official entrypoint would create that account with all privileges on MYSQL_DATABASE). +# MYSQL_MCP_USER = MYSQL_USER (or the user in MYSQL_USER_FILE) is refused before the data +# directory is initialized (the official entrypoint would create that account with all +# privileges on MYSQL_DATABASE). +# exit_code CONTAINER: its exit code once it stops, or "timeout" after 60 s. +exit_code() { + for _ in $(seq 1 60); do + case "$(docker inspect -f '{{.State.Status}}' "$1" 2>/dev/null)" in + running | restarting | created | paused) sleep 1 ;; + '') break ;; + *) docker inspect -f '{{.State.ExitCode}}' "$1"; return ;; + esac + done + echo timeout +} +refused() { # LABEL: CNAME_PLAIN must have stopped with the clash message, uninitialized + expect "$1 is refused at start" "rc=$(exit_code "$CNAME_PLAIN") $(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'MYSQL_MCP_USER must differ from MYSQL_USER')" "^rc=[1-9][0-9]* 1$" + expect "$1: nothing initialized" "$(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'Initializing database files')" "^0$" + docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 +} docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e MYSQL_DATABASE=app \ -e MYSQL_USER=app -e MYSQL_PASSWORD=apppass -e MYSQL_MCP_USER=app -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null -rc=$(timeout 60 docker wait "$CNAME_PLAIN" 2>/dev/null || echo timeout) -expect "MYSQL_MCP_USER = MYSQL_USER is refused at start" "rc=$rc $(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'MYSQL_MCP_USER must differ from MYSQL_USER')" "^rc=[1-9][0-9]* 1$" -expect "nothing initialized when refused" "$(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'Initializing database files')" "^0$" -docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 +refused "MYSQL_MCP_USER = MYSQL_USER" +docker create --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e MYSQL_DATABASE=app \ + -e MYSQL_USER_FILE=/run/app-user -e MYSQL_PASSWORD=apppass -e MYSQL_MCP_USER=app -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null +printf 'app' >"$TMPF" +docker cp "$TMPF" "$CNAME_PLAIN:/run/app-user" >/dev/null +docker start "$CNAME_PLAIN" >/dev/null +refused "MYSQL_MCP_USER = the user in MYSQL_USER_FILE" # NO_BACKSLASH_ESCAPES: the password with a backslash still works as configured. This # server also runs on another port, which the health check must follow. From 4d0ba0e7cc5755346b2127836d1002ccd2ddd7bc Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sun, 1 Nov 2026 00:32:43 +0000 Subject: [PATCH 3/4] =?UTF-8?q?fix(docker):=20review=20of=20#250=20?= =?UTF-8?q?=E2=80=94=20plain=20MCP=20user=20names;=20isolated=20binlog=20i?= =?UTF-8?q?ndex?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on f633ec4: - The official entrypoint puts MYSQL_USER into SQL unescaped, so app\q creates appq and passed the comparison with MYSQL_MCP_USER=appq. MYSQL_MCP_USER must now be a plain name ([A-Za-z0-9_.-], <= 32), and a MYSQL_USER with a backslash or quote is refused when MYSQL_MCP_USER is set. - Health check: the port lookup passes a throwaway --log-bin-index, as the official entrypoint does, so the live binary log index is never touched. - test-mysql-mcp-image.sh: both refusals (31 checks). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++-- docker/mysql-mcp/mysql-mcp-entrypoint.sh | 7 +++++++ docker/mysql-mcp/mysql-mcp-healthcheck.sh | 5 ++++- docs/MCP_COMPONENTS.md | 2 +- scripts/test-mysql-mcp-image.sh | 10 ++++++++-- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98d70ab..2a889a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -136,9 +136,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **Post-merge review of #244, #245, #246, #248** (#249; these were merged after rate-limited, not completed, CodeRabbit runs). Fixes: - - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` (or the user in `MYSQL_USER_FILE`) is refused. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check now asks mysqld for its port (`mysqld --verbose --help`), so option files, option groups, `--defaults-file` and `--no-defaults` apply exactly as for the server. It reports not ready while first-start setup runs. + - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` (or the user in `MYSQL_USER_FILE`) is refused. So is a `MYSQL_USER` with a backslash or quote, which the official entrypoint passes to SQL unescaped (`app\q` creates `appq`). `MYSQL_MCP_USER` must be a plain name. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check now asks mysqld for its port (`mysqld --verbose --help`, with a throwaway `--log-bin-index` as the official entrypoint uses), so option files, option groups, `--defaults-file` and `--no-defaults` apply exactly as for the server. It reports not ready while first-start setup runs. - **`scripts/build-docker-local.sh`:** builds 9.7 with GCC 11 (9.7.4 needs 11+), 8.0/8.4 with GCC 10, as `docker-publish.yml` does. - - **`scripts/test-mysql-mcp-image.sh`:** option-file port (`[server]` group), `MYSQL_USER` and `MYSQL_USER_FILE` clash cases (27 checks). It uses `mktemp` and a portable wait. + - **`scripts/test-mysql-mcp-image.sh`:** option-file port (`[server]` group), `MYSQL_USER` and `MYSQL_USER_FILE` clash cases, plain-name checks (31 checks). It uses `mktemp` and a portable wait. - **Plugin images pinned to their build release.** The plugin images were built for 8.0.45, 8.4.8 and 9.7.0 but based on the floating `mysql:8.0`/`mysql:8.4`/`mysql:9.7` (now 8.0.46, 8.4.13, 9.7.4), and a library built for one MySQL release can crash another. `docker-publish.yml` now builds them for **8.0.46, 8.4.13 and 9.7.4**, bases each image on `mysql:` and also tags it with the release (`mysql8.0.46`, `mysql8.4.13`, `mysql9.7.4`). The plugin job also runs on a `dry_run` dispatch, building and testing without pushing. `Dockerfile.oraclelinux8`/`9` now require `MYSQL_VERSION`. `scripts/build-docker-local.sh` builds 8.0, 8.4 or 9.7 on the same releases and rejects other versions (it used to map 9.0 to 9.0.0). - **Component images pinned to their build release.** A component works only on the exact MySQL release it was built against: in a dry run, a `component_mcp_guard` built for 8.4.8 crashed `mysql:8.4` (8.4.11). The images were built for 8.4.8 and 9.7.0 but ran on the floating `mysql:8.4`/`mysql:9.7`. `docker-publish.yml` now builds the component images for **8.4.13, 9.7.4 and 26.10.0** (the releases the floating tags now point to) and bases each one on `mysql:`. It also tags each image with its release (`mysql8.4.13-component`, `mysql9.7.4-component`, `mysql26.10.0`), and the artifact smoke test runs on the same release. `Dockerfile.component` now requires `MYSQL_VERSION` (no default). - **Component build scripts reconfigure the server tree after a failed configure.** A `.configure-ok` marker is written only after `cmake` succeeds; before, the scripts trusted `CMakeCache.txt`, which appears before configuration finishes, so a failed configure was skipped on the next run. diff --git a/docker/mysql-mcp/mysql-mcp-entrypoint.sh b/docker/mysql-mcp/mysql-mcp-entrypoint.sh index 25b3b4e..cdf5052 100755 --- a/docker/mysql-mcp/mysql-mcp-entrypoint.sh +++ b/docker/mysql-mcp/mysql-mcp-entrypoint.sh @@ -12,6 +12,8 @@ mcp_error() { if [ -n "${MYSQL_MCP_USER:-}" ]; then [ -n "${MYSQL_MCP_PASSWORD:-}" ] || mcp_error "MYSQL_MCP_USER is set but MYSQL_MCP_PASSWORD is not" [ "$MYSQL_MCP_USER" != root ] || mcp_error "MYSQL_MCP_USER cannot be root" + [[ "$MYSQL_MCP_USER" =~ ^[A-Za-z0-9_.-]{1,32}$ ]] || + mcp_error "MYSQL_MCP_USER must be 1-32 characters from A-Z, a-z, 0-9, _ . -" # The official entrypoint creates MYSQL_USER with ALL on MYSQL_DATABASE; the MCP account # must be a separate, read-only one. MYSQL_USER may come from MYSQL_USER_FILE (when both # are set, the official entrypoint refuses them itself). @@ -19,6 +21,11 @@ if [ -n "${MYSQL_MCP_USER:-}" ]; then if [ -z "$app_user" ] && [ -n "${MYSQL_USER_FILE:-}" ] && [ -r "$MYSQL_USER_FILE" ]; then app_user=$(<"$MYSQL_USER_FILE") fi + # The official entrypoint puts MYSQL_USER into SQL unescaped, so a backslash or quote in it + # changes the account name (app\q creates appq): such a name cannot be compared. + case "$app_user" in + *\\* | *\'* | *\"*) mcp_error "MYSQL_USER contains a backslash or quote; use a plain name with MYSQL_MCP_USER" ;; + esac [ "$MYSQL_MCP_USER" != "$app_user" ] || mcp_error "MYSQL_MCP_USER must differ from MYSQL_USER (that account gets all privileges on MYSQL_DATABASE)" fi diff --git a/docker/mysql-mcp/mysql-mcp-healthcheck.sh b/docker/mysql-mcp/mysql-mcp-healthcheck.sh index 44a457c..e73e428 100755 --- a/docker/mysql-mcp/mysql-mcp-healthcheck.sh +++ b/docker/mysql-mcp/mysql-mcp-healthcheck.sh @@ -11,6 +11,9 @@ # (probing as root could lock out administrators with FAILED_LOGIN_ATTEMPTS). mapfile -d '' args /dev/null || exit 1 [ "${args[0]##*/}" = mysqld ] || exit 1 -port=$("${args[0]}" "${args[@]:1}" --verbose --help 2>/dev/null | awk '$1 == "port" { print $2; exit }') +# A throwaway binary log index, as the official entrypoint uses for the same lookup, so the +# live index is never touched. +port=$("${args[0]}" "${args[@]:1}" --verbose --help --log-bin-index="$(mktemp -u)" 2>/dev/null | + awk '$1 == "port" { print $2; exit }') exec mysqladmin ping --protocol=TCP -h127.0.0.1 -P"${port:-3306}" \ -umysql-mcp-healthcheck --connect-timeout=3 --silent diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index 5a396c5..2e55e40 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -30,7 +30,7 @@ GRANT EXECUTE ON mysql_mcp.* TO 'mcp'@'%'; -- if the SQL tool pack is install | Variable | Effect | |---|---| -| `MYSQL_MCP_USER`, `MYSQL_MCP_PASSWORD` | Create this account (host `%`) for mysql-mcp-server, with `MCP_USER`, `EXECUTE` on `mysql_mcp` and `SELECT, SHOW VIEW` on the databases below. | +| `MYSQL_MCP_USER`, `MYSQL_MCP_PASSWORD` | Create this account (host `%`; a plain name of up to 32 characters from `A-Z a-z 0-9 _ . -`) for mysql-mcp-server, with `MCP_USER`, `EXECUTE` on `mysql_mcp` and `SELECT, SHOW VIEW` on the databases below. | | `MYSQL_MCP_DATABASES` | Comma-separated databases the account may read. The default is `MYSQL_DATABASE`. | | `MYSQL_MCP_ALLOWED_SCHEMAS` | Persisted as `mcp.allowed_schemas`. | | `MYSQL_MCP_MAX_EXECUTION_TIME_MS` | Persisted as `mcp.max_execution_time_ms`. | diff --git a/scripts/test-mysql-mcp-image.sh b/scripts/test-mysql-mcp-image.sh index 5817bff..d2b3cdd 100755 --- a/scripts/test-mysql-mcp-image.sh +++ b/scripts/test-mysql-mcp-image.sh @@ -103,8 +103,8 @@ exit_code() { done echo timeout } -refused() { # LABEL: CNAME_PLAIN must have stopped with the clash message, uninitialized - expect "$1 is refused at start" "rc=$(exit_code "$CNAME_PLAIN") $(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'MYSQL_MCP_USER must differ from MYSQL_USER')" "^rc=[1-9][0-9]* 1$" +refused() { # LABEL [MESSAGE]: CNAME_PLAIN must have stopped with MESSAGE, uninitialized + expect "$1 is refused at start" "rc=$(exit_code "$CNAME_PLAIN") $(docker logs "$CNAME_PLAIN" 2>&1 | grep -cF "${2:-MYSQL_MCP_USER must differ from MYSQL_USER}")" "^rc=[1-9][0-9]* 1$" expect "$1: nothing initialized" "$(docker logs "$CNAME_PLAIN" 2>&1 | grep -c 'Initializing database files')" "^0$" docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 } @@ -117,6 +117,12 @@ printf 'app' >"$TMPF" docker cp "$TMPF" "$CNAME_PLAIN:/run/app-user" >/dev/null docker start "$CNAME_PLAIN" >/dev/null refused "MYSQL_MCP_USER = the user in MYSQL_USER_FILE" +# The official entrypoint puts MYSQL_USER into SQL unescaped: app\q would create appq. +docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e MYSQL_DATABASE=app \ + -e 'MYSQL_USER=app\q' -e MYSQL_PASSWORD=apppass -e MYSQL_MCP_USER=appq -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null +refused "MYSQL_USER with a backslash" "MYSQL_USER contains a backslash or quote" +docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e "MYSQL_MCP_USER=mcp'x" -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null +refused "MYSQL_MCP_USER that is not a plain name" "MYSQL_MCP_USER must be 1-32 characters" # NO_BACKSLASH_ESCAPES: the password with a backslash still works as configured. This # server also runs on another port, which the health check must follow. From 95117dfd4f0f49c51d8c6fc67c73c3bac51262ca Mon Sep 17 00:00:00 2001 From: Alkin Tezuysal Date: Sun, 1 Nov 2026 01:44:28 +0000 Subject: [PATCH 4/4] =?UTF-8?q?fix(docker):=20review=20of=20#250=20?= =?UTF-8?q?=E2=80=94=20check=20settings=20only=20before=20init;=20trimmed?= =?UTF-8?q?=20user=20names?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on 4d0ba0e: - The MYSQL_MCP_* checks run only while the data directory is uninitialized (found as the official entrypoint finds it), so a volume created by an earlier image with a name the checks now refuse keeps starting; the settings are not used after initialization anyway. - A MYSQL_USER with leading or trailing whitespace is refused when MYSQL_MCP_USER is set (MySQL trims it: ' app ' is app). - test-mysql-mcp-image.sh: both cases (34 checks). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 +- docker/mysql-mcp/mysql-mcp-entrypoint.sh | 59 +++++++++++++++--------- docs/MCP_COMPONENTS.md | 2 +- scripts/test-mysql-mcp-image.sh | 17 ++++++- 4 files changed, 56 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a889a5..c2ed9b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -136,9 +136,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **Post-merge review of #244, #245, #246, #248** (#249; these were merged after rate-limited, not completed, CodeRabbit runs). Fixes: - - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` (or the user in `MYSQL_USER_FILE`) is refused. So is a `MYSQL_USER` with a backslash or quote, which the official entrypoint passes to SQL unescaped (`app\q` creates `appq`). `MYSQL_MCP_USER` must be a plain name. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check now asks mysqld for its port (`mysqld --verbose --help`, with a throwaway `--log-bin-index` as the official entrypoint uses), so option files, option groups, `--defaults-file` and `--no-defaults` apply exactly as for the server. It reports not ready while first-start setup runs. + - **`mysql-mcp` image:** the settings are checked by an entrypoint wrapper before the data directory is initialized. `MYSQL_MCP_USER` equal to `MYSQL_USER` (or the user in `MYSQL_USER_FILE`) is refused. So is a `MYSQL_USER` with a backslash or quote, which the official entrypoint passes to SQL unescaped (`app\q` creates `appq`), or with leading or trailing spaces, which MySQL trims. The checks run only before initialization, so an initialized volume keeps starting. `MYSQL_MCP_USER` must be a plain name. Before, `CREATE USER` failed in first-start setup and a restart skipped the setup. The health check now asks mysqld for its port (`mysqld --verbose --help`, with a throwaway `--log-bin-index` as the official entrypoint uses), so option files, option groups, `--defaults-file` and `--no-defaults` apply exactly as for the server. It reports not ready while first-start setup runs. - **`scripts/build-docker-local.sh`:** builds 9.7 with GCC 11 (9.7.4 needs 11+), 8.0/8.4 with GCC 10, as `docker-publish.yml` does. - - **`scripts/test-mysql-mcp-image.sh`:** option-file port (`[server]` group), `MYSQL_USER` and `MYSQL_USER_FILE` clash cases, plain-name checks (31 checks). It uses `mktemp` and a portable wait. + - **`scripts/test-mysql-mcp-image.sh`:** option-file port (`[server]` group), `MYSQL_USER` and `MYSQL_USER_FILE` clash cases, plain-name checks, restart of an initialized volume (34 checks). It uses `mktemp` and a portable wait. - **Plugin images pinned to their build release.** The plugin images were built for 8.0.45, 8.4.8 and 9.7.0 but based on the floating `mysql:8.0`/`mysql:8.4`/`mysql:9.7` (now 8.0.46, 8.4.13, 9.7.4), and a library built for one MySQL release can crash another. `docker-publish.yml` now builds them for **8.0.46, 8.4.13 and 9.7.4**, bases each image on `mysql:` and also tags it with the release (`mysql8.0.46`, `mysql8.4.13`, `mysql9.7.4`). The plugin job also runs on a `dry_run` dispatch, building and testing without pushing. `Dockerfile.oraclelinux8`/`9` now require `MYSQL_VERSION`. `scripts/build-docker-local.sh` builds 8.0, 8.4 or 9.7 on the same releases and rejects other versions (it used to map 9.0 to 9.0.0). - **Component images pinned to their build release.** A component works only on the exact MySQL release it was built against: in a dry run, a `component_mcp_guard` built for 8.4.8 crashed `mysql:8.4` (8.4.11). The images were built for 8.4.8 and 9.7.0 but ran on the floating `mysql:8.4`/`mysql:9.7`. `docker-publish.yml` now builds the component images for **8.4.13, 9.7.4 and 26.10.0** (the releases the floating tags now point to) and bases each one on `mysql:`. It also tags each image with its release (`mysql8.4.13-component`, `mysql9.7.4-component`, `mysql26.10.0`), and the artifact smoke test runs on the same release. `Dockerfile.component` now requires `MYSQL_VERSION` (no default). - **Component build scripts reconfigure the server tree after a failed configure.** A `.configure-ok` marker is written only after `cmake` succeeds; before, the scripts trusted `CMakeCache.txt`, which appears before configuration finishes, so a failed configure was skipped on the next run. diff --git a/docker/mysql-mcp/mysql-mcp-entrypoint.sh b/docker/mysql-mcp/mysql-mcp-entrypoint.sh index cdf5052..65358c5 100755 --- a/docker/mysql-mcp/mysql-mcp-entrypoint.sh +++ b/docker/mysql-mcp/mysql-mcp-entrypoint.sh @@ -1,7 +1,9 @@ #!/bin/bash # Entrypoint of the mysql-mcp image: checks the MYSQL_MCP_* settings, then runs the official # entrypoint. The checks run before the data directory is initialized, because a first-start -# script that fails leaves an initialized data directory whose setup a restart skips. +# script that fails leaves an initialized data directory whose setup a restart skips. Once the +# data directory exists the settings are not used again, so they are not checked (a volume +# created by an earlier image keeps starting). set -eo pipefail mcp_error() { @@ -9,28 +11,43 @@ mcp_error() { exit 1 } -if [ -n "${MYSQL_MCP_USER:-}" ]; then - [ -n "${MYSQL_MCP_PASSWORD:-}" ] || mcp_error "MYSQL_MCP_USER is set but MYSQL_MCP_PASSWORD is not" - [ "$MYSQL_MCP_USER" != root ] || mcp_error "MYSQL_MCP_USER cannot be root" - [[ "$MYSQL_MCP_USER" =~ ^[A-Za-z0-9_.-]{1,32}$ ]] || - mcp_error "MYSQL_MCP_USER must be 1-32 characters from A-Z, a-z, 0-9, _ . -" - # The official entrypoint creates MYSQL_USER with ALL on MYSQL_DATABASE; the MCP account - # must be a separate, read-only one. MYSQL_USER may come from MYSQL_USER_FILE (when both - # are set, the official entrypoint refuses them itself). - app_user=${MYSQL_USER:-} - if [ -z "$app_user" ] && [ -n "${MYSQL_USER_FILE:-}" ] && [ -r "$MYSQL_USER_FILE" ]; then - app_user=$(<"$MYSQL_USER_FILE") +check_settings() { + if [ -n "${MYSQL_MCP_USER:-}" ]; then + [ -n "${MYSQL_MCP_PASSWORD:-}" ] || mcp_error "MYSQL_MCP_USER is set but MYSQL_MCP_PASSWORD is not" + [ "$MYSQL_MCP_USER" != root ] || mcp_error "MYSQL_MCP_USER cannot be root" + [[ "$MYSQL_MCP_USER" =~ ^[A-Za-z0-9_.-]{1,32}$ ]] || + mcp_error "MYSQL_MCP_USER must be 1-32 characters from A-Z, a-z, 0-9, _ . -" + # The official entrypoint creates MYSQL_USER with ALL on MYSQL_DATABASE; the MCP account + # must be a separate, read-only one. MYSQL_USER may come from MYSQL_USER_FILE (when both + # are set, the official entrypoint refuses them itself). + local app_user=${MYSQL_USER:-} + if [ -z "$app_user" ] && [ -n "${MYSQL_USER_FILE:-}" ] && [ -r "$MYSQL_USER_FILE" ]; then + app_user=$(<"$MYSQL_USER_FILE") + fi + # The official entrypoint puts MYSQL_USER into SQL unescaped, so a backslash or quote in + # it changes the account name (app\q creates appq), and MySQL trims leading and trailing + # spaces (' app ' is app): such a name cannot be compared. + case "$app_user" in + *\\* | *\'* | *\"*) mcp_error "MYSQL_USER contains a backslash or quote; use a plain name with MYSQL_MCP_USER" ;; + [[:space:]]* | *[[:space:]]) mcp_error "MYSQL_USER starts or ends with whitespace; use a plain name with MYSQL_MCP_USER" ;; + esac + [ "$MYSQL_MCP_USER" != "$app_user" ] || + mcp_error "MYSQL_MCP_USER must differ from MYSQL_USER (that account gets all privileges on MYSQL_DATABASE)" fi - # The official entrypoint puts MYSQL_USER into SQL unescaped, so a backslash or quote in it - # changes the account name (app\q creates appq): such a name cannot be compared. - case "$app_user" in - *\\* | *\'* | *\"*) mcp_error "MYSQL_USER contains a backslash or quote; use a plain name with MYSQL_MCP_USER" ;; + case "${MYSQL_MCP_MAX_EXECUTION_TIME_MS:-}" in + *[!0-9]*) mcp_error "MYSQL_MCP_MAX_EXECUTION_TIME_MS must be a number of milliseconds" ;; esac - [ "$MYSQL_MCP_USER" != "$app_user" ] || - mcp_error "MYSQL_MCP_USER must differ from MYSQL_USER (that account gets all privileges on MYSQL_DATABASE)" +} + +# As the official entrypoint: a command starting with an option runs mysqld. +if [ "${1:0:1}" = '-' ]; then + set -- mysqld "$@" +fi +if [ "$1" = mysqld ]; then + # The data directory as the official entrypoint finds it (mysql_get_config). + datadir=$("$@" --verbose --help --log-bin-index="$(mktemp -u)" 2>/dev/null | + awk '$1 == "datadir" && /^[^ \t]/ { sub(/^[^ \t]+[ \t]+/, ""); print; exit }') + [ -d "$datadir/mysql" ] || check_settings fi -case "${MYSQL_MCP_MAX_EXECUTION_TIME_MS:-}" in -*[!0-9]*) mcp_error "MYSQL_MCP_MAX_EXECUTION_TIME_MS must be a number of milliseconds" ;; -esac exec docker-entrypoint.sh "$@" diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index 2e55e40..03bd6d9 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -35,7 +35,7 @@ GRANT EXECUTE ON mysql_mcp.* TO 'mcp'@'%'; -- if the SQL tool pack is install | `MYSQL_MCP_ALLOWED_SCHEMAS` | Persisted as `mcp.allowed_schemas`. | | `MYSQL_MCP_MAX_EXECUTION_TIME_MS` | Persisted as `mcp.max_execution_time_ms`. | -The other settings keep their defaults (read-only on). The settings are checked before the data directory is initialized, and a container with an invalid combination does not start. For example, `MYSQL_MCP_USER` must differ from `MYSQL_USER` (or the user in `MYSQL_USER_FILE`), because the official entrypoint gives that account all privileges on `MYSQL_DATABASE`. The health check asks mysqld for the port it uses (command line and option files, as mysqld resolves them) and pings it without logging in as root. The official image's own variables (`MYSQL_ROOT_PASSWORD`, `MYSQL_DATABASE`, ...) work as usual. +The other settings keep their defaults (read-only on). The settings are checked before the data directory is initialized, and a container with an invalid combination does not start. Once the data directory exists they are not used or checked again. For example, `MYSQL_MCP_USER` must differ from `MYSQL_USER` (or the user in `MYSQL_USER_FILE`), because the official entrypoint gives that account all privileges on `MYSQL_DATABASE`. The health check asks mysqld for the port it uses (command line and option files, as mysqld resolves them) and pings it without logging in as root. The official image's own variables (`MYSQL_ROOT_PASSWORD`, `MYSQL_DATABASE`, ...) work as usual. ```sh docker run -d -p 3306:3306 -e MYSQL_ROOT_PASSWORD=secret -e MYSQL_DATABASE=shop \ diff --git a/scripts/test-mysql-mcp-image.sh b/scripts/test-mysql-mcp-image.sh index d2b3cdd..ba3bff4 100755 --- a/scripts/test-mysql-mcp-image.sh +++ b/scripts/test-mysql-mcp-image.sh @@ -17,7 +17,7 @@ ok() { PASS=$((PASS + 1)); printf ' PASS %s\n' "$1"; } bad() { FAIL=$((FAIL + 1)); printf ' FAIL %s -- %s\n' "$1" "$2"; } TMPF=$(mktemp) chmod 644 "$TMPF" # copied into containers, where mysqld (user mysql) must read it -cleanup() { docker rm -f -v "$CNAME" "$CNAME_PLAIN" >/dev/null 2>&1; rm -f "$TMPF"; } +cleanup() { docker rm -f -v "$CNAME" "$CNAME_PLAIN" "$CNAME_PLAIN-2" >/dev/null 2>&1; rm -f "$TMPF"; } trap cleanup EXIT expect() { if printf '%s\n' "$2" | grep -Eq -- "$3"; then ok "$1"; else bad "$1" "got: $(printf '%s' "$2" | head -c 300)"; fi @@ -86,8 +86,18 @@ if wait_ready "$CNAME_PLAIN" testpass; then else bad "plain container" "did not start" fi -docker rm -f -v "$CNAME_PLAIN" >/dev/null 2>&1 unset PORT +# The MYSQL_MCP_* checks apply only before initialization: an initialized volume starts with +# settings the checks would refuse (an earlier image accepted more names). +docker stop "$CNAME_PLAIN" >/dev/null +docker run -d --name "$CNAME_PLAIN-2" --volumes-from "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass \ + -e "MYSQL_MCP_USER=mcp account" -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null +if wait_ready "$CNAME_PLAIN-2" testpass; then + ok "initialized volume starts with settings the first-start checks would refuse" +else + bad "initialized volume starts with settings the first-start checks would refuse" "$(docker logs "$CNAME_PLAIN-2" 2>&1 | grep -i error | head -2)" +fi +docker rm -f -v "$CNAME_PLAIN-2" "$CNAME_PLAIN" >/dev/null 2>&1 # MYSQL_MCP_USER = MYSQL_USER (or the user in MYSQL_USER_FILE) is refused before the data # directory is initialized (the official entrypoint would create that account with all @@ -121,6 +131,9 @@ refused "MYSQL_MCP_USER = the user in MYSQL_USER_FILE" docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e MYSQL_DATABASE=app \ -e 'MYSQL_USER=app\q' -e MYSQL_PASSWORD=apppass -e MYSQL_MCP_USER=appq -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null refused "MYSQL_USER with a backslash" "MYSQL_USER contains a backslash or quote" +docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e MYSQL_DATABASE=app \ + -e 'MYSQL_USER= app ' -e MYSQL_PASSWORD=apppass -e MYSQL_MCP_USER=app -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null +refused "MYSQL_USER with surrounding spaces" "MYSQL_USER starts or ends with whitespace" docker run -d --name "$CNAME_PLAIN" -e MYSQL_ROOT_PASSWORD=testpass -e "MYSQL_MCP_USER=mcp'x" -e MYSQL_MCP_PASSWORD=x "$IMAGE" >/dev/null refused "MYSQL_MCP_USER that is not a plain name" "MYSQL_MCP_USER must be 1-32 characters"