From bd05fcffccb3112812ad2fcf3978ef4f4362bdd9 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 8 Sep 2026 07:33:15 +0100 Subject: [PATCH] feat(rules): wire ResearchExtensions (RE001-RE010) into the scan path RE001-RE010 merged in PR #325 on 2026-05-26 with 27 passing tests and no caller anywhere in lib/. rules.ex:32 still carried # alias Hypatia.Rules.ResearchExtensions # wired in follow-up after PR #325 merges and rules.ex:690 a matching "delegate added in follow-up once PR #325 lands on main". That precondition was met 105 days ago. Ten working security rules had never run against a repository. This is the connection, not new rules. Four wiring points: rules.ex:31 uncomment the alias rules.ex:706 @doc + defdelegate scan_research_extensions/2 rules.ex:729 add to scan_all_estate_policies/2's parts list cli.ex:48 :research_extensions in @all_rule_modules, plus a normalization branch in collect_findings/2 Only the cli.ex path executes today: scan_all_estate_policies/2 has no caller in lib/ or test/, and its four sibling families (BaselineHealth, WorkflowHardening, SupplyChain, BranchProtection) appear nowhere in cli.ex. Adding RE to the facade alone would have reproduced the exact defect this commit fixes. Both surfaces are wired. Two seam defects had to be fixed or the wiring would have been cosmetic. 1. The :warn tier was discarded after the rules ran. Six of the ten RE rules emit `severity: :warn`. "warn" was absent from cli.ex's @severity_order, so `Map.get(@severity_order, "warn", 5)` gave it rank 5, and the filter `rank <= threshold` at the default `--severity medium` (3) dropped every one of them. Measured on this repository: 22 of 23 findings are warn-tier, so 96% of the new output would have evaporated one function after being computed. "warn" is now ranked with medium, and SARIF maps it to the "warning" level rather than the "note" catch-all. This also un-drops workflow_audit's one warn finding, which had the same fate. 2. :line was dropped at the normalization seam. RE004, RE005 and RE008 compute real line numbers but nest them under `:detail`, so a plain `Map.get(f, :line)` returns nil and SARIF falls back to startLine 1. The branch reads `get_in(f, [:detail, :line]) || Map.get(f, :line)`, which tolerates the seven rules that carry no line at all. Gates, all re-run after rebasing onto 0e91342: mix compile --force rc=0, 0 warnings tree-wide, 0 attributable to cli.ex, sarif.ex or rules/rules.ex mix test research_extensions_test 27 tests, 0 failures (unchanged) mix test (new wiring test) 8 tests, 0 failures mix test (full) 1481 tests, 1 failure, 242 excluded The single failure is ActionsLockTest:277 "parses the repository's authoritative generated lockfile", which arrived with #741. Controlled: a clean detached checkout of origin/main 0e91342 without this commit runs that file at 13 tests, 1 failure - identical. Pre-existing, and untouched by this branch, which changes no file under lib/rules/actions_lock.ex. End-to-end against this repository at default severity: ./hypatia scan . --rules research_extensions --format json 23 findings, rule_module: research_extensions on all 23 RE001 x13, RE004 x4, RE005 x5, RE008 x1 severity: 22 warn, 1 critical 10 carry a real line; SARIF startLines span 11 distinct values to 701 That the module filter holds is itself the proof of @all_rule_modules membership: parse_rules/1 silently falls back to every rule when a name does not resolve, so a mis-wired atom would have returned all modules. The critical finding is real: RE008 flags .github/workflows/dependabot-automerge.yml:56 gating on `github.actor == 'dependabot[bot]'`, which an attacker controls on pull_request_target from a fork. C1/C2 triage, since two RE rules touch pinning. RE004 concerns docker:// image tags and sha256 digests, outside actions.lock's remit entirely. RE006 advises replacing an unpinned nested `uses:` inside a composite action's action.yml with a 40-char SHA - the same shape of advice rejected from Codacy under C1. It is wired because composite action.yml files are not covered by actions.lock, and because it is advisory only: severity :warn gives it dispatch confidence 0.75, below the 0.80 propose-PR threshold, so it can alert but never open a pinning PR. RE006 did not fire on this repository. Expect RE001 to fire broadly across the estate - it flags any workflow touching secrets.* without step-security/harden-runner. Also adds `research_extensions` to both `--rules` help listings in cli.ex. The parser fails open - an unrecognised rule name filters to an empty list and silently runs *every* rule - so an undocumented module is not merely a docs gap: a user guessing the name gets a full scan and no error. Co-Authored-By: Claude Opus 5 --- lib/hypatia/cli.ex | 43 +++++- lib/hypatia/sarif.ex | 1 + lib/rules/rules.ex | 18 ++- test/research_extensions_wiring_test.exs | 160 +++++++++++++++++++++++ 4 files changed, 214 insertions(+), 8 deletions(-) create mode 100644 test/research_extensions_wiring_test.exs diff --git a/lib/hypatia/cli.ex b/lib/hypatia/cli.ex index 8857ca00..02836132 100644 --- a/lib/hypatia/cli.ex +++ b/lib/hypatia/cli.ex @@ -24,7 +24,8 @@ defmodule Hypatia.CLI do --rules Comma-separated rule modules to run (default: all) Available: root_hygiene,honest_completion,workflow_audit, - cicd_rules,code_safety,migration_rules,scorecard, + cicd_rules,research_extensions, + code_safety,migration_rules,scorecard, green_web,git_state,dependabot_alerts, secret_scanning_alerts,code_scanning_alerts, structural_drift,implementation_inside_canon @@ -45,6 +46,7 @@ defmodule Hypatia.CLI do :honest_completion, :workflow_audit, :cicd_rules, + :research_extensions, :code_safety, :migration_rules, :scorecard, @@ -61,6 +63,11 @@ defmodule Hypatia.CLI do "critical" => 1, "high" => 2, "medium" => 3, + # `:warn` is emitted by research_extensions (6 rules) and workflow_audit. + # Absent from this map it fell to the `Map.get/3` default of 5, so the + # `rank <= threshold` filter below dropped every warn finding at the + # default `--severity medium`. Ranked with medium: warn IS medium-tier. + "warn" => 3, "low" => 4, "info" => 5 } @@ -530,6 +537,36 @@ defmodule Hypatia.CLI do results end + # Research Extensions (RE001-RE010) - Snyk/StepSecurity/Endor/academic + results = + if :research_extensions in rules do + case Hypatia.Rules.ResearchExtensions.scan(repo_path) do + %{findings: findings} -> + normalized = + Enum.map(findings, fn f -> + %{ + rule_module: "research_extensions", + severity: to_string(f.severity), + type: f.rule, + file: Map.get(f, :file, "."), + # RE004 carries its line under `:detail`; the rest carry + # none. Both shapes degrade to nil, which SARIF renders + # as startLine 1 exactly as before. + line: get_in(f, [:detail, :line]) || Map.get(f, :line), + reason: f.reason, + action: to_string(f.action) + } + end) + + results ++ normalized + + _ -> + results + end + else + results + end + # Code Safety results = if :code_safety in rules do @@ -1297,8 +1334,8 @@ defmodule Hypatia.CLI do OPTIONS: --rules, -r Comma-separated rule modules (default: all) Available: root_hygiene,honest_completion, - workflow_audit,cicd_rules,code_safety, - migration_rules,scorecard,green_web, + workflow_audit,cicd_rules,research_extensions, + code_safety,migration_rules,scorecard,green_web, git_state,dependabot_alerts, secret_scanning_alerts,code_scanning_alerts, structural_drift,implementation_inside_canon diff --git a/lib/hypatia/sarif.ex b/lib/hypatia/sarif.ex index 96a6c11e..e2c38593 100644 --- a/lib/hypatia/sarif.ex +++ b/lib/hypatia/sarif.ex @@ -146,6 +146,7 @@ defmodule Hypatia.SARIF do defp level_for("critical"), do: "error" defp level_for("high"), do: "error" defp level_for("medium"), do: "warning" + defp level_for("warn"), do: "warning" defp level_for(_), do: "note" defp rel_uri("", _root), do: "." diff --git a/lib/rules/rules.ex b/lib/rules/rules.ex index 412b8ef4..af9a0b79 100644 --- a/lib/rules/rules.ex +++ b/lib/rules/rules.ex @@ -28,7 +28,7 @@ defmodule Hypatia.Rules do alias Hypatia.Rules.WorkflowHardening alias Hypatia.Rules.SupplyChain alias Hypatia.Rules.BranchProtection - # alias Hypatia.Rules.ResearchExtensions # wired in follow-up after PR #325 merges + alias Hypatia.Rules.ResearchExtensions @doc """ Run a comprehensive scan on a file's content given its path and language. @@ -694,9 +694,16 @@ defmodule Hypatia.Rules do """ defdelegate scan_branch_protection(owner, repo), to: BranchProtection, as: :scan - # ResearchExtensions (RE001-RE010) delegate added in follow-up once - # PR #325 lands on main. The facade for the other four families is - # below. + @doc """ + Run research-extension checks (RE001-RE010) drawn from Snyk, + StepSecurity, Endor Labs and the academic supply-chain literature: + absent/audit-only harden-runner, cache-key poisoning via head_ref, + tag-pinned containers, exit-swallowing test steps, unpinned nested + composite uses, workflow-level secret env, spoofable bot gates, + fromJSON(secrets), and provenance-free workflow_run artifacts. + Pure local file scan - no GitHub API. + """ + defdelegate scan_research_extensions(repo_path, opts \\ []), to: ResearchExtensions, as: :scan @doc """ Run every estate-policy rule available against a repository in one @@ -718,7 +725,8 @@ defmodule Hypatia.Rules do parts = [ BaselineHealth.scan(repo_path, opts), WorkflowHardening.scan(repo_path, opts), - SupplyChain.scan(repo_path, opts) + SupplyChain.scan(repo_path, opts), + ResearchExtensions.scan(repo_path, opts) ] parts = diff --git a/test/research_extensions_wiring_test.exs b/test/research_extensions_wiring_test.exs new file mode 100644 index 00000000..88b738be --- /dev/null +++ b/test/research_extensions_wiring_test.exs @@ -0,0 +1,160 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) + +defmodule Hypatia.Rules.ResearchExtensionsWiringTest do + @moduledoc """ + RE001-RE010 landed in PR #325 (2026-05-26) with a full test suite and + no caller. `test/research_extensions_test.exs` proves the *rules* work; + these tests prove the *wiring*, which is what was missing: + + * `Hypatia.CLI.collect_findings/2` actually reaches the module, + * the normalized finding shape survives the seam, + * RE004's line - which the module nests under `:detail` - is carried + through rather than dropped, + * the `:warn` tier renders as a real SARIF level instead of falling + to the catch-all. + + Without these, a future refactor can unwire the module and every rule + test still passes. + """ + use ExUnit.Case, async: true + + alias Hypatia.CLI + alias Hypatia.Rules + alias Hypatia.SARIF + + @tmp_dir System.tmp_dir!() + + # Trips RE001 (touches `secrets.*` with no harden-runner; :warn, no line) + # and RE004 (`docker://` pinned by tag; :warn, line nested under :detail). + @tripwire """ + name: Deploy + on: [push] + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - uses: docker://alpine:3.21 + - run: deploy --token=${{ secrets.DEPLOY_KEY }} + """ + + defp tripwire_repo do + repo = Path.join(@tmp_dir, "re_wiring_#{System.unique_integer([:positive])}") + wf = Path.join([repo, ".github", "workflows"]) + File.mkdir_p!(wf) + File.write!(Path.join(wf, "deploy.yml"), @tripwire) + on_exit(fn -> File.rm_rf!(repo) end) + repo + end + + defp re004(repo) do + repo + |> CLI.collect_findings([:research_extensions]) + |> Enum.find(&(&1.type == "RE004")) + end + + describe "CLI.collect_findings/2 reaches ResearchExtensions" do + test "the :research_extensions branch emits normalized findings" do + findings = CLI.collect_findings(tripwire_repo(), [:research_extensions]) + + refute findings == [], + "collect_findings/2 returned nothing for :research_extensions - the branch is unwired" + + assert Enum.all?(findings, &(&1.rule_module == "research_extensions")) + assert Enum.all?(findings, &is_binary(&1.severity)) + assert Enum.all?(findings, &is_binary(&1.action)) + assert "RE004" in Enum.map(findings, & &1.type) + end + + test "a different rule module does not emit research_extensions findings" do + findings = CLI.collect_findings(tripwire_repo(), [:code_safety]) + refute Enum.any?(findings, &(&1.rule_module == "research_extensions")) + end + + test "a repo with no workflows produces no research_extensions findings" do + clean = Path.join(@tmp_dir, "re_clean_#{System.unique_integer([:positive])}") + File.mkdir_p!(clean) + on_exit(fn -> File.rm_rf!(clean) end) + + assert CLI.collect_findings(clean, [:research_extensions]) == [] + end + end + + describe ":line carry-through" do + test "RE004's line, nested under :detail, survives normalization" do + f = re004(tripwire_repo()) + + assert f, "RE004 did not fire on the tripwire workflow" + + assert is_integer(f.line) and f.line > 0, + "ResearchExtensions nests RE004's line under :detail; the " <> + "normalizer must reach it, not just Map.get(f, :line)" + end + + test "the carried line renders as a non-degenerate SARIF startLine" do + repo = tripwire_repo() + f = re004(repo) + + [result] = + [f] + |> SARIF.from_findings(repo) + |> Map.fetch!("runs") + |> hd() + |> Map.fetch!("results") + + start_line = + get_in(result, ["locations", Access.at(0), "physicalLocation", "region", "startLine"]) + + assert start_line == f.line + + refute start_line == 1, + "startLine 1 is the fallback SARIF uses when :line is absent - " <> + "the line was lost at the normalization seam" + end + end + + describe "the :warn tier is not discarded" do + # Six of the ten RE rules emit `severity: :warn`. "warn" was absent from + # CLI's @severity_order, so `Map.get(@severity_order, "warn", 5)` gave it + # rank 5; the filter `rank <= threshold` at the default threshold of + # "medium" (3) then dropped every one of them *after* the rules had run. + # SARIF's level mapping mirrors that ranking, so it guards the same fix. + test "warn maps to the SARIF warning level, not the note catch-all" do + [result] = + [ + %{ + severity: "warn", + rule_module: "research_extensions", + type: "RE001", + file: ".github/workflows/deploy.yml", + reason: "no harden-runner" + } + ] + |> SARIF.from_findings("/tmp") + |> Map.fetch!("runs") + |> hd() + |> Map.fetch!("results") + + assert result["level"] == "warning", + "warn fell through to the catch-all - it is being treated as " <> + "lower than info by every consumer that ranks severities" + end + end + + describe "Rules facade" do + test "scan_research_extensions/2 delegates to the module" do + result = Rules.scan_research_extensions(tripwire_repo()) + + assert %{findings: _, total: total, by_severity: _, dispatch: _} = result + assert total > 0 + end + + test "scan_all_estate_policies/2 includes the research-extension family" do + %{findings: findings} = Rules.scan_all_estate_policies(tripwire_repo()) + + assert Enum.any?(findings, &(Map.get(&1, :rule) in ["RE001", "RE004"])), + "the estate-policy facade documents 'every estate-policy rule' " <> + "but omitted RE001-RE010" + end + end +end