Skip to content

Commit 6a59342

Browse files
ThomasFarstrikeThomasFarstrike
authored
Merge pull request #136 from bitcoin3us/security/download-manager-redact-url
DownloadManager: add redact_url= kwarg to hide auth-bearing URLs in logs
2 parents 03f3953 + 64963f9 commit 6a59342

4 files changed

Lines changed: 292 additions & 18 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ OS:
1818
- Disable the repl on hardware uart for esp32s3 targets (USB serial still works)
1919
- Remove big, rarely used font Montserrat 34, 40 and 48 to reduce build size by 218KiB - apps can still upscale or load fonts at runtime
2020

21+
Frameworks:
22+
- `DownloadManager.download_url`: add `redact_url=True` kwarg for callers fetching URLs that embed an auth secret (API key, OAuth token, LNBits readkey, xpub/ypub/zpub). When set, the URL is logged as `scheme://host/...REDACTED...`, the response-headers dump is suppressed, and exception messages have any embedded URL scrubbed. Default `False` preserves existing debug output for callers fetching public URLs (app icons, OS updates, etc.). Use case: prevents serial / REPL logs from leaking the secret-bearing URL even when DEBUG-level chatter is on.
23+
2124
0.9.5
2225
=====
2326

‎internal_filesystem/lib/mpos/net/download_manager.py‎

Lines changed: 67 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ class DownloadManager:
2828
@classmethod
2929
def download_url(cls, url, outfile=None, total_size=None,
3030
progress_callback=None, chunk_callback=None, headers=None,
31-
speed_callback=None):
31+
speed_callback=None, redact_url=False):
3232
"""Download a URL with flexible output modes (sync or async wrapper).
33-
33+
3434
This method automatically detects whether it's being called from an async context
3535
and either returns a coroutine (for await) or runs synchronously.
36-
36+
3737
Args:
3838
url (str): URL to download (required)
3939
outfile (str, optional): Path to write file. If None, returns bytes.
@@ -42,12 +42,21 @@ def download_url(cls, url, outfile=None, total_size=None,
4242
chunk_callback (coroutine, optional): async def callback(chunk: bytes)
4343
headers (dict, optional): HTTP headers (e.g., {'Range': 'bytes=1000-'})
4444
speed_callback (coroutine, optional): async def callback(bytes_per_second: float)
45-
45+
redact_url (bool, optional): Opt in to redacting the URL in log
46+
output and the response-headers dump. Set True whenever the
47+
URL embeds an auth secret in its path or query string —
48+
e.g. an API key, an OAuth token, an LNBits readkey, or an
49+
xpub/ypub/zpub (which exposes the wallet's whole derivation
50+
tree). Only the `scheme://host[:port]` prefix is kept in
51+
logs; path + query are replaced with "/...REDACTED...".
52+
Defaults to False to preserve current debug output for
53+
callers fetching public URLs (app icons, OS updates, etc.).
54+
4655
Returns:
4756
bytes: Downloaded content (if outfile and chunk_callback are None)
4857
bool: True if successful (when using outfile or chunk_callback)
4958
coroutine: If called from async context, returns awaitable
50-
59+
5160
Raises:
5261
ValueError: If both outfile and chunk_callback are provided
5362
"""
@@ -59,20 +68,44 @@ def download_url(cls, url, outfile=None, total_size=None,
5968
# We're in an async context, return the coroutine
6069
return cls._download_url_async(url, outfile, total_size,
6170
progress_callback, chunk_callback, headers,
62-
speed_callback)
71+
speed_callback, redact_url)
6372
except RuntimeError:
6473
# No running event loop, run synchronously
6574
return asyncio.run(cls._download_url_async(url, outfile, total_size,
6675
progress_callback, chunk_callback, headers,
67-
speed_callback))
76+
speed_callback, redact_url))
6877
except ImportError:
6978
# asyncio not available, shouldn't happen but handle gracefully
7079
raise ImportError("asyncio module not available")
71-
80+
81+
@staticmethod
82+
def _safe_url(url):
83+
"""Return a log-safe rendering of `url` for use when the original URL
84+
carries a secret in its path or query string. Strips everything
85+
after `scheme://host[:port]` and replaces it with "/...REDACTED...".
86+
87+
Examples:
88+
https://example.com/api/v2/xpub/zpub6q... -> https://example.com/...REDACTED...
89+
https://api.example.com:8080/p?key=abc -> https://api.example.com:8080/...REDACTED...
90+
https://example.com -> https://example.com (no path to redact)
91+
not-a-url -> ...REDACTED...
92+
"""
93+
try:
94+
scheme_end = url.find("://")
95+
if scheme_end < 0:
96+
return "...REDACTED..."
97+
path_start = url.find("/", scheme_end + 3)
98+
if path_start < 0:
99+
# No path component — nothing sensitive to strip.
100+
return url
101+
return url[:path_start] + "/...REDACTED..."
102+
except Exception:
103+
return "...REDACTED..."
104+
72105
@classmethod
73106
async def _download_url_async(cls, url, outfile=None, total_size=None,
74107
progress_callback=None, chunk_callback=None, headers=None,
75-
speed_callback=None):
108+
speed_callback=None, redact_url=False):
76109
"""Download a URL with flexible output modes.
77110
78111
Args:
@@ -83,11 +116,14 @@ async def _download_url_async(cls, url, outfile=None, total_size=None,
83116
chunk_callback (coroutine, optional): async def callback(chunk: bytes)
84117
headers (dict, optional): HTTP headers (e.g., {'Range': 'bytes=1000-'})
85118
speed_callback (coroutine, optional): async def callback(bytes_per_second: float)
86-
119+
redact_url (bool, optional): When True, log a redacted URL
120+
(scheme://host only) and suppress the response-headers dump.
121+
See `download_url` for details and use cases.
122+
87123
Returns:
88124
bytes: Downloaded content (if outfile and chunk_callback are None)
89125
bool: True if successful (when using outfile or chunk_callback)
90-
126+
91127
Raises:
92128
ValueError: If both outfile and chunk_callback are provided
93129
"""
@@ -98,6 +134,11 @@ async def _download_url_async(cls, url, outfile=None, total_size=None,
98134
"Use outfile for saving to disk, or chunk_callback for streaming."
99135
)
100136

