Remove exception tracebacks from failed tasks

   Tracebacks can expose sensitive information from an exception
   via the API. This change stops this behavior by only logging
   tracebacks and not storing them inside of tasks.

Author: aKlimau

CHANGES/+no-traceback-task.removal

@@ -0,0 +1 @@
+Stopped leaking sensitive information of failures in the task API.

pulpcore/app/models/task.py

@@ -3,7 +3,6 @@
 """
 
 import logging
-import traceback
 from gettext import gettext as _
 
 from django.contrib.postgres.fields import ArrayField, HStoreField
@@ -261,7 +260,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):
         """
         Set this Task to the failed state and save it.
 
@@ -270,11 +269,9 @@ def set_failed(self, exc, tb):
 
         Args:
             exc (Exception): The exception raised by the task.
-            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 = exception_to_dict(exc)
         rows = Task.objects.filter(
             pk=self.pk,
             state=TASK_STATES.RUNNING,

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(

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)
 

pulpcore/download/http.py

@@ -3,12 +3,15 @@
 import aiohttp
 import asyncio
 import backoff
+import socket
 
 from .base import BaseDownloader, DownloadResult
 from pulpcore.exceptions import (
     DigestValidationError,
     SizeValidationError,
     TimeoutException,
+    DnsDomainNameException,
+    ProxyAuthenticationFailedError,
 )
 
 
@@ -236,6 +239,7 @@ async def run(self, extra_data=None):
             aiohttp.ClientPayloadError,
             aiohttp.ClientResponseError,
             aiohttp.ServerDisconnectedError,
+            DnsDomainNameException,
             TimeoutError,
             TimeoutException,
             DigestValidationError,
@@ -269,7 +273,7 @@ async def download_wrapper():
                             e.message,
                         )
                     )
-                    raise e
+                    raise ProxyAuthenticationFailedError(self.proxy)
 
             return await download_wrapper()
 
@@ -289,12 +293,18 @@ 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.ClientConnectorError as e:
+            # Check if this is a DNS error
+            if isinstance(e.os_error, socket.gaierror):
+                raise DnsDomainNameException(self.url)
+            raise
         if self._close_session_on_finalize:
             await self.session.close()
         return to_return

pulpcore/exceptions/__init__.py

@@ -4,6 +4,11 @@
     TimeoutException,
     exception_to_dict,
     DomainProtectedError,
+    DnsDomainNameException,
+    UrlSchemeNotSupportedError,
+    ProxyAuthenticationFailedError,
+    InternalErrorException,
+    RepositoryVersionDeleteError,
 )
 from .validation import (
     DigestValidationError,

pulpcore/exceptions/base.py

@@ -44,6 +44,18 @@ def exception_to_dict(exc, traceback=None):
     return {"description": str(exc), "traceback": traceback}
 
 
+class InternalErrorException(PulpException):
+    """
+    Exception to signal that an unexpected internal error occurred.
+    """
+
+    def __init__(self):
+        super().__init__("PLP0000")
+
+    def __str__(self):
+        return _("An internal error occurred.")
+
+
 class ResourceImmutableError(PulpException):
     """
     Exceptions that are raised due to trying to update an immutable resource
@@ -94,3 +106,73 @@ 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 _("URL lookup failed.")
+
+
+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 ProxyAuthenticationError(PulpException):
+    """
+    Exception to signal that the proxy server requires authentication
+    but it was not provided or is invalid
+    """
+
+    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."
+        )

pulpcore/tasking/tasks.py

@@ -30,9 +30,13 @@
     TASK_WAKEUP_HANDLE,
     TASK_WAKEUP_UNBLOCK,
 )
+from pulpcore.exceptions.base import PulpException, InternalErrorException
+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,17 +74,22 @@ def _execute_task(task):
             log_task_start(task, domain)
             task_function = get_task_function(task)
             result = task_function()
+        except (PulpException, PulpGlueException):
+            # Log expected ways to fail without a stacktrace.
+            exc_type, exc, _ = sys.exc_info()
+            log_task_failed(task, exc_type, exc, None, domain)
+            task.set_failed(exc)
         except Exception:
+            # Unexpected Exceptions are most probably a programming error.
+            # Log error with stack trace and return generic error (HTTP 500).
             exc_type, exc, tb = sys.exc_info()
-            task.set_failed(exc, tb)
             log_task_failed(task, exc_type, exc, tb, domain)
-            send_task_notification(task)
+            safe_exc = InternalErrorException()
+            task.set_failed(safe_exc)
         else:
             task.set_completed(result)
             log_task_completed(task, domain)
-            send_task_notification(task)
-            return result
-        return None
+        send_task_notification(task)
 
 
 async def aexecute_task(task):
@@ -95,17 +104,23 @@ async def _aexecute_task(task):
         try:
             task_coroutine_fn = await aget_task_function(task)
             result = await task_coroutine_fn()
+        except (PulpException, PulpGlueException):
+            # Log expected ways to fail without a stacktrace.
+            exc_type, exc, _ = sys.exc_info()
+            log_task_failed(task, exc_type, exc, None, domain)
+            await sync_to_async(task.set_failed)(exc)
         except Exception:
+            # Unexpected Exceptions are most probably a programming error.
+            # Log error with stack trace and return generic error (HTTP 500).
             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)
-            send_task_notification(task)
+            safe_exc = InternalErrorException()
+            await sync_to_async(task.set_failed)(safe_exc)
         else:
             await sync_to_async(task.set_completed)(result)
             send_task_notification(task)
             log_task_completed(task, domain)
-            return result
-        return None
+        send_task_notification(task)
 
 
 def log_task_start(task, domain):
@@ -144,7 +159,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):

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 occurred." 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 occurred." 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 occurred." in ctx.value.task.error["description"]

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