fix: harden CI workflow: switch to pull_request trigger and remove unnecessary id-token:write#214
Merged
gvanrossum merged 2 commits intomicrosoft:mainfrom Mar 5, 2026
Conversation
… "id-token:write" permission Addressing two issues in the CI workflow: 1. **`pull_request_target` > `pull_request`**: Fork PRs currently run in the base repo's execution context. Switching to pull_request runs fork PRs in the fork's context, which matches the workflow's declared read-only permissions and is the recommended trigger for CI jobs that don't need write access to the base repo. 2. **Remove `id-token: write`**: No active job uses OIDC. This permission is a leftover from the commented-out `online-test` job and grants unnecessary token-minting capability. No CI behavior change. All active jobs (`check`, `format`, `offline-test`) only need `contents: read` and `pull-requests: read`, which work identically with `pull_request`.
Contributor
Author
|
Hello @gvanrossum Just FYI, this is in relation to the two CI issues I mentioned earlier. |
Collaborator
|
I will discuss this with @robgruen. But doesn't this enable a type of attack that changes the CI yml files? |
gvanrossum
reviewed
Mar 5, 2026
Collaborator
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Thanks -- two nits and then I'll approve it.
1. Removed the comment from "pull_request: " 2. Removed the "id-token: write" line
gvanrossum
approved these changes
Mar 5, 2026
Collaborator
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Thanks! Will merge now.
Contributor
Author
|
Thanks, Guido! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressing two issues in the CI workflow:
pull_request_target>pull_request: Fork PRs currently run in the base repo's execution context. Switching to pull_request runs fork PRs in the fork's context, which matches the workflow's declared read-only permissions and is the recommended trigger for CI jobs that don't need write access to the base repo.(Some references:
https://docs.github.com/en/enterprise-cloud@latest/actions/reference/security/secure-use#mitigating-the-risks-of-untrusted-code-checkout
https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ )
id-token: write: No active job uses OIDC. This permission is a leftover from the commented-outonline-testjob and grants unnecessary token-minting capability.No CI behavior change. All active jobs (
check,format,offline-test) only needcontents: readandpull-requests: read, which work identically withpull_request.