From b133b30466b8c22aab0c962fdfd115c04609281c Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Thu, 29 Feb 2024 18:06:19 +0100 Subject: [PATCH] fix(storybook-builder): define only necessary process.env globally --- .changeset/sweet-sloths-crash.md | 5 +++++ .../storybook-builder/src/generate-iframe-html.ts | 9 +++++++++ packages/storybook-builder/src/index.ts | 8 ++------ .../src/rollup-plugin-prebundle-modules.ts | 11 ++--------- .../storybook-builder/static/iframe-template.html | 2 ++ 5 files changed, 20 insertions(+), 15 deletions(-) create mode 100644 .changeset/sweet-sloths-crash.md diff --git a/.changeset/sweet-sloths-crash.md b/.changeset/sweet-sloths-crash.md new file mode 100644 index 000000000..8b1aa1297 --- /dev/null +++ b/.changeset/sweet-sloths-crash.md @@ -0,0 +1,5 @@ +--- +'@web/storybook-builder': patch +--- + +define only necessary process.env globally to improve security and decrease prebundling diff --git a/packages/storybook-builder/src/generate-iframe-html.ts b/packages/storybook-builder/src/generate-iframe-html.ts index a9798f7ff..84bdaa422 100644 --- a/packages/storybook-builder/src/generate-iframe-html.ts +++ b/packages/storybook-builder/src/generate-iframe-html.ts @@ -11,6 +11,7 @@ export async function generateIframeHtml(options: Options): Promise { 'utf-8', ); const { configType, features, presets, serverChannelUrl } = options; + const env = await presets.apply>('env'); const frameworkOptions = await presets.apply | null>('frameworkOptions'); const headHtmlSnippet = await presets.apply('previewHead'); const bodyHtmlSnippet = await presets.apply('previewBody'); @@ -26,6 +27,14 @@ export async function generateIframeHtml(options: Options): Promise { })); return ( iframeHtmlTemplate + .replace( + `'[PROCESS_ENV HERE]'`, + JSON.stringify({ + // IMPORTANT!!! + // please do not include the entire "env" to prevent bundling and deploying of sensitive data + NODE_ENV: env.NODE_ENV, + }), + ) .replace('[CONFIG_TYPE HERE]', configType || '') .replace('[LOGLEVEL HERE]', logLevel || '') .replace(`'[FRAMEWORK_OPTIONS HERE]'`, JSON.stringify(frameworkOptions)) diff --git a/packages/storybook-builder/src/index.ts b/packages/storybook-builder/src/index.ts index adac2d480..17ae5ae8b 100644 --- a/packages/storybook-builder/src/index.ts +++ b/packages/storybook-builder/src/index.ts @@ -60,8 +60,6 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s router.use('/sb-preview', express.static(previewDirOrigin, { immutable: true, maxAge: '5m' })); router.use(`/${PREBUNDLED_MODULES_DIR}`, express.static(resolve(`./${PREBUNDLED_MODULES_DIR}`))); - const env = await options.presets.apply>('env'); - const wdsStorybookConfig: DevServerConfig = { nodeResolve: true, plugins: [ @@ -74,7 +72,7 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s } }, }, - wdsPluginPrebundleModules(env), + wdsPluginPrebundleModules(), wdsPluginStorybookBuilder(options), wdsPluginExternalGlobals(globalsNameReferenceMap || globals), ], @@ -130,8 +128,6 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s }; export const build: WdsBuilder['build'] = async ({ startTime, options }) => { - const env = await options.presets.apply>('env'); - const rollupDefaultOutputOptions: OutputOptions = { dir: options.outputDir, }; @@ -146,7 +142,7 @@ export const build: WdsBuilder['build'] = async ({ startTime, options }) => { extractAssets: false, }), rollupPluginNodeResolve(), - rollupPluginPrebundleModules(env), + rollupPluginPrebundleModules(), rollupPluginStorybookBuilder(options), rollupPluginExternalGlobals(globalsNameReferenceMap || globals), ], diff --git a/packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts b/packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts index e8b133e1d..a0be1b418 100644 --- a/packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts +++ b/packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts @@ -1,4 +1,3 @@ -import { stringifyProcessEnvs } from '@storybook/core-common'; import { build } from 'esbuild'; import { join } from 'path'; import type { Plugin } from 'rollup'; @@ -6,7 +5,7 @@ import { getNodeModuleDir } from './get-node-module-dir.js'; export const PREBUNDLED_MODULES_DIR = 'node_modules/.prebundled_modules'; -export function rollupPluginPrebundleModules(env: Record): Plugin { +export function rollupPluginPrebundleModules(): Plugin { const modulePaths: Record = {}; return { @@ -37,9 +36,6 @@ export function rollupPluginPrebundleModules(env: Record): Plugi lodash: getNodeModuleDir('lodash-es'), // more optimal, but also solves esbuild incorrectly compiling lodash/_nodeUtil path: require.resolve('path-browserify'), }, - define: { - ...stringifyProcessEnvs(env), - }, plugins: [esbuildCommonjsPlugin()], }); }, @@ -64,7 +60,7 @@ function getModules() { // this is different to https://github.com/storybookjs/storybook/blob/v7.0.0/code/lib/builder-vite/src/optimizeDeps.ts#L7 // builder-vite bundles different dependencies for performance reasons -// we aim only at browserifying NodeJS dependencies (CommonJS/process.env/...) +// we aim only at browserifying dependencies which are CommonJS, or sometimes ESM with issues export const CANDIDATES = [ // @testing-library has ESM, but imports/exports are not working correctly between packages // specifically "@testing-library/user-event" has "dist/esm/utils/misc/getWindow.js" (see https://cdn.jsdelivr.net/npm/@testing-library/user-event@14.4.3/dist/esm/utils/misc/getWindow.js) @@ -82,7 +78,4 @@ export const CANDIDATES = [ // CommonJS module used in Storybook MJS files 'lodash/mapValues.js', - - // ESM, but uses `process.env.NODE_ENV` - 'tiny-invariant', ]; diff --git a/packages/storybook-builder/static/iframe-template.html b/packages/storybook-builder/static/iframe-template.html index c176909a9..d04fbf4b7 100644 --- a/packages/storybook-builder/static/iframe-template.html +++ b/packages/storybook-builder/static/iframe-template.html @@ -49,7 +49,9 @@ // We do this so that "module && module.hot" etc. in Storybook source code // doesn't fail (it will simply be disabled) window.module = undefined; + window.global = window; + window.process = { env: '[PROCESS_ENV HERE]' };