fix: handle ValueError from os.path.commonpath on cross-drive paths (Windows)

On Windows, os.path.commonpath raises ValueError when the two paths
reside on different drives (e.g. C:\ vs D:\).  This crashed ComfyUI
with an unhandled exception whenever output/input/upload directories
were on a different drive than the ComfyUI installation.

Add folder_paths.is_path_within_directory() as a safe wrapper that
catches ValueError and returns False (different drives means the path
is clearly not inside the directory).  Replace all bare
os.path.commonpath security checks across server.py, folder_paths.py,
and app/user_manager.py with this helper.

In comfy/sd1_clip.py, narrow the bare except clause to except ValueError
with a descriptive comment.

Fixes #1488
This commit is contained in:
Krishna Chaitanya Balusu 2026-03-24 22:59:32 -04:00
parent 5ebb0c2e0b
commit 40a25de926
5 changed files with 67 additions and 9 deletions

View File

@ -82,7 +82,7 @@ class UserManager():
path = user_root
# prevent leaving /{type}
if os.path.commonpath((root_dir, user_root)) != root_dir:
if not folder_paths.is_path_within_directory(user_root, root_dir):
return None
if file is not None:
@ -92,7 +92,7 @@ class UserManager():
# prevent leaving /{type}/{user}
path = os.path.abspath(os.path.join(user_root, file))
if os.path.commonpath((user_root, path)) != user_root:
if not folder_paths.is_path_within_directory(path, user_root):
return None
parent = os.path.split(path)[0]

View File

@ -425,7 +425,10 @@ def load_embed(embedding_name, embedding_directory, embedding_size, embed_key=No
try:
if os.path.commonpath((embed_dir, embed_path)) != embed_dir:
continue
except:
except ValueError:
# On Windows, commonpath raises ValueError when paths are
# on different drives. The embedding is clearly not inside
# this directory, so skip it.
continue
if not os.path.isfile(embed_path):
extensions = ['.safetensors', '.pt', '.bin']

View File

@ -11,6 +11,30 @@ from comfy.cli_args import args
supported_pt_extensions: set[str] = {'.ckpt', '.pt', '.pt2', '.bin', '.pth', '.safetensors', '.pkl', '.sft'}
def is_path_within_directory(path: str, directory: str) -> bool:
"""Check whether 'path' is located inside 'directory'.
This is a safe replacement for the pattern::
os.path.commonpath((directory, path)) == directory
which raises ``ValueError`` on Windows when the two paths reside on
different drives (e.g. ``C:\\`` vs ``D:\\``). When the drives differ the
path is clearly **not** inside the directory, so we return ``False``
instead of letting the exception propagate.
Both arguments are normalised with ``os.path.abspath`` before comparison.
"""
try:
return os.path.commonpath((os.path.abspath(path), os.path.abspath(directory))) == os.path.abspath(directory)
except ValueError:
# On Windows, commonpath raises ValueError when paths are on
# different drives. Different drives means the path is not
# within the directory.
return False
folder_names_and_paths: dict[str, tuple[list[str], set[str]]] = {}
# --base-directory - Resets all default paths configured in folder_paths with a new base path
@ -455,11 +479,10 @@ def get_save_image_path(filename_prefix: str, output_dir: str, image_width=0, im
full_output_folder = os.path.join(output_dir, subfolder)
if os.path.commonpath((output_dir, os.path.abspath(full_output_folder))) != output_dir:
if not is_path_within_directory(full_output_folder, output_dir):
err = "**** ERROR: Saving image outside the output folder is not allowed." + \
"\n full_output_folder: " + os.path.abspath(full_output_folder) + \
"\n output_dir: " + output_dir + \
"\n commonpath: " + os.path.commonpath((output_dir, os.path.abspath(full_output_folder)))
"\n output_dir: " + output_dir
logging.error(err)
raise Exception(err)

View File

@ -394,7 +394,7 @@ class PromptServer():
full_output_folder = os.path.join(upload_dir, os.path.normpath(subfolder))
filepath = os.path.abspath(os.path.join(full_output_folder, filename))
if os.path.commonpath((upload_dir, filepath)) != upload_dir:
if not folder_paths.is_path_within_directory(filepath, upload_dir):
return web.Response(status=400)
if not os.path.exists(full_output_folder):
@ -472,7 +472,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 folder_paths.is_path_within_directory(full_output_dir, output_dir):
return web.Response(status=403)
output_dir = full_output_dir
@ -529,7 +529,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 folder_paths.is_path_within_directory(full_output_dir, output_dir):
return web.Response(status=403)
output_dir = full_output_dir

View File

@ -160,3 +160,35 @@ def test_base_path_change_clears_old(set_base_dir):
for name in ["controlnet", "diffusion_models", "text_encoders"]:
assert len(folder_paths.get_folder_paths(name)) == 2
class TestIsPathWithinDirectory:
"""Tests for folder_paths.is_path_within_directory."""
def test_path_inside_directory(self, temp_dir):
inner = os.path.join(temp_dir, "sub", "file.txt")
assert folder_paths.is_path_within_directory(inner, temp_dir) is True
def test_path_is_directory_itself(self, temp_dir):
assert folder_paths.is_path_within_directory(temp_dir, temp_dir) is True
def test_path_outside_directory(self, temp_dir):
outside = os.path.join(temp_dir, "..", "other")
assert folder_paths.is_path_within_directory(outside, temp_dir) is False
def test_sibling_directory(self, temp_dir):
sibling = os.path.join(os.path.dirname(temp_dir), "sibling")
assert folder_paths.is_path_within_directory(sibling, temp_dir) is False
def test_parent_not_within_child(self, temp_dir):
parent = os.path.dirname(temp_dir)
assert folder_paths.is_path_within_directory(parent, temp_dir) is False
def test_different_drives_returns_false(self):
"""Simulate the Windows cross-drive scenario (issue #1488).
On non-Windows systems os.path.commonpath won't raise ValueError
for these paths, so we mock it to verify the except-branch.
"""
with patch("folder_paths.os.path.commonpath", side_effect=ValueError("Paths don't have the same drive")):
assert folder_paths.is_path_within_directory("D:\\data\\img.png", "C:\\ComfyUI\\output") is False