diff options
Diffstat (limited to 'packages/meshbay-node')
6 files changed, 102 insertions, 63 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/config.py b/packages/meshbay-node/src/meshbay_node/config.py index 4712312..7673a51 100644 --- a/packages/meshbay-node/src/meshbay_node/config.py +++ b/packages/meshbay-node/src/meshbay_node/config.py @@ -184,7 +184,6 @@ class RootSpec: kind: str = "generic" # generic|video|audio|photo — a view hint, unused for now writable: bool = False # RW roots accept uploads from group members removable: bool = False # operator can eject this root before unplugging the device - direct: bool = False # uploads land at root path, not in a subdirectory @dataclass @@ -225,7 +224,7 @@ class GroupConfig: for r in self.roots: r.writable = False self.roots.append(RootSpec( - path=self.upload_dir.strip(), writable=True, direct=True)) + path=self.upload_dir.strip(), writable=True)) @dataclass diff --git a/packages/meshbay-node/src/meshbay_node/ops.py b/packages/meshbay-node/src/meshbay_node/ops.py index 5b10452..3dfdc23 100644 --- a/packages/meshbay-node/src/meshbay_node/ops.py +++ b/packages/meshbay-node/src/meshbay_node/ops.py @@ -679,8 +679,7 @@ async def add_root(state: dict, group_id: str, path: str, *, from meshbay_node.config import RootSpec cfg.roots.append(RootSpec( path=str(added.path), name=added.name, kind=added.kind, - writable=added.writable, removable=added.removable, - direct=added.direct)) + writable=added.writable, removable=added.removable)) # Deliberately *not* mutating the live RootSet in place. # diff --git a/packages/meshbay-node/src/meshbay_node/roots.py b/packages/meshbay-node/src/meshbay_node/roots.py index 9d3f7cb..ffe801c 100644 --- a/packages/meshbay-node/src/meshbay_node/roots.py +++ b/packages/meshbay-node/src/meshbay_node/roots.py @@ -122,7 +122,6 @@ class Root: kind: str = "generic" writable: bool = False removable: bool = False - direct: bool = False ejected: bool = False available: bool = True @@ -226,8 +225,7 @@ class RootSet: writable=writable, removable=bool(spec.get("removable", False)), ejected=bool(spec.get("ejected", False)), - available=not bool(spec.get("ejected", False)), - direct=bool(spec.get("direct", False))) + available=not bool(spec.get("ejected", False))) _refuse_nesting(root, roots) roots.append(root) by_folded[root.folded] = root @@ -370,8 +368,6 @@ class RootSet: "ejected": r.ejected, # Backward compat for MNP 1.0 clients "upload": r.writable} - if r.direct: - d["direct"] = True out.append(d) return out diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index d341d8c..61458f2 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -214,7 +214,6 @@ JOIN_FAILURE_WINDOW = 600 # seconds # attachments from the chat alike. One visible directory the operator can look # into, back up or empty — rather than a hidden tree of per-user uuids that # nobody could read, or files scattered wherever someone happened to be looking. -UPLOAD_DIR_NAME = "uploads" def _extract_dtls_fingerprint(sdp: str) -> bytes: @@ -4016,7 +4015,12 @@ class WebRTCPeerSession: # later — the same reason the old single upload root was never guessed. # A client that names nothing is an MNP 1.0 one, and there was exactly # one destination in its world: the first writable root. - target_root_name = str(msg.get("root") or "").strip() + # `dir` is the folder being browsed, as a virtual path + # (`Media/Films/1999`); `root` is the older, coarser form and is what + # its first segment means on its own. + target_rel = str(msg.get("dir") or "").strip().strip("/") + target_root_name = (target_rel.split("/")[0] if target_rel + else str(msg.get("root") or "").strip()) upload_root = None if target_root_name: upload_root = roots.by_name(target_root_name) @@ -4052,18 +4056,43 @@ class WebRTCPeerSession: "filename": filename}) return - if upload_root.direct: - rel_dir = upload_root.name - target_dir = upload_root.path + # The folder the sender is looking at, and no subdirectory of the node's + # invention. + # + # Uploads used to be confined to `<root>/uploads/`, created on demand. + # That was the last of v5's quarantine (the per-user layer went on + # 2026-08-14, for the same reason): a shared directory nobody can + # organise is not a shared directory, and a folder appearing beside the + # operator's library because somebody sent a file is the node deciding + # how their disk is arranged. + # + # What made the quarantine worth having is not the subdirectory — it is + # the filename allowlist, the size cap, the chunk ordering, and the + # no-overwrite rule below. All four are unchanged. + # + # `resolve()` and not a join: it refuses `..`, absolute segments and + # anything whose resolved form escapes its root, symlinks included. The + # client names *where among the group's own folders*, never a path on + # the operator's filesystem. + if target_rel: + target_dir = roots.resolve(target_rel) + if target_dir is None or not target_dir.is_dir(): + self._send({"type": "error", + "detail": "Not a directory in this group", + "code": "no_such_directory", + "filename": filename}) + return + rel_dir = target_rel else: - rel_dir = f"{upload_root.name}/{UPLOAD_DIR_NAME}" - target_dir = upload_root.path / UPLOAD_DIR_NAME - try: - target_dir.mkdir(parents=True, exist_ok=True) - except OSError as e: - log.warning("Cannot create upload folder in root %r: %s", - upload_root.name, e) - self._send({"type": "error", "detail": "Upload folder unavailable", + # An MNP 1.0 client names nothing; the root itself is where its one + # destination now is. + target_dir = upload_root.path + rel_dir = upload_root.name + if not target_dir.is_dir(): + self._send({"type": "error", + "detail": f"Directory '{upload_root.name}' is " + f"currently unavailable", + "code": "root_unavailable", "filename": filename}) return diff --git a/packages/meshbay-node/tests/test_root_writable_policy.py b/packages/meshbay-node/tests/test_root_writable_policy.py index da95032..d7f2666 100644 --- a/packages/meshbay-node/tests/test_root_writable_policy.py +++ b/packages/meshbay-node/tests/test_root_writable_policy.py @@ -62,14 +62,16 @@ def _session(tmp_path: Path, user_id: str, *, def _upload(session, filename="clip.mp4", body=b"bytes"): session._do_file_upload({ - "filename": filename, "root": "shared", + "filename": filename, "dir": "shared", "chunk_index": 0, "total_chunks": 1, "data": base64.b64encode(body).decode(), }) def _uploads_dir(session) -> Path: - return session._ctx["roots"].roots[0].path / "uploads" + # The root itself: the `uploads/` subdirectory the node used to create is + # gone (see test_security_regressions._uploads_dir for why). + return session._ctx["roots"].roots[0].path # ── The door, not the button ───────────────────────────────────────────────── @@ -80,7 +82,7 @@ async def test_a_member_cannot_upload_to_a_read_only_root(tmp_path): refusal = [m for m in session.sent if m.get("type") == "error"] assert refusal and refusal[0].get("code") == "root_read_only" - assert not _uploads_dir(session).exists() + assert not (_uploads_dir(session) / "clip.mp4").exists() async def test_members_upload_normally_to_a_writable_root(tmp_path): diff --git a/packages/meshbay-node/tests/test_security_regressions.py b/packages/meshbay-node/tests/test_security_regressions.py index 9db8ac1..1a318f7 100644 --- a/packages/meshbay-node/tests/test_security_regressions.py +++ b/packages/meshbay-node/tests/test_security_regressions.py @@ -133,14 +133,22 @@ def test_the_node_never_generates_a_name_it_would_refuse(tmp_path): def _uploads_dir(session) -> Path: """ - Where this session's uploads land: uploads/ inside its first writable root. + Where an unaddressed upload lands: the first writable root itself. + + There is no `uploads/` subdirectory any more. It was the last of v5's + quarantine — the per-user layer went on 2026-08-14 — and it went for the + same reason: a folder appearing beside the operator's library because + somebody sent a file is the node deciding how their disk is arranged. The + protections that made the quarantine worth having are the allowlist, the + size cap, the chunk ordering and the no-overwrite rule, and every one of + them is asserted below, unchanged. Asked of the root set rather than assembled by hand, so a test cannot pass while agreeing with a wrong answer the code also produced. """ writable = session._ctx["roots"].writable_roots assert writable, "the fixture must give the group a writable root" - return writable[0].path / "uploads" + return writable[0].path def _session(tmp_path: Path, user_id: str) -> WebRTCPeerSession: @@ -176,7 +184,6 @@ def test_upload_cannot_overwrite_another_members_file(tmp_path): """ victim = _session(tmp_path, "victim-user") uploads = _uploads_dir(victim) - uploads.mkdir() original = uploads / "important.mp4" original.write_bytes(b"operator's original content") @@ -226,50 +233,57 @@ def test_dir_create_cannot_escape_the_shared_root(tmp_path, bad): assert set(tmp_path.rglob("*")) == before, f"created something via {bad!r}" -def test_upload_ignores_any_directory_the_client_asks_for(tmp_path): +def test_the_client_names_a_folder_and_never_a_filesystem_path(tmp_path): """ - The destination inside a root is the node's decision, and stays so. + The destination is now the folder the sender is looking at, which means the + client does choose it — and the whole of what keeps that safe is that the + choice is *resolved against the group's own roots* rather than joined to + one. - A client now names the *root* it is uploading into — it has to, once a group - can have several writable ones — but that is a name looked up in the root - table, never a path. Everything below the root is still chosen here, so the - traversal surface a client-chosen destination would open does not exist. + `RootSet.resolve()` refuses `..`, absolute segments and anything whose + resolved form escapes its root, symlinks included. So "which of this + group's folders" is answerable by a member and "which path on the + operator's disk" is not. """ session = _session(tmp_path, "user-1") + (session._ctx["roots"].roots[0].path / "sub").mkdir() + before = set(tmp_path.rglob("*")) - session._do_file_upload({ - "filename": "note.txt", "dir": "../../etc", "path": "/etc", - "chunk_index": 0, "total_chunks": 1, - "data": base64.b64encode(b"x").decode(), - }) + for bad in ("../../etc", "/etc", "shared/../..", "shared/../../etc", + "nope", "shared/missing"): + session.sent.clear() + session._do_file_upload({ + "filename": "note.txt", "dir": bad, + "chunk_index": 0, "total_chunks": 1, + "data": base64.b64encode(b"x").decode(), + }) + refusal = [m for m in session.sent if m.get("type") == "error"] + assert refusal, f"{bad!r} was accepted" + assert refusal[0].get("code") in ("no_such_root", "no_such_directory"), bad - assert (_uploads_dir(session) / "note.txt").read_bytes() == b"x" - assert not (tmp_path / "etc").exists() + assert set(tmp_path.rglob("*")) == before, "a refused upload still wrote" -@pytest.mark.parametrize("named_root", [ - "../../etc", "/etc", "shared/../..", "Shared/uploads", "nope", -]) -def test_a_root_name_is_looked_up_never_joined(tmp_path, named_root): +def test_an_upload_lands_in_the_folder_it_names(tmp_path): """ - The name the client sends is matched against the group's root table and - refused when it matches nothing. A version that joined it to a path — or - that quietly fell back to the first writable root — would turn "which - directory" into either a traversal or a file on a disk the operator did - not intend, and the second is discovered weeks later. + And in that folder itself — the `uploads/` subdirectory the node used to + create is gone. Somebody dropping a file into the folder they are looking + at expects it to be in that folder. """ session = _session(tmp_path, "user-1") - before = set(tmp_path.rglob("*")) + root = session._ctx["roots"].roots[0] + (root.path / "Albums").mkdir() session._do_file_upload({ - "filename": "note.txt", "root": named_root, + "filename": "note.txt", "dir": f"{root.name}/Albums", "chunk_index": 0, "total_chunks": 1, "data": base64.b64encode(b"x").decode(), }) - refusal = [m for m in session.sent if m.get("type") == "error"] - assert refusal and refusal[0].get("code") == "no_such_root", named_root - assert set(tmp_path.rglob("*")) == before, f"wrote something via {named_root!r}" + assert (root.path / "Albums" / "note.txt").read_bytes() == b"x" + assert not (root.path / "Albums" / "uploads").exists(), ( + "the node invented a subdirectory in the operator's library") + assert not (root.path / "uploads").exists() def test_an_upload_goes_to_the_root_it_names(tmp_path): @@ -291,13 +305,13 @@ def test_an_upload_goes_to_the_root_it_names(tmp_path): ]) session._do_file_upload({ - "filename": "note.txt", "root": "Incoming", + "filename": "note.txt", "dir": "Incoming", "chunk_index": 0, "total_chunks": 1, "data": base64.b64encode(b"x").decode(), }) - assert (incoming / "uploads" / "note.txt").read_bytes() == b"x" - assert not (media / "uploads").exists(), "it went to the first root instead" + assert (incoming / "note.txt").read_bytes() == b"x" + assert not (media / "note.txt").exists(), "it went to the first root instead" def test_a_read_only_root_refuses_an_upload(tmp_path): @@ -314,14 +328,14 @@ def test_a_read_only_root_refuses_an_upload(tmp_path): session._is_node_admin = lambda: True session._do_file_upload({ - "filename": "note.txt", "root": "Published", + "filename": "note.txt", "dir": "Published", "chunk_index": 0, "total_chunks": 1, "data": base64.b64encode(b"x").decode(), }) refusal = [m for m in session.sent if m.get("type") == "error"] assert refusal and refusal[0].get("code") == "root_read_only" - assert not (published / "uploads").exists() + assert not (published / "note.txt").exists() def test_a_fully_read_only_group_refuses_an_unaddressed_upload(tmp_path): @@ -343,7 +357,7 @@ def test_a_fully_read_only_group_refuses_an_unaddressed_upload(tmp_path): refusal = [m for m in session.sent if m.get("type") == "error"] assert refusal and refusal[0].get("code") == "no_writable_root" - assert not (published / "uploads").exists() + assert not (published / "note.txt").exists() def test_an_ejected_root_refuses_an_upload(tmp_path): @@ -362,14 +376,14 @@ def test_an_ejected_root_refuses_an_upload(tmp_path): session._ctx["roots"] = roots session._do_file_upload({ - "filename": "note.txt", "root": "USB", + "filename": "note.txt", "dir": "USB", "chunk_index": 0, "total_chunks": 1, "data": base64.b64encode(b"x").decode(), }) refusal = [m for m in session.sent if m.get("type") == "error"] assert refusal and refusal[0].get("code") == "root_unavailable" - assert not (usb / "uploads").exists() + assert not (usb / "note.txt").exists() def test_two_members_can_send_the_same_filename(tmp_path): |