diff --git a/.github/workflows/coreutils-args-drift.yml b/.github/workflows/coreutils-args-drift.yml index 79c13183e..0e73a7d86 100644 --- a/.github/workflows/coreutils-args-drift.yml +++ b/.github/workflows/coreutils-args-drift.yml @@ -16,6 +16,10 @@ name: Coreutils Args Drift # build, and tests run without repository write permission and without # persisted checkout credentials. Only the later PR job gets write # permission, and it only commits generated files after tests pass. +# +# Token split: the drift branch is pushed with the job token, but the PR is +# opened with a Doppler-resolved PAT, because the job token is not permitted +# to create pull requests. See the comment on the PR steps below. on: schedule: @@ -271,19 +275,52 @@ jobs: exit 1 fi - - name: Commit and open PR + # The job token cannot open pull requests. With "Allow GitHub Actions + # to create and approve pull requests" off — the safer setting, since + # the same switch also governs approvals — `gh pr create` is rejected + # with "GitHub Actions is not permitted to create or approve pull + # requests". Run 39234801977 regenerated the drift, pushed it, then + # died on that call: red run, no PR, drift invisible for a week. + # + # Pushing only needs `contents: write`, which the job token has, so + # the two are split: the branch goes up on the job token, the PR is + # opened with a PAT resolved from Doppler — this repo's existing + # pattern for privileged CI credentials (see ci.yml). With no PAT the + # job fails saying exactly that, instead of with a GraphQL error that + # reads like a bug in the drift tooling. + - name: Detect Doppler availability + run: echo "DOPPLER_AVAILABLE=${{ secrets.DOPPLER_TOKEN != '' }}" >> "$GITHUB_ENV" + + - name: Install Doppler CLI + if: env.DOPPLER_AVAILABLE == 'true' + uses: dopplerhq/cli-action@4819d808ab99e5cde19a0637a16536a4038fad73 # v4 + + # Resolve only the required key, and mask it before it crosses the + # step boundary. Non-fatal: a missing key is reported by the guard in + # the PR step, which can name what is actually wrong. + - name: Fetch pull-request token + id: pat + if: env.DOPPLER_AVAILABLE == 'true' + continue-on-error: true + shell: bash + env: + DOPPLER_TOKEN: ${{ secrets.DOPPLER_TOKEN }} + run: | + set -euo pipefail + token="$(doppler secrets get GITHUB_TOKEN --plain)" + printf '::add-mask::%s\n' "$token" + printf 'token=%s\n' "$token" >> "$GITHUB_OUTPUT" + + - name: Commit and push drift branch working-directory: bashkit env: GH_TOKEN: ${{ github.token }} NEW_REV: ${{ needs.regenerate.outputs.rev }} - OLD_REV: ${{ needs.regenerate.outputs.old_rev }} UTILS: ${{ needs.regenerate.outputs.utils }} MODULES: ${{ needs.regenerate.outputs.modules }} run: | set -euo pipefail branch="chore/coreutils-args-drift" - title="chore: sync uutils/coreutils argument surfaces and vendored modules" - body_file="$(mktemp)" git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" @@ -299,6 +336,28 @@ jobs: # whenever the branch already exists from a prior week's run. git push --force --set-upstream origin "$branch" + - name: Open or update pull request + working-directory: bashkit + env: + GH_TOKEN: ${{ steps.pat.outputs.token }} + HAVE_PAT: ${{ steps.pat.outputs.token != '' }} + NEW_REV: ${{ needs.regenerate.outputs.rev }} + OLD_REV: ${{ needs.regenerate.outputs.old_rev }} + UTILS: ${{ needs.regenerate.outputs.utils }} + MODULES: ${{ needs.regenerate.outputs.modules }} + run: | + set -euo pipefail + branch="chore/coreutils-args-drift" + title="chore: sync uutils/coreutils argument surfaces and vendored modules" + body_file="$(mktemp)" + + # The drift is pushed by the time this runs, so say so: the work is + # recoverable by hand and only the PR step is blocked. + if [ "$HAVE_PAT" != "true" ]; then + echo "::error::Drift was regenerated and pushed to '$branch', but no pull request could be opened. The Actions job token is not permitted to create pull requests, and no Doppler-provided GITHUB_TOKEN was available. Add a GITHUB_TOKEN secret in Doppler with 'pull-requests: write' on this repository, or open a PR from '$branch' by hand." + exit 1 + fi + cat > "$body_file" < --state open`). `gh pr view ` resolves a PR for the branch in any state, so once a drift PR is closed unmerged it keeps matching and every later run edits diff --git a/scripts/tests/test_coreutils_drift_pr.py b/scripts/tests/test_coreutils_drift_pr.py index be92d3d01..11ba18654 100644 --- a/scripts/tests/test_coreutils_drift_pr.py +++ b/scripts/tests/test_coreutils_drift_pr.py @@ -5,6 +5,10 @@ run silently edited that closed PR instead of opening a new one and the drift stopped surfacing. These tests execute the workflow's own `run` script with stubbed `gh`/`git` to pin the open-PR-only lookup. + +They also pin the token split. The Actions job token cannot create pull +requests, so the PR step runs on a Doppler-resolved PAT; with no PAT it must +fail loudly rather than skip the PR and leave the drift invisible. """ import os from pathlib import Path @@ -30,10 +34,11 @@ def setUp(self): step for job in jobs.values() for step in job.get('steps', []) - if step.get('name') == 'Commit and open PR' + if step.get('name') == 'Open or update pull request' ) - def _run(self, *, open_pr_exists: bool): + def _run(self, *, open_pr_exists: bool, have_pat: bool = True, + expect_success: bool = True): """Run the step with a closed PR present and `open_pr_exists` open ones.""" with tempfile.TemporaryDirectory() as directory: root = Path(directory) @@ -62,7 +67,8 @@ def _run(self, *, open_pr_exists: bool): 'PATH': f"{bin_dir}:{os.environ['PATH']}", 'HOME': str(root), 'GITHUB_REPOSITORY': 'everruns/bashkit', - 'GH_TOKEN': 'stub', + 'GH_TOKEN': 'stub' if have_pat else '', + 'HAVE_PAT': 'true' if have_pat else 'false', 'RUNNER_TEMP': str(root), 'NEW_REV': 'abc1234', 'OLD_REV': 'def5678', @@ -73,22 +79,57 @@ def _run(self, *, open_pr_exists: bool): ['/bin/bash', '-c', self.step['run']], env=env, capture_output=True, text=True, cwd=root, ) - self.assertEqual(result.returncode, 0, result.stderr) - return log.read_text() if log.exists() else '' + if expect_success: + self.assertEqual(result.returncode, 0, result.stderr) + else: + self.assertNotEqual(result.returncode, 0, result.stdout) + calls = log.read_text() if log.exists() else '' + return calls, result def test_opens_a_new_pr_when_only_a_closed_one_matches(self): - calls = self._run(open_pr_exists=False) + calls, _ = self._run(open_pr_exists=False) self.assertIn('pr create', calls, calls) self.assertNotIn('pr edit', calls, calls) def test_updates_the_existing_pr_when_one_is_open(self): - calls = self._run(open_pr_exists=True) + calls, _ = self._run(open_pr_exists=True) self.assertIn('pr edit', calls, calls) self.assertNotIn('pr create', calls, calls) def test_open_state_is_scoped_in_the_lookup(self): self.assertIn('--state open', self.step['run']) + def test_missing_pat_fails_loudly_without_touching_the_pr(self): + """No PAT must be a red run, not a silently skipped PR. + + The job token can push but cannot create a PR. If this step merely + skipped the PR, the drift would sit on a force-pushed branch that + nobody looks at and the run would go green. + """ + calls, result = self._run( + open_pr_exists=False, have_pat=False, expect_success=False, + ) + self.assertNotIn('pr create', calls, calls) + self.assertNotIn('pr edit', calls, calls) + self.assertIn('::error::', result.stdout, result.stdout) + # The message has to name the recovery path: the drift is already + # pushed, so the branch is where a human picks it up. + self.assertIn('chore/coreutils-args-drift', result.stdout) + + def test_push_and_pr_use_separate_tokens(self): + """The push runs on the job token; only the PR gets the PAT.""" + jobs = yaml.safe_load(WORKFLOW.read_text())['jobs'] + steps = { + step.get('name'): step + for job in jobs.values() + for step in job.get('steps', []) + } + push = steps['Commit and push drift branch'] + self.assertEqual(push['env']['GH_TOKEN'], '${{ github.token }}') + self.assertIn('git push', push['run']) + self.assertIn('steps.pat.outputs.token', self.step['env']['GH_TOKEN']) + self.assertNotIn('git push', self.step['run']) + if __name__ == '__main__': unittest.main()