RigShare 1.3.1: refuse folder paths outside the workspace (Windows)

* RigShare 1.3.1: refuse folder paths outside the workspace (Windows)

* Tests: escape test no longer assumes Windows path semantics

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## 1.3.1 (2026-10-08)
+
+Security hardening of the workspace folder routes, mainly for Windows servers.
+
+### Fixed
+
+- Folder rename, move, delete and create (and the workspace listing) could be pointed outside the workflows folder with a Windows drive prefix (`C:`), which let `rmtree` / `rename` act on folders such as models or custom_nodes. Every path is now checked before it touches the disk: segments with a colon, backslash tricks or `..` are refused, and the resolved path must stay inside the workflows folder
+- On Windows, case and aliases of a path were not seen by the access checks: `Users/alice`, `users./alice` or `USERS~1/alice` opened Alice's private folder as if it were common, and `Shared/<restricted folder>` skipped its access list. Paths are now matched to the name on disk and trailing dots, spaces and 8.3 short names are dropped before any permission check
+- New usernames and folder names can no longer end in a dot or space, start with a dot, or be a Windows device name (`CON`, `NUL`, `COM1`...). Usernames that differ only by case count as the same name, since they share one folder on Windows. Existing accounts and folders are untouched
+
+### Added
+
+- Tests for the JWT module, the server stats, and the folder routes refusing paths outside the workspace
+
 ## 1.3.0 (2026-10-04)
 
 A Performance panel, a floating monitor that docks, and Manager in its own tab.

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.0"
+version = "1.3.1"
 license = { file = "LICENSE" }
 dependencies = []
 

rigshare_core/store.py

@@ -24,7 +24,7 @@ SESSION_TTL = 30 * 24 * 3600      # a login lasts at most this long
 TOKEN_TTL = 12 * 3600              # each JWT is short-lived...
 TOKEN_REFRESH = 6 * 3600           # ...and re-issued while in use once older than this
 ISSUER = "rigshare"
-USERNAME_RE = re.compile(r"^[A-Za-z0-9_.-]{2,32}$")
+USERNAME_RE = re.compile(r"^(?!(?i:con|prn|aux|nul|com\d|lpt\d)(?:\.|$))[A-Za-z0-9_][A-Za-z0-9_.-]{0,30}[A-Za-z0-9_-]\Z")  # no leading/trailing dot, no Windows device names
 PERMS = ("edit", "queue", "manager", "admin")
 API_KEY_PREFIX = "rs_"
 
@@ -216,10 +216,14 @@ class Store:
         _verify_password(password, _hash_password("x", b"0" * 16))
         return None
 
+    def _name_taken(self, name, ignore=None):
+        """Names differing only by case share one folder on Windows, so they count as the same."""
+        return any(u.casefold() == name.casefold() for u in self.users if u != ignore)
+
     def create_user(self, username, password, display_name=None, perms=None):
         if not USERNAME_RE.match(username or ""):
             raise ValueError("Username must be 2-32 characters: letters, digits, _ . -")
-        if username in self.users:
+        if self._name_taken(username):
             raise ValueError("Username already exists")
         if len(password or "") < 8:
             raise ValueError("Password must be at least 8 characters")
@@ -239,7 +243,7 @@ class Store:
             raise KeyError(username)
         if not USERNAME_RE.match(new_username):
             raise ValueError("Username must be 2-32 characters: letters, digits, _ . -")
-        if new_username in self.users:
+        if self._name_taken(new_username, ignore=username):
             raise ValueError("Username already exists")
         with self._lock:
             user = self.users.pop(username)

rigshare_core/workspace.py

@@ -20,7 +20,7 @@ import time
 from urllib.parse import unquote
 
 WORKFLOWS = "workflows"
-FOLDER_NAME_RE = re.compile(r"^[\w][\w .()-]{0,63}$")
+FOLDER_NAME_RE = re.compile(r"^(?!(?i:con|prn|aux|nul|com\d|lpt\d)(?:\.|$))\w(?:[\w .()-]{0,62}[\w()-])?\Z")  # no trailing dot/space, no Windows device names
 ROLE_RANK = {None: 0, "view": 1, "edit": 2}
 
 
