From 09c6b164ba5a1fe31b43f6fc16b0195a15759738 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:46:36 +0100 Subject: [PATCH 1/2] fix(rules): escape `/` in three regex sigils so hypatia compiles again #862 put an unescaped `/` inside a character class in three `~r/.../` sigils (pin_integrity.ex @uses_regex and locked_refs/1, pr_automerge.ex @pin_re). In a `~r/` sigil that `/` terminates the sigil, so `mix compile` fails with MismatchedDelimiterError. Every estate Hypatia scan (hypatia-scan-reusable.yml resolves hypatia@main) has failed since. `\/` inside a character class matches the same byte set as `/`; this change is semantics-preserving and only restores compilation. The two modules' own tests (17) now run for the first time and fail on latent logic bugs in #862; nothing outside test/ calls either module. Those are fixed in a follow-up PR, kept separate so this outage fix stays three characters. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --- lib/rules/pin_integrity.ex | 4 ++-- lib/rules/pr_automerge.ex | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 6919d6dc..f3e4d506 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -53,7 +53,7 @@ defmodule Hypatia.Rules.PinIntegrity do or to the owner as `flag` when the repair is not a substitution. """ - @uses_regex ~r/^\s*-?\s*uses:\s*(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]*?)@(?[^\s#]+)\s*(?:#\s*(?.*?))?\s*$/ + @uses_regex ~r/^\s*-?\s*uses:\s*(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]*?)@(?[^\s#]+)\s*(?:#\s*(?.*?))?\s*$/ @doc """ Parse every `uses:` pin site in a workflow file. @@ -362,7 +362,7 @@ defmodule Hypatia.Rules.PinIntegrity do |> String.split("\n") |> Enum.with_index(1) |> Enum.flat_map(fn {line, number} -> - case Regex.run(~r/'(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]+)@(?[^\s']+)'/, line) do + case Regex.run(~r/'(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]+)@(?[^\s']+)'/, line) do nil -> [] diff --git a/lib/rules/pr_automerge.ex b/lib/rules/pr_automerge.ex index 780f12d8..273629b1 100644 --- a/lib/rules/pr_automerge.ex +++ b/lib/rules/pr_automerge.ex @@ -419,7 +419,7 @@ defmodule Hypatia.Rules.PrAutomerge do # - - uses: actions/checkout@v4.1.7 # so the marker is part of the match. (Without this, every real diff looks # like a non-pin change and every PR falls through to `flag`.) - @pin_re ~r/^[-+]?\s*-?\s*uses:\s*(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]*?)@(?[^\s#]+)/ + @pin_re ~r/^[-+]?\s*-?\s*uses:\s*(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]*?)@(?[^\s#]+)/ @doc "Parse a `uses:` line into `%{action, base, ref}`; `nil` when it is not one." def parse_pin(line) do From e6419d41f2ccbd1e8dbcafa991d429f38c0df80f Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:50:12 +0100 Subject: [PATCH 2/2] fix(rules): the latent logic bugs #862 shipped behind a compile error PinIntegrity and PrAutomerge never compiled on main, so their 17 tests had never run. With #876 they run; this fixes what they found. PinIntegrity - claimed_version/1: Regex.run drops trailing unmatched groups, so a `v`-led match returned a 1-element list neither clause matched -> nil. That cascaded into pi002 and pi005 never firing. - relabel_line/2 prepended `#` to relabel/2's already-`#`-led result (`##`). Now mirrors estate-pin-integrity.sh: head without `#`, comment handed over with it. - relabel/2 restores `# ` for the `#`-stripped comment pin_sites/1 emits; `#v3` and "" keep the shell mirror's byte-identical output. PrAutomerge - pin_deltas/3 returned a bare map from a flat_map callback, so every delta was flattened into {key, value} tuples. - verdict/3 dropped the scan, so decisions lacked deltas/licence_touch. - SAFETY: close_poison_only went through accept/4, i.e. a pin-only PR onto a DENYLISTED pin was classified safety=arm_auto. Now reject/4. Mutant control: reverting this alone turns exactly that test red. - decision_manifest/2 vetoes use string keys like the rest of the manifest (the schema-shaped output). Test fixture: "a patch that also edits a permissions block" carried the permissions lines as diff CONTEXT (leading space) under a mismatched hunk header, so it asserted an edit the diff did not contain. The code was right; the fixture now makes the edit real (-/+ lines). mix test: 1673 tests, 0 failures. mix compile --warnings-as-errors: clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --- lib/rules/pin_integrity.ex | 28 ++++++++++++--- lib/rules/pr_automerge.ex | 62 ++++++++++++++++++++++---------- test/rules/pr_automerge_test.exs | 16 ++++++--- 3 files changed, 77 insertions(+), 29 deletions(-) diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index f3e4d506..9eb82f5e 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -284,9 +284,13 @@ defmodule Hypatia.Rules.PinIntegrity do # `v3` is a real shape in the estate (dictask carries a poisoned pin # annotated `# v3`), so a bare major behind a `v` counts. A bare number # without the `v` does not: `# 2 jobs` is prose, not a version claim. - case Regex.run(~r/\b(?:v(\d+(?:\.\d+)*)|(\d+\.\d+(?:\.\d+)?))\b/, comment) do - [_, version, _] when version != "" -> version - [_, _, version] when version != "" -> version + # `capture: :all_but_first` still DROPS trailing unmatched groups, so a + # `v`-led match yields one element and a bare `4.38.0` yields two. + case Regex.run(~r/\b(?:v(\d+(?:\.\d+)*)|(\d+\.\d+(?:\.\d+)?))\b/, comment, + capture: :all_but_first + ) do + [version] -> version + ["", version] -> version _ -> nil end end @@ -512,7 +516,9 @@ defmodule Hypatia.Rules.PinIntegrity do defp relabel_line(line, version) when is_binary(version) do case String.split(line, "#", parts: 2) do - [head, comment] -> head <> "#" <> relabel(comment, version) + # Mirror estate-pin-integrity.sh: the head keeps no `#`, the comment + # is handed over WITH it, and relabel/2 returns it with it. + [head, comment] -> head <> relabel("#" <> comment, version) _ -> line end end @@ -537,6 +543,11 @@ defmodule Hypatia.Rules.PinIntegrity do """ @spec relabel(String.t(), nil | String.t()) :: String.t() def relabel(comment, version) when is_binary(comment) and is_binary(version) do + # pin_sites/1 hands over the comment with its `#` and spacing already + # stripped (`v3`); restore the canonical `# ` so the result is a comment. + comment = + if comment == "" or String.starts_with?(comment, "#"), do: comment, else: "# " <> comment + body = String.replace_prefix(comment, "#", "") case Regex.run(~r/^\s*/, body) do @@ -608,7 +619,14 @@ defmodule Hypatia.Rules.PinIntegrity do |> Enum.map(&Regex.named_captures(@uses_regex, &1)) |> Enum.flat_map(fn %{"action" => action, "ref" => ref} = caps -> - [%{action: action, action_base: action_base(action), ref: ref, comment: Map.get(caps, "comment", "") || ""}] + [ + %{ + action: action, + action_base: action_base(action), + ref: ref, + comment: Map.get(caps, "comment", "") || "" + } + ] _ -> [] diff --git a/lib/rules/pr_automerge.ex b/lib/rules/pr_automerge.ex index 273629b1..ed916217 100644 --- a/lib/rules/pr_automerge.ex +++ b/lib/rules/pr_automerge.ex @@ -236,6 +236,9 @@ defmodule Hypatia.Rules.PrAutomerge do delta(old, new, nil, nil, :unresolved, "none") end |> Map.put(:file, filename_of(file)) + # flat_map needs a list: a bare map would be flattened into + # its {key, value} pairs. + |> List.wrap() end end end) @@ -270,7 +273,6 @@ defmodule Hypatia.Rules.PrAutomerge do @spec body_claims(String.t()) :: [map()] def body_claims(body) when is_binary(body) do ~r/(?:Updates|Bumps)\s+\[?`?([A-Za-z0-9_.-]+(?:\/[A-Za-z0-9_.-]+)*)`?\]?(?:\([^)]*\))?\s+from\s+([0-9][^\s]*)\s+to\s+([0-9][^\s]*)/ - |> Regex.scan(body) |> Enum.map(fn [_, action, from, to] -> # Dependabot ends the sentence with a full stop; the version does not @@ -290,17 +292,25 @@ defmodule Hypatia.Rules.PrAutomerge do author = field(pr, :author) repo_archived = field(pr, :repo_archived) == true - base = %{ - change_class: "bump", - change_level: if(scan.pin_only, do: "object", else: "meta"), - route: "Patch-Bridge", - method: "squash", - pool: "P2", - safety: "flag", - attestations: [ - %{bot: "hypatia", verdict: "approve", confidence: 0.9, rationale: "classified from the diff"} - ] - } + # The decision carries the scan it rests on (deltas, licence_touch, …), + # so decision_manifest/2 and any reviewer can see what it was decided from. + base = + Map.merge(scan, %{ + change_class: "bump", + change_level: if(scan.pin_only, do: "object", else: "meta"), + route: "Patch-Bridge", + method: "squash", + pool: "P2", + safety: "flag", + attestations: [ + %{ + bot: "hypatia", + verdict: "approve", + confidence: 0.9, + rationale: "classified from the diff" + } + ] + }) cond do repo_archived -> @@ -313,10 +323,16 @@ defmodule Hypatia.Rules.PrAutomerge do reject(base, :close_poison_and_majors, "introduces_denylisted_pin_and_major_bumps", "P1") scan.poison_sites != [] and scan.pin_only -> - accept(base, :close_poison_only, "introduces_denylisted_pin", "P1") + # Closing is not merging: a poisoned pin must never arm automerge. + reject(base, :close_poison_only, "introduces_denylisted_pin", "P1") scan.poison_sites != [] -> - accept(base, :excise_poison_then_merge, "introduces_denylisted_pin_alongside_wanted_updates", "P1") + accept( + base, + :excise_poison_then_merge, + "introduces_denylisted_pin_alongside_wanted_updates", + "P1" + ) scan.major_delta -> reject(base, :flag, "major_version_delta", "P2") @@ -351,7 +367,12 @@ defmodule Hypatia.Rules.PrAutomerge do defp accept(base, disposition, blocked_by, pool) do base - |> Map.merge(%{safety: "arm_auto", pool: pool, disposition: disposition, blocked_by: blocked_by}) + |> Map.merge(%{ + safety: "arm_auto", + pool: pool, + disposition: disposition, + blocked_by: blocked_by + }) end defp reject(base, disposition, blocked_by, pool) do @@ -373,9 +394,9 @@ defmodule Hypatia.Rules.PrAutomerge do def decision_manifest(decision, pr) do vetoes = if decision.safety == "flag" do - [%{bot: "Patch-Bridge", reason: decision.blocked_by}] ++ + [%{"bot" => "Patch-Bridge", "reason" => decision.blocked_by}] ++ if decision.change_level == "meta", - do: [%{bot: "hypatia", reason: "change_level=meta"}], + do: [%{"bot" => "hypatia", "reason" => "change_level=meta"}], else: [] else [] @@ -449,8 +470,11 @@ defmodule Hypatia.Rules.PrAutomerge do |> Enum.reject(&Regex.match?(~r/^[-+]{3}/, &1)) end - defp added_lines(file), do: file |> patch_of() |> content_lines() |> Enum.filter(&String.starts_with?(&1, "+")) - defp removed_lines(file), do: file |> patch_of() |> content_lines() |> Enum.filter(&String.starts_with?(&1, "-")) + defp added_lines(file), + do: file |> patch_of() |> content_lines() |> Enum.filter(&String.starts_with?(&1, "+")) + + defp removed_lines(file), + do: file |> patch_of() |> content_lines() |> Enum.filter(&String.starts_with?(&1, "-")) defp patch_of(file), do: Map.get(file, :patch) || Map.get(file, "patch") || "" diff --git a/test/rules/pr_automerge_test.exs b/test/rules/pr_automerge_test.exs index 645b7b2d..acd3ad97 100644 --- a/test/rules/pr_automerge_test.exs +++ b/test/rules/pr_automerge_test.exs @@ -99,7 +99,8 @@ defmodule Hypatia.Rules.PrAutomergeTest do end test "a body claim is enough when the refs cannot be resolved" do - body = "Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.7 to 4.2.0.\n" + body = + "Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.7 to 4.2.0.\n" decision = PA.classify( @@ -280,11 +281,12 @@ defmodule Hypatia.Rules.PrAutomergeTest do test "a patch that also edits a permissions block is not pin-only" do patch = """ - @@ -1,4 +1,4 @@ + @@ -1,2 +1,3 @@ - - uses: github/codeql-action/init@#{@good} # v4.38.0 + - uses: github/codeql-action/init@#{@poison} # v4.38.1 - permissions: {} - contents: read + - permissions: {} + + permissions: + + contents: read """ refute PA.pin_lines_only?(file(".github/workflows/codeql.yml", patch)) @@ -306,7 +308,11 @@ defmodule Hypatia.Rules.PrAutomergeTest do claims = PA.body_claims(body) - assert Enum.any?(claims, &(&1.action == "actions/checkout" and &1.from == "4.1.7" and &1.to == "4.2.0")) + assert Enum.any?( + claims, + &(&1.action == "actions/checkout" and &1.from == "4.1.7" and &1.to == "4.2.0") + ) + assert Enum.any?(claims, &(&1.action == "github/codeql-action/init" and &1.to == "4.38.1")) end