Skip to content

feat: validate GitHub App permissions via API and add tests#70

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.

Sign up for GitHub

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

Open
jcfr wants to merge 6 commits into
base:main
Choose a base branch
from
Open

feat: validate GitHub App permissions via API and add tests #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6bac163
feat: validate GitHub App permissions via API and add tests
jcfr Feb 28, 2025
723b728
fix tests and bundle with `npm run all`
GrantBirki Mar 1, 2025
0a29629
create a new `allow_github_apps` input to allow Apps to be disabled i…
GrantBirki Mar 1, 2025
bb2d6df
update docs to reflect new inputs/outputs
GrantBirki Mar 1, 2025
59dbe2f
move `parameters.md` up a level
GrantBirki Mar 1, 2025
7ed4fc1
init using_github_apps.md docs
GrantBirki Mar 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -190,7 +190,7 @@ As seen above, we have a single example step. Perhaps you would actually use a r
| `success_reaction` | `true` | `+1` | The reaction to add to the comment that triggered the Action if its execution was successful |
| `failure_reaction` | `true` | `-1` | The reaction to add to the comment that triggered the Action if its execution failed |
| `allowed_contexts` | `true` | `pull_request` | A comma separated list of comment contexts that are allowed to trigger this IssueOps command. Pull requests and issues are the only currently supported contexts. To allow IssueOps commands to be invoked from both PRs and issues, set this option to the following: `"pull_request,issue"`. By default, the only place this Action will allow IssueOps commands from is pull requests |
| `permissions` | `true` | `"write,admin"` | The allowed GitHub permissions an actor can have to invoke IssueOps commands |
| `permissions` | `true` | `"write,admin"` | The allowed GitHub permissions an actor can have to invoke IssueOps commands. Note that permission check for GitHub App ignores this and instead expects "issues" permission set to "write". |
| `allow_drafts` | `true` | `"false"` | Whether or not to allow this IssueOps command to be run on draft pull requests |
| `allow_forks` | `true` | `"false"` | Whether or not to allow this IssueOps command to be run on forked pull requests |
| `skip_ci` | `true` | `"false"` | Whether or not to require passing CI checks before this IssueOps command can be run |
Expand All@@ -200,6 +200,7 @@ As seen above, we have a single example step. Perhaps you would actually use a r
| `allowlist_pat` | `false` | `"false"` | A GitHub personal access token with "read:org" scopes. This is only needed if you are using the "allowlist" option with a GitHub org team. For example: `"my-org/my-team"` |
| `skip_completing` | `true` | `"false"` | If set to `"true"`, skip the process of completing the Action. This is useful if you want to customize the way this Action completes - For example, custom reactions, comments, etc |
| `fork_review_bypass` | `true` | `"false"` | If set to "true", allow forks to bypass the review requirement if the operation is being made on a pull request from a fork. This option is potentially dangerous if you are checking out code in your workflow as a result of invoking this Action. If the code you are checking out has not been reviewed, then you might open yourself up to a TOCTOU vulnerability. You should always ensure that the code you are checking out has been reviewed, and that you checkout an exact commit sha rather than a ref. |
| `allow_github_apps` | `true` | `"true"` | If set to `true`, allow GitHub Apps to interact with or trigger this Action view issue/pull_request comments. If you want to explicitly prevent GitHub Apps from invoking this Action, then set this to `false`. For more information, see the [GitHub App docs for this Action](./docs/using_github_apps.md). |

## Outputs 📤

Expand All@@ -209,7 +210,7 @@ As seen above, we have a single example step. Perhaps you would actually use a r
| `continue` | ⭐ The string "true" if the workflow should continue, otherwise empty - Use this to conditionally control if your workflow should proceed or not. This is a step 2/2 check. This is the output you will want to use to determine if your IssueOps flow should _continue_ after this Action completes |
| `comment_body` | The comment body |
| `actor` | The GitHub handle of the actor that invoked the IssueOps command |
| `params` | The raw parameters that were passed into the IssueOps command (see param_separator) - Further [documentation](docs/assets/parameters.md) |
| `params` | The raw parameters that were passed into the IssueOps command (see param_separator) - Further [documentation](docs/parameters.md) |
| `comment_id` | The comment id which triggered this action |
| `issue_number` | The issue number of the pull request (or issue) that was commented on |
| `initial_reaction_id` | The reaction id for the initial reaction on the trigger comment |
Expand All@@ -221,6 +222,7 @@ As seen above, we have a single example step. Perhaps you would actually use a r
| `sha` | The commit sha if being used in the context of a pull request |
| `ref` | The ref if being used in the context of a pull request |
| `base_ref` | The base ref that the pull request is merging into (if available and run in the context of a pull request) |
| `actor_type` | The type of user/actor that triggered the IssueOps command. Values can be `User` or `Bot` |

## Allowlist 👩‍🔬

Expand Down
127 changes: 127 additions & 0 deletions __tests__/functions/valid-permissions.test.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,27 +2,45 @@ import * as core from '@actions/core'
import {validPermissions} from '../../src/functions/valid-permissions'

const setOutputMock = jest.spyOn(core, 'setOutput')
const infoMock = jest.spyOn(core, 'info')

var octokit
var context
beforeEach(() => {
jest.clearAllMocks()
jest.spyOn(core, 'setOutput').mockImplementation(() => {})
jest.spyOn(core, 'info').mockImplementation(() => {})
process.env.INPUT_PERMISSIONS = 'write,admin'
process.env.INPUT_ALLOW_GITHUB_APPS = true

context = {
repo: {
owner: 'corp',
repo: 'test'
},
actor: 'monalisa'
}

octokit = {
rest: {
users: {
getByUsername: jest.fn().mockReturnValueOnce({
status: 200,
data: {
type: 'User'
}
})
},
repos: {
getCollaboratorPermissionLevel: jest.fn().mockReturnValueOnce({
status: 200,
data: {
permission: 'write'
}
})
},
apps: {
getRepoInstallation: jest.fn()
}
}
}
Expand All@@ -31,6 +49,9 @@ beforeEach(() => {
test('determines that a user has valid permissions to invoke the Action', async () => {
expect(await validPermissions(octokit, context)).toEqual(true)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa')
expect(infoMock).toHaveBeenCalledWith(
`🔍 Detected actor type: User (${context.actor})`
)
})

test('determines that a user has does not valid permissions to invoke the Action', async () => {
Expand All@@ -47,6 +68,18 @@ test('determines that a user has does not valid permissions to invoke the Action
'👋 __monalisa__, seems as if you have not write/admin permissions in this repo, permissions: read'
)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa')
expect(setOutputMock).toHaveBeenCalledWith('actor_type', 'User')
})

test('fails to get actor information', async () => {
octokit.rest.users.getByUsername = jest.fn().mockReturnValue({
status: 500
})

expect(await validPermissions(octokit, context)).toEqual(
'Fetch user details returns non-200 status: 500'
)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa')
})

test('fails to get actor permissions due to a bad status code', async () => {
Expand All@@ -61,3 +94,97 @@ test('fails to get actor permissions due to a bad status code', async () => {
)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa')
})

test('determines that a GitHub App has valid permissions', async () => {
context.actor = 'github-actions[bot]'

octokit.rest.users.getByUsername = jest.fn().mockReturnValueOnce({
status: 200,
data: {
type: 'Bot'
}
})

octokit.rest.apps.getRepoInstallation = jest.fn().mockReturnValueOnce({
status: 200,
data: {
permissions: {
issues: 'write'
}
}
})

expect(await validPermissions(octokit, context)).toEqual(true)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'github-actions[bot]')
expect(setOutputMock).toHaveBeenCalledWith('actor_type', 'Bot')
expect(infoMock).toHaveBeenCalledWith(
`🔍 Detected actor type: Bot (${context.actor})`
)
})

test('determines that a GitHub App does not have valid permissions', async () => {
context.actor = 'monalisa[bot]'

octokit.rest.users.getByUsername = jest.fn().mockReturnValueOnce({
status: 200,
data: {
type: 'Bot'
}
})

octokit.rest.apps.getRepoInstallation = jest.fn().mockReturnValueOnce({
status: 200,
data: {
permissions: {
issues: 'read'
}
}
})

expect(await validPermissions(octokit, context)).toEqual(
'👋 __monalisa[bot]__ does not have "issues" permission set to "write". Current permissions: {"issues":"read"}'
)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa[bot]')
expect(setOutputMock).toHaveBeenCalledWith('actor_type', 'Bot')
})

test('fails since GitHub Apps are configured to be rejected', async () => {
process.env.INPUT_ALLOW_GITHUB_APPS = false
context.actor = 'monalisa[bot]'

octokit.rest.users.getByUsername = jest.fn().mockReturnValueOnce({
status: 200,
data: {
type: 'Bot'
}
})

expect(await validPermissions(octokit, context)).toEqual(
'GitHub Apps are not allowed to use this Action based on the "allow_github_apps" input.'
)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa[bot]')
expect(setOutputMock).toHaveBeenCalledWith('actor_type', 'Bot')
expect(infoMock).toHaveBeenCalledWith(
`🔍 Detected actor type: Bot (${context.actor})`
)
})

test('fails to fetch installation details for GitHub App', async () => {
context.actor = 'monalisa[bot]'

octokit.rest.users.getByUsername = jest.fn().mockReturnValueOnce({
status: 200,
data: {
type: 'Bot'
}
})

octokit.rest.apps.getRepoInstallation = jest.fn().mockReturnValueOnce({
status: 500
})

expect(await validPermissions(octokit, context)).toEqual(
'Failed to fetch GitHub App installation details: Status 500'
)
expect(setOutputMock).toHaveBeenCalledWith('actor', 'monalisa[bot]')
})
14 changes: 14 additions & 0 deletions __tests__/schemas/action.schema.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -197,6 +197,16 @@ inputs:
default:
type: string
required: true
allow_github_apps:
description:
type: string
required: true
required:
type: boolean
required: true
default:
type: string
required: true

# outputs section
outputs:
Expand DownExpand Up@@ -264,3 +274,7 @@ outputs:
description:
type: string
required: true
actor_type:
description:
type: string
required: true
10 changes: 8 additions & 2 deletions action.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,7 +33,7 @@ inputs:
required: true
default: "pull_request"
permissions:
description: 'The allowed GitHub permissions an actor can have to invoke IssueOps commands - Example: "write,admin"'
description: 'The allowed GitHub permissions an actor can have to invoke IssueOps commands - Example: "write,admin". Permission check for GitHub App ignores this and instead expects "issues" permission set to "write".'
required: true
default: "write,admin"
allow_drafts:
Expand All@@ -57,7 +57,7 @@ inputs:
required: true
default: "|"
allowlist:
description: 'A comma separated list of GitHub usernames or teams that should be allowed to use the IssueOps commands configured in this Action. If unset, then all users meeting the "permissions" requirement will be able to run commands. Example: "monalisa,octocat,my-org/my-team"'
description: 'A comma separated list of GitHub usernames or teams that should be allowed to use the IssueOps commands configured in this Action. If unset, then all users meeting the "permissions" requirement will be able to run commands. Example: "monalisa,monalisa[bot],octocat,my-org/my-team"'
required: false
default: "false"
allowlist_pat:
Expand All@@ -72,6 +72,10 @@ inputs:
description: 'If set to "true", allow forks to bypass the review requirement if the operation is being made on a pull request from a fork. This option is potentially dangerous if you are checking out code in your workflow as a result of invoking this Action. If the code you are checking out has not been reviewed, then you might open yourself up to a TOCTOU vulnerability. You should always ensure that the code you are checking out has been reviewed, and that you checkout an exact commit sha rather than a ref.'
required: true
default: "false"
allow_github_apps:
description: 'If set to "true", allow GitHub Apps to interact with or trigger this Action view issue/pull_request comments. If you want to explicitly prevent GitHub Apps from invoking this Action, then set this to "false".'
required: true
default: "true"
outputs:
triggered:
description: 'The string "true" if the trigger was found, otherwise the string "false" - Just because the workflow was triggered does not mean it should continue. This is a step 1/2 check'
Expand DownExpand Up@@ -105,6 +109,8 @@ outputs:
description: 'The ref if being used in the context of a pull request'
base_ref:
description: The base ref that the pull request is merging into (if available and run in the context of a pull request)
actor_type:
description: 'The type of user/actor that triggered the IssueOps command. Values can be "User" or "Bot"'
runs:
using: "node20"
main: "dist/index.js"
Expand Down
42 changes: 42 additions & 0 deletions dist/index.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map
View file
Open in desktop

Large diffs are not rendered by default.

0 docs/assets/parameters.md → docs/parameters.md
View file
Open in desktop
File renamed without changes.
5 changes: 5 additions & 0 deletions docs/using_github_apps.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
# Using GitHub Apps

> See the pull request that originally implemented these changes here: [#70](https://github.com/github/command/pull/70)

TODO: @jcfr please fill out some brief docs here. Thank you!
42 changes: 42 additions & 0 deletions src/functions/valid-permissions.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,8 +11,50 @@ export async function validPermissions(octokit, context) {
core.getInput('permissions', {required: true})
)

const allowGitHubApps = core.getBooleanInput('allow_github_apps', {
required: true
})

core.setOutput('actor', context.actor)

// Get Actor Type from GitHub API
const userRes = await octokit.rest.users.getByUsername({
username: context.actor
})

if (userRes.status !== 200) {
return `Fetch user details returns non-200 status: ${userRes.status}`
}

const actorType = userRes.data.type // "User" or "Bot"
core.info(`🔍 Detected actor type: ${actorType} (${context.actor})`)
core.setOutput('actor_type', actorType)

// Handle GitHub Apps (Bots)
if (actorType === 'Bot') {
if (!allowGitHubApps) {
return `GitHub Apps are not allowed to use this Action based on the "allow_github_apps" input.`
}

// Fetch installation details for the GitHub App
const installationRes = await octokit.rest.apps.getRepoInstallation({

@GrantBirkiGrantBirkiMar 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcfr I think that perhaps we are going about this the wrong way. The octokit.rest.apps.getRepoInstallation() method expects to be called from an authenticated GitHub App client. This isn't really what you likely want as then you have to pass in the same credentials of the GitHub App into the github/command Action.

That would be unnecessary as you are only trying to check to see if the GitHub App has proper permissions to interact with this repo in the same way that the Action checks if Users have the correct permissions.

I should have caught this earlier as a different approach will need to be taken.

I feel like there certainly has to be a way to check the permissions of a GitHub App on a repo without having to authenticate as the GitHub App itself but I don't know off the top of my head 🤔.

@GrantBirkiGrantBirkiMar 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing a very brief search I don't see anything jumping out to me right away to see if it is possible to figure out if a GitHub App has "proper permissions" to interact with a repository in this sense.

Note: I did not do an extensive search

If it turns out that there isn't a solid way to check the permissions of a GitHub App without directly authenticating as it, you might have to get creative here.

Some potential ideas could be a new input called: allowed_github_apps: monalisa[bot],someother[bot]. Then users would simply configure an explicit allowlist of GitHub Apps that they trust and be able to invoke workflows using the github/command Action.

Just a thought.. might have to go low tech to get this to work.

@jcfrjcfrMar 2, 2025•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up and for looking into ways of addressing this 🙏

I feel like there certainly has to be a way to check the permissions of a GitHub App on a repo without having to authenticate as the GitHub App itself

Ditto. That would be ideal.

If it turns out that there isn't a solid way to check the permissions of a GitHub App without directly authenticating as it, you might have to get creative here.
Some potential ideas could be a new input called: allowed_github_apps:monalisa[bot],someother[bot]. Then users would simply configure an explicit allowlist of GitHub Apps that they trust and be able to invoke workflows using the github/command Action.

That seems reasonable.

And this could be done by either explicitly passing my-awesome[bot] or by using the app-slug output of the actions/create-github-app-token action.

 - uses: actions/create-github-app-token@0d564482f06ca65fa9e77e2510873638c82206f2 # v1.11.5id: app-tokenwith:
app-id: ${{ vars.MY_AWESOME_WORKFLOW_APP_ID }}private-key: ${{ secrets.MY_AWESOME_WORKFLOW_APP_PRIVATE_KEY }}
- name: awesome commandid: awesome_commanduses: github/command@vX.Y.Zwith:
command: "/awesome"reaction: "rocket"allowed_contexts: "issue"permissions: "read,triage,write,maintain,admin"allowlist:
"${{ steps.app-token.outputs.app-slug }}[bot]"

@GrantBirkiGrantBirkiMar 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think explicitly passing my-awesome[bot] would be the best way to do so as not everyone would be using the app slug in combination with the actions/create-github-app-token job.

This seems like the best way forward to me.

@jcfrjcfrMar 3, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think explicitly passing my-awesome[bot] [...] best way

I started updating the PR this weekend and I will try to get that finalized asap.

Thanks for the patience :pray

...context.repo
})

if (installationRes.status !== 200) {
return `Failed to fetch GitHub App installation details: Status ${installationRes.status}`
}

const appPermissions = installationRes.data.permissions || {}

// Ensure the bot has "issues" permission set to "write"
if (appPermissions.issues !== 'write') {
return `👋 __${context.actor}__ does not have "issues" permission set to "write". Current permissions: ${JSON.stringify(appPermissions)}`
}

return true
}

// Get the permissions of the user who made the comment
const permissionRes = await octokit.rest.repos.getCollaboratorPermissionLevel(
{
Expand Down