@@ -35,10 +35,60 @@ def full_unquote(value):
     return value
 
 
+_WIN = os.name == "nt"
+_SHORT_NAME_RE = re.compile(r"~\d")  # Windows 8.3 alias of a long name (USERS~1)
+_case_root = None  # workflows directory used to restore on-disk casing (Windows only)
+
+
+def _on_disk_case(parts, base):
+    """Windows ignores case: map each segment to the casing it has on disk, so 'Users/Bob' is
+    judged as the same path it will open."""
+    out = []
+    for part in parts:
+        try:
+            names = os.listdir(base) if base else []
+        except OSError:
+            names = []
+        match = part if part in names else next((n for n in names if n.casefold() == part.casefold()), part)
+        out.append(match)
+        base = os.path.join(base, match) if base and match in names else None
+    return out
+
+
 def norm(path):
-    """Decode and normalise a userdata-relative path ('' for the root)."""
+    """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().
+    """
     path = full_unquote(str(path or "")).replace("\\", "/")
-    return posixpath.normpath("/" + path).lstrip("/") if path else ""
+    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(" ."))]
+    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)
+    return "/".join(parts)
+
+
+def safe_join(root, *parts):
+    """Join relative parts under root; refuse anything that resolves outside it."""
+    segments = [p for part in parts for p in str(part).replace("\\", "/").split("/") if p]
+    if any(":" in p or p in (".", "..") or (_WIN and p != p.rstrip(" .")) for p in segments):
+        raise PermissionError("Invalid path")
+    full = os.path.join(root, *segments)
+    base = os.path.realpath(root)
+    try:
+        inside = os.path.commonpath([base, os.path.realpath(full)]) == base
+    except ValueError:  # different drives
+        inside = False
+    if not inside:
+        raise PermissionError("Invalid path")
+    return full
 
 
 def has_app(doc):
@@ -56,6 +106,8 @@ class Workspace:
     def __init__(self, store, root):
         self.store = store
         self.root = root  # absolute path of the workflows directory
+        global _case_root
+        _case_root = root
         self.folders = store._read("folders.json", {})  # "shared/<name>" -> {owner, acl, created}
         os.makedirs(os.path.join(root, "users"), exist_ok=True)
         os.makedirs(os.path.join(root, "shared"), exist_ok=True)
@@ -147,7 +199,7 @@ class Workspace:
         return self.role(rel, kind, key, perms) is not None
 
     def _files(self, rel_dir):
-        base = os.path.join(self.root, *rel_dir.split("/")) if rel_dir else self.root
+        base = safe_join(self.root, rel_dir) if rel_dir else self.root
         out = []
         if not os.path.isdir(base):
             return out
@@ -198,7 +250,7 @@ class Workspace:
     # ----- browsing (Files tab) -------------------------------------------
 
     def _abs(self, rel):
-        return os.path.join(self.root, *[p for p in rel.split("/") if p])
+        return safe_join(self.root, rel)
 
     def _file_has_app(self, full):
         """Read a workflow file only when it changed since the last check."""
@@ -372,7 +424,7 @@ class Workspace:
         name = (name or "").strip()
         if not FOLDER_NAME_RE.match(name) or name in (".", ".."):
             raise ValueError("Folder names: up to 64 letters, digits, spaces and . _ - ( )")
-        path = os.path.join(self.root, "shared", name)
+        path = safe_join(self.root, "shared", name)
         if os.path.exists(path):
             raise ValueError("A folder with that name already exists")
         os.makedirs(path)
@@ -385,8 +437,8 @@ class Workspace:
         if not FOLDER_NAME_RE.match(new_name):
             raise ValueError("Folder names: up to 64 letters, digits, spaces and . _ - ( )")
         new_folder = f"shared/{new_name}"
-        src = os.path.join(self.root, *folder.split("/"))
-        dst = os.path.join(self.root, "shared", new_name)
+        src = safe_join(self.root, folder)
+        dst = safe_join(self.root, "shared", new_name)
         if os.path.exists(dst):
             raise ValueError("A folder with that name already exists")
         os.rename(src, dst)
@@ -395,7 +447,7 @@ class Workspace:
         return new_folder
 
     def delete_folder(self, folder):
