RigShare 1.3.2: access hardening from the Manager review

CHANGELOG.md

@@ -1,5 +1,16 @@
 # Changelog
 
+## 1.3.2 (2026-10-08)
+
+Access rules tightened, following the review of the ComfyUI Manager listing.
+
+### Fixed
+
+- Snapshots of a workflow could be listed and read by an account with no access to its folder whenever nobody had that workflow open. Snapshot access now follows the workflow's folder (private and restricted shared folders included), open or not
+- Joining a live workflow seeded a new room from the caller's document before the access check. Access is checked first, and only someone who can edit the workflow can open a new room
+- Windows path segments that end in a dot or space, or use an 8.3 short name, are now refused instead of being silently dropped, so a path can no longer be classified as a different folder than the one Windows opens
+- After signing in, the login page only follows its `next` redirect when it resolves to the same origin
+
 ## 1.3.1 (2026-10-08)
 
 Security hardening of the workspace folder routes, mainly for Windows servers.

pyproject.toml

@@ -1,7 +1,7 @@
 [project]
 name = "comfyui-rigshare"
 description = "Turn one ComfyUI server into a shared team workspace. Sign-in with accounts, permissions (edit, queue, Manager, admin) and personal API keys, enforced by the server. A private folder for each person plus shared folders with access lists, with Save asking where. Live co-editing of every shared workflow, with cursors, follow mode and conflict-free merges, in the graph or App view. Per-workflow history with a snapshot on every save, team chat with its whole history, and a CPU, RAM, GPU and queue monitor."
-version = "1.3.1"
+version = "1.3.2"
 license = { file = "LICENSE" }
 dependencies = []
 

rigshare_core/hub.py

