Skip to content

Commit ecd01a0

Browse files
fix(ci): stop checking out event-controlled refs in privileged workflows (#1738)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 050f391 commit ecd01a0

3 files changed

Lines changed: 27 additions & 6 deletions

File tree

.github/workflows/openapi-autocommit.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: API
22

33
on:
4-
pull_request_target: # zizmor: ignore[dangerous-triggers] PR code runs only in the read-only generate job.
4+
pull_request:
55
branches: [main]
66
types: [labeled]
77

@@ -14,6 +14,7 @@ concurrency:
1414
jobs:
1515
generate:
1616
name: Generate spec and client
17+
# A fork's token is read-only, so nothing could be pushed back; skip the whole run.
1718
if: >-
1819
${{ github.event.label.name == 'autocommit-openapi'
1920
&& github.event.pull_request.head.repo.full_name == github.repository }}
@@ -36,7 +37,6 @@ jobs:
3637
with:
3738
distribution: temurin
3839
java-version: 21
39-
# This executes PR-controlled Maven plugins, so this job must remain read-only and secretless.
4040
- name: Generate OpenAPI spec and client
4141
run: pnpm run generate:api
4242
- name: Upload generated files
@@ -59,7 +59,7 @@ jobs:
5959
outputs:
6060
changed: ${{ steps.commit.outputs.changed }}
6161
steps:
62-
# Do not execute files from the checkout or artifact in this job.
62+
# Holds the write token: nothing from the checkout or artifact is executed here.
6363
- name: Checkout PR commit
6464
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
6565
with:

.github/workflows/release.yml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,9 @@ jobs:
4343
steps:
4444
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
4545
with:
46-
# Pin to the exact commit CI/CD built — never main's tip, which may
47-
# already carry a newer version bump (that would skip this release).
48-
ref: ${{ github.event.workflow_run.head_sha }}
46+
ref: ${{ github.sha }}
4947
fetch-depth: 0
48+
persist-credentials: false
5049

5150
# Decides whether this commit cuts a release, and creates nothing.
5251
# The draft is created in tag-images, after the evidence gate, so a gate failure leaves no
@@ -62,6 +61,10 @@ jobs:
6261
run: |
6362
set -euo pipefail
6463
64+
# Read the commit CI/CD built, not main's tip, which may already carry a newer version bump.
65+
git merge-base --is-ancestor "$SHA" origin/main || {
66+
echo "::error::Commit $SHA is not on main"; exit 1; }
67+
6568
VERSION=$(git show "$SHA:package.json" | jq -r .version)
6669
PARENT_VERSION=$(git show "${SHA}^:package.json" | jq -r .version)
6770
TAG="v$VERSION"
@@ -141,6 +144,7 @@ jobs:
141144
with:
142145
ref: ${{ needs.release.outputs.sha }}
143146
fetch-depth: 1
147+
persist-credentials: false
144148

145149
- name: Log in to GitHub Container Registry
146150
uses: ./.github/actions/ghcr-login

scripts/ci-contract.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,23 @@ void describe("CI contract", () => {
562562
assert.doesNotMatch(source, /pull_request\.title|github\.head_ref/);
563563
});
564564

565+
void test("checks out no ref taken straight from a pull_request_target or workflow_run event", async () => {
566+
// Scorecard's Dangerous-Workflow rule, checked here because Scorecard itself runs only after merge.
567+
for (const [file, source] of await workflowSources()) {
568+
if (!/^ {2}(?:pull_request_target|workflow_run):/m.test(source)) continue;
569+
assert.doesNotMatch(
570+
source,
571+
/^ +ref:.*github\.event\.(?:pull_request|workflow_run)/m,
572+
`${file} checks out a ref taken from the triggering event`,
573+
);
574+
}
575+
// The release jobs check out the run's commit through an output; this is what makes it safe.
576+
assert.match(
577+
job(await readFile(".github/workflows/release.yml", "utf8"), "release"),
578+
/git merge-base --is-ancestor "\$SHA" origin\/main/,
579+
);
580+
});
581+
565582
void test("never invokes a repository-local action before checkout", async () => {
566583
for (const [file, source] of await workflowSources()) {
567584
for (const jobSource of source.split(/^ {2}(?=[A-Za-z][\w-]*:\s*$)/m).slice(1)) {

0 commit comments

Comments
 (0)