Skip to content
2 changes: 1 addition & 1 deletion .github/actions/find/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/find/src/findForUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
kzhou314 marked this conversation as resolved.
if (scansContext.scansToPerform.includes(plugin.name)) {
core.info(`Running plugin: ${plugin.name}`)
Expand Down
72 changes: 69 additions & 3 deletions .github/actions/find/src/pluginManager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 './pluginNpmLoader.js'
import type {NpmPluginRequest, Plugin, PluginDefaultParams} from './types.js'

// Helper to get __dirname equivalent in ES Modules
const __filename = fileURLToPath(import.meta.url)
Expand All @@ -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
Expand All @@ -47,6 +49,16 @@ export function clearCache() {
plugins.length = 0
}

// True when the object is a usable plugin (exposes a name and default export).
function isValidPlugin(plugin: Plugin | undefined): plugin is Plugin {
return typeof plugin?.name === 'string' && typeof plugin.default === 'function'
}

// True when a plugin with the same name is already loaded.
function isDuplicatePlugin(plugin: Plugin): boolean {
return plugins.some(existing => existing.name === plugin.name)
}

// exported for mocking/testing. not for actual use
export async function loadBuiltInPlugins() {
core.info('Loading built-in plugins')
Expand All @@ -55,7 +67,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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see these "not for actual" use comments are increasing in number; are they still accurate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was an existing convention I noticed, for functions that needed to be exported since it was used in testing files, but the export wouldn't actually be used anywhere in production. Technically its accurate but it can be misleading. Do we need the label here or should I clean it up?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically its accurate but it can be misleading.

Feel free to do this in a followup pull request, but yeah, I'm not sure these comments are particularly useful here since they're not programmatically enforced + nothing crazy should happen if, for some reason, you import and use one of these functions individually. But cc @abdulahmad307 in the event I'm missing something.

If we just want to separate out exports which solely exist for testing, maybe we could move these function definitions into a separate file imported both here and in tests? 🤷‍♀️

export async function loadCustomPlugins() {
core.info('Loading custom plugins')
const pluginsPath = path.join(process.cwd(), '.github/scanner-plugins/')
Expand All @@ -75,6 +87,51 @@ 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
}

// Plugin doesn't expose a usable name/default export.
if (!isValidPlugin(plugin)) {
core.warning(`Skipping NPM plugin '${request.package}' because it does not export a valid plugin`)
continue
}
// 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 (isDuplicatePlugin(plugin)) {
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,
Expand Down Expand Up @@ -105,6 +162,15 @@ export async function loadPluginsFromPath({
continue
}

if (!isValidPlugin(plugin)) {
core.warning(`Skipping plugin '${pluginFolder}' because it does not export a valid plugin`)
continue
}
if (isDuplicatePlugin(plugin)) {
core.warning(`Skipping plugin '${pluginFolder}' because a plugin named '${plugin.name}' is already loaded`)
continue
}

core.info(`Found plugin: ${plugin.name}`)
plugins.push(plugin)
}
Expand Down
23 changes: 23 additions & 0 deletions .github/actions/find/src/pluginManager/pluginNpmLoader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import {execFileSync} from 'child_process'
Comment thread
kzhou314 marked this conversation as resolved.
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<Plugin | undefined> {
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
}
}
7 changes: 7 additions & 0 deletions .github/actions/find/src/pluginManager/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,10 @@ export type Plugin = {
name: string
default: (options: PluginDefaultParams) => Promise<void>
}

// A plugin requested from an NPM package rather than a local folder.
export type NpmPluginRequest = {
name: string
package: string
version?: string
}
32 changes: 31 additions & 1 deletion .github/actions/find/src/scansContextProvider.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,45 @@
import * as core from '@actions/core'
import type {NpmPluginRequest} from './pluginManager/types.js'

type ScansContext = {
scansToPerform: Array<string>
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 || '[]')
if (!Array.isArray(parsed)) {
throw new Error(`'scans' input must be a JSON array, got: ${scansInput}`)
}
const rawScans = parsed as ScanEntry[]

Comment thread
kzhou314 marked this conversation as resolved.
// 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
Expand All @@ -19,6 +48,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
Expand Down
11 changes: 11 additions & 0 deletions .github/actions/find/tests/findForUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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},
])
})
})

it('captures every failing element of an axe violation as nodes', async () => {
Expand Down
100 changes: 100 additions & 0 deletions .github/actions/find/tests/pluginNpmLoader.test.ts
Original file line number Diff line number Diff line change
@@ -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/pluginNpmLoader.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/pluginNpmLoader.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)
})
})
30 changes: 30 additions & 0 deletions PLUGINS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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](.github/actions/find/src/pluginManager/index.ts#L91) 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/<name>/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.
Loading