Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions lib/hypatia/cli.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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 <fmt> Output format: json (default), text, github, sarif
--severity <lvl> Minimum severity to report: critical, high, medium (default), low, info
--path <dir> Path to scan (alternative to positional argument)
Expand All @@ -54,7 +55,8 @@ defmodule Hypatia.CLI do
:secret_scanning_alerts,
:code_scanning_alerts,
:structural_drift,
:implementation_inside_canon
:implementation_inside_canon,
:content_patterns
Comment thread
coderabbitai[bot] marked this conversation as resolved.
]

@severity_order %{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 <fmt> Output format: json (default), text, github, sarif, sarif
--severity, -s <lvl> Minimum severity: critical, high, medium (default), low
--path, -p <dir> Path to scan (alternative to positional arg)
Expand Down
134 changes: 125 additions & 9 deletions lib/rules/cicd_rules.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)/,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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/,
Expand Down Expand Up @@ -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 ->
Expand All @@ -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 ->
[]
Expand All @@ -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 <rule_id>` (in any comment syntax we recognise).
defp ignored?(rule_id, lines, n) do
Expand All @@ -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).
Expand Down
80 changes: 80 additions & 0 deletions test/rules/cicd_rules_content_scanner_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading