Merge pull request #134 from ScriptKittyOS/fix/fips-coverage-step

fix(s003): the fips coverage step fails on a red suite

Author: HackTuah

.github/workflows/gate.yml

@@ -371,10 +371,16 @@ jobs:
       # AC2 and AC3 by test name with this run's id in the log, and AC5's coverage line for
       # this leg. The gate above already ran the suite; this repeats the four FIPS tests with
       # their names printed, then the whole suite once more under the coverage tool.
+      #
+      # Unpiped since the fix after slice 138. It read `mix test --cover | grep -E 'Result:|Total'`,
+      # and the runner's default `bash -e {0}` has no pipefail, so the step exited with grep's status:
+      # on job 118381934625 the suite here was 1388/1389 and the step passed, with the failing test's
+      # name filtered out. The coverage threshold is 0 (mix.exs), so the suite alone decides the
+      # exit, and the summary's Total line is in the output. test/workflows_test.exs holds the rule.
       - name: The FIPS tests by name, and this leg's coverage
         run: |
           mix test --trace test/fips
-          mix test --cover | grep -E 'Result:|\| *Total'
+          mix test --cover
 
       # Slice 137, AC5 and AC7: ML-DSA-87 with the mode off. This image links OpenSSL 3.5, the only
       # runtime in CI that signs ML-DSA-87 (the gate runners link 3.0; in the mode, Red Hat's

test/workflows_test.exs

@@ -4,7 +4,7 @@ defmodule WorkflowsTest do
   @moduledoc """
   The continuous integration definitions are code, and they are checked like code.
 
-  Three properties. Every workflow and the Dependabot configuration parse as YAML, so a malformed
+  Four properties. Every workflow and the Dependabot configuration parse as YAML, so a malformed
   one fails here rather than silently not running: GitHub reports a broken workflow file on its
   own page and not on the pull request, which is the wrong place for it to be discovered. Every
   third-party action is pinned to a full commit hash rather than a tag, because a tag can be moved
@@ -13,6 +13,8 @@ defmodule WorkflowsTest do
   block granting no more than read, so that a job wanting to write has to say so itself: Scorecard's
   `Token-Permissions` check, which this repository scored **zero** on until 2026-09-23 because two
   workflows declared nothing at all and ran with whatever the repository default happened to be.
+  And no step pipes the test suite or the gate into another command without `pipefail`, because the
+  runner's default shell takes the last command's exit, so a red suite piped into `grep` passes.
 
   The third property is the one worth a sentence about why a test rather than a habit. A top-level
   write is granted to every job the workflow will ever have, including the ones nobody has written
@@ -83,6 +85,47 @@ defmodule WorkflowsTest do
     end
   end
 
+  # Added after slice 138 (its NOTES, follow-up): the fips job's coverage step ran
+  # `mix test --cover | grep ...` under the runner's default `bash -e {0}`, which has no pipefail,
+  # so the step's exit was grep's. On job 118381934625 the suite inside it was 1388/1389 and the
+  # step and the job passed, with the failing test's name filtered out of the log. A step that runs
+  # the suite or the gate must fail when they do.
+  test "no step pipes the test suite or the gate into another command without pipefail" do
+    offenders =
+      for file <- yaml_files(),
+          String.contains?(file, "/workflows/"),
+          workflow = read_yaml(file),
+          {job_id, job} <- Map.get(workflow, "jobs", %{}),
+          {step, index} <- Enum.with_index(Map.get(job, "steps", [])),
+          run = Map.get(step, "run"),
+          is_binary(run),
+          pipes_suite?(run),
+          not pipefail?(step, job, workflow, run) do
+        "#{file}: job #{job_id}, step #{Map.get(step, "name", "##{index}")}"
+      end
+
+    assert offenders == [],
+           "these steps pipe `mix test` or `mix gate` into another command, so the step exits " <>
+             "with that command's status and a red suite passes. Run them unpiped, or give the " <>
+             "step `shell: bash` (which adds -o pipefail):\n" <> Enum.join(offenders, "\n")
+  end
+
+  # A line that runs `mix test` or `mix gate` and has a single `|` after it (not `||`).
+  defp pipes_suite?(run) do
+    run
+    |> String.split("\n")
+    |> Enum.any?(&Regex.match?(~r/\bmix\s+(test|gate)\b[^|]*\|(?!\|)/, &1))
+  end
+
+  defp pipefail?(step, job, workflow, run) do
+    shell =
+      Map.get(step, "shell") || get_in(job, ["defaults", "run", "shell"]) ||
+        get_in(workflow, ["defaults", "run", "shell"])
+
+    shell == "bash" or String.contains?(run, "set -o pipefail") or
+      String.contains?(run, "set -eo pipefail")
+  end
+
   # `permissions:` is either a map of scopes, or one of the shorthands `read-all`/`write-all`.
   defp top_level_scopes("read-all"), do: []
   defp top_level_scopes("write-all"), do: [{"(every scope)", "write"}]