Skip to content

Commit

Permalink
Add a --debug flag to the CLI to help retrieve more logs.
Browse files Browse the repository at this point in the history
When the flag is set:

- The `RUNSC_DEBUG=1` environment variable is added to the outer container ;
- the stderr from the outer container is attached to the exception, and
  displayed to the user on failures.
  • Loading branch information
almet committed Oct 17, 2024
1 parent 03b3c9e commit 981fcd4
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 9 deletions.
8 changes: 7 additions & 1 deletion dangerzone/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ def print_header(s: str) -> None:
type=click.UNPROCESSED,
callback=args.validate_input_filenames,
)
@click.option(
"--debug",
"debug",
flag_value=True,
help="Run Dangerzone in debug mode, and get more logs from the conversion process.")
@click.version_option(version=get_version(), message="%(version)s")
@errors.handle_document_errors
def cli_main(
Expand All @@ -50,6 +55,7 @@ def cli_main(
filenames: List[str],
archive: bool,
dummy_conversion: bool,
debug: bool,
) -> None:
setup_logging()

Expand All @@ -58,7 +64,7 @@ def cli_main(
elif is_qubes_native_conversion():
dangerzone = DangerzoneCore(Qubes())
else:
dangerzone = DangerzoneCore(Container())
dangerzone = DangerzoneCore(Container(debug=debug))

display_banner()
if len(filenames) == 1 and output_filename:
Expand Down
16 changes: 13 additions & 3 deletions dangerzone/conversion/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,20 @@ class ConversionException(Exception):
error_message = "Unspecified error"
error_code = ERROR_SHIFT

def __init__(self, error_message: Optional[str] = None) -> None:
def __init__(
self, error_message: Optional[str] = None, logs: Optional[str] = None
) -> None:
if error_message:
self.error_message = error_message
self.logs = logs
super().__init__(self.error_message)

def __str__(self):
msg = f"{self.error_message}"
if self.logs:
msg += f" {self.logs}"
return msg

@classmethod
def get_subclasses(cls) -> List[Type["ConversionException"]]:
subclasses = [cls]
Expand Down Expand Up @@ -100,9 +109,10 @@ class UnexpectedConversionError(ConversionException):

def exception_from_error_code(
error_code: int,
logs: Optional[str] = None,
) -> Union[ConversionException, ValueError]:
"""returns the conversion exception corresponding to the error code"""
for cls in ConversionException.get_subclasses():
if cls.error_code == error_code:
return cls()
return UnexpectedConversionError(f"Unknown error code '{error_code}'")
return cls(logs=logs)
return UnexpectedConversionError(f"Unknown error code '{error_code}', logs= {logs}")
28 changes: 25 additions & 3 deletions dangerzone/isolation_provider/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import signal
import subprocess
import sys
import tempfile
from abc import ABC, abstractmethod
from pathlib import Path
from typing import IO, Callable, Iterator, Optional
Expand Down Expand Up @@ -87,12 +88,19 @@ class IsolationProvider(ABC):
Abstracts an isolation provider
"""

def __init__(self) -> None:
if getattr(sys, "dangerzone_dev", False) is True:
def __init__(self, debug) -> None:
self.debug = debug
if self.should_display_errors():
self.proc_stderr = subprocess.PIPE
else:
# We do not trust STDERR from the inner container,
# which could be exploited to ask users to open the document with
# a default PDF reader for instance.
self.proc_stderr = subprocess.DEVNULL

def should_display_errors(self) -> bool:
return self.debug or getattr(sys, "dangerzone_dev", False)

@abstractmethod
def install(self) -> bool:
pass
Expand Down Expand Up @@ -220,6 +228,17 @@ def convert_with_proc(
text = "Successfully converted document"
self.print_progress(document, False, text, 100)

if self.should_display_errors():
assert p.stderr
debug_log = read_debug_text(p.stderr, MAX_CONVERSION_LOG_CHARS)
p.stderr.close()
log.debug(
"Conversion output (doc to pixels)\n"
f"{DOC_TO_PIXELS_LOG_START}\n"
f"{debug_log}" # no need for an extra newline here
f"{DOC_TO_PIXELS_LOG_END}"
)

def print_progress(
self, document: Document, error: bool, text: str, percentage: float
) -> None:
Expand Down Expand Up @@ -252,7 +271,10 @@ def get_proc_exception(
"Encountered an I/O error during document to pixels conversion,"
f" but the status of the conversion process is unknown (PID: {p.pid})"
)
return errors.exception_from_error_code(error_code)
logs = None
if self.debug:
logs = "".join([line.decode() for line in p.stderr.readlines()])
return errors.exception_from_error_code(error_code, logs=logs)

@abstractmethod
def get_max_parallel_conversions(self) -> int:
Expand Down
7 changes: 5 additions & 2 deletions dangerzone/isolation_provider/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def get_runtime() -> str:
return runtime

@staticmethod
def get_runtime_security_args() -> List[str]:
def get_runtime_security_args(debug: bool) -> List[str]:
"""Security options applicable to the outer Dangerzone container.
Our security precautions for the outer Dangerzone container are the following:
Expand Down Expand Up @@ -137,6 +137,9 @@ def get_runtime_security_args() -> List[str]:
security_args += ["--network=none"]
security_args += ["-u", "dangerzone"]

if debug:
security_args += ["-e", "RUNSC_DEBUG=1"]

return security_args

@staticmethod
Expand Down Expand Up @@ -250,7 +253,7 @@ def exec_container(
extra_args: List[str] = [],
) -> subprocess.Popen:
container_runtime = self.get_runtime()
security_args = self.get_runtime_security_args()
security_args = self.get_runtime_security_args(self.debug)
enable_stdin = ["-i"]
set_name = ["--name", name]
prevent_leakage_args = ["--rm"]
Expand Down

0 comments on commit 981fcd4

Please sign in to comment.