diff --git a/.github/actions/find/README.md b/.github/actions/find/README.md index f674e23..4ee79e2 100644 --- a/.github/actions/find/README.md +++ b/.github/actions/find/README.md @@ -19,6 +19,14 @@ https://primer.style/octicons/ **Optional** Stringified JSON object containing `username`, `password`, `cookies`, and/or `localStorage` from an authenticated session. For example: `{"username":"some-user","password":"correct-horse-battery-staple","cookies":[{"name":"theme-preference","value":"light","domain":"primer.style","path":"/"}],"localStorage":{"https://primer.style":{"theme-preference":"light"}}}` +#### `include_screenshots` + +**Optional** Bool - whether to capture screenshots of scanned pages and include links to them in the issue + +#### `scans` + +**Optional** Stringified JSON array of scans (string) to perform. If not provided, only axe will be performed. Valid options currently include 'axe' + ### Outputs #### `findings` diff --git a/.github/actions/find/action.yml b/.github/actions/find/action.yml index 2ab8dcb..c48b7bd 100644 --- a/.github/actions/find/action.yml +++ b/.github/actions/find/action.yml @@ -1,27 +1,30 @@ -name: "Find" -description: "Finds potential accessibility gaps." +name: 'Find' +description: 'Finds potential accessibility gaps.' inputs: urls: - description: "Newline-delimited list of URLs to check for accessibility issues" + description: 'Newline-delimited list of URLs to check for accessibility issues' required: true multiline: true auth_context: description: "Stringified JSON object containing 'username', 'password', 'cookies', and/or 'localStorage' from an authenticated session" required: false include_screenshots: - description: "Whether to capture screenshots of scanned pages and include links to them in the issue" + description: 'Whether to capture screenshots of scanned pages and include links to them in the issue' + required: false + default: 'false' + scans: + description: 'Stringified JSON array of scans to perform. If not provided, only axe will be performed' required: false - default: "false" outputs: findings: - description: "List of potential accessibility gaps, as stringified JSON" + description: 'List of potential accessibility gaps, as stringified JSON' runs: - using: "node24" - main: "bootstrap.js" + using: 'node24' + main: 'bootstrap.js' branding: - icon: "compass" - color: "blue" + icon: 'compass' + color: 'blue' diff --git a/.github/actions/find/src/dynamicImport.ts b/.github/actions/find/src/dynamicImport.ts new file mode 100644 index 0000000..f00d224 --- /dev/null +++ b/.github/actions/find/src/dynamicImport.ts @@ -0,0 +1,21 @@ +// - this exists because it looks like there's no straight-forward +// way to mock the dynamic import function, so mocking this instead +// (also, if it _is_ possible to mock the dynamic import, +// there's the risk of altering/breaking the behavior of imports +// across the board - including non-dynamic imports) +// +// - also, vitest has a limitation on mocking: +// https://vitest.dev/guide/mocking/modules.html#mocking-modules-pitfalls +// +// - basically if a function is called by another function in the same file +// it can't be mocked. So this was extracted into a separate file +// +// - one thing to note is vitest does the same thing here: +// https://github.com/vitest-dev/vitest/blob/main/test/core/src/dynamic-import.ts +// - and uses that with tests here: +// https://github.com/vitest-dev/vitest/blob/main/test/core/test/mock-internals.test.ts#L27 +// +// - so this looks like a reasonable approach +export async function dynamicImport(path: string) { + return import(path) +} diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index 185f58d..2730b46 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -3,6 +3,9 @@ import AxeBuilder from '@axe-core/playwright' import playwright from 'playwright' import {AuthContext} from './AuthContext.js' import {generateScreenshots} from './generateScreenshots.js' +import {loadPlugins} from './pluginManager.js' +import {getScansContext} from './scansContextProvider.js' +import axe from 'axe-core' export async function findForUrl( url: string, @@ -19,16 +22,45 @@ export async function findForUrl( await page.goto(url) console.log(`Scanning ${page.url()}`) - let findings: Finding[] = [] + const findings: Finding[] = [] + const addFinding = (findingData: Finding) => { + findings.push(findingData) + } + try { - const rawFindings = await new AxeBuilder({page}).analyze() + const scansContext = getScansContext() + + let rawFindings: axe.AxeResults | undefined + if (scansContext.shouldPerformAxeScan) { + rawFindings = await new AxeBuilder({page}).analyze() + } + + // - this if condition is not required, but makes it easier to make assertions + // in unit tests on whether 'loadPlugins' was called or not (it does have the added + // benefit of completely skipping plugin loading if we just want axe - so thats + // a minor performance boost) + // - alternatively, we can wrap the 'plugin.default(...)' call in another function + // and make assertions on whether that function was called or not + // - the other option is to wrap each plugin in a class instance + // and make assertions on something like 'plugin.run' being called or not + if (scansContext.shouldRunPlugins) { + const plugins = await loadPlugins() + for (const plugin of plugins) { + if (scansContext.scansToPerform.includes(plugin.name)) { + console.log('Running plugin: ', plugin.name) + await plugin.default({page, addFinding, url}) + } else { + console.log(`Skipping plugin ${plugin.name} because it is not included in the 'scans' input`) + } + } + } let screenshotId: string | undefined if (includeScreenshots) { screenshotId = await generateScreenshots(page) } - findings = rawFindings.violations.map(violation => ({ + const axeFindings = rawFindings?.violations.map(violation => ({ scannerType: 'axe', url, html: violation.nodes[0].html.replace(/'/g, '''), @@ -39,6 +71,7 @@ export async function findForUrl( solutionLong: violation.nodes[0].failureSummary?.replace(/'/g, '''), screenshotId, })) + findings.push(...(axeFindings || [])) } catch (e) { console.error('Error during accessibility scan:', e) } diff --git a/.github/actions/find/src/pluginManager.ts b/.github/actions/find/src/pluginManager.ts new file mode 100644 index 0000000..46b2889 --- /dev/null +++ b/.github/actions/find/src/pluginManager.ts @@ -0,0 +1,88 @@ +import * as fs from 'fs' +import * as path from 'path' +import {fileURLToPath} from 'url' +import {dynamicImport} from './dynamicImport.js' +import type {Finding} from './types.d.js' +import playwright from 'playwright' + +// Helper to get __dirname equivalent in ES Modules +const __filename = fileURLToPath(import.meta.url) +const __dirname = path.dirname(__filename) + +export type Plugin = { + name: string + default: (options: {page: playwright.Page; addFinding: (findingData: Finding) => void; url: string}) => Promise +} + +const plugins: Plugin[] = [] +let pluginsLoaded = false + +export async function loadPlugins() { + console.log('loading plugins') + + try { + if (!pluginsLoaded) { + await loadBuiltInPlugins() + await loadCustomPlugins() + } + } catch { + plugins.length = 0 + console.log(abortError) + } finally { + pluginsLoaded = true + return plugins + } +} + +export const abortError = ` +There was an error while loading plugins. +Clearing all plugins and aborting custom plugin scans. +Please check the logs for hints as to what may have gone wrong. +` + +export function clearCache() { + pluginsLoaded = false + plugins.length = 0 +} + +// exported for mocking/testing. not for actual use +export async function loadBuiltInPlugins() { + console.log('Loading built-in plugins') + + const pluginsPath = '../../../scanner-plugins/' + await loadPluginsFromPath({ + readPath: path.join(__dirname, pluginsPath), + importPath: pluginsPath, + }) +} + +// exported for mocking/testing. not for actual use +export async function loadCustomPlugins() { + console.log('Loading custom plugins') + + const pluginsPath = process.cwd() + '/.github/scanner-plugins/' + await loadPluginsFromPath({ + readPath: pluginsPath, + importPath: pluginsPath, + }) +} + +// exported for mocking/testing. not for actual use +export async function loadPluginsFromPath({readPath, importPath}: {readPath: string; importPath: string}) { + try { + const res = fs.readdirSync(readPath) + for (const pluginFolder of res) { + const pluginFolderPath = path.join(importPath, pluginFolder) + if (fs.lstatSync(pluginFolderPath).isDirectory()) { + console.log('Found plugin: ', pluginFolder) + plugins.push(await dynamicImport(path.join(importPath, pluginFolder, '/index.js'))) + } + } + } catch (e) { + // - log errors here for granular info + console.log('error: ') + console.log(e) + // - throw error to handle aborting the plugin scans + throw e + } +} diff --git a/.github/actions/find/src/scansContextProvider.ts b/.github/actions/find/src/scansContextProvider.ts new file mode 100644 index 0000000..2d6a2ef --- /dev/null +++ b/.github/actions/find/src/scansContextProvider.ts @@ -0,0 +1,36 @@ +import core from '@actions/core' + +type ScansContext = { + scansToPerform: Array + shouldPerformAxeScan: boolean + shouldRunPlugins: boolean +} +let scansContext: ScansContext | undefined + +export function getScansContext() { + if (!scansContext) { + const scansJson = core.getInput('scans', {required: false}) + const scansToPerform = JSON.parse(scansJson || '[]') + // - 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 + // - keep in mind, 'onlyAxeScan' is not the same as 'shouldPerformAxeScan' + const onlyAxeScan = scansToPerform.length === 0 || (scansToPerform.length === 1 && scansToPerform[0] === 'axe') + + scansContext = { + scansToPerform, + // - 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 + // mark it as required + shouldPerformAxeScan: !scansJson || scansToPerform.includes('axe'), + shouldRunPlugins: scansToPerform.length > 0 && !onlyAxeScan, + } + } + + return scansContext +} + +export function clearCache() { + scansContext = undefined +} diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts new file mode 100644 index 0000000..25f4d1f --- /dev/null +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -0,0 +1,104 @@ +import {describe, it, expect, vi} from 'vitest' +import core from '@actions/core' +import {findForUrl} from '../src/findForUrl.js' +import AxeBuilder from '@axe-core/playwright' +import axe from 'axe-core' +import * as pluginManager from '../src/pluginManager.js' +import {clearCache} from '../src/scansContextProvider.js' + +vi.mock('playwright', () => ({ + default: { + chromium: { + launch: () => ({ + newContext: () => ({ + newPage: () => ({ + pageUrl: '', + goto: () => {}, + url: () => {}, + }), + close: () => {}, + }), + close: () => {}, + }), + }, + }, +})) + +vi.mock('@axe-core/playwright', () => { + const AxeBuilderMock = vi.fn() + const rawFinding = {violations: []} as unknown as axe.AxeResults + AxeBuilderMock.prototype.analyze = vi.fn(() => Promise.resolve(rawFinding)) + return {default: AxeBuilderMock} +}) + +let actionInput: string = '' +let loadedPlugins: pluginManager.Plugin[] = [] + +function clearAll() { + clearCache() + vi.clearAllMocks() +} + +describe('findForUrl', () => { + vi.spyOn(core, 'getInput').mockImplementation(() => actionInput) + vi.spyOn(pluginManager, 'loadPlugins').mockImplementation(() => Promise.resolve(loadedPlugins)) + + async function axeOnlyTest() { + clearAll() + + await findForUrl('test.com') + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(0) + } + + describe('when no scans list is provided', () => { + it('defaults to running only axe scan', async () => { + actionInput = '' + await axeOnlyTest() + }) + }) + + describe('when a scans list is provided', () => { + describe('and the list _only_ includes axe', () => { + it('runs only the axe scan', async () => { + actionInput = JSON.stringify(['axe']) + await axeOnlyTest() + }) + }) + + describe('and the list includes axe and other scans', () => { + it('runs axe and plugins', async () => { + actionInput = JSON.stringify(['axe', 'custom-scan']) + clearAll() + + await findForUrl('test.com') + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1) + }) + }) + + describe('and the list does not include axe', () => { + it('only runs plugins', async () => { + actionInput = JSON.stringify(['custom-scan']) + clearAll() + + await findForUrl('test.com') + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0) + expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1) + }) + }) + + it('should only run scans that are included in the list', async () => { + loadedPlugins = [ + {name: 'custom-scan-1', default: vi.fn()}, + {name: 'custom-scan-2', default: vi.fn()}, + ] + actionInput = JSON.stringify(['custom-scan-1']) + clearAll() + + await findForUrl('test.com') + expect(loadedPlugins[0].default).toHaveBeenCalledTimes(1) + expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0) + }) + }) +}) diff --git a/.github/actions/find/tests/pluginManager.test.ts b/.github/actions/find/tests/pluginManager.test.ts new file mode 100644 index 0000000..8e033d7 --- /dev/null +++ b/.github/actions/find/tests/pluginManager.test.ts @@ -0,0 +1,61 @@ +import {describe, it, expect, vi, beforeEach} from 'vitest' + +import * as fs from 'fs' +import * as dynamicImportModule from '../src/dynamicImport.js' +import * as pluginManager from '../src/pluginManager.js' + +// - enable spying on fs +// https://vitest.dev/guide/browser/#limitations +vi.mock('fs', {spy: true}) +vi.mock('../src/pluginManager.js', {spy: true}) + +describe('loadPlugins', () => { + vi.spyOn(dynamicImportModule, 'dynamicImport').mockImplementation(path => Promise.resolve(path)) + beforeEach(() => { + // @ts-expect-error - we don't need the full fs readdirsync + // method signature here + vi.spyOn(fs, 'readdirSync').mockImplementation(readPath => { + return [readPath + '/plugin-1', readPath + '/plugin-2'] + }) + vi.spyOn(fs, 'lstatSync').mockImplementation(() => { + return { + isDirectory: () => true, + } as unknown as fs.Stats + }) + }) + + describe('when plugins are not loaded', () => { + it('loads them', async () => { + pluginManager.clearCache() + const plugins = await pluginManager.loadPlugins() + expect(dynamicImportModule.dynamicImport).toHaveBeenCalledTimes(4) + expect(plugins.length).toBe(4) + }) + }) + + describe('when plugins are already loaded', () => { + it('caches them and doesnt load them again', async () => { + pluginManager.clearCache() + await pluginManager.loadPlugins() + await pluginManager.loadPlugins() + expect(pluginManager.loadBuiltInPlugins).toHaveBeenCalledTimes(0) + expect(pluginManager.loadCustomPlugins).toHaveBeenCalledTimes(0) + }) + }) + + describe('when there is an error loading plugins', () => { + beforeEach(() => { + vi.spyOn(fs, 'readdirSync').mockImplementation(() => { + throw new Error('test error') + }) + }) + + it('Aborts loading all plugins', async () => { + pluginManager.clearCache() + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const plugins = await pluginManager.loadPlugins() + expect(plugins.length).toBe(0) + expect(consoleLogSpy).toHaveBeenCalledWith(pluginManager.abortError) + }) + }) +}) diff --git a/.github/scanner-plugins/reflow-test/index.js b/.github/scanner-plugins/reflow-test/index.js new file mode 100644 index 0000000..0b1e887 --- /dev/null +++ b/.github/scanner-plugins/reflow-test/index.js @@ -0,0 +1,32 @@ +export default async function test({ page, addFinding, url } = {}) { + console.log('reflow test built-in'); + // Check for horizontal scrolling at 320x256 viewport + try { + await page.setViewportSize({ width: 320, height: 256 }); + const scrollWidth = await page.evaluate(() => document.documentElement.scrollWidth); + const clientWidth = await page.evaluate(() => document.documentElement.clientWidth); + + // If horizontal scroll is required (with 1px tolerance for rounding) + if (scrollWidth > clientWidth + 1) { + const htmlSnippet = await page.evaluate(() => { + return ``; + }); + + addFinding({ + scannerType: 'viewport', + ruleId: 'horizontal-scroll-320x256', + url, + html: htmlSnippet.replace(/'/g, "'"), + problemShort: 'page requires horizontal scrolling at 320x256 viewport', + problemUrl: 'https://www.w3.org/WAI/WCAG21/Understanding/reflow.html', + solutionShort: 'ensure content is responsive and does not require horizontal scrolling at small viewport sizes', + solutionLong: `The page has a scroll width of ${scrollWidth}px but a client width of only ${clientWidth}px at 320x256 viewport, requiring horizontal scrolling. This violates WCAG 2.1 Level AA Success Criterion 1.4.10 (Reflow).` + }); + } + } catch (e) { + console.error('Error checking horizontal scroll:', e); + } + +} + +export const name = 'reflow-test'; diff --git a/.github/scanner-plugins/reflow-test/package.json b/.github/scanner-plugins/reflow-test/package.json new file mode 100644 index 0000000..5bc2b0c --- /dev/null +++ b/.github/scanner-plugins/reflow-test/package.json @@ -0,0 +1,6 @@ +{ + "name": "reflow-plugin-test", + "version": "1.0.0", + "description": "A test plugin for reflow testing", + "type": "module" +} diff --git a/.gitignore b/.gitignore index f42521c..cbb4dba 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ dist node_modules -test-results \ No newline at end of file +test-results +.vscode diff --git a/action.yml b/action.yml index 932bc27..0992f3b 100644 --- a/action.yml +++ b/action.yml @@ -54,6 +54,11 @@ runs: if [ "$(realpath ".github/actions")" != "$(realpath "${ACTION_DIR}/.github/actions")" ]; then cp -a ".github/actions/." "${ACTION_DIR}/.github/actions/" fi + + mkdir -p "${ACTION_DIR}/.github/scanner-plugins" + if [ "$(realpath ".github/scanner-plugins")" != "$(realpath "${ACTION_DIR}/.github/scanner-plugins")" ]; then + cp -a ".github/scanner-plugins/." "${ACTION_DIR}/.github/scanner-plugins/." + fi - name: Restore cached results id: restore uses: ./../../_actions/github/accessibility-scanner/current/.github/actions/gh-cache/cache