Skip to content

fix(ci): keep release credentials out of the build checkout - #767

Merged
rng1995 merged 40 commits into
mainfrom
yashraj/release-checkout-credential-followup-20261006
Oct 14, 2026
Merged

rng1995 merged 40 commits into
mainfrom
yashraj/release-checkout-credential-followup-20261006

Conversation

@yashrajp22

Copy link
Copy Markdown
Collaborator

The release checkout leaves its write-scoped GitHub token in Git configuration while dependency installation and build tools run. Set persist-credentials: false so those steps do not inherit the stored credential; the release step continues to use its explicit GH_TOKEN. Also correct the pinned setup-uv action comment to its verified v5.4.2 tag.

Follow-up to #746, which has already merged.

Validation: all 11 release workflow and helper tests pass against the current-main-based commit in an isolated container. Ruff lint and formatting checks pass.

Signed-off-by: yashrajbasav <yashrajbasav@nvidia.com>
@davidjmoorembaypn

Copy link
Copy Markdown

check status

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

@yashrajp22 thank you for following up #746 and taking the release token out of the build steps' reach.

Value and readiness

By default actions/checkout writes the job's GITHUB_TOKEN (here contents: write) into the checkout's .git/config, where every later step, including uv sync and the hatchling build, can read it. persist-credentials: false removes that copy, and nothing after checkout needs it: create_github_release.py talks to GitHub only through gh api / gh release with an explicit --repo and the step-scoped GH_TOKEN, never git push. The version is static in pyproject.toml and uv.lock has no git sources, so the build needs no git credentials. The # v5.4.2 comment is accurate. Ready for maintainer review.

Material findings

None blocking. Optional follow-ups:

  1. Optional (pre-existing, not introduced here), .github/workflows/release.yml:51-59: with python-version set, setup-uv v5.4.2 runs uv venv and prepends .venv/bin to PATH (src/setup-uv.ts at the pinned SHA). So python scripts/release/... runs the venv interpreter that uv sync --extra dev populated, and any dependency startup hook (.pth) runs in the one process that holds GH_TOKEN. More generally, code in any step of a contents: write job can try to recover the job token from the runner. The robust follow-up is a two-job split: build with contents: read and upload dist/ as an artifact, then publish from a job that installs nothing.
  2. Optional: .github/workflows/ci.yml:68,84 still label the same SHA # v5.
  3. Optional: the contract test finds persist-credentials: false anywhere in the file. That is fine with one checkout, but it would not notice a second checkout without it.

PIC tradeoffs

None identified.

Verification and gaps

  • Read the full workflow and scripts/release/public/create_github_release.py at the head: no git CLI use, and every gh call passes --repo.
  • Resolved astral-sh/setup-uv tag v5.4.2 to d4b2f3b6… via the GitHub API and read its action.yml and setup-uv.ts.
  • Head 10d87459 only adds github-actions main merges; the PR's own diff has the same patch-id as f554f807, whose CI passed. CI on the current head is action_required (awaiting maintainer approval), so it has not re-run.
  • I did not execute the workflow.

@yashrajp22

Copy link
Copy Markdown
Collaborator Author

Thanks! I approved the pending CI run so the checks could start. I also checked the follow-up design suggestions; splitting the existing release job is separate from this action-pinning fix, so I have kept this PR focused on pinning the action and disabling persisted checkout credentials.

@rng1995
rng1995 merged commit 665bb56 into main Oct 14, 2026
5 checks passed
@rng1995
rng1995 deleted the yashraj/release-checkout-credential-followup-20261006 branch October 14, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants