Add permissions input#37
Conversation
| # permissions: >- | ||
| # {"members": "read"} |
There was a problem hiding this comment.
would it be possible/reasonable to have this as yaml which it then turned into json before sending it to the GH api? it would be more consistent with the representation we use in the workflow files
There was a problem hiding this comment.
@simoneb I may have missed something, but AFAICT the input values should always be a string.
The action.yml documentation is a bit vague, yet assumes the values can be converted to environment variables:
Optional Input parameters allow you to specify data that the action expects to use during runtime. GitHub stores input parameters as environment variables. Input ids with uppercase letters are converted to lowercase during runtime. We recommended using lowercase input ids.
Also, the default input value of the first example provided by this doc, despite being a number, is quoted as a string:
inputs:
numOctocats:
description: 'Number of Octocats'
required: false
default: '1'
octocatEyeColor:
description: 'Eye color of the Octocats'
required: true
Also, actions on the marketplace that need a more complex structure are often referring a file path (e.g to a JSON or YAML ) instead
|
@tibdex does this PR has a chance to get merged at some point (after your feedback and associated fixes, if any, of course )? |
src/index.ts
Outdated
| owner, | ||
| privateKey, | ||
| repo, | ||
| permissions: JSON.parse(permissionsJSON) |
There was a problem hiding this comment.
Since this input is not required, what happens if it is not specified?
There was a problem hiding this comment.
fair point, without tests it's hard to say but I suspect this may break. maybe the default value of this input should be something that parses to null or another sensible default
There was a problem hiding this comment.
Fair point:
permissionsJSON comes from getInput.
If the value is not provided, then permissionsJSON is the empty string according to the doc, and unfortunately JSON.parse('') throws. So it needs a small update.
There was a problem hiding this comment.
This case should now properly be handled :)
|
@tibdex we've had this PR open for some time now and we're using a fork of this action. Would you mind taking a look? |
|
Can you please allow me to edit your branch as explained in section |
It seems I can't allow this (I don't have the checkbox when editing the PR), because of some security setting maybe, as it could allow you to access our corporate secrets if I understand that part of the doc you mentioned correctly:
|
|
I believe the culprit is "On user-owned forks":
While this is a fork from an org instead. @tibdex feel free to just create a new branch in this repo so you have freedom to change anything. |
This PR allows to provide a
permissionsinput string containing a JSON object providing the permissions as described by https://docs.github.com/en/rest/apps/apps#create-an-installation-access-token-for-an-appI have tested it and it just works