From e0747a782999af30f37074f7689fbda25b34b827 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:43:20 +0100 Subject: [PATCH] fix(actions-lock): stop advisory findings blocking the lock wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `verify_lock_coverage` accepts the known gh-actions-lock v0.1.6 reusable-workflow `stale` false positive, and on the `valid:true` path it records advisory findings without blocking. The `valid:false` path did not: every finding that was not the accepted false positive was counted as blocking, so advisory `sha-as-ref` findings became fatal the moment one real finding flipped `valid` to false. Measured on metadatastician/burble 2026-09-22, which carries a genuine job-level reusable-workflow ref that the released tool cannot see: without the reusable lock entry -> valid=true, 7 sha-as-ref, wrapper rc 0 with the reusable lock entry -> valid=false, 7 sha-as-ref + 1 stale, stale accepted, wrapper rc 1 The wrapper printed "Accepted reusable-workflow lock coverage" and failed anyway. That held burble's workflow-safety job red and, because the step fail-fasts, its secret-baseline positive/negative controls have been skipped rather than run. Discriminate by category, not severity. gh-actions-lock v0.1.6 labels every finding `severity: warning`, `stale` included, so a severity-based exemption would swallow real desyncs and pass vacuously. `sha-as-ref` is advisory because the tool's own authoritative `valid` bit is unaffected by it. Advisory findings still never explain a `valid:false` result on their own, so the fail-closed guard is unchanged. Tests: three cases added to scripts/tests/actions-lock-update-test.sh — advisory alongside an accepted reusable false positive must pass; a real stale must still block alongside advisories; advisories alone must still fail closed. Suite is 12/12. Reverting this fix kills the first; a severity-based exemption is killed by the existing reusable-acceptance test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- scripts/tests/actions-lock-update-test.sh | 38 +++++++++++++++++++++++ scripts/update-actions-lock.sh | 28 +++++++++++++++-- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/scripts/tests/actions-lock-update-test.sh b/scripts/tests/actions-lock-update-test.sh index c4058d623..c3b73c5ba 100755 --- a/scripts/tests/actions-lock-update-test.sh +++ b/scripts/tests/actions-lock-update-test.sh @@ -49,6 +49,18 @@ if [ "${2:-}" = "--verify-local" ]; then printf '%s\n' '{"valid":true,"findings":[{"workflow":".github/workflows/ci.yml","category":"sha-as-ref","severity":"warning","dependency":"actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1"}]}' exit 1 ;; + invalid-advisory-and-reusable) + printf '%s\n' '{"valid":false,"findings":[{"workflow":".github/workflows/ci.yml","category":"sha-as-ref","severity":"warning","dependency":"actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1"},{"workflow":".github/workflows/reusable.yml","category":"stale","severity":"warning","dependency":"hyperpolymath/standards@abc123"}]}' + exit 1 + ;; + invalid-advisory-and-real-stale) + printf '%s\n' '{"valid":false,"findings":[{"workflow":".github/workflows/ci.yml","category":"sha-as-ref","severity":"warning","dependency":"actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1"},{"workflow":".github/workflows/ci.yml","category":"stale","severity":"warning","dependency":"actions/upload-artifact@v4"}]}' + exit 1 + ;; + invalid-advisory-only) + printf '%s\n' '{"valid":false,"findings":[{"workflow":".github/workflows/ci.yml","category":"sha-as-ref","severity":"warning","dependency":"actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1"}]}' + exit 1 + ;; invalid-empty) printf '%s\n' '{"valid":false,"findings":[]}' exit 1 @@ -145,6 +157,32 @@ if FAKE_VERIFY_FINDING=invalid-empty GH_BIN="$WORK/bin/fake-gh" \ fi echo "PASS: invalid result with no explainable findings fails closed" +# An advisory finding alongside the accepted reusable-workflow false positive +# must not block. Until 2026-09-22 every `sha-as-ref` became fatal the moment +# one real finding flipped `valid` to false, which held metadatastician/burble +# red on 7 advisories and skipped the secret-baseline controls behind it. +mixed_output="$(FAKE_VERIFY_FINDING=invalid-advisory-and-reusable GH_BIN="$WORK/bin/fake-gh" \ + bash "$UPDATE" --verify-local .github/workflows)" +printf '%s\n' "$mixed_output" | grep -q 'Accepted reusable-workflow lock coverage' +printf '%s\n' "$mixed_output" | grep -q 'advisory finding(s) recorded, not blocking' +echo "PASS: advisory finding alongside an accepted reusable false positive does not block" + +# The exemption must key on CATEGORY, not severity: v0.1.6 marks `stale` +# `severity: warning` too, so a severity-based exemption would pass vacuously. +if FAKE_VERIFY_FINDING=invalid-advisory-and-real-stale GH_BIN="$WORK/bin/fake-gh" \ + bash "$UPDATE" --verify-local .github/workflows >/dev/null 2>&1; then + echo "FAIL: a real stale finding was hidden by the advisory exemption" >&2 + exit 1 +fi +echo "PASS: a real stale finding still blocks alongside advisory findings" + +if FAKE_VERIFY_FINDING=invalid-advisory-only GH_BIN="$WORK/bin/fake-gh" \ + bash "$UPDATE" --verify-local .github/workflows >/dev/null 2>&1; then + echo "FAIL: advisory findings alone were accepted as explaining valid:false" >&2 + exit 1 +fi +echo "PASS: advisory findings alone cannot explain an invalid result" + if FAKE_VERIFY_FINDING=malformed-success GH_BIN="$WORK/bin/fake-gh" \ bash "$UPDATE" --verify-local .github/workflows >/dev/null 2>&1; then echo "FAIL: malformed successful verifier output was accepted" >&2 diff --git a/scripts/update-actions-lock.sh b/scripts/update-actions-lock.sh index 380517bff..aa5e59d5d 100755 --- a/scripts/update-actions-lock.sh +++ b/scripts/update-actions-lock.sh @@ -69,6 +69,22 @@ workflow_references_reusable_dependency() { ' "$workflow" } +is_advisory_category() { + # Advisory findings never affect the tool's own `valid` bit: a tree whose + # only findings are `sha-as-ref` reports `"valid": true` (measured + # 2026-09-22 on metadatastician/burble -- 7 sha-as-ref findings, valid true, + # process exit 1). They must therefore not block on the `valid:false` path + # either. + # + # Discriminate by CATEGORY, never by severity. gh-actions-lock v0.1.6 labels + # every finding `"severity": "warning"`, `stale` included, so a severity test + # would swallow real desyncs and turn this gate vacuous. + case "$1" in + sha-as-ref) return 0 ;; + *) return 1 ;; + esac +} + verify_lock_coverage() { # gh-actions-lock v0.1.6 does not recognise reusable-workflow `uses:` # paths. GitHub's startup enforcement nevertheless requires callers to @@ -106,8 +122,11 @@ verify_lock_coverage() { remaining=0 accepted=0 + advisory=0 while IFS=$'\t' read -r category workflow dependency; do - if [[ "$category" = stale ]] && + if is_advisory_category "$category"; then + advisory=$((advisory + 1)) + elif [[ "$category" = stale ]] && workflow_references_reusable_dependency "$workflow" "$dependency"; then echo "Accepted reusable-workflow lock coverage: $workflow -> $dependency" accepted=$((accepted + 1)) @@ -116,9 +135,14 @@ verify_lock_coverage() { fi done < <(printf '%s' "$result" | jq -r '.findings[] | [.category, .workflow, .dependency] | @tsv') + if [[ "$advisory" -gt 0 ]]; then + echo "actions-lock: $advisory advisory finding(s) recorded, not blocking" + fi + # `valid:false` with no findings is contradictory and cannot be explained by # the one known reusable-workflow false positive. Fail closed rather than - # turning an empty/malformed diagnostic into approval. + # turning an empty/malformed diagnostic into approval. Advisory findings do + # not explain it either, so they never satisfy this guard on their own. if [[ "$remaining" -ne 0 || "$accepted" -eq 0 ]]; then printf '%s\n' "$result" return 1