Skip to content

Commit b133b30

Browse files
committed
fix(storybook-builder): define only necessary process.env globally
1 parent 691d547 commit b133b30

File tree

5 files changed

+20
-15
lines changed

5 files changed

+20
-15
lines changed

.changeset/sweet-sloths-crash.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@web/storybook-builder': patch
3+
---
4+
5+
define only necessary process.env globally to improve security and decrease prebundling

packages/storybook-builder/src/generate-iframe-html.ts

+9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export async function generateIframeHtml(options: Options): Promise<string> {
1111
'utf-8',
1212
);
1313
const { configType, features, presets, serverChannelUrl } = options;
14+
const env = await presets.apply<Record<string, string>>('env');
1415
const frameworkOptions = await presets.apply<Record<string, any> | null>('frameworkOptions');
1516
const headHtmlSnippet = await presets.apply<PreviewHtml>('previewHead');
1617
const bodyHtmlSnippet = await presets.apply<PreviewHtml>('previewBody');
@@ -26,6 +27,14 @@ export async function generateIframeHtml(options: Options): Promise<string> {
2627
}));
2728
return (
2829
iframeHtmlTemplate
30+
.replace(
31+
`'[PROCESS_ENV HERE]'`,
32+
JSON.stringify({
33+
// IMPORTANT!!!
34+
// please do not include the entire "env" to prevent bundling and deploying of sensitive data
35+
NODE_ENV: env.NODE_ENV,
36+
}),
37+
)
2938
.replace('[CONFIG_TYPE HERE]', configType || '')
3039
.replace('[LOGLEVEL HERE]', logLevel || '')
3140
.replace(`'[FRAMEWORK_OPTIONS HERE]'`, JSON.stringify(frameworkOptions))

packages/storybook-builder/src/index.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s
6060
router.use('/sb-preview', express.static(previewDirOrigin, { immutable: true, maxAge: '5m' }));
6161
router.use(`/${PREBUNDLED_MODULES_DIR}`, express.static(resolve(`./${PREBUNDLED_MODULES_DIR}`)));
6262

63-
const env = await options.presets.apply<Record<string, string>>('env');
64-
6563
const wdsStorybookConfig: DevServerConfig = {
6664
nodeResolve: true,
6765
plugins: [
@@ -74,7 +72,7 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s
7472
}
7573
},
7674
},
77-
wdsPluginPrebundleModules(env),
75+
wdsPluginPrebundleModules(),
7876
wdsPluginStorybookBuilder(options),
7977
wdsPluginExternalGlobals(globalsNameReferenceMap || globals),
8078
],
@@ -130,8 +128,6 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s
130128
};
131129

132130
export const build: WdsBuilder['build'] = async ({ startTime, options }) => {
133-
const env = await options.presets.apply<Record<string, string>>('env');
134-
135131
const rollupDefaultOutputOptions: OutputOptions = {
136132
dir: options.outputDir,
137133
};
@@ -146,7 +142,7 @@ export const build: WdsBuilder['build'] = async ({ startTime, options }) => {
146142
extractAssets: false,
147143
}),
148144
rollupPluginNodeResolve(),
149-
rollupPluginPrebundleModules(env),
145+
rollupPluginPrebundleModules(),
150146
rollupPluginStorybookBuilder(options),
151147
rollupPluginExternalGlobals(globalsNameReferenceMap || globals),
152148
],

packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts

+2-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { stringifyProcessEnvs } from '@storybook/core-common';
21
import { build } from 'esbuild';
32
import { join } from 'path';
43
import type { Plugin } from 'rollup';
54
import { getNodeModuleDir } from './get-node-module-dir.js';
65

76
export const PREBUNDLED_MODULES_DIR = 'node_modules/.prebundled_modules';
87

9-
export function rollupPluginPrebundleModules(env: Record<string, string>): Plugin {
8+
export function rollupPluginPrebundleModules(): Plugin {
109
const modulePaths: Record<string, string> = {};
1110

1211
return {
@@ -37,9 +36,6 @@ export function rollupPluginPrebundleModules(env: Record<string, string>): Plugi
3736
lodash: getNodeModuleDir('lodash-es'), // more optimal, but also solves esbuild incorrectly compiling lodash/_nodeUtil
3837
path: require.resolve('path-browserify'),
3938
},
40-
define: {
41-
...stringifyProcessEnvs(env),
42-
},
4339
plugins: [esbuildCommonjsPlugin()],
4440
});
4541
},
@@ -64,7 +60,7 @@ function getModules() {
6460

6561
// this is different to https://github.com/storybookjs/storybook/blob/v7.0.0/code/lib/builder-vite/src/optimizeDeps.ts#L7
6662
// builder-vite bundles different dependencies for performance reasons
67-
// we aim only at browserifying NodeJS dependencies (CommonJS/process.env/...)
63+
// we aim only at browserifying dependencies which are CommonJS, or sometimes ESM with issues
6864
export const CANDIDATES = [
6965
// @testing-library has ESM, but imports/exports are not working correctly between packages
7066
// specifically "@testing-library/user-event" has "dist/esm/utils/misc/getWindow.js" (see https://cdn.jsdelivr.net/npm/@testing-library/[email protected]/dist/esm/utils/misc/getWindow.js)
@@ -82,7 +78,4 @@ export const CANDIDATES = [
8278

8379
// CommonJS module used in Storybook MJS files
8480
'lodash/mapValues.js',
85-
86-
// ESM, but uses `process.env.NODE_ENV`
87-
'tiny-invariant',
8881
];

packages/storybook-builder/static/iframe-template.html

+2
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@
4949
// We do this so that "module && module.hot" etc. in Storybook source code
5050
// doesn't fail (it will simply be disabled)
5151
window.module = undefined;
52+
5253
window.global = window;
54+
window.process = { env: '[PROCESS_ENV HERE]' };
5355
</script>
5456
<!-- [HEAD HTML SNIPPET HERE] -->
5557
</head>

0 commit comments

Comments
 (0)