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

"mrsk config" exposes env secrets #96

Closed
kjellberg opened this issue Mar 9, 2023 · 4 comments · Fixed by #182
Closed

"mrsk config" exposes env secrets #96

kjellberg opened this issue Mar 9, 2023 · 4 comments · Fixed by #182

Comments

@kjellberg
Copy link
Contributor

kjellberg commented Mar 9, 2023

There will be cases when a user is asked to copy the output of mrsk config for debugging. Currently it's a bad idea since all secrets are exposed in env_args, which also makes the following statement in README false:

Marking an ENV as secret currently only redacts its value in the output for MRSK.

@kjellberg kjellberg changed the title "mrsk-dev config" exposes env secrets "mrsk config" exposes env secrets Mar 9, 2023
@dhh
Copy link
Member

dhh commented Mar 13, 2023

Happy to see redact or similar applies to mrsk config, but the keys should be listed there.

@northeastprince
Copy link
Contributor

So something like DATABASE_URL and other credentials are meant to be listed?

@dhh
Copy link
Member

dhh commented Mar 23, 2023

Yes, all env inputs are meant to be listed. But we can add redacting.

@jeremy
Copy link
Member

jeremy commented Apr 5, 2023

Fixed in #182

@dhh dhh closed this as completed in #182 Apr 6, 2023
ncreuschling pushed a commit to ncreuschling/mrsk that referenced this issue Apr 12, 2023
* `-e [REDACTED]` → `-e SOME_SECRET=[REDACTED]`
* Replaces `Utils.redact` with `Utils.sensitive` to clarify that we're
  indicating redactability, not actually performing redaction.
* Redacts from YAML output, including `mrsk config` (fixes basecamp#96)
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 a pull request may close this issue.

4 participants