Repository navigation
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Windows result for head Host: Windows 11 26300, Python 3.13.5.
AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — no blocking findings
The release publish job no longer exposes the write token at job level or to the checkout. GH_TOKEN moves to the three steps that call gh, and the checkout keeps no credentials. I checked each gh call and each git command in the job, and none is left without the token or needs the checkout's credentials. The hosted ubuntu CTest run passed release_workflow. The release workflow does not run on pull requests, so its behaviour on a real release is not exercised here.
What I ran
- Head
c0ab0ee272fb4ecc7c24aea3f036dfd921fc1d56; basec2ba690bfc6597c3b33084057a02ffa7f0f49d10. 4 files:.github/workflows/release.yml(the publish job: jobenvlosesGH_TOKEN; the checkout getspersist-credentials: false;GH_TOKENmoves to the release-notes, draft-and-upload and publish steps),CMakeLists.txt(+2: registersrelease_workflow), andtools/tests/test_release_workflow.py(new, 23 lines: two tests on the publish job's text). - Read the publish job from
publish:to the end. Theghcalls are:gh api ... generate-notes(the release-notes step, withGH_TOKENin its ownenv), andgh release list,create,view,editanduploadin the draft-and-upload step (which hasGH_TOKEN). The last one isgh release edit --draft=false(the publish step, withGH_TOKEN). Nogitpush, fetch, pull, tag or remote command is in the job. - Hosted: the ubuntu relinker job's CTest run for this head (550 of 550 passed) ran
release_workflow(Test #36, passed, 0.06 s). - Read-only checks from the trusted main checkout: diff check, conventions and PR template, and the stack check all passed. The ABI audit is not applicable. The Python test was not run here, because the PR changes
.pyfiles. - No MarcZX10 review at this head.
Results
| Check | Result | Note |
|---|---|---|
| Diff check | passed | |
| Conventions and PR template | passed | conventions check from the main checkout |
| Mergeability and dependencies | passed | stack check passed; no declared dependencies |
Every gh call in the publish job has GH_TOKEN |
passed by reading | the draft-and-upload step's env covers its gh release view and the rest of the step |
No git command in the publish job needs the checkout's credentials |
passed by reading | none found |
release_workflow (hosted, ubuntu CTest) |
passed | Test #36, 0.06 s |
| The release workflow on a real release | not run | the release workflow does not run on pull requests |
| ABI audit | not applicable | no export changed |
| Build and runtime tests (Linux) | run hosted | see above |
| Negative controls | not run | no local build |
| Hosted CI | passed | conventions, eligibility, intake, labels, relinkers, relinker-asan, windows succeeded; linux skipped |
Read-only check output
Precheck was not run: no PR-head code is run here, and the PR changes .py files. Diff check: passed. Conventions: passed. ABI audit: not applicable. Stack check: passed. Hosted: gh api repos/boykopovar/AnyPS5/commits/c0ab0ee272fb4ecc7c24aea3f036dfd921fc1d56/check-runs, and the ubuntu relinker job log.
Negative control
Not run here. A negative control needs a local run of the Python tests and a release run, and neither is available from this head.
Findings
Blocking
None.
Non-blocking
None.
Sources
- Workflow: https://github.com/boykopovar/AnyPS5/blob/c0ab0ee272fb4ecc7c24aea3f036dfd921fc1d56/.github/workflows/release.yml
- New test: https://github.com/boykopovar/AnyPS5/blob/c0ab0ee272fb4ecc7c24aea3f036dfd921fc1d56/tools/tests/test_release_workflow.py
- Registration: https://github.com/boykopovar/AnyPS5/blob/c0ab0ee272fb4ecc7c24aea3f036dfd921fc1d56/CMakeLists.txt (the
release_workflowadd_test) - Hosted log: ubuntu relinker job for c0ab0ee (CTest 550 of 550)
Not verified
- A real release run of the publish job with the token moved (the release workflow does not run on pull requests).
To get this to a good state
- Nothing to change. The first real release after this lands will show whether the moved token works end to end.
…nd environment The publish job gave every step GH_TOKEN with contents: write, and its checkout kept its credentials on disk, so tagged repository code ran with a write token in its environment and in .git/config. The checkout no longer persists credentials, and the token is set only on the three steps that call gh.
… of checkout and environment The tests check that the publish job's checkout sets persist-credentials: false and that GH_TOKEN is not set in the job environment. Against the previous workflow both fail.
c0ab0ee to
e2857fc
Compare
What
The
publishjob inrelease.ymlgives every stepGH_TOKEN: ${{ github.token }}withcontents: write, and itsactions/checkoutkeeps its credentials on disk. So the tagged repository code that the job runs (tools/package_release.py,tools/prepare_release_notes.py) runs with a write token in its environment and in.git/config. Neither tool needs the token; I checked both and neither callsghor reads a token variable.Fix, in two commits:
b537e2400e: the checkout setspersist-credentials: false, andGH_TOKENis set only on the three steps that callgh(release notes, draft and upload, publish). The job-level environment keepsGH_REPO.c0ab0ee272:tools/tests/test_release_workflow.pychecks both points, registered inCMakeLists.txt.Impact is low: only a user who can push a
v*tag runs the job, and that user can already release. The change limits what the tagged code can do with the token.Tested
Linux, Python 3.11.2, on
upstream/mainc674f8c14dplus these commits:python3 tools/tests/test_release_workflow.py: 2 tests, OK.release.ymlonupstream/main, both tests fail.release.ymlparses as YAML (PyYAML).python3 tools/check_conventions.py --base upstream/mainandgit diff --check upstream/main: clean.tools/pr_overlap.py --branchfails closed on 11 open PRs whoseCMakeLists.txtconflicts withmain(feat(aot): discuss native ARM64 compiler architecture and integration #1835, feat(aot): consolidate native ARM64 compiler, runtime and consumers #5021, feat(agc): look up table images at run time through a device image table on the GPU heap (APS5_GPU_PATH) #7642, fix(tools): merge only the pull requests that share files or exports with the branch in pr_overlap #9539, feat(agc): serve lane-keyed, pointer and runtime image-table columns #10193, feat(freetype): implement font table validation #10897, feat(libSceIcu): implement property and case exports from ICU tables #13578, feat(agc): resolve table samplers through device image keys #13928, feat(agc): resolve runtime sampler addresses through device image keys #13929, docs(debt): separate entries with blank lines and drop derived export totals #14212, feat(run-report): write the run report when a game ends #14235). Compared by hand: none of their CMake hunks namesaudit_dump,annotate_module,unwrap_selforrelease_workflow. That is evidence, not proof.git merge-treeof this branch withmainis clean.test_quality_heuristicscannot run in the PR head tree, so it was run by hand from the main checkout ontools/tests/test_release_workflow.py: 0 findings.Not run: a tag-triggered release run. The first release after merge shows whether the three
ghsteps still have the token they need.Checklist
mainNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)