diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index cffb802d6..ee1d5ee71 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -4,9 +4,10 @@ ### New Features and Improvements -* Add native support for authentication through Azure DevOps OIDC +* Add native support for authentication through Azure DevOps OIDC. ### Bug Fixes +* Fix a security issue that resulted in bearer tokens being logged in exception messages. ### Documentation diff --git a/databricks/sdk/_base_client.py b/databricks/sdk/_base_client.py index 5af684cb6..33dbd17c9 100644 --- a/databricks/sdk/_base_client.py +++ b/databricks/sdk/_base_client.py @@ -99,7 +99,10 @@ def __init__( # Default to 60 seconds self._http_timeout_seconds = http_timeout_seconds or 60 - self._error_parser = _Parser(extra_error_customizers=extra_error_customizers) + self._error_parser = _Parser( + extra_error_customizers=extra_error_customizers, + debug_headers=debug_headers, + ) def _authenticate(self, r: requests.PreparedRequest) -> requests.PreparedRequest: if self._header_factory: diff --git a/databricks/sdk/errors/parser.py b/databricks/sdk/errors/parser.py index 64d83de05..2fefc4e2f 100644 --- a/databricks/sdk/errors/parser.py +++ b/databricks/sdk/errors/parser.py @@ -31,12 +31,15 @@ ] -def _unknown_error(response: requests.Response) -> str: +def _unknown_error(response: requests.Response, debug_headers: bool = False) -> str: """A standard error message that can be shown when an API response cannot be parsed. This error message includes a link to the issue tracker for the SDK for users to report the issue to us. + + :param response: The response object from the API request. + :param debug_headers: Whether to include headers in the request log. Defaults to False to defensively handle cases where request headers might contain sensitive data (e.g. tokens). """ - request_log = RoundTrip(response, debug_headers=True, debug_truncate_bytes=10 * 1024).generate() + request_log = RoundTrip(response, debug_headers=debug_headers, debug_truncate_bytes=10 * 1024).generate() return ( "This is likely a bug in the Databricks SDK for Python or the underlying " "API. Please report this issue with the following debugging information to the SDK issue tracker at " @@ -56,11 +59,13 @@ def __init__( self, extra_error_parsers: List[_ErrorDeserializer] = [], extra_error_customizers: List[_ErrorCustomizer] = [], + debug_headers: bool = False, ): self._error_parsers = _error_deserializers + (extra_error_parsers if extra_error_parsers is not None else []) self._error_customizers = _error_customizers + ( extra_error_customizers if extra_error_customizers is not None else [] ) + self._debug_headers = debug_headers def get_api_error(self, response: requests.Response) -> Optional[DatabricksError]: """ @@ -84,7 +89,7 @@ def get_api_error(self, response: requests.Response) -> Optional[DatabricksError ) return _error_mapper( response, - {"message": "unable to parse response. " + _unknown_error(response)}, + {"message": "unable to parse response. " + _unknown_error(response, self._debug_headers)}, ) # Private link failures happen via a redirect to the login page. From a requests-perspective, the request diff --git a/tests/test_errors.py b/tests/test_errors.py index e0032900f..57e045c3a 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -371,3 +371,47 @@ def test_get_api_error(test_case: TestCase): assert isinstance(e.value, test_case.want_err_type) assert str(e.value) == test_case.want_message assert e.value.get_error_details() == test_case.want_details + + +def test_debug_headers_disabled_by_default(): + """Test that debug_headers=False by default does not leak sensitive headers in unparseable errors.""" + # Create a response with Authorization header that cannot be parsed. + resp = requests.Response() + resp.status_code = 400 + resp.reason = "Bad Request" + resp.request = requests.Request("POST", "https://databricks.com/api/2.0/sql/statements").prepare() + resp.request.headers["Authorization"] = "Bearer secret-token-12345" + resp.request.headers["X-Databricks-Azure-SP-Management-Token"] = "secret-azure-token-67890" + resp._content = b"unparseable response" + + parser = errors._Parser(debug_headers=False) + error = parser.get_api_error(resp) + + error_message = str(error) + # Verify that sensitive tokens are NOT in the error message. + assert "secret-token-12345" not in error_message + assert "secret-azure-token-67890" not in error_message + assert "Authorization" not in error_message + assert "X-Databricks-Azure-SP-Management-Token" not in error_message + + +def test_debug_headers_enabled_shows_headers(): + """Test that debug_headers=True includes headers in unparseable error messages.""" + # Create a response with Authorization header that cannot be parsed. + resp = requests.Response() + resp.status_code = 400 + resp.reason = "Bad Request" + resp.request = requests.Request("POST", "https://databricks.com/api/2.0/sql/statements").prepare() + resp.request.headers["Authorization"] = "Bearer debug-token-12345" + resp.request.headers["X-Databricks-Azure-SP-Management-Token"] = "debug-azure-token-67890" + resp._content = b"unparseable response" + + parser = errors._Parser(debug_headers=True) + error = parser.get_api_error(resp) + + error_message = str(error) + # Verify that headers ARE included when explicitly enabled. + assert "Authorization" in error_message + assert "debug-token-12345" in error_message + assert "X-Databricks-Azure-SP-Management-Token" in error_message + assert "debug-azure-token-67890" in error_message