Skip to content

Commit

Permalink
fix: duplicated crossorigin attr
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Correa Casablanca <[email protected]>
  • Loading branch information
castarco committed Sep 11, 2024
1 parent 3204c8c commit a9bdc59
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 24 deletions.
2 changes: 1 addition & 1 deletion @kindspells/astro-shield/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@kindspells/astro-shield",
"version": "1.3.7",
"version": "1.3.8",
"description": "Astro integration to enhance your website's security with SubResource Integrity hashes, Content-Security-Policy headers, and other techniques.",
"private": false,
"type": "module",
Expand Down
36 changes: 19 additions & 17 deletions @kindspells/astro-shield/src/core.mts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ const linkStyleReplacer: ElemReplacer = (hash, attrs, setCrossorigin) =>
setCrossorigin ? ' crossorigin="anonymous"' : ''
}/>`

// TODO: Support more algorithms (different ones, and many for the same element)
const anonymousCrossOriginRegex =
/crossorigin\s*=\s*("anonymous"|'anonymous'|anonymous)/i
const integrityRegex =
/^integrity\s*=\s*("(?<integrity1>sha256-[a-z0-9+\/]{43}=)"|'(?<integrity2>sha256-[a-z0-9+\/]{43}=)')$/i
const relStylesheetRegex =
Expand Down Expand Up @@ -166,10 +167,10 @@ export const updateStaticPageSriHashes = async (

const pageHashes =
h.perPageSriHashes.get(relativeFilepath) ??
/** @type {PerPageHashes} */ ({
({
scripts: new Set(),
styles: new Set(),
})
} satisfies PerPageHashes)
h.perPageSriHashes.set(relativeFilepath, pageHashes)

let updatedContent = content
Expand Down Expand Up @@ -277,12 +278,14 @@ export const updateStaticPageSriHashes = async (
}

if (sriHash) {
const hasAnonymousCrossOrigin = anonymousCrossOriginRegex.test(attrs)

updatedContent = updatedContent.replace(
match[0],
replacer(
sriHash,
attrs ? ` ${attrs}` : '',
setCrossorigin,
setCrossorigin && !hasAnonymousCrossOrigin,
elemContent,
),
)
Expand Down Expand Up @@ -323,8 +326,7 @@ export const updateDynamicPageSriHashes = async (
const attrs = match.groups?.attrs?.trim() ?? ''
const elemContent = match.groups?.content ?? ''

/** @type {string | undefined} */
let sriHash = undefined
let sriHash: string | undefined = undefined
let setCrossorigin = false

if (attrs) {
Expand Down Expand Up @@ -452,12 +454,14 @@ export const updateDynamicPageSriHashes = async (
}

if (sriHash) {
const hasAnonymousCrossOrigin = anonymousCrossOriginRegex.test(attrs)

updatedContent = updatedContent.replace(
match[0],
replacer(
sriHash,
attrs ? ` ${attrs}` : '',
setCrossorigin,
setCrossorigin && !hasAnonymousCrossOrigin,
elemContent,
),
)
Expand Down Expand Up @@ -663,8 +667,8 @@ export async function generateSRIHashesModule(
}

if (await doesFileExist(sriHashesModule)) {
const hModule: HashesModule = (
await import(/* @vite-ignore */ sriHashesModule)
const hModule: HashesModule = await import(
/* @vite-ignore */ sriHashesModule
)

extResourceHashesChanged = !sriHashesEqual(
Expand Down Expand Up @@ -779,8 +783,7 @@ export const getMiddlewareHandler = (
globalHashes: MiddlewareHashes,
sri: Required<SRIOptions>,
): MiddlewareHandler => {
/** @satisfies {import('astro').MiddlewareHandler} */
return async (_ctx, next) => {
return (async (_ctx, next) => {
const response = await next()
const content = await response.text()

Expand All @@ -797,7 +800,7 @@ export const getMiddlewareHandler = (
headers: response.headers,
})
return patchedResponse
}
}) satisfies MiddlewareHandler
}

/**
Expand All @@ -809,8 +812,7 @@ export const getCSPMiddlewareHandler = (
securityHeadersOpts: SecurityHeadersOptions,
sri: Required<SRIOptions>,
): MiddlewareHandler => {
/** @satisfies {import('astro').MiddlewareHandler} */
return async (_ctx, next) => {
return (async (_ctx, next) => {
const response = await next()
const content = await response.text()

Expand All @@ -827,7 +829,7 @@ export const getCSPMiddlewareHandler = (
headers: patchHeaders(response.headers, pageHashes, securityHeadersOpts),
})
return patchedResponse
}
}) satisfies MiddlewareHandler
}

const middlewareVirtualModuleId = 'virtual:@kindspells/astro-shield/middleware'
Expand All @@ -847,8 +849,8 @@ const loadVirtualMiddlewareModule = async (

if (!shouldRegenerateHashesModule) {
try {
const hashesModule: HashesModule = (
await import(/* @vite-ignore */ sri.hashesModule)
const hashesModule: HashesModule = await import(
/* @vite-ignore */ sri.hashesModule
)

for (const allowedScript of sri.scriptsAllowListUrls) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type PageHashesCollection = Record<

const testsDir = new URL('.', import.meta.url).pathname
const fixturesDir = resolve(testsDir, 'fixtures')
const rootDir = resolve(testsDir, '..')
const rootDir = resolve(testsDir, '..', '..')
const distDir = resolve(rootDir, 'dist')

const getEmptyHashes = () => ({
Expand Down Expand Up @@ -624,7 +624,7 @@ describe('updateStaticPageSriHashes', () => {
<title>My Test Page</title>
</head>
<body>
<script type="module" src="/core.mjs" integrity="sha256-e91QMz4oDk+n/vnPGAOmoNDYdO61N9wDM5iFlll+6r8="></script>
<script type="module" src="/core.mjs" integrity="sha256-57NR9VGwX5U1svn4FZBRRffMg+4n3Fquhfcn6lEtk9Q="></script>
</body>
</html>`

