From f90ee618a5298d099cc125107b4e17a29e2f4b1a Mon Sep 17 00:00:00 2001 From: SNatangelo Date: Tue, 3 Nov 2026 09:51:59 +0100 Subject: [PATCH] Recognize verified Dependabot contributions in CLA checks --- .github/cla/cla_bot.py | 67 +++++++++- tests/test_cla_bot.py | 282 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+), 7 deletions(-) create mode 100644 tests/test_cla_bot.py diff --git a/.github/cla/cla_bot.py b/.github/cla/cla_bot.py index d3de504f..cbf02417 100644 --- a/.github/cla/cla_bot.py +++ b/.github/cla/cla_bot.py @@ -21,6 +21,9 @@ SOURCE = "SNatangelo/Callimachus" ARCHIVE = "SNatangelo/callimachus-legal" OWNER_ID = 248719534 +DEPENDABOT_ID = 49699333 +DEPENDABOT_LOGIN = "dependabot[bot]" +DEPENDABOT_EXEMPTION_REASON = "verified_same_source_repository_dependabot_pr_opener" VERSION = "1.1" AGREEMENT_PATH = ".github/cla/agreements/CLA-v1.1.md" AGREEMENT_SHA256 = "f89491e1cf64c52a5119adabe9928c665af0392c23af76258ce4446f003d9634" @@ -127,9 +130,49 @@ def valid_signature(record, user_id): and bool(record.get("accepted_at"))) -def contributor_ids(api: API, pr_number: int, opener): - """Include the submitter and every GitHub-linked author/coauthor, not just committer.""" - people = {opener["id"]: opener["login"]} +def trusted_dependabot_exemption(pr): + """Return an auditable exemption only for the exact same-repository bot PR.""" + if not isinstance(pr, dict): + return None + opener = pr.get("user") or {} + head_repo = ((pr.get("head") or {}).get("repo") or {}) + base_repo = ((pr.get("base") or {}).get("repo") or {}) + head_name = head_repo.get("full_name") + base_name = base_repo.get("full_name") + if ( + opener.get("id") != DEPENDABOT_ID + or opener.get("login") != DEPENDABOT_LOGIN + or opener.get("type") != "Bot" + or not isinstance(head_name, str) + or head_name.lower() != SOURCE.lower() + or head_repo.get("fork") is not False + or not isinstance(base_name, str) + or base_name.lower() != SOURCE.lower() + ): + return None + return { + "github_user_id": DEPENDABOT_ID, + "github_username": DEPENDABOT_LOGIN, + "identity_type": "Bot", + "reason": DEPENDABOT_EXEMPTION_REASON, + "head_repository": head_name, + "base_repository": base_name, + } + + +def contributor_ids(api: API, pr_number: int, opener, pr_context=None): + """Include the submitter and every GitHub-linked author/coauthor. + + A bot opener is excluded only when the full GitHub PR context independently + verifies the exact Dependabot bot and same-source-repository head. All commit + authors are still enumerated, and unlinked authors remain a hard failure. + """ + automation_exemption = trusted_dependabot_exemption(pr_context) + if pr_context is not None: + context_opener = pr_context.get("user") or {} + if any(opener.get(key) != context_opener.get(key) for key in ("id", "login", "type")): + raise Failure("Pull request opener context mismatch; manual review required") + people = {} if automation_exemption else {opener["id"]: opener["login"]} query = """query($number:Int!, $after:String) { repository(owner:"SNatangelo", name:"Callimachus") { pullRequest(number:$number) { commits(first:100, after:$after) { @@ -154,7 +197,13 @@ def contributor_ids(api: API, pr_number: int, opener): user = actor.get("user") if not user or not isinstance(user.get("databaseId"), int): raise Failure("An author/coauthor is not linked to GitHub; resolve attribution before merge") - people[user["databaseId"]] = user["login"] + uid = user["databaseId"] + login = user.get("login") + if automation_exemption and uid == DEPENDABOT_ID: + if login != DEPENDABOT_LOGIN: + raise Failure("Dependabot author identity differs from the verified PR opener") + continue + people[uid] = login if not connection["pageInfo"]["hasNextPage"]: # The original maintainer already owns his own contributions. No bot wildcard exemptions. people.pop(OWNER_ID, None) @@ -202,7 +251,8 @@ def status(state, description): "schema": 1, "version": VERSION, "sha256": AGREEMENT_SHA256, "source_repository": SOURCE, "source_path": AGREEMENT_PATH, "archive_path": archive_path, "signature_text": SIGN})) - people = contributor_ids(public, number, pr["user"]) + automation_exemption = trusted_dependabot_exemption(pr) + people = contributor_ids(public, number, pr["user"], pr_context=pr) comments = list(public.pages(f"/repos/{SOURCE}/issues/{number}/comments")) prompts = [c for c in comments if MARKER in (c.get("body") or "") and (c.get("user") or {}).get("login") == "github-actions[bot]" @@ -265,10 +315,13 @@ def status(state, description): status("failure", "CLA acceptance missing; follow the bot comment") return # This record binds the successful check to the exact PR head and version. - put_once(private, f"checks/v{VERSION}/pr-{number}/{sha}.json", json_bytes({ + check_record = { "schema": 1, "agreement_sha256": AGREEMENT_SHA256, "repository": SOURCE, "pull_request": number, "head_sha": sha, - "github_user_ids": sorted(people), "maintainer_exemption_id": OWNER_ID})) + "github_user_ids": sorted(people), "maintainer_exemption_id": OWNER_ID} + if automation_exemption: + check_record["automation_exemptions"] = [automation_exemption] + put_once(private, f"checks/v{VERSION}/pr-{number}/{sha}.json", json_bytes(check_record)) status("success", "CLA acceptance verified; versioned evidence preserved privately") except Exception: status("failure", "CLA check/archive unavailable; inspect workflow configuration") diff --git a/tests/test_cla_bot.py b/tests/test_cla_bot.py new file mode 100644 index 00000000..7f8a2de4 --- /dev/null +++ b/tests/test_cla_bot.py @@ -0,0 +1,282 @@ +import hashlib +import json +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / ".github" / "cla")) +import cla_bot + + +BOT_ID = 49699333 +BOT_LOGIN = "dependabot[bot]" +HUMAN_ID = 91234567 +HUMAN_LOGIN = "alice" + + +def make_pr(*, user=None, head_repository=cla_bot.SOURCE, head_fork=False, + base_repository=cla_bot.SOURCE, head_ref="dependabot/update"): + return { + "user": user or {"id": BOT_ID, "login": BOT_LOGIN, "type": "Bot"}, + "head": { + "ref": head_ref, + "repo": {"full_name": head_repository, "fork": head_fork}, + }, + "base": {"repo": {"full_name": base_repository}}, + } + + +class GraphQLAPI: + def __init__(self, authors): + self.authors = authors + + def request(self, method, path, data): + assert method == "POST" + assert path == "/graphql" + assert data["variables"]["number"] == 18 + return { + "data": { + "repository": { + "pullRequest": { + "commits": { + "nodes": [{ + "commit": { + "authors": { + "nodes": self.authors, + "pageInfo": {"hasNextPage": False}, + } + } + }], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + } + } + } + } + } + + +def author(user_id, login): + return {"user": {"databaseId": user_id, "login": login}} + + +def test_only_exact_same_repository_dependabot_bot_opener_is_exempt(): + pr = make_pr() + assert cla_bot.trusted_dependabot_exemption(pr) == { + "github_user_id": BOT_ID, + "github_username": BOT_LOGIN, + "identity_type": "Bot", + "reason": cla_bot.DEPENDABOT_EXEMPTION_REASON, + "head_repository": cla_bot.SOURCE, + "base_repository": cla_bot.SOURCE, + } + + +@pytest.mark.parametrize( + "pr", + [ + make_pr(head_repository="fork-owner/Callimachus", head_fork=True), + make_pr(base_repository="somewhere-else/Callimachus"), + make_pr(user={"id": 999, "login": BOT_LOGIN, "type": "Bot"}), + make_pr(user={"id": BOT_ID, "login": "someone-else[bot]", "type": "Bot"}), + make_pr(user={"id": BOT_ID, "login": BOT_LOGIN, "type": "User"}), + make_pr(user={"id": 999, "login": "someone-else[bot]", "type": "Bot"}), + ], +) +def test_fork_and_spoofed_bot_identities_are_not_exempt(pr): + assert cla_bot.trusted_dependabot_exemption(pr) is None + people = cla_bot.contributor_ids( + GraphQLAPI([]), 18, pr["user"], pr_context=pr + ) + assert pr["user"]["id"] in people + + +def test_mixed_dependabot_pr_still_requires_human_author(): + pr = make_pr(head_ref="dependabot/security-update") + exemption = cla_bot.trusted_dependabot_exemption(pr) + assert exemption is not None + people = cla_bot.contributor_ids( + GraphQLAPI([ + author(BOT_ID, BOT_LOGIN), + author(HUMAN_ID, HUMAN_LOGIN), + author(cla_bot.OWNER_ID, "SNatangelo"), + ]), + 18, + pr["user"], + pr_context=pr, + ) + assert people == {HUMAN_ID: HUMAN_LOGIN} + + +def test_forked_dependabot_pr_keeps_bot_and_human_authors_required(): + pr = make_pr(head_repository="fork-owner/Callimachus", head_fork=True, + head_ref="dependabot/security-update") + people = cla_bot.contributor_ids( + GraphQLAPI([author(BOT_ID, BOT_LOGIN), author(HUMAN_ID, HUMAN_LOGIN)]), + 18, + pr["user"], + pr_context=pr, + ) + assert people == {BOT_ID: BOT_LOGIN, HUMAN_ID: HUMAN_LOGIN} + + +def test_verified_bot_does_not_bypass_unlinked_human_author(): + pr = make_pr() + with pytest.raises(cla_bot.Failure, match="not linked to GitHub"): + cla_bot.contributor_ids( + GraphQLAPI([author(BOT_ID, BOT_LOGIN), {"user": None}]), + 18, + pr["user"], + pr_context=pr, + ) + + +def test_verified_bot_identity_mismatch_in_commit_authors_fails_closed(): + pr = make_pr() + with pytest.raises(cla_bot.Failure, match="identity differs"): + cla_bot.contributor_ids( + GraphQLAPI([author(BOT_ID, "not-dependabot[bot]")]), + 18, + pr["user"], + pr_context=pr, + ) + + +def test_bot_cannot_supply_human_acceptance(): + prompted_at = "2026-01-01T00:00:00Z" + bot_comment = { + "user": {"id": BOT_ID, "login": BOT_LOGIN, "type": "Bot"}, + "body": cla_bot.SIGN, + "created_at": "2026-01-02T00:00:00Z", + } + human_comment = { + "user": {"id": HUMAN_ID, "login": HUMAN_LOGIN, "type": "User"}, + "body": cla_bot.SIGN, + "created_at": "2026-01-02T00:00:00Z", + } + assert not cla_bot.comment_is_acceptance(bot_comment, prompted_at, {BOT_ID, HUMAN_ID}) + assert cla_bot.comment_is_acceptance(human_comment, prompted_at, {HUMAN_ID}) + + +class WorkflowPublicAPI: + def __init__(self, pr, authors): + self.pr = pr + self.authors = authors + + def request(self, method, path, data=None): + if method == "GET" and path == f"/repos/{cla_bot.SOURCE}/pulls/18": + return self.pr + if method == "POST" and path == "/graphql": + return { + "data": { + "repository": { + "pullRequest": { + "commits": { + "nodes": [{ + "commit": { + "authors": { + "nodes": self.authors, + "pageInfo": {"hasNextPage": False}, + } + } + }], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + } + } + } + } + } + if method == "POST" and ( + path.startswith(f"/repos/{cla_bot.SOURCE}/statuses/") + or path == f"/repos/{cla_bot.SOURCE}/issues/18/comments" + ): + return {} + raise AssertionError(f"Unexpected public API request: {method} {path}") + + def pages(self, path): + assert path == f"/repos/{cla_bot.SOURCE}/issues/18/comments" + return [] + + +class WorkflowPrivateAPI: + token = "test-token" + + def request(self, method, path, data=None): + if method == "GET" and path == f"/repos/{cla_bot.ARCHIVE}": + return {"private": True, "full_name": cla_bot.ARCHIVE} + if method == "GET" and path == f"/repos/{cla_bot.ARCHIVE}/branches/main": + return {} + raise AssertionError(f"Unexpected private API request: {method} {path}") + + +def run_successful_check_record(monkeypatch, *, trusted_bot): + document = b"test-only CLA text" + agreement_hash = hashlib.sha256(document).hexdigest() + monkeypatch.setattr(cla_bot, "AGREEMENT_SHA256", agreement_hash) + head_sha = "a" * 40 + if trusted_bot: + pr = make_pr() + authors = [author(BOT_ID, BOT_LOGIN), author(HUMAN_ID, HUMAN_LOGIN)] + else: + opener = {"id": HUMAN_ID, "login": HUMAN_LOGIN, "type": "User"} + pr = make_pr(user=opener) + authors = [author(HUMAN_ID, HUMAN_LOGIN)] + pr.update({ + "state": "open", + "head": {**pr["head"], "sha": head_sha}, + "html_url": f"https://github.com/{cla_bot.SOURCE}/pull/18", + }) + + def fake_read_file(api, repo, path, ref="main"): + if repo == cla_bot.SOURCE and path in (cla_bot.AGREEMENT_PATH, "CLA.md"): + return document + if repo == cla_bot.ARCHIVE and path.startswith(f"signatures/v{cla_bot.VERSION}/github-"): + record = { + "schema": 1, + "github_user_id": HUMAN_ID, + "agreement_version": cla_bot.VERSION, + "agreement_sha256": agreement_hash, + "acceptance_method": "github_comment_own_rights", + "comment_body": cla_bot.SIGN, + "comment_id": 1, + "accepted_at": "2026-01-02T00:00:00Z", + } + return cla_bot.json_bytes(record) + return None + + writes = [] + monkeypatch.setattr(cla_bot, "read_file", fake_read_file) + monkeypatch.setattr(cla_bot, "put_once", lambda api, path, content: writes.append((path, content))) + cla_bot.execute( + WorkflowPublicAPI(pr, authors), + WorkflowPrivateAPI(), + {"repository": {"id": 123}, "pull_request": {"number": 18}}, + { + "GITHUB_REPOSITORY": cla_bot.SOURCE, + "GITHUB_RUN_ID": "12345", + "CLA_WORKFLOW_SHA": "b" * 40, + }, + ) + path = f"checks/v{cla_bot.VERSION}/pr-18/{head_sha}.json" + return next(content for written_path, content in writes if written_path == path), agreement_hash + + +def test_nonbot_check_record_keeps_legacy_bytes_and_bot_is_audited_separately(monkeypatch): + human_bytes, agreement_hash = run_successful_check_record(monkeypatch, trusted_bot=False) + legacy_record = { + "schema": 1, + "agreement_sha256": agreement_hash, + "repository": cla_bot.SOURCE, + "pull_request": 18, + "head_sha": "a" * 40, + "github_user_ids": [HUMAN_ID], + "maintainer_exemption_id": cla_bot.OWNER_ID, + } + assert human_bytes == cla_bot.json_bytes(legacy_record) + assert "automation_exemptions" not in json.loads(human_bytes) + + bot_bytes, _ = run_successful_check_record(monkeypatch, trusted_bot=True) + bot_record = json.loads(bot_bytes) + exemption = cla_bot.trusted_dependabot_exemption(make_pr()) + assert bot_record["github_user_ids"] == [HUMAN_ID] + assert bot_record["automation_exemptions"] == [exemption]