diff --git a/README.md b/README.md index 5684eef..dc2ef93 100644 --- a/README.md +++ b/README.md @@ -53,18 +53,62 @@ const rootDir = new URL('.', import.meta.url).pathname export default defineConfig({ integrations: [ shield({ - // Enables SRI hashes generation for statically generated pages - enableStatic_SRI: true, // true by default - - // Enables a middleware that generates SRI hashes for dynamically - // generated pages - enableMiddleware_SRI: false, // false by default - - // This is the path where we'll generate the module containing the SRI - // hashes for your scripts and styles. There's no need to pass this - // parameter if you don't need this data, but it can be useful to - // configure your CSP policies. - sriHashesModule: resolve(rootDir, 'src', 'utils', 'sriHashes.mjs'), + sri: { + // Enables SRI hashes generation for statically generated pages + enableStatic: true, // true by default + + // Enables a middleware that generates SRI hashes for dynamically + // generated pages + enableMiddleware: false, // false by default + + // This is the path where we'll generate the module containing the SRI + // hashes for your scripts and styles. There's no need to pass this + // parameter if you don't need this data, but it can be useful to + // configure your CSP policies. + hashesModule: resolve(rootDir, 'src', 'utils', 'sriHashes.mjs'), + + // For SSR content, Cross-Origin scripts must be explicitly allow-listed + // by URL in order to be allowed by the Content Security Policy. + // + // Defaults to [] + scriptsAllowListUrls: [ + 'https://code.jquery.com/jquery-3.7.1.slim.min.js', + ], + + // For SSR content, Cross-Origin styles must be explicitly allow-listed + // by URL in order to be allowed by the Content Security Policy. + // + // Defaults to [] + stylesAllowListUrls: [ + 'https://cdn.jsdelivr.net/npm/bootstrap@5.3.3/dist/css/bootstrap.min.css', + ], + + /** + * Inline styles are usually considered unsafe because they could make it + * easier for an attacker to inject CSS rules in dynamic pages. However, they + * don't pose a serious security risk for _most_ static pages. + * + * You can disable this option in case you want to enforce a stricter policy. + * + * @type {'all' | 'static' | false} + * + * Defaults to 'all'. + */ + allowInlineStyles: 'all', + + /** + * Inline scripts are usually considered unsafe because they could make it + * easier for an attacker to inject JS code in dynamic pages. However, they + * don't pose a serious security risk for _most_ static pages. + * + * You can disable this option in case you want to enforce a stricter policy. + * + * @type {'all' | 'static' | false} + * + * Defaults to 'all'. + */ + allowInlineScript: 'all', + }, // - If set, it controls how the security headers will be generated in the // middleware. diff --git a/package.json b/package.json index c754440..a18ae90 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@kindspells/astro-shield", - "version": "1.2.0", + "version": "1.3.0", "description": "Astro integration to enhance your website's security with SubResource Integrity hashes, Content-Security-Policy headers, and other techniques.", "private": false, "type": "module", diff --git a/src/core.mjs b/src/core.mjs index 4ad1430..66dbcc2 100644 --- a/src/core.mjs +++ b/src/core.mjs @@ -41,7 +41,7 @@ export const generateSRIHash = data => { /** * @typedef {( - * hash: string, + * hash: string | null, * attrs: string, * setCrossorigin: boolean, * content?: string | undefined, @@ -50,19 +50,19 @@ export const generateSRIHash = data => { /** @type {ElemReplacer} */ const scriptReplacer = (hash, attrs, setCrossorigin, content) => - `${content ?? ''}` /** @type {ElemReplacer} */ const styleReplacer = (hash, attrs, setCrossorigin, content) => - `${content ?? ''}` /** @type {ElemReplacer} */ const linkStyleReplacer = (hash, attrs, setCrossorigin) => - `` @@ -242,7 +242,7 @@ export const updateDynamicPageSriHashes = async ( logger, content, globalHashes, - sri + sri, ) => { const processors = getRegexProcessors() @@ -331,12 +331,23 @@ export const updateDynamicPageSriHashes = async ( if (sriHash) { pageHashes[t2].add(sriHash) } else { - const resourceResponse = await fetch(src, { method: 'GET' }) - const resourceContent = await resourceResponse.arrayBuffer() + logger.warn( + `Detected reference to not-allow-listed external resource "${src}"`, + ) + if (setCrossorigin) { + updatedContent = updatedContent.replace( + match[0], + replacer(null, attrs, true, ''), + ) + } + continue - sriHash = generateSRIHash(resourceContent) - globalHashes[t2].set(src, sriHash) - pageHashes[t2].add(sriHash) + // TODO: add scape hatch to allow fetching arbitrary external resources + // const resourceResponse = await fetch(src, { method: 'GET' }) + // const resourceContent = await resourceResponse.arrayBuffer() + // sriHash = generateSRIHash(resourceContent) + // globalHashes[t2].set(src, sriHash) + // pageHashes[t2].add(sriHash) } } else { logger.warn(`Unable to process external resource: "${src}"`) @@ -517,6 +528,30 @@ export const scanForNestedResources = async (logger, dirPath, h) => { ) } +/** + * @param {Required>} sri + * @param {HashesCollection} h + */ +export const scanAllowLists = async (sri, h) => { + for (const scriptUrl of sri.scriptsAllowListUrls) { + const resourceResponse = await fetch(scriptUrl, { method: 'GET' }) + const resourceContent = await resourceResponse.arrayBuffer() + const sriHash = generateSRIHash(resourceContent) + + h.extScriptHashes.add(sriHash) + h.perResourceSriHashes.scripts.set(scriptUrl, sriHash) + } + + for (const styleUrl of sri.stylesAllowListUrls) { + const resourceResponse = await fetch(styleUrl, { method: 'GET' }) + const resourceContent = await resourceResponse.arrayBuffer() + const sriHash = generateSRIHash(resourceContent) + + h.extStyleHashes.add(sriHash) + h.perResourceSriHashes.styles.set(styleUrl, sriHash) + } +} + /** * @param {Logger} logger * @param {HashesCollection} h @@ -673,19 +708,22 @@ export const processStaticFiles = async (logger, { distDir, sri }) => { } /** + * @param {Logger} logger * @param {MiddlewareHashes} globalHashes + * @param {Required} sri * @returns {import('astro').MiddlewareHandler} */ -export const getMiddlewareHandler = globalHashes => { +export const getMiddlewareHandler = (logger, globalHashes, sri) => { /** @satisfies {import('astro').MiddlewareHandler} */ return async (_ctx, next) => { const response = await next() const content = await response.text() const { updatedContent } = await updateDynamicPageSriHashes( - console, + logger, content, globalHashes, + sri, ) const patchedResponse = new Response(updatedContent, { @@ -700,20 +738,28 @@ export const getMiddlewareHandler = globalHashes => { /** * Variant of `getMiddlewareHandler` that also applies security headers. * + * @param {Logger} logger * @param {MiddlewareHashes} globalHashes * @param {SecurityHeadersOptions} securityHeadersOpts + * @param {Required} sri * @returns {import('astro').MiddlewareHandler} */ -export const getCSPMiddlewareHandler = (globalHashes, securityHeadersOpts) => { +export const getCSPMiddlewareHandler = ( + logger, + globalHashes, + securityHeadersOpts, + sri, +) => { /** @satisfies {import('astro').MiddlewareHandler} */ return async (_ctx, next) => { const response = await next() const content = await response.text() const { updatedContent, pageHashes } = await updateDynamicPageSriHashes( - console, + logger, content, globalHashes, + sri, ) const patchedResponse = new Response(updatedContent, { @@ -764,6 +810,7 @@ const loadVirtualMiddlewareModule = async ( // We generate a provisional hashes module. It won't contain the hashes for // resources created by Astro, but it can be useful nonetheless. await scanForNestedResources(logger, publicDir, h) + await scanAllowLists(sri, h) await generateSRIHashesModule( logger, h, @@ -821,10 +868,10 @@ export const onRequest = await (async () => { return defineMiddleware(${ securityHeadersOptions !== undefined - ? `getCSPMiddlewareHandler(globalHashes, ${JSON.stringify( + ? `getCSPMiddlewareHandler(console, globalHashes, ${JSON.stringify( securityHeadersOptions, - )})` - : 'getMiddlewareHandler(globalHashes)' + )}, ${JSON.stringify(sri)})` + : `getMiddlewareHandler(console, globalHashes, ${JSON.stringify(sri)})` }) })() ` diff --git a/src/headers.mjs b/src/headers.mjs index bddf658..8af07f6 100644 --- a/src/headers.mjs +++ b/src/headers.mjs @@ -96,9 +96,13 @@ export const patchCspHeader = (plainHeaders, pageHashes, cspOpts) => { if (pageHashes.scripts.size > 0) { setSrcDirective(directives, 'script-src', pageHashes.scripts) + } else { + directives['script-src'] = "'none'" } if (pageHashes.styles.size > 0) { setSrcDirective(directives, 'style-src', pageHashes.styles) + } else { + directives['style-src'] = "'none'" } if (Object.keys(directives).length > 0) { plainHeaders['content-security-policy'] = serialiseCspDirectives(directives) diff --git a/src/main.mjs b/src/main.mjs index e97d7e7..3d54453 100644 --- a/src/main.mjs +++ b/src/main.mjs @@ -78,12 +78,12 @@ export const shield = ({ return /** @satisfies {AstroIntegration} */ { name: '@kindspells/astro-shield', hooks: { - ...((enableStatic_SRI ?? true) === true + ...(_sri.enableStatic === true ? { 'astro:build:done': getAstroBuildDone(_sri), } : undefined), - ...(enableMiddleware_SRI === true + ...(_sri.enableMiddleware === true ? { 'astro:config:setup': getAstroConfigSetup(_sri, securityHeaders), } diff --git a/tests/core.test.mts b/tests/core.test.mts index 6bff87f..90eb33c 100644 --- a/tests/core.test.mts +++ b/tests/core.test.mts @@ -7,12 +7,15 @@ import { resolve } from 'node:path' import { readdir, rm } from 'node:fs/promises' -import { beforeEach, describe, expect, it } from 'vitest' +import { assert, beforeEach, describe, expect, it } from 'vitest' import { arraysEqual, generateSRIHash, generateSRIHashesModule, + getCSPMiddlewareHandler, + getMiddlewareHandler, pageHashesEqual, + scanAllowLists, scanForNestedResources, sriHashesEqual, updateDynamicPageSriHashes, @@ -362,7 +365,7 @@ describe('updateStaticPageSriHashes', () => { My Test Page - + ` @@ -379,7 +382,7 @@ describe('updateStaticPageSriHashes', () => { expect(h.extScriptHashes.size).toBe(1) expect( h.extScriptHashes.has( - 'sha256-vSvqa4zN5DZN/gOtz1s6Xuw0MUYNKQXvUPL8pXWgHGo=', + 'sha256-Xbdu1jxIAqCjb78wAdgir+Swc5faxBuLHPm0DC/lG80=', ), ).toBe(true) expect(h.inlineScriptHashes.size).toBe(0) @@ -656,7 +659,56 @@ describe('updateDynamicPageSriHashes', () => { expect(pageHashes.styles.size).toBe(0) }) - it('adds sri hash to external script (cross origin)', async () => { + it('avoids adding sri hash to external script when not allow-listed (cross origin)', async () => { + const remoteScript = + 'https://raw.githubusercontent.com/KindSpells/astro-shield/ae9521048f2129f633c075b7f7ef24e11bbd1884/main.mjs' + const content = ` + + My Test Page + + + + + ` + + const expected = ` + + My Test Page + + + + + ` + + const h = getMiddlewareHashes() + let warnCounter = 0 + const { pageHashes, updatedContent } = await updateDynamicPageSriHashes( + { + info: () => {}, + warn: () => { + warnCounter += 1 + }, + error: () => {}, + }, + content, + h, + ) + + expect(warnCounter).toBe(1) + expect(updatedContent).toEqual(expected) + expect(h.scripts.size).toBe(0) + expect(h.styles.size).toBe(0) + expect(h.scripts.get(remoteScript)).toBeUndefined() + expect(pageHashes.scripts.size).toBe(0) + expect( + pageHashes.scripts.has( + 'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=', + ), + ).toBe(false) + expect(pageHashes.styles.size).toBe(0) + }) + + it('adds sri hash to external script when allow-listed (cross origin)', async () => { const remoteScript = 'https://raw.githubusercontent.com/KindSpells/astro-shield/ae9521048f2129f633c075b7f7ef24e11bbd1884/main.mjs' const content = ` @@ -678,6 +730,10 @@ describe('updateDynamicPageSriHashes', () => { ` const h = getMiddlewareHashes() + h.scripts.set( + remoteScript, + 'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=', + ) const { pageHashes, updatedContent } = await updateDynamicPageSriHashes( console, content, @@ -778,16 +834,11 @@ describe('updateDynamicPageSriHashes', () => { let warnCalls = 0 const testLogger = { - info(msg: string) { - return console.info(msg) - }, - warn(msg: string) { + info(_msg: string) {}, + warn(_msg: string) { warnCalls += 1 - return console.warn(msg) - }, - error(msg: string) { - return console.error(msg) }, + error(_msg: string) {}, } const h = getMiddlewareHashes() @@ -846,6 +897,33 @@ describe('updateDynamicPageSriHashes', () => { }) }) +describe('scanAllowLists', () => { + it('populates hashes collection with hashes from allow-listed resources', async () => { + const scriptUrl = + 'https://raw.githubusercontent.com/KindSpells/astro-shield/ae9521048f2129f633c075b7f7ef24e11bbd1884/main.mjs' + const styleUrl = + 'https://raw.githubusercontent.com/KindSpells/astro-shield/26fdf5399d79baa3a8ea70ded526116b0bfc06ed/e2e/fixtures/hybrid2/src/styles/normalize.css' + + const h = getEmptyHashes() + await scanAllowLists( + { + scriptsAllowListUrls: [scriptUrl], + stylesAllowListUrls: [styleUrl], + }, + h, + ) + + expect(h.extScriptHashes.size).toBe(1) + expect(h.extStyleHashes.size).toBe(1) + expect(h.perResourceSriHashes.scripts.get(scriptUrl)).toBe( + 'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=', + ) + expect(h.perResourceSriHashes.styles.get(styleUrl)).toBe( + 'sha256-7o69ZgSUx++S5DC0Ek7X2CbY4GnxxUkwGZDdybWxSG8=', + ) + }) +}) + describe('scanForNestedResources', () => { it('populates our hashes collection with hashes from nested resources', async () => { const h = getEmptyHashes() @@ -901,3 +979,263 @@ describe('generateSRIHashesModule', () => { expect(hashesModule).toHaveProperty('perResourceSriHashes') }) }) + +describe('getMiddlewareHandler', () => { + it('returns a working middleware handler', async () => { + const hashes = { + scripts: new Map(), + styles: new Map(), + } + let warnCounter = 0 + const middleware = getMiddlewareHandler( + { + info: () => {}, + warn: () => { + warnCounter += 1 + }, + error: () => {}, + }, + hashes, + { + enableStatic: true, + enableMiddleware: true, + hashesModule: undefined, + allowInlineScripts: 'all', + allowInlineStyles: 'all', + scriptsAllowListUrls: [], + stylesAllowListUrls: [], + }, + ) + type MidParams = Parameters + + const patchedResponse = await middleware( + undefined as unknown as MidParams[0], + (async () => { + return { + text: async () => ` + + + My Test Page + + + + +`, + status: 200, + statusText: 'OK', + headers: new Headers(), + } + }) as MidParams[1], + ) + + expect(warnCounter).toBe(0) + assert(patchedResponse instanceof Response) + const responseText = await patchedResponse.text() + expect(responseText).toBe(` + + + My Test Page + + + + +`) + }) + + it('protects from validating disallowed inline scripts', async () => { + const hashes = { + scripts: new Map(), + styles: new Map(), + } + + let warnCounter = 0 + const middleware = getMiddlewareHandler( + { + info: () => {}, + warn: () => { + warnCounter += 1 + }, + error: () => {}, + }, + hashes, + { + enableStatic: true, + enableMiddleware: true, + hashesModule: undefined, + allowInlineScripts: 'static', + allowInlineStyles: 'static', + scriptsAllowListUrls: [], + stylesAllowListUrls: [], + }, + ) + type MidParams = Parameters + + const patchedResponse = await middleware( + undefined as unknown as MidParams[0], + (async () => { + return { + text: async () => ` + + + My Test Page + + + + +`, + status: 200, + statusText: 'OK', + headers: new Headers(), + } + }) as MidParams[1], + ) + + expect(warnCounter).toBe(1) + assert(patchedResponse instanceof Response) + const responseText = await patchedResponse.text() + expect(patchedResponse.headers.has('content-security-policy')).toBe(false) + expect(responseText).toBe(` + + + My Test Page + + + + +`) + }) +}) + +describe('getCSPMiddlewareHandler', () => { + it('returns a working middleware handler', async () => { + const hashes = { + scripts: new Map(), + styles: new Map(), + } + let warnCounter = 0 + const middleware = getCSPMiddlewareHandler( + { + info: () => {}, + warn: () => { + warnCounter += 1 + }, + error: () => {}, + }, + hashes, + { + contentSecurityPolicy: {}, + }, + { + enableStatic: true, + enableMiddleware: true, + hashesModule: undefined, + allowInlineScripts: 'all', + allowInlineStyles: 'all', + scriptsAllowListUrls: [], + stylesAllowListUrls: [], + }, + ) + type MidParams = Parameters + + const patchedResponse = await middleware( + undefined as unknown as MidParams[0], + (async () => { + return { + text: async () => ` + + + My Test Page + + + + +`, + status: 200, + statusText: 'OK', + headers: new Headers(), + } + }) as MidParams[1], + ) + + expect(warnCounter).toBe(0) + assert(patchedResponse instanceof Response) + expect(patchedResponse.headers.has('content-security-policy')).toBe(true) + expect(patchedResponse.headers.get('content-security-policy')).toBe( + `script-src 'self' 'sha256-TWupyvVdPa1DyFqLnQMqRpuUWdS3nKPnz70IcS/1o3Q='; style-src 'none'`, + ) + const responseText = await patchedResponse.text() + expect(responseText).toBe(` + + + My Test Page + + + + +`) + }) + + it('protects from validating disallowed inline scripts', async () => { + const hashes = { + scripts: new Map(), + styles: new Map(), + } + + let warnCounter = 0 + const middleware = getCSPMiddlewareHandler( + { + info: () => {}, + warn: () => { + warnCounter += 1 + }, + error: () => {}, + }, + hashes, + { contentSecurityPolicy: {} }, + { + enableStatic: true, + enableMiddleware: true, + hashesModule: undefined, + allowInlineScripts: 'static', + allowInlineStyles: 'static', + scriptsAllowListUrls: [], + stylesAllowListUrls: [], + }, + ) + type MidParams = Parameters + + const patchedResponse = await middleware( + undefined as unknown as MidParams[0], + (async () => { + return { + text: async () => ` + + + My Test Page + + + + +`, + status: 200, + statusText: 'OK', + headers: new Headers(), + } + }) as MidParams[1], + ) + + expect(warnCounter).toBe(1) + assert(patchedResponse instanceof Response) + const responseText = await patchedResponse.text() + expect(patchedResponse.headers.has('content-security-policy')).toBe(true) + expect(responseText).toBe(` + + + My Test Page + + + + +`) + }) +}) diff --git a/tests/headers.test.mts b/tests/headers.test.mts index 6701c8b..3fed2cf 100644 --- a/tests/headers.test.mts +++ b/tests/headers.test.mts @@ -154,7 +154,7 @@ describe('patchHeaders', () => { const patchedHeaders = patchHeaders(headers, pageHashes, settings) expect(patchedHeaders.get('content-security-policy')).toBe( - "form-action 'self'; frame-ancestors 'none'", + "form-action 'self'; frame-ancestors 'none'; script-src 'none'; style-src 'none'", ) }) diff --git a/tests/main.test.mts b/tests/main.test.mts index 2395a91..00d260a 100644 --- a/tests/main.test.mts +++ b/tests/main.test.mts @@ -36,17 +36,17 @@ describe('sriCSP', () => { }) it('returns a valid AstroIntegration object for almost-default config', () => { - const integration = shield({ enableStatic_SRI: true }) + const integration = shield({ sri: { enableStatic: true } }) checkIntegration(integration) }) it('returns an "empty" integration when we disable all features', () => { - const integration = shield({ enableStatic_SRI: false }) + const integration = shield({ sri: { enableStatic: false } }) checkIntegration(integration, []) }) it('returns hooks for static & dynamic content when we enable middleware', () => { - const integration = shield({ enableMiddleware_SRI: true }) + const integration = shield({ sri: { enableMiddleware: true } }) checkIntegration(integration, ['astro:build:done', 'astro:config:setup']) }) diff --git a/vitest.config.unit.mts b/vitest.config.unit.mts index 057ed7f..3cdbb6d 100644 --- a/vitest.config.unit.mts +++ b/vitest.config.unit.mts @@ -19,10 +19,10 @@ export default defineConfig({ 'coverage-unit/**/*', ], thresholds: { - statements: 72.0, - branches: 76.0, - functions: 80.0, - lines: 72.0, + statements: 77.0, + branches: 77.0, + functions: 87.0, + lines: 77.0, }, reportsDirectory: 'coverage-unit', },