diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml index bfc3220..0c12039 100644 --- a/.github/workflows/gate.yml +++ b/.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 diff --git a/test/workflows_test.exs b/test/workflows_test.exs index 65b8206..23bc973 100644 --- a/test/workflows_test.exs +++ b/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"}]