diff --git a/.github/workflows/cla-assistant.yml b/.github/workflows/cla-assistant.yml index 44bc9f0..87ccff7 100644 --- a/.github/workflows/cla-assistant.yml +++ b/.github/workflows/cla-assistant.yml @@ -1,5 +1,5 @@ # Reusable workflow: the CLA signature check on pull requests from outside -# contributors, for the public repos (cowork, cowork-server, anton). +# contributors, for the public repos. # # The bot comments on an unsigned PR, records a signature when the contributor # replies with the agreement sentence, and re-checks on `recheck`. Signatures are @@ -11,7 +11,7 @@ # anton/.github/workflows/cla.yml # name: "MindsDB Anton CLA Assistant" # permissions: -# actions: write +# actions: read # contents: write # pull-requests: write # statuses: write @@ -24,12 +24,28 @@ # cla: # uses: mindsdb/github-actions/.github/workflows/cla-assistant.yml@ # v1 # with: -# path-to-document: 'https://github.com/mindsdb/mindsdb/blob/main/assets/contributions-agreement/individual-contributor.md' +# path-to-document: 'https://github.com/mindsdb/mindshub/blob/main/assets/contributions-agreement/individual-contributor.md' # allowlist: bot*, ZoranPandovski, ... # # The caller must declare those four permissions: the action writes the # signature file, comments on the PR, and sets the commit status, and a called # workflow can never hold more than its caller grants. +# +# `actions` is READ, not write. The only write it would buy is +# `pullRerunRunner.ts`'s re-run of a previously failed CLA run, and that API +# refuses a `GITHUB_TOKEN`, so the call fails and the action swallows the error. +# The scope cannot be dropped entirely: the same file lists the repo's workflows +# first, and `main.ts` turns any throw into a failed job. +# +# There is no runner input. This check calls the GitHub API and nothing else, so +# it has no reason to sit on a pod inside our clusters, and leaving a knob here +# is how ten repos ended up pointing it at `mdb-dev`. +# +# The ledger is created here rather than assumed, branch and file both. The +# action writes with the contents API, which 404s on a branch that does not +# exist; and its own handler for a missing FILE is dead code, because it +# compares `error.status` against the string `"404"`. A repo adopting this +# workflow would otherwise fail on its first signature either way. name: CLA Assistant @@ -52,24 +68,83 @@ on: description: "Branch the signature ledger is committed to" type: string default: 'cla' - runs-on: - description: "Runner label for the check" - type: string - default: ubuntu-latest permissions: - actions: write + actions: read contents: write pull-requests: write statuses: write jobs: CLAssistant: - runs-on: ${{ inputs.runs-on }} + # Gate the JOB, not the step. Every caller also triggers on `issue_comment`, + # which fires on a comment on any issue in the repo, from any account. At + # step level that still starts a runner and then does nothing; here the run + # reports the job as skipped and claims nothing. + # + # Every `pull_request_target` runs, and that is deliberate: the bot has to + # look at each opened, synchronized and closed PR to decide whether its + # author has signed. Narrowing this to comments only is what leaves a repo + # with a CLA check that never asks anybody to sign. + if: >- + github.event_name == 'pull_request_target' + || github.event.comment.body == 'recheck' + || github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA' + runs-on: ubuntu-latest steps: + # The ledger needs a branch AND a file, and the action supplies neither. + # + # The branch: it writes through the contents API, and that API answers 404 + # for a `branch:` that does not exist rather than creating one. + # + # The file: the action does try to handle that itself, and the code is + # dead. `setupClaCheck.ts` guards its create path with + # `error.status === "404"`, a string compared strictly against Octokit's + # numeric `RequestError.status`, so the branch never runs. A missing + # ledger file falls through to `Could not retrieve repository contents. + # Status: 404`, which `main.ts` turns into a failed job. Read out of + # `dist/index.js` at the pinned commit, which is what actually executes. + # + # So both halves are created here. Upstream is archived, so waiting for a + # fix upstream is waiting forever, and without this the first contributor + # to a new repo gets a red check with nothing to act on instead of being + # asked to sign. + - name: Ensure the signature ledger exists + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + LEDGER_BRANCH: ${{ inputs.branch }} + LEDGER_PATH: ${{ inputs.path-to-signatures }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + if gh api "/repos/${REPO}/git/ref/heads/${LEDGER_BRANCH}" >/dev/null 2>&1; then + # Expect: this is the path every run after the first one takes + echo "Ledger branch ${LEDGER_BRANCH} already exists." + else + BASE=$(gh api "/repos/${REPO}" --jq .default_branch) + SHA=$(gh api "/repos/${REPO}/git/ref/heads/${BASE}" --jq .object.sha) + # Expect: refs/heads/, created at the default branch tip + gh api -X POST "/repos/${REPO}/git/refs" \ + -f ref="refs/heads/${LEDGER_BRANCH}" -f sha="${SHA}" --jq .ref + fi + + if gh api "/repos/${REPO}/contents/${LEDGER_PATH}?ref=${LEDGER_BRANCH}" >/dev/null 2>&1; then + # Expect: this is the path every run after the first one takes + echo "Ledger file ${LEDGER_PATH} already exists on ${LEDGER_BRANCH}." + exit 0 + fi + # Three-space indent because that is what the action itself writes + # (`JSON.stringify(content, null, 3)`), so the first real signature + # lands as a one-line diff rather than a reformat of the whole file. + CONTENT=$(printf '{\n "signedContributors": []\n}' | base64 -w0) + # Expect: the blob SHA of the empty ledger, committed on the branch + gh api -X PUT "/repos/${REPO}/contents/${LEDGER_PATH}" \ + -f message="Create the CLA signature ledger" \ + -f branch="${LEDGER_BRANCH}" \ + -f content="${CONTENT}" --jq .content.sha + - name: "CLA Assistant" - if: (github.event.comment.body == 'recheck' || github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA') || github.event_name == 'pull_request_target' - uses: contributor-assistant/github-action@v2.6.1 + uses: contributor-assistant/github-action@ca4a40a7d1004f18d9960b404b97e5f30a505a08 # v2.6.1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/README.md b/README.md index e3caa23..d1c3290 100644 --- a/README.md +++ b/README.md @@ -357,19 +357,68 @@ the group name) and skipping auto-version commits with `cla-assistant.yml` runs the contributor-agreement check on the public repos. The wrapper keeps the `issue_comment` + `pull_request_target` triggers (the action -reads those payloads directly) and the four write permissions, and passes the +reads those payloads directly) and declares the permissions, and passes the per-repo agreement URL and allowlist: ```yaml +permissions: + actions: read + contents: write + pull-requests: write + statuses: write + +jobs: cla: uses: mindsdb/github-actions/.github/workflows/cla-assistant.yml@ # v1 with: - path-to-document: 'https://github.com/mindsdb/mindsdb/blob/main/assets/contributions-agreement/individual-contributor.md' + path-to-document: 'https://github.com/mindsdb/mindshub/blob/main/assets/contributions-agreement/individual-contributor.md' allowlist: bot*, ZoranPandovski, ... ``` +`mindsdb/mindshub` is the repository that holds the agreement. `mindsdb/mindsdb` +and `mindsdb/minds` both still resolve to it through a rename redirect, and +neither belongs in a new caller: a redirect stops being harmless the moment +somebody creates a repository at the old name, and this is the page a +contributor reads before agreeing to it. + Signatures are committed to the calling repo's own `cla` branch, so each repo -keeps its own ledger. +keeps its own ledger. `path-to-signatures` and `branch` default to that shape; +pass them only where a repo already keeps its ledger somewhere else. + +**A new caller needs no setup, because the reusable creates the ledger itself.** +Both halves of it, and the action supplies neither. The branch: the action writes +through the contents API, and that API answers 404 for a `branch:` that does not +exist rather than creating one. The file: the action does try, and the code is +dead — `setupClaCheck.ts` guards its create path with `error.status === "404"`, +a string compared strictly against Octokit's numeric `RequestError.status`, so +the branch never runs and a missing ledger fails the job with `Could not +retrieve repository contents. Status: 404` instead. Upstream is archived, so the +first step creates the branch at the default-branch tip and seeds an empty +ledger, byte-identical to what the action would have written. Every run after +the first short-circuits on two API calls. + +**There is no runner input, and that is the point.** This check calls the GitHub +API and nothing else, so it never needs a pod in our clusters. Ten public repos +reached `mdb-dev` through a hand-rolled copy of this job, and an outside account +opening a pull request or leaving a comment started every one of those runs with +no approval, because `pull_request_target` and `issue_comment` both execute in +base-repo context. + +**The permission that is read rather than write is `actions`.** The upstream +README asks for `actions: write`. The only write it buys is +`pullRerunRunner.ts` re-running a previously failed CLA run, and that API +refuses a `GITHUB_TOKEN`, so the call fails and the action logs and continues. +Dropping the scope altogether does break it: the same file lists the repo's +workflows first, and `main.ts` turns any throw into a failed job. + +**The job carries the event filter, not the step.** `issue_comment` fires on a +comment on any issue in the repo, from any account. Filtering inside the step +still starts a runner for every one of them. Filtering on the job means the run +reports it as skipped. Every `pull_request_target` still runs, because the bot +has to look at each opened, synchronized and closed pull request to decide +whether its author has signed. Four repos learned that the hard way, gating on +`github.event_name == 'pull_request'` while triggering on events that are not +`pull_request`, so their check quietly never asked anybody to sign. ### Prerequisites (provisioned once, org level, scoped to the release-train repos)