Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions folder_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import logging
from typing import Literal, List
from collections.abc import Collection

import json
from comfy.cli_args import args

supported_pt_extensions: set[str] = {'.ckpt', '.pt', '.pt2', '.bin', '.pth', '.safetensors', '.pkl', '.sft'}
Expand All @@ -13,11 +13,39 @@

# --base-directory - Resets all default paths configured in folder_paths with a new base path
if args.base_directory:
base_path = os.path.abspath(args.base_directory)
raw_path = os.path.abspath(args.base_directory)
else:
base_path = os.path.dirname(os.path.realpath(__file__))
raw_path = os.path.dirname(os.path.realpath(__file__))


def valid_path(saved_path,description = "Instance Folder"):
if os.path.exists(saved_path):
return saved_path

redirect_path = os.path.join(os.path.dirname(__file__),"path.redirects.json")
if os.path.exists(redirect_path):
with open("redirect_path","r") as f:
redirects = json.load(f)
if description in redirects and os.path.exists(redirects[description]):
return redirects[description]
print(f"\n[!] ALERT: {description} not found at: {saved_path}")
new_path= input(f"Please paste the updated absolute path for '{description}': ").strip()
try:
redirects = {}
if os.path.exists(redirects):
with open(redirects, "r") as f: redirects = json.load(f)
redirects[description] = new_path
with open(redirects, "w") as f: json.dump(redirects, f, indent=4)
except Exception:
pass

return new_path
Comment on lines +25 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

The redirect-file plumbing is riddled with wrong variables — it can crash and never persists.

