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

Non sustainable way of using strict CSP with Next 13 (app router) #53928

Closed
1 task done
edgarlr opened this issue Aug 11, 2023 · 5 comments
Closed
1 task done

Non sustainable way of using strict CSP with Next 13 (app router) #53928

edgarlr opened this issue Aug 11, 2023 · 5 comments
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@edgarlr
Copy link

edgarlr commented Aug 11, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #22 SMP Tue Jan 10 18:39:00 UTC 2023
    Binaries:
      Node: 18.16.0
      npm: 9.5.1
      Yarn: 1.22.19
      pnpm: 8.4.0
    Relevant packages:
      next: 13.4.1
      eslint-config-next: 13.4.1
      react: 18.2.0
      react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

App Router, Middleware / Edge (API routes, runtime)

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/csp-nextjs-self-scripts-7mx4hz

To Reproduce

1. Add CSP with nonce to next.config.js

CodeSandbox: https://codesandbox.io/p/sandbox/csp-nextjs-self-scripts-7mx4hz

  1. Run either dev or prod task and open the preview in a new tab.
  2. Open Chrome devtools console and look at the logs

2. Add nonce to all scripts using middleware.

CodeSandbox: https://codesandbox.io/p/sandbox/middleware-prefetch-p9x783

  1. Run the prod task and open the preview.
  2. Scrolls down the page and look at the middleware logs executions on the terminal.

Describe the Bug

There isn't a maintainable way to add Strict CSP to Next.js 13 (app router).

We have tested 2 alternatives but none of them completely work:

1. Add CSP with nonce to next.config.js following the docs

The script-src with nonce is blocking even the scripts coming from next.js itself. previously we were able to pass the nonce using the NextScript.getInlineScriptSource(ctx) or to the <Head> and <NextScripts> components directly, but none of them are available in the app router anymore.

2. Add nonce to all scripts at request time using middleware. Following this discussion and this example

The nonce & middleware workaround works, but the middleware is getting executed on each link prefetch. On the documentation is not specified if there's a way to ignore prefetches.

This is unmaintainable for us since our website has several links per page and high traffic, we're getting ~20-30 middleware execution per route just for a user scrolling down the whole page. We reached our middleware execution month limit in just 3 days.

Expected Behavior

Add official support for Script CSP in Next.js, in any of the next options, or any other alternative.

1. CSP with a nonce in next.config.js

Similar to how it was supported on the pages router, NextScript.getInlineScriptSource(ctx) or <Head> and <NextScripts> components directly, allow to pass the nonce to nextjs scripts.

2. Nonce using middleware.

Add support to ignore prefetch in the middleware directly on the config or matcher so it doesn't get executed at all on prefetch.

Which browser are you using? (if relevant)

Chrome 115.0.5790.170 (arm64)

How are you deploying your application? (if relevant)

Vercel

NEXT-1541

@edgarlr edgarlr added the bug Issue was opened via the bug report template. label Aug 11, 2023
@github-actions github-actions bot added the Runtime Related to Node.js or Edge Runtime with Next.js. label Aug 11, 2023
@leerob leerob added the linear: next Confirmed issue that is tracked by the Next.js team. label Aug 21, 2023
@ijjk
Copy link
Member

ijjk commented Aug 25, 2023

Hi, the nonce CSP solution mentioned can be achieved and is supported in app router already if the CSP header is set in middleware and nonce generated there. An example of this being leveraged in our tests can be seen here:

if (request.nextUrl.pathname === '/bootstrap/with-nonce') {
const nonce = crypto.randomUUID()
return NextResponse.next({
headers: {
'Content-Security-Policy': `script-src 'nonce-${nonce}' 'strict-dynamic';`,
},
})
}

@2n3g5c9
Copy link

2n3g5c9 commented Aug 26, 2023

@ijjk I'm guessing this will work for NextJS generated scripts starting from 13.4.20 (cf. #54055 (comment))?

But as @edgarlr mentioned this is not sustainable for the customer and doesn't have to be a middleware.

@ijjk
Copy link
Member

ijjk commented Aug 26, 2023

It is possible to ignore for prefetches via the middleware matcher to reduce how much middleware is executed. Here's an example to ignore for prefetches and static files:

export const config = {
  matcher: [
    /*
     * Match all request paths except for the ones starting with:
     * - api (API routes)
     * - _next/static (static files)
     * - _next/image (image optimization files)
     * - favicon.ico (favicon file)
     */
    {
      source: '/((?!api|_next/static|_next/image|favicon.ico).*)',
      missing: [{ type: 'header', key: 'next-router-prefetch' }, { type: 'header', key: 'purpose', value: 'prefetch' }]
    }
  ],
}

We're working on official docs for this in #54601

@edgarlr edgarlr changed the title Non maintainable way of using strict CSP with Next 13 (app router) Non sustainable way of using strict CSP with Next 13 (app router) Aug 30, 2023
@edgarlr
Copy link
Author

edgarlr commented Aug 30, 2023

@ijjk thanks! That was what I was looking for, even when the first scenario of just adding the nonce to the CSP in headers of the next.config.js is still not working (hopefully in the future it will), this workaround seems to help inject the nonce per request in a more sustainable way!

@edgarlr edgarlr closed this as completed Aug 30, 2023
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
Development

No branches or pull requests

4 participants