Strip custom credential headers on cross-host redirects - #1763
Open
winstoncrooker wants to merge 1 commit into
Open
Strip custom credential headers on cross-host redirects#1763winstoncrooker wants to merge 1 commit into
winstoncrooker wants to merge 1 commit into
Conversation
The main HTTP client follows redirects but net/http only strips the standard sensitive headers (Authorization, Cookie) when a redirect changes host. The custom cloud-provider token headers (X-Databricks-Azure-SP-Management-Token, X-Databricks-GCP-SA-Access-Token) were therefore forwarded to a different host on a cross-host redirect. Add a CheckRedirect to the main client that drops these headers when a redirect changes host, matching how net/http handles Authorization and how the Azure tenant-fetch client already avoids following redirects. Signed-off-by: winstoncrooker <winstoncrooker@outlook.com>
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When Azure service-principal or GCP service-account auth is configured, the SDK attaches the cloud-provider token to every request as a custom header (
X-Databricks-Azure-SP-Management-Token, the Azure ARM management bearer;X-Databricks-GCP-SA-Access-Token, the GCP service-account token). These headers are already treated as secrets by the SDK's own log redaction.The main HTTP client in
httpclient/api_client.gofollows redirects with noCheckRedirect. Go'snet/httpstrips only the standard sensitive headers (Authorization,Cookie,WWW-Authenticate) when a redirect changes host; it does not strip these custom headers. So on a cross-host redirect the cloud tokens are forwarded to the redirect target host, even thoughAuthorizationwould be stripped.The SDK already avoids this on the Azure tenant-fetch client (
config/config.gosets aCheckRedirect); this change brings the main request client in line.Change
Add a
CheckRedirectto the main client that drops the custom credential headers when a redirect changes host, preserves the default 10-redirect limit, and leaves same-host redirects untouched.Testing
make fmt test lint. Verified at runtime that with the change the tokens are stripped on a cross-host redirect (matchingAuthorization) and same-host redirects are unaffected.