From b2b817781a941e2aa3e4b32ac00eea526f53648e Mon Sep 17 00:00:00 2001 From: Vito Date: Sun, 18 Oct 2026 18:00:21 -0700 Subject: [PATCH] ci: cancel superseded PR runs and make the bandit job a real gate (#231) * ci: cancel superseded PR runs and make the bandit job a real gate No PR workflow had a concurrency group, so a new push left the previous run occupying 16 Python matrix runners, the real ComfyUI job and the three viewerjs jobs for up to 25 minutes. Add a group per workflow and PR ref that cancels the older run. Pushes to main use the run id as their group, so no main commit is ever cancelled or dropped as pending. Remove the lint job from code-quality.yml. It ran black and flake8 a second time, partly with --exit-zero, next to the protected "black and flake8" job in test.yml. The security job ran bandit with "|| true" and could never fail. At medium severity it reports 74 findings today, so it now gates on high severity only and prints the medium report without failing. The only two high findings were SHA1 and MD5 used as a gallery entry id and a LoRA preview cache filename; mark both usedforsecurity=False, which leaves every hash value unchanged. Closes #230. * ci: verify superseded PR runs are cancelled * ci: require the bandit high-severity check Review on #231 pointed out that a failing bandit job did not block a merge because its context was not required. Name the job "bandit (high severity)" so the required context is unambiguous, and add it to .github/branch-protection.json and the TESTING.md list of required checks. --- .github/branch-protection.json | 3 +- .github/workflows/code-quality.yml | 49 ++++++++-------------------- .github/workflows/test.yml | 6 ++++ .github/workflows/viewerjs-check.yml | 6 ++++ TESTING.md | 3 +- py/api/images.py | 4 ++- py/lora_utils.py | 2 +- 7 files changed, 33 insertions(+), 40 deletions(-) diff --git a/.github/branch-protection.json b/.github/branch-protection.json index 7c12c40..adc552b 100644 --- a/.github/branch-protection.json +++ b/.github/branch-protection.json @@ -18,7 +18,8 @@ "black and flake8", "frontend modules", "viewerjs", - "real ComfyUI compatibility" + "real ComfyUI compatibility", + "bandit (high severity)" ] }, "enforce_admins": true, diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index bb23262..135bee2 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -9,42 +9,15 @@ on: pull_request: branches: [main] +# Superseded PR pushes cancel their older run. Every main push gets its own +# group (run_id), so no main commit's run is ever cancelled or dropped as pending. +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: - lint: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v7 - - - name: Set up Python 3.10 - uses: actions/setup-python@v7 - with: - python-version: "3.10" - - - name: Cache pip dependencies - uses: actions/cache@v6 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-quality-${{ hashFiles('requirements-dev.txt') }} - restore-keys: | - ${{ runner.os }}-pip-quality- - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements-dev.txt - - - name: Check formatting with Black - run: black --check --diff . - - - name: Lint with flake8 - run: | - # Fail on syntax errors and undefined names - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics - # Warnings as non-blocking - flake8 . --count --exit-zero --max-line-length=88 --statistics --exclude=reference/,.git,__pycache__,web/ - security: + name: bandit (high severity) runs-on: ubuntu-latest steps: @@ -58,5 +31,9 @@ jobs: - name: Install bandit run: pip install bandit[toml] - - name: Security scan - run: bandit -r . -ll --exclude=./tests,./reference,./venv -q || true + # Medium findings are reported without failing until they are triaged. + # High severity is a hard gate. + - name: Security report (medium and above, informational) + run: bandit -r . -ll -q --exclude ./tests,./reference,./venv,./tools/vendor/node_modules --exit-zero + - name: Security scan (high severity gate) + run: bandit -r . -lll -q --exclude ./tests,./reference,./venv,./tools/vendor/node_modules diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3a5210c..754d8a3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,6 +9,12 @@ on: push: branches: [main] +# Superseded PR pushes cancel their older run. Every main push gets its own +# group (run_id), so no main commit's run is ever cancelled or dropped as pending. +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + env: PYTHONFAULTHANDLER: "1" diff --git a/.github/workflows/viewerjs-check.yml b/.github/workflows/viewerjs-check.yml index 72bf93b..449c724 100644 --- a/.github/workflows/viewerjs-check.yml +++ b/.github/workflows/viewerjs-check.yml @@ -10,6 +10,12 @@ on: permissions: contents: read +# Superseded PR pushes cancel their older run. Every main push gets its own +# group (run_id), so no main commit's run is ever cancelled or dropped as pending. +concurrency: + group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: frontend-coverage: name: frontend coverage (chromium and touch) diff --git a/TESTING.md b/TESTING.md index fcce8af..4950592 100644 --- a/TESTING.md +++ b/TESTING.md @@ -142,7 +142,8 @@ Before changing paths, SQLite, streams, restore or cleanup: ## Required merge checks `.github/branch-protection.json` records the required checks: all twelve Python/OS -combinations, Python coverage, formatting, frontend modules, `viewerjs`, and `real ComfyUI compatibility`. +combinations, Python coverage, formatting, frontend modules, `viewerjs`, `real ComfyUI compatibility`, +and `bandit (high severity)`. The branch must be current with main, and administrators follow the same checks. It does not require an additional human approval. diff --git a/py/api/images.py b/py/api/images.py index d12af2b..acf8dfb 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -386,7 +386,9 @@ def _output_image_entry( thumbnail_url += f"&root_scope={root_scope}" entry = { - "id": hashlib.sha1(str(media_path).encode("utf-8")).hexdigest()[:16], + "id": hashlib.sha1( + str(media_path).encode("utf-8"), usedforsecurity=False + ).hexdigest()[:16], "filename": media_path.name, "path": rel_path.as_posix(), "relative_path": str(rel_path), diff --git a/py/lora_utils.py b/py/lora_utils.py index 0c8b33e..454c182 100644 --- a/py/lora_utils.py +++ b/py/lora_utils.py @@ -541,7 +541,7 @@ def download_civitai_images( if not isinstance(url, str) or not is_civitai_url(url): continue - url_hash = hashlib.md5(url.encode()).hexdigest()[:12] + url_hash = hashlib.md5(url.encode(), usedforsecurity=False).hexdigest()[:12] local_path = lora_cache / f"{url_hash}.jpg" if local_path.exists():