-
Notifications
You must be signed in to change notification settings - Fork 14k
Fix/dynamic path relocation #14769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix/dynamic path relocation #14769
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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'} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -13,11 +13,38 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # --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() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new_path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Validate redirected and entered paths before returning them. Both stored redirects and Proposed guard- if description in redirects and os.path.exists(redirects[description]):
- return redirects[description]
+ redirected_path = redirects.get(description)
+ if redirected_path and os.path.isabs(redirected_path) and os.path.isdir(redirected_path):
+ return redirected_path
...
- new_path= input(f"Please paste the updated absolute path for '{description}': ").strip()
+ new_path = input(f"Please paste the updated absolute path for '{description}': ").strip()
+ if not os.path.isabs(new_path) or not os.path.isdir(new_path):
+ raise FileNotFoundError(f"{description} must be an existing absolute directory: {new_path}")As per coding guidelines, unsupported paths and bad states should fail with clear errors instead of silently producing worse behavior. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 ast-grep (0.44.0)[warning] 35-35: File path is request-/variable-derived; validate and normalize to prevent path traversal. (open-filename-from-request) [warning] 37-37: File path is request-/variable-derived; validate and normalize to prevent path traversal. (open-filename-from-request) 🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base_path = valid_path(raw_path,description = "ComfyUI Core Directory") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| models_dir = os.path.join(base_path, "models") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| models_dir =valid_path( os.path.join(base_path,"models"),description = "Model Directory") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -60,7 +87,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") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Don’t require the default output directory to pre-exist.
Proposed adjustment-output_directory =valid_path( os.path.join(base_path, "output"),"Output Directory")
+output_directory = os.path.join(base_path, "output")
+os.makedirs(output_directory, exist_ok=True)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| temp_directory = os.path.join(base_path, "temp") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| input_directory = os.path.join(base_path, "input") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_directory = os.path.join(base_path, "user") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid prompting from module import.
valid_path()is called whilefolder_paths.pyimports, so a missing base/model/output directory can block indefinitely or raiseEOFErrorin headless startup. Route relocation through an explicit validation/recovery step, or fail with a clear exception instead of callinginput()here.Also applies to: 46-48, 91-91
🤖 Prompt for AI Agents