fix: post-merge review findings (#249) for the mysql-mcp image and local build

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` (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 <args> --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, 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:<that release>` 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:<that release>`. 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.

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"]

docker/mysql-mcp/mysql-mcp-entrypoint.sh

@@ -0,0 +1,53 @@
+#!/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. 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() {
+	printf '%s [ERROR] [Entrypoint]: mysql-mcp: %s\n' "$(date --rfc-3339=seconds)" "$*" >&2
+	exit 1
+}
+
+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
+	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
+}
+
+# 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
+
+exec docker-entrypoint.sh "$@"

docker/mysql-mcp/mysql-mcp-healthcheck.sh

@@ -1,16 +1,19 @@
-#!/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 was started with (--port=N or --port N on its command line; 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 <args> --verbose --help`), so command line, option groups ([mysqld], [server],
+#   [mysqld-<version>]), --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).
-port=$(tr '\0' '\n' </proc/1/cmdline 2>/dev/null | awk '
-	prev == "--port" || prev == "-P" { p = $0 }
-	/^--port=/ { p = substr($0, 8) }
-	{ prev = $0 }
-	END { print p }')
+mapfile -d '' args </proc/1/cmdline 2>/dev/null || exit 1
+[ "${args[0]##*/}" = mysqld ] || exit 1
+# 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

docs/MCP_COMPONENTS.md

@@ -30,12 +30,12 @@ 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`. |
 
-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. 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 \

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

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" "$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
@@ -68,16 +70,72 @@ 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
+# 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 '[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"
 	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$"
 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
+# 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 [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
+}
+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_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"
+# 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_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"
 
 # 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.