-        path = os.path.join(self.root, *folder.split("/"))
+        path = safe_join(self.root, folder)
         if self._files(folder):
             raise ValueError("Move or delete the workflows inside it first")
         shutil.rmtree(path, ignore_errors=True)

tests/python/test_jwt.py

@@ -0,0 +1,60 @@
+"""HS256 tokens: round trip, tampering, expiry, issuer, algorithm confusion."""
+
+import base64
+import json
+import time
+
+import pytest
+
+from rigshare_core import jwt
+
+SECRET = b"s3cret"
+
+
+def claims(**extra):
+    return {"iss": "rig", "exp": time.time() + 60, **extra}
+
+
+def b64(obj):
+    return base64.urlsafe_b64encode(json.dumps(obj).encode()).rstrip(b"=").decode()
+
+
+def test_round_trip():
+    token = jwt.encode(claims(sub="bob"), SECRET)
+    assert jwt.decode(token, SECRET, "rig")["sub"] == "bob"
+
+
+def test_wrong_secret_and_tampered_payload():
+    token = jwt.encode(claims(sub="bob"), SECRET)
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(token, b"other", "rig")
+    head, _, sig = token.split(".")
+    forged = f"{head}.{b64(claims(sub='admin'))}.{sig}"
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(forged, SECRET, "rig")
+
+
+def test_expiry_leeway_and_not_before():
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(jwt.encode({"iss": "rig", "exp": time.time() - 3600}, SECRET), SECRET, "rig")
+    jwt.decode(jwt.encode({"iss": "rig", "exp": time.time() - 5}, SECRET), SECRET, "rig")  # inside leeway
+    assert jwt.decode(jwt.encode({"iss": "rig", "exp": time.time() - 3600}, SECRET), SECRET, "rig", verify_exp=False)
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(jwt.encode(claims(nbf=time.time() + 3600), SECRET), SECRET, "rig")
+
+
+def test_wrong_issuer():
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(jwt.encode(claims(), SECRET), SECRET, "someone-else")
+
+
+def test_alg_none_is_refused():
+    token = f"{b64({'alg': 'none', 'typ': 'JWT'})}.{b64(claims())}."
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(token, SECRET, "rig")
+
+
+@pytest.mark.parametrize("junk", ["", "a.b", "a.b.c.d", "!!!.???.***", "e30.e30.e30"])
+def test_malformed_tokens(junk):
+    with pytest.raises(jwt.InvalidToken):
+        jwt.decode(junk, SECRET, "rig")

tests/python/test_server.py

@@ -646,3 +646,23 @@ def test_performance_run_log_eta_history_and_clear(run):
             assert (await r.json())["cleared"] == 3
             assert hub.etas() == {"video": {"ms": 90000, "runs": 1}}, "the ETA starts over"
     run(scenario())
+
+
+def test_folder_routes_refuse_paths_outside_the_workspace(run, tmp_path):
+    async def scenario():
+        async with rig_server() as rig:
+            await rig.setup_admin()
+            await rig.add_user("alice", edit=True)
+            outside = tmp_path / "models"
+            outside.mkdir()
+            (outside / "keep.bin").write_bytes(b"x")
+            H, h = rig.headers, rig.http
+            targets = [str(outside), "C:" + str(outside)[2:], r"..\..\models", "shared/C:/evil"]
+            for route, extra in (("delete-folder", {}), ("rename-folder", {"name": "x"}), ("move-folder", {"dest": "@shared"})):
+                for target in targets:
+                    r = await h.post(rig.url(f"/rigshare/api/workspace/{route}"), json={"path": target, **extra}, headers=H["alice"])
+                    assert r.status in (400, 403, 404), (route, target, r.status)
+            r = await h.post(rig.url("/rigshare/api/workspace/mkdir"), json={"parent": "C:/", "name": "x"}, headers=H["alice"])
+            assert r.status in (400, 403, 404)
+            assert (outside / "keep.bin").exists()
+    run(scenario())

tests/python/test_stats.py

