refactor: TS strict (#475)

* test: set a watch

* chore: strict mode setup, lockfile update

* fix: use strict

* refactor: remove unused exports

* refactor: remove "as" assertions

* refactor: fewer eslint disables

* refactor: less eslint comments

* refactor: restore custom error handler for cert issues

.mocharc.json

@@ -1,7 +1,8 @@
 {
   "require": "ts-node/register,source-map-support/register",
   "watch-extensions": "ts",
+  "watch-files": ["src", "test"],
   "recursive": true,
   "reporter": "spec",
-  "timeout": 5000
+  "timeout": 10000
 }

package.json

@@ -228,4 +228,4 @@
       "output": []
     }
   }
-}
+}

src/commands/plugins/trust/verify.ts

@@ -9,6 +9,7 @@ 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';
+import { setErrorName } from '../../../shared/errors';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.loadMessages('@salesforce/plugin-trust', 'verify');
@@ -74,28 +75,26 @@ export class Verify extends SfCommand<VerifyResponse> {
 
       if (!meta.verified) {
         const e = messages.createError('FailedDigitalSignatureVerification');
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore override readonly .name field
-        e.name = 'FailedDigitalSignatureVerification';
-        throw e;
+        throw setErrorName(e, 'FailedDigitalSignatureVerification');
       }
       const message = `Successfully validated digital signature for ${npmName.name}.`;
 
       if (!flags.json) {
         vConfig.log(message);
-      } else {
-        return { message, verified: true };
       }
+      return { message, verified: true };
     } catch (error) {
-      const err = error as SfError;
-      logger.debug(`err reported: ${JSON.stringify(err, null, 4)}`);
+      if (!(error instanceof Error)) {
+        throw error;
+      }
+      logger.debug(`err reported: ${JSON.stringify(error, null, 4)}`);
       const response: VerifyResponse = {
         verified: false,
-        message: err.message,
+        message: error.message,
       };
 
-      if (err.name === 'NotSigned') {
-        let message: string = err.message;
+      if (error.name === 'NotSigned') {
+        let message: string = error.message;
         if (await vConfig.verifier.isAllowListed()) {
           message = `The plugin [${npmName.name}] is not digitally signed but it is allow-listed.`;
           vConfig.log(message);
@@ -107,7 +106,7 @@ export class Verify extends SfCommand<VerifyResponse> {
         }
         return response;
       }
-      throw SfError.wrap(err);
+      throw SfError.wrap(error);
     }
   }
 }

src/hooks/jitPluginInstall.ts

