From 03aaf1b7b708bfbda30d95c0ca5ad156f0f792ca Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 30 May 2025 12:00:47 -0400 Subject: [PATCH 01/14] Add sandbox behind --enable-sandbox flag --- .gitignore | 1 + comfy/cli_args.py | 7 + folder_paths.py | 9 +- main.py | 31 ++++ requirements.txt | 3 + sandbox/setup_sandbox_permissions.bat | 29 ++++ sandbox/windows_sandbox.py | 163 ++++++++++++++++++++++ tests-unit/comfy_test/folder_path_test.py | 2 +- tests-unit/comfy_test/sandbox_test.py | 36 +++++ 9 files changed, 279 insertions(+), 2 deletions(-) create mode 100644 sandbox/setup_sandbox_permissions.bat create mode 100644 sandbox/windows_sandbox.py create mode 100644 tests-unit/comfy_test/sandbox_test.py diff --git a/.gitignore b/.gitignore index 4e8cea71e..dcb2a56a3 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,4 @@ web_custom_versions/ openapi.yaml filtered-openapi.yaml uv.lock +/write-permitted/ diff --git a/comfy/cli_args.py b/comfy/cli_args.py index 741ecac3f..a3187ac90 100644 --- a/comfy/cli_args.py +++ b/comfy/cli_args.py @@ -208,6 +208,13 @@ database_default_path = os.path.abspath( ) parser.add_argument("--database-url", type=str, default=f"sqlite:///{database_default_path}", help="Specify the database URL, e.g. for an in-memory database you can use 'sqlite:///:memory:'.") +parser.add_argument( + "--enable-sandbox", + default=False, + action="store_true", + help="Enable sandbox mode. (default: False)", +) + if comfy.options.args_parsing: args = parser.parse_args() else: diff --git a/folder_paths.py b/folder_paths.py index f0b3fd103..10d145d3f 100644 --- a/folder_paths.py +++ b/folder_paths.py @@ -47,10 +47,15 @@ folder_names_and_paths["photomaker"] = ([os.path.join(models_dir, "photomaker")] folder_names_and_paths["classifiers"] = ([os.path.join(models_dir, "classifiers")], {""}) output_directory = os.path.join(base_path, "output") -temp_directory = os.path.join(base_path, "temp") input_directory = os.path.join(base_path, "input") user_directory = os.path.join(base_path, "user") +write_permitted_base_dir = os.path.join(base_path, "write-permitted") +# Temp is a subdirectory of write-permitted so the entire directory can be +# deleted and recreated as needed. +temp_directory = os.path.join(write_permitted_base_dir, "temp") + + filename_list_cache: dict[str, tuple[list[str], dict[str, float], float]] = {} class CacheHelper: @@ -130,6 +135,8 @@ def set_user_directory(user_dir: str) -> None: global user_directory user_directory = user_dir +def get_write_permitted_base_directory() -> str: + return write_permitted_base_dir #NOTE: used in http server so don't put folders that should not be accessed remotely def get_directory_by_type(type_name: str) -> str | None: diff --git a/main.py b/main.py index c8c4194d4..0b8a69354 100644 --- a/main.py +++ b/main.py @@ -10,12 +10,15 @@ from app.logger import setup_logger import itertools import utils.extra_config import logging +from sandbox import windows_sandbox import sys +import subprocess if __name__ == "__main__": #NOTE: These do not do anything on core ComfyUI, they are for custom nodes. os.environ['HF_HUB_DISABLE_TELEMETRY'] = '1' os.environ['DO_NOT_TRACK'] = '1' + setup_logger(log_level=args.verbose, use_stdout=args.log_stdout) @@ -54,6 +57,31 @@ def apply_custom_paths(): folder_paths.set_user_directory(user_dir) +def try_enable_sandbox(): + if not args.enable_sandbox: return + + # Sandbox is only supported on Windows + if os.name != 'nt': return + + if any([ + args.output_directory, + args.user_directory, + args.base_directory, + args.temp_directory + ]): + # Note: If we ever support custom directories, we should warn users if + # the directories are in a senstive location (e.g. a high level + # directory like C:\ or the user's home directory). + raise Exception("Sandbox mode is not supported when using --output-directory, " + "--user-directory, --base-directory, or --temp-directory.") + + success = windows_sandbox.try_enable_sandbox() + + if not success: + raise Exception("Unable to run ComfyUI with sandbox enabled. " + "You can rerun without --enable-sandbox.") + + def execute_prestartup_script(): def execute_script(script_path): module_name = os.path.splitext(script_path)[0] @@ -95,6 +123,7 @@ def execute_prestartup_script(): logging.info("") apply_custom_paths() +try_enable_sandbox() # Must run before executing custom node prestartup scripts execute_prestartup_script() @@ -257,6 +286,8 @@ def start_comfyui(asyncio_loop=None): folder_paths.set_temp_directory(temp_dir) cleanup_temp() + os.makedirs(folder_paths.get_temp_directory(), exist_ok=True) + if args.windows_standalone_build: try: import new_updater diff --git a/requirements.txt b/requirements.txt index 94dafea58..c3ea016c9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,3 +28,6 @@ soundfile av>=14.2.0 pydantic~=2.0 pydantic-settings~=2.0 + +# Windows only dependencies: +pywin32 ; sys_platform == "win32" diff --git a/sandbox/setup_sandbox_permissions.bat b/sandbox/setup_sandbox_permissions.bat new file mode 100644 index 000000000..c301a6a30 --- /dev/null +++ b/sandbox/setup_sandbox_permissions.bat @@ -0,0 +1,29 @@ +@echo off + +rem Check if any arguments were provided +if "%~1"=="" ( + echo No folders specified. Please provide folder names as arguments. + echo Usage: %~nx0 folder1 folder2 folder3 ... + exit /b 1 +) + +rem Process each argument as a folder +:process_folders +if "%~1"=="" goto :done + if not exist "%~1" ( + echo Creating directory: %~1 + mkdir "%~1" + ) + echo icacls "%~1" /setintegritylevel "(OI)(CI)Low" + icacls "%~1" /setintegritylevel "(OI)(CI)Low" || goto :errorexit + shift + goto :process_folders + +:done +echo Permissions set up successfully +exit /b 0 + +:errorexit +echo Sandbox permission setup script failed +pause +exit /b 1 \ No newline at end of file diff --git a/sandbox/windows_sandbox.py b/sandbox/windows_sandbox.py new file mode 100644 index 000000000..3fe643b67 --- /dev/null +++ b/sandbox/windows_sandbox.py @@ -0,0 +1,163 @@ +import logging +import win32con +import win32process +import win32security +import subprocess +import os +from win32com.shell import shellcon, shell +import win32api +import win32event +import folder_paths + + +LOW_INTEGRITY_SID_STRING = "S-1-16-4096" + +# Use absolute path to prevent command injection +ICACLS_PATH = r"C:\Windows\System32\icacls.exe" + + +def set_process_integrity_level_to_low(): + # Get current process handle + current_process = win32process.GetCurrentProcess() + + # Open process token + token = win32security.OpenProcessToken( + current_process, + win32con.TOKEN_ALL_ACCESS, + ) + + low_integrity_sid = win32security.ConvertStringSidToSid(LOW_INTEGRITY_SID_STRING) + + # Set the integrity level of the token + win32security.SetTokenInformation( + token, win32security.TokenIntegrityLevel, (low_integrity_sid, 0) + ) + + logging.info("Sandbox enabled: Process now running with low integrity token") + + +def permits_low_integrity_write(icacls_output): + permissions = [l.strip() for l in icacls_output.split("\n")] + LOW_INTEGRITY_LABEL = r"Mandatory Label\Low Mandatory Level" + + for p in permissions: + if LOW_INTEGRITY_LABEL not in p: + continue + + # Check the Low integrity label line - it should be something like + # Mandatory Label\Low Mandatory Level:(OI)(CI)(NW) or + # Mandatory Label\Low Mandatory Level:(I)(OI)(CI)(NW) + + # Note: This is a bit of a crude check with icacls - it is possible for + # a low integrity process to have write access to a directory without + # having these exact ACLs reported by icacls. Implement a more robust + # check if this situation ever occurs. + return all( + [ + # OI: Object Inheritance - all files in the directory with have low + # integrity + "(OI)" in p, + # CI: Container Inheritance - all subdirectories will have low + # integrity + "(CI)" in p, + # NW: No Writeup - processes with lower integrity cannot write to + # this directory + "(NW)" in p, + ] + ) + + +def path_is_low_integrity_writable(path): + """Check if the directory has a writable ACL by low integrity process""" + result = subprocess.run([ICACLS_PATH, path], capture_output=True, text=True) + + if result.returncode != 0: + # icacls command failed. Can happen because path doesn't exist + # or we're not allowed to access acl information of the path. + return False + + return permits_low_integrity_write(result.stdout) + + +def ensure_directories_exist(dirs): + for dir in dirs: + os.makedirs(dir, exist_ok=True) + + +def check_directory_acls(dirs): + acls_correct = True + for dir in dirs: + if not path_is_low_integrity_writable(dir): + logging.info( + f'Directory "{dir}" must be writable by low integrity ' + "processes for sandbox mode." + ) + acls_correct = False + + return acls_correct + + +def setup_permissions(dirs): + script_dir = os.path.dirname(os.path.abspath(__file__)) + bat_path = os.path.join(script_dir, "setup_sandbox_permissions.bat") + + execute_info = { + "lpVerb": "runas", # Run as administrator + "lpFile": bat_path, + "lpParameters": " ".join(dirs), + "nShow": win32con.SW_SHOWNORMAL, + # This flag is necessary to wait for the process to finish. + "fMask": shellcon.SEE_MASK_NOCLOSEPROCESS, + } + + # This is equivalent to right-clicking the bat file and selecting "Run as + # administrator" + proc_info = shell.ShellExecuteEx(**execute_info) + hProcess = proc_info["hProcess"] + + # Setup script should take a few milliseconds. Time out at 10 seconds. + win32event.WaitForSingleObject(hProcess, 10 * 1000) + exit_code = win32process.GetExitCodeProcess(hProcess) + + try: + if exit_code == win32con.STATUS_PENDING: + raise Exception("Sandbox permission script timed out") + if exit_code != 0: + raise Exception( + "Sandbox permission setup script failed. " f"Exit code: {exit_code}" + ) + finally: + win32api.CloseHandle(hProcess) + + +def try_enable_sandbox(): + write_permitted_dirs = [ + folder_paths.get_write_permitted_base_directory(), + folder_paths.get_output_directory(), + folder_paths.get_user_directory(), + ] + + ensure_directories_exist(write_permitted_dirs) + + if check_directory_acls(write_permitted_dirs): + set_process_integrity_level_to_low() + return True + + # Directory permissions are not set up correctly. Try to fix. + logging.critical( + "Some directories do not have the correct permissions for sandbox mode " + "to work. Would you like ComfyUI to fix these permissions? You will " + "receive a UAC elevation prompt. [y/n]" + ) + if input() != "y": + return False + + setup_permissions(write_permitted_dirs) + + # Check directory permissions again before enabling sandbox. + if check_directory_acls(write_permitted_dirs): + set_process_integrity_level_to_low() + return True + + # Directory permissions are still not set up correctly. Give up. + return False diff --git a/tests-unit/comfy_test/folder_path_test.py b/tests-unit/comfy_test/folder_path_test.py index 775e15c36..b93c4093a 100644 --- a/tests-unit/comfy_test/folder_path_test.py +++ b/tests-unit/comfy_test/folder_path_test.py @@ -124,7 +124,7 @@ def test_base_path_changes(set_base_dir): assert folder_paths.models_dir == os.path.join(test_dir, "models") assert folder_paths.input_directory == os.path.join(test_dir, "input") assert folder_paths.output_directory == os.path.join(test_dir, "output") - assert folder_paths.temp_directory == os.path.join(test_dir, "temp") + assert folder_paths.temp_directory == os.path.join(test_dir, "write-permitted", "temp") assert folder_paths.user_directory == os.path.join(test_dir, "user") assert os.path.join(test_dir, "custom_nodes") in folder_paths.get_folder_paths("custom_nodes") diff --git a/tests-unit/comfy_test/sandbox_test.py b/tests-unit/comfy_test/sandbox_test.py new file mode 100644 index 000000000..eb6dd9bc1 --- /dev/null +++ b/tests-unit/comfy_test/sandbox_test.py @@ -0,0 +1,36 @@ +import pytest +from sandbox import windows_sandbox + +def test_icacl_no_low_integrity_label(): + icacl_output = r""" + foo NT AUTHORITY\SYSTEM:(OI)(CI)(F) + """ + assert not windows_sandbox.permits_low_integrity_write(icacl_output) + +def test_icacl_missing_inherit_flags(): + icacl_output = r""" + foo Mandatory Label\Low Mandatory Level:(NW) + """ + assert not windows_sandbox.permits_low_integrity_write(icacl_output) + + icacl_output = r""" + foo Mandatory Label\Low Mandatory Level:(OI)(NW) + """ + assert not windows_sandbox.permits_low_integrity_write(icacl_output) + + icacl_output = r""" + foo Mandatory Label\Low Mandatory Level:(CI)(NW) + """ + assert not windows_sandbox.permits_low_integrity_write(icacl_output) + +def test_icacl_correct_acls(): + icacl_output = r""" + foo Mandatory Label\Low Mandatory Level:(I)(OI)(CI)(NW) + """ + assert windows_sandbox.permits_low_integrity_write(icacl_output) + + icacl_output = r""" + foo Mandatory Label\Low Mandatory Level:(OI)(CI)(NW) + """ + assert windows_sandbox.permits_low_integrity_write(icacl_output) + \ No newline at end of file From 0f51c167452aa87a28a1a7a2d595d39426bad08b Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 30 May 2025 18:36:57 -0400 Subject: [PATCH 02/14] Clean up --- comfy/cli_args.py | 2 +- main.py | 4 --- sandbox/setup_sandbox_permissions.bat | 4 ++- sandbox/windows_sandbox.py | 36 ++++++++++++++++----------- tests-unit/comfy_test/sandbox_test.py | 13 +++++----- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/comfy/cli_args.py b/comfy/cli_args.py index a3187ac90..50c25581a 100644 --- a/comfy/cli_args.py +++ b/comfy/cli_args.py @@ -212,7 +212,7 @@ parser.add_argument( "--enable-sandbox", default=False, action="store_true", - help="Enable sandbox mode. (default: False)", + help="Enable sandbox mode.", ) if comfy.options.args_parsing: diff --git a/main.py b/main.py index 0b8a69354..648e251a5 100644 --- a/main.py +++ b/main.py @@ -12,13 +12,11 @@ import utils.extra_config import logging from sandbox import windows_sandbox import sys -import subprocess if __name__ == "__main__": #NOTE: These do not do anything on core ComfyUI, they are for custom nodes. os.environ['HF_HUB_DISABLE_TELEMETRY'] = '1' os.environ['DO_NOT_TRACK'] = '1' - setup_logger(log_level=args.verbose, use_stdout=args.log_stdout) @@ -286,8 +284,6 @@ def start_comfyui(asyncio_loop=None): folder_paths.set_temp_directory(temp_dir) cleanup_temp() - os.makedirs(folder_paths.get_temp_directory(), exist_ok=True) - if args.windows_standalone_build: try: import new_updater diff --git a/sandbox/setup_sandbox_permissions.bat b/sandbox/setup_sandbox_permissions.bat index c301a6a30..0ff64e685 100644 --- a/sandbox/setup_sandbox_permissions.bat +++ b/sandbox/setup_sandbox_permissions.bat @@ -25,5 +25,7 @@ exit /b 0 :errorexit echo Sandbox permission setup script failed +rem Wait for a key to be pressed if unsuccessful so user can read the error +rem before the command window closes. pause -exit /b 1 \ No newline at end of file +exit /b 1 diff --git a/sandbox/windows_sandbox.py b/sandbox/windows_sandbox.py index 3fe643b67..bcb2cece5 100644 --- a/sandbox/windows_sandbox.py +++ b/sandbox/windows_sandbox.py @@ -17,26 +17,32 @@ ICACLS_PATH = r"C:\Windows\System32\icacls.exe" def set_process_integrity_level_to_low(): - # Get current process handle current_process = win32process.GetCurrentProcess() - - # Open process token token = win32security.OpenProcessToken( current_process, win32con.TOKEN_ALL_ACCESS, ) low_integrity_sid = win32security.ConvertStringSidToSid(LOW_INTEGRITY_SID_STRING) - - # Set the integrity level of the token win32security.SetTokenInformation( token, win32security.TokenIntegrityLevel, (low_integrity_sid, 0) ) logging.info("Sandbox enabled: Process now running with low integrity token") + win32security.CloseHandle(token) -def permits_low_integrity_write(icacls_output): + +def does_permit_low_integrity_write(icacls_output): + """ + Checks if an icacls output indicates that the path is writable by low + integrity processes. + + Note that currently it is a bit of a crude check - it is possible for + a low integrity process to have write access to a directory without + having these exact ACLs reported by icacls. Implement a more robust + check if this situation ever occurs. + """ permissions = [l.strip() for l in icacls_output.split("\n")] LOW_INTEGRITY_LABEL = r"Mandatory Label\Low Mandatory Level" @@ -47,11 +53,6 @@ def permits_low_integrity_write(icacls_output): # Check the Low integrity label line - it should be something like # Mandatory Label\Low Mandatory Level:(OI)(CI)(NW) or # Mandatory Label\Low Mandatory Level:(I)(OI)(CI)(NW) - - # Note: This is a bit of a crude check with icacls - it is possible for - # a low integrity process to have write access to a directory without - # having these exact ACLs reported by icacls. Implement a more robust - # check if this situation ever occurs. return all( [ # OI: Object Inheritance - all files in the directory with have low @@ -68,7 +69,7 @@ def permits_low_integrity_write(icacls_output): def path_is_low_integrity_writable(path): - """Check if the directory has a writable ACL by low integrity process""" + """Check if the path has a writable ACL by low integrity process""" result = subprocess.run([ICACLS_PATH, path], capture_output=True, text=True) if result.returncode != 0: @@ -76,7 +77,7 @@ def path_is_low_integrity_writable(path): # or we're not allowed to access acl information of the path. return False - return permits_low_integrity_write(result.stdout) + return does_permit_low_integrity_write(result.stdout) def ensure_directories_exist(dirs): @@ -98,6 +99,13 @@ def check_directory_acls(dirs): def setup_permissions(dirs): + """ + Sets the correct low integrity write permissions for the given directories + using an UAC elevation prompt. We need admin elevation because if the Comfy + directory is not under the user's profile directory (e.g. any location in a + non-C: drive), the regular user does not have permission to set the + integrity level ACLs. + """ script_dir = os.path.dirname(os.path.abspath(__file__)) bat_path = os.path.join(script_dir, "setup_sandbox_permissions.bat") @@ -115,7 +123,7 @@ def setup_permissions(dirs): proc_info = shell.ShellExecuteEx(**execute_info) hProcess = proc_info["hProcess"] - # Setup script should take a few milliseconds. Time out at 10 seconds. + # Setup script should less than a second. Time out at 10 seconds. win32event.WaitForSingleObject(hProcess, 10 * 1000) exit_code = win32process.GetExitCodeProcess(hProcess) diff --git a/tests-unit/comfy_test/sandbox_test.py b/tests-unit/comfy_test/sandbox_test.py index eb6dd9bc1..7c26c80af 100644 --- a/tests-unit/comfy_test/sandbox_test.py +++ b/tests-unit/comfy_test/sandbox_test.py @@ -5,32 +5,31 @@ def test_icacl_no_low_integrity_label(): icacl_output = r""" foo NT AUTHORITY\SYSTEM:(OI)(CI)(F) """ - assert not windows_sandbox.permits_low_integrity_write(icacl_output) + assert not windows_sandbox.does_permit_low_integrity_write(icacl_output) def test_icacl_missing_inherit_flags(): icacl_output = r""" foo Mandatory Label\Low Mandatory Level:(NW) """ - assert not windows_sandbox.permits_low_integrity_write(icacl_output) + assert not windows_sandbox.does_permit_low_integrity_write(icacl_output) icacl_output = r""" foo Mandatory Label\Low Mandatory Level:(OI)(NW) """ - assert not windows_sandbox.permits_low_integrity_write(icacl_output) + assert not windows_sandbox.does_permit_low_integrity_write(icacl_output) icacl_output = r""" foo Mandatory Label\Low Mandatory Level:(CI)(NW) """ - assert not windows_sandbox.permits_low_integrity_write(icacl_output) + assert not windows_sandbox.does_permit_low_integrity_write(icacl_output) def test_icacl_correct_acls(): icacl_output = r""" foo Mandatory Label\Low Mandatory Level:(I)(OI)(CI)(NW) """ - assert windows_sandbox.permits_low_integrity_write(icacl_output) + assert windows_sandbox.does_permit_low_integrity_write(icacl_output) icacl_output = r""" foo Mandatory Label\Low Mandatory Level:(OI)(CI)(NW) """ - assert windows_sandbox.permits_low_integrity_write(icacl_output) - \ No newline at end of file + assert windows_sandbox.does_permit_low_integrity_write(icacl_output) From 15c8efa153ef27b9f3cc5db0d21846a9d88426f3 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 30 May 2025 19:01:59 -0400 Subject: [PATCH 03/14] Use correct CloseHandle --- sandbox/windows_sandbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sandbox/windows_sandbox.py b/sandbox/windows_sandbox.py index bcb2cece5..be87bb92e 100644 --- a/sandbox/windows_sandbox.py +++ b/sandbox/windows_sandbox.py @@ -30,7 +30,7 @@ def set_process_integrity_level_to_low(): logging.info("Sandbox enabled: Process now running with low integrity token") - win32security.CloseHandle(token) + win32api.CloseHandle(token) def does_permit_low_integrity_write(icacls_output): From 46af9a9ac24e97badfdbdc9c08d95dd2fa8158ec Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 30 May 2025 19:19:12 -0400 Subject: [PATCH 04/14] Guard pywin32 imports --- main.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/main.py b/main.py index 648e251a5..f123d01e0 100644 --- a/main.py +++ b/main.py @@ -10,7 +10,6 @@ from app.logger import setup_logger import itertools import utils.extra_config import logging -from sandbox import windows_sandbox import sys if __name__ == "__main__": @@ -56,11 +55,6 @@ def apply_custom_paths(): def try_enable_sandbox(): - if not args.enable_sandbox: return - - # Sandbox is only supported on Windows - if os.name != 'nt': return - if any([ args.output_directory, args.user_directory, @@ -121,7 +115,14 @@ def execute_prestartup_script(): logging.info("") apply_custom_paths() -try_enable_sandbox() # Must run before executing custom node prestartup scripts + +if os.name == "nt" and args.enable_sandbox: + # We do not have pywin32 on non-windows platforms, so this import needs to + # be guarded. + from sandbox import windows_sandbox + # Must run before executing custom node prestartup scripts. + try_enable_sandbox() + execute_prestartup_script() From d5121f1294227bc669ad7310d1c06945d0e32f50 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 30 May 2025 19:25:55 -0400 Subject: [PATCH 05/14] Make custom_nodes write-permitted --- sandbox/windows_sandbox.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sandbox/windows_sandbox.py b/sandbox/windows_sandbox.py index be87bb92e..a5989cd40 100644 --- a/sandbox/windows_sandbox.py +++ b/sandbox/windows_sandbox.py @@ -144,6 +144,7 @@ def try_enable_sandbox(): folder_paths.get_output_directory(), folder_paths.get_user_directory(), ] + write_permitted_dirs.extend(folder_paths.get_folder_paths("custom_nodes")) ensure_directories_exist(write_permitted_dirs) From 8207d0027e57420ca5bb3a27c426ca8532522ec8 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 19:28:58 -0700 Subject: [PATCH 06/14] Move windows only requirements out to separate file --- .ci/update_windows/update.py | 11 ++++++++++- .ci/windows_base_files/run_nvidia_gpu_sandboxed.bat | 3 +++ requirements.txt | 3 --- win_only_requirements.txt | 2 ++ 4 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 .ci/windows_base_files/run_nvidia_gpu_sandboxed.bat create mode 100644 win_only_requirements.txt diff --git a/.ci/update_windows/update.py b/.ci/update_windows/update.py index 51a263203..62d7019b8 100755 --- a/.ci/update_windows/update.py +++ b/.ci/update_windows/update.py @@ -113,7 +113,8 @@ cur_path = os.path.dirname(update_py_path) req_path = os.path.join(cur_path, "current_requirements.txt") repo_req_path = os.path.join(repo_path, "requirements.txt") - +win_only_req_path = os.path.join(cur_path, "current_win_only_requirements.txt") +win_only_repo_req_path = os.path.join(repo_path, "win_only_requirements.txt") def files_equal(file1, file2): try: @@ -140,6 +141,14 @@ if not os.path.exists(req_path) or not files_equal(repo_req_path, req_path): except: pass +# TODO: Delete this once ComfyUI manager fully supports '; sys_platform == "win32"' syntax +if not os.path.exists(win_only_req_path) or not files_equal(win_only_repo_req_path, win_only_req_path): + import subprocess + try: + subprocess.check_call([sys.executable, '-s', '-m', 'pip', 'install', '-r', win_only_repo_req_path]) + shutil.copy(win_only_repo_req_path, win_only_req_path) + except: + pass stable_update_script = os.path.join(repo_path, ".ci/update_windows/update_comfyui_stable.bat") stable_update_script_to = os.path.join(cur_path, "update_comfyui_stable.bat") diff --git a/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat b/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat new file mode 100644 index 000000000..a61f4a204 --- /dev/null +++ b/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat @@ -0,0 +1,3 @@ + +.\python_embeded\python.exe -s ComfyUI\main.py --windows-standalone-build --enable-sandbox +pause diff --git a/requirements.txt b/requirements.txt index c3ea016c9..94dafea58 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,6 +28,3 @@ soundfile av>=14.2.0 pydantic~=2.0 pydantic-settings~=2.0 - -# Windows only dependencies: -pywin32 ; sys_platform == "win32" diff --git a/win_only_requirements.txt b/win_only_requirements.txt new file mode 100644 index 000000000..883d7776f --- /dev/null +++ b/win_only_requirements.txt @@ -0,0 +1,2 @@ +# Needed to implement the windows sandbox +pywin32 ; sys_platform == "win32" From 96798c03c9a1492e6ecce6192e33f4baf396e9a2 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 19:46:13 -0700 Subject: [PATCH 07/14] Temporary updater changes for testing --- .ci/update_windows/update.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/.ci/update_windows/update.py b/.ci/update_windows/update.py index 62d7019b8..3579ec3a3 100755 --- a/.ci/update_windows/update.py +++ b/.ci/update_windows/update.py @@ -5,11 +5,11 @@ import os import shutil import filecmp -def pull(repo, remote_name='origin', branch='master'): +def pull(repo, remote_name='dproy', branch='in-process-sandbox'): for remote in repo.remotes: if remote.name == remote_name: remote.fetch() - remote_master_id = repo.lookup_reference('refs/remotes/origin/%s' % (branch)).target + remote_master_id = repo.lookup_reference('refs/remotes/dproy/%s' % (branch)).target merge_result, _ = repo.merge_analysis(remote_master_id) # Up to date, do nothing if merge_result & pygit2.GIT_MERGE_ANALYSIS_UP_TO_DATE: @@ -60,8 +60,8 @@ try: except: pass -print("checking out master branch") # noqa: T201 -branch = repo.lookup_branch('master') +print("checking out in-process-sandbox branch") # noqa: T201 +branch = repo.lookup_branch('in-process-sandbox') if branch is None: try: ref = repo.lookup_reference('refs/remotes/origin/master') @@ -72,6 +72,8 @@ if branch is None: repo.checkout(ref) branch = repo.lookup_branch('master') if branch is None: + print("AAAAAAAAAAAAAAAAA") + repo.create_branch('master', repo.get(ref.target)) else: ref = repo.lookup_reference(branch.name) @@ -113,8 +115,7 @@ cur_path = os.path.dirname(update_py_path) req_path = os.path.join(cur_path, "current_requirements.txt") repo_req_path = os.path.join(repo_path, "requirements.txt") -win_only_req_path = os.path.join(cur_path, "current_win_only_requirements.txt") -win_only_repo_req_path = os.path.join(repo_path, "win_only_requirements.txt") + def files_equal(file1, file2): try: @@ -141,14 +142,6 @@ if not os.path.exists(req_path) or not files_equal(repo_req_path, req_path): except: pass -# TODO: Delete this once ComfyUI manager fully supports '; sys_platform == "win32"' syntax -if not os.path.exists(win_only_req_path) or not files_equal(win_only_repo_req_path, win_only_req_path): - import subprocess - try: - subprocess.check_call([sys.executable, '-s', '-m', 'pip', 'install', '-r', win_only_repo_req_path]) - shutil.copy(win_only_repo_req_path, win_only_req_path) - except: - pass stable_update_script = os.path.join(repo_path, ".ci/update_windows/update_comfyui_stable.bat") stable_update_script_to = os.path.join(cur_path, "update_comfyui_stable.bat") @@ -156,5 +149,6 @@ stable_update_script_to = os.path.join(cur_path, "update_comfyui_stable.bat") try: if not file_size(stable_update_script_to) > 10: shutil.copy(stable_update_script, stable_update_script_to) + pass except: pass From 5692036884887ff3b610852b8fd3f9895950eda9 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 20:12:22 -0700 Subject: [PATCH 08/14] Copy windows base files during update --- .ci/update_windows/update.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/.ci/update_windows/update.py b/.ci/update_windows/update.py index 3579ec3a3..3e50edbb1 100755 --- a/.ci/update_windows/update.py +++ b/.ci/update_windows/update.py @@ -142,6 +142,16 @@ if not os.path.exists(req_path) or not files_equal(repo_req_path, req_path): except: pass +# TODO: Delete this once ComfyUI manager fully supports '; sys_platform == "win32"' syntax +win_only_req_path = os.path.join(cur_path, "current_win_only_requirements.txt") +win_only_repo_req_path = os.path.join(repo_path, "win_only_requirements.txt") +if not os.path.exists(win_only_req_path) or not files_equal(win_only_repo_req_path, win_only_req_path): + import subprocess + try: + subprocess.check_call([sys.executable, '-s', '-m', 'pip', 'install', '-r', win_only_repo_req_path]) + shutil.copy(win_only_repo_req_path, win_only_req_path) + except: + pass stable_update_script = os.path.join(repo_path, ".ci/update_windows/update_comfyui_stable.bat") stable_update_script_to = os.path.join(cur_path, "update_comfyui_stable.bat") @@ -152,3 +162,23 @@ try: pass except: pass + +# Write this in python +# cp -r $repo_path/.ci/windows_base_files/* ../ + +# Get the parent directory of cur_path +base_dir = os.path.dirname(cur_path) + +# Source directory containing windows base files +repo_base_dir = os.path.join(repo_path, ".ci/windows_base_files") + +# Copy all files from source to parent directory +if os.path.exists(repo_base_dir): + for item in os.listdir(repo_base_dir): + if item.startswith("README"): continue + source_item = os.path.join(repo_base_dir, item) + dest_item = os.path.join(base_dir, item) + if os.path.isdir(source_item): + shutil.copytree(source_item, dest_item, dirs_exist_ok=True) + else: + shutil.copy2(source_item, dest_item) From 7c6b045d455d3569cf02e65904dde2820be0656b Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 20:45:45 -0700 Subject: [PATCH 09/14] Clean up update script --- .ci/update_windows/update.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/.ci/update_windows/update.py b/.ci/update_windows/update.py index 3e50edbb1..fb7781246 100755 --- a/.ci/update_windows/update.py +++ b/.ci/update_windows/update.py @@ -5,11 +5,11 @@ import os import shutil import filecmp -def pull(repo, remote_name='dproy', branch='in-process-sandbox'): +def pull(repo, remote_name='origin', branch='master'): for remote in repo.remotes: if remote.name == remote_name: remote.fetch() - remote_master_id = repo.lookup_reference('refs/remotes/dproy/%s' % (branch)).target + remote_master_id = repo.lookup_reference('refs/remotes/origin/%s' % (branch)).target merge_result, _ = repo.merge_analysis(remote_master_id) # Up to date, do nothing if merge_result & pygit2.GIT_MERGE_ANALYSIS_UP_TO_DATE: @@ -60,8 +60,8 @@ try: except: pass -print("checking out in-process-sandbox branch") # noqa: T201 -branch = repo.lookup_branch('in-process-sandbox') +print("checking out master branch") # noqa: T201 +branch = repo.lookup_branch('master') if branch is None: try: ref = repo.lookup_reference('refs/remotes/origin/master') @@ -72,8 +72,6 @@ if branch is None: repo.checkout(ref) branch = repo.lookup_branch('master') if branch is None: - print("AAAAAAAAAAAAAAAAA") - repo.create_branch('master', repo.get(ref.target)) else: ref = repo.lookup_reference(branch.name) @@ -159,20 +157,13 @@ stable_update_script_to = os.path.join(cur_path, "update_comfyui_stable.bat") try: if not file_size(stable_update_script_to) > 10: shutil.copy(stable_update_script, stable_update_script_to) - pass except: pass -# Write this in python -# cp -r $repo_path/.ci/windows_base_files/* ../ -# Get the parent directory of cur_path +print("Copying windows base files") base_dir = os.path.dirname(cur_path) - -# Source directory containing windows base files repo_base_dir = os.path.join(repo_path, ".ci/windows_base_files") - -# Copy all files from source to parent directory if os.path.exists(repo_base_dir): for item in os.listdir(repo_base_dir): if item.startswith("README"): continue From f6547ee737514e9c091125abfcc7f6df4dae211b Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 20:48:01 -0700 Subject: [PATCH 10/14] Remove unused pytest --- tests-unit/comfy_test/sandbox_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests-unit/comfy_test/sandbox_test.py b/tests-unit/comfy_test/sandbox_test.py index 7c26c80af..c883d9ff4 100644 --- a/tests-unit/comfy_test/sandbox_test.py +++ b/tests-unit/comfy_test/sandbox_test.py @@ -1,4 +1,3 @@ -import pytest from sandbox import windows_sandbox def test_icacl_no_low_integrity_label(): From ea53b74755cf2642bd9cef88e6b7d8d43956ced2 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 20:56:27 -0700 Subject: [PATCH 11/14] Remove unused line --- .ci/windows_base_files/run_nvidia_gpu_sandboxed.bat | 1 - 1 file changed, 1 deletion(-) diff --git a/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat b/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat index a61f4a204..e6da29cf5 100644 --- a/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat +++ b/.ci/windows_base_files/run_nvidia_gpu_sandboxed.bat @@ -1,3 +1,2 @@ - .\python_embeded\python.exe -s ComfyUI\main.py --windows-standalone-build --enable-sandbox pause From fcd53c418485f9b2d90459d975a98e039d242531 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Tue, 10 Jun 2025 21:10:36 -0700 Subject: [PATCH 12/14] Warn about no sandbox on non-windows --- main.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/main.py b/main.py index f123d01e0..b5cd543ad 100644 --- a/main.py +++ b/main.py @@ -116,12 +116,15 @@ def execute_prestartup_script(): apply_custom_paths() -if os.name == "nt" and args.enable_sandbox: - # We do not have pywin32 on non-windows platforms, so this import needs to - # be guarded. - from sandbox import windows_sandbox - # Must run before executing custom node prestartup scripts. - try_enable_sandbox() +if args.enable_sandbox: + if os.name == "nt": + # windows_sandbox imports the pywin32 module, which is not available on + # non-windows platforms, so this import needs to be guarded. + from sandbox import windows_sandbox + try_enable_sandbox() + else: + logging.warning("Sandbox mode is not supported on non-windows platforms." + "ComfyUI will run without sandbox.") execute_prestartup_script() From fc47bd23abb2aa8d43a027036b27efff49417893 Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 13 Jun 2025 17:28:43 -0400 Subject: [PATCH 13/14] Remove bat updating code --- .ci/update_windows/update.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/.ci/update_windows/update.py b/.ci/update_windows/update.py index fb7781246..b60d07934 100755 --- a/.ci/update_windows/update.py +++ b/.ci/update_windows/update.py @@ -4,6 +4,7 @@ import sys import os import shutil import filecmp +import subprocess def pull(repo, remote_name='origin', branch='master'): for remote in repo.remotes: @@ -133,7 +134,6 @@ if self_update and not files_equal(update_py_path, repo_update_py_path) and file exit() if not os.path.exists(req_path) or not files_equal(repo_req_path, req_path): - import subprocess try: subprocess.check_call([sys.executable, '-s', '-m', 'pip', 'install', '-r', repo_req_path]) shutil.copy(repo_req_path, req_path) @@ -144,7 +144,6 @@ if not os.path.exists(req_path) or not files_equal(repo_req_path, req_path): win_only_req_path = os.path.join(cur_path, "current_win_only_requirements.txt") win_only_repo_req_path = os.path.join(repo_path, "win_only_requirements.txt") if not os.path.exists(win_only_req_path) or not files_equal(win_only_repo_req_path, win_only_req_path): - import subprocess try: subprocess.check_call([sys.executable, '-s', '-m', 'pip', 'install', '-r', win_only_repo_req_path]) shutil.copy(win_only_repo_req_path, win_only_req_path) @@ -159,17 +158,3 @@ try: shutil.copy(stable_update_script, stable_update_script_to) except: pass - - -print("Copying windows base files") -base_dir = os.path.dirname(cur_path) -repo_base_dir = os.path.join(repo_path, ".ci/windows_base_files") -if os.path.exists(repo_base_dir): - for item in os.listdir(repo_base_dir): - if item.startswith("README"): continue - source_item = os.path.join(repo_base_dir, item) - dest_item = os.path.join(base_dir, item) - if os.path.isdir(source_item): - shutil.copytree(source_item, dest_item, dirs_exist_ok=True) - else: - shutil.copy2(source_item, dest_item) From 67e3beecd46ac737ef2c233adf8a2e24a6942fbd Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 13 Jun 2025 17:35:14 -0400 Subject: [PATCH 14/14] Remove blank line (and rebase) --- folder_paths.py | 1 - 1 file changed, 1 deletion(-) diff --git a/folder_paths.py b/folder_paths.py index 10d145d3f..bb46902f0 100644 --- a/folder_paths.py +++ b/folder_paths.py @@ -55,7 +55,6 @@ write_permitted_base_dir = os.path.join(base_path, "write-permitted") # deleted and recreated as needed. temp_directory = os.path.join(write_permitted_base_dir, "temp") - filename_list_cache: dict[str, tuple[list[str], dict[str, float], float]] = {} class CacheHelper: