Skip to content

Inline scripts generated as a result of appDir preventing use of strict CSP #43743

Closed

Description

@lc-51

Verify canary release

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

Provide environment information

Operating System:
Platform: darwin
Arch: arm64
Version: Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:23 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T6000
Binaries:
Node: 16.16.0
npm: 8.11.0
Yarn: N/A
pnpm: N/A
Relevant packages:
next: 13.0.3
eslint-config-next: 13.0.6
react: 18.2.0
react-dom: 18.2.0

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

App directory (appDir: true)

Link to reproduction - Issues with a link to complete (but minimal) reproduction code will be addressed faster

To Reproduce

Just setup a basic "Hello world" app using the appDir feature, with: npx create-next-app@latest --experimental-app.

Describe the Bug

I posted a Help discussion about this yesterday (#43710) but it was suggested that I create an issue for it.

I've noticed a number of inline scripts being injected into the page, all beginning self.__next_f... They seem to contain parts of random stringified components / webpack chunks. They're only generated when using the appDir. Obviously this isn't ideal from a CSP standpoint, as they're just generic inline scripts (so no src to whitelist, unlike the usual /_next/static/ scripts).

Is there a plan for these scripts to be removed in future versions, or is there a recommended way of maintaining a strict CSP (i.e. without using unsafe-inline)? Perhaps using nonces, hashes or similar?

Thanks

Expected Behavior

No inline scripts generated as a result of using the appDir feature.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

next start

Activity

  1. added
    bugIssue was opened via the bug report template.
    on Dec 5, 2022
  2. bjarn commented on Mar 21, 2023

    @bjarn

    This issue still persists in the latest 13.2.4.

  3. pepijn-vanvlaanderen commented on Mar 22, 2023

    @pepijn-vanvlaanderen

    +1

  4. lsagetlethias commented on Apr 11, 2023

    @lsagetlethias

    This issue still persists in the latest 13.3.0.
    Potentially critical.

  5. paulsouche commented on Apr 13, 2023

    @paulsouche

    Hello !

    When Generating static pages, the export worker uses a MockRequest that does not contain the headers. So the server doesn't retrieve the csp header here and the page is generated without the nonce value.

    Could it be possible somehow to generate this request with knowledge of the next config ? Meanwhile, moving to 'unsafe-inline'

    Thanks anyway for the great job

  6. danieltott commented on Apr 28, 2023

    @danieltott
    Contributor

    This is actually a bug in React.

    Right now in Next@canary, if you add a content-security-policy with a nonce-xxx value inside the script-src directive (via middleware), it will pass that nonce value to the React rendering engine. However, React is only including nonce in inline scripts, not <script src=...> scripts.

  7. zackdotcomputer commented on May 10, 2023

    @zackdotcomputer
    Contributor

    @danieltott I think that bug is valid, but either tangential to this one or else incomplete. This is related to inline scripts, which do properly get the nonce value, like you stated, but only during development. When running a production-like build with next build && next start the nonce-value is also omitted from the inline scripts, causing this issue. (This is confirmed as recently as 13.4.1)

    We are able to work around this on the pages router because the _document.tsx file can use NextScript.getInlineScriptSource(props) to get the inline-script source, which we can then generate a hash for and include as a safe sha. However, this is another thing which is either unintentionally omitted or intentionally locked down in the new app router paradigm.

  8. danieltott commented on May 10, 2023

    @danieltott
    Contributor

    @zackdotcomputer right now as of the latest canary (13.4.2-canary.4) the nonce value is there in inline scripts in production builds.

    Check out the source here: https://nextjs-csp-report-only.vercel.app/csp

    Repo: https://github.com/Sprokets/nextjs-csp-report-only

    The PR in React (react/react#26738) has been merged, and as soon as there's a new release, NextJS (via React) will render nonce attributes in all bootstrap scripts.

  9. danieltott commented on May 10, 2023

    @danieltott
    Contributor

    Here's the middleware function (full source):

    export async function middleware(request: NextRequest) {
      // generate CSP and nonce
      const { csp, nonce } = generateCsp();
    
      // Clone the request headers
      const requestHeaders = new Headers(request.headers);
    
      // set nonce request header to read in pages if needed
      requestHeaders.set('x-nonce', nonce);
    
      // set CSP header
      // switching for report-only or regular for repro on
      // https://github.com/vercel/next.js/issues/48966
      const headerKey =
        request.nextUrl.pathname === '/csp-report-only'
          ? 'content-security-policy-report-only'
          : 'content-security-policy';
    
      // Set the CSP header so that `app-render` can read it and generate tags with the nonce
      requestHeaders.set(headerKey, csp);
    
      // create new response
      const response = NextResponse.next({
        request: {
          // New request headers
          headers: requestHeaders,
        },
      });
    
      // Also set the CSP so that it is outputted to the browser
      response.headers.set(headerKey, csp);
    
      return response;
    }
  10. danieltott commented on May 10, 2023

    @danieltott
    Contributor

    The tricky part (and this may be a bug and/or new feature to fix/change, or it may be as designed) is you need to set the header twice, once as part of the request while creating the NextResponse and then again as a response header. app-render needs the header to be there in the request so that it can pull out the nonce.

  11. danieltott commented on May 10, 2023

    @danieltott
    Contributor

    @zackdotcomputer OK actually - I think this issue is closeable once nextjs 13.4.2 is released! I just did some more digging, and it looks like next.js is now using the React canary channel for react-builtin which is what Next uses for rendering.

    I assume that Next using React canary is only for Next's own canary builds, so this won't get released until React 18.3.0 is officially out.

    You can actually see this working on my repro on vercel: https://nextjs-csp-report-only.vercel.app/csp - the nonce value is present for all scripts and there are no warnings at all in the console!

  12. danieltott commented on May 10, 2023

    @danieltott
    Contributor

    OK sorry for all the comments - but it looks like that upgrade to the react 18.3 canary landed in nextjs 13.4.0 - so it's good to go! I believe this issue can be safely closed.

  13. timneutkens commented on May 11, 2023

    @timneutkens
    Contributor

    React canary is used when you enable the Server Actions flag. I'll close this issue based on @danieltott's comments. If you're still running into it let us know.

  14. danieltott commented on May 11, 2023

    @danieltott
    Contributor

    @timneutkens Just as a clarification, I don't think you need to enable server actions. I'm pretty sure React canary is used by default in the nextjs server code (the compiled react-builtin), and when any of the experimental flags are on, next uses the latest experimental release from react. See here:

    next.js/package.json

    Lines 200 to 205 in f169c32

    "react-builtin": "npm:[email protected]",
    "react-dom": "18.2.0",
    "react-dom-17": "npm:[email protected]",
    "react-dom-builtin": "npm:[email protected]",
    "react-dom-experimental-builtin": "npm:[email protected]",
    "react-experimental-builtin": "npm:[email protected]",

  15. timneutkens commented on May 11, 2023

    @timneutkens
    Contributor

    Sorry for the confusion: Without the server actions flag we use react@canary, with the server actions flag we use react@experimental indeed!

  16. igordanchenko commented on May 16, 2023

    @igordanchenko

    Is there a solution for statically-generated pages that doesn't require the use of middleware? E.g., calculate sha sums at build time, and inject them into the csp header in the next.config.js?

  17. paulsouche commented on May 22, 2023

    @paulsouche

    @danieltott I agree with @igordanchenko, in your example, all your pages are deopted

    [    ] - info Generating static pages (0/7)- warn Entire page /csp-report-only deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering /csp-report-only
    - warn Entire page / deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering /
    - warn Entire page /csp deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering /csp
    

    Any chance it could work with statically-generated pages ? Do you want us to open a new issue ?

    Thanks anyway for the great job

  18. danieltott commented on May 22, 2023

    @danieltott
    Contributor

    @paulsouche @igordanchenko CSP nonces are incompatible with static pages, since they must be different for each request. My example was deopted on purpose for this reason (by reading next/headers).

    For statically generated pages, CSP Hashes are the only solution, and there doesn't exist a built-in path forward for that in Next.js.

    I'm not a maintainer here, but my guess would be the best way forward would be a new discussion - I think having next.js handle this might end up getting a little complicated, but I could imagine a custom middleware-based solution here...

    If you do end up creating a new discussion, I'd appreciate a mention (or an update here) so I can follow along!

  19. github-actions commented on Jun 22, 2023

    @github-actions
    Contributor

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

  20. locked as resolved and limited conversation to collaborators on Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssue was opened via the bug report template.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions