Skip to content

Commit 0f6e99e

Browse files
myasnikovdaniilclaude
myasnikovdaniil
and
claude
committed
fix(ci): publish the "E2E Tests" gate as a commit status, not a check-run
Route the required "E2E Tests" merge gate through a GitHub commit status posted with the default GITHUB_TOKEN (+ statuses:write), replacing the COZYSTACK_CI app check-run. The app path needed the app granted checks:write (it isn't — checks.create 403s "Resource not accessible by integration"), and the earlier default-token check-run floated under a random check-suite ("PR size label"). A commit status has neither problem: branch protection matches it by context name, it carries no check-suite grouping, and it is safe with the default token because a fork's pull_request token is read-only and cannot post one — only same-repo runs (e2e-report, guarded !fork) and the privileged workflow_run in e2e-fork.yaml can set it. The security property is unchanged: a never-posted required status stays "Expected" and blocks the merge, so a skipped job can't smuggle a green (#3257). Removes all COZYSTACK_CI app-token machinery from e2e-fork.yaml — `resolve` posts the status directly, no `check_run_id` threading, no `open` step. Branch protection must require the "E2E Tests" status context (no GitHub App and no app-id pin needed). Assisted-By: Claude <[email protected]> Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 03b519d commit 0f6e99e

3 files changed

Lines changed: 92 additions & 146 deletions

File tree

‎.github/workflows/e2e-fork.yaml‎

Lines changed: 46 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ permissions:
4141

4242
jobs:
4343
# Decide whether this is a fork PR that needs e2e, resolve its PR number and
44-
# head SHA, and open the pending "E2E Tests" check-run so the PR is not
45-
# mergeable until we conclude it. Every decision is derived from data the fork
44+
# head SHA, and post the pending "E2E Tests" commit status so the PR is not
45+
# mergeable until `report` concludes it. Every decision is derived from data the fork
4646
# cannot forge — the workflow_run conclusion, the base-repo PR list, and the
4747
# base-repo PR file list — NOT from the presence/absence of an artifact the
4848
# fork's own build controls. Fail-closed if the fork build failed or the
@@ -60,23 +60,24 @@ jobs:
6060
contents: read
6161
actions: read
6262
pull-requests: read
63+
statuses: write
6364
outputs:
6465
should_run: ${{ steps.r.outputs.should_run }}
6566
pr_number: ${{ steps.r.outputs.pr_number }}
6667
head_sha: ${{ steps.r.outputs.head_sha }}
6768
run_id: ${{ steps.r.outputs.run_id }}
68-
check_run_id: ${{ steps.open.outputs.check_run_id }}
6969
# TIA inputs derived from TRUSTED base-repo data (the PR file list and
7070
# labels), NOT the fork-controlled merge-ref diff — so suite selection
7171
# keeps the same anti-forgery property as the docs-only decision (#3257).
7272
changed_b64: ${{ steps.r.outputs.changed_b64 }}
7373
full_e2e: ${{ steps.r.outputs.full_e2e }}
7474
steps:
75-
# Decide the verdict using this step's default (workflow_run) token, which
76-
# already has the actions:read / pull-requests:read scopes these lookups
77-
# need. It creates NO check-run — the next step publishes the verdict as
78-
# the COZYSTACK_CI app so the app token stays scoped to checks:write only.
79-
- name: Resolve fork PR and decide verdict
75+
# Decide the verdict and post the "E2E Tests" commit status directly, with
76+
# this job's default (workflow_run) token — it already has the actions:read
77+
# / pull-requests:read scopes the lookups need, plus statuses:write from the
78+
# job permissions. A commit status (not a check-run) is the merge gate; see
79+
# pull-requests.yaml's e2e-report and docs/agents/e2e-testing.md §10.
80+
- name: Resolve fork PR and post "E2E Tests" status
8081
id: r
8182
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
8283
env:
@@ -87,26 +88,29 @@ jobs:
8788
script: |
8889
const { HEAD_SHA, RUN_ID, CONCLUSION } = process.env;
8990
const { owner, repo } = context.repo;
90-
// Validate the SHA before it flows into a check-run / image tag.
91+
// Validate the SHA before it flows into a status target / image tag.
9192
if (!/^[0-9a-f]{40}$/.test(HEAD_SHA)) {
9293
core.setFailed(`unexpected head_sha '${HEAD_SHA}'`);
9394
return;
9495
}
9596
core.setOutput('head_sha', HEAD_SHA);
9697
core.setOutput('run_id', String(RUN_ID));
9798
98-
// Record a terminal verdict for the `open` step to publish as a
99-
// completed check-run. This step never creates a check itself.
100-
const decide = (conclusion, summary) => {
101-
core.setOutput('verdict_status', 'completed');
102-
core.setOutput('verdict_conclusion', conclusion);
103-
core.setOutput('verdict_summary', summary);
99+
const target_url = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`;
100+
const setStatus = (state, description) => github.rest.repos.createCommitStatus({
101+
owner, repo, sha: HEAD_SHA, state, context: 'E2E Tests', description, target_url,
102+
});
103+
// Post a TERMINAL status (docs-only success / fail-closed failure) and
104+
// stop — no privileged run needed. `report` never runs for these
105+
// (guarded on should_run).
106+
const decide = async (state, description) => {
107+
await setStatus(state, description);
104108
core.setOutput('should_run', 'false');
105109
};
106110
107-
// Fork build did not succeed → fail the required check (fail-closed).
111+
// Fork build did not succeed → fail the required status (fail-closed).
108112
if (CONCLUSION !== 'success') {
109-
decide('failure', `Pull Request workflow concluded '${CONCLUSION}'; e2e not run`);
113+
await decide('failure', `Pull Request workflow concluded '${CONCLUSION}'; e2e not run`);
110114
return;
111115
}
112116
@@ -121,7 +125,7 @@ jobs:
121125
const pr = assoc.data.find(p => p.state === 'open');
122126
const prNumber = pr ? String(pr.number) : '';
123127
if (!/^[0-9]+$/.test(prNumber)) {
124-
decide('failure', `no open PR for ${HEAD_SHA}`);
128+
await decide('failure', `no open PR for ${HEAD_SHA}`);
125129
return;
126130
}
127131
core.setOutput('pr_number', prNumber);
@@ -136,7 +140,7 @@ jobs:
136140
});
137141
const hasNonDocs = files.some(f => !f.filename.startsWith('docs/'));
138142
if (!hasNonDocs) {
139-
decide('success', 'docs-only PR (verified via PR file list) — e2e not required');
143+
await decide('success', 'docs-only PR (verified via PR file list) — e2e not required');
140144
return;
141145
}
142146
@@ -148,7 +152,7 @@ jobs:
148152
{ owner, repo, run_id: RUN_ID, per_page: 100 },
149153
);
150154
if (!arts.some(a => a.name === 'pr-patch')) {
151-
decide('failure', 'expected pr-patch artifact is missing — the fork build did not complete');
155+
await decide('failure', 'expected pr-patch artifact is missing — the fork build did not complete');
152156
return;
153157
}
154158
@@ -165,51 +169,11 @@ jobs:
165169
const fullE2e = (pr.labels || []).some(l => l.name === 'full-e2e');
166170
core.setOutput('full_e2e', fullE2e ? 'true' : 'false');
167171
168-
// Real run: the `open` step opens an in-progress check-run; `report`
169-
// concludes it.
170-
core.setOutput('verdict_status', 'in_progress');
171-
core.setOutput('verdict_summary', 'Running e2e in the privileged fork workflow…');
172+
// Real run: post a pending status now; `report` flips it to the
173+
// terminal state after publish + e2e.
174+
await setStatus('pending', 'Running e2e in the privileged fork workflow…');
172175
core.setOutput('should_run', 'true');
173176
174-
# Skipped when `resolve` set-failed on a bad SHA (fail-closed: no check →
175-
# "E2E Tests" stays unreported → PR not mergeable).
176-
- name: Generate GitHub App token
177-
id: app-token
178-
uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1
179-
with:
180-
app-id: ${{ secrets.COZYSTACK_CI_APP_ID }}
181-
private-key: ${{ secrets.COZYSTACK_CI_PRIVATE_KEY }}
182-
owner: cozystack
183-
repositories: ${{ github.event.repository.name }}
184-
permission-checks: write
185-
# Publish the verdict as the COZYSTACK_CI app so the check-run lands in the
186-
# app's own check-suite — matching the same-repo e2e-report path and the
187-
# branch-protection pin (see pull-requests.yaml e2e-report comment and
188-
# docs/agents/e2e-testing.md §10). Creating it with the default token would
189-
# file it under an arbitrary GitHub Actions suite (a labeler).
190-
- name: Open "E2E Tests" check-run (as app)
191-
id: open
192-
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
193-
env:
194-
STATUS: ${{ steps.r.outputs.verdict_status }}
195-
CONCLUSION: ${{ steps.r.outputs.verdict_conclusion }}
196-
SUMMARY: ${{ steps.r.outputs.verdict_summary }}
197-
HEAD_SHA: ${{ steps.r.outputs.head_sha }}
198-
with:
199-
github-token: ${{ steps.app-token.outputs.token }}
200-
script: |
201-
const { STATUS, CONCLUSION, SUMMARY, HEAD_SHA } = process.env;
202-
const { owner, repo } = context.repo;
203-
const params = {
204-
owner, repo, name: 'E2E Tests', head_sha: HEAD_SHA,
205-
status: STATUS,
206-
output: { title: 'E2E Tests', summary: SUMMARY },
207-
};
208-
if (STATUS === 'completed') params.conclusion = CONCLUSION;
209-
const cr = await github.rest.checks.create(params);
210-
// Only the real (in-progress) run is handed to `report` to conclude.
211-
if (STATUS === 'in_progress') core.setOutput('check_run_id', String(cr.data.id));
212-
213177
# Push the fork-built image archives to OCIR by digest and flux-push the
214178
# digest-pinned packages tree. Only trusted binaries touch the fork's content
215179
# while the OCIR token is live — no `make`/fork script runs here.
@@ -549,62 +513,42 @@ jobs:
549513
if: always()
550514
run: rm -rf "/tmp/$SANDBOX_NAME"
551515

552-
# Conclude the required "E2E Tests" check-run opened by `resolve`. Always runs
553-
# (so a failed publish/e2e still turns the check red rather than leaving it
554-
# stuck in-progress). success only when the e2e job passed.
516+
# Flip the pending "E2E Tests" commit status posted by `resolve` to its
517+
# terminal state. Always runs (so a failed publish/e2e turns the status red
518+
# rather than leaving it stuck pending). success only when the e2e job passed.
555519
report:
556520
name: Report fork E2E status
557521
needs: [resolve, publish, e2e]
558522
if: ${{ always() && needs.resolve.outputs.should_run == 'true' }}
559523
runs-on: ubuntu-latest
560524
permissions:
561525
contents: read
526+
statuses: write
562527
steps:
563-
# The pending check-run was opened by the COZYSTACK_CI app in `resolve`;
564-
# checks.update must run as the same app, so mint its token here too.
565-
- name: Generate GitHub App token
566-
id: app-token
567-
uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1
568-
with:
569-
app-id: ${{ secrets.COZYSTACK_CI_APP_ID }}
570-
private-key: ${{ secrets.COZYSTACK_CI_PRIVATE_KEY }}
571-
owner: cozystack
572-
repositories: ${{ github.event.repository.name }}
573-
permission-checks: write
574-
- name: Conclude "E2E Tests" check-run
528+
- name: Report "E2E Tests" commit status
575529
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
576530
env:
577-
CHECK_RUN_ID: ${{ needs.resolve.outputs.check_run_id }}
578531
PUBLISH_RESULT: ${{ needs.publish.result }}
579532
E2E_RESULT: ${{ needs.e2e.result }}
580533
HEAD_SHA: ${{ needs.resolve.outputs.head_sha }}
534+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
581535
with:
582-
github-token: ${{ steps.app-token.outputs.token }}
583536
script: |
584-
const { CHECK_RUN_ID, PUBLISH_RESULT, E2E_RESULT, HEAD_SHA } = process.env;
537+
const { PUBLISH_RESULT, E2E_RESULT, HEAD_SHA, RUN_URL } = process.env;
585538
const { owner, repo } = context.repo;
586-
let conclusion, summary;
539+
let state, description;
587540
if (E2E_RESULT === 'success') {
588-
conclusion = 'success';
589-
summary = 'fork e2e passed (privileged run)';
541+
state = 'success';
542+
description = 'fork e2e passed (privileged run)';
590543
} else if (PUBLISH_RESULT !== 'success') {
591-
conclusion = 'failure';
592-
summary = `image/artifact publish did not succeed (result: ${PUBLISH_RESULT})`;
544+
state = 'failure';
545+
description = `image/artifact publish did not succeed (result: ${PUBLISH_RESULT})`;
593546
} else {
594-
conclusion = 'failure';
595-
summary = `fork e2e did not pass (result: ${E2E_RESULT})`;
547+
state = 'failure';
548+
description = `fork e2e did not pass (result: ${E2E_RESULT})`;
596549
}
597-
if (CHECK_RUN_ID) {
598-
await github.rest.checks.update({
599-
owner, repo, check_run_id: Number(CHECK_RUN_ID),
600-
status: 'completed', conclusion,
601-
output: { title: 'E2E Tests', summary },
602-
});
603-
} else {
604-
await github.rest.checks.create({
605-
owner, repo, name: 'E2E Tests', head_sha: HEAD_SHA,
606-
status: 'completed', conclusion,
607-
output: { title: 'E2E Tests', summary },
608-
});
609-
}
610-
core.info(`E2E Tests → ${conclusion}: ${summary}`);
550+
await github.rest.repos.createCommitStatus({
551+
owner, repo, sha: HEAD_SHA, state, context: 'E2E Tests',
552+
description, target_url: RUN_URL,
553+
});
554+
core.info(`E2E Tests → ${state}: ${description}`);

‎.github/workflows/pull-requests.yaml‎

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -885,73 +885,73 @@ jobs:
885885
# Fork PRs skip this job; e2e-fork.yaml concludes "E2E Tests" for them, so a
886886
# fork PR stays "Expected"/not-mergeable until the privileged run reports.
887887
#
888-
# The check-run is published with the COZYSTACK_CI GitHub App token, NOT the
889-
# default GITHUB_TOKEN. A token-created check-run is owned by the app that made
890-
# it and GitHub files it under that app's check-suite; with the default token
891-
# (the "GitHub Actions" app) a floating check-run attaches to an arbitrary
892-
# GitHub Actions suite for the SHA — in practice the earliest one, a tiny
893-
# labeler workflow — so "E2E Tests" rendered under "PR size label". Publishing
894-
# as the app gives the check its own dedicated suite (shown under the app's
895-
# name) and is stricter: only the CI app can satisfy the gate, not any Actions
896-
# workflow. REQUIRES main branch protection to require "E2E Tests" from this
897-
# app's id (not app_id 15368) — see docs/agents/e2e-testing.md §10.
888+
# The required "E2E Tests" merge gate is published as a COMMIT STATUS with the
889+
# default GITHUB_TOKEN (+ statuses:write) — NOT as a job, and NOT as a
890+
# check-run.
891+
# - Not a job: a *skipped* required job counts as passed by branch
892+
# protection, which is how fork PRs used to merge with e2e never run
893+
# (#3257). A status is "success" only when this job posts it; a
894+
# never-posted required status stays "Expected" and blocks the merge.
895+
# - Not a check-run: a check-run made with the default token floats into an
896+
# arbitrary GitHub Actions check-suite (it rendered under "PR size label"),
897+
# and an app-token check-run needs the app granted checks:write. A commit
898+
# status has neither problem — branch protection matches it by context name
899+
# and it carries no check-suite grouping. Safe with the default token: a
900+
# fork's pull_request token is read-only and cannot post a status, so only
901+
# same-repo runs (this job, guarded !fork) and the privileged workflow_run
902+
# in e2e-fork.yaml can set it.
903+
# REQUIRES main branch protection to require the "E2E Tests" status context —
904+
# see docs/agents/e2e-testing.md §10.
898905
e2e-report:
899906
name: "Report E2E status"
900907
needs: ["plan", "e2e"]
901908
if: ${{ always() && !github.event.pull_request.head.repo.fork }}
902909
runs-on: ubuntu-latest
903910
permissions:
904911
contents: read
912+
statuses: write
905913
steps:
906-
- name: Generate GitHub App token
907-
id: app-token
908-
uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1
909-
with:
910-
app-id: ${{ secrets.COZYSTACK_CI_APP_ID }}
911-
private-key: ${{ secrets.COZYSTACK_CI_PRIVATE_KEY }}
912-
owner: cozystack
913-
repositories: ${{ github.event.repository.name }}
914-
permission-checks: write
915-
- name: Conclude "E2E Tests" check-run
914+
- name: Report "E2E Tests" commit status
916915
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
917916
env:
918917
PLAN_RESULT: ${{ needs.plan.result }}
919918
CODE: ${{ needs.plan.outputs.code }}
920919
E2E_RESULT: ${{ needs.e2e.result }}
921920
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
921+
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
922922
with:
923-
github-token: ${{ steps.app-token.outputs.token }}
924923
script: |
925-
const { PLAN_RESULT, CODE, E2E_RESULT, HEAD_SHA } = process.env;
926-
let conclusion, summary;
924+
const { PLAN_RESULT, CODE, E2E_RESULT, HEAD_SHA, RUN_URL } = process.env;
925+
let state, description;
927926
if (PLAN_RESULT !== 'success') {
928-
conclusion = 'failure';
929-
summary = `plan job did not succeed (result: ${PLAN_RESULT})`;
927+
state = 'failure';
928+
description = `plan job did not succeed (result: ${PLAN_RESULT})`;
930929
} else if (CODE !== 'true') {
931-
conclusion = 'success';
932-
summary = 'docs-only PR — e2e not required';
930+
state = 'success';
931+
description = 'docs-only PR — e2e not required';
933932
} else if (E2E_RESULT === 'success') {
934-
conclusion = 'success';
935-
summary = 'in-tree e2e passed';
933+
state = 'success';
934+
description = 'in-tree e2e passed';
936935
} else {
937-
conclusion = 'failure';
938-
summary = `in-tree e2e did not pass (result: ${E2E_RESULT})`;
936+
state = 'failure';
937+
description = `in-tree e2e did not pass (result: ${E2E_RESULT})`;
939938
}
940939
try {
941-
await github.rest.checks.create({
940+
await github.rest.repos.createCommitStatus({
942941
owner: context.repo.owner,
943942
repo: context.repo.repo,
944-
name: 'E2E Tests',
945-
head_sha: HEAD_SHA,
946-
status: 'completed',
947-
conclusion,
948-
output: { title: 'E2E Tests', summary },
943+
sha: HEAD_SHA,
944+
state,
945+
context: 'E2E Tests',
946+
description,
947+
target_url: RUN_URL,
949948
});
950949
} catch (err) {
951-
// The 'E2E Tests' check-run is the required merge gate. If we
952-
// cannot publish it, fail loudly and fail-closed: setFailed keeps
953-
// the job (and thus the gate) red so the PR stays non-mergeable.
954-
core.setFailed(`Failed to report E2E Tests check-run: ${err}`);
950+
// The 'E2E Tests' status is the required merge gate. If we cannot
951+
// publish it, fail loudly and fail-closed: setFailed keeps the job
952+
// red and the required status unreported ("Expected"), so the PR
953+
// stays non-mergeable.
954+
core.setFailed(`Failed to report E2E Tests status: ${err}`);
955955
return;
956956
}
957-
core.info(`E2E Tests → ${conclusion}: ${summary}`);
957+
core.info(`E2E Tests → ${state}: ${description}`);

0 commit comments

Comments
 (0)