DOC: Advocate for using action from tagged release commit shas #13

matthewfeickert · 2023-05-31T06:40:04Z

Resolves #7

For security best practices, advocate that users of the action use it from known commit shas that correspond to tagged releases.
Advocate that users use a Dependabot config file to update the action on new tags. This will bump the commit sha and also bump the release tag in the comment of the commit sha.

bsipocz

The notion sounds fine to me, and we talked about doing it anyway while writing the action, but first just wanted to get something out of the door asap.

bsipocz · 2023-05-31T19:51:27Z

README.md

@@ -11,12 +11,25 @@ jobs:
  steps:
    ...
    - name: Upload wheel
-      uses: scientific-python/upload-nightly-action@main
+      uses: scientific-python/upload-nightly-action@8f0394fd2aa0c85d7364a9958652e8994e06b23c # 0.1.0


could this be a major version instead, the same we do for checkout, or python, or etc?

It depends on how much you trust the project; releases / tags can be removed and replaced, while SHAs are a bit harder to fake. But if you trust official releases made by the org, and you just want to review the new action to make sure nothing big changed, versions are fine.

well, I would rather like to think that the releases made in the scientific-python org are official and trustworthy :)

(after all, we all use a lot of main actions, e.g. the artifact upload one, which is I agree is not super ideal as a best practice)

In that case, if you trust that bunch, versions seem fine ;)

I think that @ksunden gave a good and concise summary in matplotlib/matplotlib#26023 (comment). This is not about trusting orgs or not trusting orgs. It is about trying to be extra security cautious for anything that is publishing distributions to any package index. Is this overkill? Yeah, for sure in my mind, but it also seems like a pattern with no downsides other than you have to have your eyes track a little further across the screen to read the associated tag.

after all, we all use a lot of main actions, e.g. the artifact upload one, which is I agree is not super ideal as a best practice

I will fully agree with you though that tags are better than main, and patch version tags are better than major version tags. Though I'm also willing to agree that there should be additional security hardening than using a latest tag Docker image

upload-nightly-action/Dockerfile

Line 1 in dbd5a4d

FROM continuumio/miniconda3:latest

and so using the SHAs only really helps later releases (0.1.0 is the first commit in the repo). Also this GHA has no CI at all yet. 😬

Thanks for the detailed reply. To make this bikeshed really shiny, could I ask you to include some of Kyle's concise commentary as a comment in this readme? Or even just the link you have above would be useful.

Sure. I'm in meetings all day today though (on phone now) so feel free to add with the "Add a suggestion" tools and then accept and merge the suggestion if people would like this to get done before tomorrow.

README.md

bsipocz · 2023-06-02T00:16:48Z

README.md

@@ -11,12 +11,25 @@ jobs:
  steps:
    ...
    - name: Upload wheel
-      uses: scientific-python/upload-nightly-action@main
+      uses: scientific-python/upload-nightly-action@8f0394fd2aa0c85d7364a9958652e8994e06b23c # 0.1.0


Thanks for the detailed reply. To make this bikeshed really shiny, could I ask you to include some of Kyle's concise commentary as a comment in this readme? Or even just the link you have above would be useful.

README.md

* For security best practices, advocate that users of the action use it from known commit SHAs that correspond to tagged releases. * Advocate that users use a Dependabot config file to update the action on new tags. This will bump the commit SHA and also bump the release tag in the comment of the commit SHA. - c.f. https://learn.scientific-python.org/development/guides/gha_basic/#updating

* 'Intregration' -> 'Integration'

matthewfeickert · 2023-06-05T22:16:11Z

README.md

+<!-- c.f. https://github.com/scientific-python/upload-nightly-action/pull/13 and
+https://github.com/matplotlib/matplotlib/pull/26023#discussion_r1212539700
+for short summary of why using commit SHA -->
+


@bsipocz ...and after a long delay given too many meetings and weekend travel this is in now. Look good?

jarrodmillman · 2023-06-08T15:58:27Z

Feel free to follow-up on this in a future PR, but this looks like it is already a good improvement. Thanks!!

matthewfeickert added the documentation Improvements or additions to documentation label May 31, 2023

matthewfeickert requested review from Carreau and martinfleis May 31, 2023 06:40

matthewfeickert self-assigned this May 31, 2023

stefanv approved these changes May 31, 2023

View reviewed changes

bsipocz reviewed May 31, 2023

View reviewed changes

matthewfeickert force-pushed the docs/recommend-use-of-tags branch from f6c122b to 4de6747 Compare June 2, 2023 00:08

matthewfeickert requested a review from bsipocz June 2, 2023 00:09

bsipocz approved these changes Jun 2, 2023

View reviewed changes

matthewfeickert mentioned this pull request Jun 2, 2023

Add tests and CI #15

Closed

matthewfeickert added 2 commits June 5, 2023 17:14

DOC: Fix typo

7d347c3

* 'Intregration' -> 'Integration'

matthewfeickert force-pushed the docs/recommend-use-of-tags branch from 4de6747 to 7d347c3 Compare June 5, 2023 22:15

matthewfeickert commented Jun 5, 2023

View reviewed changes

jarrodmillman approved these changes Jun 8, 2023

View reviewed changes

Tighten up README

057df35

jarrodmillman merged commit 468089a into scientific-python:main Jun 8, 2023

matthewfeickert deleted the docs/recommend-use-of-tags branch June 8, 2023 16:00

jarrodmillman added this to the 0.2.0 milestone Jun 8, 2023

This was referenced Jan 24, 2024

CI: Pin scientific-python/upload-nightly-action to release sha pydata/xarray#8662

Merged

CI: Pin scientific-python/upload-nightly-action to release sha scikit-image/scikit-image#7306

Merged

matthewfeickert mentioned this pull request Oct 1, 2024

Write summary of when to and when not to pin actions to hash level #105

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Advocate for using action from tagged release commit shas #13

DOC: Advocate for using action from tagged release commit shas #13

matthewfeickert commented May 31, 2023

bsipocz left a comment

bsipocz May 31, 2023

stefanv May 31, 2023

bsipocz May 31, 2023

bsipocz May 31, 2023

stefanv May 31, 2023

matthewfeickert Jun 1, 2023 •

edited

Loading

bsipocz Jun 2, 2023

matthewfeickert Jun 2, 2023

bsipocz Jun 2, 2023

matthewfeickert Jun 5, 2023

jarrodmillman commented Jun 8, 2023

DOC: Advocate for using action from tagged release commit shas #13

DOC: Advocate for using action from tagged release commit shas #13

Conversation

matthewfeickert commented May 31, 2023

bsipocz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewfeickert Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarrodmillman commented Jun 8, 2023

matthewfeickert Jun 1, 2023 •

edited

Loading