need to use the token with the workflow permissions#91
Conversation
|
/describe |
|
✅ PR description has been generated and updated! |
Signed-off-by: Derek Misler <derek.misler@docker.com>
afaa6ee to
9304345
Compare
There was a problem hiding this comment.
Review Summary
Assessment: 🔴 CRITICAL
This PR replaces a scoped GitHub App token with an undocumented RELEASE_TOKEN secret in the release workflow. While this change may be necessary for editing workflows (as mentioned in the PR title), it introduces security and operational concerns that should be addressed.
Critical Issues
1 HIGH severity issue found: The token change removes explicit repository scoping and permission transparency, potentially increasing security risk if the token is compromised.
2 MEDIUM severity issues found: Git identity change may affect downstream automation; lack of permission verification could cause operational failures.
Findings
The following issues were identified in the changed code:
Recommendation: Before merging, please:
- Document the permissions and scope of
RELEASE_TOKENin the repository documentation - Verify that
RELEASE_TOKENhas only the minimum required permissions (checkout, push, PR operations on docker/pinata) - Confirm that docker/pinata's branch protection rules and automation work with
github-actions[bot]identity - Consider whether the GitHub App pattern was deliberately chosen for security reasons and if this change aligns with security policies
|
/review |
There was a problem hiding this comment.
Review Summary
Assessment: 🟢 APPROVE
Summary
This PR makes two sets of changes:
-
release.yml: Switches the
update-pinatajob from a GitHub App token toRELEASE_TOKENsecret. This is the correct approach since GitHub App tokens lack theworkflowspermission needed to push changes to.github/workflows/files. TheRELEASE_TOKEN(classic PAT with repo + workflow scopes) is already used elsewhere in this workflow and provides the necessary permissions. -
pr-describe.yml: Moves step outputs into environment variables before using them in the GitHub Script action. This is a security improvement that prevents script injection via untrusted PR metadata.
Both changes are technically sound with no bugs detected in the modified code.
Findings
No issues found. The code changes are correct and follow established patterns in the repository.
Summary
Switches the
update-pinatajob in the release workflow from a GitHub App token (viatibdex/github-app-token) to theRELEASE_TOKENsecret for authenticating with thedocker/pinatarepository. This is required because editing workflow files (.github/workflows/) demands a token with theworkflowscope, which a standard GitHub App token may not provide.Changes
.github/workflows/release.yml: Removes theGenerate GitHub App tokenstep and replaces all references tosteps.app-token.outputs.tokenwithsecrets.RELEASE_TOKENfor both thecheckoutstep and theGH_TOKENenv var used when creating/updating the PR. Also updates the git commit identity fromdocker-agent[bot]togithub-actions[bot]to match the new token's identity.How to Test
update-pinatajob completes successfully, checking outdocker/pinataand opening/updating a PR without permission errors on the workflow file change.docker/pinatashowsgithub-actions[bot]as the commit author.Closes: https://github.com/docker/gordon/issues/204