Filenames with backslashes causes a fatal error for Rex::Commands::Sync

[mrmuskrat]

Describe the bug

Rex terminates with errors (the error varies based on where it is thrown) if dealing with a filename containing backslashes.

Example:
[2023-09-20 11:16:14] INFO - Running task systemd on host
[2023-09-20 11:17:18] ERROR - Error executing task:
[2023-09-20 11:17:18] ERROR - Error running chmod 0755 /Users/mmusgrove/tmp/host/fs/etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants at /Users/mmusgrove/perl5/perlbrew/perls/perl-5.26.3/lib/site_perl/5.26.3/Rex/Interface/Fs/Base.pm line 149, <> line 145.

[2023-09-20 11:17:18] ERROR - 1 out of 1 task(s) failed:
[2023-09-20 11:17:18] ERROR - systemd failed on host
[2023-09-20 11:17:18] ERROR - Error running chmod 0755 /Users/mmusgrove/tmp/host/fs/etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants at /Users/mmusgrove/perl5/perlbrew/perls/perl-5.26.3/lib/site_perl/5.26.3/Rex/Interface/Fs/Base.pm line 149, <> line 145.

I was attempting to use rsync_down on /etc/systemd and it encountered this file:
[root@host ~]# ls -al /etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants/qemu-guest-agent.service
lrwxrwxrwx. 1 root root 48 Oct 20 2021 '/etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants/qemu-guest-agent.service' -> /usr/lib/systemd/system/qemu-guest-agent.service

The host/fs/etc/systemd/ directory exists with ~/tmp.

The problem seems to be that Rex::Interface::Fs::Base::_quotepath is not handling backslashes. I can fix it if I override that method as follows:

{
  # Allow /etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants to be synced
  no warnings 'redefine';
  *Rex::Interface::Fs::Base::_quotepath = sub {
    my ( $self, $p ) = @_;
    $p =~ s/([\\\@\$\% ])/\\$1/g; # Note the extra \\ before \@

    return $p;
  };
}

Expected behavior

I would expect the sync_down to function the same regardless of whether or not the filename contains a backslash character.

How to reproduce it

Modify the following Rexfile to use a valid hostname of a Linux system with qemu installed you can connect to with ssh
mkdir -p ~/tmp/host/fs/etc/systemd # where host is the same hostname
Place the Rexfile into ~/tmp.
cd ~/tmp
rex systemd

Code example

# Rexfile
use Rex -feature => ['1.4'];

group myservers => 'host';

desc 'Get /etc/systemd';
task 'systemd',
  group => 'myservers',
  sub {
    my $server = connection->server;
    sync_down "/etc/systemd", "$server/fs/etc/systemd/";
  };

Additional context

No response

Rex version

(R)?ex 1.14.3

Perl version

v5.26.3

Operating system running rex

Darwin Kernel Version 22.6.0: Wed Jul 5 22:21:53 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6020 arm64

Operating system managed by rex

AlmaLinux 8.8 (Sapphire Caracal)

How rex was installed?

package manager

[mrmuskrat]

If you do not have qemu you can simply create a file with a backslash as a part of the filename and then try transfer it with sync_down.

[ferki]

Thanks @mrmuskrat for the report!

I'm afraid there are two different issues involved here. One of them indeed related to backslashes in the filename, and the other one only looks like the backslash is a problem.

I expect mentioning sync_down explicitly in the issue title would also add a lot of clarity about the scope 😅

Control characters, like \x2 in the filename

The reported example fails on a chmod operation, suggesting that the file transfer itself is already considered done at that time, but the subsequent permission setting step is failing. I believe it would be useful to see the full debug output of such failure to learn more.

Could you post the debug output from rex -d ... as a file, please? Any sensitive info should be masked out, but please double check before posting.

I also wonder how I may be able to reproduce the exact same issue with filenames like these? Is the \x2 in the filename added by systemd, or qemu? Is the control character present on the source system in the first place?

Please also consider using our other support channels for a more interactive initial debugging experience (Matrix/IRC, or my Open Source Office Hours).

Plain backslash in filename

I have qemu on my systems, but I don't have systemd at hand. So I attempted to reproduce the issue by manually creating a /tmp/source/file\1 file, and run this task:

use Rex -feature => ['1.4'];

task 'test', sub {
    sync_down '/tmp/source/', '/tmp/dest/';
};

That also fails, although with a different problem depending on the connection context in effect:

• locally (effectively a cp operation): Error copying /tmp/source/file\1 -> /tmp/dest/file\1 at /home/ferki/.perlbrew/libs/perl-5.38.0@ferki/lib/perl5/Rex/Interface/Fs/Base.pm line 174.
• over an OpenSSH connection (transfer via Net::SFTP::Foreign): /tmp/source//file\1 is not readable. at /home/ferki/.perlbrew/libs/perl-5.38.0@ferki/lib/perl5/Rex/Commands/Download.pm line 129.

Summary

sync_down from Rex::Commands::Sync is indeed failing to sync a file with a \ in the filename. This will certainly be a great additional test case 👍

