diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 79b5290..8c2861a 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -12,6 +12,23 @@ https://semver.org/[Semantic Versioning]. === Added +==== `CicdRules` rule `duplicate_cron_schedule` (2026-05-30, #362) + +Flags workflows whose `on.schedule` carries redundant `cron:` triggers: two +or more entries on the same day-of-week field (e.g. three Monday crons +inherited from a workflow consolidation), or a daily `* * *` entry that +strictly covers a day-specific entry at the same `HH:MM`. Root-fixed +estate-wide in `hypatia#331` (`tests.yml`, `verify-proofs.yml`); distinct +intentional triggers (different times, or different `if:` gates on the same +day) are advisory only and never auto-removed. + +Surfaced through the facade as +`Hypatia.Rules.scan_duplicate_cron_schedules/1`; the pure predicate +`CicdRules.check_duplicate_cron_schedules/2` is covered by +`test/rules/cicd_rules_duplicate_cron_test.exs` for both sensitivity +(same-day-of-week + daily-subset oracles) and specificity (different-time +non-subset, single cron, commented-out lines). + ==== `CicdRules` rule `scorecard_wrapper_missing_job_permissions` (2026-05-30, #390) New forward-detection rule for a silent-CI-failure class: a diff --git a/CHANGELOG.md b/CHANGELOG.md index bf37675..0be4fb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ this project aims to follow [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- feat(rules): CicdRules `duplicate_cron_schedule` — flag workflows with redundant cron entries on the same day-of-week / daily-subset (#362) - feat(rules): CicdRules `scorecard_wrapper_missing_job_permissions` — flag scorecard.yml wrappers that call the standards reusable but omit `security-events: write` on the calling job (#390) - feat(rules): AffineScript hand-port pitfalls — HANDLE-as-fn-name + OCaml float ops (#332) - feat(rules): wire 4 new rule modules through the facade (#326) diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 52dc659..9f611bc 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -930,6 +930,116 @@ defmodule Hypatia.Rules.CicdRules do rel == @scorecard_wrapper_path or String.ends_with?(rel, "/" <> @scorecard_wrapper_path) end + # --------------------------------------------------------------------------- + # Duplicate cron schedules (#362) + # --------------------------------------------------------------------------- + # + # A workflow whose `on.schedule` lists multiple `cron:` entries that fire on + # the same day-of-week is usually a consolidation artefact (merged-in source + # workflows each carrying their own trigger): wasted runner minutes and log + # noise. Two shapes are flagged: + # + # 1. >= 2 entries sharing the same day-of-week field (e.g. three `* * 1` + # Monday crons inherited from pre-consolidation workflows). + # 2. A daily entry (`* * *`) and a day-specific entry at the same HH:MM — + # the specific entry is a strict subset of the daily one. + # + # Root-fixed estate-wide in hypatia#331 (tests.yml, verify-proofs.yml). + # Distinct intentional triggers (different HH:MM, or different `if:` gates on + # the same day) are advisory under shape (1) and never auto-removed. + + @doc """ + Scan `repo_path`'s workflow files for redundant `cron:` schedules (#362). + + Reads each `.github/workflows/*.{yml,yaml}` (root or nested monorepo copy) + and returns the findings from `check_duplicate_cron_schedules/2` for each. + + Returns `[%{rule:, severity:, file:, reason:, fix:, crons:}]`. + """ + def scan_duplicate_cron_schedules(repo_path) do + Path.wildcard("#{repo_path}/**/*", match_dot: true) + |> Enum.reject(&File.dir?/1) + |> Enum.map(&Path.relative_to(&1, repo_path)) + |> Enum.filter(&workflow_file?/1) + |> Enum.flat_map(fn rel -> + case File.read(Path.join(repo_path, rel)) do + {:ok, content} -> check_duplicate_cron_schedules(rel, content) + {:error, _} -> [] + end + end) + end + + @doc """ + Pure predicate behind `scan_duplicate_cron_schedules/1`: parse the `cron:` + list entries in workflow `content` and return findings for `path`. Lines + that are not 5-field cron expressions (and commented-out `# - cron:` lines) + are ignored. + """ + def check_duplicate_cron_schedules(path, content) do + crons = parse_cron_entries(content) + + same_dow = + crons + |> Enum.group_by(& &1.dow) + |> Enum.filter(fn {_dow, list} -> length(list) >= 2 end) + |> Enum.map(fn {dow, list} -> + raws = Enum.map(list, & &1.raw) + + %{ + rule: :duplicate_cron_schedule, + severity: :medium, + file: path, + reason: + "#{length(list)} cron entries fire on the same day-of-week (#{dow_label(dow)}): #{Enum.join(raws, ", ")} — likely a consolidation artefact.", + fix: + "Collapse to a single cron unless the triggers serve distinct purposes (e.g. different `if:` gates); otherwise drop the redundant entries.", + crons: raws + } + end) + + daily = Enum.filter(crons, &(&1.dow == "*")) + specific = Enum.reject(crons, &(&1.dow == "*")) + + subsets = + for d <- daily, s <- specific, d.min == s.min and d.hour == s.hour do + %{ + rule: :duplicate_cron_schedule, + severity: :medium, + file: path, + reason: + "cron `#{s.raw}` is a strict subset of the daily cron `#{d.raw}` (same HH:MM); the daily trigger already covers that day.", + fix: + "Drop the day-specific entry `#{s.raw}` — the daily `#{d.raw}` already runs at that time.", + crons: [d.raw, s.raw] + } + end + + same_dow ++ subsets + end + + defp workflow_file?(rel) do + String.contains?(rel, ".github/workflows/") and + (String.ends_with?(rel, ".yml") or String.ends_with?(rel, ".yaml")) + end + + defp parse_cron_entries(content) do + ~r/^\s*-\s*cron:\s*["']?([\d*,\/\- ]+?)["']?\s*(?:#[^\n]*)?$/m + |> Regex.scan(content) + |> Enum.map(fn [_, expr] -> String.trim(expr) end) + |> Enum.map(&parse_one_cron/1) + |> Enum.reject(&is_nil/1) + end + + defp parse_one_cron(expr) do + case String.split(expr, " ", trim: true) do + [min, hour, _dom, _mon, dow] -> %{raw: expr, min: min, hour: hour, dow: dow} + _ -> nil + end + end + + defp dow_label("*"), do: "every day" + defp dow_label(dow), do: "day-of-week #{dow}" + # --------------------------------------------------------------------------- # CI/CD Waste Detection # --------------------------------------------------------------------------- diff --git a/lib/rules/rules.ex b/lib/rules/rules.ex index eca55fb..23567bb 100644 --- a/lib/rules/rules.ex +++ b/lib/rules/rules.ex @@ -428,6 +428,12 @@ defmodule Hypatia.Rules do """ defdelegate scan_scorecard_wrapper_permissions(repo_path, opts \\ []), to: CicdRules + @doc """ + Scan workflows for redundant `cron:` schedules firing on the same + day-of-week (#362). + """ + defdelegate scan_duplicate_cron_schedules(repo_path), to: CicdRules + @doc """ Run baseline-health checks (BH001-BH007): missing required_status_checks on main, deferred-migration TODOs in dep manifests, persistent >24h red diff --git a/test/rules/cicd_rules_duplicate_cron_test.exs b/test/rules/cicd_rules_duplicate_cron_test.exs new file mode 100644 index 0000000..db0fa7a --- /dev/null +++ b/test/rules/cicd_rules_duplicate_cron_test.exs @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.Rules.CicdRules.DuplicateCronTest do + use ExUnit.Case, async: true + + alias Hypatia.Rules.CicdRules + + # #362 — multiple `cron:` entries firing on the same day-of-week are a + # consolidation-artefact waste pattern. Two shapes: same exact day-of-week + # field (>= 2 entries), and a daily `* * *` entry strictly covering a + # day-specific entry at the same HH:MM. Oracles from hypatia#331. + + @three_monday "on:\n schedule:\n - cron: 0 5 * * 1\n - cron: 30 4 * * 1\n - cron: 0 4 * * 1\n" + @daily_plus_monday "on:\n schedule:\n - cron: \"0 3 * * *\"\n - cron: \"0 3 * * 1\"\n" + @daily_plus_other_time "on:\n schedule:\n - cron: 0 2 * * *\n - cron: 0 3 * * 1\n" + @single "on:\n schedule:\n - cron: '0 3 * * *'\n" + @commented "on:\n schedule:\n - cron: 0 4 * * 1\n # - cron: 0 5 * * 1\n" + + defp wf(dir, name, body) do + path = Path.join([dir, ".github/workflows", name]) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, body) + end + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-cron-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + describe "check_duplicate_cron_schedules/2 — sensitivity" do + test "three crons on the same day-of-week (verify-proofs.yml oracle)" do + assert [finding] = + CicdRules.check_duplicate_cron_schedules("verify-proofs.yml", @three_monday) + + assert finding.rule == :duplicate_cron_schedule + assert finding.severity == :medium + assert length(finding.crons) == 3 + end + + test "daily strictly covers a same-time day-specific entry (tests.yml oracle)" do + assert [finding] = CicdRules.check_duplicate_cron_schedules("tests.yml", @daily_plus_monday) + assert finding.rule == :duplicate_cron_schedule + assert finding.reason =~ "subset" + end + end + + describe "check_duplicate_cron_schedules/2 — specificity" do + test "daily + different-time day-specific is NOT a subset (security-policy.yml)" do + assert CicdRules.check_duplicate_cron_schedules( + "security-policy.yml", + @daily_plus_other_time + ) == + [] + end + + test "a single cron is never flagged" do + assert CicdRules.check_duplicate_cron_schedules("x.yml", @single) == [] + end + + test "commented-out cron lines are ignored" do + assert CicdRules.check_duplicate_cron_schedules("x.yml", @commented) == [] + end + end + + describe "scan_duplicate_cron_schedules/1" do + test "walks workflow files and reports only the offending one", %{dir: dir} do + wf(dir, "clean.yml", @single) + wf(dir, "dirty.yml", @three_monday) + assert [finding] = CicdRules.scan_duplicate_cron_schedules(dir) + assert finding.file == ".github/workflows/dirty.yml" + end + end +end