diff --git a/.github/actions/find/action.yml b/.github/actions/find/action.yml index fb53d901..3e66cd6b 100644 --- a/.github/actions/find/action.yml +++ b/.github/actions/find/action.yml @@ -17,7 +17,7 @@ inputs: required: false default: 'false' scans: - description: 'Stringified JSON array of scans to perform. If not provided, only Axe will be performed' + description: "Stringified JSON array of scans to perform. Each entry is either a scan/plugin name (string) or, for a first-party NPM-published plugin, an object with 'name', 'package', and optional 'version'. If not provided, only Axe will be performed" required: false reduced_motion: description: 'Playwright reducedMotion setting: https://playwright.dev/docs/api/class-browser#browser-new-page-option-reduced-motion' diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index ea4f1e18..6827eeb6 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -41,7 +41,7 @@ export async function findForUrl( const scansContext = getScansContext() if (scansContext.shouldRunPlugins) { - const plugins = await loadPlugins() + const plugins = await loadPlugins(scansContext.npmPlugins) for (const plugin of plugins) { if (scansContext.scansToPerform.includes(plugin.name)) { core.info(`Running plugin: ${plugin.name}`) diff --git a/.github/actions/find/src/pluginManager/index.ts b/.github/actions/find/src/pluginManager/index.ts index c6ec5d71..ae197641 100644 --- a/.github/actions/find/src/pluginManager/index.ts +++ b/.github/actions/find/src/pluginManager/index.ts @@ -3,7 +3,8 @@ import * as path from 'path' import {fileURLToPath} from 'url' import * as core from '@actions/core' import {loadPluginViaJsFile, loadPluginViaTsFile} from './pluginFileLoaders.js' -import type {Plugin, PluginDefaultParams} from './types.js' +import {loadPluginViaNpm} from './npmPluginLoader.js' +import type {NpmPluginRequest, Plugin, PluginDefaultParams} from './types.js' // Helper to get __dirname equivalent in ES Modules const __filename = fileURLToPath(import.meta.url) @@ -20,12 +21,13 @@ export function getPlugins() { } let pluginsLoaded = false -export async function loadPlugins() { +export async function loadPlugins(npmPlugins: NpmPluginRequest[] = []) { try { if (!pluginsLoaded) { core.info('loading plugins') await loadBuiltInPlugins() await loadCustomPlugins() + await loadNpmPlugins(npmPlugins) } } catch { plugins.length = 0 @@ -55,7 +57,7 @@ export async function loadBuiltInPlugins() { await loadPluginsFromPath({pluginsPath}) } -// exported for mocking/testing. not for actual use +// export to be used for mocking/testing. not for actual use export async function loadCustomPlugins() { core.info('Loading custom plugins') const pluginsPath = path.join(process.cwd(), '.github/scanner-plugins/') @@ -75,6 +77,53 @@ export async function loadCustomPlugins() { await loadPluginsFromPath({pluginsPath, skipBuiltInPlugins: BUILT_IN_PLUGINS}) } +// 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 +export async function loadNpmPlugins(npmPlugins: NpmPluginRequest[]) { + if (npmPlugins.length === 0) { + return + } + core.info('Loading NPM plugins') + + for (const request of npmPlugins) { + // 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 + } + + const plugin = await loadPluginViaNpm(request) + if (!plugin) { + continue + } + + // Validate the package actually exports a usable plugin. + if (typeof plugin.name !== 'string' || typeof plugin.default !== 'function') { + core.warning(`Skipping NPM plugin '${request.package}' because it does not export a valid plugin`) + 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`) + continue + } + + core.info(`Found NPM plugin: ${plugin.name}`) + plugins.push(plugin) + } +} + // exported for mocking/testing. not for actual use export async function loadPluginsFromPath({ pluginsPath, diff --git a/.github/actions/find/src/pluginManager/npmPluginLoader.ts b/.github/actions/find/src/pluginManager/npmPluginLoader.ts new file mode 100644 index 00000000..ce30524b --- /dev/null +++ b/.github/actions/find/src/pluginManager/npmPluginLoader.ts @@ -0,0 +1,23 @@ +import {execFileSync} from 'child_process' +import * as core from '@actions/core' +import type {NpmPluginRequest, Plugin} from './types.js' + +// Install the package at runtime. +export function installNpmPackage(spec: string) { + execFileSync('npm', ['install', spec, '--no-save', '--no-package-lock', '--ignore-scripts'], {stdio: 'inherit'}) +} + +// Install and import a single NPM-published plugin +export async function loadPluginViaNpm(request: NpmPluginRequest): Promise { + const spec = request.version ? `${request.package}@${request.version}` : request.package + try { + core.info(`Installing NPM plugin: ${spec}`) + installNpmPackage(spec) + // Import the bare package specifier as-is; pathToFileURL would mangle it. + const imported = await import(request.package) + return imported as Plugin + } catch (e) { + core.warning(`Failed to load NPM plugin '${spec}': ${e}`) + return undefined + } +} diff --git a/.github/actions/find/src/pluginManager/types.ts b/.github/actions/find/src/pluginManager/types.ts index 8fbb315d..65b82469 100644 --- a/.github/actions/find/src/pluginManager/types.ts +++ b/.github/actions/find/src/pluginManager/types.ts @@ -10,3 +10,10 @@ export type Plugin = { name: string default: (options: PluginDefaultParams) => Promise } + +// A plugin requested from an NPM package rather than a local folder. +export type NpmPluginRequest = { + name: string + package: string + version?: string +} diff --git a/.github/actions/find/src/scansContextProvider.ts b/.github/actions/find/src/scansContextProvider.ts index 014c6d58..047f142c 100644 --- a/.github/actions/find/src/scansContextProvider.ts +++ b/.github/actions/find/src/scansContextProvider.ts @@ -1,16 +1,46 @@ import * as core from '@actions/core' +import type {NpmPluginRequest} from './pluginManager/types.js' type ScansContext = { scansToPerform: Array + npmPlugins: NpmPluginRequest[] shouldPerformAxeScan: boolean shouldRunPlugins: boolean } let scansContext: ScansContext | undefined +// A scans entry is either a plain name (core engine or local plugin folder) or +// an object describing an NPM-published plugin to install and load. +type ScanEntry = string | {name?: unknown; package?: unknown; version?: unknown} + export function getScansContext() { if (!scansContext) { const scansInput = core.getInput('scans', {required: false}) - const scansToPerform = JSON.parse(scansInput || '[]') + 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[] = [] + const npmPlugins: NpmPluginRequest[] = [] + for (const entry of rawScans) { + if (typeof entry === 'string') { + scansToPerform.push(entry) + } else if (entry && typeof entry.name === 'string' && typeof entry.package === 'string') { + scansToPerform.push(entry.name) + npmPlugins.push({ + name: entry.name, + package: entry.package, + version: typeof entry.version === 'string' ? entry.version : undefined, + }) + } else { + core.warning(`Ignoring invalid 'scans' entry: ${JSON.stringify(entry)}`) + } + } + // - if we don't have a scans input // or we do have a scans input, but it only has 1 item and its 'axe' // then we only want to run 'axe' and not the plugins @@ -19,6 +49,7 @@ export function getScansContext() { scansContext = { scansToPerform, + npmPlugins, // - if no 'scans' input is provided, we default to the existing behavior // (only axe scan) for backwards compatability. // - we can enforce using the 'scans' input in a future major release and diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts index 54244885..dd48dd81 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.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', () => { diff --git a/.github/actions/find/tests/npmPluginLoader.test.ts b/.github/actions/find/tests/npmPluginLoader.test.ts new file mode 100644 index 00000000..30a6560b --- /dev/null +++ b/.github/actions/find/tests/npmPluginLoader.test.ts @@ -0,0 +1,100 @@ +import {describe, it, expect, vi, beforeEach} from 'vitest' + +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/npmPluginLoader.js' +import type {Plugin} from '../src/pluginManager/types.js' + +vi.mock('child_process', {spy: true}) +vi.mock('@actions/core', {spy: true}) +vi.mock('../src/pluginManager/npmPluginLoader.js', {spy: true}) + +const ALLOWED = '@github/accessibility-scanner-alt-text-plugin' + +describe('npmPluginLoader', () => { + beforeEach(() => { + vi.restoreAllMocks() + vi.clearAllMocks() + }) + + describe('installNpmPackage', () => { + 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', '--no-package-lock', '--ignore-scripts'], + { + stdio: 'inherit', + }, + ) + }) + }) + + describe('loadPluginViaNpm', () => { + it('pins the version in the install spec', async () => { + const execSpy = vi.spyOn(childProcess, 'execFileSync').mockImplementation(() => Buffer.from('')) + 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', '--no-package-lock', '--ignore-scripts'], + {stdio: 'inherit'}, + ) + }) + + it('returns undefined and warns when loading fails', async () => { + vi.spyOn(childProcess, 'execFileSync').mockImplementation(() => Buffer.from('')) + const warnSpy = vi.spyOn(core, 'warning').mockImplementation(() => {}) + const plugin = await npmPluginLoader.loadPluginViaNpm({name: 'p', package: 'nonexistent-pkg-xyz'}) + expect(plugin).toBeUndefined() + expect(warnSpy).toHaveBeenCalled() + }) + }) +}) + +describe('loadNpmPlugins', () => { + beforeEach(() => { + vi.restoreAllMocks() + vi.clearAllMocks() + pluginManager.clearCache() + }) + + 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}]) + 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'}]) + expect(loadSpy).not.toHaveBeenCalled() + expect(warnSpy).toHaveBeenCalled() + expect(pluginManager.getPlugins().length).toBe(0) + }) + + 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}]) + 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}]) + 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}]) + expect(pluginManager.getPlugins().filter(plugin => plugin.name === 'dup').length).toBe(1) + }) +}) diff --git a/PLUGINS.md b/PLUGINS.md index 7f3058fa..f65ac0ef 100644 --- a/PLUGINS.md +++ b/PLUGINS.md @@ -43,6 +43,36 @@ jobs: # ... the rest of the workflow setup ``` +## Loading plugins from NPM packages + +In addition to local plugins under `./.github/scanner-plugins`, the scanner can install and load plugins published as NPM packages. This avoids having to vendor a plugin's source into your repo. + +To use an NPM plugin, pass an object (instead of a plain string) in the `scans` input with the following fields: + +- `name` — the plugin name exported by the package (used to match against `scans`, same as local plugins). +- `package` — the NPM package name to install. +- `version` — (optional) a version or dist-tag to pin. If omitted, the latest version is installed. + +Only the set of first-party packages may be loaded from NPM. Any other package is skipped with a warning. + +```yaml +jobs: + accessibility_scanner: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: github/accessibility-scanner@v3 + with: + scans: | + ["axe", {"name": "alt-text-scan", "package": "@github/accessibility-scanner-alt-text-plugin", "version": "1.0.0"}] +``` + +Notes: + +- Packages are installed at runtime with `npm install --ignore-scripts`, so install/postinstall scripts in the package will not run. Pin a `version` to avoid silently picking up future releases. +- If an NPM plugin shares a name with a built-in or local plugin, the built-in/local plugin wins and the NPM one is skipped. +- Plugin configuration works the same as local plugins: place a config-only folder at `./.github/scanner-plugins//config.json` in your repo (the plugin reads its config relative to the repo you run the workflow from). + ## Things to look out for - Plugin names should be unique. If multiple plugins have the same name, and the `scans` input array contains this name, all the plugins with that name _will_ run. However, this is not advised because if you want to turn off one plugin, you'll have to go back and change that plugin name.