search

storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.75k stars 9.33k forks source link

[Bug]: Storybook is reporting an "unsafe eval" warning for its own dependency #22859

Open kaiyoma opened 1 year ago

kaiyoma commented 1 year ago

Describe the bug

When building Storybook, I see these messages:

Use of eval in "node_modules/.pnpm/telejson@7.1.0/node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.
Use of eval in "node_modules/.pnpm/telejson@7.1.0/node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.

I've never heard of this package, and when I run pnpm why telejson it tells me that Storybook itself is the only reason this dependency exists.

To Reproduce

No response

System

Environment Info:

  System:
    OS: Windows 10 10.0.22621
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
  Binaries:
    Node: 18.16.0 - C:\Program Files\Node.js\node.EXE
    npm: 9.6.2 - C:\Program Files\Node.js\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.1778.0), Chromium (113.0.1774.57)
  npmPackages:
    @storybook/addon-actions: 7.0.17 => 7.0.17
    @storybook/addon-controls: 7.0.17 => 7.0.17
    @storybook/addon-docs: 7.0.17 => 7.0.17
    @storybook/addons: 7.0.17 => 7.0.17
    @storybook/api: 7.0.17 => 7.0.17
    @storybook/cli: 7.0.17 => 7.0.17
    @storybook/components: 7.0.17 => 7.0.17
    @storybook/core-events: 7.0.17 => 7.0.17
    @storybook/react: 7.0.17 => 7.0.17
    @storybook/react-vite: 7.0.17 => 7.0.17
    @storybook/theming: 7.0.17 => 7.0.17

Additional context

$ pnpm why telejson
Legend: production dependency, optional only, dev only

arista-cloudvision@16.0.0-alpha C:\Users\kgetz\Work\event-viewer

devDependencies:
@storybook/addon-actions 7.0.17
├─┬ @storybook/manager-api 7.0.17
│ └── telejson 7.1.0
├─┬ @storybook/preview-api 7.0.17
│ └─┬ @storybook/channel-postmessage 7.0.17
│   └── telejson 7.1.0
└── telejson 7.1.0
@storybook/addon-controls 7.0.17
├─┬ @storybook/blocks 7.0.17
│ ├─┬ @storybook/docs-tools 7.0.17
│ │ └─┬ @storybook/preview-api 7.0.17
│ │   └─┬ @storybook/channel-postmessage 7.0.17
│ │     └── telejson 7.1.0
│ ├─┬ @storybook/manager-api 7.0.17
│ │ └── telejson 7.1.0
│ ├─┬ @storybook/preview-api 7.0.17
│ │ └─┬ @storybook/channel-postmessage 7.0.17
│ │   └── telejson 7.1.0
│ └── telejson 7.1.0
├─┬ @storybook/manager-api 7.0.17
│ └── telejson 7.1.0
└─┬ @storybook/preview-api 7.0.17
  └─┬ @storybook/channel-postmessage 7.0.17
    └── telejson 7.1.0
@storybook/addon-docs 7.0.17
├─┬ @storybook/blocks 7.0.17
│ ├─┬ @storybook/docs-tools 7.0.17
│ │ └─┬ @storybook/preview-api 7.0.17
│ │   └─┬ @storybook/channel-postmessage 7.0.17
│ │     └── telejson 7.1.0
│ ├─┬ @storybook/manager-api 7.0.17
│ │ └── telejson 7.1.0
│ ├─┬ @storybook/preview-api 7.0.17
│ │ └─┬ @storybook/channel-postmessage 7.0.17
│ │   └── telejson 7.1.0
│ └── telejson 7.1.0
└─┬ @storybook/preview-api 7.0.17
  └─┬ @storybook/channel-postmessage 7.0.17
    └── telejson 7.1.0
@storybook/addons 7.0.17
├─┬ @storybook/manager-api 7.0.17
│ └── telejson 7.1.0
└─┬ @storybook/preview-api 7.0.17
  └─┬ @storybook/channel-postmessage 7.0.17
    └── telejson 7.1.0
@storybook/api 7.0.17
└─┬ @storybook/manager-api 7.0.17
  └── telejson 7.1.0
@storybook/cli 7.0.17
└─┬ @storybook/core-server 7.0.17
  ├─┬ @storybook/preview-api 7.0.17
  │ └─┬ @storybook/channel-postmessage 7.0.17
  │   └── telejson 7.1.0
  └── telejson 7.1.0