Three concrete bugs in the redirect logic:

  • Line 27 opens the string literal "redirect_path" instead of the redirect_path variable, so whenever the real JSON file exists this raises FileNotFoundError (and it's not inside the try).
  • Lines 35, 36, 38 pass the redirects dict ({}) to os.path.exists/open where the path is expected. os.path.exists({}) raises TypeError, which the bare except Exception: pass silently swallows, so the mapping is never written.

Net effect: the redirect file is never read correctly and never saved.

🐛 Proposed fix
     redirect_path = os.path.join(os.path.dirname(__file__),"path.redirects.json")
     if os.path.exists(redirect_path):
-        with open("redirect_path","r") as f:
+        with open(redirect_path,"r") as f:
             redirects = json.load(f)
             if description in redirects and os.path.exists(redirects[description]):
                 return redirects[description]
     print(f"\n[!] ALERT: {description} not found at: {saved_path}")
     new_path= input(f"Please paste the updated absolute path for '{description}': ").strip()
     try:
         redirects = {}
-        if os.path.exists(redirects):
-            with open(redirects, "r") as f: redirects = json.load(f)
+        if os.path.exists(redirect_path):
+            with open(redirect_path, "r") as f: redirects = json.load(f)
         redirects[description] = new_path
-        with open(redirects, "w") as f: json.dump(redirects, f, indent=4)
+        with open(redirect_path, "w") as f: json.dump(redirects, f, indent=4)
     except Exception:
         pass
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
redirect_path = os.path.join(os.path.dirname(__file__),"path.redirects.json")
if os.path.exists(redirect_path):
with open("redirect_path","r") as f:
redirects = json.load(f)
if description in redirects and os.path.exists(redirects[description]):
return redirects[description]
print(f"\n[!] ALERT: {description} not found at: {saved_path}")
new_path= input(f"Please paste the updated absolute path for '{description}': ").strip()
try:
redirects = {}
if os.path.exists(redirects):
with open(redirects, "r") as f: redirects = json.load(f)
redirects[description] = new_path
with open(redirects, "w") as f: json.dump(redirects, f, indent=4)
except Exception:
pass
return new_path
redirect_path = os.path.join(os.path.dirname(__file__),"path.redirects.json")
if os.path.exists(redirect_path):
with open(redirect_path,"r") as f:
redirects = json.load(f)
if description in redirects and os.path.exists(redirects[description]):
return redirects[description]
print(f"\n[!] ALERT: {description} not found at: {saved_path}")
new_path= input(f"Please paste the updated absolute path for '{description}': ").strip()
try:
redirects = {}
if os.path.exists(redirect_path):
with open(redirect_path, "r") as f: redirects = json.load(f)
redirects[description] = new_path
with open(redirect_path, "w") as f: json.dump(redirects, f, indent=4)
except Exception:
pass
return new_path
🧰 Tools
🪛 ast-grep (0.44.0)

[warning] 35-35: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(redirects, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(open-filename-from-request)


[warning] 37-37: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(redirects, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(open-filename-from-request)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@folder_paths.py` around lines 25 - 42, Fix the redirect persistence logic in
folder_paths.py: the redirect loading/saving path handling is using the wrong
variables and can fail silently. In the redirect branch, use the redirect_path
variable when opening the JSON file instead of the string literal, and in the
persistence block replace the dict-vs-path mixups so os.path.exists and open
operate on the redirect file path, not the redirects dictionary. Keep the
behavior centered around the existing redirect_path, redirects, and new_path
flow, and ensure the redirect mapping is actually read and written successfully.


models_dir = os.path.join(base_path, "models")


base_path = valid_path(raw_path,description = "ComfyUI Core Directory")

models_dir =valid_path( os.path.join(base_path,"model"),description = "Model Directory")
Comment on lines +21 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

valid_path calls input() at module import — this will hang headless/server/CI startup.

valid_path runs at import time for base_path, models_dir, and output_directory. On a fresh install these directories usually don't exist yet, so the code drops into input(...) and blocks on stdin. In a non-interactive context (systemd service, Docker, CI, the unit tests above) there's no TTY, so this either hangs forever or raises EOFError, preventing folder_paths from importing at all. Directory creation/validation shouldn't gate module import behind an interactive prompt.

🧰 Tools
🪛 ast-grep (0.44.0)

[warning] 35-35: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(redirects, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(open-filename-from-request)


[warning] 37-37: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(redirects, "w")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(open-filename-from-request)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@folder_paths.py` around lines 21 - 48, `valid_path` currently prompts with
`input()` during module import, which can block or fail in non-interactive
environments. Refactor `valid_path` and the import-time assignments for
`base_path`/`models_dir` to avoid any stdin prompt at import; instead, return a
fallback or raise a clear non-interactive error, and move path
resolution/creation behind an explicit setup step or guard.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

models_dir now points at base_path/model — this breaks the established layout.

The directory has always been models (plural), and every model subfolder plus the downstream test in tests-unit/comfy_test/folder_path_test.py expects os.path.join(base_path, "models"). Dropping the s will make ComfyUI look for models in a nonexistent directory (and, combined with valid_path, trigger the interactive prompt on startup).

🐛 Proposed fix
-models_dir =valid_path( os.path.join(base_path,"model"),description = "Model Directory")
+models_dir = valid_path(os.path.join(base_path, "models"), description="Model Directory")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
models_dir =valid_path( os.path.join(base_path,"model"),description = "Model Directory")
models_dir = valid_path(os.path.join(base_path, "models"), description="Model Directory")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@folder_paths.py` at line 48, The models directory path in folder_paths.py was
changed to the singular form, which breaks the expected folder layout and
downstream tests. Update the models_dir assignment to use the existing plural
directory name in the path-building logic inside folder_paths.py, keeping it
consistent with the rest of the model subfolder handling and the expectations in
folder path tests.

folder_names_and_paths["checkpoints"] = ([os.path.join(models_dir, "checkpoints")], supported_pt_extensions)
folder_names_and_paths["configs"] = ([os.path.join(models_dir, "configs")], [".yaml"])

Expand Down Expand Up @@ -60,7 +88,7 @@

folder_names_and_paths["detection"] = ([os.path.join(models_dir, "detection")], supported_pt_extensions)

output_directory = os.path.join(base_path, "output")
output_directory =valid_path( os.path.join(base_path, "output"),"Output Directory")
temp_directory = os.path.join(base_path, "temp")
input_directory = os.path.join(base_path, "input")
user_directory = os.path.join(base_path, "user")
Expand Down Expand Up @@ -102,6 +130,8 @@ def __exit__(self, exc_type, exc_value, traceback):
"fbx" : "model",
}



def map_legacy(folder_name: str) -> str:
legacy = {"unet": "diffusion_models",
"clip": "text_encoders"}
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.