RigShare 1.3.2: access checks without a live room, refuse Windows alias paths, same-origin login redirect

This commit is contained in:
Edouard Durand
2026-10-08 10:57:57 +00:00
parent fe45031dcb
commit 3c5e8bfa3a
8 changed files with 119 additions and 30 deletions
+11
View File
@@ -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.
+1 -1
View File
@@ -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 = []
+23 -10
View File
@@ -334,8 +334,12 @@ class Hub:
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 @@ class Hub:
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})
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": 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,
+8 -1
View File
@@ -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;
+12 -5
View File
@@ -14,7 +14,7 @@ from collections import defaultdict
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 setup(server, store, hub):
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 @@ def setup(server, store, hub):
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 @@ def setup(server, store, hub):
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)
+12 -6
View File
@@ -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)
+39
View File
@@ -666,3 +666,42 @@ def test_folder_routes_refuse_paths_outside_the_workspace(run, tmp_path):
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())
+13 -7
View File
@@ -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")