mirror of
https://github.com/freedomofpress/dangerzone.git
synced 2025-04-28 18:02:38 +02:00
Sanitize progress reports in a provider-agnostic way
Update the common `print_progress()` method in the base `IsolationProvider` class, with two extra features: 1. Always sanitize the provided text argument. 2. Mark the sanitized text argument as untrusted. This is default behavior from now on, since this function is commonly used to parse progress reports from the conversion sandbox.
This commit is contained in:
parent
cfa0c01d8f
commit
9410b68c1d
3 changed files with 73 additions and 1 deletions
|
@ -27,6 +27,8 @@ since 0.4.1, and this project adheres to [Semantic Versioning](https://semver.or
|
||||||
|
|
||||||
- Continuously scan our Python dependencies and container image for
|
- Continuously scan our Python dependencies and container image for
|
||||||
vulnerabilities ([issue #222](https://github.com/freedomofpress/dangerzone/issues/222))
|
vulnerabilities ([issue #222](https://github.com/freedomofpress/dangerzone/issues/222))
|
||||||
|
- Sanitize potentially unsafe characters from strings that are shown in a user's
|
||||||
|
terminal.
|
||||||
|
|
||||||
## Dangerzone 0.4.1
|
## Dangerzone 0.4.1
|
||||||
|
|
||||||
|
|
|
@ -6,6 +6,7 @@ from typing import Callable, Optional
|
||||||
from colorama import Fore, Style
|
from colorama import Fore, Style
|
||||||
|
|
||||||
from ..document import Document
|
from ..document import Document
|
||||||
|
from ..util import replace_control_chars
|
||||||
|
|
||||||
log = logging.getLogger(__name__)
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
@ -49,7 +50,7 @@ class IsolationProvider(ABC):
|
||||||
) -> bool:
|
) -> bool:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
def print_progress(
|
def _print_progress(
|
||||||
self, document: Document, error: bool, text: str, percentage: float
|
self, document: Document, error: bool, text: str, percentage: float
|
||||||
) -> None:
|
) -> None:
|
||||||
s = Style.BRIGHT + Fore.YELLOW + f"[doc {document.id}] "
|
s = Style.BRIGHT + Fore.YELLOW + f"[doc {document.id}] "
|
||||||
|
@ -64,6 +65,19 @@ class IsolationProvider(ABC):
|
||||||
if self.progress_callback:
|
if self.progress_callback:
|
||||||
self.progress_callback(error, text, percentage)
|
self.progress_callback(error, text, percentage)
|
||||||
|
|
||||||
|
def print_progress_trusted(
|
||||||
|
self, document: Document, error: bool, text: str, percentage: float
|
||||||
|
) -> None:
|
||||||
|
return self._print_progress(document, error, text, percentage)
|
||||||
|
|
||||||
|
def print_progress(
|
||||||
|
self, document: Document, error: bool, untrusted_text: str, percentage: float
|
||||||
|
) -> None:
|
||||||
|
text = replace_control_chars(untrusted_text)
|
||||||
|
return self.print_progress_trusted(
|
||||||
|
document, error, "UNTRUSTED> " + text, percentage
|
||||||
|
)
|
||||||
|
|
||||||
@abstractmethod
|
@abstractmethod
|
||||||
def get_max_parallel_conversions(self) -> int:
|
def get_max_parallel_conversions(self) -> int:
|
||||||
pass
|
pass
|
||||||
|
|
56
tests/isolation_provider/test_base.py
Normal file
56
tests/isolation_provider/test_base.py
Normal file
|
@ -0,0 +1,56 @@
|
||||||
|
import pytest
|
||||||
|
from colorama import Style
|
||||||
|
from pytest_mock import MockerFixture
|
||||||
|
|
||||||
|
from dangerzone.document import Document
|
||||||
|
from dangerzone.isolation_provider import base, container, qubes
|
||||||
|
|
||||||
|
from .. import sanitized_text, uncommon_text
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"provider",
|
||||||
|
[
|
||||||
|
container.Container(enable_timeouts=False),
|
||||||
|
qubes.Qubes(),
|
||||||
|
],
|
||||||
|
ids=["Container", "Qubes"],
|
||||||
|
)
|
||||||
|
def test_print_progress(
|
||||||
|
provider: base.IsolationProvider,
|
||||||
|
uncommon_text: str,
|
||||||
|
sanitized_text: str,
|
||||||
|
mocker: MockerFixture,
|
||||||
|
) -> None:
|
||||||
|
"""Test that the print_progress() method of our isolation providers sanitizes text.
|
||||||
|
|
||||||
|
Iterate our isolation providers and make sure that their print_progress() methods
|
||||||
|
sanitizes the provided text, before passing it to the logging functions and other
|
||||||
|
callbacks.
|
||||||
|
"""
|
||||||
|
d = Document()
|
||||||
|
provider.progress_callback = mocker.MagicMock()
|
||||||
|
log_info_spy = mocker.spy(base.log, "info")
|
||||||
|
log_error_spy = mocker.spy(base.log, "error")
|
||||||
|
_print_progress_spy = mocker.spy(provider, "_print_progress")
|
||||||
|
|
||||||
|
for error, untrusted_text, sanitized_text in [
|
||||||
|
(True, "normal text", "UNTRUSTED> normal text"),
|
||||||
|
(False, "normal text", "UNTRUSTED> normal text"),
|
||||||
|
(True, uncommon_text, "UNTRUSTED> " + sanitized_text),
|
||||||
|
(False, uncommon_text, "UNTRUSTED> " + sanitized_text),
|
||||||
|
]:
|
||||||
|
log_info_spy.reset_mock()
|
||||||
|
log_error_spy.reset_mock()
|
||||||
|
|
||||||
|
provider.print_progress(d, error, untrusted_text, 0)
|
||||||
|
provider.progress_callback.assert_called_with(error, sanitized_text, 0) # type: ignore [union-attr]
|
||||||
|
_print_progress_spy.assert_called_with(d, error, sanitized_text, 0)
|
||||||
|
if error:
|
||||||
|
assert log_error_spy.call_args[0][0].endswith(
|
||||||
|
sanitized_text + Style.RESET_ALL
|
||||||
|
)
|
||||||
|
log_info_spy.assert_not_called()
|
||||||
|
else:
|
||||||
|
assert log_info_spy.call_args[0][0].endswith(sanitized_text)
|
||||||
|
log_error_spy.assert_not_called()
|
Loading…
Reference in a new issue