diff --git a/.github/workflows/cla-assistant.yml b/.github/workflows/cla-assistant.yml index 4b00d04..5e6c0b6 100644 --- a/.github/workflows/cla-assistant.yml +++ b/.github/workflows/cla-assistant.yml @@ -59,9 +59,20 @@ # was a former colleague still exempt from signing. # # Bots stay, because a bot cannot sign: it cannot post the agreement sentence, -# so with no exemption its pull request is red forever. Only the two that -# actually open pull requests here are listed. `github-actions[bot]` needs no -# entry, since the action filters user id 41898282 in `graphql.ts` itself. +# so with no exemption its pull request is red forever. Three are listed, and +# the check reads COMMITTERS rather than pull request authors. +# `dependabot[bot]` and `mindsdb-release-train[bot]` open their own pull +# requests and commit them. `snyk-bot` opens none: a human opens the Snyk +# upgrade pull request and `snyk-bot` is the committer inside it, so such a PR +# has two committers and the human still signs. +# +# `snyk-bot` is also the one entry with no `[bot]` suffix. GitHub reserves that +# suffix for the account it mints alongside a GitHub App; Snyk commits from an +# ordinary account it owns, which GitHub types "User". Listing it as +# `snyk-bot[bot]` matched nobody, since matching is `pattern === committer`. +# +# `github-actions[bot]` needs no entry, since the action filters user id +# 41898282 in `graphql.ts` itself. # # Add a bot by editing the default below, not by passing the input. Anything # passed per repo drifts, which is the whole reason this moved. @@ -85,7 +96,7 @@ on: allowlist: description: "Comma-separated logins exempt from signing. Bots only; see the header. Override only for a genuine one-off." type: string - default: 'dependabot[bot], mindsdb-release-train[bot], snyk-bot[bot]' + default: 'dependabot[bot], mindsdb-release-train[bot], snyk-bot' path-to-signatures: description: "Signature ledger path inside the calling repo" type: string diff --git a/README.md b/README.md index 2358e31..cea57bb 100644 --- a/README.md +++ b/README.md @@ -536,10 +536,15 @@ an employment agreement was in place and covered the work. Bots stay, because a bot cannot sign. It cannot post the agreement sentence, so without an exemption its pull request is red forever, and `lightwood` alone has -36 Dependabot pull requests in its last 100. Two are listed, `dependabot[bot]` -and `mindsdb-release-train[bot]`, which are the only two that open pull requests -anywhere in the fleet. `github-actions[bot]` needs no entry: the action already -filters user id 41898282 in `graphql.ts`. +36 Dependabot pull requests in its last 100. Three are listed, and the check +reads committers rather than pull request authors. `dependabot[bot]` and +`mindsdb-release-train[bot]` open their own pull requests and commit them. +`snyk-bot` opens none: a human opens the Snyk upgrade pull request and +`snyk-bot` is the committer inside it, so such a PR has two committers and the +human still signs. `snyk-bot` is also the one entry with no `[bot]` suffix, +because GitHub reserves that suffix for the account it mints alongside a GitHub +App and Snyk commits from an ordinary account it owns. `github-actions[bot]` +needs no entry: the action already filters user id 41898282 in `graphql.ts`. **Never put a `*` in that list.** The action compiles `bot*` to `new RegExp("bot.*")` and calls `.test()`, which is unanchored and searches diff --git a/tests/test_cla_allowlist.py b/tests/test_cla_allowlist.py index 78ca8fb..5e98a91 100644 --- a/tests/test_cla_allowlist.py +++ b/tests/test_cla_allowlist.py @@ -27,6 +27,16 @@ # weight that reads like a decision. FILTERED_UPSTREAM = "github-actions[bot]" +# `dependabot[bot]` and `mindsdb-release-train[bot]` are GitHub App identities, +# where `[bot]` is part of the login and GitHub reserves it. Snyk instead opens +# its upgrade PRs from an ordinary account it owns, which GitHub reports as type +# "User" (`snyk-bot`, snyk-bot@snyk.io), so that login carries no suffix. +# +# The suffix therefore proves an entry is a bot, but its absence does not prove +# the entry is a person. Every such machine account is named here so the check +# below still fails on a human login that nobody named. +MACHINE_USERS = frozenset({"snyk-bot"}) + def allowlist_default() -> str: spec = yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8")) @@ -59,8 +69,14 @@ def test_every_entry_is_a_bot(): Eleven of the twenty-five names in the old per-repo lists had already left the org and were still exempt. + + An entry clears this either by carrying the reserved `[bot]` suffix or by + being named in `MACHINE_USERS`. Adding a person still means editing that + constant in front of a reviewer, rather than passing a suffix check. """ - assert [e for e in entries() if not e.endswith("[bot]")] == [] + assert [ + e for e in entries() if not e.endswith("[bot]") and e not in MACHINE_USERS + ] == [] def test_github_actions_bot_is_not_listed(): @@ -86,7 +102,14 @@ def action_matches(pattern: str, committer: str) -> bool: assert not any(action_matches(e, impostor) for e in entries()) -@pytest.mark.parametrize("bot", ["dependabot[bot]", "mindsdb-release-train[bot]"]) +@pytest.mark.parametrize( + "bot", ["dependabot[bot]", "mindsdb-release-train[bot]", "snyk-bot"] +) def test_the_bots_that_actually_open_pull_requests_stay_exempt(bot): - """A bot cannot post the agreement sentence, so dropping it means a permanent red check.""" + """A bot cannot post the agreement sentence, so dropping it means a permanent red check. + + The exact login is the assertion. `snyk-bot` was listed as `snyk-bot[bot]` + for a while and exempted nobody, because the action compares + `pattern === committer` and the real committer login has no suffix. + """ assert bot in entries()