specify ssh2 as dependency (#197)

* specify ssh2 as dependency

* reject when local signing returns non-zero exit code

* remove unnecessary promsie

* remove dependency on signing-utils

* remove from package-lock

* fix check in signing-utils

package.json

@@ -51,4 +51,4 @@
     "husky": "^8.0.3",
     "lerna": "^7.1.1"
   }
-}
+}

packages/signing-utils/src/signing-clients/local-signing-client.spec.ts

@@ -41,4 +41,42 @@ describe('LocalSigningClient', function () {
     ).trim();
     expect(signedFile).to.equal('signed content');
   });
+
+  context('when the script returns a non-zero exit code', function () {
+    beforeEach(function () {
+      writeFileSync(
+        signingScript,
+        `
+          #!/bin/bash
+          echo "Signing script called with arguments: $@"
+          >&2 echo "error - something went wrong"
+          exit 1
+          `
+      );
+    });
+
+    it('sign() rejects', async function () {
+      const localSigningClient = new LocalSigningClient({
+        signingScript: signingScript,
+        signingMethod: 'gpg',
+      });
+
+      const error = await localSigningClient.sign(fileToSign).catch((e) => e);
+      expect(error).to.be.instanceOf(Error);
+    });
+
+    it('includes the stdout and stderr of the failed script in the error', async function () {
+      const localSigningClient = new LocalSigningClient({
+        signingScript: signingScript,
+        signingMethod: 'gpg',
+      });
+
+      const error: Error = await localSigningClient
+        .sign(fileToSign)
+        .catch((e) => e);
+      const { stdout, stderr } = JSON.parse(error.message);
+      expect(stdout).to.contain('Signing script called with arguments: ');
+      expect(stderr).to.equal('error - something went wrong\n');
+    });
+  });
 });

packages/signing-utils/src/signing-clients/local-signing-client.ts

@@ -16,7 +16,10 @@ export class LocalSigningClient implements SigningClient {
     private options: Omit<SigningClientOptions, 'workingDirectory'>
   ) {}
 
