1 files changed, 49 insertions, 21 deletions

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 66ba7ef..483a6a7 100644
--- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
+++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
@@ -100,12 +100,35 @@ MAX_JOIN_ATTEMPTS = 5
 # failed pairings are also counted node-wide over a window.
 MAX_JOIN_FAILURES_WINDOW = 20
 JOIN_FAILURE_WINDOW = 600  # seconds
-UPLOAD_DIR_NAME = ".uploads"
+# Everything a member sends lands here: files from the Files panel and
+# 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"
 # Conservative allowlist: also what keeps markup out of filenames, which the node admin
 # UI used to render unescaped (finding H2).
 SAFE_UPLOAD_NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._ -]{0,127}$")
 
 
+def _free_name(directory: Path, filename: str) -> str:
+    """
+    `filename`, or the first "name (n).ext" that is not taken.
+
+    Never returns the name of a file that exists, so an upload cannot replace
+    one — the property the per-user quarantine used to provide (C5a).
+    """
+    if not (directory / filename).exists():
+        return filename
+    stem, dot, ext = filename.rpartition(".")
+    if not dot:
+        stem, ext = filename, ""
+    for n in range(2, 1000):
+        candidate = f"{stem} ({n}){dot}{ext}"
+        if not (directory / candidate).exists():
+            return candidate
+    raise FileExistsError(filename)
+
+
 def safe_subdir(shared_root: Path, rel: str) -> Path | None:
     """
     Resolve a client-supplied directory under the shared root, or refuse.
@@ -1215,30 +1238,31 @@ class WebRTCPeerSession:
             self._send({"type": "error", "detail": "No shared directory"})
             return
 
-        # 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
+        # One destination, chosen here and not by the client: uploads/ at the root
+        # of the shared directory. C5a is still honoured — the name passed the
+        # allowlist above, and an existing file is never replaced, which was the
+        # real defect (overwriting a file also made the attacker its recorded
+        # uploader, and therefore able to delete it).
+        rel_dir = UPLOAD_DIR_NAME
+        target_dir = shared_root / UPLOAD_DIR_NAME
+        target_dir.mkdir(parents=True, exist_ok=True)
 
         upload_key = f"{rel_dir}/{filename}"
         state = self._uploads.get(upload_key)
+        # A shared directory means two people can send the same name. Refusing the
+        # second is safe but silly — everyone's camera produces IMG_1234.jpg — so
+        # a free name is found instead. Never a replacement.
+        stored_name = state["stored_name"] if state else _free_name(target_dir, filename)
+        tmp_path = target_dir / f"{stored_name}.part"
+        final_path = target_dir / stored_name
+
         if chunk_index == 0:
+            # Backstop: _free_name already guarantees this, and it stays because
+            # it asserts the invariant where the write happens.
             if final_path.exists():
                 self._send({"type": "error", "detail": "File already exists"})
                 return
-            state = {"next_index": 0, "bytes": 0}
+            state = {"next_index": 0, "bytes": 0, "stored_name": stored_name}
             self._uploads[upload_key] = state
         elif state is None:
             self._send({"type": "error", "detail": "Upload not started"})
@@ -1271,15 +1295,19 @@ class WebRTCPeerSession:
             "v": MNP_VERSION,
             "chunk_index": chunk_index,
             "filename": filename,
+            # What it is actually called on disk, which a chat attachment has to
+            # reference and the uploader deserves to be told.
+            "stored_as": stored_name,
+            "dir": rel_dir,
         })
 
         if chunk_index + 1 >= total_chunks:
             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"])
-            self._audit("file_upload", f"{rel_dir}/{filename}")
-            self._register_uploader(ctx, rel_dir, filename)
+                     stored_name, total_chunks, state["bytes"])
+            self._audit("file_upload", f"{rel_dir}/{stored_name}")
+            self._register_uploader(ctx, rel_dir, stored_name)
 
     def _register_uploader(self, ctx: dict, rel_dir: str, filename: str) -> None:
         """