At the same time, the exact problem reported here does not seems to be related to \ directly, but to \x2 and/or similar control characters in the filename. That would also be a great test case 👍

Either way, I would like to gather more information (and then maybe even split into two separate issues if warranted).

Based on the information available, I agree that potential solutions would involve detecting the shells relevant for the sync operation in question and quote/escape the filenames and paths accordingly. Let's identify exact root causes first, so it's possible to write matching test cases.

Consider using better methods to sync

Please note that Rex::Commands::Sync is an escape hatch for situations when rsync is not available (e.g. when running on Windows or restricted systems). If you have rsync available, please try using sync from Rex::Commands::Rsync instead, or other purpose-made syncing methods (direct run q(rsync ...) commands, git push/pull, create/copy/extract tarball, etc.)

[ferki]

On a second look on an official Debian 12 image, it looks like the weirdly encoded character is more probably \x2d aka hyphen-minus, which makes more sense than \x2 (STX or start of text).

Apparently the problematic file mentioned in the original report somehow belongs to or related to qemu-guest-agent, but simply installing that package or running the qemu-guest-agent service does not create similar files or symlinks on my VM yet 🤔

I'll try to reproduce the original case more closely, but any hints how to end up in a comparable situation is still highly welcome.

[ferki]

Oh, I've found such a file at /usr/lib/systemd/system/system-systemd\x2dcryptsetup.slice. Trying to sync_down its whole parent directory, I got a failure too, but not the original message:

[2023-10-14 22:38:21] INFO - File /usr/lib/systemd/system//system-systemd\x2dcryptsetup.slice is not readable.
[2023-10-14 22:38:21] ERROR - Error executing task:
[2023-10-14 22:38:21] ERROR - /usr/lib/systemd/system//system-systemd\x2dcryptsetup.slice is not readable. at /home/ferki/.perlbrew/libs/perl-5.38.0@ferki/lib/perl5/Rex/Commands/Download.pm line 129.

The original error of

2023-09-20 11:17:18] ERROR - Error running chmod 0755 /Users/mmusgrove/tmp/host/fs/etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants at /Users/mmusgrove/perl5/perlbrew/perls/perl-5.26.3/lib/site_perl/5.26.3/Rex/Interface/Fs/Base.pm line 149, <> line 145.

indicates that it was trying to run chmod on the local file after the download step. I can't recreate that outcome yet :think: I feel that still indicates some missing bit, and it would be nice to clearly confirm that scenario.

I confirm that escaping \ in Rex::Interface::Fs::Base::_quotepath() seems to help finishing without an error (I did not investigate if the remote and synced content is in fact identical yet, though). I expect replacing that with a call to Net::OpenSSH::ShellQuoter would be able to handle even more corner cases.

My current hypothesis is that the readability check during preparing the download (_sftp_download in Rex::Commands::Download) fails in my case, but not in @mrmuskrat's original case. For me escaping makes the readability check pass too, and maybe in the originally reported case the same passes for some reason without the extra escaping, just to fail on the next chmod step.

[mrmuskrat]

@ferki Here's my debug log that I created today by running rex -d systemd with the Rexfile mentioned in the body of the ticket.

sync_down.log

I remember there was a reason for using sync_down instead of rsync but I don't remember what that reason was. I will reach out to the author to find out if he remembers.

[ferki]

Thanks for the debug log 👍

I think the most relevant addition info is here:

[2023-10-18 12:29:40] DEBUG - Getting fs stat from /etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants
[2023-10-18 12:29:40] DEBUG - Getting fs stat from /etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants/qemu-guest-agent.service
[2023-10-18 12:29:41] DEBUG - Creating directory host/fs/etc/systemd//system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants

indicating that the /etc/systemd/system/dev-virtio\x2dports-org.qemu.guest_agent.0.device.wants path involved in the chmod error is a directory, while I only tried to reproduce the issue with files so far.

I could successfully recreate the scenario when manually creating a directory with the problematic path name including \x2 in it 👍

Notes for implementors

Based on what I learned so far, both identified cases could be handled on this issue, probably no need to split into different cases after all.

Since currently there are no tests for sync functionality yet, working on this would require handling that first (=writing tests for the current behavior), and then adding a failing test case for this issue before solving. t/rsync.t might serve as inspiration and/or standardization opportunity (as noted in this comment).

Until then the possible workarounds are to:

• use other, more mature and reliable syncing methods, e.g. Rex::Commands::Rsync
• override Rex::Interface::Fs::Base::_quotepath() as seen at the start of this thread
• apply this patch:

diff --git a/lib/Rex/Interface/Fs/Base.pm b/lib/Rex/Interface/Fs/Base.pm
index e42b75fb..a0820394 100644
--- a/lib/Rex/Interface/Fs/Base.pm
+++ b/lib/Rex/Interface/Fs/Base.pm
@@ -199,7 +199,7 @@ sub _normalize_path {
 
 sub _quotepath {
   my ( $self, $p ) = @_;
-  $p =~ s/([\@\$\% ])/\\$1/g;
+  $p =~ s/([\\\@\$\% ])/\\$1/g;
 
   return $p;
 }