mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-12 03:55:05 +00:00
fix(api): keep error URLs readable and mask host paths in dashboard logs
This commit is contained in:
@@ -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 "<unshackle>/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 "<unshackle>/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"})
|
||||
|
||||
@@ -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>/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")
|
||||
|
||||
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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: <url>``. Use ``redact_all`` for a debug log file that other people read.
|
||||
"""
|
||||
return redact_path(redact_text(text))
|
||||
|
||||
Reference in New Issue
Block a user