-
Notifications
You must be signed in to change notification settings - Fork 42
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 debug authorization header flag #754
base: main
Are you sure you want to change the base?
Conversation
httpclient/api_client.go
Outdated
@@ -257,8 +258,8 @@ func (c *ApiClient) recordRequestLog( | |||
RequestBody: requestBody, | |||
ResponseBody: responseBody, | |||
DebugHeaders: c.config.DebugHeaders, | |||
DebugAuthorizationHeader: c.config.DebugAuthorizationHeader, |
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.
you're missing the X-Databricks-Azure-SP-Management-Token
and X-Databricks-GCP-SA-Access-Token
headers in https://github.com/databricks/databricks-sdk-go/blob/main/logger/httplog/round_trip_stringer.go#L33-L35
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.
I think you reviewed a stale version but I added this in the last commit
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #754 +/- ##
==========================================
+ Coverage 17.78% 17.79% +0.01%
==========================================
Files 106 106
Lines 14199 14201 +2
==========================================
+ Hits 2525 2527 +2
Misses 11463 11463
Partials 211 211 ☔ View full report in Codecov by Sentry. |
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.
Since by default we prevent printing of token, I think this makes user experience more safe. LGTM
Changes
DebugHeaders by default prints all headers, including the sensitive Authorization header. To allow users to print debug logs with headers but without including this sensitive header, we introduce a new flag, DebugAuthorizationHeader, which is disabled by default for users. This way, enabling debug headers does not automatically leak the token used to authenticate to our REST API.
Tests
make test
passingmake fmt
applied