From c5fff4ce8366b08669c0c8b6d30b99b94b9fefca Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 12 Sep 2026 16:15:16 +0200 Subject: fix(packaging): the hub unit can start, and nothing carries the migration path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ExecStartPre` ran `alembic -c /opt/meshbay-hub/migrations/alembic.ini upgrade head`. The build does stage that file, so the path existed and the contents were wrong: `alembic.ini` resolves `script_location` with `%(here)s`, so the copy pointed at `/opt/meshbay-hub/migrations/src/meshbay_hub/db/migrations` — which nothing installs, because the migrations ship inside `meshbay_hub`, in the shared venv. `ExecStartPre` failing stops the unit. A hub installed from the RPM or the DEB could not start at all, and nothing noticed because the one live deployment was assembled by hand — the same shape as the node unit that carried `User=` into the user unit directory. The same `%(here)s` trap was already found once on the server, where a stray `alembic.ini` resolved to a month-old snapshot of the tree. Twice is a trap rather than an accident, so the fix is that the path is no longer written down anywhere: `meshbay-hub migrate` asks the installed package where its own migrations are, which is correct for the RPM, the DEB, a venv and a checkout. The build stages no `alembic.ini`; the repo keeps its own for `alembic revision` and for deploy scripts that already work. `env.py` now prefers a URL the caller resolved over re-reading the environment itself, so `migrate --config` connects with exactly the string the server will — one resolution, not two that agree until they do not. Six tests, three of which fail against the unit as it was. They read the directives rather than the file, because searching the whole thing finds the comment explaining a directive and calls that the directive. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01T4YmK41VsEURWFdop4EEeT --- CLAUDE.md | 16 +++- docs/MESHBAY_DESIGN.md | 1 + packages/meshbay-hub/src/meshbay_hub/daemon.py | 48 ++++++++++ .../src/meshbay_hub/db/migrations/env.py | 24 ++++- .../meshbay-hub/tests/test_packaging_hub_unit.py | 101 +++++++++++++++++++++ packaging/build/build-hub.sh | 14 ++- packaging/deb/meshbay-hub/DEBIAN/control | 5 +- packaging/systemd/meshbay-hub.service | 9 +- 8 files changed, 204 insertions(+), 14 deletions(-) create mode 100644 packages/meshbay-hub/tests/test_packaging_hub_unit.py diff --git a/CLAUDE.md b/CLAUDE.md index 3027a21..10792aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,7 +122,7 @@ that produced it. | Looking for | Read | |---|---| | What a label means (`C1`, `H3`, `NS6`, `T3`, `C5b`, `W2`, `E9`, `F1`, `AV4`, …) | `docs/MESHBAY_DESIGN.md` §13 | -| What one member can cost the others (`AV1`–`AV18`) | §13.5b — the newest category, and the one the first three reviews had no question for | +| What one member can cost the others (`AV1`–`AV19`) | §13.5b — the newest category, and the one the first three reviews had no question for | | Trust model, and what the project may and may not claim | §2 | | Identity, devices, admission, recovery, the keypair bundle | §3 | | Cryptography, key hierarchy, the group and chat envelopes | §4 | @@ -148,6 +148,20 @@ These are about working on the tree rather than about the design: interface, for the web and the app alike; `packages/meshbay-client/scripts/ sync-ui.js` copies it (`npm run sync-ui`) and CI fails if the copy drifts — **never edit `packages/meshbay-client/ui/` by hand** +- **`%(here)s` in `alembic.ini` makes a copy of it correct only where it was + copied from.** The packaged unit ran `alembic -c + /opt/meshbay-hub/migrations/alembic.ini`, a file the build did stage — so the + path existed and the *contents* were wrong: `script_location` resolved to + `…/migrations/src/meshbay_hub/db/migrations`, which nothing installs, because + the migrations ship inside `meshbay_hub` in the shared venv. `ExecStartPre` + failing stops the unit, so **a hub installed from the RPM or the DEB could + not start at all**, and nothing noticed because the one live deployment was + assembled by hand. The same trap had already been found on the server, where + a stray `alembic.ini` pointed at a month-old snapshot. Twice is a trap: the + path is not written down anywhere now — `meshbay-hub migrate` asks the + installed package where its own migrations are, which is right for a + package, a venv and a checkout alike + - **`node --check` reports success on a module-syntax error, and a green one cost a full suite run.** An unclosed `.map(` inside a tagged template came back clean four times in a row. `test_spa_syntax.py` says this in its own diff --git a/docs/MESHBAY_DESIGN.md b/docs/MESHBAY_DESIGN.md index 577612d..14ec5d3 100644 --- a/docs/MESHBAY_DESIGN.md +++ b/docs/MESHBAY_DESIGN.md @@ -2532,6 +2532,7 @@ had already been asked. | **AV12** | **Every list has an upper bound on `limit` and a floor under `offset`.** Including the ones that take no authentication at all — the public group directory and the content blocklist | | **AV16** | **A bound the operator can see and change, and that a restart does not forget.** The mail allowance lives in `mail_quota`, not in a module dict — a deploy used to hand out a fresh budget, and the hub is deployed often. The values are settings with defaults in `hub.toml` and a block in the admin panel, because the hour a budget runs out is not when anyone wants to edit a file and restart; `/v1/admin/mail` says how much of the hour is left, which was previously visible only as an absence of mail | | **AV17** | **A global ceiling being reached is an event, not an absence.** When the hourly budget runs out the administrators are notified — once per hour, because a flood is what spends it and one alert per refusal buries the message under its own cause — and the panel says which of the two ceilings fell: newcomers turned away, or somebody locked out of their account unable to get back in | +| **AV19** | **Nothing carries the path to the migrations.** `meshbay-hub migrate` derives it from the installed package, so the RPM, the DEB, a venv and a checkout all agree. A unit naming `alembic.ini` names a file whose `%(here)s` stops being true the moment packaging moves it | | **AV18** | **The hub runs on exactly one worker, and says so at startup.** `_connected_nodes`, `_node_groups`, `_webrtc_answers` and the relay registry are per-process: a second worker makes a node intermittently unreachable for half its members, which is a symptom that describes something else entirely | | **AV14** | **MHP binds its audience, and the hub reads its own identity at call time.** A token is minted for one peer and accepted by that peer only. `federation.py` bound `_hub_id` and `_hub_sk_pem` at import, which is before `load_hub_keypair` runs, so it signed with `None` and called itself `meshbay.org` whatever the instance was named — and the verifier named no audience for the `aud` the issuer sets, which PyJWT refuses outright. MHP could not complete one authenticated request between two hubs | | **AV15** | **A hash is checked for shape before it is a key lookup**, on the unauthenticated blocklist endpoints a node consults | diff --git a/packages/meshbay-hub/src/meshbay_hub/daemon.py b/packages/meshbay-hub/src/meshbay_hub/daemon.py index 024ca78..9609de6 100644 --- a/packages/meshbay-hub/src/meshbay_hub/daemon.py +++ b/packages/meshbay-hub/src/meshbay_hub/daemon.py @@ -35,6 +35,12 @@ def main() -> None: prune.add_argument("--log-level", default=argparse.SUPPRESS, choices=["DEBUG", "INFO", "WARNING", "ERROR"]) + migrate = sub.add_parser( + "migrate", help="bring the database schema up to date") + migrate.add_argument("--config", type=Path, default=argparse.SUPPRESS) + migrate.add_argument("--log-level", default=argparse.SUPPRESS, + choices=["DEBUG", "INFO", "WARNING", "ERROR"]) + args = parser.parse_args() logging.basicConfig( @@ -47,6 +53,9 @@ def main() -> None: if args.command == "prune-groups": sys.exit(asyncio.run(_prune_groups(cfg, args.days, args.dry_run))) + if args.command == "migrate": + sys.exit(_migrate(cfg)) + single_worker_or_exit(cfg.server.workers) uvicorn.run( @@ -59,6 +68,45 @@ def main() -> None: ) +def migrations_dir() -> Path: + """Where this installation's migrations actually are. + + Derived from the package rather than written down, because the one place a + path like this can be correct is next to the code it describes. The RPM + installs `meshbay_hub` into a shared venv and copies `alembic.ini` to + `/opt/meshbay-hub/migrations/`, where `%(here)s/src/meshbay_hub/db/…` + resolves to a directory that does not exist — so the packaged unit's + `ExecStartPre` could never have succeeded, and a hub installed from the + package would not start at all. The same `%(here)s` trap had already been + found once on the server, with a copy of alembic.ini pointing at a + month-old snapshot of the tree. + """ + return Path(__file__).resolve().parent / "db" / "migrations" + + +def _migrate(cfg) -> int: + """`alembic upgrade head`, with the paths resolved from the installation. + + A command rather than a path in a unit file: it is correct for the RPM, + the DEB, a venv, and a checkout, and there is nothing to keep in step. + """ + from alembic import command + from alembic.config import Config + + scripts = migrations_dir() + if not (scripts / "versions").is_dir(): + print(f"meshbay-hub: no migrations at {scripts}", file=sys.stderr) + return 1 + + alembic_cfg = Config() + alembic_cfg.set_main_option("script_location", str(scripts)) + # `env.py` reads the URL from the environment the same way the server does, + # so the two cannot drift; this is set for the case where it does not. + alembic_cfg.set_main_option("sqlalchemy.url", cfg.db.url) + command.upgrade(alembic_cfg, "head") + return 0 + + def single_worker_or_exit(workers: int) -> None: """Refuse to start with more than one worker. diff --git a/packages/meshbay-hub/src/meshbay_hub/db/migrations/env.py b/packages/meshbay-hub/src/meshbay_hub/db/migrations/env.py index f572716..8908deb 100644 --- a/packages/meshbay-hub/src/meshbay_hub/db/migrations/env.py +++ b/packages/meshbay-hub/src/meshbay_hub/db/migrations/env.py @@ -19,11 +19,25 @@ if config.config_file_name is not None: target_metadata = Base.metadata -# Allow override via env var (production uses asyncpg, tests may use aiosqlite) -db_url = os.environ.get( - "MESHBAY_DATABASE_URL", - "postgresql+asyncpg://meshbay:meshbay@localhost/meshbay_hub", -) +# Three sources, in the order that keeps them from disagreeing. +# +# A caller that already resolved the URL wins: `meshbay-hub migrate` passes +# `cfg.db.url`, which is `load_config`'s answer — the very string the server +# will connect with. Reading the environment again here instead would be a +# second resolution of the same question, and the two would drift the day +# anything but the environment decides it. +# +# Then the environment, for `alembic` run by hand or from a deploy script, +# where `alembic.ini` carries only its placeholder. Then a local default, so +# a developer's checkout needs no setup. +_PLACEHOLDER = "driver://user:pass@localhost/dbname" + +db_url = config.get_main_option("sqlalchemy.url", "") +if not db_url or db_url == _PLACEHOLDER: + db_url = os.environ.get( + "MESHBAY_DATABASE_URL", + "postgresql+asyncpg://meshbay:meshbay@localhost/meshbay_hub", + ) config.set_main_option("sqlalchemy.url", db_url) diff --git a/packages/meshbay-hub/tests/test_packaging_hub_unit.py b/packages/meshbay-hub/tests/test_packaging_hub_unit.py new file mode 100644 index 0000000..7d8f5d2 --- /dev/null +++ b/packages/meshbay-hub/tests/test_packaging_hub_unit.py @@ -0,0 +1,101 @@ +""" +The hub's systemd unit, and the migration step it runs before starting. + +`ExecStartPre` was `alembic -c /opt/meshbay-hub/migrations/alembic.ini upgrade +head`. The file was staged there, so the path existed — but `alembic.ini` +resolves `script_location` with `%(here)s`, so a copy of it is only correct +where it was copied *from*. Staged into `/opt/meshbay-hub/migrations/` it +pointed at `/opt/meshbay-hub/migrations/src/meshbay_hub/db/migrations`, which +nothing installs: the migrations ship inside `meshbay_hub`, in the shared venv. + +`ExecStartPre` failing stops the unit, so **a hub installed from the RPM or the +DEB could not start at all**, and nothing noticed because the reference +deployment was assembled by hand. Exactly the shape of the node unit defect +recorded in `test_packaging_units.py`: a packaged file nobody had installed. + +The same `%(here)s` trap had already been found once on the server, where a +stray `alembic.ini` resolved to a month-old snapshot of the tree +(`QE/server-state/meshbay.org.md`). Twice is a trap, not an accident: the fix +is that nothing carries the path any more. `meshbay-hub migrate` asks the +installed package where its own migrations are. +""" + +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[3] +UNIT = ROOT / "packaging" / "systemd" / "meshbay-hub.service" +BUILD = ROOT / "packaging" / "build" / "build-hub.sh" + +pytestmark = pytest.mark.skipif( + not UNIT.exists(), reason="packaging not present") + + +def _directives() -> list[str]: + """The lines systemd acts on — comments dropped. + + Searching the whole file finds the comment explaining why a directive is + absent and calls that the directive; `test_packaging_units.py` records + that mistake being made. + """ + return [line.strip() for line in UNIT.read_text(encoding="utf-8").splitlines() + if line.strip() and not line.strip().startswith("#")] + + +def _exec_start_pre() -> list[str]: + return [d for d in _directives() if d.startswith("ExecStartPre=")] + + +def test_the_migration_step_runs_the_hubs_own_command(): + pre = _exec_start_pre() + assert len(pre) == 1, f"expected one ExecStartPre, got {pre}" + assert pre[0].endswith("meshbay-hub migrate --config /etc/meshbay/hub.toml"), pre[0] + + +def test_no_directive_names_an_alembic_config_by_path(): + """A path to `alembic.ini` in a unit is the defect itself: the file is only + correct where it was written, and a package moves it.""" + offenders = [d for d in _directives() if "alembic" in d] + assert not offenders, ( + "a unit that names alembic.ini carries a path that the packaging " + f"relocates: {offenders}") + + +def test_the_build_stages_no_alembic_config(): + """Staging a copy is what made the path exist and the contents wrong.""" + staged = [line.strip() for line in BUILD.read_text(encoding="utf-8").splitlines() + if "alembic.ini" in line and not line.strip().startswith("#")] + assert not staged, f"build-hub.sh still stages alembic.ini: {staged}" + + +def test_the_migrations_are_where_the_command_looks_for_them(): + """The other half. The unit is right only if the package carries them — + they are `.py` files inside `meshbay_hub`, so a wheel does, but nothing + said so and nothing would notice if that changed.""" + from meshbay_hub.daemon import migrations_dir + + scripts = migrations_dir() + assert (scripts / "env.py").is_file(), scripts + revisions = list((scripts / "versions").glob("*.py")) + assert revisions, f"no revisions under {scripts}" + + +def test_the_command_resolves_from_the_installed_package_not_the_checkout(): + """Derived from `meshbay_hub.__file__`, so it is correct in a venv, an RPM + and a checkout alike — which is the whole point of not writing it down.""" + import meshbay_hub + + from meshbay_hub.daemon import migrations_dir + + assert migrations_dir() == ( + Path(meshbay_hub.__file__).resolve().parent / "db" / "migrations") + + +def test_the_hub_runs_as_the_service_account_under_the_hardening(): + """Read once while here, because an ExecStartPre that cannot run is not the + only way a unit fails to start.""" + directives = _directives() + for required in ("User=meshbay", "Group=meshbay", + "NoNewPrivileges=true", "ProtectSystem=strict"): + assert required in directives, f"{required} is not in the unit" diff --git a/packaging/build/build-hub.sh b/packaging/build/build-hub.sh index be2d363..e32ba55 100755 --- a/packaging/build/build-hub.sh +++ b/packaging/build/build-hub.sh @@ -47,11 +47,15 @@ ln -sf /opt/meshbay-common/venv/bin/meshbay-hub "$ROOT/usr/bin/meshbay-hub" # --- Hub-specific assets -------------------------------------------------- mkdir -p "$ROOT/opt/meshbay-hub" -# Alembic config (migrations are inside the installed package at meshbay_hub/db/migrations/) -if [ -f "$REPO/packages/meshbay-hub/alembic.ini" ]; then - mkdir -p "$ROOT/opt/meshbay-hub/migrations" - cp "$REPO/packages/meshbay-hub/alembic.ini" "$ROOT/opt/meshbay-hub/migrations/" -fi +# No alembic.ini is staged. It resolves `script_location` with `%(here)s`, so a +# copy of it is only correct where it was copied from: staged into +# /opt/meshbay-hub/migrations/ it pointed at +# /opt/meshbay-hub/migrations/src/meshbay_hub/db/migrations, which nothing +# installs — the migrations live inside the package, in the shared venv. The +# unit runs `meshbay-hub migrate` instead, which asks the package. +# +# The same `%(here)s` trap had already been found once on the server, where a +# stray alembic.ini resolved to a month-old snapshot of the tree. # Config example. # diff --git a/packaging/deb/meshbay-hub/DEBIAN/control b/packaging/deb/meshbay-hub/DEBIAN/control index 07799a1..c5f60fa 100644 --- a/packaging/deb/meshbay-hub/DEBIAN/control +++ b/packaging/deb/meshbay-hub/DEBIAN/control @@ -11,6 +11,7 @@ Description: MeshBay Hub — identity authority and group registry server MeshBay Hub provides user registration, JWT issuance, group management, WebRTC signaling, notifications, and moderation for MeshBay networks. . - Installs hub code into the shared venv at /opt/meshbay-common/venv/ and - Alembic migrations at /opt/meshbay-hub/migrations/. + Installs hub code into the shared venv at /opt/meshbay-common/venv/. The + database migrations travel inside the package; `meshbay-hub migrate` finds + them there, and the systemd unit runs it before the server starts. Runs as a systemd service behind Caddy for HTTPS. diff --git a/packaging/systemd/meshbay-hub.service b/packaging/systemd/meshbay-hub.service index 40cdb98..1a3ca44 100644 --- a/packaging/systemd/meshbay-hub.service +++ b/packaging/systemd/meshbay-hub.service @@ -16,7 +16,14 @@ EnvironmentFile=-/etc/meshbay/hub.env Environment=MESHBAY_DATABASE_URL=postgresql+asyncpg://meshbay:CHANGEME@localhost/meshbay_hub Environment=MESHBAY_HUB_KEY=/etc/meshbay/hub_private.pem -ExecStartPre=/opt/meshbay-common/venv/bin/alembic -c /opt/meshbay-hub/migrations/alembic.ini upgrade head +# The hub's own command, not `alembic -c `. That path was +# /opt/meshbay-hub/migrations/alembic.ini, where `%(here)s/src/meshbay_hub/db/…` +# resolves to a directory the package does not install — the migrations ship +# inside meshbay_hub itself, in the shared venv. So this line could never +# succeed, and a hub installed from the RPM or the DEB would not start at all. +# `migrate` asks the installed package where its own migrations are, which is +# correct for the package, a venv, and a checkout alike. +ExecStartPre=/opt/meshbay-common/venv/bin/meshbay-hub migrate --config /etc/meshbay/hub.toml ExecStart=/opt/meshbay-common/venv/bin/meshbay-hub --config /etc/meshbay/hub.toml Restart=always RestartSec=5 -- cgit v1.2.3