Skip to content

Commit

Permalink
refactor: introduce new sri options
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Correa Casablanca <andreu@kindspells.dev>
  • Loading branch information
castarco committed Mar 26, 2024
1 parent 9252be6 commit ad3abf5
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 30 deletions.
45 changes: 17 additions & 28 deletions src/core.mjs
Expand Up @@ -701,26 +701,24 @@ const resolvedMiddlewareVirtualModuleId = `\0${middlewareVirtualModuleId}`

/**
* @param {Logger} logger
* @param {boolean} enableStatic_SRI
* @param {string | undefined} sriHashesModule
* @param {Required<SRIOptions>} sri
* @param {SecurityHeadersOptions | undefined} securityHeadersOptions
* @param {string} publicDir
* @returns {Promise<string>}
*/
const loadVirtualMiddlewareModule = async (
logger,
enableStatic_SRI,
sriHashesModule,
sri,
securityHeadersOptions,
publicDir,
) => {
let extraImports = ''
let staticHashesModuleLoader = ''

if (
enableStatic_SRI &&
sriHashesModule &&
!(await doesFileExist(sriHashesModule))
sri.enableStatic &&
sri.hashesModule &&
!(await doesFileExist(sri.hashesModule))
) {
const h = /** @satisfies {HashesCollection} */ {
inlineScriptHashes: new Set(),
Expand All @@ -740,17 +738,17 @@ const loadVirtualMiddlewareModule = async (
await generateSRIHashesModule(
logger,
h,
sriHashesModule,
sri.hashesModule,
false, // So we don't get redundant warnings
)
}

if (
enableStatic_SRI &&
sriHashesModule &&
(await doesFileExist(sriHashesModule))
sri.enableStatic &&
sri.hashesModule &&
(await doesFileExist(sri.hashesModule))
) {
extraImports = `import { perResourceSriHashes } from '${sriHashesModule}'`
extraImports = `import { perResourceSriHashes } from '${sri.hashesModule}'`
staticHashesModuleLoader = `
try {
if (perResourceSriHashes) {
Expand All @@ -769,11 +767,11 @@ try {
console.error('Failed to load static hashes module:', err)
}
`
} else if (enableStatic_SRI && sriHashesModule) {
} else if (sri.enableStatic && sri.hashesModule) {
// Highly unlikely that this happens because of the provisional hashes
// module, but the world is a strange place.
logger.warn(
`The SRI hashes module "${sriHashesModule}" did not exist at build time. You may have to run the build step again`,
`The SRI hashes module "${sri.hashesModule}" did not exist at build time. You may have to run the build step again`,
)
}

Expand Down Expand Up @@ -805,19 +803,12 @@ export const onRequest = await (async () => {

/**
* @param {Logger} logger
* @param {boolean} enableStatic_SRI
* @param {string | undefined} sriHashesModule
* @param {Required<SRIOptions>} sri
* @param {SecurityHeadersOptions | undefined} securityHeaders
* @param {string} publicDir
* @return {import('vite').Plugin}
*/
const getViteMiddlewarePlugin = (
logger,
enableStatic_SRI,
sriHashesModule,
securityHeaders,
publicDir,
) => {
const getViteMiddlewarePlugin = (logger, sri, securityHeaders, publicDir) => {
return {
name: 'vite-plugin-astro-shield',
resolveId(id) {
Expand All @@ -831,8 +822,7 @@ const getViteMiddlewarePlugin = (
case resolvedMiddlewareVirtualModuleId:
return await loadVirtualMiddlewareModule(
logger,
enableStatic_SRI,
sriHashesModule,
sri,
securityHeaders,
publicDir,
)
Expand All @@ -844,7 +834,7 @@ const getViteMiddlewarePlugin = (
}

/**
* @param {SRIOptions} sri
* @param {Required<SRIOptions>} sri
* @param {SecurityHeadersOptions | undefined} securityHeaders
* @returns
*/
Expand All @@ -854,8 +844,7 @@ export const getAstroConfigSetup = (sri, securityHeaders) => {
const publicDir = fileURLToPath(config.publicDir)
const plugin = getViteMiddlewarePlugin(
logger,
sri.enableStatic ?? true,
sri.hashesModule,
sri,
securityHeaders,
publicDir,
)
Expand Down
34 changes: 34 additions & 0 deletions src/main.d.ts
Expand Up @@ -83,6 +83,40 @@ export type SRIOptions = {
* artifact.
*/
hashesModule?: string | undefined

/**
* 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.
*
* Defaults to 'all'.
*/
allowInlineStyles?: 'all' | 'static' | false

/**
* 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.
*
* Defaults to 'all'.
*/
allowInlineScripts?: 'all' | 'static' | false

/**
* Cross-Origin scripts must be explicitly allow-listed by URL in order to be
* allowed by the Content Security Policy.
*/
scriptsAllowListUrls?: string[]

/**
* Cross-Origin styles must be explicitly allow-listed by URL in order to be
* allowed by the Content Security Policy.
*/
stylesAllowListUrls?: string[]
}

export type SecurityHeadersOptions = {
Expand Down
6 changes: 6 additions & 0 deletions src/main.mjs
Expand Up @@ -63,6 +63,12 @@ export const shield = ({
enableMiddleware: sri?.enableMiddleware ?? enableMiddleware_SRI ?? false,
enableStatic: sri?.enableStatic ?? enableStatic_SRI ?? true,
hashesModule: sri?.hashesModule ?? sriHashesModule,

allowInlineScripts: sri?.allowInlineScripts ?? 'all',
allowInlineStyles: sri?.allowInlineStyles ?? 'all',

scriptsAllowListUrls: sri?.scriptsAllowListUrls ?? [],
stylesAllowListUrls: sri?.stylesAllowListUrls ?? [],
}

if (_sri.hashesModule && _sri.enableStatic === false) {
Expand Down
4 changes: 2 additions & 2 deletions tests/core.test.mts
Expand Up @@ -362,7 +362,7 @@ describe('updateStaticPageSriHashes', () => {
<title>My Test Page</title>
</head>
<body>
<script type="module" src="/core.mjs" integrity="sha256-KrxzzNH5AjdyG84oIMGj043N5e4ZnvFjIC7HKOVJMv4="></script>
<script type="module" src="/core.mjs" integrity="sha256-n5QiD5rG5p3P6N6SMn4S3Oc0MRSrqJdbCxTiOQHNdiU="></script>
</body>
</html>`

Expand All @@ -379,7 +379,7 @@ describe('updateStaticPageSriHashes', () => {
expect(h.extScriptHashes.size).toBe(1)
expect(
h.extScriptHashes.has(
'sha256-KrxzzNH5AjdyG84oIMGj043N5e4ZnvFjIC7HKOVJMv4=',
'sha256-n5QiD5rG5p3P6N6SMn4S3Oc0MRSrqJdbCxTiOQHNdiU=',
),
).toBe(true)
expect(h.inlineScriptHashes.size).toBe(0)
Expand Down

0 comments on commit ad3abf5

Please sign in to comment.