@storybook/react 7.0.17
├─┬ @storybook/core-client 7.0.17
│ └─┬ @storybook/preview-api 7.0.17
│   └─┬ @storybook/channel-postmessage 7.0.17
│     └── telejson 7.1.0
├─┬ @storybook/docs-tools 7.0.17
│ └─┬ @storybook/preview-api 7.0.17
│   └─┬ @storybook/channel-postmessage 7.0.17
│     └── telejson 7.1.0
└─┬ @storybook/preview-api 7.0.17
  └─┬ @storybook/channel-postmessage 7.0.17
    └── telejson 7.1.0
@storybook/react-vite 7.0.17
├─┬ @storybook/builder-vite 7.0.17
│ ├─┬ @storybook/channel-postmessage 7.0.17
│ │ └── telejson 7.1.0
│ ├─┬ @storybook/channel-websocket 7.0.17
│ │ └── telejson 7.1.0
│ └─┬ @storybook/preview-api 7.0.17
│   └─┬ @storybook/channel-postmessage 7.0.17
│     └── telejson 7.1.0
└─┬ @storybook/react 7.0.17
  ├─┬ @storybook/core-client 7.0.17
  │ └─┬ @storybook/preview-api 7.0.17
  │   └─┬ @storybook/channel-postmessage 7.0.17
  │     └── telejson 7.1.0
  ├─┬ @storybook/docs-tools 7.0.17
  │ └─┬ @storybook/preview-api 7.0.17
  │   └─┬ @storybook/channel-postmessage 7.0.17
  │     └── telejson 7.1.0
  └─┬ @storybook/preview-api 7.0.17
    └─┬ @storybook/channel-postmessage 7.0.17
      └── telejson 7.1.0
storybook 7.0.17
└─┬ @storybook/cli 7.0.17
  └─┬ @storybook/core-server 7.0.17
    ├─┬ @storybook/preview-api 7.0.17
    │ └─┬ @storybook/channel-postmessage 7.0.17
    │   └── telejson 7.1.0
    └── telejson 7.1.0
shilman commented 1 year ago

@ndelangen can we get rid of this in 8.0? even if it's not a real security issue, it looks like one and i don't think it's useful to serialize functions over the channel anyway.

ndelangen commented 1 year ago

correct, I'll remove this functionality from telejson in a new major.

We should plan that work as part of storybook 8.0. It won't be much, but there's little point (or in fact counter productive) in doing in right now, when SB 8.0 is still far away.

SavkaTaras commented 10 months ago

Hello @shilman @ndelangen , I hope you are doing well.

Is it possible to remove this from version 7 as well?

Thank you, Taras

vanessayuenn commented 10 months ago

The default channelOptions in telejson.stringify has been set to { allowFunction: false } in #25564 and this fix will go out with 8.0. Since this is a breaking change, we cannot backport it to 7.x.

We are leaving this issue open as a reminder to remove the unsafe eval call from @storybook/telejson properly in 9.0.

paul-vd commented 4 months ago

It seems like this issue has not been resolved in 8.1.11

../../node_modules/.pnpm/telejson@7.2.0/node_modules/telejson/dist/index.mjs (1416:18): Use of eval in "../../node_modules/.pnpm/telejson@7.2.0/node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.
anewstead commented 4 months ago

seeing in 8.2.5 Also coming from manager-api

node_modules/.pnpm/telejson@7.2.0/node_modules/telejson/dist/index.mjs (1413:15): Use of eval in "node_modules/.pnpm/telejson@7.2.0/node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.

node_modules/.pnpm/telejson@7.2.0/node_modules/telejson/dist/index.mjs (1416:18): Use of eval in "node_modules/.pnpm/telejson@7.2.0/node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.

node_modules/.pnpm/@storybook+core@8.2.5/node_modules/@storybook/core/dist/manager-api/index.js (4764:15): Use of eval in "node_modules/.pnpm/@storybook+core@8.2.5/node_modules/@storybook/core/dist/manager-api/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.

node_modules/.pnpm/@storybook+core@8.2.5/node_modules/@storybook/core/dist/manager-api/index.js (4766:16): Use of eval in "node_modules/.pnpm/@storybook+core@8.2.5/node_modules/@storybook/core/dist/manager-api/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.

shilman commented 4 months ago

By default this code is not executed anymore in Storybook. We had a chance to remove it in 8.0, but unfortunately this got lost in the shuffle. We will definitely remove it in 9.0. Apologies in the meantime.