fix(scorecard): reconcile action-pin findings per workflow file

scripts/reconcile-scorecard-actions-lock.rb

@@ -3,6 +3,12 @@
 # Reconcile only Scorecard's inline action-pin findings with native lock coverage.
 # No lock semantics are reimplemented: gh actions-lock verifies each workflow.
 # Usage: ruby reconcile-scorecard-actions-lock.rb INPUT OUTPUT AUDIT_JSON [ROOT]
+# Exit codes:
+#   0  every eligible workflow verified; OUTPUT and AUDIT_JSON written.
+#   3  OUTPUT and AUDIT_JSON written, but at least one workflow failed native
+#      verification. Only that workflow's findings are retained; findings in
+#      verified workflows are still removed. Callers must treat this as failure.
+#   2  usage or SARIF structure error; OUTPUT is NOT written.
 require 'json'
 require 'open3'
 require 'yaml'
@@ -80,6 +86,11 @@
     end
   end
 
+  # Decide whether one workflow's `gh actions-lock --verify` result is clean
+  # enough to suppress its Scorecard pin findings. Accepts a valid result whose
+  # findings are empty (with a zero exit) or only `sha-as-ref`, and an invalid
+  # result whose only non-`sha-as-ref` findings are `stale` entries for
+  # job-level reusable refs the verifier cannot see (standards#1036).
   def self.verification_accepted?(verification, status, reusable_deps)
     return false unless verification.is_a?(Hash) && verification['findings'].is_a?(Array)
     findings = verification['findings']
@@ -103,22 +114,52 @@
     accepted_stale.positive?
   end
 
+  # Verify one workflow (+relative+ to +root+) against `actions.lock`: job-level
+  # reusable refs must be recorded in the lock, and `gh actions-lock --verify`
+  # must report an accepted result. Returns the verifier's JSON on success.
+  # Raises with the reason on any failure, including unparseable workflow YAML,
+  # a missing `gh`, or non-JSON verifier output, so the caller can confine the
+  # failure to this one file.
+  def self.verify_workflow(root, relative)
+    path = File.join(root, relative)
+    lock_path = File.join(root, '.github/workflows/actions.lock')
+    reusable_deps = reusable_workflow_deps(path)
+    unless lock_covers_reusable_deps?(lock_path, relative, reusable_deps)
+      raise "Native action-lock verification failed for #{relative}: job-level reusable ref absent from actions.lock"
+    end
+
+    stdout, stderr, status = Open3.capture3('gh', 'actions-lock', relative,
+      '--verify', '--no-interactive', '--json=valid,findings', chdir: root)
+    warn stderr unless stderr.empty?
+    verification = JSON.parse(stdout)
+    unless verification_accepted?(verification, status, reusable_deps)
+      raise "Native action-lock verification failed for #{relative}"
+    end
+    verification
+  end
+
   # Remove Scorecard action-pin findings only when they identify a remote action
   # entry in a regular workflow below +root+ and native action-lock verification
-  # succeeds. The supplied SARIF document is updated in place and returned with
-  # audit records for removed findings.
+  # of THAT workflow succeeds. Verification is per file: a workflow that fails
+  # keeps all of its findings and is recorded in the returned failures map
+  # (relative path => reason), while findings in other, verified workflows are
+  # still removed. The supplied SARIF document is updated in place and returned
+  # as [document, audit, failures], where audit lists the removed findings.
   #
-  # Invokes +gh actions-lock+ once per eligible workflow. Raises when the SARIF
-  # structure or native verification result is invalid, or verification fails.
+  # Invokes +gh actions-lock+ at most once per eligible workflow. Raises only
+  # when the SARIF structure itself is invalid.
   def self.reconcile(document, root)
     raise 'Expected a SARIF 2.1.0 document with runs' unless document.is_a?(Hash) &&
       document['version'] == '2.1.0' && document['runs'].is_a?(Array) && !document['runs'].empty?
+    document['runs'].each do |run|
+      raise 'Expected a results array' unless run.is_a?(Hash) && run['results'].is_a?(Array)
+    end
 
     root = File.realpath(root)
     verified = {}
+    failures = {}
     audit = []
     document['runs'].each do |run|
