From 0f51c167452aa87a28a1a7a2d595d39426bad08b Mon Sep 17 00:00:00 2001 From: Deepanjan Roy Date: Fri, 30 May 2025 18:36:57 -0400 Subject: [PATCH] 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)