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

Mask sensitive values logged through spans #3290

Merged
merged 2 commits into from
Dec 28, 2024

Conversation

kichristensen
Copy link
Contributor

@kichristensen kichristensen commented Dec 15, 2024

What does this change

Ensure logs written using the tracer won't contain sensitive information.

What issue does it fix

closes #3216
closes #2256

Checklist

  • Did you write tests?
  • Did you write documentation?
  • Did you change porter.yaml or a storage document record? Update the corresponding schema file.
  • If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

@kichristensen kichristensen force-pushed the tracerLogSensitiveValues branch from 1befbac to 776ee5c Compare December 15, 2024 22:12
@kichristensen kichristensen changed the title feat: Mask sensitive values logged through spans Mask sensitive values logged through spans Dec 15, 2024
@kichristensen kichristensen marked this pull request as ready for review December 16, 2024 07:02
Copy link
Member

@schristoff schristoff left a comment

Choose a reason for hiding this comment

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

One question, overall looks great. If tests are happy, so am I ;)

func (cw *CensoredWriter) Censor(b []byte) []byte {
for _, val := range cw.sensitiveValues {
if strings.TrimSpace(val) != "" {
b = bytes.Replace(b, []byte(val), []byte("*******"), -1)
Copy link
Member

Choose a reason for hiding this comment

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

qq: Should we replace []byte("*******"), with just []byte("**"),, assuming we have a very long sensitive value this can get kind of large, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@schristoff The length of the sensitive value should not matter, as it will always replace the value with *******. This will of course make the log longer if the sensitive value is very short, but otherwise the lenght should not matter

@kichristensen kichristensen merged commit a97711e into getporter:main Dec 28, 2024
40 checks passed
@kichristensen kichristensen deleted the tracerLogSensitiveValues branch December 28, 2024 22:42
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.

Sensitive variables printed when exec fails Prevent sensitive data being written to logs/traces
2 participants