Fix print preflight status check

[rocodes]

Status

Ready for review

Description

Fixes #2101

Test Plan

• Visual review
• CI
• Print success

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

• I have tested these changes in the appropriate Qubes environment
• I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
• These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

• I have updated the AppArmor profile
• No update to the AppArmor profile is required for these changes
• I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

• I have written a migration and upgraded a test database based on main and confirmed that the migration is self-contained and applies cleanly
• I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
• I need help writing a database migration
• No database schema changes are needed

[rocodes]

Tested on Qubes 4.2.2-rc2 + tip of this branch and these changes allow successful printing

[cfm]

Looks good, @rocodes! Nice that the "no surprising return values" and the "no untrusted input" goals converge here. I'll approve and merge based on visual review since (a) you've tested this (and I'm not set up to do so) and (b) this will be retested via freedomofpress/securedrop-workstation#1103.

Noting for future work: could this pattern be factored out into some sort of stderr_to_enum(stderr, Enum) helper so that it's easy to do the right thing for both goals?

[legoktm]

It would be good to document what could come before this and why we're discarding it.

[rocodes]

Yeah that's fair. It's like

securedrop-client/client/securedrop_client/export.py

Lines 211 to 212 in 937483a

	# This is a bit messy, but make sure we are just taking the last line
	# (no-op if no newline, since we already stripped whitespace above)

(comment farther down in the file) so I was a bit sparse on the documentation. Basically, we know that our status string is the last thing written to stderr, but because we flush stderr there could be other stuff written beforehand (by the OS/another process, not by us) that we don't want to pass on to the calling VM. The errors are written to the appropriate log (or to the journal) in sd-devices, but here, we're only interested in the status value.

[deeplow]

Noting for future work: could this pattern be factored out into some sort of stderr_to_enum(stderr, Enum) helper so that it's easy to do the right thing for both goals?

@cfm not sure if this is similar to what you're talking about but on Dangerzone we are using exceptions instead of Enums. Each exception is mapped to a certain qrexec server process exit code. Then the have one method which reconstructs the exceptions on the client (trusted part), based on the exit codes. This way we basically eliminate any client-side parsing.