diff options
Diffstat (limited to 'packages/meshbay-hub/tests/test_upload_controls_hidden.py')
| -rw-r--r-- | packages/meshbay-hub/tests/test_upload_controls_hidden.py | 222 |
1 files changed, 162 insertions, 60 deletions
diff --git a/packages/meshbay-hub/tests/test_upload_controls_hidden.py b/packages/meshbay-hub/tests/test_upload_controls_hidden.py index 94917d2..ae1f444 100644 --- a/packages/meshbay-hub/tests/test_upload_controls_hidden.py +++ b/packages/meshbay-hub/tests/test_upload_controls_hidden.py @@ -1,14 +1,22 @@ """ -When the operator closes uploading, the controls go — both of them. +When a directory is read-only, the controls that write to it go — both of them. -There are two ways to put a file into a group and they are in different +There are two ways to put a file into a group and they live in different components: the Upload button in the Files toolbar, and the paperclip in the chat composer. Hiding one and forgetting the other is the obvious mistake, and -the second one is the easier to forget because it does not look like an upload. +the paperclip is the easier to forget because it does not look like an upload. Nothing here is a security property. **The node refuses the upload** — that is -`test_member_upload_policy.py` in the node package. This is about not offering -somebody a button whose only outcome is an error message. +`test_root_writable_policy.py` and `test_security_regressions.py` in the node +package. This is about not offering somebody a button whose only outcome is an +error message. + +What the RO/RW refactor changed: there is no group-wide answer any more. Files +uploads into *the root being browsed*, so its button follows that root's +`writable`. Chat has no folder on screen, so the shell picks one for it. The +two therefore read different things on purpose, and the tests below pin that +each reads the right one — a stronger claim than the old "both read one +boolean", which is why that assertion is gone rather than adapted. """ import re @@ -17,10 +25,6 @@ from pathlib import Path import pytest STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" -# The group-page refactor split what used to be one app.js into one file per -# "application" plus the group shell. mayUpload itself is still derived once, -# in the shell (group-page.js) — Files and Chat each moved to their own file -# and receive it as a prop, the same shape ChatPanel already took. APP = STATIC / "app.js" GROUP_PAGE = STATIC / "group-page.js" FILES_APP = STATIC / "files-app.js" @@ -36,45 +40,93 @@ def app() -> str: return GROUP_PAGE.read_text(encoding="utf-8") -def _component(app: str, name: str) -> str: - start = app.index(f"\nfunction {name}(") - end = app.find("\nfunction ", start + 1) - return app[start:end if end != -1 else len(app)] +def _component(source: str, name: str) -> str: + start = source.index(f"\nfunction {name}(") + end = source.find("\nfunction ", start + 1) + return source[start:end if end != -1 else len(source)] # ── Both controls ─────────────────────────────────────────────────────────── def test_the_files_toolbar_hides_its_upload_button(): + """ + Gated on the root being browsed, not on a group-wide answer: with one + writable root and one read-only one, a single boolean would offer the + button in both and produce a refusal in one of them. + """ + page = _component(FILES_APP.read_text(encoding="utf-8"), "FilesPanel") + toolbar = page[page.index("file-toolbar"):] + toolbar = toolbar[:toolbar.index("breadcrumbs")] + assert "currentRootWritable" in toolbar, ( + "the Upload button is offered regardless of the directory's own flag") + + +def test_the_files_upload_button_is_not_offered_at_the_top_of_a_group(): + """ + The top level is the set of roots, which is the operator's configuration + and not a directory on anyone's disk. There is nothing to upload *into* + there, and no root name to give the node. + """ page = _component(FILES_APP.read_text(encoding="utf-8"), "FilesPanel") toolbar = page[page.index("file-toolbar"):] toolbar = toolbar[:toolbar.index("breadcrumbs")] - assert "mayUpload &&" in toolbar, "the Upload button is offered regardless" + assert "currentPath &&" in toolbar def test_the_chat_composer_hides_its_paperclip(): chat = _component(CHAT_APP.read_text(encoding="utf-8"), "ChatPanel") composer = chat[chat.index("chat-input-row"):] - assert "mayUpload &&" in composer, ( + assert "attachRoot ?" in composer, ( "the chat attachment is the second way in and is still offered") -def test_both_read_the_same_answer(app): - """Two derivations would eventually disagree, and the disagreement would - be one of them offering an upload the node refuses.""" - assert re.search(r"const mayUpload = memberUpload \|\| isNodeAdmin;", app), ( - "mayUpload is no longer derived in one place") - # Files and Chat both receive it from the same `commonProps` object the - # shell spreads into whichever app tab is active — one derivation feeding - # one object, rather than two hand-written prop attributes that could - # drift apart. +def test_the_paperclip_says_why_rather_than_vanishing(): + """ + A control that disappears leaves the reader no way to find out what would + bring it back. A group with no writable directory is a state an operator + can fix, so it is worth naming. + """ + chat = _component(CHAT_APP.read_text(encoding="utf-8"), "ChatPanel") + composer = chat[chat.index("chat-input-row"):] + assert "chat.attach_read_only" in composer + + +# ── One derivation, in the shell ──────────────────────────────────────────── + +def test_the_attachment_directory_is_decided_once(app): + """ + Two derivations would eventually disagree, and the disagreement would be + one of them offering an upload the node refuses. + """ + assert re.search(r"const attachRoot = ", app), ( + "attachRoot is no longer derived in one place") props = app[app.index("const commonProps = {"):app.index("return html`")] - assert "mayUpload," in props or "mayUpload:" in props, ( - "mayUpload is not in the shared props object every app receives") + assert "attachRoot," in props or "attachRoot:" in props, ( + "attachRoot is not in the shared props object every app receives") + +def test_an_unavailable_root_is_not_offered_as_a_destination(app): + """ + `writable` is configuration and stays true while a drive is unplugged or + ejected. Offering it anyway produces a refusal from the node with no + explanation on screen. + """ + block = app[app.index("const writableRoots"):] + block = block[:block.index("const attachRoot")] + assert "available" in block -def test_the_operator_keeps_their_own_controls(app): - assert "memberUpload || isNodeAdmin" in app, ( - "turning uploads off would hide the operator's own upload button") + +def test_files_uploads_into_the_root_it_is_showing(): + """ + The client has to name the destination now, because the node cannot choose + between several writable roots without guessing — and a guess here means a + file landing in a directory nobody was looking at. + """ + page = _component(FILES_APP.read_text(encoding="utf-8"), "FilesPanel") + upload = page[page.index("const uploadFile"):] + upload = upload[:upload.index("const makeDirectory")] + assert "root: uploadRoot" in upload, "the node is left to choose" + assert "currentPath.split('/')[0]" in upload # ── Learning the answer ───────────────────────────────────────────────────── @@ -82,54 +134,104 @@ def test_the_operator_keeps_their_own_controls(app): def test_the_answer_comes_from_the_node(app): """Not from the hub, which has no say in what may be written to someone else's disk, and no way to be believed about it.""" - assert "ack.member_upload !== false" in app, ( - "the handshake ack is what carries this") - assert "hubFetch" not in app[app.index("ack.member_upload") - 400: - app.index("ack.member_upload")] + assert "if (indexMsg.roots) setNodeRoots(indexMsg.roots)" in app, ( + "the roots table in the index payload is what carries this") + idx = app.index("setNodeRoots(indexMsg.roots)") + assert "hubFetch" not in app[idx - 400:idx] def test_an_older_node_is_treated_as_permissive(app): - """A node that predates the setting sends no such field. Reading a missing - field as "off" would close every group on the older half of the network.""" + """ + A node speaking MNP 1.0 sends roots with no `writable` at all, plus the old + group-wide flag. Reading a missing field as "read-only" would close every + group on the older half of the network. + """ + assert "ack.member_upload !== false" in app assert "!== false" in app[app.index("ack.member_upload"): app.index("ack.member_upload") + 60] + block = app[app.index("const legacyNode"):] + block = block[:block.index("const commonProps")] + assert "writable === undefined" in block, ( + "nothing distinguishes a 1.0 node from one with no writable roots") def test_a_change_reaches_people_already_connected(app): - """The operator may be someone else entirely, changing it while you have - the group open. A button that survives until the next reconnection is a - button somebody presses.""" - assert "transport.onUploadPolicy" in app + """ + The operator may be someone else entirely, ejecting a drive while you have + the group open. A file list that survives until the next reconnection is a + list somebody clicks. + """ + assert "transport.onRootsChanged" in app transport = TRANSPORT.read_text(encoding="utf-8") - assert "member_upload_ack" in transport, "nothing routes the node's notice" + assert "root_eject_ack" in transport, "nothing routes the node's notice" + +def test_the_notice_also_answers_the_operators_own_request(): + """ + The same message is both a broadcast and the reply to the request that + caused it. -def test_the_notice_still_answers_the_operators_own_request(app): - """The same message is both a broadcast and the reply to the request that - caused it — returning early on it would leave that request hanging until it - timed out.""" + Every other admin ack can be resolved and dropped, because its caller + already knows what it asked for and updates local state from that. The root + acks carry a whole table only the node can compute — availability, the name + it settled on, the eject a failed plug left in place — so resolving one + without handing it on left the operator who clicked Eject as the only + client that never saw it happen. + """ transport = TRANSPORT.read_text(encoding="utf-8") - # Scoped to member_upload_ack's own handler, not everything up to the next - # occurrence of "index_sync" — other handlers with their own, legitimate - # early `return` (index_progress, set_scan_settings_ack: neither is ever a - # reply anyone awaits) now sit between the two in the file. - block = transport[transport.index("member_upload_ack"):] - block = block[:block.index("apps_enabled_ack")] - assert "return" not in block + block = transport[transport.index("msg.type.endsWith('_ack')"):] + block = block[:block.index("_uploaders")] + assert "ROOT_ACK_TYPES" in block and "_onRootsChanged" in block, ( + "the initiating client resolves the ack and learns nothing from it") # ── Changing it ───────────────────────────────────────────────────────────── -def test_changing_it_is_signed(app): +def test_changing_a_root_is_signed(): transport = TRANSPORT.read_text(encoding="utf-8") - method = transport[transport.index("async setMemberUpload("):] - method = method[:method.index("\n async ", 1)] - assert "admin_challenge" in method and "_authorizeAdminOp" in method, ( - "an unsigned instruction would let any member turn uploads back on") + for method in ("updateRoot", "ejectRoot", "plugRoot"): + body = transport[transport.index(f"async {method}("):] + body = body[:body.index("\n async ", 1)] + assert "admin_challenge" in body and "_authorizeAdminOp" in body, ( + f"{method} is unsigned — any member could use it") def test_only_the_operator_is_offered_the_setting(): - panel = _component(GROUP_SETTINGS.read_text(encoding="utf-8"), "GroupSettingsPanel") - section = panel[panel.index("members.uploads_title") - 400: - panel.index("members.uploads_title")] - assert "isNodeAdmin && connected" in section + panel = _component(GROUP_SETTINGS.read_text(encoding="utf-8"), + "GroupSettingsPanel") + section = panel[panel.index("settings_node.shared_directories_title") - 600: + panel.index("settings_node.shared_directories_title")] + assert "isNodeAdmin &&" in section + + +def test_the_operator_is_offered_it_on_the_web_too(): + """ + An operator is not necessarily sitting at their node. The first version of + this section required the loopback API, which resolves to "not available" + in a browser — so it rendered for nobody on the web, while the upload + controls it replaced had worked there. + """ + source = GROUP_SETTINGS.read_text(encoding="utf-8") + panel = _component(source, "GroupSettingsPanel") + section = panel[panel.index("settings_node.shared_directories_title") - 600: + panel.index("settings_node.shared_directories_title")] + assert "connected ||" in section, ( + "the shared directories section still requires a local node") + + table = _component(source, "SharedDirectoriesTable") + for call in ("transport.updateRoot", "transport.ejectRoot", + "transport.plugRoot", "transport.removeRoot", + "transport.addRoot"): + assert call in table, f"{call} has no MNP route from the table" + + +def test_the_roots_shown_come_from_the_live_connection_when_there_is_one(): + """ + The loopback list is a second source, and the two drift: it is read once on + mount and after a change, while the MNP one is pushed. Preferring MNP also + keeps this table on the same data Files reads, so an eject shows in both at + the same instant. + """ + panel = _component(GROUP_SETTINGS.read_text(encoding="utf-8"), + "GroupSettingsPanel") + assert "const effectiveRoots = (connected && mnpRoots" in panel |