diff options
Diffstat (limited to 'packages')
6 files changed, 280 insertions, 76 deletions
diff --git a/packages/meshbay-common/src/meshbay_common/protocol.py b/packages/meshbay-common/src/meshbay_common/protocol.py index 50e8663..08dc47b 100644 --- a/packages/meshbay-common/src/meshbay_common/protocol.py +++ b/packages/meshbay-common/src/meshbay_common/protocol.py @@ -35,6 +35,8 @@ class MNP: # the wire contract looking as though the endpoint still existed. FILE_UPLOAD = "file_upload" # client pushes file chunk to node FILE_UPLOAD_ACK = "file_upload_ack" # node acknowledges chunk receipt + DIR_CREATE = "dir_create" # client → node: make a directory + DIR_CREATE_ACK = "dir_create_ack" # node → client: created FILE_DELETE = "file_delete" # client requests file deletion FILE_DELETE_ACK = "file_delete_ack" # node confirms deletion STREAM_REQUEST = "stream_req" # client requests MSE video stream diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index cff001b..def6c5b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -824,6 +824,9 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { const [tab, setTab] = useState('files'); const [uploading, setUploading] = useState(false); const [ulState, setUlState] = useState(null); + // Directories are not index entries, so a new empty one needs a nudge + // to appear in the breadcrumb listing. + const [nodeDirs, setNodeDirs] = useState([]); const [menuOpen, setMenuOpen] = useState(null); const [isNodeAdmin, setIsNodeAdmin] = useState(false); const [needsCode, setNeedsCode] = useState(false); @@ -916,6 +919,7 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { if (cancelled) return; const synced = msg.entries || []; setEntries(synced); + if (msg.dirs) setNodeDirs(msg.dirs); setCached(false); cacheGroupIndex(groupId, group ? group.name : groupId, synced); }; @@ -924,6 +928,7 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { if (cancelled) return; const freshEntries = indexMsg.entries || []; setEntries(freshEntries); + setNodeDirs(indexMsg.dirs || []); setCached(false); setStatus('connected'); @@ -1027,7 +1032,7 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { for (let i = 0; i < totalChunks; i++) { const slice = file.slice(i * UPLOAD_CHUNK_SIZE, (i + 1) * UPLOAD_CHUNK_SIZE); const buf = new Uint8Array(await slice.arrayBuffer()); - await transport.uploadChunk(file.name, i, totalChunks, buf); + await transport.uploadChunk(file.name, i, totalChunks, buf, currentPath); // Bytes actually acknowledged by the node, not bytes read locally. setUlState(prev => prev && { ...prev, sent: Math.min(file.size, (i + 1) * UPLOAD_CHUNK_SIZE) }); @@ -1038,13 +1043,29 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { await new Promise(r => setTimeout(r, 2500)); const indexMsg = await transport.fetchIndex(); if (indexMsg.entries) setEntries(indexMsg.entries); + if (indexMsg.dirs) setNodeDirs(indexMsg.dirs); } catch (err) { setError(err.message); } finally { setUploading(false); setUlState(null); } - }, []); + }, [currentPath]); + + const makeDirectory = useCallback(async () => { + const transport = transportRef.current; + if (!transport || !transport.connected) return; + const name = prompt(t('group.mkdir_prompt')); + if (!name || !name.trim()) return; + try { + await transport.createDirectory(currentPath, name.trim()); + const indexMsg = await transport.fetchIndex(); + if (indexMsg.entries) setEntries(indexMsg.entries); + if (indexMsg.dirs) setNodeDirs(indexMsg.dirs); + } catch (err) { + setError(err.message); + } + }, [currentPath]); const deleteFile = useCallback(async (entry) => { const transport = transportRef.current; @@ -1104,6 +1125,15 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { return sortAsc ? cmp : -cmp; }); + // The node's own listing, so an empty folder is visible, plus anything implied + // by a file path in case the two ever disagree. + for (const d of nodeDirs) { + if (!currentPath && !d.includes('/')) dirs.add(d); + else if (currentPath && d.startsWith(currentPath + '/')) { + const rest = d.slice(currentPath.length + 1); + if (!rest.includes('/')) dirs.add(rest); + } + } const subdirs = [...dirs].sort(); const baseLabel = { @@ -1205,6 +1235,8 @@ function GroupPage({ groupId, group, token, username, userId, onRefreshAuth }) { <input type="file" style="display:none" onChange=${uploadFile} disabled=${uploading} /> </label> + <button class="admin-btn" style="margin-right:8px" onClick=${makeDirectory} + disabled=${uploading}>${t('group.mkdir')}</button> <div class="breadcrumbs"> <a class="crumb" onClick=${() => setCurrentPath('')}>/</a> ${breadcrumbs.map((seg, i) => { @@ -1578,49 +1610,6 @@ function MembersPanel({ groupId, group, token, transportRef, gekRef, return html` <div class="members-panel"> - ${isAdmin && html` - <form class="invite-form" onSubmit=${doInvite}> - <h4>${t('members.invite_title')}</h4> - ${error && html`<p class="error-msg">${error}</p>`} - ${inviteCode && html` - <div class="success-msg" style="margin-bottom:8px"> - <p>${t('members.invite_code_ready', { user: inviteCode.username })}</p> - <p style="font-family:monospace;font-size:1.4em;letter-spacing:2px;margin:6px 0"> - ${inviteCode.code} - </p> - <p>${t('members.invite_code_hint')}</p> - </div> - `} - <div style="display:flex;gap:8px"> - <input type="text" placeholder="${t('members.username_placeholder')}" - value=${inviteUser} onInput=${e => setInviteUser(e.target.value)} required /> - <button class="admin-btn" type="submit" disabled=${inviting}> - ${inviting ? '...' : t('members.invite_btn')} - </button> - </div> - </form> - `} - <table class="admin-table"> - <thead> - <tr> - <th>${t('admin.col_username')}</th> - <th>${t('members.group_role')}</th> - </tr> - </thead> - <tbody> - ${members.map(m => html` - <tr key=${m.user_id}> - <td>${m.username}</td> - <td> - ${m.user_id === adminId - ? html`<span class="badge" style="background:var(--accent);color:var(--accent-text)">${t('members.owner')}</span>` - : html`<span class="badge">${t('members.member')}</span>` - } - </td> - </tr> - `)} - </tbody> - </table> ${isNodeAdmin && html` <form class="invite-form" onSubmit=${doPair}> <h4>${t('members.pair_title')}</h4> @@ -2443,7 +2432,50 @@ function AdminPage({ token }) { value=${userSearch} onInput=${e => { setUserSearch(e.target.value); loadUsers(e.target.value); }} /> <span class="settings-value">${usersTotal} total</span> </div> - <table class="admin-table"> + ${isAdmin && html` + <form class="invite-form" onSubmit=${doInvite}> + <h4>${t('members.invite_title')}</h4> + ${error && html`<p class="error-msg">${error}</p>`} + ${inviteCode && html` + <div class="success-msg" style="margin-bottom:8px"> + <p>${t('members.invite_code_ready', { user: inviteCode.username })}</p> + <p style="font-family:monospace;font-size:1.4em;letter-spacing:2px;margin:6px 0"> + ${inviteCode.code} + </p> + <p>${t('members.invite_code_hint')}</p> + </div> + `} + <div style="display:flex;gap:8px"> + <input type="text" placeholder="${t('members.username_placeholder')}" + value=${inviteUser} onInput=${e => setInviteUser(e.target.value)} required /> + <button class="admin-btn" type="submit" disabled=${inviting}> + ${inviting ? '...' : t('members.invite_btn')} + </button> + </div> + </form> + `} + <table class="admin-table"> + <thead> + <tr> + <th>${t('admin.col_username')}</th> + <th>${t('members.group_role')}</th> + </tr> + </thead> + <tbody> + ${members.map(m => html` + <tr key=${m.user_id}> + <td>${m.username}</td> + <td> + ${m.user_id === adminId + ? html`<span class="badge" style="background:var(--accent);color:var(--accent-text)">${t('members.owner')}</span>` + : html`<span class="badge">${t('members.member')}</span>` + } + </td> + </tr> + `)} + </tbody> + </table> + <table class="admin-table"> <thead><tr> <th>${t('admin.col_username')}</th> <th>${t('admin.col_role')}</th> diff --git a/packages/meshbay-hub/src/meshbay_hub/static/i18n.js b/packages/meshbay-hub/src/meshbay_hub/static/i18n.js index e3e0def..c19411b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/i18n.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/i18n.js @@ -89,6 +89,8 @@ const en = { 'group.offline_title': 'No nodes are currently online for this group.', 'group.offline_hint': 'Files will appear when a node hosting this group connects.', 'group.upload': 'Upload', + 'group.mkdir': 'New folder', + 'group.mkdir_prompt': 'Name of the new folder:', 'group.uploading': 'Uploading...', 'group.view': 'View', 'group.delete': 'Delete', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index 0a8796e..d2b24a3 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -488,15 +488,28 @@ class MeshBayTransport { this._send({ type: 'stream_req', v: '0.1', file_id: fileId }); } - async uploadChunk(filename, chunkIndex, totalChunks, data) { + async uploadChunk(filename, chunkIndex, totalChunks, data, dir) { const msg = await this._sendAndWait({ type: 'file_upload', v: '0.1', filename, + // Where the member is looking. The node confines it under the shared root + // and refuses to overwrite, so this is a destination, not a licence. + dir: dir || '', chunk_index: chunkIndex, total_chunks: totalChunks, data: data, }); + if (msg.type === 'error') throw new Error(msg.detail); + return msg; + } + + /** Create a directory under the current one. Any member may. */ + async createDirectory(dir, name) { + const msg = await this._sendAndWait({ + type: 'dir_create', v: '0.1', dir: dir || '', name, + }); + if (msg.type === 'error') throw new Error(msg.detail); return msg; } 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 fe4e3c2..66ba7ef 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -106,6 +106,37 @@ UPLOAD_DIR_NAME = ".uploads" SAFE_UPLOAD_NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._ -]{0,127}$") +def safe_subdir(shared_root: Path, rel: str) -> Path | None: + """ + Resolve a client-supplied directory under the shared root, or refuse. + + Uploads land where the member is looking now rather than in a per-user + quarantine, so the path arrives from the wire and every part of it has to be + checked: each segment against the same allowlist as filenames, and the + resolved result against the root. `..`, absolute paths, symlinks pointing + out, and anything with a separator in a segment are all refused here rather + than in the caller, so there is one place to get it right. + + The quarantine was the fix for C5a; what actually mattered in it — no + overwrite, a name allowlist, and confinement — is kept by this plus the + caller's existing checks. + """ + rel = (rel or "").strip().strip("/") + if not rel: + return shared_root + parts = [seg for seg in rel.split("/") if seg not in ("", ".")] + if any(seg == ".." or not SAFE_UPLOAD_NAME.match(seg) for seg in parts): + return None + try: + target = (shared_root / Path(*parts)).resolve() + root = shared_root.resolve() + except OSError: + return None + if target != root and root not in target.parents: + return None + return target + + def _extract_dtls_fingerprint(sdp: str) -> bytes: """Extract the DTLS SHA-256 fingerprint from SDP as raw 32 bytes.""" for line in sdp.splitlines(): @@ -301,6 +332,8 @@ class WebRTCPeerSession: self._do_chat_history(msg) elif mtype == MNP.FILE_UPLOAD: self._do_file_upload(msg) + elif mtype == MNP.DIR_CREATE: + self._do_dir_create(msg) elif mtype == MNP.FILE_DELETE: self._do_file_delete(msg) elif mtype == MNP.ADMIN_RESPONSE: @@ -834,6 +867,48 @@ class WebRTCPeerSession: self._send(reply) self._audit_join("gek_wrapped", f"group={group_id[:8]}") + def _do_dir_create(self, msg: dict) -> None: + """ + Create a directory, for any member of the group. + + Same confinement as an upload: every segment passes the name allowlist and + the result must resolve under the shared root. Making a directory is not a + privileged act — a member who can add a file can organise where it goes — + but it writes to the operator's disk, so it is audited like one. + """ + ctx = self._group_ctx() + shared_root = ctx.get("shared_root") + if not shared_root: + self._send({"type": "error", "detail": "No shared directory"}) + return + + name = str(msg.get("name", "")).strip() + if not SAFE_UPLOAD_NAME.match(name): + self._send({"type": "error", "detail": "Invalid directory name"}) + return + + parent = safe_subdir(shared_root, msg.get("dir") or "") + if parent is None or not parent.is_dir(): + self._send({"type": "error", "detail": "Invalid directory"}) + return + + target = safe_subdir(shared_root, f"{(msg.get('dir') or '').strip('/')}/{name}") + if target is None: + self._send({"type": "error", "detail": "Invalid directory"}) + return + if target.exists(): + self._send({"type": "error", "detail": "Already exists"}) + return + + target.mkdir(parents=False) + log.info("Directory created by %s: %s", self._user_id[:8], + target.relative_to(shared_root)) + self._audit("dir_create", str(target.relative_to(shared_root))) + self._send({ + "type": MNP.DIR_CREATE_ACK, "v": MNP_VERSION, + "dir": str(target.relative_to(shared_root)), + }) + async def _do_keypair_bundle_delete(self) -> None: """ Withdraw our own key backup from this node. @@ -918,8 +993,28 @@ class WebRTCPeerSession: "group_id": idx.group_id, "version": idx.version, "entries": entries, + # Directories are not index entries, so the client used to infer them + # from file paths — which means a folder someone just created, or one + # they emptied, simply did not exist as far as the UI was concerned. + "dirs": self._list_dirs(ctx.get("shared_root")), }) + @staticmethod + def _list_dirs(shared_root: Path | None) -> list[str]: + """Directories under the shared root, relative and sorted.""" + if not shared_root: + return [] + out = [] + try: + for path in sorted(shared_root.rglob("*")): + if path.is_dir() and not path.name.startswith("."): + rel = path.relative_to(shared_root) + if not any(part.startswith(".") for part in rel.parts): + out.append(str(rel)) + except OSError: + return [] + return out[:2000] + def _do_file_request(self, msg: dict) -> None: ctx = self._group_ctx() file_id = msg["file_id"] @@ -1120,21 +1215,31 @@ class WebRTCPeerSession: self._send({"type": "error", "detail": "No shared directory"}) return - # Per-user quarantine: a member can only ever write inside their own directory, - # so they cannot overwrite the operator's files or another member's (C5a). - rel_dir = f"{UPLOAD_DIR_NAME}/{self._user_id}" - user_dir = shared_root / UPLOAD_DIR_NAME / self._user_id - user_dir.mkdir(parents=True, exist_ok=True) - tmp_path = user_dir / f"{filename}.part" - final_path = user_dir / filename + # Where the member is looking, not a quarantine named after their user id. + # C5a is still honoured by what follows: the path is confined under the + # shared root (safe_subdir), the name passed the allowlist above, and an + # existing file is never overwritten — which is what made the quarantine + # necessary, since overwriting a file also made the attacker its recorded + # uploader and therefore able to delete it. + rel_dir = (msg.get("dir") or "").strip().strip("/") + target_dir = safe_subdir(shared_root, rel_dir) + if target_dir is None: + self._send({"type": "error", "detail": "Invalid directory"}) + return + if not target_dir.is_dir(): + self._send({"type": "error", "detail": "No such directory"}) + return + tmp_path = target_dir / f"{filename}.part" + final_path = target_dir / filename - state = self._uploads.get(filename) + upload_key = f"{rel_dir}/{filename}" + state = self._uploads.get(upload_key) if chunk_index == 0: if final_path.exists(): self._send({"type": "error", "detail": "File already exists"}) return state = {"next_index": 0, "bytes": 0} - self._uploads[filename] = state + self._uploads[upload_key] = state elif state is None: self._send({"type": "error", "detail": "Upload not started"}) return @@ -1151,7 +1256,7 @@ class WebRTCPeerSession: chunk_bytes = bytes(data) if state["bytes"] + len(chunk_bytes) > MAX_UPLOAD_BYTES: - self._uploads.pop(filename, None) + self._uploads.pop(upload_key, None) tmp_path.unlink(missing_ok=True) self._send({"type": "error", "detail": "Upload exceeds size limit"}) return @@ -1169,7 +1274,7 @@ class WebRTCPeerSession: }) if chunk_index + 1 >= total_chunks: - self._uploads.pop(filename, None) + self._uploads.pop(upload_key, None) tmp_path.rename(final_path) log.info("Upload complete: %s (%d chunks, %d bytes)", filename, total_chunks, state["bytes"]) diff --git a/packages/meshbay-node/tests/test_security_regressions.py b/packages/meshbay-node/tests/test_security_regressions.py index dcd9cf6..7627926 100644 --- a/packages/meshbay-node/tests/test_security_regressions.py +++ b/packages/meshbay-node/tests/test_security_regressions.py @@ -120,9 +120,14 @@ def _session(tmp_path: Path, user_id: str) -> WebRTCPeerSession: def test_upload_cannot_overwrite_another_members_file(tmp_path): """ C5a: uploads used to land in the shared root under a client-chosen name and - overwrite whatever was there. That let any member destroy the operator's files, - and — by becoming the recorded uploader of the replaced file — delete them - through the uploader path, bypassing the Ed25519 admin challenge entirely. + overwrite whatever was there. That let any member destroy the operator's + files, and — by becoming the recorded uploader of the replaced file — delete + them through the uploader path, bypassing the Ed25519 admin challenge. + + The per-user quarantine that fixed it was removed on 2026-08-14: files now go + where the member is looking, because a shared directory nobody can organise is + not a shared directory. What made the quarantine work is kept, and is what + this test now asserts — an existing file is never replaced. """ victim = _session(tmp_path, "victim-user") shared_root = victim._ctx["shared_root"] @@ -139,23 +144,14 @@ def test_upload_cannot_overwrite_another_members_file(tmp_path): }) assert original.read_bytes() == b"operator's original content" - uploaded = shared_root / ".uploads" / "attacker-user" / "important.mp4" - assert uploaded.exists(), "upload should be quarantined, not dropped" - assert uploaded.read_bytes() == b"attacker content" - - -def test_upload_rejects_out_of_order_chunks(tmp_path): - """C5a: chunk_index > 0 used to append blindly to any .part file on disk.""" - session = _session(tmp_path, "user-1") - session._do_file_upload({ - "filename": "movie.mp4", "chunk_index": 3, "total_chunks": 5, - "data": base64.b64encode(b"spliced").decode(), - }) - assert any(m.get("type") == "error" for m in session.sent) + assert any(m.get("type") == "error" for m in attacker.sent), ( + "the upload must be refused outright, not silently dropped") + assert not (shared_root / "important.mp4.part").exists(), ( + "a refused upload must leave nothing behind") def test_upload_second_attempt_cannot_replace_own_completed_file(tmp_path): - """C5a: even the original uploader goes through a fresh name, not an overwrite.""" + """C5a: even the original uploader does not get to overwrite.""" session = _session(tmp_path, "user-1") payload = {"filename": "movie.mp4", "chunk_index": 0, "total_chunks": 1, "data": base64.b64encode(b"first").decode()} @@ -164,10 +160,64 @@ def test_upload_second_attempt_cannot_replace_own_completed_file(tmp_path): session._do_file_upload(dict(payload)) assert any(m.get("type") == "error" for m in session.sent) - stored = session._ctx["shared_root"] / ".uploads" / "user-1" / "movie.mp4" + stored = session._ctx["shared_root"] / "movie.mp4" assert stored.read_bytes() == b"first" +@pytest.mark.parametrize("bad_dir", [ + "..", "../..", "/etc", "a/../../b", "./../x", "sub/../../..", + "\\..\\..", "~", "a/./../..", +]) +def test_upload_cannot_escape_the_shared_root(tmp_path, bad_dir): + """ + The destination now arrives from the client, which is a path the node did not + choose. Every segment goes through the same allowlist as a filename and the + result must resolve inside the shared root. + """ + session = _session(tmp_path, "user-1") + outside = tmp_path / "outside.txt" + + session._do_file_upload({ + "filename": "outside.txt", "dir": bad_dir, + "chunk_index": 0, "total_chunks": 1, + "data": base64.b64encode(b"escaped").decode(), + }) + + assert any(m.get("type") == "error" for m in session.sent), bad_dir + assert not outside.exists(), f"upload escaped the shared root via {bad_dir!r}" + + +@pytest.mark.parametrize("bad", [ + {"dir": "..", "name": "evil"}, + {"dir": "", "name": ".."}, + {"dir": "", "name": "a/b"}, + {"dir": "/etc", "name": "evil"}, + {"dir": "", "name": ".hidden"}, +]) +def test_dir_create_cannot_escape_the_shared_root(tmp_path, bad): + """Creating a directory is not privileged, but it still writes to a disk.""" + session = _session(tmp_path, "user-1") + before = set(tmp_path.rglob("*")) + + session._do_dir_create(bad) + + assert any(m.get("type") == "error" for m in session.sent), bad + assert set(tmp_path.rglob("*")) == before, f"created something via {bad!r}" + + +def test_upload_still_refuses_unsafe_names_in_a_subdirectory(tmp_path): + """The name allowlist is not weakened by having somewhere to put the file.""" + session = _session(tmp_path, "user-1") + (session._ctx["shared_root"] / "docs").mkdir() + + session._do_file_upload({ + "filename": "../escape.txt", "dir": "docs", + "chunk_index": 0, "total_chunks": 1, + "data": base64.b64encode(b"x").decode(), + }) + assert any(m.get("type") == "error" for m in session.sent) + + # ── H1: group isolation ────────────────────────────────────────────────────── def test_chat_store_and_peers_are_per_group(tmp_path): |