From 726734a39f801952d9eda2fc59d42dc0089aa400 Mon Sep 17 00:00:00 2001 From: naindev Date: Fri, 16 Oct 2026 09:36:29 +0200 Subject: [PATCH] feat(standards): audit dependency alerts, deploy keys, webhooks and actions New read-only rules: dependabot-alerts (off), dependabot-overdue (open alert with a patched version past 7/30/90 days for critical/high/medium), deploy-key-write, webhook-secret, branch-protection on public repositories and actions-allowlist for the organisation (restricted list, SHA pinning). All warnings or info, so a CI step with --fail-on error keeps its meaning. The audit reports and never changes keys, webhooks or two-factor. --- README.md | 6 +- standards/audit.py | 115 ++++++++++++++++++++++++++++++++-- tests/test_audit.py | 148 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 263 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3eeaab7..b092ed0 100644 --- a/README.md +++ b/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. diff --git a/standards/audit.py b/standards/audit.py index 6474ceb..d43f966 100644 --- a/standards/audit.py +++ b/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) diff --git a/tests/test_audit.py b/tests/test_audit.py index a32866a..e3f71cb 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -46,7 +46,18 @@ 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 {} @@ -54,10 +65,34 @@ def __init__( 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()