Security: pass secrets with `--from-file` instead of over the command line
brcrista opened this issue · 5 comments
I'm following up on a recent incident where a user thought they had a secret leak when running this action.
Passing secrets over the command line is insecure as it can accidentally cause the secret to leak. See https://help.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets#using-encrypted-secrets-in-a-workflow for details.
For kubectl create secret generic
, a better option would be to use --from-file
instead of --from-literal
. It would work like this:
- Instead of taking an arbitrary
arguments
string, take a set of key-value pairs like this:
with:
secrets: |
username=${{ secrets.USERNAME }}
password=${{ secrets.PASSWORD }}
storage-account-key=${{ secrets.STORAGE_ACCOUNT_KEY }}
- In the JavaScript for your action, parse these key-value pairs and write them to files as
kubectl create secret generic
expects (username.txt
,password.txt
,storage-account-key
etc.). - Build up the
kubectl
command line to run with the--from-file
arguments.
This will protect users from accidentally exposing their secrets by running this action.
If you're supplying the values for the secret in ${{ secrets.* }}
format, aren't those secrets masked from logs by default?
Yes, but secrets can leak in other ways. First is that it's exposed through the command line for the process. For a self-hosted runner, that means that other processes on the machine can see the secrets there.
Second, there's the possibility of a partial secret leak. If there's a parsing issue over the command line, a string like helloworl"d
might get turned into helloworl
. This substring wouldn't get masked from the logs. But, a user might see that and think that their secret had leaked.
@rgsubh Could you take up the fix for this?
Fixed and released in v.1.1.
Currently, the readme has the below syntax for creating generic secrets:
- uses: azure/k8s-create-secret@v1
with:
namespace: 'default'
secret-type: 'generic'
arguments: --from-literal=account-name=${{ secrets.AZURE_STORAGE_ACCOUNT }} --from-literal=access-key=${{ secrets.AZURE_STORAGE_ACCESS_KEY }}
secret-name: azure-storage
Am happy to update the README with changes from this PR, but would like to know if the below syntax for generic secrets is correct?
with:
namespace: 'default'
secret-type: 'generic'
secret-name: azure-storage
secrets: |
account-name=${{ secrets.AZURE_STORAGE_ACCOUNT }}
access-key=${{ secrets.AZURE_STORAGE_ACCESS_KEY }}