try system test

.github/workflows/clang-format-check.yml

@@ -1,5 +1,5 @@
 name: clang-format Check
-on: [push, pull_request]
+on: [pull_request]
 jobs:
   formatting-check:
     name: Formatting Check
@@ -15,6 +15,6 @@ jobs:
     - name: Run clang-format style check for C/C++/Protobuf source code.
       uses: jidicula/[email protected]
       with:
-        clang-format-version: '13'
+        clang-format-version: '18'
         check-path: ${{ matrix.path }}
         fallback-style: 'Google'

.github/workflows/codecov.yml

@@ -0,0 +1,57 @@
+name: Linux CI
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+jobs:
+  codecov:
+    runs-on: ubuntu-22.04
+
+    steps:
+      - uses: actions/checkout@v4
+      - name: Setup
+        shell: bash
+        run: |
+          mkdir -p ~/.ssh/
+          echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
+          sudo apt-get update
+          sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov
+          if [[ -z "${ACT}" ]]; then auth_header="$(git config --local --get http.https://github.com/.extraheader)"; fi
+          git submodule sync --recursive
+          git submodule update --init --force --recursive
+
+      # Restore both vcpkg and its artifacts from the GitHub cache service.
+      - name: Restore vcpkg and its artifacts.
+        uses: actions/cache@v4
+        with:
+          # The first path is where vcpkg generates artifacts while consuming the vcpkg.json manifest file.
+          # The second path is the location of vcpkg (it contains the vcpkg executable and data files).
+          # The other paths starting with '!' are exclusions: they contain temporary files generated during the build of the installed packages.
+          path: |
+            ${{ env.CMAKE_BUILD_DIR }}/vcpkg_installed/
+            ${{ env.VCPKG_ROOT }}
+            !${{ env.VCPKG_ROOT }}/buildtrees
+            !${{ env.VCPKG_ROOT }}/packages
+            !${{ env.VCPKG_ROOT }}/downloads
+          # The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service.
+          # The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm.
+          # Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already).
+          key: |
+            et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux-codecov
+
+      - name: Build with code coverage
+        run: |
+          mkdir build
+          pushd build
+          cmake -DCODE_COVERAGE=ON ../
+          make -j`nproc`
+          ./et-test
+          lcov --capture --directory . --output-file coverage.info
+          lcov --remove coverage.info '/usr/*' --output-file coverage.info # filter system-files
+          lcov --list coverage.info # debug info
+          # Uploading report to CodeCov
+          bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports"
+          popd

.github/workflows/linux_ci.yml

@@ -22,7 +22,17 @@ jobs:
           echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
           sudo apt-get update
           sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++
-          auth_header="$(git config --local --get http.https://github.com/.extraheader)"
+
+          echo "Host localhost\n  Port 2222\n\n" >> ~/.ssh/config
+
+          sudo /usr/sbin/sshd -p 2222
+
+          ssh-keygen -t rsa -f ~/.ssh/id_rsa -P "" -N ""
+          cat ~/.ssh/id_rsa.pub >> ~/.ssh/authorized_keys
+          cat ~/.ssh/id_rsa.pub >> ~/.ssh/known_hosts
+          ssh -vvvvvvv -o "StrictHostKeyChecking no" -o 'PreferredAuthentications=publickey' localhost "echo foobar" # Fails if we can't ssh into localhost without a password
+          if [[ -z "${ACT}" ]]; then auth_header="$(git config --local --get http.https://github.com/.extraheader)"; fi
+
           git submodule sync --recursive
           git submodule update --init --force --recursive
 
@@ -45,95 +55,46 @@ jobs:
           key: |
             et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux-${{ matrix.sanitize }}
 
-      - name: Test with ubsan
+      - name: Build with ubsan
         run: |
           mkdir build
           pushd build
           cmake -DSANITIZE_UNDEFINED=ON ../
           make -j`nproc`
-          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
           popd
-          rm -Rf build
+          ./test/system_tests/connect_with_jumphost.sh
+          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
         if: matrix.sanitize == 'ubsan'
 
-      - name: Test with asan
+      - name: Build with asan
         run: |
           mkdir build
           pushd build
           cmake -DSANITIZE_ADDRESS=ON ../
           make -j`nproc`
-          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
           popd
-          rm -Rf build
+          ./test/system_tests/connect_with_jumphost.sh
+          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
         if: matrix.sanitize == 'asan'
 
-      - name: Test with msan
+      - name: Build with msan
         run: |
           mkdir build
           pushd build
           cmake -DSANITIZE_MEMORY=ON ../
           make -j`nproc`
-          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
           popd
-          rm -Rf build
+          ./test/system_tests/connect_with_jumphost.sh
+          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
         if: matrix.sanitize == 'msan'
 
-      - name: Test with tsan
+      - name: Build with tsan
         run: |
           mkdir build
           pushd build
           cmake -DSANITIZE_THREAD=ON -DSANITIZE_LINK_STATIC=ON ../
           make -j`nproc`
-          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./et-test
           popd
-          rm -Rf build
+          ./test/system_tests/connect_with_jumphost.sh
+          TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ./build/et-test
         if: matrix.sanitize == 'tsan'