137+
# Compute the log-safe rendering once; used for every URL-bearing print
138+
# below. When redact_url is False this is just the original URL, so
139+
# existing behaviour is preserved verbatim.
140+
log_url = cls._safe_url(url) if redact_url else url
141+
101142
import aiohttp
102143
session = aiohttp.ClientSession()
103144
sslctx = None # for http
@@ -106,7 +147,7 @@ async def _download_url_async(cls, url, outfile=None, total_size=None,
106147
sslctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
107148
sslctx.verify_mode = ssl.CERT_OPTIONAL # CERT_REQUIRED might fail because MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED
108149

109-
print(f"DownloadManager: Downloading {url}")
150+
print(f"DownloadManager: Downloading {log_url}")
110151

111152
fd = None
112153
try:
@@ -120,7 +161,13 @@ async def _download_url_async(cls, url, outfile=None, total_size=None,
120161
raise RuntimeError(f"HTTP {response.status}")
121162

122163
# Figure out total size and starting offset (for resume support)
123-
print("DownloadManager: Response headers:", response.headers)
164+
# When redacting, suppress the headers dump entirely — response
165+
# headers can include `set-cookie`, `cf-ray` and other tokens
166+
# that correlate to the request's secret-bearing URL.
167+
if redact_url:
168+
print("DownloadManager: Response headers: <redacted>")
169+
else:
170+
print("DownloadManager: Response headers:", response.headers)
124171
resume_offset = 0 # Starting byte offset (0 for new downloads, >0 for resumed)
125172

126173
if total_size is None:
@@ -241,7 +288,7 @@ async def _download_url_async(cls, url, outfile=None, total_size=None,
241288
speed_last_update_time = current_time
242289
else:
243290
# Chunk is None, download complete
244-
print(f"DownloadManager: Finished downloading {url}")
291+
print(f"DownloadManager: Finished downloading {log_url}")
245292
if fd:
246293
fd.close()
247294
fd = None
@@ -252,7 +299,12 @@ async def _download_url_async(cls, url, outfile=None, total_size=None,
252299
return b''.join(chunks)
253300

254301
except Exception as e:
255-
print(f"DownloadManager: Exception during download: {e}")
302+
# Exception strings from aiohttp often embed the full URL —
303+
# scrub it before printing when the caller asked for redaction.
304+
err_str = str(e)
305+
if redact_url and url in err_str:
306+
err_str = err_str.replace(url, log_url)
307+
print(f"DownloadManager: Exception during download: {err_str}")
256308
if fd:
257309
fd.close()
258310
raise # Re-raise the exception instead of suppressing it

‎internal_filesystem/lib/mpos/testing/mocks.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -747,19 +747,21 @@ def __init__(self):
747747

748748
async def download_url(self, url, outfile=None, total_size=None,
749749
progress_callback=None, chunk_callback=None, headers=None,
750-
speed_callback=None):
750+
speed_callback=None, redact_url=False):
751751
"""Mock async download with flexible output modes."""
752752
self.url_received = url
753753
self.headers_received = headers
754-
754+
self.redact_url_received = redact_url
755+
755756
self.call_history.append({
756757
'url': url,
757758
'outfile': outfile,
758759
'total_size': total_size,
759760
'headers': headers,
760761
'has_progress_callback': progress_callback is not None,
761762
'has_chunk_callback': chunk_callback is not None,
762-
'has_speed_callback': speed_callback is not None
763+
'has_speed_callback': speed_callback is not None,
764+
'redact_url': redact_url,
763765
})
764766

765767
if self.should_fail:

0 commit comments

Comments
 (0)