Skip to content

Commit db51bb5

Browse files
Add plugins (#133)
## New Context After validating the plugins idea, we have decided to move forward with this approach. This PR introduces the idea of 'Plugins' to the scanner. A plugin (as described in the [PLUGINS doc](https://github.com/github/accessibility-scanner/pull/133/changes#diff-8286e7365a5a84367dd51372b6b4565edb1146925ff6f5fc126a9d7d39fc94a9)) is a custom script that performs an accessibility test/scan. It allows teams to specify their own tests in their own repos to surface accessibility issues. These plugins can be used in combination with existing scans. For example, a team can perform interactions on a page before the axe scan runs. There will also be built-in scans that will ship with the scanner. ## Original Context This is a very minimal proof of concept around the idea of using plugins to make additional scans and tests modular. Looking for feedback on general approach, pros/cons, etc... this currently works with the plugins defined in the scanner repo. I'm still working on making this work with plugins defined in the workflow repo (the repo that uses the action). There might be some issues with using absolute paths with the dynamic import() feature - but i'm still testing. I've taken @lindseywild’s reflow test and added it as a plugin for testing. the idea is each plugin lives under a new folder under the scanner-plugins folder (which lives under .github like actions). and each plugin has a primary index.js file, which we will use to interface with the plugin. all of this is just how I've built it as a proposal - we can tweak naming/folder-structure, etc... if we feel something else makes more sense
2 parents 0fcd726 + 1b7fc8d commit db51bb5

File tree

17 files changed

+553
-69
lines changed

17 files changed

+553
-69
lines changed

.github/actions/file/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export default async function () {
100100
)
101101
}
102102
} catch (error) {
103-
core.setFailed(`Failed on filing: ${filing}\n${error}`)
103+
core.setFailed(`Failed on filing: ${JSON.stringify(filing, null, 2)}\n${error}`)
104104
process.exit(1)
105105
}
106106
}

.github/actions/find/README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ configuration option.
3131
[`colorScheme`](https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme)
3232
configuration option.
3333

34+
#### `include_screenshots`
35+
36+
**Optional** Bool - whether to capture screenshots of scanned pages and include links to them in the issue
37+
38+
#### `scans`
39+
40+
**Optional** Stringified JSON array of scans (string) to perform. If not provided, only Axe will be performed.
41+
3442
### Outputs
3543

3644
#### `findings`

.github/actions/find/action.yml

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,36 @@
1-
name: "Find"
2-
description: "Finds potential accessibility gaps."
1+
name: 'Find'
2+
description: 'Finds potential accessibility gaps.'
33

44
inputs:
55
urls:
6-
description: "Newline-delimited list of URLs to check for accessibility issues"
6+
description: 'Newline-delimited list of URLs to check for accessibility issues'
77
required: true
88
multiline: true
99
auth_context:
1010
description: "Stringified JSON object containing 'username', 'password', 'cookies', and/or 'localStorage' from an authenticated session"
1111
required: false
1212
include_screenshots:
13-
description: "Whether to capture screenshots of scanned pages and include links to them in the issue"
13+
description: 'Whether to capture screenshots of scanned pages and include links to them in the issue'
14+
required: false
15+
default: 'false'
16+
scans:
17+
description: 'Stringified JSON array of scans to perform. If not provided, only Axe will be performed'
1418
required: false
15-
default: "false"
1619
reduced_motion:
17-
description: "Playwright reducedMotion setting: https://playwright.dev/docs/api/class-browser#browser-new-page-option-reduced-motion"
20+
description: 'Playwright reducedMotion setting: https://playwright.dev/docs/api/class-browser#browser-new-page-option-reduced-motion'
1821
required: false
1922
color_scheme:
20-
description: "Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme"
23+
description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme'
2124
required: false
2225

2326
outputs:
2427
findings:
25-
description: "List of potential accessibility gaps, as stringified JSON"
28+
description: 'List of potential accessibility gaps, as stringified JSON'
2629

2730
runs:
28-
using: "node24"
29-
main: "bootstrap.js"
31+
using: 'node24'
32+
main: 'bootstrap.js'
3033

3134
branding:
32-
icon: "compass"
33-
color: "blue"
35+
icon: 'compass'
36+
color: 'blue'
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// - this exists because it looks like there's no straight-forward
2+
// way to mock the dynamic import function, so mocking this instead
3+
// (also, if it _is_ possible to mock the dynamic import,
4+
// there's the risk of altering/breaking the behavior of imports
5+
// across the board - including non-dynamic imports)
6+
//
7+
// - also, vitest has a limitation on mocking:
8+
// https://vitest.dev/guide/mocking/modules.html#mocking-modules-pitfalls
9+
//
10+
// - basically if a function is called by another function in the same file
11+
// it can't be mocked. So this was extracted into a separate file
12+
//
13+
// - one thing to note is vitest does the same thing here:
14+
// https://github.com/vitest-dev/vitest/blob/main/test/core/src/dynamic-import.ts
15+
// - and uses that with tests here:
16+
// https://github.com/vitest-dev/vitest/blob/main/test/core/test/mock-internals.test.ts#L27
17+
//
18+
// - so this looks like a reasonable approach
19+
export function dynamicImport(path: string) {
20+
return import(path)
21+
}

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

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import {AxeBuilder} from '@axe-core/playwright'
33
import playwright from 'playwright'
44
import {AuthContext} from './AuthContext.js'
55
import {generateScreenshots} from './generateScreenshots.js'
6+
import {loadPlugins, invokePlugin} from './pluginManager.js'
7+
import {getScansContext} from './scansContextProvider.js'
8+
import * as core from '@actions/core'
69

