diff --git a/app/user_manager.py b/app/user_manager.py index e18afb71b..c3c1cd419 100644 --- a/app/user_manager.py +++ b/app/user_manager.py @@ -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] diff --git a/comfy/sd1_clip.py b/comfy/sd1_clip.py index 897186bba..9abcb6bd5 100644 --- a/comfy/sd1_clip.py +++ b/comfy/sd1_clip.py @@ -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'] diff --git a/folder_paths.py b/folder_paths.py index 9c96540e3..87f2b3af6 100644 --- a/folder_paths.py +++ b/folder_paths.py @@ -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) diff --git a/server.py b/server.py index 881da8e66..15c280e06 100644 --- a/server.py +++ b/server.py @@ -398,7 +398,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): @@ -476,7 +476,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 @@ -533,7 +533,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 diff --git a/tests-unit/comfy_test/folder_path_test.py b/tests-unit/comfy_test/folder_path_test.py index 775e15c36..04e492289 100644 --- a/tests-unit/comfy_test/folder_path_test.py +++ b/tests-unit/comfy_test/folder_path_test.py @@ -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