chore: don't persist git credentials on checkout in release workflow - #16248
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/2a4a8dce5bf2b0b28f628a127b31201af768a584Open in Note This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed. |
|
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| # This makes Actions fetch all Git history so that Changesets can generate changelogs with the correct commits | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
driveby change: Changesets nowadays should auto-deepen a shallow clone
| env: | ||
| NPM_CONFIG_PROVENANCE: true |
There was a problem hiding this comment.
driveby change: from what I understand, this really shouldn't be required anymore (it might have been in the past). We publish Changesets itself with provenance just fine and we don't need to specify any such environment variable.
There was a problem hiding this comment.
@Andarist are you sure? This is set so that our packages are published with provenance -- it doesn't have anything to do with whether changesets is published with provenance.
There was a problem hiding this comment.
Oh wait. I totally misread your comment. I understand now. 😆
There was a problem hiding this comment.
This the default if you use trusted publishing i believe, which is why it isn't required.
| with: | ||
| # This makes Actions fetch all Git history so that Changesets can generate changelogs with the correct commits | ||
| fetch-depth: 0 | ||
| persist-credentials: false |
teemingc
left a comment
There was a problem hiding this comment.
Thank you! Let's give it a shot
Adds `persist-credentials: false` to the 13 read-only checkouts in `ci.yml`, `audit.yml` and the platform test workflows, so the workflow token is no longer written into `.git/config` where every later step can read it. None of these jobs run an authenticated git operation after checkout. #16248 already covered the release workflow. In #15876's review, ghostdevv [suggested these exact additions](#15876 (comment)) and elliott [planned to check on them in a follow-up](#15876 (comment)). This does the mechanical half of that so it doesn't get lost. If it makes more sense folded into a bigger hardening pass, feel free to close this one. With this change, zizmor's `artipacked` audit flags only `autofix-lint.yml`, which keeps its credentials because it pushes a commit. The `secrets: inherit` part of #15529 is deliberately left untouched. Addresses the `persist-credentials` half of #15529.
I'm a Changesets maintainer. @teemingc reported issues with your publishing workflow (see a failed run here).
I think this is caused by changesets/action#670 . I have not anticipated this to clash with the default
persist-credentials: trueofactions/checkout.While I need to fix this in
changesets/action, I think there is no strong reason for you to persist this git credential here - so to unblock you sooner, I'm proposing this patchPlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits