fix(rules): restore the RE001 structural detectors deleted by #773

lib/rules/research_extensions.ex

@@ -159,11 +159,35 @@ defmodule Hypatia.Rules.ResearchExtensions do
 
   @doc """
   RE001: Reports each locally executed workflow job that references
-  `${{ secrets.* }}` without installing `step-security/harden-runner`
-  in that job. Fully commented lines and reusable-only jobs are ignored.
+  `${{ secrets.* }}` without a `step-security/harden-runner@...` step in the
+  same job. Full-line comments and reusable-workflow jobs without a direct
+  `runs-on` field are ignored.
 
-  Each warning points to the first active secret reference in the affected
-  job. Provenance: StepSecurity Harden-Runner deployment guide.
+  Each finding points to the first matching secret reference in the job.
+  Provenance: StepSecurity Harden-Runner deployment guide.
+
+  ## Why this is structural, not a regex
+
+  The check is per job, and both halves of it are indentation-sensitive:
+
+    * **`runs-on` must belong to this job directly.** A reusable job
+      (`uses:` at job level) delegates its runtime to the source workflow, so a
+      sibling job's `runs-on` must not satisfy it. `direct_job_field?/2`
+      therefore compares against the job's own field indent rather than
+      matching `runs-on:` anywhere in the block.
+
+    * **The hardener must be a real step.** `harden_runner_step?/1` walks
+      `steps:` by indent and accepts the reference only as a step entry or a
+      direct property of one. Two cases make a line-oriented match wrong in
+      *opposite* directions, so both are handled explicitly:
+
+      - `uses: "step-security/harden-runner@..."` — quoted scalars are valid
+        YAML and still install the hardener. Missing them raises a **false
+        positive** against an already-hardened job.
+      - the same text inside a block scalar (`run: |`, `env: >`) is shell or
+        data, not a step. `block_scalar?/1` opens a skip region until the
+        indent closes, because treating it as a step raises a **false
+        negative** that suppresses a genuine finding.
 
   Severity: `:warn`. Action: `:report`.
   """
@@ -185,19 +209,15 @@ defmodule Hypatia.Rules.ResearchExtensions do
       active_lines
       |> workflow_job_lines()
       |> Enum.flat_map(fn job_lines ->
-        active_content = Enum.map_join(job_lines, "\n", &elem(&1, 0))
-
         secret_line =
           Enum.find(job_lines, fn {line, _} ->
             Regex.match?(~r/\$\{\{\s*secrets\.[A-Za-z_][A-Za-z0-9_]*/, line)
           end)
 
         # Reusable jobs delegate their runtime to the source workflow. A sibling's
         # runner or hardener cannot establish this job's execution policy.
-        local_runner? = Regex.match?(~r/^\s+runs-on:/m, active_content)
-
-        installs_harden? =
-          Regex.match?(~r/^\s+(?:-\s+)?uses:\s*step-security\/harden-runner@/m, active_content)
+        local_runner? = direct_job_field?(job_lines, "runs-on")
+        installs_harden? = harden_runner_step?(job_lines)
 
         if not is_nil(secret_line) and local_runner? and not installs_harden? do
           {_source, line} = secret_line
@@ -226,6 +246,103 @@ defmodule Hypatia.Rules.ResearchExtensions do
     end)
   end
 
+  defp direct_job_field?(job_lines, field) do
+    case job_field_indent(job_lines) do
+      nil ->
+        false
+
+      field_indent ->
+        Enum.any?(job_lines, fn {line, _} ->
+          indent_of(line) == field_indent and
+            String.starts_with?(String.trim_leading(line), "#{field}:")
+        end)
+    end
+  end
+
+  defp harden_runner_step?(job_lines) do
+    with field_indent when is_integer(field_indent) <- job_field_indent(job_lines),
+         step_lines when step_lines != [] <- lines_in_steps(job_lines, field_indent),
+         step_indent when is_integer(step_indent) <- step_indent(step_lines, field_indent) do
+      case Enum.reduce_while(step_lines, {false, nil, nil}, fn {line, _},
+                                                               {in_step, property_indent,
+                                                                scalar_indent} ->
+             indent = indent_of(line)
+
+             cond do
+               scalar_indent && indent > scalar_indent ->
+                 {:cont, {in_step, property_indent, scalar_indent}}
+
+               indent == step_indent and Regex.match?(~r/^\s*-\s+/, line) ->
+                 if harden_runner_uses?(line) do
+                   {:halt, true}
+                 else
+                   {:cont, {true, nil, block_scalar?(line) && indent}}
+                 end
+
+               in_step and indent > step_indent ->
+                 direct_property? = is_nil(property_indent) or indent == property_indent
+                 property_indent = property_indent || indent
+
+                 if direct_property? and harden_runner_uses?(line) do
+                   {:halt, true}
+                 else
+                   {:cont, {in_step, property_indent, block_scalar?(line) && indent}}
+                 end
+
+               true ->
+                 {:cont, {in_step, property_indent, nil}}
+             end
+           end) do
+        true -> true
+        _ -> false
+      end
+    else
+      _ -> false
+    end
+  end
+
+  defp job_field_indent([_header | job_lines]) do
+    job_lines
+    |> Enum.map(&elem(&1, 0))
+    |> Enum.reject(&(String.trim(&1) == ""))
+    |> Enum.map(&indent_of/1)
+    |> Enum.min(fn -> nil end)
+  end
+
+  defp job_field_indent([]), do: nil
+
+  defp lines_in_steps(job_lines, field_indent) do
+    case Enum.find_index(job_lines, fn {line, _} ->
+           indent_of(line) == field_indent and
+             String.starts_with?(String.trim_leading(line), "steps:")
+         end) do
+      nil ->
+        []
+
+      index ->
+        job_lines
+        |> Enum.drop(index + 1)
+        |> Enum.take_while(fn {line, _} ->
+          String.trim(line) == "" or indent_of(line) > field_indent
+        end)
+    end
+  end
+
+  defp step_indent(step_lines, field_indent) do
+    step_lines
+    |> Enum.filter(fn {line, _} ->
+      indent_of(line) > field_indent and Regex.match?(~r/^\s*-\s+/, line)
+    end)
+    |> Enum.map(fn {line, _} -> indent_of(line) end)
+    |> Enum.min(fn -> nil end)
+  end
+
+  defp harden_runner_uses?(line) do
+    Regex.match?(~r/^\s*(?:-\s+)?uses:\s*["']?step-security\/harden-runner@/, line)
+  end
+
+  defp block_scalar?(line), do: Regex.match?(~r/:\s*[>|][0-9+-]*\s*(?:#.*)?$/, line)
+
   # Follow block-style jobs by indentation, retaining physical source lines.
   # As with the other research rules, this is a local static text analysis.
   defp workflow_job_lines(lines) do

tests/e2e.sh

@@ -141,16 +141,40 @@ if $HAS_ELIXIR; then
     # no assertion, no stacktrace, making the red job impossible to diagnose
     # from CI. Capture the run, judge it by mix's own exit status, and echo the
     # output when it fails.
+    #
+    # `tail -N` was then the second defect, and it is why five consecutive red
+    # runs were undiagnosable (issue #826). ExUnit prints failure diagnostics
+    # INLINE, as each test fails — it does not collect them at the end. Under
+    # `--trace` this suite emits roughly two lines per test across ~1600 tests,
+    # so any fixed tail window contains only trace tail and the summary: grepping
+    # the full logs of five red runs for '^\s*[0-9]+) test ' returned nothing.
+    # A window sized for a summary cannot show diagnostics emitted inline.
+    #
+    # So print the failure BLOCKS, selected by content, not by position: from the
+    # first `  1) test ...` to the end of the log. Fall back to a tail only when
+    # no failure block exists, which means the run died some other way (a compile
+    # error, or a startup crash) and the tail is then the informative part.
     _mix_log="${TMPDIR:-/tmp}/hypatia-mix-test.$$.log"
     if mix test --trace >"$_mix_log" 2>&1; then
         pass "Elixir unit tests pass"
     else
         fail_test "Elixir unit tests"
-        echo "--- mix test output (tail 120) ---"
-        tail -120 "$_mix_log"
+        _n_fail=$(grep -cE '^[[:space:]]*[0-9]+\) test ' "$_mix_log" || true)
+        echo "--- mix test: ${_n_fail} failure block(s) ---"
+        if [ "${_n_fail}" -gt 0 ]; then
+            # Everything from the first failure block onward, capped so a
+            # pathological run cannot flood the CI log.
+            sed -n '/^[[:space:]]*1) test /,$p' "$_mix_log" | head -500
+        else
+            echo "(no ExUnit failure block found — the run did not reach the tests)"
+            tail -120 "$_mix_log"
+        fi
         echo "--- end mix test output ---"
+        echo "full log retained at: $_mix_log"
+    fi
+    if [ "${HYPATIA_KEEP_MIX_LOG:-0}" != "1" ]; then
+        rm -f "$_mix_log"
     fi
-    rm -f "$_mix_log"
 
     echo ""
 fi