Open jsoref opened 6 months ago
@jsoref Sorry to hear about the concussion π I'll get this triaged for review β¨
Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert :eyes:
@jsoref Hello! π Our engineering team reviewed, and agreed with your proposed clarification. They also wanted to mention the following -
The suggested text is not quite right though - the read-only behavior applies to all pull request events triggered on a fork PR except
pull_request_target
So itβs not correct to say it only applies to
pull_request
events, it would apply topull_request_review
and the other PR/issue related events
With that adjustment in mind, you or anyone else is welcome to open a PR with this update β¨
CC @jc-clark just for visibility π
Hello @nguyenalex836,
I have read through the conversation and the suggestions from the engineering team. Please review the proposed update for the documentation to clarify the behavior of workflow job permissions. The read-only adjustment applies to all pull request-related events triggered on a fork PR except for the pull_request_target
event. This ensures that users understand the specific conditions under which write permissions remain unchanged.
Updated section:
Finally, if the workflow was triggered by a pull request from a forked repository, and the Send write tokens to workflows from pull requests setting is not selected, the permissions are adjusted to change any write permissions to read-only, except for the
pull_request_target
event. This applies to all pull request-related events, includingpull_request_review
.
Is this good to go?
@Vikranth3140: the pull_request_review
bit was a correction to my mentioning pull_request
. I think it's probably best to omit both.
You probably want to include:
For more information, see "[AUTOTITLE](/actions/using-workflows/events-that-trigger-workflows#pull_request_target)."
(That text appears in places like https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token -- see https://github.com/search?q=repo%3Agithub%2Fdocs%20see%20%22%5BAUTOTITLE%5D(%2Factions%2Fusing-workflows%2Fevents-that-trigger-workflows).%22&type=code)
Hello @jsoref,
Thank you for the feedback. I understand the need to clarify the read-only behaviour without specifying pull_request
or pull_request_review
. I will update the documentation to reflect that the read-only adjustment applies to all pull request-related events triggered on a fork PR except for the pull_request_target
event.
Additionally, I will include a reference link for further information:
Finally, if the workflow was triggered by a pull request from a forked repository, and the Send write tokens to workflows from pull requests setting is not selected, the permissions are adjusted to change any write permissions to read-only, except for the
pull_request_target
event.For more information, see "AUTOTITLE."
Is this approach good to go?
The order of the logic doesn't really work for me. It's convoluted.
I wouldn't use except there.
I suspect I'd want "isn't a pull_request_target and the send... Isn't checked".
I don't have time to think about it further today. Maybe someone else will before I get back...
@Vikranth3140 Hello! π Thank you for opening a PR for this issue! Our team will provide feedback on your proposed changes in the PR you opened π https://github.com/github/docs/pull/33566
@jsoref Thank you for providing feedback on the proposed changes β¨
Good π
Code of Conduct
What article on docs.github.com is affected?
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#how-the-permissions-are-calculated-for-a-workflow-job
What part(s) of the article would you like to see updated?
Should be changed to say clarify that if the workflow was triggered by a pull request and the job event is
pull_request_target
then write permissions will not be changed to read only.I'm still recovering from a concussion, but here's my first attempt at fixing this text:
Additional information
No response