[cdc_rsync] [cdc_stream] Switch from scp to sftp (#66)

Use sftp for deploying remote components instead of scp. sftp has the
advantage that it can also create directries, chmod files etc., so
that we can do everything in one call of sftp instead of mixing scp
and ssh calls.

The downside of sftp is that it can't switch to ~ resp. %userprofile%
for the remote side, and we have to assume that sftp starts in the
user's home dir. This is the default and works on my machines!

cdc_rsync and cdc_stream check the CDC_SFTP_COMMAND env var now and
accept --sftp-command flags. If they are not set, the corresponding
scp flag and env var is still used, with scp replaced by sftp. This is
most likely correct as sftp and scp usually reside in the same
directory and share largely identical parameters.

README.md

@@ -191,7 +191,7 @@ To build the tools from source, the following steps have to be executed on
   ```
 
 Finally, install an SSH client on the Windows device if not present.
-The file transfer tools require `ssh.exe` and `scp.exe`.
+The file transfer tools require `ssh.exe` and `sftp.exe`.
 
 ## Building
 
@@ -227,24 +227,24 @@ The two tools can be built and used independently.
 
 ## Usage
 
-The tools require a setup where you can use SSH and SCP from the Windows machine
+The tools require a setup where you can use SSH and SFTP from the Windows
-to the Linux device without entering a password, e.g. by using key-based
+machine to the Linux device without entering a password, e.g. by using key-based
 authentication.
 
-### Configuring SSH and SCP
+### Configuring SSH and SFTP
 
-By default, the tools search `ssh.exe` and `scp.exe` from the path environment
+By default, the tools search `ssh.exe` and `sftp.exe` from the path environment
 variable. If you can run the following commands in a Windows cmd without
 entering your password, you are all set:
 ```
 ssh user@linux.device.com
-scp somefile.txt user@linux.device.com:
+sftp user@linux.device.com
 ```
 Here, `user` is the Linux user and `linux.device.com` is the Linux host to
 SSH into or copy the file to.
 
 If additional arguments are required, it is recommended to provide an SSH config
-file. By default, both `ssh.exe` and `scp.exe` use the file at
+file. By default, both `ssh.exe` and `sftp.exe` use the file at
 `%USERPROFILE%\.ssh\config` on Windows, if it exists. A possible config file
 that sets a username, a port, an identity file and a known host file could look
 as follows:
@@ -256,21 +256,21 @@ Host linux_device
 	IdentityFile C:\path\to\id_rsa
 	UserKnownHostsFile C:\path\to\known_hosts
 ```
-If `ssh.exe` or `scp.exe` cannot be found, you can specify the full paths via
+If `ssh.exe` or `sftp.exe` cannot be found, you can specify the full paths via
-the command line arguments `--ssh-command` and `--scp-command` for `cdc_rsync`
+the command line arguments `--ssh-command` and `--sftp-command` for `cdc_rsync`
 and `cdc_stream start` (see below), or set the environment variables
-`CDC_SSH_COMMAND` and `CDC_SCP_COMMAND`, e.g.
+`CDC_SSH_COMMAND` and `CDC_SFTP_COMMAND`, e.g.
 ```
 set CDC_SSH_COMMAND="C:\path with space\to\ssh.exe"
-set CDC_SCP_COMMAND="C:\path with space\to\scp.exe"
+set CDC_SFTP_COMMAND="C:\path with space\to\sftp.exe"
 ```
 Note that you can also specify SSH configuration via the environment variables
 instead of using a config file:
 ```
 set CDC_SSH_COMMAND=C:\path\to\ssh.exe -p 12345 -i C:\path\to\id_rsa -oUserKnownHostsFile=C:\path\to\known_hosts
-set CDC_SCP_COMMAND=C:\path\to\scp.exe -P 12345 -i C:\path\to\id_rsa -oUserKnownHostsFile=C:\path\to\known_hosts
+set CDC_SFTP_COMMAND=C:\path\to\sftp.exe -P 12345 -i C:\path\to\id_rsa -oUserKnownHostsFile=C:\path\to\known_hosts
 ```
-Note the small `-p` for `ssh.exe` and the capital `-P` for `scp.exe`.
+Note the small `-p` for `ssh.exe` and the capital `-P` for `sftp.exe`.
 
 #### Google Specific
 
@@ -278,7 +278,7 @@ For Google internal usage, set the following environment variables to enable SSH
 authentication using a Google security key:
 ```
 set CDC_SSH_COMMAND=C:\gnubby\bin\ssh.exe
-set CDC_SCP_COMMAND=C:\gnubby\bin\scp.exe
+set CDC_SFTP_COMMAND=C:\gnubby\bin\sftp.exe
 ```
 Note that you will have to touch the security key multiple times during the
 first run. Subsequent runs only require a single touch.
@@ -352,7 +352,7 @@ instead of to the file.
 `cdc_rsync` always logs to the console. To increase log verbosity, pass `-vvv`
 for debug logs or `-vvvv` for verbose logs.
 
-For both sync and stream, the debug logs contain all SSH and SCP commands that
+For both sync and stream, the debug logs contain all SSH and SFTP commands that
 are attempted to run, which is very useful for troubleshooting. If a command
 fails unexpectedly, copy it and run it in isolation. Pass `-vv` or `-vvv` for
 additional debug output.

cdc_rsync/cdc_rsync_client.cc

@@ -109,8 +109,8 @@ CdcRsyncClient::CdcRsyncClient(const Options& options,
   if (!options_.ssh_command.empty()) {
     remote_util_.SetSshCommand(options_.ssh_command);
   }
-  if (!options_.scp_command.empty()) {
+  if (!options_.sftp_command.empty()) {
-    remote_util_.SetScpCommand(options_.scp_command);
+    remote_util_.SetSftpCommand(options_.sftp_command);
   }
 }
 
@@ -413,26 +413,10 @@ absl::Status CdcRsyncClient::DeployServer(const ServerArch& arch) {
   }
   printer_.Print(deploy_msg, true, Util::GetConsoleWidth());
 
-  // scp cdc_rsync_server to a temp location on the gamelet.
+  // sftp cdc_rsync_server to the target.
-  std::string remoteServerPath =
+  std::string commands = arch.GetDeploySftpCommands();
-      arch.RemoteToolsBinDir(ServerArch::UseCase::kScp) +
+  RETURN_IF_ERROR(remote_util_.Sftp(commands, exe_dir, /*compress=*/false),
-      arch.CdcServerFilename();
+                  "Failed to deploy cdc_rsync_server");
-  std::string remoteServerTmpPath = remoteServerPath + Util::GenerateUniqueId();
-  std::string localServerPath = path::Join(exe_dir, arch.CdcServerFilename());
-  status = remote_util_.Scp({localServerPath}, remoteServerTmpPath,
-                            /*compress=*/true);
-  if (!status.ok()) {
-    return WrapStatus(status, "Failed to copy cdc_rsync_server to instance");
-  }
-
-  // Replace cdc_rsync_server file by the temp file.
-  std::string replace_cmd =
-      arch.GetDeployReplaceCommand(remoteServerPath, remoteServerTmpPath);
-  status = remote_util_.Run(replace_cmd, "replace");
-  if (!status.ok()) {
-    return WrapStatus(status,
-                      "Failed to replace old cdc_rsync_server by new one");
-  }
 
   return absl::OkStatus();
 }

cdc_rsync/cdc_rsync_client.h

@@ -54,10 +54,14 @@ class CdcRsyncClient {
     int forward_port_first = 44450;
     int forward_port_last = 44459;
     std::string ssh_command;
-    std::string scp_command;
+    std::string sftp_command;
     std::string sources_dir;  // Base dir for files loaded for --files-from.
     PathFilter filter;
 
+    // Backwards compatibility for switching from scp to sftp.
+    // Used internally, do not use.
+    std::string deprecated_scp_command;
+
     // Compression level 0 is invalid.
     static constexpr int kMinCompressLevel = -5;
     static constexpr int kMaxCompressLevel = 22;

cdc_rsync/params.cc

@@ -21,6 +21,7 @@
 #include "absl/strings/str_split.h"
 #include "common/path.h"
 #include "common/port_range_parser.h"
+#include "common/remote_util.h"
 #include "lib/zstd.h"
 
 namespace cdc_ft {
@@ -75,9 +76,9 @@ Options:
     --ssh-command <cmd>     Path and arguments of ssh command to use, e.g.
                             "C:\path\to\ssh.exe -p 12345 -i id_rsa -oUserKnownHostsFile=known_hosts"
                             Can also be specified by the CDC_SSH_COMMAND environment variable.
-    --scp-command <cmd>     Path and arguments of scp command to use, e.g.
+    --sftp-command <cmd>    Path and arguments of sftp command to use, e.g.
-                            "C:\path\to\scp.exe -P 12345 -i id_rsa -oUserKnownHostsFile=known_hosts"
+                            "C:\path\to\sftp.exe -P 12345 -i id_rsa -oUserKnownHostsFile=known_hosts"
-                            Can also be specified by the CDC_SCP_COMMAND environment variable.
+                            Can also be specified by the CDC_SFTP_COMMAND environment variable.
     --forward-port <port>   TCP port or range used for SSH port forwarding (default: 44450-44459).
                             If a range is specified, searches for available ports (slower).
 -h  --help                  Help for cdc_rsync
@@ -85,12 +86,15 @@ Options:
 
 constexpr char kSshCommandEnvVar[] = "CDC_SSH_COMMAND";
 constexpr char kScpCommandEnvVar[] = "CDC_SCP_COMMAND";
+constexpr char kSftpCommandEnvVar[] = "CDC_SFTP_COMMAND";
 
 // Populates some parameters from environment variables.
 void PopulateFromEnvVars(Parameters* parameters) {
   path::GetEnv(kSshCommandEnvVar, &parameters->options.ssh_command)
       .IgnoreError();
-  path::GetEnv(kScpCommandEnvVar, &parameters->options.scp_command)
+  path::GetEnv(kScpCommandEnvVar, &parameters->options.deprecated_scp_command)
+      .IgnoreError();
+  path::GetEnv(kSftpCommandEnvVar, &parameters->options.sftp_command)
       .IgnoreError();
 }
 
@@ -287,8 +291,15 @@ OptionResult HandleParameter(const std::string& key, const char* value,
   }
 
   if (key == "scp-command") {
+    // Backwards compatibility. Note that this flag is hidden from the help.
     if (!ValidateValue(key, value)) return OptionResult::kError;
-    params->options.scp_command = value;
+    params->options.deprecated_scp_command = value;
+    return OptionResult::kConsumedKeyValue;
+  }
+
+  if (key == "sftp-command") {
+    if (!ValidateValue(key, value)) return OptionResult::kError;
+    params->options.sftp_command = value;
     return OptionResult::kConsumedKeyValue;
   }
 
@@ -491,6 +502,27 @@ bool Parse(int argc, const char* const* argv, Parameters* parameters) {
 
   PopUserHost(&parameters->destination, &parameters->user_host);
 
+  // Backwards compabitility after switching to sftp. Convert scp to sftp
+  // command Note that this flag is hidden from the help.
+  if (parameters->options.sftp_command.empty() &&
+      !parameters->options.deprecated_scp_command.empty()) {
+    LOG_WARNING(
+        "The CDC_SCP_COMMAND environment variable and the --scp-command flag "
+        "are deprecated. Please set CDC_SFTP_COMMAND or --sftp-command "
+        "instead.");
+
+    parameters->options.sftp_command = RemoteUtil::ScpToSftpCommand(
+        parameters->options.deprecated_scp_command);
+    if (!parameters->options.sftp_command.empty()) {
+      LOG_WARNING("Converted scp command '%s' to sftp command '%s'.",
+                  parameters->options.deprecated_scp_command,
+                  parameters->options.sftp_command);
+    } else {
+      LOG_WARNING("Failed to convert scp command '%s' to sftp command.",
+                  parameters->options.deprecated_scp_command);
+    }
+  }
+
   if (!ValidateParameters(*parameters, help)) {
     return false;
   }

cdc_rsync/params_test.cc

@@ -32,6 +32,10 @@ constexpr char kUserHostDst[] = "user@host:destination";
 constexpr char kUserHost[] = "user@host";
 constexpr char kDst[] = "destination";
 
+constexpr char kSshCommandEnvVar[] = "CDC_SSH_COMMAND";
+constexpr char kScpCommandEnvVar[] = "CDC_SCP_COMMAND";
+constexpr char kSftpCommandEnvVar[] = "CDC_SFTP_COMMAND";
+
 class TestLog : public Log {
  public:
   explicit TestLog() : Log(LogLevel::kInfo) {}
@@ -60,6 +64,11 @@ class ParamsTest : public ::testing::Test {
   void TearDown() override {
     std::cout.rdbuf(prev_stdout_);
     std::cerr.rdbuf(prev_stderr_);
+
+    // Clear env. They seem to be sticky sometimes and leak into other tests.
+    path::SetEnv(kSshCommandEnvVar, "");
+    path::SetEnv(kScpCommandEnvVar, "");
+    path::SetEnv(kSftpCommandEnvVar, "");
   }
 
  protected:
@@ -152,24 +161,57 @@ TEST_F(ParamsTest, ParseFailsOnContimeoutEqualsNoValue) {
   ExpectError(NeedsValueError("contimeout"));
 }
 
-TEST_F(ParamsTest, ParseSucceedsWithSshScpCommands) {
+TEST_F(ParamsTest, ParseSucceedsWithSshSftpCommands) {
-  const char* argv[] = {"cdc_rsync.exe",        kSrc,
+  const char* argv[] = {
-                        kUserHostDst,           "--ssh-command=sshcmd",
+      "cdc_rsync.exe",          kSrc, kUserHostDst, "--ssh-command=sshcmd",
-                        "--scp-command=scpcmd", NULL};
+      "--sftp-command=sftpcmd", NULL};
   EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
-  EXPECT_EQ(parameters_.options.scp_command, "scpcmd");
+  EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd");
   EXPECT_EQ(parameters_.options.ssh_command, "sshcmd");
 }
 
-TEST_F(ParamsTest, ParseSucceedsWithSshScpCommandsByEnvVars) {
+TEST_F(ParamsTest, ParseSucceedsWithSshSftpCommandsByEnvVars) {
-  EXPECT_OK(path::SetEnv("CDC_SSH_COMMAND", "sshcmd"));
+  EXPECT_OK(path::SetEnv(kSshCommandEnvVar, "sshcmd"));
-  EXPECT_OK(path::SetEnv("CDC_SCP_COMMAND", "scpcmd"));
+  EXPECT_OK(path::SetEnv(kSftpCommandEnvVar, "sftpcmd"));
   const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, NULL};
   EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
-  EXPECT_EQ(parameters_.options.scp_command, "scpcmd");
+  EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd");
   EXPECT_EQ(parameters_.options.ssh_command, "sshcmd");
 }
 
+TEST_F(ParamsTest, ParseSucceedsWithScpCommandFallback) {
+  const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst,
+                        "--scp-command=C:\\scp.exe foo", NULL};
+  EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
+  EXPECT_EQ(parameters_.options.sftp_command, "C:\\sftp.exe foo");
+}
+
+TEST_F(ParamsTest, ParseSucceedsWithScpCommandFallbackByEnvVar) {
+  EXPECT_OK(path::SetEnv(kScpCommandEnvVar, "C:\\scp.exe foo"));
+  const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, NULL};
+  EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
+  EXPECT_EQ(parameters_.options.sftp_command, "C:\\sftp.exe foo");
+}
+
+TEST_F(ParamsTest, ParseSucceedsWithSftpOverwritingScp) {
+  const char* argv[] = {"cdc_rsync.exe",
+                        kSrc,
+                        kUserHostDst,
+                        "--scp-command=C:\\scp.exe foo",
+                        "--sftp-command=sftpcmd",
+                        NULL};
+  EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
+  EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd");
+}
+
+TEST_F(ParamsTest, ParseSucceedsWithSftpEnvVarOverwritingScp) {
+  EXPECT_OK(path::SetEnv(kSftpCommandEnvVar, "sftpcmd"));
+  const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst,
+                        "--scp-command=C:\\scp.exe foo", NULL};
+  EXPECT_TRUE(Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
+  EXPECT_EQ(parameters_.options.sftp_command, "sftpcmd");
+}
+
 TEST_F(ParamsTest, ParseSucceedsWithNoSshCommand) {
   const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst,
                         "--ssh-command=", NULL};
@@ -178,12 +220,12 @@ TEST_F(ParamsTest, ParseSucceedsWithNoSshCommand) {
   ExpectError(NeedsValueError("ssh-command"));
 }
 
-TEST_F(ParamsTest, ParseSucceedsWithNoScpCommand) {
+TEST_F(ParamsTest, ParseSucceedsWithNoSftpCommand) {
-  const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, "--scp-command",
+  const char* argv[] = {"cdc_rsync.exe", kSrc, kUserHostDst, "--sftp-command",
                         NULL};
   EXPECT_FALSE(
       Parse(static_cast<int>(std::size(argv)) - 1, argv, &parameters_));
-  ExpectError(NeedsValueError("scp-command"));
+  ExpectError(NeedsValueError("sftp-command"));
 }
 
 TEST_F(ParamsTest, ParseFailsOnNoUserHost) {

cdc_rsync/server_arch.cc

@@ -18,8 +18,10 @@
 
 #include "absl/strings/match.h"
 #include "absl/strings/str_format.h"
+#include "absl/strings/str_split.h"
 #include "common/path.h"
 #include "common/remote_util.h"
+#include "common/util.h"
 
 namespace cdc_ft {
 
@@ -72,19 +74,13 @@ std::string ServerArch::CdcServerFilename() const {
   }
 }
 
-std::string ServerArch::RemoteToolsBinDir(UseCase use_case) const {
+std::string ServerArch::RemoteToolsBinDir() const {
   switch (type_) {
     case Type::kWindows: {
-      // TODO(ljusten): Unfortunately, scp doesn't seem to support shell var
+      return "AppData\\Roaming\\cdc-file-transfer\\bin\\";
-      // expansion, so %AppData% can't be used. This relies on scp copying
-      // files relative to %UserProfile% and %AppData% mapping to
-      // "AppData\\Roaming" relative to that.
-      std::string app_data =
-          use_case == UseCase::kScp ? "AppData\\Roaming" : "$env:appdata";
-      return app_data + "\\cdc-file-transfer\\bin\\";
     }
     case Type::kLinux:
-      return "~/.cache/cdc-file-transfer/bin/";
+      return ".cache/cdc-file-transfer/bin/";
     default:
       assert(!kErrorArchTypeUnhandled);
       return std::string();
@@ -93,11 +89,7 @@ std::string ServerArch::RemoteToolsBinDir(UseCase use_case) const {
 
 std::string ServerArch::GetStartServerCommand(int exit_code_not_found,
                                               const std::string& args) const {
-  std::string server_dir = RemoteToolsBinDir(UseCase::kSsh);
+  std::string server_path = RemoteToolsBinDir() + CdcServerFilename();
-  std::string server_path =
-      RemoteUtil::QuoteForSsh(server_dir + CdcServerFilename());
-  path::EnsureDoesNotEndWithPathSeparator(&server_dir);
-  server_dir = RemoteUtil::QuoteForSsh(server_dir);
 
   switch (type_) {
     case Type::kWindows:
@@ -106,53 +98,50 @@ std::string ServerArch::GetStartServerCommand(int exit_code_not_found,
       // a minor issue and means we display "Deploying server..." instead of
       // "Server not deployed. Deploying...";
       return RemoteUtil::QuoteForWindows(
-          absl::StrFormat("Set-StrictMode -Version 2; "
+          absl::StrFormat("powershell -Command \" "
+                          "Set-StrictMode -Version 2; "
                           "$ErrorActionPreference = 'Stop'; "
-                          "$server_dir = %s; "
+                          "if (-not (Test-Path -Path '%s')) { "
-                          "$server_path = %s; "
-                          "$u=New-Item $server_dir -ItemType Directory -Force; "
-                          "if (-not (Test-Path -Path $server_path)) { "
                           "  exit %i; "
                           "} "
-                          "& $server_path %s",
+                          "%s %s "
-                          server_dir, server_path, exit_code_not_found, args));
+                          "\"",
+                          server_path, exit_code_not_found, server_path, args));
     case Type::kLinux:
-      return absl::StrFormat(
+      return absl::StrFormat("if [ ! -f %s ]; then exit %i; fi; %s %s",
-          "mkdir -p %s; if [ ! -f %s ]; then exit %i; fi; %s %s", server_dir,
+                             server_path, exit_code_not_found, server_path,
-          server_path, exit_code_not_found, server_path, args);
+                             args);
     default:
       assert(!kErrorArchTypeUnhandled);
       return std::string();
   }
 }
 
-std::string ServerArch::GetDeployReplaceCommand(
+std::string ServerArch::GetDeploySftpCommands() const {
-    const std::string& old_server_path,
+  std::string commands;
-    const std::string& new_server_path) const {
-  const std::string old_path = RemoteUtil::QuoteForSsh(old_server_path);
-  const std::string new_path = RemoteUtil::QuoteForSsh(new_server_path);
 
-  switch (type_) {
+  // Create the remote tools bin dir if it doesn't exist yet.
-    case Type::kWindows:
+  // This assumes that sftp's remote startup directory is the home directory.
-      return RemoteUtil::QuoteForWindows(absl::StrFormat(
+  const std::string server_dir = path::ToUnix(RemoteToolsBinDir());
-          "Set-StrictMode -Version 2; "
+  std::vector<std::string> dir_parts =
-          "$ErrorActionPreference = 'Stop'; "
+      absl::StrSplit(server_dir, '/', absl::SkipEmpty());
-          "$old_path = %s; "
+  for (const std::string& dir : dir_parts) {
-          "$new_path = %s; "
+    // Use -mkdir to ignore errors if the directory already exists.
-          "if (Test-Path -Path $old_path) { "
+    commands += absl::StrFormat("-mkdir %s\ncd %s\n", dir, dir);
-          "  Get-Item -Path $old_path | "
-          "    Set-ItemProperty -Name IsReadOnly -Value $false; "
-          "} "
-          "Move-Item -Path $new_path -Destination $old_path -Force",
-          old_path, new_path));
-    case Type::kLinux:
-      return absl::StrFormat(
-          "([ ! -f %s ] || chmod u+w %s) && chmod a+x %s && mv %s %s", old_path,
-          old_path, new_path, new_path, old_path);
-    default:
-      assert(!kErrorArchTypeUnhandled);
-      return std::string();
   }
+
+  // Copy the server binary to a temp location. This assumes that sftp's local
+  // startup directory is cdc_rsync's exe dir.
+  const std::string server_file = CdcServerFilename();
+  const std::string server_temp_file = server_file + Util::GenerateUniqueId();
+  commands += absl::StrFormat("put %s %s\n", server_file, server_temp_file);
+
+  // Restore permissions in case they changed and propagate temp file.
+  commands += absl::StrFormat("-chmod 755 %s\n", server_file);
+  commands += absl::StrFormat("chmod 755 %s\n", server_temp_file);
+  commands += absl::StrFormat("rename %s %s\n", server_temp_file, server_file);
+
+  return commands;
 }
 
 }  // namespace cdc_ft

cdc_rsync/server_arch.h

@@ -29,8 +29,6 @@ class ServerArch {
     kWindows = 1,
   };
 
-  enum class UseCase { kSsh = 0, kScp = 1 };
-
   // Detects the architecture type based on the destination path, e.g. path
   // starting with C: indicate Windows.
   static Type Detect(const std::string& destination);
@@ -42,16 +40,10 @@ class ServerArch {
   std::string CdcServerFilename() const;
 
   // Returns the arch-specific directory where cdc_rsync_server is deployed.
-  // On Windows, |use_case| determines what type of env variables to use:
+  std::string RemoteToolsBinDir() const;
-  // - kSsh uses $env:appdata and works for ssh commands.
-  // - kScp uses AppData\\Roaming and works for scp commands.
-  // On Linux, this flag is ignored.
-  std::string RemoteToolsBinDir(UseCase use_case) const;
 
   // Returns an arch-specific SSH shell command that gets invoked in order to
   // start cdc_rsync_server. The command
-  // - creates RemoteToolsBinDir() if it does not exist (so that the server can
-  //   be deployed there subsequently; scp can't create directories),
   // - returns |exit_code_not_found| if cdc_rsync_server does not exist (to
   //   prevent the confusing bash output message
   //     "bash: .../cdc_rsync_server: No such file or directory"), and
@@ -59,13 +51,15 @@ class ServerArch {
   std::string GetStartServerCommand(int exit_code_not_found,
                                     const std::string& args) const;
 
-  // Returns an arch-specific SSH shell command that gets invoked after
+  // Returns an arch-specific SFTP command sequence that deploys the server
-  // cdc_rsync_server has been copied to a temp location. The command
+  // component on the target gets invoked after
-  // - makes the old cdc_rsync_server writable (if it exists),
+  // cdc_rsync_server has been copied to a temp location. The commands
-  // - makes the new cdc_rsync_server executable (Linux only) and
+  // - create the cdc-file-transfer/bin folder if it doesn't exist yet,
-  // - replaces the old cdc_rsync_server by the new one.
+  // - make the old cdc_rsync_server writable if it exists,
-  std::string GetDeployReplaceCommand(const std::string& old_server_path,
+  // - copy cdc_rsync_server to a temp location,
-                                      const std::string& new_server_path) const;
+  // - make the new cdc_rsync_server executable (Linux only) and
+  // - replaces the existing cdc_rsync_server by the temp one.
+  std::string GetDeploySftpCommands() const;
 
  private:
   Type type_;

cdc_rsync/server_arch_test.cc

@@ -60,47 +60,29 @@ TEST(ServerArchTest, CdcServerFilename) {
 }
 
 TEST(ServerArchTest, RemoteToolsBinDir) {
-  const std::string linux_ssh_dir =
+  const std::string linux_dir = ServerArch(kLinux).RemoteToolsBinDir();
-      ServerArch(kLinux).RemoteToolsBinDir(ServerArch::UseCase::kSsh);
+  EXPECT_TRUE(absl::StrContains(linux_dir, ".cache/"));
-  EXPECT_TRUE(absl::StrContains(linux_ssh_dir, "/"));
 
-  const std::string linux_scp_dir =
+  std::string win_dir = ServerArch(kWindows).RemoteToolsBinDir();
-      ServerArch(kLinux).RemoteToolsBinDir(ServerArch::UseCase::kScp);
+  EXPECT_TRUE(absl::StrContains(win_dir, "AppData\\Roaming\\"));
-  EXPECT_EQ(linux_ssh_dir, linux_scp_dir);
-
-  const std::string win_ssh_dir =
-      ServerArch(kWindows).RemoteToolsBinDir(ServerArch::UseCase::kSsh);
-  EXPECT_TRUE(absl::StrContains(win_ssh_dir, "\\"));
-  EXPECT_TRUE(absl::StrContains(win_ssh_dir, "$env:appdata"));
-
-  std::string win_scp_dir =
-      ServerArch(kWindows).RemoteToolsBinDir(ServerArch::UseCase::kScp);
-  EXPECT_TRUE(absl::StrContains(win_scp_dir, "\\"));
-  EXPECT_TRUE(absl::StrContains(win_scp_dir, "AppData\\Roaming"));
 }
 
 TEST(ServerArchTest, GetStartServerCommand) {
   std::string cmd = ServerArch(kWindows).GetStartServerCommand(123, "foo bar");
   EXPECT_TRUE(absl::StrContains(cmd, "123"));
-  EXPECT_TRUE(absl::StrContains(cmd, "foo bar"));
+  EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server.exe foo bar"));
-  EXPECT_TRUE(absl::StrContains(cmd, "New-Item "));
 
   cmd = ServerArch(kLinux).GetStartServerCommand(123, "foo bar");
   EXPECT_TRUE(absl::StrContains(cmd, "123"));
-  EXPECT_TRUE(absl::StrContains(cmd, "foo bar"));
+  EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server foo bar"));
-  EXPECT_TRUE(absl::StrContains(cmd, "mkdir -p"));
 }
 
 TEST(ServerArchTest, GetDeployReplaceCommand) {
-  std::string cmd = ServerArch(kWindows).GetDeployReplaceCommand("aaa", "bbb");
+  std::string cmd = ServerArch(kWindows).GetDeploySftpCommands();
-  EXPECT_TRUE(absl::StrContains(cmd, "aaa"));
+  EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server.exe"));
-  EXPECT_TRUE(absl::StrContains(cmd, "bbb"));
-  EXPECT_TRUE(absl::StrContains(cmd, "Move-Item "));
 
-  cmd = ServerArch(kLinux).GetDeployReplaceCommand("aaa", "bbb");
+  cmd = ServerArch(kLinux).GetDeploySftpCommands();
-  EXPECT_TRUE(absl::StrContains(cmd, "aaa"));
+  EXPECT_TRUE(absl::StrContains(cmd, "cdc_rsync_server"));
-  EXPECT_TRUE(absl::StrContains(cmd, "bbb"));
-  EXPECT_TRUE(absl::StrContains(cmd, "mv "));
 }
 
 }  // namespace

cdc_stream/asset_stream_config.cc

@@ -157,9 +157,9 @@ void AssetStreamConfig::RegisterCommandLineFlags(lyra::command& cmd,
                 "connection to the host. See also --dev-src-dir."));
 
   cmd.add_argument(
-      lyra::opt(dev_target_.scp_command, "cmd")
+      lyra::opt(dev_target_.sftp_command, "cmd")
-          .name("--dev-scp-command")
+          .name("--dev-sftp-command")
-          .help("Scp command and extra flags to use for the "
+          .help("Sftp command and extra flags to use for the "
                 "connection to the host. See also --dev-src-dir."));
 
   cmd.add_argument(
@@ -258,7 +258,7 @@ std::string AssetStreamConfig::ToString() {
   ss << "dev-user-host                = " << dev_target_.user_host << std::endl;
   ss << "dev-ssh-command              = " << dev_target_.ssh_command
      << std::endl;
-  ss << "dev-scp-command              = " << dev_target_.scp_command
+  ss << "dev-sftp-command             = " << dev_target_.sftp_command
      << std::endl;
   ss << "dev-mount-dir                = " << dev_target_.mount_dir << std::endl;
   return ss.str();

cdc_stream/cdc_fuse_manager.cc

@@ -16,6 +16,7 @@
 
 #include "absl/strings/match.h"
 #include "absl/strings/str_format.h"
+#include "absl/strings/str_split.h"
 #include "cdc_fuse_fs/constants.h"
 #include "common/gamelet_component.h"
 #include "common/log.h"
@@ -30,7 +31,7 @@ constexpr char kExeFilename[] = "cdc_stream.exe";
 constexpr char kFuseFilename[] = "cdc_fuse_fs";
 constexpr char kLibFuseFilename[] = "libfuse.so";
 constexpr char kFuseStdoutPrefix[] = "cdc_fuse_fs_stdout";
-constexpr char kRemoteToolsBinDir[] = "~/.cache/cdc-file-transfer/bin/";
+constexpr char kRemoteToolsBinDir[] = ".cache/cdc-file-transfer/bin/";
 
 // Cache directory on the gamelet to store data chunks.
 constexpr char kCacheDir[] = "~/.cache/cdc-file-transfer/chunks";
@@ -54,23 +55,25 @@ absl::Status CdcFuseManager::Deploy() {
   std::string exe_dir;
   RETURN_IF_ERROR(path::GetExeDir(&exe_dir), "Failed to get exe directory");
 
-  // Set the cwd to the exe dir and pass the filenames to scp. Otherwise, some
+  // Create the remote tools bin dir if it doesn't exist yet.
-  // scp implementations can get confused and create the wrong remote filenames.
+  // This assumes that sftp's remote startup directory is the home directory.
-  path::SetCwd(exe_dir);
+  std::vector<std::string> dir_parts =
+      absl::StrSplit(kRemoteToolsBinDir, '/', absl::SkipEmpty());
+  std::string sftp_commands;
+  for (const std::string& dir : dir_parts) {
+    // Use -mkdir to ignore errors if the directory already exists.
+    sftp_commands += absl::StrFormat("-mkdir %s\ncd %s\n", dir, dir);
+  }
 
-  // Copy FUSE to the gamelet.
+  sftp_commands += absl::StrFormat("put %s\n", kFuseFilename);
-  LOG_DEBUG("Copying FUSE");
+  sftp_commands += absl::StrFormat("put %s\n", kLibFuseFilename);
-  RETURN_IF_ERROR(remote_util_->Scp({kFuseFilename, kLibFuseFilename},
+  sftp_commands += absl::StrFormat("chmod 755 %s\n", kFuseFilename);
-                                    kRemoteToolsBinDir, /*compress=*/false),
-                  "Failed to copy FUSE to gamelet");
-  LOG_DEBUG("Copying FUSE succeeded");
 
-  // Make FUSE executable. Note that sync does it automatically.
+  LOG_DEBUG("Deploying FUSE");
-  LOG_DEBUG("Making FUSE executable");
+  RETURN_IF_ERROR(
-  std::string remotePath = path::JoinUnix(kRemoteToolsBinDir, kFuseFilename);
+      remote_util_->Sftp(sftp_commands, exe_dir, /*compress=*/false),
-  RETURN_IF_ERROR(remote_util_->Chmod("a+x", remotePath),
+      "Failed to deploy FUSE");
-                  "Failed to set executable flag on FUSE");
+  LOG_DEBUG("Deploying FUSE succeeded");
-  LOG_DEBUG("Making FUSE succeeded");
 
   return absl::OkStatus();
 }
@@ -104,14 +107,13 @@ absl::Status CdcFuseManager::Start(const std::string& mount_dir,
   // Build the remote command.
   std::string remotePath = path::JoinUnix(kRemoteToolsBinDir, kFuseFilename);
   std::string remote_command = absl::StrFormat(
-      "mkdir -p %s; LD_LIBRARY_PATH=%s %s "
+      "LD_LIBRARY_PATH=%s %s "
       "--instance=%s "
       "--components=%s --port=%i --cache_dir=%s "
       "--verbosity=%i --cleanup_timeout=%i --access_idle_timeout=%i --stats=%i "
       "--check=%i --cache_capacity=%u -- -o allow_root -o ro -o nonempty -o "
       "auto_unmount %s%s%s",
-      kRemoteToolsBinDir, kRemoteToolsBinDir, remotePath,
+      kRemoteToolsBinDir, remotePath, RemoteUtil::QuoteForSsh(instance_),
-      RemoteUtil::QuoteForSsh(instance_),
       RemoteUtil::QuoteForSsh(component_args), remote_port, kCacheDir,
       verbosity, cleanup_timeout_sec, access_idle_timeout_sec, enable_stats,
       check, cache_capacity, debug ? "-d " : "", singlethreaded ? "-s " : "",

cdc_stream/local_assets_stream_manager_client.cc

@@ -38,13 +38,13 @@ LocalAssetsStreamManagerClient::~LocalAssetsStreamManagerClient() = default;
 absl::Status LocalAssetsStreamManagerClient::StartSession(
     const std::string& src_dir, const std::string& user_host,
     const std::string& mount_dir, const std::string& ssh_command,
-    const std::string& scp_command) {
+    const std::string& sftp_command) {
   StartSessionRequest request;
   request.set_workstation_directory(src_dir);
   request.set_user_host(user_host);
   request.set_mount_dir(mount_dir);
   request.set_ssh_command(ssh_command);
-  request.set_scp_command(scp_command);
+  request.set_sftp_command(sftp_command);
 
   grpc::ClientContext context;
   StartSessionResponse response;

cdc_stream/local_assets_stream_manager_client.h

@@ -43,12 +43,12 @@ class LocalAssetsStreamManagerClient {
   // |user_host| is the Linux host, formatted as [user@:host].
   // |mount_dir| is the Linux target directory to stream to.
   // |ssh_command| is the ssh command and extra arguments to use.
-  // |scp_command| is the scp command and extra arguments to use.
+  // |sftp_command| is the sftp command and extra arguments to use.
   absl::Status StartSession(const std::string& src_dir,
                             const std::string& user_host,
                             const std::string& mount_dir,
                             const std::string& ssh_command,
-                            const std::string& scp_command);
+                            const std::string& sftp_command);
 
   // Stops the streaming session to the Linux target |user_host|:|mount_dir|.
   // |user_host| is the Linux host, formatted as [user@:host].

cdc_stream/local_assets_stream_manager_service_impl.cc

@@ -208,7 +208,7 @@ LocalAssetsStreamManagerServiceImpl::GetTargetForStadia(
   SessionTarget target;
   target.mount_dir = request.mount_dir();
   target.ssh_command = request.ssh_command();
-  target.scp_command = request.scp_command();
+  target.sftp_command = request.sftp_command();
 
   // Parse instance/project/org id.
   if (!ParseInstanceName(request.gamelet_name(), instance_id, project_id,
@@ -223,7 +223,7 @@ LocalAssetsStreamManagerServiceImpl::GetTargetForStadia(
                    InitSsh(*instance_id, *project_id, *organization_id));
 
   target.user_host = "cloudcast@" + instance_ip;
-  // Note: Port must be set with ssh_command (-p) and scp_command (-P).
+  // Note: Port must be set with ssh_command (-p) and sftp_command (-P).
   return target;
 }
 
@@ -233,7 +233,7 @@ SessionTarget LocalAssetsStreamManagerServiceImpl::GetTarget(
   target.user_host = request.user_host();
   target.mount_dir = request.mount_dir();
   target.ssh_command = request.ssh_command();
-  target.scp_command = request.scp_command();
+  target.sftp_command = request.sftp_command();
 
   *instance_id = absl::StrCat(target.user_host, ":", target.mount_dir);
   return target;

cdc_stream/session.cc

@@ -55,8 +55,8 @@ Session::Session(std::string instance_id, const SessionTarget& target,
   if (!target.ssh_command.empty()) {
     remote_util_.SetSshCommand(target.ssh_command);
   }
-  if (!target.scp_command.empty()) {
+  if (!target.sftp_command.empty()) {
-    remote_util_.SetScpCommand(target.scp_command);
+    remote_util_.SetSftpCommand(target.sftp_command);
   }
 }
 

cdc_stream/session.h

@@ -38,8 +38,8 @@ struct SessionTarget {
   std::string user_host;
   // Ssh command to use to connect to the remote target.
   std::string ssh_command;
-  // Scp command to use to copy files to the remote target.
+  // Sftp command to use to copy files to the remote target.
-  std::string scp_command;
+  std::string sftp_command;
   // Directory on the remote target where to mount the streamed directory.
   std::string mount_dir;
 };

cdc_stream/start_command.cc

@@ -22,6 +22,7 @@
 #include "common/log.h"
 #include "common/path.h"
 #include "common/process.h"
+#include "common/remote_util.h"
 #include "common/status_macros.h"
 #include "common/stopwatch.h"
 #include "common/util.h"
@@ -73,17 +74,28 @@ void StartCommand::RegisterCommandLineFlags(lyra::command& cmd) {
 
   path::GetEnv("CDC_SSH_COMMAND", &ssh_command_).IgnoreError();
   cmd.add_argument(
-      lyra::opt(ssh_command_, "ssh_command")
+      lyra::opt(ssh_command_, "cmd")
           .name("--ssh-command")
           .help("Path and arguments of ssh command to use, e.g. "
                 "\"C:\\path\\to\\ssh.exe -F config_file -p 1234\". Can also be "
                 "specified by the CDC_SSH_COMMAND environment variable."));
 
-  path::GetEnv("CDC_SCP_COMMAND", &scp_command_).IgnoreError();
+  path::GetEnv("CDC_SFTP_COMMAND", &sftp_command_).IgnoreError();
   cmd.add_argument(
-      lyra::opt(scp_command_, "scp_command")
+      lyra::opt(sftp_command_, "cmd")
+          .name("--sftp-command")
+          .help(
+              "Path and arguments of sftp command to use, e.g. "
+              "\"C:\\path\\to\\sftp.exe -F config_file -P 1234\". Can also be "
+              "specified by the CDC_SFTP_COMMAND environment variable."));
+
+  path::GetEnv("CDC_SCP_COMMAND", &deprecated_scp_command_).IgnoreError();
+  cmd.add_argument(
+      lyra::opt(deprecated_scp_command_, "cmd")
           .name("--scp-command")
-          .help("Path and arguments of scp command to use, e.g. "
+          .help("[Deprecated, use --sftp-command] Path and arguments of scp "
+                "command to "
+                "use, e.g. "
                 "\"C:\\path\\to\\scp.exe -F config_file -P 1234\". Can also be "
                 "specified by the CDC_SCP_COMMAND environment variable."));
 
@@ -106,9 +118,26 @@ absl::Status StartCommand::Run() {
   RETURN_IF_ERROR(LocalAssetsStreamManagerClient::ParseUserHostDir(
       user_host_dir_, &user_host, &mount_dir));
 
+  // Backwards compatibility after switching from scp to sftp.
+  if (sftp_command_.empty() && !deprecated_scp_command_.empty()) {
+    LOG_WARNING(
+        "The CDC_SCP_COMMAND environment variable and the --scp-command flag "
+        "are deprecated. Please set CDC_SFTP_COMMAND or --sftp-command "
+        "instead.");
+
+    sftp_command_ = RemoteUtil::ScpToSftpCommand(deprecated_scp_command_);
+    if (!sftp_command_.empty()) {
+      LOG_WARNING("Converted scp command '%s' to sftp command '%s'.",
+                  deprecated_scp_command_, sftp_command_);
+    } else {
+      LOG_WARNING("Failed to convert scp command '%s' to sftp command.",
+                  deprecated_scp_command_);
+    }
+  }
+
   LocalAssetsStreamManagerClient client(CreateChannel(service_port_));
   absl::Status status = client.StartSession(full_src_dir, user_host, mount_dir,
-                                            ssh_command_, scp_command_);
+                                            ssh_command_, sftp_command_);
 
   if (absl::IsUnavailable(status)) {
     LOG_DEBUG("StartSession status: %s", status.ToString());
@@ -122,7 +151,7 @@ absl::Status StartCommand::Run() {
       // state.
       LocalAssetsStreamManagerClient new_client(CreateChannel(service_port_));
       status = new_client.StartSession(full_src_dir, user_host, mount_dir,
-                                       ssh_command_, scp_command_);
+                                       ssh_command_, sftp_command_);
     }
   }
 

cdc_stream/start_command.h

@@ -44,9 +44,11 @@ class StartCommand : public BaseCommand {
   int verbosity_ = 0;
   uint16_t service_port_ = 0;
   std::string ssh_command_;
-  std::string scp_command_;
+  std::string sftp_command_;
   std::string src_dir_;
   std::string user_host_dir_;
+
+  std::string deprecated_scp_command_;
 };
 
 }  // namespace cdc_ft

cdc_stream/start_service_command.cc

@@ -141,7 +141,7 @@ absl::Status StartServiceCommand::RunService() {
     request.set_user_host(cfg_.dev_target().user_host);
     request.set_mount_dir(cfg_.dev_target().mount_dir);
     request.set_ssh_command(cfg_.dev_target().ssh_command);
-    request.set_scp_command(cfg_.dev_target().scp_command);
+    request.set_sftp_command(cfg_.dev_target().sftp_command);
     localassetsstreammanager::StartSessionResponse response;
     RETURN_ABSL_IF_ERROR(
         session_service.StartSession(nullptr, &request, &response));

proto/local_assets_stream_manager.proto

@@ -48,9 +48,9 @@ message StartSessionRequest {
   // SSH command to connect to the remote instance.
   // Optional, falls back to searching ssh.
   string ssh_command = 10;
-  // SCP command to copy files to the remote instance.
+  // SFTP command to copy files to the remote instance.
-  // Optional, falls back to searching scp.
+  // Optional, falls back to searching sftp.
-  string scp_command = 11;
+  string sftp_command = 11;
 
   reserved 1, 3, 4, 9;
 }