feat: use sfCommand and testing improvements

* test: fix nuts * feat: use sfCommand * test: isolate test sessions * test: 1 session with linked plugin, uninstall before re-install * test: use a 2nd unsigned plugin * test: only modify files, no plugin installation * test: restore allowlist * test: sf setup steps in `before` block * test: last test is not async * chore: remove logging * test: skip allowlist test * test: assert that allowlist was written as expected * test: set home env for allowList * test: set config dir, only run linux nut * test: manually set homedir via env * test: also set xdg * test: no only * test: set sfdx config dir directly * test: nut on macos * test: restore full test suite * test: last test only runs on macos * chore: remove comment * refactor: pr feedback * chore: bump sf-plugins-core * fix: default to NO on installation confirmation
salesforcecli · Dec 7, 2022 · d801eef · d801eef
1 parent 3d61230
commit d801eef
Show file tree

Hide file tree

Showing 12 changed files with 250 additions and 302 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -5,5 +5,5 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 module.exports = {
-  extends: ['eslint-config-salesforce-typescript', 'eslint-config-salesforce-license'],
+  extends: ['eslint-config-salesforce-typescript', 'eslint-config-salesforce-license', 'plugin:sf-plugin/recommended'],
 };
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -20,7 +20,7 @@ jobs:
     secrets: inherit
     strategy:
       matrix:
-        os: [ubuntu-latest, windows-latest]
+        os: [macos-latest, windows-latest, ubuntu-latest]
       fail-fast: false
     with:
       os: ${{ matrix.os }}
