From 3637b7fea310f4cecb8ab54f94fdc9bdcf4294f0 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:34:02 +0100 Subject: [PATCH 1/8] =?UTF-8?q?fix(rules):=20CSA006=20always=20returned=20?= =?UTF-8?q?[]=20=E2=80=94=20every=20finding=20was=20discarded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `csa006_scorecard_config_advice/2` built its findings with `findings = [f | findings]` inside an `if`, in both arms. Elixir's `if` does not leak bindings, so each assignment was dead and the function returned the outer `findings` — always `[]`. This is a LIVE defect, not latent: `:code_scanning_alerts` is in the default rule list (cli.ex:57), `cli.ex:806` calls `scan_from_path/1`, and `hypatia-scan-reusable.yml` exports GITHUB_TOKEN to the scan step, so the rule runs on all 449 consumers and has never produced a finding. It compiled clean, emitting only "variable findings is unused" — which is why `--warnings-as-errors` is worth restoring once these are cleared. Cure is structural, not an underscore. Each arm is now a function returning a list and the lists are concatenated, so the defect is unrepresentable rather than merely absent. The network calls (`fetch_repo_info/2`, `has_single_contributor?/2`) stay behind the same cheap open-alert checks as before, so request behaviour is unchanged. The decision logic is split into pure public functions so the regression test can assert a NON-EMPTY result. That matters: a test asserting only `== []` passes against the broken version, which is exactly how this survived. Controls run — the old shape returns `[]` for the same input, and the new tests fail against the old module. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011YTttTxnPc1V2sATvzBDN7 --- lib/rules/code_scanning_alerts.ex | 220 ++++++++++++++++++----------- test/code_scanning_alerts_test.exs | 86 +++++++++++ 2 files changed, 220 insertions(+), 86 deletions(-) diff --git a/lib/rules/code_scanning_alerts.ex b/lib/rules/code_scanning_alerts.ex index 4fa74a98..32505ba9 100644 --- a/lib/rules/code_scanning_alerts.ex +++ b/lib/rules/code_scanning_alerts.ex @@ -18,11 +18,11 @@ defmodule Hypatia.Rules.CodeScanningAlerts do Rule IDs: CSA001-CSA006 ### Scorecard False Positive Handling (CSA005-CSA006) - + CSA005: Auto-detects Scorecard MaintainedID and CodeReviewID alerts that are known false positives (repos <90 days old, single-contributor repos). These are reported as low-severity findings to track the noise without escalating. - + CSA006: Provides configuration advice for repos with Scorecard structural issues (e.g., single-contributor repos with CodeReviewID alerts). Helps maintainers understand why these alerts appear and what (if anything) can @@ -50,7 +50,8 @@ defmodule Hypatia.Rules.CodeScanningAlerts do @accepted_dismissals ~w(false\ positive used\ in\ tests won't\ fix) # Scorecard-specific checks that can be auto-dismissed - @scorecard_false_positive_checks ~w(MaintainedID CodeReviewID) # Checks that often produce false positives + # Checks that often produce false positives + @scorecard_false_positive_checks ~w(MaintainedID CodeReviewID) # ─── CSA001: Open code-scanning alerts ───────────────────────────────── @@ -454,13 +455,13 @@ defmodule Hypatia.Rules.CodeScanningAlerts do @doc """ CSA005: Auto-dismiss Scorecard alerts that are known false positives. - + MaintainedID: Scorecard gives score 0 to repos <90 days old. This is expected behavior and will auto-resolve after 90 days. - + CodeReviewID: Single-contributor repos cannot have code review. This is a structural limitation, not a security issue. - + This rule proactively dismisses these alerts to reduce noise in the security tab. """ def csa005_scorecard_false_positives(owner, repo) do @@ -473,20 +474,22 @@ defmodule Hypatia.Rules.CodeScanningAlerts do |> Enum.map(fn alert -> rule_id = get_in(alert, ["rule", "id"]) number = alert["number"] - + # Build dismissal reason and comment based on rule_id - {reason, comment} = + {reason, comment} = case rule_id do - "MaintainedID" -> - {"false positive", + "MaintainedID" -> + {"false positive", "Repository is less than 90 days old. Scorecard Maintained check gives score 0 for new projects. This is expected behavior and will auto-resolve after 90 days."} + "CodeReviewID" -> {"won't fix", "Single-contributor repository. Code review requires multiple human contributors. This is a structural limitation of the project, not a security issue."} + _ -> {"false positive", "Scorecard false positive - auto-dismissed by Hypatia"} end - + %{ rule: "CSA005", file: "#{owner}/#{repo}", @@ -514,7 +517,7 @@ defmodule Hypatia.Rules.CodeScanningAlerts do @doc """ CSA006: Meta-finding when a repo has Scorecard alerts that could be prevented by configuration changes (e.g., single-contributor repos with CodeReviewID). - + This helps repository maintainers understand structural issues that generate recurring alerts. """ @@ -522,88 +525,124 @@ defmodule Hypatia.Rules.CodeScanningAlerts do case fetch_alerts(owner, repo) do {:ok, alerts} -> # Count Scorecard alerts by rule_id - by_rule = + by_rule = alerts |> Enum.filter(&(get_in(&1, ["tool", "name"]) == "Scorecard")) |> Enum.group_by(&get_in(&1, ["rule", "id"])) - - findings = [] - - # Check for MaintainedID alerts (repo <90 days old) - if Map.has_key?(by_rule, "MaintainedID") do - maintained_alerts = Map.get(by_rule, "MaintainedID") - open_maintained = Enum.filter(maintained_alerts, &(&1["state"] == "open")) - - if length(open_maintained) > 0 do - # Get repo creation date - repo_info = fetch_repo_info(owner, repo) - created_at = repo_info["created_at"] - - # Check if repo is <90 days old - if is_repo_less_than_90_days?(created_at) do - findings = [ - %{ - rule: "CSA006", - file: "#{owner}/#{repo}", - severity: :info, - reason: "Repository has Scorecard MaintainedID alerts but is <90 days old - these are expected and will auto-resolve", - action: :inform, - detail: %{ - alert_count: length(open_maintained), - repo_created_at: created_at, - suggestion: "No action needed. These alerts will disappear after 90 days." - } - } - | findings - ] - end - end - end - - # Check for CodeReviewID alerts in single-contributor repos - if Map.has_key?(by_rule, "CodeReviewID") do - code_review_alerts = Map.get(by_rule, "CodeReviewID") - open_code_review = Enum.filter(code_review_alerts, &(&1["state"] == "open")) - - if length(open_code_review) > 0 do - # Check if repo has only one human contributor - if has_single_contributor?(owner, repo) do - findings = [ - %{ - rule: "CSA006", - file: "#{owner}/#{repo}", - severity: :medium, - reason: "Repository has Scorecard CodeReviewID alerts but has only one human contributor - code review is impractical", - action: :configure, - detail: %{ - alert_count: length(open_code_review), - suggestion: "Add more contributors or accept that code review is not feasible for this project." - } - } - | findings - ] - end - end - end - - findings + + # Each arm returns a list, and the lists are concatenated. Do NOT + # reintroduce a `findings = [... | findings]` rebinding inside an `if`: + # Elixir's `if` does not leak bindings, so the assignment is dead and + # the function silently returns []. That was this rule's state from its + # introduction until 2026-09-14 — it compiled clean, emitted only an + # "unused variable" warning, and produced no findings on any of the 449 + # consumers. Returning lists makes the defect unrepresentable. + maintained_id_advice(by_rule, owner, repo) ++ + code_review_id_advice(by_rule, owner, repo) {:error, _} -> [] end end + # MaintainedID alerts on a repo younger than 90 days are expected and + # self-resolving. `fetch_repo_info/2` is a network call, so it stays behind + # the open-alert check exactly as before. + # Network at the edge, decision in the middle. `fetch_repo_info/2` and + # `has_single_contributor?/2` are the only network calls, and each stays + # behind the cheap open-alert check exactly as it was before. The decision + # functions below are pure, public and therefore testable without a token — + # which is what lets the regression test assert a NON-EMPTY result. The old + # shape could only ever be tested for `[]`, which is precisely why a rule + # that always returned `[]` went unnoticed. + defp maintained_id_advice(by_rule, owner, repo) do + open_maintained = open_alerts(by_rule, "MaintainedID") + + if open_maintained == [] do + [] + else + created_at = fetch_repo_info(owner, repo)["created_at"] + maintained_id_finding(open_maintained, created_at, owner, repo) + end + end + + defp code_review_id_advice(by_rule, owner, repo) do + open_code_review = open_alerts(by_rule, "CodeReviewID") + + if open_code_review == [] do + [] + else + code_review_id_finding(open_code_review, has_single_contributor?(owner, repo), owner, repo) + end + end + + defp open_alerts(by_rule, rule_id) do + by_rule + |> Map.get(rule_id, []) + |> Enum.filter(&(&1["state"] == "open")) + end + + @doc false + # MaintainedID alerts on a repo younger than 90 days are expected and + # self-resolving, so they are advice rather than a defect. + def maintained_id_finding(open_maintained, created_at, owner, repo) do + if is_repo_less_than_90_days?(created_at) do + [ + %{ + rule: "CSA006", + file: "#{owner}/#{repo}", + severity: :info, + reason: + "Repository has Scorecard MaintainedID alerts but is <90 days old - these are expected and will auto-resolve", + action: :inform, + detail: %{ + alert_count: length(open_maintained), + repo_created_at: created_at, + suggestion: "No action needed. These alerts will disappear after 90 days." + } + } + ] + else + [] + end + end + + @doc false + # CodeReviewID alerts on a single-contributor repo are structurally + # unsatisfiable rather than a lapse in process. + def code_review_id_finding(open_code_review, single_contributor?, owner, repo) do + if single_contributor? do + [ + %{ + rule: "CSA006", + file: "#{owner}/#{repo}", + severity: :medium, + reason: + "Repository has Scorecard CodeReviewID alerts but has only one human contributor - code review is impractical", + action: :configure, + detail: %{ + alert_count: length(open_code_review), + suggestion: + "Add more contributors or accept that code review is not feasible for this project." + } + } + ] + else + [] + end + end + # ─── Helpers ─────────────────────────────────────────────────────────── # Fetch repository info from GitHub API defp fetch_repo_info(owner, repo) do token = System.get_env("GITHUB_TOKEN") - + if token == nil or token == "" do %{"created_at" => "", "contributors" => []} else url = "#{@github_api_base}/repos/#{owner}/#{repo}" - + case System.cmd( "curl", [ @@ -624,7 +663,9 @@ defmodule Hypatia.Rules.CodeScanningAlerts do {:ok, info} -> info _ -> %{"created_at" => "", "contributors" => []} end - _ -> %{"created_at" => "", "contributors" => []} + + _ -> + %{"created_at" => "", "contributors" => []} end end end @@ -635,19 +676,21 @@ defmodule Hypatia.Rules.CodeScanningAlerts do {:ok, dt, _} -> days = DateTime.diff(DateTime.utc_now(), dt, :day) days < 90 - _ -> false + + _ -> + false end end # Check if repo has only one human contributor defp has_single_contributor?(owner, repo) do token = System.get_env("GITHUB_TOKEN") - + if token == nil or token == "" do false else url = "#{@github_api_base}/repos/#{owner}/#{repo}/contributors?anon=1" - + case System.cmd( "curl", [ @@ -667,19 +710,24 @@ defmodule Hypatia.Rules.CodeScanningAlerts do case Jason.decode(body) do {:ok, contributors} when is_list(contributors) -> # Filter out bots and check human contributors - human_contributors = + human_contributors = Enum.filter(contributors, fn c -> type = c["type"] || "" login = c["login"] || "" # Exclude bot accounts - !String.contains?(login, "[bot]") && - !String.ends_with?(login, "-bot") && - type != "Bot" + !String.contains?(login, "[bot]") && + !String.ends_with?(login, "-bot") && + type != "Bot" end) + length(human_contributors) <= 1 - _ -> false + + _ -> + false end - _ -> false + + _ -> + false end end end diff --git a/test/code_scanning_alerts_test.exs b/test/code_scanning_alerts_test.exs index e191a841..4e1c6a0b 100644 --- a/test/code_scanning_alerts_test.exs +++ b/test/code_scanning_alerts_test.exs @@ -138,4 +138,90 @@ defmodule Hypatia.Rules.CodeScanningAlertsTest do refute CodeScanningAlerts.self_referential_alert?(%{"rule" => %{"id" => "x"}}) end end + + # ── CSA006 regression ────────────────────────────────────────────────── + # + # Until 2026-09-14 `csa006_scorecard_config_advice/2` ALWAYS returned []. + # Both arms built their finding with `findings = [f | findings]` inside an + # `if`, and Elixir's `if` does not leak bindings, so every assignment was + # dead. It compiled clean and emitted only an "unused variable" warning. + # + # These tests assert the arms produce a NON-EMPTY list. A test that only + # asserted `== []` would have passed against the broken version — which is + # exactly how the defect survived. The decision functions are pure so the + # positive arm needs no GITHUB_TOKEN. + describe "csa006 decision functions (the always-empty regression)" do + @a_maintained [%{"state" => "open"}, %{"state" => "open"}] + @a_code_review [%{"state" => "open"}] + + defp iso_days_ago(n), + do: DateTime.utc_now() |> DateTime.add(-n * 86_400, :second) |> DateTime.to_iso8601() + + test "maintained_id_finding FIRES for a repo younger than 90 days" do + assert [finding] = + CodeScanningAlerts.maintained_id_finding( + @a_maintained, + iso_days_ago(10), + "hyperpolymath", + "brand-new" + ) + + assert finding.rule == "CSA006" + assert finding.severity == :info + assert finding.action == :inform + assert finding.file == "hyperpolymath/brand-new" + assert finding.detail.alert_count == 2 + end + + test "maintained_id_finding is silent for a repo older than 90 days" do + assert CodeScanningAlerts.maintained_id_finding( + @a_maintained, + iso_days_ago(400), + "hyperpolymath", + "old" + ) == [] + end + + test "maintained_id_finding is silent on an unparseable created_at" do + assert CodeScanningAlerts.maintained_id_finding( + @a_maintained, + "not-a-date", + "hyperpolymath", + "weird" + ) == [] + end + + test "code_review_id_finding FIRES for a single-contributor repo" do + assert [finding] = + CodeScanningAlerts.code_review_id_finding( + @a_code_review, + true, + "hyperpolymath", + "solo" + ) + + assert finding.rule == "CSA006" + assert finding.severity == :medium + assert finding.action == :configure + assert finding.detail.alert_count == 1 + end + + test "code_review_id_finding is silent when the repo has several contributors" do + assert CodeScanningAlerts.code_review_id_finding( + @a_code_review, + false, + "hyperpolymath", + "team" + ) == [] + end + end + + describe "csa006_scorecard_config_advice/2" do + test "returns empty list when GITHUB_TOKEN is not set" do + assert CodeScanningAlerts.csa006_scorecard_config_advice( + "hyperpolymath", + "test-nonexistent" + ) == [] + end + end end From 94e627b44d967fd88f247902c69357a2db1aebdd Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:35:35 +0100 Subject: [PATCH 2/8] docs(web): @doc before a Plug route macro is silently discarded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plug's `get "/path" do` expands to a clause of `defp do_match/4`, so a preceding `@doc` attaches to a private function and Elixir discards it — 8 warnings, and 8 blocks of genuinely useful endpoint documentation that have never appeared in the generated docs and never could. Converted those 8 to `#` comments. The prose is unchanged and is now actually visible where it matters, at the route. `api_router.ex`'s `@doc` on `protect/2` is left alone: that is a real public function and its `@doc` works. No behaviour change — comments and discarded attributes both compile to nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011YTttTxnPc1V2sATvzBDN7 --- lib/hypatia/web/api_router.ex | 24 ++++++--------- lib/hypatia/web/router.ex | 58 +++++++++++++++-------------------- 2 files changed, 34 insertions(+), 48 deletions(-) diff --git a/lib/hypatia/web/api_router.ex b/lib/hypatia/web/api_router.ex index 6d5ef850..4d8cb2ee 100644 --- a/lib/hypatia/web/api_router.ex +++ b/lib/hypatia/web/api_router.ex @@ -63,11 +63,9 @@ defmodule Hypatia.Web.ApiRouter do end end - @doc """ - GET /api/recipes/:id -- single-recipe drill-down. Returns the same - shape as one row from `/api/recipes`, plus the recipe definition - itself when found in the registry. - """ + # GET /api/recipes/:id -- single-recipe drill-down. Returns the same + # shape as one row from `/api/recipes`, plus the recipe definition + # itself when found in the registry. get "/recipes/:id" do health = Hypatia.OutcomeTracker.recipe_health() row = Enum.find(health, &(&1.recipe_id == id)) @@ -80,11 +78,9 @@ defmodule Hypatia.Web.ApiRouter do end end - @doc """ - GET /api/quarantine -- everything currently auto-quarantined: - recipes (verification-rate gate) and bots (consecutive-failure / - FP-rate gate from Hypatia.Safety.Quarantine). - """ + # GET /api/quarantine -- everything currently auto-quarantined: + # recipes (verification-rate gate) and bots (consecutive-failure / + # FP-rate gate from Hypatia.Safety.Quarantine). get "/quarantine" do recipes = Hypatia.OutcomeTracker.recipe_health() @@ -102,11 +98,9 @@ defmodule Hypatia.Web.ApiRouter do }) end - @doc """ - GET /api/alerts -- Recent threshold-rule alerts emitted by - Hypatia.Watcher.Alerts (ring buffer, newest first). Powers the - dashboard alert ribbon and supports manual triage. - """ + # GET /api/alerts -- Recent threshold-rule alerts emitted by + # Hypatia.Watcher.Alerts (ring buffer, newest first). Powers the + # dashboard alert ribbon and supports manual triage. get "/alerts" do rows = case Process.whereis(Hypatia.Watcher.Alerts) do diff --git a/lib/hypatia/web/router.ex b/lib/hypatia/web/router.ex index 1faefc96..24e0e596 100644 --- a/lib/hypatia/web/router.ex +++ b/lib/hypatia/web/router.ex @@ -28,20 +28,16 @@ defmodule Hypatia.Web.Router do plug(:match) plug(:dispatch) - @doc """ - GET / -- Single-page live operational dashboard. HTML + vanilla JS, - polls /api/status and EventSource-streams /api/events. The dashboard - itself is publicly reachable; the data endpoints it calls are - loopback-only (gated in ApiRouter), so a non-local browser would - render the chrome but get 403 from the XHR/SSE calls. - """ + # GET / -- Single-page live operational dashboard. HTML + vanilla JS, + # polls /api/status and EventSource-streams /api/events. The dashboard + # itself is publicly reachable; the data endpoints it calls are + # loopback-only (gated in ApiRouter), so a non-local browser would + # render the chrome but get 403 from the XHR/SSE calls. get "/" do Hypatia.Web.Dashboard.call(conn, []) end - @doc """ - GET /health -- Basic health check for the HTTP endpoint. - """ + # GET /health -- Basic health check for the HTTP endpoint. get "/health" do health = %{ status: "ok", @@ -54,27 +50,24 @@ defmodule Hypatia.Web.Router do |> send_resp(200, Jason.encode!(health)) end - @doc """ - GET /metrics -- Prometheus text-format exposition. Publicly - reachable (NOT loopback-only) because scrapers routinely run on a - different host; there's no operational data in the metric body - that isn't already implied by the dashboard's existence. - """ + # GET /metrics -- Prometheus text-format exposition. Publicly + # reachable (NOT loopback-only) because scrapers routinely run on a + # different host; there's no operational data in the metric body + # that isn't already implied by the dashboard's existence. get "/metrics" do Hypatia.Web.Metrics.call(conn, []) end - @doc """ - GET /metrics/snapshot -- Compact JSON snapshot of estate-level - counters: repos scanned, weak points, dispatched actions, outcomes, - recipes, average confidence. Consumed by the optional Ada TUI - (`lib/tui/port.ex`) on its 10s tick, and useful as a single-call - status read for external dashboards. + # GET /metrics/snapshot -- Compact JSON snapshot of estate-level + # counters: repos scanned, weak points, dispatched actions, outcomes, + # recipes, average confidence. Consumed by the optional Ada TUI + # (`lib/tui/port.ex`) on its 10s tick, and useful as a single-call + # status read for external dashboards. - Reads from the verisim-data flat-file store via VerisimConnector; - any failure returns a degraded snapshot with status="degraded" - rather than 500, so the TUI keeps rendering. - """ + # + # Reads from the verisim-data flat-file store via VerisimConnector; + # any failure returns a degraded snapshot with status="degraded" + # rather than 500, so the TUI keeps rendering. get "/metrics/snapshot" do snapshot = Hypatia.Web.MetricsSnapshot.build() @@ -88,14 +81,13 @@ defmodule Hypatia.Web.Router do # reachable for container orchestrators. forward("/api", to: Hypatia.Web.ApiRouter) - @doc """ - POST /graphql -- GraphQL-shaped query endpoint (M14). + # POST /graphql -- GraphQL-shaped query endpoint (M14). - Minimal hand-rolled implementation; no introspection, no schema - federation, no Absinthe dep. See lib/hypatia/web/graphql.ex for - the supported field set and limitations. Loopback-only by sharing - the bearer-auth gate when HYPATIA_API_BEARER_TOKEN is configured. - """ + # + # Minimal hand-rolled implementation; no introspection, no schema + # federation, no Absinthe dep. See lib/hypatia/web/graphql.ex for + # the supported field set and limitations. Loopback-only by sharing + # the bearer-auth gate when HYPATIA_API_BEARER_TOKEN is configured. post "/graphql" do conn = Hypatia.Web.ApiRouter.protect(conn, []) From 4581dcc0a5bec3a8434f71b3cd4de45e09b4dace Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:36:32 +0100 Subject: [PATCH 3/8] =?UTF-8?q?refactor(rules):=20delete=20secret=5Fscanne?= =?UTF-8?q?r=5Fverification.ex=20=E2=80=94=20dead=20and=20unsalvageable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSV001-SSV003 have never run. `Hypatia.Rules.SecretScannerVerification` is referenced nowhere outside itself: not in the default rule list, not in `cli.ex`, not in the soundness manifest, and it has no test file. Only a comment in this repo's own soundness workflow mentions it. That is fortunate, because it does not work. Seven independent defects in 218 lines, all of which would fire on the first real invocation: 1. `System.cmd(...) |> elem(0) == 0` (twice, SSV001) compares the OUTPUT STRING to the integer 0. Always false, so both installed-flags are permanently false and SSV001 can only ever report "no scanner found". 2. `System.cmd/3` is unguarded, so a missing `gitleaks` binary raises `ErlangError :enoent` — which is the normal case on a runner that has not installed it, i.e. the exact case SSV001 exists to detect. 3. `{exit, output, _}` at the old :41 and :163 destructures a 3-tuple. `System.cmd/3` returns a 2-tuple `{output, exit_status}` — wrong arity AND wrong order, so a MatchError on every call that reaches it. 4. The trufflehog fallback rebinds `exit`/`output` inside an `if` whose value is discarded, so it is inert and the following `if exit == 0` re-tests the original gitleaks status. (This is the defect #17 named.) 5. `File.mkdir_p!` + `File.write!` write `.hypatia-evidence/` INTO the repository being scanned. A scanner must not mutate its subject. 6. `detail:` is a map, which the cli normalizer cannot render. 7. `severity: :warning` is not a severity this codebase has; the ranking map knows `critical/high/medium/low/warn`, so the finding would be ranked at the default and silently dropped. Also `@min_rules_version` was set and never read — the version check the module's own docstring promises was never written. Repairing this would be a rewrite, and a rewrite is not wanted: the plan's Phase 1c supersedes the design. Asserting that the estate's real, pinned, checksum-verified gitleaks rail (`secret-scanner-reusable.yml`, 501 repos) is present and armed is both cheaper and more truthful than reimplementing scanner invocation in Elixir. Deleting is also the honest move under AGENTS.md §6: repaired dead code READS as a working capability while remaining unproven, which is worse than its absence. Kept in its own commit so it can be reverted independently of the rest of this PR. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011YTttTxnPc1V2sATvzBDN7 --- lib/rules/secret_scanner_verification.ex | 218 ----------------------- 1 file changed, 218 deletions(-) delete mode 100644 lib/rules/secret_scanner_verification.ex diff --git a/lib/rules/secret_scanner_verification.ex b/lib/rules/secret_scanner_verification.ex deleted file mode 100644 index cd7aea97..00000000 --- a/lib/rules/secret_scanner_verification.ex +++ /dev/null @@ -1,218 +0,0 @@ -# SPDX-License-Identifier: MPL-2.0 -# Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) - -defmodule Hypatia.Rules.SecretScannerVerification do - @moduledoc """ - Verification that the correct secrets scanner version is installed and operational. - - This rule checks that the estate-wide secrets scanner (gitleaks or trufflehog) is - properly installed, configured with current rules, and producing valid output. - It collects evidence that the verification was performed. - - Rule IDs: SSV001-SSV003 - """ - - require Logger - - @evidence_dir ".hypatia-evidence" - @scanner_config ".gitleaks.toml" - @min_rules_version "8.18.0" - - # ─── SSV001: Secrets scanner installation verification ────────────────────── - - @doc """ - SSV001: Verify that a secrets scanner is installed and accessible. - - Checks for the presence of gitleaks or trufflehog in the PATH and - verifies it can execute successfully. - """ - def ssv001_scanner_installed(_owner, _repo) do - # Check for gitleaks - gitleaks_installed = System.cmd("gitleaks", ["version"], stderr_to_stdout: true) |> elem(0) == 0 - - # Check for trufflehog - trufflehog_installed = System.cmd("trufflehog", ["--version"], stderr_to_stdout: true) |> elem(0) == 0 - - if gitleaks_installed || trufflehog_installed do - scanner = if gitleaks_installed, do: "gitleaks", else: "trufflehog" - - # Get version - version_cmd = if gitleaks_installed, do: "version", else: "--version" - {exit, version_output, _} = System.cmd(scanner, [version_cmd], stderr_to_stdout: true) - version = String.trim(version_output) - - # Write evidence - evidence = %{ - rule: "SSV001", - file: ".hypatia-evidence/secrets-scanner-installation.json", - severity: :info, - reason: "Secrets scanner '#{scanner}' is installed and accessible (version: #{version})", - action: :log, - detail: %{ - scanner: scanner, - version: version, - exit_code: exit, - verified_at: DateTime.utc_now() |> to_string() - } - } - - # Ensure evidence directory exists - File.mkdir_p!(@evidence_dir) - File.write!(".hypatia-evidence/secrets-scanner-installation.json", Jason.encode!(evidence)) - - [evidence] - else - [ - %{ - rule: "SSV001", - file: ".hypatia-evidence", - severity: :high, - reason: "No secrets scanner (gitleaks or trufflehog) found in PATH", - action: :escalate, - detail: %{ - checked: ["gitleaks", "trufflehog"], - verified_at: DateTime.utc_now() |> to_string() - } - } - ] - end - end - - # ─── SSV002: Secrets scanner configuration currency ──────────────────────── - - @doc """ - SSV002: Verify that the secrets scanner configuration is current. - - Checks that the scanner configuration file (e.g., .gitleaks.toml) exists and - contains rules that match the minimum required version. - """ - def ssv002_scanner_config_current(_owner, _repo) do - if File.exists?(@scanner_config) do - # Read config file - config_content = File.read!(@scanner_config) - - # Check for version indicator in config - has_version_info = String.contains?(config_content, "version") || - String.contains?(config_content, "gitleaks") - - # Check for common rule patterns - has_rules = String.contains?(config_content, "rules") || - String.contains?(config_content, "regex") - - if has_version_info && has_rules do - evidence = %{ - rule: "SSV002", - file: @scanner_config, - severity: :info, - reason: "Secrets scanner configuration appears current and contains rules", - action: :log, - detail: %{ - config_file: @scanner_config, - has_version_info: has_version_info, - has_rules: has_rules, - verified_at: DateTime.utc_now() |> to_string() - } - } - - File.mkdir_p!(@evidence_dir) - File.write!(".hypatia-evidence/secrets-scanner-config.json", Jason.encode!(evidence)) - - [evidence] - else - [ - %{ - rule: "SSV002", - file: @scanner_config, - severity: :medium, - reason: "Secrets scanner configuration may be outdated or incomplete", - action: :review, - detail: %{ - config_file: @scanner_config, - has_version_info: has_version_info, - has_rules: has_rules - } - } - ] - end - else - [ - %{ - rule: "SSV002", - file: ".hypatia-evidence", - severity: :medium, - reason: "Secrets scanner configuration file not found", - action: :review, - detail: %{ - expected_config: @scanner_config, - verified_at: DateTime.utc_now() |> to_string() - } - } - ] - end - end - - # ─── SSV003: Secrets scanner operational verification ────────────────────── - - @doc """ - SSV003: Verify that the secrets scanner can execute successfully on the repo. - - Runs a test scan on a sample of files to ensure the scanner is operational. - """ - def ssv003_scanner_operational(_owner, _repo) do - # Try to run gitleaks detect on the repo root - {exit, output, _} = System.cmd("gitleaks", ["detect", "--no-git", "--path", ".", "--exit-code", "0"], - stderr_to_stdout: true, cwd: ".") - - # Also try trufflehog if gitleaks fails - if exit != 0 do - {exit, output, _} = System.cmd("trufflehog", ["filesystem", ".", "--no-update"], - stderr_to_stdout: true, cwd: ".") - end - - if exit == 0 do - evidence = %{ - rule: "SSV003", - file: ".hypatia-evidence", - severity: :info, - reason: "Secrets scanner executed successfully on repository", - action: :log, - detail: %{ - exit_code: exit, - output_length: byte_size(output), - verified_at: DateTime.utc_now() |> to_string() - } - } - - File.mkdir_p!(@evidence_dir) - File.write!(".hypatia-evidence/secrets-scanner-operational.json", Jason.encode!(evidence)) - - [evidence] - else - [ - %{ - rule: "SSV003", - file: ".hypatia-evidence", - severity: :warning, - reason: "Secrets scanner execution failed (exit code: #{exit})", - action: :investigate, - detail: %{ - exit_code: exit, - output: String.slice(output, 0, 500), # Truncate for safety - verified_at: DateTime.utc_now() |> to_string() - } - } - ] - end - end - - # ─── Aggregate verification ──────────────────────────────────────────────── - - @doc """ - Run all secrets scanner verification checks. - """ - def verify_secrets_scanner(owner, repo) do - ssv001_scanner_installed(owner, repo) ++ - ssv002_scanner_config_current(owner, repo) ++ - ssv003_scanner_operational(owner, repo) - end -end From 81877098285c39727f1126cd1e66f3df21000835 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:39:49 +0100 Subject: [PATCH 4/8] ci(soundness): restore --warnings-as-errors now the 13 warnings are cleared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This file's own note designated "the PR that repairs the logic defects" as the place to turn this back on. This is that PR. Controls, both arms run: * main, before this branch: mix compile --force --warnings-as-errors -> exit 1 * this branch: mix compile --force --warnings-as-errors -> exit 0 so the flag can both fail and pass, rather than being decorative. The note is rewritten because it was wrong on two counts. It said five warnings; the measured inventory was 13 across 6 classes. And it blamed the discarded `exit`/`output` rebinding for making the trufflehog fallback inert — the fallback was never reached at all, because the preceding `System.cmd(...) |> elem(0) == 0` compares a string to an integer. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011YTttTxnPc1V2sATvzBDN7 --- .github/workflows/escript-soundness.yml | 45 +++++++++++++++---------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/.github/workflows/escript-soundness.yml b/.github/workflows/escript-soundness.yml index 4e01de56..05900f3d 100644 --- a/.github/workflows/escript-soundness.yml +++ b/.github/workflows/escript-soundness.yml @@ -107,28 +107,37 @@ jobs: working-directory: source run: mix deps.get - # NOTE: deliberately NOT --warnings-as-errors, and this is not laziness. - # Turning it on today makes this gate red on arrival, because five real - # warnings sit on main — invisible until now precisely because tests.yml, - # which carried that step, has never had a single successful run. + # --warnings-as-errors is ON, and it is load-bearing. # - # Two of the five are genuine logic defects, not lint: - # * lib/rules/code_scanning_alerts.ex:544,571 rebind `findings` INSIDE an - # `if` whose value is discarded, so CSA006 builds findings and throws - # them away. - # * lib/rules/secret_scanner_verification.ex:168 rebinds `exit`/`output` - # inside `if exit != 0`, so the trufflehog fallback is inert and the - # following `if exit == 0` still tests the ORIGINAL gitleaks exit. - # Both are the same shape as the CondClauseError this gate exists to catch: - # code that reads as though it acts and does nothing. + # It was deliberately omitted when this gate was first written, because + # 13 real warnings sat on main — invisible until then precisely because + # tests.yml, which carried that step, has never had a single successful + # run. Those 13 are now cleared, so the gate is armed. # - # Silencing them with an underscore would cement live bugs; repairing them - # changes scanner behaviour across 449 consumers and must be its own - # measured PR. Restore --warnings-as-errors in that PR, not this one. + # Two of them were logic defects rather than lint, and both are the + # shape this gate exists to catch: code that reads as though it acts and + # does nothing. + # * code_scanning_alerts.ex rebound `findings` INSIDE an `if` whose + # value is discarded, so CSA006 built findings and threw them away. + # It ran on all 449 consumers and never produced a finding. + # * secret_scanner_verification.ex was dead in seven independent ways + # at once (string-vs-integer comparison, unguarded System.cmd, + # a 3-tuple destructure of a 2-tuple, the discarded rebinding, and + # writing evidence into the scanned repo). It was referenced + # nowhere, so it was deleted rather than rewritten. + # + # An earlier version of this note said "five warnings" and blamed the + # trufflehog fallback's rebinding for making it inert. Both were wrong: + # the measured count was 13, and that module never reached the fallback + # because `System.cmd(...) |> elem(0) == 0` compares a string to an + # integer and is always false. + # + # If this step goes red, do NOT silence it with an underscore — an + # unused variable here has twice meant a discarded result. Fix the + # logic, or delete the code. - name: Compile working-directory: source - run: mix compile - + run: mix compile --warnings-as-errors # The gate itself. Builds the escript fresh and asserts every rule in # test/soundness/manifest.json fires at its expected severity against the # built binary — the packaging path, not the in-process one. From eb26c92424abb1e38d08567b2ab336faa991b305 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:22:05 +0000 Subject: [PATCH 5/8] fix(rules): preserve distinct CSA006 Scorecard advice --- lib/rules/code_scanning_alerts.ex | 19 ++++++++++++++----- test/code_scanning_alerts_test.exs | 23 +++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/rules/code_scanning_alerts.ex b/lib/rules/code_scanning_alerts.ex index 32505ba9..6082ab29 100644 --- a/lib/rules/code_scanning_alerts.ex +++ b/lib/rules/code_scanning_alerts.ex @@ -321,11 +321,7 @@ defmodule Hypatia.Rules.CodeScanningAlerts do csa005_scorecard_false_positives(owner, repo) ++ csa006_scorecard_config_advice(owner, repo) - deduped = - findings - |> Enum.uniq_by(fn f -> - {f.rule, Map.get(f.detail, :alert_number, f.file)} - end) + deduped = deduplicate_findings(findings) {:ok, %{ @@ -336,6 +332,17 @@ defmodule Hypatia.Rules.CodeScanningAlerts do end end + @doc false + def deduplicate_findings(findings) do + Enum.uniq_by(findings, fn finding -> + alert_number = Map.get(finding.detail, :alert_number) + + {finding.rule, + alert_number || + {finding.file, Map.get(finding.detail, :scorecard_check)}} + end) + end + @doc """ Scan from a local repo path -- extracts owner/repo from git remote. """ @@ -596,6 +603,7 @@ defmodule Hypatia.Rules.CodeScanningAlerts do "Repository has Scorecard MaintainedID alerts but is <90 days old - these are expected and will auto-resolve", action: :inform, detail: %{ + scorecard_check: "MaintainedID", alert_count: length(open_maintained), repo_created_at: created_at, suggestion: "No action needed. These alerts will disappear after 90 days." @@ -621,6 +629,7 @@ defmodule Hypatia.Rules.CodeScanningAlerts do "Repository has Scorecard CodeReviewID alerts but has only one human contributor - code review is impractical", action: :configure, detail: %{ + scorecard_check: "CodeReviewID", alert_count: length(open_code_review), suggestion: "Add more contributors or accept that code review is not feasible for this project." diff --git a/test/code_scanning_alerts_test.exs b/test/code_scanning_alerts_test.exs index 4e1c6a0b..f0688a2c 100644 --- a/test/code_scanning_alerts_test.exs +++ b/test/code_scanning_alerts_test.exs @@ -170,6 +170,7 @@ defmodule Hypatia.Rules.CodeScanningAlertsTest do assert finding.severity == :info assert finding.action == :inform assert finding.file == "hyperpolymath/brand-new" + assert finding.detail.scorecard_check == "MaintainedID" assert finding.detail.alert_count == 2 end @@ -203,6 +204,7 @@ defmodule Hypatia.Rules.CodeScanningAlertsTest do assert finding.rule == "CSA006" assert finding.severity == :medium assert finding.action == :configure + assert finding.detail.scorecard_check == "CodeReviewID" assert finding.detail.alert_count == 1 end @@ -216,6 +218,27 @@ defmodule Hypatia.Rules.CodeScanningAlertsTest do end end + describe "deduplicate_findings/1" do + test "keeps CSA006 advice for distinct Scorecard checks without alert numbers" do + maintained = %{ + rule: "CSA006", + file: "hyperpolymath/example", + detail: %{scorecard_check: "MaintainedID"} + } + + code_review = %{ + rule: "CSA006", + file: "hyperpolymath/example", + detail: %{scorecard_check: "CodeReviewID"} + } + + assert CodeScanningAlerts.deduplicate_findings([maintained, code_review]) == [ + maintained, + code_review + ] + end + end + describe "csa006_scorecard_config_advice/2" do test "returns empty list when GITHUB_TOKEN is not set" do assert CodeScanningAlerts.csa006_scorecard_config_advice( From 8d9717fcebc01354878db358ecb64bbb324e6a01 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:23:51 +0000 Subject: [PATCH 6/8] docs(rules): document CSA006 advisory builders --- lib/rules/code_scanning_alerts.ex | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/rules/code_scanning_alerts.ex b/lib/rules/code_scanning_alerts.ex index 6082ab29..69ba1cd3 100644 --- a/lib/rules/code_scanning_alerts.ex +++ b/lib/rules/code_scanning_alerts.ex @@ -589,9 +589,12 @@ defmodule Hypatia.Rules.CodeScanningAlerts do |> Enum.filter(&(&1["state"] == "open")) end - @doc false - # MaintainedID alerts on a repo younger than 90 days are expected and - # self-resolving, so they are advice rather than a defect. + @doc """ + Builds CSA006 advice for open MaintainedID alerts on a new repository. + + Returns a single informational finding when the repository is less than + 90 days old, or an empty list when the alert does not need advice. + """ def maintained_id_finding(open_maintained, created_at, owner, repo) do if is_repo_less_than_90_days?(created_at) do [ @@ -615,9 +618,12 @@ defmodule Hypatia.Rules.CodeScanningAlerts do end end - @doc false - # CodeReviewID alerts on a single-contributor repo are structurally - # unsatisfiable rather than a lapse in process. + @doc """ + Builds CSA006 advice for open CodeReviewID alerts on a repository. + + Returns a single configuration finding for a single-contributor repository, + or an empty list when code review is feasible. + """ def code_review_id_finding(open_code_review, single_contributor?, owner, repo) do if single_contributor? do [ From 1b68de917330eced22cfc42a5adad4cfcbcae4f3 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:53:27 +0100 Subject: [PATCH 7/8] Preserve distinct CSA006 advice and document its builders (#787) Fix CSA006 deduplication by keying unnumbered findings on repository and Scorecard check, preserving separate MaintainedID and CodeReviewID advice. Add check metadata, focused regression coverage, and expanded public-function documentation. Validation was not run in this checkout. [View coding task](https://app.coderabbit.ai/code/tasks/584a27d7-82d1-56d7-b599-1ead56900432?source=coding_agent_github_pr_description) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- lib/rules/code_scanning_alerts.ex | 35 +++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/rules/code_scanning_alerts.ex b/lib/rules/code_scanning_alerts.ex index 69ba1cd3..9746bb43 100644 --- a/lib/rules/code_scanning_alerts.ex +++ b/lib/rules/code_scanning_alerts.ex @@ -332,7 +332,14 @@ defmodule Hypatia.Rules.CodeScanningAlerts do end end - @doc false + @doc """ + Remove duplicate findings while preserving the first finding for each + identity. + + Findings with an alert number are identified by their rule and alert number. + Findings without one are identified by their rule, repository path and + Scorecard check, so distinct CSA006 advice for the same repository is kept. + """ def deduplicate_findings(findings) do Enum.uniq_by(findings, fn finding -> alert_number = Map.get(finding.detail, :alert_number) @@ -522,11 +529,13 @@ defmodule Hypatia.Rules.CodeScanningAlerts do # ─── CSA006: Scorecard configuration advice ────────────────────────────── @doc """ - CSA006: Meta-finding when a repo has Scorecard alerts that could be prevented - by configuration changes (e.g., single-contributor repos with CodeReviewID). + CSA006: Return advice for open Scorecard alerts caused by repository age or + contributor count. - This helps repository maintainers understand structural issues that generate - recurring alerts. + Reports MaintainedID alerts for repositories younger than 90 days and + CodeReviewID alerts for repositories with at most one human contributor. + Returns an empty list when GitHub data is unavailable or neither condition + applies. """ def csa006_scorecard_config_advice(owner, repo) do case fetch_alerts(owner, repo) do @@ -590,10 +599,12 @@ defmodule Hypatia.Rules.CodeScanningAlerts do end @doc """ - Builds CSA006 advice for open MaintainedID alerts on a new repository. + Build CSA006 advice for open MaintainedID alerts on a young repository. - Returns a single informational finding when the repository is less than - 90 days old, or an empty list when the alert does not need advice. + `created_at` is an ISO 8601 timestamp. Returns one informational finding when + it represents a repository younger than 90 days, or an empty list when the + timestamp is older or invalid. The finding records the MaintainedID check, + counts all supplied alerts and identifies the repository as `owner/repo`. """ def maintained_id_finding(open_maintained, created_at, owner, repo) do if is_repo_less_than_90_days?(created_at) do @@ -619,10 +630,12 @@ defmodule Hypatia.Rules.CodeScanningAlerts do end @doc """ - Builds CSA006 advice for open CodeReviewID alerts on a repository. + Build CSA006 advice for open CodeReviewID alerts when code review is + impractical. - Returns a single configuration finding for a single-contributor repository, - or an empty list when code review is feasible. + Returns one medium-severity finding when `single_contributor?` is true, or an + empty list otherwise. The finding records the CodeReviewID check, counts all + supplied alerts and identifies the repository as `owner/repo`. """ def code_review_id_finding(open_code_review, single_contributor?, owner, repo) do if single_contributor? do From 33a68392f7f47b1cb15dbc11392a7068914a96dd Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:30:55 +0000 Subject: [PATCH 8/8] docs(rules): clarify code-scanning and Scorecard behavior --- lib/rules/code_scanning_alerts.ex | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/rules/code_scanning_alerts.ex b/lib/rules/code_scanning_alerts.ex index 9746bb43..bb114d52 100644 --- a/lib/rules/code_scanning_alerts.ex +++ b/lib/rules/code_scanning_alerts.ex @@ -305,7 +305,10 @@ defmodule Hypatia.Rules.CodeScanningAlerts do # ─── Comprehensive scan ──────────────────────────────────────────────── @doc """ - Run all code-scanning checks for a repository. + Run all code-scanning checks for a GitHub repository. + + Returns an error when `GITHUB_TOKEN` is unset or empty. Otherwise, returns + the deduplicated findings, their total count and counts grouped by severity. """ def scan(owner, repo) do token = System.get_env("GITHUB_TOKEN") @@ -337,7 +340,7 @@ defmodule Hypatia.Rules.CodeScanningAlerts do identity. Findings with an alert number are identified by their rule and alert number. - Findings without one are identified by their rule, repository path and + Findings without one are identified by their rule, `file` value and Scorecard check, so distinct CSA006 advice for the same repository is kept. """ def deduplicate_findings(findings) do @@ -468,15 +471,12 @@ defmodule Hypatia.Rules.CodeScanningAlerts do # ─── CSA005: Scorecard false positives (MaintainedID, CodeReviewID) ─────────── @doc """ - CSA005: Auto-dismiss Scorecard alerts that are known false positives. - - MaintainedID: Scorecard gives score 0 to repos <90 days old. This is expected - behavior and will auto-resolve after 90 days. - - CodeReviewID: Single-contributor repos cannot have code review. This is a - structural limitation, not a security issue. + CSA005: Build low-severity findings for open Scorecard MaintainedID and + CodeReviewID alerts. - This rule proactively dismisses these alerts to reduce noise in the security tab. + Each finding proposes a dismissal reason and comment for later automation; + this function does not dismiss the GitHub alert. Returns an empty list when + alerts cannot be fetched or none of the supported Scorecard checks are open. """ def csa005_scorecard_false_positives(owner, repo) do case fetch_alerts(owner, repo) do @@ -534,8 +534,9 @@ defmodule Hypatia.Rules.CodeScanningAlerts do Reports MaintainedID alerts for repositories younger than 90 days and CodeReviewID alerts for repositories with at most one human contributor. - Returns an empty list when GitHub data is unavailable or neither condition - applies. + Returns an empty list when alerts cannot be fetched or neither condition + applies. If age or contributor data is unavailable, the corresponding advice + is omitted. """ def csa006_scorecard_config_advice(owner, repo) do case fetch_alerts(owner, repo) do