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

Added noecho on secret parameters #20

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Smiddie31
Copy link

Description of changes:
Added 'noecho' to the teams, slack and chime webhook parameters masking them from the console.
At present these parameters are saved as secretmanager secrets, however the parameter values are visible in the cloudformation console.

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

@rarylson
Copy link

My personal thoughts on this:

  • Webhooks are secrets. So the NoEcho option totally makes sense here.
  • However, the project also supports deploy via Terraform. In this case, different form CloudFormation, options are versioned as well (terraform.tfvars), which includes params like SlackWebhookURL
    • So, while the Pull Request solves the problem for CloudFormation, we still have the problem for Terraform
    • I do not have any expertise on Terraform. I'm just curious about what should be the best approach.
      • Maybe the Terraform deploy should not create the secrets in Secrets Manager, but instead instruct the user to manually create them, and pass the secrets via a param like SecretsManagerName in terraform.tfvars

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