diff --git a/.github/workflows/actions.lock b/.github/workflows/actions.lock index 759c3445..04da3687 100644 --- a/.github/workflows/actions.lock +++ b/.github/workflows/actions.lock @@ -35,7 +35,7 @@ workflows: - 'dependabot/fetch-metadata@v3.1.0' '.github/workflows/dogfood-gate.yml': - 'actions/checkout@v7.0.1' - - 'hyperpolymath/a2ml-ecosystem@main' + - 'hyperpolymath/deed-ecosystem@main' - 'hyperpolymath/k9-ecosystem@main' '.github/workflows/estate-rescan.yml': - 'actions/cache@v6.1.0' @@ -294,14 +294,14 @@ dependencies: commit: 'sha1-6037f33647c3f17758a2356c80fc4a53d7e0685d' owner_id: 75048950 repo_id: 623796603 - 'hyperpolymath/a2ml-ecosystem@main': + 'hyperpolymath/deed-ecosystem@main': ref: 'main' - commit: 'sha1-c992d2882ee1e62bf5c78b5f9a1893a6a16730e4' + commit: 'sha1-f7a40a4d5cc82b2e73f861119baa6818d77a448d' owner_id: 6759885 repo_id: 1275649586 'hyperpolymath/k9-ecosystem@main': ref: 'main' - commit: 'sha1-3f250fba42e432c7ff47b48f59525bec3357136b' + commit: 'sha1-2155aa26a21758f2ba119f61bc7e0e1981c106fb' owner_id: 6759885 repo_id: 1275650185 'ruby/setup-ruby@v1.321.0': diff --git a/.github/workflows/dogfood-gate.yml b/.github/workflows/dogfood-gate.yml index 43b9a0cb..f2a1f0a0 100644 --- a/.github/workflows/dogfood-gate.yml +++ b/.github/workflows/dogfood-gate.yml @@ -47,7 +47,7 @@ jobs: - name: Validate A2ML manifests if: steps.detect.outputs.count > 0 - uses: hyperpolymath/a2ml-ecosystem/validate-action@main + uses: hyperpolymath/deed-ecosystem/validate-action@main with: path: '.' strict: 'false' diff --git a/lib/hypatia/cli.ex b/lib/hypatia/cli.ex index 8857ca00..a85e7818 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 %{ @@ -299,13 +301,17 @@ defmodule Hypatia.CLI do # ─── Finding collection across rule modules ────────────────────────── @doc """ - Run the named rule modules against `repo_path` and return normalized findings - (`%{rule_module, type, severity, file, reason, action}`). Public so the RSR - conformance oracle can delegate content-scan criteria to the live scanners - rather than reimplement per-file detection. `rules` is a list of module atoms - (e.g. `[:cicd_rules, :structural_drift]`); GitHub-API modules - (`:dependabot_alerts`, `:secret_scanning_alerts`, `:code_scanning_alerts`, - `:scorecard`) require network + token and return nothing offline. + Run the named rule modules against `repo_path` and return unsuppressed findings + normalised as `%{rule_module, type, severity, file, reason, action}` maps. + Content-pattern findings also include their one-based source `line`. Public so + the RSR conformance oracle can delegate content-scan criteria to the live + scanners rather than reimplement per-file detection. + + `rules` is a list of module atoms (for example, `[:content_patterns, + :structural_drift]`). GitHub alert modules (`:dependabot_alerts`, + `:secret_scanning_alerts`, and `:code_scanning_alerts`) require network access + and credentials; when unavailable, they write a warning to standard error and + contribute no findings. """ def collect_findings(repo_path, rules) do results = [] @@ -818,6 +824,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 +1344,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..9173d0ae 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/, @@ -750,8 +790,8 @@ defmodule Hypatia.Rules.CicdRules do Content scanner — activates the regex+applies_to rules in @blocked_patterns that were previously dormant. - Walks `repo_path`, opens any file matching one of a rule's `applies_to` - globs, and emits a finding for each regex match. Honors: + Scans files beneath `repo_path`, excluding `.git` directories, that match each + rule's `applies_to` globs and emits one finding for each matching line. Honours: * `path_allow_prefixes` — substring match against the relative file path (mirrors the glob-pattern behaviour). @@ -760,55 +800,80 @@ defmodule Hypatia.Rules.CicdRules do style entries). * `exception_repos` — list of repo names; if any matches the basename of `repo_path`, the rule is skipped for this scan. - * `negative: true` — fires when the regex does NOT match (used by - `:missing_permissions` and `:missing_spdx` which test for the - ABSENCE of an expected line). - * Inline pragma — a line starting with `# hypatia:ignore ` - or `` (for markdown/HTML) - suppresses findings for that rule on the SAME line and the - following line. Matches the convention used by other Hypatia - scanners (scanner_suppression.ex). + * `negative: true` — emits one finding at line 1 when the regex is absent. + * `skip_comment_lines: true` — ignores matching lines whose first + non-whitespace characters are `#` or `//`. + * `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 + immediately preceding line suppresses that finding. Activates these previously-dormant rules: :innerhtml_usage, :eval_in_shell, :download_then_run_shell, :hardcoded_tmp, :template_placeholder, :deno_all_perms, :v_build_in_ci (#383), - :npx_in_workflow (#383), :http_in_docs (#383). + :npx_in_workflow (#383), :http_in_docs (#383), and + :install_without_frozen_lockfile. Returns a list of findings: - [%{rule: :rule_id, reason: "...", file: "rel/path", line: N, match: "..."}] + [%{rule: :rule_id, severity: "medium", reason: "...", file: "rel/path", + line: N, match: "..."}] """ def scan_content_patterns(repo_path) do repo_name = Path.basename(repo_path) + files = repository_files(repo_path) @blocked_patterns |> Enum.filter(fn p -> Map.has_key?(p, :pattern) and Map.has_key?(p, :applies_to) end) - |> Enum.flat_map(fn rule -> scan_one_content_rule(rule, repo_path, repo_name) end) + |> Enum.flat_map(fn rule -> scan_one_content_rule(rule, repo_path, repo_name, files) end) end - defp scan_one_content_rule(rule, repo_path, repo_name) do + defp scan_one_content_rule(rule, repo_path, repo_name, files) do exception_repos = Map.get(rule, :exception_repos, []) if repo_name in exception_repos do [] else rule - |> matching_files(repo_path) + |> matching_files(files) |> Enum.flat_map(fn rel -> scan_one_file(rule, repo_path, rel) end) end end - defp matching_files(rule, repo_path) do + defp repository_files(repo_path), do: walk_repository_files(repo_path, "") + + defp walk_repository_files(path, relative_path) do + case File.ls(path) do + {:ok, entries} -> + entries + |> Enum.sort() + |> Enum.flat_map(fn entry -> + abs = Path.join(path, entry) + rel = Path.join(relative_path, entry) + + cond do + entry == ".git" -> + [] + + File.dir?(abs) -> + walk_repository_files(abs, rel) + + true -> + [rel] + end + end) + + {:error, _} -> + [] + end + end + + defp matching_files(rule, files) do globs = Map.get(rule, :applies_to, []) allow_prefixes = Map.get(rule, :path_allow_prefixes, []) exception = Map.get(rule, :exception) - Path.wildcard("#{repo_path}/**/*", match_dot: false) - |> Enum.reject(&File.dir?/1) - |> Enum.map(&Path.relative_to(&1, repo_path)) - |> Enum.filter(fn rel -> - not String.starts_with?(rel, ".git/") and - Enum.any?(globs, fn g -> glob_matches?(g, rel) end) - end) + files + |> Enum.filter(fn rel -> Enum.any?(globs, fn g -> glob_matches?(g, rel) end) end) |> Enum.reject(fn rel -> Enum.any?(allow_prefixes, &String.contains?(rel, &1)) or (is_binary(exception) and String.contains?(rel, exception)) @@ -821,18 +886,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 +918,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 +1001,16 @@ 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) and `//` + # (JS/Rust/C). `--` is a long-option prefix in workflow command lines. + defp comment_line?(line) do + t = String.trim_leading(line) + + 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..7d42ec65 100644 --- a/test/rules/cicd_rules_content_scanner_test.exs +++ b/test/rules/cicd_rules_content_scanner_test.exs @@ -77,4 +77,99 @@ 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 + + test "prunes .git while retaining other dot-directories", %{dir: dir} do + git_workflows = Path.join(dir, ".git/workflows") + File.mkdir_p!(git_workflows) + File.write!(Path.join(git_workflows, "ci.yml"), "steps:\n - run: npx prettier .\n") + + findings = CicdRules.scan_content_patterns(dir) + refute 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 "fires on a long-option line", %{dir: dir, wf: wf} do + File.write!(Path.join(wf, "long-option.yml"), "-- bun install\n") + findings = CicdRules.scan_content_patterns(dir) + assert 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