Accept files from stdin, write safe PDFs to stdout - #1528
Conversation
Remove the private _validate_* functions and their public wrapper functions that existed to work around a Click 7.x bug (issue #206). Click 7.x is no longer supported, so these wrappers are unnecessary. Promote the @errors.handle_document_errors-decorated functions directly to public names.
The ASCII art banner was printing to stdout, which would interfere with piping safe PDF data. Route all banner output to stderr instead, and update the corresponding test to read from stderr.
All human-readable status output (print_header, settings messages, OCR language validation, container image warnings, failed document display) now goes to stderr via click.echo(..., err=True), keeping stdout clean for potential PDF data output. Also use str(document) for failed doc display instead of document.input_filename, which is preparatory for data-based documents where input_filename would raise.
Extend the Document class to accept raw bytes via a new 'data' parameter instead of requiring a file path. This is the foundation for stdin support. - New 'data' parameter in __init__ (default None, backward-compatible) - Constructor requires either input_filename or data - New Document.from_stdin() classmethod reads from sys.stdin.buffer - New open() method returns BinaryIO (file or BytesIO) - New write() method writes to stdout or output file - New sanitized_input_filename property - announce_id() handles both file and stdin documents - __eq__, __hash__, __str__ handle data-based documents - Add DangerzoneCore.add_document_from_stdin() - Update tests for new constructor requirements
Decouple the isolation provider from file-based I/O by using the Document abstraction methods: - document.open() instead of open(document.input_filename, 'rb') - document.write() instead of safe_doc.save() + os.replace() This enables the provider to work with data-based documents (stdin) without modification.
Implement stdin/stdout I/O for the Dangerzone CLI. - Add _initialize_documents() handling stdin detection, validation, and document creation - Accept '-' as a filename to explicitly read from stdin - Accept implicit stdin when no filenames given and stdin is piped - Write safe PDF to stdout when no --output-filename is specified - Refuse to write to a terminal without --output-filename - Reject --archive with stdin input - Update @click.command() help text mentioning stdin - Add STDIO_DESCRIPTOR constant and skip validation for '-' - Add TestCliIO test class covering all stdin/stdout scenarios Fixes #1522
5d37a5a to
d5e3c95
Compare
almet
left a comment
almet
left a comment
There was a problem hiding this comment.
Amazing, thanks :-) I've stumbled onto this need in the past when building a web interface on top of DZ, and I can clearly see how that can be useful in general.
I'm requesting some changes on this PR, but it's headed the right direction! Thanks.
| safe_doc.save(document.sanitized_output_filename) | ||
| os.replace(document.sanitized_output_filename, document.output_filename) | ||
| # Write the safe PDF to the document's destination. | ||
| document.write(safe_doc.tobytes()) |
There was a problem hiding this comment.
Does this changes the default CLI user experience?
I believe we should output the files somewhere the user has specified, and if nothing was specified, error out and ask where this should be.
Presenting the user with the contents of the file should be done only if it was asked explicitely, e.g. by setting input and output to - (to mean it's should be read and written to a STDIN and STDOUT)
There was a problem hiding this comment.
Ok, I've made the input from stdin and output to stdout a conscious user choice in 6b065a6. This means that:
dangerzone-cli < file.pdf > safe.pdf
becomes:
dangerzone-cli - -o - < file.pdf > safe.pdf
which is more dash-y, but ultimately fine.
Note that with the above change, we no longer check for TTYs, since this no longer happens implicitly.
There was a problem hiding this comment.
If you'd like to get a less dashy experience, it's also possible to have a --stdio argument, for instance.
There was a problem hiding this comment.
Hm, people may be more familiar with the dashes than with a custom --stdio flag, so I'm ultimately ok with it.
| "Accepts file paths as arguments, or reads from stdin when no\n" | ||
| "files are given (or when '{args.STDIO_DESCRIPTOR}' is passed as a filename).\n" | ||
| "When reading from stdin, the safe PDF is written to stdout unless\n" | ||
| "--output-filename is specified. All status output goes to stderr." |
There was a problem hiding this comment.
Can this be added in the docstring instead? (Click picks it up)
There was a problem hiding this comment.
This was done on purpose actually (also, I think Click shows the docstring as a help message as well). I wanted a quick way to explain how to use dangerzone-cli, for those who encounter it for the first time. To better explain my end goal, I've rephrased and added extra stuff in 3391a26. It now looks like this:
[user@dangerzone-dev dangerzone]$ poetry run dangerzone-cli --help
Usage: dangerzone-cli [OPTIONS] [FILENAMES]...
Convert potentially dangerous documents to safe PDFs.
Documents are converted inside a sandbox, so that any embedded threats are
neutralized. You can also use OCR to add a searchable text layer to the safe
PDF, via the --ocr-lang option.
Pass one or more file paths as arguments, or use '-' to read a document from
standard input. For single-document conversions, you can write the safe PDF
to a file with -o <file>, or to standard output with -o -.
Examples:
dangerzone-cli evidence.odt # create evidence-safe.pdf
dangerzone-cli doc1.pdf doc2.docx # create doc1-safe.pdf and doc2-safe.pdf
dangerzone-cli --archive report.docx # create report-safe.pdf and move the original under 'unsafe/'
dangerzone-cli --ocr-lang eng scan.pdf # add a searchable text layer
dangerzone-cli -o safe.pdf evidence.odt # create safe.pdf
dangerzone-cli -o - evidence.odt > safe.pdf # write the safe PDF to stdout
cat evidence.pdf | dangerzone-cli - -o safe.pdf # read from stdin and write to safe.pdf
cat in.pdf | dangerzone-cli - -o - > out.pdf # read from stdin to write to stdout
It's probably a little verbose and I can drop some examples, but I mostly want to know if you find this valuable.
There was a problem hiding this comment.
Can you clarify why using docstrings didn't work? From my perspective, it looks like a way to handle the spacing and returns automatically, but I might be missing something.
Otherwise, I really like the idea of having documentation in the CLI like that, with examples it's great :-)
The example that reads "# write the safe PDF to stdout" should probably be renamed to "use unix pipes to convert to safe.pdf", or we should remove the last >.
There was a problem hiding this comment.
Can you clarify why using docstrings didn't work? From my perspective, it looks like a way to handle the spacing and returns automatically, but I might be missing something.
Oh I'm sorry! When I read your initial message, I thought that you didn't want the examples in the help message, and wanted to move them in the docstring (hence my comment).
I agree, I've moved them to the docstring and tweaked a bit the sentence you pointed out, as well as the others. See a1a8d3e.
0cad265 to
91eb8e4
Compare
91eb8e4 to
a1a8d3e
Compare
2 participants
Dangerzone CLI now accepts documents via stdin and writes safe PDFs to stdout, following standard Unix pipe conventions:
All status output (banner, progress, errors) goes to stderr so stdout stays clean for PDF data. We also take extra care to detect when we can read from stdin and when we can write to stdout, so that we don't mess with the user's TTY.
Fixes #1522
Refs #206