Skip to content

Add Secret(s) Manager security risk warning messages #216

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

Closed

Conversation

roger2hk
Copy link
Contributor

Towards #109, #110.

@roger2hk roger2hk added the documentation Improvements or additions to documentation label Mar 26, 2025
@roger2hk roger2hk requested a review from phbnf March 26, 2025 15:00
@phbnf
Copy link
Collaborator

phbnf commented Mar 26, 2025

This message isn't explicit about what's actually happening. I think it's important to be explicit to prevent someone from copy-pasting this config to bring up their production log. Here's an alternative suggestion:

# [WARNING]
# This module will hardcode unencrypted private keys in the Terraform state file.
# DO NOT use this for production logs.

Let's also put this "WARNING" banner in:

  • the secretmanager modules main.tf files
  • a README.md file in the secretmanager directories?

I really really want to make sure nobody uses this for a production log, hence the request for extra warning messages. I've created #219 to track implementation of a solution for production logs.

@roger2hk
Copy link
Contributor Author

I have proposed to move the insecure part out of the secret manager(s) modules. See #219 (comment). After that, we can put those warning messages in that insecure module instead of secret(s) manager modules which are supposed to be safe to use.

@phbnf
Copy link
Collaborator

phbnf commented Mar 26, 2025

Let's add all the messages for now since that's the state of the world, and remove them once we've designed a better process to bring up new logs safely?
I'm not sure we'll get to do this before the Alpha release. If we do, that's great, and we can remove them. If we don't, I'd rather err on the safe side for now.

@roger2hk
Copy link
Contributor Author

Ah, I've already got that done for AWS in #220 when I saw this message.

@roger2hk
Copy link
Contributor Author

Closing this as the warning messages will be covered in #220 and #222.

@roger2hk roger2hk closed this Mar 31, 2025
@roger2hk roger2hk deleted the doc-security-warning branch March 31, 2025 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants