Skip to content

Add an ID to the run time script #5288

Open

Description

@Timer

Activity

  1. added this to the 2.0.5 milestone on Oct 4, 2018
  2. PerfectPixel commented on Oct 5, 2018

    @PerfectPixel

    @Timer
    To bring forth the arguments from #5144 , let me elaborate on a few issues we encountered:

    I understand that adding a postbuild script is not difficult. We already have to add it to four repositories, I guess there are companies out there that use more. Furthermore, patching itself is not difficult, getting it right is.

    Example: myHTML.replace(/(<script>(?:.*?)<\/script>)/, `<script src="${manifest['runtime~main.js']}"></script>`). Adding an ID will help us to find the correct <script>...</script> so it is better than nothing. If for some reason the runtime script contains itself a </script> like let script = "<script src=" + src + "></script>" this will break.

    The solid solution is to parse the index.html as HTML, find the correct node, replace it, convert the content to a string and write it back. Then, I would say, we are no longer in the realm of an easy solution.

    If more scripts are inlined, this solution will break as well.

    I completely understand the desire to not create additional environment variables or configuration options as they surely add complexity. But in this case it is essentially punishing users that try to maintain a high security standard - regardless if they want to do it or are even (legally) obligated to do so).

    I can absolutely understand that saving one additional request for a small file is worth it. But while going from <script src="/static/js/runtime~main.js"></script> to an inlined script is very easy, the other way around becomes rather fragile.

    I do not intend to press this any further, I just do not think this is really a small edge-case.

    Thank you for your time & feedback 👍

  3. mgol commented on Oct 5, 2018

    @mgol

    Just for comparison, Angular CLI has always extracted the script to a separate file precisely for this issue. Before v6 it was even called somewhat confusingly inline.bundle.js.

  4. peterbe commented on Oct 5, 2018

    @peterbe

    This is a slightly off-topic comment but I do wonder if it's even a good thing to inline the JavaScript at all in the HTTP2 era. It makes it impossible to make the JS code async and if the .js file is served on the same domain there is extremely little overhead to serving it as a file over HTTP2. The HTML can be parsed sooner because it's smaller which means it can get started, in parallel, to download (and start parsing) other assets. Would love to see a simple option to disable all inlining.

    Also, because #5144 is now closed I'll post it here: If others are struggling with busted Content Security Polices as of this recent change, you might get some inspirational ideas from this blog post: https://www.peterbe.com/plog/inline-scripts-in-create-react-app-2.0-and-csp-nonces

  5. edmorley commented on Oct 5, 2018

    @edmorley

    If others are struggling with busted Content Security Polices as of this recent change, you might get some inspirational ideas from this blog post:

    I could be wrong, but I believe that using a static nonce-<hash> like that is insecure (the spec says it must be unique each time). The generated hash needs to be put into a '<hash-algorithm>-<base64-value>' attribute instead. See:
    #5144 (comment)
    https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Sources

  6. peterbe commented on Oct 5, 2018

    @peterbe

    @edmorley You're right. I rushed that. Fixed my blog post now.

    But also, I changed my mind about bothering anyway. I took @PerfectPixel 's heed and wrote a script that "uninlines" the inline script tag.
    Now my index.html is 1420 bytes (676 gzipped) instead of 2496 bytes (1194 gzipped). It's so small that it's really hard to measure any big difference and the context is a fun little side-project but I wanted to use this to put thoughts into code.

  7. 0xdeafcafe commented on Oct 8, 2018

    @0xdeafcafe
    Contributor

    Thanks @peterbe, that looks quite helpful. Although it seems quite insane the hoops you have to jump through just to get back to where we were with CRA1. This change only encourages a less secure web - with less experienced/lazy devs most likely to just turn off CSP or allow unsafe-inline...

  8. peterbe commented on Oct 8, 2018

    @peterbe

    @Timer Where should we redirect our issues about A) an option to disable inlining small scripts and B) lobby to have that option "on" by default.

  9. Timer commented on Oct 8, 2018

    @Timer
    ContributorAuthor

    This issue, @peterbe.

  10. gaearon commented on Oct 8, 2018

    @gaearon
    Contributor

    I say let’s add a way to opt out. #5309 (comment)

    I still think the majority would be better served by the inlined script though.

  11. Timer commented on Oct 8, 2018

    @Timer
    ContributorAuthor

    We can close this if we're going to add an environment variable.
    edit: actually it might still be useful, tagged for later milestone

  12. reopened this on Oct 8, 2018
  13. modified the milestones: 2.0.5, 2.x on Oct 8, 2018
  14. modified the milestones: 2.x, 3.x on Mar 10, 2019
  15. jvatic commented on Jan 24, 2020

    @jvatic

    Any update on this? I'd like to see the webpack config for inlining the runtime chunk be opt-in. IMHO having this inlined is a premature optimization at the cost of easily serving the app with a sane CSP.

    The only other option to prevent having unsafe-inline in the CSP, besides ejecting and removing this portion of the config, is to add a cryptographic hash of each runtime script to the CSP (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Sources). Neither option seems to be a sane default.

  16. kddnewton commented on Jan 24, 2020

    @kddnewton

    @jvatic you can opt out with INLINE_RUNTIME_CHUNK=false in your .env.

  17. dimaqq commented on May 12, 2020

    @dimaqq

    Or add a config override, like draft.plugins = draft.plugins.filter(p => p.constructor.name !== "InlineChunkHtmlPlugin");

  18. caub commented on Dec 22, 2021

    @caub

    a little ugly, but it's probably fine to do:

    function serveApp(res) {
      const nonce = crypto.randomBytes(16).toString('base64');
      res.set({
        'Content-Security-Policy': `....; script-src 'nonce-${nonce}'`,
        // ..
      });
      return fs.promises.readFile(path.join(__dirname, 'static', 'myapp', 'index.html'), 'utf8')
        .then(indexHtml => res.send(indexHtml.replace(/<script>/g, `<script nonce="${nonce}">`)));
    }

    (coming from #5144 nonce issue)

    edit: actually using INLINE_RUNTIME_CHUNK=false is better (seen it in https://deploy-preview-27627--material-ui.netlify.app/guides/content-security-policy/#create-react-app-cra)

  19. caub commented on Feb 24, 2022

    @caub

    Re #5144 the problem is not only for the runtime script, but also for lazy-loaded scripts, what can we do with react-scripts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions