mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-12 12:05:07 +00:00
fix(api): sanitize user-provided values in log statements to prevent log injection
Addresses 12 CodeQL high-severity log injection alerts (CWE-117) by stripping newline and control characters from user-provided values before logging.
This commit is contained in:
@@ -16,6 +16,11 @@ from typing import Any, Callable, Dict, List, Optional
|
||||
log = logging.getLogger("download_manager")
|
||||
|
||||
|
||||
def _sanitize_log(value: object) -> str:
|
||||
"""Sanitize a value for safe logging by removing newlines and control characters."""
|
||||
return str(value).replace("\n", "").replace("\r", "").replace("\x00", "")
|
||||
|
||||
|
||||
class JobStatus(Enum):
|
||||
QUEUED = "queued"
|
||||
DOWNLOADING = "downloading"
|
||||
@@ -361,7 +366,7 @@ class DownloadQueueManager:
|
||||
self._jobs[job_id] = job
|
||||
self._job_queue.put_nowait(job)
|
||||
|
||||
log.info(f"Created download job {job_id} for {service}:{title_id}")
|
||||
log.info(f"Created download job {job_id} for {_sanitize_log(service)}:{_sanitize_log(title_id)}")
|
||||
return job
|
||||
|
||||
def get_job(self, job_id: str) -> Optional[DownloadJob]:
|
||||
@@ -381,27 +386,27 @@ class DownloadQueueManager:
|
||||
if job.status == JobStatus.QUEUED:
|
||||
job.status = JobStatus.CANCELLED
|
||||
job.cancel_event.set() # Signal cancellation
|
||||
log.info(f"Cancelled queued job {job_id}")
|
||||
log.info(f"Cancelled queued job {_sanitize_log(job_id)}")
|
||||
return True
|
||||
elif job.status == JobStatus.DOWNLOADING:
|
||||
# Set the cancellation event first - this will be checked by the download thread
|
||||
job.cancel_event.set()
|
||||
job.status = JobStatus.CANCELLED
|
||||
log.info(f"Signaled cancellation for downloading job {job_id}")
|
||||
log.info(f"Signaled cancellation for downloading job {_sanitize_log(job_id)}")
|
||||
|
||||
# Cancel the active download task
|
||||
task = self._active_downloads.get(job_id)
|
||||
if task:
|
||||
task.cancel()
|
||||
log.info(f"Cancelled download task for job {job_id}")
|
||||
log.info(f"Cancelled download task for job {_sanitize_log(job_id)}")
|
||||
|
||||
process = self._download_processes.get(job_id)
|
||||
if process:
|
||||
try:
|
||||
process.terminate()
|
||||
log.info(f"Terminated worker process for job {job_id}")
|
||||
log.info(f"Terminated worker process for job {_sanitize_log(job_id)}")
|
||||
except ProcessLookupError:
|
||||
log.debug(f"Worker process for job {job_id} already exited")
|
||||
log.debug(f"Worker process for job {_sanitize_log(job_id)} already exited")
|
||||
|
||||
return True
|
||||
|
||||
|
||||
@@ -16,6 +16,12 @@ from unshackle.core.tracks import Audio, Subtitle, Video
|
||||
|
||||
log = logging.getLogger("api")
|
||||
|
||||
|
||||
def _sanitize_log(value: object) -> str:
|
||||
"""Sanitize a value for safe logging by removing newlines and control characters."""
|
||||
return str(value).replace("\n", "").replace("\r", "").replace("\x00", "")
|
||||
|
||||
|
||||
DEFAULT_DOWNLOAD_PARAMS = {
|
||||
"profile": None,
|
||||
"quality": [],
|
||||
@@ -606,7 +612,7 @@ async def list_titles_handler(data: Dict[str, Any], request: Optional[web.Reques
|
||||
service_kwargs[param_name] = False
|
||||
else:
|
||||
# Log warning for unknown required parameters
|
||||
log.warning(f"Unknown required parameter '{param_name}' for service {normalized_service}")
|
||||
log.warning(f"Unknown required parameter '{_sanitize_log(param_name)}' for service {_sanitize_log(normalized_service)}")
|
||||
|
||||
# Filter out any parameters that the service doesn't accept
|
||||
filtered_kwargs = {}
|
||||
@@ -758,7 +764,7 @@ async def list_tracks_handler(data: Dict[str, Any], request: Optional[web.Reques
|
||||
service_kwargs[param_name] = False
|
||||
else:
|
||||
# Log warning for unknown required parameters
|
||||
log.warning(f"Unknown required parameter '{param_name}' for service {normalized_service}")
|
||||
log.warning(f"Unknown required parameter '{_sanitize_log(param_name)}' for service {_sanitize_log(normalized_service)}")
|
||||
|
||||
# Filter out any parameters that the service doesn't accept
|
||||
filtered_kwargs = {}
|
||||
@@ -1184,7 +1190,7 @@ async def get_download_job_handler(job_id: str, request: Optional[web.Request] =
|
||||
except APIError:
|
||||
raise
|
||||
except Exception as e:
|
||||
log.exception(f"Error getting download job {job_id}")
|
||||
log.exception(f"Error getting download job {_sanitize_log(job_id)}")
|
||||
debug_mode = request.app.get("debug_api", False) if request else False
|
||||
return handle_api_exception(
|
||||
e,
|
||||
@@ -1221,7 +1227,7 @@ async def cancel_download_job_handler(job_id: str, request: Optional[web.Request
|
||||
except APIError:
|
||||
raise
|
||||
except Exception as e:
|
||||
log.exception(f"Error cancelling download job {job_id}")
|
||||
log.exception(f"Error cancelling download job {_sanitize_log(job_id)}")
|
||||
debug_mode = request.app.get("debug_api", False) if request else False
|
||||
return handle_api_exception(
|
||||
e,
|
||||
@@ -1670,7 +1676,7 @@ async def session_tracks_handler(
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
log.exception(f"Error getting tracks for title {title_id}")
|
||||
log.exception(f"Error getting tracks for title {_sanitize_log(title_id)}")
|
||||
debug_mode = request.app.get("debug_api", False) if request else False
|
||||
return handle_api_exception(
|
||||
e,
|
||||
|
||||
Reference in New Issue
Block a user