QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
526 stars 46 forks source link

qvm-run-vm eats command arguments #9123

Open BetoHydroxyButyrate opened 2 months ago

BetoHydroxyButyrate commented 2 months ago

How to file a helpful issue

Qubes OS release

4.2

Brief summary

According to the usage: Usage: /usr/bin/qvm-run-vm [OPTIONS] vmname command arguments However, any arguments in arguments are parsed by qvm-run-vm, rather than being passed to command.

Steps to reproduce

% qvm-run-vm passage-banking ps -lax

Expected behavior

I expect the ps -lax command to be issued on the passage-banking AppVm. The only manner to achieve this is to quote everything:

% qvm-run-vm passage-banking 'ps -lax' | tail -2
0  1000    4934    4932  20   0   8932  2688 pipe_r S    ?          0:00 logger -t qubes.VMShell+-beto-devel
0  1000    4935    4931  20   0   5612  1536 pipe_r S    ?          0:00 cat

Actual behavior

% qvm-run-vm passage-banking ps -lax
/usr/bin/qvm-run-vm: invalid option -- 'l'
/usr/bin/qvm-run-vm: invalid option -- 'a'
/usr/bin/qvm-run-vm: invalid option -- 'x'
Usage: /usr/bin/qvm-run-vm [OPTIONS] vmname command arguments
rustybird commented 2 months ago

Since these are not options for qvm-run-vm, you have to use the standard end-of-options delimiter:

qvm-run-vm -- passage-banking ps -lax

Although IMO this syntax ...

qvm-run-vm passage-banking 'ps -lax'

... is clearer at the moment: Because in either case, the destination VM is not going to get an argv but a Bash code fragment. With the first syntax, qvm-run-vm will actually just glue the arguments together with spaces to produce this Bash code fragment, which although documented in the usage message is very weird (and should be replaced by qubes.VMExec like it was for qvm-run in dom0).

rustybird commented 2 months ago

Since these are not options for qvm-run-vm, you have to use the standard end-of-options delimiter

For convenience, qvm-run-vm could avoid this by changing getopt -o htd to getopt -o +htd I think

BetoHydroxyButyrate commented 2 months ago

Since these are not options for qvm-run-vm, you have to use the standard end-of-options delimiter:

qvm-run-vm -- passage-banking ps -lax

Although IMO this syntax ...

qvm-run-vm passage-banking 'ps -lax'

... is clearer at the moment: Because in either case, the destination VM is not going to get an argv but a Bash code fragment. With the first syntax, qvm-run-vm will actually just glue the arguments together with spaces to produce this Bash code fragment, which although documented in the usage message is very weird (and should be replaced by qubes.VMExec like it was for qvm-run in dom0).

I disagree. There is nothing in the usage string Usage: /usr/bin/qvm-run-vm [OPTIONS] vmname command arguments to suggest that. It clearly implies that I can pass a null-set of [OPTIONS] and then command arguments, but no, it gets confused, This is a bug.

rustybird commented 2 months ago

Yes, on second thought having to use the end-of-options delimiter does seem more like a bug than a mere inconvenience. I don't want to advocate GNU-ish "options can also come after non-option arguments" semantics for this specific tool.

However, if you dislike rules lawyering leading to counterintuitive outcomes then (just to reiterate this, as an aside:) it really really is best to avoid the separate arguments syntax altogether at the moment, except maybe for trivial invocations. In the current implementation it's a footgun for anyone who hasn't thoroughly pondered the implications of the usage message saying that arguments are joined with spaces and passed to "bash -c". The single string syntax at least makes it more obvious that the command is Bash code.