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

[Github Action] Check all secrets are replaced by bin/generate-secrets #275

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

pvannierop
Copy link
Contributor

@pvannierop pvannierop commented Jun 14, 2024

When new secret placeholders are defined in base-secrets.yaml the bin/generate-secrets script (that should replace secret placeholders with randomized values) is sometimes not updated accordingly. This will result in the use of the word secret as a weak password.

This PR will implement a check on the occurrence of the word secret in the secrets.yaml file left after running the bin/generate-secrets script. If detect this will cause the action to fail with a message on which secrets to update.

This PR also updates the generate-secrets script to replace any field with value secret with a password.

Example output:

Not all 'secret' fields were replaced by bin/generate-secrets script. Please make sure to cover the following fields with a 'insert_secret' entry: 
management_portal.smtp.password: secret
fitbit_api_client: "secret"
fitbit_api_secret: "secret"
oura_api_client: "secret"
oura_api_secret: "secret"
radar_push_endpoint.garmin.consumerKey: "secret"
radar_push_endpoint.garmin.consumerSecret: "secret"
velero.backup.accessKey: secret
velero.backup.secretKey: secret

@pvannierop pvannierop changed the base branch from main to dev June 14, 2024 13:12
@pvannierop pvannierop force-pushed the check-secrets-action branch 14 times, most recently from e44aad2 to ed26db2 Compare June 14, 2024 13:47
@pvannierop pvannierop self-assigned this Jun 14, 2024
@keyvaann
Copy link
Collaborator

Thanks for the PR!
Since these secrets have to be provided via the user it will be a bit confusing if they're generated automatically. I suggest changing their default value to something like change_me and not generating a default value for them.

@pvannierop pvannierop force-pushed the check-secrets-action branch from ed26db2 to 398f7fa Compare June 18, 2024 07:45
@pvannierop
Copy link
Contributor Author

@keyvaann I was just discussing this with @Bdegraaf1234 yesterday. I have updated the PR with the change_me pattern. The commit that added replacement of these external secrets to the generate-secrets script was dropped.

On a side note. Would it not be safer to change the generate-secrets script to change any field that it recognizes with value secret?

@keyvaann
Copy link
Collaborator

@keyvaann I was just discussing this with @Bdegraaf1234 yesterday. I have updated the PR with the change_me pattern. The commit that added replacement of these external secrets to the generate-secrets script was dropped.

On a side note. Would it not be safer to change the generate-secrets script to change any field that it recognizes with value secret?

Thanks! Yes if we can add the secrets automatically it would be even better.

@pvannierop
Copy link
Contributor Author

@keyvaann The generate-secrets script was updated to automatically replace all remaining secret fields.

Copy link
Collaborator

@keyvaann keyvaann left a comment

Choose a reason for hiding this comment

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

Thanks!
LGTM

@keyvaann keyvaann merged commit 1cd2334 into dev Jun 21, 2024
7 checks passed
@keyvaann keyvaann deleted the check-secrets-action branch June 21, 2024 09:45
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.

2 participants