From 19d2bee179751c91bf0da472b5a824a480fa6213 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Thu, 28 May 2026 00:29:54 +0530 Subject: [PATCH 1/2] fix: create DB user with correct host scope for TCP connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frappe's new-site always creates the site DB user as @'localhost'. In MySQL's privilege tables 'localhost' matches only unix socket connections — TCP connections from 127.0.0.1 are rejected even on the same host, breaking bench when MariaDB is running in Docker or over any remote TCP connection. Fix: - Add MariaDBManager._grant_host(): returns '%' when no unix socket is detected (TCP mode), 'localhost' when a socket is present. - Add MariaDBManager.create_user(): uses the mysql/mariadb CLI (zero new dependencies) to run CREATE USER IF NOT EXISTS and GRANT with the correct host scope. - Wire create_user() into Site.create(): after frappe new-site completes, read the generated site_config.json for db_name and db_password, then call create_user() to ensure the user exists with @'%' scope for TCP connections. Fixes #4 --- bench_cli/core/site.py | 14 +++ bench_cli/managers/mariadb_manager.py | 37 ++++++ tests/test_mariadb_manager.py | 169 ++++++++++++++++++++++++++ 3 files changed, 220 insertions(+) create mode 100644 tests/test_mariadb_manager.py diff --git a/bench_cli/core/site.py b/bench_cli/core/site.py index 6a8167c..4ce29fe 100644 --- a/bench_cli/core/site.py +++ b/bench_cli/core/site.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json from pathlib import Path from typing import TYPE_CHECKING @@ -51,6 +52,19 @@ def create(self) -> None: run_command(cmd, cwd=self.bench.sites_path, stream_output=True) + # Frappe's new-site creates the DB user as @'localhost', which breaks + # TCP connections (127.0.0.1 != localhost in MySQL privilege tables). + # Re-create the user with the correct host scope when no unix socket + # is in use. Credentials are read from the generated site_config.json. + if not socket_path: + site_cfg_path = self.path / "site_config.json" + if site_cfg_path.exists(): + site_cfg = json.loads(site_cfg_path.read_text()) + db_name = site_cfg.get("db_name", "") + db_password = site_cfg.get("db_password", "") + if db_name and db_password: + MariaDBManager(mariadb).create_user(db_name, db_password, db_name) + def restore(self, db_file: str, public_files: str | None = None, private_files: str | None = None) -> None: cmd = self._frappe_call("frappe", "--site", self.config.name, "restore", db_file) if public_files: diff --git a/bench_cli/managers/mariadb_manager.py b/bench_cli/managers/mariadb_manager.py index 8fe1e02..262e176 100644 --- a/bench_cli/managers/mariadb_manager.py +++ b/bench_cli/managers/mariadb_manager.py @@ -39,6 +39,43 @@ def _apt_package(self) -> str: return f"mariadb-server-{self.config.version}" return "mariadb-server" + def _grant_host(self) -> str: + """Return the MySQL grant host for CREATE USER / GRANT statements. + + Use '%' when connecting over TCP so that the site DB user can reach + MariaDB from 127.0.0.1 (TCP loopback). Fall back to 'localhost' + only when a unix socket is detected — in MySQL's privilege tables + 'localhost' matches socket connections exclusively. + """ + return "localhost" if self._detect_socket() else "%" + + def create_user(self, username: str, password: str, db_name: str) -> None: + """Create the DB user and grant all privileges with the correct host scope. + + Frappe's new-site always creates the user as @'localhost', which + breaks TCP connections (127.0.0.1 != localhost in privilege tables). + This method uses the grant host derived from _grant_host() so TCP + connections work when no unix socket is available. + """ + grant_host = self._grant_host() + sql = ( + f"CREATE USER IF NOT EXISTS '{username}'@'{grant_host}'" + f" IDENTIFIED BY '{password}';" + f"GRANT ALL PRIVILEGES ON `{db_name}`.* TO '{username}'@'{grant_host}';" + "FLUSH PRIVILEGES;" + ) + mysql_bin = shutil.which("mariadb") or shutil.which("mysql") or "mysql" + cmd = [mysql_bin, f"-u{self.config.admin_user}"] + if self.config.root_password: + cmd.append(f"-p{self.config.root_password}") + socket_path = self._detect_socket() + if socket_path: + cmd.append(f"--socket={socket_path}") + else: + cmd += [f"-h{self.config.host}", f"-P{self.config.port}"] + cmd += ["-e", sql] + run_command(cmd) + def _detect_socket(self) -> str: if self.config.socket_path: return self.config.socket_path diff --git a/tests/test_mariadb_manager.py b/tests/test_mariadb_manager.py new file mode 100644 index 0000000..487626b --- /dev/null +++ b/tests/test_mariadb_manager.py @@ -0,0 +1,169 @@ +"""Tests for MariaDBManager.create_user() and _grant_host().""" +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import MagicMock, call, patch + +import pytest + +from bench_cli.config.bench_config import BenchConfig +from bench_cli.config.mariadb_config import MariaDBConfig +from bench_cli.config.site_config import SiteConfig +from bench_cli.core.bench import Bench +from bench_cli.core.site import Site +from bench_cli.managers.mariadb_manager import MariaDBManager + + +def make_manager(host: str = "localhost", port: int = 3306, root_password: str = "secret") -> MariaDBManager: + return MariaDBManager(MariaDBConfig(host=host, port=port, root_password=root_password)) + + +# ── _grant_host() ───────────────────────────────────────────────────────────── + + +def test_grant_host_returns_percent_when_no_socket() -> None: + manager = make_manager() + with patch.object(manager, "_detect_socket", return_value=""): + assert manager._grant_host() == "%" + + +def test_grant_host_returns_localhost_when_socket_detected() -> None: + manager = make_manager() + with patch.object(manager, "_detect_socket", return_value="/tmp/mysql.sock"): + assert manager._grant_host() == "localhost" + + +# ── create_user() ───────────────────────────────────────────────────────────── + + +def test_create_user_tcp_uses_percent_host() -> None: + manager = make_manager(host="127.0.0.1", root_password="root") + with patch.object(manager, "_detect_socket", return_value=""): + with patch("shutil.which", side_effect=lambda b: "/usr/bin/mysql" if b == "mysql" else None): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert any("'mydb'@'%'" in part for part in called_cmd) + + +def test_create_user_socket_uses_localhost_host() -> None: + manager = make_manager(root_password="root") + with patch.object(manager, "_detect_socket", return_value="/tmp/mysql.sock"): + with patch("shutil.which", side_effect=lambda b: "/usr/bin/mysql" if b == "mysql" else None): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert any("'mydb'@'localhost'" in part for part in called_cmd) + + +def test_create_user_prefers_mariadb_binary() -> None: + manager = make_manager(root_password="root") + with patch.object(manager, "_detect_socket", return_value=""): + with patch("shutil.which", side_effect=lambda b: "/usr/bin/mariadb" if b == "mariadb" else None): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert called_cmd[0] == "/usr/bin/mariadb" + + +def test_create_user_falls_back_to_mysql_binary() -> None: + manager = make_manager(root_password="root") + with patch.object(manager, "_detect_socket", return_value=""): + with patch("shutil.which", side_effect=lambda b: "/usr/bin/mysql" if b == "mysql" else None): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert called_cmd[0] == "/usr/bin/mysql" + + +def test_create_user_tcp_passes_host_and_port() -> None: + manager = make_manager(host="127.0.0.1", port=3307, root_password="root") + with patch.object(manager, "_detect_socket", return_value=""): + with patch("shutil.which", return_value="/usr/bin/mysql"): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert "-h127.0.0.1" in called_cmd + assert "-P3307" in called_cmd + + +def test_create_user_socket_passes_socket_path() -> None: + manager = make_manager(root_password="root") + with patch.object(manager, "_detect_socket", return_value="/tmp/mysql.sock"): + with patch("shutil.which", return_value="/usr/bin/mysql"): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert "--socket=/tmp/mysql.sock" in called_cmd + + +def test_create_user_includes_root_password() -> None: + manager = make_manager(root_password="s3cr3t") + with patch.object(manager, "_detect_socket", return_value=""): + with patch("shutil.which", return_value="/usr/bin/mysql"): + with patch("bench_cli.managers.mariadb_manager.run_command") as mock_run: + manager.create_user("mydb", "mypass", "mydb") + called_cmd = mock_run.call_args[0][0] + assert "-ps3cr3t" in called_cmd + + +# ── site.create() post-processing ──────────────────────────────────────────── + + +def make_bench(tmp_path: Path) -> Bench: + from bench_cli.config.app_config import AppConfig + from bench_cli.config.redis_config import RedisConfig + from bench_cli.config.worker_config import WorkerConfig + config = BenchConfig( + name="test-bench", + python_version="3.14", + apps=[AppConfig(name="frappe", repo="https://github.com/frappe/frappe", branch="version-16")], + mariadb=MariaDBConfig(host="127.0.0.1", port=3306, root_password="root"), + redis=RedisConfig(cache_port=13000, queue_port=11000, socketio_port=12000), + workers=__import__("bench_cli.config.worker_config", fromlist=["WorkerConfig"]).WorkerConfig(default_count=1, short_count=1, long_count=1), + ) + return Bench(config, tmp_path) + + +def test_site_create_calls_create_user_after_frappe_new_site(tmp_path: Path) -> None: + bench = make_bench(tmp_path) + bench.create_directories() + site = Site(SiteConfig(name="site1.localhost", apps=["frappe"]), bench) + + site_cfg_data = {"db_name": "_abc123", "db_password": "dbpass"} + site.path.mkdir(parents=True, exist_ok=True) + (site.path / "site_config.json").write_text(json.dumps(site_cfg_data)) + + with patch("bench_cli.core.site.run_command"): + with patch("bench_cli.managers.mariadb_manager.MariaDBManager._detect_socket", return_value=""): + with patch("bench_cli.managers.mariadb_manager.MariaDBManager.create_user") as mock_create: + site.create() + mock_create.assert_called_once_with("_abc123", "dbpass", "_abc123") + + +def test_site_create_skips_create_user_when_socket_used(tmp_path: Path) -> None: + bench = make_bench(tmp_path) + bench.create_directories() + site = Site(SiteConfig(name="site1.localhost", apps=["frappe"]), bench) + + site.path.mkdir(parents=True, exist_ok=True) + (site.path / "site_config.json").write_text(json.dumps({"db_name": "_abc123", "db_password": "dbpass"})) + + with patch("bench_cli.core.site.run_command"): + with patch("bench_cli.managers.mariadb_manager.MariaDBManager._detect_socket", return_value="/tmp/mysql.sock"): + with patch("bench_cli.managers.mariadb_manager.MariaDBManager.create_user") as mock_create: + site.create() + mock_create.assert_not_called() + + +def test_site_create_skips_create_user_when_site_config_missing(tmp_path: Path) -> None: + bench = make_bench(tmp_path) + bench.create_directories() + site = Site(SiteConfig(name="site1.localhost", apps=["frappe"]), bench) + + with patch("bench_cli.core.site.run_command"): + with patch("bench_cli.managers.mariadb_manager.MariaDBManager._detect_socket", return_value=""): + with patch("bench_cli.managers.mariadb_manager.MariaDBManager.create_user") as mock_create: + site.create() + mock_create.assert_not_called() From 21ba004c0334cdef8fed876d0740b2dc8f285f65 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Mon, 1 Jun 2026 01:14:51 +0530 Subject: [PATCH 2/2] fix: use --mariadb-user-host-login-scope for TCP connections Use Frappe's built-in --mariadb-user-host-login-scope option instead of post-processing with create_user(). This is cleaner and more explicit. Changes: - Pass --mariadb-user-host-login-scope='%' when no unix socket detected - Remove implicit create_user() post-processing from site.create() - Update tests to verify the flag is passed correctly - Add research-folder/ to .gitignore Addresses review feedback from @rmehta on PR #13 --- .gitignore | 3 +++ bench_cli/core/site.py | 17 +++---------- tests/test_mariadb_manager.py | 47 +++++++++++------------------------ 3 files changed, 20 insertions(+), 47 deletions(-) diff --git a/.gitignore b/.gitignore index 73996cd..424c48b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,9 @@ # Benches (created by `bench new`) benches/ +# Research +research-folder/ + # Python __pycache__/ *.py[cod] diff --git a/bench_cli/core/site.py b/bench_cli/core/site.py index 4ce29fe..6ffead3 100644 --- a/bench_cli/core/site.py +++ b/bench_cli/core/site.py @@ -1,6 +1,5 @@ from __future__ import annotations -import json from pathlib import Path from typing import TYPE_CHECKING @@ -49,22 +48,12 @@ def create(self) -> None: cmd += ["--db-host", mariadb.host, "--db-port", str(mariadb.port)] if mariadb.root_password: cmd += ["--db-root-password", mariadb.root_password] + # Use '%' host scope for TCP connections to allow connections from any host. + # MySQL treats 'localhost' (unix socket) and '127.0.0.1' (TCP) as different hosts. + cmd += ["--mariadb-user-host-login-scope", "%"] run_command(cmd, cwd=self.bench.sites_path, stream_output=True) - # Frappe's new-site creates the DB user as @'localhost', which breaks - # TCP connections (127.0.0.1 != localhost in MySQL privilege tables). - # Re-create the user with the correct host scope when no unix socket - # is in use. Credentials are read from the generated site_config.json. - if not socket_path: - site_cfg_path = self.path / "site_config.json" - if site_cfg_path.exists(): - site_cfg = json.loads(site_cfg_path.read_text()) - db_name = site_cfg.get("db_name", "") - db_password = site_cfg.get("db_password", "") - if db_name and db_password: - MariaDBManager(mariadb).create_user(db_name, db_password, db_name) - def restore(self, db_file: str, public_files: str | None = None, private_files: str | None = None) -> None: cmd = self._frappe_call("frappe", "--site", self.config.name, "restore", db_file) if public_files: diff --git a/tests/test_mariadb_manager.py b/tests/test_mariadb_manager.py index 487626b..00b962f 100644 --- a/tests/test_mariadb_manager.py +++ b/tests/test_mariadb_manager.py @@ -1,7 +1,6 @@ """Tests for MariaDBManager.create_user() and _grant_host().""" from __future__ import annotations -import json from pathlib import Path from unittest.mock import MagicMock, call, patch @@ -108,7 +107,7 @@ def test_create_user_includes_root_password() -> None: assert "-ps3cr3t" in called_cmd -# ── site.create() post-processing ──────────────────────────────────────────── +# ── site.create() with --mariadb-user-host-login-scope ────────────────────── def make_bench(tmp_path: Path) -> Bench: @@ -126,44 +125,26 @@ def make_bench(tmp_path: Path) -> Bench: return Bench(config, tmp_path) -def test_site_create_calls_create_user_after_frappe_new_site(tmp_path: Path) -> None: +def test_site_create_passes_host_scope_for_tcp(tmp_path: Path) -> None: bench = make_bench(tmp_path) bench.create_directories() - site = Site(SiteConfig(name="site1.localhost", apps=["frappe"]), bench) + site = Site(SiteConfig(name="site1.localhost", apps=["frappe"], admin_password="admin"), bench) - site_cfg_data = {"db_name": "_abc123", "db_password": "dbpass"} - site.path.mkdir(parents=True, exist_ok=True) - (site.path / "site_config.json").write_text(json.dumps(site_cfg_data)) - - with patch("bench_cli.core.site.run_command"): + with patch("bench_cli.core.site.run_command") as mock_run: with patch("bench_cli.managers.mariadb_manager.MariaDBManager._detect_socket", return_value=""): - with patch("bench_cli.managers.mariadb_manager.MariaDBManager.create_user") as mock_create: - site.create() - mock_create.assert_called_once_with("_abc123", "dbpass", "_abc123") + site.create() + called_cmd = mock_run.call_args[0][0] + assert "--mariadb-user-host-login-scope" in called_cmd + assert "%" in called_cmd -def test_site_create_skips_create_user_when_socket_used(tmp_path: Path) -> None: +def test_site_create_skips_host_scope_for_socket(tmp_path: Path) -> None: bench = make_bench(tmp_path) bench.create_directories() - site = Site(SiteConfig(name="site1.localhost", apps=["frappe"]), bench) - - site.path.mkdir(parents=True, exist_ok=True) - (site.path / "site_config.json").write_text(json.dumps({"db_name": "_abc123", "db_password": "dbpass"})) + site = Site(SiteConfig(name="site1.localhost", apps=["frappe"], admin_password="admin"), bench) - with patch("bench_cli.core.site.run_command"): + with patch("bench_cli.core.site.run_command") as mock_run: with patch("bench_cli.managers.mariadb_manager.MariaDBManager._detect_socket", return_value="/tmp/mysql.sock"): - with patch("bench_cli.managers.mariadb_manager.MariaDBManager.create_user") as mock_create: - site.create() - mock_create.assert_not_called() - - -def test_site_create_skips_create_user_when_site_config_missing(tmp_path: Path) -> None: - bench = make_bench(tmp_path) - bench.create_directories() - site = Site(SiteConfig(name="site1.localhost", apps=["frappe"]), bench) - - with patch("bench_cli.core.site.run_command"): - with patch("bench_cli.managers.mariadb_manager.MariaDBManager._detect_socket", return_value=""): - with patch("bench_cli.managers.mariadb_manager.MariaDBManager.create_user") as mock_create: - site.create() - mock_create.assert_not_called() + site.create() + called_cmd = mock_run.call_args[0][0] + assert "--mariadb-user-host-login-scope" not in called_cmd