jupyter-server / jupyter_releaser

A set of helper scripts and GitHub Actions to aid in automated releases of Python and npm packages.
https://jupyter-releaser.readthedocs.io/en/latest/
Other
46 stars 64 forks source link

Replace usage of ADMIN_GITHUB_TOKEN #505

Closed blink1073 closed 7 months ago

blink1073 commented 1 year ago

Problem

We should not have to worry about the ADMIN_GITHUB_TOKEN. It is a maintenance burden and an additional security surface area.

Proposed Solution

We should instead create a PR with the commits that were previously pushed directly, and merge it from the Action. This will allow branch protections to remain in place. We can recommend the use of environments with restricted users in the publish action, regardless of whether OIDC tokens are used (#504).

fcollonval commented 1 year ago

I tried using environment and the trusted PyPI publisher action with https://github.com/jupyterlab-contrib/search-replace

For what I saw, the only default restriction you can put on environment is adding mandatory review or delay the execution. But app may be use to perform more complex validation rules.

From my test, I'm wondering if a solution could not be to create an app (like the jupyterlab-probot e.g.) that enforces execution within an environment.

From GitHub.com forum, a way could be to use expressions: https://github.com/orgs/community/discussions/26622. But it seems annoying to maintain a users list.

blink1073 commented 1 year ago

The workflow itself has to be marked as requiring an environment. I did a full end to end test [here|https://github.com/jupyter/security/issues/63]

fcollonval commented 1 year ago

Thanks @blink1073

In what I tried I did something similarly to your test by adding mandatory review on the environment. I just found that this add a step to do the release; my laziness kicked in why one more step :wink: But yeah likely better to have to do one more click than having a bad release.

blink1073 commented 9 months ago

New plan: use the user's GITHUB_TOKEN, and recommend the use of an environment. Technically our check for the github actor's permissions would still work, but environment is still better.

To handle forwardport and silent changelog PRs, we add an option to make the PR comment to toggle the PR.

blink1073 commented 9 months ago

For release-from-releaser, we will need to keep using ADMIN_GITHUB_TOKEN. Otherwise, use secrets.GITHUB_TOKEN by default.

Add a new option for post-PR comment, that will default to @jupyterlab-bot, please restart ci.

Update the docs and examples to always use an environment.

Test this against https://github.com/blink1073/test-python-project/ before committing to the process.

First, I need to update jupyterlab-probot to return the default config if no config file is found, because it is returning an empty object:

..."msg":"GitHub request: GET https://api.github.com/repos/blink1073/.github/contents/.github%2Fjupyterlab-probot.yml - 404"}
2023-12-24T02:48:50.916860+00:00 app[web.1]: 
2023-12-24T02:48:50.916862+00:00 app[web.1]: --------------------------------
2023-12-24T02:48:50.916878+00:00 app[web.1]: Handling Issue Comment Created:
2023-12-24T02:48:50.916891+00:00 app[web.1]:     repo: blink1073/test-python-project
2023-12-24T02:48:50.916906+00:00 app[web.1]:     number: 107
2023-12-24T02:48:50.916918+00:00 app[web.1]:     config:
2023-12-24T02:48:50.916953+00:00 app[web.1]: {}
2023-12-24T02:48:50.916967+00:00 app[web.1]: Ignored
2023-12-24T02:48:50.916980+00:00 app[web.1]: Finished handling of issue comment created
blink1073 commented 7 months ago

Okay, I did some testing in my scratch repo, and here's my recommendation:

For the GitHub App:

Then, per repo:

blink1073 commented 7 months ago

https://github.com/jupyter/nbconvert/pull/2111

blink1073 commented 7 months ago

Good to go! https://github.com/jupyter/nbconvert/actions/runs/7959610271

jtpio commented 7 months ago

Nice!

Create one per org

So that means for repos hosted under a user account (not part of an organization) the user will have to install a GitHub App for their account (likely to be treated as an "org" by GitHub already)?

blink1073 commented 7 months ago

Yep, here's mine:

image
jtpio commented 7 months ago

Good to go! https://github.com/jupyter/nbconvert/actions/runs/7959610271

Looks like the commit corresponds to the user who run the workflow, which is nice for tracking and knowing who made the release (as previously discussed in https://github.com/jupyter-server/jupyter_releaser/pull/521) :+1:

image

blink1073 commented 7 months ago

Okay, I'll open separate issues to discuss removing release-from-releaser and pypi_token. The PR closing this issue will really just be a documentation update and an update to the example workflows.