fix: pass `nonce` to `next/script` with `afterInteractive` strategy #56995

kevva · 2023-10-18T10:41:53Z

What?

This fixes an issue where the nonce attribute isn't set on next/script elements that has the afterInteractive (the default) strategy resulting in <link rel="preload" as="script"/> tags without a nonce.

Why?

For apps that uses 3rd party scripts (or any script) with a nonce loaded via next/script this is necessary unless you want them all to use beforeInteractive which isn't super nice for performance.

ijjk · 2023-10-18T10:42:22Z

Allow CI Workflow Run

approve CI run for commit: cdb18a1

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

SukkaW

LGTM, as ReactDOMFloat does have nonce (https://github.com/facebook/react/blob/6db7f4209e6f32ebde298a0b7451710dd6aa3e19/packages/react-dom/src/shared/ReactDOMFloat.js#L129) supports.

kevva · 2023-10-25T15:51:29Z

@SukkaW, anything else you need from me?

kevva · 2023-11-09T11:51:21Z

@timneutkens @ijjk, sorry for the ping but any chance we can get this reviewed and merged :)?

timneutkens

Looking at these it seems there is another case that is missing the nonce on preload 🤔 https://github.com/vercel/next.js/pull/56995/files#diff-d7794cd766752e9cdff323db1f142a20fc6ac348eb399d0ec5f600ed122d4b0eR346-R348

ijjk · 2023-11-12T09:28:41Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
buildDuration	14.1s	13.9s	N/A
buildDurationCached	7.4s	6.3s	N/A
nodeModulesSize	199 MB	199 MB	⚠️ +548 B
nextStartRea..uration (ms)	395ms	406ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
2453-HASH.js gzip	31.4 kB	31.4 kB	N/A
3304.HASH.js gzip	181 B	181 B	✓
3f784ff6-HASH.js gzip	53.7 kB	53.7 kB	✓
8299-HASH.js gzip	5.04 kB	5.04 kB	N/A
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	241 B	242 B	N/A
main-HASH.js gzip	32.2 kB	32.2 kB	N/A
webpack-HASH.js gzip	1.68 kB	1.68 kB	N/A
Overall change	99 kB	99 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
_app-HASH.js gzip	196 B	197 B	N/A
_error-HASH.js gzip	184 B	184 B	✓
amp-HASH.js gzip	505 B	505 B	✓
css-HASH.js gzip	324 B	325 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	258 B	258 B	✓
head-HASH.js gzip	352 B	352 B	✓
hooks-HASH.js gzip	370 B	371 B	N/A
image-HASH.js gzip	4.21 kB	4.21 kB	✓
index-HASH.js gzip	259 B	259 B	✓
link-HASH.js gzip	2.67 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	314 B	312 B	N/A
script-HASH.js gzip	386 B	386 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	6.57 kB	6.57 kB	✓

Client Build Manifests

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
_buildManifest.js gzip	481 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
index.html gzip	529 B	529 B	✓
link.html gzip	541 B	542 B	N/A
withRouter.html gzip	524 B	523 B	N/A
Overall change	529 B	529 B	✓

Edge SSR bundle Size

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
edge-ssr.js gzip	95.3 kB	95.3 kB	N/A
page.js gzip	3.04 kB	3.04 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
middleware-b..fest.js gzip	626 B	625 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	25.5 kB	25.5 kB	N/A
edge-runtime..pack.js gzip	839 B	839 B	✓
Overall change	990 B	990 B	✓