@@ -334,8 +334,12 @@ def access(self, room, kind, key, perms):
         Access follows the workflow's folder (restricted shared folders), within
         the person's account permissions.
         """
-        if self.workspace and room.key.startswith("file:workflows/"):
-            return self.workspace.role(room.key[len("file:workflows/"):], kind, key, perms)
+        return self.access_for_key(room.key, kind, key, perms)
+
+    def access_for_key(self, room_key, kind, key, perms):
+        """Same as access(), from a room key: works before (or without) a live room."""
+        if self.workspace and room_key.startswith("file:workflows/"):
+            return self.workspace.role(room_key[len("file:workflows/"):], kind, key, perms)
         return "edit" if (perms.get("edit") or perms.get("admin")) else "view"
 
     def client_access(self, room, client):
@@ -556,16 +560,25 @@ async def dispatch(self, client, data):
             else:
                 room = self.rooms.get(key)
                 created = False
-                if room is None:
-                    doc = data.get("doc") if isinstance(data.get("doc"), dict) else {}
-                    room = Room(key, clean_text(data.get("name"), 120) or key, "file", doc)
-                    room.dirty = True
-                    self.rooms[key] = room
-                    created = True
-                if self.client_access(room, client) is None:
+                # Check access before anything is created: only someone who may edit the
+                # workflow can open a room and seed it with their document.
+                role = self.access_for_key(key, client.kind, client.key, client.perms)
+                if role is None:
+                    client.room = None
+                    await self.send(client, {"type": "room_denied", "room": key,
+                                             "name": room.name if room else clean_text(data.get("name"), 120) or key})
+                elif room is None and role != "edit":
                     client.room = None
-                    await self.send(client, {"type": "room_denied", "room": key, "name": room.name})
+                    await self.send(client, {"type": "room_denied", "room": key,
+                                             "name": clean_text(data.get("name"), 120) or key,
+                                             "reason": "Nobody has opened this workflow for editing yet."})
                 else:
+                    if room is None:
+                        doc = data.get("doc") if isinstance(data.get("doc"), dict) else {}
+                        room = Room(key, clean_text(data.get("name"), 120) or key, "file", doc)
+                        room.dirty = True
+                        self.rooms[key] = room
+                        created = True
                     client.room = key
                     room.updated = time.time()
                     await self.send(client, {"type": "room", "room": key, "name": room.name, "version": room.version,

rigshare_core/login.html

@@ -84,6 +84,13 @@
   const err = document.getElementById("e");
   const btn = document.getElementById("b");
   const next = new URLSearchParams(location.search).get("next") || "/";
+  // Follow `next` only when it resolves to this same origin (blocks //host, /\host, scheme tricks).
+  const safeNext = () => {
+    try {
+      const url = new URL(next, location.origin);
+      return url.origin === location.origin ? url.pathname + url.search + url.hash : "/";
+    } catch { return "/"; }
+  };
   if (SETUP) {
     document.title = "Set up · {{SERVER_NAME}}";
     document.getElementById("sub").textContent = "First-time setup";
@@ -112,7 +119,7 @@
       if (res.status === 409) { location.reload(); return; } // someone finished setup meanwhile
       if (!res.ok) throw new Error(data.error || "Sign in failed");
       try { localStorage.setItem("rigshare.token", data.token); } catch {}
-      location.replace(next.startsWith("/") && !next.startsWith("//") ? next : "/");
+      location.replace(safeNext());
     } catch (e) {
       err.textContent = e.message;
       btn.disabled = false;

rigshare_core/routes.py

@@ -14,7 +14,7 @@
 
 from aiohttp import web
 
-from .workspace import Workspace, norm
+from .workspace import InvalidPath, Workspace, norm as _norm
 
 log = logging.getLogger("ComfyUI-RigShare")
 
@@ -148,6 +148,13 @@ def require_admin(request):
     def error(message, status=400):
         return web.json_response({"error": message}, status=status)
 
+    def norm(path):
+        """workspace.norm, answering 403 for a path that is refused instead of adjusted."""
+        try:
+            return _norm(path)
+        except InvalidPath:
+            raise web.HTTPForbidden(text='{"error": "Invalid path"}', content_type="application/json")
+
     def set_cookie(request, response, token):
         secure = request.secure or request.headers.get("X-Forwarded-Proto") == "https"
         response.set_cookie(COOKIE, token, max_age=30 * 24 * 3600, path="/", httponly=True,
@@ -580,8 +587,7 @@ async def list_snapshots(request):
         if not key:
             require_admin(request)
             return web.json_response(store.list_snapshots())
-        room = hub.rooms.get(key)
-        if room and not room_access(request, room):
+        if hub.access_for_key(key, *who(request)) is None:
             return error("No such room", 404)
         return web.json_response(store.list_snapshots(key))
 
@@ -605,8 +611,9 @@ async def get_snapshot(request):
         snap = store.get_snapshot(request.match_info["snap_id"])
         if not snap:
             return error("Not found", 404)
-        room = hub.rooms.get(snap["meta"].get("room", ""))
-        if room and not room_access(request, room):
+        # Access follows the workflow's folder whether or not anyone has it open right now.
+        room_key = snap["meta"].get("room", "")
+        if room_key and hub.access_for_key(room_key, *who(request)) is None:
             return error("Not found", 404)
         return web.json_response(snap)
 

rigshare_core/workspace.py

@@ -55,20 +55,26 @@ def _on_disk_case(parts, base):
     return out
 
 
+class InvalidPath(PermissionError):
+    """A path Windows would read as a different one than the checks do: refused, never adjusted."""
+
+
 def norm(path):
     """Decode and normalise a userdata-relative path ('' for the root).
 
