From ad5d8fcda389d11373edbe17c1ef21c52f345ebd Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 26 May 2026 00:46:47 +0200 Subject: [PATCH] fix(backup): force TCP for mariadb-dump to match ''@'%' grant Co-Authored-By: Claude Opus 4.7 (1M context) --- src/baudolo/backup/db.py | 2 + .../test_e2e_mariadb_anonymous_preemption.py | 143 ++++++++++++++++++ tests/unit/backup/test_db_mariadb_dump.py | 68 +++++++++ 3 files changed, 213 insertions(+) create mode 100644 tests/e2e/test_e2e_mariadb_anonymous_preemption.py create mode 100644 tests/unit/backup/test_db_mariadb_dump.py diff --git a/src/baudolo/backup/db.py b/src/baudolo/backup/db.py index 427c5ab..3b732d2 100644 --- a/src/baudolo/backup/db.py +++ b/src/baudolo/backup/db.py @@ -115,8 +115,10 @@ def backup_database( dump_file = os.path.join(out_dir, f"{db_name}.backup.sql") if db_type == "mariadb": + # Force TCP so auth matches ''@'%' instead of socket -> 'localhost'. cmd = ( f"docker exec {container} /usr/bin/mariadb-dump " + f"-h 127.0.0.1 --protocol=tcp " f"-u {user} -p{password} {db_name}" ) _atomic_write_cmd(cmd, dump_file) diff --git a/tests/e2e/test_e2e_mariadb_anonymous_preemption.py b/tests/e2e/test_e2e_mariadb_anonymous_preemption.py new file mode 100644 index 0000000..d22451c --- /dev/null +++ b/tests/e2e/test_e2e_mariadb_anonymous_preemption.py @@ -0,0 +1,143 @@ +""" +Bug-repro for: mariadb-dump fails with `ERROR 1045 Access denied for user +''@'localhost' (using password: YES)` when only ''@'%' is granted and a +preempting ''@'localhost' user is present. + +The fix forces TCP loopback in baudolo.backup.db so the dump matches the +''@'%' grant instead of the socket->localhost auth row. + +This file: +- builds the exact preconditions that triggered the production failure, +- as a NEGATIVE control, runs a socket-based mariadb-dump (== the old code path) + and asserts that it fails with the literal 1045 / @'localhost' error, +- as a POSITIVE proof, calls backup_database() (where the fix lives) against + the same DB container and asserts the dump file is produced and contains the + seed data. + +Note: the volume-rsync stage of baudolo is intentionally NOT exercised here. +That stage needs root on /var/lib/docker/volumes, which is provided by the +DinD wrapper in `make test-e2e` but not by an on-host invocation. The bug we +are verifying is in the DB-dump stage, so testing backup_database() directly +keeps the assertion focused and the test runnable both on-host and in DinD. +""" + +import os +import tempfile +import unittest + +import pandas + +from baudolo.backup import db as db_mod + +from .helpers import ( + cleanup_docker, + require_docker, + run, + unique, + wait_for_mariadb, + wait_for_mariadb_sql, +) + + +class TestE2EMariaDBAnonymousPreemption(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + require_docker() + cls.prefix = unique("baudolo-e2e-mariadb-anon") + cls.db_container = f"{cls.prefix}-mariadb" + cls.db_volume = f"{cls.prefix}-mariadb-vol" + cls.containers = [cls.db_container] + cls.volumes = [cls.db_volume] + + cls.db_name = "appdb" + cls.db_user = "tcponly" + cls.db_password = "tcponlypw" + cls.root_password = "rootpw" + + run(["docker", "volume", "create", cls.db_volume]) + + # Boot WITHOUT MARIADB_USER/MARIADB_PASSWORD/MARIADB_DATABASE so the + # entrypoint does not auto-create ''@'%'. We provision the user + # explicitly below to mirror the SQL path used by svc-db-mariadb. + run([ + "docker", "run", "-d", + "--name", cls.db_container, + "-e", f"MARIADB_ROOT_PASSWORD={cls.root_password}", + "-v", f"{cls.db_volume}:/var/lib/mysql", + "mariadb:12.2", + ]) + + wait_for_mariadb(cls.db_container, root_password=cls.root_password, timeout_s=120) + + # Provision: ''@'%' (the app/backup grant) + anonymous ''@'localhost' + # (the preemption trigger). Mirrors the production state that produced + # `ERROR 1045 ... ''@'localhost' (using password: YES)`. + bootstrap_sql = ( + f"CREATE DATABASE {cls.db_name};" + f"CREATE USER '{cls.db_user}'@'%' IDENTIFIED BY '{cls.db_password}';" + f"GRANT ALL PRIVILEGES ON {cls.db_name}.* TO '{cls.db_user}'@'%';" + f"CREATE USER ''@'localhost' IDENTIFIED BY 'anonpw-not-{cls.db_password}';" + "FLUSH PRIVILEGES;" + f"CREATE TABLE {cls.db_name}.t (id INT PRIMARY KEY, v VARCHAR(50));" + f"INSERT INTO {cls.db_name}.t VALUES (1,'ok');" + ) + run([ + "docker", "exec", cls.db_container, "sh", "-lc", + f'mariadb -uroot --protocol=socket -e "{bootstrap_sql}"', + ]) + + # Sanity: '' can log in over TCP (matches '%'). If THIS fails, + # the precondition for the fix to even apply is broken. + wait_for_mariadb_sql( + cls.db_container, user=cls.db_user, password=cls.db_password, timeout_s=60 + ) + + @classmethod + def tearDownClass(cls) -> None: + cleanup_docker(containers=cls.containers, volumes=cls.volumes) + + def test_negative_control_socket_dump_fails_with_1045(self) -> None: + # Reproduces the OLD code path (no -h/--protocol). MUST fail with 1045 + # under the configured preemption. If this ever starts passing, either + # the MariaDB auth semantics changed or the anonymous-user setup did + # not take effect — in both cases the positive test below loses its + # ability to discriminate "fix works" vs "bug never reproduced". + p = run( + [ + "docker", "exec", self.db_container, "sh", "-lc", + f"mariadb-dump -u{self.db_user} -p{self.db_password} {self.db_name}", + ], + capture=True, + check=False, + ) + self.assertNotEqual(p.returncode, 0, "socket-based dump unexpectedly succeeded") + self.assertIn("1045", (p.stderr or "") + (p.stdout or "")) + self.assertIn("@'localhost'", (p.stderr or "") + (p.stdout or "")) + + def test_backup_database_succeeds_with_tcp_fix(self) -> None: + # Drives the function where the fix lives. No rsync, no privileged + # paths — just the dump that the negative-control proved is failing + # under the same preemption setup. + with tempfile.TemporaryDirectory() as volume_dir: + df = pandas.DataFrame( + [(self.db_container, self.db_name, self.db_user, self.db_password)], + columns=["instance", "database", "username", "password"], + ) + produced = db_mod.backup_database( + container=self.db_container, + volume_dir=volume_dir, + db_type="mariadb", + databases_df=df, + database_containers=[self.db_container], + ) + self.assertTrue(produced, "backup_database did not produce a dump") + dump_path = os.path.join(volume_dir, "sql", f"{self.db_name}.backup.sql") + self.assertTrue(os.path.isfile(dump_path), f"expected dump at {dump_path}") + with open(dump_path, "r", encoding="utf-8", errors="replace") as f: + content = f.read() + self.assertIn("INSERT INTO", content) + self.assertIn("'ok'", content) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/tests/unit/backup/test_db_mariadb_dump.py b/tests/unit/backup/test_db_mariadb_dump.py new file mode 100644 index 0000000..2a9667b --- /dev/null +++ b/tests/unit/backup/test_db_mariadb_dump.py @@ -0,0 +1,68 @@ +import tempfile +import unittest +from unittest.mock import patch + +import pandas + +from baudolo.backup import db as db_mod + + +def _df(rows): + return pandas.DataFrame( + rows, columns=["instance", "database", "username", "password"] + ) + + +def _capture_commands(*, db_type, rows, container): + captured = [] + + def _capture(cmd): + captured.append(cmd) + return [] + + with tempfile.TemporaryDirectory() as td: + with patch.object(db_mod, "execute_shell_command", side_effect=_capture): + db_mod.backup_database( + container=container, + volume_dir=td, + db_type=db_type, + databases_df=_df(rows), + database_containers=[container], + ) + return captured + + +class TestMariaDBDumpUsesTCP(unittest.TestCase): + # Regression guard for 'Access denied for user @localhost' when only + # ''@'%' is granted: the in-container mariadb-dump MUST force TCP so + # the connection is auth-matched against '%' instead of socket->localhost. + + def test_mariadb_dump_forces_tcp_loopback(self): + captured = _capture_commands( + db_type="mariadb", + rows=[("mariadb", "appdb", "appuser", "s3cret")], + container="mariadb", + ) + dump_cmds = [c for c in captured if "mariadb-dump" in c] + self.assertEqual(len(dump_cmds), 1, f"expected one dump command, got: {captured}") + + cmd = dump_cmds[0] + self.assertIn("-h 127.0.0.1", cmd) + self.assertIn("--protocol=tcp", cmd) + self.assertIn("-u appuser", cmd) + self.assertIn("-ps3cret", cmd) + self.assertIn(" appdb", cmd) + + def test_postgres_dump_unaffected(self): + captured = _capture_commands( + db_type="postgres", + rows=[("pg", "appdb", "appuser", "s3cret")], + container="pg", + ) + dump_cmds = [c for c in captured if "pg_dump" in c and "pg_dumpall" not in c] + self.assertEqual(len(dump_cmds), 1) + self.assertNotIn("--protocol=tcp", dump_cmds[0]) + + +if __name__ == "__main__": + unittest.main(verbosity=2)