Harden ancillary exec function

iterative · Dec 5, 2022 · ceeb11b · ceeb11b · github-actions · Dec 5, 2022
1 parent 1da5bf3
commit ceeb11b
Show file tree

Hide file tree

Showing 28 changed files with 350 additions and 113 deletions.
diff --git a/bin/cml.test.js b/bin/cml.test.js
@@ -3,7 +3,7 @@ const fetch = require('node-fetch');
 
 describe('command-line interface tests', () => {
   test('cml --help', async () => {
-    const output = await exec(`node ./bin/cml.js --help`);
+    const output = await exec('node', './bin/cml.js', '--help');
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js <command>

diff --git a/bin/cml/asset/publish.e2e.test.js b/bin/cml/asset/publish.e2e.test.js
@@ -3,36 +3,64 @@ const { exec } = require('../../../src/utils');
 
 describe('CML e2e', () => {
   test('cml publish assets/logo.png --md', async () => {
-    const output = await exec(`node ./bin/cml.js publish assets/logo.png --md`);
+    const output = await exec(
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/logo.png',
+      '--md'
+    );
 
     expect(output.startsWith('![](')).toBe(true);
   });
 
   test('cml publish assets/logo.png', async () => {
-    const output = await exec(`node ./bin/cml.js publish assets/logo.png`);
+    const output = await exec(
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/logo.png'
+    );
 
     expect(output.startsWith('https://')).toBe(true);
   });
 
   test('cml publish assets/logo.pdf --md', async () => {
     const title = 'this is awesome';
     const output = await exec(
-      `node ./bin/cml.js publish assets/logo.pdf --md --title '${title}'`
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/logo.pdf',
+      '--md',
+      '--title',
+      title
     );
 
     expect(output.startsWith(`[${title}](`)).toBe(true);
   });
 
   test('cml publish assets/logo.pdf', async () => {
-    const output = await exec(`node ./bin/cml.js publish assets/logo.pdf`);
+    const output = await exec(
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/logo.pdf'
+    );
 
     expect(output.startsWith('https://')).toBe(true);
   });
 
   test('cml publish assets/test.svg --md', async () => {
     const title = 'this is awesome';
     const output = await exec(
-      `node ./bin/cml.js publish assets/test.svg --md --title '${title}'`
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/test.svg',
+      '--md',
+      '--title',
+      title
     );
 
     expect(output.startsWith('![](') && output.endsWith(`${title}")`)).toBe(
@@ -41,23 +69,40 @@ describe('CML e2e', () => {
   });
 
   test('cml publish assets/test.svg', async () => {
-    const output = await exec(`node ./bin/cml.js publish assets/test.svg`);
+    const output = await exec(
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/test.svg'
+    );
 
     expect(output.startsWith('https://')).toBe(true);
   });
 
   test('cml publish assets/logo.pdf to file', async () => {
     const file = `cml-publish-test.md`;
 
-    await exec(`node ./bin/cml.js publish assets/logo.pdf --file ${file}`);
+    await exec(
+      'node',
+      './bin/cml.js',
+      'publish',
+      'assets/logo.pdf',
+      '--file',
+      file
+    );
 
     expect(fs.existsSync(file)).toBe(true);
     await fs.promises.unlink(file);
   });
 
   test('cml publish assets/vega-lite.json', async () => {
     const output = await exec(
-      `node ./bin/cml.js publish --mime-type=application/json assets/vega-lite.json`
+      'node',
+      './bin/cml.js',
+      'publish',
+      '--mime-type',
+      'application/json',
+      'assets/vega-lite.json'
     );
 
     expect(output.startsWith('https://')).toBe(true);
@@ -68,15 +113,23 @@ describe('CML e2e', () => {
     const { TEST_GITLAB_REPO: repo, TEST_GITLAB_TOKEN: token } = process.env;
 
     const output = await exec(
-      `node ./bin/cml.js publish --repo=${repo} --token=${token} --gitlab-uploads assets/test.svg`
+      'node',
+      './bin/cml.js',
+      'publish',
+      '--repo',
+      repo,
+      '--token',
+      token,
+      '--gitlab-uploads',
+      'assets/test.svg'
     );
 
     expect(output.startsWith('https://')).toBe(true);
   });
 
   test('cml publish /nonexistent produces file error', async () => {
     await expect(
-      exec('node ./bin/cml.js publish /nonexistent')
+      exec('node', './bin/cml.js', 'publish', '/nonexistent')
     ).rejects.toThrowError('ENOENT');
   });
 });
diff --git a/bin/cml/asset/publish.test.js b/bin/cml/asset/publish.test.js
@@ -2,7 +2,7 @@ const { exec } = require('../../../src/utils');
 
 describe('CML cli test', () => {
   test('cml publish --help', async () => {
-    const output = await exec(`node ./bin/cml.js publish --help`);
+    const output = await exec('node', './bin/cml.js', 'publish', '--help');
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js publish <asset>

diff --git a/bin/cml/check/create.e2e.test.js b/bin/cml/check/create.e2e.test.js
@@ -15,7 +15,7 @@ describe('CML e2e', () => {
 
     await fs.writeFile(path, report);
     process.env.GITHUB_ACTIONS &&
-      (await exec(`node ./bin/cml.js send-github-check ${path}`));
+      (await exec('node', './bin/cml.js', 'send-github-check', path));
   });
 
   test('cml send-github-check failure with tile "CML neutral test"', async () => {
@@ -26,7 +26,14 @@ describe('CML e2e', () => {
     await fs.writeFile(path, report);
     process.env.GITHUB_ACTIONS &&
       (await exec(
-        `node ./bin/cml.js send-github-check ${path} --title "${title}" --conclusion "${conclusion}"`
+        'node',
+        './bin/cml.js',
+        'send-github-check',
+        path,
+        '--title',
+        title,
+        '--conclusion',
+        conclusion
       ));
   });
 });
diff --git a/bin/cml/check/create.test.js b/bin/cml/check/create.test.js
@@ -2,7 +2,12 @@ const { exec } = require('../../../src/utils');
 
 describe('CML e2e', () => {
   test('cml send-github-check --help', async () => {
-    const output = await exec(`node ./bin/cml.js send-github-check --help`);
+    const output = await exec(
+      'node',
+      './bin/cml.js',
+      'send-github-check',
+      '--help'
+    );
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js send-github-check <markdown file>

diff --git a/bin/cml/comment/create.e2e.test.js b/bin/cml/comment/create.e2e.test.js
@@ -21,21 +21,30 @@ describe('Comment integration tests', () => {
 
     await fs.writeFile(path, report);
     await exec(
-      `node ./bin/cml.js send-comment --repo=${repo} --token=${token} --commit-sha=${sha} ${path}`
+      'node',
+      './bin/cml.js',
+      'send-comment',
+      '--repo',
+      repo,
+      '--token',
+      token,
+      '--commit-sha',
+      sha,
+      path
     );
   });
 
   test('cml send-comment to current repo', async () => {
     const report = `## Test Comment`;
 
     await fs.writeFile(path, report);
-    await exec(`node ./bin/cml.js send-comment ${path}`);
+    await exec('node', './bin/cml.js', 'send-comment', path);
   });
 
   test('cml send-comment --publish to current repo', async () => {
     const report = `## Test Comment\n![](assets/logo.png)`;
 
     await fs.writeFile(path, report);
-    await exec(`node ./bin/cml.js send-comment --publish ${path}`);
+    await exec('node', './bin/cml.js', 'send-comment', '--publish', path);
   });
 });
diff --git a/bin/cml/comment/create.test.js b/bin/cml/comment/create.test.js
@@ -2,7 +2,7 @@ const { exec } = require('../../../src/utils');
 
 describe('Comment integration tests', () => {
   test('cml send-comment --help', async () => {
-    const output = await exec(`node ./bin/cml.js send-comment --help`);
+    const output = await exec('node', './bin/cml.js', 'send-comment', '--help');
     expect(output).toMatchInlineSnapshot(`
       "cml.js send-comment <markdown file>
 

diff --git a/bin/cml/pr/create.e2e.test.js b/bin/cml/pr/create.e2e.test.js
@@ -2,7 +2,7 @@ const { exec } = require('../../../src/utils');
 
 describe('CML e2e', () => {
   test('cml-pr --help', async () => {
-    const output = await exec(`echo none | node ./bin/cml.js pr --help`);
+    const output = await exec('node', './bin/cml.js', 'pr', '--help');
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js pr <glob path...>

diff --git a/bin/cml/repo/prepare.test.js b/bin/cml/repo/prepare.test.js
@@ -2,7 +2,7 @@ const { exec } = require('../../../src/utils');
 
 describe('CML e2e', () => {
   test('cml-ci --help', async () => {
-    const output = await exec(`echo none | node ./bin/cml.js ci --help`);
+    const output = await exec('node', './bin/cml.js', 'ci', '--help');
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js ci

diff --git a/bin/cml/runner/launch.e2e.test.js b/bin/cml/runner/launch.e2e.test.js
@@ -15,9 +15,27 @@ const {
 
 const launchRunner = async (opts) => {
   const { cloud, type, repo, token, privateKey, name } = opts;
-  const command = `node ./bin/cml.js runner --cloud ${cloud} --cloud-type ${type} --repo ${repo} --token ${token} --cloud-ssh-private="${privateKey}" --name ${name} --cloud-spot true --idle-timeout ${IDLE_TIMEOUT}`;
-
-  const output = await exec(command);
+  const output = await exec(
+    'node',
+    './bin/cml.js',
+    'runner',
+    '--cloud',
+    cloud,
+    '--cloud-type',
+    type,
+    '--repo',
+    repo,
+    '--token',
+    token,
+    '--cloud-ssh-private',
+    privateKey,
+    '--name',
+    name,
+    '--cloud-spot',
+    'true',
+    '--idle-timeout',
+    IDLE_TIMEOUT
+  );
   const state = JSON.parse(output.split(/\n/).pop());
 
   return state;

diff --git a/bin/cml/runner/launch.js b/bin/cml/runner/launch.js
@@ -67,7 +67,13 @@ const shutdown = async (opts) => {
 
     try {
       return await exec(
-        `leo destroy-runner --cloud=${cloud} --region=${region} ${id}`
+        'leo',
+        'destroy-runner',
+        '--cloud',
+        cloud,
+        '--region',
+        region,
+        id
       );
     } catch (err) {
       winston.error(`\tFailed destroying with LEO: ${err.message}`);

diff --git a/bin/cml/runner/launch.test.js b/bin/cml/runner/launch.test.js
@@ -2,7 +2,7 @@ const { exec } = require('../../../src/utils');
 
 describe('CML e2e', () => {
   test('cml-runner --help', async () => {
-    const output = await exec(`echo none | node ./bin/cml.js runner --help`);
+    const output = await exec('node', './bin/cml.js', 'runner', '--help');
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js runner

diff --git a/bin/cml/tensorboard/connect.e2e.test.js b/bin/cml/tensorboard/connect.e2e.test.js
@@ -15,7 +15,7 @@ const isTbRunning = async () => {
 
 const rmTbDevExperiment = async (tbOutput) => {
   const id = /experiment\/([a-zA-Z0-9]{22})/.exec(tbOutput)[1];
-  await exec(`tensorboard dev delete --experiment_id ${id}`);
+  await exec('tensorboard', 'dev', 'delete', '--experiment_id', id);
 };
 
 describe('tbLink', () => {
@@ -56,9 +56,20 @@ describe('CML e2e', () => {
     const desc = 'Test experiment';
     const title = 'go to the experiment';
     const output = await exec(
-      `node ./bin/cml.js tensorboard-dev --credentials '${CREDENTIALS}' \
-        --md --title '${title}' \
-        --logdir logs --name '${name}' --description '${desc}'`
+      'node',
+      './bin/cml.js',
+      'tensorboard-dev',
+      '--credentials',
+      CREDENTIALS,
+      '--md',
+      '--title',
+      title,
+      '--logdir',
+      'logs',
+      '--name',
+      name,
+      '--description',
+      desc
     );
 
     const isRunning = await isTbRunning();
@@ -71,7 +82,13 @@ describe('CML e2e', () => {
 
   test('cml tensorboard-dev invalid creds', async () => {
     try {
-      await exec(`node ./bin/cml.js tensorboard-dev --credentials 'invalid'`);
+      await exec(
+        'node',
+        './bin/cml.js',
+        'tensorboard-dev',
+        '--credentials',
+        'invalid'
+      );
     } catch (err) {
       expect(err.message.includes('json.decoder.JSONDecodeError')).toBe(true);
     }

diff --git a/bin/cml/tensorboard/connect.js b/bin/cml/tensorboard/connect.js
@@ -87,7 +87,7 @@ exports.handler = async (opts) => {
   await fs.mkdir(path, { recursive: true });
   await fs.writeFile(`${path}/uploader-creds.json`, credentials);
 
-  const help = await exec('tensorboard dev upload -h');
+  const help = await exec('tensorboard', 'dev', 'upload', '-h');
   const extraParamsFound =
     (name || description) && help.indexOf('--description') >= 0;
   const extraParams = extraParamsFound

diff --git a/bin/cml/tensorboard/connect.test.js b/bin/cml/tensorboard/connect.test.js
@@ -2,7 +2,12 @@ const { exec } = require('../../../src/utils');
 
 describe('CML e2e', () => {
   test('cml tensorboard-dev --help', async () => {
-    const output = await exec(`node ./bin/cml.js tensorboard-dev --help`);
+    const output = await exec(
+      'node',
+      './bin/cml.js',
+      'tensorboard-dev',
+      '--help'
+    );
 
     expect(output).toMatchInlineSnapshot(`
       "cml.js tensorboard-dev

diff --git a/bin/cml/workflow/rerun.test.js b/bin/cml/workflow/rerun.test.js
@@ -3,7 +3,10 @@ const { exec } = require('../../../src/utils');
 describe('CML e2e', () => {
   test('cml-ci --help', async () => {
     const output = await exec(
-      `echo none | node ./bin/cml.js rerun-workflow --help`
+      'node',
+      './bin/cml.js',
+      'rerun-workflow',
+      '--help'
     );
 
     expect(output).toMatchInlineSnapshot(`