[Bug]: Inline scripts in built `index.html` cause CSP errors

[drzamich]

Describe the bug

In the most recent (7.4.6) version of Storybook (but also older ones), when you build, the generated index.html contains an inline JS script that loads multiple modules from the sb-addons directory like this:

<script type="module">
      import './sb-manager/runtime.js';
      import './sb-addons/links-0/manager-bundle.js';
      ...
</script>

Because of those inline scripts you cannot deploy the built Storybook to servers which disallow inline scripts - those without the 'unsafe-eval' keyword in the script-src directive of the Content-Security-Policy header. In such a case the following errors are printed to the console

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src-elem 'self' (...) 
Either the 'unsafe-inline' keyword, a hash ('sha256-RhcCYMSY2FJ3z4CfT91f0yzMRMOnSRU4Enu8927xA3E='), or a nonce ('nonce-...') is required to enable inline execution.

Having 'unsafe-eval' turned on is dangerous from the security perspective. Instead of importing modules in an inline script, the code should be extracted to a separate file and loaded via a path.

To Reproduce

1. Set up a new Storybook project with npx storybook@latest init
2. Run npm run build-storybook
3. Inspect the file in storybook-static/index.html
4. Notice the inline script at the bottom of the file

System

No response

Additional context

No response

[Himanshu-370]

I would like to give a try, Can I be assigned? @drzamich

[drzamich]

@Himanshu-370 sure, go ahead 😃

[Himanshu-370]

@drzamich Can I get the community discord link? The link on the readme says invalid.

[drzamich]

@Himanshu-370 I never used it before but for me the following link worked: https://discord.com/invite/storybook

[miszol1]

@drzamich You can use hash printed in console like this: script-src 'self' 'sha256-...' This prevents CSP error to show because this inline script always has the same code inside.

You will also find this inline script which contains bundles with auto generated IDs in names. The solution here is to remove IDs from bundle names so the inline scripts is not changed every build. You can do it in webpack options: const config: StorybookConfig = { ... webpackFinal: async (config) => { config.output.filename = '[name].iframe.bundle.js'; return config; }, };

[drzamich]

@miszol1 thanks, I know making CSP headers less strict would probably solve the issue but sadly it's not an option for me. I don't know what's the advantage of having this script inline. For me it seems that changing it to be exported could only make things better.

[miszol1]

@drzamich probably you are right about changing it to js file but using sha hashes in CSP headers does not make them less strict. Sha hash is only valid for exact code in inline script tag

[gavinbarron]

@drzamich You can use hash printed in console like this: script-src 'self' 'sha256-...' This prevents CSP error to show because this inline script always has the same code inside.

You will also find this inline script which contains bundles with auto generated IDs in names. The solution here is to remove IDs from bundle names so the inline scripts is not changed every build. You can do it in webpack options: const config: StorybookConfig = { ... webpackFinal: async (config) => { config.output.filename = '[name].iframe.bundle.js'; return config; }, };

I think that removing the file hashes is a bad idea, this could cause an issue whereby an browser continues to use a locally cached version of the file.

[gavinbarron]

@drzamich we already post process our index.html file to add some meta tags and fix the title tag for SEO so I wound up adding a step add the hashes.

The core function is this:

function readHashesForMatch(htmlDocument, match, endMatch, hashes) {
  let start = 0;
  while (htmlDocument.indexOf(match, start) > -1) {
    start = htmlDocument.indexOf(match, start);
    const end = htmlDocument.indexOf(endMatch, start);
    const script = htmlDocument.substring(start + match.length, end);
    const hash = Buffer.from(createHash('sha256').update(script).digest()).toString('base64');
    hashes.push(`'sha256-${hash}'`);
    start = end;
  }
}

Full file here: https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/main/.storybook/post-process-index-file.js

That function can be used for both inline scripts and styles, hench why the start/end match strings are passed in.

[ndelangen]

Here's the code generating this HTML: https://github.com/storybookjs/storybook/blob/2cf017f9dfb61b1f017d97831b04a9154b3053a8/code/builders/builder-webpack5/templates/preview.ejs#L46-L52 (at least for webpack)

I don't know what's the advantage of having this script inline. For me it seems that changing it to be exported could only make things better.

The reason we do it this way, is that this is the simplest way to:

• get these files to be loaded by the browser in parallel
• get the module initialization script to run in sequence

Yes, it's technically possible for us to write this output into yet-another-file, and reference that file instead ( I think that's what you mean with "to be exported"), but from storybook's perspective that is yet another level of indirection we'd rather not maintain. Considering storybook already has complete access to the html and javascript execution/generation, it does not feel like any added security changing the code to an external reference.

Additionally: an extra file would not only add a maintenance burden, but would also add yet-another-request for the browser.

I'm open to hearing why another approach would "make things better".

[mkaisercross]

@ndelangen I wanted to provide our use case which demonstrates where this issue does have a security impact.

Our use case is for a CI application built around serving static storybook builds. The application would provide user authentication among other features including a frontend for navigating between different storybooks. Since the static storybook files would be served alongside code for the application code both must share the same CSP. Since the CSP is shared this opens up the whole app to inline scripts not just the storybook app.

Considering storybook already has complete access to the html and JS execution/generation, it does not feel like any added security changing the code to an external reference.

I don't see how storybook has complete access to HTML/JS execution. At least for the storybook version I am using (8.2.9) unsafe-eval is not a requirement. Removing 'unsafe-inline' may not seem like a big security improvement in the case of running storybook locally but for developing enterprise grade storybook CI solutions it is a meaningful improvement. Also libraries that require "script-src 'unsafe-inline'" may not themselves have a way to exploit the vulnerability (such as reflected XSS) but the problem is when they are included in a larger app that has more code with potential for XSS and with unsafe-inline that app now cannot protect itself as easily from reflected inline scripts.

Also in enterprises these products often have to go through security reviews including SAST scans and if nothing else removing unsafe-inline makes storybook adoption easier for companies.