-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add secrets sanitization helpers #6107
Conversation
Codecov Report
|
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.
This still would not protect against exposure in chained tracebacks, maybe worth doing as a more general filtering function, used by logger and in run()
?
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.
This is great - will be very helpful!
There are some other escape vectors though, a good use case can be seen in #6003. That case requires sanitizing the message prior to service_check
and an exception message.
One approach would be to have the sanitize filter as a method on the AgentCheck class, then checks could call it directly.
0cbc9fa
to
64a9840
Compare
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.
Looks great
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.
💯
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.
Diff is reasonably small, no need to break this up
Great job!
Thanks all! CI failures are unrelated:
Merging… |
What does this PR do?
Add helpers to sanitize any known secrets (i.e. those manipulated by an integration) from logs, service check messages, and exception tracebacks.
Motivation
Make it easier to ensure passwords aren't leaked in logs and UI messages.
Of limited use for credentials contained in URLs (as I believe the Agent already scrubs those?), but most useful for general logs that might leak secrets, whether those leaks are known:
integrations-core/sqlserver/datadog_checks/sqlserver/sqlserver.py
Lines 663 to 667 in 42ee1d3
or unknown/possible/unproven: #5715 (comment)
In general we should register any password/secret managed by an integration.
Additional Notes
Review checklist (to be filled by reviewers)
changelog/
andintegration/
labels attached