diff --git a/CHANGES/+no-traceback-task.removal b/CHANGES/+no-traceback-task.removal new file mode 100644 index 00000000000..40c4f9ee82d --- /dev/null +++ b/CHANGES/+no-traceback-task.removal @@ -0,0 +1 @@ +Stopped leaking sensitive information of failures in the task API. diff --git a/pulpcore/app/models/task.py b/pulpcore/app/models/task.py index ed0b2e87a0d..133fb6f9014 100644 --- a/pulpcore/app/models/task.py +++ b/pulpcore/app/models/task.py @@ -272,7 +272,7 @@ def set_completed(self, result=None): ) self._cleanup_progress_reports(TASK_STATES.COMPLETED) - def set_failed(self, exc, tb): + def set_failed(self, exc, tb=None): """ Set this Task to the failed state and save it. @@ -284,8 +284,12 @@ def set_failed(self, exc, tb): tb (traceback): Traceback instance for the current exception. """ finished_at = timezone.now() - tb_str = "".join(traceback.format_tb(tb)) - error = exception_to_dict(exc, tb_str) + error = {} + if tb: + tb_str = "".join(traceback.format_tb(tb)) + error = exception_to_dict(exc, tb_str) + else: + error = exception_to_dict(exc) rows = Task.objects.filter( pk=self.pk, state=TASK_STATES.RUNNING, diff --git a/pulpcore/app/tasks/repository.py b/pulpcore/app/tasks/repository.py index b20bf3c7dbd..efa701b94bb 100644 --- a/pulpcore/app/tasks/repository.py +++ b/pulpcore/app/tasks/repository.py @@ -7,7 +7,7 @@ from asgiref.sync import sync_to_async from django.db import transaction -from rest_framework.serializers import ValidationError +from pulpcore.exceptions import RepositoryVersionDeleteError from pulpcore.app import models from pulpcore.app.models import ProgressReport @@ -44,12 +44,7 @@ def delete_version(pk): return if version.repository.versions.complete().count() <= 1: - raise ValidationError( - _( - "Cannot delete repository version. Repositories must have at least one " - "repository version." - ) - ) + raise RepositoryVersionDeleteError log.info( "Deleting and squashing version {num} of repository '{repo}'".format( diff --git a/pulpcore/download/factory.py b/pulpcore/download/factory.py index 343d45be3f6..504dae34452 100644 --- a/pulpcore/download/factory.py +++ b/pulpcore/download/factory.py @@ -2,7 +2,6 @@ import asyncio import atexit import copy -from gettext import gettext as _ from multidict import MultiDict import platform import ssl @@ -15,6 +14,7 @@ from pulpcore.app.apps import PulpAppConfig from .http import HttpDownloader from .file import FileDownloader +from pulpcore.exceptions import UrlSchemeNotSupportedError PROTOCOL_MAP = { @@ -177,7 +177,7 @@ def build(self, url, **kwargs): builder = self._handler_map[scheme] download_class = self._download_class_map[scheme] except KeyError: - raise ValueError(_("URL: {u} not supported.".format(u=url))) + raise UrlSchemeNotSupportedError(url) else: return builder(download_class, url, **kwargs) diff --git a/pulpcore/download/http.py b/pulpcore/download/http.py index 2d3f46a9374..7b301f37c2e 100644 --- a/pulpcore/download/http.py +++ b/pulpcore/download/http.py @@ -9,6 +9,8 @@ DigestValidationError, SizeValidationError, TimeoutException, + DnsDomainNameException, + ProxyAuthenticationRequiredError, ) @@ -236,6 +238,7 @@ async def run(self, extra_data=None): aiohttp.ClientPayloadError, aiohttp.ClientResponseError, aiohttp.ServerDisconnectedError, + DnsDomainNameException, TimeoutError, TimeoutException, DigestValidationError, @@ -289,12 +292,17 @@ async def _run(self, extra_data=None): """ if self.download_throttler: await self.download_throttler.acquire() - async with self.session.get( - self.url, proxy=self.proxy, proxy_auth=self.proxy_auth, auth=self.auth - ) as response: - self.raise_for_status(response) - to_return = await self._handle_response(response) - await response.release() + try: + async with self.session.get( + self.url, proxy=self.proxy, proxy_auth=self.proxy_auth, auth=self.auth + ) as response: + self.raise_for_status(response) + to_return = await self._handle_response(response) + await response.release() + except aiohttp.ClientConnectorDNSError: + raise DnsDomainNameException(self.url) + except aiohttp.ClientHttpProxyError: + raise ProxyAuthenticationRequiredError(self.proxy) if self._close_session_on_finalize: await self.session.close() return to_return diff --git a/pulpcore/exceptions/__init__.py b/pulpcore/exceptions/__init__.py index 476cd60d3da..9563733af9c 100644 --- a/pulpcore/exceptions/__init__.py +++ b/pulpcore/exceptions/__init__.py @@ -4,6 +4,10 @@ TimeoutException, exception_to_dict, DomainProtectedError, + DnsDomainNameException, + UrlSchemeNotSupportedError, + ProxyAuthenticationRequiredError, + RepositoryVersionDeleteError, ) from .validation import ( DigestValidationError, diff --git a/pulpcore/exceptions/base.py b/pulpcore/exceptions/base.py index ecddb2f084f..49979b7d0ef 100644 --- a/pulpcore/exceptions/base.py +++ b/pulpcore/exceptions/base.py @@ -94,3 +94,75 @@ def __init__(self): def __str__(self): return _("You cannot delete a domain that still contains repositories with content.") + + +class DnsDomainNameException(PulpException): + """ + Exception to signal that dns could not resolve the domain name for specified url. + """ + + def __init__(self, url): + """ + :param url: the url that dns could not resolve + :type url: str + """ + super().__init__("PLP0008") + self.url = url + + def __str__(self): + return _("Domain name was not found for {}. Check if specified url is valid.").format( + self.url + ) + + +class UrlSchemeNotSupportedError(PulpException): + """ + Exception raised when a URL scheme (e.g. 'ftp://') is provided that + Pulp does not have a registered handler for. + """ + + def __init__(self, url): + """ + :param url: The full URL that failed validation. + :type url: str + """ + super().__init__("PLP0009") + self.url = url + + def __str__(self): + return _("URL: {u} not supported.").format(u=self.url) + + +class ProxyAuthenticationRequiredError(PulpException): + """ + Exception to signal that the proxy server requires authentication + but it was not provided or is invalid (HTTP 407). + """ + + def __init__(self, proxy_url): + """ + :param proxy_url: The URL of the proxy server. + :type proxy_url: str + """ + super().__init__("PLP0010") + self.proxy_url = proxy_url + + def __str__(self): + return _( + "Proxy authentication failed for {proxy_url}. Please check your proxy credentials." + ).format(proxy_url=self.proxy_url) + + +class RepositoryVersionDeleteError(PulpException): + """ + Raised when attempting to delete a repository version that cannot be deleted + """ + + def __init__(self): + super().__init__("PLP0011") + + def __str__(self): + return _( + "Cannot delete repository version. Repositories must have at least one " + "repository version." + ) diff --git a/pulpcore/tasking/tasks.py b/pulpcore/tasking/tasks.py index c187bd91957..f1a72880913 100644 --- a/pulpcore/tasking/tasks.py +++ b/pulpcore/tasking/tasks.py @@ -30,9 +30,13 @@ TASK_WAKEUP_HANDLE, TASK_WAKEUP_UNBLOCK, ) +from pulpcore.exceptions.base import PulpException +from pulp_glue.common.exceptions import PulpException as PulpGlueException + from pulpcore.middleware import x_task_diagnostics_var from pulpcore.tasking.kafka import send_task_notification + _logger = logging.getLogger(__name__) @@ -70,10 +74,17 @@ def _execute_task(task): log_task_start(task, domain) task_function = get_task_function(task) result = task_function() + except (PulpException, PulpGlueException): + exc_type, exc, _ = sys.exc_info() + log_task_failed(task, exc_type, exc, None, domain) # Leave no traceback in logs + task.set_failed(exc) + send_task_notification(task) except Exception: exc_type, exc, tb = sys.exc_info() - task.set_failed(exc, tb) log_task_failed(task, exc_type, exc, tb, domain) + # Generic exception for user + safe_exc = Exception("An internal error occured.") + task.set_failed(safe_exc) send_task_notification(task) else: task.set_completed(result) @@ -95,10 +106,17 @@ async def _aexecute_task(task): try: task_coroutine_fn = await aget_task_function(task) result = await task_coroutine_fn() + except (PulpException, PulpGlueException): + exc_type, exc, _ = sys.exc_info() + log_task_failed(task, exc_type, exc, None, domain) # Leave no traceback in logs + await sync_to_async(task.set_failed)(exc) + send_task_notification(task) except Exception: exc_type, exc, tb = sys.exc_info() - await sync_to_async(task.set_failed)(exc, tb) log_task_failed(task, exc_type, exc, tb, domain) + # Generic exception for user + safe_exc = Exception("An internal error occured.") + await sync_to_async(task.set_failed)(safe_exc) send_task_notification(task) else: await sync_to_async(task.set_completed)(result) @@ -144,7 +162,8 @@ def log_task_failed(task, exc_type, exc, tb, domain): domain=domain.name, ) ) - _logger.info("\n".join(traceback.format_list(traceback.extract_tb(tb)))) + if tb: + _logger.info("\n".join(traceback.format_list(traceback.extract_tb(tb)))) async def aget_task_function(task): diff --git a/pulpcore/tests/functional/api/test_tasking.py b/pulpcore/tests/functional/api/test_tasking.py index 183ec2f00b6..86c597df1ee 100644 --- a/pulpcore/tests/functional/api/test_tasking.py +++ b/pulpcore/tests/functional/api/test_tasking.py @@ -468,7 +468,9 @@ def test_executes_on_api_worker_when_no_async( "pulpcore.app.tasks.test.sleep", args=(LT_TIMEOUT,), immediate=True ) monitor_task(task_href) - assert "Immediate tasks must be async functions" in ctx.value.task.error["description"] + # Assert masked internal error + # Underlying cause is ValueError("Immediate tasks must be async functions") + assert "An internal error occured." in ctx.value.task.error["description"] @pytest.mark.parallel def test_timeouts_on_api_worker(self, pulpcore_bindings, dispatch_task): @@ -484,7 +486,8 @@ def test_timeouts_on_api_worker(self, pulpcore_bindings, dispatch_task): ) task = pulpcore_bindings.TasksApi.read(task_href) assert task.state == "failed" - assert "timed out after" in task.error["description"] + # Assert masked internal error; underlying cause is asyncio.TimeoutError + assert "An internal error occured." in task.error["description"] @pytest.fixture @@ -576,4 +579,5 @@ def test_times_out_on_task_worker( exclusive_resources=[COMMON_RESOURCE], ) monitor_task(task_href) - assert "timed out after" in ctx.value.task.error["description"] + # Assert masked internal error; underlying cause is asyncio.TimeoutError + assert "An internal error occured." in ctx.value.task.error["description"] diff --git a/pulpcore/tests/functional/api/using_plugin/test_proxy.py b/pulpcore/tests/functional/api/using_plugin/test_proxy.py index 29f132ba92a..3c19007b6e0 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_proxy.py +++ b/pulpcore/tests/functional/api/using_plugin/test_proxy.py @@ -101,7 +101,7 @@ def test_sync_https_through_http_proxy_with_auth_but_auth_not_configured( try: _run_basic_sync_and_assert(file_bindings, monitor_task, remote_on_demand, file_repo) except PulpTaskError as exc: - assert "407, message='Proxy Authentication Required'" in exc.task.error["description"] + assert "Proxy authentication failed for" in exc.task.error["description"] @pytest.mark.parallel diff --git a/pyproject.toml b/pyproject.toml index 14812972ff2..fe99fb20e65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,9 +52,10 @@ dependencies = [ "opentelemetry-sdk>=1.27.0,<1.39", "opentelemetry-exporter-otlp-proto-http>=1.27.0,<1.39", "protobuf>=4.21.1,<7.0", - "pulp-glue>=0.28.0,<0.37", + "pulp-glue>=0.30.0,<0.37", "pygtrie>=2.5,<=2.5.0", "psycopg[binary]>=3.1.8,<3.4", # SemVer, not explicitely stated, but mentioned on multiple changes. + "pycares>=4.0.0,<5.0", # Explicit dependency to ensure compatibility with aiodns. "pyparsing>=3.1.0,<3.3", # Looks like only bugfixes in z-Stream. "python-gnupg>=0.5.0,<0.6", # Looks like only bugfixes in z-Stream [changelog only in git] "PyYAML>=5.1.1,<6.1", # Looks like only bugfixes in z-Stream.