Validate scans input, skip lockfile writes, and warn on name mismatches

Author: kzhou314

.github/actions/find/src/pluginManager/index.ts

@@ -77,8 +77,7 @@ export async function loadCustomPlugins() {
   await loadPluginsFromPath({pluginsPath, skipBuiltInPlugins: BUILT_IN_PLUGINS})
 }
 
-// Curated first-party packages allowed to be installed and loaded from NPM.
-// Kept intentionally small while the plugin system is being prototyped.
+// First-party packages allowed to be installed and loaded from NPM.
 const FIRST_PARTY_NPM_PLUGINS = ['@github/accessibility-scanner-alt-text-plugin']
 
 // exported for mocking/testing. not for actual use
@@ -89,7 +88,7 @@ export async function loadNpmPlugins(npmPlugins: NpmPluginRequest[]) {
   core.info('Loading NPM plugins')
 
   for (const request of npmPlugins) {
-    // Only install curated first-party packages.
+    // Only install first-party packages.
     if (!FIRST_PARTY_NPM_PLUGINS.includes(request.package)) {
       core.warning(`Skipping NPM plugin '${request.package}' because it is not a first-party package`)
       continue
@@ -106,6 +105,14 @@ export async function loadNpmPlugins(npmPlugins: NpmPluginRequest[]) {
       continue
     }
 
+    // The requested name (in 'scans') gates invocation, so a mismatch means the plugin would load but never run.
+    if (plugin.name !== request.name) {
+      core.warning(
+        `Skipping NPM plugin '${request.package}' because it exported name '${plugin.name}', which does not match requested name '${request.name}'`,
+      )
+      continue
+    }
+
     // Built-in and local plugins take precedence over NPM ones of the same name.
     if (plugins.some(existing => existing.name === plugin.name)) {
       core.info(`Skipping NPM plugin '${plugin.name}' because a plugin with that name is already loaded`)

.github/actions/find/src/pluginManager/npmPluginLoader.ts

@@ -2,9 +2,9 @@ import {execFileSync} from 'child_process'
 import * as core from '@actions/core'
 import type {NpmPluginRequest, Plugin} from './types.js'
 
-// Install the package at runtime
+// Install the package at runtime.
 export function installNpmPackage(spec: string) {
-  execFileSync('npm', ['install', spec, '--no-save', '--ignore-scripts'], {stdio: 'inherit'})
+  execFileSync('npm', ['install', spec, '--no-save', '--no-package-lock', '--ignore-scripts'], {stdio: 'inherit'})
 }
 
 // Install and import a single NPM-published plugin

.github/actions/find/src/scansContextProvider.ts

@@ -16,7 +16,12 @@ type ScanEntry = string | {name?: unknown; package?: unknown; version?: unknown}
 export function getScansContext() {
   if (!scansContext) {
     const scansInput = core.getInput('scans', {required: false})
-    const rawScans = JSON.parse(scansInput || '[]') as ScanEntry[]
+    const parsed = JSON.parse(scansInput || '[]')
+    // Fail early with a clear message instead of a cryptic 'not iterable' error later.
+    if (!Array.isArray(parsed)) {
+      throw new Error(`'scans' input must be a JSON array, got: ${scansInput}`)
+    }
+    const rawScans = parsed as ScanEntry[]
 
     // scansToPerform holds the name of every scan/plugin to run
     const scansToPerform: string[] = []

.github/actions/find/tests/findForUrl.test.ts

@@ -116,6 +116,17 @@ describe('findForUrl', () => {
       expect(loadedPlugins[0].default).toHaveBeenCalledTimes(1)
       expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0)
     })
+
+    it('forwards object-form NPM plugin entries to loadPlugins', async () => {
+      loadedPlugins = []
+      actionInput = JSON.stringify([{name: 'alt-text-scan', package: '@github/accessibility-scanner-alt-text-plugin'}])
+      clearAll()
+
+      await findForUrl('test.com')
+      expect(pluginManager.loadPlugins).toHaveBeenCalledWith([
+        {name: 'alt-text-scan', package: '@github/accessibility-scanner-alt-text-plugin', version: undefined},
+      ])
+    })
   })
 
   describe('axe finding categorization', () => {

.github/actions/find/tests/npmPluginLoader.test.ts

@@ -19,12 +19,16 @@ describe('npmPluginLoader', () => {
   })
 
   describe('installNpmPackage', () => {
-    it('installs with --no-save and --ignore-scripts', () => {
+    it('installs with --no-save, --no-package-lock and --ignore-scripts', () => {
       const execSpy = vi.spyOn(childProcess, 'execFileSync').mockImplementation(() => Buffer.from(''))
       npmPluginLoader.installNpmPackage('some-pkg@1.0.0')
-      expect(execSpy).toHaveBeenCalledWith('npm', ['install', 'some-pkg@1.0.0', '--no-save', '--ignore-scripts'], {
-        stdio: 'inherit',
-      })
+      expect(execSpy).toHaveBeenCalledWith(
+        'npm',
+        ['install', 'some-pkg@1.0.0', '--no-save', '--no-package-lock', '--ignore-scripts'],
+        {
+          stdio: 'inherit',
+        },
+      )
     })
   })
 
@@ -34,7 +38,7 @@ describe('npmPluginLoader', () => {
       await npmPluginLoader.loadPluginViaNpm({name: 'p', package: 'nonexistent-pkg-xyz', version: '2.3.4'})
       expect(execSpy).toHaveBeenCalledWith(
         'npm',
-        ['install', 'nonexistent-pkg-xyz@2.3.4', '--no-save', '--ignore-scripts'],
+        ['install', 'nonexistent-pkg-xyz@2.3.4', '--no-save', '--no-package-lock', '--ignore-scripts'],
         {stdio: 'inherit'},
       )
     })
@@ -79,6 +83,14 @@ describe('loadNpmPlugins', () => {
     expect(pluginManager.getPlugins().length).toBe(0)
   })
 
+  it('skips an NPM plugin whose exported name does not match the requested name', async () => {
+    vi.spyOn(npmPluginLoader, 'loadPluginViaNpm').mockResolvedValue({name: 'actual-name', default: vi.fn()})
+    const warnSpy = vi.spyOn(core, 'warning').mockImplementation(() => {})
+    await pluginManager.loadNpmPlugins([{name: 'requested-name', package: ALLOWED}])
+    expect(warnSpy).toHaveBeenCalled()
+    expect(pluginManager.getPlugins().length).toBe(0)
+  })
+
   it('skips an NPM plugin whose name collides with an already-loaded plugin', async () => {
     pluginManager.getPlugins().push({name: 'dup', default: vi.fn()})
     vi.spyOn(npmPluginLoader, 'loadPluginViaNpm').mockResolvedValue({name: 'dup', default: vi.fn()})