-    Segments Windows would read as something else are dropped, like '..': a colon (drive prefix
-    'C:', NTFS stream), a 8.3 short name, and trailing dots or spaces (Windows strips them, so
-    'users./bob' would open 'users/bob'). Casing is restored from disk on Windows. Anything that
-    still reaches the disk goes through safe_join().
+    A path with a segment Windows would read as something else is refused (InvalidPath), not
+    adjusted: a colon (drive prefix 'C:', NTFS stream), a 8.3 short name, or trailing dots or
+    spaces (Windows strips them, so 'users./bob' would open 'users/bob'). Casing is restored from
+    disk on Windows. Anything that still reaches the disk goes through safe_join().
     """
     path = full_unquote(str(path or "")).replace("\\", "/")
     if not path:
         return ""
     clean = posixpath.normpath("/" + path).lstrip("/")
-    parts = [p for p in clean.split("/")
-             if p and ":" not in p and not _SHORT_NAME_RE.search(p) and (not _WIN or p == p.rstrip(" ."))]
+    parts = [p for p in clean.split("/") if p]
+    for p in parts:
+        if ":" in p or (_WIN and (_SHORT_NAME_RE.search(p) or p != p.rstrip(" ."))):
+            raise InvalidPath("Invalid path")
     if _WIN and _case_root and parts:
         lead = [WORKFLOWS] if parts[0].casefold() == WORKFLOWS else []
         parts = lead + _on_disk_case(parts[len(lead):], _case_root)

tests/python/test_server.py

@@ -666,3 +666,42 @@ async def scenario():
             assert r.status in (400, 403, 404)
             assert (outside / "keep.bin").exists()
     run(scenario())
+
+
+def test_access_is_checked_without_a_live_room(run):
+    async def scenario():
+        async with rig_server() as rig:
+            await rig.setup_admin()
+            alice = await rig.add_user("alice", edit=True)
+            bob = await rig.add_user("bob", edit=True)
+            await rig.add_user("viewer")
+            h = rig.http
+            r = await h.post(rig.url("/rigshare/api/workspace/mkdir"), json={"parent": "@shared", "name": "Team"}, headers=alice)
+            assert r.status == 200
+            rig.hub.workspace.set_acl("shared/Team", {"members": {"alice": "edit"}}, "alice")
+            path = "workflows/shared/Team/plan.json"
+            doc = json.dumps({"nodes": [{"id": 1}], "links": []}).encode()
+            assert (await h.post(rig.url(f"/api/userdata/{enc(path)}"), data=doc, headers=alice)).status == 200
+            key = "file:" + path
+            assert key not in rig.hub.rooms, "nobody has it open"
+            snap_id = rig.store.list_snapshots(key)[0]["id"]
+
+            for who_, status in ((alice, 200), (bob, 404)):
+                r = await h.get(rig.url(f"/rigshare/api/snapshots?room={enc(key)}"), headers=who_)
+                assert r.status == status, who_
+                r = await h.get(rig.url(f"/rigshare/api/snapshots/{snap_id}"), headers=who_)
+                assert r.status == status, who_
+
+            # Joining a workflow you cannot open must not create (or seed) a room.
+            sock = await rig.ws("bob")
+            await sock.send({"type": "join", "room": key, "name": "plan", "doc": BASE_DOC})
+            await sock.until("room_denied")
+            assert key not in rig.hub.rooms
+
+            # A viewer cannot be the one who opens a room, so cannot seed it with their own document.
+            open_key = "file:workflows/common.json"
+            viewer_sock = await rig.ws("viewer")
+            await viewer_sock.send({"type": "join", "room": open_key, "name": "common", "doc": BASE_DOC})
+            assert "editing" in (await viewer_sock.until("room_denied"))["reason"]
+            assert open_key not in rig.hub.rooms
+    run(scenario())

tests/python/test_workspace.py

@@ -133,9 +133,15 @@ def test_rename_user_moves_private_folder(ws):
     assert ws.folders["shared/Team"]["owner"] == "alicia"
 
 
-@pytest.mark.parametrize("raw", ["C:/models", "C:", "a/D:x/b", r"..\..\x", "c%3A%5Cmodels"])
-def test_norm_drops_drive_segments(raw):
-    assert ":" not in norm(raw) and ".." not in norm(raw).split("/")
+@pytest.mark.parametrize("raw", ["C:/models", "C:", "a/D:x/b", "c%3A%5Cmodels"])
+def test_norm_refuses_drive_segments(raw):
+    with pytest.raises(PermissionError):
+        norm(raw)
+
+
+def test_norm_still_resolves_dotdot_inside_the_tree():
+    assert norm(r"..\..\x") == "x"
+    assert norm("a/../b") == "b"
 
 
 @pytest.mark.parametrize("bad", ["C:/models", "C:", "..", "a/../..", "x:y"])
@@ -164,11 +170,11 @@ def test_folder_ops_cannot_leave_the_workspace(ws, tmp_path):
     assert (outside / "keep.txt").exists()
 
 
+@pytest.mark.skipif(os.name != "nt", reason="trailing dots, spaces and short names only alias on Windows")
 @pytest.mark.parametrize("raw", ["users./bob/a.json", "users /bob", "USERS~1/bob", "users/bob./a.json"])
-def test_norm_drops_windows_aliases(raw):
-    if os.name != "nt" and ("." in raw.split("/")[0] or " " in raw.split("/")[0]):
-        pytest.skip("trailing dots and spaces only alias on Windows")
-    assert "~1" not in norm(raw)
+def test_norm_refuses_windows_aliases(raw):
+    with pytest.raises(PermissionError):
+        norm(raw)
 
 
 @pytest.mark.skipif(os.name != "nt", reason="case-insensitive filesystem")