Skip to content
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.

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

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

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

matthewfeickert
Copy link
Contributor

c.f. scientific-python/upload-nightly-action#13 for additional context.

I'll suggest @martinfleis and @keewis for reviewers.

  • [N/A] Closes #xxxx
  • [N/A] Tests added
  • [N/A] User visible changes (including notable bug fixes) are documented in whats-new.rst
  • [N/A] New functions/methods are listed in api.rst

* For security best practices, use the action from known commit shas that
  correspond to tagged releases. These can be updated via dependabot.
Copy link

welcome bot commented Jan 24, 2024

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

Copy link
Contributor

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

I suppose it doesn't hurt to specify commit hash but why not just pointing to the version tag like with practically any other action?

@matthewfeickert
Copy link
Contributor Author

matthewfeickert commented Jan 24, 2024

I suppose it doesn't hurt to specify commit hash but why not just pointing to the version tag like with practically any other action?

Good question. This is discussed in the linked PR in the PR body (here again: scientific-python/upload-nightly-action#13) but the 1 liner answer is that anytime you're touching the open source packaging supply chain you want to take additional security measures to make sure it is as hard as possible for an upstream mistake to cause a lot of pain. The https://github.com/pypa/gh-action-pypi-publish team asks that people take additional precautions as well, and we've taken a lot of recommendations from them on the subject, though I will admit that their README has a softened stance to tag name over commit sha.

If you want to go with tag names that's still better than @main so 👍, though if you let Dependabot manage it for you it really doesn't make a difference if you use the sha or the tag name, except the sha is more secure.

@max-sixty
Copy link
Collaborator

I would vote to use the version number. We want to keep up-to-date, and we're not going to be able to audit the uploader on each change — so I don't see a significant benefit from using the hash, and we lose dependabot.

@matthewfeickert
Copy link
Contributor Author

so I don't see a significant benefit from using the hash, and we lose dependabot.

c.f. dependabot/dependabot-core#4691

@max-sixty
Copy link
Collaborator

so I don't see a significant benefit from using the hash, and we lose dependabot.

c.f. dependabot/dependabot-core#4691

Oh — so dependabot will update the hash? Great!! Thanks for finding this

@keewis keewis merged commit 037a39e into pydata:main Jan 24, 2024
29 checks passed
Copy link

welcome bot commented Jan 24, 2024

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! celebration gif

@keewis
Copy link
Collaborator

keewis commented Jan 24, 2024

thanks, @matthewfeickert

@matthewfeickert matthewfeickert deleted the ci/pin-for-security branch January 24, 2024 22:57
@matthewfeickert
Copy link
Contributor Author

Oh — so dependabot will update the hash? Great!! Thanks for finding this

@max-sixty @martinfleis Yes, sorry for the short replies — was moving between things. 😬

Yeah, this was actualy a recent find for me too, but I've been pretty impressed so far.

If anything here goes screwy though in the future, please feel free to just ping me as I'm more than happy to try to help out on anything that I've introduced. But I hope this is all super smooth sailing, and thanks for being users of the uploader and thanks to the team for xarray! 🚀

@max-sixty
Copy link
Collaborator

Awesome, thank you very much @matthewfeickert !

andersy005 added a commit to TomNicholas/xarray that referenced this pull request Jan 30, 2024
* main: (153 commits)
  Add overloads to get_axis_num (pydata#8547)
  Fix CI: temporary pin pytest version to 7.4.* (pydata#8682)
  Bump the actions group with 1 update (pydata#8678)
  [namedarray] split `.set_dims()` into `.expand_dims()` and `broadcast_to()` (pydata#8380)
  Add chunk-friendly code path to `encode_cf_datetime` and `encode_cf_timedelta` (pydata#8575)
  Fix NetCDF4 C version detection (pydata#8675)
  groupby: Don't set `method` by default on flox>=0.9 (pydata#8657)
  Fix automatic broadcasting when wrapping array api class (pydata#8669)
  Fix unstack method when wrapping array api class (pydata#8668)
  Fix `variables` arg typo in `Dataset.sortby()` docstring (pydata#8670)
  dt.weekday_name - removal of function (pydata#8664)
  Add `dev` dependencies to `pyproject.toml` (pydata#8661)
  CI: Pin scientific-python/upload-nightly-action to release sha (pydata#8662)
  Update HOW_TO_RELEASE.md by clarifying where RTD build can be found (pydata#8655)
  ruff: use extend-exclude (pydata#8649)
  new whats-new section (pydata#8652)
  xfail another test on windows (pydata#8648)
  use first element of residual in _nonpolyfit_1d (pydata#8647)
  whatsnew for v2024.01.1
  implement `isnull` using `full_like` instead of `zeros_like` (pydata#7395)
  ...
@matthewfeickert
Copy link
Contributor Author

For posterity, here's a recent example of Dependabot bumping the pinned upload-nightly-action from 0.2.0 to 0.3.0 for networkx: networkx/networkx#7266

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