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

feat: provide input to optionally mask output docker password #491

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

therealdwright
Copy link
Contributor

@therealdwright therealdwright commented Aug 3, 2023

This will allow the user to specify an optional input to mask the Docker password from being leaked in workflow logs via either explicitly printing or running a job in debug mode. Since many users rely on the output, the default behaviour has been false to ensure this is not a breaking change.

This also re-arranges the inputs throughout the code and in the GH action spec to be in alphabetical order as a styling improvement.

Issue #

Closes #485

Description of changes:

  • Add new input 'mask-password', which will prevent the password from being logged to the console
  • Add a test to cover mask-password case and update existing tests
  • Re-order inputs in alphabetical order to improve styling

@therealdwright therealdwright force-pushed the correctly-mask-secrets branch 8 times, most recently from f01522b to 0020a24 Compare August 3, 2023 23:23
This will allow the user to specify an optional input to mask
the Docker password from being leaked in workflow logs via either
explicitly printing or running a job in debug mode. Since many
users rely on the output, the default behaviour has been false to
ensure this is not a breaking change.

This also re-arranges the inputs throughout the code and in the GH
action spec to be in alphabetical order as a styling improvement.

Signed-off-by: [email protected] <[email protected]>
@therealdwright
Copy link
Contributor Author

As a general comment, I would prefer my change default to be true for the masking of the password but acknowledge this will be a breaking change.

Copy link

@michaelb990 michaelb990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @therealdwright! A couple small wording tweaks, otherwise this looks good to me.

#### Login to Amazon ECR Private, then build and push a Docker image masking the password:

> [!WARNING]
> Setting mask-password to true will prevent the password GitHub output from being shared between separate jobs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> Setting mask-password to true will prevent the password GitHub output from being shared between separate jobs.
> Setting mask-password to true will prevent the password used to login to ECR from being shared between jobs.

Comment on lines +13 to +14
Mask the docker password to prevent it being printed to logs to std-out. This will prevent the
password GitHub output from being shared between separate jobs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Mask the docker password to prevent it being printed to logs to std-out. This will prevent the
password GitHub output from being shared between separate jobs.
Mask the docker password to prevent it from appearing in debug logs. This will prevent the
password used to login to Amazon ECR from being shared between jobs.

@mergify mergify bot merged commit 98f33d2 into aws-actions:main Aug 8, 2023
7 checks passed
@michaelb990
Copy link

Shoot, I pressed the wrong button. We'll tweak these in a follow-up PR and get a release out shortly.

therealdwright added a commit to therealdwright/amazon-ecr-login that referenced this pull request Aug 8, 2023
This commit addresses PR feedback left in aws-actions#491. There are no
functional changes in this commit, simply minor wording changes to
the readme.md and action.yaml

Signed-off-by: [email protected] <[email protected]>
@therealdwright therealdwright deleted the correctly-mask-secrets branch August 8, 2023 22:25
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.

Docker Token Leaked in Debug Logs
2 participants