From 76b0efbaecf36df2616907f9e8d8992d070d1a9d Mon Sep 17 00:00:00 2001 From: thebanjomatic Date: Fri, 12 Apr 2024 15:03:42 -0600 Subject: [PATCH 1/2] fix(cspNonce): don't overwrite existing nonce values Fixes: #16414 --- packages/vite/src/node/plugins/html.ts | 7 +++++++ playground/csp/__tests__/csp.spec.ts | 14 ++++++++++++++ playground/csp/index.html | 10 ++++++++++ 3 files changed, 31 insertions(+) diff --git a/packages/vite/src/node/plugins/html.ts b/packages/vite/src/node/plugins/html.ts index 232f9cc0e037a3..8a1b93174b48b5 100644 --- a/packages/vite/src/node/plugins/html.ts +++ b/packages/vite/src/node/plugins/html.ts @@ -1189,6 +1189,13 @@ export function injectNonceAttributeTagHook( parseRelAttr(attr.value).some((a) => processRelType.has(a)), )) ) { + const alreadyContainsNonce = node.attrs.some( + ({ name }) => name === 'nonce', + ) + if (alreadyContainsNonce) { + return + } + // if the closing of the start tag includes a `/`, the offset should be 2 so the nonce // is appended prior to the `/` const appendOffset = diff --git a/playground/csp/__tests__/csp.spec.ts b/playground/csp/__tests__/csp.spec.ts index 49155665a4143f..93aa7cbbc1c278 100644 --- a/playground/csp/__tests__/csp.spec.ts +++ b/playground/csp/__tests__/csp.spec.ts @@ -27,6 +27,20 @@ test('dynamic js', async () => { ) }) +test('inline js', async () => { + await expectWithRetry(() => page.textContent('.inline-js')).toBe( + 'inline-js: ok', + ) +}) + +test('nonce attributes are not repeated', async () => { + const htmlSource = await page.content() + expect(htmlSource).not.toContain(/nonce=""[^>]*nonce=""/) + await expectWithRetry(() => page.textContent('.double-nonce-js')).toBe( + 'double-nonce-js: ok', + ) +}) + test('meta[property=csp-nonce] is injected', async () => { const meta = await page.$('meta[property=csp-nonce]') expect(await (await meta.getProperty('nonce')).jsonValue()).not.toBe('') diff --git a/playground/csp/index.html b/playground/csp/index.html index e782bad46e1b8c..c29d55054921d6 100644 --- a/playground/csp/index.html +++ b/playground/csp/index.html @@ -11,3 +11,13 @@

dynamic

js: error

dynamic-js: error

+

inline-js: error

+

double-nonce-js: error

+ + From 259ba21739677d4e27d859483110d8775519e94a Mon Sep 17 00:00:00 2001 From: thebanjomatic Date: Sat, 13 Apr 2024 08:59:06 -0600 Subject: [PATCH 2/2] refactor: reduce duplication, improve readability --- packages/vite/src/node/plugins/html.ts | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/vite/src/node/plugins/html.ts b/packages/vite/src/node/plugins/html.ts index 8a1b93174b48b5..f5acc47a39d8ab 100644 --- a/packages/vite/src/node/plugins/html.ts +++ b/packages/vite/src/node/plugins/html.ts @@ -1180,31 +1180,29 @@ export function injectNonceAttributeTagHook( return } + const { nodeName, attrs, sourceCodeLocation } = node + if ( - node.nodeName === 'script' || - (node.nodeName === 'link' && - node.attrs.some( + nodeName === 'script' || + (nodeName === 'link' && + attrs.some( (attr) => attr.name === 'rel' && parseRelAttr(attr.value).some((a) => processRelType.has(a)), )) ) { - const alreadyContainsNonce = node.attrs.some( - ({ name }) => name === 'nonce', - ) - if (alreadyContainsNonce) { + // If we already have a nonce attribute, we don't need to add another one + if (attrs.some(({ name }) => name === 'nonce')) { return } + const startTagEndOffset = sourceCodeLocation!.startTag!.endOffset + // if the closing of the start tag includes a `/`, the offset should be 2 so the nonce // is appended prior to the `/` - const appendOffset = - html[node.sourceCodeLocation!.startTag!.endOffset - 2] === '/' ? 2 : 1 + const appendOffset = html[startTagEndOffset - 2] === '/' ? 2 : 1 - s.appendRight( - node.sourceCodeLocation!.startTag!.endOffset - appendOffset, - ` nonce="${nonce}"`, - ) + s.appendRight(startTagEndOffset - appendOffset, ` nonce="${nonce}"`) } })