Uh oh!
There was an error while loading. Please reload this page.
Keep the job token out of a tree the PR author controls - #105
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe ChangesRNG hygiene checkout
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uh oh!
There was an error while loading. Please reload this page.
Summary
Adds
persist-credentials: falseto therng-hygienejob's checkout inci.yml.Why
The job checks out a PR branch and then executes a script from it —
scripts/check-rng-hygiene.sh, which any PR author can edit.actions/checkoutwritesGITHUB_TOKENinto.git/configby default, leaving it readable by that PR-authored code. The job performs no git operations needing credentials.Found in a CodeRabbit comment on #104 that I merged past without reading. The same fix is going to keep and keep-esp32.
CodeRabbit also flagged on #104 that
scanner_diedcould fail open — that one was already fixed before merge (the|| scanner_diedis at the call sites, not inside the command substitution), verified on current main.It additionally asked whether
rng-hygieneis a required status check in branch protection. It is not:mainis unprotected, so the guard is advisory. That is a repo-settings decision rather than a code change, and is worth deciding separately.Test plan
ci.ymlparses as YAMLscripts/check-rng-hygiene.shstill clean on the treeSummary by CodeRabbit