Guard object scans entries and read npm plugins from cached scans context

Author: kzhou314

.github/actions/find/src/findForUrl.ts

@@ -41,7 +41,7 @@ export async function findForUrl(
     const scansContext = getScansContext()
 
     if (scansContext.shouldRunPlugins) {
-      const plugins = await loadPlugins(scansContext.npmPlugins)
+      const plugins = await loadPlugins()
       for (const plugin of plugins) {
         if (scansContext.scansToPerform.includes(plugin.name)) {
           core.info(`Running plugin: ${plugin.name}`)

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

@@ -4,7 +4,8 @@ import {fileURLToPath} from 'url'
 import * as core from '@actions/core'
 import {loadPluginViaJsFile, loadPluginViaTsFile} from './pluginFileLoaders.js'
 import {loadPluginViaNpm} from './pluginNpmLoader.js'
-import type {NpmPluginRequest, Plugin, PluginDefaultParams} from './types.js'
+import type {Plugin, PluginDefaultParams} from './types.js'
+import {getScansContext} from '../scansContextProvider.js'
 
 // Helper to get __dirname equivalent in ES Modules
 const __filename = fileURLToPath(import.meta.url)
@@ -21,13 +22,13 @@ export function getPlugins() {
 }
 let pluginsLoaded = false
 
-export async function loadPlugins(npmPlugins: NpmPluginRequest[] = []) {
+export async function loadPlugins() {
   try {
     if (!pluginsLoaded) {
       core.info('loading plugins')
       await loadBuiltInPlugins()
       await loadCustomPlugins()
-      await loadNpmPlugins(npmPlugins)
+      await loadNpmPlugins()
     }
   } catch {
     plugins.length = 0
@@ -91,7 +92,8 @@ export async function loadCustomPlugins() {
 const FIRST_PARTY_NPM_PLUGINS = ['@github/accessibility-scanner-alt-text-plugin']
 
 // exported for mocking/testing. not for actual use
-export async function loadNpmPlugins(npmPlugins: NpmPluginRequest[]) {
+export async function loadNpmPlugins() {
+  const {npmPlugins} = getScansContext()
   if (npmPlugins.length === 0) {
     return
   }

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

@@ -28,7 +28,12 @@ export function getScansContext() {
     for (const entry of rawScans) {
       if (typeof entry === 'string') {
         scansToPerform.push(entry)
-      } else if (entry && typeof entry.name === 'string' && typeof entry.package === 'string') {
+      } else if (
+        typeof entry === 'object' &&
+        entry !== null &&
+        typeof entry.name === 'string' &&
+        typeof entry.package === 'string'
+      ) {
         scansToPerform.push(entry.name)
         npmPlugins.push({
           name: entry.name,

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

@@ -117,15 +117,13 @@ describe('findForUrl', () => {
       expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0)
     })
 
-    it('forwards object-form NPM plugin entries to loadPlugins', async () => {
+    it('runs plugins when a scans entry is an object-form NPM plugin', 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},
-      ])
+      expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1)
     })
   })
 

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

@@ -4,14 +4,25 @@ import * as childProcess from 'child_process'
 import * as core from '@actions/core'
 import * as pluginManager from '../src/pluginManager/index.js'
 import * as npmPluginLoader from '../src/pluginManager/pluginNpmLoader.js'
-import type {Plugin} from '../src/pluginManager/types.js'
+import * as scansContextProvider from '../src/scansContextProvider.js'
+import type {Plugin, NpmPluginRequest} from '../src/pluginManager/types.js'
 
 vi.mock('child_process', {spy: true})
 vi.mock('@actions/core', {spy: true})
 vi.mock('../src/pluginManager/pluginNpmLoader.js', {spy: true})
+vi.mock('../src/scansContextProvider.js', {spy: true})
 
 const ALLOWED = '@github/accessibility-scanner-alt-text-plugin'
 
+function mockNpmPlugins(npmPlugins: NpmPluginRequest[]) {
+  vi.spyOn(scansContextProvider, 'getScansContext').mockReturnValue({
+    scansToPerform: npmPlugins.map(plugin => plugin.name),
+    npmPlugins,
+    shouldPerformAxeScan: false,
+    shouldRunPlugins: true,
+  })
+}
+
 describe('npmPluginLoader', () => {
   beforeEach(() => {
     vi.restoreAllMocks()
@@ -62,14 +73,16 @@ describe('loadNpmPlugins', () => {
 
   it('loads a plugin from a first-party package', async () => {
     vi.spyOn(npmPluginLoader, 'loadPluginViaNpm').mockResolvedValue({name: 'alt-text-scan', default: vi.fn()})
-    await pluginManager.loadNpmPlugins([{name: 'alt-text-scan', package: ALLOWED}])
+    mockNpmPlugins([{name: 'alt-text-scan', package: ALLOWED}])
+    await pluginManager.loadNpmPlugins()
     expect(pluginManager.getPlugins().map(plugin => plugin.name)).toContain('alt-text-scan')
   })
 
   it('skips and warns when a package is not first-party', async () => {
     const loadSpy = vi.spyOn(npmPluginLoader, 'loadPluginViaNpm').mockResolvedValue(undefined)
     const warnSpy = vi.spyOn(core, 'warning').mockImplementation(() => {})
-    await pluginManager.loadNpmPlugins([{name: 'evil', package: 'evil-pkg'}])
+    mockNpmPlugins([{name: 'evil', package: 'evil-pkg'}])
+    await pluginManager.loadNpmPlugins()
     expect(loadSpy).not.toHaveBeenCalled()
     expect(warnSpy).toHaveBeenCalled()
     expect(pluginManager.getPlugins().length).toBe(0)
@@ -78,23 +91,26 @@ describe('loadNpmPlugins', () => {
   it('skips a package that does not export a valid plugin', async () => {
     vi.spyOn(npmPluginLoader, 'loadPluginViaNpm').mockResolvedValue({name: 'bad'} as unknown as Plugin)
     const warnSpy = vi.spyOn(core, 'warning').mockImplementation(() => {})
-    await pluginManager.loadNpmPlugins([{name: 'bad', package: ALLOWED}])
+    mockNpmPlugins([{name: 'bad', package: ALLOWED}])
+    await pluginManager.loadNpmPlugins()
     expect(warnSpy).toHaveBeenCalled()
     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}])
+    mockNpmPlugins([{name: 'requested-name', package: ALLOWED}])
+    await pluginManager.loadNpmPlugins()
     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()})
-    await pluginManager.loadNpmPlugins([{name: 'dup', package: ALLOWED}])
+    mockNpmPlugins([{name: 'dup', package: ALLOWED}])
+    await pluginManager.loadNpmPlugins()
     expect(pluginManager.getPlugins().filter(plugin => plugin.name === 'dup').length).toBe(1)
   })
 })