fix: fail the bios job when validation fails

.github/workflows/validate.yml

@@ -70,14 +70,22 @@ jobs:
           git diff --name-only --diff-filter=AMR -z "$BASE_SHA"..."$HEAD_SHA" \
             -- bios/ > changed_files.zlist
 
+      # The report has to reach the pull request whatever the outcome, so the
+      # exit code is recorded here and acted on after the comment is posted.
+      # Swallowing it outright left the job green on a file that failed its
+      # hash check.
       - name: Validate BIOS files
         id: validate
         run: |
           if [ -s changed_files.zlist ]; then
+            set +e
             xargs -0 python scripts/validate_pr.py --markdown -- \
-              < changed_files.zlist > report.md 2>&1 || true
+              < changed_files.zlist > report.md 2>&1
+            echo "rc=$?" >> "$GITHUB_OUTPUT"
+            set -e
           else
             echo "No BIOS files changed" > report.md
+            echo "rc=0" >> "$GITHUB_OUTPUT"
           fi
           cat report.md
 
@@ -88,6 +96,12 @@ jobs:
         env:
           GH_TOKEN: ${{ github.token }}
 
+      - name: Fail when a file did not validate
+        if: steps.validate.outputs.rc != '0'
+        run: |
+          echo "validate_pr.py exited ${{ steps.validate.outputs.rc }}" >&2
+          exit 1
+
   validate-configs:
     runs-on: ubuntu-latest
     steps:

tests/test_audit_regressions.py

@@ -131,6 +131,41 @@ class WorkflowRegressions(unittest.TestCase):
         document["on"] = document.pop(True, document.get("on"))
         return document
 
+    def test_no_validation_step_discards_its_exit_code(self):
+        """A check whose result is thrown away is not a check.
+
+        The BIOS validation step ended in `|| true`, so validate_pr.py could
+        exit 1 on a file that failed its hash and the job stayed green. The
+        report still has to reach the pull request, so the code is recorded
+        and acted on afterwards rather than swallowed. A best-effort side
+        action such as adding a label is not a check and keeps its `|| true`.
+        """
+        checks = ("python scripts/", "unittest", "mkdocs build")
+        for name in ("validate.yml", "deploy-site.yml"):
+            workflow = self._workflow(name)
+            for job_name, job in workflow["jobs"].items():
+                for step in job.get("steps", []):
+                    # A shell continuation puts the command and its `|| true`
+                    # on different lines, so they are rejoined before scanning.
+                    body = str(step.get("run", "")).replace("\\\n", " ")
+                    for line in body.splitlines():
+                        if not any(marker in line for marker in checks):
+                            continue
+                        self.assertNotIn(
+                            "|| true",
+                            line,
+                            f"{name}:{job_name}:{step.get('name', '?')} runs a "
+                            f"check and discards its exit code: {line.strip()}",
+                        )
+
+        workflow = self._workflow("validate.yml")
+        steps = workflow["jobs"]["validate-bios"]["steps"]
+        gate = [s for s in steps if "rc != " in str(s.get("if", ""))]
+        self.assertTrue(
+            gate,
+            "nothing in validate-bios acts on the validation exit code",
+        )
+
     def test_the_suite_runs_on_a_direct_push_to_main(self):
         workflow = self._workflow("validate.yml")
         triggers = workflow["on"]