-
Notifications
You must be signed in to change notification settings - Fork 2k
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
release: always use service user for git ops #24546
Conversation
cbb96c4
to
0ef3b6a
Compare
0ef3b6a
to
49cd049
Compare
contents: write | ||
contents: read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't mentioned in the PR description. I know we've had to set this field to write
in our workflows explicitly previously but that it was unexpected. Why is it safe to change it back now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this permissions
section specifies what the actions-dispensed GITHUB_TOKEN
is capable of doing, and we only want it to pull the repo (actions/checkout
). for pushes we instead want to use the ELEVATED_GITHUB_TOKEN
attached to our bot user.
based on the commit that set this (b23fe72), I suspect we thought it needed to be write
because actions/checkout
sets github-actions bot token in local git config, which then takes precedence over the global git config that we set using the elevated token.
so to counteract all that, we both set this to read
and actions/checkout
to persist-credentials: false
, so that git config --global
takes effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
We are shoring up our user and bot accesses, and this seems to be our only usage of the github-actions bot.
These were the commits that have been flagged as needing adjustment:
We may later discover other of our repos that need a similar treatment, if they've just not been run during this information-gathering time, but at present this is the only known case. Our CE->ENT merge workflow in the ENT repo is already set up properly.