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

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

Open
kaiyoma opened this issue May 31, 2023 · 7 comments
Open

Comments

@kaiyoma
Copy link

kaiyoma commented May 31, 2023

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
Copy link
Member

shilman commented Jun 23, 2023

@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.

@shilman shilman added this to the 8.0 breaking changes milestone Jun 23, 2023
@ndelangen
Copy link
Member

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
Copy link

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

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

Thank you,
Taras

@vanessayuenn
Copy link
Contributor

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
Copy link

paul-vd commented Jul 6, 2024

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
Copy link

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
Copy link
Member

shilman commented Jul 21, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Empathy Backlog
Development

No branches or pull requests

8 participants