@@ -14,7 +14,6 @@ declare const global: TelemetryGlobal;
 
 const hook: Hook<'jit_plugin_not_installed'> = async function (opts) {
   try {
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
     global.cliTelemetry?.record({
       eventName: 'JIT_INSTALL_STARTED',
       type: 'EVENT',
@@ -25,7 +24,6 @@ const hook: Hook<'jit_plugin_not_installed'> = async function (opts) {
 
     await opts.config.runCommand('plugins:install', [`${opts.command.pluginName}@${opts.pluginVersion}`]);
 
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
     global.cliTelemetry?.record({
       eventName: 'JIT_INSTALL_SUCCESS',
       type: 'EVENT',
@@ -34,15 +32,14 @@ const hook: Hook<'jit_plugin_not_installed'> = async function (opts) {
       command: opts.command.id,
     });
   } catch (error) {
-    const err = error as Error;
-
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
     global.cliTelemetry?.record({
       eventName: 'JIT_INSTALL_FAILED',
       type: 'EVENT',
-      message: err.message,
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-member-access
-      stackTrace: err?.stack?.replace(new RegExp(os.homedir(), 'g'), AppInsights.GDPR_HIDDEN),
+      message: error instanceof Error ? error.message : 'malformed error',
+      stackTrace:
+        error instanceof Error
+          ? error?.stack?.replace(new RegExp(os.homedir(), 'g'), AppInsights.GDPR_HIDDEN)
+          : undefined,
       version: opts.pluginVersion,
       plugin: opts.command.pluginName,
       command: opts.command.id,

src/hooks/verifyInstallSignature.ts

@@ -6,7 +6,7 @@
  */
 
 import { Hook } from '@oclif/core';
-import { Logger, SfError } from '@salesforce/core';
+import { Logger } from '@salesforce/core';
 import { ux } from '@oclif/core';
 import {
   ConfigContext,
@@ -18,17 +18,6 @@ import {
 
 import { NpmName } from '../shared/NpmName';
 
-/**
- * Build a VerificationConfig. Useful for testing.
- */
-export class VerificationConfigBuilder {
-  public static build(npmName: NpmName, configContext: ConfigContext): VerificationConfig {
-    const vConfig = new VerificationConfig();
-    vConfig.verifier = new InstallationVerification().setPluginNpmName(npmName).setConfig(configContext);
-    return vConfig;
-  }
-}
-
 export const hook: Hook.PluginsPreinstall = async function (options) {
   if (options.plugin && options.plugin.type === 'npm') {
     const logger = await Logger.child('verifyInstallSignature');
@@ -37,6 +26,7 @@ export const hook: Hook.PluginsPreinstall = async function (options) {
     // skip if the plugin version being installed is listed in the CLI's JIT config
     if (
       plugin.tag &&
+      options.config.pjson.oclif.jitPlugins &&
       plugin.name in options.config.pjson.oclif.jitPlugins &&
       options.config.pjson.oclif.jitPlugins?.[plugin.name] === plugin.tag
     ) {
@@ -60,21 +50,32 @@ export const hook: Hook.PluginsPreinstall = async function (options) {
       cliRoot: options.config.root,
     };
 
-    const vConfig = VerificationConfigBuilder.build(npmName, configContext);
+    const vConfig = buildVerificationConfig(npmName, configContext);
     logger.debug('finished building the VerificationConfigBuilder');
 
     try {
       logger.debug('doing verification');
       await doInstallationCodeSigningVerification(configContext, { plugin: plugin.name, tag: plugin.tag }, vConfig);
       ux.log('Finished digital signature check.');
     } catch (error) {
-      const err = error as SfError;
-      logger.debug(err.message);
-      this.error(err);
+      if (!(error instanceof Error)) {
+        throw error;
+      }
+      logger.debug(error.message);
+      this.error(error);
     }
   } else {
     await doPrompt();
   }
 };
 
 export default hook;
+
+/**
+ * Build a VerificationConfig. Useful for testing.
+ */
+const buildVerificationConfig = (npmName: NpmName, configContext: ConfigContext): VerificationConfig => {
+  const vConfig = new VerificationConfig();
+  vConfig.verifier = new InstallationVerification().setPluginNpmName(npmName).setConfig(configContext);
+  return vConfig;
+};

src/shared/NpmName.ts

@@ -5,6 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 import { SfError } from '@salesforce/core';
+import { setErrorName } from './errors';
 
 interface NpmNameInfo {
   scope: string;
@@ -20,9 +21,11 @@ interface NpmNameInfo {
  */
 export class NpmName {
   public static readonly DEFAULT_TAG = 'latest';
-  public scope: string;
   public tag: string;
-  public name: string;
+  // next 2 props won't exist until after parse is called
+  // TODO: make this more functional and deterministic
+  public scope!: string;
+  public name!: string;
 
   /**
    * Private ctor. Use static parse method.
@@ -38,11 +41,10 @@ export class NpmName {
    */
   public static parse(npmName: string): NpmName {
     if (!npmName || npmName.length < 1) {
-      const err = new SfError('The npm name is missing or invalid.', 'MissingOrInvalidNpmName');
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore override readonly .name field
-      err.name = 'MissingOrInvalidNpmName';
-      throw err;
+      throw setErrorName(
+        new SfError('The npm name is missing or invalid.', 'MissingOrInvalidNpmName'),
+        'MissingOrInvalidNpmName'
+      );
     }
 
     const returnNpmName = new NpmName();
@@ -83,11 +85,7 @@ export class NpmName {
     } else if (subComponents.length === 1) {
       returnNpmName.name = NpmName.validateComponentString(subComponents[0]);
     } else {
-      const err = new SfError('The npm name is invalid.', 'InvalidNpmName');
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore override readonly .name field
-      err.name = 'InvalidNpmName';
-      throw err;
+      throw setErrorName(new SfError('The npm name is invalid.', 'InvalidNpmName'), 'InvalidNpmName');
     }
   }
 
@@ -102,11 +100,10 @@ export class NpmName {
     if (trimmedName && trimmedName.length > 0) {
       return trimmedName;
     } else {
-      const err = new SfError('The npm name is missing or invalid.', 'MissingOrInvalidNpmName');
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore override readonly .name field
-      err.name = 'MissingOrInvalidNpmName';
-      throw err;
+      throw setErrorName(
+        new SfError('The npm name is missing or invalid.', 'MissingOrInvalidNpmName'),
+        'MissingOrInvalidNpmName'
+      );
     }
   }
 

src/shared/errors.ts

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { SfError } from '@salesforce/core';
+
+/**
+ * convenience method for doing something SfError doesn't allow
+ * (explicitly setting the error name)
+ */
+export const setErrorName = (err: SfError, name: string): SfError => {
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+  // @ts-ignore override readonly .name field
+  err.name = name;
+  return err;
+};