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

Make mask-password the Default Behaviour #495

Open
therealdwright opened this issue Aug 9, 2023 · 3 comments
Open

Make mask-password the Default Behaviour #495

therealdwright opened this issue Aug 9, 2023 · 3 comments
Labels
feature-request A feature should be added or improved.

Comments

@therealdwright
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When running the amazon-ecr-login action with mask-password set to false
credentials can be leaked to std-out via either explicitly printing them or running
the action in DEBUG mode. This is a security concern as it means that users are
able to obtain access to users (often privileged ones) that have access to push
to ECR registries, potentially implanting malicious code if the registry does not use
immutable tags.

Describe the solution you'd like
I would like to introduce a breaking change that switches mask-password to true
as it is the more secure option. I would like to go one step further and remove the
ability not to mask the password at all if the maintainers will allow it.

Describe alternatives you've considered
N/A - it is considered a security best practise to mask secrets from logs for obvious
reasons.

Additional context
Implementing this change is breaking as there are a number of users who depend on
the ability to share the docker password between jobs as evidenced by previous
attempts to implement this feature. Masking the password prevents this behaviour
so this change should be considered breaking.

@therealdwright therealdwright added the feature-request A feature should be added or improved. label Aug 9, 2023
@lounsbrough
Copy link

I 100% agree with this, masking the credentials should be default, and ideally not masking should not even be allowed. There should be better (more secure) ways to share credentials between jobs then outputting them from this job step.

@pascalgulikers
Copy link

Curious about a better solution to share credentials between jobs. Until then: showing that you can the find keys to my house in the garden is not really desirable. See: #496

@bplessis-swi
Copy link

Curious about a better solution to share credentials between jobs.

Yeah, especially thoses jobs running on a container fetched from ECR ...
Other composite jobs could login to ECR multiple time but when the container has to be fetched by GHA that's no fun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

4 participants