-
Notifications
You must be signed in to change notification settings - Fork 383
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
Use custom marshaller to clarify redactions #4506
Conversation
Signed-off-by: Evan Anderson <[email protected]>
Note: this incorporates #4505 , as that changed the format of |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4506 +/- ##
==========================================
- Coverage 65.66% 65.60% -0.07%
==========================================
Files 211 211
Lines 31689 31691 +2
==========================================
- Hits 20810 20791 -19
- Misses 9677 9696 +19
- Partials 1202 1204 +2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Evan Anderson <[email protected]>
d7f9d6a
to
76cc4ab
Compare
Hold off on reviewing this; it looks like I missed |
Signed-off-by: Evan Anderson <[email protected]>
Signed-off-by: Evan Anderson <[email protected]>
I think I've fixed the errors from |
Signed-off-by: Evan Anderson <[email protected]>
Signed-off-by: Evan Anderson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cc @shawnh2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the late reply
LGTM thanks !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What type of PR is this?
fix: Make redacted fields show as
"[redacted]"
rather than"W3JlZGFjdGVkXQ=="
What this PR does / why we need it:
I started preparing to report a security issue with Envoy Gateway because I noticed that our logs contained a
"privateKey"
with a base64-encoded value. Eventually, while trying to get the key to line up with the certificate, I base-64 decoded the contents, and saw that the value was[redacted]
. I'm hoping this avoids future users getting freaked out like I did.As a maybe-beneficial side-effect, it shouldn't be necessary to call
Printable()
to avoid leaking XDS secret information when marshalling to/from JSON or YAML.Which issue(s) this PR fixes:
I didn't end up filing a bug yet.
Release Notes: No