From 168d6259dca11be602c0b6a1f53126d18e305f51 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Tue, 19 May 2026 13:43:47 +0400 Subject: [PATCH] ci(status): skip Status job when upstream scan was cancelled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Semgrep Status and Security Status aggregator jobs evaluate the upstream scan's `result` and emit a FAILURE check_run when that result is `cancelled`. In practice the only way the upstream gets cancelled is concurrency superseding the run (two events arrive for the same SHA — common after force-push + rapid follow-up push, which leaves a delayed webhook in GitHub's queue). That FAILURE check_run then blocks branch protection on the surviving SUCCESS run, even though the actual scan was clean. Fix: change `if: always()` → `if: ${{ !cancelled() }}` on both Status jobs. After this: - scan succeeds → Status runs, evaluates findings, passes/fails - scan fails → Status runs, sees `result=failure`, fails - scan cancelled → Status SKIPPED, no FAILURE check_run produced A skipped Status produces a NEUTRAL check_run that branch protection treats as not-failing. The surviving non-cancelled run's Status emits SUCCESS and gates the merge correctly. Security trade-off (acknowledged inline in the YAML): The previous behaviour was justified by the comment "a fork PR that's cancelled mid-flight could merge without ever being scanned." That concern is mitigated separately: - Fork PRs do not receive repo secrets (GitHub policy) - These scans are read-only, no exfiltration vector - A maintainer reviewing the PR can see the cancellation / skip in the check-rollup before clicking Merge Observed in the wild on simple-container-com/api PR #273 (c38148e) — every required-status check went CANCELLED + SUCCESS from the dual-fire, with the cancelled twin's Status job emitting the FAILURE that blocked the merge. Refs simple-container-com/api#273. Signed-off-by: Dmitrii Creed --- .github/workflows/security-scan.yml | 20 +++++++++++++++++++- .github/workflows/semgrep.yml | 20 +++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 3ae5885..500d9bf 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -146,7 +146,25 @@ jobs: name: Security Status runs-on: ubuntu-24.04 needs: [secret-scan, sbom-scan] - if: always() + # `!cancelled()` lets Status run after the scans completed + # (success/failure) but SKIPS it when the upstream scans were + # cancelled — which in practice means the whole workflow run was + # superseded by concurrency (a newer event for the same SHA). + # A skipped Status produces a NEUTRAL check_run, which branch + # protection treats as not-failing; the surviving run's Status + # reports SUCCESS and gates the merge. + # + # Before this change: `if: always()` made Status run even on + # upstream cancellation, evaluate `result=cancelled` as a hard + # error, and emit a FAILURE check_run that blocked the PR even + # though the parallel non-cancelled run was clean. + # + # The original concern (a fork PR cancelling its own scan to + # bypass coverage) is mitigated separately: fork PRs do not + # receive secrets, the security-relevant scans are read-only, + # and a maintainer reviewing the PR would still see the + # cancellation in the check-rollup before merging. + if: ${{ !cancelled() }} steps: - name: Evaluate security results diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index a33c6c8..da8c887 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -86,7 +86,25 @@ jobs: name: Semgrep Status runs-on: ubuntu-24.04 needs: [semgrep-scan] - if: always() + # `!cancelled()` lets Status run after semgrep-scan completed + # (success/failure) but SKIPS it when the scan was cancelled — + # which in practice means the whole workflow run was superseded + # by concurrency (a newer event for the same SHA). A skipped + # Status produces a NEUTRAL check_run, which branch protection + # treats as not-failing; the surviving run's Status reports + # SUCCESS and gates the merge. + # + # Before this change: `if: always()` made Status run even on + # upstream cancellation, evaluate `result=cancelled` as a hard + # error, and emit a FAILURE check_run that blocked the PR even + # though the parallel non-cancelled run was clean. + # + # The original concern (a fork PR cancelling its own scan to + # bypass coverage) is mitigated separately: fork PRs do not + # receive secrets, Semgrep is read-only, and a maintainer + # reviewing the PR would still see the cancellation in the + # check-rollup before merging. + if: ${{ !cancelled() }} steps: - name: Evaluate Semgrep results