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

CSP: Storybook 6 uses unsafe-eval #12364

Closed
ltroller opened this issue Sep 3, 2020 · 51 comments
Closed

CSP: Storybook 6 uses unsafe-eval #12364

ltroller opened this issue Sep 3, 2020 · 51 comments

Comments

@ltroller
Copy link

ltroller commented Sep 3, 2020

Description

A react component library I maintain for my company deploys a storybook to our internal Github Pages (enterprise). These have headers that set a Content Security Policy (CSP) that does not include unsafe-eval. These cannot be overwritten by <meta .../> tags as the headers have a stricter policy.

After upgrading Storybook 5.3.9 to 6.0.21, the storybook will not load any more:

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' 'unsafe-inline'".

Uncaught TypeError: __webpack_require__(...) is not a function

Expanded screenshot below.

The problem is reproduce-able locally with a minimal config, but the original issue is with a built version on a remote server.

To Reproduce

Setup minimal storybook react

  1. mkdir test-sb && cd test-sb
  2. yarn init
  3. yarn add react react-dom
  4. npx sb init

Emulate CSP & run

  1. echo "<meta http-equiv=\"Content-Security-Policy\" content=\"default-src 'self' ; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' *; media-src 'self' ; frame-src 'self';\" />" > .storybook/manager-head.html
  2. yarn storybook

Expected behavior

As previous versions, Storybook should avoid leveraging unsafe-eval

Screenshot

Screen Shot 2020-09-02 at 5 26 28 PM

System

Local setup

  OS: macOS 10.15.6
  CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
Binaries:
  Node: 14.4.0 - ~/.nvm/versions/node/v14.4.0/bin/node
  Yarn: 1.21.1 - ~/.yarn/bin/yarn
  npm: 6.14.5 - ~/.nvm/versions/node/v14.4.0/bin/npm
Browsers:
  Chrome: 85.0.4183.83
  Safari: 13.1.2
