Research learnings and ideas from similar GitHub Actions
gr2m opened this issue ยท 10 comments
Follow up to #39 (comment).
In preparation for #3 and #4 it might make sense to learn from similar actions.
I looked through them again and two things that stood out for me
- should we consider two parameters in order to set the installation for the token: either the name of the owner login or an installation id? I think an owner login should suffice, but if the installation ID is known then we would save a request.
- Proxy settings: https://github.com/peter-murray/workflow-application-token-action#proxy
There is also a lot to learn in terms of documentation, in particular how to register a GitHub App.
I was trying to migrate from the action peter-murray/workflow-application-token-action to this one, and I noticed this action does not support the private-key input as a Base64 encoded string.
Does it make sense to create an issue for that? I don't have any problem opening a PR to support that use case.
Also, an issue we have with the action peter-murray/workflow-application-token-action that didn't have any internal retry mechanism; maybe we can improve that in this action.
I will be happy to start working in some PRs to improve this action
private-key input as a Base64 encoded string
what is the use case? Maybe the decoding a base64 string could be done in a separate step before using this action?
didn't have any internal retry mechanism
For retries, @octokit has the retry plugin, it's rather heavy though, due to its dependency on bottleneck
. We try to keep the code of this action, including its dependencies, to a minimum. It might make sense to implement a custom retry in this case particular case
So joining these two use cases, I have experienced failures getting the token from a GitHub app, so we start to use an extra action to retry Wandalen/wretry.action, then we have the issue with the input that is broken with multiline strings like the format of the PEM files, so we had to do encode the private key with base64.
Also, it is annoying to keep copying and pasting the ugly syntax of an action that retries another action.
So, if this action itself has the internal retry, I don't even need the base64 encoding.
So, does it make sense to implement a retry logic inside the action?
the input that is broken with multiline strings like the format of the PEM files, so we had to do encode the private key with base64
would replacing line breaks with "\n" work? That's what we do in environments where multi-line environment variables are not supported
if this action itself has the internal retry, I don't even need the base64 encoding
Perfect ๐๐ผ
I agree we should implement a retry, could you please file a separate issue for it?
I would like to try and help implement the proxy support. Getting this working from self-hosted runners behind a corporate proxy is important for my project.
I'm interested in changing the behaviour of the app to use GITHUB_API_URL as a default but allow for an override to deal with the use case of calling GHES from GHEC. This would also support GHES to GHES requests. This is useful for services like renovatebot/dependabot that need to lookup dependencies in private repos:
https://docs.github.com/en/code-security/dependabot/working-with-dependabot/configuring-access-to-private-registries-for-dependabot
https://docs.renovatebot.com/getting-started/private-packages/
@newbloke82 makes sense, thank you for laying it out! Can you please first file an issue? I think tibdex/github-app-token
is implementing proxy support the way you suggest, right? See https://github.com/tibdex/github-app-token/blob/3eb77c7243b85c65e84acfa93fdbac02fb6bd532/action.yml#L8-L10
private-key input as a Base64 encoded string
what is the use case? Maybe the decoding a base64 string could be done in a separate step before using this action?
In my case we migrate from an action expecting the private key to be Base64 encoded. One solution is to create a new secret with the plain private key, but i just tried to test how we could decode it on the fly.
It does work, but it's quite the overhead in my opinion, especially to ensure the masking is done and multiline storage is working fine:
- name: Decode private key
id: decoding
run: |
private_key=$(echo "${{ secrets.SEMANTIC_RELEASE_PRIVATE_KEY }}" | base64 -d) &> /dev/null
while read -r line;
do
echo "::add-mask::${line}"
done <<< "$private_key"
echo 'private-key<<EOF' >> "$GITHUB_OUTPUT"
echo "$private_key" >> "$GITHUB_OUTPUT"
echo 'EOF' >> "$GITHUB_OUTPUT"
- name: Generate GitHub App Token
id: generate-token
uses: actions/create-github-app-token@31c86eb3b33c9b601a1f60f98dcbfd1d70f379b4 # v1.10.3
with:
app-id: ${{ secrets.SEMANTIC_RELEASE_APP_ID }}
private-key: ${{ steps.decoding.outputs.private-key }}
The output is a bit ugly, but at least it's concealed:
Run actions/create-github-app-token@31c86eb3b33c9b601a1f60f98dcbfd1d70f379b4
with:
app-id: ***
private-key: ***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
github-api-url: https://api.github.com/
owner and repositories not set, creating token for the current repository ("my-repo")
would replacing line breaks with "\n" work? That's what we do in environments where multi-line environment variables are not supported
This works also, and is more clean than the previous one:
- name: Decode private key
id: decoding
run: |
private_key=$(echo "${{ secrets.SEMANTIC_RELEASE_PRIVATE_KEY }}" | base64 -d | awk 'BEGIN {ORS="\\n"} {print}' | head -c -2) &> /dev/null
echo "::add-mask::$private_key"
echo "private-key=$private_key" >> "$GITHUB_OUTPUT"
- name: Generate GitHub App Token
id: generate-token
uses: actions/create-github-app-token@31c86eb3b33c9b601a1f60f98dcbfd1d70f379b4 # v1.10.3
with:
app-id: ${{ secrets.SEMANTIC_RELEASE_APP_ID }}
private-key: ${{ steps.decoding.outputs.private-key }}
If the action is not planned to handle the private key being potentially encoded in Base64, I think we should add this in the readme, to avoid people leaking their private keys (I can open a PR)
If the action is not planned to handle the private key being potentially encoded in Base64, I think we should add this in the readme, to avoid people leaking their private keys (I can open a PR)
that's a good idea and thank you for offering. Let's add a note here: https://github.com/actions/create-github-app-token?tab=readme-ov-file#private-key. You could also link to your comment where as an example on how to decode a base64-encoded key
If this comes up more often we can reconsider to add built-in base64 decoding, but so far I've only heard it from you.
private-key input as a Base64 encoded string
what is the use case? Maybe the decoding a base64 string could be done in a separate step before using this action?
To advocate for supporting Base64 encoded private keys, our use case is using AWS SecretsManager to store GitHub app credentials.
SecretsManager supports plaintext secrets or JSON objects. To store the private key (with whitespace) as a JSON key value pair we need to encode it.
We expect a secret format of:
{
"app-id": 123456,
"private-key": "base64-encoded-private-key"
}
The aws-actions/aws-secretsmanager-get-secrets action supports parse-json-secrets: true
which can format environment variables based on the properties.
We then consume this in a GitHub Actions job like this:
- name: Retrieve AWS Secret
uses: aws-actions/aws-secretsmanager-get-secrets
with:
parse-json-secrets: true
secret-ids: |
GH_APP, ${{ inputs.app-credentials-secret-name }}
- name: Generate a token
id: generate-token
uses: tibdex/github-app-token
with:
app_id: ${{ env.GH_APP_APP_ID }}
private_key: ${{ env.GH_APP_PRIVATE_KEY }}
Having native support for Base64 encoded keys is an advantage as it avoids needing to insert decoding steps ourselves.
Implementing something like the above always feels risky as it's quite easy to break the masking if you're doing development against a GitHub Actions workflow. If the private key gets leaked, we need to manually rotate it in the GitHub UI and then repopulate the AWS secret.
For compatibility with actions/create-github-app-token
and aversion to implementing decoding ourselves, we've instead ended up storing the app's id and private key in separate secrets. See implementation below.
This feels wrong because
- the app ID isn't really a secret, we just want it parameterised away so that we don't have hard coded magic numbers in various workflows
- it feels wasteful (AWS charges per secret per month) to store these in two separate values when they refer to one conceptual thing
- the app id and private key have a similar lifecycle (e.g. they both get populated at creation), storing in the same secret makes sense
- name: Retrieve AWS Secret
uses: aws-actions/aws-secretsmanager-get-secrets@v2
with:
secret-ids: |
APP_ID, ${{ inputs.app-id-secret }}
PEM_FILE, ${{ inputs.app-private-key-secret }}
- name: Generate token
id: create-github-app-token
uses: actions/create-github-app-token@v1
with:
app-id: ${{ env.APP_ID }}
private-key: ${{ env.PEM_FILE }}
The tibdex
implementation is much neater for our use case as I'm sure you'd agree. On the other hand, it is preferable to use the official implementation from https://github.com/actions. I'd really appreciate if this was added as a feature to get the best of both worlds :)