diff --git a/messages/verify.json b/messages/verify.json
diff --git a/messages/verify.md b/messages/verify.md
@@ -0,0 +1,25 @@
+# summary
+
+Validate a digital signature.
+
+# description
+
+Verifies the digital signature on an npm package matches the signature and key stored at the expected URLs.
+
+# examples
+
+- <%= config.bin %> <%= command.id %> --npm @scope/npmName --registry http://my.repo.org:4874
+
+- <%= config.bin %> <%= command.id %> --npm @scope/npmName
+
+# flags.npm
+
+Specify the npm name. This can include a tag/version.
+
+# flags.registry
+
+The registry name. The behavior is the same as npm.
+
+# FailedDigitalSignatureVerification
+
+A digital signature is specified for this plugin but it didn't verify against the certificate.
diff --git a/package.json b/package.json
@@ -9,10 +9,9 @@
   },
   "bugs": "https://github.com/forcedotcom/cli/issues",
   "dependencies": {
-    "@oclif/core": "^1.6.3",
-    "@salesforce/command": "^5.0.4",
+    "@oclif/core": "^1.20.4",
     "@salesforce/core": "^3.10.1",
-    "@salesforce/sf-plugins-core": "^1.14.1",
+    "@salesforce/sf-plugins-core": "^1.20.0",
     "got": "^11",
     "npm": "^8.12.1",
     "npm-run-path": "^4.0.1",
@@ -26,9 +25,9 @@
     "@salesforce/cli-plugins-testkit": "^3.2.0",
     "@salesforce/dev-config": "^3.0.0",
     "@salesforce/dev-scripts": "^3.1.0",
-    "@salesforce/plugin-command-reference": "^1.3.0",
+    "@salesforce/plugin-command-reference": "^2.2.8",
     "@salesforce/prettier-config": "^0.0.2",
-    "@salesforce/ts-sinon": "1.3.21",
+    "@salesforce/ts-sinon": "^1.4.2",
     "@swc/core": "^1.3.14",
     "@types/proxy-from-env": "^1.0.1",
     "@types/shelljs": "^0.8.9",
@@ -44,6 +43,7 @@
     "eslint-plugin-header": "^3.1.1",
     "eslint-plugin-import": "2.26.0",
     "eslint-plugin-jsdoc": "^39.3.6",
+    "eslint-plugin-sf-plugin": "^1.1.5",
     "husky": "^7.0.4",
     "mocha": "^9.1.3",
     "nyc": "^15.1.0",
@@ -116,7 +116,7 @@
     "test": "sf-test",
     "test:command-reference": "./bin/dev commandreference:generate --erroronwarnings",
     "test:deprecation-policy": "./bin/dev snapshot:compare",
-    "test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000 --parallel",
+    "test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000",
     "version": "oclif readme"
   },
   "publishConfig": {

diff --git a/src/commands/plugins/trust/verify.ts b/src/commands/plugins/trust/verify.ts
@@ -5,10 +5,8 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import * as os from 'os';
-import { get } from '@salesforce/ts-types';
-import { flags, FlagsConfig, SfdxCommand } from '@salesforce/command';
-import { Messages, SfError } from '@salesforce/core';
+import { SfCommand, Flags, loglevel } from '@salesforce/sf-plugins-core';
+import { Messages, SfError, Logger } from '@salesforce/core';
 import { ConfigContext, InstallationVerification, VerificationConfig } from '../../../shared/installationVerification';
 import { NpmName } from '../../../shared/NpmName';
 
@@ -20,58 +18,59 @@ export interface VerifyResponse {
   verified: boolean;
 }
 
-export class Verify extends SfdxCommand {
+export class Verify extends SfCommand<VerifyResponse> {
+  public static readonly summary = messages.getMessage('summary');
   public static readonly description = messages.getMessage('description');
-  public static readonly examples = messages.getMessage('examples').split(os.EOL);
+  public static readonly examples = messages.getMessages('examples');
   public static readonly hidden: true;
-  protected static readonly flagsConfig: FlagsConfig = {
-    npm: flags.string({
+  public static flags = {
+    npm: Flags.string({
       char: 'n',
       required: true,
-      description: messages.getMessage('flags.npm'),
+      summary: messages.getMessage('flags.npm'),
     }),
-    registry: flags.string({
+    registry: Flags.string({
       char: 'r',
-      required: false,
-      description: messages.getMessage('flags.registry'),
+      summary: messages.getMessage('flags.registry'),
     }),
+    loglevel,
   };
 
   private static getVerifier(npmName: NpmName, config: ConfigContext): InstallationVerification {
     return new InstallationVerification().setPluginNpmName(npmName).setConfig(config);
   }
 
   public async run(): Promise<VerifyResponse> {
-    this.ux.log('Checking for digital signature.');
+    const { flags } = await this.parse(Verify);
+    const logger = await Logger.child('verify');
+    this.log('Checking for digital signature.');
 
-    const npmName: NpmName = NpmName.parse(this.flags.npm as string);
+    const npmName: NpmName = NpmName.parse(flags.npm);
 
-    this.logger.debug(`running verify command for npm: ${npmName.name}`);
+    logger.debug(`running verify command for npm: ${npmName.name}`);
 
     const vConfig = new VerificationConfig();
 
     const configContext: ConfigContext = {
-      cacheDir: get(this.config, 'cacheDir') as string,
-      configDir: get(this.config, 'configDir') as string,
-      dataDir: get(this.config, 'dataDir') as string,
-      cliRoot: get(this.config, 'root') as string,
+      cacheDir: this.config.cacheDir,
+      configDir: this.config.configDir,
+      dataDir: this.config.dataDir,
+      cliRoot: this.config.root,
     };
 
-    this.logger.debug(`cacheDir: ${configContext.cacheDir}`);
-    this.logger.debug(`configDir: ${configContext.configDir}`);
-    this.logger.debug(`dataDir: ${configContext.dataDir}`);
+    logger.debug(`cacheDir: ${configContext.cacheDir}`);
+    logger.debug(`configDir: ${configContext.configDir}`);
+    logger.debug(`dataDir: ${configContext.dataDir}`);
 
     vConfig.verifier = Verify.getVerifier(npmName, configContext);
 
-    vConfig.log = this.ux.log.bind(this.ux) as (msg: string) => void;
-
-    if (this.flags.registry) {
-      process.env.SFDX_NPM_REGISTRY = this.flags.registry as string;
+    if (flags.registry) {
+      process.env.SFDX_NPM_REGISTRY = flags.registry;
     }
 
     try {
       const meta = await vConfig.verifier.verify();
-      this.logger.debug(`meta.verified: ${meta.verified}`);
+      logger.debug(`meta.verified: ${meta.verified}`);
 
       if (!meta.verified) {
         const e = messages.createError('FailedDigitalSignatureVerification');
@@ -82,14 +81,14 @@ export class Verify extends SfdxCommand {
       }
       const message = `Successfully validated digital signature for ${npmName.name}.`;
 
-      if (!this.flags.json) {
+      if (!flags.json) {
         vConfig.log(message);
       } else {
         return { message, verified: true };
       }
     } catch (error) {
       const err = error as SfError;
-      this.logger.debug(`err reported: ${JSON.stringify(err, null, 4)}`);
+      logger.debug(`err reported: ${JSON.stringify(err, null, 4)}`);
       const response: VerifyResponse = {
         verified: false,
         message: err.message,

diff --git a/src/shared/installationVerification.ts b/src/shared/installationVerification.ts
@@ -458,17 +458,13 @@ export class VerificationConfig {
 }
 
 export async function doPrompt(): Promise<void> {
-  const prompter = new Prompter();
-  const { confirm } = await prompter.prompt<{ confirm: boolean }>([
-    {
-      type: 'confirm',
-      name: 'confirm',
-      message: 'This plugin is not digitally signed and its authenticity cannot be verified. Continue installation?',
-      default: false,
-    },
-  ]);
-
-  if (!confirm) {
+  if (
+    !(await new Prompter().confirm(
+      'This plugin is not digitally signed and its authenticity cannot be verified. Continue installation?',
+      30000,
+      false
+    ))
+  ) {
     throw new SfError('The user canceled the plugin installation.', 'InstallationCanceledError');
   }
 }

diff --git a/src/shared/npmCommand.ts b/src/shared/npmCommand.ts
@@ -12,10 +12,9 @@ import * as path from 'path';
 import * as fs from 'fs';
 import npmRunPath from 'npm-run-path';
 import * as shelljs from 'shelljs';
-
 import { SfError } from '@salesforce/core';
-import { UX } from '@salesforce/command';
 import { sleep, parseJson } from '@salesforce/kit';
+import { Ux } from '@salesforce/sf-plugins-core';
 
 export type NpmMeta = {
   tarballUrl?: string;
@@ -70,11 +69,7 @@ export class NpmCommand {
       env: npmRunPath.env({ env: process.env }),
     });
     if (npmCmdResult.code !== 0) {
-      const err = new SfError(npmCmdResult.stderr, 'ShellExecError');
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore override readonly .name field
-      err.name = 'ShellExecError';
-      throw err;
+      throw new SfError(npmCmdResult.stderr, 'ShellExecError');
     }
 
     return npmCmdResult;
@@ -226,15 +221,15 @@ export class NpmModule {
   // leave it because it's stubbed in the test
   // eslint-disable-next-line class-methods-use-this
   public async pollForAvailability(checkFn: () => void): Promise<void> {
-    const ux = await UX.create();
     const isNonTTY = process.env.CI !== undefined || process.env.CIRCLECI !== undefined;
     let found = false;
     let attempts = 0;
     const maxAttempts = 300;
 
-    const start = isNonTTY ? (msg: string): UX => ux.log(msg) : (msg: string): void => ux.startSpinner(msg);
-    const update = isNonTTY ? (msg: string): UX => ux.log(msg) : (msg: string): void => ux.setSpinnerStatus(msg);
-    const stop = isNonTTY ? (msg: string): UX => ux.log(msg) : (msg: string): void => ux.stopSpinner(msg);
+    const ux = new Ux({ jsonEnabled: isNonTTY });
+    const start = isNonTTY ? (msg: string): void => ux.log(msg) : (msg: string): void => ux.spinner.start(msg);
+    const update = isNonTTY ? (msg: string): void => ux.log(msg) : (msg: string): string => (ux.spinner.status = msg);
+    const stop = isNonTTY ? (msg: string): void => ux.log(msg) : (msg: string): void => ux.spinner.stop(msg);
 
     start('Polling for new version(s) to become available on npm');
     while (!found && attempts < maxAttempts) {

diff --git a/test/hooks/verifyInstallSignatureHook.test.ts b/test/hooks/verifyInstallSignatureHook.test.ts
@@ -30,7 +30,7 @@ describe('plugin install hook', () => {
     });
     stubMethod(sandbox, vConfig.verifier, 'isAllowListed').callsFake(async () => false);
 
-    promptSpy = stubMethod(sandbox, Prompter.prototype, 'prompt').resolves({ confirm: false });
+    promptSpy = stubMethod(sandbox, Prompter.prototype, 'confirm').resolves(false);
   });
 
   afterEach(() => {

diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts
@@ -6,21 +6,36 @@
  */
 
 import * as path from 'path';
-import * as fs from 'fs/promises';
+import * as fs from 'fs';
+import * as os from 'os';
 import { expect } from 'chai';
 import { TestSession, execCmd, execInteractiveCmd, Interaction } from '@salesforce/cli-plugins-testkit';
 
-const SIGNED_MODULE_NAME = '@salesforce/plugin-user';
-const UNSIGNED_MODULE_NAME = '@mshanemc/plugin-streaming';
-let session: TestSession;
-
 describe('plugins:install commands', () => {
+  const SIGNED_MODULE_NAME = '@salesforce/plugin-user';
+  const UNSIGNED_MODULE_NAME = '@mshanemc/plugin-streaming';
+  const UNSIGNED_MODULE_NAME2 = '@mshanemc/sfdx-sosl';
+  let session: TestSession;
+
   before(async () => {
-    session = await TestSession.create();
-    await fs.mkdir(path.join(session.homeDir, '.sfdx'), { recursive: true });
+    session = await TestSession.create({ devhubAuthStrategy: 'NONE' });
+    await fs.promises.mkdir(path.join(session.homeDir, '.sfdx'), { recursive: true });
 
     const fileData: string = JSON.stringify({ acknowledged: true }, null, 2);
-    await fs.writeFile(path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), fileData);
+    await fs.promises.writeFile(path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), fileData);
+
+    const configDir = path.join(session.homeDir, '.config', 'sfdx');
+    await fs.promises.mkdir(configDir, { recursive: true });
+
+    const unsignedMod: string = JSON.stringify([UNSIGNED_MODULE_NAME2], null, 2);
+    await fs.promises.writeFile(path.join(configDir, 'unsignedPluginAllowList.json'), unsignedMod);
+
+    // ensure that this repo's version of the plugin is used and NOT the one that shipped with the CLI
+    execCmd('plugins:link .', {
+      cwd: path.dirname(session.dir),
+      ensureExitCode: 0,
+      cli: 'sfdx',
+    });
   });
 
   after(async () => {
@@ -68,45 +83,17 @@ describe('plugins:install commands', () => {
     expect(result.stdout).to.contain('Continue installation?');
     expect(result.stdout).to.contain('Finished digital signature check');
   });
-});
-
-describe('plugins:install commands', () => {
-  before(async () => {
-    session = await TestSession.create();
-    await fs.mkdir(path.join(session.homeDir, '.sfdx'), { recursive: true });
-
-    const fileData: string = JSON.stringify({ acknowledged: true }, null, 2);
-    await fs.writeFile(path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), fileData);
-
-    const configDir = path.join(session.homeDir, '.config', 'sfdx');
-    await fs.mkdir(configDir, { recursive: true });
-
-    const unsignedMod: string = JSON.stringify([UNSIGNED_MODULE_NAME], null, 2);
-    await fs.writeFile(path.join(configDir, 'unsignedPluginAllowList.json'), unsignedMod);
-
-    execCmd('plugins:link .', {
-      cwd: path.dirname(session.dir),
-      ensureExitCode: 0,
-      cli: 'sfdx',
-    });
-  });
-
-  after(async () => {
-    await session?.zip(undefined, 'artifacts');
-    try {
-      await session?.clean();
-    } catch (error) {
-      // ignore
-    }
-  });
 
-  it('plugins:install unsigned plugin in the allow list', () => {
-    const result = execCmd(`plugins:install ${UNSIGNED_MODULE_NAME}`, {
+  // yes, macos.  oclif sometimes uses XDG, which also exists on gha's ubuntu and windows runners, but isn't handled by testkit
+  // see https://salesforce-internal.slack.com/archives/G02K6C90RBJ/p1669664263661369
+  (os.platform() === 'darwin' ? it : it.skip)('plugins:install unsigned plugin in the allow list', () => {
+    expect(fs.existsSync(path.join(session.homeDir, '.config', 'sfdx'))).to.be.true;
+    const result = execCmd(`plugins:install ${UNSIGNED_MODULE_NAME2}`, {
       ensureExitCode: 0,
       cli: 'sfdx',
     });
     expect(result.shellOutput.stdout).to.contain(
-      `The plugin [${UNSIGNED_MODULE_NAME}] is not digitally signed but it is allow-listed.`
+      `The plugin [${UNSIGNED_MODULE_NAME2}] is not digitally signed but it is allow-listed.`
     );
   });
 });