@@ -0,0 +1,45 @@
+"""Server stats: every metric is optional and never raises."""
+
+from rigshare_core import stats
+
+
+class Queue:
+    def __init__(self, running, pending):
+        self.running, self.pending = running, pending
+
+    def get_current_queue_volatile(self):
+        return self.running, self.pending
+
+
+class Server:
+    def __init__(self, queue):
+        self.prompt_queue = queue
+
+
+def test_queue_items_order_and_node_count():
+    server = Server(Queue([(1, "r1", {"1": {}, "2": {}})], [(5, "p5", {"1": {}}), (3, "p3", {})]))
+    items = stats.queue_items(server)
+    assert [(i["id"], i["status"], i["nodes"]) for i in items] == [("r1", "running", 2), ("p3", "pending", 0), ("p5", "pending", 1)]
+
+
+def test_queue_items_survive_odd_entries_and_missing_queue():
+    assert stats.queue_items(Server(Queue([(1,)], [(9,)]))) == []
+    assert stats.queue_items(object()) is None
+
+
+def test_queue_counts_with_fallback():
+    assert stats._queue(Server(Queue([1], [2, 3]))) == {"running": 1, "pending": 2}
+
+    class Old:
+        def get_tasks_remaining(self):
+            return 4
+    assert stats._queue(Server(Old())) == {"running": 0, "pending": 4}
+    assert stats._queue(object()) is None
+
+
+def test_collect_stats_without_psutil_or_gpu(monkeypatch):
+    monkeypatch.setattr(stats, "psutil", None)
+    monkeypatch.setattr(stats, "pynvml", None)
+    out = stats.collect_stats(Server(Queue([], [])))
+    assert out["gpus"] == [] and "cpu" not in out and out["queue"] == {"running": 0, "pending": 0}
+    assert stats.collect_stats() == {"gpus": []}

tests/python/test_workspace.py

@@ -131,3 +131,62 @@ def test_rename_user_moves_private_folder(ws):
     ws.rename_user("alice", "alicia")
     assert os.path.exists(os.path.join(ws.root, "users", "alicia", "keep.json"))
     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("bad", ["C:/models", "C:", "..", "a/../..", "x:y"])
+def test_safe_join_refuses_escapes(ws, bad):
+    from rigshare_core.workspace import safe_join
+    with pytest.raises(PermissionError):
+        safe_join(ws.root, bad)
+
+
+def test_folder_ops_cannot_leave_the_workspace(ws, tmp_path):
+    outside = tmp_path / "models"
+    outside.mkdir()
+    (outside / "keep.txt").write_text("x")
+    # A colon segment (drive prefix) is refused outright; a plain absolute path is only ever
+    # read relative to the workflows root (a leading slash is dropped), so it can't reach `outside`.
+    for bad in ("C:" + str(outside)[2:], "shared/C:evil"):
+        with pytest.raises(PermissionError):
+            ws.delete_folder(bad)
+        with pytest.raises(PermissionError):
+            ws.rename_folder(bad, "renamed")
+    for plain in (str(outside), str(outside).replace("\\", "/")):
+        try:
+            ws.delete_folder(plain)
+        except (PermissionError, ValueError):
+            pass
+    assert (outside / "keep.txt").exists()
+
+
+@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)
+
+
+@pytest.mark.skipif(os.name != "nt", reason="case-insensitive filesystem")
+def test_case_variants_cannot_bypass_private_folders(ws):
+    touch(ws, "users/alice/a.json")
+    assert norm("Users/Alice/a.json") == "users/alice/a.json"
+    assert norm("Workflows/USERS/ALICE") == "workflows/users/alice"
+    assert ws.role(norm("USERS/alice/a.json"), "user", "bob", EDITOR) is None
+
+
+def test_name_rules():
+    from rigshare_core.workspace import FOLDER_NAME_RE
+    from rigshare_core.store import USERNAME_RE
+    for bad in ("con", "NUL", "com1.txt", "art.", "art ", "bad\n"):
+        assert not FOLDER_NAME_RE.match(bad), bad
+    for bad in ("..", "bob.", ".bob", "con", "aux", "ab\n", "a"):
+        assert not USERNAME_RE.match(bad), bad
+    for ok in ("Art (v2)", "a", "my-folder"):
+        assert FOLDER_NAME_RE.match(ok), ok
+    for ok in ("bob", "al.ice", "x_y-z", "ab"):
+        assert USERNAME_RE.match(ok), ok