Skip to content

Commit fa730a2

Browse files
committed
Fix code scanning alert: Clear-text logging of sensitive information (#27)
Address security issue identified in: https://github.com/jpstroop/fitbit-client-python/security/code-scanning/8 - Add docs/SECURITY.md with comprehensive guidance on debug mode security - Add explicit security warnings to debug output in _base.py - Update docstrings in debug-related methods to highlight security risks Debug mode intentionally includes OAuth tokens for troubleshooting, but now includes proper documentation and warnings about secure usage.
1 parent 46ddf29 commit fa730a2

File tree

5 files changed

+103
-7
lines changed

5 files changed

+103
-7
lines changed

docs/DEVELOPMENT.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,14 @@ All resource mocks are in the root [conftest.py](tests/conftest.py).
195195

196196
### Response Mocking
197197

198+
<<<<<<< Updated upstream
198199
The test suite uses the `mock_response_factory` fixture from `tests/conftest.py`
199200
to create consistent, configurable mock responses. This is the required pattern
200201
for all tests that need to mock HTTP responses.
202+
=======
203+
The test suite uses the `mock_response_factory` fixture from `tests/conftest.py` to create consistent,
204+
configurable mock responses. This is the required pattern for all tests that need to mock HTTP responses.
205+
>>>>>>> Stashed changes
201206
202207
#### Standard Mock Response Pattern
203208

@@ -253,9 +258,14 @@ mock_response.text = "<xml>content</xml>"
253258

254259
#### Parameter Validation Pattern
255260

261+
<<<<<<< Updated upstream
256262
For tests that only need to verify parameter validation or endpoint construction
257263
(not response handling), it's acceptable to use the following alternative
258264
pattern:
265+
=======
266+
For tests that only need to verify parameter validation or endpoint construction (not response handling),
267+
it's acceptable to use the following alternative pattern:
268+
>>>>>>> Stashed changes
259269
260270
```python
261271
def test_validation(resource):
@@ -268,8 +278,12 @@ def test_validation(resource):
268278
```
269279

270280
This approach provides a clean, standardized way to create mock responses with
281+
<<<<<<< Updated upstream
271282
the desired status code, data, and headers. All test files must use one of these
272283
patterns.
284+
=======
285+
the desired status code, data, and headers. All test files must use one of these patterns.
286+
>>>>>>> Stashed changes
273287
274288
## OAuth Callback Implementation
275289

docs/SECURITY.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Security Considerations
2+
3+
## Debug Mode Security Considerations and Warning
4+
5+
This library includes a debug mode feature that generates complete cURL commands
6+
for troubleshooting API requests. This feature is designed to help during
7+
development and debugging only.
8+
9+
When you enable debug mode by setting `debug=True` in any API method call:
10+
11+
```python
12+
# This will print a complete curl command to stdout
13+
client.get_profile(debug=True)
14+
```
15+
16+
The generated cURL command includes:
17+
18+
- The full API endpoint URL
19+
- All request parameters and data
20+
- The OAuth bearer token used for authentication
21+
22+
**WARNING: This exposes sensitive authentication credentials that could allow
23+
API access as the authenticated user.**
24+
25+
1. **NEVER use debug mode in production environments**
26+
2. **NEVER log debug mode output to persistent storage**
27+
3. **NEVER share debug mode output without removing the authorization token**
28+
4. **NEVER commit debug mode output to version control**
29+
30+
## General Security Considerations
31+
32+
### OAuth Token Management
33+
34+
- Store OAuth tokens securely, preferably in an encrypted format
35+
- Implement token rotation and refresh mechanisms properly
36+
- Never hard-code tokens in application code
37+
- Use environment variables or secure configuration management for client IDs
38+
and secrets
39+
40+
### Production Deployment
41+
42+
- Ensure token refresh callbacks use HTTPS
43+
- Validate that your callback URL is properly secured
44+
- Follow OAuth 2.0 best practices for authentication flows
45+
- Set appropriate token expiration times

fitbit_client/resources/_base.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
# fitbit_client/resources/_base.py
22

3+
34
# Standard library imports
45
from datetime import datetime
56
from inspect import currentframe
67
from json import JSONDecodeError
78
from json import dumps
89
from logging import getLogger
10+
import re
911
from time import sleep
1012
from typing import Dict
1113
from typing import Optional
@@ -480,7 +482,8 @@ def _make_request(
480482
requires_user_id: Whether the endpoint requires user_id in the path
481483
http_method: HTTP method to use (GET, POST, DELETE)
482484
api_version: API version to use (default: "1")
483-
debug: If True, prints a curl command to stdout to help with debugging
485+
debug: If True, prints a curl command to stdout to help with debugging.
486+
WARNING: This exposes authentication tokens! See docs/SECURITY.md for guidance.
484487
485488
Returns:
486489
JSONType: The API response in one of these formats:
@@ -516,7 +519,9 @@ def _make_request(
516519

517520
if debug:
518521
curl_command = self._build_curl_command(url, http_method, data, json, params)
519-
print(f"\n# Debug curl command for {calling_method}:")
522+
print(f"\n# DEBUG MODE: Security Warning - contains authentication tokens!")
523+
print(f"# See docs/SECURITY.md for guidance on sharing this output safely.")
524+
print(f"# Debug curl command for {calling_method}:")
520525
print(curl_command)
521526
print()
522527
return None
@@ -628,7 +633,8 @@ def _make_direct_request(self, path: str, debug: bool = False) -> JSONType:
628633
629634
Args:
630635
path: Full relative API path including query string (e.g., '/1/user/-/sleep/list.json?offset=10&limit=10')
631-
debug: If True, prints a curl command to stdout to help with debugging
636+
debug: If True, prints a curl command to stdout to help with debugging.
637+
WARNING: This exposes authentication tokens! See docs/SECURITY.md for guidance.
632638
633639
Returns:
634640
JSONDict: The API response as a dictionary
@@ -641,7 +647,9 @@ def _make_direct_request(self, path: str, debug: bool = False) -> JSONType:
641647

642648
if debug:
643649
curl_command = self._build_curl_command(url, "GET")
644-
print(f"\n# Debug curl command for {calling_method} (pagination):")
650+
print(f"\n# DEBUG MODE: Security Warning - contains authentication tokens!")
651+
print(f"# See docs/SECURITY.md for guidance on sharing this output safely.")
652+
print(f"# Debug curl command for {calling_method} (pagination):")
645653
print(curl_command)
646654
print()
647655
return {}

fitbit_client/utils/curl_debug_mixin.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ def _build_curl_command(
3737
"""
3838
Build a curl command string for debugging API requests.
3939
40+
WARNING: Security Risk - The generated command includes the actual OAuth bearer token,
41+
which should never be logged, shared, or committed to version control.
42+
See docs/SECURITY.md for guidance on securely using this feature.
43+
4044
Args:
4145
url: Full API URL
4246
http_method: HTTP method (GET, POST, DELETE)
@@ -49,7 +53,7 @@ def _build_curl_command(
4953
5054
The generated command includes:
5155
- The HTTP method (for non-GET requests)
52-
- Authorization header with OAuth token
56+
- Authorization header with OAuth token (SENSITIVE INFORMATION)
5357
- Request body (if data or json is provided)
5458
- Query parameters (if provided)
5559

tests/fitbit_client/resources/test_base.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,13 @@ def test_handle_json_response_with_invalid_json(base_resource, mock_logger):
299299
def test_make_request_json_success(base_resource, mock_oauth_session, mock_response_factory):
300300
"""Test successful JSON request"""
301301
expected_data = {"success": True}
302+
<<<<<<< Updated upstream
302303
mock_response = mock_response_factory(
303304
200, expected_data, headers={"content-type": "application/json"}
304305
)
306+
=======
307+
mock_response = mock_response_factory(200, expected_data, headers={"content-type": "application/json"})
308+
>>>>>>> Stashed changes
305309
mock_oauth_session.request.return_value = mock_response
306310

307311
result = base_resource._make_request("test/endpoint")
@@ -320,9 +324,15 @@ def test_make_request_no_content(base_resource, mock_oauth_session, mock_respons
320324
def test_make_request_xml_response(base_resource, mock_oauth_session, mock_response_factory):
321325
"""Test XML response handling"""
322326
mock_response = mock_response_factory(
327+
<<<<<<< Updated upstream
323328
200,
324329
headers={"content-type": "application/vnd.garmin.tcx+xml"},
325330
content_type="application/vnd.garmin.tcx+xml",
331+
=======
332+
200,
333+
headers={"content-type": "application/vnd.garmin.tcx+xml"},
334+
content_type="application/vnd.garmin.tcx+xml"
335+
>>>>>>> Stashed changes
326336
)
327337
mock_response.text = "<test>data</test>"
328338
mock_oauth_session.request.return_value = mock_response
@@ -331,12 +341,21 @@ def test_make_request_xml_response(base_resource, mock_oauth_session, mock_respo
331341
assert result == "<test>data</test>"
332342

333343

344+
<<<<<<< Updated upstream
334345
def test_make_request_unexpected_content_type(
335346
base_resource, mock_oauth_session, mock_response_factory
336347
):
337348
"""Test handling of unexpected content type"""
338349
mock_response = mock_response_factory(
339350
200, headers={"content-type": "text/plain"}, content_type="text/plain"
351+
=======
352+
def test_make_request_unexpected_content_type(base_resource, mock_oauth_session, mock_response_factory):
353+
"""Test handling of unexpected content type"""
354+
mock_response = mock_response_factory(
355+
200,
356+
headers={"content-type": "text/plain"},
357+
content_type="text/plain"
358+
>>>>>>> Stashed changes
340359
)
341360
mock_response.text = "some data"
342361
mock_oauth_session.request.return_value = mock_response
@@ -619,8 +638,14 @@ def test_make_direct_request_with_debug(mock_build_curl, mock_print, base_resour
619638
# Verify the curl command was built correctly
620639
mock_build_curl.assert_called_once_with("https://api.fitbit.com/test", "GET")
621640

622-
# Verify print was called with the right message pattern
623-
mock_print.assert_any_call("\n# Debug curl command for test_pagination (pagination):")
641+
# Verify security warning messages were printed
642+
mock_print.assert_any_call(
643+
"\n# DEBUG MODE: Security Warning - contains authentication tokens!"
644+
)
645+
mock_print.assert_any_call(
646+
"# See docs/SECURITY.md for guidance on sharing this output safely."
647+
)
648+
mock_print.assert_any_call("# Debug curl command for test_pagination (pagination):")
624649

625650

626651
@patch("fitbit_client.resources._base.BaseResource._handle_json_response")

0 commit comments

Comments
 (0)