From 39cd9f260e9e01f2d805737ff778b3b7b0447744 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:55:34 +0100 Subject: [PATCH 1/3] fix(rules): precision fixes for WH006/WH013, secret carve-out, npx text, proof suites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - WH006: skip reusable-workflow caller jobs (job-level `uses:`) — GitHub rejects `timeout-minutes:` there (standards#943). - extract_job_blocks: flush the final job (the last job of every workflow was never checked — silent false negative) and report each job's own line number instead of the first job's. - WH013/WH002: `git push ` is a mirror push that authenticates with its own key/token and does not consume `contents: write`; strip it before judging writes (standards#943). Bare, `origin` and `"$VAR"` pushes still count. - ScannerSuppression: `harvested-registry/` exempt for secret_detected only — third-party reference manifests (#865). - npx_in_workflow: recommend `bunx`/`bun run`; Deno is banned since 2026-09-22 (standards#938, LANGUAGE-POLICY §1.3). - honest_completion no_tests: a proof suite whose checker runs in CI (agda/lake/lean/coqc/dune/idris2) counts as tests (echo-types#271). Each change carries fires/does-not-fire regression tests. mix test: 1682 tests, 0 failures (242 excluded); strict compile clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/hypatia/scanner_suppression.ex | 7 ++- lib/rules/cicd_rules.ex | 2 +- lib/rules/honest_completion.ex | 24 ++++++++++- lib/rules/workflow_hardening.ex | 62 +++++++++++++++++++++++---- test/honest_completion_test.exs | 23 ++++++++++ test/scanner_suppression_test.exs | 26 +++++++++++ test/workflow_hardening_test.exs | 69 ++++++++++++++++++++++++++++++ 7 files changed, 202 insertions(+), 11 deletions(-) diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 1375c338..962c8e68 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 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/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/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/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index b067741a..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 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 7f9c70b25c0de71009bbf80103dfbeeb20664e2a Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:59:59 +0100 Subject: [PATCH 2/3] fix(structural_drift): SD022 only judges refs against a tree that has a src/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A root-relative `src//` can only be rename drift of the repo root's `src/` or the referencing doc's own directory's `src/`. In a repo with neither — standards, whose specs and audits quote other repos' layouts — the reference describes a foreign tree. Measured on standards main (bd9313a6): 35 SD022 findings (34 baselined as cross-repo FPs + the k9 spec `src/tea/` in standards#945) → 1. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QFphKkDVB9pUDSCD4bkz65 --- lib/rules/structural_drift.ex | 15 ++++++++++++++- test/structural_drift_test.exs | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) 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/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) From 065447fc72e2e53bf2675e6f0a0f34e3831698c6 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:01:33 +0100 Subject: [PATCH 3/3] Revert "chore(rules): roll back pin integrity and PR automerge fixes" This reverts commit 9167ac7aab0aa04f2a4af502ba8781743f55b4be. CodeRabbit's CI-fix agent rolled back the claimed_version/relabel fixes and the `mix format` output to chase checks that fail for unrelated, already-documented reasons (reusable workflows build hypatia HEAD, i.e. broken main, until this PR merges). The rollback reintroduces the Regex.run trailing-group bug and the `##`/lost-first-byte relabel bugs that the tests in this PR pin down. 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 | 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, 157 insertions(+), 53 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..c1a7ccd0 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 d0222f66..0249b1f1 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -277,17 +277,23 @@ 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 # `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 @@ -517,7 +523,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 @@ -528,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 @@ -543,14 +556,23 @@ 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] -> "#" <> lead <> String.replace_prefix(trimmed, claim, "v" <> version) - _ -> comment + [_whole, claim] -> + "#" <> out_lead <> String.replace_prefix(trimmed, claim, "v" <> version) + + _ -> + comment end _ -> @@ -613,7 +635,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 a42cb644..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. """ @@ -236,6 +246,7 @@ defmodule Hypatia.Rules.PrAutomerge do delta(old, new, nil, nil, :unresolved, "none") end |> Map.put(:file, filename_of(file)) + |> List.wrap() end end end) @@ -270,7 +281,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 +300,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 +331,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 +374,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 @@ -362,20 +390,26 @@ 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 vetoes = if decision.safety == "flag" do - [%{bot: "Patch-Bridge", reason: decision.blocked_by}] ++ + [%{"bot" => "Patch-Bridge", "reason" => decision.blocked_by}] ++ if decision.change_level == "meta", - do: [%{bot: "hypatia", reason: "change_level=meta"}], + do: [%{"bot" => "hypatia", "reason" => "change_level=meta"}], else: [] else [] @@ -449,8 +483,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/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/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 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