-      raise 'Expected a results array' unless run['results'].is_a?(Array)
       next unless run.dig('tool', 'driver', 'name') == 'Scorecard'
 
       run['results'] = run['results'].reject do |result|
@@ -135,31 +176,33 @@
         path = File.join(root, relative)
         next false unless File.file?(path) && !File.symlink?(path) &&
           File.realpath(path).start_with?(root + '/') && File.file?(File.join(root, '.github/workflows/actions.lock'))
-        next false unless action_lines(path).include?(line)
+        # A failure in one workflow is confined to that workflow: its findings
+        # are kept and every other workflow is still reconciled on its own.
+        next false if failures.key?(relative)
+
+        begin
+          lines = action_lines(path)
+        rescue StandardError => error
+          failures[relative] = "Unparseable workflow #{relative}: #{error.message}"
+          next false
+        end
+        next false unless lines.include?(line)
 
         unless verified.key?(relative)
-          lock_path = File.join(root, '.github/workflows/actions.lock')
-          reusable_deps = reusable_workflow_deps(path)
-          unless lock_covers_reusable_deps?(lock_path, relative, reusable_deps)
-            raise "Native action-lock verification failed for #{relative}: job-level reusable ref absent from actions.lock"
+          begin
+            verified[relative] = verify_workflow(root, relative)
+          rescue StandardError => error
+            failures[relative] = error.message
+            next false
           end
-
-          stdout, stderr, status = Open3.capture3('gh', 'actions-lock', relative,
-            '--verify', '--no-interactive', '--json=valid,findings', chdir: root)
-          warn stderr unless stderr.empty?
-          verification = JSON.parse(stdout)
-          unless verification_accepted?(verification, status, reusable_deps)
-            raise "Native action-lock verification failed for #{relative}"
-          end
-          verified[relative] = verification
         end
         audit << { 'file' => relative, 'line' => line, 'rule' => result['ruleId'],
           'reason' => 'False positive: native direct and transitive pins verified by gh actions-lock --verify',
           'verification' => verified.fetch(relative) }
         true
       end
     end
-    [document, audit]
+    [document, audit, failures]
   end
 end
 
@@ -168,10 +211,16 @@
     input, output, audit_path, root = ARGV
     raise 'Usage: INPUT OUTPUT AUDIT_JSON [ROOT]' unless input && output && audit_path && ARGV.length <= 4
     raise 'Keep the original SARIF as a separate artifact' if File.expand_path(input) == File.expand_path(output)
-    document, audit = ScorecardActionsLock.reconcile(JSON.parse(File.read(input)), root || Dir.pwd)
+    document, audit, failures = ScorecardActionsLock.reconcile(JSON.parse(File.read(input)), root || Dir.pwd)
     File.write(output, JSON.pretty_generate(document) + "\n")
     File.write(audit_path, JSON.pretty_generate(audit) + "\n")
     puts "Reconciled #{audit.length} verified native action-pin false positives; all other findings retained."
+    unless failures.empty?
+      failures.each { |relative, reason| warn "Scorecard reconciliation kept findings for #{relative}: #{reason}" }
+      warn "Scorecard reconciliation incomplete: #{failures.length} workflow(s) failed native action-lock " \
+           'verification; their findings are retained in the reconciled SARIF.'
+      exit 3
+    end
   rescue StandardError => error
     warn "Scorecard reconciliation failed: #{error.message}"
     exit 2

scripts/tests/reconcile-scorecard-actions-lock-test.rb

@@ -4,9 +4,14 @@
 require 'tmpdir'
 require 'fileutils'
 require 'minitest/autorun'
+require 'open3'
+require 'rbconfig'
 require_relative '../reconcile-scorecard-actions-lock'
 
 class ScorecardActionsLockTest < Minitest::Test
+  # Build a temporary repository root with one workflow, a placeholder lock and
+  # a stub `gh` on PATH that records invocations and replays canned verifier
+  # output (shared, or per workflow from responses/).
   def setup
     @root = Dir.mktmpdir('scorecard-lock-')
     @old_path = ENV.fetch('PATH')
