upload_and_run doesn't separate $command from first arg

[akarelas]

Describe the bug

When doing this:

my $output = upload_and_run <<~'BASH', 'foo';
#!/bin/bash
echo $1
BASH

...this produces an error, because it's trying to execute /tmp/${random_string}.tmpfoo on the server (observe that there's no space separating the command from the first argument).

Expected behavior

$output should equal the string foo

How to reproduce it

Write the code in the bug description inside a task, and see the error being output.

Code example

No response

Additional context

No response

Rex version

1.14.3

Perl version

v5.38.2

Operating system running rex

Void Linux

Operating system managed by rex

Debian

How rex was installed?

package manager

[akarelas]

I wonder whether we should use some shellQuoting function for the arguments, so that a single space string, or a string containing two words separated by a space, can be passed as an argument, for example.

[ferki]

Thanks for the report!

Apparent internal usage

First, I'd say Rex::Helper::Run::upload_and_run() is intended for internal Rex usage, though that does not make it impossible to (ab)use it for other purposes.

Reproducing the issue

Second, with the provided example, I get Odd number of elements in hash assignment. upload_and_run() takes arguments via its args option as an array reference, like:

use Rex;
use Rex::Helper::Run;

task 'test', sub {
    my $output = upload_and_run <<~'BASH', args => ['foo'];
#!/bin/bash
echo $1
BASH
};

With this I could reproduce the issue :+1:

I agree that the problem seems to be with a missing space between the command name and its list of arguments passed to it. I also agree if we are going to pass arguments, these should be quoted properly depending on the shell rules of the managed endpoint (this could be done by the user, though we could use Net::OpenSSH::ShellQuoter internally here.)

Hotfix

An admittedly naive hotfix could be:

diff --git a/lib/Rex/Helper/Run.pm b/lib/Rex/Helper/Run.pm
index 20d64b59..2799ba59 100644
--- a/lib/Rex/Helper/Run.pm
+++ b/lib/Rex/Helper/Run.pm
@@ -44,7 +44,7 @@ sub upload_and_run {
   }

   if ( exists $option{args} ) {
-    $command .= join( " ", @{ $option{args} } );
+    $command = join( " ", $command, @{ $option{args} } );
   }

   return i_run("$command 2>&1");

Workaround

Alternatively, do the upload and run separately on our own from a task, like:

file $my_tmp_file,
  content => <<~'BASH',
#!/bin/bash
echo $1
BASH
  mode => 0755;

run("$tmp_file foo");

In this case, it may not even have to be a temporary file, but can be deployed normally to /usr/local/bin or similar location as part of standard provisioning, instead of creating just-in-time (=this endpoints needs this script, let's make sure it's present.)

Initial thoughts on a fix

Since there are currently no tests for upload_and_run(), these should come first to define the expected behavior before jumping to any specific implementation, though. upload_and_run() does not seems to be designed for easy testability in its current form, so it may be more involved to come up with a good enough test strategy.

Using a test case based on echo may be fine for many different operating systems. I expect good coverage with a basic case:

• without any options
• using with
• using args
• perhaps using both with and args for sake of completeness

I expect such a set of tests would fail initially only due to this bug, and a follow-up commit can demonstrate a solid fix.