From 88a5a1862eb1850a8ffc03407281bd22296267e6 Mon Sep 17 00:00:00 2001
From: nahcmon <alexgaballaa@gmail.com>
Date: Mon, 8 Jun 2026 17:59:54 +0200
Subject: [PATCH] Fix ValueError in /view and /upload/mask when subfolder is on
 a different drive

os.path.commonpath raises ValueError when comparing paths that don't share
a drive (e.g. on Windows when output_dir is on C: and the resolved subfolder
ends up on D:), so a malicious or malformed `subfolder` query/field crashed
these handlers with an unhandled exception instead of returning 403.

Extract the check into is_path_within_directory(), which treats a
different-drive ValueError as "not within" and returns False, restoring the
intended 403 response.

Fixes #1488
---
 server.py                                     | 18 +++++++-
 .../test_is_path_within_directory.py          | 42 +++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 tests-unit/server_test/test_is_path_within_directory.py

diff --git a/server.py b/server.py
index 268441bd1..ec79b76d2 100644
--- a/server.py
+++ b/server.py
@@ -200,6 +200,20 @@ def create_block_external_middleware():
     return block_external_middleware
 
 
+def is_path_within_directory(path, directory):
+    """Returns True if `path` resolves to a location inside `directory`.
+
+    os.path.commonpath raises ValueError when the two paths don't share a
+    drive (e.g. on Windows when one is on C: and the other on D:). That case
+    simply means `path` cannot be inside `directory`, so it is treated as such
+    rather than letting the exception bubble up as an unhandled error.
+    """
+    try:
+        return os.path.commonpath((os.path.abspath(path), directory)) == directory
+    except ValueError:
+        return False
+
+
 class PromptServer():
     def __init__(self, loop):
         PromptServer.instance = self
@@ -477,7 +491,7 @@ class PromptServer():
 
                 if original_ref.get("subfolder", "") != "":
                     full_output_dir = os.path.join(output_dir, original_ref["subfolder"])
-                    if os.path.commonpath((os.path.abspath(full_output_dir), output_dir)) != output_dir:
+                    if not is_path_within_directory(full_output_dir, output_dir):
                         return web.Response(status=403)
                     output_dir = full_output_dir
 
@@ -534,7 +548,7 @@ class PromptServer():
 
                     if "subfolder" in request.rel_url.query:
                         full_output_dir = os.path.join(output_dir, request.rel_url.query["subfolder"])
-                        if os.path.commonpath((os.path.abspath(full_output_dir), output_dir)) != output_dir:
+                        if not is_path_within_directory(full_output_dir, output_dir):
                             return web.Response(status=403)
                         output_dir = full_output_dir
 
diff --git a/tests-unit/server_test/test_is_path_within_directory.py b/tests-unit/server_test/test_is_path_within_directory.py
new file mode 100644
index 000000000..d674c0056
--- /dev/null
+++ b/tests-unit/server_test/test_is_path_within_directory.py
@@ -0,0 +1,42 @@
+"""Tests for server.is_path_within_directory"""
+
+import os
+from unittest.mock import patch
+
+# Importing `nodes` (a transitive import of `server`) prepends the `comfy/`
+# directory to sys.path, which shadows the top-level `utils` package with
+# `comfy/utils.py`. Importing `utils` first caches the real package so later
+# `server` imports (e.g. app.frontend_management) resolve it correctly.
+import utils  # noqa: F401
+from server import is_path_within_directory
+
+
+def test_subpath_is_within_directory(tmp_path):
+    directory = str(tmp_path)
+    sub = os.path.join(directory, "subfolder")
+    assert is_path_within_directory(sub, directory) is True
+
+
+def test_directory_itself_is_within_directory(tmp_path):
+    directory = str(tmp_path)
+    assert is_path_within_directory(directory, directory) is True
+
+
+def test_sibling_path_is_not_within_directory(tmp_path):
+    directory = os.path.join(str(tmp_path), "output")
+    sibling = os.path.join(str(tmp_path), "other")
+    assert is_path_within_directory(sibling, directory) is False
+
+
+def test_parent_path_is_not_within_directory(tmp_path):
+    directory = os.path.join(str(tmp_path), "output")
+    parent = str(tmp_path)
+    assert is_path_within_directory(parent, directory) is False
+
+
+def test_different_drive_returns_false_instead_of_raising():
+    # On Windows, os.path.commonpath raises ValueError when the paths don't
+    # share a drive (e.g. comparing "C:\\output" with "D:\\secret"). That
+    # should be treated as "not within", not bubble up as an exception.
+    with patch("os.path.commonpath", side_effect=ValueError("Paths don't have the same drive")):
+        assert is_path_within_directory("D:\\secret", "C:\\output") is False