710
export async function findForUrl(
811
url: string,
@@ -23,32 +26,69 @@ export async function findForUrl(
2326
const context = await browser.newContext(contextOptions)
2427
const page = await context.newPage()
2528
await page.goto(url)
26-
console.log(`Scanning ${page.url()}`)
2729

28-
let findings: Finding[] = []
29-
try {
30-
const rawFindings = await new AxeBuilder({page}).analyze()
31-
32-
let screenshotId: string | undefined
30+
const findings: Finding[] = []
31+
const addFinding = async (findingData: Finding) => {
32+
let screenshotId
3333
if (includeScreenshots) {
3434
screenshotId = await generateScreenshots(page)
3535
}
36+
findings.push({...findingData, screenshotId})
37+
}
38+
39+
try {
40+
const scansContext = getScansContext()
41+
42+
if (scansContext.shouldRunPlugins) {
43+
const plugins = await loadPlugins()
44+
for (const plugin of plugins) {
45+
if (scansContext.scansToPerform.includes(plugin.name)) {
46+
core.info(`Running plugin: ${plugin.name}`)
47+
await invokePlugin({
48+
plugin,
49+
page,
50+
addFinding,
51+
})
52+
} else {
53+
core.info(`Skipping plugin ${plugin.name} because it is not included in the 'scans' input`)
54+
}
55+
}
56+
}
3657

37-
findings = rawFindings.violations.map(violation => ({
38-
scannerType: 'axe',
39-
url,
40-
html: violation.nodes[0].html.replace(/'/g, '''),
41-
problemShort: violation.help.toLowerCase().replace(/'/g, '''),
42-
problemUrl: violation.helpUrl.replace(/'/g, '''),
43-
ruleId: violation.id,
44-
solutionShort: violation.description.toLowerCase().replace(/'/g, '''),
45-
solutionLong: violation.nodes[0].failureSummary?.replace(/'/g, '''),
46-
screenshotId,
47-
}))
58+
if (scansContext.shouldPerformAxeScan) {
59+
await runAxeScan({page, addFinding})
60+
}
4861
} catch (e) {
49-
console.error('Error during accessibility scan:', e)
62+
core.error(`Error during accessibility scan: ${e}`)
5063
}
5164
await context.close()
5265
await browser.close()
5366
return findings
5467
}
68+
69+
async function runAxeScan({
70+
page,
71+
addFinding,
72+
}: {
73+
page: playwright.Page
74+
addFinding: (findingData: Finding, options?: {includeScreenshots?: boolean}) => Promise<void>
75+
}) {
76+
const url = page.url()
77+
core.info(`Scanning ${url}`)
78+
const rawFindings = await new AxeBuilder({page}).analyze()
79+
80+
if (rawFindings) {
81+
for (const violation of rawFindings.violations) {
82+
await addFinding({
83+
scannerType: 'axe',
84+
url,
85+
html: violation.nodes[0].html.replace(/'/g, '&apos;'),
86+
problemShort: violation.help.toLowerCase().replace(/'/g, '&apos;'),
87+
problemUrl: violation.helpUrl.replace(/'/g, '&apos;'),
88+
ruleId: violation.id,
89+
solutionShort: violation.description.toLowerCase().replace(/'/g, '&apos;'),
90+
solutionLong: violation.nodes[0].failureSummary?.replace(/'/g, '&apos;'),
91+
})
92+
}
93+
}
94+
}

.github/actions/find/src/generateScreenshots.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from 'node:fs'
22
import path from 'node:path'
33
import crypto from 'node:crypto'
44
import type {Page} from 'playwright'
5+
import * as core from '@actions/core'
56

67
// Use GITHUB_WORKSPACE to ensure screenshots are saved in the workflow workspace root
78
// where the artifact upload step can find them
@@ -12,9 +13,9 @@ export const generateScreenshots = async function (page: Page) {
1213
// Ensure screenshot directory exists
1314
if (!fs.existsSync(SCREENSHOT_DIR)) {
1415
fs.mkdirSync(SCREENSHOT_DIR, {recursive: true})
15-
console.log(`Created screenshot directory: ${SCREENSHOT_DIR}`)
16+
core.info(`Created screenshot directory: ${SCREENSHOT_DIR}`)
1617
} else {
17-
console.log(`Using existing screenshot directory ${SCREENSHOT_DIR}`)
18+
core.info(`Using existing screenshot directory ${SCREENSHOT_DIR}`)
1819
}
1920

2021
try {
@@ -28,9 +29,9 @@ export const generateScreenshots = async function (page: Page) {
2829
const filepath = path.join(SCREENSHOT_DIR, filename)
2930

3031
fs.writeFileSync(filepath, screenshotBuffer)
31-
console.log(`Screenshot saved: ${filename}`)
32+
core.info(`Screenshot saved: ${filename}`)
3233
} catch (error) {
33-
console.error('Failed to capture/save screenshot:', error)
34+
core.error(`Failed to capture/save screenshot: ${error}`)
3435
screenshotId = undefined
3536
}
3637

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import * as fs from 'fs'
2+
import * as path from 'path'
3+
import {fileURLToPath} from 'url'
4+
import {dynamicImport} from './dynamicImport.js'
5+
import type {Finding} from './types.d.js'
6+
import playwright from 'playwright'
7+
import * as core from '@actions/core'
8+
9+
// Helper to get __dirname equivalent in ES Modules
10+
const __filename = fileURLToPath(import.meta.url)
11+
const __dirname = path.dirname(__filename)
12+
13+
type PluginDefaultParams = {
14+
page: playwright.Page
15+
addFinding: (findingData: Finding) => void
16+
}
17+
18+
type Plugin = {
19+
name: string
20+
default: (options: PluginDefaultParams) => Promise<void>
21+
}
22+
23+
const plugins: Plugin[] = []
24+
let pluginsLoaded = false
25+
26+
export async function loadPlugins() {
27+
try {
28+
if (!pluginsLoaded) {
29+
core.info('loading plugins')
30+
await loadBuiltInPlugins()
31+
await loadCustomPlugins()
32+
}
33+
} catch {
34+
plugins.length = 0
35+
core.error(abortError)
36+
} finally {
37+
pluginsLoaded = true
38+
return plugins
39+
}
40+
}
41+
42+
export const abortError = `
43+
There was an error while loading plugins.
44+
Clearing all plugins and aborting custom plugin scans.
45+
Please check the logs for hints as to what may have gone wrong.
46+
`
47+
48+
export function clearCache() {
49+
pluginsLoaded = false
50+
plugins.length = 0
51+
}
52+
53+
// exported for mocking/testing. not for actual use
54+
export async function loadBuiltInPlugins() {
55+
core.info('Loading built-in plugins')
56+
57+
const pluginsPath = path.join(__dirname, '../../../scanner-plugins/')
58+
await loadPluginsFromPath({pluginsPath})
59+
}
60+
61+
// exported for mocking/testing. not for actual use
62+
export async function loadCustomPlugins() {
63+
core.info('Loading custom plugins')
64+
65+
const pluginsPath = path.join(process.cwd(), '/.github/scanner-plugins/')
66+
await loadPluginsFromPath({pluginsPath})
67+
}
68+
69+
// exported for mocking/testing. not for actual use
70+
export async function loadPluginsFromPath({pluginsPath}: {pluginsPath: string}) {
71+
try {
72+
const res = fs.readdirSync(pluginsPath)
73+
for (const pluginFolder of res) {
74+
const pluginFolderPath = path.join(pluginsPath, pluginFolder)
75+
76+
if (fs.existsSync(pluginFolderPath) && fs.lstatSync(pluginFolderPath).isDirectory()) {
77+
core.info(`Found plugin: ${pluginFolder}`)
78+
plugins.push(await dynamicImport(path.join(pluginsPath, pluginFolder, '/index.js')))
79+
}
80+
}
81+
} catch (e) {
82+
// - log errors here for granular info
83+
core.error('error: ')
84+
core.error(e as Error)
85+
// - throw error to handle aborting the plugin scans
86+
throw e
87+
}
88+
}
89+
90+
type InvokePluginParams = PluginDefaultParams & {
91+
plugin: Plugin
92+
}
93+
export function invokePlugin({plugin, page, addFinding}: InvokePluginParams) {
94+
return plugin.default({page, addFinding})
95+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as core from '@actions/core'
2+
3+
type ScansContext = {
4+
scansToPerform: Array<string>
5+
shouldPerformAxeScan: boolean
6+
shouldRunPlugins: boolean
7+
}
8+
let scansContext: ScansContext | undefined
9+
10+
export function getScansContext() {
11+
if (!scansContext) {
12+
const scansInput = core.getInput('scans', {required: false})
13+
const scansToPerform = JSON.parse(scansInput || '[]')
14+
// - if we don't have a scans input
15+
// or we do have a scans input, but it only has 1 item and its 'axe'
16+
// then we only want to run 'axe' and not the plugins
17+
// - keep in mind, 'onlyAxeScan' is not the same as 'shouldPerformAxeScan'
18+
const onlyAxeScan = scansToPerform.length === 0 || (scansToPerform.length === 1 && scansToPerform[0] === 'axe')
19+
20+
scansContext = {
21+
scansToPerform,
22+
// - if no 'scans' input is provided, we default to the existing behavior
23+
// (only axe scan) for backwards compatability.
24+
// - we can enforce using the 'scans' input in a future major release and
25+
// mark it as required
26+
shouldPerformAxeScan: !scansInput || scansToPerform.includes('axe'),
27+
shouldRunPlugins: scansToPerform.length > 0 && !onlyAxeScan,
28+
}
29+
}
30+
31+
return scansContext
32+
}
33+
34+
export function clearCache() {
35+
scansContext = undefined
36+
}

.github/actions/find/src/types.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
export type Finding = {
2+
scannerType: string
23
url: string
34
html: string
45
problemShort: string
56
problemUrl: string
67
solutionShort: string
78
solutionLong?: string
89
screenshotId?: string
10+
ruleId?: string
911
}
1012

1113
export type Cookie = {

0 commit comments

Comments
 (0)