Skip to content

Commit 5c35d7d

Browse files
committed
cascading-runs: do verify that the sender is trusted
The cascading runs feature of GitForWindows' GitHub App basically listens for Check Runs to be completed, and upon their completion, other automation is triggered. There are currently two such cascades: - When the `tag-git` Check Run in a regular Git for Windows PR has completed (which is expected to be triggered via the `/git-artifacts` slash command by a trusted user), the corresponding `git-artifacts` workflow runs have been triggered, one per supported CPU architecture. - When the `git-artifacts` Check Runs complete on a commit on git-for-windows/git's `main` branch that does _not_ correspond to a release, the `upload-snapshot` workflow needs to be triggered. In both instances, we need to validate that the source of these Check Runs is the intended one. We need that because Check Runs are also created for regular workflow jobs, and so far the validation goes by Check Run name, which is easily faked in a crafted PR. So let's verify that the Check Run events in question were sent by a trusted actor (and most notably _not_ by GitHub Actions implicit Check Runs corresponding to PR workflow runs). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent 02d5c0e commit 5c35d7d

File tree

1 file changed

+13
-0
lines changed

1 file changed

+13
-0
lines changed

GitForWindowsHelper/cascading-runs.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ const getToken = (() => {
1111
return async (context, owner, repo) => tokens[[owner, repo]] || (tokens[[owner, repo]] = await get(context, owner, repo))
1212
})()
1313

14+
const isAllowed = async (login) => {
15+
if (login === 'gitforwindowshelper[bot]') return true
16+
const getCollaboratorPermissions = require('./get-collaborator-permissions')
17+
const token = await getToken()
18+
const permission = await getCollaboratorPermissions(context, token, owner, repo, login)
19+
return ['ADMIN', 'MAINTAIN', 'WRITE'].includes(permission.toString())
20+
}
21+
1422
const triggerGitArtifactsRuns = async (context, checkRunOwner, checkRunRepo, tagGitCheckRun) => {
1523
const commitSHA = tagGitCheckRun.head_sha
1624
const conclusion = tagGitCheckRun.conclusion
@@ -107,13 +115,16 @@ const cascadingRuns = async (context, req) => {
107115
const checkRunRepo = req.body.repository.name
108116
const checkRun = req.body.check_run
109117
const name = checkRun.name
118+
const sender = req.body.sender.login
110119

111120
if (action === 'completed') {
112121
if (name === 'tag-git') {
113122
if (checkRunOwner !== 'git-for-windows' || checkRunRepo !== 'git') {
114123
throw new Error(`Refusing to handle cascading run in ${checkRunOwner}/${checkRunRepo}`)
115124
}
116125

126+
if (!await isAllowed(sender)) throw new Error(`${sender} is not allowed to do that`)
127+
117128
const comment = await triggerGitArtifactsRuns(context, checkRunOwner, checkRunRepo, checkRun)
118129

119130
const token = await getToken(context, checkRunOwner, checkRunRepo)
@@ -136,6 +147,8 @@ const cascadingRuns = async (context, req) => {
136147
if (checkRunOwner === 'git-for-windows'
137148
&& checkRunRepo === 'git'
138149
&& name.startsWith('git-artifacts-')) {
150+
if (!await isAllowed(sender)) throw new Error(`${sender} is not allowed to do that`)
151+
139152
const output = req.body.check_run.output
140153
const match = output.summary.match(
141154
/Build Git (\S+) artifacts from commit (\S+) \(tag-git run #(\d+)\)/

0 commit comments

Comments
 (0)