From ee604c239fb99dcd65bc5a348bed0ff4bf57ad9a Mon Sep 17 00:00:00 2001 From: vveerrgg Date: Tue, 10 Nov 2026 14:31:22 -0700 Subject: [PATCH] 0.3.1 housekeeping: install-path always stops its ComfyUI; Node-24 actions; SECURITY 0.3.x - scripts/install_path_check.py: ComfyUI is started inside the try, stopped by its own Popen handle in the finally (terminate, kill after the grace), and SIGTERM/SIGHUP raise so a cancelled or timed-out job still runs the finally. tests/test_install_path_stop.py. - workflows: actions/checkout v7.0.1, setup-python v7.1.0, cache(/restore,/save) v6.1.0, upload-artifact v7.0.2, all node24, pinned by SHA; ubuntu-latest -> ubuntu-24.04. - SECURITY.md: the latest 0.3.x gets fixes. --- .github/workflows/ci.yml | 6 +-- .github/workflows/install-path.yml | 12 ++--- .github/workflows/publish.yml | 4 +- SECURITY.md | 2 +- scripts/install_path_check.py | 37 +++++++++++-- tests/test_install_path_stop.py | 85 ++++++++++++++++++++++++++++++ 6 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 tests/test_install_path_stop.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6c6a06..c3e8d2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,14 +7,14 @@ permissions: contents: read jobs: test: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: deps: [floating, constraints] # floating = the pyproject ranges; constraints = the hosted build's pins steps: - - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 with: python-version: "3.12" - name: System deps diff --git a/.github/workflows/install-path.yml b/.github/workflows/install-path.yml index a851a41..947858a 100644 --- a/.github/workflows/install-path.yml +++ b/.github/workflows/install-path.yml @@ -26,13 +26,13 @@ env: DEMO_FALLBACK_SHA: 7981fef8f453b0bf29e47b3ffc00b3adcba1d22d8dbe259504f66c2beb00de17 jobs: install-path: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 timeout-minutes: 150 steps: - - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: path: pack - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 with: python-version: "3.12" - name: System deps (what the README asks a user for) @@ -53,7 +53,7 @@ jobs: if venv/bin/python -c "import phonemizer" 2>/dev/null; then echo "::error::phonemizer is installed; the test needs a venv without it"; exit 1; fi - name: Recogniser cache (fetched from the Hub once per revision, then served to the same client path) id: hfcache - uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: hf-mirror key: lsdub-recogniser-${{ env.RECOGNISER_REVISION }} @@ -66,14 +66,14 @@ jobs: --demo-fallback demo-fallback.zip --demo-fallback-sha "$DEMO_FALLBACK_SHA" - name: "Save the recogniser cache (also when another step failed; the files are checked by sha256 either way)" if: always() && steps.hfcache.outputs.cache-hit != 'true' && hashFiles('hf-mirror/pytorch_model.bin') != '' - uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: hf-mirror key: lsdub-recogniser-${{ env.RECOGNISER_REVISION }} - name: ComfyUI log if: always() run: tail -n 300 comfy/install-path-comfyui.log || true - - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 if: failure() with: name: install-path-log diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 435fd40..c974778 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -10,10 +10,10 @@ jobs: uses: ./.github/workflows/install-path.yml publish: needs: install-path - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 if: github.repository == 'HumanjavaEnterprises/comfyui-lsdub' steps: - - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Release notes from the top released CHANGELOG section (checked against pyproject) run: | python3 scripts/release_notes.py --out "$RUNNER_TEMP/notes.md" diff --git a/SECURITY.md b/SECURITY.md index 9411c8b..a473dcd 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,7 +1,7 @@ # Security ## Supported versions -The latest 0.2.x release gets security fixes. Older versions do not; please update. +The latest 0.3.x release gets security fixes. Older versions do not; please update. ## How to report Report security problems privately through GitHub's diff --git a/scripts/install_path_check.py b/scripts/install_path_check.py index 435400a..22127dd 100644 --- a/scripts/install_path_check.py +++ b/scripts/install_path_check.py @@ -28,6 +28,7 @@ import os import re import shutil +import signal import subprocess import sys import threading @@ -234,6 +235,31 @@ def check_dub(comfy, name, outputs, label): say(f'{label}: {len(renders)} render file(s), cut {os.path.relpath(cuts[0], comfy)}; Verify: {rep.splitlines()[0][:120] if rep else ""}') +def stop_comfyui(proc, grace=30): + """stop the ComfyUI this script started, and only that one: its own Popen handle (its PID), never a match by name. + Safe to call when it never started (None) or has already exited.""" + if proc is None or proc.poll() is not None: return + proc.terminate() + try: proc.wait(grace) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + + +class Stopped(SystemExit): + def __init__(self, sig): super().__init__(f'INSTALL-PATH STOPPED: signal {sig}') + + +def stop_on_signals(): + """SIGTERM/SIGHUP (a cancelled or timed-out CI job, a closed terminal) raise instead of killing this process outright, so + the finally that stops ComfyUI still runs. Ctrl-C (SIGINT) already raises KeyboardInterrupt. Returns the old handlers.""" + def raise_(sig, _frame): raise Stopped(sig) + old = {} + for sig in (signal.SIGTERM, getattr(signal, 'SIGHUP', None)): + if sig is not None: old[sig] = signal.signal(sig, raise_) + return old + + def main(): ap = argparse.ArgumentParser(description=__doc__.split('\n')[0]) ap.add_argument('--comfyui', required=True) @@ -276,9 +302,11 @@ def main(): say('recogniser files from the Hugging Face Hub') log = os.path.join(comfy, 'install-path-comfyui.log') lf = open(log, 'w') - proc = subprocess.Popen([sys.executable, 'main.py', '--cpu', '--listen', '127.0.0.1', '--port', str(a.port), '--disable-auto-launch'], - cwd=comfy, env=env, stdout=lf, stderr=subprocess.STDOUT) + proc = None + old = stop_on_signals() try: + proc = subprocess.Popen([sys.executable, 'main.py', '--cpu', '--listen', '127.0.0.1', '--port', str(a.port), '--disable-auto-launch'], + cwd=comfy, env=env, stdout=lf, stderr=subprocess.STDOUT) t0 = time.time() while True: if proc.poll() is not None: raise Fail(f'ComfyUI exited ({proc.returncode}); see {log}') @@ -337,10 +365,9 @@ def dub(name, label, kw): if failures: raise Fail(f'{len(failures)} step(s) failed:\n ' + '\n '.join(failures)) say('PASS') finally: - proc.terminate() - try: proc.wait(30) - except subprocess.TimeoutExpired: proc.kill() + stop_comfyui(proc) lf.close() + for sig, h in old.items(): signal.signal(sig, h) if __name__ == '__main__': diff --git a/tests/test_install_path_stop.py b/tests/test_install_path_stop.py new file mode 100644 index 0000000..bfae816 --- /dev/null +++ b/tests/test_install_path_stop.py @@ -0,0 +1,85 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 Humanjava Enterprises. LSDub: https://github.com/HumanjavaEnterprises/comfyui-lsdub +"""scripts/install_path_check.py always stops the ComfyUI it started (by its own Popen handle), also when a step fails +early or the job is cancelled with SIGTERM.""" + +import importlib.util +import os +import signal +import subprocess +import sys +import textwrap +import time + +import pytest + +SCRIPT = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts", "install_path_check.py") +spec = importlib.util.spec_from_file_location("install_path_check", SCRIPT) +ipc = importlib.util.module_from_spec(spec) +spec.loader.exec_module(ipc) + +SLEEPER = [sys.executable, "-c", "import time; time.sleep(600)"] + + +def alive(pid): + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + return True + + +def test_stop_none_and_exited_are_noops(): + ipc.stop_comfyui(None) + p = subprocess.Popen([sys.executable, "-c", "pass"]) + p.wait() + ipc.stop_comfyui(p) + + +def test_stop_terminates_the_child(): + p = subprocess.Popen(SLEEPER) + ipc.stop_comfyui(p, grace=10) + assert p.returncode is not None + + +def test_stop_kills_a_child_that_ignores_sigterm(): + p = subprocess.Popen( + [ + sys.executable, + "-c", + 'import signal, time, sys; signal.signal(signal.SIGTERM, signal.SIG_IGN); print("ready", flush=True); time.sleep(600)', + ], + stdout=subprocess.PIPE, + text=True, + ) + assert p.stdout.readline().strip() == "ready" + ipc.stop_comfyui(p, grace=1) + assert p.returncode == -signal.SIGKILL + + +@pytest.mark.skipif(not hasattr(signal, "SIGTERM") or os.name != "posix", reason="posix signals") +def test_sigterm_still_stops_the_child(): + """the same try/finally main() uses: SIGTERM to the script raises, the finally stops the child it started.""" + code = textwrap.dedent(f""" + import importlib.util, subprocess, sys, time + spec = importlib.util.spec_from_file_location('ipc', {SCRIPT!r}) + ipc = importlib.util.module_from_spec(spec); spec.loader.exec_module(ipc) + proc = None + old = ipc.stop_on_signals() + try: + proc = subprocess.Popen({SLEEPER!r}) + print(proc.pid, flush=True) + time.sleep(600) + finally: + ipc.stop_comfyui(proc, grace=10) + """) + runner = subprocess.Popen([sys.executable, "-c", code], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + child = int(runner.stdout.readline()) + assert alive(child) + runner.send_signal(signal.SIGTERM) + _, err = runner.communicate(timeout=30) + assert "INSTALL-PATH STOPPED" in err + t0 = time.time() + while alive(child) and time.time() - t0 < 10: + time.sleep(0.1) + assert not alive(child)