diff --git a/lib/hypatia/cli.ex b/lib/hypatia/cli.ex index 8857ca00..72f4c7f0 100644 --- a/lib/hypatia/cli.ex +++ b/lib/hypatia/cli.ex @@ -27,7 +27,8 @@ defmodule Hypatia.CLI do cicd_rules,code_safety,migration_rules,scorecard, green_web,git_state,dependabot_alerts, secret_scanning_alerts,code_scanning_alerts, - structural_drift,implementation_inside_canon + structural_drift,implementation_inside_canon, + content_patterns --format Output format: json (default), text, github, sarif --severity Minimum severity to report: critical, high, medium (default), low, info --path Path to scan (alternative to positional argument) @@ -54,7 +55,8 @@ defmodule Hypatia.CLI do :secret_scanning_alerts, :code_scanning_alerts, :structural_drift, - :implementation_inside_canon + :implementation_inside_canon, + :content_patterns ] @severity_order %{ @@ -818,6 +820,43 @@ defmodule Hypatia.CLI do results end + # ─── Content-pattern rules ─────────────────────────────────────────── + # + # `CicdRules.scan_content_patterns/1` is a glob+regex, per-line content + # engine over the `@blocked_patterns` table. It shipped complete but + # unwired: until now nothing in `lib/` called it, so every table entry + # carrying `:pattern` + `:applies_to` was dormant and only its unit test + # ever exercised it. Wiring it here makes rule authoring a matter of + # adding a table row rather than writing a module. + # + # This is the only branch that emits a real `:line`. Everything else + # normalizes without one, which is why SARIF's `startLine` was uniformly + # 1 before this landed. Suppression is NOT applied here -- the uniform + # pass below funnels every finding through ScannerSuppression exactly + # once, and doing it twice would be both redundant and a second place + # for exemptions to silently diverge. + results = + if :content_patterns in rules do + normalized = + repo_path + |> Hypatia.Rules.CicdRules.scan_content_patterns() + |> Enum.map(fn f -> + %{ + rule_module: "content_patterns", + severity: to_string(Map.get(f, :severity, "medium")), + type: to_string(f.rule), + file: f.file, + line: f.line, + reason: f.reason, + action: "flag" + } + end) + + results ++ normalized + else + results + end + # ─── Uniform suppression pass ────────────────────────────────────── # # Several rule paths above (structural_drift, code_scanning_alerts, @@ -1301,7 +1340,8 @@ defmodule Hypatia.CLI do migration_rules,scorecard,green_web, git_state,dependabot_alerts, secret_scanning_alerts,code_scanning_alerts, - structural_drift,implementation_inside_canon + structural_drift,implementation_inside_canon, + content_patterns --format, -f Output format: json (default), text, github, sarif, sarif --severity, -s Minimum severity: critical, high, medium (default), low --path, -p Path to scan (alternative to positional arg) diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index c0eecd60..9d558909 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -669,6 +669,46 @@ defmodule Hypatia.Rules.CicdRules do reason: "eval banned in shell scripts -- use direct expansion or arrays", applies_to: ["*.sh"] }, + # --- Scanner-derived rule (2026-09-01) ----------------------------- + # + # Flagged INDEPENDENTLY by both CodeRabbit and Codacy across estate PRs. + # Two scanners agreeing is the strongest signal the C2 triage gate can + # get, the fix is mechanical, and it matches the estate's own lockfile + # doctrine -- which is why this was picked as the proof-of-concept rule + # over the higher-volume "SHA-pin your actions" advice. That advice was + # REJECTED: it contradicts the standing owner ruling that + # `sha_pinning_required` is OFF and `actions.lock` IS the pin (C1). + # + # A bare `bun install` lets CI resolve versions OUTSIDE the lockfile. + # That is the same defect class as the `actions.lock` version drift + # which is the estate's dominant startup_failure killer -- CI runs + # something the lockfile never sanctioned, and nothing says so. + # + # `applies_to` is MANDATORY, not decorative: scan_content_patterns/1 + # filters on `Map.has_key?(p, :applies_to)`, so a rule without one is + # silently inert -- it looks complete in this table and can never fire. + # Six existing entries are dead this way. The four globs cover both the + # root `.github/workflows/` and the nested monorepo copies, mirroring + # `workflow_file?/1`. + # + # `skip_comment_lines` honours C4 (no matching inside comments). This + # repo has already shipped that defect once -- the `unwrap` rule matched + # commented-out code -- and a commented-out CI step is exactly where a + # bare `bun install` survives. + %{ + id: :install_without_frozen_lockfile, + pattern: ~r/\bbun\s+install\b(?![^\n]*--frozen-lockfile)/, + reason: + "CI installs must be `bun install --frozen-lockfile` -- a bare install resolves outside the lockfile and can run versions the lockfile never sanctioned", + applies_to: [ + ".github/workflows/*.yml", + ".github/workflows/*.yaml", + "**/.github/workflows/*.yml", + "**/.github/workflows/*.yaml" + ], + skip_comment_lines: true, + strip_yaml_comments: true + }, %{ id: :download_then_run_shell, pattern: ~r/\b(curl|wget)\b[^\n|;]*\|\s*(sh|bash)\b/, @@ -802,7 +842,7 @@ defmodule Hypatia.Rules.CicdRules do allow_prefixes = Map.get(rule, :path_allow_prefixes, []) exception = Map.get(rule, :exception) - Path.wildcard("#{repo_path}/**/*", match_dot: false) + Path.wildcard("#{repo_path}/**/*", match_dot: true) |> Enum.reject(&File.dir?/1) |> Enum.map(&Path.relative_to(&1, repo_path)) |> Enum.filter(fn rel -> @@ -821,18 +861,28 @@ defmodule Hypatia.Rules.CicdRules do case File.read(abs) do {:ok, content} -> negative? = Map.get(rule, :negative, false) - matched? = Regex.match?(rule.pattern, content) + matching_content = content_for_matching(rule, content) + matched? = Regex.match?(rule.pattern, matching_content) cond do # Negative rules: fire when pattern is ABSENT negative? and not matched? -> - [%{rule: rule.id, reason: rule.reason, file: rel, line: 1, match: "(absent)"}] + [ + %{ + rule: rule.id, + severity: Map.get(rule, :severity, "medium"), + reason: rule.reason, + file: rel, + line: 1, + match: "(absent)" + } + ] negative? -> [] matched? -> - line_findings(rule, rel, content) + line_findings(rule, rel, content, matching_content) true -> [] @@ -843,25 +893,80 @@ defmodule Hypatia.Rules.CicdRules do end end - defp line_findings(rule, rel, content) do + defp line_findings(rule, rel, content, matching_content) do lines = String.split(content, "\n") + matching_lines = String.split(matching_content, "\n") - lines + Enum.zip(lines, matching_lines) |> Enum.with_index(1) - |> Enum.flat_map(fn {line, n} -> + |> Enum.flat_map(fn {{line, matching_line}, n} -> cond do - not Regex.match?(rule.pattern, line) -> + not Regex.match?(rule.pattern, matching_line) -> + [] + + # C4: a rule may opt out of matching inside comments. Default false, + # so no existing rule changes behaviour. Checked BEFORE the pragma + # test because a commented-out line needs no `hypatia:ignore`. + Map.get(rule, :skip_comment_lines, false) and comment_line?(line) -> [] ignored?(rule.id, lines, n) -> [] true -> - [%{rule: rule.id, reason: rule.reason, file: rel, line: n, match: String.trim(line)}] + [ + %{ + rule: rule.id, + severity: Map.get(rule, :severity, "medium"), + reason: rule.reason, + file: rel, + line: n, + match: String.trim(line) + } + ] end end) end + defp content_for_matching(rule, content) do + if Map.get(rule, :strip_yaml_comments, false) do + content + |> String.split("\n") + |> Enum.map_join("\n", &strip_yaml_comment/1) + else + content + end + end + + defp strip_yaml_comment(line) do + line + |> String.graphemes() + |> do_strip_yaml_comment(nil, false, nil, []) + |> Enum.reverse() + |> Enum.join() + end + + defp do_strip_yaml_comment([], _quote, _escaped, _previous, acc), do: acc + + defp do_strip_yaml_comment(["#" | _rest], nil, false, previous, acc) + when previous in [nil, " ", "\t"], + do: acc + + defp do_strip_yaml_comment([char | rest], quote, escaped, _previous, acc) do + {next_quote, next_escaped} = + case {quote, escaped, char} do + {"\"", true, _} -> {"\"", false} + {"\"", false, "\\"} -> {"\"", true} + {"\"", false, "\""} -> {nil, false} + {"'", false, "'"} -> {nil, false} + {nil, false, "\""} -> {"\"", false} + {nil, false, "'"} -> {"'", false} + _ -> {quote, false} + end + + do_strip_yaml_comment(rest, next_quote, next_escaped, char, [char | acc]) + end + # Inline pragma: this line OR the previous line carries # `hypatia:ignore ` (in any comment syntax we recognise). defp ignored?(rule_id, lines, n) do @@ -871,6 +976,17 @@ defmodule Hypatia.Rules.CicdRules do String.contains?(here, needle) or String.contains?(prev, needle) end + # C4 helper: is this line ENTIRELY a comment? Deliberately conservative for + # general content rules. YAML rules can opt into the quote-aware trailing + # comment handling above. Covers `#` (YAML/shell/Elixir), `//` (JS/Rust/C) + # and `--` (SQL/Ada/Haskell/Lua). + defp comment_line?(line) do + t = String.trim_leading(line) + + String.starts_with?(t, "#") or String.starts_with?(t, "//") or + String.starts_with?(t, "--") + end + defp glob_matches?(glob, path) do # Support: "*.ext" (suffix), "**/path/**", literal "Justfile" / "Mustfile", # "*/segment/*" (substring). diff --git a/test/rules/cicd_rules_content_scanner_test.exs b/test/rules/cicd_rules_content_scanner_test.exs index 77b4beb9..2d421c1e 100644 --- a/test/rules/cicd_rules_content_scanner_test.exs +++ b/test/rules/cicd_rules_content_scanner_test.exs @@ -77,4 +77,84 @@ defmodule Hypatia.Rules.CicdRules.ContentScannerTest do refute Enum.any?(findings, &(&1.rule == :hardcoded_tmp)) end end + + # ── Regression guard: the engine must be able to SEE `.github/` ─────── + # + # `matching_files/2` enumerated with `Path.wildcard(..., match_dot: false)`, + # which never matches a dot-prefixed segment. Every workflow lives under + # `.github/`, so no workflow was reachable and the only two YAML-scoped + # rules could never fire on one. Proven with a byte-identical file: at + # `.github/workflows/ci.yml` it produced nothing; at `root-ci.yml` it fired. + # If this test ever goes red, the scanner has gone blind to CI again. + describe "dot-directory reachability" do + test "a rule fires on a file under .github/", %{dir: dir} do + wf = Path.join(dir, ".github/workflows") + File.mkdir_p!(wf) + File.write!(Path.join(wf, "ci.yml"), "steps:\n - run: npx prettier .\n") + findings = CicdRules.scan_content_patterns(dir) + assert Enum.any?(findings, &(&1.rule == :npx_in_workflow)) + end + end + + # ── Scanner-derived rule: --frozen-lockfile ─────────────────────────── + # + # Positive, canonical-fix negative, and a C4 comment case. The trio is the + # house contract: a rule that fires but cannot be satisfied by the fix it + # names is a gate that cannot pass, and one that matches commented-out + # code repeats a defect this repo has already shipped once. + describe "install_without_frozen_lockfile" do + setup %{dir: dir} do + wf = Path.join(dir, ".github/workflows") + File.mkdir_p!(wf) + {:ok, wf: wf} + end + + test "fires on a bare `bun install`, at the right line", %{dir: dir, wf: wf} do + File.write!(Path.join(wf, "ci.yml"), "steps:\n - run: echo hi\n - run: bun install\n") + findings = CicdRules.scan_content_patterns(dir) + finding = Enum.find(findings, &(&1.rule == :install_without_frozen_lockfile)) + assert finding + # Line 3, not 1 -- the content engine is the only source of a real + # `:line`, and it is what makes SARIF `startLine` non-degenerate. + assert finding.line == 3 + end + + test "does NOT fire on the canonical fix", %{dir: dir, wf: wf} do + File.write!(Path.join(wf, "ok.yml"), "steps:\n - run: bun install --frozen-lockfile\n") + findings = CicdRules.scan_content_patterns(dir) + refute Enum.any?(findings, &(&1.rule == :install_without_frozen_lockfile)) + end + + test "C4: does NOT fire on a commented-out install", %{dir: dir, wf: wf} do + File.write!(Path.join(wf, "c.yml"), "steps:\n # - run: bun install\n - run: echo ok\n") + findings = CicdRules.scan_content_patterns(dir) + refute Enum.any?(findings, &(&1.rule == :install_without_frozen_lockfile)) + end + + test "still fires when the comment marker is TRAILING, not leading", %{dir: dir, wf: wf} do + File.write!(Path.join(wf, "t.yml"), "steps:\n - run: bun install # TODO pin this\n") + findings = CicdRules.scan_content_patterns(dir) + assert Enum.any?(findings, &(&1.rule == :install_without_frozen_lockfile)) + end + + test "trailing comments cannot supply --frozen-lockfile", %{dir: dir, wf: wf} do + File.write!( + Path.join(wf, "commented-flag.yml"), + ~s(steps:\n - run: "printf '# keep'; bun install" # --frozen-lockfile\n) + ) + + findings = CicdRules.scan_content_patterns(dir) + assert Enum.any?(findings, &(&1.rule == :install_without_frozen_lockfile)) + end + + test "bun install in a trailing comment does not create a finding", %{dir: dir, wf: wf} do + File.write!( + Path.join(wf, "commented-install.yml"), + "steps:\n - run: echo ok # bun install\n" + ) + + findings = CicdRules.scan_content_patterns(dir) + refute Enum.any?(findings, &(&1.rule == :install_without_frozen_lockfile)) + end + end end