diff options
4 files changed, 147 insertions, 8 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js b/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js index f8b4aa5..ce36a40 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js @@ -88,6 +88,14 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, const displayRoots = serverRoots.map(r => optimistic[r.name] ? { ...r, ...optimistic[r.name] } : r); + // Paths are the operator's own view and do not cross MNP: the roots table + // rides in the index payload, which every member receives, and it tells them + // what exists and whether it is readable — never where on the operator's + // disk it lives. So the column appears when the answer is actually + // available (the loopback API, which is already this machine only) and is + // left out otherwise, rather than printing a row of blanks. + const hasPaths = displayRoots.some((r) => r.path); + // Which door a change goes through. MNP first: it is the only one that // exists for an operator on the web, and it is signed, which the loopback // API is not (it is authorized by being on localhost with the run token). @@ -261,7 +269,7 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, <thead> <tr> <th class="sdt-col-dir">${t('node.directory')}</th> - <th class="sdt-col-path">${t('node.root_path')}</th> + ${hasPaths && html`<th class="sdt-col-path">${t('node.root_path')}</th>`} ${canEdit && html`<th class="sdt-col-toggle">${t('node.root_rw')}</th>`} ${canEdit && !isLocal && html`<th class="sdt-col-toggle">${t('node.removable')}</th>`} <th class="sdt-col-actions"></th> @@ -283,10 +291,8 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, ${!isLocal && r.available === false && !r.ejected && html` <span class="node-root-badge node-root-badge-warn">${t('node.unavailable')}</span>`} </td> - ${/* Two roots can never share a name, so the name is the identity — - but it is the *basename*, and two libraries under different - parents look identical without this. */''} - <td class="sdt-col-path" title=${r.path || ''}>${r.path || ''}</td> + ${hasPaths && html` + <td class="sdt-col-path" title=${r.path || ''}>${r.path || ''}</td>`} ${canEdit && html` <td class="sdt-col-toggle"> <${ToggleSwitch} checked=${!!r.writable} disabled=${busy || !!r.ejected} 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)}") |