feat: support allowed github urls

messages/verify.md

@@ -38,7 +38,7 @@ A digital signature is specified for this plugin but it didn't verify against th
 
 # InstallConfirmation
 
-This plugin isn't signed by Salesforce. Only install the plugin if you trust its creator. Do you want to continue the installation?,
+%s isn't signed by Salesforce. Only install the plugin if you trust its creator. Do you want to continue the installation?,
 
 # SuggestAllowList
 

src/hooks/verifyInstallSignature.ts

@@ -6,14 +6,15 @@
  */
 
 import { Hook } from '@oclif/core';
-import { Logger } from '@salesforce/core';
+import { Logger, Messages } from '@salesforce/core';
 import { ux } from '@oclif/core';
 import {
   ConfigContext,
   doInstallationCodeSigningVerification,
   doPrompt,
   InstallationVerification,
   VerificationConfig,
+  isAllowListed,
 } from '../shared/installationVerification.js';
 
 import { NpmName } from '../shared/NpmName.js';
@@ -43,7 +44,7 @@ export const hook: Hook.PluginsPreinstall = async function (options) {
       npmName.tag = npmName.tag.slice(1);
     }
 
-    const configContext: ConfigContext = {
+    const configContext = {
       cacheDir: options.config.cacheDir,
       configDir: options.config.configDir,
       dataDir: options.config.dataDir,
@@ -64,6 +65,18 @@ export const hook: Hook.PluginsPreinstall = async function (options) {
       logger.debug(error.message);
       this.error(error);
     }
+  } else if (options.plugin.url) {
+    const isAllowed = await isAllowListed({
+      logger: await Logger.child('verifyInstallSignature'),
+      name: options.plugin.url,
+      configPath: options.config.configDir,
+    });
+    if (isAllowed) {
+      const messages = Messages.loadMessages('@salesforce/plugin-trust', 'verify');
+      ux.log(messages.getMessage('SkipSignatureCheck', [options.plugin.url]));
+    } else {
+      await doPrompt(options.plugin.url);
+    }
   } else {
     await doPrompt();
   }

src/shared/installationVerification.ts

@@ -156,6 +156,32 @@ const errorHandlerForVerify = (err: Error): Error => {
 export const getNpmRegistry = (): URL =>
   new URL(process.env.SF_NPM_REGISTRY ?? process.env.SFDX_NPM_REGISTRY ?? DEFAULT_REGISTRY);
 
+export async function isAllowListed({
+  logger,
+  configPath,
+  name,
+}: {
+  logger: Logger;
+  configPath: string;
+  name?: string;
+}): Promise<boolean> {
+  const allowListedFilePath = path.join(configPath, ALLOW_LIST_FILENAME);
+  logger.debug(`isAllowListed | allowlistFilePath: ${allowListedFilePath}`);
+  let fileContent: string;
+  try {
+    fileContent = await fs.promises.readFile(allowListedFilePath, 'utf8');
+    const allowlistArray = JSON.parse(fileContent) as string[];
+    logger.debug('isAllowListed | Successfully parsed allowlist.');
+    return name ? allowlistArray.includes(name) : false;
+  } catch (err) {
+    if (err instanceof Error && 'code' in err && err.code === 'ENOENT') {
+      return false;
+    } else {
+      throw err;
+    }
+  }
+}
+
 /**
  * class for verifying a digital signature pack of an npm
  */
@@ -236,23 +262,11 @@ export class InstallationVerification implements Verifier {
   }
 
   public async isAllowListed(): Promise<boolean> {
-    const logger = await this.getLogger();
-    const allowListedFilePath = path.join(this.getConfigPath() ?? '', ALLOW_LIST_FILENAME);
-    logger.debug(`isAllowListed | allowlistFilePath: ${allowListedFilePath}`);
-    let fileContent: string;
-    try {
-      fileContent = await fs.promises.readFile(allowListedFilePath, 'utf8');
-      const allowlistArray = JSON.parse(fileContent) as string[];
-      logger.debug('isAllowListed | Successfully parsed allowlist.');
-      const nameToFind = this.pluginNpmName?.toString();
-      return nameToFind ? allowlistArray.includes(nameToFind) : false;
-    } catch (err) {
-      if (err instanceof Error && 'code' in err && err.code === 'ENOENT') {
-        return false;
-      } else {
-        throw err;
-      }
-    }
+    return isAllowListed({
+      logger: await this.getLogger(),
+      configPath: this.getConfigPath() ?? '',
+      name: this.pluginNpmName?.toString(),
+    });
   }
 
   /**
@@ -437,9 +451,14 @@ export class VerificationConfig {
   }
 }
 
-export async function doPrompt(): Promise<void> {
+export async function doPrompt(plugin?: string): Promise<void> {
   const messages = Messages.loadMessages('@salesforce/plugin-trust', 'verify');
-  if (!(await prompts.confirm({ message: messages.getMessage('InstallConfirmation'), ms: 30_000 }))) {
+  if (
+    !(await prompts.confirm({
+      message: messages.getMessage('InstallConfirmation', [plugin ?? 'This plugin']),
+      ms: 30_000,
+    }))
+  ) {
     throw new SfError('The user canceled the plugin installation.', 'InstallationCanceledError');
   }
   // they approved the plugin.  Let them know how to automate this.
@@ -473,7 +492,7 @@ export async function doInstallationCodeSigningVerification(
         if (!verificationConfig.verifier) {
           throw new Error('VerificationConfig.verifier is not set.');
         }
-        return await doPrompt();
+        return await doPrompt(plugin.plugin);
       } else if (err.name === 'PluginNotFound' || err.name === 'PluginAccessDenied') {
         throw setErrorName(new SfError(err.message ?? 'The user canceled the plugin installation.'), '');
       }

test/hooks/verifyInstallSignatureHook.test.ts

@@ -4,22 +4,27 @@
  * 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 { expect, config } from 'chai';
+import fs from 'node:fs';
+import { assert, expect, config as chaiConfig } from 'chai';
 import sinon from 'sinon';
-
 import { stubMethod } from '@salesforce/ts-sinon';
 
 import { prompts } from '@salesforce/sf-plugins-core';
+import { Config, ux } from '@oclif/core';
 import { InstallationVerification, VerificationConfig } from '../../src/shared/installationVerification.js';
-import { hook } from '../../src/hooks/verifyInstallSignature.js';
 
-config.truncateThreshold = 0;
+chaiConfig.truncateThreshold = 0;
 
 describe('plugin install hook', () => {
   let sandbox: sinon.SinonSandbox;
   let vConfig: VerificationConfig;
   let promptSpy: sinon.SinonSpy;
   let verifySpy: sinon.SinonSpy;
+  let config: Config;
+
+  before(async () => {
+    config = await Config.load(import.meta.url);
+  });
 
   beforeEach(() => {
     sandbox = sinon.createSandbox();
@@ -34,52 +39,50 @@ describe('plugin install hook', () => {
     stubMethod(sandbox, vConfig.verifier, 'isAllowListed').callsFake(async () => false);
 
     promptSpy = stubMethod(sandbox, prompts, 'confirm').resolves(false);
+    stubMethod(sandbox, ux, 'log').callsFake(() => {});
   });
 
   afterEach(() => {
     sandbox.restore();
   });
 
   it('exits by calling this.error', async () => {
-    let calledError = false;
-    await hook.call(
-      // @ts-expect-error not a valid mock for context
-      {
-        error: () => (calledError = true),
-      },
-      {
-        plugin: { name: 'test', type: 'npm' },
-        config: {},
-      }
-    );
-    expect(calledError).to.equal(true);
-  });
-
-  it('should prompt for repo urls', async () => {
     try {
-      await hook.call(
-        // @ts-expect-error not a valid mock for context
-        {},
-        {
-          plugin: { name: 'test', type: 'repo' },
-          config: {},
-        }
-      );
+      await config.runHook('plugins:preinstall:verify:signature', {
+        plugin: { name: 'test', type: 'npm', tag: 'latest' },
+      });
+      assert.fail('Expected error to be thrown');
     } catch (error) {
       expect(error).to.have.property('name', 'InstallationCanceledError');
-      expect(promptSpy.called).to.be.true;
     }
   });
 
-  it('should skip signature verification for JIT plugins with matching version', async () => {
-    await hook.call(
-      // @ts-expect-error not a valid mock for context
-      {},
-      {
-        plugin: { name: '@ns/test', type: 'npm', tag: '1.2.3' },
-        config: { pjson: { oclif: { jitPlugins: { '@ns/test': '1.2.3' } } } },
-      }
-    );
+  it('should prompt for repo urls that are not allowlisted', async () => {
+    const result = await config.runHook('plugins:preinstall:verify:signature', {
+      plugin: { url: 'https://github.com/oclif/plugin-version', type: 'repo' },
+    });
+
+    expect(result.failures).to.have.length(1);
+    expect(result.failures[0].error).to.have.property('name', 'InstallationCanceledError');
+    expect(promptSpy.called).to.be.true;
+  });
+
+  it('should not prompt for repo urls that are allowlisted', async () => {
+    stubMethod(sandbox, fs.promises, 'readFile').resolves(JSON.stringify(['https://github.com/oclif/plugin-version']));
+    const result = await config.runHook('plugins:preinstall:verify:signature', {
+      plugin: { url: 'https://github.com/oclif/plugin-version', type: 'repo' },
+    });
+
+    expect(result.failures).to.have.length(0);
+    expect(result.successes).to.have.length(1);
+    expect(promptSpy.called).to.be.false;
+  });
+
+  it.only('should skip signature verification for JIT plugins with matching version', async () => {
+    sandbox.stub(config, 'pjson').value({ oclif: { jitPlugins: { '@ns/test': '1.2.3' } } });
+    await config.runHook('plugins:preinstall:verify:signature', {
+      plugin: { name: '@ns/test', type: 'npm', tag: '1.2.3' },
+    });
     expect(promptSpy.called).to.be.false;
     expect(verifySpy.called).to.be.false;
   });

test/nuts/plugin-install.nut.ts

@@ -73,7 +73,7 @@ describe('plugins:install commands', () => {
     );
 
     expect(replaceAllWhitespace(result.stdout)).to.contain(
-      replaceAllWhitespace(messages.getMessage('InstallConfirmation'))
+      replaceAllWhitespace(messages.getMessage('InstallConfirmation', [UNSIGNED_MODULE_NAME]))
     );
     expect(result.stdout).to.contain('Do you want to continue the installation?');
     expect(result.stderr).to.contain('The user canceled the plugin installation');
@@ -89,7 +89,7 @@ describe('plugins:install commands', () => {
       }
     );
     expect(replaceAllWhitespace(result.stdout)).to.contain(
-      replaceAllWhitespace(messages.getMessage('InstallConfirmation'))
+      replaceAllWhitespace(messages.getMessage('InstallConfirmation', [UNSIGNED_MODULE_NAME]))
     );
     expect(result.stdout).to.contain('Do you want to continue the installation?');
     expect(result.stdout).to.contain('Finished digital signature check');