-  sign(file: string): Promise<void> {
+  // we want to wrap any errors in promise rejections, so even though there is no
+  // await statement, we use an `async` function
+  // eslint-disable-next-line @typescript-eslint/require-await
+  async sign(file: string): Promise<void> {
     localClientDebug(`Signing ${file}`);
 
     const directoryOfFileToSign = path.dirname(file);
@@ -27,15 +30,27 @@ export class LocalSigningClient implements SigningClient {
         method: this.options.signingMethod,
       };
 
-      spawnSync('bash', [this.options.signingScript, path.basename(file)], {
-        cwd: directoryOfFileToSign,
-        env,
-        encoding: 'utf-8',
-      });
-
+      const { stdout, stderr, status } = spawnSync(
+        'bash',
+        [this.options.signingScript, path.basename(file)],
+        {
+          cwd: directoryOfFileToSign,
+          env,
+          encoding: 'utf-8',
+        }
+      );
+
+      localClientDebug({ stdout, stderr });
+
+      if (status !== 0) {
+        throw new Error(
+          JSON.stringify({
+            stdout,
+            stderr,
+          })
+        );
+      }
       localClientDebug(`Signed file ${file}`);
-
-      return Promise.resolve();
     } catch (error) {
       localClientDebug({ error });
       throw error;

packages/signing-utils/src/signing-clients/remote-signing-client.spec.ts

@@ -3,55 +3,16 @@ import { exec } from 'child_process';
 import { RemoteSigningClient } from './remote-signing-client';
 import { expect } from 'chai';
 import type { SSHClient } from '../ssh-client';
+import { promisify } from 'util';
 
 const getMockedSSHClient = () => {
   return {
-    getSftpConnection: () => {
-      return {
-        fastPut: async (
-          localFile: string,
-          remoteFile: string,
-          cb: (err?: Error) => void
-        ) => {
-          try {
-            await fs.copyFile(localFile, remoteFile);
-            cb();
-          } catch (err) {
-            cb(err as Error);
-          }
-        },
-        fastGet: async (
-          remoteFile: string,
-          localFile: string,
-          cb: (err?: Error) => void
-        ) => {
-          try {
-            await fs.copyFile(remoteFile, localFile);
-            cb();
-          } catch (err) {
-            cb(err as Error);
-          }
-        },
-        unlink: async (remoteFile: string, cb: (err?: Error) => void) => {
-          try {
-            await fs.unlink(remoteFile);
-            cb();
-          } catch (err) {
-            cb(err as Error);
-          }
-        },
-      };
-    },
-    exec: (command: string) => {
-      return new Promise((resolve, reject) => {
-        exec(command, { shell: 'bash' }, (err) => {
-          if (err) {
-            return reject(err);
-          }
-          return resolve('Ok');
-        });
-      });
-    },
+    // The mocked ssh client
+    copyFile: (from: string, to: string) => fs.copyFile(from, to),
+    downloadFile: (remote: string, local: string) => fs.copyFile(remote, local),
+    removeFile: fs.unlink.bind(fs.unlink),
+    exec: (command: string) =>
+      promisify(exec)(command, { shell: 'bash' }).then(() => 'Ok'),
     disconnect: () => {},
   } as unknown as SSHClient;
 };

packages/signing-utils/src/signing-clients/remote-signing-client.ts

@@ -1,12 +1,9 @@
 import path from 'path';
-import type { SFTPWrapper } from 'ssh2';
 import type { SSHClient } from '../ssh-client';
 import { debug, getEnv } from '../utils';
 import type { SigningClient, SigningClientOptions } from '.';
 
 export class RemoteSigningClient implements SigningClient {
-  private sftpConnection!: SFTPWrapper;
-
   constructor(
     private sshClient: SSHClient,
     private options: SigningClientOptions
@@ -19,13 +16,12 @@ export class RemoteSigningClient implements SigningClient {
    * - Copy the signing script to the remote machine
    */
   private async init() {
-    this.sftpConnection = await this.sshClient.getSftpConnection();
     await this.sshClient.exec(`mkdir -p ${this.options.workingDirectory}`);
 
     // Copy the signing script to the remote machine
     {
       const remoteScript = `${this.options.workingDirectory}/garasign.sh`;
-      await this.copyFile(this.options.signingScript, remoteScript);
+      await this.sshClient.copyFile(this.options.signingScript, remoteScript);
       await this.sshClient.exec(`chmod +x ${remoteScript}`);
     }
   }
@@ -36,39 +32,6 @@ export class RemoteSigningClient implements SigningClient {
     )}`;
   }
 
-  private async copyFile(file: string, remotePath: string): Promise<void> {
-    return new Promise((resolve, reject) => {
-      this.sftpConnection.fastPut(file, remotePath, (err) => {
-        if (err) {
-          return reject(err);
-        }
-        return resolve();
-      });
-    });
-  }
-
-  private async downloadFile(remotePath: string, file: string): Promise<void> {
-    return new Promise((resolve, reject) => {
-      this.sftpConnection.fastGet(remotePath, file, (err) => {
-        if (err) {
-          return reject(err);
-        }
-        return resolve();
-      });
-    });
-  }
-
-  private async removeFile(remotePath: string): Promise<void> {
-    return new Promise((resolve, reject) => {
-      this.sftpConnection.unlink(remotePath, (err) => {
-        if (err) {
-          return reject(err);
-        }
-        return resolve();
-      });
-    });
-  }
-
   private async signRemoteFile(file: string) {
     const env = getEnv();
     /**
@@ -100,18 +63,18 @@ export class RemoteSigningClient implements SigningClient {
       // establish connection
       await this.init();
 
-      await this.copyFile(file, remotePath);
+      await this.sshClient.copyFile(file, remotePath);
       debug(`SFTP: Copied file ${file} to ${remotePath}`);
 
       await this.signRemoteFile(path.basename(remotePath));
       debug(`SFTP: Signed file ${file}`);
 
-      await this.downloadFile(remotePath, file);
+      await this.sshClient.downloadFile(remotePath, file);
       debug(`SFTP: Downloaded signed file to ${file}`);
     } catch (error) {
       debug({ error });
     } finally {
-      await this.removeFile(remotePath);
+      await this.sshClient.removeFile(remotePath);
       debug(`SFTP: Removed remote file ${remotePath}`);
       this.sshClient.disconnect();
     }

packages/signing-utils/src/ssh-client.ts

@@ -79,7 +79,7 @@ export class SSHClient {
     return data;
   }
 
-  async getSftpConnection(): Promise<SFTPWrapper> {
+  private async getSftpConnection(): Promise<SFTPWrapper> {
     if (!this.connected) {
       throw new Error('Not connected to ssh server');
     }
@@ -89,4 +89,25 @@ export class SSHClient {
       (await promisify(this.sshConnection.sftp.bind(this.sshConnection))());
     return this.sftpConnection;
   }
+
+  async copyFile(file: string, remotePath: string): Promise<void> {
+    const sftpConnection = await this.getSftpConnection();
+    return promisify(sftpConnection.fastPut.bind(sftpConnection))(
+      file,
+      remotePath
+    );
+  }
+
+  async downloadFile(remotePath: string, file: string): Promise<void> {
+    const sftpConnection = await this.getSftpConnection();
+    return promisify(sftpConnection.fastGet.bind(sftpConnection))(
+      remotePath,
+      file
+    );
+  }
+
+  async removeFile(remotePath: string): Promise<void> {
+    const sftpConnection = await this.getSftpConnection();
+    return promisify(sftpConnection.unlink.bind(sftpConnection))(remotePath);
+  }
 }