Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: duplicated crossorigin attr #93

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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'],
},
})