From e5274a3b9f4d4df011ad443bd8ef22f736c039a2 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 20 Oct 2026 09:28:46 +0000 Subject: [PATCH 1/2] fix(ci): open coreutils drift PRs with a token permitted to create them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The weekly Coreutils Args Drift run regenerated the drift, force-pushed it to `chore/coreutils-args-drift`, then died on `gh pr create` with "GitHub Actions is not permitted to create or approve pull requests" (run 39234801977). The run went red after all regeneration and tests were green, and no PR ever opened, so the drift stayed invisible — the exact silent-absorption mode the open-PR-only lookup was added to prevent. The job token can push (it only needs `contents: write`) but cannot open a pull request while "Allow GitHub Actions to create and approve pull requests" is off, which is the setting worth keeping since the same switch also governs approvals. So the step is split in two: the branch goes up on `github.token`, and the PR is opened with a PAT read from Doppler's `GITHUB_TOKEN`, gated on `DOPPLER_TOKEN` and masked before it crosses a step boundary — the pattern `ci.yml` already uses for privileged CI credentials. With no PAT the PR step fails naming the branch instead of skipping the PR: the drift is pushed by then, so a green run without a PR would reintroduce silent absorption. Regression tests pin both halves: a missing PAT is a red run that touches no PR, and the push keeps the job token while only the PR step gets the PAT. --- .github/workflows/coreutils-args-drift.yml | 67 ++++++++++++++++++++-- knowledge/runtimes/coreutils-args-port.md | 18 ++++++ scripts/tests/test_coreutils_drift_pr.py | 55 +++++++++++++++--- 3 files changed, 129 insertions(+), 11 deletions(-) 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() From e8c047409be208bfdd584eb0abe31e1c5f8c35eb Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 20 Oct 2026 09:42:22 +0000 Subject: [PATCH 2/2] chore(knowledge): record the drift PAT's job boundary Introducing a credential into this workflow makes the job split a security contract, not just a layout: `regenerate` executes third-party uutils code and must stay secret-free, `open-pr` holds the PAT and must never build or run uutils. Write it down next to the token-split rationale so a later change does not move a Doppler step into the wrong job. --- knowledge/runtimes/coreutils-args-port.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/knowledge/runtimes/coreutils-args-port.md b/knowledge/runtimes/coreutils-args-port.md index 9eb6d736d..53f821fac 100644 --- a/knowledge/runtimes/coreutils-args-port.md +++ b/knowledge/runtimes/coreutils-args-port.md @@ -465,6 +465,14 @@ the PR: the drift is already pushed at that point, so a green run with no PR is exactly the silent-absorption failure mode this design exists to prevent. Regression test: `scripts/tests/test_coreutils_drift_pr.py`. +The PAT must stay confined to the PR job. `regenerate` is the job that builds +and executes third-party uutils code, and it has no Doppler step and no secret +access; `open-pr` holds the PAT but never checks out, builds or runs uutils. +That split is what keeps a compromised upstream from reaching a credential, so +a Doppler step must never be added to `regenerate`. The workflow is +`schedule` + `workflow_dispatch` only for the same reason — no pull-request +trigger can reach the secret. + The PR lookup matches **open** PRs only (`gh pr list --head --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