Expand All @@ -642,7 +642,7 @@ describe('updateStaticPageSriHashes', () => {

expect(
h.extScriptHashes.has(
'sha256-e91QMz4oDk+n/vnPGAOmoNDYdO61N9wDM5iFlll+6r8=',
'sha256-57NR9VGwX5U1svn4FZBRRffMg+4n3Fquhfcn6lEtk9Q=',
),
).toBe(true)
expect(h.inlineScriptHashes.size).toBe(0)
Expand Down Expand Up @@ -692,6 +692,48 @@ describe('updateStaticPageSriHashes', () => {
expect(h.extStyleHashes.size).toBe(0)
})

it('adds sri hash to external script without duplicating the crossorigin attribute (cross origin)', async () => {
const remoteScript =
'https://raw.githubusercontent.com/KindSpells/astro-shield/ae9521048f2129f633c075b7f7ef24e11bbd1884/main.mjs'
const content = `<html>
<head>
<title>My Test Page</title>
</head>
<body>
<script type="module" src="${remoteScript}" crossorigin="anonymous"></script>
</body>
</html>`

const expected = `<html>
<head>
<title>My Test Page</title>
</head>
<body>
<script type="module" src="${remoteScript}" crossorigin="anonymous" integrity="sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q="></script>
</body>
</html>`

const h = getEmptyHashes()
const updated = await updateStaticPageSriHashes(
console,
rootDir,
'index.html',
content,
h,
)

expect(updated).toEqual(expected)
expect(h.extScriptHashes.size).toBe(1)
expect(
h.extScriptHashes.has(
'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=',
),
).toBe(true)
expect(h.inlineScriptHashes.size).toBe(0)
expect(h.inlineStyleHashes.size).toBe(0)
expect(h.extStyleHashes.size).toBe(0)
})

it('adds sri hash to external style (same origin)', async () => {
const content = `<html>
<head>
Expand Down Expand Up @@ -1013,6 +1055,53 @@ describe('updateDynamicPageSriHashes', () => {
expect(pageHashes.styles.size).toBe(0)
})

it('adds sri hash to external script without duplicating the crossorigin attribute when allow-listed (cross origin)', async () => {
const remoteScript =
'https://raw.githubusercontent.com/KindSpells/astro-shield/ae9521048f2129f633c075b7f7ef24e11bbd1884/main.mjs'
const content = `<html>
<head>
<title>My Test Page</title>
</head>
<body>
<script type="module" src="${remoteScript}" crossorigin='anonymous'></script>
</body>
</html>`

const expected = `<html>
<head>
<title>My Test Page</title>
</head>
<body>
<script type="module" src="${remoteScript}" crossorigin='anonymous' integrity="sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q="></script>
</body>
</html>`

const h = getMiddlewareHashes()
h.scripts.set(
remoteScript,
'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=',
)
const { pageHashes, updatedContent } = await updateDynamicPageSriHashes(
console,
content,
h,
)

expect(updatedContent).toEqual(expected)
expect(h.scripts.size).toBe(1)
expect(h.styles.size).toBe(0)
expect(h.scripts.get(remoteScript)).toEqual(
'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=',
)
expect(pageHashes.scripts.size).toBe(1)
expect(
pageHashes.scripts.has(
'sha256-i4WR4ifasidZIuS67Rr6Knsy7/hK1xbVTc8ZAmnAv1Q=',
),
).toBe(true)
expect(pageHashes.styles.size).toBe(0)
})

it('adds sri hash to external style (same origin)', async () => {
const content = `<html>
<head>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { doesFileExist, scanDirectory } from '#as/fs.mts'
const testsDir = new URL('.', import.meta.url).pathname

describe('doesFileExist', () => {
it.each([['./core.test.mts'], ['../src/core.mts'], ['../src/main.mts']])(
it.each([['./core.test.mts'], ['../core.mts'], ['../main.mts']])(
'returns true for existing files',
async (filename: string) => {
expect(await doesFileExist(resolve(testsDir, filename))).toBe(true)
Expand Down
2 changes: 1 addition & 1 deletion @kindspells/astro-shield/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@

"skipLibCheck": true
},
"include": ["src/*", "tests/**/*.mts", "e2e/**/*.mts"]
"include": ["src/*", "e2e/**/*.mts"]
}
2 changes: 1 addition & 1 deletion @kindspells/astro-shield/vitest.config.unit.mts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ export default defineConfig({
},
reportsDirectory: 'coverage-unit',
},
include: ['tests/**/*.test.mts'],
include: ['src/**/tests/**/*.test.mts'],
},
})

0 comments on commit a9bdc59

Please sign in to comment.