Skip to content

Commit 7a51b96

Browse files
committed
Skip automatic docs previews for fork PRs and drop the setup-uv retry steps
actions/checkout v7 refuses to fetch a fork's head in a pull_request_target run unless the step opts in with allow-unsafe-pr-checkout. Rather than opt in, have `authorize` skip fork heads on that path so the run ends cleanly instead of failing at checkout and posting a "Preview build failed" comment. Maintainers request a fork preview with /preview-docs, which runs under issue_comment and is unaffected; same-repo branches keep their automatic previews. Covered by new scenarios in docs_preview.test.js. setup-uv v10.0.1 retries the version-manifest fetch itself, which is what the continue-on-error + retry step pairs in shared.yml were waiting for, so collapse them to a single step. Also refresh docs-preview.yml comments the actions bump made stale (default-branch context, read-only cache scope for these triggers) and note that the two script checkouts must stay ref-less.
1 parent a2dab7e commit 7a51b96

4 files changed

Lines changed: 74 additions & 73 deletions

File tree

.github/scripts/docs_preview.js

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,24 @@ async function authorize({ github, context, core }) {
2323
let slashAttempt = false;
2424

2525
if (context.eventName === 'pull_request_target') {
26-
// Gate on the *sender* (whoever caused this run — on synchronize that
27-
// is the pusher), not the PR author, so a non-admin pushing to an
28-
// admin-opened branch does not get an automatic build.
29-
const actor = context.payload.sender.login;
30-
prNumber = String(context.payload.pull_request.number);
31-
headSha = context.payload.pull_request.head.sha;
32-
const perm = await permissionFor(actor);
33-
authorized = perm.level === 'admin';
34-
core.info(`pull_request_target by ${actor} (level=${perm.level}, role=${perm.role}) → authorized=${authorized}`);
26+
const pr = context.payload.pull_request;
27+
prNumber = String(pr.number);
28+
headSha = pr.head.sha;
29+
// No automatic preview for fork PRs: actions/checkout refuses to fetch a
30+
// fork's head in a pull_request_target run. A maintainer can still request
31+
// one with /preview-docs. (head.repo is null once the fork is deleted.)
32+
const headRepo = pr.head.repo;
33+
if (!headRepo || headRepo.id !== context.payload.repository.id) {
34+
core.info(`PR #${prNumber} head is on ${headRepo ? headRepo.full_name : 'a deleted fork'}; fork PRs are previewed via /preview-docs only.`);
35+
} else {
36+
// Gate on the *sender* (whoever caused this run — on synchronize that
37+
// is the pusher), not the PR author, so a non-admin pushing to an
38+
// admin-opened branch does not get an automatic build.
39+
const actor = context.payload.sender.login;
40+
const perm = await permissionFor(actor);
41+
authorized = perm.level === 'admin';
42+
core.info(`pull_request_target by ${actor} (level=${perm.level}, role=${perm.role}) → authorized=${authorized}`);
43+
}
3544
} else {
3645
// issue_comment: the job-level `if:` already guarantees this is a PR
3746
// comment starting with /preview-docs.

.github/scripts/docs_preview.test.js

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const assert = require('node:assert/strict');
1212
const { authorize, comment } = require('./docs_preview.js');
1313

1414
const REPO = { owner: 'modelcontextprotocol', repo: 'python-sdk' };
15+
const BASE_REPO = { id: 1, full_name: 'modelcontextprotocol/python-sdk' };
1516
const HEAD = 'e4dfda7baa127ab00ebcd1d5324560cbe3cdfe42';
1617
const MARKER = '<!-- docs-preview -->';
1718

@@ -37,6 +38,26 @@ const authorizeScenarios = [
3738
event: pushed(7, 'writer'),
3839
expect: { authorized: 'false', pr_number: '7', head_sha: HEAD, slash_attempt: 'false' },
3940
},
41+
{
42+
// actions/checkout refuses a fork's head under pull_request_target, so
43+
// the run stops here instead of failing in `build`; /preview-docs still works.
44+
name: 'admin pushes to or reopens a fork PR → no automatic preview',
45+
event: pushed(7, 'admin', { fork: 'someone/python-sdk' }),
46+
expect: { authorized: 'false', pr_number: '7', head_sha: HEAD, slash_attempt: 'false' },
47+
permissionLookups: 0,
48+
},
49+
{
50+
name: 'fork PR whose fork has since been deleted → no automatic preview',
51+
event: pushed(7, 'admin', { fork: null }),
52+
expect: { authorized: 'false', pr_number: '7', head_sha: HEAD, slash_attempt: 'false' },
53+
permissionLookups: 0,
54+
},
55+
{
56+
name: 'maintainer comments /preview-docs on a fork PR → previewed like any other',
57+
pr: { fork: 'someone/python-sdk' },
58+
event: slash(7, 'maintainer'),
59+
expect: { authorized: 'true', pr_number: '7', head_sha: HEAD, slash_attempt: 'true' },
60+
},
4061
{
4162
name: 'maintainer comments /preview-docs on an open PR → preview of its current head',
4263
event: slash(7, 'maintainer'),
@@ -70,6 +91,7 @@ for (const s of authorizeScenarios) {
7091
const world = makeWorld({ pr: { number: 7, ...s.pr } });
7192
assert.deepEqual(await runAuthorize(world, s.event), s.expect);
7293
assert.equal(world.writes.length, 0);
94+
if (s.permissionLookups !== undefined) assert.equal(world.permissionLookups, s.permissionLookups);
7395
});
7496
}
7597

@@ -146,8 +168,10 @@ test('comment: a build or deploy that did not succeed is reported with the short
146168

147169
// ── Harness ────────────────────────────────────────────────────────────────
148170

149-
function pushed(number, sender) {
150-
return { eventName: 'pull_request_target', actor: sender, payload: { action: 'synchronize', pull_request: { number, head: { sha: HEAD } }, sender: { login: sender } } };
171+
// `fork`: full name of the fork the head lives on; null for a deleted fork; omitted for a same-repo branch.
172+
function pushed(number, sender, { fork } = {}) {
173+
const repo = fork === undefined ? BASE_REPO : fork === null ? null : { id: 2, full_name: fork };
174+
return { eventName: 'pull_request_target', actor: sender, payload: { action: 'synchronize', repository: BASE_REPO, pull_request: { number, head: { sha: HEAD, repo } }, sender: { login: sender } } };
151175
}
152176
function slash(number, commenter) {
153177
return { eventName: 'issue_comment', actor: commenter, payload: { action: 'created', issue: { number, pull_request: {} }, comment: { body: '/preview-docs', user: { login: commenter } } } };
@@ -173,7 +197,7 @@ async function runComment(world, env, actor) {
173197
// ── A tiny in-memory GitHub ────────────────────────────────────────────────
174198

175199
function makeWorld({ pr, comments = [] }) {
176-
const world = { pr: { state: 'open', ...pr }, comments: [], writes: [], failPermissionLookup: false, nextCommentId: 100 };
200+
const world = { pr: { state: 'open', ...pr }, comments: [], writes: [], failPermissionLookup: false, permissionLookups: 0, nextCommentId: 100 };
177201
for (const c of comments) world.comments.push({ id: world.nextCommentId++, ...c });
178202

179203
const err = (status, message = 'fake error') => Object.assign(new Error(message), { status });
@@ -183,14 +207,19 @@ function makeWorld({ pr, comments = [] }) {
183207
const rest = {
184208
repos: {
185209
getCollaboratorPermissionLevel: async ({ username }) => {
210+
world.permissionLookups++;
186211
if (world.failPermissionLookup) throw err(500, 'boom');
187212
const person = PEOPLE[username];
188213
if (!person) throw err(404, 'not a user');
189214
return { data: { ...person, user: { login: username } } };
190215
},
191216
},
192217
pulls: {
193-
get: async ({ pull_number }) => { checkPr(pull_number); return { data: { number: pull_number, state: world.pr.state, head: { sha: HEAD } } }; },
218+
get: async ({ pull_number }) => {
219+
checkPr(pull_number);
220+
const repo = world.pr.fork ? { id: 2, full_name: world.pr.fork } : BASE_REPO;
221+
return { data: { number: pull_number, state: world.pr.state, head: { sha: HEAD, repo } } };
222+
},
194223
},
195224
issues: {
196225
listComments: async ({ issue_number }) => { checkPr(issue_number); return { data: world.comments.map((c) => ({ id: c.id, body: c.body, user: { login: c.user } })) }; },

.github/workflows/docs-preview.yml

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,16 @@ name: Docs Preview
44
#
55
# Security: the build executes Python from the PR (mkdocstrings imports
66
# src/mcp, `!!python/name:` config directives run, and heads may ship their
7-
# own build scripts). The build is gated by `authorize` (admin sender for
8-
# auto-preview, admin/maintainer commenter for /preview-docs) and isolated
9-
# from Cloudflare secrets — `build` runs PR code with no secrets and hands
10-
# the static site to `deploy` via an artifact, so PR code never shares a
11-
# runner with the Cloudflare token. `authorize` and `comment` run
12-
# .github/scripts/docs_preview.js, checked out from the default branch only.
7+
# own build scripts). The build is gated by `authorize` (admin sender on a
8+
# same-repo branch for auto-preview, admin/maintainer commenter for
9+
# /preview-docs) and isolated from Cloudflare secrets — `build` runs PR code
10+
# with no secrets and hands the static site to `deploy` via an artifact, so
11+
# PR code never shares a runner with the Cloudflare token. Fork PRs get no
12+
# automatic preview: actions/checkout refuses to fetch a fork's head in a
13+
# pull_request_target run, so a maintainer requests one with /preview-docs,
14+
# which runs under issue_comment with the same gating and isolation.
15+
# `authorize` and `comment` run .github/scripts/docs_preview.js, checked out
16+
# from the default branch only; those two checkouts must never take a `ref:`.
1317
#
1418
# Required configuration:
1519
# - secrets.CLOUDFLARE_API_TOKEN (scope: Account → Cloudflare Pages → Edit)
@@ -62,6 +66,7 @@ jobs:
6266
- name: Check out the scripts (default branch)
6367
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
6468
with:
69+
# No `ref:` here, ever: this job trusts what it checks out (see header).
6570
persist-credentials: false
6671
sparse-checkout: .github/scripts
6772

@@ -88,19 +93,20 @@ jobs:
8893
- name: Install uv
8994
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
9095
with:
91-
# pull_request_target runs share the base-branch Actions cache; saving
92-
# a cache populated while untrusted PR code ran would let it poison
93-
# later trusted workflows. Mirrors publish-pypi.yml.
96+
# Keep the untrusted build away from the shared Actions cache. GitHub
97+
# already limits pull_request_target and issue_comment runs to
98+
# read-only access in the default branch's cache scope, so this is
99+
# defence in depth (and avoids a refused save in the post step).
100+
# Mirrors publish-pypi.yml.
94101
enable-cache: false
95102
version: 0.9.5
96103

97-
# pull_request_target runs this workflow file from the base branch, so
98-
# the whole recipe — dependency sync included — must come from the
99-
# checkout itself: heads that ship scripts/docs/build.sh (the Zensical
100-
# toolchain) build with it; older heads, and v1.x heads previewed via
101-
# /preview-docs, still build with MkDocs. Both arms must write the site
102-
# to site/. Keep the detection in sync with build_site() in
103-
# scripts/build-docs.sh.
104+
# Both triggers run this workflow file from the default branch (whatever
105+
# the PR targets), so the whole recipe — dependency sync included — must
106+
# come from the checkout itself: heads that ship scripts/docs/build.sh
107+
# (the Zensical toolchain) build with it; older heads and v1.x heads still
108+
# build with MkDocs. Both arms must write the site to site/. Keep the
109+
# detection in sync with build_site() in scripts/build-docs.sh.
104110
- run: |
105111
if [ -f scripts/docs/build.sh ]; then
106112
bash scripts/docs/build.sh
@@ -161,6 +167,7 @@ jobs:
161167
- name: Check out the scripts (default branch)
162168
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
163169
with:
170+
# No `ref:` here, ever: this job trusts what it checks out (see header).
164171
persist-credentials: false
165172
sparse-checkout: .github/scripts
166173

.github/workflows/shared.yml

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,7 @@ jobs:
1717
with:
1818
persist-credentials: false
1919

20-
# setup-uv's manifest fetch is a single request with a hard 5s timeout
21-
# (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
2220
- name: Install uv
23-
id: setup-uv
24-
continue-on-error: true
25-
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
26-
with:
27-
enable-cache: true
28-
version: 0.9.5
29-
30-
- name: Install uv (retry)
31-
if: steps.setup-uv.outcome == 'failure'
3221
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
3322
with:
3423
enable-cache: true
@@ -79,18 +68,7 @@ jobs:
7968
with:
8069
persist-credentials: false
8170

82-
# setup-uv's manifest fetch is a single request with a hard 5s timeout
83-
# (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
8471
- name: Install uv
85-
id: setup-uv
86-
continue-on-error: true
87-
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
88-
with:
89-
enable-cache: true
90-
version: 0.9.5
91-
92-
- name: Install uv (retry)
93-
if: steps.setup-uv.outcome == 'failure'
9472
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
9573
with:
9674
enable-cache: true
@@ -125,18 +103,7 @@ jobs:
125103
with:
126104
persist-credentials: false
127105

128-
# setup-uv's manifest fetch is a single request with a hard 5s timeout
129-
# (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
130106
- name: Install uv
131-
id: setup-uv
132-
continue-on-error: true
133-
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
134-
with:
135-
enable-cache: true
136-
version: 0.9.5
137-
138-
- name: Install uv (retry)
139-
if: steps.setup-uv.outcome == 'failure'
140107
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
141108
with:
142109
enable-cache: true
@@ -165,18 +132,7 @@ jobs:
165132
with:
166133
persist-credentials: false
167134

168-
# setup-uv's manifest fetch is a single request with a hard 5s timeout
169-
# (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
170135
- name: Install uv
171-
id: setup-uv
172-
continue-on-error: true
173-
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
174-
with:
175-
enable-cache: true
176-
version: 0.9.5
177-
178-
- name: Install uv (retry)
179-
if: steps.setup-uv.outcome == 'failure'
180136
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
181137
with:
182138
enable-cache: true

0 commit comments

Comments
 (0)