From 1221019306f501bf5fa9bcfb5a23a2321d34ba0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Correa=20Casablanca?= Date: Sun, 31 Mar 2024 03:37:15 +0200 Subject: [PATCH] fix: ensure that allowed scripts are in hashes module Signed-off-by: Andres Correa Casablanca --- .editorconfig | 4 +- @kindspells/astro-shield/e2e/e2e.test.mts | 21 ++++ .../e2e/fixtures/hybrid3/astro.config.mjs | 12 ++- .../fixtures/hybrid3/src/pages/injected.astro | 32 ++++++ @kindspells/astro-shield/src/core.mjs | 102 +++++++++++------- @kindspells/astro-shield/tests/core.test.mts | 4 +- 6 files changed, 132 insertions(+), 43 deletions(-) create mode 100644 @kindspells/astro-shield/e2e/fixtures/hybrid3/src/pages/injected.astro diff --git a/.editorconfig b/.editorconfig index 6b95311..f6ce70f 100644 --- a/.editorconfig +++ b/.editorconfig @@ -14,6 +14,8 @@ indent_style = tab indent_size = 2 trim_trailing_whitespace = true -[*.md] +[*.{md,mdx}] +charset = utf-8 indent_style = space indent_size = 2 +trim_trailing_whitespace = true diff --git a/@kindspells/astro-shield/e2e/e2e.test.mts b/@kindspells/astro-shield/e2e/e2e.test.mts index 14b7cf8..8c9e138 100644 --- a/@kindspells/astro-shield/e2e/e2e.test.mts +++ b/@kindspells/astro-shield/e2e/e2e.test.mts @@ -23,6 +23,7 @@ import { it, } from 'vitest' +import type { HashesModule } from '#as/core.mjs' import { generateSRIHash } from '#as/core.mjs' import { doesFileExist } from '#as/fs.mjs' @@ -530,4 +531,24 @@ describe('middleware (hybrid 3)', () => { "default-src 'none'; frame-ancestors 'none'; script-src 'self' 'sha256-X7QGGDHgf6XMoabXvV9pW7gl3ALyZhZlgKq1s3pwmME='; style-src 'self' 'sha256-9U7mv8FibD/D9IbGpXc86pz37l6/w4PCLpFIZuPrzh8=' 'sha256-ZlgyI5Bx/aeAyk/wSIypqeIM5PBhz9IiAek9HIiAjaI='", ) }) + + it('incorporates the allowed scripts into the generated hashes module', async () => { + const hashesModulePath = resolve(hybridDir, 'src', 'generated', 'sri.mjs') + assert(await doesFileExist(hashesModulePath)) + + const hashesModule = (await import(hashesModulePath)) as HashesModule + + assert( + Object.hasOwn( + hashesModule.perResourceSriHashes.scripts, + 'https://code.jquery.com/jquery-3.7.1.slim.min.js', + ), + ) + assert( + Object.hasOwn( + hashesModule.perResourceSriHashes.scripts, + 'https://code.jquery.com/ui/1.13.2/jquery-ui.min.js', + ), + ) + }) }) diff --git a/@kindspells/astro-shield/e2e/fixtures/hybrid3/astro.config.mjs b/@kindspells/astro-shield/e2e/fixtures/hybrid3/astro.config.mjs index af377da..8ba141b 100644 --- a/@kindspells/astro-shield/e2e/fixtures/hybrid3/astro.config.mjs +++ b/@kindspells/astro-shield/e2e/fixtures/hybrid3/astro.config.mjs @@ -24,9 +24,15 @@ export default defineConfig({ adapter: node({ mode: 'standalone' }), integrations: [ shield({ - enableStatic_SRI: true, - enableMiddleware_SRI: true, - sriHashesModule, + sri: { + enableStatic: true, + enableMiddleware: true, + hashesModule: sriHashesModule, + scriptsAllowListUrls: [ + 'https://code.jquery.com/jquery-3.7.1.slim.min.js', + 'https://code.jquery.com/ui/1.13.2/jquery-ui.min.js', + ], + }, securityHeaders: { contentSecurityPolicy: { cspDirectives: { diff --git a/@kindspells/astro-shield/e2e/fixtures/hybrid3/src/pages/injected.astro b/@kindspells/astro-shield/e2e/fixtures/hybrid3/src/pages/injected.astro new file mode 100644 index 0000000..1425901 --- /dev/null +++ b/@kindspells/astro-shield/e2e/fixtures/hybrid3/src/pages/injected.astro @@ -0,0 +1,32 @@ +--- +/* + * SPDX-FileCopyrightText: 2024 KindSpells Labs S.L. + * + * SPDX-License-Identifier: MIT + */ + +export const prerender = false +import '../styles/main.css' +--- + + + + + My Static Test Site + + + + + + + + + + +

This document simulates a page showing an injected script

+ + diff --git a/@kindspells/astro-shield/src/core.mjs b/@kindspells/astro-shield/src/core.mjs index a8d2601..e9615c0 100644 --- a/@kindspells/astro-shield/src/core.mjs +++ b/@kindspells/astro-shield/src/core.mjs @@ -21,6 +21,13 @@ import { patchHeaders } from './headers.mjs' * @typedef {import('./core.js').MiddlewareHashes} MiddlewareHashes * @typedef {import('./core.js').Logger} Logger * @typedef {import('astro').AstroIntegration} Integration + * @typedef {{ + * [k in keyof HashesCollection]: HashesCollection[k] extends Set + * ? string[] | undefined + * : (k extends 'perPageSriHashes' + * ? Record + * : Record<'scripts' | 'styles', Record>) + * }} HashesModule */ /** @@ -529,11 +536,11 @@ export const scanForNestedResources = async (logger, dirPath, h) => { } /** - * @param {Required>} sri + * @param {Pick} sri * @param {HashesCollection} h */ export const scanAllowLists = async (sri, h) => { - for (const scriptUrl of sri.scriptsAllowListUrls) { + for (const scriptUrl of sri.scriptsAllowListUrls ?? []) { const resourceResponse = await fetch(scriptUrl, { method: 'GET' }) const resourceContent = await resourceResponse.arrayBuffer() const sriHash = generateSRIHash(resourceContent) @@ -542,7 +549,7 @@ export const scanAllowLists = async (sri, h) => { h.perResourceSriHashes.scripts.set(scriptUrl, sriHash) } - for (const styleUrl of sri.stylesAllowListUrls) { + for (const styleUrl of sri.stylesAllowListUrls ?? []) { const resourceResponse = await fetch(styleUrl, { method: 'GET' }) const resourceContent = await resourceResponse.arrayBuffer() const sriHash = generateSRIHash(resourceContent) @@ -591,14 +598,9 @@ export async function generateSRIHashesModule( } if (await doesFileExist(sriHashesModule)) { - const hModule = /** - @type {{ - [k in keyof HashesCollection]: HashesCollection[k] extends Set - ? string[] | undefined - : (k extends 'perPageSriHashes' - ? Record - : Record<'scripts' | 'styles', Record>) - }} */ (await import(/* @vite-ignore */ sriHashesModule)) + const hModule = /** @type {HashesModule} */ ( + await import(/* @vite-ignore */ sriHashesModule) + ) extResourceHashesChanged = !sriHashesEqual( perResourceHashes, @@ -683,6 +685,8 @@ export const processStaticFiles = async (logger, { distDir, sri }) => { styles: new Map(), }, } + await scanAllowLists(sri, h) + await scanForNestedResources(logger, distDir, h) await scanDirectory( logger, distDir, @@ -693,7 +697,6 @@ export const processStaticFiles = async (logger, { distDir, sri }) => { sri, ) - await scanForNestedResources(logger, distDir, h) if (!sri.hashesModule) { return @@ -790,33 +793,58 @@ const loadVirtualMiddlewareModule = async ( let extraImports = '' let staticHashesModuleLoader = '' - if ( - sri.enableStatic && - sri.hashesModule && - !(await doesFileExist(sri.hashesModule)) - ) { - const h = /** @satisfies {HashesCollection} */ { - inlineScriptHashes: new Set(), - inlineStyleHashes: new Set(), - extScriptHashes: new Set(), - extStyleHashes: new Set(), - perPageSriHashes: new Map(), - perResourceSriHashes: { - scripts: new Map(), - styles: new Map(), - }, + if (sri.enableStatic && sri.hashesModule) { + let shouldRegenerateHashesModule = !(await doesFileExist(sri.hashesModule)) + + if (!shouldRegenerateHashesModule) { + try { + const hashesModule = /** @type {HashesModule} */ ( + await import(sri.hashesModule) + ) + + for (const allowedScript of sri.scriptsAllowListUrls) { + if ( + !Object.hasOwn( + hashesModule.perResourceSriHashes.scripts, + allowedScript, + ) + ) { + shouldRegenerateHashesModule = true + break + } + } + } catch (err) { + logger.warn( + `Failed to load SRI hashes module "${sri.hashesModule}", it will be re-generated:\n\t${err}`, + ) + shouldRegenerateHashesModule = true + } } - // 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, - sri.hashesModule, - false, // So we don't get redundant warnings - ) + if (shouldRegenerateHashesModule) { + const h = /** @satisfies {HashesCollection} */ { + inlineScriptHashes: new Set(), + inlineStyleHashes: new Set(), + extScriptHashes: new Set(), + extStyleHashes: new Set(), + perPageSriHashes: new Map(), + perResourceSriHashes: { + scripts: new Map(), + styles: new Map(), + }, + } + + // 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, + sri.hashesModule, + false, // So we don't get redundant warnings + ) + } } if ( diff --git a/@kindspells/astro-shield/tests/core.test.mts b/@kindspells/astro-shield/tests/core.test.mts index 2892fdd..38caa2d 100644 --- a/@kindspells/astro-shield/tests/core.test.mts +++ b/@kindspells/astro-shield/tests/core.test.mts @@ -365,7 +365,7 @@ describe('updateStaticPageSriHashes', () => { My Test Page - + ` @@ -382,7 +382,7 @@ describe('updateStaticPageSriHashes', () => { expect(h.extScriptHashes.size).toBe(1) expect( h.extScriptHashes.has( - 'sha256-iozyX5cgvSGJZLKhhN7CRl6tn/jC3vYkBm8jfGv4x78=', + 'sha256-zOEqmAz4SCAi+TcSQgdhUuurJfrfnwWqtmdTOP+bBkc=', ), ).toBe(true) expect(h.inlineScriptHashes.size).toBe(0)