Mask credentials in transport-level request failures - #341
Open
Flerpharos wants to merge 1 commit into
Open
Conversation
A connection/DNS/TLS/proxy failure raised the raw requests exception, which embeds the full signed URL -- including password=<auth_code>, login_or_email, akid, expires, and sig. The existing masking only ran for HTTP-status errors in _handle_request_status; transport exceptions produce no response and bypassed it, leaking a live auth code and replayable signed URL into logs and tracebacks. Extract the query-param masking into `_mask_sensitive_url()` (reused by the status-error path), and route all four request methods through a new `_send()` helper that catches `requests.RequestException` and re-raises it as an ApiError with the URL masked, suppressing the original (`from None`) so the unmasked URL does not survive on `__cause__`. Fixes #338. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What
Fixes #338 (HIGH, security). A connection/DNS/TLS/proxy failure raised the raw
requestsexception, which embeds the full signed URL — includingpassword=<auth_code>,login_or_email,akid,expires,sig. The existing credential masking only ran for HTTP-status errors in_handle_request_status; transport exceptions produce no response and bypassed it, leaking a live one-hour auth code and a replayable signed URL into logs/tracebacks.Fix
_mask_sensitive_url()(also now used by the status-error path — no behavior change there).stream_api_get/post,raw_api_get/post) through a new_send()helper that catchesrequests.RequestExceptionand re-raises it as anApiErrorwith the URL masked, usingfrom Noneso the unmasked URL doesn't survive on__cause__.The detail is preserved by masking the known request URL within the exception text, so error messages stay useful without the credentials.
Verification
New test
test_transport_exception_masks_credentials(forces a transport failure carrying the real signed URL; asserts the auth code,akid, and email are absent andpassword=***is present). Existingtest_client_handle_request_status_sanitizes_urlstill passes. Full suite 431 passed, 1 skipped; ruff/pyright clean.Fixes #338.
🤖 Generated with Claude Code