Skip to content
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.

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

fix: set role credentials as secrets to mask them in logs #19

Merged
merged 3 commits into from Jan 28, 2020
Merged

fix: set role credentials as secrets to mask them in logs #19

merged 3 commits into from Jan 28, 2020

Conversation

LaurenceGA
Copy link
Contributor

It's super cool that this action now supports assuming a role (#17), it prompted me to start using it instead of a custom solution.
One thing I think was overlooked is that now if you assume a role, the temporary role credentials will be in plain text in Github Actions logs. Even if they're short-lived, they're still normally powerful and I think it's better if they're masked.

This PR marks the AWS credentials from an assumed role as secrets (access key id, secret access key and session token), which will mask them in the logs. This isn't an issue using AWS credentials straight from Github Actions secrets because of course they are already marked as secrets and hence are already masked.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@LaurenceGA LaurenceGA changed the title Set role credentials as secrets to mask them in logs fix: set role credentials as secrets to mask them in logs Jan 27, 2020
@mattsb42-aws
Copy link
Member

Yup, this was a miss on my part. I hadn't dug into what exactly marking an environment variable as "secret" did.

This isn't an issue using AWS credentials straight from Github Actions secrets because of course they are already marked as secrets and hence are already masked.

Would it hurt anything to mark them secret anyway? My preference would be to just always mark them as secret, regardless of whether they are already. That way a) we don't need to rely on externally defined behavior, and b) we don't need to rely on the reader to know that external behavior exists. It would also simplify the code and reduce branching. :)

@clareliguori
Copy link
Member

Thanks for this contribution!

I agree with @mattsb42-aws that it would be valuable to always mask credentials. Getting the original creds as secrets is more of a convention; they could actually come from anywhere (like from an encrypted file in the repo, or as an output from another action) and we would always want them to be masked.

@LaurenceGA
Copy link
Contributor Author

Yeah true both good points. I don't think it would do any harm. I've updated it.

@clareliguori clareliguori merged commit e2fd53a into aws-actions:master Jan 28, 2020
@LaurenceGA LaurenceGA deleted the mask-role-credentials branch January 29, 2020 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants