feat(standards): audit dependency alerts, deploy keys, webhooks and actions

README.md

@@ -90,7 +90,11 @@ Status: in progress. The design is in `specs/next-ci-reliability` of
   `standards/standards.json` and prints each deviation as `error`, `warning` or `info`: branch
   deletion after merge, Dependabot, CODEOWNERS, secret scanning and a recognised license on public
   repositories, a CI that calls the shared gate pinned by a full SHA with a `# vX.Y.Z` comment, too
-  many leftover branches, and the organisation's two-factor requirement. An exception names a
+  many leftover branches, the organisation's two-factor requirement, Dependabot alerts that are off
+  or open past their deadline (critical 7, high 30, medium 90 days, only when a fix exists),
+  writable deploy keys, webhooks without a secret, an unprotected default branch on public
+  repositories, and an organisation that does not restrict actions to a list pinned by SHA. The
+  audit only reports; the owner changes keys, webhooks and two-factor. An exception names a
   repository and a rule, gives a reason and expires; an expired one is itself an error.
   `py -3 standards/audit.py [--fail-on error]`, or `report/ci_report.py --audit` to append it to the
   weekly report.

standards/audit.py

@@ -11,8 +11,13 @@
 Rules (per repository unless noted): delete-branch-on-merge, dependabot-config, codeowners,
 secret-scanning (public repositories, where it is free), license (public repositories),
 has-ci, shared-gate, gate-pin (the shared gate is pinned by a full commit SHA with a vX.Y.Z comment,
-which is what online zizmor verifies), topic-branches, and for the organisation org-2fa and
-org-2fa-method. Archived repositories are skipped.
+which is what online zizmor verifies), topic-branches, dependabot-alerts (off) and
+dependabot-overdue (an open alert with a patched version past its deadline, 7/30/90 days for
+critical/high/medium, `dependency_deadline_days` overrides), deploy-key-write, webhook-secret,
+branch-protection (public repositories, where it is free), and for the organisation org-2fa,
+org-2fa-method and actions-allowlist (actions restricted to a list and pinned by SHA). The audit
+only reports: deploy keys, webhooks and two-factor are the owner's to change. Archived
+repositories are skipped.
 """
 
 from __future__ import annotations
@@ -44,8 +49,18 @@
     "shared-gate": "warning",
     "topic-branches": "warning",
     "org-2fa-method": "warning",
+    "dependabot-alerts": "warning",
+    "dependabot-overdue": "warning",
+    "branch-protection": "warning",
+    "deploy-key-write": "warning",
+    "webhook-secret": "info",
+    "actions-allowlist": "warning",
 }
 
+# Days an open alert with a patched version may stay open, by severity (spec org-security-baseline
+# FR1). `dependency_deadline_days` in the standard overrides it; a missing severity has no deadline.
+DEFAULT_DEADLINE_DAYS = {"critical": 7, "high": 30, "medium": 90}
+
 
 class StandardError(ValueError):
     """The standard file cannot be used."""
@@ -145,6 +160,48 @@ def insecure_two_factor_members(self, org: str) -> list[str]:
             return []  # needs organisation owner rights
         return [member["login"] for member in json.loads(out or "[]")]
 
+    def org_actions_permissions(self, org: str) -> dict:
+        try:
+            return json.loads(self._run(["gh", "api", f"orgs/{org}/actions/permissions"]) or "{}")
+        except subprocess.CalledProcessError:
+            return {}  # needs organisation owner rights
+
+    def dependabot_alerts(self, full_name: str) -> list[dict] | None:
+        """Open alerts, or None when Dependabot alerts are disabled or cannot be read."""
+        try:
+            out = self._run(
+                [
+                    "gh",
+                    "api",
+                    f"repos/{full_name}/dependabot/alerts?state=open&per_page=100",
+                    "--paginate",
+                    "--slurp",
+                ]
+            )
+        except subprocess.CalledProcessError:
+            return None
+        pages = json.loads(out or "[]")
+        return [item for page in pages for item in page] if pages and isinstance(pages[0], list) else pages
+
+    def branch_protected(self, full_name: str, branch: str) -> bool:
+        try:
+            self._run(["gh", "api", f"repos/{full_name}/branches/{branch}/protection"])
+        except subprocess.CalledProcessError:
+            return False
+        return True
+
+    def deploy_keys(self, full_name: str) -> list[dict]:
+        try:
+            return json.loads(self._run(["gh", "api", f"repos/{full_name}/keys"]) or "[]")
+        except subprocess.CalledProcessError:
+            return []
+
+    def hooks(self, full_name: str) -> list[dict]:
+        try:
+            return json.loads(self._run(["gh", "api", f"repos/{full_name}/hooks"]) or "[]")
+        except subprocess.CalledProcessError:
+            return []
+
 
 def validate_standard(standard: dict) -> None:
     for rule, severity in standard.get("severity", {}).items():
@@ -208,17 +265,59 @@ def add(repo: str, rule: str, message: str, level: str | None = None) -> None:
             "org-2fa-method",
             "members with an insecure second factor: " + ", ".join(insecure),
         )
+    permissions = gh.org_actions_permissions(org)
+    if permissions and (
+        permissions.get("allowed_actions") != "selected"
+        or not permissions.get("sha_pinning_required")
+    ):
+        add(
+            org,
+            "actions-allowlist",
+            "actions are not restricted to a list or not pinned by SHA "
+            f"(allowed_actions={permissions.get('allowed_actions')}, "
+            f"sha_pinning_required={bool(permissions.get('sha_pinning_required'))})",
+        )
 
     for repo in gh.org_repos(org):
         if repo.get("archived"):
             continue
         full = repo["full_name"]
         public = not repo.get("private", True)
-        _audit_repo(gh, standard, repo, full, public, add)
+        _audit_repo(gh, standard, repo, full, public, now, add)
     return findings
 
 
-def _audit_repo(gh, standard: dict, repo: dict, full: str, public: bool, add) -> None:
+def _audit_security(gh, standard: dict, full: str, now: dt.datetime, add) -> None:
+    deadlines = {**DEFAULT_DEADLINE_DAYS, **standard.get("dependency_deadline_days", {})}
+    alerts = gh.dependabot_alerts(full)
+    if alerts is None:
+        add(full, "dependabot-alerts", "Dependabot alerts are disabled or cannot be read")
+    for item in alerts or []:
+        patched = (item.get("security_vulnerability") or {}).get("first_patched_version")
+        level = (item.get("security_advisory") or {}).get("severity")
+        if not patched or level not in deadlines:
+            continue  # nothing to upgrade to yet, or a severity without a deadline
+        opened = dt.datetime.fromisoformat(item["created_at"].replace("Z", "+00:00"))
+        age = (now - opened).days
+        if age > deadlines[level]:
+            package = ((item.get("dependency") or {}).get("package") or {}).get("name", "?")
+            add(
+                full,
+                "dependabot-overdue",
+                f"{level} alert on `{package}` open {age} days, a fix exists "
+                f"(deadline {deadlines[level]} days)",
+            )
+    for key in gh.deploy_keys(full):
+        if not key.get("read_only", True):
+            add(full, "deploy-key-write", f"deploy key `{key.get('title', '?')}` can write")
+    for hook in gh.hooks(full):
+        if hook.get("active") and not (hook.get("config") or {}).get("secret"):
+            add(full, "webhook-secret", "an active webhook has no secret (the URL is the credential)")
+
+
+def _audit_repo(
+    gh, standard: dict, repo: dict, full: str, public: bool, now: dt.datetime, add
+) -> None:
     if not repo.get("delete_branch_on_merge"):
         add(
             full,
@@ -267,7 +366,15 @@ def _audit_repo(gh, standard: dict, repo: dict, full: str, public: bool, add) ->
         spdx = (repo.get("license") or {}).get("spdx_id")
         if spdx in (None, "NOASSERTION"):
             add(full, "license", "GitHub does not recognise the license")
+        branch = repo.get("default_branch", "main")
+        if not gh.branch_protected(full, branch):
+            add(
+                full,
+                "branch-protection",
+                f"`{branch}` is not protected (free on public repositories)",
+            )
 
+    _audit_security(gh, standard, full, now, add)
     _audit_gate(gh, workflow_names, full, add)
 
     limit = standard.get("max_topic_branches", 8)

tests/test_audit.py

@@ -46,18 +46,53 @@ def repo(
 
 class FakeGithub:
     def __init__(
-        self, repos, files=None, dirs=None, branches=None, two_fa=True, insecure=()
+        self,
+        repos,
+        files=None,
+        dirs=None,
+        branches=None,
+        two_fa=True,
+        insecure=(),
+        alerts=None,
+        protected=True,
+        keys=None,
+        hooks=None,
+        actions=None,
     ):
         self._repos = repos
         self._files = files or {}
         self._dirs = dirs or {}
         self._branches = branches or {}
         self._two_fa = two_fa
         self._insecure = list(insecure)
+        self._alerts = alerts or {}
+        self._protected = protected
+        self._keys = keys or {}
+        self._hooks = hooks or {}
+        self._actions = (
+            {"allowed_actions": "selected", "sha_pinning_required": True}
+            if actions is None
+            else actions
+        )
 
     def org(self, org):
         return {"two_factor_requirement_enabled": self._two_fa}
 
+    def org_actions_permissions(self, org):
+        return self._actions
+
+    def dependabot_alerts(self, full_name):
+        return self._alerts.get(full_name, [])
+
+    def branch_protected(self, full_name, branch):
+        return self._protected
+
+    def deploy_keys(self, full_name):
+        return self._keys.get(full_name, [])
+
+    def hooks(self, full_name):
+        return self._hooks.get(full_name, [])
+
     def org_repos(self, org):
         return self._repos
 
@@ -243,6 +278,90 @@ def test_members_with_an_insecure_second_factor_are_a_warning(self):
         self.assertIn("david", org_findings[0].message)
 
 
+def alert(severity, days_old, patched="1.2.3", package="left-pad"):
+    created = NOW - dt.timedelta(days=days_old)
+    return {
+        "created_at": created.isoformat().replace("+00:00", "Z"),
+        "dependency": {"package": {"name": package}},
+        "security_advisory": {"severity": severity},
+        "security_vulnerability": {
+            "first_patched_version": {"identifier": patched} if patched else None
+        },
+    }
+
+
+class SecurityRules(unittest.TestCase):
+    def world(self, **kw):
+        files, dirs = good_files("app")
+        return FakeGithub([repo("app")], files, dirs, **kw)
+
+    def test_disabled_dependabot_alerts_are_a_warning(self):
+        gh = self.world(alerts={f"{ORG}/app": None})
+        findings = audit.audit(gh, standard(), NOW)
+        self.assertEqual(rules(findings), {"dependabot-alerts"})
+        self.assertEqual(findings[0].severity, "warning")
+
+    def test_an_alert_inside_its_deadline_is_not_a_finding(self):
+        gh = self.world(alerts={f"{ORG}/app": [alert("critical", 6), alert("high", 29)]})
+        self.assertEqual(audit.audit(gh, standard(), NOW), [])
+
+    def test_an_alert_past_its_deadline_names_the_package_and_the_age(self):
+        gh = self.world(alerts={f"{ORG}/app": [alert("critical", 9, package="happy-dom")]})
+        findings = audit.audit(gh, standard(), NOW)
+        self.assertEqual(rules(findings), {"dependabot-overdue"})
+        self.assertIn("happy-dom", findings[0].message)
+        self.assertIn("9 days", findings[0].message)
+
+    def test_an_alert_without_a_patched_version_is_never_overdue(self):
+        gh = self.world(alerts={f"{ORG}/app": [alert("high", 400, patched=None)]})
+        self.assertEqual(audit.audit(gh, standard(), NOW), [])
+
+    def test_the_deadlines_come_from_the_standard(self):
+        gh = self.world(alerts={f"{ORG}/app": [alert("medium", 20)]})
+        std = standard(dependency_deadline_days={"medium": 10})
+        self.assertEqual(rules(audit.audit(gh, std, NOW)), {"dependabot-overdue"})
+
+    def test_a_public_repository_must_protect_its_default_branch(self):
+        files, dirs = good_files("lib")
+        gh = FakeGithub([repo("lib", private=False)], files, dirs, protected=False)
+        self.assertEqual(rules(audit.audit(gh, standard(), NOW), "lib"), {"branch-protection"})
+
+    def test_a_private_repository_is_not_asked_for_paid_branch_protection(self):
+        self.assertEqual(audit.audit(self.world(protected=False), standard(), NOW), [])
+
+    def test_a_writable_deploy_key_is_listed_by_title_without_key_material(self):
+        key = {"title": "release-tags", "read_only": False, "key": "ssh-ed25519 AAAA"}
+        gh = self.world(keys={f"{ORG}/app": [key, {"title": "ro", "read_only": True}]})
+        findings = audit.audit(gh, standard(), NOW)
+        self.assertEqual(rules(findings), {"deploy-key-write"})
+        self.assertIn("release-tags", findings[0].message)
+        self.assertNotIn("AAAA", findings[0].message)
+        self.assertEqual(len(findings), 1)
+
+    def test_a_webhook_without_a_secret_is_an_observation(self):
+        hook = {"active": True, "config": {"url": "https://discord.com/api/webhooks/x"}}
+        gh = self.world(hooks={f"{ORG}/app": [hook]})
+        findings = audit.audit(gh, standard(), NOW)
+        self.assertEqual(rules(findings), {"webhook-secret"})
+        self.assertEqual(findings[0].severity, "info")
+        self.assertNotIn("discord.com/api", findings[0].message)
+
+    def test_a_webhook_with_a_secret_is_fine(self):
+        hook = {"active": True, "config": {"url": "https://x.example/h", "secret": "********"}}
+        self.assertEqual(
+            audit.audit(self.world(hooks={f"{ORG}/app": [hook]}), standard(), NOW), []
+        )
+
+    def test_actions_must_be_restricted_and_pinned_by_sha(self):
+        gh = self.world(actions={"allowed_actions": "all", "sha_pinning_required": False})
+        findings = audit.audit(gh, standard(), NOW)
+        self.assertEqual({f.rule for f in findings}, {"actions-allowlist"})
+        self.assertEqual(len(findings), 1)
+
+    def test_unreadable_actions_permissions_are_skipped(self):
+        self.assertEqual(audit.audit(self.world(actions={}), standard(), NOW), [])
+
+
 class Exceptions(unittest.TestCase):
     def finding_world(self):
         files, dirs = good_files("app")
@@ -375,6 +494,33 @@ def test_insecure_two_factor_members_are_listed_by_login(self):
         self.outputs["2fa_insecure"] = json.dumps([{"login": "david"}])
         self.assertEqual(self.gh.insecure_two_factor_members(ORG), ["david"])
 
+    def test_disabled_dependabot_alerts_read_as_none(self):
+        self.errors["dependabot/alerts"] = "Dependabot alerts are disabled"
+        self.assertIsNone(self.gh.dependabot_alerts(f"{ORG}/app"))
+
+    def test_open_alerts_are_flattened_across_pages(self):
+        self.outputs["dependabot/alerts"] = json.dumps([[{"a": 1}], [{"a": 2}]])
+        self.assertEqual(self.gh.dependabot_alerts(f"{ORG}/app"), [{"a": 1}, {"a": 2}])
+        self.assertIn("state=open", " ".join(self.commands[0]))
+
+    def test_an_unprotected_branch_reads_as_false(self):
+        self.errors["branches/main/protection"] = "Branch not protected"
+        self.assertFalse(self.gh.branch_protected(f"{ORG}/app", "main"))
+
+    def test_a_protected_branch_reads_as_true(self):
+        self.outputs["branches/main/protection"] = "{}"
+        self.assertTrue(self.gh.branch_protected(f"{ORG}/app", "main"))
+
+    def test_keys_and_hooks_that_cannot_be_read_are_empty(self):
+        self.errors["/keys"] = "HTTP 403"
+        self.errors["/hooks"] = "HTTP 403"
+        self.assertEqual(self.gh.deploy_keys(f"{ORG}/app"), [])
+        self.assertEqual(self.gh.hooks(f"{ORG}/app"), [])
+
+    def test_actions_permissions_that_cannot_be_read_are_empty(self):
+        self.errors["actions/permissions"] = "HTTP 403"
+        self.assertEqual(self.gh.org_actions_permissions(ORG), {})
+
 
 if __name__ == "__main__":
     unittest.main()