diff --git a/tests/core/test_debug_logging.py b/tests/core/test_debug_logging.py index cdbd6064..deffcd37 100644 --- a/tests/core/test_debug_logging.py +++ b/tests/core/test_debug_logging.py @@ -16,7 +16,14 @@ from unshackle.core.utilities import ( log_event, timed_operation, ) -from unshackle.core.utils.redact import redact_all, redact_path, redact_text, redact_url, safe_display_url +from unshackle.core.utils.redact import ( + redact_all, + redact_path, + redact_secrets, + redact_text, + redact_url, + safe_display_url, +) from unshackle.core.utils.subprocess import log_tool_run pytestmark = pytest.mark.unit @@ -266,6 +273,15 @@ def test_redact_all_composes_secrets_urls_and_paths(): assert "/temp" in out +def test_redact_secrets_keeps_the_url_but_drops_credentials_and_paths(): + root = str(Path(__import__("unshackle.core.utils.redact", fromlist=["x"]).__file__).resolve().parents[3]) + out = redact_secrets(f"404 Not Found: https://u:p@cdn.x/a/b.mpd?token=t (cwd {root}/temp)") + assert "cdn.x/a/b.mpd" in out + assert "u:p@" not in out + assert "token=t" not in out + assert "/temp" in out + + def test_logged_output_dir_is_path_redacted(debug_log: Path): root = str(Path(__import__("unshackle.core.utils.redact", fromlist=["x"]).__file__).resolve().parents[3]) log_event("downloader_start", context={"output_dir": f"{root}/temp", "filename": "Audio_3eeab4ed.mp4"}) diff --git a/tests/remote/unit/test_dashboard.py b/tests/remote/unit/test_dashboard.py index 5a5a2daf..af9ea7ba 100644 --- a/tests/remote/unit/test_dashboard.py +++ b/tests/remote/unit/test_dashboard.py @@ -69,6 +69,28 @@ def test_ring_since_and_level() -> None: assert ring.seq == 5 assert [r["msg"] for r in ring.since(3)] == ["m3", "m4"] assert [r["msg"] for r in ring.since(0, "warning")] == ["m3"] + + +def test_ring_masks_host_paths_and_secrets_in_records() -> None: + """The ring feeds both /api/logs and the SSE stream, so it must mask on the way in.""" + from pathlib import Path + + import unshackle.core.utils.redact as redact_mod + + root = str(Path(redact_mod.__file__).resolve().parents[3]) + ring = RingLogHandler(maxlen=3) + ring.setFormatter(logging.Formatter("%(message)s")) + logger = logging.getLogger("test.ring.redact") + logger.setLevel(logging.INFO) + logger.addHandler(ring) + logger.error(f'File "{root}/unshackle/services/HMAX/__init__.py" 404: https://api.x/y?token=abc') + logger.removeHandler(ring) + + msg = ring.records[-1]["msg"] + assert root not in msg + assert "/unshackle/services/HMAX" in msg + assert "token=abc" not in msg + assert "https://api.x/y" in msg assert ring.since(0, logger="test") and not ring.since(0, logger="other") diff --git a/unshackle/core/api/errors.py b/unshackle/core/api/errors.py index c6b262a3..623e2a32 100644 --- a/unshackle/core/api/errors.py +++ b/unshackle/core/api/errors.py @@ -14,7 +14,7 @@ from typing import Any from aiohttp import web -from unshackle.core.utils.redact import redact_all +from unshackle.core.utils.redact import redact_secrets class APIErrorCode(str, Enum): @@ -152,7 +152,7 @@ def build_error_response( } if isinstance(error, Exception): - debug_info["traceback"] = redact_all(traceback.format_exc()) + debug_info["traceback"] = redact_secrets(traceback.format_exc()) if extra_debug_info: debug_info.update(extra_debug_info) @@ -187,7 +187,7 @@ def categorize_exception( if "proxy" in str(root).lower(): return APIError( error_code=APIErrorCode.INVALID_PROXY, - message=f"The proxy for this session failed: {redact_all(str(root))}", + message=f"The proxy for this session failed: {redact_secrets(str(root))}", details={**context, "reason": "proxy_error"}, retryable=False, ) @@ -195,7 +195,7 @@ def categorize_exception( if any(keyword in exc_str for keyword in ["auth", "login", "credential", "unauthorized", "forbidden", "token"]): return APIError( error_code=APIErrorCode.AUTH_FAILED, - message=f"Authentication failed: {redact_all(str(exc))}", + message=f"Authentication failed: {redact_secrets(str(exc))}", details={**context, "reason": "authentication_error"}, retryable=False, ) @@ -214,7 +214,7 @@ def categorize_exception( ) or exc_type in ["ConnectionError", "TimeoutError", "URLError", "SSLError"]: return APIError( error_code=APIErrorCode.NETWORK_ERROR, - message=f"Network error occurred: {redact_all(str(exc))}", + message=f"Network error occurred: {redact_secrets(str(exc))}", details={**context, "reason": "network_connectivity"}, retryable=True, http_status=503, @@ -223,7 +223,7 @@ def categorize_exception( if any(keyword in exc_str for keyword in ["geofence", "region", "not available in", "territory"]): return APIError( error_code=APIErrorCode.GEOFENCE, - message=f"Content not available in your region: {redact_all(str(exc))}", + message=f"Content not available in your region: {redact_secrets(str(exc))}", details={**context, "reason": "geofence_restriction"}, retryable=False, ) @@ -231,7 +231,7 @@ def categorize_exception( if any(keyword in exc_str for keyword in ["not found", "404", "does not exist", "invalid id"]): return APIError( error_code=APIErrorCode.NOT_FOUND, - message=f"Resource not found: {redact_all(str(exc))}", + message=f"Resource not found: {redact_secrets(str(exc))}", details={**context, "reason": "not_found"}, retryable=False, ) @@ -239,7 +239,7 @@ def categorize_exception( if any(keyword in exc_str for keyword in ["rate limit", "too many requests", "429", "throttle"]): return APIError( error_code=APIErrorCode.RATE_LIMITED, - message=f"Rate limit exceeded: {redact_all(str(exc))}", + message=f"Rate limit exceeded: {redact_secrets(str(exc))}", details={**context, "reason": "rate_limited"}, retryable=True, http_status=429, @@ -248,7 +248,7 @@ def categorize_exception( if any(keyword in exc_str for keyword in ["drm", "license", "widevine", "playready", "decrypt"]): return APIError( error_code=APIErrorCode.DRM_ERROR, - message=f"DRM error: {redact_all(str(exc))}", + message=f"DRM error: {redact_secrets(str(exc))}", details={**context, "reason": "drm_failure"}, retryable=False, ) @@ -256,7 +256,7 @@ def categorize_exception( if any(keyword in exc_str for keyword in ["service unavailable", "503", "maintenance", "temporarily unavailable"]): return APIError( error_code=APIErrorCode.SERVICE_UNAVAILABLE, - message=f"Service temporarily unavailable: {redact_all(str(exc))}", + message=f"Service temporarily unavailable: {redact_secrets(str(exc))}", details={**context, "reason": "service_unavailable"}, retryable=True, http_status=503, @@ -268,14 +268,14 @@ def categorize_exception( ]: return APIError( error_code=APIErrorCode.INVALID_INPUT, - message=f"Invalid input: {redact_all(str(exc))}", + message=f"Invalid input: {redact_secrets(str(exc))}", details={**context, "reason": "validation_failed"}, retryable=False, ) return APIError( error_code=APIErrorCode.INTERNAL_ERROR, - message=f"An unexpected error occurred: {redact_all(str(exc))}", + message=f"An unexpected error occurred: {redact_secrets(str(exc))}", details={**context, "exception_type": exc_type}, retryable=False, ) diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index e5fbdee3..c6640269 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -34,7 +34,7 @@ from unshackle.core.titles import Episode, Movie, Song, Title_T from unshackle.core.tracks import Audio, Subtitle, Tracks, Video from unshackle.core.utilities import declared_kwargs from unshackle.core.utils.collections import ci_get -from unshackle.core.utils.redact import REDACTED, URL_USERINFO_RE, redact_all, redact_path, redact_text +from unshackle.core.utils.redact import REDACTED, URL_USERINFO_RE, redact_all, redact_secrets, redact_text log = logging.getLogger("api") @@ -1993,11 +1993,6 @@ async def dashboard_logs_handler(request: web.Request) -> web.Response: except ValueError: since = 0 records = ring.since(since, request.query.get("level"), request.query.get("logger")) - # The ring buffer stores raw log lines; a captured traceback carries host paths. Mask them - # on the way out, the same treatment the job-detail API gives its stored traceback. - for rec in records: - if isinstance(rec, dict) and isinstance(rec.get("message"), str): - rec["message"] = redact_path(rec["message"]) return web.json_response({"seq": ring.seq, "records": records}) @@ -3021,7 +3016,7 @@ async def session_create_handler(data: Dict[str, Any], request: Optional[web.Req except (Exception, SystemExit) as e: log.exception("Auth failed for session %s", session_id) session.auth_status = AuthStatus.FAILED - session.auth_error = redact_all(str(e)) + session.auth_error = redact_secrets(str(e)) bridge.status = AuthStatus.FAILED asyncio.create_task(run_auth()) diff --git a/unshackle/core/api/stats.py b/unshackle/core/api/stats.py index 8a5a6b11..865d60eb 100644 --- a/unshackle/core/api/stats.py +++ b/unshackle/core/api/stats.py @@ -14,6 +14,7 @@ from aiohttp import web from unshackle.core import __code_hash__, __version__ from unshackle.core.api.events import bus from unshackle.core.config import config +from unshackle.core.utils.redact import redact_secrets RATE_LIMIT_WINDOW = 3600.0 @@ -227,7 +228,7 @@ class RingLogHandler(logging.Handler): "ts": record.created, "level": record.levelname, "logger": record.name, - "msg": self.format(record), + "msg": redact_secrets(self.format(record)), } self.records.append(item) bus.publish("log", item) diff --git a/unshackle/core/utils/redact.py b/unshackle/core/utils/redact.py index ce82da99..bc1a47cd 100644 --- a/unshackle/core/utils/redact.py +++ b/unshackle/core/utils/redact.py @@ -162,3 +162,13 @@ def redact_url(text: Optional[str]) -> Optional[str]: def redact_all(text: Optional[str]) -> Optional[str]: """Full redaction for logged strings: secrets, then URLs, then local path prefixes.""" return redact_path(redact_url(redact_text(text))) + + +def redact_secrets(text: Optional[str]) -> Optional[str]: + """Redact text that goes back to the caller: secrets and local path prefixes only. + + URLs stay readable. The caller supplied the URL that failed, or already holds it, so a + collapsed URL hides nothing from them. It only removes the useful part of a message like + ``404 Not Found: ``. Use ``redact_all`` for a debug log file that other people read. + """ + return redact_path(redact_text(text))