Next Runtimes

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
app-page-exp...dev.js gzip	170 kB	170 kB	✓
app-page-exp..prod.js gzip	97 kB	97 kB	✓
app-page-tur..prod.js gzip	98.8 kB	98.8 kB	✓
app-page-tur..prod.js gzip	93 kB	93 kB	✓
app-page.run...dev.js gzip	144 kB	144 kB	✓
app-page.run..prod.js gzip	91.5 kB	91.5 kB	✓
app-route-ex...dev.js gzip	21.4 kB	21.4 kB	✓
app-route-ex..prod.js gzip	15.2 kB	15.2 kB	✓
app-route-tu..prod.js gzip	15.2 kB	15.2 kB	✓
app-route-tu..prod.js gzip	14.9 kB	14.9 kB	✓
app-route.ru...dev.js gzip	21.1 kB	21.1 kB	✓
app-route.ru..prod.js gzip	14.9 kB	14.9 kB	✓
pages-api-tu..prod.js gzip	9.55 kB	9.55 kB	✓
pages-api.ru...dev.js gzip	9.82 kB	9.82 kB	✓
pages-api.ru..prod.js gzip	9.55 kB	9.55 kB	✓
pages-turbo...prod.js gzip	22.5 kB	22.5 kB	✓
pages.runtim...dev.js gzip	23.1 kB	23.1 kB	✓
pages.runtim..prod.js gzip	22.4 kB	22.4 kB	✓
server.runti..prod.js gzip	51 kB	51 kB	✓
Overall change	945 kB	945 kB	✓

build cache Overall increase ⚠️

	vercel/next.js canary	kevva/next.js fix-after-interactive-nonce	Change
0.pack gzip	1.58 MB	1.58 MB	⚠️ +149 B
index.pack gzip	106 kB	106 kB	N/A
Overall change	1.58 MB	1.58 MB	⚠️ +149 B

Diff details

Diff for middleware.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Commit: b32d9a9

ijjk · 2023-11-12T09:29:14Z

Failing test suites

Commit: b32d9a9

pnpm test test/integration/app-dir-export/test/dynamicpage-prod.test.ts

app dir - with output export - dynamic api route prod > production mode > should work in prod with dynamicPage 'force-static'

Expand output

● app dir - with output export - dynamic api route prod › production mode › should work in prod with dynamicPage 'force-static'

thrown: "Exceeded timeout of 60000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

  14 |             'Page with `dynamic = "force-dynamic"` couldn\'t be exported. `output: "export"` requires all pages be renderable statically',
  15 |         },
> 16 |       ])(
     |        ^
  17 |         'should work in prod with dynamicPage $dynamicPage',
  18 |         async ({ dynamicPage, expectedErrMsg }) => {
  19 |           await runTests({ isDev: false, dynamicPage, expectedErrMsg })

  at ../node_modules/.pnpm/jest-each@29.7.0/node_modules/jest-each/build/bind.js:47:15
      at Array.forEach (<anonymous>)
  at integration/app-dir-export/test/dynamicpage-prod.test.ts:16:8
  at integration/app-dir-export/test/dynamicpage-prod.test.ts:4:56
  at Object.describe (integration/app-dir-export/test/dynamicpage-prod.test.ts:3:1)

Read more about building and testing Next.js in contributing.md.

kevva · 2023-11-12T11:29:12Z

Looking at these it seems there is anothr case that is missing the nonce on preload 🤔 #56995 (files)

Not sure they're needed if nonce exist on their script tags which it does. Let me double check that though.

oioki · 2024-02-27T18:37:53Z

Hey @kevva
I am trying to implement nonce-based CSP in getsentry/sentry-docs#9203. It generally works, the only piece missing is a nonce on that link preload tag.
I tried your fix locally, and it works, i.e. the nonce is being added to the link preload tag, so I'm quite excited.

It seems like this PR is almost ready, we just need to decide on the beforeInteractive case and fix a couple of failing tests (unfortunately I don't see why due to The logs for this run have expired and are no longer available). Would appreciate moving this forward. 🙂

karlhorky · 2024-03-14T06:26:19Z

Not sure they're needed if nonce exist on their script tags which it does.

@kevva The nonce attribute is also on the other failing <script> tag (the afterInteractive one) and there it also requires the nonce on preload.

So I guess it's required, and the suggestion from @timneutkens should be implemented?

Cc @leerob @ijjk @gnoff @danieltott this is currently causing strict CSPs to report errors on preload requests when using next/script (<Script> component). The <link rel="preload"> that is generated with ReactDOM.preload() does not have the nonce attribute.

kevva · 2024-03-28T20:27:29Z

@timneutkens, I addressed the beforeInteractive case.

lynettelopez · 2024-04-01T14:38:31Z

@timneutkens @karlhorky My team is hoping this PR will be merged in soon. It's currently blocking our transition from Pages Router to App Router because of the CSP requirement our broader org sets. This feels like it's close to the finish line—anything I can do to speed this up?

changes applied

ijjk

Added change and test look good just expanded it a bit, thanks for the PR!

kevva requested review from timneutkens, ijjk, shuding, huozhi, feedthejim, ztanner and wyattjoh as code owners October 18, 2023 10:41

ijjk added area: tests type: next labels Oct 18, 2023

kevva force-pushed the fix-after-interactive-nonce branch from 43ac5d0 to 43b467c Compare October 20, 2023 11:31

SukkaW approved these changes Oct 21, 2023

View reviewed changes

kevva force-pushed the fix-after-interactive-nonce branch from 43b467c to cdb18a1 Compare November 6, 2023 18:14

timneutkens previously requested changes Nov 12, 2023

View reviewed changes

timneutkens added the CI approved Approve running CI for fork label Nov 12, 2023

kevva added 2 commits March 28, 2024 12:30

fix: pass nonce to next/script with afterInteractive strategy

f74c446

chore: remove unneeded script code

b04a847

kevva force-pushed the fix-after-interactive-nonce branch from 21a5e90 to b04a847 Compare March 28, 2024 19:32

fix: pass nonce for beforeInteractive strategy

762b16b

kevva requested a review from timneutkens March 28, 2024 20:21

Merge branch 'canary' into fix-after-interactive-nonce

0701835

update manifest

ca9f255

add browser test

b32d9a9

ijjk approved these changes Apr 1, 2024

View reviewed changes

ijjk enabled auto-merge (squash) April 1, 2024 23:09

ijjk merged commit d2ac9c6 into vercel:canary Apr 1, 2024
72 checks passed

r34son mentioned this pull request Apr 5, 2024

Next does not add nonce to link rel="preload" tag #64091

Closed

github-actions bot added the locked label Apr 16, 2024

github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: pass `nonce` to `next/script` with `afterInteractive` strategy #56995

fix: pass `nonce` to `next/script` with `afterInteractive` strategy #56995

kevva commented Oct 18, 2023 •

edited

ijjk commented Oct 18, 2023 •

edited

SukkaW left a comment •

edited

kevva commented Oct 25, 2023

kevva commented Nov 9, 2023

timneutkens left a comment •

edited

ijjk commented Nov 12, 2023 •

edited

ijjk commented Nov 12, 2023 •

edited

kevva commented Nov 12, 2023 •

edited

oioki commented Feb 27, 2024

karlhorky commented Mar 14, 2024 •

edited

kevva commented Mar 28, 2024

lynettelopez commented Apr 1, 2024

ijjk left a comment

fix: pass nonce to next/script with afterInteractive strategy #56995

fix: pass nonce to next/script with afterInteractive strategy #56995

Conversation

kevva commented Oct 18, 2023 • edited

What?

Why?

ijjk commented Oct 18, 2023 • edited

SukkaW left a comment • edited

Choose a reason for hiding this comment

kevva commented Oct 25, 2023

kevva commented Nov 9, 2023

timneutkens left a comment • edited

Choose a reason for hiding this comment

ijjk commented Nov 12, 2023 • edited

Stats from current PR

ijjk commented Nov 12, 2023 • edited

Failing test suites

kevva commented Nov 12, 2023 • edited

oioki commented Feb 27, 2024

karlhorky commented Mar 14, 2024 • edited

kevva commented Mar 28, 2024

lynettelopez commented Apr 1, 2024

ijjk left a comment

Choose a reason for hiding this comment

fix: pass `nonce` to `next/script` with `afterInteractive` strategy #56995

fix: pass `nonce` to `next/script` with `afterInteractive` strategy #56995

kevva commented Oct 18, 2023 •

edited

ijjk commented Oct 18, 2023 •

edited

SukkaW left a comment •

edited

timneutkens left a comment •

edited

ijjk commented Nov 12, 2023 •

edited

ijjk commented Nov 12, 2023 •

edited

kevva commented Nov 12, 2023 •

edited

karlhorky commented Mar 14, 2024 •

edited