npmPackages:
  @storybook/addon-actions: ^6.0.21 => 6.0.21 
  @storybook/addon-essentials: ^6.0.21 => 6.0.21 
  @storybook/addon-links: ^6.0.21 => 6.0.21 
  @storybook/react: ^6.0.21 => 6.0.21 ```

@khanzzirfan
Copy link

khanzzirfan commented Sep 3, 2020

Same issue I'm facing as well. I tried building storybook using below command and here are more details

build-storybook -s ./assets -c "${StoryBookConfigFolder}" -o build/docs --no-dll --quiet

image

Its looks like new Function syntax is causing the problem. below screenshot is from vendor bundle.js file

image

@khanzzirfan
Copy link

Can anyone offer help here ? or point me the direction to fix the csp error.

Its unfortunate. we have to rollback to storybook v4. just for the CSP issues.

@shilman
Copy link
Member

shilman commented Sep 12, 2020

@khanzzirfan this is the offending code:

https://github.com/storybookjs/telejson/blob/master/src/index.ts#L269

@ndelangen can you help guide this?

@khanzzirfan
Copy link

Also realised there is use of eval as well.
https://github.com/storybookjs/telejson/blob/master/src/index.ts#L288

@johnhunter
Copy link
Contributor

Looks similar to this issue storybookjs/telejson#18
It was closed but not completely resolved.

@khanzzirfan
Copy link

The short answer from @johnhunter is there is no way to fix this issue and to take exemption from CSP policies. @shilman any other suggestions you rekon please?

@intuitivecat
Copy link

intuitivecat commented Sep 21, 2020

we also cannot host SB5+ externally due to this,

@ndelangen mentioned this in the other issue report

It only depends on that (reviver) for a very select set of features,

my hope was to be able to bypass the feature as the CSP violation is at run-time and not in a code scanner. however I could not see a path to do that in the code.

@shilman
Copy link
Member

shilman commented Sep 22, 2020

If there were an option flag to disable this feature, would this satisfy your CSP?

@intuitivecat
Copy link

I cannot speak for others as the errors above vary , but my error stack I think it would.

react_devtools_backend.js:2273 EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' 'unsafe-inline' ... redacted ...

    at new Function (<anonymous>)
    at Array.revive (vendors~main.9664a462e85d088dc937.bundle.js:123)
    at JSON.parse (<anonymous>)
    at parse (vendors~main.9664a462e85d088dc937.bundle.js:123)
    at e.value (vendors~main.9664a462e85d088dc937.bundle.js:123)

I should be able to help test a proposed change, even without a release as I should be able to build storybook_static local and deploy.

@khanzzirfan
Copy link

khanzzirfan commented Sep 23, 2020

Somehow we got csp whitelisted for new Function and eval for internally hosted storybook as workaround for now. but the exemptions is only for few months and have to renewed again with ISG department.

@ltroller
Copy link
Author

Thanks to all for looking into this!
It's been a couple months now, so I'll go ahead and ask:
Are there any updates on the situation?

@stale
Copy link

stale bot commented Dec 25, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Dec 25, 2020
@digicide
Copy link

digicide commented Feb 8, 2021

This is still a dealbreaker for many of us. It looks like everyone at my company is just staying on version 5 so they can continue publishing to github pages. I am very interested in the aforementioned option flag which disables the offending feature.

@stale stale bot removed the inactive label Feb 8, 2021
@shilman
Copy link
Member

shilman commented Feb 16, 2021

If you want this fixed, please upvote by adding a 👍 to the issue description. We use this to help prioritize!

@shilman
Copy link
Member

shilman commented Sep 22, 2021

Problem summary

The telejson library that serializes objects between the user's "preview" UI and the surrounding "manager" UI uses eval to stringify & parse functions, which causes the CSP problem.

Proposed sketch of a solution

  1. Telejson already has an option allowFunction that throws away functions instead of serializing them. This option is inspected in stringify. It should also be inspected in parse.

  2. Furthermore, there should be a way to globally configure Storybook in .storybook/main.js. Proposal:

module.exports = {
  core: {
    serializeFunctions: false,
  }
}
  1. Finally, the code in createChannel should set allowFunction globally for the channel so that every request filters out functions.

@johnhunter
Copy link
Contributor

Proposed sketch of a solution

Sounds good @shilman, global configuration makes sense since its only a problem for Storybooks hosted with CSP. Thanks for looking at this.

@ndelangen
Copy link
Member

Yeah that proposal makes sense to me!

@shilman
Copy link
Member

shilman commented Oct 21, 2021

ZOMG!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-beta.17 containing PR #16415 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Oct 21, 2021
@gjou-tlnd
Copy link

gjou-tlnd commented Nov 23, 2021

Hi there,
Looks like there is an implementation issue in the fix:
The allowFunction value provided in global.CHANNEL_OPTIONS is not taken into account but overriden by the spread default.

const {
target,
// telejson options
allowRegExp,
allowFunction = true,
allowSymbol,
allowDate,
allowUndefined,
allowClass,
maxDepth = 25,
space,
lazyEval,
} = options || {};
const c = Object.fromEntries(
Object.entries({
allowRegExp,
allowFunction,
allowSymbol,
allowDate,
allowUndefined,
allowClass,
maxDepth,
space,
lazyEval,
}).filter(([k, v]) => typeof v !== 'undefined')
);
const stringifyOptions = { ...(global.CHANNEL_OPTIONS || {}), ...c };

I can provide a fix PR for this.

@ndelangen ndelangen reopened this Nov 23, 2021
@shilman
Copy link
Member

shilman commented Mar 10, 2022

@phobetron yes, it should def be in there. do you have a repro we can look at?

@phobetron
Copy link

@shilman No, nothing public, and we also can't publish to a public environment until we can get it to work without adding the unsafe-eval CSP header.

Here's the main.js file:

module.exports = {
  async webpackFinal(config) {
    config.resolve.roots = [assetsPath, 'node_modules']

    return config
  },
  staticDirs: [assetsPath],
  stories: [path.resolve(storyPath, '**/*.stories.mdx'), path.resolve(storyPath, '**/*.stories.@(js|jsx|ts|tsx)')],
  addons: [
    '@storybook/preset-scss',
    '@storybook/addon-essentials',
    '@storybook/addon-links',
    'storybook-addon-designs'
  ],
  framework: '@storybook/vue3',
  core: {
    serializeFunctions: false,
    builder: 'webpack5'
  }
}

Here's a list of our dependencies:

    "@babel/core": "^7.17.5",
    "@storybook/addon-actions": "^6.4.19",
    "@storybook/addon-essentials": "^6.4.19",
    "@storybook/addon-links": "^6.4.19",
    "@storybook/builder-webpack5": "^6.4.19",
    "@storybook/manager-webpack5": "^6.4.19",
    "@storybook/preset-scss": "^1.0.3",
    "@storybook/vue3": "^6.4.19",
    "@typescript-eslint/eslint-plugin": "^4.15.1",
    "@typescript-eslint/parser": "^4.15.1",
    "@vue/eslint-config-prettier": "^6.0.0",
    "@vue/eslint-config-typescript": "^7.0.0",
    "babel-loader": "^8.2.3",
    "sass": "^1.35.1",
    "sass-loader": "^10.2.0",
    "storybook-addon-designs": "^6.2.1",
    "typescript": "^4.1.5",
    "vue": "^3.2.25",
    "vue-loader": "^16.8.3",
    "webpack": "^5.70.0"

Our Storybook instance is currently new and mostly empty, so it's safe to say there's no offending code on our end.

@shilman
Copy link
Member

shilman commented Mar 11, 2022

@phobetron Your main.js doesn't disable function serialization which is what we added to fix the problem. Can you share your updated main.js that doesn't work?

@phobetron
Copy link

@shilman serializeFunctions: false from your proposal is in the code I pasted, inside core, exactly as in your proposal. Is there an undocumented flag that is different from your proposal?

@phobetron
Copy link

phobetron commented Mar 15, 2022

@shilman Can we expect any update on this? Is there any documentation?

@shilman
Copy link
Member

shilman commented Mar 15, 2022

hi @phobetron sorry i missed the previous notification. please try the following to disable function serialization:

https://github.com/storybookjs/storybook/blob/next/examples/react-ts/.storybook/main.ts#L26

@phobetron
Copy link

Thanks @shilman!

For anyone landing from a search result, here's the fix:

// .storybook/main.js
module.exports = {
  // ...
  core: {
    // ...
    channelOptions: { allowFunction: false, maxDepth: 10 }
  }
}

@asarosi
Copy link

asarosi commented Mar 26, 2022

Hi @shilman , @phobetron ,

I tried to add the flag, but I'm still getting the same issue.
image

This is my main.ts
image
do you have any idea what did I miss or what can be the cause of the issue?

@vinay3206
Copy link

Is there any update on the issue ? I am also facing the same issue after updating the main.ts.
@asarosi, @shilman , @phobetron

@shilman
Copy link
Member

shilman commented Aug 29, 2022

@vinay3206 what version of storybook are you using?

@vinay3206
Copy link

@shilman

@storybook/react v6.5.10

"@react-theming/storybook-addon": "^1.1.7",
"@storybook/addon-docs": "^6.5.10",
"@storybook/addon-essentials": "^6.5.10",
"@storybook/addon-links": "^6.5.10",
"@storybook/addons": "^6.5.10",
"@storybook/builder-webpack5": "^6.5.10",
"@storybook/manager-webpack5": "^6.5.10",
"@storybook/react": "^6.5.10",
"@storybook/theming": "^6.5.10",

@vinay3206
Copy link

vinay3206 commented Aug 29, 2022

@shilman

main.js looks like this

module.exports = {
  stories: ["../src/**/*.stories.mdx", "../src/**/*.stories.@(js|jsx|ts|tsx)"],
  addons: [
    "@storybook/addon-links",
    "@storybook/addon-essentials",
    "@react-theming/storybook-addon",
  ],
  core: {
    builder: { name: 'webpack5' },
    channelOptions: { allowFunction: false, maxDepth: 10 },
  }
};

@clesgourgues
Copy link

@vinay3206 did you manage to fix your issue ? I have the same on my storybook deploy on netlify, the fix didn't work neither for me.

@vinay3206
Copy link

@clesgourgues , nope not able to solve it. We updated our CSP policy to allow it for dev env.

@markusbkk
Copy link

Same issue here.

Strangely, I don't run into the same issue with my auto generated, Stencil component based stories, using the generator script (automatedStories.js) from theksquaregroup

@shilman
Copy link
Member

shilman commented Jan 11, 2023

Do any of you have a reproduction you can share? If there's a bug here somewhere we'd absolutely love to get it fixed.

@markusbkk
Copy link

Do any of you have a reproduction you can share? If there's a bug here somewhere we'd absolutely love to get it fixed.

I just upgraded to next. It's possible the issue has already been fixed in Storybook 7. Will report back as soon as I've dealt with the typical node_modules issues. Sigh

@markusbkk
Copy link

Do any of you have a reproduction you can share? If there's a bug here somewhere we'd absolutely love to get it fixed.

I just upgraded to next. It's possible the issue has already been fixed in Storybook 7. Will report back as soon as I've dealt with the typical node_modules issues. Sigh

My builds keep failing with the message build-storybook: not found, even after multiple re-installs of all node_modules.

Did things move in Storybook 7, @shilman ?

@shilman
Copy link
Member

shilman commented Jan 11, 2023

@markusbkk

Migration guide: https://storybook.js.org/migration-guides/7.0

@markusbkk
Copy link

@markusbkk

Migration guide: https://storybook.js.org/migration-guides/7.0

Thanks. That gets me one step closer.

Now it's crashing at Building manager.. though.

TypeError: path2.replaceAll is not a function
ERR! at sanitizeBase (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:3195)
ERR! at (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:3891)
ERR! at Array.map ()
ERR! at wrapManagerEntries (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:3528)
ERR! at getConfig (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:6097)
ERR! at async Promise.all (index 1)
ERR! at getData (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:4706)
ERR! at builder (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:8636)
ERR! at Module.build (/home/user/stencil-test-angular/node_modules/@storybook/builder-manager/dist/index.js:1:10058)
ERR! at async buildStaticStandalone (/home/user/stencil-test-angular/node_modules/@storybook/core-server/dist/index.js:16:62)

@ndelangen
Copy link
Member

What version of node are you running in?

Storybook 7 requires node 16 or higher.

@markusbkk
Copy link

What version of node are you running in?

Storybook 7 requires node 16 or higher.

I'm running 14.20.1 thanks to some stupid legacy project at work that can't be easily upgraded.
Thanks. That explains it.

@clesgourgues
Copy link

@shilman yes, your help would be much appreciated ! we are currently facing the CSP issue, after adding the Mock service worker addon. Our app is a CRA, with a craco config. The issue pops on the deployed version of our Storybook.

Our main.js :

module.exports = {
  stories: ["../src/**/*.stories.mdx", "../src/**/*.stories.@(ts|tsx)"],
  addons: [
    "@storybook/addon-links",
    "@storybook/addon-essentials",
    "@storybook/preset-create-react-app",
    "storybook-react-i18next",
    "storybook-addon-designs",
  ],
  core: {
    builder: "webpack5",
    channelOptions: { allowFunction: false, maxDepth: 10, lazyEval: false },
    disableTelemetry: true,
  },
  staticDirs: ["../public"],
  typescript: {
    check: false,
    checkOptions: {},
    reactDocgen: "react-docgen-typescript",
    reactDocgenTypescriptOptions: {
      allowSyntheticDefaultImports: false,
      esModuleInterop: false,
      propFilter: (prop, component) => {
        if (prop.declarations !== undefined && prop.declarations.length > 0) {
          const hasPropAdditionalDescription = prop.declarations.find((declaration) => {
            // return !declaration.fileName.includes("node_modules");
            // Prevent props from HTMLDivElement etc.
            return !declaration.fileName.includes("@types/react");
          });
          return Boolean(hasPropAdditionalDescription);
        }

        return true;
      },
      shouldExtractLiteralValuesFromEnum: true,
      shouldRemoveUndefinedFromOptional: true,
    },
  },
};

our dev dependencies :

"devDependencies": {
    "@apollo/react-testing": "4.0.0",
    "@craco/craco": "6.4.5",
    "@faker-js/faker": "7.6.0",
    "@graphql-codegen/cli": "2.13.11",
    "@graphql-codegen/introspection": "2.2.1",
    "@graphql-codegen/schema-ast": "2.5.1",
    "@graphql-codegen/typescript": "2.8.1",
    "@graphql-codegen/typescript-apollo-client-helpers": "2.2.6",
    "@graphql-codegen/typescript-operations": "2.5.6",
    "@graphql-codegen/typescript-react-apollo": "3.3.6",
    "@storybook/addon-actions": "6.5.13",
    "@storybook/addon-essentials": "6.5.13",
    "@storybook/addon-links": "6.5.13",
    "@storybook/builder-webpack5": "6.5.13",
    "@storybook/cli": "6.5.13",
    "@storybook/manager-webpack5": "6.5.13",
    "@storybook/node-logger": "6.5.13",
    "@storybook/preset-create-react-app": "4.1.2",
    "@storybook/react": "6.5.13",
    "@storybook/testing-react": "1.3.0",
    "@testing-library/jest-dom": "5.16.5",
    "@testing-library/react": "13.4.0",
    "@types/history": "5.0.0",
    "@types/jest": "29.2.2",
    "@types/jquery": "3.5.14",
    "@types/lodash": "4.14.188",
    "@types/mapbox-gl": "2.6.1",
    "@types/node": "18.11.9",
    "@types/preval.macro": "3.0.0",
    "@types/qs": "6.9.7",
    "@types/rails__actioncable": "6.1.6",
    "@types/react": "18.0.25",
    "@types/react-dom": "18.0.8",
    "@types/react-reconciler": "0.28.0",
    "@types/uuid": "8.3.4",
    "eslint": "8.27.0",
    "eslint-config-prettier": "8.5.0",
    "eslint-plugin-i18n-json": "4.0.0",
    "eslint-plugin-import": "2.26.0",
    "eslint-plugin-jsx-expressions": "1.3.1",
    "eslint-plugin-sort-keys-fix": "1.1.2",
    "eslint-plugin-typescript-sort-keys": "2.1.0",
    "factory.ts": "1.3.0",
    "http-proxy-middleware": "2.0.6",
    "jest-canvas-mock": "2.4.0",
    "msw": "0.49.2",
    "msw-storybook-addon": "1.6.3",
    "prettier": "2.7.1",
    "pretty-quick": "3.1.3",
    "react-scripts": "5.0.1",
    "source-map-explorer": "2.5.3",
    "storybook": "6.5.13",
    "storybook-addon-designs": "6.3.1",
    "storybook-react-i18next": "1.1.2",
    "svgo": "3.0.0",
    "ts-jest": "29.0.3",
    "type-coverage": "2.22.0",
    "webpack": "5.74.0"
  },

Before adding msw, we already had several warnings :
Capture d’écran 2023-01-12 à 10 08 44

And then this error :
Capture d’écran 2023-01-12 à 10 08 51

@ndelangen
Copy link
Member

@clesgourgues looks like you'll have to change your CSP, to allow script tags with inline script, this is something we started using in storybook 7.0.

@clesgourgues
Copy link

thanks @ndelangen does it mean we'll have to do it anyway if we upgrade to V7 ?

@markusbkk
Copy link

@clesgourgues looks like you'll have to change your CSP, to allow script tags with inline script, this is something we started using in storybook 7.0.

Yea. I just realized this too, after finally upgrading everything to 7.0 earlier today.

So upgrading didn't resolve the issue. It actually made it worse because now I'm presented with only a white screen and CSP errors inside the browser console.

@johnhunter
Copy link
Contributor

I remember with V6 TeleJSON was updated to make the eval technique opt in. I guess it is now necessary with v7.

The way we resolved this with v6 was to relax our CSP for our Storybook hosting. We could justify that as it was only available internally. For businesses that use Storybook for public facing sites this may be a deal-breaker (it will be flagged in security audits)

There is maybe a Storybook product consideration to be made. Does the value of using "unsafe-eval" techniques override the needs of some consumers of the product? Perhaps this is just something we have to live with for the convenience and great features. It might be worth making this clear in documentation so it is a known constraint when evaluating Storybook.

@ndelangen
Copy link
Member

Ow I might have not read the error message right @clesgourgues. You're running into the unsafe-eval problem? In Storybook 6.5?

That should be fixable by core.channelOptions.allowFunction = false, it's strange that it isn't.

Besides that, in storybook 7 we started using pure ESM in the browser including ESM in a script-tag contents, because that's a way to ensure a bunch of code is loaded and executed in the correct order.

I remember with V6 TeleJSON was updated to make the eval technique opt in. I guess it is now necessary with v7.

No? This should not be the case.. if it is.. that's a bug. Would you be able to create a reproduction?

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

No branches or pull requests