diff options
Diffstat (limited to 'packages/meshbay-node')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/ops.py | 6 | ||||
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/roots.py | 18 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_root_paths_are_operator_only.py | 115 |
3 files changed, 136 insertions, 3 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/ops.py b/packages/meshbay-node/src/meshbay_node/ops.py index 3dfdc23..4c759c8 100644 --- a/packages/meshbay-node/src/meshbay_node/ops.py +++ b/packages/meshbay-node/src/meshbay_node/ops.py @@ -362,7 +362,11 @@ async def list_groups(state: dict) -> dict: "has_gek": bool(ctx.get("gek")), "file_count": idx.count if idx else 0, "index_version": idx.version if idx else 0, - "roots": roots.describe() if roots else [], + # With paths: this answers the loopback API, which is the + # operator's own channel. `meshbay-node root list` printed "?" for + # every directory without it — it was reading a field the member + # form of this deliberately omits. + "roots": roots.describe(with_paths=True) if roots else [], "peers": sum(1 for p in peers.values() if p.get("group_id") == gid), }) roster = state.get("roster") diff --git a/packages/meshbay-node/src/meshbay_node/roots.py b/packages/meshbay-node/src/meshbay_node/roots.py index ffe801c..d288231 100644 --- a/packages/meshbay-node/src/meshbay_node/roots.py +++ b/packages/meshbay-node/src/meshbay_node/roots.py @@ -357,8 +357,14 @@ class RootSet: "available" if live else "UNAVAILABLE", root.path) return changed - def describe(self) -> list[dict]: - """Per-root state for the index payload and the admin UI.""" + def describe(self, *, with_paths: bool = False) -> list[dict]: + """ + Per-root state for the index payload and the admin UI. + + Deliberately no paths by default: this is what every member receives. + `with_paths=True` is the operator's own view, over a channel that is + already theirs alone (loopback + run token). + """ out = [] for r in self.roots: d: dict = {"name": r.name, "kind": r.kind, @@ -368,6 +374,14 @@ class RootSet: "ejected": r.ejected, # Backward compat for MNP 1.0 clients "upload": r.writable} + # `with_paths` is for the operator's *own* channels only — the + # loopback API and the CLI reading it, both of which already + # require being on this machine with the run token. A member is + # told what exists and whether it is readable, never where on the + # operator's disk it lives, and the index payload every member + # receives must keep calling this without the flag. + if with_paths: + d["path"] = str(r.path) out.append(d) return out diff --git a/packages/meshbay-node/tests/test_root_paths_are_operator_only.py b/packages/meshbay-node/tests/test_root_paths_are_operator_only.py new file mode 100644 index 0000000..6ba2e08 --- /dev/null +++ b/packages/meshbay-node/tests/test_root_paths_are_operator_only.py @@ -0,0 +1,115 @@ +""" +Where a directory lives on the operator's disk is theirs, not the group's. + +`RootSet.describe()` feeds two very different audiences. The index payload goes +to every member, and has always deliberately carried no paths — a member is +told what exists and whether it is readable, not that the library sits in +`/media/<the operator's name>/BACKUP2`. The loopback API answers the operator +themselves, over a channel that already requires being on their machine with +the run token, where the path is exactly what they are asking for. + +`meshbay-node root list` printed `?` for every directory because it read a +field the member form omits. Nothing caught it: the CLI reads a dict, the +payload is a dict, and neither end says what keys it owes the other. + +Both halves matter and they pull opposite ways, so both are asserted here — a +test that only checked the operator gets paths would be satisfied by putting +them in the member payload too. +""" + +import inspect +import re +from pathlib import Path + +import pytest + +from meshbay_node import daemon as daemon_mod +from meshbay_node import ops +from meshbay_node.roots import RootSet + + +def _roots(tmp_path: Path) -> RootSet: + for name in ("Films", "Albums"): + (tmp_path / name).mkdir() + return RootSet.build([ + {"path": str(tmp_path / "Films"), "writable": True}, + {"path": str(tmp_path / "Albums"), "removable": True}, + ]) + + +# ── The member's half ──────────────────────────────────────────────────────── + +def test_the_default_form_carries_no_path(tmp_path): + described = _roots(tmp_path).describe() + assert described, "no roots described" + assert not any("path" in d for d in described), ( + "the index payload every member receives would carry the operator's " + "filesystem layout") + + +def test_the_default_form_still_says_what_a_member_needs(tmp_path): + """The counter-property: dropping the path must not drop the rest.""" + described = _roots(tmp_path).describe() + for d in described: + assert set(d) >= {"name", "kind", "available", "writable", + "removable", "ejected"} + + +def test_the_index_payload_is_built_without_paths(): + """ + Read from the source, because the alternative is asserting it about a + payload built by a test rather than by the node. + """ + from meshbay_node.indexer import indexer as indexer_mod + source = inspect.getsource(indexer_mod) + for call in re.findall(r"roots\.describe\([^)]*\)", source): + assert "with_paths" not in call, ( + f"the indexer builds the member-facing roots table as {call} — " + f"that payload goes to everyone in the group") + + +# ── The operator's half ────────────────────────────────────────────────────── + +def test_the_operator_form_carries_the_path(tmp_path): + described = _roots(tmp_path).describe(with_paths=True) + assert all(d.get("path") for d in described) + assert described[0]["path"] == str(tmp_path / "Films") + + +def test_the_loopback_api_asks_for_paths(): + """ + `list_groups` answers the operator's own channel, and the CLI's `root list` + prints what it returns. Asking for the member form there is what printed a + column of question marks. + """ + source = inspect.getsource(ops.list_groups) + assert "describe(with_paths=True)" in source, ( + "list_groups uses the member form, so every path it reports is missing") + + +def test_the_cli_only_reads_fields_the_payload_carries(): + """ + The gap this whole file exists for. The CLI reads a dict and the API + returns a dict; nothing between them says which keys are owed, so a name + that is simply absent prints as a placeholder and looks like a node + problem. + """ + source = inspect.getsource(daemon_mod.main) + start = source.index('if args.command == "root":') + block = source[start:source.index('if args.command == "operator":', start)] + + read = set(re.findall(r"r\.get\(['\"](\w+)['\"]", block)) + read |= set(re.findall(r"r\[['\"](\w+)['\"]\]", block)) + assert read, "the root CLI no longer reads the payload this way" + + class _Any: + path = Path("/tmp/x") + name = "x" + kind = "generic" + writable = removable = ejected = False + available = True + + offered = set(RootSet(roots=[_Any()]).describe(with_paths=True)[0]) + assert read <= offered, ( + f"the `root` CLI reads keys the loopback payload does not carry: " + f"{sorted(read - offered)}") |