@@ -20,6 +25,14 @@ def setup
         actions-lock\\ .github/workflows/*.yml\\ --verify\\ --no-interactive\\ --json=valid,findings) ;;
         *) exit 97 ;;
       esac
+      # Per-workflow responses: responses/<basename>.json (+ .exit) override
+      # the shared TEST_LOCK_JSON / TEST_LOCK_EXIT, so one run can mix a
+      # verifying workflow with a failing one.
+      name=$(basename "$2")
+      if [ -f "responses/$name.json" ]; then
+        cat "responses/$name.json"
+        exit "$(cat "responses/$name.exit" 2>/dev/null || echo 0)"
+      fi
       printf '%s' "$TEST_LOCK_JSON"
       exit "${TEST_LOCK_EXIT:-0}"
     SH
@@ -65,14 +78,27 @@ def test_missing_lock_and_foreign_tools_are_not_filtered
     refute File.exist?(File.join(@root, 'invocation'))
   end
 
+  # A verifier result that is malformed, invalid or paired with a non-zero exit
+  # never removes a finding: the finding is kept, no audit record is written,
+  # and the workflow is reported as a failure.
   def test_invalid_verification_never_becomes_a_clean_result
     ['{}', '{"valid":false,"findings":[]}', '{"valid":true}', '[]', 'not JSON'].each do |invalid|
       ENV['TEST_LOCK_JSON'] = invalid
-      assert_raises(StandardError) { ScorecardActionsLock.reconcile(document(finding), @root) }
+      assert_verification_failed(document(finding), '.github/workflows/ci.yml')
     end
     ENV['TEST_LOCK_JSON'] = '{"valid":true,"findings":[]}'
     ENV['TEST_LOCK_EXIT'] = '1'
-    assert_raises(StandardError) { ScorecardActionsLock.reconcile(document(finding), @root) }
+    assert_verification_failed(document(finding), '.github/workflows/ci.yml')
+  end
+
+  # Assert that reconciling +doc+ removes nothing, records no audit entry and
+  # reports +relative+ as a failed workflow.
+  def assert_verification_failed(doc, relative)
+    original = JSON.parse(JSON.generate(doc))
+    result, audit, failures = ScorecardActionsLock.reconcile(doc, @root)
+    assert_equal original, result
+    assert_empty audit
+    assert_equal [relative], failures.keys
   end
 
   def test_workflow_symlinks_are_not_filtered
@@ -132,7 +158,10 @@ def test_mixed_workflow_with_job_level_reusable_ref_is_not_rejected_as_stale
     assert_equal 1, audit.length
   end
 
-  def test_arm_d_job_level_reusable_ref_absent_from_lock_raises
+  # Arm D of standards#1036: a job-level reusable ref absent from actions.lock
+  # is a verification failure even though gh actions-lock passes it vacuously;
+  # the finding is kept and the workflow reported, never silently dropped.
+  def test_arm_d_job_level_reusable_ref_absent_from_lock_is_kept_and_reported
     missing_sha = 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef'
     File.write(File.join(@root, '.github/workflows/scorecard.yml'), <<~YAML)
       name: Scorecard
@@ -150,11 +179,90 @@ def test_arm_d_job_level_reusable_ref_absent_from_lock_raises
     ENV['TEST_LOCK_JSON'] = '{"valid":true,"findings":[]}'
     ENV['TEST_LOCK_EXIT'] = '0'
 
-    assert_raises(StandardError) do
-      ScorecardActionsLock.reconcile(
-        document(finding(path: '.github/workflows/scorecard.yml', line: 5)),
-        @root
-      )
-    end
+    assert_verification_failed(document(finding(path: '.github/workflows/scorecard.yml', line: 5)),
+                               '.github/workflows/scorecard.yml')
+  end
+  # Write a second workflow and a per-file verifier response that FAILS for it
+  # (a real lock desync, as on metadatastician/ZenodoDeposits.jl quality.yml).
+  def add_failing_workflow(name = 'quality.yml')
+    File.write(File.join(@root, ".github/workflows/#{name}"),
+               "jobs:\n  lint:\n    steps:\n      - uses: actions/checkout@v7.0.0\n")
+    FileUtils.mkdir_p(File.join(@root, 'responses'))
+    File.write(File.join(@root, "responses/#{name}.json"), JSON.generate(
+      'valid' => false,
+      'findings' => [{ 'category' => 'ref-changed', 'severity' => 'error',
+                       'dependency' => 'actions/checkout@v7.0.0' }]
+    ))
+    File.write(File.join(@root, "responses/#{name}.exit"), '1')
+    ".github/workflows/#{name}"
+  end
+
+  # The regression for the whole-document abort: one workflow failing native
+  # verification must not stop reconciliation of any other workflow. The
+  # failing file comes FIRST so an abort (or a "stop filtering after the first
+  # failure" variant) leaves the verified file's findings in place.
+  def test_one_failing_workflow_does_not_unreconcile_the_others
+    bad = add_failing_workflow
+    bad_findings = [finding(path: bad, line: 4), finding(path: bad, line: 4)]
+    good_findings = [finding, finding]
+    result, audit, failures = ScorecardActionsLock.reconcile(document(*bad_findings, *good_findings), @root)
+
+    assert_equal bad_findings, result['runs'][0]['results'], 'failing workflow keeps every finding; verified one loses its'
+    assert_equal ['.github/workflows/ci.yml'] * 2, audit.map { |a| a['file'] }
+    assert_equal [bad], failures.keys
+    assert_match(/Native action-lock verification failed for #{Regexp.escape(bad)}/, failures[bad])
+    invocations = File.readlines(File.join(@root, 'invocation'))
+    assert_equal 2, invocations.length, 'gh runs once per workflow, failures included'
+  end
+
+  # A workflow whose YAML cannot be parsed is a failure of that workflow only.
+  def test_unparseable_workflow_is_confined_to_itself
+    File.write(File.join(@root, '.github/workflows/broken.yml'), "jobs: [unclosed\n")
+    broken = finding(path: '.github/workflows/broken.yml', line: 1)
+    result, audit, failures = ScorecardActionsLock.reconcile(document(broken, finding), @root)
+    assert_equal [broken], result['runs'][0]['results']
+    assert_equal 1, audit.length
+    assert_equal ['.github/workflows/broken.yml'], failures.keys
+  end
+
+  # Run the script as scorecard-reusable.yml does and return [status, stderr].
+  def run_cli(input_doc, out, audit)
+    input = File.join(@root, 'results.sarif')
+    File.write(input, input_doc.is_a?(String) ? input_doc : JSON.generate(input_doc))
+    script = File.expand_path('../reconcile-scorecard-actions-lock.rb', __dir__)
+    _, stderr, status = Open3.capture3(RbConfig.ruby, script, input, out, audit, @root)
+    [status, stderr]
+  end
+
+  # Workflow contract: on a partial failure the reconciled SARIF IS written (so
+  # the select step uploads it, not the raw SARIF) and the exit is non-zero (so
+  # the run still fails by design).
+  def test_cli_partial_failure_writes_output_and_exits_nonzero
+    bad = add_failing_workflow
+    out = File.join(@root, 'out.sarif')
+    audit = File.join(@root, 'audit.json')
+    status, stderr = run_cli(document(finding(path: bad, line: 4), finding), out, audit)
+    assert_equal 3, status.exitstatus
+    assert File.size?(out), 'reconciled SARIF must be written on a partial failure'
+    kept = JSON.parse(File.read(out))['runs'][0]['results']
+    assert_equal [bad], kept.map { |r| r.dig('locations', 0, 'physicalLocation', 'artifactLocation', 'uri') }
+    assert_equal 1, JSON.parse(File.read(audit)).length
+    assert_match(/kept findings for #{Regexp.escape(bad)}/, stderr)
+  end
+
+  # Workflow contract: success exits 0; a structural error exits 2 and writes
+  # no output, so the workflow falls back to the raw SARIF and fails.
+  def test_cli_success_and_structural_error_exit_codes
+    out = File.join(@root, 'out.sarif')
+    audit = File.join(@root, 'audit.json')
+    status, = run_cli(document(finding), out, audit)
+    assert_equal 0, status.exitstatus
+    assert_empty JSON.parse(File.read(out))['runs'][0]['results']
+
+    File.unlink(out)
+    status, stderr = run_cli('{"version":"2.0.0","runs":[]}', out, audit)
+    assert_equal 2, status.exitstatus
+    refute File.exist?(out), 'no reconciled SARIF on a structural error'
+    assert_match(/Scorecard reconciliation failed/, stderr)
   end
 end