diff --git a/lib/rules/git_state.ex b/lib/rules/git_state.ex index ffca7dda..44dfbb58 100644 --- a/lib/rules/git_state.ex +++ b/lib/rules/git_state.ex @@ -14,7 +14,11 @@ defmodule Hypatia.Rules.GitState do - sustainabot: advisory for unpushed changes - seambot: verify sync after push - Rule IDs: GS001-GS007 + Rule IDs: GS001-GS008 + + GS008 is the odd one out: it reports on the *scan environment* (a shallow + clone) rather than on the repository's sync state, because a truncated + history silently invalidates every history-dependent finding in the report. """ # ─── GS001: Uncommitted changes ──────────────────────────────────────── @@ -373,6 +377,66 @@ defmodule Hypatia.Rules.GitState do end end + # ─── GS008: Shallow clone (history-coverage instrument health) ──────── + + @doc """ + GS008: Detect a shallow clone -- `.git/shallow` present. + + Severity: medium. This is not a defect in the repository; it is a defect in + the *scan environment*, and it is reported so that no history-dependent + claim made by this scan is read as a pass. + + `standards/.github/workflows/hypatia-scan-reusable.yml` checks out at + `fetch-depth: 0` (verified against origin/main 2026-09-15, line 29), so in + production this fires on no consumer at all. When it does fire, that rail has + regressed, and every history-sensitive assertion in the same report -- + including `workflow_audit`'s secret-history coverage check -- is vacuous. + A history gate that sees no history is a fake gate by construction, which is + the failure class this rule exists to make loud rather than silent. + + Deliberately a filesystem probe and not `git rev-parse --is-shallow-repository`. + An unguarded `System.cmd` raises `ErlangError :enoent` wherever git is absent + from PATH -- the exact defect that got `lib/rules/secret_scanner_verification.ex` + deleted, and one that would crash a scan rather than report a finding. + + Severity is deliberately `:medium`, not `:high`. The reusable's blocking gate + refuses `high` and `critical`, and this condition is controlled by a single + shared line in `standards`; at `:high` one regression there would turn every + consumer red simultaneously. Medium is visible at the default threshold and + blocks nothing. Promote only after measuring a non-zero fire count. + """ + def gs008_shallow_clone(repo_path) do + if File.exists?(Path.join(repo_path, ".git/shallow")) do + [ + %{ + rule: "GS008", + # ⚠ MUST NOT be `.git/shallow`, however natural that reads. `.git/` + # is in `@universal_excludes` (scanner_suppression.ex), so a finding + # anchored there is silently deleted by the path filter between + # `scan/1` and the CLI output -- the rule fires, `scan/1` returns it, + # and nothing reaches the report. Measured 2026-09-15: with + # `file: ".git/shallow"` this rule was a complete no-op on a real + # `git clone --depth 1`, while passing a full-clone test perfectly. + # The exclusion exists to avoid scanning files *inside* `.git/`; it + # also eats findings *about* it. The subject here is the repository's + # clone depth, so `"."` is both correct and safe, matching GS005/GS007. + file: ".", + severity: :medium, + reason: + "Shallow clone -- `.git/shallow` is present, so this repository's history is " <> + "truncated. Any history-dependent finding in this scan (notably secret-history " <> + "coverage) is vacuous and MUST NOT be read as a pass. The hypatia scan rail " <> + "checks out at `fetch-depth: 0`; a shallow tree here means that has regressed. " <> + "Fix: set `fetch-depth: 0` on the checkout step feeding the scan, or run " <> + "`git fetch --unshallow`.", + action: :deepen_checkout + } + ] + else + [] + end + end + # ─── Comprehensive scan ─────────────────────────────────────────────── @doc """ @@ -391,7 +455,8 @@ defmodule Hypatia.Rules.GitState do gs004_stale_remote_refs(repo_path) ++ gs005_detached_head(repo_path) ++ gs006_not_on_default_branch(repo_path) ++ - gs007_stale_remote_branches(repo_path) + gs007_stale_remote_branches(repo_path) ++ + gs008_shallow_clone(repo_path) %{ findings: findings, diff --git a/lib/rules/workflow_audit.ex b/lib/rules/workflow_audit.ex index ae18365e..fd895b71 100644 --- a/lib/rules/workflow_audit.ex +++ b/lib/rules/workflow_audit.ex @@ -92,8 +92,18 @@ defmodule Hypatia.Rules.WorkflowAudit do concurrency_missing = check_concurrency_missing_readonly(workflow_contents) heading_regex_issues = check_unanchored_heading_regex(workflow_contents) d_burn_issues = check_d_burn_double_trigger(workflow_contents) + secret_history = check_secret_history_coverage(workflow_contents) %{ + # `flawed_regexes` was computed at the top of this function and + # counted into `flawed_regex_count:` below, but was never added to + # this list -- so the rule ran on every scan, reported a count + # nothing consumes, and discarded every finding it produced. The + # cli.ex normalizer's own comment names `flawed_regex` as one of the + # `:rule`-keyed sources it handles, so the omission was accidental, + # not a deliberate mute. Restoring it is gate-safe: the rule emits + # `severity: :low` (rank 4), below the reusable's blocking threshold + # of high/critical and below the default `--severity medium`. findings: missing ++ unpinned ++ @@ -117,8 +127,11 @@ defmodule Hypatia.Rules.WorkflowAudit do codeql_missing_actions ++ concurrency_missing ++ heading_regex_issues ++ - d_burn_issues, + d_burn_issues ++ + flawed_regexes ++ + secret_history, missing_count: length(missing), + secret_history_count: length(secret_history), unpinned_count: length(unpinned), wrong_pin_count: length(wrong_pins), permission_issues: length(permission_issues), @@ -1603,6 +1616,141 @@ defmodule Hypatia.Rules.WorkflowAudit do def check_flawed_regex(_), do: [] + # ─── Secret-history coverage (Phase 1c) ─────────────────────────────── + + # A repository is covered when it calls the estate rail, which is the only + # invocation whose history behaviour is guaranteed by construction: + # `standards/.github/workflows/secret-scanner-reusable.yml` checks out at + # `fetch-depth: 0`, runs a working-tree pass AND a full-history pass, and the + # history pass already refuses to report a pass on a shallow checkout. + @secret_history_rail "secret-scanner-reusable.yml" + + # A direct invocation counts as coverage too -- the estate does not mandate + # the rail -- but only its presence is asserted here, not its depth, which is + # what `secret_scan_without_history` below is for. + @direct_secret_scanners ~r/\b(gitleaks|trufflehog)\b/ + + @doc """ + Check that the repository actually has a secret-history gate. + + This is a **coverage** assertion, not a reimplementation. Full-history secret + scanning already exists on ~398 repositories via the estate rail; duplicating + `gitleaks detect` in Elixir alongside it would add cost and a second thing to + keep correct. What was missing is the assertion that the gate is *there*. + + Measured across 573 deduplicated local clones on 2026-09-15: 409 call the + hypatia scan rail, 398 call the secret-scanner rail, and **11 run hypatia with + no secret-history gate of any kind** -- no rail call and no direct `gitleaks` + or `trufflehog` invocation. Those 11 are the fire population. + + Severity is `:medium` by measurement, not by taste. The reusable's blocking + gate refuses `high` and `critical`; at `:high` this would put 11 repositories' + `main` into a permanently-red required check the moment it shipped, with no + PR to review, because hypatia is resolved by `git ls-remote ... HEAD` at + runtime. Medium is visible at the default `--severity medium` threshold and + blocks nothing. + + Deliberately NOT asserted: `secrets: inherit` on the caller. The rail's own + header (lines 28-33) requires it so the gitleaks-*action*'s `GITHUB_TOKEN` + reference resolves -- but the same header records that #500 replaced that + action with a pinned, checksum-verified binary, which removed the reason. + Probing for a requirement the artefact itself documents as superseded would + false-positive across the fleet. + + This check reads the repository's own workflow files only. Whether the + *history* a scan can see is real is a property of the checkout, and is + reported separately by `git_state`'s GS008 shallow-clone probe -- a + history-dependent finding produced under a truncated history is vacuous, and + the two findings are meant to be read together. + """ + def check_secret_history_coverage(workflow_contents) when is_map(workflow_contents) do + # An absence finding has no file to name, so it anchors on the directory + # that is its subject. Do not contort this into a filename to satisfy a + # fixture-matching gate. + contents = Map.values(workflow_contents) + + calls_rail? = Enum.any?(contents, &String.contains?(&1, @secret_history_rail)) + + direct_files = + Enum.filter(workflow_contents, fn {_f, c} -> Regex.match?(@direct_secret_scanners, c) end) + + cond do + calls_rail? -> + [] + + direct_files == [] -> + [ + %{ + rule: "missing_secret_history_scan", + severity: :medium, + file: ".github/workflows", + description: + "No secret-history scan is wired in this repository -- no call to " <> + "`#{@secret_history_rail}` and no direct `gitleaks`/`trufflehog` invocation. " <> + "A secret committed and later deleted is invisible to every working-tree " <> + "scan, so this repository has no coverage for the class of leak that " <> + "matters most. Fix: add a caller for " <> + "`hyperpolymath/standards/.github/workflows/#{@secret_history_rail}`, which " <> + "checks out at `fetch-depth: 0` and runs both a working-tree and a " <> + "full-history pass.", + action: :add_secret_history_scan + } + ] + + true -> + # A direct scanner exists. It only provides history coverage if the + # checkout feeding it is unshallow AND the scanner is not restricted to + # working-tree mode. `gitleaks --no-git` reads the filesystem only, so a + # deleted-but-committed secret is missed however deep the checkout is. + Enum.flat_map(direct_files, fn {filename, content} -> + no_git_only? = + String.contains?(content, "--no-git") and + not Regex.match?(~r/gitleaks\s+detect(?![^\n]*--no-git)/, content) + + deep_checkout? = Regex.match?(~r/fetch-depth:\s*0\b/, content) + + cond do + no_git_only? -> + [ + %{ + rule: "secret_scan_without_history", + severity: :low, + file: filename, + description: + "`#{filename}` runs a secret scanner in working-tree mode only " <> + "(`--no-git`), so a secret that was committed and later deleted is " <> + "never examined. Fix: add a second pass without `--no-git`, or call " <> + "`#{@secret_history_rail}`, which runs both passes.", + action: :add_history_pass + } + ] + + not deep_checkout? -> + [ + %{ + rule: "secret_scan_without_history", + severity: :low, + file: filename, + description: + "`#{filename}` invokes a secret scanner but its checkout does not set " <> + "`fetch-depth: 0`, so the scan sees a shallow history and cannot find " <> + "a secret that was committed and later deleted. A history scan " <> + "without history reports a pass it did not earn. Fix: set " <> + "`fetch-depth: 0` on the checkout step, or call " <> + "`#{@secret_history_rail}`.", + action: :set_fetch_depth_zero + } + ] + + true -> + [] + end + end) + end + end + + def check_secret_history_coverage(_), do: [] + # ─── WF018: Scorecard wrapper missing job-level permissions ─────────── # # Caller-of-`scorecard-reusable.yml` workflow without job-level diff --git a/test/rules/secret_history_coverage_test.exs b/test/rules/secret_history_coverage_test.exs new file mode 100644 index 00000000..e169599c --- /dev/null +++ b/test/rules/secret_history_coverage_test.exs @@ -0,0 +1,211 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) + +defmodule Hypatia.Rules.SecretHistoryCoverageTest do + @moduledoc """ + Phase 1c — the secret-history coverage rule and its companion shallow-clone + instrument-health probe. + + The governing requirement was that a history rule must "emit a loud, distinct + finding when `.git/shallow` exists -- never silently return `[]`. A history + rule that sees no history is a fake gate by construction." Most of the value + in this file is therefore in the NEGATIVE-shaped assertions: that the rule + fires when it should, not merely that it stays quiet when it should. + """ + use ExUnit.Case, async: true + + alias Hypatia.Rules.GitState + alias Hypatia.Rules.WorkflowAudit + + @rail "secret-scanner-reusable.yml" + + defp types(findings), do: Enum.map(findings, & &1.rule) + + # ─── Coverage: the repo has no secret-history gate at all ────────────── + + test "a repo with workflows but no secret-history gate is flagged" do + contents = %{ + "ci.yml" => """ + name: CI + on: [push] + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + """ + } + + findings = WorkflowAudit.check_secret_history_coverage(contents) + + assert types(findings) == ["missing_secret_history_scan"] + [f] = findings + assert f.severity == :medium + assert f.action == :add_secret_history_scan + end + + test "calling the estate rail counts as coverage and emits nothing" do + contents = %{ + "secret-scanner.yml" => """ + jobs: + scan: + uses: hyperpolymath/standards/.github/workflows/#{@rail}@main + secrets: inherit + """ + } + + assert WorkflowAudit.check_secret_history_coverage(contents) == [] + end + + test "the rail short-circuits even when another workflow looks weak" do + # A repo may call the rail AND run its own shallow working-tree scan. The + # rail already guarantees a full-history pass, so the weak one is not a + # coverage gap and must not be reported as one. + contents = %{ + "secret-scanner.yml" => "uses: ./.github/workflows/#{@rail}@main", + "extra.yml" => "run: gitleaks detect --source . --no-git" + } + + assert WorkflowAudit.check_secret_history_coverage(contents) == [] + end + + # ─── Coverage: a direct scanner exists but cannot see history ────────── + + test "a direct scanner restricted to --no-git is flagged as working-tree only" do + contents = %{ + "sec.yml" => """ + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - run: gitleaks detect --source . --no-git --redact + """ + } + + findings = WorkflowAudit.check_secret_history_coverage(contents) + + assert types(findings) == ["secret_scan_without_history"] + assert hd(findings).action == :add_history_pass + end + + test "a direct scanner without fetch-depth: 0 is flagged as shallow-fed" do + contents = %{ + "sec.yml" => """ + steps: + - uses: actions/checkout@v4 + - run: gitleaks detect --source . --redact + """ + } + + findings = WorkflowAudit.check_secret_history_coverage(contents) + + assert types(findings) == ["secret_scan_without_history"] + assert hd(findings).action == :set_fetch_depth_zero + end + + test "a direct scanner in history mode on a deep checkout is accepted" do + contents = %{ + "sec.yml" => """ + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - run: gitleaks detect --source . --redact + """ + } + + assert WorkflowAudit.check_secret_history_coverage(contents) == [] + end + + test "both gitleaks and trufflehog count as a direct scanner" do + contents = %{ + "sec.yml" => """ + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - run: trufflehog filesystem . --no-git + """ + } + + assert types(WorkflowAudit.check_secret_history_coverage(contents)) == + ["secret_scan_without_history"] + end + + test "a non-map input is tolerated rather than raising" do + # hypatia is resolved by `git ls-remote ... HEAD` at runtime by the scan + # reusable, so an unhandled raise here reaches ~449 consumers with no PR. + assert WorkflowAudit.check_secret_history_coverage(nil) == [] + end + + # ─── Instrument health: GS008 shallow clone ──────────────────────────── + + describe "GS008 shallow clone" do + setup do + dir = Path.join(System.tmp_dir!(), "hypatia-gs008-#{System.unique_integer([:positive])}") + File.mkdir_p!(Path.join(dir, ".git")) + on_exit(fn -> File.rm_rf(dir) end) + {:ok, dir: dir} + end + + test "fires when .git/shallow is present", %{dir: dir} do + File.write!(Path.join(dir, ".git/shallow"), "deadbeef\n") + + findings = GitState.gs008_shallow_clone(dir) + + assert types(findings) == ["GS008"] + [f] = findings + assert f.severity == :medium + assert f.action == :deepen_checkout + # The message must say what is unsafe, not merely that a file exists. + assert f.reason =~ "vacuous" + end + + test "is silent when the clone has full history", %{dir: dir} do + refute File.exists?(Path.join(dir, ".git/shallow")) + assert GitState.gs008_shallow_clone(dir) == [] + end + + test "the finding is NOT anchored inside .git/", %{dir: dir} do + # REGRESSION FLOOR. `.git/` is in `@universal_excludes` + # (scanner_suppression.ex), so a finding whose `file:` sits under `.git/` + # is silently deleted by the path filter between `GitState.scan/1` and the + # CLI's output. Measured 2026-09-15: anchored at `.git/shallow`, this rule + # returned a finding from `scan/1` and produced NOTHING in the report on a + # real `git clone --depth 1` -- a complete no-op that still passed a + # full-clone test. Anchor stays at the repository root. + File.write!(Path.join(dir, ".git/shallow"), "deadbeef\n") + + [f] = GitState.gs008_shallow_clone(dir) + + refute String.starts_with?(f.file, ".git/") + assert f.file == "." + end + + test "scan/1 includes GS008 in its findings", %{dir: dir} do + # Guards the wiring, not the predicate: the rule existing but never being + # summed into `scan/1` is the same defect class as `flawed_regex`, which + # was computed and counted for its entire life without ever being added + # to the findings list. + File.write!(Path.join(dir, ".git/shallow"), "deadbeef\n") + + %{findings: findings} = GitState.scan(dir) + + assert "GS008" in types(findings) + end + end + + # ─── Wiring: the restored flawed_regex findings ──────────────────────── + + test "flawed_regex findings reach audit/3's findings list" do + # They were computed and counted but never concatenated, so the rule ran on + # every scan in the estate and discarded everything it found. + contents = %{"ci.yml" => ~s( - run: grep "foo.bar" README.md\n)} + + %{findings: findings, flawed_regex_count: count} = + WorkflowAudit.audit(["ci.yml"], contents) + + assert count > 0 + assert "flawed_regex" in Enum.map(findings, &(&1[:rule] || &1[:type])) + end +end