Skip to content

Add gh release create to the publishing workflow #3597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Apr 22, 2025
Merged

Add gh release create to the publishing workflow #3597

merged 4 commits into from
Apr 22, 2025
+11 −5

Conversation

illia-v
Copy link
Member

@illia-v illia-v commented Apr 19, 2025

Closes #3590

Here I add a missing command because of which the last release job failed.

Since we always change releases manually to add release notes, I added a --draft flag to the command so that the release is visible to users only after we add the release notes. Let me know if you are OK with this.

I tested the command by creating a release draft in my fork from my laptop, uploading a file to the created draft worked well too.

@illia-v illia-v added the Skip Changelog Pull requests that don't require a changelog entry label Apr 19, 2025
@pquentin
Copy link
Member

Thank you! What does zizmor thinks about the new YAML code?

@illia-v
Copy link
Member Author

illia-v commented Apr 20, 2025

Thank you! What does zizmor thinks about the new YAML code?

It reports the same error which existed before for gh release upload ${{ github.ref_name }} dist/* --repo ${{ github.repository }}

error[template-injection]: code injection via template expansion
  --> ./.github/workflows/publish.yml:75:7
   |
75 |       - name: "Upload dists to a new GitHub Release Draft"
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
76 |         env:
77 |           GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
78 | /       run: |
79 | |         gh release create ${{ github.ref_name }} \
...  |
84 | |           --verify-tag
85 | |         gh release upload ${{ github.ref_name }} dist/* --repo ${{ github.repository }}
   | |_______________________________________________________________________________________^ github.ref_name may expand into attacker-controllable code
   |
   = note: audit confidence → High

I used default environment variables to fix the error

pquentin
pquentin previously approved these changes Apr 22, 2025
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! It's unclear to me whether this new version is safe or just not parsed by zizmor. In any case, we should use quotes, which is what shellcheck would suggest. After that, LGTM!

@illia-v
Copy link
Member Author

illia-v commented Apr 22, 2025

It's unclear to me whether this new version is safe or just not parsed by zizmor.

Replacing with environment variables is suggested by zizmor docs

@illia-v @pquentin
Quote environment variables
3e10518 
Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>
@illia-v illia-v requested a review from pquentin April 22, 2025 10:46
pquentin
pquentin approved these changes Apr 22, 2025
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@illia-v illia-v merged commit 8e80f37 into urllib3:main Apr 22, 2025
33 of 35 checks passed
@illia-v illia-v deleted the publish-gh-release branch April 22, 2025 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pquentin pquentin pquentin approved these changes

@sethmlarson sethmlarson Awaiting requested review from sethmlarson sethmlarson is a code owner

@shazow shazow Awaiting requested review from shazow shazow is a code owner

Assignees
No one assigned
Labels
Skip Changelog Pull requests that don't require a changelog entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Step creating a release on GitHub is missing
2 participants
@illia-v @pquentin