diff --git a/standards/phase1/check.py b/standards/phase1/check.py index 4255442..38bea90 100644 --- a/standards/phase1/check.py +++ b/standards/phase1/check.py @@ -22,7 +22,8 @@ def _load(p: Path): try: - return yaml.safe_load(p.read_text()) or {} + res = yaml.safe_load(p.read_text()) + return {} if res is None else res except Exception as exc: # report, never crash the warn-only run return {"__error__": str(exc)} @@ -35,14 +36,17 @@ def check_repo(root: Path) -> list[dict]: for p in files: n = f".github/workflows/{p.name}" y = _load(p) + if not isinstance(y, dict): + add("yaml-parse", n, "top-level YAML document must be a mapping"); continue if "__error__" in y: - add("yaml-parse", n, y["__error__"][:200]); continue - jobs = y.get("jobs") or {} - if "permissions" not in y and not all("permissions" in (j or {}) for j in jobs.values()): + add("yaml-parse", n, str(y["__error__"])[:200]); continue + jobs = y.get("jobs") if isinstance(y.get("jobs"), dict) else {} + if "permissions" not in y and not all(isinstance(j, dict) and "permissions" in j for j in jobs.values()): add("least-privilege-permissions", n, "no top-level or per-job `permissions:`") top_conc = "concurrency" in y for jn, j in jobs.items(): - j = j or {} + if not isinstance(j, dict): + continue where = f"{n}#{jn}" if "uses" in j: # reusable workflow call u = str(j["uses"]) @@ -58,8 +62,10 @@ def check_repo(root: Path) -> list[dict]: add("concurrency", where, "no workflow- or job-level concurrency group") if j.get("continue-on-error") is True: add("no-masked-failures", where, "job continue-on-error: true") - for i, s in enumerate(j.get("steps") or []): - s = s or {} + steps = j.get("steps") if isinstance(j.get("steps"), list) else [] + for i, s in enumerate(steps): + if not isinstance(s, dict): + continue u = s.get("uses") if u and not str(u).startswith(("./", "docker://")) and not SHA.search(str(u)): add("pin-by-sha", f"{where} step {i+1}", f"action not pinned by SHA: {u}") @@ -73,10 +79,20 @@ def check_repo(root: Path) -> list[dict]: add("dependabot-configured", ".github/dependabot.yml", "missing") else: y = _load(db[0]) - for u in (y.get("updates") or []): - eco = u.get("package-ecosystem") - if (u.get("schedule") or {}).get("interval") != "weekly" or not u.get("groups"): - add("dependabot-grouped-weekly", ".github/dependabot.yml", f"{eco}: needs weekly schedule + groups") + n_db = f".github/{db[0].name}" + if not isinstance(y, dict): + add("yaml-parse", n_db, "top-level YAML document must be a mapping") + elif "__error__" in y: + add("yaml-parse", n_db, str(y["__error__"])[:200]) + else: + updates = y.get("updates") if isinstance(y.get("updates"), list) else [] + for u in updates: + if not isinstance(u, dict): + continue + eco = u.get("package-ecosystem") + sched = u.get("schedule") if isinstance(u.get("schedule"), dict) else {} + if sched.get("interval") != "weekly" or not u.get("groups"): + add("dependabot-grouped-weekly", n_db, f"{eco}: needs weekly schedule + groups") for man, locks in LOCKS.items(): if (root / man).exists() and not any((root / l).exists() for l in locks): add("lockfile-committed", man, "manifest at root without a committed lockfile") diff --git a/tests/test_phase1_warn.py b/tests/test_phase1_warn.py index 25b93f4..a5cf140 100644 --- a/tests/test_phase1_warn.py +++ b/tests/test_phase1_warn.py @@ -24,6 +24,33 @@ def test_warn_only_exit_zero_and_enforce_exit_one(self): self.assertEqual(check.main([str(FX / "fail")]), 0) self.assertEqual(check.main([str(FX / "fail"), "--enforce"]), 1) + def test_non_mapping_yaml_top_level_emits_finding_without_crash(self): + import tempfile + with tempfile.TemporaryDirectory() as tmpdir: + tmppath = Path(tmpdir) + wfdir = tmppath / ".github" / "workflows" + wfdir.mkdir(parents=True) + + # Non-empty sequence top level + (wfdir / "list.yml").write_text("- one\n- two\n") + # Scalar top level + (wfdir / "scalar.yml").write_text("just_a_string\n") + # Valid mapping control + (wfdir / "valid.yml").write_text("name: valid\non: push\njobs: {}\npermissions: {}\nconcurrency: c\n") + + findings = check.check_repo(tmppath) + yaml_parse_findings = [f for f in findings if f["rule"] == "yaml-parse"] + self.assertEqual(len(yaml_parse_findings), 2) + self.assertEqual(yaml_parse_findings[0]["where"], ".github/workflows/list.yml") + self.assertEqual(yaml_parse_findings[0]["message"], "top-level YAML document must be a mapping") + self.assertEqual(yaml_parse_findings[1]["where"], ".github/workflows/scalar.yml") + self.assertEqual(yaml_parse_findings[1]["message"], "top-level YAML document must be a mapping") + + # In warn-only mode (default), main exits 0 + self.assertEqual(check.main([str(tmppath)]), 0) + # Under --enforce, main exits 1 without a traceback + self.assertEqual(check.main([str(tmppath), "--enforce"]), 1) + class HonestGreen(unittest.TestCase): def test_all_success_is_green(self):