-
-  codecov:
-    runs-on: ubuntu-22.04
-
-    steps:
-      - uses: actions/checkout@v4
-      - name: Setup
-        shell: bash
-        run: |
-          mkdir -p ~/.ssh/
-          echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
-          sudo apt-get update
-          sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov
-          auth_header="$(git config --local --get http.https://github.com/.extraheader)"
-          git submodule sync --recursive
-          git submodule update --init --force --recursive
-
-      # Restore both vcpkg and its artifacts from the GitHub cache service.
-      - name: Restore vcpkg and its artifacts.
-        uses: actions/cache@v4
-        with:
-          # The first path is where vcpkg generates artifacts while consuming the vcpkg.json manifest file.
-          # The second path is the location of vcpkg (it contains the vcpkg executable and data files).
-          # The other paths starting with '!' are exclusions: they contain temporary files generated during the build of the installed packages.
-          path: |
-            ${{ env.CMAKE_BUILD_DIR }}/vcpkg_installed/
-            ${{ env.VCPKG_ROOT }}
-            !${{ env.VCPKG_ROOT }}/buildtrees
-            !${{ env.VCPKG_ROOT }}/packages
-            !${{ env.VCPKG_ROOT }}/downloads
-          # The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service.
-          # The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm.
-          # Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already).
-          key: |
-            et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-linux
-
-      - name: Test with code coverage
-        run: |
-          mkdir build
-          pushd build
-          cmake -DCODE_COVERAGE=ON ../
-          make -j`nproc`
-          ./et-test
-          lcov --capture --directory . --output-file coverage.info
-          lcov --remove coverage.info '/usr/*' --output-file coverage.info # filter system-files
-          lcov --list coverage.info # debug info
-          # Uploading report to CodeCov
-          bash <(curl -s https://codecov.io/bash) -f coverage.info || echo "Codecov did not collect coverage reports"
-          popd

src/terminal/ParseConfigFile.hpp

@@ -1431,6 +1431,7 @@ int parse_ssh_config_file(const char *targethost, struct Options *options,
   }
 
   ifstream infile(expandedFilename);
+  free(expandedFilename);
   if (!infile.good()) {
     LOG(INFO) << filename << " not found";
     return 0;

src/terminal/SshSetupHandler.cpp

@@ -27,7 +27,8 @@ string genCommand(const string& passkey, const string& id,
 
 string SshSetupHandler::SetupSsh(const string& user, const string& host,
                                  const string& host_alias, int port,
-                                 const string& jumphost, int jport, bool kill,
+                                 const string& jumphost,
+                                 const string& jServerFifo, bool kill,
                                  int vlevel, const string& cmd_prefix,
                                  const string& serverFifo,
                                  const std::vector<std::string>& ssh_options) {
@@ -72,6 +73,9 @@ string SshSetupHandler::SetupSsh(const string& user, const string& host,
 
   ssh_args.push_back(SSH_SCRIPT_DST);
 
+  std::string ssh_concat;
+  for (const auto& piece : ssh_args) ssh_concat += piece + " ";
+  VLOG(1) << "Trying ssh with args: " << ssh_concat << endl;
   auto sshBuffer = SubprocessToStringInteractive("ssh", ssh_args);
 
   try {
@@ -107,9 +111,12 @@ string SshSetupHandler::SetupSsh(const string& user, const string& host,
   if (!jumphost.empty()) {
     /* If jumphost is set, we need to pass dst host and port to jumphost
      * and connect to jumphost here */
-    string cmdoptions{"--verbose=" + std::to_string(vlevel)};
-    string jump_cmdoptions = cmdoptions + " --jump --dsthost=" + host +
-                             " --dstport=" + to_string(port);
+    string jump_cmdoptions{"--verbose=" + std::to_string(vlevel)};
+    if (!jServerFifo.empty()) {
+      jump_cmdoptions += " --serverfifo=" + jServerFifo;
+    }
+    jump_cmdoptions = jump_cmdoptions + " --jump --dsthost=" + host +
+                      " --dstport=" + to_string(port);
     string SSH_SCRIPT_JUMP = genCommand(passkey, id, clientTerm, user, kill,
                                         cmd_prefix, jump_cmdoptions);
 

src/terminal/SshSetupHandler.hpp

@@ -8,8 +8,8 @@ class SshSetupHandler {
  public:
   static string SetupSsh(const string &user, const string &host,
                          const string &host_alias, int port,
-                         const string &jumphost, int jport, bool kill,
-                         int vlevel, const string &etterminal_path,
+                         const string &jumphost, const string &jServerFifo,
+                         bool kill, int vlevel, const string &etterminal_path,
                          const string &serverFifo,
                          const std::vector<std::string> &ssh_options);
   static const string ETTERMINAL_BIN;

src/terminal/TelemetryService.cpp

@@ -209,11 +209,12 @@ TelemetryService::TelemetryService(const bool _allow,
     logSendingThread.reset(new thread([this]() {
       auto nextDumpTime = std::chrono::system_clock::now();
       while (true) {
-        bool lastRun = shuttingDown;
+        bool lastRun;
         string payload;
         int logBufferSize;
         {
           lock_guard<recursive_mutex> guard(logMutex);
+          lastRun = shuttingDown;
           logBufferSize = (int)logBuffer.size();
         }
         if (logBufferSize) {
@@ -253,6 +254,7 @@ TelemetryService::TelemetryService(const bool _allow,
 }
 
 TelemetryService::~TelemetryService() {
+  lock_guard<recursive_mutex> guard(logMutex);
   if (!shuttingDown) {
     cerr << "Destroyed telemetryService without a shutdown";
   }
@@ -283,10 +285,13 @@ void TelemetryService::logToDatadog(const string& logText, el::Level logLevel,
 }
 
 void TelemetryService::shutdown() {
-  if (shuttingDown) {
-    return;
+  {
+    lock_guard<recursive_mutex> guard(logMutex);
+    if (shuttingDown) {
+      return;
+    }
+    shuttingDown = true;
   }
-  shuttingDown = true;
 #ifdef USE_SENTRY
   sentry_shutdown();
 #endif